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

Implement per-room rate-limiting for remote joins #12710

Closed
jimmackenzie opened this issue May 11, 2022 · 6 comments · Fixed by #13276
Closed

Implement per-room rate-limiting for remote joins #12710

jimmackenzie opened this issue May 11, 2022 · 6 comments · Fixed by #13276
Assignees
Labels
A-Federated-Join joins over federation generally suck T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.

Comments

@jimmackenzie
Copy link

jimmackenzie commented May 11, 2022

Description:

Synapse includes a few different levers for controlling the flow of actions. We should add a new lever, and limit the number of joins to a room in a given time, including remote joins. I.e. if there has recently been lots of joins in a room (local or remote joins), then servers should ratelimit both local joins and remote attempts to join via /make_join.

By using the existing rate-limit infrastructure, we can tackle some of the issues set out in #12578 around disruptive mass-join events. Rate-limiting helps to prevent performance concerns from handling too many join events at once, and also gives time and space for room moderators to put in place further protections.

Ideally, this limit could be adjusted on a room-by-room basis. Large public rooms would be able to adjust their individual rate-limits, while the default would protect the majority of rooms

@DMRobertson DMRobertson added the T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. label May 11, 2022
@erikjohnston
Copy link
Member

Ideally, this limit could be adjusted on a room-by-room basis. Large public rooms would be able to adjust their individual rate-limits, while the default would protect the majority of rooms

This would probably need to be specced. Though I think even having quite a liberal rate limit would help against any join spam issues

@DMRobertson
Copy link
Contributor

DMRobertson commented Jun 1, 2022

What is the proposal here? A per-room rate limit on /make_join, /make_knock, /send_join and /send_knock?

Edit: I ask because Erik talks of spec work---is there a suggestion that this should be enforced across federation?

@MadLittleMods MadLittleMods added the A-Federated-Join joins over federation generally suck label Jun 3, 2022
@DMRobertson
Copy link
Contributor

  • track the rate of joins (local, remote) across all servers that we see on a room-by-room basis
  • on a request to join/knock, 429 if the threshold for this room has been exceeded
  • knocks are similar, but let's just do joins as a first step for now.
    • ideally rate limit knocks too with a separate knob in the future.
  • don't ignore incoming remote joins if they would exceed the threshold of join rate
  • rate limiter doesn't allow you to update the counters without requesting an action. Change needed there.
  • threshold and this behaviour is limited to one server only. on by default with a sensible default value, but a knob to configure it in an emergency. single large global limit for now.

@DMRobertson
Copy link
Contributor

  • track the rate of joins (local, remote) across all servers that we see on a room-by-room basis

I think this is hard to do across multiple workers. Maybe we can use redis for this? Idk how much effort it's worth putting into this though(?)

@DMRobertson
Copy link
Contributor

@DMRobertson
Copy link
Contributor

Ideas:

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Federated-Join joins over federation generally suck T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.
Projects
None yet
4 participants