[CSP] Add url-hash support to script-src [chromium/src : main]
Mustafa Emre Acer (Gerrit)
Mustafa Emre Acer added 1 comment![]( "Open in Gerrit")
Hi Camille and Carlos, could you PTAL at this as well? It's not fully ready, I'll need to fix the unit tests and add more of them, but if you could take a preliminary look that would be great. I split this mostly from crrev.com/c/6496953. Thanks!
Related details
- Camille Lamy
- Carlos IL
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Carlos IL (Gerrit)
Carlos IL added 2 comments![]( "Open in Gerrit")
Thanks! For the most part the approach LGTM mod using the separate flag (where I left the comment and also in content_security_policy.cc when parsing).
if (base::FeatureList::IsEnabled(network::features::kCSPScriptSrcV2) &&I'm cheating since I reviewed this after sending my CL out, but I think we should use the V1 flag here to keep them separate
Related details
- Camille Lamy
- Mustafa Emre Acer
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Camille Lamy (Gerrit)
Camille Lamy added 3 comments![]( "Open in Gerrit")
if (ParseURLHash(expression, url_hash.get())) {I think there is also a CSP parser inside blink, should it be updated?
frame.src = `support/iframe.html?pipe=header(Content-Security-Policy,${policy})`It would also be good to add tests for CSP in Meta Tags (similar to here). For a meta tag test, it would also be good to check that it cannot relax a CSP set in the header. This can be done in a follow up CL.
Related details
- Mustafa Emre Acer
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mustafa Emre Acer (Gerrit)
Mustafa Emre Acer added 4 comments![]( "Open in Gerrit")
PTAL? This doesn't have the hash reporting implemented for url hashes yet. I'll do it in a followup using crrev.com/c/6633943 as the starting point
if (ParseURLHash(expression, url_hash.get())) {I think there is also a CSP parser inside blink, should it be updated?
It seems blink calls into this via mojo: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/network/http_parsers.cc;l=1067;drc=ab5261de6f730d1378a27c400f8640440f662303
Searching for some of the keywords didn't bring up any other place: https://source.chromium.org/search?q=%22wasm-unsafe-eval%22%20-out%2F%20-web_tests%2F&ss=chromium&start=21
Also, the new WPTs work as-is, so I think this is the only policy parsing code.
if (base::FeatureList::IsEnabled(network::features::kCSPScriptSrcV2) &&I'm cheating since I reviewed this after sending my CL out, but I think we should use the V1 flag here to keep them separate
Done. I extracted the feature flag for now and there's some overlap with the VirtualTestSuites file, but I'll eventually rebase this on top of your CL.
frame.src = `support/iframe.html?pipe=header(Content-Security-Policy,${policy})`It would also be good to add tests for CSP in Meta Tags (similar to here). For a meta tag test, it would also be good to check that it cannot relax a CSP set in the header. This can be done in a follow up CL.
Done.
Modified script-url-allowed-by-hash.https to add the same set of tests using a meta tag.
Added url-hash-in-header-and-meta.https to check that we use the most strict policy.
Related details
- Camille Lamy
- Carlos IL
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |