Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fetch() "no-cors": cross-origin to same-origin redirect taints response #737

Open
annevk opened this issue May 25, 2018 · 18 comments
Open
Labels
security/privacy There are security or privacy implications topic: html topic: redirects

Comments

@annevk
Copy link
Member

annevk commented May 25, 2018

Although per step 5 of main fetch https://fetch.spec.whatwg.org/#main-fetch if you're on a same-origin URL again after a redirect the tainting gets reset to "basic", that doesn't appear to happen in implementations.

At least for fetch() they'll taint once you go cross-origin.

I don't think they're consistent in that however as cross-origin to same-origin for images will allow image data extraction.

I guess this requires more research.

@annevk annevk added the security/privacy There are security or privacy implications label May 25, 2018
@domfarolino
Copy link
Member

domfarolino commented May 27, 2018

Interesting; Chrome is indeed one of the violators of not resetting tainting back to "basic" after a fetch() lands on same-origin after cross-origin redirect, but that's a simple fix. Are you considering a spec change here depending on what implementations are doing?

@annevk
Copy link
Member Author

annevk commented May 28, 2018

I was initially, but I'm wondering if what browsers do here is preferable, since even though you can read such responses as the result of navigation (<iframe>) or image loads, that doesn't allow you to read the bytes of all such responses (e.g., a PDF).

So I think we probably want to keep this restriction intact where we can (and break it for navigation, images, scripts?, ?).

@wanderview
Copy link
Member

I think we should avoid reseting back to basic tainting where we can. This is consistent with how cors requests can never become same-origin again once they cross an origin boundary. It would be a bit unexpected to me to make no-cors less restrictive.

@annevk
Copy link
Member Author

annevk commented May 29, 2018

I think I added the reset because of <img> and the way <iframe> works (which is "navigate", not "no-cors"). And because I was sloppy I didn't test fetch() I guess. I'm not entirely sure yet how to fix this, but I agree with you on the desirable outcome.

@domfarolino
Copy link
Member

(Whoops, this slipped through the cracks in my email). I agree with this as well here, and think we should avoid resetting back to basic tainting.

@youennf
Copy link
Collaborator

youennf commented May 30, 2018

+1 to not resetting tainting.
I haven't tested it on WebKit but if we are resetting tainting in some cases like canvas, this is probably a bug.

yutakahirano added a commit to web-platform-tests/wpt that referenced this issue Aug 20, 2018
This is discussed at whatwg/fetch#737, but
currently implementors have a somewhat consistent behavior. I will
update the test if we change the behavior.
@yutakahirano
Copy link
Member

Hi, I'm going to add a test because I was about to change the behavior without noticing this issue. I don't have a strong opinion whether we should reset tainting, but I strongly want to have a consistent behavior across fetch() and other initiators.

@yutakahirano
Copy link
Member

The test is here. web-platform-tests/wpt#12566

