search

hashicorp / vault

A tool for secrets management, encryption as a service, and privileged access management
https://www.vaultproject.io/
Other
31.05k stars 4.2k forks source link

Consistent Token Renewal Logic When Configured Policies Not Equivalent #8449

Open mdgreenfield opened 4 years ago

mdgreenfield commented 4 years ago

Describe the bug The following auth plugins renew logic all enforce equivalent policies between the requests token and in the configured auth role.

However, the following auth backends do not enforce this.

Per the docs, "Lease renewal will fail if the token is not renewable, the token has already been revoked, or if the token has already reached its maximum TTL." Admittedly, "if the token is not renewable" is open to interpretation. However, all auth backends should be consistent in how they handle token renewals.

I realize that having the logic to fail a token renewal if the policies differ forces clients to re-authenticate to get a more up-to-date token more quickly, however AFAIK there is no security reason not to allow token renewal just because the policies configured on an auth role has changed.

Additionally, because other auth backends do not enforce this, adding the equivalent policy check to those could potentially break some clients, though, hopefully those clients are smart enough to re-authenticate with Vault.

Expected behavior For all auth backends, if a token has been issued a set of policies, re-configuration of the corresponding auth role policies should not cause token renewals to fail.

Environment:

jefferai commented 4 years ago

We can't just change this...it's changed behavior. Inconsistency is bad but sudden changes on upgrade are worse. This needs to have a broader discussion.

Also, you're changing it in those PRs the opposite way of how we have been trending -- there's a reason that the newer backends do it differently (maybe, I think -- point is, we need a discussion).

mdgreenfield commented 4 years ago

Thanks @jefferai. Definitely hear you about sudden changes.

Regarding the PRs, should the Vault maintainer choose to pursue addressing this I'm happy to update the PRs to be consistent with how your team(s) is trending. My hope is to have the renew logic consistent across plugins. From the vault-plugin-auth-example repo and the relatively recent vault-plugin-auth-cf repo it looked like the auth plugins were not enforcing equivalent policy checks on renew.

remilapeyre commented 1 year ago

There is also a bug in the way policies are compared for the auth method that check them during renewal: the comparison is case sensitive but policy names are actually case insensitive.

This means that having a policy name with an uppercase letter will make it impossible to renew a token. I opened a PR for this at: https://github.com/hashicorp/vault/pull/16484