Skip to content

Commit

Permalink
ENH Add samesite attribute to cookies.
Browse files Browse the repository at this point in the history
Co-authored-by: pine3ree <[email protected]>
  • Loading branch information
GuySartorelli and pine3ree committed May 30, 2022
1 parent 6f27dad commit 8488833
Show file tree
Hide file tree
Showing 7 changed files with 154 additions and 7 deletions.
13 changes: 11 additions & 2 deletions docs/en/02_Developer_Guides/09_Security/04_Secure_Coding.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
```
Expand All @@ -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.

Expand Down
18 changes: 18 additions & 0 deletions docs/en/02_Developer_Guides/18_Cookies_And_Sessions/01_Cookies.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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`

Expand All @@ -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.
Expand Down
55 changes: 55 additions & 0 deletions docs/en/04_Changelogs/4.12.0.md
Original file line number Diff line number Diff line change
@@ -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!

<!--- Changes below this line will be automatically regenerated -->

<!--- Changes above this line will be automatically regenerated -->
6 changes: 6 additions & 0 deletions src/Control/Cookie.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
24 changes: 22 additions & 2 deletions src/Control/CookieJar.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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')) {
Expand All @@ -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');
}
}
29 changes: 27 additions & 2 deletions src/Control/Session.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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');

Expand All @@ -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)) {
Expand Down Expand Up @@ -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
*
Expand Down

0 comments on commit 8488833

Please sign in to comment.