From 81168426afcd935ad93c1e51564080361bc3b56a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Wed, 12 Oct 2022 17:09:25 +0200 Subject: [PATCH 01/11] Check external commit criteria - Extract the key package from the update path --- libs/wire-api/src/Wire/API/Error/Galley.hs | 9 + .../src/Wire/API/Routes/Public/Galley.hs | 9 + services/galley/src/Galley/API/MLS/Message.hs | 158 ++++++++++++++---- 3 files changed, 143 insertions(+), 33 deletions(-) diff --git a/libs/wire-api/src/Wire/API/Error/Galley.hs b/libs/wire-api/src/Wire/API/Error/Galley.hs index 171f894b3d5..824b084f234 100644 --- a/libs/wire-api/src/Wire/API/Error/Galley.hs +++ b/libs/wire-api/src/Wire/API/Error/Galley.hs @@ -83,6 +83,9 @@ data GalleyError | MLSClientSenderUserMismatch | MLSWelcomeMismatch | MLSMissingGroupInfo + | MLSMissingExternalInit + | MLSRemovalUserMismatch + | MLSExternalCommitRemoveProposal | -- NoBindingTeamMembers | NoBindingTeam @@ -206,6 +209,12 @@ type instance MapError 'MLSWelcomeMismatch = 'StaticError 400 "mls-welcome-misma type instance MapError 'MLSMissingGroupInfo = 'StaticError 404 "mls-missing-group-info" "The conversation has no group information" +type instance MapError 'MLSMissingExternalInit = 'StaticError 400 "mls-missing-external-init" "The external commit is missing an external init proposal" + +type instance MapError 'MLSRemovalUserMismatch = 'StaticError 403 "mls-removal-user-mismatch" "The external commit attempts to remove a client from a user other than themselves" + +type instance MapError 'MLSExternalCommitRemoveProposal = 'StaticError 403 "mls-external-commit-proposals" "The external commit has to have at most one remove proposal" + type instance MapError 'NoBindingTeamMembers = 'StaticError 403 "non-binding-team-members" "Both users must be members of the same binding team" type instance MapError 'NoBindingTeam = 'StaticError 403 "no-binding-team" "Operation allowed only on binding teams" diff --git a/libs/wire-api/src/Wire/API/Routes/Public/Galley.hs b/libs/wire-api/src/Wire/API/Routes/Public/Galley.hs index 50df785b63d..b72904b19b8 100644 --- a/libs/wire-api/src/Wire/API/Routes/Public/Galley.hs +++ b/libs/wire-api/src/Wire/API/Routes/Public/Galley.hs @@ -1361,6 +1361,7 @@ type MLSMessagingAPI = :> CanThrow 'LegalHoldNotEnabled :> CanThrow 'MLSClientMismatch :> CanThrow 'MLSCommitMissingReferences + :> CanThrow 'MLSExternalCommitRemoveProposal :> CanThrow 'MLSKeyPackageRefNotFound :> CanThrow 'MLSProposalNotFound :> CanThrow 'MLSProtocolErrorTag @@ -1370,6 +1371,8 @@ type MLSMessagingAPI = :> CanThrow 'MLSUnsupportedProposal :> CanThrow 'MLSClientSenderUserMismatch :> CanThrow 'MLSGroupConversationMismatch + :> CanThrow 'MLSMissingExternalInit + :> CanThrow 'MLSRemovalUserMismatch :> CanThrow 'MissingLegalholdConsent :> CanThrow MLSProposalFailure :> "messages" @@ -1387,6 +1390,7 @@ type MLSMessagingAPI = :> CanThrow 'LegalHoldNotEnabled :> CanThrow 'MLSClientMismatch :> CanThrow 'MLSCommitMissingReferences + :> CanThrow 'MLSExternalCommitRemoveProposal :> CanThrow 'MLSKeyPackageRefNotFound :> CanThrow 'MLSProposalNotFound :> CanThrow 'MLSProtocolErrorTag @@ -1396,6 +1400,8 @@ type MLSMessagingAPI = :> CanThrow 'MLSUnsupportedProposal :> CanThrow 'MLSClientSenderUserMismatch :> CanThrow 'MLSGroupConversationMismatch + :> CanThrow 'MLSMissingExternalInit + :> CanThrow 'MLSRemovalUserMismatch :> CanThrow 'MissingLegalholdConsent :> CanThrow MLSProposalFailure :> "messages" @@ -1413,6 +1419,7 @@ type MLSMessagingAPI = :> CanThrow 'LegalHoldNotEnabled :> CanThrow 'MLSClientMismatch :> CanThrow 'MLSCommitMissingReferences + :> CanThrow 'MLSExternalCommitRemoveProposal :> CanThrow 'MLSKeyPackageRefNotFound :> CanThrow 'MLSProposalNotFound :> CanThrow 'MLSProtocolErrorTag @@ -1423,6 +1430,8 @@ type MLSMessagingAPI = :> CanThrow 'MLSClientSenderUserMismatch :> CanThrow 'MLSGroupConversationMismatch :> CanThrow 'MLSWelcomeMismatch + :> CanThrow 'MLSMissingExternalInit + :> CanThrow 'MLSRemovalUserMismatch :> CanThrow 'MissingLegalholdConsent :> CanThrow MLSProposalFailure :> "commit-bundles" diff --git a/services/galley/src/Galley/API/MLS/Message.hs b/services/galley/src/Galley/API/MLS/Message.hs index bda3e0344ac..5709e4e0bf8 100644 --- a/services/galley/src/Galley/API/MLS/Message.hs +++ b/services/galley/src/Galley/API/MLS/Message.hs @@ -103,7 +103,10 @@ type MLSMessageStaticErrors = ErrorS 'MLSCommitMissingReferences, ErrorS 'MLSSelfRemovalNotAllowed, ErrorS 'MLSClientSenderUserMismatch, - ErrorS 'MLSGroupConversationMismatch + ErrorS 'MLSGroupConversationMismatch, + ErrorS 'MLSMissingExternalInit, + ErrorS 'MLSRemovalUserMismatch, + ErrorS 'MLSExternalCommitRemoveProposal ] type MLSBundleStaticErrors = @@ -119,14 +122,17 @@ postMLSMessageFromLocalUserV1 :: ErrorS 'ConvAccessDenied, ErrorS 'ConvMemberNotFound, ErrorS 'ConvNotFound, + ErrorS 'MissingLegalholdConsent, + ErrorS 'MLSClientSenderUserMismatch, ErrorS 'MLSCommitMissingReferences, + ErrorS 'MLSExternalCommitRemoveProposal, + ErrorS 'MLSGroupConversationMismatch, + ErrorS 'MLSMissingExternalInit, ErrorS 'MLSProposalNotFound, + ErrorS 'MLSRemovalUserMismatch, ErrorS 'MLSSelfRemovalNotAllowed, ErrorS 'MLSStaleMessage, ErrorS 'MLSUnsupportedMessage, - ErrorS 'MLSClientSenderUserMismatch, - ErrorS 'MLSGroupConversationMismatch, - ErrorS 'MissingLegalholdConsent, Input (Local ()), ProposalStore, Resource, @@ -152,14 +158,17 @@ postMLSMessageFromLocalUser :: ErrorS 'ConvAccessDenied, ErrorS 'ConvMemberNotFound, ErrorS 'ConvNotFound, + ErrorS 'MissingLegalholdConsent, + ErrorS 'MLSClientSenderUserMismatch, ErrorS 'MLSCommitMissingReferences, + ErrorS 'MLSExternalCommitRemoveProposal, + ErrorS 'MLSGroupConversationMismatch, + ErrorS 'MLSMissingExternalInit, ErrorS 'MLSProposalNotFound, + ErrorS 'MLSRemovalUserMismatch, ErrorS 'MLSSelfRemovalNotAllowed, ErrorS 'MLSStaleMessage, ErrorS 'MLSUnsupportedMessage, - ErrorS 'MLSClientSenderUserMismatch, - ErrorS 'MLSGroupConversationMismatch, - ErrorS 'MissingLegalholdConsent, Input (Local ()), ProposalStore, Resource, @@ -186,6 +195,9 @@ postMLSCommitBundle :: Error FederationError, Error InternalError, Error MLSProtocolError, + ErrorS 'MLSExternalCommitRemoveProposal, + ErrorS 'MLSMissingExternalInit, + ErrorS 'MLSRemovalUserMismatch, Input (Local ()), Input Opts, Input UTCTime, @@ -216,6 +228,9 @@ postMLSCommitBundleFromLocalUser :: '[ BrigAccess, Error FederationError, Error InternalError, + ErrorS 'MLSExternalCommitRemoveProposal, + ErrorS 'MLSMissingExternalInit, + ErrorS 'MLSRemovalUserMismatch, Input (Local ()), Input Opts, Input UTCTime, @@ -247,11 +262,13 @@ postMLSCommitBundleToLocalConv :: Error FederationError, Error InternalError, Error MLSProtocolError, + ErrorS 'MLSExternalCommitRemoveProposal, + ErrorS 'MLSMissingExternalInit, + ErrorS 'MLSRemovalUserMismatch, Input (Local ()), - Input UTCTime, Input Opts, + Input UTCTime, ProposalStore, - BrigAccess, Resource, TinyLog ] @@ -355,18 +372,21 @@ postMLSMessage :: ErrorS 'ConvAccessDenied, ErrorS 'ConvMemberNotFound, ErrorS 'ConvNotFound, + ErrorS 'MissingLegalholdConsent, ErrorS 'MLSClientSenderUserMismatch, ErrorS 'MLSCommitMissingReferences, + ErrorS 'MLSExternalCommitRemoveProposal, ErrorS 'MLSGroupConversationMismatch, + ErrorS 'MLSMissingExternalInit, ErrorS 'MLSProposalNotFound, + ErrorS 'MLSRemovalUserMismatch, ErrorS 'MLSSelfRemovalNotAllowed, ErrorS 'MLSStaleMessage, ErrorS 'MLSUnsupportedMessage, - ErrorS 'MissingLegalholdConsent, - Resource, - TinyLog, + Input (Local ()), ProposalStore, - Input (Local ()) + Resource, + TinyLog ] r ) => @@ -418,16 +438,20 @@ postMLSMessageToLocalConv :: '[ Error FederationError, Error InternalError, ErrorS 'ConvNotFound, - ErrorS 'MLSUnsupportedMessage, - ErrorS 'MLSStaleMessage, - ErrorS 'MLSProposalNotFound, ErrorS 'MissingLegalholdConsent, + ErrorS 'MLSClientSenderUserMismatch, ErrorS 'MLSCommitMissingReferences, + ErrorS 'MLSExternalCommitRemoveProposal, + ErrorS 'MLSMissingExternalInit, + ErrorS 'MLSProposalNotFound, + ErrorS 'MLSRemovalUserMismatch, ErrorS 'MLSSelfRemovalNotAllowed, - Resource, - TinyLog, + ErrorS 'MLSStaleMessage, + ErrorS 'MLSUnsupportedMessage, + Input (Local ()), ProposalStore, - Input (Local ()) + Resource, + TinyLog ] r ) => @@ -526,17 +550,21 @@ type HasProposalEffects r = data ProposalAction = ProposalAction { paAdd :: ClientMap, - paRemove :: ClientMap + paRemove :: ClientMap, + -- The backend does not process external init proposals, but still it needs + -- to know if a commit has one when processing external commits + paExternalInit :: Maybe () } instance Semigroup ProposalAction where - ProposalAction add1 rem1 <> ProposalAction add2 rem2 = + ProposalAction add1 rem1 init1 <> ProposalAction add2 rem2 init2 = ProposalAction (Map.unionWith mappend add1 add2) (Map.unionWith mappend rem1 rem2) + (init1 <> init2) instance Monoid ProposalAction where - mempty = ProposalAction mempty mempty + mempty = ProposalAction mempty mempty mempty paAddClient :: Qualified (UserId, (ClientId, KeyPackageRef)) -> ProposalAction paAddClient quc = mempty {paAdd = Map.singleton (fmap fst quc) (Set.singleton (snd (qUnqualified quc)))} @@ -544,6 +572,9 @@ paAddClient quc = mempty {paAdd = Map.singleton (fmap fst quc) (Set.singleton (s paRemoveClient :: Qualified (UserId, (ClientId, KeyPackageRef)) -> ProposalAction paRemoveClient quc = mempty {paRemove = Map.singleton (fmap fst quc) (Set.singleton (snd (qUnqualified quc)))} +paExternalInitPresent :: ProposalAction +paExternalInitPresent = mempty {paExternalInit = Just ()} + getCommitData :: ( HasProposalEffects r, Member (ErrorS 'ConvNotFound) r, @@ -578,8 +609,12 @@ processCommit :: Member (Error FederationError) r, Member (Error InternalError) r, Member (ErrorS 'ConvNotFound) r, + Member (ErrorS 'MLSClientSenderUserMismatch) r, Member (ErrorS 'MLSCommitMissingReferences) r, + Member (ErrorS 'MLSExternalCommitRemoveProposal) r, + Member (ErrorS 'MLSMissingExternalInit) r, Member (ErrorS 'MLSProposalNotFound) r, + Member (ErrorS 'MLSRemovalUserMismatch) r, Member (ErrorS 'MLSSelfRemovalNotAllowed) r, Member (ErrorS 'MLSStaleMessage) r, Member (ErrorS 'MissingLegalholdConsent) r, @@ -602,12 +637,17 @@ processCommit qusr senderClient con lconv cm epoch sender commit = do processCommitWithAction qusr senderClient con lconv cm epoch groupId action sender Nothing commit processCommitWithAction :: + forall r. ( HasProposalEffects r, Member (Error FederationError) r, Member (Error InternalError) r, Member (ErrorS 'ConvNotFound) r, + Member (ErrorS 'MLSClientSenderUserMismatch) r, Member (ErrorS 'MLSCommitMissingReferences) r, + Member (ErrorS 'MLSExternalCommitRemoveProposal) r, + Member (ErrorS 'MLSMissingExternalInit) r, Member (ErrorS 'MLSProposalNotFound) r, + Member (ErrorS 'MLSRemovalUserMismatch) r, Member (ErrorS 'MLSSelfRemovalNotAllowed) r, Member (ErrorS 'MLSStaleMessage) r, Member (ErrorS 'MissingLegalholdConsent) r, @@ -634,7 +674,7 @@ processCommitWithAction qusr senderClient con lconv cm epoch groupId action send let ttlSeconds :: Int = 600 -- 10 minutes withCommitLock groupId epoch (fromIntegral ttlSeconds) $ do checkEpoch epoch (tUnqualified lconv) - postponedKeyPackageRefUpdate <- + (postponedKeyPackageRefUpdate, actionWithUpdate) <- if epoch == Epoch 0 then do -- this is a newly created conversation, and it should contain exactly one @@ -660,25 +700,57 @@ processCommitWithAction qusr senderClient con lconv cm epoch groupId action send throw (InternalErrorWithDescription "Unexpected creator client set") -- the sender of the first commit must be a member _ -> throw (mlsProtocolError "Unexpected sender") - pure $ pure () -- no key package ref update necessary + pure $ (pure (), action) -- no key package ref update necessary else case (sender, upLeaf <$> cPath commit) of (MemberSender senderRef, Just updatedKeyPackage) -> do updatedRef <- kpRef' updatedKeyPackage & note (mlsProtocolError "Could not compute key package ref") -- postpone key package ref update until other checks/processing passed case senderClient of - Just cli -> pure $ updateKeyPackageMapping lconv qusr cli (Just senderRef) updatedRef - Nothing -> pure $ pure () - (_, Nothing) -> pure $ pure () -- ignore commits without update path + Just cli -> pure (updateKeyPackageMapping lconv qusr cli (Just senderRef) updatedRef, action) + Nothing -> pure (pure (), action) + (_, Nothing) -> pure (pure (), action) -- ignore commits without update path + (NewMemberSender, Just newKeyPackage) -> do + -- this is an external commit + when (paExternalInit action == mempty) $ + throwS @'MLSMissingExternalInit + unless (paAdd action == mempty) $ + throwS @'MLSExternalCommitRemoveProposal + + cid <- case kpIdentity (rmValue newKeyPackage) of + Left e -> throw (mlsProtocolError $ "Failed to parse the client identity: " <> e) + Right v -> pure v + newRef <- + kpRef' newKeyPackage + & note (mlsProtocolError "An invalid key package in the update path") + + -- check if there is a key package ref in the remove proposal + remRef <- + if Map.null (paRemove action) + then pure Nothing + else do + (remCid, r) <- derefUser (paRemove action) qusr + unless (cidQualifiedUser cid == cidQualifiedUser remCid) $ + throwS @'MLSRemovalUserMismatch + pure (Just r) + + updateKeyPackageMapping lconv qusr (ciClient cid) remRef newRef + + pure (pure (), action {paRemove = mempty}) _ -> throw (mlsProtocolError "Unexpected sender") - -- check all pending proposals are referenced in the commit - allPendingProposals <- getAllPendingProposals groupId epoch - let referencedProposals = Set.fromList $ mapMaybe (\x -> preview Proposal._Ref x) (cProposals commit) - unless (all (`Set.member` referencedProposals) allPendingProposals) $ - throwS @'MLSCommitMissingReferences + -- FUTUREWORK: Resubmit backend-provided proposals when processing an + -- external commit. + -- + -- check all pending proposals are referenced in the commit. Skip the check + -- if this is an external commit. + when (sender /= NewMemberSender) $ do + allPendingProposals <- getAllPendingProposals groupId epoch + let referencedProposals = Set.fromList $ mapMaybe (\x -> preview Proposal._Ref x) (cProposals commit) + unless (all (`Set.member` referencedProposals) allPendingProposals) $ + throwS @'MLSCommitMissingReferences -- process and execute proposals - updates <- executeProposalAction qusr con lconv cm action + updates <- executeProposalAction qusr con lconv cm actionWithUpdate -- update key package ref if necessary postponedKeyPackageRefUpdate @@ -691,6 +763,22 @@ processCommitWithAction qusr senderClient con lconv cm epoch groupId action send . gipGroupState pure updates + where + derefUser :: ClientMap -> Qualified UserId -> Sem r (ClientIdentity, KeyPackageRef) + derefUser (Map.toList -> l) user = case l of + [(u, s)] -> do + unless (user == u) $ + throwS @'MLSClientSenderUserMismatch + ref <- snd <$> ensureSingleton s + ci <- derefKeyPackage ref + unless (cidQualifiedUser ci == user) $ + throwS @'MLSClientSenderUserMismatch + pure (ci, ref) + _ -> throwS @'MLSExternalCommitRemoveProposal + ensureSingleton :: Set a -> Sem r a + ensureSingleton (Set.toList -> l) = case l of + [e] -> pure e + _ -> throwS @'MLSExternalCommitRemoveProposal -- | Note: Use this only for KeyPackage that are already validated updateKeyPackageMapping :: @@ -784,6 +872,10 @@ applyProposal convId (AddProposal kp) = do applyProposal _conv (RemoveProposal ref) = do qclient <- cidQualifiedClient <$> derefKeyPackage ref pure (paRemoveClient ((,ref) <$$> qclient)) +applyProposal _conv (ExternalInitProposal _) = + -- only record the fact there was an external init proposal, but do not + -- process it in any way. + pure paExternalInitPresent applyProposal _conv _ = pure mempty checkProposalCipherSuite :: From 3cece551e24f87b277e548c7bf988f06ba2e6741 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Thu, 20 Oct 2022 14:02:22 +0200 Subject: [PATCH 02/11] Fix the serialiseMLS instance for `Sender 'MLSPlainText` --- libs/wire-api/src/Wire/API/MLS/Message.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/wire-api/src/Wire/API/MLS/Message.hs b/libs/wire-api/src/Wire/API/MLS/Message.hs index 28594f36237..2393aa06e2a 100644 --- a/libs/wire-api/src/Wire/API/MLS/Message.hs +++ b/libs/wire-api/src/Wire/API/MLS/Message.hs @@ -266,7 +266,7 @@ instance SerialiseMLS (Sender 'MLSPlainText) where serialiseMLS (PreconfiguredSender x) = do serialiseMLS PreconfiguredSenderTag put x - serialiseMLS NewMemberSender = serialiseMLS NewMemberSender + serialiseMLS NewMemberSender = serialiseMLS NewMemberSenderTag data family MessagePayload (tag :: WireFormatTag) :: * From a7b32f6e1e4c2a31fd1bd9423853742e7cebbc74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Tue, 25 Oct 2022 16:34:54 +0200 Subject: [PATCH 03/11] Update the mls-test-cli reference --- nix/pkgs/mls-test-cli/default.nix | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/nix/pkgs/mls-test-cli/default.nix b/nix/pkgs/mls-test-cli/default.nix index a9a9657d731..7d7d6961133 100644 --- a/nix/pkgs/mls-test-cli/default.nix +++ b/nix/pkgs/mls-test-cli/default.nix @@ -15,11 +15,11 @@ rustPlatform.buildRustPackage rec { src = fetchFromGitHub { owner = "wireapp"; repo = "mls-test-cli"; - sha256 = "sha256-xYL9KNcirCARb1Rp41einOpq0ut5adlqMIAEiwYXkzg="; - rev = "d46624fb49c900facc8853fa86e3ecf51fd0dcdb"; + sha256 = "sha256-/XQ/9oQTPkRqgMzDGRm+Oh9jgkdeDM1vRJ6/wEf2+bY="; + rev = "c6f80be2839ac1ed2894e96044541d1c3cf6ecdf"; }; doCheck = false; - cargoSha256 = "sha256-FGFyS/tLlD+3JQX7vkKq4nW+WQI1FFnpugzfFBi/eQE="; + cargoSha256 = "sha256-AlZrxa7f5JwxxrzFBgeFSaYU6QttsUpfLYfq1HzsdbE="; cargoDepsHook = '' mkdir -p mls-test-cli-${version}-vendor.tar.gz/ring/.git ''; From 873b0ee86e69c6c38681a714d9c2a225287760f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Fri, 14 Oct 2022 15:16:32 +0200 Subject: [PATCH 04/11] Integration tests for external commits --- services/galley/test/integration/API/MLS.hs | 66 ++++++++++++++++++- .../galley/test/integration/API/MLS/Util.hs | 49 +++++++++++++- 2 files changed, 113 insertions(+), 2 deletions(-) diff --git a/services/galley/test/integration/API/MLS.hs b/services/galley/test/integration/API/MLS.hs index 8aa9bfb2941..38b5e7ee3cc 100644 --- a/services/galley/test/integration/API/MLS.hs +++ b/services/galley/test/integration/API/MLS.hs @@ -106,11 +106,17 @@ tests s = test s "add remote user to a conversation" testAddRemoteUser, test s "return error when commit is locked" testCommitLock, test s "add user to a conversation with proposal + commit" testAddUserBareProposalCommit, - test s "post commit that references a unknown proposal" testUnknownProposalRefCommit, + test s "post commit that references an unknown proposal" testUnknownProposalRefCommit, test s "post commit that is not referencing all proposals" testCommitNotReferencingAllProposals, test s "admin removes user from a conversation" testAdminRemovesUserFromConv, test s "admin removes user from a conversation but doesn't list all clients" testRemoveClientsIncomplete ], + testGroup + "External commit" + [ test s "non-member attempts to join a conversation" testExternalCommitNotMember, + test s "join a conversation with the same client" testExternalCommitSameClient, + test s "join a conversation with a new client" testExternalCommitNewClient + ], testGroup "Application Message" [ testGroup @@ -948,6 +954,64 @@ testLocalToRemoteNonMember = do const (Just "no-conversation-member") === fmap Wai.label . responseJsonError +testExternalCommitNotMember :: TestM () +testExternalCommitNotMember = do + [alice, bob] <- createAndConnectUsers (replicate 2 Nothing) + + runMLSTest $ do + [alice1, alice2, bob1] <- traverse createMLSClient [alice, alice, bob] + traverse_ uploadNewKeyPackage [bob1, alice2] + (_, qcnv) <- setupMLSGroup alice1 + + -- so that we have the public group state + void $ createAddCommit alice1 [alice] >>= sendAndConsumeCommitBundle + + pgs <- + LBS.toStrict . fromJust . responseBody + <$> getGroupInfo (ciUser alice1) qcnv + mp <- createExternalCommit bob1 (Just pgs) qcnv + bundle <- createBundle mp + postCommitBundle (ciUser (mpSender mp)) bundle + !!! const 404 === statusCode + +testExternalCommitSameClient :: TestM () +testExternalCommitSameClient = do + [alice, bob] <- createAndConnectUsers (replicate 2 Nothing) + + runMLSTest $ do + [alice1, bob1] <- traverse createMLSClient [alice, bob] + void $ uploadNewKeyPackage bob1 + (_, qcnv) <- setupMLSGroup alice1 + void $ createAddCommit alice1 [bob] >>= sendAndConsumeCommitBundle + + let rejoiner = alice1 + ecEvents <- createExternalCommit rejoiner Nothing qcnv >>= sendAndConsumeCommitBundle + liftIO $ + assertBool "No events after external commit expected" (null ecEvents) + + message <- createApplicationMessage bob1 "hello" + void $ sendAndConsumeMessage message + +testExternalCommitNewClient :: TestM () +testExternalCommitNewClient = do + [alice, bob] <- createAndConnectUsers (replicate 2 Nothing) + + runMLSTest $ do + [alice1, bob1] <- traverse createMLSClient [alice, bob] + void $ uploadNewKeyPackage bob1 + (_, qcnv) <- setupMLSGroup alice1 + void $ createAddCommit alice1 [bob] >>= sendAndConsumeCommitBundle + + nc <- createMLSClient bob + ecEvents <- createExternalCommit nc Nothing qcnv >>= sendAndConsumeCommitBundle + liftIO $ + assertBool "No events after external commit expected" (null ecEvents) + + message <- createApplicationMessage nc "hello" + void $ sendAndConsumeMessage message + +-- the list of members should be [alice1, bob1] + testAppMessage :: TestM () testAppMessage = do users@(alice : _) <- createAndConnectUsers (replicate 4 Nothing) diff --git a/services/galley/test/integration/API/MLS/Util.hs b/services/galley/test/integration/API/MLS/Util.hs index 48dee48ed1f..174cabdb964 100644 --- a/services/galley/test/integration/API/MLS/Util.hs +++ b/services/galley/test/integration/API/MLS/Util.hs @@ -35,6 +35,7 @@ import qualified Data.ByteArray as BA import qualified Data.ByteString as BS import qualified Data.ByteString.Base64.URL as B64U import Data.ByteString.Conversion +import qualified Data.ByteString.Lazy as LBS import Data.Domain import Data.Hex import Data.Id @@ -257,7 +258,9 @@ mlscli :: HasCallStack => ClientIdentity -> [String] -> Maybe ByteString -> MLST mlscli qcid args mbstdin = do bd <- State.gets mlsBaseDir let cdir = bd cid2Str qcid - liftIO $ spawn (proc "mls-test-cli" (["--store", cdir "store"] <> args)) mbstdin + liftIO $ do + -- Imports.setEnv "RUST_BACKTRACE" "full" + spawn (proc "mls-test-cli" (["--store", cdir "store"] <> args)) mbstdin createWireClient :: HasCallStack => Qualified UserId -> MLSTest ClientIdentity createWireClient qusr = do @@ -510,6 +513,50 @@ createAddCommit cid users = do kps <- concat <$> traverse (bundleKeyPackages <=< claimKeyPackages cid) users createAddCommitWithKeyPackages cid kps +createExternalCommit :: + HasCallStack => + ClientIdentity -> + Maybe ByteString -> + Qualified ConvId -> + MLSTest MessagePackage +createExternalCommit qcid mpgs qcnv = do + bd <- State.gets mlsBaseDir + gNew <- nextGroupFile qcid + pgsFile <- liftIO $ emptyTempFile bd "pgs" + pgs <- case mpgs of + Nothing -> + LBS.toStrict . fromJust . responseBody + <$> getGroupInfo (ciUser qcid) qcnv + Just v -> pure v + commit <- + mlscli + qcid + [ "external-commit", + "--group-state-in", + "-", + "--group-state-out", + pgsFile, + "--group-out", + gNew + ] + (Just pgs) + + State.modify $ \mls -> + mls + { mlsNewMembers = Set.singleton qcid -- This might be a different client + -- than those that have been in the + -- group from before. + } + + newPgs <- liftIO $ BS.readFile pgsFile + pure $ + MessagePackage + { mpSender = qcid, + mpMessage = commit, + mpWelcome = Nothing, + mpPublicGroupState = Just newPgs + } + createAddProposals :: HasCallStack => ClientIdentity -> [Qualified UserId] -> MLSTest [MessagePackage] createAddProposals cid users = do kps <- concat <$> traverse (bundleKeyPackages <=< claimKeyPackages cid) users From c69966d5cf271ab29fde975f62f34d3af30c9660 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Wed, 26 Oct 2022 11:11:57 +0200 Subject: [PATCH 05/11] Add a changelog --- changelog.d/2-features/mls-external-commits | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/2-features/mls-external-commits diff --git a/changelog.d/2-features/mls-external-commits b/changelog.d/2-features/mls-external-commits new file mode 100644 index 00000000000..ae05f3d41b2 --- /dev/null +++ b/changelog.d/2-features/mls-external-commits @@ -0,0 +1 @@ +Introduce support for external commits in MLS From 0224a420dd6dfd030aa6604310d21c42baf486cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Fri, 28 Oct 2022 10:02:10 +0200 Subject: [PATCH 06/11] Use MLSProtocolError instead of specific errors --- libs/wire-api/src/Wire/API/Error/Galley.hs | 9 ---- .../src/Wire/API/Routes/Public/Galley.hs | 9 ---- services/galley/src/Galley/API/MLS/Message.hs | 51 +++++-------------- 3 files changed, 12 insertions(+), 57 deletions(-) diff --git a/libs/wire-api/src/Wire/API/Error/Galley.hs b/libs/wire-api/src/Wire/API/Error/Galley.hs index 824b084f234..171f894b3d5 100644 --- a/libs/wire-api/src/Wire/API/Error/Galley.hs +++ b/libs/wire-api/src/Wire/API/Error/Galley.hs @@ -83,9 +83,6 @@ data GalleyError | MLSClientSenderUserMismatch | MLSWelcomeMismatch | MLSMissingGroupInfo - | MLSMissingExternalInit - | MLSRemovalUserMismatch - | MLSExternalCommitRemoveProposal | -- NoBindingTeamMembers | NoBindingTeam @@ -209,12 +206,6 @@ type instance MapError 'MLSWelcomeMismatch = 'StaticError 400 "mls-welcome-misma type instance MapError 'MLSMissingGroupInfo = 'StaticError 404 "mls-missing-group-info" "The conversation has no group information" -type instance MapError 'MLSMissingExternalInit = 'StaticError 400 "mls-missing-external-init" "The external commit is missing an external init proposal" - -type instance MapError 'MLSRemovalUserMismatch = 'StaticError 403 "mls-removal-user-mismatch" "The external commit attempts to remove a client from a user other than themselves" - -type instance MapError 'MLSExternalCommitRemoveProposal = 'StaticError 403 "mls-external-commit-proposals" "The external commit has to have at most one remove proposal" - type instance MapError 'NoBindingTeamMembers = 'StaticError 403 "non-binding-team-members" "Both users must be members of the same binding team" type instance MapError 'NoBindingTeam = 'StaticError 403 "no-binding-team" "Operation allowed only on binding teams" diff --git a/libs/wire-api/src/Wire/API/Routes/Public/Galley.hs b/libs/wire-api/src/Wire/API/Routes/Public/Galley.hs index b72904b19b8..50df785b63d 100644 --- a/libs/wire-api/src/Wire/API/Routes/Public/Galley.hs +++ b/libs/wire-api/src/Wire/API/Routes/Public/Galley.hs @@ -1361,7 +1361,6 @@ type MLSMessagingAPI = :> CanThrow 'LegalHoldNotEnabled :> CanThrow 'MLSClientMismatch :> CanThrow 'MLSCommitMissingReferences - :> CanThrow 'MLSExternalCommitRemoveProposal :> CanThrow 'MLSKeyPackageRefNotFound :> CanThrow 'MLSProposalNotFound :> CanThrow 'MLSProtocolErrorTag @@ -1371,8 +1370,6 @@ type MLSMessagingAPI = :> CanThrow 'MLSUnsupportedProposal :> CanThrow 'MLSClientSenderUserMismatch :> CanThrow 'MLSGroupConversationMismatch - :> CanThrow 'MLSMissingExternalInit - :> CanThrow 'MLSRemovalUserMismatch :> CanThrow 'MissingLegalholdConsent :> CanThrow MLSProposalFailure :> "messages" @@ -1390,7 +1387,6 @@ type MLSMessagingAPI = :> CanThrow 'LegalHoldNotEnabled :> CanThrow 'MLSClientMismatch :> CanThrow 'MLSCommitMissingReferences - :> CanThrow 'MLSExternalCommitRemoveProposal :> CanThrow 'MLSKeyPackageRefNotFound :> CanThrow 'MLSProposalNotFound :> CanThrow 'MLSProtocolErrorTag @@ -1400,8 +1396,6 @@ type MLSMessagingAPI = :> CanThrow 'MLSUnsupportedProposal :> CanThrow 'MLSClientSenderUserMismatch :> CanThrow 'MLSGroupConversationMismatch - :> CanThrow 'MLSMissingExternalInit - :> CanThrow 'MLSRemovalUserMismatch :> CanThrow 'MissingLegalholdConsent :> CanThrow MLSProposalFailure :> "messages" @@ -1419,7 +1413,6 @@ type MLSMessagingAPI = :> CanThrow 'LegalHoldNotEnabled :> CanThrow 'MLSClientMismatch :> CanThrow 'MLSCommitMissingReferences - :> CanThrow 'MLSExternalCommitRemoveProposal :> CanThrow 'MLSKeyPackageRefNotFound :> CanThrow 'MLSProposalNotFound :> CanThrow 'MLSProtocolErrorTag @@ -1430,8 +1423,6 @@ type MLSMessagingAPI = :> CanThrow 'MLSClientSenderUserMismatch :> CanThrow 'MLSGroupConversationMismatch :> CanThrow 'MLSWelcomeMismatch - :> CanThrow 'MLSMissingExternalInit - :> CanThrow 'MLSRemovalUserMismatch :> CanThrow 'MissingLegalholdConsent :> CanThrow MLSProposalFailure :> "commit-bundles" diff --git a/services/galley/src/Galley/API/MLS/Message.hs b/services/galley/src/Galley/API/MLS/Message.hs index 5709e4e0bf8..be0450a53a3 100644 --- a/services/galley/src/Galley/API/MLS/Message.hs +++ b/services/galley/src/Galley/API/MLS/Message.hs @@ -103,10 +103,7 @@ type MLSMessageStaticErrors = ErrorS 'MLSCommitMissingReferences, ErrorS 'MLSSelfRemovalNotAllowed, ErrorS 'MLSClientSenderUserMismatch, - ErrorS 'MLSGroupConversationMismatch, - ErrorS 'MLSMissingExternalInit, - ErrorS 'MLSRemovalUserMismatch, - ErrorS 'MLSExternalCommitRemoveProposal + ErrorS 'MLSGroupConversationMismatch ] type MLSBundleStaticErrors = @@ -125,11 +122,8 @@ postMLSMessageFromLocalUserV1 :: ErrorS 'MissingLegalholdConsent, ErrorS 'MLSClientSenderUserMismatch, ErrorS 'MLSCommitMissingReferences, - ErrorS 'MLSExternalCommitRemoveProposal, ErrorS 'MLSGroupConversationMismatch, - ErrorS 'MLSMissingExternalInit, ErrorS 'MLSProposalNotFound, - ErrorS 'MLSRemovalUserMismatch, ErrorS 'MLSSelfRemovalNotAllowed, ErrorS 'MLSStaleMessage, ErrorS 'MLSUnsupportedMessage, @@ -161,11 +155,8 @@ postMLSMessageFromLocalUser :: ErrorS 'MissingLegalholdConsent, ErrorS 'MLSClientSenderUserMismatch, ErrorS 'MLSCommitMissingReferences, - ErrorS 'MLSExternalCommitRemoveProposal, ErrorS 'MLSGroupConversationMismatch, - ErrorS 'MLSMissingExternalInit, ErrorS 'MLSProposalNotFound, - ErrorS 'MLSRemovalUserMismatch, ErrorS 'MLSSelfRemovalNotAllowed, ErrorS 'MLSStaleMessage, ErrorS 'MLSUnsupportedMessage, @@ -195,9 +186,6 @@ postMLSCommitBundle :: Error FederationError, Error InternalError, Error MLSProtocolError, - ErrorS 'MLSExternalCommitRemoveProposal, - ErrorS 'MLSMissingExternalInit, - ErrorS 'MLSRemovalUserMismatch, Input (Local ()), Input Opts, Input UTCTime, @@ -228,9 +216,6 @@ postMLSCommitBundleFromLocalUser :: '[ BrigAccess, Error FederationError, Error InternalError, - ErrorS 'MLSExternalCommitRemoveProposal, - ErrorS 'MLSMissingExternalInit, - ErrorS 'MLSRemovalUserMismatch, Input (Local ()), Input Opts, Input UTCTime, @@ -262,9 +247,6 @@ postMLSCommitBundleToLocalConv :: Error FederationError, Error InternalError, Error MLSProtocolError, - ErrorS 'MLSExternalCommitRemoveProposal, - ErrorS 'MLSMissingExternalInit, - ErrorS 'MLSRemovalUserMismatch, Input (Local ()), Input Opts, Input UTCTime, @@ -375,11 +357,8 @@ postMLSMessage :: ErrorS 'MissingLegalholdConsent, ErrorS 'MLSClientSenderUserMismatch, ErrorS 'MLSCommitMissingReferences, - ErrorS 'MLSExternalCommitRemoveProposal, ErrorS 'MLSGroupConversationMismatch, - ErrorS 'MLSMissingExternalInit, ErrorS 'MLSProposalNotFound, - ErrorS 'MLSRemovalUserMismatch, ErrorS 'MLSSelfRemovalNotAllowed, ErrorS 'MLSStaleMessage, ErrorS 'MLSUnsupportedMessage, @@ -441,10 +420,7 @@ postMLSMessageToLocalConv :: ErrorS 'MissingLegalholdConsent, ErrorS 'MLSClientSenderUserMismatch, ErrorS 'MLSCommitMissingReferences, - ErrorS 'MLSExternalCommitRemoveProposal, - ErrorS 'MLSMissingExternalInit, ErrorS 'MLSProposalNotFound, - ErrorS 'MLSRemovalUserMismatch, ErrorS 'MLSSelfRemovalNotAllowed, ErrorS 'MLSStaleMessage, ErrorS 'MLSUnsupportedMessage, @@ -611,10 +587,7 @@ processCommit :: Member (ErrorS 'ConvNotFound) r, Member (ErrorS 'MLSClientSenderUserMismatch) r, Member (ErrorS 'MLSCommitMissingReferences) r, - Member (ErrorS 'MLSExternalCommitRemoveProposal) r, - Member (ErrorS 'MLSMissingExternalInit) r, Member (ErrorS 'MLSProposalNotFound) r, - Member (ErrorS 'MLSRemovalUserMismatch) r, Member (ErrorS 'MLSSelfRemovalNotAllowed) r, Member (ErrorS 'MLSStaleMessage) r, Member (ErrorS 'MissingLegalholdConsent) r, @@ -644,10 +617,7 @@ processCommitWithAction :: Member (ErrorS 'ConvNotFound) r, Member (ErrorS 'MLSClientSenderUserMismatch) r, Member (ErrorS 'MLSCommitMissingReferences) r, - Member (ErrorS 'MLSExternalCommitRemoveProposal) r, - Member (ErrorS 'MLSMissingExternalInit) r, Member (ErrorS 'MLSProposalNotFound) r, - Member (ErrorS 'MLSRemovalUserMismatch) r, Member (ErrorS 'MLSSelfRemovalNotAllowed) r, Member (ErrorS 'MLSStaleMessage) r, Member (ErrorS 'MissingLegalholdConsent) r, @@ -711,10 +681,12 @@ processCommitWithAction qusr senderClient con lconv cm epoch groupId action send (_, Nothing) -> pure (pure (), action) -- ignore commits without update path (NewMemberSender, Just newKeyPackage) -> do -- this is an external commit - when (paExternalInit action == mempty) $ - throwS @'MLSMissingExternalInit - unless (paAdd action == mempty) $ - throwS @'MLSExternalCommitRemoveProposal + when (paExternalInit action == mempty) . + throw . mlsProtocolError $ + "The external commit is missing an external init proposal" + unless (paAdd action == mempty) . + throw . mlsProtocolError $ + "The external commit must not have add proposals" cid <- case kpIdentity (rmValue newKeyPackage) of Left e -> throw (mlsProtocolError $ "Failed to parse the client identity: " <> e) @@ -729,8 +701,9 @@ processCommitWithAction qusr senderClient con lconv cm epoch groupId action send then pure Nothing else do (remCid, r) <- derefUser (paRemove action) qusr - unless (cidQualifiedUser cid == cidQualifiedUser remCid) $ - throwS @'MLSRemovalUserMismatch + unless (cidQualifiedUser cid == cidQualifiedUser remCid) . + throw . mlsProtocolError $ + "The external commit attempts to remove a client from a user other than themselves" pure (Just r) updateKeyPackageMapping lconv qusr (ciClient cid) remRef newRef @@ -774,11 +747,11 @@ processCommitWithAction qusr senderClient con lconv cm epoch groupId action send unless (cidQualifiedUser ci == user) $ throwS @'MLSClientSenderUserMismatch pure (ci, ref) - _ -> throwS @'MLSExternalCommitRemoveProposal + _ -> throw . mlsProtocolError $ "The external commit must have at most one remove proposal" ensureSingleton :: Set a -> Sem r a ensureSingleton (Set.toList -> l) = case l of [e] -> pure e - _ -> throwS @'MLSExternalCommitRemoveProposal + _ -> throw . mlsProtocolError $ "The external commit must have at most one remove proposal" -- | Note: Use this only for KeyPackage that are already validated updateKeyPackageMapping :: From 20162df1610df92973876b3fdbb30b523a437a7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Fri, 28 Oct 2022 10:12:00 +0200 Subject: [PATCH 07/11] Replace `Maybe ()` with `Any` --- services/galley/src/Galley/API/MLS/Message.hs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/galley/src/Galley/API/MLS/Message.hs b/services/galley/src/Galley/API/MLS/Message.hs index be0450a53a3..3a4d50435f8 100644 --- a/services/galley/src/Galley/API/MLS/Message.hs +++ b/services/galley/src/Galley/API/MLS/Message.hs @@ -529,7 +529,7 @@ data ProposalAction = ProposalAction paRemove :: ClientMap, -- The backend does not process external init proposals, but still it needs -- to know if a commit has one when processing external commits - paExternalInit :: Maybe () + paExternalInit :: Any } instance Semigroup ProposalAction where @@ -549,7 +549,7 @@ paRemoveClient :: Qualified (UserId, (ClientId, KeyPackageRef)) -> ProposalActio paRemoveClient quc = mempty {paRemove = Map.singleton (fmap fst quc) (Set.singleton (snd (qUnqualified quc)))} paExternalInitPresent :: ProposalAction -paExternalInitPresent = mempty {paExternalInit = Just ()} +paExternalInitPresent = mempty {paExternalInit = Any True} getCommitData :: ( HasProposalEffects r, From 9e1087534d7f12f2ab0eda50b52de63f5febd5ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Fri, 28 Oct 2022 10:14:43 +0200 Subject: [PATCH 08/11] Remove debugging code --- services/galley/test/integration/API/MLS/Util.hs | 1 - 1 file changed, 1 deletion(-) diff --git a/services/galley/test/integration/API/MLS/Util.hs b/services/galley/test/integration/API/MLS/Util.hs index 174cabdb964..18a64c12a02 100644 --- a/services/galley/test/integration/API/MLS/Util.hs +++ b/services/galley/test/integration/API/MLS/Util.hs @@ -259,7 +259,6 @@ mlscli qcid args mbstdin = do bd <- State.gets mlsBaseDir let cdir = bd cid2Str qcid liftIO $ do - -- Imports.setEnv "RUST_BACKTRACE" "full" spawn (proc "mls-test-cli" (["--store", cdir "store"] <> args)) mbstdin createWireClient :: HasCallStack => Qualified UserId -> MLSTest ClientIdentity From 345014587cac9c8932af7d9bff422b8ef985d26f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Fri, 28 Oct 2022 10:19:04 +0200 Subject: [PATCH 09/11] Apply formatting --- services/galley/src/Galley/API/MLS/Message.hs | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/services/galley/src/Galley/API/MLS/Message.hs b/services/galley/src/Galley/API/MLS/Message.hs index 3a4d50435f8..bf6cf822153 100644 --- a/services/galley/src/Galley/API/MLS/Message.hs +++ b/services/galley/src/Galley/API/MLS/Message.hs @@ -681,12 +681,14 @@ processCommitWithAction qusr senderClient con lconv cm epoch groupId action send (_, Nothing) -> pure (pure (), action) -- ignore commits without update path (NewMemberSender, Just newKeyPackage) -> do -- this is an external commit - when (paExternalInit action == mempty) . - throw . mlsProtocolError $ - "The external commit is missing an external init proposal" - unless (paAdd action == mempty) . - throw . mlsProtocolError $ - "The external commit must not have add proposals" + when (paExternalInit action == mempty) + . throw + . mlsProtocolError + $ "The external commit is missing an external init proposal" + unless (paAdd action == mempty) + . throw + . mlsProtocolError + $ "The external commit must not have add proposals" cid <- case kpIdentity (rmValue newKeyPackage) of Left e -> throw (mlsProtocolError $ "Failed to parse the client identity: " <> e) @@ -701,9 +703,10 @@ processCommitWithAction qusr senderClient con lconv cm epoch groupId action send then pure Nothing else do (remCid, r) <- derefUser (paRemove action) qusr - unless (cidQualifiedUser cid == cidQualifiedUser remCid) . - throw . mlsProtocolError $ - "The external commit attempts to remove a client from a user other than themselves" + unless (cidQualifiedUser cid == cidQualifiedUser remCid) + . throw + . mlsProtocolError + $ "The external commit attempts to remove a client from a user other than themselves" pure (Just r) updateKeyPackageMapping lconv qusr (ciClient cid) remRef newRef From be9381c75e046878760f99e3a4003afdc13251d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Fri, 28 Oct 2022 11:47:15 +0200 Subject: [PATCH 10/11] fixup! Use MLSProtocolError instead of specific errors --- services/galley/src/Galley/API/MLS/Message.hs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/services/galley/src/Galley/API/MLS/Message.hs b/services/galley/src/Galley/API/MLS/Message.hs index bf6cf822153..d37af5b920c 100644 --- a/services/galley/src/Galley/API/MLS/Message.hs +++ b/services/galley/src/Galley/API/MLS/Message.hs @@ -740,6 +740,9 @@ processCommitWithAction qusr senderClient con lconv cm epoch groupId action send pure updates where + throwRemProposal = + throw . mlsProtocolError $ + "The external commit must have at most one remove proposal" derefUser :: ClientMap -> Qualified UserId -> Sem r (ClientIdentity, KeyPackageRef) derefUser (Map.toList -> l) user = case l of [(u, s)] -> do @@ -750,11 +753,11 @@ processCommitWithAction qusr senderClient con lconv cm epoch groupId action send unless (cidQualifiedUser ci == user) $ throwS @'MLSClientSenderUserMismatch pure (ci, ref) - _ -> throw . mlsProtocolError $ "The external commit must have at most one remove proposal" + _ -> throwRemProposal ensureSingleton :: Set a -> Sem r a ensureSingleton (Set.toList -> l) = case l of [e] -> pure e - _ -> throw . mlsProtocolError $ "The external commit must have at most one remove proposal" + _ -> throwRemProposal -- | Note: Use this only for KeyPackage that are already validated updateKeyPackageMapping :: From 650aa8876e94ede1dff3b518ea2c288d0cb515e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Fri, 28 Oct 2022 13:25:48 +0200 Subject: [PATCH 11/11] Validate key package before replacing the old one --- services/galley/src/Galley/API/MLS/Message.hs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/services/galley/src/Galley/API/MLS/Message.hs b/services/galley/src/Galley/API/MLS/Message.hs index d37af5b920c..0f6fe3438df 100644 --- a/services/galley/src/Galley/API/MLS/Message.hs +++ b/services/galley/src/Galley/API/MLS/Message.hs @@ -709,6 +709,13 @@ processCommitWithAction qusr senderClient con lconv cm epoch groupId action send $ "The external commit attempts to remove a client from a user other than themselves" pure (Just r) + -- first perform checks and map the key package if valid + addKeyPackageRef + newRef + (cidQualifiedUser cid) + (ciClient cid) + (Data.convId <$> qUntagged lconv) + -- now it is safe to update the mapping without further checks updateKeyPackageMapping lconv qusr (ciClient cid) remRef newRef pure (pure (), action {paRemove = mempty})