From 4091feb6934f8fa227c7eb19a4d739d316eabf6e Mon Sep 17 00:00:00 2001 From: Remon van de Kamp Date: Wed, 8 Aug 2018 22:20:43 +0200 Subject: [PATCH] Add SameSite cookies to FrameWorkBundle Uses `session.cookie_samesite` for PHP >= 7.3. For PHP < 7.3 it first does a session_start(), find the emitted header, changes it, and emits it again with the value for SameSite added. --- UPGRADE-4.2.md | 5 +- UPGRADE-5.0.md | 1 + .../Bundle/FrameworkBundle/CHANGELOG.md | 1 + .../DependencyInjection/Configuration.php | 2 + .../FrameworkExtension.php | 2 +- .../DependencyInjection/ConfigurationTest.php | 1 + .../HttpFoundation/Session/SessionUtils.php | 59 +++++++++++++++++++ .../Handler/AbstractSessionHandler.php | 30 +++------- .../Session/Storage/NativeSessionStorage.php | 22 ++++++- .../Handler/Fixtures/with_samesite.expected | 16 +++++ .../Handler/Fixtures/with_samesite.php | 13 ++++ .../Storage/NativeSessionStorageTest.php | 4 ++ 12 files changed, 129 insertions(+), 27 deletions(-) create mode 100644 src/Symfony/Component/HttpFoundation/Session/SessionUtils.php create mode 100644 src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/Fixtures/with_samesite.expected create mode 100644 src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/Fixtures/with_samesite.php diff --git a/UPGRADE-4.2.md b/UPGRADE-4.2.md index bfc72e947d0f7..5f57f5dc71b04 100644 --- a/UPGRADE-4.2.md +++ b/UPGRADE-4.2.md @@ -42,14 +42,14 @@ Form * Deprecated calling `FormRenderer::searchAndRenderBlock` for fields which were already rendered. Instead of expecting such calls to return empty strings, check if the field has already been rendered. - + Before: ```twig {% for field in fieldsWithPotentialDuplicates %} {{ form_widget(field) }} {% endfor %} ``` - + After: ```twig {% for field in fieldsWithPotentialDuplicates if not field.rendered %} @@ -83,6 +83,7 @@ FrameworkBundle is UTF-8 (see kernel's `getCharset()` method), it is recommended to set it to `true`: this will generate 404s for non-UTF-8 URLs, which are incompatible with you app anyway, and will allow dumping optimized routers and using Unicode classes in requirements. + * Added support for the SameSite attribute for session cookies. It is highly recommended to set this setting (`framework.session.cookie_samesite`) to `lax` for increased security against CSRF attacks. Messenger --------- diff --git a/UPGRADE-5.0.md b/UPGRADE-5.0.md index 58a08d5d69358..f23329b7d5800 100644 --- a/UPGRADE-5.0.md +++ b/UPGRADE-5.0.md @@ -92,6 +92,7 @@ FrameworkBundle * Warming up a router in `RouterCacheWarmer` that does not implement the `WarmableInterface` is not supported anymore. * The `RequestDataCollector` class has been removed. Use the `Symfony\Component\HttpKernel\DataCollector\RequestDataCollector` class instead. * Removed `Symfony\Bundle\FrameworkBundle\Controller\Controller`. Use `Symfony\Bundle\FrameworkBundle\Controller\AbstractController` instead. + * Added support for the SameSite attribute for session cookies. It is highly recommended to set this setting (`framework.session.cookie_samesite`) to `lax` for increased security against CSRF attacks. HttpFoundation -------------- diff --git a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md index 6c2dea7e5e168..b2e4efab73bdb 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md +++ b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md @@ -29,6 +29,7 @@ CHANGELOG * Deprecated `Symfony\Bundle\FrameworkBundle\Controller\ControllerNameParser` * The `container.service_locator` tag of `ServiceLocator`s is now autoconfigured. * Add the ability to search a route in `debug:router`. + * Add the ability to use SameSite cookies for sessions. 4.0.0 ----- diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php index 6625570009b48..3e59f00bc5dd4 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php @@ -19,6 +19,7 @@ use Symfony\Component\Config\Definition\Builder\TreeBuilder; use Symfony\Component\Config\Definition\ConfigurationInterface; use Symfony\Component\Form\Form; +use Symfony\Component\HttpFoundation\Cookie; use Symfony\Component\Lock\Lock; use Symfony\Component\Lock\Store\SemaphoreStore; use Symfony\Component\Messenger\MessageBusInterface; @@ -484,6 +485,7 @@ private function addSessionSection(ArrayNodeDefinition $rootNode) ->scalarNode('cookie_domain')->end() ->enumNode('cookie_secure')->values(array(true, false, 'auto'))->end() ->booleanNode('cookie_httponly')->defaultTrue()->end() + ->enumNode('cookie_samesite')->values(array(null, Cookie::SAMESITE_LAX, Cookie::SAMESITE_STRICT))->defaultNull()->end() ->booleanNode('use_cookies')->end() ->scalarNode('gc_divisor')->end() ->scalarNode('gc_probability')->defaultValue(1)->end() diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 289bd67aca22b..240e29a84e5de 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -757,7 +757,7 @@ private function registerSessionConfiguration(array $config, ContainerBuilder $c // session storage $container->setAlias('session.storage', $config['storage_id'])->setPrivate(true); $options = array('cache_limiter' => '0'); - foreach (array('name', 'cookie_lifetime', 'cookie_path', 'cookie_domain', 'cookie_secure', 'cookie_httponly', 'use_cookies', 'gc_maxlifetime', 'gc_probability', 'gc_divisor') as $key) { + foreach (array('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]; } diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php index 8dbda3e78deed..6e762d6c9bf42 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php @@ -232,6 +232,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', diff --git a/src/Symfony/Component/HttpFoundation/Session/SessionUtils.php b/src/Symfony/Component/HttpFoundation/Session/SessionUtils.php new file mode 100644 index 0000000000000..a377e7fc2027f --- /dev/null +++ b/src/Symfony/Component/HttpFoundation/Session/SessionUtils.php @@ -0,0 +1,59 @@ + + * + * 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 + * @author Rémon van de Kamp + * + * @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(string $sessionName, string $sessionId): ?string + { + $sessionCookie = null; + $sessionCookiePrefix = sprintf(' %s=', urlencode($sessionName)); + $sessionCookieWithId = sprintf('%s%s;', $sessionCookiePrefix, urlencode($sessionId)); + $otherCookies = array(); + 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; + } +} diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/AbstractSessionHandler.php b/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/AbstractSessionHandler.php index b1465716cbb3a..c521bf7fcaa46 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/AbstractSessionHandler.php +++ b/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, @@ -121,31 +123,13 @@ 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"?.', \get_class($this))); } - $sessionCookie = sprintf(' %s=', urlencode($this->sessionName)); - $sessionCookieWithId = sprintf('%s%s;', $sessionCookie, urlencode($sessionId)); - $sessionCookieFound = false; - $otherCookies = array(); - 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); + if (null === $cookie) { + if (\PHP_VERSION_ID < 70300) { + setcookie($this->sessionName, '', 0, ini_get('session.cookie_path'), ini_get('session.cookie_domain'), ini_get('session.cookie_secure'), ini_get('session.cookie_httponly')); } else { - $otherCookies[] = $h; - } - } - if ($sessionCookieFound) { - header_remove('Set-Cookie'); - foreach ($otherCookies as $h) { - header($h, false); + setcookie($this->sessionName, '', 0, ini_get('session.cookie_path'), ini_get('session.cookie_domain'), ini_get('session.cookie_secure'), ini_get('session.cookie_httponly'), ini_get('session.cookie_samesite')); } - } else { - setcookie($this->sessionName, '', 0, ini_get('session.cookie_path'), ini_get('session.cookie_domain'), ini_get('session.cookie_secure'), ini_get('session.cookie_httponly')); } } diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php b/src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php index bf54f19e3fe82..04f90a30fd4ce 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php +++ b/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 * gc_divisor, "100" * gc_maxlifetime, "1440" * gc_probability, "1" @@ -143,6 +150,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)); + } + } + $this->loadSession(); return true; @@ -347,7 +361,7 @@ public function setOptions(array $options) $validOptions = array_flip(array( 'cache_expire', 'cache_limiter', 'cookie_domain', 'cookie_httponly', - 'cookie_lifetime', 'cookie_path', 'cookie_secure', + 'cookie_lifetime', 'cookie_path', 'cookie_secure', 'cookie_samesite', 'gc_divisor', 'gc_maxlifetime', 'gc_probability', 'lazy_write', 'name', 'referer_check', 'serialize_handler', 'use_strict_mode', 'use_cookies', @@ -359,6 +373,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); } } diff --git a/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/Fixtures/with_samesite.expected b/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/Fixtures/with_samesite.expected new file mode 100644 index 0000000000000..d20fb88ec052f --- /dev/null +++ b/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 diff --git a/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/Fixtures/with_samesite.php b/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/Fixtures/with_samesite.php new file mode 100644 index 0000000000000..2d32792a8f818 --- /dev/null +++ b/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/Fixtures/with_samesite.php @@ -0,0 +1,13 @@ + 'lax')); +$storage->setSaveHandler(new TestSessionHandler()); +$storage->start(); + +$_SESSION = array('foo' => 'bar'); + +ob_start(function ($buffer) { return str_replace(session_id(), 'random_session_id', $buffer); }); diff --git a/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/NativeSessionStorageTest.php b/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/NativeSessionStorageTest.php index 52da2947cbcb7..fbb951163773f 100644 --- a/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/NativeSessionStorageTest.php +++ b/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/NativeSessionStorageTest.php @@ -171,6 +171,10 @@ public function testCookieOptions() 'cookie_httponly' => false, ); + if (\PHP_VERSION_ID >= 70300) { + $options['cookie_samesite'] = 'lax'; + } + $this->getStorage($options); $temp = session_get_cookie_params(); $gco = array();