🇨🇦Canada @gapple

Yeah, the introduction of the nonce service so far has been to a) offer a single service so that modules aren't adding multiple nonces on the same request, and b) work for front-end libraries that dynamically load scripts (e.g. google tag manager). Having nonced server-side content has been less important, since there's solutions like passing data with drupalSettings to a static script.

✨ Provides a filter to add nonce attribute to inline scripts. Needs work takes the same approach of using a placeholder for a nonce, but isn't complete yet to apply it to the header.

I don't entirely like having a static cache that the lazy builder methods add to, and is later consumed by the alter subscriber. I'm pondering if the subscriber would be able to parse the attached placeholders itself for the necessary parameters. It seems like it should be possible to add more information to the placeholder alongside the #lazy_builder callback if more information is needed.
Ideally, I think it should be possible to add a new category to #attached for CSP to use directly - then the placeholder system isn't doing a bunch of find+replace on the entire page content unnecessarily. It would also allow something that uses its own lazy builder to pass up CSP information to the containing page separately - otherwise with Big Pipe the lazy-built content is too late to add information to the header.
Unfortunately core currently restricts the possible keys in #attached, but I think it's reasonable to lift this restriction (I have another module that needs to replace the core service to allow another key, and it can't cooperate with more modules that want their own new allowed keys).

This approach could very likely have issues with mixing 'unsafe-inline' and nonces, since when they bubble up it's unclear if a nonce would need to be removed for something that requires 'unsafe-inline', and if that nonce-able thing would need a different allowed source when it can't be nonced (it could be recommended to always add both with the expectation that CSP will remove the nonce if it needs to, but this will result in extra items if the nonce is kept) ✨ Add helper for safely appending nonce/hash sources Fixed . Having ✨ Enable conditional/alternate directive values Active to pass along the alternate values and resolve them at once in the final policy would make the DX a lot clearer, and be less likely to give unexpected results.

Thanks for highlighting those issues for some DX improvements - I'll probably work on adding those in on their own.

I'll add a bunch of small notes to the MR, but I think this will be a 2.1.x or later feature.

Clarify adding element attributes; bigger inline JavaScript warning

Make inline JavaScript warning much more prominent, add example of rewriting JS loader to library

Clarify setting additional HTML element attributes, and other things

@sascha_meissner it looks like the original author of that section confused using the braces format of YAML dictionary notation for a JSON object.

The following are equivalent YAML:

https://maps.googleapis.com/maps/api/js?key=myownapikey&signed_in=true&libraries=drawing&callback=initMap:
  type: external
  attributes:
    defer: true
    async: true
    data-test: map-link


https://maps.googleapis.com/maps/api/js?key=myownapikey&signed_in=true&libraries=drawing&callback=initMap: {
  type: external,
  attributes: {
    defer: true,
    async: true,
    data-test: map-link
  }
}

Using braces for YAML dictionaries is good for keeping items with few properties to a single line, and it ignores line indentation within the braces.  For more complex items, using the indented multi-line notation is usually easier to read.

gapple → created an issue.

One possibility might be to break up the alter event into two phases:

• Global alterations, which are not dependent on context (but with adding appropriate cache tags to the event object). This could even be preemptively generated on cache rebuild.
  The module's own config would be one instance (tagged with config:csp.settings), or the sources added by library info (tagged with library_info).
• Per-request alterations that act on things like which libraries are attached to the request, or if something on the page wants to use a nonce.
  CKEditor requires style-src 'unsafe-inline', which is only added if it's attached to the page's libraries.

Something like google_tag straddles both, because it is mostly likely present on every page, but would preferentially use a nonce - and the order of alterations when attempting to use a nonce is important ( ✨ Add helper for safely appending nonce/hash sources Fixed , ✨ Enable conditional/alternate directive values Active )

gapple → created an issue.

Noticed some extra cases to test:
- if appending a new nonce or hash and the directive already includes a nonce, hash, or strict-dynamic (which disable 'unsafe-inline'), then the value should be appended (and not the fallback).
- if requiring 'unsafe-inline' and the directive includes 'unsafe-hashes'

Some cleanup is possible with 'unsafe-hashes':
- if 'unsafe-hashes' isn't present on an attribute directive (e.g. it was copied from the base directive), any hashes can be removed since they're not effective (assume they correspond to permitted hashes of elements).
- remove 'unsafe-hashes' from an element directive.

