From ebf200cc086623454f57677e566c5d4b0a97019d Mon Sep 17 00:00:00 2001 From: Stefan Berthold Date: Thu, 9 Oct 2025 15:53:52 +0000 Subject: [PATCH 1/3] reject MLS messages while in epoch 0 --- changelog.d/3-bug-fixes/mls-message-epoch-0 | 1 + services/galley/src/Galley/API/MLS/Message.hs | 7 +++++-- 2 files changed, 6 insertions(+), 2 deletions(-) create mode 100644 changelog.d/3-bug-fixes/mls-message-epoch-0 diff --git a/changelog.d/3-bug-fixes/mls-message-epoch-0 b/changelog.d/3-bug-fixes/mls-message-epoch-0 new file mode 100644 index 0000000000..723e838f4b --- /dev/null +++ b/changelog.d/3-bug-fixes/mls-message-epoch-0 @@ -0,0 +1 @@ +Reject messages in MLS groups while in epoch 0. diff --git a/services/galley/src/Galley/API/MLS/Message.hs b/services/galley/src/Galley/API/MLS/Message.hs index 548021fffb..67d5c6c517 100644 --- a/services/galley/src/Galley/API/MLS/Message.hs +++ b/services/galley/src/Galley/API/MLS/Message.hs @@ -506,10 +506,13 @@ postMLSMessageToLocalConv qusr c con msg ctype convOrSubId = do for_ convOrSub.ciphersuite $ \ciphersuite -> do checkConversationOutOfSync mempty lConvOrSub ciphersuite - -- reject application messages older than 2 epochs - -- FUTUREWORK: consider rejecting this message if the conversation epoch is 0 + -- reject application messages for epoch 0 let epochInt :: Epoch -> Integer epochInt = fromIntegral . epochNumber + when (epochInt msg.epoch == 0) . throw $ + mlsProtocolError "Application messages at epoch 0 are not supported" + + -- reject application messages older than 2 epochs case convOrSub.mlsMeta.cnvmlsActiveData of Nothing -> throw $ mlsProtocolError "Application messages at epoch 0 are not supported" Just activeData -> From e167202f7320e878884cbe5658196d8473680b39 Mon Sep 17 00:00:00 2001 From: Stefan Berthold Date: Fri, 10 Oct 2025 08:55:12 +0000 Subject: [PATCH 2/3] add test --- integration/test/MLS/Util.hs | 15 ++++++++++----- integration/test/Test/MLS.hs | 22 ++++++++++++++++++++++ integration/test/Test/MLS/Reset.hs | 6 +++--- 3 files changed, 35 insertions(+), 8 deletions(-) diff --git a/integration/test/MLS/Util.hs b/integration/test/MLS/Util.hs index 8e50d2998c..80c023645a 100644 --- a/integration/test/MLS/Util.hs +++ b/integration/test/MLS/Util.hs @@ -1015,16 +1015,17 @@ removeMemberFromChannel user channel userToBeRemoved = do } resetMLSConversation :: - (HasCallStack, MakesValue cid, MakesValue conv) => - cid -> + (HasCallStack, MakesValue user, MakesValue conv) => + user -> + ClientIdentity -> conv -> App Value -resetMLSConversation cid conv = do +resetMLSConversation user cid conv = do convId <- objConvId conv mlsConv <- getMLSConv convId - resetConversation cid mlsConv.groupId mlsConv.epoch >>= assertStatus 200 + resetConversation user mlsConv.groupId mlsConv.epoch >>= assertStatus 200 - conv' <- getConversation cid convId >>= getJSON 200 + conv' <- getConversation user convId >>= getJSON 200 groupId <- conv' %. "group_id" & asString groupId `shouldNotMatch` (mlsConv.groupId :: String) conv' %. "epoch" `shouldMatchInt` 0 @@ -1043,4 +1044,8 @@ resetMLSConversation cid conv = do ) $ Map.delete convId mls.convs } + + mlsConv' <- getMLSConv convId' + keys <- getMLSPublicKeys user >>= getJSON 200 + resetClientGroup mlsConv'.ciphersuite cid mlsConv'.groupId convId' keys pure conv' diff --git a/integration/test/Test/MLS.hs b/integration/test/Test/MLS.hs index c55dd81a5d..5a4d622db2 100644 --- a/integration/test/Test/MLS.hs +++ b/integration/test/Test/MLS.hs @@ -92,6 +92,28 @@ testPastStaleApplicationMessage otherDomain = do -- bob's application messages are now rejected void $ postMLSMessage bob1 msg2.message >>= getJSON 409 +testEpochZeroApplicationMessage :: (HasCallStack) => App () +testEpochZeroApplicationMessage = do + [alice] <- createAndConnectUsers [make OwnDomain] + alice1 <- createMLSClient def alice + conv <- createNewGroup def alice1 + void $ createAddCommit alice1 conv [] >>= sendAndConsumeCommitBundle + mlsConv <- getMLSConv conv + + -- send message, make sure that's succeeding + msg <- createApplicationMessage mlsConv.convId alice1 "group is initialised" + postMLSMessage alice1 msg.message >>= assertStatus 201 + + -- reset conversation, so it exists on server and client with epoch 0 + convId' <- objConvId =<< resetMLSConversation alice alice1 conv + + -- send message, make sure that's failing + msg' <- createApplicationMessage convId' alice1 "group not initialised" + postMLSMessage alice1 msg'.message >>= flip withResponse \resp -> do + j <- getJSON 400 resp + j %. "label" `shouldMatch` "mls-protocol-error" + j %. "message" `shouldMatch` "Application messages at epoch 0 are not supported" + testFutureStaleApplicationMessage :: (HasCallStack) => App () testFutureStaleApplicationMessage = do [alice, bob, charlie] <- createAndConnectUsers [OwnDomain, OwnDomain, OwnDomain] diff --git a/integration/test/Test/MLS/Reset.hs b/integration/test/Test/MLS/Reset.hs index fac4b0a32b..37b7a96809 100644 --- a/integration/test/Test/MLS/Reset.hs +++ b/integration/test/Test/MLS/Reset.hs @@ -62,7 +62,7 @@ testResetSelfConversation = do void $ createAddCommit alice1 convId [alice] >>= sendAndConsumeCommitBundle mlsConv <- getMLSConv convId - conv' <- resetMLSConversation alice conv + conv' <- resetMLSConversation alice alice1 conv conv' %. "group_id" `shouldNotMatch` (mlsConv.groupId :: String) conv' %. "epoch" `shouldMatchInt` 0 convId' <- objConvId conv' @@ -83,7 +83,7 @@ testResetOne2OneConversation = do otherDomain <- asString OtherDomain conv <- getMLSOne2OneConversation alice bob >>= getJSON 200 convOwnerDomain <- asString $ conv %. "conversation.qualified_id.domain" - let user = if convOwnerDomain == otherDomain then bob else alice + let (user, cid) = if convOwnerDomain == otherDomain then (bob, bob1) else (alice, alice1) convId <- objConvId (conv %. "conversation") resetOne2OneGroup def alice1 conv @@ -91,7 +91,7 @@ testResetOne2OneConversation = do void $ createPendingProposalCommit convId alice1 >>= sendAndConsumeCommitBundle mlsConv <- getMLSConv convId - conv' <- resetMLSConversation user (conv %. "conversation") + conv' <- resetMLSConversation user cid (conv %. "conversation") conv' %. "group_id" `shouldNotMatch` (mlsConv.groupId :: String) conv' %. "epoch" `shouldMatchInt` 0 convId' <- objConvId conv' From 05c16899b65dda9efaa5e22e9a9d3dde9aca4518 Mon Sep 17 00:00:00 2001 From: Stefan Berthold Date: Tue, 18 Nov 2025 13:45:49 +0000 Subject: [PATCH 3/3] fixup! add test --- integration/test/API/Galley.hs | 2 +- integration/test/MLS/Util.hs | 11 +++++------ integration/test/Test/MLS.hs | 2 +- integration/test/Test/MLS/Reset.hs | 4 ++-- 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/integration/test/API/Galley.hs b/integration/test/API/Galley.hs index c6295e4463..96fb09923b 100644 --- a/integration/test/API/Galley.hs +++ b/integration/test/API/Galley.hs @@ -902,7 +902,7 @@ getSelfMember user conv = do req <- baseRequest user Galley Versioned (joinHttpPath ["conversations", domain, cnv, "self"]) submit "GET" req -resetConversation :: (MakesValue user) => user -> String -> Word64 -> App Response +resetConversation :: (HasCallStack, MakesValue user) => user -> String -> Word64 -> App Response resetConversation user groupId epoch = do req <- baseRequest user Galley Versioned (joinHttpPath ["mls", "reset-conversation"]) let payload = object ["group_id" .= groupId, "epoch" .= epoch] diff --git a/integration/test/MLS/Util.hs b/integration/test/MLS/Util.hs index 80c023645a..827b317b15 100644 --- a/integration/test/MLS/Util.hs +++ b/integration/test/MLS/Util.hs @@ -1015,17 +1015,16 @@ removeMemberFromChannel user channel userToBeRemoved = do } resetMLSConversation :: - (HasCallStack, MakesValue user, MakesValue conv) => - user -> + (HasCallStack, MakesValue conv) => ClientIdentity -> conv -> App Value -resetMLSConversation user cid conv = do +resetMLSConversation cid conv = do convId <- objConvId conv mlsConv <- getMLSConv convId - resetConversation user mlsConv.groupId mlsConv.epoch >>= assertStatus 200 + resetConversation cid mlsConv.groupId mlsConv.epoch >>= assertStatus 200 - conv' <- getConversation user convId >>= getJSON 200 + conv' <- getConversation cid convId >>= getJSON 200 groupId <- conv' %. "group_id" & asString groupId `shouldNotMatch` (mlsConv.groupId :: String) conv' %. "epoch" `shouldMatchInt` 0 @@ -1046,6 +1045,6 @@ resetMLSConversation user cid conv = do } mlsConv' <- getMLSConv convId' - keys <- getMLSPublicKeys user >>= getJSON 200 + keys <- getMLSPublicKeys cid >>= getJSON 200 resetClientGroup mlsConv'.ciphersuite cid mlsConv'.groupId convId' keys pure conv' diff --git a/integration/test/Test/MLS.hs b/integration/test/Test/MLS.hs index 5a4d622db2..d8d0eb8222 100644 --- a/integration/test/Test/MLS.hs +++ b/integration/test/Test/MLS.hs @@ -105,7 +105,7 @@ testEpochZeroApplicationMessage = do postMLSMessage alice1 msg.message >>= assertStatus 201 -- reset conversation, so it exists on server and client with epoch 0 - convId' <- objConvId =<< resetMLSConversation alice alice1 conv + convId' <- objConvId =<< resetMLSConversation alice1 conv -- send message, make sure that's failing msg' <- createApplicationMessage convId' alice1 "group not initialised" diff --git a/integration/test/Test/MLS/Reset.hs b/integration/test/Test/MLS/Reset.hs index 37b7a96809..88d12ec891 100644 --- a/integration/test/Test/MLS/Reset.hs +++ b/integration/test/Test/MLS/Reset.hs @@ -62,7 +62,7 @@ testResetSelfConversation = do void $ createAddCommit alice1 convId [alice] >>= sendAndConsumeCommitBundle mlsConv <- getMLSConv convId - conv' <- resetMLSConversation alice alice1 conv + conv' <- resetMLSConversation alice1 conv conv' %. "group_id" `shouldNotMatch` (mlsConv.groupId :: String) conv' %. "epoch" `shouldMatchInt` 0 convId' <- objConvId conv' @@ -91,7 +91,7 @@ testResetOne2OneConversation = do void $ createPendingProposalCommit convId alice1 >>= sendAndConsumeCommitBundle mlsConv <- getMLSConv convId - conv' <- resetMLSConversation user cid (conv %. "conversation") + conv' <- resetMLSConversation cid (conv %. "conversation") conv' %. "group_id" `shouldNotMatch` (mlsConv.groupId :: String) conv' %. "epoch" `shouldMatchInt` 0 convId' <- objConvId conv'