🇨🇦Canada @gapple

gapple → changed the visibility of the branch 3411278-remove-code-for to hidden.

The easiest way is to use attachments, either on a relevant render element or with hook_page_attachments(), to add the csp_nonce property with the necessary fallback values.
If you're adding the JS through a library, this should include your analytics_url domain, and any others that piwik includes, and probably not Csp::POLICY_UNSAFE_INLINE.

  $element['#attached']['csp_nonce'] = [
    'script' => [Csp::POLICY_UNSAFE_INLINE],
  ];

The nonce is not automatically added to the header by using the csp.nonce JS library as a dependency because of the need to specify fallback values.
If some other functionality on your page requires 'unsafe-inline', then a nonce will not be added to the header, but any placeholders will still receive a nonce value.

csp_extras is obsolete for Drupal 10.1 and later. 8.x-1.x disables any functionality and will display a site status warning when installed on Drupal 10.1 or later, and 2.x has no functionality.

It should be uninstalled on any sites running 10.1 or later, and is only present in 2.x releases to facilitate being uninstalled.

First, a nonce shouldn't be required if you (1) insert the JavaScript snippet via a library, and (2) authorize any necessary external domains in script-src & script-src-elem.

The Google Tag module and its merge request to support CSP may be a helpful reference ✨ Support Content Security Policy nonce Active . Any information the script needs for gtag configuration as well as a nonce value is passed via drupalSettings.
It only uses a nonce to authorize its script loader to add scripts from additional third party domains, but it still needs to specify those third party domains as a fallback for compatibility, otherwise expected features may be blocked when a page is unable to use a nonce due to something else on the page being incompatible.
If piwik doesn't load further scripts from a domain not already authorized in the policy, then it doesn't require a nonce.

----

Using a nonce for an inline script is something that could use some improved documentation, but is not something I encourage 🙂.
My, now quite old article, on converting inline JS for D8+ only briefly covers part of it, since the nonce feature didn't exist at the time.

My preferred approach is:

1. Place the snippet in a file, added to the page via a library
2. Alter the snippet to add the nonce value to the script element that it inserts into the page:
   

         var scripts = document.getElementsByTagName('script')[0], tags = document.createElement('script');
         tags.setAttribute('nonce', drupalSettings.csp.nonce);
       

3. If you're not hard-coding your {{ analytics_id }} and {{ analytics_url }} values in the js file, pass them to the front-end with drupalSettings in hook_page_attachments
4. Make the library dependent on the csp.nonce library (which implicitly requires the core.drupalSettings library.

The other option is to use a placeholder for the nonce attribute's value → on the inline script and the attribute on the inserted script element so that every request receives a new nonce value. The placeholder can be retrieved from the Nonce Builder service in the relevant theme hook to add as a template variable, and the lazy builder for replacing the placeholder added the element's #attached property or to the page via hook_page_attachments.

Clarify handling service parameter changes

Fixes are applied to 2.x. 8.x-1.x will not be updated for 11.x support.

I found an instance where the CSP module isn't able to prevent core from throwing an exception on the presence of an unexpected key in #attached by decorating the core service: 💬 Questions about adding nonces Active

If a render element uses a lazy builder, and the lazy builder adds the additional #attached property on its return value, the core service is the one that calls the the lazy builder after which it will throw an exception on the unsupported key. CSP doesn't have an opportunity to filter the key before core checks it.

I wrote the code in the comment box without running it so there may be typos or bugs 😜.

🤔 decorating would currently require that the datalayer lazy builder be executed before \Drupal\Core\Render\HtmlResponseAttachmentsProcessor::processAttachments(), so that CSP's decorator of that class can hide it's own '#attached' properties that core doesn't like, but it looks like that method is actually executing render element lazy builders itself.
It may require ✨ Allow additional keys in #attached Active in core to work, so that core ignores the ['#attached']['csp_nonce']

In that case, it's probably necessary to either:
a) patch core with the above change

