When altering a policy, implementations need to consider fallback directives, duplicating and modifying when necessary.

For example, if only default-src 'self' is enabled, but style-src-elem 'unsafe-inline' needs to be added:
- Just enabling style-src-elem with the necessary directive would cause problems, by now blocking 'self'
- Adding 'unsafe-inline' to default-src would also enabled it for script-src
- style-src also needs to be enabled for browsers which don't yet implement style-src-elem

This requires the following template:

      if ($policy->hasDirective('style-src')) {
        $policy->appendDirective('style-src', [Csp::POLICY_UNSAFE_INLINE]);
      }
      elseif ($policy->hasDirective('default-src')) {
        $scriptDirective = array_merge($policy->getDirective('default-src'), [Csp::POLICY_UNSAFE_INLINE]);
        $policy->setDirective('style-src', $scriptDirective);
      }

      if ($policy->hasDirective('style-src-elem')) {
        $policy->appendDirective('style-src-elem', [Csp::POLICY_UNSAFE_INLINE]);
      }
      elseif ($policy->hasDirective('style-src')) {
        $scriptDirective = array_merge($policy->getDirective('style-src'), [Csp::POLICY_UNSAFE_INLINE]);
        $policy->setDirective('style-src-elem', $scriptDirective);
      }
      // If default-src is set, style-src was already created above if
      // necessary, so no need to fallback further for style-src-elem.

which could probably be made available as a helper method:

  Csp::appendWithFallback($policy, 'style-src-elem', [Csp::POLICY_UNSAFE_INLINE]);
CommentFileSizeAuthor
#5 csp-3099423-5.patch5.21 KBgapple
#2 csp-3099423-2.patch3.92 KBgapple

Comments

gapple created an issue. See original summary.

gapple’s picture

gapple
he/they
Primary language English
commented
Status: Active » Needs review
StatusFileSize
new csp-3099423-2.patch3.92 KB

fallbackAwareAppendIfEnabled() feels like an awkward method name, but I think it works.

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented

I did not review but big +1 for DX.

Also, should existing event subscribers in the modules use this new method?

gapple’s picture

gapple
he/they
Primary language English
commented

A potential issue with the current patch is handling if 'none' is set on a fallback directive. The module takes a strict approach when optimizing the sources, removing all other sources if 'none' is present (This appears to differ from current browser interpretations, which ignore 'none' if another source is present).

e.g.
- Site is configured with default-src 'none', and font-src is not enabled.
- A theme wants to enable font-src https://fonts.gstatic.com
- The helper method will copy the default-src value before appending the additional source, resulting in default-src 'none'; font-src 'none' https://fonts.gstatic.com
- The module will reduce the policy to default-src 'none'; font-src 'none' (and then because they are identical, just default-src 'none')

This could be worked around by enabling the font-src directive in configuration with an empty value. If no sources are added to the directive it will be omitted (and fallback to default-src), and if a source is added then the fallback policy will not be copied.

This should be mitigated in many cases because default-src 'none' will require enabling sources on more specific directives. For example script-src 'self' must be enabled at a minimum or functionality will be broken. This may be more likely with something like frame-src 'none' to disable iframes by default, which would also prevent any page-specific modifications from allowing an iframe on a particular page.

gapple’s picture

gapple
he/they
Primary language English
commented
StatusFileSize
new csp-3099423-5.patch5.21 KB

This patch adds handling of 'none': if a fallback directive includes it, then the specified directive is initialized with an empty source list so that only the new value is set.

E.g.

  $policy = new Csp();
  $policy->setDirective('default-src', [Csp::POLICY_NONE, 'https://example.org']);
  $policy->fallbackAwareAppendIfEnabled(
    'script-src',
    [Csp::POLICY_SELF]
  );
  $policy->fallbackAwareAppendIfEnabled(
    'script-src-attr',
    [Csp::POLICY_UNSAFE_INLINE]
  );

will result in the policy default-src 'none'; script-src 'self'; script-src-attr 'self' 'unsafe-inline'

----

If a directive is enabled in the module config with an empty value (or via another alter subscriber), then the fallback directive's value will not be copied

E.g.

  $policy = new Csp();
  $policy->setDirective('default-src', [Csp::POLICY_NONE]);
  $policy->setDirective('script-src', [Csp::POLICY_SELF]);
  $policy->setDirective('script-src-attr', []);
  $policy->fallbackAwareAppendIfEnabled(
    'script-src-attr',
    [Csp::POLICY_UNSAFE_INLINE]
  );

will result in the policy default-src 'none'; script-src 'self'; script-src-attr 'unsafe-inline'

  • gapple committed 8c582d4 on 8.x-1.x 
    Issue #3099423: Helper for appending sources to directives with fallback
gapple’s picture

gapple
he/they
Primary language English
commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.