Permalink
Browse files

feature #24523 [HttpFoundation] Make sessions secure and lazy (nicola…

…s-grekas)

This PR was merged into the 3.4 branch.

Discussion
----------

[HttpFoundation] Make sessions secure and lazy

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | not yet
| Fixed tickets | #6388, #6036, #12375, #12325
| License       | MIT
| Doc PR        | -

The `SessionUpdateTimestampHandlerInterface` (new to PHP 7.0) is mostly undocumented, and just not implemented anywhere. Yet, it's required to implement session fixation preventions and lazy write in userland session handlers (there is https://wiki.php.net/rfc/session-read_only-lazy_write which describes the behavior.)

By implementing it, we would make Symfony session handling much better and stronger. Meanwhile, doing some cookie headers management, this also gives the opportunity to fix the "don't start if session is only read issue".

So, here we are for the general idea. Now needs more (and green) tests, and review of course.

Commits
-------

347939c [HttpFoundation] Make sessions secure and lazy
  • Loading branch information...
fabpot committed Oct 16, 2017
2 parents f757cd6 + 347939c commit 1f4025a0aea5f87c0fc7f1c419eb6df5022a9be7
Showing with 984 additions and 104 deletions.
  1. +5 −8 UPGRADE-3.4.md
  2. +6 −5 UPGRADE-4.0.md
  3. +1 −1 composer.json
  4. +1 −0 src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md
  5. +5 −2 src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
  6. +1 −8 src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
  7. +9 −3 src/Symfony/Bundle/FrameworkBundle/Resources/config/session.xml
  8. +1 −0 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php
  9. +3 −2 src/Symfony/Component/HttpFoundation/CHANGELOG.md
  10. +165 −0 src/Symfony/Component/HttpFoundation/Session/Storage/Handler/AbstractSessionHandler.php
  11. +9 −9 src/Symfony/Component/HttpFoundation/Session/Storage/Handler/MemcachedSessionHandler.php
  12. +32 −13 src/Symfony/Component/HttpFoundation/Session/Storage/Handler/MongoDbSessionHandler.php
  13. +4 −2 src/Symfony/Component/HttpFoundation/Session/Storage/Handler/NativeSessionHandler.php
  14. +14 −6 src/Symfony/Component/HttpFoundation/Session/Storage/Handler/NullSessionHandler.php
  15. +35 −9 src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php
  16. +89 −0 src/Symfony/Component/HttpFoundation/Session/Storage/Handler/StrictSessionHandler.php
  17. +4 −0 src/Symfony/Component/HttpFoundation/Session/Storage/Handler/WriteCheckSessionHandler.php
  18. +27 −17 src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php
  19. +0 −4 src/Symfony/Component/HttpFoundation/Session/Storage/Proxy/AbstractProxy.php
  20. +0 −4 src/Symfony/Component/HttpFoundation/Session/Storage/Proxy/SessionHandlerProxy.php
  21. +61 −0 src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/AbstractSessionHandlerTest.php
  22. +152 −0 src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/Fixtures/common.inc
  23. +17 −0 src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/Fixtures/empty_destroys.expected
  24. +8 −0 src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/Fixtures/empty_destroys.php
  25. +14 −0 src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/Fixtures/read_only.expected
  26. +8 −0 src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/Fixtures/read_only.php
  27. +24 −0 src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/Fixtures/regenerate.expected
  28. +10 −0 src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/Fixtures/regenerate.php
  29. +20 −0 src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/Fixtures/storage.expected
  30. +24 −0 src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/Fixtures/storage.php
  31. +15 −0 src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/Fixtures/with_cookie.expected
  32. +8 −0 src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/Fixtures/with_cookie.php
  33. +3 −0 src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/NativeSessionHandlerTest.php
  34. +3 −0 src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/PdoSessionHandlerTest.php
  35. +189 −0 src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/StrictSessionHandlerTest.php
  36. +2 −0 src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/WriteCheckSessionHandlerTest.php
  37. +4 −4 src/Symfony/Component/HttpFoundation/Tests/Session/Storage/NativeSessionStorageTest.php
  38. +0 −2 src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Proxy/AbstractProxyTest.php
  39. +0 −1 src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Proxy/SessionHandlerProxyTest.php
  40. +2 −1 src/Symfony/Component/HttpFoundation/composer.json
  41. +9 −3 src/Symfony/Component/Security/Csrf/TokenStorage/NativeSessionTokenStorage.php
View
@@ -126,6 +126,8 @@ Form
FrameworkBundle
---------------
* The `session.use_strict_mode` option has been deprecated and is enabled by default.
* The `cache:clear` command doesn't clear "app" PSR-6 cache pools anymore,
but still clears "system" ones.
Use the `cache:pool:clear` command to clear "app" pools instead.
@@ -235,18 +237,13 @@ HttpFoundation
* The `Symfony\Component\HttpFoundation\Session\Storage\Handler\NativeSessionHandler`
class has been deprecated and will be removed in 4.0. Use the `\SessionHandler` class instead.
* The `Symfony\Component\HttpFoundation\Session\Storage\Proxy\AbstractProxy` class has been
deprecated and will be removed in 4.0. Use your `\SessionHandlerInterface` implementation directly.
* The `Symfony\Component\HttpFoundation\Session\Storage\Handler\WriteCheckSessionHandler` class has been
deprecated and will be removed in 4.0. Implement `SessionUpdateTimestampHandlerInterface` or
extend `AbstractSessionHandler` instead.
* The `Symfony\Component\HttpFoundation\Session\Storage\Proxy\NativeProxy` class has been
deprecated and will be removed in 4.0. Use your `\SessionHandlerInterface` implementation directly.
* The `Symfony\Component\HttpFoundation\Session\Storage\Proxy\SessionHandlerProxy` class has been
deprecated and will be removed in 4.0. Use your `\SessionHandlerInterface` implementation directly.
* `NativeSessionStorage::setSaveHandler()` now takes an instance of `\SessionHandlerInterface` as argument.
Not passing it is deprecated and will throw a `TypeError` in 4.0.
* Using `Symfony\Component\HttpFoundation\Session\Storage\Handler\MongoDbSessionHandler` with the legacy mongo extension
has been deprecated and will be removed in 4.0. Use it with the mongodb/mongodb package and ext-mongodb instead.
View
@@ -329,6 +329,8 @@ Form
FrameworkBundle
---------------
* The `session.use_strict_mode` option has been removed and strict mode is always enabled.
* The `validator.mapping.cache.doctrine.apc` service has been removed.
* The "framework.trusted_proxies" configuration option and the corresponding "kernel.trusted_proxies" parameter have been removed. Use the `Request::setTrustedProxies()` method in your front controller instead.
@@ -542,12 +544,11 @@ HttpFoundation
* The ability to check only for cacheable HTTP methods using `Request::isMethodSafe()` is
not supported anymore, use `Request::isMethodCacheable()` instead.
* The `Symfony\Component\HttpFoundation\Session\Storage\Handler\NativeSessionHandler`,
`Symfony\Component\HttpFoundation\Session\Storage\Proxy\AbstractProxy`,
`Symfony\Component\HttpFoundation\Session\Storage\Proxy\NativeProxy` and
`Symfony\Component\HttpFoundation\Session\Storage\Proxy\SessionHandlerProxy` classes have been removed.
* The `Symfony\Component\HttpFoundation\Session\Storage\Handler\WriteCheckSessionHandler` class has been
removed. Implement `SessionUpdateTimestampHandlerInterface` or extend `AbstractSessionHandler` instead.
* `NativeSessionStorage::setSaveHandler()` now requires an instance of `\SessionHandlerInterface` as argument.
* The `Symfony\Component\HttpFoundation\Session\Storage\Handler\NativeSessionHandler` and
`Symfony\Component\HttpFoundation\Session\Storage\Proxy\NativeProxy` classes have been removed.
* The `Symfony\Component\HttpFoundation\Session\Storage\Handler\MongoDbSessionHandler` does not work with the legacy
mongo extension anymore. It requires mongodb/mongodb package and ext-mongodb.
View
@@ -30,7 +30,7 @@
"symfony/polyfill-intl-icu": "~1.0",
"symfony/polyfill-mbstring": "~1.0",
"symfony/polyfill-php56": "~1.0",
"symfony/polyfill-php70": "~1.0",
"symfony/polyfill-php70": "~1.6",
"symfony/polyfill-util": "~1.0"
},
"replace": {
@@ -4,6 +4,7 @@ CHANGELOG
3.4.0
-----
* Session `use_strict_mode` is now enabled by default and the corresponding option has been deprecated
* Made the `cache:clear` command to *not* clear "app" PSR-6 cache pools anymore,
but to still clear "system" ones; use the `cache:pool:clear` command to clear "app" pools instead
* Always register a minimalist logger that writes in `stderr`
@@ -462,11 +462,14 @@ private function addSessionSection(ArrayNodeDefinition $rootNode)
->scalarNode('gc_divisor')->end()
->scalarNode('gc_probability')->defaultValue(1)->end()
->scalarNode('gc_maxlifetime')->end()
->booleanNode('use_strict_mode')->end()
->booleanNode('use_strict_mode')
->defaultTrue()
->setDeprecated('The "%path%.%node%" option is enabled by default and deprecated since Symfony 3.4. It will be always enabled in 4.0.')
->end()
->scalarNode('save_path')->defaultValue('%kernel.cache_dir%/sessions')->end()
->integerNode('metadata_update_threshold')
->defaultValue('0')
->info('seconds to wait between 2 session metadata updates, it will also prevent the session handler to write if the session has not changed')
->info('seconds to wait between 2 session metadata updates')
->end()
->end()
->end()
@@ -916,14 +916,7 @@ private function registerSessionConfiguration(array $config, ContainerBuilder $c
$container->getDefinition('session.storage.native')->replaceArgument(1, null);
$container->getDefinition('session.storage.php_bridge')->replaceArgument(0, null);
} else {
$handlerId = $config['handler_id'];
if ($config['metadata_update_threshold'] > 0) {
$container->getDefinition('session.handler.write_check')->addArgument(new Reference($handlerId));
$handlerId = 'session.handler.write_check';
}
$container->setAlias('session.handler', $handlerId)->setPrivate(true);
$container->setAlias('session.handler', $config['handler_id'])->setPrivate(true);
}
$container->setParameter('session.save_path', $config['save_path']);
@@ -48,11 +48,17 @@
<argument type="service" id="session.storage.metadata_bag" />
</service>
<service id="session.handler.native_file" class="Symfony\Component\HttpFoundation\Session\Storage\Handler\NativeFileSessionHandler">
<argument>%session.save_path%</argument>
<service id="session.handler.native_file" class="Symfony\Component\HttpFoundation\Session\Storage\Handler\StrictSessionHandler">
<argument type="service">
<service class="Symfony\Component\HttpFoundation\Session\Storage\Handler\NativeFileSessionHandler">
<argument>%session.save_path%</argument>
</service>
</argument>
</service>
<service id="session.handler.write_check" class="Symfony\Component\HttpFoundation\Session\Storage\Handler\WriteCheckSessionHandler" />
<service id="session.handler.write_check" class="Symfony\Component\HttpFoundation\Session\Storage\Handler\WriteCheckSessionHandler">
<deprecated>The "%service_id%" service is deprecated since Symfony 3.4 and will be removed in 4.0. Use the `session.lazy_write` ini setting instead.</deprecated>
</service>
<service id="session_listener" class="Symfony\Component\HttpKernel\EventListener\SessionListener">
<tag name="kernel.event_subscriber" />
@@ -301,6 +301,7 @@ protected static function getBundleDefaultConfig()
'gc_probability' => 1,
'save_path' => '%kernel.cache_dir%/sessions',
'metadata_update_threshold' => '0',
'use_strict_mode' => true,
),
'request' => array(
'enabled' => false,
@@ -4,8 +4,9 @@ CHANGELOG
3.4.0
-----
* deprecated the `NativeSessionHandler` class,
* deprecated the `AbstractProxy`, `NativeProxy` and `SessionHandlerProxy` classes,
* implemented PHP 7.0's `SessionUpdateTimestampHandlerInterface` with a new
`AbstractSessionHandler` base class and a new `StrictSessionHandler` wrapper
* deprecated the `WriteCheckSessionHandler`, `NativeSessionHandler` and `NativeProxy` classes
* deprecated setting session save handlers that do not implement `\SessionHandlerInterface` in `NativeSessionStorage::setSaveHandler()`
* deprecated using `MongoDbSessionHandler` with the legacy mongo extension; use it with the mongodb/mongodb package and ext-mongodb instead
* deprecated `MemcacheSessionHandler`; use `MemcachedSessionHandler` instead
@@ -0,0 +1,165 @@
<?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\Storage\Handler;
/**
* This abstract session handler provides a generic implementation
* of the PHP 7.0 SessionUpdateTimestampHandlerInterface,
* enabling strict and lazy session handling.
*
* @author Nicolas Grekas <p@tchwork.com>
*/
abstract class AbstractSessionHandler implements \SessionHandlerInterface, \SessionUpdateTimestampHandlerInterface
{
private $sessionName;
private $prefetchId;
private $prefetchData;
private $newSessionId;
private $igbinaryEmptyData;
/**
* {@inheritdoc}
*/
public function open($savePath, $sessionName)
{
$this->sessionName = $sessionName;
return true;
}
/**
* @param string $sessionId
*
* @return string
*/
abstract protected function doRead($sessionId);
/**
* @param string $sessionId
* @param string $data
*
* @return bool
*/
abstract protected function doWrite($sessionId, $data);
/**
* @param string $sessionId
*
* @return bool
*/
abstract protected function doDestroy($sessionId);
/**
* {@inheritdoc}
*/
public function validateId($sessionId)
{
$this->prefetchData = $this->read($sessionId);
$this->prefetchId = $sessionId;
return '' !== $this->prefetchData;
}
/**
* {@inheritdoc}
*/
public function read($sessionId)
{
if (null !== $this->prefetchId) {
$prefetchId = $this->prefetchId;
$prefetchData = $this->prefetchData;
$this->prefetchId = $this->prefetchData = null;
if ($prefetchId === $sessionId || '' === $prefetchData) {
$this->newSessionId = '' === $prefetchData ? $sessionId : null;
return $prefetchData;
}
}
$data = $this->doRead($sessionId);
$this->newSessionId = '' === $data ? $sessionId : null;
if (\PHP_VERSION_ID < 70000) {
$this->prefetchData = $data;
}
return $data;
}
/**
* {@inheritdoc}
*/
public function write($sessionId, $data)
{
if (\PHP_VERSION_ID < 70000 && $this->prefetchData) {
$readData = $this->prefetchData;
$this->prefetchData = null;
if ($readData === $data) {
return $this->updateTimestamp($sessionId, $data);
}
}
if (null === $this->igbinaryEmptyData) {
// see https://github.com/igbinary/igbinary/issues/146
$this->igbinaryEmptyData = \function_exists('igbinary_serialize') ? igbinary_serialize(array()) : '';
}
if ('' === $data || $this->igbinaryEmptyData === $data) {
return $this->destroy($sessionId);
}
$this->newSessionId = null;
return $this->doWrite($sessionId, $data);
}
/**
* {@inheritdoc}
*/
public function destroy($sessionId)
{
if (\PHP_VERSION_ID < 70000) {
$this->prefetchData = null;
}
if (!headers_sent() && ini_get('session.use_cookies')) {
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;
}
} else {
$otherCookies[] = $h;
}
}
if ($sessionCookieFound) {
header_remove('Set-Cookie');
foreach ($otherCookies as $h) {
header('Set-Cookie:'.$h, false);
}
} 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'));
}
}
return $this->newSessionId === $sessionId || $this->doDestroy($sessionId);
}
}
@@ -19,7 +19,7 @@
*
* @author Drak <drak@zikula.org>
*/
class MemcachedSessionHandler implements \SessionHandlerInterface
class MemcachedSessionHandler extends AbstractSessionHandler
{
/**
* @var \Memcached Memcached driver
@@ -39,7 +39,7 @@ class MemcachedSessionHandler implements \SessionHandlerInterface
/**
* List of available options:
* * prefix: The prefix to use for the memcached keys in order to avoid collision
* * expiretime: The time to live in seconds
* * expiretime: The time to live in seconds.
*
* @param \Memcached $memcached A \Memcached instance
* @param array $options An associative array of Memcached options
@@ -63,39 +63,39 @@ public function __construct(\Memcached $memcached, array $options = array())
/**
* {@inheritdoc}
*/
public function open($savePath, $sessionName)
public function close()
{
return true;
}
/**
* {@inheritdoc}
*/
public function close()
protected function doRead($sessionId)
{
return true;
return $this->memcached->get($this->prefix.$sessionId) ?: '';
}
/**
* {@inheritdoc}
*/
public function read($sessionId)
public function updateTimestamp($sessionId, $data)
{
return $this->memcached->get($this->prefix.$sessionId) ?: '';
return $this->memcached->touch($this->prefix.$sessionId, time() + $this->ttl);
}
/**
* {@inheritdoc}
*/
public function write($sessionId, $data)
protected function doWrite($sessionId, $data)
{
return $this->memcached->set($this->prefix.$sessionId, $data, time() + $this->ttl);
}
/**
* {@inheritdoc}
*/
public function destroy($sessionId)
protected function doDestroy($sessionId)
{
$result = $this->memcached->delete($this->prefix.$sessionId);
Oops, something went wrong.

0 comments on commit 1f4025a

Please sign in to comment.