Page MenuHomePhabricator

Rate limiting gateway should allow cross-origin requests from web browsers to read the HTTP 429 response
Closed, ResolvedPublic

Description

In order to support better error messages (T418953) and error logging (T418957) in components that make cross-origin requests, such as https://meta.wikimedia.org/wiki/Special:GlobalWatchlist, the rate limiting gateway must allow cross-origin requests from web browsers to read the HTTP 429 response.

In practice, I think we'll need the following, although maybe this plan can be improved:

  • HTTP OPTIONS requests must not be subject to rate limits, in order to allow the preflight request to reach MediaWiki (which will validate the origin and return appropriate response to allow or disallow the request), so that the browser may allow the real request that will return a 429 to go through
    • (I think trying to implement the preflight logic correctly in the gateway would be too much)
  • HTTP GET/POST responses generated by the gateway should include the following headers:
    • Access-Control-Allow-Credentials: true
    • Access-Control-Allow-Origin: the value of the Origin header
      • (while validating the origin is necessary to avoid leaking information to external websites, I think it is sufficient if the preflight request does it, and we can just return it without checking here – this means that allowing the preflight request to reach MediaWiki is critical to validate the origin and avoid information leaks)
    • Access-Control-Expose-Headers: Retry-After

If this is too complex to do, then we should decline this, with the knowledge that we won't have correct error messages and error logging for cross-wiki API requests from web browsers when the rate limits are exceeded.

Related Objects

StatusSubtypeAssignedTask
OpenNone
OpenNone
Resolveddaniel
DeclinedClement_Goubert
Resolveddaniel
Resolveddaniel
Resolveddaniel
Resolvedpmiazga
Resolvedhnowlan
Resolveddaniel
Resolveddaniel
ResolvedClement_Goubert
Resolveddaniel
Resolvedpmiazga
Resolveddaniel
Resolvedpmiazga
Resolveddaniel
Resolvedbrennen
Resolveddaniel
ResolvedClement_Goubert
Resolveddaniel
Resolveddaniel
DeclinedNone
ResolvedClement_Goubert
ResolvedJgiannelos
Resolvedtchin
InvalidNone
Resolveddaniel
Resolveddaniel
Resolveddaniel
ResolvedSLyngshede-WMF
Resolvedtaavi
Resolvedtaavi
ResolvedNone
ResolvedClement_Goubert
ResolvedClement_Goubert
DuplicateClement_Goubert
Resolveddaniel
Resolvedmatmarex
Resolvedmatmarex
Resolvedmatmarex
ResolvedBUG REPORTmatmarex
ResolvedBUG REPORTdaniel
ResolvedBUG REPORTmatmarex
ResolvedBUG REPORTdaniel
Opendaniel
Resolveddaniel
Resolveddaniel
Resolvedmatmarex
Resolvedmatmarex
Resolveddaniel
Resolveddaniel
Resolvedamastilovic
Resolveddaniel
Opendaniel
Resolveddaniel
Opendaniel
Resolvedjijiki
Resolvedaaron
ResolvedBlake
ResolvedBlake
Resolveddaniel
Duplicatedaniel
Resolveddaniel
Resolveddaniel
Resolveddaniel
OpenNone
ResolvedKCVelaga_WMF
OpenNone
ResolvedClement_Goubert

Event Timeline

Preflight is extra tricky, what if the OPTIONS request is throttled but the actual request is not (e.g. because we reach the end of the hour)? All kinds of nasty security issues lie that way.

Another preflight complication is that the preflight response headers from Envoy and MediaWiki need to be in sync, otherwise e.g. a client might set the Api-User-Agent since it knows MediaWiki allows that, but the preflight response won't have it in Access-Control-Allow-Headers and so the browser will abort the request.

(CORS aborts are a massive pain, they don't show up in Chrome's network debugger tab, and IIRC they are not caught by normal JS error handling; you certainly won't get any information on why the request was rejected.)

Since at the Envoy layer we are concerned with blocking scrapers, not with DDOS protection, by far the easiest is to just always let OPTIONS requests through to MediaWiki.

Since at the Envoy layer we are concerned with blocking scrapers, not with DDOS protection, by far the easiest is to just always let OPTIONS requests through to MediaWiki.

Yes, indeed. I'll just bypass rate limiting for OPTIONS requests.

HTTP GET/POST responses with a 429 status should include the following headers if the origin query parameter is present

Why only on 429? Wouldn't they also be needed when Envoy returns a different error, e.g. 404 for an unknown route?

  • Access-Control-Allow-Origin: the value of the origin query parameter

The API sometimes outputs Access-Control-Allow-Origin: * even without a query parameter. I think the simplest thing to do is to repeat the value of the Origin header.

Change #1248461 had a related patch set uploaded (by Daniel Kinzler; author: Daniel Kinzler):

[operations/deployment-charts@master] rest-gateway: add CORS support

https://gerrit.wikimedia.org/r/1248461

HTTP GET/POST responses with a 429 status should include the following headers if the origin query parameter is present

Why only on 429? Wouldn't they also be needed when Envoy returns a different error, e.g. 404 for an unknown route?

Good point, it should probably be on all responses generated by the gateway. I was only thinking of rate limits, but T419034 shows another case.

  • Access-Control-Allow-Origin: the value of the origin query parameter

The API sometimes outputs Access-Control-Allow-Origin: * even without a query parameter. I think the simplest thing to do is to repeat the value of the Origin header.

Yeah, you're right, that's more correct.

Change #1250675 had a related patch set uploaded (by Daniel Kinzler; author: Daniel Kinzler):

[operations/deployment-charts@master] rest gateway: add second Lua filter for header handling

https://gerrit.wikimedia.org/r/1250675

Change #1251289 had a related patch set uploaded (by Daniel Kinzler; author: Daniel Kinzler):

[operations/deployment-charts@master] rest-gateway: do not limit pre-flight requests

https://gerrit.wikimedia.org/r/1251289

Change #1251289 merged by jenkins-bot:

[operations/deployment-charts@master] rest-gateway: do not limit pre-flight requests

https://gerrit.wikimedia.org/r/1251289

sbassett added a project: SecTeam-Processed.
sbassett added subscribers: Catrope, sbassett.

Hey @daniel - @Catrope was going to have a look at some of the related patches from the perspective of the Security-Team.

Hey @daniel - @Catrope was going to have a look at some of the related patches from the perspective of the Security-Team.

Excellent.

One aspect I'd like to check on is this: we are exempting all OPTIONS requests from rate limiting and JWT checks. Is that the right thing to do?

One aspect I'd like to check on is this: we are exempting all OPTIONS requests from rate limiting and JWT checks. Is that the right thing to do?

From everything I'm seeing, the rate-limit exemption is potentially dangerous, depending on what the threat-model for basic DoSes are for the gateway.

One aspect I'd like to check on is this: we are exempting all OPTIONS requests from rate limiting and JWT checks. Is that the right thing to do?

From everything I'm seeing, the rate-limit exemption is potentially dangerous, depending on what the threat-model for basic DoSes are for the gateway.

For basic DoS protection we are generally relying on the edge limits. But we could apply a different high-limits rate limit policy instead of none at all. Do you know if there have been DoS attempts in the past using OPTIONS requests?

Change #1248461 merged by jenkins-bot:

[operations/deployment-charts@master] rest-gateway rate limiting: add CORS headers

https://gerrit.wikimedia.org/r/1248461

For basic DoS protection we are generally relying on the edge limits. But we could apply a different high-limits rate limit policy instead of none at all. Do you know if there have been DoS attempts in the past using OPTIONS requests?

I'm not aware of any specific Wikimedia incidents, but I certainly wouldn't put it outside the realm of possibilities. And as a fan of layered defenses, I would encourage having a separate high-limits rate limit in addition to what happens at the edge. But I believe @Catrope wanted to provide some feedback here as well.

Change #1250675 merged by jenkins-bot:

[operations/deployment-charts@master] rest gateway: add second Lua filter for header handling

https://gerrit.wikimedia.org/r/1250675

Deployed and tested