Finished off script CSP rules

- Added caching for custom html head parsing to add nonce.
- Also moved api docs page into web routes to prevent issues.

app/Http/Middleware/ApplyCspRules.php

@@ -2,14 +2,23 @@
 
 namespace BookStack\Http\Middleware;
 
+use BookStack\Util\CspService;
 use Closure;
 use Illuminate\Http\Request;
-use Illuminate\Support\Str;
-use Symfony\Component\HttpFoundation\Response;
-
 
 class ApplyCspRules
 {
+
+    /**
+     * @var CspService
+     */
+    protected $cspService;
+
+    public function __construct(CspService $cspService)
+    {
+        $this->cspService = $cspService;
+    }
+
     /**
      * Handle an incoming request.
      *
@@ -20,50 +29,17 @@ class ApplyCspRules
      */
     public function handle($request, Closure $next)
     {
-        $nonce = Str::random(24);
-        view()->share('cspNonce', $nonce);
-
-        // TODO - Assess whether image/style/iframe CSP rules should be set
-        // TODO - Extract nonce application to custom head content in a way that's cacheable.
-        // TODO - Fix remaining CSP issues and test lots
+        view()->share('cspNonce', $this->cspService->getNonce());
+        if ($this->cspService->allowedIFrameHostsConfigured()) {
+            config()->set('session.same_site', 'none');
+        }
 
         $response = $next($request);
 
-        $this->setFrameAncestors($response);
-        $this->setScriptSrc($response, $nonce);
+        $this->cspService->setFrameAncestors($response);
+        $this->cspService->setScriptSrc($response);
 
         return $response;
     }
 
-    /**
-     * Sets CSP 'script-src' headers to restrict the forms of script that can
-     * run on the page.
-     */
-    public function setScriptSrc(Response $response, string $nonce)
-    {
-        $parts = [
-            '\'self\'',
-            '\'nonce-' . $nonce . '\'',
-            '\'strict-dynamic\'',
-        ];
-        $response->headers->set('Content-Security-Policy', 'script-src ' . implode(' ', $parts));
-    }
-
-    /**
-     * 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.
-     */
-    protected function setFrameAncestors(Response $response)
-    {
-        $iframeHosts = collect(explode(' ', config('app.iframe_hosts', '')))->filter();
-
-        if ($iframeHosts->count() > 0) {
-            config()->set('session.same_site', 'none');
-        }
-
-        $iframeHosts->prepend("'self'");
-        $cspValue = 'frame-ancestors ' . $iframeHosts->join(' ');
-        $response->headers->set('Content-Security-Policy', $cspValue);
-    }
 }

app/Providers/AppServiceProvider.php

@@ -12,6 +12,7 @@ use BookStack\Entities\Models\Chapter;
 use BookStack\Entities\Models\Page;
 use BookStack\Settings\Setting;
 use BookStack\Settings\SettingService;
+use BookStack\Util\CspService;
 use Illuminate\Contracts\Cache\Repository;
 use Illuminate\Database\Eloquent\Relations\Relation;
 use Illuminate\Support\Facades\View;
@@ -71,5 +72,9 @@ class AppServiceProvider extends ServiceProvider
         $this->app->singleton(SocialAuthService::class, function ($app) {
             return new SocialAuthService($app->make(SocialiteFactory::class), $app->make(LoginService::class));
         });
+
+        $this->app->singleton(CspService::class, function($app) {
+            return new CspService();
+        });
     }
 }

app/Theming/CustomHtmlHeadContentProvider.php

@@ -0,0 +1,63 @@
+<?php
+
+namespace BookStack\Theming;
+
+use BookStack\Util\CspService;
+use BookStack\Util\HtmlContentFilter;
+use BookStack\Util\HtmlNonceApplicator;
+use Illuminate\Contracts\Cache\Repository as Cache;
+
+class CustomHtmlHeadContentProvider
+{
+    /**
+     * @var CspService
+     */
+    protected $cspService;
+
+    /**
+     * @var Cache
+     */
+    protected $cache;
+
+    public function __construct(CspService $cspService, Cache $cache)
+    {
+        $this->cspService = $cspService;
+        $this->cache = $cache;
+    }
+
+    /**
+     * Fetch our custom HTML head content prepared for use on web pages.
+     * Content has a nonce applied for CSP.
+     */
+    public function forWeb(): string
+    {
+        $content = $this->getSourceContent();
+        $hash = md5($content);
+        $html = $this->cache->remember('custom-head-web:' . $hash, 86400, function() use ($content) {
+            return HtmlNonceApplicator::prepare($content);
+        });
+        return HtmlNonceApplicator::apply($html, $this->cspService->getNonce());
+    }
+
+    /**
+     * Fetch our custom HTML head content prepared for use in export formats.
+     * Scripts are stripped to avoid potential issues.
+     */
+    public function forExport(): string
+    {
+        $content = $this->getSourceContent();
+        $hash = md5($content);
+        return $this->cache->remember('custom-head-export:' . $hash, 86400, function() use ($content) {
+             return HtmlContentFilter::removeScripts($content);
+        });
+    }
+
+    /**
+     * Get the original custom head content to use.
+     */
+    protected function getSourceContent(): string
+    {
+        return setting('app-custom-head', '');
+    }
+
+}