b)
Use another method than $build['datalayer']['#attached']['csp_nonce'] to add the nonce to the header.
If you know datalayer is on every page, you could have your own subscriber to the CspEvents::POLICY_ALTER event always call \Drupal::service('csp.policy_helper')->appendNonce($policy, 'script', [Csp::POLICY_UNSAFE_INLINE]);

or c)
use hook_page_bottom(), weighted to ensure it's executed after datalayer_page_bottom(), to add the attribute and #attached values so they exist before the final page rendering flow is started.

@lavanyatalwar can you detail your testing steps, including how you ensured that the browser is requesting a new token on login while the maximum is present in the database?

If a user tries to remember on more browsers than is configured to be allowed, the token for that user that is next to expire is deleted.

This means it's either:
- If token expiry is refreshed when used: the least recently used token
- If token expiry is fixed based on creation: the oldest created token

https://git.drupalcode.org/project/persistent_login/-/blob/2.x/src/Token...

You could decorate datalayer's lazy builder class to alter the element when it's being built.

mymodule.services.yml

  datalayer.lazy_builders.mymodule:
    public: false
    class: Drupal\mymodule\DatalayerBuilder
    decorates: datalayer.lazy_builders
    arguments: ['@datalayer.lazy_builders.inner', '@csp.nonce_builder']

mymodule/src/DatalayerBuilder.php

namespace Drupal\mymodule;

use Drupal\csp\Csp;
use Drupal\csp\NonceBuilder;
use Drupal\datalayer\DatalayerLazyBuilders;
use Drupal\Core\Security\Attribute\TrustedCallback;

class DatalayerBuilder {

  public function __construct(
    protected DatalayerLazyBuilders $decorated,
    protected NonceBuilder $nonce_builder
  ) {
  
  }

  #[TrustedCallback]
  public function build() {
    $build = $decorated->lazyScriptTag();

    $placeholderKey = $nonce_builder->getPlaceholderKey();

    $build['datalayer']['#attributes']['nonce'] = $placeholderKey;
    $build['datalayer']['#attached']['csp_nonce'] = [
      'script' => [Csp::POLICY_UNSAFE_INLINE],
    ];
    $build['datalayer']['#attached']['placeholders'][$placeholderKey] = [
      '#lazy_builder' => ['csp.nonce_builder:renderNonce', []],
    ];

    return $build;
  }
}

----

Setting $build['datalayer']['#attached']['csp_nonce'] tells CSP module that it needs to add the nonce generated for each request to the header if possible, and the placeholder in the script's attribute will be replaced with the same value.

I think I misunderstood - I thought you had meant adding the nonce as a property to the JS dataLayer object.

The module's documentation has an example of how to add a nonce to a render element → . In this case since datalayer is already rendering in a lazybuilder, it doesn't have to (shouldn't?) use CSP's nonce lazybuilder.
I'm pretty sure #attached gets bubbled up from lazy-built elements for CSP to have access to in it's response processor, but that would have to be double-checked.

if (\Drupal::service('module_handler')->moduleExists('csp')) {
  $element['#attributes']['nonce'] = \Drupal::service('csp.nonce')->getValue();
  $element['#attached']['csp_nonce'] = [
    'script' => [Csp::POLICY_UNSAFE_INLINE],
  ];
}

This requires CSP >= 2.1.0, so the module should also add a conflict to its composer.json to ensure it's not installed with an older version that would cause core to through an exception when encountering ['#attached']['csp_nonce'] :

"conflict": {
  "drupal/csp": "<2.1.0-beta1"
}

gapple → changed the visibility of the branch 3203811-csp-domain to hidden.

I don't think you would want to add the nonce to the dataLayer, but you can access the nonce server-side through the csp.nonce service, or in scripts with drupalSettings.csp.nonce by adding the csp.nonce library as a dependency.

Closing out 7.x issues

Closing out 7.x issues

Closing out 7.x issues.

Closing out 7.x issues

Closing out 7.x issues.

Closing out 7.x issues

Closing out 7.x issues.

Closing out 7.x issues

Closing out 7.x issues.

Closing out 7.x issues.

Closing out 7.x issues.
Updated version since this is reportedly still an issue with 8.x releases.

Closing out 7.x issues.
This issue is not known to be present in 8.x releases.

Closing out 7.x issues

Thank you for the bug report.

The disabled field value was still in the form data and getting set to config when it shouldn't have been. 2.2.1 is now available with a fix 🙂.

----

SecKit and CSP are fine to work together, as long as the CSP setting in SecKit are not used since one module would overwrite the other's header value. (CSP will add a warning to the site status if it detects Seckit's CSP fields are enabled).

