Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/19921.feature
Original file line number Diff line number Diff line change
@@ -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).
10 changes: 10 additions & 0 deletions docs/usage/configuration/config_documentation.md
Original file line number Diff line number Diff line change
Expand Up @@ -4292,6 +4292,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. Device-list updates (login/logout, new device, key rotation) are normally sent to every user who shares a room with the changed user, and federated to every server in those rooms. For a very large room this fan-out can be expensive, waking many `/sync` streams and triggering `/keys/query` re-fetch storms. This option is intended for large, unencrypted, auto-joined "directory" rooms used only to make users mutually visible (e.g. in the user directory), where device-list tracking provides no benefit. **This option only takes effect for _unencrypted_ rooms.** Device-list tracking exists solely to serve end-to-end encryption, so excluding an encrypted room would break E2EE (peers would stop being notified of each other's key/device changes, leading to failed decryption and sending to stale devices). Any encrypted room listed here is therefore silently ignored and continues to receive device-list updates as normal. Defaults to `[]`.

Example configuration:
```yaml
exclude_rooms_from_device_list_updates:
- '!bigdirectory:example.com'
```
---
## Opentracing

Configuration options related to Opentracing support.
Expand Down
22 changes: 22 additions & 0 deletions schema/synapse-config.schema.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5314,6 +5314,28 @@ 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. Device-list
updates (login/logout, new device, key rotation) are normally sent to
every user who shares a room with the changed user, and federated to every
server in those rooms. For a very large room this fan-out can be
expensive, waking many `/sync` streams and triggering `/keys/query`
re-fetch storms. This option is intended for large, unencrypted,
auto-joined "directory" rooms used only to make users mutually visible
(e.g. in the user directory), where device-list tracking provides no
benefit. **This option only takes effect for _unencrypted_ rooms.**
Device-list tracking exists solely to serve end-to-end encryption, so
excluding an encrypted room would break E2EE (peers would stop being
notified of each other's key/device changes, leading to failed decryption
and sending to stale devices). Any encrypted room listed here is therefore
silently ignored and continues to receive device-list updates as normal.
items:
type: string
default: []
examples:
- - "!bigdirectory:example.com"
opentracing:
type: object
description: >-
Expand Down
4 changes: 4 additions & 0 deletions synapse/config/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down
99 changes: 90 additions & 9 deletions synapse/handlers/device.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,14 @@ def __init__(self, hs: "HomeServer"):
hs.config.registration.dont_notify_new_devices_for
)

# Rooms configured to be excluded from device-list update fan-out. Note
# that this is only honoured for *unencrypted* rooms: excluding an
# encrypted room would break E2EE, so encrypted rooms in this set are
# silently 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(
Expand Down Expand Up @@ -517,6 +525,47 @@ 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
) -> set[str]:
"""Drop rooms configured to be excluded from device-list updates, but
ONLY if they are not encrypted.

Device-list tracking exists solely to serve end-to-end encryption, so it
is only safe to stop tracking a room if it is unencrypted. Encrypted
rooms in the exclusion set are therefore silently kept to preserve E2EE.

Args:
room_ids: The rooms to filter.

Returns:
The room IDs with excluded (and unencrypted) rooms removed.
"""
excluded = self._rooms_to_exclude_from_device_list_updates
result = set(room_ids)
if not excluded:
# Fast path: nothing configured, so nothing to filter. Avoids any
# per-room encryption lookups.
return result

# Only rooms that are BOTH configured-excluded AND unencrypted may be
# dropped: encrypted rooms must keep flowing device-list updates or E2EE
# breaks. Look the encryption up in a single bulk (cached) call for just
# the candidate rooms rather than per-room, since `get_room_encryption`
# is a `@cachedList` stub that only resolves via `bulk_get_room_encryption`.
candidates = result & excluded
if not candidates:
return result

encryption = await self.store.bulk_get_room_encryption(candidates)
for room_id in candidates:
# Drop only when we positively know the room is unencrypted (`None`).
# An encryption algorithm (encrypted) or the room-unknown sentinel are
# both kept, so we never silently stop tracking an encrypted room.
if encryption.get(room_id) is None:
result.discard(room_id)
return result

@cancellable
async def get_device_changes_in_shared_rooms(
self,
Expand Down Expand Up @@ -550,13 +599,18 @@ async def get_device_changes_in_shared_rooms(
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)
# back on looking for device updates for all users we share a room with.
#
# We derive the tracked users from the `room_ids` we were passed (which
# the caller has already filtered through
# `_filter_device_list_excluded_rooms`) rather than
# `get_users_who_share_room_with_user`, which would recompute the user's
# rooms from scratch and re-widen back to every shared-room peer,
# including those 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)
Expand Down Expand Up @@ -590,6 +644,13 @@ async def get_user_ids_changed(

joined_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) so we don't fan out re-fetches of device
# keys for peers we only share such a room with.
joined_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(
Expand Down Expand Up @@ -744,10 +805,22 @@ 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.
#
# Drop any rooms configured to be excluded from device-list updates
# (unencrypted rooms only) from the recurring shared-rooms computation,
# so this matches `get_user_ids_changed` and the other device-list
# endpoints (notably on the too-old-token fallback in
# `get_device_changes_in_shared_rooms`). We filter narrowly here rather
# than the `joined_room_ids` parameter itself, as the latter is also
# used below to decide which newly-left users to keep tracking, which we
# intentionally leave 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,
)
Expand Down Expand Up @@ -978,6 +1051,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,
Expand Down
157 changes: 156 additions & 1 deletion tests/handlers/test_device.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -499,6 +499,161 @@ 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})
)


class DehydrationTestCase(unittest.HomeserverTestCase):
servlets = [
admin.register_servlets_for_client_rest_resource,
Expand Down
Loading