From 1dbf57a2b54559521c57fa9ef2307f5206ddce2e Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Thu, 8 Sep 2022 14:49:37 +0100 Subject: [PATCH 01/10] Define an error code --- synapse/api/errors.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/synapse/api/errors.py b/synapse/api/errors.py index e6dea89c6d09..2381aa4e8320 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -100,6 +100,12 @@ class Codes(str, Enum): UNREDACTED_CONTENT_DELETED = "FI.MAU.MSC2815_UNREDACTED_CONTENT_DELETED" + # Returned for federation requests where we can't process a request as we + # can't ensure the sending server is in a room which is partial-stated on + # our side. + # Part of MSC3706-rei1. + UNABLE_DUE_TO_PARTIAL_STATE = "ORG.MATRIX.MSC3706_UNABLE_DUE_TO_PARTIAL_STATE" + class CodeMessageException(RuntimeError): """An exception with integer code and message string attributes. From 5bd2c5dbf3e494175526917f248dd708f55f6438 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Thu, 15 Sep 2022 11:39:33 +0100 Subject: [PATCH 02/10] Add `assert_host_in_room` --- synapse/handlers/event_auth.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/synapse/handlers/event_auth.py b/synapse/handlers/event_auth.py index c3ddc5d18253..320aa0cb7bff 100644 --- a/synapse/handlers/event_auth.py +++ b/synapse/handlers/event_auth.py @@ -160,6 +160,27 @@ async def check_host_in_room(self, room_id: str, host: str) -> bool: with Measure(self._clock, "check_host_in_room"): return await self._store.is_host_joined(room_id, host) + async def assert_host_in_room( + self, room_id: str, host: str, allow_partial_state_rooms: bool = False + ) -> None: + """ + Asserts that the host is in the room, or raises an AuthError. + + If the room is partial-stated, we raise an AuthError with the + UNABLE_DUE_TO_PARTIAL_STATE flag, unless `allow_partial_state_rooms` is true. + """ + if not allow_partial_state_rooms and await self._store.is_partial_state_room( + room_id + ): + raise AuthError( + 403, + "Unable to authorise you right now; room is partial-stated here.", + errcode=Codes.UNABLE_DUE_TO_PARTIAL_STATE, + ) + + if not await self.check_host_in_room(room_id, host): + raise AuthError(403, "Host not in room.") + async def check_restricted_join_rules( self, state_ids: StateMap[str], From 261c4bf94737896278fe145ee567dec89cf9104e Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Thu, 15 Sep 2022 12:09:24 +0100 Subject: [PATCH 03/10] Use assert_host_in_room --- synapse/federation/federation_server.py | 11 +++-------- synapse/handlers/federation.py | 14 +++----------- 2 files changed, 6 insertions(+), 19 deletions(-) diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py index 3bf84cf62515..907940e19eb0 100644 --- a/synapse/federation/federation_server.py +++ b/synapse/federation/federation_server.py @@ -530,13 +530,10 @@ async def _process_edu(edu_dict: JsonDict) -> None: async def on_room_state_request( self, origin: str, room_id: str, event_id: str ) -> Tuple[int, JsonDict]: + await self._event_auth_handler.assert_host_in_room(room_id, origin) origin_host, _ = parse_server_name(origin) await self.check_server_matches_acl(origin_host, room_id) - in_room = await self._event_auth_handler.check_host_in_room(room_id, origin) - if not in_room: - raise AuthError(403, "Host not in room.") - # we grab the linearizer to protect ourselves from servers which hammer # us. In theory we might already have the response to this query # in the cache so we could return it without waiting for the linearizer @@ -560,13 +557,10 @@ async def on_state_ids_request( if not event_id: raise NotImplementedError("Specify an event") + await self._event_auth_handler.assert_host_in_room(room_id, origin) origin_host, _ = parse_server_name(origin) await self.check_server_matches_acl(origin_host, room_id) - in_room = await self._event_auth_handler.check_host_in_room(room_id, origin) - if not in_room: - raise AuthError(403, "Host not in room.") - resp = await self._state_ids_resp_cache.wrap( (room_id, event_id), self._on_state_ids_request_compute, @@ -955,6 +949,7 @@ async def on_event_auth( self, origin: str, room_id: str, event_id: str ) -> Tuple[int, Dict[str, Any]]: async with self._server_linearizer.queue((origin, room_id)): + await self._event_auth_handler.assert_host_in_room(room_id, origin) origin_host, _ = parse_server_name(origin) await self.check_server_matches_acl(origin_host, room_id) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index dd4b9f66d10e..983c5ed03303 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -1150,9 +1150,7 @@ async def get_state_ids_for_pdu(self, room_id: str, event_id: str) -> List[str]: async def on_backfill_request( self, origin: str, room_id: str, pdu_list: List[str], limit: int ) -> List[EventBase]: - in_room = await self._event_auth_handler.check_host_in_room(room_id, origin) - if not in_room: - raise AuthError(403, "Host not in room.") + await self._event_auth_handler.assert_host_in_room(room_id, origin) # Synapse asks for 100 events per backfill request. Do not allow more. limit = min(limit, 100) @@ -1199,11 +1197,7 @@ async def get_persisted_pdu( ) if event: - in_room = await self._event_auth_handler.check_host_in_room( - event.room_id, origin - ) - if not in_room: - raise AuthError(403, "Host not in room.") + await self._event_auth_handler.assert_host_in_room(event.room_id, origin) events = await filter_events_for_server( self._storage_controllers, origin, [event] @@ -1221,9 +1215,7 @@ async def on_get_missing_events( latest_events: List[str], limit: int, ) -> List[EventBase]: - in_room = await self._event_auth_handler.check_host_in_room(room_id, origin) - if not in_room: - raise AuthError(403, "Host not in room.") + await self._event_auth_handler.assert_host_in_room(room_id, origin) # Only allow up to 20 events to be retrieved per request. limit = min(limit, 20) From 58a2ca158a6d8f0c66b8f612d94f3bf5065a9866 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Thu, 15 Sep 2022 12:23:28 +0100 Subject: [PATCH 04/10] Simplify control flow of get_persisted_pdu (drive-by cleanup) --- synapse/handlers/federation.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 983c5ed03303..a48f873a9a07 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -1196,17 +1196,17 @@ async def get_persisted_pdu( event_id, allow_none=True, allow_rejected=True ) - if event: - await self._event_auth_handler.assert_host_in_room(event.room_id, origin) - - events = await filter_events_for_server( - self._storage_controllers, origin, [event] - ) - event = events[0] - return event - else: + if not event: return None + await self._event_auth_handler.assert_host_in_room(event.room_id, origin) + + events = await filter_events_for_server( + self._storage_controllers, origin, [event] + ) + event = events[0] + return event + async def on_get_missing_events( self, origin: str, From 66d7230c4e2621b5897e9aa39cc614c83b72e906 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Thu, 15 Sep 2022 15:49:21 +0100 Subject: [PATCH 05/10] Rename check_ to is_ to more accurately describe it --- synapse/handlers/event_auth.py | 4 ++-- synapse/handlers/federation.py | 4 ++-- synapse/handlers/federation_event.py | 2 +- synapse/handlers/receipts.py | 2 +- synapse/handlers/room_summary.py | 6 ++---- synapse/handlers/typing.py | 2 +- tests/handlers/test_typing.py | 2 +- 7 files changed, 10 insertions(+), 12 deletions(-) diff --git a/synapse/handlers/event_auth.py b/synapse/handlers/event_auth.py index 320aa0cb7bff..f74fae77e0ec 100644 --- a/synapse/handlers/event_auth.py +++ b/synapse/handlers/event_auth.py @@ -156,7 +156,7 @@ async def get_user_which_could_invite( Codes.UNABLE_TO_GRANT_JOIN, ) - async def check_host_in_room(self, room_id: str, host: str) -> bool: + async def is_host_in_room(self, room_id: str, host: str) -> bool: with Measure(self._clock, "check_host_in_room"): return await self._store.is_host_joined(room_id, host) @@ -178,7 +178,7 @@ async def assert_host_in_room( errcode=Codes.UNABLE_DUE_TO_PARTIAL_STATE, ) - if not await self.check_host_in_room(room_id, host): + if not await self.is_host_in_room(room_id, host): raise AuthError(403, "Host not in room.") async def check_restricted_join_rules( diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index a48f873a9a07..583d5ecd7749 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -804,7 +804,7 @@ async def on_make_join_request( ) # now check that we are *still* in the room - is_in_room = await self._event_auth_handler.check_host_in_room( + is_in_room = await self._event_auth_handler.is_host_in_room( room_id, self.server_name ) if not is_in_room: @@ -1249,7 +1249,7 @@ async def exchange_third_party_invite( "state_key": target_user_id, } - if await self._event_auth_handler.check_host_in_room(room_id, self.hs.hostname): + if await self._event_auth_handler.is_host_in_room(room_id, self.hs.hostname): room_version_obj = await self.store.get_room_version(room_id) builder = self.event_builder_factory.for_room_version( room_version_obj, event_dict diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py index efcdb8405783..2d7cde750609 100644 --- a/synapse/handlers/federation_event.py +++ b/synapse/handlers/federation_event.py @@ -238,7 +238,7 @@ async def on_receive_pdu(self, origin: str, pdu: EventBase) -> None: # # Note that if we were never in the room then we would have already # dropped the event, since we wouldn't know the room version. - is_in_room = await self._event_auth_handler.check_host_in_room( + is_in_room = await self._event_auth_handler.is_host_in_room( room_id, self._server_name ) if not is_in_room: diff --git a/synapse/handlers/receipts.py b/synapse/handlers/receipts.py index d2bdb9c8be79..afaf3261dfc9 100644 --- a/synapse/handlers/receipts.py +++ b/synapse/handlers/receipts.py @@ -70,7 +70,7 @@ async def _received_remote_receipt(self, origin: str, content: JsonDict) -> None # If we're not in the room just ditch the event entirely. This is # probably an old server that has come back and thinks we're still in # the room (or we've been rejoined to the room by a state reset). - is_in_room = await self.event_auth_handler.check_host_in_room( + is_in_room = await self.event_auth_handler.is_host_in_room( room_id, self.server_name ) if not is_in_room: diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py index ebd445adcaf3..8d08625237bc 100644 --- a/synapse/handlers/room_summary.py +++ b/synapse/handlers/room_summary.py @@ -609,7 +609,7 @@ async def _is_local_room_accessible( # If this is a request over federation, check if the host is in the room or # has a user who could join the room. elif origin: - if await self._event_auth_handler.check_host_in_room( + if await self._event_auth_handler.is_host_in_room( room_id, origin ) or await self._store.is_host_invited(room_id, origin): return True @@ -624,9 +624,7 @@ async def _is_local_room_accessible( await self._event_auth_handler.get_rooms_that_allow_join(state_ids) ) for space_id in allowed_rooms: - if await self._event_auth_handler.check_host_in_room( - space_id, origin - ): + if await self._event_auth_handler.is_host_in_room(space_id, origin): return True logger.info( diff --git a/synapse/handlers/typing.py b/synapse/handlers/typing.py index a4cd8b8f0cee..0d8466af11d4 100644 --- a/synapse/handlers/typing.py +++ b/synapse/handlers/typing.py @@ -340,7 +340,7 @@ async def _recv_edu(self, origin: str, content: JsonDict) -> None: # If we're not in the room just ditch the event entirely. This is # probably an old server that has come back and thinks we're still in # the room (or we've been rejoined to the room by a state reset). - is_in_room = await self.event_auth_handler.check_host_in_room( + is_in_room = await self.event_auth_handler.is_host_in_room( room_id, self.server_name ) if not is_in_room: diff --git a/tests/handlers/test_typing.py b/tests/handlers/test_typing.py index 8adba29d7f9c..1a247f12e821 100644 --- a/tests/handlers/test_typing.py +++ b/tests/handlers/test_typing.py @@ -129,7 +129,7 @@ async def check_user_in_room(room_id: str, requester: Requester) -> None: async def check_host_in_room(room_id: str, server_name: str) -> bool: return room_id == ROOM_ID - hs.get_event_auth_handler().check_host_in_room = check_host_in_room + hs.get_event_auth_handler().is_host_in_room = check_host_in_room async def get_current_hosts_in_room(room_id: str): return {member.domain for member in self.room_members} From cfb8b315a814b287d9a5b9d5fe806b658eea22e2 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Thu, 15 Sep 2022 15:56:55 +0100 Subject: [PATCH 06/10] Newsfile Signed-off-by: Olivier Wilkinson (reivilibre) --- changelog.d/13823.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/13823.misc diff --git a/changelog.d/13823.misc b/changelog.d/13823.misc new file mode 100644 index 000000000000..527d79f4b225 --- /dev/null +++ b/changelog.d/13823.misc @@ -0,0 +1 @@ +Faster Remote Room Joins: tell remote homeservers that we are unable to authorise them if they query a room which has partial state on our server. \ No newline at end of file From 9ac8b45589e5e6c2a28672cc83cbb2718ec35c2d Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Thu, 22 Sep 2022 14:14:06 +0100 Subject: [PATCH 07/10] flag -> error code --- synapse/handlers/event_auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/event_auth.py b/synapse/handlers/event_auth.py index f74fae77e0ec..576df8e084e9 100644 --- a/synapse/handlers/event_auth.py +++ b/synapse/handlers/event_auth.py @@ -167,7 +167,7 @@ async def assert_host_in_room( Asserts that the host is in the room, or raises an AuthError. If the room is partial-stated, we raise an AuthError with the - UNABLE_DUE_TO_PARTIAL_STATE flag, unless `allow_partial_state_rooms` is true. + UNABLE_DUE_TO_PARTIAL_STATE error code, unless `allow_partial_state_rooms` is true. """ if not allow_partial_state_rooms and await self._store.is_partial_state_room( room_id From 89fdb723fe8bf3ac0c68d0918194bdbe6200f4f4 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Thu, 22 Sep 2022 14:15:51 +0100 Subject: [PATCH 08/10] Add warning about incorrect results in partial stated rooms --- synapse/handlers/event_auth.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/synapse/handlers/event_auth.py b/synapse/handlers/event_auth.py index 576df8e084e9..b123b8cb5a05 100644 --- a/synapse/handlers/event_auth.py +++ b/synapse/handlers/event_auth.py @@ -168,6 +168,10 @@ async def assert_host_in_room( If the room is partial-stated, we raise an AuthError with the UNABLE_DUE_TO_PARTIAL_STATE error code, unless `allow_partial_state_rooms` is true. + + If allow_partial_state_rooms is True and the room is partial-stated, + this function may return an incorrect result as we are not able to fully + track server membership in a room without full state. """ if not allow_partial_state_rooms and await self._store.is_partial_state_room( room_id From 41fcf5c40b6ade35335edb6e4f36d56388b0f2d5 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Thu, 22 Sep 2022 17:48:14 +0100 Subject: [PATCH 09/10] Change error code to MSC3895 --- synapse/api/errors.py | 4 ++-- synapse/config/experimental.py | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/synapse/api/errors.py b/synapse/api/errors.py index 2381aa4e8320..1c6b53aa24f0 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -103,8 +103,8 @@ class Codes(str, Enum): # Returned for federation requests where we can't process a request as we # can't ensure the sending server is in a room which is partial-stated on # our side. - # Part of MSC3706-rei1. - UNABLE_DUE_TO_PARTIAL_STATE = "ORG.MATRIX.MSC3706_UNABLE_DUE_TO_PARTIAL_STATE" + # Part of MSC3895. + UNABLE_DUE_TO_PARTIAL_STATE = "ORG.MATRIX.MSC3895_UNABLE_DUE_TO_PARTIAL_STATE" class CodeMessageException(RuntimeError): diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index bf27f6c10139..595eb007a52c 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -63,7 +63,8 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: # MSC3706 (server-side support for partial state in /send_join responses) self.msc3706_enabled: bool = experimental.get("msc3706_enabled", False) - # experimental support for faster joins over federation (msc2775, msc3706) + # experimental support for faster joins over federation + # (MSC2775, MSC3706, MSC3895) # requires a target server with msc3706_enabled enabled. self.faster_joins_enabled: bool = experimental.get("faster_joins", False) From d613f098a8495d8c4b058f0b875da6c82c9240a6 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Thu, 22 Sep 2022 18:23:04 +0100 Subject: [PATCH 10/10] Remove redundant Measure block --- synapse/handlers/event_auth.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/synapse/handlers/event_auth.py b/synapse/handlers/event_auth.py index b123b8cb5a05..8249ca1ed26c 100644 --- a/synapse/handlers/event_auth.py +++ b/synapse/handlers/event_auth.py @@ -31,7 +31,6 @@ from synapse.events.builder import EventBuilder from synapse.events.snapshot import EventContext from synapse.types import StateMap, get_domain_from_id -from synapse.util.metrics import Measure if TYPE_CHECKING: from synapse.server import HomeServer @@ -157,8 +156,7 @@ async def get_user_which_could_invite( ) async def is_host_in_room(self, room_id: str, host: str) -> bool: - with Measure(self._clock, "check_host_in_room"): - return await self._store.is_host_joined(room_id, host) + return await self._store.is_host_joined(room_id, host) async def assert_host_in_room( self, room_id: str, host: str, allow_partial_state_rooms: bool = False