app/Util/CspService.php

@@ -0,0 +1,72 @@
+<?php
+
+namespace BookStack\Util;
+
+use Illuminate\Support\Str;
+use Symfony\Component\HttpFoundation\Response;
+
+class CspService
+{
+    /** @var string */
+    protected $nonce;
+
+    public function __construct(string $nonce = '')
+    {
+        $this->nonce = $nonce ?: Str::random(16);
+    }
+
+    /**
+     * Get the nonce value for CSP.
+     */
+    public function getNonce(): string
+    {
+        return $this->nonce;
+    }
+
+    /**
+     * Sets CSP 'script-src' headers to restrict the forms of script that can
+     * run on the page.
+     */
+    public function setScriptSrc(Response $response)
+    {
+        if (config('app.allow_content_scripts')) {
+            return;
+        }
+
+        $parts = [
+            '\'nonce-' . $this->nonce . '\'',
+            '\'strict-dynamic\'',
+        ];
+        $value = 'script-src ' . implode(' ', $parts);
+        $response->headers->set('Content-Security-Policy', $value, false);
+    }
+
+    /**
+     * 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.
+     */
+    public function setFrameAncestors(Response $response)
+    {
+        $iframeHosts = $this->getAllowedIframeHosts();
+        array_unshift($iframeHosts, "'self'");
+        $cspValue = 'frame-ancestors ' . implode(' ', $iframeHosts);
+        $response->headers->set('Content-Security-Policy', $cspValue, false);
+    }
+
+    /**
+     * Check if the user has configured some allowed iframe hosts.
+     */
+    public function allowedIFrameHostsConfigured(): bool
+    {
+        return count($this->getAllowedIframeHosts()) > 0;
+    }
+
+
+    protected function getAllowedIframeHosts(): array
+    {
+        $hosts = config('app.iframe_hosts', '');
+        return array_filter(explode(' ', $hosts));
+    }
+
+}

app/Util/HtmlNonceApplicator.php

