Support Content Security Policy (csp)

Attaching patch matching current MR

The last submitted patch, 5: editoria11y-add_csp_support-3388093-5.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Alright...this code is straightforward, but I need convincing that controlling these settings in Editoria11y rather than CSP is a good idea. Having Editoria11y undo these settings dynamically is going to affect every node on the site for logged in users, meaning the de facto CSP policy would be dramatically different from what it looks like on the CSP settings page. Is that scope of an override better than just documenting that you shouldn't check "disable inline styles" in CSP?

Altering the Content Security Policy in each module is a pattern that other modules are following:

- #3137987: Support Content-Security-Policy →
- #3137978: Support Content-Security-Policy →
- #3097993: Integration with Content Security Policy →
- In google_tag module

Is it possible to load Editoria11y's styles from a stylesheet instead of inline? That would avoid the need for this CSP alteration altogether.

Is that scope of an override better than just documenting that you shouldn't check "disable inline styles" in CSP?

Only adding the unsafe-inline CSP directive to pages where the user has the ability to use Editoria11y is better than adding unsafe-inline to ALL pages when not needed, like for anonymous visitors.

Is it possible to load Editoria11y's styles from a stylesheet instead of inline? That would avoid the need for this CSP alteration altogether.

I'm afraid it is not feasible -- abstracting the fixed styles is easy, but there are quite a few things I'm calculating on the fly.

That said -- from what I'm reading, it looks like CSP restrictions can permit inline tags if they include a server-generated hash or nonce. Adding a that would not be difficult at all if the CSP module supports that approach.

If you were able to get me generated nonces in the .module file, I could handle modifying the library to accept it as a parameter and add it to all the injections!

Leveraging nonces instead would definitely be ideal. However, I don't believe the CSP module currently has an API to support it. See #3086924: Allow script / style by nonce →

Bother. I've spent some time on this and identified two ways I might be able to rewrite and not need inline styles, but both would require substantial rewrites and may hurt performance. It seems like nonces really are the "right" way to do this.

I'll test/merge your PR and document the issue until CSP adds nonce support.

@ChrisSnyder can you help me with https://git.drupalcode.org/issue/editoria11y-3388093/-/jobs/96695#L48 ? I pulled this to try to make the tests happier with the syntax, and I'm not succeeding.

Would it be possible to update the version of PHP you are testing with?

You are testing with an EOL version of PHP. PHP 7.4 is no longer supported by Drupal → . Constructor property promotion was not available until PHP 8.0.

Oh that makes sense. I can do that.

Alright...this code is straightforward, but I need convincing that controlling these settings in Editoria11y rather than CSP is a good idea. Having Editoria11y undo these settings dynamically is going to affect every node on the site for logged in users, meaning the de facto CSP policy would be dramatically different from what it looks like on the CSP settings page. Is that scope of an override better than just documenting that you shouldn't check "disable inline styles" in CSP?

The default config settings for the CSP module do not enable 'unsafe-inline', and the module only conditionally adds it for core libraries when required (e.g. for ckeditor and core/drupal.ajax in the earlier versions of core where it's still required). My philosophy is that it is better to only add a significant exception when necessary (and hopefully it will become increasingly less necessary), and modules should implement an event subscriber if they require any changes not captured by the default library asset parsing. It's a tradeoff that the effective policy for any page may vary but modules that implement should just work out of the box for site builders - being able to see the effective value in browser tools and any violation reports should make debugging straightforward.

it looks like CSP restrictions can permit inline tags if they include a server-generated nonce. Adding a that would not be difficult at all if the CSP module supports that approach.

Nonces present a bit of an integration challenge. Because applying a nonce (or hash) to a directive disables 'unsafe-inline', a module that wishes to use it must only add a nonce if the directive doesn't already include 'unsafe-inline' (without a nonce), so that functionality from other modules is not blocked. For now it might be enough to just lower the module event listener's priority so that other modules add their 'unsafe-inline' first if they need to. (this is what Attach Inline does, but it will currently probably have issues with another module also adding a nonce).
Nonces can also only be applied to elements (<link rel="stylesheet">, <script><code>, <code><style>), and not attributes (style="...", onclick="..."). (I would need to investigate if all browsers support 'unsafe-hashes' for attributes yet, but that can potentially make for a large policy header).

In the longer term, nonces would definitely be easier than hashes at my end.

It looks like inline style attribute manipulation by approved scripts is still permitted, so long as they do not manipulate the style tag as text. Am I reading that correctly? Moz Docs suggest that this would be permitted from an approved script:
document.querySelector("div").style.display = "none";

If so, I could eventually rewrite my necessarily-inline styles as a serious of querySelectorAll loops. Less performant, but so it goes.

For now -- I do want to maintain PHP 7.4 as an option for some of my legacy users, so I need to rewrite that function before merging.

It looks like inline style attribute manipulation by approved scripts is still permitted, so long as they do not manipulate the style tag as text. Am I reading that correctly?

Yes, you are reading that correctly. That is how I was able to resolve the CSP issue in 🐛 Avoid use of inline styles Fixed .

I updated the MR to remove the use of the PHP 8.0 feature.

Update patch to match MR.

Thank you! Will leave open until I tag a release.

Automatically closed - issue fixed for 2 weeks with no activity.