added best_supported_rsa_hash

russh/examples/client_exec_interactive.rs

@@ -112,7 +112,13 @@ 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),
+                        session.best_supported_rsa_hash().await?.flatten(),
+                    ),
+                )
                 .await?;
 
             if !auth_res.success() {

russh/examples/client_exec_simple.rs

@@ -99,7 +99,13 @@ 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),
+                    session.best_supported_rsa_hash().await.unwrap().flatten(),
+                ),
+            )
             .await?;
 
         if !auth_res.success() {

russh/src/auth.rs

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

russh/src/client/encrypted.rs

@@ -16,20 +16,18 @@ 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, Reader};
-use ssh_key::{Algorithm, HashAlg, PrivateKey};
+use ssh_key::Algorithm;
 
 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::{
@@ -291,17 +289,15 @@ impl Session {
     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<_>>(),
-            );
-        }
+        self.server_sig_algs = Some(
+            algs.0
+                .iter()
+                .filter_map(|x| Algorithm::from_str(x).ok())
+                .inspect(|x| {
+                    debug!("  * {x:?}");
+                })
+                .collect::<Vec<_>>(),
+        );
         Ok(())
     }
 
@@ -848,39 +844,6 @@ 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,
@@ -905,12 +868,7 @@ impl Encrypted {
                     password.encode(&mut self.write)?;
                     true
                 }
