Implement a "semi automatic" Nonce settings

Created on 20 October 2021, about 3 years ago
Updated 14 September 2024, 2 months ago

Problem/Motivation

When using inline JS scripts or CSS the directive script-src requires the 'unsafe-inline' value, which flags the CSP verification as an error:

This can be addressed by :
1 - Removing the 'unsa-inlline' - which in a lot cases it's not possible (example: facebook's pixel)
2 - Implement a sha-X hash - this has the drawback of having to recalculate the hash every time there's a change in the script source file.
3 - implement the 'nonce-BASE64' - Best case scenario (at least in my opinion) since it can per site. So you have a base 64 hash that is defined in the header, applied to any

tag so CSP knows it's a legitimate call.

Steps to reproduce

Access any CSP validation tool (https://observatory.mozilla.org/analyze/ for example) and check if the nonce implementation is correct.

Proposed resolution

In the module - Add a check box: "Use nonce" - Add a field: "Nonce hash" - If the option is checked, add the nonce Hash value to all script tags (using a hook?) and to the CSP directive.

Remaining tasks

- Add a check box: "Use nonce" - Add a field: "Nonce hash" - A hook alter to add the nonce hash to all script tags

User interface changes

Module settings page Add a check box: "Use nonce" Add a field: "Nonce hash"

API changes

Data model changes

Feature request
Status

Needs work

Version

2.0

Component

Miscellaneous

Created by

🇧🇷Brazil barone Belo Horizonte

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Open in Jenkins → Open on Drupal.org →
    Core: 10.0.7 + Environment: PHP 7.3 & MySQL 5.7
    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

  • 🇦🇺Australia dpi Perth, Australia

    mcdruid credited dpi .

  • 🇬🇧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
  • 🇮🇳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
  • 🇦🇺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' 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).

  • 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...

  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 8.1 & MySQL 8
    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
  • 🇬🇧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.

  • Open in Jenkins → Open on Drupal.org →
    Core: 10.0.7 + Environment: PHP 8.2 & MySQL 8
    last update over 1 year ago
    2 fail
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.1.4 + Environment: PHP 8.2 & MySQL 8
    last update 9 months ago
    Patch Failed to Apply
  • First commit to issue fork.
  • Pipeline finished with Failed
    2 months ago
    Total: 231s
    #279276
  • 🇳🇱Netherlands timovos

    Patch for 10.3.4

  • 🇫🇮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, 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.

  • 🇫🇮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.

Production build 0.71.5 2024