pull bot pushed a commit to hoojaoh/web-platform-tests that referenced this issue Aug 21, 2018
…gin (web-platform-tests#12566)

This is discussed at whatwg/fetch#737, but
currently implementors have a somewhat consistent behavior. I will
update the test if we change the behavior.
zcorpan pushed a commit to web-platform-tests/wpt that referenced this issue Aug 27, 2018
…gin (#12566)

This is discussed at whatwg/fetch#737, but
currently implementors have a somewhat consistent behavior. I will
update the test if we change the behavior.
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Aug 29, 2018
…ting back to the original origin, a=testonly

Automatic update from web-platform-testsAdd tests for cross-origin redirects getting back to the original origin (#12566)

This is discussed at whatwg/fetch#737, but
currently implementors have a somewhat consistent behavior. I will
update the test if we change the behavior.
--

wpt-commits: c5400eeca28ce5b1dd10d442f0c39a6274d8522d
wpt-pr: 12566
jankeromnes pushed a commit to jankeromnes/gecko that referenced this issue Aug 29, 2018
…ting back to the original origin, a=testonly

Automatic update from web-platform-testsAdd tests for cross-origin redirects getting back to the original origin (#12566)

This is discussed at whatwg/fetch#737, but
currently implementors have a somewhat consistent behavior. I will
update the test if we change the behavior.
--

wpt-commits: c5400eeca28ce5b1dd10d442f0c39a6274d8522d
wpt-pr: 12566
annevk added a commit that referenced this issue Nov 16, 2018
This also addresses #737 in that now A -> B -> A would be considered cross-origin even for "no-cors", but leaving that open for further plumbing in HTML et al to override that in select cases (e.g., <img>).

Fixes #756.
@annevk
Copy link
Member Author

annevk commented Nov 16, 2018

I created a fix for this for Fetch that I think we should land unless it has a bug I didn't see: #834.

However, A -> B -> A is considered same-origin for <img>, <script>, and similar such contexts and we'll need to make sure that's defined properly there (by having HTML poke into the opaque response) and tested.

I'll leave this issue open until that's fully taken care of.

Hope that seems reasonable to everyone.

@youennf
Copy link
Collaborator

youennf commented Nov 16, 2018

In WebKit, there is no specific handling for img or script with that regards.
I would believe A->B->A would end up being considered cross origin.
I'll check that further.

@annevk
Copy link
Member Author

annevk commented Nov 16, 2018

That sounds exciting, though for <img> a workaround would likely be to use <iframe> which I'm 99% sure on only considers the final response for origin comparisons.

@youennf
Copy link
Collaborator

youennf commented Nov 17, 2018

Uploaded a test in web-platform-tests/wpt#14112
According that test, Firefox and Chrome do treat A->B->A image as same origin while Safari does not. Not sure about Edge.
It might be worth authoring a similar test for scripts and stylesheets.

That sounds exciting, though for <img> a workaround would likely be to use <iframe> which I'm 99% sure on only considers the final response for origin comparisons.

You are probably right, we probably only consider final URLs for iframes origin checks.

annevk added a commit that referenced this issue Nov 20, 2018
This also addresses #737 in that now A -> B -> A would be considered cross-origin even for "no-cors", but leaving that open to discuss whether HTML et al need to override that in select cases (e.g., <img>).

Fixes #756.
@domfarolino
Copy link
Member

Was looking at https://html.spec.whatwg.org/multipage/canvas.html#drawing-images:the-image-argument-is-not-origin-clean which sets a CanvasRenderingContext2D's origin-clean flag to false if image is not origin-clean; it seems like the only criteria considered when determining whether or not an image is not origin-clean is the image's origin with respect to that of the entry settings object.

I think that the origin of an image that undergoes A->B->A redirects will be same-origin with that of the entry settings object, even though the response is CORS-cross-origin. It seems if we want the behavior that @youennf's test proposes, we'd have to make the definition of is not origin-clean also consider whether image's origin is CORS-cross-origin right, similarly to what we do muted errors? (As opposed to what we thought before, where HTML would have to be updated after #834 to allow reading this data from a canvas if desirable)

@annevk
Copy link
Member Author

annevk commented Nov 23, 2018

Yeah, the current definition is wrong, it also doesn't match what browsers do when document.domain is involved, see https://www.w3.org/Bugs/Public/show_bug.cgi?id=28374. I forgot that either way we're gonna have to change HTML.

@yutakahirano
Copy link
Member

I accidentally changed Chromium behavior months ago, and it's aligned with the new spec. I've had no complaints so far, so I'm leaning to just accepting the new behavior without having an experiment for compatibility.

annevk pushed a commit to web-platform-tests/wpt that referenced this issue Dec 10, 2018
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Dec 11, 2018
This adds tests that stylesheets that result from requests that were
redirected cross-origin are considered cross-origin.

Note that A->B->A redirects, which redirect from cross-origin to
same-origin, are considered cross-origin. See
whatwg/fetch#737 and
whatwg/fetch#834.

In Blink, we have redirect tests at
http/tests/security/cannot-read-cssrules-redirect.html. This WPT
addition will supersede that test, but I won't yet remove it since
it asserts the opposite for the A->B->A case. I can remove the test
when Blink changes to pass this WPT test.

Bug: 911974
Change-Id: Ie015c0390829299de7c29cff6685ddfcd774c66f
aarongable pushed a commit to chromium/chromium that referenced this issue Dec 11, 2018
This adds tests that stylesheets that result from requests that were
redirected cross-origin are considered cross-origin.

Note that A->B->A redirects, which redirect from cross-origin to
same-origin, are considered cross-origin. See
whatwg/fetch#737 and
whatwg/fetch#834.

In Blink, we have redirect tests at
http/tests/security/cannot-read-cssrules-redirect.html. This WPT
addition will supersede that test, but I won't yet remove it since
it asserts the opposite for the A->B->A case. I can remove the test
when Blink changes to pass this WPT test.

Bug: 911974
Change-Id: Ie015c0390829299de7c29cff6685ddfcd774c66f
Reviewed-on: https://chromium-review.googlesource.com/c/1370162
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615475}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Dec 11, 2018
This adds tests that stylesheets that result from requests that were
redirected cross-origin are considered cross-origin.

Note that A->B->A redirects, which redirect from cross-origin to
same-origin, are considered cross-origin. See
whatwg/fetch#737 and
whatwg/fetch#834.

In Blink, we have redirect tests at
http/tests/security/cannot-read-cssrules-redirect.html. This WPT
addition will supersede that test, but I won't yet remove it since
it asserts the opposite for the A->B->A case. I can remove the test
when Blink changes to pass this WPT test.

Bug: 911974
Change-Id: Ie015c0390829299de7c29cff6685ddfcd774c66f
Reviewed-on: https://chromium-review.googlesource.com/c/1370162
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615475}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Dec 11, 2018
This adds tests that stylesheets that result from requests that were
redirected cross-origin are considered cross-origin.

Note that A->B->A redirects, which redirect from cross-origin to
same-origin, are considered cross-origin. See
whatwg/fetch#737 and
whatwg/fetch#834.

In Blink, we have redirect tests at
http/tests/security/cannot-read-cssrules-redirect.html. This WPT
addition will supersede that test, but I won't yet remove it since
it asserts the opposite for the A->B->A case. I can remove the test
when Blink changes to pass this WPT test.

Bug: 911974
Change-Id: Ie015c0390829299de7c29cff6685ddfcd774c66f
Reviewed-on: https://chromium-review.googlesource.com/c/1370162
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615475}
@mfalken
Copy link

mfalken commented Dec 11, 2018

I added A->B->A tests for stylesheets in web-platform-tests/wpt#14452. It looks like Chrome and Firefox currently fail it, but I'm working on changing Chrome to pass it at https://chromium-review.googlesource.com/c/chromium/src/+/1367331 (will send a Blink Intent to Ship shortly).

aarongable pushed a commit to chromium/chromium that referenced this issue Dec 14, 2018
Use ResourceResponse::ResponseUrl() to set the base URL, and use
ResponseResponse::GetType() to determine whether the resonse is
CORS-same-origin.

This CL has three web-exposed changes.

1. Use the response URL rather than the last request URL as the base URL
   of the stylesheet. This aligns with the standard. See
   whatwg/fetch#146 and WPT results indicate
   Firefox, Edge, and Safari use the response URL. This only matters if
   the response came from a service worker, as the URLs only differ
   when the service worker intercepts the request and responds with a
   different URL via respondWith(fetch(other_url)).

   This is covered by the WPT:
   service-workers/service-worker/fetch-request-css-base-url.https.html

   The test doesn't completely pass yet because the search query part of
   the URL gets chopped off for FetchEvent.request.referrer, but the base
   URL is correct.

   Chrome Status: https://www.chromestatus.com/feature/5642183499579392

2. Consider A->B->A redirects to be cross-origin rather than
   same-origin. Previously, this was considered same-origin. See the
   discussion in whatwg/fetch#737 and change
   whatwg/fetch#834.

   This change makes the following WPT test pass:
   css/cssom/stylesheet-same-origin.sub.html

   It also affects the web test:
   http/tests/security/cannot-read-cssrules-redirect.html

   This test is updated to match the behavior change. It can be removed
   later since it is redundant with the WPT test.

3. Consider load failures to be cross-origin rather than same-origin.
   That is, accessing |styleSheet.rules| throws a SecurityError if the
   load failed.  This aligns with the specification:
   - cssRules checks the `origin-clean` flag:
     https://drafts.csswg.org/cssom/#dom-cssstylesheet-cssrules
   - This is set to true iff CORS-same-origin:
     https://html.spec.whatwg.org/multipage/links.html#link-type-stylesheet
   - CORS-same-origin is false on kError:
     https://html.spec.whatwg.org/multipage/urls-and-fetching.html#cors-same-origin

   This change makes the following WPT test pass:
   css/cssom/stylesheet-same-origin.sub.html

   It also affects the web tests:
   register-bypassing-scheme-partial.html
   require-sri-for-style-blocked.php

   These tests are updated to match the behavior change.

Chrome Status: https://www.chromestatus.com/feature/5642183499579392
Intent to Ship: https://groups.google.com/a/chromium.org/d/msg/blink-dev/7OSy00oxVpk/siufiQVBBwAJ

Bug: 911974
Change-Id: I9add3162596963eee66f60f339cfd9911bc151cd
Reviewed-on: https://chromium-review.googlesource.com/c/1367331
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616580}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Dec 14, 2018
…ipts, a=testonly

Automatic update from web-platform-tests
Add muted error tests for redirected scripts

Helps with whatwg/fetch#737.
--

wpt-commits: 18bb80e6e1254b36db6467fdd1aa1e9e7748bb58
wpt-pr: 14218
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Dec 14, 2018
…s., a=testonly

Automatic update from web-platform-tests
WPT: CSS: Add cross-origin redirect tests.

This adds tests that stylesheets that result from requests that were
redirected cross-origin are considered cross-origin.

Note that A->B->A redirects, which redirect from cross-origin to
same-origin, are considered cross-origin. See
whatwg/fetch#737 and
whatwg/fetch#834.

In Blink, we have redirect tests at
http/tests/security/cannot-read-cssrules-redirect.html. This WPT
addition will supersede that test, but I won't yet remove it since
it asserts the opposite for the A->B->A case. I can remove the test
when Blink changes to pass this WPT test.

Bug: 911974
Change-Id: Ie015c0390829299de7c29cff6685ddfcd774c66f
Reviewed-on: https://chromium-review.googlesource.com/c/1370162
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615475}

--

wpt-commits: 600dd6cb4295d0bcfc867b8877287d485d3b0e4e
wpt-pr: 14452
mykmelez pushed a commit to mykmelez/gecko that referenced this issue Dec 15, 2018
…ipts, a=testonly

