Merge branch 'main' into dependabot/go_modules/lib/hpke/scripts/githu…

…b.com/cloudflare/circl-1.3.7

.github/actions/prepare-build/action.yml

@@ -31,7 +31,7 @@ runs:
       if: ${{ runner.os == 'macOS' }}
       shell: bash
       run: |
-        brew install llvm pkg-config nasm
+        brew install llvm pkg-config nasm go
         ln -s "/usr/local/opt/llvm/bin/clang-format" "/usr/local/bin/clang-format"
         ln -s "/usr/local/opt/llvm/bin/clang-tidy" "/usr/local/bin/clang-tidy"
 

.github/workflows/main_ci.yml

@@ -38,7 +38,6 @@ jobs:
         crypto: [openssl_1.1, openssl_3, boringssl]
 
     env:
-      BUILD_DIR: "${RUNNER_TEMP}/build_${{ matrix.crypto }}"
       CRYPTO_DIR: "./alternatives/${{ matrix.crypto }}"
 
     steps:
@@ -55,18 +54,12 @@ jobs:
 
     - name: Build
       run: |
-        cmake -B "${{ env.BUILD_DIR }}" -DVCPKG_MANIFEST_DIR="${{ env.CRYPTO_DIR }}" -DTESTING=ON
-        cmake --build "${{ env.BUILD_DIR }}"
+        cmake -B "${{ runner.temp }}/build_${{ matrix.crypto }}" -DVCPKG_MANIFEST_DIR="${{ env.CRYPTO_DIR }}" -DTESTING=ON
+        cmake --build "${{ runner.temp }}/build_${{ matrix.crypto }}"
 
-    - name: Unit Test (non-Windows)
-      if: matrix.os != 'windows-latest'
+    - name: Unit Test
       run: |
-        cmake --build "${{ env.BUILD_DIR }}" --target test
-
-    - name: Unit Test (Windows)
-      if: matrix.os == 'windows-latest'
-      run: |
-        cmake --build "${{ env.BUILD_DIR }}" --target RUN_TESTS
+        ctest --test-dir "${{ runner.temp }}/build_${{ matrix.crypto }}"
 
   interop-test:
     if: github.event.pull_request.draft == false
@@ -75,7 +68,6 @@ jobs:
     runs-on: ubuntu-latest
 
     env:
-      BUILD_DIR: "${RUNNER_TEMP}/build_openssl_1.1"
       CRYPTO_DIR: "./alternatives/openssl_1.1"
 
     steps:
@@ -92,8 +84,8 @@ jobs:
 
     - name: Build
       run: |
-        cmake -B "${{ env.BUILD_DIR }}" -DVCPKG_MANIFEST_DIR="${{ env.CRYPTO_DIR }}"
-        cmake --build "${{ env.BUILD_DIR }}"
+        cmake -B "${{ runner.temp }}/build_openssl_1.1" -DVCPKG_MANIFEST_DIR="${{ env.CRYPTO_DIR }}"
+        cmake --build "${{ runner.temp }}/build_openssl_1.1"
 
     - name: Build (Interop Harness)
       run: |
@@ -124,7 +116,6 @@ jobs:
         crypto: [openssl_1.1, openssl_3, boringssl]
 
     env:
-      BUILD_DIR: "${RUNNER_TEMP}/build_${{ matrix.crypto }}"
       CRYPTO_DIR: "./alternatives/${{ matrix.crypto }}"
 
     steps:
@@ -141,6 +132,6 @@ jobs:
 
     - name: Build with clang-tidy
       run: |
-        cmake -B "${{ env.BUILD_DIR }}" -DVCPKG_MANIFEST_DIR="${{ env.CRYPTO_DIR }}" \
-                                        -DTESTING=ON -DCLANG_TIDY=ON -DSANITIZERS=ON
-        cmake --build "${{ env.BUILD_DIR }}"
+        cmake -B "${{ runner.temp }}/build_${{ matrix.crypto }}" -DVCPKG_MANIFEST_DIR="${{ env.CRYPTO_DIR }}" \
+                                                                 -DTESTING=ON -DCLANG_TIDY=ON -DSANITIZERS=ON
+        cmake --build "${{ runner.temp }}/build_${{ matrix.crypto }}"

