[PM-5728] Enable Zeroize support for keys and EncStrings

crates/bitwarden-crypto/Cargo.toml

@@ -18,12 +18,13 @@ 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",
 ], default-features = false }
 base64 = ">=0.21.2, <0.22"
 cbc = { version = ">=0.1.2, <0.2", features = ["alloc"] }
+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 +40,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)?;
 
@@ -46,10 +46,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);
     }
@@ -64,7 +64,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);
 
@@ -80,12 +80,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))
 }
@@ -98,11 +98,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)
@@ -122,8 +122,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::*;
@@ -145,7 +145,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,
             (
@@ -176,7 +176,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!");
     }
@@ -186,8 +186,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

@@ -10,48 +10,53 @@ use crate::{
     AsymmetricCryptoKey, 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
+pub use internal::AsymmetricEncString;
+#[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
+    /// [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, 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 `AsymmetricEncString`.

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),

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

@@ -1,25 +1,38 @@
+use std::pin::Pin;
+
 use rsa::RsaPrivateKey;
 
 use super::key_encryptable::CryptoKey;
 use crate::error::{CryptoError, Result};
 
 /// An asymmetric encryption key. Used to encrypt and decrypt [`EncString`](crate::EncString)
 pub struct AsymmetricCryptoKey {
-    pub(crate) key: RsaPrivateKey,
+    pub(crate) key: Pin<Box<RsaPrivateKey>>,
 }
 
+// Note that RsaPrivateKey already implements ZeroizeOnDrop, so we don't need to do anything
+// We add this assertion to make sure that this is still true in the future
+const _: () = {
+    fn assert_zeroize_on_drop<T: zeroize::ZeroizeOnDrop>() {}
+    fn assert_all() {
+        assert_zeroize_on_drop::<RsaPrivateKey>();
+    }
+};
+
+impl zeroize::ZeroizeOnDrop for AsymmetricCryptoKey {}
+
 impl AsymmetricCryptoKey {
     pub fn from_pem(pem: &str) -> Result<Self> {
         use rsa::pkcs8::DecodePrivateKey;
         Ok(Self {
-            key: rsa::RsaPrivateKey::from_pkcs8_pem(pem).map_err(|_| CryptoError::InvalidKey)?,
+            key: Box::pin(RsaPrivateKey::from_pkcs8_pem(pem).map_err(|_| CryptoError::InvalidKey)?),
         })
     }
 
     pub fn from_der(der: &[u8]) -> Result<Self> {
         use rsa::pkcs8::DecodePrivateKey;
         Ok(Self {
-            key: rsa::RsaPrivateKey::from_pkcs8_der(der).map_err(|_| CryptoError::InvalidKey)?,
+            key: Box::pin(RsaPrivateKey::from_pkcs8_der(der).map_err(|_| CryptoError::InvalidKey)?),
         })
     }