diff --git a/docs/en/02_Developer_Guides/09_Security/04_Secure_Coding.md b/docs/en/02_Developer_Guides/09_Security/04_Secure_Coding.md index 4d2b5cb8629..51c67a3f1a9 100644 --- a/docs/en/02_Developer_Guides/09_Security/04_Secure_Coding.md +++ b/docs/en/02_Developer_Guides/09_Security/04_Secure_Coding.md @@ -762,11 +762,15 @@ disable this behaviour using `CanonicalURLMiddleware::singleton()->setForceBasic configuration in YAML. We also want to ensure cookies are not shared between secure and non-secure sessions, so we must tell Silverstripe CMS to -use a [secure session](https://docs.silverstripe.org/en/3/developer_guides/cookies_and_sessions/sessions/#secure-session-cookie). -To do this, you may set the `cookie_secure` parameter to `true` in your `config.yml` for `Session` +use a [secure session](/developer_guides/cookies_and_sessions/sessions/#secure-session-cookie). +To do this, you may set the `cookie_secure` parameter to `true` in your `config.yml` for `Session`. + +It is also a good idea to set the `samesite` attribute for the session cookie to `Strict` unless you have a specific use case for +sharing the session cookie across domains. ```yml SilverStripe\Control\Session: + cookie_samesite: 'Strict' cookie_secure: true ``` @@ -784,6 +788,11 @@ SilverStripe\Core\Injector\Injector: TokenCookieSecure: true ``` +[info] +There is not currently an easy way to pass a `samesite` attribute value for setting this cookie - but you can set the +default value for the attribute for all cookies. See [the main cookies documentation](/developer_guides/cookies_and_sessions/cookies#samesite-attribute) for more information. +[/info] + For other cookies set by your application we should also ensure the users are provided with secure cookies by setting the "Secure" and "HTTPOnly" flags. These flags prevent them from being stolen by an attacker through javascript. diff --git a/docs/en/02_Developer_Guides/18_Cookies_And_Sessions/01_Cookies.md b/docs/en/02_Developer_Guides/18_Cookies_And_Sessions/01_Cookies.md index 4b14705237f..cb9b3075c07 100644 --- a/docs/en/02_Developer_Guides/18_Cookies_And_Sessions/01_Cookies.md +++ b/docs/en/02_Developer_Guides/18_Cookies_And_Sessions/01_Cookies.md @@ -5,6 +5,10 @@ icon: cookie-bite --- # Cookies + +Note that cookies can have security implications - before setting your own cookies, make sure to read through the +[secure coding](/developer_guides/security/secure_coding#secure-sessions-cookies-and-tls-https) documentation. + ## Accessing and Manipulating Cookies Cookies are a mechanism for storing data in the remote browser and thus tracking or identifying return users. @@ -52,6 +56,20 @@ Cookie::force_expiry($name, $path = null, $domain = null); // Cookie::force_expiry('MyApplicationPreference') ``` +### Samesite attribute + +The `samesite` attribute is set on all cookies with a default value of `Lax`. You can change the default value by setting the `default_samesite` value on the +[Cookie](api:SilverStripe\Control\Cookie) class: + +```yml +SilverStripe\Control\Cookie: + default_samesite: 'Strict' +``` + +[info] +Note that this _doesn't_ apply for the session cookie, which is handled separately. See [Sessions](/developer_guides/cookies_and_sessions/sessions#samesite-attribute). +[/info] + ## Cookie_Backend The [Cookie](api:SilverStripe\Control\Cookie) class manipulates and sets cookies using a [Cookie_Backend](api:SilverStripe\Control\Cookie_Backend). The backend is in charge of the logic diff --git a/docs/en/02_Developer_Guides/18_Cookies_And_Sessions/02_Sessions.md b/docs/en/02_Developer_Guides/18_Cookies_And_Sessions/02_Sessions.md index 75a7bdc105c..548f661c857 100644 --- a/docs/en/02_Developer_Guides/18_Cookies_And_Sessions/02_Sessions.md +++ b/docs/en/02_Developer_Guides/18_Cookies_And_Sessions/02_Sessions.md @@ -101,7 +101,19 @@ including form and page comment information. None of this is vital but `clear_al $session->clearAll(); ``` -## Secure Session Cookie +## Cookies + +### Samesite attribute + +The session cookie is handled slightly differently than most cookies on the site, which provides the opportunity to handle the samesite attribute separately from other cookies. +You can change the `samesite` attribute for session cookies like so: + +```yml +SilverStripe\Control\Session: + cookie_samesite: 'Strict' +``` + +### Secure Session Cookie In certain circumstances, you may want to use a different `session_name` cookie when using the `https` protocol for security purposes. To do this, you may set the `cookie_secure` parameter to `true` on your `config.yml` @@ -113,6 +125,8 @@ SilverStripe\Control\Session: This uses the session_name `SECSESSID` for `https` connections instead of the default `PHPSESSID`. Doing so adds an extra layer of security to your session cookie since you no longer share `http` and `https` sessions. +Note that if you set `cookie_samesite` to `None` (which is _strongly_ discouraged), the `cookie_secure` value will _always_ be `true`. + ## Relaxing checks around user agent strings Out of the box, Silverstripe CMS will invalidate a user's session if the `User-Agent` header changes. This provides some supplemental protection against session high-jacking attacks. diff --git a/docs/en/04_Changelogs/4.12.0.md b/docs/en/04_Changelogs/4.12.0.md new file mode 100644 index 00000000000..aaa2aa6e2bd --- /dev/null +++ b/docs/en/04_Changelogs/4.12.0.md @@ -0,0 +1,55 @@ +--- +title: 4.12.0 (unreleased) +--- + +# 4.12.0 (unreleased) + +## Overview + +- [Regression test and Security audit](#audit) +- [Features and enhancements](#features-and-enhancements) + - [Samesite attribute on cookies](#cookies-samesite) + - [Other features](#other-features) +- [Bugfixes](#bugfixes) + +## Regression test and Security audit{#audit} + +This release has been comprehensively regression tested and passed to a third party for a security-focused audit. + +While it is still advised that you perform your own due diligence when upgrading your project, this work is performed to ensure a safe and secure upgrade with each recipe release. + +## Features and enhancements {#features-and-enhancements} + +### Samesite attribute on cookies {#cookies-samesite} + +The `samesite` attribute is now set on all cookies. To avoid backward compatability issues, the `Lax` value has been set by default, but we recommend reviewing the requirements of your project and setting an appropriate value. + +The default value can be set for all cookies (except for the session cookie) in yaml configuration like so: + +```yml +SilverStripe\Control\Cookie: + default_samesite: 'Strict' +``` + +Check out the [cookies documentation](/developer_guides/cookies_and_sessions/cookies#samesite-attribute) for more information. + +The session cookie is handled separately. You can configure it like so: + +```yml +SilverStripe\Control\Session: + cookie_samesite: 'Strict' +``` + +Note that if you set the `samesite` attribute to `None`, the `secure` is automatically set to `true` as required by the specification. + +For more information about the `samesite` attribute check out https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite + +### Other new features {#other-features} + +## Bugfixes {#bugfixes} + +This release includes a number of bug fixes to improve a broad range of areas. Check the change logs for full details of these fixes split by module. Thank you to the community members that helped contribute these fixes as part of the release! + + + + diff --git a/src/Control/Cookie.php b/src/Control/Cookie.php index 9c48b4ced6d..50f997c1f27 100644 --- a/src/Control/Cookie.php +++ b/src/Control/Cookie.php @@ -19,6 +19,12 @@ class Cookie */ private static $report_errors = true; + /** + * Must be "Strict", "Lax", or "None" + * @config + */ + private static string $default_samesite = 'Lax'; + /** * Fetch the current instance of the cookie backend. * diff --git a/src/Control/CookieJar.php b/src/Control/CookieJar.php index 764d52af5f7..d9c7d129da9 100644 --- a/src/Control/CookieJar.php +++ b/src/Control/CookieJar.php @@ -18,7 +18,6 @@ */ class CookieJar implements Cookie_Backend { - /** * Hold the cookies that were existing at time of instantiation (ie: The ones * sent to PHP by the browser) @@ -168,9 +167,17 @@ protected function outputCookie( $secure = false, $httpOnly = true ) { + $sameSite = $this->getSameSite($name); // if headers aren't sent, we can set the cookie if (!headers_sent($file, $line)) { - return setcookie($name ?? '', $value ?? '', $expiry ?? 0, $path ?? '', $domain ?? '', $secure ?? false, $httpOnly ?? false); + return setcookie($name ?? '', $value ?? '', [ + 'expires' => $expiry ?? 0, + 'path' => $path ?? '', + 'domain' => $domain ?? '', + 'secure' => $sameSite === 'None' ? true : (bool) $secure, + 'httponly' => $httpOnly ?? false, + 'samesite' => $sameSite, + ]); } if (Cookie::config()->uninherited('report_errors')) { @@ -180,4 +187,17 @@ protected function outputCookie( } return false; } + + /** + * Get the correct samesite value - Session cookies use a different configuration variable. + * + * @deprecated 5.0 The relevant methods will include a `$sameSite` parameter instead. + */ + private function getSameSite(string $name): string + { + if ($name === session_name()) { + return Session::config()->get('cookie_samesite'); + } + return Cookie::config()->get('default_samesite'); + } } diff --git a/src/Control/Session.php b/src/Control/Session.php index 1075c22cea1..7c41ade351e 100644 --- a/src/Control/Session.php +++ b/src/Control/Session.php @@ -135,6 +135,12 @@ class Session */ private static $cookie_name_secure = 'SECSESSID'; + /** + * Must be "Strict", "Lax", or "None". + * @config + */ + private static string $cookie_samesite = 'Lax'; + /** * Name of session cache limiter to use. * Defaults to '' to disable cache limiter entirely. @@ -288,7 +294,6 @@ public function start(HTTPRequest $request) $path = Director::baseURL(); } $domain = $this->config()->get('cookie_domain'); - $secure = Director::is_https($request) && $this->config()->get('cookie_secure'); $session_path = $this->config()->get('session_store_path'); $timeout = $this->config()->get('timeout'); @@ -307,7 +312,16 @@ public function start(HTTPRequest $request) $data = []; if (!session_id() && (!headers_sent() || $this->requestContainsSessionId($request))) { if (!headers_sent()) { - session_set_cookie_params($timeout ?: 0, $path, $domain ?: null, $secure, true); + $sameSite = static::config()->get('cookie_samesite'); + $secure = $this->isCookieSecure($sameSite, Director::is_https($request)); + session_set_cookie_params([ + 'lifetime' => $timeout ?: 0, + 'path' => $path, + 'domain' => $domain ?: null, + 'secure' => $secure, + 'httponly' => true, + 'samesite' => $sameSite, + ]); $limiter = $this->config()->get('sessionCacheLimiter'); if (isset($limiter)) { @@ -354,6 +368,17 @@ public function start(HTTPRequest $request) $this->started = true; } + /** + * Determines what the value for the `secure` cookie attribute should be. + */ + private function isCookieSecure(string $sameSite, bool $isHttps): bool + { + if ($sameSite === 'None') { + return true; + } + return $isHttps && $this->config()->get('cookie_secure'); + } + /** * Destroy this session *