Improve memory security around KDF

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

@@ -1,12 +1,13 @@
 use std::num::NonZeroU32;
 
-use base64::{engine::general_purpose::STANDARD, Engine};
+use base64::engine::general_purpose::STANDARD;
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 
 use super::utils::{derive_kdf_key, stretch_kdf_key};
 use crate::{
-    util, CryptoError, EncString, KeyDecryptable, Result, SensitiveVec, SymmetricCryptoKey, UserKey,
+    util, CryptoError, EncString, KeyDecryptable, Result, SensitiveString, SensitiveVec,
+    SymmetricCryptoKey, UserKey,
 };
 
 /// Key Derivation Function for Bitwarden Account
@@ -63,6 +64,7 @@ pub enum HashPurpose {
 /// Master Key.
 ///
 /// Derived from the users master password, used to protect the [UserKey].
+#[derive(Debug)]
 pub struct MasterKey(SymmetricCryptoKey);
 
 impl MasterKey {
@@ -72,18 +74,17 @@ impl MasterKey {
 
     /// Derives a users master key from their password, email and KDF.
     pub fn derive(password: &SensitiveVec, email: &[u8], kdf: &Kdf) -> Result<Self> {
-        derive_kdf_key(password.expose(), email, kdf).map(Self)
+        derive_kdf_key(password, email, kdf).map(Self)
     }
 
     /// Derive the master key hash, used for local and remote password validation.
     pub fn derive_master_key_hash(
         &self,
         password: &SensitiveVec,
         purpose: HashPurpose,
-    ) -> Result<String> {
+    ) -> Result<SensitiveString> {
         let hash = util::pbkdf2(&self.0.key, password.expose(), purpose as u32);
-
-        Ok(STANDARD.encode(hash))
+        Ok(hash.encode_base64(STANDARD))
     }
 
     /// Generate a new random user key and encrypt it with the master key.
@@ -192,7 +193,8 @@ mod tests {
             "ZF6HjxUTSyBHsC+HXSOhZoXN+UuMnygV5YkWXCY4VmM=",
             master_key
                 .derive_master_key_hash(&password, HashPurpose::ServerAuthorization)
-                .unwrap(),
+                .unwrap()
+                .expose(),
         );
     }
 
@@ -212,7 +214,8 @@ mod tests {
             "PR6UjYmjmppTYcdyTiNbAhPJuQQOmynKbdEl1oyi/iQ=",
             master_key
                 .derive_master_key_hash(&password, HashPurpose::ServerAuthorization)
-                .unwrap(),
+                .unwrap()
+                .expose(),
         );
     }
 

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

@@ -3,7 +3,7 @@ use crate::{
         key_encryptable::CryptoKey,
         utils::{derive_kdf_key, stretch_kdf_key},
     },
-    EncString, Kdf, KeyEncryptable, Result, SymmetricCryptoKey,
+    EncString, Kdf, KeyEncryptable, Result, SensitiveVec, SymmetricCryptoKey,
 };
 
 /// Pin Key.
@@ -17,7 +17,7 @@ impl PinKey {
     }
 
     /// Derives a users pin key from their password, email and KDF.
-    pub fn derive(password: &[u8], salt: &[u8], kdf: &Kdf) -> Result<Self> {
+    pub fn derive(password: &SensitiveVec, salt: &[u8], kdf: &Kdf) -> Result<Self> {
         derive_kdf_key(password, salt, kdf).map(Self)
     }
 }

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

@@ -7,7 +7,7 @@ use rand::Rng;
 use zeroize::Zeroize;
 
 use super::key_encryptable::CryptoKey;
-use crate::{CryptoError, SensitiveString, SensitiveVec};
+use crate::{CryptoError, Sensitive, SensitiveString, SensitiveVec};
 
 /// A symmetric encryption key. Used to encrypt and decrypt [`EncString`](crate::EncString)
 pub struct SymmetricCryptoKey {
@@ -82,6 +82,15 @@ impl TryFrom<SensitiveString> for SymmetricCryptoKey {
     }
 }
 