Automatic update from web-platform-tests
Add muted error tests for redirected scripts

Helps with whatwg/fetch#737.
--

wpt-commits: 18bb80e6e1254b36db6467fdd1aa1e9e7748bb58
wpt-pr: 14218
mykmelez pushed a commit to mykmelez/gecko that referenced this issue Dec 15, 2018
…s., a=testonly

Automatic update from web-platform-tests
WPT: CSS: Add cross-origin redirect tests.

This adds tests that stylesheets that result from requests that were
redirected cross-origin are considered cross-origin.

Note that A->B->A redirects, which redirect from cross-origin to
same-origin, are considered cross-origin. See
whatwg/fetch#737 and
whatwg/fetch#834.

In Blink, we have redirect tests at
http/tests/security/cannot-read-cssrules-redirect.html. This WPT
addition will supersede that test, but I won't yet remove it since
it asserts the opposite for the A->B->A case. I can remove the test
when Blink changes to pass this WPT test.

Bug: 911974
Change-Id: Ie015c0390829299de7c29cff6685ddfcd774c66f
Reviewed-on: https://chromium-review.googlesource.com/c/1370162
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615475}

--

wpt-commits: 600dd6cb4295d0bcfc867b8877287d485d3b0e4e
wpt-pr: 14452
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 3, 2019
…ting back to the original origin, a=testonly

