Merge pull request #11 from fheinze/use-object-structure-for-csp-rules

FEATURE: config object format for directives to support merging CSP config split into different files

Author: t-heuser

Classes/Command/CspConfigCommandController.php

@@ -31,7 +31,7 @@ class CspConfigCommandController extends CommandController
 
     /**
      * @Flow\InjectConfiguration(path="content-security-policy")
-     * @var string[][][]
+     * @var array<string, array<string, array<string|int, string|bool>>>
      */
     protected array $configuration;
 

Classes/Exceptions/DirectivesNormalizerException.php

@@ -0,0 +1,17 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Flowpack\ContentSecurityPolicy\Exceptions;
+
+use Neos\Flow\Exception;
+
+class DirectivesNormalizerException extends Exception
+{
+    public function __construct(string $reason)
+    {
+        parent::__construct(
+            "Invalid yaml config provided. {$reason} Please check your settings.",
+        );
+    }
+}

Classes/Factory/PolicyFactory.php

@@ -4,40 +4,74 @@
 
 namespace Flowpack\ContentSecurityPolicy\Factory;
 
+use Flowpack\ContentSecurityPolicy\Exceptions\DirectivesNormalizerException;
 use Flowpack\ContentSecurityPolicy\Exceptions\InvalidDirectiveException;
+use Flowpack\ContentSecurityPolicy\Helpers\DirectivesNormalizer;
 use Flowpack\ContentSecurityPolicy\Model\Nonce;
 use Flowpack\ContentSecurityPolicy\Model\Policy;
 use Neos\Flow\Annotations as Flow;
