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/admin_console/policy.rs b/crates/bitwarden/src/admin_console/policy.rs index 7e87b8623..d8ed0b761 100644 --- a/crates/bitwarden/src/admin_console/policy.rs +++ b/crates/bitwarden/src/admin_console/policy.rs @@ -6,7 +6,7 @@ use serde::{Deserialize, Serialize}; use serde_repr::{Deserialize_repr, Serialize_repr}; use uuid::Uuid; -use crate::error::{Error, Result}; +use crate::error::{require, Error, Result}; #[derive(Serialize, Deserialize, Debug, JsonSchema)] pub struct Policy { @@ -41,11 +41,11 @@ impl TryFrom for Policy { fn try_from(policy: PolicyResponseModel) -> Result { Ok(Self { - id: policy.id.ok_or(Error::MissingFields)?, - organization_id: policy.organization_id.ok_or(Error::MissingFields)?, - r#type: policy.r#type.ok_or(Error::MissingFields)?.into(), + id: require!(policy.id), + organization_id: require!(policy.organization_id), + r#type: require!(policy.r#type).into(), data: policy.data, - enabled: policy.enabled.ok_or(Error::MissingFields)?, + enabled: require!(policy.enabled), }) } } 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..cca52512b 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}; @@ -14,7 +14,7 @@ use crate::{ AccessToken, JWTToken, }, client::{LoginMethod, ServiceAccountLoginMethod}, - error::{Error, Result}, + error::{require, Error, Result}, secrets_manager::state::{self, ClientState}, Client, }; @@ -59,19 +59,17 @@ 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()?; // This should always be Some() when logging in with an access token - let organization_id = access_token_obj - .organization - .ok_or(Error::MissingFields)? + let organization_id = require!(access_token_obj.organization) .parse() .map_err(|_| Error::InvalidResponse)?; @@ -125,7 +123,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/login/api_key.rs b/crates/bitwarden/src/auth/login/api_key.rs index 72d05897a..5d7fdcd96 100644 --- a/crates/bitwarden/src/auth/login/api_key.rs +++ b/crates/bitwarden/src/auth/login/api_key.rs @@ -9,7 +9,7 @@ use crate::{ JWTToken, }, client::{LoginMethod, UserLoginMethod}, - error::{Error, Result}, + error::{require, Result}, Client, }; @@ -44,12 +44,8 @@ pub(crate) async fn login_api_key( kdf, })); - let user_key: EncString = r.key.as_deref().ok_or(Error::MissingFields)?.parse()?; - let private_key: EncString = r - .private_key - .as_deref() - .ok_or(Error::MissingFields)? - .parse()?; + let user_key: EncString = require!(r.key.as_deref()).parse()?; + let private_key: EncString = require!(r.private_key.as_deref()).parse()?; client.initialize_user_crypto(&input.password, user_key, private_key)?; } diff --git a/crates/bitwarden/src/auth/login/auth_request.rs b/crates/bitwarden/src/auth/login/auth_request.rs index ee2aef254..887809e4b 100644 --- a/crates/bitwarden/src/auth/login/auth_request.rs +++ b/crates/bitwarden/src/auth/login/auth_request.rs @@ -13,7 +13,7 @@ use crate::{ auth_request::new_auth_request, }, client::{LoginMethod, UserLoginMethod}, - error::{Error, Result}, + error::{require, Result}, mobile::crypto::{AuthRequestMethod, InitUserCryptoMethod, InitUserCryptoRequest}, Client, }; @@ -50,7 +50,7 @@ pub(crate) async fn send_new_auth_request( fingerprint: auth.fingerprint, email, device_identifier, - auth_request_id: res.id.ok_or(Error::MissingFields)?, + auth_request_id: require!(res.id), access_code: auth.access_code, private_key: auth.private_key, }) @@ -103,11 +103,11 @@ pub(crate) async fn complete_auth_request( let method = match res.master_password_hash { Some(_) => AuthRequestMethod::MasterKey { - protected_master_key: res.key.ok_or(Error::MissingFields)?.parse()?, - auth_request_key: r.key.ok_or(Error::MissingFields)?.parse()?, + protected_master_key: require!(res.key).parse()?, + auth_request_key: require!(r.key).parse()?, }, None => AuthRequestMethod::UserKey { - protected_user_key: res.key.ok_or(Error::MissingFields)?.parse()?, + protected_user_key: require!(res.key).parse()?, }, }; @@ -116,7 +116,7 @@ pub(crate) async fn complete_auth_request( .initialize_user_crypto(InitUserCryptoRequest { kdf_params: kdf, email: auth_req.email, - private_key: r.private_key.ok_or(Error::MissingFields)?, + private_key: require!(r.private_key), method: InitUserCryptoMethod::AuthRequest { request_private_key: auth_req.private_key, method, diff --git a/crates/bitwarden/src/auth/login/password.rs b/crates/bitwarden/src/auth/login/password.rs index 9fa62a566..8ae9daebc 100644 --- a/crates/bitwarden/src/auth/login/password.rs +++ b/crates/bitwarden/src/auth/login/password.rs @@ -24,7 +24,7 @@ pub(crate) async fn login_password( ) -> Result { use bitwarden_crypto::{EncString, HashPurpose}; - use crate::{auth::determine_password_hash, client::UserLoginMethod, error::Error}; + use crate::{auth::determine_password_hash, client::UserLoginMethod, error::require}; info!("password logging in"); debug!("{:#?}, {:#?}", client, input); @@ -49,12 +49,8 @@ pub(crate) async fn login_password( kdf: input.kdf.to_owned(), })); - let user_key: EncString = r.key.as_deref().ok_or(Error::MissingFields)?.parse()?; - let private_key: EncString = r - .private_key - .as_deref() - .ok_or(Error::MissingFields)? - .parse()?; + let user_key: EncString = require!(r.key.as_deref()).parse()?; + let private_key: EncString = require!(r.private_key.as_deref()).parse()?; client.initialize_user_crypto(&input.password, user_key, private_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/error.rs b/crates/bitwarden/src/error.rs index bd040e51b..59fae58d2 100644 --- a/crates/bitwarden/src/error.rs +++ b/crates/bitwarden/src/error.rs @@ -24,8 +24,8 @@ pub enum Error { #[error("The response received was invalid and could not be processed")] InvalidResponse, - #[error("The response received was missing some of the required fields")] - MissingFields, + #[error("The response received was missing some of the required fields: {0}")] + MissingFields(&'static str), #[error("Cryptography error, {0}")] Crypto(#[from] bitwarden_crypto::CryptoError), @@ -133,4 +133,18 @@ macro_rules! impl_bitwarden_error { impl_bitwarden_error!(ApiError); impl_bitwarden_error!(IdentityError); +/// This macro is used to require that a value is present or return an error otherwise. +/// It is equivalent to using `val.ok_or(Error::MissingFields)?`, but easier to use and +/// with a more descriptive error message. +/// Note that this macro will return early from the function if the value is not present. +macro_rules! require { + ($val:expr) => { + match $val { + Some(val) => val, + None => return Err($crate::error::Error::MissingFields(stringify!($val))), + } + }; +} +pub(crate) use require; + pub type Result = std::result::Result; 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/platform/domain.rs b/crates/bitwarden/src/platform/domain.rs index c903dd5a5..482cb1f59 100644 --- a/crates/bitwarden/src/platform/domain.rs +++ b/crates/bitwarden/src/platform/domain.rs @@ -1,7 +1,7 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use crate::error::{Error, Result}; +use crate::error::{require, Error, Result}; #[derive(Serialize, Deserialize, Debug, JsonSchema)] pub struct GlobalDomains { @@ -15,9 +15,9 @@ impl TryFrom for GlobalDomains { fn try_from(global_domains: bitwarden_api_api::models::GlobalDomains) -> Result { Ok(Self { - r#type: global_domains.r#type.ok_or(Error::MissingFields)?, - domains: global_domains.domains.ok_or(Error::MissingFields)?, - excluded: global_domains.excluded.ok_or(Error::MissingFields)?, + r#type: require!(global_domains.r#type), + domains: require!(global_domains.domains), + excluded: require!(global_domains.excluded), }) } } diff --git a/crates/bitwarden/src/platform/get_user_api_key.rs b/crates/bitwarden/src/platform/get_user_api_key.rs index 2eaa21894..3e408d926 100644 --- a/crates/bitwarden/src/platform/get_user_api_key.rs +++ b/crates/bitwarden/src/platform/get_user_api_key.rs @@ -10,7 +10,7 @@ use serde::{Deserialize, Serialize}; use super::SecretVerificationRequest; use crate::{ client::{LoginMethod, UserLoginMethod}, - error::{Error, Result}, + error::{require, Error, Result}, Client, }; @@ -75,9 +75,7 @@ pub struct UserApiKeyResponse { impl UserApiKeyResponse { pub(crate) fn process_response(response: ApiKeyResponseModel) -> Result { - match response.api_key { - Some(api_key) => Ok(UserApiKeyResponse { api_key }), - None => Err(Error::MissingFields), - } + let api_key = require!(response.api_key); + Ok(UserApiKeyResponse { api_key }) } } diff --git a/crates/bitwarden/src/platform/sync.rs b/crates/bitwarden/src/platform/sync.rs index c1a039137..5a5c6ee82 100644 --- a/crates/bitwarden/src/platform/sync.rs +++ b/crates/bitwarden/src/platform/sync.rs @@ -9,7 +9,7 @@ use super::domain::GlobalDomains; use crate::{ admin_console::Policy, client::{encryption_settings::EncryptionSettings, Client}, - error::{Error, Result}, + error::{require, Error, Result}, vault::{Cipher, Collection, Folder}, }; @@ -25,10 +25,7 @@ pub(crate) async fn sync(client: &mut Client, input: &SyncRequest) -> Result = sync - .profile - .as_ref() - .ok_or(Error::MissingFields)? + let org_keys: Vec<_> = require!(sync.profile.as_ref()) .organizations .as_deref() .unwrap_or_default() @@ -86,8 +83,8 @@ impl SyncResponse { response: SyncResponseModel, enc: &EncryptionSettings, ) -> Result { - let profile = *response.profile.ok_or(Error::MissingFields)?; - let ciphers = response.ciphers.ok_or(Error::MissingFields)?; + let profile = require!(response.profile); + let ciphers = require!(response.ciphers); fn try_into_iter(iter: In) -> Result where @@ -99,13 +96,13 @@ impl SyncResponse { } Ok(SyncResponse { - profile: ProfileResponse::process_response(profile, enc)?, - folders: try_into_iter(response.folders.ok_or(Error::MissingFields)?)?, - collections: try_into_iter(response.collections.ok_or(Error::MissingFields)?)?, + profile: ProfileResponse::process_response(*profile, enc)?, + folders: try_into_iter(require!(response.folders))?, + collections: try_into_iter(require!(response.collections))?, ciphers: try_into_iter(ciphers)?, domains: response.domains.map(|d| (*d).try_into()).transpose()?, - policies: try_into_iter(response.policies.ok_or(Error::MissingFields)?)?, - sends: try_into_iter(response.sends.ok_or(Error::MissingFields)?)?, + policies: try_into_iter(require!(response.policies))?, + sends: try_into_iter(require!(response.sends))?, }) } } @@ -115,7 +112,7 @@ impl ProfileOrganizationResponse { response: ProfileOrganizationResponseModel, ) -> Result { Ok(ProfileOrganizationResponse { - id: response.id.ok_or(Error::MissingFields)?, + id: require!(response.id), }) } } @@ -126,9 +123,9 @@ impl ProfileResponse { _enc: &EncryptionSettings, ) -> Result { Ok(ProfileResponse { - id: response.id.ok_or(Error::MissingFields)?, - name: response.name.ok_or(Error::MissingFields)?, - email: response.email.ok_or(Error::MissingFields)?, + id: require!(response.id), + name: require!(response.name), + email: require!(response.email), //key: response.key, //private_key: response.private_key, organizations: response diff --git a/crates/bitwarden/src/secrets_manager/projects/delete.rs b/crates/bitwarden/src/secrets_manager/projects/delete.rs index 55f792669..05c808c7e 100644 --- a/crates/bitwarden/src/secrets_manager/projects/delete.rs +++ b/crates/bitwarden/src/secrets_manager/projects/delete.rs @@ -7,7 +7,7 @@ use uuid::Uuid; use crate::{ client::Client, - error::{Error, Result}, + error::{require, Result}, }; #[derive(Serialize, Deserialize, Debug, JsonSchema)] @@ -62,7 +62,7 @@ impl ProjectDeleteResponse { response: BulkDeleteResponseModel, ) -> Result { Ok(ProjectDeleteResponse { - id: response.id.ok_or(Error::MissingFields)?, + id: require!(response.id), error: response.error, }) } diff --git a/crates/bitwarden/src/secrets_manager/projects/project_response.rs b/crates/bitwarden/src/secrets_manager/projects/project_response.rs index 1e6f6a158..af67df9ab 100644 --- a/crates/bitwarden/src/secrets_manager/projects/project_response.rs +++ b/crates/bitwarden/src/secrets_manager/projects/project_response.rs @@ -7,7 +7,7 @@ use uuid::Uuid; use crate::{ client::encryption_settings::EncryptionSettings, - error::{Error, Result}, + error::{require, Result}, }; #[derive(Serialize, Deserialize, Debug, JsonSchema)] @@ -25,27 +25,19 @@ impl ProjectResponse { response: ProjectResponseModel, enc: &EncryptionSettings, ) -> Result { - let organization_id = response.organization_id.ok_or(Error::MissingFields)?; + let organization_id = require!(response.organization_id); - let name = response - .name - .ok_or(Error::MissingFields)? + let name = require!(response.name) .parse::()? .decrypt(enc, &Some(organization_id))?; Ok(ProjectResponse { - id: response.id.ok_or(Error::MissingFields)?, + id: require!(response.id), organization_id, name, - creation_date: response - .creation_date - .ok_or(Error::MissingFields)? - .parse()?, - revision_date: response - .revision_date - .ok_or(Error::MissingFields)? - .parse()?, + creation_date: require!(response.creation_date).parse()?, + revision_date: require!(response.revision_date).parse()?, }) } } diff --git a/crates/bitwarden/src/secrets_manager/secrets/delete.rs b/crates/bitwarden/src/secrets_manager/secrets/delete.rs index 19337e7a1..f3fe264e1 100644 --- a/crates/bitwarden/src/secrets_manager/secrets/delete.rs +++ b/crates/bitwarden/src/secrets_manager/secrets/delete.rs @@ -7,7 +7,7 @@ use uuid::Uuid; use crate::{ client::Client, - error::{Error, Result}, + error::{require, Result}, }; #[derive(Serialize, Deserialize, Debug, JsonSchema)] @@ -62,7 +62,7 @@ impl SecretDeleteResponse { response: BulkDeleteResponseModel, ) -> Result { Ok(SecretDeleteResponse { - id: response.id.ok_or(Error::MissingFields)?, + id: require!(response.id), error: response.error, }) } diff --git a/crates/bitwarden/src/secrets_manager/secrets/list.rs b/crates/bitwarden/src/secrets_manager/secrets/list.rs index 7fa46d164..48a507667 100644 --- a/crates/bitwarden/src/secrets_manager/secrets/list.rs +++ b/crates/bitwarden/src/secrets_manager/secrets/list.rs @@ -8,7 +8,7 @@ use uuid::Uuid; use crate::{ client::{encryption_settings::EncryptionSettings, Client}, - error::{Error, Result}, + error::{require, Result}, }; #[derive(Serialize, Deserialize, Debug, JsonSchema)] @@ -93,16 +93,14 @@ impl SecretIdentifierResponse { response: SecretsWithProjectsInnerSecret, enc: &EncryptionSettings, ) -> Result { - let organization_id = response.organization_id.ok_or(Error::MissingFields)?; + let organization_id = require!(response.organization_id); - let key = response - .key - .ok_or(Error::MissingFields)? + let key = require!(response.key) .parse::()? .decrypt(enc, &Some(organization_id))?; Ok(SecretIdentifierResponse { - id: response.id.ok_or(Error::MissingFields)?, + id: require!(response.id), organization_id, key, }) diff --git a/crates/bitwarden/src/secrets_manager/secrets/secret_response.rs b/crates/bitwarden/src/secrets_manager/secrets/secret_response.rs index fe1a4d342..ffaa7f7e0 100644 --- a/crates/bitwarden/src/secrets_manager/secrets/secret_response.rs +++ b/crates/bitwarden/src/secrets_manager/secrets/secret_response.rs @@ -9,7 +9,7 @@ use uuid::Uuid; use crate::{ client::encryption_settings::EncryptionSettings, - error::{Error, Result}, + error::{require, Result}, }; #[derive(Serialize, Deserialize, Debug, JsonSchema)] @@ -51,19 +51,13 @@ impl SecretResponse { ) -> Result { let org_id = response.organization_id; - let key = response - .key - .ok_or(Error::MissingFields)? + let key = require!(response.key) .parse::()? .decrypt(enc, &org_id)?; - let value = response - .value - .ok_or(Error::MissingFields)? + let value = require!(response.value) .parse::()? .decrypt(enc, &org_id)?; - let note = response - .note - .ok_or(Error::MissingFields)? + let note = require!(response.note) .parse::()? .decrypt(enc, &org_id)?; @@ -73,21 +67,15 @@ impl SecretResponse { .and_then(|p| p.id); Ok(SecretResponse { - id: response.id.ok_or(Error::MissingFields)?, - organization_id: org_id.ok_or(Error::MissingFields)?, + id: require!(response.id), + organization_id: require!(org_id), project_id: project, key, value, note, - creation_date: response - .creation_date - .ok_or(Error::MissingFields)? - .parse()?, - revision_date: response - .revision_date - .ok_or(Error::MissingFields)? - .parse()?, + creation_date: require!(response.creation_date).parse()?, + revision_date: require!(response.revision_date).parse()?, }) } } diff --git a/crates/bitwarden/src/secrets_manager/state.rs b/crates/bitwarden/src/secrets_manager/state.rs index 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/tool/exporters/mod.rs b/crates/bitwarden/src/tool/exporters/mod.rs index 45bdfd3fd..2da6ac833 100644 --- a/crates/bitwarden/src/tool/exporters/mod.rs +++ b/crates/bitwarden/src/tool/exporters/mod.rs @@ -4,7 +4,7 @@ use schemars::JsonSchema; use crate::{ client::{LoginMethod, UserLoginMethod}, - error::{Error, Result}, + error::{require, Error, Result}, vault::{ login::LoginUriView, Cipher, CipherType, CipherView, Collection, FieldView, Folder, FolderView, SecureNoteType, @@ -83,7 +83,7 @@ impl TryFrom for bitwarden_exporters::Folder { fn try_from(value: FolderView) -> Result { Ok(Self { - id: value.id.ok_or(Error::MissingFields)?, + id: require!(value.id), name: value.name, }) } @@ -95,7 +95,7 @@ impl TryFrom for bitwarden_exporters::Cipher { fn try_from(value: CipherView) -> Result { let r = match value.r#type { CipherType::Login => { - let l = value.login.ok_or(Error::MissingFields)?; + let l = require!(value.login); bitwarden_exporters::CipherType::Login(Box::new(bitwarden_exporters::Login { username: l.username, password: l.password, @@ -118,7 +118,7 @@ impl TryFrom for bitwarden_exporters::Cipher { }, )), CipherType::Card => { - let c = value.card.ok_or(Error::MissingFields)?; + let c = require!(value.card); bitwarden_exporters::CipherType::Card(Box::new(bitwarden_exporters::Card { cardholder_name: c.cardholder_name, exp_month: c.exp_month, @@ -129,7 +129,7 @@ impl TryFrom for bitwarden_exporters::Cipher { })) } CipherType::Identity => { - let i = value.identity.ok_or(Error::MissingFields)?; + let i = require!(value.identity); bitwarden_exporters::CipherType::Identity(Box::new(bitwarden_exporters::Identity { title: i.title, first_name: i.first_name, @@ -154,7 +154,7 @@ impl TryFrom for bitwarden_exporters::Cipher { }; Ok(Self { - id: value.id.ok_or(Error::MissingFields)?, + id: require!(value.id), folder_id: value.folder_id, name: value.name, notes: value.notes, 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..fa85b5969 100644 --- a/crates/bitwarden/src/vault/cipher/cipher.rs +++ b/crates/bitwarden/src/vault/cipher/cipher.rs @@ -15,7 +15,7 @@ use super::{ login, secure_note, }; use crate::{ - error::{Error, Result}, + error::{require, Error, Result}, vault::password_history, }; @@ -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(()) } @@ -384,9 +384,9 @@ impl TryFrom for Cipher { organization_id: cipher.organization_id, folder_id: cipher.folder_id, collection_ids: cipher.collection_ids.unwrap_or_default(), - name: EncString::try_from_optional(cipher.name)?.ok_or(Error::MissingFields)?, + name: require!(EncString::try_from_optional(cipher.name)?), notes: EncString::try_from_optional(cipher.notes)?, - r#type: cipher.r#type.ok_or(Error::MissingFields)?.into(), + r#type: require!(cipher.r#type).into(), login: cipher.login.map(|l| (*l).try_into()).transpose()?, identity: cipher.identity.map(|i| (*i).try_into()).transpose()?, card: cipher.card.map(|c| (*c).try_into()).transpose()?, @@ -412,9 +412,9 @@ impl TryFrom for Cipher { .password_history .map(|p| p.into_iter().map(|p| p.try_into()).collect()) .transpose()?, - creation_date: cipher.creation_date.ok_or(Error::MissingFields)?.parse()?, + creation_date: require!(cipher.creation_date).parse()?, deleted_date: cipher.deleted_date.map(|d| d.parse()).transpose()?, - revision_date: cipher.revision_date.ok_or(Error::MissingFields)?.parse()?, + revision_date: require!(cipher.revision_date).parse()?, key: EncString::try_from_optional(cipher.key)?, }) } diff --git a/crates/bitwarden/src/vault/cipher/field.rs b/crates/bitwarden/src/vault/cipher/field.rs index 25a318d6f..1f18a9537 100644 --- a/crates/bitwarden/src/vault/cipher/field.rs +++ b/crates/bitwarden/src/vault/cipher/field.rs @@ -7,7 +7,7 @@ use serde::{Deserialize, Serialize}; use serde_repr::{Deserialize_repr, Serialize_repr}; use super::linked_id::LinkedIdType; -use crate::error::{Error, Result}; +use crate::error::{require, Error, Result}; #[derive(Clone, Copy, Serialize_repr, Deserialize_repr, Debug, JsonSchema)] #[repr(u8)] @@ -70,7 +70,7 @@ impl TryFrom for Field { Ok(Self { name: EncString::try_from_optional(model.name)?, value: EncString::try_from_optional(model.value)?, - r#type: model.r#type.map(|t| t.into()).ok_or(Error::MissingFields)?, + r#type: require!(model.r#type).into(), linked_id: model .linked_id .map(|id| (id as u32).try_into()) diff --git a/crates/bitwarden/src/vault/cipher/linked_id.rs b/crates/bitwarden/src/vault/cipher/linked_id.rs index 77429438e..b52d756c6 100644 --- a/crates/bitwarden/src/vault/cipher/linked_id.rs +++ b/crates/bitwarden/src/vault/cipher/linked_id.rs @@ -112,7 +112,7 @@ impl TryFrom for LinkedIdType { 416 => Ok(LinkedIdType::Identity(IdentityLinkedIdType::FirstName)), 417 => Ok(LinkedIdType::Identity(IdentityLinkedIdType::LastName)), 418 => Ok(LinkedIdType::Identity(IdentityLinkedIdType::FullName)), - _ => Err(Error::MissingFields), + _ => Err(Error::MissingFields("LinkedIdType")), } } } diff --git a/crates/bitwarden/src/vault/cipher/login.rs b/crates/bitwarden/src/vault/cipher/login.rs index e5731dbee..aed691d9a 100644 --- a/crates/bitwarden/src/vault/cipher/login.rs +++ b/crates/bitwarden/src/vault/cipher/login.rs @@ -8,7 +8,7 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use serde_repr::{Deserialize_repr, Serialize_repr}; -use crate::error::{Error, Result}; +use crate::error::{require, Error, Result}; #[derive(Clone, Copy, Serialize_repr, Deserialize_repr, Debug, JsonSchema)] #[repr(u8)] @@ -222,22 +222,22 @@ impl TryFrom for Fido2Cre fn try_from(value: bitwarden_api_api::models::CipherFido2CredentialModel) -> Result { Ok(Self { - credential_id: value.credential_id.ok_or(Error::MissingFields)?.parse()?, - key_type: value.key_type.ok_or(Error::MissingFields)?.parse()?, - key_algorithm: value.key_algorithm.ok_or(Error::MissingFields)?.parse()?, - key_curve: value.key_curve.ok_or(Error::MissingFields)?.parse()?, - key_value: value.key_value.ok_or(Error::MissingFields)?.parse()?, - rp_id: value.rp_id.ok_or(Error::MissingFields)?.parse()?, + credential_id: require!(value.credential_id).parse()?, + key_type: require!(value.key_type).parse()?, + key_algorithm: require!(value.key_algorithm).parse()?, + key_curve: require!(value.key_curve).parse()?, + key_value: require!(value.key_value).parse()?, + rp_id: require!(value.rp_id).parse()?, user_handle: EncString::try_from_optional(value.user_handle) .ok() .flatten(), user_name: EncString::try_from_optional(value.user_name).ok().flatten(), - counter: value.counter.ok_or(Error::MissingFields)?.parse()?, + counter: require!(value.counter).parse()?, rp_name: EncString::try_from_optional(value.rp_name).ok().flatten(), user_display_name: EncString::try_from_optional(value.user_display_name) .ok() .flatten(), - discoverable: value.discoverable.ok_or(Error::MissingFields)?.parse()?, + discoverable: require!(value.discoverable).parse()?, creation_date: value.creation_date.parse()?, }) } diff --git a/crates/bitwarden/src/vault/cipher/secure_note.rs b/crates/bitwarden/src/vault/cipher/secure_note.rs index eba4ea2fb..ebcbc3fa4 100644 --- a/crates/bitwarden/src/vault/cipher/secure_note.rs +++ b/crates/bitwarden/src/vault/cipher/secure_note.rs @@ -4,7 +4,7 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use serde_repr::{Deserialize_repr, Serialize_repr}; -use crate::error::{Error, Result}; +use crate::error::{require, Error, Result}; #[derive(Clone, Copy, Serialize_repr, Deserialize_repr, Debug, JsonSchema)] #[repr(u8)] @@ -48,7 +48,7 @@ impl TryFrom for SecureNote { fn try_from(model: CipherSecureNoteModel) -> Result { Ok(Self { - r#type: model.r#type.map(|t| t.into()).ok_or(Error::MissingFields)?, + r#type: require!(model.r#type).into(), }) } } diff --git a/crates/bitwarden/src/vault/collection.rs b/crates/bitwarden/src/vault/collection.rs index d20e5d729..ce881cb98 100644 --- a/crates/bitwarden/src/vault/collection.rs +++ b/crates/bitwarden/src/vault/collection.rs @@ -6,7 +6,7 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use uuid::Uuid; -use crate::error::{Error, Result}; +use crate::error::{require, Error, Result}; #[derive(Serialize, Deserialize, Debug, JsonSchema)] #[serde(rename_all = "camelCase", deny_unknown_fields)] @@ -66,8 +66,8 @@ impl TryFrom for Collection { fn try_from(collection: CollectionDetailsResponseModel) -> Result { Ok(Collection { id: collection.id, - organization_id: collection.organization_id.ok_or(Error::MissingFields)?, - name: collection.name.ok_or(Error::MissingFields)?.parse()?, + organization_id: require!(collection.organization_id), + name: require!(collection.name).parse()?, external_id: collection.external_id, hide_passwords: collection.hide_passwords.unwrap_or(false), read_only: collection.read_only.unwrap_or(false), diff --git a/crates/bitwarden/src/vault/folder.rs b/crates/bitwarden/src/vault/folder.rs index edd1cac42..65b18e54b 100644 --- a/crates/bitwarden/src/vault/folder.rs +++ b/crates/bitwarden/src/vault/folder.rs @@ -7,7 +7,7 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use uuid::Uuid; -use crate::error::{Error, Result}; +use crate::error::{require, Error, Result}; #[derive(Serialize, Deserialize, Debug, JsonSchema)] #[serde(rename_all = "camelCase")] @@ -55,8 +55,8 @@ impl TryFrom for Folder { fn try_from(folder: FolderResponseModel) -> Result { Ok(Folder { id: folder.id, - name: EncString::try_from_optional(folder.name)?.ok_or(Error::MissingFields)?, - revision_date: folder.revision_date.ok_or(Error::MissingFields)?.parse()?, + name: require!(EncString::try_from_optional(folder.name)?), + revision_date: require!(folder.revision_date).parse()?, }) } } diff --git a/crates/bitwarden/src/vault/send.rs b/crates/bitwarden/src/vault/send.rs index 144933899..37e6efcc9 100644 --- a/crates/bitwarden/src/vault/send.rs +++ b/crates/bitwarden/src/vault/send.rs @@ -13,7 +13,7 @@ use serde::{Deserialize, Serialize}; use serde_repr::{Deserialize_repr, Serialize_repr}; use uuid::Uuid; -use crate::error::{Error, Result}; +use crate::error::{require, Error, Result}; const SEND_ITERATIONS: u32 = 100_000; @@ -308,19 +308,19 @@ impl TryFrom for Send { Ok(Send { id: send.id, access_id: send.access_id, - name: send.name.ok_or(Error::MissingFields)?.parse()?, + name: require!(send.name).parse()?, notes: EncString::try_from_optional(send.notes)?, - key: send.key.ok_or(Error::MissingFields)?.parse()?, + key: require!(send.key).parse()?, password: send.password, - r#type: send.r#type.ok_or(Error::MissingFields)?.into(), + r#type: require!(send.r#type).into(), file: send.file.map(|f| (*f).try_into()).transpose()?, text: send.text.map(|t| (*t).try_into()).transpose()?, max_access_count: send.max_access_count.map(|s| s as u32), - access_count: send.access_count.ok_or(Error::MissingFields)? as u32, + access_count: require!(send.access_count) as u32, disabled: send.disabled.unwrap_or(false), hide_email: send.hide_email.unwrap_or(false), - revision_date: send.revision_date.ok_or(Error::MissingFields)?.parse()?, - deletion_date: send.deletion_date.ok_or(Error::MissingFields)?.parse()?, + revision_date: require!(send.revision_date).parse()?, + deletion_date: require!(send.deletion_date).parse()?, expiration_date: send.expiration_date.map(|s| s.parse()).transpose()?, }) } @@ -341,7 +341,7 @@ impl TryFrom for SendFile { fn try_from(file: SendFileModel) -> Result { Ok(SendFile { id: file.id, - file_name: file.file_name.ok_or(Error::MissingFields)?.parse()?, + file_name: require!(file.file_name).parse()?, size: file.size.map(|v| v.to_string()), size_name: file.size_name, }) @@ -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); }