Port: Premature Rotation For autorotate (#12563)

* port of ldap fix for early cred rotation

* some more porting

* another couple lines to port

* final commits before report

* remove deadlock

* needs testing

* updates

* Sync with OpenLDAP PR

* Update the update error handling for items not found in the queue

* WIP unit tests
* Need to configure DB mount correctly, with db type mockv5
* Need to find a way to inject errors into that mock db

* throw error on role creation failure

* do not swallow error on role creation

* comment out wip tests and add in a test for disallowed role

* Use newly generated password in WAL

Co-authored-by: Michael Golowka <72365+pcman312@users.noreply.github.com>

* return err on popFromRotationQueueByKey error; cleanup on setStaticAccount

* test: fix TestPlugin_lifecycle

* Uncomment and fix unit tests
* Use mock database plugin to inject errors
* Tidy test code to rely less on code internals where possible
* Some stronger test assertions

* Undo logging updates

* Add changelog

* Remove ticker and background threads from WAL tests

* Keep pre-existing API behaviour of allowing update static role to act as a create

* Switch test back to update operation

* Revert my revert, and fix some test bugs

* Fix TestBackend_StaticRole_LockRegression

* clean up defer on TestPlugin_lifecycle

* unwrap reqs on cleanup

* setStaticAccount: don't hold a write lock

* TestStoredWALsCorrectlyProcessed: set replication state to unknown

Co-authored-by: Tom Proctor <tomhjp@users.noreply.github.com>
Co-authored-by: Michael Golowka <72365+pcman312@users.noreply.github.com>
Co-authored-by: Calvin Leung Huang <1883212+calvn@users.noreply.github.com>

builtin/logical/database/path_roles.go

@@ -6,6 +6,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/hashicorp/go-multierror"
 	"github.com/hashicorp/go-secure-stdlib/strutil"
 	v4 "github.com/hashicorp/vault/sdk/database/dbplugin"
 	"github.com/hashicorp/vault/sdk/framework"
@@ -215,7 +216,28 @@ func (b *databaseBackend) pathStaticRoleDelete(ctx context.Context, req *logical
 		return nil, err
 	}
 
-	return nil, nil
+	walIDs, err := framework.ListWAL(ctx, req.Storage)
+	if err != nil {
+		return nil, err
+	}
+	var merr *multierror.Error
+	for _, walID := range walIDs {
+		wal, err := b.findStaticWAL(ctx, req.Storage, walID)
+		if err != nil {
+			merr = multierror.Append(merr, err)
+			continue
+		}
+		if wal != nil && name == wal.RoleName {
+			b.Logger().Debug("deleting WAL for deleted role", "WAL ID", walID, "role", name)
+			err = framework.DeleteWAL(ctx, req.Storage, walID)
+			if err != nil {
+				b.Logger().Debug("failed to delete WAL for deleted role", "WAL ID", walID, "error", err)
+				merr = multierror.Append(merr, err)
+			}
+		}
+	}
+
+	return nil, merr.ErrorOrNil()
 }
 
 func (b *databaseBackend) pathStaticRoleRead(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) {
@@ -482,19 +504,34 @@ func (b *databaseBackend) pathStaticRoleCreateUpdate(ctx context.Context, req *l
 
 	// Only call setStaticAccount if we're creating the role for the
 	// first time
+	var item *queue.Item
 	switch req.Operation {
 	case logical.CreateOperation:
 		// setStaticAccount calls Storage.Put and saves the role to storage
 		resp, err := b.setStaticAccount(ctx, req.Storage, &setStaticAccountInput{
 			RoleName: name,
 			Role:     role,
-			CreateUser: createRole,
 		})
 		if err != nil {
+			if resp != nil && resp.WALID != "" {
+				b.Logger().Debug("deleting WAL for failed role creation", "WAL ID", resp.WALID, "role", name)
+				walDeleteErr := framework.DeleteWAL(ctx, req.Storage, resp.WALID)
+				if walDeleteErr != nil {
+					b.Logger().Debug("failed to delete WAL for failed role creation", "WAL ID", resp.WALID, "error", walDeleteErr)
+					var merr *multierror.Error
+					merr = multierror.Append(merr, err)
+					merr = multierror.Append(merr, fmt.Errorf("failed to clean up WAL from failed role creation: %w", walDeleteErr))
+					err = merr.ErrorOrNil()
+				}
+			}
+
 			return nil, err
 		}
 		// guard against RotationTime not being set or zero-value
 		lvr = resp.RotationTime
+		item = &queue.Item{
+			Key: name,
+		}
 	case logical.UpdateOperation:
 		// store updated Role
 		entry, err := logical.StorageEntryJSON(databaseStaticRolePath+name, role)
@@ -504,17 +541,16 @@ func (b *databaseBackend) pathStaticRoleCreateUpdate(ctx context.Context, req *l
 		if err := req.Storage.Put(ctx, entry); err != nil {
 			return nil, err
 		}
-
+		item, err = b.popFromRotationQueueByKey(name)
-		// In case this is an update, remove any previous version of the item from
+		if err != nil {
-		// the queue
+			return nil, err
-		b.popFromRotationQueueByKey(name)
+		}
 	}
 
+	item.Priority = lvr.Add(role.StaticAccount.RotationPeriod).Unix()
+
 	// Add their rotation to the queue
-	if err := b.pushItem(&queue.Item{
+	if err := b.pushItem(item); err != nil {
-		Key:      name,
-		Priority: lvr.Add(role.StaticAccount.RotationPeriod).Unix(),
-	}); err != nil {
 		return nil, err
 	}
 

builtin/logical/database/path_roles_test.go

@@ -3,13 +3,16 @@ package database
 import (
 	"context"
 	"errors"
+	"strings"
 	"testing"
 	"time"
 
 	"github.com/go-test/deep"
 	"github.com/hashicorp/vault/helper/namespace"
 	postgreshelper "github.com/hashicorp/vault/helper/testhelpers/postgresql"
+	v5 "github.com/hashicorp/vault/sdk/database/dbplugin/v5"
 	"github.com/hashicorp/vault/sdk/logical"
+	"github.com/stretchr/testify/mock"
 )
 
 var dataKeys = []string{"username", "password", "last_vault_rotation", "rotation_period"}
@@ -43,7 +46,7 @@ func TestBackend_StaticRole_Config(t *testing.T) {
 		"connection_url":    connURL,
 		"plugin_name":       "postgresql-database-plugin",
 		"verify_connection": false,
-		"allowed_roles":     []string{"*"},
+		"allowed_roles":     []string{"plugin-role-test"},
 		"name":              "plugin-test",
 	}
 	req := &logical.Request{
@@ -61,6 +64,7 @@ func TestBackend_StaticRole_Config(t *testing.T) {
 	// ordering, so each case cleans up by deleting the role
 	testCases := map[string]struct {
 		account  map[string]interface{}
+		path     string
 		expected map[string]interface{}
 		err      error
 	}{
@@ -69,6 +73,7 @@ func TestBackend_StaticRole_Config(t *testing.T) {
 				"username":        dbUser,
 				"rotation_period": "5400s",
 			},
+			path: "plugin-role-test",
 			expected: map[string]interface{}{
 				"username":        dbUser,
 				"rotation_period": float64(5400),
@@ -78,8 +83,17 @@ func TestBackend_StaticRole_Config(t *testing.T) {
 			account: map[string]interface{}{
 				"username": dbUser,
 			},
+			path: "plugin-role-test",
 			err:  errors.New("rotation_period is required to create static accounts"),
 		},
+		"disallowed role config": {
+			account: map[string]interface{}{
+				"username":        dbUser,
+				"rotation_period": "5400s",
+			},
+			path: "disallowed-role",
+			err:  errors.New("\"disallowed-role\" is not an allowed role"),
+		},
 	}
 
 	for name, tc := range testCases {
@@ -94,9 +108,11 @@ func TestBackend_StaticRole_Config(t *testing.T) {
 				data[k] = v
 			}
 
+			path := "static-roles/" + tc.path
+
 			req := &logical.Request{
 				Operation: logical.CreateOperation,
-				Path:      "static-roles/plugin-role-test",
+				Path:      path,
 				Storage:   config.StorageView,
 				Data:      data,
 			}
@@ -516,6 +532,135 @@ func TestBackend_StaticRole_Role_name_check(t *testing.T) {
 	}
 }
 
+func TestWALsStillTrackedAfterUpdate(t *testing.T) {
+	ctx := context.Background()
+	b, storage, mockDB := getBackend(t)
+	defer b.Cleanup(ctx)
+	configureDBMount(t, storage)
+
+	createRole(t, b, storage, mockDB, "hashicorp")
+
+	generateWALFromFailedRotation(t, b, storage, mockDB, "hashicorp")
+	requireWALs(t, storage, 1)
+
+	resp, err := b.HandleRequest(ctx, &logical.Request{
+		Operation: logical.UpdateOperation,
+		Path:      "static-roles/hashicorp",
+		Storage:   storage,
+		Data: map[string]interface{}{
+			"username":        "hashicorp",
+			"dn":              "uid=hashicorp,ou=users,dc=hashicorp,dc=com",
+			"rotation_period": "600s",
+		},
+	})
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatal(resp, err)
+	}
+	walIDs := requireWALs(t, storage, 1)
+
+	// Now when we trigger a manual rotate, it should use the WAL's new password
+	// which will tell us that the in-memory structure still kept track of the
+	// WAL in addition to it still being in storage.
+	wal, err := b.findStaticWAL(ctx, storage, walIDs[0])
+	if err != nil {
+		t.Fatal(err)
+	}
+	rotateRole(t, b, storage, mockDB, "hashicorp")
+	role, err := b.StaticRole(ctx, storage, "hashicorp")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if role.StaticAccount.Password != wal.NewPassword {
+		t.Fatal()
+	}
+	requireWALs(t, storage, 0)
+}
+
+func TestWALsDeletedOnRoleCreationFailed(t *testing.T) {
+	ctx := context.Background()
+	b, storage, mockDB := getBackend(t)
+	defer b.Cleanup(ctx)
+	configureDBMount(t, storage)
+
+	for i := 0; i < 3; i++ {
+		mockDB.On("UpdateUser", mock.Anything, mock.Anything).
+			Return(v5.UpdateUserResponse{}, errors.New("forced error")).
+			Once()
+		resp, err := b.HandleRequest(ctx, &logical.Request{
+			Operation: logical.CreateOperation,
+			Path:      "static-roles/hashicorp",
+			Storage:   storage,
+			Data: map[string]interface{}{
+				"username":        "hashicorp",
+				"db_name":         "mockv5",
+				"rotation_period": "5s",
+			},
+		})
+		if err == nil {
+			t.Fatal("expected error from DB")
+		}
+		if !strings.Contains(err.Error(), "forced error") {
+			t.Fatal("expected forced error message", resp, err)
+		}
+	}
+
+	requireWALs(t, storage, 0)
+}
+
+func TestWALsDeletedOnRoleDeletion(t *testing.T) {
+	ctx := context.Background()
+	b, storage, mockDB := getBackend(t)
+	defer b.Cleanup(ctx)
+	configureDBMount(t, storage)
+
+	// Create the roles
+	roleNames := []string{"hashicorp", "2"}
+	for _, roleName := range roleNames {
+		createRole(t, b, storage, mockDB, roleName)
+	}
+
+	// Fail to rotate the roles
+	for _, roleName := range roleNames {
+		generateWALFromFailedRotation(t, b, storage, mockDB, roleName)
+	}
+
+	// Should have 2 WALs hanging around
+	requireWALs(t, storage, 2)
+
+	// Delete one of the static roles
+	resp, err := b.HandleRequest(ctx, &logical.Request{
+		Operation: logical.DeleteOperation,
+		Path:      "static-roles/hashicorp",
+		Storage:   storage,
+	})
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatal(resp, err)
+	}
+
+	// 1 WAL should be cleared by the delete
+	requireWALs(t, storage, 1)
+}
+
+func createRole(t *testing.T, b *databaseBackend, storage logical.Storage, mockDB *mockNewDatabase, roleName string) {
+	t.Helper()
+	mockDB.On("UpdateUser", mock.Anything, mock.Anything).
+		Return(v5.UpdateUserResponse{}, nil).
+		Once()
+	resp, err := b.HandleRequest(context.Background(), &logical.Request{
+		Operation: logical.CreateOperation,
+		Path:      "static-roles/" + roleName,
+		Storage:   storage,
+		Data: map[string]interface{}{
+			"username":        roleName,
+			"db_name":         "mockv5",
+			"rotation_period": "86400s",
+		},
+	})
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatal(resp, err)
+	}
+}
+
 const testRoleStaticCreate = `
 CREATE ROLE "{{name}}" WITH
   LOGIN

builtin/logical/database/path_rotate_credentials.go

@@ -169,10 +169,14 @@ func (b *databaseBackend) pathRotateRoleCredentialsUpdate() framework.OperationF
 			}
 		}
 
-		resp, err := b.setStaticAccount(ctx, req.Storage, &setStaticAccountInput{
+		input := &setStaticAccountInput{
 			RoleName: name,
 			Role:     role,
-		})
+		}
+		if walID, ok := item.Value.(string); ok {
+			input.WALID = walID
+		}
+		resp, err := b.setStaticAccount(ctx, req.Storage, input)
 		// if err is not nil, we need to attempt to update the priority and place
 		// this item back on the queue. The err should still be returned at the end
 		// of this method.
@@ -188,6 +192,8 @@ func (b *databaseBackend) pathRotateRoleCredentialsUpdate() framework.OperationF
 			}
 		} else {
 			item.Priority = resp.RotationTime.Add(role.StaticAccount.RotationPeriod).Unix()
+			// Clear any stored WAL ID as we must have successfully deleted our WAL to get here.
+			item.Value = ""
 		}
 
 		// Add their rotation to the queue
@@ -195,8 +201,13 @@ func (b *databaseBackend) pathRotateRoleCredentialsUpdate() framework.OperationF
 			return nil, err
 		}
 
+		if err != nil {
+			return nil, fmt.Errorf("unable to finish rotating credentials; retries will "+
+				"continue in the background but it is also safe to retry manually: %w", err)
+		}
+
 		// return any err from the setStaticAccount call
-		return nil, err
+		return nil, nil
 	}
 }
 

builtin/logical/database/rotation.go

@@ -7,9 +7,8 @@ import (
 	"strconv"
 	"time"
 
-	"github.com/hashicorp/go-multierror"
+	"github.com/hashicorp/errwrap"
 	"github.com/hashicorp/go-secure-stdlib/strutil"
-	v4 "github.com/hashicorp/vault/sdk/database/dbplugin"
 	v5 "github.com/hashicorp/vault/sdk/database/dbplugin/v5"
 	"github.com/hashicorp/vault/sdk/framework"
 	"github.com/hashicorp/vault/sdk/helper/consts"
@@ -66,21 +65,34 @@ func (b *databaseBackend) populateQueue(ctx context.Context, s logical.Storage)
 
 		item := queue.Item{
 			Key:      roleName,
-			Priority: role.StaticAccount.LastVaultRotation.Add(role.StaticAccount.RotationPeriod).Unix(),
+			Priority: role.StaticAccount.NextRotationTime().Unix(),
 		}
 
 		// Check if role name is in map
 		walEntry := walMap[roleName]
 		if walEntry != nil {
 			// Check walEntry last vault time
-			if !walEntry.LastVaultRotation.IsZero() && walEntry.LastVaultRotation.Before(role.StaticAccount.LastVaultRotation) {
+			if walEntry.LastVaultRotation.IsZero() {
+				// A WAL's last Vault rotation can only ever be 0 for a role that
+				// was never successfully created. So we know this WAL couldn't
+				// have been created for this role we just retrieved from storage.
+				// i.e. it must be a hangover from a previous attempt at creating
+				// a role with the same name
+				log.Debug("deleting WAL with zero last rotation time", "WAL ID", walEntry.walID, "created", walEntry.walCreatedAt)
+				if err := framework.DeleteWAL(ctx, s, walEntry.walID); err != nil {
+					log.Warn("unable to delete zero-time WAL", "error", err, "WAL ID", walEntry.walID)
+				}
+			} else if walEntry.LastVaultRotation.Before(role.StaticAccount.LastVaultRotation) {
 				// WAL's last vault rotation record is older than the role's data, so
 				// delete and move on
+				log.Debug("deleting outdated WAL", "WAL ID", walEntry.walID, "created", walEntry.walCreatedAt)
 				if err := framework.DeleteWAL(ctx, s, walEntry.walID); err != nil {
 					log.Warn("unable to delete WAL", "error", err, "WAL ID", walEntry.walID)
 				}
 			} else {
-				log.Info("adjusting priority for Role")
+				log.Info("found WAL for role",
+					"role", item.Key,
+					"WAL ID", walEntry.walID)
 				item.Value = walEntry.walID
 				item.Priority = time.Now().Unix()
 			}
@@ -114,13 +126,14 @@ func (b *databaseBackend) runTicker(ctx context.Context, queueTickInterval time.
 // credential setting or rotation in the event of partial failure.
 type setCredentialsWAL struct {
 	NewPassword string `json:"new_password"`
-	OldPassword string `json:"old_password"`
 	RoleName    string `json:"role_name"`
 	Username    string `json:"username"`
 
 	LastVaultRotation time.Time `json:"last_vault_rotation"`
 
+	// Private fields which will not be included in json.Marshal/Unmarshal.
 	walID        string
+	walCreatedAt int64 // Unix time at which the WAL was created.
 }
 
 // rotateCredentials sets a new password for a static account. This method is
@@ -194,19 +207,8 @@ func (b *databaseBackend) rotateCredential(ctx context.Context, s logical.Storag
 	// If there is a WAL entry related to this Role, the corresponding WAL ID
 	// should be stored in the Item's Value field.
 	if walID, ok := item.Value.(string); ok {
-		walEntry, err := b.findStaticWAL(ctx, s, walID)
-		if err != nil {
-			b.logger.Error("error finding static WAL", "error", err)
-			item.Priority = time.Now().Add(10 * time.Second).Unix()
-			if err := b.pushItem(item); err != nil {
-				b.logger.Error("unable to push item on to queue", "error", err)
-			}
-		}
-		if walEntry != nil && walEntry.NewPassword != "" {
-			input.Password = walEntry.NewPassword
 		input.WALID = walID
 	}
-	}
 
 	resp, err := b.setStaticAccount(ctx, s, input)
 	if err != nil {
@@ -226,6 +228,8 @@ func (b *databaseBackend) rotateCredential(ctx context.Context, s logical.Storag
 		// Go to next item
 		return true
 	}
+	// Clear any stored WAL ID as we must have successfully deleted our WAL to get here.
+	item.Value = ""
 
 	lvr := resp.RotationTime
 	if lvr.IsZero() {
@@ -256,8 +260,8 @@ func (b *databaseBackend) findStaticWAL(ctx context.Context, s logical.Storage,
 	data := wal.Data.(map[string]interface{})
 	walEntry := setCredentialsWAL{
 		walID:        id,
+		walCreatedAt: wal.CreatedAt,
 		NewPassword:  data["new_password"].(string),
-		OldPassword: data["old_password"].(string),
 		RoleName:     data["role_name"].(string),
 		Username:     data["username"].(string),
 	}
@@ -273,14 +277,11 @@ func (b *databaseBackend) findStaticWAL(ctx context.Context, s logical.Storage,
 type setStaticAccountInput struct {
 	RoleName string
 	Role     *roleEntry
-	Password   string
-	CreateUser bool
 	WALID    string
 }
 
 type setStaticAccountOutput struct {
 	RotationTime time.Time
-	Password     string
 	// Optional return field, in the event WAL was created and not destroyed
 	// during the operation
 	WALID string
@@ -300,7 +301,6 @@ type setStaticAccountOutput struct {
 // This method does not perform any operations on the priority queue. Those
 // tasks must be handled outside of this method.
 func (b *databaseBackend) setStaticAccount(ctx context.Context, s logical.Storage, input *setStaticAccountInput) (*setStaticAccountOutput, error) {
-	var merr error
 	if input == nil || input.Role == nil || input.RoleName == "" {
 		return nil, errors.New("input was empty when attempting to set credentials for static account")
 	}
@@ -311,6 +311,9 @@ func (b *databaseBackend) setStaticAccount(ctx context.Context, s logical.Storag
 	if err != nil {
 		return output, err
 	}
+	if dbConfig == nil {
+		return output, errors.New("the config is currently unset")
+	}
 
 	// If role name isn't in the database's allowed roles, send back a
 	// permission denied.
@@ -330,31 +333,47 @@ func (b *databaseBackend) setStaticAccount(ctx context.Context, s logical.Storag
 	// Use password from input if available. This happens if we're restoring from
 	// a WAL item or processing the rotation queue with an item that has a WAL
 	// associated with it
-	newPassword := input.Password
+	var newPassword string
-	if newPassword == "" {
+	if output.WALID != "" {
+		wal, err := b.findStaticWAL(ctx, s, output.WALID)
+		if err != nil {
+			return output, errwrap.Wrapf("error retrieving WAL entry: {{err}}", err)
+		}
+
+		switch {
+		case wal != nil && wal.NewPassword != "":
+			newPassword = wal.NewPassword
+		default:
+			if wal == nil {
+				b.Logger().Error("expected role to have WAL, but WAL not found in storage", "role", input.RoleName, "WAL ID", output.WALID)
+			} else {
+				b.Logger().Error("expected WAL to have a new password set, but empty", "role", input.RoleName, "WAL ID", output.WALID)
+				err = framework.DeleteWAL(ctx, s, output.WALID)
+				if err != nil {
+					b.Logger().Warn("failed to delete WAL with no new password", "error", err, "WAL ID", output.WALID)
+				}
+			}
+			// If there's anything wrong with the WAL in storage, we'll need
+			// to generate a fresh WAL and password
+			output.WALID = ""
+		}
+	}
+
+	if output.WALID == "" {
 		newPassword, err = dbi.database.GeneratePassword(ctx, b.System(), dbConfig.PasswordPolicy)
 		if err != nil {
 			return output, err
 		}
-	}
-	output.Password = newPassword
-
-	config := v4.StaticUserConfig{
-		Username: input.Role.StaticAccount.Username,
-		Password: newPassword,
-	}
-
-	if output.WALID == "" {
 		output.WALID, err = framework.PutWAL(ctx, s, staticWALKey, &setCredentialsWAL{
 			RoleName:          input.RoleName,
-			Username:          config.Username,
+			Username:          input.Role.StaticAccount.Username,
-			NewPassword:       config.Password,
+			NewPassword:       newPassword,
-			OldPassword:       input.Role.StaticAccount.Password,
 			LastVaultRotation: input.Role.StaticAccount.LastVaultRotation,
 		})
 		if err != nil {
 			return output, fmt.Errorf("error writing WAL entry: %w", err)
 		}
+		b.Logger().Debug("writing WAL", "role", input.RoleName, "WAL ID", output.WALID)
 	}
 
 	updateReq := v5.UpdateUserRequest{
@@ -389,12 +408,13 @@ func (b *databaseBackend) setStaticAccount(ctx context.Context, s logical.Storag
 
 	// Cleanup WAL after successfully rotating and pushing new item on to queue
 	if err := framework.DeleteWAL(ctx, s, output.WALID); err != nil {
-		merr = multierror.Append(merr, err)
+		b.Logger().Warn("error deleting WAL", "WAL ID", output.WALID, "error", err)
-		return output, merr
+		return output, err
 	}
+	b.Logger().Debug("deleted WAL", "WAL ID", output.WALID)
 
 	// The WAL has been deleted, return new setStaticAccountOutput without it
-	return &setStaticAccountOutput{RotationTime: lvr}, merr
+	return &setStaticAccountOutput{RotationTime: lvr}, nil
 }
 
 // initQueue preforms the necessary checks and initializations needed to perform
@@ -493,13 +513,39 @@ func (b *databaseBackend) loadStaticWALs(ctx context.Context, s logical.Storage)
 			continue
 		}
 		if role == nil || role.StaticAccount == nil {
+			b.Logger().Debug("deleting WAL with nil role or static account", "WAL ID", walEntry.walID)
 			if err := framework.DeleteWAL(ctx, s, walEntry.walID); err != nil {
 				b.Logger().Warn("unable to delete WAL", "error", err, "WAL ID", walEntry.walID)
 			}
 			continue
 		}
 
-		walEntry.walID = walID
+		if existingWALEntry, exists := walMap[walEntry.RoleName]; exists {
+			b.Logger().Debug("multiple WALs detected for role", "role", walEntry.RoleName,
+				"loaded WAL ID", existingWALEntry.walID, "created at", existingWALEntry.walCreatedAt, "last vault rotation", existingWALEntry.LastVaultRotation,
+				"candidate WAL ID", walEntry.walID, "created at", walEntry.walCreatedAt, "last vault rotation", walEntry.LastVaultRotation)
+
+			if walEntry.walCreatedAt > existingWALEntry.walCreatedAt {
+				// If the existing WAL is older, delete it from storage and fall
+				// through to inserting our current WAL into the map.
+				b.Logger().Debug("deleting stale loaded WAL", "WAL ID", existingWALEntry.walID)
+				err = framework.DeleteWAL(ctx, s, existingWALEntry.walID)
+				if err != nil {
+					b.Logger().Warn("unable to delete loaded WAL", "error", err, "WAL ID", existingWALEntry.walID)
+				}
+			} else {
+				// If we already have a more recent WAL entry in the map, delete
+				// this one and continue onto the next WAL.
+				b.Logger().Debug("deleting stale candidate WAL", "WAL ID", walEntry.walID)
+				err = framework.DeleteWAL(ctx, s, walID)
+				if err != nil {
+					b.Logger().Warn("unable to delete candidate WAL", "error", err, "WAL ID", walEntry.walID)
+				}
+				continue
+			}
+		}
+
+		b.Logger().Debug("loaded WAL", "WAL ID", walID)
 		walMap[walEntry.RoleName] = walEntry
 	}
 	return walMap, nil

builtin/logical/database/rotation_test.go

@@ -3,6 +3,8 @@ package database
 import (
 	"context"
 	"database/sql"
+	"errors"
+	"fmt"
 	"log"
 	"os"
 	"strings"
@@ -13,11 +15,15 @@ import (
 	"github.com/hashicorp/vault/helper/namespace"
 	"github.com/hashicorp/vault/helper/testhelpers/mongodb"
 	postgreshelper "github.com/hashicorp/vault/helper/testhelpers/postgresql"
+	v5 "github.com/hashicorp/vault/sdk/database/dbplugin/v5"
 	"github.com/hashicorp/vault/sdk/framework"
+	"github.com/hashicorp/vault/sdk/helper/consts"
 	"github.com/hashicorp/vault/sdk/helper/dbtxn"
 	"github.com/hashicorp/vault/sdk/logical"
+	"github.com/hashicorp/vault/sdk/queue"
 	"github.com/lib/pq"
 	mongodbatlasapi "github.com/mongodb/go-client-mongodb-atlas/mongodbatlas"
+	"github.com/stretchr/testify/mock"
 	"go.mongodb.org/mongo-driver/mongo"
 	"go.mongodb.org/mongo-driver/mongo/options"
 )
@@ -974,15 +980,25 @@ func TestBackend_StaticRole_LockRegression(t *testing.T) {
 	}
 
 	createTestPGUser(t, connURL, dbUser, dbUserDefaultPassword, testRoleStaticCreate)
-	for i := 0; i < 25; i++ {
+	data = map[string]interface{}{
-		data := map[string]interface{}{
 		"name":                "plugin-role-test",
 		"db_name":             "plugin-test",
 		"rotation_statements": testRoleStaticUpdate,
 		"username":            dbUser,
 		"rotation_period":     "7s",
 	}
+	req = &logical.Request{
+		Operation: logical.CreateOperation,
+		Path:      "static-roles/plugin-role-test",
+		Storage:   config.StorageView,
+		Data:      data,
+	}
 
+	resp, err = b.HandleRequest(namespace.RootContext(nil), req)
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatalf("err:%s resp:%#v\n", err, resp)
+	}
+	for i := 0; i < 25; i++ {
 		req = &logical.Request{
 			Operation: logical.UpdateOperation,
 			Path:      "static-roles/plugin-role-test",
@@ -1094,6 +1110,300 @@ func TestBackend_StaticRole_Rotate_Invalid_Role(t *testing.T) {
 	}
 }
 
+func TestRollsPasswordForwardsUsingWAL(t *testing.T) {
+	ctx := context.Background()
+	b, storage, mockDB := getBackend(t)
+	defer b.Cleanup(ctx)
+	configureDBMount(t, storage)
+	createRole(t, b, storage, mockDB, "hashicorp")
+
+	role, err := b.StaticRole(ctx, storage, "hashicorp")
+	if err != nil {
+		t.Fatal(err)
+	}
+	oldPassword := role.StaticAccount.Password
+
+	generateWALFromFailedRotation(t, b, storage, mockDB, "hashicorp")
+
+	walIDs := requireWALs(t, storage, 1)
+	wal, err := b.findStaticWAL(ctx, storage, walIDs[0])
+	if err != nil {
+		t.Fatal(err)
+	}
+	role, err = b.StaticRole(ctx, storage, "hashicorp")
+	if err != nil {
+		t.Fatal(err)
+	}
+	// Role's password should still be the WAL's old password
+	if role.StaticAccount.Password != oldPassword {
+		t.Fatal(role.StaticAccount.Password, oldPassword)
+	}
+
+	rotateRole(t, b, storage, mockDB, "hashicorp")
+
+	role, err = b.StaticRole(ctx, storage, "hashicorp")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if role.StaticAccount.Password != wal.NewPassword {
+		t.Fatal("role password", role.StaticAccount.Password, "WAL new password", wal.NewPassword)
+	}
+	// WAL should be cleared by the successful rotate
+	requireWALs(t, storage, 0)
+}
+
+func TestStoredWALsCorrectlyProcessed(t *testing.T) {
+	const walNewPassword = "new-password-from-wal"
+	for _, tc := range []struct {
+		name         string
+		shouldRotate bool
+		wal          *setCredentialsWAL
+	}{
+		{
+			"WAL is kept and used for roll forward",
+			true,
+			&setCredentialsWAL{
+				RoleName:          "hashicorp",
+				Username:          "hashicorp",
+				NewPassword:       walNewPassword,
+				LastVaultRotation: time.Now().Add(time.Hour),
+			},
+		},
+		{
+			"zero-time WAL is discarded on load",
+			false,
+			&setCredentialsWAL{
+				RoleName:          "hashicorp",
+				Username:          "hashicorp",
+				NewPassword:       walNewPassword,
+				LastVaultRotation: time.Time{},
+			},
+		},
+		{
+			"empty-password WAL is kept but a new password is generated",
+			true,
+			&setCredentialsWAL{
+				RoleName:          "hashicorp",
+				Username:          "hashicorp",
+				NewPassword:       "",
+				LastVaultRotation: time.Now().Add(time.Hour),
+			},
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			ctx := context.Background()
+			config := logical.TestBackendConfig()
+			storage := &logical.InmemStorage{}
+			config.StorageView = storage
+			b := Backend(config)
+			defer b.Cleanup(ctx)
+			mockDB := setupMockDB(b)
+			if err := b.Setup(ctx, config); err != nil {
+				t.Fatal(err)
+			}
+			b.credRotationQueue = queue.New()
+			configureDBMount(t, config.StorageView)
+			createRole(t, b, config.StorageView, mockDB, "hashicorp")
+			role, err := b.StaticRole(ctx, config.StorageView, "hashicorp")
+			if err != nil {
+				t.Fatal(err)
+			}
+			initialPassword := role.StaticAccount.Password
+
+			// Set up a WAL for our test case
+			framework.PutWAL(ctx, config.StorageView, staticWALKey, tc.wal)
+			requireWALs(t, config.StorageView, 1)
+			// Reset the rotation queue to simulate startup memory state
+			b.credRotationQueue = queue.New()
+
+			// Now finish the startup process by populating the queue, which should discard the WAL
+			b.initQueue(ctx, config, consts.ReplicationUnknown)
+
+			if tc.shouldRotate {
+				requireWALs(t, storage, 1)
+			} else {
+				requireWALs(t, storage, 0)
+			}
+
+			// Run one tick
+			mockDB.On("UpdateUser", mock.Anything, mock.Anything).
+				Return(v5.UpdateUserResponse{}, nil).
+				Once()
+			b.rotateCredentials(ctx, storage)
+			requireWALs(t, storage, 0)
+
+			role, err = b.StaticRole(ctx, storage, "hashicorp")
+			if err != nil {
+				t.Fatal(err)
+			}
+			item, err := b.popFromRotationQueueByKey("hashicorp")
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			if tc.shouldRotate {
+				if tc.wal.NewPassword != "" {
+					// Should use WAL's new_password field
+					if role.StaticAccount.Password != walNewPassword {
+						t.Fatal()
+					}
+				} else {
+					// Should rotate but ignore WAL's new_password field
+					if role.StaticAccount.Password == initialPassword {
+						t.Fatal()
+					}
+					if role.StaticAccount.Password == walNewPassword {
+						t.Fatal()
+					}
+				}
+			} else {
+				// Ensure the role was not promoted for early rotation
+				if item.Priority < time.Now().Add(time.Hour).Unix() {
+					t.Fatal("priority should be for about a week away, but was", item.Priority)
+				}
+				if role.StaticAccount.Password != initialPassword {
+					t.Fatal("password should not have been rotated yet")
+				}
+			}
+		})
+	}
+}
+
+func TestDeletesOlderWALsOnLoad(t *testing.T) {
+	ctx := context.Background()
+	b, storage, mockDB := getBackend(t)
+	defer b.Cleanup(ctx)
+	configureDBMount(t, storage)
+	createRole(t, b, storage, mockDB, "hashicorp")
+
+	// Create 4 WALs, with a clear winner for most recent.
+	wal := &setCredentialsWAL{
+		RoleName:          "hashicorp",
+		Username:          "hashicorp",
+		NewPassword:       "some-new-password",
+		LastVaultRotation: time.Now(),
+	}
+	for i := 0; i < 3; i++ {
+		_, err := framework.PutWAL(ctx, storage, staticWALKey, wal)
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+	time.Sleep(2 * time.Second)
+	// We expect this WAL to have the latest createdAt timestamp
+	walID, err := framework.PutWAL(ctx, storage, staticWALKey, wal)
+	if err != nil {
+		t.Fatal(err)
+	}
+	requireWALs(t, storage, 4)
+
+	walMap, err := b.loadStaticWALs(ctx, storage)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(walMap) != 1 || walMap["hashicorp"] == nil || walMap["hashicorp"].walID != walID {
+		t.Fatal()
+	}
+	requireWALs(t, storage, 1)
+}
+
+func generateWALFromFailedRotation(t *testing.T, b *databaseBackend, storage logical.Storage, mockDB *mockNewDatabase, roleName string) {
+	t.Helper()
+	mockDB.On("UpdateUser", mock.Anything, mock.Anything).
+		Return(v5.UpdateUserResponse{}, errors.New("forced error")).
+		Once()
+	_, err := b.HandleRequest(context.Background(), &logical.Request{
+		Operation: logical.UpdateOperation,
+		Path:      "rotate-role/" + roleName,
+		Storage:   storage,
+	})
+	if err == nil {
+		t.Fatal("expected error")
+	}
+}
+
+func rotateRole(t *testing.T, b *databaseBackend, storage logical.Storage, mockDB *mockNewDatabase, roleName string) {
+	t.Helper()
+	mockDB.On("UpdateUser", mock.Anything, mock.Anything).
+		Return(v5.UpdateUserResponse{}, nil).
+		Once()
+	resp, err := b.HandleRequest(context.Background(), &logical.Request{
+		Operation: logical.UpdateOperation,
+		Path:      "rotate-role/" + roleName,
+		Storage:   storage,
+	})
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatal(resp, err)
+	}
+}
+
+// returns a slice of the WAL IDs in storage
+func requireWALs(t *testing.T, storage logical.Storage, expectedCount int) []string {
+	t.Helper()
+	wals, err := storage.List(context.Background(), "wal/")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(wals) != expectedCount {
+		t.Fatal("expected WALs", expectedCount, "got", len(wals))
+	}
+
+	return wals
+}
+
+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 {
+		t.Fatal(err)
+	}
+	b.credRotationQueue = queue.New()
+	b.populateQueue(context.Background(), config.StorageView)
+
+	mockDB := setupMockDB(b)
+
+	return b, config.StorageView, mockDB
+}
+
+func setupMockDB(b *databaseBackend) *mockNewDatabase {
+	mockDB := &mockNewDatabase{}
+	mockDB.On("Initialize", mock.Anything, mock.Anything).Return(v5.InitializeResponse{}, nil)
+	mockDB.On("Close").Return(nil)
+	dbw := databaseVersionWrapper{
+		v5: mockDB,
+	}
+
+	dbi := &dbPluginInstance{
+		database: dbw,
+		id:       "foo-id",
+		name:     "mockV5",
+	}
+	b.connections["mockv5"] = dbi
+
+	return mockDB
+}
+
+// configureDBMount puts config directly into storage to avoid the DB engine's
+// plugin init code paths, allowing us to use a manually populated mock DB object.
+func configureDBMount(t *testing.T, storage logical.Storage) {
+	t.Helper()
+	entry, err := logical.StorageEntryJSON(fmt.Sprintf("config/mockv5"), &DatabaseConfig{
+		AllowedRoles: []string{"*"},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	err = storage.Put(context.Background(), entry)
+	if err != nil {
+		t.Fatal(err)
+	}
+}
+
 // capturePasswords captures the current passwords at the time of calling, and
 // returns a map of username / passwords building off of the input map
 func capturePasswords(t *testing.T, b logical.Backend, config *logical.BackendConfig, testCases []string, pws map[string][]string) map[string][]string {

builtin/logical/database/versioning_large_test.go

@@ -81,8 +81,12 @@ func TestPlugin_lifecycle(t *testing.T) {
 
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {
-			cleanupReqs := []*logical.Request{}
+			var cleanupReqs []*logical.Request
-			defer cleanup(t, b, cleanupReqs)
+			defer func() {
+				// Do not defer cleanup directly so that we can populate the
+				// slice before the function gets executed.
+				cleanup(t, b, cleanupReqs)
+			}()
 
 			// /////////////////////////////////////////////////////////////////
 			// Configure
@@ -173,7 +177,7 @@ func TestPlugin_lifecycle(t *testing.T) {
 			// Create static role
 			staticRoleName := "static-role"
 			req = &logical.Request{
-				Operation: logical.UpdateOperation,
+				Operation: logical.CreateOperation,
 				Path:      fmt.Sprintf("static-roles/%s", staticRoleName),
 				Storage:   config.StorageView,
 				Data: map[string]interface{}{

changelog/12563.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+secrets/db: Fix bug where Vault can rotate static role passwords early during start up under certain conditions.
+```