Merge 8d0443f into backport/VAULT-33653/next-vault-rotation/verbally-…

…crucial-catfish

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() {

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 {
@@ -932,7 +940,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())
 }
@@ -981,7 +989,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)

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

builtin/logical/database/rotation_test.go

@@ -1669,14 +1669,38 @@ 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()
 	config.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(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{}

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.
+```