Add MlsGroup endpoint to create Add commit without path (openmls#1629)

CHANGELOG.md

@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+- [#1629](https://github.com/openmls/openmls/pull/1629): Add `add_members_without_update` function to `MlsGroup` to allow the creation of add-only commits
 - [#1506](https://github.com/openmls/openmls/pull/1506): Add `StagedWelcome` and `StagedCoreWelcome` to make joining a group staged in order to inspect the `Welcome` message. This was followed up with PR [#1533](https://github.com/openmls/openmls/pull/1533) to adjust the API.
 - [#1516](https://github.com/openmls/openmls/pull/1516): Add `MlsGroup::clear_pending_proposals` to the public API; this allows users to clear a group's internal `ProposalStore`
 - [#1565](https://github.com/openmls/openmls/pull/1565): Add new `StorageProvider` trait to the `openmls_traits` crate.

book/src/user_manual/add_members.md

@@ -10,6 +10,12 @@ Members can be added to the group atomically with the `.add_members()` function.
 
 The function returns the tuple `(MlsMessageOut, Welcome)`. The `MlsMessageOut` contains a Commit message that needs to be fanned out to existing group members. The `Welcome` message must be sent to the newly added members.
 
+### Adding members without update
+
+The `.add_members_without_update()` function functions the same as the `.add_members()` function, except that it will only include an update to the sender's key material if the sender's proposal store includes a proposal that requires a path. For a list of proposals and an indication whether they require a `path` (i.e. a key material update) see [Section 17.4 of RFC 9420](https://www.rfc-editor.org/rfc/rfc9420.html#section-17.4).
+
+Not sending an update means that the sender will not achieve post-compromise security with this particular commit. However, not sending an update saves on performance both in terms of computation and bandwidth. Using `.add_members_without_update()` can thus be a useful option if the ciphersuite of the group features large public keys and/or expensive encryption operations.
+
 ## Proposal
 
 Members can also be added as a proposal (without the corresponding Commit message) by using the `.propose_add_member()` function:

openmls/src/group/core_group/create_commit_params.rs

@@ -53,7 +53,6 @@ impl<'a> CreateCommitParamsBuilder<'a> {
         self.ccp.inline_proposals = inline_proposals;
         self
     }
-    #[cfg(test)]
     pub(crate) fn force_self_update(mut self, force_self_update: bool) -> Self {
         self.ccp.force_self_update = force_self_update;
         self

openmls/src/group/mls_group/membership.rs

@@ -20,7 +20,9 @@ impl MlsGroup {
     /// New members are added by providing a `KeyPackage` for each member.
     ///
     /// This operation results in a Commit with a `path`, i.e. it includes an
-    /// update of the committer's leaf [KeyPackage].
+    /// update of the committer's leaf [KeyPackage]. To add members without
+    /// forcing an update of the committer's leaf [KeyPackage], use
+    /// [`Self::add_members_without_update()`].
     ///
     /// If successful, it returns a triple of [`MlsMessageOut`]s, where the first
     /// contains the commit, the second one the [`Welcome`] and the third an optional [GroupInfo] that
@@ -39,6 +41,53 @@ impl MlsGroup {
     ) -> Result<
         (MlsMessageOut, MlsMessageOut, Option<GroupInfo>),
         AddMembersError<Provider::StorageError>,
+    > {
+        self.add_members_internal(provider, signer, key_packages, true)
+    }
+
+    /// Adds members to the group.
+    ///
+    /// New members are added by providing a `KeyPackage` for each member.
+    ///
+    /// This operation results in a Commit that does not necessarily include a
+    /// `path`, i.e. an update of the committer's leaf [KeyPackage]. In
+    /// particular, it will only include a path if the group's proposal store
+    /// includes one or more proposals that require a path (see [Section 17.4 of
+    /// RFC 9420](https://www.rfc-editor.org/rfc/rfc9420.html#section-17.4) for
+    /// a list of proposals and whether they require a path).
+    ///
+    /// If successful, it returns a triple of [`MlsMessageOut`]s, where the
+    /// first contains the commit, the second one the [`Welcome`] and the third
+    /// an optional [GroupInfo] that will be [Some] if the group has the
+    /// `use_ratchet_tree_extension` flag set.
+    ///
+    /// Returns an error if there is a pending commit.
+    ///
+    /// [`Welcome`]: crate::messages::Welcome
+    // FIXME: #1217
+    #[allow(clippy::type_complexity)]
+    pub fn add_members_without_update<Provider: OpenMlsProvider>(
+        &mut self,
+        provider: &Provider,
+        signer: &impl Signer,
+        key_packages: &[KeyPackage],
+    ) -> Result<
+        (MlsMessageOut, MlsMessageOut, Option<GroupInfo>),
+        AddMembersError<Provider::StorageError>,
+    > {
+        self.add_members_internal(provider, signer, key_packages, false)
+    }
+
+    #[allow(clippy::type_complexity)]
+    fn add_members_internal<Provider: OpenMlsProvider>(
+        &mut self,
+        provider: &Provider,
+        signer: &impl Signer,
+        key_packages: &[KeyPackage],
+        force_self_update: bool,
+    ) -> Result<
+        (MlsMessageOut, MlsMessageOut, Option<GroupInfo>),
+        AddMembersError<Provider::StorageError>,
     > {
         self.is_operational()?;
 
@@ -61,6 +110,7 @@ impl MlsGroup {
         let params = CreateCommitParams::builder()
             .framing_parameters(self.framing_parameters())
             .inline_proposals(inline_proposals)
+            .force_self_update(force_self_update)
             .build();
         let create_commit_result = self.group.create_commit(params, provider, signer)?;
 

openmls/src/group/tests_and_kats/tests/group.rs

@@ -1,198 +1,96 @@
-use crate::{
-    framing::*,
-    group::{tests_and_kats::utils::generate_key_package, *},
-    schedule::psk::store::ResumptionPskStore,
-    test_utils::*,
-    *,
-};
-use group::tests_and_kats::utils::generate_credential_with_key;
-use mls_group::tests_and_kats::utils::{setup_alice_bob_group, setup_client};
+use crate::{framing::*, group::*, test_utils::*, *};
+use mls_group::tests_and_kats::utils::{setup_alice_bob, setup_alice_bob_group, setup_client};
 use treesync::LeafNodeParameters;
 
 #[openmls_test::openmls_test]
 fn create_commit_optional_path(
     ciphersuite: Ciphersuite,
     provider: &impl crate::storage::OpenMlsProvider,
 ) {
-    let group_aad = b"Alice's test group";
-    // Framing parameters
-    let framing_parameters = FramingParameters::new(group_aad, WireFormat::PublicMessage);
-
     // Define identities
-    let alice_credential_with_keys = generate_credential_with_key(
-        b"Alice".to_vec(),
-        ciphersuite.signature_algorithm(),
-        provider,
-    );
-    let bob_credential_with_keys =
-        generate_credential_with_key(b"Bob".to_vec(), ciphersuite.signature_algorithm(), provider);
-
-    // Generate Bob's KeyPackage
-    let bob_key_package = generate_key_package(
-        ciphersuite,
-        Extensions::empty(),
-        provider,
-        bob_credential_with_keys,
-    );
+    let (alice_credential_with_key, alice_signer, bob_kpb, _bob_signer) =
+        setup_alice_bob(ciphersuite, provider);
 
     // Alice creates a group
-    let mut group_alice = CoreGroup::builder(
-        GroupId::random(provider.rand()),
-        ciphersuite,
-        alice_credential_with_keys.credential_with_key,
-    )
-    .build(provider, &alice_credential_with_keys.signer)
-    .expect("Error creating CoreGroup.");
+    let mut alice_group = MlsGroup::builder()
+        .ciphersuite(ciphersuite)
+        .with_wire_format_policy(PURE_PLAINTEXT_WIRE_FORMAT_POLICY)
+        .build(provider, &alice_signer, alice_credential_with_key)
+        .unwrap();
 
     // Alice proposes to add Bob with forced self-update
     // Even though there are only Add Proposals, this should generated a path field
     // on the Commit
-    let bob_add_proposal = group_alice
-        .create_add_proposal(
-            framing_parameters,
-            bob_key_package.key_package().clone(),
-            &alice_credential_with_keys.signer,
-        )
-        .expect("Could not create proposal.");
-
-    group_alice.proposal_store_mut().empty();
-    group_alice.proposal_store_mut().add(
-        QueuedProposal::from_authenticated_content_by_ref(
-            ciphersuite,
-            provider.crypto(),
-            bob_add_proposal,
-        )
-        .unwrap(),
-    );
+    let (commit_message, _welcome, _) = alice_group
+        .add_members(provider, &alice_signer, &[bob_kpb.key_package().clone()])
+        .unwrap();
 
-    let params = CreateCommitParams::builder()
-        .framing_parameters(framing_parameters)
-        .build();
-    let create_commit_result = match group_alice.create_commit(
-        params, /* No PSK fetcher */
-        provider,
-        &alice_credential_with_keys.signer,
-    ) {
-        Ok(c) => c,
-        Err(e) => panic!("Error creating commit: {e:?}"),
-    };
-    let commit = match create_commit_result.commit.content() {
-        FramedContentBody::Commit(commit) => commit,
+    let commit = match commit_message.body() {
+        MlsMessageBodyOut::PublicMessage(pm) => match pm.content() {
+            FramedContentBody::Commit(commit) => commit,
+            _ => panic!(),
+        },
         _ => panic!(),
     };
+
     assert!(commit.has_path());
 
+    alice_group
+        .clear_pending_commit(provider.storage())
+        .unwrap();
+
     // Alice adds Bob without forced self-update
-    // Since there are only Add Proposals, this does not generate a path field on
-    // the Commit Creating a second proposal to add the same member should
-    // not fail, only committing that proposal should fail
-    let bob_add_proposal = group_alice
-        .create_add_proposal(
-            framing_parameters,
-            bob_key_package.key_package().clone(),
-            &alice_credential_with_keys.signer,
-        )
-        .expect("Could not create proposal.");
-
-    group_alice.proposal_store_mut().empty();
-    group_alice.proposal_store_mut().add(
-        QueuedProposal::from_authenticated_content_by_ref(
-            ciphersuite,
-            provider.crypto(),
-            bob_add_proposal,
-        )
-        .unwrap(),
-    );
+    let (commit_message, welcome, _) = alice_group
+        .add_members_without_update(provider, &alice_signer, &[bob_kpb.key_package().clone()])
+        .unwrap();
 
-    let params = CreateCommitParams::builder()
-        .framing_parameters(framing_parameters)
-        .force_self_update(false)
-        .build();
-    let create_commit_result =
-        match group_alice.create_commit(params, provider, &alice_credential_with_keys.signer) {
-            Ok(c) => c,
-            Err(e) => panic!("Error creating commit: {e:?}"),
-        };
-    let commit = match create_commit_result.commit.content() {
-        FramedContentBody::Commit(commit) => commit,
+    let commit = match commit_message.body() {
+        MlsMessageBodyOut::PublicMessage(pm) => match pm.content() {
+            FramedContentBody::Commit(commit) => commit,
+            _ => panic!(),
+        },
         _ => panic!(),
     };
+
     assert!(!commit.has_path());
 
     // Alice applies the Commit without the forced self-update
-    group_alice
-        .merge_commit(provider, create_commit_result.staged_commit)
-        .expect("error merging pending commit");
-    let ratchet_tree = group_alice.public_group().export_ratchet_tree();
+    alice_group.merge_pending_commit(provider).unwrap();
+    let ratchet_tree = alice_group.export_ratchet_tree();
 
     // Bob creates group from Welcome
-    let group_bob = StagedCoreWelcome::new_from_welcome(
-        create_commit_result
-            .welcome_option
-            .expect("An unexpected error occurred."),
-        Some(ratchet_tree.into()),
-        bob_key_package,
+    let bob_group = StagedWelcome::new_from_welcome(
         provider,
-        ResumptionPskStore::new(1024),
+        &MlsGroupJoinConfig::default(),
+        welcome.into_welcome().unwrap(),
+        Some(ratchet_tree.into()),
     )
-    .and_then(|staged_join| staged_join.into_core_group(provider))
-    .unwrap_or_else(|e| panic!("Error creating group from Welcome: {e:?}"));
+    .unwrap()
+    .into_group(provider)
+    .unwrap();
 
     assert_eq!(
-        group_alice.public_group().export_ratchet_tree(),
-        group_bob.public_group().export_ratchet_tree()
+        alice_group.export_ratchet_tree(),
+        bob_group.export_ratchet_tree()
     );
 
     // Alice updates
-    let mut alice_new_leaf_node = group_alice.own_leaf_node().unwrap().clone();
-    alice_new_leaf_node
-        .update(
-            ciphersuite,
-            provider,
-            &alice_credential_with_keys.signer,
-            group_alice.group_id().clone(),
-            group_alice.own_leaf_index(),
-            LeafNodeParameters::default(),
-        )
+    let (commit_message, _, _) = alice_group
+        .self_update(provider, &alice_signer, LeafNodeParameters::default())
         .unwrap();
-    let alice_update_proposal = group_alice
-        .create_update_proposal(
-            framing_parameters,
-            alice_new_leaf_node,
-            &alice_credential_with_keys.signer,
-        )
-        .expect("Could not create proposal.");
-
-    group_alice.proposal_store_mut().empty();
-    group_alice.proposal_store_mut().add(
-        QueuedProposal::from_authenticated_content_by_ref(
-            ciphersuite,
-            provider.crypto(),
-            alice_update_proposal,
-        )
-        .unwrap(),
-    );
 
-    // Only UpdateProposal
-    let params = CreateCommitParams::builder()
-        .framing_parameters(framing_parameters)
-        .force_self_update(false)
-        .build();
-    let create_commit_result =
-        match group_alice.create_commit(params, provider, &alice_credential_with_keys.signer) {
-            Ok(c) => c,
-            Err(e) => panic!("Error creating commit: {e:?}"),
-        };
-    let commit = match create_commit_result.commit.content() {
-        FramedContentBody::Commit(commit) => commit,
+    let commit = match commit_message.body() {
+        MlsMessageBodyOut::PublicMessage(pm) => match pm.content() {
+            FramedContentBody::Commit(commit) => commit,
+            _ => panic!(),
+        },
         _ => panic!(),
     };
+
     assert!(commit.has_path());
 
     // Apply UpdateProposal
-    group_alice
-        .merge_commit(provider, create_commit_result.staged_commit)
-        .expect("error merging pending commit");
+    alice_group.merge_pending_commit(provider).unwrap();
 }
 
 #[openmls_test::openmls_test]