From e2bb9c1c608db1eb831e0305436177f7d11b00bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garci=CC=81a?= Date: Mon, 12 May 2025 12:36:12 +0200 Subject: [PATCH 1/8] Make context local Ids autogenerated --- .../bitwarden-core/src/key_management/mod.rs | 4 +- crates/bitwarden-crypto/src/store/context.rs | 26 ++++--- crates/bitwarden-crypto/src/traits/key_id.rs | 17 ++++- crates/bitwarden-crypto/src/traits/mod.rs | 4 +- crates/bitwarden-send/src/send.rs | 4 +- .../bitwarden-vault/src/cipher/attachment.rs | 6 +- crates/bitwarden-vault/src/cipher/cipher.rs | 22 ++---- .../src/pure_crypto.rs | 76 ++++++------------- 8 files changed, 67 insertions(+), 92 deletions(-) diff --git a/crates/bitwarden-core/src/key_management/mod.rs b/crates/bitwarden-core/src/key_management/mod.rs index dd13ab21c..b2190bd8d 100644 --- a/crates/bitwarden-core/src/key_management/mod.rs +++ b/crates/bitwarden-core/src/key_management/mod.rs @@ -16,14 +16,14 @@ key_ids! { User, Organization(uuid::Uuid), #[local] - Local(&'static str), + Local(uuid::Uuid), } #[asymmetric] pub enum AsymmetricKeyId { UserPrivateKey, #[local] - Local(&'static str), + Local(uuid::Uuid), } pub KeyIds => SymmetricKeyId, AsymmetricKeyId; diff --git a/crates/bitwarden-crypto/src/store/context.rs b/crates/bitwarden-crypto/src/store/context.rs index 261067af8..19be9005c 100644 --- a/crates/bitwarden-crypto/src/store/context.rs +++ b/crates/bitwarden-crypto/src/store/context.rs @@ -133,15 +133,13 @@ impl KeyStoreContext<'_, Ids> { /// /// * `encryption_key` - The key id used to decrypt the `encrypted_key`. It must already exist /// in the context - /// * `new_key_id` - The key id where the decrypted key will be stored. If it already exists, it - /// will be overwritten /// * `encrypted_key` - The key to decrypt pub fn unwrap_symmetric_key( &mut self, encryption_key: Ids::Symmetric, - new_key_id: Ids::Symmetric, encrypted_key: &EncString, ) -> Result { + let new_key_id = Ids::Symmetric::new_local(); let mut new_key_material = self.decrypt_data_with_symmetric_key(encryption_key, encrypted_key)?; @@ -245,7 +243,8 @@ impl KeyStoreContext<'_, Ids> { } /// Generate a new random symmetric key and store it in the context - pub fn generate_symmetric_key(&mut self, key_id: Ids::Symmetric) -> Result { + pub fn generate_symmetric_key(&mut self) -> Result { + let key_id = Ids::Symmetric::new_local(); let key = SymmetricCryptoKey::make_aes256_cbc_hmac_key(); #[allow(deprecated)] self.set_symmetric_key(key_id, key)?; @@ -258,11 +257,11 @@ impl KeyStoreContext<'_, Ids> { /// Bitwarden `clients` repository. pub fn derive_shareable_key( &mut self, - key_id: Ids::Symmetric, secret: Zeroizing<[u8; 16]>, name: &str, info: Option<&str>, ) -> Result { + let key_id = Ids::Symmetric::new_local(); #[allow(deprecated)] self.set_symmetric_key( key_id, @@ -322,6 +321,12 @@ impl KeyStoreContext<'_, Ids> { Ok(()) } + pub fn add_local_symmetric_key(&mut self, key: SymmetricCryptoKey) -> Result { + let key_id = Ids::Symmetric::new_local(); + self.local_symmetric_keys.upsert(key_id, key); + Ok(key_id) + } + #[deprecated(note = "This function should ideally never be used outside this crate")] pub fn set_asymmetric_key( &mut self, @@ -379,6 +384,8 @@ impl KeyStoreContext<'_, Ids> { #[cfg(test)] #[allow(deprecated)] mod tests { + use uuid::Uuid; + use crate::{ store::{tests::DataView, KeyStore}, traits::tests::{TestIds, TestSymmKey}, @@ -412,7 +419,7 @@ mod tests { let mut ctx = store.context(); // Generate and insert a key - let key_1_id = TestSymmKey::C(1); + let key_1_id = TestSymmKey::C(Uuid::new_v4()); let key_1 = SymmetricCryptoKey::make_aes256_cbc_hmac_key(); ctx.set_symmetric_key(key_1_id, key_1.clone()).unwrap(); @@ -420,7 +427,7 @@ mod tests { assert!(ctx.has_symmetric_key(key_1_id)); // Generate and insert a new key - let key_2_id = TestSymmKey::C(2); + let key_2_id = TestSymmKey::C(Uuid::new_v4()); let key_2 = SymmetricCryptoKey::make_aes256_cbc_hmac_key(); ctx.set_symmetric_key(key_2_id, key_2.clone()).unwrap(); @@ -431,10 +438,7 @@ mod tests { let key_2_enc = ctx.wrap_symmetric_key(key_1_id, key_2_id).unwrap(); // Decrypt the new key with the old key in a different identifier - let new_key_id = TestSymmKey::C(3); - - ctx.unwrap_symmetric_key(key_1_id, new_key_id, &key_2_enc) - .unwrap(); + let new_key_id = ctx.unwrap_symmetric_key(key_1_id, &key_2_enc).unwrap(); // Now `key_2_id` and `new_key_id` contain the same key, so we should be able to encrypt // with one and decrypt with the other diff --git a/crates/bitwarden-crypto/src/traits/key_id.rs b/crates/bitwarden-crypto/src/traits/key_id.rs index ba997a5b0..e81396805 100644 --- a/crates/bitwarden-crypto/src/traits/key_id.rs +++ b/crates/bitwarden-crypto/src/traits/key_id.rs @@ -23,6 +23,8 @@ pub trait KeyId: /// Returns whether the key is local to the current context or shared globally by the /// key store. See [crate::store::KeyStoreContext] for more information. fn is_local(&self) -> bool; + + fn new_local() -> Self; } /// Represents a set of all the key identifiers that need to be defined to use a key store. @@ -81,6 +83,12 @@ macro_rules! key_ids { key_ids!(@variant_value $( $variant_tag )? ), )* } } + + fn new_local() -> Self { + $( + { key_ids!(@new_local $variant $( $variant_tag )? ) } + )* + } } )+ @@ -99,10 +107,15 @@ macro_rules! key_ids { ( @variant_value local ) => { true }; ( @variant_value ) => { false }; + + ( @new_local $variant:ident local ) => { Self::$variant(uuid::Uuid::new_v4()) }; + ( @new_local $variant:ident ) => {{}}; } #[cfg(test)] pub(crate) mod tests { + use uuid::Uuid; + use crate::{ traits::tests::{TestAsymmKey, TestSymmKey}, KeyId, @@ -112,10 +125,10 @@ pub(crate) mod tests { fn test_local() { assert!(!TestSymmKey::A(0).is_local()); assert!(!TestSymmKey::B((4, 10)).is_local()); - assert!(TestSymmKey::C(8).is_local()); + assert!(TestSymmKey::C(Uuid::new_v4()).is_local()); assert!(!TestAsymmKey::A(0).is_local()); assert!(!TestAsymmKey::B.is_local()); - assert!(TestAsymmKey::C("test").is_local()); + assert!(TestAsymmKey::C(Uuid::new_v4()).is_local()); } } diff --git a/crates/bitwarden-crypto/src/traits/mod.rs b/crates/bitwarden-crypto/src/traits/mod.rs index 28b811e36..5def73101 100644 --- a/crates/bitwarden-crypto/src/traits/mod.rs +++ b/crates/bitwarden-crypto/src/traits/mod.rs @@ -25,7 +25,7 @@ pub(crate) mod tests { B((u8, u8)), #[local] - C(u8), + C(uuid::Uuid), } #[asymmetric] @@ -33,7 +33,7 @@ pub(crate) mod tests { A(u8), B, #[local] - C(&'static str), + C(uuid::Uuid), } pub TestIds => TestSymmKey, TestAsymmKey; diff --git a/crates/bitwarden-send/src/send.rs b/crates/bitwarden-send/src/send.rs index a9f54ed4c..67b6c6d2a 100644 --- a/crates/bitwarden-send/src/send.rs +++ b/crates/bitwarden-send/src/send.rs @@ -143,8 +143,6 @@ pub struct SendListView { pub expiration_date: Option>, } -const SEND_KEY: SymmetricKeyId = SymmetricKeyId::Local("send_key"); - impl Send { pub fn get_key( ctx: &mut KeyStoreContext, @@ -160,7 +158,7 @@ impl Send { key: &[u8], ) -> Result { let key = Zeroizing::new(key.try_into().map_err(|_| CryptoError::InvalidKeyLen)?); - ctx.derive_shareable_key(SEND_KEY, key, "send", Some("send")) + ctx.derive_shareable_key(key, "send", Some("send")) } } diff --git a/crates/bitwarden-vault/src/cipher/attachment.rs b/crates/bitwarden-vault/src/cipher/attachment.rs index 5d997f631..e75239899 100644 --- a/crates/bitwarden-vault/src/cipher/attachment.rs +++ b/crates/bitwarden-vault/src/cipher/attachment.rs @@ -60,7 +60,6 @@ pub struct AttachmentFileView<'a> { pub attachment: AttachmentView, pub contents: &'a [u8], } -const ATTACHMENT_KEY: SymmetricKeyId = SymmetricKeyId::Local("attachment_key"); impl IdentifyKey for AttachmentFileView<'_> { fn key_identifier(&self) -> SymmetricKeyId { @@ -85,7 +84,7 @@ impl Encryptable for Attachment // Because this is a new attachment, we have to generate a key for it, encrypt the contents // with it, and then encrypt the key with the cipher key - let attachment_key = ctx.generate_symmetric_key(ATTACHMENT_KEY)?; + let attachment_key = ctx.generate_symmetric_key()?; let encrypted_contents = self.contents.encrypt(ctx, attachment_key)?; attachment.key = Some(ctx.wrap_symmetric_key(ciphers_key, attachment_key)?); @@ -122,8 +121,7 @@ impl Decryptable> for AttachmentFile { // Version 2 or 3, `AttachmentKey` or `CipherKey(AttachmentKey)` if let Some(attachment_key) = &self.attachment.key { - let content_key = - ctx.unwrap_symmetric_key(ciphers_key, ATTACHMENT_KEY, attachment_key)?; + let content_key = ctx.unwrap_symmetric_key(ciphers_key, attachment_key)?; self.contents.decrypt(ctx, content_key) } else { // Legacy attachment version 1, use user/org key diff --git a/crates/bitwarden-vault/src/cipher/cipher.rs b/crates/bitwarden-vault/src/cipher/cipher.rs index 36a1b011b..e77af2666 100644 --- a/crates/bitwarden-vault/src/cipher/cipher.rs +++ b/crates/bitwarden-vault/src/cipher/cipher.rs @@ -322,9 +322,8 @@ impl Cipher { key: SymmetricKeyId, ciphers_key: &Option, ) -> Result { - const CIPHER_KEY: SymmetricKeyId = SymmetricKeyId::Local("cipher_key"); match ciphers_key { - Some(ciphers_key) => ctx.unwrap_symmetric_key(key, CIPHER_KEY, ciphers_key), + Some(ciphers_key) => ctx.unwrap_symmetric_key(key, ciphers_key), None => Ok(key), } } @@ -455,9 +454,7 @@ impl CipherView { ) -> Result<(), CryptoError> { let old_ciphers_key = Cipher::decrypt_cipher_key(ctx, key, &self.key)?; - const NEW_KEY: SymmetricKeyId = SymmetricKeyId::Local("new_cipher_key"); - - let new_key = ctx.generate_symmetric_key(NEW_KEY)?; + let new_key = ctx.generate_symmetric_key()?; self.reencrypt_attachment_keys(ctx, old_ciphers_key, new_key)?; self.reencrypt_fido2_credentials(ctx, old_ciphers_key, new_key)?; @@ -929,9 +926,8 @@ mod tests { let mut original_cipher = generate_cipher(); { - const CIPHER_KEY: SymmetricKeyId = SymmetricKeyId::Local("test_cipher_key"); let mut ctx = key_store.context(); - let cipher_key = ctx.generate_symmetric_key(CIPHER_KEY).unwrap(); + let cipher_key = ctx.generate_symmetric_key().unwrap(); original_cipher.key = Some( ctx.wrap_symmetric_key(SymmetricKeyId::User, cipher_key) @@ -1053,9 +1049,7 @@ mod tests { // Attachment has a key that is encrypted with the user key, as the cipher has no key itself let (attachment_key_enc, attachment_key_val) = { let mut ctx = key_store.context(); - let attachment_key = ctx - .generate_symmetric_key(SymmetricKeyId::Local("test_attachment_key")) - .unwrap(); + let attachment_key = ctx.generate_symmetric_key().unwrap(); let attachment_key_enc = ctx .wrap_symmetric_key(SymmetricKeyId::User, attachment_key) .unwrap(); @@ -1120,17 +1114,13 @@ mod tests { let mut ctx = key_store.context(); - let cipher_key = ctx - .generate_symmetric_key(SymmetricKeyId::Local("test_cipher_key")) - .unwrap(); + let cipher_key = ctx.generate_symmetric_key().unwrap(); let cipher_key_enc = ctx .wrap_symmetric_key(SymmetricKeyId::User, cipher_key) .unwrap(); // Attachment has a key that is encrypted with the cipher key - let attachment_key = ctx - .generate_symmetric_key(SymmetricKeyId::Local("test_attachment_key")) - .unwrap(); + let attachment_key = ctx.generate_symmetric_key().unwrap(); let attachment_key_enc = ctx.wrap_symmetric_key(cipher_key, attachment_key).unwrap(); let mut cipher = generate_cipher(); diff --git a/crates/bitwarden-wasm-internal/src/pure_crypto.rs b/crates/bitwarden-wasm-internal/src/pure_crypto.rs index 85a34b49b..7df7abe7c 100644 --- a/crates/bitwarden-wasm-internal/src/pure_crypto.rs +++ b/crates/bitwarden-wasm-internal/src/pure_crypto.rs @@ -1,6 +1,6 @@ use std::str::FromStr; -use bitwarden_core::key_management::{KeyIds, SymmetricKeyId}; +use bitwarden_core::key_management::KeyIds; use bitwarden_crypto::{ AsymmetricCryptoKey, AsymmetricPublicCryptoKey, CryptoError, Decryptable, EncString, Encryptable, Kdf, KeyDecryptable, KeyEncryptable, KeyStore, MasterKey, SymmetricCryptoKey, @@ -117,22 +117,13 @@ impl PureCrypto { ) -> Result { let tmp_store: KeyStore = KeyStore::default(); let mut context = tmp_store.context(); - #[allow(deprecated)] - context.set_symmetric_key( - SymmetricKeyId::Local("wrapping_key"), - SymmetricCryptoKey::try_from(wrapping_key)?, - )?; - #[allow(deprecated)] - context.set_symmetric_key( - SymmetricKeyId::Local("key_to_wrap"), - SymmetricCryptoKey::try_from(key_to_be_wrapped)?, - )?; + let wrapping_key = + context.add_local_symmetric_key(SymmetricCryptoKey::try_from(wrapping_key)?)?; + let key_to_wrap = + context.add_local_symmetric_key(SymmetricCryptoKey::try_from(key_to_be_wrapped)?)?; // Note: The order of arguments is different here, and should probably be refactored Ok(context - .wrap_symmetric_key( - SymmetricKeyId::Local("wrapping_key"), - SymmetricKeyId::Local("key_to_wrap"), - )? + .wrap_symmetric_key(wrapping_key, key_to_wrap)? .to_string()) } @@ -144,19 +135,14 @@ impl PureCrypto { ) -> Result, CryptoError> { let tmp_store: KeyStore = KeyStore::default(); let mut context = tmp_store.context(); - #[allow(deprecated)] - context.set_symmetric_key( - SymmetricKeyId::Local("wrapping_key"), - SymmetricCryptoKey::try_from(wrapping_key)?, - )?; + let wrapping_key = + context.add_local_symmetric_key(SymmetricCryptoKey::try_from(wrapping_key)?)?; + // Note: The order of arguments is different here, and should probably be refactored - context.unwrap_symmetric_key( - SymmetricKeyId::Local("wrapping_key"), - SymmetricKeyId::Local("wrapped_key"), - &EncString::from_str(wrapped_key.as_str())?, - )?; + let unwrapped = context + .unwrap_symmetric_key(wrapping_key, &EncString::from_str(wrapped_key.as_str())?)?; #[allow(deprecated)] - let key = context.dangerous_get_symmetric_key(SymmetricKeyId::Local("wrapped_key"))?; + let key = context.dangerous_get_symmetric_key(unwrapped)?; Ok(key.to_encoded()) } @@ -171,14 +157,11 @@ impl PureCrypto { ) -> Result { let tmp_store: KeyStore = KeyStore::default(); let mut context = tmp_store.context(); - #[allow(deprecated)] - context.set_symmetric_key( - SymmetricKeyId::Local("wrapping_key"), - SymmetricCryptoKey::try_from(wrapping_key)?, - )?; + let wrapping_key = + context.add_local_symmetric_key(SymmetricCryptoKey::try_from(wrapping_key)?)?; // Note: The order of arguments is different here, and should probably be refactored Ok(encapsulation_key - .encrypt(&mut context, SymmetricKeyId::Local("wrapping_key"))? + .encrypt(&mut context, wrapping_key)? .to_string()) } @@ -190,14 +173,10 @@ impl PureCrypto { ) -> Result, CryptoError> { let tmp_store: KeyStore = KeyStore::default(); let mut context = tmp_store.context(); - #[allow(deprecated)] - context.set_symmetric_key( - SymmetricKeyId::Local("wrapping_key"), - SymmetricCryptoKey::try_from(wrapping_key)?, - )?; + let wrapping_key = + context.add_local_symmetric_key(SymmetricCryptoKey::try_from(wrapping_key)?)?; // Note: The order of arguments is different here, and should probably be refactored - EncString::from_str(wrapped_key.as_str())? - .decrypt(&mut context, SymmetricKeyId::Local("wrapping_key")) + EncString::from_str(wrapped_key.as_str())?.decrypt(&mut context, wrapping_key) } /// Wraps (encrypts) a PKCS8 DER encoded decapsulation (private) key using a symmetric wrapping @@ -208,14 +187,11 @@ impl PureCrypto { ) -> Result { let tmp_store: KeyStore = KeyStore::default(); let mut context = tmp_store.context(); - #[allow(deprecated)] - context.set_symmetric_key( - SymmetricKeyId::Local("wrapping_key"), - SymmetricCryptoKey::try_from(wrapping_key)?, - )?; + let wrapping_key = + context.add_local_symmetric_key(SymmetricCryptoKey::try_from(wrapping_key)?)?; // Note: The order of arguments is different here, and should probably be refactored Ok(decapsulation_key - .encrypt(&mut context, SymmetricKeyId::Local("wrapping_key"))? + .encrypt(&mut context, wrapping_key)? .to_string()) } @@ -227,14 +203,10 @@ impl PureCrypto { ) -> Result, CryptoError> { let tmp_store: KeyStore = KeyStore::default(); let mut context = tmp_store.context(); - #[allow(deprecated)] - context.set_symmetric_key( - SymmetricKeyId::Local("wrapping_key"), - SymmetricCryptoKey::try_from(wrapping_key)?, - )?; + let wrapping_key = + context.add_local_symmetric_key(SymmetricCryptoKey::try_from(wrapping_key)?)?; // Note: The order of arguments is different here, and should probably be refactored - EncString::from_str(wrapped_key.as_str())? - .decrypt(&mut context, SymmetricKeyId::Local("wrapping_key")) + EncString::from_str(wrapped_key.as_str())?.decrypt(&mut context, wrapping_key) } /// Encapsulates (encrypts) a symmetric key using an asymmetric encapsulation key (public key) From 7218aba67686bf3163eb7eb0a0f2b8d6dbaa110b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garci=CC=81a?= Date: Mon, 12 May 2025 12:52:00 +0200 Subject: [PATCH 2/8] Update docs --- crates/bitwarden-crypto/src/store/context.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/bitwarden-crypto/src/store/context.rs b/crates/bitwarden-crypto/src/store/context.rs index 19be9005c..2a685642e 100644 --- a/crates/bitwarden-crypto/src/store/context.rs +++ b/crates/bitwarden-crypto/src/store/context.rs @@ -51,11 +51,10 @@ use crate::{ /// # } /// # } /// -/// const LOCAL_KEY: SymmKeyId = SymmKeyId::Local("local_key_id"); /// /// impl Encryptable for Data { /// fn encrypt(&self, ctx: &mut KeyStoreContext, key: SymmKeyId) -> Result { -/// let local_key_id = ctx.unwrap_symmetric_key(key, LOCAL_KEY, &self.key)?; +/// let local_key_id = ctx.unwrap_symmetric_key(key, &self.key)?; /// self.name.encrypt(ctx, local_key_id) /// } /// } From ab25b955571d75d685e5c1204affd2b093eb2f68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garci=CC=81a?= Date: Thu, 12 Jun 2025 18:39:05 +0200 Subject: [PATCH 3/8] Add comments --- crates/bitwarden-crypto/src/store/context.rs | 1 + crates/bitwarden-crypto/src/traits/key_id.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/crates/bitwarden-crypto/src/store/context.rs b/crates/bitwarden-crypto/src/store/context.rs index 92d853266..4982cf7bd 100644 --- a/crates/bitwarden-crypto/src/store/context.rs +++ b/crates/bitwarden-crypto/src/store/context.rs @@ -323,6 +323,7 @@ impl KeyStoreContext<'_, Ids> { Ok(()) } + /// Add a new symmetric key to the local context, returning a new unique identifier for it. pub fn add_local_symmetric_key(&mut self, key: SymmetricCryptoKey) -> Result { let key_id = Ids::Symmetric::new_local(); self.local_symmetric_keys.upsert(key_id, key); diff --git a/crates/bitwarden-crypto/src/traits/key_id.rs b/crates/bitwarden-crypto/src/traits/key_id.rs index 0396fb37d..b6a091736 100644 --- a/crates/bitwarden-crypto/src/traits/key_id.rs +++ b/crates/bitwarden-crypto/src/traits/key_id.rs @@ -25,6 +25,7 @@ pub trait KeyId: /// key store. See [crate::store::KeyStoreContext] for more information. fn is_local(&self) -> bool; + /// Creates a new unique local key identifier. fn new_local() -> Self; } From d9707e03b5c561874b5702623fd8e106906f0b06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garci=CC=81a?= Date: Thu, 12 Jun 2025 18:50:49 +0200 Subject: [PATCH 4/8] Fix tests --- crates/bitwarden-crypto/src/store/context.rs | 5 ++++- crates/bitwarden-crypto/src/store/mod.rs | 4 +++- crates/bitwarden-crypto/src/traits/key_id.rs | 4 +++- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/crates/bitwarden-crypto/src/store/context.rs b/crates/bitwarden-crypto/src/store/context.rs index 4982cf7bd..6e38f89c6 100644 --- a/crates/bitwarden-crypto/src/store/context.rs +++ b/crates/bitwarden-crypto/src/store/context.rs @@ -33,11 +33,14 @@ use crate::{ /// # #[symmetric] /// # pub enum SymmKeyId { /// # User, -/// # Local(&'static str), +/// # #[local] +/// # Local(uuid::Uuid), /// # } /// # #[asymmetric] /// # pub enum AsymmKeyId { /// # UserPrivate, +/// # #[local] +/// # Local(uuid::Uuid), /// # } /// # pub Ids => SymmKeyId, AsymmKeyId; /// # } diff --git a/crates/bitwarden-crypto/src/store/mod.rs b/crates/bitwarden-crypto/src/store/mod.rs index f447f58b2..06169f587 100644 --- a/crates/bitwarden-crypto/src/store/mod.rs +++ b/crates/bitwarden-crypto/src/store/mod.rs @@ -52,11 +52,13 @@ pub use context::KeyStoreContext; /// pub enum SymmKeyId { /// User, /// #[local] -/// Local(&'static str) +/// Local(uuid::Uuid), /// } /// #[asymmetric] /// pub enum AsymmKeyId { /// UserPrivate, +/// #[local] +/// Local(uuid::Uuid), /// } /// pub Ids => SymmKeyId, AsymmKeyId; /// } diff --git a/crates/bitwarden-crypto/src/traits/key_id.rs b/crates/bitwarden-crypto/src/traits/key_id.rs index b6a091736..aca111d07 100644 --- a/crates/bitwarden-crypto/src/traits/key_id.rs +++ b/crates/bitwarden-crypto/src/traits/key_id.rs @@ -48,12 +48,14 @@ pub trait KeyIds { /// User, /// Org(uuid::Uuid), /// #[local] -/// Local(&'static str), +/// Local(uuid::Uuid), /// } /// /// #[asymmetric] /// pub enum AsymmKeyId { /// PrivateKey, +/// #[local] +/// Local(uuid::Uuid), /// } /// pub Ids => SymmKeyId, AsymmKeyId; /// } From 237cb200025221a827c7a7d6d3560612a32b3bf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garci=CC=81a?= Date: Thu, 26 Jun 2025 19:04:08 +0200 Subject: [PATCH 5/8] Add signing keys --- .../src/key_management/crypto.rs | 7 +++---- crates/bitwarden-crypto/src/store/context.rs | 20 +++++++++---------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/crates/bitwarden-core/src/key_management/crypto.rs b/crates/bitwarden-core/src/key_management/crypto.rs index 3a1cd66a4..05a015720 100644 --- a/crates/bitwarden-core/src/key_management/crypto.rs +++ b/crates/bitwarden-core/src/key_management/crypto.rs @@ -20,7 +20,7 @@ use {tsify_next::Tsify, wasm_bindgen::prelude::*}; use crate::{ client::{encryption_settings::EncryptionSettingsError, LoginMethod, UserLoginMethod}, - key_management::{AsymmetricKeyId, SigningKeyId, SymmetricKeyId}, + key_management::{AsymmetricKeyId, SymmetricKeyId}, Client, NotAuthenticatedError, VaultLockedError, WrongPasswordError, }; @@ -595,9 +595,8 @@ pub fn make_user_signing_keys_for_enrollment( // Make new keypair and sign the public key with it let signature_keypair = SigningKey::make(SignatureAlgorithm::Ed25519); - let temporary_signature_keypair_id = SigningKeyId::Local("temporary_key_for_rotation"); - #[allow(deprecated)] - ctx.set_signing_key(temporary_signature_keypair_id, signature_keypair.clone())?; + let temporary_signature_keypair_id = ctx.add_local_signing_key(signature_keypair.clone())?; + let signed_public_key = ctx.make_signed_public_key( AsymmetricKeyId::UserPrivateKey, temporary_signature_keypair_id, diff --git a/crates/bitwarden-crypto/src/store/context.rs b/crates/bitwarden-crypto/src/store/context.rs index 3a42e866b..97598dbfc 100644 --- a/crates/bitwarden-crypto/src/store/context.rs +++ b/crates/bitwarden-crypto/src/store/context.rs @@ -259,20 +259,13 @@ impl KeyStoreContext<'_, Ids> { /// Generate a new random symmetric key and store it in the context pub fn generate_symmetric_key(&mut self) -> Result { - let key_id = Ids::Symmetric::new_local(); - let key = SymmetricCryptoKey::make_aes256_cbc_hmac_key(); - #[allow(deprecated)] - self.set_symmetric_key(key_id, key)?; - Ok(key_id) + self.add_local_symmetric_key(SymmetricCryptoKey::make_aes256_cbc_hmac_key()) } /// Generate a new signature key using the current default algorithm, and store it in the /// context - pub fn make_signing_key(&mut self, key_id: Ids::Signing) -> Result { - let key = SigningKey::make(SignatureAlgorithm::default_algorithm()); - #[allow(deprecated)] - self.set_signing_key(key_id, key)?; - Ok(key_id) + pub fn make_signing_key(&mut self) -> Result { + self.add_local_signing_key(SigningKey::make(SignatureAlgorithm::default_algorithm())) } /// Derive a shareable key using hkdf from secret and name and store it in the context. @@ -397,6 +390,13 @@ impl KeyStoreContext<'_, Ids> { Ok(()) } + /// Add a new signing key to the local context, returning a new unique identifier for it. + pub fn add_local_signing_key(&mut self, key: SigningKey) -> Result { + let key_id = Ids::Signing::new_local(); + self.local_signing_keys.upsert(key_id, key); + Ok(key_id) + } + /// Sets a signing key in the context #[deprecated(note = "This function should ideally never be used outside this crate")] pub fn set_signing_key(&mut self, key_id: Ids::Signing, key: SigningKey) -> Result<()> { From 7b2b49fdca193ce77bc020fedea8ea707557cdb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garci=CC=81a?= Date: Mon, 30 Jun 2025 15:54:36 +0200 Subject: [PATCH 6/8] Introduce opaque local ids --- .../bitwarden-core/src/key_management/mod.rs | 6 +-- crates/bitwarden-crypto/src/lib.rs | 2 +- crates/bitwarden-crypto/src/store/context.rs | 22 +++++------ crates/bitwarden-crypto/src/store/mod.rs | 4 +- crates/bitwarden-crypto/src/traits/key_id.rs | 39 +++++++++++++------ crates/bitwarden-crypto/src/traits/mod.rs | 8 ++-- 6 files changed, 48 insertions(+), 33 deletions(-) diff --git a/crates/bitwarden-core/src/key_management/mod.rs b/crates/bitwarden-core/src/key_management/mod.rs index 5d90aadcd..57eed5b9e 100644 --- a/crates/bitwarden-core/src/key_management/mod.rs +++ b/crates/bitwarden-core/src/key_management/mod.rs @@ -25,21 +25,21 @@ key_ids! { User, Organization(uuid::Uuid), #[local] - Local(uuid::Uuid), + Local(LocalId), } #[asymmetric] pub enum AsymmetricKeyId { UserPrivateKey, #[local] - Local(uuid::Uuid), + Local(LocalId), } #[signing] pub enum SigningKeyId { UserSigningKey, #[local] - Local(uuid::Uuid), + Local(LocalId), } pub KeyIds => SymmetricKeyId, AsymmetricKeyId, SigningKeyId; diff --git a/crates/bitwarden-crypto/src/lib.rs b/crates/bitwarden-crypto/src/lib.rs index 60428d8a4..b6009efd0 100644 --- a/crates/bitwarden-crypto/src/lib.rs +++ b/crates/bitwarden-crypto/src/lib.rs @@ -39,7 +39,7 @@ pub use signing::*; mod traits; mod xchacha20; pub use traits::{ - CompositeEncryptable, Decryptable, IdentifyKey, KeyId, KeyIds, PrimitiveEncryptable, + CompositeEncryptable, Decryptable, IdentifyKey, KeyId, KeyIds, LocalId, PrimitiveEncryptable, }; pub use zeroizing_alloc::ZeroAlloc as ZeroizingAllocator; diff --git a/crates/bitwarden-crypto/src/store/context.rs b/crates/bitwarden-crypto/src/store/context.rs index ed2c41582..4e32f87f1 100644 --- a/crates/bitwarden-crypto/src/store/context.rs +++ b/crates/bitwarden-crypto/src/store/context.rs @@ -10,7 +10,7 @@ use super::KeyStoreInner; use crate::{ derive_shareable_key, error::UnsupportedOperation, signing, store::backend::StoreBackend, AsymmetricCryptoKey, BitwardenLegacyKeyBytes, ContentFormat, CryptoError, EncString, KeyId, - KeyIds, Result, Signature, SignatureAlgorithm, SignedObject, SignedPublicKey, + KeyIds, LocalId, Result, Signature, SignatureAlgorithm, SignedObject, SignedPublicKey, SignedPublicKeyMessage, SigningKey, SymmetricCryptoKey, UnsignedSharedKey, }; @@ -38,13 +38,13 @@ use crate::{ /// # pub enum SymmKeyId { /// # User, /// # #[local] -/// # Local(uuid::Uuid), +/// # Local(LocalId), /// # } /// # #[asymmetric] /// # pub enum AsymmKeyId { /// # UserPrivate, /// # #[local] -/// # Local(uuid::Uuid), +/// # Local(LocalId), /// # } /// # #[signing] /// # pub enum SigningKeyId { @@ -184,7 +184,7 @@ impl KeyStoreContext<'_, Ids> { _ => return Err(CryptoError::InvalidKey), }; - let new_key_id = Ids::Symmetric::new_local(); + let new_key_id = Ids::Symmetric::new_local(LocalId::new()); #[allow(deprecated)] self.set_symmetric_key(new_key_id, key)?; @@ -324,7 +324,7 @@ impl KeyStoreContext<'_, Ids> { name: &str, info: Option<&str>, ) -> Result { - let key_id = Ids::Symmetric::new_local(); + let key_id = Ids::Symmetric::new_local(LocalId::new()); #[allow(deprecated)] self.set_symmetric_key( key_id, @@ -413,7 +413,7 @@ impl KeyStoreContext<'_, Ids> { /// Add a new symmetric key to the local context, returning a new unique identifier for it. pub fn add_local_symmetric_key(&mut self, key: SymmetricCryptoKey) -> Result { - let key_id = Ids::Symmetric::new_local(); + let key_id = Ids::Symmetric::new_local(LocalId::new()); self.local_symmetric_keys.upsert(key_id, key); Ok(key_id) } @@ -438,7 +438,7 @@ impl KeyStoreContext<'_, Ids> { /// Add a new signing key to the local context, returning a new unique identifier for it. pub fn add_local_signing_key(&mut self, key: SigningKey) -> Result { - let key_id = Ids::Signing::new_local(); + let key_id = Ids::Signing::new_local(LocalId::new()); self.local_signing_keys.upsert(key_id, key); Ok(key_id) } @@ -529,7 +529,6 @@ impl KeyStoreContext<'_, Ids> { #[allow(deprecated)] mod tests { use serde::{Deserialize, Serialize}; - use uuid::Uuid; use crate::{ store::{ @@ -537,7 +536,7 @@ mod tests { KeyStore, }, traits::tests::{TestIds, TestSigningKey, TestSymmKey}, - CompositeEncryptable, CryptoError, Decryptable, SignatureAlgorithm, SigningKey, + CompositeEncryptable, CryptoError, Decryptable, LocalId, SignatureAlgorithm, SigningKey, SigningNamespace, SymmetricCryptoKey, }; @@ -579,11 +578,12 @@ mod tests { #[test] fn test_key_encryption() { let store: KeyStore = KeyStore::default(); + let local = LocalId::new(); let mut ctx = store.context(); // Generate and insert a key - let key_1_id = TestSymmKey::C(Uuid::new_v4()); + let key_1_id = TestSymmKey::C(local); let key_1 = SymmetricCryptoKey::make_aes256_cbc_hmac_key(); ctx.set_symmetric_key(key_1_id, key_1.clone()).unwrap(); @@ -591,7 +591,7 @@ mod tests { assert!(ctx.has_symmetric_key(key_1_id)); // Generate and insert a new key - let key_2_id = TestSymmKey::C(Uuid::new_v4()); + let key_2_id = TestSymmKey::C(local); let key_2 = SymmetricCryptoKey::make_aes256_cbc_hmac_key(); ctx.set_symmetric_key(key_2_id, key_2.clone()).unwrap(); diff --git a/crates/bitwarden-crypto/src/store/mod.rs b/crates/bitwarden-crypto/src/store/mod.rs index 9a07cf5e3..fe802dfd9 100644 --- a/crates/bitwarden-crypto/src/store/mod.rs +++ b/crates/bitwarden-crypto/src/store/mod.rs @@ -52,13 +52,13 @@ pub use context::KeyStoreContext; /// pub enum SymmKeyId { /// User, /// #[local] -/// Local(uuid::Uuid), +/// Local(LocalId), /// } /// #[asymmetric] /// pub enum AsymmKeyId { /// UserPrivate, /// #[local] -/// Local(uuid::Uuid), +/// Local(LocalId), /// } /// #[signing] /// pub enum SigningKeyId { diff --git a/crates/bitwarden-crypto/src/traits/key_id.rs b/crates/bitwarden-crypto/src/traits/key_id.rs index 5f6bcd540..87d54c102 100644 --- a/crates/bitwarden-crypto/src/traits/key_id.rs +++ b/crates/bitwarden-crypto/src/traits/key_id.rs @@ -26,7 +26,7 @@ pub trait KeyId: fn is_local(&self) -> bool; /// Creates a new unique local key identifier. - fn new_local() -> Self; + fn new_local(id: LocalId) -> Self; } /// Represents a set of all the key identifiers that need to be defined to use a key store. @@ -40,6 +40,17 @@ pub trait KeyIds { type Signing: KeyId; } +/// An opaque identifier for a local key. Currently only contains a unique ID, but it can be +/// extended to contain scope information to allow cleanup on scope exit. +#[derive(Debug, Clone, Copy, Hash, Eq, PartialEq, Ord, PartialOrd)] +pub struct LocalId(pub(crate) uuid::Uuid); + +impl LocalId { + pub(crate) fn new() -> Self { + LocalId(uuid::Uuid::new_v4()) + } +} + /// Just a small derive_like macro that can be used to generate the key identifier enums. /// Example usage: /// ```rust @@ -50,14 +61,14 @@ pub trait KeyIds { /// User, /// Org(uuid::Uuid), /// #[local] -/// Local(uuid::Uuid), +/// Local(LocalId), /// } /// /// #[asymmetric] /// pub enum AsymmKeyId { /// PrivateKey, /// #[local] -/// Local(uuid::Uuid), +/// Local(LocalId), /// } /// /// #[signing] @@ -81,6 +92,9 @@ macro_rules! key_ids { )+ $ids_vis:vis $ids_name:ident => $symm_name:ident, $asymm_name:ident, $signing_name:ident; ) => { + + use $crate::LocalId; + $( #[derive(std::fmt::Debug, Clone, Copy, std::hash::Hash, Eq, PartialEq, Ord, PartialOrd)] #[allow(missing_docs)] @@ -99,9 +113,9 @@ macro_rules! key_ids { )* } } - fn new_local() -> Self { + fn new_local(id: LocalId) -> Self { $( - { key_ids!(@new_local $variant $( $variant_tag )? ) } + { key_ids!(@new_local $variant id $( $variant_tag )? ) } )* } } @@ -126,31 +140,32 @@ macro_rules! key_ids { ( @variant_value local ) => { true }; ( @variant_value ) => { false }; - ( @new_local $variant:ident local ) => { Self::$variant(uuid::Uuid::new_v4()) }; - ( @new_local $variant:ident ) => {{}}; + ( @new_local $variant:ident $id:ident local ) => { Self::$variant($id) }; + ( @new_local $variant:ident $id:ident ) => {{}}; } #[cfg(test)] pub(crate) mod tests { - use uuid::Uuid; use crate::{ traits::tests::{TestAsymmKey, TestSigningKey, TestSymmKey}, - KeyId, + KeyId, LocalId, }; #[test] fn test_local() { + let local = LocalId::new(); + assert!(!TestSymmKey::A(0).is_local()); assert!(!TestSymmKey::B((4, 10)).is_local()); - assert!(TestSymmKey::C(Uuid::new_v4()).is_local()); + assert!(TestSymmKey::C(local).is_local()); assert!(!TestAsymmKey::A(0).is_local()); assert!(!TestAsymmKey::B.is_local()); - assert!(TestAsymmKey::C(Uuid::new_v4()).is_local()); + assert!(TestAsymmKey::C(local).is_local()); assert!(!TestSigningKey::A(0).is_local()); assert!(!TestSigningKey::B.is_local()); - assert!(TestSigningKey::C(Uuid::new_v4()).is_local()); + assert!(TestSigningKey::C(local).is_local()); } } diff --git a/crates/bitwarden-crypto/src/traits/mod.rs b/crates/bitwarden-crypto/src/traits/mod.rs index 4ba00a197..54946075d 100644 --- a/crates/bitwarden-crypto/src/traits/mod.rs +++ b/crates/bitwarden-crypto/src/traits/mod.rs @@ -5,7 +5,7 @@ mod decryptable; pub use decryptable::Decryptable; pub(crate) mod key_id; -pub use key_id::{KeyId, KeyIds}; +pub use key_id::{KeyId, KeyIds, LocalId}; /// Types implementing [IdentifyKey] are capable of knowing which cryptographic key is /// needed to encrypt/decrypt them. @@ -27,7 +27,7 @@ pub(crate) mod tests { B((u8, u8)), #[local] - C(uuid::Uuid), + C(LocalId), } #[asymmetric] @@ -35,7 +35,7 @@ pub(crate) mod tests { A(u8), B, #[local] - C(uuid::Uuid), + C(LocalId), } #[signing] @@ -43,7 +43,7 @@ pub(crate) mod tests { A(u8), B, #[local] - C(uuid::Uuid), + C(LocalId), } pub TestIds => TestSymmKey, TestAsymmKey, TestSigningKey; From 817cf6e2c724106edb140804ce103ca3b814f5ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garci=CC=81a?= Date: Mon, 30 Jun 2025 15:55:53 +0200 Subject: [PATCH 7/8] Fmt --- crates/bitwarden-crypto/src/store/context.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/bitwarden-crypto/src/store/context.rs b/crates/bitwarden-crypto/src/store/context.rs index 4e32f87f1..58371f7e3 100644 --- a/crates/bitwarden-crypto/src/store/context.rs +++ b/crates/bitwarden-crypto/src/store/context.rs @@ -436,13 +436,6 @@ impl KeyStoreContext<'_, Ids> { Ok(()) } - /// Add a new signing key to the local context, returning a new unique identifier for it. - pub fn add_local_signing_key(&mut self, key: SigningKey) -> Result { - let key_id = Ids::Signing::new_local(LocalId::new()); - self.local_signing_keys.upsert(key_id, key); - Ok(key_id) - } - /// Sets a signing key in the context #[deprecated(note = "This function should ideally never be used outside this crate")] pub fn set_signing_key(&mut self, key_id: Ids::Signing, key: SigningKey) -> Result<()> { @@ -454,6 +447,13 @@ impl KeyStoreContext<'_, Ids> { Ok(()) } + /// Add a new signing key to the local context, returning a new unique identifier for it. + pub fn add_local_signing_key(&mut self, key: SigningKey) -> Result { + let key_id = Ids::Signing::new_local(LocalId::new()); + self.local_signing_keys.upsert(key_id, key); + Ok(key_id) + } + pub(crate) fn decrypt_data_with_symmetric_key( &self, key: Ids::Symmetric, From c4cf8ec1816f791f80aa246841c0502ade0e9312 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garci=CC=81a?= Date: Mon, 30 Jun 2025 16:51:12 +0200 Subject: [PATCH 8/8] Missing docs --- crates/bitwarden-crypto/src/store/context.rs | 2 ++ crates/bitwarden-crypto/src/store/mod.rs | 2 ++ crates/bitwarden-crypto/src/traits/key_id.rs | 2 ++ 3 files changed, 6 insertions(+) diff --git a/crates/bitwarden-crypto/src/store/context.rs b/crates/bitwarden-crypto/src/store/context.rs index 58371f7e3..63ec3784b 100644 --- a/crates/bitwarden-crypto/src/store/context.rs +++ b/crates/bitwarden-crypto/src/store/context.rs @@ -49,6 +49,8 @@ use crate::{ /// # #[signing] /// # pub enum SigningKeyId { /// # UserSigning, +/// # #[local] +/// # Local(LocalId), /// # } /// # pub Ids => SymmKeyId, AsymmKeyId, SigningKeyId; /// # } diff --git a/crates/bitwarden-crypto/src/store/mod.rs b/crates/bitwarden-crypto/src/store/mod.rs index fe802dfd9..55fd69d11 100644 --- a/crates/bitwarden-crypto/src/store/mod.rs +++ b/crates/bitwarden-crypto/src/store/mod.rs @@ -63,6 +63,8 @@ pub use context::KeyStoreContext; /// #[signing] /// pub enum SigningKeyId { /// UserSigning, +/// #[local] +/// Local(LocalId), /// } /// pub Ids => SymmKeyId, AsymmKeyId, SigningKeyId; /// } diff --git a/crates/bitwarden-crypto/src/traits/key_id.rs b/crates/bitwarden-crypto/src/traits/key_id.rs index 87d54c102..a1b756d0d 100644 --- a/crates/bitwarden-crypto/src/traits/key_id.rs +++ b/crates/bitwarden-crypto/src/traits/key_id.rs @@ -74,6 +74,8 @@ impl LocalId { /// #[signing] /// pub enum SigningKeyId { /// SigningKey, +/// #[local] +/// Local(LocalId), /// } /// /// pub Ids => SymmKeyId, AsymmKeyId, SigningKeyId;