diff options
author | Justus Winter <justus@sequoia-pgp.org> | 2023-02-27 14:54:15 +0100 |
---|---|---|
committer | Justus Winter <justus@sequoia-pgp.org> | 2023-02-27 16:01:53 +0100 |
commit | 39bf91d6812f499327752803a6ecfada74a9b606 (patch) | |
tree | 7c0e16fc4ae7520fd9c687d1066b115e5e287cb5 | |
parent | b86ae96059f4560258babf4657ffbb426073457e (diff) |
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.
-rw-r--r-- | openpgp/src/crypto/aead.rs | 49 | ||||
-rw-r--r-- | openpgp/src/crypto/backend/cng/aead.rs | 21 | ||||
-rw-r--r-- | openpgp/src/crypto/backend/nettle/aead.rs | 8 | ||||
-rw-r--r-- | openpgp/src/crypto/backend/openssl/aead.rs | 30 | ||||
-rw-r--r-- | openpgp/src/crypto/backend/rust/aead.rs | 23 | ||||
-rw-r--r-- | openpgp/src/packet/skesk.rs | 15 |
6 files changed, 50 insertions, 96 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 { diff --git a/openpgp/src/packet/skesk.rs b/openpgp/src/packet/skesk.rs index 412c59c1..694f7496 100644 --- a/openpgp/src/packet/skesk.rs +++ b/openpgp/src/packet/skesk.rs @@ -465,12 +465,15 @@ impl SKESK5 { ctx.update(&ad)?; // Encrypt the session key with the KEK. - let mut esk = vec![0u8; session_key.len()]; - ctx.encrypt(&mut esk, session_key)?; - - // Digest. - let mut digest = vec![0u8; esk_aead.digest_size()?]; - ctx.digest(&mut digest)?; + let mut esk_digest = + vec![0u8; session_key.len() + esk_aead.digest_size()?]; + ctx.encrypt_seal(&mut esk_digest, session_key)?; + + let digest = esk_digest[session_key.len()..].to_vec(); + let esk = { + crate::vec_truncate(&mut esk_digest, session_key.len()); + esk_digest + }; SKESK5::new(esk_algo, esk_aead, s2k, iv.into_boxed_slice(), esk.into(), digest.into_boxed_slice()) |