Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AppRole Tidy with many dangling accessors blocks secret creation #8381

Closed
chrishoffman opened this issue Feb 19, 2020 · 1 comment · Fixed by #8418
Closed

AppRole Tidy with many dangling accessors blocks secret creation #8381

chrishoffman opened this issue Feb 19, 2020 · 1 comment · Fixed by #8418
Labels
auth/approle bug Used to indicate a potential bug
Milestone

Comments

@chrishoffman
Copy link
Contributor

When the tidy operation in AppRole is running with many dangling accessors, it locks all the secret ID locks which prevents secret ids from being created. Depending on how long the tidy operation takes, this can cause an outage.

The locks in question can be found here:
https://github.com/hashicorp/vault/blob/master/builtin/credential/approle/path_tidy_user_id.go#L168-L171

We should look at a way to either only lock when making modifications or possibly remove the locks in a safe way to allow for secret creation without potentially deleting the accessor.

@chrishoffman chrishoffman added this to the 1.4 milestone Feb 19, 2020
@catsby catsby added auth/approle bug Used to indicate a potential bug labels Feb 20, 2020
@catsby
Copy link
Member

catsby commented Feb 20, 2020

Could this lock phase be moved down to line ~205 where we know the actual secretIDHMAC values, and only grab the lock(s) needed?

searchloop:
for _, roleNameHMAC := range roleNameHMACs {
secretIDHMACs, err := s.List(ctx, fmt.Sprintf("%s%s", secretIDPrefixToUse, roleNameHMAC))
if err != nil {
return err
}
for _, v := range secretIDHMACs {
if v == entry.SecretIDHMAC {
found = true
logger.Trace("accessor verified, not removing")
break searchloop
}
}
}
if !found {
logger.Trace("could not verify dangling accessor, removing")
err = s.Delete(ctx, entryIndex)
if err != nil {
return err
}
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auth/approle bug Used to indicate a potential bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants