From ed075ead9eaadd4856c8d67fd0c4384fb0f02952 Mon Sep 17 00:00:00 2001 From: Konrad Kohbrok Date: Wed, 14 Aug 2024 08:30:55 +0200 Subject: [PATCH] Fix `MlsGroup::delete()` function to delete encryption keypairs (#1644) --- basic_credential/src/lib.rs | 12 ++++++ memory_storage/src/lib.rs | 25 +++++++---- openmls/src/group/mls_group/mod.rs | 15 +++++-- .../tests_and_kats/tests/mls_group.rs | 41 +++++++++++++++++-- 4 files changed, 77 insertions(+), 16 deletions(-) diff --git a/basic_credential/src/lib.rs b/basic_credential/src/lib.rs index 350ea4fc6..9dda632ba 100644 --- a/basic_credential/src/lib.rs +++ b/basic_credential/src/lib.rs @@ -137,6 +137,18 @@ impl SignatureKeyPair { .flatten() } + /// Delete a signature key pair from the key store. + pub fn delete>( + store: &T, + public_key: &[u8], + signature_scheme: SignatureScheme, + ) -> Result<(), T::Error> { + let id = StorageId { + value: id(public_key, signature_scheme), + }; + store.delete_signature_key_pair(&id) + } + /// Get the public key as byte slice. pub fn public(&self) -> &[u8] { self.public.as_ref() diff --git a/memory_storage/src/lib.rs b/memory_storage/src/lib.rs index b59c3b787..9635be228 100644 --- a/memory_storage/src/lib.rs +++ b/memory_storage/src/lib.rs @@ -203,10 +203,7 @@ impl MemoryStorage { log::trace!("{}", std::backtrace::Backtrace::capture()); let value: Vec> = match values.get(&storage_key) { - Some(list_bytes) => { - println!("{}", String::from_utf8(list_bytes.to_vec()).unwrap()); - serde_json::from_slice(list_bytes).unwrap() - } + Some(list_bytes) => serde_json::from_slice(list_bytes).unwrap(), None => vec![], }; @@ -426,7 +423,9 @@ impl StorageProvider for MemoryStorage { let values = self.values.read().unwrap(); let key = build_key::(TREE_LABEL, group_id); - let value = values.get(&key).unwrap(); + let Some(value) = values.get(&key) else { + return Ok(None); + }; let value = serde_json::from_slice(value).unwrap(); Ok(value) @@ -442,7 +441,9 @@ impl StorageProvider for MemoryStorage { let values = self.values.read().unwrap(); let key = build_key::(GROUP_CONTEXT_LABEL, group_id); - let value = values.get(&key).unwrap(); + let Some(value) = values.get(&key) else { + return Ok(None); + }; let value = serde_json::from_slice(value).unwrap(); Ok(value) @@ -458,7 +459,9 @@ impl StorageProvider for MemoryStorage { let values = self.values.read().unwrap(); let key = build_key::(INTERIM_TRANSCRIPT_HASH_LABEL, group_id); - let value = values.get(&key).unwrap(); + let Some(value) = values.get(&key) else { + return Ok(None); + }; let value = serde_json::from_slice(value).unwrap(); Ok(value) @@ -474,7 +477,9 @@ impl StorageProvider for MemoryStorage { let values = self.values.read().unwrap(); let key = build_key::(CONFIRMATION_TAG_LABEL, group_id); - let value = values.get(&key).unwrap(); + let Some(value) = values.get(&key) else { + return Ok(None); + }; let value = serde_json::from_slice(value).unwrap(); Ok(value) @@ -492,7 +497,9 @@ impl StorageProvider for MemoryStorage { let key = build_key::(SIGNATURE_KEY_PAIR_LABEL, public_key); - let value = values.get(&key).unwrap(); + let Some(value) = values.get(&key) else { + return Ok(None); + }; let value = serde_json::from_slice(value).unwrap(); Ok(value) diff --git a/openmls/src/group/mls_group/mod.rs b/openmls/src/group/mls_group/mod.rs index 5a4363fb7..1538f2021 100644 --- a/openmls/src/group/mls_group/mod.rs +++ b/openmls/src/group/mls_group/mod.rs @@ -369,16 +369,23 @@ impl MlsGroup { Ok(build()) } - /// Remove the persisted state from storage - pub fn delete( + /// Remove the persisted state of this group from storage. Note that + /// signature key material is not managed by OpenMLS and has to be removed + /// from the storage provider separately (if desired). + pub fn delete( &mut self, - storage: &StorageProvider, - ) -> Result<(), StorageProvider::Error> { + storage: &Storage, + ) -> Result<(), Storage::Error> { self.group.delete(storage)?; storage.delete_group_config(self.group_id())?; storage.clear_proposal_queue::(self.group_id())?; storage.delete_own_leaf_nodes(self.group_id())?; storage.delete_group_state(self.group_id())?; + storage.delete_encryption_epoch_key_pairs( + self.group_id(), + &self.epoch(), + self.own_leaf_index().u32(), + )?; self.proposal_store_mut().empty(); diff --git a/openmls/src/group/mls_group/tests_and_kats/tests/mls_group.rs b/openmls/src/group/mls_group/tests_and_kats/tests/mls_group.rs index dc08ebf34..849885fb3 100644 --- a/openmls/src/group/mls_group/tests_and_kats/tests/mls_group.rs +++ b/openmls/src/group/mls_group/tests_and_kats/tests/mls_group.rs @@ -1,6 +1,8 @@ use mls_group::tests_and_kats::utils::setup_client; +use openmls_basic_credential::SignatureKeyPair; +use openmls_rust_crypto::MemoryStorage; use openmls_test::openmls_test; -use openmls_traits::OpenMlsProvider as _; +use openmls_traits::{storage::CURRENT_VERSION, OpenMlsProvider as _}; use tls_codec::{Deserialize, Serialize}; use crate::{ @@ -1260,7 +1262,7 @@ fn builder_pattern() { fn update_group_context_with_unknown_extension() { let alice_provider = Provider::default(); let (alice_credential_with_key, _alice_kpb, alice_signer, _alice_pk) = - setup_client("Alice", ciphersuite, &alice_provider); + setup_client("Alice", ciphersuite, provider); // === Define the unknown group context extension and initial data === const UNKNOWN_EXTENSION_TYPE: u16 = 0xff11; @@ -1288,7 +1290,7 @@ fn update_group_context_with_unknown_extension>:: + delete_key_package(alice_provider.storage(),&alice_kpb.key_package().hash_ref(provider.crypto()).unwrap()) + .unwrap(); + >:: + delete_encryption_key_pair(alice_provider.storage(),alice_kpb.key_package().leaf_node().encryption_key()).unwrap(); + + // alice creates MlsGroup + let mut alice_group = MlsGroup::builder() + .ciphersuite(ciphersuite) + .use_ratchet_tree_extension(true) + .build(provider, &alice_signer, alice_credential_with_key) + .expect("error creating group for alice using builder"); + + SignatureKeyPair::delete( + alice_provider.storage(), + alice_pk.as_slice(), + ciphersuite.signature_algorithm(), + ) + .unwrap(); + + // alice deletes the group + alice_group.delete(alice_provider.storage()).unwrap(); + + assert!(alice_provider.storage().values.read().unwrap().is_empty()); +}