Updated CSP with frame-src rules

- Configurable via 'ALLOWED_IFRAME_SOURCES' .env option.
- Also updated how CSP rules are set, with a single header being used
  instead of many.
- Also applied CSP rules to HTML export outputs.
- Updated tests to cover.

For #3314

.env.example.complete

@@ -331,6 +331,13 @@ ALLOW_UNTRUSTED_SERVER_FETCHING=false
 # Setting this option will also auto-adjust cookies to be SameSite=None.
 ALLOWED_IFRAME_HOSTS=null
 
+# A list of sources/hostnames that can be loaded within iframes within BookStack.
+# Space separated if multiple. BookStack host domain is auto-inferred.
+# Can be set to a lone "*" to allow all sources for iframe content (Not advised).
+# Defaults to a set of common services.
+# Current host and source for the "DRAWIO" setting will be auto-appended to the sources configured.
+ALLOWED_IFRAME_SOURCES="https://*.draw.io https://*.youtube.com https://*.youtube-nocookie.com https://*.vimeo.com"
+
 # The default and maximum item-counts for listing API requests.
 API_DEFAULT_ITEM_COUNT=100
 API_MAX_ITEM_COUNT=500

app/Config/app.php

@@ -57,6 +57,13 @@ return [
     // Space separated if multiple. BookStack host domain is auto-inferred.
     'iframe_hosts' => env('ALLOWED_IFRAME_HOSTS', null),
 
+    // A list of sources/hostnames that can be loaded within iframes within BookStack.
+    // Space separated if multiple. BookStack host domain is auto-inferred.
+    // Can be set to a lone "*" to allow all sources for iframe content (Not advised).
+    // Defaults to a set of common services.
+    // Current host and source for the "DRAWIO" setting will be auto-appended to the sources configured.
+    'iframe_sources' => env('ALLOWED_IFRAME_SOURCES', 'https://*.draw.io https://*.youtube.com https://*.youtube-nocookie.com https://*.vimeo.com'),
+
     // Application timezone for back-end date functions.
     'timezone' => env('APP_TIMEZONE', 'UTC'),
 

app/Entities/Tools/ExportFormatter.php

@@ -7,6 +7,7 @@ use BookStack\Entities\Models\Chapter;
 use BookStack\Entities\Models\Page;
 use BookStack\Entities\Tools\Markdown\HtmlToMarkdown;
 use BookStack\Uploads\ImageService;
+use BookStack\Util\CspService;
 use DOMDocument;
 use DOMElement;
 use DOMXPath;
@@ -15,16 +16,18 @@ use Throwable;
 
 class ExportFormatter
 {
-    protected $imageService;
-    protected $pdfGenerator;
+    protected ImageService $imageService;
+    protected PdfGenerator $pdfGenerator;
+    protected CspService $cspService;
 
     /**
      * ExportService constructor.
      */
-    public function __construct(ImageService $imageService, PdfGenerator $pdfGenerator)
+    public function __construct(ImageService $imageService, PdfGenerator $pdfGenerator, CspService $cspService)
     {
         $this->imageService = $imageService;
         $this->pdfGenerator = $pdfGenerator;
+        $this->cspService = $cspService;
     }
 
     /**
@@ -39,6 +42,7 @@ class ExportFormatter
         $pageHtml = view('pages.export', [
             'page'       => $page,
             'format'     => 'html',
+            'cspContent' => $this->cspService->getCspMetaTagValue(),
         ])->render();
 
         return $this->containHtml($pageHtml);
@@ -59,6 +63,7 @@ class ExportFormatter
             'chapter'    => $chapter,
             'pages'      => $pages,
             'format'     => 'html',
+            'cspContent' => $this->cspService->getCspMetaTagValue(),
         ])->render();
 
         return $this->containHtml($html);
@@ -76,6 +81,7 @@ class ExportFormatter
             'book'         => $book,
             'bookChildren' => $bookTree,
             'format'       => 'html',
+            'cspContent'   => $this->cspService->getCspMetaTagValue(),
         ])->render();
 
         return $this->containHtml($html);

app/Http/Middleware/ApplyCspRules.php

@@ -8,10 +8,7 @@ use Illuminate\Http\Request;
 
 class ApplyCspRules
 {
-    /**
-     * @var CspService
-     */
-    protected $cspService;
+    protected CspService $cspService;
 
     public function __construct(CspService $cspService)
     {
@@ -35,10 +32,8 @@ class ApplyCspRules
 
         $response = $next($request);
 
-        $this->cspService->setFrameAncestors($response);
-        $this->cspService->setScriptSrc($response);
-        $this->cspService->setObjectSrc($response);
-        $this->cspService->setBaseUri($response);
+        $cspHeader = $this->cspService->getCspHeader();
+        $response->headers->set('Content-Security-Policy', $cspHeader, false);
 
         return $response;
     }

app/Util/CspService.php

@@ -3,12 +3,10 @@
 namespace BookStack\Util;
 
 use Illuminate\Support\Str;
-use Symfony\Component\HttpFoundation\Response;
 
 class CspService
 {
-    /** @var string */
-    protected $nonce;
+    protected string $nonce;
 
     public function __construct(string $nonce = '')
     {
@@ -24,37 +22,34 @@ class CspService
     }
 
     /**
-     * Sets CSP 'script-src' headers to restrict the forms of script that can
-     * run on the page.
+     * Get the CSP headers for the application
      */
-    public function setScriptSrc(Response $response)
+    public function getCspHeader(): string
     {
-        if (config('app.allow_content_scripts')) {
-            return;
-        }
-
-        $parts = [
-            'http:',
-            'https:',
-            '\'nonce-' . $this->nonce . '\'',
-            '\'strict-dynamic\'',
+        $headers = [
+            $this->getFrameAncestors(),
+            $this->getFrameSrc(),
+            $this->getScriptSrc(),
+            $this->getObjectSrc(),
+            $this->getBaseUri(),
         ];
 
-        $value = 'script-src ' . implode(' ', $parts);
-        $response->headers->set('Content-Security-Policy', $value, false);
+        return implode('; ', array_filter($headers));
     }
 
     /**
-     * Sets CSP "frame-ancestors" headers to restrict the hosts that BookStack can be
-     * iframed within. Also adjusts the cookie samesite options so that cookies will
-     * operate in the third-party context.
+     * Get the CSP rules for the application for a HTML meta tag.
      */
-    public function setFrameAncestors(Response $response)
+    public function getCspMetaTagValue(): string
     {
-        $iframeHosts = $this->getAllowedIframeHosts();
-        array_unshift($iframeHosts, "'self'");
-        $cspValue = 'frame-ancestors ' . implode(' ', $iframeHosts);
-        $response->headers->set('Content-Security-Policy', $cspValue, false);
+        $headers = [
+            $this->getFrameSrc(),
+            $this->getScriptSrc(),
+            $this->getObjectSrc(),
+            $this->getBaseUri(),
+        ];
+
+        return implode('; ', array_filter($headers));
     }
 
     /**
@@ -66,25 +61,65 @@ class CspService
     }
 
     /**
-     * Sets CSP 'object-src' headers to restrict the types of dynamic content
-     * that can be embedded on the page.
+     * Create CSP 'script-src' rule to restrict the forms of script that can run on the page.
      */
-    public function setObjectSrc(Response $response)
+    protected function getScriptSrc(): string
     {
         if (config('app.allow_content_scripts')) {
-            return;
+            return '';
         }
 
-        $response->headers->set('Content-Security-Policy', 'object-src \'self\'', false);
+        $parts = [
+            'http:',
+            'https:',
+            '\'nonce-' . $this->nonce . '\'',
+            '\'strict-dynamic\'',
+        ];
+
+        return 'script-src ' . implode(' ', $parts);
     }
 
     /**
-     * Sets CSP 'base-uri' headers to restrict what base tags can be set on
+     * Create CSP "frame-ancestors" rule to restrict the hosts that BookStack can be iframed within.
+     */
+    protected function getFrameAncestors(): string
+    {
+        $iframeHosts = $this->getAllowedIframeHosts();
+        array_unshift($iframeHosts, "'self'");
+        return 'frame-ancestors ' . implode(' ', $iframeHosts);
+    }
+
+    /**
+     * Creates CSP "frame-src" rule to restrict what hosts/sources can be loaded
+     * within iframes to provide an allow-list-style approach to iframe content.
+     */
+    protected function getFrameSrc(): string
+    {
+        $iframeHosts = $this->getAllowedIframeSources();
+        array_unshift($iframeHosts, "'self'");
+        return 'frame-src ' . implode(' ', $iframeHosts);
+    }
+
+    /**
+     * Creates CSP 'object-src' rule to restrict the types of dynamic content
+     * that can be embedded on the page.
+     */
+    protected function getObjectSrc(): string
+    {
+        if (config('app.allow_content_scripts')) {
+            return '';
+        }
+
+        return "object-src 'self'";
+    }
+
+    /**
+     * Creates CSP 'base-uri' rule to restrict what base tags can be set on
      * the page to prevent manipulation of relative links.
      */
-    public function setBaseUri(Response $response)
+    protected function getBaseUri(): string
     {
-        $response->headers->set('Content-Security-Policy', 'base-uri \'self\'', false);
+        return "base-uri 'self'";
     }
 
     protected function getAllowedIframeHosts(): array
@@ -93,4 +128,21 @@ class CspService
 
         return array_filter(explode(' ', $hosts));
     }
+
+    protected function getAllowedIframeSources(): array
+    {
+        $sources = config('app.iframe_sources', '');
+        $hosts = array_filter(explode(' ', $sources));
+
+        // Extract drawing service url to allow embedding if active
+        $drawioConfigValue = config('services.drawio');
+        if ($drawioConfigValue) {
+            $drawioSource = is_string($drawioConfigValue) ? $drawioConfigValue : 'https://embed.diagrams.net/';
+            $drawioSourceParsed = parse_url($drawioSource);
+            $drawioHost = $drawioSourceParsed['scheme'] . '://' . $drawioSourceParsed['host'];
+            $hosts[] = $drawioHost;
+        }
+
+        return $hosts;
+    }
 }

resources/views/layouts/export.blade.php

@@ -4,6 +4,10 @@
     <meta http-equiv="Content-Type" content="text/html; charset=utf-8"/>
     <title>@yield('title')</title>
 
+    @if($cspContent ?? false)
+        <meta http-equiv="Content-Security-Policy" content="{{ $cspContent }}">
+    @endif
+
     @include('common.export-styles', ['format' => $format, 'engine' => $engine ?? ''])
     @include('common.export-custom-head')
 </head>

tests/Entity/ExportTest.php

@@ -268,7 +268,7 @@ class ExportTest extends TestCase
         foreach ($entities as $entity) {
             $resp = $this->asEditor()->get($entity->getUrl('/export/html'));
             $resp->assertDontSee('window.donkey');
-            $resp->assertDontSee('script');
+            $resp->assertDontSee('<script', false);
             $resp->assertSee('.my-test-class { color: red; }');
         }
     }
@@ -448,4 +448,18 @@ class ExportTest extends TestCase
         $resp = $this->get($page->getUrl('/export/pdf'));
         $resp->assertStatus(500); // Bad response indicates wkhtml usage
     }
