Skip to content
This repository was archived by the owner on Apr 26, 2024. It is now read-only.
Merged
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/8008.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add rate limiting to users joining rooms.
12 changes: 12 additions & 0 deletions docs/sample_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -746,6 +746,10 @@ log_config: "CONFDIR/SERVERNAME.log.config"
# - one for ratelimiting redactions by room admins. If this is not explicitly
# set then it uses the same ratelimiting as per rc_message. This is useful
# to allow room admins to deal with abuse quickly.
# - two for ratelimiting number of rooms a user can join, "local" for when
# users are joining rooms the server is already in (this is cheap) vs
# "remote" for when users are trying to join rooms not on the server (which
# can be more expensive)
#
# The defaults are as shown below.
#
Expand All @@ -771,6 +775,14 @@ log_config: "CONFDIR/SERVERNAME.log.config"
#rc_admin_redaction:
# per_second: 1
# burst_count: 50
#
#rc_joins:
# local:
# per_second: 0.1
# burst_count: 3
# remote:
# per_second: 0.01
# burst_count: 3


# Ratelimiting settings for incoming federation
Expand Down
21 changes: 21 additions & 0 deletions synapse/config/ratelimiting.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,15 @@ def read_config(self, config, **kwargs):
if rc_admin_redaction:
self.rc_admin_redaction = RateLimitConfig(rc_admin_redaction)

self.rc_joins_local = RateLimitConfig(
config.get("rc_joins", {}).get("local", {}),
defaults={"per_second": 0.1, "burst_count": 3},
)
self.rc_joins_remote = RateLimitConfig(
config.get("rc_joins", {}).get("remote", {}),
defaults={"per_second": 0.01, "burst_count": 3},
)

def generate_config_section(self, **kwargs):
return """\
## Ratelimiting ##
Expand All @@ -118,6 +127,10 @@ def generate_config_section(self, **kwargs):
# - one for ratelimiting redactions by room admins. If this is not explicitly
# set then it uses the same ratelimiting as per rc_message. This is useful
# to allow room admins to deal with abuse quickly.
# - two for ratelimiting number of rooms a user can join, "local" for when
# users are joining rooms the server is already in (this is cheap) vs
# "remote" for when users are trying to join rooms not on the server (which
# can be more expensive)
#
# The defaults are as shown below.
#
Expand All @@ -143,6 +156,14 @@ def generate_config_section(self, **kwargs):
#rc_admin_redaction:
# per_second: 1
# burst_count: 50
#
#rc_joins:
# local:
# per_second: 0.1
# burst_count: 3
# remote:
# per_second: 0.01
# burst_count: 3
Comment on lines +160 to +166
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't we want to allow faster local joins than remote joins?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And we do! The burst count is the same but the rate is larger for locals

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I read these both as seconds per request instead of requests per second. Ignore me. 😢



# Ratelimiting settings for incoming federation
Expand Down
37 changes: 35 additions & 2 deletions synapse/handlers/room_member.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@

from synapse import types
from synapse.api.constants import MAX_DEPTH, EventTypes, Membership
from synapse.api.errors import AuthError, Codes, SynapseError
from synapse.api.errors import AuthError, Codes, LimitExceededError, SynapseError
from synapse.api.ratelimiting import Ratelimiter
from synapse.api.room_versions import EventFormatVersions
from synapse.crypto.event_signing import compute_event_reference_hash
from synapse.events import EventBase
Expand Down Expand Up @@ -77,6 +78,17 @@ def __init__(self, hs):
if self._is_on_event_persistence_instance:
self.persist_event_storage = hs.get_storage().persistence

self._join_rate_limiter_local = Ratelimiter(
clock=self.clock,
rate_hz=hs.config.ratelimiting.rc_joins_local.per_second,
burst_count=hs.config.ratelimiting.rc_joins_local.burst_count,
)
self._join_rate_limiter_remote = Ratelimiter(
clock=self.clock,
rate_hz=hs.config.ratelimiting.rc_joins_remote.per_second,
burst_count=hs.config.ratelimiting.rc_joins_remote.burst_count,
)

# This is only used to get at ratelimit function, and
# maybe_kick_guest_users. It's fine there are multiple of these as
# it doesn't store state.
Expand Down Expand Up @@ -441,7 +453,28 @@ async def _update_membership(
# so don't really fit into the general auth process.
raise AuthError(403, "Guest access not allowed")

if not is_host_in_room:
if is_host_in_room:
time_now_s = self.clock.time()
allowed, time_allowed = self._join_rate_limiter_local.can_do_action(
requester.user.to_string(),
)

if not allowed:
raise LimitExceededError(
retry_after_ms=int(1000 * (time_allowed - time_now_s))
)

else:
time_now_s = self.clock.time()
allowed, time_allowed = self._join_rate_limiter_remote.can_do_action(
requester.user.to_string(),
)

if not allowed:
raise LimitExceededError(
retry_after_ms=int(1000 * (time_allowed - time_now_s))
)
Comment on lines +456 to +476
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Part of me feels like there must be something here to factor out, but not really sure it would be that easy.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I know right, but I think this is a case where its best not to go down that rabbit hole


inviter = await self._get_inviter(target.to_string(), room_id)
if inviter and not self.hs.is_mine(inviter):
remote_room_hosts.append(inviter.domain)
Expand Down
4 changes: 4 additions & 0 deletions tests/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,10 @@ def default_config(name, parse=False):
"account": {"per_second": 10000, "burst_count": 10000},
"failed_attempts": {"per_second": 10000, "burst_count": 10000},
},
"rc_joins": {
"local": {"per_second": 10000, "burst_count": 10000},
"remote": {"per_second": 10000, "burst_count": 10000},
},
"saml2_enabled": False,
"public_baseurl": None,
"default_identity_server": None,
Expand Down