Skip to content

Commit

Permalink
Backport of add nil check for secret id entry on delete via accessor …
Browse files Browse the repository at this point in the history
…into release/1.13.x (#19351)

* backport of commit 8154be6

* empty commit to re-run tests

---------

Co-authored-by: davidadeleon <56207066+davidadeleon@users.noreply.github.com>
  • Loading branch information
1 parent c6c35dc commit a0beacd
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 2 deletions.
13 changes: 11 additions & 2 deletions builtin/credential/approle/path_role.go
Original file line number Diff line number Diff line change
Expand Up @@ -1964,12 +1964,21 @@ func (b *backend) pathRoleSecretIDAccessorDestroyUpdateDelete(ctx context.Contex
return nil, fmt.Errorf("failed to create HMAC of role_name: %w", err)
}

entryIndex := fmt.Sprintf("%s%s/%s", role.SecretIDPrefix, roleNameHMAC, accessorEntry.SecretIDHMAC)

lock := b.secretIDLock(accessorEntry.SecretIDHMAC)
lock.Lock()
defer lock.Unlock()

// Verify we have a valid SecretID Storage Entry
entry, err := b.nonLockedSecretIDStorageEntry(ctx, req.Storage, role.SecretIDPrefix, roleNameHMAC, accessorEntry.SecretIDHMAC)
if err != nil {
return nil, err
}
if entry == nil {
return logical.ErrorResponse("invalid secret id accessor"), logical.ErrPermissionDenied
}

entryIndex := fmt.Sprintf("%s%s/%s", role.SecretIDPrefix, roleNameHMAC, accessorEntry.SecretIDHMAC)

// Delete the accessor of the SecretID first
if err := b.deleteSecretIDAccessorEntry(ctx, req.Storage, secretIDAccessor, role.SecretIDPrefix); err != nil {
return nil, err
Expand Down
56 changes: 56 additions & 0 deletions builtin/credential/approle/path_role_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2696,3 +2696,59 @@ func TestAppRole_SecretID_WithTTL(t *testing.T) {
})
}
}

// TestAppRole_RoleSecretIDAccessorCrossDelete tests deleting a secret id via
// secret id accessor belonging to a different role
func TestAppRole_RoleSecretIDAccessorCrossDelete(t *testing.T) {
var resp *logical.Response
var err error
b, storage := createBackendWithStorage(t)

// Create First Role
createRole(t, b, storage, "role1", "a,b")
_ = b.requestNoErr(t, &logical.Request{
Operation: logical.UpdateOperation,
Storage: storage,
Path: "role/role1/secret-id",
})

// Create Second Role
createRole(t, b, storage, "role2", "a,b")
_ = b.requestNoErr(t, &logical.Request{
Operation: logical.UpdateOperation,
Storage: storage,
Path: "role/role2/secret-id",
})

// Get role2 secretID Accessor
resp = b.requestNoErr(t, &logical.Request{
Operation: logical.ListOperation,
Storage: storage,
Path: "role/role2/secret-id",
})

// Read back role2 secretID Accessor information
hmacSecretID := resp.Data["keys"].([]string)[0]
_ = b.requestNoErr(t, &logical.Request{
Operation: logical.UpdateOperation,
Storage: storage,
Path: "role/role2/secret-id-accessor/lookup",
Data: map[string]interface{}{
"secret_id_accessor": hmacSecretID,
},
})

// Attempt to destroy role2 secretID accessor using role1 path
_, err = b.HandleRequest(context.Background(), &logical.Request{
Operation: logical.UpdateOperation,
Storage: storage,
Path: "role/role1/secret-id-accessor/destroy",
Data: map[string]interface{}{
"secret_id_accessor": hmacSecretID,
},
})

if err == nil {
t.Fatalf("expected error")
}
}
3 changes: 3 additions & 0 deletions changelog/19186.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
auth/approle: Add nil check for the secret ID entry when deleting via secret id accessor preventing cross role secret id deletion
```

0 comments on commit a0beacd

Please sign in to comment.