From 92922288dd55ce0f77acc83eea9068cad28dccd9 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 2 Jan 2021 02:43:50 +0000 Subject: [PATCH] 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 | 6 ++ app/Config/app.php | 4 ++ app/Config/session.php | 9 ++- app/Http/Kernel.php | 1 + app/Http/Middleware/ControlIframeSecurity.php | 36 ++++++++++ phpunit.xml | 4 +- tests/SecurityHeaderTest.php | 71 +++++++++++++++++++ 7 files changed, 127 insertions(+), 4 deletions(-) create mode 100644 app/Http/Middleware/ControlIframeSecurity.php create mode 100644 tests/SecurityHeaderTest.php diff --git a/.env.example.complete b/.env.example.complete index 19643a49f..e3dbdb857 100644 --- 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 +# 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 diff --git a/app/Config/app.php b/app/Config/app.php index 7297048b4..a4367d484 100755 --- 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', ''), + // 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'), diff --git a/app/Config/session.php b/app/Config/session.php index 37f1627bb..571836bd2 100644 --- a/app/Config/session.php +++ b/app/Config/session.php @@ -1,5 +1,7 @@ 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', ]; diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php index a0c45ea89..532942f23 100644 --- a/app/Http/Kernel.php +++ b/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, diff --git a/app/Http/Middleware/ControlIframeSecurity.php b/app/Http/Middleware/ControlIframeSecurity.php new file mode 100644 index 000000000..cc8034413 --- /dev/null +++ b/app/Http/Middleware/ControlIframeSecurity.php @@ -0,0 +1,36 @@ +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 ad7c6f43a..8d69a5fdd 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -24,6 +24,8 @@ + + @@ -35,6 +37,7 @@ + @@ -47,7 +50,6 @@ - diff --git a/tests/SecurityHeaderTest.php b/tests/SecurityHeaderTest.php new file mode 100644 index 000000000..db095ff70 --- /dev/null +++ b/tests/SecurityHeaderTest.php @@ -0,0 +1,71 @@ +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