Perform a CSP check when consuming preloaded response

[noamr]

Closes whatwg/html#7686

• At least two implementers are interested (and none opposed):
  • Implemented in WebKit & Chromiun
• Tests are written and can be reviewed and commented upon at:
  • Test preload behavior with dynamic CSP change web-platform-tests/wpt#33109
  • Test that consumption & preload of CSP-blocked resources are both reported web-platform-tests/wpt#33205
• Implementation bugs are filed:
  • Chrome: N/A
  • Firefox: 1759002
  • Safari: 238019

(See WHATWG Working Mode: Changes for more details.)

---

Preview | Diff

[annevk]

From the discussion in that HTML issue this doesn't seem like the correct fix as the other policies (e.g., Mixed Content blocking) do not end up being enforced.

[noamr]

From the discussion in that HTML issue this doesn't seem like the correct fix as the other policies (e.g., Mixed Content blocking) do not end up being enforced.

Unless I am missing something, the other policies are not modifiable after the document is created, so there is no need to re-check them. That's true at least for mixed content and was discussed in the HTML issue.

[annevk]

• It seems that "Run report Content Security Policy violations for request" wouldn't happen.
• Mike's scenario of scheme upgrades seems problematic. Yes, perhaps there's an HTTP cache entry, but is that guaranteed? And either way, wouldn't it result in a service worker being asked whereas if the scheme upgrade happened before that wouldn't be the case? (Scheme upgrades is also what Mixed Content Level 2 does.)

I haven't done a complete check, but this seems concerning and also suggests a potential mismatch with implementations that might cause further issues in the future.

[noamr]

• It seems that "Run report Content Security Policy violations for request" wouldn't happen.

Does that need to happen again? The request in question had already been processed.

But I agree that this is indeed concerning. Not so much for the preload case where we can define that all the checks happen a second time, but I'm more concerned about other memory cache scenarios that are not clearly specified.

[noamr]

• It seems that "Run report Content Security Policy violations for request" wouldn't happen.
• Mike's scenario of scheme upgrades seems problematic. Yes, perhaps there's an HTTP cache entry, but is that guaranteed? And either way, wouldn't it result in a service worker being asked whereas if the scheme upgrade happened before that wouldn't be the case? (Scheme upgrades is also what Mixed Content Level 2 does.)

I haven't done a complete check, but this seems concerning and also suggests a potential mismatch with implementations that might cause further issues in the future.

I refactored preload handling - where a preload scenario goes through the same checks as any other fetch, but uses the preloaded response instead of seeking it in network or cache in main fetch. This seems to be the safest, and allows us to incorporate different memory cache scenarios later by folding them into consume a preloaded resource. WDYT?

[annevk]

This looks really solid. This would result in reporting twice, right? Is that a case we might want to adjust to not do that or is it fine?

[noamr]

This looks really solid. This would result in reporting twice, right? Is that a case we might want to adjust to not do that or is it fine?

I think it's ok that it reports twice if both the preload and actual request are in violation but I will add a test to verify

[noamr]

This looks really solid. This would result in reporting twice, right? Is that a case we might want to adjust to not do that or is it fine?

I wrote a WPT for this - apparently Chrome reports twice and Firefox/Safari reports once. I wonder if it's necessary to report twice or we can leave this optional.

[annevk]

@noamr I guess we could make it conditional upon preloaded response candidate, but I think what Chrome does is reasonable. You might also get slightly different failures which could be interesting to know about. Let's file a bug against Safari? Firefox's behavior is likely due to it not enforcing policies at all.

I pushed a smallish fixup commit that also changes the variable name from req to request. I think that ended up being wrong in the original PR due to the text moving around. Could you confirm it's now correct?

[noamr]

@noamr I guess we could make it conditional upon preloaded response candidate, but I think what Chrome does is reasonable. You might also get slightly different failures which could be interesting to know about. Let's file a bug against Safari? Firefox's behavior is likely due to it not enforcing policies at all.

SG, posted a WebKit bug and we can merge the WPT alongside the PR AFAIC