Skip to content

Commit

Permalink
[PM-6108] Add support for missing AES decrypt types (#335)
Browse files Browse the repository at this point in the history
## Type of change
```
- [ ] Bug fix
- [ ] New feature development
- [x] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other
```

## Objective
Add support for the missing AES decryption types: `AesCbc256_B64` and
`AesCbc128_HmacSha256_B64`.

Note that `SymmetricCryptoKey` at the moment expects 32 byte keys, and
AES128 used 16 byte keys, so the 16byte key+mac get parsed as a single
32byte key. At the moment we just split it by hand, but ultimately we
would need to handle this better in a future refactor.
  • Loading branch information
dani-garcia authored Feb 8, 2024
1 parent aa30b65 commit f57262c
Show file tree
Hide file tree
Showing 2 changed files with 104 additions and 7 deletions.
60 changes: 57 additions & 3 deletions crates/bitwarden-crypto/src/aes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
//! [KeyEncryptable][crate::KeyEncryptable] & [KeyDecryptable][crate::KeyDecryptable] instead.
use aes::cipher::{
block_padding::Pkcs7, typenum::U32, BlockDecryptMut, BlockEncryptMut, KeyIvInit,
block_padding::Pkcs7,
typenum::{U16, U32},
BlockDecryptMut, BlockEncryptMut, KeyIvInit,
};
use generic_array::GenericArray;
use hmac::Mac;
Expand Down Expand Up @@ -109,6 +111,42 @@ fn encrypt_aes256_internal(
(iv, data)
}

/// Decrypt using AES-128 in CBC mode.
///
/// Behaves similar to [decrypt_aes128_hmac], but does not validate the MAC.
fn decrypt_aes128(iv: &[u8; 16], data: Vec<u8>, key: &GenericArray<u8, U16>) -> Result<Vec<u8>> {
// Decrypt data
let iv = GenericArray::from_slice(iv);
let mut data = data;
let decrypted_key_slice = cbc::Decryptor::<aes::Aes128>::new(key, iv)
.decrypt_padded_mut::<Pkcs7>(&mut data)
.map_err(|_| CryptoError::KeyDecrypt)?;

// Data is decrypted in place and returns a subslice of the original Vec, to avoid cloning it,
// we truncate to the subslice length
let decrypted_len = decrypted_key_slice.len();
data.truncate(decrypted_len);

Ok(data)
}

/// Decrypt using AES-128 in CBC mode with MAC.
///
/// Behaves similar to [decrypt_aes128], but also validates the MAC.
pub fn decrypt_aes128_hmac(
iv: &[u8; 16],
mac: &[u8; 32],
data: Vec<u8>,
mac_key: &GenericArray<u8, U16>,
key: &GenericArray<u8, U16>,
) -> Result<Vec<u8>> {
let res = generate_mac(mac_key, iv, &data)?;
if res.ct_ne(mac).into() {
return Err(CryptoError::InvalidMac);
}
decrypt_aes128(iv, data, key)
}

/// Generate a MAC using HMAC-SHA256.
fn generate_mac(mac_key: &[u8], iv: &[u8], data: &[u8]) -> Result<[u8; 32]> {
let mut hmac = PbkdfSha256Hmac::new_from_slice(mac_key).expect("HMAC can take key of any size");
Expand All @@ -124,14 +162,17 @@ fn generate_mac(mac_key: &[u8], iv: &[u8], data: &[u8]) -> Result<[u8; 32]> {
#[cfg(test)]
mod tests {
use base64::{engine::general_purpose::STANDARD, Engine};
use generic_array::sequence::GenericSequence;
use generic_array::{sequence::GenericSequence, ArrayLength};
use rand::SeedableRng;

use super::*;

/// Helper function for generating a `GenericArray` of size 32 with each element being
/// a multiple of a given increment, starting from a given offset.
fn generate_generic_array(offset: u8, increment: u8) -> GenericArray<u8, U32> {
fn generate_generic_array<N: ArrayLength<u8>>(
offset: u8,
increment: u8,
) -> GenericArray<u8, N> {
GenericArray::generate(|i| offset + i as u8 * increment)
}

Expand Down Expand Up @@ -170,6 +211,19 @@ mod tests {
assert_eq!(mac.len(), 32);
}

#[test]
fn test_decrypt_aes128() {
let iv = generate_vec(16, 0, 1);
let iv: &[u8; 16] = iv.as_slice().try_into().unwrap();
let key = generate_generic_array(0, 1);

let data = STANDARD.decode("dC0X+2IjFbeL4WLLg2jX7Q==").unwrap();

let decrypted = decrypt_aes128(iv, data, &key).unwrap();

assert_eq!(String::from_utf8(decrypted).unwrap(), "EncryptMe!");
}

#[test]
fn test_decrypt_aes256() {
let iv = generate_vec(16, 0, 1);
Expand Down
51 changes: 47 additions & 4 deletions crates/bitwarden-crypto/src/enc_string/symmetric.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,13 +236,26 @@ impl KeyEncryptable<SymmetricCryptoKey, EncString> for &[u8] {
impl KeyDecryptable<SymmetricCryptoKey, Vec<u8>> for EncString {
fn decrypt_with_key(&self, key: &SymmetricCryptoKey) -> Result<Vec<u8>> {
match self {
EncString::AesCbc256_B64 { iv, data } => {
let dec = crate::aes::decrypt_aes256(iv, data.clone(), &key.key)?;
Ok(dec)
}
EncString::AesCbc128_HmacSha256_B64 { iv, mac, data } => {
// TODO: SymmetricCryptoKey is designed to handle 32 byte keys only, but this
// variant uses a 16 byte key This means the key+mac are going to be
// parsed as a single 32 byte key, at the moment we split it manually
// When refactoring the key handling, this should be fixed.
let enc_key = key.key[0..16].into();
let mac_key = key.key[16..32].into();
let dec = crate::aes::decrypt_aes128_hmac(iv, mac, data.clone(), mac_key, enc_key)?;
Ok(dec)
}
EncString::AesCbc256_HmacSha256_B64 { iv, mac, data } => {
let mac_key = key.mac_key.as_ref().ok_or(CryptoError::InvalidMac)?;
let dec =
crate::aes::decrypt_aes256_hmac(iv, mac, data.clone(), mac_key, &key.key)?;
Ok(dec)
}
_ => Err(CryptoError::InvalidKey),
}
}
}
Expand Down Expand Up @@ -277,7 +290,7 @@ mod tests {
use schemars::schema_for;

use super::EncString;
use crate::{derive_symmetric_key, KeyDecryptable, KeyEncryptable};
use crate::{derive_symmetric_key, KeyDecryptable, KeyEncryptable, SymmetricCryptoKey};

#[test]
fn test_enc_string_roundtrip() {
Expand Down Expand Up @@ -343,7 +356,9 @@ mod tests {
data,
&[93, 118, 241, 43, 16, 211, 135, 233, 150, 136, 221, 71, 140, 125, 141, 215]
);
}
} else {
panic!("Invalid variant")
};
}

#[test]
Expand All @@ -368,7 +383,35 @@ mod tests {
data,
&[50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69]
);
}
} else {
panic!("Invalid variant")
};
}

#[test]
fn test_decrypt_cbc256() {
let key = "hvBMMb1t79YssFZkpetYsM3deyVuQv4r88Uj9gvYe08=";
let key: SymmetricCryptoKey = key.parse().unwrap();

let enc_str = "0.NQfjHLr6za7VQVAbrpL81w==|wfrjmyJ0bfwkQlySrhw8dA==";
let enc_string: EncString = enc_str.parse().unwrap();
assert_eq!(enc_string.enc_type(), 0);

let dec_str: String = enc_string.decrypt_with_key(&key).unwrap();
assert_eq!(dec_str, "EncryptMe!");
}

#[test]
fn test_decrypt_cbc128_hmac() {
let key = "Gt1aZ8kTTgkF80bLtb7LiMZBcxEA2FA5mbvV4x7K208=";
let key: SymmetricCryptoKey = key.parse().unwrap();

let enc_str = "1.CU/oG4VZuxbHoZSDZjCLQw==|kb1HGwAk+fQ275ORfLf5Ew==|8UaEYHyqRZcG37JWhYBOBdEatEXd1u1/wN7OuImolcM=";
let enc_string: EncString = enc_str.parse().unwrap();
assert_eq!(enc_string.enc_type(), 1);

let dec_str: String = enc_string.decrypt_with_key(&key).unwrap();
assert_eq!(dec_str, "EncryptMe!");
}

#[test]
Expand Down

0 comments on commit f57262c

Please sign in to comment.