diff --git a/openmls/src/group/core_group/mod.rs b/openmls/src/group/core_group/mod.rs index 9e19b62833..9c8ff5ef06 100644 --- a/openmls/src/group/core_group/mod.rs +++ b/openmls/src/group/core_group/mod.rs @@ -243,7 +243,7 @@ impl CoreGroupBuilder { /// [`OpenMlsCryptoProvider`]. pub(crate) async fn build( self, - backend: &impl OpenMlsCryptoProvider, + backend: &impl OpenMlsCryptoProvider, signer: &impl Signer, ) -> Result> { let (public_group_builder, commit_secret, leaf_keypair) = @@ -270,7 +270,7 @@ impl CoreGroupBuilder { .map_err(LibraryError::unexpected_crypto_error)?, &serialized_group_context, ) - .map_err(LibraryError::unexpected_crypto_error)?; + .map_err(LibraryError::unexpected_crypto_error)?; // TODO(#1357) let resumption_psk_store = ResumptionPskStore::new(32); @@ -407,7 +407,7 @@ impl CoreGroup { self.context(), signer, ) - .map_err(ValidationError::LibraryError) + .map_err(ValidationError::LibraryError) } // 11.1.4. PreSharedKey @@ -437,7 +437,7 @@ impl CoreGroup { pub(crate) fn members_support_extensions<'a>( &self, extensions: &Extensions, - pending_proposals: impl Iterator, + pending_proposals: impl Iterator, ) -> Result<(), MemberExtensionValidationError> { let required_extension = extensions .iter() @@ -470,7 +470,7 @@ impl CoreGroup { &self, framing_parameters: FramingParameters, extensions: Extensions, - pending_proposals: impl Iterator, + pending_proposals: impl Iterator, signer: &impl Signer, ) -> Result { // Ensure that the group supports all the extensions that are wanted. @@ -485,7 +485,7 @@ impl CoreGroup { self.context(), signer, ) - .map_err(|e| e.into()) + .map_err(|e| e.into()) } /// Create a `ReInit` proposal pub(crate) fn create_reinit_proposal( @@ -512,7 +512,7 @@ impl CoreGroup { self.context(), signer, ) - .map_err(|e| e.into()) + .map_err(|e| e.into()) } // Create application message pub(crate) fn create_application_message( @@ -815,7 +815,7 @@ impl CoreGroup { /// Returns an error if access to the key store fails. pub(super) async fn store_epoch_keypairs( &self, - backend: &impl OpenMlsCryptoProvider, + backend: &impl OpenMlsCryptoProvider, epoch_encryption_keypair: EpochEncryptionKeyPair, ) -> Result<(), KeyStore::Error> { let k = EpochKeypairId::new( @@ -835,7 +835,7 @@ impl CoreGroup { /// Returns `None` if access to the key store fails. pub(super) async fn read_epoch_keypairs( &self, - backend: &impl OpenMlsCryptoProvider, + backend: &impl OpenMlsCryptoProvider, ) -> EpochEncryptionKeyPair { let k = EpochKeypairId::new( self.group_id(), @@ -855,7 +855,7 @@ impl CoreGroup { /// Returns an error if access to the key store fails. pub(super) async fn delete_previous_epoch_keypairs( &self, - backend: &impl OpenMlsCryptoProvider, + backend: &impl OpenMlsCryptoProvider, ) -> Result<(), KeyStore::Error> { let k = EpochKeypairId::new( self.group_id(), @@ -871,7 +871,7 @@ impl CoreGroup { pub(crate) async fn create_commit( &self, mut params: CreateCommitParams<'_>, - backend: &impl OpenMlsCryptoProvider, + backend: &impl OpenMlsCryptoProvider, signer: &impl Signer, ) -> Result> { let ciphersuite = self.ciphersuite(); @@ -890,15 +890,15 @@ impl CoreGroup { params.inline_proposals(), self.own_leaf_index(), ) - .map_err(|e| match e { - crate::group::errors::ProposalQueueError::LibraryError(e) => e.into(), - crate::group::errors::ProposalQueueError::ProposalNotFound => { - CreateCommitError::MissingProposal - } - crate::group::errors::ProposalQueueError::SenderError(_) => { - CreateCommitError::WrongProposalSenderType - } - })?; + .map_err(|e| match e { + crate::group::errors::ProposalQueueError::LibraryError(e) => e.into(), + crate::group::errors::ProposalQueueError::ProposalNotFound => { + CreateCommitError::MissingProposal + } + crate::group::errors::ProposalQueueError::SenderError(_) => { + CreateCommitError::WrongProposalSenderType + } + })?; // TODO: #581 Filter proposals by support // 11.2: @@ -968,7 +968,7 @@ impl CoreGroup { all_proposals.find_map(|p| { match p { - Proposal::Update(UpdateProposal{leaf_node}) => Some(leaf_node.clone()), + Proposal::Update(UpdateProposal { leaf_node }) => Some(leaf_node.clone()), _ => None, } }) @@ -985,7 +985,7 @@ impl CoreGroup { params.commit_type(), signer, params.take_credential_with_key(), - apply_proposals_values.extensions + apply_proposals_values.extensions, )? } else { // If path is not needed, update the group context and return @@ -994,6 +994,11 @@ impl CoreGroup { PathComputationResult::default() }; + let update_path_leaf_node = path_computation_result + .encrypted_path + .as_ref() + .map(|path| path.leaf_node().clone()); + // Create commit message let commit = Commit { proposals: proposal_reference_list, @@ -1023,7 +1028,7 @@ impl CoreGroup { self.group_epoch_secrets().init_secret(), &serialized_provisional_group_context, ) - .map_err(LibraryError::unexpected_crypto_error)?; + .map_err(LibraryError::unexpected_crypto_error)?; // Prepare the PskSecret let psk_secret = { @@ -1032,7 +1037,7 @@ impl CoreGroup { &self.resumption_psk_store, &apply_proposals_values.presharedkeys, ) - .await?; + .await?; PskSecret::new(backend, ciphersuite, psks).await? }; @@ -1154,6 +1159,7 @@ impl CoreGroup { // The committer is not allowed to include their own update // proposal, so there is no extra keypair to store here. None, + update_path_leaf_node, ); let staged_commit_state = match params.commit_type() { CommitType::Member => StagedCommitState::GroupMember(Box::new(staged_commit_state)), @@ -1185,7 +1191,7 @@ impl MlsGroup { /// re-export pub async fn delete_previous_epoch_keypairs( &self, - backend: &impl OpenMlsCryptoProvider, + backend: &impl OpenMlsCryptoProvider, ) -> Result<(), KeyStore::Error> { self.group.delete_previous_epoch_keypairs(backend).await } diff --git a/openmls/src/group/core_group/staged_commit.rs b/openmls/src/group/core_group/staged_commit.rs index ba658a227b..8576d84d4c 100644 --- a/openmls/src/group/core_group/staged_commit.rs +++ b/openmls/src/group/core_group/staged_commit.rs @@ -43,7 +43,7 @@ impl CoreGroup { &init_secret, serialized_provisional_group_context, ) - .map_err(LibraryError::unexpected_crypto_error)? + .map_err(LibraryError::unexpected_crypto_error)? } else { JoinerSecret::new( backend, @@ -51,7 +51,7 @@ impl CoreGroup { epoch_secrets.init_secret(), serialized_provisional_group_context, ) - .map_err(LibraryError::unexpected_crypto_error)? + .map_err(LibraryError::unexpected_crypto_error)? }; // Prepare the PskSecret @@ -61,7 +61,7 @@ impl CoreGroup { &self.resumption_psk_store, &apply_proposals_values.presharedkeys, ) - .await?; + .await?; PskSecret::new(backend, self.ciphersuite(), psks).await? }; @@ -151,7 +151,7 @@ impl CoreGroup { } // Determine if Commit has a path - let (commit_secret, new_keypairs, new_leaf_keypair_option) = + let (commit_secret, new_keypairs, new_leaf_keypair_option, update_path_leaf_node) = if let Some(path) = commit.path.clone() { // Update the public group // ValSem202: Path must be the right length @@ -194,7 +194,14 @@ impl CoreGroup { debug_assert!(false); None }; - (commit_secret, new_keypairs, new_leaf_keypair_option) + + // Return the leaf node in the update path so the credential can be validated. + // Since the diff has already been updated, this should be the same as the leaf + // at the sender index. + let update_path_leaf_node = Some(path.leaf_node().clone()); + debug_assert_eq!(diff.leaf(sender_index), path.leaf_node().into()); + + (commit_secret, new_keypairs, new_leaf_keypair_option, update_path_leaf_node) } else { if apply_proposals_values.path_required { // ValSem201 @@ -208,6 +215,7 @@ impl CoreGroup { CommitSecret::zero_secret(ciphersuite, self.version()), vec![], None, + None, ) }; @@ -264,6 +272,7 @@ impl CoreGroup { staged_diff, new_keypairs, new_leaf_keypair_option, + update_path_leaf_node, ))); Ok(StagedCommit::new(proposal_queue, staged_commit_state)) @@ -276,7 +285,7 @@ impl CoreGroup { /// might throw a `LibraryError`. pub(crate) async fn merge_commit( &mut self, - backend: &impl OpenMlsCryptoProvider, + backend: &impl OpenMlsCryptoProvider, staged_commit: StagedCommit, ) -> Result, MergeCommitError> { // Get all keypairs from the old epoch, so we can later store the ones @@ -300,7 +309,7 @@ impl CoreGroup { async fn merge_member_commit( &mut self, - backend: &impl OpenMlsCryptoProvider, + backend: &impl OpenMlsCryptoProvider, mut old_epoch_keypairs: EpochEncryptionKeyPair, mut state: Box, is_member: bool, @@ -343,7 +352,7 @@ impl CoreGroup { return Err(LibraryError::custom( "We should have all the private key material we need.", ) - .into()); + .into()); } // Store the relevant keys under the new epoch @@ -390,7 +399,7 @@ impl CoreGroup { leaf_node_keypairs, backend, ) - .await + .await } } @@ -419,27 +428,27 @@ impl StagedCommit { } /// Returns the Add proposals that are covered by the Commit message as in iterator over [QueuedAddProposal]. - pub fn add_proposals(&self) -> impl Iterator { + pub fn add_proposals(&self) -> impl Iterator { self.staged_proposal_queue.add_proposals() } /// Returns the Remove proposals that are covered by the Commit message as in iterator over [QueuedRemoveProposal]. - pub fn remove_proposals(&self) -> impl Iterator { + pub fn remove_proposals(&self) -> impl Iterator { self.staged_proposal_queue.remove_proposals() } /// Returns the Update proposals that are covered by the Commit message as in iterator over [QueuedUpdateProposal]. - pub fn update_proposals(&self) -> impl Iterator { + pub fn update_proposals(&self) -> impl Iterator { self.staged_proposal_queue.update_proposals() } /// Returns the PresharedKey proposals that are covered by the Commit message as in iterator over [QueuedPskProposal]. - pub fn psk_proposals(&self) -> impl Iterator { + pub fn psk_proposals(&self) -> impl Iterator { self.staged_proposal_queue.psk_proposals() } /// Returns an interator over all [`QueuedProposal`]s - pub fn queued_proposals(&self) -> impl Iterator { + pub fn queued_proposals(&self) -> impl Iterator { self.staged_proposal_queue.queued_proposals() } @@ -469,6 +478,17 @@ impl StagedCommit { StagedCommitState::ExternalMember(diff) => &diff.staged_diff.confirmation_tag, } } + + + /// Returns the leaf node of the (optional) update path. + pub fn get_update_path_leaf_node(&self) -> Option<&LeafNode> { + match self.state { + StagedCommitState::PublicState(_) => None, + StagedCommitState::GroupMember(ref member) | StagedCommitState::ExternalMember(ref member) => { + member.update_path_leaf_node.as_ref() + } + } + } } /// This struct is used internally by [StagedCommit] to encapsulate all the modified group state. @@ -479,6 +499,7 @@ pub(crate) struct MemberStagedCommitState { staged_diff: StagedPublicGroupDiff, new_keypairs: Vec, maybe_new_leaf_keypair: Option, + update_path_leaf_node: Option, } impl MemberStagedCommitState { @@ -488,6 +509,7 @@ impl MemberStagedCommitState { staged_diff: StagedPublicGroupDiff, new_keypairs: Vec, maybe_new_leaf_keypair: Option, + update_path_leaf_node: Option, ) -> Self { Self { group_epoch_secrets, @@ -495,6 +517,7 @@ impl MemberStagedCommitState { staged_diff, new_keypairs, maybe_new_leaf_keypair, + update_path_leaf_node, } }