From b424a929d5e8242b3848d171f6789e992eda54ef Mon Sep 17 00:00:00 2001 From: Hinton Date: Thu, 2 May 2024 18:55:32 +0200 Subject: [PATCH] Replace SensitiveString with the new implementation --- .../src/enc_string/symmetric.rs | 9 +- .../src/keys/asymmetric_crypto_key.rs | 2 +- .../bitwarden-crypto/src/sensitive/base64.rs | 36 ++++++ .../src/sensitive/decrypted.rs | 4 +- crates/bitwarden-crypto/src/sensitive/mod.rs | 5 +- .../src/sensitive/sensitive.rs | 91 +------------ .../bitwarden-crypto/src/sensitive/string.rs | 121 ++++++++++++++++-- crates/bitwarden-crypto/src/uniffi_support.rs | 4 +- crates/bitwarden-exporters/src/csv.rs | 12 +- .../bitwarden-exporters/src/encrypted_json.rs | 4 +- crates/bitwarden/src/auth/auth_request.rs | 9 +- crates/bitwarden/src/auth/login/password.rs | 2 +- crates/bitwarden/src/auth/login/two_factor.rs | 2 +- .../bitwarden/src/auth/password/strength.rs | 2 +- crates/bitwarden/src/auth/register.rs | 2 +- .../src/platform/get_user_api_key.rs | 2 +- .../projects/project_response.rs | 2 +- .../src/secrets_manager/secrets/list.rs | 2 +- .../secrets/secret_response.rs | 6 +- crates/bitwarden/src/secrets_manager/state.rs | 2 +- crates/bitwarden/src/vault/cipher/cipher.rs | 63 +++++---- crates/bitwarden/src/vault/cipher/login.rs | 6 +- crates/bitwarden/src/vault/send.rs | 2 +- crates/bw/src/auth/login.rs | 8 +- crates/bw/src/main.rs | 2 +- crates/memory-testing/src/main.rs | 10 +- 26 files changed, 225 insertions(+), 185 deletions(-) create mode 100644 crates/bitwarden-crypto/src/sensitive/base64.rs diff --git a/crates/bitwarden-crypto/src/enc_string/symmetric.rs b/crates/bitwarden-crypto/src/enc_string/symmetric.rs index 1a0c9f359..505bd8b03 100644 --- a/crates/bitwarden-crypto/src/enc_string/symmetric.rs +++ b/crates/bitwarden-crypto/src/enc_string/symmetric.rs @@ -8,7 +8,8 @@ use serde::Deserialize; use super::{check_length, from_b64, from_b64_vec, split_enc_string}; use crate::{ error::{CryptoError, EncStringParseError, Result}, - DecryptedString, DecryptedVec, KeyDecryptable, KeyEncryptable, LocateKey, SymmetricCryptoKey, + DecryptedString, DecryptedVec, KeyDecryptable, KeyEncryptable, LocateKey, SensitiveString, + SymmetricCryptoKey, }; /// # Encrypted string primitive @@ -281,6 +282,12 @@ impl KeyEncryptable for String { } } +impl KeyEncryptable for SensitiveString { + fn encrypt_with_key(self, key: &SymmetricCryptoKey) -> crate::Result { + self.as_str().as_bytes().encrypt_with_key(key) + } +} + impl KeyDecryptable for EncString { fn decrypt_with_key(&self, key: &SymmetricCryptoKey) -> Result { let dec: DecryptedVec = self.decrypt_with_key(key)?; diff --git a/crates/bitwarden-crypto/src/keys/asymmetric_crypto_key.rs b/crates/bitwarden-crypto/src/keys/asymmetric_crypto_key.rs index 13230659b..532914dfc 100644 --- a/crates/bitwarden-crypto/src/keys/asymmetric_crypto_key.rs +++ b/crates/bitwarden-crypto/src/keys/asymmetric_crypto_key.rs @@ -72,7 +72,7 @@ impl AsymmetricCryptoKey { use rsa::pkcs8::DecodePrivateKey; Ok(Self { key: Box::pin( - RsaPrivateKey::from_pkcs8_pem(pem.expose()).map_err(|_| CryptoError::InvalidKey)?, + RsaPrivateKey::from_pkcs8_pem(pem.as_str()).map_err(|_| CryptoError::InvalidKey)?, ), }) } diff --git a/crates/bitwarden-crypto/src/sensitive/base64.rs b/crates/bitwarden-crypto/src/sensitive/base64.rs new file mode 100644 index 000000000..2f93586e6 --- /dev/null +++ b/crates/bitwarden-crypto/src/sensitive/base64.rs @@ -0,0 +1,36 @@ +use zeroize::Zeroize; + +use crate::{CryptoError, Sensitive, SensitiveString, SensitiveVec}; + +impl SensitiveString { + pub fn decode_base64(self, engine: T) -> Result { + // Prevent accidental copies by allocating the full size + let len = base64::decoded_len_estimate(self.len()); + let mut value = SensitiveVec::new(Box::new(Vec::with_capacity(len))); + + engine + .decode_vec(self.as_str(), &mut value.value) + .map_err(|_| CryptoError::InvalidKey)?; + + Ok(value) + } +} + +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(inner.len(), padding).expect("Valid length"); + + let mut value = SensitiveVec::new(Box::new(vec![0u8; len])); + engine + .encode_slice(inner, &mut value.value[..len]) + .expect("Valid base64 string length"); + + value.try_into().expect("Valid base64 string") + } +} diff --git a/crates/bitwarden-crypto/src/sensitive/decrypted.rs b/crates/bitwarden-crypto/src/sensitive/decrypted.rs index 02096d081..f196c5b4c 100644 --- a/crates/bitwarden-crypto/src/sensitive/decrypted.rs +++ b/crates/bitwarden-crypto/src/sensitive/decrypted.rs @@ -1,11 +1,11 @@ use zeroize::Zeroize; -use crate::{CryptoError, CryptoKey, KeyEncryptable, Sensitive}; +use crate::{CryptoError, CryptoKey, KeyEncryptable, Sensitive, SensitiveString}; /// Type alias for a [`Sensitive`] value to denote decrypted data. pub type Decrypted = Sensitive; pub type DecryptedVec = Decrypted>; -pub type DecryptedString = Decrypted; +pub type DecryptedString = SensitiveString; impl + Zeroize + Clone, Key: CryptoKey, Output> KeyEncryptable for Decrypted diff --git a/crates/bitwarden-crypto/src/sensitive/mod.rs b/crates/bitwarden-crypto/src/sensitive/mod.rs index 6a04a8ab0..1657ba2b5 100644 --- a/crates/bitwarden-crypto/src/sensitive/mod.rs +++ b/crates/bitwarden-crypto/src/sensitive/mod.rs @@ -1,7 +1,8 @@ #[allow(clippy::module_inception)] mod sensitive; -pub use sensitive::{Sensitive, SensitiveString, SensitiveVec}; +pub use sensitive::{Sensitive, SensitiveVec}; mod decrypted; pub use decrypted::{Decrypted, DecryptedString, DecryptedVec}; mod string; -pub use string::BitString; +pub use string::SensitiveString; +mod base64; diff --git a/crates/bitwarden-crypto/src/sensitive/sensitive.rs b/crates/bitwarden-crypto/src/sensitive/sensitive.rs index 2ae54d7a2..1d81989cb 100644 --- a/crates/bitwarden-crypto/src/sensitive/sensitive.rs +++ b/crates/bitwarden-crypto/src/sensitive/sensitive.rs @@ -8,6 +8,7 @@ use schemars::JsonSchema; use serde::{Deserialize, Deserializer, Serialize, Serializer}; use zeroize::{Zeroize, ZeroizeOnDrop}; +use super::string::SensitiveString; use crate::CryptoError; /// Wrapper for sensitive values which makes a best effort to enforce zeroization of the inner value @@ -28,12 +29,6 @@ pub struct Sensitive { /// To avoid this, use Vec::with_capacity to preallocate the capacity you need. pub type SensitiveVec = Sensitive>; -/// Important: This type does not protect against reallocations made by the String. -/// This means that if you insert any characters past the capacity, the data will be copied to a -/// new allocation and the old allocation will not be zeroized. -/// To avoid this, use String::with_capacity to preallocate the capacity you need. -pub type SensitiveString = Sensitive; - 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 @@ -86,14 +81,7 @@ impl TryFrom for SensitiveString { let value = std::mem::take(&mut v.value); let rtn = String::from_utf8(*value).map_err(|_| CryptoError::InvalidUtf8String); - rtn.map(|v| Sensitive::new(Box::new(v))) - } -} - -impl From for SensitiveVec { - fn from(mut s: SensitiveString) -> Self { - let value = std::mem::take(&mut s.value); - Sensitive::new(Box::new(value.into_bytes())) + rtn.map(|v| SensitiveString::new(v)) } } @@ -103,64 +91,6 @@ impl> From>> for SensitiveVec { } } -impl SensitiveString { - pub fn decode_base64(self, engine: T) -> Result { - // Prevent accidental copies by allocating the full size - let len = base64::decoded_len_estimate(self.value.len()); - let mut value = SensitiveVec::new(Box::new(Vec::with_capacity(len))); - - engine - .decode_vec(self.value.as_ref(), &mut value.value) - .map_err(|_| CryptoError::InvalidKey)?; - - Ok(value) - } - - #[inline(always)] - pub fn len(&self) -> usize { - self.value.len() - } - - #[inline(always)] - pub fn is_empty(&self) -> bool { - self.value.is_empty() - } - - // The predicate is specifically a fn() and not a closure to forbid capturing values - // from the environment, which would make it easier to accidentally leak some data. - // For example, the following won't compile with fn() but would work with impl Fn(): - // ``` - // let mut chars = Mutex::new(Vec::new()); - // self.any_chars(|c| {chars.lock().unwrap().push(c); true}); - // ``` - // Note that this is not a perfect solution, as it is still possible to leak the characters by - // using a global variable or a static variable. Also `char` implements Copy so it's hard to - // ensure the compiler is not making a copy of the character. - #[inline(always)] - pub fn any_chars(&self, predicate: fn(char) -> bool) -> bool { - self.value.chars().any(predicate) - } -} - -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(inner.len(), padding).expect("Valid length"); - - let mut value = SensitiveVec::new(Box::new(vec![0u8; len])); - engine - .encode_slice(inner, &mut value.value[..len]) - .expect("Valid base64 string length"); - - value.try_into().expect("Valid base64 string") - } -} - impl Default for Sensitive { fn default() -> Self { Self::new(Box::default()) @@ -186,11 +116,7 @@ impl> PartialEq for Sensitive { self.value.as_ref().eq(other) } } -impl PartialEq<&str> for SensitiveString { - fn eq(&self, other: &&str) -> bool { - self.value.as_ref().eq(other) - } -} + impl PartialEq<&[u8]> for SensitiveVec { fn eq(&self, other: &&[u8]) -> bool { self.value.as_ref().eq(other) @@ -248,12 +174,6 @@ mod tests { #[test] fn test_debug() { - let string = SensitiveString::test("test"); - assert_eq!( - format!("{:?}", string), - "Sensitive { type: \"alloc::string::String\", value: \"********\" }" - ); - let vector = Sensitive::new(Box::new(vec![1, 2, 3])); assert_eq!( format!("{:?}", vector), @@ -265,8 +185,6 @@ mod tests { fn test_schemars() { #[derive(JsonSchema)] struct TestStruct { - #[allow(dead_code)] - s: SensitiveString, #[allow(dead_code)] v: SensitiveVec, } @@ -279,9 +197,6 @@ mod tests { "type": "object", "required": ["s", "v"], "properties": { - "s": { - "$ref": "#/definitions/String" - }, "v": { "$ref": "#/definitions/Array_of_uint8" } diff --git a/crates/bitwarden-crypto/src/sensitive/string.rs b/crates/bitwarden-crypto/src/sensitive/string.rs index 53d3053ff..fcf706193 100644 --- a/crates/bitwarden-crypto/src/sensitive/string.rs +++ b/crates/bitwarden-crypto/src/sensitive/string.rs @@ -1,11 +1,30 @@ -use zeroize::Zeroizing; +use std::borrow::Cow; + +use schemars::JsonSchema; +use zeroize::{Zeroize, Zeroizing}; + +use crate::{Sensitive, SensitiveVec}; /// A sensitive string that supports string operations. -pub struct BitString { +/// +/// Important: The `SensitiveString` protects against reallocations in the internal string. However, +/// be careful when using any str slices as cloning them will create a `String` which is not +/// protected. +#[derive(Eq, Clone, Zeroize)] +pub struct SensitiveString { inner: Zeroizing, } -impl BitString { +impl std::fmt::Debug for SensitiveString { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Sensitive") + .field("type", &std::any::type_name::()) + .field("value", &"********") + .finish() + } +} + +impl SensitiveString { #[inline(always)] pub fn new(inner: String) -> Self { Self { @@ -65,9 +84,37 @@ impl BitString { pub fn is_empty(&self) -> bool { self.inner.is_empty() } + + // The predicate is specifically a fn() and not a closure to forbid capturing values + // from the environment, which would make it easier to accidentally leak some data. + // For example, the following won't compile with fn() but would work with impl Fn(): + // ``` + // let mut chars = Mutex::new(Vec::new()); + // self.any_chars(|c| {chars.lock().unwrap().push(c); true}); + // ``` + // Note that this is not a perfect solution, as it is still possible to leak the characters by + // using a global variable or a static variable. Also `char` implements Copy so it's hard to + // ensure the compiler is not making a copy of the character. + #[inline(always)] + pub fn any_chars(&self, predicate: fn(char) -> bool) -> bool { + self.as_str().chars().any(predicate) + } +} + +impl Default for SensitiveString { + fn default() -> Self { + Self::new(String::default()) + } +} + +impl From for SensitiveVec { + fn from(mut s: SensitiveString) -> Self { + let value: String = std::mem::take(&mut s.inner); + Sensitive::new(Box::new(value.into_bytes())) + } } -impl std::ops::Index> for BitString { +impl std::ops::Index> for SensitiveString { type Output = str; #[inline] @@ -76,7 +123,7 @@ impl std::ops::Index> for BitString { } } -impl std::ops::Index> for BitString { +impl std::ops::Index> for SensitiveString { type Output = str; #[inline] @@ -85,13 +132,65 @@ impl std::ops::Index> for BitString { } } +impl PartialEq for SensitiveString { + fn eq(&self, other: &Self) -> bool { + self.inner.eq(&other.inner) + } +} + +impl PartialEq<&str> for SensitiveString { + fn eq(&self, other: &&str) -> bool { + self.inner.as_str().eq(*other) + } +} + +/// Transparently expose the inner value for serialization +impl JsonSchema for SensitiveString { + fn schema_name() -> String { + String::schema_name() + } + + fn schema_id() -> Cow<'static, str> { + String::schema_id() + } + + fn json_schema(gen: &mut schemars::gen::SchemaGenerator) -> schemars::schema::Schema { + String::json_schema(gen) + } +} + +/// Unfortunately once we serialize a `SensitiveString` we can't control the future memory. +impl serde::Serialize for SensitiveString { + fn serialize(&self, serializer: S) -> Result { + self.inner.serialize(serializer) + } +} + +impl<'de> serde::Deserialize<'de> for SensitiveString { + fn deserialize>(deserializer: D) -> Result { + Ok(Self::new(String::deserialize(deserializer)?)) + } +} + +// We use a lot of `&str` and `&[u8]` in our tests, so we expose this helper +// to make it easier. +// IMPORTANT: This should not be used outside of test code +// Note that we can't just mark it with #[cfg(test)] because that only applies +// when testing this crate, not when testing other crates that depend on it. +// By at least limiting it to &'static reference we should be able to avoid accidental usages +impl SensitiveString { + pub fn test>(value: S) -> Self { + Self::new(value.into()) + } +} + #[cfg(test)] mod tests { use super::*; #[test] fn test_bit_string() { - let mut bit_string = BitString::new("hello".to_string()); + let mut bit_string = SensitiveString::new("hello".to_string()); bit_string.push_str(" world"); assert_eq!(bit_string.inner.as_str(), "hello world"); @@ -99,31 +198,31 @@ mod tests { #[test] fn test_len() { - let s = BitString::new("Hello, world!".to_owned()); + let s = SensitiveString::new("Hello, world!".to_owned()); assert_eq!(s.len(), 13); } #[test] fn test_is_empty() { - let s = BitString::new("".to_owned()); + let s = SensitiveString::new("".to_owned()); assert!(s.is_empty()); } #[test] fn test_is_not_empty() { - let s = BitString::new("Not empty".to_owned()); + let s = SensitiveString::new("Not empty".to_owned()); assert!(!s.is_empty()); } #[test] fn test_index_range() { - let s = BitString::new("Hello, world!".to_owned()); + let s = SensitiveString::new("Hello, world!".to_owned()); assert_eq!(&s[0..5], "Hello"); } #[test] fn test_index_range_from() { - let s = BitString::new("Hello, world!".to_owned()); + let s = SensitiveString::new("Hello, world!".to_owned()); assert_eq!(&s[7..], "world!"); } } diff --git a/crates/bitwarden-crypto/src/uniffi_support.rs b/crates/bitwarden-crypto/src/uniffi_support.rs index f7269f96c..10f3213ff 100644 --- a/crates/bitwarden-crypto/src/uniffi_support.rs +++ b/crates/bitwarden-crypto/src/uniffi_support.rs @@ -52,10 +52,10 @@ impl UniffiCustomTypeConverter for SensitiveString { type Builtin = String; fn into_custom(val: Self::Builtin) -> uniffi::Result { - Ok(SensitiveString::new(Box::new(val))) + Ok(SensitiveString::new(val)) } fn from_custom(obj: Self) -> Self::Builtin { - obj.expose().to_owned() + obj.as_str().to_owned() } } diff --git a/crates/bitwarden-exporters/src/csv.rs b/crates/bitwarden-exporters/src/csv.rs index d4bbe5174..d304e49cb 100644 --- a/crates/bitwarden-exporters/src/csv.rs +++ b/crates/bitwarden-exporters/src/csv.rs @@ -79,7 +79,7 @@ fn vec_serialize(x: &[DecryptedString], s: S) -> Result where S: Serializer, { - let iter = itertools::Itertools::intersperse(x.iter().map(|s| s.expose().as_str()), ","); + let iter = itertools::Itertools::intersperse(x.iter().map(|s| s.as_str()), ","); let result: Sensitive = Sensitive::new(Box::new(iter.collect())); s.serialize_str(result.expose()) @@ -101,14 +101,8 @@ where .map(|f| { format!( "{}: {}", - f.name - .as_ref() - .map(|n| n.expose().as_str()) - .unwrap_or_default(), - f.value - .as_ref() - .map(|n| n.expose().as_str()) - .unwrap_or_default(), + f.name.as_ref().map(|n| n.as_str()).unwrap_or_default(), + f.value.as_ref().map(|n| n.as_str()).unwrap_or_default(), ) }) .collect::>() diff --git a/crates/bitwarden-exporters/src/encrypted_json.rs b/crates/bitwarden-exporters/src/encrypted_json.rs index 31b31a827..ecb1e89bc 100644 --- a/crates/bitwarden-exporters/src/encrypted_json.rs +++ b/crates/bitwarden-exporters/src/encrypted_json.rs @@ -47,14 +47,14 @@ 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.into(), salt.expose().as_bytes(), &kdf)?; + let key = PinKey::derive(&password.into(), salt.as_str().as_bytes(), &kdf)?; let enc_key_validation = Uuid::new_v4().to_string(); let encrypted_export = EncryptedJsonExport { encrypted: true, password_protected: true, - salt: salt.expose().to_string(), + salt: salt.as_str().to_string(), kdf_type, kdf_iterations, kdf_memory, diff --git a/crates/bitwarden/src/auth/auth_request.rs b/crates/bitwarden/src/auth/auth_request.rs index ca47ad579..515cce2c3 100644 --- a/crates/bitwarden/src/auth/auth_request.rs +++ b/crates/bitwarden/src/auth/auth_request.rs @@ -167,11 +167,8 @@ mod tests { let private_key = "MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCzLtEUdxfcLxDj84yaGFsVF5hZ8Hjlb08NMQDy1RnBma06I3ZESshLYzVz4r/gegMn9OOltfV/Yxlyvida8oW6qdlfJ7AVz6Oa8pV7BiL40C7b76+oqraQpyYw2HChANB1AhXL9SqWngKmLZwjA7qiCrmcc0kZHeOb4KnKtp9iVvPVs+8veFvKgYO4ba2AAOHKFdR0W55/agXfAy+fWUAkC8mc9ikyJdQWaPV6OZvC2XFkOseBQm9Rynudh3BQpoWiL6w620efe7t5k+02/EyOFJL9f/XEEjM/+Yo0t3LAfkuhHGeKiRST59Xc9hTEmyJTeVXROtz+0fjqOp3xkaObAgMBAAECggEACs4xhnO0HaZhh1/iH7zORMIRXKeyxP2LQiTR8xwN5JJ9wRWmGAR9VasS7EZFTDidIGVME2u/h4s5EqXnhxfO+0gGksVvgNXJ/qw87E8K2216g6ZNo6vSGA7H1GH2voWwejJ4/k/cJug6dz2S402rRAKh2Wong1arYHSkVlQp3diiMa5FHAOSE+Cy09O2ZsaF9IXQYUtlW6AVXFrBEPYH2kvkaPXchh8VETMijo6tbvoKLnUHe+wTaDMls7hy8exjtVyI59r3DNzjy1lNGaGb5QSnFMXR+eHhPZc844Wv02MxC15zKABADrl58gpJyjTl6XpDdHCYGsmGpVGH3X9TQQKBgQDz/9beFjzq59ve6rGwn+EtnQfSsyYT+jr7GN8lNEXb3YOFXBgPhfFIcHRh2R00Vm9w2ApfAx2cd8xm2I6HuvQ1Os7g26LWazvuWY0Qzb+KaCLQTEGH1RnTq6CCG+BTRq/a3J8M4t38GV5TWlzv8wr9U4dl6FR4efjb65HXs1GQ4QKBgQC7/uHfrOTEHrLeIeqEuSl0vWNqEotFKdKLV6xpOvNuxDGbgW4/r/zaxDqt0YBOXmRbQYSEhmO3oy9J6XfE1SUln0gbavZeW0HESCAmUIC88bDnspUwS9RxauqT5aF8ODKN/bNCWCnBM1xyonPOs1oT1nyparJVdQoG//Y7vkB3+wKBgBqLqPq8fKAp3XfhHLfUjREDVoiLyQa/YI9U42IOz9LdxKNLo6p8rgVthpvmnRDGnpUuS+KOWjhdqDVANjF6G3t3DG7WNl8Rh5Gk2H4NhFswfSkgQrjebFLlBy9gjQVCWXt8KSmjvPbiY6q52Aaa8IUjA0YJAregvXxfopxO+/7BAoGARicvEtDp7WWnSc1OPoj6N14VIxgYcI7SyrzE0d/1x3ffKzB5e7qomNpxKzvqrVP8DzG7ydh8jaKPmv1MfF8tpYRy3AhmN3/GYwCnPqT75YYrhcrWcVdax5gmQVqHkFtIQkRSCIftzPLlpMGKha/YBV8c1fvC4LD0NPh/Ynv0gtECgYEAyOZg95/kte0jpgUEgwuMrzkhY/AaUJULFuR5MkyvReEbtSBQwV5tx60+T95PHNiFooWWVXiLMsAgyI2IbkxVR1Pzdri3gWK5CTfqb7kLuaj/B7SGvBa2Sxo478KS5K8tBBBWkITqo+wLC0mn3uZi1dyMWO1zopTA+KtEGF2dtGQ="; let enc_user_key = "4.dxbd5OMwi/Avy7DQxvLV+Z7kDJgHBtg/jAbgYNO7QU0Zii4rLFNco2lS5aS9z42LTZHc2p5HYwn2ZwkZNfHsQ6//d5q40MDgGYJMKBXOZP62ZHhct1XsvYBmtcUtIOm5j2HSjt2pjEuGAc1LbyGIWRJJQ3Lp1ULbL2m71I+P23GF36JyOM8SUWvpvxE/3+qqVhRFPG2VqMCYa2kLLxwVfUmpV+KKjX1TXsrq6pfJIwHNwHw4h7MSfD8xTy2bx4MiBt638Z9Vt1pGsSQkh9RgPvCbnhuCpZQloUgJ8ByLVEcrlKx3yaaxiQXvte+ZhuOI7rGdjmoVoOzisooje4JgYw==".parse().unwrap(); - let dec = auth_request_decrypt_user_key( - SensitiveString::new(Box::new(private_key.to_owned())), - enc_user_key, - ) - .unwrap(); + let dec = auth_request_decrypt_user_key(SensitiveString::test(private_key), enc_user_key) + .unwrap(); assert_eq!( dec.to_vec(), @@ -190,7 +187,7 @@ mod tests { let enc_master_key = "4.dxbd5OMwi/Avy7DQxvLV+Z7kDJgHBtg/jAbgYNO7QU0Zii4rLFNco2lS5aS9z42LTZHc2p5HYwn2ZwkZNfHsQ6//d5q40MDgGYJMKBXOZP62ZHhct1XsvYBmtcUtIOm5j2HSjt2pjEuGAc1LbyGIWRJJQ3Lp1ULbL2m71I+P23GF36JyOM8SUWvpvxE/3+qqVhRFPG2VqMCYa2kLLxwVfUmpV+KKjX1TXsrq6pfJIwHNwHw4h7MSfD8xTy2bx4MiBt638Z9Vt1pGsSQkh9RgPvCbnhuCpZQloUgJ8ByLVEcrlKx3yaaxiQXvte+ZhuOI7rGdjmoVoOzisooje4JgYw==".parse().unwrap(); let enc_user_key = "2.Q/2PhzcC7GdeiMHhWguYAQ==|GpqzVdr0go0ug5cZh1n+uixeBC3oC90CIe0hd/HWA/pTRDZ8ane4fmsEIcuc8eMKUt55Y2q/fbNzsYu41YTZzzsJUSeqVjT8/iTQtgnNdpo=|dwI+uyvZ1h/iZ03VQ+/wrGEFYVewBUUl/syYgjsNMbE=".parse().unwrap(); let dec = auth_request_decrypt_master_key( - SensitiveString::new(Box::new(private_key.to_owned())), + SensitiveString::test(private_key), enc_master_key, enc_user_key, ) diff --git a/crates/bitwarden/src/auth/login/password.rs b/crates/bitwarden/src/auth/login/password.rs index db392025f..73a711fdb 100644 --- a/crates/bitwarden/src/auth/login/password.rs +++ b/crates/bitwarden/src/auth/login/password.rs @@ -40,7 +40,7 @@ pub(crate) async fn login_password( client, &input.email, &input.two_factor, - password_hash.expose(), + password_hash.as_str(), ) .await?; diff --git a/crates/bitwarden/src/auth/login/two_factor.rs b/crates/bitwarden/src/auth/login/two_factor.rs index d0533cd39..dbdf4afd3 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.expose().clone()), + master_password_hash: Some(password_hash.as_str().to_owned()), otp: None, auth_request_access_code: None, secret: None, diff --git a/crates/bitwarden/src/auth/password/strength.rs b/crates/bitwarden/src/auth/password/strength.rs index d958ae7c2..6e5a44cd0 100644 --- a/crates/bitwarden/src/auth/password/strength.rs +++ b/crates/bitwarden/src/auth/password/strength.rs @@ -14,7 +14,7 @@ pub(crate) fn password_strength( let mut arr: Vec<_> = inputs.iter().map(String::as_str).collect(); arr.extend(GLOBAL_INPUTS); - zxcvbn(password.expose(), &arr).map_or(0, |e| e.score()) + zxcvbn(password.as_str(), &arr).map_or(0, |e| e.score()) } fn email_to_user_inputs(email: &str) -> Vec { diff --git a/crates/bitwarden/src/auth/register.rs b/crates/bitwarden/src/auth/register.rs index 50e4e41cb..d7105293c 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.expose().clone(), + master_password_hash: keys.master_password_hash.as_str().to_owned(), master_password_hint: req.password_hint, captcha_response: None, // TODO: Add key: Some(keys.encrypted_user_key), diff --git a/crates/bitwarden/src/platform/get_user_api_key.rs b/crates/bitwarden/src/platform/get_user_api_key.rs index 14e961da5..48679f07b 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.map(|h| h.expose().clone()), + master_password_hash: master_password_hash.map(|h| h.as_str().to_owned()), otp: input.otp.as_ref().cloned(), secret: None, auth_request_access_code: None, diff --git a/crates/bitwarden/src/secrets_manager/projects/project_response.rs b/crates/bitwarden/src/secrets_manager/projects/project_response.rs index bf56f2006..2938cf522 100644 --- a/crates/bitwarden/src/secrets_manager/projects/project_response.rs +++ b/crates/bitwarden/src/secrets_manager/projects/project_response.rs @@ -37,7 +37,7 @@ impl ProjectResponse { Ok(ProjectResponse { id: require!(response.id), organization_id, - name: name.expose().to_owned(), + name: name.as_str().to_owned(), creation_date: require!(response.creation_date).parse()?, revision_date: require!(response.revision_date).parse()?, diff --git a/crates/bitwarden/src/secrets_manager/secrets/list.rs b/crates/bitwarden/src/secrets_manager/secrets/list.rs index b2473dd88..f8970611e 100644 --- a/crates/bitwarden/src/secrets_manager/secrets/list.rs +++ b/crates/bitwarden/src/secrets_manager/secrets/list.rs @@ -105,7 +105,7 @@ impl SecretIdentifierResponse { Ok(SecretIdentifierResponse { id: require!(response.id), organization_id, - key: key.expose().to_owned(), + key: key.as_str().to_owned(), }) } } diff --git a/crates/bitwarden/src/secrets_manager/secrets/secret_response.rs b/crates/bitwarden/src/secrets_manager/secrets/secret_response.rs index 6f7446362..f9c507608 100644 --- a/crates/bitwarden/src/secrets_manager/secrets/secret_response.rs +++ b/crates/bitwarden/src/secrets_manager/secrets/secret_response.rs @@ -71,9 +71,9 @@ impl SecretResponse { id: require!(response.id), organization_id: require!(org_id), project_id: project, - key: key.expose().to_owned(), - value: value.expose().to_owned(), - note: note.expose().to_owned(), + key: key.as_str().to_owned(), + value: value.as_str().to_owned(), + note: note.as_str().to_owned(), creation_date: require!(response.creation_date).parse()?, revision_date: require!(response.revision_date).parse()?, diff --git a/crates/bitwarden/src/secrets_manager/state.rs b/crates/bitwarden/src/secrets_manager/state.rs index aadea564d..aeae520e6 100644 --- a/crates/bitwarden/src/secrets_manager/state.rs +++ b/crates/bitwarden/src/secrets_manager/state.rs @@ -36,7 +36,7 @@ pub fn get(state_file: &Path, access_token: &AccessToken) -> Result let encrypted_state: EncString = file_content.parse()?; let decrypted_state: DecryptedString = encrypted_state.decrypt_with_key(&access_token.encryption_key)?; - let client_state: ClientState = serde_json::from_str(decrypted_state.expose())?; + let client_state: ClientState = serde_json::from_str(decrypted_state.as_str())?; if client_state.version != STATE_VERSION { return Err(Error::InvalidStateFileVersion); diff --git a/crates/bitwarden/src/vault/cipher/cipher.rs b/crates/bitwarden/src/vault/cipher/cipher.rs index dd0bebbaf..4de93b8f6 100644 --- a/crates/bitwarden/src/vault/cipher/cipher.rs +++ b/crates/bitwarden/src/vault/cipher/cipher.rs @@ -1,6 +1,6 @@ use bitwarden_api_api::models::CipherDetailsResponseModel; use bitwarden_crypto::{ - BitString, CryptoError, DecryptedString, DecryptedVec, EncString, KeyContainer, KeyDecryptable, + CryptoError, DecryptedString, DecryptedVec, EncString, KeyContainer, KeyDecryptable, KeyEncryptable, LocateKey, SensitiveString, SymmetricCryptoKey, }; use chrono::{DateTime, Utc}; @@ -293,42 +293,38 @@ fn build_subtitle_card( brand: Option, number: Option, ) -> SensitiveString { - let brand: Option = brand.filter(|b: &SensitiveString| !b.expose().is_empty()); + let brand: Option = brand.filter(|b: &SensitiveString| !b.is_empty()); // We only want to expose the last 4 or 5 digits of the card number - let number: Option = number - .map(|n| BitString::new(n.expose().clone())) - .filter(|n| n.len() > 4) - .map(|n| { - // For AMEX cards show 5 digits instead of 4 - let desired_len = match &n[0..2] { - "34" | "37" => 5, - _ => 4, - }; - let start = n.len() - desired_len; - - let mut str = BitString::with_capacity(desired_len + 1); - str.push('*'); - str.push_str(&n[start..]); + let number: Option = number.filter(|n| n.len() > 4).map(|n| { + // For AMEX cards show 5 digits instead of 4 + let desired_len = match &n[0..2] { + "34" | "37" => 5, + _ => 4, + }; + let start = n.len() - desired_len; - str - }) - .map(|n| SensitiveString::new(Box::new(n.as_str().to_string()))); + let mut str = SensitiveString::with_capacity(desired_len + 1); + str.push('*'); + str.push_str(&n[start..]); + + str + }); match (brand, number) { (Some(brand), Some(number)) => { - let length = brand.expose().len() + 2 + number.expose().len(); + let length = brand.len() + 2 + number.len(); - let mut str = BitString::with_capacity(length); - str.push_str(brand.expose()); + let mut str = SensitiveString::with_capacity(length); + str.push_str(brand.as_str()); str.push_str(", "); - str.push_str(number.expose()); + str.push_str(number.as_str()); - SensitiveString::new(Box::new(str.as_str().to_string())) + str } (Some(brand), None) => brand, (None, Some(number)) => number, - _ => SensitiveString::new(Box::new("".to_owned())), + _ => SensitiveString::new("".to_owned()), } } @@ -340,24 +336,23 @@ fn build_subtitle_identity( last_name: Option, ) -> SensitiveString { let first_name: Option = - first_name.filter(|f: &SensitiveString| !f.expose().is_empty()); - let last_name: Option = - last_name.filter(|l: &SensitiveString| !l.expose().is_empty()); + first_name.filter(|f: &SensitiveString| !f.is_empty()); + let last_name: Option = last_name.filter(|l: &SensitiveString| !l.is_empty()); match (first_name, last_name) { (Some(first_name), Some(last_name)) => { - let length = first_name.expose().len() + 1 + last_name.expose().len(); + let length = first_name.len() + 1 + last_name.len(); - let mut str = SensitiveString::new(Box::new(String::with_capacity(length))); - str.expose_mut().push_str(first_name.expose()); - str.expose_mut().push(' '); - str.expose_mut().push_str(last_name.expose()); + let mut str = SensitiveString::with_capacity(length); + str.push_str(first_name.as_str()); + str.push(' '); + str.push_str(last_name.as_str()); str } (Some(first_name), None) => first_name, (None, Some(last_name)) => last_name, - _ => SensitiveString::new(Box::new("".to_owned())), + _ => SensitiveString::new("".to_owned()), } } diff --git a/crates/bitwarden/src/vault/cipher/login.rs b/crates/bitwarden/src/vault/cipher/login.rs index 456d6223b..c09b446d5 100644 --- a/crates/bitwarden/src/vault/cipher/login.rs +++ b/crates/bitwarden/src/vault/cipher/login.rs @@ -58,7 +58,7 @@ impl LoginUriView { let uri_hash: Sensitive> = Sensitive::new(Box::new( sha2::Sha256::new() - .chain_update(uri.expose().as_bytes()) + .chain_update(uri.as_str().as_bytes()) .finalize(), )); @@ -69,7 +69,7 @@ impl LoginUriView { if let Some(uri) = &self.uri { let uri_hash: SensitiveVec = Sensitive::new(Box::new( sha2::Sha256::new() - .chain_update(uri.expose().as_bytes()) + .chain_update(uri.as_str().as_bytes()) .finalize(), )) .into(); @@ -301,7 +301,7 @@ mod tests { uri.generate_checksum(); assert_eq!( - uri.uri_checksum.unwrap().expose(), + uri.uri_checksum.unwrap(), "OWk2vQvwYD1nhLZdA+ltrpBWbDa2JmHyjUEWxRZSS8w=" ); } diff --git a/crates/bitwarden/src/vault/send.rs b/crates/bitwarden/src/vault/send.rs index dba36e44a..961a996eb 100644 --- a/crates/bitwarden/src/vault/send.rs +++ b/crates/bitwarden/src/vault/send.rs @@ -310,7 +310,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.map(|p| SensitiveString::new(Box::new(p))), + password: send.password.map(|p| SensitiveString::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()?, diff --git a/crates/bw/src/auth/login.rs b/crates/bw/src/auth/login.rs index ccc2024d9..aa8201af0 100644 --- a/crates/bw/src/auth/login.rs +++ b/crates/bw/src/auth/login.rs @@ -15,9 +15,7 @@ use log::{debug, error, info}; pub(crate) async fn login_password(mut client: Client, email: Option) -> Result<()> { let email = text_prompt_when_none("Email", email)?; - let password = SensitiveString::new(Box::new( - Password::new("Password").without_confirmation().prompt()?, - )); + let password = SensitiveString::new(Password::new("Password").without_confirmation().prompt()?); let kdf = client.auth().prelogin(email.clone()).await?; @@ -102,9 +100,7 @@ pub(crate) async fn login_api_key( let client_id = text_prompt_when_none("Client ID", client_id)?; let client_secret = text_prompt_when_none("Client Secret", client_secret)?; - let password = SensitiveString::new(Box::new( - Password::new("Password").without_confirmation().prompt()?, - )); + let password = SensitiveString::new(Password::new("Password").without_confirmation().prompt()?); let result = client .auth() diff --git a/crates/bw/src/main.rs b/crates/bw/src/main.rs index d973c4074..20d8e1f8f 100644 --- a/crates/bw/src/main.rs +++ b/crates/bw/src/main.rs @@ -192,7 +192,7 @@ async fn process_commands() -> Result<()> { let mut client = bitwarden::Client::new(settings); let email = text_prompt_when_none("Email", email)?; - let password = SensitiveString::new(Box::new(Password::new("Password").prompt()?)); + let password = SensitiveString::new(Password::new("Password").prompt()?); client .auth() diff --git a/crates/memory-testing/src/main.rs b/crates/memory-testing/src/main.rs index 5f500522e..37d27a5c3 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::{BitString, MasterKey, SensitiveString, SensitiveVec, SymmetricCryptoKey}; +use bitwarden_crypto::{MasterKey, SensitiveString, SensitiveVec, SymmetricCryptoKey}; fn wait_for_dump() { println!("Waiting for dump..."); @@ -22,7 +22,7 @@ fn main() { let mut symmetric_keys = Vec::new(); for case in cases.symmetric_key { - let key = SensitiveString::new(Box::new(case.key)); + let key = SensitiveString::new(case.key); let key = SymmetricCryptoKey::try_from(key).unwrap(); symmetric_keys.push((key.to_vec(), key)); } @@ -30,7 +30,7 @@ fn main() { let mut master_keys = Vec::new(); for case in cases.master_key { - let password: SensitiveVec = SensitiveString::new(Box::new(case.password)).into(); + let password: SensitiveVec = SensitiveString::new(case.password).into(); let key = MasterKey::derive(&password, case.email.as_bytes(), &case.kdf).unwrap(); let hash = key @@ -45,8 +45,8 @@ fn main() { let mut bit_strings = Vec::new(); for case in cases.bit_string { - let mut left = BitString::new(case.left); - let right = BitString::new(case.right); + let mut left = SensitiveString::new(case.left); + let right = SensitiveString::new(case.right); left.push_str(right.as_str()); bit_strings.push(left);