Merge remote-tracking branch 'upstream/main' into np/pull-upstream

.github/workflows/interop.yml

@@ -66,14 +66,16 @@ jobs:
         run: |
           git clone https://github.com/mlswg/mls-implementations.git
           cd mls-implementations
-          git checkout f07090a844ebece12c064ce94ab853fd477db12f
+          git checkout 8a6ee96bc732abca77d872babf1830ccfec7fa49
 
       - name: test-runner | Install dependencies
         run: |
           sudo apt-get -y install protoc-gen-go
           echo $(go env GOPATH)/bin >> $GITHUB_PATH
           go install google.golang.org/protobuf/cmd/protoc-gen-go@latest
           go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@latest
+          cd mls-implementations
+          go get -u google.golang.org/grpc
 
       - name: test-runner | Build
         run: |
@@ -84,7 +86,6 @@ jobs:
           make run-go || echo "Build despite errors."
           cd test-runner
           # TODO(#1366)
-          go get -u google.golang.org/grpc
           go mod tidy -e
           patch main.go main.go.patch
           go build

memory_storage/src/lib.rs

@@ -867,14 +867,6 @@ impl StorageProvider<CURRENT_VERSION> for MemoryStorage {
         self.append::<CURRENT_VERSION>(OWN_LEAF_NODES_LABEL, &key, value)
     }
 
