From 5903c390ffbfcf7c0c63f1dcc0035326b493ab2d Mon Sep 17 00:00:00 2001 From: Paolo Capriotti Date: Wed, 12 Jul 2023 09:55:27 +0200 Subject: [PATCH 1/4] Test stale application messages --- integration/test/Test/MLS.hs | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/integration/test/Test/MLS.hs b/integration/test/Test/MLS.hs index 7cd2b466a1..5c53958c74 100644 --- a/integration/test/Test/MLS.hs +++ b/integration/test/Test/MLS.hs @@ -39,6 +39,33 @@ testSendMessageNoReturnToSender = do ) wsSender +testStaleApplicationMessage :: HasCallStack => Domain -> App () +testStaleApplicationMessage otherDomain = do + [alice, bob, charlie, dave, eve] <- + createAndConnectUsers [OwnDomain, otherDomain, OwnDomain, OwnDomain, OwnDomain] + [alice1, bob1, charlie1] <- traverse createMLSClient [alice, bob, charlie] + traverse_ uploadNewKeyPackage [bob1, charlie1] + void $ createNewGroup alice1 + + -- alice adds bob first + void $ createAddCommit alice1 [bob] >>= sendAndConsumeCommitBundle + + -- bob prepares some application messages + [msg1, msg2] <- replicateM 2 $ createApplicationMessage bob1 "hi alice" + + -- alice adds charlie and dave with different commits + void $ createAddCommit alice1 [charlie] >>= sendAndConsumeCommitBundle + void $ createAddCommit alice1 [dave] >>= sendAndConsumeCommitBundle + + -- bob's application messages still go through + void $ postMLSMessage bob1 msg1.message >>= getJSON 201 + + -- alice adds eve + void $ createAddCommit alice1 [eve] >>= sendAndConsumeCommitBundle + + -- bob's application messages are now rejected + void $ postMLSMessage bob1 msg2.message >>= getJSON 409 + testMixedProtocolUpgrade :: HasCallStack => Domain -> App () testMixedProtocolUpgrade secondDomain = do (alice, tid) <- createTeam OwnDomain From d01015e63e495a16be1942b07aee5b1c4a0d007c Mon Sep 17 00:00:00 2001 From: Paolo Capriotti Date: Mon, 17 Jul 2023 09:44:26 +0200 Subject: [PATCH 2/4] Reject stale application messages Application messages for epochs before 2 ago are now rejected with an `MLSStaleMessage` error. --- services/galley/src/Galley/API/MLS/Message.hs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/services/galley/src/Galley/API/MLS/Message.hs b/services/galley/src/Galley/API/MLS/Message.hs index 3db183b2b2..fe1e181aee 100644 --- a/services/galley/src/Galley/API/MLS/Message.hs +++ b/services/galley/src/Galley/API/MLS/Message.hs @@ -371,6 +371,7 @@ postMLSMessageToLocalConv :: Sem r ([LocalConversationUpdate], Maybe UnreachableUsers) postMLSMessageToLocalConv qusr c con msg ctype convOrSubId = do lConvOrSub <- fetchConvOrSub qusr msg.groupId ctype convOrSubId + let convOrSub = tUnqualified lConvOrSub for_ msg.sender $ \sender -> void $ getSenderIdentity qusr c sender lConvOrSub @@ -380,12 +381,23 @@ postMLSMessageToLocalConv qusr c con msg ctype convOrSubId = do IncomingMessageContentPublic pub -> case pub.content of FramedContentCommit _commit -> throwS @'MLSUnsupportedMessage FramedContentApplicationData _ -> throwS @'MLSUnsupportedMessage + -- proposal message FramedContentProposal prop -> processProposal qusr lConvOrSub msg.groupId msg.epoch pub prop IncomingMessageContentPrivate -> do - when ((tUnqualified lConvOrSub).migrationState == MLSMigrationMixed) $ + -- application message: + + -- reject all application messages if the conv is in mixed state + when (convOrSub.migrationState == MLSMigrationMixed) $ throwS @'MLSUnsupportedMessage + -- reject application messages older than 2 epochs + when + ( msg.epoch.epochNumber + < convOrSub.mlsMeta.cnvmlsEpoch.epochNumber - 2 + ) + $ throwS @'MLSStaleMessage + unreachables <- propagateMessage qusr (Just c) lConvOrSub con msg.rawMessage (tUnqualified lConvOrSub).members pure ([], unreachables) From 98e6ad273f215c45a3f6885b553eb97791287ed1 Mon Sep 17 00:00:00 2001 From: Paolo Capriotti Date: Wed, 12 Jul 2023 10:00:53 +0200 Subject: [PATCH 3/4] Add CHANGELOG entry --- changelog.d/2-features/mls-stale-app-messages | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/2-features/mls-stale-app-messages diff --git a/changelog.d/2-features/mls-stale-app-messages b/changelog.d/2-features/mls-stale-app-messages new file mode 100644 index 0000000000..5005ccbac9 --- /dev/null +++ b/changelog.d/2-features/mls-stale-app-messages @@ -0,0 +1 @@ +MLS application messages for older epochs are now rejected From dc4e7691360ea5c80ebe461e3baa0590107ba491 Mon Sep 17 00:00:00 2001 From: Paolo Capriotti Date: Mon, 17 Jul 2023 14:50:40 +0200 Subject: [PATCH 4/4] Fix unsigned calculation with epoch number --- services/galley/src/Galley/API/MLS/Message.hs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/services/galley/src/Galley/API/MLS/Message.hs b/services/galley/src/Galley/API/MLS/Message.hs index fe1e181aee..d7c721a66b 100644 --- a/services/galley/src/Galley/API/MLS/Message.hs +++ b/services/galley/src/Galley/API/MLS/Message.hs @@ -392,10 +392,10 @@ postMLSMessageToLocalConv qusr c con msg ctype convOrSubId = do throwS @'MLSUnsupportedMessage -- reject application messages older than 2 epochs + let epochInt :: Epoch -> Integer + epochInt = fromIntegral . epochNumber when - ( msg.epoch.epochNumber - < convOrSub.mlsMeta.cnvmlsEpoch.epochNumber - 2 - ) + (epochInt msg.epoch < epochInt convOrSub.mlsMeta.cnvmlsEpoch - 2) $ throwS @'MLSStaleMessage unreachables <- propagateMessage qusr (Just c) lConvOrSub con msg.rawMessage (tUnqualified lConvOrSub).members