This would also cause problems for a feature like ✨ Provides a filter to add nonce attribute to inline scripts. Needs work , and anything else which uses nonces, which need to alter the policy on every request.

I altered the parameter order so that a fallback value is always required, and added a note on the method docblock about properly calling requireUnsafeInline() early.

✨ Add helper for safely appending nonce/hash sources Fixed will help modules that want to use a nonce or need 'unsafe-inline' to do so properly, but is order dependent:

• If 'unsafe-inline' is required it needs to be added to the policy early in the Alter event
• If a nonce or hash is added, it needs to be done later in the Alter event

Otherwise fallback values (other than 'unsafe-inline') for the features that use a hash or nonce won't be present in the directive as necessary, and they could be blocked by the policy.

Having a way to collect all alternates and only resolve them when the final policy is being built will make it so that the calling order is not relevant, and developers are less likely to encounter issues if they're not aware of the need for proper ordering of policy modifications.

Thanks!

I added an 8-byte random string to the placeholder key. It's only present server side and never exposed in the request, so just needs to be sufficiently long that an attempt at enumerating guesses is unlikely to escape notice.
As a property on NonceFilter, all placeholders in the same request will have the same key (which is fine). Since those page fragments can be cached with the placeholder, a page may end up with multiple placeholders (which is also fine, if just slightly more processing for the final render).

For big pipe, it looks like it differentiates between placeholders that are html elements (<filter-placeholder ... />) which will have their rendering delayed until after the main page content is flushed for JS to swap, and "attribute-safe" (filter-placeholder) that it will replace server-side in the main page content 👍.

