- Issue created by @dpi
- Issue was unassigned.
- Status changed to Needs review
10 months ago 8:40am 15 May 2024 - 🇦🇺Australia dpi Perth, Australia
This code set requires 10.2, which will be the lowest supported version of Drupal in one month. People using unsupported versions of Drupal can continue to use existing CSP releases.
Code utilises these changes:
- https://www.drupal.org/node/3357408 →
- https://www.drupal.org/node/3349470 →
- https://www.drupal.org/node/3395436 →Note: lint for 80 chars is incorrect, 120 already passed ✨ Drupal.Arrays.Array.LongLineDeclaration make me write less readable code Fixed . CI has an older coder I guess.
- Status changed to Needs work
10 months ago 11:05am 17 May 2024 - 🇨🇦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.
- 🇨🇦Canada gapple
I've got an MR (against 8.x-1.x) in ✨ Provides a filter to add nonce attribute to inline scripts. Needs work that enables adding directives through #attached like this:
$element = [ '#attached' => [ 'csp' => [ 'script-src' => 'https://example.com', ], ], ];
I think it will work fine with Big Pipe, but I need to confirm that. The additional attached data needs to bubble up so it can be applied to the page header before Big Pipe renders its placeholdered content, so as long as the csp data isn't attached to content rendered within a lazy_builder callback it should be fine.
I'll extract that for 2.1.0 in this issue. I'll use some of the work in MR #37 separately (I like the possibility of using enums 🙂), but I should focus on getting a stable 2.0.0 release first.
- 🇦🇺Australia dpi Perth, Australia
Thanks @gapple, unfortunately I've been busy on other things so I havn't read above, and I'm good to delay work for now, as I'm using the original code happily on a project.
- First commit to issue fork.
- Assigned to gapple
- 🇨🇦Canada gapple
New MR to extract the
#attached
functionality from ✨ Provides a filter to add nonce attribute to inline scripts. Needs work , so that a render element could define sources to append to directives, or use a placeholder to replace with a nonce value:$element[ '#attached' => [ 'csp' => [ 'img-src' => ['cdn.example.com'], 'style-src' => [Csp::POLICY_UNSAFE_INLINE], 'style-src-elem' => [Csp::POLICY_UNSAFE_INLINE], ], 'csp_nonce' => [ 'script' => Csp::POLICY_UNSAFE_INLINE, ], 'placeholders' => [ $this->nonceBuilder->getPlaceholderKey() => [ '#lazy_builder' => ['csp.nonce_builder:renderNonce', []], ], ], ], ];
- Still needs tests
- A fallback cannot be provided for hash values - Status changed to Needs review
7 months ago 9:32am 24 July 2024 - 🇨🇦Canada gapple
Added tests, and also a
csp_hash
option.The format for each is a little different
'csp' => [ 'any directive' => ['source'], 'csp_nonce' => [ 'script or style' => ['fallback'], 'csp_hash' => [ 'script or style directive' => [ 'hash' => ['fallback'], ], ]
$element = [ '#attached' => [ 'csp' => [ 'img-src' => ['cdn.example.com'], 'style-src' => [Csp::POLICY_UNSAFE_INLINE], 'style-src-elem' => [Csp::POLICY_UNSAFE_INLINE], ], 'csp_nonce' => [ 'script' => Csp::POLICY_UNSAFE_INLINE, ], 'placeholders' => [ $this->nonceBuilder->getPlaceholderKey() => [ '#lazy_builder' => ['csp.nonce_builder:renderNonce', []], ], ], 'csp_hash' => [ 'sha256-abcdef' => [Csp::POLICY_UNSAFE_INLINE], ], ],
- Status changed to Needs work
7 months ago 2:55am 16 August 2024 - 🇨🇦Canada gapple
Feature has been merged; Changing status to Needs Work for writing a Change Notice and updating documentation.
I'll probably do a 2.1.0-beta1 release with some other changes after those are complete.Minimum version has been bumped to 10.1, in order to use the
TrustedCallback
attribute.Opened some additional issues for other changes that were in the original merge request, to be added to future releases
🌱 Use Autowire & Autoconfigure for services Active
🌱 Use Enums Active - Status changed to Fixed
7 months ago 11:24pm 16 August 2024 - 🇨🇦Canada gapple
Change Notice and Docs update are complete
https://www.drupal.org/node/3468616 →
https://www.drupal.org/docs/extending-drupal/contributed-modules/contrib... → Automatically closed - issue fixed for 2 weeks with no activity.