mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-07 00:01:49 +00:00
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VF3v6LA9399gNphmZaHYG3
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
1db28b80b2
commit
efc62d8e78
@@ -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]
|
||||
|
||||
Reference in New Issue
Block a user