openpgp: Combine ciphertext and tag in Aead::decrypt_verify.

  - It is easier (and cheaper) to tear apart in backends that need
    ciphertext and tag to be separate than to combine it for backends
    that expect the tag to be appended to the ciphertext.

  - The caller doesn't have to do anything, because in OpenPGP on the
    wire the tag is already appended to the ciphertext.  The one
    exception is our current implementation of SKESKv5, but in our
    upcoming SKESKv6 implementation, we store the tag appended to the
    ciphertext, so it will be easy to use this interface there.

6 files changed, 58 insertions, 23 deletions

diff --git a/openpgp/src/crypto/aead.rs b/openpgp/src/crypto/aead.rs
index feee035f..700a2bcb 100644
--- a/openpgp/src/crypto/aead.rs
+++ b/openpgp/src/crypto/aead.rs
@@ -59,9 +59,9 @@ pub trait Aead : seal::Sealed {
     /// Length of the digest in bytes.
     fn digest_size(&self) -> usize;
 
-    /// Decrypt one block `src` to `dst` and verify if the digest
-    /// matches `digest`.
-    fn decrypt_verify(&mut self, dst: &mut [u8], src: &[u8], digest: &[u8]) -> Result<()>;
+    /// Decrypt one chunk `src` to `dst` and verify that the digest is
+    /// correct.
+    fn decrypt_verify(&mut self, dst: &mut [u8], src: &[u8]) -> Result<()>;
 }
 
 /// Whether AEAD cipher is used for data encryption or decryption.
@@ -401,7 +401,7 @@ impl<'a, S: Schedule> Decryptor<'a, S> {
                     &mut plaintext[pos..pos + to_decrypt]
                 };
 
