- last update
over 1 year ago Composer error. Unable to continue. - 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
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
- 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
Please credit @dpi who came up with the element_info_alter idea
- 🇬🇧United Kingdom mcdruid 🇬🇧🇪🇺
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 ).
- Assigned to rajeshreeputra
- Issue was unassigned.
- 🇦🇺Australia dpi Perth, Australia
Thanks, good to see this can be automated for third parties that participate in attaching scripts consistently with
html_tag
. - 🇦🇺Australia dpi Perth, Australia
Sorry about the unassign, must be a stale form^
- Assigned to rajeshreeputra
- Status changed to Needs work
over 1 year ago 10:08am 19 June 2023 - 🇮🇳India rajeshreeputra Pune
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.
- Status changed to RTBC
over 1 year ago 11:03am 19 June 2023 - 🇦🇺Australia dpi Perth, Australia
Special characters look like a good sanity test to make sure things aren't encoded oddly / hashed correctly.
- 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
Yep, that's just test code, only used in the test
Happy to change it to any other inline JS - 🇨🇦Canada gapple
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'
forcore/drupal.ajax
to function, if any inline scripts are added to the page through a mechanism other than ahtml_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). - Issue was unassigned.
- 🇦🇺Australia geoffreyr
@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...
- last update
over 1 year ago 2 fail - 🇬🇧United Kingdom mcdruid 🇬🇧🇪🇺
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.
- Status changed to Needs work
over 1 year ago 2:18pm 27 July 2023 - 🇬🇧United Kingdom mcdruid 🇬🇧🇪🇺
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.
- last update
over 1 year ago 2 fail - last update
9 months ago Patch Failed to Apply - First commit to issue fork.
- Merge request !37Issue 3245008: Implement a "semi automatic" Nonce settings → (Open) created by sokru
- 🇫🇮Finland sokru
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".
- 🇨🇦Canada gapple
@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, assprintf('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. - 🇫🇮Finland sokru
@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.