+use Psr\Log\LoggerInterface;
 
 /**
  * @Flow\Scope("singleton")
  */
 class PolicyFactory
 {
     /**
-     * @param string[][] $defaultDirectives
-     * @param string[][] $customDirectives
+     * @Flow\InjectConfiguration(path="throw-invalid-directive-exception")
+     */
+    protected bool $throwInvalidDirectiveException;
+
+    /**
+     * @Flow\Inject
+     */
+    protected LoggerInterface $logger;
+
+    /**
+     * @Flow\Inject
+     *
+     */
+
+    /**
+     * @param array<string, array<string|int, string|bool>> $defaultDirectives
+     * @param array<string, array<string|int, string|bool>> $customDirectives
      * @throws InvalidDirectiveException
+     * @throws DirectivesNormalizerException
      */
     public function create(Nonce $nonce, array $defaultDirectives, array $customDirectives): Policy
     {
-        $resultDirectives = $defaultDirectives;
-        foreach ($customDirectives as $key => $customDirective) {
+        $normalizedDefaultDirectives = DirectivesNormalizer::normalize($defaultDirectives, $this->logger);
+        $normalizedCustomDirectives = DirectivesNormalizer::normalize($customDirectives, $this->logger);
+
+        $resultDirectives = $normalizedDefaultDirectives;
+        foreach ($normalizedCustomDirectives as $key => $customDirective) {
             if (array_key_exists($key, $resultDirectives)) {
                 $resultDirectives[$key] = array_merge($resultDirectives[$key], $customDirective);
             } else {
                 // Custom directive is not present in default, still needs to be added.
                 $resultDirectives[$key] = $customDirective;
             }
-
             $resultDirectives[$key] = array_unique($resultDirectives[$key]);
         }
 
         $policy = new Policy();
         $policy->setNonce($nonce);
 
         foreach ($resultDirectives as $directive => $values) {
-            $policy->addDirective($directive, $values);
+            try {
+                $policy->addDirective($directive, $values);
+            } catch (InvalidDirectiveException $e
+            ) {
+                if ($this->throwInvalidDirectiveException) {
+                    // For development we want to make sure directives are configured correctly.
+                    throw $e;
+                } else {
+                    // In production we just log the error and continue. If a directive is invalid, we still
+                    // want to apply the rest of the policy.
+                    $this->logger->critical($e->getMessage());
+                    continue;
+                }
+            }
         }
 
         return $policy;

Classes/Helpers/DirectivesNormalizer.php

@@ -0,0 +1,71 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Flowpack\ContentSecurityPolicy\Helpers;
+
+use Flowpack\ContentSecurityPolicy\Exceptions\DirectivesNormalizerException;
+use Psr\Log\LoggerInterface;
+
+/**
+ * Helper to support normalization of directives from different formats.
+ * The old format supported yaml lists. Now key-value pairs should be used for directives.
+ * In the future we will deprecate the list format!
+ *
+ * We also cleanup of empty directives and entries here before further processing.
+ */
+class DirectivesNormalizer
+{
+    /**
+     * @param array<string, array<string|int, string|bool>> $directives
+     * @return string[][]
+     * @throws DirectivesNormalizerException
+     */
+    public static function normalize(array $directives, LoggerInterface $logger): array
+    {
+        $result = [];
+        // directives e.g. script-src:
+        foreach ($directives as $directive => $values) {
+            if(is_array($values) && sizeof($values) > 0) {
+                $normalizedValues = [];
+                $firstKeyType = null;
+                // values e.g. 'self', 'unsafe-inline' OR key-value pairs e.g. example.com: true
+                foreach ($values as $key => $value) {
+                    $keyType = gettype($key);
+                    $valueType = gettype($value);
+                    if ($firstKeyType === null) {
+                        $firstKeyType = $keyType;
+                    } else {
+                        if ($keyType !== $firstKeyType) {
+                            // we do not allow mixed key types -> this should be marked as an error in the IDE as well
+                            // as Flow should throw an exception here. But just to be sure, we add this check.
+                            throw new DirectivesNormalizerException('Directives must be defined as a list OR an object.');
+                        }
+                    }
+
+                    if($keyType === 'integer' && $valueType === 'string' && !empty($value)) {
+                        // old configuration format using list
+                        $normalizedValues[] = $value;
+                        $logger->warning('Using list format for CSP directives is deprecated and will be removed in future versions. Please use key-value pairs with boolean values instead.');
+                    } elseif($keyType === 'string') {
+                        // new configuration format using key-value pairs
+                        if($valueType === 'boolean') {
+                            if($value === true && !empty($key)) {
+                                $normalizedValues[] = $key;
+                            }
+                        } else {
+                            // We chose a format similar to NodeType constraints yaml configuration.
+                            throw new DirectivesNormalizerException('When using keys in your yaml, the values must be boolean.');
+                        }
+                    }
+                }
+                if(!empty($normalizedValues)) {
+                    // we also clean up empty directives here
+                    $result[$directive] = $normalizedValues;
+                }
+            }
+        }
+
+        return $result;
+    }
+}

Classes/Http/CspHeaderMiddleware.php

@@ -5,6 +5,7 @@
 namespace Flowpack\ContentSecurityPolicy\Http;
 
 use Exception;
+use Flowpack\ContentSecurityPolicy\Exceptions\DirectivesNormalizerException;
 use Flowpack\ContentSecurityPolicy\Exceptions\InvalidDirectiveException;
 use Flowpack\ContentSecurityPolicy\Factory\PolicyFactory;
 use Flowpack\ContentSecurityPolicy\Helpers\TagHelper;
@@ -16,7 +17,6 @@
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Server\MiddlewareInterface;
 use Psr\Http\Server\RequestHandlerInterface;
-use Psr\Log\LoggerInterface;
 
 class CspHeaderMiddleware implements MiddlewareInterface
 {
@@ -27,11 +27,6 @@ class CspHeaderMiddleware implements MiddlewareInterface
      */
     protected bool $enabled;
 
-    /**
-     * @Flow\Inject
-     */
-    protected LoggerInterface $logger;
-
     /**
      * @Flow\Inject
      */
@@ -44,12 +39,14 @@ class CspHeaderMiddleware implements MiddlewareInterface
 
     /**
      * @Flow\InjectConfiguration(path="content-security-policy")
-     * @var string[][][]
+     * @var array<string, array<string, array<string|int, string|bool>>>
      */
     protected array $configuration;
 
     /**
      * @inheritDoc
+     * @throws InvalidDirectiveException
+     * @throws DirectivesNormalizerException
      */
     public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
     {
@@ -58,13 +55,7 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
             return $response;
         }
 
-        try {
-            $policy = $this->getPolicyByCurrentContext($request);
-        } catch (Exception $exception) {
-            $this->logger->critical($exception->getMessage(), ['exception' => $exception]);
-
-            return $response;
-        }
+        $policy = $this->getPolicyByCurrentContext($request);
 
         if ($policy->hasNonceDirectiveValue()) {
             $body = $response->getBody();
@@ -78,6 +69,7 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
 
     /**
      * @throws InvalidDirectiveException
+     * @throws DirectivesNormalizerException
      */
     private function getPolicyByCurrentContext(ServerRequestInterface $request): Policy
     {
@@ -131,7 +123,7 @@ private function checkTagAndReplaceUsingACallback(
             function ($hits) use ($hitCallback) {
                 $tagMarkup = $hits[0];
                 $tagName = $hits[1];
-                
+
                 return call_user_func(
                     $hitCallback,
                     $tagMarkup,

Classes/Model/Policy.php

@@ -6,6 +6,7 @@
 
 use Flowpack\ContentSecurityPolicy\Exceptions\InvalidDirectiveException;
 use Neos\Flow\Annotations as Flow;
+use Psr\Log\LoggerInterface;
 
 class Policy
 {

Configuration/Production/Settings.yaml

@@ -0,0 +1,3 @@
+Flowpack:
+  ContentSecurityPolicy:
+    throw-invalid-directive-exception: false

Configuration/Settings.yaml

@@ -2,69 +2,70 @@ Flowpack:
   ContentSecurityPolicy:
     enabled: true
     report-only: false
+    throw-invalid-directive-exception: true
     content-security-policy:
       default:
         base-uri:
-          - 'self'
+         'self': true
         connect-src:
-          - 'self'
+          'self': true
         default-src:
-          - 'self'
+          'self': true
         form-action:
-          - 'self'
+          'self': true
         img-src:
-          - 'self'
+          'self': true
         media-src:
-          - 'self'
+          'self': true
         frame-src:
-          - 'self'
+          'self': true
         object-src:
-          - 'self'
+          'self': true
         script-src:
-          - 'self'
+          'self': true
         style-src:
-          - 'self'
+          'self': true
         style-src-attr:
-          - 'self'
+          'self': true
         style-src-elem:
-          - 'self'
+          'self': true
         font-src:
-          - 'self'
+          'self': true
       custom: [ ]
       backend:
         base-uri:
-          - 'self'
+          'self': true
         connect-src:
-          - 'self'
+          'self': true
         default-src:
-          - 'self'
+          'self': true
         form-action:
-          - 'self'
+          'self': true
         img-src:
-          - 'self'
-          - 'data:'
+          'self': true
+          'data:': true
         media-src:
-          - 'self'
+          'self': true
         frame-src:
-          - 'self'
+          'self': true
         object-src:
-          - 'self'
+          'self': true
         script-src:
-          - 'self'
-          - 'unsafe-inline'
-          - 'unsafe-eval'
+          'self': true
+          'unsafe-inline': true
+          'unsafe-eval': true
         style-src:
-          - 'self'
-          - 'unsafe-inline'
+          'self': true
+          'unsafe-inline': true
         style-src-attr:
-          - 'self'
-          - 'unsafe-inline'
+          'self': true
+          'unsafe-inline': true
         style-src-elem:
-          - 'self'
-          - 'unsafe-inline'
+          'self': true
+          'unsafe-inline': true
         font-src:
-          - 'self'
-          - 'data:'
+          'self': true
+          'data:': true
       custom-backend: [ ]
 
 Neos: