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

Application Services are rate-limited by IP address on /login #8846

Closed
anoadragon453 opened this issue Nov 30, 2020 · 1 comment · Fixed by #8920
Closed

Application Services are rate-limited by IP address on /login #8846

anoadragon453 opened this issue Nov 30, 2020 · 1 comment · Fixed by #8920
Assignees
Labels
A-Application-Service Related to AS support z-bug (Deprecated Label) Z-Help-Wanted We know exactly how to fix this issue, and would be grateful for any contribution

Comments

@anoadragon453
Copy link
Member

Since MSC2778's implementation application services are able to use /login to login as an interested user and create a device which aids them in performing end-to-end encryption.

However, /login has a ratelimit on it which is IP-based:

async def on_POST(self, request: SynapseRequest):
self._address_ratelimiter.ratelimit(request.getClientIP())
login_submission = parse_json_object_from_request(request)
try:
if login_submission["type"] == LoginRestServlet.APPSERVICE_TYPE:
appservice = self.auth.get_appservice_by_req(request)
result = await self._do_appservice_login(login_submission, appservice)
elif self.jwt_enabled and (

This is obviously quite bad news for application services that need to login to a lot of user accounts on startup or somesuch. Currently there is no way to override this ratelimit.

As a solution, I propose only calling upon the ratelimiter for non uk.half-shot.msc2778.login.application_service identifier types (aka non-AS requests). If the request cannot be authenticated via an access token, it will immediately be rejected.

We may also want to move this line:

login_submission = parse_json_object_from_request(request)

as someone could technically supply a huge JSON body over and over and upset the server that way.

@anoadragon453 anoadragon453 added z-bug (Deprecated Label) Z-Help-Wanted We know exactly how to fix this issue, and would be grateful for any contribution p1 labels Nov 30, 2020
@anoadragon453
Copy link
Member Author

@Half-Shot helpfully notes that application services have a rate limit option in their registration file, which we should probably respect here.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Application-Service Related to AS support z-bug (Deprecated Label) Z-Help-Wanted We know exactly how to fix this issue, and would be grateful for any contribution
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants