summary refs log tree commit diff
diff options
context:
space:
mode:
authorAndrew Morgan <andrew@amorgan.xyz>2021-04-23 14:12:47 +0100
committerAndrew Morgan <andrew@amorgan.xyz>2021-04-23 14:12:47 +0100
commitd2b3c47ba31fec1df4d06f1c2aa7ed29e309498d (patch)
tree7a80e170fdec14dfd3e78cd4984ff39c89beb48a
parentMerge commit 'd9f1dccba' into anoa/dinsic_release_1_31_0 (diff)
parentClean up the user directory sample config section (#9385) (diff)
downloadsynapse-d2b3c47ba31fec1df4d06f1c2aa7ed29e309498d.tar.xz
Merge commit 'e22b71810' into anoa/dinsic_release_1_31_0
-rw-r--r--changelog.d/8957.feature1
-rw-r--r--changelog.d/9385.feature1
-rw-r--r--changelog.d/9432.misc1
-rw-r--r--docs/sample_config.yaml58
-rw-r--r--synapse/api/constants.py7
-rw-r--r--synapse/api/ratelimiting.py10
-rw-r--r--synapse/config/_base.py17
-rw-r--r--synapse/config/ratelimiting.py10
-rw-r--r--synapse/config/user_directory.py87
-rw-r--r--synapse/federation/federation_server.py20
-rw-r--r--synapse/handlers/devicemessage.py24
-rw-r--r--synapse/handlers/events.py4
-rw-r--r--synapse/handlers/initial_sync.py4
-rw-r--r--synapse/rest/client/v2_alpha/sendtodevice.py4
-rw-r--r--tests/rest/media/v1/test_media_storage.py69
15 files changed, 231 insertions, 86 deletions
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/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/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/docs/sample_config.yaml b/docs/sample_config.yaml
index 454fa74e8f..1d89343485 100644
--- a/docs/sample_config.yaml
+++ b/docs/sample_config.yaml
@@ -2724,29 +2724,41 @@ 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
-#
-#  # If this is set, user search will be delegated to this ID server instead
-#  # of synapse performing the search itself.
-#  # This is an experimental API.
-#  defer_to_id_server: https://id.example.com
+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
+
+    # If this is set, user search will be delegated to this ID server instead
+    # of Synapse performing the search itself.
+    # This is an experimental API.
+    #
+    #defer_to_id_server: https://id.example.com
+
+    # 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/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/_base.py b/synapse/config/_base.py
index 40af1979c4..57f454fc9f 100644
--- a/synapse/config/_base.py
+++ b/synapse/config/_base.py
@@ -22,7 +22,7 @@ from collections import OrderedDict
 from hashlib import sha256
 from io import open as io_open
 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
@@ -148,7 +148,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
diff --git a/synapse/config/ratelimiting.py b/synapse/config/ratelimiting.py
index 070eb1b761..944051422e 100644
--- a/synapse/config/ratelimiting.py
+++ b/synapse/config/ratelimiting.py
@@ -105,6 +105,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/config/user_directory.py b/synapse/config/user_directory.py
index 306e0cc8a4..ab7770e472 100644
--- a/synapse/config/user_directory.py
+++ b/synapse/config/user_directory.py
@@ -24,50 +24,55 @@ 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_defer_to_id_server = None
-        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_defer_to_id_server = user_directory_config.get(
-                "defer_to_id_server", None
-            )
-            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_defer_to_id_server = user_directory_config.get(
+            "defer_to_id_server", None
+        )
+        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
-        #
-        #  # If this is set, user search will be delegated to this ID server instead
-        #  # of synapse performing the search itself.
-        #  # This is an experimental API.
-        #  defer_to_id_server: https://id.example.com
+        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
+
+            # If this is set, user search will be delegated to this ID server instead
+            # of Synapse performing the search itself.
+            # This is an experimental API.
+            #
+            #defer_to_id_server: https://id.example.com
+
+            # 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
         """
diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py
index e84fad9d77..de7c2e5f77 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.config.api import DEFAULT_ROOM_STATE_TYPES
 from synapse.events import EventBase
@@ -945,6 +946,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]]
     ):
@@ -993,7 +1001,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]
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(