mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-06 16:02:23 +00:00
fix: accept any PEM wrapping for the guest key, keep the public-key check
The SPKI guard decoded the body with the strict RFC 7468 decoder, which enforces 64-column wrapping, so a legitimate public key wrapped otherwise (or a PKCS#1 `RSA PUBLIC KEY`) was refused where jsonwebtoken would have parsed it. Decode the body leniently like jsonwebtoken, then require the DER to be a public-key structure: an SPKI (RSA or EC) or a PKCS#1 RSA public key. Private material satisfies neither, so the round-13 bypass stays closed. Test adds a one-line (non-64-column) public key as a positive control. 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
97d06f99b0
commit
f5713621c8
Generated
+1
@@ -15602,6 +15602,7 @@ dependencies = [
|
||||
"pep440_rs",
|
||||
"phf 0.11.3",
|
||||
"pin-project-lite",
|
||||
"pkcs1",
|
||||
"postgres-native-tls 0.5.3",
|
||||
"prometheus",
|
||||
"quick_cache",
|
||||
|
||||
@@ -601,6 +601,7 @@ const-str = "0.5"
|
||||
constant_time_eq = "0.3.1"
|
||||
rsa = "^0"
|
||||
spki = { version = "0.7", features = ["pem"] }
|
||||
pkcs1 = "0.7"
|
||||
aes-gcm = "0.10.3"
|
||||
async_zip = { version = "0.0.17", features = ["tokio", "tokio-fs", "deflate", "chrono"] }
|
||||
once_cell = "1.17.1"
|
||||
|
||||
@@ -110,6 +110,7 @@ systemstat.workspace = true
|
||||
size.workspace = true
|
||||
rsa = { workspace = true, optional = true }
|
||||
spki = { workspace = true }
|
||||
pkcs1 = { workspace = true }
|
||||
aes-gcm = { workspace = true, optional = true }
|
||||
|
||||
semver.workspace = true
|
||||
|
||||
@@ -80,20 +80,30 @@ pub async fn key_source(db: &DB, w_id: &str) -> Result<Option<GuestJwtKeySource>
|
||||
/// EC keys the ES family. Anything symmetric has no PEM form, so HS* is unreachable
|
||||
/// from here by construction; the JWKS path refuses it explicitly.
|
||||
pub fn decoding_key_from_pem(pem: &str) -> Result<(DecodingKey, &'static [Algorithm])> {
|
||||
use spki::der::{Decode, Document};
|
||||
use base64::{engine::general_purpose::STANDARD, Engine};
|
||||
use spki::der::Decode;
|
||||
let pem = pem.trim();
|
||||
// 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. Require
|
||||
// the key to parse as a SubjectPublicKeyInfo, which private-key DER cannot satisfy.
|
||||
let (_, doc) = Document::from_pem(pem)
|
||||
.map_err(|e| Error::BadRequest(format!("not a valid PEM key: {e}")))?;
|
||||
spki::SubjectPublicKeyInfoRef::from_der(doc.as_bytes()).map_err(|_| {
|
||||
Error::BadRequest(
|
||||
"expected an RSA or EC public key in SPKI form (-----BEGIN PUBLIC KEY-----)"
|
||||
.to_string(),
|
||||
// `PUBLIC KEY` would be stored and then served back through the settings response. Decode
|
||||
// the body leniently, as jsonwebtoken does (tolerating any wrapping the strict RFC 7468
|
||||
// decoder would refuse), then require it to be a public-key structure: an SPKI (RSA or EC)
|
||||
// or a PKCS#1 RSA public key. Private-key DER satisfies neither.
|
||||
let der = STANDARD
|
||||
.decode(
|
||||
pem.lines()
|
||||
.filter(|l| !l.trim_start().starts_with("-----"))
|
||||
.flat_map(|l| l.split_whitespace())
|
||||
.collect::<String>(),
|
||||
)
|
||||
})?;
|
||||
.map_err(|e| Error::BadRequest(format!("guest key is not valid PEM: {e}")))?;
|
||||
let is_public = spki::SubjectPublicKeyInfoRef::from_der(&der).is_ok()
|
||||
|| pkcs1::RsaPublicKey::from_der(&der).is_ok();
|
||||
if !is_public {
|
||||
return Err(Error::BadRequest(
|
||||
"expected an RSA or EC public key in PEM form (-----BEGIN PUBLIC KEY-----)".to_string(),
|
||||
));
|
||||
}
|
||||
if let Ok(key) = DecodingKey::from_rsa_pem(pem.as_bytes()) {
|
||||
return Ok((key, &RSA_ALGORITHMS));
|
||||
}
|
||||
@@ -589,6 +599,15 @@ y9rTR828ADcaZ63Ej1oL4GcqmGhODxCLy1YKKcy0FHzChqPMV6g=\n\
|
||||
// only parsing the DER as a SubjectPublicKeyInfo refuses it. A real public key still
|
||||
// parses, so the guard is not vacuous.
|
||||
assert!(decoding_key_from_pem(RSA_PUBLIC).is_ok());
|
||||
// The same key with its body on one line (not 64-column wrapped): jsonwebtoken accepts
|
||||
// any wrapping, so the guard must too rather than lean on the strict RFC 7468 decoder.
|
||||
let body: String = RSA_PUBLIC
|
||||
.lines()
|
||||
.filter(|l| !l.starts_with("-----"))
|
||||
.collect();
|
||||
let one_line = format!("-----BEGIN PUBLIC KEY-----\n{body}\n-----END PUBLIC KEY-----\n");
|
||||
assert!(decoding_key_from_pem(&one_line).is_ok());
|
||||
|
||||
assert!(decoding_key_from_pem(PRIV1).is_err());
|
||||
// The same PKCS#8 EC private key, relabelled `PUBLIC KEY`.
|
||||
assert!(decoding_key_from_pem(&PRIV1.replace("PRIVATE KEY", "PUBLIC KEY")).is_err());
|
||||
|
||||
Reference in New Issue
Block a user