fix(oidc): key credential cache on (cache_key, subject, extra_claims)#104
Draft
alukach wants to merge 1 commit into
Draft
fix(oidc): key credential cache on (cache_key, subject, extra_claims)#104alukach wants to merge 1 commit into
alukach wants to merge 1 commit into
Conversation
The credential cache keyed on `cache_key` alone (the role ARN). But the backend's authorization gate — an AWS role trust policy, or the Azure/GCP equivalent — conditions on the *minted assertion* (its `subject` and any `extra_claims`) and is evaluated at mint time, inside the exchange. A cache hit skips the mint, so it skips that gate: two subjects sharing a role would share a cached credential, letting the second subject ride on credentials the trust policy might have denied it. `get_credentials` already receives `subject` and `extra_claims`, so fold them into the effective key. Doing it here (not at the call site) closes the footgun for every caller — none can forget to scope by identity. Length-prefixed framing keeps the key unambiguous so no crafted subject/ARN can forge another tuple's key. Cache granularity becomes per-(backend, identity), which is the correct scope — credentials already are per-subject — so hit rate is unaffected in practice. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
🚀 Latest commit deployed to https://multistore-proxy-pr-104.development-seed.workers.dev
|
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.
Problem
OidcCredentialProvider::get_credentialscaches minted backend credentials keyed oncache_keyalone (the role ARN). But the backend's authorization gate — an AWS role trust policy, or the Azure/GCP equivalent — conditions on the minted assertion (itssubject, plus anyextra_claims) and is evaluated at mint time, inside the exchange.A cache hit skips the mint, so it skips that gate. Two subjects that share a role therefore share a cached credential: the second subject rides on credentials the trust policy might have denied it. The subject here is the per-connection identity (
scv1:conn:{id}), which is exactly what an account owner conditions their trust policy on to restrict which connections/account-products may assume a shared role.Fix
get_credentialsalready receivessubjectandextra_claims, so fold them into the effective cache key: entries are now keyed per(cache_key, subject, extra_claims).get_credentials(not at the call site) so no caller can forget to scope by identity — it's closed for every backend.<len>:<value>) keeps the key unambiguous by construction: no crafted subject/ARN can forge another tuple's key. The key is a security boundary, so it can't rely on a delimiter that a value might contain.(backend, identity), which is the correct scope — credentials already are per-subject — so hit rate is unaffected in practice.Context
This is the L1 (in-isolate) half of a two-tier fix. The L2 (cross-isolate, Cloudflare Cache API) tier in
data.source.coophad the same role-ARN-only keying and is fixed in that repo's PR #175 by keying on(RoleArn, RoleSessionName). This PR fixes the root cause at the layer that owns the cache and has the rawsubject(lossless, vs. the sanitized/truncatedRoleSessionNamethe L2 form exposes).Tests
same_backend_different_subject_makes_separate_calls— same role, two subjects → two mints (the security property).credential_cache_key_is_unambiguous— length-prefix framing can't collide across tuples; subject and claims are part of the key; identical inputs still hit.🤖 Generated with Claude Code