CKEditor is broken without 'unsafe-inlne'

I was led astray by comments in #2789139: [upstream] CSP requires 'unsafe-inline' because of CKEditor 4 and #2893566: Update CKEditor library to 4.7.1, and had thought the latest CKEditor 4 was compliant.

My original intention was to have a single policy across the site, in order to reduce the computation required for each page load and to allow HTTP/2's HPACK to be more effective.

I think CKEditor's reliance on 'unsafe-inline' will make it a requirement that each page have the opportunity to alter the policy, and then only apply 'unsafe-inline' to the pages that include the core/ckeditor library.

Having a per-page opportunity to alter the policy is a great idea, especially at this point in time where CSP is still relatively new, particularly in the Drupal space, and people are getting their heads around the idea that inline javascript is bad. We currently have a patch to your module that always adds 'unsafe-inline' to the header, which means all pages have it; this way we use your lovely generated headers and can claim to "have CSP" but everything still works. Of course we would prefer to limit the directive only to those pages that actually need it.

If I might make a suggestion -- perhaps rather than looking for the core/ckeditor library specifically, you could make this configurable so that the user can specify "bad" libraries or "bad" modules. There are a huge number of Drupal modules out there, and many of them will include inline js. Being able to apply 'unsafe-inline' only to pages which use one or more of a list of user-specified modules would provide a strong middle-ground and increase adoption of the CSP module. You could even maintain (or have a community-maintained) list of "known-bad" modules, which by naming and shaming might encourage individual module developers or development teams to update their modules so that they don't use inline javascript...

CSP will take the responsibility of adjusting any settings for core modules and libraries, but I would like to avoid having to keep up with changes in contrib modules.
Once an API is available for contrib modules to specify any additional changes they need, issues can be opened against those modules to integrate with the API.
CSP could also add a notice or warning to the status page that a module is requiring 'unsafe-inline'.

I've started a documentation page to keep track of popular modules that rely on inline code, link to any issues to remove inline scripts for those modules, and link to alternative modules that don't use inline scripts.

Maybe it would be helpful to have another documentation page summarizing the support status of the top contrib modules?

The documentation idea is good, and I see you've started it. If I get some time I'll try and work out which of the modules we're using require the 'unsafe-inline' and add to it.

On a related note -- would you be willing to accept a patch to have optional settings to always force 'unsafe-inline'? We're currently patching the module as follows (ignore that we're setting enforce and the force-unsafe-inline options to true by default):

diff -Naur modules/csp.orig/config/install/csp.settings.yml modules/csp/config/install/csp.settings.yml
--- modules/csp/config/install/csp.settings.yml	2018-02-07 06:17:42.000000000 +0000
+++ modules/csp/config/install/csp.settings.yml	2018-02-12 10:57:01.000000000 +0000
@@ -1,3 +1,5 @@
-enforce: false
+enforce: true
 report:
   handler: csp-module
+force-script-unsafe-inline: true
+force-style-unsafe-inline: true
\ No newline at end of file
diff -Naur modules/csp.orig/src/EventSubscriber/ResponseCspSubscriber.php modules/csp/src/EventSubscriber/ResponseCspSubscriber.php
--- modules/csp/src/EventSubscriber/ResponseCspSubscriber.php	2018-02-07 06:17:42.000000000 +0000
+++ modules/csp/src/EventSubscriber/ResponseCspSubscriber.php	2018-02-12 10:56:16.000000000 +0000
@@ -125,6 +125,9 @@
       $policy->appendDirective('style-src', $styleHosts);
     }
 
+    if($cspConfig->get('force-script-unsafe-inline')) $policy->appendDirective('script-src', [Csp::POLICY_UNSAFE_INLINE]);
+    if($cspConfig->get('force-style-unsafe-inline')) $policy->appendDirective('style-src', [Csp::POLICY_UNSAFE_INLINE]);
+
     // Prior to Drupal 8.6, in order to support IE9, CssCollectionRenderer
     // outputs more than 31 stylesheets as inline @import statements.
     // @see https://www.drupal.org/node/2897408
diff -Naur modules/csp.orig/src/Form/CspSettingsForm.php modules/csp/src/Form/CspSettingsForm.php
--- modules/csp/src/Form/CspSettingsForm.php	2018-02-07 06:17:42.000000000 +0000
+++ modules/csp/src/Form/CspSettingsForm.php	2018-02-12 10:56:36.000000000 +0000
@@ -105,6 +105,20 @@
       ],
     ];
 
+    $form['force-script-unsafe-inline'] = [
+      '#type' => 'checkbox',
+      '#title' => $this->t('Force-script-unsafe-inline'),
+      '#description' => $this->t('Force headers to always include \'unsafe-inline\' in the \'script-src\' section.  This is needed if you are using legacy inline javascript in any of your Drupal code or modules.'),
+      '#default_value' => $config->get('force-script-unsafe-inline'),
+    ];
+
+    $form['force-style-unsafe-inline'] = [
+      '#type' => 'checkbox',
+      '#title' => $this->t('Force-style-unsafe-inline'),
+      '#description' => $this->t('Force headers to always include \'unsafe-inline\' in the \'style-src\' section.  This is needed if you are using legacy inline styles in any of your Drupal code or modules.'),
+      '#default_value' => $config->get('force-style-unsafe-inline'),
+    ];
+
     return parent::buildForm($form, $form_state);
   }
 
@@ -137,6 +151,9 @@
     $config = $this->config('csp.settings')
       ->set('enforce', $form_state->getValue('enforce'));
 
+    $config->set('force-script-unsafe-inline', $form_state->getValue('force-script-unsafe-inline'));
+    $config->set('force-style-unsafe-inline', $form_state->getValue('force-style-unsafe-inline'));
+
     $reportHandler = $form_state->getValue(['report', 'handler']);
     $config->set('report.handler', $reportHandler);
     if ($reportHandler == 'report-uri-com') {

Adding these options (off by default for the module) would allow people to add in the override until all of their modules are fixed...

#2895243: Configuration for manual policy additions will allow adding the unsafe flags or additional domains that are not automatically included in the policy.

My approach to this issue will be:

1) Always enable 'unsafe-inline' for now, due to core/ckeditor
2) After #2895243: Configuration for manual policy additions is complete, 'unsafe-inline' will be set as a default in configuration but can be disabled.
3) #2895245: API for modules to alter policy will allow csp.module to flag the individual core libraries which require 'unsafe-inline'
4) #2943432: Only apply 'unsafe' flags when dependent libraries are included on page will only add unsafe flags when a library that requires it is included on the page, and the configuration defaults can remove 'unsafe-inline'

I've committed the first change to always enable script-src: 'unsafe-inline' for now since it breaks any buttons in the ckeditor interface. My understanding is that ckeditor can work without style-src: 'unsafe-inline'.

#2895243: Configuration for manual policy additions will put the control of both properties into configuration.

That's brilliant, thanks. Is there any chance that the first unsafe-inline commit for ckeditor (https://www.drupal.org/commitlog/commit/93322/c397cfb3e47bda16ca01384c20...) might make it into the beta build? We'd rather not use the dev build, but that commit does solve the issue for us for now (and I love your other changes longer term).

You're correct by the way that ckeditor is fine without style-src: 'unsafe-inline', we put that in because of something else (I think it might have been Modernzr).

I originally didn't think there was enough changes to justify a release, but in combination with all the refactoring and unit test updates that were done I think it does make sense now.

I've created the 8.x-1.0-beta2 release, and it should be ready shortly.

Excellent, thank you.