I'm rather blurry on how CORS works in detail, but it's unclear to me why the REST API should not (at some point) respond with Access-Controll-Allow-Credentials. Because in general, that endpoint does want to use credentials, right?

In any case, note that CSRF tokens are only required with cookie based credentials. When an Authorization header is present, tokens are not required (and iirc not even allowed).

In T261053#6405980, @daniel wrote:

I'm rather blurry on how CORS works in detail, but it's unclear to me why the REST API should not (at some point) respond with Access-Controll-Allow-Credentials. Because in general, that endpoint does want to use credentials, right?

Access-Controll-Allow-Credentials refers to credentials the browser controls (Cookies & HTTP Basic) not credentials it doesn't (OAuth).

If these two conditions are true:

1. Endpoints that have sideffects (writes) will never be simple requests
2. Non-trusted origins may not use Cookies or HTTP Basic Authorization.

Then there is no need for the CSRF tokens even when using Sessions.

As far as I can tell, #1 is always the case because the Content-Type is application/json or the method is something other than GET, HEAD, or POST.

I'm not sure why we would ever allow #2. $wgCrossSiteAJAXdomains specifies a list of domains that are allowed to make cross-origin authenticated (using Session Cookies) requests to MediaWiki and the documentation explicitly states that using * is insecure.

See T232176 for more details.

In any case, note that CSRF tokens are only required with cookie based credentials. When an Authorization header is present, tokens are not required (and iirc not even allowed).

The only reason I bring it up is because it makes an unnecessary step for developers. Developers should be able to use fetch() from the same-origin without needing to first request the CSRF tokens.

This is impossible with the Action API because it fails point #1 due to the Content-Type being application/x-www-form-urlencoded. The browser will send send the POST request (with the credentials attached) without first issuing the preflight request to determine if the credentials can be sent. The REST API, thankfully, does not have the same restrictions.

Looking at posts like https://security.stackexchange.com/questions/170477/csrf-with-json-post-when-content-type-must-be-application-json and https://blog.appsecco.com/exploiting-csrf-on-json-endpoints-with-flash-and-redirects-681d4ad6b31b it seems that a) you are right and b) we shouldn't rely on it.

In any case, OAuth is the recommended authentication method for out REST API. The only reason that it's not required is that core doesn't support out of the box.

Ah, this one is actually very clear on why we shouldn't rely on CORS to prevent CSRF: https://www.nccgroup.com/us/about-us/newsroom-and-events/blog/2017/september/common-csrf-prevention-misconceptions/.

In T261053#6407645, @daniel wrote:

Ah, this one is actually very clear on why we shouldn't rely on CORS to prevent CSRF: https://www.nccgroup.com/us/about-us/newsroom-and-events/blog/2017/september/common-csrf-prevention-misconceptions/.

You could use the same argument to say that we should require CSRF tokens for requests made with OAuth (separating Authorization from CSRF protection). You're always relying on the same-origin policy in order to prevent CSRF attacks, afaik, there isn't another way to do it and that's the point of the policy.

For instance, our code relies on the token endpoint not responding with an Access-Control-Allow-Credentials header. If that were added in the future, someone could read the CSRF token and make a cross-origin write.

Doing nothing already prevents CSRF attacks. We aren't utilizing CORS to do so, the same-origin policy does this for us. I think you could make the opposite argument: You shouldn't rely on a CSRF token to protect you from CSRF attacks. If the token can be read cross origin (via an exploit in Flash or whatever), then the token is irrelevant. The same-origin policy protects this.

I think it would be better to implement code that would reject requests with a Content-Type (on write) that is unsafe (I think we already do?) and would be better for demonstrable security anyways.

Tagging @EvanProdromou for product perspective and the Security-Team for further assessment. I personally only have superficial knowledge in the area, a no strong oppinions other than it's generally better to be safe than sorry.

We're going to discuss this at our tech planning meeting. It'd be best if @dbarratt can attend, so I'll make sure we invite him when this is discussed.

In T261053#6410481, @eprodromou wrote:

We're going to discuss this at our tech planning meeting. It'd be best if @dbarratt can attend, so I'll make sure we invite him when this is discussed.

👍

Change 623130 had a related patch set uploaded (by Dbarratt; owner: Dbarratt):
[mediawiki/core@master] Add protection against CSRF attacks so the token can be removed

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

In T261053#6421041, @gerritbot wrote:

Change 623130 had a related patch set uploaded (by Dbarratt; owner: Dbarratt):
[mediawiki/core@master] Add protection against CSRF attacks so the token can be removed

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

Created the patch as a starting point. It doesn't remove the tokens, just adds protection against CSRF so the tokens can be deprecated.

Untagging Security-Team. Seems to be a good discussion here and the basic reasoning makes sense.

In T261053#6407680, @dbarratt wrote:

You could use the same argument to say that we should require CSRF tokens for requests made with OAuth (separating Authorization from CSRF protection).

OAuth does not rely on the browser to prevent an attack - the attack is prevented by the attacker not having any way to obtain the OAuth token. The proposed scheme relies on a number of browser behaviors (like the inability to make non-preflighted PUT requests) which have been broken in the past. CSRF tokens are more similar in that regard, but they rely on a smaller set of behaviors (not being able to read the content of cross-domain responses without the appropriate response headers), and they are the linchpin of the security framework of the entire web, so browser vendors test them super thoroughly. That is less the case for other cross-domain behavior.

In T261053#6444336, @Tgr wrote:

OAuth does not rely on the browser to prevent an attack - the attack is prevented by the attacker not having any way to obtain the OAuth token.

The OAuth Token gets exposed to the browser when making requests, so it's entirely possible the browser could expose this (accidentally) to other sites making requests (through caching?).

My point is just that we're implementing a security feature on top of an existing security feature that, in my opinion, provides more than adequate protection. That's not inherently bad, but there is a cost in doing so. If we understand the cost and still want to do it, that's totally fine. I created this task to be sensitive to the people who are paying that cost, which is the clients who use the API.

In T261053#6444432, @dbarratt wrote:

The OAuth Token gets exposed to the browser when making requests, so it's entirely possible the browser could expose this (accidentally) to other sites making requests (through caching?).

I don't see how that would happen, barring some unrealistically fundamental browser bug. The same-origin policy is a critical security feature that's tested very thoroughly, and such a bug would break the entire web and would be noticed quickly. Using a security mechanism few other sites are using is always more risky in that regard.

I created this task to be sensitive to the people who are paying that cost, which is the clients who use the API.

Fair, but it doesn't seem like a big cost. MediaWiki JS will presumably provide an abstraction just like it does for the action API tokens, and all other clients will use OAuth anyway as obtaining a session cookie is much more cumbersome and fragile for them.

In T261053#6444540, @Tgr wrote:

Fair, but it doesn't seem like a big cost. MediaWiki JS will presumably provide an abstraction just like it does for the action API tokens, and all other clients will use OAuth anyway as obtaining a session cookie is much more cumbersome and fragile for them.

That's fair. I do think it adds a level of complexity (and latency) that isn't needed, but if it's not worth the risk then we can close this task. :)