From b209bca4a6ab0636f9fc76ca7cd7f737bc28f387 Mon Sep 17 00:00:00 2001 From: "Evgeny @ SimpleX Chat" <259188159+evgeny-simplex@users.noreply.github.com> Date: Tue, 3 Nov 2026 21:58:53 +0000 Subject: [PATCH] core: use the member key sent by the channel owner on role change (#7883) * core: use the member key sent by the channel owner on role change * core: check the member key on role change in one branch, rename test * core: describe the role change test by what it checks --- src/Simplex/Chat/Library/Subscriber.hs | 14 ++++---- tests/ChatTests/Groups.hs | 50 ++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 6 deletions(-) diff --git a/src/Simplex/Chat/Library/Subscriber.hs b/src/Simplex/Chat/Library/Subscriber.hs index d36bdc194e..18bc7aab74 100644 --- a/src/Simplex/Chat/Library/Subscriber.hs +++ b/src/Simplex/Chat/Library/Subscriber.hs @@ -3462,11 +3462,13 @@ processAgentMessageConn cxt user@User {userId} entity gks_ corrId agentConnId ag let allowCreate = useRelays' gInfo && senderRole == GROwner && isRosterRole memRole && isJust memberKey_ withStore' (\db -> runExceptT $ getCreateUnknownGMByMemberId db cxt user gInfo memId (nameFromMemberId memId) defaultRole allowCreate) >>= \case Right (Just (member, created)) - -- just created (keyless, and allowCreate ensured the event carries its key): pin key + role - | created, Just (MemberKey pubKey) <- memberKey_ -> - let gEvent = RGEMemberRole (groupMemberId' member) (fromLocalProfile $ memberProfile member) memRole - in changeMemberRole gInfo member created (\db -> void $ applyMemberKeyRole db member pubKey memRole) gEvent (not $ useRelays' gInfo) - -- known member: apply the role (its key is established via roster/intro; the event's key is ignored) + -- allowCreate ensured the event carries its key: keep a different stored key, else pin the key if absent and set the role + | allowCreate, Just (MemberKey pubKey) <- memberKey_ -> case memberPubKey member of + Just k | k /= pubKey -> messageError ("x.grp.mem.role: member key conflict, keeping trusted key, memberId=" <> safeDecodeUtf8 (strEncode memId)) $> Nothing + _ -> + let gEvent = RGEMemberRole (groupMemberId' member) (fromLocalProfile $ memberProfile member) memRole + in changeMemberRole gInfo member created (\db -> void $ applyMemberKeyRole db member pubKey memRole) gEvent (not $ useRelays' gInfo) + -- known member: apply the role (the event's key is ignored) | otherwise -> let gEvent = RGEMemberRole (groupMemberId' member) (fromLocalProfile $ memberProfile member) memRole in changeMemberRole gInfo member created (\db -> updateGroupMemberRole db user member memRole) gEvent (not $ useRelays' gInfo) @@ -3475,7 +3477,7 @@ processAgentMessageConn cxt user@User {userId} entity gks_ corrId agentConnId ag | otherwise -> messageError "x.grp.mem.role with unknown member ID" $> Nothing where GroupMember {memberId = membershipMemId} = membership - -- applyMember writes the change (role, or role + pinned key for a freshly TOFU-created member); + -- applyMember writes the change (role, or role + pinned key for a keyless member); -- the delivery scope (relay forwarding) is computed on the pre-change role changeMemberRole gInfo' member@GroupMember {memberRole = fromRole} created applyMember gEvent createItem | fromRole == GRRelay = diff --git a/tests/ChatTests/Groups.hs b/tests/ChatTests/Groups.hs index 34217f36e4..c5fe2a263a 100644 --- a/tests/ChatTests/Groups.hs +++ b/tests/ChatTests/Groups.hs @@ -352,6 +352,8 @@ chatGroupTests = do it "role transitions update the roster (mod <-> admin, admin -> non-roster)" testChannelRoleTransitionsUpdateRoster it "malicious relay cannot downgrade or re-key a roster-established moderator via XGrpMemNew" testChannelRelayCannotDowngradeRosterMember it "malicious relay cannot forge a privileged member via XGrpMemNew forwarded as the owner" testChannelRelayCannotForgePrivilegedMember + it "owner promotion with a different member key keeps the member role" testChannelPromotionDifferentKeyKeepsRole + it "owner promotion pins the key of a member known without a key" testChannelPromotionPinsMemberKey it "should add relay to channel with roster (relay caches roster before joinable)" testChannelAddRelayWithRoster it "roster blob spanning multiple chunks reassembles" testChannelRosterMultipartReassembly it "corrupted roster blob is rejected on digest mismatch" testChannelRosterDigestMismatchRejected @@ -11337,6 +11339,54 @@ testChannelRelayCannotForgePrivilegedMember ps = withCCTransaction cc $ \db -> DB.query db "SELECT member_role, member_pub_key FROM group_members WHERE local_display_name = ?" (Only name) +testChannelPromotionDifferentKeyKeepsRole :: HasCallStack => TestParams -> IO () +testChannelPromotionDifferentKeyKeepsRole ps = + withNewTestChat ps "alice" aliceProfile $ \alice -> + withNewTestChatOpts ps relayTestOpts "bob" bobProfile $ \bob -> + withNewTestChat ps "cath" cathProfile $ \cath -> + withNewTestChat ps "dan" danProfile $ \dan -> do + (shortLink, fullLink) <- prepareChannel1Relay "team" alice bob + memberJoinChannel "team" [bob] [alice] shortLink fullLink cath + memberJoinChannel "team" [bob] [alice] shortLink fullLink dan + danId <- memberIdByName alice "dan" + connId <- relayConnIdToMember bob "cath" + g <- C.newRandom + kp <- atomically $ C.generateKeyPair g + let relayKey = fst kp :: C.PublicKeyEd25519 + memInfo = MemberInfo {memberId = danId, memberRole = GRObserver, v = Nothing, profile = danProfile, memberKey = Just (MemberKey relayKey)} + agentSendBody bob connId $ chatMsgToBody $ ChatMessage chatInitialVRange Nothing (XGrpMemNew memInfo Nothing) + cath <## "#team: bob introduced dan (Daniel) in the channel" + threadDelay 1000000 + alice ##> "/mr #team dan admin" + alice <## "#team: you changed the role of dan to admin (signed)" + concurrentlyN_ + [ bob <## "#team: alice changed the role of dan from observer to admin (signed)", + dan <## "#team: alice changed your role from observer to admin (signed)", + cath <##. "error: x.grp.mem.role: member key conflict, keeping trusted key, memberId=" + ] + checkMemberRow cath "dan" (Just "observer") + +testChannelPromotionPinsMemberKey :: HasCallStack => TestParams -> IO () +testChannelPromotionPinsMemberKey ps = + withNewTestChat ps "alice" aliceProfile $ \alice -> + withNewTestChatOpts ps relayTestOpts "bob" bobProfile $ \bob -> + withNewTestChat ps "cath" cathProfile $ \cath -> + withNewTestChat ps "dan" danProfile $ \dan -> do + (shortLink, fullLink) <- prepareChannel1Relay "team" alice bob + memberJoinChannel "team" [bob] [alice] shortLink fullLink cath + memberJoinChannel "team" [bob] [alice] shortLink fullLink dan + threadDelay 1000000 + alice ##> "/block for all #team dan" + alice <## "#team: you blocked dan (signed)" + bob <## "#team: alice blocked dan (signed)" + cath <##. "#team: alice blocked an unknown member, creating unknown member record" + cath .<##. ("#team: alice blocked", "(signed)") + threadDelay 1000000 + promoteChannelMember "team" alice bob dan [cath] + danId <- getMemberIdByName alice "dan" + (_, danKey) <- getMemberRoleKey alice danId + getMemberRoleKey cath danId `shouldReturn` ("member", danKey) + testChannelRemoveMemberSigned :: HasCallStack => TestParams -> IO () testChannelRemoveMemberSigned ps = withNewTestChat ps "alice" aliceProfile $ \alice ->