diff --git a/mls-rs/src/client.rs b/mls-rs/src/client.rs index cb610c0e..e6a380a1 100644 --- a/mls-rs/src/client.rs +++ b/mls-rs/src/client.rs @@ -653,23 +653,20 @@ where self.config.clone(), group_info_msg, ) - .await? .build() .await } - #[cfg_attr(not(mls_build_async), maybe_async::must_be_sync)] - pub async fn external_commit_builder( + pub fn external_commit_builder( &self, group_info_msg: MlsMessage, ) -> Result, MlsError> { - ExternalCommitBuilder::new( + Ok(ExternalCommitBuilder::new( self.signer()?.clone(), self.signing_identity()?.0.clone(), self.config.clone(), group_info_msg, - ) - .await + )) } /// Load an existing group state into this client using the @@ -1007,10 +1004,7 @@ mod tests { .signing_identity(new_client_identity.clone(), secret_key, TEST_CIPHER_SUITE) .build(); - let mut builder = new_client - .external_commit_builder(group_info_msg) - .await - .unwrap(); + let mut builder = new_client.external_commit_builder(group_info_msg).unwrap(); if do_remove { builder = builder.with_removal(1); @@ -1125,7 +1119,6 @@ mod tests { let (_, external_commit) = carol .external_commit_builder(group_info_msg) - .await .unwrap() .build() .await diff --git a/mls-rs/src/group/commit/builder.rs b/mls-rs/src/group/commit/builder.rs index ca017cd2..73fa594c 100644 --- a/mls-rs/src/group/commit/builder.rs +++ b/mls-rs/src/group/commit/builder.rs @@ -8,6 +8,7 @@ use alloc::{vec, vec::Vec}; use mls_rs_codec::{MlsDecode, MlsEncode, MlsSize}; use mls_rs_core::{crypto::SignatureSecretKey, error::IntoAnyError}; +use crate::mls_rules::{ProposalBundle, ProposalSource}; use crate::{ cipher_suite::CipherSuite, client::MlsError, @@ -269,7 +270,7 @@ where /// a resumption PSK into the current commit that is being built. #[cfg(feature = "psk")] pub fn add_resumption_psk_for_group( - self, + mut self, psk_epoch: u64, group_id: Vec, psk: crate::psk::PreSharedKey, diff --git a/mls-rs/src/group/commit/processor.rs b/mls-rs/src/group/commit/processor.rs index f94a4e27..61781b85 100644 --- a/mls-rs/src/group/commit/processor.rs +++ b/mls-rs/src/group/commit/processor.rs @@ -5,7 +5,10 @@ use alloc::boxed::Box; use alloc::vec::Vec; -use mls_rs_core::{group::ConfirmedTranscriptHash, time::MlsTime}; +use mls_rs_core::{ + group::{ConfirmedTranscriptHash, GroupContext}, + time::MlsTime, +}; use crate::{ client_config::ClientConfig, @@ -134,9 +137,6 @@ pub(crate) async fn process_commit( let id_provider = commit_processor.processor.identity_provider(); let cs_provider = commit_processor.processor.cipher_suite_provider(); - // TODO remove - let user_rules = commit_processor.processor.mls_rules(); - let mut provisional_state = commit_processor .processor .group_state() @@ -309,7 +309,6 @@ impl CommitProcessor<'_, C> { pub fn with_resumption_psk(mut self, id: ResumptionPsk, psk: crate::psk::PreSharedKey) -> Self { self.0.psks.push((JustPreSharedKeyID::Resumption(id), psk)); self - } pub fn proposals_mut(&mut self) -> &mut ProposalBundle { @@ -358,7 +357,6 @@ impl CommitProcessor<'_, C> { pub fn context(&self) -> &GroupContext { self.0.processor.context() } - } impl Group { diff --git a/mls-rs/src/group/external_commit.rs b/mls-rs/src/group/external_commit.rs index 1945ebe8..f535c344 100644 --- a/mls-rs/src/group/external_commit.rs +++ b/mls-rs/src/group/external_commit.rs @@ -4,7 +4,6 @@ use mls_rs_core::{ crypto::SignatureSecretKey, extension::ExtensionList, identity::SigningIdentity, - protocol_version::ProtocolVersion, }; use crate::{ @@ -13,8 +12,8 @@ use crate::{ cipher_suite_provider, epoch::SenderDataSecret, key_schedule::{InitSecret, KeySchedule}, - proposal::{ExternalInit, Proposal, RemoveProposal}, - EpochSecrets, ExternalPubExt, LeafIndex, LeafNode, MlsError, TreeKemPrivate, + proposal::{ExternalInit, Proposal}, + EpochSecrets, ExternalPubExt, LeafNode, MlsError, TreeKemPrivate, }, mls_rules::{ProposalBundle, ProposalSource}, Group, MlsMessage, @@ -39,7 +38,7 @@ use crate::group::{ PreSharedKeyProposal, {JustPreSharedKeyID, PreSharedKeyID}, }; -use super::{validate_tree_and_info_joiner, ExportedTree}; +use super::{validate_tree_and_info_joiner, ExportedTree, Sender}; /// A builder that aids with the construction of an external commit. #[cfg_attr(all(feature = "ffi", not(test)), safer_ffi_gen::ffi_type(opaque))] @@ -49,64 +48,39 @@ pub struct ExternalCommitBuilder { leaf_node_extensions: ExtensionList, config: C, tree_data: Option>, + to_remove: Option, external_psks: Vec<(ExternalPskId, PreSharedKey)>, #[cfg(feature = "psk")] authenticated_data: Vec, #[cfg(feature = "custom_proposal")] - received_custom_proposals: Vec, - proposals: ProposalBundle, - group_info: GroupInfo, - protocol_version: ProtocolVersion, - init_secret: InitSecret, + custom_proposals: Vec, + #[cfg(feature = "custom_proposal")] + received_custom_proposals: Vec, + group_info: MlsMessage, } impl ExternalCommitBuilder { - #[cfg_attr(not(mls_build_async), maybe_async::must_be_sync)] - pub(crate) async fn new( + pub(crate) fn new( signer: SignatureSecretKey, signing_identity: SigningIdentity, config: C, group_info: MlsMessage, - ) -> Result { - let protocol_version = group_info.version; - - if !config.version_supported(protocol_version) { - return Err(MlsError::UnsupportedProtocolVersion(protocol_version)); - } - - let group_info = group_info - .into_group_info() - .ok_or(MlsError::UnexpectedMessageType)?; - - let cipher_suite = cipher_suite_provider( - config.crypto_provider(), - group_info.group_context.cipher_suite, - )?; - - let external_pub_ext = group_info - .extensions - .get_as::()? - .ok_or(MlsError::MissingExternalPubExtension)?; - - let (init_secret, kem_output) = - InitSecret::encode_for_external(&cipher_suite, &external_pub_ext.external_pub).await?; - - let builder = Self { - tree_data: None, - authenticated_data: Vec::new(), + ) -> Self { + Self { signer, signing_identity, - leaf_node_extensions: Default::default(), config, + group_info, + tree_data: None, + authenticated_data: Vec::new(), + leaf_node_extensions: Default::default(), + to_remove: Default::default(), + #[cfg(feature = "custom_proposal")] + custom_proposals: Vec::new(), #[cfg(feature = "custom_proposal")] received_custom_proposals: Vec::new(), - proposals: Default::default(), - group_info, - protocol_version, - init_secret, - }; - - Ok(builder.with_proposal(Proposal::ExternalInit(ExternalInit { kem_output }))) + external_psks: Default::default(), + } } #[must_use] @@ -123,9 +97,10 @@ impl ExternalCommitBuilder { /// Propose the removal of an old version of the client as part of the external commit. /// Only one such proposal is allowed. pub fn with_removal(self, to_remove: u32) -> Self { - self.with_proposal(Proposal::Remove(RemoveProposal { - to_remove: LeafIndex(to_remove), - })) + Self { + to_remove: Some(to_remove), + ..self + } } #[must_use] @@ -147,8 +122,9 @@ impl ExternalCommitBuilder { #[cfg(feature = "custom_proposal")] #[must_use] /// Insert a [`CustomProposal`] into the current commit that is being built. - pub fn with_custom_proposal(self, proposal: CustomProposal) -> Self { - self.with_proposal(Proposal::Custom(proposal)) + pub fn with_custom_proposal(mut self, proposal: CustomProposal) -> Self { + self.custom_proposals.push(Proposal::Custom(proposal)); + self } #[cfg(all(feature = "custom_proposal", feature = "by_ref_proposal"))] @@ -160,20 +136,9 @@ impl ExternalCommitBuilder { /// The authenticity of the proposal is NOT fully verified. It is only verified the /// same way as by [`ExternalGroup`](`crate::external_client::ExternalGroup`). /// The proposal MUST be an MlsPlaintext, else the [`Self::build`] function will fail. - pub fn with_received_custom_proposal(mut self, proposal: MlsMessage) -> Result { - let MlsMessagePayload::Plain(plaintext) = proposal.payload else { - return Err(MlsError::UnexpectedMessageType); - }; - - let super::Content::Proposal(proposal) = plaintext.content.content.clone() else { - return Err(MlsError::UnexpectedMessageType); - }; - - // We store proposal to verify authenticity later. At this point this may not be possible if we - // don't have the tree. - self.received_custom_proposals.push(plaintext); - - Ok(self.with_proposal(*proposal)) + pub fn with_received_custom_proposal(mut self, proposal: MlsMessage) -> Self { + self.received_custom_proposals.push(proposal); + self } /// Change the committer's leaf node extensions as part of making this commit. @@ -184,17 +149,19 @@ impl ExternalCommitBuilder { } } - fn with_proposal(mut self, proposal: Proposal) -> Self { - self.proposals - .add(proposal, Sender::NewMemberCommit, ProposalSource::ByValue); - - self - } - /// Build the external commit using a GroupInfo message provided by an existing group member. #[cfg_attr(not(mls_build_async), maybe_async::must_be_sync)] pub async fn build(self) -> Result<(Group, MlsMessage), MlsError> { - let group_info = self.group_info; + let protocol_version = self.group_info.version; + + if !self.config.version_supported(protocol_version) { + return Err(MlsError::UnsupportedProtocolVersion(protocol_version)); + } + + let group_info = self + .group_info + .into_group_info() + .ok_or(MlsError::UnexpectedMessageType)?; let cipher_suite = cipher_suite_provider( self.config.crypto_provider(), @@ -202,7 +169,7 @@ impl ExternalCommitBuilder { )?; let public_tree = validate_tree_and_info_joiner( - self.protocol_version, + protocol_version, &group_info, self.tree_data, &self.config.identity_provider(), @@ -227,11 +194,19 @@ impl ExternalCommitBuilder { secret_tree: SecretTree::empty(), }; + let external_pub_ext = group_info + .extensions + .get_as::()? + .ok_or(MlsError::MissingExternalPubExtension)?; + + let (init_secret, kem_output) = + InitSecret::encode_for_external(&cipher_suite, &external_pub_ext.external_pub).await?; + let (mut group, _) = Group::join_with( self.config, group_info, public_tree, - KeySchedule::new(self.init_secret), + KeySchedule::new(init_secret), epoch_secrets, TreeKemPrivate::new_for_external(), self.signer, @@ -240,6 +215,10 @@ impl ExternalCommitBuilder { let mut proposals = vec![Proposal::ExternalInit(ExternalInit { kem_output })]; + if let Some(to_remove) = self.to_remove { + proposals.push(Proposal::Remove(to_remove.into())); + } + #[cfg(feature = "psk")] let psks = self .external_psks @@ -267,12 +246,30 @@ impl ExternalCommitBuilder { #[cfg(all(feature = "custom_proposal", feature = "by_ref_proposal"))] for message in self.received_custom_proposals { - verify_plaintext_authentication(&cipher_suite, message, None, &group.state).await?; + let MlsMessagePayload::Plain(plaintext) = message.payload else { + return Err(MlsError::UnexpectedMessageType); + }; + + let super::Content::Proposal(proposal) = plaintext.content.content.clone() else { + return Err(MlsError::UnexpectedMessageType); + }; + + verify_plaintext_authentication(&cipher_suite, plaintext, None, &group.state).await?; + + proposals.push(*proposal); } + let proposal_bundle = + proposals + .into_iter() + .fold(ProposalBundle::default(), |mut bundle, proposal| { + bundle.add(proposal, Sender::NewMemberCommit, ProposalSource::ByValue); + bundle + }); + let (commit_output, pending_commit) = group .commit_internal( - self.proposals, + proposal_bundle, Some(&leaf_node), self.authenticated_data, Default::default(), @@ -281,6 +278,7 @@ impl ExternalCommitBuilder { None, #[cfg(feature = "psk")] psks, + Sender::NewMemberCommit, ) .await?; diff --git a/mls-rs/src/group/mod.rs b/mls-rs/src/group/mod.rs index cbeb7ca3..1bc236bc 100644 --- a/mls-rs/src/group/mod.rs +++ b/mls-rs/src/group/mod.rs @@ -2379,7 +2379,7 @@ mod tests { let group = test_group(protocol_version, cipher_suite).await; let info = group - .group_info_message(false) + .group_info_message(true) .await .unwrap() .into_group_info() @@ -2395,10 +2395,11 @@ mod tests { group.group.config, info_msg, ) - .await - .map(|_| {}); + .build() + .await; - assert_matches!(res, Err(MlsError::MissingExternalPubExtension)); + // assert_matches! needs Debug which Group doesn't have + assert!(matches!(res, Err(MlsError::MissingExternalPubExtension))); } #[maybe_async::test(not(mls_build_async), async(mls_build_async, crate::futures_test))] @@ -2421,7 +2422,6 @@ mod tests { test_client .external_commit_builder(commit_output.external_commit_group_info.unwrap()) - .await .unwrap() .build() .await @@ -2535,7 +2535,6 @@ mod tests { .await .unwrap(), ) - .await .unwrap() .build() .await @@ -2582,7 +2581,6 @@ mod tests { .await .unwrap(), ) - .await .unwrap() .with_tree_data(alice_group.export_tree().into_owned()) .build() @@ -3931,7 +3929,6 @@ mod tests { let commit = client_with_custom_rules(b"bob", mls_rules) .await .external_commit_builder(group_info) - .await .unwrap() .with_custom_proposal(CustomProposal::new(TEST_CUSTOM_PROPOSAL_TYPE, vec![])) .build() @@ -3965,10 +3962,8 @@ mod tests { let (_, commit) = client_with_custom_rules(b"bob", mls_rules) .await .external_commit_builder(group_info) - .await .unwrap() .with_received_custom_proposal(by_ref) - .unwrap() .build() .await .unwrap(); diff --git a/mls-rs/src/group/proposal_cache.rs b/mls-rs/src/group/proposal_cache.rs index 9f6cd253..614eddce 100644 --- a/mls-rs/src/group/proposal_cache.rs +++ b/mls-rs/src/group/proposal_cache.rs @@ -6,7 +6,7 @@ use alloc::vec::Vec; use super::{ message_processor::ProvisionalState, - proposal_filter::prepare_proposals_for_mls_rules, + mls_rules::{CommitDirection, CommitSource}, GroupState, ProposalOrRef, }; use crate::{ @@ -35,7 +35,7 @@ use crate::tree_kem::leaf_node::LeafNode; #[cfg(feature = "by_ref_proposal")] use mls_rs_codec::{MlsDecode, MlsEncode, MlsSize}; -use mls_rs_core::{crypto::CipherSuiteProvider, error::IntoAnyError, identity::IdentityProvider}; +use mls_rs_core::{crypto::CipherSuiteProvider, identity::IdentityProvider}; #[cfg(feature = "by_ref_proposal")] use core::fmt::{self, Debug}; @@ -238,7 +238,6 @@ impl GroupState { where C: IdentityProvider, CSP: CipherSuiteProvider, - F: MlsRules, { #[cfg(feature = "by_ref_proposal")] let all_proposals = proposals.clone(); @@ -376,6 +375,8 @@ pub(crate) mod test_utils { cipher_suite_provider: CSP, group_context_extensions: ExtensionList, psks: Vec, + } + impl<'a, CSP> CommitReceiver<'a, BasicWithCustomProvider, CSP> { pub fn new( tree: &'a TreeKemPublic, @@ -405,7 +406,7 @@ pub(crate) mod test_utils { CSP: CipherSuiteProvider, { #[cfg(feature = "by_ref_proposal")] - pub fn with_identity_provider(self, validator: V) -> CommitReceiver<'a, V, F, CSP> + pub fn with_identity_provider(self, validator: V) -> CommitReceiver<'a, V, CSP> where V: IdentityProvider, { @@ -608,7 +609,6 @@ mod tests { identity::basic::BasicIdentityProvider, identity::test_utils::{get_test_signing_identity, BasicWithCustomProvider}, key_package::test_utils::test_key_package, - mls_rules::{CommitOptions}, tree_kem::{ leaf_node::{ test_utils::{ @@ -642,7 +642,6 @@ mod tests { use crate::group::proposal::CustomProposal; use assert_matches::assert_matches; - use core::convert::Infallible; use itertools::Itertools; use mls_rs_core::crypto::{CipherSuite, CipherSuiteProvider}; use mls_rs_core::extension::ExtensionList; @@ -1871,14 +1870,6 @@ mod tests { self } - fn with_user_rules(self, f: G) -> CommitSender<'a, C, G, CSP> - { - CommitSender { - tree: self.tree, - sender: self.sender, - cache: self.cache, - additional_proposals: self.additional_proposals, - identity_provider: self.identity_provider, fn with_psks(self, psks: Vec) -> CommitSender<'a, C, CSP> { CommitSender { psks, ..self } } diff --git a/mls-rs/test_harness_integration/src/main.rs b/mls-rs/test_harness_integration/src/main.rs index eaf55506..3a554f60 100644 --- a/mls-rs/test_harness_integration/src/main.rs +++ b/mls-rs/test_harness_integration/src/main.rs @@ -400,7 +400,7 @@ impl MlsClient for MlsClientImpl { builder.with_external_psk(psk.psk_id.into(), psk.psk_secret.into()) }); - let (group, commit) = builder.build(group_info.clone()).map_err(abort)?; + let (group, commit) = builder.build().map_err(abort)?; let epoch_authenticator = group.epoch_authenticator().map_err(abort)?.to_vec(); diff --git a/mls-rs/tests/client_tests.rs b/mls-rs/tests/client_tests.rs index eeb920a3..d12fd65c 100644 --- a/mls-rs/tests/client_tests.rs +++ b/mls-rs/tests/client_tests.rs @@ -513,7 +513,6 @@ async fn external_commits_work( let (new_group, commit) = client .external_commit_builder(group_info) - .await .unwrap() .build() .await