-                aead.decrypt_verify(buffer, &chunk[..to_decrypt], &chunk[to_decrypt..])?;
+                aead.decrypt_verify(buffer, chunk)?;
 
                 if double_buffer {
                     let to_copy = plaintext.len() - pos;
@@ -437,7 +437,7 @@ impl<'a, S: Schedule> Decryptor<'a, S> {
 
                 let final_digest = self.source.data(final_digest_size)?;
 
-                aead.decrypt_verify(&mut [], &[], final_digest)?;
+                aead.decrypt_verify(&mut [], final_digest)?;
 
                 // Consume the data only on success so that we keep
                 // returning the error.
diff --git a/openpgp/src/crypto/backend/cng/aead.rs b/openpgp/src/crypto/backend/cng/aead.rs
index c714efbb..a3aaefb4 100644
--- a/openpgp/src/crypto/backend/cng/aead.rs
+++ b/openpgp/src/crypto/backend/cng/aead.rs
@@ -127,7 +127,7 @@ macro_rules! impl_aead {
                 dst[src.len()..].copy_from_slice(&tag[..]);
                 Ok(())
             }
-            fn decrypt_verify(&mut self, _dst: &mut [u8], _src: &[u8], _digest: &[u8]) -> Result<()> {
+            fn decrypt_verify(&mut self, _dst: &mut [u8], _src: &[u8]) -> Result<()> {
                 panic!("AEAD decryption called in the encryption context")
             }
         }
@@ -141,7 +141,14 @@ macro_rules! impl_aead {
             fn encrypt_seal(&mut self, _dst: &mut [u8], _src: &[u8]) -> Result<()> {
                 panic!("AEAD encryption called in the decryption context")
             }
-            fn decrypt_verify(&mut self, dst: &mut [u8], src: &[u8], digest: &[u8]) -> Result<()> {
+            fn decrypt_verify(&mut self, dst: &mut [u8], src: &[u8]) -> Result<()> {
+                debug_assert_eq!(dst.len() + self.digest_size(), src.len());
+
+                // Split src into ciphertext and digest.
+                let l = self.digest_size();
+                let digest = &src[src.len().saturating_sub(l)..];
+                let src = &src[..src.len().saturating_sub(l)];
+
                 let len = core::cmp::min(dst.len(), src.len());
                 dst[..len].copy_from_slice(&src[..len]);
                 self.decrypt_unauthenticated_hazmat(&mut dst[..len]);
diff --git a/openpgp/src/crypto/backend/nettle/aead.rs b/openpgp/src/crypto/backend/nettle/aead.rs
index ad6014d7..486269b1 100644
--- a/openpgp/src/crypto/backend/nettle/aead.rs
+++ b/openpgp/src/crypto/backend/nettle/aead.rs
@@ -24,11 +24,25 @@ impl<T: nettle::aead::Aead> Aead for T {
         self.digest(&mut dst[src.len()..]);
         Ok(())
     }
-    fn decrypt_verify(&mut self, dst: &mut [u8], src: &[u8], digest: &[u8]) -> Result<()> {
-        self.decrypt(dst, src);
-        let mut chunk_digest = vec![0u8; self.digest_size()];
+    fn decrypt_verify(&mut self, dst: &mut [u8], src: &[u8]) -> Result<()> {
+        debug_assert!(src.len() >= self.digest_size());
+        debug_assert_eq!(dst.len() + self.digest_size(), src.len());
 
-        self.digest(&mut chunk_digest);
+        // Split src into ciphertext and digest.
+        let l = self.digest_size();
+        let ciphertext = &src[..src.len().saturating_sub(l)];
+        let digest = &src[src.len().saturating_sub(l)..];
+
+        // Decrypt the chunk.
+        self.decrypt(dst, ciphertext);
+
+        // Compute the digest, storing it on the stack.
+        let mut chunk_digest_store = [0u8; 16];
+        debug_assert!(chunk_digest_store.len() >= l);
+        let chunk_digest = &mut chunk_digest_store[..l];
+        self.digest(chunk_digest);
+
+        // Authenticate the chunk.
         if secure_cmp(&chunk_digest[..], digest)
              != Ordering::Equal && ! DANGER_DISABLE_AUTHENTICATION
             {
diff --git a/openpgp/src/crypto/backend/openssl/aead.rs b/openpgp/src/crypto/backend/openssl/aead.rs
index 469539c0..cc8735fb 100644
--- a/openpgp/src/crypto/backend/openssl/aead.rs
+++ b/openpgp/src/crypto/backend/openssl/aead.rs
@@ -25,19 +25,22 @@ impl Aead for OpenSslContext {
         Ok(())
     }
 
-    fn decrypt_verify(&mut self, dst: &mut [u8], src: &[u8], digest: &[u8]) -> Result<()> {
-        // SAFETY: This condition makes the unsafe calls below correct.
-        if dst.len() != src.len() {
-            return Err(
-                Error::InvalidArgument("src and dst need to be of the same length".into()).into(),
-            );
-        }
+    fn decrypt_verify(&mut self, dst: &mut [u8], src: &[u8]) -> Result<()> {
+        debug_assert!(src.len() >= self.digest_size());
+        debug_assert_eq!(dst.len() + self.digest_size(), src.len());
+
+        // Split src into ciphertext and tag.
+        let l = self.digest_size();
+        let ciphertext = &src[..src.len().saturating_sub(l)];
+        let tag = &src[src.len().saturating_sub(l)..];
 
         // SAFETY: Process completely one full chunk.  Since `update`
         // is not being called again with partial block info and the
         // cipher is finalized afterwards these two calls are safe.
-        let size = unsafe { self.ctx.cipher_update_unchecked(src, Some(dst))? };
-        self.ctx.set_tag(digest)?;
+        let size = unsafe {
+            self.ctx.cipher_update_unchecked(ciphertext, Some(dst))?
+        };
+        self.ctx.set_tag(tag)?;
         unsafe { self.ctx.cipher_final_unchecked(&mut dst[size..])? };
         Ok(())
     }
diff --git a/openpgp/src/crypto/backend/rust/aead.rs b/openpgp/src/crypto/backend/rust/aead.rs
index 1e4ca4c0..90b62a91 100644
--- a/openpgp/src/crypto/backend/rust/aead.rs
+++ b/openpgp/src/crypto/backend/rust/aead.rs
@@ -59,7 +59,7 @@ where
         Ok(())
     }
 
-    fn decrypt_verify(&mut self, _dst: &mut [u8], _src: &[u8], _digest: &[u8]) -> Result<()> {
+    fn decrypt_verify(&mut self, _dst: &mut [u8], _src: &[u8]) -> Result<()> {
         panic!("AEAD decryption called in the encryption context")
     }
 }
@@ -77,7 +77,14 @@ where
         panic!("AEAD encryption called in the decryption context")
     }
 
-    fn decrypt_verify(&mut self, dst: &mut [u8], src: &[u8], digest: &[u8]) -> Result<()> {
+    fn decrypt_verify(&mut self, dst: &mut [u8], src: &[u8]) -> Result<()> {
+        debug_assert_eq!(dst.len() + self.digest_size(), src.len());
+
+        // Split src into ciphertext and digest.
+        let l = self.digest_size();
+        let digest = &src[src.len().saturating_sub(l)..];
+        let src = &src[..src.len().saturating_sub(l)];
+
         let len = core::cmp::min(dst.len(), src.len());
         dst[..len].copy_from_slice(&src[..len]);
         self.decrypt_unauthenticated_hazmat(&mut dst[..len]);
diff --git a/openpgp/src/packet/skesk.rs b/openpgp/src/packet/skesk.rs
index 782511bb..6dab9d8a 100644
--- a/openpgp/src/packet/skesk.rs
+++ b/openpgp/src/packet/skesk.rs
@@ -502,7 +502,11 @@ impl SKESK5 {
                 CipherOp::Decrypt)?;
 
             let mut plain: SessionKey = vec![0; esk.len()].into();
-            cipher.decrypt_verify(&mut plain, esk, &self.aead_digest[..])?;
+            let mut chunk =
+                Vec::with_capacity(esk.len() + self.aead_digest.len());
+            chunk.extend_from_slice(esk);
+            chunk.extend_from_slice(&self.aead_digest);
+            cipher.decrypt_verify(&mut plain, &chunk)?;
             Ok((SymmetricAlgorithm::Unencrypted, plain))
         } else {
             Err(Error::MalformedPacket(