Allow CSP to be added by render elements

Created on 15 May 2024, about 1 month ago
Updated 24 May 2024, about 1 month ago

Problem/Motivation

Right now CSP supports adding a massive amount of headers upfront in configuration. However many of the headers are not actually needed on all pages.

Additionally, some `unsafe-` directives may be required, but not needed on all pages. Only on pages that need them, for example some media may need it.

Content should be able to inform CSP which directives are actually required on each page load, while keeping full cacheability of page content.

Some gripes I have with the Nonce service introduced in ✨ Add nonce service Fixed include its inability to automatically add directives to headers. And if the nonce was triggered during the lifetime of a regular controller response, then on subsequent cached responses, the headers are not added. Because the nonce trigger is no longer triggered

It should support regenerating nonces on every page.

Proposed resolution

I'm proposing including an appropriate utility and service for allowing controllers, i.e anything involving render arrays, to inform CSP of which headers to add to a response.

Pages must remain fully cacheable

Things like nonces must be rebuilt and substituted on every page load.

The proposal contains developer (PHP) only changes.

The code would not do anything, unless invoked. Except the policy subscriber listener, which would finish-fast.

Remaining tasks

Follow up tasks, particularly in other contrib modules, could make use of this new utility. Adding CSP when required. For example GTM, or Media plugin implementations.

User interface changes

Nil.

API changes

New utility.
New service for handling callbacks and hooks.

Data model changes

Nil.

----

Id like to acknowledge @larowlan and @acbramley for informing me about the Nonce service and the placeholdering strategy.

✨ Feature request
Status

Needs work

Version

2.0

Component

Code

Created by

🇦🇺Australia dpi Perth, Australia

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

Merge Requests

Comments & Activities

  • Issue created by @dpi
  • Merge request !37CSP On Demand → (Open) created by dpi
  • Pipeline finished with Failed
    about 1 month ago
    Total: 165s
    #173115
  • Pipeline finished with Success
    about 1 month ago
    Total: 169s
    #173133
  • Issue was unassigned.
  • Status changed to Needs review about 1 month ago
  • 🇦🇺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.

  • Pipeline finished with Success
    about 1 month ago
    Total: 180s
    #173138
  • Pipeline finished with Success
    about 1 month ago
    Total: 149s
    #173148
  • Pipeline finished with Success
    about 1 month ago
    Total: 199s
    #173169
  • Pipeline finished with Success
    about 1 month ago
    Total: 207s
    #173183
  • Status changed to Needs work about 1 month ago
  • 🇨🇦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.

Production build 0.69.0 2024