Mark generate_random_bytes to return sensitive (#726)

Make `generate_random_bytes` return `Sensitive`. Also migrate
`derive_shareable_key` to accept a sensitive input.

.vscode/settings.json

@@ -21,6 +21,7 @@
     "totp",
     "uniffi",
     "wordlist",
+    "Zeroize",
     "zxcvbn"
   ]
 }

crates/bitwarden-crypto/src/keys/shareable_key.rs

@@ -3,47 +3,58 @@ 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.
 ///
 /// 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<Box<GenericArray<u8, U64>>> =
         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==");
     }
 }

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)
 }
 

crates/bitwarden-crypto/src/sensitive/sensitive.rs

@@ -54,6 +54,13 @@ impl<V: Zeroize> Sensitive<V> {
     }
 }
 
+/// Helper to convert a `Sensitive<[u8, N]>` to a `SensitiveVec`.
+impl<const N: usize> From<Sensitive<[u8; N]>> for SensitiveVec {
+    fn from(sensitive: Sensitive<[u8; N]>) -> Self {
+        SensitiveVec::new(Box::new(sensitive.value.to_vec()))
+    }
+}
+
 /// Helper to convert a `Sensitive<Vec<u8>>` to a `Sensitive<String>`, care is taken to ensure any
 /// intermediate copies are zeroed to avoid leaking sensitive data.
 impl TryFrom<SensitiveVec> for SensitiveString {

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<sha2::Sha256>;
 pub(crate) const PBKDF_SHA256_HMAC_OUT_SIZE: usize =
@@ -30,11 +31,12 @@ pub(crate) fn hkdf_expand<T: ArrayLength<u8>>(
 }
 
 /// Generate random bytes that are cryptographically secure
-pub fn generate_random_bytes<T>() -> T
+pub fn generate_random_bytes<T>() -> Sensitive<T>
 where
     Standard: Distribution<T>,
+    T: Zeroize,
 {
-    rand::thread_rng().gen()
+    Sensitive::new(Box::new(rand::thread_rng().gen::<T>()))
 }
 
 pub fn pbkdf2(password: &[u8], salt: &[u8], rounds: u32) -> [u8; PBKDF_SHA256_HMAC_OUT_SIZE] {

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,

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"));
 

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<SymmetricCryptoKey, CryptoError> {
-        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<SymmetricCryptoKey, Send> 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),