diff --git a/CHANGELOG.md b/CHANGELOG.md index cebc013ba2c..6e9f1cf30f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -66,6 +66,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Removed the `unsafe` keyword for the `Platform` trait. - Replaced the mechanism RPC traits in `service` with a single `MechanismImpl` trait. - Made the `mechanisms` module private. Mechanism implementation can still be accessed via the `Mechanism` enum. +- Changed the `Aes256Cbc` mechanism to use no padding instead of zero padding. ### Fixed diff --git a/src/mechanisms/aes256cbc.rs b/src/mechanisms/aes256cbc.rs index 81993a0d9c4..6258a3eadc7 100644 --- a/src/mechanisms/aes256cbc.rs +++ b/src/mechanisms/aes256cbc.rs @@ -15,10 +15,9 @@ impl MechanismImpl for super::Aes256Cbc { request: &request::Encrypt, ) -> Result { use aes::Aes256; - use cbc::cipher::{block_padding::ZeroPadding, BlockEncryptMut, KeyIvInit}; + use cbc::cipher::{block_padding::NoPadding, BlockEncryptMut, KeyIvInit}; type Aes256CbcEnc = cbc::Encryptor; - // TODO: perhaps use NoPadding and have client pad, to emphasize spec-conformance? let key_id = request.key; let key = keystore.load_key(key::Secrecy::Secret, None, &key_id)?; @@ -51,11 +50,9 @@ impl MechanismImpl for super::Aes256Cbc { // hprintln!(" aes256cbc encrypting l = {}B: {:?}", l, &buffer).ok(); // Encrypt message in-place. - // &buffer[..pos] is used as a message and &buffer[pos..] as a reserved space for padding. - // The padding space should be big enough for padding, otherwise method will return Err(BlockModeError). let ciphertext = cipher - .encrypt_padded_mut::(&mut buffer, l) - .unwrap(); + .encrypt_padded_mut::(&mut buffer, l) + .map_err(|_| Error::MechanismParamInvalid)?; let ciphertext = Message::from_slice(ciphertext).unwrap(); Ok(reply::Encrypt { @@ -104,9 +101,8 @@ impl MechanismImpl for super::Aes256Cbc { request: &request::Decrypt, ) -> Result { use aes::Aes256; - use cbc::cipher::{block_padding::ZeroPadding, BlockDecryptMut, KeyIvInit}; + use cbc::cipher::{block_padding::NoPadding, BlockDecryptMut, KeyIvInit}; - // TODO: perhaps use NoPadding and have client pad, to emphasize spec-conformance? type Aes256CbcDec = cbc::Decryptor; let key_id = request.key; @@ -140,13 +136,12 @@ impl MechanismImpl for super::Aes256Cbc { // let l = buffer.len(); // Decrypt message in-place. - // Returns an error if buffer length is not multiple of block size and - // if after decoding message has malformed padding. + // Returns an error if buffer length is not multiple of block size // hprintln!("encrypted: {:?}", &buffer).ok(); // hprintln!("symmetric key: {:?}", &symmetric_key).ok(); let plaintext = cipher - .decrypt_padded_mut::(&mut buffer) - .unwrap(); + .decrypt_padded_mut::(&mut buffer) + .map_err(|_| Error::MechanismParamInvalid)?; // hprintln!("decrypted: {:?}", &plaintext).ok(); let plaintext = Message::from_slice(plaintext).unwrap();