Closed Bug 1483815 Opened 5 years ago Closed 5 years ago

Implement additional CORS restrictions

Categories

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

Product:
Core
Component:
DOM: Networking
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla65
Tracking Flags:
Tracking Status
firefox65 --- fixed

People

(Reporter: annevk, Assigned: jkt)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete, Whiteboard: [necko-triaged])

Attachments

(1 file)

Bug 1483815 - Implement stricter CORS checking for headers.
47 bytes, text/x-phabricator-request
Details | Review 
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.
Component: DOM → DOM: Networking
Christoph - can you comment on the priority of this feature?
Flags: needinfo?(ckerschb)
Blocks: 1447631
(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
Assignee: nobody → jkt
Status: NEW → ASSIGNED
Flags: needinfo?(ckerschb) → needinfo?(jkt)
Priority: P3 → P1
Whiteboard: [necko-triaged]
Attachment #9024447 - Attachment description: Bug 1483815 - Intial implementation of stricter CORS checking, broken. → Bug 1483815 - Implement stricter CORS checking for headers.
Pushed by jkingston@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a46028ac9dbb
Implement stricter CORS checking for headers. r=ckerschb
https://hg.mozilla.org/mozilla-central/rev/a46028ac9dbb
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
status-firefox65: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Pushed by rmaries@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/ca9df8b30295
Implement stricter CORS checking for headers: set wpt as pass. a=wpt-fix
See Also: → 1507514
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?
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?

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".

Flags: needinfo?(jkt)
Keywords: dev-doc-needed → dev-doc-complete

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:

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.

Flags: needinfo?(jkt) → needinfo?(fscholz)

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

Flags: needinfo?(fscholz)

(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

You need to log in before you can comment on or make changes to this bug.