diff --git a/changelog.d/19749.feature b/changelog.d/19749.feature new file mode 100644 index 00000000000..8c32d7534c6 --- /dev/null +++ b/changelog.d/19749.feature @@ -0,0 +1,2 @@ +Partial [MSC4311](https://github.com/matrix-org/matrix-spec-proposals/pull/4311) implementation: `invite_room_state` and `knock_room_state` are now exchanged + as full PDUs over federation, allowing receiving servers to verify room IDs. Contributed by @FrenchGithubUser @Famedly. diff --git a/synapse/events/utils.py b/synapse/events/utils.py index f038fb5578d..63407c290bb 100644 --- a/synapse/events/utils.py +++ b/synapse/events/utils.py @@ -1019,15 +1019,6 @@ def strip_event(event: EventBase) -> JsonDict: Stripped state events can only have the `sender`, `type`, `state_key` and `content` properties present. """ - # MSC4311: Ensure the create event is available on invites and knocks. - # TODO: Implement the rest of MSC4311 - if ( - event.room_version.msc4291_room_ids_as_hashes - and event.type == EventTypes.Create - and event.get_state_key() == "" - ): - return event.get_pdu_json() - return { "type": event.type, "state_key": event.state_key, @@ -1036,6 +1027,22 @@ def strip_event(event: EventBase) -> JsonDict: } +def strip_event_dict(pdu_dict: JsonDict) -> JsonDict: + """Strip a PDU dict to stripped state format (4 fields only). + + Used to convert full PDUs received over federation (MSC4311) to the + stripped state format expected by the Client-Server API. + + Callers must pre-filter to ensure pdu_dict has a non-empty "type" key. + """ + return { + "type": pdu_dict["type"], + "state_key": pdu_dict.get("state_key", ""), + "content": pdu_dict.get("content", {}), + "sender": pdu_dict.get("sender", ""), + } + + def parse_stripped_state_event(raw_stripped_event: Any) -> StrippedStateEvent | None: """ Given a raw value from an event's `unsigned` field, attempt to parse it into a diff --git a/synapse/federation/federation_client.py b/synapse/federation/federation_client.py index 78a1900c731..391fa4a30b1 100644 --- a/synapse/federation/federation_client.py +++ b/synapse/federation/federation_client.py @@ -72,6 +72,7 @@ from synapse.logging.opentracing import SynapseTags, log_kv, set_tag, tag_args, trace from synapse.metrics import SERVER_NAME_LABEL from synapse.types import JsonDict, StrCollection, UserID, get_domain_from_id +from synapse.types.state import StateFilter from synapse.util.async_helpers import concurrently_execute from synapse.util.caches.expiringcache import ExpiringCache from synapse.util.duration import Duration @@ -1350,6 +1351,27 @@ async def _do_send_invite( """ time_now = self._clock.time_msec() + stripped = pdu.unsigned.get("invite_room_state", []) + if stripped and pdu.room_version.msc4291_room_ids_as_hashes: + # MSC4311: upgrade stripped state to full PDUs so the receiving server + # can verify the room ID. Use the stripped state as a guide for which + # events to look up. + types = [ + (e.get("type", ""), e.get("state_key", "")) + for e in stripped + if isinstance(e, dict) and e.get("type") + ] + state_filter = StateFilter.from_types(types) + state_ids = await self.store.get_partial_filtered_current_state_ids( + pdu.room_id, state_filter + ) + state_events = await self.store.get_events(state_ids.values()) + invite_room_state_pdus: list = [ + e.get_pdu_json() for e in state_events.values() + ] + else: + invite_room_state_pdus = stripped + try: return await self.transport_layer.send_invite_v2( destination=destination, @@ -1358,7 +1380,7 @@ async def _do_send_invite( content={ "event": pdu.get_pdu_json(time_now), "room_version": room_version.identifier, - "invite_room_state": pdu.unsigned.get("invite_room_state", []), + "invite_room_state": invite_room_state_pdus, }, ) except HttpResponseException as e: diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py index 1bbe1444223..e6f0f161615 100644 --- a/synapse/federation/federation_server.py +++ b/synapse/federation/federation_server.py @@ -957,19 +957,22 @@ async def on_send_knock_request( Returns: The stripped room state. """ - _, context = await self._on_send_membership_event( + event, context = await self._on_send_membership_event( origin, content, Membership.KNOCK, room_id ) - # Retrieve stripped state events from the room and send them back to the remote - # server. This will allow the remote server's clients to display information - # related to the room while the knock request is pending. - stripped_room_state = ( - await self.store.get_stripped_room_state_from_event_context( + if event.room_version.msc4291_room_ids_as_hashes: + # MSC4311: return full PDUs so the knocking server can validate the room ID. + knock_room_state = await self.store.get_room_state_pdus_from_event_context( context, self._room_prejoin_state_types ) - ) - return {"knock_room_state": stripped_room_state} + else: + knock_room_state = ( + await self.store.get_stripped_room_state_from_event_context( + context, self._room_prejoin_state_types + ) + ) + return {"knock_room_state": knock_room_state} async def _on_send_membership_event( self, origin: str, content: JsonDict, membership_type: str, room_id: str diff --git a/synapse/federation/transport/server/federation.py b/synapse/federation/transport/server/federation.py index d783e6da518..8e3a84a3df9 100644 --- a/synapse/federation/transport/server/federation.py +++ b/synapse/federation/transport/server/federation.py @@ -28,10 +28,11 @@ Sequence, ) -from synapse.api.constants import Direction, EduTypes +from synapse.api.constants import Direction, EduTypes, EventTypes from synapse.api.errors import Codes, SynapseError -from synapse.api.room_versions import RoomVersions +from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, RoomVersions from synapse.api.urls import FEDERATION_UNSTABLE_PREFIX, FEDERATION_V2_PREFIX +from synapse.events.utils import strip_event_dict from synapse.federation.transport.server._base import ( Authenticator, BaseFederationServlet, @@ -522,9 +523,29 @@ async def on_PUT( if not isinstance(invite_room_state, list): invite_room_state = [] - # Synapse expects invite_room_state to be in unsigned, as it is in v1 - # API - + room_version_obj = KNOWN_ROOM_VERSIONS.get(room_version) + if room_version_obj and room_version_obj.msc4291_room_ids_as_hashes: + # MSC4311: invite_room_state contains full PDUs. Warn if m.room.create + # is absent, then strip to 4-field format for C-S API storage. + create_event_present = any( + isinstance(e, dict) + and e.get("type") == EventTypes.Create + and e.get("state_key") == "" + for e in invite_room_state + ) + if not create_event_present: + logger.warning( + "invite_room_state from %s for room %s is missing m.room.create", + origin, + room_id, + ) + invite_room_state = [ + strip_event_dict(e) + for e in invite_room_state + if isinstance(e, dict) and e.get("type") + ] + + # Synapse expects invite_room_state to be in unsigned, as it is in v1 API event.setdefault("unsigned", {})["invite_room_state"] = invite_room_state result = await self.handler.on_invite_request( diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index b3444dd2ef0..f4b95f3076a 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -59,6 +59,7 @@ from synapse.event_auth import validate_event_for_room_version from synapse.events import EventBase from synapse.events.snapshot import EventContext, UnpersistedEventContextBase +from synapse.events.utils import strip_event_dict from synapse.events.validator import EventValidator from synapse.federation.federation_client import InvalidResponseError from synapse.handlers.pagination import PURGE_PAGINATION_LOCK_NAME @@ -865,19 +866,37 @@ async def do_knock( # further information about the room in the form of partial state events knock_response = await self.federation_client.send_knock(target_hosts, event) - # Store any stripped room state events in the "unsigned" key of the event. + # Store room state events in the "unsigned" key of the event. # This is a bit of a hack and is cribbing off of invites. Basically we # store the room state here and retrieve it again when this event appears # in the invitee's sync stream. It is stripped out for all other local users. - stripped_room_state = knock_response.get("knock_room_state") + knock_room_state = knock_response.get("knock_room_state") - if stripped_room_state is None: + if knock_room_state is None: raise KeyError("Missing 'knock_room_state' field in send_knock response") - if not isinstance(stripped_room_state, list): + if not isinstance(knock_room_state, list): raise TypeError("'knock_room_state' has wrong type") - event.unsigned["knock_room_state"] = stripped_room_state + # MSC4311: knock_room_state contains full PDUs over federation. + # Validate that m.room.create is present, then strip to stripped state for clients. + create_event_present = any( + isinstance(e, dict) + and e.get("type") == EventTypes.Create + and e.get("state_key") == "" + for e in knock_room_state + ) + if not create_event_present: + logger.warning( + "knock_room_state for room %s is missing m.room.create event", + event.room_id, + ) + + event.unsigned["knock_room_state"] = [ + strip_event_dict(e) + for e in knock_room_state + if isinstance(e, dict) and e.get("type") + ] context = EventContext.for_outlier(self._storage_controllers) stream_id = await self._federation_event_handler.persist_events_and_notify( diff --git a/synapse/storage/databases/main/events_worker.py b/synapse/storage/databases/main/events_worker.py index cc79b8042bd..6e250994145 100644 --- a/synapse/storage/databases/main/events_worker.py +++ b/synapse/storage/databases/main/events_worker.py @@ -1151,6 +1151,42 @@ async def get_stripped_room_state_from_event_context( return [strip_event(e) for e in state_to_include.values()] + async def get_room_state_pdus_from_event_context( + self, + context: EventContext, + state_keys_to_include: StateFilter, + membership_user_id: str | None = None, + ) -> list[JsonDict]: + """ + Retrieve room state events as full PDUs, for use in federation + invite_room_state and knock_room_state (MSC4311). + + Args: + context: The event context to retrieve state of the room from. + state_keys_to_include: The state events to include, for each event type. + membership_user_id: An optional user ID to include the membership state + events of. + + Returns: + A list of full PDU dicts representing the room state. + """ + if membership_user_id: + types = chain( + state_keys_to_include.to_types(), + [(EventTypes.Member, membership_user_id)], + ) + state_filter = StateFilter.from_types(types) + else: + state_filter = state_keys_to_include + selected_state_ids = await context.get_current_state_ids(state_filter) + + assert selected_state_ids is not None + + selected_state_ids = state_filter.filter_state(selected_state_ids) + state_to_include = await self.get_events(selected_state_ids.values()) + + return [e.get_pdu_json() for e in state_to_include.values()] + def _maybe_start_fetch_thread(self) -> None: """Starts an event fetch thread if we are not yet at the maximum number.""" with self._event_fetch_lock: diff --git a/tests/federation/test_federation_server.py b/tests/federation/test_federation_server.py index a40e0b06807..5f15987055d 100644 --- a/tests/federation/test_federation_server.py +++ b/tests/federation/test_federation_server.py @@ -43,6 +43,7 @@ from synapse.util.clock import Clock from tests import unittest +from tests.server import FakeChannel from tests.unittest import override_config logger = logging.getLogger(__name__) @@ -714,6 +715,132 @@ def test_send_join_contributes_to_room_join_rate_limit_and_is_limited(self) -> N # is probably sufficient to reassure that the bucket is updated. +class MSC4311FederationInviteTestCase(unittest.FederatingHomeserverTestCase): + """MSC4311: Tests for invite_room_state validation and stripping over federation.""" + + servlets = [ + admin.register_servlets, + room.register_servlets, + login.register_servlets, + ] + + def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: + super().prepare(reactor, clock, hs) + self.local_user = self.register_user("user", "pass") + self.remote_room_id = f"!room:{self.OTHER_SERVER_NAME}" + self.remote_sender = f"@creator:{self.OTHER_SERVER_NAME}" + + def _make_invite_request( + self, + invite_room_state: list, + room_version: str = RoomVersions.V10.identifier, + ) -> FakeChannel: + rv = KNOWN_ROOM_VERSIONS[room_version] + room_create_event = make_event_from_dict( + self.add_hashes_and_signatures_from_other_server( + { + "room_id": self.remote_room_id, + "sender": self.remote_sender, + "depth": 1, + "origin_server_ts": 1, + "type": EventTypes.Create, + "state_key": "", + "content": { + "creator": self.remote_sender, + "room_version": room_version, + }, + "auth_events": [], + "prev_events": [], + }, + rv, + ), + rv, + ) + invite_event = make_event_from_dict( + self.add_hashes_and_signatures_from_other_server( + { + "room_id": self.remote_room_id, + "sender": self.remote_sender, + "depth": 2, + "origin_server_ts": 2, + "type": EventTypes.Member, + "state_key": self.local_user, + "content": {"membership": Membership.INVITE}, + "auth_events": [room_create_event.event_id], + "prev_events": [room_create_event.event_id], + }, + rv, + ), + rv, + ) + return self.make_signed_federation_request( + "PUT", + f"/_matrix/federation/v2/invite/{self.remote_room_id}/{invite_event.event_id}", + content={ + "event": invite_event.get_dict(), + "invite_room_state": invite_room_state, + "room_version": room_version, + }, + ) + + def test_full_pdus_stripped_for_client(self) -> None: + """invite_room_state full PDUs are stripped to 4 fields for the C-S API.""" + rv = KNOWN_ROOM_VERSIONS[RoomVersions.V12.identifier] + create_pdu = make_event_from_dict( + self.add_hashes_and_signatures_from_other_server( + { + "room_id": self.remote_room_id, + "sender": self.remote_sender, + "depth": 1, + "origin_server_ts": 1, + "type": EventTypes.Create, + "state_key": "", + "content": { + "room_version": RoomVersions.V12.identifier, + }, + "auth_events": [], + "prev_events": [], + }, + rv, + ), + rv, + ) + # A full PDU has signatures, hashes, etc. + self.assertIn("signatures", create_pdu.get_pdu_json()) + + channel = self._make_invite_request( + invite_room_state=[create_pdu.get_pdu_json()], + room_version=RoomVersions.V12.identifier, + ) + self.assertEqual(channel.code, 200, channel.json_body) + + # Retrieve the stored invite event and verify invite_room_state is stripped. + store = self.hs.get_datastores().main + invite_memberships = self.get_success( + store.get_invited_rooms_for_local_user(self.local_user) + ) + self.assertEqual(len(invite_memberships), 1) + invite_event = self.get_success(store.get_event(invite_memberships[0].event_id)) + invite_state = invite_event.unsigned.get("invite_room_state", []) + + create_events = [e for e in invite_state if e.get("type") == EventTypes.Create] + self.assertEqual(len(create_events), 1) + create = create_events[0] + # Must be stripped state: only these 4 fields + self.assertIn("type", create) + self.assertIn("state_key", create) + self.assertIn("sender", create) + self.assertIn("content", create) + self.assertNotIn("signatures", create) + self.assertNotIn("hashes", create) + self.assertNotIn("auth_events", create) + + def test_missing_create_event_warns_but_accepts(self) -> None: + """invite_room_state without m.room.create is accepted with a warning.""" + channel = self._make_invite_request(invite_room_state=[]) + self.assertEqual(channel.code, 200, channel.json_body) + + class StripUnsignedFromEventsTestCase(unittest.TestCase): """ Test to make sure that we handle the raw JSON events from federation carefully and diff --git a/tests/federation/transport/test_knocking.py b/tests/federation/transport/test_knocking.py index ec705676cce..cec316def36 100644 --- a/tests/federation/transport/test_knocking.py +++ b/tests/federation/transport/test_knocking.py @@ -24,7 +24,7 @@ from twisted.internet.testing import MemoryReactor from synapse.api.constants import EventTypes, JoinRules, Membership -from synapse.api.room_versions import RoomVersion, RoomVersions +from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, RoomVersion, RoomVersions from synapse.events import EventBase, builder from synapse.events.snapshot import EventContext from synapse.rest import admin @@ -43,6 +43,7 @@ def send_example_state_events_to_room( hs: "HomeServer", room_id: str, sender: str, + room_version: str = RoomVersions.V7.identifier, ) -> OrderedDict: """Adds some state to a room. State events are those that should be sent to a knocking user after they knock on the room, as well as some state that *shouldn't* be sent @@ -69,7 +70,6 @@ def send_example_state_events_to_room( self.get_success( event_injection.inject_event( hs, - room_version=RoomVersions.V7.identifier, room_id=room_id, sender=sender, type="com.example.secret", @@ -138,7 +138,6 @@ def send_example_state_events_to_room( self.get_success( event_injection.inject_event( hs, - room_version=RoomVersions.V7.identifier, room_id=room_id, sender=sender, type=event_type, @@ -149,11 +148,12 @@ def send_example_state_events_to_room( # Finally, we expect to see the m.room.create event of the room as part of the # stripped state. We don't need to inject this event though. + rv_obj = KNOWN_ROOM_VERSIONS[room_version] + create_content: dict = {"room_version": room_version} + if not rv_obj.implicit_room_creator: + create_content["creator"] = sender room_state[EventTypes.Create] = { - "content": { - "creator": sender, - "room_version": RoomVersions.V7.identifier, - }, + "content": create_content, "state_key": "", } @@ -164,11 +164,11 @@ def check_knock_room_state_against_room_state( knock_room_state: list[dict], expected_room_state: dict, ) -> None: - """Test a list of stripped room state events received over federation against a + """Test a list of state events received over federation or sync against a dict of expected state events. Args: - knock_room_state: The list of room state that was received over federation. + knock_room_state: The list of room state events received. expected_room_state: A dict containing the room state we expect to see in `knock_room_state`. """ @@ -189,9 +189,6 @@ def check_knock_room_state_against_room_state( expected_room_state[event_type]["state_key"], event["state_key"] ) - # Ensure the event has been stripped - self.assertNotIn("signatures", event) - # Pop once we've found and processed a state event expected_room_state.pop(event_type) @@ -250,17 +247,18 @@ def test_room_state_returned_when_knocking(self) -> None: fake_knocking_user_id = "@user:other.example.com" - # Create a room with a room version that includes knocking + # Create a room with a room version that includes knocking and full PDU state + # (msc4291_room_ids_as_hashes required for MSC4311 full PDU knock_room_state) room_id = self.helper.create_room_as( "u1", is_public=False, - room_version=RoomVersions.V7.identifier, + room_version=RoomVersions.V12.identifier, tok=user_token, ) # Update the join rules and add additional state to the room to check for later expected_room_state = self.send_example_state_events_to_room( - self.hs, room_id, user_id + self.hs, room_id, user_id, room_version=RoomVersions.V12.identifier ) channel = self.make_signed_federation_request( @@ -271,7 +269,7 @@ def test_room_state_returned_when_knocking(self) -> None: fake_knocking_user_id, # Inform the remote that we support the room version of the room we're # knocking on - RoomVersions.V7.identifier, + RoomVersions.V12.identifier, ), ) self.assertEqual(200, channel.code, channel.result) @@ -296,7 +294,7 @@ def test_room_state_returned_when_knocking(self) -> None: self.clock, self.hs.hostname, self.hs.signing_key, - room_version=RoomVersions.V7, + room_version=RoomVersions.V12, event_dict=knock_event, ) @@ -317,7 +315,11 @@ def test_room_state_returned_when_knocking(self) -> None: # Check that we got the stripped room state in return room_state_events = channel.json_body["knock_room_state"] - # Validate the stripped room state events + # Validate content/state_key of each event self.check_knock_room_state_against_room_state( room_state_events, expected_room_state ) + + # MSC4311: knock_room_state over federation must be full PDUs, not stripped state + for event in room_state_events: + self.assertIn("signatures", event)