diff --git a/crates/bitwarden-crypto/src/enc_string/symmetric.rs b/crates/bitwarden-crypto/src/enc_string/symmetric.rs index 12d47d4da..675ac1bb3 100644 --- a/crates/bitwarden-crypto/src/enc_string/symmetric.rs +++ b/crates/bitwarden-crypto/src/enc_string/symmetric.rs @@ -332,8 +332,10 @@ impl KeyEncryptableWithContentType for &[u8] { impl KeyDecryptable> for EncString { fn decrypt_with_key(&self, key: &SymmetricCryptoKey) -> Result> { match (self, key) { - (EncString::Aes256Cbc_B64 { iv, data }, SymmetricCryptoKey::Aes256CbcKey(key)) => { - crate::aes::decrypt_aes256(iv, data.clone(), &key.enc_key) + (EncString::Aes256Cbc_B64 { .. }, SymmetricCryptoKey::Aes256CbcKey(_)) => { + Err(CryptoError::OperationNotSupported( + UnsupportedOperationError::DecryptionNotImplementedForKey, + )) } ( EncString::Aes256Cbc_HmacSha256_B64 { iv, mac, data }, @@ -578,7 +580,7 @@ mod tests { } #[test] - fn test_decrypt_cbc256() { + fn test_decrypt_fails_for_cbc256_keys() { let key = "hvBMMb1t79YssFZkpetYsM3deyVuQv4r88Uj9gvYe08=".to_string(); let key = SymmetricCryptoKey::try_from(key).unwrap(); @@ -586,8 +588,16 @@ mod tests { 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!"); + let result: Result = enc_string.decrypt_with_key(&key); + assert!( + matches!( + result, + Err(CryptoError::OperationNotSupported( + crate::error::UnsupportedOperationError::DecryptionNotImplementedForKey + )), + ), + "Expected decrypt to fail when using deprecated type 0 key", + ); } #[test] diff --git a/crates/bitwarden-crypto/src/error.rs b/crates/bitwarden-crypto/src/error.rs index 9853bd6a9..95af20538 100644 --- a/crates/bitwarden-crypto/src/error.rs +++ b/crates/bitwarden-crypto/src/error.rs @@ -79,6 +79,8 @@ pub enum CryptoError { pub enum UnsupportedOperationError { #[error("Encryption is not implemented for key")] EncryptionNotImplementedForKey, + #[error("Decryption is not implemented for key")] + DecryptionNotImplementedForKey, } #[derive(Debug, Error)] diff --git a/crates/bitwarden-crypto/src/keys/master_key.rs b/crates/bitwarden-crypto/src/keys/master_key.rs index fb2ab08f3..9f5f4499d 100644 --- a/crates/bitwarden-crypto/src/keys/master_key.rs +++ b/crates/bitwarden-crypto/src/keys/master_key.rs @@ -143,11 +143,9 @@ pub(super) fn decrypt_user_key( // Legacy. user_keys were encrypted using `Aes256Cbc_B64` a long time ago. We've since // moved to using `Aes256Cbc_HmacSha256_B64`. However, we still need to support // decrypting these old keys. - EncString::Aes256Cbc_B64 { .. } => { - let legacy_key = SymmetricCryptoKey::Aes256CbcKey(super::Aes256CbcKey { - enc_key: Box::pin(GenericArray::clone_from_slice(key)), - }); - user_key.decrypt_with_key(&legacy_key)? + EncString::Aes256Cbc_B64 { iv, ref data } => { + let legacy_key = Box::pin(GenericArray::clone_from_slice(key)); + crate::aes::decrypt_aes256(&iv, data.clone(), &legacy_key)? } EncString::Aes256Cbc_HmacSha256_B64 { .. } => { let stretched_key = SymmetricCryptoKey::Aes256CbcHmacKey(stretch_key(key)?); @@ -155,7 +153,7 @@ pub(super) fn decrypt_user_key( } EncString::Cose_Encrypt0_B64 { .. } => { return Err(CryptoError::OperationNotSupported( - crate::error::UnsupportedOperationError::EncryptionNotImplementedForKey, + crate::error::UnsupportedOperationError::DecryptionNotImplementedForKey, )); } }; @@ -330,4 +328,38 @@ mod tests { ] ); } + + #[test] + fn test_decrypt_cbc256() { + let master_key = "hvBMMb1t79YssFZkpetYsM3deyVuQv4r88Uj9gvYe08=".to_string(); + let master_key = SymmetricCryptoKey::try_from(master_key).unwrap(); + let SymmetricCryptoKey::Aes256CbcKey(master_key) = &master_key else { + panic!("Incorrect key type for test used"); + }; + + let master_key = MasterKey::KdfKey(KdfDerivedKeyMaterial(master_key.enc_key.clone())); + + let enc_str = "0.tn/heK4HLbbEe+yEkC+kvw==|8QM94f7aVTtjm/bmvRdVxOxiLiiZtHYYO7+oBdjFCkilncesx0iVrXPl+tMKqW+Jo7+FtZdPNsTrL6RdoG7i5QbCRVwK+9010+xm7MTQY8s="; + let enc_string: EncString = enc_str.parse().unwrap(); + + let decrypted = master_key.decrypt_user_key(enc_string).unwrap(); + let SymmetricCryptoKey::Aes256CbcHmacKey(decrypted) = &decrypted else { + panic!("Failed to decrypt Aes256CbcHmacKey from Aes256CbcKey encrypted EncString") + }; + + assert_eq!( + decrypted.enc_key.as_slice(), + [ + 116, 170, 187, 43, 80, 212, 193, 202, 234, 181, 57, 66, 151, 249, 59, 47, 70, 16, + 57, 4, 170, 78, 85, 241, 152, 232, 91, 57, 9, 87, 209, 245, + ] + ); + assert_eq!( + decrypted.mac_key.as_slice(), + [ + 40, 245, 106, 140, 2, 225, 138, 213, 98, 223, 92, 168, 135, 208, 22, 194, 31, 21, + 178, 252, 203, 198, 35, 174, 53, 218, 254, 151, 235, 57, 7, 98, + ] + ); + } } diff --git a/crates/bitwarden-crypto/src/store/context.rs b/crates/bitwarden-crypto/src/store/context.rs index b01b0e0a2..66b141773 100644 --- a/crates/bitwarden-crypto/src/store/context.rs +++ b/crates/bitwarden-crypto/src/store/context.rs @@ -169,10 +169,10 @@ impl KeyStoreContext<'_, Ids> { let wrapping_key = self.get_symmetric_key(wrapping_key)?; let key = match (wrapped_key, wrapping_key) { - (EncString::Aes256Cbc_B64 { iv, data }, SymmetricCryptoKey::Aes256CbcKey(key)) => { - SymmetricCryptoKey::try_from(&BitwardenLegacyKeyBytes::from( - crate::aes::decrypt_aes256(iv, data.clone(), &key.enc_key)?, - ))? + (EncString::Aes256Cbc_B64 { .. }, SymmetricCryptoKey::Aes256CbcKey(_)) => { + return Err(CryptoError::OperationNotSupported( + UnsupportedOperationError::DecryptionNotImplementedForKey, + )); } ( EncString::Aes256Cbc_HmacSha256_B64 { iv, mac, data }, @@ -517,8 +517,10 @@ impl KeyStoreContext<'_, Ids> { let key = self.get_symmetric_key(key)?; match (data, key) { - (EncString::Aes256Cbc_B64 { iv, data }, SymmetricCryptoKey::Aes256CbcKey(key)) => { - crate::aes::decrypt_aes256(iv, data.clone(), &key.enc_key) + (EncString::Aes256Cbc_B64 { .. }, SymmetricCryptoKey::Aes256CbcKey(_)) => { + Err(CryptoError::OperationNotSupported( + UnsupportedOperationError::DecryptionNotImplementedForKey, + )) } ( EncString::Aes256Cbc_HmacSha256_B64 { iv, mac, data }, @@ -605,8 +607,8 @@ mod tests { use crate::{ AsymmetricCryptoKey, AsymmetricPublicCryptoKey, CompositeEncryptable, CoseKeyBytes, - CoseSerializable, CryptoError, Decryptable, KeyDecryptable, LocalId, Pkcs8PrivateKeyBytes, - SignatureAlgorithm, SigningKey, SigningNamespace, SymmetricCryptoKey, + CoseSerializable, CryptoError, Decryptable, EncString, KeyDecryptable, LocalId, + Pkcs8PrivateKeyBytes, SignatureAlgorithm, SigningKey, SigningNamespace, SymmetricCryptoKey, store::{ KeyStore, tests::{Data, DataView}, @@ -894,4 +896,91 @@ mod tests { "Expected encrypt to fail with KeyOperationNotSupported", ); } + + #[test] + fn test_encrypt_decrypt_data_fails_when_key_is_type_0() { + let store = KeyStore::::default(); + let mut ctx = store.context_mut(); + + let key_id = TestSymmKey::A(0); + let key = SymmetricCryptoKey::Aes256CbcKey(crate::Aes256CbcKey { + enc_key: Box::pin([0u8; 32].into()), + }); + ctx.set_symmetric_key_internal(key_id, key).unwrap(); + + let data_to_encrypt: Vec = vec![1, 2, 3, 4, 5]; + let result = ctx.encrypt_data_with_symmetric_key( + key_id, + &data_to_encrypt, + crate::ContentFormat::OctetStream, + ); + assert!( + matches!( + result, + Err(CryptoError::OperationNotSupported( + crate::error::UnsupportedOperationError::EncryptionNotImplementedForKey + )) + ), + "Expected encrypt to fail when using deprecated type 0 keys", + ); + + let data_to_decrypt = EncString::Aes256Cbc_B64 { + iv: [0; 16], + data: data_to_encrypt, + }; // dummy value; shouldn't matter + let result = ctx.decrypt_data_with_symmetric_key(key_id, &data_to_decrypt); + assert!( + matches!( + result, + Err(CryptoError::OperationNotSupported( + crate::error::UnsupportedOperationError::DecryptionNotImplementedForKey + )) + ), + "Expected decrypt to fail when using deprecated type 0 keys", + ); + } + + #[test] + fn test_wrap_unwrap_key_fails_when_key_is_type_0() { + let store = KeyStore::::default(); + let mut ctx = store.context_mut(); + + let wrapping_key_id = TestSymmKey::A(0); + let wrapping_key = SymmetricCryptoKey::Aes256CbcKey(crate::Aes256CbcKey { + enc_key: Box::pin([0u8; 32].into()), + }); + ctx.set_symmetric_key_internal(wrapping_key_id, wrapping_key) + .unwrap(); + + let key_to_wrap_id = TestSymmKey::A(1); + let key_to_wrap = SymmetricCryptoKey::make_aes256_cbc_hmac_key(); + ctx.set_symmetric_key_internal(key_to_wrap_id, key_to_wrap) + .unwrap(); + + let result = ctx.wrap_symmetric_key(wrapping_key_id, key_to_wrap_id); + assert!( + matches!( + result, + Err(CryptoError::OperationNotSupported( + crate::error::UnsupportedOperationError::EncryptionNotImplementedForKey + )) + ), + "Expected encrypt to fail when using deprecated type 0 keys", + ); + + let wrapped_key = &EncString::Aes256Cbc_B64 { + iv: [0; 16], + data: vec![0], + }; // dummy value; shouldn't matter + let result = ctx.unwrap_symmetric_key(wrapping_key_id, wrapped_key); + assert!( + matches!( + result, + Err(CryptoError::OperationNotSupported( + crate::error::UnsupportedOperationError::DecryptionNotImplementedForKey + )) + ), + "Expected decrypt to fail when using deprecated type 0 keys", + ); + } }