From 8c869f26f1ffeb56f122af440569ebbdb63f3db1 Mon Sep 17 00:00:00 2001 From: Hinton Date: Fri, 19 Jan 2024 14:40:54 +0100 Subject: [PATCH 01/22] Explore introducing a Decrypted struct --- Cargo.lock | 15 +++++++++ crates/bitwarden-crypto/Cargo.toml | 1 + crates/bitwarden-crypto/src/decrypted.rs | 32 +++++++++++++++++++ .../src/enc_string/symmetric.rs | 26 +++++++-------- crates/bitwarden-crypto/src/lib.rs | 2 ++ 5 files changed, 63 insertions(+), 13 deletions(-) create mode 100644 crates/bitwarden-crypto/src/decrypted.rs diff --git a/Cargo.lock b/Cargo.lock index 960a7707f..bfbb5cbb0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -445,6 +445,7 @@ dependencies = [ "thiserror", "uniffi", "uuid", + "zeroize", ] [[package]] @@ -4339,6 +4340,20 @@ name = "zeroize" version = "1.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "525b4ec142c6b68a2d10f01f7bbf6755599ca3f81ea53b8431b7dd348f5fdb2d" +dependencies = [ + "zeroize_derive", +] + +[[package]] +name = "zeroize_derive" +version = "1.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ce36e65b0d2999d2aafac989fb249189a141aee1f53c612c1f37d72631959f69" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.47", +] [[package]] name = "zxcvbn" diff --git a/crates/bitwarden-crypto/Cargo.toml b/crates/bitwarden-crypto/Cargo.toml index ddcaceb06..b04e993ad 100644 --- a/crates/bitwarden-crypto/Cargo.toml +++ b/crates/bitwarden-crypto/Cargo.toml @@ -39,6 +39,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 = ["zeroize_derive"] } [dev-dependencies] rand_chacha = "0.3.1" diff --git a/crates/bitwarden-crypto/src/decrypted.rs b/crates/bitwarden-crypto/src/decrypted.rs new file mode 100644 index 000000000..c218aee22 --- /dev/null +++ b/crates/bitwarden-crypto/src/decrypted.rs @@ -0,0 +1,32 @@ +use zeroize::{Zeroize, ZeroizeOnDrop}; + +use crate::CryptoError; + +#[derive(Zeroize, ZeroizeOnDrop)] +pub struct Decrypted { + value: V, +} + +impl Decrypted { + pub fn new(value: V) -> Self { + Self { value } + } + + pub fn expose(&self) -> &V { + &self.value + } +} + +impl TryFrom>> for Decrypted { + type Error = CryptoError; + + fn try_from(v: Decrypted>) -> Result { + let mut str = v.expose().to_owned(); + + let rtn = String::from_utf8(str).map_err(|_| CryptoError::InvalidUtf8String); + + str.zeroize(); + + rtn.map(Decrypted::new) + } +} diff --git a/crates/bitwarden-crypto/src/enc_string/symmetric.rs b/crates/bitwarden-crypto/src/enc_string/symmetric.rs index d6dd44374..b9e4ee9e9 100644 --- a/crates/bitwarden-crypto/src/enc_string/symmetric.rs +++ b/crates/bitwarden-crypto/src/enc_string/symmetric.rs @@ -7,7 +7,7 @@ use serde::Deserialize; use super::{check_length, from_b64, from_b64_vec, split_enc_string}; use crate::{ error::{CryptoError, EncStringParseError, Result}, - KeyDecryptable, KeyEncryptable, LocateKey, SymmetricCryptoKey, + Decrypted, KeyDecryptable, KeyEncryptable, LocateKey, SymmetricCryptoKey, }; /// # Encrypted string primitive @@ -228,13 +228,13 @@ impl KeyEncryptable for &[u8] { } } -impl KeyDecryptable> for EncString { - fn decrypt_with_key(&self, key: &SymmetricCryptoKey) -> Result> { +impl KeyDecryptable>> for EncString { + fn decrypt_with_key(&self, key: &SymmetricCryptoKey) -> Result>> { 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)?; - Ok(dec) + Ok(Decrypted::new(dec)) } _ => Err(CryptoError::InvalidKey), } @@ -247,10 +247,10 @@ impl KeyEncryptable for String { } } -impl KeyDecryptable for EncString { - fn decrypt_with_key(&self, key: &SymmetricCryptoKey) -> Result { - let dec: Vec = self.decrypt_with_key(key)?; - String::from_utf8(dec).map_err(|_| CryptoError::InvalidUtf8String) +impl KeyDecryptable> for EncString { + fn decrypt_with_key(&self, key: &SymmetricCryptoKey) -> Result> { + let dec: Decrypted> = self.decrypt_with_key(key)?; + dec.try_into() } } @@ -269,17 +269,17 @@ impl schemars::JsonSchema for EncString { #[cfg(test)] mod tests { use super::EncString; - use crate::{derive_symmetric_key, KeyDecryptable, KeyEncryptable}; + use crate::{derive_symmetric_key, Decrypted, KeyDecryptable, KeyEncryptable}; #[test] fn test_enc_string_roundtrip() { let key = derive_symmetric_key("test"); - let test_string = "encrypted_test_string".to_string(); - let cipher = test_string.clone().encrypt_with_key(&key).unwrap(); + let test_string = "encrypted_test_string"; + let cipher = test_string.to_owned().encrypt_with_key(&key).unwrap(); - let decrypted_str: String = cipher.decrypt_with_key(&key).unwrap(); - assert_eq!(decrypted_str, test_string); + let decrypted_str: Decrypted = cipher.decrypt_with_key(&key).unwrap(); + assert_eq!(decrypted_str.expose(), test_string); } #[test] diff --git a/crates/bitwarden-crypto/src/lib.rs b/crates/bitwarden-crypto/src/lib.rs index 3c1aced24..2d4b5aa9b 100644 --- a/crates/bitwarden-crypto/src/lib.rs +++ b/crates/bitwarden-crypto/src/lib.rs @@ -40,6 +40,8 @@ pub use util::generate_random_bytes; mod wordlist; pub use util::pbkdf2; pub use wordlist::EFF_LONG_WORD_LIST; +mod decrypted; +pub use decrypted::Decrypted; #[cfg(feature = "mobile")] uniffi::setup_scaffolding!(); From 74a5b084540bdc2f8fe111aa5af262643352663c Mon Sep 17 00:00:00 2001 From: Hinton Date: Fri, 19 Jan 2024 15:03:33 +0100 Subject: [PATCH 02/22] Replace with std::mem::take --- crates/bitwarden-crypto/src/decrypted.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/crates/bitwarden-crypto/src/decrypted.rs b/crates/bitwarden-crypto/src/decrypted.rs index c218aee22..f267ba311 100644 --- a/crates/bitwarden-crypto/src/decrypted.rs +++ b/crates/bitwarden-crypto/src/decrypted.rs @@ -20,13 +20,10 @@ impl Decrypted { impl TryFrom>> for Decrypted { type Error = CryptoError; - fn try_from(v: Decrypted>) -> Result { - let mut str = v.expose().to_owned(); - - let rtn = String::from_utf8(str).map_err(|_| CryptoError::InvalidUtf8String); - - str.zeroize(); + fn try_from(mut v: Decrypted>) -> Result { + let value = std::mem::take(&mut v.value); + let rtn = String::from_utf8(value).map_err(|_| CryptoError::InvalidUtf8String); rtn.map(Decrypted::new) } } From d5f2ae5a25d3db53b71965010337c25f262e3ceb Mon Sep 17 00:00:00 2001 From: Hinton Date: Thu, 25 Jan 2024 10:38:22 +0100 Subject: [PATCH 03/22] Document and introduce type alias --- crates/bitwarden-crypto/src/decrypted.rs | 12 ++++++++++-- .../bitwarden-crypto/src/enc_string/symmetric.rs | 15 ++++++++------- crates/bitwarden-crypto/src/keys/device_key.rs | 10 ++++++---- crates/bitwarden-crypto/src/keys/master_key.rs | 5 +++-- 4 files changed, 27 insertions(+), 15 deletions(-) diff --git a/crates/bitwarden-crypto/src/decrypted.rs b/crates/bitwarden-crypto/src/decrypted.rs index f267ba311..140c99a37 100644 --- a/crates/bitwarden-crypto/src/decrypted.rs +++ b/crates/bitwarden-crypto/src/decrypted.rs @@ -2,6 +2,9 @@ use zeroize::{Zeroize, ZeroizeOnDrop}; use crate::CryptoError; +/// A wrapper for decrypted values. +/// +/// Implements `Zeroize` and `ZeroizeOnDrop` to ensure that the value is zeroized on drop. Please be careful if cloning or copying the inner value using `expose` since any copy will not be zeroized. #[derive(Zeroize, ZeroizeOnDrop)] pub struct Decrypted { value: V, @@ -12,18 +15,23 @@ impl Decrypted { Self { value } } + /// Expose the inner value. By exposing the inner value, you take responsibility for ensuring that any copy of the value is zeroized. pub fn expose(&self) -> &V { &self.value } } -impl TryFrom>> for Decrypted { +/// Helper to convert a `Decrypted>` to a `Decrypted`, care is taken to ensure any intermediate copies are zeroed to avoid leaking sensitive data. +impl TryFrom for DecryptedString { type Error = CryptoError; - fn try_from(mut v: Decrypted>) -> Result { + fn try_from(mut v: DecryptedVec) -> Result { let value = std::mem::take(&mut v.value); let rtn = String::from_utf8(value).map_err(|_| CryptoError::InvalidUtf8String); rtn.map(Decrypted::new) } } + +pub type DecryptedVec = Decrypted>; +pub type DecryptedString = Decrypted; diff --git a/crates/bitwarden-crypto/src/enc_string/symmetric.rs b/crates/bitwarden-crypto/src/enc_string/symmetric.rs index ed832b32b..ee6c22b30 100644 --- a/crates/bitwarden-crypto/src/enc_string/symmetric.rs +++ b/crates/bitwarden-crypto/src/enc_string/symmetric.rs @@ -6,6 +6,7 @@ use serde::Deserialize; use super::{check_length, from_b64, from_b64_vec, split_enc_string}; use crate::{ + decrypted::{DecryptedString, DecryptedVec}, error::{CryptoError, EncStringParseError, Result}, Decrypted, KeyDecryptable, KeyEncryptable, LocateKey, SymmetricCryptoKey, }; @@ -228,8 +229,8 @@ impl KeyEncryptable for &[u8] { } } -impl KeyDecryptable>> for EncString { - fn decrypt_with_key(&self, key: &SymmetricCryptoKey) -> Result>> { +impl KeyDecryptable for EncString { + fn decrypt_with_key(&self, key: &SymmetricCryptoKey) -> Result { match self { EncString::AesCbc256_HmacSha256_B64 { iv, mac, data } => { let mac_key = key.mac_key.ok_or(CryptoError::InvalidMac)?; @@ -247,9 +248,9 @@ impl KeyEncryptable for String { } } -impl KeyDecryptable> for EncString { - fn decrypt_with_key(&self, key: &SymmetricCryptoKey) -> Result> { - let dec: Decrypted> = self.decrypt_with_key(key)?; +impl KeyDecryptable for EncString { + fn decrypt_with_key(&self, key: &SymmetricCryptoKey) -> Result { + let dec: DecryptedVec = self.decrypt_with_key(key)?; dec.try_into() } } @@ -269,7 +270,7 @@ impl schemars::JsonSchema for EncString { #[cfg(test)] mod tests { use super::EncString; - use crate::{derive_symmetric_key, Decrypted, KeyDecryptable, KeyEncryptable}; + use crate::{decrypted::DecryptedString, derive_symmetric_key, KeyDecryptable, KeyEncryptable}; #[test] fn test_enc_string_roundtrip() { @@ -278,7 +279,7 @@ mod tests { let test_string = "encrypted_test_string"; let cipher = test_string.to_owned().encrypt_with_key(&key).unwrap(); - let decrypted_str: Decrypted = cipher.decrypt_with_key(&key).unwrap(); + let decrypted_str: DecryptedString = cipher.decrypt_with_key(&key).unwrap(); assert_eq!(decrypted_str.expose(), test_string); } diff --git a/crates/bitwarden-crypto/src/keys/device_key.rs b/crates/bitwarden-crypto/src/keys/device_key.rs index 6588944fe..413194066 100644 --- a/crates/bitwarden-crypto/src/keys/device_key.rs +++ b/crates/bitwarden-crypto/src/keys/device_key.rs @@ -1,6 +1,6 @@ use crate::{ - error::Result, AsymmetricCryptoKey, AsymmetricEncString, EncString, KeyDecryptable, - KeyEncryptable, SymmetricCryptoKey, UserKey, + decrypted::DecryptedVec, error::Result, AsymmetricCryptoKey, AsymmetricEncString, EncString, + KeyDecryptable, KeyEncryptable, SymmetricCryptoKey, UserKey, }; /// Device Key @@ -60,8 +60,10 @@ impl DeviceKey { protected_device_private_key: EncString, protected_user_key: AsymmetricEncString, ) -> Result { - let device_private_key: Vec = protected_device_private_key.decrypt_with_key(&self.0)?; - let device_private_key = AsymmetricCryptoKey::from_der(device_private_key.as_slice())?; + let device_private_key: DecryptedVec = + protected_device_private_key.decrypt_with_key(&self.0)?; + let device_private_key = + AsymmetricCryptoKey::from_der(device_private_key.expose().as_slice())?; let dec: Vec = protected_user_key.decrypt_with_key(&device_private_key)?; let user_key: SymmetricCryptoKey = dec.as_slice().try_into()?; diff --git a/crates/bitwarden-crypto/src/keys/master_key.rs b/crates/bitwarden-crypto/src/keys/master_key.rs index bdd626a1a..7e60bc24b 100644 --- a/crates/bitwarden-crypto/src/keys/master_key.rs +++ b/crates/bitwarden-crypto/src/keys/master_key.rs @@ -7,6 +7,7 @@ use serde::{Deserialize, Serialize}; use sha2::Digest; use crate::{ + decrypted::DecryptedVec, util::{self, hkdf_expand}, EncString, KeyDecryptable, Result, SymmetricCryptoKey, UserKey, }; @@ -59,8 +60,8 @@ impl MasterKey { pub fn decrypt_user_key(&self, user_key: EncString) -> Result { let stretched_key = stretch_master_key(self)?; - let dec: Vec = user_key.decrypt_with_key(&stretched_key)?; - SymmetricCryptoKey::try_from(dec.as_slice()) + let dec: DecryptedVec = user_key.decrypt_with_key(&stretched_key)?; + SymmetricCryptoKey::try_from(dec.expose().as_slice()) } pub fn encrypt_user_key(&self, user_key: &SymmetricCryptoKey) -> Result { From 97ecc2c91a5ce206326e98b26b5e0713b906768e Mon Sep 17 00:00:00 2001 From: Hinton Date: Thu, 25 Jan 2024 10:40:31 +0100 Subject: [PATCH 04/22] Format --- crates/bitwarden-crypto/src/decrypted.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/crates/bitwarden-crypto/src/decrypted.rs b/crates/bitwarden-crypto/src/decrypted.rs index 140c99a37..f0bb5455d 100644 --- a/crates/bitwarden-crypto/src/decrypted.rs +++ b/crates/bitwarden-crypto/src/decrypted.rs @@ -4,7 +4,9 @@ use crate::CryptoError; /// A wrapper for decrypted values. /// -/// Implements `Zeroize` and `ZeroizeOnDrop` to ensure that the value is zeroized on drop. Please be careful if cloning or copying the inner value using `expose` since any copy will not be zeroized. +/// Implements `Zeroize` and `ZeroizeOnDrop` to ensure that the value is zeroized on drop. Please be +/// careful if cloning or copying the inner value using `expose` since any copy will not be +/// zeroized. #[derive(Zeroize, ZeroizeOnDrop)] pub struct Decrypted { value: V, @@ -15,13 +17,15 @@ impl Decrypted { Self { value } } - /// Expose the inner value. By exposing the inner value, you take responsibility for ensuring that any copy of the value is zeroized. + /// Expose the inner value. By exposing the inner value, you take responsibility for ensuring + /// that any copy of the value is zeroized. pub fn expose(&self) -> &V { &self.value } } -/// Helper to convert a `Decrypted>` to a `Decrypted`, care is taken to ensure any intermediate copies are zeroed to avoid leaking sensitive data. +/// Helper to convert a `Decrypted>` to a `Decrypted`, care is taken to ensure any +/// intermediate copies are zeroed to avoid leaking sensitive data. impl TryFrom for DecryptedString { type Error = CryptoError; From 449466a6f1afcc6b324f03762b399ebfe0ecad73 Mon Sep 17 00:00:00 2001 From: Hinton Date: Thu, 25 Jan 2024 17:00:38 +0100 Subject: [PATCH 05/22] Add Decrypted to asymmetric --- .../bitwarden-crypto/src/enc_string/asymmetric.rs | 14 ++++++++------ crates/bitwarden-crypto/src/keys/device_key.rs | 4 ++-- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/crates/bitwarden-crypto/src/enc_string/asymmetric.rs b/crates/bitwarden-crypto/src/enc_string/asymmetric.rs index ea969f4f9..0d4d45419 100644 --- a/crates/bitwarden-crypto/src/enc_string/asymmetric.rs +++ b/crates/bitwarden-crypto/src/enc_string/asymmetric.rs @@ -6,6 +6,7 @@ use serde::Deserialize; use super::{from_b64_vec, split_enc_string}; use crate::{ + decrypted::{DecryptedString, DecryptedVec}, error::{CryptoError, EncStringParseError, Result}, rsa::encrypt_rsa2048_oaep_sha1, AsymmetricCryptoKey, AsymmetricEncryptable, KeyDecryptable, @@ -160,8 +161,8 @@ impl AsymmetricEncString { } } -impl KeyDecryptable> for AsymmetricEncString { - fn decrypt_with_key(&self, key: &AsymmetricCryptoKey) -> Result> { +impl KeyDecryptable for AsymmetricEncString { + fn decrypt_with_key(&self, key: &AsymmetricCryptoKey) -> Result { use AsymmetricEncString::*; match self { Rsa2048_OaepSha256_B64 { data } => key.key.decrypt(Oaep::new::(), data), @@ -175,14 +176,15 @@ impl KeyDecryptable> for AsymmetricEncString { key.key.decrypt(Oaep::new::(), data) } } + .map(DecryptedVec::new) .map_err(|_| CryptoError::KeyDecrypt) } } -impl KeyDecryptable for AsymmetricEncString { - fn decrypt_with_key(&self, key: &AsymmetricCryptoKey) -> Result { - let dec: Vec = self.decrypt_with_key(key)?; - String::from_utf8(dec).map_err(|_| CryptoError::InvalidUtf8String) +impl KeyDecryptable for AsymmetricEncString { + fn decrypt_with_key(&self, key: &AsymmetricCryptoKey) -> Result { + let dec: DecryptedVec = self.decrypt_with_key(key)?; + dec.try_into() } } diff --git a/crates/bitwarden-crypto/src/keys/device_key.rs b/crates/bitwarden-crypto/src/keys/device_key.rs index 413194066..b83fb88ca 100644 --- a/crates/bitwarden-crypto/src/keys/device_key.rs +++ b/crates/bitwarden-crypto/src/keys/device_key.rs @@ -65,8 +65,8 @@ impl DeviceKey { let device_private_key = AsymmetricCryptoKey::from_der(device_private_key.expose().as_slice())?; - let dec: Vec = protected_user_key.decrypt_with_key(&device_private_key)?; - let user_key: SymmetricCryptoKey = dec.as_slice().try_into()?; + let dec: DecryptedVec = protected_user_key.decrypt_with_key(&device_private_key)?; + let user_key: SymmetricCryptoKey = dec.expose().as_slice().try_into()?; Ok(UserKey(user_key)) } From ed4d92b68bd5377814b3ff0b32f5785513d5c4f9 Mon Sep 17 00:00:00 2001 From: Hinton Date: Thu, 25 Jan 2024 19:15:03 +0100 Subject: [PATCH 06/22] Use Decrypt everywhere --- crates/bitwarden-crypto/src/decrypted.rs | 71 ++++++++++++++++++- .../src/enc_string/asymmetric.rs | 14 ++-- .../src/keys/asymmetric_crypto_key.rs | 7 +- crates/bitwarden-crypto/src/keys/mod.rs | 2 +- crates/bitwarden-crypto/src/lib.rs | 2 +- crates/bitwarden/src/auth/auth_request.rs | 6 +- .../bitwarden/src/auth/login/access_token.rs | 7 +- .../src/client/encryption_settings.rs | 12 ++-- crates/bitwarden/src/mobile/crypto.rs | 6 +- .../src/mobile/vault/client_attachments.rs | 1 + .../src/mobile/vault/client_sends.rs | 7 +- .../projects/project_response.rs | 6 +- .../src/secrets_manager/secrets/list.rs | 6 +- .../secrets/secret_response.rs | 14 ++-- crates/bitwarden/src/secrets_manager/state.rs | 6 +- crates/bitwarden/src/uniffi_support.rs | 16 ++++- .../bitwarden/src/vault/cipher/attachment.rs | 14 ++-- crates/bitwarden/src/vault/cipher/card.rs | 16 ++--- crates/bitwarden/src/vault/cipher/cipher.rs | 38 +++++----- crates/bitwarden/src/vault/cipher/field.rs | 8 +-- crates/bitwarden/src/vault/cipher/identity.rs | 40 +++++------ crates/bitwarden/src/vault/cipher/login.rs | 12 ++-- crates/bitwarden/src/vault/collection.rs | 5 +- crates/bitwarden/src/vault/folder.rs | 6 +- .../bitwarden/src/vault/password_history.rs | 5 +- crates/bitwarden/src/vault/send.rs | 27 ++++--- 26 files changed, 216 insertions(+), 138 deletions(-) diff --git a/crates/bitwarden-crypto/src/decrypted.rs b/crates/bitwarden-crypto/src/decrypted.rs index f0bb5455d..093267fcd 100644 --- a/crates/bitwarden-crypto/src/decrypted.rs +++ b/crates/bitwarden-crypto/src/decrypted.rs @@ -1,13 +1,20 @@ +use std::{ + borrow::Cow, + fmt::{self, Formatter}, +}; + +use schemars::JsonSchema; +use serde::{Deserialize, Deserializer, Serialize, Serializer}; use zeroize::{Zeroize, ZeroizeOnDrop}; -use crate::CryptoError; +use crate::{CryptoError, CryptoKey, KeyEncryptable}; /// A wrapper for decrypted values. /// /// Implements `Zeroize` and `ZeroizeOnDrop` to ensure that the value is zeroized on drop. Please be /// careful if cloning or copying the inner value using `expose` since any copy will not be /// zeroized. -#[derive(Zeroize, ZeroizeOnDrop)] +#[derive(Zeroize, ZeroizeOnDrop, PartialEq, Clone)] pub struct Decrypted { value: V, } @@ -37,5 +44,65 @@ impl TryFrom for DecryptedString { } } +impl Default for Decrypted { + fn default() -> Self { + Self::new(V::default()) + } +} + +impl fmt::Debug for Decrypted { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Decrypted") + .field("value", &"********") + .finish() + } +} + +/// Unfortunately once we serialize a `DecryptedString` we can't control the future memory. +impl Serialize for Decrypted { + fn serialize(&self, serializer: S) -> Result { + self.value.serialize(serializer) + } +} + +impl<'de, V: Zeroize + Deserialize<'de>> Deserialize<'de> for Decrypted { + fn deserialize>(deserializer: D) -> Result { + Ok(Self::new(V::deserialize(deserializer)?)) + } +} + +/// Transparently expose the inner value for serialization +impl JsonSchema for Decrypted { + fn schema_name() -> String { + V::schema_name() + } + + fn schema_id() -> Cow<'static, str> { + V::schema_id() + } + + fn json_schema(gen: &mut schemars::gen::SchemaGenerator) -> schemars::schema::Schema { + V::json_schema(gen) + } +} + +/** +impl> KeyEncryptable + for Decrypted +{ + fn encrypt_with_key(self, key: &K) -> Result { + self.value.clone().encrypt_with_key(key) + } +} +**/ + +impl + Zeroize + Clone, Key: CryptoKey, Output> + KeyEncryptable for Decrypted +{ + fn encrypt_with_key(self, key: &Key) -> Result { + self.value.clone().encrypt_with_key(key) + } +} + pub type DecryptedVec = Decrypted>; pub type DecryptedString = Decrypted; diff --git a/crates/bitwarden-crypto/src/enc_string/asymmetric.rs b/crates/bitwarden-crypto/src/enc_string/asymmetric.rs index 0d4d45419..322d2c12c 100644 --- a/crates/bitwarden-crypto/src/enc_string/asymmetric.rs +++ b/crates/bitwarden-crypto/src/enc_string/asymmetric.rs @@ -202,6 +202,8 @@ impl schemars::JsonSchema for AsymmetricEncString { #[cfg(test)] mod tests { + use crate::DecryptedString; + use super::{AsymmetricCryptoKey, AsymmetricEncString, KeyDecryptable}; const RSA_PRIVATE_KEY: &str = "-----BEGIN PRIVATE KEY----- @@ -241,8 +243,8 @@ XKZBokBGnjFnTnKcs7nv/O8= assert_eq!(enc_string.enc_type(), 3); - let res: String = enc_string.decrypt_with_key(&private_key).unwrap(); - assert_eq!(res, "EncryptMe!"); + let res: DecryptedString = enc_string.decrypt_with_key(&private_key).unwrap(); + assert_eq!(res.expose(), "EncryptMe!"); } #[test] @@ -253,8 +255,8 @@ XKZBokBGnjFnTnKcs7nv/O8= assert_eq!(enc_string.enc_type(), 4); - let res: String = enc_string.decrypt_with_key(&private_key).unwrap(); - assert_eq!(res, "EncryptMe!"); + let res: DecryptedString = enc_string.decrypt_with_key(&private_key).unwrap(); + assert_eq!(res.expose(), "EncryptMe!"); } #[test] @@ -265,8 +267,8 @@ XKZBokBGnjFnTnKcs7nv/O8= assert_eq!(enc_string.enc_type(), 6); - let res: String = enc_string.decrypt_with_key(&private_key).unwrap(); - assert_eq!(res, "EncryptMe!"); + let res: DecryptedString = enc_string.decrypt_with_key(&private_key).unwrap(); + assert_eq!(res.expose(), "EncryptMe!"); } #[test] diff --git a/crates/bitwarden-crypto/src/keys/asymmetric_crypto_key.rs b/crates/bitwarden-crypto/src/keys/asymmetric_crypto_key.rs index 560e7e4cc..25110f359 100644 --- a/crates/bitwarden-crypto/src/keys/asymmetric_crypto_key.rs +++ b/crates/bitwarden-crypto/src/keys/asymmetric_crypto_key.rs @@ -103,7 +103,8 @@ mod tests { use base64::{engine::general_purpose::STANDARD, Engine}; use crate::{ - AsymmetricCryptoKey, AsymmetricEncString, AsymmetricPublicCryptoKey, KeyDecryptable, + AsymmetricCryptoKey, AsymmetricEncString, AsymmetricPublicCryptoKey, DecryptedString, + KeyDecryptable, }; #[test] @@ -201,8 +202,8 @@ DnqOsltgPomWZ7xVfMkm9niL2OA= let encrypted = AsymmetricEncString::encrypt_rsa2048_oaep_sha1(plaintext.as_bytes(), &public_key) .unwrap(); - let decrypted: String = encrypted.decrypt_with_key(&private_key).unwrap(); + let decrypted: DecryptedString = encrypted.decrypt_with_key(&private_key).unwrap(); - assert_eq!(plaintext, decrypted); + assert_eq!(plaintext, decrypted.expose()); } } diff --git a/crates/bitwarden-crypto/src/keys/mod.rs b/crates/bitwarden-crypto/src/keys/mod.rs index 285e58b55..c876c33ed 100644 --- a/crates/bitwarden-crypto/src/keys/mod.rs +++ b/crates/bitwarden-crypto/src/keys/mod.rs @@ -1,5 +1,5 @@ mod key_encryptable; -pub use key_encryptable::{KeyDecryptable, KeyEncryptable}; +pub use key_encryptable::{CryptoKey, KeyDecryptable, KeyEncryptable}; mod master_key; pub use master_key::{HashPurpose, Kdf, MasterKey}; mod shareable_key; diff --git a/crates/bitwarden-crypto/src/lib.rs b/crates/bitwarden-crypto/src/lib.rs index 2d4b5aa9b..94cf7a9da 100644 --- a/crates/bitwarden-crypto/src/lib.rs +++ b/crates/bitwarden-crypto/src/lib.rs @@ -41,7 +41,7 @@ mod wordlist; pub use util::pbkdf2; pub use wordlist::EFF_LONG_WORD_LIST; mod decrypted; -pub use decrypted::Decrypted; +pub use decrypted::{Decrypted, DecryptedString, DecryptedVec}; #[cfg(feature = "mobile")] uniffi::setup_scaffolding!(); diff --git a/crates/bitwarden/src/auth/auth_request.rs b/crates/bitwarden/src/auth/auth_request.rs index facdc8f82..a65c5cf9c 100644 --- a/crates/bitwarden/src/auth/auth_request.rs +++ b/crates/bitwarden/src/auth/auth_request.rs @@ -57,10 +57,12 @@ pub(crate) fn auth_request_decrypt_user_key( private_key: String, user_key: AsymmetricEncString, ) -> Result { + use bitwarden_crypto::DecryptedString; + let key = AsymmetricCryptoKey::from_der(&STANDARD.decode(private_key)?)?; - let key: String = user_key.decrypt_with_key(&key)?; + let key: DecryptedString = user_key.decrypt_with_key(&key)?; - Ok(key.parse()?) + Ok(key.expose().parse()?) } /// Approve an auth request. diff --git a/crates/bitwarden/src/auth/login/access_token.rs b/crates/bitwarden/src/auth/login/access_token.rs index e4cce33e4..bd4d9acb1 100644 --- a/crates/bitwarden/src/auth/login/access_token.rs +++ b/crates/bitwarden/src/auth/login/access_token.rs @@ -1,7 +1,7 @@ use std::path::{Path, PathBuf}; use base64::{engine::general_purpose::STANDARD, Engine}; -use bitwarden_crypto::{EncString, KeyDecryptable, SymmetricCryptoKey}; +use bitwarden_crypto::{DecryptedVec, EncString, KeyDecryptable, SymmetricCryptoKey}; use chrono::Utc; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -53,7 +53,8 @@ pub(crate) async fn login_access_token( // Extract the encrypted payload and use the access token encryption key to decrypt it let payload: EncString = r.encrypted_payload.parse()?; - let decrypted_payload: Vec = payload.decrypt_with_key(&access_token.encryption_key)?; + let decrypted_payload: DecryptedVec = + payload.decrypt_with_key(&access_token.encryption_key)?; // Once decrypted, we have to JSON decode to extract the organization encryption key #[derive(serde::Deserialize)] @@ -62,7 +63,7 @@ pub(crate) async fn login_access_token( encryption_key: String, } - let payload: Payload = serde_json::from_slice(&decrypted_payload)?; + let payload: Payload = serde_json::from_slice(decrypted_payload.expose())?; let encryption_key = STANDARD.decode(payload.encryption_key.clone())?; let encryption_key = SymmetricCryptoKey::try_from(encryption_key.as_slice())?; diff --git a/crates/bitwarden/src/client/encryption_settings.rs b/crates/bitwarden/src/client/encryption_settings.rs index 8b4c5197e..20a914b52 100644 --- a/crates/bitwarden/src/client/encryption_settings.rs +++ b/crates/bitwarden/src/client/encryption_settings.rs @@ -54,11 +54,11 @@ impl EncryptionSettings { user_key: SymmetricCryptoKey, private_key: EncString, ) -> Result { - use bitwarden_crypto::KeyDecryptable; + use bitwarden_crypto::{DecryptedVec, KeyDecryptable}; let private_key = { - let dec: Vec = private_key.decrypt_with_key(&user_key)?; - Some(AsymmetricCryptoKey::from_der(&dec)?) + let dec: DecryptedVec = private_key.decrypt_with_key(&user_key)?; + Some(AsymmetricCryptoKey::from_der(dec.expose())?) }; Ok(EncryptionSettings { @@ -83,7 +83,7 @@ impl EncryptionSettings { &mut self, org_enc_keys: Vec<(Uuid, AsymmetricEncString)>, ) -> Result<&mut Self> { - use bitwarden_crypto::KeyDecryptable; + use bitwarden_crypto::{DecryptedVec, KeyDecryptable}; use crate::error::Error; @@ -95,9 +95,9 @@ impl EncryptionSettings { // Decrypt the org keys with the private key for (org_id, org_enc_key) in org_enc_keys { - let dec: Vec = org_enc_key.decrypt_with_key(private_key)?; + let dec: DecryptedVec = org_enc_key.decrypt_with_key(private_key)?; - let org_key = SymmetricCryptoKey::try_from(dec.as_slice())?; + let org_key = SymmetricCryptoKey::try_from(dec.expose().as_slice())?; self.org_keys.insert(org_id, org_key); } diff --git a/crates/bitwarden/src/mobile/crypto.rs b/crates/bitwarden/src/mobile/crypto.rs index bd71af2be..d2c012e49 100644 --- a/crates/bitwarden/src/mobile/crypto.rs +++ b/crates/bitwarden/src/mobile/crypto.rs @@ -158,18 +158,20 @@ pub fn derive_pin_key(client: &mut Client, pin: String) -> Result Result { + use bitwarden_crypto::DecryptedString; + let user_key = client .get_encryption_settings()? .get_key(&None) .ok_or(Error::VaultLocked)?; - let pin: String = encrypted_pin.decrypt_with_key(user_key)?; + let pin: DecryptedString = encrypted_pin.decrypt_with_key(user_key)?; let login_method = client .login_method .as_ref() .ok_or(Error::NotAuthenticated)?; - derive_pin_protected_user_key(&pin, login_method, user_key) + derive_pin_protected_user_key(pin.expose(), login_method, user_key) } #[cfg(feature = "internal")] diff --git a/crates/bitwarden/src/mobile/vault/client_attachments.rs b/crates/bitwarden/src/mobile/vault/client_attachments.rs index c436f10fd..3912f611e 100644 --- a/crates/bitwarden/src/mobile/vault/client_attachments.rs +++ b/crates/bitwarden/src/mobile/vault/client_attachments.rs @@ -65,6 +65,7 @@ impl<'a> ClientAttachments<'a> { } .decrypt_with_key(key) .map_err(Error::Crypto) + .map(|s| s.expose().to_owned()) } pub async fn decrypt_file( &self, diff --git a/crates/bitwarden/src/mobile/vault/client_sends.rs b/crates/bitwarden/src/mobile/vault/client_sends.rs index 45d9a7825..131c9285f 100644 --- a/crates/bitwarden/src/mobile/vault/client_sends.rs +++ b/crates/bitwarden/src/mobile/vault/client_sends.rs @@ -1,6 +1,8 @@ use std::path::Path; -use bitwarden_crypto::{Decryptable, EncString, Encryptable, KeyDecryptable, KeyEncryptable}; +use bitwarden_crypto::{ + Decryptable, DecryptedVec, EncString, Encryptable, KeyDecryptable, KeyEncryptable, +}; use super::client_vault::ClientVault; use crate::{ @@ -48,7 +50,8 @@ impl<'a> ClientSends<'a> { let key = Send::get_key(&send.key, key)?; let buf = EncString::from_buffer(encrypted_buffer)?; - Ok(buf.decrypt_with_key(&key)?) + let dec: DecryptedVec = buf.decrypt_with_key(&key)?; + Ok(dec.expose().to_owned()) } pub async fn encrypt(&self, send_view: SendView) -> Result { diff --git a/crates/bitwarden/src/secrets_manager/projects/project_response.rs b/crates/bitwarden/src/secrets_manager/projects/project_response.rs index 1e6f6a158..fdedc6942 100644 --- a/crates/bitwarden/src/secrets_manager/projects/project_response.rs +++ b/crates/bitwarden/src/secrets_manager/projects/project_response.rs @@ -1,5 +1,5 @@ use bitwarden_api_api::models::ProjectResponseModel; -use bitwarden_crypto::{Decryptable, EncString}; +use bitwarden_crypto::{Decryptable, DecryptedString, EncString}; use chrono::{DateTime, Utc}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -27,7 +27,7 @@ impl ProjectResponse { ) -> Result { let organization_id = response.organization_id.ok_or(Error::MissingFields)?; - let name = response + let name: DecryptedString = response .name .ok_or(Error::MissingFields)? .parse::()? @@ -36,7 +36,7 @@ impl ProjectResponse { Ok(ProjectResponse { id: response.id.ok_or(Error::MissingFields)?, organization_id, - name, + name: name.expose().to_owned(), creation_date: response .creation_date diff --git a/crates/bitwarden/src/secrets_manager/secrets/list.rs b/crates/bitwarden/src/secrets_manager/secrets/list.rs index 7fa46d164..d18050d77 100644 --- a/crates/bitwarden/src/secrets_manager/secrets/list.rs +++ b/crates/bitwarden/src/secrets_manager/secrets/list.rs @@ -1,7 +1,7 @@ use bitwarden_api_api::models::{ SecretWithProjectsListResponseModel, SecretsWithProjectsInnerSecret, }; -use bitwarden_crypto::{Decryptable, EncString}; +use bitwarden_crypto::{Decryptable, DecryptedString, EncString}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use uuid::Uuid; @@ -95,7 +95,7 @@ impl SecretIdentifierResponse { ) -> Result { let organization_id = response.organization_id.ok_or(Error::MissingFields)?; - let key = response + let key: DecryptedString = response .key .ok_or(Error::MissingFields)? .parse::()? @@ -104,7 +104,7 @@ impl SecretIdentifierResponse { Ok(SecretIdentifierResponse { id: response.id.ok_or(Error::MissingFields)?, organization_id, - key, + key: key.expose().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 fe1a4d342..534e0abba 100644 --- a/crates/bitwarden/src/secrets_manager/secrets/secret_response.rs +++ b/crates/bitwarden/src/secrets_manager/secrets/secret_response.rs @@ -1,7 +1,7 @@ use bitwarden_api_api::models::{ BaseSecretResponseModel, BaseSecretResponseModelListResponseModel, SecretResponseModel, }; -use bitwarden_crypto::{Decryptable, EncString}; +use bitwarden_crypto::{Decryptable, DecryptedString, EncString}; use chrono::{DateTime, Utc}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -51,17 +51,17 @@ impl SecretResponse { ) -> Result { let org_id = response.organization_id; - let key = response + let key: DecryptedString = response .key .ok_or(Error::MissingFields)? .parse::()? .decrypt(enc, &org_id)?; - let value = response + let value: DecryptedString = response .value .ok_or(Error::MissingFields)? .parse::()? .decrypt(enc, &org_id)?; - let note = response + let note: DecryptedString = response .note .ok_or(Error::MissingFields)? .parse::()? @@ -76,9 +76,9 @@ impl SecretResponse { id: response.id.ok_or(Error::MissingFields)?, organization_id: org_id.ok_or(Error::MissingFields)?, project_id: project, - key, - value, - note, + key: key.expose().to_owned(), + value: value.expose().to_owned(), + note: note.expose().to_owned(), creation_date: response .creation_date diff --git a/crates/bitwarden/src/secrets_manager/state.rs b/crates/bitwarden/src/secrets_manager/state.rs index 4efa4403b..f631d421c 100644 --- a/crates/bitwarden/src/secrets_manager/state.rs +++ b/crates/bitwarden/src/secrets_manager/state.rs @@ -1,6 +1,6 @@ use std::{fmt::Debug, path::Path}; -use bitwarden_crypto::{EncString, KeyDecryptable, KeyEncryptable}; +use bitwarden_crypto::{DecryptedString, EncString, KeyDecryptable, KeyEncryptable}; use serde::{Deserialize, Serialize}; use crate::{ @@ -32,8 +32,8 @@ pub fn get(state_file: &Path, access_token: &AccessToken) -> Result let file_content = std::fs::read_to_string(state_file)?; let encrypted_state: EncString = file_content.parse()?; - let decrypted_state: String = encrypted_state.decrypt_with_key(&access_token.encryption_key)?; - let client_state: ClientState = serde_json::from_str(&decrypted_state)?; + let decrypted_state: DecryptedString = encrypted_state.decrypt_with_key(&access_token.encryption_key)?; + let client_state: ClientState = serde_json::from_str(decrypted_state.expose())?; if client_state.version != STATE_VERSION { return Err(Error::InvalidStateFileVersion); diff --git a/crates/bitwarden/src/uniffi_support.rs b/crates/bitwarden/src/uniffi_support.rs index 7da53c8b5..7293346ce 100644 --- a/crates/bitwarden/src/uniffi_support.rs +++ b/crates/bitwarden/src/uniffi_support.rs @@ -1,6 +1,6 @@ use std::{num::NonZeroU32, str::FromStr}; -use bitwarden_crypto::{AsymmetricEncString, EncString}; +use bitwarden_crypto::{AsymmetricEncString, DecryptedString, EncString}; use uuid::Uuid; use crate::UniffiCustomTypeConverter; @@ -22,6 +22,20 @@ impl UniffiCustomTypeConverter for AsymmetricEncString { } } +uniffi::custom_type!(DecryptedString, String); + +impl UniffiCustomTypeConverter for DecryptedString { + type Builtin = String; + + fn into_custom(val: Self::Builtin) -> uniffi::Result { + Ok(Self::new(val)) + } + + fn from_custom(obj: Self) -> Self::Builtin { + (*obj.expose()).to_owned() + } +} + type DateTime = chrono::DateTime; uniffi::custom_type!(DateTime, std::time::SystemTime); diff --git a/crates/bitwarden/src/vault/cipher/attachment.rs b/crates/bitwarden/src/vault/cipher/attachment.rs index 40f32dcfb..0f165d20a 100644 --- a/crates/bitwarden/src/vault/cipher/attachment.rs +++ b/crates/bitwarden/src/vault/cipher/attachment.rs @@ -1,6 +1,4 @@ -use bitwarden_crypto::{ - CryptoError, EncString, KeyDecryptable, KeyEncryptable, SymmetricCryptoKey, -}; +use bitwarden_crypto::{CryptoError, DecryptedString, DecryptedVec, EncString, KeyDecryptable, KeyEncryptable, SymmetricCryptoKey}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -28,7 +26,7 @@ pub struct AttachmentView { pub url: Option, pub size: Option, pub size_name: Option, - pub file_name: Option, + pub file_name: Option, pub key: Option, } @@ -75,18 +73,18 @@ impl<'a> KeyEncryptable for Attachm } } -impl KeyDecryptable> for AttachmentFile { - fn decrypt_with_key(&self, key: &SymmetricCryptoKey) -> Result, CryptoError> { +impl KeyDecryptable for AttachmentFile { + fn decrypt_with_key(&self, key: &SymmetricCryptoKey) -> Result { let ciphers_key = Cipher::get_cipher_key(key, &self.cipher.key)?; let ciphers_key = ciphers_key.as_ref().unwrap_or(key); - let attachment_key: Vec = self + let attachment_key: DecryptedVec = self .attachment .key .as_ref() .ok_or(CryptoError::MissingKey)? .decrypt_with_key(ciphers_key)?; - let attachment_key = SymmetricCryptoKey::try_from(attachment_key.as_slice())?; + let attachment_key = SymmetricCryptoKey::try_from(attachment_key.expose().as_slice())?; self.contents.decrypt_with_key(&attachment_key) } diff --git a/crates/bitwarden/src/vault/cipher/card.rs b/crates/bitwarden/src/vault/cipher/card.rs index cd61a17d8..b80e9f06f 100644 --- a/crates/bitwarden/src/vault/cipher/card.rs +++ b/crates/bitwarden/src/vault/cipher/card.rs @@ -1,7 +1,5 @@ use bitwarden_api_api::models::CipherCardModel; -use bitwarden_crypto::{ - CryptoError, EncString, KeyDecryptable, KeyEncryptable, SymmetricCryptoKey, -}; +use bitwarden_crypto::{CryptoError, DecryptedString, EncString, KeyDecryptable, KeyEncryptable, SymmetricCryptoKey}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -23,12 +21,12 @@ pub struct Card { #[serde(rename_all = "camelCase", deny_unknown_fields)] #[cfg_attr(feature = "mobile", derive(uniffi::Record))] pub struct CardView { - pub cardholder_name: Option, - pub exp_month: Option, - pub exp_year: Option, - pub code: Option, - pub brand: Option, - pub number: Option, + pub cardholder_name: Option, + pub exp_month: Option, + pub exp_year: Option, + pub code: Option, + pub brand: Option, + pub number: Option, } impl KeyEncryptable for CardView { diff --git a/crates/bitwarden/src/vault/cipher/cipher.rs b/crates/bitwarden/src/vault/cipher/cipher.rs index 34f01ca60..586d43429 100644 --- a/crates/bitwarden/src/vault/cipher/cipher.rs +++ b/crates/bitwarden/src/vault/cipher/cipher.rs @@ -1,8 +1,5 @@ use bitwarden_api_api::models::CipherDetailsResponseModel; -use bitwarden_crypto::{ - CryptoError, EncString, KeyContainer, KeyDecryptable, KeyEncryptable, LocateKey, - SymmetricCryptoKey, -}; +use bitwarden_crypto::{CryptoError, DecryptedString, DecryptedVec, EncString, KeyContainer, KeyDecryptable, KeyEncryptable, LocateKey, SymmetricCryptoKey}; use chrono::{DateTime, Utc}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -86,8 +83,8 @@ pub struct CipherView { pub key: Option, - pub name: String, - pub notes: Option, + pub name: DecryptedString, + pub notes: Option, pub r#type: CipherType, pub login: Option, @@ -120,7 +117,7 @@ pub struct CipherListView { pub folder_id: Option, pub collection_ids: Vec, - pub name: String, + pub name: DecryptedString, pub sub_title: String, pub r#type: CipherType, @@ -218,8 +215,8 @@ impl Cipher { ciphers_key .as_ref() .map(|k| { - let key: Vec = k.decrypt_with_key(key)?; - SymmetricCryptoKey::try_from(key.as_slice()) + let key: DecryptedVec = k.decrypt_with_key(key)?; + SymmetricCryptoKey::try_from(key.expose().as_slice()) }) .transpose() } @@ -230,7 +227,7 @@ impl Cipher { let Some(login) = &self.login else { return Ok(String::new()); }; - login.username.decrypt_with_key(key)?.unwrap_or_default() + login.username.decrypt_with_key(key)?.map(|s: DecryptedString| s.expose().to_owned()).unwrap_or_default() } CipherType::SecureNote => String::new(), CipherType::Card => { @@ -240,25 +237,26 @@ impl Cipher { let mut sub_title = String::new(); if let Some(brand) = &card.brand { - let brand: String = brand.decrypt_with_key(key)?; - sub_title.push_str(&brand); + let brand: DecryptedString = brand.decrypt_with_key(key)?; + sub_title.push_str(brand.expose()); } if let Some(number) = &card.number { - let number: String = number.decrypt_with_key(key)?; - let number_len = number.len(); + let number: DecryptedString = number.decrypt_with_key(key)?; + let n = number.expose(); + let number_len = n.len(); if number_len > 4 { if !sub_title.is_empty() { sub_title.push_str(", "); } // On AMEX cards we show 5 digits instead of 4 - let digit_count = match &number[0..2] { + let digit_count = match &n[0..2] { "34" | "37" => 5, _ => 4, }; - sub_title.push_str(&number[(number_len - digit_count)..]); + sub_title.push_str(&n[(number_len - digit_count)..]); } } @@ -271,16 +269,16 @@ impl Cipher { let mut sub_title = String::new(); if let Some(first_name) = &identity.first_name { - let first_name: String = first_name.decrypt_with_key(key)?; - sub_title.push_str(&first_name); + let first_name: DecryptedString = first_name.decrypt_with_key(key)?; + sub_title.push_str(first_name.expose()); } if let Some(last_name) = &identity.last_name { if !sub_title.is_empty() { sub_title.push(' '); } - let last_name: String = last_name.decrypt_with_key(key)?; - sub_title.push_str(&last_name); + let last_name: DecryptedString = last_name.decrypt_with_key(key)?; + sub_title.push_str(last_name.expose()); } sub_title diff --git a/crates/bitwarden/src/vault/cipher/field.rs b/crates/bitwarden/src/vault/cipher/field.rs index db896474f..7bcdf6acf 100644 --- a/crates/bitwarden/src/vault/cipher/field.rs +++ b/crates/bitwarden/src/vault/cipher/field.rs @@ -1,7 +1,5 @@ use bitwarden_api_api::models::CipherFieldModel; -use bitwarden_crypto::{ - CryptoError, EncString, KeyDecryptable, KeyEncryptable, SymmetricCryptoKey, -}; +use bitwarden_crypto::{CryptoError, DecryptedString, EncString, KeyDecryptable, KeyEncryptable, SymmetricCryptoKey}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use serde_repr::{Deserialize_repr, Serialize_repr}; @@ -34,8 +32,8 @@ pub struct Field { #[serde(rename_all = "camelCase", deny_unknown_fields)] #[cfg_attr(feature = "mobile", derive(uniffi::Record))] pub struct FieldView { - name: Option, - value: Option, + name: Option, + value: Option, r#type: FieldType, linked_id: Option, diff --git a/crates/bitwarden/src/vault/cipher/identity.rs b/crates/bitwarden/src/vault/cipher/identity.rs index f59166eec..a30fc9641 100644 --- a/crates/bitwarden/src/vault/cipher/identity.rs +++ b/crates/bitwarden/src/vault/cipher/identity.rs @@ -1,7 +1,5 @@ use bitwarden_api_api::models::CipherIdentityModel; -use bitwarden_crypto::{ - CryptoError, EncString, KeyDecryptable, KeyEncryptable, SymmetricCryptoKey, -}; +use bitwarden_crypto::{CryptoError, DecryptedString, EncString, KeyDecryptable, KeyEncryptable, SymmetricCryptoKey}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -35,24 +33,24 @@ pub struct Identity { #[serde(rename_all = "camelCase", deny_unknown_fields)] #[cfg_attr(feature = "mobile", derive(uniffi::Record))] pub struct IdentityView { - pub title: Option, - pub first_name: Option, - pub middle_name: Option, - pub last_name: Option, - pub address1: Option, - pub address2: Option, - pub address3: Option, - pub city: Option, - pub state: Option, - pub postal_code: Option, - pub country: Option, - pub company: Option, - pub email: Option, - pub phone: Option, - pub ssn: Option, - pub username: Option, - pub passport_number: Option, - pub license_number: Option, + pub title: Option, + pub first_name: Option, + pub middle_name: Option, + pub last_name: Option, + pub address1: Option, + pub address2: Option, + pub address3: Option, + pub city: Option, + pub state: Option, + pub postal_code: Option, + pub country: Option, + pub company: Option, + pub email: Option, + pub phone: Option, + pub ssn: Option, + pub username: Option, + pub passport_number: Option, + pub license_number: Option, } impl KeyEncryptable for IdentityView { diff --git a/crates/bitwarden/src/vault/cipher/login.rs b/crates/bitwarden/src/vault/cipher/login.rs index ea5b1f357..507529625 100644 --- a/crates/bitwarden/src/vault/cipher/login.rs +++ b/crates/bitwarden/src/vault/cipher/login.rs @@ -1,7 +1,5 @@ use bitwarden_api_api::models::{CipherLoginModel, CipherLoginUriModel}; -use bitwarden_crypto::{ - CryptoError, EncString, KeyDecryptable, KeyEncryptable, SymmetricCryptoKey, -}; +use bitwarden_crypto::{CryptoError, DecryptedString, EncString, KeyDecryptable, KeyEncryptable, SymmetricCryptoKey}; use chrono::{DateTime, Utc}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -34,7 +32,7 @@ pub struct LoginUri { #[serde(rename_all = "camelCase", deny_unknown_fields)] #[cfg_attr(feature = "mobile", derive(uniffi::Record))] pub struct LoginUriView { - pub uri: Option, + pub uri: Option, pub r#match: Option, } @@ -55,12 +53,12 @@ pub struct Login { #[serde(rename_all = "camelCase", deny_unknown_fields)] #[cfg_attr(feature = "mobile", derive(uniffi::Record))] pub struct LoginView { - pub username: Option, - pub password: Option, + pub username: Option, + pub password: Option, pub password_revision_date: Option>, pub uris: Option>, - pub totp: Option, + pub totp: Option, pub autofill_on_page_load: Option, } diff --git a/crates/bitwarden/src/vault/collection.rs b/crates/bitwarden/src/vault/collection.rs index d20e5d729..518520f86 100644 --- a/crates/bitwarden/src/vault/collection.rs +++ b/crates/bitwarden/src/vault/collection.rs @@ -1,6 +1,7 @@ use bitwarden_api_api::models::CollectionDetailsResponseModel; use bitwarden_crypto::{ - CryptoError, EncString, KeyContainer, KeyDecryptable, LocateKey, SymmetricCryptoKey, + CryptoError, DecryptedString, EncString, KeyContainer, KeyDecryptable, LocateKey, + SymmetricCryptoKey, }; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -29,7 +30,7 @@ pub struct CollectionView { id: Option, organization_id: Uuid, - name: String, + name: DecryptedString, external_id: Option, hide_passwords: bool, diff --git a/crates/bitwarden/src/vault/folder.rs b/crates/bitwarden/src/vault/folder.rs index 17d1d40aa..a51ef6b19 100644 --- a/crates/bitwarden/src/vault/folder.rs +++ b/crates/bitwarden/src/vault/folder.rs @@ -1,7 +1,5 @@ use bitwarden_api_api::models::FolderResponseModel; -use bitwarden_crypto::{ - CryptoError, EncString, KeyDecryptable, KeyEncryptable, LocateKey, SymmetricCryptoKey, -}; +use bitwarden_crypto::{CryptoError, DecryptedString, EncString, KeyDecryptable, KeyEncryptable, LocateKey, SymmetricCryptoKey}; use chrono::{DateTime, Utc}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -23,7 +21,7 @@ pub struct Folder { #[cfg_attr(feature = "mobile", derive(uniffi::Record))] pub struct FolderView { id: Option, - name: String, + name: DecryptedString, revision_date: DateTime, } diff --git a/crates/bitwarden/src/vault/password_history.rs b/crates/bitwarden/src/vault/password_history.rs index 2ec20116b..2a67d3a2e 100644 --- a/crates/bitwarden/src/vault/password_history.rs +++ b/crates/bitwarden/src/vault/password_history.rs @@ -1,6 +1,7 @@ use bitwarden_api_api::models::CipherPasswordHistoryModel; use bitwarden_crypto::{ - CryptoError, EncString, KeyDecryptable, KeyEncryptable, LocateKey, SymmetricCryptoKey, + CryptoError, DecryptedString, EncString, KeyDecryptable, KeyEncryptable, LocateKey, + SymmetricCryptoKey, }; use chrono::{DateTime, Utc}; use schemars::JsonSchema; @@ -20,7 +21,7 @@ pub struct PasswordHistory { #[serde(rename_all = "camelCase", deny_unknown_fields)] #[cfg_attr(feature = "mobile", derive(uniffi::Record))] pub struct PasswordHistoryView { - password: String, + password: DecryptedString, last_used_date: DateTime, } diff --git a/crates/bitwarden/src/vault/send.rs b/crates/bitwarden/src/vault/send.rs index aba67e00c..dbb7b8fe7 100644 --- a/crates/bitwarden/src/vault/send.rs +++ b/crates/bitwarden/src/vault/send.rs @@ -3,10 +3,7 @@ use base64::{ Engine, }; use bitwarden_api_api::models::{SendFileModel, SendResponseModel, SendTextModel}; -use bitwarden_crypto::{ - derive_shareable_key, generate_random_bytes, CryptoError, EncString, KeyDecryptable, - KeyEncryptable, LocateKey, SymmetricCryptoKey, -}; +use bitwarden_crypto::{derive_shareable_key, generate_random_bytes, CryptoError, EncString, KeyDecryptable, KeyEncryptable, LocateKey, SymmetricCryptoKey, DecryptedString, DecryptedVec}; use chrono::{DateTime, Utc}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -33,7 +30,7 @@ pub struct SendFile { #[cfg_attr(feature = "mobile", derive(uniffi::Record))] pub struct SendFileView { pub id: Option, - pub file_name: String, + pub file_name: DecryptedString, pub size: Option, /// Readable size, ex: "4.2 KB" or "1.43 GB" pub size_name: Option, @@ -51,7 +48,7 @@ pub struct SendText { #[serde(rename_all = "camelCase", deny_unknown_fields)] #[cfg_attr(feature = "mobile", derive(uniffi::Record))] pub struct SendTextView { - pub text: Option, + pub text: Option, pub hidden: bool, } @@ -96,8 +93,8 @@ pub struct SendView { pub id: Option, pub access_id: Option, - pub name: String, - pub notes: Option, + pub name: DecryptedString, + pub notes: Option, /// Base64 encoded key pub key: Option, /// Replace or add a password to an existing send. The SDK will always return None when @@ -122,14 +119,14 @@ pub struct SendView { pub expiration_date: Option>, } -#[derive(Serialize, Deserialize, Debug, JsonSchema)] +#[derive(Serialize, Deserialize, JsonSchema)] #[serde(rename_all = "camelCase", deny_unknown_fields)] #[cfg_attr(feature = "mobile", derive(uniffi::Record))] pub struct SendListView { pub id: Option, pub access_id: Option, - pub name: String, + pub name: DecryptedString, pub r#type: SendType, pub disabled: bool, @@ -144,8 +141,8 @@ impl Send { send_key: &EncString, enc_key: &SymmetricCryptoKey, ) -> Result { - let key: Vec = send_key.decrypt_with_key(enc_key)?; - Self::derive_shareable_key(&key) + let key: DecryptedVec = send_key.decrypt_with_key(enc_key)?; + Self::derive_shareable_key(key.expose()) } fn derive_shareable_key(key: &[u8]) -> Result { @@ -200,8 +197,8 @@ impl KeyDecryptable for Send { // For sends, we first decrypt the send key with the user key, and stretch it to it's full // size For the rest of the fields, we ignore the provided SymmetricCryptoKey and // the stretched key - let k: Vec = self.key.decrypt_with_key(key)?; - let key = Send::derive_shareable_key(&k)?; + let k: DecryptedVec = self.key.decrypt_with_key(key)?; + let key = Send::derive_shareable_key(k.expose())?; Ok(SendView { id: self.id, @@ -209,7 +206,7 @@ impl KeyDecryptable for Send { name: self.name.decrypt_with_key(&key).ok().unwrap_or_default(), notes: self.notes.decrypt_with_key(&key).ok().flatten(), - key: Some(URL_SAFE_NO_PAD.encode(k)), + key: Some(URL_SAFE_NO_PAD.encode(k.expose())), new_password: None, has_password: self.password.is_some(), From 18cf0d6e3c582e4f99b7d97ffdecdca6e1724b97 Mon Sep 17 00:00:00 2001 From: Hinton Date: Fri, 26 Jan 2024 10:35:52 +0100 Subject: [PATCH 07/22] Use boxed value and fix tests --- crates/bitwarden-crypto/src/decrypted.rs | 41 +++++++++++++------ .../src/enc_string/asymmetric.rs | 5 +-- .../src/enc_string/symmetric.rs | 2 +- crates/bitwarden/src/secrets_manager/state.rs | 3 +- crates/bitwarden/src/uniffi_support.rs | 2 +- .../bitwarden/src/vault/cipher/attachment.rs | 7 +++- crates/bitwarden/src/vault/cipher/card.rs | 4 +- crates/bitwarden/src/vault/cipher/cipher.rs | 11 ++++- crates/bitwarden/src/vault/cipher/field.rs | 4 +- crates/bitwarden/src/vault/cipher/identity.rs | 4 +- crates/bitwarden/src/vault/cipher/login.rs | 4 +- crates/bitwarden/src/vault/folder.rs | 5 ++- crates/bitwarden/src/vault/send.rs | 23 ++++++----- 13 files changed, 78 insertions(+), 37 deletions(-) diff --git a/crates/bitwarden-crypto/src/decrypted.rs b/crates/bitwarden-crypto/src/decrypted.rs index 093267fcd..a235e5c40 100644 --- a/crates/bitwarden-crypto/src/decrypted.rs +++ b/crates/bitwarden-crypto/src/decrypted.rs @@ -5,22 +5,27 @@ use std::{ use schemars::JsonSchema; use serde::{Deserialize, Deserializer, Serialize, Serializer}; -use zeroize::{Zeroize, ZeroizeOnDrop}; +use zeroize::Zeroize; use crate::{CryptoError, CryptoKey, KeyEncryptable}; -/// A wrapper for decrypted values. +/// Wrapper for decrypted values which makes a best effort to enforce zeroization of the inner value +/// on drop. The inner value exposes a [`Decrypted::expose`] method which returns a reference to the +/// inner value. Care must be taken to avoid accidentally exposing the inner value through copying +/// or cloning. /// -/// Implements `Zeroize` and `ZeroizeOnDrop` to ensure that the value is zeroized on drop. Please be -/// careful if cloning or copying the inner value using `expose` since any copy will not be -/// zeroized. -#[derive(Zeroize, ZeroizeOnDrop, PartialEq, Clone)] +/// Internally [`Decrypted`] contains a [`Box`] which ensures the value is placed on the heap. It +/// implements the [`Drop`] trait which calls `zeroize` on the inner value. +#[derive(PartialEq, Clone)] pub struct Decrypted { - value: V, + value: Box, } impl Decrypted { - pub fn new(value: V) -> Self { + /// Create a new [`Decrypted`] 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. + pub fn new(value: Box) -> Self { Self { value } } @@ -31,6 +36,18 @@ impl Decrypted { } } +impl Zeroize for Decrypted { + fn zeroize(&mut self) { + self.value.zeroize() + } +} + +impl Drop for Decrypted { + fn drop(&mut self) { + self.zeroize() + } +} + /// Helper to convert a `Decrypted>` to a `Decrypted`, care is taken to ensure any /// intermediate copies are zeroed to avoid leaking sensitive data. impl TryFrom for DecryptedString { @@ -39,14 +56,14 @@ impl TryFrom for DecryptedString { fn try_from(mut v: DecryptedVec) -> Result { let value = std::mem::take(&mut v.value); - let rtn = String::from_utf8(value).map_err(|_| CryptoError::InvalidUtf8String); - rtn.map(Decrypted::new) + let rtn = String::from_utf8(*value).map_err(|_| CryptoError::InvalidUtf8String); + rtn.map(|v| Decrypted::new(Box::new(v))) } } impl Default for Decrypted { fn default() -> Self { - Self::new(V::default()) + Self::new(Box::default()) } } @@ -67,7 +84,7 @@ impl Serialize for Decrypted { impl<'de, V: Zeroize + Deserialize<'de>> Deserialize<'de> for Decrypted { fn deserialize>(deserializer: D) -> Result { - Ok(Self::new(V::deserialize(deserializer)?)) + Ok(Self::new(Box::new(V::deserialize(deserializer)?))) } } diff --git a/crates/bitwarden-crypto/src/enc_string/asymmetric.rs b/crates/bitwarden-crypto/src/enc_string/asymmetric.rs index 322d2c12c..6f65d85d1 100644 --- a/crates/bitwarden-crypto/src/enc_string/asymmetric.rs +++ b/crates/bitwarden-crypto/src/enc_string/asymmetric.rs @@ -176,7 +176,7 @@ impl KeyDecryptable for AsymmetricEncString { key.key.decrypt(Oaep::new::(), data) } } - .map(DecryptedVec::new) + .map(|v| DecryptedVec::new(Box::new(v))) .map_err(|_| CryptoError::KeyDecrypt) } } @@ -202,9 +202,8 @@ impl schemars::JsonSchema for AsymmetricEncString { #[cfg(test)] mod tests { - use crate::DecryptedString; - use super::{AsymmetricCryptoKey, AsymmetricEncString, KeyDecryptable}; + use crate::DecryptedString; const RSA_PRIVATE_KEY: &str = "-----BEGIN PRIVATE KEY----- MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCXRVrCX+2hfOQS diff --git a/crates/bitwarden-crypto/src/enc_string/symmetric.rs b/crates/bitwarden-crypto/src/enc_string/symmetric.rs index ee6c22b30..e6aafcd36 100644 --- a/crates/bitwarden-crypto/src/enc_string/symmetric.rs +++ b/crates/bitwarden-crypto/src/enc_string/symmetric.rs @@ -235,7 +235,7 @@ impl KeyDecryptable for EncString { 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)?; - Ok(Decrypted::new(dec)) + Ok(Decrypted::new(Box::new(dec))) } _ => Err(CryptoError::InvalidKey), } diff --git a/crates/bitwarden/src/secrets_manager/state.rs b/crates/bitwarden/src/secrets_manager/state.rs index f631d421c..eaea08365 100644 --- a/crates/bitwarden/src/secrets_manager/state.rs +++ b/crates/bitwarden/src/secrets_manager/state.rs @@ -32,7 +32,8 @@ pub fn get(state_file: &Path, access_token: &AccessToken) -> Result let file_content = std::fs::read_to_string(state_file)?; let encrypted_state: EncString = file_content.parse()?; - let decrypted_state: DecryptedString = encrypted_state.decrypt_with_key(&access_token.encryption_key)?; + let decrypted_state: DecryptedString = + encrypted_state.decrypt_with_key(&access_token.encryption_key)?; let client_state: ClientState = serde_json::from_str(decrypted_state.expose())?; if client_state.version != STATE_VERSION { diff --git a/crates/bitwarden/src/uniffi_support.rs b/crates/bitwarden/src/uniffi_support.rs index 7293346ce..1040a74d9 100644 --- a/crates/bitwarden/src/uniffi_support.rs +++ b/crates/bitwarden/src/uniffi_support.rs @@ -28,7 +28,7 @@ impl UniffiCustomTypeConverter for DecryptedString { type Builtin = String; fn into_custom(val: Self::Builtin) -> uniffi::Result { - Ok(Self::new(val)) + Ok(Self::new(Box::new(val))) } fn from_custom(obj: Self) -> Self::Builtin { diff --git a/crates/bitwarden/src/vault/cipher/attachment.rs b/crates/bitwarden/src/vault/cipher/attachment.rs index 0f165d20a..e5004743d 100644 --- a/crates/bitwarden/src/vault/cipher/attachment.rs +++ b/crates/bitwarden/src/vault/cipher/attachment.rs @@ -1,4 +1,7 @@ -use bitwarden_crypto::{CryptoError, DecryptedString, DecryptedVec, EncString, KeyDecryptable, KeyEncryptable, SymmetricCryptoKey}; +use bitwarden_crypto::{ + CryptoError, DecryptedString, DecryptedVec, EncString, KeyDecryptable, KeyEncryptable, + SymmetricCryptoKey, +}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -192,6 +195,6 @@ mod tests { .decrypt_with_key(&user_key) .unwrap(); - assert_eq!(dec, original); + assert_eq!(dec.expose(), &original); } } diff --git a/crates/bitwarden/src/vault/cipher/card.rs b/crates/bitwarden/src/vault/cipher/card.rs index b80e9f06f..be1574905 100644 --- a/crates/bitwarden/src/vault/cipher/card.rs +++ b/crates/bitwarden/src/vault/cipher/card.rs @@ -1,5 +1,7 @@ use bitwarden_api_api::models::CipherCardModel; -use bitwarden_crypto::{CryptoError, DecryptedString, EncString, KeyDecryptable, KeyEncryptable, SymmetricCryptoKey}; +use bitwarden_crypto::{ + CryptoError, DecryptedString, EncString, KeyDecryptable, KeyEncryptable, SymmetricCryptoKey, +}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; diff --git a/crates/bitwarden/src/vault/cipher/cipher.rs b/crates/bitwarden/src/vault/cipher/cipher.rs index 586d43429..160775766 100644 --- a/crates/bitwarden/src/vault/cipher/cipher.rs +++ b/crates/bitwarden/src/vault/cipher/cipher.rs @@ -1,5 +1,8 @@ use bitwarden_api_api::models::CipherDetailsResponseModel; -use bitwarden_crypto::{CryptoError, DecryptedString, DecryptedVec, EncString, KeyContainer, KeyDecryptable, KeyEncryptable, LocateKey, SymmetricCryptoKey}; +use bitwarden_crypto::{ + CryptoError, DecryptedString, DecryptedVec, EncString, KeyContainer, KeyDecryptable, + KeyEncryptable, LocateKey, SymmetricCryptoKey, +}; use chrono::{DateTime, Utc}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -227,7 +230,11 @@ impl Cipher { let Some(login) = &self.login else { return Ok(String::new()); }; - login.username.decrypt_with_key(key)?.map(|s: DecryptedString| s.expose().to_owned()).unwrap_or_default() + login + .username + .decrypt_with_key(key)? + .map(|s: DecryptedString| s.expose().to_owned()) + .unwrap_or_default() } CipherType::SecureNote => String::new(), CipherType::Card => { diff --git a/crates/bitwarden/src/vault/cipher/field.rs b/crates/bitwarden/src/vault/cipher/field.rs index 7bcdf6acf..e50e977a5 100644 --- a/crates/bitwarden/src/vault/cipher/field.rs +++ b/crates/bitwarden/src/vault/cipher/field.rs @@ -1,5 +1,7 @@ use bitwarden_api_api::models::CipherFieldModel; -use bitwarden_crypto::{CryptoError, DecryptedString, EncString, KeyDecryptable, KeyEncryptable, SymmetricCryptoKey}; +use bitwarden_crypto::{ + CryptoError, DecryptedString, EncString, KeyDecryptable, KeyEncryptable, SymmetricCryptoKey, +}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use serde_repr::{Deserialize_repr, Serialize_repr}; diff --git a/crates/bitwarden/src/vault/cipher/identity.rs b/crates/bitwarden/src/vault/cipher/identity.rs index a30fc9641..efb247a09 100644 --- a/crates/bitwarden/src/vault/cipher/identity.rs +++ b/crates/bitwarden/src/vault/cipher/identity.rs @@ -1,5 +1,7 @@ use bitwarden_api_api::models::CipherIdentityModel; -use bitwarden_crypto::{CryptoError, DecryptedString, EncString, KeyDecryptable, KeyEncryptable, SymmetricCryptoKey}; +use bitwarden_crypto::{ + CryptoError, DecryptedString, EncString, KeyDecryptable, KeyEncryptable, SymmetricCryptoKey, +}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; diff --git a/crates/bitwarden/src/vault/cipher/login.rs b/crates/bitwarden/src/vault/cipher/login.rs index 507529625..b634bc273 100644 --- a/crates/bitwarden/src/vault/cipher/login.rs +++ b/crates/bitwarden/src/vault/cipher/login.rs @@ -1,5 +1,7 @@ use bitwarden_api_api::models::{CipherLoginModel, CipherLoginUriModel}; -use bitwarden_crypto::{CryptoError, DecryptedString, EncString, KeyDecryptable, KeyEncryptable, SymmetricCryptoKey}; +use bitwarden_crypto::{ + CryptoError, DecryptedString, EncString, KeyDecryptable, KeyEncryptable, SymmetricCryptoKey, +}; use chrono::{DateTime, Utc}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; diff --git a/crates/bitwarden/src/vault/folder.rs b/crates/bitwarden/src/vault/folder.rs index a51ef6b19..616fbcfec 100644 --- a/crates/bitwarden/src/vault/folder.rs +++ b/crates/bitwarden/src/vault/folder.rs @@ -1,5 +1,8 @@ use bitwarden_api_api::models::FolderResponseModel; -use bitwarden_crypto::{CryptoError, DecryptedString, EncString, KeyDecryptable, KeyEncryptable, LocateKey, SymmetricCryptoKey}; +use bitwarden_crypto::{ + CryptoError, DecryptedString, EncString, KeyDecryptable, KeyEncryptable, LocateKey, + SymmetricCryptoKey, +}; use chrono::{DateTime, Utc}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; diff --git a/crates/bitwarden/src/vault/send.rs b/crates/bitwarden/src/vault/send.rs index dbb7b8fe7..31271f791 100644 --- a/crates/bitwarden/src/vault/send.rs +++ b/crates/bitwarden/src/vault/send.rs @@ -3,7 +3,10 @@ use base64::{ Engine, }; use bitwarden_api_api::models::{SendFileModel, SendResponseModel, SendTextModel}; -use bitwarden_crypto::{derive_shareable_key, generate_random_bytes, CryptoError, EncString, KeyDecryptable, KeyEncryptable, LocateKey, SymmetricCryptoKey, DecryptedString, DecryptedVec}; +use bitwarden_crypto::{ + derive_shareable_key, generate_random_bytes, CryptoError, DecryptedString, DecryptedVec, + EncString, KeyDecryptable, KeyEncryptable, LocateKey, SymmetricCryptoKey, +}; use chrono::{DateTime, Utc}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -358,7 +361,7 @@ impl TryFrom for SendText { #[cfg(test)] mod tests { - use bitwarden_crypto::{KeyDecryptable, KeyEncryptable}; + use bitwarden_crypto::{DecryptedString, KeyDecryptable, KeyEncryptable}; use super::{Send, SendText, SendTextView, SendType}; use crate::{ @@ -442,7 +445,7 @@ mod tests { let expected = SendView { id: "3d80dd72-2d14-4f26-812c-b0f0018aa144".parse().ok(), access_id: Some("ct2APRQtJk-BLLDwAYqhRA".to_owned()), - name: "Test".to_string(), + name: DecryptedString::new(Box::new("Test".to_string())), notes: None, key: Some("Pgui0FK85cNhBGWHAlBHBw".to_owned()), new_password: None, @@ -450,7 +453,7 @@ mod tests { r#type: SendType::Text, file: None, text: Some(SendTextView { - text: Some("This is a test".to_owned()), + text: Some(DecryptedString::new(Box::new("This is a test".to_owned()))), hidden: false, }), max_access_count: None, @@ -473,7 +476,7 @@ mod tests { let view = SendView { id: "3d80dd72-2d14-4f26-812c-b0f0018aa144".parse().ok(), access_id: Some("ct2APRQtJk-BLLDwAYqhRA".to_owned()), - name: "Test".to_string(), + name: DecryptedString::new(Box::new("Test".to_string())), notes: None, key: Some("Pgui0FK85cNhBGWHAlBHBw".to_owned()), new_password: None, @@ -481,7 +484,7 @@ mod tests { r#type: SendType::Text, file: None, text: Some(SendTextView { - text: Some("This is a test".to_owned()), + text: Some(DecryptedString::new(Box::new("This is a test".to_owned()))), hidden: false, }), max_access_count: None, @@ -511,7 +514,7 @@ mod tests { let view = SendView { id: None, access_id: Some("ct2APRQtJk-BLLDwAYqhRA".to_owned()), - name: "Test".to_string(), + name: DecryptedString::new(Box::new("Test".to_string())), notes: None, key: None, new_password: None, @@ -519,7 +522,7 @@ mod tests { r#type: SendType::Text, file: None, text: Some(SendTextView { - text: Some("This is a test".to_owned()), + text: Some(DecryptedString::new(Box::new("This is a test".to_owned()))), hidden: false, }), max_access_count: None, @@ -552,7 +555,7 @@ mod tests { let view = SendView { id: None, access_id: Some("ct2APRQtJk-BLLDwAYqhRA".to_owned()), - name: "Test".to_owned(), + name: DecryptedString::new(Box::new("Test".to_owned())), notes: None, key: Some("Pgui0FK85cNhBGWHAlBHBw".to_owned()), new_password: Some("abc123".to_owned()), @@ -560,7 +563,7 @@ mod tests { r#type: SendType::Text, file: None, text: Some(SendTextView { - text: Some("This is a test".to_owned()), + text: Some(DecryptedString::new(Box::new("This is a test".to_owned()))), hidden: false, }), max_access_count: None, From b848625233aba6548ad731b862505e2759e50749 Mon Sep 17 00:00:00 2001 From: Hinton Date: Mon, 29 Jan 2024 13:37:29 +0100 Subject: [PATCH 08/22] Resolve review feedback --- crates/bitwarden-crypto/Cargo.toml | 5 +---- crates/bitwarden-crypto/src/decrypted.rs | 27 +++--------------------- 2 files changed, 4 insertions(+), 28 deletions(-) diff --git a/crates/bitwarden-crypto/Cargo.toml b/crates/bitwarden-crypto/Cargo.toml index 8919138e7..1920fa991 100644 --- a/crates/bitwarden-crypto/Cargo.toml +++ b/crates/bitwarden-crypto/Cargo.toml @@ -41,10 +41,7 @@ subtle = ">=2.5.0, <3.0" thiserror = ">=1.0.40, <2.0" uniffi = { version = "=0.25.2", optional = true } uuid = { version = ">=1.3.3, <2.0", features = ["serde"] } -zeroize = { version = ">=1.7.0, <2.0", features = [ - "zeroize_derive", - "aarch64", -] } +zeroize = { version = ">=1.7.0, <2.0", features = ["derive", "aarch64"] } [dev-dependencies] rand_chacha = "0.3.1" diff --git a/crates/bitwarden-crypto/src/decrypted.rs b/crates/bitwarden-crypto/src/decrypted.rs index 51ee504fe..6b84cc378 100644 --- a/crates/bitwarden-crypto/src/decrypted.rs +++ b/crates/bitwarden-crypto/src/decrypted.rs @@ -5,7 +5,7 @@ use std::{ use schemars::JsonSchema; use serde::{Deserialize, Deserializer, Serialize, Serializer}; -use zeroize::Zeroize; +use zeroize::{Zeroize, ZeroizeOnDrop}; use crate::{CryptoError, CryptoKey, KeyEncryptable}; @@ -16,7 +16,7 @@ use crate::{CryptoError, CryptoKey, KeyEncryptable}; /// /// Internally [`Decrypted`] contains a [`Box`] which ensures the value is placed on the heap. It /// implements the [`Drop`] trait which calls `zeroize` on the inner value. -#[derive(PartialEq, Clone)] +#[derive(PartialEq, Clone, Zeroize, ZeroizeOnDrop)] pub struct Decrypted { value: Box, } @@ -42,18 +42,6 @@ impl Decrypted { } } -impl Zeroize for Decrypted { - fn zeroize(&mut self) { - self.value.zeroize() - } -} - -impl Drop for Decrypted { - fn drop(&mut self) { - self.zeroize() - } -} - /// Helper to convert a `Decrypted>` to a `Decrypted`, care is taken to ensure any /// intermediate copies are zeroed to avoid leaking sensitive data. impl TryFrom for DecryptedString { @@ -76,6 +64,7 @@ impl Default for Decrypted { impl fmt::Debug for Decrypted { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { f.debug_struct("Decrypted") + .field("type", &std::any::type_name::()) .field("value", &"********") .finish() } @@ -109,16 +98,6 @@ impl JsonSchema for Decrypted { } } -/** -impl> KeyEncryptable - for Decrypted -{ - fn encrypt_with_key(self, key: &K) -> Result { - self.value.clone().encrypt_with_key(key) - } -} -**/ - impl + Zeroize + Clone, Key: CryptoKey, Output> KeyEncryptable for Decrypted { From d85fc9fdbc3a2e666f4a374bc7a91e4546b62373 Mon Sep 17 00:00:00 2001 From: Hinton Date: Mon, 29 Jan 2024 14:09:45 +0100 Subject: [PATCH 09/22] Extract a common sensitive type. --- crates/bitwarden-crypto/src/decrypted.rs | 110 ------------ .../src/enc_string/asymmetric.rs | 3 +- .../src/enc_string/symmetric.rs | 5 +- .../bitwarden-crypto/src/keys/device_key.rs | 2 +- .../bitwarden-crypto/src/keys/master_key.rs | 3 +- crates/bitwarden-crypto/src/lib.rs | 4 +- .../src/sensitive/decrypted.rs | 16 ++ crates/bitwarden-crypto/src/sensitive/mod.rs | 4 + .../src/sensitive/sensitive.rs | 170 ++++++++++++++++++ 9 files changed, 197 insertions(+), 120 deletions(-) delete mode 100644 crates/bitwarden-crypto/src/decrypted.rs create mode 100644 crates/bitwarden-crypto/src/sensitive/decrypted.rs create mode 100644 crates/bitwarden-crypto/src/sensitive/mod.rs create mode 100644 crates/bitwarden-crypto/src/sensitive/sensitive.rs diff --git a/crates/bitwarden-crypto/src/decrypted.rs b/crates/bitwarden-crypto/src/decrypted.rs deleted file mode 100644 index 6b84cc378..000000000 --- a/crates/bitwarden-crypto/src/decrypted.rs +++ /dev/null @@ -1,110 +0,0 @@ -use std::{ - borrow::Cow, - fmt::{self, Formatter}, -}; - -use schemars::JsonSchema; -use serde::{Deserialize, Deserializer, Serialize, Serializer}; -use zeroize::{Zeroize, ZeroizeOnDrop}; - -use crate::{CryptoError, CryptoKey, KeyEncryptable}; - -/// Wrapper for decrypted values which makes a best effort to enforce zeroization of the inner value -/// on drop. The inner value exposes a [`Decrypted::expose`] method which returns a reference to the -/// inner value. Care must be taken to avoid accidentally exposing the inner value through copying -/// or cloning. -/// -/// Internally [`Decrypted`] contains a [`Box`] which ensures the value is placed on the heap. It -/// implements the [`Drop`] trait which calls `zeroize` on the inner value. -#[derive(PartialEq, Clone, Zeroize, ZeroizeOnDrop)] -pub struct Decrypted { - value: Box, -} - -impl Decrypted { - /// Create a new [`Decrypted`] 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. - 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. - 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. - pub fn expose_mut(&mut self) -> &mut V { - &mut self.value - } -} - -/// Helper to convert a `Decrypted>` to a `Decrypted`, care is taken to ensure any -/// intermediate copies are zeroed to avoid leaking sensitive data. -impl TryFrom for DecryptedString { - type Error = CryptoError; - - fn try_from(mut v: DecryptedVec) -> Result { - let value = std::mem::take(&mut v.value); - - let rtn = String::from_utf8(*value).map_err(|_| CryptoError::InvalidUtf8String); - rtn.map(|v| Decrypted::new(Box::new(v))) - } -} - -impl Default for Decrypted { - fn default() -> Self { - Self::new(Box::default()) - } -} - -impl fmt::Debug for Decrypted { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - f.debug_struct("Decrypted") - .field("type", &std::any::type_name::()) - .field("value", &"********") - .finish() - } -} - -/// Unfortunately once we serialize a `DecryptedString` we can't control the future memory. -impl Serialize for Decrypted { - fn serialize(&self, serializer: S) -> Result { - self.value.serialize(serializer) - } -} - -impl<'de, V: Zeroize + Deserialize<'de>> Deserialize<'de> for Decrypted { - fn deserialize>(deserializer: D) -> Result { - Ok(Self::new(Box::new(V::deserialize(deserializer)?))) - } -} - -/// Transparently expose the inner value for serialization -impl JsonSchema for Decrypted { - fn schema_name() -> String { - V::schema_name() - } - - fn schema_id() -> Cow<'static, str> { - V::schema_id() - } - - fn json_schema(gen: &mut schemars::gen::SchemaGenerator) -> schemars::schema::Schema { - V::json_schema(gen) - } -} - -impl + Zeroize + Clone, Key: CryptoKey, Output> - KeyEncryptable for Decrypted -{ - fn encrypt_with_key(self, key: &Key) -> Result { - self.value.clone().encrypt_with_key(key) - } -} - -pub type DecryptedVec = Decrypted>; -pub type DecryptedString = Decrypted; diff --git a/crates/bitwarden-crypto/src/enc_string/asymmetric.rs b/crates/bitwarden-crypto/src/enc_string/asymmetric.rs index 3a1874271..76cb45fbb 100644 --- a/crates/bitwarden-crypto/src/enc_string/asymmetric.rs +++ b/crates/bitwarden-crypto/src/enc_string/asymmetric.rs @@ -7,10 +7,9 @@ use serde::Deserialize; use super::{from_b64_vec, split_enc_string}; use crate::{ - decrypted::{DecryptedString, DecryptedVec}, error::{CryptoError, EncStringParseError, Result}, rsa::encrypt_rsa2048_oaep_sha1, - AsymmetricCryptoKey, AsymmetricEncryptable, KeyDecryptable, + AsymmetricCryptoKey, AsymmetricEncryptable, KeyDecryptable, {DecryptedString, DecryptedVec}, }; // This module is a workaround to avoid deprecated warnings that come from the ZeroizeOnDrop diff --git a/crates/bitwarden-crypto/src/enc_string/symmetric.rs b/crates/bitwarden-crypto/src/enc_string/symmetric.rs index 3b52c7628..53496622e 100644 --- a/crates/bitwarden-crypto/src/enc_string/symmetric.rs +++ b/crates/bitwarden-crypto/src/enc_string/symmetric.rs @@ -7,9 +7,8 @@ use serde::Deserialize; use super::{check_length, from_b64, from_b64_vec, split_enc_string}; use crate::{ - decrypted::{DecryptedString, DecryptedVec}, error::{CryptoError, EncStringParseError, Result}, - KeyDecryptable, KeyEncryptable, LocateKey, SymmetricCryptoKey, + KeyDecryptable, KeyEncryptable, LocateKey, SymmetricCryptoKey, {DecryptedString, DecryptedVec}, }; /// # Encrypted string primitive @@ -281,7 +280,7 @@ impl schemars::JsonSchema for EncString { #[cfg(test)] mod tests { use super::EncString; - use crate::{decrypted::DecryptedString, derive_symmetric_key, KeyDecryptable, KeyEncryptable}; + use crate::{derive_symmetric_key, DecryptedString, KeyDecryptable, KeyEncryptable}; #[test] fn test_enc_string_roundtrip() { diff --git a/crates/bitwarden-crypto/src/keys/device_key.rs b/crates/bitwarden-crypto/src/keys/device_key.rs index 26ff6fe62..f140c3c4c 100644 --- a/crates/bitwarden-crypto/src/keys/device_key.rs +++ b/crates/bitwarden-crypto/src/keys/device_key.rs @@ -1,5 +1,5 @@ use crate::{ - decrypted::DecryptedVec, error::Result, AsymmetricCryptoKey, AsymmetricEncString, EncString, + error::Result, AsymmetricCryptoKey, AsymmetricEncString, DecryptedVec, EncString, KeyDecryptable, KeyEncryptable, SymmetricCryptoKey, UserKey, }; diff --git a/crates/bitwarden-crypto/src/keys/master_key.rs b/crates/bitwarden-crypto/src/keys/master_key.rs index 6fe93696e..6a6602c6b 100644 --- a/crates/bitwarden-crypto/src/keys/master_key.rs +++ b/crates/bitwarden-crypto/src/keys/master_key.rs @@ -8,9 +8,8 @@ use serde::{Deserialize, Serialize}; use sha2::Digest; use crate::{ - decrypted::DecryptedVec, util::{self, hkdf_expand}, - EncString, KeyDecryptable, Result, SymmetricCryptoKey, UserKey, + DecryptedVec, EncString, KeyDecryptable, Result, SymmetricCryptoKey, UserKey, }; #[derive(Serialize, Deserialize, Debug, JsonSchema, Clone)] diff --git a/crates/bitwarden-crypto/src/lib.rs b/crates/bitwarden-crypto/src/lib.rs index 94cf7a9da..8c368580a 100644 --- a/crates/bitwarden-crypto/src/lib.rs +++ b/crates/bitwarden-crypto/src/lib.rs @@ -40,8 +40,8 @@ pub use util::generate_random_bytes; mod wordlist; pub use util::pbkdf2; pub use wordlist::EFF_LONG_WORD_LIST; -mod decrypted; -pub use decrypted::{Decrypted, DecryptedString, DecryptedVec}; +mod sensitive; +pub use sensitive::*; #[cfg(feature = "mobile")] uniffi::setup_scaffolding!(); diff --git a/crates/bitwarden-crypto/src/sensitive/decrypted.rs b/crates/bitwarden-crypto/src/sensitive/decrypted.rs new file mode 100644 index 000000000..02096d081 --- /dev/null +++ b/crates/bitwarden-crypto/src/sensitive/decrypted.rs @@ -0,0 +1,16 @@ +use zeroize::Zeroize; + +use crate::{CryptoError, CryptoKey, KeyEncryptable, Sensitive}; + +/// Type alias for a [`Sensitive`] value to denote decrypted data. +pub type Decrypted = Sensitive; +pub type DecryptedVec = Decrypted>; +pub type DecryptedString = Decrypted; + +impl + Zeroize + Clone, Key: CryptoKey, Output> + KeyEncryptable for Decrypted +{ + fn encrypt_with_key(self, key: &Key) -> Result { + self.value.clone().encrypt_with_key(key) + } +} diff --git a/crates/bitwarden-crypto/src/sensitive/mod.rs b/crates/bitwarden-crypto/src/sensitive/mod.rs new file mode 100644 index 000000000..f38368580 --- /dev/null +++ b/crates/bitwarden-crypto/src/sensitive/mod.rs @@ -0,0 +1,4 @@ +mod sensitive; +pub use sensitive::{Sensitive, SensitiveString, SensitiveVec}; +mod decrypted; +pub use decrypted::{Decrypted, DecryptedString, DecryptedVec}; diff --git a/crates/bitwarden-crypto/src/sensitive/sensitive.rs b/crates/bitwarden-crypto/src/sensitive/sensitive.rs new file mode 100644 index 000000000..86dd93ef5 --- /dev/null +++ b/crates/bitwarden-crypto/src/sensitive/sensitive.rs @@ -0,0 +1,170 @@ +use std::{ + borrow::Cow, + fmt::{self, Formatter}, +}; + +use schemars::JsonSchema; +use serde::{Deserialize, Deserializer, Serialize, Serializer}; +use zeroize::{Zeroize, ZeroizeOnDrop}; + +use crate::CryptoError; + +/// Wrapper for sensitive values which makes a best effort to enforce zeroization of the inner value +/// on drop. The inner value exposes a [`Sensitive::expose`] method which returns a reference to the +/// inner value. Care must be taken to avoid accidentally exposing the inner value through copying +/// or cloning. +/// +/// Internally [`Sensitive`] contains a [`Box`] which ensures the value is placed on the heap. It +/// implements the [`Drop`] trait which calls `zeroize` on the inner value. +#[derive(PartialEq, Clone, Zeroize, ZeroizeOnDrop)] +pub struct Sensitive { + pub(super) value: Box, +} + +pub type SensitiveVec = Sensitive>; +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 + /// initial stack allocation presuming the value is not used before being boxed. + 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. + 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. + pub fn expose_mut(&mut self) -> &mut V { + &mut self.value + } +} + +/// Helper to convert a `Sensitive>` to a `Sensitive`, care is taken to ensure any +/// intermediate copies are zeroed to avoid leaking sensitive data. +impl TryFrom for SensitiveString { + type Error = CryptoError; + + fn try_from(mut v: SensitiveVec) -> Result { + 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 Default for Sensitive { + fn default() -> Self { + Self::new(Box::default()) + } +} + +impl fmt::Debug for Sensitive { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Sensitive") + .field("type", &std::any::type_name::()) + .field("value", &"********") + .finish() + } +} + +/// Unfortunately once we serialize a `SensitiveString` we can't control the future memory. +impl Serialize for Sensitive { + fn serialize(&self, serializer: S) -> Result { + self.value.serialize(serializer) + } +} + +impl<'de, V: Zeroize + Deserialize<'de>> Deserialize<'de> for Sensitive { + fn deserialize>(deserializer: D) -> Result { + Ok(Self::new(Box::new(V::deserialize(deserializer)?))) + } +} + +/// Transparently expose the inner value for serialization +impl JsonSchema for Sensitive { + fn schema_name() -> String { + V::schema_name() + } + + fn schema_id() -> Cow<'static, str> { + V::schema_id() + } + + fn json_schema(gen: &mut schemars::gen::SchemaGenerator) -> schemars::schema::Schema { + V::json_schema(gen) + } +} + +#[cfg(test)] +mod tests { + use schemars::schema_for; + + use super::*; + + #[test] + fn test_debug() { + let string = Sensitive::new(Box::new("test".to_string())); + assert_eq!( + format!("{:?}", string), + "Sensitive { type: \"alloc::string::String\", value: \"********\" }" + ); + + let vector = Sensitive::new(Box::new(vec![1, 2, 3])); + assert_eq!( + format!("{:?}", vector), + "Sensitive { type: \"alloc::vec::Vec\", value: \"********\" }" + ); + } + + #[test] + fn test_schemars() { + #[derive(JsonSchema)] + struct TestStruct { + #[allow(dead_code)] + s: SensitiveString, + #[allow(dead_code)] + v: SensitiveVec, + } + + let schema = schema_for!(TestStruct); + let json = serde_json::to_string_pretty(&schema).unwrap(); + let expected = r##"{ + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "TestStruct", + "type": "object", + "required": ["s", "v"], + "properties": { + "s": { + "$ref": "#/definitions/String" + }, + "v": { + "$ref": "#/definitions/Array_of_uint8" + } + }, + "definitions": { + "Array_of_uint8": { + "type": "array", + "items": { + "type": "integer", + "format": "uint8", + "minimum": 0.0 + } + }, + "String": { + "type": "string" + } + } + }"##; + + assert_eq!( + json.parse::().unwrap(), + expected.parse::().unwrap() + ); + } +} From d7eedaa94020ce01fea3d74a83b3bcf5165406a1 Mon Sep 17 00:00:00 2001 From: Hinton Date: Mon, 29 Jan 2024 14:11:45 +0100 Subject: [PATCH 10/22] Format --- crates/bitwarden-crypto/src/enc_string/asymmetric.rs | 2 +- crates/bitwarden-crypto/src/enc_string/symmetric.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bitwarden-crypto/src/enc_string/asymmetric.rs b/crates/bitwarden-crypto/src/enc_string/asymmetric.rs index 76cb45fbb..9af344389 100644 --- a/crates/bitwarden-crypto/src/enc_string/asymmetric.rs +++ b/crates/bitwarden-crypto/src/enc_string/asymmetric.rs @@ -9,7 +9,7 @@ use super::{from_b64_vec, split_enc_string}; use crate::{ error::{CryptoError, EncStringParseError, Result}, rsa::encrypt_rsa2048_oaep_sha1, - AsymmetricCryptoKey, AsymmetricEncryptable, KeyDecryptable, {DecryptedString, DecryptedVec}, + AsymmetricCryptoKey, AsymmetricEncryptable, DecryptedString, DecryptedVec, KeyDecryptable, }; // This module is a workaround to avoid deprecated warnings that come from the ZeroizeOnDrop diff --git a/crates/bitwarden-crypto/src/enc_string/symmetric.rs b/crates/bitwarden-crypto/src/enc_string/symmetric.rs index 53496622e..e65a64c60 100644 --- a/crates/bitwarden-crypto/src/enc_string/symmetric.rs +++ b/crates/bitwarden-crypto/src/enc_string/symmetric.rs @@ -8,7 +8,7 @@ use serde::Deserialize; use super::{check_length, from_b64, from_b64_vec, split_enc_string}; use crate::{ error::{CryptoError, EncStringParseError, Result}, - KeyDecryptable, KeyEncryptable, LocateKey, SymmetricCryptoKey, {DecryptedString, DecryptedVec}, + DecryptedString, DecryptedVec, KeyDecryptable, KeyEncryptable, LocateKey, SymmetricCryptoKey, }; /// # Encrypted string primitive From ba14e8c952629c95e6120bcddfc4e25e56868c16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garci=CC=81a?= Date: Thu, 7 Mar 2024 13:43:17 +0100 Subject: [PATCH 11/22] Update exporters to use Sensitive --- Cargo.lock | 1 + .../src/sensitive/sensitive.rs | 14 +- crates/bitwarden-exporters/Cargo.toml | 1 + crates/bitwarden-exporters/src/csv.rs | 69 ++++--- .../bitwarden-exporters/src/encrypted_json.rs | 64 +++--- crates/bitwarden-exporters/src/json.rs | 193 +++++++++--------- crates/bitwarden-exporters/src/lib.rs | 68 +++--- crates/bitwarden/src/tool/exporters/mod.rs | 18 +- crates/bitwarden/src/vault/cipher/cipher.rs | 6 +- 9 files changed, 230 insertions(+), 204 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5c9d0fb58..71f98409a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -444,6 +444,7 @@ dependencies = [ "bitwarden-crypto", "chrono", "csv", + "itertools 0.12.1", "serde", "serde_json", "thiserror", diff --git a/crates/bitwarden-crypto/src/sensitive/sensitive.rs b/crates/bitwarden-crypto/src/sensitive/sensitive.rs index 86dd93ef5..7a3c202a5 100644 --- a/crates/bitwarden-crypto/src/sensitive/sensitive.rs +++ b/crates/bitwarden-crypto/src/sensitive/sensitive.rs @@ -101,6 +101,18 @@ impl JsonSchema for Sensitive { } } +impl Sensitive { + // We use a lot of `&str` 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 str we should be able to avoid accidental usages + pub fn test(value: &'static str) -> Self { + Self::new(Box::new(value.to_string())) + } +} + #[cfg(test)] mod tests { use schemars::schema_for; @@ -109,7 +121,7 @@ mod tests { #[test] fn test_debug() { - let string = Sensitive::new(Box::new("test".to_string())); + let string = Sensitive::test("test"); assert_eq!( format!("{:?}", string), "Sensitive { type: \"alloc::string::String\", value: \"********\" }" diff --git a/crates/bitwarden-exporters/Cargo.toml b/crates/bitwarden-exporters/Cargo.toml index 40316437f..4f8364179 100644 --- a/crates/bitwarden-exporters/Cargo.toml +++ b/crates/bitwarden-exporters/Cargo.toml @@ -22,6 +22,7 @@ chrono = { version = ">=0.4.26, <0.5", features = [ "std", ], default-features = false } csv = "1.3.0" +itertools = "0.12.1" serde = { version = ">=1.0, <2.0", features = ["derive"] } serde_json = ">=1.0.96, <2.0" thiserror = ">=1.0.40, <2.0" diff --git a/crates/bitwarden-exporters/src/csv.rs b/crates/bitwarden-exporters/src/csv.rs index 644eeb030..ac6995588 100644 --- a/crates/bitwarden-exporters/src/csv.rs +++ b/crates/bitwarden-exporters/src/csv.rs @@ -1,9 +1,9 @@ use std::collections::HashMap; +use bitwarden_crypto::{DecryptedString, Sensitive}; use csv::Writer; use serde::Serializer; use thiserror::Error; -use uuid::Uuid; use crate::{Cipher, CipherType, Field, Folder}; @@ -14,7 +14,7 @@ pub enum CsvError { } pub(crate) fn export_csv(folders: Vec, ciphers: Vec) -> Result { - let folders: HashMap = folders.into_iter().map(|f| (f.id, f.name)).collect(); + let folders: HashMap<_, _> = folders.into_iter().map(|f| (f.id, f.name)).collect(); let rows = ciphers .into_iter() @@ -59,27 +59,30 @@ pub(crate) fn export_csv(folders: Vec, ciphers: Vec) -> Result, + folder: Option, #[serde(serialize_with = "bool_serialize")] favorite: bool, r#type: String, - name: String, - notes: Option, + name: DecryptedString, + notes: Option, #[serde(serialize_with = "fields_serialize")] fields: Vec, reprompt: u8, #[serde(serialize_with = "vec_serialize")] - login_uri: Vec, - login_username: Option, - login_password: Option, - login_totp: Option, + login_uri: Vec, + login_username: Option, + login_password: Option, + login_totp: Option, } -fn vec_serialize(x: &[String], s: S) -> Result +fn vec_serialize(x: &[DecryptedString], s: S) -> Result where S: Serializer, { - s.serialize_str(x.join(",").as_str()) + let iter = itertools::Itertools::intersperse(x.iter().map(|s| s.expose().as_str()), ","); + let result: Sensitive = Sensitive::new(Box::new(iter.collect())); + + s.serialize_str(result.expose()) } fn bool_serialize(x: &bool, s: S) -> Result @@ -98,8 +101,14 @@ where .map(|f| { format!( "{}: {}", - f.name.to_owned().unwrap_or_default(), - f.value.to_owned().unwrap_or_default() + f.name + .as_ref() + .map(|n| n.expose().to_owned()) + .unwrap_or_default(), + f.value + .as_ref() + .map(|n| n.expose().to_owned()) + .unwrap_or_default(), ) }) .collect::>() @@ -118,24 +127,24 @@ mod tests { let folders = vec![ Folder { id: "d55d65d7-c161-40a4-94ca-b0d20184d91a".parse().unwrap(), - name: "Test Folder A".to_string(), + name: DecryptedString::test("Test Folder A"), }, Folder { id: "583e7665-0126-4d37-9139-b0d20184dd86".parse().unwrap(), - name: "Test Folder B".to_string(), + name: DecryptedString::test("Test Folder B"), }, ]; let ciphers = vec![ Cipher { id: "d55d65d7-c161-40a4-94ca-b0d20184d91a".parse().unwrap(), folder_id: None, - name: "test@bitwarden.com".to_string(), + name: DecryptedString::test("test@bitwarden.com"), notes: None, r#type: CipherType::Login(Box::new(Login { - username: Some("test@bitwarden.com".to_string()), - password: Some("Abc123".to_string()), + username: Some(DecryptedString::test("test@bitwarden.com")), + password: Some(DecryptedString::test("Abc123")), login_uris: vec![LoginUri { - uri: Some("https://google.com".to_string()), + uri: Some(DecryptedString::test("https://google.com")), r#match: None, }], totp: None, @@ -150,29 +159,29 @@ mod tests { Cipher { id: "7dd81bd0-cc72-4f42-96e7-b0fc014e71a3".parse().unwrap(), folder_id: Some("583e7665-0126-4d37-9139-b0d20184dd86".parse().unwrap()), - name: "Steam Account".to_string(), + name: DecryptedString::test("Steam Account"), notes: None, r#type: CipherType::Login(Box::new(Login { - username: Some("steam".to_string()), - password: Some("3Pvb8u7EfbV*nJ".to_string()), + username: Some(DecryptedString::test("steam")), + password: Some(DecryptedString::test("3Pvb8u7EfbV*nJ")), login_uris: vec![LoginUri { - uri: Some("https://steampowered.com".to_string()), + uri: Some(DecryptedString::test("https://steampowered.com")), r#match: None, }], - totp: Some("steam://ABCD123".to_string()), + totp: Some(DecryptedString::test("steam://ABCD123")), })), favorite: true, reprompt: 0, fields: vec![ Field { - name: Some("Test".to_string()), - value: Some("v".to_string()), + name: Some(DecryptedString::test("Test")), + value: Some(DecryptedString::test("v")), r#type: 0, linked_id: None, }, Field { - name: Some("Hidden".to_string()), - value: Some("asdfer".to_string()), + name: Some(DecryptedString::test("Hidden")), + value: Some(DecryptedString::test("asdfer")), r#type: 1, linked_id: None, }, @@ -200,7 +209,7 @@ mod tests { let ciphers = vec![Cipher { id: "d55d65d7-c161-40a4-94ca-b0d20184d91a".parse().unwrap(), folder_id: None, - name: "My Card".to_string(), + name: DecryptedString::test("My Card"), notes: None, r#type: CipherType::Card(Box::new(Card { cardholder_name: None, @@ -229,7 +238,7 @@ mod tests { let ciphers = vec![Cipher { id: "d55d65d7-c161-40a4-94ca-b0d20184d91a".parse().unwrap(), folder_id: None, - name: "My Identity".to_string(), + name: DecryptedString::test("My Identity"), notes: None, r#type: CipherType::Identity(Box::new(Identity { title: None, diff --git a/crates/bitwarden-exporters/src/encrypted_json.rs b/crates/bitwarden-exporters/src/encrypted_json.rs index 1bbfd2660..c9a529091 100644 --- a/crates/bitwarden-exporters/src/encrypted_json.rs +++ b/crates/bitwarden-exporters/src/encrypted_json.rs @@ -83,6 +83,8 @@ pub(crate) struct EncryptedJsonExport { mod tests { use std::num::NonZeroU32; + use bitwarden_crypto::DecryptedString; + use super::*; use crate::{ Card, Cipher, CipherType, Field, Identity, Login, LoginUri, SecureNote, SecureNoteType, @@ -93,24 +95,24 @@ mod tests { let _export = export_encrypted_json( vec![Folder { id: "942e2984-1b9a-453b-b039-b107012713b9".parse().unwrap(), - name: "Important".to_string(), + name: DecryptedString::test("Important"), }], vec![ Cipher { id: "25c8c414-b446-48e9-a1bd-b10700bbd740".parse().unwrap(), folder_id: Some("942e2984-1b9a-453b-b039-b107012713b9".parse().unwrap()), - name: "Bitwarden".to_string(), - notes: Some("My note".to_string()), + name: DecryptedString::test("Bitwarden"), + notes: Some(DecryptedString::test("My note")), r#type: CipherType::Login(Box::new(Login { - username: Some("test@bitwarden.com".to_string()), - password: Some("asdfasdfasdf".to_string()), + username: Some(DecryptedString::test("test@bitwarden.com")), + password: Some(DecryptedString::test("asdfasdfasdf")), login_uris: vec![LoginUri { - uri: Some("https://vault.bitwarden.com".to_string()), + uri: Some(DecryptedString::test("https://vault.bitwarden.com")), r#match: None, }], - totp: Some("ABC".to_string()), + totp: Some(DecryptedString::test("ABC")), })), favorite: true, @@ -118,31 +120,31 @@ mod tests { fields: vec![ Field { - name: Some("Text".to_string()), - value: Some("A".to_string()), + name: Some(DecryptedString::test("Text")), + value: Some(DecryptedString::test("A")), r#type: 0, linked_id: None, }, Field { - name: Some("Hidden".to_string()), - value: Some("B".to_string()), + name: Some(DecryptedString::test("Hidden")), + value: Some(DecryptedString::test("B")), r#type: 1, linked_id: None, }, Field { - name: Some("Boolean (true)".to_string()), - value: Some("true".to_string()), + name: Some(DecryptedString::test("Boolean (true)")), + value: Some(DecryptedString::test("true")), r#type: 2, linked_id: None, }, Field { - name: Some("Boolean (false)".to_string()), - value: Some("false".to_string()), + name: Some(DecryptedString::test("Boolean (false)")), + value: Some(DecryptedString::test("false")), r#type: 2, linked_id: None, }, Field { - name: Some("Linked".to_string()), + name: Some(DecryptedString::test("Linked")), value: None, r#type: 3, linked_id: Some(101), @@ -157,8 +159,8 @@ mod tests { id: "23f0f877-42b1-4820-a850-b10700bc41eb".parse().unwrap(), folder_id: None, - name: "My secure note".to_string(), - notes: Some("Very secure!".to_string()), + name: DecryptedString::test("My secure note"), + notes: Some(DecryptedString::test("Very secure!")), r#type: CipherType::SecureNote(Box::new(SecureNote { r#type: SecureNoteType::Generic, @@ -177,16 +179,16 @@ mod tests { id: "3ed8de45-48ee-4e26-a2dc-b10701276c53".parse().unwrap(), folder_id: None, - name: "My card".to_string(), + name: DecryptedString::test("My card"), notes: None, r#type: CipherType::Card(Box::new(Card { - cardholder_name: Some("John Doe".to_string()), - exp_month: Some("1".to_string()), - exp_year: Some("2032".to_string()), - code: Some("123".to_string()), - brand: Some("Visa".to_string()), - number: Some("4111111111111111".to_string()), + cardholder_name: Some(DecryptedString::test("John Doe")), + exp_month: Some(DecryptedString::test("1")), + exp_year: Some(DecryptedString::test("2032")), + code: Some(DecryptedString::test("123")), + brand: Some(DecryptedString::test("Visa")), + number: Some(DecryptedString::test("4111111111111111")), })), favorite: false, @@ -202,14 +204,14 @@ mod tests { id: "41cc3bc1-c3d9-4637-876c-b10701273712".parse().unwrap(), folder_id: Some("942e2984-1b9a-453b-b039-b107012713b9".parse().unwrap()), - name: "My identity".to_string(), + name: DecryptedString::test("My identity"), notes: None, r#type: CipherType::Identity(Box::new(Identity { - title: Some("Mr".to_string()), - first_name: Some("John".to_string()), + title: Some(DecryptedString::test("Mr")), + first_name: Some(DecryptedString::test("John")), middle_name: None, - last_name: Some("Doe".to_string()), + last_name: Some(DecryptedString::test("Doe")), address1: None, address2: None, address3: None, @@ -217,11 +219,11 @@ mod tests { state: None, postal_code: None, country: None, - company: Some("Bitwarden".to_string()), + company: Some(DecryptedString::test("Bitwarden")), email: None, phone: None, ssn: None, - username: Some("JDoe".to_string()), + username: Some(DecryptedString::test("JDoe")), passport_number: None, license_number: None, })), diff --git a/crates/bitwarden-exporters/src/json.rs b/crates/bitwarden-exporters/src/json.rs index 3f6c72c1f..f2c889f9f 100644 --- a/crates/bitwarden-exporters/src/json.rs +++ b/crates/bitwarden-exporters/src/json.rs @@ -1,3 +1,4 @@ +use bitwarden_crypto::DecryptedString; use chrono::{DateTime, Utc}; use thiserror::Error; use uuid::Uuid; @@ -36,7 +37,7 @@ struct JsonExport { #[serde(rename_all = "camelCase")] struct JsonFolder { id: Uuid, - name: String, + name: DecryptedString, } impl From for JsonFolder { @@ -57,8 +58,8 @@ struct JsonCipher { organization_id: Option, collection_ids: Option>, - name: String, - notes: Option, + name: DecryptedString, + notes: Option, r#type: u8, #[serde(skip_serializing_if = "Option::is_none")] @@ -75,7 +76,7 @@ struct JsonCipher { #[serde(skip_serializing_if = "Vec::is_empty")] fields: Vec, - password_history: Option>, + password_history: Option>, revision_date: DateTime, creation_date: DateTime, @@ -85,11 +86,11 @@ struct JsonCipher { #[derive(serde::Serialize)] #[serde(rename_all = "camelCase")] struct JsonLogin { - username: Option, - password: Option, + username: Option, + password: Option, uris: Vec, - totp: Option, - fido2_credentials: Vec, + totp: Option, + fido2_credentials: Vec, } impl From for JsonLogin { @@ -107,7 +108,7 @@ impl From for JsonLogin { #[derive(serde::Serialize)] #[serde(rename_all = "camelCase")] struct JsonLoginUri { - uri: Option, + uri: Option, r#match: Option, } @@ -137,12 +138,12 @@ impl From for JsonSecureNote { #[derive(serde::Serialize)] #[serde(rename_all = "camelCase")] struct JsonCard { - cardholder_name: Option, - exp_month: Option, - exp_year: Option, - code: Option, - brand: Option, - number: Option, + cardholder_name: Option, + exp_month: Option, + exp_year: Option, + code: Option, + brand: Option, + number: Option, } impl From for JsonCard { @@ -161,24 +162,24 @@ impl From for JsonCard { #[derive(serde::Serialize)] #[serde(rename_all = "camelCase")] struct JsonIdentity { - title: Option, - first_name: Option, - middle_name: Option, - last_name: Option, - address1: Option, - address2: Option, - address3: Option, - city: Option, - state: Option, - postal_code: Option, - country: Option, - company: Option, - email: Option, - phone: Option, - ssn: Option, - username: Option, - passport_number: Option, - license_number: Option, + title: Option, + first_name: Option, + middle_name: Option, + last_name: Option, + address1: Option, + address2: Option, + address3: Option, + city: Option, + state: Option, + postal_code: Option, + country: Option, + company: Option, + email: Option, + phone: Option, + ssn: Option, + username: Option, + passport_number: Option, + license_number: Option, } impl From for JsonIdentity { @@ -209,8 +210,8 @@ impl From for JsonIdentity { #[derive(serde::Serialize)] #[serde(rename_all = "camelCase")] struct JsonField { - name: Option, - value: Option, + name: Option, + value: Option, r#type: u8, linked_id: Option, } @@ -278,17 +279,17 @@ mod tests { id: "25c8c414-b446-48e9-a1bd-b10700bbd740".parse().unwrap(), folder_id: Some("942e2984-1b9a-453b-b039-b107012713b9".parse().unwrap()), - name: "Bitwarden".to_string(), - notes: Some("My note".to_string()), + name: DecryptedString::test("Bitwarden"), + notes: Some(DecryptedString::test("My note")), r#type: CipherType::Login(Box::new(Login { - username: Some("test@bitwarden.com".to_string()), - password: Some("asdfasdfasdf".to_string()), + username: Some(DecryptedString::test("test@bitwarden.com")), + password: Some(DecryptedString::test("asdfasdfasdf")), login_uris: vec![LoginUri { - uri: Some("https://vault.bitwarden.com".to_string()), + uri: Some(DecryptedString::test("https://vault.bitwarden.com")), r#match: None, }], - totp: Some("ABC".to_string()), + totp: Some(DecryptedString::test("ABC")), })), favorite: true, @@ -296,31 +297,31 @@ mod tests { fields: vec![ Field { - name: Some("Text".to_string()), - value: Some("A".to_string()), + name: Some(DecryptedString::test("Text")), + value: Some(DecryptedString::test("A")), r#type: 0, linked_id: None, }, Field { - name: Some("Hidden".to_string()), - value: Some("B".to_string()), + name: Some(DecryptedString::test("Hidden")), + value: Some(DecryptedString::test("B")), r#type: 1, linked_id: None, }, Field { - name: Some("Boolean (true)".to_string()), - value: Some("true".to_string()), + name: Some(DecryptedString::test("Boolean (true)")), + value: Some(DecryptedString::test("true")), r#type: 2, linked_id: None, }, Field { - name: Some("Boolean (false)".to_string()), - value: Some("false".to_string()), + name: Some(DecryptedString::test("Boolean (false)")), + value: Some(DecryptedString::test("false")), r#type: 2, linked_id: None, }, Field { - name: Some("Linked".to_string()), + name: Some(DecryptedString::test("Linked")), value: None, r#type: 3, linked_id: Some(101), @@ -406,8 +407,8 @@ mod tests { id: "23f0f877-42b1-4820-a850-b10700bc41eb".parse().unwrap(), folder_id: None, - name: "My secure note".to_string(), - notes: Some("Very secure!".to_string()), + name: DecryptedString::test("My secure note"), + notes: Some(DecryptedString::test("Very secure!")), r#type: CipherType::SecureNote(Box::new(SecureNote { r#type: SecureNoteType::Generic, @@ -456,16 +457,16 @@ mod tests { id: "3ed8de45-48ee-4e26-a2dc-b10701276c53".parse().unwrap(), folder_id: None, - name: "My card".to_string(), + name: DecryptedString::test("My card"), notes: None, r#type: CipherType::Card(Box::new(Card { - cardholder_name: Some("John Doe".to_string()), - exp_month: Some("1".to_string()), - exp_year: Some("2032".to_string()), - code: Some("123".to_string()), - brand: Some("Visa".to_string()), - number: Some("4111111111111111".to_string()), + cardholder_name: Some(DecryptedString::test("John Doe")), + exp_month: Some(DecryptedString::test("1")), + exp_year: Some(DecryptedString::test("2032")), + code: Some(DecryptedString::test("123")), + brand: Some(DecryptedString::test("Visa")), + number: Some(DecryptedString::test("4111111111111111")), })), favorite: false, @@ -516,14 +517,14 @@ mod tests { id: "41cc3bc1-c3d9-4637-876c-b10701273712".parse().unwrap(), folder_id: Some("942e2984-1b9a-453b-b039-b107012713b9".parse().unwrap()), - name: "My identity".to_string(), + name: DecryptedString::test("My identity"), notes: None, r#type: CipherType::Identity(Box::new(Identity { - title: Some("Mr".to_string()), - first_name: Some("John".to_string()), + title: Some(DecryptedString::test("Mr")), + first_name: Some(DecryptedString::test("John")), middle_name: None, - last_name: Some("Doe".to_string()), + last_name: Some(DecryptedString::test("Doe")), address1: None, address2: None, address3: None, @@ -531,11 +532,11 @@ mod tests { state: None, postal_code: None, country: None, - company: Some("Bitwarden".to_string()), + company: Some(DecryptedString::test("Bitwarden")), email: None, phone: None, ssn: None, - username: Some("JDoe".to_string()), + username: Some(DecryptedString::test("JDoe")), passport_number: None, license_number: None, })), @@ -608,24 +609,24 @@ mod tests { let export = export_json( vec![Folder { id: "942e2984-1b9a-453b-b039-b107012713b9".parse().unwrap(), - name: "Important".to_string(), + name: DecryptedString::test("Important"), }], vec![ Cipher { id: "25c8c414-b446-48e9-a1bd-b10700bbd740".parse().unwrap(), folder_id: Some("942e2984-1b9a-453b-b039-b107012713b9".parse().unwrap()), - name: "Bitwarden".to_string(), - notes: Some("My note".to_string()), + name: DecryptedString::test("Bitwarden"), + notes: Some(DecryptedString::test("My note")), r#type: CipherType::Login(Box::new(Login { - username: Some("test@bitwarden.com".to_string()), - password: Some("asdfasdfasdf".to_string()), + username: Some(DecryptedString::test("test@bitwarden.com")), + password: Some(DecryptedString::test("asdfasdfasdf")), login_uris: vec![LoginUri { - uri: Some("https://vault.bitwarden.com".to_string()), + uri: Some(DecryptedString::test("https://vault.bitwarden.com")), r#match: None, }], - totp: Some("ABC".to_string()), + totp: Some(DecryptedString::test("ABC")), })), favorite: true, @@ -633,31 +634,31 @@ mod tests { fields: vec![ Field { - name: Some("Text".to_string()), - value: Some("A".to_string()), + name: Some(DecryptedString::test("Text")), + value: Some(DecryptedString::test("A")), r#type: 0, linked_id: None, }, Field { - name: Some("Hidden".to_string()), - value: Some("B".to_string()), + name: Some(DecryptedString::test("Hidden")), + value: Some(DecryptedString::test("B")), r#type: 1, linked_id: None, }, Field { - name: Some("Boolean (true)".to_string()), - value: Some("true".to_string()), + name: Some(DecryptedString::test("Boolean (true)")), + value: Some(DecryptedString::test("true")), r#type: 2, linked_id: None, }, Field { - name: Some("Boolean (false)".to_string()), - value: Some("false".to_string()), + name: Some(DecryptedString::test("Boolean (false)")), + value: Some(DecryptedString::test("false")), r#type: 2, linked_id: None, }, Field { - name: Some("Linked".to_string()), + name: Some(DecryptedString::test("Linked")), value: None, r#type: 3, linked_id: Some(101), @@ -672,8 +673,8 @@ mod tests { id: "23f0f877-42b1-4820-a850-b10700bc41eb".parse().unwrap(), folder_id: None, - name: "My secure note".to_string(), - notes: Some("Very secure!".to_string()), + name: DecryptedString::test("My secure note"), + notes: Some(DecryptedString::test("Very secure!")), r#type: CipherType::SecureNote(Box::new(SecureNote { r#type: SecureNoteType::Generic, @@ -692,16 +693,16 @@ mod tests { id: "3ed8de45-48ee-4e26-a2dc-b10701276c53".parse().unwrap(), folder_id: None, - name: "My card".to_string(), + name: DecryptedString::test("My card"), notes: None, r#type: CipherType::Card(Box::new(Card { - cardholder_name: Some("John Doe".to_string()), - exp_month: Some("1".to_string()), - exp_year: Some("2032".to_string()), - code: Some("123".to_string()), - brand: Some("Visa".to_string()), - number: Some("4111111111111111".to_string()), + cardholder_name: Some(DecryptedString::test("John Doe")), + exp_month: Some(DecryptedString::test("1")), + exp_year: Some(DecryptedString::test("2032")), + code: Some(DecryptedString::test("123")), + brand: Some(DecryptedString::test("Visa")), + number: Some(DecryptedString::test("4111111111111111")), })), favorite: false, @@ -717,14 +718,14 @@ mod tests { id: "41cc3bc1-c3d9-4637-876c-b10701273712".parse().unwrap(), folder_id: Some("942e2984-1b9a-453b-b039-b107012713b9".parse().unwrap()), - name: "My identity".to_string(), + name: DecryptedString::test("My identity"), notes: None, r#type: CipherType::Identity(Box::new(Identity { - title: Some("Mr".to_string()), - first_name: Some("John".to_string()), + title: Some(DecryptedString::test("Mr")), + first_name: Some(DecryptedString::test("John")), middle_name: None, - last_name: Some("Doe".to_string()), + last_name: Some(DecryptedString::test("Doe")), address1: None, address2: None, address3: None, @@ -732,11 +733,11 @@ mod tests { state: None, postal_code: None, country: None, - company: Some("Bitwarden".to_string()), + company: Some(DecryptedString::test("Bitwarden")), email: None, phone: None, ssn: None, - username: Some("JDoe".to_string()), + username: Some(DecryptedString::test("JDoe")), passport_number: None, license_number: None, })), diff --git a/crates/bitwarden-exporters/src/lib.rs b/crates/bitwarden-exporters/src/lib.rs index f17d31a2d..22febad8f 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::{DecryptedString, Kdf}; use chrono::{DateTime, Utc}; use thiserror::Error; use uuid::Uuid; @@ -22,7 +22,7 @@ pub enum Format { /// that is not tied to the internal vault models. We may revisit this in the future. pub struct Folder { pub id: Uuid, - pub name: String, + pub name: DecryptedString, } /// Export representation of a Bitwarden cipher. @@ -33,8 +33,8 @@ pub struct Cipher { pub id: Uuid, pub folder_id: Option, - pub name: String, - pub notes: Option, + pub name: DecryptedString, + pub notes: Option, pub r#type: CipherType, @@ -50,8 +50,8 @@ pub struct Cipher { #[derive(Clone)] pub struct Field { - pub name: Option, - pub value: Option, + pub name: Option, + pub value: Option, pub r#type: u8, pub linked_id: Option, } @@ -75,24 +75,24 @@ impl ToString for CipherType { } pub struct Login { - pub username: Option, - pub password: Option, + pub username: Option, + pub password: Option, pub login_uris: Vec, - pub totp: Option, + pub totp: Option, } pub struct LoginUri { - pub uri: Option, + pub uri: Option, pub r#match: Option, } pub struct Card { - pub cardholder_name: Option, - pub exp_month: Option, - pub exp_year: Option, - pub code: Option, - pub brand: Option, - pub number: Option, + pub cardholder_name: Option, + pub exp_month: Option, + pub exp_year: Option, + pub code: Option, + pub brand: Option, + pub number: Option, } pub struct SecureNote { @@ -104,24 +104,24 @@ pub enum SecureNoteType { } pub struct Identity { - pub title: Option, - pub first_name: Option, - pub middle_name: Option, - pub last_name: Option, - pub address1: Option, - pub address2: Option, - pub address3: Option, - pub city: Option, - pub state: Option, - pub postal_code: Option, - pub country: Option, - pub company: Option, - pub email: Option, - pub phone: Option, - pub ssn: Option, - pub username: Option, - pub passport_number: Option, - pub license_number: Option, + pub title: Option, + pub first_name: Option, + pub middle_name: Option, + pub last_name: Option, + pub address1: Option, + pub address2: Option, + pub address3: Option, + pub city: Option, + pub state: Option, + pub postal_code: Option, + pub country: Option, + pub company: Option, + pub email: Option, + pub phone: Option, + pub ssn: Option, + pub username: Option, + pub passport_number: Option, + pub license_number: Option, } #[derive(Error, Debug)] diff --git a/crates/bitwarden/src/tool/exporters/mod.rs b/crates/bitwarden/src/tool/exporters/mod.rs index 9e9e99ed5..5982dfe7e 100644 --- a/crates/bitwarden/src/tool/exporters/mod.rs +++ b/crates/bitwarden/src/tool/exporters/mod.rs @@ -206,7 +206,7 @@ impl From for bitwarden_exporters::SecureNoteType { mod tests { use std::num::NonZeroU32; - use bitwarden_crypto::Kdf; + use bitwarden_crypto::{DecryptedString, Kdf}; use chrono::{DateTime, Utc}; use super::*; @@ -216,7 +216,7 @@ mod tests { fn test_try_from_folder_view() { let view = FolderView { id: Some("fd411a1a-fec8-4070-985d-0e6560860e69".parse().unwrap()), - name: "test_name".to_string(), + name: DecryptedString::test("test_name"), revision_date: "2024-01-30T17:55:36.150Z".parse().unwrap(), }; @@ -226,7 +226,7 @@ mod tests { f.id, "fd411a1a-fec8-4070-985d-0e6560860e69".parse().unwrap() ); - assert_eq!(f.name, "test_name".to_string()); + assert_eq!(f.name.expose(), "test_name"); } #[test] @@ -234,8 +234,8 @@ mod tests { let cipher_view = CipherView { r#type: CipherType::Login, login: Some(LoginView { - username: Some("test_username".to_string()), - password: Some("test_password".to_string()), + username: Some(DecryptedString::test("test_username")), + password: Some(DecryptedString::test("test_password")), password_revision_date: None, uris: None, totp: None, @@ -246,7 +246,7 @@ mod tests { folder_id: None, collection_ids: vec![], key: None, - name: "My login".to_string(), + name: DecryptedString::test("My login"), notes: None, identity: None, card: None, @@ -272,7 +272,7 @@ mod tests { "fd411a1a-fec8-4070-985d-0e6560860e69".parse().unwrap() ); assert_eq!(cipher.folder_id, None); - assert_eq!(cipher.name, "My login".to_string()); + assert_eq!(cipher.name.expose(), "My login"); assert_eq!(cipher.notes, None); assert!(!cipher.favorite); assert_eq!(cipher.reprompt, 0); @@ -288,8 +288,8 @@ mod tests { assert_eq!(cipher.deleted_date, None); if let bitwarden_exporters::CipherType::Login(l) = cipher.r#type { - assert_eq!(l.username, Some("test_username".to_string())); - assert_eq!(l.password, Some("test_password".to_string())); + assert_eq!(l.username.unwrap().expose(), "test_username"); + assert_eq!(l.password.unwrap().expose(), "test_password"); assert!(l.login_uris.is_empty()); assert_eq!(l.totp, None); } else { diff --git a/crates/bitwarden/src/vault/cipher/cipher.rs b/crates/bitwarden/src/vault/cipher/cipher.rs index e981227c0..865ae778d 100644 --- a/crates/bitwarden/src/vault/cipher/cipher.rs +++ b/crates/bitwarden/src/vault/cipher/cipher.rs @@ -432,8 +432,8 @@ mod tests { CipherView { r#type: CipherType::Login, login: Some(login::LoginView { - username: Some("test_username".to_string()), - password: Some("test_password".to_string()), + username: Some(DecryptedString::test("test_username")), + password: Some(DecryptedString::test("test_password")), password_revision_date: None, uris: None, totp: None, @@ -444,7 +444,7 @@ mod tests { folder_id: None, collection_ids: vec![], key: None, - name: "My test login".to_string(), + name: DecryptedString::test("My test login"), notes: None, identity: None, card: None, From 19fecaecd0ff72b30c57a8bf60f69aa2d3f6febe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garci=CC=81a?= Date: Thu, 7 Mar 2024 18:21:27 +0100 Subject: [PATCH 12/22] Add support for base64 encode/decode for the Sensitive types --- .../src/enc_string/asymmetric.rs | 16 ++- .../src/enc_string/symmetric.rs | 11 +- .../src/keys/asymmetric_crypto_key.rs | 129 ++++++++++-------- .../bitwarden-crypto/src/keys/device_key.rs | 33 ++--- .../bitwarden-crypto/src/keys/master_key.rs | 6 +- .../src/keys/shareable_key.rs | 4 +- .../src/keys/symmetric_crypto_key.rs | 53 +++---- crates/bitwarden-crypto/src/sensitive/mod.rs | 1 + .../src/sensitive/sensitive.rs | 29 ++++ crates/bitwarden-exporters/src/csv.rs | 4 +- crates/bitwarden/src/auth/auth_request.rs | 39 +++--- .../bitwarden/src/auth/login/access_token.rs | 14 +- crates/bitwarden/src/client/access_token.rs | 2 +- .../src/client/encryption_settings.rs | 6 +- crates/bitwarden/src/mobile/crypto.rs | 34 ++--- crates/bitwarden/src/secrets_manager/state.rs | 8 +- .../bitwarden/src/vault/cipher/attachment.rs | 16 ++- crates/bitwarden/src/vault/cipher/cipher.rs | 6 +- crates/bitwarden/src/vault/send.rs | 2 +- 19 files changed, 232 insertions(+), 181 deletions(-) diff --git a/crates/bitwarden-crypto/src/enc_string/asymmetric.rs b/crates/bitwarden-crypto/src/enc_string/asymmetric.rs index 7b9fc9ebf..f4c8b0f7c 100644 --- a/crates/bitwarden-crypto/src/enc_string/asymmetric.rs +++ b/crates/bitwarden-crypto/src/enc_string/asymmetric.rs @@ -210,9 +210,11 @@ mod tests { use schemars::schema_for; use super::{AsymmetricCryptoKey, AsymmetricEncString, KeyDecryptable}; - use crate::DecryptedString; + use crate::{DecryptedString, SensitiveString}; - const RSA_PRIVATE_KEY: &str = "-----BEGIN PRIVATE KEY----- + fn rsa_private_key_string() -> SensitiveString { + SensitiveString::test( + "-----BEGIN PRIVATE KEY----- MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCXRVrCX+2hfOQS 8HzYUS2oc/jGVTZpv+/Ryuoh9d8ihYX9dd0cYh2tl6KWdFc88lPUH11Oxqy20Rk2 e5r/RF6T9yM0Me3NPnaKt+hlhLtfoc0h86LnhD56A9FDUfuI0dVnPcrwNv0YJIo9 @@ -239,11 +241,13 @@ AoEZ18y6/KIjpSMpqC92Nnk/EBM9EYe6Cf4eA9ApAoGAeqEUg46UTlJySkBKURGp Is3v1kkf5I0X8DnOhwb+HPxNaiEdmO7ckm8+tPVgppLcG0+tMdLjigFQiDUQk2y3 WjyxP5ZvXu7U96jaJRI8PFMoE06WeVYcdIzrID2HvqH+w0UQJFrLJ/0Mn4stFAEz XKZBokBGnjFnTnKcs7nv/O8= ------END PRIVATE KEY-----"; +-----END PRIVATE KEY-----", + ) + } #[test] fn test_enc_string_rsa2048_oaep_sha256_b64() { - let private_key = AsymmetricCryptoKey::from_pem(RSA_PRIVATE_KEY).unwrap(); + let private_key = AsymmetricCryptoKey::from_pem(rsa_private_key_string()).unwrap(); let enc_str: &str = "3.YFqzW9LL/uLjCnl0RRLtndzGJ1FV27mcwQwGjfJPOVrgCX9nJSUYCCDd0iTIyOZ/zRxG47b6L1Z3qgkEfcxjmrSBq60gijc3E2TBMAg7OCLVcjORZ+i1sOVOudmOPWro6uA8refMrg4lqbieDlbLMzjVEwxfi5WpcL876cD0vYyRwvLO3bzFrsE7x33HHHtZeOPW79RqMn5efsB5Dj9wVheC9Ix9AYDjbo+rjg9qR6guwKmS7k2MSaIQlrDR7yu8LP+ePtiSjx+gszJV5jQGfcx60dtiLQzLS/mUD+RmU7B950Bpx0H7x56lT5yXZbWK5YkoP6qd8B8D2aKbP68Ywg=="; let enc_string: AsymmetricEncString = enc_str.parse().unwrap(); @@ -255,7 +259,7 @@ XKZBokBGnjFnTnKcs7nv/O8= #[test] fn test_enc_string_rsa2048_oaep_sha1_b64() { - let private_key = AsymmetricCryptoKey::from_pem(RSA_PRIVATE_KEY).unwrap(); + let private_key = AsymmetricCryptoKey::from_pem(rsa_private_key_string()).unwrap(); let enc_str: &str = "4.ZheRb3PCfAunyFdQYPfyrFqpuvmln9H9w5nDjt88i5A7ug1XE0LJdQHCIYJl0YOZ1gCOGkhFu/CRY2StiLmT3iRKrrVBbC1+qRMjNNyDvRcFi91LWsmRXhONVSPjywzrJJXglsztDqGkLO93dKXNhuKpcmtBLsvgkphk/aFvxbaOvJ/FHdK/iV0dMGNhc/9tbys8laTdwBlI5xIChpRcrfH+XpSFM88+Bu03uK67N9G6eU1UmET+pISJwJvMuIDMqH+qkT7OOzgL3t6I0H2LDj+CnsumnQmDsvQzDiNfTR0IgjpoE9YH2LvPXVP2wVUkiTwXD9cG/E7XeoiduHyHjw=="; let enc_string: AsymmetricEncString = enc_str.parse().unwrap(); @@ -267,7 +271,7 @@ XKZBokBGnjFnTnKcs7nv/O8= #[test] fn test_enc_string_rsa2048_oaep_sha1_hmac_sha256_b64() { - let private_key = AsymmetricCryptoKey::from_pem(RSA_PRIVATE_KEY).unwrap(); + let private_key = AsymmetricCryptoKey::from_pem(rsa_private_key_string()).unwrap(); let enc_str: &str = "6.ThnNc67nNr7GELyuhGGfsXNP2zJnNqhrIsjntEQ27r2qmn8vwdHbTbfO0cwt6YgSibDN0PjiCZ1O3Wb/IFq+vwvyRwFqF9145wBF8CQCbkhV+M0XvO99kh0daovtt120Nve/5ETI5PbPag9VdalKRQWZypJaqQHm5TAQVf4F5wtLlCLMBkzqTk+wkFe7BPMTGn07T+O3eJbTxXvyMZewQ7icJF0MZVA7VyWX9qElmZ89FCKowbf1BMr5pbcQ+0KdXcSVW3to43VkTp7k7COwsuH3M/i1AuVP5YN8ixjyRpvaeGqX/ap2nCHK2Wj5VxgCGT7XEls6ZknnAp9nB9qVjQ==|s3ntw5H/KKD/qsS0lUghTHl5Sm9j6m7YEdNHf0OeAFQ="; let enc_string: AsymmetricEncString = enc_str.parse().unwrap(); diff --git a/crates/bitwarden-crypto/src/enc_string/symmetric.rs b/crates/bitwarden-crypto/src/enc_string/symmetric.rs index ca536f43c..867e48f8e 100644 --- a/crates/bitwarden-crypto/src/enc_string/symmetric.rs +++ b/crates/bitwarden-crypto/src/enc_string/symmetric.rs @@ -306,7 +306,8 @@ mod tests { use super::EncString; use crate::{ - derive_symmetric_key, DecryptedString, KeyDecryptable, KeyEncryptable, SymmetricCryptoKey, + derive_symmetric_key, DecryptedString, KeyDecryptable, KeyEncryptable, SensitiveString, + SymmetricCryptoKey, }; #[test] @@ -407,8 +408,8 @@ mod tests { #[test] fn test_decrypt_cbc256() { - let key = "hvBMMb1t79YssFZkpetYsM3deyVuQv4r88Uj9gvYe08="; - let key: SymmetricCryptoKey = key.parse().unwrap(); + let key = SensitiveString::test("hvBMMb1t79YssFZkpetYsM3deyVuQv4r88Uj9gvYe08="); + let key = SymmetricCryptoKey::try_from(key).unwrap(); let enc_str = "0.NQfjHLr6za7VQVAbrpL81w==|wfrjmyJ0bfwkQlySrhw8dA=="; let enc_string: EncString = enc_str.parse().unwrap(); @@ -420,8 +421,8 @@ mod tests { #[test] fn test_decrypt_cbc128_hmac() { - let key = "Gt1aZ8kTTgkF80bLtb7LiMZBcxEA2FA5mbvV4x7K208="; - let key: SymmetricCryptoKey = key.parse().unwrap(); + let key = SensitiveString::test("Gt1aZ8kTTgkF80bLtb7LiMZBcxEA2FA5mbvV4x7K208="); + let key = SymmetricCryptoKey::try_from(key).unwrap(); let enc_str = "1.CU/oG4VZuxbHoZSDZjCLQw==|kb1HGwAk+fQ275ORfLf5Ew==|8UaEYHyqRZcG37JWhYBOBdEatEXd1u1/wN7OuImolcM="; let enc_string: EncString = enc_str.parse().unwrap(); diff --git a/crates/bitwarden-crypto/src/keys/asymmetric_crypto_key.rs b/crates/bitwarden-crypto/src/keys/asymmetric_crypto_key.rs index a360f86bc..9636defe1 100644 --- a/crates/bitwarden-crypto/src/keys/asymmetric_crypto_key.rs +++ b/crates/bitwarden-crypto/src/keys/asymmetric_crypto_key.rs @@ -3,7 +3,10 @@ use std::pin::Pin; use rsa::{pkcs8::DecodePublicKey, RsaPrivateKey, RsaPublicKey}; use super::key_encryptable::CryptoKey; -use crate::error::{CryptoError, Result}; +use crate::{ + error::{CryptoError, Result}, + SensitiveString, SensitiveVec, +}; /// Trait to allow both [`AsymmetricCryptoKey`] and [`AsymmetricPublicCryptoKey`] to be used to /// encrypt [AsymmetricEncString](crate::AsymmetricEncString). @@ -20,9 +23,9 @@ pub struct AsymmetricPublicCryptoKey { impl AsymmetricPublicCryptoKey { /// Build a public key from the SubjectPublicKeyInfo DER. - pub fn from_der(der: &[u8]) -> Result { + pub fn from_der(der: SensitiveVec) -> Result { Ok(Self { - key: rsa::RsaPublicKey::from_public_key_der(der) + key: rsa::RsaPublicKey::from_public_key_der(der.expose()) .map_err(|_| CryptoError::InvalidKey)?, }) } @@ -65,17 +68,21 @@ impl AsymmetricCryptoKey { } } - pub fn from_pem(pem: &str) -> Result { + pub fn from_pem(pem: SensitiveString) -> Result { use rsa::pkcs8::DecodePrivateKey; Ok(Self { - key: Box::pin(RsaPrivateKey::from_pkcs8_pem(pem).map_err(|_| CryptoError::InvalidKey)?), + key: Box::pin( + RsaPrivateKey::from_pkcs8_pem(pem.expose()).map_err(|_| CryptoError::InvalidKey)?, + ), }) } - pub fn from_der(der: &[u8]) -> Result { + pub fn from_der(der: SensitiveVec) -> Result { use rsa::pkcs8::DecodePrivateKey; Ok(Self { - key: Box::pin(RsaPrivateKey::from_pkcs8_der(der).map_err(|_| CryptoError::InvalidKey)?), + key: Box::pin( + RsaPrivateKey::from_pkcs8_der(der.expose()).map_err(|_| CryptoError::InvalidKey)?, + ), }) } @@ -117,16 +124,17 @@ impl std::fmt::Debug for AsymmetricCryptoKey { #[cfg(test)] mod tests { - use base64::{engine::general_purpose::STANDARD, Engine}; + use base64::engine::general_purpose::STANDARD; use crate::{ AsymmetricCryptoKey, AsymmetricEncString, AsymmetricPublicCryptoKey, DecryptedString, - KeyDecryptable, + KeyDecryptable, SensitiveString, }; #[test] fn test_asymmetric_crypto_key() { - let pem_key_str = "-----BEGIN PRIVATE KEY----- + let pem_key_str = SensitiveString::test( + "-----BEGIN PRIVATE KEY----- MIIEwAIBADANBgkqhkiG9w0BAQEFAASCBKowggSmAgEAAoIBAQDiTQVuzhdygFz5 qv14i+XFDGTnDravzUQT1hPKPGUZOUSZ1gwdNgkWqOIaOnR65BHEnL0sp4bnuiYc afeK2JAW5Sc8Z7IxBNSuAwhQmuKx3RochMIiuCkI2/p+JvUQoJu6FBNm8OoJ4Cwm @@ -153,67 +161,68 @@ Zy2o6PPxhfkagaDjqEeN9Lrs5LD4nEvDkr5CG1vOjmMCgYEAvIBFKRm31NyF8jLi CVuPwC5PzrW5iThDmsWTaXFpB3esUsbICO2pEz872oeQS+Em4GO5vXUlpbbFPzup PFhA8iMJ8TAvemhvc7oM0OZqpU6p3K4seHf6BkwLxumoA3vDJfovu9RuXVcJVOnf DnqOsltgPomWZ7xVfMkm9niL2OA= ------END PRIVATE KEY-----"; +-----END PRIVATE KEY-----", + ); - let der_key_vec = STANDARD.decode("MIIEwAIBADANBgkqhkiG9w0BAQEFAASCBKowggSmAgEAAoIBAQDiTQVuzhdygFz5qv14i+XFDGTnDravzUQT1hPKPGUZOUSZ1gwdNgkWqOIaOnR65BHEnL0sp4bnuiYcafeK2JAW5Sc8Z7IxBNSuAwhQmuKx3RochMIiuCkI2/p+JvUQoJu6FBNm8OoJ4CwmqqHGZESMfnpQDCuDrB3JdJEdXhtmnl0C48sGjOk3WaBMcgGqn8LbJDUlyu1zdqyvb0waJf0iV4PJm2fkUl7+57D/2TkpbCqURVnZK1FFIEg8mr6FzSN1F2pOfktkNYZwP7MSNR7o81CkRSCMr7EkIVa+MZYMBx106BMK7FXgWB7nbSpsWKxBk7ZDHkID2famrEcVtrzDAgMBAAECggEBAKwq9OssGGKgjhvUnyrLJHAZ0dqIMyzk+dotkLjX4gKiszJmyqiep6N5sStLNbsZMPtoU/RZMCW0VbJgXFhiEp2YkZU/Py5UAoqw++53J+kx0d/IkPphKbb3xUec0+1mg5O6GljDCQuiZXS1dIa/WfeZcezclW6Dz9WovY6ePjJ+8vEBR1icbNKzyeINd6MtPtpcgQPHtDwHvhPyUDbKDYGbLvjh9nui8h4+ZUlXKuVRjB0ChxiKV1xJRjkrEVoulOOicd5r597WfB2ghax3pvRZ4MdXemCXm3gQYqPVKachvGU+1cPQR/MBJZpxT+EZA97xwtFS3gqwbxJaNFcoE8ECgYEA9OaeYZhQPDo485tI1u/Z7L/3PNape9hBQIXoW7+MgcQ5NiWqYh8Jnj43EIYa0wM/ECQINr1Za8Q5e6KRJ30FcU+kfyjuQ0jeXdNELGU/fx5XXNg/vV8GevHwxRlwzqZTCg6UExUZzbYEQqd7l+wPyETGeua5xCEywA1nX/D101kCgYEA7I6aMFjhEjO71RmzNhqjKJt6DOghoOfQTjhaaanNEhLYSbenFz1mlb21mW67ulmz162saKdIYLxQNJIP8ZPmxh4ummOJI8w9ClHfo8WuCI2hCjJ19xbQJocSbTA5aJg6lA1IDVZMDbQwsnAByPRGpaLHBT/Q9ByeKvCMB+9amXsCgYEAx65yXSkP4sumPBrVHUub6MntERIGRxBgw/drKcPZEMWp0FiNwEuGUBxyUWrG3F69QK/gcqGZE6F/LSu0JvptQaKqgXQiMYJsrRvhbkFvsHpQyUcZUZL1ebFjm5HOxPAgrQaN/bEqxOwwNRjSUWEMzUImg3c06JIZCzbinvudtKECgYEAkY3JF/iIPI/yglP27lKDlCfeeHSYxI3+oTKRhzSAxx8rUGidenJAXeDGDauR/T7Wpt3pGNfddBBK9Z3uC4Iq3DqUCFE4f/taj7ADAJ1Q0Vh7/28/IJM77ojr8J1cpZwNZy2o6PPxhfkagaDjqEeN9Lrs5LD4nEvDkr5CG1vOjmMCgYEAvIBFKRm31NyF8jLiCVuPwC5PzrW5iThDmsWTaXFpB3esUsbICO2pEz872oeQS+Em4GO5vXUlpbbFPzupPFhA8iMJ8TAvemhvc7oM0OZqpU6p3K4seHf6BkwLxumoA3vDJfovu9RuXVcJVOnfDnqOsltgPomWZ7xVfMkm9niL2OA=").unwrap(); + let der_key_vec = SensitiveString::test("MIIEwAIBADANBgkqhkiG9w0BAQEFAASCBKowggSmAgEAAoIBAQDiTQVuzhdygFz5qv14i+XFDGTnDravzUQT1hPKPGUZOUSZ1gwdNgkWqOIaOnR65BHEnL0sp4bnuiYcafeK2JAW5Sc8Z7IxBNSuAwhQmuKx3RochMIiuCkI2/p+JvUQoJu6FBNm8OoJ4CwmqqHGZESMfnpQDCuDrB3JdJEdXhtmnl0C48sGjOk3WaBMcgGqn8LbJDUlyu1zdqyvb0waJf0iV4PJm2fkUl7+57D/2TkpbCqURVnZK1FFIEg8mr6FzSN1F2pOfktkNYZwP7MSNR7o81CkRSCMr7EkIVa+MZYMBx106BMK7FXgWB7nbSpsWKxBk7ZDHkID2famrEcVtrzDAgMBAAECggEBAKwq9OssGGKgjhvUnyrLJHAZ0dqIMyzk+dotkLjX4gKiszJmyqiep6N5sStLNbsZMPtoU/RZMCW0VbJgXFhiEp2YkZU/Py5UAoqw++53J+kx0d/IkPphKbb3xUec0+1mg5O6GljDCQuiZXS1dIa/WfeZcezclW6Dz9WovY6ePjJ+8vEBR1icbNKzyeINd6MtPtpcgQPHtDwHvhPyUDbKDYGbLvjh9nui8h4+ZUlXKuVRjB0ChxiKV1xJRjkrEVoulOOicd5r597WfB2ghax3pvRZ4MdXemCXm3gQYqPVKachvGU+1cPQR/MBJZpxT+EZA97xwtFS3gqwbxJaNFcoE8ECgYEA9OaeYZhQPDo485tI1u/Z7L/3PNape9hBQIXoW7+MgcQ5NiWqYh8Jnj43EIYa0wM/ECQINr1Za8Q5e6KRJ30FcU+kfyjuQ0jeXdNELGU/fx5XXNg/vV8GevHwxRlwzqZTCg6UExUZzbYEQqd7l+wPyETGeua5xCEywA1nX/D101kCgYEA7I6aMFjhEjO71RmzNhqjKJt6DOghoOfQTjhaaanNEhLYSbenFz1mlb21mW67ulmz162saKdIYLxQNJIP8ZPmxh4ummOJI8w9ClHfo8WuCI2hCjJ19xbQJocSbTA5aJg6lA1IDVZMDbQwsnAByPRGpaLHBT/Q9ByeKvCMB+9amXsCgYEAx65yXSkP4sumPBrVHUub6MntERIGRxBgw/drKcPZEMWp0FiNwEuGUBxyUWrG3F69QK/gcqGZE6F/LSu0JvptQaKqgXQiMYJsrRvhbkFvsHpQyUcZUZL1ebFjm5HOxPAgrQaN/bEqxOwwNRjSUWEMzUImg3c06JIZCzbinvudtKECgYEAkY3JF/iIPI/yglP27lKDlCfeeHSYxI3+oTKRhzSAxx8rUGidenJAXeDGDauR/T7Wpt3pGNfddBBK9Z3uC4Iq3DqUCFE4f/taj7ADAJ1Q0Vh7/28/IJM77ojr8J1cpZwNZy2o6PPxhfkagaDjqEeN9Lrs5LD4nEvDkr5CG1vOjmMCgYEAvIBFKRm31NyF8jLiCVuPwC5PzrW5iThDmsWTaXFpB3esUsbICO2pEz872oeQS+Em4GO5vXUlpbbFPzupPFhA8iMJ8TAvemhvc7oM0OZqpU6p3K4seHf6BkwLxumoA3vDJfovu9RuXVcJVOnfDnqOsltgPomWZ7xVfMkm9niL2OA=").decode_base64(STANDARD).unwrap(); // Load the two different formats and check they are the same key let pem_key = AsymmetricCryptoKey::from_pem(pem_key_str).unwrap(); - let der_key = AsymmetricCryptoKey::from_der(&der_key_vec).unwrap(); + let der_key = AsymmetricCryptoKey::from_der(der_key_vec.clone()).unwrap(); assert_eq!(pem_key.key, der_key.key); // Check that the keys can be converted back to DER - assert_eq!(der_key.to_der().unwrap(), der_key_vec); - assert_eq!(pem_key.to_der().unwrap(), der_key_vec); + assert_eq!(&der_key.to_der().unwrap(), der_key_vec.expose()); + assert_eq!(&pem_key.to_der().unwrap(), der_key_vec.expose()); } #[test] fn test_encrypt_public_decrypt_private() { - let private_key = STANDARD - .decode(concat!( - "MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCu9xd+vmkIPoqH", - "NejsFZzkd1xuCn1TqGTT7ANhAEnbI/yaVt3caI30kwUC2WIToFpNgu7Ej0x2TteY", - "OgrLrdcC4jy1SifmKYv/v3ZZxrd/eqttmH2k588panseRwHK3LVk7xA+URhQ/bjL", - "gPM59V0uR1l+z1fmooeJPFz5WSXNObc9Jqnh45FND+U/UYHXTLSomTn7jgZFxJBK", - "veS7q6Lat7wAnYZCF2dnPmhZoJv+SKPltA8HAGsgQGWBF1p5qxV1HrAUk8kBBnG2", - "paj0w8p5UM6RpDdCuvKH7j1LiuWffn3b9Z4dgzmE7jsMmvzoQtypzIKaSxhqzvFO", - "od9V8dJdAgMBAAECggEAGGIYjOIB1rOKkDHP4ljXutI0mCRPl3FMDemiBeppoIfZ", - "G/Q3qpAKmndDt0Quwh/yfcNdvZhf1kwCCTWri/uPz5fSUIyDV3TaTRu0ZWoHaBVj", - "Hxylg+4HRZUQj+Vi50/PWr/jQmAAVMcrMfcoTl82q2ynmP/R1vM3EsXOCjTliv5B", - "XlMPRjj/9PDBH0dnnVcAPDOpflzOTL2f4HTFEMlmg9/tZBnd96J/cmfhjAv9XpFL", - "FBAFZzs5pz0rwCNSR8QZNonnK7pngVUlGDLORK58y84tGmxZhGdne3CtCWey/sJ4", - "7QF0Pe8YqWBU56926IY6DcSVBuQGZ6vMCNlU7J8D2QKBgQDXyh3t2TicM/n1QBLk", - "zLoGmVUmxUGziHgl2dnJiGDtyOAU3+yCorPgFaCie29s5qm4b0YEGxUxPIrRrEro", - "h0FfKn9xmr8CdmTPTcjJW1+M7bxxq7oBoU/QzKXgIHlpeCjjnvPJt0PcNkNTjCXv", - "shsrINh2rENoe/x79eEfM/N5eQKBgQDPkYSmYyALoNq8zq0A4BdR+F5lb5Fj5jBH", - "Jk68l6Uti+0hRbJ2d1tQTLkU+eCPQLGBl6fuc1i4K5FV7v14jWtRPdD7wxrkRi3j", - "ilqQwLBOU6Bj3FK4DvlLF+iYTuBWj2/KcxflXECmsjitKHLK6H7kFEiuJql+NAHU", - "U9EFXepLBQKBgQDQ+HCnZ1bFHiiP8m7Zl9EGlvK5SwlnPV9s+F1KJ4IGhCNM09UM", - "ZVfgR9F5yCONyIrPiyK40ylgtwqQJlOcf281I8irUXpsfg7+Gou5Q31y0r9NLUpC", - "Td8niyePtqMdGjouxD2+OHXFCd+FRxFt4IMi7vnxYr0csAVAXkqWlw7PsQKBgH/G", - "/PnQm7GM3BrOwAGB8dksJDAddkshMScblezTDYP0V43b8firkTLliCo5iNum357/", - "VQmdSEhXyag07yR/Kklg3H2fpbZQ3X7tdMMXW3FcWagfwWw9C4oGtdDM/Z1Lv23J", - "XDR9je8QV4OBGul+Jl8RfYx3kG94ZIfo8Qt0vP5hAoGARjAzdCGYz42NwaUk8n94", - "W2RuKHtTV9vtjaAbfPFbZoGkT7sXNJVlrA0C+9f+H9rOTM3mX59KrjmLVzde4Vhs", - "avWMShuK4vpAiDQLU7GyABvi5CR6Ld+AT+LSzxHhVe0ASOQPNCA2SOz3RQvgPi7R", - "GDgRMUB6cL3IRVzcR0dC6cY=", - )) - .unwrap(); - - let public_key = STANDARD - .decode(concat!( - "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEArvcXfr5pCD6KhzXo7BWc", - "5Hdcbgp9U6hk0+wDYQBJ2yP8mlbd3GiN9JMFAtliE6BaTYLuxI9Mdk7XmDoKy63X", - "AuI8tUon5imL/792Wca3f3qrbZh9pOfPKWp7HkcByty1ZO8QPlEYUP24y4DzOfVd", - "LkdZfs9X5qKHiTxc+VklzTm3PSap4eORTQ/lP1GB10y0qJk5+44GRcSQSr3ku6ui", - "2re8AJ2GQhdnZz5oWaCb/kij5bQPBwBrIEBlgRdaeasVdR6wFJPJAQZxtqWo9MPK", - "eVDOkaQ3Qrryh+49S4rln3592/WeHYM5hO47DJr86ELcqcyCmksYas7xTqHfVfHS", - "XQIDAQAB", - )) - .unwrap(); - - let private_key = AsymmetricCryptoKey::from_der(&private_key).unwrap(); - let public_key = AsymmetricPublicCryptoKey::from_der(&public_key).unwrap(); + let private_key = SensitiveString::test(concat!( + "MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCu9xd+vmkIPoqH", + "NejsFZzkd1xuCn1TqGTT7ANhAEnbI/yaVt3caI30kwUC2WIToFpNgu7Ej0x2TteY", + "OgrLrdcC4jy1SifmKYv/v3ZZxrd/eqttmH2k588panseRwHK3LVk7xA+URhQ/bjL", + "gPM59V0uR1l+z1fmooeJPFz5WSXNObc9Jqnh45FND+U/UYHXTLSomTn7jgZFxJBK", + "veS7q6Lat7wAnYZCF2dnPmhZoJv+SKPltA8HAGsgQGWBF1p5qxV1HrAUk8kBBnG2", + "paj0w8p5UM6RpDdCuvKH7j1LiuWffn3b9Z4dgzmE7jsMmvzoQtypzIKaSxhqzvFO", + "od9V8dJdAgMBAAECggEAGGIYjOIB1rOKkDHP4ljXutI0mCRPl3FMDemiBeppoIfZ", + "G/Q3qpAKmndDt0Quwh/yfcNdvZhf1kwCCTWri/uPz5fSUIyDV3TaTRu0ZWoHaBVj", + "Hxylg+4HRZUQj+Vi50/PWr/jQmAAVMcrMfcoTl82q2ynmP/R1vM3EsXOCjTliv5B", + "XlMPRjj/9PDBH0dnnVcAPDOpflzOTL2f4HTFEMlmg9/tZBnd96J/cmfhjAv9XpFL", + "FBAFZzs5pz0rwCNSR8QZNonnK7pngVUlGDLORK58y84tGmxZhGdne3CtCWey/sJ4", + "7QF0Pe8YqWBU56926IY6DcSVBuQGZ6vMCNlU7J8D2QKBgQDXyh3t2TicM/n1QBLk", + "zLoGmVUmxUGziHgl2dnJiGDtyOAU3+yCorPgFaCie29s5qm4b0YEGxUxPIrRrEro", + "h0FfKn9xmr8CdmTPTcjJW1+M7bxxq7oBoU/QzKXgIHlpeCjjnvPJt0PcNkNTjCXv", + "shsrINh2rENoe/x79eEfM/N5eQKBgQDPkYSmYyALoNq8zq0A4BdR+F5lb5Fj5jBH", + "Jk68l6Uti+0hRbJ2d1tQTLkU+eCPQLGBl6fuc1i4K5FV7v14jWtRPdD7wxrkRi3j", + "ilqQwLBOU6Bj3FK4DvlLF+iYTuBWj2/KcxflXECmsjitKHLK6H7kFEiuJql+NAHU", + "U9EFXepLBQKBgQDQ+HCnZ1bFHiiP8m7Zl9EGlvK5SwlnPV9s+F1KJ4IGhCNM09UM", + "ZVfgR9F5yCONyIrPiyK40ylgtwqQJlOcf281I8irUXpsfg7+Gou5Q31y0r9NLUpC", + "Td8niyePtqMdGjouxD2+OHXFCd+FRxFt4IMi7vnxYr0csAVAXkqWlw7PsQKBgH/G", + "/PnQm7GM3BrOwAGB8dksJDAddkshMScblezTDYP0V43b8firkTLliCo5iNum357/", + "VQmdSEhXyag07yR/Kklg3H2fpbZQ3X7tdMMXW3FcWagfwWw9C4oGtdDM/Z1Lv23J", + "XDR9je8QV4OBGul+Jl8RfYx3kG94ZIfo8Qt0vP5hAoGARjAzdCGYz42NwaUk8n94", + "W2RuKHtTV9vtjaAbfPFbZoGkT7sXNJVlrA0C+9f+H9rOTM3mX59KrjmLVzde4Vhs", + "avWMShuK4vpAiDQLU7GyABvi5CR6Ld+AT+LSzxHhVe0ASOQPNCA2SOz3RQvgPi7R", + "GDgRMUB6cL3IRVzcR0dC6cY=", + )) + .decode_base64(STANDARD) + .unwrap(); + + let public_key = SensitiveString::test(concat!( + "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEArvcXfr5pCD6KhzXo7BWc", + "5Hdcbgp9U6hk0+wDYQBJ2yP8mlbd3GiN9JMFAtliE6BaTYLuxI9Mdk7XmDoKy63X", + "AuI8tUon5imL/792Wca3f3qrbZh9pOfPKWp7HkcByty1ZO8QPlEYUP24y4DzOfVd", + "LkdZfs9X5qKHiTxc+VklzTm3PSap4eORTQ/lP1GB10y0qJk5+44GRcSQSr3ku6ui", + "2re8AJ2GQhdnZz5oWaCb/kij5bQPBwBrIEBlgRdaeasVdR6wFJPJAQZxtqWo9MPK", + "eVDOkaQ3Qrryh+49S4rln3592/WeHYM5hO47DJr86ELcqcyCmksYas7xTqHfVfHS", + "XQIDAQAB", + )) + .decode_base64(STANDARD) + .unwrap(); + + let private_key = AsymmetricCryptoKey::from_der(private_key).unwrap(); + let public_key = AsymmetricPublicCryptoKey::from_der(public_key).unwrap(); let plaintext = "Hello, world!"; let encrypted = diff --git a/crates/bitwarden-crypto/src/keys/device_key.rs b/crates/bitwarden-crypto/src/keys/device_key.rs index 65f51a259..898e0ba8c 100644 --- a/crates/bitwarden-crypto/src/keys/device_key.rs +++ b/crates/bitwarden-crypto/src/keys/device_key.rs @@ -1,8 +1,6 @@ -use std::str::FromStr; - use crate::{ error::Result, AsymmetricCryptoKey, AsymmetricEncString, CryptoError, DecryptedVec, EncString, - KeyDecryptable, KeyEncryptable, SymmetricCryptoKey, + KeyDecryptable, KeyEncryptable, SensitiveString, SymmetricCryptoKey, }; /// Device Key @@ -40,7 +38,7 @@ impl DeviceKey { let data = user_key.to_vec(); let protected_user_key = - AsymmetricEncString::encrypt_rsa2048_oaep_sha1(&data, &device_private_key)?; + AsymmetricEncString::encrypt_rsa2048_oaep_sha1(data.expose(), &device_private_key)?; let protected_device_public_key = device_private_key .to_public_der()? @@ -51,7 +49,7 @@ impl DeviceKey { .encrypt_with_key(&device_key.0)?; Ok(TrustDeviceResponse { - device_key: device_key.to_base64(), + device_key: device_key.to_base64().expose().to_owned(), protected_user_key, protected_device_private_key, protected_device_public_key, @@ -66,26 +64,24 @@ impl DeviceKey { ) -> Result { let device_private_key: DecryptedVec = protected_device_private_key.decrypt_with_key(&self.0)?; - let device_private_key = - AsymmetricCryptoKey::from_der(device_private_key.expose().as_slice())?; + let device_private_key = AsymmetricCryptoKey::from_der(device_private_key)?; - let mut dec: DecryptedVec = protected_user_key.decrypt_with_key(&device_private_key)?; - let user_key: SymmetricCryptoKey = dec.expose_mut().as_mut_slice().try_into()?; + let dec: DecryptedVec = protected_user_key.decrypt_with_key(&device_private_key)?; + let user_key = SymmetricCryptoKey::try_from(dec)?; Ok(user_key) } - fn to_base64(&self) -> String { + fn to_base64(&self) -> SensitiveString { self.0.to_base64() } } -impl FromStr for DeviceKey { - type Err = CryptoError; +impl TryFrom for DeviceKey { + type Error = CryptoError; - fn from_str(s: &str) -> Result { - let key = s.parse::()?; - Ok(DeviceKey(key)) + fn try_from(value: SensitiveString) -> Result { + SymmetricCryptoKey::try_from(value).map(DeviceKey) } } @@ -100,10 +96,9 @@ mod tests { let result = DeviceKey::trust_device(&key).unwrap(); - let decrypted = result - .device_key - .parse::() - .unwrap() + let device_key = SensitiveString::new(Box::new(result.device_key)); + let device_key = DeviceKey::try_from(device_key).unwrap(); + let decrypted = device_key .decrypt_user_key( result.protected_device_private_key, result.protected_user_key, diff --git a/crates/bitwarden-crypto/src/keys/master_key.rs b/crates/bitwarden-crypto/src/keys/master_key.rs index 2603093e7..792bda949 100644 --- a/crates/bitwarden-crypto/src/keys/master_key.rs +++ b/crates/bitwarden-crypto/src/keys/master_key.rs @@ -61,15 +61,15 @@ impl MasterKey { pub fn decrypt_user_key(&self, user_key: EncString) -> Result { let stretched_key = stretch_kdf_key(&self.0)?; - let mut dec: DecryptedVec = user_key.decrypt_with_key(&stretched_key)?; - SymmetricCryptoKey::try_from(dec.expose_mut().as_mut_slice()) + let dec: DecryptedVec = user_key.decrypt_with_key(&stretched_key)?; + SymmetricCryptoKey::try_from(dec) } pub fn encrypt_user_key(&self, user_key: &SymmetricCryptoKey) -> Result { let stretched_key = stretch_kdf_key(&self.0)?; EncString::encrypt_aes256_hmac( - user_key.to_vec().as_slice(), + user_key.to_vec().expose(), stretched_key.mac_key.as_ref().unwrap(), &stretched_key.key, ) diff --git a/crates/bitwarden-crypto/src/keys/shareable_key.rs b/crates/bitwarden-crypto/src/keys/shareable_key.rs index fbb2d2e44..2b9ec837f 100644 --- a/crates/bitwarden-crypto/src/keys/shareable_key.rs +++ b/crates/bitwarden-crypto/src/keys/shareable_key.rs @@ -37,9 +37,9 @@ mod tests { #[test] fn test_derive_shareable_key() { let key = derive_shareable_key(*b"&/$%F1a895g67HlX", "test_key", None); - assert_eq!(key.to_base64(), "4PV6+PcmF2w7YHRatvyMcVQtI7zvCyssv/wFWmzjiH6Iv9altjmDkuBD1aagLVaLezbthbSe+ktR+U6qswxNnQ=="); + assert_eq!(key.to_base64().expose(), "4PV6+PcmF2w7YHRatvyMcVQtI7zvCyssv/wFWmzjiH6Iv9altjmDkuBD1aagLVaLezbthbSe+ktR+U6qswxNnQ=="); let key = derive_shareable_key(*b"67t9b5g67$%Dh89n", "test_key", Some("test")); - assert_eq!(key.to_base64(), "F9jVQmrACGx9VUPjuzfMYDjr726JtL300Y3Yg+VYUnVQtQ1s8oImJ5xtp1KALC9h2nav04++1LDW4iFD+infng=="); + assert_eq!(key.to_base64().expose(), "F9jVQmrACGx9VUPjuzfMYDjr726JtL300Y3Yg+VYUnVQtQ1s8oImJ5xtp1KALC9h2nav04++1LDW4iFD+infng=="); } } diff --git a/crates/bitwarden-crypto/src/keys/symmetric_crypto_key.rs b/crates/bitwarden-crypto/src/keys/symmetric_crypto_key.rs index 0c165bb40..23d09cbca 100644 --- a/crates/bitwarden-crypto/src/keys/symmetric_crypto_key.rs +++ b/crates/bitwarden-crypto/src/keys/symmetric_crypto_key.rs @@ -1,13 +1,13 @@ -use std::{pin::Pin, str::FromStr}; +use std::pin::Pin; use aes::cipher::typenum::U32; -use base64::{engine::general_purpose::STANDARD, Engine}; +use base64::engine::general_purpose::STANDARD; use generic_array::GenericArray; use rand::Rng; -use zeroize::{Zeroize, Zeroizing}; +use zeroize::Zeroize; use super::key_encryptable::CryptoKey; -use crate::CryptoError; +use crate::{CryptoError, SensitiveString, SensitiveVec}; /// A symmetric encryption key. Used to encrypt and decrypt [`EncString`](crate::EncString) pub struct SymmetricCryptoKey { @@ -59,31 +59,35 @@ impl SymmetricCryptoKey { self.key.len() + self.mac_key.as_ref().map_or(0, |mac| mac.len()) } - pub fn to_base64(&self) -> String { - let mut buf = self.to_vec(); - - let result = STANDARD.encode(&buf); - buf.zeroize(); - result + pub fn to_base64(&self) -> SensitiveString { + self.to_vec().encode_base64(STANDARD) } - pub fn to_vec(&self) -> Zeroizing> { - let mut buf = Vec::with_capacity(self.total_len()); + pub fn to_vec(&self) -> SensitiveVec { + let mut buf = SensitiveVec::new(Box::new(Vec::with_capacity(self.total_len()))); - buf.extend_from_slice(&self.key); + buf.expose_mut().extend_from_slice(&self.key); if let Some(mac) = &self.mac_key { - buf.extend_from_slice(mac); + buf.expose_mut().extend_from_slice(mac); } - Zeroizing::new(buf) + buf } } -impl FromStr for SymmetricCryptoKey { - type Err = CryptoError; +impl TryFrom for SymmetricCryptoKey { + type Error = CryptoError; - fn from_str(s: &str) -> Result { - let mut bytes = STANDARD.decode(s).map_err(|_| CryptoError::InvalidKey)?; - SymmetricCryptoKey::try_from(bytes.as_mut_slice()) + fn try_from(value: SensitiveString) -> Result { + SymmetricCryptoKey::try_from(value.decode_base64(STANDARD)?) + } +} + +impl TryFrom for SymmetricCryptoKey { + type Error = CryptoError; + + fn try_from(mut value: SensitiveVec) -> Result { + let val = value.expose_mut(); + SymmetricCryptoKey::try_from(val.as_mut_slice()) } } @@ -138,19 +142,18 @@ pub fn derive_symmetric_key(name: &str) -> SymmetricCryptoKey { #[cfg(test)] mod tests { - use std::str::FromStr; - use super::{derive_symmetric_key, SymmetricCryptoKey}; + use crate::SensitiveString; #[test] fn test_symmetric_crypto_key() { let key = derive_symmetric_key("test"); - let key2 = SymmetricCryptoKey::from_str(&key.to_base64()).unwrap(); + let key2 = SymmetricCryptoKey::try_from(key.to_base64()).unwrap(); assert_eq!(key.key, key2.key); assert_eq!(key.mac_key, key2.mac_key); - let key = "UY4B5N4DA4UisCNClgZtRr6VLy9ZF5BXXC7cDZRqourKi4ghEMgISbCsubvgCkHf5DZctQjVot11/vVvN9NNHQ=="; - let key2 = SymmetricCryptoKey::from_str(key).unwrap(); + let key = SensitiveString::test("UY4B5N4DA4UisCNClgZtRr6VLy9ZF5BXXC7cDZRqourKi4ghEMgISbCsubvgCkHf5DZctQjVot11/vVvN9NNHQ=="); + let key2 = SymmetricCryptoKey::try_from(key.clone()).unwrap(); assert_eq!(key, key2.to_base64()); } } diff --git a/crates/bitwarden-crypto/src/sensitive/mod.rs b/crates/bitwarden-crypto/src/sensitive/mod.rs index f38368580..0da2329b9 100644 --- a/crates/bitwarden-crypto/src/sensitive/mod.rs +++ b/crates/bitwarden-crypto/src/sensitive/mod.rs @@ -1,3 +1,4 @@ +#[allow(clippy::module_inception)] mod sensitive; pub use sensitive::{Sensitive, SensitiveString, SensitiveVec}; mod decrypted; diff --git a/crates/bitwarden-crypto/src/sensitive/sensitive.rs b/crates/bitwarden-crypto/src/sensitive/sensitive.rs index 7a3c202a5..67bbb180a 100644 --- a/crates/bitwarden-crypto/src/sensitive/sensitive.rs +++ b/crates/bitwarden-crypto/src/sensitive/sensitive.rs @@ -58,6 +58,35 @@ impl TryFrom for SensitiveString { } } +impl SensitiveString { + pub fn decode_base64(self, engine: T) -> Result { + // Preallocate a Vec with the necessary capacity + 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) + } +} + +impl SensitiveVec { + pub fn encode_base64(self, engine: T) -> SensitiveString { + use base64::engine::Config; + + // Preallocate a String with the necessary capacity + 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))); + + engine.encode_string(self.value.as_ref(), &mut value.value); + + value + } +} + impl Default for Sensitive { fn default() -> Self { Self::new(Box::default()) diff --git a/crates/bitwarden-exporters/src/csv.rs b/crates/bitwarden-exporters/src/csv.rs index ac6995588..a1ebae5db 100644 --- a/crates/bitwarden-exporters/src/csv.rs +++ b/crates/bitwarden-exporters/src/csv.rs @@ -103,11 +103,11 @@ where "{}: {}", f.name .as_ref() - .map(|n| n.expose().to_owned()) + .map(|n| n.expose().as_str()) .unwrap_or_default(), f.value .as_ref() - .map(|n| n.expose().to_owned()) + .map(|n| n.expose().as_str()) .unwrap_or_default(), ) }) diff --git a/crates/bitwarden/src/auth/auth_request.rs b/crates/bitwarden/src/auth/auth_request.rs index 84570fe4c..3532b40b4 100644 --- a/crates/bitwarden/src/auth/auth_request.rs +++ b/crates/bitwarden/src/auth/auth_request.rs @@ -1,6 +1,7 @@ use base64::{engine::general_purpose::STANDARD, Engine}; use bitwarden_crypto::{ fingerprint, AsymmetricCryptoKey, AsymmetricEncString, AsymmetricPublicCryptoKey, + SensitiveString, }; #[cfg(feature = "mobile")] use bitwarden_crypto::{EncString, KeyDecryptable, SymmetricCryptoKey}; @@ -59,12 +60,11 @@ pub(crate) fn auth_request_decrypt_user_key( ) -> Result { use bitwarden_crypto::DecryptedVec; - let key = AsymmetricCryptoKey::from_der(&STANDARD.decode(private_key)?)?; - let mut key: DecryptedVec = user_key.decrypt_with_key(&key)?; + let private_key = SensitiveString::new(Box::new(private_key)); + let key = AsymmetricCryptoKey::from_der(private_key.decode_base64(STANDARD)?)?; + let key: DecryptedVec = user_key.decrypt_with_key(&key)?; - Ok(SymmetricCryptoKey::try_from( - key.expose_mut().as_mut_slice(), - )?) + Ok(SymmetricCryptoKey::try_from(key)?) } /// Decrypt the user key using the private key generated previously. @@ -76,11 +76,10 @@ pub(crate) fn auth_request_decrypt_master_key( ) -> Result { use bitwarden_crypto::{DecryptedVec, MasterKey}; - let key = AsymmetricCryptoKey::from_der(&STANDARD.decode(private_key)?)?; - let mut master_key: DecryptedVec = master_key.decrypt_with_key(&key)?; - let master_key = MasterKey::new(SymmetricCryptoKey::try_from( - master_key.expose_mut().as_mut_slice(), - )?); + let private_key = SensitiveString::new(Box::new(private_key)); + let key = AsymmetricCryptoKey::from_der(private_key.decode_base64(STANDARD)?)?; + let master_key: DecryptedVec = master_key.decrypt_with_key(&key)?; + let master_key = MasterKey::new(SymmetricCryptoKey::try_from(master_key)?); Ok(master_key.decrypt_user_key(user_key)?) } @@ -92,21 +91,20 @@ pub(crate) fn approve_auth_request( client: &mut Client, public_key: String, ) -> Result { - let public_key = AsymmetricPublicCryptoKey::from_der(&STANDARD.decode(public_key)?)?; + let public_key = SensitiveString::new(Box::new(public_key)); + let public_key = AsymmetricPublicCryptoKey::from_der(public_key.decode_base64(STANDARD)?)?; let enc = client.get_encryption_settings()?; let key = enc.get_key(&None).ok_or(Error::VaultLocked)?; Ok(AsymmetricEncString::encrypt_rsa2048_oaep_sha1( - &key.to_vec(), + key.to_vec().expose(), &public_key, )?) } #[test] fn test_auth_request() { - use zeroize::Zeroizing; - let request = new_auth_request("test@bitwarden.com").unwrap(); let secret: &[u8] = &[ @@ -116,14 +114,15 @@ fn test_auth_request() { 67, 35, 61, 245, 93, ]; + let private_key = SensitiveString::new(Box::new(request.private_key.clone())); let private_key = - AsymmetricCryptoKey::from_der(&STANDARD.decode(&request.private_key).unwrap()).unwrap(); + AsymmetricCryptoKey::from_der(private_key.decode_base64(STANDARD).unwrap()).unwrap(); let encrypted = AsymmetricEncString::encrypt_rsa2048_oaep_sha1(secret, &private_key).unwrap(); let decrypted = auth_request_decrypt_user_key(request.private_key, encrypted).unwrap(); - assert_eq!(decrypted.to_vec(), Zeroizing::new(secret.to_owned())); + assert_eq!(decrypted.to_vec().expose(), secret); } #[cfg(test)] @@ -173,8 +172,8 @@ mod tests { let dec = auth_request_decrypt_user_key(private_key.to_owned(), enc_user_key).unwrap(); assert_eq!( - dec.to_vec().as_ref(), - vec![ + dec.to_vec().expose(), + &[ 201, 37, 234, 213, 21, 75, 40, 70, 149, 213, 234, 16, 19, 251, 162, 245, 161, 74, 34, 245, 211, 151, 211, 192, 95, 10, 117, 50, 88, 223, 23, 157 ] @@ -192,8 +191,8 @@ mod tests { .unwrap(); assert_eq!( - dec.to_vec().as_ref(), - vec![ + dec.to_vec().expose(), + &[ 109, 128, 172, 147, 206, 123, 134, 95, 16, 36, 155, 113, 201, 18, 186, 230, 216, 212, 173, 188, 74, 11, 134, 131, 137, 242, 105, 178, 105, 126, 52, 139, 248, 91, 215, 21, 128, 91, 226, 222, 165, 67, 251, 34, 83, 81, 77, 147, 225, 76, 13, 41, diff --git a/crates/bitwarden/src/auth/login/access_token.rs b/crates/bitwarden/src/auth/login/access_token.rs index dd45a71c3..60570f421 100644 --- a/crates/bitwarden/src/auth/login/access_token.rs +++ b/crates/bitwarden/src/auth/login/access_token.rs @@ -1,7 +1,9 @@ use std::path::{Path, PathBuf}; -use base64::{engine::general_purpose::STANDARD, Engine}; -use bitwarden_crypto::{DecryptedVec, EncString, KeyDecryptable, SymmetricCryptoKey}; +use base64::engine::general_purpose::STANDARD; +use bitwarden_crypto::{ + DecryptedVec, EncString, KeyDecryptable, SensitiveString, SymmetricCryptoKey, +}; use chrono::Utc; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -60,12 +62,12 @@ pub(crate) async fn login_access_token( #[derive(serde::Deserialize)] struct Payload { #[serde(rename = "encryptionKey")] - encryption_key: String, + encryption_key: SensitiveString, } let payload: Payload = serde_json::from_slice(decrypted_payload.expose())?; - let mut encryption_key = STANDARD.decode(payload.encryption_key.clone())?; - let encryption_key = SymmetricCryptoKey::try_from(encryption_key.as_mut_slice())?; + let encryption_key = payload.encryption_key.clone().decode_base64(STANDARD)?; + let encryption_key = SymmetricCryptoKey::try_from(encryption_key)?; let access_token_obj: JWTToken = r.access_token.parse()?; @@ -126,7 +128,7 @@ fn load_tokens_from_state( let organization_id: Uuid = organization_id .parse() .map_err(|_| "Bad organization id.")?; - let encryption_key: SymmetricCryptoKey = client_state.encryption_key.parse()?; + let encryption_key = SymmetricCryptoKey::try_from(client_state.encryption_key)?; client.set_tokens(client_state.token, None, time_till_expiration as u64); client.initialize_crypto_single_key(encryption_key); diff --git a/crates/bitwarden/src/client/access_token.rs b/crates/bitwarden/src/client/access_token.rs index d7cad2fca..0b0d0def3 100644 --- a/crates/bitwarden/src/client/access_token.rs +++ b/crates/bitwarden/src/client/access_token.rs @@ -79,7 +79,7 @@ mod tests { "ec2c1d46-6a4b-4751-a310-af9601317f2d" ); assert_eq!(token.client_secret, "C2IgxjjLF7qSshsbwe8JGcbM075YXw"); - assert_eq!(token.encryption_key.to_base64(), "H9/oIRLtL9nGCQOVDjSMoEbJsjWXSOCb3qeyDt6ckzS3FhyboEDWyTP/CQfbIszNmAVg2ExFganG1FVFGXO/Jg=="); + assert_eq!(token.encryption_key.to_base64().expose(), "H9/oIRLtL9nGCQOVDjSMoEbJsjWXSOCb3qeyDt6ckzS3FhyboEDWyTP/CQfbIszNmAVg2ExFganG1FVFGXO/Jg=="); } #[test] diff --git a/crates/bitwarden/src/client/encryption_settings.rs b/crates/bitwarden/src/client/encryption_settings.rs index 6bd474c24..81110b24b 100644 --- a/crates/bitwarden/src/client/encryption_settings.rs +++ b/crates/bitwarden/src/client/encryption_settings.rs @@ -58,7 +58,7 @@ impl EncryptionSettings { let private_key = { let dec: DecryptedVec = private_key.decrypt_with_key(&user_key)?; - Some(AsymmetricCryptoKey::from_der(dec.expose())?) + Some(AsymmetricCryptoKey::from_der(dec)?) }; Ok(EncryptionSettings { @@ -95,9 +95,9 @@ impl EncryptionSettings { // Decrypt the org keys with the private key for (org_id, org_enc_key) in org_enc_keys { - let mut dec: DecryptedVec = org_enc_key.decrypt_with_key(private_key)?; + let dec: DecryptedVec = org_enc_key.decrypt_with_key(private_key)?; - let org_key = SymmetricCryptoKey::try_from(dec.expose_mut().as_mut_slice())?; + let org_key = SymmetricCryptoKey::try_from(dec)?; self.org_keys.insert(org_id, org_key); } diff --git a/crates/bitwarden/src/mobile/crypto.rs b/crates/bitwarden/src/mobile/crypto.rs index 1a9b5d073..f1de334a8 100644 --- a/crates/bitwarden/src/mobile/crypto.rs +++ b/crates/bitwarden/src/mobile/crypto.rs @@ -86,7 +86,7 @@ pub enum AuthRequestMethod { #[cfg(feature = "internal")] pub async fn initialize_user_crypto(client: &mut Client, req: InitUserCryptoRequest) -> Result<()> { - use bitwarden_crypto::DeviceKey; + use bitwarden_crypto::{DeviceKey, SensitiveString}; use crate::auth::{auth_request_decrypt_master_key, auth_request_decrypt_user_key}; @@ -105,7 +105,8 @@ pub async fn initialize_user_crypto(client: &mut Client, req: InitUserCryptoRequ client.initialize_user_crypto(&password, user_key, private_key)?; } InitUserCryptoMethod::DecryptedKey { decrypted_user_key } => { - let user_key = decrypted_user_key.parse::()?; + let decrypted_user_key = SensitiveString::new(Box::new(decrypted_user_key)); + let user_key = SymmetricCryptoKey::try_from(decrypted_user_key)?; client.initialize_user_crypto_decrypted_key(user_key, private_key)?; } InitUserCryptoMethod::Pin { @@ -138,7 +139,8 @@ pub async fn initialize_user_crypto(client: &mut Client, req: InitUserCryptoRequ protected_device_private_key, device_protected_user_key, } => { - let device_key = device_key.parse::()?; + let device_key = SensitiveString::new(Box::new(device_key)); + let device_key = DeviceKey::try_from(device_key)?; let user_key = device_key .decrypt_user_key(protected_device_private_key, device_protected_user_key)?; @@ -172,7 +174,7 @@ pub async fn get_user_encryption_key(client: &mut Client) -> Result { .get_key(&None) .ok_or(Error::VaultLocked)?; - Ok(user_key.to_base64()) + Ok(user_key.to_base64().expose().to_owned()) } #[cfg(feature = "internal")] @@ -293,15 +295,16 @@ pub(super) fn enroll_admin_password_reset( client: &mut Client, public_key: String, ) -> Result { - use base64::{engine::general_purpose::STANDARD, Engine}; - use bitwarden_crypto::AsymmetricPublicCryptoKey; + use base64::engine::general_purpose::STANDARD; + use bitwarden_crypto::{AsymmetricPublicCryptoKey, SensitiveString}; - let public_key = AsymmetricPublicCryptoKey::from_der(&STANDARD.decode(public_key)?)?; + let public_key = SensitiveString::new(Box::new(public_key)); + let public_key = AsymmetricPublicCryptoKey::from_der(public_key.decode_base64(STANDARD)?)?; let enc = client.get_encryption_settings()?; let key = enc.get_key(&None).ok_or(Error::VaultLocked)?; Ok(AsymmetricEncString::encrypt_rsa2048_oaep_sha1( - &key.to_vec(), + key.to_vec().expose(), &public_key, )?) } @@ -485,10 +488,10 @@ mod tests { #[cfg(feature = "internal")] #[test] fn test_enroll_admin_password_reset() { - use std::{num::NonZeroU32, ops::Deref}; + use std::num::NonZeroU32; - use base64::{engine::general_purpose::STANDARD, Engine}; - use bitwarden_crypto::{AsymmetricCryptoKey, DecryptedVec}; + use base64::engine::general_purpose::STANDARD; + use bitwarden_crypto::{AsymmetricCryptoKey, DecryptedVec, SensitiveString}; let mut client = Client::new(None); client.set_login_method(LoginMethod::User(UserLoginMethod::Username { @@ -510,8 +513,9 @@ mod tests { let encrypted = enroll_admin_password_reset(&mut client, public_key.to_owned()).unwrap(); 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 private_key = SensitiveString::test(private_key); let private_key = - AsymmetricCryptoKey::from_der(&STANDARD.decode(private_key).unwrap()).unwrap(); + AsymmetricCryptoKey::from_der(private_key.decode_base64(STANDARD).unwrap()).unwrap(); let decrypted: DecryptedVec = encrypted.decrypt_with_key(&private_key).unwrap(); let expected = client @@ -519,9 +523,7 @@ mod tests { .unwrap() .get_key(&None) .unwrap() - .to_vec() - .deref() - .clone(); - assert_eq!(decrypted.expose(), &expected); + .to_vec(); + assert_eq!(decrypted.expose(), expected.expose()); } } diff --git a/crates/bitwarden/src/secrets_manager/state.rs b/crates/bitwarden/src/secrets_manager/state.rs index eaea08365..00f25bedd 100644 --- a/crates/bitwarden/src/secrets_manager/state.rs +++ b/crates/bitwarden/src/secrets_manager/state.rs @@ -1,6 +1,8 @@ use std::{fmt::Debug, path::Path}; -use bitwarden_crypto::{DecryptedString, EncString, KeyDecryptable, KeyEncryptable}; +use bitwarden_crypto::{ + DecryptedString, EncString, KeyDecryptable, KeyEncryptable, SensitiveString, +}; use serde::{Deserialize, Serialize}; use crate::{ @@ -15,11 +17,11 @@ const STATE_VERSION: u32 = 1; pub struct ClientState { pub(crate) version: u32, pub(crate) token: String, - pub(crate) encryption_key: String, + pub(crate) encryption_key: SensitiveString, } impl ClientState { - pub fn new(token: String, encryption_key: String) -> Self { + pub fn new(token: String, encryption_key: SensitiveString) -> Self { Self { version: STATE_VERSION, token, diff --git a/crates/bitwarden/src/vault/cipher/attachment.rs b/crates/bitwarden/src/vault/cipher/attachment.rs index 13db1cd84..7219fe479 100644 --- a/crates/bitwarden/src/vault/cipher/attachment.rs +++ b/crates/bitwarden/src/vault/cipher/attachment.rs @@ -67,7 +67,12 @@ impl<'a> KeyEncryptable for Attachm // with it, and then encrypt the key with the cipher key let attachment_key = SymmetricCryptoKey::generate(rand::thread_rng()); let encrypted_contents = self.contents.encrypt_with_key(&attachment_key)?; - attachment.key = Some(attachment_key.to_vec().encrypt_with_key(ciphers_key)?); + attachment.key = Some( + attachment_key + .to_vec() + .expose() + .encrypt_with_key(ciphers_key)?, + ); Ok(AttachmentEncryptResult { attachment: attachment.encrypt_with_key(ciphers_key)?, @@ -81,14 +86,13 @@ impl KeyDecryptable for AttachmentFile { let ciphers_key = Cipher::get_cipher_key(key, &self.cipher.key)?; let ciphers_key = ciphers_key.as_ref().unwrap_or(key); - let mut attachment_key: DecryptedVec = self + let attachment_key: DecryptedVec = self .attachment .key .as_ref() .ok_or(CryptoError::MissingKey)? .decrypt_with_key(ciphers_key)?; - let attachment_key = - SymmetricCryptoKey::try_from(attachment_key.expose_mut().as_mut_slice())?; + let attachment_key = SymmetricCryptoKey::try_from(attachment_key)?; self.contents.decrypt_with_key(&attachment_key) } @@ -138,7 +142,7 @@ impl TryFrom for Attachment #[cfg(test)] mod tests { use base64::{engine::general_purpose::STANDARD, Engine}; - use bitwarden_crypto::{EncString, KeyDecryptable, SymmetricCryptoKey}; + use bitwarden_crypto::{EncString, KeyDecryptable, SensitiveString, SymmetricCryptoKey}; use crate::vault::{ cipher::cipher::{CipherRepromptType, CipherType}, @@ -147,7 +151,7 @@ mod tests { #[test] fn test_attachment_key() { - let user_key : SymmetricCryptoKey = "w2LO+nwV4oxwswVYCxlOfRUseXfvU03VzvKQHrqeklPgiMZrspUe6sOBToCnDn9Ay0tuCBn8ykVVRb7PWhub2Q==".parse().unwrap(); + let user_key = SymmetricCryptoKey::try_from(SensitiveString::test("w2LO+nwV4oxwswVYCxlOfRUseXfvU03VzvKQHrqeklPgiMZrspUe6sOBToCnDn9Ay0tuCBn8ykVVRb7PWhub2Q==")).unwrap(); let attachment = Attachment { id: None, diff --git a/crates/bitwarden/src/vault/cipher/cipher.rs b/crates/bitwarden/src/vault/cipher/cipher.rs index 865ae778d..898da568c 100644 --- a/crates/bitwarden/src/vault/cipher/cipher.rs +++ b/crates/bitwarden/src/vault/cipher/cipher.rs @@ -218,8 +218,8 @@ impl Cipher { ciphers_key .as_ref() .map(|k| { - let mut key: DecryptedVec = k.decrypt_with_key(key)?; - SymmetricCryptoKey::try_from(key.expose_mut().as_mut_slice()) + let key: DecryptedVec = k.decrypt_with_key(key)?; + SymmetricCryptoKey::try_from(key) }) .transpose() } @@ -301,7 +301,7 @@ impl CipherView { let new_key = SymmetricCryptoKey::generate(rand::thread_rng()); - self.key = Some(new_key.to_vec().encrypt_with_key(key)?); + self.key = Some(new_key.to_vec().expose().encrypt_with_key(key)?); Ok(()) } } diff --git a/crates/bitwarden/src/vault/send.rs b/crates/bitwarden/src/vault/send.rs index 09706e951..14ea297b1 100644 --- a/crates/bitwarden/src/vault/send.rs +++ b/crates/bitwarden/src/vault/send.rs @@ -394,7 +394,7 @@ mod tests { // Get the send key let send_key = Send::get_key(&send_key, k).unwrap(); let send_key_b64 = send_key.to_base64(); - assert_eq!(send_key_b64, "IR9ImHGm6rRuIjiN7csj94bcZR5WYTJj5GtNfx33zm6tJCHUl+QZlpNPba8g2yn70KnOHsAODLcR0um6E3MAlg=="); + assert_eq!(send_key_b64.expose(), "IR9ImHGm6rRuIjiN7csj94bcZR5WYTJj5GtNfx33zm6tJCHUl+QZlpNPba8g2yn70KnOHsAODLcR0um6E3MAlg=="); } fn build_encryption_settings() -> EncryptionSettings { From ef8ba375fdc0b5e5e19dae61dd701b4a23215338 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garci=CC=81a?= Date: Mon, 11 Mar 2024 11:44:46 +0100 Subject: [PATCH 13/22] Make base64 memory test required --- crates/memory-testing/src/bin/analyze-dumps.rs | 4 +--- crates/memory-testing/src/main.rs | 9 +++++---- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/crates/memory-testing/src/bin/analyze-dumps.rs b/crates/memory-testing/src/bin/analyze-dumps.rs index fee72f2e5..9957bc1cd 100644 --- a/crates/memory-testing/src/bin/analyze-dumps.rs +++ b/crates/memory-testing/src/bin/analyze-dumps.rs @@ -115,9 +115,7 @@ fn main() -> io::Result<()> { mac_final_pos.is_empty(), ); - // TODO: At the moment we are not zeroizing the base64 key in from_str, so this test is - // ignored - add_row( + error |= add_row( &mut table, format!("Symm. Key in Base64, case {}", idx), &b64_initial_pos, diff --git a/crates/memory-testing/src/main.rs b/crates/memory-testing/src/main.rs index 96e4ae175..ee781683c 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, str::FromStr}; +use std::{env, io::Read, path::Path, process}; -use bitwarden_crypto::SymmetricCryptoKey; +use bitwarden_crypto::{SensitiveString, SymmetricCryptoKey}; fn wait_for_dump() { println!("Waiting for dump..."); @@ -22,8 +22,9 @@ fn main() { let mut symmetric_keys = Vec::new(); let mut symmetric_keys_as_vecs = Vec::new(); - for case in &cases.symmetric_key { - let key = SymmetricCryptoKey::from_str(&case.key).unwrap(); + 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); } From b87bd9520e43666609453cf67d288629eb3e6679 Mon Sep 17 00:00:00 2001 From: Hinton Date: Mon, 22 Apr 2024 15:15:16 +0200 Subject: [PATCH 14/22] Use ::test in tests --- crates/bitwarden/src/vault/send.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/bitwarden/src/vault/send.rs b/crates/bitwarden/src/vault/send.rs index 4648419f7..a5537d8e1 100644 --- a/crates/bitwarden/src/vault/send.rs +++ b/crates/bitwarden/src/vault/send.rs @@ -448,7 +448,7 @@ mod tests { let expected = SendView { id: "3d80dd72-2d14-4f26-812c-b0f0018aa144".parse().ok(), access_id: Some("ct2APRQtJk-BLLDwAYqhRA".to_owned()), - name: SensitiveString::new(Box::new("Test".to_string())), + name: SensitiveString::test("Test"), notes: None, key: Some("Pgui0FK85cNhBGWHAlBHBw".to_owned()), new_password: None, @@ -456,7 +456,7 @@ mod tests { r#type: SendType::Text, file: None, text: Some(SendTextView { - text: Some(SensitiveString::new(Box::new("This is a test".to_owned()))), + text: Some(SensitiveString::test("This is a test")), hidden: false, }), max_access_count: None, @@ -479,7 +479,7 @@ mod tests { let view = SendView { id: "3d80dd72-2d14-4f26-812c-b0f0018aa144".parse().ok(), access_id: Some("ct2APRQtJk-BLLDwAYqhRA".to_owned()), - name: SensitiveString::new(Box::new("Test".to_string())), + name: SensitiveString::test("Test"), notes: None, key: Some("Pgui0FK85cNhBGWHAlBHBw".to_owned()), new_password: None, @@ -487,7 +487,7 @@ mod tests { r#type: SendType::Text, file: None, text: Some(SendTextView { - text: Some(SensitiveString::new(Box::new("This is a test".to_owned()))), + text: Some(SensitiveString::test("This is a test")), hidden: false, }), max_access_count: None, @@ -517,7 +517,7 @@ mod tests { let view = SendView { id: None, access_id: Some("ct2APRQtJk-BLLDwAYqhRA".to_owned()), - name: SensitiveString::new(Box::new("Test".to_string())), + name: SensitiveString::test("Test"), notes: None, key: None, new_password: None, @@ -525,7 +525,7 @@ mod tests { r#type: SendType::Text, file: None, text: Some(SendTextView { - text: Some(SensitiveString::new(Box::new("This is a test".to_owned()))), + text: Some(SensitiveString::test("This is a test")), hidden: false, }), max_access_count: None, @@ -558,7 +558,7 @@ mod tests { let view = SendView { id: None, access_id: Some("ct2APRQtJk-BLLDwAYqhRA".to_owned()), - name: SensitiveString::new(Box::new("Test".to_owned())), + name: SensitiveString::test("Test"), notes: None, key: Some("Pgui0FK85cNhBGWHAlBHBw".to_owned()), new_password: Some("abc123".to_owned()), @@ -566,7 +566,7 @@ mod tests { r#type: SendType::Text, file: None, text: Some(SendTextView { - text: Some(SensitiveString::new(Box::new("This is a test".to_owned()))), + text: Some(SensitiveString::test("This is a test")), hidden: false, }), max_access_count: None, From 37b18ad58b2e9d5c22892365c4ba74c8d6606900 Mon Sep 17 00:00:00 2001 From: Hinton Date: Mon, 22 Apr 2024 15:39:32 +0200 Subject: [PATCH 15/22] Mark send key as sensitive --- crates/bitwarden/src/vault/send.rs | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/crates/bitwarden/src/vault/send.rs b/crates/bitwarden/src/vault/send.rs index a5537d8e1..efd236580 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, DecryptedString, DecryptedVec, - EncString, KeyDecryptable, KeyEncryptable, LocateKey, SymmetricCryptoKey, + EncString, KeyDecryptable, KeyEncryptable, LocateKey, SensitiveVec, SymmetricCryptoKey, }; use chrono::{DateTime, Utc}; use schemars::JsonSchema; @@ -99,7 +99,7 @@ pub struct SendView { pub name: DecryptedString, pub notes: Option, /// Base64 encoded key - pub key: Option, + pub key: Option, /// Replace or add a password to an existing send. The SDK will always return None when /// decrypting a [Send] /// TODO: We should revisit this, one variant is to have `[Create, Update]SendView` DTOs. @@ -209,7 +209,7 @@ impl KeyDecryptable for Send { name: self.name.decrypt_with_key(&key).ok().unwrap_or_default(), notes: self.notes.decrypt_with_key(&key).ok().flatten(), - key: Some(URL_SAFE_NO_PAD.encode(k.expose())), + key: Some(k.encode_base64(URL_SAFE_NO_PAD)), new_password: None, has_password: self.password.is_some(), @@ -260,18 +260,21 @@ impl KeyEncryptable for SendView { // the stretched key let k = match (self.key, self.id) { // Existing send, decrypt key - (Some(k), _) => URL_SAFE_NO_PAD - .decode(k) + (Some(k), _) => k + .decode_base64(URL_SAFE_NO_PAD) .map_err(|_| CryptoError::InvalidKey)?, // New send, generate random key (None, None) => { - let key: [u8; 16] = generate_random_bytes(); - key.to_vec() + let mut key: [u8; 16] = generate_random_bytes(); + let out = SensitiveVec::new(Box::new(key.to_vec())); + key.fill(0); + + out } // Existing send without key _ => return Err(CryptoError::InvalidKey), }; - let send_key = Send::derive_shareable_key(&k)?; + let send_key = Send::derive_shareable_key(k.expose())?; Ok(Send { id: self.id, @@ -279,9 +282,10 @@ impl KeyEncryptable for SendView { name: self.name.encrypt_with_key(&send_key)?, notes: self.notes.encrypt_with_key(&send_key)?, - key: k.encrypt_with_key(key)?, + key: k.expose().encrypt_with_key(key)?, password: self.new_password.map(|password| { - let password = bitwarden_crypto::pbkdf2(password.as_bytes(), &k, SEND_ITERATIONS); + let password = + bitwarden_crypto::pbkdf2(password.as_bytes(), k.expose(), SEND_ITERATIONS); STANDARD.encode(password) }), @@ -450,7 +454,7 @@ mod tests { access_id: Some("ct2APRQtJk-BLLDwAYqhRA".to_owned()), name: SensitiveString::test("Test"), notes: None, - key: Some("Pgui0FK85cNhBGWHAlBHBw".to_owned()), + key: Some(SensitiveString::test("Pgui0FK85cNhBGWHAlBHBw")), new_password: None, has_password: false, r#type: SendType::Text, @@ -481,7 +485,7 @@ mod tests { access_id: Some("ct2APRQtJk-BLLDwAYqhRA".to_owned()), name: SensitiveString::test("Test"), notes: None, - key: Some("Pgui0FK85cNhBGWHAlBHBw".to_owned()), + key: Some(SensitiveString::test("Pgui0FK85cNhBGWHAlBHBw")), new_password: None, has_password: false, r#type: SendType::Text, @@ -560,7 +564,7 @@ mod tests { access_id: Some("ct2APRQtJk-BLLDwAYqhRA".to_owned()), name: SensitiveString::test("Test"), notes: None, - key: Some("Pgui0FK85cNhBGWHAlBHBw".to_owned()), + key: Some(SensitiveString::test("Pgui0FK85cNhBGWHAlBHBw")), new_password: Some("abc123".to_owned()), has_password: false, r#type: SendType::Text, From 1f731a86292c0e77644ddb460c34d66cbcbc9ca9 Mon Sep 17 00:00:00 2001 From: Hinton Date: Mon, 22 Apr 2024 19:21:36 +0200 Subject: [PATCH 16/22] Remove accidentally duplicated generate_cipher --- crates/bitwarden/src/vault/cipher/cipher.rs | 37 --------------------- 1 file changed, 37 deletions(-) diff --git a/crates/bitwarden/src/vault/cipher/cipher.rs b/crates/bitwarden/src/vault/cipher/cipher.rs index 9f5b8c633..bfd746b53 100644 --- a/crates/bitwarden/src/vault/cipher/cipher.rs +++ b/crates/bitwarden/src/vault/cipher/cipher.rs @@ -516,43 +516,6 @@ mod tests { fn test_generate_cipher_key() { let key = SymmetricCryptoKey::generate(rand::thread_rng()); - fn generate_cipher() -> CipherView { - CipherView { - r#type: CipherType::Login, - login: Some(login::LoginView { - username: Some(DecryptedString::test("test_username")), - password: Some(DecryptedString::test("test_password")), - password_revision_date: None, - uris: None, - totp: None, - autofill_on_page_load: None, - fido2_credentials: None, - }), - id: "fd411a1a-fec8-4070-985d-0e6560860e69".parse().ok(), - organization_id: None, - folder_id: None, - collection_ids: vec![], - key: None, - name: DecryptedString::test("My test login"), - notes: None, - identity: None, - card: None, - secure_note: None, - favorite: false, - reprompt: CipherRepromptType::None, - organization_use_totp: true, - edit: true, - view_password: true, - local_data: None, - attachments: None, - fields: None, - password_history: None, - creation_date: "2024-01-30T17:55:36.150Z".parse().unwrap(), - deleted_date: None, - revision_date: "2024-01-30T17:55:36.150Z".parse().unwrap(), - } - } - let original_cipher = generate_cipher(); // Check that the cipher gets encrypted correctly without it's own key From ef08b7c9b9680ceb6e0c23deb7a6f51ab581a1ae Mon Sep 17 00:00:00 2001 From: Hinton Date: Tue, 23 Apr 2024 11:11:55 +0200 Subject: [PATCH 17/22] Convert Send::derive_shareable_key to expect SensitiveVec --- .../bitwarden-crypto/src/sensitive/sensitive.rs | 12 ++++++++++++ crates/bitwarden/src/vault/send.rs | 15 +++++++-------- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/crates/bitwarden-crypto/src/sensitive/sensitive.rs b/crates/bitwarden-crypto/src/sensitive/sensitive.rs index 481058064..b412bf192 100644 --- a/crates/bitwarden-crypto/src/sensitive/sensitive.rs +++ b/crates/bitwarden-crypto/src/sensitive/sensitive.rs @@ -61,6 +61,18 @@ impl From> for SensitiveVec { } } +/// Helper to convert a `SensitiveVec` to a `Sensitive<[u8, N]>`. +impl TryFrom for Sensitive<[u8; N]> { + type Error = CryptoError; + + fn try_from(v: SensitiveVec) -> Result { + Ok(Sensitive::new(Box::new( + TryInto::<[u8; N]>::try_into(v.expose().as_slice()) + .map_err(|_| CryptoError::InvalidKey)?, + ))) + } +} + /// Helper to convert a `Sensitive>` to a `Sensitive`, care is taken to ensure any /// intermediate copies are zeroed to avoid leaking sensitive data. impl TryFrom for SensitiveString { diff --git a/crates/bitwarden/src/vault/send.rs b/crates/bitwarden/src/vault/send.rs index 92a2f46f7..98e5f016c 100644 --- a/crates/bitwarden/src/vault/send.rs +++ b/crates/bitwarden/src/vault/send.rs @@ -5,7 +5,8 @@ use base64::{ use bitwarden_api_api::models::{SendFileModel, SendResponseModel, SendTextModel}; use bitwarden_crypto::{ derive_shareable_key, generate_random_bytes, CryptoError, DecryptedString, DecryptedVec, - EncString, KeyDecryptable, KeyEncryptable, LocateKey, Sensitive, SymmetricCryptoKey, + EncString, KeyDecryptable, KeyEncryptable, LocateKey, Sensitive, SensitiveVec, + SymmetricCryptoKey, }; use chrono::{DateTime, Utc}; use schemars::JsonSchema; @@ -145,13 +146,11 @@ impl Send { enc_key: &SymmetricCryptoKey, ) -> Result { let key: DecryptedVec = send_key.decrypt_with_key(enc_key)?; - Self::derive_shareable_key(key.expose()) + Self::derive_shareable_key(key) } - fn derive_shareable_key(key: &[u8]) -> Result { - let key = Sensitive::new(Box::new( - key.try_into().map_err(|_| CryptoError::InvalidKeyLen)?, - )); + fn derive_shareable_key(key: SensitiveVec) -> Result { + let key = key.try_into()?; Ok(derive_shareable_key(key, "send", Some("send"))) } } @@ -203,7 +202,7 @@ impl KeyDecryptable for Send { // size For the rest of the fields, we ignore the provided SymmetricCryptoKey and // the stretched key let k: DecryptedVec = self.key.decrypt_with_key(key)?; - let key = Send::derive_shareable_key(k.expose())?; + let key = Send::derive_shareable_key(k)?; Ok(SendView { id: self.id, @@ -273,7 +272,7 @@ impl KeyEncryptable for SendView { // Existing send without key _ => return Err(CryptoError::InvalidKey), }; - let send_key = Send::derive_shareable_key(k.expose())?; + let send_key = Send::derive_shareable_key(k)?; Ok(Send { id: self.id, From 16ea5c3d4a32e442e2a7bf0aa11bf25dfaca0660 Mon Sep 17 00:00:00 2001 From: Hinton Date: Tue, 23 Apr 2024 12:06:05 +0200 Subject: [PATCH 18/22] Fix compile --- crates/bitwarden-crypto/src/sensitive/sensitive.rs | 4 ++-- crates/bitwarden/src/vault/send.rs | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/bitwarden-crypto/src/sensitive/sensitive.rs b/crates/bitwarden-crypto/src/sensitive/sensitive.rs index b412bf192..b51b2c1a5 100644 --- a/crates/bitwarden-crypto/src/sensitive/sensitive.rs +++ b/crates/bitwarden-crypto/src/sensitive/sensitive.rs @@ -62,10 +62,10 @@ impl From> for SensitiveVec { } /// Helper to convert a `SensitiveVec` to a `Sensitive<[u8, N]>`. -impl TryFrom for Sensitive<[u8; N]> { +impl TryFrom<&SensitiveVec> for Sensitive<[u8; N]> { type Error = CryptoError; - fn try_from(v: SensitiveVec) -> Result { + fn try_from(v: &SensitiveVec) -> Result { Ok(Sensitive::new(Box::new( TryInto::<[u8; N]>::try_into(v.expose().as_slice()) .map_err(|_| CryptoError::InvalidKey)?, diff --git a/crates/bitwarden/src/vault/send.rs b/crates/bitwarden/src/vault/send.rs index 98e5f016c..d75902ed9 100644 --- a/crates/bitwarden/src/vault/send.rs +++ b/crates/bitwarden/src/vault/send.rs @@ -146,10 +146,10 @@ impl Send { enc_key: &SymmetricCryptoKey, ) -> Result { let key: DecryptedVec = send_key.decrypt_with_key(enc_key)?; - Self::derive_shareable_key(key) + Self::derive_shareable_key(&key) } - fn derive_shareable_key(key: SensitiveVec) -> Result { + fn derive_shareable_key(key: &SensitiveVec) -> Result { let key = key.try_into()?; Ok(derive_shareable_key(key, "send", Some("send"))) } @@ -202,7 +202,7 @@ impl KeyDecryptable for Send { // size For the rest of the fields, we ignore the provided SymmetricCryptoKey and // the stretched key let k: DecryptedVec = self.key.decrypt_with_key(key)?; - let key = Send::derive_shareable_key(k)?; + let key = Send::derive_shareable_key(&k)?; Ok(SendView { id: self.id, @@ -272,7 +272,7 @@ impl KeyEncryptable for SendView { // Existing send without key _ => return Err(CryptoError::InvalidKey), }; - let send_key = Send::derive_shareable_key(k)?; + let send_key = Send::derive_shareable_key(&k)?; Ok(Send { id: self.id, From 31a02f862d24fef6eaac5ba7398bde1e011e1052 Mon Sep 17 00:00:00 2001 From: Hinton Date: Tue, 23 Apr 2024 12:56:06 +0200 Subject: [PATCH 19/22] Refactor sub_title to not expose temporary values --- .../src/sensitive/sensitive.rs | 2 +- crates/bitwarden/src/vault/cipher/cipher.rs | 253 ++++++++++++++---- 2 files changed, 199 insertions(+), 56 deletions(-) diff --git a/crates/bitwarden-crypto/src/sensitive/sensitive.rs b/crates/bitwarden-crypto/src/sensitive/sensitive.rs index b51b2c1a5..08595bcbd 100644 --- a/crates/bitwarden-crypto/src/sensitive/sensitive.rs +++ b/crates/bitwarden-crypto/src/sensitive/sensitive.rs @@ -61,7 +61,7 @@ impl From> for SensitiveVec { } } -/// Helper to convert a `SensitiveVec` to a `Sensitive<[u8, N]>`. +/// Helper to convert a `&SensitiveVec` to a `Sensitive<[u8, N]>`. impl TryFrom<&SensitiveVec> for Sensitive<[u8; N]> { type Error = CryptoError; diff --git a/crates/bitwarden/src/vault/cipher/cipher.rs b/crates/bitwarden/src/vault/cipher/cipher.rs index bfd746b53..fe9bd9e0d 100644 --- a/crates/bitwarden/src/vault/cipher/cipher.rs +++ b/crates/bitwarden/src/vault/cipher/cipher.rs @@ -1,7 +1,7 @@ use bitwarden_api_api::models::CipherDetailsResponseModel; use bitwarden_crypto::{ CryptoError, DecryptedString, DecryptedVec, EncString, KeyContainer, KeyDecryptable, - KeyEncryptable, LocateKey, SymmetricCryptoKey, + KeyEncryptable, LocateKey, SensitiveString, SymmetricCryptoKey, }; use chrono::{DateTime, Utc}; use schemars::JsonSchema; @@ -121,7 +121,7 @@ pub struct CipherListView { pub collection_ids: Vec, pub name: DecryptedString, - pub sub_title: String, + pub sub_title: DecryptedString, pub r#type: CipherType, @@ -236,76 +236,129 @@ impl Cipher { .transpose() } - fn get_decrypted_subtitle(&self, key: &SymmetricCryptoKey) -> Result { + fn get_decrypted_subtitle( + &self, + key: &SymmetricCryptoKey, + ) -> Result { Ok(match self.r#type { CipherType::Login => { let Some(login) = &self.login else { - return Ok(String::new()); + return Ok(SensitiveString::default()); }; - login - .username - .decrypt_with_key(key)? - .map(|s: DecryptedString| s.expose().to_owned()) - .unwrap_or_default() + login.username.decrypt_with_key(key)?.unwrap_or_default() } - CipherType::SecureNote => String::new(), + CipherType::SecureNote => SensitiveString::default(), CipherType::Card => { let Some(card) = &self.card else { - return Ok(String::new()); + return Ok(SensitiveString::default()); }; - let mut sub_title = String::new(); - - if let Some(brand) = &card.brand { - let brand: DecryptedString = brand.decrypt_with_key(key)?; - sub_title.push_str(brand.expose()); - } - - if let Some(number) = &card.number { - let number: DecryptedString = number.decrypt_with_key(key)?; - let n = number.expose(); - let number_len = n.len(); - if number_len > 4 { - if !sub_title.is_empty() { - sub_title.push_str(", "); - } - - // On AMEX cards we show 5 digits instead of 4 - let digit_count = match &n[0..2] { - "34" | "37" => 5, - _ => 4, - }; - - sub_title.push_str(&n[(number_len - digit_count)..]); - } - } - - sub_title + + build_subtitle_card( + card.brand + .as_ref() + .map(|b| b.decrypt_with_key(key)) + .transpose()?, + card.number + .as_ref() + .map(|n| n.decrypt_with_key(key)) + .transpose()?, + ) } CipherType::Identity => { let Some(identity) = &self.identity else { - return Ok(String::new()); + return Ok(SensitiveString::default()); }; - let mut sub_title = String::new(); - - if let Some(first_name) = &identity.first_name { - let first_name: DecryptedString = first_name.decrypt_with_key(key)?; - sub_title.push_str(first_name.expose()); - } - - if let Some(last_name) = &identity.last_name { - if !sub_title.is_empty() { - sub_title.push(' '); - } - let last_name: DecryptedString = last_name.decrypt_with_key(key)?; - sub_title.push_str(last_name.expose()); - } - - sub_title + + build_subtitle_identity( + identity + .first_name + .as_ref() + .map(|f| f.decrypt_with_key(key)) + .transpose()?, + identity + .last_name + .as_ref() + .map(|l| l.decrypt_with_key(key)) + .transpose()?, + ) } }) } } +/// Builds the subtitle for a card cipher +/// +/// Care is taken to avoid leaking sensitive data by allocating the full size of the subtitle +fn build_subtitle_card( + brand: Option, + number: Option, +) -> SensitiveString { + let brand: Option = brand.filter(|b: &SensitiveString| !b.expose().is_empty()); + + // We only want to expose the last 4 or 5 digits of the card number + let number: Option = number + .filter(|b: &SensitiveString| b.expose().len() > 4) + .map(|n| { + // For AMEX cards show 5 digits instead of 4 + let desired_len = match &n.expose()[0..2] { + "34" | "37" => 5, + _ => 4, + }; + let start = n.expose().len() - desired_len; + + let mut str = SensitiveString::new(Box::new(String::with_capacity(desired_len + 1))); + str.expose_mut().push('*'); + str.expose_mut().push_str(&n.expose()[start..]); + + str + }); + + match (brand, number) { + (Some(brand), Some(number)) => { + let length = brand.expose().len() + 2 + number.expose().len(); + + let mut str = SensitiveString::new(Box::new(String::with_capacity(length))); + str.expose_mut().push_str(brand.expose()); + str.expose_mut().push_str(", "); + str.expose_mut().push_str(number.expose()); + + str + } + (Some(brand), None) => brand, + (None, Some(number)) => number, + _ => SensitiveString::new(Box::new("".to_owned())), + } +} + +/// Builds the subtitle for a card cipher +/// +/// Care is taken to avoid leaking sensitive data by allocating the full size of the subtitle +fn build_subtitle_identity( + first_name: Option, + 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()); + + match (first_name, last_name) { + (Some(first_name), Some(last_name)) => { + let length = first_name.expose().len() + 1 + last_name.expose().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()); + + str + } + (Some(first_name), None) => first_name, + (None, Some(last_name)) => last_name, + _ => SensitiveString::new(Box::new("".to_owned())), + } +} + impl CipherView { pub fn generate_cipher_key(&mut self, key: &SymmetricCryptoKey) -> Result<()> { let ciphers_key = Cipher::get_cipher_key(key, &self.key)?; @@ -590,4 +643,94 @@ mod tests { let org_key = enc.get_key(&Some(org)).unwrap(); assert!(cipher.encrypt_with_key(org_key).is_err()); } + + #[test] + fn test_build_subtitle_card_visa() { + let brand = Some(DecryptedString::test("Visa")); + let number = Some(DecryptedString::test("4111111111111111")); + + let subtitle = build_subtitle_card(brand, number); + assert_eq!(subtitle.expose(), "Visa, *1111"); + } + + #[test] + fn test_build_subtitle_card_mastercard() { + let brand = Some(DecryptedString::test("Mastercard")); + let number = Some(DecryptedString::test("5555555555554444")); + + let subtitle = build_subtitle_card(brand, number); + assert_eq!(subtitle.expose(), "Mastercard, *4444"); + } + + #[test] + fn test_build_subtitle_card_amex() { + let brand = Some(DecryptedString::test("Amex")); + let number = Some(DecryptedString::test("378282246310005")); + + let subtitle = build_subtitle_card(brand, number); + assert_eq!(subtitle.expose(), "Amex, *10005"); + } + + #[test] + fn test_build_subtitle_card_underflow() { + let brand = Some(DecryptedString::test("Mastercard")); + let number = Some(DecryptedString::test("4")); + + let subtitle = build_subtitle_card(brand, number); + assert_eq!(subtitle.expose(), "Mastercard"); + } + + #[test] + fn test_build_subtitle_card_only_brand() { + let brand = Some(DecryptedString::test("Mastercard")); + let number = None; + + let subtitle = build_subtitle_card(brand, number); + assert_eq!(subtitle.expose(), "Mastercard"); + } + + #[test] + fn test_build_subtitle_card_only_card() { + let brand = None; + let number = Some(DecryptedString::test("5555555555554444")); + + let subtitle = build_subtitle_card(brand, number); + assert_eq!(subtitle.expose(), "*4444"); + } + + #[test] + fn test_build_subtitle_identity() { + let first_name = Some(DecryptedString::test("John")); + let last_name = Some(DecryptedString::test("Doe")); + + let subtitle = build_subtitle_identity(first_name, last_name); + assert_eq!(subtitle.expose(), "John Doe"); + } + + #[test] + fn test_build_subtitle_identity_only_first() { + let first_name = Some(DecryptedString::test("John")); + let last_name = None; + + let subtitle = build_subtitle_identity(first_name, last_name); + assert_eq!(subtitle.expose(), "John"); + } + + #[test] + fn test_build_subtitle_identity_only_last() { + let first_name = None; + let last_name = Some(DecryptedString::test("Doe")); + + let subtitle = build_subtitle_identity(first_name, last_name); + assert_eq!(subtitle.expose(), "Doe"); + } + + #[test] + fn test_build_subtitle_identity_none() { + let first_name = None; + let last_name = None; + + let subtitle = build_subtitle_identity(first_name, last_name); + assert_eq!(subtitle.expose(), ""); + } } From 0d40054e24bb55996356e455e68d88256d776b38 Mon Sep 17 00:00:00 2001 From: Hinton Date: Tue, 23 Apr 2024 18:27:18 +0200 Subject: [PATCH 20/22] Shore up uri checksum --- .../src/sensitive/sensitive.rs | 7 ++++ crates/bitwarden/src/vault/cipher/login.rs | 34 +++++++++++-------- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/crates/bitwarden-crypto/src/sensitive/sensitive.rs b/crates/bitwarden-crypto/src/sensitive/sensitive.rs index dac688f46..9673c47e5 100644 --- a/crates/bitwarden-crypto/src/sensitive/sensitive.rs +++ b/crates/bitwarden-crypto/src/sensitive/sensitive.rs @@ -3,6 +3,7 @@ use std::{ fmt::{self, Formatter}, }; +use generic_array::{ArrayLength, GenericArray}; use schemars::JsonSchema; use serde::{Deserialize, Deserializer, Serialize, Serializer}; use zeroize::{Zeroize, ZeroizeOnDrop}; @@ -93,6 +94,12 @@ impl From for SensitiveVec { } } +impl> From>> for SensitiveVec { + fn from(val: Sensitive>) -> Self { + SensitiveVec::new(Box::new(val.value.to_vec())) + } +} + impl SensitiveString { pub fn decode_base64(self, engine: T) -> Result { // Prevent accidental copies by allocating the full size diff --git a/crates/bitwarden/src/vault/cipher/login.rs b/crates/bitwarden/src/vault/cipher/login.rs index 1612d869d..bc474c573 100644 --- a/crates/bitwarden/src/vault/cipher/login.rs +++ b/crates/bitwarden/src/vault/cipher/login.rs @@ -1,13 +1,15 @@ -use base64::{engine::general_purpose::STANDARD, Engine}; +use base64::engine::general_purpose::STANDARD; use bitwarden_api_api::models::{CipherLoginModel, CipherLoginUriModel}; use bitwarden_crypto::{ - CryptoError, DecryptedString, EncString, KeyDecryptable, KeyEncryptable, SensitiveString, - SymmetricCryptoKey, + CryptoError, DecryptedString, EncString, KeyDecryptable, KeyEncryptable, Sensitive, + SensitiveVec, SymmetricCryptoKey, }; use chrono::{DateTime, Utc}; +use hmac::digest::generic_array::GenericArray; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use serde_repr::{Deserialize_repr, Serialize_repr}; +use sha2::Digest; use crate::error::{require, Error, Result}; @@ -50,26 +52,28 @@ impl LoginUriView { let Some(cs) = &self.uri_checksum else { return false; }; - let Ok(cs) = STANDARD.decode(cs.expose()) else { + let Ok(cs) = cs.clone().decode_base64(STANDARD) else { return false; }; - use sha2::Digest; - let uri_hash = sha2::Sha256::new() - .chain_update(uri.expose().as_bytes()) - .finalize(); + let uri_hash: Sensitive> = Sensitive::new(Box::new( + sha2::Sha256::new() + .chain_update(uri.expose().as_bytes()) + .finalize(), + )); - uri_hash.as_slice() == cs + uri_hash.expose().as_slice() == cs.expose() } pub(crate) fn generate_checksum(&mut self) { if let Some(uri) = &self.uri { - use sha2::Digest; - let uri_hash = sha2::Sha256::new() - .chain_update(uri.expose().as_bytes()) - .finalize(); - let uri_hash = STANDARD.encode(uri_hash.as_slice()); - self.uri_checksum = Some(SensitiveString::new(Box::new(uri_hash))); + let uri_hash: SensitiveVec = Sensitive::new(Box::new( + sha2::Sha256::new() + .chain_update(uri.expose().as_bytes()) + .finalize(), + )) + .into(); + self.uri_checksum = Some(uri_hash.encode_base64(STANDARD)) } } } From da787a25d7756a3c0fd554a2dc3d90b406a25207 Mon Sep 17 00:00:00 2001 From: Hinton Date: Thu, 25 Apr 2024 10:29:01 +0200 Subject: [PATCH 21/22] Remove converter for DecryptedString --- crates/bitwarden/src/uniffi_support.rs | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/crates/bitwarden/src/uniffi_support.rs b/crates/bitwarden/src/uniffi_support.rs index 83f04e0de..bf1eade70 100644 --- a/crates/bitwarden/src/uniffi_support.rs +++ b/crates/bitwarden/src/uniffi_support.rs @@ -1,6 +1,6 @@ use std::num::NonZeroU32; -use bitwarden_crypto::{AsymmetricEncString, DecryptedString, EncString, SensitiveString}; +use bitwarden_crypto::{AsymmetricEncString, EncString, SensitiveString}; use uuid::Uuid; use crate::UniffiCustomTypeConverter; @@ -18,18 +18,6 @@ uniffi::ffi_converter_forward!( crate::UniFfiTag ); -impl UniffiCustomTypeConverter for DecryptedString { - type Builtin = String; - - fn into_custom(val: Self::Builtin) -> uniffi::Result { - Ok(Self::new(Box::new(val))) - } - - fn from_custom(obj: Self) -> Self::Builtin { - (*obj.expose()).to_owned() - } -} - type DateTime = chrono::DateTime; uniffi::custom_type!(DateTime, std::time::SystemTime); From 7baa6f155a24e6b70f3931b104905633b4938a66 Mon Sep 17 00:00:00 2001 From: Hinton Date: Thu, 25 Apr 2024 10:43:30 +0200 Subject: [PATCH 22/22] Review feedback --- crates/bitwarden/src/auth/tde.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/bitwarden/src/auth/tde.rs b/crates/bitwarden/src/auth/tde.rs index ff6328834..9b21aaa18 100644 --- a/crates/bitwarden/src/auth/tde.rs +++ b/crates/bitwarden/src/auth/tde.rs @@ -1,6 +1,6 @@ -use base64::{engine::general_purpose::STANDARD, Engine}; +use base64::engine::general_purpose::STANDARD; use bitwarden_crypto::{ - AsymmetricEncString, AsymmetricPublicCryptoKey, DeviceKey, EncString, Kdf, SensitiveVec, + AsymmetricEncString, AsymmetricPublicCryptoKey, DeviceKey, EncString, Kdf, SensitiveString, SymmetricCryptoKey, TrustDeviceResponse, UserKey, }; @@ -15,9 +15,9 @@ pub(super) fn make_register_tde_keys( org_public_key: String, remember_device: bool, ) -> Result { - let public_key = AsymmetricPublicCryptoKey::from_der(SensitiveVec::new(Box::new( - STANDARD.decode(org_public_key)?, - )))?; + let public_key = AsymmetricPublicCryptoKey::from_der( + SensitiveString::new(Box::new(org_public_key)).decode_base64(STANDARD)?, + )?; let mut rng = rand::thread_rng();