From 1d32ea3df2eba6eaa21a0d24c3b5851b9eef36da Mon Sep 17 00:00:00 2001 From: Jan Winkelmann <146678+keks@users.noreply.github.com> Date: Wed, 17 Jan 2024 17:24:35 +0100 Subject: [PATCH 1/4] Process GroupContextExtensions Proposals (#1475) Adds support for GroupContextExtensions proposals. The relevant validation has been added both to the code and the documentation. Co-authored-by: Jan Winkelmann (keks) --- book/src/message_validation.md | 2 + openmls/src/framing/validation.rs | 1 + openmls/src/group/core_group/mod.rs | 17 ++-- openmls/src/group/core_group/proposals.rs | 11 +-- openmls/src/group/core_group/staged_commit.rs | 10 ++- .../src/group/core_group/test_proposals.rs | 53 ----------- openmls/src/group/errors.rs | 31 ++++++- openmls/src/group/group_context.rs | 4 + openmls/src/group/mls_group/errors.rs | 12 ++- openmls/src/group/mls_group/proposal.rs | 9 +- openmls/src/group/mls_group/test_mls_group.rs | 89 ++++++++++++++++++- openmls/src/group/public_group/diff.rs | 5 ++ .../public_group/diff/apply_proposals.rs | 13 +++ .../group/public_group/diff/compute_path.rs | 5 +- .../src/group/public_group/staged_commit.rs | 5 +- openmls/src/group/public_group/validation.rs | 40 ++++++++- openmls/src/treesync/node/leaf_node.rs | 29 +++--- .../treesync/node/leaf_node/capabilities.rs | 14 ++- .../kats/kat_tree_operations.rs | 5 +- 19 files changed, 252 insertions(+), 103 deletions(-) diff --git a/book/src/message_validation.md b/book/src/message_validation.md index efdaa92761..8c4e2cf37e 100644 --- a/book/src/message_validation.md +++ b/book/src/message_validation.md @@ -80,6 +80,8 @@ The following is a list of the individual semantic validation steps performed by | `ValSem205` | Confirmation tag must be successfully verified | ✅ | ✅ | `openmls/src/group/tests/test_commit_validation.rs` | | `ValSem206` | Path leaf node encryption key must be unique among proposals & members | ✅ | ✅ | `openmls/src/group/tests/test_commit_validation.rs` | | `ValSem207` | Path encryption keys must be unique among proposals & members | ✅ | ✅ | `openmls/src/group/tests/test_commit_validation.rs` | +| `ValSem208` | Only one GroupContextExtensions proposal in a commit | ✅ | | | +| `ValSem209` | GroupContextExtensions proposals may only contain extensions support by all members | ✅ | | | ### External Commit message validation diff --git a/openmls/src/framing/validation.rs b/openmls/src/framing/validation.rs index 7108b70b73..391cdc2af7 100644 --- a/openmls/src/framing/validation.rs +++ b/openmls/src/framing/validation.rs @@ -55,6 +55,7 @@ use super::{ /// - ValSem005 /// - ValSem007 /// - ValSem009 +#[derive(Debug)] pub(crate) struct DecryptedMessage { verifiable_content: VerifiableAuthenticatedContentIn, } diff --git a/openmls/src/group/core_group/mod.rs b/openmls/src/group/core_group/mod.rs index b56b53f8b6..827966452c 100644 --- a/openmls/src/group/core_group/mod.rs +++ b/openmls/src/group/core_group/mod.rs @@ -452,10 +452,9 @@ impl CoreGroup { let required_capabilities = required_extension.as_required_capabilities_extension()?; // Ensure we support all the capabilities. required_capabilities.check_support()?; - // TODO #566/#1361: This needs to be re-enabled once we support GCEs - /* self.own_leaf_node()? - .capabilities() - .supports_required_capabilities(required_capabilities)?; */ + self.own_leaf_node()? + .capabilities() + .supports_required_capabilities(required_capabilities)?; // Ensure that all other leaf nodes support all the required // extensions as well. @@ -890,6 +889,11 @@ impl CoreGroup { .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(); @@ -915,12 +919,13 @@ impl CoreGroup { apply_proposals_values.exclusion_list(), params.commit_type(), signer, - params.take_credential_with_key() + params.take_credential_with_key(), + 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())?; + diff.update_group_context(provider.crypto(), apply_proposals_values.extensions.clone())?; PathComputationResult::default() }; diff --git a/openmls/src/group/core_group/proposals.rs b/openmls/src/group/core_group/proposals.rs index e5344133b6..57920e43f8 100644 --- a/openmls/src/group/core_group/proposals.rs +++ b/openmls/src/group/core_group/proposals.rs @@ -507,7 +507,7 @@ impl ProposalQueue { } } Proposal::GroupContextExtensions(_) => { - // TODO: Validate proposal? + valid_proposals.add(queued_proposal.proposal_reference()); proposal_pool.insert(queued_proposal.proposal_reference(), queued_proposal); } Proposal::AppAck(_) => unimplemented!("See #291"), @@ -530,10 +530,11 @@ impl ProposalQueue { // Only retain `adds` and `valid_proposals` let mut proposal_queue = ProposalQueue::default(); for proposal_reference in adds.iter().chain(valid_proposals.iter()) { - proposal_queue.add(match proposal_pool.get(proposal_reference) { - Some(queued_proposal) => queued_proposal.clone(), - None => return Err(ProposalQueueError::ProposalNotFound), - }); + let queued_proposal = proposal_pool + .get(proposal_reference) + .cloned() + .ok_or(ProposalQueueError::ProposalNotFound)?; + proposal_queue.add(queued_proposal); } Ok((proposal_queue, contains_own_updates)) } diff --git a/openmls/src/group/core_group/staged_commit.rs b/openmls/src/group/core_group/staged_commit.rs index 8fccbf42b2..0b69aa3f37 100644 --- a/openmls/src/group/core_group/staged_commit.rs +++ b/openmls/src/group/core_group/staged_commit.rs @@ -165,7 +165,10 @@ impl CoreGroup { )?; // Update group context - diff.update_group_context(provider.crypto())?; + diff.update_group_context( + provider.crypto(), + apply_proposals_values.extensions.clone(), + )?; let decryption_keypairs: Vec<&EncryptionKeyPair> = old_epoch_keypairs .iter() @@ -221,7 +224,10 @@ impl CoreGroup { } // Even if there is no path, we have to update the group context. - diff.update_group_context(provider.crypto())?; + diff.update_group_context( + provider.crypto(), + apply_proposals_values.extensions.clone(), + )?; ( CommitSecret::zero_secret(ciphersuite, self.version()), diff --git a/openmls/src/group/core_group/test_proposals.rs b/openmls/src/group/core_group/test_proposals.rs index 397237bda1..4f9812c9a1 100644 --- a/openmls/src/group/core_group/test_proposals.rs +++ b/openmls/src/group/core_group/test_proposals.rs @@ -22,7 +22,6 @@ use crate::{ messages::proposals::{AddProposal, Proposal, ProposalOrRef, ProposalType}, schedule::psk::store::ResumptionPskStore, test_utils::*, - treesync::errors::LeafNodeValidationError, versions::ProtocolVersion, }; @@ -310,58 +309,6 @@ fn test_required_unsupported_proposals(ciphersuite: Ciphersuite, provider: &impl ) } -#[apply(ciphersuites_and_providers)] -fn test_required_extension_key_package_mismatch( - ciphersuite: Ciphersuite, - provider: &impl OpenMlsProvider, -) { - // Basic group setup. - let group_aad = b"Alice's test group"; - let framing_parameters = FramingParameters::new(group_aad, WireFormat::PublicMessage); - - let (alice_credential, _, alice_signer, _alice_pk) = - setup_client("Alice", ciphersuite, provider); - let (_bob_credential_with_key, bob_key_package_bundle, _, _) = - setup_client("Bob", ciphersuite, provider); - let bob_key_package = bob_key_package_bundle.key_package(); - - // Set required capabilities - let extensions = &[ - ExtensionType::RequiredCapabilities, - ExtensionType::ApplicationId, - ]; - let proposals = &[ - ProposalType::GroupContextExtensions, - ProposalType::Add, - ProposalType::Remove, - ProposalType::Update, - ]; - let credentials = &[CredentialType::Basic]; - let required_capabilities = - RequiredCapabilitiesExtension::new(extensions, proposals, credentials); - - let alice_group = CoreGroup::builder( - GroupId::random(provider.rand()), - CryptoConfig::with_default_version(ciphersuite), - alice_credential, - ) - .with_required_capabilities(required_capabilities) - .build(provider, &alice_signer) - .expect("Error creating CoreGroup."); - - let e = alice_group - .create_add_proposal( - framing_parameters, - bob_key_package.clone(), - &alice_signer, - ) - .expect_err("Proposal was created even though the key package didn't support the required extensions."); - assert_eq!( - e, - CreateAddProposalError::LeafNodeValidation(LeafNodeValidationError::UnsupportedExtensions) - ); -} - #[apply(ciphersuites_and_providers)] fn test_group_context_extensions(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { // Basic group setup. diff --git a/openmls/src/group/errors.rs b/openmls/src/group/errors.rs index 1432c494a0..fbc1296710 100644 --- a/openmls/src/group/errors.rs +++ b/openmls/src/group/errors.rs @@ -189,6 +189,11 @@ pub enum StageCommitError { /// See [`UpdatePathError`] for more details. #[error(transparent)] VerifiedUpdatePathError(#[from] UpdatePathError), + /// See [`GroupContextExtensionsProposalValidationError`] for more details. + #[error(transparent)] + GroupContextExtensionsProposalValidationError( + #[from] GroupContextExtensionsProposalValidationError, + ), } /// Create commit error @@ -233,6 +238,11 @@ pub enum CreateCommitError { /// See [`InvalidExtensionError`] for more details. #[error(transparent)] InvalidExtensionError(#[from] InvalidExtensionError), + /// See [`GroupContextExtensionsProposalValidationError`] for more details. + #[error(transparent)] + GroupContextExtensionsProposalValidationError( + #[from] GroupContextExtensionsProposalValidationError, + ), } /// Validation error @@ -500,16 +510,13 @@ pub(crate) enum CoreGroupParseMessageError { /// Create group context ext proposal error #[derive(Error, Debug, PartialEq, Clone)] -pub(crate) enum CreateGroupContextExtProposalError { +pub enum CreateGroupContextExtProposalError { /// See [`LibraryError`] for more details. #[error(transparent)] LibraryError(#[from] LibraryError), /// See [`KeyPackageExtensionSupportError`] for more details. #[error(transparent)] KeyPackageExtensionSupport(#[from] KeyPackageExtensionSupportError), - /// See [`TreeSyncError`] for more details. - #[error(transparent)] - TreeSyncError(#[from] TreeSyncError), /// See [`ExtensionError`] for more details. #[error(transparent)] Extension(#[from] ExtensionError), @@ -528,3 +535,19 @@ pub enum MergeCommitError { #[error("Error accessing the key store.")] KeyStoreError(KeyStoreError), } + +/// Error validation a GroupContextExtensions proposal. +#[derive(Error, Debug, PartialEq, Clone)] +pub enum GroupContextExtensionsProposalValidationError { + /// Commit has more than one GroupContextExtensions proposal. + #[error("Commit has more than one GroupContextExtensions proposal.")] + TooManyGCEProposals, + /// See [`LibraryError`] for more details. + #[error(transparent)] + LibraryError(#[from] LibraryError), + /// The new extensions set contails extensions that are not supported by all group members. + #[error( + "The new extensions set contails extensions that are not supported by all group members." + )] + ExtensionNotSupportedByAllMembers, +} diff --git a/openmls/src/group/group_context.rs b/openmls/src/group/group_context.rs index 8344e07625..d6b1cc93a9 100644 --- a/openmls/src/group/group_context.rs +++ b/openmls/src/group/group_context.rs @@ -159,6 +159,10 @@ impl GroupContext { self.confirmed_transcript_hash.as_slice() } + pub(crate) fn set_extensions(&mut self, extensions: Extensions) { + self.extensions = extensions; + } + /// Return the extensions. pub fn extensions(&self) -> &Extensions { &self.extensions diff --git a/openmls/src/group/mls_group/errors.rs b/openmls/src/group/mls_group/errors.rs index a23e47667e..2b7a4fc207 100644 --- a/openmls/src/group/mls_group/errors.rs +++ b/openmls/src/group/mls_group/errors.rs @@ -10,9 +10,12 @@ use thiserror::Error; use crate::{ error::LibraryError, extensions::errors::InvalidExtensionError, - group::errors::{ - CreateAddProposalError, CreateCommitError, MergeCommitError, StageCommitError, - ValidationError, + group::{ + errors::{ + CreateAddProposalError, CreateCommitError, MergeCommitError, StageCommitError, + ValidationError, + }, + CreateGroupContextExtProposalError, }, schedule::errors::PskError, treesync::errors::{LeafNodeValidationError, PublicTreeError}, @@ -317,4 +320,7 @@ pub enum ProposalError { /// See [`ValidationError`] for more details. #[error(transparent)] ValidationError(#[from] ValidationError), + /// See [`CreateGroupContextExtProposalError`] for more details. + #[error(transparent)] + CreateGroupContextExtProposalError(#[from] CreateGroupContextExtProposalError), } diff --git a/openmls/src/group/mls_group/proposal.rs b/openmls/src/group/mls_group/proposal.rs index b15b21d75d..bc00077640 100644 --- a/openmls/src/group/mls_group/proposal.rs +++ b/openmls/src/group/mls_group/proposal.rs @@ -328,10 +328,11 @@ impl MlsGroup { ) -> Result<(MlsMessageOut, ProposalRef), ProposalError<()>> { self.is_operational()?; - let proposal = self - .group - .create_group_context_ext_proposal(self.framing_parameters(), extensions, signer) - .unwrap(); + let proposal = self.group.create_group_context_ext_proposal( + self.framing_parameters(), + extensions, + signer, + )?; let queued_proposal = QueuedProposal::from_authenticated_content_by_ref( self.ciphersuite(), diff --git a/openmls/src/group/mls_group/test_mls_group.rs b/openmls/src/group/mls_group/test_mls_group.rs index b22ec84b6a..2587a2066e 100644 --- a/openmls/src/group/mls_group/test_mls_group.rs +++ b/openmls/src/group/mls_group/test_mls_group.rs @@ -721,7 +721,7 @@ fn test_pending_commit_logic(ciphersuite: Ciphersuite, provider: &impl OpenMlsPr // Let's add bob let (proposal, _) = alice_group .propose_add_member(provider, &alice_signer, bob_key_package) - .expect("error creating self-update proposal"); + .expect("error creating add-bob proposal"); let alice_processed_message = alice_group .process_message(provider, proposal.into_protocol_message().unwrap()) @@ -1002,6 +1002,93 @@ fn remove_prosposal_by_ref(ciphersuite: Ciphersuite, provider: &impl OpenMlsProv _ => unreachable!("Expected a StagedCommit."), } } +// +// Test that the builder pattern accurately configures the new group. +#[apply(ciphersuites_and_providers)] +fn group_context_extensions_proposal(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { + let (alice_credential_with_key, _alice_kpb, alice_signer, _alice_pk) = + setup_client("Alice", ciphersuite, provider); + + // === Alice creates a group === + let mut alice_group = MlsGroup::builder() + .build(provider, &alice_signer, alice_credential_with_key) + .expect("error creating group using builder"); + + let required_capabilities = alice_group + .group() + .group_context_extensions() + .required_capabilities() + .expect("couldn't get required_capabilities"); + + // no required extensions + assert!(required_capabilities.extension_types().is_empty()); + + let new_extensions = Extensions::single(Extension::RequiredCapabilities( + RequiredCapabilitiesExtension::new(&[ExtensionType::RequiredCapabilities], &[], &[]), + )); + + let new_extensions_2 = Extensions::single(Extension::RequiredCapabilities( + RequiredCapabilitiesExtension::new(&[ExtensionType::RatchetTree], &[], &[]), + )); + + alice_group + .propose_group_context_extensions(provider, new_extensions.clone(), &alice_signer) + .expect("failed to build group context extensions proposal"); + + assert_eq!(alice_group.pending_proposals().count(), 1); + + alice_group + .commit_to_pending_proposals(provider, &alice_signer) + .expect("failed to commit to pending proposals"); + + alice_group + .merge_pending_commit(provider) + .expect("error merging pending commit"); + + let required_capabilities = alice_group + .group() + .group_context_extensions() + .required_capabilities() + .expect("couldn't get required_capabilities"); + + // has required_capabilities as required capability + assert!(required_capabilities.extension_types() == [ExtensionType::RequiredCapabilities]); + + // === committing to two group context extensions should fail + + alice_group + .propose_group_context_extensions(provider, new_extensions, &alice_signer) + .expect("failed to build group context extensions proposal"); + + // the proposals need to be different or they will be deduplicated + alice_group + .propose_group_context_extensions(provider, new_extensions_2, &alice_signer) + .expect("failed to build group context extensions proposal"); + + assert_eq!(alice_group.pending_proposals().count(), 2); + + alice_group + .commit_to_pending_proposals(provider, &alice_signer) + .expect_err( + "expected error when committing to multiple group context extensions proposals", + ); + + // === can't update required required_capabilities to extensions that existing group members + // are not capable of + + // contains unsupported extension + let new_extensions = Extensions::single(Extension::RequiredCapabilities( + RequiredCapabilitiesExtension::new(&[ExtensionType::Unknown(0xf042)], &[], &[]), + )); + + alice_group + .propose_group_context_extensions(provider, new_extensions, &alice_signer) + .expect_err("expected an error building GCE proposal with bad required_capabilities"); + + // TODO: we need to test that processing a commit with multiple group context extensions + // proposal also fails. however, we can't generate this commit, because our functions for + // constructing commits does not permit it. See #1476 +} // Test that the builder pattern accurately configures the new group. #[apply(ciphersuites_and_providers)] diff --git a/openmls/src/group/public_group/diff.rs b/openmls/src/group/public_group/diff.rs index d9c7e94e01..e2b3eafb75 100644 --- a/openmls/src/group/public_group/diff.rs +++ b/openmls/src/group/public_group/diff.rs @@ -13,6 +13,7 @@ use super::PublicGroup; use crate::{ binary_tree::{array_representation::TreeSize, LeafNodeIndex}, error::LibraryError, + extensions::Extensions, framing::{mls_auth_content::AuthenticatedContent, public_message::InterimTranscriptHashInput}, group::GroupContext, messages::{proposals::AddProposal, ConfirmationTag, EncryptedGroupSecrets}, @@ -201,6 +202,7 @@ impl<'a> PublicGroupDiff<'a> { pub(crate) fn update_group_context( &mut self, crypto: &impl OpenMlsCrypto, + extensions: Option, ) -> Result<(), LibraryError> { // Calculate tree hash let new_tree_hash = self @@ -208,6 +210,9 @@ impl<'a> PublicGroupDiff<'a> { .compute_tree_hashes(crypto, self.group_context().ciphersuite())?; self.group_context.update_tree_hash(new_tree_hash); self.group_context.increment_epoch(); + if let Some(extensions) = extensions { + self.group_context.set_extensions(extensions); + } Ok(()) } diff --git a/openmls/src/group/public_group/diff/apply_proposals.rs b/openmls/src/group/public_group/diff/apply_proposals.rs index cf25af9d94..470c41a183 100644 --- a/openmls/src/group/public_group/diff/apply_proposals.rs +++ b/openmls/src/group/public_group/diff/apply_proposals.rs @@ -12,12 +12,14 @@ use crate::{ use super::*; /// This struct contain the return values of the `apply_proposals()` function +#[derive(Debug)] pub(crate) struct ApplyProposalsValues { pub(crate) path_required: bool, pub(crate) self_removed: bool, pub(crate) invitation_list: Vec<(LeafNodeIndex, AddProposal)>, pub(crate) presharedkeys: Vec, pub(crate) external_init_proposal_option: Option, + pub(crate) extensions: Option, } impl ApplyProposalsValues { @@ -140,6 +142,16 @@ impl<'a> PublicGroupDiff<'a> { }) .collect(); + // apply group context extension proposal + let extensions = proposal_queue + .filtered_by_type(ProposalType::GroupContextExtensions) + .find_map(|queued_proposal| match queued_proposal.proposal() { + Proposal::GroupContextExtensions(extensions) => { + Some(extensions.extensions().clone()) + } + _ => None, + }); + let proposals_require_path = proposal_queue .queued_proposals() .any(|p| p.proposal().is_path_required()); @@ -159,6 +171,7 @@ impl<'a> PublicGroupDiff<'a> { invitation_list, presharedkeys, external_init_proposal_option, + extensions, }) } } diff --git a/openmls/src/group/public_group/diff/compute_path.rs b/openmls/src/group/public_group/diff/compute_path.rs index 5561dabfed..b41d8784b9 100644 --- a/openmls/src/group/public_group/diff/compute_path.rs +++ b/openmls/src/group/public_group/diff/compute_path.rs @@ -7,6 +7,7 @@ use crate::{ binary_tree::LeafNodeIndex, credentials::CredentialWithKey, error::LibraryError, + extensions::Extensions, group::{ config::CryptoConfig, core_group::create_commit_params::CommitType, errors::CreateCommitError, @@ -35,6 +36,7 @@ pub(crate) struct PathComputationResult { } impl<'a> PublicGroupDiff<'a> { + #[allow(clippy::too_many_arguments)] pub(crate) fn compute_path( &mut self, provider: &impl OpenMlsProvider, @@ -43,6 +45,7 @@ impl<'a> PublicGroupDiff<'a> { commit_type: CommitType, signer: &impl Signer, credential_with_key: Option, + extensions: Option, ) -> Result> { let version = self.group_context().protocol_version(); let ciphersuite = self.group_context().ciphersuite(); @@ -101,7 +104,7 @@ impl<'a> PublicGroupDiff<'a> { // After we've processed the path, we can update the group context s.t. // the updated group context is used for path secret encryption. Note // that we have not yet updated the confirmed transcript hash. - self.update_group_context(provider.crypto())?; + self.update_group_context(provider.crypto(), extensions)?; let serialized_group_context = self .group_context() diff --git a/openmls/src/group/public_group/staged_commit.rs b/openmls/src/group/public_group/staged_commit.rs index c7cfa5a25b..2e583017e0 100644 --- a/openmls/src/group/public_group/staged_commit.rs +++ b/openmls/src/group/public_group/staged_commit.rs @@ -87,6 +87,9 @@ impl PublicGroup { // ValSem107 // ValSem108 self.validate_remove_proposals(&proposal_queue)?; + // ValSem208 + // ValSem209 + self.validate_group_context_extensions_proposal(&proposal_queue)?; // ValSem401 // ValSem402 // ValSem403 @@ -226,7 +229,7 @@ impl PublicGroup { }; // Update group context - diff.update_group_context(crypto)?; + diff.update_group_context(crypto, apply_proposals_values.extensions.clone())?; // Update the confirmed transcript hash before we compute the confirmation tag. diff.update_confirmed_transcript_hash(crypto, mls_content)?; diff --git a/openmls/src/group/public_group/validation.rs b/openmls/src/group/public_group/validation.rs index 6db6328592..81ab909084 100644 --- a/openmls/src/group/public_group/validation.rs +++ b/openmls/src/group/public_group/validation.rs @@ -6,7 +6,8 @@ use std::collections::{BTreeSet, HashSet}; use openmls_traits::types::VerifiableCiphersuite; use super::PublicGroup; -#[cfg(test)] +use crate::group::GroupContextExtensionsProposalValidationError; +use crate::prelude::LibraryError; use crate::treesync::errors::LeafNodeValidationError; use crate::{ binary_tree::array_representation::LeafNodeIndex, @@ -515,7 +516,42 @@ impl PublicGroup { /// Returns a [`LeafNodeValidationError`] if an [`ExtensionType`] /// in `extensions` is not supported by a leaf in this tree. - #[cfg(test)] + pub(crate) fn validate_group_context_extensions_proposal( + &self, + proposal_queue: &ProposalQueue, + ) -> Result<(), GroupContextExtensionsProposalValidationError> { + let mut iter = proposal_queue.filtered_by_type(ProposalType::GroupContextExtensions); + + match iter.next() { + Some(queued_proposal) => match queued_proposal.proposal() { + Proposal::GroupContextExtensions(extensions) => { + let ext_type_list: Vec<_> = extensions + .extensions() + .iter() + .map(|ext| ext.extension_type()) + .collect(); + + self.check_extension_support(&ext_type_list).map_err(|_| GroupContextExtensionsProposalValidationError::ExtensionNotSupportedByAllMembers)? + } + _ => { + return Err(GroupContextExtensionsProposalValidationError::LibraryError( + LibraryError::custom( + "found non-gce proposal when filtered for gce proposals", + ), + )) + } + }, + None => return Ok(()), + } + + match iter.next() { + Some(_) => Err(GroupContextExtensionsProposalValidationError::TooManyGCEProposals), + None => Ok(()), + } + } + + /// Returns a [`LeafNodeValidationError`] if an [`ExtensionType`] + /// in `extensions` is not supported by a leaf in this tree. pub(crate) fn check_extension_support( &self, extensions: &[crate::extensions::ExtensionType], diff --git a/openmls/src/treesync/node/leaf_node.rs b/openmls/src/treesync/node/leaf_node.rs index b50aa7bf67..fd1f47cf4f 100644 --- a/openmls/src/treesync/node/leaf_node.rs +++ b/openmls/src/treesync/node/leaf_node.rs @@ -27,7 +27,6 @@ use crate::{ versions::ProtocolVersion, }; -#[cfg(test)] use crate::treesync::errors::LeafNodeValidationError; mod capabilities; @@ -382,6 +381,20 @@ impl LeafNode { .contains(extension_type) || default_extensions().iter().any(|et| et == extension_type) } + /// + /// Check whether the this leaf node supports all the required extensions + /// in the provided list. + pub(crate) fn check_extension_support( + &self, + extensions: &[ExtensionType], + ) -> Result<(), LeafNodeValidationError> { + for required in extensions.iter() { + if !self.supports_extension(required) { + return Err(LeafNodeValidationError::UnsupportedExtensions); + } + } + Ok(()) + } } #[cfg(test)] @@ -411,20 +424,6 @@ impl LeafNode { pub fn capabilities_mut(&mut self) -> &mut Capabilities { &mut self.payload.capabilities } - - /// Check whether the this leaf node supports all the required extensions - /// in the provided list. - pub(crate) fn check_extension_support( - &self, - extensions: &[ExtensionType], - ) -> Result<(), LeafNodeValidationError> { - for required in extensions.iter() { - if !self.supports_extension(required) { - return Err(LeafNodeValidationError::UnsupportedExtensions); - } - } - Ok(()) - } } #[cfg(any(feature = "test-utils", test))] diff --git a/openmls/src/treesync/node/leaf_node/capabilities.rs b/openmls/src/treesync/node/leaf_node/capabilities.rs index e07ff477c8..f7eb9c5ac0 100644 --- a/openmls/src/treesync/node/leaf_node/capabilities.rs +++ b/openmls/src/treesync/node/leaf_node/capabilities.rs @@ -137,7 +137,7 @@ impl Capabilities { if required_capabilities .extension_types() .iter() - .any(|e| !self.extensions().contains(e)) + .any(|e| !(self.extensions().contains(e) || default_extensions().contains(e))) { return Err(LeafNodeValidationError::UnsupportedExtensions); } @@ -145,7 +145,7 @@ impl Capabilities { if required_capabilities .proposal_types() .iter() - .any(|p| !self.proposals().contains(p)) + .any(|p| !(self.proposals().contains(p) || default_proposals().contains(p))) { return Err(LeafNodeValidationError::UnsupportedProposals); } @@ -153,7 +153,7 @@ impl Capabilities { if required_capabilities .credential_types() .iter() - .any(|c| !self.credentials().contains(c)) + .any(|c| !(self.credentials().contains(c) || default_credentials().contains(c))) { return Err(LeafNodeValidationError::UnsupportedCredentials); } @@ -216,7 +216,13 @@ pub(super) fn default_ciphersuites() -> Vec { /// All extensions defined in the MLS spec are considered "default" by the spec. pub(super) fn default_extensions() -> Vec { - vec![ExtensionType::ApplicationId] + vec![ + ExtensionType::ApplicationId, + ExtensionType::RatchetTree, + ExtensionType::RequiredCapabilities, + ExtensionType::ExternalPub, + ExtensionType::ExternalSenders, + ] } /// All proposals defined in the MLS spec are considered "default" by the spec. 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 1857191f8b..7726940212 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 @@ -111,8 +111,9 @@ fn run_test_vector(test: TestElement, provider: &impl OpenMlsProvider) -> Result let mut diff = group.empty_diff(); - diff.apply_proposals(&proposal_queue, None).unwrap(); - diff.update_group_context(provider.crypto()).unwrap(); + let apply_proposal_values = diff.apply_proposals(&proposal_queue, None).unwrap(); + diff.update_group_context(provider.crypto(), apply_proposal_values.extensions.clone()) + .unwrap(); let staged_diff = diff .into_staged_diff(provider.crypto(), ciphersuite) From 4d5716e0eda31d8e84c8a6c3135f2ec3e49c36ba Mon Sep 17 00:00:00 2001 From: Konrad Kohbrok Date: Thu, 18 Jan 2024 13:07:28 +0100 Subject: [PATCH 2/4] Add leaf node configuration options to MlsGroup builder (#1477) --- book/src/user_manual/group_config.md | 2 + openmls/src/extensions/errors.rs | 5 ++ openmls/src/group/core_group/mod.rs | 17 +++++ openmls/src/group/mls_group/builder.rs | 23 +++++- openmls/src/group/mls_group/config.rs | 29 +++++++- openmls/src/group/mls_group/test_mls_group.rs | 32 +++++++++ openmls/src/group/public_group/builder.rs | 71 +++++++++++++------ openmls/tests/book_code.rs | 15 +++- 8 files changed, 168 insertions(+), 26 deletions(-) diff --git a/book/src/user_manual/group_config.md b/book/src/user_manual/group_config.md index cd826f9adc..2f498569d3 100644 --- a/book/src/user_manual/group_config.md +++ b/book/src/user_manual/group_config.md @@ -19,6 +19,8 @@ Two very similar structs can help configure groups upon their creation: `MlsGrou | ------------------------------ | ------------------------------- | ------------------------------------------------------------------------------------------------ | | `required_capabilities` | `RequiredCapabilitiesExtension` | Required capabilities (extensions and proposal types). | | `external_senders` | `ExternalSendersExtensions` | List credentials of non-group members that are allowed to send proposals to the group. | +| `capabilities` . | `Capabilities` | Lists the capabilities of the group's creator. | +| `leaf_extensions` . | `Extensions` | Extensions to be included in the group creator's leaf | Both ways of group configurations can be specified by using the struct's builder pattern, or choosing their default values. The default value contains safe values for all parameters and is suitable for scenarios without particular requirements. diff --git a/openmls/src/extensions/errors.rs b/openmls/src/extensions/errors.rs index bdc7f9d58a..e8eb93a310 100644 --- a/openmls/src/extensions/errors.rs +++ b/openmls/src/extensions/errors.rs @@ -97,4 +97,9 @@ pub enum InvalidExtensionError { "The provided extension list contains an extension that is not allowed in group contexts." )] IllegalInGroupContext, + /// The provided extension list contains an extension that is not allowed in leaf nodes + #[error( + "The provided extension list contains an extension that is not allowed in leaf nodes." + )] + IllegalInLeafNodes, } diff --git a/openmls/src/group/core_group/mod.rs b/openmls/src/group/core_group/mod.rs index 827966452c..5dcfd3226d 100644 --- a/openmls/src/group/core_group/mod.rs +++ b/openmls/src/group/core_group/mod.rs @@ -39,6 +39,7 @@ 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}, }; @@ -189,6 +190,11 @@ impl CoreGroupBuilder { .with_required_capabilities(required_capabilities); 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 + } /// Set the [`ExternalSendersExtension`] of the [`CoreGroup`]. pub(crate) fn with_external_senders( mut self, @@ -217,6 +223,17 @@ impl CoreGroupBuilder { 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; diff --git a/openmls/src/group/mls_group/builder.rs b/openmls/src/group/mls_group/builder.rs index ed8ec42069..bcd27d8f66 100644 --- a/openmls/src/group/mls_group/builder.rs +++ b/openmls/src/group/mls_group/builder.rs @@ -8,7 +8,7 @@ use crate::{ CoreGroupBuildError, CoreGroupConfig, GroupId, MlsGroupCreateConfig, MlsGroupCreateConfigBuilder, NewGroupError, ProposalStore, WireFormatPolicy, }, - prelude::{LibraryError, Lifetime, SenderRatchetConfiguration}, + prelude::{Capabilities, LibraryError, Lifetime, SenderRatchetConfiguration}, }; use super::{InnerState, MlsGroup, MlsGroupState}; @@ -73,6 +73,8 @@ impl MlsGroupBuilder { .with_required_capabilities(mls_group_create_config.required_capabilities.clone()) .with_external_senders(mls_group_create_config.external_senders.clone()) .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) @@ -200,6 +202,14 @@ impl MlsGroupBuilder { self } + /// Sets the group creator's [`Capabilities`] + pub fn with_capabilities(mut self, capabilities: Capabilities) -> Self { + self.mls_group_create_config_builder = self + .mls_group_create_config_builder + .capabilities(capabilities); + self + } + /// Sets the initial group context extensions pub fn with_group_context_extensions( mut self, @@ -210,4 +220,15 @@ impl MlsGroupBuilder { .with_group_context_extensions(extensions)?; Ok(self) } + + /// Sets the initial leaf node extensions + pub fn with_leaf_node_extensions( + mut self, + extensions: Extensions, + ) -> Result { + self.mls_group_create_config_builder = self + .mls_group_create_config_builder + .with_leaf_node_extensions(extensions)?; + Ok(self) + } } diff --git a/openmls/src/group/mls_group/config.rs b/openmls/src/group/mls_group/config.rs index c270ece7d2..673548ad5d 100644 --- a/openmls/src/group/mls_group/config.rs +++ b/openmls/src/group/mls_group/config.rs @@ -30,7 +30,7 @@ use super::*; use crate::{ extensions::errors::InvalidExtensionError, group::config::CryptoConfig, key_packages::Lifetime, - tree::sender_ratchet::SenderRatchetConfiguration, + tree::sender_ratchet::SenderRatchetConfiguration, treesync::node::leaf_node::Capabilities, }; use serde::{Deserialize, Serialize}; @@ -85,6 +85,8 @@ impl MlsGroupJoinConfig { pub struct MlsGroupCreateConfig { /// Required capabilities (extensions and proposal types) pub(crate) required_capabilities: RequiredCapabilitiesExtension, + /// Capabilities advertised in the creator's leaf node + pub(crate) capabilities: Capabilities, /// Senders authorized to send external remove proposals pub(crate) external_senders: ExternalSendersExtension, /// Lifetime of the own leaf node @@ -95,6 +97,8 @@ pub struct MlsGroupCreateConfig { pub(crate) join_config: MlsGroupJoinConfig, /// List of initial group context extensions pub(crate) group_context_extensions: Extensions, + /// List of initial leaf node extensions + pub(crate) leaf_node_extensions: Extensions, } /// Builder struct for an [`MlsGroupJoinConfig`]. @@ -298,6 +302,12 @@ impl MlsGroupCreateConfigBuilder { self } + /// Sets the `capabilities` of the group creator's leaf node. + pub fn capabilities(mut self, capabilities: Capabilities) -> Self { + self.config.capabilities = capabilities; + self + } + /// Sets the `sender_ratchet_configuration` property of the MlsGroupCreateConfig. /// See [`SenderRatchetConfiguration`] for more information. pub fn sender_ratchet_configuration( @@ -345,6 +355,23 @@ impl MlsGroupCreateConfigBuilder { Ok(self) } + /// Sets extensions of the group creator's [`LeafNode`]. + pub fn with_leaf_node_extensions( + mut self, + extensions: Extensions, + ) -> Result { + // None of the default extensions are leaf node extensions, so only + // unknown extensions can be leaf node extensions. + let is_valid_in_leaf_node = extensions + .iter() + .all(|e| matches!(e.extension_type(), ExtensionType::Unknown(_))); + if !is_valid_in_leaf_node { + return Err(InvalidExtensionError::IllegalInLeafNodes); + } + self.config.leaf_node_extensions = extensions; + Ok(self) + } + /// Finalizes the builder and retursn an `[MlsGroupCreateConfig`]. pub fn build(self) -> MlsGroupCreateConfig { self.config diff --git a/openmls/src/group/mls_group/test_mls_group.rs b/openmls/src/group/mls_group/test_mls_group.rs index 2587a2066e..d97c9d39d9 100644 --- a/openmls/src/group/mls_group/test_mls_group.rs +++ b/openmls/src/group/mls_group/test_mls_group.rs @@ -5,10 +5,12 @@ use tls_codec::{Deserialize, Serialize}; use crate::{ binary_tree::LeafNodeIndex, + extensions::errors::InvalidExtensionError, framing::*, group::{config::CryptoConfig, errors::*, *}, key_packages::*, messages::proposals::*, + prelude::Capabilities, test_utils::test_framework::{ errors::ClientError, noop_authentication_service, ActionType::Commit, CodecUse, MlsGroupTestSetup, @@ -1109,6 +1111,17 @@ fn builder_pattern(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { let test_sender_ratchet_config = SenderRatchetConfiguration::new(10, 2000); let test_max_past_epochs = 10; let test_number_of_resumption_psks = 5; + let test_capabilities = Capabilities::new( + None, + None, + Some(&[ExtensionType::Unknown(0xff00)]), + None, + None, + ); + let test_leaf_extensions = Extensions::single(Extension::Unknown( + 0xff00, + UnknownExtension(vec![0x00, 0x01, 0x02]), + )); // === Alice creates a group === let alice_group = MlsGroup::builder() @@ -1122,6 +1135,9 @@ fn builder_pattern(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { .use_ratchet_tree_extension(true) .max_past_epochs(test_max_past_epochs) .number_of_resumption_psks(test_number_of_resumption_psks) + .with_leaf_node_extensions(test_leaf_extensions.clone()) + .unwrap() + .with_capabilities(test_capabilities.clone()) .build(provider, &alice_signer, alice_credential_with_key) .expect("error creating group using builder"); @@ -1161,4 +1177,20 @@ fn builder_pattern(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { .life_time() .expect("leaf doesn't have a lifetime"); assert_eq!(lifetime, &test_lifetime); + let own_leaf = alice_group.own_leaf_node().expect("can't find own leaf"); + let capabilities = own_leaf.capabilities(); + assert_eq!(capabilities, &test_capabilities); + let leaf_extensions = own_leaf.extensions(); + assert_eq!(leaf_extensions, &test_leaf_extensions); + + // Make sure that building with an invalid leaf node extension fails + let invalid_leaf_extensions = + Extensions::single(Extension::ApplicationId(ApplicationIdExtension::new(&[ + 0x00, 0x01, 0x02, + ]))); + + let builder_err = MlsGroup::builder() + .with_leaf_node_extensions(invalid_leaf_extensions) + .expect_err("successfully built group with invalid leaf extensions"); + assert_eq!(builder_err, InvalidExtensionError::IllegalInLeafNodes); } diff --git a/openmls/src/group/public_group/builder.rs b/openmls/src/group/public_group/builder.rs index 3da16bdf98..54d1edd453 100644 --- a/openmls/src/group/public_group/builder.rs +++ b/openmls/src/group/public_group/builder.rs @@ -8,7 +8,7 @@ use crate::{ errors::{ExtensionError, InvalidExtensionError}, Extension, Extensions, ExternalSendersExtension, RequiredCapabilitiesExtension, }, - group::{config::CryptoConfig, GroupContext, GroupId}, + group::{config::CryptoConfig, ExtensionType, GroupContext, GroupId}, key_packages::Lifetime, messages::ConfirmationTag, schedule::CommitSecret, @@ -25,9 +25,10 @@ pub(crate) struct TempBuilderPG1 { credential_with_key: CredentialWithKey, lifetime: Option, required_capabilities: Option, + capabilities: Option, external_senders: Option, - leaf_extensions: Option, - group_context_extensions: Option, + leaf_node_extensions: Extensions, + group_context_extensions: Extensions, } impl TempBuilderPG1 { @@ -44,6 +45,11 @@ impl TempBuilderPG1 { self } + pub(crate) fn with_capabilities(mut self, capabilities: Capabilities) -> Self { + self.capabilities = Some(capabilities); + self + } + pub(crate) fn with_external_senders( mut self, external_senders: ExternalSendersExtension, @@ -64,7 +70,23 @@ impl TempBuilderPG1 { if !is_valid_in_group_context { return Err(InvalidExtensionError::IllegalInGroupContext); } - self.group_context_extensions = Some(extensions); + self.group_context_extensions = extensions; + Ok(self) + } + + pub(crate) fn with_leaf_node_extensions( + mut self, + extensions: Extensions, + ) -> Result { + // None of the default extensions are leaf node extensions, so only + // unknown extensions can be leaf node extensions. + let is_valid_in_leaf_node = extensions + .iter() + .all(|e| matches!(e.extension_type(), ExtensionType::Unknown(_))); + if !is_valid_in_leaf_node { + return Err(InvalidExtensionError::IllegalInLeafNodes); + } + self.leaf_node_extensions = extensions; Ok(self) } @@ -73,24 +95,30 @@ impl TempBuilderPG1 { provider: &impl OpenMlsProvider, signer: &impl Signer, ) -> Result<(TempBuilderPG2, CommitSecret, EncryptionKeyPair), PublicGroupBuildError> { - let capabilities = self - .required_capabilities - .as_ref() - .map(|re| re.extension_types()); + // Capabilities are either provided by the builder or a minimal set of + // capabilities is created from the crypto config and the required + // capabilities + let capabilities = self.capabilities.unwrap_or(Capabilities::new( + Some(&[self.crypto_config.version]), + Some(&[self.crypto_config.ciphersuite]), + self.required_capabilities + .as_ref() + .map(|re| re.extension_types()), + self.required_capabilities + .as_ref() + .map(|re| re.proposal_types()), + self.required_capabilities + .as_ref() + .map(|re| re.credential_types()), + )); let (treesync, commit_secret, leaf_keypair) = TreeSync::new( provider, signer, self.crypto_config, self.credential_with_key, self.lifetime.unwrap_or_default(), - Capabilities::new( - Some(&[self.crypto_config.version]), // TODO: Allow more versions - Some(&[self.crypto_config.ciphersuite]), // TODO: allow more ciphersuites - capabilities, - None, - None, - ), - self.leaf_extensions.unwrap_or(Extensions::empty()), + capabilities, + self.leaf_node_extensions, )?; let required_capabilities = self.required_capabilities.unwrap_or_default(); required_capabilities.check_support().map_err(|e| match e { @@ -104,11 +132,7 @@ impl TempBuilderPG1 { })?; let required_capabilities = Extension::RequiredCapabilities(required_capabilities); - let mut group_context_extensions = if let Some(exts) = self.group_context_extensions { - exts - } else { - Extensions::empty() - }; + let mut group_context_extensions = self.group_context_extensions; group_context_extensions.add_or_replace(required_capabilities); if let Some(ext_senders) = self.external_senders { group_context_extensions.add_or_replace(Extension::ExternalSenders(ext_senders)); @@ -191,9 +215,10 @@ impl PublicGroup { credential_with_key, lifetime: None, required_capabilities: None, + capabilities: None, external_senders: None, - leaf_extensions: None, - group_context_extensions: None, + leaf_node_extensions: Extensions::empty(), + group_context_extensions: Extensions::empty(), } } } diff --git a/openmls/tests/book_code.rs b/openmls/tests/book_code.rs index b6b8c1399b..c3ef6832a0 100644 --- a/openmls/tests/book_code.rs +++ b/openmls/tests/book_code.rs @@ -133,11 +133,24 @@ fn book_operations(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { 10, // out_of_order_tolerance 2000, // maximum_forward_distance )) + .crypto_config(CryptoConfig::with_default_version(ciphersuite)) + .capabilities(Capabilities::new( + None, // Defaults to the group's protocol version + None, // Defaults to the group's ciphersuite + None, // Defaults to all basic extension types + None, // Defaults to all basic proposal types + Some(&[CredentialType::Basic]), + )) + // Example leaf extension + .with_leaf_node_extensions(Extensions::single(Extension::Unknown( + 0xff00, + UnknownExtension(vec![0, 1, 2, 3]), + ))) + .expect("failed to configure leaf extensions") .external_senders(vec![ExternalSender::new( ds_credential_with_key.signature_key.clone(), ds_credential_with_key.credential.clone(), )]) - .crypto_config(CryptoConfig::with_default_version(ciphersuite)) .use_ratchet_tree_extension(true) .build(); // ANCHOR_END: mls_group_create_config_example From 0d67da863b88f19226558229a6aeba021338914e Mon Sep 17 00:00:00 2001 From: Konrad Kohbrok Date: Thu, 18 Jan 2024 15:50:15 +0100 Subject: [PATCH 3/4] Clean up `MlsGroup*` builders (#1478) --- CHANGELOG.md | 4 + book/src/user_manual/group_config.md | 3 +- openmls/src/group/core_group/mod.rs | 23 +---- .../src/group/core_group/test_proposals.rs | 15 +++- openmls/src/group/mls_group/builder.rs | 33 +++----- openmls/src/group/mls_group/config.rs | 35 +------- openmls/src/group/mls_group/test_mls_group.rs | 13 +-- openmls/src/group/public_group/builder.rs | 83 +++++++------------ .../group/tests/external_remove_proposal.rs | 7 +- openmls/tests/book_code.rs | 15 ++-- 10 files changed, 81 insertions(+), 150 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 16712e2efc..a5f0809f13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - [#1464](https://github.com/openmls/openmls/pull/1464): Add builder pattern for `MlsGroup`; split `MlsGroupJoinConfig` into `MlsGroupCreateConfig` and `MlsGroupJoinConfig` +- [#1473](https://github.com/openmls/openmls/pull/1473): Allow setting group context extensions when building an MlsGroup(Config). +- [#1475](https://github.com/openmls/openmls/pull/1475): Fully process GroupContextExtension proposals +- [#1477](https://github.com/openmls/openmls/pull/1477): Allow setting leaf node extensions and capabilities of the group creator when creating an MlsGroup(Config) +- [#1478](https://github.com/openmls/openmls/pull/1478): Remove explicit functions to set `RequiredCapabilitiesExtension` and `ExternalSendersExtension` when building an MlsGroup(Config) in favor of the more general function to set group context extensions ## 0.5.0 (XXXX-XX-XX) diff --git a/book/src/user_manual/group_config.md b/book/src/user_manual/group_config.md index 2f498569d3..55e6f5b742 100644 --- a/book/src/user_manual/group_config.md +++ b/book/src/user_manual/group_config.md @@ -17,8 +17,7 @@ Two very similar structs can help configure groups upon their creation: `MlsGrou | Name | Type | Explanation | | ------------------------------ | ------------------------------- | ------------------------------------------------------------------------------------------------ | -| `required_capabilities` | `RequiredCapabilitiesExtension` | Required capabilities (extensions and proposal types). | -| `external_senders` | `ExternalSendersExtensions` | List credentials of non-group members that are allowed to send proposals to the group. | +| `group_context_extensions` | `Extensions` | Optional group-level extensions, e.g. `RequiredCapabilitiesExtension`. | | `capabilities` . | `Capabilities` | Lists the capabilities of the group's creator. | | `leaf_extensions` . | `Extensions` | Extensions to be included in the group creator's leaf | diff --git a/openmls/src/group/core_group/mod.rs b/openmls/src/group/core_group/mod.rs index 5dcfd3226d..b17891cf1e 100644 --- a/openmls/src/group/core_group/mod.rs +++ b/openmls/src/group/core_group/mod.rs @@ -180,33 +180,12 @@ impl CoreGroupBuilder { self.psk_ids = psk_ids; self } - /// Set the [`RequiredCapabilitiesExtension`] of the [`CoreGroup`]. - pub(crate) fn with_required_capabilities( - mut self, - required_capabilities: RequiredCapabilitiesExtension, - ) -> Self { - self.public_group_builder = self - .public_group_builder - .with_required_capabilities(required_capabilities); - 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 } - /// Set the [`ExternalSendersExtension`] of the [`CoreGroup`]. - pub(crate) fn with_external_senders( - mut self, - external_senders: ExternalSendersExtension, - ) -> Self { - if !external_senders.is_empty() { - self.public_group_builder = self - .public_group_builder - .with_external_senders(external_senders); - } - self - } /// Sets initial group context extensions. Note that RequiredCapabilities /// extensions will be overwritten, and should be set using a call to diff --git a/openmls/src/group/core_group/test_proposals.rs b/openmls/src/group/core_group/test_proposals.rs index 4f9812c9a1..21209b115c 100644 --- a/openmls/src/group/core_group/test_proposals.rs +++ b/openmls/src/group/core_group/test_proposals.rs @@ -298,7 +298,10 @@ fn test_required_unsupported_proposals(ciphersuite: Ciphersuite, provider: &impl CryptoConfig::with_default_version(ciphersuite), alice_credential, ) - .with_required_capabilities(required_capabilities) + .with_group_context_extensions(Extensions::single(Extension::RequiredCapabilities( + required_capabilities, + ))) + .unwrap() .build(provider, &alice_signer) .expect_err( "CoreGroup creation must fail because AppAck proposals aren't supported in OpenMLS yet.", @@ -339,7 +342,10 @@ fn test_group_context_extensions(ciphersuite: Ciphersuite, provider: &impl OpenM CryptoConfig::with_default_version(ciphersuite), alice_credential, ) - .with_required_capabilities(required_capabilities) + .with_group_context_extensions(Extensions::single(Extension::RequiredCapabilities( + required_capabilities, + ))) + .unwrap() .build(provider, &alice_signer) .expect("Error creating CoreGroup."); @@ -417,7 +423,10 @@ fn test_group_context_extension_proposal_fails( CryptoConfig::with_default_version(ciphersuite), alice_credential, ) - .with_required_capabilities(required_capabilities) + .with_group_context_extensions(Extensions::single(Extension::RequiredCapabilities( + required_capabilities, + ))) + .unwrap() .build(provider, &alice_signer) .expect("Error creating CoreGroup."); diff --git a/openmls/src/group/mls_group/builder.rs b/openmls/src/group/mls_group/builder.rs index bcd27d8f66..6607727dbb 100644 --- a/openmls/src/group/mls_group/builder.rs +++ b/openmls/src/group/mls_group/builder.rs @@ -2,13 +2,16 @@ use openmls_traits::{key_store::OpenMlsKeyStore, signatures::Signer, OpenMlsProv use crate::{ credentials::CredentialWithKey, - extensions::{errors::InvalidExtensionError, Extensions, ExternalSendersExtension}, + error::LibraryError, + extensions::{errors::InvalidExtensionError, Extensions}, group::{ config::CryptoConfig, public_group::errors::PublicGroupBuildError, CoreGroup, CoreGroupBuildError, CoreGroupConfig, GroupId, MlsGroupCreateConfig, MlsGroupCreateConfigBuilder, NewGroupError, ProposalStore, WireFormatPolicy, }, - prelude::{Capabilities, LibraryError, Lifetime, SenderRatchetConfiguration}, + key_packages::Lifetime, + tree::sender_ratchet::SenderRatchetConfiguration, + treesync::node::leaf_node::Capabilities, }; use super::{InnerState, MlsGroup, MlsGroupState}; @@ -70,8 +73,6 @@ impl MlsGroupBuilder { credential_with_key, ) .with_config(group_config) - .with_required_capabilities(mls_group_create_config.required_capabilities.clone()) - .with_external_senders(mls_group_create_config.external_senders.clone()) .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()) @@ -194,22 +195,6 @@ impl MlsGroupBuilder { self } - /// Sets the `external_senders` property of the MlsGroup. - pub fn external_senders(mut self, external_senders: ExternalSendersExtension) -> Self { - self.mls_group_create_config_builder = self - .mls_group_create_config_builder - .external_senders(external_senders); - self - } - - /// Sets the group creator's [`Capabilities`] - pub fn with_capabilities(mut self, capabilities: Capabilities) -> Self { - self.mls_group_create_config_builder = self - .mls_group_create_config_builder - .capabilities(capabilities); - self - } - /// Sets the initial group context extensions pub fn with_group_context_extensions( mut self, @@ -231,4 +216,12 @@ impl MlsGroupBuilder { .with_leaf_node_extensions(extensions)?; Ok(self) } + + /// Sets the group creator's [`Capabilities`] + pub fn with_capabilities(mut self, capabilities: Capabilities) -> Self { + self.mls_group_create_config_builder = self + .mls_group_create_config_builder + .capabilities(capabilities); + self + } } diff --git a/openmls/src/group/mls_group/config.rs b/openmls/src/group/mls_group/config.rs index 673548ad5d..884e160388 100644 --- a/openmls/src/group/mls_group/config.rs +++ b/openmls/src/group/mls_group/config.rs @@ -83,12 +83,8 @@ impl MlsGroupJoinConfig { /// more information about the different configuration values. #[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] pub struct MlsGroupCreateConfig { - /// Required capabilities (extensions and proposal types) - pub(crate) required_capabilities: RequiredCapabilitiesExtension, /// Capabilities advertised in the creator's leaf node pub(crate) capabilities: Capabilities, - /// Senders authorized to send external remove proposals - pub(crate) external_senders: ExternalSendersExtension, /// Lifetime of the own leaf node pub(crate) lifetime: Lifetime, /// Ciphersuite and protocol version @@ -191,21 +187,11 @@ impl MlsGroupCreateConfig { self.join_config.use_ratchet_tree_extension } - /// Returns the [`MlsGroupCreateConfig`] required capabilities extension. - pub fn required_capabilities(&self) -> &RequiredCapabilitiesExtension { - &self.required_capabilities - } - /// Returns the [`MlsGroupCreateConfig`] sender ratchet configuration. pub fn sender_ratchet_configuration(&self) -> &SenderRatchetConfiguration { &self.join_config.sender_ratchet_configuration } - /// Returns the [`MlsGroupCreateConfig`] external senders extension - pub fn external_senders(&self) -> &ExternalSendersExtension { - &self.external_senders - } - /// Returns the [`Extensions`] set as the initial group context. /// This does not contain the initial group context extensions /// added from builder calls to `external_senders` or `required_capabilities`. @@ -293,15 +279,6 @@ impl MlsGroupCreateConfigBuilder { self } - /// Sets the `required_capabilities` property of the MlsGroupCreateConfig. - pub fn required_capabilities( - mut self, - required_capabilities: RequiredCapabilitiesExtension, - ) -> Self { - self.config.required_capabilities = required_capabilities; - self - } - /// Sets the `capabilities` of the group creator's leaf node. pub fn capabilities(mut self, capabilities: Capabilities) -> Self { self.config.capabilities = capabilities; @@ -330,17 +307,7 @@ impl MlsGroupCreateConfigBuilder { self } - /// Sets the `external_senders` property of the MlsGroupCreateConfig. - pub fn external_senders(mut self, external_senders: ExternalSendersExtension) -> Self { - self.config.external_senders = external_senders; - 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. + /// Sets initial group context extensions. pub fn with_group_context_extensions( mut self, extensions: Extensions, diff --git a/openmls/src/group/mls_group/test_mls_group.rs b/openmls/src/group/mls_group/test_mls_group.rs index d97c9d39d9..3052980b8f 100644 --- a/openmls/src/group/mls_group/test_mls_group.rs +++ b/openmls/src/group/mls_group/test_mls_group.rs @@ -1016,14 +1016,12 @@ fn group_context_extensions_proposal(ciphersuite: Ciphersuite, provider: &impl O .build(provider, &alice_signer, alice_credential_with_key) .expect("error creating group using builder"); - let required_capabilities = alice_group + // No required capabilities, so no specifically required extensions. + assert!(alice_group .group() .group_context_extensions() .required_capabilities() - .expect("couldn't get required_capabilities"); - - // no required extensions - assert!(required_capabilities.extension_types().is_empty()); + .is_none()); let new_extensions = Extensions::single(Extension::RequiredCapabilities( RequiredCapabilitiesExtension::new(&[ExtensionType::RequiredCapabilities], &[], &[]), @@ -1128,7 +1126,10 @@ fn builder_pattern(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { .with_group_id(test_group_id.clone()) .padding_size(test_padding_size) .sender_ratchet_configuration(test_sender_ratchet_config.clone()) - .external_senders(test_external_senders.clone()) + .with_group_context_extensions(Extensions::single(Extension::ExternalSenders( + test_external_senders.clone(), + ))) + .unwrap() .crypto_config(test_crypto_config) .with_wire_format_policy(test_wire_format_policy) .lifetime(test_lifetime) diff --git a/openmls/src/group/public_group/builder.rs b/openmls/src/group/public_group/builder.rs index 54d1edd453..0e02a525fc 100644 --- a/openmls/src/group/public_group/builder.rs +++ b/openmls/src/group/public_group/builder.rs @@ -6,7 +6,7 @@ use crate::{ error::LibraryError, extensions::{ errors::{ExtensionError, InvalidExtensionError}, - Extension, Extensions, ExternalSendersExtension, RequiredCapabilitiesExtension, + Extensions, }, group::{config::CryptoConfig, ExtensionType, GroupContext, GroupId}, key_packages::Lifetime, @@ -24,9 +24,7 @@ pub(crate) struct TempBuilderPG1 { crypto_config: CryptoConfig, credential_with_key: CredentialWithKey, lifetime: Option, - required_capabilities: Option, capabilities: Option, - external_senders: Option, leaf_node_extensions: Extensions, group_context_extensions: Extensions, } @@ -37,29 +35,11 @@ impl TempBuilderPG1 { self } - pub(crate) fn with_required_capabilities( - mut self, - required_capabilities: RequiredCapabilitiesExtension, - ) -> Self { - self.required_capabilities = Some(required_capabilities); - self - } - pub(crate) fn with_capabilities(mut self, capabilities: Capabilities) -> Self { self.capabilities = Some(capabilities); self } - pub(crate) fn with_external_senders( - mut self, - external_senders: ExternalSendersExtension, - ) -> Self { - if !external_senders.is_empty() { - self.external_senders = Some(external_senders); - } - self - } - pub(crate) fn with_group_context_extensions( mut self, extensions: Extensions, @@ -95,21 +75,37 @@ impl TempBuilderPG1 { provider: &impl OpenMlsProvider, signer: &impl Signer, ) -> Result<(TempBuilderPG2, CommitSecret, EncryptionKeyPair), PublicGroupBuildError> { - // Capabilities are either provided by the builder or a minimal set of - // capabilities is created from the crypto config and the required - // capabilities + // If there are no capabilities, we want to provide a default version + // plus anything in the required capabilities. + let (required_extensions, required_proposals, required_credentials) = + if let Some(required_capabilities) = + self.group_context_extensions.required_capabilities() + { + // Also, while we're at it, check if we support all required + // capabilities ourselves. + required_capabilities.check_support().map_err(|e| match e { + ExtensionError::UnsupportedProposalType => { + PublicGroupBuildError::UnsupportedProposalType + } + ExtensionError::UnsupportedExtensionType => { + PublicGroupBuildError::UnsupportedExtensionType + } + _ => LibraryError::custom("Unexpected ExtensionError").into(), + })?; + ( + Some(required_capabilities.extension_types()), + Some(required_capabilities.proposal_types()), + Some(required_capabilities.credential_types()), + ) + } else { + (None, None, None) + }; let capabilities = self.capabilities.unwrap_or(Capabilities::new( Some(&[self.crypto_config.version]), Some(&[self.crypto_config.ciphersuite]), - self.required_capabilities - .as_ref() - .map(|re| re.extension_types()), - self.required_capabilities - .as_ref() - .map(|re| re.proposal_types()), - self.required_capabilities - .as_ref() - .map(|re| re.credential_types()), + required_extensions, + required_proposals, + required_credentials, )); let (treesync, commit_secret, leaf_keypair) = TreeSync::new( provider, @@ -120,29 +116,12 @@ impl TempBuilderPG1 { capabilities, self.leaf_node_extensions, )?; - let required_capabilities = self.required_capabilities.unwrap_or_default(); - required_capabilities.check_support().map_err(|e| match e { - ExtensionError::UnsupportedProposalType => { - PublicGroupBuildError::UnsupportedProposalType - } - ExtensionError::UnsupportedExtensionType => { - PublicGroupBuildError::UnsupportedExtensionType - } - _ => LibraryError::custom("Unexpected ExtensionError").into(), - })?; - let required_capabilities = Extension::RequiredCapabilities(required_capabilities); - - let mut group_context_extensions = self.group_context_extensions; - group_context_extensions.add_or_replace(required_capabilities); - if let Some(ext_senders) = self.external_senders { - group_context_extensions.add_or_replace(Extension::ExternalSenders(ext_senders)); - } let group_context = GroupContext::create_initial_group_context( self.crypto_config.ciphersuite, self.group_id, treesync.tree_hash().to_vec(), - group_context_extensions, + self.group_context_extensions, ); let next_builder = TempBuilderPG2 { treesync, @@ -214,9 +193,7 @@ impl PublicGroup { crypto_config, credential_with_key, lifetime: None, - required_capabilities: None, capabilities: None, - external_senders: None, leaf_node_extensions: Extensions::empty(), group_context_extensions: Extensions::empty(), } diff --git a/openmls/src/group/tests/external_remove_proposal.rs b/openmls/src/group/tests/external_remove_proposal.rs index bb057e4e35..e29d3d707b 100644 --- a/openmls/src/group/tests/external_remove_proposal.rs +++ b/openmls/src/group/tests/external_remove_proposal.rs @@ -30,7 +30,10 @@ fn new_test_group( let mls_group_config = MlsGroupCreateConfig::builder() .wire_format_policy(wire_format_policy) .crypto_config(CryptoConfig::with_default_version(ciphersuite)) - .external_senders(external_senders) + .with_group_context_extensions(Extensions::single(Extension::ExternalSenders( + external_senders, + ))) + .unwrap() .build(); ( @@ -336,6 +339,6 @@ fn external_remove_proposal_should_fail_when_no_external_senders( .unwrap_err(); assert_eq!( error, - ProcessMessageError::ValidationError(ValidationError::NoExternalSendersExtension) + ProcessMessageError::ValidationError(ValidationError::UnauthorizedExternalSender) ); } diff --git a/openmls/tests/book_code.rs b/openmls/tests/book_code.rs index c3ef6832a0..9bf48cc137 100644 --- a/openmls/tests/book_code.rs +++ b/openmls/tests/book_code.rs @@ -133,6 +133,13 @@ fn book_operations(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { 10, // out_of_order_tolerance 2000, // maximum_forward_distance )) + .with_group_context_extensions(Extensions::single(Extension::ExternalSenders(vec![ + ExternalSender::new( + ds_credential_with_key.signature_key.clone(), + ds_credential_with_key.credential.clone(), + ), + ]))) + .expect("error adding external senders extension to group context extensions") .crypto_config(CryptoConfig::with_default_version(ciphersuite)) .capabilities(Capabilities::new( None, // Defaults to the group's protocol version @@ -147,10 +154,6 @@ fn book_operations(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { UnknownExtension(vec![0, 1, 2, 3]), ))) .expect("failed to configure leaf extensions") - .external_senders(vec![ExternalSender::new( - ds_credential_with_key.signature_key.clone(), - ds_credential_with_key.credential.clone(), - )]) .use_ratchet_tree_extension(true) .build(); // ANCHOR_END: mls_group_create_config_example @@ -214,10 +217,6 @@ fn book_operations(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { 10, // out_of_order_tolerance 2000, // maximum_forward_distance )) - .external_senders(vec![ExternalSender::new( - ds_credential_with_key.signature_key, - ds_credential_with_key.credential, - )]) .crypto_config(CryptoConfig::with_default_version(ciphersuite)) .use_ratchet_tree_extension(true) .build(provider, &alice_signature_keys, alice_credential.clone()) From 303967cf2304c78e9d622b0cd381b7a15dcb5fb9 Mon Sep 17 00:00:00 2001 From: Konrad Kohbrok Date: Fri, 19 Jan 2024 14:19:13 +0100 Subject: [PATCH 4/4] Enable the use of unknown/opaque extensions (#1479) --- CHANGELOG.md | 1 + book/src/user_manual/group_config.md | 4 + openmls/src/extensions/mod.rs | 15 --- .../src/extensions/required_capabilities.rs | 5 - .../src/group/core_group/test_proposals.rs | 72 ++++++++---- openmls/src/group/mls_group/test_mls_group.rs | 111 ++++++++++++++++-- 6 files changed, 157 insertions(+), 51 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a5f0809f13..11a1376e1d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - [#1475](https://github.com/openmls/openmls/pull/1475): Fully process GroupContextExtension proposals - [#1477](https://github.com/openmls/openmls/pull/1477): Allow setting leaf node extensions and capabilities of the group creator when creating an MlsGroup(Config) - [#1478](https://github.com/openmls/openmls/pull/1478): Remove explicit functions to set `RequiredCapabilitiesExtension` and `ExternalSendersExtension` when building an MlsGroup(Config) in favor of the more general function to set group context extensions +- [#1479](https://github.com/openmls/openmls/pull/1479): Allow the use of extensions with `ExtensionType::Unknown` in group context, key packages and leaf nodes ## 0.5.0 (XXXX-XX-XX) diff --git a/book/src/user_manual/group_config.md b/book/src/user_manual/group_config.md index 55e6f5b742..dbaf943c14 100644 --- a/book/src/user_manual/group_config.md +++ b/book/src/user_manual/group_config.md @@ -34,3 +34,7 @@ Example create configuration: ```rust,no_run,noplayground {{#include ../../../openmls/tests/book_code.rs:mls_group_create_config_example}} ``` + +## Unknown extensions + +Some extensions carry data, but don't alter the behaviour of the protocol (e.g. the application_id extension). OpenMLS allows the use of arbitrary such extensions in the group context, key packages and leaf nodes. Such extensions can be instantiated and retrieved through the use of the `UnknownExtension` struct and the `ExtensionType::Unknown` extension type. Such "unknown" extensions are handled transparently by OpenMLS, but can be used by the application, e.g. to have a group agree on pieces of data. diff --git a/openmls/src/extensions/mod.rs b/openmls/src/extensions/mod.rs index 9c2554d015..20f46beec5 100644 --- a/openmls/src/extensions/mod.rs +++ b/openmls/src/extensions/mod.rs @@ -167,21 +167,6 @@ impl From for u16 { } } -impl ExtensionType { - /// Check whether an [`ExtensionType`] is supported or not. - pub fn is_supported(&self) -> bool { - matches!( - self, - ExtensionType::ApplicationId - | ExtensionType::RatchetTree - | ExtensionType::RequiredCapabilities - | ExtensionType::ExternalPub - | ExtensionType::ExternalSenders - | ExtensionType::LastResort - ) - } -} - /// # Extension /// /// An extension is one of the [`Extension`] enum values. diff --git a/openmls/src/extensions/required_capabilities.rs b/openmls/src/extensions/required_capabilities.rs index d00081beaf..9dd5b0f9bb 100644 --- a/openmls/src/extensions/required_capabilities.rs +++ b/openmls/src/extensions/required_capabilities.rs @@ -78,11 +78,6 @@ impl RequiredCapabilitiesExtension { /// Check if all extension and proposal types are supported. pub(crate) fn check_support(&self) -> Result<(), ExtensionError> { - for extension in self.extension_types() { - if !extension.is_supported() { - return Err(ExtensionError::UnsupportedExtensionType); - } - } for proposal in self.proposal_types() { if !proposal.is_supported() { return Err(ExtensionError::UnsupportedProposalType); diff --git a/openmls/src/group/core_group/test_proposals.rs b/openmls/src/group/core_group/test_proposals.rs index 21209b115c..197c2d3272 100644 --- a/openmls/src/group/core_group/test_proposals.rs +++ b/openmls/src/group/core_group/test_proposals.rs @@ -312,6 +312,56 @@ fn test_required_unsupported_proposals(ciphersuite: Ciphersuite, provider: &impl ) } +#[apply(ciphersuites_and_providers)] +fn test_required_extension_key_package_mismatch( + ciphersuite: Ciphersuite, + provider: &impl OpenMlsProvider, +) { + // Basic group setup. + let group_aad = b"Alice's test group"; + let framing_parameters = FramingParameters::new(group_aad, WireFormat::PublicMessage); + + let (alice_credential, _, alice_signer, _alice_pk) = + setup_client("Alice", ciphersuite, provider); + let (_bob_credential_with_key, bob_key_package_bundle, _, _) = + setup_client("Bob", ciphersuite, provider); + let bob_key_package = bob_key_package_bundle.key_package(); + + // Set required capabilities + let extensions = &[ExtensionType::Unknown(0xff00)]; + // We don't support unknown proposals (yet) + let proposals = &[]; + let credentials = &[CredentialType::Basic]; + let required_capabilities = + RequiredCapabilitiesExtension::new(extensions, proposals, credentials); + + let alice_group = CoreGroup::builder( + GroupId::random(provider.rand()), + CryptoConfig::with_default_version(ciphersuite), + alice_credential, + ) + .with_group_context_extensions(Extensions::single(Extension::RequiredCapabilities( + required_capabilities, + ))) + .expect("error adding group context extensions") + .build(provider, &alice_signer) + .expect("Error creating CoreGroup."); + + let e = alice_group + .create_add_proposal( + framing_parameters, + bob_key_package.clone(), + &alice_signer, + ) + .expect_err("Proposal was created even though the key package didn't support the required extensions."); + assert_eq!( + e, + CreateAddProposalError::LeafNodeValidation( + crate::treesync::errors::LeafNodeValidationError::UnsupportedExtensions + ) + ); +} + #[apply(ciphersuites_and_providers)] fn test_group_context_extensions(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { // Basic group setup. @@ -430,28 +480,6 @@ fn test_group_context_extension_proposal_fails( .build(provider, &alice_signer) .expect("Error creating CoreGroup."); - // TODO: openmls/openmls#1130 add a test for unsupported required capabilities. - // We can't test this right now because we don't have a capability - // that is not a "default" proposal or extension. - // // Alice tries to add a required capability she doesn't support herself. - // let required_application_id = Extension::RequiredCapabilities( - // RequiredCapabilitiesExtension::new(&[ExtensionType::ApplicationId], &[]), - // ); - // let e = alice_group.create_group_context_ext_proposal( - // framing_parameters, - // &alice_credential_bundle, - // &[required_application_id.clone()], - // provider, - // ).expect_err("Alice was able to create a gce proposal with a required extensions she doesn't support."); - // assert_eq!( - // e, - // CreateGroupContextExtProposalError::TreeSyncError( - // crate::treesync::errors::TreeSyncError::UnsupportedExtension - // ) - // ); - // - // // Well, this failed luckily. - // Adding Bob let bob_add_proposal = alice_group .create_add_proposal(framing_parameters, bob_key_package.clone(), &alice_signer) diff --git a/openmls/src/group/mls_group/test_mls_group.rs b/openmls/src/group/mls_group/test_mls_group.rs index 3052980b8f..55658d3ce6 100644 --- a/openmls/src/group/mls_group/test_mls_group.rs +++ b/openmls/src/group/mls_group/test_mls_group.rs @@ -1101,10 +1101,19 @@ fn builder_pattern(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { let test_lifetime = Lifetime::new(3600); let test_wire_format_policy = PURE_CIPHERTEXT_WIRE_FORMAT_POLICY; let test_padding_size = 100; - let test_external_senders = vec![ExternalSender::new( + let test_external_senders = Extension::ExternalSenders(vec![ExternalSender::new( alice_credential_with_key.signature_key.clone(), alice_credential_with_key.credential.clone(), - )]; + )]); + let test_required_capabilities = Extension::RequiredCapabilities( + RequiredCapabilitiesExtension::new(&[ExtensionType::Unknown(0xff00)], &[], &[]), + ); + let test_gc_extensions = Extensions::from_vec(vec![ + test_external_senders.clone(), + test_required_capabilities.clone(), + ]) + .expect("error creating group context extensions"); + let test_crypto_config = CryptoConfig::with_default_version(ciphersuite); let test_sender_ratchet_config = SenderRatchetConfiguration::new(10, 2000); let test_max_past_epochs = 10; @@ -1126,10 +1135,8 @@ fn builder_pattern(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { .with_group_id(test_group_id.clone()) .padding_size(test_padding_size) .sender_ratchet_configuration(test_sender_ratchet_config.clone()) - .with_group_context_extensions(Extensions::single(Extension::ExternalSenders( - test_external_senders.clone(), - ))) - .unwrap() + .with_group_context_extensions(test_gc_extensions.clone()) + .expect("error adding group context extension to builder") .crypto_config(test_crypto_config) .with_wire_format_policy(test_wire_format_policy) .lifetime(test_lifetime) @@ -1137,7 +1144,7 @@ fn builder_pattern(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { .max_past_epochs(test_max_past_epochs) .number_of_resumption_psks(test_number_of_resumption_psks) .with_leaf_node_extensions(test_leaf_extensions.clone()) - .unwrap() + .expect("error adding leaf node extension to builder") .with_capabilities(test_capabilities.clone()) .build(provider, &alice_signer, alice_credential_with_key) .expect("error creating group using builder"); @@ -1165,13 +1172,19 @@ fn builder_pattern(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { let external_senders = group_context .extensions() .external_senders() - .expect("error getting external senders"); - assert_eq!(external_senders, &test_external_senders); + .expect("error getting external senders") + .to_vec(); + assert_eq!( + Extension::ExternalSenders(external_senders), + test_external_senders + ); let crypto_config = CryptoConfig { ciphersuite, version: group_context.protocol_version(), }; assert_eq!(crypto_config, test_crypto_config); + let extensions = group_context.extensions(); + assert_eq!(extensions, &test_gc_extensions); let lifetime = alice_group .own_leaf() .expect("error getting own leaf") @@ -1195,3 +1208,83 @@ fn builder_pattern(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { .expect_err("successfully built group with invalid leaf extensions"); assert_eq!(builder_err, InvalidExtensionError::IllegalInLeafNodes); } + +// Test that unknown group context and leaf node extensions can be used in groups +#[apply(ciphersuites_and_providers)] +fn unknown_extensions(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { + let (alice_credential_with_key, _alice_kpb, alice_signer, _alice_pk) = + setup_client("Alice", ciphersuite, provider); + + let unknown_gc_extension = Extension::Unknown(0xff00, UnknownExtension(vec![0, 1, 2, 3])); + let unknown_leaf_extension = Extension::Unknown(0xff01, UnknownExtension(vec![4, 5, 6, 7])); + let unknown_kp_extension = Extension::Unknown(0xff02, UnknownExtension(vec![8, 9, 10, 11])); + let required_extensions = &[ + ExtensionType::Unknown(0xff00), + ExtensionType::Unknown(0xff01), + ]; + let required_capabilities = + Extension::RequiredCapabilities(RequiredCapabilitiesExtension::new(&[], &[], &[])); + let capabilities = Capabilities::new(None, None, Some(required_extensions), None, None); + let test_gc_extensions = Extensions::from_vec(vec![ + unknown_gc_extension.clone(), + required_capabilities.clone(), + ]) + .expect("error creating group context extensions"); + let test_kp_extensions = Extensions::single(unknown_kp_extension.clone()); + + // === Alice creates a group === + let config = CryptoConfig { + ciphersuite, + version: crate::versions::ProtocolVersion::default(), + }; + let mut alice_group = MlsGroup::builder() + .crypto_config(config) + .with_capabilities(capabilities.clone()) + .with_leaf_node_extensions(Extensions::single(unknown_leaf_extension.clone())) + .expect("error adding unknown leaf extension to builder") + .with_group_context_extensions(test_gc_extensions.clone()) + .expect("error adding unknown extension to builder") + .build(provider, &alice_signer, alice_credential_with_key) + .expect("error creating group using builder"); + + // Check that everything was added successfully + let group_context = alice_group.export_group_context(); + assert_eq!(group_context.extensions(), &test_gc_extensions); + let leaf_node = alice_group.own_leaf().expect("error getting own leaf"); + assert_eq!( + leaf_node.extensions(), + &Extensions::single(unknown_leaf_extension) + ); + + // Now let's add Bob to the group and make sure that he joins the group successfully + + // === Alice adds Bob === + let (bob_credential_with_key, _bob_kpb, bob_signer, _bob_pk) = + setup_client("Bob", ciphersuite, provider); + + // Generate a KP that supports the unknown extensions + let bob_key_package = KeyPackage::builder() + .leaf_node_capabilities(capabilities) + .key_package_extensions(test_kp_extensions.clone()) + .build(config, provider, &bob_signer, bob_credential_with_key) + .expect("error building key package"); + + assert_eq!( + bob_key_package.extensions(), + &Extensions::single(unknown_kp_extension) + ); + + // alice adds bob and bob processes the welcome to ensure that the unknown + // extensions are processed correctly + let (_, welcome, _) = alice_group + .add_members(provider, &alice_signer, &[bob_key_package.clone()]) + .unwrap(); + alice_group.merge_pending_commit(provider).unwrap(); + let _bob_group = MlsGroup::new_from_welcome( + provider, + &MlsGroupJoinConfig::default(), + welcome.into_welcome().unwrap(), + Some(alice_group.export_ratchet_tree().into()), + ) + .expect("Error creating group from Welcome"); +}