Refactor Commit Creation and Handling (#431)

* Refactor commit handling to allow for more agility.

* Split out format-independent part of commit handling

* Refactor commit handling allow more flexibility

* Refactor commit generation to allow more flexibility

* Disable expensive tests by default (ctest will still run them)

* Finish consolidating and clean up

* Clean up TranscriptHash API

* Makefile cleanup

* Update TranscriptHash to take bytes

* clang-format

* Fix transcript interop test

* clang-tidy

* clang-format

.clang-tidy

@@ -3,8 +3,10 @@ Checks:            '*,
                     -altera-*,
                     -fuchsia-*,
                     -abseil-string-find-startswith,
+                    -boost-use-ranges,
                     -bugprone-easily-swappable-parameters,
                     -bugprone-exception-escape,
+                    -bugprone-chained-comparison,
                     -cert-err58-cpp,
                     -cppcoreguidelines-avoid-const-or-ref-data-members,
                     -cppcoreguidelines-avoid-magic-numbers,
@@ -32,6 +34,9 @@ Checks:            '*,
                     -readability-function-cognitive-complexity,
                     -readability-identifier-length,
                     -readability-magic-numbers,
+                    -readability-math-missing-parentheses,
+                    -readability-redundant-casting,
                     '
 WarningsAsErrors:  '*'
 HeaderFilterRegex: '*'
+ExcludeHeaderFilterRegex: 'catch_test_macros.hpp'

Makefile

@@ -7,6 +7,7 @@
 BUILD_DIR=build
 TEST_DIR=build/test
 CLANG_FORMAT=clang-format -i
+CLANG_FORMAT_EXCLUDE="test_vectors.cpp"
 CLANG_TIDY=OFF
 OPENSSL11_MANIFEST=alternatives/openssl_1.1
 OPENSSL3_MANIFEST=alternatives/openssl_3
@@ -98,8 +99,8 @@ cclean:
 	rm -rf ${BUILD_DIR}
 
 format:
-	find include -iname "*.h" -or -iname "*.cpp" | xargs ${CLANG_FORMAT}
-	find src -iname "*.h" -or -iname "*.cpp" | xargs ${CLANG_FORMAT}
-	find test -iname "*.h" -or -iname "*.cpp" | xargs ${CLANG_FORMAT}
-	find cmd -iname "*.h" -or -iname "*.cpp" | xargs ${CLANG_FORMAT}
-	find lib -iname "*.h" -or -iname "*.cpp" | grep -v "test_vectors.cpp" |  xargs ${CLANG_FORMAT}
+	for dir in include src test lib; \
+	do \
+		find $${dir} -iname "*.h" -or -iname "*.cpp" | grep -v ${CLANG_FORMAT_EXCLUDE} \
+		| xargs ${CLANG_FORMAT}; \
+	done

include/mls/common.h

@@ -249,6 +249,13 @@ contains(const Container& c, const Value& val)
   return std::find(c.begin(), c.end(), val) != c.end();
 }
 
+template<typename Container, typename Value>
+auto
+find(const Container& c, const Value& val)
+{
+  return std::find(c.begin(), c.end(), val);
+}
+
 template<typename Container, typename UnaryPredicate>
 auto
 find_if(Container& c, const UnaryPredicate& pred)

include/mls/key_schedule.h

@@ -106,6 +106,7 @@ struct KeyScheduleEpoch
   bytes epoch_authenticator;
   bytes external_secret;
   bytes confirmation_key;
+  bytes confirmation_tag;
   bytes membership_key;
   bytes resumption_psk;
   bytes init_secret;
@@ -118,6 +119,7 @@ struct KeyScheduleEpoch
   static KeyScheduleEpoch joiner(CipherSuite suite_in,
                                  const bytes& joiner_secret,
                                  const std::vector<PSKWithSecret>& psks,
+                                 const bytes& confirmed_transcript_hash,
                                  const bytes& context);
 
   // Ciphersuite-only initializer, used by external joiner
@@ -136,10 +138,10 @@ struct KeyScheduleEpoch
   KeyScheduleEpoch next(const bytes& commit_secret,
                         const std::vector<PSKWithSecret>& psks,
                         const std::optional<bytes>& force_init_secret,
+                        const bytes& confirmed_transcript_hash,
                         const bytes& context) const;
 
   GroupKeySource encryption_keys(LeafCount size) const;
