added the missing option to specify the hash algorithm to be used wit…

…h an RSA key

russh-keys/src/agent/server.rs

@@ -16,7 +16,8 @@ use tokio::time::sleep;
 use {std, tokio};
 
 use super::{msg, Constraint};
-use crate::helpers::{sign_workaround, EncodedExt};
+use crate::helpers::{sign_workaround_encoded, EncodedExt};
+use crate::key::PrivateKeyWithHashAlg;
 use crate::Error;
 
 #[derive(Clone)]
@@ -342,8 +343,8 @@ impl<S: AsyncRead + AsyncWrite + Send + Unpin + 'static, A: Agent + Send + Sync
         writebuf.push(msg::SIGN_RESPONSE);
         let data = Bytes::decode(r)?;
 
-        let signature = sign_workaround(&key, &data)?;
-        signature.encoded()?.encode(writebuf)?;
+        sign_workaround_encoded(&PrivateKeyWithHashAlg::new(key, None)?, &data)?
+            .encode(writebuf)?;
 
         let len = writebuf.len();
         BigEndian::write_u32(writebuf, (len - 4) as u32);

russh-keys/src/helpers.rs

@@ -41,36 +41,117 @@ macro_rules! map_err {
 }
 
 pub use map_err;
-use ssh_key::PrivateKey;
 
