From acfe95b29d6eb7cae47c4b1d09b14750b692fec1 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Thu, 2 Jul 2026 11:10:53 -0700 Subject: [PATCH] fix(oidc): key credential cache on (cache_key, subject, extra_claims) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- Cargo.lock | 20 +++---- crates/oidc-provider/src/lib.rs | 94 +++++++++++++++++++++++++++++++-- 2 files changed, 100 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c454468..26354cc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1121,7 +1121,7 @@ dependencies = [ [[package]] name = "multistore" -version = "0.6.2" +version = "0.6.3" dependencies = [ "async-trait", "base64", @@ -1147,7 +1147,7 @@ dependencies = [ [[package]] name = "multistore-cf-workers" -version = "0.6.2" +version = "0.6.3" dependencies = [ "async-trait", "bytes", @@ -1170,7 +1170,7 @@ dependencies = [ [[package]] name = "multistore-cf-workers-example" -version = "0.6.2" +version = "0.6.3" dependencies = [ "bytes", "console_error_panic_hook", @@ -1194,7 +1194,7 @@ dependencies = [ [[package]] name = "multistore-lambda" -version = "0.6.2" +version = "0.6.3" dependencies = [ "bytes", "http", @@ -1214,7 +1214,7 @@ dependencies = [ [[package]] name = "multistore-metering" -version = "0.6.2" +version = "0.6.3" dependencies = [ "bytes", "futures", @@ -1226,7 +1226,7 @@ dependencies = [ [[package]] name = "multistore-oidc-provider" -version = "0.6.2" +version = "0.6.3" dependencies = [ "base64", "chrono", @@ -1246,7 +1246,7 @@ dependencies = [ [[package]] name = "multistore-path-mapping" -version = "0.6.2" +version = "0.6.3" dependencies = [ "multistore", "percent-encoding", @@ -1255,7 +1255,7 @@ dependencies = [ [[package]] name = "multistore-server" -version = "0.6.2" +version = "0.6.3" dependencies = [ "axum", "bytes", @@ -1277,7 +1277,7 @@ dependencies = [ [[package]] name = "multistore-static-config" -version = "0.6.2" +version = "0.6.3" dependencies = [ "chrono", "multistore", @@ -1289,7 +1289,7 @@ dependencies = [ [[package]] name = "multistore-sts" -version = "0.6.2" +version = "0.6.3" dependencies = [ "aes-gcm", "base64", diff --git a/crates/oidc-provider/src/lib.rs b/crates/oidc-provider/src/lib.rs index 2555063..c74e8b3 100644 --- a/crates/oidc-provider/src/lib.rs +++ b/crates/oidc-provider/src/lib.rs @@ -89,10 +89,19 @@ impl OidcCredentialProvider { /// Get credentials for a backend, using cached values when available. /// /// `exchange` describes how to trade the self-signed JWT for cloud - /// credentials (AWS, Azure, GCP). `cache_key` identifies the backend for - /// caching purposes (e.g. the role ARN). + /// credentials (AWS, Azure, GCP). `cache_key` identifies the backend (e.g. + /// the role ARN). /// - /// Concurrent calls for the same `cache_key` are single-flighted: only one + /// Credentials are cached per `(cache_key, subject, extra_claims)`, **not** + /// per `cache_key` alone. The backend's authorization gate (an AWS role trust + /// policy, an Azure/GCP equivalent) conditions on the *minted assertion* — + /// its `subject` and any `extra_claims` — and is evaluated at mint time. A + /// cache hit skips the mint, so it skips that gate; keying on `cache_key` + /// alone would let a credential minted for one subject be served to a + /// different subject that shares the backend but that the trust policy would + /// reject. So every input the gate sees is part of the key. + /// + /// Concurrent calls for the same effective key are single-flighted: only one /// JWT mint + exchange runs, and the rest await its result. A cached value /// is reused until it nears expiry, then proactively re-minted. pub async fn get_credentials>( @@ -102,8 +111,9 @@ impl OidcCredentialProvider { subject: &str, extra_claims: &[(&str, &str)], ) -> Result, OidcProviderError> { + let key = credential_cache_key(cache_key, subject, extra_claims); self.cache - .get_or_fetch(cache_key, || async { + .get_or_fetch(&key, || async { // Cache miss (or due for refresh): mint a JWT and exchange it. let token = self.signer @@ -120,6 +130,30 @@ impl OidcCredentialProvider { } } +/// Build the effective credential-cache key from every input that changes the +/// minted assertion the backend's trust policy evaluates: the backend +/// `cache_key`, the `subject`, and each `extra_claims` pair. +/// +/// Length-prefixed (`:`) rather than delimiter-joined so no crafted +/// component value can collide with a different tuple — the key is a security +/// boundary (share a credential across subjects and you bypass a per-subject +/// trust-policy denial), so it must be unambiguous by construction. +fn credential_cache_key(cache_key: &str, subject: &str, extra_claims: &[(&str, &str)]) -> String { + let mut key = String::new(); + let mut push = |part: &str| { + key.push_str(&part.len().to_string()); + key.push(':'); + key.push_str(part); + }; + push(cache_key); + push(subject); + for (k, v) in extra_claims { + push(k); + push(v); + } + key +} + /// Errors produced by this crate. #[derive(Debug, thiserror::Error)] pub enum OidcProviderError { @@ -335,6 +369,58 @@ mod tests { assert_eq!(http.calls(), 2); } + #[tokio::test] + async fn same_backend_different_subject_makes_separate_calls() { + // Security boundary: two subjects sharing a role must NOT share a cached + // credential — the role's trust policy conditions on the subject at mint + // time, and a cache hit would bypass a denial for the second subject. + let http = MockHttp::new(); + let provider = OidcCredentialProvider::new( + test_signer(), + http.clone(), + "https://issuer.example.com".into(), + "sts.amazonaws.com".into(), + ); + let exchange = exchange::aws::AwsExchange::new("arn:aws:iam::123:role/Test".into()); + + provider + .get_credentials("role-a", &exchange, "scv1:conn:A", &[]) + .await + .unwrap(); + provider + .get_credentials("role-a", &exchange, "scv1:conn:B", &[]) + .await + .unwrap(); + + assert_eq!(http.calls(), 2, "distinct subjects must each mint"); + } + + #[test] + fn credential_cache_key_is_unambiguous() { + // Length-prefix framing must not let a crafted value forge another + // tuple's key: (role="a:b", sub="c") and (role="a", sub="b:c") share the + // naive `a:b\x1fc` vs `a\x1fb:c` hazard under a plain delimiter, but must + // differ here. + assert_ne!( + credential_cache_key("a:b", "c", &[]), + credential_cache_key("a", "b:c", &[]), + ); + // Subject is part of the key; claims are too. + assert_ne!( + credential_cache_key("role", "subA", &[]), + credential_cache_key("role", "subB", &[]), + ); + assert_ne!( + credential_cache_key("role", "sub", &[]), + credential_cache_key("role", "sub", &[("x", "1")]), + ); + // Same inputs → same key (cache actually hits). + assert_eq!( + credential_cache_key("role", "sub", &[("x", "1")]), + credential_cache_key("role", "sub", &[("x", "1")]), + ); + } + #[test] fn signed_jwt_is_verifiable_via_jwks_public_key() { use base64::Engine;