openpgp: Change `decrypt` into `decrypt_verify`.

  - Some backends want to verify the AEAD block by themselves and need
    the tag to be passed in.

  - Change two step `decrypt` + `digest` into a one step `decrypt_verify`.

  - Old backends are modified to work like they did previously by
    utilizing decryption and the digest operation.

  - New backends can implement `decrypt_verify` using their respective
    cryptographic primitives.

5 files changed, 70 insertions, 46 deletions

diff --git a/openpgp/src/crypto/aead.rs b/openpgp/src/crypto/aead.rs
index fc147b41..9d43c1a5 100644
--- a/openpgp/src/crypto/aead.rs
+++ b/openpgp/src/crypto/aead.rs
@@ -15,7 +15,6 @@ use crate::utils::{
 use crate::Error;
 use crate::Result;
 use crate::crypto::SessionKey;
-use crate::crypto::mem::secure_cmp;
 use crate::seal;
 use crate::parse::Cookie;
 
@@ -32,12 +31,6 @@ const MAX_CHUNK_SIZE: usize = 1 << 22; // 4MiB
 /// Maximum size of any Nonce used by an AEAD mode.
 pub const MAX_NONCE_LEN: usize = 16;
 
-/// Disables authentication checks.
-///
-/// This is DANGEROUS, and is only useful for debugging problems with
-/// malformed AEAD-encrypted messages.
-const DANGER_DISABLE_AUTHENTICATION: bool = false;
-
 /// Converts a chunk size to a usize.
 pub(crate) fn chunk_size_usize(chunk_size: u64) -> Result<usize> {
     chunk_size.try_into()
@@ -63,13 +56,16 @@ pub trait Aead : seal::Sealed {
     /// Encrypts one block `src` to `dst`.
     fn encrypt(&mut self, dst: &mut [u8], src: &[u8]);
     /// Decrypts one block `src` to `dst`.
-    fn decrypt(&mut self, dst: &mut [u8], src: &[u8]);
 
     /// Produce the digest.
     fn digest(&mut self, digest: &mut [u8]);
 
     /// 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<()>;
 }
 
 /// Whether AEAD cipher is used for data encryption or decryption.
@@ -299,13 +295,8 @@ impl<'a, S: Schedule> Decryptor<'a, S> {
     // is less than `plaintext.len()`, then it is either because we
     // reached the end of the input or an error occurred.
     fn read_helper(&mut self, plaintext: &mut [u8]) -> Result<usize> {
-        use std::cmp::Ordering;
-
         let mut pos = 0;
 
-        // Buffer to hold a digest.
-        let mut digest = vec![0u8; self.digest_size];
-
         // 1. Copy any buffered data.
         if !self.buffer.is_empty() {
             let to_copy = cmp::min(self.buffer.len(), plaintext.len());
@@ -418,15 +409,7 @@ impl<'a, S: Schedule> Decryptor<'a, S> {
                     &mut plaintext[pos..pos + to_decrypt]
                 };
 
-                aead.decrypt(buffer, &chunk[..to_decrypt]);
-
-                // Check digest.
-                aead.digest(&mut digest);
-                if secure_cmp(&digest[..], &chunk[to_decrypt..])
-                    != Ordering::Equal && ! DANGER_DISABLE_AUTHENTICATION
-                {
-                    return Err(Error::ManipulatedMessage.into());
-                }
+                aead.decrypt_verify(buffer, &chunk[..to_decrypt], &chunk[to_decrypt..])?;
 
                 if double_buffer {
                     let to_copy = plaintext.len() - pos;
@@ -464,15 +447,9 @@ impl<'a, S: Schedule> Decryptor<'a, S> {
                             })
                     })?;
 
-                aead.digest(&mut digest);
-
                 let final_digest = self.source.data(final_digest_size)?;
-                if final_digest.len() != final_digest_size
-                    || secure_cmp(&digest[..], final_digest) != Ordering::Equal
-                    && ! DANGER_DISABLE_AUTHENTICATION
-                {
-                    return Err(Error::ManipulatedMessage.into());
-                }
+
+                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 a7b87613..fb95b150 100644
--- a/openpgp/src/crypto/backend/cng/aead.rs
+++ b/openpgp/src/crypto/backend/cng/aead.rs
@@ -1,7 +1,9 @@
 //! Implementation of AEAD using Windows CNG API.
+use std::cmp::Ordering;
 
 use crate::{Error, Result};
 use crate::crypto::aead::{Aead, CipherOp};
+use crate::crypto::mem::secure_cmp;
 use crate::seal;
 use crate::types::{AEADAlgorithm, SymmetricAlgorithm};
 
@@ -10,6 +12,12 @@ use win_crypto_ng::symmetric::{BlockCipherKey, Aes};
 use win_crypto_ng::symmetric::block_cipher::generic_array::{GenericArray, ArrayLength};
 use win_crypto_ng::symmetric::block_cipher::generic_array::typenum::{U128, U192, U256};
 
+/// Disables authentication checks.
+///
+/// This is DANGEROUS, and is only useful for debugging problems with
+/// malformed AEAD-encrypted messages.
+const DANGER_DISABLE_AUTHENTICATION: bool = false;
+
 trait GenericArrayExt<T, N: ArrayLength<T>> {
     const LEN: usize;
 
@@ -94,7 +102,7 @@ macro_rules! impl_aead {
                 dst[..len].copy_from_slice(&src[..len]);
                 EaxOnline::<$type, Encrypt>::encrypt(self, &mut dst[..len])
             }
-            fn decrypt(&mut self, _dst: &mut [u8], _src: &[u8]) {
+            fn decrypt_verify(&mut self, _dst: &mut [u8], _src: &[u8], _digest: &[u8]) -> Result<()> {
                 panic!("AEAD decryption called in the encryption context")
             }
         }
@@ -113,10 +121,19 @@ macro_rules! impl_aead {
             fn encrypt(&mut self, _dst: &mut [u8], _src: &[u8]) {
                 panic!("AEAD encryption called in the decryption context")
             }
-            fn decrypt(&mut self, dst: &mut [u8], src: &[u8]) {
+            fn decrypt_verify(&mut self, dst: &mut [u8], src: &[u8], digest: &[u8]) -> Result<()> {
                 let len = core::cmp::min(dst.len(), src.len());
                 dst[..len].copy_from_slice(&src[..len]);
-                self.decrypt_unauthenticated_hazmat(&mut dst[..len])
+                self.decrypt_unauthenticated_hazmat(&mut dst[..len]);
+                let mut chunk_digest = vec![0u8; self.digest_size()];
+
+                self.digest(&mut chunk_digest)?;
+                if secure_cmp(&chunk_digest[..], digest)
+                    != Ordering::Equal && ! DANGER_DISABLE_AUTHENTICATION
+                {
+                    return Err(Error::ManipulatedMessage.into());
+                }
+                Ok(())
             }
         }
         impl seal::Sealed for EaxOnline<$type, Decrypt> {}
diff --git a/openpgp/src/crypto/backend/nettle/aead.rs b/openpgp/src/crypto/backend/nettle/aead.rs
index e7ae9d85..000d3156 100644
--- a/openpgp/src/crypto/backend/nettle/aead.rs
+++ b/openpgp/src/crypto/backend/nettle/aead.rs
@@ -1,12 +1,21 @@
 //! Implementation of AEAD using Nettle cryptographic library.
+use std::cmp::Ordering;
+
 use nettle::{aead, cipher};
 
 use crate::{Error, Result};
 
 use crate::crypto::aead::{Aead, CipherOp};
+use crate::crypto::mem::secure_cmp;
 use crate::seal;
 use crate::types::{AEADAlgorithm, SymmetricAlgorithm};
 
+/// Disables authentication checks.
+///
+/// This is DANGEROUS, and is only useful for debugging problems with
+/// malformed AEAD-encrypted messages.
+const DANGER_DISABLE_AUTHENTICATION: bool = false;
+
 impl<T: nettle::aead::Aead> seal::Sealed for T {}
 impl<T: nettle::aead::Aead> Aead for T {
     fn update(&mut self, ad: &[u8]) {
@@ -15,8 +24,17 @@ impl<T: nettle::aead::Aead> Aead for T {
     fn encrypt(&mut self, dst: &mut [u8], src: &[u8]) {
         self.encrypt(dst, src)
     }
-    fn decrypt(&mut self, dst: &mut [u8], src: &[u8]) {
-        self.decrypt(dst, src)
+    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()];
+
+        self.digest(&mut chunk_digest);
+        if secure_cmp(&chunk_digest[..], digest)
+             != Ordering::Equal && ! DANGER_DISABLE_AUTHENTICATION
+            {
+                 return Err(Error::ManipulatedMessage.into());
+            }
+        Ok(())
     }
     fn digest(&mut self, digest: &mut [u8]) {
         self.digest(digest)
diff --git a/openpgp/src/crypto/backend/rust/aead.rs b/openpgp/src/crypto/backend/rust/aead.rs
index e5fcdb0b..f4dbc198 100644
--- a/openpgp/src/crypto/backend/rust/aead.rs
+++ b/openpgp/src/crypto/backend/rust/aead.rs
@@ -1,6 +1,7 @@
 //! Implementation of AEAD using pure Rust cryptographic libraries.
 
 use std::cmp;
+use std::cmp::Ordering;
 
 use cipher::{BlockCipher, NewBlockCipher};
 use cipher::block::Block;
@@ -10,9 +11,16 @@ use generic_array::{ArrayLength, GenericArray};
 
 use crate::{Error, Result};
 use crate::crypto::aead::{Aead, CipherOp};
+use crate::crypto::mem::secure_cmp;
 use crate::seal;
 use crate::types::{AEADAlgorithm, SymmetricAlgorithm};
 
+/// Disables authentication checks.
+///
+/// This is DANGEROUS, and is only useful for debugging problems with
+/// malformed AEAD-encrypted messages.
+const DANGER_DISABLE_AUTHENTICATION: bool = false;
+
 trait GenericArrayExt<T, N: ArrayLength<T>> {
     const LEN: usize;
 
@@ -56,7 +64,7 @@ where
         Self::encrypt(self, &mut dst[..len])
     }
 
-    fn decrypt(&mut self, _dst: &mut [u8], _src: &[u8]) {
+    fn decrypt_verify(&mut self, _dst: &mut [u8], _src: &[u8], _digest: &[u8]) -> Result<()> {
         panic!("AEAD decryption called in the encryption context")
     }
 }
@@ -83,10 +91,20 @@ where
         panic!("AEAD encryption called in the decryption context")
     }
 
-    fn decrypt(&mut self, dst: &mut [u8], src: &[u8]) {
+    fn decrypt_verify(&mut self, dst: &mut [u8], src: &[u8], digest: &[u8]) -> Result<()> {
         let len = core::cmp::min(dst.len(), src.len());
         dst[..len].copy_from_slice(&src[..len]);
-        self.decrypt_unauthenticated_hazmat(&mut dst[..len])
+        self.decrypt_unauthenticated_hazmat(&mut dst[..len]);
+
+        let mut chunk_digest = vec![0u8; self.digest_size()];
+
+        self.digest(&mut chunk_digest)?;
+        if secure_cmp(&chunk_digest[..], digest)
+             != Ordering::Equal && ! DANGER_DISABLE_AUTHENTICATION
+            {
+                 return Err(Error::ManipulatedMessage.into());
+            }
+        Ok(())
     }
 }
 
diff --git a/openpgp/src/packet/skesk.rs b/openpgp/src/packet/skesk.rs
index 96cb0119..fbbd0aac 100644
--- a/openpgp/src/packet/skesk.rs
+++ b/openpgp/src/packet/skesk.rs
@@ -501,14 +501,8 @@ impl SKESK5 {
                       self.aead_algo.into()];
             cipher.update(&ad);
             let mut plain: SessionKey = vec![0; esk.len()].into();
-            let mut digest = vec![0; self.aead_algo.digest_size()?];
-            cipher.decrypt(&mut plain, esk);
-            cipher.digest(&mut digest);
-            if digest[..] == self.aead_digest[..] {
-                Ok((SymmetricAlgorithm::Unencrypted, plain))
-            } else {
-                Err(Error::ManipulatedMessage.into())
-            }
+            cipher.decrypt_verify(&mut plain, esk, &self.aead_digest[..])?;
+            Ok((SymmetricAlgorithm::Unencrypted, plain))
         } else {
             Err(Error::MalformedPacket(
                 "No encrypted session key in v5 SKESK packet".into())