From 32ca764888b3d6fb5152c019c8c5c439f97c8580 Mon Sep 17 00:00:00 2001 From: Isaiah Inuwa Date: Mon, 6 Oct 2025 11:46:03 -0500 Subject: [PATCH 1/7] Add methods to create rotateable key sets from PRF --- .../src/key_management/crypto.rs | 17 +- .../src/key_management/crypto_client.rs | 10 +- crates/bitwarden-crypto/src/keys/mod.rs | 2 + crates/bitwarden-crypto/src/keys/prf.rs | 44 ++++ crates/bitwarden-crypto/src/lib.rs | 3 +- .../src/store/key_rotation.rs | 201 +++++++++++++++++- crates/bitwarden-uniffi/src/crypto.rs | 8 +- 7 files changed, 275 insertions(+), 10 deletions(-) create mode 100644 crates/bitwarden-crypto/src/keys/prf.rs diff --git a/crates/bitwarden-core/src/key_management/crypto.rs b/crates/bitwarden-core/src/key_management/crypto.rs index aa53b9245..b6fa67a10 100644 --- a/crates/bitwarden-core/src/key_management/crypto.rs +++ b/crates/bitwarden-core/src/key_management/crypto.rs @@ -8,9 +8,10 @@ use std::collections::HashMap; use bitwarden_crypto::{ AsymmetricCryptoKey, CoseSerializable, CryptoError, EncString, Kdf, KeyDecryptable, - KeyEncryptable, MasterKey, Pkcs8PrivateKeyBytes, PrimitiveEncryptable, SignatureAlgorithm, - SignedPublicKey, SigningKey, SpkiPublicKeyBytes, SymmetricCryptoKey, UnsignedSharedKey, - UserKey, dangerous_get_v2_rotated_account_keys, safe::PasswordProtectedKeyEnvelopeError, + KeyEncryptable, MasterKey, Pkcs8PrivateKeyBytes, PrimitiveEncryptable, RotateableKeySet, + SignatureAlgorithm, SignedPublicKey, SigningKey, SpkiPublicKeyBytes, SymmetricCryptoKey, + UnsignedSharedKey, UserKey, dangerous_get_v2_rotated_account_keys, + derive_symmetric_key_from_prf, safe::PasswordProtectedKeyEnvelopeError, }; use bitwarden_encoding::B64; use bitwarden_error::bitwarden_error; @@ -498,6 +499,16 @@ fn derive_pin_protected_user_key( Ok(derived_key.encrypt_user_key(user_key)?) } +pub(super) fn make_prf_user_key_set( + client: &Client, + prf: B64, +) -> Result { + let prf_key = derive_symmetric_key_from_prf(prf.as_bytes())?; + let ctx = client.internal.get_key_store().context(); + let key_set = RotateableKeySet::new(&ctx, &prf_key, SymmetricKeyId::User)?; + Ok(key_set) +} + #[allow(missing_docs)] #[bitwarden_error(flat)] #[derive(Debug, thiserror::Error)] diff --git a/crates/bitwarden-core/src/key_management/crypto_client.rs b/crates/bitwarden-core/src/key_management/crypto_client.rs index 08a18168f..712a86006 100644 --- a/crates/bitwarden-core/src/key_management/crypto_client.rs +++ b/crates/bitwarden-core/src/key_management/crypto_client.rs @@ -1,4 +1,4 @@ -use bitwarden_crypto::{CryptoError, Decryptable, Kdf}; +use bitwarden_crypto::{CryptoError, Decryptable, Kdf, RotateableKeySet}; #[cfg(feature = "internal")] use bitwarden_crypto::{EncString, UnsignedSharedKey}; use bitwarden_encoding::B64; @@ -18,7 +18,7 @@ use crate::key_management::{ crypto::{ DerivePinKeyResponse, InitOrgCryptoRequest, InitUserCryptoRequest, UpdatePasswordResponse, derive_pin_key, derive_pin_user_key, enroll_admin_password_reset, get_user_encryption_key, - initialize_org_crypto, initialize_user_crypto, + initialize_org_crypto, initialize_user_crypto, make_prf_user_key_set, }, }; use crate::{ @@ -172,6 +172,12 @@ impl CryptoClient { derive_pin_user_key(&self.client, encrypted_pin) } + /// Creates a new rotateable key set for the current user key protected + /// by a key derived from the given PRF. + pub fn make_prf_user_key_set(&self, prf: B64) -> Result { + make_prf_user_key_set(&self.client, prf) + } + /// Prepares the account for being enrolled in the admin password reset feature. This encrypts /// the users [UserKey][bitwarden_crypto::UserKey] with the organization's public key. pub fn enroll_admin_password_reset( diff --git a/crates/bitwarden-crypto/src/keys/mod.rs b/crates/bitwarden-crypto/src/keys/mod.rs index 1e6cda4db..f0a263ddb 100644 --- a/crates/bitwarden-crypto/src/keys/mod.rs +++ b/crates/bitwarden-crypto/src/keys/mod.rs @@ -33,4 +33,6 @@ pub use kdf::{ default_pbkdf2_iterations, }; pub(crate) use key_id::{KEY_ID_SIZE, KeyId}; +mod prf; pub(crate) mod utils; +pub use prf::derive_symmetric_key_from_prf; diff --git a/crates/bitwarden-crypto/src/keys/prf.rs b/crates/bitwarden-crypto/src/keys/prf.rs new file mode 100644 index 000000000..84d08cbaf --- /dev/null +++ b/crates/bitwarden-crypto/src/keys/prf.rs @@ -0,0 +1,44 @@ +use crate::{CryptoError, SymmetricCryptoKey, utils::stretch_key}; + +/// Takes the output of a PRF and derives a symmetric key +pub fn derive_symmetric_key_from_prf(prf: &[u8]) -> Result { + let (secret, _) = prf + .split_at_checked(32) + .ok_or_else(|| CryptoError::InvalidKeyLen)?; + let secret: [u8; 32] = secret.try_into().unwrap(); + if secret.iter().all(|b| *b == b'\0') { + return Err(CryptoError::ZeroNumber); + } + Ok(SymmetricCryptoKey::Aes256CbcHmacKey(stretch_key( + &Box::pin(secret.into()), + )?)) +} + +#[cfg(test)] +mod tests { + use super::*; + #[test] + fn test_prf_succeeds() { + let prf = [ + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, + 24, 25, 26, 27, 28, 29, 30, 31, + ]; + derive_symmetric_key_from_prf(&prf).unwrap(); + } + + #[test] + fn test_zero_key_fails() { + let prf = [ + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, + ]; + let err = derive_symmetric_key_from_prf(&prf).unwrap_err(); + assert!(matches!(err, CryptoError::ZeroNumber)); + } + #[test] + fn test_short_prf_fails() { + let prf = [0, 1, 2, 3, 4, 5, 6, 7, 8]; + let err = derive_symmetric_key_from_prf(&prf).unwrap_err(); + assert!(matches!(err, CryptoError::InvalidKeyLen)); + } +} diff --git a/crates/bitwarden-crypto/src/lib.rs b/crates/bitwarden-crypto/src/lib.rs index 34de79131..a76d20168 100644 --- a/crates/bitwarden-crypto/src/lib.rs +++ b/crates/bitwarden-crypto/src/lib.rs @@ -32,7 +32,8 @@ mod wordlist; pub use wordlist::EFF_LONG_WORD_LIST; mod store; pub use store::{ - KeyStore, KeyStoreContext, RotatedUserKeys, dangerous_get_v2_rotated_account_keys, + KeyStore, KeyStoreContext, RotateableKeySet, RotatedUserKeys, + dangerous_get_v2_rotated_account_keys, }; mod cose; pub use cose::CoseSerializable; diff --git a/crates/bitwarden-crypto/src/store/key_rotation.rs b/crates/bitwarden-crypto/src/store/key_rotation.rs index 0d97331c2..57cb1db13 100644 --- a/crates/bitwarden-crypto/src/store/key_rotation.rs +++ b/crates/bitwarden-crypto/src/store/key_rotation.rs @@ -1,7 +1,10 @@ +use serde::{Deserialize, Serialize}; + use crate::{ - CoseKeyBytes, CoseSerializable, CryptoError, EncString, KeyEncryptable, KeyIds, - KeyStoreContext, SignedPublicKey, SignedPublicKeyMessage, SpkiPublicKeyBytes, - SymmetricCryptoKey, + AsymmetricCryptoKey, AsymmetricPublicCryptoKey, CoseKeyBytes, CoseSerializable, CryptoError, + EncString, KeyDecryptable, KeyEncryptable, KeyIds, KeyStoreContext, Pkcs8PrivateKeyBytes, + SignedPublicKey, SignedPublicKeyMessage, SpkiPublicKeyBytes, SymmetricCryptoKey, + UnsignedSharedKey, }; /// Rotated set of account keys @@ -45,6 +48,123 @@ pub fn dangerous_get_v2_rotated_account_keys( }) } +/// A set of keys where a given `EncryptionKey` is protected by an encrypted public/private +/// key-pair. The `EncryptionKey` is used to encrypt/decrypt data, while the public/private key-pair +/// is used to rotate the `EncryptionKey`. +/// +/// The `PrivateKey` is protected by an `ExternalKey`, such as a `DeviceKey`, or `PrfKey`, +/// and the `PublicKey` is protected by the `EncryptionKey`. This setup allows: +/// +/// - Access to `EncryptionKey` by knowing the `ExternalKey` +/// - Rotation to a `NewEncryptionKey` by knowing the current `EncryptionKey`, without needing access to +/// the `ExternalKey` +#[derive(Serialize, Deserialize, Debug)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +#[cfg_attr(feature = "uniffi", derive(uniffi::Record))] +#[cfg_attr( + feature = "wasm", + derive(tsify::Tsify), + tsify(into_wasm_abi, from_wasm_abi) +)] +pub struct RotateableKeySet { + /// `EncryptionKey` protected by encapsulation key + encapsulated_encryption_key: UnsignedSharedKey, + /// Encapsulation key protected by `EncryptionKey` + encrypted_encapsulation_key: EncString, + /// Decapsulation key protected by `ExternalKey` + wrapped_decapsulation_key: EncString, +} + +impl RotateableKeySet { + /// Create a set of keys to allow access to the user key via the provided + /// symmetric wrapping key while allowing the user key to be rotated. + pub fn new( + ctx: &KeyStoreContext, + wrapping_key: &SymmetricCryptoKey, + key_to_wrap: Ids::Symmetric, + ) -> Result { + let key_pair = AsymmetricCryptoKey::make(crate::PublicKeyEncryptionAlgorithm::RsaOaepSha1); + + // This uses this deprecated method and other methods directly on the other keys + // rather than the key store context because we don't want the keys to + // wind up being stored in the borrowed context. + #[allow(deprecated)] + let key_to_wrap_instance = ctx.dangerous_get_symmetric_key(key_to_wrap)?; + // encapsulate encryption key + let encapsulated_encryption_key = UnsignedSharedKey::encapsulate_key_unsigned( + key_to_wrap_instance, + &key_pair.to_public_key(), + )?; + + // wrap decapsulation key + let wrapped_decapsulation_key = key_pair.to_der()?.encrypt_with_key(wrapping_key)?; + + // wrap encapsulation key with encryption key + // Note: Usually, a public key is - by definition - public, so this should not be necessary. + // The specific use-case for this function is to enable rotateable key sets, where + // the "public key" is not public, with the intent of preventing the server from being able + // to overwrite the user key unlocked by the rotateable keyset. + let encrypted_encapsulation_key = key_pair + .to_public_key() + .to_der()? + .encrypt_with_key(&key_to_wrap_instance)?; + + Ok(RotateableKeySet { + encapsulated_encryption_key, + encrypted_encapsulation_key, + wrapped_decapsulation_key, + }) + } + + // TODO: Eventually, the webauthn-login-strategy service should be migrated + // to use this method, and we can remove the #[allow(dead_code)] attribute. + #[allow(dead_code)] + fn unlock( + &self, + ctx: &mut KeyStoreContext, + unwrapping_key: &SymmetricCryptoKey, + key_to_unwrap: Ids::Symmetric, + ) -> Result<(), CryptoError> { + let priv_key_bytes: Vec = self + .wrapped_decapsulation_key + .decrypt_with_key(unwrapping_key)?; + let decapsulation_key = + AsymmetricCryptoKey::from_der(&Pkcs8PrivateKeyBytes::from(priv_key_bytes))?; + let encryption_key = self + .encapsulated_encryption_key + .decapsulate_key_unsigned(&decapsulation_key)?; + #[allow(deprecated)] + ctx.set_symmetric_key(key_to_unwrap, encryption_key)?; + Ok(()) + } +} + +fn rotate_key_set( + ctx: &KeyStoreContext, + key_set: RotateableKeySet, + old_encryption_key_id: Ids::Symmetric, + new_encryption_key_id: Ids::Symmetric, +) -> Result { + let pub_key_bytes = ctx.decrypt_data_with_symmetric_key( + old_encryption_key_id, + &key_set.encrypted_encapsulation_key, + )?; + let pub_key = SpkiPublicKeyBytes::from(pub_key_bytes); + let encapsulation_key = AsymmetricPublicCryptoKey::from_der(&pub_key)?; + // TODO: There is no method to store only the public key in the store, so we + // have pull out the encryption key to encapsulate it manually. + #[allow(deprecated)] + let new_encryption_key = ctx.dangerous_get_symmetric_key(new_encryption_key_id)?; + let new_encapsulated_key = + UnsignedSharedKey::encapsulate_key_unsigned(new_encryption_key, &encapsulation_key)?; + let new_encrypted_encapsulation_key = pub_key.encrypt_with_key(new_encryption_key)?; + Ok(RotateableKeySet { + encapsulated_encryption_key: new_encapsulated_key, + encrypted_encapsulation_key: new_encrypted_encapsulation_key, + wrapped_decapsulation_key: key_set.wrapped_decapsulation_key, + }) +} + #[cfg(test)] mod tests { use super::*; @@ -137,4 +257,79 @@ mod tests { .unwrap() ); } + + #[test] + fn test_rotateable_key_set_can_unlock() { + // generate initial keys + let external_key = SymmetricCryptoKey::make_aes256_cbc_hmac_key(); + // set up store + let store: KeyStore = KeyStore::default(); + let mut ctx = store.context_mut(); + let original_encryption_key_id = TestSymmKey::A(0); + ctx.generate_symmetric_key(original_encryption_key_id) + .unwrap(); + + // create key set + let key_set = + RotateableKeySet::new(&ctx, &external_key, original_encryption_key_id).unwrap(); + + // unlock key set + let unwrapped_encryption_key_id = TestSymmKey::A(1); + key_set + .unlock(&mut ctx, &external_key, unwrapped_encryption_key_id) + .unwrap(); + + #[allow(deprecated)] + let original_key = ctx + .dangerous_get_symmetric_key(original_encryption_key_id) + .unwrap(); + #[allow(deprecated)] + let unwrapped_key = ctx + .dangerous_get_symmetric_key(unwrapped_encryption_key_id) + .unwrap(); + assert_eq!(original_key, unwrapped_key); + } + + #[test] + fn test_rotateable_key_set_rotation() { + // generate initial keys + let external_key = SymmetricCryptoKey::make_aes256_cbc_hmac_key(); + // set up store + let store: KeyStore = KeyStore::default(); + let mut ctx = store.context_mut(); + let original_encryption_key_id = TestSymmKey::A(1); + ctx.generate_symmetric_key(original_encryption_key_id) + .unwrap(); + + // create key set + let key_set = + RotateableKeySet::new(&ctx, &external_key, original_encryption_key_id).unwrap(); + + // rotate + let new_encryption_key_id = TestSymmKey::A(2_1); + ctx.generate_symmetric_key(new_encryption_key_id).unwrap(); + let new_key_set = rotate_key_set( + &ctx, + key_set, + original_encryption_key_id, + new_encryption_key_id, + ) + .unwrap(); + + // After rotation, the new key set should be unlocked by the same + // external key and return the new encryption key. + let unwrapped_encryption_key_id = TestSymmKey::A(2_2); + new_key_set + .unlock(&mut ctx, &external_key, unwrapped_encryption_key_id) + .unwrap(); + #[allow(deprecated)] + let new_encryption_key = ctx + .dangerous_get_symmetric_key(new_encryption_key_id) + .unwrap(); + #[allow(deprecated)] + let unwrapped_encryption_key = ctx + .dangerous_get_symmetric_key(unwrapped_encryption_key_id) + .unwrap(); + assert_eq!(new_encryption_key, unwrapped_encryption_key); + } } diff --git a/crates/bitwarden-uniffi/src/crypto.rs b/crates/bitwarden-uniffi/src/crypto.rs index d43881ed0..d253059ad 100644 --- a/crates/bitwarden-uniffi/src/crypto.rs +++ b/crates/bitwarden-uniffi/src/crypto.rs @@ -2,7 +2,7 @@ use bitwarden_core::key_management::crypto::{ DeriveKeyConnectorRequest, DerivePinKeyResponse, EnrollPinResponse, InitOrgCryptoRequest, InitUserCryptoRequest, UpdateKdfResponse, UpdatePasswordResponse, }; -use bitwarden_crypto::{EncString, Kdf, UnsignedSharedKey}; +use bitwarden_crypto::{EncString, Kdf, RotateableKeySet, UnsignedSharedKey}; use bitwarden_encoding::B64; use crate::error::Result; @@ -88,6 +88,12 @@ impl CryptoClient { Ok(self.0.derive_key_connector(request)?) } + /// Creates the a new rotateable key set for the current user key protected + /// by a key derived from the given PRF. + pub fn make_prf_user_key_set(&self, prf: B64) -> Result { + Ok(self.0.make_prf_user_key_set(prf)?) + } + /// Create the data necessary to update the user's kdf settings. The user's encryption key is /// re-encrypted for the password under the new kdf settings. This returns the new encrypted /// user key and the new password hash but does not update sdk state. From 4b8b249c0f9505d159e49959e04ab9e052faa045 Mon Sep 17 00:00:00 2001 From: Isaiah Inuwa Date: Mon, 6 Oct 2025 13:42:06 -0500 Subject: [PATCH 2/7] Mark unused method --- crates/bitwarden-crypto/src/store/key_rotation.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/bitwarden-crypto/src/store/key_rotation.rs b/crates/bitwarden-crypto/src/store/key_rotation.rs index 57cb1db13..90ea12674 100644 --- a/crates/bitwarden-crypto/src/store/key_rotation.rs +++ b/crates/bitwarden-crypto/src/store/key_rotation.rs @@ -139,6 +139,7 @@ impl RotateableKeySet { } } +#[allow(dead_code)] fn rotate_key_set( ctx: &KeyStoreContext, key_set: RotateableKeySet, From eabb6cfc1fa4fd92da4c268481295cd49e024807 Mon Sep 17 00:00:00 2001 From: Isaiah Inuwa Date: Tue, 7 Oct 2025 11:20:42 -0500 Subject: [PATCH 3/7] Clarify implicit PRF truncation --- crates/bitwarden-crypto/src/keys/prf.rs | 36 +++++++++++++++++-------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/crates/bitwarden-crypto/src/keys/prf.rs b/crates/bitwarden-crypto/src/keys/prf.rs index 84d08cbaf..3591460af 100644 --- a/crates/bitwarden-crypto/src/keys/prf.rs +++ b/crates/bitwarden-crypto/src/keys/prf.rs @@ -1,11 +1,14 @@ use crate::{CryptoError, SymmetricCryptoKey, utils::stretch_key}; -/// Takes the output of a PRF and derives a symmetric key +/// Takes the output of a PRF and derives a symmetric key. +/// +/// The PRF output must be at least 32 bytes long. pub fn derive_symmetric_key_from_prf(prf: &[u8]) -> Result { let (secret, _) = prf .split_at_checked(32) .ok_or_else(|| CryptoError::InvalidKeyLen)?; let secret: [u8; 32] = secret.try_into().unwrap(); + // Don't allow uninitialized PRFs if secret.iter().all(|b| *b == b'\0') { return Err(CryptoError::ZeroNumber); } @@ -17,28 +20,39 @@ pub fn derive_symmetric_key_from_prf(prf: &[u8]) -> Result = (0..32).map(|_| 0).collect(); let err = derive_symmetric_key_from_prf(&prf).unwrap_err(); assert!(matches!(err, CryptoError::ZeroNumber)); } + #[test] fn test_short_prf_fails() { - let prf = [0, 1, 2, 3, 4, 5, 6, 7, 8]; + let prf = pseudorandom_bytes(9); let err = derive_symmetric_key_from_prf(&prf).unwrap_err(); assert!(matches!(err, CryptoError::InvalidKeyLen)); } + + #[test] + fn test_long_prf_truncated_to_proper_length() { + let long_prf = pseudorandom_bytes(33); + let prf = pseudorandom_bytes(32); + let key1 = derive_symmetric_key_from_prf(&long_prf).unwrap(); + let key2 = derive_symmetric_key_from_prf(&prf).unwrap(); + assert_eq!(key1, key2); + } + + /// This returns the same bytes deterministically for a given length. + fn pseudorandom_bytes(len: usize) -> Vec { + (0..len).map(|x| (x % 255) as u8).collect() + } } From 748f6d64f705be7bf87752b45b6bb51feb2f0fd2 Mon Sep 17 00:00:00 2001 From: Isaiah Inuwa Date: Tue, 7 Oct 2025 12:56:22 -0500 Subject: [PATCH 4/7] Use upstream/downstream key terminology for rotateable key set --- .../src/store/key_rotation.rs | 144 +++++++++--------- 1 file changed, 71 insertions(+), 73 deletions(-) diff --git a/crates/bitwarden-crypto/src/store/key_rotation.rs b/crates/bitwarden-crypto/src/store/key_rotation.rs index 90ea12674..3162af132 100644 --- a/crates/bitwarden-crypto/src/store/key_rotation.rs +++ b/crates/bitwarden-crypto/src/store/key_rotation.rs @@ -48,16 +48,16 @@ pub fn dangerous_get_v2_rotated_account_keys( }) } -/// A set of keys where a given `EncryptionKey` is protected by an encrypted public/private -/// key-pair. The `EncryptionKey` is used to encrypt/decrypt data, while the public/private key-pair -/// is used to rotate the `EncryptionKey`. +/// A set of keys where a given `DownstreamKey` is protected by an encrypted public/private +/// key-pair. The `DownstreamKey` is used to encrypt/decrypt data, while the public/private key-pair +/// is used to rotate the `DownstreamKey`. /// -/// The `PrivateKey` is protected by an `ExternalKey`, such as a `DeviceKey`, or `PrfKey`, -/// and the `PublicKey` is protected by the `EncryptionKey`. This setup allows: +/// The `PrivateKey` is protected by an `UpstreamKey`, such as a `DeviceKey`, or `PrfKey`, +/// and the `PublicKey` is protected by the `DownstreamKey`. This setup allows: /// -/// - Access to `EncryptionKey` by knowing the `ExternalKey` -/// - Rotation to a `NewEncryptionKey` by knowing the current `EncryptionKey`, without needing access to -/// the `ExternalKey` +/// - Access to `DownstreamKey` by knowing the `UpstreamKey` +/// - Rotation to a `NewDownstreamKey` by knowing the current `DownstreamKey`, without needing access to +/// the `UpstreamKey` #[derive(Serialize, Deserialize, Debug)] #[serde(rename_all = "camelCase", deny_unknown_fields)] #[cfg_attr(feature = "uniffi", derive(uniffi::Record))] @@ -67,21 +67,21 @@ pub fn dangerous_get_v2_rotated_account_keys( tsify(into_wasm_abi, from_wasm_abi) )] pub struct RotateableKeySet { - /// `EncryptionKey` protected by encapsulation key - encapsulated_encryption_key: UnsignedSharedKey, - /// Encapsulation key protected by `EncryptionKey` + /// `DownstreamKey` protected by encapsulation key + encapsulated_downstream_key: UnsignedSharedKey, + /// Encapsulation key protected by `DownstreamKey` encrypted_encapsulation_key: EncString, - /// Decapsulation key protected by `ExternalKey` - wrapped_decapsulation_key: EncString, + /// Decapsulation key protected by `UpstreamKey` + encrypted_decapsulation_key: EncString, } impl RotateableKeySet { - /// Create a set of keys to allow access to the user key via the provided - /// symmetric wrapping key while allowing the user key to be rotated. + /// Create a set of keys to allow access to the downstream key via the provided + /// upstream key while allowing the downstream key to be rotated. pub fn new( ctx: &KeyStoreContext, - wrapping_key: &SymmetricCryptoKey, - key_to_wrap: Ids::Symmetric, + upstream_key: &SymmetricCryptoKey, + downstream_key_id: Ids::Symmetric, ) -> Result { let key_pair = AsymmetricCryptoKey::make(crate::PublicKeyEncryptionAlgorithm::RsaOaepSha1); @@ -89,30 +89,28 @@ impl RotateableKeySet { // rather than the key store context because we don't want the keys to // wind up being stored in the borrowed context. #[allow(deprecated)] - let key_to_wrap_instance = ctx.dangerous_get_symmetric_key(key_to_wrap)?; - // encapsulate encryption key - let encapsulated_encryption_key = UnsignedSharedKey::encapsulate_key_unsigned( - key_to_wrap_instance, - &key_pair.to_public_key(), - )?; + let downstream_key = ctx.dangerous_get_symmetric_key(downstream_key_id)?; + // encapsulate downstream key + let encapsulated_downstream_key = + UnsignedSharedKey::encapsulate_key_unsigned(downstream_key, &key_pair.to_public_key())?; - // wrap decapsulation key - let wrapped_decapsulation_key = key_pair.to_der()?.encrypt_with_key(wrapping_key)?; + // wrap decapsulation key with upstream key + let encrypted_decapsulation_key = key_pair.to_der()?.encrypt_with_key(upstream_key)?; - // wrap encapsulation key with encryption key + // wrap encapsulation key with downstream key // Note: Usually, a public key is - by definition - public, so this should not be necessary. // The specific use-case for this function is to enable rotateable key sets, where // the "public key" is not public, with the intent of preventing the server from being able - // to overwrite the user key unlocked by the rotateable keyset. + // to overwrite the downstream key unlocked by the rotateable keyset. let encrypted_encapsulation_key = key_pair .to_public_key() .to_der()? - .encrypt_with_key(&key_to_wrap_instance)?; + .encrypt_with_key(&downstream_key)?; Ok(RotateableKeySet { - encapsulated_encryption_key, + encapsulated_downstream_key, encrypted_encapsulation_key, - wrapped_decapsulation_key, + encrypted_decapsulation_key, }) } @@ -122,19 +120,19 @@ impl RotateableKeySet { fn unlock( &self, ctx: &mut KeyStoreContext, - unwrapping_key: &SymmetricCryptoKey, - key_to_unwrap: Ids::Symmetric, + upstream_key: &SymmetricCryptoKey, + downstream_key_id: Ids::Symmetric, ) -> Result<(), CryptoError> { let priv_key_bytes: Vec = self - .wrapped_decapsulation_key - .decrypt_with_key(unwrapping_key)?; + .encrypted_decapsulation_key + .decrypt_with_key(upstream_key)?; let decapsulation_key = AsymmetricCryptoKey::from_der(&Pkcs8PrivateKeyBytes::from(priv_key_bytes))?; - let encryption_key = self - .encapsulated_encryption_key + let downstream_key = self + .encapsulated_downstream_key .decapsulate_key_unsigned(&decapsulation_key)?; #[allow(deprecated)] - ctx.set_symmetric_key(key_to_unwrap, encryption_key)?; + ctx.set_symmetric_key(downstream_key_id, downstream_key)?; Ok(()) } } @@ -143,26 +141,26 @@ impl RotateableKeySet { fn rotate_key_set( ctx: &KeyStoreContext, key_set: RotateableKeySet, - old_encryption_key_id: Ids::Symmetric, - new_encryption_key_id: Ids::Symmetric, + old_downstream_key_id: Ids::Symmetric, + new_downstream_key_id: Ids::Symmetric, ) -> Result { let pub_key_bytes = ctx.decrypt_data_with_symmetric_key( - old_encryption_key_id, + old_downstream_key_id, &key_set.encrypted_encapsulation_key, )?; let pub_key = SpkiPublicKeyBytes::from(pub_key_bytes); let encapsulation_key = AsymmetricPublicCryptoKey::from_der(&pub_key)?; // TODO: There is no method to store only the public key in the store, so we - // have pull out the encryption key to encapsulate it manually. + // have pull out the downstream key to encapsulate it manually. #[allow(deprecated)] - let new_encryption_key = ctx.dangerous_get_symmetric_key(new_encryption_key_id)?; + let new_downstream_key = ctx.dangerous_get_symmetric_key(new_downstream_key_id)?; let new_encapsulated_key = - UnsignedSharedKey::encapsulate_key_unsigned(new_encryption_key, &encapsulation_key)?; - let new_encrypted_encapsulation_key = pub_key.encrypt_with_key(new_encryption_key)?; + UnsignedSharedKey::encapsulate_key_unsigned(new_downstream_key, &encapsulation_key)?; + let new_encrypted_encapsulation_key = pub_key.encrypt_with_key(new_downstream_key)?; Ok(RotateableKeySet { - encapsulated_encryption_key: new_encapsulated_key, + encapsulated_downstream_key: new_encapsulated_key, encrypted_encapsulation_key: new_encrypted_encapsulation_key, - wrapped_decapsulation_key: key_set.wrapped_decapsulation_key, + encrypted_decapsulation_key: key_set.encrypted_decapsulation_key, }) } @@ -262,75 +260,75 @@ mod tests { #[test] fn test_rotateable_key_set_can_unlock() { // generate initial keys - let external_key = SymmetricCryptoKey::make_aes256_cbc_hmac_key(); + let upstream_key = SymmetricCryptoKey::make_aes256_cbc_hmac_key(); // set up store let store: KeyStore = KeyStore::default(); let mut ctx = store.context_mut(); - let original_encryption_key_id = TestSymmKey::A(0); - ctx.generate_symmetric_key(original_encryption_key_id) + let original_downstream_key_id = TestSymmKey::A(0); + ctx.generate_symmetric_key(original_downstream_key_id) .unwrap(); // create key set let key_set = - RotateableKeySet::new(&ctx, &external_key, original_encryption_key_id).unwrap(); + RotateableKeySet::new(&ctx, &upstream_key, original_downstream_key_id).unwrap(); // unlock key set - let unwrapped_encryption_key_id = TestSymmKey::A(1); + let unwrapped_downstream_key_id = TestSymmKey::A(1); key_set - .unlock(&mut ctx, &external_key, unwrapped_encryption_key_id) + .unlock(&mut ctx, &upstream_key, unwrapped_downstream_key_id) .unwrap(); #[allow(deprecated)] - let original_key = ctx - .dangerous_get_symmetric_key(original_encryption_key_id) + let original_downstream_key = ctx + .dangerous_get_symmetric_key(original_downstream_key_id) .unwrap(); #[allow(deprecated)] - let unwrapped_key = ctx - .dangerous_get_symmetric_key(unwrapped_encryption_key_id) + let unwrapped_downstream_key = ctx + .dangerous_get_symmetric_key(unwrapped_downstream_key_id) .unwrap(); - assert_eq!(original_key, unwrapped_key); + assert_eq!(original_downstream_key, unwrapped_downstream_key); } #[test] fn test_rotateable_key_set_rotation() { // generate initial keys - let external_key = SymmetricCryptoKey::make_aes256_cbc_hmac_key(); + let upstream_key = SymmetricCryptoKey::make_aes256_cbc_hmac_key(); // set up store let store: KeyStore = KeyStore::default(); let mut ctx = store.context_mut(); - let original_encryption_key_id = TestSymmKey::A(1); - ctx.generate_symmetric_key(original_encryption_key_id) + let original_downstream_key_id = TestSymmKey::A(1); + ctx.generate_symmetric_key(original_downstream_key_id) .unwrap(); // create key set let key_set = - RotateableKeySet::new(&ctx, &external_key, original_encryption_key_id).unwrap(); + RotateableKeySet::new(&ctx, &upstream_key, original_downstream_key_id).unwrap(); // rotate - let new_encryption_key_id = TestSymmKey::A(2_1); - ctx.generate_symmetric_key(new_encryption_key_id).unwrap(); + let new_downstream_key_id = TestSymmKey::A(2_1); + ctx.generate_symmetric_key(new_downstream_key_id).unwrap(); let new_key_set = rotate_key_set( &ctx, key_set, - original_encryption_key_id, - new_encryption_key_id, + original_downstream_key_id, + new_downstream_key_id, ) .unwrap(); // After rotation, the new key set should be unlocked by the same - // external key and return the new encryption key. - let unwrapped_encryption_key_id = TestSymmKey::A(2_2); + // upstream key and return the new downstream key. + let unwrapped_downstream_key_id = TestSymmKey::A(2_2); new_key_set - .unlock(&mut ctx, &external_key, unwrapped_encryption_key_id) + .unlock(&mut ctx, &upstream_key, unwrapped_downstream_key_id) .unwrap(); #[allow(deprecated)] - let new_encryption_key = ctx - .dangerous_get_symmetric_key(new_encryption_key_id) + let new_downstream_key = ctx + .dangerous_get_symmetric_key(new_downstream_key_id) .unwrap(); #[allow(deprecated)] - let unwrapped_encryption_key = ctx - .dangerous_get_symmetric_key(unwrapped_encryption_key_id) + let unwrapped_downstream_key = ctx + .dangerous_get_symmetric_key(unwrapped_downstream_key_id) .unwrap(); - assert_eq!(new_encryption_key, unwrapped_encryption_key); + assert_eq!(new_downstream_key, unwrapped_downstream_key); } } From f986b43951f8a004d2a46911d27e571589a1eacc Mon Sep 17 00:00:00 2001 From: Isaiah Inuwa Date: Tue, 7 Oct 2025 14:00:11 -0500 Subject: [PATCH 5/7] Fix lints --- crates/bitwarden-crypto/Cargo.toml | 2 +- crates/bitwarden-crypto/src/keys/prf.rs | 6 ++---- crates/bitwarden-crypto/src/store/key_rotation.rs | 6 +++--- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/crates/bitwarden-crypto/Cargo.toml b/crates/bitwarden-crypto/Cargo.toml index 0da4085fc..e1fdbedbb 100644 --- a/crates/bitwarden-crypto/Cargo.toml +++ b/crates/bitwarden-crypto/Cargo.toml @@ -20,7 +20,7 @@ no-memory-hardening = [] # Disable memory hardening features uniffi = [ "bitwarden-encoding/uniffi", "dep:bitwarden-uniffi-error", - "dep:uniffi" + "dep:uniffi", ] # Uniffi bindings wasm = ["dep:tsify", "dep:wasm-bindgen"] # WASM support diff --git a/crates/bitwarden-crypto/src/keys/prf.rs b/crates/bitwarden-crypto/src/keys/prf.rs index 3591460af..188b2ffc2 100644 --- a/crates/bitwarden-crypto/src/keys/prf.rs +++ b/crates/bitwarden-crypto/src/keys/prf.rs @@ -4,10 +4,8 @@ use crate::{CryptoError, SymmetricCryptoKey, utils::stretch_key}; /// /// The PRF output must be at least 32 bytes long. pub fn derive_symmetric_key_from_prf(prf: &[u8]) -> Result { - let (secret, _) = prf - .split_at_checked(32) - .ok_or_else(|| CryptoError::InvalidKeyLen)?; - let secret: [u8; 32] = secret.try_into().unwrap(); + let (secret, _) = prf.split_at_checked(32).ok_or(CryptoError::InvalidKeyLen)?; + let secret: [u8; 32] = secret.try_into().expect("length to be 32 bytes"); // Don't allow uninitialized PRFs if secret.iter().all(|b| *b == b'\0') { return Err(CryptoError::ZeroNumber); diff --git a/crates/bitwarden-crypto/src/store/key_rotation.rs b/crates/bitwarden-crypto/src/store/key_rotation.rs index 3162af132..a3d0d27b2 100644 --- a/crates/bitwarden-crypto/src/store/key_rotation.rs +++ b/crates/bitwarden-crypto/src/store/key_rotation.rs @@ -56,8 +56,8 @@ pub fn dangerous_get_v2_rotated_account_keys( /// and the `PublicKey` is protected by the `DownstreamKey`. This setup allows: /// /// - Access to `DownstreamKey` by knowing the `UpstreamKey` -/// - Rotation to a `NewDownstreamKey` by knowing the current `DownstreamKey`, without needing access to -/// the `UpstreamKey` +/// - Rotation to a `NewDownstreamKey` by knowing the current `DownstreamKey`, without needing +/// access to the `UpstreamKey` #[derive(Serialize, Deserialize, Debug)] #[serde(rename_all = "camelCase", deny_unknown_fields)] #[cfg_attr(feature = "uniffi", derive(uniffi::Record))] @@ -105,7 +105,7 @@ impl RotateableKeySet { let encrypted_encapsulation_key = key_pair .to_public_key() .to_der()? - .encrypt_with_key(&downstream_key)?; + .encrypt_with_key(downstream_key)?; Ok(RotateableKeySet { encapsulated_downstream_key, From 0dce15ef6cb34ee842c6d1e895f02b75299552d6 Mon Sep 17 00:00:00 2001 From: Isaiah Inuwa Date: Tue, 7 Oct 2025 14:00:11 -0500 Subject: [PATCH 6/7] Feature-gate wasm-only imports --- crates/bitwarden-vault/src/cipher/cipher_client.rs | 12 +++++++++--- crates/bitwarden-vault/src/collection_client.rs | 1 + 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/crates/bitwarden-vault/src/cipher/cipher_client.rs b/crates/bitwarden-vault/src/cipher/cipher_client.rs index 060e1bcfd..94fe857ee 100644 --- a/crates/bitwarden-vault/src/cipher/cipher_client.rs +++ b/crates/bitwarden-vault/src/cipher/cipher_client.rs @@ -1,14 +1,20 @@ -use bitwarden_core::{Client, OrganizationId, key_management::SymmetricKeyId}; -use bitwarden_crypto::{CompositeEncryptable, IdentifyKey, SymmetricCryptoKey}; +#[cfg(feature = "wasm")] +use bitwarden_core::key_management::SymmetricKeyId; +use bitwarden_core::{Client, OrganizationId}; +use bitwarden_crypto::IdentifyKey; +#[cfg(feature = "wasm")] +use bitwarden_crypto::{CompositeEncryptable, SymmetricCryptoKey}; #[cfg(feature = "wasm")] use bitwarden_encoding::B64; #[cfg(feature = "wasm")] use wasm_bindgen::prelude::*; use super::EncryptionContext; +#[cfg(feature = "wasm")] +use crate::Fido2CredentialFullView; use crate::{ Cipher, CipherError, CipherListView, CipherView, DecryptError, EncryptError, - Fido2CredentialFullView, cipher::cipher::DecryptCipherListResult, + cipher::cipher::DecryptCipherListResult, }; #[allow(missing_docs)] diff --git a/crates/bitwarden-vault/src/collection_client.rs b/crates/bitwarden-vault/src/collection_client.rs index f74cf7be2..230d82099 100644 --- a/crates/bitwarden-vault/src/collection_client.rs +++ b/crates/bitwarden-vault/src/collection_client.rs @@ -5,6 +5,7 @@ use bitwarden_collections::{ tree::{NodeItem, Tree}, }; use bitwarden_core::Client; +#[cfg(feature = "wasm")] use serde::{Deserialize, Serialize}; #[cfg(feature = "wasm")] use tsify::Tsify; From b9bb1f06cd5720b7c5e2f2ce7956216a0702a838 Mon Sep 17 00:00:00 2001 From: Isaiah Inuwa Date: Tue, 7 Oct 2025 15:19:29 -0500 Subject: [PATCH 7/7] Lint with correct version of cargo-sort --- crates/bitwarden-crypto/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bitwarden-crypto/Cargo.toml b/crates/bitwarden-crypto/Cargo.toml index e1fdbedbb..0da4085fc 100644 --- a/crates/bitwarden-crypto/Cargo.toml +++ b/crates/bitwarden-crypto/Cargo.toml @@ -20,7 +20,7 @@ no-memory-hardening = [] # Disable memory hardening features uniffi = [ "bitwarden-encoding/uniffi", "dep:bitwarden-uniffi-error", - "dep:uniffi", + "dep:uniffi" ] # Uniffi bindings wasm = ["dep:tsify", "dep:wasm-bindgen"] # WASM support