Define Cross-Origin-Resource-Policy response header #733

annevk · 2018-05-24T10:33:51Z

This header makes it easier for sites to block unwanted "no-cors" cross-origin requests.

Tests: ...

Fixes #687.

Preview | Diff

mikewest

Thanks! I have a few quick thoughts, only one real issue.

mikewest · 2018-05-24T10:44:10Z

fetch.bs

+<p>Its <a for=header>value</a> <a>ABNF</a>:
+
+<pre>
+Cross-Origin-Resource-Policy     = %x73.61.6D.65 / %x73.61.6D.65.2D.73.69.74.65 ; "same" / "same-site"; case-sensitive</pre>


Nit: The last semicolon should be a comma (similar to the origin-or-null definition).

mikewest · 2018-05-24T10:46:25Z

fetch.bs

+  <var>request</var>'s <a for=request>current url</a>'s <a for=url>origin</a>, then return
+  <b>allowed</b>.
+
+  <p class=note>A cross-origin response redirecting to a same or same-site resource with the


Nit: This is a little unclear. Is the redirect target same-site with the request's origin, or with the cross-origin response?

The former was the intent, but yeah, that needs to be clarified, if we indeed want this behavior.

mikewest · 2018-05-24T10:50:32Z

fetch.bs

+
+   <li><p><var>request</var>'s <a for=request>current url</a>'s <a for=url>host</a>
+   <a>is a registrable domain suffix of or is equal to</a> <var>request</var>'s
+   <a for=request>origin</a>'s <a for=origin>host</a>


I think this isn't broad enough. The bidirectional is a registrable domain suffix of or is equal to comparison would return false for a request whose origin's host is subdomain.subdomain.example.com and whose current url is notsubdomain.example.com.

