Improve CSP handling for google analytics script #531
Assignees
Comments
aomarks
added a commit
that referenced
this issue
Sep 30, 2021
- For all requests other than our HTML entrypoints and the playground worker script, serve a super strict policy, just in case a response that shouldn't normally allow any code execution somehow actually does. Based on [this](w3c/webappsec#520 (comment)) and [this](webhintio/hint#3403 (comment)) comment. - Add some directives that I found through https://csp-evaluator.withgoogle.com/ and [this comment](w3c/webappsec#520 (comment)). - Temporary fix for inline Google Analytics script which was being reported as a violation. See #531 for details. Will fix properly in followup.
Merged
aomarks
added a commit
that referenced
this issue
Oct 1, 2021
- Handle 304 cases. The reason we were getting a lot of CSP errors since #532 was that we weren't accounting for 304 Not Modified responses. In those cases, no `Content-Type` header is set, because the browser will already know it from the most recent 200 response. Our middleware incorrectly assumed it could check `type` after awaiting the downstream Koa static middleware. We now instead set the policy based on the path of the request (anything ending in `/`), since that will work for both 200 and 304 responses. There's an interesting question about whether we *should* set a CSP at all for a 304 response. There's some discussion about that [here](w3c/webappsec-csp#161) and [here](https://bugs.chromium.org/p/chromium/issues/detail?id=174301). Chrome *does* update the CSP if it's included in a 304 response, otherwise it ignores it in favor of the CSP from the last 200 response (same with all headers, except for the ones enumerated [here](https://chromium.googlesource.com/chromium/src/net/+/refs/heads/main/http/http_response_headers.cc#81)). The advantage of including it seems to be that if we update the CSP, but the content (and hence ETag) of a file has *not* changed, then the browser would otherwise continue using the stale CSP. OTOH, if you are using a nonce based script allowlist (we use hashes instead currently), then this wouldn't really work unless you did something clever to make the ETag aware of some CSP version. - Fix policy for Google Analytics. We covered the origin needed for the initial script, but then *that* script goes on to require a connection to a different origin. - Hook up a new Google Analytics test ID that I just created, so that we will catch CSP reports related to Google Analytics during dev and in PR builds going forward. Part of #517 Part of #531
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
We currently have a hard-coded sha256 hash to allow the inline google analytics script to run.
If we ever updated that script, or updated our Google Analytics ID, this could easily get out of sync.
We should automate generation of this hash just like we do for other inline scripts. The reason we can't do this with the exact same method is that the Google Analytics ID needs to be baked into the source.
It would also be a good idea to register another Google Analytics account to use for dev and testing, because the main reason I missed this in my initial set of CSP PRs was that the Google Analytics script is only present at all in the main production build. (The reason for registering another account would be to avoid polluting our main account with dev/testing page loads).
The text was updated successfully, but these errors were encountered: