[PM-5728] Enable Zeroize support for keys and EncStrings (#515)

## Type of change
```
- [ ] Bug fix
- [x] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other
```

## Objective
- Implemented Zeroize support for the encryption keys and the
encStrings.
- Boxed the internals of the encryption keys to avoid Rust from moving
them around on the stack.
- Changed some functions that passed around owned copies of the
GenericArrays to pass references instead. GenericArray implements Copy,
so passing them around by value only means more stack copies if the
compiler can't optimize them out.
- Changed the from impls for SymmCryptoKey to take mut slices so we can
zeroize them after load.

crates/bitwarden-crypto/Cargo.toml

@@ -18,12 +18,14 @@ default = []
 mobile = ["uniffi"]
 
 [dependencies]
-aes = ">=0.8.2, <0.9"
+aes = { version = ">=0.8.2, <0.9", features = ["zeroize"] }
 argon2 = { version = ">=0.5.0, <0.6", features = [
     "alloc",
+    "zeroize",
 ], default-features = false }
 base64 = ">=0.21.2, <0.22"
-cbc = { version = ">=0.1.2, <0.2", features = ["alloc"] }
+cbc = { version = ">=0.1.2, <0.2", features = ["alloc", "zeroize"] }
+generic-array = { version = ">=0.14.7, <1.0", features = ["zeroize"] }
 hkdf = ">=0.12.3, <0.13"
 hmac = ">=0.12.1, <0.13"
 num-bigint = ">=0.4, <0.5"
@@ -39,6 +41,7 @@ subtle = ">=2.5.0, <3.0"
 thiserror = ">=1.0.40, <2.0"
 uniffi = { version = "=0.25.2", optional = true }
 uuid = { version = ">=1.3.3, <2.0", features = ["serde"] }
+zeroize = { version = ">=1.7.0, <2.0", features = ["derive", "aarch64"] }
 
 [dev-dependencies]
 rand_chacha = "0.3.1"

crates/bitwarden-crypto/src/aes.rs

@@ -6,9 +6,9 @@
 //! [KeyEncryptable][crate::KeyEncryptable] & [KeyDecryptable][crate::KeyDecryptable] instead.
 
 use aes::cipher::{
-    block_padding::Pkcs7, generic_array::GenericArray, typenum::U32, BlockDecryptMut,
-    BlockEncryptMut, KeyIvInit,
+    block_padding::Pkcs7, typenum::U32, BlockDecryptMut, BlockEncryptMut, KeyIvInit,
 };
+use generic_array::GenericArray;
 use hmac::Mac;
 use subtle::ConstantTimeEq;
 
@@ -23,12 +23,12 @@ use crate::{
 pub(crate) fn decrypt_aes256(
     iv: &[u8; 16],
     data: Vec<u8>,
-    key: GenericArray<u8, U32>,
+    key: &GenericArray<u8, U32>,
 ) -> Result<Vec<u8>> {
     // Decrypt data
     let iv = GenericArray::from_slice(iv);
     let mut data = data;
-    let decrypted_key_slice = cbc::Decryptor::<aes::Aes256>::new(&key, iv)
+    let decrypted_key_slice = cbc::Decryptor::<aes::Aes256>::new(key, iv)
         .decrypt_padded_mut::<Pkcs7>(&mut data)
         .map_err(|_| CryptoError::KeyDecrypt)?;
 
@@ -47,10 +47,10 @@ pub(crate) fn decrypt_aes256_hmac(
     iv: &[u8; 16],
     mac: &[u8; 32],
     data: Vec<u8>,
-    mac_key: GenericArray<u8, U32>,
-    key: GenericArray<u8, U32>,
+    mac_key: &GenericArray<u8, U32>,
+    key: &GenericArray<u8, U32>,
 ) -> Result<Vec<u8>> {
-    let res = generate_mac(&mac_key, iv, &data)?;
+    let res = generate_mac(mac_key, iv, &data)?;
     if res.ct_ne(mac).into() {
         return Err(CryptoError::InvalidMac);
     }
@@ -65,7 +65,7 @@ pub(crate) fn decrypt_aes256_hmac(
 ///
 /// A AesCbc256_B64 EncString
 #[allow(unused)]
-pub(crate) fn encrypt_aes256(data_dec: &[u8], key: GenericArray<u8, U32>) -> ([u8; 16], Vec<u8>) {
+pub(crate) fn encrypt_aes256(data_dec: &[u8], key: &GenericArray<u8, U32>) -> ([u8; 16], Vec<u8>) {
     let rng = rand::thread_rng();
     let (iv, data) = encrypt_aes256_internal(rng, data_dec, key);
 
@@ -81,12 +81,12 @@ pub(crate) fn encrypt_aes256(data_dec: &[u8], key: GenericArray<u8, U32>) -> ([u
 /// A AesCbc256_HmacSha256_B64 EncString
 pub(crate) fn encrypt_aes256_hmac(
     data_dec: &[u8],
-    mac_key: GenericArray<u8, U32>,
-    key: GenericArray<u8, U32>,
+    mac_key: &GenericArray<u8, U32>,
+    key: &GenericArray<u8, U32>,
 ) -> Result<([u8; 16], [u8; 32], Vec<u8>)> {
     let rng = rand::thread_rng();
     let (iv, data) = encrypt_aes256_internal(rng, data_dec, key);
-    let mac = generate_mac(&mac_key, &iv, &data)?;
+    let mac = generate_mac(mac_key, &iv, &data)?;
 
     Ok((iv, mac, data))
 }
@@ -99,11 +99,11 @@ pub(crate) fn encrypt_aes256_hmac(
 fn encrypt_aes256_internal(
     mut rng: impl rand::RngCore,
     data_dec: &[u8],
-    key: GenericArray<u8, U32>,
+    key: &GenericArray<u8, U32>,
 ) -> ([u8; 16], Vec<u8>) {
     let mut iv = [0u8; 16];
     rng.fill_bytes(&mut iv);
-    let data = cbc::Encryptor::<aes::Aes256>::new(&key, &iv.into())
+    let data = cbc::Encryptor::<aes::Aes256>::new(key, &iv.into())
         .encrypt_padded_vec_mut::<Pkcs7>(data_dec);
 
     (iv, data)
@@ -123,8 +123,8 @@ fn generate_mac(mac_key: &[u8], iv: &[u8], data: &[u8]) -> Result<[u8; 32]> {
 
 #[cfg(test)]
 mod tests {
-    use aes::cipher::generic_array::sequence::GenericSequence;
     use base64::{engine::general_purpose::STANDARD, Engine};
+    use generic_array::sequence::GenericSequence;
     use rand::SeedableRng;
 
     use super::*;
@@ -146,7 +146,7 @@ mod tests {
         let key = generate_generic_array(0, 1);
 
         let rng = rand_chacha::ChaCha8Rng::from_seed([0u8; 32]);
-        let result = encrypt_aes256_internal(rng, "EncryptMe!".as_bytes(), key);
+        let result = encrypt_aes256_internal(rng, "EncryptMe!".as_bytes(), &key);
         assert_eq!(
             result,
             (
@@ -177,7 +177,7 @@ mod tests {
         let key = generate_generic_array(0, 1);
         let data = STANDARD.decode("ByUF8vhyX4ddU9gcooznwA==").unwrap();
 
-        let decrypted = decrypt_aes256(iv, data, key).unwrap();
+        let decrypted = decrypt_aes256(iv, data, &key).unwrap();
 
         assert_eq!(String::from_utf8(decrypted).unwrap(), "EncryptMe!");
     }
@@ -187,8 +187,8 @@ mod tests {
         let key = generate_generic_array(0, 1);
         let data = "EncryptMe!";
 
-        let (iv, encrypted) = encrypt_aes256(data.as_bytes(), key);
-        let decrypted = decrypt_aes256(&iv, encrypted, key).unwrap();
+        let (iv, encrypted) = encrypt_aes256(data.as_bytes(), &key);
+        let decrypted = decrypt_aes256(&iv, encrypted, &key).unwrap();
 
         assert_eq!(String::from_utf8(decrypted).unwrap(), "EncryptMe!");
     }

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

@@ -1,6 +1,7 @@
 use std::{fmt::Display, str::FromStr};
 
 use base64::{engine::general_purpose::STANDARD, Engine};
+pub use internal::AsymmetricEncString;
 use rsa::Oaep;
 use serde::Deserialize;
 
@@ -11,48 +12,53 @@ use crate::{
     AsymmetricCryptoKey, AsymmetricEncryptable, KeyDecryptable,
 };
 
-/// # Encrypted string primitive
-///
-/// [AsymmetricEncString] is a Bitwarden specific primitive that represents an asymmetrically
-/// encrypted string. They are used together with the KeyDecryptable and KeyEncryptable traits to
-/// encrypt and decrypt data using [AsymmetricCryptoKey]s.
-///
-/// The flexibility of the [AsymmetricEncString] type allows for different encryption algorithms to
-/// be used which is represented by the different variants of the enum.
-///
-/// ## Note
-///
-/// For backwards compatibility we will rarely if ever be able to remove support for decrypting old
-/// variants, but we should be opinionated in which variants are used for encrypting.
-///
-/// ## Variants
-/// - [Rsa2048_OaepSha256_B64](AsymmetricEncString::Rsa2048_OaepSha256_B64)
-/// - [Rsa2048_OaepSha1_B64](AsymmetricEncString::Rsa2048_OaepSha1_B64)
-///
-/// ## Serialization
-///
-/// [AsymmetricEncString] implements [Display] and [FromStr] to allow for easy serialization and
-/// uses a custom scheme to represent the different variants.
-///
-/// The scheme is one of the following schemes:
-/// - `[type].[data]`
-///
-/// Where:
-/// - `[type]`: is a digit number representing the variant.
-/// - `[data]`: is the encrypted data.
-#[derive(Clone)]
-#[allow(unused, non_camel_case_types)]
-pub enum AsymmetricEncString {
-    /// 3
-    Rsa2048_OaepSha256_B64 { data: Vec<u8> },
-    /// 4
-    Rsa2048_OaepSha1_B64 { data: Vec<u8> },
-    /// 5
-    #[deprecated]
-    Rsa2048_OaepSha256_HmacSha256_B64 { data: Vec<u8>, mac: Vec<u8> },
-    /// 6
-    #[deprecated]
-    Rsa2048_OaepSha1_HmacSha256_B64 { data: Vec<u8>, mac: Vec<u8> },
+// This module is a workaround to avoid deprecated warnings that come from the ZeroizeOnDrop
+// macro expansion
+#[allow(deprecated)]
+mod internal {
+    /// # Encrypted string primitive
+    ///
+    /// [AsymmetricEncString] is a Bitwarden specific primitive that represents an asymmetrically
+    /// encrypted string. They are used together with the KeyDecryptable and KeyEncryptable
+    /// traits to encrypt and decrypt data using [crate::AsymmetricCryptoKey]s.
+    ///
+    /// The flexibility of the [AsymmetricEncString] type allows for different encryption algorithms
+    /// to be used which is represented by the different variants of the enum.
+    ///
+    /// ## Note
+    ///
+    /// For backwards compatibility we will rarely if ever be able to remove support for decrypting
+    /// old variants, but we should be opinionated in which variants are used for encrypting.
+    ///
+    /// ## Variants
+    /// - [Rsa2048_OaepSha256_B64](AsymmetricEncString::Rsa2048_OaepSha256_B64)
+    /// - [Rsa2048_OaepSha1_B64](AsymmetricEncString::Rsa2048_OaepSha1_B64)
+    ///
+    /// ## Serialization
+    ///
+    /// [AsymmetricEncString] implements [std::fmt::Display] and [std::str::FromStr] to allow for
+    /// easy serialization and uses a custom scheme to represent the different variants.
+    ///
+    /// The scheme is one of the following schemes:
+    /// - `[type].[data]`
+    ///
+    /// Where:
+    /// - `[type]`: is a digit number representing the variant.
+    /// - `[data]`: is the encrypted data.
+    #[derive(Clone, zeroize::ZeroizeOnDrop)]
+    #[allow(unused, non_camel_case_types)]
+    pub enum AsymmetricEncString {
+        /// 3
+        Rsa2048_OaepSha256_B64 { data: Vec<u8> },
+        /// 4
+        Rsa2048_OaepSha1_B64 { data: Vec<u8> },
+        /// 5
+        #[deprecated]
+        Rsa2048_OaepSha256_HmacSha256_B64 { data: Vec<u8>, mac: Vec<u8> },
+        /// 6
+        #[deprecated]
+        Rsa2048_OaepSha1_HmacSha256_B64 { data: Vec<u8>, mac: Vec<u8> },
+    }
 }
 
 /// To avoid printing sensitive information, [AsymmetricEncString] debug prints to

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

@@ -1,7 +1,8 @@
 use std::{fmt::Display, str::FromStr};
 
-use aes::cipher::{generic_array::GenericArray, typenum::U32};
+use aes::cipher::typenum::U32;
 use base64::{engine::general_purpose::STANDARD, Engine};
+use generic_array::GenericArray;
 use serde::Deserialize;
 
 use super::{check_length, from_b64, from_b64_vec, split_enc_string};
@@ -43,7 +44,7 @@ use crate::{
 /// - `[iv]`: (optional) is the initialization vector used for encryption.
 /// - `[data]`: is the encrypted data.
 /// - `[mac]`: (optional) is the MAC used to validate the integrity of the data.
-#[derive(Clone)]
+#[derive(Clone, zeroize::ZeroizeOnDrop)]
 #[allow(unused, non_camel_case_types)]
 pub enum EncString {
     /// 0
@@ -204,8 +205,8 @@ impl serde::Serialize for EncString {
 impl EncString {
     pub fn encrypt_aes256_hmac(
         data_dec: &[u8],
-        mac_key: GenericArray<u8, U32>,
-        key: GenericArray<u8, U32>,
+        mac_key: &GenericArray<u8, U32>,
+        key: &GenericArray<u8, U32>,
     ) -> Result<EncString> {
         let (iv, mac, data) = crate::aes::encrypt_aes256_hmac(data_dec, mac_key, key)?;
         Ok(EncString::AesCbc256_HmacSha256_B64 { iv, mac, data })
@@ -224,16 +225,21 @@ impl EncString {
 impl LocateKey for EncString {}
 impl KeyEncryptable<SymmetricCryptoKey, EncString> for &[u8] {
     fn encrypt_with_key(self, key: &SymmetricCryptoKey) -> Result<EncString> {
-        EncString::encrypt_aes256_hmac(self, key.mac_key.ok_or(CryptoError::InvalidMac)?, key.key)
+        EncString::encrypt_aes256_hmac(
+            self,
+            key.mac_key.as_ref().ok_or(CryptoError::InvalidMac)?,
+            &key.key,
+        )
     }
 }
 
 impl KeyDecryptable<SymmetricCryptoKey, Vec<u8>> for EncString {
     fn decrypt_with_key(&self, key: &SymmetricCryptoKey) -> Result<Vec<u8>> {
         match self {
             EncString::AesCbc256_HmacSha256_B64 { iv, mac, data } => {
-                let mac_key = key.mac_key.ok_or(CryptoError::InvalidMac)?;
-                let dec = crate::aes::decrypt_aes256_hmac(iv, mac, data.clone(), mac_key, key.key)?;
+                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),