From dfc508ef58a6d3fcd967a3f60c1104802db3d081 Mon Sep 17 00:00:00 2001 From: Kyle Lape Date: Fri, 30 Aug 2024 11:57:17 -0500 Subject: [PATCH 1/3] ROX-25912: Make Kube SA token m2m auth config dynamic This allows customers to add their own role mappings for this config. If a customer breaks config-controller auth, they can simply restart Central to get it back to a working state. Adds a function configureConfigControllerAccess to the initialization of the token exchangers that ensures the config-controller has access to Central APIs via k8s service account token m2m auth. What this function does in plain english: * See if any existing m2m configs from the db are for the kube sa issuer * If yes, make sure the role mapping for config-controller is present * If no, create a new m2m config for kube sa issuer like we do today and save it to the db --- central/auth/datastore/datastore_impl.go | 97 +++++++++--- central/auth/datastore/datastore_impl_test.go | 139 +++++++++++++++++- 2 files changed, 206 insertions(+), 30 deletions(-) diff --git a/central/auth/datastore/datastore_impl.go b/central/auth/datastore/datastore_impl.go index 40903dd9629d1..e638e70925f2d 100644 --- a/central/auth/datastore/datastore_impl.go +++ b/central/auth/datastore/datastore_impl.go @@ -9,12 +9,12 @@ import ( "github.com/stackrox/rox/central/auth/m2m" "github.com/stackrox/rox/central/auth/store" "github.com/stackrox/rox/generated/storage" - "github.com/stackrox/rox/pkg/defaults/accesscontrol" "github.com/stackrox/rox/pkg/env" pgPkg "github.com/stackrox/rox/pkg/postgres" "github.com/stackrox/rox/pkg/sac" "github.com/stackrox/rox/pkg/sac/resources" "github.com/stackrox/rox/pkg/sync" + "github.com/stackrox/rox/pkg/uuid" ) var ( @@ -53,14 +53,19 @@ func (d *datastoreImpl) listAuthM2MConfigsNoLock(ctx context.Context) ([]*storag } func (d *datastoreImpl) UpsertAuthM2MConfig(ctx context.Context, + config *storage.AuthMachineToMachineConfig) (*storage.AuthMachineToMachineConfig, error) { + d.mutex.Lock() + defer d.mutex.Unlock() + + return d.UpsertAuthM2MConfigNoLock(ctx, config) +} + +func (d *datastoreImpl) UpsertAuthM2MConfigNoLock(ctx context.Context, config *storage.AuthMachineToMachineConfig) (*storage.AuthMachineToMachineConfig, error) { if err := sac.VerifyAuthzOK(accessSAC.WriteAllowed(ctx)); err != nil { return nil, err } - d.mutex.Lock() - defer d.mutex.Unlock() - // Get the existing stored config, if any. storedConfig, exists, err := d.getAuthM2MConfigNoLock(ctx, config.GetId()) if err != nil { @@ -142,28 +147,13 @@ func (d *datastoreImpl) InitializeTokenExchangers() error { configs, err := d.listAuthM2MConfigsNoLock(ctx) if err != nil { - return err + return fmt.Errorf("Failed to list auth m2m configs: %w", err) } - kubeSAIssuer, err := d.issuerFetcher.GetServiceAccountIssuer() - if err != nil { - return fmt.Errorf("Failed to get service account issuer: %w", err) + if err = d.configureConfigControllerAccess(configs); err != nil { + return fmt.Errorf("Failed to configure config controller access: %w", err) } - // Unconditionally add K8s service account exchanger. - // This is required for config-controller auth. - configs = append(configs, &storage.AuthMachineToMachineConfig{ - Type: storage.AuthMachineToMachineConfig_KUBE_SERVICE_ACCOUNT, - TokenExpirationDuration: "1m", - Mappings: []*storage.AuthMachineToMachineConfig_Mapping{{ - // sub stands for "subject identifier", a required field on an OIDC token - Key: "sub", - ValueExpression: configControllerServiceAccountName, - Role: accesscontrol.ConfigController, - }}, - Issuer: kubeSAIssuer, - }) - tokenExchangerErrors := []error{} for _, config := range configs { if err := d.set.UpsertTokenExchanger(ctx, config); err != nil { @@ -174,6 +164,69 @@ func (d *datastoreImpl) InitializeTokenExchangers() error { return errors.Join(tokenExchangerErrors...) } +// configureConfigControllerAccess ensures the config-controller has access to Central APIs via k8s service account token m2m auth +// +// What this function does in plain english: +// +// * See if any existing m2m configs from the db are for the kube sa issuer +// * If yes, make sure the role mapping for config-controller is present +// * If no, create a new m2m config for kube sa issuer like we do today and save it to the db +// +// This allows customers to add their own role mappings for this config. +// If a customer breaks config-controller auth, they can simply restart Central to get it back to a working state. +func (d *datastoreImpl) configureConfigControllerAccess(configs []*storage.AuthMachineToMachineConfig) error { + kubeSAIssuer, err := d.issuerFetcher.GetServiceAccountIssuer() + if err != nil { + return fmt.Errorf("Failed to get service account issuer: %w", err) + } + + var kubeSAConfig *storage.AuthMachineToMachineConfig + + for _, config := range configs { + if config.Issuer == kubeSAIssuer { + kubeSAConfig = config + break + } + } + + if kubeSAConfig == nil { + kubeSAConfig = &storage.AuthMachineToMachineConfig{ + Id: uuid.NewV4().String(), + Type: storage.AuthMachineToMachineConfig_KUBE_SERVICE_ACCOUNT, + TokenExpirationDuration: "1h", + Mappings: []*storage.AuthMachineToMachineConfig_Mapping{}, + Issuer: kubeSAIssuer, + } + } + + var mappingFound bool + for _, mapping := range kubeSAConfig.Mappings { + if mapping.Key == "sub" && mapping.ValueExpression == configControllerServiceAccountName && mapping.Role == "Configuration Controller" { + mappingFound = true + break + } + } + + if !mappingFound { + kubeSAConfig.Mappings = append(kubeSAConfig.Mappings, &storage.AuthMachineToMachineConfig_Mapping{ + Key: "sub", + ValueExpression: configControllerServiceAccountName, + Role: "Configuration Controller", + }) + } + + ctx := sac.WithGlobalAccessScopeChecker(context.Background(), sac.AllowFixedScopes( + sac.AccessModeScopeKeys(storage.Access_READ_WRITE_ACCESS), sac.ResourceScopeKeys(resources.Access))) + + // This inits the token exchanger, too + _, err = d.UpsertAuthM2MConfigNoLock(ctx, kubeSAConfig) + if err != nil { + return fmt.Errorf("Failed to upsert auth m2m config: %w", err) + } + + return err +} + // wrapRollback wraps the error with potential rollback errors. // In the case the giving config is not nil, it will attempt to rollback the exchanger in the set in addition to // rolling back the transaction. diff --git a/central/auth/datastore/datastore_impl_test.go b/central/auth/datastore/datastore_impl_test.go index 4c6bb351a600d..cdbffa3705a77 100644 --- a/central/auth/datastore/datastore_impl_test.go +++ b/central/auth/datastore/datastore_impl_test.go @@ -4,6 +4,7 @@ package datastore import ( "context" + "fmt" "testing" "github.com/stackrox/rox/central/auth/m2m/mocks" @@ -13,6 +14,7 @@ import ( rolePostgresStore "github.com/stackrox/rox/central/role/store/role/postgres" accessScopePostgresStore "github.com/stackrox/rox/central/role/store/simpleaccessscope/postgres" "github.com/stackrox/rox/generated/storage" + "github.com/stackrox/rox/pkg/env" "github.com/stackrox/rox/pkg/errox" "github.com/stackrox/rox/pkg/postgres/pgtest" "github.com/stackrox/rox/pkg/sac" @@ -24,14 +26,15 @@ import ( ) const ( - testRole1 = "New-Admin" - testRole2 = "Super-Admin" - testRole3 = "Super Continuous Integration" - testIssuer = "https://localhost" + testRole1 = "New-Admin" + testRole2 = "Super-Admin" + testRole3 = "Super Continuous Integration" + configController = "Configuration Controller" + testIssuer = "https://localhost" ) var ( - testRoles = set.NewFrozenStringSet(testRole1, testRole2, testRole3) + testRoles = set.NewFrozenStringSet(testRole1, testRole2, testRole3, configController) ) func TestAuthDatastorePostgres(t *testing.T) { @@ -68,7 +71,7 @@ func (s *datastorePostgresTestSuite) SetupTest() { return nil, nil }) - s.addRoles() + s.addRoles(roleStore) controller := gomock.NewController(s.T()) s.mockSet = mocks.NewMockTokenExchangerSet(controller) @@ -93,11 +96,131 @@ func (s *datastorePostgresTestSuite) TestKubeServiceAccountConfig() { issuerFetcher.EXPECT().GetServiceAccountIssuer().Return(testIssuer, nil).Times(1) mockSet.EXPECT().UpsertTokenExchanger(gomock.Any(), gomock.Any()).Return(nil).Times(1) + mockSet.EXPECT().GetTokenExchanger(gomock.Any()).Return(nil, false).Times(1) authDataStore := New(store, mockSet, issuerFetcher) s.NoError(authDataStore.InitializeTokenExchangers()) } +func getTestConfig(configs []*storage.AuthMachineToMachineConfig) *storage.AuthMachineToMachineConfig { + for _, config := range configs { + if config.Issuer == testIssuer { + return config + } + } + return nil +} + +type authDataStoreMutatorFunc func(authDataStore DataStore) +type authDataStoreValidatorFunc func(configs []*storage.AuthMachineToMachineConfig) + +func (s *datastorePostgresTestSuite) kubeSAM2MConfig(authDataStoreMutator authDataStoreMutatorFunc, authDataStoreValidator authDataStoreValidatorFunc) { + controller := gomock.NewController(s.T()) + defer controller.Finish() + store := store.New(s.pool.DB) + + mockSet := mocks.NewMockTokenExchangerSet(controller) + issuerFetcher := mocks.NewMockServiceAccountIssuerFetcher(controller) + + issuerFetcher.EXPECT().GetServiceAccountIssuer().Return(testIssuer, nil).Times(2) + mockSet.EXPECT().UpsertTokenExchanger(gomock.Any(), gomock.Any()).Return(nil).AnyTimes() + mockSet.EXPECT().GetTokenExchanger(gomock.Any()).Return(nil, false).AnyTimes() + mockSet.EXPECT().RemoveTokenExchanger(gomock.AssignableToTypeOf("")).Return(nil).AnyTimes() + + authDataStore := New(store, mockSet, issuerFetcher) + s.NoError(authDataStore.InitializeTokenExchangers()) + authDataStoreMutator(authDataStore) + + // Emulate restarting Central by creating a new data store and token exchanger set + mockSet = mocks.NewMockTokenExchangerSet(controller) + mockSet.EXPECT().UpsertTokenExchanger(gomock.Any(), gomock.Any()).Return(nil).AnyTimes() + mockSet.EXPECT().GetTokenExchanger(gomock.Any()).Return(nil, false).AnyTimes() + + authDataStore = New(store, mockSet, issuerFetcher) + s.NoError(authDataStore.InitializeTokenExchangers()) + + configs, err := authDataStore.ListAuthM2MConfigs(s.ctx) + s.NoError(err) + authDataStoreValidator(configs) +} + +func (s *datastorePostgresTestSuite) TestKubeSAM2MConfigPersistsAfterDelete() { + authDataStoreMutator := func(authDataStore DataStore) { + configs, err := authDataStore.ListAuthM2MConfigs(s.ctx) + s.NoError(err) + kubeSAConfig := getTestConfig(configs) + s.NotNil(kubeSAConfig) + s.NoError(authDataStore.RemoveAuthM2MConfig(s.ctx, kubeSAConfig.Id)) + } + authDataStoreValidator := func(configs []*storage.AuthMachineToMachineConfig) { + kubeSAConfig := getTestConfig(configs) + s.NotNil(kubeSAConfig) + s.Equal(1, len(kubeSAConfig.Mappings)) + s.Equal("sub", kubeSAConfig.Mappings[0].Key) + s.Equal("Configuration Controller", kubeSAConfig.Mappings[0].Role) + s.Contains(kubeSAConfig.Mappings[0].ValueExpression, "config-controller") + } + + s.kubeSAM2MConfig(authDataStoreMutator, authDataStoreValidator) +} + +func (s *datastorePostgresTestSuite) TestKubeSAM2MConfigPersistsAfterRestart() { + authDataStoreMutator := func(authDataStore DataStore) {} + authDataStoreValidator := func(configs []*storage.AuthMachineToMachineConfig) { + kubeSAConfig := getTestConfig(configs) + s.NotNil(kubeSAConfig) + s.Equal(1, len(kubeSAConfig.Mappings)) + s.Equal("sub", kubeSAConfig.Mappings[0].Key) + s.Equal("Configuration Controller", kubeSAConfig.Mappings[0].Role) + s.Contains(kubeSAConfig.Mappings[0].ValueExpression, "config-controller") + } + + s.kubeSAM2MConfig(authDataStoreMutator, authDataStoreValidator) +} + +func (s *datastorePostgresTestSuite) TestKubeSAM2MConfigPersistsAfterModification() { + testMapping := storage.AuthMachineToMachineConfig_Mapping{ + Key: "sub", + Role: testRole1, + ValueExpression: "system:serviceaccount:my-namespace:my-service-account", + } + configControllerMapping := storage.AuthMachineToMachineConfig_Mapping{ + Key: "sub", + Role: configController, + ValueExpression: fmt.Sprintf("system:serviceaccount:%s:config-controller", env.Namespace.Setting()), + } + + authDataStoreMutator := func(authDataStore DataStore) { + configs, err := authDataStore.ListAuthM2MConfigs(s.ctx) + s.NoError(err) + kubeSAConfig := getTestConfig(configs) + s.NotNil(kubeSAConfig) + kubeSAConfig.Mappings = []*storage.AuthMachineToMachineConfig_Mapping{&testMapping} + _, err = authDataStore.UpsertAuthM2MConfig(s.ctx, kubeSAConfig) + s.NoError(err) + } + authDataStoreValidator := func(configs []*storage.AuthMachineToMachineConfig) { + kubeSAConfig := getTestConfig(configs) + s.NotNil(kubeSAConfig) + s.Equal(2, len(kubeSAConfig.Mappings)) + for _, mapping := range []*storage.AuthMachineToMachineConfig_Mapping{&testMapping, &configControllerMapping} { + found := false + for _, kubeSAMapping := range kubeSAConfig.Mappings { + fmt.Printf("key=%s; role=%s; valueExpression=%s\n", kubeSAMapping.Key, kubeSAMapping.Role, kubeSAMapping.ValueExpression) + if kubeSAMapping.Key == mapping.Key && kubeSAMapping.Role == mapping.Role && kubeSAMapping.ValueExpression == mapping.ValueExpression { + found = true + break + } + } + if !found { + s.FailNowf("Failed to find role mapping", "key=%s; role=%s; valueExpression=%s", mapping.Key, mapping.Role, mapping.ValueExpression) + } + } + } + + s.kubeSAM2MConfig(authDataStoreMutator, authDataStoreValidator) +} + func (s *datastorePostgresTestSuite) TestAddFKConstraint() { config, err := s.authDataStore.UpsertAuthM2MConfig(s.ctx, &storage.AuthMachineToMachineConfig{ Id: "80c053c2-24a7-4b97-bd69-85b3a511241e", @@ -172,7 +295,7 @@ func (s *datastorePostgresTestSuite) TestAddUniqueIssuerConstraint() { s.ErrorIs(err, errox.AlreadyExists) } -func (s *datastorePostgresTestSuite) addRoles() { +func (s *datastorePostgresTestSuite) addRoles(roleStore rolePostgresStore.Store) { permSetID := uuid.NewV4().String() accessScopeID := uuid.NewV4().String() s.Require().NoError(s.roleDataStore.AddPermissionSet(s.ctx, &storage.PermissionSet{ @@ -193,7 +316,7 @@ func (s *datastorePostgresTestSuite) addRoles() { })) for _, role := range testRoles.AsSlice() { - s.Require().NoError(s.roleDataStore.AddRole(s.ctx, &storage.Role{ + s.Require().NoError(roleStore.Upsert(s.ctx, &storage.Role{ Name: role, Description: "test role", PermissionSetId: permSetID, From ffeba87b18c070c24666ff77f9a164f495717e4f Mon Sep 17 00:00:00 2001 From: Kyle Lape Date: Mon, 31 Mar 2025 08:49:11 -0500 Subject: [PATCH 2/3] Address PR comments --- central/auth/datastore/datastore_impl.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/central/auth/datastore/datastore_impl.go b/central/auth/datastore/datastore_impl.go index e638e70925f2d..d1e15c12b66f8 100644 --- a/central/auth/datastore/datastore_impl.go +++ b/central/auth/datastore/datastore_impl.go @@ -147,11 +147,11 @@ func (d *datastoreImpl) InitializeTokenExchangers() error { configs, err := d.listAuthM2MConfigsNoLock(ctx) if err != nil { - return fmt.Errorf("Failed to list auth m2m configs: %w", err) + return pkgErrors.Wrap(err, "Failed to list auth m2m configs") } if err = d.configureConfigControllerAccess(configs); err != nil { - return fmt.Errorf("Failed to configure config controller access: %w", err) + return pkgErrors.Wrap(err, "Failed to configure config controller access") } tokenExchangerErrors := []error{} @@ -177,7 +177,7 @@ func (d *datastoreImpl) InitializeTokenExchangers() error { func (d *datastoreImpl) configureConfigControllerAccess(configs []*storage.AuthMachineToMachineConfig) error { kubeSAIssuer, err := d.issuerFetcher.GetServiceAccountIssuer() if err != nil { - return fmt.Errorf("Failed to get service account issuer: %w", err) + return pkgErrors.Wrap(err, "Failed to get service account issuer") } var kubeSAConfig *storage.AuthMachineToMachineConfig @@ -221,10 +221,10 @@ func (d *datastoreImpl) configureConfigControllerAccess(configs []*storage.AuthM // This inits the token exchanger, too _, err = d.UpsertAuthM2MConfigNoLock(ctx, kubeSAConfig) if err != nil { - return fmt.Errorf("Failed to upsert auth m2m config: %w", err) + return pkgErrors.Wrap(err, "Failed to upsert auth m2m config") } - return err + return nil } // wrapRollback wraps the error with potential rollback errors. From 6bbda7519569b9bb0a981ca04de4d4ede19596b1 Mon Sep 17 00:00:00 2001 From: Kyle Lape Date: Tue, 1 Apr 2025 10:49:36 -0500 Subject: [PATCH 3/3] Make method private --- central/auth/datastore/datastore_impl.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/central/auth/datastore/datastore_impl.go b/central/auth/datastore/datastore_impl.go index d1e15c12b66f8..65d5ddbd7fb62 100644 --- a/central/auth/datastore/datastore_impl.go +++ b/central/auth/datastore/datastore_impl.go @@ -57,10 +57,10 @@ func (d *datastoreImpl) UpsertAuthM2MConfig(ctx context.Context, d.mutex.Lock() defer d.mutex.Unlock() - return d.UpsertAuthM2MConfigNoLock(ctx, config) + return d.upsertAuthM2MConfigNoLock(ctx, config) } -func (d *datastoreImpl) UpsertAuthM2MConfigNoLock(ctx context.Context, +func (d *datastoreImpl) upsertAuthM2MConfigNoLock(ctx context.Context, config *storage.AuthMachineToMachineConfig) (*storage.AuthMachineToMachineConfig, error) { if err := sac.VerifyAuthzOK(accessSAC.WriteAllowed(ctx)); err != nil { return nil, err @@ -219,7 +219,7 @@ func (d *datastoreImpl) configureConfigControllerAccess(configs []*storage.AuthM sac.AccessModeScopeKeys(storage.Access_READ_WRITE_ACCESS), sac.ResourceScopeKeys(resources.Access))) // This inits the token exchanger, too - _, err = d.UpsertAuthM2MConfigNoLock(ctx, kubeSAConfig) + _, err = d.upsertAuthM2MConfigNoLock(ctx, kubeSAConfig) if err != nil { return pkgErrors.Wrap(err, "Failed to upsert auth m2m config") }