Ensured csp sanitises tags using a hash only if the visited page is cached

app/code/Magento/Csp/Helper/InlineUtil.php

@@ -9,8 +9,9 @@
 namespace Magento\Csp\Helper;
 
 use Magento\Csp\Api\InlineUtilInterface;
-use Magento\Csp\Model\Collector\ConfigCollector;
 use Magento\Csp\Model\Collector\DynamicCollector;
+use Magento\Csp\Model\InlineUtil\HashGenerator;
+use Magento\Csp\Model\InlineUtil\InlineWhitelistStategy;
 use Magento\Csp\Model\Policy\FetchPolicy;
 use Magento\Framework\App\ObjectManager;
 use Magento\Framework\View\Helper\SecureHtmlRender\EventHandlerData;
@@ -40,11 +41,6 @@ class InlineUtil implements InlineUtilInterface, SecurityProcessorInterface
      */
     private $htmlRenderer;
 
-    /**
-     * @var ConfigCollector
-     */
-    private $configCollector;
-
     private static $tagMeta = [
         'script' => ['id' => 'script-src', 'remote' => ['src'], 'hash' => true],
         'style' => ['id' => 'style-src', 'remote' => [], 'hash' => true],
@@ -62,33 +58,34 @@ class InlineUtil implements InlineUtilInterface, SecurityProcessorInterface
         'frame' => ['id' => 'frame-src', 'remote' => ['src']]
     ];
 
+    /**
+     * @var InlineWhitelistStategy
+     */
+    private InlineWhitelistStategy $inlineWhitelistStategy;
+
+    /**
+     * @var HashGenerator
+     */
+    private HashGenerator $hashGenerator;
+
     /**
      * @param DynamicCollector $dynamicCollector
+     * @param InlineWhitelistStategy $inlineWhitelistStategy
      * @param bool $useUnsafeHashes Use 'unsafe-hashes' policy (not supported by CSP v2).
      * @param HtmlRenderer|null $htmlRenderer
-     * @param ConfigCollector|null $configCollector
      */
     public function __construct(
-        DynamicCollector $dynamicCollector,
-        bool $useUnsafeHashes = false,
-        ?HtmlRenderer $htmlRenderer = null,
-        ?ConfigCollector $configCollector = null
+        DynamicCollector       $dynamicCollector,
+        InlineWhitelistStategy $inlineWhitelistStategy,
+        HashGenerator $hashGenerator,
+        bool                   $useUnsafeHashes = false,
+        ?HtmlRenderer          $htmlRenderer = null
     ) {
         $this->dynamicCollector = $dynamicCollector;
         $this->useUnsafeHashes = $useUnsafeHashes;
         $this->htmlRenderer = $htmlRenderer ?? ObjectManager::getInstance()->get(HtmlRenderer::class);
-        $this->configCollector = $configCollector ?? ObjectManager::getInstance()->get(ConfigCollector::class);
-    }
-
-    /**
-     * Generate fetch policy hash for some content.
-     *
-     * @param string $content
-     * @return array Hash data to insert into a FetchPolicy.
-     */
-    private function generateHashValue(string $content): array
-    {
-        return [base64_encode(hash('sha256', $content, true)) => 'sha256'];
+        $this->inlineWhitelistStategy = $inlineWhitelistStategy;
+        $this->hashGenerator = $hashGenerator;
     }
 
     /**
@@ -196,23 +193,8 @@ public function processTag(TagData $tagData): TagData
                     new FetchPolicy($policyId, false, $remotes)
                 );
             }
-            if ($tagData->getContent()
-                && !empty(self::$tagMeta[$tagData->getTag()]['hash'])
-                && $this->isInlineDisabled(self::$tagMeta[$tagData->getTag()]['id'])
-            ) {
-                $this->dynamicCollector->add(
-                    new FetchPolicy(
-                        $policyId,
-                        false,
-                        [],
-                        [],
-                        false,
-                        false,
-                        false,
-                        [],
-                        $this->generateHashValue($tagData->getContent())
-                    )
-                );
+            if (!empty(self::$tagMeta[$tagData->getTag()]['hash'])) {
+                $tagData = $this->inlineWhitelistStategy->getSanitisedTag($tagData, $policyId);
             }
         }
 
@@ -234,7 +216,7 @@ public function processEventHandler(EventHandlerData $eventHandlerData): EventHa
                 false,
                 false,
                 [],
-                $this->generateHashValue($eventHandlerData->getCode()),
+                $this->hashGenerator->generateHashValue($eventHandlerData->getCode()),
                 false,
                 true
             );
@@ -245,21 +227,4 @@ public function processEventHandler(EventHandlerData $eventHandlerData): EventHa
 
         return $eventHandlerData;
     }
-
-    /**
-     * Check if inline sources are prohibited.
-     *
-     * @param string $policyId
-     * @return bool
-     */
-    private function isInlineDisabled(string $policyId): bool
-    {
-        foreach ($this->configCollector->collect() as $policy) {
-            if ($policy->getId() === $policyId && $policy instanceof FetchPolicy) {
-                return !$policy->isInlineAllowed();
-            }
-        }
-
-        return false;
-    }
 }

app/code/Magento/Csp/Model/InlineUtil/CspNonceProvider.php

