From efc62d8e78b4e94594d06e4214ffb7d1cb1bedcd Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Fri, 4 Sep 2026 16:06:57 +0200 Subject: [PATCH] fix: bound the guest PEM key length before decoding or storing decoding_key_from_pem decoded an unbounded PEM: a well-formed key with an oversized modulus passes the structural check, is stored in the unbounded TEXT column, and is reparsed on every guest-JWT request. Refuse one longer than MAX_GUEST_PEM_LEN (8 KiB) at the same choke point the save path validates through, the way the JWKS URL is bounded. Also tighten two cap tests to assert their specific error rather than a substring another cap shares. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01VF3v6LA9399gNphmZaHYG3 --- backend/windmill-common/src/guest_jwt.rs | 31 ++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/backend/windmill-common/src/guest_jwt.rs b/backend/windmill-common/src/guest_jwt.rs index b8099f913f..df356ddba2 100644 --- a/backend/windmill-common/src/guest_jwt.rs +++ b/backend/windmill-common/src/guest_jwt.rs @@ -83,6 +83,14 @@ pub fn decoding_key_from_pem(pem: &str) -> Result<(DecodingKey, &'static [Algori use base64::{engine::general_purpose::STANDARD, Engine}; use spki::der::Decode; let pem = pem.trim(); + // The key is admin-set into an unbounded `TEXT` column and reparsed on every guest-JWT + // request; a well-formed key with an oversized modulus would pass the checks below. Refuse + // one larger than any real public key before decoding or storing it. + if pem.len() > MAX_GUEST_PEM_LEN { + return Err(Error::BadRequest(format!( + "guest key is longer than {MAX_GUEST_PEM_LEN} bytes" + ))); + } // A verification key must be public. jsonwebtoken 8.3 keys the public/private distinction // off the PEM label alone and never inspects the DER, so private material relabelled // `PUBLIC KEY` would be stored and then served back through the settings response. Decode @@ -256,6 +264,11 @@ const JWKS_MAX_KEYS: usize = 50; /// alone. A real JWKS URL is well under this; the check runs before the URL is ever cached. const MAX_JWKS_URL_LEN: usize = 2048; +/// A guest verification key is admin-set into an unbounded `TEXT` column and reparsed on every +/// guest-JWT request. A real public key PEM is under a few KB (RSA-16384 SPKI is ~2.8 KB), so +/// this bounds the stored and reparsed bytes without refusing any real key. +const MAX_GUEST_PEM_LEN: usize = 8 * 1024; + /// 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 @@ -566,7 +579,7 @@ mod tests { .collect(); let body = serde_json::json!({ "keys": keys }).to_string(); let err = parse_jwks_keys(body.as_bytes()).unwrap_err().to_string(); - assert!(err.contains("more than"), "{err}"); + assert!(err.contains("usable signing keys"), "{err}"); } #[test] @@ -585,11 +598,25 @@ mod tests { #[tokio::test] async fn an_overlong_jwks_url_is_refused() { // The cache keys on the URL string, so an unbounded URL is refused before it is cached. + // Assert the length error specifically: a bogus URL would fail the fetch regardless. let url = format!( "https://issuer.example.com/{}", "a".repeat(MAX_JWKS_URL_LEN) ); - assert!(fetch_jwks(&url).await.is_err()); + let err = fetch_jwks(&url).await.unwrap_err().to_string(); + assert!(err.contains("longer than"), "{err}"); + } + + #[test] + fn an_oversized_pem_is_refused() { + // A well-formed key body padded past the cap: refused for its length before decoding, + // so an oversized-but-valid key cannot be stored and reparsed on every request. + let big = format!( + "-----BEGIN PUBLIC KEY-----\n{}\n-----END PUBLIC KEY-----\n", + "A".repeat(MAX_GUEST_PEM_LEN) + ); + let err = decoding_key_from_pem(&big).err().unwrap().to_string(); + assert!(err.contains("longer than"), "{err}"); } #[test]