From cc2477541c8cd6597d13be793db184dd0367fbff Mon Sep 17 00:00:00 2001 From: Kegan Dougal <7190048+kegsay@users.noreply.github.com> Date: Fri, 16 Oct 2026 14:57:00 +0200 Subject: [PATCH] Name our own user on a join we send to a member key room MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A membership event carries the claim about who its member key belongs to, and a join that makes no claim leaves its sender attributable to nobody: not routable, not shown to clients, soft failed by every server that receives anything further from them. The joins this server sent to member key rooms made no claim. The gap was in where the linkage information is filled in. For events a client asks us to create, that happens while the event dict is being translated into member keys, which is also where a client's own attempt to assert linkage information is thrown away. A join sent over federation never goes through that path: the template comes from the resident server, we fill in the client's content and sign it, and nothing along the way says who we are. So fill it in there too, once the room version coming back from `/make_join` shows the room wants it: our domain, and the user's localpart unless they are erased, in which case their name is not published again. Whatever the client asked for is dropped first, as it is everywhere else — a client may not say who a member key belongs to, only the server holding it may. The event is then signed by the member key and co-signed by us, which is what makes the claim worth anything to the server receiving it. Also stop recording a link against a member key this server holds. Links describe other servers' keys, and for our own we already know the answer, so the row would be at best redundant. It becomes actively wrong once a member key is moved between servers: the new server's claim about it is still not ours to write down against our own copy. The tests drive the outbound half with a stubbed resident server, so what we put on the wire is checked rather than assumed. The tests for the inbound half build their events by hand and so never exercised this. Co-Authored-By: Claude Opus 5 (1M context) --- synapse/federation/federation_client.py | 12 +++ synapse/handlers/member_keys.py | 17 ++++ synapse/handlers/member_links.py | 9 ++ tests/federation/test_member_key_links.py | 114 +++++++++++++++++++++- 4 files changed, 151 insertions(+), 1 deletion(-) diff --git a/synapse/federation/federation_client.py b/synapse/federation/federation_client.py index 2c4812eeae..2395a4502a 100644 --- a/synapse/federation/federation_client.py +++ b/synapse/federation/federation_client.py @@ -1074,6 +1074,18 @@ class FederationClient(FederationBase): await self._member_key_handler.persist_pending_join_key( room_id, member_key ) + # Say who the key belongs to, and sign for it: a membership + # event with no linkage information leaves the member + # attributable to nobody, unroutable, and soft failed + # everywhere. Whatever the client asked for is replaced by + # what we are willing to vouch for. + pdu_dict["content"].pop(EventContentFields.MSC4430_LOCALPART, None) + pdu_dict["content"].pop(EventContentFields.MSC4430_DOMAIN, None) + pdu_dict["content"].update( + await self._member_key_handler.linkage_content_for_local_user( + user_id + ) + ) signers = await self._member_key_handler.get_signers_for_event_dict( room_version, pdu_dict ) diff --git a/synapse/handlers/member_keys.py b/synapse/handlers/member_keys.py index 4ae8801110..804f60991c 100644 --- a/synapse/handlers/member_keys.py +++ b/synapse/handlers/member_keys.py @@ -698,6 +698,23 @@ class MemberKeyHandler: return signers + async def linkage_content_for_local_user(self, user_id: str) -> JsonDict: + """The linkage information this server vouches for about one of its + users: our domain, and their localpart unless they are erased, in which + case their name is not published again. + + The counterpart of `_is_our_linkage`, for the places that have to fill + linkage information in rather than check it: an event we build for one + of our users in a room we are not in yet (a join over federation), where + `translate_event_dict` has no part to play. + """ + user = UserID.from_string(user_id) + assert self._is_mine_id(user_id), f"{user_id} is not a local user" + content: JsonDict = {EventContentFields.MSC4430_DOMAIN: user.domain} + if not await self._store.is_user_erased(user_id): + content[EventContentFields.MSC4430_LOCALPART] = user.localpart + return content + async def _is_our_linkage(self, user_id: str, content: JsonDict) -> bool: """Whether a membership event's linkage information is exactly what this server vouches for about `user_id`: our domain, and their localpart diff --git a/synapse/handlers/member_links.py b/synapse/handlers/member_links.py index b7f45e7011..07c993d5e5 100644 --- a/synapse/handlers/member_links.py +++ b/synapse/handlers/member_links.py @@ -292,6 +292,15 @@ class MemberLinkHandler: if not self._enabled or not member_keys: return + # A key of ours is not somebody else's claim to check: we know who it + # belongs to, and `msc4430_member_links` describes only other servers' + # keys. (After a member key is moved to another server, that server's + # claim is still not ours to record against our own copy of the key.) + ours = await self._store.get_local_member_keys_by_key(room_id, member_keys) + member_keys = [key for key in member_keys if ours.get(key) is None] + if not member_keys: + return + state_ids = await self._current_member_event_ids(room_id, member_keys) events = await self._store.get_events(list(state_ids.values())) diff --git a/tests/federation/test_member_key_links.py b/tests/federation/test_member_key_links.py index 3d7f0dbedd..c629f28c56 100644 --- a/tests/federation/test_member_key_links.py +++ b/tests/federation/test_member_key_links.py @@ -46,7 +46,7 @@ from synapse.crypto.event_signing import ( compute_event_signature, compute_member_key_room_signature, ) -from synapse.events import make_event_from_dict +from synapse.events import EventBase, make_event_from_dict from synapse.rest import admin from synapse.rest.client import login, room, sync from synapse.server import HomeServer @@ -460,3 +460,115 @@ class MemberKeyFederationTestCase(unittest.FederatingHomeserverTestCase): f"?ver={ROOM_VERSION.identifier}", ) self.assertEqual(HTTPStatus.BAD_REQUEST, channel.code, channel.result) + + +class OutboundMemberKeyJoinTestCase(unittest.FederatingHomeserverTestCase): + """The join *we* send when one of our users joins another server's member + key room: it must name our user and be signed for, or the room we join will + know nothing about who we are.""" + + servlets = [ + admin.register_servlets, + login.register_servlets, + room.register_servlets, + ] + + def default_config(self) -> JsonDict: + config = super().default_config() + config.setdefault("experimental_features", {})["msc4430_enabled"] = True + return config + + def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: + super().prepare(reactor, clock, hs) + self.store = hs.get_datastores().main + self.federation_client = hs.get_federation_client() + self.member_key_handler = hs.get_member_key_handler() + self.alice = self.register_user("alice", "pass") + self.room_id = "!room:" + OTHER + + def _make_membership_event(self, content: JsonDict | None = None) -> EventBase: + """Drive the outbound half of a join: the remote returns a template for + our member key, and we complete and sign it.""" + member_key = self.get_success( + self.member_key_handler.member_key_for_remote_join(self.room_id, self.alice) + ) + + async def template( + destination: str, + room_id: str, + user_id: str, + membership: str, + params: object, + ) -> JsonDict: + # What a resident of a member key room answers `/make_join` with. + self.assertEqual( + params[MemberKeys.MAKE_MEMBERSHIP_PARAM], # type: ignore[index] + member_key, + ) + return { + "room_version": ROOM_VERSION.identifier, + "event": { + "type": EventTypes.Member, + "room_id": room_id, + "sender": member_key, + "state_key": member_key, + "content": {"membership": Membership.JOIN}, + "depth": 12, + "prev_events": [], + "prev_state_events": [], + "origin_server_ts": self.clock.time_msec(), + }, + } + + self.federation_client.transport_layer.make_membership_event = template # type: ignore[method-assign] + _, event, _ = self.get_success( + self.federation_client.make_membership_event( + [OTHER], + self.room_id, + self.alice, + Membership.JOIN, + content if content is not None else {}, + params={MemberKeys.MAKE_MEMBERSHIP_PARAM: member_key}, + member_key=member_key, + ) + ) + return event + + def test_join_names_our_user_and_is_signed_for(self) -> None: + event = self._make_membership_event() + + self.assertEqual(event.content[EventContentFields.MSC4430_LOCALPART], "alice") + self.assertEqual( + event.content[EventContentFields.MSC4430_DOMAIN], self.hs.hostname + ) + # Signed by the member key, and vouched for by us. + self.assertEqual( + set(event.signatures.as_dict()), + {MemberKeys.SIGNATURE_ENTITY, self.hs.hostname}, + ) + # The key is kept, so a later event in that room is signed by the same one. + local_key = self.get_success( + self.store.get_local_member_key(self.room_id, self.alice) + ) + assert local_key is not None + self.assertEqual(local_key.member_key, event.sender) + + def test_clients_cannot_assert_linkage_on_a_remote_join(self) -> None: + event = self._make_membership_event( + { + EventContentFields.MSC4430_LOCALPART: "mallory", + EventContentFields.MSC4430_DOMAIN: "evil.example", + } + ) + self.assertEqual(event.content[EventContentFields.MSC4430_LOCALPART], "alice") + self.assertEqual( + event.content[EventContentFields.MSC4430_DOMAIN], self.hs.hostname + ) + + def test_erased_user_joins_without_a_localpart(self) -> None: + self.get_success(self.store.mark_user_erased(self.alice)) + event = self._make_membership_event() + self.assertNotIn(EventContentFields.MSC4430_LOCALPART, event.content) + self.assertEqual( + event.content[EventContentFields.MSC4430_DOMAIN], self.hs.hostname + )