Instead, I think we need to calculate the relevant registrable domain (perhaps by pointing to the (not terribly clear) algorithm on https://publicsuffix.org/list/) for both the request's origin and the request's current URL, and compare those.

(And, actually, looking at that algorithm again, I'm not sure it does what it needs to do for document.domain either... It doesn't calculate the registrable domain at all, but the public suffix. I'll file a separate bug.)

whatwg/html#3711

Time to revisit whatwg/url#72? I suspect having "registrable domain" and "same-site" as a primitive available will be quite useful going forward.

We also need to think to what extent we want to do scheme/port-comparisons as well.

Yeah. Ok, assuming I can dig myself out from under everything else, would you prefer those concepts be defined in HTML, Fetch, or URL?

Note: this is now tracked by whatwg/url#391.

mikewest · 2018-05-24T10:58:43Z

fetch.bs

+
+ <li><p>Let <var>policy</var> be the <a>combined value</a> with
+ `<a http-header><code>Cross-Origin-Resource-Policy</code></a>` and <var>response</var>'s
+ <a for=response>header list</a>.


It might be worth adding a note here about the meaning of same, same or same, same-site, as I believe this means that both will be ignored.

Yeah, that's the intent. Perhaps we should split on , though given that it seems likely we'll add support for multiple explicit origins later.

I don't think it matters in this patch, but I agree that that's probably what we'd want to do in a future patch (e.g. define this as a structured header list whose members are strings that we parse as origins or something.

Hmm, wouldn't that be incompatible with same and same-site as tokens? And origins cannot be used as tokens, at least not unless httpwg/http-extensions#629 is fixed somehow.

I don't think there's any requirement that all the items in a list are the same type. @mnot can confirm, but my understanding is that we could have same and same-site as "labels" and some other type ("string", I suppose) for origins.

youennf

PR sounds good to me, in particular:

When happens the CORP check. It is handling all responses even intermediate ones
CORP does apply to redirections but not to the final response-after-redirection if it is again same origin

The one thing that might need further discussion is whether to apply CORP on preflight responses.
It seems this PR implies that CORP should be checked. WebKit does not enforce it and it makes sense to me to keep it that way.
This could for instance ease deployment strategies such as "stick that CORP: same-site header on any response"

FWIW, the WebKit patch aligning with this PR should land shortly.
I will upstream the tests we have so far to WPT.
They should be further completed by header value parsing, preflight and challenge response dedicated tests.

annevk · 2018-05-28T08:55:15Z

fetch.bs

@@ -3736,6 +3794,9 @@ Range Requests</cite>. [[HTTP-RANGE]] However, this is not widely supported by b
    </ol>
  </ol>

+ <li><p>If the <a>cross-origin resource policy check</a> with <var>request</var> and


@youennf if we fixed #741 and skipped this step if the CORS flag is set, we'd address the CORS-preflight issue I think.

annevk · 2018-05-29T08:56:47Z

I pushed a commit that should address the review feedback.

It builds upon whatwg/url#391 (nearly done) and #743 (needs review).

@mnot can you confirm that structured headers will allow mixing of identifiers and strings?

annevk · 2018-05-30T06:20:56Z

FYI: I rebased and renamed same to same-origin.

annevk · 2018-06-07T10:05:25Z

Dependencies have been merged, plan of action is at #687 (comment). TL;DR: this will merge next week barring any objections.

Supplements #11171. For whatwg/fetch#733.

This header makes it easier for sites to block unwanted "no-cors" cross-origin requests. Tests: * web-platform-tests/wpt#11171 * web-platform-tests/wpt#11427 * web-platform-tests/wpt#11428 Follow-up: #760. Fixes #687.

annevk · 2018-06-08T11:17:58Z

fetch.bs

+   <var>request</var>'s <a for=request>current url</a>'s <a for=url>host</a>
+   <li><var>request</var>'s <a for=request>origin</a>'s <a for=url>scheme</a> is
+   "<code>https</code>" or <var>response</var>'s <a for=response>HTTPS state</a> is
+   "<code>none</code>"


@mikewest is this too cute and should I be checking the scheme of request's current url instead or is this fine/preferred?

I feel like I had to think about what this means more than I'd like to. An explicit comparison seems simpler. Or maybe just skipping the check around? Is something like this equivalent?

If response's HTTPS state is modern, request's origin's scheme is https.

That doesn't seem equivalent as it doesn't clearly evaluate to a boolean.

For whatwg/fetch#733. WebKit export of https://bugs.webkit.org/show_bug.cgi?id=185840.

Supplements #11171. For whatwg/fetch#733.

mikewest

LGTM. This patch is a pretty reasonable definition of the feature, thanks for putting it together, @annevk!

That said, I think there's enough subtlety here that it would be worth writing up a separate document that walked through the details, expectations, and rationales behind the header's design. The underlying proposal went through a few mutations, and it feels like we're doing it a disservice by only recording the algorithm, and none of the ancillary justification. If I pointed a developer to this section of Fetch, I suspect they'd have no idea what problem it attempts to solve, or why it's relevant to them at all.

Anyway, LGTM to land the definition as something browser vendors can rely on while implementing, but I hope we can collectively find time to fill in some of the details at greater length.

mikewest · 2018-06-18T09:36:42Z

fetch.bs

+   <var>request</var>'s <a for=request>current url</a>'s <a for=url>host</a>
+   <li><var>request</var>'s <a for=request>origin</a>'s <a for=url>scheme</a> is
+   "<code>https</code>" or <var>response</var>'s <a for=response>HTTPS state</a> is
+   "<code>none</code>"


I feel like I had to think about what this means more than I'd like to. An explicit comparison seems simpler. Or maybe just skipping the check around? Is something like this equivalent?

If response's HTTPS state is modern, request's origin's scheme is https.

mikewest · 2018-06-18T09:48:38Z

fetch.bs

+  <p class="note no-backref">A cross-origin response redirecting to a response that is
+  <a>same origin</a> or <a>same site</a> with the initial request and has a
+  `<a http-header><code>Cross-Origin-Resource-Policy</code></a>` header specified, does not affect
+  anything. I.e., <var>request</var>'s <a for=request>tainted origin flag</a> is not checked.


Nit: I'd rephrase this a little bit. Perhaps something like "The cross-origin resource policy check does not consider a request's redirect chain when processing a given response's Cross-Origin-Resource-Policy header, but only the relationship between the requesting origin and the response which asserts the header."?

Rewrote, but didn't use "redirect chain" as that's not really defined.

annevk · 2018-06-18T12:20:36Z

I filed #767 on adding advice.

For whatwg/fetch#733. WebKit export of https://bugs.webkit.org/show_bug.cgi?id=185840.

Supplements #11171. For whatwg/fetch#733.

annevk · 2018-06-18T13:34:56Z

Bugs:

…stonly Automatic update from web-platform-testsFetch: Cross-Origin-Resource-Policy tests For whatwg/fetch#733. WebKit export of https://bugs.webkit.org/show_bug.cgi?id=185840. -- wpt-commits: 53f7340307c1c0fa4ab96e79d88c69a7870030f4 wpt-pr: 11171

…in-Resource-Policy, a=testonly Automatic update from web-platform-testsFetch: basic syntax tests for Cross-Origin-Resource-Policy Supplements #11171. For whatwg/fetch#733. -- wpt-commits: b7373b42eeac24ff6cb3ed494ffbf09e781287da wpt-pr: 11427

…: same-site's scheme restriction, a=testonly Automatic update from web-platform-testsFetch: test Cross-Origin-Resource-Policy: same-site's scheme restriction Supplements #11171. For whatwg/fetch#733. -- wpt-commits: 7f0a106f3d5e9d3e7f70ba52aae896a3fffc2cc6 wpt-pr: 11428

…stonly Automatic update from web-platform-testsFetch: Cross-Origin-Resource-Policy tests For whatwg/fetch#733. WebKit export of https://bugs.webkit.org/show_bug.cgi?id=185840. -- wpt-commits: 53f7340307c1c0fa4ab96e79d88c69a7870030f4 wpt-pr: 11171 UltraBlame original commit: 2d3d375085ab145c32c6a3de5589739a3a5ceaf0

…in-Resource-Policy, a=testonly Automatic update from web-platform-testsFetch: basic syntax tests for Cross-Origin-Resource-Policy Supplements #11171. For whatwg/fetch#733. -- wpt-commits: b7373b42eeac24ff6cb3ed494ffbf09e781287da wpt-pr: 11427 UltraBlame original commit: 0d5c1298c115f6ccc0b1e19e7494b1dd72ae7f18

…: same-site's scheme restriction, a=testonly Automatic update from web-platform-testsFetch: test Cross-Origin-Resource-Policy: same-site's scheme restriction Supplements #11171. For whatwg/fetch#733. -- wpt-commits: 7f0a106f3d5e9d3e7f70ba52aae896a3fffc2cc6 wpt-pr: 11428 UltraBlame original commit: 2088a3b878a40a0f734fa8e04ade1417b951e1d4

…stonly Automatic update from web-platform-testsFetch: Cross-Origin-Resource-Policy tests For whatwg/fetch#733. WebKit export of https://bugs.webkit.org/show_bug.cgi?id=185840. -- wpt-commits: 53f7340307c1c0fa4ab96e79d88c69a7870030f4 wpt-pr: 11171 UltraBlame original commit: 2d3d375085ab145c32c6a3de5589739a3a5ceaf0

…in-Resource-Policy, a=testonly Automatic update from web-platform-testsFetch: basic syntax tests for Cross-Origin-Resource-Policy Supplements #11171. For whatwg/fetch#733. -- wpt-commits: b7373b42eeac24ff6cb3ed494ffbf09e781287da wpt-pr: 11427 UltraBlame original commit: 0d5c1298c115f6ccc0b1e19e7494b1dd72ae7f18

…: same-site's scheme restriction, a=testonly Automatic update from web-platform-testsFetch: test Cross-Origin-Resource-Policy: same-site's scheme restriction Supplements #11171. For whatwg/fetch#733. -- wpt-commits: 7f0a106f3d5e9d3e7f70ba52aae896a3fffc2cc6 wpt-pr: 11428 UltraBlame original commit: 2088a3b878a40a0f734fa8e04ade1417b951e1d4

…stonly Automatic update from web-platform-testsFetch: Cross-Origin-Resource-Policy tests For whatwg/fetch#733. WebKit export of https://bugs.webkit.org/show_bug.cgi?id=185840. -- wpt-commits: 53f7340307c1c0fa4ab96e79d88c69a7870030f4 wpt-pr: 11171 UltraBlame original commit: 2d3d375085ab145c32c6a3de5589739a3a5ceaf0

…in-Resource-Policy, a=testonly Automatic update from web-platform-testsFetch: basic syntax tests for Cross-Origin-Resource-Policy Supplements #11171. For whatwg/fetch#733. -- wpt-commits: b7373b42eeac24ff6cb3ed494ffbf09e781287da wpt-pr: 11427 UltraBlame original commit: 0d5c1298c115f6ccc0b1e19e7494b1dd72ae7f18

…: same-site's scheme restriction, a=testonly Automatic update from web-platform-testsFetch: test Cross-Origin-Resource-Policy: same-site's scheme restriction Supplements #11171. For whatwg/fetch#733. -- wpt-commits: 7f0a106f3d5e9d3e7f70ba52aae896a3fffc2cc6 wpt-pr: 11428 UltraBlame original commit: 2088a3b878a40a0f734fa8e04ade1417b951e1d4

annevk requested review from mikewest and youennf May 24, 2018 10:34

annevk mentioned this pull request May 24, 2018

Cross-Origin-Resource-Policy (was: From-Origin) #687

Closed

mikewest reviewed May 24, 2018

View reviewed changes

annevk mentioned this pull request May 24, 2018

Identifiers httpwg/http-extensions#629

Closed

annevk added addition/proposal New features or enhancements security/privacy There are security or privacy implications needs tests Moving the issue forward requires someone to write tests labels May 24, 2018

youennf reviewed May 25, 2018

View reviewed changes

youennf mentioned this pull request May 25, 2018

Cross-Origin-Resource-Policy tests web-platform-tests/wpt#11171

Merged

annevk mentioned this pull request May 28, 2018

CORS-preflight fetch #741

Closed

annevk commented May 28, 2018

View reviewed changes

annevk force-pushed the annevk/corp branch from d27c5df to 7ee7a08 Compare May 29, 2018 08:41

annevk added do not merge yet Pull request must not be merged per rationale in comment and removed needs tests Moving the issue forward requires someone to write tests labels May 29, 2018

annevk force-pushed the annevk/corp branch from a2fd262 to f31e50a Compare May 30, 2018 06:16

annevk added a commit to web-platform-tests/wpt that referenced this pull request Jun 8, 2018

Fetch: test Cross-Origin-Resource-Policy: same-site's scheme restriction

3403f42

Supplements #11171. For whatwg/fetch#733.

annevk mentioned this pull request Jun 8, 2018

Fetch: test Cross-Origin-Resource-Policy: same-site's scheme restriction web-platform-tests/wpt#11428

Merged

Define Cross-Origin-Resource-Policy response header

ce08dff

This header makes it easier for sites to block unwanted "no-cors" cross-origin requests. Tests: * web-platform-tests/wpt#11171 * web-platform-tests/wpt#11427 * web-platform-tests/wpt#11428 Follow-up: #760. Fixes #687.

annevk force-pushed the annevk/corp branch from 4dbf64d to ce08dff Compare June 8, 2018 11:00

prevent HTTPS responses from being accessed without secure transport

e18fa04

annevk commented Jun 8, 2018

View reviewed changes

annevk removed the do not merge yet Pull request must not be merged per rationale in comment label Jun 18, 2018

annevk pushed a commit to youennf/web-platform-tests that referenced this pull request Jun 18, 2018

Fetch: Cross-Origin-Resource-Policy tests

eab3b0e

For whatwg/fetch#733. WebKit export of https://bugs.webkit.org/show_bug.cgi?id=185840.

annevk added a commit to web-platform-tests/wpt that referenced this pull request Jun 18, 2018

Fetch: test Cross-Origin-Resource-Policy: same-site's scheme restriction

cd37c77

Supplements #11171. For whatwg/fetch#733.

mikewest approved these changes Jun 18, 2018

View reviewed changes

annevk mentioned this pull request Jun 18, 2018

Cross-Origin-Resource-Policy advice #767

Open

rewrite note

ed96a17

foolip pushed a commit to web-platform-tests/wpt that referenced this pull request Jun 18, 2018

Fetch: Cross-Origin-Resource-Policy tests

53f7340

For whatwg/fetch#733. WebKit export of https://bugs.webkit.org/show_bug.cgi?id=185840.

annevk added a commit to web-platform-tests/wpt that referenced this pull request Jun 18, 2018

Fetch: basic syntax tests for Cross-Origin-Resource-Policy

b7373b4

Supplements #11171. For whatwg/fetch#733.

annevk added a commit to web-platform-tests/wpt that referenced this pull request Jun 18, 2018

Fetch: test Cross-Origin-Resource-Policy: same-site's scheme restriction

b8b37d4

Supplements #11171. For whatwg/fetch#733.

annevk added a commit to web-platform-tests/wpt that referenced this pull request Jun 18, 2018

Fetch: test Cross-Origin-Resource-Policy: same-site's scheme restriction

7f0a106

Supplements #11171. For whatwg/fetch#733.

annevk merged commit 0cec471 into master Jun 18, 2018

annevk deleted the annevk/corp branch June 18, 2018 13:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Define Cross-Origin-Resource-Policy response header #733

Define Cross-Origin-Resource-Policy response header #733

annevk commented May 24, 2018 •

edited by pr-preview bot

Loading

mikewest left a comment

mikewest May 24, 2018

mikewest May 24, 2018

annevk May 24, 2018

mikewest May 24, 2018

mikewest May 24, 2018

mikewest May 24, 2018

annevk May 24, 2018

mikewest May 24, 2018

annevk May 28, 2018

mikewest May 24, 2018

annevk May 24, 2018

mikewest May 24, 2018

annevk May 24, 2018

mikewest May 24, 2018

youennf left a comment

annevk May 28, 2018

annevk commented May 29, 2018

annevk commented May 30, 2018

annevk commented Jun 7, 2018

annevk Jun 8, 2018

mikewest Jun 18, 2018

annevk Jun 18, 2018

mikewest left a comment

mikewest Jun 18, 2018

mikewest Jun 18, 2018

annevk Jun 18, 2018

annevk commented Jun 18, 2018

annevk commented Jun 18, 2018

Define Cross-Origin-Resource-Policy response header #733

Define Cross-Origin-Resource-Policy response header #733

Conversation

annevk commented May 24, 2018 • edited by pr-preview bot Loading

mikewest left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youennf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

annevk commented May 29, 2018

annevk commented May 30, 2018

annevk commented Jun 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikewest left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

annevk commented Jun 18, 2018

annevk commented Jun 18, 2018

annevk commented May 24, 2018 •

edited by pr-preview bot

Loading