pkg/virtual/apiexport: impersonate requests

Currently, privilege escalation can be provoked when creating APIBindings
for exported resources.

This fixes it by impersonating virtual API export requests with a
service account that is bound to the requested workspace.

pkg/virtual/apiexport/builder/build.go

@@ -27,14 +27,17 @@ import (
 	"github.com/kcp-dev/logicalcluster/v3"
 
 	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apiserver/pkg/authentication/serviceaccount"
 	"k8s.io/apiserver/pkg/authorization/authorizer"
 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
 	genericapiserver "k8s.io/apiserver/pkg/server"
+	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/klog/v2"
 
 	apisv1alpha1 "github.com/kcp-dev/kcp/pkg/apis/apis/v1alpha1"
 	"github.com/kcp-dev/kcp/pkg/authorization"
+	"github.com/kcp-dev/kcp/pkg/authorization/bootstrap"
 	kcpclientset "github.com/kcp-dev/kcp/pkg/client/clientset/versioned/cluster"
 	kcpinformers "github.com/kcp-dev/kcp/pkg/client/informers/externalversions"
 	virtualapiexportauth "github.com/kcp-dev/kcp/pkg/virtual/apiexport/authorizer"
@@ -53,8 +56,8 @@ const VirtualWorkspaceName string = "apiexport"
 
 func BuildVirtualWorkspace(
 	rootPathPrefix string,
+	cfg *rest.Config,
 	kubeClusterClient, deepSARClient kcpkubernetesclientset.ClusterInterface,
-	dynamicClusterClient kcpdynamic.ClusterInterface,
 	kcpClusterClient kcpclientset.ClusterInterface,
 	cachedKcpInformers kcpinformers.SharedInformerFactory,
 ) ([]rootapiserver.NamedVirtualWorkspace, error) {
@@ -86,6 +89,37 @@ func BuildVirtualWorkspace(
 		}),
 
 		BootstrapAPISetManagement: func(mainConfig genericapiserver.CompletedConfig) (apidefinition.APIDefinitionSetGetter, error) {
+			dynamicClient, err := kcpdynamic.NewForConfig(cfg)
+			if err != nil {
+				return nil, fmt.Errorf("error creating privileged dynamic kcp client: %w", err)
+			}
+
+			impersonatedDynamicClientGetter := func(ctx context.Context) (kcpdynamic.ClusterInterface, error) {
+				cluster, err := genericapirequest.ValidClusterFrom(ctx)
+				if err != nil {
+					return nil, fmt.Errorf("error getting valid cluster from context: %w", err)
+				}
+
+				// Wildcard requests cannot be impersonated against a concrete cluster.
+				if cluster.Wildcard {
+					return dynamicClient, nil
+				}
+
+				impersonationConfig := rest.CopyConfig(cfg)
+				impersonationConfig.Impersonate = rest.ImpersonationConfig{
+					UserName: "system:serviceaccount:default:rest",
+					Groups:   []string{bootstrap.SystemKcpAdminGroup},
+					Extra: map[string][]string{
+						serviceaccount.ClusterNameKey: {cluster.Name.Path().String()},
+					},
+				}
+				impersonatedClient, err := kcpdynamic.NewForConfig(impersonationConfig)
+				if err != nil {
+					return nil, fmt.Errorf("error generating dynamic client: %w", err)
+				}
+				return impersonatedClient, nil
+			}
+
 			apiReconciler, err := apireconciler.NewAPIReconciler(
 				kcpClusterClient,
 				cachedKcpInformers.Apis().V1alpha1().APIResourceSchemas(),
@@ -100,7 +134,7 @@ func BuildVirtualWorkspace(
 						})
 					}
 
-					storageBuilder := provideDelegatingRestStorage(ctx, dynamicClusterClient, identityHash, wrapper)
+					storageBuilder := provideDelegatingRestStorage(ctx, impersonatedDynamicClientGetter, identityHash, wrapper)
 					def, err := apiserver.CreateServingInfoFor(mainConfig, apiResourceSchema, version, storageBuilder)
 					if err != nil {
 						cancelFn()
@@ -112,7 +146,7 @@ func BuildVirtualWorkspace(
 					}, nil
 				},
 				func(ctx context.Context, clusterName logicalcluster.Name, apiExportName string) (apidefinition.APIDefinition, error) {
-					restProvider, err := provideAPIExportFilteredRestStorage(ctx, dynamicClusterClient, clusterName, apiExportName)
+					restProvider, err := provideAPIExportFilteredRestStorage(ctx, impersonatedDynamicClientGetter, clusterName, apiExportName)
 					if err != nil {
 						return nil, err
 					}

pkg/virtual/apiexport/builder/forwarding.go

@@ -20,7 +20,6 @@ import (
 	"context"
 	"fmt"
 
-	kcpdynamic "github.com/kcp-dev/client-go/dynamic"
 	"github.com/kcp-dev/logicalcluster/v3"
 
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
@@ -39,7 +38,7 @@ import (
 	registry "github.com/kcp-dev/kcp/pkg/virtual/framework/forwardingregistry"
 )
 
-func provideAPIExportFilteredRestStorage(ctx context.Context, clusterClient kcpdynamic.ClusterInterface, clusterName logicalcluster.Name, exportName string) (apiserver.RestProviderFunc, error) {
+func provideAPIExportFilteredRestStorage(ctx context.Context, dynamicClusterClientFunc registry.DynamicClusterClientFunc, clusterName logicalcluster.Name, exportName string) (apiserver.RestProviderFunc, error) {
 	labelSelector := map[string]string{
 		apisv1alpha1.InternalAPIBindingExportLabelKey: permissionclaims.ToAPIBindingExportLabelValue(clusterName, exportName),
 	}
@@ -48,11 +47,11 @@ func provideAPIExportFilteredRestStorage(ctx context.Context, clusterClient kcpd
 		return nil, fmt.Errorf("unable to create a selector from the provided labels")
 	}
 
-	return registry.ProvideReadOnlyRestStorage(ctx, clusterClient, registry.WithStaticLabelSelector(requirements), nil)
+	return registry.ProvideReadOnlyRestStorage(ctx, dynamicClusterClientFunc, registry.WithStaticLabelSelector(requirements), nil)
 }
 
 // provideDelegatingRestStorage returns a forwarding storage build function, with an optional storage wrapper e.g. to add label based filtering.
-func provideDelegatingRestStorage(ctx context.Context, clusterClient kcpdynamic.ClusterInterface, apiExportIdentityHash string, wrapper registry.StorageWrapper) apiserver.RestProviderFunc {
+func provideDelegatingRestStorage(ctx context.Context, dynamicClusterClientFunc registry.DynamicClusterClientFunc, apiExportIdentityHash string, wrapper registry.StorageWrapper) apiserver.RestProviderFunc {
 	return func(resource schema.GroupVersionResource, kind schema.GroupVersionKind, listKind schema.GroupVersionKind, typer runtime.ObjectTyper, tableConvertor rest.TableConvertor, namespaceScoped bool, schemaValidator *validate.SchemaValidator, subresourcesSchemaValidator map[string]*validate.SchemaValidator, structuralSchema *structuralschema.Structural) (mainStorage rest.Storage, subresourceStorages map[string]rest.Storage) {
 		statusSchemaValidate, statusEnabled := subresourcesSchemaValidator["status"]
 
@@ -86,7 +85,7 @@ func provideDelegatingRestStorage(ctx context.Context, clusterClient kcpdynamic.
 			nil,
 			tableConvertor,
 			nil,
-			clusterClient,
+			dynamicClusterClientFunc,
 			nil,
 			wrapper,
 		)

pkg/virtual/apiexport/options/options.go

@@ -19,7 +19,6 @@ package options
 import (
 	"path"
 
-	kcpdynamic "github.com/kcp-dev/client-go/dynamic"
 	kcpkubernetesclientset "github.com/kcp-dev/client-go/kubernetes"
 	"github.com/spf13/pflag"
 
@@ -67,14 +66,10 @@ func (o *APIExport) NewVirtualWorkspaces(
 	if err != nil {
 		return nil, err
 	}
-	dynamicClusterClient, err := kcpdynamic.NewForConfig(config)
-	if err != nil {
-		return nil, err
-	}
 	deepSARClient, err := kcpkubernetesclientset.NewForConfig(authorization.WithDeepSARConfig(rest.CopyConfig(config)))
 	if err != nil {
 		return nil, err
 	}
 
-	return builder.BuildVirtualWorkspace(path.Join(rootPathPrefix, builder.VirtualWorkspaceName), kubeClusterClient, deepSARClient, dynamicClusterClient, kcpClusterClient, cachedKcpInformers)
+	return builder.BuildVirtualWorkspace(path.Join(rootPathPrefix, builder.VirtualWorkspaceName), config, kubeClusterClient, deepSARClient, kcpClusterClient, cachedKcpInformers)
 }

pkg/virtual/framework/forwardingregistry/rest.go

@@ -19,8 +19,6 @@ package forwardingregistry
 import (
 	"context"
 
-	kcpdynamic "github.com/kcp-dev/client-go/dynamic"
-
 	structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
 	"k8s.io/apiextensions-apiserver/pkg/registry/customresource"
 	"k8s.io/apimachinery/pkg/api/validation/path"
@@ -71,7 +69,7 @@ func NewStorage(
 	categories []string,
 	tableConvertor rest.TableConvertor,
 	replicasPathMapping fieldmanager.ResourcePathMappings,
-	dynamicClusterClient kcpdynamic.ClusterInterface,
+	dynamicClusterClientFunc DynamicClusterClientFunc,
 	patchConflictRetryBackoff *wait.Backoff,
 	wrapper StorageWrapper,
 ) (mainStorage, statusStorage *StoreFuncs) {
@@ -99,7 +97,7 @@ func NewStorage(
 		factory, listFactory, destroyer,
 		strategy, tableConvertor,
 		resource, apiExportIdentityHash, categories,
-		dynamicClusterClient, []string{}, *patchConflictRetryBackoff, ctx.Done(),
+		dynamicClusterClientFunc, []string{}, *patchConflictRetryBackoff, ctx.Done(),
 	)
 	if wrapper != nil {
 		wrapper.Decorate(resource.GroupResource(), store)
@@ -110,7 +108,7 @@ func NewStorage(
 		factory, listFactory, destroyer,
 		statusStrategy, tableConvertor,
 		resource, apiExportIdentityHash, categories,
-		dynamicClusterClient, []string{"status"}, *patchConflictRetryBackoff, ctx.Done(),
+		dynamicClusterClientFunc, []string{"status"}, *patchConflictRetryBackoff, ctx.Done(),
 	)
 	delegateUpdate := statusStore.UpdaterFunc
 	statusStore.UpdaterFunc = func(ctx context.Context, name string, objInfo rest.UpdatedObjectInfo, createValidation rest.ValidateObjectFunc, updateValidation rest.ValidateObjectUpdateFunc, forceAllowCreate bool, options *metav1.UpdateOptions) (runtime.Object, bool, error) {
@@ -126,7 +124,7 @@ func NewStorage(
 
 // ProvideReadOnlyRestStorage returns a commonly used REST storage that forwards calls to a dynamic client,
 // but only for read-only requests.
-func ProvideReadOnlyRestStorage(ctx context.Context, clusterClient kcpdynamic.ClusterInterface, wrapper StorageWrapper, identities map[schema.GroupResource]string) (apiserver.RestProviderFunc, error) {
+func ProvideReadOnlyRestStorage(ctx context.Context, dynamicClusterClientFunc DynamicClusterClientFunc, wrapper StorageWrapper, identities map[schema.GroupResource]string) (apiserver.RestProviderFunc, error) {
 	return func(
 		resource schema.GroupVersionResource,
 		kind schema.GroupVersionKind,
@@ -162,7 +160,7 @@ func ProvideReadOnlyRestStorage(ctx context.Context, clusterClient kcpdynamic.Cl
 			nil,
 			tableConvertor,
 			nil,
-			clusterClient,
+			dynamicClusterClientFunc,
 			nil,
 			wrapper,
 		)

pkg/virtual/framework/forwardingregistry/rest_test.go

@@ -112,7 +112,7 @@ func newStorage(t *testing.T, clusterClient kcpdynamic.ClusterInterface, apiExpo
 		nil,
 		table,
 		nil,
-		clusterClient,
+		func(ctx context.Context) (kcpdynamic.ClusterInterface, error) { return clusterClient, nil },
 		patchConflictRetryBackoff,
 		forwardingregistry.StorageWrapperFunc(func(_ schema.GroupResource, store *forwardingregistry.StoreFuncs) {
 		}))

pkg/virtual/framework/forwardingregistry/store.go

@@ -65,6 +65,8 @@ type Strategy interface {
 	rest.ResetFieldsStrategy
 }
 
+type DynamicClusterClientFunc func(ctx context.Context) (kcpdynamic.ClusterInterface, error)
+
 func DefaultDynamicDelegatedStoreFuncs(
 	factory FactoryFunc,
 	listFactory ListFactoryFunc,
@@ -74,13 +76,13 @@ func DefaultDynamicDelegatedStoreFuncs(
 	resource schema.GroupVersionResource,
 	apiExportIdentityHash string,
 	categories []string,
-	dynamicClusterClient kcpdynamic.ClusterInterface,
+	dynamicClusterClientFunc DynamicClusterClientFunc,
 	subResources []string,
 	patchConflictRetryBackoff wait.Backoff,
 	stopWatchesCh <-chan struct{},
 ) *StoreFuncs {
-	client := clientGetter(dynamicClusterClient, strategy.NamespaceScoped(), resource, apiExportIdentityHash)
-	listerWatcher := listerWatcherGetter(dynamicClusterClient, strategy.NamespaceScoped(), resource, apiExportIdentityHash)
+	client := clientGetter(dynamicClusterClientFunc, strategy.NamespaceScoped(), resource, apiExportIdentityHash)
+	listerWatcher := listerWatcherGetter(dynamicClusterClientFunc, strategy.NamespaceScoped(), resource, apiExportIdentityHash)
 	s := &StoreFuncs{}
 	s.FactoryFunc = factory
 	s.ListFactoryFunc = listFactory
@@ -263,18 +265,24 @@ func DefaultDynamicDelegatedStoreFuncs(
 	return s
 }
 
-func clientGetter(dynamicClusterClient kcpdynamic.ClusterInterface, namespaceScoped bool, resource schema.GroupVersionResource, apiExportIdentityHash string) func(ctx context.Context) (dynamic.ResourceInterface, error) {
+func clientGetter(dynamicClusterClientFunc DynamicClusterClientFunc, namespaceScoped bool, resource schema.GroupVersionResource, apiExportIdentityHash string) func(ctx context.Context) (dynamic.ResourceInterface, error) {
 	return func(ctx context.Context) (dynamic.ResourceInterface, error) {
 		cluster, err := genericapirequest.ValidClusterFrom(ctx)
 		if err != nil {
 			return nil, apiErrorBadRequest(err)
 		}
+
 		gvr := resource
 		clusterName := cluster.Name
 		if apiExportIdentityHash != "" {
 			gvr.Resource += ":" + apiExportIdentityHash
 		}
 
+		dynamicClusterClient, err := dynamicClusterClientFunc(ctx)
+		if err != nil {
+			return nil, fmt.Errorf("error generating dynamic client: %w", err)
+		}
+
 		if namespaceScoped {
 			if namespace, ok := genericapirequest.NamespaceFrom(ctx); ok {
 				return dynamicClusterClient.Cluster(clusterName.Path()).Resource(gvr).Namespace(namespace), nil
@@ -292,7 +300,7 @@ type listerWatcher interface {
 	Watch(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error)
 }
 
-func listerWatcherGetter(dynamicClusterClient kcpdynamic.ClusterInterface, namespaceScoped bool, resource schema.GroupVersionResource, apiExportIdentityHash string) func(ctx context.Context) (listerWatcher, error) {
+func listerWatcherGetter(dynamicClusterClientFunc DynamicClusterClientFunc, namespaceScoped bool, resource schema.GroupVersionResource, apiExportIdentityHash string) func(ctx context.Context) (listerWatcher, error) {
 	return func(ctx context.Context) (listerWatcher, error) {
 		cluster, err := genericapirequest.ValidClusterFrom(ctx)
 		if err != nil {
@@ -304,6 +312,11 @@ func listerWatcherGetter(dynamicClusterClient kcpdynamic.ClusterInterface, names
 		}
 		namespace, namespaceSet := genericapirequest.NamespaceFrom(ctx)
 
+		dynamicClusterClient, err := dynamicClusterClientFunc(ctx)
+		if err != nil {
+			return nil, fmt.Errorf("error generating dynamic client: %w", err)
+		}
+
 		switch {
 		case cluster.Wildcard:
 			if namespaceScoped && namespaceSet && namespace != metav1.NamespaceAll {

pkg/virtual/initializingworkspaces/builder/forwarding.go

@@ -72,7 +72,7 @@ func filteredLogicalClusterReadOnlyRestStorage(
 
 	return registry.ProvideReadOnlyRestStorage(
 		ctx,
-		clusterClient,
+		func(ctx context.Context) (kcpdynamic.ClusterInterface, error) { return clusterClient, nil },
 		registry.WithStaticLabelSelector(requirements),
 		nil,
 	)
@@ -130,7 +130,7 @@ func delegatingLogicalClusterReadOnlyRestStorage(
 			nil,
 			tableConvertor,
 			nil,
-			clusterClient,
+			func(ctx context.Context) (kcpdynamic.ClusterInterface, error) { return clusterClient, nil },
 			nil,
 			&registry.StorageWrappers{
 				registry.WithStaticLabelSelector(requirements),

pkg/virtual/syncer/builder/forwarding.go

@@ -71,7 +71,7 @@ func NewSyncerRestProvider(ctx context.Context, clusterClient kcpdynamic.Cluster
 			nil,
 			tableConvertor,
 			nil,
-			clusterClient,
+			func(ctx context.Context) (kcpdynamic.ClusterInterface, error) { return clusterClient, nil },
 			nil,
 			wrapper,
 		)
@@ -167,7 +167,7 @@ func NewUpSyncerRestProvider(ctx context.Context, clusterClient kcpdynamic.Clust
 			nil,
 			tableConvertor,
 			nil,
-			clusterClient,
+			func(ctx context.Context) (kcpdynamic.ClusterInterface, error) { return clusterClient, nil },
 			nil,
 			wrapper,
 		)

test/e2e/virtual/apiexport/authorizer_test.go

@@ -479,7 +479,9 @@ func apply(t *testing.T, ctx context.Context, workspace logicalcluster.Path, cfg
 		}()
 
 		mapping, err := mapper.RESTMapping(gvk.GroupKind(), gvk.Version)
-		require.NoError(t, err)
+		if err != nil {
+			return fmt.Errorf("error getting REST mapping: %w", err)
+		}
 
 		dynamicClient, err := kcpdynamic.NewForConfig(cfg)
 		require.NoError(t, err)