@@ -0,0 +1,86 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+
+declare(strict_types=1);
+
+namespace Magento\Csp\Model\InlineUtil;
+
+use Magento\Csp\Model\Collector\DynamicCollector;
+use Magento\Csp\Model\Policy\FetchPolicy;
+use Magento\Framework\Exception\LocalizedException;
+use Magento\Framework\Math\Random;
+
+/**
+ * This helper class is used to provide nonce for CSP
+ *
+ * It also adds a nonce to the CSP header.
+ */
+class CspNonceProvider
+{
+    /**
+     * @var string
+     */
+    private const NONCE_LENGTH = 32;
+
+    /**
+     * @var string
+     */
+    private string $nonce;
+
+    /**
+     * @var Random
+     */
+    private Random $random;
+
+    /**
+     * @var DynamicCollector
+     */
+    private DynamicCollector $dynamicCollector;
+
+    /**
+     * @param Random $random
+     * @param DynamicCollector $dynamicCollector
+     */
+    public function __construct(
+        Random $random,
+        DynamicCollector $dynamicCollector
+    ) {
+        $this->random = $random;
+        $this->dynamicCollector = $dynamicCollector;
+    }
+
+    /**
+     * Generate nonce and add it to the CSP header
+     *
+     * @return string
+     * @throws LocalizedException
+     */
+    public function generateNonce(): string
+    {
+        if (empty($this->nonce)) {
+            $this->nonce = $this->random->getRandomString(
+                self::NONCE_LENGTH,
+                Random::CHARS_DIGITS . Random::CHARS_LOWERS
+            );
+
+            $policy = new FetchPolicy(
+                'script-src',
+                false,
+                [],
+                [],
+                false,
+                false,
+                false,
+                [$this->nonce],
+                []
+            );
+
+            $this->dynamicCollector->add($policy);
+        }
+
+        return base64_encode($this->nonce);
+    }
+}

app/code/Magento/Csp/Model/InlineUtil/HashGenerator.php

@@ -0,0 +1,18 @@
+<?php
+declare(strict_types=1);
+
+namespace Magento\Csp\Model\InlineUtil;
+
+class HashGenerator
+{
+    /**
+     * Generate fetch policy hash for some content.
+     *
+     * @param string $content
+     * @return array Hash data to insert into a FetchPolicy.
+     */
+    public function generateHashValue(string $content): array
+    {
+        return [base64_encode(hash('sha256', $content, true)) => 'sha256'];
+    }
+}

app/code/Magento/Csp/Model/InlineUtil/InlineWhitelistStategy.php

@@ -0,0 +1,108 @@
+<?php
+declare(strict_types=1);
+
+namespace Magento\Csp\Model\InlineUtil;
+
+use Magento\Csp\Model\Collector\ConfigCollector;
+use Magento\Csp\Model\Collector\DynamicCollector;
+use Magento\Csp\Model\Policy\FetchPolicy;
+use Magento\Framework\App\ObjectManager;
+use Magento\Framework\Exception\LocalizedException;
+use Magento\Framework\View\Helper\SecureHtmlRender\TagData;
+use Magento\Framework\View\LayoutInterface;
+
+class InlineWhitelistStategy
+{
+    private ?CspNonceProvider $nonceProvider;
+    private LayoutInterface $generalLayout;
+    private DynamicCollector $dynamicCollector;
+    private ?ConfigCollector $configCollector;
+    private HashGenerator $hashGenerator;
+
+    /**
+     * @param LayoutInterface $generalLayout
+     * @param DynamicCollector $dynamicCollector
+     * @param HashGenerator $hashGenerator
+     * @param ConfigCollector|null $configCollector
+     * @param CspNonceProvider|null $nonceProvider
+     */
+    public function __construct(
+        LayoutInterface   $generalLayout,
+        DynamicCollector  $dynamicCollector,
+        HashGenerator     $hashGenerator,
+        ?ConfigCollector  $configCollector = null,
+        ?CspNonceProvider $nonceProvider = null
+    ) {
+        $this->nonceProvider = $nonceProvider ?? ObjectManager::getInstance()->get(CspNonceProvider::class);
+        $this->configCollector = $configCollector ?? ObjectManager::getInstance()->get(ConfigCollector::class);
+        $this->generalLayout = $generalLayout;
+        $this->dynamicCollector = $dynamicCollector;
+        $this->hashGenerator = $hashGenerator;
+    }
+
+    /**
+     * Return a tag that is safe to be used
+     *
+     * @param TagData $tagData
+     * @param string $policyId
+     * @return TagData
+     * @throws LocalizedException
+     */
+    public function getSanitisedTag(TagData $tagData, string $policyId): TagData
+    {
+        if (!$tagData->getContent()
+            || !$this->isInlineDisabled($policyId)
+        ) {
+            return $tagData;
+        }
+
+        if ($tagData->getTag() === 'script'
+            && !$this->generalLayout->isCacheable()
+        ) {
+            $nonce = $this->nonceProvider->generateNonce();
+            $tagAttributes = $tagData->getAttributes();
+            $tagAttributes['nonce'] = $nonce;
+            $newTagData = new TagData(
+                $tagData->getTag(),
+                $tagAttributes,
+                $tagData->getContent(),
+                $tagData->isTextContent()
+            );
+
+            $tagData = $newTagData;
+        } else {
+            $this->dynamicCollector->add(
+                new FetchPolicy(
+                    $policyId,
+                    false,
+                    [],
+                    [],
+                    false,
+                    false,
+                    false,
+                    [],
+                    $this->hashGenerator->generateHashValue($tagData->getContent())
+                )
+            );
+        }
+
+        return $tagData;
+    }
+
+    /**
+     * Check if inline sources are prohibited.
+     *
+     * @param string $policyId
+     * @return bool
+     */
+    private function isInlineDisabled(string $policyId): bool
+    {
+        foreach ($this->configCollector->collect() as $policy) {
+            if ($policy->getId() === $policyId && $policy instanceof FetchPolicy) {
+                return !$policy->isInlineAllowed();
+            }
+        }
+
+        return false;
+    }
+}