Implement a "semi automatic" Nonce settings

This works in my testing.

It adds a new pre render callback to any #type => html_tag elements that are of #tag script.

Turn it on with a checkbox.

Uses a lazy builder so its render cache safe

Adds an update hook and test coverage as well

Please credit @dpi who came up with the element_info_alter idea

mcdruid → credited dpi → .

Per chat with @larowlan this looks good to me at glance, but will ask @Rajeshreeputra for a proper review as they've volunteered to help maintain seckit ( 💬 Offering to maintain Security Kit Active ).

Thanks, good to see this can be automated for third parties that participate in attaching scripts consistently with html_tag.

Sorry about the unassign, must be a stale form^

Patch applied cleanly:

On existing site and ran database update I can see option to enable Nounce under CSP.

Enabled the nounce and verified same it work as expected.

Overall changes looks good, the only weird thing noticed is:

+
/**
+ * Implements hook_form_FORM_ID_alter().
+ */
+function seckit_test_form_seckit_settings_form_alter(array &$form, FormStateInterface $form_state, string $form_id): void {
+  $form['test_script'] = [
+    '#type' => 'html_tag',
+    '#weight' => 50,
+    '#tag' => 'script',
+    '#attributes' => ['id' => 'testScript'],
+    '#value' => Markup::create('console.log("👋");'),
+  ];
+}

'#value' => Markup::create('console.log("👋");'), - this special characters in console log.

Special characters look like a good sanity test to make sure things aren't encoded oddly / hashed correctly.

Yep, that's just test code, only used in the test
Happy to change it to any other inline JS

Adding a nonce to every inline script html_tag element gives me some concern, compared to requiring an attribute on the render element to specifically opt-in to having a nonce applied. I'm open to "it should be reasonable to assume that any content is sanitized / restricted from editing by non-privileged users / etc before being added to an html_tag element (and user's shouldn't be able to arbitrarily insert new html_tag elements)", but I'm wary of this feature being used to make warnings go away without exploring ways to address the root cause, and opening up unintended leniency in a site's policy.

As more of a user knowledge or integration issue, adding a nonce to the policy will disable 'unsafe-inline' and block any other scripts that rely on it.
While 9.5+ / 10.0+ no longer require 'unsafe-inline' for core/drupal.ajax to function, if any inline scripts are added to the page through a mechanism other than a html_tag element (e.g. directly in a theme template, or with a raw markup element), they would be blocked by the policy if a nonce is included.
Any site continuing to use CKEditor 4 (from core in 9.x or contrib in 10.x) will break if this feature is used since CKE4 relies on inline attributes (which can't have a nonce applied).

@larowlan's patch works nicely for inline scripts, but to be able to use the 'strict-dynamic' directive, the nonce needs to be added to every script tag, not just inline JS. At some point I might take a crack at tweaking the patch to provide configuration for linked scripts as well, so we can harden our CSP config a bit further. That said, I'm probably missing something that's already out there that would make this work...

This seems like it's a pretty decent implementation of the feature.

Whether sites will actually want to use it is a somewhat different question; as with many aspects of CSP (and seckit's other features) it's hard to determine exactly what sane defaults should be; turning everything on will almost certainly break things.

Seems like a useful addition to the toolkit that the module provides though.

Looks like 3x tests are failing for D9:

• Drupal\Tests\seckit\Functional\SecKitTestCaseTest::testCspWithoutVendorPrefixes
• Drupal\Tests\seckit\Functional\SecKitTestCaseTest::testCspWithCspVendorPrefix
• Drupal\Tests\seckit\Functional\SecKitTestCaseTest::testCspWithWebkitCspVendorPrefix

In each case, the new nonce is being added to the CSP but that's not what the assertion expects - e.g. the test got the following CSP header:

"default-src *; script-src * 'nonce--8mf3Awmh4ut1257OFh-xWL-yGx8g65B'; object-src *; style-src *; img-src *; media-src *; frame-src *; frame-ancestors *; child-src *; font-src *; connect-src *; report-uri /subdirectory/report-csp-violation; upgrade-insecure-requests"

...but expected:

"default-src *; script-src *; object-src *; style-src *; img-src *; media-src *; frame-src *; frame-ancestors *; child-src *; font-src *; connect-src *; report-uri /subdirectory/report-csp-violation; upgrade-insecure-requests" 

So...

1) Do we expect the nonce to appear like this within the header?

2) If so, can we adjust these tests accordingly?

FWIW it looks like a fluke that the nonce began with a dash in the example above resulting in two consecutive dashes, this wasn't the case in the other two test fails.

Patch for 10.3.4

I thought I would be working on the failing tests, but after some reading on the subject I think the nonce should not be handled on application or backend level, becuase like @gapple mentioned on #2:

A nonce must be randomly generated per-response, not just per-site.

that's going to require disabling all the caches. This article describes an approach, where backend just provide a placeholder for nonce and proxy replaces the placeholder for each request.

Therefore I suggest we either close this issue as "won't fix" or re-title this for suggestion number 2 from issue summary "Implement a sha-X hash".

@sokru My understanding is that whole page caching of a response with a nonce can be acceptable: https://serverfault.com/a/1064775/66144
Using a render placeholder ensures that a new nonce is sent for any request from an authenticated user or otherwise not served from the page cache, where new markup could be placed on the page but could only be aware of an outdated nonce value.

An issue with the proxy / edge cache worker example is that that using a predictable placeholder like NGINX_CSP_NONCE could allow any injected content to use that value as well and have it replaced with the valid nonce.
The current patch also has this issue, as sprintf('nonce_%s', Crypt::hashBase64(__METHOD__)) will result in a known static value. The CSP module uses a random value in its placeholder so that it's not predictable between requests, but using the site's hash_salt would would also work as a way to provide a consistent but unique and unpredictable placeholder value for a site.

@gapple Caching the nonce might be acceptable for someone, but tools like https://csp-evaluator.withgoogle.com/ or https://github.com/google/csp-evaluator for offline usage, will mark it as a violation: "This CSP contains static nonces. Static nonces allow attackers to bypass the CSP since they know the nonce value. Nonces should be cryptographically random."

Quote from https://content-security-policy.com/nonce/ under subtitle "Cons of using a Nonce vs a Hash":

The nonce needs to be generated dynamically, and must be different for each request

Netlify platform has CSP plugin, enabling it will result a random nonce value on each request. You can try by running curl -I https://app.netlify.com/ |grep 'content-security-policy' multiple times and nonce is different on each request.

Hi @rajeshreeputra, and @timovos

I tried and tested the patch--20 for my application to introduce nonce-#hash value. I found something and thought i need to share:

• The Nonce support is working fine after apply the patch, that gives the nonce support with hash value in inline script.
• The CSP Header looks fine with the nonce hash value

Issues/observation:

• After apply patch in the configuration, nonce support toggle button is not working as expected, this is gives always nonce support even with active or inactive.
• In the CKEditor, if any script available the nonce support is not provided for ckeditor, and removing 'unsafe-inline', the script inside CKeditor not in usable state

Tried many way to resolve and support nonce with CKEditor script but still no hope, looking for the solution.

Thanks in advance.