+
+    public function test_html_exports_contain_csp_meta_tag()
+    {
+        $entities = [
+            Page::query()->first(),
+            Book::query()->first(),
+            Chapter::query()->first(),
+        ];
+
+        foreach ($entities as $entity) {
+            $resp = $this->asEditor()->get($entity->getUrl('/export/html'));
+            $resp->assertElementExists('head meta[http-equiv="Content-Security-Policy"][content*="script-src "]');
+        }
+    }
 }

tests/SecurityHeaderTest.php

@@ -119,6 +119,25 @@ class SecurityHeaderTest extends TestCase
         $this->assertEquals('base-uri \'self\'', $scriptHeader);
     }
 
+    public function test_frame_src_csp_header_set()
+    {
+        $resp = $this->get('/');
+        $scriptHeader = $this->getCspHeader($resp, 'frame-src');
+        $this->assertEquals('frame-src \'self\' https://*.draw.io https://*.youtube.com https://*.youtube-nocookie.com https://*.vimeo.com', $scriptHeader);
+    }
+
+    public function test_frame_src_csp_header_has_drawio_host_added()
+    {
+        config()->set([
+            'app.iframe_sources' => 'https://example.com',
+            'services.drawio'   => 'https://diagrams.example.com/testing?cat=dog',
+        ]);
+
+        $resp = $this->get('/');
+        $scriptHeader = $this->getCspHeader($resp, 'frame-src');
+        $this->assertEquals('frame-src \'self\' https://example.com https://diagrams.example.com', $scriptHeader);
+    }
+
     public function test_cache_control_headers_are_strict_on_responses_when_logged_in()
     {
         $this->asEditor();
@@ -133,10 +152,14 @@ class SecurityHeaderTest extends TestCase
      */
     protected function getCspHeader(TestResponse $resp, string $type): string
     {
-        $cspHeaders = collect($resp->headers->all('Content-Security-Policy'));
+        $cspHeaders = explode('; ', $resp->headers->get('Content-Security-Policy'));
 
-        return $cspHeaders->filter(function ($val) use ($type) {
-            return strpos($val, $type) === 0;
-        })->first() ?? '';
+        foreach ($cspHeaders as $cspHeader) {
+            if (strpos($cspHeader, $type) === 0) {
+                return $cspHeader;
+            }
+        }
+
+        return '';
     }
 }