Refactor block-cipher packet-length probing

Author: mjc

russh/Cargo.toml

@@ -44,7 +44,7 @@ bytes.workspace = true
 cbc = { version = "0.1" }
 cbc_0_2 = { package = "cbc", version = "0.2.0" }
 cipher = "0.5.1" # only pinned due to a cargo-minimal-versions failure in 0.5.0
-ctr = "0.9"
+ctr = "0.9.2"
 ctr_0_10 = { package = "ctr", version = "0.10.0" }
 curve25519-dalek = "=5.0.0-pre.6"
 crypto-bigint = { version = "=0.7.0-rc.28", features = ["alloc"] }

russh/src/cipher/block.rs

@@ -34,9 +34,11 @@ fn new_cipher_from_slices<C: KeyIvInit>(k: &[u8], n: &[u8]) -> C {
     )
 }
 
-pub struct SshBlockCipher<C: BlockStreamCipher + KeySizeUser + IvSizeUser>(pub PhantomData<C>);
+pub struct SshBlockCipher<C: BlockStreamCipher + PacketLengthProbe + KeySizeUser + IvSizeUser>(
+    pub PhantomData<C>,
+);
 
-impl<C: BlockStreamCipher + KeySizeUser + IvSizeUser + KeyIvInit + Clone + Send + 'static>
+impl<C: BlockStreamCipher + PacketLengthProbe + KeySizeUser + IvSizeUser + KeyIvInit + Send + 'static>
     super::Cipher for SshBlockCipher<C>
 {
     fn key_len(&self) -> usize {
@@ -78,7 +80,7 @@ impl<C: BlockStreamCipher + KeySizeUser + IvSizeUser + KeyIvInit + Clone + Send
     }
 }
 
