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.

diff --git a/app/Http/Middleware/ApplyCspRules.php b/app/Http/Middleware/ApplyCspRules.php
index 2889d80b0e25118c4834ded3fbed8ab15d40ba36..4c2b1e1da0d5587fc26f7fe0ddf30ca17cd062a6 100644 (file)
--- a/app/Http/Middleware/ApplyCspRules.php
+++ b/app/Http/Middleware/ApplyCspRules.php
@@ -2,14 +2,23 @@
 
 namespace BookStack\Http\Middleware;
 
 
 namespace BookStack\Http\Middleware;
 

+use BookStack\Util\CspService;

 use Closure;
 use Illuminate\Http\Request;
 use Closure;
 use Illuminate\Http\Request;

-use Illuminate\Support\Str;
-use Symfony\Component\HttpFoundation\Response;
-

 
 class ApplyCspRules
 {
 
 class ApplyCspRules
 {

+
+    /**
+     * @var CspService
+     */
+    protected $cspService;
+
+    public function __construct(CspService $cspService)
+    {
+        $this->cspService = $cspService;
+    }
+

     /**
      * Handle an incoming request.
      *
     /**
      * Handle an incoming request.
      *


@@ -20,50 +29,17 @@ class ApplyCspRules
      */
     public function handle($request, Closure $next)
     {
      */
     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);
 
 
         $response = $next($request);
 

-        $this->setFrameAncestors($response);
-        $this->setScriptSrc($response, $nonce);
+        $this->cspService->setFrameAncestors($response);
+        $this->cspService->setScriptSrc($response);

 
         return $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);
-    }

 }
 }

diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php
index 145a7645b72254904dcaa890f1cf2b082918841c..1119d87df023ce4a82cf24bb0815ed8210c34bd4 100644 (file)
--- a/app/Providers/AppServiceProvider.php
+++ b/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\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;
 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(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();
+        });

     }
 }
     }
 }

diff --git a/app/Theming/CustomHtmlHeadContentProvider.php b/app/Theming/CustomHtmlHeadContentProvider.php
new file mode 100644 (file)
index 0000000..6110d5a
--- /dev/null
+++ b/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', '');
+    }
+
+}
\ No newline at end of file

diff --git a/app/Util/CspService.php b/app/Util/CspService.php
new file mode 100644 (file)
index 0000000..2728aae
--- /dev/null
+++ b/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));
+    }
+
+}
\ No newline at end of file

diff --git a/app/Util/HtmlNonceApplicator.php b/app/Util/HtmlNonceApplicator.php
index eb2cf26873f59218911b0eaf7ace0d5124666f66..e66625bf2675311999e7ed87fc920918c58feafb 100644 (file)
--- a/app/Util/HtmlNonceApplicator.php
+++ b/app/Util/HtmlNonceApplicator.php
@@ -9,10 +9,13 @@ use DOMXPath;
 
 class HtmlNonceApplicator
 {
 
 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;
     {
         if (empty($html)) {
             return $html;


@@ -26,11 +29,11 @@ class HtmlNonceApplicator
 
         // Apply to scripts
         $scriptElems = $xPath->query('//script');
 
         // Apply to scripts
         $scriptElems = $xPath->query('//script');

-        static::addNonceAttributes($scriptElems, $nonce);
+        static::addNonceAttributes($scriptElems, static::$placeholder);

 
         // Apply to styles
         $styleElems = $xPath->query('//style');
 
         // Apply to styles
         $styleElems = $xPath->query('//style');

-        static::addNonceAttributes($styleElems, $nonce);
+        static::addNonceAttributes($styleElems, static::$placeholder);

 
         $returnHtml = '';
         $topElems = $doc->documentElement->childNodes->item(0)->childNodes;
 
         $returnHtml = '';
         $topElems = $doc->documentElement->childNodes->item(0)->childNodes;


@@ -41,11 +44,19 @@ class HtmlNonceApplicator
         return $returnHtml;
     }
 
         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) {
     {
         /** @var DOMElement $node */
         foreach ($nodes as $node) {

-            $node->setAttribute('nonce', $nonce);
+            $node->setAttribute('nonce', $attrValue);

         }
     }
 
         }
     }
 

diff --git a/resources/views/common/custom-head.blade.php b/resources/views/common/custom-head.blade.php
index 3199fc179d2f6d5b79550abf65384ef47ad897eb..6f88bd43f7a9cd77d8d1aec9a85c665fadb27e7c 100644 (file)
--- a/resources/views/common/custom-head.blade.php
+++ b/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 -->
 @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
\ No newline at end of file
 <!-- End custom user content -->
 @endif
\ No newline at end of file

