diff --git a/.vscode/settings.json b/.vscode/settings.json index e92fcfb76..a1b091d7f 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -21,6 +21,7 @@ "totp", "uniffi", "wordlist", + "Zeroize", "zxcvbn" ] } diff --git a/crates/bitwarden-crypto/src/keys/shareable_key.rs b/crates/bitwarden-crypto/src/keys/shareable_key.rs index e84e05ca3..490e6b56e 100644 --- a/crates/bitwarden-crypto/src/keys/shareable_key.rs +++ b/crates/bitwarden-crypto/src/keys/shareable_key.rs @@ -3,10 +3,12 @@ use std::pin::Pin; use aes::cipher::typenum::U64; use generic_array::GenericArray; use hmac::Mac; +use zeroize::Zeroize; use crate::{ keys::SymmetricCryptoKey, util::{hkdf_expand, PbkdfSha256Hmac}, + Sensitive, }; /// Derive a shareable key using hkdf from secret and name. @@ -14,36 +16,45 @@ use crate::{ /// A specialized variant of this function was called `CryptoService.makeSendKey` in the Bitwarden /// `clients` repository. pub fn derive_shareable_key( - secret: [u8; 16], + secret: Sensitive<[u8; 16]>, name: &str, info: Option<&str>, ) -> SymmetricCryptoKey { // Because all inputs are fixed size, we can unwrap all errors here without issue - - // TODO: Are these the final `key` and `info` parameters or should we change them? I followed - // the pattern used for sends - let res = PbkdfSha256Hmac::new_from_slice(format!("bitwarden-{}", name).as_bytes()) + let mut res = PbkdfSha256Hmac::new_from_slice(format!("bitwarden-{}", name).as_bytes()) .expect("hmac new_from_slice should not fail") - .chain_update(secret) + .chain_update(secret.expose()) .finalize() .into_bytes(); let mut key: Pin>> = hkdf_expand(&res, info).expect("Input is a valid size"); + // Zeroize the temporary buffer + res.zeroize(); + SymmetricCryptoKey::try_from(key.as_mut_slice()).expect("Key is a valid size") } #[cfg(test)] mod tests { use super::derive_shareable_key; + use crate::Sensitive; #[test] fn test_derive_shareable_key() { - let key = derive_shareable_key(*b"&/$%F1a895g67HlX", "test_key", None); + let key = derive_shareable_key( + Sensitive::new(Box::new(*b"&/$%F1a895g67HlX")), + "test_key", + None, + ); assert_eq!(key.to_base64().expose(), "4PV6+PcmF2w7YHRatvyMcVQtI7zvCyssv/wFWmzjiH6Iv9altjmDkuBD1aagLVaLezbthbSe+ktR+U6qswxNnQ=="); - let key = derive_shareable_key(*b"67t9b5g67$%Dh89n", "test_key", Some("test")); + let key = derive_shareable_key( + Sensitive::new(Box::new(*b"67t9b5g67$%Dh89n")), + "test_key", + Some("test"), + ); 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 23d09cbca..b58468c7f 100644 --- a/crates/bitwarden-crypto/src/keys/symmetric_crypto_key.rs +++ b/crates/bitwarden-crypto/src/keys/symmetric_crypto_key.rs @@ -134,9 +134,9 @@ impl std::fmt::Debug for SymmetricCryptoKey { #[cfg(test)] pub fn derive_symmetric_key(name: &str) -> SymmetricCryptoKey { - use crate::{derive_shareable_key, generate_random_bytes}; + use crate::{derive_shareable_key, generate_random_bytes, Sensitive}; - let secret: [u8; 16] = generate_random_bytes(); + let secret: Sensitive<[u8; 16]> = generate_random_bytes(); derive_shareable_key(secret, name, None) } diff --git a/crates/bitwarden-crypto/src/sensitive/sensitive.rs b/crates/bitwarden-crypto/src/sensitive/sensitive.rs index 996b20330..481058064 100644 --- a/crates/bitwarden-crypto/src/sensitive/sensitive.rs +++ b/crates/bitwarden-crypto/src/sensitive/sensitive.rs @@ -54,6 +54,13 @@ impl Sensitive { } } +/// Helper to convert a `Sensitive<[u8, N]>` to a `SensitiveVec`. +impl From> for SensitiveVec { + fn from(sensitive: Sensitive<[u8; N]>) -> Self { + SensitiveVec::new(Box::new(sensitive.value.to_vec())) + } +} + /// 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 { diff --git a/crates/bitwarden-crypto/src/util.rs b/crates/bitwarden-crypto/src/util.rs index ba60db366..b38cb0e00 100644 --- a/crates/bitwarden-crypto/src/util.rs +++ b/crates/bitwarden-crypto/src/util.rs @@ -7,8 +7,9 @@ use rand::{ distributions::{Distribution, Standard}, Rng, }; +use zeroize::Zeroize; -use crate::{CryptoError, Result}; +use crate::{CryptoError, Result, Sensitive}; pub(crate) type PbkdfSha256Hmac = hmac::Hmac; pub(crate) const PBKDF_SHA256_HMAC_OUT_SIZE: usize = @@ -30,11 +31,12 @@ pub(crate) fn hkdf_expand>( } /// Generate random bytes that are cryptographically secure -pub fn generate_random_bytes() -> T +pub fn generate_random_bytes() -> Sensitive where Standard: Distribution, + T: Zeroize, { - rand::thread_rng().gen() + Sensitive::new(Box::new(rand::thread_rng().gen::())) } pub fn pbkdf2(password: &[u8], salt: &[u8], rounds: u32) -> [u8; PBKDF_SHA256_HMAC_OUT_SIZE] { diff --git a/crates/bitwarden-exporters/src/encrypted_json.rs b/crates/bitwarden-exporters/src/encrypted_json.rs index 1bbfd2660..6dabc90d5 100644 --- a/crates/bitwarden-exporters/src/encrypted_json.rs +++ b/crates/bitwarden-exporters/src/encrypted_json.rs @@ -1,5 +1,7 @@ -use base64::{engine::general_purpose::STANDARD, Engine}; -use bitwarden_crypto::{generate_random_bytes, Kdf, KeyEncryptable, PinKey}; +use base64::engine::general_purpose::STANDARD; +use bitwarden_crypto::{ + generate_random_bytes, Kdf, KeyEncryptable, PinKey, Sensitive, SensitiveVec, +}; use serde::Serialize; use thiserror::Error; use uuid::Uuid; @@ -43,16 +45,16 @@ pub(crate) fn export_encrypted_json( ), }; - let salt: [u8; 16] = generate_random_bytes(); - let salt = STANDARD.encode(salt); - let key = PinKey::derive(password.as_bytes(), salt.as_bytes(), &kdf)?; + let salt: Sensitive<[u8; 16]> = generate_random_bytes(); + let salt = SensitiveVec::from(salt).encode_base64(STANDARD); + let key = PinKey::derive(password.as_bytes(), salt.expose().as_bytes(), &kdf)?; let enc_key_validation = Uuid::new_v4().to_string(); let encrypted_export = EncryptedJsonExport { encrypted: true, password_protected: true, - salt, + salt: salt.expose().to_string(), kdf_type, kdf_iterations, kdf_memory, diff --git a/crates/bitwarden/src/auth/access_token.rs b/crates/bitwarden/src/auth/access_token.rs index 4b6f050c8..0bf43d29a 100644 --- a/crates/bitwarden/src/auth/access_token.rs +++ b/crates/bitwarden/src/auth/access_token.rs @@ -1,7 +1,7 @@ use std::{fmt::Debug, str::FromStr}; use base64::Engine; -use bitwarden_crypto::{derive_shareable_key, SymmetricCryptoKey}; +use bitwarden_crypto::{derive_shareable_key, Sensitive, SymmetricCryptoKey}; use uuid::Uuid; use crate::{error::AccessTokenInvalidError, util::STANDARD_INDIFFERENT}; @@ -45,12 +45,12 @@ impl FromStr for AccessToken { let encryption_key = STANDARD_INDIFFERENT .decode(encryption_key) .map_err(AccessTokenInvalidError::InvalidBase64)?; - let encryption_key: [u8; 16] = encryption_key.try_into().map_err(|e: Vec<_>| { + let encryption_key = Sensitive::new(encryption_key.try_into().map_err(|e: Vec<_>| { AccessTokenInvalidError::InvalidBase64Length { expected: 16, got: e.len(), } - })?; + })?); let encryption_key = derive_shareable_key(encryption_key, "accesstoken", Some("sm-access-token")); diff --git a/crates/bitwarden/src/vault/send.rs b/crates/bitwarden/src/vault/send.rs index 81341d0e3..550da84aa 100644 --- a/crates/bitwarden/src/vault/send.rs +++ b/crates/bitwarden/src/vault/send.rs @@ -5,7 +5,7 @@ use base64::{ use bitwarden_api_api::models::{SendFileModel, SendResponseModel, SendTextModel}; use bitwarden_crypto::{ derive_shareable_key, generate_random_bytes, CryptoError, EncString, KeyDecryptable, - KeyEncryptable, LocateKey, SymmetricCryptoKey, + KeyEncryptable, LocateKey, Sensitive, SensitiveVec, SymmetricCryptoKey, }; use chrono::{DateTime, Utc}; use schemars::JsonSchema; @@ -149,7 +149,9 @@ impl Send { } fn derive_shareable_key(key: &[u8]) -> Result { - let key = key.try_into().map_err(|_| CryptoError::InvalidKeyLen)?; + let key = Sensitive::new(Box::new( + key.try_into().map_err(|_| CryptoError::InvalidKeyLen)?, + )); Ok(derive_shareable_key(key, "send", Some("send"))) } } @@ -265,8 +267,8 @@ impl KeyEncryptable for SendView { .map_err(|_| CryptoError::InvalidKey)?, // New send, generate random key (None, None) => { - let key: [u8; 16] = generate_random_bytes(); - key.to_vec() + let key: Sensitive<[u8; 16]> = generate_random_bytes(); + SensitiveVec::from(key).expose().to_owned() } // Existing send without key _ => return Err(CryptoError::InvalidKey),