-  bytes confirmation_tag(const bytes& confirmed_transcript_hash) const;
   bytes do_export(const std::string& label,
                   const bytes& context,
                   size_t size) const;
@@ -161,10 +163,12 @@ struct KeyScheduleEpoch
                    const bytes& init_secret,
                    const bytes& commit_secret,
                    const bytes& psk_secret,
+                   const bytes& confirmed_transcript_hash,
                    const bytes& context);
   KeyScheduleEpoch next_raw(const bytes& commit_secret,
                             const bytes& psk_secret,
                             const std::optional<bytes>& force_init_secret,
+                            const bytes& confirmed_transcript_hash,
                             const bytes& context) const;
   static bytes welcome_secret_raw(CipherSuite suite,
                                   const bytes& joiner_secret,
@@ -174,6 +178,7 @@ struct KeyScheduleEpoch
   KeyScheduleEpoch(CipherSuite suite_in,
                    const bytes& joiner_secret,
                    const bytes& psk_secret,
+                   const bytes& confirmed_transcript_hash,
                    const bytes& context);
 };
 
@@ -194,10 +199,10 @@ struct TranscriptHash
                  bytes confirmed_in,
                  const bytes& confirmation_tag);
 
-  void update(const AuthenticatedContent& content_auth);
-  void update_confirmed(const AuthenticatedContent& content_auth);
+  // Updating hashes
+  bytes new_confirmed(const bytes& transcript_hash_input) const;
+  void set_confirmed(bytes confirmed_transcript_hash);
   void update_interim(const bytes& confirmation_tag);
-  void update_interim(const AuthenticatedContent& content_auth);
 };
 
 bool

include/mls/state.h

@@ -321,17 +321,37 @@ class State
     const bytes& leaf_secret,
     const std::optional<CommitOpts>& opts,
     const MessageOpts& msg_opts,
-    CommitParams params);
+    const CommitParams& params);
+
+  struct CommitMaterials;
+  CommitMaterials prepare_commit(const bytes& leaf_secret,
+                                 const std::optional<CommitOpts>& opts,
+                                 const CommitParams& params) const;
+  Welcome welcome(bool inline_tree,
+                  const std::vector<PSKWithSecret>& psks,
+                  const std::vector<KeyPackage>& joiners,
+                  const std::vector<std::optional<bytes>>& path_secrets) const;
 
-  std::optional<State> handle(
-    const MLSMessage& msg,
-    std::optional<State> cached_state,
-    const std::optional<CommitParams>& expected_params);
   std::optional<State> handle(
     const ValidatedContent& val_content,
     std::optional<State> cached_state,
     const std::optional<CommitParams>& expected_params);
 
