Skip to content

Commit 3b7cd20

Browse files
eligrubbeligrubb
committed
[PM-18938] isolate decryption support for 32 byte user keys
1 parent f75a58c commit 3b7cd20

File tree

4 files changed

+151
-18
lines changed

4 files changed

+151
-18
lines changed

‎crates/bitwarden-crypto/src/enc_string/symmetric.rs‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -311,8 +311,10 @@ impl KeyEncryptableWithContentType<SymmetricCryptoKey, EncString> for &[u8] {
311311
impl KeyDecryptable<SymmetricCryptoKey, Vec<u8>> for EncString {
312312
fn decrypt_with_key(&self, key: &SymmetricCryptoKey) -> Result<Vec<u8>> {
313313
match (self, key) {
314-
(EncString::Aes256Cbc_B64 { iv, data }, SymmetricCryptoKey::Aes256CbcKey(key)) => {
315-
crate::aes::decrypt_aes256(iv, data.clone(), &key.enc_key)
314+
(EncString::Aes256Cbc_B64 { .. }, SymmetricCryptoKey::Aes256CbcKey(_)) => {
315+
Err(CryptoError::OperationNotSupported(
316+
UnsupportedOperationError::DecryptionNotImplementedForKey,
317+
))
316318
}
317319
(
318320
EncString::Aes256Cbc_HmacSha256_B64 { iv, mac, data },
@@ -557,16 +559,24 @@ mod tests {
557559
}
558560

559561
#[test]
560-
fn test_decrypt_cbc256() {
562+
fn test_decrypt_fails_for_cbc256_keys() {
561563
let key = "hvBMMb1t79YssFZkpetYsM3deyVuQv4r88Uj9gvYe08=".to_string();
562564
let key = SymmetricCryptoKey::try_from(key).unwrap();
563565

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

568-
let dec_str: String = enc_string.decrypt_with_key(&key).unwrap();
569-
assert_eq!(dec_str, "EncryptMe!");
570+
let result: Result<String, CryptoError> = enc_string.decrypt_with_key(&key);
571+
assert!(
572+
matches!(
573+
result,
574+
Err(CryptoError::OperationNotSupported(
575+
crate::error::UnsupportedOperationError::DecryptionNotImplementedForKey
576+
)),
577+
),
578+
"Expected decrypt to fail when using deprecated type 0 key",
579+
);
570580
}
571581

572582
#[test]

‎crates/bitwarden-crypto/src/error.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,8 @@ pub enum CryptoError {
7979
pub enum UnsupportedOperationError {
8080
#[error("Encryption is not implemented for key")]
8181
EncryptionNotImplementedForKey,
82+
#[error("Decryption is not implemented for key")]
83+
DecryptionNotImplementedForKey,
8284
}
8385

8486
#[derive(Debug, Error)]

‎crates/bitwarden-crypto/src/keys/master_key.rs‎

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -143,11 +143,9 @@ pub(super) fn decrypt_user_key(
143143
// Legacy. user_keys were encrypted using `Aes256Cbc_B64` a long time ago. We've since
144144
// moved to using `Aes256Cbc_HmacSha256_B64`. However, we still need to support
145145
// decrypting these old keys.
146-
EncString::Aes256Cbc_B64 { .. } => {
147-
let legacy_key = SymmetricCryptoKey::Aes256CbcKey(super::Aes256CbcKey {
148-
enc_key: Box::pin(GenericArray::clone_from_slice(key)),
149-
});
150-
user_key.decrypt_with_key(&legacy_key)?
146+
EncString::Aes256Cbc_B64 { iv, ref data } => {
147+
let legacy_key = Box::pin(GenericArray::clone_from_slice(key));
148+
crate::aes::decrypt_aes256(&iv, data.clone(), &legacy_key)?
151149
}
152150
EncString::Aes256Cbc_HmacSha256_B64 { .. } => {
153151
let stretched_key = SymmetricCryptoKey::Aes256CbcHmacKey(stretch_key(key)?);
@@ -330,4 +328,38 @@ mod tests {
330328
]
331329
);
332330
}
331+
332+
#[test]
333+
fn test_decrypt_cbc256() {
334+
let master_key = "hvBMMb1t79YssFZkpetYsM3deyVuQv4r88Uj9gvYe08=".to_string();
335+
let master_key = SymmetricCryptoKey::try_from(master_key).unwrap();
336+
let SymmetricCryptoKey::Aes256CbcKey(master_key) = &master_key else {
337+
panic!("Incorrect key type for test used");
338+
};
339+
340+
let master_key = MasterKey::KdfKey(KdfDerivedKeyMaterial(master_key.enc_key.clone()));
341+
342+
let enc_str = "0.tn/heK4HLbbEe+yEkC+kvw==|8QM94f7aVTtjm/bmvRdVxOxiLiiZtHYYO7+oBdjFCkilncesx0iVrXPl+tMKqW+Jo7+FtZdPNsTrL6RdoG7i5QbCRVwK+9010+xm7MTQY8s=";
343+
let enc_string: EncString = enc_str.parse().unwrap();
344+
345+
let decrypted = master_key.decrypt_user_key(enc_string).unwrap();
346+
let SymmetricCryptoKey::Aes256CbcHmacKey(decrypted) = &decrypted else {
347+
panic!("Failed to decrypt Aes256CbcHmacKey from Aes256CbcKey encrypted EncString")
348+
};
349+
350+
assert_eq!(
351+
decrypted.enc_key.as_slice(),
352+
[
353+
116, 170, 187, 43, 80, 212, 193, 202, 234, 181, 57, 66, 151, 249, 59, 47, 70, 16,
354+
57, 4, 170, 78, 85, 241, 152, 232, 91, 57, 9, 87, 209, 245,
355+
]
356+
);
357+
assert_eq!(
358+
decrypted.mac_key.as_slice(),
359+
[
360+
40, 245, 106, 140, 2, 225, 138, 213, 98, 223, 92, 168, 135, 208, 22, 194, 31, 21,
361+
178, 252, 203, 198, 35, 174, 53, 218, 254, 151, 235, 57, 7, 98,
362+
]
363+
);
364+
}
333365
}

‎crates/bitwarden-crypto/src/store/context.rs‎

Lines changed: 97 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -169,10 +169,10 @@ impl<Ids: KeyIds> KeyStoreContext<'_, Ids> {
169169
let wrapping_key = self.get_symmetric_key(wrapping_key)?;
170170

171171
let key = match (wrapped_key, wrapping_key) {
172-
(EncString::Aes256Cbc_B64 { iv, data }, SymmetricCryptoKey::Aes256CbcKey(key)) => {
173-
SymmetricCryptoKey::try_from(&BitwardenLegacyKeyBytes::from(
174-
crate::aes::decrypt_aes256(iv, data.clone(), &key.enc_key)?,
175-
))?
172+
(EncString::Aes256Cbc_B64 { .. }, SymmetricCryptoKey::Aes256CbcKey(_)) => {
173+
return Err(CryptoError::OperationNotSupported(
174+
UnsupportedOperationError::DecryptionNotImplementedForKey,
175+
));
176176
}
177177
(
178178
EncString::Aes256Cbc_HmacSha256_B64 { iv, mac, data },
@@ -517,8 +517,10 @@ impl<Ids: KeyIds> KeyStoreContext<'_, Ids> {
517517
let key = self.get_symmetric_key(key)?;
518518

519519
match (data, key) {
520-
(EncString::Aes256Cbc_B64 { iv, data }, SymmetricCryptoKey::Aes256CbcKey(key)) => {
521-
crate::aes::decrypt_aes256(iv, data.clone(), &key.enc_key)
520+
(EncString::Aes256Cbc_B64 { .. }, SymmetricCryptoKey::Aes256CbcKey(_)) => {
521+
Err(CryptoError::OperationNotSupported(
522+
UnsupportedOperationError::DecryptionNotImplementedForKey,
523+
))
522524
}
523525
(
524526
EncString::Aes256Cbc_HmacSha256_B64 { iv, mac, data },
@@ -605,8 +607,8 @@ mod tests {
605607

606608
use crate::{
607609
AsymmetricCryptoKey, AsymmetricPublicCryptoKey, CompositeEncryptable, CoseKeyBytes,
608-
CoseSerializable, CryptoError, Decryptable, KeyDecryptable, LocalId, Pkcs8PrivateKeyBytes,
609-
SignatureAlgorithm, SigningKey, SigningNamespace, SymmetricCryptoKey,
610+
CoseSerializable, CryptoError, Decryptable, EncString, KeyDecryptable, LocalId,
611+
Pkcs8PrivateKeyBytes, SignatureAlgorithm, SigningKey, SigningNamespace, SymmetricCryptoKey,
610612
store::{
611613
KeyStore,
612614
tests::{Data, DataView},
@@ -894,4 +896,91 @@ mod tests {
894896
"Expected encrypt to fail with KeyOperationNotSupported",
895897
);
896898
}
899+
900+
#[test]
901+
fn test_encrypt_decrypt_data_fails_when_key_is_type_0() {
902+
let store = KeyStore::<TestIds>::default();
903+
let mut ctx = store.context_mut();
904+
905+
let key_id = TestSymmKey::A(0);
906+
let key = SymmetricCryptoKey::Aes256CbcKey(crate::Aes256CbcKey {
907+
enc_key: Box::pin([0u8; 32].into()),
908+
});
909+
ctx.set_symmetric_key_internal(key_id, key).unwrap();
910+
911+
let data_to_encrypt: Vec<u8> = vec![1, 2, 3, 4, 5];
912+
let result = ctx.encrypt_data_with_symmetric_key(
913+
key_id,
914+
&data_to_encrypt,
915+
crate::ContentFormat::OctetStream,
916+
);
917+
assert!(
918+
matches!(
919+
result,
920+
Err(CryptoError::OperationNotSupported(
921+
crate::error::UnsupportedOperationError::EncryptionNotImplementedForKey
922+
))
923+
),
924+
"Expected encrypt to fail when using deprecated type 0 keys",
925+
);
926+
927+
let data_to_decrypt = EncString::Aes256Cbc_B64 {
928+
iv: [0; 16],
929+
data: data_to_encrypt,
930+
}; // dummy value; shouldn't matter
931+
let result = ctx.decrypt_data_with_symmetric_key(key_id, &data_to_decrypt);
932+
assert!(
933+
matches!(
934+
result,
935+
Err(CryptoError::OperationNotSupported(
936+
crate::error::UnsupportedOperationError::DecryptionNotImplementedForKey
937+
))
938+
),
939+
"Expected decrypt to fail when using deprecated type 0 keys",
940+
);
941+
}
942+
943+
#[test]
944+
fn test_wrap_unwrap_key_fails_when_key_is_type_0() {
945+
let store = KeyStore::<TestIds>::default();
946+
let mut ctx = store.context_mut();
947+
948+
let wrapping_key_id = TestSymmKey::A(0);
949+
let wrapping_key = SymmetricCryptoKey::Aes256CbcKey(crate::Aes256CbcKey {
950+
enc_key: Box::pin([0u8; 32].into()),
951+
});
952+
ctx.set_symmetric_key_internal(wrapping_key_id, wrapping_key)
953+
.unwrap();
954+
955+
let key_to_wrap_id = TestSymmKey::A(1);
956+
let key_to_wrap = SymmetricCryptoKey::make_aes256_cbc_hmac_key();
957+
ctx.set_symmetric_key_internal(key_to_wrap_id, key_to_wrap)
958+
.unwrap();
959+
960+
let result = ctx.wrap_symmetric_key(wrapping_key_id, key_to_wrap_id);
961+
assert!(
962+
matches!(
963+
result,
964+
Err(CryptoError::OperationNotSupported(
965+
crate::error::UnsupportedOperationError::EncryptionNotImplementedForKey
966+
))
967+
),
968+
"Expected encrypt to fail when using deprecated type 0 keys",
969+
);
970+
971+
let wrapped_key = &EncString::Aes256Cbc_B64 {
972+
iv: [0; 16],
973+
data: vec![0],
974+
}; // dummy value; shouldn't matter
975+
let result = ctx.unwrap_symmetric_key(wrapping_key_id, wrapped_key);
976+
assert!(
977+
matches!(
978+
result,
979+
Err(CryptoError::OperationNotSupported(
980+
crate::error::UnsupportedOperationError::DecryptionNotImplementedForKey
981+
))
982+
),
983+
"Expected decrypt to fail when using deprecated type 0 keys",
984+
);
985+
}
897986
}

0 commit comments

Comments
 (0)