[WIP] Improving Content-Security-Policy Interaction

[ScottHamper]

Hey All,

This is related to Issue #385. One of the big benefits (among others) of the CSP HTTP response header is that it can significantly mitigate the risk/impact of XSS vulnerabilities on a website.

Through the CSP header, the web server makes a bunch of promises to the browser about where it will load specific types of content from. For example, by promising that there will never be any JavaScript that is inline in the HTML (e.g., <script>alert('inline JS XSS');</script>), the browser knows that any inline JS that is present is unintentional/risky/injected and can be blocked without breaking the normal operation of the website. In other words, CSP allows website owners to specify a white list for where they get resources from.

Currently, the admin plugin has a couple issues that prevent it from integrating well with the CSP header:

• URLs for Google Fonts use the protocol-relative syntax (//fonts.googleapis.com), which means that a CSP that only allows assets loaded over HTTPS cannot be applied to a Grav site running over HTTP. This provides a pain point when developing on localhost, as I want to have the same CSP applied locally as I do in production. Otherwise, it's necessary to manage two separate .htaccess files just for a single HTTP header difference - one which allows http://fonts... and one which allows https://fonts.... Realistically, there's no downside to always loading these resources over HTTPS.
• There's multiple instances of inline JavaScript in HTML, which defeats some of the power of the CSP header (the CSP has to allow all inline JS to be executed, malicious or otherwise).

So far this pull request takes care of the first issue above. However, the second issue is more involved to resolve. It looks like some of the inline scripts use Twig features to compose dynamic JS, so they can't simply be pulled into external JS files. For example, /themes/grav/templates/partials/javascript-config.html.twig does this.

Two possible ways I can think of to resolve this would be:

• Create a static JS file that pulls dynamic info using AJAX.
• Have PHP dynamically produce a JS file when a specific URL is requested.

Is this something someone on the core team would be interested in taking on?

For completeness, this is the CSP header I'm looking to apply on the website I'm developing (at least for the start - the following is only intended to allow a default Grav installation to run without triggering browser errors):

Content-Security-Policy: default-src 'none'; font-src 'self' https://fonts.gstatic.com; img-src 'self' https://www.gravatar.com; object-src 'none'; script-src 'self'; style-src 'self' https://fonts.googleapis.com

[ScottHamper]

I have also discovered that the inline SVG elements in /themes/grav/templates/partials/logo.html.twig for Grav's logo counts as inline styles and trigger CSP unless 'unsafe-inline' is allowed. These graphics should be moved out into their own external SVG files and included on the page using an <img> element. It's worth noting that even in this situation, Firefox currently has a bug where it will still apply the CSP to the SVG even though it shouldn't, but this should not be the concern of the Grav Admin plugin.

Here's an updated "ideal" CSP that I'd like to apply for a default Grav install which disallows inline styles/scripts (now that I've worked with it more):

Content-Security-Policy: default-src 'none'; font-src 'self' https://fonts.gstatic.com; img-src 'self' https://www.gravatar.com; object-src 'none'; script-src 'self'; style-src 'self' https://fonts.googleapis.com; connect-src 'self' https://getgrav.org

[rhukster]

Regarding inline SVG, the benefit is you can directly manipulate them via CSS so you can change the 'fill' color etc. This is supremely useful and we use this a lot. Moving to an external SVG file would mean you are treating the SVG as a separate request and can no longer manipulate the SVG via CSS. This is not an acceptable solution in my book.

[ScottHamper]

Hey @rhukster ,

I've messed around with SVGs a little bit and have found an alternative resolution that could work well! It turns out that having inline SVGs isn't itself the problem, but having style attributes on the SVG elements/children is. So it's possible to have an inline SVG and to apply custom fill colors, etc, via an external CSS file. This satisfies the current use case for SVGs in the plugin while still allowing for a CSP that denies inline styles.

See the attached files for a simplified proof-of-concept:
svg-csp-poc.zip

EDIT: A bonus of this is that it doesn't run up against the Firefox bug I mentioned in my last comment, so that's cool too.

EDIT2: Let me know if I should go ahead and implement this solution as part of the PR - I'm happy to do that work. The only work I don't feel comfortable doing is resolving the inline JS issues, as I'm not very familiar with PHP and the code base. However, I could give it a shot with some guidance on where to start and what approach to take.

[rhukster]

Sure make it part of your PR and i'll review. Cheers!

[ScottHamper]

Alright, after further investigation it seems like removing all inline CSS is more work than I care to take on. There's just too many places:

• /themes/grav/app/updates/index.js - Line 52
• /themes/grav/templates/forms/fields/colorpicker/colorpicker.html.twig - Line 33
• /themes/grav/templates/forms/fields/list/list.html.twig - Lines 115, 154
• /themes/grav/templates/pages.html.twig - Lines 46, 103

And then there's more in the other dependency plugins.

Regardless, I'd be happy if you still merged this request to take care of the protocol-relative URL for Google Fonts. In addition to CSP woes, Google enforces a redirect from HTTP -> HTTPS, so the protocol-relative URL only gets in the way.

[justindanger]

Any progress on this at all? I won't be able to use Grav for my next project if I can't figure out a way to use the admin plugin while denying unsafe-inline in the CSP. I would be ok with allowing it only for the admin plugin but can't figure out how to implement that due to my unfamiliarity with the subject.

[bricebou]

Hi,

Any progress on that subject ? Accessing the Admin panel require to add unsafe-inline in the CSP for scripts...

Any idea ?

[ScottHamper]

@rhukster : CSP issues have gotten even worse sometime between v1.9.19 and 1.10.12 - it looks like webpack is now including eval calls into the admin pages, requiring any CSP to permit 'unsafe-eval' for the entire domain. I don't know much about webpack, but it sounds like it uses eval for development builds, and other approaches for production builds. Should Grav Admin releases be using a production build configuration for webpack?

[mahagr]

CC @w00fz