+impl<const N: usize> TryFrom<Sensitive<[u8; N]>> for SymmetricCryptoKey {
+    type Error = CryptoError;
+
+    fn try_from(mut value: Sensitive<[u8; N]>) -> Result<Self, Self::Error> {
+        let val = value.expose_mut();
+        SymmetricCryptoKey::try_from(val.as_mut_slice())
+    }
+}
+
 impl TryFrom<SensitiveVec> for SymmetricCryptoKey {
     type Error = CryptoError;
 
@@ -134,7 +143,7 @@ impl std::fmt::Debug for SymmetricCryptoKey {
 
 #[cfg(test)]
 pub fn derive_symmetric_key(name: &str) -> SymmetricCryptoKey {
-    use crate::{derive_shareable_key, generate_random_bytes, Sensitive};
+    use crate::{derive_shareable_key, generate_random_bytes};
 
     let secret: Sensitive<[u8; 16]> = generate_random_bytes();
     derive_shareable_key(secret, name, None)

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

@@ -3,12 +3,17 @@ use std::pin::Pin;
 use generic_array::{typenum::U32, GenericArray};
 use sha2::Digest;
 
-use crate::{util::hkdf_expand, Kdf, Result, SymmetricCryptoKey};
+use crate::{util::hkdf_expand, Kdf, Result, Sensitive, SensitiveVec, SymmetricCryptoKey};
 
 /// Derive a generic key from a secret and salt using the provided KDF.
-pub(super) fn derive_kdf_key(secret: &[u8], salt: &[u8], kdf: &Kdf) -> Result<SymmetricCryptoKey> {
-    let mut hash = match kdf {
-        Kdf::PBKDF2 { iterations } => crate::util::pbkdf2(secret, salt, iterations.get()),
+#[inline(always)]
+pub(super) fn derive_kdf_key(
+    secret: &SensitiveVec,
+    salt: &[u8],
+    kdf: &Kdf,
+) -> Result<SymmetricCryptoKey> {
+    let hash = match kdf {
+        Kdf::PBKDF2 { iterations } => crate::util::pbkdf2(secret.expose(), salt, iterations.get()),
 
         Kdf::Argon2id {
             iterations,
@@ -30,12 +35,22 @@ pub(super) fn derive_kdf_key(secret: &[u8], salt: &[u8], kdf: &Kdf) -> Result<Sy
 
             let salt_sha = sha2::Sha256::new().chain_update(salt).finalize();
 
-            let mut hash = [0u8; 32];
-            argon.hash_password_into(secret, &salt_sha, &mut hash)?;
+            let mut hash = Sensitive::new(Box::new([0u8; 32]));
+            let mut blocks = Sensitive::new(Box::new(vec![
+                argon2::Block::default();
+                argon.params().block_count()
+            ]));
+            argon.hash_password_into_with_memory(
+                secret.expose(),
+                &salt_sha,
+                hash.expose_mut(),
+                blocks.expose_mut(),
+            )?;
             hash
         }
     };
-    SymmetricCryptoKey::try_from(hash.as_mut_slice())
+
+    SymmetricCryptoKey::try_from(hash)
 }
 
 pub(super) fn stretch_kdf_key(k: &SymmetricCryptoKey) -> Result<SymmetricCryptoKey> {

crates/bitwarden-crypto/src/sensitive/sensitive.rs

@@ -37,18 +37,21 @@ impl<V: Zeroize> Sensitive<V> {
     /// Create a new [`Sensitive`] value. In an attempt to avoid accidentally placing this on the
     /// stack it only accepts a [`Box`] value. The rust compiler should be able to optimize away the
     /// initial stack allocation presuming the value is not used before being boxed.
+    #[inline(always)]
     pub fn new(value: Box<V>) -> Self {
         Self { value }
     }
 
     /// Expose the inner value. By exposing the inner value, you take responsibility for ensuring
     /// that any copy of the value is zeroized.
+    #[inline(always)]
     pub fn expose(&self) -> &V {
         &self.value
     }
 
     /// Expose the inner value mutable. By exposing the inner value, you take responsibility for
     /// ensuring that any copy of the value is zeroized.
+    #[inline(always)]
     pub fn expose_mut(&mut self) -> &mut V {
         &mut self.value
     }
@@ -95,18 +98,22 @@ impl SensitiveString {
     }
 }
 
-impl SensitiveVec {
-    pub fn encode_base64<T: base64::Engine>(self, engine: T) -> SensitiveString {
+impl<T: Zeroize + AsRef<[u8]>> Sensitive<T> {
+    pub fn encode_base64<E: base64::Engine>(self, engine: E) -> SensitiveString {
         use base64::engine::Config;
 
+        let inner: &[u8] = self.value.as_ref().as_ref();
+
         // Prevent accidental copies by allocating the full size
         let padding = engine.config().encode_padding();
-        let len = base64::encoded_len(self.value.len(), padding).expect("Valid length");
-        let mut value = SensitiveString::new(Box::new(String::with_capacity(len)));
+        let len = base64::encoded_len(inner.len(), padding).expect("Valid length");
 
-        engine.encode_string(self.value.as_ref(), &mut value.value);
+        let mut value = SensitiveVec::new(Box::new(vec![0u8; len]));
+        engine
+            .encode_slice(inner, &mut value.value[..len])
+            .expect("Valid base64 string length");
 
-        value
+        value.try_into().expect("Valid base64 string")
     }
 }
 

crates/bitwarden-crypto/src/util.rs

@@ -39,9 +39,18 @@ where
     Sensitive::new(Box::new(rand::thread_rng().gen::<T>()))
 }
 
-pub fn pbkdf2(password: &[u8], salt: &[u8], rounds: u32) -> [u8; PBKDF_SHA256_HMAC_OUT_SIZE] {
-    pbkdf2::pbkdf2_array::<PbkdfSha256Hmac, PBKDF_SHA256_HMAC_OUT_SIZE>(password, salt, rounds)
-        .expect("hash is a valid fixed size")
+#[inline(always)]
+pub fn pbkdf2(
+    password: &[u8],
+    salt: &[u8],
+    rounds: u32,
+) -> Sensitive<[u8; PBKDF_SHA256_HMAC_OUT_SIZE]> {
+    let mut hash = Sensitive::new(Box::new([0u8; PBKDF_SHA256_HMAC_OUT_SIZE]));
+
+    pbkdf2::pbkdf2::<PbkdfSha256Hmac>(password, salt, rounds, hash.expose_mut())
+        .expect("hash is a valid fixed size");
+
+    hash
 }
 
 #[cfg(test)]

crates/bitwarden-exporters/src/encrypted_json.rs

@@ -1,6 +1,6 @@
 use base64::engine::general_purpose::STANDARD;
 use bitwarden_crypto::{
-    generate_random_bytes, Kdf, KeyEncryptable, PinKey, Sensitive, SensitiveVec,
+    generate_random_bytes, Kdf, KeyEncryptable, PinKey, Sensitive, SensitiveString, SensitiveVec,
 };
 use serde::Serialize;
 use thiserror::Error;
@@ -26,7 +26,7 @@ pub enum EncryptedJsonError {
 pub(crate) fn export_encrypted_json(
     folders: Vec<Folder>,
     ciphers: Vec<Cipher>,
-    password: String,
+    password: SensitiveString,
     kdf: Kdf,
 ) -> Result<String, EncryptedJsonError> {
     let decrypted_export = export_json(folders, ciphers)?;
@@ -47,7 +47,7 @@ pub(crate) fn export_encrypted_json(
 
     let salt: Sensitive<[u8; 16]> = generate_random_bytes();
     let salt = SensitiveVec::from(salt).encode_base64(STANDARD);
-    let key = PinKey::derive(password.as_bytes(), salt.expose().as_bytes(), &kdf)?;
+    let key = PinKey::derive(&password.into(), salt.expose().as_bytes(), &kdf)?;
 
     let enc_key_validation = Uuid::new_v4().to_string();
 
@@ -238,7 +238,7 @@ mod tests {
                     deleted_date: None,
                 },
             ],
-            "password".to_string(),
+            SensitiveString::test("password"),
             Kdf::PBKDF2 {
                 iterations: NonZeroU32::new(600_000).unwrap(),
             },

crates/bitwarden-exporters/src/lib.rs

@@ -1,4 +1,4 @@
-use bitwarden_crypto::Kdf;
+use bitwarden_crypto::{Kdf, SensitiveString};
 use chrono::{DateTime, Utc};
 use thiserror::Error;
 use uuid::Uuid;
@@ -13,7 +13,7 @@ use encrypted_json::export_encrypted_json;
 pub enum Format {
     Csv,
     Json,
-    EncryptedJson { password: String, kdf: Kdf },
+    EncryptedJson { password: SensitiveString, kdf: Kdf },
 }
 
 /// Export representation of a Bitwarden folder.

crates/bitwarden-uniffi/src/auth/mod.rs

@@ -54,7 +54,7 @@ impl ClientAuth {
         password: SensitiveString,
         kdf_params: Kdf,
         purpose: HashPurpose,
-    ) -> Result<String> {
+    ) -> Result<SensitiveString> {
         Ok(self
             .0
              .0
@@ -103,15 +103,15 @@ impl ClientAuth {
     pub async fn validate_password(
         &self,
         password: SensitiveString,
-        password_hash: String,
+        password_hash: SensitiveString,
     ) -> Result<bool> {
         Ok(self
             .0
              .0
             .write()
             .await
             .auth()
-            .validate_password(password, password_hash.to_string())?)
+            .validate_password(password, password_hash)?)
     }
 
     /// Validate the user password without knowing the password hash
@@ -124,7 +124,7 @@ impl ClientAuth {
         &self,
         password: SensitiveString,
         encrypted_user_key: String,
-    ) -> Result<String> {
+    ) -> Result<SensitiveString> {
         Ok(self
             .0
              .0

crates/bitwarden/src/auth/client_auth.rs

@@ -115,7 +115,7 @@ impl<'a> ClientAuth<'a> {
     pub fn validate_password(
         &self,
         password: SensitiveString,
-        password_hash: String,
+        password_hash: SensitiveString,
     ) -> Result<bool> {
         validate_password(self.client, password, password_hash)
     }
@@ -124,7 +124,7 @@ impl<'a> ClientAuth<'a> {
         &self,
         password: SensitiveString,
         encrypted_user_key: String,
-    ) -> Result<String> {
+    ) -> Result<SensitiveString> {
         validate_password_user_key(self.client, password, encrypted_user_key)
     }
 

crates/bitwarden/src/auth/login/password.rs

@@ -36,8 +36,13 @@ pub(crate) async fn login_password(
     let password_hash =
         master_key.derive_master_key_hash(&password_vec, HashPurpose::ServerAuthorization)?;
 
-    let response =
-        request_identity_tokens(client, &input.email, &input.two_factor, &password_hash).await?;
+    let response = request_identity_tokens(
+        client,
+        &input.email,
+        &input.two_factor,
+        password_hash.expose(),
+    )
+    .await?;
 
     if let IdentityTokenResponse::Authenticated(r) = &response {
         client.set_tokens(

crates/bitwarden/src/auth/login/two_factor.rs

@@ -33,7 +33,7 @@ pub(crate) async fn send_two_factor_email(
     bitwarden_api_api::apis::two_factor_api::two_factor_send_email_login_post(
         &config.api,
         Some(TwoFactorEmailRequestModel {
-            master_password_hash: Some(password_hash),
+            master_password_hash: Some(password_hash.expose().clone()),
             otp: None,
             auth_request_access_code: None,
             secret: None,

crates/bitwarden/src/auth/mod.rs

@@ -11,7 +11,7 @@ pub use jwt_token::JWTToken;
 #[cfg(feature = "internal")]
 mod register;
 #[cfg(feature = "internal")]
-use bitwarden_crypto::{HashPurpose, MasterKey, SensitiveVec};
+use bitwarden_crypto::{HashPurpose, MasterKey, SensitiveString, SensitiveVec};
 #[cfg(feature = "internal")]
 pub use register::{RegisterKeyResponse, RegisterRequest};
 #[cfg(feature = "internal")]
@@ -34,7 +34,7 @@ fn determine_password_hash(
     kdf: &Kdf,
     password: &SensitiveVec,
     purpose: HashPurpose,
-) -> Result<String> {
+) -> Result<SensitiveString> {
     let master_key = MasterKey::derive(password, email.as_bytes(), kdf)?;
     Ok(master_key.derive_master_key_hash(password, purpose)?)
 }
@@ -59,6 +59,9 @@ mod tests {
 
         let result = determine_password_hash(email, &kdf, &password, purpose).unwrap();
 
-        assert_eq!(result, "7kTqkF1pY/3JeOu73N9kR99fDDe9O1JOZaVc7KH3lsU=");
+        assert_eq!(
+            result.expose(),
+            "7kTqkF1pY/3JeOu73N9kR99fDDe9O1JOZaVc7KH3lsU="
+        );
     }
 }