From 99b0b622513c8bef75aa4738c59262cfb0ff6162 Mon Sep 17 00:00:00 2001 From: Oscar Hinton Date: Thu, 11 Jan 2024 13:22:13 +0100 Subject: [PATCH] Refactor make_user_key to call SymmetricCryptoKey::generate (#495) The `MasterKey` previously had intimate knowledge about `SymmetricCryptKey` in order to be able to generate one. This refactor extracts the knowledge back into `SymmetricCryptoKey` by creating a `SymmetricCryptoKey::generate`. --- .../src/crypto/enc_string/symmetric.rs | 6 ++-- crates/bitwarden/src/crypto/master_key.rs | 15 ++++----- crates/bitwarden/src/crypto/mod.rs | 2 ++ .../src/crypto/symmetric_crypto_key.rs | 33 ++++++++++++++----- 4 files changed, 37 insertions(+), 19 deletions(-) diff --git a/crates/bitwarden/src/crypto/enc_string/symmetric.rs b/crates/bitwarden/src/crypto/enc_string/symmetric.rs index 798994a1c..3fe0e8ef8 100644 --- a/crates/bitwarden/src/crypto/enc_string/symmetric.rs +++ b/crates/bitwarden/src/crypto/enc_string/symmetric.rs @@ -274,11 +274,13 @@ impl schemars::JsonSchema for EncString { #[cfg(test)] mod tests { use super::EncString; - use crate::crypto::{KeyDecryptable, KeyEncryptable, SymmetricCryptoKey}; + use crate::crypto::{ + symmetric_crypto_key::derive_symmetric_key, KeyDecryptable, KeyEncryptable, + }; #[test] fn test_enc_string_roundtrip() { - let key = SymmetricCryptoKey::generate("test"); + let key = derive_symmetric_key("test"); let test_string = "encrypted_test_string".to_string(); let cipher = test_string.clone().encrypt_with_key(&key).unwrap(); diff --git a/crates/bitwarden/src/crypto/master_key.rs b/crates/bitwarden/src/crypto/master_key.rs index e259ec334..726759ac7 100644 --- a/crates/bitwarden/src/crypto/master_key.rs +++ b/crates/bitwarden/src/crypto/master_key.rs @@ -1,6 +1,5 @@ use aes::cipher::{generic_array::GenericArray, typenum::U32}; use base64::{engine::general_purpose::STANDARD, Engine}; -use rand::Rng; use schemars::JsonSchema; use sha2::Digest; @@ -69,10 +68,7 @@ fn make_user_key( mut rng: impl rand::RngCore, master_key: &MasterKey, ) -> Result<(UserKey, EncString)> { - let mut user_key = [0u8; 64]; - rng.fill(&mut user_key); - - let user_key = SymmetricCryptoKey::try_from(user_key.as_slice())?; + let user_key = SymmetricCryptoKey::generate(&mut rng); let protected = master_key.encrypt_user_key(&user_key)?; Ok((UserKey::new(user_key), protected)) } @@ -134,7 +130,10 @@ mod tests { use rand::SeedableRng; use super::{make_user_key, stretch_master_key, HashPurpose, MasterKey}; - use crate::{client::kdf::Kdf, crypto::SymmetricCryptoKey}; + use crate::{ + client::kdf::Kdf, + crypto::{symmetric_crypto_key::derive_symmetric_key, SymmetricCryptoKey}, + }; #[test] fn test_master_key_derive_pbkdf2() { @@ -292,9 +291,9 @@ mod tests { #[test] fn test_make_user_key2() { - let master_key = MasterKey(SymmetricCryptoKey::generate("test1")); + let master_key = MasterKey(derive_symmetric_key("test1")); - let user_key = SymmetricCryptoKey::generate("test2"); + let user_key = derive_symmetric_key("test2"); let encrypted = master_key.encrypt_user_key(&user_key).unwrap(); let decrypted = master_key.decrypt_user_key(encrypted).unwrap(); diff --git a/crates/bitwarden/src/crypto/mod.rs b/crates/bitwarden/src/crypto/mod.rs index 94db974d3..5de0cdd06 100644 --- a/crates/bitwarden/src/crypto/mod.rs +++ b/crates/bitwarden/src/crypto/mod.rs @@ -23,6 +23,7 @@ use aes::cipher::{generic_array::GenericArray, ArrayLength, Unsigned}; use hmac::digest::OutputSizeUser; +#[cfg(any(test, feature = "internal"))] use rand::{ distributions::{Distribution, Standard}, Rng, @@ -79,6 +80,7 @@ fn hkdf_expand>(prk: &[u8], info: Option<&str>) -> Result() -> T where Standard: Distribution, diff --git a/crates/bitwarden/src/crypto/symmetric_crypto_key.rs b/crates/bitwarden/src/crypto/symmetric_crypto_key.rs index f6eda5e48..5487dfd98 100644 --- a/crates/bitwarden/src/crypto/symmetric_crypto_key.rs +++ b/crates/bitwarden/src/crypto/symmetric_crypto_key.rs @@ -2,11 +2,9 @@ use std::str::FromStr; use aes::cipher::{generic_array::GenericArray, typenum::U32}; use base64::{engine::general_purpose::STANDARD, Engine}; +use rand::Rng; -use crate::{ - crypto::{derive_shareable_key, generate_random_bytes}, - error::{CryptoError, Error}, -}; +use crate::error::{CryptoError, Error}; /// A symmetric encryption key. Used to encrypt and decrypt [`EncString`](crate::crypto::EncString) pub struct SymmetricCryptoKey { @@ -18,9 +16,18 @@ impl SymmetricCryptoKey { const KEY_LEN: usize = 32; const MAC_LEN: usize = 32; - pub fn generate(name: &str) -> Self { - let secret: [u8; 16] = generate_random_bytes(); - derive_shareable_key(secret, name, None) + /// Generate a new random [SymmetricCryptoKey] + pub fn generate(mut rng: impl rand::RngCore) -> Self { + let mut key = [0u8; Self::KEY_LEN]; + let mut mac_key = [0u8; Self::MAC_LEN]; + + rng.fill(&mut key); + rng.fill(&mut mac_key); + + SymmetricCryptoKey { + key: key.into(), + mac_key: Some(mac_key.into()), + } } pub fn to_base64(&self) -> String { @@ -81,15 +88,23 @@ impl std::fmt::Debug for SymmetricCryptoKey { } } +#[cfg(test)] +pub fn derive_symmetric_key(name: &str) -> SymmetricCryptoKey { + use crate::crypto::{derive_shareable_key, generate_random_bytes}; + + let secret: [u8; 16] = generate_random_bytes(); + derive_shareable_key(secret, name, None) +} + #[cfg(test)] mod tests { use std::str::FromStr; - use super::SymmetricCryptoKey; + use super::{derive_symmetric_key, SymmetricCryptoKey}; #[test] fn test_symmetric_crypto_key() { - let key = SymmetricCryptoKey::generate("test"); + let key = derive_symmetric_key("test"); let key2 = SymmetricCryptoKey::from_str(&key.to_base64()).unwrap(); assert_eq!(key.key, key2.key); assert_eq!(key.mac_key, key2.mac_key);