From 7d3a9fd5cd0784efcea3333574310d14596e0e8e Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Fri, 4 Sep 2026 15:19:42 +0200 Subject: [PATCH] fix: bound a JWKS cache entry by key count, not only bytes JWKS_MAX_BYTES caps one response and the cache caps entry count (200), but a densely packed 1 MiB body parses into thousands of keys, so one entry could retain a few MB and a workspace admin rotating URLs could grow the shared process by hundreds of MB. Refuse a set larger than JWKS_MAX_KEYS (50) in parse_jwks_keys, which bounds retention at the source; real issuers publish a handful of keys. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01VF3v6LA9399gNphmZaHYG3 --- backend/windmill-common/src/guest_jwt.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/backend/windmill-common/src/guest_jwt.rs b/backend/windmill-common/src/guest_jwt.rs index 47c138ce7b..f76490cc2f 100644 --- a/backend/windmill-common/src/guest_jwt.rs +++ b/backend/windmill-common/src/guest_jwt.rs @@ -241,6 +241,11 @@ const JWKS_NEGATIVE_TTL: Duration = Duration::from_secs(30); /// but the server it names may be attacker-controlled, and a real key set is a few KB. const JWKS_MAX_BYTES: usize = 1 << 20; +/// A real JWKS carries a handful of keys. A set larger than this is refused so one cache +/// entry cannot retain a densely packed body (the byte cap alone allows thousands of minimal +/// JWKs), which a workspace admin rotating URLs could otherwise use to grow the shared process. +const JWKS_MAX_KEYS: usize = 50; + /// Fetch a JWKS, keeping only the keys usable here. The URL was set by a workspace /// admin, so it is validated against private ranges and the connect is pinned to the /// validated addresses; redirects are not followed for the same reason. The body is @@ -293,6 +298,11 @@ fn parse_jwks_keys(body: &[u8]) -> Result> { .get("keys") .and_then(|k| k.as_array()) .ok_or_else(|| Error::BadRequest("JWKS has no `keys` array".to_string()))?; + if entries.len() > JWKS_MAX_KEYS { + return Err(Error::BadRequest(format!( + "JWKS holds more than {JWKS_MAX_KEYS} keys" + ))); + } let keys: HashMap = entries .iter() .filter_map(|entry| serde_json::from_value::(entry.clone()).ok()) @@ -518,6 +528,17 @@ mod tests { assert!(parse_jwks_keys(body.as_bytes()).is_err()); } + #[test] + fn an_oversized_jwks_is_refused() { + // The byte cap alone allows thousands of minimal keys in one entry; the count cap + // bounds what a single cache entry can retain. + let keys: Vec<_> = (0..=JWKS_MAX_KEYS) + .map(|i| serde_json::json!({"kty":"RSA","kid":format!("k{i}"),"n":"aa","e":"AQAB"})) + .collect(); + let body = serde_json::json!({ "keys": keys }).to_string(); + assert!(parse_jwks_keys(body.as_bytes()).is_err()); + } + #[test] fn key_ops_without_verify_is_refused() { let enc = jwk(serde_json::json!({"kty":"RSA","key_ops":["encrypt"],"n":"aa","e":"AQAB"}));