From d5f2ae5a25d3db53b71965010337c25f262e3ceb Mon Sep 17 00:00:00 2001 From: Hinton Date: Thu, 25 Jan 2024 10:38:22 +0100 Subject: [PATCH] Document and introduce type alias --- crates/bitwarden-crypto/src/decrypted.rs | 12 ++++++++++-- .../bitwarden-crypto/src/enc_string/symmetric.rs | 15 ++++++++------- crates/bitwarden-crypto/src/keys/device_key.rs | 10 ++++++---- crates/bitwarden-crypto/src/keys/master_key.rs | 5 +++-- 4 files changed, 27 insertions(+), 15 deletions(-) diff --git a/crates/bitwarden-crypto/src/decrypted.rs b/crates/bitwarden-crypto/src/decrypted.rs index f267ba311..140c99a37 100644 --- a/crates/bitwarden-crypto/src/decrypted.rs +++ b/crates/bitwarden-crypto/src/decrypted.rs @@ -2,6 +2,9 @@ use zeroize::{Zeroize, ZeroizeOnDrop}; use crate::CryptoError; +/// A wrapper for decrypted values. +/// +/// Implements `Zeroize` and `ZeroizeOnDrop` to ensure that the value is zeroized on drop. Please be careful if cloning or copying the inner value using `expose` since any copy will not be zeroized. #[derive(Zeroize, ZeroizeOnDrop)] pub struct Decrypted { value: V, @@ -12,18 +15,23 @@ impl Decrypted { Self { value } } + /// Expose the inner value. By exposing the inner value, you take responsibility for ensuring that any copy of the value is zeroized. pub fn expose(&self) -> &V { &self.value } } -impl TryFrom>> for Decrypted { +/// Helper to convert a `Decrypted>` to a `Decrypted`, care is taken to ensure any intermediate copies are zeroed to avoid leaking sensitive data. +impl TryFrom for DecryptedString { type Error = CryptoError; - fn try_from(mut v: Decrypted>) -> Result { + fn try_from(mut v: DecryptedVec) -> Result { let value = std::mem::take(&mut v.value); let rtn = String::from_utf8(value).map_err(|_| CryptoError::InvalidUtf8String); rtn.map(Decrypted::new) } } + +pub type DecryptedVec = Decrypted>; +pub type DecryptedString = Decrypted; diff --git a/crates/bitwarden-crypto/src/enc_string/symmetric.rs b/crates/bitwarden-crypto/src/enc_string/symmetric.rs index ed832b32b..ee6c22b30 100644 --- a/crates/bitwarden-crypto/src/enc_string/symmetric.rs +++ b/crates/bitwarden-crypto/src/enc_string/symmetric.rs @@ -6,6 +6,7 @@ use serde::Deserialize; use super::{check_length, from_b64, from_b64_vec, split_enc_string}; use crate::{ + decrypted::{DecryptedString, DecryptedVec}, error::{CryptoError, EncStringParseError, Result}, Decrypted, KeyDecryptable, KeyEncryptable, LocateKey, SymmetricCryptoKey, }; @@ -228,8 +229,8 @@ impl KeyEncryptable for &[u8] { } } -impl KeyDecryptable>> for EncString { - fn decrypt_with_key(&self, key: &SymmetricCryptoKey) -> Result>> { +impl KeyDecryptable for EncString { + fn decrypt_with_key(&self, key: &SymmetricCryptoKey) -> Result { match self { EncString::AesCbc256_HmacSha256_B64 { iv, mac, data } => { let mac_key = key.mac_key.ok_or(CryptoError::InvalidMac)?; @@ -247,9 +248,9 @@ impl KeyEncryptable for String { } } -impl KeyDecryptable> for EncString { - fn decrypt_with_key(&self, key: &SymmetricCryptoKey) -> Result> { - let dec: Decrypted> = self.decrypt_with_key(key)?; +impl KeyDecryptable for EncString { + fn decrypt_with_key(&self, key: &SymmetricCryptoKey) -> Result { + let dec: DecryptedVec = self.decrypt_with_key(key)?; dec.try_into() } } @@ -269,7 +270,7 @@ impl schemars::JsonSchema for EncString { #[cfg(test)] mod tests { use super::EncString; - use crate::{derive_symmetric_key, Decrypted, KeyDecryptable, KeyEncryptable}; + use crate::{decrypted::DecryptedString, derive_symmetric_key, KeyDecryptable, KeyEncryptable}; #[test] fn test_enc_string_roundtrip() { @@ -278,7 +279,7 @@ mod tests { let test_string = "encrypted_test_string"; let cipher = test_string.to_owned().encrypt_with_key(&key).unwrap(); - let decrypted_str: Decrypted = cipher.decrypt_with_key(&key).unwrap(); + let decrypted_str: DecryptedString = cipher.decrypt_with_key(&key).unwrap(); assert_eq!(decrypted_str.expose(), test_string); } diff --git a/crates/bitwarden-crypto/src/keys/device_key.rs b/crates/bitwarden-crypto/src/keys/device_key.rs index 6588944fe..413194066 100644 --- a/crates/bitwarden-crypto/src/keys/device_key.rs +++ b/crates/bitwarden-crypto/src/keys/device_key.rs @@ -1,6 +1,6 @@ use crate::{ - error::Result, AsymmetricCryptoKey, AsymmetricEncString, EncString, KeyDecryptable, - KeyEncryptable, SymmetricCryptoKey, UserKey, + decrypted::DecryptedVec, error::Result, AsymmetricCryptoKey, AsymmetricEncString, EncString, + KeyDecryptable, KeyEncryptable, SymmetricCryptoKey, UserKey, }; /// Device Key @@ -60,8 +60,10 @@ impl DeviceKey { protected_device_private_key: EncString, protected_user_key: AsymmetricEncString, ) -> Result { - let device_private_key: Vec = protected_device_private_key.decrypt_with_key(&self.0)?; - let device_private_key = AsymmetricCryptoKey::from_der(device_private_key.as_slice())?; + let device_private_key: DecryptedVec = + protected_device_private_key.decrypt_with_key(&self.0)?; + let device_private_key = + AsymmetricCryptoKey::from_der(device_private_key.expose().as_slice())?; let dec: Vec = protected_user_key.decrypt_with_key(&device_private_key)?; let user_key: SymmetricCryptoKey = dec.as_slice().try_into()?; diff --git a/crates/bitwarden-crypto/src/keys/master_key.rs b/crates/bitwarden-crypto/src/keys/master_key.rs index bdd626a1a..7e60bc24b 100644 --- a/crates/bitwarden-crypto/src/keys/master_key.rs +++ b/crates/bitwarden-crypto/src/keys/master_key.rs @@ -7,6 +7,7 @@ use serde::{Deserialize, Serialize}; use sha2::Digest; use crate::{ + decrypted::DecryptedVec, util::{self, hkdf_expand}, EncString, KeyDecryptable, Result, SymmetricCryptoKey, UserKey, }; @@ -59,8 +60,8 @@ impl MasterKey { pub fn decrypt_user_key(&self, user_key: EncString) -> Result { let stretched_key = stretch_master_key(self)?; - let dec: Vec = user_key.decrypt_with_key(&stretched_key)?; - SymmetricCryptoKey::try_from(dec.as_slice()) + let dec: DecryptedVec = user_key.decrypt_with_key(&stretched_key)?; + SymmetricCryptoKey::try_from(dec.expose().as_slice()) } pub fn encrypt_user_key(&self, user_key: &SymmetricCryptoKey) -> Result {