diff --git a/backend/Cargo.lock b/backend/Cargo.lock index d77be5e976..941c3c29ee 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -15618,6 +15618,7 @@ dependencies = [ "serde_yml", "sha2 0.10.9", "size", + "spki", "sqlx", "strum", "strum_macros", diff --git a/backend/Cargo.toml b/backend/Cargo.toml index 25f2165bd2..e58cb3f5d0 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -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" diff --git a/backend/windmill-common/Cargo.toml b/backend/windmill-common/Cargo.toml index e2c3233183..76b7b8cbf3 100644 --- a/backend/windmill-common/Cargo.toml +++ b/backend/windmill-common/Cargo.toml @@ -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 diff --git a/backend/windmill-common/src/guest_jwt.rs b/backend/windmill-common/src/guest_jwt.rs index c82f5ac06e..d496cc6a01 100644 --- a/backend/windmill-common/src/guest_jwt.rs +++ b/backend/windmill-common/src/guest_jwt.rs @@ -80,15 +80,20 @@ pub async fn key_source(db: &DB, w_id: &str) -> Result /// 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]