From fa33f5ef12a7c3eec98c8527f21d355c472a1576 Mon Sep 17 00:00:00 2001 From: cameronvoell Date: Wed, 3 Apr 2024 11:21:34 -0700 Subject: [PATCH 1/9] remove test only distinction for group_context_ext_proposal --- openmls/src/group/core_group/mod.rs | 9 +++++---- openmls/src/group/mls_group/proposal.rs | 1 - openmls/src/messages/proposals.rs | 1 - 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/openmls/src/group/core_group/mod.rs b/openmls/src/group/core_group/mod.rs index b3f02738c1..992b5aa775 100644 --- a/openmls/src/group/core_group/mod.rs +++ b/openmls/src/group/core_group/mod.rs @@ -1043,11 +1043,7 @@ impl CoreGroup { group_info: group_info.filter(|_| self.use_ratchet_tree_extension), }) } -} -// Test functions -#[cfg(test)] -impl CoreGroup { pub(crate) fn create_group_context_ext_proposal( &self, framing_parameters: FramingParameters, @@ -1082,6 +1078,11 @@ impl CoreGroup { ) .map_err(|e| e.into()) } +} + +// Test functions +#[cfg(test)] +impl CoreGroup { pub(crate) fn use_ratchet_tree_extension(&self) -> bool { self.use_ratchet_tree_extension diff --git a/openmls/src/group/mls_group/proposal.rs b/openmls/src/group/mls_group/proposal.rs index bc00077640..8d5f2be753 100644 --- a/openmls/src/group/mls_group/proposal.rs +++ b/openmls/src/group/mls_group/proposal.rs @@ -319,7 +319,6 @@ impl MlsGroup { } } - #[cfg(test)] pub fn propose_group_context_extensions( &mut self, provider: &impl OpenMlsProvider, diff --git a/openmls/src/messages/proposals.rs b/openmls/src/messages/proposals.rs index 6bf5813c55..fd1e0953a1 100644 --- a/openmls/src/messages/proposals.rs +++ b/openmls/src/messages/proposals.rs @@ -494,7 +494,6 @@ pub struct GroupContextExtensionProposal { impl GroupContextExtensionProposal { /// Create a new [`GroupContextExtensionProposal`]. - #[cfg(test)] pub(crate) fn new(extensions: Extensions) -> Self { Self { extensions } } From c6bc4cd804ae91190f0f380319e40524a6648074 Mon Sep 17 00:00:00 2001 From: cameronvoell Date: Tue, 9 Apr 2024 09:47:02 -0700 Subject: [PATCH 2/9] Added test and new update_group_context_extensions function --- openmls/src/group/errors.rs | 2 + openmls/src/group/mls_group/proposal.rs | 52 +++++- openmls/src/group/mls_group/test_mls_group.rs | 163 ++++++++++++++++++ openmls/src/messages/proposals.rs | 2 +- 4 files changed, 215 insertions(+), 4 deletions(-) diff --git a/openmls/src/group/errors.rs b/openmls/src/group/errors.rs index 81623b13be..6d09b153b4 100644 --- a/openmls/src/group/errors.rs +++ b/openmls/src/group/errors.rs @@ -507,6 +507,8 @@ pub enum CreateGroupContextExtProposalError { /// See [`LeafNodeValidationError`] for more details. #[error(transparent)] LeafNodeValidation(#[from] LeafNodeValidationError), + #[error(transparent)] + GroupStateError(#[from] MlsGroupStateError), } /// Error merging a commit. diff --git a/openmls/src/group/mls_group/proposal.rs b/openmls/src/group/mls_group/proposal.rs index 8d5f2be753..1bb0eeac13 100644 --- a/openmls/src/group/mls_group/proposal.rs +++ b/openmls/src/group/mls_group/proposal.rs @@ -1,10 +1,10 @@ +use core_group::create_commit_params::CreateCommitParams; use openmls_traits::{ key_store::OpenMlsKeyStore, signatures::Signer, types::Ciphersuite, OpenMlsProvider, }; use super::{ - errors::{ProposalError, ProposeAddMemberError, ProposeRemoveMemberError}, - MlsGroup, + core_group, errors::{ProposalError, ProposeAddMemberError, ProposeRemoveMemberError}, CreateGroupContextExtProposalError, GroupContextExtensionProposal, GroupContextExtensionsProposalValidationError, MlsGroup, MlsGroupState, PendingCommitState, Proposal }; use crate::{ binary_tree::LeafNodeIndex, @@ -14,7 +14,7 @@ use crate::{ framing::MlsMessageOut, group::{errors::CreateAddProposalError, GroupId, QueuedProposal}, key_packages::KeyPackage, - messages::proposals::ProposalOrRefType, + messages::{group_info::GroupInfo, proposals::ProposalOrRefType}, prelude::LibraryError, schedule::PreSharedKeyId, treesync::LeafNode, @@ -349,4 +349,50 @@ impl MlsGroup { Ok((mls_message, proposal_ref)) } + + pub fn update_group_context_extensions( + &mut self, + provider: &impl OpenMlsProvider, + extensions: Extensions, + signer: &impl Signer, + ) -> Result<(MlsMessageOut, MlsMessageOut, Option), CreateGroupContextExtProposalError> + { + self.is_operational()?; + + // if key_packages.is_empty() { + // return Err(CreateGroupContextExtProposalError::EmptyInput(EmptyInputError::AddMembers)); + // } + + // Create inline add proposals from key packages + let mut inline_proposals = vec![]; + inline_proposals.push(Proposal::GroupContextExtensions(GroupContextExtensionProposal { + extensions, + })); + + let params = CreateCommitParams::builder() + .framing_parameters(self.framing_parameters()) + .proposal_store(&self.proposal_store) + .inline_proposals(inline_proposals) + .build(); + let create_commit_result = self.group.create_commit(params, provider, signer).unwrap(); + let welcome = match create_commit_result.welcome_option { + Some(welcome) => welcome, + None => { + return Err(LibraryError::custom("No secrets to generate commit message.").into()) + } + }; + let mls_messages = self.content_to_mls_message(create_commit_result.commit, provider)?; + self.group_state = MlsGroupState::PendingCommit(Box::new(PendingCommitState::Member( + create_commit_result.staged_commit, + ))); + + // Since the state of the group might be changed, arm the state flag + self.flag_state_change(); + + Ok(( + mls_messages, + MlsMessageOut::from_welcome(welcome, self.group.version()), + create_commit_result.group_info, + )) + } } diff --git a/openmls/src/group/mls_group/test_mls_group.rs b/openmls/src/group/mls_group/test_mls_group.rs index 1a58e4877f..466e972735 100644 --- a/openmls/src/group/mls_group/test_mls_group.rs +++ b/openmls/src/group/mls_group/test_mls_group.rs @@ -1544,6 +1544,169 @@ fn unknown_extensions(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) .expect("Error creating group from staged join"); } +// Test the successful update of Group Context Extension with type Extension::Unknown(0xff11) +#[apply(ciphersuites_and_providers)] +fn update_group_context_with_unknown_extension( + ciphersuite: Ciphersuite, + provider: &impl OpenMlsProvider, +) { + let (alice_credential_with_key, _alice_kpb, alice_signer, _alice_pk) = + setup_client("Alice", ciphersuite, provider); + + // === Define the unknown group context extension and initial data === + let unknown_extension_data = vec![1, 2]; + let unknown_gc_extension = Extension::Unknown(0xff11, UnknownExtension(unknown_extension_data)); + let required_extension_types = &[ExtensionType::Unknown(0xff11)]; + let required_capabilities = Extension::RequiredCapabilities( + RequiredCapabilitiesExtension::new(required_extension_types, &[], &[]), + ); + let capabilities = Capabilities::new(None, None, Some(required_extension_types), None, None); + let test_gc_extensions = Extensions::from_vec(vec![ + unknown_gc_extension.clone(), + required_capabilities.clone(), + ]) + .expect("error creating test group context extensions"); + let mls_group_create_config = MlsGroupCreateConfig::builder() + .with_group_context_extensions(test_gc_extensions.clone()) + .expect("error adding unknown extension to config") + .capabilities(capabilities.clone()) + .crypto_config(CryptoConfig::with_default_version(ciphersuite)) + .build(); + + // === Alice creates a group === + let mut alice_group = MlsGroup::new( + provider, + &alice_signer, + &mls_group_create_config, + alice_credential_with_key, + ) + .expect("error creating group"); + + // === Verify the initial group context extension data is correct === + let group_context_extensions = alice_group.group().context().extensions(); + let mut extracted_data = None; + for extension in group_context_extensions.iter() { + if let Extension::Unknown(0xff11, UnknownExtension(data)) = extension { + extracted_data = Some(data.clone()); + } + } + assert_eq!( + extracted_data.unwrap(), + vec![1, 2], + "The data of Extension::Unknown(0xff11) does not match the expected data" + ); + + // === Alice adds Bob === + let (bob_credential_with_key, _bob_kpb, bob_signer, _bob_pk) = + setup_client("Bob", ciphersuite, provider); + + let bob_key_package = KeyPackage::builder() + .leaf_node_capabilities(capabilities) + .build( + CryptoConfig::with_default_version(ciphersuite), + provider, + &bob_signer, + bob_credential_with_key, + ) + .expect("error building key package"); + + let (_, welcome, _) = alice_group + .add_members(provider, &alice_signer, &[bob_key_package.clone()]) + .unwrap(); + alice_group.merge_pending_commit(provider).unwrap(); + + let welcome: MlsMessageIn = welcome.into(); + let welcome = welcome + .into_welcome() + .expect("expected message to be a welcome"); + + let bob_group = StagedWelcome::new_from_welcome( + provider, + &MlsGroupJoinConfig::default(), + welcome, + Some(alice_group.export_ratchet_tree().into()), + ) + .expect("Error creating staged join from Welcome") + .into_group(provider) + .expect("Error creating group from staged join"); + + // === Verify Bob's initial group context extension data is correct === + let group_context_extensions = bob_group.group().context().extensions(); + let mut extracted_data_2 = None; + for extension in group_context_extensions.iter() { + if let Extension::Unknown(0xff11, UnknownExtension(data)) = extension { + extracted_data_2 = Some(data.clone()); + } + } + assert_eq!( + extracted_data_2.unwrap(), + vec![1, 2], + "The data of Extension::Unknown(0xff11) does not match the expected data" + ); + + // === Propose the new group context extension === + let updated_unknown_extension_data = vec![3, 4]; // Sample data for the extension + let updated_unknown_gc_extension = Extension::Unknown( + 0xff11, + UnknownExtension(updated_unknown_extension_data.clone()), + ); + + let mut updated_extensions = test_gc_extensions.clone(); + updated_extensions.add_or_replace(updated_unknown_gc_extension); + alice_group + .propose_group_context_extensions(provider, updated_extensions, &alice_signer) + .expect("failed to propose group context extensions with unknown extension"); + + assert_eq!( + alice_group.pending_proposals().count(), + 1, + "Expected one pending proposal" + ); + + // === Commit to the proposed group context extension === + alice_group + .commit_to_pending_proposals(provider, &alice_signer) + .expect("failed to commit to pending group context extensions"); + + alice_group + .merge_pending_commit(provider) + .expect("error merging pending commit"); + + alice_group + .save(provider.key_store()) + .expect("error saving group"); + + // === Verify the group context extension was updated === + let group_context_extensions = alice_group.group().context().extensions(); + let mut extracted_data_updated = None; + for extension in group_context_extensions.iter() { + if let Extension::Unknown(0xff11, UnknownExtension(data)) = extension { + extracted_data_updated = Some(data.clone()); + } + } + assert_eq!( + extracted_data_updated.unwrap(), + vec![3, 4], + "The data of Extension::Unknown(0xff11) does not match the expected data" + ); + + // === Verify Bob sees the group context extension updated === + let bob_group_loaded = MlsGroup::load(bob_group.group().group_id(), provider.key_store()) + .expect("error loading group"); + let group_context_extensions_2 = bob_group_loaded.export_group_context().extensions(); + let mut extracted_data_2 = None; + for extension in group_context_extensions_2.iter() { + if let Extension::Unknown(0xff11, UnknownExtension(data)) = extension { + extracted_data_2 = Some(data.clone()); + } + } + assert_eq!( + extracted_data_2.unwrap(), + vec![3, 4], + "The data of Extension::Unknown(0xff11) does not match the expected data" + ); +} + #[apply(ciphersuites_and_providers)] fn join_multiple_groups_last_resort_extension( ciphersuite: Ciphersuite, diff --git a/openmls/src/messages/proposals.rs b/openmls/src/messages/proposals.rs index fd1e0953a1..a754aecc9e 100644 --- a/openmls/src/messages/proposals.rs +++ b/openmls/src/messages/proposals.rs @@ -489,7 +489,7 @@ pub struct AppAckProposal { TlsSize, )] pub struct GroupContextExtensionProposal { - extensions: Extensions, + pub(crate) extensions: Extensions, } impl GroupContextExtensionProposal { From 8c58f21a17fb231f3a8ba6717bd0f51e6182b595 Mon Sep 17 00:00:00 2001 From: cameronvoell Date: Tue, 9 Apr 2024 09:59:43 -0700 Subject: [PATCH 3/9] change how we return welcome --- openmls/src/group/mls_group/proposal.rs | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/openmls/src/group/mls_group/proposal.rs b/openmls/src/group/mls_group/proposal.rs index 1bb0eeac13..d3bd05c3e1 100644 --- a/openmls/src/group/mls_group/proposal.rs +++ b/openmls/src/group/mls_group/proposal.rs @@ -355,14 +355,10 @@ impl MlsGroup { provider: &impl OpenMlsProvider, extensions: Extensions, signer: &impl Signer, - ) -> Result<(MlsMessageOut, MlsMessageOut, Option), CreateGroupContextExtProposalError> + ) -> Result<(MlsMessageOut, Option, Option), CreateGroupContextExtProposalError> { self.is_operational()?; - // if key_packages.is_empty() { - // return Err(CreateGroupContextExtProposalError::EmptyInput(EmptyInputError::AddMembers)); - // } - // Create inline add proposals from key packages let mut inline_proposals = vec![]; inline_proposals.push(Proposal::GroupContextExtensions(GroupContextExtensionProposal { @@ -375,12 +371,7 @@ impl MlsGroup { .inline_proposals(inline_proposals) .build(); let create_commit_result = self.group.create_commit(params, provider, signer).unwrap(); - let welcome = match create_commit_result.welcome_option { - Some(welcome) => welcome, - None => { - return Err(LibraryError::custom("No secrets to generate commit message.").into()) - } - }; + let mls_messages = self.content_to_mls_message(create_commit_result.commit, provider)?; self.group_state = MlsGroupState::PendingCommit(Box::new(PendingCommitState::Member( create_commit_result.staged_commit, @@ -391,7 +382,9 @@ impl MlsGroup { Ok(( mls_messages, - MlsMessageOut::from_welcome(welcome, self.group.version()), + create_commit_result + .welcome_option + .map(|w| MlsMessageOut::from_welcome(w, self.group.version())), create_commit_result.group_info, )) } From 7ed6765d365418a2aa61908ee2c128ffde010443 Mon Sep 17 00:00:00 2001 From: cameronvoell Date: Wed, 10 Apr 2024 11:02:46 -0700 Subject: [PATCH 4/9] added function comments and updated formatting --- openmls/src/group/core_group/mod.rs | 2 +- openmls/src/group/mls_group/proposal.rs | 26 ++++++++++++++++--------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/openmls/src/group/core_group/mod.rs b/openmls/src/group/core_group/mod.rs index 992b5aa775..ed9eaefb80 100644 --- a/openmls/src/group/core_group/mod.rs +++ b/openmls/src/group/core_group/mod.rs @@ -1044,6 +1044,7 @@ impl CoreGroup { }) } + /// Create a new group context extension proposal pub(crate) fn create_group_context_ext_proposal( &self, framing_parameters: FramingParameters, @@ -1083,7 +1084,6 @@ impl CoreGroup { // Test functions #[cfg(test)] impl CoreGroup { - pub(crate) fn use_ratchet_tree_extension(&self) -> bool { self.use_ratchet_tree_extension } diff --git a/openmls/src/group/mls_group/proposal.rs b/openmls/src/group/mls_group/proposal.rs index d3bd05c3e1..f636288601 100644 --- a/openmls/src/group/mls_group/proposal.rs +++ b/openmls/src/group/mls_group/proposal.rs @@ -4,7 +4,10 @@ use openmls_traits::{ }; use super::{ - core_group, errors::{ProposalError, ProposeAddMemberError, ProposeRemoveMemberError}, CreateGroupContextExtProposalError, GroupContextExtensionProposal, GroupContextExtensionsProposalValidationError, MlsGroup, MlsGroupState, PendingCommitState, Proposal + core_group, + errors::{ProposalError, ProposeAddMemberError, ProposeRemoveMemberError}, + CreateGroupContextExtProposalError, GroupContextExtensionProposal, MlsGroup, MlsGroupState, + PendingCommitState, Proposal, }; use crate::{ binary_tree::LeafNodeIndex, @@ -319,6 +322,10 @@ impl MlsGroup { } } + /// Creates a proposals with a new set of `extensions` for the group context. + /// + /// Returns an error when the group does not support all the required capabilities + /// in the new `extensions`. pub fn propose_group_context_extensions( &mut self, provider: &impl OpenMlsProvider, @@ -355,15 +362,16 @@ impl MlsGroup { provider: &impl OpenMlsProvider, extensions: Extensions, signer: &impl Signer, - ) -> Result<(MlsMessageOut, Option, Option), CreateGroupContextExtProposalError> - { + ) -> Result< + (MlsMessageOut, Option, Option), + CreateGroupContextExtProposalError, + > { self.is_operational()?; - // Create inline add proposals from key packages - let mut inline_proposals = vec![]; - inline_proposals.push(Proposal::GroupContextExtensions(GroupContextExtensionProposal { - extensions, - })); + // Create group context extension proposals + let inline_proposals = vec![Proposal::GroupContextExtensions( + GroupContextExtensionProposal { extensions }, + )]; let params = CreateCommitParams::builder() .framing_parameters(self.framing_parameters()) @@ -371,7 +379,7 @@ impl MlsGroup { .inline_proposals(inline_proposals) .build(); let create_commit_result = self.group.create_commit(params, provider, signer).unwrap(); - + let mls_messages = self.content_to_mls_message(create_commit_result.commit, provider)?; self.group_state = MlsGroupState::PendingCommit(Box::new(PendingCommitState::Member( create_commit_result.staged_commit, From 73950043a0182ea11896aed646916a8572d5fca6 Mon Sep 17 00:00:00 2001 From: cameronvoell Date: Wed, 10 Apr 2024 11:05:58 -0700 Subject: [PATCH 5/9] cleaned up import --- openmls/src/group/mls_group/proposal.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/openmls/src/group/mls_group/proposal.rs b/openmls/src/group/mls_group/proposal.rs index f636288601..6cfffc3ea8 100644 --- a/openmls/src/group/mls_group/proposal.rs +++ b/openmls/src/group/mls_group/proposal.rs @@ -1,10 +1,9 @@ -use core_group::create_commit_params::CreateCommitParams; use openmls_traits::{ key_store::OpenMlsKeyStore, signatures::Signer, types::Ciphersuite, OpenMlsProvider, }; use super::{ - core_group, + core_group::create_commit_params::CreateCommitParams, errors::{ProposalError, ProposeAddMemberError, ProposeRemoveMemberError}, CreateGroupContextExtProposalError, GroupContextExtensionProposal, MlsGroup, MlsGroupState, PendingCommitState, Proposal, From eca0be9712ac00aff2474cd385624657c84d6e97 Mon Sep 17 00:00:00 2001 From: cameronvoell Date: Wed, 10 Apr 2024 12:03:55 -0700 Subject: [PATCH 6/9] removed unwrap in update_group_context_extensions --- openmls/src/group/core_group/mod.rs | 4 ++-- .../src/group/core_group/test_proposals.rs | 8 ++++---- openmls/src/group/errors.rs | 4 +++- openmls/src/group/mls_group/errors.rs | 2 +- openmls/src/group/mls_group/proposal.rs | 20 +++++++++++-------- 5 files changed, 22 insertions(+), 16 deletions(-) diff --git a/openmls/src/group/core_group/mod.rs b/openmls/src/group/core_group/mod.rs index ed9eaefb80..af0dca4164 100644 --- a/openmls/src/group/core_group/mod.rs +++ b/openmls/src/group/core_group/mod.rs @@ -1045,12 +1045,12 @@ impl CoreGroup { } /// Create a new group context extension proposal - pub(crate) fn create_group_context_ext_proposal( + pub(crate) fn create_group_context_ext_proposal( &self, framing_parameters: FramingParameters, extensions: Extensions, signer: &impl Signer, - ) -> Result { + ) -> Result> { // Ensure that the group supports all the extensions that are wanted. let required_extension = extensions .iter() diff --git a/openmls/src/group/core_group/test_proposals.rs b/openmls/src/group/core_group/test_proposals.rs index c1972e08a2..41ac185d23 100644 --- a/openmls/src/group/core_group/test_proposals.rs +++ b/openmls/src/group/core_group/test_proposals.rs @@ -1,5 +1,5 @@ use openmls_rust_crypto::OpenMlsRustCrypto; -use openmls_traits::{types::Ciphersuite, OpenMlsProvider}; +use openmls_traits::{key_store::OpenMlsKeyStore, types::Ciphersuite, OpenMlsProvider}; use super::CoreGroup; use crate::{ @@ -543,9 +543,9 @@ fn test_group_context_extension_proposal_fails( } #[apply(ciphersuites_and_providers)] -fn test_group_context_extension_proposal( +fn test_group_context_extension_proposal( ciphersuite: Ciphersuite, - provider: &impl OpenMlsProvider, + provider: &impl OpenMlsProvider, ) { // Basic group setup. let group_aad = b"Alice's test group"; @@ -617,7 +617,7 @@ fn test_group_context_extension_proposal( &[CredentialType::Basic], )); let gce_proposal = alice_group - .create_group_context_ext_proposal( + .create_group_context_ext_proposal::( framing_parameters, Extensions::single(required_application_id), &alice_signer, diff --git a/openmls/src/group/errors.rs b/openmls/src/group/errors.rs index 6d09b153b4..b316fc62da 100644 --- a/openmls/src/group/errors.rs +++ b/openmls/src/group/errors.rs @@ -494,7 +494,7 @@ pub(crate) enum CoreGroupParseMessageError { /// Create group context ext proposal error #[derive(Error, Debug, PartialEq, Clone)] -pub enum CreateGroupContextExtProposalError { +pub enum CreateGroupContextExtProposalError { /// See [`LibraryError`] for more details. #[error(transparent)] LibraryError(#[from] LibraryError), @@ -509,6 +509,8 @@ pub enum CreateGroupContextExtProposalError { LeafNodeValidation(#[from] LeafNodeValidationError), #[error(transparent)] GroupStateError(#[from] MlsGroupStateError), + #[error(transparent)] + CreateCommitError(#[from] CreateCommitError), } /// Error merging a commit. diff --git a/openmls/src/group/mls_group/errors.rs b/openmls/src/group/mls_group/errors.rs index 2b7a4fc207..01d4c82744 100644 --- a/openmls/src/group/mls_group/errors.rs +++ b/openmls/src/group/mls_group/errors.rs @@ -322,5 +322,5 @@ pub enum ProposalError { ValidationError(#[from] ValidationError), /// See [`CreateGroupContextExtProposalError`] for more details. #[error(transparent)] - CreateGroupContextExtProposalError(#[from] CreateGroupContextExtProposalError), + CreateGroupContextExtProposalError(#[from] CreateGroupContextExtProposalError), } diff --git a/openmls/src/group/mls_group/proposal.rs b/openmls/src/group/mls_group/proposal.rs index 6cfffc3ea8..82b34e78e0 100644 --- a/openmls/src/group/mls_group/proposal.rs +++ b/openmls/src/group/mls_group/proposal.rs @@ -325,15 +325,15 @@ impl MlsGroup { /// /// Returns an error when the group does not support all the required capabilities /// in the new `extensions`. - pub fn propose_group_context_extensions( + pub fn propose_group_context_extensions( &mut self, - provider: &impl OpenMlsProvider, + provider: &impl OpenMlsProvider, extensions: Extensions, signer: &impl Signer, - ) -> Result<(MlsMessageOut, ProposalRef), ProposalError<()>> { + ) -> Result<(MlsMessageOut, ProposalRef), ProposalError> { self.is_operational()?; - let proposal = self.group.create_group_context_ext_proposal( + let proposal = self.group.create_group_context_ext_proposal::( self.framing_parameters(), extensions, signer, @@ -356,14 +356,18 @@ impl MlsGroup { Ok((mls_message, proposal_ref)) } - pub fn update_group_context_extensions( + /// Updates group context extensions + /// + /// Returns an error when the group does not support all the required capabilities + /// in the new `extensions`. + pub fn update_group_context_extensions( &mut self, - provider: &impl OpenMlsProvider, + provider: &impl OpenMlsProvider, extensions: Extensions, signer: &impl Signer, ) -> Result< (MlsMessageOut, Option, Option), - CreateGroupContextExtProposalError, + CreateGroupContextExtProposalError, > { self.is_operational()?; @@ -377,7 +381,7 @@ impl MlsGroup { .proposal_store(&self.proposal_store) .inline_proposals(inline_proposals) .build(); - let create_commit_result = self.group.create_commit(params, provider, signer).unwrap(); + let create_commit_result = self.group.create_commit(params, provider, signer)?; let mls_messages = self.content_to_mls_message(create_commit_result.commit, provider)?; self.group_state = MlsGroupState::PendingCommit(Box::new(PendingCommitState::Member( From 58b8bdeb4e69c5e88b0bf17dbc82478120701b71 Mon Sep 17 00:00:00 2001 From: cameronvoell Date: Wed, 10 Apr 2024 12:35:49 -0700 Subject: [PATCH 7/9] add comments for errors. ignore clippy on type complexity for now --- openmls/src/group/errors.rs | 4 +++- openmls/src/group/mls_group/proposal.rs | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/openmls/src/group/errors.rs b/openmls/src/group/errors.rs index b316fc62da..c4bd00a17c 100644 --- a/openmls/src/group/errors.rs +++ b/openmls/src/group/errors.rs @@ -507,8 +507,10 @@ pub enum CreateGroupContextExtProposalError { /// See [`LeafNodeValidationError`] for more details. #[error(transparent)] LeafNodeValidation(#[from] LeafNodeValidationError), + /// See [`MlsGroupStateError`] for more details. #[error(transparent)] - GroupStateError(#[from] MlsGroupStateError), + MlsGroupStateError(#[from] MlsGroupStateError), + /// See [`CreateCommitError`] for more details. #[error(transparent)] CreateCommitError(#[from] CreateCommitError), } diff --git a/openmls/src/group/mls_group/proposal.rs b/openmls/src/group/mls_group/proposal.rs index 82b34e78e0..e5593d5b89 100644 --- a/openmls/src/group/mls_group/proposal.rs +++ b/openmls/src/group/mls_group/proposal.rs @@ -360,6 +360,7 @@ impl MlsGroup { /// /// Returns an error when the group does not support all the required capabilities /// in the new `extensions`. + #[allow(clippy::type_complexity)] pub fn update_group_context_extensions( &mut self, provider: &impl OpenMlsProvider, From 88bf75ce1681bf0e542351324394bd623ff98c60 Mon Sep 17 00:00:00 2001 From: raphaelrobert Date: Wed, 3 Apr 2024 15:21:48 +0200 Subject: [PATCH 8/9] Clippy fix for CI --- openmls/src/framing/mls_content.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/openmls/src/framing/mls_content.rs b/openmls/src/framing/mls_content.rs index fff459e171..c70ffb00e4 100644 --- a/openmls/src/framing/mls_content.rs +++ b/openmls/src/framing/mls_content.rs @@ -227,7 +227,8 @@ pub(crate) fn framed_content_tbs_serialized<'context, W: Write>( // NewMemberCommit. written += match serialized_context.into() { Some(context) if matches!(sender, Sender::Member(_) | Sender::NewMemberCommit) => { - writer.write(context)? + writer.write_all(context)?; + context.len() } _ => 0, }; From ad48359e42311caa28e8d79c392c3285eb116111 Mon Sep 17 00:00:00 2001 From: Konrad Kohbrok Date: Tue, 9 Apr 2024 12:22:12 +0200 Subject: [PATCH 9/9] Disable MLS++ interop on CI (#1555) Due to a recent vcpkg failure (in turn due to a bug in CMake), this PR disables interop testing with MLS++ on CI. Co-authored-by: raphaelrobert --- .github/workflows/interop.yml | 95 ++++++++++++++++++++++------------- 1 file changed, 59 insertions(+), 36 deletions(-) diff --git a/.github/workflows/interop.yml b/.github/workflows/interop.yml index 161176372d..3ab21bb1fc 100644 --- a/.github/workflows/interop.yml +++ b/.github/workflows/interop.yml @@ -26,39 +26,39 @@ jobs: # --------------------------------------------------------------------------------------- - - name: MLS++ | Checkout - run: | - git clone https://github.com/cisco/mlspp.git - cd mlspp - git checkout 623acd0839d1117e8665b6bd52eecad1ce05438d - - - name: MLS++ | Install dependencies | 1/2 - uses: lukka/run-vcpkg@v11 - with: - vcpkgDirectory: "${{ github.workspace }}/vcpkg" - vcpkgGitCommitId: "70992f64912b9ab0e60e915ab7421faa197524b7" - vcpkgJsonGlob: "mlspp/vcpkg.json" - runVcpkgInstall: true - - - name: MLS++ | Install dependencies | 2/2 - uses: lukka/run-vcpkg@v11 - with: - vcpkgDirectory: "${{ github.workspace }}/vcpkg" - vcpkgGitCommitId: "70992f64912b9ab0e60e915ab7421faa197524b7" - vcpkgJsonGlob: "mlspp/cmd/interop/vcpkg.json" - runVcpkgInstall: true - - - name: MLS++ | Build | 1/2 - working-directory: mlspp - run: | - cmake . -DCMAKE_TOOLCHAIN_FILE=${{ github.workspace }}/vcpkg/scripts/buildsystems/vcpkg.cmake - make - - - name: MLS++ | Build | 2/2 - working-directory: mlspp/cmd/interop - run: | - cmake . -DCMAKE_TOOLCHAIN_FILE=${{ github.workspace }}/vcpkg/scripts/buildsystems/vcpkg.cmake - make + #- name: MLS++ | Checkout + # run: | + # git clone https://github.com/cisco/mlspp.git + # cd mlspp + # git checkout 623acd0839d1117e8665b6bd52eecad1ce05438d + + #- name: MLS++ | Install dependencies | 1/2 + # uses: lukka/run-vcpkg@v11 + # with: + # vcpkgDirectory: "${{ github.workspace }}/vcpkg" + # vcpkgGitCommitId: "70992f64912b9ab0e60e915ab7421faa197524b7" + # vcpkgJsonGlob: "mlspp/vcpkg.json" + # runVcpkgInstall: true + + #- name: MLS++ | Install dependencies | 2/2 + # uses: lukka/run-vcpkg@v11 + # with: + # vcpkgDirectory: "${{ github.workspace }}/vcpkg" + # vcpkgGitCommitId: "70992f64912b9ab0e60e915ab7421faa197524b7" + # vcpkgJsonGlob: "mlspp/cmd/interop/vcpkg.json" + # runVcpkgInstall: true + + #- name: MLS++ | Build | 1/2 + # working-directory: mlspp + # run: | + # cmake . -DCMAKE_TOOLCHAIN_FILE=${{ github.workspace }}/vcpkg/scripts/buildsystems/vcpkg.cmake + # make + + #- name: MLS++ | Build | 2/2 + # working-directory: mlspp/cmd/interop + # run: | + # cmake . -DCMAKE_TOOLCHAIN_FILE=${{ github.workspace }}/vcpkg/scripts/buildsystems/vcpkg.cmake + # make # --------------------------------------------------------------------------------------- @@ -89,10 +89,32 @@ jobs: # --------------------------------------------------------------------------------------- - - name: Test interoperability +# - name: Test interoperability +# run: | +# ./target/debug/interop_client& +# ./mlspp/cmd/interop/mlspp_client -live 12345& +# +# cd mls-implementations/interop +# # TODO(#1238): +# # * Add `commit.json` as soon as group context extensions proposals are supported. +# # Note: It's also possible to remove GCE proposals by hand from `commit.json`. +# # But let's not do this in CI for now and hope that it isn't needed anymore soon. +# for scenario in {welcome_join.json,external_join.json,application.json}; +# do +# echo Running configs/$scenario +# errors=$(./test-runner/test-runner -fail-fast -client localhost:50051 -client localhost:12345 -config=configs/$scenario | grep error | wc -l) +# if [ "$errors" = "0" ]; +# then +# echo "Success"; +# else +# echo "Failed"; +# exit 1; +# fi +# done + + - name: Test interoperability (OpenMLS only) run: | ./target/debug/interop_client& - ./mlspp/cmd/interop/mlspp_client -live 12345& cd mls-implementations/interop # TODO(#1238): @@ -102,7 +124,7 @@ jobs: for scenario in {welcome_join.json,external_join.json,application.json}; do echo Running configs/$scenario - errors=$(./test-runner/test-runner -fail-fast -client localhost:50051 -client localhost:12345 -config=configs/$scenario | grep error | wc -l) + errors=$(./test-runner/test-runner -fail-fast -client localhost:50051 -config=configs/$scenario | grep error | wc -l) if [ "$errors" = "0" ]; then echo "Success"; @@ -111,3 +133,4 @@ jobs: exit 1; fi done + \ No newline at end of file