Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MSC3059: Limits API — Part 3: Federated rate limiting #3059

Closed
wants to merge 13 commits into from
Closed

MSC3059: Limits API — Part 3: Federated rate limiting #3059

wants to merge 13 commits into from

Conversation

erkinalp
Copy link

@erkinalp erkinalp commented Mar 12, 2021

This MSC brings long wanted per-user rate limiting API, with various scoping choices.
Rendered

Signed-off by: Erkin Alp Güney <erkinalp9035@gmail.com>
@erkinalp erkinalp changed the title MSC3059: Federated rate limiting MSC3059: Limits API — Part 3: Federated rate limiting Mar 12, 2021
Signed-off by: Erkin Alp Güney <erkinalp9035@gmail.com>
Signed-off by: Erkin Alp Güney <erkinalp9035@gmail.com>
Signed-off by: Erkin Alp Güney <erkinalp9035@gmail.com>
@turt2live turt2live added client-server Client-Server API kind:feature MSC for not-core and not-maintenance stuff proposal A matrix spec change proposal proposal-in-review labels Mar 12, 2021
Signed-off by: Erkin Alp Güney <erkinalp9035@gmail.com>
Signed-off by: Erkin Alp Güney <erkinalp9035@gmail.com>
Signed-off by: Erkin Alp Güney <erkinalp9035@gmail.com>
Signed-off by: Erkin Alp Güney <erkinalp9035@gmail.com>
Signed off by: Erkin Alp Güney <erkinalp9035@gmail.com>
Signed-off by: Erkin Alp Güney <erkinalp9035@gmail.com>
@turt2live turt2live added the needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. label Jun 8, 2021
Signed-off by: Erkin Alp Güney <erkinalp9035@gmail.com>
Signed-off by: Erkin Alp Güney <erkinalp9035@gmail.com>
Signed-off by: Erkin Alp Güney <erkinalp9035@gmail.com>
Copy link
Member

@KitsuneRal KitsuneRal left a comment

Choose a reason for hiding this comment

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

This describes a pretty elaborate rate limiting logic, without justifying the complexity. Given that rate limiting as described here gets intertwined with state resolution (affecting auth chains, unless I'm mistaking), I'd expect much more careful and detailed observation of why's and how's than this MSC provides as yet. Also, similar to Part 1, it's not clear if the per-user limiting should be added to CS API at all.


Rate limit events in an end-to-end encrpyted room that only cover end-to-end
encrypted events shall also be sent end-to-end encrypted, and otherwise be
rejected by the homeserver as unauthorised.
Copy link
Member

Choose a reason for hiding this comment

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

How would homeservers enforce such rate limits if they can't decrypt those, by definition?

In more detail: The limits shall be enforced by the initiating user's homeserver,
based on the server's own time of receiving the event and checked by other servers
and receiving clients, again based of the same `origin_server_ts` value. An event
is to be rejected immediately if it exceeds the rate limit set for that kind of event.
Copy link
Member

Choose a reason for hiding this comment

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

Prefer active voice over passive - in this particular case, it's not clear if it's just the initiating user's homeserver that rejects the event, or also other parties you mentioned in the previous sentence (and in that latter case you really should leave a note on how that works given that homeservers' wallclocks can, and do, come in ridiculous discrepancy with each other; also in that latter case there's an opportunity to spoof timestamps, thereby evading rate limits).

unless all the limit has entirely been spent by rate limiting events.
If a rate limit is reached by a rate limiting event otherwise, those
actions are to be taken in order, until rate limits are obeyed again:
1. The last rate limit of the same scope sent from same homeserver
Copy link
Member

Choose a reason for hiding this comment

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

"Last" by which order?

encrypted events shall also be sent end-to-end encrypted, and otherwise be
rejected by the homeserver as unauthorised.

## Per-user per-server event rate limiting semantics
Copy link
Member

Choose a reason for hiding this comment

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

For this whole group the same question as for Limits Part 1 applies: how much should this really be unified across homeserver implementations, as long as it's down to homeserver admins to set these anyway? And by the way, how do you check authorisation?

and at most `n` roles from the list, `exclude({n})`, exclude exactly `n` roles
from the list. `all` is a placeholder representing all the roles in the list
and can be used as a parameter in those inclusion or exclusion operators.
The valid combinatoric operators for users are `include` and `exclude`.
Copy link
Member

Choose a reason for hiding this comment

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

Is there a practical need for that? That defines some elaborate machinery - have you seen a need for that in real-world situations?

Copy link
Author

@erkinalp erkinalp Jun 20, 2021

Choose a reason for hiding this comment

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

Microsoft Exchange and on-premises installations of Office 365 (and by extension, Microsoft Teams) have a group policy feature allowing domain managers and team managers to enable/disable various features using policies against users/groups. The proposed scoping mechanism is intended to provide a similar override mechanism on Matrix (though, only for lowering the limits or disabling a feature completely, not for increasing the limits), as a second layer over usual level or role based access control.

Copy link
Member

Choose a reason for hiding this comment

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

Group policies in Active Directory are a huge beast, developed over a couple of decades(!). I really urge you to not try to stuff it in here. Also, "because has it" is a rather poor rationale. Again, you should explain WHY on this MSC's own merits, not something else's.

@erkinalp erkinalp marked this pull request as draft June 20, 2021 14:17
@richvdh richvdh deleted the branch matrix-org:master August 27, 2021 18:24
@richvdh richvdh closed this Aug 27, 2021
@turt2live turt2live added the abandoned A proposal where the author/shepherd is not responsive label Aug 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
abandoned A proposal where the author/shepherd is not responsive client-server Client-Server API kind:feature MSC for not-core and not-maintenance stuff needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. proposal A matrix spec change proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants