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

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

@@ -20,6 +20,7 @@ use Symfony\Component\Config\Definition\Builder\TreeBuilder;
 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;
@@ -490,6 +491,7 @@ class Configuration implements ConfigurationInterface
                         ->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()

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

@@ -867,7 +867,7 @@ class FrameworkExtension extends Extension
         // 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];
             }

src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php

@@ -436,6 +436,7 @@ class ConfigurationTest extends TestCase
                 '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',

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

src/Symfony/Component/HttpFoundation/Session/Storage/Handler/AbstractSessionHandler.php

@@ -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,
@@ -27,7 +29,7 @@ abstract class AbstractSessionHandler implements \SessionHandlerInterface, \Sess
     private $igbinaryEmptyData;
 
     /**
-     * {@inheritdoc}
+     * @return bool
      */
     public function open($savePath, $sessionName)
     {
@@ -62,7 +64,7 @@ abstract class AbstractSessionHandler implements \SessionHandlerInterface, \Sess
     abstract protected function doDestroy($sessionId);
 
     /**
-     * {@inheritdoc}
+     * @return bool
      */
     public function validateId($sessionId)
     {
@@ -73,7 +75,7 @@ abstract class AbstractSessionHandler implements \SessionHandlerInterface, \Sess
     }
 
     /**
-     * {@inheritdoc}
+     * @return string
      */
     public function read($sessionId)
     {
@@ -99,7 +101,7 @@ abstract class AbstractSessionHandler implements \SessionHandlerInterface, \Sess
     }
 
     /**
-     * {@inheritdoc}
+     * @return bool
      */
     public function write($sessionId, $data)
     {
@@ -124,7 +126,7 @@ abstract class AbstractSessionHandler implements \SessionHandlerInterface, \Sess
     }
 
     /**
-     * {@inheritdoc}
+     * @return bool
      */
     public function destroy($sessionId)
     {
@@ -135,32 +137,24 @@ abstract class AbstractSessionHandler implements \SessionHandlerInterface, \Sess
             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;
+            $cookie = SessionUtils::popSessionCookie($this->sessionName, $sessionId);
 
-                    if (11 !== strpos($h, $sessionCookieWithId, 11)) {
-                        $otherCookies[] = $h;
-                    }
+            /*
+             * 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;
+                    $params = session_get_cookie_params();
+                    unset($params['lifetime']);
+                    setcookie($this->sessionName, '', $params);
                 }
             }
-            if ($sessionCookieFound) {
-                header_remove('Set-Cookie');
-                foreach ($otherCookies as $h) {
-                    header($h, false);
-                }
-            } 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));
-            }
         }
 
         return $this->newSessionId === $sessionId || $this->doDestroy($sessionId);

src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php

@@ -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;
@@ -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.
@@ -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"
@@ -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)
     {
@@ -150,6 +155,13 @@ class NativeSessionStorage implements SessionStorageInterface
             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;
@@ -215,6 +227,13 @@ class NativeSessionStorage implements SessionStorageInterface
         // @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;
     }
 
@@ -223,6 +242,7 @@ class NativeSessionStorage implements SessionStorageInterface
      */
     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) {
@@ -248,7 +268,11 @@ class NativeSessionStorage implements SessionStorageInterface
             session_write_close();
         } finally {
             restore_error_handler();
-            $_SESSION = $session;
+
+            // Restore only if not empty
+            if ($_SESSION) {
+                $_SESSION = $session;
+            }
         }
 
         $this->closed = true;
@@ -347,7 +371,7 @@ class NativeSessionStorage implements SessionStorageInterface
 
         $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',
@@ -360,6 +384,12 @@ class NativeSessionStorage implements SessionStorageInterface
 
         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);
             }
         }
@@ -381,7 +411,7 @@ class NativeSessionStorage implements SessionStorageInterface
      * @see https://php.net/sessionhandlerinterface
      * @see https://php.net/sessionhandler
      *
-     * @param \SessionHandlerInterface|null $saveHandler
+     * @param AbstractProxy|\SessionHandlerInterface|null $saveHandler
      *
      * @throws \InvalidArgumentException
      */

src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/Fixtures/storage.expected

@@ -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

src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/Fixtures/with_cookie_and_session.expected

@@ -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

src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/Fixtures/with_samesite.expected

@@ -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

src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/Fixtures/with_samesite.php

@@ -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); });

src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/Fixtures/with_samesite_and_migration.expected

@@ -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

src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/Fixtures/with_samesite_and_migration.php

@@ -0,0 +1,15 @@
+<?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'];
+
+$storage->regenerate(true);
+
+ob_start(function ($buffer) { return preg_replace('~_sf2_meta.*$~m', '', str_replace(session_id(), 'random_session_id', $buffer)); });

src/Symfony/Component/HttpFoundation/Tests/Session/Storage/NativeSessionStorageTest.php

@@ -36,7 +36,7 @@ class NativeSessionStorageTest extends TestCase
     protected function setUp()
     {
         $this->iniSet('session.save_handler', 'files');
-        $this->iniSet('session.save_path', $this->savePath = sys_get_temp_dir().'/sf2test');
+        $this->iniSet('session.save_path', $this->savePath = sys_get_temp_dir().'/sftest');
         if (!is_dir($this->savePath)) {
             mkdir($this->savePath);
         }
@@ -167,6 +167,10 @@ class NativeSessionStorageTest extends TestCase
             'cookie_httponly' => false,
         ];
 
+        if (\PHP_VERSION_ID >= 70300) {
+            $options['cookie_samesite'] = 'lax';
+        }
+
         $this->getStorage($options);
         $temp = session_get_cookie_params();
         $gco = [];
@@ -175,8 +179,6 @@ class NativeSessionStorageTest extends TestCase
             $gco['cookie_'.$key] = $value;
         }
 
-        unset($gco['cookie_samesite']);
-
         $this->assertEquals($options, $gco);
     }