README.md

@@ -12,7 +12,7 @@ MLSPP requires a few prerequisite libraries in order to fully build.
 
 * [nlohmann::json](https://github.com/nlohmann/json) - Tested with latest versions.
 * Cryptography Library - OpenSSL 1.1.1, OpenSSL 3.0, BoringSSL compatible (see details below)
-* [doctest](https://github.com/doctest/doctest) - Tested with latest versions.  Only required when building the test suite.
+* [Catch2](https://github.com/catchorg/Catch2) - Only required when building the test suite.
 
 ### Installing Prerequisites 
 

include/mls/state.h

@@ -397,6 +397,8 @@ class State
   static bool valid_restart(const std::vector<CachedProposal>& proposals,
                             ResumptionPSKUsage allowed_usage);
 
+  static bool valid_external_proposal_type(const Proposal::Type proposal_type);
+
   CommitParams infer_commit_type(
     const std::optional<LeafIndex>& sender,
     const std::vector<CachedProposal>& proposals,

lib/bytes/include/bytes/bytes.h

@@ -17,7 +17,11 @@ struct bytes
   bytes& operator=(bytes&&) = default;
 
   // Zeroize on drop
-  ~bytes() { std::fill(_data.begin(), _data.end(), uint8_t(0)); }
+  ~bytes()
+  {
+    auto ptr = static_cast<volatile uint8_t*>(_data.data());
+    std::fill(ptr, ptr + _data.size(), uint8_t(0));
+  }
 
   // Mimic std::vector ctors
   bytes(size_t count, const uint8_t& value = 0)

src/state.cpp

@@ -1250,6 +1250,12 @@ State::cache_proposal(AuthenticatedContent content_auth)
   }
 
   const auto& proposal = var::get<Proposal>(content_auth.content.content);
+
+  if (content_auth.content.sender.sender_type() == SenderType::external &&
+      !valid_external_proposal_type(proposal.proposal_type())) {
+    throw ProtocolError("Invalid external proposal");
+  }
+
   if (!valid(sender_location, proposal)) {
     throw ProtocolError("Invalid proposal");
   }
@@ -1632,7 +1638,6 @@ State::valid(std::optional<LeafIndex> sender, const Proposal& proposal) const
 {
   const auto specifically_valid = overloaded{
     [&](const Update& update) { return valid(opt::get(sender), update); },
-
     [&](const auto& proposal) { return valid(proposal); },
   };
   return var::visit(specifically_valid, proposal.content);
@@ -1870,6 +1875,22 @@ State::valid_restart(const std::vector<CachedProposal>& proposals,
   return acceptable_psks && found_allowed;
 }
 
+bool
+State::valid_external_proposal_type(const Proposal::Type proposal_type)
+{
+  switch (proposal_type) {
+    case ProposalType::add:
+    case ProposalType::remove:
+    case ProposalType::psk:
+    case ProposalType::reinit:
+    case ProposalType::group_context_extensions:
+      return true;
+
+    default:
+      return false;
+  }
+}
+
 bool
 // NOLINTNEXTLINE(readability-convert-member-functions-to-static)
 State::valid_external(const std::vector<CachedProposal>& proposals) const

src/treekem.cpp

@@ -903,18 +903,19 @@ TreeKEMPublicKey::parent_hashes(
   }
 
   // The list of nodes for whom parent hashes are computed, namely: Direct path
-  // excluding root, including leaf
+  // excluding the last entry, including leaf
   auto from_node = NodeIndex(from);
   auto dp = fdp;
+  auto [last, _res_last] = dp.back();
   dp.pop_back();
   dp.insert(dp.begin(), { from_node, {} });
 
   if (dp.size() != path_nodes.size()) {
     throw ProtocolError("Malformed UpdatePath");
   }
 
-  // Parent hash for all the parents, starting from the root
-  auto last = NodeIndex::root(size);
+  // Parent hash for all the parents, starting from the last entry of the
+  // filtered direct path
   auto last_hash = bytes{};
   auto ph = std::vector<bytes>(dp.size());
   for (int i = static_cast<int>(dp.size()) - 1; i >= 0; i--) {

test/state.cpp

@@ -1069,3 +1069,197 @@ TEST_CASE_METHOD(RelatedGroupTest, "Reinitialize the group")
     REQUIRE(state == new_states[0]);
   }
 }
+
+TEST_CASE_METHOD(StateTest, "Parent Hash with Empty Left Subtree")
+{
+  // Create a group with 4 members
+  auto state_0 = State(group_id,
+                       suite,
+                       leaf_privs[0],
+                       identity_privs[0],
+                       key_packages[0].leaf_node,
+                       ExtensionList{});
+
+  const auto adds = std::vector{
+    state_0.add_proposal(key_packages[1]),
+    state_0.add_proposal(key_packages[2]),
+    state_0.add_proposal(key_packages[3]),
+  };
+
+  auto [_commit0, welcome0, new_state_0] =
+    state_0.commit(fresh_secret(), CommitOpts{ adds, true, false, {} }, {});
+  silence_unused(_commit0);
+  state_0 = new_state_0;
+
+  auto state_2 = State(init_privs[2],
+                       leaf_privs[2],
+                       identity_privs[2],
+                       key_packages[2],
+                       welcome0,
+                       std::nullopt,
+                       {});
+  // Member @2 removes the members on the left side of the tree
+  const auto removes = std::vector{
+    state_2.remove_proposal(LeafIndex{ 0 }),
+    state_2.remove_proposal(LeafIndex{ 1 }),
+  };
+
+  auto [commit2, welcome2, new_state_2] =
+    state_2.commit(fresh_secret(), CommitOpts{ removes, true, false, {} }, {});
+  silence_unused(commit2);
+  silence_unused(welcome2);
+  state_2 = new_state_2;
+
+  // Member @2 should have a valid tree, even though its filtered direct path no
+  // longer goes to the root.
+  REQUIRE(state_2.tree().parent_hash_valid());
+}
+
+class ExternalSenderTest : public StateTest
+{
+protected:
+  const SignaturePrivateKey external_sig_priv =
+    SignaturePrivateKey::generate(suite);
+  const Credential external_sender_cred = Credential::basic({ 0 });
+  const bytes psk_id = from_ascii("psk ID");
+  ExtensionList group_extensions;
+
+  ExternalSenderTest()
+  {
+    group_extensions.add(ExternalSendersExtension{ {
+      { external_sig_priv.public_key, external_sender_cred },
+    } });
+
+    // Initialize the creator's state
+    states.emplace_back(group_id,
+                        suite,
+                        leaf_privs[0],
+                        identity_privs[0],
+                        key_packages[0].leaf_node,
+                        group_extensions);
+
+    // Add a second member so that we can test removal proposal
+    auto add = states[0].add_proposal(key_packages[1]);
+    auto [commit, welcome, new_state] = states[0].commit(
+      fresh_secret(), CommitOpts{ { add }, true, false, {} }, {});
+    states[0] = new_state;
+
+    silence_unused(commit);
+
+    states.push_back({ init_privs[1],
+                       leaf_privs[1],
+                       identity_privs[1],
+                       key_packages[1],
+                       welcome,
+                       std::nullopt,
+                       {} });
+  }
+
+  PublicMessage GenerateExternalSenderProposal(const Proposal& proposal)
+  {
+    auto group_context = states[0].group_context();
+
+    auto proposal_content = GroupContent{ group_context.group_id,
+                                          group_context.epoch,
+                                          { ExternalSenderIndex{ 0 } },
+                                          {},
+                                          proposal };
+
+    auto content_auth_original =
+      AuthenticatedContent::sign(WireFormat::mls_public_message,
+                                 proposal_content,
+                                 suite,
+                                 external_sig_priv,
+                                 group_context);
+
+    return PublicMessage::protect(
+      content_auth_original, suite, std::nullopt, group_context);
+  }
+};
+
+TEST_CASE_METHOD(ExternalSenderTest,
+                 "Allows Expected Proposals from External Sender")
+{
+  // For expected proposals, we ensure that calling State::handle with the
+  // proposal does not throw an exception.
+
+  // Add
+  auto add_proposal = Proposal{ Add{ key_packages[2] } };
+  auto ext_add_message = GenerateExternalSenderProposal(add_proposal);
+
+  REQUIRE(!states[0].handle(ext_add_message).has_value());
+
+  // Remove
+  auto remove_proposal = Proposal{ Remove{ LeafIndex{ 1 } } };
+  auto ext_remove_message = GenerateExternalSenderProposal(remove_proposal);
+
+  REQUIRE(!states[0].handle(ext_remove_message).has_value());
+
+  // PSK
+  auto group_context = states[0].group_context();
+  auto psk_proposal =
+    Proposal{ PreSharedKey{ ResumptionPSK{ ResumptionPSKUsage::application,
+                                           group_context.group_id,
+                                           group_context.epoch },
+                            random_bytes(suite.secret_size()) } };
+  auto ext_psk_message = GenerateExternalSenderProposal(psk_proposal);
+
+  REQUIRE(!states[0].handle(ext_psk_message).has_value());
+
+  // ReInit
+  auto updated_extensions = group_extensions;
+  updated_extensions.add(CustomExtension{ 0xa0 });
+
+  auto reinit_proposal = Proposal{ ReInit{ group_context.group_id,
+                                           ProtocolVersion::mls10,
+                                           group_context.cipher_suite,
+                                           updated_extensions } };
+  auto ext_reinit_message = GenerateExternalSenderProposal(reinit_proposal);
+
+  REQUIRE(!states[0].handle(ext_reinit_message).has_value());
+
+  // GroupContextExtensions
+
+  auto group_context_proposal =
+    Proposal{ GroupContextExtensions{ updated_extensions } };
+  auto ext_group_context_message =
+    GenerateExternalSenderProposal(group_context_proposal);
+
+  REQUIRE(!states[0].handle(ext_group_context_message).has_value());
+}
+
+TEST_CASE_METHOD(ExternalSenderTest,
+                 "Refuses Unexpected Proposals from External Sender")
+{
+  // For unexpected proposals, we ensure that calling State::handle with the
+  // throws the expected exception.
+
+  // The proposals throw bad_optional_access since the validation calls
+  // opt::get(sender) on a nullopt sender
+
+  // Update
+  auto update_proposal = Proposal{ Update{ key_packages[1].leaf_node } };
+  auto ext_update_message = GenerateExternalSenderProposal(update_proposal);
+
+  REQUIRE_THROWS_WITH(states[0].handle(ext_update_message),
+                      "Invalid external proposal");
+
+  // ExternalInit
+  auto group_info = states[0].group_info(false);
+  auto maybe_external_pub = group_info.extensions.find<ExternalPubExtension>();
+
+  REQUIRE(maybe_external_pub.has_value());
+
+  const auto& external_pub = opt::get(maybe_external_pub).external_pub;
+
+  auto [kem_output, force_init_secret] =
+    KeyScheduleEpoch::external_init(suite, external_pub);
+  silence_unused(force_init_secret);
+
+  auto external_init_proposal = Proposal{ ExternalInit{ kem_output } };
+  auto external_init_message =
+    GenerateExternalSenderProposal(external_init_proposal);
+
+  REQUIRE_THROWS_WITH(states[0].handle(external_init_message),
+                      "Invalid external proposal");
+}