From c4a55ac4a425f1c98a6f52ffea8b6e00815591be Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 19 Feb 2021 08:19:54 -0500 Subject: Fix style checking due to updated black. --- synapse/handlers/profile.py | 3 +-- tests/handlers/test_user_directory.py | 5 ++++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py index d933dd3f01..dd59392bda 100644 --- a/synapse/handlers/profile.py +++ b/synapse/handlers/profile.py @@ -310,8 +310,7 @@ class ProfileHandler(BaseHandler): await self._update_join_states(requester, target_user) async def on_profile_query(self, args: JsonDict) -> JsonDict: - """Handles federation profile query requests. - """ + """Handles federation profile query requests.""" if not self.hs.config.allow_profile_lookup_over_federation: raise SynapseError( diff --git a/tests/handlers/test_user_directory.py b/tests/handlers/test_user_directory.py index 8dddc12204..98b2f5b383 100644 --- a/tests/handlers/test_user_directory.py +++ b/tests/handlers/test_user_directory.py @@ -619,7 +619,10 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase): [self.assertIn(user, remote_users) for user in received_user_id_ordering[3:]] def _add_user_to_room( - self, room_id: str, room_version: RoomVersion, user_id: str, + self, + room_id: str, + room_version: RoomVersion, + user_id: str, ): # Add a user to the room. builder = self.event_builder_factory.for_room_version( -- cgit 1.5.1 From a1901abd6bb7be4887f439350144a9c8ca548208 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 19 Feb 2021 08:32:21 -0500 Subject: Add documentation and type hints to parse_duration. (#9432) --- changelog.d/9432.misc | 1 + synapse/config/_base.py | 17 +++++++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) create mode 100644 changelog.d/9432.misc diff --git a/changelog.d/9432.misc b/changelog.d/9432.misc new file mode 100644 index 0000000000..1e07da2033 --- /dev/null +++ b/changelog.d/9432.misc @@ -0,0 +1 @@ +Add documentation and type hints to `parse_duration`. diff --git a/synapse/config/_base.py b/synapse/config/_base.py index 97399eb9ba..e89decda34 100644 --- a/synapse/config/_base.py +++ b/synapse/config/_base.py @@ -21,7 +21,7 @@ import os from collections import OrderedDict from hashlib import sha256 from textwrap import dedent -from typing import Any, Iterable, List, MutableMapping, Optional +from typing import Any, Iterable, List, MutableMapping, Optional, Union import attr import jinja2 @@ -147,7 +147,20 @@ class Config: return int(value) * size @staticmethod - def parse_duration(value): + def parse_duration(value: Union[str, int]) -> int: + """Convert a duration as a string or integer to a number of milliseconds. + + If an integer is provided it is treated as milliseconds and is unchanged. + + String durations can have a suffix of 's', 'm', 'h', 'd', 'w', or 'y'. + No suffix is treated as milliseconds. + + Args: + value: The duration to parse. + + Returns: + The number of milliseconds in the duration. + """ if isinstance(value, int): return value second = 1000 -- cgit 1.5.1 From 3a2fe5054fee9566ffc7bd233e7bfc03339eee71 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 19 Feb 2021 15:52:04 +0000 Subject: Add test --- tests/rest/media/v1/test_media_storage.py | 69 +++++++++++++++++++++++++++++-- 1 file changed, 66 insertions(+), 3 deletions(-) diff --git a/tests/rest/media/v1/test_media_storage.py b/tests/rest/media/v1/test_media_storage.py index 0789b12392..36d1e6bc4a 100644 --- a/tests/rest/media/v1/test_media_storage.py +++ b/tests/rest/media/v1/test_media_storage.py @@ -231,9 +231,11 @@ class MediaRepoTests(unittest.HomeserverTestCase): def prepare(self, reactor, clock, hs): - self.media_repo = hs.get_media_repository_resource() - self.download_resource = self.media_repo.children[b"download"] - self.thumbnail_resource = self.media_repo.children[b"thumbnail"] + media_resource = hs.get_media_repository_resource() + self.download_resource = media_resource.children[b"download"] + self.thumbnail_resource = media_resource.children[b"thumbnail"] + self.store = hs.get_datastore() + self.media_repo = hs.get_media_repository() self.media_id = "example.com/12345" @@ -357,6 +359,67 @@ class MediaRepoTests(unittest.HomeserverTestCase): """ self._test_thumbnail("scale", None, False) + def test_thumbnail_repeated_thumbnail(self): + """Test that fetching the same thumbnail works, and deleting the on disk + thumbnail regenerates it. + """ + self._test_thumbnail( + "scale", self.test_image.expected_scaled, self.test_image.expected_found + ) + + if not self.test_image.expected_found: + return + + # Fetching again should work, without re-requesting the image from the + # remote. + params = "?width=32&height=32&method=scale" + channel = make_request( + self.reactor, + FakeSite(self.thumbnail_resource), + "GET", + self.media_id + params, + shorthand=False, + await_result=False, + ) + self.pump() + + self.assertEqual(channel.code, 200) + if self.test_image.expected_scaled: + self.assertEqual( + channel.result["body"], + self.test_image.expected_scaled, + channel.result["body"], + ) + + # Deleting the thumbnail on disk then re-requesting it should work as + # Synapse should regenerate missing thumbnails. + origin, media_id = self.media_id.split("/") + info = self.get_success(self.store.get_cached_remote_media(origin, media_id)) + file_id = info["filesystem_id"] + + thumbnail_dir = self.media_repo.filepaths.remote_media_thumbnail_dir( + origin, file_id + ) + shutil.rmtree(thumbnail_dir, ignore_errors=True) + + channel = make_request( + self.reactor, + FakeSite(self.thumbnail_resource), + "GET", + self.media_id + params, + shorthand=False, + await_result=False, + ) + self.pump() + + self.assertEqual(channel.code, 200) + if self.test_image.expected_scaled: + self.assertEqual( + channel.result["body"], + self.test_image.expected_scaled, + channel.result["body"], + ) + def _test_thumbnail(self, method, expected_body, expected_found): params = "?width=32&height=32&method=" + method channel = make_request( -- cgit 1.5.1 From fc8b3d88097d2c985e8eae9779cdb4b23e1a8ef6 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 19 Feb 2021 13:20:34 -0500 Subject: Ratelimit cross-user key sharing requests. (#8957) --- changelog.d/8957.feature | 1 + synapse/api/constants.py | 7 +++++-- synapse/api/ratelimiting.py | 10 ++++++---- synapse/config/ratelimiting.py | 10 ++++++++++ synapse/federation/federation_server.py | 20 ++++++++++++++++++-- synapse/handlers/devicemessage.py | 24 ++++++++++++++++++++++-- synapse/handlers/events.py | 4 ++-- synapse/handlers/initial_sync.py | 4 ++-- synapse/rest/client/v2_alpha/sendtodevice.py | 4 +--- 9 files changed, 67 insertions(+), 17 deletions(-) create mode 100644 changelog.d/8957.feature diff --git a/changelog.d/8957.feature b/changelog.d/8957.feature new file mode 100644 index 0000000000..fa8961f840 --- /dev/null +++ b/changelog.d/8957.feature @@ -0,0 +1 @@ +Add rate limiters to cross-user key sharing requests. diff --git a/synapse/api/constants.py b/synapse/api/constants.py index af8d59cf87..691f8f9adf 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -98,11 +98,14 @@ class EventTypes: Retention = "m.room.retention" - Presence = "m.presence" - Dummy = "org.matrix.dummy_event" +class EduTypes: + Presence = "m.presence" + RoomKeyRequest = "m.room_key_request" + + class RejectedReason: AUTH_ERROR = "auth_error" diff --git a/synapse/api/ratelimiting.py b/synapse/api/ratelimiting.py index 5d9d5a228f..c3f07bc1a3 100644 --- a/synapse/api/ratelimiting.py +++ b/synapse/api/ratelimiting.py @@ -14,7 +14,7 @@ # limitations under the License. from collections import OrderedDict -from typing import Any, Optional, Tuple +from typing import Hashable, Optional, Tuple from synapse.api.errors import LimitExceededError from synapse.types import Requester @@ -42,7 +42,9 @@ class Ratelimiter: # * How many times an action has occurred since a point in time # * The point in time # * The rate_hz of this particular entry. This can vary per request - self.actions = OrderedDict() # type: OrderedDict[Any, Tuple[float, int, float]] + self.actions = ( + OrderedDict() + ) # type: OrderedDict[Hashable, Tuple[float, int, float]] def can_requester_do_action( self, @@ -82,7 +84,7 @@ class Ratelimiter: def can_do_action( self, - key: Any, + key: Hashable, rate_hz: Optional[float] = None, burst_count: Optional[int] = None, update: bool = True, @@ -175,7 +177,7 @@ class Ratelimiter: def ratelimit( self, - key: Any, + key: Hashable, rate_hz: Optional[float] = None, burst_count: Optional[int] = None, update: bool = True, diff --git a/synapse/config/ratelimiting.py b/synapse/config/ratelimiting.py index def33a60ad..847d25122c 100644 --- a/synapse/config/ratelimiting.py +++ b/synapse/config/ratelimiting.py @@ -102,6 +102,16 @@ class RatelimitConfig(Config): defaults={"per_second": 0.01, "burst_count": 3}, ) + # Ratelimit cross-user key requests: + # * For local requests this is keyed by the sending device. + # * For requests received over federation this is keyed by the origin. + # + # Note that this isn't exposed in the configuration as it is obscure. + self.rc_key_requests = RateLimitConfig( + config.get("rc_key_requests", {}), + defaults={"per_second": 20, "burst_count": 100}, + ) + self.rc_3pid_validation = RateLimitConfig( config.get("rc_3pid_validation") or {}, defaults={"per_second": 0.003, "burst_count": 5}, diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py index 8d4bb621e7..2f832b47f6 100644 --- a/synapse/federation/federation_server.py +++ b/synapse/federation/federation_server.py @@ -34,7 +34,7 @@ from twisted.internet import defer from twisted.internet.abstract import isIPAddress from twisted.python import failure -from synapse.api.constants import EventTypes, Membership +from synapse.api.constants import EduTypes, EventTypes, Membership from synapse.api.errors import ( AuthError, Codes, @@ -44,6 +44,7 @@ from synapse.api.errors import ( SynapseError, UnsupportedRoomVersionError, ) +from synapse.api.ratelimiting import Ratelimiter from synapse.api.room_versions import KNOWN_ROOM_VERSIONS from synapse.events import EventBase from synapse.federation.federation_base import FederationBase, event_from_pdu_json @@ -869,6 +870,13 @@ class FederationHandlerRegistry: # EDU received. self._edu_type_to_instance = {} # type: Dict[str, List[str]] + # A rate limiter for incoming room key requests per origin. + self._room_key_request_rate_limiter = Ratelimiter( + clock=self.clock, + rate_hz=self.config.rc_key_requests.per_second, + burst_count=self.config.rc_key_requests.burst_count, + ) + def register_edu_handler( self, edu_type: str, handler: Callable[[str, JsonDict], Awaitable[None]] ): @@ -917,7 +925,15 @@ class FederationHandlerRegistry: self._edu_type_to_instance[edu_type] = instance_names async def on_edu(self, edu_type: str, origin: str, content: dict): - if not self.config.use_presence and edu_type == "m.presence": + if not self.config.use_presence and edu_type == EduTypes.Presence: + return + + # If the incoming room key requests from a particular origin are over + # the limit, drop them. + if ( + edu_type == EduTypes.RoomKeyRequest + and not self._room_key_request_rate_limiter.can_do_action(origin) + ): return # Check if we have a handler on this instance diff --git a/synapse/handlers/devicemessage.py b/synapse/handlers/devicemessage.py index 1aa7d803b5..7db4f48965 100644 --- a/synapse/handlers/devicemessage.py +++ b/synapse/handlers/devicemessage.py @@ -16,7 +16,9 @@ import logging from typing import TYPE_CHECKING, Any, Dict +from synapse.api.constants import EduTypes from synapse.api.errors import SynapseError +from synapse.api.ratelimiting import Ratelimiter from synapse.logging.context import run_in_background from synapse.logging.opentracing import ( get_active_span_text_map, @@ -25,7 +27,7 @@ from synapse.logging.opentracing import ( start_active_span, ) from synapse.replication.http.devices import ReplicationUserDevicesResyncRestServlet -from synapse.types import JsonDict, UserID, get_domain_from_id +from synapse.types import JsonDict, Requester, UserID, get_domain_from_id from synapse.util import json_encoder from synapse.util.stringutils import random_string @@ -78,6 +80,12 @@ class DeviceMessageHandler: ReplicationUserDevicesResyncRestServlet.make_client(hs) ) + self._ratelimiter = Ratelimiter( + clock=hs.get_clock(), + rate_hz=hs.config.rc_key_requests.per_second, + burst_count=hs.config.rc_key_requests.burst_count, + ) + async def on_direct_to_device_edu(self, origin: str, content: JsonDict) -> None: local_messages = {} sender_user_id = content["sender"] @@ -168,15 +176,27 @@ class DeviceMessageHandler: async def send_device_message( self, - sender_user_id: str, + requester: Requester, message_type: str, messages: Dict[str, Dict[str, JsonDict]], ) -> None: + sender_user_id = requester.user.to_string() + set_tag("number_of_messages", len(messages)) set_tag("sender", sender_user_id) local_messages = {} remote_messages = {} # type: Dict[str, Dict[str, Dict[str, JsonDict]]] for user_id, by_device in messages.items(): + # Ratelimit local cross-user key requests by the sending device. + if ( + message_type == EduTypes.RoomKeyRequest + and user_id != sender_user_id + and self._ratelimiter.can_do_action( + (sender_user_id, requester.device_id) + ) + ): + continue + # we use UserID.from_string to catch invalid user ids if self.is_mine(UserID.from_string(user_id)): messages_by_device = { diff --git a/synapse/handlers/events.py b/synapse/handlers/events.py index 3e23f82cf7..f46cab7325 100644 --- a/synapse/handlers/events.py +++ b/synapse/handlers/events.py @@ -17,7 +17,7 @@ import logging import random from typing import TYPE_CHECKING, Iterable, List, Optional -from synapse.api.constants import EventTypes, Membership +from synapse.api.constants import EduTypes, EventTypes, Membership from synapse.api.errors import AuthError, SynapseError from synapse.events import EventBase from synapse.handlers.presence import format_user_presence_state @@ -113,7 +113,7 @@ class EventStreamHandler(BaseHandler): states = await presence_handler.get_states(users) to_add.extend( { - "type": EventTypes.Presence, + "type": EduTypes.Presence, "content": format_user_presence_state(state, time_now), } for state in states diff --git a/synapse/handlers/initial_sync.py b/synapse/handlers/initial_sync.py index 78c3e5a10b..71a5076672 100644 --- a/synapse/handlers/initial_sync.py +++ b/synapse/handlers/initial_sync.py @@ -18,7 +18,7 @@ from typing import TYPE_CHECKING, Optional, Tuple from twisted.internet import defer -from synapse.api.constants import EventTypes, Membership +from synapse.api.constants import EduTypes, EventTypes, Membership from synapse.api.errors import SynapseError from synapse.events.validator import EventValidator from synapse.handlers.presence import format_user_presence_state @@ -412,7 +412,7 @@ class InitialSyncHandler(BaseHandler): return [ { - "type": EventTypes.Presence, + "type": EduTypes.Presence, "content": format_user_presence_state(s, time_now), } for s in states diff --git a/synapse/rest/client/v2_alpha/sendtodevice.py b/synapse/rest/client/v2_alpha/sendtodevice.py index a3dee14ed4..79c1b526ee 100644 --- a/synapse/rest/client/v2_alpha/sendtodevice.py +++ b/synapse/rest/client/v2_alpha/sendtodevice.py @@ -56,10 +56,8 @@ class SendToDeviceRestServlet(servlet.RestServlet): content = parse_json_object_from_request(request) assert_params_in_dict(content, ("messages",)) - sender_user_id = requester.user.to_string() - await self.device_message_handler.send_device_message( - sender_user_id, message_type, content["messages"] + requester, message_type, content["messages"] ) response = (200, {}) # type: Tuple[int, dict] -- cgit 1.5.1 From e22b71810ee60a24af39f95f35967e2156adc829 Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Mon, 22 Feb 2021 11:44:31 +0000 Subject: Clean up the user directory sample config section (#9385) The user directory sample config section was a little messy, and didn't adhere to our [recommended config format guidelines](https://github.com/matrix-org/synapse/blob/develop/docs/code_style.md#configuration-file-format). This PR cleans that up a bit. --- changelog.d/9385.feature | 1 + docs/sample_config.yaml | 47 ++++++++++++++++----------- synapse/config/user_directory.py | 69 +++++++++++++++++++++------------------- 3 files changed, 67 insertions(+), 50 deletions(-) create mode 100644 changelog.d/9385.feature diff --git a/changelog.d/9385.feature b/changelog.d/9385.feature new file mode 100644 index 0000000000..cbe3922de8 --- /dev/null +++ b/changelog.d/9385.feature @@ -0,0 +1 @@ + Add a configuration option, `user_directory.prefer_local_users`, which when enabled will make it more likely for users on the same server as you to appear above other users. \ No newline at end of file diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index c660c62620..4dbef41b7e 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -2544,24 +2544,35 @@ spam_checker: # User Directory configuration # -# 'enabled' defines whether users can search the user directory. If -# false then empty responses are returned to all queries. Defaults to -# true. -# -# 'search_all_users' defines whether to search all users visible to your HS -# when searching the user directory, rather than limiting to users visible -# in public rooms. Defaults to false. If you set it True, you'll have to -# rebuild the user_directory search indexes, see -# https://github.com/matrix-org/synapse/blob/master/docs/user_directory.md -# -# 'prefer_local_users' defines whether to prioritise local users in -# search query results. If True, local users are more likely to appear above -# remote users when searching the user directory. Defaults to false. -# -#user_directory: -# enabled: true -# search_all_users: false -# prefer_local_users: false +user_directory: + # Defines whether users can search the user directory. If false then + # empty responses are returned to all queries. Defaults to true. + # + # Uncomment to disable the user directory. + # + #enabled: false + + # Defines whether to search all users visible to your HS when searching + # the user directory, rather than limiting to users visible in public + # rooms. Defaults to false. + # + # If you set it true, you'll have to rebuild the user_directory search + # indexes, see: + # https://github.com/matrix-org/synapse/blob/master/docs/user_directory.md + # + # Uncomment to return search results containing all known users, even if that + # user does not share a room with the requester. + # + #search_all_users: true + + # Defines whether to prefer local users in search query results. + # If True, local users are more likely to appear above remote users + # when searching the user directory. Defaults to false. + # + # Uncomment to prefer local over remote users in user directory search + # results. + # + #prefer_local_users: true # User Consent configuration diff --git a/synapse/config/user_directory.py b/synapse/config/user_directory.py index 89dbebd148..8d05ef173c 100644 --- a/synapse/config/user_directory.py +++ b/synapse/config/user_directory.py @@ -24,41 +24,46 @@ class UserDirectoryConfig(Config): section = "userdirectory" def read_config(self, config, **kwargs): - self.user_directory_search_enabled = True - self.user_directory_search_all_users = False - self.user_directory_search_prefer_local_users = False - user_directory_config = config.get("user_directory", None) - if user_directory_config: - self.user_directory_search_enabled = user_directory_config.get( - "enabled", True - ) - self.user_directory_search_all_users = user_directory_config.get( - "search_all_users", False - ) - self.user_directory_search_prefer_local_users = user_directory_config.get( - "prefer_local_users", False - ) + user_directory_config = config.get("user_directory") or {} + self.user_directory_search_enabled = user_directory_config.get("enabled", True) + self.user_directory_search_all_users = user_directory_config.get( + "search_all_users", False + ) + self.user_directory_search_prefer_local_users = user_directory_config.get( + "prefer_local_users", False + ) def generate_config_section(self, config_dir_path, server_name, **kwargs): return """ # User Directory configuration # - # 'enabled' defines whether users can search the user directory. If - # false then empty responses are returned to all queries. Defaults to - # true. - # - # 'search_all_users' defines whether to search all users visible to your HS - # when searching the user directory, rather than limiting to users visible - # in public rooms. Defaults to false. If you set it True, you'll have to - # rebuild the user_directory search indexes, see - # https://github.com/matrix-org/synapse/blob/master/docs/user_directory.md - # - # 'prefer_local_users' defines whether to prioritise local users in - # search query results. If True, local users are more likely to appear above - # remote users when searching the user directory. Defaults to false. - # - #user_directory: - # enabled: true - # search_all_users: false - # prefer_local_users: false + user_directory: + # Defines whether users can search the user directory. If false then + # empty responses are returned to all queries. Defaults to true. + # + # Uncomment to disable the user directory. + # + #enabled: false + + # Defines whether to search all users visible to your HS when searching + # the user directory, rather than limiting to users visible in public + # rooms. Defaults to false. + # + # If you set it true, you'll have to rebuild the user_directory search + # indexes, see: + # https://github.com/matrix-org/synapse/blob/master/docs/user_directory.md + # + # Uncomment to return search results containing all known users, even if that + # user does not share a room with the requester. + # + #search_all_users: true + + # Defines whether to prefer local users in search query results. + # If True, local users are more likely to appear above remote users + # when searching the user directory. Defaults to false. + # + # Uncomment to prefer local over remote users in user directory search + # results. + # + #prefer_local_users: true """ -- cgit 1.5.1