mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-07 00:01:49 +00:00
fix: validate the guest key is a public SPKI, not just its PEM label
jsonwebtoken 8.3 decides public vs private from the PEM label alone and never inspects the DER, so private material relabelled `PUBLIC KEY` (or a PKCS#1 key relabelled `RSA PUBLIC KEY`) passed the earlier label check and would be stored, then served back through the settings response. decoding_key_from_pem now parses the DER as a SubjectPublicKeyInfo, which only public keys satisfy, before it is persisted or used. The regression test relabels complete, valid private keys (EC PKCS#8, RSA PKCS#1) so the guard is what refuses them, not malformed DER. 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
51776ef87c
commit
97d06f99b0
Generated
+1
@@ -15618,6 +15618,7 @@ dependencies = [
|
||||
"serde_yml",
|
||||
"sha2 0.10.9",
|
||||
"size",
|
||||
"spki",
|
||||
"sqlx",
|
||||
"strum",
|
||||
"strum_macros",
|
||||
|
||||
@@ -600,6 +600,7 @@ const_format = { version = "0.2.35", features = ["rust_1_64", "rust_1_51"] }
|
||||
const-str = "0.5"
|
||||
constant_time_eq = "0.3.1"
|
||||
rsa = "^0"
|
||||
spki = { version = "0.7", features = ["pem"] }
|
||||
aes-gcm = "0.10.3"
|
||||
async_zip = { version = "0.0.17", features = ["tokio", "tokio-fs", "deflate", "chrono"] }
|
||||
once_cell = "1.17.1"
|
||||
|
||||
@@ -109,6 +109,7 @@ pep440_rs.workspace = true
|
||||
systemstat.workspace = true
|
||||
size.workspace = true
|
||||
rsa = { workspace = true, optional = true }
|
||||
spki = { workspace = true }
|
||||
aes-gcm = { workspace = true, optional = true }
|
||||
|
||||
semver.workspace = true
|
||||
|
||||
@@ -80,15 +80,20 @@ 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};
|
||||
let pem = pem.trim();
|
||||
// A verification key must be public. jsonwebtoken 8.3's `from_rsa_pem` also accepts private
|
||||
// encodings (PKCS#1, PKCS#8), so a pasted private key would be stored and then served back
|
||||
// through the settings response. Refuse any private PEM before it is parsed or persisted.
|
||||
if pem.contains("PRIVATE KEY") {
|
||||
return Err(Error::BadRequest(
|
||||
"expected a PEM public key (-----BEGIN PUBLIC KEY-----), not a private key".to_string(),
|
||||
));
|
||||
}
|
||||
// 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(),
|
||||
)
|
||||
})?;
|
||||
if let Ok(key) = DecodingKey::from_rsa_pem(pem.as_bytes()) {
|
||||
return Ok((key, &RSA_ALGORITHMS));
|
||||
}
|
||||
@@ -533,16 +538,61 @@ mod tests {
|
||||
.is_err());
|
||||
}
|
||||
|
||||
// A real RSA public key (SPKI). Its private counterpart is RSA_PKCS1_PRIVATE below.
|
||||
const RSA_PUBLIC: &str = "-----BEGIN PUBLIC KEY-----\n\
|
||||
MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAx3J0fQcHp2ZlMI4rCVsY\n\
|
||||
tirATZPWyPD7exoYWPInhV5xjbY2Fe8IVFaZszQcQbCXZjBtFp2fj0tBTow8BeOy\n\
|
||||
X9LJPyKeho/j68FycuDVg7JCzG0TWtsnh/V23WkrlKIfmMqS3+YUyFavROTcAN1T\n\
|
||||
5BcFHLAi/4Q2qy0JjXBdZ8avelzZrQ/T67/Kcsoct/pvnEDT2YRsSbA7VMWaxWh8\n\
|
||||
MYJ7GNV/10YT2c5CBJGSLbyRSVWk2IwfnM9Cl9n/5NE6TkSetYQ2xlqKTONp5W43\n\
|
||||
UzW1NAeqKCxPQfN/ADjwW18nk2o7xj1kMF4rBlhsTm9ClE71nwi5NxsvMOdVxytZ\n\
|
||||
ZQIDAQAB\n\
|
||||
-----END PUBLIC KEY-----\n";
|
||||
|
||||
// A complete, valid PKCS#1 RSA *private* key (the counterpart of RSA_PUBLIC) with its
|
||||
// armor relabelled `RSA PUBLIC KEY`. jsonwebtoken's from_rsa_pem accepts it under that
|
||||
// label; the SPKI parse is what refuses it. Complete on purpose: malformed DER would fail
|
||||
// for the wrong reason and let a real bypass through unnoticed.
|
||||
const RSA_PKCS1_PRIVATE_AS_PUBLIC: &str = "-----BEGIN RSA PUBLIC KEY-----\n\
|
||||
MIIEogIBAAKCAQEAx3J0fQcHp2ZlMI4rCVsYtirATZPWyPD7exoYWPInhV5xjbY2\n\
|
||||
Fe8IVFaZszQcQbCXZjBtFp2fj0tBTow8BeOyX9LJPyKeho/j68FycuDVg7JCzG0T\n\
|
||||
Wtsnh/V23WkrlKIfmMqS3+YUyFavROTcAN1T5BcFHLAi/4Q2qy0JjXBdZ8avelzZ\n\
|
||||
rQ/T67/Kcsoct/pvnEDT2YRsSbA7VMWaxWh8MYJ7GNV/10YT2c5CBJGSLbyRSVWk\n\
|
||||
2IwfnM9Cl9n/5NE6TkSetYQ2xlqKTONp5W43UzW1NAeqKCxPQfN/ADjwW18nk2o7\n\
|
||||
xj1kMF4rBlhsTm9ClE71nwi5NxsvMOdVxytZZQIDAQABAoIBAD+IbaQQM7d3Dj/X\n\
|
||||
4cyyqJ4K40QzFmXfIfTWXLAkv0MkUR7XzsXQ5YHcLkzgCipAwxGp1m4wWs4OJmkL\n\
|
||||
kek8XatZnYLPl9j8iBmm/zqp9Unk5JNzIYm9KwwLvMgOAvRvaopE6WGKTM9+kYls\n\
|
||||
L8rUti7/yECZuSRU7Qc9KwBTrWVrXK+RBtBqZYQXb92BFxq0N3Qp+utLNdFcO5sW\n\
|
||||
7d8gKp3ipQt5z9ZAB2pYMw7ZTzonF4C7HdyrbYXztvYrxuw1imMkQ9iFFhdn3/76\n\
|
||||
qFR7XwaFrld8DECGaH/652kV6zaSQijbBTeXF4zsgwXY4BHMVmZKaXH5unw8Gbmo\n\
|
||||
WCoLbLcCgYEA4vTo5kCiKXnlBp3W1Zpg4cml6Wzo0UDldF4kkuXQxhdQA38c0ise\n\
|
||||
Cocf6qyAqz1L2TxQ/9WCL2oIP1AY9XqnQ0cJtYIosGWORz4tPe67M8inB/GBotFI\n\
|
||||
pmQNVSIjqbgKVi0x+UzmFjitINFPf461lDdJTwhsv9TQRbHrXErvON8CgYEA4PhV\n\
|
||||
GYMJu46tqFVtD/koWAQRLmeaZXhxP5lMSmQjdYCa3ys5lccvTlzoF9K8immMQkIx\n\
|
||||
gyOazmEtFnK4IXmEY1wg2NIHuJM7/maoM2rozbjXBxsYM7Xw2QX7BHXqG6Ia/Bij\n\
|
||||
ZaRJdumCVRJv7OshQTGuqDIzd3l5WEqg11XYgjsCgYAP3v6Wc3ijm+GXN9x5LYWO\n\
|
||||
5JIUo8gYMgiZvaejGi0iXSj8RZxXWiqMo+xodc29q9itBVnIuj6TYD/ZZZmJOR2P\n\
|
||||
R9128vYzd7aeZsu1JAe1VFfR52KgZzBEaoTAKlYCHVujsR9ohqckcKwyulBr5Cfw\n\
|
||||
iHk47KbmN1SlOw7xclAOUwKBgAuxHEsdIk5bFe9fsTFZU51vaK0uuTl4zvntL6fW\n\
|
||||
GHms21+p0W5VUcIS1gUW8LGI1r9CzWvxV8RODJfUEnm65QR870AVek0/aajJEQjL\n\
|
||||
D5pRdutpnxJg7El7JBaRQj95Z0mexi8sIJ1LeXiOYr6/YZUPzfHz2fTlnUbXahCG\n\
|
||||
55+tAoGAI811NTb7kuuIPYuj4raDW88QVNX2xB3+p9lXGolB4jPgsUEjgSvLgH9S\n\
|
||||
Q/LwEBiCYVyii8MvWsIZpHvSGyOoty2p19/CAvrAOfpEVlnXQeiX+mh09p1mQbfM\n\
|
||||
y9rTR828ADcaZ63Ej1oL4GcqmGhODxCLy1YKKcy0FHzChqPMV6g=\n\
|
||||
-----END RSA PUBLIC KEY-----\n";
|
||||
|
||||
#[test]
|
||||
fn a_private_pem_is_refused() {
|
||||
// A verification key must be public; a private key must never be stored, as it is
|
||||
// served back through the settings response. PRIV1 is a real EC private key the PEM
|
||||
// parser would otherwise accept; the RSA label covers jsonwebtoken's PKCS#1/#8 case.
|
||||
// A verification key must be public and must never be stored otherwise: it is served
|
||||
// back through the settings response. jsonwebtoken keys the public/private split off
|
||||
// the PEM label, so private DER relabelled with a public armor slips a label check;
|
||||
// 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());
|
||||
assert!(decoding_key_from_pem(PRIV1).is_err());
|
||||
assert!(decoding_key_from_pem(
|
||||
"-----BEGIN RSA PRIVATE KEY-----\nMIIBOgIBAAJBAKj\n-----END RSA PRIVATE KEY-----\n"
|
||||
)
|
||||
.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());
|
||||
assert!(decoding_key_from_pem(RSA_PKCS1_PRIVATE_AS_PUBLIC).is_err());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
Reference in New Issue
Block a user