Motivations to this task:
- Problem with CKEditor - incompatible with CSP: https://www.drupal.org/project/seckit/issues/2604236
- "because you can't even edit views settings when CSP is enabled. (and unsafe-eval is disabled)" - https://www.drupal.org/project/seckit/issues/2844205

The proposal is to be able to add path-based configuration override to the SecKit config.

Let's say you have a CKEditor on node add / edit pages that's only accessible for admins.
You have only one choice right now: allow unsafe-* things for everyone in CSP.
With this proposal, you'd be able to add a config override for:
node/add/*
and
node/*/edit

And activate the `unsafe-*` directives only there, which limits the possible source of attacks.

The proposed configuration form change:
proposed UI

CommentFileSizeAuthor
#7 3064813-seckit-config-override-7.patch46.77 KBaron novak
#6 3064813-seckit-config-override-6.patch46.13 KBaron novak
#5 3064813-seckit-config-override-5.patch45.7 KBaron novak
#4 3064813-seckit-config-override-4.patch45.91 KBaron novak
#3 3064813-seckit-config-override-3.patch17.79 KBaron novak
#2 3064813-seckit-config-override-1.patch4.8 KBaron novak
Screenshot_20190628_152359.png49.23 KBaron novak

Comments

Aron Novak created an issue. See original summary.

aron novak’s picture

aron novak
Primary language Hungarian
Location Hungary, Budapest
commented
StatusFileSize
new 3064813-seckit-config-override-1.patch4.8 KB

The patch is just the start.
Still need to:
- handle configuration saving / loading
- properly delete the selected override
- use the proper override in web/modules/contrib/seckit/src/EventSubscriber/SecKitEventSubscriber.php

aron novak’s picture

aron novak
Primary language Hungarian
Location Hungary, Budapest
commented
StatusFileSize
new 3064813-seckit-config-override-3.patch17.79 KB

The config storage part is there, the form is partially converted to be able to use default values either from the base config or from the override.

aron novak’s picture

aron novak
Primary language Hungarian
Location Hungary, Budapest
commented
StatusFileSize
new 3064813-seckit-config-override-4.patch45.91 KB

This version has the proposed way of the logic, it still needs testing and finalization.

aron novak’s picture

aron novak
Primary language Hungarian
Location Hungary, Budapest
commented
StatusFileSize
new 3064813-seckit-config-override-5.patch45.7 KB

Now the patch can be used to maintain configuration overrides, it started to work, however the patch still needs some work before it's ready for review.

aron novak’s picture

aron novak
Primary language Hungarian
Location Hungary, Budapest
commented
StatusFileSize
new 3064813-seckit-config-override-6.patch46.13 KB

Now it has a bit better CRUD for the overrides itself.

aron novak’s picture

aron novak
Primary language Hungarian
Location Hungary, Budapest
commented
StatusFileSize
new 3064813-seckit-config-override-7.patch46.77 KB

The form CRUD is not perfect, but now it has flexible pattern handling.

dagmar’s picture

dagmar
Primary language Spanish
Location Argentina
commented 
+++ b/src/Form/SecKitSettingsForm.php
@@ -721,13 +728,92 @@ class SecKitSettingsForm extends ConfigFormBase {
+          '#type' => 'textfield',

This textfield is too short for production purposes. What about change this into a textarea?

aron novak’s picture

aron novak
Primary language Hungarian
Location Hungary, Budapest
commented

@dagmar : Good point, the patch generally needs work, indeed the UX would be much better with a textarea, ofter we need several paths listed for the same set of configuration.

mxr576’s picture

mxr576
Primary language Hungarian
Location Hungary
commented

Any update on this? It seems we can either use add CSP - which is a generic security requirement nowadays - or use Ckeditor.

mxr576’s picture

mxr576
Primary language Hungarian
Location Hungary
commented
Related issues: +#3100151: Adding JS/CSS assets in AJAX responses requires 'unsafe-inline' Content Security Policy

It looks like the problem with CSP is not just about Ckeditor and Views UI.

gapple’s picture

gapple
he/they
Primary language English
commented

The Content-Security-Policy module (https://www.drupal.org/project/csp) has the ability to apply policy alterations per-request via an event subscriber, and includes handlers to apply 'unsafe-inline' as needed when individual core libraries (ckeditor, ajax) are added to the page.

lunk rat’s picture

lunk rat commented

#12 sounds nice, but the CSP module is not available for D7.

A feature in SecKit to allow override of CSP per-path (or even better would be conditionally when certain libraries are loaded) would still be a welcome feature on SecKit, especially if it gets backported to D7.

queenvictoria’s picture

queenvictoria commented

FYI There is now a D8/9 version of the CSP module.
https://www.drupal.org/project/csp