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](RustCrypto/RSA#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](GitoxideLabs/gitoxide#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.

Author: plaflamme

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",

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",
 ] }

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<Vec<u8>> {
     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()?

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<Private
                 started = true;
                 format = Some(Format::Openssh);
             } else if l == "-----BEGIN RSA PRIVATE KEY-----" {
-                started = true;
-                format = Some(Format::Rsa);
+                #[cfg(feature = "rsa")]
+                {
+                    started = true;
+                    format = Some(Format::Rsa);
+                }
+                #[cfg(not(feature = "rsa"))]
+                {
+                    return Err(Error::UnsupportedKeyType {
+                        key_type_string: "RSA".to_string(),
+                        key_type_raw: vec![],
+                    });
+                }
             } else if l == "-----BEGIN ENCRYPTED PRIVATE KEY-----" {
                 started = true;
                 format = Some(Format::Pkcs8Encrypted);
@@ -92,6 +100,7 @@ pub fn decode_secret_key(secret: &str, password: Option<&str>) -> Result<Private
     let secret = BASE64_MIME.decode(secret.as_bytes())?;
     match format {
         Some(Format::Openssh) => 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<W: Write>(
     Ok(())
 }
 
-fn decode_rsa_pkcs1_der(secret: &[u8]) -> Result<RsaKeypair, Error> {
+#[cfg(feature = "rsa")]
+fn decode_rsa_pkcs1_der(secret: &[u8]) -> Result<ssh_key::private::RsaKeypair, Error> {
+    use pkcs1::DecodeRsaPrivateKey;
+    use std::convert::TryInto;
+
     Ok(rsa::RsaPrivateKey::from_pkcs1_der(secret)?.try_into()?)
 }

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)
     }

russh/src/keys/format/pkcs8.rs

@@ -49,6 +49,7 @@ fn pkcs8_pki_into_keypair_data(pki: PrivateKeyInfo<'_>) -> Result<KeypairData, E
                 private: pk,
             }))
         }
+        #[cfg(feature = "rsa")]
         pkcs1::ALGORITHM_OID => {
             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<Vec<u8>, 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()?

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,

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,13 +755,15 @@ xV/JrzLAwPoKk3bkqys3bUmgo6DxVC/6RmMwPQ0rmpw78kOgEej90g==
 -----END RSA PRIVATE KEY-----
 ";
 
+    #[cfg(feature = "rsa")]
     #[test]
     fn test_pkcs8() {
         env_logger::try_init().unwrap_or(());
         println!("test");
         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