From ebb5b7c0d1529b3f54c4c3244342ba29af4d29bb Mon Sep 17 00:00:00 2001 From: "Jan Winkelmann (keks)" Date: Mon, 19 Aug 2024 11:32:32 +0200 Subject: [PATCH 01/15] update test and add fix --- openmls/src/group/core_group/process.rs | 4 ++ .../tests_and_kats/tests/past_secrets.rs | 47 ++++++++++++------- 2 files changed, 34 insertions(+), 17 deletions(-) diff --git a/openmls/src/group/core_group/process.rs b/openmls/src/group/core_group/process.rs index e9b6cac05..342e9f59a 100644 --- a/openmls/src/group/core_group/process.rs +++ b/openmls/src/group/core_group/process.rs @@ -301,6 +301,10 @@ impl CoreGroup { self.message_secrets_store .add(past_epoch, message_secrets, leaves); } + provider + .storage() + .write_message_secrets(self.group_id(), &self.message_secrets_store) + .map_err(MergeCommitError::StorageError)?; Ok(()) } } diff --git a/openmls/src/group/tests_and_kats/tests/past_secrets.rs b/openmls/src/group/tests_and_kats/tests/past_secrets.rs index b5158e673..8e0569bff 100644 --- a/openmls/src/group/tests_and_kats/tests/past_secrets.rs +++ b/openmls/src/group/tests_and_kats/tests/past_secrets.rs @@ -7,11 +7,16 @@ use crate::{ treesync::LeafNodeParameters, }; +use openmls_traits::OpenMlsProvider as _; + #[openmls_test::openmls_test] -fn test_past_secrets_in_group( +fn test_past_secrets_in_group( ciphersuite: Ciphersuite, - provider: &impl crate::storage::OpenMlsProvider, + provider: &Provider, ) { + let alice_provider = &mut Provider::default(); + let bob_provider = &mut Provider::default(); + // Test this for different parameters for max_epochs in (0..10usize).step_by(2) { let group_id = GroupId::from_slice(b"Test Group"); @@ -20,19 +25,19 @@ fn test_past_secrets_in_group( let alice_credential_with_keys = generate_credential_with_key( b"Alice".to_vec(), ciphersuite.signature_algorithm(), - provider, + alice_provider, ); let bob_credential_with_keys = generate_credential_with_key( b"Bob".to_vec(), ciphersuite.signature_algorithm(), - provider, + bob_provider, ); // Generate KeyPackages let bob_key_package = generate_key_package( ciphersuite, Extensions::empty(), - provider, + bob_provider, bob_credential_with_keys, ); @@ -45,7 +50,7 @@ fn test_past_secrets_in_group( // === Alice creates a group === let mut alice_group = MlsGroup::new_with_group_id( - provider, + alice_provider, &alice_credential_with_keys.signer, &mls_group_create_config, group_id.clone(), @@ -56,14 +61,14 @@ fn test_past_secrets_in_group( // Alice adds Bob let (_message, welcome, _group_info) = alice_group .add_members( - provider, + alice_provider, &alice_credential_with_keys.signer, &[bob_key_package.key_package().clone()], ) .expect("An unexpected error occurred."); alice_group - .merge_pending_commit(provider) + .merge_pending_commit(alice_provider) .expect("error merging pending commit"); let welcome: MlsMessageIn = welcome.into(); @@ -72,13 +77,13 @@ fn test_past_secrets_in_group( .expect("expected message to be a welcome"); let mut bob_group = StagedWelcome::new_from_welcome( - provider, + bob_provider, mls_group_create_config.join_config(), welcome, Some(alice_group.export_ratchet_tree().into()), ) .expect("Error creating staged join from Welcome") - .into_group(provider) + .into_group(bob_provider) .expect("Error creating group from staged join"); // Generate application message for different epochs @@ -88,14 +93,18 @@ fn test_past_secrets_in_group( for _ in 0..max_epochs { let application_message = alice_group - .create_message(provider, &alice_credential_with_keys.signer, &[1, 2, 3]) + .create_message( + alice_provider, + &alice_credential_with_keys.signer, + &[1, 2, 3], + ) .expect("An unexpected error occurred."); application_messages.push(application_message.into_protocol_message().unwrap()); let (message, _welcome, _group_info) = alice_group .self_update( - provider, + alice_provider, &alice_credential_with_keys.signer, LeafNodeParameters::default(), ) @@ -104,7 +113,7 @@ fn test_past_secrets_in_group( update_commits.push(message.clone()); alice_group - .merge_pending_commit(provider) + .merge_pending_commit(alice_provider) .expect("error merging pending commit"); } @@ -112,14 +121,14 @@ fn test_past_secrets_in_group( for update_commit in update_commits { let bob_processed_message = bob_group - .process_message(provider, update_commit.into_protocol_message().unwrap()) + .process_message(bob_provider, update_commit.into_protocol_message().unwrap()) .expect("An unexpected error occurred."); if let ProcessedMessageContent::StagedCommitMessage(staged_commit) = bob_processed_message.into_content() { bob_group - .merge_staged_commit(provider, *staged_commit) + .merge_staged_commit(bob_provider, *staged_commit) .expect("Error merging commit."); } else { unreachable!("Expected a StagedCommit."); @@ -128,10 +137,14 @@ fn test_past_secrets_in_group( // === Test application messages from older epochs === + let mut bob_group = MlsGroup::load(bob_provider.storage(), &group_id) + .expect("error re-loading bob's group") + .expect("no such group"); + // The first messages should fail for application_message in application_messages.iter().take(max_epochs / 2) { let err = bob_group - .process_message(provider, application_message.clone()) + .process_message(bob_provider, application_message.clone()) .expect_err("An unexpected error occurred."); assert!(matches!( err, @@ -144,7 +157,7 @@ fn test_past_secrets_in_group( // The last messages should not fail for application_message in application_messages.iter().skip(max_epochs / 2) { let bob_processed_message = bob_group - .process_message(provider, application_message.clone()) + .process_message(bob_provider, application_message.clone()) .expect("An unexpected error occurred."); if let ProcessedMessageContent::ApplicationMessage(application_message) = From 19d7a90fc6fb2dd1fa6456e18523f9324fcb71fe Mon Sep 17 00:00:00 2001 From: "Jan Winkelmann (keks)" Date: Mon, 19 Aug 2024 14:45:25 +0200 Subject: [PATCH 02/15] get rid of CoreGroup::merge_staged_commit --- openmls/src/group/core_group/process.rs | 23 ------------------- openmls/src/group/core_group/staged_commit.rs | 14 ++++++++--- openmls/src/group/mls_group/processing.rs | 2 +- 3 files changed, 12 insertions(+), 27 deletions(-) diff --git a/openmls/src/group/core_group/process.rs b/openmls/src/group/core_group/process.rs index 342e9f59a..794289c61 100644 --- a/openmls/src/group/core_group/process.rs +++ b/openmls/src/group/core_group/process.rs @@ -284,27 +284,4 @@ impl CoreGroup { Ok((old_epoch_keypairs, leaf_node_keypairs)) } - - /// Merge a [StagedCommit] into the group after inspection - pub(crate) fn merge_staged_commit( - &mut self, - provider: &Provider, - staged_commit: StagedCommit, - ) -> Result<(), MergeCommitError> { - // Save the past epoch - let past_epoch = self.context().epoch(); - // Get all the full leaves - let leaves = self.public_group().members().collect(); - // Merge the staged commit into the group state and store the secret tree from the - // previous epoch in the message secrets store. - if let Some(message_secrets) = self.merge_commit(provider, staged_commit)? { - self.message_secrets_store - .add(past_epoch, message_secrets, leaves); - } - provider - .storage() - .write_message_secrets(self.group_id(), &self.message_secrets_store) - .map_err(MergeCommitError::StorageError)?; - Ok(()) - } } diff --git a/openmls/src/group/core_group/staged_commit.rs b/openmls/src/group/core_group/staged_commit.rs index b5d3f5d56..76a55afaa 100644 --- a/openmls/src/group/core_group/staged_commit.rs +++ b/openmls/src/group/core_group/staged_commit.rs @@ -319,7 +319,7 @@ impl CoreGroup { &mut self, provider: &Provider, staged_commit: StagedCommit, - ) -> Result, MergeCommitError> { + ) -> Result<(), MergeCommitError> { // Get all keypairs from the old epoch, so we can later store the ones // that are still relevant in the new epoch. let old_epoch_keypairs = self.read_epoch_keypairs(provider.storage()); @@ -329,9 +329,15 @@ impl CoreGroup { .merge_diff(staged_state.into_staged_diff()); self.store(provider.storage()) .map_err(MergeCommitError::StorageError)?; - Ok(None) + Ok(()) } StagedCommitState::GroupMember(state) => { + // Save the past epoch + let past_epoch = self.context().epoch(); + // Get all the full leaves + let leaves = self.public_group().members().collect(); + // Merge the staged commit into the group state and store the secret tree from the + // previous epoch in the message secrets store. self.group_epoch_secrets = state.group_epoch_secrets; // Replace the previous message secrets with the new ones and return the previous message secrets @@ -340,6 +346,8 @@ impl CoreGroup { &mut message_secrets, self.message_secrets_store.message_secrets_mut(), ); + self.message_secrets_store + .add(past_epoch, message_secrets, leaves); self.public_group.merge_diff(state.staged_diff); @@ -407,7 +415,7 @@ impl CoreGroup { .map_err(MergeCommitError::StorageError)?; self.proposal_store_mut().empty(); - Ok(Some(message_secrets)) + Ok(()) } } } diff --git a/openmls/src/group/mls_group/processing.rs b/openmls/src/group/mls_group/processing.rs index 1a22eb683..4b297a3e2 100644 --- a/openmls/src/group/mls_group/processing.rs +++ b/openmls/src/group/mls_group/processing.rs @@ -142,7 +142,7 @@ impl MlsGroup { .map_err(MergeCommitError::StorageError)?; // Merge staged commit - self.group.merge_staged_commit(provider, staged_commit)?; + self.group.merge_commit(provider, staged_commit)?; // Extract and store the resumption psk for the current epoch let resumption_psk = self.group.group_epoch_secrets().resumption_psk(); From bf0439b1667587e6df6dfd3717fcc89594d7404a Mon Sep 17 00:00:00 2001 From: "Jan Winkelmann (keks)" Date: Mon, 19 Aug 2024 15:42:32 +0200 Subject: [PATCH 03/15] fix clippy --- openmls/src/group/core_group/process.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openmls/src/group/core_group/process.rs b/openmls/src/group/core_group/process.rs index 794289c61..1fb9c23c1 100644 --- a/openmls/src/group/core_group/process.rs +++ b/openmls/src/group/core_group/process.rs @@ -3,7 +3,7 @@ use core_group::proposals::QueuedProposal; use crate::{ framing::mls_content::FramedContentBody, group::{ - errors::{MergeCommitError, StageCommitError, ValidationError}, + errors::{StageCommitError, ValidationError}, mls_group::errors::ProcessMessageError, }, }; From 1cb25615f1dab65d9922826bf0e4786ebc3639c4 Mon Sep 17 00:00:00 2001 From: "Jan Winkelmann (keks)" Date: Thu, 22 Aug 2024 11:50:25 +0200 Subject: [PATCH 04/15] bump versions and fix build error --- basic_credential/Cargo.toml | 8 ++++---- delivery-service/ds/Cargo.toml | 1 + libcrux_crypto/Cargo.toml | 6 +++--- memory_storage/Cargo.toml | 4 ++-- openmls/Cargo.toml | 16 ++++++++-------- openmls_rust_crypto/Cargo.toml | 6 +++--- openmls_test/Cargo.toml | 8 ++++---- traits/Cargo.toml | 2 +- 8 files changed, 26 insertions(+), 25 deletions(-) diff --git a/basic_credential/Cargo.toml b/basic_credential/Cargo.toml index 84016983c..507515bab 100644 --- a/basic_credential/Cargo.toml +++ b/basic_credential/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "openmls_basic_credential" -version = "0.3.0-pre.1" +version = "0.3.0-pre.2" authors = ["OpenMLS Authors"] edition = "2021" description = "A Basic Credential implementation for OpenMLS" @@ -10,7 +10,7 @@ repository = "https://github.com/openmls/openmls/tree/main/basic_credential" readme = "README.md" [dependencies] -openmls_traits = { version = "0.3.0-pre.2", path = "../traits" } +openmls_traits = { version = "0.3.0-pre.3", path = "../traits" } tls_codec = { workspace = true } serde = "1.0" @@ -20,5 +20,5 @@ p256 = { version = "0.13" } rand = "0.8" [features] -clonable = [] # Make the keys clonable -test-utils = [] # Only use for tests! +clonable = [] # Make the keys clonable +test-utils = [] # Only use for tests! diff --git a/delivery-service/ds/Cargo.toml b/delivery-service/ds/Cargo.toml index 5d7589267..03b43c0dd 100644 --- a/delivery-service/ds/Cargo.toml +++ b/delivery-service/ds/Cargo.toml @@ -18,6 +18,7 @@ serde = { version = "1.0", features = ["derive"] } uuid = { version = "1", features = ["serde", "v4"] } clap = "4" base64 = "0.13" +time = ">=0.3.36" openmls = { path = "../../openmls", features = ["test-utils"] } diff --git a/libcrux_crypto/Cargo.toml b/libcrux_crypto/Cargo.toml index a2cb16811..789f71cc7 100644 --- a/libcrux_crypto/Cargo.toml +++ b/libcrux_crypto/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "openmls_libcrux_crypto" -version = "0.1.0-pre.2" +version = "0.1.0-pre.3" edition = "2021" authors = ["OpenMLS Authors"] description = "A crypto backend for OpenMLS based on libcrux implementing openmls_traits." @@ -12,7 +12,7 @@ readme = "../README.md" [dependencies] getrandom = "0.2.12" libcrux = { version = "=0.0.2-alpha.3", features = ["rand"] } -openmls_traits = { version = "0.3.0-pre.2", path = "../traits" } -openmls_memory_storage = { version = "0.3.0-pre.2", path = "../memory_storage" } +openmls_traits = { version = "0.3.0-pre.3", path = "../traits" } +openmls_memory_storage = { version = "0.3.0-pre.3", path = "../memory_storage" } rand = "0.8.5" tls_codec.workspace = true diff --git a/memory_storage/Cargo.toml b/memory_storage/Cargo.toml index 4b4c1dc69..7833d71db 100644 --- a/memory_storage/Cargo.toml +++ b/memory_storage/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "openmls_memory_storage" authors = ["OpenMLS Authors"] -version = "0.3.0-pre.2" +version = "0.3.0-pre.3" edition = "2021" description = "A very basic storage for OpenMLS implementing openmls_traits." license = "MIT" @@ -10,7 +10,7 @@ repository = "https://github.com/openmls/openmls/tree/main/memory_storage" readme = "README.md" [dependencies] -openmls_traits = { version = "0.3.0-pre.2", path = "../traits" } +openmls_traits = { version = "0.3.0-pre.3", path = "../traits" } thiserror = "1.0" serde_json = "1.0" serde = { version = "1.0", features = ["derive"] } diff --git a/openmls/Cargo.toml b/openmls/Cargo.toml index c96b8cf55..6ca7d9470 100644 --- a/openmls/Cargo.toml +++ b/openmls/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "openmls" -version = "0.6.0-pre.2" +version = "0.6.0-pre.3" authors = ["OpenMLS Authors"] edition = "2021" description = "A Rust implementation of the Messaging Layer Security (MLS) protocol, as defined in RFC 9420." @@ -12,17 +12,17 @@ keywords = ["MLS", "IETF", "RFC9420", "Encryption", "E2EE"] exclude = ["/test_vectors"] [dependencies] -openmls_traits = { version = "0.3.0-pre.2", path = "../traits" } -openmls_rust_crypto = { version = "0.3.0-pre.1", path = "../openmls_rust_crypto", optional = true } -openmls_basic_credential = { version = "0.3.0-pre.1", path = "../basic_credential", optional = true, features = [ +openmls_traits = { version = "0.3.0-pre.3", path = "../traits" } +openmls_rust_crypto = { version = "0.3.0-pre.2", path = "../openmls_rust_crypto", optional = true } +openmls_basic_credential = { version = "0.3.0-pre.2", path = "../basic_credential", optional = true, features = [ "clonable", "test-utils", ] } -openmls_memory_storage = { version = "0.3.0-pre.2", path = "../memory_storage", features = [ +openmls_memory_storage = { version = "0.3.0-pre.3", path = "../memory_storage", features = [ "test-utils", ], optional = true } -openmls_test = { version = "0.1.0-pre.1", path = "../openmls_test", optional = true } -openmls_libcrux_crypto = { version = "0.1.0-pre.2", path = "../libcrux_crypto", optional = true } +openmls_test = { version = "0.1.0-pre.2", path = "../openmls_test", optional = true } +openmls_libcrux_crypto = { version = "0.1.0-pre.3", path = "../libcrux_crypto", optional = true } serde = { version = "^1.0", features = ["derive"] } log = { version = "0.4", features = ["std"] } tls_codec = { workspace = true } @@ -70,7 +70,7 @@ criterion = { version = "^0.5", default-features = false } # need to disable def hex = { version = "0.4", features = ["serde"] } itertools = "0.10" lazy_static = "1.4" -openmls_traits = { version = "0.3.0-pre.2", path = "../traits", features = [ +openmls_traits = { version = "0.3.0-pre.3", path = "../traits", features = [ "test-utils", ] } pretty_env_logger = "0.5" diff --git a/openmls_rust_crypto/Cargo.toml b/openmls_rust_crypto/Cargo.toml index d25544542..1f914742e 100644 --- a/openmls_rust_crypto/Cargo.toml +++ b/openmls_rust_crypto/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "openmls_rust_crypto" authors = ["OpenMLS Authors"] -version = "0.3.0-pre.1" +version = "0.3.0-pre.2" edition = "2021" description = "A crypto backend for OpenMLS implementing openmls_traits using RustCrypto primitives." license = "MIT" @@ -10,8 +10,8 @@ repository = "https://github.com/openmls/openmls/tree/main/openmls_rust_crypto" readme = "README.md" [dependencies] -openmls_traits = { version = "0.3.0-pre.2", path = "../traits" } -openmls_memory_storage = { version = "0.3.0-pre.2", path = "../memory_storage" } +openmls_traits = { version = "0.3.0-pre.3", path = "../traits" } +openmls_memory_storage = { version = "0.3.0-pre.3", path = "../memory_storage" } hpke = { version = "0.2.0", package = "hpke-rs", default-features = false, features = [ "hazmat", "serialization", diff --git a/openmls_test/Cargo.toml b/openmls_test/Cargo.toml index 02b9cbe1c..61d530ff7 100644 --- a/openmls_test/Cargo.toml +++ b/openmls_test/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "openmls_test" -version = "0.1.0-pre.1" +version = "0.1.0-pre.2" authors = ["OpenMLS Authors"] edition = "2021" description = "Test utility used by OpenMLS" @@ -23,6 +23,6 @@ ansi_term = "0.12.1" quote = "1.0" rstest = { version = "0.17" } rstest_reuse = { version = "0.5" } -openmls_rust_crypto = { version = "0.3.0-pre.1", path = "../openmls_rust_crypto" } -openmls_libcrux_crypto = { version = "0.1.0-pre.2", path = "../libcrux_crypto", optional = true } -openmls_traits = { version = "0.3.0-pre.2", path = "../traits" } +openmls_rust_crypto = { version = "0.3.0-pre.2", path = "../openmls_rust_crypto" } +openmls_libcrux_crypto = { version = "0.1.0-pre.3", path = "../libcrux_crypto", optional = true } +openmls_traits = { version = "0.3.0-pre.3", path = "../traits" } diff --git a/traits/Cargo.toml b/traits/Cargo.toml index dad103802..a8c6dbe28 100644 --- a/traits/Cargo.toml +++ b/traits/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "openmls_traits" -version = "0.3.0-pre.2" +version = "0.3.0-pre.3" authors = ["OpenMLS Authors"] edition = "2021" description = "Traits used by OpenMLS" From 9f1d5cf2d37973342f9f271ac05851222b2e229c Mon Sep 17 00:00:00 2001 From: Konrad Kohbrok Date: Thu, 29 Aug 2024 13:32:26 +0200 Subject: [PATCH 05/15] Slightly refactor public group storage interface (#1649) * make function naming consistent in PublicStorage * refactor and make PublicGroup::delete public --- openmls/src/group/core_group/mod.rs | 2 +- openmls/src/group/public_group/mod.rs | 14 +++++++------- traits/src/public_storage.rs | 4 ++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/openmls/src/group/core_group/mod.rs b/openmls/src/group/core_group/mod.rs index d3f7f08ec..31bbd08c1 100644 --- a/openmls/src/group/core_group/mod.rs +++ b/openmls/src/group/core_group/mod.rs @@ -758,7 +758,7 @@ impl CoreGroup { &self, storage: &Storage, ) -> Result<(), Storage::Error> { - self.public_group.delete(storage)?; + PublicGroup::delete(self.group_id(), storage)?; storage.delete_own_leaf_index(self.group_id())?; storage.delete_group_epoch_secrets(self.group_id())?; storage.delete_message_secrets(self.group_id())?; diff --git a/openmls/src/group/public_group/mod.rs b/openmls/src/group/public_group/mod.rs index 32dffa739..043712a57 100644 --- a/openmls/src/group/public_group/mod.rs +++ b/openmls/src/group/public_group/mod.rs @@ -396,14 +396,14 @@ impl PublicGroup { } /// Deletes the [`PublicGroup`] from storage. - pub(crate) fn delete( - &self, + pub fn delete( + group_id: &GroupId, storage: &Storage, ) -> Result<(), Storage::PublicError> { - storage.delete_tree(self.group_id())?; - storage.delete_confirmation_tag(self.group_id())?; - storage.delete_context(self.group_id())?; - storage.delete_interim_transcript_hash(self.group_id())?; + storage.delete_tree(group_id)?; + storage.delete_confirmation_tag(group_id)?; + storage.delete_context(group_id)?; + storage.delete_interim_transcript_hash(group_id)?; Ok(()) } @@ -413,7 +413,7 @@ impl PublicGroup { storage: &Storage, group_id: &GroupId, ) -> Result, Storage::PublicError> { - let treesync = storage.treesync(group_id)?; + let treesync = storage.tree(group_id)?; let proposals: Vec<(ProposalRef, QueuedProposal)> = storage.queued_proposals(group_id)?; let group_context = storage.group_context(group_id)?; let interim_transcript_hash: Option = diff --git a/traits/src/public_storage.rs b/traits/src/public_storage.rs index 56a7f12f3..3a193c733 100644 --- a/traits/src/public_storage.rs +++ b/traits/src/public_storage.rs @@ -76,7 +76,7 @@ pub trait PublicStorageProvider { ) -> Result, Self::PublicError>; /// Returns the TreeSync tree for the group with group id `group_id`. - fn treesync< + fn tree< GroupId: crate::storage::traits::GroupId, TreeSync: crate::storage::traits::TreeSync, >( @@ -233,7 +233,7 @@ where >::queued_proposals(self, group_id) } - fn treesync< + fn tree< GroupId: crate::storage::traits::GroupId, TreeSync: crate::storage::traits::TreeSync, >( From 72c9d107050b64e08307f373fafadb2d62f0ea8e Mon Sep 17 00:00:00 2001 From: Konrad Kohbrok Date: Fri, 30 Aug 2024 13:16:30 +0200 Subject: [PATCH 06/15] Remove `CoreGroup` (#1647) --- cli/src/openmls_rust_persistent_crypto.rs | 2 +- memory_storage/src/lib.rs | 10 +- openmls/src/framing/validation.rs | 8 +- .../group/core_group/create_commit_params.rs | 95 -- openmls/src/group/core_group/mod.rs | 1207 ----------------- .../core_group/new_from_external_init.rs | 144 -- .../src/group/core_group/new_from_welcome.rs | 309 ----- openmls/src/group/core_group/process.rs | 287 ---- openmls/src/group/errors.rs | 42 +- openmls/src/group/mls_group/application.rs | 18 +- openmls/src/group/mls_group/builder.rs | 142 +- openmls/src/group/mls_group/create_commit.rs | 393 ++++++ openmls/src/group/mls_group/creation.rs | 388 +++++- openmls/src/group/mls_group/exporting.rs | 83 +- openmls/src/group/mls_group/membership.rs | 25 +- openmls/src/group/mls_group/mod.rs | 514 +++++-- .../{core_group => mls_group}/past_secrets.rs | 0 openmls/src/group/mls_group/processing.rs | 257 +++- openmls/src/group/mls_group/proposal.rs | 149 +- .../proposal_store.rs} | 0 .../staged_commit.rs | 25 +- .../tests_and_kats/tests/core_group.rs | 737 ---------- .../tests/create_commit_params.rs | 2 +- .../tests_and_kats/tests/mls_group.rs | 753 +++++++++- .../mls_group/tests_and_kats/tests/mod.rs | 1 - .../tests_and_kats/tests/proposals.rs | 6 +- openmls/src/group/mls_group/updates.rs | 17 +- openmls/src/group/mod.rs | 12 +- openmls/src/group/public_group/builder.rs | 4 - .../public_group/diff/apply_proposals.rs | 2 +- .../group/public_group/diff/compute_path.rs | 2 +- openmls/src/group/public_group/mod.rs | 15 +- openmls/src/group/public_group/process.rs | 7 +- .../src/group/public_group/staged_commit.rs | 8 +- openmls/src/group/public_group/tests.rs | 4 +- openmls/src/group/public_group/validation.rs | 6 +- openmls/src/group/tests_and_kats/mod.rs | 2 - .../tests_and_kats/tests/commit_validation.rs | 20 +- .../group/tests_and_kats/tests/encoding.rs | 4 +- .../tests/external_commit_validation.rs | 14 +- .../tests/external_remove_proposal.rs | 1 - .../src/group/tests_and_kats/tests/framing.rs | 2 +- .../tests/framing_validation.rs | 16 +- .../tests/group_context_extensions.rs | 16 +- .../tests/proposal_validation.rs | 20 +- .../src/group/tests_and_kats/tree_printing.rs | 22 - openmls/src/group/tests_and_kats/utils.rs | 14 +- openmls/src/prelude.rs | 2 +- openmls/src/schedule/mod.rs | 2 +- openmls/src/storage.rs | 3 +- openmls/src/storage/kat_storage_stability.rs | 18 +- .../src/test_utils/test_framework/errors.rs | 2 +- .../kats/kat_message_protection.rs | 7 +- openmls/src/treesync/diff.rs | 2 +- openmls/src/treesync/mod.rs | 22 +- .../kats/kat_tree_operations.rs | 5 +- .../tests_and_kats/kats/kat_treekem.rs | 2 +- openmls/tests/external_commit.rs | 4 +- traits/src/storage.rs | 1 - 59 files changed, 2553 insertions(+), 3322 deletions(-) delete mode 100644 openmls/src/group/core_group/create_commit_params.rs delete mode 100644 openmls/src/group/core_group/mod.rs delete mode 100644 openmls/src/group/core_group/new_from_external_init.rs delete mode 100644 openmls/src/group/core_group/new_from_welcome.rs delete mode 100644 openmls/src/group/core_group/process.rs create mode 100644 openmls/src/group/mls_group/create_commit.rs rename openmls/src/group/{core_group => mls_group}/past_secrets.rs (100%) rename openmls/src/group/{core_group/proposals.rs => mls_group/proposal_store.rs} (100%) rename openmls/src/group/{core_group => mls_group}/staged_commit.rs (96%) delete mode 100644 openmls/src/group/mls_group/tests_and_kats/tests/core_group.rs delete mode 100644 openmls/src/group/tests_and_kats/tree_printing.rs diff --git a/cli/src/openmls_rust_persistent_crypto.rs b/cli/src/openmls_rust_persistent_crypto.rs index 2c01d9a58..65504ec19 100644 --- a/cli/src/openmls_rust_persistent_crypto.rs +++ b/cli/src/openmls_rust_persistent_crypto.rs @@ -1,6 +1,6 @@ //! # OpenMLS Default Crypto Provider //! -//! This is an implementation of the [`OpenMlsCryptoProvider`] trait to use with +//! This is an implementation of the [`OpenMlsProvider`] trait to use with //! OpenMLS. use openmls_rust_crypto::{MemoryStorage, RustCrypto}; diff --git a/memory_storage/src/lib.rs b/memory_storage/src/lib.rs index 9635be228..91f01d948 100644 --- a/memory_storage/src/lib.rs +++ b/memory_storage/src/lib.rs @@ -262,18 +262,16 @@ const GROUP_CONTEXT_LABEL: &[u8] = b"GroupContext"; const INTERIM_TRANSCRIPT_HASH_LABEL: &[u8] = b"InterimTranscriptHash"; const CONFIRMATION_TAG_LABEL: &[u8] = b"ConfirmationTag"; -// related to CoreGroup -const OWN_LEAF_NODE_INDEX_LABEL: &[u8] = b"OwnLeafNodeIndex"; -const EPOCH_SECRETS_LABEL: &[u8] = b"EpochSecrets"; -const RESUMPTION_PSK_STORE_LABEL: &[u8] = b"ResumptionPsk"; -const MESSAGE_SECRETS_LABEL: &[u8] = b"MessageSecrets"; - // related to MlsGroup const JOIN_CONFIG_LABEL: &[u8] = b"MlsGroupJoinConfig"; const OWN_LEAF_NODES_LABEL: &[u8] = b"OwnLeafNodes"; const GROUP_STATE_LABEL: &[u8] = b"GroupState"; const QUEUED_PROPOSAL_LABEL: &[u8] = b"QueuedProposal"; const PROPOSAL_QUEUE_REFS_LABEL: &[u8] = b"ProposalQueueRefs"; +const OWN_LEAF_NODE_INDEX_LABEL: &[u8] = b"OwnLeafNodeIndex"; +const EPOCH_SECRETS_LABEL: &[u8] = b"EpochSecrets"; +const RESUMPTION_PSK_STORE_LABEL: &[u8] = b"ResumptionPsk"; +const MESSAGE_SECRETS_LABEL: &[u8] = b"MessageSecrets"; impl StorageProvider for MemoryStorage { type Error = MemoryStorageError; diff --git a/openmls/src/framing/validation.rs b/openmls/src/framing/validation.rs index 914e75d05..5f470ef94 100644 --- a/openmls/src/framing/validation.rs +++ b/openmls/src/framing/validation.rs @@ -24,16 +24,14 @@ // TODO #106/#151: Update the above diagram use openmls_traits::{crypto::OpenMlsCrypto, types::Ciphersuite}; +use proposal_store::QueuedProposal; use crate::{ binary_tree::LeafNodeIndex, ciphersuite::signable::Verifiable, error::LibraryError, extensions::ExternalSendersExtension, - group::{ - core_group::{proposals::QueuedProposal, staged_commit::StagedCommit}, - errors::ValidationError, - }, + group::{errors::ValidationError, mls_group::staged_commit::StagedCommit}, tree::sender_ratchet::SenderRatchetConfiguration, treesync::TreeSync, versions::ProtocolVersion, @@ -96,7 +94,7 @@ impl DecryptedMessage { pub(crate) fn from_inbound_ciphertext( ciphertext: PrivateMessageIn, crypto: &impl OpenMlsCrypto, - group: &mut CoreGroup, + group: &mut MlsGroup, sender_ratchet_configuration: &SenderRatchetConfiguration, ) -> Result { // This will be refactored with #265. diff --git a/openmls/src/group/core_group/create_commit_params.rs b/openmls/src/group/core_group/create_commit_params.rs deleted file mode 100644 index c5fbf4646..000000000 --- a/openmls/src/group/core_group/create_commit_params.rs +++ /dev/null @@ -1,95 +0,0 @@ -//! Builder for [CreateCommitParams] that is used in [CoreGroup::create_commit()] - -use serde::{Deserialize, Serialize}; - -use crate::{ - credentials::CredentialWithKey, framing::FramingParameters, messages::proposals::Proposal, -}; - -#[cfg(doc)] -use super::CoreGroup; -use super::LeafNodeParameters; - -/// Can be used to denote the type of a commit. -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] -pub(crate) enum CommitType { - External(CredentialWithKey), - Member, -} - -pub(crate) struct CreateCommitParams<'a> { - framing_parameters: FramingParameters<'a>, // Mandatory - inline_proposals: Vec, // Optional - force_self_update: bool, // Optional - commit_type: CommitType, // Optional (default is `Member`) - leaf_node_parameters: LeafNodeParameters, // Optional -} - -pub(crate) struct TempBuilderCCPM0 {} - -pub(crate) struct CreateCommitParamsBuilder<'a> { - ccp: CreateCommitParams<'a>, -} - -impl TempBuilderCCPM0 { - pub(crate) fn framing_parameters( - self, - framing_parameters: FramingParameters, - ) -> CreateCommitParamsBuilder { - CreateCommitParamsBuilder { - ccp: CreateCommitParams { - framing_parameters, - inline_proposals: vec![], - force_self_update: true, - commit_type: CommitType::Member, - leaf_node_parameters: LeafNodeParameters::default(), - }, - } - } -} - -impl<'a> CreateCommitParamsBuilder<'a> { - pub(crate) fn inline_proposals(mut self, inline_proposals: Vec) -> Self { - self.ccp.inline_proposals = inline_proposals; - self - } - pub(crate) fn force_self_update(mut self, force_self_update: bool) -> Self { - self.ccp.force_self_update = force_self_update; - self - } - pub(crate) fn commit_type(mut self, commit_type: CommitType) -> Self { - self.ccp.commit_type = commit_type; - self - } - pub(crate) fn leaf_node_parameters(mut self, leaf_node_parameters: LeafNodeParameters) -> Self { - self.ccp.leaf_node_parameters = leaf_node_parameters; - self - } - pub(crate) fn build(self) -> CreateCommitParams<'a> { - self.ccp - } -} - -impl<'a> CreateCommitParams<'a> { - pub(crate) fn builder() -> TempBuilderCCPM0 { - TempBuilderCCPM0 {} - } - pub(crate) fn framing_parameters(&self) -> &FramingParameters { - &self.framing_parameters - } - pub(crate) fn inline_proposals(&self) -> &[Proposal] { - &self.inline_proposals - } - pub(crate) fn set_inline_proposals(&mut self, inline_proposals: Vec) { - self.inline_proposals = inline_proposals; - } - pub(crate) fn force_self_update(&self) -> bool { - self.force_self_update - } - pub(crate) fn commit_type(&self) -> &CommitType { - &self.commit_type - } - pub(crate) fn leaf_node_parameters(&self) -> &LeafNodeParameters { - &self.leaf_node_parameters - } -} diff --git a/openmls/src/group/core_group/mod.rs b/openmls/src/group/core_group/mod.rs deleted file mode 100644 index 31bbd08c1..000000000 --- a/openmls/src/group/core_group/mod.rs +++ /dev/null @@ -1,1207 +0,0 @@ -//! ### Don't Panic! -//! -//! Functions in this module should never panic. However, if there is a bug in -//! the implementation, a function will return an unrecoverable `LibraryError`. -//! This means that some functions that are not expected to fail and throw an -//! error, will still return a `Result` since they may throw a `LibraryError`. - -// Private -pub(super) mod new_from_welcome; - -// Crate -pub(crate) mod create_commit_params; -pub(crate) mod new_from_external_init; -pub(crate) mod past_secrets; -pub(crate) mod process; -pub(crate) mod proposals; -pub(crate) mod staged_commit; - -use log::{debug, trace}; -use openmls_traits::{ - crypto::OpenMlsCrypto, signatures::Signer, storage::StorageProvider as _, types::Ciphersuite, -}; -use serde::{Deserialize, Serialize}; -use tls_codec::Serialize as TlsSerializeTrait; - -use self::{ - create_commit_params::{CommitType, CreateCommitParams}, - node::leaf_node::Capabilities, - past_secrets::MessageSecretsStore, - staged_commit::{MemberStagedCommitState, StagedCommit, StagedCommitState}, -}; - -use super::{ - builder::TempBuilderPG1, - errors::{ - CoreGroupBuildError, CreateAddProposalError, CreateCommitError, ExporterError, - ValidationError, - }, - group_context::*, - public_group::{diff::compute_path::PathComputationResult, PublicGroup}, -}; - -use crate::{ - binary_tree::array_representation::{LeafNodeIndex, TreeSize}, - ciphersuite::{signable::Signable, HpkePublicKey}, - credentials::*, - error::LibraryError, - extensions::errors::InvalidExtensionError, - framing::{mls_auth_content::AuthenticatedContent, *}, - group::*, - key_packages::*, - messages::{ - group_info::{GroupInfo, GroupInfoTBS, VerifiableGroupInfo}, - proposals::*, - *, - }, - schedule::{ - message_secrets::*, - psk::{load_psks, store::ResumptionPskStore, PskSecret}, - *, - }, - storage::{OpenMlsProvider, StorageProvider}, - tree::{secret_tree::SecretTreeError, sender_ratchet::SenderRatchetConfiguration}, - treesync::{node::encryption_keys::EncryptionKeyPair, *}, - versions::ProtocolVersion, -}; - -#[cfg(test)] -use super::errors::CreateGroupContextExtProposalError; -#[cfg(test)] -use crate::treesync::node::leaf_node::TreePosition; - -#[derive(Debug)] -pub(crate) struct CreateCommitResult { - pub(crate) commit: AuthenticatedContent, - pub(crate) welcome_option: Option, - pub(crate) staged_commit: StagedCommit, - pub(crate) group_info: Option, -} - -/// A member in the group is identified by this [`Member`] struct. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub struct Member { - /// The member's leaf index in the ratchet tree. - pub index: LeafNodeIndex, - /// The member's credential. - pub credential: Credential, - /// The member's public HPHKE encryption key. - pub encryption_key: Vec, - /// The member's public signature key. - pub signature_key: Vec, -} - -impl Member { - /// Create new member. - pub fn new( - index: LeafNodeIndex, - encryption_key: Vec, - signature_key: Vec, - credential: Credential, - ) -> Self { - Self { - index, - encryption_key, - signature_key, - credential, - } - } -} - -/// A [`StagedCoreWelcome`] can be inspected and then turned into a [`CoreGroup`]. -/// This allows checking who authored the Welcome message. -#[derive(Debug)] -pub(crate) struct StagedCoreWelcome { - public_group: PublicGroup, - group_epoch_secrets: GroupEpochSecrets, - own_leaf_index: LeafNodeIndex, - - /// Group config. - /// Set to true if the ratchet tree extension is added to the `GroupInfo`. - /// Defaults to `false`. - use_ratchet_tree_extension: bool, - - /// A [`MessageSecretsStore`] that stores message secrets. - /// By default this store has the length of 1, i.e. only the [`MessageSecrets`] - /// of the current epoch is kept. - /// If more secrets from past epochs should be kept in order to be - /// able to decrypt application messages from previous epochs, the size of - /// the store must be increased through [`max_past_epochs()`]. - message_secrets_store: MessageSecretsStore, - - /// Resumption psk store. This is where the resumption psks are kept in a rollover list. - pub(crate) resumption_psk_store: ResumptionPskStore, - - /// The [`VerifiableGroupInfo`] from the [`Welcome`] message. - verifiable_group_info: VerifiableGroupInfo, - - /// The key package bundle used for this welcome. - pub(crate) key_package_bundle: KeyPackageBundle, - - /// If we got a path secret, these are the derived path keys. - path_keypairs: Option>, -} - -#[derive(Debug)] -#[cfg_attr(any(test, feature = "test-utils"), derive(Clone, PartialEq))] -pub(crate) struct CoreGroup { - public_group: PublicGroup, - group_epoch_secrets: GroupEpochSecrets, - own_leaf_index: LeafNodeIndex, - /// Group config. - /// Set to true if the ratchet tree extension is added to the `GroupInfo`. - /// Defaults to `false`. - use_ratchet_tree_extension: bool, - /// A [`MessageSecretsStore`] that stores message secrets. - /// By default this store has the length of 1, i.e. only the [`MessageSecrets`] - /// of the current epoch is kept. - /// If more secrets from past epochs should be kept in order to be - /// able to decrypt application messages from previous epochs, the size of - /// the store must be increased through [`max_past_epochs()`]. - message_secrets_store: MessageSecretsStore, - // Resumption psk store. This is where the resumption psks are kept in a rollover list. - pub(crate) resumption_psk_store: ResumptionPskStore, -} - -/// Builder for [`CoreGroup`]. -pub(crate) struct CoreGroupBuilder { - public_group_builder: TempBuilderPG1, - config: Option, - psk_ids: Vec, - max_past_epochs: usize, -} - -impl CoreGroupBuilder { - /// Create a new [`CoreGroupBuilder`]. - pub(crate) fn new( - group_id: GroupId, - ciphersuite: Ciphersuite, - credential_with_key: CredentialWithKey, - ) -> Self { - let public_group_builder = PublicGroup::builder(group_id, ciphersuite, credential_with_key); - Self { - config: None, - psk_ids: vec![], - max_past_epochs: 0, - public_group_builder, - } - } - - /// Set the [`CoreGroupConfig`] of the [`CoreGroup`]. - pub(crate) fn with_config(mut self, config: CoreGroupConfig) -> Self { - self.config = Some(config); - self - } - - /// Set the [`Capabilities`] of the group's creator. - pub(crate) fn with_capabilities(mut self, capabilities: Capabilities) -> Self { - self.public_group_builder = self.public_group_builder.with_capabilities(capabilities); - self - } - - /// Sets initial group context extensions. Note that RequiredCapabilities - /// extensions will be overwritten, and should be set using a call to - /// `required_capabilities`. If `ExternalSenders` extensions are provided - /// both in this call, and a call to `external_senders`, only the one from - /// the call to `external_senders` will be included. - pub(crate) fn with_group_context_extensions( - mut self, - extensions: Extensions, - ) -> Result { - self.public_group_builder = self - .public_group_builder - .with_group_context_extensions(extensions)?; - Ok(self) - } - - /// Sets extensions of the group creator's [`LeafNode`]. - pub(crate) fn with_leaf_node_extensions( - mut self, - extensions: Extensions, - ) -> Result { - self.public_group_builder = self - .public_group_builder - .with_leaf_node_extensions(extensions)?; - Ok(self) - } - - /// Set the number of past epochs the group should keep secrets. - pub fn with_max_past_epoch_secrets(mut self, max_past_epochs: usize) -> Self { - self.max_past_epochs = max_past_epochs; - self - } - - /// Set the [`Lifetime`] for the own leaf in the group. - pub fn with_lifetime(mut self, lifetime: Lifetime) -> Self { - self.public_group_builder = self.public_group_builder.with_lifetime(lifetime); - self - } - - /// Build the [`CoreGroup`]. - /// Any values that haven't been set in the builder are set to their default - /// values (which might be random). - /// - /// This function performs cryptographic operations and there requires an - /// [`OpenMlsProvider`]. - pub(crate) fn build( - self, - provider: &Provider, - signer: &impl Signer, - ) -> Result> { - let (public_group_builder, commit_secret, leaf_keypair) = - self.public_group_builder.get_secrets(provider, signer)?; - - let ciphersuite = public_group_builder.group_context().ciphersuite(); - let config = self.config.unwrap_or_default(); - - let serialized_group_context = public_group_builder - .group_context() - .tls_serialize_detached() - .map_err(LibraryError::missing_bound_check)?; - - debug!("Created group {:x?}", public_group_builder.group_id()); - trace!(" >>> with {:?}, {:?}", ciphersuite, config); - // Derive an initial joiner secret based on the commit secret. - // Derive an epoch secret from the joiner secret. - // We use a random `InitSecret` for initialization. - let joiner_secret = JoinerSecret::new( - provider.crypto(), - ciphersuite, - commit_secret, - &InitSecret::random(ciphersuite, provider.rand()) - .map_err(LibraryError::unexpected_crypto_error)?, - &serialized_group_context, - ) - .map_err(LibraryError::unexpected_crypto_error)?; - - // TODO(#1357) - let resumption_psk_store = ResumptionPskStore::new(32); - - // Prepare the PskSecret - let psk_secret = { - let psks = load_psks(provider.storage(), &resumption_psk_store, &self.psk_ids)?; - - PskSecret::new(provider.crypto(), ciphersuite, psks)? - }; - - let mut key_schedule = - KeySchedule::init(ciphersuite, provider.crypto(), &joiner_secret, psk_secret)?; - key_schedule - .add_context(provider.crypto(), &serialized_group_context) - .map_err(|_| LibraryError::custom("Using the key schedule in the wrong state"))?; - - let epoch_secrets = key_schedule - .epoch_secrets(provider.crypto(), ciphersuite) - .map_err(|_| LibraryError::custom("Using the key schedule in the wrong state"))?; - - let (group_epoch_secrets, message_secrets) = epoch_secrets.split_secrets( - serialized_group_context, - TreeSize::new(1), - LeafNodeIndex::new(0u32), - ); - - let initial_confirmation_tag = message_secrets - .confirmation_key() - .tag(provider.crypto(), ciphersuite, &[]) - .map_err(LibraryError::unexpected_crypto_error)?; - - let message_secrets_store = - MessageSecretsStore::new_with_secret(self.max_past_epochs, message_secrets); - - let public_group = public_group_builder - .with_confirmation_tag(initial_confirmation_tag) - .build(provider.crypto())?; - - let group = CoreGroup { - public_group, - group_epoch_secrets, - use_ratchet_tree_extension: config.add_ratchet_tree_extension, - message_secrets_store, - own_leaf_index: LeafNodeIndex::new(0), - resumption_psk_store, - }; - - // Store the group state - group - .store(provider.storage()) - .map_err(CoreGroupBuildError::StorageError)?; - - // Store the private key of the own leaf in the key store as an epoch keypair. - group - .store_epoch_keypairs(provider.storage(), &[leaf_keypair]) - .map_err(CoreGroupBuildError::StorageError)?; - - Ok(group) - } -} - -impl CoreGroup { - /// Get a builder for [`CoreGroup`]. - pub(crate) fn builder( - group_id: GroupId, - ciphersuite: Ciphersuite, - credential_with_key: CredentialWithKey, - ) -> CoreGroupBuilder { - CoreGroupBuilder::new(group_id, ciphersuite, credential_with_key) - } - - // === Create handshake messages === - // TODO: share functionality between these. - - // 11.1.1. Add - // struct { - // KeyPackage key_package; - // } Add; - pub(crate) fn create_add_proposal( - &self, - framing_parameters: FramingParameters, - joiner_key_package: KeyPackage, - signer: &impl Signer, - ) -> Result { - if let Some(required_capabilities) = self.required_capabilities() { - joiner_key_package - .leaf_node() - .capabilities() - .supports_required_capabilities(required_capabilities)?; - } - let add_proposal = AddProposal { - key_package: joiner_key_package, - }; - let proposal = Proposal::Add(add_proposal); - AuthenticatedContent::member_proposal( - framing_parameters, - self.own_leaf_index(), - proposal, - self.context(), - signer, - ) - .map_err(|e| e.into()) - } - - // 11.1.2. Update - // struct { - // KeyPackage key_package; - // } Update; - pub(crate) fn create_update_proposal( - &self, - framing_parameters: FramingParameters, - // XXX: There's no need to own this. The [`UpdateProposal`] should - // operate on a reference to make this more efficient. - leaf_node: LeafNode, - signer: &impl Signer, - ) -> Result { - let update_proposal = UpdateProposal { leaf_node }; - let proposal = Proposal::Update(update_proposal); - AuthenticatedContent::member_proposal( - framing_parameters, - self.own_leaf_index(), - proposal, - self.context(), - signer, - ) - } - - // 11.1.3. Remove - // struct { - // KeyPackageRef removed; - // } Remove; - pub(crate) fn create_remove_proposal( - &self, - framing_parameters: FramingParameters, - removed: LeafNodeIndex, - signer: &impl Signer, - ) -> Result { - if self.public_group().leaf(removed).is_none() { - return Err(ValidationError::UnknownMember); - } - let remove_proposal = RemoveProposal { removed }; - let proposal = Proposal::Remove(remove_proposal); - AuthenticatedContent::member_proposal( - framing_parameters, - self.own_leaf_index(), - proposal, - self.context(), - signer, - ) - .map_err(ValidationError::LibraryError) - } - - // 11.1.4. PreSharedKey - // struct { - // PreSharedKeyID psk; - // } PreSharedKey; - // TODO: #751 - pub(crate) fn create_presharedkey_proposal( - &self, - framing_parameters: FramingParameters, - psk: PreSharedKeyId, - signer: &impl Signer, - ) -> Result { - let presharedkey_proposal = PreSharedKeyProposal::new(psk); - let proposal = Proposal::PreSharedKey(presharedkey_proposal); - AuthenticatedContent::member_proposal( - framing_parameters, - self.own_leaf_index(), - proposal, - self.context(), - signer, - ) - } - - pub(crate) fn create_custom_proposal( - &self, - framing_parameters: FramingParameters, - custom_proposal: CustomProposal, - signer: &impl Signer, - ) -> Result { - let proposal = Proposal::Custom(custom_proposal); - AuthenticatedContent::member_proposal( - framing_parameters, - self.own_leaf_index(), - proposal, - self.context(), - signer, - ) - } - - // Create application message - pub(crate) fn create_application_message( - &mut self, - aad: &[u8], - msg: &[u8], - padding_size: usize, - provider: &Provider, - signer: &impl Signer, - ) -> Result> { - let public_message = AuthenticatedContent::new_application( - self.own_leaf_index(), - aad, - msg, - self.context(), - signer, - )?; - self.encrypt(public_message, padding_size, provider) - } - - // Encrypt an PublicMessage into an PrivateMessage - pub(crate) fn encrypt( - &mut self, - public_message: AuthenticatedContent, - padding_size: usize, - provider: &Provider, - ) -> Result> { - let msg = PrivateMessage::try_from_authenticated_content( - &public_message, - self.ciphersuite(), - provider, - self.message_secrets_store.message_secrets_mut(), - padding_size, - )?; - - provider - .storage() - .write_message_secrets(self.group_id(), &self.message_secrets_store) - .map_err(MessageEncryptionError::StorageError)?; - - Ok(msg) - } - - /// Exporter - pub(crate) fn export_secret( - &self, - crypto: &impl OpenMlsCrypto, - label: &str, - context: &[u8], - key_length: usize, - ) -> Result, ExporterError> { - if key_length > u16::MAX.into() { - log::error!("Got a key that is larger than u16::MAX"); - return Err(ExporterError::KeyLengthTooLong); - } - Ok(self - .group_epoch_secrets - .exporter_secret() - .derive_exported_secret(self.ciphersuite(), crypto, label, context, key_length) - .map_err(LibraryError::unexpected_crypto_error)?) - } - - pub(crate) fn export_group_info( - &self, - crypto: &impl OpenMlsCrypto, - signer: &impl Signer, - with_ratchet_tree: bool, - ) -> Result { - let extensions = { - let ratchet_tree_extension = || { - Extension::RatchetTree(RatchetTreeExtension::new( - self.public_group().export_ratchet_tree(), - )) - }; - - let external_pub_extension = || { - let external_pub = self - .group_epoch_secrets() - .external_secret() - .derive_external_keypair(crypto, self.ciphersuite()) - .map_err(LibraryError::unexpected_crypto_error)? - .public; - Ok(Extension::ExternalPub(ExternalPubExtension::new( - HpkePublicKey::from(external_pub), - ))) - }; - - if with_ratchet_tree { - Extensions::from_vec(vec![ratchet_tree_extension(), external_pub_extension()?]) - .map_err(|_| { - LibraryError::custom( - "There should not have been duplicate extensions here.", - ) - })? - } else { - Extensions::single(external_pub_extension()?) - } - }; - - // Create to-be-signed group info. - let group_info_tbs = GroupInfoTBS::new( - self.context().clone(), - extensions, - self.message_secrets() - .confirmation_key() - .tag( - crypto, - self.ciphersuite(), - self.context().confirmed_transcript_hash(), - ) - .map_err(LibraryError::unexpected_crypto_error)?, - self.own_leaf_index(), - ); - - // Sign to-be-signed group info. - group_info_tbs - .sign(signer) - .map_err(|_| LibraryError::custom("Signing failed")) - } - - /// Returns the epoch authenticator - pub(crate) fn epoch_authenticator(&self) -> &EpochAuthenticator { - self.group_epoch_secrets().epoch_authenticator() - } - - /// Returns the resumption PSK secret - pub(crate) fn resumption_psk_secret(&self) -> &ResumptionPskSecret { - self.group_epoch_secrets().resumption_psk() - } - - /// Returns a reference to the public group. - pub(crate) fn public_group(&self) -> &PublicGroup { - &self.public_group - } - - /// Returns a reference to the proposal store. - pub(crate) fn proposal_store(&self) -> &ProposalStore { - self.public_group.proposal_store() - } - - /// Returns a mutable reference to the proposal store. - pub(crate) fn proposal_store_mut(&mut self) -> &mut ProposalStore { - self.public_group.proposal_store_mut() - } - - /// Get the ciphersuite implementation used in this group. - pub(crate) fn ciphersuite(&self) -> Ciphersuite { - self.public_group.ciphersuite() - } - - /// Get the MLS version used in this group. - pub(crate) fn version(&self) -> ProtocolVersion { - self.public_group.version() - } - - /// Get the group context - pub(crate) fn context(&self) -> &GroupContext { - self.public_group.group_context() - } - - /// Get the group ID - pub(crate) fn group_id(&self) -> &GroupId { - self.public_group.group_id() - } - - /// Get the required capabilities extension of this group. - pub(crate) fn required_capabilities(&self) -> Option<&RequiredCapabilitiesExtension> { - self.public_group.required_capabilities() - } - - /// Get the leaf index of this client. - pub(crate) fn own_leaf_index(&self) -> LeafNodeIndex { - self.own_leaf_index - } - - /// Get a reference to the group epoch secrets from the group - pub(crate) fn group_epoch_secrets(&self) -> &GroupEpochSecrets { - &self.group_epoch_secrets - } - - /// Get a reference to the message secrets from a group - pub(crate) fn message_secrets(&self) -> &MessageSecrets { - self.message_secrets_store.message_secrets() - } - - /// Sets the size of the [`MessageSecretsStore`], i.e. the number of past - /// epochs to keep. - /// This allows application messages from previous epochs to be decrypted. - pub(crate) fn set_max_past_epochs(&mut self, max_past_epochs: usize) { - self.message_secrets_store.resize(max_past_epochs); - } - - /// Get the message secrets. Either from the secrets store or from the group. - pub(crate) fn message_secrets_mut( - &mut self, - epoch: GroupEpoch, - ) -> Result<&mut MessageSecrets, SecretTreeError> { - if epoch < self.context().epoch() { - self.message_secrets_store - .secrets_for_epoch_mut(epoch) - .ok_or(SecretTreeError::TooDistantInThePast) - } else { - Ok(self.message_secrets_store.message_secrets_mut()) - } - } - - /// Get the message secrets. Either from the secrets store or from the group. - pub(crate) fn message_secrets_for_epoch( - &self, - epoch: GroupEpoch, - ) -> Result<&MessageSecrets, SecretTreeError> { - if epoch < self.context().epoch() { - self.message_secrets_store - .secrets_for_epoch(epoch) - .ok_or(SecretTreeError::TooDistantInThePast) - } else { - Ok(self.message_secrets_store.message_secrets()) - } - } - - /// Get the message secrets and leaves for the given epoch. Either from the - /// secrets store or from the group. - /// - /// Note that the leaves vector is empty for message secrets of the current - /// epoch. The caller can use treesync in this case. - pub(crate) fn message_secrets_and_leaves_mut( - &mut self, - epoch: GroupEpoch, - ) -> Result<(&mut MessageSecrets, &[Member]), MessageDecryptionError> { - if epoch < self.context().epoch() { - self.message_secrets_store - .secrets_and_leaves_for_epoch_mut(epoch) - .ok_or({ - MessageDecryptionError::SecretTreeError(SecretTreeError::TooDistantInThePast) - }) - } else { - // No need for leaves here. The tree of the current epoch is - // available to the caller. - Ok((self.message_secrets_store.message_secrets_mut(), &[])) - } - } - - pub(crate) fn own_leaf_node(&self) -> Result<&LeafNode, LibraryError> { - self.public_group() - .leaf(self.own_leaf_index()) - .ok_or_else(|| LibraryError::custom("Tree has no own leaf.")) - } - - /// Stores the [`CoreGroup`]. Called from methods creating a new group and mutating an - /// existing group, both inside [`CoreGroup`] and in [`MlsGroup`]. - pub(super) fn store( - &self, - storage: &Storage, - ) -> Result<(), Storage::Error> { - let group_id = self.group_id(); - - self.public_group.store(storage)?; - storage.write_own_leaf_index(group_id, &self.own_leaf_index())?; - storage.write_group_epoch_secrets(group_id, &self.group_epoch_secrets)?; - storage.write_message_secrets(group_id, &self.message_secrets_store)?; - storage.write_resumption_psk_store(group_id, &self.resumption_psk_store)?; - - Ok(()) - } - - /// Loads a [`CoreGroup`]. Called in [`MlsGroup::load`]. - pub(super) fn load( - storage: &Storage, - group_id: &GroupId, - use_ratchet_tree_extension: Option, - ) -> Result, Storage::Error> { - let public_group = PublicGroup::load(storage, group_id)?; - let group_epoch_secrets = storage.group_epoch_secrets(group_id)?; - let own_leaf_index = storage.own_leaf_index(group_id)?; - let message_secrets_store = storage.message_secrets(group_id)?; - let resumption_psk_store = storage.resumption_psk_store(group_id)?; - - let build = || -> Option { - Some(Self { - public_group: public_group?, - group_epoch_secrets: group_epoch_secrets?, - own_leaf_index: own_leaf_index?, - use_ratchet_tree_extension: use_ratchet_tree_extension?, - message_secrets_store: message_secrets_store?, - resumption_psk_store: resumption_psk_store?, - }) - }; - - Ok(build()) - } - - pub(super) fn delete( - &self, - storage: &Storage, - ) -> Result<(), Storage::Error> { - PublicGroup::delete(self.group_id(), storage)?; - storage.delete_own_leaf_index(self.group_id())?; - storage.delete_group_epoch_secrets(self.group_id())?; - storage.delete_message_secrets(self.group_id())?; - storage.delete_all_resumption_psk_secrets(self.group_id())?; - - Ok(()) - } - - /// Store the given [`EncryptionKeyPair`]s in the `provider`'s key store - /// indexed by this group's [`GroupId`] and [`GroupEpoch`]. - /// - /// Returns an error if access to the key store fails. - pub(super) fn store_epoch_keypairs( - &self, - store: &Storage, - keypair_references: &[EncryptionKeyPair], - ) -> Result<(), Storage::Error> { - store.write_encryption_epoch_key_pairs( - self.group_id(), - &self.context().epoch(), - self.own_leaf_index().u32(), - keypair_references, - ) - } - - /// Read the [`EncryptionKeyPair`]s of this group and its current - /// [`GroupEpoch`] from the `provider`'s storage. - /// - /// Returns an empty vector if access to the store fails or it can't find - /// any keys. - pub(super) fn read_epoch_keypairs( - &self, - store: &Storage, - ) -> Vec { - store - .encryption_epoch_key_pairs( - self.group_id(), - &self.context().epoch(), - self.own_leaf_index().u32(), - ) - .unwrap_or_default() - } - - /// Delete the [`EncryptionKeyPair`]s from the previous [`GroupEpoch`] from - /// the `provider`'s key store. - /// - /// Returns an error if access to the key store fails. - pub(super) fn delete_previous_epoch_keypairs( - &self, - store: &Storage, - ) -> Result<(), Storage::Error> { - store.delete_encryption_epoch_key_pairs( - self.group_id(), - &GroupEpoch::from(self.context().epoch().as_u64() - 1), - self.own_leaf_index().u32(), - ) - } - - pub(crate) fn create_commit( - &self, - params: CreateCommitParams, - provider: &Provider, - signer: &impl Signer, - ) -> Result> { - let ciphersuite = self.ciphersuite(); - - let sender = match params.commit_type() { - CommitType::External(_) => Sender::NewMemberCommit, - CommitType::Member => Sender::build_member(self.own_leaf_index()), - }; - - // Filter proposals - let (proposal_queue, contains_own_updates) = ProposalQueue::filter_proposals( - ciphersuite, - provider.crypto(), - sender.clone(), - self.proposal_store(), - params.inline_proposals(), - self.own_leaf_index(), - ) - .map_err(|e| match e { - ProposalQueueError::LibraryError(e) => e.into(), - ProposalQueueError::ProposalNotFound => CreateCommitError::MissingProposal, - ProposalQueueError::UpdateFromExternalSender => { - CreateCommitError::WrongProposalSenderType - } - })?; - - // TODO: #581 Filter proposals by support - // 11.2: - // Proposals with a non-default proposal type MUST NOT be included in a commit - // unless the proposal type is supported by all the members of the group that - // will process the Commit (i.e., not including any members being added - // or removed by the Commit). - - let proposal_reference_list = proposal_queue.commit_list(); - - // Validate the proposals by doing the following checks: - - // ValSem113: All Proposals: The proposal type must be supported by all - // members of the group - self.public_group - .validate_proposal_type_support(&proposal_queue)?; - // ValSem101 - // ValSem102 - // ValSem103 - // ValSem104 - self.public_group - .validate_key_uniqueness(&proposal_queue, None)?; - // ValSem105 - self.public_group.validate_add_proposals(&proposal_queue)?; - // ValSem106 - // ValSem109 - self.public_group.validate_capabilities(&proposal_queue)?; - // ValSem107 - // ValSem108 - self.public_group - .validate_remove_proposals(&proposal_queue)?; - self.public_group - .validate_pre_shared_key_proposals(&proposal_queue)?; - // Validate update proposals for member commits - if let Sender::Member(sender_index) = &sender { - // ValSem110 - // ValSem111 - // ValSem112 - self.public_group - .validate_update_proposals(&proposal_queue, *sender_index)?; - } - - // ValSem208 - // ValSem209 - self.public_group - .validate_group_context_extensions_proposal(&proposal_queue)?; - - // Make a copy of the public group to apply proposals safely - let mut diff = self.public_group.empty_diff(); - - // Apply proposals to tree - let apply_proposals_values = - diff.apply_proposals(&proposal_queue, self.own_leaf_index())?; - if apply_proposals_values.self_removed && params.commit_type() == &CommitType::Member { - return Err(CreateCommitError::CannotRemoveSelf); - } - - let path_computation_result = - // If path is needed, compute path values - if apply_proposals_values.path_required - || contains_own_updates - || params.force_self_update() - || !params.leaf_node_parameters().is_empty() - { - // Process the path. This includes updating the provisional - // group context by updating the epoch and computing the new - // tree hash. - diff.compute_path( - provider, - self.own_leaf_index(), - apply_proposals_values.exclusion_list(), - params.commit_type(), - params.leaf_node_parameters(), - signer, - apply_proposals_values.extensions.clone() - )? - } else { - // If path is not needed, update the group context and return - // empty path processing results - diff.update_group_context(provider.crypto(), apply_proposals_values.extensions.clone())?; - PathComputationResult::default() - }; - - let update_path_leaf_node = path_computation_result - .encrypted_path - .as_ref() - .map(|path| path.leaf_node().clone()); - - // Create commit message - let commit = Commit { - proposals: proposal_reference_list, - path: path_computation_result.encrypted_path, - }; - - // Build AuthenticatedContent - let mut authenticated_content = AuthenticatedContent::commit( - *params.framing_parameters(), - sender, - commit, - self.public_group.group_context(), - signer, - )?; - - // Update the confirmed transcript hash using the commit we just created. - diff.update_confirmed_transcript_hash(provider.crypto(), &authenticated_content)?; - - let serialized_provisional_group_context = diff - .group_context() - .tls_serialize_detached() - .map_err(LibraryError::missing_bound_check)?; - - let joiner_secret = JoinerSecret::new( - provider.crypto(), - ciphersuite, - path_computation_result.commit_secret, - self.group_epoch_secrets().init_secret(), - &serialized_provisional_group_context, - ) - .map_err(LibraryError::unexpected_crypto_error)?; - - // Prepare the PskSecret - let psk_secret = { - let psks = load_psks( - provider.storage(), - &self.resumption_psk_store, - &apply_proposals_values.presharedkeys, - )?; - - PskSecret::new(provider.crypto(), ciphersuite, psks)? - }; - - // Create key schedule - let mut key_schedule = - KeySchedule::init(ciphersuite, provider.crypto(), &joiner_secret, psk_secret)?; - - let serialized_provisional_group_context = diff - .group_context() - .tls_serialize_detached() - .map_err(LibraryError::missing_bound_check)?; - - let welcome_secret = key_schedule - .welcome(provider.crypto(), self.ciphersuite()) - .map_err(|_| LibraryError::custom("Using the key schedule in the wrong state"))?; - key_schedule - .add_context(provider.crypto(), &serialized_provisional_group_context) - .map_err(|_| LibraryError::custom("Using the key schedule in the wrong state"))?; - let provisional_epoch_secrets = key_schedule - .epoch_secrets(provider.crypto(), self.ciphersuite()) - .map_err(|_| LibraryError::custom("Using the key schedule in the wrong state"))?; - - // Calculate the confirmation tag - let confirmation_tag = provisional_epoch_secrets - .confirmation_key() - .tag( - provider.crypto(), - self.ciphersuite(), - diff.group_context().confirmed_transcript_hash(), - ) - .map_err(LibraryError::unexpected_crypto_error)?; - - // Set the confirmation tag - authenticated_content.set_confirmation_tag(confirmation_tag.clone()); - - diff.update_interim_transcript_hash( - ciphersuite, - provider.crypto(), - confirmation_tag.clone(), - )?; - - // only computes the group info if necessary - let group_info = if !apply_proposals_values.invitation_list.is_empty() - || self.use_ratchet_tree_extension - { - // Create the ratchet tree extension if necessary - let external_pub = provisional_epoch_secrets - .external_secret() - .derive_external_keypair(provider.crypto(), ciphersuite) - .map_err(LibraryError::unexpected_crypto_error)? - .public; - let external_pub_extension = - Extension::ExternalPub(ExternalPubExtension::new(external_pub.into())); - let other_extensions: Extensions = if self.use_ratchet_tree_extension { - Extensions::from_vec(vec![ - Extension::RatchetTree(RatchetTreeExtension::new(diff.export_ratchet_tree())), - external_pub_extension, - ])? - } else { - Extensions::single(external_pub_extension) - }; - - // Create to-be-signed group info. - let group_info_tbs = { - GroupInfoTBS::new( - diff.group_context().clone(), - other_extensions, - confirmation_tag, - self.own_leaf_index(), - ) - }; - // Sign to-be-signed group info. - Some(group_info_tbs.sign(signer)?) - } else { - None - }; - - // Check if new members were added and, if so, create welcome messages - let welcome_option = if !apply_proposals_values.invitation_list.is_empty() { - // Encrypt GroupInfo object - let (welcome_key, welcome_nonce) = welcome_secret - .derive_welcome_key_nonce(provider.crypto(), self.ciphersuite()) - .map_err(LibraryError::unexpected_crypto_error)?; - let encrypted_group_info = welcome_key - .aead_seal( - provider.crypto(), - group_info - .as_ref() - .ok_or_else(|| LibraryError::custom("GroupInfo was not computed"))? - .tls_serialize_detached() - .map_err(LibraryError::missing_bound_check)? - .as_slice(), - &[], - &welcome_nonce, - ) - .map_err(LibraryError::unexpected_crypto_error)?; - - // Create group secrets for later use, so we can afterwards consume the - // `joiner_secret`. - let encrypted_secrets = diff.encrypt_group_secrets( - &joiner_secret, - apply_proposals_values.invitation_list, - path_computation_result.plain_path.as_deref(), - &apply_proposals_values.presharedkeys, - &encrypted_group_info, - provider.crypto(), - self.own_leaf_index(), - )?; - - // Create welcome message - let welcome = Welcome::new(self.ciphersuite(), encrypted_secrets, encrypted_group_info); - Some(welcome) - } else { - None - }; - - let (provisional_group_epoch_secrets, provisional_message_secrets) = - provisional_epoch_secrets.split_secrets( - serialized_provisional_group_context, - diff.tree_size(), - self.own_leaf_index(), - ); - - let staged_commit_state = MemberStagedCommitState::new( - provisional_group_epoch_secrets, - provisional_message_secrets, - diff.into_staged_diff(provider.crypto(), ciphersuite)?, - path_computation_result.new_keypairs, - // The committer is not allowed to include their own update - // proposal, so there is no extra keypair to store here. - None, - update_path_leaf_node, - ); - let staged_commit = StagedCommit::new( - proposal_queue, - StagedCommitState::GroupMember(Box::new(staged_commit_state)), - ); - - Ok(CreateCommitResult { - commit: authenticated_content, - welcome_option, - staged_commit, - group_info: group_info.filter(|_| self.use_ratchet_tree_extension), - }) - } - - /// Create a new group context extension proposal - pub(crate) fn create_group_context_ext_proposal( - &self, - framing_parameters: FramingParameters, - extensions: Extensions, - signer: &impl Signer, - ) -> Result> - { - // Ensure that the group supports all the extensions that are wanted. - let required_extension = extensions - .iter() - .find(|extension| extension.extension_type() == ExtensionType::RequiredCapabilities); - if let Some(required_extension) = required_extension { - let required_capabilities = required_extension.as_required_capabilities_extension()?; - // Ensure we support all the capabilities. - self.own_leaf_node()? - .capabilities() - .supports_required_capabilities(required_capabilities)?; - - // Ensure that all other leaf nodes support all the required - // extensions as well. - self.public_group() - .check_extension_support(required_capabilities.extension_types())?; - } - let proposal = GroupContextExtensionProposal::new(extensions); - let proposal = Proposal::GroupContextExtensions(proposal); - AuthenticatedContent::member_proposal( - framing_parameters, - self.own_leaf_index(), - proposal, - self.context(), - signer, - ) - .map_err(|e| e.into()) - } -} - -// Test functions -#[cfg(test)] -impl CoreGroup { - pub(crate) fn set_own_leaf_index(&mut self, own_leaf_index: LeafNodeIndex) { - self.own_leaf_index = own_leaf_index; - } - - pub(crate) fn own_tree_position(&self) -> TreePosition { - TreePosition::new(self.group_id().clone(), self.own_leaf_index()) - } - - pub(crate) fn message_secrets_store(&self) -> &MessageSecretsStore { - &self.message_secrets_store - } - - pub(crate) fn set_group_context(&mut self, group_context: GroupContext) { - self.public_group.set_group_context(group_context) - } -} - -// Test and test-utils functions -#[cfg_attr(all(not(test), feature = "test-utils"), allow(dead_code))] -#[cfg(any(feature = "test-utils", test))] -impl CoreGroup { - pub(crate) fn context_mut(&mut self) -> &mut GroupContext { - self.public_group.context_mut() - } - - pub(crate) fn message_secrets_test_mut(&mut self) -> &mut MessageSecrets { - self.message_secrets_store.message_secrets_mut() - } - - pub(crate) fn print_ratchet_tree(&self, message: &str) { - println!("{}: {}", message, self.public_group().export_ratchet_tree()); - } - - pub(crate) fn resumption_psk_store(&self) -> &ResumptionPskStore { - &self.resumption_psk_store - } -} - -/// Configuration for core group. -#[derive(Clone, Copy, Default, Debug)] -pub(crate) struct CoreGroupConfig { - /// Flag whether to send the ratchet tree along with the `GroupInfo` or not. - /// Defaults to false. - pub(crate) add_ratchet_tree_extension: bool, -} diff --git a/openmls/src/group/core_group/new_from_external_init.rs b/openmls/src/group/core_group/new_from_external_init.rs deleted file mode 100644 index e8c387416..000000000 --- a/openmls/src/group/core_group/new_from_external_init.rs +++ /dev/null @@ -1,144 +0,0 @@ -use crate::{ - binary_tree::array_representation::LeafNodeIndex, - group::{ - core_group::create_commit_params::{CommitType, CreateCommitParams}, - errors::ExternalCommitError, - }, - messages::proposals::{ExternalInitProposal, Proposal}, - storage::OpenMlsProvider, -}; - -use super::CoreGroup; -use crate::group::core_group::*; - -pub(crate) type ExternalCommitResult = (CoreGroup, CreateCommitResult); - -impl CoreGroup { - /// Join a group without the help of an internal member. This function - /// requires a [GroupInfo], as well as the corresponding public tree - /// `nodes`. After the group state is initialized, this function creates an - /// `ExternalInit` proposal and commits it along with the given proposals by - /// reference and by value. - /// - /// Returns the new `CoreGroup` object, as well as the `PublicMessage` - /// containing the commit. - /// - /// Note: If there is a group member in the group with the same identity as us, - /// this will create a remove proposal. - pub(crate) fn join_by_external_commit( - provider: &Provider, - signer: &impl Signer, - mut params: CreateCommitParams, - ratchet_tree: Option, - verifiable_group_info: VerifiableGroupInfo, - ) -> Result> { - // Build the ratchet tree - - // Set nodes either from the extension or from the `nodes_option`. - // If we got a ratchet tree extension in the welcome, we enable it for - // this group. Note that this is not strictly necessary. But there's - // currently no other mechanism to enable the extension. - let (ratchet_tree, enable_ratchet_tree_extension) = - match verifiable_group_info.extensions().ratchet_tree() { - Some(extension) => (extension.ratchet_tree().clone(), true), - None => match ratchet_tree { - Some(ratchet_tree) => (ratchet_tree, false), - None => return Err(ExternalCommitError::MissingRatchetTree), - }, - }; - - let (public_group, group_info) = PublicGroup::from_external( - provider.crypto(), - provider.storage(), - ratchet_tree, - verifiable_group_info, - // Existing proposals are discarded when joining by external commit. - ProposalStore::new(), - )?; - let group_context = public_group.group_context(); - - // Obtain external_pub from GroupInfo extensions. - let external_pub = group_info - .extensions() - .external_pub() - .ok_or(ExternalCommitError::MissingExternalPub)? - .external_pub(); - - let (init_secret, kem_output) = InitSecret::from_group_context( - provider.crypto(), - group_context, - external_pub.as_slice(), - ) - .map_err(|_| ExternalCommitError::UnsupportedCiphersuite)?; - - // The `EpochSecrets` we create here are essentially zero, with the - // exception of the `InitSecret`, which is all we need here for the - // external commit. - let epoch_secrets = EpochSecrets::with_init_secret( - provider.crypto(), - group_info.group_context().ciphersuite(), - init_secret, - ) - .map_err(LibraryError::unexpected_crypto_error)?; - let (group_epoch_secrets, message_secrets) = epoch_secrets.split_secrets( - group_context - .tls_serialize_detached() - .map_err(LibraryError::missing_bound_check)?, - public_group.tree_size(), - // We use a fake own index of 0 here, as we're not going to use the - // tree for encryption until after the first commit. This issue is - // tracked in #767. - LeafNodeIndex::new(0u32), - ); - let message_secrets_store = MessageSecretsStore::new_with_secret(0, message_secrets); - - let external_init_proposal = Proposal::ExternalInit(ExternalInitProposal::from(kem_output)); - - let mut inline_proposals = vec![external_init_proposal]; - - // If there is a group member in the group with the same identity as us, - // commit a remove proposal. - let signature_key = match params.commit_type() { - CommitType::External(credential_with_key) => { - credential_with_key.signature_key.as_slice() - } - _ => return Err(ExternalCommitError::MissingCredential), - }; - if let Some(us) = public_group - .members() - .find(|member| member.signature_key == signature_key) - { - let remove_proposal = Proposal::Remove(RemoveProposal { removed: us.index }); - inline_proposals.push(remove_proposal); - }; - - let own_leaf_index = public_group.leftmost_free_index(inline_proposals.iter().map(Some))?; - params.set_inline_proposals(inline_proposals); - - let group = CoreGroup { - public_group, - use_ratchet_tree_extension: enable_ratchet_tree_extension, - group_epoch_secrets, - message_secrets_store, - own_leaf_index, - // TODO(#1357) - resumption_psk_store: ResumptionPskStore::new(32), - }; - - // Immediately create the commit to add ourselves to the group. - let create_commit_result = group.create_commit(params, provider, signer); - debug_assert!( - create_commit_result.is_ok(), - "Error creating commit {create_commit_result:?}" - ); - - group - .store(provider.storage()) - .map_err(ExternalCommitError::StorageError)?; - - Ok(( - group, - create_commit_result.map_err(|_| ExternalCommitError::CommitError)?, - )) - } -} diff --git a/openmls/src/group/core_group/new_from_welcome.rs b/openmls/src/group/core_group/new_from_welcome.rs deleted file mode 100644 index ae653b6a0..000000000 --- a/openmls/src/group/core_group/new_from_welcome.rs +++ /dev/null @@ -1,309 +0,0 @@ -use log::debug; - -use crate::{ - group::{core_group::*, errors::WelcomeError}, - schedule::psk::store::ResumptionPskStore, - storage::OpenMlsProvider, - treesync::errors::{DerivePathError, PublicTreeError}, -}; - -impl StagedCoreWelcome { - /// Create a staged join from a welcome message. The purpose of this type is to be able to - /// extract information, such as the identify of who created the welcome, before joining the - /// group. - /// Note: calling this function will consume the key material for decrypting the [`Welcome`] - /// message, even if the caller does not turn the [`StagedCoreWelcome`] into a [`CoreGroup`]. - pub fn new_from_welcome( - welcome: Welcome, - ratchet_tree: Option, - key_package_bundle: KeyPackageBundle, - provider: &Provider, - resumption_psk_store: ResumptionPskStore, - ) -> Result> { - log::debug!("CoreGroup::new_from_welcome_internal"); - let (ciphersuite, group_secrets, key_schedule, verifiable_group_info) = process_welcome( - welcome, - &key_package_bundle, - provider, - &resumption_psk_store, - )?; - - build_staged_welcome( - verifiable_group_info, - ratchet_tree, - provider, - key_package_bundle, - key_schedule, - ciphersuite, - resumption_psk_store, - group_secrets, - ) - } - - /// Returns the [`LeafNodeIndex`] of the group member that authored the [`Welcome`] message. - pub fn welcome_sender_index(&self) -> LeafNodeIndex { - self.verifiable_group_info.signer() - } - - /// Returns the [`LeafNode`] of the group member that authored the [`Welcome`] message. - pub fn welcome_sender(&self) -> Result<&LeafNode, LibraryError> { - let sender_index = self.welcome_sender_index(); - self.public_group - .leaf(sender_index) - .ok_or(LibraryError::custom( - "no leaf with given welcome sender index exists", - )) - } - - /// Consumes the [`StagedCoreWelcome`] and returns the respective [`CoreGroup`]. - pub fn into_core_group( - self, - provider: &Provider, - ) -> Result> { - // If we got a path secret, derive the path (which also checks if the - // public keys match) and store the derived keys in the key store. - let group_keypairs = if let Some(path_keypairs) = self.path_keypairs { - let mut keypairs = vec![self.key_package_bundle.encryption_key_pair()]; - keypairs.extend_from_slice(&path_keypairs); - keypairs - } else { - vec![self.key_package_bundle.encryption_key_pair()] - }; - - let group = CoreGroup { - public_group: self.public_group, - group_epoch_secrets: self.group_epoch_secrets, - own_leaf_index: self.own_leaf_index, - use_ratchet_tree_extension: self.use_ratchet_tree_extension, - message_secrets_store: self.message_secrets_store, - resumption_psk_store: self.resumption_psk_store, - }; - - group - .store(provider.storage()) - .map_err(WelcomeError::StorageError)?; - group - .store_epoch_keypairs(provider.storage(), group_keypairs.as_slice()) - .map_err(WelcomeError::StorageError)?; - - Ok(group) - } -} - -#[allow(clippy::too_many_arguments)] -pub(in crate::group) fn build_staged_welcome( - verifiable_group_info: VerifiableGroupInfo, - ratchet_tree: Option, - provider: &Provider, - key_package_bundle: KeyPackageBundle, - mut key_schedule: KeySchedule, - ciphersuite: Ciphersuite, - mut resumption_psk_store: ResumptionPskStore, - group_secrets: GroupSecrets, -) -> Result> { - // Build the ratchet tree and group - - // Set nodes either from the extension or from the `nodes_option`. - // If we got a ratchet tree extension in the welcome, we enable it for - // this group. Note that this is not strictly necessary. But there's - // currently no other mechanism to enable the extension. - let (ratchet_tree, enable_ratchet_tree_extension) = - match verifiable_group_info.extensions().ratchet_tree() { - Some(extension) => (extension.ratchet_tree().clone(), true), - None => match ratchet_tree { - Some(ratchet_tree) => (ratchet_tree, false), - None => return Err(WelcomeError::MissingRatchetTree), - }, - }; - - // Since there is currently only the external pub extension, there is no - // group info extension of interest here. - let (public_group, _group_info_extensions) = PublicGroup::from_external( - provider.crypto(), - provider.storage(), - ratchet_tree, - verifiable_group_info.clone(), - ProposalStore::new(), - )?; - - // Find our own leaf in the tree. - let own_leaf_index = public_group - .members() - .find_map(|m| { - if m.signature_key - == key_package_bundle - .key_package() - .leaf_node() - .signature_key() - .as_slice() - { - Some(m.index) - } else { - None - } - }) - .ok_or(WelcomeError::PublicTreeError( - PublicTreeError::MalformedTree, - ))?; - - let (group_epoch_secrets, message_secrets) = { - let serialized_group_context = public_group - .group_context() - .tls_serialize_detached() - .map_err(LibraryError::missing_bound_check)?; - - // TODO #751: Implement PSK - key_schedule - .add_context(provider.crypto(), &serialized_group_context) - .map_err(|_| LibraryError::custom("Using the key schedule in the wrong state"))?; - - let epoch_secrets = key_schedule - .epoch_secrets(provider.crypto(), ciphersuite) - .map_err(|_| LibraryError::custom("Using the key schedule in the wrong state"))?; - - epoch_secrets.split_secrets( - serialized_group_context, - public_group.tree_size(), - own_leaf_index, - ) - }; - - let confirmation_tag = message_secrets - .confirmation_key() - .tag( - provider.crypto(), - ciphersuite, - public_group.group_context().confirmed_transcript_hash(), - ) - .map_err(LibraryError::unexpected_crypto_error)?; - - // Verify confirmation tag - if &confirmation_tag != public_group.confirmation_tag() { - log::error!("Confirmation tag mismatch"); - log_crypto!(trace, " Got: {:x?}", confirmation_tag); - log_crypto!(trace, " Expected: {:x?}", public_group.confirmation_tag()); - debug_assert!(false, "Confirmation tag mismatch"); - - // in some tests we need to be able to proceed despite the tag being wrong, - // e.g. to test whether a later validation check is performed correctly. - if !crate::skip_validation::is_disabled::confirmation_tag() { - return Err(WelcomeError::ConfirmationTagMismatch); - } - } - - let message_secrets_store = MessageSecretsStore::new_with_secret(0, message_secrets); - - // Extract and store the resumption PSK for the current epoch. - let resumption_psk = group_epoch_secrets.resumption_psk(); - resumption_psk_store.add(public_group.group_context().epoch(), resumption_psk.clone()); - - let welcome_sender_index = verifiable_group_info.signer(); - let path_keypairs = if let Some(path_secret) = group_secrets.path_secret { - let (path_keypairs, _commit_secret) = public_group - .derive_path_secrets( - provider.crypto(), - ciphersuite, - path_secret, - welcome_sender_index, - own_leaf_index, - ) - .map_err(|e| match e { - DerivePathError::LibraryError(e) => e.into(), - DerivePathError::PublicKeyMismatch => { - WelcomeError::PublicTreeError(PublicTreeError::PublicKeyMismatch) - } - })?; - Some(path_keypairs) - } else { - None - }; - - let group = StagedCoreWelcome { - public_group, - group_epoch_secrets, - own_leaf_index, - use_ratchet_tree_extension: enable_ratchet_tree_extension, - message_secrets_store, - resumption_psk_store, - verifiable_group_info, - key_package_bundle, - path_keypairs, - }; - - Ok(group) -} - -/// Process a Welcome message up to the point where the ratchet tree is required. -pub(in crate::group) fn process_welcome( - welcome: Welcome, - key_package_bundle: &KeyPackageBundle, - provider: &Provider, - resumption_psk_store: &ResumptionPskStore, -) -> Result< - (Ciphersuite, GroupSecrets, KeySchedule, VerifiableGroupInfo), - WelcomeError, -> { - let ciphersuite = welcome.ciphersuite(); - let Some(egs) = welcome.find_encrypted_group_secret( - key_package_bundle - .key_package() - .hash_ref(provider.crypto())?, - ) else { - return Err(WelcomeError::JoinerSecretNotFound); - }; - if ciphersuite != key_package_bundle.key_package().ciphersuite() { - let e = WelcomeError::CiphersuiteMismatch; - debug!("new_from_welcome {:?}", e); - return Err(e); - } - let group_secrets = GroupSecrets::try_from_ciphertext( - key_package_bundle.init_private_key(), - egs.encrypted_group_secrets(), - welcome.encrypted_group_info(), - ciphersuite, - provider.crypto(), - )?; - let psk_secret = { - let psks = load_psks( - provider.storage(), - resumption_psk_store, - &group_secrets.psks, - )?; - - PskSecret::new(provider.crypto(), ciphersuite, psks)? - }; - let key_schedule = KeySchedule::init( - ciphersuite, - provider.crypto(), - &group_secrets.joiner_secret, - psk_secret, - )?; - let (welcome_key, welcome_nonce) = key_schedule - .welcome(provider.crypto(), ciphersuite) - .map_err(|_| LibraryError::custom("Using the key schedule in the wrong state"))? - .derive_welcome_key_nonce(provider.crypto(), ciphersuite) - .map_err(LibraryError::unexpected_crypto_error)?; - let verifiable_group_info = VerifiableGroupInfo::try_from_ciphertext( - &welcome_key, - &welcome_nonce, - welcome.encrypted_group_info(), - &[], - provider.crypto(), - )?; - if let Some(required_capabilities) = verifiable_group_info.extensions().required_capabilities() - { - // Also check that our key package actually supports the extensions. - // Per spec the sender must have checked this. But you never know. - key_package_bundle - .key_package() - .leaf_node() - .capabilities() - .supports_required_capabilities(required_capabilities)?; - } - Ok(( - ciphersuite, - group_secrets, - key_schedule, - verifiable_group_info, - )) -} diff --git a/openmls/src/group/core_group/process.rs b/openmls/src/group/core_group/process.rs deleted file mode 100644 index 1fb9c23c1..000000000 --- a/openmls/src/group/core_group/process.rs +++ /dev/null @@ -1,287 +0,0 @@ -use core_group::proposals::QueuedProposal; - -use crate::{ - framing::mls_content::FramedContentBody, - group::{ - errors::{StageCommitError, ValidationError}, - mls_group::errors::ProcessMessageError, - }, -}; - -use super::*; - -impl CoreGroup { - /// This processing function does most of the semantic verifications. - /// It returns a [ProcessedMessage] enum. - /// Checks the following semantic validation: - /// - ValSem008 - /// - ValSem010 - /// - ValSem101 - /// - ValSem102 - /// - ValSem104 - /// - ValSem106 - /// - ValSem107 - /// - ValSem108 - /// - ValSem110 - /// - ValSem111 - /// - ValSem112 - /// - ValSem113: All Proposals: The proposal type must be supported by all - /// members of the group - /// - ValSem200 - /// - ValSem201 - /// - ValSem202: Path must be the right length - /// - ValSem203: Path secrets must decrypt correctly - /// - ValSem204: Public keys from Path must be verified and match the - /// private keys from the direct path - /// - ValSem205 - /// - ValSem240 - /// - ValSem241 - /// - ValSem242 - /// - ValSem244 - /// - ValSem246 (as part of ValSem010) - pub(crate) fn process_unverified_message( - &self, - provider: &Provider, - unverified_message: UnverifiedMessage, - old_epoch_keypairs: Vec, - leaf_node_keypairs: Vec, - ) -> Result { - // Checks the following semantic validation: - // - ValSem010 - // - ValSem246 (as part of ValSem010) - let (content, credential) = - unverified_message.verify(self.ciphersuite(), provider.crypto(), self.version())?; - - match content.sender() { - Sender::Member(_) | Sender::NewMemberCommit | Sender::NewMemberProposal => { - let sender = content.sender().clone(); - let authenticated_data = content.authenticated_data().to_owned(); - - let content = match content.content() { - FramedContentBody::Application(application_message) => { - ProcessedMessageContent::ApplicationMessage(ApplicationMessage::new( - application_message.as_slice().to_owned(), - )) - } - FramedContentBody::Proposal(_) => { - let proposal = Box::new(QueuedProposal::from_authenticated_content_by_ref( - self.ciphersuite(), - provider.crypto(), - content, - )?); - - if matches!(sender, Sender::NewMemberProposal) { - ProcessedMessageContent::ExternalJoinProposalMessage(proposal) - } else { - ProcessedMessageContent::ProposalMessage(proposal) - } - } - FramedContentBody::Commit(_) => { - let staged_commit = self.stage_commit( - &content, - old_epoch_keypairs, - leaf_node_keypairs, - provider, - )?; - ProcessedMessageContent::StagedCommitMessage(Box::new(staged_commit)) - } - }; - - Ok(ProcessedMessage::new( - self.group_id().clone(), - self.context().epoch(), - sender, - authenticated_data, - content, - credential, - )) - } - Sender::External(_) => { - let sender = content.sender().clone(); - let data = content.authenticated_data().to_owned(); - match content.content() { - FramedContentBody::Application(_) => { - Err(ProcessMessageError::UnauthorizedExternalApplicationMessage) - } - FramedContentBody::Proposal(Proposal::Remove(_)) => { - let content = ProcessedMessageContent::ProposalMessage(Box::new( - QueuedProposal::from_authenticated_content_by_ref( - self.ciphersuite(), - provider.crypto(), - content, - )?, - )); - Ok(ProcessedMessage::new( - self.group_id().clone(), - self.context().epoch(), - sender, - data, - content, - credential, - )) - } - // TODO #151/#106 - FramedContentBody::Proposal(_) => { - Err(ProcessMessageError::UnsupportedProposalType) - } - FramedContentBody::Commit(_) => unimplemented!(), - } - } - } - } - - /// This function is used to parse messages from the DS. It checks for - /// syntactic errors and does semantic validation as well. If the input is a - /// [PrivateMessage] message, it will be decrypted. It returns a - /// [ProcessedMessage] enum. Checks the following semantic validation: - /// - ValSem002 - /// - ValSem003 - /// - ValSem004 - /// - ValSem005 - /// - ValSem006 - /// - ValSem007 - /// - ValSem008 - /// - ValSem009 - /// - ValSem010 - /// - ValSem101 - /// - ValSem102 - /// - ValSem104 - /// - ValSem106 - /// - ValSem107 - /// - ValSem108 - /// - ValSem110 - /// - ValSem111 - /// - ValSem112 - /// - ValSem113: All Proposals: The proposal type must be supported by all - /// members of the group - /// - ValSem200 - /// - ValSem201 - /// - ValSem202: Path must be the right length - /// - ValSem203: Path secrets must decrypt correctly - /// - ValSem204: Public keys from Path must be verified and match the - /// private keys from the direct path - /// - ValSem205 - /// - ValSem240 - /// - ValSem241 - /// - ValSem242 - /// - ValSem244 - /// - ValSem245 - /// - ValSem246 (as part of ValSem010) - pub(crate) fn process_message( - &mut self, - provider: &Provider, - message: impl Into, - sender_ratchet_configuration: &SenderRatchetConfiguration, - own_leaf_nodes: &[LeafNode], - ) -> Result { - let message: ProtocolMessage = message.into(); - - // Checks the following semantic validation: - // - ValSem002 - // - ValSem003 - // - ValSem006 - // - ValSem007 MembershipTag presence - let decrypted_message = - self.decrypt_message(provider.crypto(), message, sender_ratchet_configuration)?; - - let unverified_message = self - .public_group - .parse_message(decrypted_message, &self.message_secrets_store) - .map_err(ProcessMessageError::from)?; - - // If this is a commit, we need to load the private key material we need for decryption. - let (old_epoch_keypairs, leaf_node_keypairs) = - if let ContentType::Commit = unverified_message.content_type() { - self.read_decryption_keypairs(provider, own_leaf_nodes)? - } else { - (vec![], vec![]) - }; - - self.process_unverified_message( - provider, - unverified_message, - old_epoch_keypairs, - leaf_node_keypairs, - ) - } - - /// Performs framing validation and, if necessary, decrypts the given message. - /// - /// Returns the [`DecryptedMessage`] if processing is successful, or a - /// [`ValidationError`] if it is not. - /// - /// Checks the following semantic validation: - /// - ValSem002 - /// - ValSem003 - /// - ValSem006 - /// - ValSem007 MembershipTag presence - pub(crate) fn decrypt_message( - &mut self, - crypto: &impl OpenMlsCrypto, - message: ProtocolMessage, - sender_ratchet_configuration: &SenderRatchetConfiguration, - ) -> Result { - // Checks the following semantic validation: - // - ValSem002 - // - ValSem003 - self.public_group.validate_framing(&message)?; - - let epoch = message.epoch(); - - // Checks the following semantic validation: - // - ValSem006 - // - ValSem007 MembershipTag presence - match message { - ProtocolMessage::PublicMessage(public_message) => { - // If the message is older than the current epoch, we need to fetch the correct secret tree first. - let message_secrets = - self.message_secrets_for_epoch(epoch).map_err(|e| match e { - SecretTreeError::TooDistantInThePast => ValidationError::NoPastEpochData, - _ => LibraryError::custom( - "Unexpected error while retrieving message secrets for epoch.", - ) - .into(), - })?; - DecryptedMessage::from_inbound_public_message( - public_message, - message_secrets, - message_secrets.serialized_context().to_vec(), - crypto, - self.ciphersuite(), - ) - } - ProtocolMessage::PrivateMessage(ciphertext) => { - // If the message is older than the current epoch, we need to fetch the correct secret tree first - DecryptedMessage::from_inbound_ciphertext( - ciphertext, - crypto, - self, - sender_ratchet_configuration, - ) - } - } - } - - /// Helper function to read decryption keypairs. - pub(super) fn read_decryption_keypairs( - &self, - provider: &impl OpenMlsProvider, - own_leaf_nodes: &[LeafNode], - ) -> Result<(Vec, Vec), StageCommitError> { - // All keys from the previous epoch are potential decryption keypairs. - let old_epoch_keypairs = self.read_epoch_keypairs(provider.storage()); - - // If we are processing an update proposal that originally came from - // us, the keypair corresponding to the leaf in the update is also a - // potential decryption keypair. - let leaf_node_keypairs = own_leaf_nodes - .iter() - .map(|leaf_node| { - EncryptionKeyPair::read(provider, leaf_node.encryption_key()) - .ok_or(StageCommitError::MissingDecryptionKey) - }) - .collect::, StageCommitError>>()?; - - Ok((old_epoch_keypairs, leaf_node_keypairs)) - } -} diff --git a/openmls/src/group/errors.rs b/openmls/src/group/errors.rs index 9d242d2ed..fca535281 100644 --- a/openmls/src/group/errors.rs +++ b/openmls/src/group/errors.rs @@ -5,7 +5,7 @@ use thiserror::Error; pub use super::mls_group::errors::*; -use super::public_group::errors::{CreationFromExternalError, PublicGroupBuildError}; +use super::public_group::errors::CreationFromExternalError; use crate::{ ciphersuite::signable::SignatureError, error::LibraryError, @@ -431,16 +431,6 @@ pub enum CreateAddProposalError { // === Crate errors === -/// Exporter error -#[derive(Error, Debug, PartialEq, Clone)] -pub(crate) enum ExporterError { - /// See [`LibraryError`] for more details. - #[error(transparent)] - LibraryError(#[from] LibraryError), - #[error("The requested key length is not supported (too large).")] - KeyLengthTooLong, -} - /// Proposal queue error #[derive(Error, Debug, PartialEq, Clone)] pub(crate) enum ProposalQueueError { @@ -470,36 +460,6 @@ pub(crate) enum FromCommittedProposalsError { SelfRemoval, } -// Core group build error -#[derive(Error, Debug, PartialEq, Clone)] -pub(crate) enum CoreGroupBuildError { - /// See [`LibraryError`] for more details. - #[error(transparent)] - LibraryError(#[from] LibraryError), - /// See [`PublicGroupBuildError`] for more details. - #[error(transparent)] - PublicGroupBuildError(#[from] PublicGroupBuildError), - /// See [`PskError`] for more details. - #[error(transparent)] - Psk(#[from] PskError), - /// Error storing leaf private key in key store. - #[error("Error saving data to storage: {0}.")] - StorageError(StorageError), -} - -// CoreGroup parse message error -#[derive(Error, Debug, PartialEq, Clone)] -pub(crate) enum CoreGroupParseMessageError { - /// See [`LibraryError`] for more details. - #[error(transparent)] - LibraryError(#[from] LibraryError), - //#[error(transparent)] - //FramingValidationError(#[from] FramingValidationError), - /// See [`ValidationError`] for more details. - #[error(transparent)] - ValidationError(#[from] ValidationError), -} - /// Create group context ext proposal error #[derive(Error, Debug, PartialEq, Clone)] pub enum CreateGroupContextExtProposalError { diff --git a/openmls/src/group/mls_group/application.rs b/openmls/src/group/mls_group/application.rs index 1811973a5..8e80f61db 100644 --- a/openmls/src/group/mls_group/application.rs +++ b/openmls/src/group/mls_group/application.rs @@ -30,22 +30,22 @@ impl MlsGroup { )); } + let authenticated_content = AuthenticatedContent::new_application( + self.own_leaf_index(), + &self.aad, + message, + self.context(), + signer, + )?; let ciphertext = self - .group - .create_application_message( - &self.aad, - message, - self.configuration().padding_size(), - provider, - signer, - ) + .encrypt(authenticated_content, provider) // We know the application message is wellformed and we have the key material of the current epoch .map_err(|_| LibraryError::custom("Malformed plaintext"))?; self.reset_aad(); Ok(MlsMessageOut::from_private_message( ciphertext, - self.group.version(), + self.version(), )) } } diff --git a/openmls/src/group/mls_group/builder.rs b/openmls/src/group/mls_group/builder.rs index 5644620de..0cc854da9 100644 --- a/openmls/src/group/mls_group/builder.rs +++ b/openmls/src/group/mls_group/builder.rs @@ -1,26 +1,34 @@ use openmls_traits::{signatures::Signer, types::Ciphersuite}; +use tls_codec::Serialize; use crate::{ + binary_tree::array_representation::TreeSize, credentials::CredentialWithKey, error::LibraryError, extensions::{errors::InvalidExtensionError, Extensions}, group::{ - public_group::errors::PublicGroupBuildError, CoreGroup, CoreGroupBuildError, - CoreGroupConfig, GroupId, MlsGroupCreateConfig, MlsGroupCreateConfigBuilder, NewGroupError, - WireFormatPolicy, + public_group::errors::PublicGroupBuildError, GroupId, MlsGroupCreateConfig, + MlsGroupCreateConfigBuilder, NewGroupError, PublicGroup, WireFormatPolicy, }, key_packages::Lifetime, + prelude::LeafNodeIndex, + schedule::{ + psk::{load_psks, store::ResumptionPskStore, PskSecret}, + InitSecret, JoinerSecret, KeySchedule, PreSharedKeyId, + }, storage::OpenMlsProvider, tree::sender_ratchet::SenderRatchetConfiguration, treesync::node::leaf_node::Capabilities, }; -use super::{MlsGroup, MlsGroupState}; +use super::{past_secrets::MessageSecretsStore, MlsGroup, MlsGroupState}; #[derive(Default, Debug)] pub struct MlsGroupBuilder { group_id: Option, mls_group_create_config_builder: MlsGroupCreateConfigBuilder, + max_past_epochs: Option, + psk_ids: Vec, } impl MlsGroupBuilder { @@ -61,67 +69,103 @@ impl MlsGroupBuilder { let group_id = self .group_id .unwrap_or_else(|| GroupId::random(provider.rand())); - // TODO #751 - let group_config = CoreGroupConfig { - add_ratchet_tree_extension: mls_group_create_config - .join_config - .use_ratchet_tree_extension, - }; + let ciphersuite = mls_group_create_config.ciphersuite; + + let (public_group_builder, commit_secret, leaf_keypair) = + PublicGroup::builder(group_id, ciphersuite, credential_with_key) + .with_group_context_extensions( + mls_group_create_config.group_context_extensions.clone(), + )? + .with_leaf_node_extensions(mls_group_create_config.leaf_node_extensions.clone())? + .with_lifetime(*mls_group_create_config.lifetime()) + .with_capabilities(mls_group_create_config.capabilities.clone()) + .get_secrets(provider, signer) + .map_err(|e| match e { + PublicGroupBuildError::LibraryError(e) => NewGroupError::LibraryError(e), + PublicGroupBuildError::InvalidExtensions(e) => e.into(), + })?; - let mut group = CoreGroup::builder( - group_id, - mls_group_create_config.ciphersuite, - credential_with_key, + let serialized_group_context = public_group_builder + .group_context() + .tls_serialize_detached() + .map_err(LibraryError::missing_bound_check)?; + + // Derive an initial joiner secret based on the commit secret. + // Derive an epoch secret from the joiner secret. + // We use a random `InitSecret` for initialization. + let joiner_secret = JoinerSecret::new( + provider.crypto(), + ciphersuite, + commit_secret, + &InitSecret::random(ciphersuite, provider.rand()) + .map_err(LibraryError::unexpected_crypto_error)?, + &serialized_group_context, ) - .with_config(group_config) - .with_group_context_extensions(mls_group_create_config.group_context_extensions.clone())? - .with_leaf_node_extensions(mls_group_create_config.leaf_node_extensions.clone())? - .with_capabilities(mls_group_create_config.capabilities.clone()) - .with_max_past_epoch_secrets(mls_group_create_config.join_config.max_past_epochs) - .with_lifetime(*mls_group_create_config.lifetime()) - .build(provider, signer) - .map_err(|e| match e { - CoreGroupBuildError::LibraryError(e) => e.into(), - // We don't support PSKs yet - CoreGroupBuildError::Psk(e) => { + .map_err(LibraryError::unexpected_crypto_error)?; + + // TODO(#1357) + let mut resumption_psk_store = ResumptionPskStore::new(32); + + // Prepare the PskSecret + let psk_secret = load_psks(provider.storage(), &resumption_psk_store, &self.psk_ids) + .and_then(|psks| PskSecret::new(provider.crypto(), ciphersuite, psks)) + .map_err(|e| { log::debug!("Unexpected PSK error: {:?}", e); - LibraryError::custom("Unexpected PSK error").into() - } - CoreGroupBuildError::StorageError(e) => NewGroupError::StorageError(e), - CoreGroupBuildError::PublicGroupBuildError(e) => match e { - PublicGroupBuildError::LibraryError(e) => e.into(), - PublicGroupBuildError::InvalidExtensions(e) => NewGroupError::InvalidExtensions(e), - }, - })?; + LibraryError::custom("Unexpected PSK error") + })?; + + let mut key_schedule = + KeySchedule::init(ciphersuite, provider.crypto(), &joiner_secret, psk_secret)?; + key_schedule + .add_context(provider.crypto(), &serialized_group_context) + .map_err(|_| LibraryError::custom("Using the key schedule in the wrong state"))?; + + let epoch_secrets = key_schedule + .epoch_secrets(provider.crypto(), ciphersuite) + .map_err(|_| LibraryError::custom("Using the key schedule in the wrong state"))?; + + let (group_epoch_secrets, message_secrets) = epoch_secrets.split_secrets( + serialized_group_context, + TreeSize::new(1), + LeafNodeIndex::new(0u32), + ); + + let initial_confirmation_tag = message_secrets + .confirmation_key() + .tag(provider.crypto(), ciphersuite, &[]) + .map_err(LibraryError::unexpected_crypto_error)?; + + let message_secrets_store = MessageSecretsStore::new_with_secret( + self.max_past_epochs.unwrap_or_default(), + message_secrets, + ); + + let public_group = public_group_builder + .with_confirmation_tag(initial_confirmation_tag) + .build(provider.crypto())?; // We already add a resumption PSK for epoch 0 to make things more unified. - let resumption_psk = group.group_epoch_secrets().resumption_psk(); - group - .resumption_psk_store - .add(group.context().epoch(), resumption_psk.clone()); + let resumption_psk = group_epoch_secrets.resumption_psk(); + resumption_psk_store.add(public_group.group_context().epoch(), resumption_psk.clone()); let mls_group = MlsGroup { mls_group_config: mls_group_create_config.join_config.clone(), - group, own_leaf_nodes: vec![], aad: vec![], group_state: MlsGroupState::Operational, + public_group, + group_epoch_secrets, + own_leaf_index: LeafNodeIndex::new(0), + message_secrets_store, + resumption_psk_store, }; - use openmls_traits::storage::StorageProvider as _; - - provider - .storage() - .write_mls_join_config(mls_group.group_id(), &mls_group.mls_group_config) - .map_err(NewGroupError::StorageError)?; - provider - .storage() - .write_group_state(mls_group.group_id(), &mls_group.group_state) - .map_err(NewGroupError::StorageError)?; mls_group - .group .store(provider.storage()) .map_err(NewGroupError::StorageError)?; + mls_group + .store_epoch_keypairs(provider.storage(), &[leaf_keypair]) + .map_err(NewGroupError::StorageError)?; Ok(mls_group) } diff --git a/openmls/src/group/mls_group/create_commit.rs b/openmls/src/group/mls_group/create_commit.rs new file mode 100644 index 000000000..f712f6536 --- /dev/null +++ b/openmls/src/group/mls_group/create_commit.rs @@ -0,0 +1,393 @@ +//! Defines the `CreateCommit` trait and its implementation for `MlsGroup`. + +use super::*; +use crate::{credentials::CredentialWithKey, treesync::LeafNodeParameters}; + +/// Can be used to denote the type of a commit. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub(crate) enum CommitType { + External(CredentialWithKey), + Member, +} + +pub(crate) struct CreateCommitParams<'a> { + framing_parameters: FramingParameters<'a>, // Mandatory + inline_proposals: Vec, // Optional + force_self_update: bool, // Optional + commit_type: CommitType, // Optional (default is `Member`) + leaf_node_parameters: LeafNodeParameters, // Optional +} + +pub(crate) struct TempBuilderCCPM0 {} + +pub(crate) struct CreateCommitParamsBuilder<'a> { + ccp: CreateCommitParams<'a>, +} + +impl TempBuilderCCPM0 { + pub(crate) fn framing_parameters( + self, + framing_parameters: FramingParameters, + ) -> CreateCommitParamsBuilder { + CreateCommitParamsBuilder { + ccp: CreateCommitParams { + framing_parameters, + inline_proposals: vec![], + force_self_update: true, + commit_type: CommitType::Member, + leaf_node_parameters: LeafNodeParameters::default(), + }, + } + } +} + +impl<'a> CreateCommitParamsBuilder<'a> { + pub(crate) fn inline_proposals(mut self, inline_proposals: Vec) -> Self { + self.ccp.inline_proposals = inline_proposals; + self + } + pub(crate) fn force_self_update(mut self, force_self_update: bool) -> Self { + self.ccp.force_self_update = force_self_update; + self + } + pub(crate) fn commit_type(mut self, commit_type: CommitType) -> Self { + self.ccp.commit_type = commit_type; + self + } + pub(crate) fn leaf_node_parameters(mut self, leaf_node_parameters: LeafNodeParameters) -> Self { + self.ccp.leaf_node_parameters = leaf_node_parameters; + self + } + pub(crate) fn build(self) -> CreateCommitParams<'a> { + self.ccp + } +} + +impl<'a> CreateCommitParams<'a> { + pub(crate) fn builder() -> TempBuilderCCPM0 { + TempBuilderCCPM0 {} + } + pub(crate) fn framing_parameters(&self) -> &FramingParameters { + &self.framing_parameters + } + pub(crate) fn inline_proposals(&self) -> &[Proposal] { + &self.inline_proposals + } + pub(crate) fn set_inline_proposals(&mut self, inline_proposals: Vec) { + self.inline_proposals = inline_proposals; + } + pub(crate) fn force_self_update(&self) -> bool { + self.force_self_update + } + pub(crate) fn commit_type(&self) -> &CommitType { + &self.commit_type + } + pub(crate) fn leaf_node_parameters(&self) -> &LeafNodeParameters { + &self.leaf_node_parameters + } +} + +impl MlsGroup { + pub(crate) fn create_commit( + &self, + params: CreateCommitParams, + provider: &Provider, + signer: &impl Signer, + ) -> Result> { + let ciphersuite = self.ciphersuite(); + + let sender = match params.commit_type() { + CommitType::External(_) => Sender::NewMemberCommit, + CommitType::Member => Sender::build_member(self.own_leaf_index()), + }; + + // Filter proposals + let (proposal_queue, contains_own_updates) = ProposalQueue::filter_proposals( + ciphersuite, + provider.crypto(), + sender.clone(), + self.proposal_store(), + params.inline_proposals(), + self.own_leaf_index(), + ) + .map_err(|e| match e { + ProposalQueueError::LibraryError(e) => e.into(), + ProposalQueueError::ProposalNotFound => CreateCommitError::MissingProposal, + ProposalQueueError::UpdateFromExternalSender => { + CreateCommitError::WrongProposalSenderType + } + })?; + + // TODO: #581 Filter proposals by support + // 11.2: + // Proposals with a non-default proposal type MUST NOT be included in a commit + // unless the proposal type is supported by all the members of the group that + // will process the Commit (i.e., not including any members being added + // or removed by the Commit). + + let proposal_reference_list = proposal_queue.commit_list(); + + // Validate the proposals by doing the following checks: + + // ValSem113: All Proposals: The proposal type must be supported by all + // members of the group + self.public_group + .validate_proposal_type_support(&proposal_queue)?; + // ValSem101 + // ValSem102 + // ValSem103 + // ValSem104 + self.public_group + .validate_key_uniqueness(&proposal_queue, None)?; + // ValSem105 + self.public_group.validate_add_proposals(&proposal_queue)?; + // ValSem106 + // ValSem109 + self.public_group.validate_capabilities(&proposal_queue)?; + // ValSem107 + // ValSem108 + self.public_group + .validate_remove_proposals(&proposal_queue)?; + self.public_group + .validate_pre_shared_key_proposals(&proposal_queue)?; + // Validate update proposals for member commits + if let Sender::Member(sender_index) = &sender { + // ValSem110 + // ValSem111 + // ValSem112 + self.public_group + .validate_update_proposals(&proposal_queue, *sender_index)?; + } + + // ValSem208 + // ValSem209 + self.public_group + .validate_group_context_extensions_proposal(&proposal_queue)?; + + // Make a copy of the public group to apply proposals safely + let mut diff = self.public_group.empty_diff(); + + // Apply proposals to tree + let apply_proposals_values = + diff.apply_proposals(&proposal_queue, self.own_leaf_index())?; + if apply_proposals_values.self_removed && params.commit_type() == &CommitType::Member { + return Err(CreateCommitError::CannotRemoveSelf); + } + + let path_computation_result = + // If path is needed, compute path values + if apply_proposals_values.path_required + || contains_own_updates + || params.force_self_update() + || !params.leaf_node_parameters().is_empty() + { + // Process the path. This includes updating the provisional + // group context by updating the epoch and computing the new + // tree hash. + diff.compute_path( + provider, + self.own_leaf_index(), + apply_proposals_values.exclusion_list(), + params.commit_type(), + params.leaf_node_parameters(), + signer, + apply_proposals_values.extensions.clone() + )? + } else { + // If path is not needed, update the group context and return + // empty path processing results + diff.update_group_context(provider.crypto(), apply_proposals_values.extensions.clone())?; + PathComputationResult::default() + }; + + let update_path_leaf_node = path_computation_result + .encrypted_path + .as_ref() + .map(|path| path.leaf_node().clone()); + + // Create commit message + let commit = Commit { + proposals: proposal_reference_list, + path: path_computation_result.encrypted_path, + }; + + // Build AuthenticatedContent + let mut authenticated_content = AuthenticatedContent::commit( + *params.framing_parameters(), + sender, + commit, + self.public_group.group_context(), + signer, + )?; + + // Update the confirmed transcript hash using the commit we just created. + diff.update_confirmed_transcript_hash(provider.crypto(), &authenticated_content)?; + + let serialized_provisional_group_context = diff + .group_context() + .tls_serialize_detached() + .map_err(LibraryError::missing_bound_check)?; + + let joiner_secret = JoinerSecret::new( + provider.crypto(), + ciphersuite, + path_computation_result.commit_secret, + self.group_epoch_secrets().init_secret(), + &serialized_provisional_group_context, + ) + .map_err(LibraryError::unexpected_crypto_error)?; + + // Prepare the PskSecret + let psk_secret = { + let psks = load_psks( + provider.storage(), + &self.resumption_psk_store, + &apply_proposals_values.presharedkeys, + )?; + + PskSecret::new(provider.crypto(), ciphersuite, psks)? + }; + + // Create key schedule + let mut key_schedule = + KeySchedule::init(ciphersuite, provider.crypto(), &joiner_secret, psk_secret)?; + + let serialized_provisional_group_context = diff + .group_context() + .tls_serialize_detached() + .map_err(LibraryError::missing_bound_check)?; + + let welcome_secret = key_schedule + .welcome(provider.crypto(), self.ciphersuite()) + .map_err(|_| LibraryError::custom("Using the key schedule in the wrong state"))?; + key_schedule + .add_context(provider.crypto(), &serialized_provisional_group_context) + .map_err(|_| LibraryError::custom("Using the key schedule in the wrong state"))?; + let provisional_epoch_secrets = key_schedule + .epoch_secrets(provider.crypto(), self.ciphersuite()) + .map_err(|_| LibraryError::custom("Using the key schedule in the wrong state"))?; + + // Calculate the confirmation tag + let confirmation_tag = provisional_epoch_secrets + .confirmation_key() + .tag( + provider.crypto(), + self.ciphersuite(), + diff.group_context().confirmed_transcript_hash(), + ) + .map_err(LibraryError::unexpected_crypto_error)?; + + // Set the confirmation tag + authenticated_content.set_confirmation_tag(confirmation_tag.clone()); + + diff.update_interim_transcript_hash( + ciphersuite, + provider.crypto(), + confirmation_tag.clone(), + )?; + + // only computes the group info if necessary + let group_info = if !apply_proposals_values.invitation_list.is_empty() + || self.configuration().use_ratchet_tree_extension + { + // Create the ratchet tree extension if necessary + let external_pub = provisional_epoch_secrets + .external_secret() + .derive_external_keypair(provider.crypto(), ciphersuite) + .map_err(LibraryError::unexpected_crypto_error)? + .public; + let external_pub_extension = + Extension::ExternalPub(ExternalPubExtension::new(external_pub.into())); + let other_extensions: Extensions = if self.configuration().use_ratchet_tree_extension { + Extensions::from_vec(vec![ + Extension::RatchetTree(RatchetTreeExtension::new(diff.export_ratchet_tree())), + external_pub_extension, + ])? + } else { + Extensions::single(external_pub_extension) + }; + + // Create to-be-signed group info. + let group_info_tbs = { + GroupInfoTBS::new( + diff.group_context().clone(), + other_extensions, + confirmation_tag, + self.own_leaf_index(), + ) + }; + // Sign to-be-signed group info. + Some(group_info_tbs.sign(signer)?) + } else { + None + }; + + // Check if new members were added and, if so, create welcome messages + let welcome_option = if !apply_proposals_values.invitation_list.is_empty() { + // Encrypt GroupInfo object + let (welcome_key, welcome_nonce) = welcome_secret + .derive_welcome_key_nonce(provider.crypto(), self.ciphersuite()) + .map_err(LibraryError::unexpected_crypto_error)?; + let encrypted_group_info = welcome_key + .aead_seal( + provider.crypto(), + group_info + .as_ref() + .ok_or_else(|| LibraryError::custom("GroupInfo was not computed"))? + .tls_serialize_detached() + .map_err(LibraryError::missing_bound_check)? + .as_slice(), + &[], + &welcome_nonce, + ) + .map_err(LibraryError::unexpected_crypto_error)?; + + // Create group secrets for later use, so we can afterwards consume the + // `joiner_secret`. + let encrypted_secrets = diff.encrypt_group_secrets( + &joiner_secret, + apply_proposals_values.invitation_list, + path_computation_result.plain_path.as_deref(), + &apply_proposals_values.presharedkeys, + &encrypted_group_info, + provider.crypto(), + self.own_leaf_index(), + )?; + + // Create welcome message + let welcome = Welcome::new(self.ciphersuite(), encrypted_secrets, encrypted_group_info); + Some(welcome) + } else { + None + }; + + let (provisional_group_epoch_secrets, provisional_message_secrets) = + provisional_epoch_secrets.split_secrets( + serialized_provisional_group_context, + diff.tree_size(), + self.own_leaf_index(), + ); + + let staged_commit_state = MemberStagedCommitState::new( + provisional_group_epoch_secrets, + provisional_message_secrets, + diff.into_staged_diff(provider.crypto(), ciphersuite)?, + path_computation_result.new_keypairs, + // The committer is not allowed to include their own update + // proposal, so there is no extra keypair to store here. + None, + update_path_leaf_node, + ); + let staged_commit = StagedCommit::new( + proposal_queue, + StagedCommitState::GroupMember(Box::new(staged_commit_state)), + ); + + Ok(CreateCommitResult { + commit: authenticated_content, + welcome_option, + staged_commit, + group_info: group_info.filter(|_| self.configuration().use_ratchet_tree_extension), + }) + } +} diff --git a/openmls/src/group/mls_group/creation.rs b/openmls/src/group/mls_group/creation.rs index d473f3e31..5c72a8e06 100644 --- a/openmls/src/group/mls_group/creation.rs +++ b/openmls/src/group/mls_group/creation.rs @@ -1,24 +1,29 @@ +use errors::NewGroupError; use openmls_traits::{signatures::Signer, storage::StorageProvider as StorageProviderTrait}; use super::{builder::MlsGroupBuilder, *}; use crate::{ credentials::CredentialWithKey, - group::{ - core_group::create_commit_params::{CommitType, CreateCommitParams}, - errors::{ExternalCommitError, WelcomeError}, - }, + group::errors::{ExternalCommitError, WelcomeError}, messages::{ group_info::{GroupInfo, VerifiableGroupInfo}, Welcome, }, - schedule::psk::{store::ResumptionPskStore, PreSharedKeyId}, + schedule::{ + psk::{store::ResumptionPskStore, PreSharedKeyId}, + EpochSecrets, InitSecret, + }, storage::OpenMlsProvider, treesync::{ + errors::{DerivePathError, PublicTreeError}, node::leaf_node::{Capabilities, LeafNodeParameters}, RatchetTreeIn, }, }; +#[cfg(doc)] +use crate::key_packages::KeyPackage; + impl MlsGroup { // === Group creation === @@ -100,40 +105,118 @@ impl MlsGroup { .with_capabilities(capabilities.unwrap_or_default()) .with_extensions(extensions.unwrap_or_default()) .build(); - let params = CreateCommitParams::builder() + let mut params = CreateCommitParams::builder() .framing_parameters(framing_parameters) .commit_type(CommitType::External(credential_with_key)) .leaf_node_parameters(leaf_node_parameters) .build(); - let (mut group, create_commit_result) = CoreGroup::join_by_external_commit( - provider, - signer, - params, + + // Build the ratchet tree + + // Set nodes either from the extension or from the `nodes_option`. + // If we got a ratchet tree extension in the welcome, we enable it for + // this group. Note that this is not strictly necessary. But there's + // currently no other mechanism to enable the extension. + let ratchet_tree = match verifiable_group_info.extensions().ratchet_tree() { + Some(extension) => extension.ratchet_tree().clone(), + None => match ratchet_tree { + Some(ratchet_tree) => ratchet_tree, + None => return Err(ExternalCommitError::MissingRatchetTree), + }, + }; + + let (public_group, group_info) = PublicGroup::from_external( + provider.crypto(), + provider.storage(), ratchet_tree, verifiable_group_info, + // Existing proposals are discarded when joining by external commit. + ProposalStore::new(), )?; - group.set_max_past_epochs(mls_group_config.max_past_epochs); + let group_context = public_group.group_context(); + + // Obtain external_pub from GroupInfo extensions. + let external_pub = group_info + .extensions() + .external_pub() + .ok_or(ExternalCommitError::MissingExternalPub)? + .external_pub(); + + let (init_secret, kem_output) = InitSecret::from_group_context( + provider.crypto(), + group_context, + external_pub.as_slice(), + ) + .map_err(|_| ExternalCommitError::UnsupportedCiphersuite)?; + + // The `EpochSecrets` we create here are essentially zero, with the + // exception of the `InitSecret`, which is all we need here for the + // external commit. + let epoch_secrets = EpochSecrets::with_init_secret( + provider.crypto(), + group_info.group_context().ciphersuite(), + init_secret, + ) + .map_err(LibraryError::unexpected_crypto_error)?; + let (group_epoch_secrets, message_secrets) = epoch_secrets.split_secrets( + group_context + .tls_serialize_detached() + .map_err(LibraryError::missing_bound_check)?, + public_group.tree_size(), + // We use a fake own index of 0 here, as we're not going to use the + // tree for encryption until after the first commit. This issue is + // tracked in #767. + LeafNodeIndex::new(0u32), + ); + let message_secrets_store = MessageSecretsStore::new_with_secret(0, message_secrets); + + let external_init_proposal = Proposal::ExternalInit(ExternalInitProposal::from(kem_output)); + + let mut inline_proposals = vec![external_init_proposal]; + + // If there is a group member in the group with the same identity as us, + // commit a remove proposal. + let signature_key = match params.commit_type() { + CommitType::External(credential_with_key) => { + credential_with_key.signature_key.as_slice() + } + _ => return Err(ExternalCommitError::MissingCredential), + }; + if let Some(us) = public_group + .members() + .find(|member| member.signature_key == signature_key) + { + let remove_proposal = Proposal::Remove(RemoveProposal { removed: us.index }); + inline_proposals.push(remove_proposal); + }; - let mls_group = MlsGroup { + let own_leaf_index = public_group.leftmost_free_index(inline_proposals.iter().map(Some))?; + params.set_inline_proposals(inline_proposals); + + let mut mls_group = MlsGroup { mls_group_config: mls_group_config.clone(), - group, own_leaf_nodes: vec![], aad: vec![], - group_state: MlsGroupState::PendingCommit(Box::new(PendingCommitState::External( - create_commit_result.staged_commit, - ))), + group_state: MlsGroupState::Operational, + public_group, + group_epoch_secrets, + own_leaf_index, + message_secrets_store, + resumption_psk_store: ResumptionPskStore::new(32), }; - provider - .storage() - .write_mls_join_config(mls_group.group_id(), &mls_group.mls_group_config) - .map_err(ExternalCommitError::StorageError)?; - provider - .storage() - .write_group_state(mls_group.group_id(), &mls_group.group_state) - .map_err(ExternalCommitError::StorageError)?; + mls_group.set_max_past_epochs(mls_group_config.max_past_epochs); + + // Immediately create the commit to add ourselves to the group. + let create_commit_result = mls_group + .create_commit(params, provider, signer) + .map_err(|_| ExternalCommitError::CommitError)?; + + mls_group.group_state = MlsGroupState::PendingCommit(Box::new( + PendingCommitState::External(create_commit_result.staged_commit), + )); + mls_group - .group .store(provider.storage()) .map_err(ExternalCommitError::StorageError)?; @@ -170,14 +253,65 @@ impl ProcessedWelcome { let (resumption_psk_store, key_package_bundle) = keys_for_welcome(mls_group_config, &welcome, provider)?; - let (ciphersuite, group_secrets, key_schedule, verifiable_group_info) = - crate::group::core_group::new_from_welcome::process_welcome( - welcome, - &key_package_bundle, - provider, + let ciphersuite = welcome.ciphersuite(); + let Some(egs) = welcome.find_encrypted_group_secret( + key_package_bundle + .key_package() + .hash_ref(provider.crypto())?, + ) else { + return Err(WelcomeError::JoinerSecretNotFound); + }; + if ciphersuite != key_package_bundle.key_package().ciphersuite() { + let e = WelcomeError::CiphersuiteMismatch; + log::debug!("new_from_welcome {:?}", e); + return Err(e); + } + let group_secrets = GroupSecrets::try_from_ciphertext( + key_package_bundle.init_private_key(), + egs.encrypted_group_secrets(), + welcome.encrypted_group_info(), + ciphersuite, + provider.crypto(), + )?; + let psk_secret = { + let psks = load_psks( + provider.storage(), &resumption_psk_store, + &group_secrets.psks, )?; + PskSecret::new(provider.crypto(), ciphersuite, psks)? + }; + let key_schedule = KeySchedule::init( + ciphersuite, + provider.crypto(), + &group_secrets.joiner_secret, + psk_secret, + )?; + let (welcome_key, welcome_nonce) = key_schedule + .welcome(provider.crypto(), ciphersuite) + .map_err(|_| LibraryError::custom("Using the key schedule in the wrong state"))? + .derive_welcome_key_nonce(provider.crypto(), ciphersuite) + .map_err(LibraryError::unexpected_crypto_error)?; + let verifiable_group_info = VerifiableGroupInfo::try_from_ciphertext( + &welcome_key, + &welcome_nonce, + welcome.encrypted_group_info(), + &[], + provider.crypto(), + )?; + if let Some(required_capabilities) = + verifiable_group_info.extensions().required_capabilities() + { + // Also check that our key package actually supports the extensions. + // As per the spec, the sender must have checked this. But you never know. + key_package_bundle + .key_package() + .leaf_node() + .capabilities() + .supports_required_capabilities(required_capabilities)?; + } + Ok(Self { mls_group_config: mls_group_config.clone(), ciphersuite, @@ -206,24 +340,139 @@ impl ProcessedWelcome { /// Consume the `ProcessedWelcome` and combine it witht he ratchet tree into /// a `StagedWelcome`. pub fn into_staged_welcome( - self, + mut self, provider: &Provider, ratchet_tree: Option, ) -> Result> { - let group = crate::group::core_group::new_from_welcome::build_staged_welcome( - self.verifiable_group_info, + // Build the ratchet tree and group + + // Set nodes either from the extension or from the `nodes_option`. + // If we got a ratchet tree extension in the welcome, we enable it for + // this group. Note that this is not strictly necessary. But there's + // currently no other mechanism to enable the extension. + let ratchet_tree = match self.verifiable_group_info.extensions().ratchet_tree() { + Some(extension) => extension.ratchet_tree().clone(), + None => match ratchet_tree { + Some(ratchet_tree) => ratchet_tree, + None => return Err(WelcomeError::MissingRatchetTree), + }, + }; + + // Since there is currently only the external pub extension, there is no + // group info extension of interest here. + let (public_group, _group_info_extensions) = PublicGroup::from_external( + provider.crypto(), + provider.storage(), ratchet_tree, - provider, - self.key_package_bundle, - self.key_schedule, - self.ciphersuite, - self.resumption_psk_store, - self.group_secrets, + self.verifiable_group_info.clone(), + ProposalStore::new(), )?; + // Find our own leaf in the tree. + let own_leaf_index = public_group + .members() + .find_map(|m| { + if m.signature_key + == self + .key_package_bundle + .key_package() + .leaf_node() + .signature_key() + .as_slice() + { + Some(m.index) + } else { + None + } + }) + .ok_or(WelcomeError::PublicTreeError( + PublicTreeError::MalformedTree, + ))?; + + let (group_epoch_secrets, message_secrets) = { + let serialized_group_context = public_group + .group_context() + .tls_serialize_detached() + .map_err(LibraryError::missing_bound_check)?; + + // TODO #751: Implement PSK + self.key_schedule + .add_context(provider.crypto(), &serialized_group_context) + .map_err(|_| LibraryError::custom("Using the key schedule in the wrong state"))?; + + let epoch_secrets = self + .key_schedule + .epoch_secrets(provider.crypto(), self.ciphersuite) + .map_err(|_| LibraryError::custom("Using the key schedule in the wrong state"))?; + + epoch_secrets.split_secrets( + serialized_group_context, + public_group.tree_size(), + own_leaf_index, + ) + }; + + let confirmation_tag = message_secrets + .confirmation_key() + .tag( + provider.crypto(), + self.ciphersuite, + public_group.group_context().confirmed_transcript_hash(), + ) + .map_err(LibraryError::unexpected_crypto_error)?; + + // Verify confirmation tag + if &confirmation_tag != public_group.confirmation_tag() { + log::error!("Confirmation tag mismatch"); + log_crypto!(trace, " Got: {:x?}", confirmation_tag); + log_crypto!(trace, " Expected: {:x?}", public_group.confirmation_tag()); + debug_assert!(false, "Confirmation tag mismatch"); + + // in some tests we need to be able to proceed despite the tag being wrong, + // e.g. to test whether a later validation check is performed correctly. + if !crate::skip_validation::is_disabled::confirmation_tag() { + return Err(WelcomeError::ConfirmationTagMismatch); + } + } + + let message_secrets_store = MessageSecretsStore::new_with_secret(0, message_secrets); + + // Extract and store the resumption PSK for the current epoch. + let resumption_psk = group_epoch_secrets.resumption_psk(); + self.resumption_psk_store + .add(public_group.group_context().epoch(), resumption_psk.clone()); + + let welcome_sender_index = self.verifiable_group_info.signer(); + let path_keypairs = if let Some(path_secret) = self.group_secrets.path_secret { + let (path_keypairs, _commit_secret) = public_group + .derive_path_secrets( + provider.crypto(), + self.ciphersuite, + path_secret, + welcome_sender_index, + own_leaf_index, + ) + .map_err(|e| match e { + DerivePathError::LibraryError(e) => e.into(), + DerivePathError::PublicKeyMismatch => { + WelcomeError::PublicTreeError(PublicTreeError::PublicKeyMismatch) + } + })?; + Some(path_keypairs) + } else { + None + }; + let staged_welcome = StagedWelcome { mls_group_config: self.mls_group_config, - group, + public_group, + group_epoch_secrets, + own_leaf_index, + message_secrets_store, + resumption_psk_store: self.resumption_psk_store, + verifiable_group_info: self.verifiable_group_info, + key_package_bundle: self.key_package_bundle, + path_keypairs, }; Ok(staged_welcome) @@ -244,37 +493,29 @@ impl StagedWelcome { welcome: Welcome, ratchet_tree: Option, ) -> Result> { - let (resumption_psk_store, key_package_bundle) = - keys_for_welcome(mls_group_config, &welcome, provider)?; - - let group = StagedCoreWelcome::new_from_welcome( - welcome, - ratchet_tree, - key_package_bundle, - provider, - resumption_psk_store, - )?; - - let staged_welcome = StagedWelcome { - mls_group_config: mls_group_config.clone(), - group, - }; + let processed_welcome = + ProcessedWelcome::new_from_welcome(provider, mls_group_config, welcome)?; - Ok(staged_welcome) + processed_welcome.into_staged_welcome(provider, ratchet_tree) } /// Returns the [`LeafNodeIndex`] of the group member that authored the [`Welcome`] message. /// /// [`Welcome`]: crate::messages::Welcome pub fn welcome_sender_index(&self) -> LeafNodeIndex { - self.group.welcome_sender_index() + self.verifiable_group_info.signer() } /// Returns the [`LeafNode`] of the group member that authored the [`Welcome`] message. /// /// [`Welcome`]: crate::messages::Welcome pub fn welcome_sender(&self) -> Result<&LeafNode, LibraryError> { - self.group.welcome_sender() + let sender_index = self.welcome_sender_index(); + self.public_group + .leaf(sender_index) + .ok_or(LibraryError::custom( + "no leaf with given welcome sender index exists", + )) } /// Consumes the [`StagedWelcome`] and returns the respective [`MlsGroup`]. @@ -282,24 +523,35 @@ impl StagedWelcome { self, provider: &Provider, ) -> Result> { - let mut group = self.group.into_core_group(provider)?; - group.set_max_past_epochs(self.mls_group_config.max_past_epochs); + // If we got a path secret, derive the path (which also checks if the + // public keys match) and store the derived keys in the key store. + let group_keypairs = if let Some(path_keypairs) = self.path_keypairs { + let mut keypairs = vec![self.key_package_bundle.encryption_key_pair()]; + keypairs.extend_from_slice(&path_keypairs); + keypairs + } else { + vec![self.key_package_bundle.encryption_key_pair()] + }; - let mls_group = MlsGroup { + let mut mls_group = MlsGroup { mls_group_config: self.mls_group_config, - group, own_leaf_nodes: vec![], aad: vec![], group_state: MlsGroupState::Operational, + public_group: self.public_group, + group_epoch_secrets: self.group_epoch_secrets, + own_leaf_index: self.own_leaf_index, + message_secrets_store: self.message_secrets_store, + resumption_psk_store: self.resumption_psk_store, }; - provider - .storage() - .write_mls_join_config(mls_group.group_id(), &mls_group.mls_group_config) + mls_group + .store_epoch_keypairs(provider.storage(), group_keypairs.as_slice()) .map_err(WelcomeError::StorageError)?; - provider - .storage() - .write_group_state(mls_group.group_id(), &MlsGroupState::Operational) + mls_group.set_max_past_epochs(mls_group.mls_group_config.max_past_epochs); + + mls_group + .store(provider.storage()) .map_err(WelcomeError::StorageError)?; Ok(mls_group) diff --git a/openmls/src/group/mls_group/exporting.rs b/openmls/src/group/mls_group/exporting.rs index 09fa39eff..9d670a8bd 100644 --- a/openmls/src/group/mls_group/exporting.rs +++ b/openmls/src/group/mls_group/exporting.rs @@ -1,6 +1,11 @@ +use errors::{ExportGroupInfoError, ExportSecretError}; use openmls_traits::signatures::Signer; -use crate::{group::errors::ExporterError, schedule::EpochAuthenticator, storage::OpenMlsProvider}; +use crate::{ + ciphersuite::HpkePublicKey, + schedule::{EpochAuthenticator, ResumptionPskSecret}, + storage::OpenMlsProvider, +}; use super::*; @@ -21,14 +26,17 @@ impl MlsGroup { ) -> Result, ExportSecretError> { let crypto = provider.crypto(); + if key_length > u16::MAX.into() { + log::error!("Got a key that is larger than u16::MAX"); + return Err(ExportSecretError::KeyLengthTooLong); + } + if self.is_active() { Ok(self - .group - .export_secret(crypto, label, context, key_length) - .map_err(|e| match e { - ExporterError::LibraryError(e) => e.into(), - ExporterError::KeyLengthTooLong => ExportSecretError::KeyLengthTooLong, - })?) + .group_epoch_secrets + .exporter_secret() + .derive_exported_secret(self.ciphersuite(), crypto, label, context, key_length) + .map_err(LibraryError::unexpected_crypto_error)?) } else { Err(ExportSecretError::GroupStateError( MlsGroupStateError::UseAfterEviction, @@ -38,18 +46,18 @@ impl MlsGroup { /// Returns the epoch authenticator of the current epoch. pub fn epoch_authenticator(&self) -> &EpochAuthenticator { - self.group.epoch_authenticator() + self.group_epoch_secrets().epoch_authenticator() } /// Returns the resumption PSK secret of the current epoch. pub fn resumption_psk_secret(&self) -> &ResumptionPskSecret { - self.group.resumption_psk_secret() + self.group_epoch_secrets().resumption_psk() } /// Returns a resumption psk for a given epoch. If no resumption psk /// is available for that epoch, `None` is returned. pub fn get_past_resumption_psk(&self, epoch: GroupEpoch) -> Option<&ResumptionPskSecret> { - self.group.resumption_psk_store.get(epoch) + self.resumption_psk_store.get(epoch) } /// Export a group info object for this group. @@ -59,9 +67,56 @@ impl MlsGroup { signer: &impl Signer, with_ratchet_tree: bool, ) -> Result { - Ok(self - .group - .export_group_info(provider.crypto(), signer, with_ratchet_tree)? - .into()) + let extensions = { + let ratchet_tree_extension = || { + Extension::RatchetTree(RatchetTreeExtension::new( + self.public_group().export_ratchet_tree(), + )) + }; + + let external_pub_extension = || -> Result { + let external_pub = self + .group_epoch_secrets() + .external_secret() + .derive_external_keypair(provider.crypto(), self.ciphersuite()) + .map_err(LibraryError::unexpected_crypto_error)? + .public; + Ok(Extension::ExternalPub(ExternalPubExtension::new( + HpkePublicKey::from(external_pub), + ))) + }; + + if with_ratchet_tree { + Extensions::from_vec(vec![ratchet_tree_extension(), external_pub_extension()?]) + .map_err(|_| { + LibraryError::custom( + "There should not have been duplicate extensions here.", + ) + })? + } else { + Extensions::single(external_pub_extension()?) + } + }; + + // Create to-be-signed group info. + let group_info_tbs = GroupInfoTBS::new( + self.context().clone(), + extensions, + self.message_secrets() + .confirmation_key() + .tag( + provider.crypto(), + self.ciphersuite(), + self.context().confirmed_transcript_hash(), + ) + .map_err(LibraryError::unexpected_crypto_error)?, + self.own_leaf_index(), + ); + + // Sign to-be-signed group info. + let group_info = group_info_tbs + .sign(signer) + .map_err(|_| LibraryError::custom("Signing failed"))?; + Ok(group_info.into()) } } diff --git a/openmls/src/group/mls_group/membership.rs b/openmls/src/group/mls_group/membership.rs index a8d87538d..1cb4dc65f 100644 --- a/openmls/src/group/mls_group/membership.rs +++ b/openmls/src/group/mls_group/membership.rs @@ -2,16 +2,17 @@ //! //! This module contains membership-related operations and exposes [`RemoveOperation`]. -use core_group::create_commit_params::CreateCommitParams; +use errors::EmptyInputError; use openmls_traits::{signatures::Signer, storage::StorageProvider as _}; +use proposal_store::QueuedRemoveProposal; use super::{ errors::{AddMembersError, LeaveGroupError, RemoveMembersError}, *, }; use crate::{ - binary_tree::array_representation::LeafNodeIndex, messages::group_info::GroupInfo, - storage::OpenMlsProvider, treesync::LeafNode, + binary_tree::array_representation::LeafNodeIndex, key_packages::KeyPackage, + messages::group_info::GroupInfo, storage::OpenMlsProvider, treesync::LeafNode, }; impl MlsGroup { @@ -112,7 +113,7 @@ impl MlsGroup { .inline_proposals(inline_proposals) .force_self_update(force_self_update) .build(); - let create_commit_result = self.group.create_commit(params, provider, signer)?; + let create_commit_result = self.create_commit(params, provider, signer)?; let welcome = match create_commit_result.welcome_option { Some(welcome) => welcome, @@ -139,14 +140,14 @@ impl MlsGroup { self.reset_aad(); Ok(( mls_messages, - MlsMessageOut::from_welcome(welcome, self.group.version()), + MlsMessageOut::from_welcome(welcome, self.version()), create_commit_result.group_info, )) } /// Returns a reference to the own [`LeafNode`]. pub fn own_leaf(&self) -> Option<&LeafNode> { - self.group.public_group().leaf(self.group.own_leaf_index()) + self.public_group().leaf(self.own_leaf_index()) } /// Removes members from the group. @@ -194,7 +195,7 @@ impl MlsGroup { .framing_parameters(self.framing_parameters()) .inline_proposals(inline_proposals) .build(); - let create_commit_result = self.group.create_commit(params, provider, signer)?; + let create_commit_result = self.create_commit(params, provider, signer)?; // Convert PublicMessage messages to MLSMessage and encrypt them if required by // the configuration @@ -216,7 +217,7 @@ impl MlsGroup { mls_message, create_commit_result .welcome_option - .map(|w| MlsMessageOut::from_welcome(w, self.group.version())), + .map(|w| MlsMessageOut::from_welcome(w, self.version())), create_commit_result.group_info, )) } @@ -234,9 +235,8 @@ impl MlsGroup { ) -> Result> { self.is_operational()?; - let removed = self.group.own_leaf_index(); + let removed = self.own_leaf_index(); let remove_proposal = self - .group .create_remove_proposal(self.framing_parameters(), removed, signer) .map_err(|_| LibraryError::custom("Creating a self removal should not fail"))?; @@ -264,14 +264,13 @@ impl MlsGroup { /// Returns a list of [`Member`]s in the group. pub fn members(&self) -> impl Iterator + '_ { - self.group.public_group().members() + self.public_group().members() } /// Returns the [`Credential`] of a member corresponding to the given /// leaf index. Returns `None` if the member can not be found in this group. pub fn member(&self, leaf_index: LeafNodeIndex) -> Option<&Credential> { - self.group - .public_group() + self.public_group() // This will return an error if the member can't be found. .leaf(leaf_index) .map(|leaf| leaf.credential()) diff --git a/openmls/src/group/mls_group/mod.rs b/openmls/src/group/mls_group/mod.rs index 1538f2021..e4d6abe26 100644 --- a/openmls/src/group/mls_group/mod.rs +++ b/openmls/src/group/mls_group/mod.rs @@ -3,30 +3,51 @@ //! This module contains [`MlsGroup`] and its submodules. //! -#[cfg(any(feature = "test-utils", test))] -use crate::schedule::message_secrets::MessageSecrets; +use create_commit::{CommitType, CreateCommitParams}; +use past_secrets::MessageSecretsStore; +use proposal_store::ProposalQueue; +use serde::{Deserialize, Serialize}; +use staged_commit::{MemberStagedCommitState, StagedCommitState}; +use tls_codec::Serialize as _; #[cfg(test)] -use openmls_traits::crypto::OpenMlsCrypto; +use crate::treesync::node::leaf_node::TreePosition; -#[cfg(test)] -use crate::prelude::SenderRatchetConfiguration; - -use super::proposals::{ProposalStore, QueuedProposal}; +use super::{ + diff::compute_path::PathComputationResult, + proposal_store::{ProposalStore, QueuedProposal}, +}; use crate::{ binary_tree::array_representation::LeafNodeIndex, - ciphersuite::hash_ref::ProposalRef, + ciphersuite::{hash_ref::ProposalRef, signable::Signable}, credentials::Credential, error::LibraryError, framing::{mls_auth_content::AuthenticatedContent, *}, - group::*, - key_packages::{KeyPackage, KeyPackageBundle}, - messages::{proposals::*, GroupSecrets}, - schedule::ResumptionPskSecret, + group::{ + CreateCommitError, CreateGroupContextExtProposalError, Extension, ExtensionType, + Extensions, ExternalPubExtension, GroupContext, GroupEpoch, GroupId, MlsGroupJoinConfig, + MlsGroupStateError, OutgoingWireFormatPolicy, ProposalQueueError, PublicGroup, + RatchetTreeExtension, RequiredCapabilitiesExtension, StagedCommit, + }, + key_packages::KeyPackageBundle, + messages::{ + group_info::{GroupInfo, GroupInfoTBS, VerifiableGroupInfo}, + proposals::*, + Commit, GroupSecrets, Welcome, + }, + schedule::{ + message_secrets::MessageSecrets, + psk::{load_psks, store::ResumptionPskStore, PskSecret}, + GroupEpochSecrets, JoinerSecret, KeySchedule, + }, storage::{OpenMlsProvider, StorageProvider}, - treesync::{node::leaf_node::LeafNode, RatchetTree}, + treesync::{ + node::{encryption_keys::EncryptionKeyPair, leaf_node::LeafNode}, + RatchetTree, + }, + versions::ProtocolVersion, }; -use openmls_traits::types::Ciphersuite; +use openmls_traits::{signatures::Signer, storage::StorageProvider as _, types::Ciphersuite}; // Private mod application; @@ -39,15 +60,57 @@ use config::*; // Crate pub(crate) mod config; +pub(crate) mod create_commit; pub(crate) mod errors; pub(crate) mod membership; +pub(crate) mod past_secrets; pub(crate) mod processing; pub(crate) mod proposal; +pub(crate) mod proposal_store; +pub(crate) mod staged_commit; // Tests #[cfg(test)] pub(crate) mod tests_and_kats; +#[derive(Debug)] +pub(crate) struct CreateCommitResult { + pub(crate) commit: AuthenticatedContent, + pub(crate) welcome_option: Option, + pub(crate) staged_commit: StagedCommit, + pub(crate) group_info: Option, +} + +/// A member in the group is identified by this [`Member`] struct. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct Member { + /// The member's leaf index in the ratchet tree. + pub index: LeafNodeIndex, + /// The member's credential. + pub credential: Credential, + /// The member's public HPHKE encryption key. + pub encryption_key: Vec, + /// The member's public signature key. + pub signature_key: Vec, +} + +impl Member { + /// Create new member. + pub fn new( + index: LeafNodeIndex, + encryption_key: Vec, + signature_key: Vec, + credential: Credential, + ) -> Self { + Self { + index, + encryption_key, + signature_key, + credential, + } + } +} + /// Pending Commit state. Differentiates between Commits issued by group members /// and External Commits. #[derive(Debug, Serialize, Deserialize)] @@ -161,11 +224,23 @@ pub enum MlsGroupState { #[derive(Debug)] #[cfg_attr(feature = "test-utils", derive(Clone, PartialEq))] pub struct MlsGroup { - // The group configuration. See [`MlsGroupJoinConfig`] for more information. + /// The group configuration. See [`MlsGroupJoinConfig`] for more information. mls_group_config: MlsGroupJoinConfig, - // the internal `CoreGroup` used for lower level operations. See `CoreGroup` for more - // information. - group: CoreGroup, + /// The public state of the group. + public_group: PublicGroup, + /// Epoch-specific secrets of the group. + group_epoch_secrets: GroupEpochSecrets, + /// The own leaf index in the ratchet tree. + own_leaf_index: LeafNodeIndex, + /// A [`MessageSecretsStore`] that stores message secrets. + /// By default this store has the length of 1, i.e. only the [`MessageSecrets`] + /// of the current epoch is kept. + /// If more secrets from past epochs should be kept in order to be + /// able to decrypt application messages from previous epochs, the size of + /// the store must be increased through [`max_past_epochs()`]. + message_secrets_store: MessageSecretsStore, + // Resumption psk store. This is where the resumption psks are kept in a rollover list. + resumption_psk_store: ResumptionPskStore, // Own [`LeafNode`]s that were created for update proposals and that // are needed in case an update proposal is committed by another group // member. The vector is emptied after every epoch change. @@ -214,7 +289,7 @@ impl MlsGroup { /// Returns the group's ciphersuite. pub fn ciphersuite(&self) -> Ciphersuite { - self.group.ciphersuite() + self.public_group.ciphersuite() } /// Returns whether the own client is still a member of the group or if it @@ -229,8 +304,7 @@ impl MlsGroup { if !self.is_active() { return Err(MlsGroupStateError::UseAfterEviction); } - self.group - .public_group() + self.public_group .leaf(self.own_leaf_index()) .map(|node| node.credential()) .ok_or_else(|| LibraryError::custom("Own leaf node missing").into()) @@ -238,22 +312,22 @@ impl MlsGroup { /// Returns the leaf index of the client in the tree owning this group. pub fn own_leaf_index(&self) -> LeafNodeIndex { - self.group.own_leaf_index() + self.own_leaf_index } /// Returns the leaf node of the client in the tree owning this group. pub fn own_leaf_node(&self) -> Option<&LeafNode> { - self.group.own_leaf_node().ok() + self.public_group().leaf(self.own_leaf_index()) } /// Returns the group ID. pub fn group_id(&self) -> &GroupId { - self.group.group_id() + self.public_group.group_id() } /// Returns the epoch. pub fn epoch(&self) -> GroupEpoch { - self.group.context().epoch() + self.public_group.group_context().epoch() } /// Returns an `Iterator` over pending proposals. @@ -326,7 +400,7 @@ impl MlsGroup { /// Get a reference to the group context [`Extensions`] of this [`MlsGroup`]. pub fn extensions(&self) -> &Extensions { - self.group.public_group().group_context().extensions() + self.public_group().group_context().extensions() } /// Returns the index of the sender of a staged, external commit. @@ -334,7 +408,7 @@ impl MlsGroup { &self, commit: &StagedCommit, ) -> Result { - self.group.public_group().ext_commit_sender_index(commit) + self.public_group().ext_commit_sender_index(commit) } // === Storage Methods === @@ -344,24 +418,25 @@ impl MlsGroup { storage: &Storage, group_id: &GroupId, ) -> Result, Storage::Error> { - let group_config = storage.mls_group_join_config(group_id)?; - let core_group = CoreGroup::load( - storage, - group_id, - group_config - .as_ref() - .map(|config: &MlsGroupJoinConfig| config.use_ratchet_tree_extension), - )?; + let public_group = PublicGroup::load(storage, group_id)?; + let group_epoch_secrets = storage.group_epoch_secrets(group_id)?; + let own_leaf_index = storage.own_leaf_index(group_id)?; + let message_secrets_store = storage.message_secrets(group_id)?; + let resumption_psk_store = storage.resumption_psk_store(group_id)?; + let mls_group_config = storage.mls_group_join_config(group_id)?; let own_leaf_nodes = storage.own_leaf_nodes(group_id)?; - let aad = Vec::new(); let group_state = storage.group_state(group_id)?; let build = || -> Option { Some(Self { - mls_group_config: group_config?, - group: core_group?, + public_group: public_group?, + group_epoch_secrets: group_epoch_secrets?, + own_leaf_index: own_leaf_index?, + message_secrets_store: message_secrets_store?, + resumption_psk_store: resumption_psk_store?, + mls_group_config: mls_group_config?, own_leaf_nodes, - aad, + aad: vec![], group_state: group_state?, }) }; @@ -376,11 +451,17 @@ impl MlsGroup { &mut self, storage: &Storage, ) -> Result<(), Storage::Error> { - self.group.delete(storage)?; + PublicGroup::delete(storage, self.group_id())?; + storage.delete_own_leaf_index(self.group_id())?; + storage.delete_group_epoch_secrets(self.group_id())?; + storage.delete_message_secrets(self.group_id())?; + storage.delete_all_resumption_psk_secrets(self.group_id())?; 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.clear_proposal_queue::(self.group_id())?; + + self.proposal_store_mut().empty(); storage.delete_encryption_epoch_key_pairs( self.group_id(), &self.epoch(), @@ -396,12 +477,253 @@ impl MlsGroup { /// Exports the Ratchet Tree. pub fn export_ratchet_tree(&self) -> RatchetTree { - self.group.public_group().export_ratchet_tree() + self.public_group().export_ratchet_tree() + } +} + +// Crate-public functions +impl MlsGroup { + /// Get the required capabilities extension of this group. + pub(crate) fn required_capabilities(&self) -> Option<&RequiredCapabilitiesExtension> { + self.public_group.required_capabilities() + } + + /// Get a reference to the group epoch secrets from the group + pub(crate) fn group_epoch_secrets(&self) -> &GroupEpochSecrets { + &self.group_epoch_secrets + } + + /// Get a reference to the message secrets from a group + pub(crate) fn message_secrets(&self) -> &MessageSecrets { + self.message_secrets_store.message_secrets() + } + + /// Sets the size of the [`MessageSecretsStore`], i.e. the number of past + /// epochs to keep. + /// This allows application messages from previous epochs to be decrypted. + pub(crate) fn set_max_past_epochs(&mut self, max_past_epochs: usize) { + self.message_secrets_store.resize(max_past_epochs); + } + + /// Get the message secrets. Either from the secrets store or from the group. + pub(crate) fn message_secrets_mut( + &mut self, + epoch: GroupEpoch, + ) -> Result<&mut MessageSecrets, SecretTreeError> { + if epoch < self.context().epoch() { + self.message_secrets_store + .secrets_for_epoch_mut(epoch) + .ok_or(SecretTreeError::TooDistantInThePast) + } else { + Ok(self.message_secrets_store.message_secrets_mut()) + } + } + + /// Get the message secrets. Either from the secrets store or from the group. + pub(crate) fn message_secrets_for_epoch( + &self, + epoch: GroupEpoch, + ) -> Result<&MessageSecrets, SecretTreeError> { + if epoch < self.context().epoch() { + self.message_secrets_store + .secrets_for_epoch(epoch) + .ok_or(SecretTreeError::TooDistantInThePast) + } else { + Ok(self.message_secrets_store.message_secrets()) + } + } + + /// Get the message secrets and leaves for the given epoch. Either from the + /// secrets store or from the group. + /// + /// Note that the leaves vector is empty for message secrets of the current + /// epoch. The caller can use treesync in this case. + pub(crate) fn message_secrets_and_leaves_mut( + &mut self, + epoch: GroupEpoch, + ) -> Result<(&mut MessageSecrets, &[Member]), MessageDecryptionError> { + if epoch < self.context().epoch() { + self.message_secrets_store + .secrets_and_leaves_for_epoch_mut(epoch) + .ok_or({ + MessageDecryptionError::SecretTreeError(SecretTreeError::TooDistantInThePast) + }) + } else { + // No need for leaves here. The tree of the current epoch is + // available to the caller. + Ok((self.message_secrets_store.message_secrets_mut(), &[])) + } + } + + /// Create a new group context extension proposal + pub(crate) fn create_group_context_ext_proposal( + &self, + framing_parameters: FramingParameters, + extensions: Extensions, + signer: &impl Signer, + ) -> Result> + { + // Ensure that the group supports all the extensions that are wanted. + let required_extension = extensions + .iter() + .find(|extension| extension.extension_type() == ExtensionType::RequiredCapabilities); + if let Some(required_extension) = required_extension { + let required_capabilities = required_extension.as_required_capabilities_extension()?; + // Ensure we support all the capabilities. + self.own_leaf_node() + .ok_or_else(|| LibraryError::custom("Tree has no own leaf."))? + .capabilities() + .supports_required_capabilities(required_capabilities)?; + + // Ensure that all other leaf nodes support all the required + // extensions as well. + self.public_group() + .check_extension_support(required_capabilities.extension_types())?; + } + let proposal = GroupContextExtensionProposal::new(extensions); + let proposal = Proposal::GroupContextExtensions(proposal); + AuthenticatedContent::member_proposal( + framing_parameters, + self.own_leaf_index(), + proposal, + self.context(), + signer, + ) + .map_err(|e| e.into()) + } + + // Encrypt an AuthenticatedContent into an PrivateMessage + pub(crate) fn encrypt( + &mut self, + public_message: AuthenticatedContent, + provider: &Provider, + ) -> Result> { + let padding_size = self.configuration().padding_size(); + let msg = PrivateMessage::try_from_authenticated_content( + &public_message, + self.ciphersuite(), + provider, + self.message_secrets_store.message_secrets_mut(), + padding_size, + )?; + + provider + .storage() + .write_message_secrets(self.group_id(), &self.message_secrets_store) + .map_err(MessageEncryptionError::StorageError)?; + + Ok(msg) + } + + /// Group framing parameters + pub(crate) fn framing_parameters(&self) -> FramingParameters { + FramingParameters::new( + &self.aad, + self.mls_group_config.wire_format_policy().outgoing(), + ) + } + + /// Returns a reference to the proposal store. + pub(crate) fn proposal_store(&self) -> &ProposalStore { + self.public_group.proposal_store() + } + + /// Returns a mutable reference to the proposal store. + pub(crate) fn proposal_store_mut(&mut self) -> &mut ProposalStore { + self.public_group.proposal_store_mut() + } + + /// Get the group context + pub(crate) fn context(&self) -> &GroupContext { + self.public_group.group_context() + } + + /// Get the MLS version used in this group. + pub(crate) fn version(&self) -> ProtocolVersion { + self.public_group.version() + } + + /// Resets the AAD. + #[inline] + pub(crate) fn reset_aad(&mut self) { + self.aad.clear(); + } + + /// Returns a reference to the public group. + pub(crate) fn public_group(&self) -> &PublicGroup { + &self.public_group } } // Private methods of MlsGroup impl MlsGroup { + /// Store the given [`EncryptionKeyPair`]s in the `provider`'s key store + /// indexed by this group's [`GroupId`] and [`GroupEpoch`]. + /// + /// Returns an error if access to the key store fails. + pub(super) fn store_epoch_keypairs( + &self, + store: &Storage, + keypair_references: &[EncryptionKeyPair], + ) -> Result<(), Storage::Error> { + store.write_encryption_epoch_key_pairs( + self.group_id(), + &self.context().epoch(), + self.own_leaf_index().u32(), + keypair_references, + ) + } + + /// Read the [`EncryptionKeyPair`]s of this group and its current + /// [`GroupEpoch`] from the `provider`'s storage. + /// + /// Returns an empty vector if access to the store fails or it can't find + /// any keys. + pub(super) fn read_epoch_keypairs( + &self, + store: &Storage, + ) -> Vec { + store + .encryption_epoch_key_pairs( + self.group_id(), + &self.context().epoch(), + self.own_leaf_index().u32(), + ) + .unwrap_or_default() + } + + /// Delete the [`EncryptionKeyPair`]s from the previous [`GroupEpoch`] from + /// the `provider`'s key store. + /// + /// Returns an error if access to the key store fails. + pub(super) fn delete_previous_epoch_keypairs( + &self, + store: &Storage, + ) -> Result<(), Storage::Error> { + store.delete_encryption_epoch_key_pairs( + self.group_id(), + &GroupEpoch::from(self.context().epoch().as_u64() - 1), + self.own_leaf_index().u32(), + ) + } + + /// Stores the state of this group. Only to be called from constructors to + /// store the initial state of the group. + pub(super) fn store( + &self, + storage: &Storage, + ) -> Result<(), Storage::Error> { + self.public_group.store(storage)?; + storage.write_group_epoch_secrets(self.group_id(), &self.group_epoch_secrets)?; + storage.write_own_leaf_index(self.group_id(), &self.own_leaf_index)?; + storage.write_message_secrets(self.group_id(), &self.message_secrets_store)?; + storage.write_resumption_psk_store(self.group_id(), &self.resumption_psk_store)?; + storage.write_mls_join_config(self.group_id(), &self.mls_group_config)?; + storage.write_group_state(self.group_id(), &self.group_state)?; + + Ok(()) + } + /// Converts PublicMessage to MlsMessage. Depending on whether handshake /// message should be encrypted, PublicMessage messages are encrypted to /// PrivateMessage first. @@ -418,36 +740,23 @@ impl MlsGroup { plaintext.set_membership_tag( provider.crypto(), self.ciphersuite(), - self.group.message_secrets().membership_key(), - self.group.message_secrets().serialized_context(), + self.message_secrets().membership_key(), + self.message_secrets().serialized_context(), )?; } plaintext.into() } OutgoingWireFormatPolicy::AlwaysCiphertext => { let ciphertext = self - .group - .encrypt( - mls_auth_content, - self.configuration().padding_size(), - provider, - ) + .encrypt(mls_auth_content, provider) // We can be sure the encryption will work because the plaintext was created by us .map_err(|_| LibraryError::custom("Malformed plaintext"))?; - MlsMessageOut::from_private_message(ciphertext, self.group.version()) + MlsMessageOut::from_private_message(ciphertext, self.version()) } }; Ok(msg) } - /// Group framing parameters - pub(crate) fn framing_parameters(&self) -> FramingParameters { - FramingParameters::new( - &self.aad, - self.mls_group_config.wire_format_policy().outgoing(), - ) - } - /// Check if the group is operational. Throws an error if the group is /// inactive or if there is a pending commit. fn is_operational(&self) -> Result<(), MlsGroupStateError> { @@ -457,89 +766,58 @@ impl MlsGroup { MlsGroupState::Operational => Ok(()), } } - - /// Returns a reference to the proposal store. - pub(crate) fn proposal_store(&self) -> &ProposalStore { - self.group.proposal_store() - } - - /// Returns a mutable reference to the proposal store. - pub(crate) fn proposal_store_mut(&mut self) -> &mut ProposalStore { - self.group.proposal_store_mut() - } - - /// Resets the AAD. - #[inline] - pub(crate) fn reset_aad(&mut self) { - self.aad.clear(); - } } // Methods used in tests impl MlsGroup { #[cfg(any(feature = "test-utils", test))] pub fn export_group_context(&self) -> &GroupContext { - self.group.context() + self.context() } #[cfg(any(feature = "test-utils", test))] pub fn tree_hash(&self) -> &[u8] { - self.group.public_group().group_context().tree_hash() + self.public_group().group_context().tree_hash() } #[cfg(any(feature = "test-utils", test))] pub(crate) fn message_secrets_test_mut(&mut self) -> &mut MessageSecrets { - self.group.message_secrets_test_mut() + self.message_secrets_store.message_secrets_mut() } #[cfg(any(feature = "test-utils", test))] pub fn print_ratchet_tree(&self, message: &str) { - self.group.print_ratchet_tree(message) + println!("{}: {}", message, self.public_group().export_ratchet_tree()); } #[cfg(any(feature = "test-utils", test))] pub(crate) fn context_mut(&mut self) -> &mut GroupContext { - self.group.context_mut() + self.public_group.context_mut() } - // Encrypt an AuthenticatedContent into a PrivateMessage. Only needed for - // the message protection KAT. #[cfg(test)] - pub(crate) fn encrypt( - &mut self, - public_message: AuthenticatedContent, - padding_size: usize, - provider: &Provider, - ) -> Result> { - self.group.encrypt(public_message, padding_size, provider) + pub(crate) fn set_own_leaf_index(&mut self, own_leaf_index: LeafNodeIndex) { + self.own_leaf_index = own_leaf_index; } #[cfg(test)] - // Decrypt a ProtocolMessage. Only needed for the message protection KAT. - pub(crate) fn decrypt_message( - &mut self, - crypto: &impl OpenMlsCrypto, - message: ProtocolMessage, - sender_ratchet_configuration: &SenderRatchetConfiguration, - ) -> Result { - self.group - .decrypt_message(crypto, message, sender_ratchet_configuration) + pub(crate) fn own_tree_position(&self) -> TreePosition { + TreePosition::new(self.group_id().clone(), self.own_leaf_index()) } #[cfg(test)] - pub(crate) fn set_group_context(&mut self, group_context: GroupContext) { - self.group.set_group_context(group_context) + pub(crate) fn message_secrets_store(&self) -> &MessageSecretsStore { + &self.message_secrets_store } #[cfg(test)] - pub(crate) fn set_own_leaf_index(&mut self, own_leaf_index: LeafNodeIndex) { - self.group.set_own_leaf_index(own_leaf_index) + pub(crate) fn resumption_psk_store(&self) -> &ResumptionPskStore { + &self.resumption_psk_store } - /// Returns the underlying [CoreGroup]. #[cfg(test)] - pub(crate) fn group(&self) -> &CoreGroup { - &self.group + pub(crate) fn set_group_context(&mut self, group_context: GroupContext) { + self.public_group.set_group_context(group_context) } } @@ -549,9 +827,29 @@ impl MlsGroup { pub struct StagedWelcome { // The group configuration. See [`MlsGroupJoinConfig`] for more information. mls_group_config: MlsGroupJoinConfig, - // The internal `CoreGroup` used for lower level operations. See `CoreGroup` for more - // information. - group: StagedCoreWelcome, + public_group: PublicGroup, + group_epoch_secrets: GroupEpochSecrets, + own_leaf_index: LeafNodeIndex, + + /// A [`MessageSecretsStore`] that stores message secrets. + /// By default this store has the length of 1, i.e. only the [`MessageSecrets`] + /// of the current epoch is kept. + /// If more secrets from past epochs should be kept in order to be + /// able to decrypt application messages from previous epochs, the size of + /// the store must be increased through [`max_past_epochs()`]. + message_secrets_store: MessageSecretsStore, + + /// Resumption psk store. This is where the resumption psks are kept in a rollover list. + resumption_psk_store: ResumptionPskStore, + + /// The [`VerifiableGroupInfo`] from the [`Welcome`] message. + verifiable_group_info: VerifiableGroupInfo, + + /// The key package bundle used for this welcome. + key_package_bundle: KeyPackageBundle, + + /// If we got a path secret, these are the derived path keys. + path_keypairs: Option>, } /// A `Welcome` message that has been processed but not staged yet. diff --git a/openmls/src/group/core_group/past_secrets.rs b/openmls/src/group/mls_group/past_secrets.rs similarity index 100% rename from openmls/src/group/core_group/past_secrets.rs rename to openmls/src/group/mls_group/past_secrets.rs diff --git a/openmls/src/group/mls_group/processing.rs b/openmls/src/group/mls_group/processing.rs index 4b297a3e2..6d098f636 100644 --- a/openmls/src/group/mls_group/processing.rs +++ b/openmls/src/group/mls_group/processing.rs @@ -2,16 +2,17 @@ use std::mem; -use core_group::staged_commit::StagedCommit; -use openmls_traits::{signatures::Signer, storage::StorageProvider as _}; +use errors::{CommitToPendingProposalsError, MergePendingCommitError}; +use openmls_traits::{crypto::OpenMlsCrypto, signatures::Signer, storage::StorageProvider as _}; -use crate::storage::OpenMlsProvider; use crate::{ - group::core_group::create_commit_params::CreateCommitParams, messages::group_info::GroupInfo, + framing::mls_content::FramedContentBody, + group::{errors::MergeCommitError, StageCommitError, ValidationError}, + messages::group_info::GroupInfo, + storage::OpenMlsProvider, + tree::sender_ratchet::SenderRatchetConfiguration, }; -use crate::group::errors::MergeCommitError; - use super::{errors::ProcessMessageError, *}; impl MlsGroup { @@ -52,11 +53,33 @@ impl MlsGroup { // Parse the message let sender_ratchet_configuration = self.configuration().sender_ratchet_configuration().clone(); - self.group.process_message( + + // Checks the following semantic validation: + // - ValSem002 + // - ValSem003 + // - ValSem006 + // - ValSem007 MembershipTag presence + let decrypted_message = + self.decrypt_message(provider.crypto(), message, &sender_ratchet_configuration)?; + + let unverified_message = self + .public_group + .parse_message(decrypted_message, &self.message_secrets_store) + .map_err(ProcessMessageError::from)?; + + // If this is a commit, we need to load the private key material we need for decryption. + let (old_epoch_keypairs, leaf_node_keypairs) = + if let ContentType::Commit = unverified_message.content_type() { + self.read_decryption_keypairs(provider, &self.own_leaf_nodes)? + } else { + (vec![], vec![]) + }; + + self.process_unverified_message( provider, - message, - &sender_ratchet_configuration, - &self.own_leaf_nodes, + unverified_message, + old_epoch_keypairs, + leaf_node_keypairs, ) } @@ -99,7 +122,7 @@ impl MlsGroup { let params = CreateCommitParams::builder() .framing_parameters(self.framing_parameters()) .build(); - let create_commit_result = self.group.create_commit(params, provider, signer)?; + let create_commit_result = self.create_commit(params, provider, signer)?; // Convert PublicMessage messages to MLSMessage and encrypt them if required by // the configuration @@ -120,7 +143,7 @@ impl MlsGroup { mls_message, create_commit_result .welcome_option - .map(|w| MlsMessageOut::from_welcome(w, self.group.version())), + .map(|w| MlsMessageOut::from_welcome(w, self.version())), create_commit_result.group_info, )) } @@ -142,16 +165,15 @@ impl MlsGroup { .map_err(MergeCommitError::StorageError)?; // Merge staged commit - self.group.merge_commit(provider, staged_commit)?; + self.merge_commit(provider, staged_commit)?; // Extract and store the resumption psk for the current epoch - let resumption_psk = self.group.group_epoch_secrets().resumption_psk(); - self.group - .resumption_psk_store - .add(self.group.context().epoch(), resumption_psk.clone()); + let resumption_psk = self.group_epoch_secrets().resumption_psk(); + self.resumption_psk_store + .add(self.context().epoch(), resumption_psk.clone()); provider .storage() - .write_resumption_psk_store(self.group_id(), &self.group.resumption_psk_store) + .write_resumption_psk_store(self.group_id(), &self.resumption_psk_store) .map_err(MergeCommitError::StorageError)?; // Delete own KeyPackageBundles @@ -186,4 +208,203 @@ impl MlsGroup { MlsGroupState::Operational => Ok(()), } } + + /// Helper function to read decryption keypairs. + pub(super) fn read_decryption_keypairs( + &self, + provider: &impl OpenMlsProvider, + own_leaf_nodes: &[LeafNode], + ) -> Result<(Vec, Vec), StageCommitError> { + // All keys from the previous epoch are potential decryption keypairs. + let old_epoch_keypairs = self.read_epoch_keypairs(provider.storage()); + + // If we are processing an update proposal that originally came from + // us, the keypair corresponding to the leaf in the update is also a + // potential decryption keypair. + let leaf_node_keypairs = own_leaf_nodes + .iter() + .map(|leaf_node| { + EncryptionKeyPair::read(provider, leaf_node.encryption_key()) + .ok_or(StageCommitError::MissingDecryptionKey) + }) + .collect::, StageCommitError>>()?; + + Ok((old_epoch_keypairs, leaf_node_keypairs)) + } + + /// This processing function does most of the semantic verifications. + /// It returns a [ProcessedMessage] enum. + /// Checks the following semantic validation: + /// - ValSem008 + /// - ValSem010 + /// - ValSem101 + /// - ValSem102 + /// - ValSem104 + /// - ValSem106 + /// - ValSem107 + /// - ValSem108 + /// - ValSem110 + /// - ValSem111 + /// - ValSem112 + /// - ValSem113: All Proposals: The proposal type must be supported by all + /// members of the group + /// - ValSem200 + /// - ValSem201 + /// - ValSem202: Path must be the right length + /// - ValSem203: Path secrets must decrypt correctly + /// - ValSem204: Public keys from Path must be verified and match the + /// private keys from the direct path + /// - ValSem205 + /// - ValSem240 + /// - ValSem241 + /// - ValSem242 + /// - ValSem244 + /// - ValSem246 (as part of ValSem010) + pub(crate) fn process_unverified_message( + &self, + provider: &Provider, + unverified_message: UnverifiedMessage, + old_epoch_keypairs: Vec, + leaf_node_keypairs: Vec, + ) -> Result { + // Checks the following semantic validation: + // - ValSem010 + // - ValSem246 (as part of ValSem010) + let (content, credential) = + unverified_message.verify(self.ciphersuite(), provider.crypto(), self.version())?; + + match content.sender() { + Sender::Member(_) | Sender::NewMemberCommit | Sender::NewMemberProposal => { + let sender = content.sender().clone(); + let authenticated_data = content.authenticated_data().to_owned(); + + let content = match content.content() { + FramedContentBody::Application(application_message) => { + ProcessedMessageContent::ApplicationMessage(ApplicationMessage::new( + application_message.as_slice().to_owned(), + )) + } + FramedContentBody::Proposal(_) => { + let proposal = Box::new(QueuedProposal::from_authenticated_content_by_ref( + self.ciphersuite(), + provider.crypto(), + content, + )?); + + if matches!(sender, Sender::NewMemberProposal) { + ProcessedMessageContent::ExternalJoinProposalMessage(proposal) + } else { + ProcessedMessageContent::ProposalMessage(proposal) + } + } + FramedContentBody::Commit(_) => { + let staged_commit = self.stage_commit( + &content, + old_epoch_keypairs, + leaf_node_keypairs, + provider, + )?; + ProcessedMessageContent::StagedCommitMessage(Box::new(staged_commit)) + } + }; + + Ok(ProcessedMessage::new( + self.group_id().clone(), + self.context().epoch(), + sender, + authenticated_data, + content, + credential, + )) + } + Sender::External(_) => { + let sender = content.sender().clone(); + let data = content.authenticated_data().to_owned(); + match content.content() { + FramedContentBody::Application(_) => { + Err(ProcessMessageError::UnauthorizedExternalApplicationMessage) + } + FramedContentBody::Proposal(Proposal::Remove(_)) => { + let content = ProcessedMessageContent::ProposalMessage(Box::new( + QueuedProposal::from_authenticated_content_by_ref( + self.ciphersuite(), + provider.crypto(), + content, + )?, + )); + Ok(ProcessedMessage::new( + self.group_id().clone(), + self.context().epoch(), + sender, + data, + content, + credential, + )) + } + // TODO #151/#106 + FramedContentBody::Proposal(_) => { + Err(ProcessMessageError::UnsupportedProposalType) + } + FramedContentBody::Commit(_) => unimplemented!(), + } + } + } + } + + /// Performs framing validation and, if necessary, decrypts the given message. + /// + /// Returns the [`DecryptedMessage`] if processing is successful, or a + /// [`ValidationError`] if it is not. + /// + /// Checks the following semantic validation: + /// - ValSem002 + /// - ValSem003 + /// - ValSem006 + /// - ValSem007 MembershipTag presence + pub(crate) fn decrypt_message( + &mut self, + crypto: &impl OpenMlsCrypto, + message: ProtocolMessage, + sender_ratchet_configuration: &SenderRatchetConfiguration, + ) -> Result { + // Checks the following semantic validation: + // - ValSem002 + // - ValSem003 + self.public_group.validate_framing(&message)?; + + let epoch = message.epoch(); + + // Checks the following semantic validation: + // - ValSem006 + // - ValSem007 MembershipTag presence + match message { + ProtocolMessage::PublicMessage(public_message) => { + // If the message is older than the current epoch, we need to fetch the correct secret tree first. + let message_secrets = + self.message_secrets_for_epoch(epoch).map_err(|e| match e { + SecretTreeError::TooDistantInThePast => ValidationError::NoPastEpochData, + _ => LibraryError::custom( + "Unexpected error while retrieving message secrets for epoch.", + ) + .into(), + })?; + DecryptedMessage::from_inbound_public_message( + public_message, + message_secrets, + message_secrets.serialized_context().to_vec(), + crypto, + self.ciphersuite(), + ) + } + ProtocolMessage::PrivateMessage(ciphertext) => { + // If the message is older than the current epoch, we need to fetch the correct secret tree first + DecryptedMessage::from_inbound_ciphertext( + ciphertext, + crypto, + self, + sender_ratchet_configuration, + ) + } + } + } } diff --git a/openmls/src/group/mls_group/proposal.rs b/openmls/src/group/mls_group/proposal.rs index bf1fdb3de..a8ce08688 100644 --- a/openmls/src/group/mls_group/proposal.rs +++ b/openmls/src/group/mls_group/proposal.rs @@ -1,24 +1,25 @@ use openmls_traits::{signatures::Signer, storage::StorageProvider as _, types::Ciphersuite}; use super::{ - core_group::create_commit_params::CreateCommitParams, - errors::{ProposalError, ProposeAddMemberError, ProposeRemoveMemberError}, - CreateGroupContextExtProposalError, CustomProposal, GroupContextExtensionProposal, MlsGroup, - MlsGroupState, PendingCommitState, Proposal, RemoveProposalError, + create_commit::CreateCommitParams, + errors::{ProposalError, ProposeAddMemberError, ProposeRemoveMemberError, RemoveProposalError}, + AddProposal, CreateGroupContextExtProposalError, CustomProposal, FramingParameters, + GroupContextExtensionProposal, MlsGroup, MlsGroupState, PendingCommitState, + PreSharedKeyProposal, Proposal, QueuedProposal, RemoveProposal, UpdateProposal, }; use crate::{ binary_tree::LeafNodeIndex, ciphersuite::hash_ref::ProposalRef, credentials::Credential, extensions::Extensions, - framing::MlsMessageOut, - group::{errors::CreateAddProposalError, GroupId, QueuedProposal}, + framing::{mls_auth_content::AuthenticatedContent, MlsMessageOut}, + group::{errors::CreateAddProposalError, GroupId, ValidationError}, key_packages::KeyPackage, messages::{group_info::GroupInfo, proposals::ProposalOrRefType}, prelude::LibraryError, schedule::PreSharedKeyId, storage::{OpenMlsProvider, StorageProvider}, - treesync::LeafNodeParameters, + treesync::{LeafNode, LeafNodeParameters}, versions::ProtocolVersion, }; @@ -72,9 +73,7 @@ macro_rules! impl_propose_fun { ) -> Result<(MlsMessageOut, ProposalRef), ProposalError> { self.is_operational()?; - let proposal = self - .group - .$group_fun(self.framing_parameters(), value, signer)?; + let proposal = self.$group_fun(self.framing_parameters(), value, signer)?; let queued_proposal = QueuedProposal::from_authenticated_content( self.ciphersuite(), @@ -87,7 +86,7 @@ macro_rules! impl_propose_fun { log::trace!("Storing proposal in queue {:?}", queued_proposal); provider .storage() - .queue_proposal(self.group.group_id(), &proposal_ref, &queued_proposal) + .queue_proposal(self.group_id(), &proposal_ref, &queued_proposal) .map_err(ProposalError::StorageError)?; self.proposal_store_mut().add(queued_proposal); @@ -238,7 +237,6 @@ impl MlsGroup { self.is_operational()?; let add_proposal = self - .group .create_add_proposal(self.framing_parameters(), key_package.clone(), signer) .map_err(|e| match e { CreateAddProposalError::LibraryError(e) => e.into(), @@ -279,7 +277,6 @@ impl MlsGroup { self.is_operational()?; let remove_proposal = self - .group .create_remove_proposal(self.framing_parameters(), member, signer) .map_err(|_| ProposeRemoveMemberError::UnknownMember)?; @@ -314,7 +311,6 @@ impl MlsGroup { { // Find the user for the credential first. let member_index = self - .group .public_group() .members() .find(|m| &m.credential == member) @@ -339,7 +335,6 @@ impl MlsGroup { ) -> Result<(MlsMessageOut, ProposalRef), ProposalError> { // Find the user for the credential first. let member_index = self - .group .public_group() .members() .find(|m| &m.credential == member) @@ -366,7 +361,7 @@ impl MlsGroup { ) -> Result<(MlsMessageOut, ProposalRef), ProposalError> { self.is_operational()?; - let proposal = self.group.create_group_context_ext_proposal::( + let proposal = self.create_group_context_ext_proposal::( self.framing_parameters(), extensions, signer, @@ -420,7 +415,7 @@ impl MlsGroup { .framing_parameters(self.framing_parameters()) .inline_proposals(inline_proposals) .build(); - let create_commit_result = self.group.create_commit(params, provider, signer)?; + let create_commit_result = self.create_commit(params, provider, signer)?; let mls_messages = self.content_to_mls_message(create_commit_result.commit, provider)?; @@ -440,7 +435,7 @@ impl MlsGroup { mls_messages, create_commit_result .welcome_option - .map(|w| MlsMessageOut::from_welcome(w, self.group.version())), + .map(|w| MlsMessageOut::from_welcome(w, self.version())), create_commit_result.group_info, )) } @@ -458,4 +453,122 @@ impl MlsGroup { .remove(proposal_ref) .ok_or(RemoveProposalError::ProposalNotFound) } + + // === Create handshake messages === + + // 12.1.1. Add + // struct { + // KeyPackage key_package; + // } Add; + pub(crate) fn create_add_proposal( + &self, + framing_parameters: FramingParameters, + joiner_key_package: KeyPackage, + signer: &impl Signer, + ) -> Result { + if let Some(required_capabilities) = self.required_capabilities() { + joiner_key_package + .leaf_node() + .capabilities() + .supports_required_capabilities(required_capabilities)?; + } + let add_proposal = AddProposal { + key_package: joiner_key_package, + }; + let proposal = Proposal::Add(add_proposal); + AuthenticatedContent::member_proposal( + framing_parameters, + self.own_leaf_index(), + proposal, + self.context(), + signer, + ) + .map_err(|e| e.into()) + } + + // 12.1.2. Update + // struct { + // LeafNode leaf_node; + // } Update; + pub(crate) fn create_update_proposal( + &self, + framing_parameters: FramingParameters, + // XXX: There's no need to own this. The [`UpdateProposal`] should + // operate on a reference to make this more efficient. + leaf_node: LeafNode, + signer: &impl Signer, + ) -> Result { + let update_proposal = UpdateProposal { leaf_node }; + let proposal = Proposal::Update(update_proposal); + AuthenticatedContent::member_proposal( + framing_parameters, + self.own_leaf_index(), + proposal, + self.context(), + signer, + ) + } + + // 12.1.3. Remove + // struct { + // uint32 removed; + // } Remove; + pub(crate) fn create_remove_proposal( + &self, + framing_parameters: FramingParameters, + removed: LeafNodeIndex, + signer: &impl Signer, + ) -> Result { + if self.public_group().leaf(removed).is_none() { + return Err(ValidationError::UnknownMember); + } + let remove_proposal = RemoveProposal { removed }; + let proposal = Proposal::Remove(remove_proposal); + AuthenticatedContent::member_proposal( + framing_parameters, + self.own_leaf_index(), + proposal, + self.context(), + signer, + ) + .map_err(ValidationError::LibraryError) + } + + // 12.1.4. PreSharedKey + // struct { + // PreSharedKeyID psk; + // } PreSharedKey; + // TODO: #751 + pub(crate) fn create_presharedkey_proposal( + &self, + framing_parameters: FramingParameters, + psk: PreSharedKeyId, + signer: &impl Signer, + ) -> Result { + let presharedkey_proposal = PreSharedKeyProposal::new(psk); + let proposal = Proposal::PreSharedKey(presharedkey_proposal); + AuthenticatedContent::member_proposal( + framing_parameters, + self.own_leaf_index(), + proposal, + self.context(), + signer, + ) + } + + pub(crate) fn create_custom_proposal( + &self, + framing_parameters: FramingParameters, + custom_proposal: CustomProposal, + signer: &impl Signer, + ) -> Result { + let proposal = Proposal::Custom(custom_proposal); + AuthenticatedContent::member_proposal( + framing_parameters, + self.own_leaf_index(), + proposal, + self.context(), + signer, + ) + } } diff --git a/openmls/src/group/core_group/proposals.rs b/openmls/src/group/mls_group/proposal_store.rs similarity index 100% rename from openmls/src/group/core_group/proposals.rs rename to openmls/src/group/mls_group/proposal_store.rs diff --git a/openmls/src/group/core_group/staged_commit.rs b/openmls/src/group/mls_group/staged_commit.rs similarity index 96% rename from openmls/src/group/core_group/staged_commit.rs rename to openmls/src/group/mls_group/staged_commit.rs index 76a55afaa..82a56c1f4 100644 --- a/openmls/src/group/core_group/staged_commit.rs +++ b/openmls/src/group/mls_group/staged_commit.rs @@ -1,20 +1,31 @@ use core::fmt::Debug; use std::mem; -use public_group::diff::{apply_proposals::ApplyProposalsValues, StagedPublicGroupDiff}; +use openmls_traits::storage::StorageProvider; +use serde::{Deserialize, Serialize}; +use tls_codec::Serialize as _; -use self::public_group::staged_commit::PublicStagedCommitState; +use super::proposal_store::{ + QueuedAddProposal, QueuedPskProposal, QueuedRemoveProposal, QueuedUpdateProposal, +}; -use super::{super::errors::*, *}; +use super::{ + super::errors::*, load_psks, Credential, Extension, GroupContext, GroupEpochSecrets, GroupId, + JoinerSecret, KeySchedule, LeafNode, LibraryError, MessageSecrets, MlsGroup, OpenMlsProvider, + Proposal, ProposalQueue, PskSecret, QueuedProposal, Sender, +}; use crate::{ ciphersuite::{hash_ref::ProposalRef, Secret}, framing::mls_auth_content::AuthenticatedContent, + group::public_group::{ + diff::{apply_proposals::ApplyProposalsValues, StagedPublicGroupDiff}, + staged_commit::PublicStagedCommitState, + }, + schedule::{CommitSecret, EpochAuthenticator, EpochSecrets, InitSecret, PreSharedKeyId}, treesync::node::encryption_keys::EncryptionKeyPair, }; -use openmls_traits::storage::StorageProvider as _; - -impl CoreGroup { +impl MlsGroup { fn derive_epoch_secrets( &self, provider: &impl OpenMlsProvider, @@ -96,7 +107,7 @@ impl CoreGroup { /// - Verifies the confirmation tag /// /// Returns a [StagedCommit] that can be inspected and later merged into the - /// group state with [CoreGroup::merge_commit()] This function does the + /// group state with [MlsGroup::merge_commit()] This function does the /// following checks: /// - ValSem101 /// - ValSem102 diff --git a/openmls/src/group/mls_group/tests_and_kats/tests/core_group.rs b/openmls/src/group/mls_group/tests_and_kats/tests/core_group.rs deleted file mode 100644 index 405453615..000000000 --- a/openmls/src/group/mls_group/tests_and_kats/tests/core_group.rs +++ /dev/null @@ -1,737 +0,0 @@ -use core::panic; - -use frankenstein::{FrankenFramedContentBody, FrankenPublicMessage}; -use mls_group::tests_and_kats::utils::{ - flip_last_byte, setup_alice_bob, setup_alice_bob_group, setup_client, -}; -use tls_codec::Serialize; - -use crate::{ - binary_tree::*, - ciphersuite::{signable::Signable, AeadNonce}, - credentials::*, - framing::*, - group::{errors::*, *}, - key_packages::*, - messages::{group_info::GroupInfoTBS, *}, - prelude::LeafNodeParameters, - schedule::psk::{ExternalPsk, PreSharedKeyId, Psk}, - test_utils::*, - treesync::errors::ApplyUpdatePathError, -}; - -#[openmls_test::openmls_test] -fn failed_groupinfo_decryption( - ciphersuite: Ciphersuite, - provider: &impl crate::storage::OpenMlsProvider, -) { - let epoch = 123; - let group_id = GroupId::random(provider.rand()); - let tree_hash = vec![1, 2, 3, 4, 5, 6, 7, 8, 9]; - let confirmed_transcript_hash = vec![1, 1, 1]; - let extensions = Extensions::empty(); - let confirmation_tag = ConfirmationTag(Mac { - mac_value: vec![1, 2, 3, 4, 5, 6, 7, 8, 9].into(), - }); - - // Create credentials and keys - let (alice_credential_with_key, alice_signature_keys) = - test_utils::new_credential(provider, b"Alice", ciphersuite.signature_algorithm()); - - let key_package_bundle = KeyPackageBundle::generate( - provider, - &alice_signature_keys, - ciphersuite, - alice_credential_with_key, - ); - - let group_info_tbs = { - let group_context = GroupContext::new( - ciphersuite, - group_id, - epoch, - tree_hash, - confirmed_transcript_hash, - Extensions::empty(), - ); - - GroupInfoTBS::new( - group_context, - extensions, - confirmation_tag, - LeafNodeIndex::new(0), - ) - }; - - // Generate key and nonce for the symmetric cipher. - let welcome_key = AeadKey::random(ciphersuite, provider.rand()); - let welcome_nonce = AeadNonce::random(provider.rand()); - - // Generate receiver key pair. - let receiver_key_pair = provider - .crypto() - .derive_hpke_keypair( - ciphersuite.hpke_config(), - Secret::random(ciphersuite, provider.rand()) - .expect("Not enough randomness.") - .as_slice(), - ) - .expect("error deriving receiver hpke key pair"); - let hpke_context = b"group info welcome test info"; - let group_secrets = b"these should be the group secrets"; - let mut encrypted_group_secrets = hpke::encrypt_with_label( - receiver_key_pair.public.as_slice(), - "Welcome", - hpke_context, - group_secrets, - ciphersuite, - provider.crypto(), - ) - .unwrap(); - - let group_info = group_info_tbs - .sign(&alice_signature_keys) - .expect("Error signing group info"); - - // Mess with the ciphertext by flipping the last byte. - flip_last_byte(&mut encrypted_group_secrets); - - let broken_secrets = vec![EncryptedGroupSecrets::new( - key_package_bundle - .key_package - .hash_ref(provider.crypto()) - .expect("Could not hash KeyPackage."), - encrypted_group_secrets, - )]; - - // Encrypt the group info. - let encrypted_group_info = welcome_key - .aead_seal( - provider.crypto(), - &group_info - .tls_serialize_detached() - .expect("An unexpected error occurred."), - &[], - &welcome_nonce, - ) - .expect("An unexpected error occurred."); - - // Now build the welcome message. - let broken_welcome = Welcome::new(ciphersuite, broken_secrets, encrypted_group_info); - - let error = StagedWelcome::new_from_welcome( - provider, - &MlsGroupJoinConfig::default(), - broken_welcome, - None, - ) - .and_then(|staged_join| staged_join.into_group(provider)) - .expect_err("Creation of mls group from a broken Welcome was successful."); - - assert!(matches!( - error, - WelcomeError::GroupSecrets(GroupSecretsError::DecryptionFailed) - )) -} - -/// Test what happens if the KEM ciphertext for the receiver in the UpdatePath -/// is broken. -#[openmls_test::openmls_test] -fn update_path() { - // === Alice creates a group with her and Bob === - let ( - mut group_alice, - _alice_signature_keys, - mut group_bob, - bob_signature_keys, - _bob_credential_with_key, - ) = setup_alice_bob_group(ciphersuite, provider); - - // === Bob updates and commits === - let mut bob_new_leaf_node = group_bob.own_leaf_node().unwrap().clone(); - bob_new_leaf_node - .update( - ciphersuite, - provider, - &bob_signature_keys, - group_bob.group_id().clone(), - group_bob.own_leaf_index(), - LeafNodeParameters::default(), - ) - .unwrap(); - - let (update_bob, _welcome_option, _group_info_option) = group_bob - .self_update(provider, &bob_signature_keys, LeafNodeParameters::default()) - .expect("Could not create proposal."); - - // Now we break Alice's HPKE ciphertext in Bob's commit by breaking - // apart the commit, manipulating the ciphertexts and the piecing it - // back together. - let pm = match update_bob.body { - mls_group::MlsMessageBodyOut::PublicMessage(pm) => pm, - _ => panic!("Wrong message type"), - }; - - let franken_pm = FrankenPublicMessage::from(pm.clone()); - let mut content = franken_pm.content.clone(); - let FrankenFramedContentBody::Commit(ref mut commit) = content.body else { - panic!("Unexpected content type"); - }; - let Some(ref mut path) = commit.path else { - panic!("No path in commit."); - }; - - for node in &mut path.nodes { - for eps in &mut node.encrypted_path_secrets { - let mut eps_ctxt_vec = Vec::::from(eps.ciphertext.clone()); - eps_ctxt_vec[0] ^= 0xff; - eps.ciphertext = eps_ctxt_vec.into(); - } - } - - // Rebuild the PublicMessage with the new content - let group_context = group_bob.export_group_context().clone(); - let membership_key = group_bob - .group() - .message_secrets() - .membership_key() - .as_slice(); - - let broken_message = FrankenPublicMessage::auth( - provider, - ciphersuite, - &bob_signature_keys, - content, - Some(&group_context.into()), - Some(membership_key), - Some(pm.confirmation_tag().unwrap().0.mac_value.clone()), - ); - - let protocol_message = - ProtocolMessage::PublicMessage(PublicMessage::from(broken_message).into()); - - let result = group_alice.process_message(provider, protocol_message); - assert_eq!( - result.expect_err("Successful processing of a broken commit."), - ProcessMessageError::InvalidCommit(StageCommitError::UpdatePathError( - ApplyUpdatePathError::UnableToDecrypt - )) - ); -} - -// Test several scenarios when PSKs are used in a group -#[openmls_test::openmls_test] -fn psks() { - // Basic group setup. - let ( - alice_credential_with_key, - alice_signature_keys, - bob_key_package_bundle, - bob_signature_keys, - ) = setup_alice_bob(ciphersuite, provider); - - // === Alice creates a group with a PSK === - let psk_id = vec![1u8, 2, 3]; - - let secret = Secret::random(ciphersuite, provider.rand()).expect("Not enough randomness."); - let external_psk = ExternalPsk::new(psk_id); - let preshared_key_id = - PreSharedKeyId::new(ciphersuite, provider.rand(), Psk::External(external_psk)) - .expect("An unexpected error occured."); - preshared_key_id.store(provider, secret.as_slice()).unwrap(); - let mut alice_group = MlsGroup::builder() - .ciphersuite(ciphersuite) - .with_wire_format_policy(PURE_PLAINTEXT_WIRE_FORMAT_POLICY) - .build(provider, &alice_signature_keys, alice_credential_with_key) - .expect("Error creating group."); - - // === Alice creates a PSK proposal === - log::info!(" >>> Creating psk proposal ..."); - let (_psk_proposal, _proposal_ref) = alice_group - .propose_external_psk(provider, &alice_signature_keys, preshared_key_id) - .expect("Could not create PSK proposal"); - - // === Alice adds Bob (and commits to PSK proposal) === - let (_commit, welcome, _group_info_option) = alice_group - .add_members( - provider, - &alice_signature_keys, - &[bob_key_package_bundle.key_package().clone()], - ) - .expect("Could not create commit"); - - log::info!(" >>> Merging commit ..."); - - alice_group - .merge_pending_commit(provider) - .expect("Could not merge commit"); - - let ratchet_tree = alice_group.export_ratchet_tree(); - - let mut bob_group = StagedWelcome::new_from_welcome( - provider, - &MlsGroupJoinConfig::default(), - welcome.into_welcome().unwrap(), - Some(ratchet_tree.into()), - ) - .expect("Could not stage welcome") - .into_group(provider) - .expect("Could not create group from welcome"); - - // === Bob updates and commits === - let (_commit, _welcome_option, _group_info_option) = bob_group - .self_update(provider, &bob_signature_keys, LeafNodeParameters::default()) - .expect("An unexpected error occurred."); -} - -// Test several scenarios when PSKs are used in a group -#[openmls_test::openmls_test] -fn staged_commit_creation( - ciphersuite: Ciphersuite, - provider: &impl crate::storage::OpenMlsProvider, -) { - // Basic group setup. - let (alice_credential_with_key, alice_signature_keys, bob_key_package_bundle, _) = - setup_alice_bob(ciphersuite, provider); - - // === Alice creates a group === - let mut alice_group = MlsGroup::builder() - .ciphersuite(ciphersuite) - .with_wire_format_policy(PURE_PLAINTEXT_WIRE_FORMAT_POLICY) - .build(provider, &alice_signature_keys, alice_credential_with_key) - .expect("Error creating group."); - - // === Alice adds Bob === - let (_commit, welcome, _group_info_option) = alice_group - .add_members( - provider, - &alice_signature_keys, - &[bob_key_package_bundle.key_package().clone()], - ) - .expect("Could not create commit"); - - alice_group - .merge_pending_commit(provider) - .expect("Could not merge commit"); - - let ratchet_tree = alice_group.export_ratchet_tree(); - - let bob_group = StagedWelcome::new_from_welcome( - provider, - &MlsGroupJoinConfig::default(), - welcome.into_welcome().unwrap(), - Some(ratchet_tree.into()), - ) - .expect("Could not stage welcome") - .into_group(provider) - .expect("Could not create group from welcome"); - - // Let's make sure we end up in the same group state. - assert_eq!( - bob_group.epoch_authenticator(), - alice_group.epoch_authenticator() - ); - assert_eq!( - bob_group.export_ratchet_tree(), - alice_group.export_ratchet_tree() - ) -} - -// Test processing of own commits -#[openmls_test::openmls_test] -fn own_commit_processing( - ciphersuite: Ciphersuite, - provider: &impl crate::storage::OpenMlsProvider, -) { - // Basic group setup. - let (alice_credential_with_key, alice_signature_keys) = - test_utils::new_credential(provider, b"Alice", ciphersuite.signature_algorithm()); - - // === Alice creates a group === - let mut alice_group = MlsGroup::builder() - .ciphersuite(ciphersuite) - .with_wire_format_policy(PURE_PLAINTEXT_WIRE_FORMAT_POLICY) - .build(provider, &alice_signature_keys, alice_credential_with_key) - .expect("Error creating group."); - - // Alice creates a commit - let (commit_out, _welcome_option, _group_info_option) = alice_group - .self_update( - provider, - &alice_signature_keys, - LeafNodeParameters::default(), - ) - .expect("Could not create commit"); - - let commit_in = MlsMessageIn::from(commit_out); - - // Alice attempts to process her own commit - let error = alice_group - .process_message(provider, commit_in.into_protocol_message().unwrap()) - .expect_err("no error while processing own commit"); - assert_eq!( - error, - ProcessMessageError::InvalidCommit(StageCommitError::OwnCommit) - ); -} - -#[openmls_test::openmls_test] -fn proposal_application_after_self_was_removed( - ciphersuite: Ciphersuite, - provider: &impl crate::storage::OpenMlsProvider, -) { - // We're going to test if proposals are still applied, even after a client - // notices that it was removed from a group. We do so by having Alice - // create a group, add Bob and then create a commit where Bob is removed and - // Charlie is added in a single commit (by Alice). We then check if - // everyone's membership list is as expected. - - // Basic group setup. - let (alice_credential_with_key, _, alice_signature_keys, _pk) = - setup_client("Alice", ciphersuite, provider); - let (_, bob_kpb, _, _) = setup_client("Bob", ciphersuite, provider); - let (_, charlie_kpb, _, _) = setup_client("Charlie", ciphersuite, provider); - - let join_group_config = MlsGroupJoinConfig::builder() - .wire_format_policy(PURE_PLAINTEXT_WIRE_FORMAT_POLICY) - .build(); - - let mut alice_group = MlsGroup::builder() - .ciphersuite(ciphersuite) - .with_wire_format_policy(PURE_PLAINTEXT_WIRE_FORMAT_POLICY) - .build(provider, &alice_signature_keys, alice_credential_with_key) - .expect("Error creating group."); - - let (_commit, welcome, _group_info_option) = alice_group - .add_members( - provider, - &alice_signature_keys, - &[bob_kpb.key_package().clone()], - ) - .expect("Could not create commit"); - - alice_group - .merge_pending_commit(provider) - .expect("Could not merge commit"); - - let ratchet_tree = alice_group.export_ratchet_tree(); - - let mut bob_group = StagedWelcome::new_from_welcome( - provider, - &join_group_config, - welcome.into_welcome().unwrap(), - Some(ratchet_tree.into()), - ) - .expect("Could not stage welcome") - .into_group(provider) - .expect("Could not create group from welcome"); - - // Alice adds Charlie and removes Bob in the same commit. - // She first creates a proposal to remove Bob - let bob_index = alice_group - .members() - .find( - |Member { - index: _, - credential, - .. - }| { credential.serialized_content() == b"Bob" }, - ) - .expect("Couldn't find Bob in tree.") - .index; - - assert_eq!(bob_index.u32(), 1); - - let (bob_remove_proposal, _bob_remove_proposal_ref) = alice_group - .propose_remove_member(provider, &alice_signature_keys, bob_index) - .expect("Could not create proposal"); - - // Bob processes the proposal - let processed_message = bob_group - .process_message( - provider, - bob_remove_proposal.into_protocol_message().unwrap(), - ) - .unwrap(); - - let staged_proposal = match processed_message.into_content() { - ProcessedMessageContent::ProposalMessage(proposal) => *proposal, - _ => panic!("Wrong message type"), - }; - - bob_group - .store_pending_proposal(provider.storage(), staged_proposal) - .expect("Error storing proposal"); - - // Alice then commit to the proposal and at the same time adds Charlie - let (commit, welcome, _group_info_option) = alice_group - .add_members( - provider, - &alice_signature_keys, - &[charlie_kpb.key_package().clone()], - ) - .expect("Could not create commit"); - - // Alice merges her own commit - alice_group - .merge_pending_commit(provider) - .expect("Could not merge commit"); - - // Bob processes the commit - println!("Bob processes the commit"); - let processed_message = bob_group - .process_message(provider, commit.into_protocol_message().unwrap()) - .unwrap(); - - let staged_commit = match processed_message.into_content() { - ProcessedMessageContent::StagedCommitMessage(commit) => *commit, - _ => panic!("Wrong message type"), - }; - - bob_group - .merge_staged_commit(provider, staged_commit) - .expect("Error merging commit."); - - // Charlie processes the welcome - println!("Charlie processes the commit"); - let ratchet_tree = alice_group.export_ratchet_tree(); - - let charlie_group = StagedWelcome::new_from_welcome( - provider, - &join_group_config, - welcome.into_welcome().unwrap(), - Some(ratchet_tree.into()), - ) - .expect("Error staging welcome.") - .into_group(provider) - .expect("Error creating group from welcome."); - - // We can now check that Bob correctly processed his commit and applied the changes - // to his tree after he was removed by comparing membership lists. In - // particular, Bob's list should show that he was removed and Charlie was - // added. - let alice_members = alice_group.members(); - - let bob_members = bob_group.members(); - - let charlie_members = charlie_group.members(); - - for (alice_member, (bob_member, charlie_member)) in - alice_members.zip(bob_members.zip(charlie_members)) - { - // Note that we can't compare encryption keys for Bob because they - // didn't get updated. - assert_eq!(alice_member.index, bob_member.index); - - let alice_id = alice_member.credential.serialized_content(); - let bob_id = bob_member.credential.serialized_content(); - let charlie_id = charlie_member.credential.serialized_content(); - assert_eq!(alice_id, bob_id); - assert_eq!(alice_member.signature_key, bob_member.signature_key); - assert_eq!(charlie_member.index, bob_member.index); - assert_eq!(charlie_id, bob_id); - assert_eq!(charlie_member.signature_key, bob_member.signature_key); - assert_eq!(charlie_member.encryption_key, alice_member.encryption_key); - } - - let mut bob_members = bob_group.members(); - - let member = bob_members.next().unwrap(); - let bob_next_id = member.credential.serialized_content(); - assert_eq!(bob_next_id, b"Alice"); - let member = bob_members.next().unwrap(); - let bob_next_id = member.credential.serialized_content(); - assert_eq!(bob_next_id, b"Charlie"); -} - -#[openmls_test::openmls_test] -fn proposal_application_after_self_was_removed_ref( - ciphersuite: Ciphersuite, - provider: &impl crate::storage::OpenMlsProvider, -) { - // We're going to test if proposals are still applied, even after a client - // notices that it was removed from a group. We do so by having Alice - // create a group, add Bob and then create a commit where Bob is removed and - // Charlie is added in a single commit (by Alice). We then check if - // everyone's membership list is as expected. - - // Basic group setup. - let (alice_credential_with_key, _, alice_signature_keys, _pk) = - setup_client("Alice", ciphersuite, provider); - let (_, bob_kpb, _, _) = setup_client("Bob", ciphersuite, provider); - let (_, charlie_kpb, _, _) = setup_client("Charlie", ciphersuite, provider); - - let join_group_config = MlsGroupJoinConfig::builder() - .wire_format_policy(PURE_PLAINTEXT_WIRE_FORMAT_POLICY) - .build(); - - let mut alice_group = MlsGroup::builder() - .ciphersuite(ciphersuite) - .with_wire_format_policy(PURE_PLAINTEXT_WIRE_FORMAT_POLICY) - .build(provider, &alice_signature_keys, alice_credential_with_key) - .expect("Error creating group."); - - let (_commit, welcome, _group_info_option) = alice_group - .add_members( - provider, - &alice_signature_keys, - &[bob_kpb.key_package().clone()], - ) - .expect("Could not create commit"); - - alice_group - .merge_pending_commit(provider) - .expect("Could not merge commit"); - - let ratchet_tree = alice_group.export_ratchet_tree(); - - let mut bob_group = StagedWelcome::new_from_welcome( - provider, - &join_group_config, - welcome.into_welcome().unwrap(), - Some(ratchet_tree.into()), - ) - .expect("Could not stage welcome") - .into_group(provider) - .expect("Could not create group from welcome"); - - // Alice adds Charlie and removes Bob in the same commit. - // She first creates a proposal to remove Bob - let bob_index = alice_group - .members() - .find( - |Member { - index: _, - credential, - .. - }| { credential.serialized_content() == b"Bob" }, - ) - .expect("Couldn't find Bob in tree.") - .index; - - assert_eq!(bob_index.u32(), 1); - - let (bob_remove_proposal, _bob_remove_proposal_ref) = alice_group - .propose_remove_member(provider, &alice_signature_keys, bob_index) - .expect("Could not create proposal"); - - let (charlie_add_proposal, _charlie_add_proposal_ref) = alice_group - .propose_add_member(provider, &alice_signature_keys, charlie_kpb.key_package()) - .expect("Could not create proposal"); - - // Bob processes the proposals - let processed_message = bob_group - .process_message( - provider, - bob_remove_proposal.into_protocol_message().unwrap(), - ) - .unwrap(); - - let staged_proposal = match processed_message.into_content() { - ProcessedMessageContent::ProposalMessage(proposal) => *proposal, - _ => panic!("Wrong message type"), - }; - - bob_group - .store_pending_proposal(provider.storage(), staged_proposal) - .expect("Error storing proposal"); - - let processed_message = bob_group - .process_message( - provider, - charlie_add_proposal.into_protocol_message().unwrap(), - ) - .unwrap(); - - let staged_proposal = match processed_message.into_content() { - ProcessedMessageContent::ProposalMessage(proposal) => *proposal, - _ => panic!("Wrong message type"), - }; - - bob_group - .store_pending_proposal(provider.storage(), staged_proposal) - .expect("Error storing proposal"); - - // Alice then commits to the proposal and at the same time adds Charlie - alice_group.print_ratchet_tree("Alice's tree before commit\n"); - let alice_rt_before = alice_group.export_ratchet_tree(); - let (commit, welcome, _group_info_option) = alice_group - .commit_to_pending_proposals(provider, &alice_signature_keys) - .expect("Could not create commit"); - - // Alice merges her own commit - alice_group - .merge_pending_commit(provider) - .expect("Could not merge commit"); - alice_group.print_ratchet_tree("Alice's tree after commit\n"); - - // Bob processes the commit - println!("Bob processes the commit"); - bob_group.print_ratchet_tree("Bob's tree before processing the commit\n"); - let bob_rt_before = bob_group.export_ratchet_tree(); - assert_eq!(alice_rt_before, bob_rt_before); - let processed_message = bob_group - .process_message(provider, commit.into_protocol_message().unwrap()) - .unwrap(); - println!("Bob finished processesing the commit"); - - let staged_commit = match processed_message.into_content() { - ProcessedMessageContent::StagedCommitMessage(commit) => *commit, - _ => panic!("Wrong message type"), - }; - - bob_group - .merge_staged_commit(provider, staged_commit) - .expect("Error merging commit."); - - // Charlie processes the welcome - println!("Charlie processes the commit"); - let ratchet_tree = alice_group.export_ratchet_tree(); - - let charlie_group = StagedWelcome::new_from_welcome( - provider, - &join_group_config, - welcome.unwrap().into_welcome().unwrap(), - Some(ratchet_tree.into()), - ) - .expect("Error staging welcome.") - .into_group(provider) - .expect("Error creating group from welcome."); - - // We can now check that Bob correctly processed his and applied the changes - // to his tree after he was removed by comparing membership lists. In - // particular, Bob's list should show that he was removed and Charlie was - // added. - let alice_members = alice_group.members(); - - let bob_members = bob_group.members(); - - let charlie_members = charlie_group.members(); - - for (alice_member, (bob_member, charlie_member)) in - alice_members.zip(bob_members.zip(charlie_members)) - { - // Note that we can't compare encryption keys for Bob because they - // didn't get updated. - assert_eq!(alice_member.index, bob_member.index); - - let alice_id = alice_member.credential.serialized_content(); - let bob_id = bob_member.credential.serialized_content(); - let charlie_id = charlie_member.credential.serialized_content(); - assert_eq!(alice_id, bob_id); - assert_eq!(alice_member.signature_key, bob_member.signature_key); - assert_eq!(charlie_member.index, bob_member.index); - assert_eq!(charlie_id, bob_id); - assert_eq!(charlie_member.signature_key, bob_member.signature_key); - assert_eq!(charlie_member.encryption_key, alice_member.encryption_key); - } - - let mut bob_members = bob_group.members(); - - let member = bob_members.next().unwrap(); - let bob_next_id = member.credential.serialized_content(); - assert_eq!(bob_next_id, b"Alice"); - let member = bob_members.next().unwrap(); - let bob_next_id = member.credential.serialized_content(); - assert_eq!(bob_next_id, b"Charlie"); -} diff --git a/openmls/src/group/mls_group/tests_and_kats/tests/create_commit_params.rs b/openmls/src/group/mls_group/tests_and_kats/tests/create_commit_params.rs index 876b8b94c..d160f7238 100644 --- a/openmls/src/group/mls_group/tests_and_kats/tests/create_commit_params.rs +++ b/openmls/src/group/mls_group/tests_and_kats/tests/create_commit_params.rs @@ -1,6 +1,6 @@ use crate::group::{ + create_commit::CreateCommitParams, mls_group::{FramingParameters, Proposal, WireFormat}, - CreateCommitParams, }; // Tests that the builder for CreateCommitParams works as expected 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 849885fb3..76a5b529f 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,23 +1,34 @@ -use mls_group::tests_and_kats::utils::setup_client; +use mls_group::tests_and_kats::utils::{ + flip_last_byte, setup_alice_bob, setup_alice_bob_group, setup_client, +}; use openmls_basic_credential::SignatureKeyPair; use openmls_rust_crypto::MemoryStorage; use openmls_test::openmls_test; use openmls_traits::{storage::CURRENT_VERSION, OpenMlsProvider as _}; +use signable::Signable; use tls_codec::{Deserialize, Serialize}; use crate::{ binary_tree::LeafNodeIndex, + credentials::test_utils::new_credential, extensions::errors::InvalidExtensionError, framing::*, group::{errors::*, *}, key_packages::*, - messages::proposals::*, - test_utils::test_framework::{ - errors::ClientError, noop_authentication_service, ActionType::Commit, CodecUse, - MlsGroupTestSetup, + messages::{ + group_info::GroupInfoTBS, proposals::*, EncryptedGroupSecrets, GroupSecretsError, Welcome, + }, + prelude::ConfirmationTag, + schedule::{ExternalPsk, PreSharedKeyId, Psk}, + test_utils::{ + frankenstein::{FrankenFramedContentBody, FrankenPublicMessage}, + test_framework::{ + errors::ClientError, noop_authentication_service, ActionType::Commit, CodecUse, + MlsGroupTestSetup, + }, }, tree::sender_ratchet::SenderRatchetConfiguration, - treesync::{node::leaf_node::Capabilities, LeafNodeParameters}, + treesync::{errors::ApplyUpdatePathError, node::leaf_node::Capabilities, LeafNodeParameters}, }; #[openmls_test] @@ -358,7 +369,7 @@ fn test_invalid_plaintext() { // Store the context and membership key so that we can re-compute the membership tag later. let client_groups = client.groups.read().unwrap(); let client_group = client_groups.get(&group_id).unwrap(); - let membership_key = client_group.group().message_secrets().membership_key(); + let membership_key = client_group.message_secrets().membership_key(); // Tamper with the message such that signature verification fails // Once #574 is addressed the new function from there should be used to manipulate the signature. @@ -379,7 +390,7 @@ fn test_invalid_plaintext() { client.provider.crypto(), ciphersuite, membership_key, - client_group.group().message_secrets().serialized_context(), + client_group.message_secrets().serialized_context(), ) .unwrap() } @@ -1298,7 +1309,7 @@ fn update_group_context_with_unknown_extension pm, + _ => panic!("Wrong message type"), + }; + + let franken_pm = FrankenPublicMessage::from(pm.clone()); + let mut content = franken_pm.content.clone(); + let FrankenFramedContentBody::Commit(ref mut commit) = content.body else { + panic!("Unexpected content type"); + }; + let Some(ref mut path) = commit.path else { + panic!("No path in commit."); + }; + + for node in &mut path.nodes { + for eps in &mut node.encrypted_path_secrets { + let mut eps_ctxt_vec = Vec::::from(eps.ciphertext.clone()); + eps_ctxt_vec[0] ^= 0xff; + eps.ciphertext = eps_ctxt_vec.into(); + } + } + + // Rebuild the PublicMessage with the new content + let group_context = group_bob.export_group_context().clone(); + let membership_key = group_bob.message_secrets().membership_key().as_slice(); + + let broken_message = FrankenPublicMessage::auth( + provider, + ciphersuite, + &bob_signature_keys, + content, + Some(&group_context.into()), + Some(membership_key), + Some(pm.confirmation_tag().unwrap().0.mac_value.clone()), + ); + + let protocol_message = + ProtocolMessage::PublicMessage(PublicMessage::from(broken_message).into()); + + let result = group_alice.process_message(provider, protocol_message); + assert_eq!( + result.expect_err("Successful processing of a broken commit."), + ProcessMessageError::InvalidCommit(StageCommitError::UpdatePathError( + ApplyUpdatePathError::UnableToDecrypt + )) + ); +} + +// Test several scenarios when PSKs are used in a group +#[openmls_test::openmls_test] +fn psks() { + // Basic group setup. + let ( + alice_credential_with_key, + alice_signature_keys, + bob_key_package_bundle, + bob_signature_keys, + ) = setup_alice_bob(ciphersuite, provider); + + // === Alice creates a group with a PSK === + let psk_id = vec![1u8, 2, 3]; + + let secret = Secret::random(ciphersuite, provider.rand()).expect("Not enough randomness."); + let external_psk = ExternalPsk::new(psk_id); + let preshared_key_id = + PreSharedKeyId::new(ciphersuite, provider.rand(), Psk::External(external_psk)) + .expect("An unexpected error occured."); + preshared_key_id.store(provider, secret.as_slice()).unwrap(); + let mut alice_group = MlsGroup::builder() + .ciphersuite(ciphersuite) + .with_wire_format_policy(PURE_PLAINTEXT_WIRE_FORMAT_POLICY) + .build(provider, &alice_signature_keys, alice_credential_with_key) + .expect("Error creating group."); + + // === Alice creates a PSK proposal === + log::info!(" >>> Creating psk proposal ..."); + let (_psk_proposal, _proposal_ref) = alice_group + .propose_external_psk(provider, &alice_signature_keys, preshared_key_id) + .expect("Could not create PSK proposal"); + + // === Alice adds Bob (and commits to PSK proposal) === + let (_commit, welcome, _group_info_option) = alice_group + .add_members( + provider, + &alice_signature_keys, + &[bob_key_package_bundle.key_package().clone()], + ) + .expect("Could not create commit"); + + log::info!(" >>> Merging commit ..."); + + alice_group + .merge_pending_commit(provider) + .expect("Could not merge commit"); + + let ratchet_tree = alice_group.export_ratchet_tree(); + + let mut bob_group = StagedWelcome::new_from_welcome( + provider, + &MlsGroupJoinConfig::default(), + welcome.into_welcome().unwrap(), + Some(ratchet_tree.into()), + ) + .expect("Could not stage welcome") + .into_group(provider) + .expect("Could not create group from welcome"); + + // === Bob updates and commits === + let (_commit, _welcome_option, _group_info_option) = bob_group + .self_update(provider, &bob_signature_keys, LeafNodeParameters::default()) + .expect("An unexpected error occurred."); +} + +// Test several scenarios when PSKs are used in a group +#[openmls_test::openmls_test] +fn staged_commit_creation( + ciphersuite: Ciphersuite, + provider: &impl crate::storage::OpenMlsProvider, +) { + // Basic group setup. + let (alice_credential_with_key, alice_signature_keys, bob_key_package_bundle, _) = + setup_alice_bob(ciphersuite, provider); + + // === Alice creates a group === + let mut alice_group = MlsGroup::builder() + .ciphersuite(ciphersuite) + .with_wire_format_policy(PURE_PLAINTEXT_WIRE_FORMAT_POLICY) + .build(provider, &alice_signature_keys, alice_credential_with_key) + .expect("Error creating group."); + + // === Alice adds Bob === + let (_commit, welcome, _group_info_option) = alice_group + .add_members( + provider, + &alice_signature_keys, + &[bob_key_package_bundle.key_package().clone()], + ) + .expect("Could not create commit"); + + alice_group + .merge_pending_commit(provider) + .expect("Could not merge commit"); + + let ratchet_tree = alice_group.export_ratchet_tree(); + + let bob_group = StagedWelcome::new_from_welcome( + provider, + &MlsGroupJoinConfig::default(), + welcome.into_welcome().unwrap(), + Some(ratchet_tree.into()), + ) + .expect("Could not stage welcome") + .into_group(provider) + .expect("Could not create group from welcome"); + + // Let's make sure we end up in the same group state. + assert_eq!( + bob_group.epoch_authenticator(), + alice_group.epoch_authenticator() + ); + assert_eq!( + bob_group.export_ratchet_tree(), + alice_group.export_ratchet_tree() + ) +} + +// Test processing of own commits +#[openmls_test::openmls_test] +fn own_commit_processing( + ciphersuite: Ciphersuite, + provider: &impl crate::storage::OpenMlsProvider, +) { + // Basic group setup. + let (alice_credential_with_key, alice_signature_keys) = + new_credential(provider, b"Alice", ciphersuite.signature_algorithm()); + + // === Alice creates a group === + let mut alice_group = MlsGroup::builder() + .ciphersuite(ciphersuite) + .with_wire_format_policy(PURE_PLAINTEXT_WIRE_FORMAT_POLICY) + .build(provider, &alice_signature_keys, alice_credential_with_key) + .expect("Error creating group."); + + // Alice creates a commit + let (commit_out, _welcome_option, _group_info_option) = alice_group + .self_update( + provider, + &alice_signature_keys, + LeafNodeParameters::default(), + ) + .expect("Could not create commit"); + + let commit_in = MlsMessageIn::from(commit_out); + + // Alice attempts to process her own commit + let error = alice_group + .process_message(provider, commit_in.into_protocol_message().unwrap()) + .expect_err("no error while processing own commit"); + assert_eq!( + error, + ProcessMessageError::InvalidCommit(StageCommitError::OwnCommit) + ); +} + +#[openmls_test::openmls_test] +fn proposal_application_after_self_was_removed( + ciphersuite: Ciphersuite, + provider: &impl crate::storage::OpenMlsProvider, +) { + // We're going to test if proposals are still applied, even after a client + // notices that it was removed from a group. We do so by having Alice + // create a group, add Bob and then create a commit where Bob is removed and + // Charlie is added in a single commit (by Alice). We then check if + // everyone's membership list is as expected. + + // Basic group setup. + let (alice_credential_with_key, _, alice_signature_keys, _pk) = + setup_client("Alice", ciphersuite, provider); + let (_, bob_kpb, _, _) = setup_client("Bob", ciphersuite, provider); + let (_, charlie_kpb, _, _) = setup_client("Charlie", ciphersuite, provider); + + let join_group_config = MlsGroupJoinConfig::builder() + .wire_format_policy(PURE_PLAINTEXT_WIRE_FORMAT_POLICY) + .build(); + + let mut alice_group = MlsGroup::builder() + .ciphersuite(ciphersuite) + .with_wire_format_policy(PURE_PLAINTEXT_WIRE_FORMAT_POLICY) + .build(provider, &alice_signature_keys, alice_credential_with_key) + .expect("Error creating group."); + + let (_commit, welcome, _group_info_option) = alice_group + .add_members( + provider, + &alice_signature_keys, + &[bob_kpb.key_package().clone()], + ) + .expect("Could not create commit"); + + alice_group + .merge_pending_commit(provider) + .expect("Could not merge commit"); + + let ratchet_tree = alice_group.export_ratchet_tree(); + + let mut bob_group = StagedWelcome::new_from_welcome( + provider, + &join_group_config, + welcome.into_welcome().unwrap(), + Some(ratchet_tree.into()), + ) + .expect("Could not stage welcome") + .into_group(provider) + .expect("Could not create group from welcome"); + + // Alice adds Charlie and removes Bob in the same commit. + // She first creates a proposal to remove Bob + let bob_index = alice_group + .members() + .find( + |Member { + index: _, + credential, + .. + }| { credential.serialized_content() == b"Bob" }, + ) + .expect("Couldn't find Bob in tree.") + .index; + + assert_eq!(bob_index.u32(), 1); + + let (bob_remove_proposal, _bob_remove_proposal_ref) = alice_group + .propose_remove_member(provider, &alice_signature_keys, bob_index) + .expect("Could not create proposal"); + + // Bob processes the proposal + let processed_message = bob_group + .process_message( + provider, + bob_remove_proposal.into_protocol_message().unwrap(), + ) + .unwrap(); + + let staged_proposal = match processed_message.into_content() { + ProcessedMessageContent::ProposalMessage(proposal) => *proposal, + _ => panic!("Wrong message type"), + }; + + bob_group + .store_pending_proposal(provider.storage(), staged_proposal) + .expect("Error storing proposal"); + + // Alice then commit to the proposal and at the same time adds Charlie + let (commit, welcome, _group_info_option) = alice_group + .add_members( + provider, + &alice_signature_keys, + &[charlie_kpb.key_package().clone()], + ) + .expect("Could not create commit"); + + // Alice merges her own commit + alice_group + .merge_pending_commit(provider) + .expect("Could not merge commit"); + + // Bob processes the commit + println!("Bob processes the commit"); + let processed_message = bob_group + .process_message(provider, commit.into_protocol_message().unwrap()) + .unwrap(); + + let staged_commit = match processed_message.into_content() { + ProcessedMessageContent::StagedCommitMessage(commit) => *commit, + _ => panic!("Wrong message type"), + }; + + bob_group + .merge_staged_commit(provider, staged_commit) + .expect("Error merging commit."); + + // Charlie processes the welcome + println!("Charlie processes the commit"); + let ratchet_tree = alice_group.export_ratchet_tree(); + + let charlie_group = StagedWelcome::new_from_welcome( + provider, + &join_group_config, + welcome.into_welcome().unwrap(), + Some(ratchet_tree.into()), + ) + .expect("Error staging welcome.") + .into_group(provider) + .expect("Error creating group from welcome."); + + // We can now check that Bob correctly processed his commit and applied the changes + // to his tree after he was removed by comparing membership lists. In + // particular, Bob's list should show that he was removed and Charlie was + // added. + let alice_members = alice_group.members(); + + let bob_members = bob_group.members(); + + let charlie_members = charlie_group.members(); + + for (alice_member, (bob_member, charlie_member)) in + alice_members.zip(bob_members.zip(charlie_members)) + { + // Note that we can't compare encryption keys for Bob because they + // didn't get updated. + assert_eq!(alice_member.index, bob_member.index); + + let alice_id = alice_member.credential.serialized_content(); + let bob_id = bob_member.credential.serialized_content(); + let charlie_id = charlie_member.credential.serialized_content(); + assert_eq!(alice_id, bob_id); + assert_eq!(alice_member.signature_key, bob_member.signature_key); + assert_eq!(charlie_member.index, bob_member.index); + assert_eq!(charlie_id, bob_id); + assert_eq!(charlie_member.signature_key, bob_member.signature_key); + assert_eq!(charlie_member.encryption_key, alice_member.encryption_key); + } + + let mut bob_members = bob_group.members(); + + let member = bob_members.next().unwrap(); + let bob_next_id = member.credential.serialized_content(); + assert_eq!(bob_next_id, b"Alice"); + let member = bob_members.next().unwrap(); + let bob_next_id = member.credential.serialized_content(); + assert_eq!(bob_next_id, b"Charlie"); +} + +#[openmls_test::openmls_test] +fn proposal_application_after_self_was_removed_ref( + ciphersuite: Ciphersuite, + provider: &impl crate::storage::OpenMlsProvider, +) { + // We're going to test if proposals are still applied, even after a client + // notices that it was removed from a group. We do so by having Alice + // create a group, add Bob and then create a commit where Bob is removed and + // Charlie is added in a single commit (by Alice). We then check if + // everyone's membership list is as expected. + + // Basic group setup. + let (alice_credential_with_key, _, alice_signature_keys, _pk) = + setup_client("Alice", ciphersuite, provider); + let (_, bob_kpb, _, _) = setup_client("Bob", ciphersuite, provider); + let (_, charlie_kpb, _, _) = setup_client("Charlie", ciphersuite, provider); + + let join_group_config = MlsGroupJoinConfig::builder() + .wire_format_policy(PURE_PLAINTEXT_WIRE_FORMAT_POLICY) + .build(); + + let mut alice_group = MlsGroup::builder() + .ciphersuite(ciphersuite) + .with_wire_format_policy(PURE_PLAINTEXT_WIRE_FORMAT_POLICY) + .build(provider, &alice_signature_keys, alice_credential_with_key) + .expect("Error creating group."); + + let (_commit, welcome, _group_info_option) = alice_group + .add_members( + provider, + &alice_signature_keys, + &[bob_kpb.key_package().clone()], + ) + .expect("Could not create commit"); + + alice_group + .merge_pending_commit(provider) + .expect("Could not merge commit"); + + let ratchet_tree = alice_group.export_ratchet_tree(); + + let mut bob_group = StagedWelcome::new_from_welcome( + provider, + &join_group_config, + welcome.into_welcome().unwrap(), + Some(ratchet_tree.into()), + ) + .expect("Could not stage welcome") + .into_group(provider) + .expect("Could not create group from welcome"); + + // Alice adds Charlie and removes Bob in the same commit. + // She first creates a proposal to remove Bob + let bob_index = alice_group + .members() + .find( + |Member { + index: _, + credential, + .. + }| { credential.serialized_content() == b"Bob" }, + ) + .expect("Couldn't find Bob in tree.") + .index; + + assert_eq!(bob_index.u32(), 1); + + let (bob_remove_proposal, _bob_remove_proposal_ref) = alice_group + .propose_remove_member(provider, &alice_signature_keys, bob_index) + .expect("Could not create proposal"); + + let (charlie_add_proposal, _charlie_add_proposal_ref) = alice_group + .propose_add_member(provider, &alice_signature_keys, charlie_kpb.key_package()) + .expect("Could not create proposal"); + + // Bob processes the proposals + let processed_message = bob_group + .process_message( + provider, + bob_remove_proposal.into_protocol_message().unwrap(), + ) + .unwrap(); + + let staged_proposal = match processed_message.into_content() { + ProcessedMessageContent::ProposalMessage(proposal) => *proposal, + _ => panic!("Wrong message type"), + }; + + bob_group + .store_pending_proposal(provider.storage(), staged_proposal) + .expect("Error storing proposal"); + + let processed_message = bob_group + .process_message( + provider, + charlie_add_proposal.into_protocol_message().unwrap(), + ) + .unwrap(); + + let staged_proposal = match processed_message.into_content() { + ProcessedMessageContent::ProposalMessage(proposal) => *proposal, + _ => panic!("Wrong message type"), + }; + + bob_group + .store_pending_proposal(provider.storage(), staged_proposal) + .expect("Error storing proposal"); + + // Alice then commits to the proposal and at the same time adds Charlie + alice_group.print_ratchet_tree("Alice's tree before commit\n"); + let alice_rt_before = alice_group.export_ratchet_tree(); + let (commit, welcome, _group_info_option) = alice_group + .commit_to_pending_proposals(provider, &alice_signature_keys) + .expect("Could not create commit"); + + // Alice merges her own commit + alice_group + .merge_pending_commit(provider) + .expect("Could not merge commit"); + alice_group.print_ratchet_tree("Alice's tree after commit\n"); + + // Bob processes the commit + println!("Bob processes the commit"); + bob_group.print_ratchet_tree("Bob's tree before processing the commit\n"); + let bob_rt_before = bob_group.export_ratchet_tree(); + assert_eq!(alice_rt_before, bob_rt_before); + let processed_message = bob_group + .process_message(provider, commit.into_protocol_message().unwrap()) + .unwrap(); + println!("Bob finished processesing the commit"); + + let staged_commit = match processed_message.into_content() { + ProcessedMessageContent::StagedCommitMessage(commit) => *commit, + _ => panic!("Wrong message type"), + }; + + bob_group + .merge_staged_commit(provider, staged_commit) + .expect("Error merging commit."); + + // Charlie processes the welcome + println!("Charlie processes the commit"); + let ratchet_tree = alice_group.export_ratchet_tree(); + + let charlie_group = StagedWelcome::new_from_welcome( + provider, + &join_group_config, + welcome.unwrap().into_welcome().unwrap(), + Some(ratchet_tree.into()), + ) + .expect("Error staging welcome.") + .into_group(provider) + .expect("Error creating group from welcome."); + + // We can now check that Bob correctly processed his and applied the changes + // to his tree after he was removed by comparing membership lists. In + // particular, Bob's list should show that he was removed and Charlie was + // added. + let alice_members = alice_group.members(); + + let bob_members = bob_group.members(); + + let charlie_members = charlie_group.members(); + + for (alice_member, (bob_member, charlie_member)) in + alice_members.zip(bob_members.zip(charlie_members)) + { + // Note that we can't compare encryption keys for Bob because they + // didn't get updated. + assert_eq!(alice_member.index, bob_member.index); + + let alice_id = alice_member.credential.serialized_content(); + let bob_id = bob_member.credential.serialized_content(); + let charlie_id = charlie_member.credential.serialized_content(); + assert_eq!(alice_id, bob_id); + assert_eq!(alice_member.signature_key, bob_member.signature_key); + assert_eq!(charlie_member.index, bob_member.index); + assert_eq!(charlie_id, bob_id); + assert_eq!(charlie_member.signature_key, bob_member.signature_key); + assert_eq!(charlie_member.encryption_key, alice_member.encryption_key); + } + + let mut bob_members = bob_group.members(); + + let member = bob_members.next().unwrap(); + let bob_next_id = member.credential.serialized_content(); + assert_eq!(bob_next_id, b"Alice"); + let member = bob_members.next().unwrap(); + let bob_next_id = member.credential.serialized_content(); + assert_eq!(bob_next_id, b"Charlie"); +} diff --git a/openmls/src/group/mls_group/tests_and_kats/tests/mod.rs b/openmls/src/group/mls_group/tests_and_kats/tests/mod.rs index 6281969e6..edf01830e 100644 --- a/openmls/src/group/mls_group/tests_and_kats/tests/mod.rs +++ b/openmls/src/group/mls_group/tests_and_kats/tests/mod.rs @@ -1,6 +1,5 @@ //! Test and Known Answer Test (KAT) modules for the MLS group. -mod core_group; mod create_commit_params; mod external_init; mod mls_group; diff --git a/openmls/src/group/mls_group/tests_and_kats/tests/proposals.rs b/openmls/src/group/mls_group/tests_and_kats/tests/proposals.rs index 6b28abad5..1004ec83c 100644 --- a/openmls/src/group/mls_group/tests_and_kats/tests/proposals.rs +++ b/openmls/src/group/mls_group/tests_and_kats/tests/proposals.rs @@ -9,10 +9,10 @@ use crate::{ group::{ errors::*, mls_group::{ + proposal_store::{ProposalQueue, ProposalStore, QueuedProposal}, tests_and_kats::utils::{setup_alice_bob_group, setup_client}, ProcessedMessageContent, }, - proposals::{ProposalQueue, ProposalStore, QueuedProposal}, GroupContext, GroupId, MlsGroup, MlsGroupJoinConfig, StagedWelcome, }, key_packages::{KeyPackageBundle, KeyPackageIn}, @@ -306,7 +306,7 @@ fn required_extension_key_package_mismatch( ))) .unwrap() .build(provider, &alice_signer, alice_credential) - .expect("Error creating CoreGroup."); + .expect("Error creating MlsGroup."); let e = alice_group.propose_add_member(provider, &alice_signer, bob_key_package) .expect_err("Proposal was created even though the key package didn't support the required extensions."); @@ -402,7 +402,7 @@ fn group_context_extension_proposal_fails( ))) .unwrap() .build(provider, &alice_signer, alice_credential) - .expect("Error creating CoreGroup."); + .expect("Error creating MlsGroup."); // Adding Bob let (_commit, welcome, _group_info_option) = alice_group diff --git a/openmls/src/group/mls_group/updates.rs b/openmls/src/group/mls_group/updates.rs index 10bc74e08..7ffd3dec2 100644 --- a/openmls/src/group/mls_group/updates.rs +++ b/openmls/src/group/mls_group/updates.rs @@ -1,4 +1,4 @@ -use core_group::create_commit_params::CreateCommitParams; +use errors::{ProposeSelfUpdateError, SelfUpdateError}; use openmls_traits::{signatures::Signer, storage::StorageProvider as _}; use crate::{ @@ -40,7 +40,7 @@ impl MlsGroup { .build(); // Create Commit over all proposals. // TODO #751 - let create_commit_result = self.group.create_commit(params, provider, signer)?; + let create_commit_result = self.create_commit(params, provider, signer)?; // Convert PublicMessage messages to MLSMessage and encrypt them if required by // the configuration @@ -56,8 +56,7 @@ impl MlsGroup { .storage() .write_group_state(self.group_id(), &self.group_state) .map_err(SelfUpdateError::StorageError)?; - self.group - .store(provider.storage()) + self.store(provider.storage()) .map_err(SelfUpdateError::StorageError)?; self.reset_aad(); @@ -65,7 +64,7 @@ impl MlsGroup { mls_message, create_commit_result .welcome_option - .map(|w| MlsMessageOut::from_welcome(w, self.group.version())), + .map(|w| MlsMessageOut::from_welcome(w, self.version())), create_commit_result.group_info, )) } @@ -86,7 +85,6 @@ impl MlsGroup { // The new leaf node will be applied later when the proposal is // committed. let mut own_leaf = self - .group .public_group() .leaf(self.own_leaf_index()) .ok_or_else(|| LibraryError::custom("The tree is broken. Couldn't find own leaf."))? @@ -101,11 +99,8 @@ impl MlsGroup { leaf_node_parmeters, )?; - let update_proposal = self.group.create_update_proposal( - self.framing_parameters(), - own_leaf.clone(), - signer, - )?; + let update_proposal = + self.create_update_proposal(self.framing_parameters(), own_leaf.clone(), signer)?; provider .storage() diff --git a/openmls/src/group/mod.rs b/openmls/src/group/mod.rs index ac1ec6127..58b79850c 100644 --- a/openmls/src/group/mod.rs +++ b/openmls/src/group/mod.rs @@ -16,28 +16,24 @@ use crate::ciphersuite::*; use crate::utils::*; // Crate -pub(crate) mod core_group; -pub(crate) mod public_group; -pub(crate) use core_group::*; pub(crate) mod errors; pub(crate) mod mls_group; +pub(crate) mod public_group; // Public -pub use core_group::proposals::*; -pub use core_group::staged_commit::StagedCommit; pub use errors::*; pub use group_context::GroupContext; pub use mls_group::config::*; pub use mls_group::membership::*; -pub use mls_group::*; +pub use mls_group::proposal_store::*; +pub use mls_group::staged_commit::StagedCommit; +pub use mls_group::{Member, *}; pub use public_group::*; // Private mod group_context; // Tests -#[cfg(test)] -pub(crate) use core_group::create_commit_params::*; #[cfg(any(feature = "test-utils", test))] pub(crate) mod tests_and_kats; diff --git a/openmls/src/group/public_group/builder.rs b/openmls/src/group/public_group/builder.rs index 529872672..d213020fd 100644 --- a/openmls/src/group/public_group/builder.rs +++ b/openmls/src/group/public_group/builder.rs @@ -139,10 +139,6 @@ impl TempBuilderPG2 { pub(crate) fn group_context(&self) -> &GroupContext { &self.group_context } - - pub(crate) fn group_id(&self) -> &GroupId { - self.group_context.group_id() - } } pub(crate) struct PublicGroupBuilder { diff --git a/openmls/src/group/public_group/diff/apply_proposals.rs b/openmls/src/group/public_group/diff/apply_proposals.rs index 470c41a18..e8dd36808 100644 --- a/openmls/src/group/public_group/diff/apply_proposals.rs +++ b/openmls/src/group/public_group/diff/apply_proposals.rs @@ -4,7 +4,7 @@ use crate::{ binary_tree::LeafNodeIndex, error::LibraryError, framing::Sender, - group::ProposalQueue, + group::proposal_store::ProposalQueue, messages::proposals::{AddProposal, ExternalInitProposal, Proposal, ProposalType}, schedule::psk::PreSharedKeyId, }; diff --git a/openmls/src/group/public_group/diff/compute_path.rs b/openmls/src/group/public_group/diff/compute_path.rs index fcb608f4d..d0e9bc894 100644 --- a/openmls/src/group/public_group/diff/compute_path.rs +++ b/openmls/src/group/public_group/diff/compute_path.rs @@ -8,7 +8,7 @@ use crate::{ credentials::CredentialWithKey, error::LibraryError, extensions::Extensions, - group::{core_group::create_commit_params::CommitType, errors::CreateCommitError}, + group::{create_commit::CommitType, errors::CreateCommitError}, schedule::CommitSecret, storage::OpenMlsProvider, treesync::{ diff --git a/openmls/src/group/public_group/mod.rs b/openmls/src/group/public_group/mod.rs index 043712a57..d4646a81f 100644 --- a/openmls/src/group/public_group/mod.rs +++ b/openmls/src/group/public_group/mod.rs @@ -8,7 +8,7 @@ //! as associated helper structs the goal of which is to enable this //! functionality. //! -//! To avoid duplication of code and functionality, [`CoreGroup`] internally +//! To avoid duplication of code and functionality, [`MlsGroup`] internally //! relies on a [`PublicGroup`] as well. #[cfg(test)] @@ -21,7 +21,10 @@ use self::{ diff::{PublicGroupDiff, StagedPublicGroupDiff}, errors::CreationFromExternalError, }; -use super::{GroupContext, GroupId, Member, ProposalStore, QueuedProposal, StagedCommit}; +use super::{ + proposal_store::{ProposalStore, QueuedProposal}, + GroupContext, GroupId, Member, StagedCommit, +}; #[cfg(test)] use crate::treesync::{node::parent_node::PlainUpdatePathNode, treekem::UpdatePathNode}; use crate::{ @@ -48,7 +51,7 @@ use crate::{ versions::ProtocolVersion, }; #[cfg(doc)] -use crate::{framing::PublicMessage, group::CoreGroup}; +use crate::{framing::PublicMessage, group::MlsGroup}; pub(crate) mod builder; pub(crate) mod diff; @@ -377,9 +380,9 @@ impl PublicGroup { } /// Stores the [`PublicGroup`] to storage. Called from methods creating a new group and mutating an - /// existing group, both inside [`PublicGroup`] and in [`CoreGroup`]. + /// existing group, both inside [`PublicGroup`] and in [`MlsGroup`]. /// - /// [`CoreGroup`]: crate::group::core_group::CoreGroup + /// [`MlsGroup`]: crate::group::MlsGroup pub(crate) fn store( &self, storage: &Storage, @@ -397,8 +400,8 @@ impl PublicGroup { /// Deletes the [`PublicGroup`] from storage. pub fn delete( - group_id: &GroupId, storage: &Storage, + group_id: &GroupId, ) -> Result<(), Storage::PublicError> { storage.delete_tree(group_id)?; storage.delete_confirmation_tag(group_id)?; diff --git a/openmls/src/group/public_group/process.rs b/openmls/src/group/public_group/process.rs index a689ec367..c04426c01 100644 --- a/openmls/src/group/public_group/process.rs +++ b/openmls/src/group/public_group/process.rs @@ -1,3 +1,6 @@ +//! This module contains the implementation of the processing functions for +//! public groups. + use openmls_traits::crypto::OpenMlsCrypto; use tls_codec::Serialize; @@ -10,8 +13,8 @@ use crate::{ ProcessedMessageContent, ProtocolMessage, Sender, SenderContext, UnverifiedMessage, }, group::{ - core_group::proposals::QueuedProposal, errors::ValidationError, - mls_group::errors::ProcessMessageError, past_secrets::MessageSecretsStore, + errors::ValidationError, mls_group::errors::ProcessMessageError, + past_secrets::MessageSecretsStore, proposal_store::QueuedProposal, }, messages::proposals::Proposal, }; diff --git a/openmls/src/group/public_group/staged_commit.rs b/openmls/src/group/public_group/staged_commit.rs index 187fc45e3..ef423179d 100644 --- a/openmls/src/group/public_group/staged_commit.rs +++ b/openmls/src/group/public_group/staged_commit.rs @@ -2,8 +2,7 @@ use super::{super::errors::*, *}; use crate::{ framing::{mls_auth_content::AuthenticatedContent, mls_content::FramedContentBody, Sender}, group::{ - core_group::{proposals::ProposalQueue, staged_commit::StagedCommitState}, - StagedCommit, + mls_group::staged_commit::StagedCommitState, proposal_store::ProposalQueue, StagedCommit, }, messages::{proposals::ProposalOrRef, Commit}, }; @@ -170,15 +169,12 @@ impl PublicGroup { /// - Applies the proposals covered by the commit to the tree /// - Applies the (optional) update path to the tree /// - Updates the [`GroupContext`] - /// - /// A similar function to this exists in [`CoreGroup`], which in addition - /// does the following: /// - Decrypts and derives the path secrets /// - Initializes the key schedule for epoch rollover /// - Verifies the confirmation tag /// /// Returns a [`StagedCommit`] that can be inspected and later merged into - /// the group state either with [`CoreGroup::merge_commit()`] or + /// the group state either with [`MlsGroup::merge_commit()`] or /// [`PublicGroup::merge_diff()`] This function does the following checks: /// - ValSem101 /// - ValSem102 diff --git a/openmls/src/group/public_group/tests.rs b/openmls/src/group/public_group/tests.rs index 5cf00bcfe..79c05d815 100644 --- a/openmls/src/group/public_group/tests.rs +++ b/openmls/src/group/public_group/tests.rs @@ -7,8 +7,8 @@ use crate::{ ProcessedMessageContent, ProtocolMessage, Sender, }, group::{ - mls_group::tests_and_kats::utils::setup_client, GroupId, MlsGroup, MlsGroupCreateConfig, - ProposalStore, StagedCommit, PURE_PLAINTEXT_WIRE_FORMAT_POLICY, + mls_group::tests_and_kats::utils::setup_client, proposal_store::ProposalStore, GroupId, + MlsGroup, MlsGroupCreateConfig, StagedCommit, PURE_PLAINTEXT_WIRE_FORMAT_POLICY, }, messages::proposals::Proposal, }; diff --git a/openmls/src/group/public_group/validation.rs b/openmls/src/group/public_group/validation.rs index abfc52597..828dcdcb3 100644 --- a/openmls/src/group/public_group/validation.rs +++ b/openmls/src/group/public_group/validation.rs @@ -7,6 +7,7 @@ use openmls_traits::types::VerifiableCiphersuite; use super::PublicGroup; use crate::extensions::RequiredCapabilitiesExtension; +use crate::group::proposal_store::ProposalQueue; use crate::group::GroupContextExtensionsProposalValidationError; use crate::prelude::LibraryError; use crate::treesync::errors::LeafNodeValidationError; @@ -19,7 +20,7 @@ use crate::{ group::{ errors::{ExternalCommitValidationError, ProposalValidationError, ValidationError}, past_secrets::MessageSecretsStore, - Member, ProposalQueue, + Member, }, messages::{ proposals::{Proposal, ProposalOrRefType, ProposalType}, @@ -87,8 +88,7 @@ impl PublicGroup { if let Sender::Member(leaf_index) = sender { // If the sender is a member, it has to be in the tree, except if // it's an application message. Then it might be okay if it's in an - // old secret tree instance, but we'll leave that to the CoreGroup - // to validate. + // old secret tree instance. let is_in_secrets_store = if let Some(mss) = message_secrets_store_option { mss.epoch_has_leaf(verifiable_content.epoch(), *leaf_index) } else { diff --git a/openmls/src/group/tests_and_kats/mod.rs b/openmls/src/group/tests_and_kats/mod.rs index 62632607f..7ee62ea9d 100644 --- a/openmls/src/group/tests_and_kats/mod.rs +++ b/openmls/src/group/tests_and_kats/mod.rs @@ -2,7 +2,5 @@ mod kats; #[cfg(test)] mod tests; -#[cfg(any(feature = "test-utils", test))] -pub(crate) mod tree_printing; #[cfg(test)] pub(crate) mod utils; diff --git a/openmls/src/group/tests_and_kats/tests/commit_validation.rs b/openmls/src/group/tests_and_kats/tests/commit_validation.rs index acfd5fdb6..5c6d21d69 100644 --- a/openmls/src/group/tests_and_kats/tests/commit_validation.rs +++ b/openmls/src/group/tests_and_kats/tests/commit_validation.rs @@ -1,7 +1,9 @@ //! This module tests the validation of commits as defined in //! https://openmls.tech/book/message_validation.html#commit-message-validation +use create_commit::CreateCommitParams; use openmls_traits::{prelude::*, signatures::Signer, types::Ciphersuite}; +use proposal_store::QueuedProposal; use tls_codec::{Deserialize, Serialize}; use crate::group::tests_and_kats::utils::{ @@ -220,14 +222,14 @@ fn test_valsem200() { let mut signed_plaintext: PublicMessage = signed_plaintext.into(); - let membership_key = alice_group.group().message_secrets().membership_key(); + let membership_key = alice_group.message_secrets().membership_key(); signed_plaintext .set_membership_tag( provider.crypto(), ciphersuite, membership_key, - alice_group.group().message_secrets().serialized_context(), + alice_group.message_secrets().serialized_context(), ) .expect("error refreshing membership tag"); @@ -317,7 +319,7 @@ fn test_valsem201() { let gce_proposal = || { queued(Proposal::GroupContextExtensions( - GroupContextExtensionProposal::new(alice_group.group().context().extensions().clone()), + GroupContextExtensionProposal::new(alice_group.context().extensions().clone()), )) }; @@ -326,7 +328,7 @@ fn test_valsem201() { // TODO: #916 when/if AppAck proposal are implemented (path not required) // TODO: #751 when ReInit proposal validation are implemented (path not required). Currently one // cannot distinguish when the commit has a single ReInit proposal from the commit without proposals - // in [CoreGroup::apply_proposals()] + // in [MlsGroup::apply_proposals()] let cases = vec![ (vec![add_proposal()], false), (vec![psk_proposal()], false), @@ -357,7 +359,6 @@ fn test_valsem201() { .force_self_update(false) .build(); let commit = alice_group - .group() .create_commit(params, provider, &alice_credential.signer) .unwrap() .commit; @@ -370,13 +371,13 @@ fn test_valsem201() { }; let mut commit: PublicMessage = commit.into(); - let membership_key = alice_group.group().message_secrets().membership_key(); + let membership_key = alice_group.message_secrets().membership_key(); commit .set_membership_tag( provider.crypto(), ciphersuite, membership_key, - alice_group.group().message_secrets().serialized_context(), + alice_group.message_secrets().serialized_context(), ) .unwrap(); // verify that a path is indeed required when the commit is received @@ -678,7 +679,6 @@ fn test_valsem204() { }) .collect(); let new_nodes = alice_group - .group() .public_group() .encrypt_path( provider, @@ -775,14 +775,14 @@ fn test_valsem205() { plaintext.set_confirmation_tag(Some(new_confirmation_tag)); // Since the membership tag covers the confirmation tag, we have to refresh it. - let membership_key = alice_group.group().message_secrets().membership_key(); + let membership_key = alice_group.message_secrets().membership_key(); plaintext .set_membership_tag( provider.crypto(), ciphersuite, membership_key, - alice_group.group().message_secrets().serialized_context(), + alice_group.message_secrets().serialized_context(), ) .expect("error refreshing membership tag"); diff --git a/openmls/src/group/tests_and_kats/tests/encoding.rs b/openmls/src/group/tests_and_kats/tests/encoding.rs index f5b014b60..8b0cf45ed 100644 --- a/openmls/src/group/tests_and_kats/tests/encoding.rs +++ b/openmls/src/group/tests_and_kats/tests/encoding.rs @@ -34,9 +34,7 @@ fn create_encoding_test_setup(provider: &impl crate::storage::OpenMlsProvider) - for &ciphersuite in provider.crypto().supported_ciphersuites().iter() { let test_group = TestGroupConfig { ciphersuite, - config: CoreGroupConfig { - add_ratchet_tree_extension: true, - }, + use_ratchet_tree_extension: true, members: vec![alice_config.clone(), bob_config.clone()], }; test_group_configs.push(test_group); diff --git a/openmls/src/group/tests_and_kats/tests/external_commit_validation.rs b/openmls/src/group/tests_and_kats/tests/external_commit_validation.rs index df1031b64..a7ec4e35e 100644 --- a/openmls/src/group/tests_and_kats/tests/external_commit_validation.rs +++ b/openmls/src/group/tests_and_kats/tests/external_commit_validation.rs @@ -251,15 +251,13 @@ fn test_valsem242() { group_id: alice_group.group_id().clone(), version: Default::default(), ciphersuite, - extensions: alice_group.group().context().extensions().clone(), + extensions: alice_group.context().extensions().clone(), })) }; let gce_proposal = { ProposalOrRef::Proposal(Proposal::GroupContextExtensions( - GroupContextExtensionProposal::new( - alice_group.group().context().extensions().clone(), - ), + GroupContextExtensionProposal::new(alice_group.context().extensions().clone()), )) }; @@ -535,12 +533,8 @@ fn test_valsem246() { // This shows that the message is actually signed using this credential. let decrypted_message = DecryptedMessage::from_inbound_public_message( public_message_commit.clone().into(), - alice_group.group().message_secrets(), - alice_group - .group() - .message_secrets() - .serialized_context() - .to_vec(), + alice_group.message_secrets(), + alice_group.message_secrets().serialized_context().to_vec(), provider.crypto(), ciphersuite, ) diff --git a/openmls/src/group/tests_and_kats/tests/external_remove_proposal.rs b/openmls/src/group/tests_and_kats/tests/external_remove_proposal.rs index 3a5ad98a5..dc3f83623 100644 --- a/openmls/src/group/tests_and_kats/tests/external_remove_proposal.rs +++ b/openmls/src/group/tests_and_kats/tests/external_remove_proposal.rs @@ -112,7 +112,6 @@ fn external_remove_proposal_should_remove_member() { // DS is an allowed external sender of the group assert!(alice_group - .group() .context() .extensions() .iter() diff --git a/openmls/src/group/tests_and_kats/tests/framing.rs b/openmls/src/group/tests_and_kats/tests/framing.rs index 8d133260f..8bc42a42e 100644 --- a/openmls/src/group/tests_and_kats/tests/framing.rs +++ b/openmls/src/group/tests_and_kats/tests/framing.rs @@ -35,7 +35,7 @@ fn padding(provider: &impl crate::storage::OpenMlsProvider) { for &ciphersuite in provider.crypto().supported_ciphersuites().iter() { let test_group = TestGroupConfig { ciphersuite, - config: CoreGroupConfig::default(), + use_ratchet_tree_extension: true, members: vec![alice_config.clone()], }; test_group_configs.push(test_group); diff --git a/openmls/src/group/tests_and_kats/tests/framing_validation.rs b/openmls/src/group/tests_and_kats/tests/framing_validation.rs index 3c515a11a..dde81b6fb 100644 --- a/openmls/src/group/tests_and_kats/tests/framing_validation.rs +++ b/openmls/src/group/tests_and_kats/tests/framing_validation.rs @@ -297,8 +297,8 @@ fn test_valsem004() { .set_membership_tag( provider.crypto(), ciphersuite, - alice_group.group().message_secrets().membership_key(), - alice_group.group().message_secrets().serialized_context(), + alice_group.message_secrets().membership_key(), + alice_group.message_secrets().serialized_context(), ) .expect("Error setting membership tag."); @@ -357,8 +357,8 @@ fn test_valsem005() { .set_membership_tag( provider.crypto(), ciphersuite, - alice_group.group().message_secrets().membership_key(), - alice_group.group().message_secrets().serialized_context(), + alice_group.message_secrets().membership_key(), + alice_group.message_secrets().serialized_context(), ) .expect("Error setting membership tag."); @@ -574,8 +574,8 @@ fn test_valsem009() { .set_membership_tag( provider.crypto(), ciphersuite, - alice_group.group().message_secrets().membership_key(), - alice_group.group().message_secrets().serialized_context(), + alice_group.message_secrets().membership_key(), + alice_group.message_secrets().serialized_context(), ) .expect("Error setting membership tag."); @@ -636,8 +636,8 @@ fn test_valsem010() { .set_membership_tag( provider.crypto(), ciphersuite, - alice_group.group().message_secrets().membership_key(), - alice_group.group().message_secrets().serialized_context(), + alice_group.message_secrets().membership_key(), + alice_group.message_secrets().serialized_context(), ) .expect("Error setting membership tag."); diff --git a/openmls/src/group/tests_and_kats/tests/group_context_extensions.rs b/openmls/src/group/tests_and_kats/tests/group_context_extensions.rs index 542f4367f..f450778de 100644 --- a/openmls/src/group/tests_and_kats/tests/group_context_extensions.rs +++ b/openmls/src/group/tests_and_kats/tests/group_context_extensions.rs @@ -496,7 +496,7 @@ fn fail_insufficient_extensiontype_capabilities_add_valno103() { group_context.confirmed_transcript_hash() ); - let secrets = alice.group.group().message_secrets(); + let secrets = alice.group.message_secrets(); let membership_key = secrets.membership_key().as_slice(); let franken_commit = frankenstein::FrankenMlsMessage { @@ -620,7 +620,7 @@ fn fail_insufficient_extensiontype_capabilities_update_valno103() { // prepare data needed for proposal let group_context = bob.group.export_group_context().clone(); - let secrets = bob.group.group().message_secrets(); + let secrets = bob.group.message_secrets(); let membership_key = secrets.membership_key().as_slice(); // build MlsMessage containing the proposal @@ -670,7 +670,7 @@ fn fail_insufficient_extensiontype_capabilities_update_valno103() { // prepare data needed for making the message authentic let group_context = alice.group.export_group_context().clone(); - let secrets = alice.group.group().message_secrets(); + let secrets = alice.group.message_secrets(); let membership_key = secrets.membership_key().as_slice(); let franken_commit = frankenstein::FrankenMlsMessage { @@ -763,7 +763,7 @@ fn fail_key_package_version_valno201() { key_package.resign(&charlie.signer); let group_context = alice.group.export_group_context(); - let membership_key = alice.group.group().message_secrets().membership_key(); + let membership_key = alice.group.message_secrets().membership_key(); let franken_commit_message = frankenstein::FrankenMlsMessage { version, @@ -825,7 +825,6 @@ fn fail_2_gce_proposals_1_commit_valno308() { // No required capabilities, so no specifically required extensions. assert!(alice .group - .group() .context() .extensions() .required_capabilities() @@ -876,7 +875,7 @@ fn fail_2_gce_proposals_1_commit_valno308() { group_context.confirmed_transcript_hash() ); - let secrets = alice.group.group().message_secrets(); + let secrets = alice.group.message_secrets(); let membership_key = secrets.membership_key().as_slice(); *msg = frankenstein::FrankenPublicMessage::auth( @@ -932,7 +931,6 @@ fn fail_unsupported_gces_add_valno1001() { // No required capabilities, so no specifically required extensions. assert!(alice .group - .group() .context() .extensions() .required_capabilities() @@ -989,7 +987,7 @@ fn fail_unsupported_gces_add_valno1001() { extension_types.push(0xf003); let group_context = bob.group.export_group_context().clone(); - let secrets = bob.group.group().message_secrets(); + let secrets = bob.group.message_secrets(); let membership_key = secrets.membership_key().as_slice(); let franken_commit_message = frankenstein::FrankenMlsMessage { @@ -1055,7 +1053,6 @@ fn proposal() { // No required capabilities, so no specifically required extensions. assert!(alice .group - .group() .context() .extensions() .required_capabilities() @@ -1076,7 +1073,6 @@ fn proposal() { let required_capabilities = alice .group - .group() .context() .extensions() .required_capabilities() diff --git a/openmls/src/group/tests_and_kats/tests/proposal_validation.rs b/openmls/src/group/tests_and_kats/tests/proposal_validation.rs index ae81628a2..465a6743f 100644 --- a/openmls/src/group/tests_and_kats/tests/proposal_validation.rs +++ b/openmls/src/group/tests_and_kats/tests/proposal_validation.rs @@ -6,6 +6,7 @@ use openmls_traits::{ prelude::{openmls_types::*, *}, signatures::Signer, }; +use proposal_store::QueuedProposal; use tls_codec::{Deserialize, Serialize}; use crate::group::tests_and_kats::utils::{ @@ -213,17 +214,14 @@ fn insert_proposal_and_resign( ciphersuite, ); - let membership_key = committer_group.group().message_secrets().membership_key(); + let membership_key = committer_group.message_secrets().membership_key(); signed_plaintext .set_membership_tag( provider.crypto(), ciphersuite, membership_key, - committer_group - .group() - .message_secrets() - .serialized_context(), + committer_group.message_secrets().serialized_context(), ) .expect("error refreshing membership tag"); @@ -933,7 +931,6 @@ fn test_valsem103_valsem104(ciphersuite: Ciphersuite, provider: &impl OpenMlsPro // We now pull bob's public key from his leaf. let bob_encryption_key = bob_group - .group() .own_leaf_node() .expect("No own leaf") .encryption_key() @@ -1683,13 +1680,11 @@ fn test_valsem110() { // We begin by creating a leaf node with a colliding encryption key. let bob_leaf_node = bob_group - .group() .own_leaf_node() .expect("error getting own leaf node") .clone(); let alice_encryption_key = alice_group - .group() .own_leaf_node() .unwrap() .encryption_key() @@ -1711,7 +1706,7 @@ fn test_valsem110() { franken_leaf_node.encryption_key = alice_encryption_key.key().clone(); franken_leaf_node.leaf_node_source = FrankenLeafNodeSource::Update; franken_leaf_node.resign( - Some(bob_group.group().own_tree_position().into()), + Some(bob_group.own_tree_position().into()), &bob_credential_with_key_and_signer.signer, ); @@ -1740,11 +1735,7 @@ fn test_valsem110() { // Rebuild the PublicMessage with the new content let group_context = bob_group.export_group_context().clone(); - let membership_key = bob_group - .group() - .message_secrets() - .membership_key() - .as_slice(); + let membership_key = bob_group.message_secrets().membership_key().as_slice(); let new_public_message = FrankenPublicMessage::auth( provider, @@ -1837,7 +1828,6 @@ fn test_valsem110() { // We have to store the keypair with the proper label s.t. Bob can actually // process the commit. let leaf_keypair = alice_group - .group() .read_epoch_keypairs(provider.storage()) .into_iter() .find(|keypair| keypair.public_key() == &alice_encryption_key) diff --git a/openmls/src/group/tests_and_kats/tree_printing.rs b/openmls/src/group/tests_and_kats/tree_printing.rs deleted file mode 100644 index 6ecf49ba3..000000000 --- a/openmls/src/group/tests_and_kats/tree_printing.rs +++ /dev/null @@ -1,22 +0,0 @@ -//! A framework to create integration tests of the "raw" core_group API. -//! # Test utils -//! -//! Most tests require to set up groups, clients, credentials, and identities. -//! This module implements helpers to do that. - -#[cfg(any(feature = "test-utils", test))] -fn log2(x: u32) -> usize { - if x == 0 { - return 0; - } - let mut k = 0; - while (x >> k) > 0 { - k += 1 - } - k - 1 -} - -#[cfg(any(feature = "test-utils", test))] -pub(crate) fn root(size: u32) -> u32 { - (1 << log2(size)) - 1 -} diff --git a/openmls/src/group/tests_and_kats/utils.rs b/openmls/src/group/tests_and_kats/utils.rs index 9907b7fd5..08aaedf88 100644 --- a/openmls/src/group/tests_and_kats/utils.rs +++ b/openmls/src/group/tests_and_kats/utils.rs @@ -1,4 +1,4 @@ -//! A framework to create integration tests of the "raw" core_group API. +//! A framework to create integration tests of the MlsGroup API. //! # Test utils //! //! Most tests require to set up groups, clients, credentials, and identities. @@ -30,7 +30,7 @@ pub(crate) struct TestClientConfig { /// Configuration of a group meant to be used in a test setup. pub(crate) struct TestGroupConfig { pub(crate) ciphersuite: Ciphersuite, - pub(crate) config: CoreGroupConfig, + pub(crate) use_ratchet_tree_extension: bool, pub(crate) members: Vec, } @@ -125,7 +125,7 @@ pub(crate) fn setup( let mls_group = MlsGroup::builder() .with_group_id(GroupId::from_slice(&group_id.to_be_bytes())) .ciphersuite(group_config.ciphersuite) - .use_ratchet_tree_extension(group_config.config.add_ratchet_tree_extension) + .use_ratchet_tree_extension(group_config.use_ratchet_tree_extension) .with_wire_format_policy(PURE_PLAINTEXT_WIRE_FORMAT_POLICY) .build( provider, @@ -236,10 +236,10 @@ fn test_setup(provider: &impl crate::storage::OpenMlsProvider) { name: "TestClientConfigB", ciphersuites: vec![Ciphersuite::MLS_128_DHKEMX25519_CHACHA20POLY1305_SHA256_Ed25519], }; - let group_config = CoreGroupConfig::default(); + let use_ratchet_tree_extension = true; let test_group_config = TestGroupConfig { ciphersuite: Ciphersuite::MLS_128_DHKEMX25519_CHACHA20POLY1305_SHA256_Ed25519, - config: group_config, + use_ratchet_tree_extension, members: vec![test_client_config_a.clone(), test_client_config_b.clone()], }; let test_setup_config = TestSetupConfig { @@ -329,14 +329,14 @@ pub(crate) fn resign_message( let mut signed_plaintext: PublicMessage = signed_plaintext.into(); - let membership_key = alice_group.group().message_secrets().membership_key(); + let membership_key = alice_group.message_secrets().membership_key(); signed_plaintext .set_membership_tag( provider.crypto(), ciphersuite, membership_key, - alice_group.group().message_secrets().serialized_context(), + alice_group.message_secrets().serialized_context(), ) .expect("error refreshing membership tag"); signed_plaintext diff --git a/openmls/src/prelude.rs b/openmls/src/prelude.rs index 7e34d2d80..b21bf7102 100644 --- a/openmls/src/prelude.rs +++ b/openmls/src/prelude.rs @@ -2,7 +2,7 @@ //! Include this to get access to all the public functions of OpenMLS. // MlsGroup -pub use crate::group::{core_group::Member, *}; +pub use crate::group::{Member, *}; pub use crate::group::public_group::{errors::*, PublicGroup}; diff --git a/openmls/src/schedule/mod.rs b/openmls/src/schedule/mod.rs index ea8aec63b..fabed6585 100644 --- a/openmls/src/schedule/mod.rs +++ b/openmls/src/schedule/mod.rs @@ -732,7 +732,7 @@ impl ExporterSecret { /// Derive a `Secret` from the exporter secret. We return `Vec` here, so /// it can be used outside of OpenMLS. This function is made available for - /// use from the outside through [`crate::group::core_group::export_secret`]. + /// use from the outside through [`MlsGroup::export_secret`]. pub(crate) fn derive_exported_secret( &self, ciphersuite: Ciphersuite, diff --git a/openmls/src/storage.rs b/openmls/src/storage.rs index ff6813283..ac30c62d9 100644 --- a/openmls/src/storage.rs +++ b/openmls/src/storage.rs @@ -10,10 +10,11 @@ use openmls_traits::storage::{traits, Entity, Key, CURRENT_VERSION}; use crate::binary_tree::LeafNodeIndex; +use crate::group::proposal_store::QueuedProposal; use crate::group::{MlsGroupJoinConfig, MlsGroupState}; use crate::{ ciphersuite::hash_ref::ProposalRef, - group::{GroupContext, GroupId, InterimTranscriptHash, QueuedProposal}, + group::{GroupContext, GroupId, InterimTranscriptHash}, messages::ConfirmationTag, treesync::{LeafNode, TreeSync}, }; diff --git a/openmls/src/storage/kat_storage_stability.rs b/openmls/src/storage/kat_storage_stability.rs index 29692c105..15e506524 100644 --- a/openmls/src/storage/kat_storage_stability.rs +++ b/openmls/src/storage/kat_storage_stability.rs @@ -477,13 +477,7 @@ fn test(ciphersuite: Ciphersuite, provider: &Provider) { // we are in the right epoch assert_eq!(alice_group_new_group.epoch(), 0.into()); - assert_eq!( - alice_group_new_group - .group() - .resumption_psk_store() - .cursor(), - 1 - ); + assert_eq!(alice_group_new_group.resumption_psk_store().cursor(), 1); // dropping to prevent accidentally using the wrong provider or group later drop(alice_group_new_group); @@ -536,7 +530,6 @@ fn test(ciphersuite: Ciphersuite, provider: &Provider) { assert_eq!(alice_group_pending_add_commit.epoch(), 0.into()); assert_eq!( alice_group_pending_add_commit - .group() .resumption_psk_store() .cursor(), 1 @@ -575,13 +568,7 @@ fn test(ciphersuite: Ciphersuite, provider: &Provider) { // we are in the right epoch assert_eq!(alice_group_bob_added.epoch(), 1.into()); - assert_eq!( - alice_group_bob_added - .group() - .resumption_psk_store() - .cursor(), - 2 - ); + assert_eq!(alice_group_bob_added.resumption_psk_store().cursor(), 2); // dropping to prevent accidentally using the wrong provider or group later drop(alice_group_bob_added); @@ -644,7 +631,6 @@ fn test(ciphersuite: Ciphersuite, provider: &Provider) { assert_eq!(alice_group_pending_gce_commit.epoch(), 1.into()); assert_eq!( alice_group_pending_gce_commit - .group() .resumption_psk_store() .cursor(), 2 diff --git a/openmls/src/test_utils/test_framework/errors.rs b/openmls/src/test_utils/test_framework/errors.rs index 3375426c1..f2b9f024e 100644 --- a/openmls/src/test_utils/test_framework/errors.rs +++ b/openmls/src/test_utils/test_framework/errors.rs @@ -88,7 +88,7 @@ pub enum ClientError { /// See [`MergeCommitError`] for more details. #[error(transparent)] MergeCommitError(#[from] MergeCommitError), - /// See [`StorageError>`] for more details. + /// See `StorageError` for more details. #[error(transparent)] KeyStoreError(#[from] StorageError), /// See [`LibraryError`] for more details. diff --git a/openmls/src/tree/tests_and_kats/kats/kat_message_protection.rs b/openmls/src/tree/tests_and_kats/kats/kat_message_protection.rs index 14d14eafb..d030bee01 100644 --- a/openmls/src/tree/tests_and_kats/kats/kat_message_protection.rs +++ b/openmls/src/tree/tests_and_kats/kats/kat_message_protection.rs @@ -289,7 +289,7 @@ pub fn run_test_vector( ) .unwrap(); let my_proposal_priv = sender_group - .encrypt(proposal_authenticated_content, 0, provider) + .encrypt(proposal_authenticated_content, provider) .unwrap(); let my_proposal_priv_out = MlsMessageOut::from_private_message( my_proposal_priv, @@ -359,9 +359,8 @@ pub fn run_test_vector( .unwrap(); let processed_unverified_message = group - .group() .public_group() - .parse_message(decrypted_message, group.group().message_secrets_store()) + .parse_message(decrypted_message, group.message_secrets_store()) .unwrap(); let processed_message: AuthenticatedContent = processed_unverified_message .verify(ciphersuite, provider.crypto(), ProtocolVersion::Mls10) @@ -406,7 +405,7 @@ pub fn run_test_vector( mac_value: vec![0; 32].into(), // Set a fake mac, we don't check it. })); let my_commit_pub = sender_group - .encrypt(commit_authenticated_content, 0, provider) + .encrypt(commit_authenticated_content, provider) .unwrap(); let my_commit_priv_out = MlsMessageOut::from_private_message( my_commit_pub, diff --git a/openmls/src/treesync/diff.rs b/openmls/src/treesync/diff.rs index 5d3ef2df9..f16cfb387 100644 --- a/openmls/src/treesync/diff.rs +++ b/openmls/src/treesync/diff.rs @@ -36,7 +36,7 @@ use super::{ treesync_node::{TreeSyncLeafNode, TreeSyncParentNode}, LeafNode, TreeSync, TreeSyncParentHashError, }; -use crate::group::{create_commit_params::CommitType, GroupId}; +use crate::group::{create_commit::CommitType, GroupId}; use crate::{ binary_tree::{ array_representation::{ diff --git a/openmls/src/treesync/mod.rs b/openmls/src/treesync/mod.rs index fbb60de19..4c542de50 100644 --- a/openmls/src/treesync/mod.rs +++ b/openmls/src/treesync/mod.rs @@ -44,10 +44,7 @@ use self::{ #[cfg(test)] use crate::binary_tree::array_representation::ParentNodeIndex; #[cfg(any(feature = "test-utils", test))] -use crate::{ - binary_tree::array_representation::level, group::tests_and_kats::tree_printing::root, - test_utils::bytes_to_hex, -}; +use crate::{binary_tree::array_representation::level, test_utils::bytes_to_hex}; use crate::{ binary_tree::{ array_representation::{is_node_in_tree, tree::TreeNode, LeafNodeIndex, TreeSize}, @@ -275,6 +272,23 @@ impl From for RatchetTree { } } +#[cfg(any(feature = "test-utils", test))] +fn log2(x: u32) -> usize { + if x == 0 { + return 0; + } + let mut k = 0; + while (x >> k) > 0 { + k += 1 + } + k - 1 +} + +#[cfg(any(feature = "test-utils", test))] +pub(crate) fn root(size: u32) -> u32 { + (1 << log2(size)) - 1 +} + #[cfg(any(feature = "test-utils", test))] impl fmt::Display for RatchetTree { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { diff --git a/openmls/src/treesync/tests_and_kats/kats/kat_tree_operations.rs b/openmls/src/treesync/tests_and_kats/kats/kat_tree_operations.rs index 4aaf4dc95..bdede7cfc 100644 --- a/openmls/src/treesync/tests_and_kats/kats/kat_tree_operations.rs +++ b/openmls/src/treesync/tests_and_kats/kats/kat_tree_operations.rs @@ -30,7 +30,10 @@ use crate::{ ciphersuite::{Mac, Secret}, extensions::Extensions, framing::Sender, - group::{GroupContext, GroupEpoch, GroupId, ProposalQueue, PublicGroup, QueuedProposal}, + group::{ + proposal_store::{ProposalQueue, QueuedProposal}, + GroupContext, GroupEpoch, GroupId, PublicGroup, + }, messages::{proposals::Proposal, proposals_in::ProposalIn, ConfirmationTag}, test_utils::*, treesync::{node::NodeIn, RatchetTree, RatchetTreeIn, TreeSync}, diff --git a/openmls/src/treesync/tests_and_kats/kats/kat_treekem.rs b/openmls/src/treesync/tests_and_kats/kats/kat_treekem.rs index 1f6f6c50c..98882bd69 100644 --- a/openmls/src/treesync/tests_and_kats/kats/kat_treekem.rs +++ b/openmls/src/treesync/tests_and_kats/kats/kat_treekem.rs @@ -9,7 +9,7 @@ use tls_codec::{Deserialize as TlsDeserializeTrait, Serialize as TlsSerializeTra use crate::{ binary_tree::{array_representation::ParentNodeIndex, LeafNodeIndex}, extensions::{Extensions, RatchetTreeExtension}, - group::{CommitType, GroupContext, GroupEpoch, GroupId}, + group::{create_commit::CommitType, GroupContext, GroupEpoch, GroupId}, messages::PathSecret, prelude_test::Secret, schedule::CommitSecret, diff --git a/openmls/tests/external_commit.rs b/openmls/tests/external_commit.rs index dd85fc5bc..7d086ae58 100644 --- a/openmls/tests/external_commit.rs +++ b/openmls/tests/external_commit.rs @@ -144,7 +144,9 @@ fn test_group_info() { &bob_signature_keys, None, verifiable_group_info, - &MlsGroupJoinConfig::default(), + &MlsGroupJoinConfig::builder() + .use_ratchet_tree_extension(true) + .build(), None, None, b"", diff --git a/traits/src/storage.rs b/traits/src/storage.rs index 593474e4a..c221f4710 100644 --- a/traits/src/storage.rs +++ b/traits/src/storage.rs @@ -600,7 +600,6 @@ pub mod traits { pub trait GroupState: Entity {} pub trait GroupEpochSecrets: Entity {} pub trait LeafNodeIndex: Entity {} - pub trait GroupUseRatchetTreeExtension: Entity {} pub trait MessageSecrets: Entity {} pub trait ResumptionPskStore: Entity {} pub trait KeyPackage: Entity {} From 45bdb582ed2be7e10642dec2a9347177fa0b116a Mon Sep 17 00:00:00 2001 From: raphaelrobert Date: Mon, 2 Sep 2024 20:54:58 +0200 Subject: [PATCH 07/15] Update links --- Developer.md | 4 ++-- README.md | 8 ++------ book/src/introduction.md | 4 ++-- openmls/src/group/mls_group/config.rs | 2 +- .../src/group/tests_and_kats/tests/commit_validation.rs | 2 +- .../src/group/tests_and_kats/tests/framing_validation.rs | 2 +- .../src/group/tests_and_kats/tests/proposal_validation.rs | 2 +- openmls/src/lib.rs | 2 +- 8 files changed, 11 insertions(+), 15 deletions(-) diff --git a/Developer.md b/Developer.md index 2f389c437..dd6f016a4 100644 --- a/Developer.md +++ b/Developer.md @@ -35,5 +35,5 @@ You can start by looking at the [open issues](https://github.com/openmls/openmls OpenMLS adheres to the [Contributor Covenant](https://www.contributor-covenant.org/) Code of Coduct. Please read the [Code of Conduct](https://github.com/openmls/openmls/tree/main/CODE_OF_CONDUCT.md) carefully. -[book-main-link]: https://openmls.tech/openmls/book -[docs-main-link]: https://openmls.tech/openmls/doc/openmls/index.html +[book-main-link]: https://latest.openmls.tech/book +[docs-main-link]: https://latest.openmls.tech/doc/openmls/index.html diff --git a/README.md b/README.md index b59388472..dc586a669 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,6 @@ [![OpenMLS List][list-image]][list-link] [![Tests & Checks][gh-tests-image]](https://github.com/openmls/openmls/actions/workflows/tests.yml?branch=main) - [![codecov][codecov-image]](https://codecov.io/gh/openmls/openmls) [![Docs][docs-release-badge]][docs-release-link] @@ -12,7 +11,7 @@ ![Rust Version][rustc-image] *OpenMLS* is a Rust implementation of the Messaging Layer Security (MLS) protocol, as specified in [RFC 9420](https://datatracker.ietf.org/doc/html/rfc9420). - + It is a software library that can serve as a building block in applications that require end-to-end encryption of messages. It has a safe and easy-to-use interface that hides the complexity of the underlying cryptographic operations. @@ -33,9 +32,6 @@ OpenMLS is built and tested on the Github CI for the following rust targets. - i686-pc-windows-msvc - x86_64-apple-darwin - - ### Unsupported, but built on CI The Github CI also builds (but doesn't test) the following rust targets. @@ -78,7 +74,7 @@ OpenMLS is maintained and developed by [Phoenix R&D] and [Cryspen]. [docs-release-badge]: https://img.shields.io/badge/docs-release-blue.svg?style=for-the-badge [docs-release-link]: https://docs.rs/crate/openmls/latest [book-release-badge]: https://img.shields.io/badge/book-release-blue.svg?style=for-the-badge -[book-release-link]: https://openmls.tech/book +[book-release-link]: https://book.openmls.tech [drone-image]: https://img.shields.io/drone/build/openmls/openmls/main?label=ARM64%20Build%20Status&logo=drone&style=for-the-badge [codecov-image]: https://img.shields.io/codecov/c/github/openmls/openmls/main?logo=codecov&style=for-the-badge [gh-tests-image]: https://img.shields.io/github/actions/workflow/status/openmls/openmls/tests.yml?branch=main&style=for-the-badge&logo=github diff --git a/book/src/introduction.md b/book/src/introduction.md index 14c86633a..aa9fd8cb2 100644 --- a/book/src/introduction.md +++ b/book/src/introduction.md @@ -2,11 +2,11 @@ -{{#include ../../README.md:2:14}} +{{#include ../../README.md:2:13}} OpenMLS provides a high-level API to create and manage MLS groups. It supports basic ciphersuites and an interchangeable cryptographic provider, key store, and random number generator. This book provides guidance on using OpenMLS and its `MlsGroup` API to perform basic group operations, illustrated with examples. -{{#include ../../README.md:20:}} +{{#include ../../README.md:19:}} diff --git a/openmls/src/group/mls_group/config.rs b/openmls/src/group/mls_group/config.rs index faf4804ad..8754d7a81 100644 --- a/openmls/src/group/mls_group/config.rs +++ b/openmls/src/group/mls_group/config.rs @@ -79,7 +79,7 @@ impl MlsGroupJoinConfig { } /// Specifies configuration for the creation of an [`MlsGroup`]. Refer to the -/// [User Manual](https://openmls.tech/book/user_manual/group_config.html) for +/// [User Manual](https://book.openmls.tech/user_manual/group_config.html) for /// more information about the different configuration values. #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] pub struct MlsGroupCreateConfig { diff --git a/openmls/src/group/tests_and_kats/tests/commit_validation.rs b/openmls/src/group/tests_and_kats/tests/commit_validation.rs index 5c6d21d69..2d42d5ad6 100644 --- a/openmls/src/group/tests_and_kats/tests/commit_validation.rs +++ b/openmls/src/group/tests_and_kats/tests/commit_validation.rs @@ -1,5 +1,5 @@ //! This module tests the validation of commits as defined in -//! https://openmls.tech/book/message_validation.html#commit-message-validation +//! https://book.openmls.tech/message_validation.html#commit-message-validation use create_commit::CreateCommitParams; use openmls_traits::{prelude::*, signatures::Signer, types::Ciphersuite}; diff --git a/openmls/src/group/tests_and_kats/tests/framing_validation.rs b/openmls/src/group/tests_and_kats/tests/framing_validation.rs index dde81b6fb..e935e6517 100644 --- a/openmls/src/group/tests_and_kats/tests/framing_validation.rs +++ b/openmls/src/group/tests_and_kats/tests/framing_validation.rs @@ -1,5 +1,5 @@ //! This module tests the validation of message framing as defined in -//! https://openmls.tech/book/message_validation.html#semantic-validation-of-message-framing +//! https://book.openmls.tech/message_validation.html#semantic-validation-of-message-framing use openmls_traits::prelude::{openmls_types::Ciphersuite, *}; use tls_codec::{Deserialize, Serialize}; diff --git a/openmls/src/group/tests_and_kats/tests/proposal_validation.rs b/openmls/src/group/tests_and_kats/tests/proposal_validation.rs index 465a6743f..f313144b6 100644 --- a/openmls/src/group/tests_and_kats/tests/proposal_validation.rs +++ b/openmls/src/group/tests_and_kats/tests/proposal_validation.rs @@ -1,5 +1,5 @@ //! This module tests the validation of proposals as defined in -//! https://openmls.tech/book/message_validation.html#semantic-validation-of-proposals-covered-by-a-commit +//! https://book.openmls.tech/message_validation.html#semantic-validation-of-proposals-covered-by-a-commit use crate::{storage::OpenMlsProvider, test_utils::frankenstein::*, treesync::LeafNodeParameters}; use openmls_traits::{ diff --git a/openmls/src/lib.rs b/openmls/src/lib.rs index 1a5930560..b1da4ace2 100644 --- a/openmls/src/lib.rs +++ b/openmls/src/lib.rs @@ -136,7 +136,7 @@ //! ``` //! //! [//]: # "links and badges" -//! [user Manual]: https://openmls.tech/book +//! [user Manual]: https://book.openmls.tech #![cfg_attr(docsrs, feature(doc_cfg))] #![cfg_attr(not(test), forbid(unsafe_code))] #![cfg_attr(not(feature = "test-utils"), deny(missing_docs))] From 09cd6b862e09c4273c15437c15f5dc4872014ccd Mon Sep 17 00:00:00 2001 From: Franziskus Kiefer Date: Tue, 3 Sep 2024 17:00:34 +0200 Subject: [PATCH 08/15] bump crate versions for v0.6 --- basic_credential/Cargo.toml | 4 ++-- libcrux_crypto/Cargo.toml | 6 +++--- memory_storage/Cargo.toml | 4 ++-- openmls/Cargo.toml | 16 ++++++++-------- openmls_rust_crypto/Cargo.toml | 6 +++--- openmls_test/Cargo.toml | 8 ++++---- traits/Cargo.toml | 2 +- 7 files changed, 23 insertions(+), 23 deletions(-) diff --git a/basic_credential/Cargo.toml b/basic_credential/Cargo.toml index 507515bab..dc5997071 100644 --- a/basic_credential/Cargo.toml +++ b/basic_credential/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "openmls_basic_credential" -version = "0.3.0-pre.2" +version = "0.3.0" authors = ["OpenMLS Authors"] edition = "2021" description = "A Basic Credential implementation for OpenMLS" @@ -10,7 +10,7 @@ repository = "https://github.com/openmls/openmls/tree/main/basic_credential" readme = "README.md" [dependencies] -openmls_traits = { version = "0.3.0-pre.3", path = "../traits" } +openmls_traits = { version = "0.3.0", path = "../traits" } tls_codec = { workspace = true } serde = "1.0" diff --git a/libcrux_crypto/Cargo.toml b/libcrux_crypto/Cargo.toml index 789f71cc7..09d394d9b 100644 --- a/libcrux_crypto/Cargo.toml +++ b/libcrux_crypto/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "openmls_libcrux_crypto" -version = "0.1.0-pre.3" +version = "0.1.0" edition = "2021" authors = ["OpenMLS Authors"] description = "A crypto backend for OpenMLS based on libcrux implementing openmls_traits." @@ -12,7 +12,7 @@ readme = "../README.md" [dependencies] getrandom = "0.2.12" libcrux = { version = "=0.0.2-alpha.3", features = ["rand"] } -openmls_traits = { version = "0.3.0-pre.3", path = "../traits" } -openmls_memory_storage = { version = "0.3.0-pre.3", path = "../memory_storage" } +openmls_traits = { version = "0.3.0", path = "../traits" } +openmls_memory_storage = { version = "0.3.0", path = "../memory_storage" } rand = "0.8.5" tls_codec.workspace = true diff --git a/memory_storage/Cargo.toml b/memory_storage/Cargo.toml index 7833d71db..a3eba5fbb 100644 --- a/memory_storage/Cargo.toml +++ b/memory_storage/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "openmls_memory_storage" authors = ["OpenMLS Authors"] -version = "0.3.0-pre.3" +version = "0.3.0" edition = "2021" description = "A very basic storage for OpenMLS implementing openmls_traits." license = "MIT" @@ -10,7 +10,7 @@ repository = "https://github.com/openmls/openmls/tree/main/memory_storage" readme = "README.md" [dependencies] -openmls_traits = { version = "0.3.0-pre.3", path = "../traits" } +openmls_traits = { version = "0.3.0", path = "../traits" } thiserror = "1.0" serde_json = "1.0" serde = { version = "1.0", features = ["derive"] } diff --git a/openmls/Cargo.toml b/openmls/Cargo.toml index 6ca7d9470..07e6715fb 100644 --- a/openmls/Cargo.toml +++ b/openmls/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "openmls" -version = "0.6.0-pre.3" +version = "0.6.0" authors = ["OpenMLS Authors"] edition = "2021" description = "A Rust implementation of the Messaging Layer Security (MLS) protocol, as defined in RFC 9420." @@ -12,17 +12,17 @@ keywords = ["MLS", "IETF", "RFC9420", "Encryption", "E2EE"] exclude = ["/test_vectors"] [dependencies] -openmls_traits = { version = "0.3.0-pre.3", path = "../traits" } -openmls_rust_crypto = { version = "0.3.0-pre.2", path = "../openmls_rust_crypto", optional = true } -openmls_basic_credential = { version = "0.3.0-pre.2", path = "../basic_credential", optional = true, features = [ +openmls_traits = { version = "0.3.0", path = "../traits" } +openmls_rust_crypto = { version = "0.3.0", path = "../openmls_rust_crypto", optional = true } +openmls_basic_credential = { version = "0.3.0", path = "../basic_credential", optional = true, features = [ "clonable", "test-utils", ] } -openmls_memory_storage = { version = "0.3.0-pre.3", path = "../memory_storage", features = [ +openmls_memory_storage = { version = "0.3.0", path = "../memory_storage", features = [ "test-utils", ], optional = true } -openmls_test = { version = "0.1.0-pre.2", path = "../openmls_test", optional = true } -openmls_libcrux_crypto = { version = "0.1.0-pre.3", path = "../libcrux_crypto", optional = true } +openmls_test = { version = "0.1.0", path = "../openmls_test", optional = true } +openmls_libcrux_crypto = { version = "0.1.0", path = "../libcrux_crypto", optional = true } serde = { version = "^1.0", features = ["derive"] } log = { version = "0.4", features = ["std"] } tls_codec = { workspace = true } @@ -70,7 +70,7 @@ criterion = { version = "^0.5", default-features = false } # need to disable def hex = { version = "0.4", features = ["serde"] } itertools = "0.10" lazy_static = "1.4" -openmls_traits = { version = "0.3.0-pre.3", path = "../traits", features = [ +openmls_traits = { version = "0.3.0", path = "../traits", features = [ "test-utils", ] } pretty_env_logger = "0.5" diff --git a/openmls_rust_crypto/Cargo.toml b/openmls_rust_crypto/Cargo.toml index 1f914742e..4e299cf12 100644 --- a/openmls_rust_crypto/Cargo.toml +++ b/openmls_rust_crypto/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "openmls_rust_crypto" authors = ["OpenMLS Authors"] -version = "0.3.0-pre.2" +version = "0.3.0" edition = "2021" description = "A crypto backend for OpenMLS implementing openmls_traits using RustCrypto primitives." license = "MIT" @@ -10,8 +10,8 @@ repository = "https://github.com/openmls/openmls/tree/main/openmls_rust_crypto" readme = "README.md" [dependencies] -openmls_traits = { version = "0.3.0-pre.3", path = "../traits" } -openmls_memory_storage = { version = "0.3.0-pre.3", path = "../memory_storage" } +openmls_traits = { version = "0.3.0", path = "../traits" } +openmls_memory_storage = { version = "0.3.0", path = "../memory_storage" } hpke = { version = "0.2.0", package = "hpke-rs", default-features = false, features = [ "hazmat", "serialization", diff --git a/openmls_test/Cargo.toml b/openmls_test/Cargo.toml index 61d530ff7..bcf51692a 100644 --- a/openmls_test/Cargo.toml +++ b/openmls_test/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "openmls_test" -version = "0.1.0-pre.2" +version = "0.1.0" authors = ["OpenMLS Authors"] edition = "2021" description = "Test utility used by OpenMLS" @@ -23,6 +23,6 @@ ansi_term = "0.12.1" quote = "1.0" rstest = { version = "0.17" } rstest_reuse = { version = "0.5" } -openmls_rust_crypto = { version = "0.3.0-pre.2", path = "../openmls_rust_crypto" } -openmls_libcrux_crypto = { version = "0.1.0-pre.3", path = "../libcrux_crypto", optional = true } -openmls_traits = { version = "0.3.0-pre.3", path = "../traits" } +openmls_rust_crypto = { version = "0.3.0", path = "../openmls_rust_crypto" } +openmls_libcrux_crypto = { version = "0.1.0", path = "../libcrux_crypto", optional = true } +openmls_traits = { version = "0.3.0", path = "../traits" } diff --git a/traits/Cargo.toml b/traits/Cargo.toml index a8c6dbe28..7176e9fb5 100644 --- a/traits/Cargo.toml +++ b/traits/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "openmls_traits" -version = "0.3.0-pre.3" +version = "0.3.0" authors = ["OpenMLS Authors"] edition = "2021" description = "Traits used by OpenMLS" From 5a67488c4236a0082f56b063f5555e1dbf6322d4 Mon Sep 17 00:00:00 2001 From: Franziskus Kiefer Date: Tue, 3 Sep 2024 17:04:40 +0200 Subject: [PATCH 09/15] update changeloog for v0.6 --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 15f382821..ebc35c698 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## 0.6.0-pre.2 (2024-08-XX) +## 0.6.0 (2024-09-04) ### Added From 9ecb0ce50090bdda38bde6d1439ef02745ebee32 Mon Sep 17 00:00:00 2001 From: "Jan Winkelmann (keks)" Date: Thu, 5 Sep 2024 15:04:50 +0200 Subject: [PATCH 10/15] fix docs --- openmls/src/ciphersuite/signable.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openmls/src/ciphersuite/signable.rs b/openmls/src/ciphersuite/signable.rs index 52b885fb4..39b3f1267 100644 --- a/openmls/src/ciphersuite/signable.rs +++ b/openmls/src/ciphersuite/signable.rs @@ -131,12 +131,12 @@ pub trait Verifiable: Sized { pk: &OpenMlsSignaturePublicKey, ) -> Result; - /// Verifies the payload against the given `credential`. + /// Verifies the payload against the given public key. /// The signature is fetched via the [`Verifiable::signature()`] function and /// the payload via [`Verifiable::unsigned_payload()`]. /// /// Returns `Ok(())` if the signature is valid and - /// `CredentialError::InvalidSignature` otherwise. + /// [`SignatureError::VerificationError`] otherwise. fn verify_no_out( &self, crypto: &impl OpenMlsCrypto, From 39e55a087a7d5a26bd618d1a3a1debebf5f95b8b Mon Sep 17 00:00:00 2001 From: "Jan Winkelmann (keks)" Date: Tue, 17 Sep 2024 11:46:24 +0200 Subject: [PATCH 11/15] Add check and (basic) test for check 701: remove can't contain index to blank leaf --- openmls/src/group/public_group/validation.rs | 5 + .../tests_and_kats/tests/remove_operation.rs | 126 ++++++++++++++++++ 2 files changed, 131 insertions(+) diff --git a/openmls/src/group/public_group/validation.rs b/openmls/src/group/public_group/validation.rs index 828dcdcb3..eb3b919f6 100644 --- a/openmls/src/group/public_group/validation.rs +++ b/openmls/src/group/public_group/validation.rs @@ -423,6 +423,11 @@ impl PublicGroup { if !self.treesync().is_leaf_in_tree(removed) { return Err(ProposalValidationError::UnknownMemberRemoval); } + + // removed node can not be blank (check 701) + if self.treesync().leaf(removed).is_none() { + return Err(ProposalValidationError::UnknownMemberRemoval); + } } Ok(()) diff --git a/openmls/src/group/tests_and_kats/tests/remove_operation.rs b/openmls/src/group/tests_and_kats/tests/remove_operation.rs index d5daa0540..8bac5db00 100644 --- a/openmls/src/group/tests_and_kats/tests/remove_operation.rs +++ b/openmls/src/group/tests_and_kats/tests/remove_operation.rs @@ -4,6 +4,132 @@ use crate::group::tests_and_kats::utils::{generate_credential_with_key, generate use crate::{framing::*, group::*}; use openmls_traits::prelude::*; +#[openmls_test::openmls_test] +fn remove_blank() { + let group_id = GroupId::from_slice(b"Test Group"); + + let alice_provider = &Provider::default(); + let bob_provider = &Provider::default(); + let charlie_provider = &Provider::default(); + + // Generate credentials with keys + let alice_credential_with_key_and_signer = generate_credential_with_key( + "Alice".into(), + ciphersuite.signature_algorithm(), + alice_provider, + ); + + let bob_credential_with_key_and_signer = generate_credential_with_key( + "Bob".into(), + ciphersuite.signature_algorithm(), + bob_provider, + ); + + let charlie_credential_with_key_and_signer = generate_credential_with_key( + "Charlie".into(), + ciphersuite.signature_algorithm(), + charlie_provider, + ); + + // Generate KeyPackages + let bob_key_package = generate_key_package( + ciphersuite, + Extensions::empty(), + bob_provider, + bob_credential_with_key_and_signer.clone(), + ); + let charlie_key_package = generate_key_package( + ciphersuite, + Extensions::empty(), + charlie_provider, + charlie_credential_with_key_and_signer, + ); + + // Define the MlsGroup configuration + let mls_group_create_config = MlsGroupCreateConfig::builder() + .ciphersuite(ciphersuite) + .build(); + + // === Alice creates a group === + let mut alice_group = MlsGroup::new_with_group_id( + alice_provider, + &alice_credential_with_key_and_signer.signer, + &mls_group_create_config, + group_id, + alice_credential_with_key_and_signer + .credential_with_key + .clone(), + ) + .expect("An unexpected error occurred."); + + // === Alice adds Bob & Charlie === + + let (_message, welcome, _group_info) = alice_group + .add_members( + alice_provider, + &alice_credential_with_key_and_signer.signer, + &[ + bob_key_package.key_package().clone(), + charlie_key_package.key_package().clone(), + ], + ) + .expect("An unexpected error occurred."); + alice_group + .merge_pending_commit(alice_provider) + .expect("error merging pending commit"); + + let welcome: MlsMessageIn = welcome.into(); + let welcome = welcome + .into_welcome() + .expect("expected message to be a welcome"); + + let mut bob_group = StagedWelcome::new_from_welcome( + bob_provider, + mls_group_create_config.join_config(), + welcome.clone(), + Some(alice_group.export_ratchet_tree().into()), + ) + .expect("Error creating staged join from Welcome") + .into_group(bob_provider) + .expect("Error creating group from staged join"); + + let mut charlie_group = StagedWelcome::new_from_welcome( + charlie_provider, + mls_group_create_config.join_config(), + welcome, + Some(alice_group.export_ratchet_tree().into()), + ) + .expect("Error creating staged join from Welcome") + .into_group(charlie_provider) + .expect("Error creating group from staged join"); + + // === Remove operation === + + let bob_index = bob_group.own_leaf_index(); + + alice_group + .remove_members( + alice_provider, + &alice_credential_with_key_and_signer.signer, + &[bob_index], + ) + .expect("error removing member the first time"); + + alice_group.merge_pending_commit(alice_provider).unwrap(); + + // try removing bob a second time + alice_group + .remove_members( + alice_provider, + &alice_credential_with_key_and_signer.signer, + &[bob_index], + ) + .expect_err("using API to re-remove someone should fail"); + + // TODO: craft another remove commit that removes bob's (blank) index and check that merging + // that fails +} + // Tests the different variants of the RemoveOperation enum. #[openmls_test::openmls_test] fn test_remove_operation_variants() { From 9867c8b255196a9dc04c00e19c143608685da879 Mon Sep 17 00:00:00 2001 From: "Jan Winkelmann (keks)" Date: Thu, 19 Sep 2024 17:33:09 +0200 Subject: [PATCH 12/15] pacify clippy --- .../group/tests_and_kats/tests/remove_operation.rs | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/openmls/src/group/tests_and_kats/tests/remove_operation.rs b/openmls/src/group/tests_and_kats/tests/remove_operation.rs index 8bac5db00..4679559e1 100644 --- a/openmls/src/group/tests_and_kats/tests/remove_operation.rs +++ b/openmls/src/group/tests_and_kats/tests/remove_operation.rs @@ -83,7 +83,7 @@ fn remove_blank() { .into_welcome() .expect("expected message to be a welcome"); - let mut bob_group = StagedWelcome::new_from_welcome( + let bob_group = StagedWelcome::new_from_welcome( bob_provider, mls_group_create_config.join_config(), welcome.clone(), @@ -93,16 +93,6 @@ fn remove_blank() { .into_group(bob_provider) .expect("Error creating group from staged join"); - let mut charlie_group = StagedWelcome::new_from_welcome( - charlie_provider, - mls_group_create_config.join_config(), - welcome, - Some(alice_group.export_ratchet_tree().into()), - ) - .expect("Error creating staged join from Welcome") - .into_group(charlie_provider) - .expect("Error creating group from staged join"); - // === Remove operation === let bob_index = bob_group.own_leaf_index(); From 7493e0acfdbc5741bd182dba559c5ea68f0548b8 Mon Sep 17 00:00:00 2001 From: "Jan Winkelmann (keks)" Date: Thu, 19 Sep 2024 17:34:43 +0200 Subject: [PATCH 13/15] use new id scheme for searchability --- openmls/src/group/public_group/validation.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openmls/src/group/public_group/validation.rs b/openmls/src/group/public_group/validation.rs index eb3b919f6..8d6ec6e44 100644 --- a/openmls/src/group/public_group/validation.rs +++ b/openmls/src/group/public_group/validation.rs @@ -424,7 +424,7 @@ impl PublicGroup { return Err(ProposalValidationError::UnknownMemberRemoval); } - // removed node can not be blank (check 701) + // valn0701: removed node can not be blank if self.treesync().leaf(removed).is_none() { return Err(ProposalValidationError::UnknownMemberRemoval); } From ebcc6e43dbdc96896c2f8e67301dec13f114834d Mon Sep 17 00:00:00 2001 From: Lucas Franceschino Date: Wed, 2 Oct 2024 15:53:57 +0200 Subject: [PATCH 14/15] Update CONTRIBUTING.md --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 41d011a3f..fdcb67965 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -5,7 +5,7 @@ These are mostly guidelines, not rules. Use your best judgement, and feel free to propose changes to this document in a pull request. The processes described here is not to pester you but to increase and maintain code quality. -Before contributing, please read the [Code of Conduct](https://github.com/openmls/openmls/CODE_OF_CONDUCT.md) carefully. +Before contributing, please read the [Code of Conduct](CODE_OF_CONDUCT.md) carefully. #### Table Of Contents From 90d16048f390ad64ee0dfef924947e280f055596 Mon Sep 17 00:00:00 2001 From: Nicholas Molnar <65710+neekolas@users.noreply.github.com> Date: Fri, 4 Oct 2024 08:31:53 -0700 Subject: [PATCH 15/15] Update code to match new styles --- openmls/src/group/mls_group/membership.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/openmls/src/group/mls_group/membership.rs b/openmls/src/group/mls_group/membership.rs index 38b85e4be..50e1768b1 100644 --- a/openmls/src/group/mls_group/membership.rs +++ b/openmls/src/group/mls_group/membership.rs @@ -2,7 +2,7 @@ //! //! This module contains membership-related operations and exposes [`RemoveOperation`]. -use errors::EmptyInputError; +use errors::{EmptyInputError, UpdateGroupMembershipError}; use openmls_traits::{signatures::Signer, storage::StorageProvider as _}; use proposal_store::QueuedRemoveProposal; @@ -59,8 +59,10 @@ impl MlsGroup { let params = CreateCommitParams::builder() .framing_parameters(self.framing_parameters()) .inline_proposals(proposals) + .force_self_update(true) .build(); - let create_commit_result = self.group.create_commit(params, provider, signer)?; + + let create_commit_result = self.create_commit(params, provider, signer)?; // Convert PublicMessage messages to MLSMessage and encrypt them if required by // the configuration @@ -83,7 +85,7 @@ impl MlsGroup { mls_messages, create_commit_result .welcome_option - .map(|w| MlsMessageOut::from_welcome(w, self.group.version())), + .map(|w| MlsMessageOut::from_welcome(w, self.version())), create_commit_result.group_info, )) } @@ -350,8 +352,7 @@ impl MlsGroup { /// Returns the [`Member`] corresponding to the given /// leaf index. Returns `None` if the member can not be found in this group. pub fn member_at(&self, leaf_index: LeafNodeIndex) -> Option { - self.group - .public_group() + self.public_group() // This will return an error if the member can't be found. .leaf(leaf_index) .map(|leaf_node| {