client: verify certificate host keys

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,27 @@ 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 = Certificate::from_bytes(&server_host_key_blob).ok();
+                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 +295,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 +336,7 @@ impl ClientKex {
 
                 self.state = ClientKexState::WaitingForNewKeys {
                     server_host_key,
+                    server_host_certificate,
                     newkeys,
                 };
 
@@ -328,6 +347,7 @@ impl ClientKex {
             }
             ClientKexState::WaitingForNewKeys {
                 server_host_key,
+                server_host_certificate,
                 newkeys,
             } => {
                 // At this point the exchange is complete
@@ -349,6 +369,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

@@ -1652,6 +1652,7 @@ async fn reply<H: Handler>(
                 }
                 KexProgress::Done {
                     server_host_key,
+                    server_host_certificate,
                     newkeys,
                 } => {
                     debug!("kex impl has completed");
@@ -1684,7 +1685,16 @@ async fn reply<H: Handler>(
                         session.pending_len = 0;
                     } else {
                         // This is the initial kex
-                        if let Some(server_host_key) = &server_host_key {
+                        // 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_certificate(certificate).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?;
                             if !check {
                                 return Err(crate::Error::UnknownKey.into());
@@ -2151,6 +2161,22 @@ pub trait Handler: Sized + Send {
     /// 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.
+    ///
+    /// 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) }
+    }
+
     #[allow(unused_variables)]
     fn check_server_key(
         &mut self,

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/negotiation.rs

@@ -21,7 +21,7 @@ use ssh_encoding::{Decode, Encode};
 use ssh_key::{Algorithm, EcdsaCurve, HashAlg, PrivateKey};
 
 use crate::cipher::CIPHERS;
-use crate::helpers::NameList;
+use crate::helpers::{AlgorithmExt, NameList};
 use crate::kex::{
     EXTENSION_OPENSSH_STRICT_KEX_AS_CLIENT, EXTENSION_OPENSSH_STRICT_KEX_AS_SERVER, KexCause,
 };
@@ -68,6 +68,18 @@ pub struct Preferred {
     pub kex: Cow<'static, [kex::Name]>,
     /// Preferred host & public key algorithms.
     pub key: Cow<'static, [Algorithm]>,
+    /// Host-key certificate algorithms to advertise, most preferred first.
+    ///
+    /// A parallel list because [`Algorithm`] cannot represent one: it maps
+    /// `ssh-ed25519-cert-v01@openssh.com` back to `Ed25519`, so a certificate
+    /// placed in `key` would be advertised under the plain name and the server
+    /// would never send a certificate at all.
+    ///
+    /// Empty by default. Advertising a certificate algorithm makes a server
+    /// prove its identity with a certificate instead of a bare key, which a
+    /// client can only act on once it knows which authorities it trusts — so
+    /// turning it on is the caller's decision, never a default.
+    pub host_key_certificates: Cow<'static, [&'static str]>,
     /// Preferred symmetric ciphers.
     pub cipher: Cow<'static, [cipher::Name]>,
     /// Preferred MAC algorithms.
@@ -152,6 +164,7 @@ const COMPRESSION_ORDER: &[compression::Name] = &[
 impl Preferred {
     pub const DEFAULT: Preferred = Preferred {
         kex: Cow::Borrowed(SAFE_KEX_ORDER),
+        host_key_certificates: Cow::Borrowed(&[]),
         key: Cow::Borrowed(&[
             Algorithm::Ed25519,
             Algorithm::Ecdsa {
@@ -178,6 +191,7 @@ impl Preferred {
 
     pub const COMPRESSED: Preferred = Preferred {
         kex: Cow::Borrowed(SAFE_KEX_ORDER),
+        host_key_certificates: Cow::Borrowed(&[]),
         key: Preferred::DEFAULT.key,
         cipher: Cow::Borrowed(CIPHER_ORDER),
         mac: Cow::Borrowed(SAFE_HMAC_ORDER),
@@ -270,8 +284,22 @@ pub(crate) trait Select {
             None => pref.key.iter().map(ToOwned::to_owned).collect::<Vec<_>>(),
         };
 
-        let (key_both_first, key_algorithm) =
-            Self::select(&possible_host_key_algos[..], &key_list, AlgorithmKind::Key)?;
+        // A certificate the server offers wins over a bare key, when this side
+        // asked for one at all. The algorithm kept is the plain one the
+        // certificate contains: that is what signs the exchange, and it is what
+        // every later step needs. Whether a certificate arrived is decided by
+        // reading the blob, not by remembering this choice.
+        let offered_certificate = pref
+            .host_key_certificates
+            .iter()
+            .find(|name| key_list.0.iter().any(|offered| offered == *name));
+        let (key_both_first, key_algorithm) = match offered_certificate {
+            Some(name) => (
+                key_list.0.first().map(|first| first == *name).unwrap_or(false),
+                Algorithm::new_certificate_ext(name).map_err(|_| Error::KexInit)?,
+            ),
+            None => Self::select(&possible_host_key_algos[..], &key_list, AlgorithmKind::Key)?,
+        };
 
         // Cipher
 
@@ -460,7 +488,15 @@ pub(crate) fn write_kex(
             )
             .encode(w)?;
         } else {
-            NameList(prefs.key.iter().map(ToString::to_string).collect()).encode(w)?;
+            NameList(
+                prefs
+                    .host_key_certificates
+                    .iter()
+                    .map(|name| (*name).to_string())
+                    .chain(prefs.key.iter().map(ToString::to_string))
+                    .collect(),
+            )
+            .encode(w)?;
         }
 
         // cipher client to server
@@ -523,7 +559,7 @@ mod tests {
     use ssh_encoding::Encode;
 
     use super::*;
-    use crate::helpers::NameList;
+    use crate::helpers::{AlgorithmExt, NameList};
 
     /// Build a minimal KEXINIT payload with a custom kex name-list and
     /// `first_kex_packet_follows` flag. All other lists come from the default

russh/src/server/encrypted.rs

@@ -511,6 +511,7 @@ mod tests {
     fn test_auth_session() -> Session {
         let mut config = Config::default();
         config.preferred = Preferred {
+            host_key_certificates: Cow::Borrowed(&[]),
             kex: Cow::Owned(vec![KEX_NONE]),
             key: Cow::Owned(vec![ssh_key::Algorithm::Ed25519]),
             cipher: Cow::Owned(vec![cipher::NONE]),

russh/src/server/kex.rs

@@ -145,6 +145,7 @@ impl ServerKex {
                     })?;
 
                     return Ok(KexProgress::Done {
+                        server_host_certificate: None,
                         newkeys,
                         server_host_key: None,
                     });
@@ -336,6 +337,7 @@ impl ServerKex {
 
                 debug!("new keys received");
                 Ok(KexProgress::Done {
+                    server_host_certificate: None,
                     newkeys,
                     server_host_key: None,
                 })

russh/src/tests.rs

@@ -159,6 +159,7 @@ mod compress {
 
     fn preferred_zlib() -> Preferred {
         Preferred {
+            host_key_certificates: Cow::Borrowed(&[]),
             compression: Cow::Borrowed(&[
                 crate::compression::ZLIB,
                 crate::compression::ZLIB_LEGACY,
@@ -942,6 +943,7 @@ pub(crate) mod raw_no_crypto {
 
     fn no_crypto_preferred() -> Preferred {
         Preferred {
+            host_key_certificates: Cow::Borrowed(&[]),
             kex: Cow::Owned(vec![kex::NONE]),
             key: Cow::Owned(vec![Algorithm::Ed25519]),
             cipher: Cow::Owned(vec![cipher::NONE]),