Generalize CSP headers #2248
Generalize CSP headers #2248
Conversation
nmattia
force-pushed
the
nm-integrity-3
branch
from
b5fa135 to
0481f6d
Compare
This updates the way we create the CSP headers (and in particular, the `script-src` clause). Before this, and due to a (now resolved) bug in Firefox, we could not specify `integrity=` attributes for whitelisting scripts in HTML. Instead we implemented a work around where we injected a new inline script (with known hash) which in turn loaded the actual JS script. This was unconventional, somewhat unsafe (because it didn't actually include the script's hash) and brittle (we hard coded some script names, like `index.js`, `index2.js`, etc). This updates the frontend bundle to include `integrity=` attributes in HTML script tags, and updates CSP header creation to extract those integrity hashes and whitelist them in the generated asset list. The CSP header creation is also changed to keep a reference to the security headers certified alongside the assets. This ensures the security headers do not need to be recomputed with every asset, and clarifies that assets were certified with specific headers. The CSP header error handling in the test suite is updated for better error reporting. Firefox references: * https://bugzilla.mozilla.org/show_bug.cgi?id=1409200 * https://www.mozilla.org/en-US/firefox/116.0a1/releasenotes/
nmattia
force-pushed
the
nm-integrity-3
branch
from
03899ad to
4f21fad
Compare
| fn certified_asset(asset_name: &str, certificate_version: Option<u16>) -> Option<CertifiedAsset> { | ||
| /// Read an asset from memory, returning the associated HTTP code, content and full list of | ||
| /// headers that were certified with the asset. | ||
| fn get_asset( |
There was a problem hiding this comment.
Given that the CertifiedAssets struct holds the shared headers already, it would IMHO be nicer if the get_certified_asset function would include the shared headers directly.
There was a problem hiding this comment.
I thought about that at first and then found it confusing that sometimes the CertifiedAsset includes the shared headers, sometimes not. WDYT?
There was a problem hiding this comment.
Simplest would be to just remove the concept of shared_headers altogether and just duplicate them among all assets.
I don't think we are resource constrained enough to justify the complexity of deduplication. But that should probably go into a different PR.
There was a problem hiding this comment.
Fyi @frederikrothenberger : Nicolas and I had a discussion about shared_headers on slack last week, and I thought it is nice not to have them duplicated, hence the changes in this PR. But I don't feel strongly about it.
There was a problem hiding this comment.
I don't think we are resource constrained enough to justify the complexity of deduplication.
I agree
| fn certified_asset(asset_name: &str, certificate_version: Option<u16>) -> Option<CertifiedAsset> { | ||
| /// Read an asset from memory, returning the associated HTTP code, content and full list of | ||
| /// headers that were certified with the asset. | ||
| fn get_asset( |
There was a problem hiding this comment.
Fyi @frederikrothenberger : Nicolas and I had a discussion about shared_headers on slack last week, and I thought it is nice not to have them duplicated, hence the changes in this PR. But I don't feel strongly about it.
This updates the way we create the CSP headers (and in particular, the
script-srcclause).Before this, and due to a (now resolved) bug in Firefox, we could not specify
integrity=attributes for whitelisting scripts in HTML. Instead we implemented a work around where we injected a new inline script (with known hash) which in turn loaded the actual JS script.This was unconventional, somewhat unsafe (because it didn't actually include the script's hash) and brittle (we hard coded some script names, like
index.js,index2.js, etc).This updates the frontend bundle to include
integrity=attributes in HTML script tags, and updates CSP header creation to extract those integrity hashes and whitelist them in the generated asset list.The CSP header creation is also changed to keep a reference to the security headers certified alongside the assets. This ensures the security headers do not need to be recomputed with every asset, and clarifies that assets were certified with specific headers.
The CSP header error handling in the test suite is updated for better error reporting.
Firefox references:
🟡 Some screens were changed