From 9995a2b1822d3dc0c283e5b04194c25948da1c9a Mon Sep 17 00:00:00 2001 From: Gautier DI FOLCO Date: Tue, 10 Nov 2026 16:17:11 +0000 Subject: [PATCH 1/5] WPB-29949: heal ghost MLS member-client rows from the ratchet tree Ghost clients (mls_group_member_client rows lost to the external-commit re-join race fixed in WPB-29648) left affected MLS groups permanently stuck: the client could send but never received fan-out, and remove/re-add failed with 400 mls-invalid-leaf-node-index because the ghost's leaf was absent from galley's IndexMap. --- changelog.d/3-bug-fixes/WPB-29949 | 1 + .../templates/galley/configmap.yaml | 3 + charts/wire-server/values.yaml | 1 + .../src/developer/reference/config-options.md | 21 ++ .../wire-federation-v0/values.yaml.gotmpl | 1 + hack/helm_vars/wire-server/values.yaml.gotmpl | 1 + integration/test/API/GalleyInternal.hs | 34 +++ integration/test/Test/MLS.hs | 84 ++++++++ .../src/Wire/API/Routes/Internal/Galley.hs | 16 ++ .../src/Wire/ConversationStore.hs | 4 + .../src/Wire/ConversationStore/Cassandra.hs | 36 ++++ .../Wire/ConversationStore/MLS/Reconcile.hs | 111 ++++++++++ .../src/Wire/ConversationStore/Postgres.hs | 89 ++++++++ .../Wire/ConversationSubsystem/Federation.hs | 2 + .../Wire/ConversationSubsystem/Interpreter.hs | 3 + .../MLS/GroupInfoCheck.hs | 10 +- .../Wire/ConversationSubsystem/MLS/Message.hs | 10 +- .../Wire/ConversationSubsystem/MLS/Repair.hs | 176 +++++++++++++++ .../src/Wire/Options/Galley.hs | 3 + .../ConversationStore/MLS/ReconcileSpec.hs | 203 ++++++++++++++++++ libs/wire-subsystems/wire-subsystems.cabal | 3 + .../src/Wire/BackgroundWorker/Env.hs | 2 + .../background-worker/src/Wire/Effects.hs | 4 +- .../Wire/BackendNotificationPusherSpec.hs | 2 + .../background-worker/test/Test/Wire/Util.hs | 1 + services/galley/galley.integration.yaml | 1 + services/galley/src/Galley/API/Internal.hs | 22 ++ services/galley/src/Galley/App.hs | 4 +- 28 files changed, 843 insertions(+), 5 deletions(-) create mode 100644 changelog.d/3-bug-fixes/WPB-29949 create mode 100644 libs/wire-subsystems/src/Wire/ConversationStore/MLS/Reconcile.hs create mode 100644 libs/wire-subsystems/src/Wire/ConversationSubsystem/MLS/Repair.hs create mode 100644 libs/wire-subsystems/test/unit/Wire/ConversationStore/MLS/ReconcileSpec.hs diff --git a/changelog.d/3-bug-fixes/WPB-29949 b/changelog.d/3-bug-fixes/WPB-29949 new file mode 100644 index 00000000000..12920b0674b --- /dev/null +++ b/changelog.d/3-bug-fixes/WPB-29949 @@ -0,0 +1 @@ +Galley can now self-heal lost MLS member-client rows ("ghost clients"). diff --git a/charts/wire-server/templates/galley/configmap.yaml b/charts/wire-server/templates/galley/configmap.yaml index f1887040ea8..241a5e0905b 100644 --- a/charts/wire-server/templates/galley/configmap.yaml +++ b/charts/wire-server/templates/galley/configmap.yaml @@ -116,6 +116,9 @@ data: {{- if .settings.checkGroupInfo }} checkGroupInfo: {{ .settings.checkGroupInfo }} {{- end }} + {{- if .settings.mlsGroupInfoRepair }} + mlsGroupInfoRepair: {{ .settings.mlsGroupInfoRepair }} + {{- end }} meetings: {{- with .settings.meetings }} {{- if .validityPeriod }} diff --git a/charts/wire-server/values.yaml b/charts/wire-server/values.yaml index f93685ba07e..5c30e8a6f3e 100644 --- a/charts/wire-server/values.yaml +++ b/charts/wire-server/values.yaml @@ -154,6 +154,7 @@ galley: maxRateLimitedKeys: 100000 # Estimated memory usage: 4 MB checkGroupInfo: false + mlsGroupInfoRepair: false meetings: validityPeriod: "48h" diff --git a/docs/src/developer/reference/config-options.md b/docs/src/developer/reference/config-options.md index e458269773f..3fe50099ed2 100644 --- a/docs/src/developer/reference/config-options.md +++ b/docs/src/developer/reference/config-options.md @@ -1335,6 +1335,27 @@ Enabling consistency checks makes Galley parse the ratchet tree extension in the group info of every commit and compare it to the group state after applying the commit. +#### MLS group info repair + +To enable repairing MLS member-client rows from the ratchet tree stored +in the group info on every commit: +``` +# galley.yaml +config: + settings: + mlsGroupInfoRepair: true +``` + +Enabling the repair makes Galley reconcile the +`mls_group_member_client`/`mls_history_client` rows with the ratchet +tree of the previously processed commit bundle before validating a new +bundle, restoring rows lost to e.g. the external-commit re-join race +(WPB-29648). Off by default; enabling it is the rollout step, and +disabling it is the rollback. Note: history-client rows are only +present for Postgres-resident conversations, so healing of +`mls_history_client` rows applies to those; conversations still served +from Cassandra only get their `mls_group_member_client` rows healed. + #### Disabling API versions It is possible to disable one ore more API versions. When an API version is disabled it won’t be advertised on the `GET /api-version` endpoint, neither in the `supported`, nor in the `development` section. Requests made to any endpoint of a disabled API version will result in the same error response as a request made to an API version that does not exist. diff --git a/hack/helm_vars/wire-federation-v0/values.yaml.gotmpl b/hack/helm_vars/wire-federation-v0/values.yaml.gotmpl index 52abc30d9b6..d0581d490fe 100644 --- a/hack/helm_vars/wire-federation-v0/values.yaml.gotmpl +++ b/hack/helm_vars/wire-federation-v0/values.yaml.gotmpl @@ -168,6 +168,7 @@ galley: maxTeamSize: 32 maxFanoutSize: 18 maxConvSize: 16 + mlsGroupInfoRepair: true conversationCodeURI: https://kube-staging-nginz-https.zinfra.io/conversation-join/ # See helmfile for the real value federationDomain: federation-test-helper.wire-federation-v0.svc.cluster.local diff --git a/hack/helm_vars/wire-server/values.yaml.gotmpl b/hack/helm_vars/wire-server/values.yaml.gotmpl index fb04660f05c..ea702806283 100644 --- a/hack/helm_vars/wire-server/values.yaml.gotmpl +++ b/hack/helm_vars/wire-server/values.yaml.gotmpl @@ -323,6 +323,7 @@ galley: maxTeamSize: 32 maxFanoutSize: 18 maxConvSize: 16 + mlsGroupInfoRepair: true conversationCodeURI: https://kube-staging-nginz-https.zinfra.io/conversation-join/ # See helmfile for the real value federationDomain: integration.example.com diff --git a/integration/test/API/GalleyInternal.hs b/integration/test/API/GalleyInternal.hs index b41418632e6..fadce06d4dc 100644 --- a/integration/test/API/GalleyInternal.hs +++ b/integration/test/API/GalleyInternal.hs @@ -135,6 +135,40 @@ getMLSMemberClients user conv = do ("/i/group/" <> BS.unpack (B64U.encodeUnpadded gidBytes) <> "/member-clients") submit "GET" req +-- | DELETE /i/group/:gid/member-clients/:domain/:uid/:client — damage +-- injector / manual repair tool. 'owner' selects the backend the request is +-- sent to; 'victim' identifies whose client row is deleted. +deleteMLSMemberClient :: + (HasCallStack, MakesValue owner, MakesValue victim, MakesValue client) => + owner -> + ConvId -> + victim -> + client -> + App Response +deleteMLSMemberClient owner conv victim client = do + gid <- case conv.groupId of + Just g -> pure g + Nothing -> assertFailure "conversation has no MLS group id" + gidBytes <- either (assertFailure . ("invalid MLS group id " <>) . (gid <>)) pure (B64.decode (cs gid)) + vid <- objId victim + vdomain <- objDomain victim + cidStr <- asString client + req <- + baseRequest + owner + Galley + Unversioned + ( "/i/group/" + <> BS.unpack (B64U.encodeUnpadded gidBytes) + <> "/member-clients/" + <> vdomain + <> "/" + <> vid + <> "/" + <> cidStr + ) + submit "DELETE" req + -- | https://staging-nginz-https.zinfra.io/api-internal/swagger-ui/galley/#/galley/get_i_teams__tid__features_legalhold legalholdIsEnabled :: (HasCallStack, MakesValue tid, MakesValue uid) => tid -> uid -> App Response legalholdIsEnabled tid uid = do diff --git a/integration/test/Test/MLS.hs b/integration/test/Test/MLS.hs index beb95e4bb25..8f7bb1fb8fc 100644 --- a/integration/test/Test/MLS.hs +++ b/integration/test/Test/MLS.hs @@ -1068,6 +1068,90 @@ testExternalCommitDuplicateClient = do resp.status `shouldMatchInt` 400 resp.json %. "label" `shouldMatch` "mls-protocol-error" +-- | A ghost client (member-client row lost) is healed from the stored ratchet +-- tree on the next commit bundle; remove/re-add then works again +-- (no 400 mls-invalid-leaf-node-index). +testGhostMemberRowRepair :: (HasCallStack) => App () +testGhostMemberRowRepair = do + [alice, bob] <- createAndConnectUsers [OwnDomain, OwnDomain] + [alice1, bob1] <- traverse (createMLSClient def) [alice, bob] + traverse_ (void . uploadNewKeyPackage def) [bob1] + convId <- createNewGroup def alice1 + void $ createAddCommit alice1 convId [bob] >>= sendAndConsumeCommitBundle + + -- Inject the damage: delete bob1's member-client row. + void $ I.deleteMLSMemberClient alice convId bob bob1.client >>= assertSuccess + + -- Sanity: bob1 no longer receives fan-out. + withWebSockets [alice1, bob1] $ \[_, wsBob] -> do + mp <- createApplicationMessage convId alice1 "before repair" + bindResponse (postMLSMessage mp.sender mp.message) $ \resp -> + resp.status `shouldMatchInt` 201 + expectFailure (const $ pure ()) + $ awaitMatch + (\m -> nPayload m %. "type" `isEqual` "conversation.mls-message-add") + wsBob + + -- Any commit bundle triggers the repair from the stored ratchet tree. + void $ createExternalCommit convId alice1 Nothing >>= sendAndConsumeCommitBundle + + -- bob1's row is back with its TRUE leaf index. + gs <- getClientGroupState bob1 + groupData <- assertJust "no MLS group state for bob1" (Map.lookup convId gs.groups) + members <- readGroupState groupData + idx <- + assertJust "bob1 missing from group state" + $ listToMaybe [idx | (RegularClient c, idx) <- members, c == bob1] + rows <- I.getMLSMemberClients alice convId >>= getJSON 200 >>= (%. "member_clients") >>= asList + idxs <- for rows $ \r -> do + ci <- r %. "client" + uid <- ci %. "user_id" & asString + client <- ci %. "client_id" & asString + idx' <- r %. "leaf_index" & asIntegral @Word32 + pure (uid, client, idx') + [idx' | (u, c, idx') <- idxs, u == bob1.user, c == bob1.client] `shouldMatch` [idx] + + -- bob1 receives messages again. + withWebSockets [alice1, bob1] $ \[_, wsBob] -> do + mp <- createApplicationMessage convId alice1 "after repair" + bindResponse (postMLSMessage mp.sender mp.message) $ \resp -> + resp.status `shouldMatchInt` 201 + n <- awaitMatch (\n -> nPayload n %. "type" `isEqual` "conversation.mls-message-add") wsBob + nPayload n %. "data" `shouldMatch` T.decodeUtf8 (Base64.encode mp.message) + + -- Remove/re-add bob without the 400 mls-invalid-leaf-node-index. + mp <- createRemoveCommit alice1 convId [bob1] + void $ sendAndConsumeCommitBundle mp + -- bob1 is in membersToBeRemoved for the remove commit, so the harness never + -- lets it consume that commit and its local group state goes stale; reset it + -- or the re-add welcome trips the existing-client assertion in consumeWelcome. + setClientGroupState bob1 def + void $ uploadNewKeyPackage def bob1 + void $ createAddCommit alice1 convId [bob] >>= sendAndConsumeCommitBundle + +-- | WPB-29648: with the repair disabled, removing a ghost client (whose +-- member-client row was lost) is rejected with the pre-fix failure from the +-- bug report: 400 mls-invalid-leaf-node-index. +testGhostRemoveRejectedWithoutRepair :: (HasCallStack) => App () +testGhostRemoveRejectedWithoutRepair = do + withModifiedBackend + (def {galleyCfg = setField "settings.mlsGroupInfoRepair" (False :: Bool)}) + $ \domain -> do + [alice, bob] <- createAndConnectUsers [domain, domain] + [alice1, bob1] <- traverse (createMLSClient def) [alice, bob] + traverse_ (uploadNewKeyPackage def) [bob1] + convId <- createNewGroup def alice1 + void $ createAddCommit alice1 convId [bob] >>= sendAndConsumeCommitBundle + + -- Inject the damage: delete bob1's member-client row. + void $ I.deleteMLSMemberClient alice convId bob bob1.client >>= assertSuccess + + -- Remove-commit bundle for the ghost client is rejected with the exact + -- error from the bug report (no healing happened to fix the indexMap). + mp <- createRemoveCommit alice1 convId [bob1] + err <- postMLSCommitBundle mp.sender (mkBundle mp) >>= getJSON 400 + err %. "label" `shouldMatch` "mls-invalid-leaf-node-index" + testExternalCommitSameClient :: (HasCallStack) => App () testExternalCommitSameClient = do [alice, bob] <- createAndConnectUsers [OwnDomain, OwnDomain] diff --git a/libs/wire-api/src/Wire/API/Routes/Internal/Galley.hs b/libs/wire-api/src/Wire/API/Routes/Internal/Galley.hs index 8aad4a81282..f2d7dab2fe9 100644 --- a/libs/wire-api/src/Wire/API/Routes/Internal/Galley.hs +++ b/libs/wire-api/src/Wire/API/Routes/Internal/Galley.hs @@ -199,6 +199,22 @@ type InternalAPIBase = '[JSON] (Respond 200 "MLS member clients" MLSMemberClientList) ) + -- Deletes a single member-client row; support/ops manual repair tool and + -- damage injector for integration tests. + :<|> Named + "delete-mls-member-client" + ( Summary "Delete an MLS conversation member client row" + :> "group" + :> Capture "gid" GroupId + :> "member-clients" + :> Capture "domain" Domain + :> Capture "uid" UserId + :> Capture "client" ClientId + :> MultiVerb1 + 'DELETE + '[JSON] + (RespondEmpty 204 "Deleted") + ) :<|> Named "guard-legalhold-policy-conflicts" ( "guard-legalhold-policy-conflicts" diff --git a/libs/wire-subsystems/src/Wire/ConversationStore.hs b/libs/wire-subsystems/src/Wire/ConversationStore.hs index 62da8bfa657..c196536a5ef 100644 --- a/libs/wire-subsystems/src/Wire/ConversationStore.hs +++ b/libs/wire-subsystems/src/Wire/ConversationStore.hs @@ -131,6 +131,10 @@ data ConversationStore m a where RemoveAllMLSClients :: GroupId -> ConversationStore m () LookupMLSClients :: GroupId -> ConversationStore m (ClientMap LeafIndex) LookupHistoryClients :: GroupId -> ConversationStore m [(HistoryClientId, Int32, Bool)] + -- | Reconcile member-client rows of a group with the authoritative + -- ratchet-tree leaf map. Pending-removal rows are untouched. Returns + -- (inserted, updated, deleted). + ReconcileMLSClients :: GroupId -> IndexMap -> ConversationStore m (Int, Int, Int) LookupMLSClientLeafIndices :: GroupId -> ConversationStore m (ClientMap LeafIndex, IndexMap) -- SUB CONVERSATION OPERATIONS UpsertSubConversation :: ConvId -> SubConvId -> GroupId -> ConversationStore m SubConversation diff --git a/libs/wire-subsystems/src/Wire/ConversationStore/Cassandra.hs b/libs/wire-subsystems/src/Wire/ConversationStore/Cassandra.hs index 9e93d7f8010..c86d7ec27df 100644 --- a/libs/wire-subsystems/src/Wire/ConversationStore/Cassandra.hs +++ b/libs/wire-subsystems/src/Wire/ConversationStore/Cassandra.hs @@ -76,6 +76,7 @@ import Wire.ConversationStore qualified as ConvStore import Wire.ConversationStore.Cassandra.Instances () import Wire.ConversationStore.Cassandra.Queries qualified as Cql import Wire.ConversationStore.Cassandra.Queries qualified as Queries +import Wire.ConversationStore.MLS.Reconcile (MlsReconcileOps (..), mlsReconcileOps) import Wire.ConversationStore.MLS.Types import Wire.ConversationStore.Migration.Cleanup import Wire.ConversationStore.Postgres (interpretConversationStoreToPostgres) @@ -774,6 +775,31 @@ lookupMLSClientRows groupId = lookupMLSClients :: GroupId -> Client (ClientMap LeafIndex) lookupMLSClients = fmap mkClientMap . lookupMLSClientRows +-- | Reconcile member-client rows with the tree leaves. History clients live +-- exclusively in Postgres in the dual-write era, so the diff covers member +-- rows only here. Inserts and leaf updates reuse 'Cql.addMLSClient' (an +-- upsert that clears @removal_pending@), which is safe because both target +-- identities that reconcile proved are non-pending. +reconcileMLSClients :: GroupId -> IndexMap -> Client (Int, Int, Int) +reconcileMLSClients groupId leaves = do + memberRows <- lookupMLSClientRows groupId + let ops = mlsReconcileOps memberRows [] leaves + retry x5 . batch $ do + setType BatchLogged + setConsistency LocalQuorum + -- addMLSClient is an upsert that clears removal_pending; inserts and leaf + -- updates are the same write here, and both only target identities the + -- diff proved are non-pending. + for_ (ops.roMemberInsert <> ops.roMemberUpdateLeaf) $ \(dom, usr, c, idx) -> + addPrepQuery Cql.addMLSClient (groupId, dom, usr, c, idx) + for_ ops.roMemberDelete $ \(dom, usr, c) -> + addPrepQuery Cql.removeMLSClient (groupId, dom, usr, c) + pure + ( length ops.roMemberInsert, + length ops.roMemberUpdateLeaf, + length ops.roMemberDelete + ) + ----------------------------------------------------------------------------------------- -- SUB CONVERSATION STORE @@ -1035,6 +1061,9 @@ interpretConversationStoreToCassandra client = interpret $ \case UpsertMLSClient gid ci idx -> do logEffect "ConversationStore.UpsertMLSClient" embedClient client $ addMLSClients gid (cidQualifiedUser ci) (Set.singleton (ciClient ci, idx)) + ReconcileMLSClients gid leaves -> do + logEffect "ConversationStore.ReconcileMLSClients" + embedClient client $ reconcileMLSClients gid leaves AddHistoryClient groupId hid idx -> do logEffect "ConversationStore.AddHistoryClient" interpretConversationStoreToPostgres $ ConvStore.addHistoryClient groupId hid idx @@ -1451,6 +1480,13 @@ interpretConversationStoreToCassandraAndPostgres client = interpret $ \case isConvInPostgres cid >>= \case False -> embedClient client $ addMLSClients gid (cidQualifiedUser ci) (Set.singleton (ciClient ci, idx)) True -> interpretConversationStoreToPostgres (ConvStore.upsertMLSClient gid ci idx) + ReconcileMLSClients gid leaves -> do + logEffect "ConversationStore.ReconcileMLSClients" + cid <- groupIdToConvId gid + withMigrationLockAndCleanup client LockShared (Left cid) $ + isConvInPostgres cid >>= \case + False -> embedClient client $ reconcileMLSClients gid leaves + True -> interpretConversationStoreToPostgres (ConvStore.reconcileMLSClients gid leaves) AddHistoryClient groupId hid idx -> do logEffect "ConversationStore.AddHistoryClient" interpretConversationStoreToPostgres $ ConvStore.addHistoryClient groupId hid idx diff --git a/libs/wire-subsystems/src/Wire/ConversationStore/MLS/Reconcile.hs b/libs/wire-subsystems/src/Wire/ConversationStore/MLS/Reconcile.hs new file mode 100644 index 00000000000..3db3bd6eac5 --- /dev/null +++ b/libs/wire-subsystems/src/Wire/ConversationStore/MLS/Reconcile.hs @@ -0,0 +1,111 @@ +-- This file is part of the Wire Server implementation. +-- +-- Copyright (C) 2026 Wire Swiss GmbH +-- +-- This program is free software: you can redistribute it and/or modify it +-- under the terms of the GNU Affero General Public License as published by +-- the Free Software Foundation, either version 3 of the License, or (at your +-- option) any later version. +-- +-- This program is distributed in the hope that it will be useful, but WITHOUT +-- ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +-- FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License +-- for more details. +-- +-- You should have received a copy of the GNU Affero General Public License +-- along with this program. If not, see . + +-- | Reconciliation of the stored @mls_group_member_client@ and +-- @mls_history_client@ rows against the authoritative ratchet tree stored in +-- the group's GroupInfo. +-- +-- Ghost clients (rows lost to the external-commit re-join race fixed in +-- WPB-29648) manifest as tree leaves that have no corresponding row. This +-- module computes the minimal set of writes that brings the rows back in sync +-- with the tree. +module Wire.ConversationStore.MLS.Reconcile + ( MlsReconcileOps (..), + mlsReconcileOps, + ) +where + +import Data.Domain (Domain) +import Data.Id (ClientId, HistoryClientId, UserId) +import Data.IntMap qualified as IntMap +import Data.Map qualified as Map +import Imports +import Wire.API.MLS.Credential (ClientIdentity (..), GroupMember (..)) +import Wire.ConversationStore.MLS.Types (IndexMap, unIndexMap) + +-- | The writes needed to bring the member-client rows in line with the +-- ratchet-tree leaves. +data MlsReconcileOps = MlsReconcileOps + { -- | leaf, removal_pending = false + roMemberInsert :: [(Domain, UserId, ClientId, Int32)], + roMemberUpdateLeaf :: [(Domain, UserId, ClientId, Int32)], + roMemberDelete :: [(Domain, UserId, ClientId)], + roHistoryInsert :: [(HistoryClientId, Int32)], + roHistoryUpdateLeaf :: [(HistoryClientId, Int32)], + roHistoryDelete :: [HistoryClientId] + } + deriving stock (Eq, Show) + +-- | Compute the diff between the current rows and the tree leaves. +-- +-- Rules: +-- +-- * Rows with @removal_pending == True@ are ignored entirely: their leaf is +-- still occupied in the clients' trees and pending remove proposals must +-- stay resolvable. +-- * A tree leaf with no row is inserted (pending = false). +-- * A row whose leaf differs from the tree leaf is updated. +-- * A non-pending row whose client is absent from the tree is deleted (the +-- row-write of a previous commit was lost). +mlsReconcileOps :: + -- | Current @mls_group_member_client@ rows + [(Domain, UserId, ClientId, Int32, Bool)] -> + -- | Current @mls_history_client@ rows + [(HistoryClientId, Int32, Bool)] -> + -- | Tree leaves (as extracted from the stored GroupInfo) + IndexMap -> + MlsReconcileOps +mlsReconcileOps members history leaves = + let (mIns, mUpd, mDel) = + diffRows + ( Map.fromList + [ ((cid.ciDomain, cid.ciUser, cid.ciClient), fromIntegral i) + | (i, RegularClient cid) <- IntMap.assocs treeMembers + ] + ) + (Map.fromList [((d, u, c), (leafIdx, pending)) | (d, u, c, leafIdx, pending) <- members]) + (hIns, hUpd, hDel) = + diffRows + (Map.fromList [(h, fromIntegral i) | (i, HistoryClient h) <- IntMap.assocs treeMembers]) + (Map.fromList [(k, (leafIdx, pending)) | (k, leafIdx, pending) <- history]) + in MlsReconcileOps + { roMemberInsert = [withKey k l | (k, l) <- mIns], + roMemberUpdateLeaf = [withKey k l | (k, l) <- mUpd], + roMemberDelete = mDel, + roHistoryInsert = hIns, + roHistoryUpdateLeaf = hUpd, + roHistoryDelete = hDel + } + where + treeMembers = unIndexMap leaves + + withKey :: (Domain, UserId, ClientId) -> Int32 -> (Domain, UserId, ClientId, Int32) + withKey (dom, usr, cid) l = (dom, usr, cid, l) + + -- The three reconcile rules for one row kind. Pending rows block inserts + -- (any row, incl. pending) but are never updated or deleted. + diffRows :: + (Ord k) => + Map k Int32 -> + Map k (Int32, Bool) -> + ([(k, Int32)], [(k, Int32)], [k]) + diffRows tree current = + let active = Map.filter (not . snd) current + ins = [(k, l) | (k, l) <- Map.toList tree, not (Map.member k current)] + upd = [(k, l) | (k, l) <- Map.toList tree, Just (rowLeaf, _) <- [Map.lookup k active], rowLeaf /= l] + del = [k | (k, _) <- Map.toList active, not (Map.member k tree)] + in (ins, upd, del) diff --git a/libs/wire-subsystems/src/Wire/ConversationStore/Postgres.hs b/libs/wire-subsystems/src/Wire/ConversationStore/Postgres.hs index aa6f85cb3de..1bf99351dba 100644 --- a/libs/wire-subsystems/src/Wire/ConversationStore/Postgres.hs +++ b/libs/wire-subsystems/src/Wire/ConversationStore/Postgres.hs @@ -29,6 +29,7 @@ import Data.Range import Data.Set qualified as Set import Data.Text qualified as Text import Data.Time +import Data.UUID (UUID) import Data.Vector (Vector) import Data.Vector qualified as Vector import GHC.Records (HasField) @@ -57,6 +58,7 @@ import Wire.API.Pagination import Wire.API.PostgresMarshall import Wire.API.Provider.Service import Wire.ConversationStore +import Wire.ConversationStore.MLS.Reconcile (MlsReconcileOps (..), mlsReconcileOps) import Wire.ConversationStore.MLS.Types import Wire.Postgres import Wire.Sem.Paging.Cassandra @@ -118,6 +120,7 @@ interpretConversationStoreToPostgres = interpret $ \case RemoveAllMLSClients gid -> removeAllMLSClientsImpl gid LookupMLSClients lcnv -> lookupMLSClientsImpl lcnv LookupHistoryClients gid -> lookupHistoryClientsImpl gid + ReconcileMLSClients gid leaves -> reconcileMLSClientsImpl gid leaves LookupMLSClientLeafIndices lcnv -> lookupMLSClientLeafIndicesImpl lcnv UpsertSubConversation convId subConvId groupId -> createSubConversationImpl convId subConvId groupId GetSubConversation convId subConvId -> getSubConversationImpl convId subConvId @@ -1137,6 +1140,92 @@ selectHistoryClients = lookupHistoryClientsImpl :: (PGConstraints r) => GroupId -> Sem r [(HistoryClientId, Int32, Bool)] lookupHistoryClientsImpl gid = runStatement gid selectHistoryClients +reconcileMLSClientsImpl :: (PGConstraints r) => GroupId -> IndexMap -> Sem r (Int, Int, Int) +reconcileMLSClientsImpl gid leaves = do + memberRows <- runStatement gid selectMLSClients + historyRows <- runStatement gid selectHistoryClients + let ops = mlsReconcileOps memberRows historyRows leaves + (insDomains, insUsers, insClients, insLeaves) = unzip4 ops.roMemberInsert + (updDomains, updUsers, updClients, updLeaves) = unzip4 ops.roMemberUpdateLeaf + (delDomains, delUsers, delClients) = unzip3 ops.roMemberDelete + (insHistoryIds, insHistoryLeaves) = unzip ops.roHistoryInsert + (updHistoryIds, updHistoryLeaves) = unzip ops.roHistoryUpdateLeaf + runPipeline $ + Pipeline.statement (gid, insDomains, insUsers, insClients, insLeaves) insertMembers + *> Pipeline.statement (gid, updDomains, updUsers, updClients, updLeaves) updateMemberLeaves + *> Pipeline.statement (gid, delDomains, delUsers, delClients) deleteStaleMembers + *> Pipeline.statement (gid, insHistoryIds, insHistoryLeaves) insertHistoryClients + *> Pipeline.statement (gid, updHistoryIds, updHistoryLeaves) updateHistoryLeaves + *> Pipeline.statement (gid, ops.roHistoryDelete) deleteHistoryClients + pure + ( length ops.roMemberInsert + length ops.roHistoryInsert, + length ops.roMemberUpdateLeaf + length ops.roHistoryUpdateLeaf, + length ops.roMemberDelete + length ops.roHistoryDelete + ) + where + insertMembers :: Hasql.Statement (GroupId, [Domain], [UserId], [ClientId], [Int32]) () + insertMembers = + lmapPG @(ByteString, Vector Text, Vector UUID, Vector Text, Vector Int32) @(GroupId, [Domain], [UserId], [ClientId], [Int32]) + [resultlessStatement|INSERT INTO mls_group_member_client + (group_id, user_domain, "user", client, leaf_node_index, removal_pending) + SELECT g.group_id, t.domain, t.usr, t.client, t.idx, false + FROM UNNEST($2 :: text[], $3 :: uuid[], $4 :: text[], $5 :: integer[]) AS t(domain, usr, client, idx) + CROSS JOIN (SELECT ($1 :: bytea) AS group_id) g + ON CONFLICT (group_id, user_domain, "user", client) DO NOTHING + |] + + updateMemberLeaves :: Hasql.Statement (GroupId, [Domain], [UserId], [ClientId], [Int32]) () + updateMemberLeaves = + lmapPG @(ByteString, Vector Text, Vector UUID, Vector Text, Vector Int32) @(GroupId, [Domain], [UserId], [ClientId], [Int32]) + -- alias 'cl': a bare 'client' would be ambiguous against + -- mls_group_member_client.client under UPDATE ... FROM + [resultlessStatement|UPDATE mls_group_member_client + SET leaf_node_index = idx + FROM UNNEST($2 :: text[], $3 :: uuid[], $4 :: text[], $5 :: integer[]) AS t(domain, usr, cl, idx) + WHERE group_id = ($1 :: bytea) + AND user_domain = t.domain + AND "user" = t.usr + AND client = t.cl + |] + + deleteStaleMembers :: Hasql.Statement (GroupId, [Domain], [UserId], [ClientId]) () + deleteStaleMembers = + lmapPG @(ByteString, Vector Text, Vector UUID, Vector Text) @(GroupId, [Domain], [UserId], [ClientId]) + [resultlessStatement|DELETE FROM mls_group_member_client + WHERE group_id = ($1 :: bytea) + AND (user_domain, "user", client) IN + (SELECT * FROM UNNEST($2 :: text[], $3 :: uuid[], $4 :: text[])) + |] + + insertHistoryClients :: Hasql.Statement (GroupId, [HistoryClientId], [Int32]) () + insertHistoryClients = + lmapPG @(ByteString, Vector UUID, Vector Int32) @(GroupId, [HistoryClientId], [Int32]) + [resultlessStatement|INSERT INTO mls_history_client + (group_id, id, leaf_node_index, removal_pending) + SELECT g.group_id, t.id, t.idx, false + FROM UNNEST($2 :: uuid[], $3 :: integer[]) AS t(id, idx) + CROSS JOIN (SELECT ($1 :: bytea) AS group_id) g + ON CONFLICT (group_id, id) DO NOTHING + |] + + updateHistoryLeaves :: Hasql.Statement (GroupId, [HistoryClientId], [Int32]) () + updateHistoryLeaves = + lmapPG @(ByteString, Vector UUID, Vector Int32) @(GroupId, [HistoryClientId], [Int32]) + [resultlessStatement|UPDATE mls_history_client + SET leaf_node_index = idx + FROM UNNEST($2 :: uuid[], $3 :: integer[]) AS t(hid, idx) + WHERE group_id = ($1 :: bytea) + AND id = t.hid + |] + + deleteHistoryClients :: Hasql.Statement (GroupId, [HistoryClientId]) () + deleteHistoryClients = + lmapPG @(ByteString, Vector UUID) @(GroupId, [HistoryClientId]) + [resultlessStatement|DELETE FROM mls_history_client + WHERE group_id = ($1 :: bytea) + AND id IN (SELECT * FROM UNNEST($2 :: uuid[])) + |] + lookupMLSClientLeafIndicesImpl :: (PGConstraints r) => GroupId -> Sem r (ClientMap LeafIndex, IndexMap) lookupMLSClientLeafIndicesImpl gid = do regularClients <- runStatement gid selectMLSClients diff --git a/libs/wire-subsystems/src/Wire/ConversationSubsystem/Federation.hs b/libs/wire-subsystems/src/Wire/ConversationSubsystem/Federation.hs index 6ffc523c7f9..e707b7c7d0f 100644 --- a/libs/wire-subsystems/src/Wire/ConversationSubsystem/Federation.hs +++ b/libs/wire-subsystems/src/Wire/ConversationSubsystem/Federation.hs @@ -88,6 +88,7 @@ import Wire.ConversationSubsystem.MLS.IncomingMessage import Wire.ConversationSubsystem.MLS.Message import Wire.ConversationSubsystem.MLS.One2One import Wire.ConversationSubsystem.MLS.Removal +import Wire.ConversationSubsystem.MLS.Repair (GroupInfoRepairEnabled) import Wire.ConversationSubsystem.MLS.SubConversation hiding (leaveSubConversation) import Wire.ConversationSubsystem.MLS.Util import Wire.ConversationSubsystem.MLS.Welcome @@ -706,6 +707,7 @@ sendMLSCommitBundle :: Member NotificationSubsystem r, Member (Input (Local ())) r, Member (Input (Maybe GroupInfoCheckEnabled)) r, + Member (Input (Maybe GroupInfoRepairEnabled)) r, Member (Input (Maybe (MLSKeysByPurpose MLSPrivateKeys))) r, Member Now r, Member LegalHoldStore r, diff --git a/libs/wire-subsystems/src/Wire/ConversationSubsystem/Interpreter.hs b/libs/wire-subsystems/src/Wire/ConversationSubsystem/Interpreter.hs index c4b36378e17..83a59bec6d1 100644 --- a/libs/wire-subsystems/src/Wire/ConversationSubsystem/Interpreter.hs +++ b/libs/wire-subsystems/src/Wire/ConversationSubsystem/Interpreter.hs @@ -20,6 +20,7 @@ module Wire.ConversationSubsystem.Interpreter ( interpretConversationSubsystem, GroupInfoCheckEnabled (..), + GroupInfoRepairEnabled (..), IntraListing (..), ConversationSubsystemError (..), ) @@ -62,6 +63,7 @@ import Wire.ConversationSubsystem.MLS.GroupInfo qualified as MLSGroupInfo import Wire.ConversationSubsystem.MLS.GroupInfoCheck (GroupInfoCheckEnabled (..)) import Wire.ConversationSubsystem.MLS.Message qualified as MLSMessage import Wire.ConversationSubsystem.MLS.Removal qualified as MLSRemoval +import Wire.ConversationSubsystem.MLS.Repair (GroupInfoRepairEnabled (..)) import Wire.ConversationSubsystem.MLS.Reset qualified as MLSReset import Wire.ConversationSubsystem.MLS.SubConversation qualified as MLSSubConversation import Wire.ConversationSubsystem.Message (IntraListing (..)) @@ -125,6 +127,7 @@ interpretConversationSubsystem :: Member (Input ConversationSubsystemConfig) r, Member (Input (Local ())) r, Member (Input (Maybe GroupInfoCheckEnabled)) r, + Member (Input (Maybe GroupInfoRepairEnabled)) r, Member ProposalStore r, Member LegalHoldStore r, Member TeamStore r, diff --git a/libs/wire-subsystems/src/Wire/ConversationSubsystem/MLS/GroupInfoCheck.hs b/libs/wire-subsystems/src/Wire/ConversationSubsystem/MLS/GroupInfoCheck.hs index d8ffca0bbc3..f87a81435c2 100644 --- a/libs/wire-subsystems/src/Wire/ConversationSubsystem/MLS/GroupInfoCheck.hs +++ b/libs/wire-subsystems/src/Wire/ConversationSubsystem/MLS/GroupInfoCheck.hs @@ -17,6 +17,7 @@ module Wire.ConversationSubsystem.MLS.GroupInfoCheck ( checkGroupState, + groupInfoLeaves, GroupInfoMismatch (..), GroupInfoCheckEnabled (..), ) @@ -76,6 +77,12 @@ checkGroupState convOrSub newLeaves groupInfo = do groupStateMismatch :: IndexMap -> GroupInfo -> Either Text (Maybe GroupInfoMismatch) groupStateMismatch leaves groupInfo = do + giLeaves <- groupInfoLeaves groupInfo + pure $ guard (leaves /= giLeaves) $> GroupInfoMismatch (imAssocs leaves) + +-- | Build the 'IndexMap' implied by the ratchet tree stored in a GroupInfo. +groupInfoLeaves :: GroupInfo -> Either Text IndexMap +groupInfoLeaves groupInfo = do trees <- first (const "Could not parse ratchet tree extension in GroupInfo") @@ -83,8 +90,7 @@ groupStateMismatch leaves groupInfo = do tree :: RatchetTree <- case trees of (tree : _) -> pure tree _ -> Left "No ratchet tree extension found in GroupInfo" - giLeaves <- imFromList <$> traverse (traverse getIdentity) (ratchetTreeLeaves tree) - pure $ guard (leaves /= giLeaves) $> GroupInfoMismatch (imAssocs leaves) + imFromList <$> traverse (traverse getIdentity) (ratchetTreeLeaves tree) where getIdentity :: LeafNode -> Either Text GroupMember getIdentity leaf = fst <$> credentialIdentityAndKey leaf.credential diff --git a/libs/wire-subsystems/src/Wire/ConversationSubsystem/MLS/Message.hs b/libs/wire-subsystems/src/Wire/ConversationSubsystem/MLS/Message.hs index efefd635272..0719ff12e02 100644 --- a/libs/wire-subsystems/src/Wire/ConversationSubsystem/MLS/Message.hs +++ b/libs/wire-subsystems/src/Wire/ConversationSubsystem/MLS/Message.hs @@ -101,6 +101,7 @@ import Wire.ConversationSubsystem.MLS.One2One import Wire.ConversationSubsystem.MLS.OutOfSync import Wire.ConversationSubsystem.MLS.Propagate import Wire.ConversationSubsystem.MLS.Proposal +import Wire.ConversationSubsystem.MLS.Repair (GroupInfoRepairEnabled, repairMLSMemberClients) import Wire.ConversationSubsystem.MLS.Util import Wire.ConversationSubsystem.MLS.Welcome (sendWelcomes) import Wire.ConversationSubsystem.Util @@ -577,6 +578,7 @@ postMLSCommitBundle :: Member (Error GroupInfoDiagnostics) r, Member (Error MLSOutOfSyncError) r, Member (Input (Maybe GroupInfoCheckEnabled)) r, + Member (Input (Maybe GroupInfoRepairEnabled)) r, Member Random r, Member Resource r, Members MLSMessageStaticErrors r, @@ -610,6 +612,7 @@ postMLSCommitBundleFromLocalUser :: Member (Error GroupInfoDiagnostics) r, Member (Error MLSOutOfSyncError) r, Member (Input (Maybe GroupInfoCheckEnabled)) r, + Member (Input (Maybe GroupInfoRepairEnabled)) r, Member (Input (Maybe (MLSKeysByPurpose MLSPrivateKeys))) r, Member Random r, Member Resource r, @@ -645,6 +648,7 @@ postMLSCommitBundleToLocalConv :: Member (Error MLSOutOfSyncError) r, Member (Input EnableOutOfSyncCheck) r, Member (Input (Maybe GroupInfoCheckEnabled)) r, + Member (Input (Maybe GroupInfoRepairEnabled)) r, Member Random r, Member Resource r, Members MLSMessageStaticErrors r, @@ -664,7 +668,11 @@ postMLSCommitBundleToLocalConv :: Local ConvOrSubConvId -> Sem r [LocalConversationUpdate] postMLSCommitBundleToLocalConv qusr c conn bundle ctype lConvOrSubId = do - lConvOrSub <- fetchConvOrSub qusr bundle.groupId ctype lConvOrSubId + lConvOrSub0 <- fetchConvOrSub qusr bundle.groupId ctype lConvOrSubId + -- Heal ghost clients from the authoritative ratchet tree before anything + -- consumes convOrSub.indexMap (WPB-29648 follow-up). + changed <- repairMLSMemberClients lConvOrSub0 + lConvOrSub <- if changed then fetchConvOrSub qusr bundle.groupId ctype lConvOrSubId else pure lConvOrSub0 let convOrSub = tUnqualified lConvOrSub -- validate application message diff --git a/libs/wire-subsystems/src/Wire/ConversationSubsystem/MLS/Repair.hs b/libs/wire-subsystems/src/Wire/ConversationSubsystem/MLS/Repair.hs new file mode 100644 index 00000000000..281ab5edbf2 --- /dev/null +++ b/libs/wire-subsystems/src/Wire/ConversationSubsystem/MLS/Repair.hs @@ -0,0 +1,176 @@ +-- This file is part of the Wire Server implementation. +-- +-- Copyright (C) 2026 Wire Swiss GmbH +-- +-- This program is free software: you can redistribute it and/or modify it +-- under the terms of the GNU Affero General Public License as published by +-- the Free Software Foundation, either version 3 of the License, or (at your +-- option) any later version. +-- +-- This program is distributed in the hope that it will be useful, but WITHOUT +-- ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +-- FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License +-- for more details. +-- +-- You should have received a copy of the GNU Affero General Public License +-- along with this program. If not, see . + +-- | Heal ghost clients: reconcile the stored @mls_group_member_client@ / +-- @mls_history_client@ rows with the authoritative ratchet tree persisted in +-- the group's GroupInfo. Runs before every commit bundle is validated, so a +-- group whose rows were lost (e.g. to the external-commit re-join race fixed +-- in WPB-29648) self-heals instead of rejecting every subsequent commit. +-- +-- The reconcile runs under the per-group commit lock (skipped, not failed, if +-- the lock is held by a concurrent commit) and only when the stored GroupInfo +-- epoch matches the conversation epoch, so a concurrent commit or a +-- partially-failed commit cannot be rolled back by a stale tree. +module Wire.ConversationSubsystem.MLS.Repair + ( GroupInfoRepairEnabled (..), + repairMLSMemberClients, + ) +where + +import Data.Hex (hex) +import Data.Qualified (Local, tUnqualified) +import Data.Time (NominalDiffTime) +import Imports +import Polysemy (Member, Sem) +import Polysemy.Input (Input, input) +import Polysemy.Resource (Resource, bracket) +import Polysemy.TinyLog (TinyLog) +import Polysemy.TinyLog qualified as TinyLog +import System.Logger qualified as Log +import Wire.API.Conversation.Protocol qualified as Protocol +import Wire.API.MLS.Epoch (Epoch (..), epochNumber) +import Wire.API.MLS.Group (GroupId (..)) +import Wire.API.MLS.GroupInfo (GroupInfoData (..)) +import Wire.API.MLS.GroupInfo qualified as GroupInfo +import Wire.API.MLS.Serialisation (decodeMLS') +import Wire.ConversationStore +import Wire.ConversationStore.MLS.Types (ConvOrSubConv) +import Wire.ConversationSubsystem.MLS.GroupInfoCheck (groupInfoLeaves) + +newtype GroupInfoRepairEnabled = GroupInfoRepairEnabled Bool + deriving stock (Eq, Ord, Show) + +repairTTL :: NominalDiffTime +repairTTL = fromIntegral (60 :: Int) + +-- | Returns True if rows were changed (caller must refetch the conversation). +repairMLSMemberClients :: + ( Member ConversationStore r, + Member MLSCommitLockStore r, + Member Resource r, + Member TinyLog r, + Member (Input (Maybe GroupInfoRepairEnabled)) r + ) => + Local ConvOrSubConv -> + Sem r Bool +repairMLSMemberClients lConvOrSub = do + enabled <- isGroupInfoRepairEnabled + case (enabled, convOrSub.mlsMeta.cnvmlsActiveData) of + (True, Just activeData) -> do + let gid = convOrSub.mlsMeta.cnvmlsGroupId + epoch = Protocol.epoch activeData + -- Cheap read-only precheck: the healthy case (no diff) never takes the + -- per-group commit lock, so the common path costs no extra LWT. + suspected <- precheck convOrSub gid epoch + if not suspected + then pure False + else do + lockAcquired <- acquireCommitLock gid epoch repairTTL + if lockAcquired == NotAcquired + then pure False -- a concurrent commit owns the group; leave it alone + else + bracket + (pure ()) + (\() -> releaseCommitLock gid epoch) + (const (repairUnderLock convOrSub gid epoch)) + _ -> pure False + where + convOrSub = tUnqualified lConvOrSub + +-- Read-only check: True if the stored tree disagrees with the row-derived +-- index map (or the stored groupinfo cannot be interpreted, which we log). +precheck :: + ( Member ConversationStore r, + Member TinyLog r + ) => + ConvOrSubConv -> + GroupId -> + Epoch -> + Sem r Bool +precheck convOrSub gid epoch = do + mGroupInfoData <- getConvOrSubGroupInfo convOrSub.id + case mGroupInfoData of + Nothing -> pure False + Just groupInfoData -> + case decodeMLS' (unGroupInfoData groupInfoData) of + -- Undecodable stored groupinfo: nothing authoritative to heal from. + Left e -> warnSkip gid "undecodable stored groupinfo" (Just e) >> pure False + Right groupInfo + -- Stale tree (rows advanced past the stored groupinfo): healing from it + -- would roll back the last commit's membership changes. + | GroupInfo.epoch groupInfo.tbs.groupContext /= epoch -> do + TinyLog.warn $ + Log.msg ("skipping MLS member-client repair" :: ByteString) + . Log.field "groupId" ("0x" <> hex (unGroupId gid)) + . Log.field "reason" ("stored groupinfo epoch mismatch" :: ByteString) + . Log.field "conversationEpoch" (epochNumber epoch) + . Log.field "groupInfoEpoch" (epochNumber (GroupInfo.epoch groupInfo.tbs.groupContext)) + pure False + | otherwise -> + case groupInfoLeaves groupInfo of + Left e -> warnSkip gid "cannot extract leaves from stored groupinfo" (Just e) >> pure False + Right leaves -> pure (leaves /= convOrSub.indexMap) + +-- Re-check under the commit lock, then write the diff. +repairUnderLock :: + ( Member ConversationStore r, + Member TinyLog r + ) => + ConvOrSubConv -> + GroupId -> + Epoch -> + Sem r Bool +repairUnderLock convOrSub gid epoch = do + suspected <- precheck convOrSub gid epoch + if not suspected + then pure False -- a concurrent commit healed or changed things meanwhile + else do + -- precheck just told us the leaves differ; recompute the diff to write. + mGroupInfoData <- getConvOrSubGroupInfo convOrSub.id + case mGroupInfoData of + Just groupInfoData + | Right groupInfo <- decodeMLS' (unGroupInfoData groupInfoData), + Right leaves <- groupInfoLeaves groupInfo -> do + (ins, upd, del) <- reconcileMLSClients gid leaves + TinyLog.warn $ + Log.msg ("repaired MLS member-client rows from ratchet tree" :: ByteString) + . Log.field "groupId" ("0x" <> hex (unGroupId gid)) + . Log.field "epoch" (epochNumber epoch) + . Log.field "inserted" ins + . Log.field "updated" upd + . Log.field "deleted" del + pure (ins + upd + del > 0) + _ -> pure False -- vanished mid-flight; the next bundle retries + +warnSkip :: + (Member TinyLog r) => + GroupId -> + ByteString -> + Maybe Text -> + Sem r () +warnSkip gid reason mDetail = + TinyLog.warn $ + Log.msg ("skipping MLS member-client repair" :: ByteString) + . Log.field "groupId" ("0x" <> hex (unGroupId gid)) + . Log.field "reason" reason + . Log.field "detail" (fromMaybe "" mDetail) + +isGroupInfoRepairEnabled :: (Member (Input (Maybe GroupInfoRepairEnabled)) r) => Sem r Bool +isGroupInfoRepairEnabled = + input <&> \case + Just (GroupInfoRepairEnabled b) -> b + Nothing -> False diff --git a/libs/wire-subsystems/src/Wire/Options/Galley.hs b/libs/wire-subsystems/src/Wire/Options/Galley.hs index 1d4f7763e4f..6fe20bb8d50 100644 --- a/libs/wire-subsystems/src/Wire/Options/Galley.hs +++ b/libs/wire-subsystems/src/Wire/Options/Galley.hs @@ -61,6 +61,7 @@ module Wire.Options.Galley passwordHashingOptions, passwordHashingRateLimit, checkGroupInfo, + mlsGroupInfoRepair, meetings, validityPeriod, legacyTimeZone, @@ -176,6 +177,8 @@ data Settings = Settings _passwordHashingRateLimit :: RateLimitConfig, -- | Check group info _checkGroupInfo :: !(Maybe Bool), + -- | Repair MLS member-client rows from the stored ratchet tree + _mlsGroupInfoRepair :: !(Maybe Bool), -- | Configuration for meetings _meetings :: !(Maybe MeetingsConfig), -- | Raw licence payload: base64url-encoded JSON 'LicenceSpec' as produced by diff --git a/libs/wire-subsystems/test/unit/Wire/ConversationStore/MLS/ReconcileSpec.hs b/libs/wire-subsystems/test/unit/Wire/ConversationStore/MLS/ReconcileSpec.hs new file mode 100644 index 00000000000..e3bf22c0340 --- /dev/null +++ b/libs/wire-subsystems/test/unit/Wire/ConversationStore/MLS/ReconcileSpec.hs @@ -0,0 +1,203 @@ +-- This file is part of the Wire Server implementation. +-- +-- Copyright (C) 2026 Wire Swiss GmbH +-- +-- This program is free software: you can redistribute it and/or modify it +-- under the terms of the GNU Affero General Public License as published by +-- the Free Software Foundation, either version 3 of the License, or (at your +-- option) any later version. +-- +-- This program is distributed in the hope that it will be useful, but WITHOUT +-- ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +-- FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License +-- for more details. +-- +-- You should have received a copy of the GNU Affero General Public License +-- along with this program. If not, see . + +module Wire.ConversationStore.MLS.ReconcileSpec (spec) where + +import Data.Bifunctor (first) +import Data.Domain (Domain (..)) +import Data.Id +import Data.UUID qualified as UUID +import Imports +import Test.Hspec +import Wire.API.MLS.Credential +import Wire.ConversationStore.MLS.Reconcile +import Wire.ConversationStore.MLS.Types + +spec :: Spec +spec = describe "Wire.ConversationStore.MLS.Reconcile" do + let dom = Domain "example.com" + dom2 = Domain "other.example.com" + + mkUser :: String -> UserId + mkUser s = Id (fromJust (UUID.fromString s)) + + user1, user2, user3 :: UserId + user1 = mkUser "00000000-0000-0000-0000-000000000001" + user2 = mkUser "00000000-0000-0000-0000-000000000002" + user3 = mkUser "00000000-0000-0000-0000-000000000003" + + cid1, cid2 :: ClientId + cid1 = ClientId 1 + cid2 = ClientId 2 + + hist1, hist2 :: HistoryClientId + hist1 = Id (fromJust (UUID.fromString "00000000-0000-0000-0000-000000000011")) + hist2 = Id (fromJust (UUID.fromString "00000000-0000-0000-0000-000000000012")) + + leaf :: Int32 -> GroupMember -> (Int32, GroupMember) + leaf i m = (i, m) + + -- Leaf indices are Int32 in the row shape but LeafIndex in the tree map. + tree :: [(Int32, GroupMember)] -> IndexMap + tree = imFromList . map (first fromIntegral) + + describe "regular member rows" do + it "inserts a row for a tree leaf with no row (ghost client)" do + let leaves = tree [leaf 0 (regular dom user1 cid1)] + rows = [] + mlsReconcileOps rows [] leaves + `shouldBe` MlsReconcileOps + { roMemberInsert = [(dom, user1, cid1, 0)], + roMemberUpdateLeaf = [], + roMemberDelete = [], + roHistoryInsert = [], + roHistoryUpdateLeaf = [], + roHistoryDelete = [] + } + + it "updates the leaf when a row points at a different leaf" do + let leaves = tree [leaf 3 (regular dom user1 cid1)] + rows = [(dom, user1, cid1, 0, False)] + ops = mlsReconcileOps rows [] leaves + ops.roMemberUpdateLeaf `shouldBe` [(dom, user1, cid1, 3)] + ops.roMemberInsert `shouldBe` [] + ops.roMemberDelete `shouldBe` [] + + it "does nothing when rows match the tree" do + let leaves = + tree + [ leaf 0 (regular dom user1 cid1), + leaf 1 (regular dom2 user2 cid2) + ] + rows = + [ (dom, user1, cid1, 0, False), + (dom2, user2, cid2, 1, False) + ] + mlsReconcileOps rows [] leaves + `shouldBe` emptyOps + + it "deletes a non-pending row absent from the tree" do + let leaves = tree [leaf 0 (regular dom user1 cid1)] + rows = + [ (dom, user1, cid1, 0, False), + (dom, user2, cid2, 1, False) + ] + mlsReconcileOps rows [] leaves + `shouldBe` emptyOps {roMemberDelete = [(dom, user2, cid2)]} + + it "ignores pending rows entirely" do + let leaves = tree [leaf 0 (regular dom user1 cid1)] + -- pending row for user2/cid2 at leaf 1: leaf is unoccupied in the + -- tree (that's fine for pending rows) and must NOT be deleted. + rows = + [ (dom, user1, cid1, 0, False), + (dom, user2, cid2, 1, True) + ] + mlsReconcileOps rows [] leaves + `shouldBe` emptyOps + + it "does not update a pending row even if its leaf differs" do + let leaves = tree [leaf 5 (regular dom user1 cid1)] + rows = + [ (dom, user1, cid1, 0, True) + ] + mlsReconcileOps rows [] leaves + `shouldBe` emptyOps + + it "updates leaf of matching identity at a new index and keeps other rows" do + let leaves = + tree + [ leaf 2 (regular dom user1 cid1), + leaf 4 (regular dom2 user3 cid1) + ] + rows = + [ (dom, user1, cid1, 0, False), + (dom2, user3, cid1, 4, False), + (dom, user2, cid2, 1, True) + ] + ops = mlsReconcileOps rows [] leaves + ops.roMemberUpdateLeaf `shouldBe` [(dom, user1, cid1, 2)] + ops.roMemberInsert `shouldBe` [] + ops.roMemberDelete `shouldBe` [] + + describe "history client rows" do + it "inserts a row for a tree history-client leaf with no row" do + let leaves = tree [leaf 0 (HistoryClient hist1)] + rows = [] + mlsReconcileOps [] rows leaves + `shouldBe` emptyOps {roHistoryInsert = [(hist1, 0)]} + + it "updates the leaf when a history row points at a different leaf" do + let leaves = tree [leaf 2 (HistoryClient hist1)] + rows = [(hist1, 0, False)] + mlsReconcileOps [] rows leaves + `shouldBe` emptyOps {roHistoryUpdateLeaf = [(hist1, 2)]} + + it "deletes a non-pending history row absent from the tree" do + let leaves = tree [leaf 0 (HistoryClient hist1)] + rows = [(hist1, 0, False), (hist2, 1, False)] + mlsReconcileOps [] rows leaves + `shouldBe` emptyOps {roHistoryDelete = [hist2]} + + it "ignores pending history rows entirely" do + let leaves = tree [leaf 0 (HistoryClient hist1)] + rows = [(hist1, 0, False), (hist2, 1, True)] + mlsReconcileOps [] rows leaves + `shouldBe` emptyOps + + describe "combined" do + it "handles regular and history leaves in one diff" do + let leaves = + tree + [ leaf 0 (regular dom user1 cid1), + leaf 1 (HistoryClient hist1), + leaf 2 (regular dom2 user2 cid2) + ] + memberRows = + [ (dom, user1, cid1, 0, False), + -- stale leaf + (dom2, user2, cid2, 5, False), + -- ghost: absent + (dom, user3, cid2, 3, False), + -- pending: untouched + (dom, user2, cid2, 4, True) + ] + historyRows = + [ (hist1, 1, False), + -- stale leaf + (hist2, 6, True) + ] + ops = mlsReconcileOps memberRows historyRows leaves + ops.roMemberInsert `shouldBe` [] + ops.roMemberUpdateLeaf `shouldBe` [(dom2, user2, cid2, 2)] + ops.roMemberDelete `shouldBe` [(dom, user3, cid2)] + ops.roHistoryInsert `shouldBe` [] + ops.roHistoryUpdateLeaf `shouldBe` [] + ops.roHistoryDelete `shouldBe` [] + where + emptyOps = + MlsReconcileOps + { roMemberInsert = [], + roMemberUpdateLeaf = [], + roMemberDelete = [], + roHistoryInsert = [], + roHistoryUpdateLeaf = [], + roHistoryDelete = [] + } + +regular :: Domain -> UserId -> ClientId -> GroupMember +regular dom usr c = RegularClient (ClientIdentity dom usr c) diff --git a/libs/wire-subsystems/wire-subsystems.cabal b/libs/wire-subsystems/wire-subsystems.cabal index 96c6bedb54f..0682d0033b2 100644 --- a/libs/wire-subsystems/wire-subsystems.cabal +++ b/libs/wire-subsystems/wire-subsystems.cabal @@ -280,6 +280,7 @@ library Wire.ConversationStore.Migration Wire.ConversationStore.Migration.Cleanup Wire.ConversationStore.Migration.Types + Wire.ConversationStore.MLS.Reconcile Wire.ConversationStore.MLS.Types Wire.ConversationStore.Postgres Wire.ConversationSubsystem @@ -317,6 +318,7 @@ library Wire.ConversationSubsystem.MLS.Propagate Wire.ConversationSubsystem.MLS.Proposal Wire.ConversationSubsystem.MLS.Removal + Wire.ConversationSubsystem.MLS.Repair Wire.ConversationSubsystem.MLS.Reset Wire.ConversationSubsystem.MLS.SubConversation Wire.ConversationSubsystem.MLS.Util @@ -662,6 +664,7 @@ test-suite wire-subsystems-tests Wire.AuthenticationSubsystem.InterpreterSpec Wire.BrigAPIAccess.RpcSpec Wire.ClientSubsystem.InterpreterSpec + Wire.ConversationStore.MLS.ReconcileSpec Wire.ConversationSubsystem.AdminlessGroupsSpec Wire.ConversationSubsystem.InterpreterSpec Wire.ConversationSubsystem.MessageSpec diff --git a/services/background-worker/src/Wire/BackgroundWorker/Env.hs b/services/background-worker/src/Wire/BackgroundWorker/Env.hs index 8c539b64d3e..a9863e0679c 100644 --- a/services/background-worker/src/Wire/BackgroundWorker/Env.hs +++ b/services/background-worker/src/Wire/BackgroundWorker/Env.hs @@ -114,6 +114,7 @@ data Env = Env guestLinkTTLSeconds :: !(Maybe GuestLinkTTLSeconds), passwordHashingOptions :: !PasswordHashingOptions, checkGroupInfo :: !(Maybe Bool), + mlsGroupInfoRepair :: !(Maybe Bool), convCodeURI :: Either HttpsUrl (Map Domain HttpsUrl), passwordHashingRateLimitEnv :: RateLimitEnv, license :: Maybe Text @@ -200,6 +201,7 @@ mkEnv opts galleyOpts = do passwordHashingOptions = galleyOpts._settings._passwordHashingOptions checkGroupInfo = galleyOpts._settings._checkGroupInfo license = opts.license + mlsGroupInfoRepair = galleyOpts._settings._mlsGroupInfoRepair workerRunningGauge <- mkWorkerRunningGauge hasqlPool <- initPostgresPool opts.postgresqlPool galleyOpts._postgresql galleyOpts._postgresqlPassword arbiterConnStr <- mkArbiterConnectionString galleyOpts._postgresql galleyOpts._postgresqlPassword diff --git a/services/background-worker/src/Wire/Effects.hs b/services/background-worker/src/Wire/Effects.hs index a86d37c2bb7..c57e29a7aa8 100644 --- a/services/background-worker/src/Wire/Effects.hs +++ b/services/background-worker/src/Wire/Effects.hs @@ -74,7 +74,7 @@ import Wire.CodeStore.Postgres (interpretCodeStoreToPostgres) import Wire.ConversationStore (ConversationStore, MLSCommitLockStore) import Wire.ConversationStore.Cassandra (MigrationError (..), interpretConversationStoreByMigration, interpretMLSCommitLockStoreToCassandra) import Wire.ConversationSubsystem (ConversationSubsystem) -import Wire.ConversationSubsystem.Interpreter (ConversationSubsystemError, GroupInfoCheckEnabled (..), IntraListing (..), interpretConversationSubsystem) +import Wire.ConversationSubsystem.Interpreter (ConversationSubsystemError, GroupInfoCheckEnabled (..), GroupInfoRepairEnabled (..), IntraListing (..), interpretConversationSubsystem) import Wire.ExternalAccess (ExternalAccess) import Wire.ExternalAccess.External import Wire.FeaturesConfigSubsystem (FeaturesConfigSubsystem, getAllTeamFeaturesForServer) @@ -192,6 +192,7 @@ makeVerifiedRequestFreshManagerIO logger fpr url reqBuilder = do type BackgroundWorkerEffects = '[ ConversationSubsystem, + Input (Maybe GroupInfoRepairEnabled), MeetingNotifier, TeamCollaboratorsSubsystem, Input AllTeamFeatures, @@ -368,6 +369,7 @@ runBackgroundWorkerEffects env extEnv requestId mJobId = . runInputSem getAllTeamFeaturesForServer . interpretTeamCollaboratorsSubsystem . discardMeetingNotifier + . runInputConst @(Maybe GroupInfoRepairEnabled) (GroupInfoRepairEnabled <$> env.mlsGroupInfoRepair) . interpretConversationSubsystem where interpretTeamFeatureStore = case env.postgresMigration.teamFeatures of diff --git a/services/background-worker/test/Test/Wire/BackendNotificationPusherSpec.hs b/services/background-worker/test/Test/Wire/BackendNotificationPusherSpec.hs index 9a5e6424162..4dc6fb0162d 100644 --- a/services/background-worker/test/Test/Wire/BackendNotificationPusherSpec.hs +++ b/services/background-worker/test/Test/Wire/BackendNotificationPusherSpec.hs @@ -518,6 +518,7 @@ spec = do passwordHashingOptions = PasswordHashingScrypt checkGroupInfo = Nothing license = Nothing + mlsGroupInfoRepair = Nothing arbiterConnStr = secretText "" convCodeURI = Left (fromRight (error "Failed to parse test HttpsUrl") $ httpsUrlFromText "https://localhost") featureFlags = def @@ -590,6 +591,7 @@ spec = do passwordHashingOptions = PasswordHashingScrypt checkGroupInfo = Nothing license = Nothing + mlsGroupInfoRepair = Nothing arbiterConnStr = secretText "" convCodeURI = Left (fromRight (error "Failed to parse test HttpsUrl") $ httpsUrlFromText "https://localhost") featureFlags = def diff --git a/services/background-worker/test/Test/Wire/Util.hs b/services/background-worker/test/Test/Wire/Util.hs index 8b896ab30f0..3beb5742d4c 100644 --- a/services/background-worker/test/Test/Wire/Util.hs +++ b/services/background-worker/test/Test/Wire/Util.hs @@ -93,6 +93,7 @@ testEnv = do passwordHashingOptions = PasswordHashingScrypt checkGroupInfo = Nothing license = Nothing + mlsGroupInfoRepair = Nothing arbiterConnStr = secretText "" convCodeURI = Left (fromRight (error "Failed to parse test HttpsUrl") $ httpsUrlFromText "https://localhost") featureFlags = def diff --git a/services/galley/galley.integration.yaml b/services/galley/galley.integration.yaml index 984ec57dee3..cb027aff027 100644 --- a/services/galley/galley.integration.yaml +++ b/services/galley/galley.integration.yaml @@ -52,6 +52,7 @@ settings: exposeInvitationURLsTeamAllowlist: [] maxConvSize: 16 intraListing: false + mlsGroupInfoRepair: true conversationCodeURI: https://account.wire.com/conversation-join/ concurrentDeletionEvents: 1024 deleteConvThrottleMillis: 0 diff --git a/services/galley/src/Galley/API/Internal.hs b/services/galley/src/Galley/API/Internal.hs index 4e4b9d3d777..081daa5540e 100644 --- a/services/galley/src/Galley/API/Internal.hs +++ b/services/galley/src/Galley/API/Internal.hs @@ -30,11 +30,13 @@ import Control.Exception.Safe (catchAny) import Control.Lens hiding (Getter, Setter, (.=)) import Data.ByteString.UTF8 qualified as UTF8 import Data.Default +import Data.Domain (Domain, domainText) import Data.Id as Id import Data.Json.Util (ToJSONObject (toJSONObject)) import Data.Map qualified as Map import Data.Qualified import Data.Range +import Data.Set qualified as Set import Data.Singletons import Data.Time import Galley.API.LegalHold (unsetTeamLegalholdWhitelistedH) @@ -116,6 +118,7 @@ internalAPI = <@> mkNamedAPI @"connect" createConnectConversation <@> mkNamedAPI @"get-conversation-clients" iGetMLSClientListForConv <@> mkNamedAPI @"get-conversation-member-clients" iGetMLSMemberClients + <@> mkNamedAPI @"delete-mls-member-client" iDeleteMLSMemberClient <@> mkNamedAPI @"guard-legalhold-policy-conflicts" guardLegalholdPolicyConflictsH <@> legalholdWhitelistedTeamsAPI <@> iTeamsAPI @@ -559,3 +562,22 @@ iGetMLSMemberClients gid = do | (quid, clients) <- Map.assocs (unClientMap cm), (cid, idx) <- Map.assocs clients ] + +-- | Deletes a single member-client row; support/ops manual repair tool and +-- damage injector for integration tests. +iDeleteMLSMemberClient :: + forall r. + (Member ConversationStore r, Member P.TinyLog r) => + GroupId -> + Domain -> + UserId -> + ClientId -> + Sem r () +iDeleteMLSMemberClient gid dom uid cid = do + P.warn $ + Log.msg ("deleting MLS member-client row" :: ByteString) + . Log.field "groupId" (UTF8.fromString (show gid)) + . Log.field "userDomain" (domainText dom) + . Log.field "user" (idToText uid) + . Log.field "client" (clientToText cid) + ConversationStore.removeMLSClients gid (Qualified uid dom) (Set.singleton cid) diff --git a/services/galley/src/Galley/App.hs b/services/galley/src/Galley/App.hs index a0d76bded9a..6b1f4d2cd89 100644 --- a/services/galley/src/Galley/App.hs +++ b/services/galley/src/Galley/App.hs @@ -110,7 +110,7 @@ import Wire.CodeStore.Postgres import Wire.ConversationStore (ConversationStore, MLSCommitLockStore) import Wire.ConversationStore.Cassandra (MigrationError (..), interpretConversationStoreByMigration, interpretMLSCommitLockStoreToCassandra) import Wire.ConversationSubsystem -import Wire.ConversationSubsystem.Interpreter (ConversationSubsystemError, GroupInfoCheckEnabled (..), IntraListing (IntraListing), interpretConversationSubsystem) +import Wire.ConversationSubsystem.Interpreter (ConversationSubsystemError, GroupInfoCheckEnabled (..), GroupInfoRepairEnabled (..), IntraListing (IntraListing), interpretConversationSubsystem) import Wire.CustomBackendStore import Wire.CustomBackendStore.Cassandra import Wire.CustomBackendStore.DualWrite @@ -203,6 +203,7 @@ import Wire.UserGroupStore.Postgres (interpretUserGroupStoreToPostgres) type GalleyEffects = '[ MeetingsSubsystem, ConversationSubsystem, + Input (Maybe GroupInfoRepairEnabled), Licensing, MeetingNotifier, JobSubsystem, @@ -594,6 +595,7 @@ evalGalley e = . interpretJobSubsystem defaultJobSubsystemConfig . interpretMeetingNotifier . licenseInterpreter + . runInputConst (GroupInfoRepairEnabled <$> e._options._settings._mlsGroupInfoRepair) . interpretConversationSubsystem . Meeting.interpretMeetingsSubsystem (meetingSystemConfig (e ^. options)) where From e5da1fcb5610edc035652a60de3f9bb464614853 Mon Sep 17 00:00:00 2001 From: Gautier DI FOLCO Date: Fri, 13 Nov 2026 14:52:51 +0100 Subject: [PATCH 2/5] Hello CI From 3e14a80187c2f99ad5efd6f90d82ddfdc47fdcb1 Mon Sep 17 00:00:00 2001 From: Gautier DI FOLCO Date: Fri, 13 Nov 2026 16:48:41 +0000 Subject: [PATCH 3/5] WPB-29949: trigger ghost-row repair with a non-member external commit --- integration/test/Test/MLS.hs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/integration/test/Test/MLS.hs b/integration/test/Test/MLS.hs index 8f7bb1fb8fc..5dc3753b0f9 100644 --- a/integration/test/Test/MLS.hs +++ b/integration/test/Test/MLS.hs @@ -1074,8 +1074,8 @@ testExternalCommitDuplicateClient = do testGhostMemberRowRepair :: (HasCallStack) => App () testGhostMemberRowRepair = do [alice, bob] <- createAndConnectUsers [OwnDomain, OwnDomain] - [alice1, bob1] <- traverse (createMLSClient def) [alice, bob] - traverse_ (void . uploadNewKeyPackage def) [bob1] + [alice1, alice2, bob1] <- traverse (createMLSClient def) [alice, alice, bob] + traverse_ (uploadNewKeyPackage def) [alice2, bob1] convId <- createNewGroup def alice1 void $ createAddCommit alice1 convId [bob] >>= sendAndConsumeCommitBundle @@ -1092,8 +1092,11 @@ testGhostMemberRowRepair = do (\m -> nPayload m %. "type" `isEqual` "conversation.mls-message-add") wsBob - -- Any commit bundle triggers the repair from the stored ratchet tree. - void $ createExternalCommit convId alice1 Nothing >>= sendAndConsumeCommitBundle + -- Any commit bundle triggers the repair from the stored ratchet tree; the + -- external commit must come from a non-member (galley rejects external commits + -- from existing members without a self-remove proposal), so a fresh client of + -- alice joins here. + void $ createExternalCommit convId alice2 Nothing >>= sendAndConsumeCommitBundle -- bob1's row is back with its TRUE leaf index. gs <- getClientGroupState bob1 From 02e0fc939cb27c391ea34d67a75876091251a894 Mon Sep 17 00:00:00 2001 From: Gautier DI FOLCO Date: Fri, 13 Nov 2026 18:07:59 +0000 Subject: [PATCH 4/5] WPB-29949: defer alice2 key package until the external join The first commit of a conversation includes the sender in the Add action, so galley checks that all of alice's MLS-capable clients are covered. An already-registered key package for the non-member client alice2 made that check fail with 409 mls-client-mismatch. Upload alice2's key package right before her external commit instead, and give bob1 two key packages so the final re-add can still claim one. --- integration/test/Test/MLS.hs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/integration/test/Test/MLS.hs b/integration/test/Test/MLS.hs index 5dc3753b0f9..f20fbb48ef5 100644 --- a/integration/test/Test/MLS.hs +++ b/integration/test/Test/MLS.hs @@ -1075,7 +1075,12 @@ testGhostMemberRowRepair :: (HasCallStack) => App () testGhostMemberRowRepair = do [alice, bob] <- createAndConnectUsers [OwnDomain, OwnDomain] [alice1, alice2, bob1] <- traverse (createMLSClient def) [alice, alice, bob] - traverse_ (uploadNewKeyPackage def) [alice2, bob1] + -- Bob needs a KP for the add commit and another for the final re-add. + -- Alice2's KP must NOT exist yet: the first commit of a conversation + -- validates the sender's user against brig (all MLS-capable clients must be + -- covered by Add proposals), so an unused KP for alice2 here would make + -- galley reject the commit with 409 mls-client-mismatch. + traverse_ (uploadNewKeyPackage def) [bob1, bob1] convId <- createNewGroup def alice1 void $ createAddCommit alice1 convId [bob] >>= sendAndConsumeCommitBundle @@ -1092,6 +1097,8 @@ testGhostMemberRowRepair = do (\m -> nPayload m %. "type" `isEqual` "conversation.mls-message-add") wsBob + -- alice2 needs a registered KP to join via external commit. + void $ uploadNewKeyPackage def alice2 -- Any commit bundle triggers the repair from the stored ratchet tree; the -- external commit must come from a non-member (galley rejects external commits -- from existing members without a self-remove proposal), so a fresh client of From b02addaabdf871aa24558fce3cdc973f12926916 Mon Sep 17 00:00:00 2001 From: Gautier DI FOLCO Date: Fri, 13 Nov 2026 19:30:52 +0000 Subject: [PATCH 5/5] WPB-29949: keep bob1's keystore when resetting group state setClientGroupState bob1 def also wiped the mls-cli keystore holding bob1's registered signature key, so the re-add key package was generated with a fresh identity key and brig rejected it with 400 'Unrecognised signature key'. Reset only the group state. --- integration/test/Test/MLS.hs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/integration/test/Test/MLS.hs b/integration/test/Test/MLS.hs index f20fbb48ef5..b9cb81fc592 100644 --- a/integration/test/Test/MLS.hs +++ b/integration/test/Test/MLS.hs @@ -1133,9 +1133,13 @@ testGhostMemberRowRepair = do mp <- createRemoveCommit alice1 convId [bob1] void $ sendAndConsumeCommitBundle mp -- bob1 is in membersToBeRemoved for the remove commit, so the harness never - -- lets it consume that commit and its local group state goes stale; reset it - -- or the re-add welcome trips the existing-client assertion in consumeWelcome. - setClientGroupState bob1 def + -- lets it consume that commit and its local group state goes stale; reset + -- the group state (keeping the keystore with bob1's registered signature + -- key, or the re-add KP upload fails with 400 'Unrecognised signature key') + -- or the re-add welcome trips the existing-client assertion in + -- consumeWelcome. + gs0 <- getClientGroupState bob1 + setClientGroupState bob1 def {keystore = gs0.keystore, credType = gs0.credType} void $ uploadNewKeyPackage def bob1 void $ createAddCommit alice1 convId [bob] >>= sendAndConsumeCommitBundle