diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index e3c06d5371..2dd03bc44c 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -330,6 +330,18 @@ Example configuration: limit_profile_requests_to_users_who_share_rooms: true ``` --- +### `limit_key_queries_to_users_who_share_rooms` + +Use this option to require a user to share a room with another user in order +to retrieve their published keys. Only checked on Client-Server requests. +Key queries from other servers should be checked by the requesting server. +Defaults to false. + +Example configuration: +```yaml +limit_key_queries_to_users_who_share_rooms: true +``` +--- ### `include_profile_data_on_invite` Use this option to prevent a user's profile data from being retrieved and diff --git a/synapse/config/server.py b/synapse/config/server.py index 6b29983617..9e61f5c1d5 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -416,6 +416,13 @@ class ServerConfig(Config): False, ) + # Whether to require sharing a room with a user to retrieve their + # keys + self.limit_key_queries_to_users_who_share_rooms = config.get( + "limit_key_queries_to_users_who_share_rooms", + False, + ) + # Whether to retrieve and display profile data for a user when they # are invited to a room self.include_profile_data_on_invite = config.get( diff --git a/synapse/handlers/e2e_keys.py b/synapse/handlers/e2e_keys.py index 540995e062..bda85fc86e 100644 --- a/synapse/handlers/e2e_keys.py +++ b/synapse/handlers/e2e_keys.py @@ -158,7 +158,33 @@ class E2eKeysHandler: the number of in-flight queries at a time. """ async with self._query_devices_linearizer.queue((from_user_id, from_device_id)): - device_keys_query: Dict[str, List[str]] = query_body.get("device_keys", {}) + + async def filter_device_key_query( + query: Dict[str, List[str]], + ) -> Dict[str, List[str]]: + if not self.config.server.limit_key_queries_to_users_who_share_rooms: + # Only ignore invalid user IDs, which is the same behaviour as if + # the user existed but had no keys. + return { + user_id: v + for user_id, v in query.items() + if UserID.is_valid(user_id) + } + + # Strip invalid user IDs and user IDs the requesting user does not share rooms with. + result: Dict[str, List[str]] = {} + from_rooms = await self.store.get_rooms_for_user(from_user_id) + for user_id, v in query.items(): + if not UserID.is_valid(user_id): + continue + rooms = await self.store.get_rooms_for_user(user_id) + if not rooms.isdisjoint(from_rooms): + result[user_id] = v + return result + + device_keys_query: Dict[str, List[str]] = await filter_device_key_query( + query_body.get("device_keys", {}) + ) # separate users by domain. # make a map from domain to user_id to device_ids @@ -166,11 +192,6 @@ class E2eKeysHandler: remote_queries = {} for user_id, device_ids in device_keys_query.items(): - if not UserID.is_valid(user_id): - # Ignore invalid user IDs, which is the same behaviour as if - # the user existed but had no keys. - continue - # we use UserID.from_string to catch invalid user ids if self.is_mine(UserID.from_string(user_id)): local_query[user_id] = device_ids diff --git a/tests/handlers/test_e2e_keys.py b/tests/handlers/test_e2e_keys.py index e67efcc17f..f4e453c2ff 100644 --- a/tests/handlers/test_e2e_keys.py +++ b/tests/handlers/test_e2e_keys.py @@ -1896,3 +1896,135 @@ class E2eKeysHandlerTestCase(unittest.HomeserverTestCase): self.assertEqual( remaining_key_ids, {"AAAAAAAAAA", "BAAAAA", "BAAAAB", "BAAAAAAAAA"} ) + + @override_config({"limit_key_queries_to_users_who_share_rooms": True}) + def test_query_devices_remote_restricted_not_in_shared_room(self) -> None: + """Tests that querying keys for a remote user that we don't share a room + with returns nothing. + """ + + remote_user_id = "@test:other" + local_user_id = "@test:test" + + remote_master_key = "85T7JXPFBAySB/jwby4S3lBPTqY3+Zg53nYuGmu1ggY" + remote_self_signing_key = "QeIiFEjluPBtI7WQdG365QKZcFs9kqmHir6RBD0//nQ" + + self.hs.get_federation_client().query_client_keys = mock.AsyncMock( # type: ignore[method-assign] + return_value={ + "device_keys": {remote_user_id: {}}, + "master_keys": { + remote_user_id: { + "user_id": remote_user_id, + "usage": ["master"], + "keys": {"ed25519:" + remote_master_key: remote_master_key}, + }, + }, + "self_signing_keys": { + remote_user_id: { + "user_id": remote_user_id, + "usage": ["self_signing"], + "keys": { + "ed25519:" + + remote_self_signing_key: remote_self_signing_key + }, + } + }, + } + ) + + e2e_handler = self.hs.get_e2e_keys_handler() + + query_result = self.get_success( + e2e_handler.query_devices( + { + "device_keys": {remote_user_id: []}, + }, + timeout=10, + from_user_id=local_user_id, + from_device_id="some_device_id", + ) + ) + + self.assertEqual( + query_result, + { + "device_keys": {}, + "failures": {}, + "master_keys": {}, + "self_signing_keys": {}, + "user_signing_keys": {}, + }, + ) + + @override_config({"limit_key_queries_to_users_who_share_rooms": True}) + def test_query_devices_remote_restricted_in_shared_room(self) -> None: + """Tests that querying keys for a remote user that we share a room + with returns the cross signing keys correctly. + """ + + remote_user_id = "@test:other" + local_user_id = "@test:test" + + # Pretend we're sharing a room with the user we're querying. If not, + # `_query_devices_for_destination` will return early. + self.store.get_rooms_for_user = mock.AsyncMock(return_value={"some_room_id"}) + + remote_master_key = "85T7JXPFBAySB/jwby4S3lBPTqY3+Zg53nYuGmu1ggY" + remote_self_signing_key = "QeIiFEjluPBtI7WQdG365QKZcFs9kqmHir6RBD0//nQ" + + self.hs.get_federation_client().query_user_devices = mock.AsyncMock( # type: ignore[method-assign] + return_value={ + "user_id": remote_user_id, + "stream_id": 1, + "devices": [], + "master_key": { + "user_id": remote_user_id, + "usage": ["master"], + "keys": {"ed25519:" + remote_master_key: remote_master_key}, + }, + "self_signing_key": { + "user_id": remote_user_id, + "usage": ["self_signing"], + "keys": { + "ed25519:" + remote_self_signing_key: remote_self_signing_key + }, + }, + } + ) + + e2e_handler = self.hs.get_e2e_keys_handler() + + query_result = self.get_success( + e2e_handler.query_devices( + { + "device_keys": {remote_user_id: []}, + }, + timeout=10, + from_user_id=local_user_id, + from_device_id="some_device_id", + ) + ) + + self.assertEqual(query_result["failures"], {}) + self.assertEqual( + query_result["master_keys"], + { + remote_user_id: { + "user_id": remote_user_id, + "usage": ["master"], + "keys": {"ed25519:" + remote_master_key: remote_master_key}, + } + }, + ) + self.assertEqual( + query_result["self_signing_keys"], + { + remote_user_id: { + "user_id": remote_user_id, + "usage": ["self_signing"], + "keys": { + "ed25519:" + remote_self_signing_key: remote_self_signing_key + }, + } + }, + )