Skip to content

Commit

Permalink
Add sensitive types to SymmetricCryptoKey (#672)
Browse files Browse the repository at this point in the history
## Type of change
```
- [ ] Bug fix
- [ ] New feature development
- [x] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other
```

## Objective
This is a small subset of #536, which only contains the minimum code to
protect the import/export functions on SymmetricCryptoKey. It also
enables the from_base64 test on SymmetricCryptoKey as that passes now.

After this PR is merged I'll be expanding the use of Sensitive to other
parts of the codebase while adding some extra memory testing checks to
validate that it works for them.
  • Loading branch information
dani-garcia authored Apr 1, 2024
1 parent 9cd9a15 commit 538b9f2
Show file tree
Hide file tree
Showing 26 changed files with 381 additions and 105 deletions.
16 changes: 9 additions & 7 deletions crates/bitwarden-crypto/src/enc_string/symmetric.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand Down
34 changes: 15 additions & 19 deletions crates/bitwarden-crypto/src/keys/device_key.rs
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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]
Expand All @@ -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()?
Expand All @@ -65,25 +63,25 @@ impl DeviceKey {
protected_user_key: AsymmetricEncString,
) -> Result<SymmetricCryptoKey> {
let device_private_key: Vec<u8> = 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<u8> = protected_user_key.decrypt_with_key(&device_private_key)?;
let user_key: SymmetricCryptoKey = dec.as_mut_slice().try_into()?;
let dec: Vec<u8> = 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<SensitiveString> for DeviceKey {
type Error = CryptoError;

fn from_str(s: &str) -> Result<Self, Self::Err> {
let key = s.parse::<SymmetricCryptoKey>()?;
Ok(DeviceKey(key))
fn try_from(value: SensitiveString) -> Result<Self, Self::Error> {
SymmetricCryptoKey::try_from(value).map(DeviceKey)
}
}

Expand All @@ -98,10 +96,8 @@ mod tests {

let result = DeviceKey::trust_device(&key).unwrap();

let decrypted = result
.device_key
.parse::<DeviceKey>()
.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,
Expand Down
2 changes: 1 addition & 1 deletion crates/bitwarden-crypto/src/keys/master_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion crates/bitwarden-crypto/src/keys/mod.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
4 changes: 2 additions & 2 deletions crates/bitwarden-crypto/src/keys/shareable_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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==");
}
}
53 changes: 28 additions & 25 deletions crates/bitwarden-crypto/src/keys/symmetric_crypto_key.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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<Vec<u8>> {
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<SensitiveString> for SymmetricCryptoKey {
type Error = CryptoError;

fn from_str(s: &str) -> Result<Self, Self::Err> {
let mut bytes = STANDARD.decode(s).map_err(|_| CryptoError::InvalidKey)?;
SymmetricCryptoKey::try_from(bytes.as_mut_slice())
fn try_from(value: SensitiveString) -> Result<Self, Self::Error> {
SymmetricCryptoKey::try_from(value.decode_base64(STANDARD)?)
}
}

impl TryFrom<SensitiveVec> for SymmetricCryptoKey {
type Error = CryptoError;

fn try_from(mut value: SensitiveVec) -> Result<Self, Self::Error> {
let val = value.expose_mut();
SymmetricCryptoKey::try_from(val.as_mut_slice())
}
}

Expand Down Expand Up @@ -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());
}
}
2 changes: 2 additions & 0 deletions crates/bitwarden-crypto/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!();
Expand Down
16 changes: 16 additions & 0 deletions crates/bitwarden-crypto/src/sensitive/decrypted.rs
Original file line number Diff line number Diff line change
@@ -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<V> = Sensitive<V>;
pub type DecryptedVec = Decrypted<Vec<u8>>;
pub type DecryptedString = Decrypted<String>;

impl<T: KeyEncryptable<Key, Output> + Zeroize + Clone, Key: CryptoKey, Output>
KeyEncryptable<Key, Output> for Decrypted<T>
{
fn encrypt_with_key(self, key: &Key) -> Result<Output, CryptoError> {
self.value.clone().encrypt_with_key(key)
}
}
5 changes: 5 additions & 0 deletions crates/bitwarden-crypto/src/sensitive/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#[allow(clippy::module_inception)]
mod sensitive;
pub use sensitive::{Sensitive, SensitiveString, SensitiveVec};
mod decrypted;
pub use decrypted::{Decrypted, DecryptedString, DecryptedVec};
Loading

0 comments on commit 538b9f2

Please sign in to comment.