fixup

Author: Eugeny

russh/examples/client_exec_interactive.rs

@@ -69,7 +69,7 @@ impl client::Handler for Client {
 
     async fn check_server_key(
         &mut self,
-        _server_public_key: &ssh_key::PublicKey,
+        _server_public_key: &PublicKeyOrCertificate,
     ) -> Result<bool, Self::Error> {
         Ok(true)
     }

russh/examples/client_exec_simple.rs

@@ -63,7 +63,7 @@ impl client::Handler for Client {
 
     async fn check_server_key(
         &mut self,
-        _server_public_key: &ssh_key::PublicKey,
+        _server_public_key: &PublicKeyOrCertificate,
     ) -> Result<bool, Self::Error> {
         Ok(true)
     }

russh/examples/client_open_direct_tcpip.rs

@@ -62,7 +62,7 @@ impl client::Handler for Client {
 
     async fn check_server_key(
         &mut self,
-        _server_public_key: &ssh_key::PublicKey,
+        _server_public_key: &PublicKeyOrCertificate,
     ) -> Result<bool, Self::Error> {
         Ok(true)
     }

russh/examples/sftp_client.rs

@@ -14,7 +14,7 @@ impl client::Handler for Client {
 
     async fn check_server_key(
         &mut self,
-        server_public_key: &ssh_key::PublicKey,
+        server_public_key: &PublicKeyOrCertificate,
     ) -> Result<bool, Self::Error> {
         info!("check_server_key: {server_public_key:?}");
         Ok(true)

russh/src/cert.rs

@@ -9,7 +9,7 @@ use crate::keys::key::PrivateKeyWithHashAlg;
 
 #[derive(Debug)]
 #[allow(clippy::large_enum_variant)]
-pub(crate) enum PublicKeyOrCertificate {
+pub enum PublicKeyOrCertificate {
     PublicKey {
         key: PublicKey,
         hash_alg: Option<HashAlg>,
@@ -26,9 +26,24 @@ impl From<&PrivateKeyWithHashAlg> for PublicKeyOrCertificate {
     }
 }
 
+impl From<Certificate> for PublicKeyOrCertificate {
+    fn from(cert: Certificate) -> Self {
+        PublicKeyOrCertificate::Certificate(cert)
+    }
+}
+
+impl From<PublicKey> for PublicKeyOrCertificate {
+    fn from(key: PublicKey) -> Self {
+        PublicKeyOrCertificate::PublicKey {
+            key,
+            hash_alg: None,
+        }
+    }
+}
+
 impl PublicKeyOrCertificate {
     #[cfg(not(target_arch = "wasm32"))]
-    pub fn decode(pubkey_algo: &str, buf: &[u8]) -> Result<Self, ssh_key::Error> {
+    pub(crate) 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(_)) => {
@@ -43,4 +58,20 @@ impl PublicKeyOrCertificate {
             )?)),
         }
     }
+
+    pub fn public_key(&self) -> PublicKey {
+        match self {
+            PublicKeyOrCertificate::PublicKey { key, .. } => key.clone(),
+            PublicKeyOrCertificate::Certificate(cert) => {
+                PublicKey::new(cert.public_key().clone(), "")
+            }
+        }
+    }
+
+    pub fn certificate(&self) -> Option<&Certificate> {
+        match self {
+            PublicKeyOrCertificate::PublicKey { .. } => None,
+            PublicKeyOrCertificate::Certificate(cert) => Some(cert),
+        }
+    }
 }

russh/src/client/kex.rs

@@ -272,7 +272,11 @@ impl ClientKex {
                 // looks like a bad signature and is computed entirely locally,
                 // so there is nothing on the wire to compare against.
                 let server_host_key_blob = Bytes::decode(r)?;
-                let server_host_certificate = Certificate::from_bytes(&server_host_key_blob).ok();
+                let server_host_certificate = if names.host_key_is_certificate {
+                    Some(Certificate::from_bytes(&server_host_key_blob)?)
+                } else {
+                    None
+                };
                 let server_host_key = match &server_host_certificate {
                     // The certificate's own signature is checked by the client
                     // against its trusted authorities, not here; what the key

russh/src/client/mod.rs

@@ -58,6 +58,7 @@ use tokio::sync::mpsc::{
 use tokio::sync::oneshot;
 
 pub use crate::auth::AuthResult;
+use crate::cert::PublicKeyOrCertificate;
 use crate::channels::{
     Channel, ChannelMsg, ChannelReadHalf, ChannelRef, ChannelWriteHalf, WindowSizeRef,
 };
@@ -1198,8 +1199,7 @@ impl Session {
             // Keep reading the network for window adjustments, but leave
             // application output in its bounded receivers while a channel is
             // window-blocked.
-            let can_receive_outbound =
-                !self.kex.active() && !self.common.has_any_pending_data();
+            let can_receive_outbound = !self.kex.active() && !self.common.has_any_pending_data();
             tokio::select! {
                 r = &mut reading => {
                     let (stream_read, mut buffer, mut opening_cipher) = match r {
@@ -1690,12 +1690,12 @@ async fn reply<H: Handler>(
                         // something the client was ever told to trust — asking
                         // about it as well would invite an implementation to
                         // answer yes to the wrong question.
-                        if let Some(certificate) = &server_host_certificate {
-                            if !handler.check_server_certificate(certificate).await? {
+                        if let Some(certificate) = server_host_certificate {
+                            if !handler.check_server_key(&certificate.into()).await? {
                                 return Err(crate::Error::UnknownKey.into());
                             }
-                        } else if let Some(server_host_key) = &server_host_key {
-                            let check = handler.check_server_key(server_host_key).await?;
+                        } else if let Some(server_host_key) = server_host_key {
+                            let check = handler.check_server_key(&server_host_key.into()).await?;
                             if !check {
                                 return Err(crate::Error::UnknownKey.into());
                             }
@@ -1754,7 +1754,7 @@ mod tests {
     impl Handler for TestHandler {
         type Error = crate::Error;
 
-        async fn check_server_key(&mut self, _: &ssh_key::PublicKey) -> Result<bool, Self::Error> {
+        async fn check_server_key(&mut self, _: &PublicKeyOrCertificate) -> Result<bool, Self::Error> {
             Ok(true)
         }
     }
@@ -2158,29 +2158,17 @@ pub trait Handler: Sized + Send {
         async { Ok(()) }
     }
 
-    /// Called to check the server's public key. This is a very important
-    /// step to help prevent man-in-the-middle attacks. The default
-    /// implementation rejects all keys.
-    #[allow(unused_variables)]
-    /// Called instead of [`Self::check_server_key`] when the server proved its
-    /// identity with a certificate.
+    /// Called to check the server's public key or certificate.
+    /// This is a very important step to help prevent man-in-the-middle attacks.
+    /// The default implementation rejects all keys, and you must override it.
     ///
-    /// Defaults to refusing. A client that has not been taught which
-    /// authorities it trusts cannot answer this question, and answering it
-    /// wrongly accepts any machine whose operator can obtain a certificate from
-    /// anyone at all — so silence has to mean no.
-    #[allow(unused_variables)]
-    fn check_server_certificate(
-        &mut self,
-        certificate: &ssh_key::Certificate,
-    ) -> impl std::future::Future<Output = Result<bool, Self::Error>> + Send {
-        async { Ok(false) }
-    }
-
+    /// The library verifies the key exchange signature before this call,
+    /// but it's up to the implementation to decide whether the key or certificate
+    /// is trusted.
     #[allow(unused_variables)]
     fn check_server_key(
         &mut self,
-        server_public_key: &ssh_key::PublicKey,
+        server_public_key: &PublicKeyOrCertificate,
     ) -> impl Future<Output = Result<bool, Self::Error>> + Send {
         async { Ok(false) }
     }

russh/src/client/test.rs

@@ -7,7 +7,8 @@ mod tests {
     use ssh_key::PrivateKey;
     use tokio::net::TcpListener;
 
-    // Import client types directly since we're in the client module
+    use crate::cert::PublicKeyOrCertificate;
+// Import client types directly since we're in the client module
     use crate::client::{Config, Handler, connect};
     use crate::keys::PrivateKeyWithHashAlg;
     use rand::rng;
@@ -74,7 +75,7 @@ mod tests {
     impl Handler for Client {
         type Error = Error;
 
-        async fn check_server_key(&mut self, _: &ssh_key::PublicKey) -> Result<bool, Self::Error> {
+        async fn check_server_key(&mut self, _: &PublicKeyOrCertificate) -> Result<bool, Self::Error> {
             Ok(true)
         }
     }

russh/src/keys/mod.rs

@@ -86,6 +86,8 @@ pub mod known_hosts;
 #[cfg(not(target_arch = "wasm32"))]
 pub use known_hosts::{check_known_hosts, check_known_hosts_path};
 
+pub use crate::cert::PublicKeyOrCertificate;
+
 #[derive(Debug, Error)]
 pub enum Error {
     /// The key could not be read, for an unknown reason