support automatic RSA key hash detection using server-sig-algs extens…

…ion (Eugeny#452)

russh/examples/client_exec_interactive.rs

@@ -11,7 +11,6 @@ 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::*;
@@ -113,7 +112,7 @@ impl Session {
         // use publickey authentication, with or without certificate
         if openssh_cert.is_none() {
             let auth_res = session
-                .authenticate_publickey(user, PrivateKeyWithHashAlg::new(Arc::new(key_pair), None)?)
+                .authenticate_publickey(user, Arc::new(key_pair))
                 .await?;
 
             if !auth_res.success() {

russh/examples/client_exec_simple.rs

@@ -10,7 +10,6 @@ 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::*;
@@ -86,7 +85,10 @@ impl Session {
         let config = client::Config {
             inactivity_timeout: Some(Duration::from_secs(5)),
             preferred: Preferred {
-                kex: Cow::Owned(vec![russh::kex::DH_GEX_SHA256]),
+                kex: Cow::Owned(vec![
+                    russh::kex::CURVE25519_PRE_RFC_8731,
+                    russh::kex::EXTENSION_SUPPORT_AS_CLIENT,
+                ]),
                 ..Default::default()
             },
             ..<_>::default()
@@ -97,7 +99,7 @@ impl Session {
 
         let mut session = client::connect(config, addrs, sh).await?;
         let auth_res = session
-            .authenticate_publickey(user, PrivateKeyWithHashAlg::new(Arc::new(key_pair), None)?)
+            .authenticate_publickey(user, Arc::new(key_pair))
             .await?;
 
         if !auth_res.success() {

russh/src/auth.rs

@@ -23,7 +23,6 @@ use thiserror::Error;
 use tokio::io::{AsyncRead, AsyncWrite};
 
 use crate::helpers::NameList;
-use crate::keys::key::PrivateKeyWithHashAlg;
 use crate::CryptoVec;
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
@@ -192,7 +191,10 @@ pub enum Method {
         password: String,
     },
     PublicKey {
-        key: PrivateKeyWithHashAlg,
+        key: Arc<PrivateKey>,
+        /// None = based on server-sig-algs
+        /// Some(None) = SHA1
+        hash_alg: Option<Option<HashAlg>>,
     },
     OpenSshCertificate {
         key: Arc<PrivateKey>,

russh/src/client/encrypted.rs

@@ -15,21 +15,25 @@
 use std::cell::RefCell;
 use std::convert::TryInto;
 use std::ops::Deref;
+use std::str::FromStr;
+use std::sync::Arc;
 
 use bytes::Bytes;
 use log::{debug, error, info, trace, warn};
-use ssh_encoding::{Decode, Encode};
+use ssh_encoding::{Decode, Encode, Reader};
+use ssh_key::{Algorithm, HashAlg, PrivateKey};
 
 use super::IncomingSshPacket;
 use crate::auth::AuthRequest;
 use crate::cert::PublicKeyOrCertificate;
 use crate::client::{Handler, Msg, Prompt, Reply, Session};
 use crate::helpers::{map_err, sign_with_hash_alg, AlgorithmExt, EncodedExt, NameList};
 use crate::keys::key::parse_public_key;
+use crate::keys::PrivateKeyWithHashAlg;
 use crate::parsing::{ChannelOpenConfirmation, ChannelType, OpenChannelMessage};
 use crate::session::{Encrypted, EncryptedState, GlobalRequestResponse};
 use crate::{
-    auth, msg, Channel, ChannelId, ChannelMsg, ChannelOpenFailure, ChannelParams, CryptoVec,
+    auth, msg, Channel, ChannelId, ChannelMsg, ChannelOpenFailure, ChannelParams, CryptoVec, Error,
     MethodSet, Sig,
 };
 
@@ -89,8 +93,8 @@ impl Session {
                                 }
                             }
                         }
-                        Some((&msg::EXT_INFO, r)) => {
-                            return self.handle_ext_info(client, r);
+                        Some((&msg::EXT_INFO, mut r)) => {
+                            return self.handle_ext_info(&mut r).map_err(Into::into);
                         }
                         other => {
                             debug!("unknown message: {other:?}");
@@ -249,8 +253,8 @@ impl Session {
                                 _ => {}
                             }
                         }
-                        Some((&msg::EXT_INFO, r)) => {
-                            return self.handle_ext_info(client, r);
+                        Some((&msg::EXT_INFO, mut r)) => {
+                            return self.handle_ext_info(&mut r).map_err(Into::into);
                         }
                         other => {
                             debug!("unknown message: {other:?}");
@@ -269,8 +273,35 @@ impl Session {
         }
     }
 
-    fn handle_ext_info<H: Handler>(&mut self, _client: &mut H, r: &[u8]) -> Result<(), H::Error> {
-        debug!("Received EXT_INFO: {:?}", r);
+    fn handle_ext_info(&mut self, r: &mut impl Reader) -> Result<(), Error> {
+        let n_extensions = u32::decode(r)? as usize;
+        debug!("Received EXT_INFO, {n_extensions:?} extensions");
+        for _ in 0..n_extensions {
+            let name = String::decode(r)?;
+            if name == "server-sig-algs" {
+                self.handle_server_sig_algs_ext(r)?;
+            } else {
+                let data = Vec::<u8>::decode(r)?;
+                debug!("* {name:?} (unknown, data: {data:?})");
+            }
+        }
+        Ok(())
+    }
+
+    fn handle_server_sig_algs_ext(&mut self, r: &mut impl Reader) -> Result<(), Error> {
+        let algs = NameList::decode(r)?;
+        debug!("* server-sig-algs");
+        if let Some(ref mut enc) = self.common.encrypted {
+            enc.server_sig_algs = Some(
+                algs.0
+                    .iter()
+                    .filter_map(|x| Algorithm::from_str(x).ok())
+                    .inspect(|x| {
+                        debug!("  * {x:?}");
+                    })
+                    .collect::<Vec<_>>(),
+            );
+        }
         Ok(())
     }
 
@@ -817,6 +848,39 @@ impl Session {
 }
 
 impl Encrypted {
+    fn pick_hash_alg_for_key(
+        &self,
+        key: Arc<PrivateKey>,
+        hash_alg_choice: Option<Option<HashAlg>>,
+    ) -> Result<PrivateKeyWithHashAlg, Error> {
+        Ok(match hash_alg_choice {
+            Some(hash_alg) => PrivateKeyWithHashAlg::new(key.clone(), hash_alg)?,
+            None => {
+                if key.algorithm().is_rsa() {
+                    PrivateKeyWithHashAlg::new(key.clone(), self.best_key_hash_alg())?
+                } else {
+                    PrivateKeyWithHashAlg::new(key.clone(), None)?
+                }
+            }
+        })
+    }
+
+    fn best_key_hash_alg(&self) -> Option<HashAlg> {
+        if let Some(ref ssa) = self.server_sig_algs {
+            let possible_algs = [
+                Some(ssh_key::HashAlg::Sha512),
+                Some(ssh_key::HashAlg::Sha256),
+                None,
+            ];
+            for alg in possible_algs.into_iter() {
+                if ssa.contains(&Algorithm::Rsa { hash: alg }) {
+                    return alg;
+                }
+            }
+        }
+        None
+    }
+
     fn write_auth_request(
         &mut self,
         user: &str,
@@ -841,7 +905,12 @@ impl Encrypted {
                     password.encode(&mut self.write)?;
                     true
                 }
-                auth::Method::PublicKey { ref key } => {
+                auth::Method::PublicKey {
+                    ref key,
+                    ref hash_alg,
+                } => {
+                    let key = self.pick_hash_alg_for_key(key.clone(), *hash_alg)?;
+
                     user.encode(&mut self.write)?;
                     "ssh-connection".encode(&mut self.write)?;
                     "publickey".encode(&mut self.write)?;
@@ -925,12 +994,13 @@ impl Encrypted {
         buffer: &mut CryptoVec,
     ) -> Result<(), crate::Error> {
         match method {
-            auth::Method::PublicKey { ref key } => {
+            auth::Method::PublicKey { key, hash_alg } => {
+                let key = self.pick_hash_alg_for_key(key.clone(), *hash_alg)?;
                 let i0 =
-                    self.client_make_to_sign(user, &PublicKeyOrCertificate::from(key), buffer)?;
+                    self.client_make_to_sign(user, &PublicKeyOrCertificate::from(&key), buffer)?;
 
                 // Extend with self-signature.
-                sign_with_hash_alg(key, buffer)?.encode(&mut *buffer)?;
+                sign_with_hash_alg(&key, buffer)?.encode(&mut *buffer)?;
 
                 push_packet!(self.write, {
                     #[allow(clippy::indexing_slicing)] // length checked

russh/src/client/mod.rs

@@ -59,7 +59,6 @@ pub use crate::auth::AuthResult;
 use crate::channels::{Channel, ChannelMsg, ChannelRef, WindowSizeRef};
 use crate::cipher::{self, clear, OpeningKey};
 use crate::kex::{KexCause, KexProgress, SessionKexState};
-use crate::keys::key::PrivateKeyWithHashAlg;
 use crate::msg::{is_kex_msg, validate_server_msg_strict_kex};
 use crate::session::{CommonSession, EncryptedState, GlobalRequestResponse, NewKeys};
 use crate::ssh_read::SshRead;
@@ -360,16 +359,44 @@ impl<H: Handler> Handle<H> {
     }
 
     /// Perform public key-based SSH authentication.
+    /// This method will automatically select the best hash function
+    /// if the server supports the `server-sig-algs` protocol extension
+    /// and will fall back to SHA-1 otherwise.
     pub async fn authenticate_publickey<U: Into<String>>(
         &mut self,
         user: U,
-        key: PrivateKeyWithHashAlg,
+        key: Arc<PrivateKey>,
+    ) -> Result<AuthResult, crate::Error> {
+        let user = user.into();
+        self.sender
+            .send(Msg::Authenticate {
+                user,
+                method: auth::Method::PublicKey {
+                    key,
+                    hash_alg: None,
+                },
+            })
+            .await
+            .map_err(|_| crate::Error::SendError)?;
+        self.wait_recv_reply().await
+    }
+
+    /// Perform public key-based SSH authentication
+    /// with an explicit hash algorithm selection (for RSA keys).
+    pub async fn authenticate_publickey_with_hash<U: Into<String>>(
+        &mut self,
+        user: U,
+        key: Arc<PrivateKey>,
+        hash_alg: Option<HashAlg>,
     ) -> Result<AuthResult, crate::Error> {
         let user = user.into();
         self.sender
             .send(Msg::Authenticate {
                 user,
-                method: auth::Method::PublicKey { key },
+                method: auth::Method::PublicKey {
+                    key,
+                    hash_alg: Some(hash_alg),
+                },
             })
             .await
             .map_err(|_| crate::Error::SendError)?;

russh/src/lib.rs

@@ -178,7 +178,7 @@ pub enum Error {
     UnknownAlgo,
 
     /// No common algorithm found during key exchange.
-    #[error("No common algorithm")]
+    #[error("No common {kind:?} algorithm - ours: {ours:?}, theirs: {theirs:?}")]
     NoCommonAlgo {
         kind: AlgorithmKind,
         ours: Vec<String>,

russh/src/session.rs

@@ -21,6 +21,7 @@ use std::num::Wrapping;
 use byteorder::{BigEndian, ByteOrder};
 use log::{debug, trace};
 use ssh_encoding::Encode;
+use ssh_key::Algorithm;
 use tokio::sync::oneshot;
 
 use crate::cipher::OpeningKey;
@@ -52,6 +53,7 @@ pub(crate) struct Encrypted {
     pub client_compression: crate::compression::Compression,
     pub decompress: crate::compression::Decompress,
     pub rekey_wanted: bool,
+    pub server_sig_algs: Option<Vec<Algorithm>>,
 }
 
 pub(crate) struct CommonSession<Config> {
@@ -151,6 +153,7 @@ impl<C> CommonSession<C> {
             client_compression: newkeys.names.client_compression,
             decompress: crate::compression::Decompress::None,
             rekey_wanted: false,
+            server_sig_algs: None,
         });
         self.remote_to_local = newkeys.cipher.remote_to_local;
         self.packet_writer

russh/src/tests.rs

@@ -15,7 +15,6 @@ mod compress {
 
     use super::server::{Server as _, Session};
     use super::*;
-    use crate::keys::key::PrivateKeyWithHashAlg;
     use crate::server::Msg;
 
     #[tokio::test]
@@ -53,7 +52,7 @@ mod compress {
         let authenticated = session
             .authenticate_publickey(
                 std::env::var("USER").unwrap_or("user".to_owned()),
-                PrivateKeyWithHashAlg::new(Arc::new(client_key), None).unwrap(),
+                Arc::new(client_key),
             )
             .await
             .unwrap()
@@ -146,7 +145,6 @@ mod channels {
     use tokio::io::{AsyncReadExt, AsyncWriteExt};
 
     use super::*;
-    use crate::keys::key::PrivateKeyWithHashAlg;
     use crate::CryptoVec;
 
     async fn test_session<RC, RS, CH, SH, F1, F2>(
@@ -197,7 +195,7 @@ mod channels {
             let authenticated = session
                 .authenticate_publickey(
                     std::env::var("USER").unwrap_or("user".to_owned()),
-                    PrivateKeyWithHashAlg::new(Arc::new(client_key), None).unwrap(),
+                    Arc::new(client_key),
                 )
                 .await
                 .unwrap();

russh/tests/test_backpressure.rs

@@ -4,7 +4,6 @@ use std::sync::Arc;
 use futures::FutureExt;
 use rand::RngCore;
 use rand_core::OsRng;
-use russh::keys::key::PrivateKeyWithHashAlg;
 use russh::server::{self, Auth, Msg, Server as _, Session};
 use russh::{client, Channel, ChannelMsg};
 use ssh_key::PrivateKey;
@@ -41,7 +40,7 @@ async fn stream(addr: SocketAddr, data: &[u8], tx: watch::Sender<()>) -> Result<
 
     let mut session = russh::client::connect(config, addr, Client).await?;
     let channel = match session
-        .authenticate_publickey("user", PrivateKeyWithHashAlg::new(key, None).unwrap())
+        .authenticate_publickey("user", key)
         .await
         .map(|x| x.success())
     {

russh/tests/test_data_stream.rs

@@ -3,7 +3,6 @@ use std::sync::Arc;
 
 use rand::RngCore;
 use rand_core::OsRng;
-use russh::keys::key::PrivateKeyWithHashAlg;
 use russh::server::{self, Auth, Msg, Server as _, Session};
 use russh::{client, Channel};
 use ssh_key::PrivateKey;
@@ -36,7 +35,7 @@ async fn stream(addr: SocketAddr, data: &[u8]) -> Result<(), anyhow::Error> {
 
     let mut session = russh::client::connect(config, addr, Client).await?;
     let mut channel = match session
-        .authenticate_publickey("user", PrivateKeyWithHashAlg::new(key, None)?)
+        .authenticate_publickey("user", key)
         .await
         .map(|x| x.success())
     {