-// TODO only needed until https://github.com/RustCrypto/SSH/pull/318 is released
-#[doc(hidden)]
-pub fn sign_workaround(
-    key: &PrivateKey,
-    data: &[u8],
-) -> Result<ssh_key::Signature, signature::Error> {
-    Ok(match key.key_data() {
-        ssh_key::private::KeypairData::Rsa(rsa_keypair) => {
-            let pk = rsa::RsaPrivateKey::from_components(
-                <rsa::BigUint as std::convert::TryFrom<_>>::try_from(&rsa_keypair.public.n)?,
-                <rsa::BigUint as std::convert::TryFrom<_>>::try_from(&rsa_keypair.public.e)?,
-                <rsa::BigUint as std::convert::TryFrom<_>>::try_from(&rsa_keypair.private.d)?,
-                vec![
-                    <rsa::BigUint as std::convert::TryFrom<_>>::try_from(&rsa_keypair.private.p)?,
-                    <rsa::BigUint as std::convert::TryFrom<_>>::try_from(&rsa_keypair.private.q)?,
-                ],
-            )?;
-            let signature = signature::Signer::try_sign(
-                &rsa::pkcs1v15::SigningKey::<sha2::Sha512>::new(pk),
-                data,
-            )?;
-            ssh_key::Signature::new(
-                ssh_key::Algorithm::Rsa {
-                    hash: Some(ssh_key::HashAlg::Sha512),
-                },
-                <rsa::pkcs1v15::Signature as signature::SignatureEncoding>::to_vec(&signature),
-            )?
+mod signature_workarounds {
+    use rsa::{Pkcs1v15Sign, RsaPrivateKey};
+    use sha1::Sha1;
+    use sha2::{Digest, Sha256, Sha512};
+    use ssh_encoding::Encode;
+    use ssh_key::{Algorithm, HashAlg};
+
+    use crate::helpers::EncodedExt;
+    use crate::key::PrivateKeyWithHashAlg;
+
+    fn sign_rsa_with_hash_alg_encoded(
+        key: &RsaPrivateKey,
+        message: &[u8],
+        hash_alg: Option<HashAlg>,
+    ) -> ssh_key::Result<Vec<u8>> {
+        let signature = key.sign(
+            match hash_alg {
+                Some(HashAlg::Sha256) => Pkcs1v15Sign::new::<sha2::Sha256>(),
+                Some(HashAlg::Sha512) => Pkcs1v15Sign::new::<sha2::Sha512>(),
+                None => Pkcs1v15Sign::new::<sha1::Sha1>(),
+                _ => unreachable!(),
+            },
+            &match hash_alg {
+                Some(HashAlg::Sha256) => Sha256::digest(message).to_vec(),
+                Some(HashAlg::Sha512) => Sha512::digest(message).to_vec(),
+                None => Sha1::digest(message).to_vec(),
+                _ => unreachable!(),
+            },
+        )?;
+
+        // due to internal stable ssh_key hijinks, it's impossible to construct a pure ssh-rsa signature in any way so we just encode it manually
+
+        let mut buf = Vec::new();
+        Algorithm::Rsa { hash: hash_alg }.encode(&mut buf)?;
+        signature.to_vec().encode(&mut buf)?;
+        dbg!(&buf);
+        Ok(buf)
+    }
+
+    // TODO only needed until https://github.com/RustCrypto/SSH/pull/318 is released
+    // and until RSA-SHA1 signatures are implemented
+    pub fn sign_workaround_encoded(
+        key: &PrivateKeyWithHashAlg,
+        data: &[u8],
+    ) -> ssh_key::Result<Vec<u8>> {
+        dbg!(&key);
+        Ok(match key.key_data() {
+            ssh_key::private::KeypairData::Rsa(rsa_keypair) => {
+                let pk = rsa::RsaPrivateKey::from_components(
+                    <rsa::BigUint as std::convert::TryFrom<_>>::try_from(&rsa_keypair.public.n)?,
+                    <rsa::BigUint as std::convert::TryFrom<_>>::try_from(&rsa_keypair.public.e)?,
+                    <rsa::BigUint as std::convert::TryFrom<_>>::try_from(&rsa_keypair.private.d)?,
+                    vec![
+                        <rsa::BigUint as std::convert::TryFrom<_>>::try_from(
+                            &rsa_keypair.private.p,
+                        )?,
+                        <rsa::BigUint as std::convert::TryFrom<_>>::try_from(
+                            &rsa_keypair.private.q,
+                        )?,
+                    ],
+                )?;
+                let Algorithm::Rsa { hash } = key.algorithm() else {
+                    unreachable!();
+                };
+                sign_rsa_with_hash_alg_encoded(&pk, data, hash)?
+            }
+            keypair => signature::Signer::try_sign(keypair, data)?.encoded()?,
+        })
+    }
+}
+
+mod algorithm {
+    use ssh_key::{Algorithm, HashAlg};
+
+    pub trait AlgorithmExt {
+        fn hash_alg(&self) -> Option<HashAlg>;
+        fn with_hash_alg(&self, hash_alg: Option<HashAlg>) -> Self;
+        fn new_certificate_ext(algo: &str) -> Result<Self, ssh_key::Error>
+        where
+            Self: Sized;
+    }
+
+    impl AlgorithmExt for Algorithm {
+        fn hash_alg(&self) -> Option<HashAlg> {
+            match self {
+                Algorithm::Rsa { hash } => *hash,
+                _ => None,
+            }
+        }
+
+        fn with_hash_alg(&self, hash_alg: Option<HashAlg>) -> Self {
+            match self {
+                Algorithm::Rsa { .. } => Algorithm::Rsa { hash: hash_alg },
+                x => x.clone(),
+            }
         }
-        keypair => signature::Signer::try_sign(keypair, data)?,
-    })
+
+        fn new_certificate_ext(algo: &str) -> Result<Self, ssh_key::Error> {
+            match algo {
+                "rsa-sha2-256-cert-v01@openssh.com" => Ok(Algorithm::Rsa {
+                    hash: Some(HashAlg::Sha256),
+                }),
+                "rsa-sha2-512-cert-v01@openssh.com" => Ok(Algorithm::Rsa {
+                    hash: Some(HashAlg::Sha512),
+                }),
+                x => Algorithm::new_certificate(x),
+            }
+        }
+    }
 }
+
+#[doc(hidden)]
+pub use {algorithm::AlgorithmExt, signature_workarounds::sign_workaround_encoded};

russh-keys/src/key.rs

@@ -46,6 +46,54 @@ pub fn safe_rng() -> impl rand::CryptoRng + rand::RngCore {
     rand::thread_rng()
 }
 
+mod private_key_with_hash_alg {
+    use std::ops::Deref;
+    use std::sync::Arc;
+
+    use ssh_key::Algorithm;
+
+    use crate::helpers::AlgorithmExt;
+
+    /// Helper structure to correlate a key and (in case of RSA) a hash algorithm.
+    /// Only used for authentication, not key storage as RSA keys do not inherently
+    /// have a hash algorithm associated with them.
+    #[derive(Clone, Debug)]
+    pub struct PrivateKeyWithHashAlg {
+        key: Arc<crate::PrivateKey>,
+        hash_alg: Option<crate::HashAlg>,
+    }
+
+    impl PrivateKeyWithHashAlg {
+        pub fn new(
+            key: Arc<crate::PrivateKey>,
+            hash_alg: Option<crate::HashAlg>,
+        ) -> Result<Self, crate::Error> {
+            if hash_alg.is_some() && !key.algorithm().is_rsa() {
+                return Err(crate::Error::InvalidParameters);
+            }
+            Ok(Self { key, hash_alg })
+        }
+
+        pub fn algorithm(&self) -> Algorithm {
+            self.key.algorithm().with_hash_alg(self.hash_alg)
+        }
+
+        pub fn hash_alg(&self) -> Option<crate::HashAlg> {
+            self.hash_alg
+        }
+    }
+
+    impl Deref for PrivateKeyWithHashAlg {
+        type Target = crate::PrivateKey;
+
+        fn deref(&self) -> &Self::Target {
+            &self.key
+        }
+    }
+}
+
+pub use private_key_with_hash_alg::PrivateKeyWithHashAlg;
+
 pub const ALL_KEY_TYPES: &[Algorithm] = &[
     Algorithm::Dsa,
     Algorithm::Ecdsa {

russh/examples/client_exec_interactive.rs

@@ -11,6 +11,7 @@ use std::time::Duration;
 use anyhow::Result;
 use async_trait::async_trait;
 use clap::Parser;
+use key::PrivateKeyWithHashAlg;
 use log::info;
 use russh::keys::*;
 use russh::*;
@@ -112,7 +113,7 @@ impl Session {
         // use publickey authentication, with or without certificate
         if openssh_cert.is_none() {
             let auth_res = session
-                .authenticate_publickey(user, Arc::new(key_pair))
+                .authenticate_publickey(user, PrivateKeyWithHashAlg::new(Arc::new(key_pair), None)?)
                 .await?;
 
             if !auth_res {

russh/examples/client_exec_simple.rs

@@ -9,6 +9,7 @@ use std::time::Duration;
 use anyhow::Result;
 use async_trait::async_trait;
 use clap::Parser;
+use key::PrivateKeyWithHashAlg;
 use log::info;
 use russh::keys::*;
 use russh::*;
@@ -91,7 +92,7 @@ impl Session {
 
         let mut session = client::connect(config, addrs, sh).await?;
         let auth_res = session
-            .authenticate_publickey(user, Arc::new(key_pair))
+            .authenticate_publickey(user, PrivateKeyWithHashAlg::new(Arc::new(key_pair), None)?)
             .await?;
 
         if !auth_res {

russh/src/auth.rs

@@ -18,6 +18,7 @@ use std::sync::Arc;
 use async_trait::async_trait;
 use bitflags::bitflags;
 use russh_keys::helpers::NameList;
+use russh_keys::key::PrivateKeyWithHashAlg;
 use ssh_key::{Certificate, PrivateKey};
 use thiserror::Error;
 use tokio::io::{AsyncRead, AsyncWrite};
@@ -86,7 +87,7 @@ pub enum Method {
         password: String,
     },
     PublicKey {
-        key: Arc<PrivateKey>,
+        key: PrivateKeyWithHashAlg,
     },
     OpenSshCertificate {
         key: Arc<PrivateKey>,

russh/src/cert.rs

@@ -1,48 +1,43 @@
 use core::str;
 
+use russh_keys::helpers::AlgorithmExt;
+use russh_keys::key::PrivateKeyWithHashAlg;
 use ssh_encoding::Decode;
 use ssh_key::public::KeyData;
 use ssh_key::{Algorithm, Certificate, HashAlg, PublicKey};
 
 #[derive(Debug)]
 pub(crate) enum PublicKeyOrCertificate {
-    PublicKey(PublicKey),
+    PublicKey {
+        key: PublicKey,
+        hash_alg: Option<HashAlg>,
+    },
     Certificate(Certificate),
 }
 
+impl From<&PrivateKeyWithHashAlg> for PublicKeyOrCertificate {
+    fn from(key: &PrivateKeyWithHashAlg) -> Self {
+        PublicKeyOrCertificate::PublicKey {
+            key: key.public_key().clone(),
+            hash_alg: key.hash_alg(),
+        }
+    }
+}
+
 impl PublicKeyOrCertificate {
     pub fn decode(pubkey_algo: &str, buf: &[u8]) -> Result<Self, ssh_key::Error> {
         let mut reader = buf;
         match Algorithm::new_certificate_ext(pubkey_algo) {
             Ok(Algorithm::Other(_)) | Err(ssh_key::Error::Encoding(_)) => {
                 // Did not match a known cert algorithm
-                Ok(PublicKeyOrCertificate::PublicKey(
-                    KeyData::decode(&mut reader)?.into(),
-                ))
+                Ok(PublicKeyOrCertificate::PublicKey {
+                    key: KeyData::decode(&mut reader)?.into(),
+                    hash_alg: Algorithm::new(pubkey_algo)?.hash_alg(),
+                })
             }
             _ => Ok(PublicKeyOrCertificate::Certificate(Certificate::decode(
                 &mut reader,
             )?)),
         }
     }
 }
-
-trait AlgorithmExt {
-    fn new_certificate_ext(algo: &str) -> Result<Self, ssh_key::Error>
-    where
-        Self: Sized;
-}
-
-impl AlgorithmExt for Algorithm {
-    fn new_certificate_ext(algo: &str) -> Result<Self, ssh_key::Error> {
-        match algo {
-            "rsa-sha2-256-cert-v01@openssh.com" => Ok(Algorithm::Rsa {
-                hash: Some(HashAlg::Sha256),
-            }),
-            "rsa-sha2-512-cert-v01@openssh.com" => Ok(Algorithm::Rsa {
-                hash: Some(HashAlg::Sha512),
-            }),
-            x => Algorithm::new_certificate(x),
-        }
-    }
-}