From 74ba29d97742c5dad9d7c994781e511e29b568d1 Mon Sep 17 00:00:00 2001 From: Philippe Laflamme Date: Thu, 31 Jul 2025 23:41:16 -0400 Subject: [PATCH] feat: add a feature toggle for `rsa` The `rsa` crate has an [open CVE](https://www.cvedetails.com/cve/CVE-2023-49092/). which seems [non-trivial to fix](https://github.com/RustCrypto/RSA/issues/19). There has been some recent progress, but it's still unclear if it will actually fix the issue. I'm actively working on getting native SSH support in gitoxide [here](https://github.com/GitoxideLabs/gitoxide/pull/2081) and this particular CVE may become an issue for that integration. This commit adds a feature toggle to enable/disable the `rsa` dependency. This also fixes a race condition in the `test_agent` test. --- Cargo.toml | 3 --- russh/Cargo.toml | 7 ++++--- russh/src/helpers.rs | 7 +++---- russh/src/keys/format/mod.rs | 25 +++++++++++++++++++------ russh/src/keys/format/pkcs5.rs | 13 ++++++++++++- russh/src/keys/format/pkcs8.rs | 2 ++ russh/src/keys/key.rs | 9 ++++++--- russh/src/keys/mod.rs | 27 +++++++++++++++++++++------ 8 files changed, 67 insertions(+), 26 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 766b149d..237af1bb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,15 +15,12 @@ home = "0.5" hmac = "0.12" log = "0.4.11" rand = "0.8" -rsa = "0.9" sha1 = { version = "0.10.5", features = ["oid"] } sha2 = { version = "0.10.6", features = ["oid"] } signature = "2.2" ssh-encoding = { version = "0.2", features = ["bytes"] } ssh-key = { version = "=0.6.11", features = [ "ed25519", - "rsa", - "rsa-sha1", "p256", "p384", "p521", diff --git a/russh/Cargo.toml b/russh/Cargo.toml index 47719109..fa8da16d 100644 --- a/russh/Cargo.toml +++ b/russh/Cargo.toml @@ -13,7 +13,7 @@ version = "0.53.0" rust-version = "1.75" [features] -default = ["flate2", "aws-lc-rs"] +default = ["flate2", "aws-lc-rs", "rsa"] aws-lc-rs = ["dep:aws-lc-rs"] async-trait = ["dep:async-trait"] legacy-ed25519-pkcs8-parser = ["yasna"] @@ -22,6 +22,7 @@ des = ["dep:des"] # Danger: DSA algorithm is insecure. dsa = ["ssh-key/dsa"] ring = ["dep:ring"] +rsa = ["dep:rsa", "dep:pkcs1", "ssh-key/rsa", "ssh-key/rsa-sha1"] _bench = ["dep:criterion"] [dependencies] @@ -60,13 +61,13 @@ p256 = { version = "0.13", features = ["ecdh"] } p384 = { version = "0.13", features = ["ecdh"] } p521 = { version = "0.13", features = ["ecdh"] } pbkdf2 = "0.12" -pkcs1 = "0.7" +pkcs1 = { version = "0.7", optional = true } pkcs5 = "0.7" pkcs8 = { version = "0.10", features = ["pkcs5", "encryption"] } rand_core = { version = "0.6.4", features = ["getrandom", "std"] } rand.workspace = true ring = { version = "0.17.14", optional = true } -rsa.workspace = true +rsa = { version = "0.9", optional = true } russh-cryptovec = { version = "0.52.0", path = "../cryptovec", features = [ "ssh-encoding", ] } diff --git a/russh/src/helpers.rs b/russh/src/helpers.rs index b54c5baa..f8cc7bef 100644 --- a/russh/src/helpers.rs +++ b/russh/src/helpers.rs @@ -1,8 +1,6 @@ use std::fmt::Debug; use ssh_encoding::{Decode, Encode}; -use ssh_key::private::KeypairData; -use ssh_key::Algorithm; #[doc(hidden)] pub trait EncodedExt { @@ -68,8 +66,9 @@ pub use map_err; #[doc(hidden)] pub fn sign_with_hash_alg(key: &PrivateKeyWithHashAlg, data: &[u8]) -> ssh_key::Result> { Ok(match key.key_data() { - KeypairData::Rsa(rsa_keypair) => { - let Algorithm::Rsa { hash } = key.algorithm() else { + #[cfg(feature = "rsa")] + ssh_key::private::KeypairData::Rsa(rsa_keypair) => { + let ssh_key::Algorithm::Rsa { hash } = key.algorithm() else { unreachable!(); }; signature::Signer::try_sign(&(rsa_keypair, hash), data)?.encoded()? diff --git a/russh/src/keys/format/mod.rs b/russh/src/keys/format/mod.rs index d2e6d4e0..c85d2506 100644 --- a/russh/src/keys/format/mod.rs +++ b/russh/src/keys/format/mod.rs @@ -1,9 +1,6 @@ -use std::convert::TryInto; use std::io::Write; use data_encoding::{BASE64_MIME, HEXLOWER_PERMISSIVE}; -use pkcs1::DecodeRsaPrivateKey; -use ssh_key::private::RsaKeypair; use ssh_key::PrivateKey; use super::is_base64_char; @@ -37,6 +34,7 @@ pub enum Encryption { #[derive(Clone, Debug)] enum Format { + #[cfg(feature = "rsa")] Rsa, Openssh, Pkcs5Encrypted(Encryption), @@ -76,8 +74,18 @@ pub fn decode_secret_key(secret: &str, password: Option<&str>) -> Result) -> Result decode_openssh(&secret, password), + #[cfg(feature = "rsa")] Some(Format::Rsa) => Ok(decode_rsa_pkcs1_der(&secret)?.into()), Some(Format::Pkcs5Encrypted(enc)) => decode_pkcs5(&secret, password, enc), Some(Format::Pkcs8Encrypted) | Some(Format::Pkcs8) => { @@ -133,6 +142,10 @@ pub fn encode_pkcs8_pem_encrypted( Ok(()) } -fn decode_rsa_pkcs1_der(secret: &[u8]) -> Result { +#[cfg(feature = "rsa")] +fn decode_rsa_pkcs1_der(secret: &[u8]) -> Result { + use pkcs1::DecodeRsaPrivateKey; + use std::convert::TryInto; + Ok(rsa::RsaPrivateKey::from_pkcs1_der(secret)?.try_into()?) } diff --git a/russh/src/keys/format/pkcs5.rs b/russh/src/keys/format/pkcs5.rs index 47d4fc18..6d5e5b83 100644 --- a/russh/src/keys/format/pkcs5.rs +++ b/russh/src/keys/format/pkcs5.rs @@ -29,7 +29,18 @@ pub fn decode_pkcs5( } Encryption::Aes256Cbc(_) => unimplemented!(), }; - super::decode_rsa_pkcs1_der(&sec).map(Into::into) + // TODO: presumably pkcs5 could contain non-RSA keys? + #[cfg(feature = "rsa")] + { + super::decode_rsa_pkcs1_der(&sec).map(Into::into) + } + #[cfg(not(feature = "rsa"))] + { + Err(Error::UnsupportedKeyType { + key_type_string: "RSA".to_string(), + key_type_raw: vec![], + }) + } } else { Err(Error::KeyIsEncrypted) } diff --git a/russh/src/keys/format/pkcs8.rs b/russh/src/keys/format/pkcs8.rs index 5753af9a..d9bf947f 100644 --- a/russh/src/keys/format/pkcs8.rs +++ b/russh/src/keys/format/pkcs8.rs @@ -49,6 +49,7 @@ fn pkcs8_pki_into_keypair_data(pki: PrivateKeyInfo<'_>) -> Result { let sk = &pkcs1::RsaPrivateKey::try_from(pki.private_key)?; let pk = rsa::RsaPrivateKey::from_components( @@ -137,6 +138,7 @@ pub fn encode_pkcs8(key: &ssh_key::PrivateKey) -> Result, Error> { let sk: ed25519_dalek::SigningKey = pair.try_into()?; sk.to_pkcs8_der()? } + #[cfg(feature = "rsa")] ssh_key::private::KeypairData::Rsa(ref pair) => { let sk: rsa::RsaPrivateKey = pair.try_into()?; sk.to_pkcs8_der()? diff --git a/russh/src/keys/key.rs b/russh/src/keys/key.rs index 81f4ccc9..344500c7 100644 --- a/russh/src/keys/key.rs +++ b/russh/src/keys/key.rs @@ -14,7 +14,7 @@ // use ssh_encoding::Decode; use ssh_key::public::KeyData; -use ssh_key::{Algorithm, EcdsaCurve, HashAlg, PublicKey}; +use ssh_key::{Algorithm, EcdsaCurve, PublicKey}; use crate::keys::Error; @@ -109,12 +109,15 @@ pub const ALL_KEY_TYPES: &[Algorithm] = &[ curve: EcdsaCurve::NistP521, }, Algorithm::Ed25519, + #[cfg(feature = "rsa")] Algorithm::Rsa { hash: None }, + #[cfg(feature = "rsa")] Algorithm::Rsa { - hash: Some(HashAlg::Sha256), + hash: Some(ssh_key::HashAlg::Sha256), }, + #[cfg(feature = "rsa")] Algorithm::Rsa { - hash: Some(HashAlg::Sha512), + hash: Some(ssh_key::HashAlg::Sha512), }, Algorithm::SkEcdsaSha2NistP256, Algorithm::SkEd25519, diff --git a/russh/src/keys/mod.rs b/russh/src/keys/mod.rs index f5889e61..9f1c4b90 100644 --- a/russh/src/keys/mod.rs +++ b/russh/src/keys/mod.rs @@ -5,7 +5,7 @@ //! //! The following example shows how to do all these in a single example: //! start and SSH agent server, connect to it with a client, decipher -//! an encrypted private key (the password is `b"blabla"`), send it to +//! an encrypted ED25519 private key (the password is `b"blabla"`), send it to //! the agent, and ask the agent to sign a piece of data //! (`b"Please sign this"`, below). //! @@ -21,7 +21,7 @@ //! } //! } //! -//! const PKCS8_ENCRYPTED: &'static str = "-----BEGIN ENCRYPTED PRIVATE KEY-----\nMIIFLTBXBgkqhkiG9w0BBQ0wSjApBgkqhkiG9w0BBQwwHAQITo1O0b8YrS0CAggA\nMAwGCCqGSIb3DQIJBQAwHQYJYIZIAWUDBAEqBBBtLH4T1KOfo1GGr7salhR8BIIE\n0KN9ednYwcTGSX3hg7fROhTw7JAJ1D4IdT1fsoGeNu2BFuIgF3cthGHe6S5zceI2\nMpkfwvHbsOlDFWMUIAb/VY8/iYxhNmd5J6NStMYRC9NC0fVzOmrJqE1wITqxtORx\nIkzqkgFUbaaiFFQPepsh5CvQfAgGEWV329SsTOKIgyTj97RxfZIKA+TR5J5g2dJY\nj346SvHhSxJ4Jc0asccgMb0HGh9UUDzDSql0OIdbnZW5KzYJPOx+aDqnpbz7UzY/\nP8N0w/pEiGmkdkNyvGsdttcjFpOWlLnLDhtLx8dDwi/sbEYHtpMzsYC9jPn3hnds\nTcotqjoSZ31O6rJD4z18FOQb4iZs3MohwEdDd9XKblTfYKM62aQJWH6cVQcg+1C7\njX9l2wmyK26Tkkl5Qg/qSfzrCveke5muZgZkFwL0GCcgPJ8RixSB4GOdSMa/hAMU\nkvFAtoV2GluIgmSe1pG5cNMhurxM1dPPf4WnD+9hkFFSsMkTAuxDZIdDk3FA8zof\nYhv0ZTfvT6V+vgH3Hv7Tqcxomy5Qr3tj5vvAqqDU6k7fC4FvkxDh2mG5ovWvc4Nb\nXv8sed0LGpYitIOMldu6650LoZAqJVv5N4cAA2Edqldf7S2Iz1QnA/usXkQd4tLa\nZ80+sDNv9eCVkfaJ6kOVLk/ghLdXWJYRLenfQZtVUXrPkaPpNXgD0dlaTN8KuvML\nUw/UGa+4ybnPsdVflI0YkJKbxouhp4iB4S5ACAwqHVmsH5GRnujf10qLoS7RjDAl\no/wSHxdT9BECp7TT8ID65u2mlJvH13iJbktPczGXt07nBiBse6OxsClfBtHkRLzE\nQF6UMEXsJnIIMRfrZQnduC8FUOkfPOSXc8r9SeZ3GhfbV/DmWZvFPCpjzKYPsM5+\nN8Bw/iZ7NIH4xzNOgwdp5BzjH9hRtCt4sUKVVlWfEDtTnkHNOusQGKu7HkBF87YZ\nRN/Nd3gvHob668JOcGchcOzcsqsgzhGMD8+G9T9oZkFCYtwUXQU2XjMN0R4VtQgZ\nrAxWyQau9xXMGyDC67gQ5xSn+oqMK0HmoW8jh2LG/cUowHFAkUxdzGadnjGhMOI2\nzwNJPIjF93eDF/+zW5E1l0iGdiYyHkJbWSvcCuvTwma9FIDB45vOh5mSR+YjjSM5\nnq3THSWNi7Cxqz12Q1+i9pz92T2myYKBBtu1WDh+2KOn5DUkfEadY5SsIu/Rb7ub\n5FBihk2RN3y/iZk+36I69HgGg1OElYjps3D+A9AjVby10zxxLAz8U28YqJZm4wA/\nT0HLxBiVw+rsHmLP79KvsT2+b4Diqih+VTXouPWC/W+lELYKSlqnJCat77IxgM9e\nYIhzD47OgWl33GJ/R10+RDoDvY4koYE+V5NLglEhbwjloo9Ryv5ywBJNS7mfXMsK\n/uf+l2AscZTZ1mhtL38efTQCIRjyFHc3V31DI0UdETADi+/Omz+bXu0D5VvX+7c6\nb1iVZKpJw8KUjzeUV8yOZhvGu3LrQbhkTPVYL555iP1KN0Eya88ra+FUKMwLgjYr\nJkUx4iad4dTsGPodwEP/Y9oX/Qk3ZQr+REZ8lg6IBoKKqqrQeBJ9gkm1jfKE6Xkc\nCog3JMeTrb3LiPHgN6gU2P30MRp6L1j1J/MtlOAr5rux\n-----END ENCRYPTED PRIVATE KEY-----\n"; +//! const PKCS8_ENCRYPTED: &'static str = "-----BEGIN ENCRYPTED PRIVATE KEY-----\nMIGjMF8GCSqGSIb3DQEFDTBSMDEGCSqGSIb3DQEFDDAkBBAWQiUHKoocuxfoZ/hF\nYTjkAgIIADAMBggqhkiG9w0CCQUAMB0GCWCGSAFlAwQBKgQQ83d1d5/S2wz475uC\nCUrE7QRAvdVpD5e3zKH/MZjilWrMOm6cyI1LKBCssLztPyvOALtroLAPlp7WYWfu\n9Sncmm7u14n2lia7r1r5I3VBsVuH0g==\n-----END ENCRYPTED PRIVATE KEY-----\n"; //! //! #[cfg(unix)] //! fn main() { @@ -137,6 +137,7 @@ pub enum Error { #[error(transparent)] IO(#[from] std::io::Error), + #[cfg(feature = "rsa")] #[error("Rsa: {0}")] Rsa(#[from] rsa::Error), @@ -152,6 +153,7 @@ pub enum Error { Der(#[from] der::Error), #[error("Spki: {0}")] Spki(#[from] spki::Error), + #[cfg(feature = "rsa")] #[error("Pkcs1: {0}")] Pkcs1(#[from] pkcs1::Error), #[error("Pkcs8: {0}")] @@ -301,6 +303,7 @@ dP3jryYgvsCIBAA5jMWSjrmnOTXhidqcOy4xYCrAttzSnZ/cUadfBenL+DQq6neffw7j8r sJWR7W+cGvJ/vLsw== -----END OPENSSH PRIVATE KEY-----"; + #[cfg(feature = "rsa")] const RSA_KEY: &str = "-----BEGIN OPENSSH PRIVATE KEY----- b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAABFwAAAAdzc2gtcn NhAAAAAwEAAQAAAQEAuSvQ9m76zhRB4m0BUKPf17lwccj7KQ1Qtse63AOqP/VYItqEH8un @@ -377,6 +380,7 @@ Z9w7lshQhqowtrbLDFw4rXAxZuE= // We can't encode attributes, skip test_decode_encode_symmetry. } + #[cfg(feature = "rsa")] #[test] fn test_decode_rsa_secret_key() { env_logger::try_init().unwrap_or(()); @@ -509,6 +513,7 @@ Ve0k2ddxoEsSE15H4lgNHM2iuYKzIqZJOReHRCTff6QGgMYPDqDfFfL1Hc1Ntql0pwAAAA parse_public_key_base64(key).unwrap(); } + #[cfg(feature = "rsa")] #[test] fn test_nikao() { env_logger::try_init().unwrap_or(()); @@ -543,6 +548,7 @@ QaChXiDsryJZwsRnruvMRX9nedtqHrgnIsJLTXjppIhGhq5Kg4RQfOU= decode_secret_key(key, None).unwrap(); } + #[cfg(feature = "rsa")] #[test] fn test_decode_pkcs8_rsa_secret_key() { // Generated using: ssh-keygen -t rsa -b 1024 -m pkcs8 -f $file @@ -680,6 +686,7 @@ ocyR assert_eq!(original_key_bytes, encoded_key_bytes); } + #[cfg(feature = "rsa")] #[test] fn test_o01eg() { env_logger::try_init().unwrap_or(()); @@ -718,6 +725,7 @@ br8gXU8KyiY9sZVbmplRPF+ar462zcI2kt0a18mr0vbrdqp2eMjb37QDbVBJ+rPE decode_secret_key(key, Some("12345")).unwrap(); } + #[cfg(feature = "rsa")] pub const PKCS8_RSA: &str = "-----BEGIN RSA PRIVATE KEY----- MIIEpAIBAAKCAQEAwBGetHjW+3bDQpVktdemnk7JXgu1NBWUM+ysifYLDBvJ9ttX GNZSyQKA4v/dNr0FhAJ8I9BuOTjYCy1YfKylhl5D/DiSSXFPsQzERMmGgAlYvU2U @@ -747,6 +755,7 @@ xV/JrzLAwPoKk3bkqys3bUmgo6DxVC/6RmMwPQ0rmpw78kOgEej90g== -----END RSA PRIVATE KEY----- "; + #[cfg(feature = "rsa")] #[test] fn test_pkcs8() { env_logger::try_init().unwrap_or(()); @@ -754,6 +763,7 @@ xV/JrzLAwPoKk3bkqys3bUmgo6DxVC/6RmMwPQ0rmpw78kOgEej90g== decode_secret_key(PKCS8_RSA, Some("blabla")).unwrap(); } + #[cfg(feature = "rsa")] const PKCS8_ENCRYPTED: &str = "-----BEGIN ENCRYPTED PRIVATE KEY----- MIIFLTBXBgkqhkiG9w0BBQ0wSjApBgkqhkiG9w0BBQwwHAQITo1O0b8YrS0CAggA MAwGCCqGSIb3DQIJBQAwHQYJYIZIAWUDBAEqBBBtLH4T1KOfo1GGr7salhR8BIIE @@ -815,6 +825,7 @@ Cog3JMeTrb3LiPHgN6gU2P30MRp6L1j1J/MtlOAr5rux ssh_key::PublicKey::decode(&key).unwrap(); } + #[cfg(feature = "rsa")] #[test] fn test_pkcs8_encrypted() { env_logger::try_init().unwrap_or(()); @@ -877,21 +888,21 @@ Cog3JMeTrb3LiPHgN6gU2P30MRp6L1j1J/MtlOAr5rux } #[tokio::test] - #[cfg(unix)] + #[cfg(all(unix, feature = "rsa"))] async fn test_client_agent_rsa() { let key = decode_secret_key(PKCS8_ENCRYPTED, Some("blabla")).unwrap(); test_client_agent(key).await.expect("ssh-agent test failed") } #[tokio::test] - #[cfg(unix)] + #[cfg(all(unix, feature = "rsa"))] async fn test_client_agent_openssh_rsa() { let key = decode_secret_key(RSA_KEY, None).unwrap(); test_client_agent(key).await.expect("ssh-agent test failed") } #[test] - #[cfg(unix)] + #[cfg(all(unix, feature = "rsa"))] fn test_agent() { env_logger::try_init().unwrap_or(()); let dir = tempfile::tempdir().unwrap(); @@ -912,9 +923,10 @@ Cog3JMeTrb3LiPHgN6gU2P30MRp6L1j1J/MtlOAr5rux } } let agent_path_ = agent_path.clone(); + let (tx, rx) = tokio::sync::oneshot::channel(); core.spawn(async move { let mut listener = tokio::net::UnixListener::bind(&agent_path_).unwrap(); - + let _ = tx.send(()); agent::server::serve( Incoming { listener: &mut listener, @@ -923,9 +935,12 @@ Cog3JMeTrb3LiPHgN6gU2P30MRp6L1j1J/MtlOAr5rux ) .await }); + let key = decode_secret_key(PKCS8_ENCRYPTED, Some("blabla")).unwrap(); core.block_on(async move { let public = key.public_key(); + // make sure the listener created the file handle + rx.await.unwrap(); let stream = tokio::net::UnixStream::connect(&agent_path).await.unwrap(); let mut client = agent::client::AgentClient::connect(stream); client