Skip to content

Commit

Permalink
Update rotation logic to generate new password when a retried passwor…
Browse files Browse the repository at this point in the history
…d fails (#125) (#126)
  • Loading branch information
vinay-gopalan authored Dec 5, 2024
1 parent 996f3b8 commit d9aafd8
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 5 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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:
Expand Down
116 changes: 113 additions & 3 deletions path_static_roles_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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{}{
Expand All @@ -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,
Expand Down
12 changes: 12 additions & 0 deletions rotation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
}

Expand Down Expand Up @@ -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
}

Expand Down
4 changes: 2 additions & 2 deletions rotation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down

0 comments on commit d9aafd8

Please sign in to comment.