-    fn clear_own_leaf_nodes<GroupId: traits::GroupId<CURRENT_VERSION>>(
-        &self,
-        group_id: &GroupId,
-    ) -> Result<(), Self::Error> {
-        let key = serde_json::to_vec(group_id)?;
-        self.delete::<CURRENT_VERSION>(OWN_LEAF_NODES_LABEL, &key)
-    }
-
     fn aad<GroupId: traits::GroupId<CURRENT_VERSION>>(
         &self,
         group_id: &GroupId,

memory_storage/src/test_store.rs

@@ -487,13 +487,6 @@ impl StorageProvider<V_TEST> for MemoryStorage {
         todo!()
     }
 
-    fn clear_own_leaf_nodes<GroupId: traits::GroupId<V_TEST>>(
-        &self,
-        _group_id: &GroupId,
-    ) -> Result<(), Self::Error> {
-        todo!()
-    }
-
     fn aad<GroupId: traits::GroupId<V_TEST>>(
         &self,
         _group_id: &GroupId,

openmls/Cargo.toml

@@ -36,6 +36,7 @@ openmls_memory_storage = { path = "../memory_storage", features = [
 ], optional = true }
 openmls_test = { path = "../openmls_test", optional = true }
 openmls_libcrux_crypto = { path = "../libcrux_crypto", optional = true }
+once_cell = { version = "1.19.0", optional = true }
 
 [features]
 default = ["backtrace"]
@@ -49,6 +50,7 @@ test-utils = [
     "dep:openmls_basic_credential",
     "dep:openmls_memory_storage",
     "dep:openmls_test",
+    "dep:once_cell",
 ]
 libcrux-provider = [
     "dep:openmls_libcrux_crypto",

openmls/src/framing/mls_auth_content.rs

@@ -51,7 +51,7 @@ pub(crate) struct FramedContentAuthData {
 }
 
 impl FramedContentAuthData {
-    pub(super) fn deserialize<R: Read>(
+    pub(crate) fn deserialize<R: Read>(
         bytes: &mut R,
         content_type: ContentType,
     ) -> Result<Self, tls_codec::Error> {

openmls/src/group/core_group/new_from_welcome.rs

@@ -183,7 +183,12 @@ pub(in crate::group) fn build_staged_welcome<Provider: OpenMlsProvider>(
         log_crypto!(trace, "  Got:      {:x?}", confirmation_tag);
         log_crypto!(trace, "  Expected: {:x?}", public_group.confirmation_tag());
         debug_assert!(false, "Confirmation tag mismatch");
-        return Err(WelcomeError::ConfirmationTagMismatch);
+
+        // in some tests we need to be able to proceed despite the tag being wrong,
+        // e.g. to test whether a later validation check is performed correctly.
+        if !crate::skip_validation::is_disabled::confirmation_tag() {
+            return Err(WelcomeError::ConfirmationTagMismatch);
+        }
     }
 
     let message_secrets_store = MessageSecretsStore::new_with_secret(0, message_secrets);

openmls/src/group/core_group/staged_commit.rs

@@ -285,7 +285,12 @@ impl CoreGroup {
             // TODO: We have tests expecting this error.
             //       They need to be rewritten.
             // debug_assert!(false, "Confirmation tag mismatch");
-            return Err(StageCommitError::ConfirmationTagMismatch);
+
+            // in some tests we need to be able to proceed despite the tag being wrong,
+            // e.g. to test whether a later validation check is performed correctly.
+            if !crate::skip_validation::is_disabled::confirmation_tag() {
+                return Err(StageCommitError::ConfirmationTagMismatch);
+            }
         }
 
         diff.update_interim_transcript_hash(ciphersuite, provider.crypto(), own_confirmation_tag)?;

openmls/src/group/mls_group/processing.rs

@@ -156,7 +156,7 @@ impl MlsGroup {
         self.own_leaf_nodes.clear();
         provider
             .storage()
-            .clear_own_leaf_nodes(self.group_id())
+            .delete_own_leaf_nodes(self.group_id())
             .map_err(MergeCommitError::StorageError)?;
 
         // Delete a potential pending commit

openmls/src/group/mls_group/test_mls_group.rs

@@ -11,9 +11,12 @@ use crate::{
     key_packages::*,
     messages::proposals::*,
     prelude::Capabilities,
-    test_utils::test_framework::{
-        errors::ClientError, noop_authentication_service, ActionType::Commit, CodecUse,
-        MlsGroupTestSetup,
+    test_utils::{
+        frankenstein::{self, FrankenMlsMessage},
+        test_framework::{
+            errors::ClientError, noop_authentication_service, ActionType::Commit, CodecUse,
+            MlsGroupTestSetup,
+        },
     },
     tree::sender_ratchet::SenderRatchetConfiguration,
 };
@@ -1143,15 +1146,57 @@ fn remove_prosposal_by_ref(
 // Test that the builder pattern accurately configures the new group.
 #[openmls_test]
 fn group_context_extensions_proposal() {
+    let alice_provider = &mut Provider::default();
+    let bob_provider = &mut Provider::default();
     let (alice_credential_with_key, _alice_kpb, alice_signer, _alice_pk) =
-        setup_client("Alice", ciphersuite, provider);
+        setup_client("Alice", ciphersuite, alice_provider);
+    let (bob_credential_with_key, _bob_kpb, bob_signer, _bob_pk) =
+        setup_client("bob", ciphersuite, bob_provider);
 
     // === Alice creates a group ===
     let mut alice_group = MlsGroup::builder()
         .ciphersuite(ciphersuite)
-        .build(provider, &alice_signer, alice_credential_with_key)
+        .with_wire_format_policy(WireFormatPolicy::new(
+            OutgoingWireFormatPolicy::AlwaysPlaintext,
+            IncomingWireFormatPolicy::Mixed,
+        ))
+        .build(alice_provider, &alice_signer, alice_credential_with_key)
         .expect("error creating group using builder");
 
+    // === Alice adds Bob ===
+    let bob_key_package = KeyPackage::builder()
+        .build(
+            ciphersuite,
+            bob_provider,
+            &bob_signer,
+            bob_credential_with_key,
+        )
+        .expect("error building key package");
+
+    let (_, welcome, _) = alice_group
+        .add_members(
+            alice_provider,
+            &alice_signer,
+            &[bob_key_package.key_package().clone()],
+        )
+        .unwrap();
+    alice_group.merge_pending_commit(alice_provider).unwrap();
+
+    let welcome: MlsMessageIn = welcome.into();
+    let welcome = welcome
+        .into_welcome()
+        .expect("expected message to be a welcome");
+
+    let mut bob_group = StagedWelcome::new_from_welcome(
+        bob_provider,
+        alice_group.configuration(),
+        welcome,
+        Some(alice_group.export_ratchet_tree().into()),
+    )
+    .expect("Error creating staged join from Welcome")
+    .into_group(bob_provider)
+    .expect("Error creating group from staged join");
+
     // No required capabilities, so no specifically required extensions.
     assert!(alice_group
         .group()
@@ -1168,20 +1213,104 @@ fn group_context_extensions_proposal() {
         RequiredCapabilitiesExtension::new(&[ExtensionType::RatchetTree], &[], &[]),
     ));
 
-    alice_group
-        .propose_group_context_extensions(provider, new_extensions.clone(), &alice_signer)
+    let (proposal, _) = alice_group
+        .propose_group_context_extensions(alice_provider, new_extensions.clone(), &alice_signer)
         .expect("failed to build group context extensions proposal");
 
+    let proc_msg = bob_group
+        .process_message(bob_provider, proposal.into_protocol_message().unwrap())
+        .unwrap();
+    match proc_msg.into_content() {
+        ProcessedMessageContent::ProposalMessage(proposal) => bob_group
+            .store_pending_proposal(bob_provider.storage(), *proposal)
+            .unwrap(),
+        _ => unreachable!(),
+    };
+
     assert_eq!(alice_group.pending_proposals().count(), 1);
 
-    alice_group
-        .commit_to_pending_proposals(provider, &alice_signer)
+    let (commit, _, _) = alice_group
+        .commit_to_pending_proposals(alice_provider, &alice_signer)
         .expect("failed to commit to pending proposals");
 
+    // we'll change the commit we feed to bob to include two GCE proposals
+    let mut franken_commit = FrankenMlsMessage::tls_deserialize(
+        &mut commit.tls_serialize_detached().unwrap().as_slice(),
+    )
+    .unwrap();
+
+    // Craft a commit that has two GroupContextExtension proposals. This is forbidden by the RFC.
+    // Change the commit before alice commits, so alice's state is still in the old epoch and we can
+    // use her state to forge the macs and signatures
+    match &mut franken_commit.body {
+        frankenstein::FrankenMlsMessageBody::PublicMessage(msg) => {
+            match &mut msg.content.body {
+                frankenstein::FrankenFramedContentBody::Commit(commit) => {
+                    let second_gces = frankenstein::FrankenProposalOrRef::Proposal(
+                        frankenstein::FrankenProposal::GroupContextExtensions(vec![
+                            frankenstein::FrankenExtension::LastResort,
+                        ]),
+                    );
+
+                    commit.proposals.push(second_gces);
+                }
+                _ => unreachable!(),
+            }
+
+            let group_context = alice_group.export_group_context().clone();
+
+            let bob_group_context = bob_group.export_group_context();
+            assert_eq!(
+                bob_group_context.confirmed_transcript_hash(),
+                group_context.confirmed_transcript_hash()
+            );
+
+            let secrets = alice_group.group.message_secrets();
+            let membership_key = secrets.membership_key().as_slice();
+
+            *msg = frankenstein::FrankenPublicMessage::auth(
+                alice_provider,
+                group_context.ciphersuite(),
+                &alice_signer,
+                msg.content.clone(),
+                Some(&group_context.into()),
+                Some(membership_key),
+                // this is a dummy confirmation_tag:
+                Some(vec![0u8; 32].into()),
+            );
+        }
+        _ => unreachable!(),
+    }
+
+    // alice merges the unmodified commit
     alice_group
-        .merge_pending_commit(provider)
+        .merge_pending_commit(alice_provider)
         .expect("error merging pending commit");
 
+    let fake_commit = MlsMessageIn::tls_deserialize(
+        &mut franken_commit.tls_serialize_detached().unwrap().as_slice(),
+    )
+    .unwrap();
+
+    let fake_commit_protocol_msg = fake_commit.into_protocol_message().unwrap();
+
+    let err = {
+        let validation_skip_handle = crate::skip_validation::checks::confirmation_tag::handle();
+        validation_skip_handle.with_disabled(|| {
+            bob_group.process_message(bob_provider, fake_commit_protocol_msg.clone())
+        })
+    }
+    .expect_err("expected an error");
+
+    assert!(matches!(
+        err,
+        ProcessMessageError::InvalidCommit(
+            StageCommitError::GroupContextExtensionsProposalValidationError(
+                GroupContextExtensionsProposalValidationError::TooManyGCEProposals
+            )
+        )
+    ));
+
     let required_capabilities = alice_group
         .group()
         .context()
@@ -1195,18 +1324,18 @@ fn group_context_extensions_proposal() {
     // === committing to two group context extensions should fail
 
     alice_group
-        .propose_group_context_extensions(provider, new_extensions, &alice_signer)
+        .propose_group_context_extensions(alice_provider, new_extensions, &alice_signer)
         .expect("failed to build group context extensions proposal");
 
     // the proposals need to be different or they will be deduplicated
     alice_group
-        .propose_group_context_extensions(provider, new_extensions_2, &alice_signer)
+        .propose_group_context_extensions(alice_provider, new_extensions_2, &alice_signer)
         .expect("failed to build group context extensions proposal");
 
     assert_eq!(alice_group.pending_proposals().count(), 2);
 
     alice_group
-        .commit_to_pending_proposals(provider, &alice_signer)
+        .commit_to_pending_proposals(alice_provider, &alice_signer)
         .expect_err(
             "expected error when committing to multiple group context extensions proposals",
         );
@@ -1220,12 +1349,8 @@ fn group_context_extensions_proposal() {
     ));
 
     alice_group
-        .propose_group_context_extensions(provider, new_extensions, &alice_signer)
+        .propose_group_context_extensions(alice_provider, new_extensions, &alice_signer)
         .expect_err("expected an error building GCE proposal with bad required_capabilities");
-
-    // TODO: we need to test that processing a commit with multiple group context extensions
-    //       proposal also fails. however, we can't generate this commit, because our functions for
-    //       constructing commits does not permit it. See #1476
 }
 
 // Test that the builder pattern accurately configures the new group.

openmls/src/lib.rs

@@ -189,6 +189,7 @@ pub mod storage;
 
 // Private
 mod binary_tree;
+mod skip_validation;
 mod tree;
 
 /// Single place, re-exporting the most used public functions.