From a265ba7fa59d5a6c0ba59d8f3bb0d9eb4f70ac06 Mon Sep 17 00:00:00 2001 From: Marc Brinkmann Date: Sat, 3 Oct 2020 16:07:00 +0200 Subject: [PATCH 01/37] Create initial `aes_ctr` module --- Cargo.toml | 3 ++ src/aes_ctr.rs | 134 +++++++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 2 + 3 files changed, 139 insertions(+) create mode 100644 src/aes_ctr.rs diff --git a/Cargo.toml b/Cargo.toml index 2ce7e602..026672a4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,6 +11,8 @@ Library to support the reading and writing of zip files. edition = "2018" [dependencies] +aes = { version = "0.5.0", optional = true } +arrayvec = { version = "0.5.1", optional = true } flate2 = { version = "1.0.0", default-features = false, optional = true } time = { version = "0.1", optional = true } byteorder = "1.3" @@ -24,6 +26,7 @@ rand = "0.7" walkdir = "2" [features] +aes-crypto = ["aes", "arrayvec"] deflate = ["flate2/rust_backend"] deflate-miniz = ["flate2/default"] deflate-zlib = ["flate2/zlib"] diff --git a/src/aes_ctr.rs b/src/aes_ctr.rs new file mode 100644 index 00000000..3ab74725 --- /dev/null +++ b/src/aes_ctr.rs @@ -0,0 +1,134 @@ +use aes::block_cipher::generic_array::GenericArray; +use aes::{BlockCipher, NewBlockCipher}; +use arrayvec::{Array, ArrayVec}; +use std::{any, fmt, io}; + +/// AES-128. +#[derive(Debug)] +pub struct Aes128; +/// AES-192 +#[derive(Debug)] +pub struct Aes192; +/// AES-256. +#[derive(Debug)] +pub struct Aes256; + +/// An AES cipher kind. +pub trait AesKind { + /// Key type. + type Key: Array; + /// Cipher used to decrypt. + type Cipher; +} + +impl AesKind for Aes256 { + type Key = [u8; 32]; + + type Cipher = aes::Aes256; +} + +/// An AES-CTR key stream generator. +/// +/// Implements the slightly non-standard AES-CTR variant used by WinZip AES encryption. +/// +/// Typical AES-CTR implementations combine a nonce with a 64 bit counter. WinZIP AES instead uses +/// no nonce and also uses a different byte order (little endian) than NIST (big endian). +/// +/// The stream implements the `Read` trait; encryption or decryption is performed by XOR-ing the +/// bytes from the key stream with the ciphertext/plaintext. +struct AesCtrZipKeyStream { + counter: u128, + cipher: C::Cipher, + buffer: ArrayVec, +} + +impl fmt::Debug for AesCtrZipKeyStream +where + C: AesKind, +{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "AesCtrZipKeyStream<{}>(counter: {})", + any::type_name::(), + self.counter + ) + } +} + +impl AesCtrZipKeyStream +where + C: AesKind, + C::Cipher: NewBlockCipher, +{ + #[allow(dead_code)] + /// Creates a new zip variant AES-CTR key stream. + pub fn new(key: &C::Key) -> AesCtrZipKeyStream { + AesCtrZipKeyStream { + counter: 1, + cipher: C::Cipher::new_varkey(key.as_slice()).expect("key should have correct size"), + buffer: ArrayVec::new(), + } + } +} + +impl io::Read for AesCtrZipKeyStream +where + C: AesKind, + C::Cipher: BlockCipher, +{ + fn read(&mut self, buf: &mut [u8]) -> std::io::Result { + if self.buffer.len() == 0 { + // Note: AES block size is always 16 bytes, same as u128. + let mut block = GenericArray::clone_from_slice(&self.counter.to_le_bytes()); + self.cipher.encrypt_block(&mut block); + self.counter += 1; + self.buffer = block.into_iter().collect(); + } + + let target_len = buf.len().min(self.buffer.len()); + + buf.copy_from_slice(&self.buffer[0..target_len]); + self.buffer.drain(0..target_len); + Ok(target_len) + } +} + +#[cfg(test)] +/// XORs a slice in place with another slice. +#[inline] +pub fn xor(dest: &mut [u8], src: &[u8]) { + for (lhs, rhs) in dest.iter_mut().zip(src.iter()) { + *lhs ^= *rhs; + } +} + +#[cfg(test)] +mod tests { + use super::{xor, Aes256, AesCtrZipKeyStream}; + use std::io::Read; + + #[test] + fn simple_example() { + let ciphertext: [u8; 5] = [0xdc, 0x99, 0x93, 0x5e, 0xbf]; + let expected_plaintext = &[b'a', b's', b'd', b'f', b'\n']; + let key = [ + 0xd1, 0x51, 0xa6, 0xab, 0x53, 0x68, 0xd7, 0xb7, 0xbf, 0x49, 0xf7, 0xf5, 0x8a, 0x4e, + 0x10, 0x36, 0x25, 0x1c, 0x13, 0xba, 0x12, 0x45, 0x37, 0x65, 0xa9, 0xe4, 0xed, 0x9f, + 0x4a, 0xa8, 0xda, 0x3b, + ]; + + let mut key_stream = AesCtrZipKeyStream::::new(&key); + + let mut key_buf = [0u8; 5]; + key_stream.read(&mut key_buf).unwrap(); + + let mut plaintext = ciphertext; + eprintln!("{:?}", plaintext); + + xor(&mut plaintext, &key_buf); + eprintln!("{:?}", plaintext); + + assert_eq!(&plaintext, expected_plaintext); + } +} diff --git a/src/lib.rs b/src/lib.rs index 3b39ab4f..df398ce7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -10,6 +10,8 @@ pub use crate::read::ZipArchive; pub use crate::types::DateTime; pub use crate::write::ZipWriter; +#[cfg(feature = "aes-crypto")] +mod aes_ctr; mod compression; mod cp437; mod crc32; From 9f6ee0f4b6224051fff6209e2f3b9d56dfbf3c96 Mon Sep 17 00:00:00 2001 From: Marc Brinkmann Date: Sat, 3 Oct 2020 16:24:22 +0200 Subject: [PATCH 02/37] Add `crypt` convenience function --- src/aes_ctr.rs | 53 ++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 49 insertions(+), 4 deletions(-) diff --git a/src/aes_ctr.rs b/src/aes_ctr.rs index 3ab74725..8da8b4ad 100644 --- a/src/aes_ctr.rs +++ b/src/aes_ctr.rs @@ -1,8 +1,12 @@ use aes::block_cipher::generic_array::GenericArray; use aes::{BlockCipher, NewBlockCipher}; use arrayvec::{Array, ArrayVec}; +use std::io::Read; use std::{any, fmt, io}; +/// Internal block size of an AES cipher. +const AES_BLOCK_SIZE: usize = 16; + /// AES-128. #[derive(Debug)] pub struct Aes128; @@ -72,11 +76,38 @@ where } } +impl AesCtrZipKeyStream +where + C: AesKind, + C::Cipher: BlockCipher, +{ + /// Decrypt or encrypt given data. + pub fn crypt(&mut self, mut data: &mut [u8]) { + while data.len() > 0 { + let mut buffer: [u8; AES_BLOCK_SIZE] = [0u8; AES_BLOCK_SIZE]; + + let target_len = data.len().min(AES_BLOCK_SIZE); + + // Fill buffer with enough data to decrypt the next block. + debug_assert_eq!( + self.read(&mut buffer[0..target_len]) + .expect("reading key stream should never fail"), + target_len + ); + + xor(&mut data[0..target_len], &buffer.as_slice()[0..target_len]); + + data = &mut data[target_len..]; + } + } +} + impl io::Read for AesCtrZipKeyStream where C: AesKind, C::Cipher: BlockCipher, { + #[inline] fn read(&mut self, buf: &mut [u8]) -> std::io::Result { if self.buffer.len() == 0 { // Note: AES block size is always 16 bytes, same as u128. @@ -94,10 +125,11 @@ where } } -#[cfg(test)] /// XORs a slice in place with another slice. #[inline] pub fn xor(dest: &mut [u8], src: &[u8]) { + debug_assert_eq!(dest.len(), src.len()); + for (lhs, rhs) in dest.iter_mut().zip(src.iter()) { *lhs ^= *rhs; } @@ -124,11 +156,24 @@ mod tests { key_stream.read(&mut key_buf).unwrap(); let mut plaintext = ciphertext; - eprintln!("{:?}", plaintext); - xor(&mut plaintext, &key_buf); - eprintln!("{:?}", plaintext); + assert_eq!(&plaintext, expected_plaintext); + } + #[test] + fn crypt_simple_example() { + let ciphertext: [u8; 5] = [0xdc, 0x99, 0x93, 0x5e, 0xbf]; + let expected_plaintext = &[b'a', b's', b'd', b'f', b'\n']; + let key = [ + 0xd1, 0x51, 0xa6, 0xab, 0x53, 0x68, 0xd7, 0xb7, 0xbf, 0x49, 0xf7, 0xf5, 0x8a, 0x4e, + 0x10, 0x36, 0x25, 0x1c, 0x13, 0xba, 0x12, 0x45, 0x37, 0x65, 0xa9, 0xe4, 0xed, 0x9f, + 0x4a, 0xa8, 0xda, 0x3b, + ]; + + let mut key_stream = AesCtrZipKeyStream::::new(&key); + + let mut plaintext = ciphertext; + key_stream.crypt(&mut plaintext); assert_eq!(&plaintext, expected_plaintext); } } From a5d1905db6f5d3287821d38413a05576199702ba Mon Sep 17 00:00:00 2001 From: Marc Brinkmann Date: Sat, 3 Oct 2020 16:28:59 +0200 Subject: [PATCH 03/37] Simpify `aes_ctr` API to just `crypt` --- src/aes_ctr.rs | 75 ++++++++++++-------------------------------------- 1 file changed, 17 insertions(+), 58 deletions(-) diff --git a/src/aes_ctr.rs b/src/aes_ctr.rs index 8da8b4ad..2d1a4f5e 100644 --- a/src/aes_ctr.rs +++ b/src/aes_ctr.rs @@ -82,46 +82,25 @@ where C::Cipher: BlockCipher, { /// Decrypt or encrypt given data. - pub fn crypt(&mut self, mut data: &mut [u8]) { - while data.len() > 0 { - let mut buffer: [u8; AES_BLOCK_SIZE] = [0u8; AES_BLOCK_SIZE]; - - let target_len = data.len().min(AES_BLOCK_SIZE); - - // Fill buffer with enough data to decrypt the next block. - debug_assert_eq!( - self.read(&mut buffer[0..target_len]) - .expect("reading key stream should never fail"), - target_len - ); - - xor(&mut data[0..target_len], &buffer.as_slice()[0..target_len]); - - data = &mut data[target_len..]; - } - } -} - -impl io::Read for AesCtrZipKeyStream -where - C: AesKind, - C::Cipher: BlockCipher, -{ #[inline] - fn read(&mut self, buf: &mut [u8]) -> std::io::Result { - if self.buffer.len() == 0 { - // Note: AES block size is always 16 bytes, same as u128. - let mut block = GenericArray::clone_from_slice(&self.counter.to_le_bytes()); - self.cipher.encrypt_block(&mut block); - self.counter += 1; - self.buffer = block.into_iter().collect(); + fn crypt(&mut self, mut target: &mut [u8]) { + while target.len() > 0 { + if self.buffer.len() == 0 { + // Note: AES block size is always 16 bytes, same as u128. + let mut block = GenericArray::clone_from_slice(&self.counter.to_le_bytes()); + + // TODO: Use trait. + self.cipher.encrypt_block(&mut block); + self.counter += 1; + self.buffer = block.into_iter().collect(); + } + + let target_len = target.len().min(self.buffer.len()); + + xor(&mut target[0..target_len], &self.buffer[0..target_len]); + self.buffer.drain(0..target_len); + target = &mut target[target_len..]; } - - let target_len = buf.len().min(self.buffer.len()); - - buf.copy_from_slice(&self.buffer[0..target_len]); - self.buffer.drain(0..target_len); - Ok(target_len) } } @@ -140,26 +119,6 @@ mod tests { use super::{xor, Aes256, AesCtrZipKeyStream}; use std::io::Read; - #[test] - fn simple_example() { - let ciphertext: [u8; 5] = [0xdc, 0x99, 0x93, 0x5e, 0xbf]; - let expected_plaintext = &[b'a', b's', b'd', b'f', b'\n']; - let key = [ - 0xd1, 0x51, 0xa6, 0xab, 0x53, 0x68, 0xd7, 0xb7, 0xbf, 0x49, 0xf7, 0xf5, 0x8a, 0x4e, - 0x10, 0x36, 0x25, 0x1c, 0x13, 0xba, 0x12, 0x45, 0x37, 0x65, 0xa9, 0xe4, 0xed, 0x9f, - 0x4a, 0xa8, 0xda, 0x3b, - ]; - - let mut key_stream = AesCtrZipKeyStream::::new(&key); - - let mut key_buf = [0u8; 5]; - key_stream.read(&mut key_buf).unwrap(); - - let mut plaintext = ciphertext; - xor(&mut plaintext, &key_buf); - assert_eq!(&plaintext, expected_plaintext); - } - #[test] fn crypt_simple_example() { let ciphertext: [u8; 5] = [0xdc, 0x99, 0x93, 0x5e, 0xbf]; From 4afe4d3b2c57107bc970a392103025cc6be10f43 Mon Sep 17 00:00:00 2001 From: Marc Brinkmann Date: Sat, 3 Oct 2020 17:04:32 +0200 Subject: [PATCH 04/37] Optimize AES code, use less copies --- src/aes_ctr.rs | 54 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/src/aes_ctr.rs b/src/aes_ctr.rs index 2d1a4f5e..b544996a 100644 --- a/src/aes_ctr.rs +++ b/src/aes_ctr.rs @@ -1,8 +1,7 @@ use aes::block_cipher::generic_array::GenericArray; use aes::{BlockCipher, NewBlockCipher}; -use arrayvec::{Array, ArrayVec}; -use std::io::Read; -use std::{any, fmt, io}; +use byteorder::WriteBytesExt; +use std::{any, fmt}; /// Internal block size of an AES cipher. const AES_BLOCK_SIZE: usize = 16; @@ -20,14 +19,13 @@ pub struct Aes256; /// An AES cipher kind. pub trait AesKind { /// Key type. - type Key: Array; + type Key: AsRef<[u8]>; /// Cipher used to decrypt. type Cipher; } impl AesKind for Aes256 { type Key = [u8; 32]; - type Cipher = aes::Aes256; } @@ -40,10 +38,15 @@ impl AesKind for Aes256 { /// /// The stream implements the `Read` trait; encryption or decryption is performed by XOR-ing the /// bytes from the key stream with the ciphertext/plaintext. -struct AesCtrZipKeyStream { +pub struct AesCtrZipKeyStream { + /// Current AES counter. counter: u128, + /// AES cipher instance. cipher: C::Cipher, - buffer: ArrayVec, + /// Stores the currently available keystream bytes. + buffer: [u8; AES_BLOCK_SIZE], + /// Number of bytes already used up from `buffer`. + pos: usize, } impl fmt::Debug for AesCtrZipKeyStream @@ -65,13 +68,13 @@ where C: AesKind, C::Cipher: NewBlockCipher, { - #[allow(dead_code)] /// Creates a new zip variant AES-CTR key stream. pub fn new(key: &C::Key) -> AesCtrZipKeyStream { AesCtrZipKeyStream { counter: 1, - cipher: C::Cipher::new_varkey(key.as_slice()).expect("key should have correct size"), - buffer: ArrayVec::new(), + cipher: C::Cipher::new_varkey(key.as_ref()).expect("key should have correct size"), + buffer: [0u8; AES_BLOCK_SIZE], + pos: AES_BLOCK_SIZE, } } } @@ -85,21 +88,26 @@ where #[inline] fn crypt(&mut self, mut target: &mut [u8]) { while target.len() > 0 { - if self.buffer.len() == 0 { + if self.pos == AES_BLOCK_SIZE { // Note: AES block size is always 16 bytes, same as u128. - let mut block = GenericArray::clone_from_slice(&self.counter.to_le_bytes()); - - // TODO: Use trait. - self.cipher.encrypt_block(&mut block); + self.buffer + .as_mut() + .write_u128::(self.counter) + .expect("did not expect u128 le conversion to fail"); + self.cipher + .encrypt_block(GenericArray::from_mut_slice(&mut self.buffer)); self.counter += 1; - self.buffer = block.into_iter().collect(); + self.pos = 0; } - let target_len = target.len().min(self.buffer.len()); + let target_len = target.len().min(AES_BLOCK_SIZE - self.pos); - xor(&mut target[0..target_len], &self.buffer[0..target_len]); - self.buffer.drain(0..target_len); + xor( + &mut target[0..target_len], + &self.buffer[self.pos..(self.pos + target_len)], + ); target = &mut target[target_len..]; + self.pos += target_len; } } } @@ -116,8 +124,7 @@ pub fn xor(dest: &mut [u8], src: &[u8]) { #[cfg(test)] mod tests { - use super::{xor, Aes256, AesCtrZipKeyStream}; - use std::io::Read; + use super::{Aes256, AesCtrZipKeyStream}; #[test] fn crypt_simple_example() { @@ -134,5 +141,10 @@ mod tests { let mut plaintext = ciphertext; key_stream.crypt(&mut plaintext); assert_eq!(&plaintext, expected_plaintext); + + // Round-tripping should yield the ciphertext again. + let mut key_stream = AesCtrZipKeyStream::::new(&key); + key_stream.crypt(&mut plaintext); + assert_eq!(plaintext, ciphertext); } } From b3ec81335fadad48d56d9bef9b16bcfafbd19336 Mon Sep 17 00:00:00 2001 From: Marc Brinkmann Date: Sat, 3 Oct 2020 17:06:39 +0200 Subject: [PATCH 05/37] Remove `arrayvec` dependency --- Cargo.toml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 026672a4..44299d5c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,7 +12,6 @@ edition = "2018" [dependencies] aes = { version = "0.5.0", optional = true } -arrayvec = { version = "0.5.1", optional = true } flate2 = { version = "1.0.0", default-features = false, optional = true } time = { version = "0.1", optional = true } byteorder = "1.3" @@ -26,7 +25,7 @@ rand = "0.7" walkdir = "2" [features] -aes-crypto = ["aes", "arrayvec"] +aes-crypto = ["aes"] deflate = ["flate2/rust_backend"] deflate-miniz = ["flate2/default"] deflate-zlib = ["flate2/zlib"] From 4877a6afd463610ddb7447da8248a5e659f8ecfc Mon Sep 17 00:00:00 2001 From: Lireer Date: Sat, 3 Oct 2020 18:17:57 +0200 Subject: [PATCH 06/37] test different aes modes and data sizes --- src/aes_ctr.rs | 180 ++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 170 insertions(+), 10 deletions(-) diff --git a/src/aes_ctr.rs b/src/aes_ctr.rs index b544996a..a7b498f9 100644 --- a/src/aes_ctr.rs +++ b/src/aes_ctr.rs @@ -24,6 +24,16 @@ pub trait AesKind { type Cipher; } +impl AesKind for Aes128 { + type Key = [u8; 16]; + type Cipher = aes::Aes128; +} + +impl AesKind for Aes192 { + type Key = [u8; 24]; + type Cipher = aes::Aes192; +} + impl AesKind for Aes256 { type Key = [u8; 32]; type Cipher = aes::Aes256; @@ -72,7 +82,7 @@ where pub fn new(key: &C::Key) -> AesCtrZipKeyStream { AesCtrZipKeyStream { counter: 1, - cipher: C::Cipher::new_varkey(key.as_ref()).expect("key should have correct size"), + cipher: C::Cipher::new(GenericArray::from_slice(key.as_ref())), buffer: [0u8; AES_BLOCK_SIZE], pos: AES_BLOCK_SIZE, } @@ -124,27 +134,177 @@ pub fn xor(dest: &mut [u8], src: &[u8]) { #[cfg(test)] mod tests { - use super::{Aes256, AesCtrZipKeyStream}; + use super::{Aes128, Aes192, Aes256, AesCtrZipKeyStream}; + // The data used in these tests was generated with p7zip without any compression. + // It's not possible to recreate the exact same data, since a random salt is used for encryption. + // `7z a -phelloworld -mem=AES256 -mx=0 aes256_40byte.zip 40byte_data.txt` #[test] - fn crypt_simple_example() { - let ciphertext: [u8; 5] = [0xdc, 0x99, 0x93, 0x5e, 0xbf]; - let expected_plaintext = &[b'a', b's', b'd', b'f', b'\n']; + fn crypt_aes_256_0_byte() { + let ciphertext = []; + let expected_plaintext = &[]; let key = [ - 0xd1, 0x51, 0xa6, 0xab, 0x53, 0x68, 0xd7, 0xb7, 0xbf, 0x49, 0xf7, 0xf5, 0x8a, 0x4e, - 0x10, 0x36, 0x25, 0x1c, 0x13, 0xba, 0x12, 0x45, 0x37, 0x65, 0xa9, 0xe4, 0xed, 0x9f, - 0x4a, 0xa8, 0xda, 0x3b, + 0x0b, 0xec, 0x2e, 0xf2, 0x46, 0xf0, 0x7e, 0x35, 0x16, 0x54, 0xe0, 0x98, 0x10, 0xb3, + 0x18, 0x55, 0x24, 0xa3, 0x9e, 0x0e, 0x40, 0xe7, 0x92, 0xad, 0xb2, 0x8a, 0x48, 0xf4, + 0x5c, 0xd0, 0xc0, 0x54, ]; + eprintln!("{}", key.len()); let mut key_stream = AesCtrZipKeyStream::::new(&key); let mut plaintext = ciphertext; key_stream.crypt(&mut plaintext); - assert_eq!(&plaintext, expected_plaintext); + assert_eq!(plaintext.to_vec(), expected_plaintext.to_vec()); // Round-tripping should yield the ciphertext again. let mut key_stream = AesCtrZipKeyStream::::new(&key); key_stream.crypt(&mut plaintext); - assert_eq!(plaintext, ciphertext); + assert_eq!(plaintext.to_vec(), ciphertext.to_vec()); + } + + #[test] + fn crypt_aes_128_5_byte() { + let ciphertext = [0x98, 0xa9, 0x8c, 0x26, 0x0e]; + let expected_plaintext = &b"asdf\n"; + let key = [ + 0xe0, 0x25, 0x7b, 0x57, 0x97, 0x6a, 0xa4, 0x23, 0xab, 0x94, 0xaa, 0x44, 0xfd, 0x47, + 0x4f, 0xa5, + ]; + eprintln!("{}", key.len()); + + let mut key_stream = AesCtrZipKeyStream::::new(&key); + + let mut plaintext = ciphertext; + key_stream.crypt(&mut plaintext); + assert_eq!(plaintext.to_vec(), expected_plaintext.to_vec()); + + // Round-tripping should yield the ciphertext again. + let mut key_stream = AesCtrZipKeyStream::::new(&key); + key_stream.crypt(&mut plaintext); + assert_eq!(plaintext.to_vec(), ciphertext.to_vec()); + } + + #[test] + fn crypt_aes_192_5_byte() { + let ciphertext = [0x36, 0x55, 0x5c, 0x61, 0x3c]; + let expected_plaintext = &b"asdf\n"; + let key = [ + 0xe4, 0x4a, 0x88, 0x52, 0x8f, 0xf7, 0x0b, 0x81, 0x7b, 0x75, 0xf1, 0x74, 0x21, 0x37, + 0x8c, 0x90, 0xad, 0xbe, 0x4a, 0x65, 0xa8, 0x96, 0x0e, 0xcc, + ]; + eprintln!("{}", key.len()); + + let mut key_stream = AesCtrZipKeyStream::::new(&key); + + let mut plaintext = ciphertext; + key_stream.crypt(&mut plaintext); + assert_eq!(plaintext.to_vec(), expected_plaintext.to_vec()); + + // Round-tripping should yield the ciphertext again. + let mut key_stream = AesCtrZipKeyStream::::new(&key); + key_stream.crypt(&mut plaintext); + assert_eq!(plaintext.to_vec(), ciphertext.to_vec()); + } + + #[test] + fn crypt_aes_256_5_byte() { + let ciphertext = [0xc2, 0x47, 0xc0, 0xdc, 0x56]; + let expected_plaintext = &b"asdf\n"; + let key = [ + 0x79, 0x5e, 0x17, 0xf2, 0xc6, 0x3d, 0x28, 0x9b, 0x4b, 0x4b, 0xbb, 0xa9, 0xba, 0xc9, + 0xa5, 0xee, 0x3a, 0x4f, 0x0f, 0x4b, 0x29, 0xbd, 0xe9, 0xb8, 0x41, 0x9c, 0x41, 0xa5, + 0x15, 0xb2, 0x86, 0xab, + ]; + eprintln!("{}", key.len()); + + let mut key_stream = AesCtrZipKeyStream::::new(&key); + + let mut plaintext = ciphertext; + key_stream.crypt(&mut plaintext); + assert_eq!(plaintext.to_vec(), expected_plaintext.to_vec()); + + // Round-tripping should yield the ciphertext again. + let mut key_stream = AesCtrZipKeyStream::::new(&key); + key_stream.crypt(&mut plaintext); + assert_eq!(plaintext.to_vec(), ciphertext.to_vec()); + } + + #[test] + fn crypt_aes_128_40_byte() { + let ciphertext = [ + 0xcf, 0x72, 0x6b, 0xa1, 0xb2, 0x0f, 0xdf, 0xaa, 0x10, 0xad, 0x9c, 0x7f, 0x6d, 0x1c, + 0x8d, 0xb5, 0x16, 0x7e, 0xbb, 0x11, 0x69, 0x52, 0x8c, 0x89, 0x80, 0x32, 0xaa, 0x76, + 0xa6, 0x18, 0x31, 0x98, 0xee, 0xdd, 0x22, 0x68, 0xb7, 0xe6, 0x77, 0xd2, + ]; + let expected_plaintext = b"Lorem ipsum dolor sit amet, consectetur\n"; + let key = [ + 0x43, 0x2b, 0x6d, 0xbe, 0x05, 0x76, 0x6c, 0x9e, 0xde, 0xca, 0x3b, 0xf8, 0xaf, 0x5d, + 0x81, 0xb6, + ]; + eprintln!("{}", key.len()); + + let mut key_stream = AesCtrZipKeyStream::::new(&key); + + let mut plaintext = ciphertext; + key_stream.crypt(&mut plaintext); + assert_eq!(plaintext.to_vec(), expected_plaintext.to_vec()); + + // Round-tripping should yield the ciphertext again. + let mut key_stream = AesCtrZipKeyStream::::new(&key); + key_stream.crypt(&mut plaintext); + assert_eq!(plaintext.to_vec(), ciphertext.to_vec()); + } + + #[test] + fn crypt_aes_192_40_byte() { + let ciphertext = [ + 0xa6, 0xfc, 0x52, 0x79, 0x2c, 0x6c, 0xfe, 0x68, 0xb1, 0xa8, 0xb3, 0x07, 0x52, 0x8b, + 0x82, 0xa6, 0x87, 0x9c, 0x72, 0x42, 0x3a, 0xf8, 0xc6, 0xa9, 0xc9, 0xfb, 0x61, 0x19, + 0x37, 0xb9, 0x56, 0x62, 0xf4, 0xfc, 0x5e, 0x7a, 0xdd, 0x55, 0x0a, 0x48, + ]; + let expected_plaintext = b"Lorem ipsum dolor sit amet, consectetur\n"; + let key = [ + 0xac, 0x92, 0x41, 0xba, 0xde, 0xd9, 0x02, 0xfe, 0x40, 0x92, 0x20, 0xf6, 0x56, 0x03, + 0xfe, 0xae, 0x1b, 0xba, 0x01, 0x97, 0x97, 0x79, 0xbb, 0xa6, + ]; + eprintln!("{}", key.len()); + + let mut key_stream = AesCtrZipKeyStream::::new(&key); + + let mut plaintext = ciphertext; + key_stream.crypt(&mut plaintext); + assert_eq!(plaintext.to_vec(), expected_plaintext.to_vec()); + + // Round-tripping should yield the ciphertext again. + let mut key_stream = AesCtrZipKeyStream::::new(&key); + key_stream.crypt(&mut plaintext); + assert_eq!(plaintext.to_vec(), ciphertext.to_vec()); + } + + #[test] + fn crypt_aes_256_40_byte() { + let ciphertext = [ + 0xa9, 0x99, 0xbd, 0xea, 0x82, 0x9b, 0x8f, 0x2f, 0xb7, 0x52, 0x2f, 0x6b, 0xd8, 0xf6, + 0xab, 0x0e, 0x24, 0x51, 0x9e, 0x18, 0x0f, 0xc0, 0x8f, 0x54, 0x15, 0x80, 0xae, 0xbc, + 0xa0, 0x5c, 0x8a, 0x11, 0x8d, 0x14, 0x7e, 0xc5, 0xb4, 0xae, 0xd3, 0x37, + ]; + let expected_plaintext = b"Lorem ipsum dolor sit amet, consectetur\n"; + let key = [ + 0x64, 0x7c, 0x7a, 0xde, 0xf0, 0xf2, 0x61, 0x49, 0x1c, 0xf1, 0xf1, 0xe3, 0x37, 0xfc, + 0xe1, 0x4d, 0x4a, 0x77, 0xd4, 0xeb, 0x9e, 0x3d, 0x75, 0xce, 0x9a, 0x3e, 0x10, 0x50, + 0xc2, 0x07, 0x36, 0xb6, + ]; + eprintln!("{}", key.len()); + + let mut key_stream = AesCtrZipKeyStream::::new(&key); + + let mut plaintext = ciphertext; + key_stream.crypt(&mut plaintext); + assert_eq!(plaintext.to_vec(), expected_plaintext.to_vec()); + + // Round-tripping should yield the ciphertext again. + let mut key_stream = AesCtrZipKeyStream::::new(&key); + key_stream.crypt(&mut plaintext); + assert_eq!(plaintext.to_vec(), ciphertext.to_vec()); } } From 852ab625fbe07edd3952e6079184e0215f98915c Mon Sep 17 00:00:00 2001 From: Lireer Date: Sun, 4 Oct 2020 12:50:01 +0200 Subject: [PATCH 07/37] initial aes reader --- Cargo.toml | 10 +++-- src/aes.rs | 86 ++++++++++++++++++++++++++++++++++++++ src/compression.rs | 7 +++- src/lib.rs | 2 +- src/read.rs | 100 ++++++++++++++++++++++++++++++++++----------- src/types.rs | 25 ++++++++++++ src/write.rs | 6 +++ 7 files changed, 207 insertions(+), 29 deletions(-) create mode 100644 src/aes.rs diff --git a/Cargo.toml b/Cargo.toml index 44299d5c..e4e09aad 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,13 +11,16 @@ Library to support the reading and writing of zip files. edition = "2018" [dependencies] -aes = { version = "0.5.0", optional = true } -flate2 = { version = "1.0.0", default-features = false, optional = true } -time = { version = "0.1", optional = true } +aes = "0.5.0" byteorder = "1.3" bzip2 = { version = "0.4", optional = true } crc32fast = "1.0" +flate2 = { version = "1.0.0", default-features = false, optional = true } +hmac = "0.9.0" +pbkdf2 = "0.5.0" +sha-1 = "0.9.1" thiserror = "1.0" +time = { version = "0.1", optional = true } [dev-dependencies] bencher = "0.1" @@ -25,7 +28,6 @@ rand = "0.7" walkdir = "2" [features] -aes-crypto = ["aes"] deflate = ["flate2/rust_backend"] deflate-miniz = ["flate2/default"] deflate-zlib = ["flate2/zlib"] diff --git a/src/aes.rs b/src/aes.rs new file mode 100644 index 00000000..d78bbcfa --- /dev/null +++ b/src/aes.rs @@ -0,0 +1,86 @@ +use crate::types::AesMode; +use std::io; + +use byteorder::{LittleEndian, ReadBytesExt}; +use hmac::Hmac; +use sha1::Sha1; + +/// The length of the password verifcation value in bytes +const PWD_VERIFY_LENGTH: u64 = 2; +/// The length of the authentication code in bytes +const AUTH_CODE_LENGTH: u64 = 10; +/// The number of iterations used with PBKDF2 +const ITERATION_COUNT: u32 = 1000; +/// AES block size in bytes +const BLOCK_SIZE: usize = 16; + +// an aes encrypted file starts with a salt, whose length depends on the used aes mode +// followed by a 2 byte password verification value +// then the variable length encrypted data +// and lastly a 10 byte authentication code +pub(crate) struct AesReader { + reader: R, + aes_mode: AesMode, + salt_length: usize, + data_length: u64, +} + +impl AesReader { + pub fn new(reader: R, aes_mode: AesMode, compressed_size: u64) -> AesReader { + let salt_length = aes_mode.salt_length(); + let data_length = compressed_size - (PWD_VERIFY_LENGTH + AUTH_CODE_LENGTH + salt_length); + + Self { + reader, + aes_mode, + salt_length: salt_length as usize, + data_length, + } + } + + pub fn validate(mut self, password: &[u8]) -> Result>, io::Error> { + // the length of the salt depends on the used key size + let mut salt = vec![0; self.salt_length as usize]; + self.reader.read_exact(&mut salt).unwrap(); + + // next are 2 bytes used for password verification + let mut pwd_verification_value = vec![0; PWD_VERIFY_LENGTH as usize]; + self.reader.read_exact(&mut pwd_verification_value).unwrap(); + + // derive a key from the password and salt + // the length depends on the aes key length + let derived_key_len = (2 * self.aes_mode.key_length() + PWD_VERIFY_LENGTH) as usize; + let mut derived_key: Vec = vec![0; derived_key_len]; + + // use PBKDF2 with HMAC-Sha1 to derive the key + pbkdf2::pbkdf2::>(password, &salt, ITERATION_COUNT, &mut derived_key); + + // the last 2 bytes should equal the password verification value + if pwd_verification_value != &derived_key[derived_key_len - 2..] { + // wrong password + return Ok(None); + } + + // the first key_length bytes are used as decryption key + let decrypt_key = &derived_key[0..self.aes_mode.key_length() as usize]; + + panic!("Validating AesReader"); + } +} + +pub(crate) struct AesReaderValid { + reader: AesReader, +} + +impl io::Read for AesReaderValid { + fn read(&mut self, mut buf: &mut [u8]) -> std::io::Result { + panic!("Reading from AesReaderValid") + } +} + +impl AesReaderValid { + /// Consumes this decoder, returning the underlying reader. + pub fn into_inner(self) -> R { + self.reader.reader + } +} diff --git a/src/compression.rs b/src/compression.rs index 5fdde070..84e69d15 100644 --- a/src/compression.rs +++ b/src/compression.rs @@ -24,6 +24,10 @@ pub enum CompressionMethod { /// Compress the file using BZIP2 #[cfg(feature = "bzip2")] Bzip2, + /// Encrypted using AES. + /// The actual compression method has to be taken from the AES extra data field + /// or from `ZipFileData`. + AES, /// Unsupported compression method #[deprecated(since = "0.5.7", note = "use the constants instead")] Unsupported(u16), @@ -85,7 +89,7 @@ impl CompressionMethod { 8 => CompressionMethod::Deflated, #[cfg(feature = "bzip2")] 12 => CompressionMethod::Bzip2, - + 99 => CompressionMethod::AES, v => CompressionMethod::Unsupported(v), } } @@ -107,6 +111,7 @@ impl CompressionMethod { CompressionMethod::Deflated => 8, #[cfg(feature = "bzip2")] CompressionMethod::Bzip2 => 12, + CompressionMethod::AES => 99, CompressionMethod::Unsupported(v) => v, } } diff --git a/src/lib.rs b/src/lib.rs index df398ce7..45c6c2e1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -10,7 +10,7 @@ pub use crate::read::ZipArchive; pub use crate::types::DateTime; pub use crate::write::ZipWriter; -#[cfg(feature = "aes-crypto")] +mod aes; mod aes_ctr; mod compression; mod cp437; diff --git a/src/read.rs b/src/read.rs index 97bccd2d..e1afaf03 100644 --- a/src/read.rs +++ b/src/read.rs @@ -1,19 +1,19 @@ //! Types for reading ZIP archives +use crate::aes::{AesReaderValid, AesReader}; use crate::compression::CompressionMethod; +use crate::cp437::FromCp437; use crate::crc32::Crc32Reader; use crate::result::{InvalidPassword, ZipError, ZipResult}; use crate::spec; +use crate::types::{AesMode, DateTime, System, ZipFileData}; use crate::zipcrypto::{ZipCryptoReader, ZipCryptoReaderValid, ZipCryptoValidator}; +use byteorder::{LittleEndian, ReadBytesExt}; use std::borrow::Cow; use std::collections::HashMap; use std::io::{self, prelude::*}; use std::path::{Component, Path}; -use crate::cp437::FromCp437; -use crate::types::{DateTime, System, ZipFileData}; -use byteorder::{LittleEndian, ReadBytesExt}; - #[cfg(any( feature = "deflate", feature = "deflate-miniz", @@ -57,6 +57,7 @@ pub struct ZipArchive { enum CryptoReader<'a> { Plaintext(io::Take<&'a mut dyn Read>), ZipCrypto(ZipCryptoReaderValid>), + Aes(AesReaderValid>), } impl<'a> Read for CryptoReader<'a> { @@ -64,6 +65,7 @@ impl<'a> Read for CryptoReader<'a> { match self { CryptoReader::Plaintext(r) => r.read(buf), CryptoReader::ZipCrypto(r) => r.read(buf), + CryptoReader::Aes(r) => r.read(buf), } } } @@ -74,6 +76,7 @@ impl<'a> CryptoReader<'a> { match self { CryptoReader::Plaintext(r) => r, CryptoReader::ZipCrypto(r) => r.into_inner(), + CryptoReader::Aes(r) => r.into_inner(), } } } @@ -164,6 +167,8 @@ fn make_crypto_reader<'a>( using_data_descriptor: bool, reader: io::Take<&'a mut dyn io::Read>, password: Option<&[u8]>, + aes_mode: Option, + compressed_size: u64, ) -> ZipResult, InvalidPassword>> { #[allow(deprecated)] { @@ -172,9 +177,9 @@ fn make_crypto_reader<'a>( } } - let reader = match password { - None => CryptoReader::Plaintext(reader), - Some(password) => { + let reader = match (password, aes_mode) { + (None, _) => CryptoReader::Plaintext(reader), + (Some(password), None) => { let validator = if using_data_descriptor { ZipCryptoValidator::InfoZipMsdosTime(last_modified_time.timepart()) } else { @@ -185,6 +190,12 @@ fn make_crypto_reader<'a>( Some(r) => CryptoReader::ZipCrypto(r), } } + (Some(password), Some(aes_mode)) => { + match AesReader::new(reader, aes_mode, compressed_size).validate(&password)? { + None => return Ok(Err(InvalidPassword)), + Some(r) => CryptoReader::Aes(r), + } + } }; Ok(Ok(reader)) } @@ -502,6 +513,8 @@ impl ZipArchive { data.using_data_descriptor, limit_reader, password, + data.aes_mode, + data.compressed_size ) { Ok(Ok(crypto_reader)) => Ok(Ok(ZipFile { crypto_reader: Some(crypto_reader), @@ -595,12 +608,21 @@ pub(crate) fn central_header_to_zip_file( data_start: 0, external_attributes: external_file_attributes, large_file: false, + aes_mode: None, }; - + match parse_extra_field(&mut result) { Ok(..) | Err(ZipError::Io(..)) => {} Err(e) => return Err(e), } + + + let aes_enabled = result.compression_method == CompressionMethod::AES; + if aes_enabled && result.aes_mode.is_none() { + return Err(ZipError::InvalidArchive( + "AES encryption without AES extra data field", + )); + } // Account for shifted zip offsets. result.header_start += archive_offset; @@ -615,24 +637,53 @@ fn parse_extra_field(file: &mut ZipFileData) -> ZipResult<()> { let kind = reader.read_u16::()?; let len = reader.read_u16::()?; let mut len_left = len as i64; - // Zip64 extended information extra field - if kind == 0x0001 { - if file.uncompressed_size == 0xFFFFFFFF { - file.large_file = true; - file.uncompressed_size = reader.read_u64::()?; - len_left -= 8; + match kind { + // Zip64 extended information extra field + 0x0001 => { + if file.uncompressed_size == 0xFFFFFFFF { + file.large_file = true; + file.uncompressed_size = reader.read_u64::()?; + len_left -= 8; + } + if file.compressed_size == 0xFFFFFFFF { + file.large_file = true; + file.compressed_size = reader.read_u64::()?; + len_left -= 8; + } + if file.header_start == 0xFFFFFFFF { + file.header_start = reader.read_u64::()?; + len_left -= 8; + } } - if file.compressed_size == 0xFFFFFFFF { - file.large_file = true; - file.compressed_size = reader.read_u64::()?; - len_left -= 8; + 0x9901 => { + // AES + if len != 7 { + return Err(ZipError::UnsupportedArchive( + "AES extra data field has an unsupported length", + )); + } + let _vendor_version = reader.read_u16::()?; // TODO: CRC value handling changes + let vendor_id = reader.read_u16::()?; + let aes_mode = reader.read_u8()?; + let compression_method = reader.read_u16::()?; + + if vendor_id != 0x4541 { + return Err(ZipError::InvalidArchive("Invalid AES vendor")); + } + match aes_mode { + 0x01 => file.aes_mode = Some(AesMode::Aes128), + 0x02 => file.aes_mode = Some(AesMode::Aes192), + 0x03 => file.aes_mode = Some(AesMode::Aes256), + _ => return Err(ZipError::InvalidArchive("Invalid AES encryption strength")), + }; + file.compression_method = { + #[allow(deprecated)] + CompressionMethod::from_u16(compression_method) + }; } - if file.header_start == 0xFFFFFFFF { - file.header_start = reader.read_u64::()?; - len_left -= 8; + _ => { + // Other fields are ignored } - // Unparsed fields: - // u32: disk start number } // We could also check for < 0 to check for errors @@ -950,6 +1001,7 @@ pub fn read_zipfile_from_stream<'a, R: io::Read>( // from standard input, this field is set to zero.' external_attributes: 0, large_file: false, + aes_mode: None, }; match parse_extra_field(&mut result) { @@ -975,6 +1027,8 @@ pub fn read_zipfile_from_stream<'a, R: io::Read>( result.using_data_descriptor, limit_reader, None, + None, + result.compressed_size, )? .unwrap(); diff --git a/src/types.rs b/src/types.rs index 026aa150..c2f40f4a 100644 --- a/src/types.rs +++ b/src/types.rs @@ -248,6 +248,8 @@ pub struct ZipFileData { pub external_attributes: u32, /// Reserve local ZIP64 extra field pub large_file: bool, + /// AES mode if applicable + pub aes_mode: Option, } impl ZipFileData { @@ -298,6 +300,28 @@ impl ZipFileData { } } +/// AES variant used. +#[derive(Copy, Clone, Debug, PartialEq)] +pub enum AesMode { + Aes128, + Aes192, + Aes256, +} + +impl AesMode { + pub fn salt_length(&self) -> u64 { + self.key_length() / 2 + } + + pub fn key_length(&self) -> u64 { + match self { + Self::Aes128 => 16, + Self::Aes192 => 24, + Self::Aes256 => 32, + } + } +} + #[cfg(test)] mod test { #[test] @@ -332,6 +356,7 @@ mod test { central_header_start: 0, external_attributes: 0, large_file: false, + aes_mode: None, }; assert_eq!( data.file_name_sanitized(), diff --git a/src/write.rs b/src/write.rs index 05c3666a..05236505 100644 --- a/src/write.rs +++ b/src/write.rs @@ -334,6 +334,7 @@ impl ZipWriter { central_header_start: 0, external_attributes: permissions << 16, large_file: options.large_file, + aes_mode: None, }; write_local_file_header(writer, &file)?; @@ -830,6 +831,11 @@ impl GenericZipWriter { CompressionMethod::Bzip2 => { GenericZipWriter::Bzip2(BzEncoder::new(bare, bzip2::Compression::default())) } + CompressionMethod::AES => { + return Err(ZipError::UnsupportedArchive( + "AES compression is not supported for writing", + )) + } CompressionMethod::Unsupported(..) => { return Err(ZipError::UnsupportedArchive("Unsupported compression")) } From 12260f56233000e8d77dc51d7cd402f3b95efb70 Mon Sep 17 00:00:00 2001 From: Lireer Date: Thu, 8 Oct 2020 18:25:30 +0200 Subject: [PATCH 08/37] disable crc32 checks when handling aes encrypted data --- README.md | 2 +- src/crc32.rs | 16 ++++++++++------ src/read.rs | 18 ++++++++++-------- 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index e489b98d..b4df15a7 100644 --- a/README.md +++ b/README.md @@ -51,7 +51,7 @@ All of these are enabled by default. MSRV ---- -Our current Minimum Supported Rust Version is **1.36.0**. When adding features, +Our current Minimum Supported Rust Version is **1.42.0**. When adding features, we will follow these guidelines: - We will always support the latest four minor Rust versions. This gives you a 6 diff --git a/src/crc32.rs b/src/crc32.rs index b351aa01..5c34f25a 100644 --- a/src/crc32.rs +++ b/src/crc32.rs @@ -10,15 +10,19 @@ pub struct Crc32Reader { inner: R, hasher: Hasher, check: u32, + /// Signals if `inner` stores aes encrypted data. + /// AE-2 encrypted data doesn't use crc and sets the value to 0. + aes_encrypted: bool, } impl Crc32Reader { /// Get a new Crc32Reader which check the inner reader against checksum. - pub fn new(inner: R, checksum: u32) -> Crc32Reader { + pub fn new(inner: R, checksum: u32, aes_encrypted: bool) -> Crc32Reader { Crc32Reader { inner, hasher: Hasher::new(), check: checksum, + aes_encrypted, } } @@ -34,7 +38,7 @@ impl Crc32Reader { impl Read for Crc32Reader { fn read(&mut self, buf: &mut [u8]) -> io::Result { let count = match self.inner.read(buf) { - Ok(0) if !buf.is_empty() && !self.check_matches() => { + Ok(0) if !buf.is_empty() && !self.check_matches() && !self.aes_encrypted => { return Err(io::Error::new(io::ErrorKind::Other, "Invalid checksum")) } Ok(n) => n, @@ -55,10 +59,10 @@ mod test { let data: &[u8] = b""; let mut buf = [0; 1]; - let mut reader = Crc32Reader::new(data, 0); + let mut reader = Crc32Reader::new(data, 0, false); assert_eq!(reader.read(&mut buf).unwrap(), 0); - let mut reader = Crc32Reader::new(data, 1); + let mut reader = Crc32Reader::new(data, 1, false); assert!(reader .read(&mut buf) .unwrap_err() @@ -71,7 +75,7 @@ mod test { let data: &[u8] = b"1234"; let mut buf = [0; 1]; - let mut reader = Crc32Reader::new(data, 0x9be3e0a3); + let mut reader = Crc32Reader::new(data, 0x9be3e0a3, false); assert_eq!(reader.read(&mut buf).unwrap(), 1); assert_eq!(reader.read(&mut buf).unwrap(), 1); assert_eq!(reader.read(&mut buf).unwrap(), 1); @@ -86,7 +90,7 @@ mod test { let data: &[u8] = b"1234"; let mut buf = [0; 5]; - let mut reader = Crc32Reader::new(data, 0x9be3e0a3); + let mut reader = Crc32Reader::new(data, 0x9be3e0a3, false); assert_eq!(reader.read(&mut buf[..0]).unwrap(), 0); assert_eq!(reader.read(&mut buf).unwrap(), 4); } diff --git a/src/read.rs b/src/read.rs index e1afaf03..4b60d246 100644 --- a/src/read.rs +++ b/src/read.rs @@ -1,6 +1,6 @@ //! Types for reading ZIP archives -use crate::aes::{AesReaderValid, AesReader}; +use crate::aes::{AesReader, AesReaderValid}; use crate::compression::CompressionMethod; use crate::cp437::FromCp437; use crate::crc32::Crc32Reader; @@ -205,8 +205,11 @@ fn make_reader<'a>( crc32: u32, reader: CryptoReader<'a>, ) -> ZipFileReader<'a> { + let aes_encrypted = matches!(reader, CryptoReader::Aes(_)); match compression_method { - CompressionMethod::Stored => ZipFileReader::Stored(Crc32Reader::new(reader, crc32)), + CompressionMethod::Stored => { + ZipFileReader::Stored(Crc32Reader::new(reader, crc32, aes_encrypted)) + } #[cfg(any( feature = "deflate", feature = "deflate-miniz", @@ -214,12 +217,12 @@ fn make_reader<'a>( ))] CompressionMethod::Deflated => { let deflate_reader = DeflateDecoder::new(reader); - ZipFileReader::Deflated(Crc32Reader::new(deflate_reader, crc32)) + ZipFileReader::Deflated(Crc32Reader::new(deflate_reader, crc32, aes_encrypted)) } #[cfg(feature = "bzip2")] CompressionMethod::Bzip2 => { let bzip2_reader = BzDecoder::new(reader); - ZipFileReader::Bzip2(Crc32Reader::new(bzip2_reader, crc32)) + ZipFileReader::Bzip2(Crc32Reader::new(bzip2_reader, crc32, aes_encrypted)) } _ => panic!("Compression method not supported"), } @@ -514,7 +517,7 @@ impl ZipArchive { limit_reader, password, data.aes_mode, - data.compressed_size + data.compressed_size, ) { Ok(Ok(crypto_reader)) => Ok(Ok(ZipFile { crypto_reader: Some(crypto_reader), @@ -610,13 +613,12 @@ pub(crate) fn central_header_to_zip_file( large_file: false, aes_mode: None, }; - + match parse_extra_field(&mut result) { Ok(..) | Err(ZipError::Io(..)) => {} Err(e) => return Err(e), } - - + let aes_enabled = result.compression_method == CompressionMethod::AES; if aes_enabled && result.aes_mode.is_none() { return Err(ZipError::InvalidArchive( From d25d6f5f57417439b0d7ed5136accbb2510c9476 Mon Sep 17 00:00:00 2001 From: Lireer Date: Thu, 8 Oct 2020 18:30:27 +0200 Subject: [PATCH 09/37] finalize AesReader validation and most of decryption --- src/aes.rs | 112 +++++++++++++++++++++++++++++++++++-------------- src/aes_ctr.rs | 44 ++++++++++--------- src/types.rs | 4 +- 3 files changed, 106 insertions(+), 54 deletions(-) diff --git a/src/aes.rs b/src/aes.rs index d78bbcfa..d4df982b 100644 --- a/src/aes.rs +++ b/src/aes.rs @@ -1,86 +1,134 @@ +use crate::aes_ctr; use crate::types::AesMode; -use std::io; - -use byteorder::{LittleEndian, ReadBytesExt}; -use hmac::Hmac; +use hmac::{Hmac, Mac, NewMac}; use sha1::Sha1; +use std::io::{Error, Read}; /// The length of the password verifcation value in bytes -const PWD_VERIFY_LENGTH: u64 = 2; +const PWD_VERIFY_LENGTH: usize = 2; /// The length of the authentication code in bytes -const AUTH_CODE_LENGTH: u64 = 10; +const AUTH_CODE_LENGTH: usize = 10; /// The number of iterations used with PBKDF2 const ITERATION_COUNT: u32 = 1000; -/// AES block size in bytes -const BLOCK_SIZE: usize = 16; + +fn cipher_from_mode(aes_mode: AesMode, key: &[u8]) -> Box { + match aes_mode { + AesMode::Aes128 => Box::new(aes_ctr::AesCtrZipKeyStream::::new(key)) + as Box, + AesMode::Aes192 => Box::new(aes_ctr::AesCtrZipKeyStream::::new(key)) + as Box, + AesMode::Aes256 => Box::new(aes_ctr::AesCtrZipKeyStream::::new(key)) + as Box, + } +} // an aes encrypted file starts with a salt, whose length depends on the used aes mode // followed by a 2 byte password verification value // then the variable length encrypted data // and lastly a 10 byte authentication code -pub(crate) struct AesReader { +pub struct AesReader { reader: R, aes_mode: AesMode, - salt_length: usize, data_length: u64, } -impl AesReader { +impl AesReader { pub fn new(reader: R, aes_mode: AesMode, compressed_size: u64) -> AesReader { - let salt_length = aes_mode.salt_length(); - let data_length = compressed_size - (PWD_VERIFY_LENGTH + AUTH_CODE_LENGTH + salt_length); + let data_length = compressed_size + - (PWD_VERIFY_LENGTH + AUTH_CODE_LENGTH + aes_mode.salt_length()) as u64; Self { reader, aes_mode, - salt_length: salt_length as usize, data_length, } } - pub fn validate(mut self, password: &[u8]) -> Result>, io::Error> { - // the length of the salt depends on the used key size - let mut salt = vec![0; self.salt_length as usize]; - self.reader.read_exact(&mut salt).unwrap(); + /// Read the AES header bytes and validate the password. + /// + /// Even if the validation succeeds, there is still a 1 in 65536 chance that an incorrect + /// password was provided. + /// It isn't possible to check the authentication code in this step. This will be done after + /// reading and decrypting the file. + pub fn validate(mut self, password: &[u8]) -> Result>, Error> { + + let salt_length = self.aes_mode.salt_length(); + let key_length = self.aes_mode.key_length(); + + let mut salt = vec![0; salt_length]; + self.reader.read_exact(&mut salt)?; // next are 2 bytes used for password verification - let mut pwd_verification_value = vec![0; PWD_VERIFY_LENGTH as usize]; - self.reader.read_exact(&mut pwd_verification_value).unwrap(); + let mut pwd_verification_value = vec![0; PWD_VERIFY_LENGTH]; + self.reader.read_exact(&mut pwd_verification_value)?; // derive a key from the password and salt // the length depends on the aes key length - let derived_key_len = (2 * self.aes_mode.key_length() + PWD_VERIFY_LENGTH) as usize; + let derived_key_len = 2 * key_length + PWD_VERIFY_LENGTH; let mut derived_key: Vec = vec![0; derived_key_len]; // use PBKDF2 with HMAC-Sha1 to derive the key pbkdf2::pbkdf2::>(password, &salt, ITERATION_COUNT, &mut derived_key); + let decrypt_key = &derived_key[0..key_length]; + let hmac_key = &derived_key[key_length..key_length * 2]; + let pwd_verify = &derived_key[derived_key_len - 2..]; // the last 2 bytes should equal the password verification value - if pwd_verification_value != &derived_key[derived_key_len - 2..] { + if pwd_verification_value != pwd_verify { // wrong password return Ok(None); } - // the first key_length bytes are used as decryption key - let decrypt_key = &derived_key[0..self.aes_mode.key_length() as usize]; + let cipher = cipher_from_mode(self.aes_mode, &decrypt_key); + let hmac = Hmac::::new_varkey(hmac_key).unwrap(); - panic!("Validating AesReader"); + Ok(Some(AesReaderValid { + reader: self.reader, + data_remaining: self.data_length, + cipher, + hmac, + })) } } -pub(crate) struct AesReaderValid { - reader: AesReader, +pub struct AesReaderValid { + reader: R, + data_remaining: u64, + cipher: Box, + hmac: Hmac, } -impl io::Read for AesReaderValid { - fn read(&mut self, mut buf: &mut [u8]) -> std::io::Result { - panic!("Reading from AesReaderValid") +impl Read for AesReaderValid { + fn read(&mut self, buf: &mut [u8]) -> std::io::Result { + // get the number of bytes to read, compare as u64 to make sure we can read more than + // 2^32 bytes even on 32 bit systems. + let bytes_to_read = self.data_remaining.min(buf.len() as u64) as usize; + let read = self.reader.read(&mut buf[0..bytes_to_read])?; + + self.cipher.crypt_in_place(&mut buf[0..read]); + self.data_remaining -= read as u64; + + // Update the hmac with the decrypted data + self.hmac.update(&buf[0..read]); + if self.data_remaining <= 0 { + // if there is no data left to read, check the integrity of the data + let mut read_auth = [0; 10]; + self.reader.read_exact(&mut read_auth)?; + let computed_auth = self.hmac.finalize_reset(); + // FIXME: The mac uses the whole sha1 hash each step + // Zip uses HMAC-Sha1-80, which throws away the second half each time + // see https://www.winzip.com/win/en/aes_info.html#auth-faq + // if computed_auth.into_bytes().as_slice() != &read_auth { + // } + } + + Ok(read) } } -impl AesReaderValid { +impl AesReaderValid { /// Consumes this decoder, returning the underlying reader. pub fn into_inner(self) -> R { - self.reader.reader + self.reader } } diff --git a/src/aes_ctr.rs b/src/aes_ctr.rs index a7b498f9..efda6b77 100644 --- a/src/aes_ctr.rs +++ b/src/aes_ctr.rs @@ -79,24 +79,24 @@ where C::Cipher: NewBlockCipher, { /// Creates a new zip variant AES-CTR key stream. - pub fn new(key: &C::Key) -> AesCtrZipKeyStream { + pub fn new(key: &[u8]) -> AesCtrZipKeyStream { AesCtrZipKeyStream { counter: 1, - cipher: C::Cipher::new(GenericArray::from_slice(key.as_ref())), + cipher: C::Cipher::new(GenericArray::from_slice(key)), buffer: [0u8; AES_BLOCK_SIZE], pos: AES_BLOCK_SIZE, } } } -impl AesCtrZipKeyStream +impl AesCipher for AesCtrZipKeyStream where C: AesKind, C::Cipher: BlockCipher, { /// Decrypt or encrypt given data. #[inline] - fn crypt(&mut self, mut target: &mut [u8]) { + fn crypt_in_place(&mut self, mut target: &mut [u8]) { while target.len() > 0 { if self.pos == AES_BLOCK_SIZE { // Note: AES block size is always 16 bytes, same as u128. @@ -122,9 +122,13 @@ where } } +pub trait AesCipher { + fn crypt_in_place(&mut self, target: &mut [u8]); +} + /// XORs a slice in place with another slice. #[inline] -pub fn xor(dest: &mut [u8], src: &[u8]) { +fn xor(dest: &mut [u8], src: &[u8]) { debug_assert_eq!(dest.len(), src.len()); for (lhs, rhs) in dest.iter_mut().zip(src.iter()) { @@ -134,7 +138,7 @@ pub fn xor(dest: &mut [u8], src: &[u8]) { #[cfg(test)] mod tests { - use super::{Aes128, Aes192, Aes256, AesCtrZipKeyStream}; + use super::{Aes128, Aes192, Aes256, AesCipher, AesCtrZipKeyStream}; // The data used in these tests was generated with p7zip without any compression. // It's not possible to recreate the exact same data, since a random salt is used for encryption. @@ -153,12 +157,12 @@ mod tests { let mut key_stream = AesCtrZipKeyStream::::new(&key); let mut plaintext = ciphertext; - key_stream.crypt(&mut plaintext); + key_stream.crypt_in_place(&mut plaintext); assert_eq!(plaintext.to_vec(), expected_plaintext.to_vec()); // Round-tripping should yield the ciphertext again. let mut key_stream = AesCtrZipKeyStream::::new(&key); - key_stream.crypt(&mut plaintext); + key_stream.crypt_in_place(&mut plaintext); assert_eq!(plaintext.to_vec(), ciphertext.to_vec()); } @@ -175,12 +179,12 @@ mod tests { let mut key_stream = AesCtrZipKeyStream::::new(&key); let mut plaintext = ciphertext; - key_stream.crypt(&mut plaintext); + key_stream.crypt_in_place(&mut plaintext); assert_eq!(plaintext.to_vec(), expected_plaintext.to_vec()); // Round-tripping should yield the ciphertext again. let mut key_stream = AesCtrZipKeyStream::::new(&key); - key_stream.crypt(&mut plaintext); + key_stream.crypt_in_place(&mut plaintext); assert_eq!(plaintext.to_vec(), ciphertext.to_vec()); } @@ -197,12 +201,12 @@ mod tests { let mut key_stream = AesCtrZipKeyStream::::new(&key); let mut plaintext = ciphertext; - key_stream.crypt(&mut plaintext); + key_stream.crypt_in_place(&mut plaintext); assert_eq!(plaintext.to_vec(), expected_plaintext.to_vec()); // Round-tripping should yield the ciphertext again. let mut key_stream = AesCtrZipKeyStream::::new(&key); - key_stream.crypt(&mut plaintext); + key_stream.crypt_in_place(&mut plaintext); assert_eq!(plaintext.to_vec(), ciphertext.to_vec()); } @@ -220,12 +224,12 @@ mod tests { let mut key_stream = AesCtrZipKeyStream::::new(&key); let mut plaintext = ciphertext; - key_stream.crypt(&mut plaintext); + key_stream.crypt_in_place(&mut plaintext); assert_eq!(plaintext.to_vec(), expected_plaintext.to_vec()); // Round-tripping should yield the ciphertext again. let mut key_stream = AesCtrZipKeyStream::::new(&key); - key_stream.crypt(&mut plaintext); + key_stream.crypt_in_place(&mut plaintext); assert_eq!(plaintext.to_vec(), ciphertext.to_vec()); } @@ -246,12 +250,12 @@ mod tests { let mut key_stream = AesCtrZipKeyStream::::new(&key); let mut plaintext = ciphertext; - key_stream.crypt(&mut plaintext); + key_stream.crypt_in_place(&mut plaintext); assert_eq!(plaintext.to_vec(), expected_plaintext.to_vec()); // Round-tripping should yield the ciphertext again. let mut key_stream = AesCtrZipKeyStream::::new(&key); - key_stream.crypt(&mut plaintext); + key_stream.crypt_in_place(&mut plaintext); assert_eq!(plaintext.to_vec(), ciphertext.to_vec()); } @@ -272,12 +276,12 @@ mod tests { let mut key_stream = AesCtrZipKeyStream::::new(&key); let mut plaintext = ciphertext; - key_stream.crypt(&mut plaintext); + key_stream.crypt_in_place(&mut plaintext); assert_eq!(plaintext.to_vec(), expected_plaintext.to_vec()); // Round-tripping should yield the ciphertext again. let mut key_stream = AesCtrZipKeyStream::::new(&key); - key_stream.crypt(&mut plaintext); + key_stream.crypt_in_place(&mut plaintext); assert_eq!(plaintext.to_vec(), ciphertext.to_vec()); } @@ -299,12 +303,12 @@ mod tests { let mut key_stream = AesCtrZipKeyStream::::new(&key); let mut plaintext = ciphertext; - key_stream.crypt(&mut plaintext); + key_stream.crypt_in_place(&mut plaintext); assert_eq!(plaintext.to_vec(), expected_plaintext.to_vec()); // Round-tripping should yield the ciphertext again. let mut key_stream = AesCtrZipKeyStream::::new(&key); - key_stream.crypt(&mut plaintext); + key_stream.crypt_in_place(&mut plaintext); assert_eq!(plaintext.to_vec(), ciphertext.to_vec()); } } diff --git a/src/types.rs b/src/types.rs index c2f40f4a..d2a7645d 100644 --- a/src/types.rs +++ b/src/types.rs @@ -309,11 +309,11 @@ pub enum AesMode { } impl AesMode { - pub fn salt_length(&self) -> u64 { + pub fn salt_length(&self) -> usize { self.key_length() / 2 } - pub fn key_length(&self) -> u64 { + pub fn key_length(&self) -> usize { match self { Self::Aes128 => 16, Self::Aes192 => 24, From e69df5cf64f47c560097e70dc02c9d6728c06648 Mon Sep 17 00:00:00 2001 From: Lireer Date: Fri, 9 Oct 2020 16:05:10 +0200 Subject: [PATCH 10/37] finalize aes decryption --- Cargo.toml | 1 + src/aes.rs | 45 +++++++++++++++++++++++++++++---------------- 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index e4e09aad..b6c2fc3b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,6 +14,7 @@ edition = "2018" aes = "0.5.0" byteorder = "1.3" bzip2 = { version = "0.4", optional = true } +constant_time_eq = "0.1.5" crc32fast = "1.0" flate2 = { version = "1.0.0", default-features = false, optional = true } hmac = "0.9.0" diff --git a/src/aes.rs b/src/aes.rs index d4df982b..e727285a 100644 --- a/src/aes.rs +++ b/src/aes.rs @@ -1,8 +1,9 @@ use crate::aes_ctr; use crate::types::AesMode; +use constant_time_eq::constant_time_eq; use hmac::{Hmac, Mac, NewMac}; use sha1::Sha1; -use std::io::{Error, Read}; +use std::io::{self, Read}; /// The length of the password verifcation value in bytes const PWD_VERIFY_LENGTH: usize = 2; @@ -50,8 +51,7 @@ impl AesReader { /// password was provided. /// It isn't possible to check the authentication code in this step. This will be done after /// reading and decrypting the file. - pub fn validate(mut self, password: &[u8]) -> Result>, Error> { - + pub fn validate(mut self, password: &[u8]) -> io::Result>> { let salt_length = self.aes_mode.salt_length(); let key_length = self.aes_mode.key_length(); @@ -99,27 +99,40 @@ pub struct AesReaderValid { } impl Read for AesReaderValid { - fn read(&mut self, buf: &mut [u8]) -> std::io::Result { + fn read(&mut self, buf: &mut [u8]) -> io::Result { + if self.data_remaining == 0 { + return Ok(0); + } + // get the number of bytes to read, compare as u64 to make sure we can read more than // 2^32 bytes even on 32 bit systems. let bytes_to_read = self.data_remaining.min(buf.len() as u64) as usize; let read = self.reader.read(&mut buf[0..bytes_to_read])?; + // Update the hmac with the encrypted data + self.hmac.update(&buf[0..read]); + + // decrypt the data self.cipher.crypt_in_place(&mut buf[0..read]); self.data_remaining -= read as u64; - // Update the hmac with the decrypted data - self.hmac.update(&buf[0..read]); - if self.data_remaining <= 0 { - // if there is no data left to read, check the integrity of the data - let mut read_auth = [0; 10]; - self.reader.read_exact(&mut read_auth)?; - let computed_auth = self.hmac.finalize_reset(); - // FIXME: The mac uses the whole sha1 hash each step - // Zip uses HMAC-Sha1-80, which throws away the second half each time - // see https://www.winzip.com/win/en/aes_info.html#auth-faq - // if computed_auth.into_bytes().as_slice() != &read_auth { - // } + // if there is no data left to read, check the integrity of the data + if self.data_remaining == 0 { + // Zip uses HMAC-Sha1-80, which only uses the first half of the hash + // see https://www.winzip.com/win/en/aes_info.html#auth-faq + let mut read_auth_code = [0; AUTH_CODE_LENGTH]; + self.reader.read_exact(&mut read_auth_code)?; + let computed_auth_code = &self.hmac.finalize_reset().into_bytes()[0..AUTH_CODE_LENGTH]; + + // use constant time comparison to mitigate timing attacks + if !constant_time_eq(computed_auth_code, &read_auth_code) { + return Err( + io::Error::new( + io::ErrorKind::InvalidData, + "Invalid authentication code, this could be due to an invalid password or errors in the data" + ) + ); + } } Ok(read) From 8ffc2d154598f4d2ebf57cec3bea9ae0a2dffb30 Mon Sep 17 00:00:00 2001 From: Lireer Date: Fri, 9 Oct 2020 16:19:16 +0200 Subject: [PATCH 11/37] cargo fmt and clippy --- src/aes_ctr.rs | 2 +- src/read.rs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/aes_ctr.rs b/src/aes_ctr.rs index efda6b77..9f4c2be4 100644 --- a/src/aes_ctr.rs +++ b/src/aes_ctr.rs @@ -97,7 +97,7 @@ where /// Decrypt or encrypt given data. #[inline] fn crypt_in_place(&mut self, mut target: &mut [u8]) { - while target.len() > 0 { + while !target.is_empty() { if self.pos == AES_BLOCK_SIZE { // Note: AES block size is always 16 bytes, same as u128. self.buffer diff --git a/src/read.rs b/src/read.rs index 4b60d246..3c22e5be 100644 --- a/src/read.rs +++ b/src/read.rs @@ -54,6 +54,7 @@ pub struct ZipArchive { comment: Vec, } +#[allow(clippy::large_enum_variant)] enum CryptoReader<'a> { Plaintext(io::Take<&'a mut dyn Read>), ZipCrypto(ZipCryptoReaderValid>), @@ -346,7 +347,7 @@ impl ZipArchive { let mut files = Vec::new(); let mut names_map = HashMap::new(); - if let Err(_) = reader.seek(io::SeekFrom::Start(directory_start)) { + if reader.seek(io::SeekFrom::Start(directory_start)).is_err() { return Err(ZipError::InvalidArchive( "Could not seek to start of central directory", )); From ff23539624c047cc1c60ac256a371cad85b75173 Mon Sep 17 00:00:00 2001 From: Lireer Date: Fri, 9 Oct 2020 17:21:11 +0200 Subject: [PATCH 12/37] differentiate between ae1 and ae2 --- src/crc32.rs | 11 ++++++----- src/read.rs | 46 ++++++++++++++++++++++++++++++---------------- src/types.rs | 10 ++++++++-- 3 files changed, 44 insertions(+), 23 deletions(-) diff --git a/src/crc32.rs b/src/crc32.rs index 5c34f25a..745cd560 100644 --- a/src/crc32.rs +++ b/src/crc32.rs @@ -12,17 +12,18 @@ pub struct Crc32Reader { check: u32, /// Signals if `inner` stores aes encrypted data. /// AE-2 encrypted data doesn't use crc and sets the value to 0. - aes_encrypted: bool, + ae2_encrypted: bool, } impl Crc32Reader { - /// Get a new Crc32Reader which check the inner reader against checksum. - pub fn new(inner: R, checksum: u32, aes_encrypted: bool) -> Crc32Reader { + /// Get a new Crc32Reader which checks the inner reader against checksum. + /// The check is disabled if `ae2_encrypted == true`. + pub fn new(inner: R, checksum: u32, ae2_encrypted: bool) -> Crc32Reader { Crc32Reader { inner, hasher: Hasher::new(), check: checksum, - aes_encrypted, + ae2_encrypted, } } @@ -38,7 +39,7 @@ impl Crc32Reader { impl Read for Crc32Reader { fn read(&mut self, buf: &mut [u8]) -> io::Result { let count = match self.inner.read(buf) { - Ok(0) if !buf.is_empty() && !self.check_matches() && !self.aes_encrypted => { + Ok(0) if !buf.is_empty() && !self.check_matches() && !self.ae2_encrypted => { return Err(io::Error::new(io::ErrorKind::Other, "Invalid checksum")) } Ok(n) => n, diff --git a/src/read.rs b/src/read.rs index 3c22e5be..f837e971 100644 --- a/src/read.rs +++ b/src/read.rs @@ -6,7 +6,7 @@ use crate::cp437::FromCp437; use crate::crc32::Crc32Reader; use crate::result::{InvalidPassword, ZipError, ZipResult}; use crate::spec; -use crate::types::{AesMode, DateTime, System, ZipFileData}; +use crate::types::{AesMode, AesVendorVersion, DateTime, System, ZipFileData}; use crate::zipcrypto::{ZipCryptoReader, ZipCryptoReaderValid, ZipCryptoValidator}; use byteorder::{LittleEndian, ReadBytesExt}; use std::borrow::Cow; @@ -58,7 +58,10 @@ pub struct ZipArchive { enum CryptoReader<'a> { Plaintext(io::Take<&'a mut dyn Read>), ZipCrypto(ZipCryptoReaderValid>), - Aes(AesReaderValid>), + Aes { + reader: AesReaderValid>, + vendor_version: AesVendorVersion, + }, } impl<'a> Read for CryptoReader<'a> { @@ -66,7 +69,7 @@ impl<'a> Read for CryptoReader<'a> { match self { CryptoReader::Plaintext(r) => r.read(buf), CryptoReader::ZipCrypto(r) => r.read(buf), - CryptoReader::Aes(r) => r.read(buf), + CryptoReader::Aes { reader: r, .. } => r.read(buf), } } } @@ -77,7 +80,7 @@ impl<'a> CryptoReader<'a> { match self { CryptoReader::Plaintext(r) => r, CryptoReader::ZipCrypto(r) => r.into_inner(), - CryptoReader::Aes(r) => r.into_inner(), + CryptoReader::Aes { reader: r, .. } => r.into_inner(), } } } @@ -168,7 +171,7 @@ fn make_crypto_reader<'a>( using_data_descriptor: bool, reader: io::Take<&'a mut dyn io::Read>, password: Option<&[u8]>, - aes_mode: Option, + aes_info: Option<(AesMode, AesVendorVersion)>, compressed_size: u64, ) -> ZipResult, InvalidPassword>> { #[allow(deprecated)] @@ -178,7 +181,7 @@ fn make_crypto_reader<'a>( } } - let reader = match (password, aes_mode) { + let reader = match (password, aes_info) { (None, _) => CryptoReader::Plaintext(reader), (Some(password), None) => { let validator = if using_data_descriptor { @@ -191,10 +194,13 @@ fn make_crypto_reader<'a>( Some(r) => CryptoReader::ZipCrypto(r), } } - (Some(password), Some(aes_mode)) => { + (Some(password), Some((aes_mode, vendor_version))) => { match AesReader::new(reader, aes_mode, compressed_size).validate(&password)? { None => return Ok(Err(InvalidPassword)), - Some(r) => CryptoReader::Aes(r), + Some(r) => CryptoReader::Aes { + reader: r, + vendor_version, + }, } } }; @@ -206,10 +212,13 @@ fn make_reader<'a>( crc32: u32, reader: CryptoReader<'a>, ) -> ZipFileReader<'a> { - let aes_encrypted = matches!(reader, CryptoReader::Aes(_)); + let ae2_encrypted = matches!(reader, CryptoReader::Aes { + vendor_version: AesVendorVersion::Ae2, + .. + }); match compression_method { CompressionMethod::Stored => { - ZipFileReader::Stored(Crc32Reader::new(reader, crc32, aes_encrypted)) + ZipFileReader::Stored(Crc32Reader::new(reader, crc32, ae2_encrypted)) } #[cfg(any( feature = "deflate", @@ -218,12 +227,12 @@ fn make_reader<'a>( ))] CompressionMethod::Deflated => { let deflate_reader = DeflateDecoder::new(reader); - ZipFileReader::Deflated(Crc32Reader::new(deflate_reader, crc32, aes_encrypted)) + ZipFileReader::Deflated(Crc32Reader::new(deflate_reader, crc32, ae2_encrypted)) } #[cfg(feature = "bzip2")] CompressionMethod::Bzip2 => { let bzip2_reader = BzDecoder::new(reader); - ZipFileReader::Bzip2(Crc32Reader::new(bzip2_reader, crc32, aes_encrypted)) + ZipFileReader::Bzip2(Crc32Reader::new(bzip2_reader, crc32, ae2_encrypted)) } _ => panic!("Compression method not supported"), } @@ -665,7 +674,7 @@ fn parse_extra_field(file: &mut ZipFileData) -> ZipResult<()> { "AES extra data field has an unsupported length", )); } - let _vendor_version = reader.read_u16::()?; // TODO: CRC value handling changes + let vendor_version = reader.read_u16::()?; let vendor_id = reader.read_u16::()?; let aes_mode = reader.read_u8()?; let compression_method = reader.read_u16::()?; @@ -673,10 +682,15 @@ fn parse_extra_field(file: &mut ZipFileData) -> ZipResult<()> { if vendor_id != 0x4541 { return Err(ZipError::InvalidArchive("Invalid AES vendor")); } + let vendor_version = match vendor_version { + 0x0001 => AesVendorVersion::Ae1, + 0x0002 => AesVendorVersion::Ae2, + _ => return Err(ZipError::InvalidArchive("Invalid AES vendor version")), + }; match aes_mode { - 0x01 => file.aes_mode = Some(AesMode::Aes128), - 0x02 => file.aes_mode = Some(AesMode::Aes192), - 0x03 => file.aes_mode = Some(AesMode::Aes256), + 0x01 => file.aes_mode = Some((AesMode::Aes128, vendor_version)), + 0x02 => file.aes_mode = Some((AesMode::Aes192, vendor_version)), + 0x03 => file.aes_mode = Some((AesMode::Aes256, vendor_version)), _ => return Err(ZipError::InvalidArchive("Invalid AES encryption strength")), }; file.compression_method = { diff --git a/src/types.rs b/src/types.rs index d2a7645d..9d9371bc 100644 --- a/src/types.rs +++ b/src/types.rs @@ -249,7 +249,7 @@ pub struct ZipFileData { /// Reserve local ZIP64 extra field pub large_file: bool, /// AES mode if applicable - pub aes_mode: Option, + pub aes_mode: Option<(AesMode, AesVendorVersion)>, } impl ZipFileData { @@ -300,8 +300,14 @@ impl ZipFileData { } } +#[derive(Copy, Clone, Debug)] +pub enum AesVendorVersion { + Ae1, + Ae2, +} + /// AES variant used. -#[derive(Copy, Clone, Debug, PartialEq)] +#[derive(Copy, Clone, Debug)] pub enum AesMode { Aes128, Aes192, From 2911282c36c9fc23d662951c16286378378b3a33 Mon Sep 17 00:00:00 2001 From: Lireer Date: Fri, 9 Oct 2020 17:21:21 +0200 Subject: [PATCH 13/37] fix benchmarks --- benches/read_entry.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/benches/read_entry.rs b/benches/read_entry.rs index 25c0b94a..84398316 100644 --- a/benches/read_entry.rs +++ b/benches/read_entry.rs @@ -3,7 +3,7 @@ use bencher::{benchmark_group, benchmark_main}; use std::io::{Cursor, Read, Write}; use bencher::Bencher; -use rand::Rng; +use rand::RngCore; use zip::{ZipArchive, ZipWriter}; fn generate_random_archive(size: usize) -> Vec { From 0820cc4fe2db48fd87686cffc342ebbd01ba3e81 Mon Sep 17 00:00:00 2001 From: Lireer Date: Sat, 10 Oct 2020 18:13:14 +0200 Subject: [PATCH 14/37] fix more clippy warnings --- src/read.rs | 5 ++--- src/types.rs | 5 +---- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/read.rs b/src/read.rs index f837e971..a048c3b3 100644 --- a/src/read.rs +++ b/src/read.rs @@ -1112,9 +1112,8 @@ mod test { v.extend_from_slice(include_bytes!("../tests/data/mimetype.zip")); let mut reader = io::Cursor::new(v); loop { - match read_zipfile_from_stream(&mut reader).unwrap() { - None => break, - _ => (), + if read_zipfile_from_stream(&mut reader).unwrap().is_none() { + break; } } } diff --git a/src/types.rs b/src/types.rs index 9d9371bc..23dfffb8 100644 --- a/src/types.rs +++ b/src/types.rs @@ -273,10 +273,7 @@ impl ZipFileData { ::std::path::Path::new(&filename) .components() - .filter(|component| match *component { - ::std::path::Component::Normal(..) => true, - _ => false, - }) + .filter(|component| matches!(*component, ::std::path::Component::Normal(..))) .fold(::std::path::PathBuf::new(), |mut path, ref cur| { path.push(cur.as_os_str()); path From 354993d906afbaaf806a10b1c713f24f0aaabbd7 Mon Sep 17 00:00:00 2001 From: Lireer Date: Sat, 10 Oct 2020 19:38:58 +0200 Subject: [PATCH 15/37] feature gate aes decryption --- Cargo.toml | 11 +++---- src/aes.rs | 34 +++++++++++++++++++-- src/aes_ctr.rs | 7 ----- src/crc32.rs | 42 ++++++++++++++++++++++---- src/lib.rs | 2 ++ src/read.rs | 81 +++++++++++++++++++++++++++++++++++--------------- src/types.rs | 33 ++++---------------- src/write.rs | 1 + 8 files changed, 139 insertions(+), 72 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index b6c2fc3b..42b560f4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,15 +11,15 @@ Library to support the reading and writing of zip files. edition = "2018" [dependencies] -aes = "0.5.0" +aes = { version = "0.5.0", optional = true } byteorder = "1.3" bzip2 = { version = "0.4", optional = true } -constant_time_eq = "0.1.5" +constant_time_eq = { version = "0.1.5", optional = true } crc32fast = "1.0" flate2 = { version = "1.0.0", default-features = false, optional = true } -hmac = "0.9.0" -pbkdf2 = "0.5.0" -sha-1 = "0.9.1" +hmac = {version = "0.9.0", optional = true } +pbkdf2 = {version = "0.5.0", optional = true } +sha-1 = {version = "0.9.1", optional = true } thiserror = "1.0" time = { version = "0.1", optional = true } @@ -29,6 +29,7 @@ rand = "0.7" walkdir = "2" [features] +aes-crypto = [ "aes", "constant_time_eq", "hmac", "pbkdf2", "sha-1" ] deflate = ["flate2/rust_backend"] deflate-miniz = ["flate2/default"] deflate-zlib = ["flate2/zlib"] diff --git a/src/aes.rs b/src/aes.rs index e727285a..7847df01 100644 --- a/src/aes.rs +++ b/src/aes.rs @@ -1,5 +1,4 @@ use crate::aes_ctr; -use crate::types::AesMode; use constant_time_eq::constant_time_eq; use hmac::{Hmac, Mac, NewMac}; use sha1::Sha1; @@ -23,7 +22,38 @@ fn cipher_from_mode(aes_mode: AesMode, key: &[u8]) -> Box usize { + self.key_length() / 2 + } + + pub fn key_length(&self) -> usize { + match self { + Self::Aes128 => 16, + Self::Aes192 => 24, + Self::Aes256 => 32, + } + } +} + +// An aes encrypted file starts with a salt, whose length depends on the used aes mode // followed by a 2 byte password verification value // then the variable length encrypted data // and lastly a 10 byte authentication code diff --git a/src/aes_ctr.rs b/src/aes_ctr.rs index 9f4c2be4..1497a287 100644 --- a/src/aes_ctr.rs +++ b/src/aes_ctr.rs @@ -152,7 +152,6 @@ mod tests { 0x18, 0x55, 0x24, 0xa3, 0x9e, 0x0e, 0x40, 0xe7, 0x92, 0xad, 0xb2, 0x8a, 0x48, 0xf4, 0x5c, 0xd0, 0xc0, 0x54, ]; - eprintln!("{}", key.len()); let mut key_stream = AesCtrZipKeyStream::::new(&key); @@ -174,7 +173,6 @@ mod tests { 0xe0, 0x25, 0x7b, 0x57, 0x97, 0x6a, 0xa4, 0x23, 0xab, 0x94, 0xaa, 0x44, 0xfd, 0x47, 0x4f, 0xa5, ]; - eprintln!("{}", key.len()); let mut key_stream = AesCtrZipKeyStream::::new(&key); @@ -196,7 +194,6 @@ mod tests { 0xe4, 0x4a, 0x88, 0x52, 0x8f, 0xf7, 0x0b, 0x81, 0x7b, 0x75, 0xf1, 0x74, 0x21, 0x37, 0x8c, 0x90, 0xad, 0xbe, 0x4a, 0x65, 0xa8, 0x96, 0x0e, 0xcc, ]; - eprintln!("{}", key.len()); let mut key_stream = AesCtrZipKeyStream::::new(&key); @@ -219,7 +216,6 @@ mod tests { 0xa5, 0xee, 0x3a, 0x4f, 0x0f, 0x4b, 0x29, 0xbd, 0xe9, 0xb8, 0x41, 0x9c, 0x41, 0xa5, 0x15, 0xb2, 0x86, 0xab, ]; - eprintln!("{}", key.len()); let mut key_stream = AesCtrZipKeyStream::::new(&key); @@ -245,7 +241,6 @@ mod tests { 0x43, 0x2b, 0x6d, 0xbe, 0x05, 0x76, 0x6c, 0x9e, 0xde, 0xca, 0x3b, 0xf8, 0xaf, 0x5d, 0x81, 0xb6, ]; - eprintln!("{}", key.len()); let mut key_stream = AesCtrZipKeyStream::::new(&key); @@ -271,7 +266,6 @@ mod tests { 0xac, 0x92, 0x41, 0xba, 0xde, 0xd9, 0x02, 0xfe, 0x40, 0x92, 0x20, 0xf6, 0x56, 0x03, 0xfe, 0xae, 0x1b, 0xba, 0x01, 0x97, 0x97, 0x79, 0xbb, 0xa6, ]; - eprintln!("{}", key.len()); let mut key_stream = AesCtrZipKeyStream::::new(&key); @@ -298,7 +292,6 @@ mod tests { 0xe1, 0x4d, 0x4a, 0x77, 0xd4, 0xeb, 0x9e, 0x3d, 0x75, 0xce, 0x9a, 0x3e, 0x10, 0x50, 0xc2, 0x07, 0x36, 0xb6, ]; - eprintln!("{}", key.len()); let mut key_stream = AesCtrZipKeyStream::::new(&key); diff --git a/src/crc32.rs b/src/crc32.rs index 745cd560..22c262a6 100644 --- a/src/crc32.rs +++ b/src/crc32.rs @@ -10,6 +10,7 @@ pub struct Crc32Reader { inner: R, hasher: Hasher, check: u32, + #[cfg(feature = "aes-crypto")] /// Signals if `inner` stores aes encrypted data. /// AE-2 encrypted data doesn't use crc and sets the value to 0. ae2_encrypted: bool, @@ -18,11 +19,16 @@ pub struct Crc32Reader { impl Crc32Reader { /// Get a new Crc32Reader which checks the inner reader against checksum. /// The check is disabled if `ae2_encrypted == true`. - pub fn new(inner: R, checksum: u32, ae2_encrypted: bool) -> Crc32Reader { + pub(crate) fn new( + inner: R, + checksum: u32, + #[cfg(feature = "aes-crypto")] ae2_encrypted: bool, + ) -> Crc32Reader { Crc32Reader { inner, hasher: Hasher::new(), check: checksum, + #[cfg(feature = "aes-crypto")] ae2_encrypted, } } @@ -38,8 +44,12 @@ impl Crc32Reader { impl Read for Crc32Reader { fn read(&mut self, buf: &mut [u8]) -> io::Result { + let invalid_check = !buf.is_empty() && !self.check_matches(); + #[cfg(feature = "aes-crypto")] + let invalid_check = invalid_check && !self.ae2_encrypted; + let count = match self.inner.read(buf) { - Ok(0) if !buf.is_empty() && !self.check_matches() && !self.ae2_encrypted => { + Ok(0) if invalid_check => { return Err(io::Error::new(io::ErrorKind::Other, "Invalid checksum")) } Ok(n) => n, @@ -60,10 +70,20 @@ mod test { let data: &[u8] = b""; let mut buf = [0; 1]; - let mut reader = Crc32Reader::new(data, 0, false); + let mut reader = Crc32Reader::new( + data, + 0, + #[cfg(feature = "aes-crypto")] + false, + ); assert_eq!(reader.read(&mut buf).unwrap(), 0); - let mut reader = Crc32Reader::new(data, 1, false); + let mut reader = Crc32Reader::new( + data, + 1, + #[cfg(feature = "aes-crypto")] + false, + ); assert!(reader .read(&mut buf) .unwrap_err() @@ -76,7 +96,12 @@ mod test { let data: &[u8] = b"1234"; let mut buf = [0; 1]; - let mut reader = Crc32Reader::new(data, 0x9be3e0a3, false); + let mut reader = Crc32Reader::new( + data, + 0x9be3e0a3, + #[cfg(feature = "aes-crypto")] + false, + ); assert_eq!(reader.read(&mut buf).unwrap(), 1); assert_eq!(reader.read(&mut buf).unwrap(), 1); assert_eq!(reader.read(&mut buf).unwrap(), 1); @@ -91,7 +116,12 @@ mod test { let data: &[u8] = b"1234"; let mut buf = [0; 5]; - let mut reader = Crc32Reader::new(data, 0x9be3e0a3, false); + let mut reader = Crc32Reader::new( + data, + 0x9be3e0a3, + #[cfg(feature = "aes-crypto")] + false, + ); assert_eq!(reader.read(&mut buf[..0]).unwrap(), 0); assert_eq!(reader.read(&mut buf).unwrap(), 4); } diff --git a/src/lib.rs b/src/lib.rs index 45c6c2e1..1abf42ec 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -10,7 +10,9 @@ pub use crate::read::ZipArchive; pub use crate::types::DateTime; pub use crate::write::ZipWriter; +#[cfg(feature = "aes-crypto")] mod aes; +#[cfg(feature = "aes-crypto")] mod aes_ctr; mod compression; mod cp437; diff --git a/src/read.rs b/src/read.rs index a048c3b3..59191001 100644 --- a/src/read.rs +++ b/src/read.rs @@ -1,12 +1,13 @@ //! Types for reading ZIP archives -use crate::aes::{AesReader, AesReaderValid}; +#[cfg(feature = "aes-crypto")] +use crate::aes::{AesMode, AesReader, AesReaderValid, AesVendorVersion}; use crate::compression::CompressionMethod; use crate::cp437::FromCp437; use crate::crc32::Crc32Reader; use crate::result::{InvalidPassword, ZipError, ZipResult}; use crate::spec; -use crate::types::{AesMode, AesVendorVersion, DateTime, System, ZipFileData}; +use crate::types::{DateTime, System, ZipFileData}; use crate::zipcrypto::{ZipCryptoReader, ZipCryptoReaderValid, ZipCryptoValidator}; use byteorder::{LittleEndian, ReadBytesExt}; use std::borrow::Cow; @@ -58,6 +59,7 @@ pub struct ZipArchive { enum CryptoReader<'a> { Plaintext(io::Take<&'a mut dyn Read>), ZipCrypto(ZipCryptoReaderValid>), + #[cfg(feature = "aes-crypto")] Aes { reader: AesReaderValid>, vendor_version: AesVendorVersion, @@ -69,6 +71,7 @@ impl<'a> Read for CryptoReader<'a> { match self { CryptoReader::Plaintext(r) => r.read(buf), CryptoReader::ZipCrypto(r) => r.read(buf), + #[cfg(feature = "aes-crypto")] CryptoReader::Aes { reader: r, .. } => r.read(buf), } } @@ -80,6 +83,7 @@ impl<'a> CryptoReader<'a> { match self { CryptoReader::Plaintext(r) => r, CryptoReader::ZipCrypto(r) => r.into_inner(), + #[cfg(feature = "aes-crypto")] CryptoReader::Aes { reader: r, .. } => r.into_inner(), } } @@ -171,8 +175,8 @@ fn make_crypto_reader<'a>( using_data_descriptor: bool, reader: io::Take<&'a mut dyn io::Read>, password: Option<&[u8]>, - aes_info: Option<(AesMode, AesVendorVersion)>, - compressed_size: u64, + #[cfg(feature = "aes-crypto")] aes_info: Option<(AesMode, AesVendorVersion)>, + #[cfg(feature = "aes-crypto")] compressed_size: u64, ) -> ZipResult, InvalidPassword>> { #[allow(deprecated)] { @@ -181,8 +185,20 @@ fn make_crypto_reader<'a>( } } + #[cfg(not(feature = "aes-crypto"))] + let aes_info: Option<()> = None; + let reader = match (password, aes_info) { - (None, _) => CryptoReader::Plaintext(reader), + #[cfg(feature = "aes-crypto")] + (Some(password), Some((aes_mode, vendor_version))) => { + match AesReader::new(reader, aes_mode, compressed_size).validate(&password)? { + None => return Ok(Err(InvalidPassword)), + Some(r) => CryptoReader::Aes { + reader: r, + vendor_version, + }, + } + } (Some(password), None) => { let validator = if using_data_descriptor { ZipCryptoValidator::InfoZipMsdosTime(last_modified_time.timepart()) @@ -194,15 +210,7 @@ fn make_crypto_reader<'a>( Some(r) => CryptoReader::ZipCrypto(r), } } - (Some(password), Some((aes_mode, vendor_version))) => { - match AesReader::new(reader, aes_mode, compressed_size).validate(&password)? { - None => return Ok(Err(InvalidPassword)), - Some(r) => CryptoReader::Aes { - reader: r, - vendor_version, - }, - } - } + _ => CryptoReader::Plaintext(reader), }; Ok(Ok(reader)) } @@ -212,14 +220,19 @@ fn make_reader<'a>( crc32: u32, reader: CryptoReader<'a>, ) -> ZipFileReader<'a> { + #[cfg(feature = "aes-crypto")] let ae2_encrypted = matches!(reader, CryptoReader::Aes { vendor_version: AesVendorVersion::Ae2, .. }); + match compression_method { - CompressionMethod::Stored => { - ZipFileReader::Stored(Crc32Reader::new(reader, crc32, ae2_encrypted)) - } + CompressionMethod::Stored => ZipFileReader::Stored(Crc32Reader::new( + reader, + crc32, + #[cfg(feature = "aes-crypto")] + ae2_encrypted, + )), #[cfg(any( feature = "deflate", feature = "deflate-miniz", @@ -227,12 +240,22 @@ fn make_reader<'a>( ))] CompressionMethod::Deflated => { let deflate_reader = DeflateDecoder::new(reader); - ZipFileReader::Deflated(Crc32Reader::new(deflate_reader, crc32, ae2_encrypted)) + ZipFileReader::Deflated(Crc32Reader::new( + deflate_reader, + crc32, + #[cfg(feature = "aes-crypto")] + ae2_encrypted, + )) } #[cfg(feature = "bzip2")] CompressionMethod::Bzip2 => { let bzip2_reader = BzDecoder::new(reader); - ZipFileReader::Bzip2(Crc32Reader::new(bzip2_reader, crc32, ae2_encrypted)) + ZipFileReader::Bzip2(Crc32Reader::new( + bzip2_reader, + crc32, + #[cfg(feature = "aes-crypto")] + ae2_encrypted, + )) } _ => panic!("Compression method not supported"), } @@ -526,7 +549,9 @@ impl ZipArchive { data.using_data_descriptor, limit_reader, password, + #[cfg(feature = "aes-crypto")] data.aes_mode, + #[cfg(feature = "aes-crypto")] data.compressed_size, ) { Ok(Ok(crypto_reader)) => Ok(Ok(ZipFile { @@ -621,6 +646,7 @@ pub(crate) fn central_header_to_zip_file( data_start: 0, external_attributes: external_file_attributes, large_file: false, + #[cfg(feature = "aes-crypto")] aes_mode: None, }; @@ -629,11 +655,14 @@ pub(crate) fn central_header_to_zip_file( Err(e) => return Err(e), } - let aes_enabled = result.compression_method == CompressionMethod::AES; - if aes_enabled && result.aes_mode.is_none() { - return Err(ZipError::InvalidArchive( - "AES encryption without AES extra data field", - )); + #[cfg(feature = "aes-crypto")] + { + let aes_enabled = result.compression_method == CompressionMethod::AES; + if aes_enabled && result.aes_mode.is_none() { + return Err(ZipError::InvalidArchive( + "AES encryption without AES extra data field", + )); + } } // Account for shifted zip offsets. @@ -667,6 +696,7 @@ fn parse_extra_field(file: &mut ZipFileData) -> ZipResult<()> { len_left -= 8; } } + #[cfg(feature = "aes-crypto")] 0x9901 => { // AES if len != 7 { @@ -1018,6 +1048,7 @@ pub fn read_zipfile_from_stream<'a, R: io::Read>( // from standard input, this field is set to zero.' external_attributes: 0, large_file: false, + #[cfg(feature = "aes-crypto")] aes_mode: None, }; @@ -1044,7 +1075,9 @@ pub fn read_zipfile_from_stream<'a, R: io::Read>( result.using_data_descriptor, limit_reader, None, + #[cfg(feature = "aes-crypto")] None, + #[cfg(feature = "aes-crypto")] result.compressed_size, )? .unwrap(); diff --git a/src/types.rs b/src/types.rs index 23dfffb8..d54ab75a 100644 --- a/src/types.rs +++ b/src/types.rs @@ -1,5 +1,8 @@ //! Types that specify what is contained in a ZIP. +#[cfg(feature = "aes-crypto")] +use crate::aes::{AesMode, AesVendorVersion}; + #[derive(Clone, Copy, Debug, PartialEq)] pub enum System { Dos = 0, @@ -248,6 +251,7 @@ pub struct ZipFileData { pub external_attributes: u32, /// Reserve local ZIP64 extra field pub large_file: bool, + #[cfg(feature = "aes-crypto")] /// AES mode if applicable pub aes_mode: Option<(AesMode, AesVendorVersion)>, } @@ -297,34 +301,6 @@ impl ZipFileData { } } -#[derive(Copy, Clone, Debug)] -pub enum AesVendorVersion { - Ae1, - Ae2, -} - -/// AES variant used. -#[derive(Copy, Clone, Debug)] -pub enum AesMode { - Aes128, - Aes192, - Aes256, -} - -impl AesMode { - pub fn salt_length(&self) -> usize { - self.key_length() / 2 - } - - pub fn key_length(&self) -> usize { - match self { - Self::Aes128 => 16, - Self::Aes192 => 24, - Self::Aes256 => 32, - } - } -} - #[cfg(test)] mod test { #[test] @@ -359,6 +335,7 @@ mod test { central_header_start: 0, external_attributes: 0, large_file: false, + #[cfg(feature = "aes-crypto")] aes_mode: None, }; assert_eq!( diff --git a/src/write.rs b/src/write.rs index 05236505..2e85127c 100644 --- a/src/write.rs +++ b/src/write.rs @@ -334,6 +334,7 @@ impl ZipWriter { central_header_start: 0, external_attributes: permissions << 16, large_file: options.large_file, + #[cfg(feature = "aes-crypto")] aes_mode: None, }; write_local_file_header(writer, &file)?; From 5532fd6f09eb4c9d61a05dacf85d3b62677a332e Mon Sep 17 00:00:00 2001 From: Lireer Date: Wed, 14 Oct 2020 15:08:10 +0200 Subject: [PATCH 16/37] Document aes related modules --- src/aes.rs | 6 ++++++ src/aes_ctr.rs | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/src/aes.rs b/src/aes.rs index 7847df01..8baa796c 100644 --- a/src/aes.rs +++ b/src/aes.rs @@ -1,3 +1,9 @@ +//! Implementation of the AES decryption for zip files. +//! +//! This was implemented according to the [WinZip specification](https://www.winzip.com/win/en/aes_info.html). +//! Note that using CRC with AES depends on the specific encryption specification used, AE-1 or AE-2. +//! AE-2 doesn't set the CRC field correctly, even though some zip files still have CRC set even with AE-2. + use crate::aes_ctr; use constant_time_eq::constant_time_eq; use hmac::{Hmac, Mac, NewMac}; diff --git a/src/aes_ctr.rs b/src/aes_ctr.rs index 1497a287..e23eb3bb 100644 --- a/src/aes_ctr.rs +++ b/src/aes_ctr.rs @@ -1,3 +1,9 @@ +//! A counter mode (CTR) for AES to work with the encryption used in zip files. +//! +//! This was implemented since the zip specification requires the mode to not use a nonce and uses a +//! different byte order (little endian) than NIST (big endian). +//! See [AesCtrZipKeyStream](./struct.AesCtrZipKeyStream.html) for more information. + use aes::block_cipher::generic_array::GenericArray; use aes::{BlockCipher, NewBlockCipher}; use byteorder::WriteBytesExt; From 5f0ae55eaeabe02f8f9d197bbf6a830773c05844 Mon Sep 17 00:00:00 2001 From: Lireer Date: Wed, 14 Oct 2020 15:36:23 +0200 Subject: [PATCH 17/37] Document possible panics --- src/aes.rs | 10 ++++++++++ src/aes_ctr.rs | 4 ++++ 2 files changed, 14 insertions(+) diff --git a/src/aes.rs b/src/aes.rs index 8baa796c..7f8bc19e 100644 --- a/src/aes.rs +++ b/src/aes.rs @@ -17,6 +17,11 @@ const AUTH_CODE_LENGTH: usize = 10; /// The number of iterations used with PBKDF2 const ITERATION_COUNT: u32 = 1000; +/// Create a AesCipher depending on the used `AesMode` and the given `key`. +/// +/// # Panics +/// +/// This panics if `key` doesn't have the correct size for the chosen aes mode. fn cipher_from_mode(aes_mode: AesMode, key: &[u8]) -> Box { match aes_mode { AesMode::Aes128 => Box::new(aes_ctr::AesCtrZipKeyStream::::new(key)) @@ -127,6 +132,11 @@ impl AesReader { } } +/// A reader for aes encrypted files, which has already passed the first password check. +/// +/// There is a 1 in 65536 chance that an invalid password passes that check. +/// After the data has been read and decrypted an HMAC will be checked and provide a final means +/// to check if either the password is invalid or if the data has been changed. pub struct AesReaderValid { reader: R, data_remaining: u64, diff --git a/src/aes_ctr.rs b/src/aes_ctr.rs index e23eb3bb..307c7530 100644 --- a/src/aes_ctr.rs +++ b/src/aes_ctr.rs @@ -85,6 +85,10 @@ where C::Cipher: NewBlockCipher, { /// Creates a new zip variant AES-CTR key stream. + /// + /// # Panics + /// + /// This panics if `key` doesn't have the correct size for cipher `C`. pub fn new(key: &[u8]) -> AesCtrZipKeyStream { AesCtrZipKeyStream { counter: 1, From ed94e8b36944f1077fdd08c32c54234c7e50b3ec Mon Sep 17 00:00:00 2001 From: Lireer Date: Wed, 14 Oct 2020 16:02:03 +0200 Subject: [PATCH 18/37] test if using the wrong key size panics --- src/aes_ctr.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/aes_ctr.rs b/src/aes_ctr.rs index 307c7530..bba4ffe0 100644 --- a/src/aes_ctr.rs +++ b/src/aes_ctr.rs @@ -150,6 +150,12 @@ fn xor(dest: &mut [u8], src: &[u8]) { mod tests { use super::{Aes128, Aes192, Aes256, AesCipher, AesCtrZipKeyStream}; + #[test] + #[should_panic] + fn new_with_wrong_key_size() { + AesCtrZipKeyStream::::new(&[1, 2, 3, 4, 5]); + } + // The data used in these tests was generated with p7zip without any compression. // It's not possible to recreate the exact same data, since a random salt is used for encryption. // `7z a -phelloworld -mem=AES256 -mx=0 aes256_40byte.zip 40byte_data.txt` From 48b52a7e8606ca4c057a0f2f68ef718af618d0c5 Mon Sep 17 00:00:00 2001 From: Lireer Date: Wed, 14 Oct 2020 16:06:56 +0200 Subject: [PATCH 19/37] move AesMode and AesVendorVersion out of aes-crypto feature --- src/aes.rs | 34 ++-------------------------------- src/read.rs | 23 +++++++++++------------ src/types.rs | 38 +++++++++++++++++++++++++++++++++----- src/write.rs | 1 - 4 files changed, 46 insertions(+), 50 deletions(-) diff --git a/src/aes.rs b/src/aes.rs index 7f8bc19e..5fa636f2 100644 --- a/src/aes.rs +++ b/src/aes.rs @@ -5,6 +5,7 @@ //! AE-2 doesn't set the CRC field correctly, even though some zip files still have CRC set even with AE-2. use crate::aes_ctr; +use crate::types::AesMode; use constant_time_eq::constant_time_eq; use hmac::{Hmac, Mac, NewMac}; use sha1::Sha1; @@ -18,7 +19,7 @@ const AUTH_CODE_LENGTH: usize = 10; const ITERATION_COUNT: u32 = 1000; /// Create a AesCipher depending on the used `AesMode` and the given `key`. -/// +/// /// # Panics /// /// This panics if `key` doesn't have the correct size for the chosen aes mode. @@ -33,37 +34,6 @@ fn cipher_from_mode(aes_mode: AesMode, key: &[u8]) -> Box usize { - self.key_length() / 2 - } - - pub fn key_length(&self) -> usize { - match self { - Self::Aes128 => 16, - Self::Aes192 => 24, - Self::Aes256 => 32, - } - } -} - // An aes encrypted file starts with a salt, whose length depends on the used aes mode // followed by a 2 byte password verification value // then the variable length encrypted data diff --git a/src/read.rs b/src/read.rs index 59191001..80b6e7d7 100644 --- a/src/read.rs +++ b/src/read.rs @@ -1,13 +1,13 @@ //! Types for reading ZIP archives #[cfg(feature = "aes-crypto")] -use crate::aes::{AesMode, AesReader, AesReaderValid, AesVendorVersion}; +use crate::aes::{AesReader, AesReaderValid}; use crate::compression::CompressionMethod; use crate::cp437::FromCp437; use crate::crc32::Crc32Reader; use crate::result::{InvalidPassword, ZipError, ZipResult}; use crate::spec; -use crate::types::{DateTime, System, ZipFileData}; +use crate::types::{AesMode, AesVendorVersion, DateTime, System, ZipFileData}; use crate::zipcrypto::{ZipCryptoReader, ZipCryptoReaderValid, ZipCryptoValidator}; use byteorder::{LittleEndian, ReadBytesExt}; use std::borrow::Cow; @@ -175,7 +175,7 @@ fn make_crypto_reader<'a>( using_data_descriptor: bool, reader: io::Take<&'a mut dyn io::Read>, password: Option<&[u8]>, - #[cfg(feature = "aes-crypto")] aes_info: Option<(AesMode, AesVendorVersion)>, + aes_info: Option<(AesMode, AesVendorVersion)>, #[cfg(feature = "aes-crypto")] compressed_size: u64, ) -> ZipResult, InvalidPassword>> { #[allow(deprecated)] @@ -185,10 +185,13 @@ fn make_crypto_reader<'a>( } } - #[cfg(not(feature = "aes-crypto"))] - let aes_info: Option<()> = None; - let reader = match (password, aes_info) { + #[cfg(not(feature = "aes-crypto"))] + (Some(_), Some(_)) => { + return Err(ZipError::UnsupportedArchive( + "AES encrypted files cannot be decrypted without the aes-crypto feature.", + )) + } #[cfg(feature = "aes-crypto")] (Some(password), Some((aes_mode, vendor_version))) => { match AesReader::new(reader, aes_mode, compressed_size).validate(&password)? { @@ -210,7 +213,8 @@ fn make_crypto_reader<'a>( Some(r) => CryptoReader::ZipCrypto(r), } } - _ => CryptoReader::Plaintext(reader), + (None, Some(_)) => return Ok(Err(InvalidPassword)), + (None, None) => CryptoReader::Plaintext(reader), }; Ok(Ok(reader)) } @@ -549,7 +553,6 @@ impl ZipArchive { data.using_data_descriptor, limit_reader, password, - #[cfg(feature = "aes-crypto")] data.aes_mode, #[cfg(feature = "aes-crypto")] data.compressed_size, @@ -646,7 +649,6 @@ pub(crate) fn central_header_to_zip_file( data_start: 0, external_attributes: external_file_attributes, large_file: false, - #[cfg(feature = "aes-crypto")] aes_mode: None, }; @@ -696,7 +698,6 @@ fn parse_extra_field(file: &mut ZipFileData) -> ZipResult<()> { len_left -= 8; } } - #[cfg(feature = "aes-crypto")] 0x9901 => { // AES if len != 7 { @@ -1048,7 +1049,6 @@ pub fn read_zipfile_from_stream<'a, R: io::Read>( // from standard input, this field is set to zero.' external_attributes: 0, large_file: false, - #[cfg(feature = "aes-crypto")] aes_mode: None, }; @@ -1075,7 +1075,6 @@ pub fn read_zipfile_from_stream<'a, R: io::Read>( result.using_data_descriptor, limit_reader, None, - #[cfg(feature = "aes-crypto")] None, #[cfg(feature = "aes-crypto")] result.compressed_size, diff --git a/src/types.rs b/src/types.rs index d54ab75a..4f6ef75b 100644 --- a/src/types.rs +++ b/src/types.rs @@ -1,8 +1,5 @@ //! Types that specify what is contained in a ZIP. -#[cfg(feature = "aes-crypto")] -use crate::aes::{AesMode, AesVendorVersion}; - #[derive(Clone, Copy, Debug, PartialEq)] pub enum System { Dos = 0, @@ -251,7 +248,6 @@ pub struct ZipFileData { pub external_attributes: u32, /// Reserve local ZIP64 extra field pub large_file: bool, - #[cfg(feature = "aes-crypto")] /// AES mode if applicable pub aes_mode: Option<(AesMode, AesVendorVersion)>, } @@ -301,6 +297,39 @@ impl ZipFileData { } } +/// The encryption specification used to encrypt a file with AES. +/// +/// According to the [specification](https://www.winzip.com/win/en/aes_info.html#winzip11) AE-2 +/// does not make use of the CRC check. +#[derive(Copy, Clone, Debug)] +pub enum AesVendorVersion { + Ae1, + Ae2, +} + +/// AES variant used. +#[derive(Copy, Clone, Debug)] +pub enum AesMode { + Aes128, + Aes192, + Aes256, +} + +#[cfg(feature = "aes-crypto")] +impl AesMode { + pub fn salt_length(&self) -> usize { + self.key_length() / 2 + } + + pub fn key_length(&self) -> usize { + match self { + Self::Aes128 => 16, + Self::Aes192 => 24, + Self::Aes256 => 32, + } + } +} + #[cfg(test)] mod test { #[test] @@ -335,7 +364,6 @@ mod test { central_header_start: 0, external_attributes: 0, large_file: false, - #[cfg(feature = "aes-crypto")] aes_mode: None, }; assert_eq!( diff --git a/src/write.rs b/src/write.rs index 2e85127c..05236505 100644 --- a/src/write.rs +++ b/src/write.rs @@ -334,7 +334,6 @@ impl ZipWriter { central_header_start: 0, external_attributes: permissions << 16, large_file: options.large_file, - #[cfg(feature = "aes-crypto")] aes_mode: None, }; write_local_file_header(writer, &file)?; From 8f352c30f1233c2bdd63a0f0fdb2d91a12326aa9 Mon Sep 17 00:00:00 2001 From: Lireer Date: Wed, 14 Oct 2020 16:26:39 +0200 Subject: [PATCH 20/37] add missing documentation --- src/aes.rs | 5 +++++ src/aes_ctr.rs | 1 + 2 files changed, 6 insertions(+) diff --git a/src/aes.rs b/src/aes.rs index 5fa636f2..001a2804 100644 --- a/src/aes.rs +++ b/src/aes.rs @@ -62,6 +62,11 @@ impl AesReader { /// password was provided. /// It isn't possible to check the authentication code in this step. This will be done after /// reading and decrypting the file. + /// + /// # Returns + /// + /// If the password verification failed `Ok(None)` will be returned to match the validate + /// method of ZipCryptoReader. pub fn validate(mut self, password: &[u8]) -> io::Result>> { let salt_length = self.aes_mode.salt_length(); let key_length = self.aes_mode.key_length(); diff --git a/src/aes_ctr.rs b/src/aes_ctr.rs index bba4ffe0..c0391277 100644 --- a/src/aes_ctr.rs +++ b/src/aes_ctr.rs @@ -132,6 +132,7 @@ where } } +/// This trait allows using generic AES ciphers with different key sizes. pub trait AesCipher { fn crypt_in_place(&mut self, target: &mut [u8]); } From 75e8f6bab5a6525014f6f52c6eb608ab46de48af Mon Sep 17 00:00:00 2001 From: Lireer Date: Wed, 14 Oct 2020 16:29:08 +0200 Subject: [PATCH 21/37] use less feature gates if no further dependencies are needed --- src/compression.rs | 6 +++--- src/crc32.rs | 40 ++++++------------------------------- src/read.rs | 50 ++++++++++++++++++---------------------------- src/write.rs | 2 +- 4 files changed, 29 insertions(+), 69 deletions(-) diff --git a/src/compression.rs b/src/compression.rs index 84e69d15..83e0b45b 100644 --- a/src/compression.rs +++ b/src/compression.rs @@ -27,7 +27,7 @@ pub enum CompressionMethod { /// Encrypted using AES. /// The actual compression method has to be taken from the AES extra data field /// or from `ZipFileData`. - AES, + Aes, /// Unsupported compression method #[deprecated(since = "0.5.7", note = "use the constants instead")] Unsupported(u16), @@ -89,7 +89,7 @@ impl CompressionMethod { 8 => CompressionMethod::Deflated, #[cfg(feature = "bzip2")] 12 => CompressionMethod::Bzip2, - 99 => CompressionMethod::AES, + 99 => CompressionMethod::Aes, v => CompressionMethod::Unsupported(v), } } @@ -111,7 +111,7 @@ impl CompressionMethod { CompressionMethod::Deflated => 8, #[cfg(feature = "bzip2")] CompressionMethod::Bzip2 => 12, - CompressionMethod::AES => 99, + CompressionMethod::Aes => 99, CompressionMethod::Unsupported(v) => v, } } diff --git a/src/crc32.rs b/src/crc32.rs index 22c262a6..ebace898 100644 --- a/src/crc32.rs +++ b/src/crc32.rs @@ -10,7 +10,6 @@ pub struct Crc32Reader { inner: R, hasher: Hasher, check: u32, - #[cfg(feature = "aes-crypto")] /// Signals if `inner` stores aes encrypted data. /// AE-2 encrypted data doesn't use crc and sets the value to 0. ae2_encrypted: bool, @@ -19,16 +18,11 @@ pub struct Crc32Reader { impl Crc32Reader { /// Get a new Crc32Reader which checks the inner reader against checksum. /// The check is disabled if `ae2_encrypted == true`. - pub(crate) fn new( - inner: R, - checksum: u32, - #[cfg(feature = "aes-crypto")] ae2_encrypted: bool, - ) -> Crc32Reader { + pub(crate) fn new(inner: R, checksum: u32, ae2_encrypted: bool) -> Crc32Reader { Crc32Reader { inner, hasher: Hasher::new(), check: checksum, - #[cfg(feature = "aes-crypto")] ae2_encrypted, } } @@ -44,9 +38,7 @@ impl Crc32Reader { impl Read for Crc32Reader { fn read(&mut self, buf: &mut [u8]) -> io::Result { - let invalid_check = !buf.is_empty() && !self.check_matches(); - #[cfg(feature = "aes-crypto")] - let invalid_check = invalid_check && !self.ae2_encrypted; + let invalid_check = !buf.is_empty() && !self.check_matches() && !self.ae2_encrypted; let count = match self.inner.read(buf) { Ok(0) if invalid_check => { @@ -70,20 +62,10 @@ mod test { let data: &[u8] = b""; let mut buf = [0; 1]; - let mut reader = Crc32Reader::new( - data, - 0, - #[cfg(feature = "aes-crypto")] - false, - ); + let mut reader = Crc32Reader::new(data, 0, false); assert_eq!(reader.read(&mut buf).unwrap(), 0); - let mut reader = Crc32Reader::new( - data, - 1, - #[cfg(feature = "aes-crypto")] - false, - ); + let mut reader = Crc32Reader::new(data, 1, false); assert!(reader .read(&mut buf) .unwrap_err() @@ -96,12 +78,7 @@ mod test { let data: &[u8] = b"1234"; let mut buf = [0; 1]; - let mut reader = Crc32Reader::new( - data, - 0x9be3e0a3, - #[cfg(feature = "aes-crypto")] - false, - ); + let mut reader = Crc32Reader::new(data, 0x9be3e0a3, false); assert_eq!(reader.read(&mut buf).unwrap(), 1); assert_eq!(reader.read(&mut buf).unwrap(), 1); assert_eq!(reader.read(&mut buf).unwrap(), 1); @@ -116,12 +93,7 @@ mod test { let data: &[u8] = b"1234"; let mut buf = [0; 5]; - let mut reader = Crc32Reader::new( - data, - 0x9be3e0a3, - #[cfg(feature = "aes-crypto")] - false, - ); + let mut reader = Crc32Reader::new(data, 0x9be3e0a3, false); assert_eq!(reader.read(&mut buf[..0]).unwrap(), 0); assert_eq!(reader.read(&mut buf).unwrap(), 4); } diff --git a/src/read.rs b/src/read.rs index 80b6e7d7..23affbec 100644 --- a/src/read.rs +++ b/src/read.rs @@ -87,6 +87,14 @@ impl<'a> CryptoReader<'a> { CryptoReader::Aes { reader: r, .. } => r.into_inner(), } } + + /// Returns `true` if the data is encrypted using AE2. + pub fn is_ae2_encrypted(&self) -> bool { + #[cfg(feature = "aes-crypto")] + return matches!(self, CryptoReader::Aes { vendor_version: AesVendorVersion::Ae2, .. }); + #[cfg(not(feature = "aes-crypto"))] + false + } } enum ZipFileReader<'a> { @@ -224,19 +232,12 @@ fn make_reader<'a>( crc32: u32, reader: CryptoReader<'a>, ) -> ZipFileReader<'a> { - #[cfg(feature = "aes-crypto")] - let ae2_encrypted = matches!(reader, CryptoReader::Aes { - vendor_version: AesVendorVersion::Ae2, - .. - }); + let ae2_encrypted = reader.is_ae2_encrypted(); match compression_method { - CompressionMethod::Stored => ZipFileReader::Stored(Crc32Reader::new( - reader, - crc32, - #[cfg(feature = "aes-crypto")] - ae2_encrypted, - )), + CompressionMethod::Stored => { + ZipFileReader::Stored(Crc32Reader::new(reader, crc32, ae2_encrypted)) + } #[cfg(any( feature = "deflate", feature = "deflate-miniz", @@ -244,22 +245,12 @@ fn make_reader<'a>( ))] CompressionMethod::Deflated => { let deflate_reader = DeflateDecoder::new(reader); - ZipFileReader::Deflated(Crc32Reader::new( - deflate_reader, - crc32, - #[cfg(feature = "aes-crypto")] - ae2_encrypted, - )) + ZipFileReader::Deflated(Crc32Reader::new(deflate_reader, crc32, ae2_encrypted)) } #[cfg(feature = "bzip2")] CompressionMethod::Bzip2 => { let bzip2_reader = BzDecoder::new(reader); - ZipFileReader::Bzip2(Crc32Reader::new( - bzip2_reader, - crc32, - #[cfg(feature = "aes-crypto")] - ae2_encrypted, - )) + ZipFileReader::Bzip2(Crc32Reader::new(bzip2_reader, crc32, ae2_encrypted)) } _ => panic!("Compression method not supported"), } @@ -657,14 +648,11 @@ pub(crate) fn central_header_to_zip_file( Err(e) => return Err(e), } - #[cfg(feature = "aes-crypto")] - { - let aes_enabled = result.compression_method == CompressionMethod::AES; - if aes_enabled && result.aes_mode.is_none() { - return Err(ZipError::InvalidArchive( - "AES encryption without AES extra data field", - )); - } + let aes_enabled = result.compression_method == CompressionMethod::Aes; + if aes_enabled && result.aes_mode.is_none() { + return Err(ZipError::InvalidArchive( + "AES encryption without AES extra data field", + )); } // Account for shifted zip offsets. diff --git a/src/write.rs b/src/write.rs index 05236505..1c912966 100644 --- a/src/write.rs +++ b/src/write.rs @@ -831,7 +831,7 @@ impl GenericZipWriter { CompressionMethod::Bzip2 => { GenericZipWriter::Bzip2(BzEncoder::new(bare, bzip2::Compression::default())) } - CompressionMethod::AES => { + CompressionMethod::Aes => { return Err(ZipError::UnsupportedArchive( "AES compression is not supported for writing", )) From c5e55c04fd17853e5b1d5614e86f76ba114c599a Mon Sep 17 00:00:00 2001 From: Lireer Date: Mon, 9 Nov 2020 16:33:45 +0100 Subject: [PATCH 22/37] bump MSRV to 1.42 --- .github/workflows/ci.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 83741894..c2fea1fe 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -16,7 +16,7 @@ jobs: strategy: matrix: os: [ubuntu-latest, macOS-latest, windows-latest] - rust: [stable, 1.36.0] + rust: [stable, 1.42.0] steps: - uses: actions/checkout@master @@ -54,4 +54,4 @@ jobs: run: cargo fmt --all -- --check - name: Docs - run: cargo doc \ No newline at end of file + run: cargo doc From 09ad713d2011ab84d245ee92d3f2c0a346a83b69 Mon Sep 17 00:00:00 2001 From: Lireer Date: Mon, 9 Nov 2020 17:04:45 +0100 Subject: [PATCH 23/37] update crypto dependencies --- Cargo.toml | 8 ++++---- src/aes_ctr.rs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 42b560f4..1d02bd3d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,15 +11,15 @@ Library to support the reading and writing of zip files. edition = "2018" [dependencies] -aes = { version = "0.5.0", optional = true } +aes = { version = "0.6.0", optional = true } byteorder = "1.3" bzip2 = { version = "0.4", optional = true } constant_time_eq = { version = "0.1.5", optional = true } crc32fast = "1.0" flate2 = { version = "1.0.0", default-features = false, optional = true } -hmac = {version = "0.9.0", optional = true } -pbkdf2 = {version = "0.5.0", optional = true } -sha-1 = {version = "0.9.1", optional = true } +hmac = {version = "0.10.1", optional = true } +pbkdf2 = {version = "0.6.0", optional = true } +sha-1 = {version = "0.9.2", optional = true } thiserror = "1.0" time = { version = "0.1", optional = true } diff --git a/src/aes_ctr.rs b/src/aes_ctr.rs index c0391277..f5d5f7cf 100644 --- a/src/aes_ctr.rs +++ b/src/aes_ctr.rs @@ -4,7 +4,7 @@ //! different byte order (little endian) than NIST (big endian). //! See [AesCtrZipKeyStream](./struct.AesCtrZipKeyStream.html) for more information. -use aes::block_cipher::generic_array::GenericArray; +use aes::cipher::generic_array::GenericArray; use aes::{BlockCipher, NewBlockCipher}; use byteorder::WriteBytesExt; use std::{any, fmt}; From 46f65d4d49f9f181f2889d04dbca97a51f7c0f2b Mon Sep 17 00:00:00 2001 From: Lireer Date: Mon, 9 Nov 2020 17:20:17 +0100 Subject: [PATCH 24/37] add aes-crypto feature to default and update README --- Cargo.toml | 2 +- README.md | 3 ++- src/aes.rs | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1d02bd3d..5aaf2660 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,7 +34,7 @@ deflate = ["flate2/rust_backend"] deflate-miniz = ["flate2/default"] deflate-zlib = ["flate2/zlib"] unreserved = [] -default = ["bzip2", "deflate", "time"] +default = ["aes-crypto", "bzip2", "deflate", "time"] [[bench]] name = "read_entry" diff --git a/README.md b/README.md index b4df15a7..85427f0c 100644 --- a/README.md +++ b/README.md @@ -42,8 +42,9 @@ zip = { version = "0.5", default-features = false } The features available are: -* `deflate`: Enables the deflate compression algorithm, which is the default for zipfiles +* `aes-crypto`: Enables decryption of files which were encrypted with AES. Supports AE-1 and AE-2 methods. * `bzip2`: Enables the BZip2 compression algorithm. +* `deflate`: Enables the deflate compression algorithm, which is the default for zipfiles * `time`: Enables features using the [time](https://github.com/rust-lang-deprecated/time) crate. All of these are enabled by default. diff --git a/src/aes.rs b/src/aes.rs index 001a2804..79cc0170 100644 --- a/src/aes.rs +++ b/src/aes.rs @@ -2,7 +2,7 @@ //! //! This was implemented according to the [WinZip specification](https://www.winzip.com/win/en/aes_info.html). //! Note that using CRC with AES depends on the specific encryption specification used, AE-1 or AE-2. -//! AE-2 doesn't set the CRC field correctly, even though some zip files still have CRC set even with AE-2. +//! If the file is marked as encrypted with AE-2 the CRC field is ignored, even if it isn't set to 0. use crate::aes_ctr; use crate::types::AesMode; From bb97711761f30441bc944311c61b9e97c601aa54 Mon Sep 17 00:00:00 2001 From: Lireer Date: Tue, 25 Jan 2022 20:39:22 +0100 Subject: [PATCH 25/37] explain trait guarantee violation of read impl --- src/aes.rs | 11 ++++++++++- src/aes_ctr.rs | 2 +- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/aes.rs b/src/aes.rs index 2b057f68..4e8abff7 100644 --- a/src/aes.rs +++ b/src/aes.rs @@ -120,6 +120,15 @@ pub struct AesReaderValid { } impl Read for AesReaderValid { + /// This implementation does not fulfill all requirements set in the trait documentation. + /// + /// ```txt + /// "If an error is returned then it must be guaranteed that no bytes were read." + /// ``` + /// + /// Whether this applies to errors that occur while reading the encrypted data depends on the + /// underlying reader. If the error occurs while verifying the HMAC, the reader might become + /// practically unusable, since its position after the error is not known. fn read(&mut self, buf: &mut [u8]) -> io::Result { if self.data_remaining == 0 { return Ok(0); @@ -129,13 +138,13 @@ impl Read for AesReaderValid { // 2^32 bytes even on 32 bit systems. let bytes_to_read = self.data_remaining.min(buf.len() as u64) as usize; let read = self.reader.read(&mut buf[0..bytes_to_read])?; + self.data_remaining -= read as u64; // Update the hmac with the encrypted data self.hmac.update(&buf[0..read]); // decrypt the data self.cipher.crypt_in_place(&mut buf[0..read]); - self.data_remaining -= read as u64; // if there is no data left to read, check the integrity of the data if self.data_remaining == 0 { diff --git a/src/aes_ctr.rs b/src/aes_ctr.rs index f5d5f7cf..db8957ee 100644 --- a/src/aes_ctr.rs +++ b/src/aes_ctr.rs @@ -104,7 +104,7 @@ where C: AesKind, C::Cipher: BlockCipher, { - /// Decrypt or encrypt given data. + /// Decrypt or encrypt `target`. #[inline] fn crypt_in_place(&mut self, mut target: &mut [u8]) { while !target.is_empty() { From 35d8f04496b9b5438aff22b04487a0f3f3235e69 Mon Sep 17 00:00:00 2001 From: Lireer Date: Tue, 25 Jan 2022 20:42:51 +0100 Subject: [PATCH 26/37] "fix" clippy warnings --- src/aes.rs | 2 +- src/read.rs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/aes.rs b/src/aes.rs index 4e8abff7..bbf30a8b 100644 --- a/src/aes.rs +++ b/src/aes.rs @@ -95,7 +95,7 @@ impl AesReader { return Ok(None); } - let cipher = cipher_from_mode(self.aes_mode, &decrypt_key); + let cipher = cipher_from_mode(self.aes_mode, decrypt_key); let hmac = Hmac::::new_varkey(hmac_key).unwrap(); Ok(Some(AesReaderValid { diff --git a/src/read.rs b/src/read.rs index eeff3808..e5a84c48 100644 --- a/src/read.rs +++ b/src/read.rs @@ -185,6 +185,7 @@ fn find_content<'a>( Ok((reader as &mut dyn Read).take(data.compressed_size)) } +#[allow(clippy::too_many_arguments)] fn make_crypto_reader<'a>( compression_method: crate::compression::CompressionMethod, crc32: u32, @@ -211,7 +212,7 @@ fn make_crypto_reader<'a>( } #[cfg(feature = "aes-crypto")] (Some(password), Some((aes_mode, vendor_version))) => { - match AesReader::new(reader, aes_mode, compressed_size).validate(&password)? { + match AesReader::new(reader, aes_mode, compressed_size).validate(password)? { None => return Ok(Err(InvalidPassword)), Some(r) => CryptoReader::Aes { reader: r, From 3a7189371173133e244dbfbb7b32bf6e35425ba2 Mon Sep 17 00:00:00 2001 From: Lireer Date: Tue, 25 Jan 2022 20:57:27 +0100 Subject: [PATCH 27/37] run cargo fmt --- src/aes.rs | 4 ++-- src/compression.rs | 2 +- src/read.rs | 8 +++++++- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/aes.rs b/src/aes.rs index bbf30a8b..4aabccd0 100644 --- a/src/aes.rs +++ b/src/aes.rs @@ -121,11 +121,11 @@ pub struct AesReaderValid { impl Read for AesReaderValid { /// This implementation does not fulfill all requirements set in the trait documentation. - /// + /// /// ```txt /// "If an error is returned then it must be guaranteed that no bytes were read." /// ``` - /// + /// /// Whether this applies to errors that occur while reading the encrypted data depends on the /// underlying reader. If the error occurs while verifying the HMAC, the reader might become /// practically unusable, since its position after the error is not known. diff --git a/src/compression.rs b/src/compression.rs index 4b7a52cd..6f634a46 100644 --- a/src/compression.rs +++ b/src/compression.rs @@ -25,7 +25,7 @@ pub enum CompressionMethod { #[cfg(feature = "bzip2")] Bzip2, /// Encrypted using AES. - /// + /// /// The actual compression method has to be taken from the AES extra data field /// or from `ZipFileData`. Aes, diff --git a/src/read.rs b/src/read.rs index e5a84c48..ff0e755c 100644 --- a/src/read.rs +++ b/src/read.rs @@ -94,7 +94,13 @@ impl<'a> CryptoReader<'a> { /// Returns `true` if the data is encrypted using AE2. pub fn is_ae2_encrypted(&self) -> bool { #[cfg(feature = "aes-crypto")] - return matches!(self, CryptoReader::Aes { vendor_version: AesVendorVersion::Ae2, .. }); + return matches!( + self, + CryptoReader::Aes { + vendor_version: AesVendorVersion::Ae2, + .. + } + ); #[cfg(not(feature = "aes-crypto"))] false } From c17df86dbf6658bdeadbe476e2437dcb97d6c9dd Mon Sep 17 00:00:00 2001 From: Lireer Date: Tue, 25 Jan 2022 21:51:57 +0100 Subject: [PATCH 28/37] test decryption of aes encrypted files --- tests/aes_encryption.rs | 78 +++++++++++++++++++++++++++++++++++++ tests/data/aes_archive.zip | Bin 0 -> 908 bytes 2 files changed, 78 insertions(+) create mode 100644 tests/aes_encryption.rs create mode 100644 tests/data/aes_archive.zip diff --git a/tests/aes_encryption.rs b/tests/aes_encryption.rs new file mode 100644 index 00000000..84099d76 --- /dev/null +++ b/tests/aes_encryption.rs @@ -0,0 +1,78 @@ +use std::io::{self, Read}; +use zip::ZipArchive; + +const SECRET_CONTENT: &str = "Lorem ipsum dolor sit amet"; + +const PASSWORD: &[u8] = b"helloworld"; + +#[test] +fn aes256_encrypted_uncompressed_file() { + let mut v = Vec::new(); + v.extend_from_slice(include_bytes!("../tests/data/aes_archive.zip")); + let mut archive = ZipArchive::new(io::Cursor::new(v)).expect("couldn't open test zip file"); + + let mut file = archive + .by_name_decrypt("secret_data_256_uncompressed", PASSWORD) + .expect("couldn't find file in archive") + .expect("invalid password"); + assert_eq!("secret_data_256_uncompressed", file.name()); + + let mut content = String::new(); + file.read_to_string(&mut content) + .expect("couldn't read encrypted file"); + assert_eq!(SECRET_CONTENT, content); +} + +#[test] +fn aes256_encrypted_file() { + let mut v = Vec::new(); + v.extend_from_slice(include_bytes!("../tests/data/aes_archive.zip")); + let mut archive = ZipArchive::new(io::Cursor::new(v)).expect("couldn't open test zip file"); + + let mut file = archive + .by_name_decrypt("secret_data_256", PASSWORD) + .expect("couldn't find file in archive") + .expect("invalid password"); + assert_eq!("secret_data_256", file.name()); + + let mut content = String::new(); + file.read_to_string(&mut content) + .expect("couldn't read encrypted and compressed file"); + assert_eq!(SECRET_CONTENT, content); +} + +#[test] +fn aes192_encrypted_file() { + let mut v = Vec::new(); + v.extend_from_slice(include_bytes!("../tests/data/aes_archive.zip")); + let mut archive = ZipArchive::new(io::Cursor::new(v)).expect("couldn't open test zip file"); + + let mut file = archive + .by_name_decrypt("secret_data_192", PASSWORD) + .expect("couldn't find file in archive") + .expect("invalid password"); + assert_eq!("secret_data_192", file.name()); + + let mut content = String::new(); + file.read_to_string(&mut content) + .expect("couldn't read encrypted file"); + assert_eq!(SECRET_CONTENT, content); +} + +#[test] +fn aes128_encrypted_file() { + let mut v = Vec::new(); + v.extend_from_slice(include_bytes!("../tests/data/aes_archive.zip")); + let mut archive = ZipArchive::new(io::Cursor::new(v)).expect("couldn't open test zip file"); + + let mut file = archive + .by_name_decrypt("secret_data_128", PASSWORD) + .expect("couldn't find file in archive") + .expect("invalid password"); + assert_eq!("secret_data_128", file.name()); + + let mut content = String::new(); + file.read_to_string(&mut content) + .expect("couldn't read encrypted file"); + assert_eq!(SECRET_CONTENT, content); +} diff --git a/tests/data/aes_archive.zip b/tests/data/aes_archive.zip new file mode 100644 index 0000000000000000000000000000000000000000..4cf1fd21ece84b21b9c332bdf033e71b0a4bbd43 GIT binary patch literal 908 zcmWIWW@a&FW@JcaDA-^b!T<{i8;pg;}V>i{aVb+wGGS5wX^`86{5e>iRq_1H8m%cmz zZc8!L79(7?SQ^1>VPaqi*u4GAjekt@SP0`(_f@r zY%-JFRj&LwA!AlX`Sw2{2%E~FHkskD$;i|UW)m|5L$#Rk*G$u_nd{EyX5Uh)+#n)% zch?)norw%gljPFOId4ky{HSn$bEnj0!KHQ5%>ng~GN0R?+k~*K63sRlw9o_E7GIi| zoS$1zlv-Sznu28Y>RG~xNefOm`{?f~;f~L7xo|tDRX>#@^Y2R0Z;MO0ZTMbrHh$t$ z*vS~xqHx`#>CRq{ztfM~2Y53w*)!vcI(?w0K|rBlNh63xi%2d81&}BMBZCA(!{uX) z`a(At8w78E)dSH`KFo!{$VRpcGXjzAN&^~-Vwa^6vRw=xKe2-Cvg7#A+lW;(Ch*RF|u6^58rZt?NZwJ0_-m+A7&RY(vj`Lj6h_&?0}(w>@W1_M7EgW iza%f%;wLM=fISK2qgc$!1`2-`AlwTydM6MwFaQ919PY^g literal 0 HcmV?d00001 From 85bb91fb50b65973a59b70d31243560fb5a30287 Mon Sep 17 00:00:00 2001 From: Lireer Date: Wed, 26 Jan 2022 14:52:10 +0100 Subject: [PATCH 29/37] update aes-crypto dependencies --- Cargo.toml | 8 ++++---- src/aes.rs | 25 ++++++++++++++++++++++--- src/aes_ctr.rs | 4 ++-- 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f0829d91..95c4e7a9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,15 +11,15 @@ Library to support the reading and writing of zip files. edition = "2018" [dependencies] -aes = { version = "0.6.0", optional = true } +aes = { version = "0.7.5", optional = true } byteorder = "1.3" bzip2 = { version = "0.4", optional = true } constant_time_eq = { version = "0.1.5", optional = true } crc32fast = "1.1.1" flate2 = { version = "1.0.0", default-features = false, optional = true } -hmac = {version = "0.10.1", optional = true } -pbkdf2 = {version = "0.6.0", optional = true } -sha-1 = {version = "0.9.2", optional = true } +hmac = {version = "0.12.0", optional = true} +pbkdf2 = {version = "0.10.0", optional = true } +sha-1 = {version = "0.10.0", optional = true } time = { version = "0.3", features = ["formatting", "macros" ], optional = true } zstd = { version = "0.10", optional = true } diff --git a/src/aes.rs b/src/aes.rs index 4aabccd0..fc65577a 100644 --- a/src/aes.rs +++ b/src/aes.rs @@ -6,8 +6,9 @@ use crate::aes_ctr; use crate::types::AesMode; +use aes::cipher::generic_array::{typenum::Unsigned, GenericArray}; use constant_time_eq::constant_time_eq; -use hmac::{Hmac, Mac, NewMac}; +use hmac::{digest::crypto_common::KeySizeUser, Hmac, Mac}; use sha1::Sha1; use std::io::{self, Read}; @@ -96,13 +97,14 @@ impl AesReader { } let cipher = cipher_from_mode(self.aes_mode, decrypt_key); - let hmac = Hmac::::new_varkey(hmac_key).unwrap(); + let hmac = Hmac::::new_from_slice(hmac_key).unwrap(); Ok(Some(AesReaderValid { reader: self.reader, data_remaining: self.data_length, cipher, hmac, + finalized: false, })) } } @@ -117,6 +119,7 @@ pub struct AesReaderValid { data_remaining: u64, cipher: Box, hmac: Hmac, + finalized: bool, } impl Read for AesReaderValid { @@ -148,11 +151,27 @@ impl Read for AesReaderValid { // if there is no data left to read, check the integrity of the data if self.data_remaining == 0 { + assert!( + !self.finalized, + "Tried to use an already finalized HMAC. This is a bug!" + ); + self.finalized = true; + // Zip uses HMAC-Sha1-80, which only uses the first half of the hash // see https://www.winzip.com/win/en/aes_info.html#auth-faq let mut read_auth_code = [0; AUTH_CODE_LENGTH]; self.reader.read_exact(&mut read_auth_code)?; - let computed_auth_code = &self.hmac.finalize_reset().into_bytes()[0..AUTH_CODE_LENGTH]; + + // The following call to `finalize` consumes `hmac` so we replace `self.hmac` with a + // dummy that uses a `Key` made up of only zeroes. `self.hmac` should not be used after + // this. + let hmac = std::mem::replace( + &mut self.hmac, + Hmac::new(&GenericArray::from_slice( + &vec![0; as KeySizeUser>::KeySize::to_usize()], + )), + ); + let computed_auth_code = &hmac.finalize().into_bytes()[0..AUTH_CODE_LENGTH]; // use constant time comparison to mitigate timing attacks if !constant_time_eq(computed_auth_code, &read_auth_code) { diff --git a/src/aes_ctr.rs b/src/aes_ctr.rs index db8957ee..25cd09ac 100644 --- a/src/aes_ctr.rs +++ b/src/aes_ctr.rs @@ -5,7 +5,7 @@ //! See [AesCtrZipKeyStream](./struct.AesCtrZipKeyStream.html) for more information. use aes::cipher::generic_array::GenericArray; -use aes::{BlockCipher, NewBlockCipher}; +use aes::{BlockEncrypt, NewBlockCipher}; use byteorder::WriteBytesExt; use std::{any, fmt}; @@ -102,7 +102,7 @@ where impl AesCipher for AesCtrZipKeyStream where C: AesKind, - C::Cipher: BlockCipher, + C::Cipher: BlockEncrypt, { /// Decrypt or encrypt `target`. #[inline] From 2e0684442978a96f0b5c35dc4025058df94c5cc7 Mon Sep 17 00:00:00 2001 From: Lireer Date: Wed, 26 Jan 2022 14:53:19 +0100 Subject: [PATCH 30/37] fix clippy warning and shorten links in tests --- src/aes.rs | 2 +- tests/aes_encryption.rs | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/aes.rs b/src/aes.rs index fc65577a..5f943571 100644 --- a/src/aes.rs +++ b/src/aes.rs @@ -167,7 +167,7 @@ impl Read for AesReaderValid { // this. let hmac = std::mem::replace( &mut self.hmac, - Hmac::new(&GenericArray::from_slice( + Hmac::new(GenericArray::from_slice( &vec![0; as KeySizeUser>::KeySize::to_usize()], )), ); diff --git a/tests/aes_encryption.rs b/tests/aes_encryption.rs index 84099d76..f3c6310d 100644 --- a/tests/aes_encryption.rs +++ b/tests/aes_encryption.rs @@ -8,7 +8,7 @@ const PASSWORD: &[u8] = b"helloworld"; #[test] fn aes256_encrypted_uncompressed_file() { let mut v = Vec::new(); - v.extend_from_slice(include_bytes!("../tests/data/aes_archive.zip")); + v.extend_from_slice(include_bytes!("data/aes_archive.zip")); let mut archive = ZipArchive::new(io::Cursor::new(v)).expect("couldn't open test zip file"); let mut file = archive @@ -26,7 +26,7 @@ fn aes256_encrypted_uncompressed_file() { #[test] fn aes256_encrypted_file() { let mut v = Vec::new(); - v.extend_from_slice(include_bytes!("../tests/data/aes_archive.zip")); + v.extend_from_slice(include_bytes!("data/aes_archive.zip")); let mut archive = ZipArchive::new(io::Cursor::new(v)).expect("couldn't open test zip file"); let mut file = archive @@ -44,7 +44,7 @@ fn aes256_encrypted_file() { #[test] fn aes192_encrypted_file() { let mut v = Vec::new(); - v.extend_from_slice(include_bytes!("../tests/data/aes_archive.zip")); + v.extend_from_slice(include_bytes!("data/aes_archive.zip")); let mut archive = ZipArchive::new(io::Cursor::new(v)).expect("couldn't open test zip file"); let mut file = archive @@ -62,7 +62,7 @@ fn aes192_encrypted_file() { #[test] fn aes128_encrypted_file() { let mut v = Vec::new(); - v.extend_from_slice(include_bytes!("../tests/data/aes_archive.zip")); + v.extend_from_slice(include_bytes!("data/aes_archive.zip")); let mut archive = ZipArchive::new(io::Cursor::new(v)).expect("couldn't open test zip file"); let mut file = archive From cfc74a558a9a378eb5b78b23e3bd30848d5efa6a Mon Sep 17 00:00:00 2001 From: Lireer Date: Thu, 27 Jan 2022 12:18:24 +0100 Subject: [PATCH 31/37] use same SHA-1 crate with new name --- Cargo.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 95c4e7a9..2ed72e48 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,7 +19,7 @@ crc32fast = "1.1.1" flate2 = { version = "1.0.0", default-features = false, optional = true } hmac = {version = "0.12.0", optional = true} pbkdf2 = {version = "0.10.0", optional = true } -sha-1 = {version = "0.10.0", optional = true } +sha1 = {version = "0.10.0", optional = true } time = { version = "0.3", features = ["formatting", "macros" ], optional = true } zstd = { version = "0.10", optional = true } @@ -29,7 +29,7 @@ getrandom = "0.2" walkdir = "2" [features] -aes-crypto = [ "aes", "constant_time_eq", "hmac", "pbkdf2", "sha-1" ] +aes-crypto = [ "aes", "constant_time_eq", "hmac", "pbkdf2", "sha1" ] deflate = ["flate2/rust_backend"] deflate-miniz = ["flate2/default"] deflate-zlib = ["flate2/zlib"] From fddad8965decead95a1021d5113962e61e4de197 Mon Sep 17 00:00:00 2001 From: Lireer Date: Sun, 30 Jan 2022 14:30:31 +0100 Subject: [PATCH 32/37] deduplicate aes testing code --- src/aes_ctr.rs | 119 ++++++++++++++++--------------------------------- 1 file changed, 38 insertions(+), 81 deletions(-) diff --git a/src/aes_ctr.rs b/src/aes_ctr.rs index 25cd09ac..44bcdb48 100644 --- a/src/aes_ctr.rs +++ b/src/aes_ctr.rs @@ -149,7 +149,27 @@ fn xor(dest: &mut [u8], src: &[u8]) { #[cfg(test)] mod tests { - use super::{Aes128, Aes192, Aes256, AesCipher, AesCtrZipKeyStream}; + use super::{Aes128, Aes192, Aes256, AesCipher, AesCtrZipKeyStream, AesKind}; + use aes::{BlockEncrypt, NewBlockCipher}; + + /// Checks whether `crypt_in_place` produces the correct plaintext after one use and yields the + /// cipertext again after applying it again. + fn roundtrip(key: &[u8], ciphertext: &mut [u8], expected_plaintext: &[u8]) + where + Aes: AesKind, + Aes::Cipher: NewBlockCipher + BlockEncrypt, + { + let mut key_stream = AesCtrZipKeyStream::::new(key); + + let mut plaintext: Vec = ciphertext.to_vec(); + key_stream.crypt_in_place(plaintext.as_mut_slice()); + assert_eq!(plaintext, expected_plaintext.to_vec()); + + // Round-tripping should yield the ciphertext again. + let mut key_stream = AesCtrZipKeyStream::::new(key); + key_stream.crypt_in_place(&mut plaintext); + assert_eq!(plaintext, ciphertext.to_vec()); + } #[test] #[should_panic] @@ -162,7 +182,7 @@ mod tests { // `7z a -phelloworld -mem=AES256 -mx=0 aes256_40byte.zip 40byte_data.txt` #[test] fn crypt_aes_256_0_byte() { - let ciphertext = []; + let mut ciphertext = []; let expected_plaintext = &[]; let key = [ 0x0b, 0xec, 0x2e, 0xf2, 0x46, 0xf0, 0x7e, 0x35, 0x16, 0x54, 0xe0, 0x98, 0x10, 0xb3, @@ -170,85 +190,49 @@ mod tests { 0x5c, 0xd0, 0xc0, 0x54, ]; - let mut key_stream = AesCtrZipKeyStream::::new(&key); - - let mut plaintext = ciphertext; - key_stream.crypt_in_place(&mut plaintext); - assert_eq!(plaintext.to_vec(), expected_plaintext.to_vec()); - - // Round-tripping should yield the ciphertext again. - let mut key_stream = AesCtrZipKeyStream::::new(&key); - key_stream.crypt_in_place(&mut plaintext); - assert_eq!(plaintext.to_vec(), ciphertext.to_vec()); + roundtrip::(&key, &mut ciphertext, expected_plaintext); } #[test] fn crypt_aes_128_5_byte() { - let ciphertext = [0x98, 0xa9, 0x8c, 0x26, 0x0e]; - let expected_plaintext = &b"asdf\n"; + let mut ciphertext = [0x98, 0xa9, 0x8c, 0x26, 0x0e]; + let expected_plaintext = b"asdf\n"; let key = [ 0xe0, 0x25, 0x7b, 0x57, 0x97, 0x6a, 0xa4, 0x23, 0xab, 0x94, 0xaa, 0x44, 0xfd, 0x47, 0x4f, 0xa5, ]; - let mut key_stream = AesCtrZipKeyStream::::new(&key); - - let mut plaintext = ciphertext; - key_stream.crypt_in_place(&mut plaintext); - assert_eq!(plaintext.to_vec(), expected_plaintext.to_vec()); - - // Round-tripping should yield the ciphertext again. - let mut key_stream = AesCtrZipKeyStream::::new(&key); - key_stream.crypt_in_place(&mut plaintext); - assert_eq!(plaintext.to_vec(), ciphertext.to_vec()); + roundtrip::(&key, &mut ciphertext, expected_plaintext); } #[test] fn crypt_aes_192_5_byte() { - let ciphertext = [0x36, 0x55, 0x5c, 0x61, 0x3c]; - let expected_plaintext = &b"asdf\n"; + let mut ciphertext = [0x36, 0x55, 0x5c, 0x61, 0x3c]; + let expected_plaintext = b"asdf\n"; let key = [ 0xe4, 0x4a, 0x88, 0x52, 0x8f, 0xf7, 0x0b, 0x81, 0x7b, 0x75, 0xf1, 0x74, 0x21, 0x37, 0x8c, 0x90, 0xad, 0xbe, 0x4a, 0x65, 0xa8, 0x96, 0x0e, 0xcc, ]; - let mut key_stream = AesCtrZipKeyStream::::new(&key); - - let mut plaintext = ciphertext; - key_stream.crypt_in_place(&mut plaintext); - assert_eq!(plaintext.to_vec(), expected_plaintext.to_vec()); - - // Round-tripping should yield the ciphertext again. - let mut key_stream = AesCtrZipKeyStream::::new(&key); - key_stream.crypt_in_place(&mut plaintext); - assert_eq!(plaintext.to_vec(), ciphertext.to_vec()); + roundtrip::(&key, &mut ciphertext, expected_plaintext); } #[test] fn crypt_aes_256_5_byte() { - let ciphertext = [0xc2, 0x47, 0xc0, 0xdc, 0x56]; - let expected_plaintext = &b"asdf\n"; + let mut ciphertext = [0xc2, 0x47, 0xc0, 0xdc, 0x56]; + let expected_plaintext = b"asdf\n"; let key = [ 0x79, 0x5e, 0x17, 0xf2, 0xc6, 0x3d, 0x28, 0x9b, 0x4b, 0x4b, 0xbb, 0xa9, 0xba, 0xc9, 0xa5, 0xee, 0x3a, 0x4f, 0x0f, 0x4b, 0x29, 0xbd, 0xe9, 0xb8, 0x41, 0x9c, 0x41, 0xa5, 0x15, 0xb2, 0x86, 0xab, ]; - let mut key_stream = AesCtrZipKeyStream::::new(&key); - - let mut plaintext = ciphertext; - key_stream.crypt_in_place(&mut plaintext); - assert_eq!(plaintext.to_vec(), expected_plaintext.to_vec()); - - // Round-tripping should yield the ciphertext again. - let mut key_stream = AesCtrZipKeyStream::::new(&key); - key_stream.crypt_in_place(&mut plaintext); - assert_eq!(plaintext.to_vec(), ciphertext.to_vec()); + roundtrip::(&key, &mut ciphertext, expected_plaintext); } #[test] fn crypt_aes_128_40_byte() { - let ciphertext = [ + let mut ciphertext = [ 0xcf, 0x72, 0x6b, 0xa1, 0xb2, 0x0f, 0xdf, 0xaa, 0x10, 0xad, 0x9c, 0x7f, 0x6d, 0x1c, 0x8d, 0xb5, 0x16, 0x7e, 0xbb, 0x11, 0x69, 0x52, 0x8c, 0x89, 0x80, 0x32, 0xaa, 0x76, 0xa6, 0x18, 0x31, 0x98, 0xee, 0xdd, 0x22, 0x68, 0xb7, 0xe6, 0x77, 0xd2, @@ -259,21 +243,12 @@ mod tests { 0x81, 0xb6, ]; - let mut key_stream = AesCtrZipKeyStream::::new(&key); - - let mut plaintext = ciphertext; - key_stream.crypt_in_place(&mut plaintext); - assert_eq!(plaintext.to_vec(), expected_plaintext.to_vec()); - - // Round-tripping should yield the ciphertext again. - let mut key_stream = AesCtrZipKeyStream::::new(&key); - key_stream.crypt_in_place(&mut plaintext); - assert_eq!(plaintext.to_vec(), ciphertext.to_vec()); + roundtrip::(&key, &mut ciphertext, expected_plaintext); } #[test] fn crypt_aes_192_40_byte() { - let ciphertext = [ + let mut ciphertext = [ 0xa6, 0xfc, 0x52, 0x79, 0x2c, 0x6c, 0xfe, 0x68, 0xb1, 0xa8, 0xb3, 0x07, 0x52, 0x8b, 0x82, 0xa6, 0x87, 0x9c, 0x72, 0x42, 0x3a, 0xf8, 0xc6, 0xa9, 0xc9, 0xfb, 0x61, 0x19, 0x37, 0xb9, 0x56, 0x62, 0xf4, 0xfc, 0x5e, 0x7a, 0xdd, 0x55, 0x0a, 0x48, @@ -284,21 +259,12 @@ mod tests { 0xfe, 0xae, 0x1b, 0xba, 0x01, 0x97, 0x97, 0x79, 0xbb, 0xa6, ]; - let mut key_stream = AesCtrZipKeyStream::::new(&key); - - let mut plaintext = ciphertext; - key_stream.crypt_in_place(&mut plaintext); - assert_eq!(plaintext.to_vec(), expected_plaintext.to_vec()); - - // Round-tripping should yield the ciphertext again. - let mut key_stream = AesCtrZipKeyStream::::new(&key); - key_stream.crypt_in_place(&mut plaintext); - assert_eq!(plaintext.to_vec(), ciphertext.to_vec()); + roundtrip::(&key, &mut ciphertext, expected_plaintext); } #[test] fn crypt_aes_256_40_byte() { - let ciphertext = [ + let mut ciphertext = [ 0xa9, 0x99, 0xbd, 0xea, 0x82, 0x9b, 0x8f, 0x2f, 0xb7, 0x52, 0x2f, 0x6b, 0xd8, 0xf6, 0xab, 0x0e, 0x24, 0x51, 0x9e, 0x18, 0x0f, 0xc0, 0x8f, 0x54, 0x15, 0x80, 0xae, 0xbc, 0xa0, 0x5c, 0x8a, 0x11, 0x8d, 0x14, 0x7e, 0xc5, 0xb4, 0xae, 0xd3, 0x37, @@ -310,15 +276,6 @@ mod tests { 0xc2, 0x07, 0x36, 0xb6, ]; - let mut key_stream = AesCtrZipKeyStream::::new(&key); - - let mut plaintext = ciphertext; - key_stream.crypt_in_place(&mut plaintext); - assert_eq!(plaintext.to_vec(), expected_plaintext.to_vec()); - - // Round-tripping should yield the ciphertext again. - let mut key_stream = AesCtrZipKeyStream::::new(&key); - key_stream.crypt_in_place(&mut plaintext); - assert_eq!(plaintext.to_vec(), ciphertext.to_vec()); + roundtrip::(&key, &mut ciphertext, expected_plaintext); } } From 49f7501c5fd791a927ac7ee855f344b98ce18f32 Mon Sep 17 00:00:00 2001 From: Lireer Date: Sun, 30 Jan 2022 15:10:07 +0100 Subject: [PATCH 33/37] add and use AES associated constant --- src/compression.rs | 9 +++++++-- src/read.rs | 2 +- src/write.rs | 2 +- tests/aes_encryption.rs | 2 ++ 4 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/compression.rs b/src/compression.rs index 6f634a46..603e6d8c 100644 --- a/src/compression.rs +++ b/src/compression.rs @@ -28,6 +28,7 @@ pub enum CompressionMethod { /// /// The actual compression method has to be taken from the AES extra data field /// or from `ZipFileData`. + #[cfg(feature = "aes-crypto")] Aes, /// Compress the file using ZStandard #[cfg(feature = "zstd")] @@ -77,6 +78,10 @@ impl CompressionMethod { pub const JPEG: Self = CompressionMethod::Unsupported(96); pub const WAVPACK: Self = CompressionMethod::Unsupported(97); pub const PPMD: Self = CompressionMethod::Unsupported(98); + #[cfg(feature = "aes-crypto")] + pub const AES: Self = CompressionMethod::Aes; + #[cfg(not(feature = "aes-crypto"))] + pub const AES: Self = CompressionMethod::Unsupported(99); } impl CompressionMethod { /// Converts an u16 to its corresponding CompressionMethod @@ -96,9 +101,9 @@ impl CompressionMethod { 8 => CompressionMethod::Deflated, #[cfg(feature = "bzip2")] 12 => CompressionMethod::Bzip2, - 99 => CompressionMethod::Aes, #[cfg(feature = "zstd")] 93 => CompressionMethod::Zstd, + 99 => CompressionMethod::AES, v => CompressionMethod::Unsupported(v), } @@ -121,7 +126,7 @@ impl CompressionMethod { CompressionMethod::Deflated => 8, #[cfg(feature = "bzip2")] CompressionMethod::Bzip2 => 12, - CompressionMethod::Aes => 99, + CompressionMethod::AES => 99, #[cfg(feature = "zstd")] CompressionMethod::Zstd => 93, diff --git a/src/read.rs b/src/read.rs index ff0e755c..2463a563 100644 --- a/src/read.rs +++ b/src/read.rs @@ -669,7 +669,7 @@ pub(crate) fn central_header_to_zip_file( Err(e) => return Err(e), } - let aes_enabled = result.compression_method == CompressionMethod::Aes; + let aes_enabled = result.compression_method == CompressionMethod::AES; if aes_enabled && result.aes_mode.is_none() { return Err(ZipError::InvalidArchive( "AES encryption without AES extra data field", diff --git a/src/write.rs b/src/write.rs index 3fac7459..6a3403fb 100644 --- a/src/write.rs +++ b/src/write.rs @@ -848,7 +848,7 @@ impl GenericZipWriter { CompressionMethod::Bzip2 => { GenericZipWriter::Bzip2(BzEncoder::new(bare, bzip2::Compression::default())) } - CompressionMethod::Aes => { + CompressionMethod::AES => { return Err(ZipError::UnsupportedArchive( "AES compression is not supported for writing", )) diff --git a/tests/aes_encryption.rs b/tests/aes_encryption.rs index f3c6310d..4b393ebf 100644 --- a/tests/aes_encryption.rs +++ b/tests/aes_encryption.rs @@ -1,3 +1,5 @@ +#![cfg(feature = "aes-crypto")] + use std::io::{self, Read}; use zip::ZipArchive; From 3d56021052c326719d0f1aa03e861943053020c0 Mon Sep 17 00:00:00 2001 From: Lireer Date: Sun, 30 Jan 2022 15:14:47 +0100 Subject: [PATCH 34/37] use hmac reset feature for finalize_reset method --- Cargo.toml | 2 +- src/aes.rs | 15 ++------------- 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 2ed72e48..c953b6c9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,7 +17,7 @@ bzip2 = { version = "0.4", optional = true } constant_time_eq = { version = "0.1.5", optional = true } crc32fast = "1.1.1" flate2 = { version = "1.0.0", default-features = false, optional = true } -hmac = {version = "0.12.0", optional = true} +hmac = { version = "0.12.0", optional = true, features = ["reset"] } pbkdf2 = {version = "0.10.0", optional = true } sha1 = {version = "0.10.0", optional = true } time = { version = "0.3", features = ["formatting", "macros" ], optional = true } diff --git a/src/aes.rs b/src/aes.rs index 5f943571..8997705c 100644 --- a/src/aes.rs +++ b/src/aes.rs @@ -6,9 +6,8 @@ use crate::aes_ctr; use crate::types::AesMode; -use aes::cipher::generic_array::{typenum::Unsigned, GenericArray}; use constant_time_eq::constant_time_eq; -use hmac::{digest::crypto_common::KeySizeUser, Hmac, Mac}; +use hmac::{Hmac, Mac}; use sha1::Sha1; use std::io::{self, Read}; @@ -161,17 +160,7 @@ impl Read for AesReaderValid { // see https://www.winzip.com/win/en/aes_info.html#auth-faq let mut read_auth_code = [0; AUTH_CODE_LENGTH]; self.reader.read_exact(&mut read_auth_code)?; - - // The following call to `finalize` consumes `hmac` so we replace `self.hmac` with a - // dummy that uses a `Key` made up of only zeroes. `self.hmac` should not be used after - // this. - let hmac = std::mem::replace( - &mut self.hmac, - Hmac::new(GenericArray::from_slice( - &vec![0; as KeySizeUser>::KeySize::to_usize()], - )), - ); - let computed_auth_code = &hmac.finalize().into_bytes()[0..AUTH_CODE_LENGTH]; + let computed_auth_code = &self.hmac.finalize_reset().into_bytes()[0..AUTH_CODE_LENGTH]; // use constant time comparison to mitigate timing attacks if !constant_time_eq(computed_auth_code, &read_auth_code) { From 8f061f882b58f90fbe1a87440620c5b0b9cb7c9c Mon Sep 17 00:00:00 2001 From: Lireer Date: Sun, 30 Jan 2022 15:26:34 +0100 Subject: [PATCH 35/37] fix nightly clippy warning --- src/write.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/write.rs b/src/write.rs index 6a3403fb..a3aa0e0a 100644 --- a/src/write.rs +++ b/src/write.rs @@ -790,7 +790,7 @@ impl Drop for ZipWriter { fn drop(&mut self) { if !self.inner.is_closed() { if let Err(e) = self.finalize() { - let _ = write!(&mut io::stderr(), "ZipWriter drop failed: {:?}", e); + let _ = write!(io::stderr(), "ZipWriter drop failed: {:?}", e); } } } From 91745d5d273485f81be276e7ee6a39497016c65f Mon Sep 17 00:00:00 2001 From: Lireer Date: Sun, 30 Jan 2022 15:28:50 +0100 Subject: [PATCH 36/37] use `assert_eq` instead of `debug_assert_eq` --- src/aes_ctr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/aes_ctr.rs b/src/aes_ctr.rs index 44bcdb48..0f34335c 100644 --- a/src/aes_ctr.rs +++ b/src/aes_ctr.rs @@ -140,7 +140,7 @@ pub trait AesCipher { /// XORs a slice in place with another slice. #[inline] fn xor(dest: &mut [u8], src: &[u8]) { - debug_assert_eq!(dest.len(), src.len()); + assert_eq!(dest.len(), src.len()); for (lhs, rhs) in dest.iter_mut().zip(src.iter()) { *lhs ^= *rhs; From c8aece8f7bacf0226239bacad223a365b3e019a6 Mon Sep 17 00:00:00 2001 From: Lireer Date: Sun, 30 Jan 2022 15:32:40 +0100 Subject: [PATCH 37/37] fix nightly clippy warnings in examples --- examples/extract.rs | 2 +- examples/file_info.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/extract.rs b/examples/extract.rs index b02eb4cd..7b8860ca 100644 --- a/examples/extract.rs +++ b/examples/extract.rs @@ -30,7 +30,7 @@ fn real_main() -> i32 { } } - if (&*file.name()).ends_with('/') { + if (*file.name()).ends_with('/') { println!("File {} extracted to \"{}\"", i, outpath.display()); fs::create_dir_all(&outpath).unwrap(); } else { diff --git a/examples/file_info.rs b/examples/file_info.rs index 824278df..64969b66 100644 --- a/examples/file_info.rs +++ b/examples/file_info.rs @@ -34,7 +34,7 @@ fn real_main() -> i32 { } } - if (&*file.name()).ends_with('/') { + if (*file.name()).ends_with('/') { println!( "Entry {} is a directory with name \"{}\"", i,