diff --git a/builtin/logical/database/path_creds_create.go b/builtin/logical/database/path_creds_create.go index 53ca3b7bdb..5da9cba6cb 100644 --- a/builtin/logical/database/path_creds_create.go +++ b/builtin/logical/database/path_creds_create.go @@ -256,9 +256,11 @@ func (b *databaseBackend) pathStaticCredsRead() framework.OperationFunc { } respData := map[string]interface{}{ - "username": role.StaticAccount.Username, - "ttl": role.StaticAccount.CredentialTTL().Seconds(), - "last_vault_rotation": role.StaticAccount.LastVaultRotation, + "username": role.StaticAccount.Username, + "ttl": role.StaticAccount.CredentialTTL().Seconds(), + } + if !role.StaticAccount.LastVaultRotation.IsZero() { + respData["last_vault_rotation"] = role.StaticAccount.LastVaultRotation } if role.StaticAccount.UsesRotationPeriod() { diff --git a/builtin/logical/database/path_roles.go b/builtin/logical/database/path_roles.go index 1e93a50234..dc3f1d1fc3 100644 --- a/builtin/logical/database/path_roles.go +++ b/builtin/logical/database/path_roles.go @@ -713,14 +713,22 @@ func (b *databaseBackend) pathStaticRoleCreateUpdate(ctx context.Context, req *l switch req.Operation { case logical.CreateOperation: if role.SkipImportRotation { - b.Logger().Trace("skipping static role import rotation", "role", name) - // synthetically set lastVaultRotation to now, so that it gets - // queued correctly - lastVaultRotation = time.Now() - // we intentionally do not set role.StaticAccount.LastVaultRotation + b.Logger().Debug("skipping static role import rotation", "role", name) + + // Synthetically set lastVaultRotation to now, so that it gets + // queued correctly. + // NOTE: We intentionally do not set role.StaticAccount.LastVaultRotation // because the zero value indicates Vault has not rotated the // password yet + lastVaultRotation = time.Now() + + // NextVaultRotation allows calculating the TTL on GET /static-creds + // requests and to calculate the queue priority in populateQueue() + // across restarts. We can't rely on LastVaultRotation in these + // cases bacause, when import rotation is skipped, LastVaultRotation + // is set to a zero value in storage. role.StaticAccount.SetNextVaultRotation(lastVaultRotation) + // we were told to not rotate, just add the entry err := b.StoreStaticRole(ctx, req.Storage, role) if err != nil { @@ -936,7 +944,7 @@ type staticAccount struct { // querying for the next schedule expiry since the last known vault rotation. func (s *staticAccount) NextRotationTime() time.Time { if s.UsesRotationPeriod() { - return s.LastVaultRotation.Add(s.RotationPeriod) + return s.NextVaultRotation } return s.Schedule.Next(time.Now()) } @@ -985,7 +993,8 @@ func (s *staticAccount) ShouldRotate(priority int64, t time.Time) bool { return priority <= t.Unix() && s.IsInsideRotationWindow(t) } -// SetNextVaultRotation +// SetNextVaultRotation sets the next vault rotation to time t plus the role's +// rotation period or to the next schedule. func (s *staticAccount) SetNextVaultRotation(t time.Time) { if s.UsesRotationPeriod() { s.NextVaultRotation = t.Add(s.RotationPeriod) diff --git a/builtin/logical/database/path_roles_test.go b/builtin/logical/database/path_roles_test.go index 0ad01efe02..d1fe247873 100644 --- a/builtin/logical/database/path_roles_test.go +++ b/builtin/logical/database/path_roles_test.go @@ -1388,6 +1388,22 @@ func createRoleWithData(t *testing.T, b *databaseBackend, s logical.Storage, moc } } +func readStaticCred(t *testing.T, b *databaseBackend, s logical.Storage, mockDB *mockNewDatabase, roleName string) *logical.Response { + t.Helper() + mockDB.On("UpdateUser", mock.Anything, mock.Anything). + Return(v5.UpdateUserResponse{}, nil). + Once() + resp, err := b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.ReadOperation, + Path: "static-creds/" + roleName, + Storage: s, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatal(resp, err) + } + return resp +} + const testRoleStaticCreate = ` CREATE ROLE "{{name}}" WITH LOGIN diff --git a/builtin/logical/database/rotation_test.go b/builtin/logical/database/rotation_test.go index 6ef586f367..e6d4104771 100644 --- a/builtin/logical/database/rotation_test.go +++ b/builtin/logical/database/rotation_test.go @@ -1664,6 +1664,29 @@ func requireWALs(t *testing.T, storage logical.Storage, expectedCount int) []str return wals } +// getBackendWithConfig returns an initialized test backend for the given +// BackendConfig +func getBackendInitQueue(t *testing.T, c *logical.BackendConfig, tickInterval string) (*databaseBackend, *logical.BackendConfig, *mockNewDatabase) { + t.Helper() + // make queue ticks more frequent for tests + c.Config[queueTickIntervalKey] = tickInterval + c.StorageView = &logical.InmemStorage{} + // Create and init the backend ourselves instead of using a Factory because + // the factory function kicks off threads that cause racy tests. + b := Backend(c) + ctx := context.Background() + if err := b.Setup(ctx, c); err != nil { + t.Fatal(err) + } + b.schedule = &TestSchedule{} + b.credRotationQueue = queue.New() + b.initQueue(ctx, c) + + mockDB := setupMockDB(b) + + return b, c, mockDB +} + func getBackend(t *testing.T) (*databaseBackend, logical.Storage, *mockNewDatabase) { t.Helper() config := logical.TestBackendConfig() @@ -1671,7 +1694,8 @@ func getBackend(t *testing.T) (*databaseBackend, logical.Storage, *mockNewDataba // Create and init the backend ourselves instead of using a Factory because // the factory function kicks off threads that cause racy tests. b := Backend(config) - if err := b.Setup(context.Background(), config); err != nil { + ctx := context.Background() + if err := b.Setup(ctx, config); err != nil { t.Fatal(err) } b.schedule = &TestSchedule{} diff --git a/changelog/29537.txt b/changelog/29537.txt new file mode 100644 index 0000000000..bb57707af6 --- /dev/null +++ b/changelog/29537.txt @@ -0,0 +1,3 @@ +```release-note:bug +database: Fix a bug where static role passwords are erroneously rotated across backend restarts when using skip import rotation. +```