diff --git a/crates/bitwarden-core/src/key_management/crypto.rs b/crates/bitwarden-core/src/key_management/crypto.rs index 800ce8085..6d177a263 100644 --- a/crates/bitwarden-core/src/key_management/crypto.rs +++ b/crates/bitwarden-core/src/key_management/crypto.rs @@ -604,8 +604,6 @@ pub(crate) fn make_v2_keys_for_v1_user( let key_store = client.internal.get_key_store(); let mut ctx = key_store.context(); - let temporary_user_key_id = SymmetricKeyId::Local("temporary_user_key"); - let temporary_signing_key_id = SigningKeyId::Local("temporary_signing_key"); // Re-use existing private key let private_key_id = AsymmetricKeyId::UserPrivateKey; @@ -631,13 +629,10 @@ pub(crate) fn make_v2_keys_for_v1_user( // New user key let user_key = SymmetricCryptoKey::make_xchacha20_poly1305_key(); - #[allow(deprecated)] - ctx.set_symmetric_key(temporary_user_key_id, user_key.clone())?; // New signing key let signing_key = SigningKey::make(SignatureAlgorithm::Ed25519); - #[allow(deprecated)] - ctx.set_signing_key(temporary_signing_key_id, signing_key.clone())?; + let temporary_signing_key_id = ctx.add_local_signing_key(signing_key.clone())?; // Sign existing public key let signed_public_key = ctx.make_signed_public_key(private_key_id, temporary_signing_key_id)?; diff --git a/crates/bitwarden-core/src/key_management/mod.rs b/crates/bitwarden-core/src/key_management/mod.rs index 8ce9ded25..e3362775c 100644 --- a/crates/bitwarden-core/src/key_management/mod.rs +++ b/crates/bitwarden-core/src/key_management/mod.rs @@ -35,21 +35,21 @@ key_ids! { User, Organization(uuid::Uuid), #[local] - Local(&'static str), + Local(LocalId), } #[asymmetric] pub enum AsymmetricKeyId { UserPrivateKey, #[local] - Local(&'static str), + Local(LocalId), } #[signing] pub enum SigningKeyId { UserSigningKey, #[local] - Local(&'static str), + Local(LocalId), } pub KeyIds => SymmetricKeyId, AsymmetricKeyId, SigningKeyId; diff --git a/crates/bitwarden-core/src/key_management/security_state.rs b/crates/bitwarden-core/src/key_management/security_state.rs index 9a423ebb5..9132ab34d 100644 --- a/crates/bitwarden-core/src/key_management/security_state.rs +++ b/crates/bitwarden-core/src/key_management/security_state.rs @@ -152,7 +152,7 @@ mod tests { use bitwarden_crypto::{KeyStore, SignatureAlgorithm, SigningKey}; use super::*; - use crate::key_management::{KeyIds, SigningKeyId}; + use crate::key_management::KeyIds; #[test] fn test_security_state_signing() { @@ -162,12 +162,8 @@ mod tests { let user_id = uuid::Uuid::new_v4(); let security_state = SecurityState::initialize_for_user(user_id); let signing_key = SigningKey::make(SignatureAlgorithm::Ed25519); - #[allow(deprecated)] - ctx.set_signing_key(SigningKeyId::Local(""), signing_key.clone()) - .unwrap(); - let signed_security_state = security_state - .sign(SigningKeyId::Local(""), &mut ctx) - .unwrap(); + let key = ctx.add_local_signing_key(signing_key.clone()).unwrap(); + let signed_security_state = security_state.sign(key, &mut ctx).unwrap(); let verifying_key = signing_key.to_verifying_key(); let verified_security_state = signed_security_state diff --git a/crates/bitwarden-crypto/examples/protect_key_with_password.rs b/crates/bitwarden-crypto/examples/protect_key_with_password.rs index 3aa57fca0..e8ebf1774 100644 --- a/crates/bitwarden-crypto/examples/protect_key_with_password.rs +++ b/crates/bitwarden-crypto/examples/protect_key_with_password.rs @@ -22,7 +22,7 @@ fn main() { // Alice has a vault protected with a symmetric key. She wants the symmetric key protected with // a PIN. let vault_key = ctx - .generate_symmetric_key(ExampleSymmetricKey::VaultKey) + .generate_symmetric_key() .expect("Generating vault key should work"); // Seal the key with the PIN @@ -44,7 +44,7 @@ fn main() { ) .expect("Deserializing envelope should work"); deserialized - .unseal(ExampleSymmetricKey::VaultKey, pin, &mut ctx) + .unseal(pin, &mut ctx) .expect("Unsealing should work"); // Alice wants to change her password; also her KDF settings are below the minimums. @@ -55,15 +55,16 @@ fn main() { disk.save("vault_key_envelope", (&envelope).into()); // Alice wants to change the protected key. This requires creating a new envelope - ctx.generate_symmetric_key(ExampleSymmetricKey::VaultKey) + let vault_key = ctx + .generate_symmetric_key() .expect("Generating vault key should work"); - let envelope = PasswordProtectedKeyEnvelope::seal(ExampleSymmetricKey::VaultKey, "0000", &ctx) - .expect("Sealing should work"); + let envelope = + PasswordProtectedKeyEnvelope::seal(vault_key, "0000", &ctx).expect("Sealing should work"); disk.save("vault_key_envelope", (&envelope).into()); // Alice tries the password but it is wrong assert!(matches!( - envelope.unseal(ExampleSymmetricKey::VaultKey, "9999", &mut ctx), + envelope.unseal("9999", &mut ctx), Err(PasswordProtectedKeyEnvelopeError::WrongPassword) )); } @@ -92,17 +93,21 @@ key_ids! { #[symmetric] pub enum ExampleSymmetricKey { #[local] - VaultKey + VaultKey(LocalId) } #[asymmetric] pub enum ExampleAsymmetricKey { Key(u8), + #[local] + Local(LocalId) } #[signing] pub enum ExampleSigningKey { Key(u8), + #[local] + Local(LocalId) } pub ExampleIds => ExampleSymmetricKey, ExampleAsymmetricKey, ExampleSigningKey; diff --git a/crates/bitwarden-crypto/src/lib.rs b/crates/bitwarden-crypto/src/lib.rs index 0c714fb23..e7cad10d4 100644 --- a/crates/bitwarden-crypto/src/lib.rs +++ b/crates/bitwarden-crypto/src/lib.rs @@ -42,7 +42,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/safe/password_protected_key_envelope.rs b/crates/bitwarden-crypto/src/safe/password_protected_key_envelope.rs index 6b28ffec6..5a6e5f99f 100644 --- a/crates/bitwarden-crypto/src/safe/password_protected_key_envelope.rs +++ b/crates/bitwarden-crypto/src/safe/password_protected_key_envelope.rs @@ -138,15 +138,12 @@ impl PasswordProtectedKeyEnvelope { /// context. pub fn unseal( &self, - target_keyslot: Ids::Symmetric, password: &str, ctx: &mut KeyStoreContext, ) -> Result { let key = self.unseal_ref(password)?; - #[allow(deprecated)] - ctx.set_symmetric_key(target_keyslot, key) - .map_err(|_| PasswordProtectedKeyEnvelopeError::KeyStoreError)?; - Ok(target_keyslot) + ctx.add_local_symmetric_key(key) + .map_err(|_| PasswordProtectedKeyEnvelopeError::KeyStoreError) } fn unseal_ref( @@ -508,12 +505,12 @@ mod tests { let envelope = PasswordProtectedKeyEnvelope::try_from(&TESTVECTOR_COSEKEY_ENVELOPE.to_vec()) .expect("Key envelope should be valid"); - envelope - .unseal(TestSymmKey::A(0), TESTVECTOR_PASSWORD, &mut ctx) + let key = envelope + .unseal(TESTVECTOR_PASSWORD, &mut ctx) .expect("Unsealing should succeed"); #[allow(deprecated)] let unsealed_key = ctx - .dangerous_get_symmetric_key(TestSymmKey::A(0)) + .dangerous_get_symmetric_key(key) .expect("Key should exist in the key store"); assert_eq!( unsealed_key.to_encoded().to_vec(), @@ -528,12 +525,12 @@ mod tests { let envelope = PasswordProtectedKeyEnvelope::try_from(&TESTVECTOR_LEGACYKEY_ENVELOPE.to_vec()) .expect("Key envelope should be valid"); - envelope - .unseal(TestSymmKey::A(0), TESTVECTOR_PASSWORD, &mut ctx) + let key = envelope + .unseal(TESTVECTOR_PASSWORD, &mut ctx) .expect("Unsealing should succeed"); #[allow(deprecated)] let unsealed_key = ctx - .dangerous_get_symmetric_key(TestSymmKey::A(0)) + .dangerous_get_symmetric_key(key) .expect("Key should exist in the key store"); assert_eq!( unsealed_key.to_encoded().to_vec(), @@ -556,14 +553,12 @@ mod tests { // Unseal the key from the envelope let deserialized: PasswordProtectedKeyEnvelope = PasswordProtectedKeyEnvelope::try_from(&serialized).unwrap(); - deserialized - .unseal(TestSymmKey::A(1), password, &mut ctx) - .unwrap(); + let key = deserialized.unseal(password, &mut ctx).unwrap(); // Verify that the unsealed key matches the original key #[allow(deprecated)] let unsealed_key = ctx - .dangerous_get_symmetric_key(TestSymmKey::A(1)) + .dangerous_get_symmetric_key(key) .expect("Key should exist in the key store"); #[allow(deprecated)] @@ -578,7 +573,7 @@ mod tests { fn test_make_envelope_legacy_key() { let key_store = KeyStore::::default(); let mut ctx: KeyStoreContext<'_, TestIds> = key_store.context_mut(); - let test_key = ctx.generate_symmetric_key(TestSymmKey::A(0)).unwrap(); + let test_key = ctx.generate_symmetric_key().unwrap(); let password = "test_password"; @@ -589,14 +584,12 @@ mod tests { // Unseal the key from the envelope let deserialized: PasswordProtectedKeyEnvelope = PasswordProtectedKeyEnvelope::try_from(&serialized).unwrap(); - deserialized - .unseal(TestSymmKey::A(1), password, &mut ctx) - .unwrap(); + let key = deserialized.unseal(password, &mut ctx).unwrap(); // Verify that the unsealed key matches the original key #[allow(deprecated)] let unsealed_key = ctx - .dangerous_get_symmetric_key(TestSymmKey::A(1)) + .dangerous_get_symmetric_key(key) .expect("Key should exist in the key store"); #[allow(deprecated)] @@ -645,7 +638,7 @@ mod tests { let deserialized: PasswordProtectedKeyEnvelope = PasswordProtectedKeyEnvelope::try_from(&(&envelope).into()).unwrap(); assert!(matches!( - deserialized.unseal(TestSymmKey::A(1), wrong_password, &mut ctx), + deserialized.unseal(wrong_password, &mut ctx), Err(PasswordProtectedKeyEnvelopeError::WrongPassword) )); } diff --git a/crates/bitwarden-crypto/src/store/context.rs b/crates/bitwarden-crypto/src/store/context.rs index 2e11e809a..a887eba63 100644 --- a/crates/bitwarden-crypto/src/store/context.rs +++ b/crates/bitwarden-crypto/src/store/context.rs @@ -10,9 +10,9 @@ use super::KeyStoreInner; use crate::{ derive_shareable_key, error::UnsupportedOperation, signing, store::backend::StoreBackend, AsymmetricCryptoKey, BitwardenLegacyKeyBytes, ContentFormat, CryptoError, EncString, KeyId, - KeyIds, PublicKeyEncryptionAlgorithm, Result, RotatedUserKeys, Signature, SignatureAlgorithm, - SignedObject, SignedPublicKey, SignedPublicKeyMessage, SigningKey, SymmetricCryptoKey, - UnsignedSharedKey, + KeyIds, LocalId, PublicKeyEncryptionAlgorithm, Result, RotatedUserKeys, Signature, + SignatureAlgorithm, SignedObject, SignedPublicKey, SignedPublicKeyMessage, SigningKey, + SymmetricCryptoKey, UnsignedSharedKey, }; /// The context of a crypto operation using [super::KeyStore] @@ -38,15 +38,20 @@ use crate::{ /// # #[symmetric] /// # pub enum SymmKeyId { /// # User, -/// # Local(&'static str), +/// # #[local] +/// # Local(LocalId), /// # } /// # #[asymmetric] /// # pub enum AsymmKeyId { /// # UserPrivate, +/// # #[local] +/// # Local(LocalId), /// # } /// # #[signing] /// # pub enum SigningKeyId { /// # UserSigning, +/// # #[local] +/// # Local(LocalId), /// # } /// # pub Ids => SymmKeyId, AsymmKeyId, SigningKeyId; /// # } @@ -60,11 +65,10 @@ use crate::{ /// # } /// # } /// -/// const LOCAL_KEY: SymmKeyId = SymmKeyId::Local("local_key_id"); /// /// impl CompositeEncryptable for Data { /// fn encrypt_composite(&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) /// } /// } @@ -150,7 +154,6 @@ impl KeyStoreContext<'_, Ids> { pub fn unwrap_symmetric_key( &mut self, wrapping_key: Ids::Symmetric, - new_key_id: Ids::Symmetric, wrapped_key: &EncString, ) -> Result { let wrapping_key = self.get_symmetric_key(wrapping_key)?; @@ -184,6 +187,8 @@ impl KeyStoreContext<'_, Ids> { _ => return Err(CryptoError::InvalidKey), }; + let new_key_id = Ids::Symmetric::new_local(LocalId::new()); + #[allow(deprecated)] self.set_symmetric_key(new_key_id, key)?; @@ -302,11 +307,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 { - let key = SymmetricCryptoKey::make_aes256_cbc_hmac_key(); - #[allow(deprecated)] - self.set_symmetric_key(key_id, key)?; - Ok(key_id) + pub fn generate_symmetric_key(&mut self) -> Result { + self.add_local_symmetric_key(SymmetricCryptoKey::make_aes256_cbc_hmac_key()) } /// Generate a new random xchacha20-poly1305 symmetric key and store it in the context @@ -323,20 +325,15 @@ impl KeyStoreContext<'_, Ids> { /// Makes a new asymmetric encryption key using the current default algorithm, and stores it in /// the context - pub fn make_asymmetric_key(&mut self, key_id: Ids::Asymmetric) -> Result { + pub fn make_asymmetric_key(&mut self) -> Result { let key = AsymmetricCryptoKey::make(PublicKeyEncryptionAlgorithm::RsaOaepSha1); - #[allow(deprecated)] - self.set_asymmetric_key(key_id, key)?; - Ok(key_id) + self.add_local_asymmetric_key(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. @@ -345,11 +342,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(LocalId::new()); #[allow(deprecated)] self.set_symmetric_key( key_id, @@ -439,6 +436,13 @@ 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(LocalId::new()); + self.local_symmetric_keys.upsert(key_id, key); + Ok(key_id) + } + #[deprecated(note = "This function should ideally never be used outside this crate")] #[allow(missing_docs)] pub fn set_asymmetric_key( @@ -457,6 +461,16 @@ impl KeyStoreContext<'_, Ids> { Ok(()) } + /// Add a new asymmetric key to the local context, returning a new unique identifier for it. + pub fn add_local_asymmetric_key( + &mut self, + key: AsymmetricCryptoKey, + ) -> Result { + let key_id = Ids::Asymmetric::new_local(LocalId::new()); + self.local_asymmetric_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<()> { @@ -468,6 +482,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, @@ -561,11 +582,10 @@ mod tests { tests::{Data, DataView}, KeyStore, }, - traits::tests::{TestAsymmKey, TestIds, TestSigningKey, TestSymmKey}, + traits::tests::{TestIds, TestSigningKey, TestSymmKey}, AsymmetricCryptoKey, AsymmetricPublicCryptoKey, CompositeEncryptable, CoseKeyBytes, - CoseSerializable, CryptoError, Decryptable, KeyDecryptable, Pkcs8PrivateKeyBytes, - PublicKeyEncryptionAlgorithm, SignatureAlgorithm, SigningKey, SigningNamespace, - SymmetricCryptoKey, + CoseSerializable, CryptoError, Decryptable, KeyDecryptable, LocalId, Pkcs8PrivateKeyBytes, + SignatureAlgorithm, SigningKey, SigningNamespace, SymmetricCryptoKey, }; #[test] @@ -606,11 +626,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(1); + 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(); @@ -618,7 +639,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(local); let key_2 = SymmetricCryptoKey::make_aes256_cbc_hmac_key(); ctx.set_symmetric_key(key_2_id, key_2.clone()).unwrap(); @@ -629,10 +650,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 @@ -685,24 +703,18 @@ mod tests { .unwrap(); // Unwrap the keys - let unwrapped_key_2 = ctx - .unwrap_symmetric_key(key_aes_1_id, key_aes_2_id, &wrapped_key_1_2) + let _unwrapped_key_2 = ctx + .unwrap_symmetric_key(key_aes_1_id, &wrapped_key_1_2) .unwrap(); - let unwrapped_key_3 = ctx - .unwrap_symmetric_key(key_aes_1_id, key_xchacha_3_id, &wrapped_key_1_3) + let _unwrapped_key_3 = ctx + .unwrap_symmetric_key(key_aes_1_id, &wrapped_key_1_3) .unwrap(); - let unwrapped_key_1 = ctx - .unwrap_symmetric_key(key_xchacha_3_id, key_aes_1_id, &wrapped_key_3_1) + let _unwrapped_key_1 = ctx + .unwrap_symmetric_key(key_xchacha_3_id, &wrapped_key_3_1) .unwrap(); - let unwrapped_key_4 = ctx - .unwrap_symmetric_key(key_xchacha_3_id, key_xchacha_4_id, &wrapped_key_3_4) + let _unwrapped_key_4 = ctx + .unwrap_symmetric_key(key_xchacha_3_id, &wrapped_key_3_4) .unwrap(); - - // Assert that the unwrapped keys are the same as the original keys - assert_eq!(unwrapped_key_2, key_aes_2_id); - assert_eq!(unwrapped_key_3, key_xchacha_3_id); - assert_eq!(unwrapped_key_1, key_aes_1_id); - assert_eq!(unwrapped_key_4, key_xchacha_4_id); } #[test] @@ -761,18 +773,9 @@ mod tests { let store: KeyStore = KeyStore::default(); let mut ctx = store.context_mut(); - // Generate a new user key - let current_user_private_key_id = TestAsymmKey::A(0); - let current_user_signing_key_id = TestSigningKey::A(0); - // Make the keys - ctx.generate_symmetric_key(TestSymmKey::A(0)).unwrap(); - ctx.make_signing_key(current_user_signing_key_id).unwrap(); - ctx.set_asymmetric_key( - current_user_private_key_id, - AsymmetricCryptoKey::make(PublicKeyEncryptionAlgorithm::RsaOaepSha1), - ) - .unwrap(); + let current_user_signing_key_id = ctx.make_signing_key().unwrap(); + let current_user_private_key_id = ctx.make_asymmetric_key().unwrap(); // Get the rotated account keys let rotated_keys = ctx diff --git a/crates/bitwarden-crypto/src/store/key_rotation.rs b/crates/bitwarden-crypto/src/store/key_rotation.rs index 60509c69b..4ac21e3cb 100644 --- a/crates/bitwarden-crypto/src/store/key_rotation.rs +++ b/crates/bitwarden-crypto/src/store/key_rotation.rs @@ -49,9 +49,8 @@ pub fn dangerous_get_v2_rotated_account_keys( mod tests { use super::*; use crate::{ - traits::tests::{TestAsymmKey, TestIds, TestSigningKey, TestSymmKey}, - AsymmetricCryptoKey, KeyDecryptable, KeyStore, Pkcs8PrivateKeyBytes, - PublicKeyEncryptionAlgorithm, SigningKey, + traits::tests::TestIds, AsymmetricCryptoKey, KeyDecryptable, KeyStore, + Pkcs8PrivateKeyBytes, SigningKey, }; #[test] @@ -59,19 +58,9 @@ mod tests { let store: KeyStore = KeyStore::default(); let mut ctx = store.context_mut(); - // Generate a new user key - let current_user_private_key_id = TestAsymmKey::A(0); - let current_user_signing_key_id = TestSigningKey::A(0); - // Make the keys - ctx.generate_symmetric_key(TestSymmKey::A(0)).unwrap(); - ctx.make_signing_key(current_user_signing_key_id).unwrap(); - #[allow(deprecated)] - ctx.set_asymmetric_key( - current_user_private_key_id, - AsymmetricCryptoKey::make(PublicKeyEncryptionAlgorithm::RsaOaepSha1), - ) - .unwrap(); + let current_user_signing_key_id = ctx.make_signing_key().unwrap(); + let current_user_private_key_id = ctx.make_asymmetric_key().unwrap(); // Get the rotated account keys let rotated_keys = dangerous_get_v2_rotated_account_keys( diff --git a/crates/bitwarden-crypto/src/store/mod.rs b/crates/bitwarden-crypto/src/store/mod.rs index bff22081d..d1ebe215c 100644 --- a/crates/bitwarden-crypto/src/store/mod.rs +++ b/crates/bitwarden-crypto/src/store/mod.rs @@ -55,15 +55,19 @@ pub use key_rotation::*; /// pub enum SymmKeyId { /// User, /// #[local] -/// Local(&'static str) +/// Local(LocalId), /// } /// #[asymmetric] /// pub enum AsymmKeyId { /// UserPrivate, +/// #[local] +/// Local(LocalId), /// } /// #[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 b9f8082c4..a1b756d0d 100644 --- a/crates/bitwarden-crypto/src/traits/key_id.rs +++ b/crates/bitwarden-crypto/src/traits/key_id.rs @@ -24,6 +24,9 @@ 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; + + /// Creates a new unique local key identifier. + fn new_local(id: LocalId) -> Self; } /// Represents a set of all the key identifiers that need to be defined to use a key store. @@ -37,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 @@ -47,17 +61,21 @@ pub trait KeyIds { /// User, /// Org(uuid::Uuid), /// #[local] -/// Local(&'static str), +/// Local(LocalId), /// } /// /// #[asymmetric] /// pub enum AsymmKeyId { /// PrivateKey, +/// #[local] +/// Local(LocalId), /// } /// /// #[signing] /// pub enum SigningKeyId { /// SigningKey, +/// #[local] +/// Local(LocalId), /// } /// /// pub Ids => SymmKeyId, AsymmKeyId, SigningKeyId; @@ -76,6 +94,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)] @@ -93,6 +114,12 @@ macro_rules! key_ids { key_ids!(@variant_value $( $variant_tag )? ), )* } } + + fn new_local(id: LocalId) -> Self { + $( + { key_ids!(@new_local $variant id $( $variant_tag )? ) } + )* + } } )+ @@ -114,27 +141,33 @@ macro_rules! key_ids { ( @variant_value local ) => { true }; ( @variant_value ) => { false }; + + ( @new_local $variant:ident $id:ident local ) => { Self::$variant($id) }; + ( @new_local $variant:ident $id:ident ) => {{}}; } #[cfg(test)] pub(crate) mod tests { + 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(8).is_local()); + assert!(TestSymmKey::C(local).is_local()); assert!(!TestAsymmKey::A(0).is_local()); assert!(!TestAsymmKey::B.is_local()); - assert!(TestAsymmKey::C("test").is_local()); + assert!(TestAsymmKey::C(local).is_local()); assert!(!TestSigningKey::A(0).is_local()); assert!(!TestSigningKey::B.is_local()); - assert!(TestSigningKey::C("test").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 4b9bab2b6..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(u8), + C(LocalId), } #[asymmetric] @@ -35,7 +35,7 @@ pub(crate) mod tests { A(u8), B, #[local] - C(&'static str), + C(LocalId), } #[signing] @@ -43,7 +43,7 @@ pub(crate) mod tests { A(u8), B, #[local] - C(&'static str), + C(LocalId), } pub TestIds => TestSymmKey, TestAsymmKey, TestSigningKey; diff --git a/crates/bitwarden-send/src/send.rs b/crates/bitwarden-send/src/send.rs index dc3af947d..1466364ab 100644 --- a/crates/bitwarden-send/src/send.rs +++ b/crates/bitwarden-send/src/send.rs @@ -146,8 +146,6 @@ pub struct SendListView { pub expiration_date: Option>, } -const SEND_KEY: SymmetricKeyId = SymmetricKeyId::Local("send_key"); - impl Send { #[allow(missing_docs)] pub fn get_key( @@ -164,7 +162,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 2bfa4edb7..a219a7ffe 100644 --- a/crates/bitwarden-vault/src/cipher/attachment.rs +++ b/crates/bitwarden-vault/src/cipher/attachment.rs @@ -78,7 +78,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 { @@ -105,7 +104,7 @@ impl CompositeEncryptable // 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 = OctetStreamBytes::from(self.contents).encrypt(ctx, attachment_key)?; attachment.key = Some(ctx.wrap_symmetric_key(ciphers_key, attachment_key)?); @@ -143,8 +142,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 @@ -178,7 +176,7 @@ impl Decryptable for Attachment { ) -> Result { #[cfg(feature = "wasm")] let decrypted_key = if let Some(attachment_key) = &self.key { - let content_key_id = ctx.unwrap_symmetric_key(key, ATTACHMENT_KEY, attachment_key)?; + let content_key_id = ctx.unwrap_symmetric_key(key, attachment_key)?; #[allow(deprecated)] let actual_key = ctx.dangerous_get_symmetric_key(content_key_id)?; diff --git a/crates/bitwarden-vault/src/cipher/cipher.rs b/crates/bitwarden-vault/src/cipher/cipher.rs index cce9a2360..bc11b46f8 100644 --- a/crates/bitwarden-vault/src/cipher/cipher.rs +++ b/crates/bitwarden-vault/src/cipher/cipher.rs @@ -403,9 +403,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), } } @@ -450,9 +449,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)?; @@ -486,8 +483,8 @@ impl CipherView { if let Some(attachments) = &mut self.attachments { for attachment in attachments { if let Some(attachment_key) = &mut attachment.key { - let tmp_attachment_key_id = SymmetricKeyId::Local("attachment_key"); - ctx.unwrap_symmetric_key(old_key, tmp_attachment_key_id, attachment_key)?; + let tmp_attachment_key_id = + ctx.unwrap_symmetric_key(old_key, attachment_key)?; *attachment_key = ctx.wrap_symmetric_key(new_key, tmp_attachment_key_id)?; } } @@ -976,9 +973,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) @@ -993,12 +989,8 @@ mod tests { // Make sure that the cipher key is decryptable let wrapped_key = original_cipher.key.unwrap(); let mut ctx = key_store.context(); - ctx.unwrap_symmetric_key( - SymmetricKeyId::User, - SymmetricKeyId::Local("test_cipher_key"), - &wrapped_key, - ) - .unwrap(); + ctx.unwrap_symmetric_key(SymmetricKeyId::User, &wrapped_key) + .unwrap(); } #[test] @@ -1037,16 +1029,14 @@ mod tests { .unwrap(); // Re-encrypt the cipher key with a new wrapping key - let new_key_id: SymmetricKeyId = SymmetricKeyId::Local("new_cipher_key"); - #[allow(deprecated)] - ctx.set_symmetric_key(new_key_id, new_key).unwrap(); + let new_key_id = ctx.add_local_symmetric_key(new_key).unwrap(); cipher.reencrypt_cipher_keys(&mut ctx, new_key_id).unwrap(); // Check that the cipher key can be unwrapped with the new key assert!(cipher.key.is_some()); assert!(ctx - .unwrap_symmetric_key(new_key_id, new_key_id, &cipher.key.unwrap()) + .unwrap_symmetric_key(new_key_id, &cipher.key.unwrap()) .is_ok()); } @@ -1058,8 +1048,9 @@ mod tests { let mut cipher = generate_cipher(); // The cipher does not have a key, so re-encryption should not add one + let new_cipher_key = ctx.generate_symmetric_key().unwrap(); cipher - .reencrypt_cipher_keys(&mut ctx, SymmetricKeyId::Local("new_cipher_key")) + .reencrypt_cipher_keys(&mut ctx, new_cipher_key) .unwrap(); // Check that the cipher key is still None @@ -1147,9 +1138,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(); @@ -1187,11 +1176,7 @@ mod tests { let new_attachment_key = cipher.attachments.unwrap()[0].key.clone().unwrap(); let mut ctx = key_store.context(); let new_attachment_key_id = ctx - .unwrap_symmetric_key( - org_key, - SymmetricKeyId::Local("test_attachment_key"), - &new_attachment_key, - ) + .unwrap_symmetric_key(org_key, &new_attachment_key) .unwrap(); #[allow(deprecated)] let new_attachment_key_dec = ctx @@ -1223,17 +1208,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(); @@ -1258,11 +1239,7 @@ mod tests { // Check that the cipher key has been re-encrypted with the org key, let wrapped_new_cipher_key = cipher.key.clone().unwrap(); let new_cipher_key_dec = ctx - .unwrap_symmetric_key( - org_key, - SymmetricKeyId::Local("test_cipher_key"), - &wrapped_new_cipher_key, - ) + .unwrap_symmetric_key(org_key, &wrapped_new_cipher_key) .unwrap(); #[allow(deprecated)] let new_cipher_key_dec = ctx.dangerous_get_symmetric_key(new_cipher_key_dec).unwrap(); diff --git a/crates/bitwarden-vault/src/cipher/cipher_client.rs b/crates/bitwarden-vault/src/cipher/cipher_client.rs index 45e9d846a..11bb32ae1 100644 --- a/crates/bitwarden-vault/src/cipher/cipher_client.rs +++ b/crates/bitwarden-vault/src/cipher/cipher_client.rs @@ -1,4 +1,4 @@ -use bitwarden_core::{key_management::SymmetricKeyId, Client, OrganizationId}; +use bitwarden_core::{Client, OrganizationId}; use bitwarden_crypto::{CompositeEncryptable, IdentifyKey, SymmetricCryptoKey}; #[cfg(feature = "wasm")] use wasm_bindgen::prelude::*; @@ -73,9 +73,7 @@ impl CiphersClient { let mut ctx = key_store.context(); // Set the new key in the key store context - const NEW_KEY_ID: SymmetricKeyId = SymmetricKeyId::Local("new_cipher_key"); - #[allow(deprecated)] - ctx.set_symmetric_key(NEW_KEY_ID, new_key)?; + let new_key_id = ctx.add_local_symmetric_key(new_key)?; if cipher_view.key.is_none() && self @@ -84,12 +82,12 @@ impl CiphersClient { .get_flags() .enable_cipher_key_encryption { - cipher_view.generate_cipher_key(&mut ctx, NEW_KEY_ID)?; + cipher_view.generate_cipher_key(&mut ctx, new_key_id)?; } else { - cipher_view.reencrypt_cipher_keys(&mut ctx, NEW_KEY_ID)?; + cipher_view.reencrypt_cipher_keys(&mut ctx, new_key_id)?; } - let cipher = cipher_view.encrypt_composite(&mut ctx, NEW_KEY_ID)?; + let cipher = cipher_view.encrypt_composite(&mut ctx, new_key_id)?; Ok(EncryptionContext { cipher, diff --git a/crates/bitwarden-wasm-internal/src/pure_crypto.rs b/crates/bitwarden-wasm-internal/src/pure_crypto.rs index eb1aefa7e..1b7895f5d 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; #[allow(deprecated)] use bitwarden_crypto::dangerous_derive_kdf_material; use bitwarden_crypto::{ @@ -135,18 +135,13 @@ impl PureCrypto { let mut context = tmp_store.context(); let wrapping_key = SymmetricCryptoKey::try_from(&BitwardenLegacyKeyBytes::from(wrapping_key))?; - #[allow(deprecated)] - context.set_symmetric_key(SymmetricKeyId::Local("wrapping_key"), wrapping_key)?; + let wrapping_key = context.add_local_symmetric_key(wrapping_key)?; let key_to_be_wrapped = SymmetricCryptoKey::try_from(&BitwardenLegacyKeyBytes::from(key_to_be_wrapped))?; - #[allow(deprecated)] - context.set_symmetric_key(SymmetricKeyId::Local("key_to_wrap"), key_to_be_wrapped)?; + let key_to_wrap = context.add_local_symmetric_key(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()) } @@ -160,16 +155,12 @@ impl PureCrypto { let mut context = tmp_store.context(); let wrapping_key = SymmetricCryptoKey::try_from(&BitwardenLegacyKeyBytes::from(wrapping_key))?; - #[allow(deprecated)] - context.set_symmetric_key(SymmetricKeyId::Local("wrapping_key"), wrapping_key)?; + let wrapping_key = context.add_local_symmetric_key(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().to_vec()) } @@ -184,13 +175,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(&BitwardenLegacyKeyBytes::from(wrapping_key))?, - )?; + let wrapping_key = context.add_local_symmetric_key(SymmetricCryptoKey::try_from( + &BitwardenLegacyKeyBytes::from(wrapping_key), + )?)?; Ok(SpkiPublicKeyBytes::from(encapsulation_key) - .encrypt(&mut context, SymmetricKeyId::Local("wrapping_key"))? + .encrypt(&mut context, wrapping_key)? .to_string()) } @@ -202,13 +191,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(&BitwardenLegacyKeyBytes::from(wrapping_key))?, - )?; - EncString::from_str(wrapped_key.as_str())? - .decrypt(&mut context, SymmetricKeyId::Local("wrapping_key")) + let wrapping_key = context.add_local_symmetric_key(SymmetricCryptoKey::try_from( + &BitwardenLegacyKeyBytes::from(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 @@ -219,13 +205,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(&BitwardenLegacyKeyBytes::from(wrapping_key))?, - )?; + let wrapping_key = context.add_local_symmetric_key(SymmetricCryptoKey::try_from( + &BitwardenLegacyKeyBytes::from(wrapping_key), + )?)?; Ok(Pkcs8PrivateKeyBytes::from(decapsulation_key) - .encrypt(&mut context, SymmetricKeyId::Local("wrapping_key"))? + .encrypt(&mut context, wrapping_key)? .to_string()) } @@ -237,13 +221,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(&BitwardenLegacyKeyBytes::from(wrapping_key))?, - )?; - EncString::from_str(wrapped_key.as_str())? - .decrypt(&mut context, SymmetricKeyId::Local("wrapping_key")) + let wrapping_key = context.add_local_symmetric_key(SymmetricCryptoKey::try_from( + &BitwardenLegacyKeyBytes::from(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)