-pub struct OpeningKey<C: BlockStreamCipher> {
+pub struct OpeningKey<C: BlockStreamCipher + PacketLengthProbe> {
     pub(crate) cipher: C,
     pub(crate) mac: Box<dyn Mac + Send>,
 }
@@ -88,7 +90,9 @@ pub struct SealingKey<C: BlockStreamCipher> {
     pub(crate) mac: Box<dyn Mac + Send>,
 }
 
-impl<C: BlockStreamCipher + KeySizeUser + IvSizeUser + Clone> super::OpeningKey for OpeningKey<C> {
+impl<C: BlockStreamCipher + PacketLengthProbe + KeySizeUser + IvSizeUser> super::OpeningKey
+    for OpeningKey<C>
+{
     fn packet_length_to_read_for_block_length(&self) -> usize {
         16
     }
@@ -108,9 +112,7 @@ impl<C: BlockStreamCipher + KeySizeUser + IvSizeUser + Clone> super::OpeningKey
             #[allow(clippy::unwrap_used, clippy::indexing_slicing)]
             encrypted_packet_length[..4].try_into().unwrap()
         } else {
-            let mut cipher = self.cipher.clone();
-
-            cipher.decrypt_data(&mut first_block);
+            self.cipher.decrypt_packet_length_block(&mut first_block);
 
             // Fine because of self.packet_length_to_read_for_block_length()
             #[allow(clippy::unwrap_used, clippy::indexing_slicing)]
@@ -212,6 +214,10 @@ pub trait BlockStreamCipher {
     fn decrypt_data(&mut self, data: &mut [u8]);
 }
 
+pub(crate) trait PacketLengthProbe {
+    fn decrypt_packet_length_block(&self, first_block: &mut [u8; 16]);
+}
+
 impl<T: StreamCipher> BlockStreamCipher for T {
     fn encrypt_data(&mut self, data: &mut [u8]) {
         self.apply_keystream(data);
@@ -222,16 +228,48 @@ impl<T: StreamCipher> BlockStreamCipher for T {
     }
 }
 
+impl<T: StreamCipher + Clone> PacketLengthProbe for T {
+    fn decrypt_packet_length_block(&self, first_block: &mut [u8; 16]) {
+        let mut cipher = self.clone();
+        cipher.apply_keystream(first_block);
+    }
+}
+
 #[cfg(test)]
 mod tests {
+    use aes::cipher::KeyIvInit;
+    use aes::cipher::StreamCipher;
+    use aes::Aes128;
     use aes::cipher::{IvSizeUser, KeySizeUser};
+    use ctr::Ctr128BE;
     use digest::typenum::U16;
     use tokio::io::AsyncWriteExt;
 
-    use super::{BlockStreamCipher, OpeningKey};
+    use super::{BlockStreamCipher, OpeningKey, PacketLengthProbe};
     use crate::mac::MacAlgorithm;
     use crate::sshbuffer::SSHBuffer;
 
+    #[test]
+    fn stream_cipher_probe_does_not_advance_cipher_state() {
+        let plaintext = *b"0123456789ABCDEF";
+        let key = fixture_bytes::<16>(7);
+        let iv = fixture_bytes::<16>(3);
+
+        let mut encryptor = Ctr128BE::<Aes128>::new(&key.into(), &iv.into());
+        let mut ciphertext = plaintext;
+        encryptor.apply_keystream(&mut ciphertext);
+
+        let cipher = Ctr128BE::<Aes128>::new(&key.into(), &iv.into());
+        let mut probed_block = ciphertext;
+        cipher.decrypt_packet_length_block(&mut probed_block);
+        assert_eq!(probed_block, plaintext);
+
+        let mut decrypted = ciphertext;
+        let mut cipher_after_probe = cipher;
+        cipher_after_probe.decrypt_data(&mut decrypted);
+        assert_eq!(decrypted, plaintext);
+    }
+
     #[test]
     fn decrypt_packet_length_uses_independent_cipher_state() -> std::io::Result<()> {
         let runtime = tokio::runtime::Builder::new_current_thread()
@@ -292,4 +330,20 @@ mod tests {
             }
         }
     }
+
+    impl PacketLengthProbe for OwnedStateCipher {
+        fn decrypt_packet_length_block(&self, first_block: &mut [u8; 16]) {
+            if let Some(prefix) = first_block.get_mut(..4) {
+                prefix.copy_from_slice(&[0, 0, 0, 12]);
+            }
+        }
+    }
+
+    fn fixture_bytes<const N: usize>(seed: u8) -> [u8; N] {
+        let mut bytes = [0; N];
+        for (i, byte) in bytes.iter_mut().enumerate() {
+            *byte = seed.wrapping_add(i as u8);
+        }
+        bytes
+    }
 }

russh/src/cipher/cbc.rs

@@ -7,7 +7,7 @@ use digest::crypto_common::InnerUser;
 #[allow(deprecated)]
 use digest::generic_array::GenericArray;
 
-use super::block::BlockStreamCipher;
+use super::block::{BlockStreamCipher, PacketLengthProbe};
 
 // Allow deprecated generic-array 0.14 usage until RustCrypto crates (cipher, cbc, etc.)
 // upgrade to generic-array 1.x. Remove this when dependencies no longer use 0.14.
@@ -19,7 +19,6 @@ where
     GenericArray::from_slice(chunk).clone()
 }
 
-#[derive(Clone)]
 pub struct CbcWrapper<C: BlockEncrypt + BlockCipher + BlockDecrypt> {
     encryptor: Encryptor<C>,
     decryptor: Decryptor<C>,
@@ -51,6 +50,20 @@ impl<C: BlockEncrypt + BlockCipher + BlockDecrypt> BlockStreamCipher for CbcWrap
     }
 }
 
+impl<C: BlockEncrypt + BlockCipher + BlockDecrypt + Clone> PacketLengthProbe for CbcWrapper<C>
+where
+    C: BlockDecryptMut,
+{
+    fn decrypt_packet_length_block(&self, first_block: &mut [u8; 16]) {
+        let mut decryptor = self.decryptor.clone();
+        for chunk in first_block.chunks_exact_mut(C::block_size()) {
+            let mut block = generic_array_from_slice(chunk);
+            decryptor.decrypt_block_mut(&mut block);
+            chunk.copy_from_slice(&block);
+        }
+    }
+}
+
 impl<C: BlockEncrypt + BlockCipher + BlockDecrypt + Clone> InnerIvInit for CbcWrapper<C>
 where
     C: BlockEncryptMut + BlockCipher,
@@ -63,3 +76,59 @@ where
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use aes::cipher::KeyIvInit;
+    use aes::Aes128;
+    #[cfg(feature = "des")]
+    use des::TdesEde3;
+
+    use super::{BlockStreamCipher, CbcWrapper, PacketLengthProbe};
+
+    #[test]
+    fn packet_length_probe_does_not_advance_cbc_decryptor_state() {
+        let plaintext = *b"0123456789ABCDEF";
+        let key = fixture_bytes::<16>(11);
+        let iv = fixture_bytes::<16>(5);
+
+        let mut encryptor = CbcWrapper::<Aes128>::new(&key.into(), &iv.into());
+        let mut ciphertext = plaintext;
+        encryptor.encrypt_data(&mut ciphertext);
+
+        let cipher = CbcWrapper::<Aes128>::new(&key.into(), &iv.into());
+        let mut probed_block = ciphertext;
+        cipher.decrypt_packet_length_block(&mut probed_block);
+        assert_eq!(probed_block, plaintext);
+
+        let mut decrypted = ciphertext;
+        let mut cipher_after_probe = cipher;
+        cipher_after_probe.decrypt_data(&mut decrypted);
+        assert_eq!(decrypted, plaintext);
+    }
+
+    #[cfg(feature = "des")]
+    #[test]
+    fn packet_length_probe_respects_3des_block_size() {
+        let plaintext = *b"0123456789ABCDEF";
+        let key = fixture_bytes::<24>(11);
+        let iv = fixture_bytes::<8>(5);
+
+        let mut encryptor = CbcWrapper::<TdesEde3>::new(&key.into(), &iv.into());
+        let mut ciphertext = plaintext;
+        encryptor.encrypt_data(&mut ciphertext);
+
+        let cipher = CbcWrapper::<TdesEde3>::new(&key.into(), &iv.into());
+        let mut probed_block = ciphertext;
+        cipher.decrypt_packet_length_block(&mut probed_block);
+        assert_eq!(probed_block, plaintext);
+    }
+
+    fn fixture_bytes<const N: usize>(seed: u8) -> [u8; N] {
+        let mut bytes = [0; N];
+        for (i, byte) in bytes.iter_mut().enumerate() {
+            *byte = seed.wrapping_add(i as u8);
+        }
+        bytes
+    }
+}