-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
nginx: add Content-Security-Policy-Report-Only header to all non-wordpress content sites #57
Conversation
I suggest adding Are inline scripts "unsafe" and a major problem for us? At least on static sites, I think inline scripts and styles tend to be idiomatic and don't pose much risk, assuming you do disallow connections of course. (And using nonce tokens is hard to reconcile with a static site.) For sites that want to allow arbitrary connections, I suppose disabling inline scripts can form an interesting compromise, but afaik we don't have such sites at the moment. WordPress requires and commonly uses inline Eliminating inline tags would make abuse much much harder, but also significantly limits what the web platform can do. For us that might not be worth trying to satisfy and workaround. What do you think? |
What kind of traffic does this accept? Is there a sample rate by default? Unfortunately, neither CSP nor Report-To header support a sampling rate by default. The NEL (Network Error Logging) spec, which uses the same reporting API, does have sampling. There is w3c/webappsec-csp#227 and w3c/reporting#45 for bringing this to the Reporting spec more generally, so that it can benefit CSP. The only option we have today is to sample in the policy itself. That is, to conditionally emit the header (or, when enforced, conditonally include the |
There's no sample rate, but logs are not written to a file by default. You have to live stream logs to view them, which means any logs when not live streaming are disregarded. If the logs are crazy when streaming, we can turn it off and address the most common ones and keep going from there until there are no logs and we've addressed all reports. At that point, we can shut down the API. That said, if you think it's necessary to build in sampling we can do that. Ideally, we get the header close enough that logs are few even without sampling. |
Perhaps it's best to focus on one site at a time, and build in sampling. |
@timmywil You could push this to staging only, to get early information without live traffic. That way we can get a glimpse of anything that's easily reproduced by us first. Once done testing, you can force push |
I figured out a way to test each site locally. I'll document what I find here. |
Will update as I find more. The CSP header for the API sites, and all wordpress sites, will not be handled in this PR, but instead set in jquery/jquery-wp-content#463
Notes
|
Rebased and removed the header for wordpress sites, which will be handled by jquery-wp-content |
@@ -51,6 +51,12 @@ server { | |||
include /etc/nginx/fastcgi_params; | |||
} | |||
<%- end -%> | |||
|
|||
location / { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Afaik location blocks are mutually exclusive, so this means this won't apply to .php
URLs.
It seems add_header
is valid in server blocks directly as well, if you want to do that for miscweb sites (i.e. above before the first location block). https://nginx.org/en/docs/http/ngx_http_headers_module.html
This is fine to me either way as an incremental start. I am making the assumption that this new catch-all location
block will inherit the index
and try_files
directives from the server block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of these three, only gruntjs has a staging site. So we can confirm overall catalogue validation and workings of the nginx config snippet there on stage, before prod if you like.
This PR adds the "Report-Only" version of the CSP header to all content sites, which has no real effect other than to send reports of violations to an API. I've set up a temporary API on cloudflare to accept those reports. We can watch the logs to determine if there are any issues that need to be addressed before switching to real CSP headers.
Fixes gh-54