+  void handle_proposal(const AuthenticatedContent& content_auth);
+  State handle_commit(const AuthenticatedContent& content_auth,
+                      std::optional<State> cached_state,
+                      const std::optional<CommitParams>& expected_params) const;
+
+  State ratchet(TreeKEMPublicKey new_tree,
+                LeafIndex committer,
+                const std::optional<NodeIndex>& path_secret_decrypt_node,
+                const std::optional<HPKECiphertext>& encrypted_path_secret,
+                ExtensionList extensions,
+                const std::vector<PSKWithSecret>& psks,
+                const std::optional<bytes>& force_init_secret,
+                const bytes& confirmed_transcript_hash,
+                const bytes& confirmation_tag) const;
+
   // Create an MLSMessage encapsulating some content
   template<typename Inner>
   AuthenticatedContent sign(const Sender& sender,
@@ -345,24 +365,26 @@ class State
   MLSMessage protect_full(Inner&& content, const MessageOpts& msg_opts);
 
   // Apply the changes requested by various messages
-  LeafIndex apply(const Add& add);
-  void apply(LeafIndex target, const Update& update);
-  void apply(LeafIndex target,
-             const Update& update,
-             const HPKEPrivateKey& leaf_priv);
-  LeafIndex apply(const Remove& remove);
-  void apply(const GroupContextExtensions& gce);
-  std::vector<LeafIndex> apply(const std::vector<CachedProposal>& proposals,
-                               Proposal::Type required_type);
-  std::tuple<std::vector<LeafIndex>, std::vector<PSKWithSecret>> apply(
-    const std::vector<CachedProposal>& proposals);
+  static LeafIndex apply(TreeKEMPublicKey& tree, const Add& add);
+  static void apply(TreeKEMPublicKey& tree,
+                    LeafIndex target,
+                    const Update& update);
+  static LeafIndex apply(TreeKEMPublicKey& tree, const Remove& remove);
+  static std::vector<LeafIndex> apply(
+    TreeKEMPublicKey& tree,
+    const std::vector<CachedProposal>& proposals,
+    Proposal::Type required_type);
+  std::tuple<TreeKEMPublicKey,
+             std::vector<LeafIndex>,
+             std::vector<PSKWithSecret>,
+             ExtensionList>
+  apply(const std::vector<CachedProposal>& proposals) const;
 
   // Verify that a specific key package or all members support a given set of
   // extensions
   bool extensions_supported(const ExtensionList& exts) const;
 
   // Extract proposals and PSKs from cache
-  void cache_proposal(AuthenticatedContent content_auth);
   std::optional<CachedProposal> resolve(
     const ProposalOrRef& id,
     std::optional<LeafIndex> sender_index) const;
@@ -409,11 +431,6 @@ class State
   friend bool operator==(const State& lhs, const State& rhs);
   friend bool operator!=(const State& lhs, const State& rhs);
 
-  // Derive and set the secrets for an epoch, given some new entropy
-  void update_epoch_secrets(const bytes& commit_secret,
-                            const std::vector<PSKWithSecret>& psks,
-                            const std::optional<bytes>& force_init_secret);
-
   // Signature verification over a handshake message
   bool verify_internal(const AuthenticatedContent& content_auth) const;
   bool verify_external(const AuthenticatedContent& content_auth) const;
@@ -425,8 +442,15 @@ class State
   // Convert a Roster entry into LeafIndex
   LeafIndex leaf_for_roster_entry(RosterIndex index) const;
 
-  // Create a draft successor state
-  State successor() const;
+  // Create a successor state
+  State successor(LeafIndex index,
+                  TreeKEMPublicKey tree,
+                  TreeKEMPrivateKey tree_priv,
+                  ExtensionList extensions,
+                  const bytes& confirmed_transcript_hash,
+                  bool has_path,
+                  const std::vector<PSKWithSecret>& psks,
+                  const std::optional<bytes>& force_init_secret) const;
 };
 
 } // namespace MLS_NAMESPACE

include/mls/treekem.h

@@ -94,6 +94,12 @@ struct TreeKEMPrivateKey
              const UpdatePath& path,
              const std::vector<LeafIndex>& except);
 
+  void decap(LeafIndex from,
+             const TreeKEMPublicKey& pub,
+             const bytes& context,
+             const NodeIndex& decrypt_node,
+             const HPKECiphertext& encrypted_path_secret);
+
   void truncate(LeafCount size);
 
   bool consistent(const TreeKEMPrivateKey& other) const;
@@ -150,6 +156,17 @@ struct TreeKEMPublicKey
   std::optional<LeafNode> leaf_node(LeafIndex index) const;
   std::vector<NodeIndex> resolve(NodeIndex index) const;
 
+  struct DecapCoords
+  {
+    size_t ancestor_node_index;
+    size_t resolution_node_index;
+    NodeIndex resolution_node;
+  };
+  DecapCoords decap_coords(
+    LeafIndex to,
+    LeafIndex from,
+    const std::vector<LeafIndex>& joiner_locations) const;
+
   template<typename UnaryPredicate>
   bool all_leaves(const UnaryPredicate& pred) const
   {

lib/hpke/src/base64.cpp

@@ -57,7 +57,7 @@ to_base64url(const bytes& data)
 bytes
 from_base64(const std::string& enc)
 {
-  if (enc.length() == 0) {
+  if (enc.empty()) {
     return {};
   }
 

lib/hpke/src/group.cpp

@@ -728,7 +728,7 @@ struct ECKeyGroup : public EVPGroup
   }
 #endif
 
-  static inline int group_to_nid(Group::ID group_id)
+  static int group_to_nid(Group::ID group_id)
   {
     switch (group_id) {
       case Group::ID::P256:
@@ -862,7 +862,7 @@ struct RawKeyGroup : public EVPGroup
 private:
   const int evp_type;
 
-  static inline int group_to_evp(Group::ID group_id)
+  static int group_to_evp(Group::ID group_id)
   {
     switch (group_id) {
       case Group::ID::X25519: