Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Rate limiting invites per issuer #13125

Merged
merged 4 commits into from
Jun 30, 2022
Merged

Rate limiting invites per issuer #13125

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b38984a
Rate limiting invites per issuer
Yoric Jun 28, 2022
cb37541
WIP: ChangeLog
Yoric Jun 28, 2022
064300e
WIP: Applied feedback
Yoric Jun 30, 2022
68b0ba2
Update changelog.d/13125.feature
reivilibre Jun 30, 2022
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/13125.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Rate-limiting local invites by issuer.
reivilibre marked this conversation as resolved.
Show resolved Hide resolved
5 changes: 5 additions & 0 deletions synapse/config/ratelimiting.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,11 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
defaults={"per_second": 0.003, "burst_count": 5},
)

self.rc_invites_per_issuer = RateLimitConfig(
Copy link
Member

Choose a reason for hiding this comment

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

@Yoric, @reivilibre: please do remember to add new config settings to the documentation, otherwise people get angry.

Copy link
Member

Choose a reason for hiding this comment

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

see #13330

config.get("rc_invites", {}).get("per_issuer", {}),
defaults={"per_second": 0.003, "burst_count": 5},
)
Copy link
Contributor

Choose a reason for hiding this comment

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

These defaults seem very harsh for someone creating a new room or upgrading a room (if that counts here?).

e.g. if I'm making a group chat with 10 friends, then the first 5 will be invited by the burst count, but the next 5 will take me 1666 seconds to invite — that's almost half an hour!!!

I don't know what's best here; maybe the burst count should be quite a bit higher (obviously this is kinder to spammers, but I think the current number definitely will obstruct normal usage of the service. I've invited more than 5 people to a new room before.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right. Moving to same default rate limits as rc_invites_per_room.


self.rc_third_party_invite = RateLimitConfig(
config.get("rc_third_party_invite", {}),
defaults={
Expand Down
20 changes: 18 additions & 2 deletions synapse/handlers/room_member.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,19 +101,33 @@ def __init__(self, hs: "HomeServer"):
burst_count=hs.config.ratelimiting.rc_joins_remote.burst_count,
)

# Ratelimiter for invites, keyed by room (across all issuers, all
# recipients).
self._invites_per_room_limiter = Ratelimiter(
store=self.store,
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(

# Ratelimiter for invites, keyed by recipient (across all rooms, all
# issuers).
self._invites_per_recipient_limiter = Ratelimiter(
store=self.store,
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,
)

# Ratelimiter for invites, keyed by issuer (across all rooms, all
# recipients).
self._invites_per_issuer_limiter = Ratelimiter(
store=self.store,
clock=self.clock,
rate_hz=hs.config.ratelimiting.rc_invites_per_issuer.per_second,
burst_count=hs.config.ratelimiting.rc_invites_per_issuer.burst_count,
)

self._third_party_invite_limiter = Ratelimiter(
store=self.store,
clock=self.clock,
Expand Down Expand Up @@ -258,7 +272,9 @@ async def ratelimit_invite(
if room_id:
await self._invites_per_room_limiter.ratelimit(requester, room_id)

await self._invites_per_user_limiter.ratelimit(requester, invitee_user_id)
await self._invites_per_recipient_limiter.ratelimit(requester, invitee_user_id)
if requester is not None:
await self._invites_per_issuer_limiter.ratelimit(requester, requester.user)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
await self._invites_per_issuer_limiter.ratelimit(requester, requester.user)
await self._invites_per_issuer_limiter.ratelimit(requester)

No point having a key in the ratelimiter if it's always the same as the requester, I think.


async def _local_membership_update(
self,
Expand Down