Are the errors being reported on all pages (e.g. while you're viewing the settings form with all options disabled, or only public pages)? Do you see a Content-Security-Policy or Content-Security-Policy in the response headers in the browser developer tools? Do you have a proxy service caching pages between the server and your browser?

The module tags its own cached info and any page responses with the settings key, so they should be automatically invalidated when you save the settings page. Private responses, such as for authenticated users, should always bypass any page caching and generate the header based on the latest settings (provided the cache backend understands cache tag invalidations).

Is it worth it to include the contrib minor version? It's additional detail than previously, where the Compatibility-Major.PatchLevel scheme melded together changes that would now be separated to semantic minor and patch releases. I expect in most cases update functions are going to be tied to minor-level changes, so there would be a lot of M0101, M0201, M0301....

The minimum schema version sets the lower bound of numbering at 8000 while it still exists, but the default recommendation could just be Mdddd (major without leading zero + 4 digit increment). Only a note would be required for pre-stable 0.x.x versions to start the update numbering at 8001.
Contrib modules could choose to include the minor version if they will be concurrently supporting minor releases for a longer duration, or just leave an arbitrary gap (e.g. +10) to account for the potential of patch-level updates.

Applying default-src 'none' to other private files should only have an impact if sites were serving HTML files, and would be irrelevant to other responses like image files, so it might be worth leaving for discussion in 📌 Add a default CSP and clickjacking defence and minimal API for CSP to core Active or breaking out to a separate issue. It could then be done in combination to applying the same change for the files directory htaccess so that private and public files would have consistent behaviour, like SVGs would with this issue.

@timohuisman that's odd, but I guess not unexpected that a response might not have a content type header.

I think I would prefer to update the patch so that it only excludes content types and response classes that it knows are safe.

I don't think a short persistent login lifetime is useful, since PL cookies are intended to have a longer lifetime than session cookies, and are ineffective in the inverse case.

If you can better outline your use-case, and how it's addressed by allowing short PL lifetimes, please reopen the issue.

Quick fix to add new words to cspell dictionary

Made a MR with changes from patch file.

@damienmckenna
Possibly - the full violation report should include an original-policy property that you can verify if clients are receiving the updated directive.
Including a hash, nonce, or using 'strict-dynamic' would also disable inline scripts, but it sounds like you're not using any of those.

('inline' isn't a valid directive source, but it should just be ignored).

Updated the patch from #105 in a MR

- Move allowedSlots property declaration to constructor; replaces mismatched default value on property (#106)
- Added missing early return in set() (#113)
- Override setMultiple() to only count() extra items once before removal (#108)
- Code style fixes

gapple → made their first commit to this issue’s fork.

Can you elaborate on your use-case of this short of a token lifetime, and why you would like to do this with Persistent Login rather than setting a short session lifetime or use a module that enforces a session timeout after a configurable period?

With the module's recommended setting of the cookie_lifetime to 0, a browser will keep the session cookie until the browser is closed (provided it isn't still kept longer because of a session restore feature). With browsers remaining open in the background even when all windows are closed, the short-lifetime PL token could expire before the browser cleans up the session cookie.
Since the session cookie takes precedence, it will in most cases likely outlive a short persistent login.

The test includes a couple keys previously supported by core, but not an unknown key (e.g. the csp example in the issue summary)

Something that came up in #2868079: Add a default Content-Security-Policy-header for svg files → is that there may be a need for different policies in certain cases - uploaded files should probably have scripts and styles blocked, though SVGs may need inline styles, image data, and fonts. Private files can have the header applied from the event subscriber, but public files would require an addition to .htaccess.

Instead of a single set of policies, the service parameter could be keyed by use:

default:
  report_only:
  enforced: "script-src * 'unsafe-inline'; object-src 'none'; frame-ancestors 'self'"
private_files:
  report_only:
  enforced: "default-src 'none'"
svg:
  report_only:
  enforced: "default-src 'none'; style-src 'unsafe-inline'; image-src data: font-src: data:"

👋 CSP module maintainer here.
CSP module provides APIs for modules to amend a site's policy → . In most cases it's pretty straightforward, but gets more involved for cases where it's not just adding an additional external domain to a directive. The goal of the module is that anyone can install the module for a security improvement, that it's easily configurable for site builders to add exceptions not captured automatically (& ideally harden the default config with basic knowledge), but also flexible for developers to handle any more complex uses (and to do so safely without breaking other site functionality).
Module subscribers with the same priority will execute after core, so it's the module's responsibility to preserve/amend/replace any value previously set by core.

CSP has its own issue for applying a different policy to private files ( ✨ Provide different CSP policy for private files Active ), though there hasn't been any work done on it so far. If a special value is added to core for SVGs, then I'll update the module to match. If there's consensus on what should be added to core, I'll probably have a new release of the module before it's committed to core 🙂.
Alternately, the CSP module would be a good way to evaluate a potential solution (at least for private files, I'm not sure about the module altering htaccess), with users who have opted in to using a policy on their site. It would have to be an optional feature for the current major version, but could be enabled by default for new modules installs (and a site status message could prompt existing sites to try enabling it).

This issue for core would add a static policy that can be configured in a service parameter: 📌 Add a default CSP and clickjacking defence and minimal API for CSP to core Active
Instead of providing a single policy set, maybe it should be keyed policies to allow for additional cases (e.g. default, private-files, svg, etc, each with report-only and enforced values).

Better description of alter priority; example of nonce & fallback

Opened 🐛 ConfigFormBase::formatMultipleViolationsMessage() return type change will cause error Active as a bug from this change.

gapple → created an issue.

Ran into an issue with the the return type change, since I was developing a module using the latest version of core and copied the new return type hint.

When using the new return type in an override with a stable version of core <=11.0.5, the child class is returning a wider type than it's parent.
When using the old return type in an override with 11.x-dev, parent::formatMultipleViolationsMessage() returns a wider type than the child class.

For a module to maintain compatibility with older versions of core and not break if it calls the parent method which now returns Markup, it will need to change the value returned from its parent:

    $parent = parent::formatMultipleViolationsMessage($form_element_name, $violations);
    if ($parent instanceof TranslatableMarkup) {
      return $parent;
    }
    else {
      // phpcs:ignore Drupal.Semantics.FunctionT.NotLiteralString
      return $this->t((string) $parent);
    }

Once the module drops support for older versions of core, it can update its typehint and remove the extra code.

\Drupal\update\UpdateSettingsForm also needs its return type updated.

gapple → created an issue.

The current MR would address multiple violations on a non-sequence value, but if an item within a sequence has multiple violations it looks like only the last one will be kept.

ConfigFormBase::formatMultipleViolationsMessage() also assumes it's being called on a sequence, so multiple constraint violations on a non-sequence value would still be formatted like "Entry 0: Constraint 1 message \n Entry 1: Constraint 2 message".
Forms can override the method to change the output based on the form element name in this case though.

Found a couple more incorrect references

gapple → created an issue.

Changes to provide a form error if an unavailable reporting plugin is referenced in config are in the 2.1.1 release.

Be sure to run module updates, and let me know if you continue to see a similar error.

Is this after updating from 1.x to 2.x? Do you have a pending site update?

https://www.drupal.org/docs/extending-drupal/contributed-modules/contrib... →
The sitelog reporting handler was removed from 2.x in favour of the Reporting API module → , and an update will disable policy violation reporting on a directive if it has not been changed to a supported alternative.

It is still possible to better handle the configuration referencing an unavailable reporting plugin there though...

🤞 that's a final fix for this issue.

Just wanted to also note that the tokens in the database aren't accepted even if provided by a browser, and this cleanup is only really needed for sites with tokens configured to a permanent lifetime, so there isn't a problem if the update is not completed on a site (provided it doesn't block any other necessary updates).

That's odd... On the last iteration $sandbox['#finished'] = $sandbox['processed'] / $sandbox['total']; should equal 1 to complete the batch.
Are there tokens stored for the anonymous user (uid:0)? That would increase $sandbox['total'] to one more than will be returned in the $token_users queries - but that shouldn't happen so some checks might need to be added elsewhere 😬.

Didn't catch @jabeler's review before - updating to RTBC :)

Bumping priority - CSP 1.x is still the most used version at this time, but is no longer receiving feature updates.

This is fixed in 8.x-1.9 and 2.2.3

The double-hashing was fixed in 8.x-1.7 and 2.2.1
Cleaning up tokens for deleted users was fixed in 8.x-1.9 and 2.2.3

Thanks for the quick patch & review :)

New patch releases should be available shortly.

greggles → credited 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.

@rgpublic A hash can't be used on the external gtm or gtag js because their contents aren't known or static. The loader file could change at any time, making the hash invalid, and any additional dynamically loaded scripts won't be permitted.
The tag manager script loader will propagate the nonce to additional scripts (as verified by @chrissnyder in #12), without requiring the originating page to know the script contents (for a hash) or source domain (for non-tag manager scripts) to place in the policy.

@berliner From the one post I've seen that's worked through the impact of full-page caching with a CSP nonce https://serverfault.com/a/1064775/66144:

Let's say you as an attacker are served the same nonce as your victim on a response to the URL /foo/bar. Now you could use this nonce in your XSS payload. Let's consider two of the most common XSS scenario's:
- URL parameter based reflected XSS: /foo/bar?payload=... will likely have a different cache key than /foo/bar, therefore causing the user to be delivered a fresh response with a new nonce. The attack fails.
- Stored XSS in dynamic HTML: your payload is now included when the user loads /foo/bar. However, if the user refreshes and gets a response from the cache, then this won't include your injected XSS payload. Alternatively, if they get a fresh response they will also get a fresh nonce. In either case the attack fails.

Now, this doesn't mean that this situation can never be exploited: for example, if /foo/bar has vulnerable JavaScript that modifies the DOM in a way that the attacker payload can be injected it might possible to use the cached nonce. Think of DOM based XSS attacks via data in the # segment of a URL or in AJAX responses.

It will be much trickier for an attacker to exploit the latter situation though, which may be sufficient since the point of CSP is to just provide defense-in-depth; it can not guarantee immunity for 100% of XSS attacks.

For Drupal, that means to not use AJAX responses to inject unsanitized markup, from user provided data, containing an inline script with a nonce attribute. This would also only be an effective attack against anonymous users on a single page for the duration of the calling page's cache lifetime, and other pages that receive the injected inline script with a nonce (and the original page when it's re-rendered with a new nonce) will send a violation report that should prompt investigation to address the underlying markup injection.

(AddJsCommand for adding new script files to the page through AJAX does not (currently) propagate the page's nonce to the new scripts, so they must be permitted by the originating page's policy through another source like 'self'.)

The CSP module includes a test that authenticated users receive a unique nonce for each request of a page that is cacheable in the dynamic page cache.

----

I've seen an approach where an edge cache worker handles replacing a placeholder value from the application, so that even responses served from the cache receive a unique nonce. But if the worker fails then responses are receiving a truly static value for all requests, and some examples I've seen have used a potentially predictable placeholder value such that markup injection would also be replaced with a valid nonce.
This would be possible with the CSP module by swapping out the Nonce service to one that responds with a static nonce value that includes some secret shared between the application and edge cache.

Removed "Define accepted keys" option from summary, update how CSP now uses #attached.

Slightly different title; "Don't prevent modules from using #attached for additional data" would mean the same thing.

gapple → created an issue.

gapple → created an issue.

- I'm not sure if this could be done entirely in csp.schema.yml by directives extending/overriding the basic constraints in the csp_directive_source_list schema, or would need a custom validator.
- Would a custom validator receive context of the parent config item, in order to be able to filter the list of valid flags?

gapple → created an issue.

gapple → created an issue.

gapple → created an issue.

There's some additional validation constraints that are useful:
- The reporting plugin id →
- Required / optional mapping keys

Some possible future enhancements:
- Only permit the 'flags' property on relevant directives
- Validate the 'flags' values permitted for the respective directive
- Validate directives sequence keys

Being fixed in one merge request on parent issue.

Being fixed in one merge request on parent issue

Rebased.

CSP is now working around core by decorating the core attachments processor service, but it would be nice to not have to do this, since it also requires any module implementing CSP's new options to do a moduleExists() check.

- Squashed and rebased
- Draft Change Record created →
- Postponed issue for 12.x created 📌 [12.x] Set default Content-Security-Policy in services.yml Postponed

I'm not sure why tests are failing on ImageStylesPathAndUrlTest:L315

Remaining questions:
- Service parameter name: http.response.content_security_policy (like http.response.debug_cacheability_headers) or csp.config (like cors.config) or ?
- Should any X-Frame-Options header present on a response be removed, since browsers will ignore it when CSP is present?
- Review additional directives to add by default: script-src * 'unsafe-inline'; object-src 'none'

gapple → created an issue.

I'm not sure about best practices, but I do have thoughts :)

Ideally, other modules use CSP's APIs so that site builders, or anyone else, just needs to enable the module and the site's policy will be updated as needed for the module's functionality to work. CSP handles any adjustments for core, but shouldn't take on the responsibility for every other contrib module or hosting platform.
It's definitely a challenge that every site may need a pretty bespoke policy for its functionality (and the dev/support team's capacity for monitoring and managing its strictness).

A lot of cases for scripts and styles are handled transparently by parsing library info, but more complex cases or non script/style sources will currently need to implement an alter subscriber (e.g. Google Tag Manager / Google Analytics, which uses a loader script and can use a nonce, and also needs to modify img-src and connect-src [ Merge Request pending on google_tag ✨ Support Content Security Policy nonce Active ]; or Google Fonts, where a local CSS file needs an external domain included in font-src).
The render attached info should enable supporting some cases in a more direct way than an alter subscriber, and some more cases for libraries would be supported if/when ✨ Enable specifying additional directives in library definitions Active is implemented.

If someone's implementing a custom module instead of a public contrib module, then I'm not particularly worried. I assume by them implementing a CSP policy they'll know to make necessary policy modifications, and they can use the contrib module's subscriber as a reference.

There's a possibility of another contrib module that extends CSP's config form with a new tab that has checkboxes to enable policy modifications for additional services that are used without a module, but that could get fairly long and messy for a lot of services that any particular site is not going to use.

There was a GitHub repo some time ago that documented the directives required for a lot of different services - I'll have to search if it still exists and is up to date. If there isn't a good reference somewhere else, a page could be added to the module's documentation for services commonly used on Drupal sites, and then site builders could choose to apply the changes in their configuration or a custom subscriber according to their preference (and anyone can contribute new services or changes to the documentation).

----

Where the policy values are static and global (and the config form could be disabled so it's not used), adding them using config overrides in settings.php would also be an option, and would make it clearer that the base value is enabled too (so, e.g., img-src data: isn't enabled without 'self' when uncommented).

An option could be for libraries to specify if they should be a conditional include, so that their sources are only added when the library is present on the page instead of the current behaviour of always being added on every request. This would be one method to resolve the issue where a module may have optional libraries that are only included when a certain feature is enabled (e.g. IIRC webform has some libraries with external domains that are not used by default).