8000 [HttpFoundation] Make sessions secure and lazy by nicolas-grekas · Pull Request #24523 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[HttpFoundation] Make sessions secure and lazy #24523

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

Merged
merged 1 commit into from
Oct 16, 2017
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 5 additions & 8 deletions UPGRADE-3.4.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.

Expand Down
11 changes: 6 additions & 5 deletions UPGRADE-4.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

those or no alternative for WriteCheckSessionHandler. only session.lazy_write option is.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

session.lazy_write requires updateTimestamp(), thus SessionUpdateTimestampHandlerInterface


* `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-mon 1E0A godb.
Expand Down
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the info of this option is wrong now

it will also prevent the session handler to write if the session has not changed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also as lazy_write is only available since php 7, users with lower php versions will not have lazy write anymore

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. I just added an emulation of lazy writes in the abstract handler for PHP 5.

->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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
Expand Down
12 changes: 9 additions & 3 deletions src/Symfony/Bundle/FrameworkBundle/Resources/config/session.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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=" A851 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" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 3 additions & 2 deletions src/Symfony/Component/HttpFoundation/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10000
Original file line number Diff line number Diff line change
@@ -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}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i thought we don't add inheritdoc anymore which seems useless.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that was asked for, but rejected AFAIK

*/
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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
*
* @author Drak <drak@zikula.org>
*/
class MemcachedSessionHandler implements \SessionHandlerInterface
class MemcachedSessionHandler extends AbstractSessionHandler
{
/**
* @var \Memcached Memcached driver
Expand All @@ -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
Expand All @@ -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);

Expand Down
Loading
0