From 14d0b7e2a7af7357d0bf91a53b907a2e03761e93 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Tue, 4 Apr 2023 11:38:32 -0400 Subject: [PATCH] Don't double-handle ReInit Commits (#341) * Don't double-handle ReInit Commits * CI errors --- include/mls/state.h | 4 ++++ src/state.cpp | 26 ++++++++++++++++---------- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/include/mls/state.h b/include/mls/state.h index b8fbeb90..2d7b448f 100644 --- a/include/mls/state.h +++ b/include/mls/state.h @@ -315,6 +315,10 @@ class State const MLSMessage& msg, std::optional cached_state, const std::optional& expected_params); + std::optional handle( + const AuthenticatedContent& content_auth, + std::optional cached_state, + const std::optional& expected_params); // Create an MLSMessage encapsulating some content template diff --git a/src/state.cpp b/src/state.cpp index 76c5c338..248ed7fc 100644 --- a/src/state.cpp +++ b/src/state.cpp @@ -719,12 +719,19 @@ State::handle(const MLSMessage& msg, std::optional cached_state, const std::optional& expected_params) { - // Verify the signature on the message auto content_auth = unprotect_to_content_auth(msg); if (!verify(content_auth)) { throw InvalidParameterError("Message signature failed to verify"); } + return handle(content_auth, std::move(cached_state), expected_params); +} + +std::optional +State::handle(const AuthenticatedContent& content_auth, + std::optional cached_state, + const std::optional& expected_params) +{ // Validate the GroupContent const auto& content = content_auth.content; if (content.group_id != _group_id) { @@ -1051,17 +1058,16 @@ State::reinit_commit(const bytes& leaf_secret, State::Tombstone State::handle_reinit_commit(const MLSMessage& commit_msg) { + // Verify the signature and process the commit + auto content_auth = unprotect_to_content_auth(commit_msg); + if (!verify(content_auth)) { + throw InvalidParameterError("Message signature failed to verify"); + } + auto new_state = - opt::get(handle(commit_msg, std::nullopt, ReInitCommitParams{})); + opt::get(handle(content_auth, std::nullopt, ReInitCommitParams{})); - // XXX(RLB): This is pretty brute force, replicating a bunch of logic in - // State::handle() so that we can find the ReInit commit. There is probably a - // more elegant way to extract the reinit parameters. - // - // XXX(RLB): We also skip a bunch of checks that are done in State::handle(), - // on the theory that they will have already been done in the State::handle() - // call above. - auto content_auth = unprotect_to_content_auth(commit_msg); + // Extract the ReInit and create the Tombstone const auto& commit = var::get(content_auth.content.content); const auto proposals = must_resolve(commit.proposals, std::nullopt); if (!valid_reinit(proposals)) {