WIP: Group Info On Commit

mls-rs/src/group/commit.rs

@@ -89,6 +89,10 @@ pub struct CommitOutput {
     /// `ratchet_tree_extension` is not used according to
     /// [`MlsRules::encryption_options`].
     pub ratchet_tree: Option<Vec<u8>>,
+    /// A group info that can be provided to new members in order to enable external commit
+    /// functionality. This value is set if [`MlsRules::commit_options`] returns
+    /// `allow_external_commit` set to true.
+    pub external_commit_group_info: Option<MlsMessage>,
 }
 
 #[cfg_attr(all(feature = "ffi", not(test)), ::safer_ffi_gen::safer_ffi_gen)]
@@ -381,7 +385,7 @@ where
         proposals: Vec<Proposal>,
         external_leaf: Option<&LeafNode>,
         authenticated_data: Vec<u8>,
-        group_info_extensions: ExtensionList,
+        mut welcome_group_info_extensions: ExtensionList,
         new_signer: Option<SignatureSecretKey>,
         new_signing_identity: Option<SigningIdentity>,
     ) -> Result<CommitOutput, MlsError> {
@@ -554,20 +558,6 @@ where
 
         provisional_group_context.confirmed_transcript_hash = confirmed_transcript_hash;
 
-        // Add the ratchet tree extension if necessary
-        let mut extensions = ExtensionList::new();
-
-        if commit_options.ratchet_tree_extension {
-            let ratchet_tree_ext = RatchetTreeExt {
-                tree_data: provisional_state.public_tree.export_node_data(),
-            };
-
-            extensions.set_from(ratchet_tree_ext)?;
-        }
-
-        // Add in any user provided extensions
-        extensions.append(group_info_extensions);
-
         let key_schedule_result = KeySchedule::from_key_schedule(
             &self.key_schedule,
             &commit_secret,
@@ -588,11 +578,55 @@ where
 
         auth_content.auth.confirmation_tag = Some(confirmation_tag.clone());
 
-        let group_info = self
+        let ratchet_tree_ext = commit_options
+            .ratchet_tree_extension
+            .then(|| RatchetTreeExt {
+                tree_data: provisional_state.public_tree.export_node_data(),
+            });
+
+        // Generate external commit group info if required by commit_options
+        let external_commit_group_info = match commit_options.allow_external_commit {
+            true => {
+                let mut extensions = ExtensionList::new();
+
+                extensions.set_from({
+                    self.key_schedule
+                        .get_external_key_pair_ext(&self.cipher_suite_provider)
+                        .await?
+                })?;
+
+                if let Some(ref ratchet_tree_ext) = ratchet_tree_ext {
+                    extensions.set_from(ratchet_tree_ext.clone())?;
+                }
+
+                let info = self
+                    .make_group_info(
+                        &provisional_group_context,
+                        extensions,
+                        &confirmation_tag,
+                        new_signer_ref,
+                    )
+                    .await?;
+
+                let msg =
+                    MlsMessage::new(self.protocol_version(), MlsMessagePayload::GroupInfo(info));
+
+                Some(msg)
+            }
+            false => None,
+        };
+
+        // Build the group info that will be placed into the welcome messages.
+        // Add the ratchet tree extension if necessary
+        if let Some(ratchet_tree_ext) = ratchet_tree_ext {
+            welcome_group_info_extensions.set_from(ratchet_tree_ext)?;
+        }
+
+        let welcome_group_info = self
             .make_group_info(
-                provisional_group_context,
-                extensions,
-                confirmation_tag,
+                &provisional_group_context,
+                welcome_group_info_extensions,
+                &confirmation_tag,
                 new_signer_ref,
             )
             .await?;
@@ -606,7 +640,9 @@ where
         )
         .await?;
 
-        let encrypted_group_info = welcome_secret.encrypt(&group_info).await?;
+        let encrypted_group_info = welcome_secret
+            .encrypt(&welcome_group_info.mls_encode_to_vec()?)
+            .await?;
 
         // Encrypt path secrets and joiner secret to new members
         let path_secrets = path_secrets.as_ref();
@@ -690,6 +726,7 @@ where
             commit_message,
             welcome_messages,
             ratchet_tree,
+            external_commit_group_info,
         })
     }
 