-                auth::Method::PublicKey {
-                    ref key,
-                    ref hash_alg,
-                } => {
-                    let key = self.pick_hash_alg_for_key(key.clone(), *hash_alg)?;
-
+                auth::Method::PublicKey { ref key } => {
                     user.encode(&mut self.write)?;
                     "ssh-connection".encode(&mut self.write)?;
                     "publickey".encode(&mut self.write)?;
@@ -994,13 +952,12 @@ impl Encrypted {
         buffer: &mut CryptoVec,
     ) -> Result<(), crate::Error> {
         match method {
-            auth::Method::PublicKey { key, hash_alg } => {
-                let key = self.pick_hash_alg_for_key(key.clone(), *hash_alg)?;
+            auth::Method::PublicKey { key } => {
                 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

@@ -47,7 +47,7 @@ use kex::ClientKex;
 use log::{debug, error, trace};
 use russh_util::time::Instant;
 use ssh_encoding::Decode;
-use ssh_key::{Certificate, HashAlg, PrivateKey, PublicKey};
+use ssh_key::{Algorithm, Certificate, HashAlg, PrivateKey, PublicKey};
 use tokio::io::{AsyncRead, AsyncWrite, AsyncWriteExt, ReadHalf, WriteHalf};
 use tokio::pin;
 use tokio::sync::mpsc::{
@@ -59,6 +59,7 @@ 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::PrivateKeyWithHashAlg;
 use crate::msg::{is_kex_msg, validate_server_msg_strict_kex};
 use crate::session::{CommonSession, EncryptedState, GlobalRequestResponse, NewKeys};
 use crate::ssh_read::SshRead;
@@ -90,6 +91,7 @@ pub struct Session {
     inbound_channel_sender: Sender<Msg>,
     inbound_channel_receiver: Receiver<Msg>,
     open_global_requests: VecDeque<GlobalRequestResponse>,
+    server_sig_algs: Option<Vec<Algorithm>>,
 }
 
 impl Drop for Session {
@@ -180,6 +182,9 @@ pub enum Msg {
     },
     Channel(ChannelId, ChannelMsg),
     Rekey,
+    GetServerSigAlgs {
+        reply_channel: oneshot::Sender<Option<Vec<Algorithm>>>,
+    },
 }
 
 impl From<(ChannelId, ChannelMsg)> for Msg {
@@ -359,44 +364,22 @@ 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.
+    ///
+    /// For RSA keys, you'll need to decide on which hash algorithm to use.
+    /// This is the difference between what is also known as
+    /// `ssh-rsa`, `rsa-sha2-256`, and `rsa-sha2-512` "keys" in OpenSSH.
+    /// You can use [Handle::best_supported_rsa_hash] to automatically
+    /// figure out the best hash algorithm for RSA keys.
     pub async fn authenticate_publickey<U: Into<String>>(
         &mut self,
         user: U,
-        key: Arc<PrivateKey>,
+        key: PrivateKeyWithHashAlg,
     ) -> 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,
-                    hash_alg: Some(hash_alg),
-                },
+                method: auth::Method::PublicKey { key },
             })
             .await
             .map_err(|_| crate::Error::SendError)?;
@@ -507,6 +490,39 @@ impl<H: Handler> Handle<H> {
         }
     }
 
+    /// Returns the best RSA hash algorithm supported by the server,
+    /// as indicated by the `server-sig-algs` extension.
+    /// If the server does not support the extension,
+    /// `None` is returned. In this case you may still attempt an authentication
+    /// with `rsa-sha2-256` or `rsa-sha2-512` and hope for the best.
+    /// If the server supports the extension, but does not support `rsa-sha2-*`,
+    /// `Some(None)` is returned.
+    pub async fn best_supported_rsa_hash(&self) -> Result<Option<Option<HashAlg>>, Error> {
+        let (sender, receiver) = oneshot::channel();
+
+        self.sender
+            .send(Msg::GetServerSigAlgs {
+                reply_channel: sender,
+            })
+            .await
+            .map_err(|_| crate::Error::SendError)?;
+
+        if let Some(ssa) = receiver.await.map_err(|_| Error::Inconsistent)? {
+            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 Ok(Some(alg));
+                }
+            }
+        }
+
+        Ok(None)
+    }
+
     /// Request a session channel (the most basic type of
     /// channel). This function returns `Some(..)` immediately if the
     /// connection is authenticated, but the channel only becomes
@@ -896,6 +912,7 @@ impl Session {
             pending_reads: Vec::new(),
             pending_len: 0,
             open_global_requests: VecDeque::new(),
+            server_sig_algs: None,
         }
     }
 
@@ -1255,6 +1272,9 @@ impl Session {
             }
             Msg::Channel(id, ChannelMsg::Close) => self.close(id)?,
             Msg::Rekey => self.initiate_rekey()?,
+            Msg::GetServerSigAlgs { reply_channel } => {
+                let _ = reply_channel.send(self.server_sig_algs.clone());
+            }
             msg => {
                 // should be unreachable, since the receiver only gets
                 // messages from methods implemented within russh

russh/src/keys/agent/server.rs

@@ -341,7 +341,7 @@ impl<S: AsyncRead + AsyncWrite + Send + Unpin + 'static, A: Agent + Send + Sync
         writebuf.push(msg::SIGN_RESPONSE);
         let data = Bytes::decode(r)?;
 
-        sign_with_hash_alg(&PrivateKeyWithHashAlg::new(key, None)?, &data)?.encode(writebuf)?;
+        sign_with_hash_alg(&PrivateKeyWithHashAlg::new(key, None), &data)?.encode(writebuf)?;
 
         let len = writebuf.len();
         BigEndian::write_u32(writebuf, (len - 4) as u32);

russh/src/keys/key.rs

@@ -65,16 +65,16 @@ mod private_key_with_hash_alg {
     impl PrivateKeyWithHashAlg {
         /// Direct constructor.
         ///
-        /// Will fail if you specify a `hash_alg` for a key type other than RSA.
         /// For RSA, passing `None` is mapped to the legacy `sha-rsa` (SHA-1).
+        /// For other keys, `hash_alg` is ignored.
         pub fn new(
             key: Arc<crate::keys::PrivateKey>,
-            hash_alg: Option<crate::keys::HashAlg>,
-        ) -> Result<Self, crate::keys::Error> {
-            if hash_alg.is_some() && !key.algorithm().is_rsa() {
-                return Err(crate::keys::Error::InvalidParameters);
+            mut hash_alg: Option<crate::keys::HashAlg>,
+        ) -> Self {
+            if !key.algorithm().is_rsa() {
+                hash_alg = None;
             }
-            Ok(Self { key, hash_alg })
+            Self { key, hash_alg }
         }
 
         pub fn algorithm(&self) -> Algorithm {

russh/src/server/kex.rs

@@ -272,8 +272,7 @@ impl ServerKex {
                 // Hash signature
                 debug!("signing with key {:?}", key);
                 let signature = sign_with_hash_alg(
-                    &PrivateKeyWithHashAlg::new(Arc::new(key.clone()), signature_hash_alg)
-                        .map_err(Into::into)?,
+                    &PrivateKeyWithHashAlg::new(Arc::new(key.clone()), signature_hash_alg),
                     &hash,
                 )
                 .map_err(Into::into)?;

russh/src/session.rs

@@ -21,7 +21,6 @@ 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;
@@ -53,7 +52,6 @@ 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> {
@@ -153,7 +151,6 @@ 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