diff --git a/crates/bitwarden-crypto/src/enc_string/symmetric.rs b/crates/bitwarden-crypto/src/enc_string/symmetric.rs index d5489fc96..0217a4b93 100644 --- a/crates/bitwarden-crypto/src/enc_string/symmetric.rs +++ b/crates/bitwarden-crypto/src/enc_string/symmetric.rs @@ -290,14 +290,16 @@ mod tests { use schemars::schema_for; use super::EncString; - use crate::{derive_symmetric_key, KeyDecryptable, KeyEncryptable, SymmetricCryptoKey}; + use crate::{ + derive_symmetric_key, KeyDecryptable, KeyEncryptable, SensitiveString, SymmetricCryptoKey, + }; #[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); @@ -390,8 +392,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(); @@ -403,8 +405,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/device_key.rs b/crates/bitwarden-crypto/src/keys/device_key.rs index 876ae7b75..d33e0dc32 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, EncString, - KeyDecryptable, KeyEncryptable, SymmetricCryptoKey, + error::Result, AsymmetricCryptoKey, AsymmetricEncString, CryptoError, DecryptedVec, EncString, + KeyDecryptable, KeyEncryptable, SensitiveString, SymmetricCryptoKey, }; /// Device Key @@ -16,7 +14,7 @@ pub struct DeviceKey(SymmetricCryptoKey); #[cfg_attr(feature = "mobile", derive(uniffi::Record))] pub struct TrustDeviceResponse { /// Base64 encoded device key - pub device_key: String, + pub device_key: SensitiveString, /// UserKey encrypted with DevicePublicKey pub protected_user_key: AsymmetricEncString, /// DevicePrivateKey encrypted with [DeviceKey] @@ -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()? @@ -65,25 +63,25 @@ impl DeviceKey { 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 = AsymmetricCryptoKey::from_der(&device_private_key)?; - let mut dec: Vec = protected_user_key.decrypt_with_key(&device_private_key)?; - let user_key: SymmetricCryptoKey = dec.as_mut_slice().try_into()?; + let dec: Vec = protected_user_key.decrypt_with_key(&device_private_key)?; + let dec = DecryptedVec::new(Box::new(dec)); + 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) } } @@ -98,10 +96,8 @@ mod tests { let result = DeviceKey::trust_device(&key).unwrap(); - let decrypted = result - .device_key - .parse::() - .unwrap() + let device_key = DeviceKey::try_from(result.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 8e6d2575b..b6921fc6b 100644 --- a/crates/bitwarden-crypto/src/keys/master_key.rs +++ b/crates/bitwarden-crypto/src/keys/master_key.rs @@ -67,7 +67,7 @@ impl MasterKey { 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() diff --git a/crates/bitwarden-crypto/src/keys/mod.rs b/crates/bitwarden-crypto/src/keys/mod.rs index 5bfd32b8b..435f1011b 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/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/lib.rs b/crates/bitwarden-crypto/src/lib.rs index 3c1aced24..8c368580a 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 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..0da2329b9 --- /dev/null +++ b/crates/bitwarden-crypto/src/sensitive/mod.rs @@ -0,0 +1,5 @@ +#[allow(clippy::module_inception)] +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..996b20330 --- /dev/null +++ b/crates/bitwarden-crypto/src/sensitive/sensitive.rs @@ -0,0 +1,220 @@ +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, +} + +/// Important: This type does not protect against reallocations made by the Vec. +/// This means that if you insert any elements past the capacity, the data will be copied to a +/// new allocation and the old allocation will not be zeroized. +/// To avoid this, use Vec::with_capacity to preallocate the capacity you need. +pub type SensitiveVec = Sensitive>; + +/// Important: This type does not protect against reallocations made by the String. +/// This means that if you insert any characters past the capacity, the data will be copied to a +/// new allocation and the old allocation will not be zeroized. +/// To avoid this, use String::with_capacity to preallocate the capacity you need. +pub type SensitiveString = Sensitive; + +impl Sensitive { + /// Create a new [`Sensitive`] value. In an attempt to avoid accidentally placing this on the + /// stack it only accepts a [`Box`] value. The rust compiler should be able to optimize away the + /// 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 SensitiveString { + pub fn decode_base64(self, engine: T) -> Result { + // Prevent accidental copies by allocating the full size + let len = base64::decoded_len_estimate(self.value.len()); + let mut value = SensitiveVec::new(Box::new(Vec::with_capacity(len))); + + engine + .decode_vec(self.value.as_ref(), &mut value.value) + .map_err(|_| CryptoError::InvalidKey)?; + + Ok(value) + } +} + +impl SensitiveVec { + pub fn encode_base64(self, engine: T) -> SensitiveString { + use base64::engine::Config; + + // Prevent accidental copies by allocating the full size + let padding = engine.config().encode_padding(); + let len = base64::encoded_len(self.value.len(), padding).expect("Valid length"); + let mut value = SensitiveString::new(Box::new(String::with_capacity(len))); + + engine.encode_string(self.value.as_ref(), &mut value.value); + + value + } +} + +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) + } +} + +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; + + use super::*; + + #[test] + fn test_debug() { + let string = Sensitive::test("test"); + assert_eq!( + format!("{:?}", string), + "Sensitive { type: \"alloc::string::String\", value: \"********\" }" + ); + + let vector = Sensitive::new(Box::new(vec![1, 2, 3])); + assert_eq!( + format!("{:?}", vector), + "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() + ); + } +} diff --git a/crates/bitwarden-crypto/src/uniffi_support.rs b/crates/bitwarden-crypto/src/uniffi_support.rs index 7f1249da0..f99b55da5 100644 --- a/crates/bitwarden-crypto/src/uniffi_support.rs +++ b/crates/bitwarden-crypto/src/uniffi_support.rs @@ -1,6 +1,8 @@ use std::{num::NonZeroU32, str::FromStr}; -use crate::{AsymmetricEncString, CryptoError, EncString, UniffiCustomTypeConverter}; +use crate::{ + AsymmetricEncString, CryptoError, EncString, SensitiveString, UniffiCustomTypeConverter, +}; uniffi::custom_type!(NonZeroU32, u32); @@ -43,3 +45,24 @@ impl UniffiCustomTypeConverter for AsymmetricEncString { obj.to_string() } } + +uniffi::custom_type!(SensitiveString, String); + +impl UniffiCustomTypeConverter for SensitiveString { + type Builtin = String; + + fn into_custom(val: Self::Builtin) -> uniffi::Result { + Ok(SensitiveString::new(Box::new(val))) + } + + fn from_custom(obj: Self) -> Self::Builtin { + obj.expose().to_owned() + } +} + +/// Uniffi doesn't seem to be generating the SensitiveString unless it's being used by +/// a record somewhere. This is a workaround to make sure the type is generated. +#[derive(uniffi::Record)] +struct SupportSensitiveString { + sensitive_string: SensitiveString, +} diff --git a/crates/bitwarden-uniffi/src/crypto.rs b/crates/bitwarden-uniffi/src/crypto.rs index 0f877d52e..f3abe694a 100644 --- a/crates/bitwarden-uniffi/src/crypto.rs +++ b/crates/bitwarden-uniffi/src/crypto.rs @@ -3,7 +3,7 @@ use std::sync::Arc; use bitwarden::mobile::crypto::{ DerivePinKeyResponse, InitOrgCryptoRequest, InitUserCryptoRequest, UpdatePasswordResponse, }; -use bitwarden_crypto::{AsymmetricEncString, EncString}; +use bitwarden_crypto::{AsymmetricEncString, EncString, SensitiveString}; use crate::{error::Result, Client}; @@ -40,7 +40,7 @@ impl ClientCrypto { /// Get the uses's decrypted encryption key. Note: It's very important /// to keep this key safe, as it can be used to decrypt all of the user's data - pub async fn get_user_encryption_key(&self) -> Result { + pub async fn get_user_encryption_key(&self) -> Result { Ok(self .0 .0 diff --git a/crates/bitwarden-uniffi/src/uniffi_support.rs b/crates/bitwarden-uniffi/src/uniffi_support.rs index 663d5c41e..a49e347b6 100644 --- a/crates/bitwarden-uniffi/src/uniffi_support.rs +++ b/crates/bitwarden-uniffi/src/uniffi_support.rs @@ -1,7 +1,8 @@ -use bitwarden_crypto::{AsymmetricEncString, EncString}; +use bitwarden_crypto::{AsymmetricEncString, EncString, SensitiveString}; // Forward the type definitions to the main bitwarden crate type DateTime = chrono::DateTime; uniffi::ffi_converter_forward!(DateTime, bitwarden::UniFfiTag, crate::UniFfiTag); uniffi::ffi_converter_forward!(EncString, bitwarden::UniFfiTag, crate::UniFfiTag); uniffi::ffi_converter_forward!(AsymmetricEncString, bitwarden::UniFfiTag, crate::UniFfiTag); +uniffi::ffi_converter_forward!(SensitiveString, bitwarden::UniFfiTag, crate::UniFfiTag); diff --git a/crates/bitwarden/src/auth/access_token.rs b/crates/bitwarden/src/auth/access_token.rs index 667a78529..4b6f050c8 100644 --- a/crates/bitwarden/src/auth/access_token.rs +++ b/crates/bitwarden/src/auth/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/auth/auth_request.rs b/crates/bitwarden/src/auth/auth_request.rs index 18c71afba..04dc9fdce 100644 --- a/crates/bitwarden/src/auth/auth_request.rs +++ b/crates/bitwarden/src/auth/auth_request.rs @@ -92,15 +92,13 @@ pub(crate) fn approve_auth_request( 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] = &[ @@ -117,7 +115,7 @@ fn test_auth_request() { 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)] @@ -167,8 +165,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 ] @@ -186,8 +184,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 46c3f2b50..a8c017009 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 base64::engine::general_purpose::STANDARD; +use bitwarden_crypto::{EncString, KeyDecryptable, SensitiveString, SymmetricCryptoKey}; use chrono::Utc; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -59,12 +59,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)?; - 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()?; @@ -125,7 +125,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/auth/tde.rs b/crates/bitwarden/src/auth/tde.rs index 1a3de3026..0bbbb904a 100644 --- a/crates/bitwarden/src/auth/tde.rs +++ b/crates/bitwarden/src/auth/tde.rs @@ -22,7 +22,7 @@ pub(super) fn make_register_tde_keys( let key_pair = user_key.make_key_pair()?; let admin_reset = - AsymmetricEncString::encrypt_rsa2048_oaep_sha1(&user_key.0.to_vec(), &public_key)?; + AsymmetricEncString::encrypt_rsa2048_oaep_sha1(user_key.0.to_vec().expose(), &public_key)?; let device_key = if remember_device { Some(DeviceKey::trust_device(&user_key.0)?) diff --git a/crates/bitwarden/src/mobile/client_crypto.rs b/crates/bitwarden/src/mobile/client_crypto.rs index 6ef65975d..26451ffaa 100644 --- a/crates/bitwarden/src/mobile/client_crypto.rs +++ b/crates/bitwarden/src/mobile/client_crypto.rs @@ -1,5 +1,5 @@ #[cfg(feature = "internal")] -use bitwarden_crypto::{AsymmetricEncString, EncString}; +use bitwarden_crypto::{AsymmetricEncString, EncString, SensitiveString}; use crate::Client; #[cfg(feature = "internal")] @@ -28,7 +28,7 @@ impl<'a> ClientCrypto<'a> { } #[cfg(feature = "internal")] - pub async fn get_user_encryption_key(&mut self) -> Result { + pub async fn get_user_encryption_key(&mut self) -> Result { get_user_encryption_key(self.client).await } diff --git a/crates/bitwarden/src/mobile/crypto.rs b/crates/bitwarden/src/mobile/crypto.rs index 214643023..b8891aa09 100644 --- a/crates/bitwarden/src/mobile/crypto.rs +++ b/crates/bitwarden/src/mobile/crypto.rs @@ -2,7 +2,9 @@ use std::collections::HashMap; use bitwarden_crypto::{AsymmetricEncString, EncString}; #[cfg(feature = "internal")] -use bitwarden_crypto::{KeyDecryptable, KeyEncryptable, MasterKey, SymmetricCryptoKey}; +use bitwarden_crypto::{ + KeyDecryptable, KeyEncryptable, MasterKey, SensitiveString, SymmetricCryptoKey, +}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -86,7 +88,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::{DecryptedString, DeviceKey}; use crate::auth::{auth_request_decrypt_master_key, auth_request_decrypt_user_key}; @@ -105,7 +107,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 = DecryptedString::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 +141,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 = DecryptedString::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)?; @@ -166,7 +170,7 @@ pub async fn initialize_org_crypto(client: &mut Client, req: InitOrgCryptoReques } #[cfg(feature = "internal")] -pub async fn get_user_encryption_key(client: &mut Client) -> Result { +pub async fn get_user_encryption_key(client: &mut Client) -> Result { let user_key = client .get_encryption_settings()? .get_key(&None) @@ -299,7 +303,7 @@ pub(super) fn enroll_admin_password_reset( 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, )?) } @@ -483,7 +487,7 @@ 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; @@ -516,10 +520,7 @@ mod tests { .get_encryption_settings() .unwrap() .get_key(&None) - .unwrap() - .to_vec() - .deref() - .clone(); - assert_eq!(decrypted, expected); + .unwrap(); + assert_eq!(&decrypted, expected.to_vec().expose()); } } diff --git a/crates/bitwarden/src/secrets_manager/state.rs b/crates/bitwarden/src/secrets_manager/state.rs index b2b6f6a8e..c677ab7e9 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::{EncString, KeyDecryptable, KeyEncryptable, SensitiveString}; use serde::{Deserialize, Serialize}; use crate::{ @@ -15,11 +15,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/uniffi_support.rs b/crates/bitwarden/src/uniffi_support.rs index b23a9cbef..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, EncString}; +use bitwarden_crypto::{AsymmetricEncString, EncString, SensitiveString}; use uuid::Uuid; use crate::UniffiCustomTypeConverter; @@ -12,6 +12,11 @@ uniffi::ffi_converter_forward!( bitwarden_crypto::UniFfiTag, crate::UniFfiTag ); +uniffi::ffi_converter_forward!( + SensitiveString, + bitwarden_crypto::UniFfiTag, + crate::UniFfiTag +); 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 1ec6be6fe..a573e92b0 100644 --- a/crates/bitwarden/src/vault/cipher/attachment.rs +++ b/crates/bitwarden/src/vault/cipher/attachment.rs @@ -66,7 +66,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)?, @@ -136,7 +141,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}, @@ -145,7 +150,7 @@ mod tests { #[test] fn test_attachment_key() { - let user_key : SymmetricCryptoKey = "w2LO+nwV4oxwswVYCxlOfRUseXfvU03VzvKQHrqeklPgiMZrspUe6sOBToCnDn9Ay0tuCBn8ykVVRb7PWhub2Q==".parse().unwrap(); + let user_key : SymmetricCryptoKey = SensitiveString::test("w2LO+nwV4oxwswVYCxlOfRUseXfvU03VzvKQHrqeklPgiMZrspUe6sOBToCnDn9Ay0tuCBn8ykVVRb7PWhub2Q==").try_into().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 f0138beb9..5df26ab34 100644 --- a/crates/bitwarden/src/vault/cipher/cipher.rs +++ b/crates/bitwarden/src/vault/cipher/cipher.rs @@ -308,7 +308,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 144933899..b6351baa1 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 { 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); }