diff --git a/crates/bitwarden-crypto/src/keys/master_key.rs b/crates/bitwarden-crypto/src/keys/master_key.rs index 5a24125af..490297a75 100644 --- a/crates/bitwarden-crypto/src/keys/master_key.rs +++ b/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,7 +74,7 @@ impl MasterKey { /// Derives a users master key from their password, email and KDF. pub fn derive(password: &SensitiveVec, email: &[u8], kdf: &Kdf) -> Result { - 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. @@ -80,10 +82,9 @@ impl MasterKey { &self, password: &SensitiveVec, purpose: HashPurpose, - ) -> Result { + ) -> Result { 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(), ); } diff --git a/crates/bitwarden-crypto/src/keys/pin_key.rs b/crates/bitwarden-crypto/src/keys/pin_key.rs index 475b7ffd9..261743a58 100644 --- a/crates/bitwarden-crypto/src/keys/pin_key.rs +++ b/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 { + pub fn derive(password: &SensitiveVec, salt: &[u8], kdf: &Kdf) -> Result { derive_kdf_key(password, salt, kdf).map(Self) } } diff --git a/crates/bitwarden-crypto/src/keys/symmetric_crypto_key.rs b/crates/bitwarden-crypto/src/keys/symmetric_crypto_key.rs index b58468c7f..c5cbbd1d0 100644 --- a/crates/bitwarden-crypto/src/keys/symmetric_crypto_key.rs +++ b/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 for SymmetricCryptoKey { } } +impl TryFrom> for SymmetricCryptoKey { + type Error = CryptoError; + + fn try_from(mut value: Sensitive<[u8; N]>) -> Result { + let val = value.expose_mut(); + SymmetricCryptoKey::try_from(val.as_mut_slice()) + } +} + impl TryFrom 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) diff --git a/crates/bitwarden-crypto/src/keys/utils.rs b/crates/bitwarden-crypto/src/keys/utils.rs index a2df336e1..cfe528f99 100644 --- a/crates/bitwarden-crypto/src/keys/utils.rs +++ b/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 { - 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 { + 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 Result { diff --git a/crates/bitwarden-crypto/src/sensitive/sensitive.rs b/crates/bitwarden-crypto/src/sensitive/sensitive.rs index 33e6fddc1..84d4ba5ca 100644 --- a/crates/bitwarden-crypto/src/sensitive/sensitive.rs +++ b/crates/bitwarden-crypto/src/sensitive/sensitive.rs @@ -37,18 +37,21 @@ impl Sensitive { /// 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) -> 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(self, engine: T) -> SensitiveString { +impl> Sensitive { + pub fn encode_base64(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") } } diff --git a/crates/bitwarden-crypto/src/util.rs b/crates/bitwarden-crypto/src/util.rs index b38cb0e00..166294612 100644 --- a/crates/bitwarden-crypto/src/util.rs +++ b/crates/bitwarden-crypto/src/util.rs @@ -39,9 +39,18 @@ where Sensitive::new(Box::new(rand::thread_rng().gen::())) } -pub fn pbkdf2(password: &[u8], salt: &[u8], rounds: u32) -> [u8; PBKDF_SHA256_HMAC_OUT_SIZE] { - pbkdf2::pbkdf2_array::(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::(password, salt, rounds, hash.expose_mut()) + .expect("hash is a valid fixed size"); + + hash } #[cfg(test)] diff --git a/crates/bitwarden-exporters/src/encrypted_json.rs b/crates/bitwarden-exporters/src/encrypted_json.rs index 6dabc90d5..c1e982871 100644 --- a/crates/bitwarden-exporters/src/encrypted_json.rs +++ b/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, ciphers: Vec, - password: String, + password: SensitiveString, kdf: Kdf, ) -> Result { 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(), }, diff --git a/crates/bitwarden-exporters/src/lib.rs b/crates/bitwarden-exporters/src/lib.rs index f17d31a2d..9fbbfd9d1 100644 --- a/crates/bitwarden-exporters/src/lib.rs +++ b/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. diff --git a/crates/bitwarden-uniffi/src/auth/mod.rs b/crates/bitwarden-uniffi/src/auth/mod.rs index 5c54a6cdb..b5fef1ef8 100644 --- a/crates/bitwarden-uniffi/src/auth/mod.rs +++ b/crates/bitwarden-uniffi/src/auth/mod.rs @@ -54,7 +54,7 @@ impl ClientAuth { password: SensitiveString, kdf_params: Kdf, purpose: HashPurpose, - ) -> Result { + ) -> Result { Ok(self .0 .0 @@ -103,7 +103,7 @@ impl ClientAuth { pub async fn validate_password( &self, password: SensitiveString, - password_hash: String, + password_hash: SensitiveString, ) -> Result { Ok(self .0 @@ -111,7 +111,7 @@ impl ClientAuth { .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 { + ) -> Result { Ok(self .0 .0 diff --git a/crates/bitwarden/src/auth/client_auth.rs b/crates/bitwarden/src/auth/client_auth.rs index 487877b33..b3aac3641 100644 --- a/crates/bitwarden/src/auth/client_auth.rs +++ b/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 { validate_password(self.client, password, password_hash) } @@ -124,7 +124,7 @@ impl<'a> ClientAuth<'a> { &self, password: SensitiveString, encrypted_user_key: String, - ) -> Result { + ) -> Result { validate_password_user_key(self.client, password, encrypted_user_key) } diff --git a/crates/bitwarden/src/auth/login/password.rs b/crates/bitwarden/src/auth/login/password.rs index cae51d6eb..db392025f 100644 --- a/crates/bitwarden/src/auth/login/password.rs +++ b/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( diff --git a/crates/bitwarden/src/auth/login/two_factor.rs b/crates/bitwarden/src/auth/login/two_factor.rs index 9b432392f..d0533cd39 100644 --- a/crates/bitwarden/src/auth/login/two_factor.rs +++ b/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, diff --git a/crates/bitwarden/src/auth/mod.rs b/crates/bitwarden/src/auth/mod.rs index 1c6395299..34a14aaf1 100644 --- a/crates/bitwarden/src/auth/mod.rs +++ b/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 { +) -> Result { 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=" + ); } } diff --git a/crates/bitwarden/src/auth/password/validate.rs b/crates/bitwarden/src/auth/password/validate.rs index 8758251d3..b28e5aa63 100644 --- a/crates/bitwarden/src/auth/password/validate.rs +++ b/crates/bitwarden/src/auth/password/validate.rs @@ -11,7 +11,7 @@ use crate::{ pub(crate) fn validate_password( client: &Client, password: SensitiveString, - password_hash: String, + password_hash: SensitiveString, ) -> Result { let login_method = client .login_method @@ -29,7 +29,7 @@ pub(crate) fn validate_password( HashPurpose::LocalAuthorization, )?; - Ok(hash == password_hash) + Ok(hash.expose() == password_hash.expose()) } } } else { @@ -40,9 +40,9 @@ pub(crate) fn validate_password( #[cfg(feature = "internal")] pub(crate) fn validate_password_user_key( client: &Client, - password: bitwarden_crypto::SensitiveString, + password: SensitiveString, encrypted_user_key: String, -) -> Result { +) -> Result { let login_method = client .login_method .as_ref() @@ -99,7 +99,7 @@ mod tests { })); let password = SensitiveString::test("password123"); - let password_hash = "7kTqkF1pY/3JeOu73N9kR99fDDe9O1JOZaVc7KH3lsU=".to_string(); + let password_hash = SensitiveString::test("7kTqkF1pY/3JeOu73N9kR99fDDe9O1JOZaVc7KH3lsU="); let result = validate_password(&client, password, password_hash); @@ -146,10 +146,11 @@ mod tests { ) .unwrap(); - assert_eq!(result, "aOvkBXFhSdgrBWR3hZCMRoML9+h5yRblU3lFphCdkeA="); - assert!( - validate_password(&client, password.try_into().unwrap(), result.to_string()).unwrap() - ) + assert_eq!( + result.expose(), + "aOvkBXFhSdgrBWR3hZCMRoML9+h5yRblU3lFphCdkeA=" + ); + assert!(validate_password(&client, password.try_into().unwrap(), result).unwrap()) } #[cfg(feature = "internal")] diff --git a/crates/bitwarden/src/auth/register.rs b/crates/bitwarden/src/auth/register.rs index 09e60ac98..50e4e41cb 100644 --- a/crates/bitwarden/src/auth/register.rs +++ b/crates/bitwarden/src/auth/register.rs @@ -32,7 +32,7 @@ pub(super) async fn register(client: &mut Client, req: RegisterRequest) -> Resul Some(RegisterRequestModel { name: req.name, email: req.email, - master_password_hash: keys.master_password_hash, + master_password_hash: keys.master_password_hash.expose().clone(), master_password_hint: req.password_hint, captcha_response: None, // TODO: Add key: Some(keys.encrypted_user_key), @@ -75,7 +75,7 @@ pub(super) fn make_register_keys( #[cfg_attr(feature = "mobile", derive(uniffi::Record))] pub struct RegisterKeyResponse { - pub master_password_hash: String, + pub master_password_hash: SensitiveString, pub encrypted_user_key: String, pub keys: RsaKeyPair, } diff --git a/crates/bitwarden/src/mobile/client_kdf.rs b/crates/bitwarden/src/mobile/client_kdf.rs index 8597f6f8e..2a5c792bd 100644 --- a/crates/bitwarden/src/mobile/client_kdf.rs +++ b/crates/bitwarden/src/mobile/client_kdf.rs @@ -13,7 +13,7 @@ impl<'a> ClientKdf<'a> { password: SensitiveString, kdf_params: Kdf, purpose: HashPurpose, - ) -> Result { + ) -> Result { hash_password(self.client, email, password, kdf_params, purpose).await } } diff --git a/crates/bitwarden/src/mobile/crypto.rs b/crates/bitwarden/src/mobile/crypto.rs index 4027302df..00d0e56b7 100644 --- a/crates/bitwarden/src/mobile/crypto.rs +++ b/crates/bitwarden/src/mobile/crypto.rs @@ -190,7 +190,7 @@ pub async fn get_user_encryption_key(client: &mut Client) -> Result Result { +) -> Result { let password_vec = password.into(); let master_key = MasterKey::derive(&password_vec, email.as_bytes(), &kdf_params)?; diff --git a/crates/bitwarden/src/platform/get_user_api_key.rs b/crates/bitwarden/src/platform/get_user_api_key.rs index 0bb99dd17..14e961da5 100644 --- a/crates/bitwarden/src/platform/get_user_api_key.rs +++ b/crates/bitwarden/src/platform/get_user_api_key.rs @@ -55,7 +55,7 @@ fn get_secret_verification_request( }) .transpose()?; Ok(SecretVerificationRequestModel { - master_password_hash, + master_password_hash: master_password_hash.map(|h| h.expose().clone()), otp: input.otp.as_ref().cloned(), secret: None, auth_request_access_code: None, diff --git a/crates/bitwarden/src/tool/exporters/mod.rs b/crates/bitwarden/src/tool/exporters/mod.rs index 2da6ac833..b2a81ad6b 100644 --- a/crates/bitwarden/src/tool/exporters/mod.rs +++ b/crates/bitwarden/src/tool/exporters/mod.rs @@ -1,4 +1,4 @@ -use bitwarden_crypto::Decryptable; +use bitwarden_crypto::{Decryptable, SensitiveString}; use bitwarden_exporters::export; use schemars::JsonSchema; @@ -20,7 +20,7 @@ pub use client_exporter::ClientExporters; pub enum ExportFormat { Csv, Json, - EncryptedJson { password: String }, + EncryptedJson { password: SensitiveString }, } pub(super) fn export_vault( @@ -321,7 +321,7 @@ mod tests { convert_format( &client, ExportFormat::EncryptedJson { - password: "password".to_string() + password: SensitiveString::test("password") } ) .unwrap(), diff --git a/crates/bitwarden/src/vault/send.rs b/crates/bitwarden/src/vault/send.rs index e681e5468..387b51a21 100644 --- a/crates/bitwarden/src/vault/send.rs +++ b/crates/bitwarden/src/vault/send.rs @@ -5,7 +5,7 @@ use base64::{ use bitwarden_api_api::models::{SendFileModel, SendResponseModel, SendTextModel}; use bitwarden_crypto::{ derive_shareable_key, generate_random_bytes, CryptoError, EncString, KeyDecryptable, - KeyEncryptable, LocateKey, Sensitive, SensitiveVec, SymmetricCryptoKey, + KeyEncryptable, LocateKey, Sensitive, SensitiveString, SensitiveVec, SymmetricCryptoKey, }; use chrono::{DateTime, Utc}; use schemars::JsonSchema; @@ -73,7 +73,7 @@ pub struct Send { pub name: EncString, pub notes: Option, pub key: EncString, - pub password: Option, + pub password: Option, pub r#type: SendType, pub file: Option, @@ -284,7 +284,7 @@ impl KeyEncryptable for SendView { key: k.encrypt_with_key(key)?, password: self.new_password.map(|password| { let password = bitwarden_crypto::pbkdf2(password.as_bytes(), &k, SEND_ITERATIONS); - STANDARD.encode(password) + password.encode_base64(STANDARD) }), r#type: self.r#type, @@ -313,7 +313,7 @@ impl TryFrom for Send { name: require!(send.name).parse()?, notes: EncString::try_from_optional(send.notes)?, key: require!(send.key).parse()?, - password: send.password, + password: send.password.map(|p| SensitiveString::new(Box::new(p))), r#type: require!(send.r#type).into(), file: send.file.map(|f| (*f).try_into()).transpose()?, text: send.text.map(|t| (*t).try_into()).transpose()?, @@ -583,8 +583,8 @@ mod tests { let send: Send = view.encrypt_with_key(key).unwrap(); assert_eq!( - send.password, - Some("vTIDfdj3FTDbejmMf+mJWpYdMXsxfeSd1Sma3sjCtiQ=".to_owned()) + send.password.as_ref().map(|p| p.expose().as_str()), + Some("vTIDfdj3FTDbejmMf+mJWpYdMXsxfeSd1Sma3sjCtiQ=") ); let v: SendView = send.decrypt_with_key(key).unwrap(); diff --git a/crates/memory-testing/Dockerfile b/crates/memory-testing/Dockerfile index fdcf5de00..601800355 100644 --- a/crates/memory-testing/Dockerfile +++ b/crates/memory-testing/Dockerfile @@ -3,11 +3,26 @@ ############################################### FROM rust:1.76 AS build -# Copy required project files -COPY . /app - -# Build project WORKDIR /app + +# Copy dependency files and create dummy files to allow cargo to build the dependencies in a separate stage +COPY Cargo.toml Cargo.lock /app/ +COPY crates/bitwarden-crypto/Cargo.toml /app/crates/bitwarden-crypto/ +COPY crates/memory-testing/Cargo.toml /app/crates/memory-testing/ + +RUN mkdir -p /app/crates/bitwarden-crypto/src \ + && mkdir -p /app/crates/memory-testing/src \ + && touch /app/crates/bitwarden-crypto/src/lib.rs \ + && echo 'fn main(){}' > /app/crates/memory-testing/src/main.rs \ + && cargo build -p memory-testing + +# Delete dummy files and copy the actual source code +RUN rm /app/crates/bitwarden-crypto/src/lib.rs /app/crates/memory-testing/src/main.rs +COPY crates/bitwarden-crypto/src /app/crates/bitwarden-crypto/src +COPY crates/memory-testing/src /app/crates/memory-testing/src + +# Build the project. We use touch to force a rebuild of the now real files +RUN touch /app/crates/bitwarden-crypto/src/lib.rs /app/crates/memory-testing/src/main.rs RUN cargo build -p memory-testing ############################################### @@ -20,7 +35,8 @@ USER root RUN apt-get update && apt-get install -y --no-install-recommends gdb=13.1-3 && apt-get clean && rm -rf /var/lib/apt/lists/* -# Copy built project from the build stage -COPY --from=build /app/target/debug/memory-testing /app/target/debug/capture-dumps /app/crates/memory-testing/cases.json ./ +# Copy built project from the build stage and the cases.json file +COPY --from=build /app/target/debug/memory-testing /app/target/debug/capture-dumps ./ +COPY crates/memory-testing/cases.json . CMD [ "/capture-dumps", "./memory-testing", "/output" ] diff --git a/crates/memory-testing/cases.json b/crates/memory-testing/cases.json index eb85e2aca..5923e350a 100644 --- a/crates/memory-testing/cases.json +++ b/crates/memory-testing/cases.json @@ -5,5 +5,37 @@ "decrypted_key_hex": "15f85554ff1f9852196355a646cccf9919950b15cd5957097df3eb6e4cb04cfb", "decrypted_mac_hex": "4136481f858193f83f6c5468b3617acf7dfba3db2a325aa33017d885e5a31085" } + ], + "master_key": [ + { + "password": "123412341234", + "email": "test@mail.com", + + "kdf": { + "pBKDF2": { + "iterations": 100000 + } + }, + + "key_hex": "2d55ac8e33bd14ee9eee26fa651163a41049a37b3b20c914a8b6abc9a38a89d5", + "hash": "gQ9O5ZhtQ23dL5r93e4BL04ATYOJVEvBAOwYsDDEJFA=", + "hash_hex": "810f4ee5986d436ddd2f9afdddee012f4e004d8389544bc100ec18b030c42450" + }, + { + "password": "asdfasdfasdf", + "email": "test@mail.com", + + "kdf": { + "argon2id": { + "iterations": 3, + "memory": 4, + "parallelism": 1 + } + }, + + "key_hex": "3bc0520a0abff0097d521ce0ee5e5b1cee301939a84742623c0c1697d7a4bd46", + "hash": "lHkprdORlICVJ4Umwi94Uz/nATK6Y7If7e+iFoabzh0=", + "hash_hex": "947929add391948095278526c22f78533fe70132ba63b21fedefa216869bce1d" + } ] } diff --git a/crates/memory-testing/run_test.sh b/crates/memory-testing/run_test.sh index 627e5dacd..750bbc8ed 100755 --- a/crates/memory-testing/run_test.sh +++ b/crates/memory-testing/run_test.sh @@ -1,3 +1,5 @@ +set -eo pipefail + # Move to the root of the repository cd "$(dirname "$0")" cd ../../ @@ -5,7 +7,7 @@ cd ../../ BASE_DIR="./crates/memory-testing" mkdir -p $BASE_DIR/output -rm $BASE_DIR/output/* +rm $BASE_DIR/output/* || true cargo build -p memory-testing diff --git a/crates/memory-testing/src/bin/analyze-dumps.rs b/crates/memory-testing/src/bin/analyze-dumps.rs index 9957bc1cd..97a71b29b 100644 --- a/crates/memory-testing/src/bin/analyze-dumps.rs +++ b/crates/memory-testing/src/bin/analyze-dumps.rs @@ -1,5 +1,6 @@ use std::{env, fmt::Display, io, path::Path, process}; +use bitwarden_crypto::Kdf; use memory_testing::*; fn find_subarrays(needle: &[u8], haystack: &[u8]) -> Vec { @@ -124,6 +125,63 @@ fn main() -> io::Result<()> { ); } + for (idx, case) in cases.master_key.iter().enumerate() { + let password = case.password.as_bytes().to_vec(); + let key = hex::decode(&case.key_hex).unwrap(); + let hash_b64 = case.hash.as_bytes().to_vec(); + let hash = hex::decode(&case.hash_hex).unwrap(); + + let pass_initial_pos = find_subarrays(&password, &initial_core); + let key_initial_pos = find_subarrays(&key, &initial_core); + let hashb64_initial_pos = find_subarrays(&hash_b64, &initial_core); + let hash_initial_pos = find_subarrays(&hash, &initial_core); + + let pass_final_pos = find_subarrays(&password, &final_core); + let key_final_pos = find_subarrays(&key, &final_core); + let hashb64_final_pos = find_subarrays(&hash_b64, &final_core); + let hash_final_pos = find_subarrays(&hash, &final_core); + + error |= add_row( + &mut table, + format!("Master Key password, case {}", idx), + &pass_initial_pos, + &pass_final_pos, + pass_final_pos.is_empty(), + ); + + // At the moment the argon library is producing some hard to pinpoint leaks + // Upgrading to the latest pre-release version solves at least one of them + let allowed_leaks = if matches!(case.kdf, Kdf::Argon2id { .. }) { + 2 + } else { + 0 + }; + + error |= add_row( + &mut table, + format!("Master Key, case {}", idx), + &key_initial_pos, + &key_final_pos, + key_final_pos.len() <= allowed_leaks, + ); + + error |= add_row( + &mut table, + format!("Master Key Hash B64, case {}", idx), + &hashb64_initial_pos, + &hashb64_final_pos, + hashb64_final_pos.is_empty(), + ); + + error |= add_row( + &mut table, + format!("Master Key Hash, case {}", idx), + &hash_initial_pos, + &hash_final_pos, + hash_final_pos.is_empty(), + ); + } + println!("{table}"); process::exit(if error { 1 } else { 0 }); diff --git a/crates/memory-testing/src/bin/capture-dumps.rs b/crates/memory-testing/src/bin/capture-dumps.rs index f43905867..caa8b8dfd 100644 --- a/crates/memory-testing/src/bin/capture-dumps.rs +++ b/crates/memory-testing/src/bin/capture-dumps.rs @@ -41,7 +41,7 @@ fn main() -> io::Result<()> { let stdin = proc.stdin.as_mut().expect("Valid stdin"); // Wait a bit for it to process - sleep(Duration::from_secs(3)); + sleep(Duration::from_millis(1500)); // Dump the process before the variables are freed let initial_core = @@ -52,7 +52,7 @@ fn main() -> io::Result<()> { stdin.flush()?; // Wait a bit for it to process - sleep(Duration::from_secs(1)); + sleep(Duration::from_millis(500)); // Dump the process after the variables are freed let final_core = diff --git a/crates/memory-testing/src/lib.rs b/crates/memory-testing/src/lib.rs index e633756d1..f87966d3b 100644 --- a/crates/memory-testing/src/lib.rs +++ b/crates/memory-testing/src/lib.rs @@ -1,5 +1,6 @@ use std::path::Path; +use bitwarden_crypto::Kdf; use zeroize::Zeroize; pub const TEST_STRING: &str = "THIS IS USED TO CHECK THAT THE MEMORY IS DUMPED CORRECTLY"; @@ -18,6 +19,8 @@ pub fn load_cases(base_dir: &Path) -> Cases { #[derive(serde::Deserialize)] pub struct Cases { pub symmetric_key: Vec, + + pub master_key: Vec, } #[derive(serde::Deserialize)] @@ -27,3 +30,15 @@ pub struct SymmetricKeyCases { pub decrypted_key_hex: String, pub decrypted_mac_hex: String, } + +#[derive(serde::Deserialize)] +pub struct MasterKeyCases { + pub password: String, + + pub email: String, + pub kdf: Kdf, + + pub key_hex: String, + pub hash: String, + pub hash_hex: String, +} diff --git a/crates/memory-testing/src/main.rs b/crates/memory-testing/src/main.rs index ee781683c..c50efb6b1 100644 --- a/crates/memory-testing/src/main.rs +++ b/crates/memory-testing/src/main.rs @@ -1,6 +1,6 @@ use std::{env, io::Read, path::Path, process}; -use bitwarden_crypto::{SensitiveString, SymmetricCryptoKey}; +use bitwarden_crypto::{MasterKey, SensitiveString, SensitiveVec, SymmetricCryptoKey}; fn wait_for_dump() { println!("Waiting for dump..."); @@ -20,23 +20,37 @@ fn main() { let cases = memory_testing::load_cases(base_dir); let mut symmetric_keys = Vec::new(); - let mut symmetric_keys_as_vecs = Vec::new(); for case in cases.symmetric_key { let key = SensitiveString::new(Box::new(case.key)); let key = SymmetricCryptoKey::try_from(key).unwrap(); - symmetric_keys_as_vecs.push(key.to_vec()); - symmetric_keys.push(key); + symmetric_keys.push((key.to_vec(), key)); + } + + let mut master_keys = Vec::new(); + + for case in cases.master_key { + let password: SensitiveVec = SensitiveString::new(Box::new(case.password)).into(); + + let key = MasterKey::derive(&password, case.email.as_bytes(), &case.kdf).unwrap(); + let hash = key + .derive_master_key_hash( + &password, + bitwarden_crypto::HashPurpose::ServerAuthorization, + ) + .unwrap(); + + master_keys.push((key, hash)); } // Make a memory dump before the variables are freed wait_for_dump(); // Use all the variables so the compiler doesn't decide to remove them - println!("{test_string} {symmetric_keys:?} {symmetric_keys_as_vecs:?}"); + println!("{test_string} {symmetric_keys:?} {master_keys:?} "); drop(symmetric_keys); - drop(symmetric_keys_as_vecs); + drop(master_keys); // After the variables are dropped, we want to make another dump wait_for_dump();