Skip to content

Commit

Permalink
Ratelimit invites by room and target user (matrix-org#9258)
Browse files Browse the repository at this point in the history
  • Loading branch information
erikjohnston authored Jan 29, 2021
1 parent e19396d commit f2c1560
Show file tree
Hide file tree
Showing 9 changed files with 192 additions and 4 deletions.
1 change: 1 addition & 0 deletions changelog.d/9258.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add ratelimits to invites in rooms and to specific users.
10 changes: 10 additions & 0 deletions docs/sample_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -825,6 +825,8 @@ log_config: "CONFDIR/SERVERNAME.log.config"
# "remote" for when users are trying to join rooms not on the server (which
# can be more expensive)
# - one for ratelimiting how often a user or IP can attempt to validate a 3PID.
# - two for ratelimiting how often invites can be sent in a room or to a
# specific user.
#
# The defaults are as shown below.
#
Expand Down Expand Up @@ -862,6 +864,14 @@ log_config: "CONFDIR/SERVERNAME.log.config"
#rc_3pid_validation:
# per_second: 0.003
# burst_count: 5
#
#rc_invites:
# per_room:
# per_second: 0.3
# burst_count: 10
# per_user:
# per_second: 0.003
# burst_count: 5

# Ratelimiting settings for incoming federation
#
Expand Down
19 changes: 19 additions & 0 deletions synapse/config/ratelimiting.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,15 @@ def read_config(self, config, **kwargs):
defaults={"per_second": 0.003, "burst_count": 5},
)

self.rc_invites_per_room = RateLimitConfig(
config.get("rc_invites", {}).get("per_room", {}),
defaults={"per_second": 0.3, "burst_count": 10},
)
self.rc_invites_per_user = RateLimitConfig(
config.get("rc_invites", {}).get("per_user", {}),
defaults={"per_second": 0.003, "burst_count": 5},
)

def generate_config_section(self, **kwargs):
return """\
## Ratelimiting ##
Expand Down Expand Up @@ -137,6 +146,8 @@ def generate_config_section(self, **kwargs):
# "remote" for when users are trying to join rooms not on the server (which
# can be more expensive)
# - one for ratelimiting how often a user or IP can attempt to validate a 3PID.
# - two for ratelimiting how often invites can be sent in a room or to a
# specific user.
#
# The defaults are as shown below.
#
Expand Down Expand Up @@ -174,6 +185,14 @@ def generate_config_section(self, **kwargs):
#rc_3pid_validation:
# per_second: 0.003
# burst_count: 5
#
#rc_invites:
# per_room:
# per_second: 0.3
# burst_count: 10
# per_user:
# per_second: 0.003
# burst_count: 5
# Ratelimiting settings for incoming federation
#
Expand Down
2 changes: 1 addition & 1 deletion synapse/federation/federation_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -810,7 +810,7 @@ async def _do_send_invite(
"User's homeserver does not support this room version",
Codes.UNSUPPORTED_ROOM_VERSION,
)
elif e.code == 403:
elif e.code in (403, 429):
raise e.to_synapse_error()
else:
raise
Expand Down
4 changes: 4 additions & 0 deletions synapse/handlers/federation.py
Original file line number Diff line number Diff line change
Expand Up @@ -1617,6 +1617,10 @@ async def on_invite_request(
if event.state_key == self._server_notices_mxid:
raise SynapseError(HTTPStatus.FORBIDDEN, "Cannot invite this user")

# We retrieve the room member handler here as to not cause a cyclic dependency
member_handler = self.hs.get_room_member_handler()
member_handler.ratelimit_invite(event.room_id, event.state_key)

# keep a record of the room version, if we don't yet know it.
# (this may get overwritten if we later get a different room version in a
# join dance).
Expand Down
7 changes: 7 additions & 0 deletions synapse/handlers/room.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,10 @@ def __init__(self, hs: "HomeServer"):

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:
Expand Down Expand Up @@ -662,6 +666,9 @@ async def create_room(
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")

await self.event_creation_handler.assert_accepted_privacy_policy(requester)

power_level_content_override = config.get("power_level_content_override")
Expand Down
25 changes: 23 additions & 2 deletions synapse/handlers/room_member.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,17 @@ def __init__(self, hs: "HomeServer"):
burst_count=hs.config.ratelimiting.rc_joins_remote.burst_count,
)

self._invites_per_room_limiter = Ratelimiter(
clock=self.clock,
rate_hz=hs.config.ratelimiting.rc_invites_per_room.per_second,
burst_count=hs.config.ratelimiting.rc_invites_per_room.burst_count,
)
self._invites_per_user_limiter = Ratelimiter(
clock=self.clock,
rate_hz=hs.config.ratelimiting.rc_invites_per_user.per_second,
burst_count=hs.config.ratelimiting.rc_invites_per_user.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 @@ -144,6 +155,12 @@ async def _user_left_room(self, target: UserID, room_id: str) -> None:
"""
raise NotImplementedError()

def ratelimit_invite(self, room_id: str, invitee_user_id: str):
"""Ratelimit invites by room and by target user.
"""
self._invites_per_room_limiter.ratelimit(room_id)
self._invites_per_user_limiter.ratelimit(invitee_user_id)

