ssh-cipher: add AAD support to ChaCha20Poly1305 (#281)

From PROTOCOL.chacha20poly1305:

  Once the entire packet has been received, the MAC MUST be checked
  before decryption. A per-packet Poly1305 key is generated as described
  above and the MAC tag calculated using Poly1305 with this key over the
  ciphertext of the packet length and the payload together.

This adds an `aad_len` parameter which decomposes the input buffer into
a portion to be only authenticated (in packet encryption, this is used
for a 4-byte encrypted length header), which comes prior to the portion
to be encrypted.

Ideally we could implement the `AeadInPlace` trait, however this
approach has been used instead because the protocol uses unpadded
Poly1305, where we don't support buffered input and it must be computed
from a single contiguous slice using `Poly1305::compute_unpadded`.

Closes #279

ssh-cipher/Cargo.toml

@@ -33,6 +33,9 @@ des = { version = "=0.9.0-pre.1", optional = true, default-features = false }
 poly1305 = { version = "0.9.0-rc.0", optional = true, default-features = false }
 subtle = { version = "2", optional = true, default-features = false }
 
+[dev-dependencies]
+hex-literal = "0.4"
+
 [features]
 std = []
 

ssh-cipher/src/chacha20poly1305.rs

@@ -1,11 +1,12 @@
 //! OpenSSH variant of ChaCha20Poly1305.
 
+pub use chacha20::ChaCha20Legacy as ChaCha20;
+
 use crate::Tag;
 use aead::{
     array::typenum::{U0, U16, U32, U8},
-    AeadCore, AeadInPlace, Error, KeyInit, KeySizeUser, Result,
+    AeadCore, Error, KeyInit, KeySizeUser, Result,
 };
-use chacha20::ChaCha20Legacy as ChaCha20;
 use cipher::{KeyIvInit, StreamCipher, StreamCipherSeek};
 use poly1305::Poly1305;
 use subtle::ConstantTimeEq;
@@ -20,9 +21,9 @@ pub type ChaChaNonce = chacha20::LegacyNonce;
 /// as described in [PROTOCOL.chacha20poly1305].
 ///
 /// Differences from ChaCha20Poly1305-IETF as described in [RFC8439]:
-/// - The input of Poly1305 is not padded.
-/// - AAD is unsupported.
-/// - The lengths of ciphertext (and AAD) are not authenticated using Poly1305.
+/// - Nonce is 64-bit instead of 96-bit (i.e. uses legacy "djb" ChaCha20 variant).
+/// - The AAD and ciphertext inputs of Poly1305 are not padded.
+/// - The lengths of ciphertext and AAD are not authenticated using Poly1305.
 ///
 /// [PROTOCOL.chacha20poly1305]: https://cvsweb.openbsd.org/src/usr.bin/ssh/PROTOCOL.chacha20poly1305?annotate=HEAD
 /// [RFC8439]: https://datatracker.ietf.org/doc/html/rfc8439
@@ -49,24 +50,46 @@ impl AeadCore for ChaCha20Poly1305 {
     type CiphertextOverhead = U0;
 }
 
-impl AeadInPlace for ChaCha20Poly1305 {
-    fn encrypt_in_place_detached(
-        &self,
-        nonce: &ChaChaNonce,
-        associated_data: &[u8],
-        buffer: &mut [u8],
-    ) -> Result<Tag> {
-        Cipher::new(&self.key, nonce).encrypt(associated_data, buffer)
+impl ChaCha20Poly1305 {
+    /// Encrypt the provided `buffer` in-place, returning the Poly1305 authentication tag.
+    ///
+    /// The input `buffer` should contain the concatenation of any additional associated data (AAD)
+    /// and the plaintext to be encrypted, where in the context of the SSH packet encryption
+    /// protocol the AAD represents an encrypted packet length, which is itself 4-bytes / 64-bits.
+    ///
+    /// `aad_len` is the length of the AAD in bytes:
+    /// - In the context of SSH packet encryption, this should be `4`.
+    /// - In the context of SSH key encryption, `aad_len` should be `0`.
+    ///
+    /// The first `aad_len` bytes of `buffer` will be unmodified after encryption is completed.
+    /// Only the data after `aad_len` will be encrypted.
+    ///
+    /// The resulting `Tag` authenticates both the AAD and the ciphertext in the buffer.
+    pub fn encrypt(&self, nonce: &ChaChaNonce, buffer: &mut [u8], aad_len: usize) -> Result<Tag> {
+        Cipher::new(&self.key, nonce).encrypt(buffer, aad_len)
     }
 
-    fn decrypt_in_place_detached(
+    /// Decrypt the provided `buffer` in-place, verifying it against the provided Poly1305
+    /// authentication `tag`.
+    ///
+    /// The input `buffer` should contain the concatenation of any additional associated data (AAD)
+    /// and the ciphertext to be authenticated, where in the context of the SSH packet encryption
+    /// protocol the AAD represents an encrypted packet length, which is itself 4-bytes / 64-bits.
+    ///
+    /// `aad_len` is the length of the AAD in bytes:
+    /// - In the context of SSH packet encryption, this should be `4`.
+    /// - In the context of SSH key encryption, `aad_len` should be `0`.
+    ///
+    /// The first `aad_len` bytes of `buffer` will be unmodified after decryption completes
+    /// successfully. Only data after `aad_len` will be decrypted.
+    pub fn decrypt(
         &self,
         nonce: &ChaChaNonce,
-        associated_data: &[u8],
         buffer: &mut [u8],
-        tag: &Tag,
+        tag: Tag,
+        aad_len: usize,
     ) -> Result<()> {
-        Cipher::new(&self.key, nonce).decrypt(associated_data, buffer, *tag)
+        Cipher::new(&self.key, nonce).decrypt(buffer, tag, aad_len)
     }
 }
 
@@ -93,37 +116,72 @@ impl Cipher {
 
     /// Encrypt the provided `buffer` in-place, returning the Poly1305 authentication tag.
     #[inline]
-    pub fn encrypt(mut self, associated_data: &[u8], buffer: &mut [u8]) -> Result<Tag> {
-        // TODO(tarcieri): support associated data (RustCrypto/SSH#279)
-        if !associated_data.is_empty() {
+    pub fn encrypt(mut self, buffer: &mut [u8], aad_len: usize) -> Result<Tag> {
+        if buffer.len() < aad_len {
             return Err(Error);
         }
 
-        self.cipher.apply_keystream(buffer);
+        self.cipher.apply_keystream(&mut buffer[aad_len..]);
         Ok(self.mac.compute_unpadded(buffer))
     }
 
     /// Decrypt the provided `buffer` in-place, verifying it against the provided Poly1305
     /// authentication `tag`.
-    ///
-    /// In the event tag verification fails, [`Error::Crypto`] is returned, and `buffer` is not
-    /// modified.
-    ///
-    /// Upon success, `Ok(())` is returned and `buffer` is rewritten with the decrypted plaintext.
     #[inline]
-    pub fn decrypt(mut self, associated_data: &[u8], buffer: &mut [u8], tag: Tag) -> Result<()> {
-        // TODO(tarcieri): support associated data (RustCrypto/SSH#279)
-        if !associated_data.is_empty() {
+    pub fn decrypt(mut self, buffer: &mut [u8], tag: Tag, aad_len: usize) -> Result<()> {
+        if buffer.len() < aad_len {
             return Err(Error);
         }
 
         let expected_tag = self.mac.compute_unpadded(buffer);
 
         if expected_tag.ct_eq(&tag).into() {
-            self.cipher.apply_keystream(buffer);
+            self.cipher.apply_keystream(&mut buffer[aad_len..]);
             Ok(())
         } else {
             Err(Error)
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::{ChaCha20Poly1305, KeyInit};
+    use hex_literal::hex;
+
+    #[test]
+    fn test_vector() {
+        let key = hex!("379a8ca9e7e705763633213511e8d92eb148a46f1dd0045ec8164e5d23e456eb");
+        let nonce = hex!("0000000000000003");
+        let aad = hex!("5709db2d");
+        let plaintext = hex!("06050000000c7373682d7573657261757468de5949ab061f");
+        let ciphertext = hex!("6dcfb03be8a55e7f0220465672edd921489ea0171198e8a7");
+        let tag = hex!("3e82fe0a2db7128d58ef8d9047963ca3");
+
+        const AAD_LEN: usize = 4;
+        const PT_LEN: usize = 24;
+        assert_eq!(aad.len(), AAD_LEN);
+        assert_eq!(plaintext.len(), PT_LEN);
+
+        let cipher = ChaCha20Poly1305::new(key.as_ref());
+        let mut buffer = [0u8; AAD_LEN + PT_LEN];
+        let (a, p) = buffer.split_at_mut(AAD_LEN);
+        a.copy_from_slice(&aad);
+        p.copy_from_slice(&plaintext);
+
+        let actual_tag = cipher
+            .encrypt(nonce.as_ref(), &mut buffer, AAD_LEN)
+            .unwrap();
+
+        assert_eq!(&buffer[..AAD_LEN], aad);
+        assert_eq!(&buffer[AAD_LEN..], ciphertext);
+        assert_eq!(actual_tag, tag);
+
+        cipher
+            .decrypt(nonce.as_ref(), &mut buffer, actual_tag, AAD_LEN)
+            .unwrap();
+
+        assert_eq!(&buffer[..AAD_LEN], aad);
+        assert_eq!(&buffer[AAD_LEN..], plaintext);
+    }
+}

ssh-cipher/src/lib.rs

@@ -34,25 +34,26 @@ mod decryptor;
 mod encryptor;
 
 pub use crate::error::{Error, Result};
+pub use cipher;
 
 #[cfg(any(feature = "aes-cbc", feature = "aes-ctr", feature = "tdes"))]
 pub use crate::{decryptor::Decryptor, encryptor::Encryptor};
 
 #[cfg(feature = "chacha20poly1305")]
-pub use crate::chacha20poly1305::{ChaCha20Poly1305, ChaChaKey, ChaChaNonce};
+pub use crate::chacha20poly1305::{ChaCha20, ChaCha20Poly1305, ChaChaKey, ChaChaNonce};
 
 use cipher::array::{typenum::U16, Array};
 use core::{fmt, str};
 use encoding::{Label, LabelError};
 
 #[cfg(feature = "aes-gcm")]
 use {
-    aead::array::typenum::U12,
+    aead::{array::typenum::U12, AeadInPlace},
     aes_gcm::{Aes128Gcm, Aes256Gcm},
 };
 
 #[cfg(any(feature = "aes-gcm", feature = "chacha20poly1305"))]
-use aead::{AeadInPlace, KeyInit};
+use aead::KeyInit;
 
 /// AES-128 in block chaining (CBC) mode
 const AES128_CBC: &str = "aes128-cbc";
@@ -260,7 +261,7 @@ impl Cipher {
                 let nonce = iv.try_into().map_err(|_| Error::IvSize)?;
                 let tag = tag.ok_or(Error::TagSize)?;
                 ChaCha20Poly1305::new(key)
-                    .decrypt_in_place_detached(nonce, b"", buffer, &tag)
+                    .decrypt(nonce, buffer, tag, 0)
                     .map_err(|_| Error::Crypto)
             }
             // Use `Decryptor` for non-AEAD modes
@@ -322,7 +323,7 @@ impl Cipher {
                 let key = key.try_into().map_err(|_| Error::KeySize)?;
                 let nonce = iv.try_into().map_err(|_| Error::IvSize)?;
                 let tag = ChaCha20Poly1305::new(key)
-                    .encrypt_in_place_detached(nonce, b"", buffer)
+                    .encrypt(nonce, buffer, 0)
                     .map_err(|_| Error::Crypto)?;
                 Ok(Some(tag))
             }