@@ -698,15 +735,15 @@ where
     #[cfg_attr(not(mls_build_async), maybe_async::must_be_sync)]
     async fn make_group_info(
         &self,
-        group_context: GroupContext,
+        group_context: &GroupContext,
         extensions: ExtensionList,
-        confirmation_tag: ConfirmationTag,
+        confirmation_tag: &ConfirmationTag,
         signer: &SignatureSecretKey,
-    ) -> Result<Vec<u8>, MlsError> {
+    ) -> Result<GroupInfo, MlsError> {
         let mut group_info = GroupInfo {
-            group_context,
+            group_context: group_context.clone(),
             extensions,
-            confirmation_tag, // The confirmation_tag from the MlsPlaintext object
+            confirmation_tag: confirmation_tag.clone(), // The confirmation_tag from the MlsPlaintext object
             signer: LeafIndex(self.current_member_index()),
             signature: vec![],
         };
@@ -718,7 +755,7 @@ where
             .sign(&self.cipher_suite_provider, signer, &())
             .await?;
 
-        group_info.mls_encode_to_vec().map_err(Into::into)
+        Ok(group_info)
     }
 
     fn make_welcome_message(
@@ -770,6 +807,7 @@ mod tests {
 
     use mls_rs_core::{
         error::IntoAnyError,
+        extension::ExtensionType,
         identity::{CredentialType, IdentityProvider},
         time::MlsTime,
     };
@@ -1244,6 +1282,79 @@ mod tests {
         assert!(commit.ratchet_tree.is_none());
     }
 
+    #[maybe_async::test(not(mls_build_async), async(mls_build_async, crate::futures_test))]
+    async fn commit_includes_external_commit_group_info_if_requested() {
+        let mut group = test_group_custom(
+            TEST_PROTOCOL_VERSION,
+            TEST_CIPHER_SUITE,
+            Default::default(),
+            None,
+            Some(
+                CommitOptions::new()
+                    .with_allow_external_commit(true)
+                    .with_ratchet_tree_extension(false),
+            ),
+        )
+        .await
+        .group;
+
+        let commit = group.commit(vec![]).await.unwrap();
+
+        let info = commit
+            .external_commit_group_info
+            .unwrap()
+            .into_group_info()
+            .unwrap();
+
+        assert!(!info.extensions.has_extension(ExtensionType::RATCHET_TREE));
+        assert!(info.extensions.has_extension(ExtensionType::EXTERNAL_PUB));
+    }
+
+    #[maybe_async::test(not(mls_build_async), async(mls_build_async, crate::futures_test))]
+    async fn commit_includes_external_commit_and_tree_if_requested() {
+        let mut group = test_group_custom(
+            TEST_PROTOCOL_VERSION,
+            TEST_CIPHER_SUITE,
+            Default::default(),
+            None,
+            Some(
+                CommitOptions::new()
+                    .with_allow_external_commit(true)
+                    .with_ratchet_tree_extension(true),
+            ),
+        )
+        .await
+        .group;
+
+        let commit = group.commit(vec![]).await.unwrap();
+
+        let info = commit
+            .external_commit_group_info
+            .unwrap()
+            .into_group_info()
+            .unwrap();
+
+        assert!(info.extensions.has_extension(ExtensionType::RATCHET_TREE));
+        assert!(info.extensions.has_extension(ExtensionType::EXTERNAL_PUB));
+    }
+
+    #[maybe_async::test(not(mls_build_async), async(mls_build_async, crate::futures_test))]
+    async fn commit_does_not_include_external_commit_group_info_if_not_requested() {
+        let mut group = test_group_custom(
+            TEST_PROTOCOL_VERSION,
+            TEST_CIPHER_SUITE,
+            Default::default(),
+            None,
+            Some(CommitOptions::new().with_allow_external_commit(false)),
+        )
+        .await
+        .group;
+
+        let commit = group.commit(vec![]).await.unwrap();
+
+        assert!(commit.external_commit_group_info.is_none());
+    }
+
     #[maybe_async::test(not(mls_build_async), async(mls_build_async, crate::futures_test))]
     async fn member_identity_is_validated_against_new_extensions() {
         let alice = client_with_test_extension(b"alice").await;

mls-rs/src/group/key_schedule.rs

@@ -3,6 +3,7 @@
 // SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
 use crate::client::MlsError;
+use crate::extension::ExternalPubExt;
 use crate::group::{GroupContext, MembershipTag};
 use crate::psk::secret::PskSecret;
 #[cfg(feature = "psk")]
@@ -239,6 +240,16 @@ impl KeySchedule {
             .await
             .map_err(|e| MlsError::CryptoProviderError(e.into_any_error()))
     }
+
+    #[cfg_attr(not(mls_build_async), maybe_async::must_be_sync)]
+    pub async fn get_external_key_pair_ext<P: CipherSuiteProvider>(
+        &self,
+        cipher_suite: &P,
+    ) -> Result<ExternalPubExt, MlsError> {
+        let (_external_secret, external_pub) = self.get_external_key_pair(cipher_suite).await?;
+
+        Ok(ExternalPubExt { external_pub })
+    }
 }
 
 #[derive(MlsEncode, MlsSize)]

mls-rs/src/group/mls_rules.rs

@@ -37,6 +37,7 @@ pub struct CommitOptions {
     pub path_required: bool,
     pub ratchet_tree_extension: bool,
     pub single_welcome_message: bool,
+    pub allow_external_commit: bool,
 }
 
 impl Default for CommitOptions {
@@ -45,6 +46,7 @@ impl Default for CommitOptions {
             path_required: false,
             ratchet_tree_extension: true,
             single_welcome_message: true,
+            allow_external_commit: false,
         }
     }
 }
@@ -74,6 +76,13 @@ impl CommitOptions {
             ..self
         }
     }