@@ -9,10 +9,13 @@ use DOMXPath;
 
 class HtmlNonceApplicator
 {
+    protected static $placeholder = '[CSP_NONCE_VALUE]';
+
     /**
-     * Apply the given nonce to all scripts and styles in the given html.
+     * Prepare the given HTML content with nonce attributes including a placeholder
+     * value which we can target later.
      */
-    public static function apply(string $html, string $nonce): string
+    public static function prepare(string $html): string
     {
         if (empty($html)) {
             return $html;
@@ -26,11 +29,11 @@ class HtmlNonceApplicator
 
         // Apply to scripts
         $scriptElems = $xPath->query('//script');
-        static::addNonceAttributes($scriptElems, $nonce);
+        static::addNonceAttributes($scriptElems, static::$placeholder);
 
         // Apply to styles
         $styleElems = $xPath->query('//style');
-        static::addNonceAttributes($styleElems, $nonce);
+        static::addNonceAttributes($styleElems, static::$placeholder);
 
         $returnHtml = '';
         $topElems = $doc->documentElement->childNodes->item(0)->childNodes;
@@ -41,11 +44,19 @@ class HtmlNonceApplicator
         return $returnHtml;
     }
 
-    protected static function addNonceAttributes(DOMNodeList $nodes, string $nonce): void
+    /**
+     * Apply the give nonce value to the given prepared HTML.
+     */
+    public static function apply(string $html, string $nonce): string
+    {
+        return str_replace(static::$placeholder, $nonce, $html);
+    }
+
+    protected static function addNonceAttributes(DOMNodeList $nodes, string $attrValue): void
     {
         /** @var DOMElement $node */
         foreach ($nodes as $node) {
-            $node->setAttribute('nonce', $nonce);
+            $node->setAttribute('nonce', $attrValue);
         }
     }
 

resources/views/common/custom-head.blade.php

@@ -1,5 +1,7 @@
+@inject('headContent', 'BookStack\Theming\CustomHtmlHeadContentProvider')
+
 @if(setting('app-custom-head') && \Route::currentRouteName() !== 'settings')
 <!-- Custom user content -->
-{!! \BookStack\Util\HtmlNonceApplicator::apply(setting('app-custom-head'), $cspNonce) !!}
+{!! $headContent->forWeb() !!}
 <!-- End custom user content -->
 @endif

resources/views/common/export-custom-head.blade.php

@@ -1,5 +1,7 @@
+@inject('headContent', 'BookStack\Theming\CustomHtmlHeadContentProvider')
+
 @if(setting('app-custom-head'))
 <!-- Custom user content -->
-{!! \BookStack\Util\HtmlContentFilter::removeScripts(setting('app-custom-head')) !!}
+{!! $headContent->forExport() !!}
 <!-- End custom user content -->
 @endif

resources/views/layouts/base.blade.php

@@ -15,7 +15,6 @@
     <meta property="og:title" content="{{ isset($pageTitle) ? $pageTitle . ' | ' : '' }}{{ setting('app-name') }}">
     <meta property="og:url" content="{{ url()->current() }}">
     @stack('social-meta')
-    
 
     <!-- Styles and Fonts -->
     <link rel="stylesheet" href="{{ versioned_asset('dist/styles.css') }}">
@@ -51,7 +50,7 @@
     </div>
 
     @yield('bottom')
-    <script src="{{ versioned_asset('dist/app.js') }}"></script>
+    <script src="{{ versioned_asset('dist/app.js') }}" nonce="{{ $cspNonce }}"></script>
     @yield('scripts')
 
 </body>

resources/views/pages/edit.blade.php

@@ -1,7 +1,7 @@
 @extends('layouts.base')
 
 @section('head')
-    <script src="{{ url('/libs/tinymce/tinymce.min.js?ver=4.9.4') }}"></script>
+    <script src="{{ url('/libs/tinymce/tinymce.min.js?ver=4.9.4') }}" nonce="{{ $cspNonce }}"></script>
 @stop
 
 @section('body-class', 'flexbox')

routes/api.php

@@ -5,7 +5,6 @@
  * Routes have a uri prefix of /api/.
  * Controllers are all within app/Http/Controllers/Api.
  */
-Route::get('docs', 'ApiDocsController@display');
 Route::get('docs.json', 'ApiDocsController@json');
 
 Route::get('books', 'BookApiController@list');

routes/web.php

@@ -10,6 +10,9 @@ Route::group(['middleware' => 'auth'], function () {
     Route::get('/uploads/images/{path}', 'Images\ImageController@showImage')
         ->where('path', '.*$');
 
+    // API docs routes
+    Route::get('/api/docs', 'Api\ApiDocsController@display');
+
     Route::get('/pages/recently-updated', 'PageController@showRecentlyUpdated');
 
     // Shelves

tests/Api/ApiDocsTest.php

@@ -2,7 +2,6 @@
 
 namespace Tests\Api;
 
-use BookStack\Auth\User;
 use Tests\TestCase;
 
 class ApiDocsTest extends TestCase
@@ -11,16 +10,6 @@ class ApiDocsTest extends TestCase
 
     protected $endpoint = '/api/docs';
 
-    public function test_docs_page_not_visible_to_normal_viewers()
-    {
-        $viewer = $this->getViewer();
-        $resp = $this->actingAs($viewer)->get($this->endpoint);
-        $resp->assertStatus(403);
-
-        $resp = $this->actingAsApiEditor()->get($this->endpoint);
-        $resp->assertStatus(200);
-    }
-
     public function test_docs_page_returns_view_with_docs_content()
     {
         $resp = $this->actingAsApiEditor()->get($this->endpoint);
@@ -42,19 +31,4 @@ class ApiDocsTest extends TestCase
             ]],
         ]);
     }
