Added iframe CSP, improved session cookie security

Added iframe CSP headers with configuration via .env.
Updated session cookies to be lax by default, dynamically changing to
none when iframes configured to allow third-party control.
Updated cookie security to be auto-secure if a https APP_URL is set.

Related to #2427 and #2207.

diff --git a/.env.example.complete b/.env.example.complete
index 19643a49f6d83aa6a576fc7d171f52c38a483f2f..e3dbdb857ddde1c7393e944a17ab9bea0f6b7104 100644 (file)
--- a/.env.example.complete
+++ b/.env.example.complete
@@ -273,6 +273,12 @@ ALLOW_CONTENT_SCRIPTS=false
 # Contents of the robots.txt file can be overridden, making this option obsolete.
 ALLOW_ROBOTS=null
 
 # Contents of the robots.txt file can be overridden, making this option obsolete.
 ALLOW_ROBOTS=null
 

+# A list of hosts that BookStack can be iframed within.
+# Space separated if multiple. BookStack host domain is auto-inferred.
+# For Example: ALLOWED_IFRAME_HOSTS="https://example.com https://a.example.com"
+# Setting this option will also auto-adjust cookies to be SameSite=None.
+ALLOWED_IFRAME_HOSTS=null
+

 # The default and maximum item-counts for listing API requests.
 API_DEFAULT_ITEM_COUNT=100
 API_MAX_ITEM_COUNT=500
 # The default and maximum item-counts for listing API requests.
 API_DEFAULT_ITEM_COUNT=100
 API_MAX_ITEM_COUNT=500

