Merge pull request #16 from openmls/keks/xmtp-merge-upstream

Merge upstream changes

CHANGELOG.md

@@ -10,6 +10,11 @@ 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
+- [#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)
 

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
 

book/src/user_manual/group_config.md

@@ -17,8 +17,9 @@ 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                                            |
 
 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.
 
@@ -33,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.

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,
 }

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);

openmls/src/framing/validation.rs

@@ -55,6 +55,7 @@ use super::{
 ///  - ValSem005
 ///  - ValSem007
 ///  - ValSem009
+#[derive(Debug)]
 pub(crate) struct DecryptedMessage {
     verifiable_content: VerifiableAuthenticatedContentIn,
 }

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},
 };
@@ -179,26 +180,10 @@ 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 [`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);
-        }
+
+    /// 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
     }
 
@@ -217,6 +202,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, InvalidExtensionError> {
+        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;
@@ -452,10 +448,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 +885,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 +915,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()
             };
 

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))
     }

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()),

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,
 };
 
@@ -299,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.",
@@ -326,16 +328,9 @@ fn test_required_extension_key_package_mismatch(
     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 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);
@@ -345,7 +340,10 @@ fn test_required_extension_key_package_mismatch(
         CryptoConfig::with_default_version(ciphersuite),
         alice_credential,
     )
-    .with_required_capabilities(required_capabilities)
+    .with_group_context_extensions(Extensions::single(Extension::RequiredCapabilities(
+        required_capabilities,
+    )))
+    .expect("error adding group context extensions")
     .build(provider, &alice_signer)
     .expect("Error creating CoreGroup.");
 
@@ -358,7 +356,9 @@ fn test_required_extension_key_package_mismatch(
         .expect_err("Proposal was created even though the key package didn't support the required extensions.");
     assert_eq!(
         e,
-        CreateAddProposalError::LeafNodeValidation(LeafNodeValidationError::UnsupportedExtensions)
+        CreateAddProposalError::LeafNodeValidation(
+            crate::treesync::errors::LeafNodeValidationError::UnsupportedExtensions
+        )
     );
 }
 
@@ -392,7 +392,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.");
 
@@ -470,32 +473,13 @@ 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.");
 
-    // 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)

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<KeyStoreError> {
     /// 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<KeyStoreError> {
     #[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,
+}