Don't ratelimit server-sent delayed events #18018
Conversation
When the server sends a delayed event upon timeout / on-demand sending, don't apply ratelimiting to the event create+send attempt, because it is a send that the server expects to be able to send. What should (and does) get rate-limited are client requests to manage delayed events.
There was a problem hiding this comment.
This change goes against the security considerations of MSC4140. Specifically: https://github.com/matrix-org/matrix-spec-proposals/blob/toger5/expiring-events-keep-alive/proposals/4140-delayed-events-futures.md#rate-limiting-at-the-point-of-sending
To make sure that this doesn't get forgotten I've made a tracking issue and added context there: #18021 (comment)
To be clear: I don't think it is a good idea to be removing this rate limit at this point without a planned action of how the risk would be mitigated in future.
The client request ratelimit is trivially overridden by a malicious homeserver without any code modifications since it is exposed in the Synapse config. Synapse should not be relying on other homeservers to play nice. If the proposed fix is merged it seems like this entire feature will just be a massive new abuse vector. |
If you're referring to non-ratelimited delayed events arriving via federation, I believe that would be protected against by federation-level ratelimiting. Otherwise, there'd already be the possibility of a malicious homeserver disabling all ratelimiting & flooding remote rooms with events. |
I misunderstood this change as affecting the processing of incoming events rather than outgoing. In that case, is this a potential federation DoS issue? It appears a user could currently schedule an unlimited number of delayed events. Is "a limit on the maximum number of outstanding delayed events" going to be implemented in Synapse as suggested by the MSC? |
Yes, this is something I'm also working on. |
|
I'm going to take this off the review queue until we come up with a workable solution -> #18021 Feel free to add it back on afterwards (if you don't have permissions to, poke someone on the team). |
Fixes #18021
When the server sends a delayed event upon timeout / on-demand sending, don't apply ratelimiting to the event create+send attempt, because it is a send that the server expects to be able to send.
What should (and does) get rate-limited are client requests to manage delayed events.
Pull Request Checklist
EventStoretoEventWorkerStore.".code blocks.(run the linters)