diff --git a/app/Config/app.php b/app/Config/app.php
index 7297048b4a33130df700274791a9b8833a15dc9c..a4367d4841345ea15bd581428113c3c07957847b 100755 (executable)
--- a/app/Config/app.php
+++ b/app/Config/app.php
@@ -52,6 +52,10 @@ return [
     // and used by BookStack in URL generation.
     'url' => env('APP_URL', '') === 'http://bookstack.dev' ? '' : env('APP_URL', ''),
 
     // and used by BookStack in URL generation.
     'url' => env('APP_URL', '') === 'http://bookstack.dev' ? '' : env('APP_URL', ''),
 

+    // A list of hosts that BookStack can be iframed within.
+    // Space separated if multiple. BookStack host domain is auto-inferred.
+    'iframe_hosts' => env('ALLOWED_IFRAME_HOSTS', null),
+

     // Application timezone for back-end date functions.
     'timezone' => env('APP_TIMEZONE', 'UTC'),
 
     // Application timezone for back-end date functions.
     'timezone' => env('APP_TIMEZONE', 'UTC'),
 

diff --git a/app/Config/session.php b/app/Config/session.php
index 37f1627bb5f5c151ae01748cdc06b8f5c7e7eeb2..571836bd2af2276a20732bfbe86459e83343551f 100644 (file)
--- a/app/Config/session.php
+++ b/app/Config/session.php
@@ -1,5 +1,7 @@
 <?php
 
 <?php
 

+use \Illuminate\Support\Str;
+

 /**
  * Session configuration options.
  *
 /**
  * Session configuration options.
  *


@@ -69,7 +71,8 @@ return [
     // By setting this option to true, session cookies will only be sent back
     // to the server if the browser has a HTTPS connection. This will keep
     // the cookie from being sent to you if it can not be done securely.
     // By setting this option to true, session cookies will only be sent back
     // to the server if the browser has a HTTPS connection. This will keep
     // the cookie from being sent to you if it can not be done securely.

-    'secure' => env('SESSION_SECURE_COOKIE', false),
+    'secure' => env('SESSION_SECURE_COOKIE', null)
+        ?? Str::startsWith(env('APP_URL'), 'https:'),

 
     // HTTP Access Only
     // Setting this value to true will prevent JavaScript from accessing the
 
     // HTTP Access Only
     // Setting this value to true will prevent JavaScript from accessing the


@@ -80,6 +83,6 @@ return [
     // This option determines how your cookies behave when cross-site requests
     // take place, and can be used to mitigate CSRF attacks. By default, we
     // do not enable this as other CSRF protection services are in place.
     // This option determines how your cookies behave when cross-site requests
     // take place, and can be used to mitigate CSRF attacks. By default, we
     // do not enable this as other CSRF protection services are in place.

-    // Options: lax, strict
-    'same_site' => null,
+    // Options: lax, strict, none
+    'same_site' => 'lax',

 ];
 ];

diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php
index a0c45ea896acdb18aed235ec7b51b14133c5a32f..532942f23e5b8f4b396fdd6601c002ddcf4390fd 100644 (file)
--- a/app/Http/Kernel.php
+++ b/app/Http/Kernel.php
@@ -22,6 +22,7 @@ class Kernel extends HttpKernel
      */
     protected $middlewareGroups = [
         'web' => [
      */
     protected $middlewareGroups = [
         'web' => [

+            \BookStack\Http\Middleware\ControlIframeSecurity::class,

             \BookStack\Http\Middleware\EncryptCookies::class,
             \Illuminate\Cookie\Middleware\AddQueuedCookiesToResponse::class,
             \Illuminate\Session\Middleware\StartSession::class,
             \BookStack\Http\Middleware\EncryptCookies::class,
             \Illuminate\Cookie\Middleware\AddQueuedCookiesToResponse::class,
             \Illuminate\Session\Middleware\StartSession::class,

diff --git a/app/Http/Middleware/ControlIframeSecurity.php b/app/Http/Middleware/ControlIframeSecurity.php
new file mode 100644 (file)
index 0000000..cc80344
--- /dev/null
+++ b/app/Http/Middleware/ControlIframeSecurity.php
@@ -0,0 +1,36 @@
+<?php
+
+namespace BookStack\Http\Middleware;
+
+use Closure;
+use Symfony\Component\HttpFoundation\Response;
+
+/**
+ * Sets CSP 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.
+ */
+class ControlIframeSecurity
+{
+    /**
+     * Handle an incoming request.
+     *
+     * @param  \Illuminate\Http\Request  $request
+     * @param  \Closure  $next
+     * @return mixed
+     */
+    public function handle($request, Closure $next)
+    {
+        $iframeHosts = collect(explode(' ', config('app.iframe_hosts', '')))->filter();
+        if ($iframeHosts->count() > 0) {
+            config()->set('session.same_site', 'none');
+        }
+
+        $iframeHosts->prepend("'self'");
+
+        $response = $next($request);
+        $cspValue = 'frame-ancestors ' . $iframeHosts->join(' ');
+        $response->headers->set('Content-Security-Policy', $cspValue);
+        return $response;
+    }
+}

diff --git a/phpunit.xml b/phpunit.xml
index ad7c6f43a5d551eec767dadda32f047d64ded014..8d69a5fddd3a8eee35101cceff94c6e3acd567ec 100644 (file)
--- a/phpunit.xml
+++ b/phpunit.xml
@@ -24,6 +24,8 @@
         <server name="APP_LANG" value="en"/>
         <server name="APP_THEME" value="none"/>
         <server name="APP_AUTO_LANG_PUBLIC" value="true"/>
         <server name="APP_LANG" value="en"/>
         <server name="APP_THEME" value="none"/>
         <server name="APP_AUTO_LANG_PUBLIC" value="true"/>

+        <server name="APP_URL" value="http://bookstack.dev"/>
+        <server name="ALLOWED_IFRAME_HOSTS" value=""/>

         <server name="CACHE_DRIVER" value="array"/>
         <server name="SESSION_DRIVER" value="array"/>
         <server name="QUEUE_CONNECTION" value="sync"/>
         <server name="CACHE_DRIVER" value="array"/>
         <server name="SESSION_DRIVER" value="array"/>
         <server name="QUEUE_CONNECTION" value="sync"/>


@@ -35,6 +37,7 @@
         <server name="DISABLE_EXTERNAL_SERVICES" value="true"/>
         <server name="AVATAR_URL" value=""/>
         <server name="LDAP_VERSION" value="3"/>
         <server name="DISABLE_EXTERNAL_SERVICES" value="true"/>
         <server name="AVATAR_URL" value=""/>
         <server name="LDAP_VERSION" value="3"/>

+        <server name="SESSION_SECURE_COOKIE" value="null"/>

         <server name="STORAGE_TYPE" value="local"/>
         <server name="STORAGE_ATTACHMENT_TYPE" value="local"/>
         <server name="STORAGE_IMAGE_TYPE" value="local"/>
         <server name="STORAGE_TYPE" value="local"/>
         <server name="STORAGE_ATTACHMENT_TYPE" value="local"/>
         <server name="STORAGE_IMAGE_TYPE" value="local"/>


@@ -47,7 +50,6 @@
         <server name="GOOGLE_AUTO_REGISTER" value=""/>
         <server name="GOOGLE_AUTO_CONFIRM_EMAIL" value=""/>
         <server name="GOOGLE_SELECT_ACCOUNT" value=""/>
         <server name="GOOGLE_AUTO_REGISTER" value=""/>
         <server name="GOOGLE_AUTO_CONFIRM_EMAIL" value=""/>
         <server name="GOOGLE_SELECT_ACCOUNT" value=""/>

-        <server name="APP_URL" value="http://bookstack.dev"/>

         <server name="DEBUGBAR_ENABLED" value="false"/>
         <server name="SAML2_ENABLED" value="false"/>
         <server name="API_REQUESTS_PER_MIN" value="180"/>
         <server name="DEBUGBAR_ENABLED" value="false"/>
         <server name="SAML2_ENABLED" value="false"/>
         <server name="API_REQUESTS_PER_MIN" value="180"/>

diff --git a/tests/SecurityHeaderTest.php b/tests/SecurityHeaderTest.php
new file mode 100644 (file)
index 0000000..db095ff
--- /dev/null
+++ b/tests/SecurityHeaderTest.php
@@ -0,0 +1,71 @@
+<?php namespace Tests;
+
+
+use Illuminate\Support\Str;
+
+class SecurityHeaderTest extends TestCase
+{
+
+    public function test_cookies_samesite_lax_by_default()
+    {
+        $resp = $this->get("/");
+        foreach ($resp->headers->getCookies() as $cookie) {
+            $this->assertEquals("lax", $cookie->getSameSite());
+        }
+    }
+
+    public function test_cookies_samesite_none_when_iframe_hosts_set()
+    {
+        $this->runWithEnv("ALLOWED_IFRAME_HOSTS", "http://example.com", function() {
+            $resp = $this->get("/");
+            foreach ($resp->headers->getCookies() as $cookie) {
+                $this->assertEquals("none", $cookie->getSameSite());
+            }
+        });
+    }
+
+    public function test_secure_cookies_controlled_by_app_url()
+    {
+        $this->runWithEnv("APP_URL", "http://example.com", function() {
+            $resp = $this->get("/");
+            foreach ($resp->headers->getCookies() as $cookie) {
+                $this->assertFalse($cookie->isSecure());
+            }
+        });
+
+        $this->runWithEnv("APP_URL", "https://example.com", function() {
+            $resp = $this->get("/");
+            foreach ($resp->headers->getCookies() as $cookie) {
+                $this->assertTrue($cookie->isSecure());
+            }
+        });
+    }
+
+    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');
+        });
+
+        $this->assertTrue($frameHeaders->count() === 1);
+        $this->assertEquals('frame-ancestors \'self\'', $frameHeaders->first());
+    }
+
+    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');
+            });
+
+            $this->assertTrue($frameHeaders->count() === 1);
+            $this->assertEquals('frame-ancestors \'self\' https://a.example.com https://b.example.com', $frameHeaders->first());
+        });
+
+    }
+
+}
\ No newline at end of file