diff --git a/resources/views/common/export-custom-head.blade.php b/resources/views/common/export-custom-head.blade.php
index f428e9fe9b0ecf6d291aba85b9f34d07569b5e19..2452d6b8e646ceff733678a93b34be14ed38f9a8 100644 (file)
--- a/resources/views/common/export-custom-head.blade.php
+++ b/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 -->
 @if(setting('app-custom-head'))
 <!-- Custom user content -->

-{!! \BookStack\Util\HtmlContentFilter::removeScripts(setting('app-custom-head')) !!}
+{!! $headContent->forExport() !!}

 <!-- End custom user content -->
 @endif
\ No newline at end of file
 <!-- End custom user content -->
 @endif
\ No newline at end of file

diff --git a/resources/views/layouts/base.blade.php b/resources/views/layouts/base.blade.php
index 6a45b4209ac2b2b3425620f51d4eb6007d20dfe4..1f28e354ce8e119f03a0281fde89563eaee371d5 100644 (file)
--- a/resources/views/layouts/base.blade.php
+++ b/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')
     <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') }}">
 
     <!-- Styles and Fonts -->
     <link rel="stylesheet" href="{{ versioned_asset('dist/styles.css') }}">


@@ -51,7 +50,7 @@
     </div>
 
     @yield('bottom')
     </div>
 
     @yield('bottom')

-    <script src="{{ versioned_asset('dist/app.js') }}"></script>
+    <script src="{{ versioned_asset('dist/app.js') }}" nonce="{{ $cspNonce }}"></script>

     @yield('scripts')
 
 </body>
     @yield('scripts')
 
 </body>

diff --git a/resources/views/pages/edit.blade.php b/resources/views/pages/edit.blade.php
index db518b0d45af321621ee91bec4f86d46237630cc..6d2c3d484d43574ede66250a58347cef8c3f2692 100644 (file)
--- a/resources/views/pages/edit.blade.php
+++ b/resources/views/pages/edit.blade.php
@@ -1,7 +1,7 @@
 @extends('layouts.base')
 
 @section('head')
 @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')
 @stop
 
 @section('body-class', 'flexbox')

diff --git a/routes/api.php b/routes/api.php
index a6ed0c8f110702c007db4feceb8075a5f1ab6971..83a411219833ae3daa9048cb21e88ba1d539f29e 100644 (file)
--- a/routes/api.php
+++ b/routes/api.php
@@ -5,7 +5,6 @@
  * Routes have a uri prefix of /api/.
  * Controllers are all within app/Http/Controllers/Api.
  */
  * 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');
 Route::get('docs.json', 'ApiDocsController@json');
 
 Route::get('books', 'BookApiController@list');

diff --git a/routes/web.php b/routes/web.php
index a823b73c88d13ee3c3ac3427024d67ce191098a2..08adeceb947318a9f017f55424e4b5e91e09ccb7 100644 (file)
--- a/routes/web.php
+++ b/routes/web.php
@@ -10,6 +10,9 @@ Route::group(['middleware' => 'auth'], function () {
     Route::get('/uploads/images/{path}', 'Images\ImageController@showImage')
         ->where('path', '.*$');
 
     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
     Route::get('/pages/recently-updated', 'PageController@showRecentlyUpdated');
 
     // Shelves

diff --git a/tests/Api/ApiDocsTest.php b/tests/Api/ApiDocsTest.php
index 90d107eb34aa7b170d73fe2999014c7789248176..062adce5376821a8b1914b734039c107edf6fa2b 100644 (file)
--- a/tests/Api/ApiDocsTest.php
+++ b/tests/Api/ApiDocsTest.php
@@ -2,7 +2,6 @@
 
 namespace Tests\Api;
 
 
 namespace Tests\Api;
 

-use BookStack\Auth\User;

 use Tests\TestCase;
 
 class ApiDocsTest extends TestCase
 use Tests\TestCase;
 
 class ApiDocsTest extends TestCase


@@ -11,16 +10,6 @@ class ApiDocsTest extends TestCase
 
     protected $endpoint = '/api/docs';
 
 
     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);
     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);
-    }

 }
 }

diff --git a/tests/SecurityHeaderTest.php b/tests/SecurityHeaderTest.php
index 888dac8106af4b8088ecf2f28fb8d82bd96d12f6..57f4ab0df3fd7dc33f8031415b0181b3519d99f4 100644 (file)
--- a/tests/SecurityHeaderTest.php
+++ b/tests/SecurityHeaderTest.php
@@ -2,7 +2,7 @@
 
 namespace Tests;
 
 
 namespace Tests;
 

-use Illuminate\Support\Str;
+use BookStack\Util\CspService;

 
 class SecurityHeaderTest extends TestCase
 {
 
 class SecurityHeaderTest extends TestCase
 {


@@ -44,26 +44,75 @@ class SecurityHeaderTest extends TestCase
     public function test_iframe_csp_self_only_by_default()
     {
         $resp = $this->get('/');
     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('/');
     }
 
     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() ?? '';
+    }

 }
 }