diff --git a/CHANGELOG.md b/CHANGELOG.md index 59c110f..49eb4b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ ## Unreleased +BUG FIXES: + +* Update static role rotation to generate a new password after 2 failed attempts (https://github.com/hashicorp/vault-plugin-secrets-openldap/pull/125) + ## v0.14.3 BUG FIXES: diff --git a/path_static_roles_test.go b/path_static_roles_test.go index ae6f925..136ba56 100644 --- a/path_static_roles_test.go +++ b/path_static_roles_test.go @@ -435,6 +435,112 @@ func TestRoles(t *testing.T) { }) } +func TestRoles_NewPasswordGeneration(t *testing.T) { + ctx := context.Background() + b, storage := getBackend(false) + defer b.Cleanup(ctx) + configureOpenLDAPMount(t, b, storage) + + // Create the role + roleName := "hashicorp" + createRole(t, b, storage, roleName) + + t.Run("rotation failures should generate new password on retry", func(t *testing.T) { + // Fail to rotate the role + generateWALFromFailedRotation(t, b, storage, roleName) + + // Get WAL + walIDs := requireWALs(t, storage, 1) + wal, err := b.findStaticWAL(ctx, storage, walIDs[0]) + if err != nil || wal == nil { + t.Fatal(err) + } + + // Store password + initialPassword := wal.NewPassword + + // Rotate role manually and fail again with same password + generateWALFromFailedRotation(t, b, storage, roleName) + + // Ensure WAL is deleted since retrying initial password failed + requireWALs(t, storage, 0) + + // Successfully rotate the role + _, err = b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.UpdateOperation, + Path: "rotate-role/" + roleName, + Storage: storage, + }) + if err != nil { + t.Fatal(err) + } + + // Ensure WAL is flushed since request was successful + requireWALs(t, storage, 0) + + // Read the credential + resp := readStaticCred(t, b, storage, roleName) + + // Confirm successful rotation used new credential + // Assert previous failing credential is not being used + if resp.Data["password"] == initialPassword { + t.Fatalf("expected password to be different after second retry") + } + + }) + + t.Run("updating password policy should generate new password", func(t *testing.T) { + // Fail to rotate the role + generateWALFromFailedRotation(t, b, storage, roleName) + + // Get WAL + walIDs := requireWALs(t, storage, 1) + wal, err := b.findStaticWAL(ctx, storage, walIDs[0]) + if err != nil || wal == nil { + t.Fatal(err) + } + + expectedPassword := wal.NewPassword + + // Update Password Policy + configureOpenLDAPMountWithPasswordPolicy(t, b, storage, testPasswordPolicy1, true) + + // Rotate role manually and fail again + generateWALFromFailedRotation(t, b, storage, roleName) + // Get WAL + walIDs = requireWALs(t, storage, 1) + wal, err = b.findStaticWAL(ctx, storage, walIDs[0]) + if err != nil || wal == nil { + t.Fatal(err) + } + + // confirm new password is generated and is different from previous password + newPassword := wal.NewPassword + if expectedPassword == newPassword { + t.Fatalf("expected password to be different on second retry") + } + + // confirm new password uses policy + if newPassword != testPasswordFromPolicy1 { + t.Fatalf("expected password %s, got %s", testPasswordFromPolicy1, newPassword) + } + + // Successfully rotate the role + _, err = b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.UpdateOperation, + Path: "rotate-role/" + roleName, + Storage: storage, + }) + if err != nil { + t.Fatal(err) + } + + // Ensure WAL is flushed + walIDs = requireWALs(t, storage, 0) + }) + +} + func TestListRoles(t *testing.T) { t.Run("list roles", func(t *testing.T) { b, storage := getBackend(false) @@ -631,10 +737,10 @@ func TestWALsDeletedOnRoleDeletion(t *testing.T) { func configureOpenLDAPMount(t *testing.T, b *backend, storage logical.Storage) { t.Helper() - configureOpenLDAPMountWithPasswordPolicy(t, b, storage, "") + configureOpenLDAPMountWithPasswordPolicy(t, b, storage, "", false) } -func configureOpenLDAPMountWithPasswordPolicy(t *testing.T, b *backend, storage logical.Storage, policy string) { +func configureOpenLDAPMountWithPasswordPolicy(t *testing.T, b *backend, storage logical.Storage, policy string, isUpdate bool) { t.Helper() data := map[string]interface{}{ @@ -648,8 +754,12 @@ func configureOpenLDAPMountWithPasswordPolicy(t *testing.T, b *backend, storage data["password_policy"] = policy } + operation := logical.CreateOperation + if isUpdate { + operation = logical.UpdateOperation + } resp, err := b.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.CreateOperation, + Operation: operation, Path: configPath, Storage: storage, Data: data, diff --git a/rotation.go b/rotation.go index ea3f4fc..6d91045 100644 --- a/rotation.go +++ b/rotation.go @@ -321,6 +321,7 @@ func (b *backend) setStaticAccountPassword(ctx context.Context, s logical.Storag } var newPassword string + var usedCredentialFromPreviousRotation bool if output.WALID != "" { wal, err := b.findStaticWAL(ctx, s, output.WALID) if err != nil { @@ -344,6 +345,7 @@ func (b *backend) setStaticAccountPassword(ctx context.Context, s logical.Storag default: // Reuse the password from the existing WAL entry newPassword = wal.NewPassword + usedCredentialFromPreviousRotation = true } } @@ -384,6 +386,16 @@ func (b *backend) setStaticAccountPassword(ctx context.Context, s logical.Storag err = b.client.UpdateUserPassword(config.LDAP, input.Role.StaticAccount.Username, newPassword) } if err != nil { + if usedCredentialFromPreviousRotation { + b.Logger().Debug("password stored in WAL failed, deleting WAL", "role", input.RoleName, "WAL ID", output.WALID) + if err := framework.DeleteWAL(ctx, s, output.WALID); err != nil { + b.Logger().Warn("failed to delete WAL", "error", err, "WAL ID", output.WALID) + } + + // Generate a new WAL entry and credential for next attempt + output.WALID = "" + } + return output, err } diff --git a/rotation_test.go b/rotation_test.go index 47170d6..7da6f02 100644 --- a/rotation_test.go +++ b/rotation_test.go @@ -228,7 +228,7 @@ func TestPasswordPolicyModificationInvalidatesWAL(t *testing.T) { b, storage := getBackend(false) defer b.Cleanup(ctx) - configureOpenLDAPMountWithPasswordPolicy(t, b, storage, testPasswordPolicy1) + configureOpenLDAPMountWithPasswordPolicy(t, b, storage, testPasswordPolicy1, false) createRole(t, b, storage, "hashicorp") // Create a WAL entry from a partial failure to rotate @@ -245,7 +245,7 @@ func TestPasswordPolicyModificationInvalidatesWAL(t *testing.T) { } // Update the password policy on the configuration - configureOpenLDAPMountWithPasswordPolicy(t, b, storage, testPasswordPolicy2) + configureOpenLDAPMountWithPasswordPolicy(t, b, storage, testPasswordPolicy2, false) // Manually rotate the role. It should not use the password from the WAL entry // created earlier. Instead, it should result in generation of a new password