async def _local_membership_update(
self,
requester: Requester,
Expand Down Expand Up @@ -387,8 +404,12 @@ async def update_membership_locked(
raise SynapseError(403, "This room has been blocked on this server")

if effective_membership_state == Membership.INVITE:
target_id = target.to_string()
if ratelimit:
self.ratelimit_invite(room_id, target_id)

# block any attempts to invite the server notices mxid
if target.to_string() == self._server_notices_mxid:
if target_id == self._server_notices_mxid:
raise SynapseError(HTTPStatus.FORBIDDEN, "Cannot invite this user")

block_invite = False
Expand All @@ -412,7 +433,7 @@ async def update_membership_locked(
block_invite = True

if not await self.spam_checker.user_may_invite(
requester.user.to_string(), target.to_string(), room_id
requester.user.to_string(), target_id, room_id
):
logger.info("Blocking invite due to spam checker")
block_invite = True
Expand Down
93 changes: 92 additions & 1 deletion tests/handlers/test_federation.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
from unittest import TestCase

from synapse.api.constants import EventTypes
from synapse.api.errors import AuthError, Codes, SynapseError
from synapse.api.errors import AuthError, Codes, LimitExceededError, SynapseError
from synapse.api.room_versions import RoomVersions
from synapse.events import EventBase
from synapse.federation.federation_base import event_from_pdu_json
Expand Down Expand Up @@ -191,6 +191,97 @@ def test_rejected_state_event_state(self):

self.assertEqual(sg, sg2)

@unittest.override_config(
{"rc_invites": {"per_room": {"per_second": 0.5, "burst_count": 3}}}
)
def test_invite_by_room_ratelimit(self):
"""Tests that invites from federation in a room are actually rate-limited.
"""
other_server = "otherserver"
other_user = "@otheruser:" + other_server

# create the room
user_id = self.register_user("kermit", "test")
tok = self.login("kermit", "test")
room_id = self.helper.create_room_as(room_creator=user_id, tok=tok)
room_version = self.get_success(self.store.get_room_version(room_id))

def create_invite_for(local_user):
return event_from_pdu_json(
{
"type": EventTypes.Member,
"content": {"membership": "invite"},
"room_id": room_id,
"sender": other_user,
"state_key": local_user,
"depth": 32,
"prev_events": [],
"auth_events": [],
"origin_server_ts": self.clock.time_msec(),
},
room_version,
)

for i in range(3):
self.get_success(
self.handler.on_invite_request(
other_server,
create_invite_for("@user-%d:test" % (i,)),
room_version,
)
)

self.get_failure(
self.handler.on_invite_request(
other_server, create_invite_for("@user-4:test"), room_version,
),
exc=LimitExceededError,
)

@unittest.override_config(
{"rc_invites": {"per_user": {"per_second": 0.5, "burst_count": 3}}}
)
def test_invite_by_user_ratelimit(self):
"""Tests that invites from federation to a particular user are
actually rate-limited.
"""
other_server = "otherserver"
other_user = "@otheruser:" + other_server

# create the room
user_id = self.register_user("kermit", "test")
tok = self.login("kermit", "test")

def create_invite():
room_id = self.helper.create_room_as(room_creator=user_id, tok=tok)
room_version = self.get_success(self.store.get_room_version(room_id))
return event_from_pdu_json(
{
"type": EventTypes.Member,
"content": {"membership": "invite"},
"room_id": room_id,
"sender": other_user,
"state_key": "@user:test",
"depth": 32,
"prev_events": [],
"auth_events": [],
"origin_server_ts": self.clock.time_msec(),
},
room_version,
)

for i in range(3):
event = create_invite()
self.get_success(
self.handler.on_invite_request(other_server, event, event.room_version,)
)

event = create_invite()
self.get_failure(
self.handler.on_invite_request(other_server, event, event.room_version,),
exc=LimitExceededError,
)

def _build_and_send_join_event(self, other_server, other_user, room_id):
join_event = self.get_success(
self.handler.on_make_join_request(other_server, room_id, other_user)
Expand Down
35 changes: 35 additions & 0 deletions tests/rest/client/v1/test_rooms.py
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,41 @@ def test_rooms_members_other_custom_keys(self):
self.assertEquals(json.loads(content), channel.json_body)


class RoomInviteRatelimitTestCase(RoomBase):
user_id = "@sid1:red"

servlets = [
admin.register_servlets,
profile.register_servlets,
room.register_servlets,
]

@unittest.override_config(
{"rc_invites": {"per_room": {"per_second": 0.5, "burst_count": 3}}}
)
def test_invites_by_rooms_ratelimit(self):
"""Tests that invites in a room are actually rate-limited."""
room_id = self.helper.create_room_as(self.user_id)

for i in range(3):
self.helper.invite(room_id, self.user_id, "@user-%s:red" % (i,))

self.helper.invite(room_id, self.user_id, "@user-4:red", expect_code=429)

@unittest.override_config(
{"rc_invites": {"per_user": {"per_second": 0.5, "burst_count": 3}}}
)
def test_invites_by_users_ratelimit(self):
"""Tests that invites to a specific user are actually rate-limited."""

for i in range(3):
room_id = self.helper.create_room_as(self.user_id)
self.helper.invite(room_id, self.user_id, "@other-users:red")

room_id = self.helper.create_room_as(self.user_id)
self.helper.invite(room_id, self.user_id, "@other-users:red", expect_code=429)


class RoomJoinRatelimitTestCase(RoomBase):
user_id = "@sid1:red"

Expand Down

0 comments on commit f2c1560

Please sign in to comment.