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

Rate limit key share requests #8677

Closed
erikjohnston opened this issue Oct 28, 2020 · 1 comment · Fixed by #8957
Closed

Rate limit key share requests #8677

erikjohnston opened this issue Oct 28, 2020 · 1 comment · Fixed by #8957
Assignees
Labels
A-Performance Performance, both client-facing and admin-facing

Comments

@erikjohnston
Copy link
Member

There is a bug in one of the clients that causes a lot of cross user key share requests, which then stacks up in the device_inbox for devices leading to issues on clients (potentially, maybe). These should be both relatively rare and safe to drop, so we should rate limit them.

We may also want to consider dropping key share requests to devices that already have a large (ish) number of device messages in their inbox.

We should back out #8675 after this is done.

@anoadragon453 anoadragon453 added the A-Performance Performance, both client-facing and admin-facing label Oct 28, 2020
@clokep
Copy link
Member

clokep commented Oct 30, 2020

I'm guessing the work is something like:

  1. Add a new rate limiter for incoming federation EDUs.
  2. Replace the patch in federation_server.py from Patch to temporarily drop cross-user m.key_share_requests #8675 with a check for the rate-limiter. If over the limit, drop the request.
  3. Replace the patch in devicemessage.py from Patch to temporarily drop cross-user m.key_share_requests #8675 with a check for the rate-limiter, raising a 429 if over the limit?
  4. Add tests.

Does something like that sound about right?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Performance Performance, both client-facing and admin-facing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants