[BUGFIX] Allow strict-dynamic only for applicable CSP directives

Using 'strict-dynamic' (SourceKeyword::strictDynamic) should only
be allowed for script-src*[1] directives. Using it for any other
directive produces strange side effects, since it implicitly adds
a CSP nonce value to that directive when compiling the policy.

Thus, this change
+ ensures 'strict-dynamic' is applicable for script-src* when
  using Policy::set, Policy::extend or the constructor
+ removes invalid policy purges when 'strict-dynamic' was used,
  since that was only valid for CSP level 3 - ignoring these
  directives is supposed to be handled by the browser[2], not by
  the server-side API, in order to preserve CSP v1/v2 BC.

[1] https://w3c.github.io/webappsec-csp/#allow-all-inline
> 'strict-dynamic' only applies to scripts, not other resource types.

[2] https://w3c.github.io/webappsec-csp/#strict-dynamic-usage
> 8.2. Usage of "'strict-dynamic'"
> If present in a script-src or default-src directive, it has two
> main effects:
> * host-source and scheme-source expressions, as well as the
>   "'unsafe-inline'" and "'self' keyword-sources will be ignored
>   when loading script.

Resolves: #101460
Releases: main, 12.4
Change-Id: I95f696b92b60efef42367c2536b93b855a52522d
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/80207
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
Tested-by: core-ci <typo3@b13.com>
Tested-by: Benjamin Franzke <ben@bnf.dev>
Reviewed-by: Benjamin Franzke <ben@bnf.dev>

typo3/sysext/core/Classes/Security/ContentSecurityPolicy/Policy.php

@@ -41,9 +41,11 @@ class Policy
     public function __construct(SourceCollection|SourceInterface ...$sources)
     {
         $this->directives = new Map();
+        $directive = Directive::DefaultSrc;
         $collection = $this->asMergedSourceCollection(...$sources);
+        $collection = $this->purgeNonApplicableSources($directive, $collection);
         if (!$collection->isEmpty()) {
-            $this->directives[Directive::DefaultSrc] = $collection;
+            $this->directives[$directive] = $collection;
         }
     }
 
@@ -88,6 +90,7 @@ public function default(SourceCollection|SourceInterface ...$sources): self
     public function extend(Directive $directive, SourceCollection|SourceInterface ...$sources): self
     {
         $collection = $this->asMergedSourceCollection(...$sources);
+        $collection = $this->purgeNonApplicableSources($directive, $collection);
         if ($collection->isEmpty()) {
             return $this;
         }
@@ -121,6 +124,7 @@ public function reduce(Directive $directive, SourceCollection|SourceInterface ..
     public function set(Directive $directive, SourceCollection|SourceInterface ...$sources): self
     {
         $collection = $this->asMergedSourceCollection(...$sources);
+        $collection = $this->purgeNonApplicableSources($directive, $collection);
         return $this->changeDirectiveSources($directive, $collection);
     }
 
@@ -180,8 +184,8 @@ public function prepare(): self
             }
         }
         foreach ($directives as $directive => $collection) {
-            // applies implicit changes to sources in case 'strict-dynamic' is used
-            if ($collection->contains(SourceKeyword::strictDynamic)) {
+            // applies implicit changes to sources in case 'strict-dynamic' is used for applicable directives
+            if ($collection->contains(SourceKeyword::strictDynamic) && SourceKeyword::strictDynamic->isApplicable($directive)) {
                 $directives[$directive] = SourceKeyword::strictDynamic->applySourceImplications($collection) ?? $collection;
             }
         }
@@ -286,4 +290,13 @@ protected function asMergedSourceCollection(SourceCollection|SourceInterface ...
         }
         return $target;
     }
+
+    protected function purgeNonApplicableSources(Directive $directive, SourceCollection $collection): SourceCollection
+    {
+        $sources = array_filter(
+            $collection->sources,
+            static fn (SourceInterface $source): bool => $source instanceof SourceKeyword ? $source->isApplicable($directive) : true
+        );
+        return new SourceCollection(...$sources);
+    }
 }

typo3/sysext/core/Classes/Security/ContentSecurityPolicy/SourceKeyword.php

@@ -51,25 +51,23 @@ public function isApplicable(Directive $directive): bool
             Directive::ScriptSrc, Directive::ScriptSrcAttr, Directive::ScriptSrcElem,
             Directive::StyleSrc, Directive::StyleSrcAttr, Directive::StyleSrcElem,
         ];
+        $onlyApplicableTo[self::strictDynamic] = [
+            Directive::ScriptSrc, Directive::ScriptSrcAttr, Directive::ScriptSrcElem,
+        ];
         return !isset($onlyApplicableTo[$this]) || in_array($directive, $onlyApplicableTo[$this], true);
     }
 
     public function applySourceImplications(SourceCollection $sources): ?SourceCollection
     {
-        if ($this !== self::strictDynamic) {
-            return null;
-        }
-        // adjust existing directives when using 'strict-dynamic' (e.g. for Google Maps)
-        // see https://www.w3.org/TR/CSP3/#strict-dynamic-usage
-        $newSources = $sources
-            ->without(SourceKeyword::self, SourceKeyword::unsafeInline)
-            ->withoutTypes(UriValue::class, SourceScheme::class);
-        if (!$sources->contains(self::nonceProxy) && !$sources->containsTypes(Nonce::class)) {
-            $newSources = $newSources->with(self::nonceProxy);
-        }
-        if ($sources === $newSources) {
-            return null;
+        // apply implications for `'strict-dynamic'`
+        if ($this === self::strictDynamic) {
+            // add nonce-proxy in case it's not defined
+            if (!$sources->contains(self::nonceProxy)
+                && !$sources->containsTypes(Nonce::class)
+            ) {
+                return $sources->with(self::nonceProxy);
+            }
         }
-        return $newSources;
+        return null;
     }
 }

typo3/sysext/core/Tests/Functional/Security/ContentSecurityPolicy/PolicyTest.php

@@ -144,6 +144,22 @@ public function nonceProxyIsCompiled(): void
         self::assertSame("default-src 'self' 'nonce-{$this->nonce->b64}'", $policy->compile($this->nonce));
     }
 
+    /**
+     * `strict-dynamic` is only allowed for `script-src*` and implicitly adds a `nonce-proxy`.
+     *
+     * @test
+     */
+    public function strictDynamicIsApplied(): void
+    {
+        $policy = (new Policy(SourceKeyword::self, SourceKeyword::strictDynamic))
+            ->extend(Directive::ScriptSrc, SourceKeyword::strictDynamic)
+            ->extend(Directive::StyleSrc, SourceKeyword::strictDynamic);
+        self::assertSame(
+            "default-src 'self'; script-src 'self' 'strict-dynamic' 'nonce-{$this->nonce->b64}'",
+            $policy->compile($this->nonce)
+        );
+    }
+
     /**
      * @test
      */