Use secrecy to ensure forward secrecy of the transport layer in Keys

assh/Cargo.toml

@@ -28,7 +28,7 @@ either.workspace = true
 tracing.workspace = true
 thiserror.workspace = true
 strum = { version = "0.26.1", features = ["derive"] }
-securefmt = "0.1.4"
+secrecy = "0.10.3"
 
 ssh-key.workspace = true
 ssh-packet.workspace = true

assh/src/algorithm/kex.rs

@@ -77,10 +77,11 @@ impl Kex {
                     <[u8; 32]>::try_from(&*ecdh.q_s).map_err(|_| Error::KexError)?,
                 );
 
+                // TODO: (security) use `secrecy` to encapsulate this value
                 let secret: MpInt = e_c.diffie_hellman(&q_s).to_bytes().to_vec().into();
 
                 let k_s = ssh_key::PublicKey::from_bytes(&ecdh.k_s)?;
-                let exchange = EcdhExchange {
+                let hash = EcdhExchange {
                     v_c: &v_c.to_string().into_bytes().into(),
                     v_s: &v_s.to_string().into_bytes().into(),
                     i_c: &{
@@ -97,8 +98,8 @@ impl Kex {
                     q_c: &q_c.as_ref().to_vec().into(),
                     q_s: &q_s.to_bytes().to_vec().into(),
                     k: &secret,
-                };
-                let hash = exchange.hash::<Hash>();
+                }
+                .hash::<Hash>();
 
                 Verifier::verify(&k_s, &hash, &Signature::try_from(&*ecdh.signature)?)?;
 
@@ -162,12 +163,13 @@ impl Kex {
                     <[u8; 32]>::try_from(&*ecdh.q_c).map_err(|_| Error::KexError)?,
                 );
 
+                // TODO: (security) use `secrecy` to encapsulate this value
                 let secret: MpInt = e_s.diffie_hellman(&q_c).to_bytes().to_vec().into();
 
                 let k_s = key.public_key().to_bytes()?.into();
                 let q_s = q_s.as_ref().to_vec().into();
 
-                let exchange = EcdhExchange {
+                let hash = EcdhExchange {
                     v_c: &v_c.to_string().into_bytes().into(),
                     v_s: &v_s.to_string().into_bytes().into(),
                     i_c: &{
@@ -184,8 +186,8 @@ impl Kex {
                     q_c: &q_c.to_bytes().to_vec().into(),
                     q_s: &q_s,
                     k: &secret,
-                };
-                let hash = exchange.hash::<Hash>();
+                }
+                .hash::<Hash>();
 
                 let signature = Signer::sign(key, &hash);
                 stream

assh/src/stream/keys.rs

@@ -1,19 +1,19 @@
 use digest::{Digest, FixedOutputReset};
-use securefmt::Debug;
+use secrecy::SecretBox;
 use ssh_packet::Mac;
 
 use super::algorithm::Cipher;
 
 #[derive(Debug, Default)]
 pub struct Keys {
-    #[sensitive]
-    pub iv: Vec<u8>,
+    /// Cipher _initialization vector_.
+    pub iv: SecretBox<Vec<u8>>,
 
-    #[sensitive]
-    pub key: Vec<u8>,
+    /// Cipher _key_.
+    pub key: SecretBox<Vec<u8>>,
 
-    #[sensitive]
-    pub hmac: Vec<u8>,
+    /// Hmac _key_.
+    pub hmac: SecretBox<Vec<u8>>,
 }
 
 impl Keys {
@@ -59,28 +59,28 @@ impl Keys {
         kind: u8,
         session_id: &[u8],
         size: usize,
-    ) -> Vec<u8> {
-        let mut hasher = D::new()
-            .chain_update((secret.as_ref().len() as u32).to_be_bytes())
-            .chain_update(secret)
-            .chain_update(hash)
-            .chain_update([kind])
-            .chain_update(session_id);
-
-        let mut key = hasher.finalize_reset().to_vec();
-
-        while key.len() < size {
-            hasher = hasher
+    ) -> SecretBox<Vec<u8>> {
+        SecretBox::<Vec<u8>>::init_with_mut(|key| {
+            let mut hasher = D::new()
                 .chain_update((secret.as_ref().len() as u32).to_be_bytes())
                 .chain_update(secret)
                 .chain_update(hash)
-                .chain_update(&key);
+                .chain_update([kind])
+                .chain_update(session_id);
 
             key.extend_from_slice(&hasher.finalize_reset());
-        }
 
-        key.resize(size, 0);
+            while key.len() < size {
+                hasher = hasher
+                    .chain_update((secret.as_ref().len() as u32).to_be_bytes())
+                    .chain_update(secret)
+                    .chain_update(hash)
+                    .chain_update(&*key);
+
+                key.extend_from_slice(&hasher.finalize_reset());
+            }
 
-        key
+            key.truncate(size);
+        })
     }
 }

assh/src/stream/transport.rs

@@ -1,5 +1,5 @@
 use rand::Rng;
-use securefmt::Debug;
+use secrecy::ExposeSecret;
 use ssh_packet::{CipherCore, Mac, OpeningCipher, SealingCipher};
 
 use crate::{
@@ -9,8 +9,6 @@ use crate::{
 
 use super::Keys;
 
-// TODO: (feature) Provide forward secrecy of keys with `secrecy` or `zeroize`.
-
 #[derive(Debug, Default)]
 pub struct TransportPair {
     pub rx: Transport,
@@ -19,9 +17,7 @@ pub struct TransportPair {
 
 #[derive(Debug, Default)]
 pub struct Transport {
-    #[sensitive]
     pub chain: Keys,
-    #[sensitive]
     pub state: Option<CipherState>,
     pub cipher: algorithm::Cipher,
     pub hmac: algorithm::Hmac,
@@ -46,8 +42,8 @@ impl OpeningCipher for Transport {
         if self.cipher != Cipher::None {
             self.cipher.decrypt(
                 &mut self.state,
-                &self.chain.key,
-                &self.chain.iv,
+                self.chain.key.expose_secret(),
+                self.chain.iv.expose_secret(),
                 buf.as_mut(),
             )?;
         }
@@ -58,7 +54,7 @@ impl OpeningCipher for Transport {
     fn open<B: AsRef<[u8]>>(&mut self, buf: B, mac: Vec<u8>, seq: u32) -> Result<(), Self::Err> {
         if self.mac().size() > 0 {
             self.hmac
-                .verify(seq, buf.as_ref(), &self.chain.hmac, &mac)?;
+                .verify(seq, buf.as_ref(), self.chain.hmac.expose_secret(), &mac)?;
         }
 
         Ok(())
@@ -91,8 +87,8 @@ impl SealingCipher for Transport {
         if self.cipher != Cipher::None {
             self.cipher.encrypt(
                 &mut self.state,
-                &self.chain.key,
-                &self.chain.iv,
+                self.chain.key.expose_secret(),
+                self.chain.iv.expose_secret(),
                 buf.as_mut(),
             )?;
         }
@@ -101,6 +97,8 @@ impl SealingCipher for Transport {
     }
 
     fn seal<B: AsRef<[u8]>>(&mut self, buf: B, seq: u32) -> Result<Vec<u8>, Self::Err> {
-        Ok(self.hmac.sign(seq, buf.as_ref(), &self.chain.hmac))
+        Ok(self
+            .hmac
+            .sign(seq, buf.as_ref(), self.chain.hmac.expose_secret()))
     }
 }