Merge branch 'km/cose' into km/cose-signatures

Author: quexten

crates/bitwarden-core/src/auth/tde.rs

@@ -17,7 +17,7 @@ pub(super) fn make_register_tde_keys(
 ) -> Result<RegisterTdeKeyResponse, EncryptionSettingsError> {
     let public_key = AsymmetricPublicCryptoKey::from_der(&STANDARD.decode(org_public_key)?)?;
 
-    let user_key = UserKey::new(SymmetricCryptoKey::generate());
+    let user_key = UserKey::new(SymmetricCryptoKey::generate_aes256_cbc_hmac());
     let key_pair = user_key.make_key_pair()?;
 
     let admin_reset = UnauthenticatedSharedKey::encapsulate_key_unsigned(&user_key.0, &public_key)?;

crates/bitwarden-crypto/Cargo.toml

@@ -42,6 +42,7 @@ num-bigint = ">=0.4, <0.5"
 num-traits = ">=0.2.15, <0.3"
 pbkdf2 = { version = ">=0.12.1, <0.13", default-features = false }
 rand = ">=0.8.5, <0.9"
+rand_chacha = ">=0.3.1, <0.4.0"
 rayon = ">=1.8.1, <2.0"
 rsa = ">=0.9.2, <0.10"
 schemars = { workspace = true }

crates/bitwarden-crypto/README.md

@@ -16,7 +16,7 @@ secure.
 use bitwarden_crypto::{SymmetricCryptoKey, KeyEncryptable, KeyDecryptable, CryptoError};
 
 async fn example() -> Result<(), CryptoError> {
-  let key = SymmetricCryptoKey::generate();
+  let key = SymmetricCryptoKey::generate_aes256_cbc_hmac();
 
   let data = "Hello, World!".to_owned();
   let encrypted = data.clone().encrypt_with_key(&key)?;

crates/bitwarden-crypto/src/enc_string/asymmetric.rs

@@ -263,11 +263,10 @@ XKZBokBGnjFnTnKcs7nv/O8=
     #[test]
     fn test_enc_string_rsa2048_oaep_sha256_b64() {
         let key_pair = AsymmetricCryptoKey::from_pem(RSA_PRIVATE_KEY).unwrap();
-        let enc_str: &str = "3.BfwZTwBYbU5WQ5X7Vm8yl0hYmHTRdkVACCRZYcqhcjicoaPVDEP03CIRmtnppu0aXOppoQzhw5S2OKTUaqoOGKZg7+PrmVEhjiUFfVAptInBD6XGHZ0Z3u3F+JY1E3xIFebOFiX7KLQ+7D0bJhBEnl8P7phmanKF3Cil5ayDGRpAjAsBHMwlNRKXy05YpYs3/x+V+zjlxVrBU9gYFCpacKUbxT51I8tf21ISqo6H9ZBwqDE2QUPhYJl5op7SJgySdd3YCKnsObXa8fFj2OwxGLAXJAyvF6qZyl08RO/ZYUOOOPlbC7ywXxAISw3qmrwxqpLSBqAm9BYPa/zxBnTHrA==";
+        let enc_str: &str = "3.SUx5gWrgmAKs/S1BoQrqOmx2Hl5fPVBVHokW17Flvm4TpBnJJRkfoitp7Jc4dfazPYjWGlckJz6X+qe+/AWilS1mxtzS0PmDy7tS5xP0GRlB39dstCd5jDw1wPmTbXiLcQ5VTvzpRAfRMEYVveTsEvVTByvEYAGSn4TnCsUDykyhRbD0YcJ4r1KHLs1b3BCBy2M1Gl5nmwckH08CAXaf8VfuBFStAGRKueovqp4euneQla+4G4fXdVvb8qKPnu0iVuALIE6nUNmeOiA3xN3d+akMxbbGxrQ1Ca4TYWjHVdj9C6abngQHkjKNYQwGUXrYo160hP4LIHn/huK6bZe5dQ==";
         let enc_string: UnauthenticatedSharedKey = enc_str.parse().unwrap();
 
-        let test_bytes = vec![0u8; 64];
-        let test_key = SymmetricCryptoKey::try_from(test_bytes).unwrap();
+        let test_key = SymmetricCryptoKey::generate_seeded_for_unit_tests("test");
         assert_eq!(enc_string.enc_type(), 3);
 
         let res = enc_string.decapsulate_key_unsigned(&key_pair).unwrap();
@@ -277,29 +276,28 @@ XKZBokBGnjFnTnKcs7nv/O8=
     #[test]
     fn test_enc_string_rsa2048_oaep_sha1_b64() {
         let private_key = AsymmetricCryptoKey::from_pem(RSA_PRIVATE_KEY).unwrap();
-        let enc_str: &str = "4.KhZmkc7f2WYuZGm/xlKZOK4c5JSwd9JtJvmyk0R+ZCqbRnZi5XNJaqnMiJjiqeLztE97bHRGWyDPvhyIisr7jLi35vL/Znpg3QzSMEDNI7aAM2FwJbCzdUrFDa/h08edv816AL1hAOqtGmjpfRL1j+47hlAiF3/srFCeePHkj0+CmHpHN13BN1XkLKk58mETKh8ky/ZUW2s4NjZaZ/Wxh6I9sv28L+u1hekKxDOdNKBnmqsh8WRBOtmZm1ZM9WI6aPA5tXgp30vxWrc1AsZ5Ts0aVkm8UzPTWuU9d/O9ICAQkr1hX58qO6M5geP+NvaG3UGymw0zp6Hdgz239XYpKg==";
+        let enc_str: &str = "4.DMD1D5r6BsDDd7C/FE1eZbMCKrmryvAsCKj6+bO54gJNUxisOI7SDcpPLRXf+JdhqY15pT+wimQ5cD9C+6OQ6s71LFQHewXPU29l9Pa1JxGeiKqp37KLYf+1IS6UB2K3ANN35C52ZUHh2TlzIS5RuntxnpCw7APbcfpcnmIdLPJBtuj/xbFd6eBwnI3GSe5qdS6/Ixdd0dgsZcpz3gHJBKmIlSo0YN60SweDq3kTJwox9xSqdCueIDg5U4khc7RhjYx8b33HXaNJj3DwgIH8iLj+lqpDekogr630OhHG3XRpvl4QzYO45bmHb8wAh67Dj70nsZcVg6bAEFHdSFohww==";
         let enc_string: UnauthenticatedSharedKey = enc_str.parse().unwrap();
 
-        let test_bytes = vec![0u8; 64];
-        let test_bytes = SymmetricCryptoKey::try_from(test_bytes).unwrap();
+        let test_key = SymmetricCryptoKey::generate_seeded_for_unit_tests("test");
         assert_eq!(enc_string.enc_type(), 4);
 
         let res = enc_string.decapsulate_key_unsigned(&private_key).unwrap();
-        assert_eq!(res, test_bytes);
+        assert_eq!(res, test_key);
     }
 
     #[test]
     fn test_enc_string_rsa2048_oaep_sha1_hmac_sha256_b64() {
         let private_key = AsymmetricCryptoKey::from_pem(RSA_PRIVATE_KEY).unwrap();
-        let enc_str: &str = "6.KhZmkc7f2WYuZGm/xlKZOK4c5JSwd9JtJvmyk0R+ZCqbRnZi5XNJaqnMiJjiqeLztE97bHRGWyDPvhyIisr7jLi35vL/Znpg3QzSMEDNI7aAM2FwJbCzdUrFDa/h08edv816AL1hAOqtGmjpfRL1j+47hlAiF3/srFCeePHkj0+CmHpHN13BN1XkLKk58mETKh8ky/ZUW2s4NjZaZ/Wxh6I9sv28L+u1hekKxDOdNKBnmqsh8WRBOtmZm1ZM9WI6aPA5tXgp30vxWrc1AsZ5Ts0aVkm8UzPTWuU9d/O9ICAQkr1hX58qO6M5geP+NvaG3UGymw0zp6Hdgz239XYpKg==|AA==";
+        let enc_str: &str = "6.DMD1D5r6BsDDd7C/FE1eZbMCKrmryvAsCKj6+bO54gJNUxisOI7SDcpPLRXf+JdhqY15pT+wimQ5cD9C+6OQ6s71LFQHewXPU29l9Pa1JxGeiKqp37KLYf+1IS6UB2K3ANN35C52ZUHh2TlzIS5RuntxnpCw7APbcfpcnmIdLPJBtuj/xbFd6eBwnI3GSe5qdS6/Ixdd0dgsZcpz3gHJBKmIlSo0YN60SweDq3kTJwox9xSqdCueIDg5U4khc7RhjYx8b33HXaNJj3DwgIH8iLj+lqpDekogr630OhHG3XRpvl4QzYO45bmHb8wAh67Dj70nsZcVg6bAEFHdSFohww==|AA==";
         let enc_string: UnauthenticatedSharedKey = enc_str.parse().unwrap();
 
-        let test_bytes = vec![0u8; 64];
-        let test_key = SymmetricCryptoKey::try_from(test_bytes).unwrap();
+        let test_key: SymmetricCryptoKey =
+            SymmetricCryptoKey::generate_seeded_for_unit_tests("test");
         assert_eq!(enc_string.enc_type(), 6);
 
         let res = enc_string.decapsulate_key_unsigned(&private_key).unwrap();
-        assert_eq!(res, test_key);
+        assert_eq!(res.to_base64(), test_key.to_base64());
     }
 
     #[test]

crates/bitwarden-crypto/src/keys/asymmetric_crypto_key.rs

@@ -216,7 +216,7 @@ DnqOsltgPomWZ7xVfMkm9niL2OA=
         let private_key = AsymmetricCryptoKey::from_der(&private_key).unwrap();
         let public_key = AsymmetricPublicCryptoKey::from_der(&public_key).unwrap();
 
-        let raw_key = SymmetricCryptoKey::generate();
+        let raw_key = SymmetricCryptoKey::generate_aes256_cbc_hmac();
         let encrypted =
             UnauthenticatedSharedKey::encapsulate_key_unsigned(&raw_key, &public_key).unwrap();
         let decrypted = encrypted.decapsulate_key_unsigned(&private_key).unwrap();

crates/bitwarden-crypto/src/keys/device_key.rs

@@ -30,7 +30,7 @@ impl DeviceKey {
     /// from EncSettings.
     pub fn trust_device(user_key: &SymmetricCryptoKey) -> Result<TrustDeviceResponse> {
         let mut rng = rand::thread_rng();
-        let device_key = DeviceKey(SymmetricCryptoKey::generate());
+        let device_key = DeviceKey(SymmetricCryptoKey::generate_aes256_cbc_hmac());
 
         let device_private_key = AsymmetricCryptoKey::generate(&mut rng);
 

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

@@ -160,8 +160,14 @@ pub(super) fn decrypt_user_key(
 }
 
 /// Generate a new random user key and encrypt it with the master key.
-fn make_user_key(rng: impl rand::RngCore, master_key: &MasterKey) -> Result<(UserKey, EncString)> {
-    let user_key = SymmetricCryptoKey::generate_internal(rng, false);
+///
+/// WARNING: This function should only be used with a proper cryptographic random number generator.
+/// If you do not have a good reason for using this, use [MasterKey::make_user_key] instead.
+fn make_user_key(
+    mut rng: impl rand::RngCore,
+    master_key: &MasterKey,
+) -> Result<(UserKey, EncString)> {
+    let user_key = SymmetricCryptoKey::generate_aes256_cbc_hmac_internal(&mut rng);
     let protected = master_key.encrypt_user_key(&user_key)?;
     Ok((UserKey::new(user_key), protected))
 }

crates/bitwarden-crypto/src/keys/symmetric_crypto_key.rs

@@ -4,15 +4,17 @@ use aes::cipher::typenum::U32;
 use base64::{engine::general_purpose::STANDARD, Engine};
 use coset::{iana, CborSerializable, Label, RegisteredLabelWithPrivate};
 use generic_array::GenericArray;
-use rand::Rng;
+use rand::{Rng, SeedableRng};
+use rand_chacha::ChaChaRng;
+use sha2::Digest;
 use subtle::{Choice, ConstantTimeEq};
 use zeroize::{Zeroize, ZeroizeOnDrop};
 
 use super::{key_encryptable::CryptoKey, key_id::KeyId};
 use crate::{cose, CryptoError};
 
-/// Aes256CbcKey is a symmetric encryption key, consisting of one 256-bit key,
-/// used to decrypt legacy type 0 encstrings. The data is not autenticated
+/// [Aes256CbcKey] is a symmetric encryption key, consisting of one 256-bit key,
+/// used to decrypt legacy type 0 enc strings. The data is not authenticated
 /// so this should be used with caution, and removed where possible.
 #[derive(ZeroizeOnDrop, Clone)]
 pub struct Aes256CbcKey {
@@ -32,7 +34,7 @@ impl PartialEq for Aes256CbcKey {
     }
 }
 
-/// Aes256CbcHmacKey is a symmetric encryption key consisting
+/// [Aes256CbcHmacKey] is a symmetric encryption key consisting
 /// of two 256-bit keys, one for encryption and one for MAC
 #[derive(ZeroizeOnDrop, Clone)]
 pub struct Aes256CbcHmacKey {
@@ -88,49 +90,46 @@ impl SymmetricCryptoKey {
     // enc type 2 old static format
     const AES256_CBC_HMAC_KEY_LEN: usize = 64;
 
+    /// Generate a new random AES256_CBC [SymmetricCryptoKey]
+    ///
+    /// WARNING: This function should only be used with a proper cryptographic RNG. If you do not
+    /// have a good reason for using this function, use
+    /// [SymmetricCryptoKey::generate_aes256_cbc_hmac] instead.
+    pub(crate) fn generate_aes256_cbc_hmac_internal(rng: &mut impl Rng) -> Self {
+        let mut enc_key = Box::pin(GenericArray::<u8, U32>::default());
+        let mut mac_key = Box::pin(GenericArray::<u8, U32>::default());
+
+        rng.fill(enc_key.as_mut_slice());
+        rng.fill(mac_key.as_mut_slice());
+
+        SymmetricCryptoKey::Aes256CbcHmacKey(Aes256CbcHmacKey { enc_key, mac_key })
+    }
+
     /**
      * Generate a new random AES256_CBC_HMAC [SymmetricCryptoKey]
      */
-    pub fn generate() -> Self {
+    pub fn generate_aes256_cbc_hmac() -> Self {
         let mut rng = rand::thread_rng();
-        Self::generate_internal(&mut rng, false)
+        Self::generate_aes256_cbc_hmac_internal(&mut rng)
     }
 
     /**
      * Generate a new random XChaCha20Poly1305 [SymmetricCryptoKey]
      */
     pub fn generate_xchacha20() -> Self {
         let mut rng = rand::thread_rng();
-        Self::generate_internal(&mut rng, true)
-    }
-
-    /// Generate a new random [SymmetricCryptoKey]
-    /// @param rng: A random number generator
-    /// @param xchacha20: If true, generate an XChaCha20Poly1305 key, otherwise generate an
-    /// AES256_CBC_HMAC key
-    pub(crate) fn generate_internal(mut rng: impl rand::RngCore, xchacha20: bool) -> Self {
-        if !xchacha20 {
-            let mut enc_key = Box::pin(GenericArray::<u8, U32>::default());
-            let mut mac_key = Box::pin(GenericArray::<u8, U32>::default());
-
-            rng.fill(enc_key.as_mut_slice());
-            rng.fill(mac_key.as_mut_slice());
-
-            SymmetricCryptoKey::Aes256CbcHmacKey(Aes256CbcHmacKey { enc_key, mac_key })
-        } else {
-            let mut enc_key = Box::pin(GenericArray::<u8, U32>::default());
-            rng.fill(enc_key.as_mut_slice());
-            SymmetricCryptoKey::XChaCha20Poly1305Key(XChaCha20Poly1305Key {
-                enc_key,
-                key_id: *KeyId::generate().as_bytes(),
-            })
-        }
+        let mut enc_key = Box::pin(GenericArray::<u8, U32>::default());
+        rng.fill(enc_key.as_mut_slice());
+        SymmetricCryptoKey::XChaCha20Poly1305Key(XChaCha20Poly1305Key {
+            enc_key,
+            key_id: *KeyId::generate().as_bytes(),
+        })
     }
 
     /// Encodes the key to a byte array representation, that is separated by size.
-    /// `SymmetricCryptoKey::Aes256CbcHmacKey` and `SymmetricCryptoKey::Aes256CbcKey` are
-    /// encoded as 64 and 32 bytes respectively. `SymmetricCryptoKey::XChaCha20Poly1305Key`
-    /// is encoded as at least 65 bytes, by using padding defined in `pad_key`.
+    /// [SymmetricCryptoKey::Aes256CbcHmacKey] and [SymmetricCryptoKey::Aes256CbcKey] are
+    /// encoded as 64 and 32 bytes respectively. [SymmetricCryptoKey::XChaCha20Poly1305Key]
+    /// is encoded as at least 65 bytes, using padding.
     ///
     /// This can be used for storage and transmission in the old byte array format.
     /// When the wrapping key is a COSE key, and the wrapped key is a COSE key, then this should
@@ -148,6 +147,20 @@ impl SymmetricCryptoKey {
         }
     }
 
+    /// Generate a new random [SymmetricCryptoKey] for unit tests. Note: DO NOT USE THIS
+    /// IN PRODUCTION CODE.
+    pub fn generate_seeded_for_unit_tests(seed: &str) -> Self {
+        // Keep this separate from the other generate function to not break test vectors.
+        let mut seeded_rng = ChaChaRng::from_seed(sha2::Sha256::digest(seed.as_bytes()).into());
+        let mut enc_key = Box::pin(GenericArray::<u8, U32>::default());
+        let mut mac_key = Box::pin(GenericArray::<u8, U32>::default());
+
+        seeded_rng.fill(enc_key.as_mut_slice());
+        seeded_rng.fill(mac_key.as_mut_slice());
+
+        SymmetricCryptoKey::Aes256CbcHmacKey(Aes256CbcHmacKey { enc_key, mac_key })
+    }
+
     pub(crate) fn to_encoded_raw(&self) -> Vec<u8> {
         match self {
             SymmetricCryptoKey::Aes256CbcKey(key) => key.enc_key.to_vec(),
@@ -299,6 +312,12 @@ fn parse_cose_key(cose_key: &coset::CoseKey) -> Result<SymmetricCryptoKey, Crypt
     }
 }
 
+impl From<Aes256CbcHmacKey> for SymmetricCryptoKey {
+    fn from(key: Aes256CbcHmacKey) -> Self {
+        SymmetricCryptoKey::Aes256CbcHmacKey(key)
+    }
+}
+
 impl CryptoKey for SymmetricCryptoKey {}
 
 // We manually implement these to make sure we don't print any sensitive data
@@ -312,29 +331,29 @@ impl std::fmt::Debug for SymmetricCryptoKey {
 /// The last N bytes of the padded bytes all have the value N.
 /// For example, padded to size 4, the value 0,0 becomes 0,0,2,2.
 ///
-/// Keys that have the type `SymmetricCryptoKey::XChaCha20Poly1305Key` must be distinguishable
-/// from `SymmetricCryptoKey::Aes256CbcHmacKey` keys, when both are encoded as byte arrays
+/// Keys that have the type [SymmetricCryptoKey::XChaCha20Poly1305Key] must be distinguishable
+/// from [SymmetricCryptoKey::Aes256CbcHmacKey] keys, when both are encoded as byte arrays
 /// with no additional content format included in the encoding message. For this reason, the
 /// padding is used to make sure that the byte representation uniquely separates the keys by
-/// size of the byte array. The previous key types `SymmetricCryptoKey::Aes256CbcHmacKey` and
-/// `SymmetricCryptoKey::Aes256CbcKey` are 64 and 32 bytes long respectively.
+/// size of the byte array. The previous key types [SymmetricCryptoKey::Aes256CbcHmacKey] and
+/// [SymmetricCryptoKey::Aes256CbcKey] are 64 and 32 bytes long respectively.
 fn pad_key(key_bytes: &mut Vec<u8>, min_length: usize) {
     // at least 1 byte of padding is required
     let pad_bytes = min_length.saturating_sub(key_bytes.len()).max(1);
     let padded_length = max(min_length, key_bytes.len() + 1);
     key_bytes.resize(padded_length, pad_bytes as u8);
 }
 
-/// Unpad a key that is padded using the PKCS7-like padding defined by `pad_key`.
+/// Unpad a key that is padded using the PKCS7-like padding defined by [pad_key].
 /// The last N bytes of the padded bytes all have the value N.
 /// For example, padded to size 4, the value 0,0 becomes 0,0,2,2.
 ///
-/// Keys that have the type `SymmetricCryptoKey::XChaCha20Poly1305Key` must be distinguishable
-/// from `SymmetricCryptoKey::Aes256CbcHmacKey` keys, when both are encoded as byte arrays
+/// Keys that have the type [SymmetricCryptoKey::XChaCha20Poly1305Key] must be distinguishable
+/// from [SymmetricCryptoKey::Aes256CbcHmacKey] keys, when both are encoded as byte arrays
 /// with no additional content format included in the encoding message. For this reason, the
 /// padding is used to make sure that the byte representation uniquely separates the keys by
-/// size of the byte array the previous key types `SymmetricCryptoKey::Aes256CbcHmacKey` and
-/// `SymmetricCryptoKey::Aes256CbcKey` are 64 and 32 bytes long respectively.
+/// size of the byte array the previous key types [SymmetricCryptoKey::Aes256CbcHmacKey] and
+/// [SymmetricCryptoKey::Aes256CbcKey] are 64 and 32 bytes long respectively.
 fn unpad_key(key_bytes: &[u8]) -> &[u8] {
     // this unwrap is safe, the input is always at least 1 byte long
     #[allow(clippy::unwrap_used)]
@@ -373,7 +392,7 @@ mod tests {
 
     #[test]
     fn test_encode_decode_old_symmetric_crypto_key() {
-        let key = SymmetricCryptoKey::generate_internal(rand::thread_rng(), false);
+        let key = SymmetricCryptoKey::generate_aes256_cbc_hmac();
         let encoded = key.to_encoded();
         let decoded = SymmetricCryptoKey::try_from(encoded).unwrap();
         assert_eq!(key, decoded);
@@ -391,10 +410,10 @@ mod tests {
 
     #[test]
     fn test_encode_xchacha20_poly1305_key() {
-        let key = SymmetricCryptoKey::generate_internal(rand::thread_rng(), true);
-        let key_vec = key.to_encoded();
-        let key_vec_utf8_lossy = String::from_utf8_lossy(&key_vec);
-        println!("key_vec: {:?}", key_vec_utf8_lossy);
+        let key = SymmetricCryptoKey::generate_xchacha20();
+        let encoded = key.to_encoded();
+        let decoded = SymmetricCryptoKey::try_from(encoded).unwrap();
+        assert_eq!(key, decoded);
     }
 
     #[test]

crates/bitwarden-crypto/src/store/backend/implementation/mod.rs

@@ -17,7 +17,7 @@ mod tests {
     fn test_creates_a_valid_store() {
         let mut store = create_store::<TestSymmKey>();
 
-        let key = SymmetricCryptoKey::generate();
+        let key = SymmetricCryptoKey::generate_aes256_cbc_hmac();
         store.upsert(TestSymmKey::A(0), key.clone());
 
         assert_eq!(