agent/proxy: cancel the previous auto-auth token when re-authenticating - #32060
Open
golgoth31 wants to merge 3 commits into
Open
agent/proxy: cancel the previous auto-auth token when re-authenticating#32060golgoth31 wants to merge 3 commits into
golgoth31 wants to merge 3 commits into
Conversation
When auto-auth obtains a new token, the lease cache adds it alongside the one it replaces and leaves the old index in place with a live context. Vault revokes that token's child leases when it expires, so every lifetime watcher derived from it can only ever receive a 403 from sys/leases/renew. Those watchers keep retrying until each gives up on its own, which can take minutes and produce a large volume of requests that are guaranteed to be denied. RegisterAutoAuthToken now records the current auto-auth token and cancels the context of the one it replaces. A lease's renewal context is already derived from its token's context, so cancelling it stops the derived lease renewals and lets startRenewing evict them. The cascade already existed; nothing was driving it on re-authentication. The cancellation runs before the "token already cached" short-circuit, because that is the path taken after a restart with a persistent cache, where restoreTokens has already re-added the previous token. Recording the token only after that check would leave the tracker empty in exactly the case where the stale watchers are most likely to be running. Empty tokens are ignored: the sink can be handed one while auto-auth is shutting down, and that must not cancel the live token's context. As a side effect this also stops a junk index being persisted and clobbering the auto-auth token stored in the bolt meta bucket. Fixes hashicorp#25712
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Deployment failed with the following error: Learn More: https://vercel.com/docs/concepts/projects/project-configuration |
|
Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement Learn more about why HashiCorp requires a CLA and what the CLA includes Have you signed the CLA already but the status is still pending? Recheck it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #25712.
The problem
When auto-auth re-authenticates,
RegisterAutoAuthTokenadds the new token to the lease cache but never removes the one it replaces:https://github.com/hashicorp/vault/blob/main/command/agentproxyshared/cache/lease_cache.go#L1763-L1819
If the token is already indexed the call is a no-op; otherwise a new index is created with a context derived from the cache's base context. The previous token's index stays in the cache and its context is never cancelled.
Vault revokes a token's child leases when that token expires, so every lifetime watcher derived from the old token can only ever get
permission deniedback fromsys/leases/renew. Those watchers keep retrying until each one individually gives up.This is what #25712 reports, and it is still reproducible on
v2.0.3.Reproduction and measurements
Reproduced deliberately on Vault Agent 1.14.0 by setting a Kubernetes auth role's
token_max_ttlto 300s and a database secrets engine lease TTL of 600s, so the lease outlives its parent token. Three agent pods, one hour, 16 auto-auth generations each.From the agent's own debug logs, for one rotation:
The agent gets a healthy replacement token within 10 seconds. The orphaned watchers keep going for another 76 and 102 seconds.
From the Vault audit log over the same window, one token/lease pair, one pod:
Across three pods rotating out of phase this sustained roughly 1 100 req/s of requests that could not succeed, in ~2 minute bursts on every rotation, against a single-leader cluster. The commenter on #25712 reports the same magnitude — "from 30 request per seconds to over 1100 for 1 hour and 10 minutes" — on GCP rather than AWS, so this does not look auth-method specific.
Worth noting for anyone searching: none of this appears in the agent logs at
info. The lease cache logs renewal activity atdebug, and the failing requests themselves are never logged at all — only the eventualrenewal halted; evicting from cacheis.The change
RegisterAutoAuthTokennow records the current auto-auth token and cancels the context of the one it replaces.A lease's renewal context is already derived from its token's context:
https://github.com/hashicorp/vault/blob/main/command/agentproxyshared/cache/lease_cache.go#L544
so cancelling the token's context cascades to every lease obtained with it — the watchers stop and the entries are evicted through the existing
deferinstartRenewing. The cascade mechanism was already there; nothing was calling it on rotation.Three details worth pointing at during review:
restoreTokenshas already re-added the previous token, so auto-auth's first registration finds it and returns early. Recording the token only after that check would leave the tracker empty exactly in the scenario where the stale watchers are most likely to be running.TokenTypeindex andBoltStorage.Setthen overwrote theAutoAuthTokenmeta key with it, so on the next startpreviousTokencame back empty and the agent did a full re-authentication instead oflookup-selfwith the persisted token. Skipping the empty registration keeps that meta key intact.cancelPreviousAutoAuthToken, notevict...— the index itself stays in the cache with a cancelled context, which is what the"token"branch ofhandleCacheClearalready does. Only the derived leases are evicted, bystartRenewing'sdefer.Guarded by a dedicated mutex rather than the existing
c.l, sincec.lprotectsbaseCtxInfoandcreateCtxInfotakes anRLockon it.Trade-off to weigh
This cancels on any auto-auth token change, not only on expiry.
auth.goalso re-authenticates when the auth method reports new credentials (credCh) or after a transient lifetime-watcher error, and in those cases the previous token may still be valid. Its leases are then dropped from the cache and re-fetched on next use instead of being kept renewed. Nothing is revoked and nothing leaks — the failure direction is "less cached material" — but it is a behaviour change and it is documented in the function comment.The narrower alternative would be to drive the cancellation from a signal that the token is genuinely gone (the
InvalidToken/ permanent-lookup-failure paths) rather than from "a different token arrived". Happy to rework it that way if you would prefer; it did not seem worth the extra coupling for a case where the old token is almost always dead.Only the auto-auth token is affected. Client tokens that applications send through the proxy are indexed separately and are untouched.
This does not change the
api.LifetimeWatcherbackoff fixed in #26383; it removes the need to rely on it for this particular case.Testing
Three tests, measured against
main:main..._CancelsPreviousToken..._CancelsAfterRestore..._IgnoresEmptyTokenThe first two are regression tests for the reported bug — the second specifically covers the persistent-cache restore path. The third covers the empty-token guard and the requirement that an empty write leave the tracked token in place.
On
main:go test -race ./command/agentproxyshared/cache/...passes: 146 tests across 4 packages.go vetreports only the two pre-existing findings inlistener.goandstatic_secret_cache_updater.go, both untouched here.