From d266e3a3a32e7ce8ea59c6364524775d424541c8 Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Tue, 7 Jul 2026 12:03:57 +0200 Subject: [PATCH] Add exclude_rooms_from_device_list_updates (unencrypted rooms only) to curb device-list fan-out --- changelog.d/19934.feature | 1 + .../configuration/config_documentation.md | 10 + schema/synapse-config.schema.yaml | 15 ++ synapse/config/server.py | 4 + synapse/handlers/device.py | 99 +++++++- tests/handlers/test_device.py | 217 +++++++++++++++++- 6 files changed, 333 insertions(+), 13 deletions(-) create mode 100644 changelog.d/19934.feature diff --git a/changelog.d/19934.feature b/changelog.d/19934.feature new file mode 100644 index 00000000000..61ddaeedd45 --- /dev/null +++ b/changelog.d/19934.feature @@ -0,0 +1 @@ +Add an `exclude_rooms_from_device_list_updates` configuration option to curb device-list update fan-out in large unencrypted rooms (encrypted rooms are ignored to preserve end-to-end encryption). diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index d028d65fe33..bf0c05e6216 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -4290,6 +4290,16 @@ exclude_rooms_from_sync: - '!foo:example.com' ``` --- +### `exclude_rooms_from_device_list_updates` + +*(array)* A list of rooms to exclude from device-list update fan-out. A device-list change (login/logout, new device, key rotation) is normally sent to every user sharing a room with the affected user and federated to every server in those rooms, which is expensive in very large rooms. Only *unencrypted* rooms can be excluded: device-list tracking exists to serve end-to-end encryption, so any encrypted room listed here is ignored and keeps receiving updates. Defaults to `[]`. + +Example configuration: +```yaml +exclude_rooms_from_device_list_updates: +- '!foo:example.com' +``` +--- ## Opentracing Configuration options related to Opentracing support. diff --git a/schema/synapse-config.schema.yaml b/schema/synapse-config.schema.yaml index 1a2caddbfda..a7c376c51eb 100644 --- a/schema/synapse-config.schema.yaml +++ b/schema/synapse-config.schema.yaml @@ -5310,6 +5310,21 @@ properties: default: [] examples: - - "!foo:example.com" + exclude_rooms_from_device_list_updates: + type: array + description: >- + A list of rooms to exclude from device-list update fan-out. A device-list + change (login/logout, new device, key rotation) is normally sent to every + user sharing a room with the affected user and federated to every server + in those rooms, which is expensive in very large rooms. Only *unencrypted* + rooms can be excluded: device-list tracking exists to serve end-to-end + encryption, so any encrypted room listed here is ignored and keeps + receiving updates. + items: + type: string + default: [] + examples: + - - "!foo:example.com" opentracing: type: object description: >- diff --git a/synapse/config/server.py b/synapse/config/server.py index ca94c224ea5..37e060c5f1c 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -896,6 +896,10 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: config.get("exclude_rooms_from_sync") or [] ) + self.rooms_to_exclude_from_device_list_updates: list[str] = ( + config.get("exclude_rooms_from_device_list_updates") or [] + ) + delete_stale_devices_after: str | None = ( config.get("delete_stale_devices_after") or None ) diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py index 22254666487..853a01aff7f 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -139,6 +139,13 @@ def __init__(self, hs: "HomeServer"): hs.config.registration.dont_notify_new_devices_for ) + # Rooms excluded from device-list update fan-out. Only honoured for + # *unencrypted* rooms — excluding an encrypted room would break E2EE, so + # encrypted rooms here are kept (see `_filter_device_list_excluded_rooms`). + self._rooms_to_exclude_from_device_list_updates = frozenset( + hs.config.server.rooms_to_exclude_from_device_list_updates + ) + self.device_list_updater = DeviceListWorkerUpdater(hs) self._task_scheduler.register_action( @@ -517,6 +524,42 @@ async def get_device(self, user_id: str, device_id: str) -> JsonDict: return device + async def _filter_device_list_excluded_rooms( + self, room_ids: StrCollection + ) -> frozenset[str]: + """Drop excluded rooms from `room_ids`, but only if they are unencrypted. + + Device-list tracking serves end-to-end encryption, so it is only safe to + stop tracking an unencrypted room; encrypted rooms in the exclusion set + are kept. + + Args: + room_ids: The rooms to filter. + + Returns: + `room_ids` with excluded, unencrypted rooms removed. + """ + excluded = self._rooms_to_exclude_from_device_list_updates + result = set(room_ids) + if not excluded: + # Nothing configured, so nothing to drop. + return frozenset(result) + + # Only rooms that are both excluded and unencrypted may be dropped. Look + # up encryption in one bulk call (`get_room_encryption` is a `@cached` + # stub; `bulk_get_room_encryption` is its real `@cachedList` impl). + candidates = result & excluded + if not candidates: + return frozenset(result) + + encryption = await self.store.bulk_get_room_encryption(candidates) + for room_id in candidates: + # `None` means positively unencrypted, so drop it. An algorithm + # string (encrypted) or the room-unknown sentinel are kept. + if encryption.get(room_id) is None: + result.discard(room_id) + return frozenset(result) + @cancellable async def get_device_changes_in_shared_rooms( self, @@ -549,14 +592,14 @@ async def get_device_changes_in_shared_rooms( changed_users.update(changed) return changed_users - # If the DB returned None then the `from_token` is too old, so we fall - # back on looking for device updates for all users. - - users_who_share_room = await self.store.get_users_who_share_room_with_user( - user_id - ) - - tracked_users = set(users_who_share_room) + # `from_token` is too old, so fall back to all users we share a room + # with. We derive them from the already-filtered `room_ids` rather than + # `get_users_who_share_room_with_user`, which would re-widen to peers in + # excluded rooms. + tracked_users: set[str] = set() + for room_id in room_ids: + user_ids = await self.store.get_users_in_room(room_id) + tracked_users.update(user_ids) # Always tell the user about their own devices tracked_users.add(user_id) @@ -590,6 +633,15 @@ async def get_user_ids_changed( joined_room_ids = await self.store.get_rooms_for_user(user_id) + # Rooms we track device-list updates for: joined rooms minus excluded + # (unencrypted) ones. Used to find *other* users joining/leaving, so we + # don't re-fetch keys for peers we only share an excluded room with. The + # full `joined_room_ids` is kept for the still-share-a-room check in + # `generate_sync_entry_for_device_list`, matching `/sync`. + tracked_room_ids = await self._filter_device_list_excluded_rooms( + joined_room_ids + ) + # Get the set of rooms that the user has joined/left membership_changes = ( await self.store.get_current_state_delta_membership_changes_for_user( @@ -624,7 +676,9 @@ async def get_user_ids_changed( # TODO: Only pull out membership events? state_changes = await self.store.get_current_state_deltas_for_rooms( - joined_room_ids, from_token=from_token.room_key, to_token=now_token.room_key + tracked_room_ids, + from_token=from_token.room_key, + to_token=now_token.room_key, ) for delta in state_changes: if delta.event_type != EventTypes.Member: @@ -744,15 +798,28 @@ async def generate_sync_entry_for_device_list( users_that_have_changed = set() # Step 1a, check for changes in devices of users we share a room - # with + # with. + # + # Exclude the configured (unencrypted) rooms from the shared-rooms + # lookup. We filter here rather than the `joined_room_ids` parameter, + # which is also used below for the still-share-a-room check and left + # unfiltered. + shared_room_ids = await self._filter_device_list_excluded_rooms(joined_room_ids) users_that_have_changed = await self.get_device_changes_in_shared_rooms( user_id, - joined_room_ids, + shared_room_ids, from_token=since_token, now_token=now_token, ) - # Step 1b, check for newly joined rooms + # Step 1b, check for newly joined rooms. + # + # Skip excluded (unencrypted) rooms: otherwise joining one would pull in + # every existing member as "changed" and trigger a mass `/keys/query` + # re-fetch. Encrypted rooms are still expanded, so E2EE is unaffected. + newly_joined_rooms = await self._filter_device_list_excluded_rooms( + newly_joined_rooms + ) for room_id in newly_joined_rooms: joined_users = await self.store.get_users_in_room(room_id) newly_joined_or_invited_or_knocked_users.update(joined_users) @@ -978,6 +1045,14 @@ async def notify_device_update( room_ids = await self.store.get_rooms_for_user(user_id) + # Drop any rooms configured to be excluded from device-list updates + # (unencrypted rooms only). Doing this before recording the change means + # the change is neither recorded against these rooms nor federated to + # their hosts (federation destinations are derived from the recorded + # `device_lists_changes_in_room` rows), and local members who only share + # an excluded room won't be woken. + room_ids = await self._filter_device_list_excluded_rooms(room_ids) + position = await self.store.add_device_change_to_streams( user_id, device_ids, diff --git a/tests/handlers/test_device.py b/tests/handlers/test_device.py index 736f251c277..9407192b57a 100644 --- a/tests/handlers/test_device.py +++ b/tests/handlers/test_device.py @@ -45,7 +45,7 @@ from synapse.federation.units import Transaction from synapse.handlers.device import MAX_DEVICE_DISPLAY_NAME_LEN, DeviceWriterHandler from synapse.rest import admin -from synapse.rest.client import devices, login, register +from synapse.rest.client import devices, login, register, room from synapse.server import HomeServer from synapse.storage.databases.main.appservice import _make_exclusive_regex from synapse.types import ( @@ -499,6 +499,221 @@ def test_delete_device_removes_refresh_tokens(self) -> None: self.assertIsNone(remaining_refresh_token) +class DeviceListExcludedRoomsTestCase(unittest.HomeserverTestCase): + """Tests for `exclude_rooms_from_device_list_updates`. + + A configured room should be dropped from device-list update fan-out, but + ONLY if it is unencrypted: excluding an encrypted room would break E2EE, so + encrypted rooms in the exclusion set must keep being tracked. + """ + + servlets = [ + admin.register_servlets, + login.register_servlets, + register.register_servlets, + room.register_servlets, + ] + + def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: + handler = hs.get_device_handler() + assert isinstance(handler, DeviceWriterHandler) + self.handler = handler + self.store = hs.get_datastores().main + self.event_sources = hs.get_event_sources() + + self.user1 = self.register_user("user1", "pass") + self.user1_tok = self.login(self.user1, "pass", device_id="user1device") + self.user2 = self.register_user("user2", "pass") + self.user2_tok = self.login(self.user2, "pass", device_id="user2device") + + def _create_shared_room(self, encrypted: bool = False) -> str: + """Create a room owned by user1 that user2 also joins.""" + room_id = self.helper.create_room_as(self.user1, tok=self.user1_tok) + if encrypted: + self.helper.send_state( + room_id, + EventTypes.RoomEncryption, + {"algorithm": RoomEncryptionAlgorithms.MEGOLM_V1_AES_SHA2}, + tok=self.user1_tok, + ) + self.helper.join(room_id, self.user2, tok=self.user2_tok) + return room_id + + def _changed_users_for_user2(self) -> set: + """Have user1 change their device, then return the set of users that + user2's device-list stream reports as changed since just before.""" + from_token = self.event_sources.get_current_token() + + self.get_success(self.handler.notify_device_update(self.user1, ["user1device"])) + + result = self.get_success( + self.handler.get_user_ids_changed(self.user2, from_token) + ) + return set(result.changed) + + def test_change_in_shared_room_is_notified(self) -> None: + """Baseline: with no exclusion, a peer sharing a room IS notified.""" + self._create_shared_room() + + self.assertIn(self.user1, self._changed_users_for_user2()) + + def test_excluded_unencrypted_room_is_not_notified(self) -> None: + """A change in an excluded, unencrypted room does NOT notify a peer who + only shares that room.""" + room_id = self._create_shared_room(encrypted=False) + self.handler._rooms_to_exclude_from_device_list_updates = frozenset({room_id}) + + self.assertNotIn(self.user1, self._changed_users_for_user2()) + + def test_non_excluded_room_still_notified(self) -> None: + """A peer sharing a *non-excluded* room IS still notified, even when + another (unshared) room is excluded.""" + room_id = self._create_shared_room(encrypted=False) + self.handler._rooms_to_exclude_from_device_list_updates = frozenset( + {"!someotherroom:test"} + ) + # Sanity check the shared room isn't the excluded one. + self.assertNotEqual(room_id, "!someotherroom:test") + + self.assertIn(self.user1, self._changed_users_for_user2()) + + def test_excluded_but_encrypted_room_is_still_notified(self) -> None: + """If the excluded room is ENCRYPTED, the peer IS still notified: the + guard preserves E2EE.""" + room_id = self._create_shared_room(encrypted=True) + # Ensure the encryption state is visible via the bulk (cached) lookup + # the handler actually uses (`get_room_encryption` is a stub). + self.assertIsNotNone( + self.get_success(self.store.bulk_get_room_encryption({room_id})).get( + room_id + ) + ) + self.handler._rooms_to_exclude_from_device_list_updates = frozenset({room_id}) + + self.assertIn(self.user1, self._changed_users_for_user2()) + + def _changed_via_classic_sync_fallback(self, joined_room_ids: set) -> set: + """Run `generate_sync_entry_for_device_list` (the classic /sync path) + with the too-old-token fallback in `get_device_changes_in_shared_rooms` + forced, and return the reported changed users for user2.""" + from_token = self.event_sources.get_current_token() + # Record a device change for user1 so they appear in the per-user + # `device_lists_stream` that the fallback consults. + self.get_success(self.handler.notify_device_update(self.user1, ["user1device"])) + now_token = self.event_sources.get_current_token() + + # Force `get_device_list_changes_in_rooms` to report the token as too + # old, dropping into the fallback code path. + with patch.object( + self.store, + "get_device_list_changes_in_rooms", + AsyncMock(return_value=None), + ): + result = self.get_success( + self.handler.generate_sync_entry_for_device_list( + user_id=self.user2, + since_token=from_token, + now_token=now_token, + joined_room_ids=joined_room_ids, + newly_joined_rooms=set(), + newly_joined_or_invited_or_knocked_users=set(), + newly_left_rooms=set(), + newly_left_users=set(), + ) + ) + return set(result.changed) + + def test_classic_sync_fallback_excludes_unencrypted_room(self) -> None: + """The classic /sync path (`generate_sync_entry_for_device_list`) passes + an *unfiltered* joined-rooms set, so the filter must be applied inside + that method. On the too-old-token fallback a peer sharing only an + excluded, unencrypted room must NOT be reported as changed.""" + room_id = self._create_shared_room(encrypted=False) + + # Sanity: without exclusion, the fallback DOES surface user1 (otherwise + # the assertion below would pass vacuously). + self.handler._rooms_to_exclude_from_device_list_updates = frozenset() + self.assertIn(self.user1, self._changed_via_classic_sync_fallback({room_id})) + + # With the room excluded (and unencrypted), user1 is filtered out. + self.handler._rooms_to_exclude_from_device_list_updates = frozenset({room_id}) + self.assertNotIn(self.user1, self._changed_via_classic_sync_fallback({room_id})) + + @override_config({"exclude_rooms_from_device_list_updates": ["!excluded:test"]}) + def test_config_populates_handler(self) -> None: + """The config option is wired through to the handler attribute the + filter reads. Guards against a mis-named config key or attribute, which + the other tests (which set the attribute directly) would not catch.""" + self.assertEqual( + self.handler._rooms_to_exclude_from_device_list_updates, + frozenset({"!excluded:test"}), + ) + + def test_newly_joined_excluded_room_does_not_pull_in_members(self) -> None: + """Joining an excluded, unencrypted room must not report its existing + members as changed: otherwise the join alone would trigger a + `/keys/query` re-fetch of every member.""" + # user1 owns the room; user2 has NOT joined it yet. + excluded_room = self.helper.create_room_as(self.user1, tok=self.user1_tok) + self.handler._rooms_to_exclude_from_device_list_updates = frozenset( + {excluded_room} + ) + + from_token = self.event_sources.get_current_token() + self.helper.join(excluded_room, self.user2, tok=self.user2_tok) + changed = set( + self.get_success( + self.handler.get_user_ids_changed(self.user2, from_token) + ).changed + ) + self.assertNotIn(self.user1, changed) + + # Control: joining a *non-excluded* room DOES pull the existing member + # in, so the assertion above is not vacuous. + other_room = self.helper.create_room_as(self.user1, tok=self.user1_tok) + from_token = self.event_sources.get_current_token() + self.helper.join(other_room, self.user2, tok=self.user2_tok) + changed = set( + self.get_success( + self.handler.get_user_ids_changed(self.user2, from_token) + ).changed + ) + self.assertIn(self.user1, changed) + + def test_excluded_room_not_recorded_for_federation(self) -> None: + """A device change in an excluded, unencrypted room is not recorded + against that room in `device_lists_changes_in_room`. Federation pokes + (and local room wake-ups) derive from those rows, so an unrecorded room + is never federated to its servers.""" + room_id = self._create_shared_room(encrypted=False) + + def recorded_changes_in_room() -> set: + from_token = self.event_sources.get_current_token() + self.get_success( + self.handler.notify_device_update(self.user1, ["user1device"]) + ) + now_token = self.event_sources.get_current_token() + return set( + self.get_success( + self.handler.get_device_changes_in_shared_rooms( + self.user2, + [room_id], + from_token=from_token, + now_token=now_token, + ) + ) + ) + + # Control: without exclusion, user1's change IS recorded against the + # room (so it would wake local streams and federate). + self.assertIn(self.user1, recorded_changes_in_room()) + + # With the room excluded (and unencrypted), the change is not recorded + # against it, so nothing fans out to that room's members or servers. + self.handler._rooms_to_exclude_from_device_list_updates = frozenset({room_id}) + self.assertNotIn(self.user1, recorded_changes_in_room()) + + class DehydrationTestCase(unittest.HomeserverTestCase): servlets = [ admin.register_servlets_for_client_rest_resource,