diff --git a/pkg/admission/reservednames/admission.go b/pkg/admission/reservednames/admission.go index fc7c919a315..6846e97ca25 100644 --- a/pkg/admission/reservednames/admission.go +++ b/pkg/admission/reservednames/admission.go @@ -20,6 +20,7 @@ import ( "context" "io" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apiserver/pkg/admission" @@ -80,6 +81,11 @@ func NewReservedNames() *ReservedNames { tenancyv1alpha1.Kind("WorkspaceType"), tenancyv1alpha1.WorkspaceTypeReservedNames()..., ), + newReservedNameFn( + corev1.Resource("serviceaccounts"), + corev1.SchemeGroupVersion.WithKind("ServiceAccount").GroupKind(), + "kcp-rest", + ), }, } } diff --git a/pkg/authorization/bootstrap/policy.go b/pkg/authorization/bootstrap/policy.go index 15346b33e17..7bab92a6001 100644 --- a/pkg/authorization/bootstrap/policy.go +++ b/pkg/authorization/bootstrap/policy.go @@ -46,6 +46,17 @@ const ( SystemKcpWorkspaceAccessGroup = "system:kcp:workspace:access" ) +const ( + // SystemMastersGroup is the group inherited from k8s codebase - all powerful, all knowing! + // Users should not be added to this group. + SystemMastersGroup = "system:masters" +) + +const ( + // SystemServiceAccountDefaultRest is the default service account for fake rest client. + SystemServiceAccountDefaultRest = "system:serviceaccount:default:kcp-rest" +) + // ClusterRoleBindings return default rolebindings to the default roles. func clusterRoleBindings() []rbacv1.ClusterRoleBinding { return []rbacv1.ClusterRoleBinding{ diff --git a/pkg/proxy/server.go b/pkg/proxy/server.go index 0c6fe645bd6..16cd7257905 100644 --- a/pkg/proxy/server.go +++ b/pkg/proxy/server.go @@ -69,11 +69,13 @@ func NewServer(ctx context.Context, c CompletedConfig) (*Server, error) { return shardClient, nil }, ) - handler, err := NewHandler(ctx, s.CompletedConfig.Options, s.IndexController) if err != nil { return s, err } + // This must run before WithImpersonation upstream handler to + // catch this before user data is lost. + handler = server.WithImpersonationGatekeeper(handler) failedHandler := frontproxyfilters.NewUnauthorizedHandler() handler = frontproxyfilters.WithOptionalAuthentication( diff --git a/pkg/server/handler.go b/pkg/server/handler.go index 6c31f15f48f..89923d065e5 100644 --- a/pkg/server/handler.go +++ b/pkg/server/handler.go @@ -30,6 +30,7 @@ import ( "github.com/emicklei/go-restful/v3" jwt2 "gopkg.in/square/go-jose.v2/jwt" + authenticationv1 "k8s.io/api/authentication/v1" apiextensionsapiserver "k8s.io/apiextensions-apiserver/pkg/apiserver" "k8s.io/apiextensions-apiserver/pkg/kcp" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -47,6 +48,8 @@ import ( clientgotransport "k8s.io/client-go/transport" "k8s.io/klog/v2" "k8s.io/kubernetes/pkg/controlplane/apiserver/miniaggregator" + + authorizationbootstrap "github.com/kcp-dev/kcp/pkg/authorization/bootstrap" ) var ( @@ -253,6 +256,83 @@ func WithRequestIdentity(handler http.Handler) http.Handler { }) } +type privilege int + +const ( + superPrivileged privilege = iota + priviledged + unprivileged +) + +var ( + // specialGroups specify groups with special meaning kcp. Lower privilege (= higher number) + // cannot impersonate higher privilege levels. + specialGroups = map[string]privilege{ + authorizationbootstrap.SystemMastersGroup: superPrivileged, + authorizationbootstrap.SystemKcpAdminGroup: priviledged, + } +) + +// WithImpersonationGatekeeper checks the request for impersonations and validates them, +// if they are valid. If they are not, will return a 403. +// We check for impersonation in the request headers, early to avoid it being propagated to +// the backend services. +func WithImpersonationGatekeeper(handler http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + // Impersonation check is only done when impersonation is requested. + // And impersonations is only allowed for the users, who have metadata in the ctx. + impersonationUser := req.Header.Get(authenticationv1.ImpersonateUserHeader) + if len(impersonationUser) != 0 { + requestor, exists := request.UserFrom(req.Context()) + if !exists { + responsewriters.ErrorNegotiated( + apierrors.NewForbidden(schema.GroupResource{}, "", fmt.Errorf("impersonation is not allowed for the requestor")), + errorCodecs, schema.GroupVersion{}, w, req) + return + } + + if validImpersonation(requestor.GetGroups(), req.Header[authenticationv1.ImpersonateGroupHeader]) { + handler.ServeHTTP(w, req) + return + } + } + + responsewriters.ErrorNegotiated( + apierrors.NewForbidden(schema.GroupResource{}, "", fmt.Errorf("impersonation is not allowed for the requestor")), + errorCodecs, schema.GroupVersion{}, w, req) + }) +} + +// validImpersonation checks if a user with given privilege levels can impersonate requested groups. +func validImpersonation(userGroups []string, requestedGroups []string) bool { + userMaxPrivilege := unprivileged // Start with the lowest privilege + + // Determine the highest privilege the user has + for _, userGroup := range userGroups { + if priv, exists := specialGroups[userGroup]; exists && priv < userMaxPrivilege { + userMaxPrivilege = priv + } + } + + // Iterate through each requested group and verify if user's privilege allows impersonation + for _, requestedGroup := range requestedGroups { + requestedPrivilege, exists := specialGroups[requestedGroup] + + // If requested group is not recognized, treat it as the lowest privilege level + if !exists { + requestedPrivilege = unprivileged + } + + // Check if user's max privilege is less than or equal to the requested group's privilege + if userMaxPrivilege > requestedPrivilege { + return false // Deny impersonation if user's privilege is not sufficient + } + } + + // If all checks pass, impersonation is allowed + return true +} + func processResourceIdentity(req *http.Request, requestInfo *request.RequestInfo) (*http.Request, error) { if !requestInfo.IsResourceRequest { return req, nil diff --git a/pkg/server/handler_test.go b/pkg/server/handler_test.go index 0924e151a66..c0d662796d2 100644 --- a/pkg/server/handler_test.go +++ b/pkg/server/handler_test.go @@ -25,6 +25,8 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apiserver/pkg/endpoints/request" + + authorizationbootstrap "github.com/kcp-dev/kcp/pkg/authorization/bootstrap" ) func TestProcessResourceIdentity(t *testing.T) { @@ -153,3 +155,79 @@ func TestProcessResourceIdentity(t *testing.T) { }) } } + +func TestCheckImpersonation(t *testing.T) { + var systemUserGroup = "system:user:group" + nonExistingGroup := "non-existing-group" + specialGroups = map[string]privilege{ + authorizationbootstrap.SystemMastersGroup: superPrivileged, + authorizationbootstrap.SystemKcpAdminGroup: priviledged, + systemUserGroup: unprivileged, + } + + tests := []struct { + name string + userGroups []string + requestedGroups []string + expectedResult bool + }{ + { + name: "Single group - allowed", + userGroups: []string{authorizationbootstrap.SystemMastersGroup}, + requestedGroups: []string{authorizationbootstrap.SystemKcpAdminGroup}, + expectedResult: true, + }, + { + name: "Multiple groups - allowed", + userGroups: []string{authorizationbootstrap.SystemMastersGroup}, + requestedGroups: []string{authorizationbootstrap.SystemKcpAdminGroup, systemUserGroup}, + expectedResult: true, + }, + { + name: "Single group - not allowed", + userGroups: []string{authorizationbootstrap.SystemKcpAdminGroup}, + requestedGroups: []string{authorizationbootstrap.SystemMastersGroup}, + expectedResult: false, + }, + { + name: "Multiple groups - mixed permissions", + userGroups: []string{authorizationbootstrap.SystemKcpAdminGroup}, + requestedGroups: []string{authorizationbootstrap.SystemMastersGroup, systemUserGroup}, + expectedResult: false, + }, + { + name: "Multiple groups - lower permissions only", + userGroups: []string{systemUserGroup}, + requestedGroups: []string{authorizationbootstrap.SystemKcpAdminGroup, authorizationbootstrap.SystemMastersGroup}, + expectedResult: false, + }, + { + name: "Empty user groups", + userGroups: []string{}, + requestedGroups: []string{authorizationbootstrap.SystemKcpAdminGroup}, + expectedResult: false, + }, + { + name: "Empty requested groups", + userGroups: []string{authorizationbootstrap.SystemMastersGroup}, + requestedGroups: []string{}, + expectedResult: true, + }, + { + name: "Unknown requested group", + userGroups: []string{authorizationbootstrap.SystemMastersGroup}, + requestedGroups: []string{nonExistingGroup}, + expectedResult: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := validImpersonation(tt.userGroups, tt.requestedGroups) + if result != tt.expectedResult { + t.Errorf("checkImpersonation(%v, %v) = %v; want %v", + tt.userGroups, tt.requestedGroups, result, tt.expectedResult) + } + }) + } +} diff --git a/pkg/virtual/apiexport/builder/build.go b/pkg/virtual/apiexport/builder/build.go index 16bf42603b7..c88ca302c1e 100644 --- a/pkg/virtual/apiexport/builder/build.go +++ b/pkg/virtual/apiexport/builder/build.go @@ -36,7 +36,7 @@ import ( "k8s.io/klog/v2" "github.com/kcp-dev/kcp/pkg/authorization" - "github.com/kcp-dev/kcp/pkg/authorization/bootstrap" + authorizationbootstrap "github.com/kcp-dev/kcp/pkg/authorization/bootstrap" virtualapiexportauth "github.com/kcp-dev/kcp/pkg/virtual/apiexport/authorizer" "github.com/kcp-dev/kcp/pkg/virtual/apiexport/controllers/apireconciler" "github.com/kcp-dev/kcp/pkg/virtual/apiexport/schemas" @@ -116,8 +116,8 @@ func BuildVirtualWorkspace( impersonationConfig := rest.CopyConfig(cfg) impersonationConfig.Impersonate = rest.ImpersonationConfig{ - UserName: "system:serviceaccount:default:rest", - Groups: []string{bootstrap.SystemKcpAdminGroup}, + UserName: authorizationbootstrap.SystemServiceAccountDefaultRest, + Groups: []string{authorizationbootstrap.SystemKcpAdminGroup}, Extra: map[string][]string{ serviceaccount.ClusterNameKey: {cluster.Name.Path().String()}, }, diff --git a/test/e2e/authorizer/impersonate_test.go b/test/e2e/authorizer/impersonate_test.go new file mode 100644 index 00000000000..4ed63a586b1 --- /dev/null +++ b/test/e2e/authorizer/impersonate_test.go @@ -0,0 +1,112 @@ +/* +Copyright 2024 The KCP Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package authorizer + +import ( + "context" + "testing" + "time" + + kcpkubernetesclientset "github.com/kcp-dev/client-go/kubernetes" + "github.com/stretchr/testify/require" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/rest" + + "github.com/kcp-dev/kcp/test/e2e/framework" +) + +func TestImpersonation(t *testing.T) { + t.Parallel() + framework.Suite(t, "control-plane") + + ctx, cancelFn := context.WithCancel(context.Background()) + t.Cleanup(cancelFn) + + server := framework.SharedKcpServer(t) + cfg := server.BaseConfig(t) + user1workspace, _ := framework.NewOrganizationFixture(t, server) + user2workspace, _ := framework.NewOrganizationFixture(t, server) + + kubeClusterClient, err := kcpkubernetesclientset.NewForConfig(cfg) + require.NoError(t, err) + + t.Log("Make user-1 an admin of the workspace1") + framework.AdmitWorkspaceAccess(ctx, t, kubeClusterClient, user1workspace, []string{"user-1"}, nil, true) + user1Cfg := framework.StaticTokenUserConfig("user-1", cfg) + user1KubeClusterClient, err := kcpkubernetesclientset.NewForConfig(user1Cfg) + require.NoError(t, err) + + t.Log("User-1 should be able to read secrets") + require.Eventually(t, func() bool { + _, err := user1KubeClusterClient.Cluster(user1workspace).CoreV1().Secrets("default").List(ctx, metav1.ListOptions{}) + return err == nil + }, wait.ForeverTestTimeout, time.Millisecond*100, "user-1 should be able to read secrets") + + t.Log("Make user-2 an admin of the worksapce2") + framework.AdmitWorkspaceAccess(ctx, t, kubeClusterClient, user2workspace, []string{"user-2"}, nil, true) + user2Cfg := framework.StaticTokenUserConfig("user-2", cfg) + user2KubeClusterClient, err := kcpkubernetesclientset.NewForConfig(user2Cfg) + require.NoError(t, err) + + t.Log("User-2 should be able to read secrets") + require.Eventually(t, func() bool { + _, err := user2KubeClusterClient.Cluster(user2workspace).CoreV1().Secrets("default").List(ctx, metav1.ListOptions{}) + return err == nil + }, wait.ForeverTestTimeout, time.Millisecond*100, "user-2 should be able to read secrets") + + t.Log("User-1 should NOT be able to read secrets in user-2 workspace") + require.Eventually(t, func() bool { + _, err := user1KubeClusterClient.Cluster(user2workspace).CoreV1().Secrets("default").List(ctx, metav1.ListOptions{}) + return apierrors.IsForbidden(err) + }, wait.ForeverTestTimeout, time.Millisecond*100, "User-1 should NOT be able to read secrets in user-2 workspace") + + t.Log("Make user-1 impersonate user-2 as system:masters") + impersonationConfig := rest.CopyConfig(user1Cfg) + impersonationConfig.Impersonate = rest.ImpersonationConfig{ + UserName: "user-2", + Groups: []string{"system:masters"}, + } + impersonatedClient, err := kcpkubernetesclientset.NewForConfig(impersonationConfig) + require.NoError(t, err) + + t.Log("As user-1 with system:masters, we should NOT be able to read secrets in user 2 workspace") + require.Eventually(t, func() bool { + _, err := impersonatedClient.Cluster(user2workspace).CoreV1().Secrets("default").List(ctx, metav1.ListOptions{}) + return apierrors.IsForbidden(err) + }, wait.ForeverTestTimeout, time.Millisecond*100, "as user-1 with system:masters, we should NOT be able to read secrets") + + t.Log("Make user-1 impersonate system:serviceaccount:default:kcp-rest as system:kcp:admin") + impersonationKCPAdminConfig := rest.CopyConfig(user1Cfg) + impersonationKCPAdminConfig.Impersonate = rest.ImpersonationConfig{ + UserName: "system:serviceaccount:default:kcp-rest", + Groups: []string{"system:kcp:admin"}, + } + impersonatedKCPAdminClient, err := kcpkubernetesclientset.NewForConfig(impersonationKCPAdminConfig) + require.NoError(t, err) + + t.Log("As user-1 with kcp-admin:system:kcp:admin, we should NOT be able to read secrets in user1 workspace") + require.Eventually(t, func() bool { + _, err := impersonatedKCPAdminClient.Cluster(user2workspace).CoreV1().Secrets("default").List(ctx, metav1.ListOptions{}) + return apierrors.IsForbidden(err) + }, wait.ForeverTestTimeout, time.Millisecond*100, "as user-1 with kcp-admin:system:kcp:admin, we should NOT be able to read secrets") + + // TODO: Add test to check for warrant impersonation once https://github.com/kcp-dev/kcp/pull/3156/ + // is merged and the feature is available in the API server. +}