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]