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.

.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
 
+# 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

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', ''),
 
+    // 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'),
 

app/Config/session.php

@@ -1,5 +1,7 @@
 <?php
 
+use \Illuminate\Support\Str;
+
 /**
  * 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.
-    '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
@@ -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.
-    // Options: lax, strict
-    'same_site' => null,
+    // Options: lax, strict, none
+    'same_site' => 'lax',
 ];

app/Http/Kernel.php

@@ -22,6 +22,7 @@ class Kernel extends HttpKernel
      */
     protected $middlewareGroups = [
         'web' => [
+            \BookStack\Http\Middleware\ControlIframeSecurity::class,
             \BookStack\Http\Middleware\EncryptCookies::class,
             \Illuminate\Cookie\Middleware\AddQueuedCookiesToResponse::class,
             \Illuminate\Session\Middleware\StartSession::class,

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;
+    }
+}

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_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"/>
@@ -35,6 +37,7 @@
         <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"/>
@@ -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="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"/>

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());
+        });
+
+    }
+
+}