-
-    public function test_docs_page_visible_by_public_user_if_given_permission()
-    {
-        $this->setSettings(['app-public' => true]);
-        $guest = User::getDefault();
-
-        $this->startSession();
-        $resp = $this->get('/api/docs');
-        $resp->assertStatus(403);
-
-        $this->giveUserPermissions($guest, ['access-api']);
-
-        $resp = $this->get('/api/docs');
-        $resp->assertStatus(200);
-    }
 }

tests/SecurityHeaderTest.php

@@ -2,7 +2,7 @@
 
 namespace Tests;
 
-use Illuminate\Support\Str;
+use BookStack\Util\CspService;
 
 class SecurityHeaderTest extends TestCase
 {
@@ -44,26 +44,75 @@ class SecurityHeaderTest extends TestCase
     public function test_iframe_csp_self_only_by_default()
     {
         $resp = $this->get('/');
-        $cspHeaders = collect($resp->headers->get('Content-Security-Policy'));
-        $frameHeaders = $cspHeaders->filter(function ($val) {
-            return Str::startsWith($val, 'frame-ancestors');
-        });
+        $frameHeader = $this->getCspHeader($resp, 'frame-ancestors');
 
-        $this->assertTrue($frameHeaders->count() === 1);
-        $this->assertEquals('frame-ancestors \'self\'', $frameHeaders->first());
+        $this->assertEquals('frame-ancestors \'self\'', $frameHeader);
     }
 
     public function test_iframe_csp_includes_extra_hosts_if_configured()
     {
         $this->runWithEnv('ALLOWED_IFRAME_HOSTS', 'https://a.example.com https://b.example.com', function () {
             $resp = $this->get('/');
-            $cspHeaders = collect($resp->headers->get('Content-Security-Policy'));
-            $frameHeaders = $cspHeaders->filter(function ($val) {
-                return Str::startsWith($val, 'frame-ancestors');
-            });
+            $frameHeader = $this->getCspHeader($resp, 'frame-ancestors');
 
-            $this->assertTrue($frameHeaders->count() === 1);
-            $this->assertEquals('frame-ancestors \'self\' https://a.example.com https://b.example.com', $frameHeaders->first());
+            $this->assertNotEmpty($frameHeader);
+            $this->assertEquals('frame-ancestors \'self\' https://a.example.com https://b.example.com', $frameHeader);
         });
     }
+
+    public function test_script_csp_set_on_responses()
+    {
+        $resp = $this->get('/');
+        $scriptHeader = $this->getCspHeader($resp, 'script-src');
+        $this->assertStringContainsString('\'strict-dynamic\'', $scriptHeader);
+        $this->assertStringContainsString('\'nonce-', $scriptHeader);
+    }
+
+    public function test_script_csp_nonce_matches_nonce_used_in_custom_head()
+    {
+        $this->setSettings(['app-custom-head' => '<script>console.log("cat");</script>']);
+        $resp = $this->get('/login');
+        $scriptHeader = $this->getCspHeader($resp, 'script-src');
+
+        $nonce = app()->make(CspService::class)->getNonce();
+        $this->assertStringContainsString('nonce-' . $nonce, $scriptHeader);
+        $resp->assertSee('<script nonce="' . $nonce . '">console.log("cat");</script>');
+    }
+
+    public function test_script_csp_nonce_changes_per_request()
+    {
+        $resp = $this->get('/');
+        $firstHeader = $this->getCspHeader($resp, 'script-src');
+
+        $this->refreshApplication();
+
+        $resp = $this->get('/');
+        $secondHeader = $this->getCspHeader($resp, 'script-src');
+
+        $this->assertNotEquals($firstHeader, $secondHeader);
+    }
+
+    public function test_allow_content_scripts_settings_controls_csp_script_headers()
+    {
+        config()->set('app.allow_content_scripts', true);
+        $resp = $this->get('/');
+        $scriptHeader = $this->getCspHeader($resp, 'script-src');
+        $this->assertEmpty($scriptHeader);
+
+        config()->set('app.allow_content_scripts', false);
+        $resp = $this->get('/');
+        $scriptHeader = $this->getCspHeader($resp, 'script-src');
+        $this->assertNotEmpty($scriptHeader);
+    }
+
+    /**
+     * Get the value of the first CSP header of the given type.
+     */
+    protected function getCspHeader(TestResponse $resp, string $type): string
+    {
+        $cspHeaders = collect($resp->headers->all('Content-Security-Policy'));
+        return $cspHeaders->filter(function ($val) use ($type) {
+            return strpos($val, $type) === 0;
+        })->first() ?? '';
+    }
 }