openpgp: Rework the AEAD abstraction.

  - Combine `encrypt` and `tag` to `encrypt_seal` similarly to we
    previously combined `decrypt_verify`.  This better matches AEAD
    constructions, and the original interface was mostly informed by
    Nettle's relatively low-level interface.

5 files changed, 41 insertions, 90 deletions

diff --git a/openpgp/src/crypto/aead.rs b/openpgp/src/crypto/aead.rs
index 43a1aaad..7b9dc3fb 100644
--- a/openpgp/src/crypto/aead.rs
+++ b/openpgp/src/crypto/aead.rs
@@ -53,11 +53,11 @@ pub trait Aead : seal::Sealed {
     /// Adds associated data `ad`.
     fn update(&mut self, ad: &[u8]) -> Result<()>;
 
-    /// Encrypts one block `src` to `dst`.
-    fn encrypt(&mut self, dst: &mut [u8], src: &[u8]) -> Result<()>;
-
-    /// Produce the digest.
-    fn digest(&mut self, digest: &mut [u8]) -> Result<()>;
+    /// Encrypts one chunk `src` to `dst` adding a digest.
+    ///
+    /// Note: `dst` must be large enough to accommodate both the
+    /// ciphertext and the digest!
+    fn encrypt_seal(&mut self, dst: &mut [u8], src: &[u8]) -> Result<()>;
 
     /// Length of the digest in bytes.
     fn digest_size(&self) -> usize;
@@ -633,7 +633,7 @@ impl<W: io::Write, S: Schedule> Encryptor<W, S> {
             chunk_index: 0,
             bytes_encrypted: 0,
             buffer: Vec::with_capacity(chunk_size),
-            scratch: vec![0; chunk_size],
+            scratch: vec![0; chunk_size + aead.digest_size()?],
         })
     }
 
@@ -667,15 +667,12 @@ impl<W: io::Write, S: Schedule> Encryptor<W, S> {
                 let inner = self.inner.as_mut().unwrap();
 
                 // Encrypt the chunk.
-                aead.encrypt(&mut self.scratch, &self.buffer)?;
-                self.bytes_encrypted += self.scratch.len() as u64;
+                aead.encrypt_seal(&mut self.scratch, &self.buffer)?;
+                self.bytes_encrypted += self.chunk_size as u64;
                 self.chunk_index += 1;
+                // XXX: clear plaintext buffer.
                 crate::vec_truncate(&mut self.buffer, 0);
                 inner.write_all(&self.scratch)?;
-
-                // Write digest.
-                aead.digest(&mut self.scratch[..self.digest_size])?;
-                inner.write_all(&self.scratch[..self.digest_size])?;
             }
         }
 
@@ -696,14 +693,10 @@ impl<W: io::Write, S: Schedule> Encryptor<W, S> {
                 let inner = self.inner.as_mut().unwrap();
 
                 // Encrypt the chunk.
-                aead.encrypt(&mut self.scratch, chunk)?;
-                self.bytes_encrypted += self.scratch.len() as u64;
+                aead.encrypt_seal(&mut self.scratch, chunk)?;
+                self.bytes_encrypted += self.chunk_size as u64;
                 self.chunk_index += 1;
                 inner.write_all(&self.scratch)?;
-
-                // Write digest.
-                aead.digest(&mut self.scratch[..self.digest_size])?;
-                inner.write_all(&self.scratch[..self.digest_size])?;
             } else {
                 // Stash for later.
                 assert!(self.buffer.is_empty());
@@ -729,17 +722,19 @@ impl<W: io::Write, S: Schedule> Encryptor<W, S> {
                     })??;
 
                 // Encrypt the chunk.
-                unsafe { self.scratch.set_len(self.buffer.len()) }
-                aead.encrypt(&mut self.scratch, &self.buffer)?;
-                self.bytes_encrypted += self.scratch.len() as u64;
+                unsafe {
+                    // Safety: remaining data is less than the chunk
+                    // size.  The vector has capacity chunk size plus
+                    // digest size.
+                    debug_assert!(self.buffer.len() < self.chunk_size);
+                    self.scratch.set_len(self.buffer.len() + aead.digest_size())
+                }
+                aead.encrypt_seal(&mut self.scratch, &self.buffer)?;
+                self.bytes_encrypted += self.buffer.len() as u64;
                 self.chunk_index += 1;
+                // XXX: clear plaintext buffer
                 crate::vec_truncate(&mut self.buffer, 0);
                 inner.write_all(&self.scratch)?;
-
-                // Write digest.
-                unsafe { self.scratch.set_len(self.digest_size) }
-                aead.digest(&mut self.scratch[..self.digest_size])?;
-                inner.write_all(&self.scratch[..self.digest_size])?;
             }
 
             // Write final digest.
