1699373 - COEP/CORP image load fails on reload

Status: RESOLVED FIXED

(Core :: Networking: Cache, defect, P1)

Description

[Hanno Boeck]

Attachment: Screencast video showing the behavior

I discovered an odd behavior in combination with the Cross-Origin-Embedder-Policy / Cross-Origin-Resource-Policy headers.

I have a testcase setup at
https://d.q2.re/
(I will try to leave that online for the time this bug is analyzed)

The host sends "Cross-Origin-Embedder-Policy: require-corp", which would mean it can only load external images from other hosts if they explicitly allow this via CORP.
The first image does not send any special headers, so as expected the load is blocked. The second image sends "Cross-Origin-Resource-Policy: cross-origin", so the image load should work.

It does work when one accesses the page initially. However after a click on reload the image disappears. I have recorded this in a video which I will attach.

My guess is that the CORP property of the image is somehow not preserved when the image is loaded from cache (as you can see in the network tab the image request gets a 304 not modified and the image should be fetched from the cache).

Comment 1

[Frederik Braun [:freddy]]

When reloading we serve headers from our cache and the cache does not seem to store the CORP header (at least it doesn't send them). Confirmed when inspecting headers in devtools network tab.

Comment 2

[Anne (:annevk)]

Thanks Hanno!

Valentin, is this something you can tackle?

Comment 3

[Frederik Braun [:freddy]]

Uh, this needs a priority bump. It's preventing people from shipping cross-origin isolation features.
We can't tell people to use security features to protect against real attacks, when they are dysfunctional :/

Valentin? Jens?

Comment 4

[Valentin Gosu [:valentin] (he/him)]

Thanks for the poke. Sorry I missed the ni for so long.
I assume the issue is somehow caused by the cached resource missing the CORP header.
From what I can tell the 304 resource doesn't even have a contentType (judging by what I see in the video)

Comment 5

[Valentin Gosu [:valentin] (he/him)]

Hanno, the test case in comment 0 doesn't work anymore.
Do you still have it available?

Comment 6

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1699373 - Add WPT for COEP/CORP image reloading r=annevk

Comment 7

[Hanno Boeck]

I have restored the testcase, it should work again. Can also confirm that the same behavior shows up in firefox 92.

Comment 8

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1699373 - Call ProcessCrossOrigin*Header methods after loading cached headers r=#necko

Previously we called ProcessCrossOriginEmbedderPolicy in
nsHttpChannel::ContinueProcessResponse1, but we only loaded the cached
response headers in ContinueProcessResponse3, meaning that we incorrectly
reported a missing header for the revalidated resource.

This change moves the header checking calls to ContinueProcessNormal and
ContinueProcessRedirection instead, so they get executed after processing
the cached headers.

Depends on D125129

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/218ce41d771c
Add WPT for COEP/CORP image reloading r=annevk
https://hg.mozilla.org/integration/autoland/rev/c1e0fff3e513
Call ProcessCrossOrigin*Header methods after loading cached headers r=necko-reviewers,dragana

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/218ce41d771c
https://hg.mozilla.org/mozilla-central/rev/c1e0fff3e513

Comment 11

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/30881 for changes under testing/web-platform/tests

Comment 12

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 13

[Mike Kaply [:mkaply]]

Apparently this fixed regressions in the ESR. See bug 1749009.

Can this be easily uplifted to the ESR?

Comment 14

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 9240513 [details]
Bug 1699373 - Call ProcessCrossOrigin*Header methods after loading cached headers r=#necko

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Fixes issue from bug 1749009:

recaptcha doesn't work any more with firefox 91 esr via sso squid proxy

• User impact if declined: Some pages will not work properly on ESR
• Fix Landed on Version: 94
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The fix has been on release channel for a couple releases with no regressions.

Comment 15

[Pascal Chevrel:pascalc (PTO until April 26)]

https://hg.mozilla.org/releases/mozilla-esr91/rev/6f296ebeda37
https://hg.mozilla.org/releases/mozilla-esr91/rev/0cb2ab8a5857

Comment 16

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9240513 [details]
Bug 1699373 - Call ProcessCrossOrigin*Header methods after loading cached headers r=#necko

Approved for ESR