Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HttpFoundation][FrameworkBundle] fix support for samesite in session cookies #35605

Merged
merged 1 commit into from Feb 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -20,6 +20,7 @@
use Symfony\Component\Config\Definition\ConfigurationInterface;
use Symfony\Component\DependencyInjection\Exception\LogicException;
use Symfony\Component\Form\Form;
use Symfony\Component\HttpFoundation\Cookie;
use Symfony\Component\Lock\Lock;
use Symfony\Component\Lock\Store\SemaphoreStore;
use Symfony\Component\PropertyInfo\PropertyInfoExtractorInterface;
Expand Down Expand Up @@ -490,6 +491,7 @@ private function addSessionSection(ArrayNodeDefinition $rootNode)
->scalarNode('cookie_domain')->end()
->booleanNode('cookie_secure')->end()
->booleanNode('cookie_httponly')->defaultTrue()->end()
->enumNode('cookie_samesite')->values([null, Cookie::SAMESITE_LAX, Cookie::SAMESITE_STRICT])->defaultNull()->end()
->booleanNode('use_cookies')->end()
->scalarNode('gc_divisor')->end()
->scalarNode('gc_probability')->defaultValue(1)->end()
Expand Down
Expand Up @@ -867,7 +867,7 @@ private function registerSessionConfiguration(array $config, ContainerBuilder $c
// session storage
$container->setAlias('session.storage', $config['storage_id'])->setPrivate(true);
$options = ['cache_limiter' => '0'];
foreach (['name', 'cookie_lifetime', 'cookie_path', 'cookie_domain', 'cookie_secure', 'cookie_httponly', 'use_cookies', 'gc_maxlifetime', 'gc_probability', 'gc_divisor', 'use_strict_mode'] as $key) {
foreach (['name', 'cookie_lifetime', 'cookie_path', 'cookie_domain', 'cookie_secure', 'cookie_httponly', 'cookie_samesite', 'use_cookies', 'gc_maxlifetime', 'gc_probability', 'gc_divisor'] as $key) {
if (isset($config[$key])) {
$options[$key] = $config[$key];
}
Expand Down
Expand Up @@ -436,6 +436,7 @@ protected static function getBundleDefaultConfig()
'storage_id' => 'session.storage.native',
'handler_id' => 'session.handler.native_file',
'cookie_httponly' => true,
'cookie_samesite' => null,
'gc_probability' => 1,
'save_path' => '%kernel.cache_dir%/sessions',
'metadata_update_threshold' => '0',
Expand Down
59 changes: 59 additions & 0 deletions src/Symfony/Component/HttpFoundation/Session/SessionUtils.php
@@ -0,0 +1,59 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\HttpFoundation\Session;

/**
* Session utility functions.
*
* @author Nicolas Grekas <p@tchwork.com>
* @author Rémon van de Kamp <rpkamp@gmail.com>
*
* @internal
*/
final class SessionUtils
{
/**
* Find the session header amongst the headers that are to be sent, remove it, and return
* it so the caller can process it further.
*/
public static function popSessionCookie($sessionName, $sessionId)
{
$sessionCookie = null;
$sessionCookiePrefix = sprintf(' %s=', urlencode($sessionName));
$sessionCookieWithId = sprintf('%s%s;', $sessionCookiePrefix, urlencode($sessionId));
$otherCookies = [];
foreach (headers_list() as $h) {
if (0 !== stripos($h, 'Set-Cookie:')) {
continue;
}
if (11 === strpos($h, $sessionCookiePrefix, 11)) {
$sessionCookie = $h;

if (11 !== strpos($h, $sessionCookieWithId, 11)) {
$otherCookies[] = $h;
}
} else {
$otherCookies[] = $h;
}
}
if (null === $sessionCookie) {
return null;
}

header_remove('Set-Cookie');
foreach ($otherCookies as $h) {
header($h, false);
}

return $sessionCookie;
}
}
Expand Up @@ -11,6 +11,8 @@

namespace Symfony\Component\HttpFoundation\Session\Storage\Handler;

use Symfony\Component\HttpFoundation\Session\SessionUtils;

/**
* This abstract session handler provides a generic implementation
* of the PHP 7.0 SessionUpdateTimestampHandlerInterface,
Expand All @@ -27,7 +29,7 @@ abstract class AbstractSessionHandler implements \SessionHandlerInterface, \Sess
private $igbinaryEmptyData;

/**
* {@inheritdoc}
* @return bool
*/
public function open($savePath, $sessionName)
{
Expand Down Expand Up @@ -62,7 +64,7 @@ abstract protected function doWrite($sessionId, $data);
abstract protected function doDestroy($sessionId);

/**
* {@inheritdoc}
* @return bool
*/
public function validateId($sessionId)
{
Expand All @@ -73,7 +75,7 @@ public function validateId($sessionId)
}

/**
* {@inheritdoc}
* @return string
*/
public function read($sessionId)
{
Expand All @@ -99,7 +101,7 @@ public function read($sessionId)
}

/**
* {@inheritdoc}
* @return bool
*/
public function write($sessionId, $data)
{
Expand All @@ -124,7 +126,7 @@ public function write($sessionId, $data)
}

/**
* {@inheritdoc}
* @return bool
*/
public function destroy($sessionId)
{
Expand All @@ -135,31 +137,23 @@ public function destroy($sessionId)
if (!$this->sessionName) {
throw new \LogicException(sprintf('Session name cannot be empty, did you forget to call "parent::open()" in "%s"?.', static::class));
}
$sessionCookie = sprintf(' %s=', urlencode($this->sessionName));
$sessionCookieWithId = sprintf('%s%s;', $sessionCookie, urlencode($sessionId));
$sessionCookieFound = false;
$otherCookies = [];
foreach (headers_list() as $h) {
if (0 !== stripos($h, 'Set-Cookie:')) {
continue;
}
if (11 === strpos($h, $sessionCookie, 11)) {
$sessionCookieFound = true;

if (11 !== strpos($h, $sessionCookieWithId, 11)) {
$otherCookies[] = $h;
}
$cookie = SessionUtils::popSessionCookie($this->sessionName, $sessionId);

/*
* We send an invalidation Set-Cookie header (zero lifetime)
* when either the session was started or a cookie with
* the session name was sent by the client (in which case
* we know it's invalid as a valid session cookie would've
* started the session).
*/
if (null === $cookie || isset($_COOKIE[$this->sessionName])) {
if (\PHP_VERSION_ID < 70300) {
setcookie($this->sessionName, '', 0, ini_get('session.cookie_path'), ini_get('session.cookie_domain'), filter_var(ini_get('session.cookie_secure'), FILTER_VALIDATE_BOOLEAN), filter_var(ini_get('session.cookie_httponly'), FILTER_VALIDATE_BOOLEAN));
} else {
$otherCookies[] = $h;
}
}
if ($sessionCookieFound) {
header_remove('Set-Cookie');
foreach ($otherCookies as $h) {
header($h, false);
$params = session_get_cookie_params();
unset($params['lifetime']);
setcookie($this->sessionName, '', $params);
}
} else {
setcookie($this->sessionName, '', 0, ini_get('session.cookie_path'), ini_get('session.cookie_domain'), filter_var(ini_get('session.cookie_secure'), FILTER_VALIDATE_BOOLEAN), filter_var(ini_get('session.cookie_httponly'), FILTER_VALIDATE_BOOLEAN));
}
}

Expand Down
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Component\HttpFoundation\Session\Storage;

use Symfony\Component\HttpFoundation\Session\SessionBagInterface;
use Symfony\Component\HttpFoundation\Session\SessionUtils;
use Symfony\Component\HttpFoundation\Session\Storage\Handler\StrictSessionHandler;
use Symfony\Component\HttpFoundation\Session\Storage\Proxy\AbstractProxy;
use Symfony\Component\HttpFoundation\Session\Storage\Proxy\SessionHandlerProxy;
Expand Down Expand Up @@ -48,6 +49,11 @@ class NativeSessionStorage implements SessionStorageInterface
*/
protected $metadataBag;

/**
* @var string|null
*/
private $emulateSameSite;

/**
* Depending on how you want the storage driver to behave you probably
* want to override this constructor entirely.
Expand All @@ -67,6 +73,7 @@ class NativeSessionStorage implements SessionStorageInterface
* cookie_lifetime, "0"
* cookie_path, "/"
* cookie_secure, ""
* cookie_samesite, null
* entropy_file, ""
* entropy_length, "0"
* gc_divisor, "100"
Expand Down Expand Up @@ -94,9 +101,7 @@ class NativeSessionStorage implements SessionStorageInterface
* trans_sid_hosts, $_SERVER['HTTP_HOST']
* trans_sid_tags, "a=href,area=href,frame=src,form="
*
* @param array $options Session configuration options
* @param \SessionHandlerInterface|null $handler
* @param MetadataBag $metaBag MetadataBag
* @param AbstractProxy|\SessionHandlerInterface|null $handler
*/
public function __construct(array $options = [], $handler = null, MetadataBag $metaBag = null)
{
Expand Down Expand Up @@ -150,6 +155,13 @@ public function start()
throw new \RuntimeException('Failed to start the session');
}

if (null !== $this->emulateSameSite) {
$originalCookie = SessionUtils::popSessionCookie(session_name(), session_id());
if (null !== $originalCookie) {
header(sprintf('%s; SameSite=%s', $originalCookie, $this->emulateSameSite), false);
}
}

$this->loadSession();

return true;
Expand Down Expand Up @@ -215,6 +227,13 @@ public function regenerate($destroy = false, $lifetime = null)
// @see https://bugs.php.net/70013
$this->loadSession();

if (null !== $this->emulateSameSite) {
$originalCookie = SessionUtils::popSessionCookie(session_name(), session_id());
if (null !== $originalCookie) {
header(sprintf('%s; SameSite=%s', $originalCookie, $this->emulateSameSite), false);
}
}

return $isRegenerated;
}

Expand All @@ -223,6 +242,7 @@ public function regenerate($destroy = false, $lifetime = null)
*/
public function save()
{
// Store a copy so we can restore the bags in case the session was not left empty
$session = $_SESSION;

foreach ($this->bags as $bag) {
Expand All @@ -248,7 +268,11 @@ public function save()
session_write_close();
} finally {
restore_error_handler();
$_SESSION = $session;

// Restore only if not empty
if ($_SESSION) {
$_SESSION = $session;
}
}

$this->closed = true;
Expand Down Expand Up @@ -347,7 +371,7 @@ public function setOptions(array $options)

$validOptions = array_flip([
'cache_expire', 'cache_limiter', 'cookie_domain', 'cookie_httponly',
'cookie_lifetime', 'cookie_path', 'cookie_secure',
'cookie_lifetime', 'cookie_path', 'cookie_secure', 'cookie_samesite',
'entropy_file', 'entropy_length', 'gc_divisor',
'gc_maxlifetime', 'gc_probability', 'hash_bits_per_character',
'hash_function', 'lazy_write', 'name', 'referer_check',
Expand All @@ -360,6 +384,12 @@ public function setOptions(array $options)

foreach ($options as $key => $value) {
if (isset($validOptions[$key])) {
if ('cookie_samesite' === $key && \PHP_VERSION_ID < 70300) {
// PHP < 7.3 does not support same_site cookies. We will emulate it in
// the start() method instead.
$this->emulateSameSite = $value;
continue;
}
ini_set('url_rewriter.tags' !== $key ? 'session.'.$key : $key, $value);
}
}
Expand All @@ -381,7 +411,7 @@ public function setOptions(array $options)
* @see https://php.net/sessionhandlerinterface
* @see https://php.net/sessionhandler
*
* @param \SessionHandlerInterface|null $saveHandler
* @param AbstractProxy|\SessionHandlerInterface|null $saveHandler
*
* @throws \InvalidArgumentException
*/
Expand Down
Expand Up @@ -11,10 +11,11 @@ $_SESSION is not empty
write
destroy
close
$_SESSION is not empty
$_SESSION is empty
Array
(
[0] => Content-Type: text/plain; charset=utf-8
[1] => Cache-Control: max-age=0, private, must-revalidate
[2] => Set-Cookie: sid=deleted; expires=Thu, 01-Jan-1970 00:00:01 GMT; Max-Age=0; path=/; secure; HttpOnly
)
shutdown
Expand Up @@ -20,5 +20,6 @@ Array
[0] => Content-Type: text/plain; charset=utf-8
[1] => Cache-Control: max-age=10800, private, must-revalidate
[2] => Set-Cookie: abc=def
[3] => Set-Cookie: sid=deleted; expires=Thu, 01-Jan-1970 00:00:01 GMT; Max-Age=0; path=/; secure; HttpOnly
)
shutdown
@@ -0,0 +1,16 @@
open
validateId
read
doRead:
read

write
doWrite: foo|s:3:"bar";
close
Array
(
[0] => Content-Type: text/plain; charset=utf-8
[1] => Cache-Control: max-age=0, private, must-revalidate
[2] => Set-Cookie: sid=random_session_id; path=/; secure; HttpOnly; SameSite=lax
)
shutdown
@@ -0,0 +1,13 @@
<?php

require __DIR__.'/common.inc';

use Symfony\Component\HttpFoundation\Session\Storage\NativeSessionStorage;

$storage = new NativeSessionStorage(['cookie_samesite' => 'lax']);
$storage->setSaveHandler(new TestSessionHandler());
$storage->start();

$_SESSION = ['foo' => 'bar'];

ob_start(function ($buffer) { return str_replace(session_id(), 'random_session_id', $buffer); });
@@ -0,0 +1,23 @@
open
validateId
read
doRead:
read
destroy
close
open
validateId
read
doRead:
read

write
doWrite: foo|s:3:"bar";
close
Array
(
[0] => Content-Type: text/plain; charset=utf-8
[1] => Cache-Control: max-age=0, private, must-revalidate
[2] => Set-Cookie: sid=random_session_id; path=/; secure; HttpOnly; SameSite=lax
)
shutdown