Skip to content

Commit

Permalink
Improve zeroization of key buffer (#1069)
Browse files Browse the repository at this point in the history
Currently, the temporary buffer used for deriving shareable keys is
[manually
zeroized](https://github.com/bitwarden/sdk/blob/76417172489d5790babbe14bb8c6ad8b3aac2a33/crates/bitwarden-crypto/src/keys/shareable_key.rs#L32-L33).
While documentation indicates that preceding `expect` calls cannot fail,
this still seems brittle to future changes.

This PR places the buffer into a `Zeroizing` wrapper. It is still the
case that zeroization may not occur on a panic, but this was already the
case with the existing implementation, which would never zeroize in such
a case.
  • Loading branch information
AaronFeickert authored Sep 23, 2024
1 parent eeda462 commit 7ff270d
Showing 1 changed file with 8 additions and 9 deletions.
17 changes: 8 additions & 9 deletions crates/bitwarden-crypto/src/keys/shareable_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::pin::Pin;
use aes::cipher::typenum::U64;
use generic_array::GenericArray;
use hmac::Mac;
use zeroize::{Zeroize, Zeroizing};
use zeroize::Zeroizing;

use crate::{
keys::SymmetricCryptoKey,
Expand All @@ -20,18 +20,17 @@ pub fn derive_shareable_key(
info: Option<&str>,
) -> SymmetricCryptoKey {
// Because all inputs are fixed size, we can unwrap all errors here without issue
let mut res = PbkdfSha256Hmac::new_from_slice(format!("bitwarden-{}", name).as_bytes())
.expect("hmac new_from_slice should not fail")
.chain_update(secret)
.finalize()
.into_bytes();
let res = Zeroizing::new(
PbkdfSha256Hmac::new_from_slice(format!("bitwarden-{}", name).as_bytes())
.expect("hmac new_from_slice should not fail")
.chain_update(secret)
.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")
}

Expand Down

0 comments on commit 7ff270d

Please sign in to comment.