Stepping through with a debugger, I found a few things:
- The placeholder is present until the Response event, when HtmlResponseSubscriber processes attachments and does the replacement, so this method works as expected with the Dynamic Page Cache, and nonce attribute appears to receive a new value whenever the page is rendered with any changes 👍
- The placeholder replacement takes place over the entire page response. Most placeholders use a hash of the lazy_builder method and arguments, which is potentially predictable. Not an issue in most cases, where it would just allow duplicating a page fragment elsewhere on the page, but for a CSP nonce, it could allow an XSS vulnerability to use the placeholder to permit it's own script with the nonce 😱. I think adding a random string to the placeholder should be sufficient to prevent its misuse.
- Big Pipe appears to wrap the placeholder for it to do dynamic replacement via JS, which will prevent the inline script from working properly (AFAICT - the script will be blocked when loaded with the placeholder value in the nonce attribute, and I don't think it will be re-evaluated after the proper value is placed). Need to find a way to have the nonce placeholder processed through the normal core method, and not via big pipe.

It's really important that nonce values are not cached in page fragments - if a cached nonce is repeatably sent for markup on one section of a page, another component with an XSS vulnerability could hard code that nonce value to permit a malicious script.

I think this is the use for placeholdering - the filtered markup can be cached in the Dynamic Page Cache with the placeholder, which is only replaced later in the rendering process with the actual value. (having the nonce cached in the Full Page Cache is fine, since the nonce will change if any markup on the page changes). I made this change in my MR, and it appears to work.

Using cache context to bubble up that markup on the page needs the nonce applied in the headers feels a little hack-y, but it could work. I think the limitation for the current code is that it doesn't indicate whether scripts and/or styles need the nonce value applied to the corresponding directives - I think using cache context would require separate contexts for each.
An alternative that I think I might like the semantics of a bit more is attaching a new (empty) library (e.g. 'csp/inline-js-nonce' / 'csp/inline-css-nonce'), that a subscriber to the alter event can then check for.

(This is the second time I want Drupal core to allow additional keys in render arrays' attachments so that more info can bubble up...)

I fixed my off-by-one, and helped confirm that there shouldn't be any issues with func_get_arg() by adding a couple tests of the possible parameters that could be passed if the module was being extended based on an older module version.

gapple → created an issue.

Current changes applied in other issues

gapple → created an issue.

The particular lines should only be relevant for modules that extended that class against the very briefly used parameter order in 1.22 and 1.23.

I think I counted wrong when specifying the parameter index through 😬

Current fixes applied

Current fixes should have been applied in other commits / issues

gapple → created an issue.

gapple → created an issue.

gapple → created an issue.

Removal was merged to 2.x, since it's still at alpha release.

I would like to get a stable release of Reporting done before adding the deprecation warning to 1.x, but will need to review its roadmap and pending changes first.

gapple → changed the visibility of the branch 3017868-remove-db-reporting to hidden.

Started on removal for the 2.x branch.
1.x branch will still need deprecation warning.

Looks like Webprofiler has a decoration solution now:
https://git.drupalcode.org/project/webprofiler/-/commit/43e8aba836c7a696...

(though this: $this->dataCollector->setPlaceholders($attachments['big_pipe_placeholders'] ?? []); looks like it might not handle core's placeholders when bigpipe is disabled - $attached['html_response_attachment_placeholders'])

xjm → credited gapple → .

It might not be possible to get around this conflict without core changes that allow everyone to decorate instead of replace the service
#3096061: [upstream] html_response.attachments_processor service must be replaced instead of decorated →

I'm not sure that I can help here beyond validating that Drupal and the CSP module are outputting the header as expected.
Rather than setting default-src, you can set the more specific directives to see which one is actually being violated and needs the additional protocol without allowing it more broadly. (The module will remove additional directives if they fallback to default-src and are set to the same value though, so you will need to disable or change default-src while testing - even just adding a fake url would work).

blob: is the correct formatting to use in that field for the protocol (as checked by the form validation)

My own approach would be to have a field on legacy content that would enable the relevant CSP changes but have the values set elsewhere (or hard coded in a custom policy alter listener), assuming that legacy pages have a common set of requirements.

I'd be open to working on changes like bubbling up metadata so that overrides could be better targeted to only apply when relevant content is rendered, but I won't be working on adding a full configuration form to entities myself.

@Summit

This module is unsupported and obsolete since the discontinuation of Universal Analytics, and is unrelated to the google_analytics and google_tag modules which support the new GA4 system.

Do you have a more concrete example of where this would be needed, and the policy changes aren't connected to a library or other information available from the response object during the policy alter event?
🌱 Better CSP support for themes Active has some other ideas for how policies could be granularly added from page components, like providing values for additional directives in library definitions, or bubbling up data from render arrays.

Adding CSP overrides to entities through the UI would also have user permission concerns, since it would potentially allow something like permitting script-src 'unsafe-inline' alongside an XSS vulnerability on the corresponding page.

gapple → created an issue.

Different use case, but there may be some overlap with https://www.drupal.org/project/login_history →

For SEO: no.

This is only required if you're using google features that load assets through country-code top level domains. Basic analytics functionality does not require these domains, but some of their ad-related features do.

A slightly rough MR:
- Introduce a new service parameter for setting static CSP values
- A late acting response subscriber will translate X-Frame-Options to Content-Secutity-Policy: frame-ancestors
- If a CSP policy is set (via service parameter, or another module like CSP or seckit), then that value is not changed.
- The X-Frame-Options header is always removed - either it's being replaced by core with an equivalent CSP policy, or we assume that the CSP policy set by the user has their desired frame-ancestors value (including the option of being omitted).
- If X-Frame-Options is not set to SAMEORIGIN, then a deprecation warning is issued to use CSP to set the value. (If the value is still the default SAMEORIGIN, then a future version of Drupal changing to frame-ancestors 'self' will not change browser behaviour).
- ALLOW-FROM is ignored by modern browsers (and equivalent to not sending the X-Frame-Options header), but is translated to a CSP value if used.

In a future version of Drupal:
- The service parameter can be set to a default enforced CSP value
- The late acting event subscriber can be completely removed
- Core can stop setting X-Frame-Options
- Modules (such as CSP or seckit) will still override core's default (now static) value.

Why the CSP policy value:
- script-src * 'unsafe-inline' will only block eval(). Not including 'unsafe-inline' would have the potential to break existing sites (and hopefully its presence leads people to explore making sure it's not required...)
- object-src 'none' is recommended to block legacy HTML elements
- frame-ancestors replaces X-Frame-Options
- I don't think there's other values that can safely be added as a default enforced policy directive without a reasonable risk of negatively affecting some sites. (maybe base-uri 'self'?)

gapple → created an issue.

I haven't coalesced my thoughts about what information is relevant for each of the project page, readme, and documentation pages (personally, I don't think just copying the project page to a readme file is useful, and I think there's few things that should be in a readme that aren't also worth having on the project page).

Generic recommendations are kind of difficult, since build practices, modules used, third-party services, whether CSP is being added to a new or existing project, and developer knowledge vary so much.

The MDN web docs are a good resource on the available directives, and has some recommendations, but isn't necessarily beginner friendly
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Securi...

Two approaches to configuring a policy:

1. Set the strictest possible report-only policy (e.g. default-src 'none'; script-src 'self'; style-src 'self'; img-src 'self'), and then selectively allow sources as needed based on the violation reports after browsing around your site. Either do not set a reporting handler so you don't spam/DOS your own site log (and then check the violations in your browser console), or use Report-URI.com's wizard to ingest and summarize the reports. Deploy your report-only policy to staging/production, monitor for violations, and adjust. Once you're not receiving violation reports, promote your policy to enforced.

   I think this approach may be more suited for new sites, were problems on staging / pre-production are likely to be found by devs and testers, and not site users.

2. Start with a very lenient policy and incrementally narrow restrictions on your report-only policy. Monitor small changes for violations (in staging and then production) and adjust before promoting to enforced. This is sort of the approach the module's default configuration takes: the enforced policy is rather lenient, but if the report-only policy doesn't result in violations in can be promoted. A directive like font-src is one that's usually pretty easy to add to a policy to restrict loading to only your chosen font service, but can't be included in the module's default policy.

   I think this approach is more suited for established sites, where prematurely enforcing a policy could result in breaking existing functionality for site users. (New functionality breaking because a needed adjustment to the enforced policy is not deployed at the same time, I think is less significant concern - and testing should probably catch issues before that happens).

📌 Improve default config Active has some philosophy behind the module's default configuration and some options being considered for 2.0.0 that I would appreciate feedback on.

If you have specific questions on getting started with CSP, feel free to ping me on the drupal slack - either privately or start a thread in the #security-discussion channel. I really value learning about other people's implementations so I can eventually work on a guide to help others get started!

@jedihe was the one who added the addBack() in #7 / #8

https://git.drupalcode.org/project/drupal/-/merge_requests/3844/diffs?co...

I haven't checked the latest code in the merge request, but if IIRC, the addBack() was necessary because while in most cases the affected element has a wrapper with one of the relevant .js-form-item, .js-form-submit, .js-form-wrapper classes and the find() call will return the proper child node, there are some cases where the class is applied to the element itself.
While closest() will return the element it is called on if it matches the selector, find() does not. addBack() will append the element that was returned by closest() if it matches the list of element types.
Only one of find() or addBack() should return an element.

It should have the same result as this, without needing a variable or conditional block:

elem = eventTarget.closest(wrapperClasses);
if (!elem.is(tagsSupportDisable)) {
  elem = elem.find(tagsSupportDisable)
}

With IE no longer being supported, core could replace its default X-Frame-Options: SAMEORIGIN with Content-Security-Policy: frame-ancestors 'self'. Starting to always add a default CSP header could cause problems for a site that sets X-Frame-Options: DENY though. (It looks like IE was the only browser to support ALLOW-FROM).

A possible transition is:
[11.1] Add a late-subscriber that translates x-frame-options to a CSP frame-ancestors if the response does not have a CSP header (assume if a CSP header is set but does not include frame-ancestors, then it was intended to allow any frame parent). Issue a deprecation warning if the response has a value other than X-Frame-Options: SAMEORIGIN to notify users to use CSP instead.
[12.0] Remove the late subscriber, and replace X-Frame-Options with a static CSP policy that includes frame-ancestors 'self'. A site that uses the Content-Security-Policy module, SecKit, or a custom listener to set the CSP header will override this default value.

My suggestion for a minimal static policy that core can add by default:

  Content-Security-Policy: script-src * 'unsafe-inline'; object-src 'none'; frame-ancestors 'self'

The simplest way to allow modifying the default CSP (or adding a CSP-Report-Only), would be to have a container parameter. Users wanting a more flexible implementation can use the CSP module.

Restoring priority

Added some small changes:
- Cleanup unused data provider
- Don't require yaml extension for tests
- Symfony doesn't accept !php/const for the value of a backed enum (BackedEnumValue::Maybe->value)

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

I think this issue has finished its usefulness - the most notable modules I'm aware of have support.

Feel free to ping me in Slack or open another issue for help supporting a particular module :)

The idea of user-inputted inline scripts still scares me, but I don't think it's something that shouldn't be possible for adequately trusted roles.

I think that there should be some help text to indicate that the filter should only be enabled for trusted roles.

A potentially expected complement could be to enable inline style blocks? That could be a separate filter, or a single filter configurable to enable either scripts or styles as desired.

There's also a gap that enabling the filter will add a nonce to the markup, but that won't permit any scripts it processes unless the nonce is added to the header through some other means ( ✨ Add helper for safely appending nonce/hash sources Fixed will help with safely adding the nonce or 'unsafe-inline' fallback).

I've just set the report-only options to always be default - I don't think having it switch to enforced if that's the only enabled policy provides much benefit, and having it always show the first possible tab is clear and consistent behaviour (that also reduces the likelyhood of making inadvertent changes to the enforced policy).

Leveraging hook_rebuild to warm the cache is a good suggestion, but the responsibility of storing the data to cache should stay in the service, since there are occasions other than a full cache rebuild where it's possible that the items are not in the cache. If they aren't recreated on demand if missing, then every future page request is going to parse the site's library info files.

If a site is having issues with MySQL locks on cache tables, it seems like there's many places where that could occur, so my thoughts would be:
- The database server's isolation level might need to be adjusted ( docs → )
- The site might benefit from moving to a different cache backend like memcache or redis

----

I'm not sure if the database cache backend can be configured to ignore a timeout when writing a value, since a page can render fine after skipping storage (and hopefully it's taken care of by the request that's holding the lock).

I think I agree with that change - IIRC, my thought was that viewing that there is an enforced policy and its values would be important.
But if editing is the priority then having the report-only policy default makes sense. There's also the tab summary which indicates that a policy is enabled (and how many directives), and ✨ Copy configuration between report-only and enforced Active would help with suggested workflow of tweaking the report-only policy, then copying it to enforced after testing.

I would not switch the order of the form tabs though. There is a following line that causes the switch to have the enforced policy tab as default if it is enabled:

      if ($config->get($policyTypeKey . '.enable')) {
        $form['policies']['#default_tab'] = 'edit-' . $policyTypeKey;
      }

I'm going to close this given that it hasn't received any +1s or follows in 2 years, and the URI report handler provides sufficient functionality.

I think having this as part of 2.1.0, after core support for 8.1 is dropped, since this isn't necessary to introduce in 2.0.0.

I think this change should be done alongside a 2.x pre-release version, and before 2.0.0

gapple → created an issue.

I think easier to require ^10.2 for 2.0.0, and skip backporting support for <=10.1 in the 8.x-1.x branch.

gapple → created an issue.

@mfb thanks for the info

Looks like this was changed behaviour in PHP 7.3, to default to splitting messages on newlines with the no-ctrl option (pre-7.3 behaviour can be configured with the raw option)
https://www.php.net/manual/en/errorfunc.configuration.php#ini.syslog.filter

CSP could detect the logging service and ini option to avoid pretty printing for syslog, but that seems like it could be better as an upstream option on the syslog service to strip newlines? AFAIK it's not uncommon for modules to send strings with newline to the logger, particularly with pre formatting?

(as a sidenote, the goal is to remove logging violations from CSP module, and recommend the Reporting API → module with logging to the database by default once that functionality is available)

I've got a helper class/method for adding nonces that I think is ready for merging to the CSP module: ✨ Add helper for safely appending nonce/hash sources Fixed

Something I want to check on, is that gin is currently still supporting core ^9 | ^10 while csp dev is currently dropping support for D9. Is it worth it for csp to include the new feature in a D9 compatible release to avoid upgrade conflicts for sites, or will gin be dropping D9 support for new releases soon as well?

I've got a helper class/method for adding nonces that I think is ready for merging to the CSP module: ✨ Add helper for safely appending nonce/hash sources Fixed

Something I want to check on, is that google_tag is currently still supporting core ^9.5 | ^10 while csp dev is currently dropping support for D9. Is it worth it for csp to include the new feature in a D9 compatible release to avoid upgrade conflicts for sites, or will google_tag be dropping D9 support for new releases soon as well?