Automatic update from web-platform-testsAdd tests for cross-origin redirects getting back to the original origin (#12566)

This is discussed at whatwg/fetch#737, but
currently implementors have a somewhat consistent behavior. I will
update the test if we change the behavior.
--

wpt-commits: c5400eeca28ce5b1dd10d442f0c39a6274d8522d
wpt-pr: 12566

UltraBlame original commit: b17979968e272b9e46c009a8cdbba327f0885f0a
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 3, 2019
…ting back to the original origin, a=testonly

Automatic update from web-platform-testsAdd tests for cross-origin redirects getting back to the original origin (#12566)

This is discussed at whatwg/fetch#737, but
currently implementors have a somewhat consistent behavior. I will
update the test if we change the behavior.
--

wpt-commits: c5400eeca28ce5b1dd10d442f0c39a6274d8522d
wpt-pr: 12566

UltraBlame original commit: b17979968e272b9e46c009a8cdbba327f0885f0a
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 3, 2019
…ting back to the original origin, a=testonly

Automatic update from web-platform-testsAdd tests for cross-origin redirects getting back to the original origin (#12566)

This is discussed at whatwg/fetch#737, but
currently implementors have a somewhat consistent behavior. I will
update the test if we change the behavior.
--

wpt-commits: c5400eeca28ce5b1dd10d442f0c39a6274d8522d
wpt-pr: 12566

UltraBlame original commit: b17979968e272b9e46c009a8cdbba327f0885f0a
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 3, 2019
…ipts, a=testonly

Automatic update from web-platform-tests
Add muted error tests for redirected scripts

Helps with whatwg/fetch#737.
--

wpt-commits: 18bb80e6e1254b36db6467fdd1aa1e9e7748bb58
wpt-pr: 14218

UltraBlame original commit: d1852804c36e57cb123dc6a336c58e986e16e02f
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 3, 2019
…s., a=testonly

Automatic update from web-platform-tests
WPT: CSS: Add cross-origin redirect tests.

This adds tests that stylesheets that result from requests that were
redirected cross-origin are considered cross-origin.

Note that A->B->A redirects, which redirect from cross-origin to
same-origin, are considered cross-origin. See
whatwg/fetch#737 and
whatwg/fetch#834.

In Blink, we have redirect tests at
http/tests/security/cannot-read-cssrules-redirect.html. This WPT
addition will supersede that test, but I won't yet remove it since
it asserts the opposite for the A->B->A case. I can remove the test
when Blink changes to pass this WPT test.

Bug: 911974
Change-Id: Ie015c0390829299de7c29cff6685ddfcd774c66f
Reviewed-on: https://chromium-review.googlesource.com/c/1370162
Reviewed-by: Rune Lillesveen <futharkchromium.org>
Commit-Queue: Matt Falkenhagen <falkenchromium.org>
Cr-Commit-Position: refs/heads/master{#615475}

--

wpt-commits: 600dd6cb4295d0bcfc867b8877287d485d3b0e4e
wpt-pr: 14452

UltraBlame original commit: 004e8fbaeead9b556d1972586b1a17e7647b09e8
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 3, 2019
…ipts, a=testonly

Automatic update from web-platform-tests
Add muted error tests for redirected scripts

Helps with whatwg/fetch#737.
--

wpt-commits: 18bb80e6e1254b36db6467fdd1aa1e9e7748bb58
wpt-pr: 14218

UltraBlame original commit: d1852804c36e57cb123dc6a336c58e986e16e02f
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 3, 2019
…s., a=testonly

Automatic update from web-platform-tests
WPT: CSS: Add cross-origin redirect tests.

This adds tests that stylesheets that result from requests that were
redirected cross-origin are considered cross-origin.

Note that A->B->A redirects, which redirect from cross-origin to
same-origin, are considered cross-origin. See
whatwg/fetch#737 and
whatwg/fetch#834.

In Blink, we have redirect tests at
http/tests/security/cannot-read-cssrules-redirect.html. This WPT
addition will supersede that test, but I won't yet remove it since
it asserts the opposite for the A->B->A case. I can remove the test
when Blink changes to pass this WPT test.

Bug: 911974
Change-Id: Ie015c0390829299de7c29cff6685ddfcd774c66f
Reviewed-on: https://chromium-review.googlesource.com/c/1370162
Reviewed-by: Rune Lillesveen <futharkchromium.org>
Commit-Queue: Matt Falkenhagen <falkenchromium.org>
Cr-Commit-Position: refs/heads/master{#615475}

--

wpt-commits: 600dd6cb4295d0bcfc867b8877287d485d3b0e4e
wpt-pr: 14452

UltraBlame original commit: 004e8fbaeead9b556d1972586b1a17e7647b09e8
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 3, 2019
…ipts, a=testonly

Automatic update from web-platform-tests
Add muted error tests for redirected scripts

Helps with whatwg/fetch#737.
--

wpt-commits: 18bb80e6e1254b36db6467fdd1aa1e9e7748bb58
wpt-pr: 14218

UltraBlame original commit: d1852804c36e57cb123dc6a336c58e986e16e02f
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 3, 2019
…s., a=testonly

Automatic update from web-platform-tests
WPT: CSS: Add cross-origin redirect tests.

This adds tests that stylesheets that result from requests that were
redirected cross-origin are considered cross-origin.

Note that A->B->A redirects, which redirect from cross-origin to
same-origin, are considered cross-origin. See
whatwg/fetch#737 and
whatwg/fetch#834.

In Blink, we have redirect tests at
http/tests/security/cannot-read-cssrules-redirect.html. This WPT
addition will supersede that test, but I won't yet remove it since
it asserts the opposite for the A->B->A case. I can remove the test
when Blink changes to pass this WPT test.

Bug: 911974
Change-Id: Ie015c0390829299de7c29cff6685ddfcd774c66f
Reviewed-on: https://chromium-review.googlesource.com/c/1370162
Reviewed-by: Rune Lillesveen <futharkchromium.org>
Commit-Queue: Matt Falkenhagen <falkenchromium.org>
Cr-Commit-Position: refs/heads/master{#615475}

--

wpt-commits: 600dd6cb4295d0bcfc867b8877287d485d3b0e4e
wpt-pr: 14452

UltraBlame original commit: 004e8fbaeead9b556d1972586b1a17e7647b09e8
@karendolan
Copy link

karendolan commented Sep 15, 2020

Hi, is there a way to tell if Safari 13 is able to past the test written for this https://github.com/web-platform-tests/wpt/pull/14112/files?

@yutakahirano
Copy link
Member

ns-rsilva pushed a commit to ns-rsilva/chromium that referenced this issue Apr 25, 2024
This adds tests that stylesheets that result from requests that were
redirected cross-origin are considered cross-origin.

Note that A->B->A redirects, which redirect from cross-origin to
same-origin, are considered cross-origin. See
whatwg/fetch#737 and
whatwg/fetch#834.

In Blink, we have redirect tests at
http/tests/security/cannot-read-cssrules-redirect.html. This WPT
addition will supersede that test, but I won't yet remove it since
it asserts the opposite for the A->B->A case. I can remove the test
when Blink changes to pass this WPT test.

Bug: 911974
Change-Id: Ie015c0390829299de7c29cff6685ddfcd774c66f
Reviewed-on: https://chromium-review.googlesource.com/c/1370162
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615475}
Former-commit-id: 02d7d52f7396cb7a5e17c83e1da77fb4877047f7
ns-rsilva pushed a commit to ns-rsilva/chromium that referenced this issue Apr 25, 2024
Use ResourceResponse::ResponseUrl() to set the base URL, and use
ResponseResponse::GetType() to determine whether the resonse is
CORS-same-origin.

This CL has three web-exposed changes.

1. Use the response URL rather than the last request URL as the base URL
   of the stylesheet. This aligns with the standard. See
   whatwg/fetch#146 and WPT results indicate
   Firefox, Edge, and Safari use the response URL. This only matters if
   the response came from a service worker, as the URLs only differ
   when the service worker intercepts the request and responds with a
   different URL via respondWith(fetch(other_url)).

   This is covered by the WPT:
   service-workers/service-worker/fetch-request-css-base-url.https.html

   The test doesn't completely pass yet because the search query part of
   the URL gets chopped off for FetchEvent.request.referrer, but the base
   URL is correct.

   Chrome Status: https://www.chromestatus.com/feature/5642183499579392

2. Consider A->B->A redirects to be cross-origin rather than
   same-origin. Previously, this was considered same-origin. See the
   discussion in whatwg/fetch#737 and change
   whatwg/fetch#834.

   This change makes the following WPT test pass:
   css/cssom/stylesheet-same-origin.sub.html

   It also affects the web test:
   http/tests/security/cannot-read-cssrules-redirect.html

   This test is updated to match the behavior change. It can be removed
   later since it is redundant with the WPT test.

3. Consider load failures to be cross-origin rather than same-origin.
   That is, accessing |styleSheet.rules| throws a SecurityError if the
   load failed.  This aligns with the specification:
   - cssRules checks the `origin-clean` flag:
     https://drafts.csswg.org/cssom/#dom-cssstylesheet-cssrules
   - This is set to true iff CORS-same-origin:
     https://html.spec.whatwg.org/multipage/links.html#link-type-stylesheet
   - CORS-same-origin is false on kError:
     https://html.spec.whatwg.org/multipage/urls-and-fetching.html#cors-same-origin

   This change makes the following WPT test pass:
   css/cssom/stylesheet-same-origin.sub.html

   It also affects the web tests:
   register-bypassing-scheme-partial.html
   require-sri-for-style-blocked.php

   These tests are updated to match the behavior change.

Chrome Status: https://www.chromestatus.com/feature/5642183499579392
Intent to Ship: https://groups.google.com/a/chromium.org/d/msg/blink-dev/7OSy00oxVpk/siufiQVBBwAJ

Bug: 911974
Change-Id: I9add3162596963eee66f60f339cfd9911bc151cd
Reviewed-on: https://chromium-review.googlesource.com/c/1367331
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616580}
Former-commit-id: 8b30f526e1c760773511f48faa80c0347e49234c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
security/privacy There are security or privacy implications topic: html topic: redirects
Development

No branches or pull requests

7 participants