@@ -753,7 +748,7 @@ impl<W: io::Write, S: Schedule> Encryptor<W, S> {
                             Ok::<Box<dyn Aead>, anyhow::Error>(aead)
                         })
                 })??;
-            aead.digest(&mut self.scratch[..self.digest_size])?;
+            aead.encrypt_seal(&mut self.scratch[..self.digest_size], b"")?;
             inner.write_all(&self.scratch[..self.digest_size])?;
 
             Ok(inner)
diff --git a/openpgp/src/crypto/backend/cng/aead.rs b/openpgp/src/crypto/backend/cng/aead.rs
index 7389c1f0..afc6b7c9 100644
--- a/openpgp/src/crypto/backend/cng/aead.rs
+++ b/openpgp/src/crypto/backend/cng/aead.rs
@@ -96,15 +96,14 @@ macro_rules! impl_aead {
             fn digest_size(&self) -> usize {
                 <eax::Tag as GenericArrayExt<_, _>>::LEN
             }
-            fn digest(&mut self, digest: &mut [u8]) -> Result<()> {
-                let tag = self.tag_clone();
-                digest[..tag.len()].copy_from_slice(&tag[..]);
-                Ok(())
-            }
-            fn encrypt(&mut self, dst: &mut [u8], src: &[u8]) -> Result<()> {
+            fn encrypt_seal(&mut self, dst: &mut [u8], src: &[u8]) -> Result<()> {
+                debug_assert_eq!(dst.len(), src.len() + self.digest_size());
                 let len = core::cmp::min(dst.len(), src.len());
                 dst[..len].copy_from_slice(&src[..len]);
                 EaxOnline::<$type, Encrypt>::encrypt(self, &mut dst[..len]);
+
+                let tag = self.tag_clone();
+                dst[src.len()..].copy_from_slice(&tag[..]);
                 Ok(())
             }
             fn decrypt_verify(&mut self, _dst: &mut [u8], _src: &[u8], _digest: &[u8]) -> Result<()> {
@@ -122,21 +121,15 @@ macro_rules! impl_aead {
             fn digest_size(&self) -> usize {
                 <eax::Tag as GenericArrayExt<_, _>>::LEN
             }
-            fn digest(&mut self, digest: &mut [u8]) -> Result<()> {
-                let tag = self.tag_clone();
-                digest[..tag.len()].copy_from_slice(&tag[..]);
-                Ok(())
-            }
-            fn encrypt(&mut self, _dst: &mut [u8], _src: &[u8]) -> Result<()> {
+            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<()> {
                 let len = core::cmp::min(dst.len(), src.len());
                 dst[..len].copy_from_slice(&src[..len]);
                 self.decrypt_unauthenticated_hazmat(&mut dst[..len]);
-                let mut chunk_digest = vec![0u8; self.digest_size()];
 
-                self.digest(&mut chunk_digest)?;
+                let chunk_digest = self.tag_clone();
                 if secure_cmp(&chunk_digest[..], digest)
                     != Ordering::Equal && ! DANGER_DISABLE_AUTHENTICATION
                 {
diff --git a/openpgp/src/crypto/backend/nettle/aead.rs b/openpgp/src/crypto/backend/nettle/aead.rs
index 3cdbc42e..df2f0f64 100644
--- a/openpgp/src/crypto/backend/nettle/aead.rs
+++ b/openpgp/src/crypto/backend/nettle/aead.rs
@@ -22,8 +22,10 @@ impl<T: nettle::aead::Aead> Aead for T {
         self.update(ad);
         Ok(())
     }
-    fn encrypt(&mut self, dst: &mut [u8], src: &[u8]) -> Result<()> {
+    fn encrypt_seal(&mut self, dst: &mut [u8], src: &[u8]) -> Result<()> {
+        debug_assert_eq!(dst.len(), src.len() + self.digest_size());
         self.encrypt(dst, src);
+        self.digest(&mut dst[src.len()..]);
         Ok(())
     }
     fn decrypt_verify(&mut self, dst: &mut [u8], src: &[u8], digest: &[u8]) -> Result<()> {
@@ -38,10 +40,6 @@ impl<T: nettle::aead::Aead> Aead for T {
             }
         Ok(())
     }
-    fn digest(&mut self, digest: &mut [u8]) -> Result<()> {
-        self.digest(digest);
-        Ok(())
-    }
     fn digest_size(&self) -> usize {
         self.digest_size()
     }
diff --git a/openpgp/src/crypto/backend/openssl/aead.rs b/openpgp/src/crypto/backend/openssl/aead.rs
index f107a6fa..5e73ff0a 100644
--- a/openpgp/src/crypto/backend/openssl/aead.rs
+++ b/openpgp/src/crypto/backend/openssl/aead.rs
@@ -10,12 +10,6 @@ use openssl::cipher_ctx::CipherCtx;
 
 struct OpenSslContext {
     ctx: CipherCtx,
-    // The last chunk to be processed does not call `encrypt` thus
-    // leaves the crypter in non-finalized state.  This makes the
-    // `get_tag` function of the crypter panic when calling `digest`.
-    // If this flag is set to `false` it means the crypter needs to be
-    // finalized.
-    finalized: bool,
 }
 
 impl Aead for OpenSslContext {
@@ -24,20 +18,15 @@ impl Aead for OpenSslContext {
         Ok(())
     }
 
-    fn encrypt(&mut self, dst: &mut [u8], src: &[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 encrypt_seal(&mut self, dst: &mut [u8], src: &[u8]) -> Result<()> {
+        debug_assert_eq!(dst.len(), src.len() + self.digest_size());
 
         // 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))? };
         unsafe { self.ctx.cipher_final_unchecked(&mut dst[size..])? };
-        self.finalized = true;
+        self.ctx.tag(&mut dst[src.len()..])?;
         Ok(())
     }
 
@@ -58,18 +47,6 @@ impl Aead for OpenSslContext {
         Ok(())
     }
 
-    fn digest(&mut self, digest: &mut [u8]) -> Result<()> {
-        if !self.finalized {
-            // SAFETY: If we reach this point the `update` has not
-            // been called at all with chunk data (only with AAD data)
-            // `final` will not return any data but it must be called
-            // exactly once so that the `tag` function succeeds.
-            unsafe { self.ctx.cipher_final_unchecked(&mut [])? };
-        }
-        self.ctx.tag(digest)?;
-        Ok(())
-    }
-
     fn digest_size(&self) -> usize {
         self.ctx.block_size()
     }
@@ -114,7 +91,6 @@ impl AEADAlgorithm {
                 ctx.set_padding(false);
                 Ok(Box::new(OpenSslContext {
                     ctx,
-                    finalized: false,
                 }))
             }
             _ => Err(Error::UnsupportedAEADAlgorithm(*self).into()),
diff --git a/openpgp/src/crypto/backend/rust/aead.rs b/openpgp/src/crypto/backend/rust/aead.rs
index e58583a1..5c57e79a 100644
--- a/openpgp/src/crypto/backend/rust/aead.rs
+++ b/openpgp/src/crypto/backend/rust/aead.rs
@@ -54,16 +54,13 @@ where
         eax::Tag::LEN
     }
 
-    fn digest(&mut self, digest: &mut [u8]) -> Result<()> {
-        let tag = self.tag_clone();
-        digest[..tag.len()].copy_from_slice(&tag[..]);
-        Ok(())
-    }
-
-    fn encrypt(&mut self, dst: &mut [u8], src: &[u8]) -> Result<()> {
+    fn encrypt_seal(&mut self, dst: &mut [u8], src: &[u8]) -> Result<()> {
+        debug_assert_eq!(dst.len(), src.len() + self.digest_size());
         let len = cmp::min(dst.len(), src.len());
         dst[..len].copy_from_slice(&src[..len]);
         Self::encrypt(self, &mut dst[..len]);
+        let tag = self.tag_clone();
+        dst[src.len()..].copy_from_slice(&tag[..]);
         Ok(())
     }
 
@@ -86,13 +83,7 @@ where
         eax::Tag::LEN
     }
 
-    fn digest(&mut self, digest: &mut [u8]) -> Result<()> {
-        let tag = self.tag_clone();
-        digest[..tag.len()].copy_from_slice(&tag[..]);
-        Ok(())
-    }
-
-    fn encrypt(&mut self, _dst: &mut [u8], _src: &[u8]) -> Result<()> {
+    fn encrypt_seal(&mut self, _dst: &mut [u8], _src: &[u8]) -> Result<()> {
         panic!("AEAD encryption called in the decryption context")
     }
 
@@ -101,9 +92,7 @@ where
         dst[..len].copy_from_slice(&src[..len]);
         self.decrypt_unauthenticated_hazmat(&mut dst[..len]);
 
-        let mut chunk_digest = vec![0u8; self.digest_size()];
-
-        self.digest(&mut chunk_digest)?;
+        let chunk_digest = self.tag_clone();
         if secure_cmp(&chunk_digest[..], digest)
              != Ordering::Equal && ! DANGER_DISABLE_AUTHENTICATION
             {