summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--openpgp/src/crypto/aead.rs352
-rw-r--r--openpgp/src/parse/parse.rs3
-rw-r--r--openpgp/src/serialize/stream.rs124
3 files changed, 260 insertions, 219 deletions
diff --git a/openpgp/src/crypto/aead.rs b/openpgp/src/crypto/aead.rs
index e27f493d..f5b8424e 100644
--- a/openpgp/src/crypto/aead.rs
+++ b/openpgp/src/crypto/aead.rs
@@ -1,6 +1,6 @@
use std::cmp;
use std::fmt;
-use std::io::{self, Read};
+use std::io;
use nettle::{aead, cipher};
use buffered_reader::BufferedReader;
@@ -198,221 +198,187 @@ impl<'a> Decryptor<'a> {
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.len() > 0 {
let to_copy = cmp::min(self.buffer.len(), plaintext.len());
&plaintext[..to_copy].copy_from_slice(&self.buffer[..to_copy]);
self.buffer.drain(..to_copy);
- pos = to_copy;
- }
- if pos == plaintext.len() {
- return Ok(pos);
+ pos = to_copy;
+ if pos == plaintext.len() {
+ return Ok(pos);
+ }
}
- // 2. Decrypt as many whole chunks as `plaintext` can hold.
- let n_chunks = (plaintext.len() - pos) / self.chunk_size;
+ // 2. Decrypt the data a chunk at a time until we've filled
+ // `plaintext`.
+ //
+ // Unfortunately, framing is hard.
+ //
+ // Recall: AEAD data is of the form:
+ //
+ // [ chunk1 ][ tag1 ] ... [ chunkN ][ tagN ][ tagF ]
+ //
+ // And, all chunks are the same size except for the last
+ // chunk, which may be shorter.
+ //
+ // The naive approach to decryption is to read a chunk and a
+ // tag at a time. Unfortunately, this may not work if the
+ // last chunk is a partial chunk.
+ //
+ // Assume that the chunk size is 32 bytes and the digest size
+ // is 16 bytes, and consider a message with 17 bytes of data.
+ // That message will be encrypted as follows:
+ //
+ // [ chunk1 ][ tag1 ][ tagF ]
+ // 17B 16B 16B
+ //
+ // If we read a chunk and a digest, we'll successfully read 48
+ // bytes of data. Unfortunately, we'll have over read: the
+ // last 15 bytes are from the final tag.
+ //
+ // To correctly handle this case, we have to make sure that
+ // there are at least a tag worth of bytes left over when we
+ // read a chunk and a tag.
+
+ let n_chunks
+ = (plaintext.len() - pos + self.chunk_size - 1) / self.chunk_size;
let chunk_digest_size = self.chunk_size + self.digest_size;
- let mut to_copy = n_chunks * self.chunk_size;
- let to_read = n_chunks * chunk_digest_size;
-
- let mut ciphertext = Vec::new();
- let result = (&mut self.source).take(to_read as u64)
- .read_to_end(&mut ciphertext);
- let short_read;
- match result {
- Ok(amount) => {
- if to_read != 0 && amount == 0 {
- // Exhausted source.
- return Ok(pos);
- }
-
- // Recall: AEAD data is of the form:
- //
- // [ chunk1 ][ tag1 ] ... [ chunkN ][ tagN ][ tag ]
- //
- // And, all chunks are the same size except for the
- // last chunk, which may be shorter.
- //
- // Because only the last chunk can be shorter, if the
- // amount read is less than `chunk_size + tag_size`,
- // then we know that we've read the last chunk.
- //
- // Unfortunately, this is not sufficient: if the last
- // chunk is `chunk_size - tag size` bytes large, then
- // when we read it, we'll read `chunk_size + tag_size`
- // bytes, because we'll have also read the final tag!
- //
- // We can detect this by also checking for EOF.
- short_read = amount < to_copy
- || (amount > 0 && self.source.eof());
- to_copy = amount;
- ciphertext.truncate(to_copy);
- },
- // We encountered an error, but we did read some.
- Err(_) if pos > 0 => return Ok(pos),
- Err(e) => return Err(e.into()),
- }
-
- // Buffer to hold digests.
- let mut digest = vec![0u8; self.digest_size];
+ let final_digest_size = self.digest_size;
- // At the end of the stream, there is an additional tag. Be
- // careful not to consume this tag.
- let ciphertext_end = if short_read {
- ciphertext.len() - self.digest_size
- } else {
- ciphertext.len()
- };
-
- for chunk in (&ciphertext[..ciphertext_end]).chunks(chunk_digest_size) {
+ for _ in 0..n_chunks {
let mut aead = self.make_aead()?;
-
// Digest the associated data.
self.hash_associated_data(&mut aead, false);
- // Decrypt the chunk.
- aead.decrypt(
- &mut plaintext[pos..pos + chunk.len() - self.digest_size],
- &chunk[..chunk.len() - self.digest_size]);
- self.bytes_decrypted += (chunk.len() - self.digest_size) as u64;
-
- // Check digest.
- aead.digest(&mut digest);
- let dig_ord = secure_cmp(&digest[..],
- &chunk[chunk.len() - self.digest_size..]);
- if dig_ord != Ordering::Equal {
- return Err(Error::ManipulatedMessage.into());
- }
-
- // Increase index, update position in plaintext.
- self.chunk_index += 1;
- pos += chunk.len() - self.digest_size;
- }
-
- if short_read {
- // We read the whole ciphertext, now check the final digest.
- let mut aead = self.make_aead()?;
- self.hash_associated_data(&mut aead, true);
-
- let mut nada = [0; 0];
- aead.decrypt(&mut nada, b"");
- aead.digest(&mut digest);
-
- let dig_ord = secure_cmp(&digest[..], &ciphertext[ciphertext_end..]);
- if dig_ord != Ordering::Equal {
- return Err(Error::ManipulatedMessage.into());
- }
- }
-
- if short_read || pos == plaintext.len() {
- return Ok(pos);
- }
-
- // 3. The last bit is a partial chunk. Buffer it.
- let mut to_copy = plaintext.len() - pos;
- assert!(0 < to_copy);
- assert!(to_copy < self.chunk_size);
-
- let mut ciphertext = Vec::new();
- let result = (&mut self.source).take(chunk_digest_size as u64)
- .read_to_end(&mut ciphertext);
- let short_read;
- match result {
- Ok(amount) => {
- if amount == 0 {
+ // Do a little dance to avoid exclusively locking
+ // `self.source`.
+ let to_read = chunk_digest_size + final_digest_size;
+ let result = {
+ match self.source.data(to_read) {
+ Ok(_) => Ok(self.source.buffer()),
+ Err(err) => Err(err),
+ }
+ };
+
+ let check_final_tag;
+ let chunk = match result {
+ Ok(chunk) => {
+ if chunk.len() == 0 {
+ // Exhausted source.
+ return Ok(pos);
+ }
+
+ if chunk.len() < final_digest_size {
+ if pos > 0 {
+ return Ok(pos);
+ } else {
+ return Err(Error::ManipulatedMessage.into());
+ }
+ }
+
+ check_final_tag = chunk.len() < to_read;
+
+ // Return the chunk.
+ &chunk[..cmp::min(chunk.len(), to_read) - final_digest_size]
+ },
+ // We encountered an error, but we've already read
+ // something, so return that.
+ Err(_) if pos > 0 => return Ok(pos),
+ Err(e) => return Err(e.into()),
+ };
+
+ assert!(chunk.len() <= chunk_digest_size);
+
+ if chunk.len() == 0 {
+ // There is nothing to decrypt: all that is left is
+ // the final tag.
+ } else if chunk.len() <= self.digest_size {
+ // A chunk has to include at least one byte and a tag.
+ if pos > 0 {
return Ok(pos);
+ } else {
+ return Err(Error::ManipulatedMessage.into());
+ }
+ } else {
+ // Decrypt the chunk and check the tag.
+ let to_decrypt = chunk.len() - self.digest_size;
+
+ // If plaintext doesn't have enough room for the whole
+ // chunk, then we have to double buffer.
+ let double_buffer = to_decrypt > plaintext.len() - pos;
+ let buffer = if double_buffer {
+ while self.buffer.len() < to_decrypt {
+ self.buffer.push(0u8);
+ }
+ &mut self.buffer[..]
+ } else {
+ &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
+ {
+ if pos > 0 {
+ return Ok(pos);
+ } else {
+ return Err(Error::ManipulatedMessage.into());
+ }
}
- // Recall: AEAD data is of the form:
- //
- // [ chunk1 ][ tag1 ] ... [ chunkN ][ tagN ][ tag ]
- //
- // And, all chunks are the same size except for the
- // last chunk, which may be shorter.
- //
- // Because only the last chunk can be shorter, if the
- // amount read is less than `chunk_size + tag_size`,
- // then we know that we've read the last chunk.
- //
- // Unfortunately, this is not sufficient: if the last
- // chunk is `chunk_size - tag size` bytes large, then
- // when we read it, we'll read `chunk_size + tag_size`
- // bytes, because we'll have also read the final tag!
- //
- // We can detect this by also checking for EOF.
- short_read = amount < chunk_digest_size || self.source.eof();
-
- // Make sure `ciphertext` is not larger than the
- // amount of data that was actually read.
- ciphertext.truncate(amount);
-
- // Make sure we don't read more than is available.
- to_copy = cmp::min(to_copy,
- ciphertext.len() - self.digest_size
- - if short_read {
- self.digest_size
- } else {
- 0
- });
- },
- // We encountered an error, but we did read some.
- Err(_) if pos > 0 => return Ok(pos),
- Err(e) => return Err(e.into()),
- }
- assert!(ciphertext.len() <= self.chunk_size + self.digest_size);
-
- let mut aead = self.make_aead()?;
-
- // Digest the associated data.
- self.hash_associated_data(&mut aead, false);
-
- // At the end of the stream, there is an additional tag. Be
- // careful not to consume this tag.
- let ciphertext_end = if short_read {
- ciphertext.len() - self.digest_size
- } else {
- ciphertext.len()
- };
+ if double_buffer {
+ let to_copy = plaintext.len() - pos;
+ assert!(0 < to_copy);
+ assert!(to_copy < self.chunk_size);
+
+ &plaintext[pos..pos + to_copy]
+ .copy_from_slice(&self.buffer[..to_copy]);
+ self.buffer.drain(..to_copy);
+ pos += to_copy;
+ } else {
+ pos += to_decrypt;
+ }
- while self.buffer.len() < ciphertext_end - self.digest_size {
- self.buffer.push(0u8);
- }
- self.buffer.truncate(ciphertext_end - self.digest_size);
-
- // Decrypt the chunk.
- aead.decrypt(&mut self.buffer,
- &ciphertext[..ciphertext_end - self.digest_size]);
- self.bytes_decrypted += (ciphertext_end - self.digest_size) as u64;
-
- // Check digest.
- aead.digest(&mut digest);
- let mac_ord = secure_cmp(
- &digest[..],
- &ciphertext[ciphertext_end - self.digest_size..ciphertext_end]);
- if mac_ord != Ordering::Equal {
- return Err(Error::ManipulatedMessage.into());
- }
+ // Increase index, update position in plaintext.
+ self.chunk_index += 1;
+ self.bytes_decrypted += to_decrypt as u64;
- // Increase index.
- self.chunk_index += 1;
+ // Consume the data only on success so that we keep
+ // returning the error.
+ self.source.consume(chunk.len());
+ }
- &plaintext[pos..pos + to_copy].copy_from_slice(&self.buffer[..to_copy]);
- self.buffer.drain(..to_copy);
- pos += to_copy;
+ if check_final_tag {
+ // We read the whole ciphertext, now check the final digest.
+ let mut aead = self.make_aead()?;
+ self.hash_associated_data(&mut aead, true);
- if short_read {
- // We read the whole ciphertext, now check the final digest.
- let mut aead = self.make_aead()?;
- self.hash_associated_data(&mut aead, true);
+ let mut nada = [0; 0];
+ aead.decrypt(&mut nada, b"");
+ aead.digest(&mut digest);
- let mut nada = [0; 0];
- aead.decrypt(&mut nada, b"");
- 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
+ {
+ if pos > 0 {
+ return Ok(pos);
+ } else {
+ return Err(Error::ManipulatedMessage.into());
+ }
+ }
- let dig_ord = secure_cmp(&digest[..], &ciphertext[ciphertext_end..]);
- if dig_ord != Ordering::Equal {
- return Err(Error::ManipulatedMessage.into());
+ // Consume the data only on success so that we keep
+ // returning the error.
+ self.source.consume(final_digest_size);
+ break;
}
}
diff --git a/openpgp/src/parse/parse.rs b/openpgp/src/parse/parse.rs
index 2883ef3c..5a5a356e 100644
--- a/openpgp/src/parse/parse.rs
+++ b/openpgp/src/parse/parse.rs
@@ -3619,7 +3619,8 @@ impl<'a> PacketParser<'a> {
// `aead::Decryptor` won't see EOF and think that
// it has a partial block and it needs to verify
// the final chunk.
- let amount = aed.chunk_digest_size()? + 1;
+ let amount
+ = aed.chunk_digest_size()? + aed.aead().digest_size()?;
let data = self.data(amount)?;
let dec = aead::Decryptor::new(
1, aed.symmetric_algo(), aed.aead(), aed.chunk_size(),
diff --git a/openpgp/src/serialize/stream.rs b/openpgp/src/serialize/stream.rs
index cd4ebd15..aa8b30a5 100644
--- a/openpgp/src/serialize/stream.rs
+++ b/openpgp/src/serialize/stream.rs
@@ -1551,7 +1551,7 @@ mod test {
}
#[test]
- fn aead_short_read() {
+ fn aead_messages() {
// AEAD data is of the form:
//
// [ chunk1 ][ tag1 ] ... [ chunkN ][ tagN ][ tag ]
@@ -1571,9 +1571,10 @@ mod test {
//
// Make sure we handle this situation correctly.
+ use std::cmp;
+
use crate::constants::KeyFlags;
use crate::parse::{
- Parse,
stream::{
Decryptor,
DecryptionHelper,
@@ -1581,9 +1582,13 @@ mod test {
MessageStructure,
},
};
+ use crate::tpk::{TPKBuilder, CipherSuite};
+ use crate::serialize::stream::{LiteralWriter, Message};
- let tsk =
- TPK::from_bytes(crate::tests::key("testy-private.pgp")).unwrap();
+ let (tsk, _) = TPKBuilder::new()
+ .set_cipher_suite(CipherSuite::Cv25519)
+ .add_encryption_subkey()
+ .generate().unwrap();
// Build a vector of recipients to hand to Encryptor.
let recipients =
@@ -1594,21 +1599,10 @@ mod test {
.map(|(_, _, key)| key.into())
.collect::<Vec<_>>();
- let mut o = vec![];
- {
- let m = Message::new(&mut o);
- let encryptor = Encryptor::new(
- m, &[], &recipients, None, AEADAlgorithm::EAX)
- .unwrap();
- let mut literal = LiteralWriter::new(encryptor, None, None, None)
- .unwrap();
- literal.write_all(&vec![0; 4071]).unwrap();
- }
-
- struct Helper {
- tsk: TPK,
+ struct Helper<'a> {
+ tsk: &'a TPK,
};
- impl VerificationHelper for Helper {
+ impl<'a> VerificationHelper for Helper<'a> {
fn get_public_keys(&mut self, _ids: &[KeyID]) -> Result<Vec<TPK>> {
Ok(Vec::new())
}
@@ -1616,7 +1610,7 @@ mod test {
Ok(())
}
}
- impl DecryptionHelper for Helper {
+ impl<'a> DecryptionHelper for Helper<'a> {
fn decrypt<D>(&mut self, pkesks: &[PKESK], _skesks: &[SKESK],
mut decrypt: D) -> Result<Option<crate::Fingerprint>>
where D: FnMut(SymmetricAlgorithm, &SessionKey) -> Result<()>
@@ -1624,7 +1618,6 @@ mod test {
let mut keypair = self.tsk.keys_all()
.key_flags(
KeyFlags::default()
- .set_encrypt_at_rest(true)
.set_encrypt_for_transport(true))
.map(|(_, _, key)| key).next().unwrap()
.clone().mark_parts_secret().into_keypair().unwrap();
@@ -1634,10 +1627,91 @@ mod test {
}
}
- let h = Helper { tsk };
- let mut v = Decryptor::from_bytes(&o, h, None).unwrap();
- let mut content = Vec::new();
- v.read_to_end(&mut content).unwrap();
- assert_eq!(content.len(), 4071);
+ for chunks in 0..3 {
+ for msg_len in
+ cmp::max(24, chunks * Encryptor::AEAD_CHUNK_SIZE) - 24
+ ..chunks * Encryptor::AEAD_CHUNK_SIZE + 24
+ {
+ eprintln!("Encrypting message of size: {}", msg_len);
+
+ let mut content : Vec<u8> = Vec::new();
+ for i in 0..msg_len {
+ content.push(b'0' + ((i % 10) as u8));
+ }
+
+ let mut msg = vec![];
+ {
+ let m = Message::new(&mut msg);
+ let encryptor = Encryptor::new(
+ m, &[], &recipients, None, AEADAlgorithm::EAX)
+ .unwrap();
+ let mut literal = LiteralWriter::new(encryptor, None, None, None)
+ .unwrap();
+ literal.write_all(&content).unwrap();
+ // literal.finalize().unwrap();
+ }
+
+ for &read_len in [
+ 37,
+ Encryptor::AEAD_CHUNK_SIZE - 1,
+ Encryptor::AEAD_CHUNK_SIZE,
+ 100 * Encryptor::AEAD_CHUNK_SIZE
+ ].into_iter() {
+ for &do_err in [ false, true ].into_iter() {
+ let mut msg = msg.clone();
+ if do_err {
+ let l = msg.len() - 1;
+ if msg[l] == 0 {
+ msg[l] = 1;
+ } else {
+ msg[l] = 0;
+ }
+ }
+
+ let h = Helper { tsk: &tsk };
+ // Note: a corrupted message is only guaranteed
+ // to error out before it returns EOF.
+ let mut v = match Decryptor::from_bytes(&msg, h, None) {
+ Ok(v) => v,
+ Err(_) if do_err => continue,
+ Err(err) => panic!("Decrypting message: {}", err),
+ };
+
+ let mut buffer = Vec::new();
+ while buffer.len() < read_len {
+ buffer.push(0);
+ }
+
+ let mut decrypted_content = Vec::new();
+ loop {
+ match v.read(&mut buffer[..read_len]) {
+ Ok(0) if do_err =>
+ panic!("Expected an error, got EOF"),
+ Ok(0) => break,
+ Ok(len) =>
+ decrypted_content.extend_from_slice(
+ &buffer[..len]),
+ Err(_) if do_err => break,
+ Err(err) =>
+ panic!("Decrypting data: {:?}", err),
+ }
+ }
+
+ if do_err {
+ // If we get an error once, we should get
+ // one again.
+ for _ in 0..3 {
+ assert!(v.read(&mut buffer[..read_len]).is_err());
+ }
+ }
+
+ // We only corrupted the final tag, so we
+ // should get all of the content.
+ assert_eq!(msg_len, decrypted_content.len());
+ assert_eq!(content, decrypted_content);
+ }
+ }
+ }
+ }
}
}