+
+    pub fn with_allow_external_commit(self, allow_external_commit: bool) -> Self {
+        Self {
+            allow_external_commit,
+            ..self
+        }
+    }
 }
 
 /// Options controlling encryption of control and application messages

mls-rs/src/group/mod.rs

@@ -1347,12 +1347,9 @@ where
         let mut extensions = ExtensionList::new();
 
         extensions.set_from({
-            let (_external_secret, external_pub) = self
-                .key_schedule
-                .get_external_key_pair(&self.cipher_suite_provider)
-                .await?;
-
-            ExternalPubExt { external_pub }
+            self.key_schedule
+                .get_external_key_pair_ext(&self.cipher_suite_provider)
+                .await?
         })?;
 
         self.group_info_message_internal(extensions, with_tree_in_extension)
@@ -2393,6 +2390,30 @@ mod tests {
         assert_matches!(res, Err(MlsError::MissingExternalPubExtension));
     }
 
+    #[maybe_async::test(not(mls_build_async), async(mls_build_async, crate::futures_test))]
+    async fn external_commit_via_commit_options_round_trip() {
+        let mut group = test_group_custom(
+            TEST_PROTOCOL_VERSION,
+            TEST_CIPHER_SUITE,
+            vec![],
+            None,
+            CommitOptions::default()
+                .with_allow_external_commit(true)
+                .into(),
+        );
+
+        let commit_output = group.group.commit(vec![]).unwrap();
+
+        let (test_client, _) =
+            test_client_with_key_pkg(TEST_PROTOCOL_VERSION, TEST_CIPHER_SUITE, "bob").await;
+
+        test_client
+            .external_commit_builder()
+            .unwrap()
+            .build(commit_output.external_commit_group_info.unwrap())
+            .unwrap();
+    }
+
     #[maybe_async::test(not(mls_build_async), async(mls_build_async, crate::futures_test))]
     async fn test_path_update_preference() {
         let protocol_version = TEST_PROTOCOL_VERSION;

mls-rs/src/group/state_repo.rs

@@ -325,7 +325,10 @@ mod tests {
         assert_eq!(test_repo.pending_commit.updates.len(), 1);
         assert!(test_repo.pending_commit.inserts.is_empty());
 
-        assert_eq!(test_repo.pending_commit.updates.get(0).unwrap(), &to_update);
+        assert_eq!(
+            test_repo.pending_commit.updates.first().unwrap(),
+            &to_update
+        );
 
         // Make sure you can access an epoch pending update
         let psk_id = ResumptionPsk {

mls-rs/src/group/test_utils.rs

@@ -83,6 +83,7 @@ impl TestGroup {
             mut welcome_messages,
             ratchet_tree,
             commit_message,
+            ..
         } = self
             .group
             .commit_builder()

mls-rs/src/tree_kem/mod.rs

@@ -1269,12 +1269,8 @@ mod tests {
         let original_leaf_count = tree.occupied_leaf_count();
 
         // Remove two leaves from the tree
-        let expected_result: Vec<(LeafIndex, LeafNode)> = indexes
-            .clone()
-            .into_iter()
-            .zip(key_packages)
-            .map(|(index, ln)| (index, ln))
-            .collect();
+        let expected_result: Vec<(LeafIndex, LeafNode)> =
+            indexes.clone().into_iter().zip(key_packages).collect();
 
         let res = tree
             .remove_leaves(

mls-rs/src/tree_kem/private.rs

@@ -213,7 +213,7 @@ mod tests {
         let (public_tree, mut charlie_private, alice_private, path_secret) =
             update_secrets_setup(cipher_suite).await;
 
-        let existing_private = charlie_private.secret_keys.get(0).cloned().unwrap();
+        let existing_private = charlie_private.secret_keys.first().cloned().unwrap();
 
         // Add the secrets for Charlie to his private key
         charlie_private
@@ -295,6 +295,6 @@ mod tests {
         assert!(private_key.secret_keys.iter().skip(1).all(|n| n.is_none()));
 
         // The secret key for our leaf should have been updated accordingly
-        assert_eq!(private_key.secret_keys.get(0).unwrap(), &Some(new_secret));
+        assert_eq!(private_key.secret_keys.first().unwrap(), &Some(new_secret));
     }
 }