diff options
author | Brendan Abolivier <babolivier@matrix.org> | 2021-05-12 16:05:28 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-05-12 16:05:28 +0200 |
commit | a683028d81606708f686b890c0a44f5a20b54798 (patch) | |
tree | 62d52062df2ebf4617b5297f1315f1bf6314d8ee /synapse | |
parent | Change the format of access tokens away from macaroons (#5588) (diff) | |
download | synapse-a683028d81606708f686b890c0a44f5a20b54798.tar.xz |
Correctly ratelimit invites when creating a room (#9968)
* Correctly ratelimit invites when creating a room Also allow ratelimiting for more than one action at a time.
Diffstat (limited to 'synapse')
-rw-r--r-- | synapse/api/ratelimiting.py | 22 | ||||
-rw-r--r-- | synapse/handlers/room.py | 27 | ||||
-rw-r--r-- | synapse/handlers/room_member.py | 25 |
3 files changed, 62 insertions, 12 deletions
diff --git a/synapse/api/ratelimiting.py b/synapse/api/ratelimiting.py index 2244b8a340..b9a10283f4 100644 --- a/synapse/api/ratelimiting.py +++ b/synapse/api/ratelimiting.py @@ -57,6 +57,7 @@ class Ratelimiter: rate_hz: Optional[float] = None, burst_count: Optional[int] = None, update: bool = True, + n_actions: int = 1, _time_now_s: Optional[int] = None, ) -> Tuple[bool, float]: """Can the entity (e.g. user or IP address) perform the action? @@ -76,6 +77,9 @@ class Ratelimiter: burst_count: How many actions that can be performed before being limited. Overrides the value set during instantiation if set. update: Whether to count this check as performing the action + n_actions: The number of times the user wants to do this action. If the user + cannot do all of the actions, the user's action count is not incremented + at all. _time_now_s: The current time. Optional, defaults to the current time according to self.clock. Only used by tests. @@ -124,17 +128,20 @@ class Ratelimiter: time_delta = time_now_s - time_start performed_count = action_count - time_delta * rate_hz if performed_count < 0: - # Allow, reset back to count 1 - allowed = True + performed_count = 0 time_start = time_now_s - action_count = 1.0 - elif performed_count > burst_count - 1.0: + + # This check would be easier read as performed_count + n_actions > burst_count, + # but performed_count might be a very precise float (with lots of numbers + # following the point) in which case Python might round it up when adding it to + # n_actions. Writing it this way ensures it doesn't happen. + if performed_count > burst_count - n_actions: # Deny, we have exceeded our burst count allowed = False else: # We haven't reached our limit yet allowed = True - action_count += 1.0 + action_count = performed_count + n_actions if update: self.actions[key] = (action_count, time_start, rate_hz) @@ -182,6 +189,7 @@ class Ratelimiter: rate_hz: Optional[float] = None, burst_count: Optional[int] = None, update: bool = True, + n_actions: int = 1, _time_now_s: Optional[int] = None, ): """Checks if an action can be performed. If not, raises a LimitExceededError @@ -201,6 +209,9 @@ class Ratelimiter: burst_count: How many actions that can be performed before being limited. Overrides the value set during instantiation if set. update: Whether to count this check as performing the action + n_actions: The number of times the user wants to do this action. If the user + cannot do all of the actions, the user's action count is not incremented + at all. _time_now_s: The current time. Optional, defaults to the current time according to self.clock. Only used by tests. @@ -216,6 +227,7 @@ class Ratelimiter: rate_hz=rate_hz, burst_count=burst_count, update=update, + n_actions=n_actions, _time_now_s=time_now_s, ) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index fb4823a5cc..835d874cee 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -32,7 +32,14 @@ from synapse.api.constants import ( RoomCreationPreset, RoomEncryptionAlgorithms, ) -from synapse.api.errors import AuthError, Codes, NotFoundError, StoreError, SynapseError +from synapse.api.errors import ( + AuthError, + Codes, + LimitExceededError, + NotFoundError, + StoreError, + SynapseError, +) from synapse.api.filtering import Filter from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, RoomVersion from synapse.events import EventBase @@ -126,10 +133,6 @@ class RoomCreationHandler(BaseHandler): self.third_party_event_rules = hs.get_third_party_event_rules() - self._invite_burst_count = ( - hs.config.ratelimiting.rc_invites_per_room.burst_count - ) - async def upgrade_room( self, requester: Requester, old_room_id: str, new_version: RoomVersion ) -> str: @@ -676,8 +679,18 @@ class RoomCreationHandler(BaseHandler): invite_3pid_list = [] invite_list = [] - if len(invite_list) + len(invite_3pid_list) > self._invite_burst_count: - raise SynapseError(400, "Cannot invite so many users at once") + if invite_list or invite_3pid_list: + try: + # If there are invites in the request, see if the ratelimiting settings + # allow that number of invites to be sent from the current user. + await self.room_member_handler.ratelimit_multiple_invites( + requester, + room_id=None, + n_invites=len(invite_list) + len(invite_3pid_list), + update=False, + ) + except LimitExceededError: + raise SynapseError(400, "Cannot invite so many users at once") await self.event_creation_handler.assert_accepted_privacy_policy(requester) diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index 20700fc5a8..9a092da715 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -163,6 +163,31 @@ class RoomMemberHandler(metaclass=abc.ABCMeta): async def forget(self, user: UserID, room_id: str) -> None: raise NotImplementedError() + async def ratelimit_multiple_invites( + self, + requester: Optional[Requester], + room_id: Optional[str], + n_invites: int, + update: bool = True, + ): + """Ratelimit more than one invite sent by the given requester in the given room. + + Args: + requester: The requester sending the invites. + room_id: The room the invites are being sent in. + n_invites: The amount of invites to ratelimit for. + update: Whether to update the ratelimiter's cache. + + Raises: + LimitExceededError: The requester can't send that many invites in the room. + """ + await self._invites_per_room_limiter.ratelimit( + requester, + room_id, + update=update, + n_actions=n_invites, + ) + async def ratelimit_invite( self, requester: Optional[Requester], |