1483815 - Implement additional CORS restrictions

Status: RESOLVED FIXED

(Core :: DOM: Networking, enhancement, P1)

Description

[Anne (:annevk)]

Change to the standard: https://github.com/whatwg/fetch/pull/736.

Tests: https://github.com/web-platform-tests/wpt/pull/11432.

Comment 1

[Anne (:annevk)]

https://github.com/whatwg/fetch/pull/829 and https://github.com/web-platform-tests/wpt/pull/13921 complement this further. Both Chrome and Safari have already done some work here and are therefore better at protecting servers. It'd be good to align.

Comment 2

[Selena Deckelmann :selenamarie :selena]

Christoph - can you comment on the priority of this feature?

Comment 3

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Selena Deckelmann :selenamarie :selena use ni? pronoun: she from comment #2)
> Christoph - can you comment on the priority of this feature?

I've discussed things with Anne on Slack and I think we should pump up the priority on this one - probably even P1. Finally I also think we should re-open Bug 1447631 and investigate further.

Probably :jkt can fix that for us.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1447631#c23

Comment 4

[Jonathan Kingston [:jkt] he/him]

Attachment: Bug 1483815 - Implement stricter CORS checking for headers.

Comment 5

[Jonathan Kingston [:jkt] he/him]

Try push here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=41b883d2f9441b3ec70f2560806584f6b1871b54

Comment 6

[Jonathan Kingston [:jkt] he/him]

New try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5e54d7a083d46b11a9f0d99809eb24b98bd63a7a

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a46028ac9dbb
Implement stricter CORS checking for headers. r=ckerschb

Comment 8

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/a46028ac9dbb

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/ca9df8b30295
Implement stricter CORS checking for headers: set wpt as pass. a=wpt-fix

Comment 10

[Chris Mills [:cmills]]

Note to MDN writers:

I've added a note covering this to the Fx 65 rel notes:
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/65#Security

In terms of the docs, I'm not sure if this needs mentioning on the Accept, Accept-Language, and Content-Language ref pages?

Comment 11

[Jonathan Kingston [:jkt] he/him]

Thanks Chris.

Nah currently we didn't mention the existing restrictions so I suspect there isn't much value unless we have a page dedicated to CORS or the safelisted headers that explain it in more detail?

Comment 12

[Florian Scholz (Open Web Docs)]

The main MDN CORS article is this one: https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS

Certain HTTP header reference pages like "Accept-Language" mention "CORS-safelisted request-header: yes"
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Accept-Language

Which then links to a page
https://developer.mozilla.org/en-US/docs/Glossary/Simple_header

I'm not sure I understand the new restrictions introduced here, but can you have a look and see if we should mention this on any of these pages?

If not, then I'm happy to just have it on the Fx 65 rel notes and we can set this bug to "dev-doc-complete".

Comment 13

[Jonathan Kingston [:jkt] he/him]

It's a little complex I guess and I dunno if it's worthy of MDN, however it is at least inconsistent.

https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS mentions

• Content-Type (but note the additional requirements below)
• The only allowed values for the Content-Type header are:
  • application/x-www-form-urlencoded
  • multipart/form-data
  • text/plain

This is somewhat true but ignores that there can be a boundary in the Content-Type like text/plain; charset=US-ASCII which is somewhat explained in the parsing of the essence: https://mimesniff.spec.whatwg.org/#mime-type-miscellaneous the essence is used to check the allowed values in that list on MDN.

The changes here are to narrow what the values of a CORS-safelisted request-header are permissable under CORS. This link is already in MDN and perhaps it's worthwhile just mentioning there are caveats to the values?

So for example the change reduced the values permissible as a safelisted header:

• Accept-Language and Content-Language can only be values with 0-9, A-Z, a-z, space or *,=.;=
• Content-Type and Accept can't contain https://fetch.spec.whatwg.org/#cors-unsafe-request-header-byte which is: "():<>?@[\]{}, Delete, Tab and control characters: 0x00 to 0x19

For values not considered safelisted then we use a preflight request to check that the server can handle these values. The change is mostly to prevent websites attacking webservers (local or remote) that don't do correct security checking; they are leaking data or adding more functionality than they should be doing to a third party.

Comment 14

[Florian Scholz (Open Web Docs)]

Thanks so much for your answer, :jkt! This sheds some light.

I'm cc'ing Mike aka sideshowbarker who is one of the main contributors to our CORS docs and knows a lot more about this than me. Mike, would you be willing to help out with updates here?

I think we could mention the reduced values permissible as a safelisted header on these pages:
https://developer.mozilla.org/en-US/docs/Glossary/Simple_header
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Accept
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Accept-Language
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Language
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Type

Comment 15

[Michael[tm] Smith [:sideshowbarker]]

(In reply to Florian Scholz [:fscholz] (MDN) from comment #14)

I'm cc'ing Mike aka sideshowbarker who is one of the main contributors to our CORS docs and knows a lot more about this than me. Mike, would you be willing to help out with updates here?

Yeah, I’d be happy to put some time into this

Comment 16

[Florian Scholz (Open Web Docs)]

This came up again here: https://bugzilla.mozilla.org/show_bug.cgi?id=1309358#c30

As part of that, I've now updated these pages:
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Headers
https://developer.mozilla.org/en-US/docs/Glossary/CORS-safelisted_request_header (especially the additional restriction section)
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Accept
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Accept-Language
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Language
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Type

Marking as dev-doc-complete but let me know if there is more left to do. Would also appreciate a review if you have a minute, :sideshowbarker.
Thanks!