Support host certificates in client (#752)

Co-authored-by: biao29 <hi@ctx.st>

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

@@ -6,7 +6,7 @@ use std::sync::Arc;
 use bytes::Bytes;
 use log::{debug, error, warn};
 use ssh_encoding::{Decode, Encode};
-use ssh_key::{Mpint, PublicKey, Signature};
+use ssh_key::{Certificate, Mpint, PublicKey, Signature};
 
 use super::IncomingSshPacket;
 use crate::client::{Config, NewKeys};
@@ -38,6 +38,7 @@ enum ClientKexState {
     },
     WaitingForNewKeys {
         server_host_key: PublicKey,
+        server_host_certificate: Option<Certificate>,
         newkeys: NewKeys,
     },
 }
@@ -152,6 +153,7 @@ impl ClientKex {
                     })?;
 
                     return Ok(KexProgress::Done {
+                        server_host_certificate: None,
                         newkeys,
                         server_host_key: None,
                     });
@@ -263,11 +265,31 @@ impl ClientKex {
                 #[allow(clippy::indexing_slicing)] // length checked
                 let r = &mut &input.buffer[1..];
 
-                let server_host_key = Bytes::decode(r)?; // server public key.
-                let server_host_key = parse_public_key(&server_host_key)?;
+                // The raw blob is kept as well as the parsed key. It is what
+                // goes into the exchange hash below: for a certificate the
+                // parsed form is only the key *inside* it, and re-encoding that
+                // would hash something the server never sent — a failure that
+                // 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 = 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
+                    // exchange is signed with is the key the certificate
+                    // contains. The two are separate proofs and collapsing them
+                    // would accept a certificate nobody vouched for.
+                    Some(certificate) => PublicKey::new(certificate.public_key().clone(), ""),
+                    None => parse_public_key(&server_host_key_blob)?,
+                };
                 debug!(
-                    "received server host key: {:?}",
-                    server_host_key.to_openssh()
+                    "received server host key: {:?} (certificate: {})",
+                    server_host_key.to_openssh(),
+                    server_host_certificate.is_some()
                 );
 
                 let server_ephemeral = Bytes::decode(r)?;
@@ -277,7 +299,7 @@ impl ClientKex {
                 kex.compute_shared_secret(&self.exchange.server_ephemeral)?;
 
                 let mut pubkey_vec = Vec::new();
-                server_host_key.to_bytes()?.encode(&mut pubkey_vec)?;
+                server_host_key_blob.encode(&mut pubkey_vec)?;
 
                 let exchange = &self.exchange;
                 let hash = HASH_BUFFER.with({
@@ -318,6 +340,7 @@ impl ClientKex {
 
                 self.state = ClientKexState::WaitingForNewKeys {
                     server_host_key,
+                    server_host_certificate,
                     newkeys,
                 };
 
@@ -328,6 +351,7 @@ impl ClientKex {
             }
             ClientKexState::WaitingForNewKeys {
                 server_host_key,
+                server_host_certificate,
                 newkeys,
             } => {
                 // At this point the exchange is complete
@@ -349,6 +373,7 @@ impl ClientKex {
                 ensure_end(&r)?;
 
                 Ok(KexProgress::Done {
+                    server_host_certificate,
                     newkeys,
                     server_host_key: Some(server_host_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,
 };
@@ -1804,6 +1805,7 @@ async fn reply<H: Handler>(
                 }
                 KexProgress::Done {
                     server_host_key,
+                    server_host_certificate,
                     newkeys,
                 } => {
                     debug!("kex impl has completed");
@@ -1836,8 +1838,17 @@ async fn reply<H: Handler>(
                         session.pending_len = 0;
                     } else {
                         // This is the initial kex
-                        if let Some(server_host_key) = &server_host_key {
-                            let check = handler.check_server_key(server_host_key).await?;
+                        // A certificate replaces the key check rather than
+                        // adding to it. The key inside a certificate is not
+                        // 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_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.into()).await?;
                             if !check {
                                 return Err(crate::Error::UnknownKey.into());
                             }
@@ -1895,7 +1906,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)
         }
     }
@@ -2299,13 +2310,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.
+    /// 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.
+    ///
+    /// 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,6 +7,7 @@ mod tests {
     use ssh_key::PrivateKey;
     use tokio::net::TcpListener;
 
+    use crate::cert::PublicKeyOrCertificate;
     // Import client types directly since we're in the client module
     use crate::Error;
     use crate::client::{Config, Handler, connect};
@@ -74,7 +75,10 @@ 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/kex/mod.rs

@@ -44,7 +44,7 @@ use p521::NistP521;
 use sha1::Sha1;
 use sha2::{Sha256, Sha384, Sha512};
 use ssh_encoding::{Encode, Writer};
-use ssh_key::PublicKey;
+use ssh_key::{Certificate, PublicKey};
 
 use crate::cipher::CIPHERS;
 use crate::client::GexParams;
@@ -121,6 +121,14 @@ pub(crate) enum KexProgress<T> {
     },
     Done {
         server_host_key: Option<PublicKey>,
+        /// The certificate the server presented, when it presented one.
+        ///
+        /// Carried beside the key rather than replacing it: the key exchange is
+        /// signed with the key the certificate contains, so both are needed —
+        /// one to know what signed the handshake, the other to decide whether
+        /// anyone vouches for it. Collapsing them would leave the second
+        /// question unasked.
+        server_host_certificate: Option<Certificate>,
         newkeys: NewKeys,
     },
 }

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