EB-134: Add a Content Security Policy to the website #81
Conversation
Working towards a Content Security Policy (CSP) compliant website.
Work towards CSP compliance. Params for tables are now passed in the html with "data-" prefixed attributes.
Work towards CSP compliance.
Work towards CSP compliance.
- Disable JBrowse's use of google analytics for each species. Need to allow unsafe-inline to let JBrowse analytics work (i.e. can't just whitelist google analytics in the CSP). - Don't inline JS to check for config on genome browser page.
There was a problem hiding this comment.
Thanks for this thorough work. It was interesting to learn about CSP using the linked resources.
How did the idea to enforce a CSP come from originally ? I am divided: while the extraction of inline code is a valuable refactoring in itself, I wonder if maintaining a list of directives in nginx,conf is worth it. Quoting OWASP:
Even on a fully static website, which does not accept any user input, a CSP can be used to enforce the use of Subresource Integrity (SRI). This can help prevent malicious code from being loaded on the website if one of the third-party sites hosting JavaScript files (such as analytics scripts) is compromised.
It seems to imply that the main benefit of CSP for a static site like ours would be to check the integrity of scripts, rather than mitigating unlikely XSS threat by restricting subresource origins. But we actually omit the former.
I don't strongly object merging this though, but some documentation snippet should perhaps be included to avoid some head scratching the day one of us includes a new third-party script, everything working well locally, buy breaks when deployed because of a stale nginx.conf 😅
| style-src 'self' 'unsafe-inline' https://fonts.googleapis.com https://cdn.jsdelivr.net https://cdn.datatables.net https://unpkg.com; | ||
| font-src 'self' https://fonts.gstatic.com https://cdn.jsdelivr.net; | ||
| img-src 'self' data: https://tile.gbif.org https://api.gbif.org; | ||
| connect-src 'self' http://matomo.dc.scilifelab.se/ https://raw.githubusercontent.com https://analytics.jbrowse.org; |
There was a problem hiding this comment.
Is there a reason you opted for the trailing / syntax here, and the http scheme 🤔
| /* | ||
| Google reCAPTCHA v2 callback | ||
| */ | ||
| var onloadCallback = function () { |
There was a problem hiding this comment.
Why not opt for document.addEventListener here as in init_datatables.js (for example) ? I was puzzled, by what magic is the event listener registered ? Is it a convention used by Google reCAPTCHA ?
| @@ -8,9 +8,16 @@ | |||
|
|
|||
| {{ define "script_includes" }} | |||
|
|
|||
| <!-- Load the Google reCAPTCHA API asynchronously and defer its execution until the page has finished parsing --> | |||
| <!-- onload callback function needs to be loaded before reCAPTCHA API to avoid race conditions. --> | |||
There was a problem hiding this comment.
This comment is a bit cryptic to me, perhaps related to my comment above regarding onLoadCallback
| g.async = true; g.src = u + 'matomo.js'; s.parentNode.insertBefore(g, s); | ||
| })(); | ||
| </script> | ||
| {{ $matomoJS := resources.Get "js/matomo.js" | fingerprint | minify }} |
There was a problem hiding this comment.
A Hugo idiom I like for all these cases:
{{ with resources.Get "js/matomo.js" | fingerprint | minify }}
<script src="{{ .RelPermalink }}"></script>
{{ else }}
{/* Error handling */}
{{ end }}
This PR is about adding a content security policy to the website.
This essentially involves removing inline JS and creating a whitelist inside the
docker/nginx-custom.conffor all the external connections we expect to make.The genome browser page posed some extra challenges when creating this:
'unsafe-inline'from thescript-srcline required updating each speciesconfig.jsonto disable google analytics. Just whitelisting google analytics webpages did not work.'unsafe-inline'fromstyle-srcwas unsuccessful because JBrowse relies on it. I think it is okay to leave'unsafe-inline'instyle-srcand we can be happy we got the rest.@brinkdp and I already spoke and he is working on including the disable google analytics param in the
config.jsonfor any future species. (If the param is not included the browser page still works as expected, you just see some errors if you open up the console).