8000 feature #24523 [HttpFoundation] Make sessions secure and lazy (nicola… · symfony/symfony@1f4025a · GitHub
[go: up one dir, main page]
Skip to content

Commit 1f4025a

Browse files
fabpotfabpot
committed
feature #24523 [HttpFoundation] Make sessions secure and lazy (nicolas-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
2 parents f757cd6 + 347939c commit 1f4025a
Copy full SHA for 1f4025a

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+984
-104
lines changed

‎UPGRADE-3.4.md

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,8 @@ Form
126126
FrameworkBundle
127127
---------------
128128

129+
* The `session.use_strict_mode` option has been deprecated and is enabled by default.
130+
129131
* The `cache:clear` command doesn't clear "app" PSR-6 cache pools anymore,
130132
but still clears "system" ones.
131133
Use the `cache:pool:clear` command to clear "app" pools instead.
@@ -235,18 +237,13 @@ HttpFoundation
235237
* The `Symfony\Component\HttpFoundation\Session\Storage\Handler\NativeSessionHandler`
236238
class has been deprecated and will be removed in 4.0. Use the `\SessionHandler` class instead.
237239

238-
* The `Symfony\Component\HttpFoundation\Session\Storage\Proxy\AbstractProxy` class has been
239-
deprecated and will be removed in 4.0. Use your `\SessionHandlerInterface` implementation directly.
240+
* The `Symfony\Component\HttpFoundation\Session\Storage\Handler\WriteCheckSessionHandler` class has been
241+
deprecated and will be removed in 4.0. Implement `SessionUpdateTimestampHandlerInterface` or
242+
extend `AbstractSessionHandler` instead.
240243

241244
* The `Symfony\Component\HttpFoundation\Session\Storage\Proxy\NativeProxy` class has been
242245
deprecated and will be removed in 4.0. Use your `\SessionHandlerInterface` implementation directly.
243246

244-
* The `Symfony\Component\HttpFoundation\Session\Storage\Proxy\SessionHandlerProxy` class has been
245-
deprecated and will be removed in 4.0. Use your `\SessionHandlerInterface` implementation directly.
246-
247-
* `NativeSessionStorage::setSaveHandler()` now takes an instance of `\SessionHandlerInterface` as argument.
248-
Not passing it is deprecated and will throw a `TypeError` in 4.0.
249-
250247
* Using `Symfony\Component\HttpFoundation\Session\Storage\Handler\MongoDbSessionHandler` with the legacy mongo extension
251248
has been deprecated and will be removed in 4.0. Use it with the mongodb/mongodb package and ext-mongodb instead.
252249

‎UPGRADE-4.0.md

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,8 @@ Form
329329
FrameworkBundle
330330
---------------
331331

332+
* The `session.use_strict_mode` option has been removed and strict mode is always enabled.
333+
332334
* The `validator.mapping.cache.doctrine.apc` service has been removed.
333335

334336
* 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
542544
* The ability to check only for cacheable HTTP methods using `Request::isMethodSafe()` is
543545
not supported anymore, use `Request::isMethodCacheable()` instead.
544546

545-
* The `Symfony\Component\HttpFoundation\Session\Storage\Handler\NativeSessionHandler`,
546-
`Symfony\Component\HttpFoundation\Session\Storage\Proxy\AbstractProxy`,
547-
`Symfony\Component\HttpFoundation\Session\Storage\Proxy\NativeProxy` and
548-
`Symfony\Component\HttpFoundation\Session\Storage\Proxy\SessionHandlerProxy` classes have been removed.
547+
* The `Symfony\Component\HttpFoundation\Session\Storage\Handler\WriteCheckSessionHandler` class has been
548+
removed. Implement `SessionUpdateTimestampHandlerInterface` or extend `AbstractSessionHandler` instead.
549549

550-
* `NativeSessionStorage::setSaveHandler()` now requires an instance of `\SessionHandlerInterface` as argument.
550+
* The `Symfony\Component\HttpFoundation\Session\Storage\Handler\NativeSessionHandler` and
551+
`Symfony\Component\HttpFoundation\Session\Storage\Proxy\NativeProxy` classes have been removed.
551552

552553
* The `Symfony\Component\HttpFoundation\Session\Storage\Handler\MongoDbSessionHandler` does not work with the legacy
553554
mongo extension anymore. It requires mongodb/mongodb package and ext-mongodb.

‎composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
"symfony/polyfill-intl-icu": "~1.0",
3131
"symfony/polyfill-mbstring": "~1.0",
3232
"symfony/polyfill-php56": "~1.0",
33-
"symfony/polyfill-php70": "~1.0",
33+
"symfony/polyfill-php70": "~1.6",
3434
"symfony/polyfill-util": "~1.0"
3535
},
3636
"replace": {

‎src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ CHANGELOG
44
3.4.0
55
-----
66

7+
* Session `use_strict_mode` is now enabled by default and the corresponding option has been deprecated
78
* Made the `cache:clear` command to *not* clear "app" PSR-6 cache pools anymore,
89
but to still clear "system" ones; use the `cache:pool:clear` command to clear "app" pools instead
910
* Always register a minimalist logger that writes in `stderr`

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -462,11 +462,14 @@ private function addSessionSection(ArrayNodeDefinition $rootNode)
462462
->scalarNode('gc_divisor')->end()
463463
->scalarNode('gc_probability')->defaultValue(1)->end()
464464
->scalarNode('gc_maxlifetime')->end()
465-
->booleanNode('use_strict_mode')->end()
465+
->booleanNode('use_strict_mode')
466+
->defaultTrue()
467+
->setDeprecated('The "%path%.%node%" option is enabled by default and deprecated since Symfony 3.4. It will be always enabled in 4.0.')
468+
->end()
466469
->scalarNode('save_path')->defaultValue('%kernel.cache_dir%/sessions')->end()
467470
->integerNode('metadata_update_threshold')
468471
->defaultValue('0')
469-
->info('seconds to wait between 2 session metadata updates, it will also prevent the session handler to write if the session has not changed')
472+
->info('seconds to wait between 2 session metadata updates')
470473
->end()
471474
->end()
472475
->end()

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

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -916,14 +916,7 @@ private function registerSessionConfiguration(array $config, ContainerBuilder $c
916916
$container->getDefinition('session.storage.native')->replaceArgument(1, null);
917917
$container->getDefinition('session.storage.php_bridge')->replaceArgument(0, null);
918918
} else {
919-
$handlerId = $config['handler_id'];
920-
921-
if ($config['metadata_update_threshold'] > 0) {
922-
$container->getDefinition('session.handler.write_check')->addArgument(new Reference($handlerId));
923-
$handlerId = 'session.handler.write_check';
924-
}
925-
926-
$container->setAlias('session.handler', $handlerId)->setPrivate(true);
919+
$container->setAlias('session.handler', $config['handler_id'])->setPrivate(true);
927920
}
928921

929922
$container->setParameter('session.save_path', $config['save_path']);

‎src/Symfony/Bundle/FrameworkBundle/Resources/config/session.xml

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,17 @@
4848
<argument type="service" id="session.storage.metadata_bag" />
4949
</service>
5050

51-
<service id="session.handler.native_file" class="Symfony\Component\HttpFoundation\Session\Storage\Handler\NativeFileSessionHandler">
52-
<argument>%session.save_path%</argument>
51+
<service id="session.handler.native_file" class="Symfony\Component\HttpFoundation\Session\Storage\Handler\StrictSessionHandler">
52+
<argument type="service">
53+
<service class="Symfony\Component\HttpFoundation\Session\Storage\Handler\NativeFileSessionHandler">
54+
<argument>%session.save_path%</argument>
55+
</service>
56+
</argument>
5357
</service>
5458

55-
<service id="session.handler.write_check" class="Symfony\Component\HttpFoundation\Session\Storage\Handler\WriteCheckSessionHandler" />
59+
<service id="session.handler.write_check" class="Symfony\Component\HttpFoundation\Session\Storage\Handler\WriteCheckSessionHandler">
60+
<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>
61+
</service>
5662

5763
<service id="session_listener" class="Symfony\Component\HttpKernel\EventListener\SessionListener">
5864
<tag name="kernel.event_subscriber" />

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,7 @@ protected static function getBundleDefaultConfig()
301301
'gc_probability' => 1,
302302
'save_path' => '%kernel.cache_dir%/sessions',
303303
'metadata_update_threshold' => '0',
304+
'use_strict_mode' => true,
304305
),
305306
'request' => array(
306307
'enabled' => false,

‎src/Symfony/Component/HttpFoundation/CHANGELOG.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,9 @@ CHANGELOG
44
3.4.0
55
-----
66

7-
* deprecated the `NativeSessionHandler` class,
8-
* deprecated the `AbstractProxy`, `NativeProxy` and `SessionHandlerProxy` classes,
7+
* implemented PHP 7.0's `SessionUpdateTimestampHandlerInterface` with a new
8+
`AbstractSessionHandler` base class and a new `StrictSessionHandler` wrapper
9+
* deprecated the `WriteCheckSessionHandler`, `NativeSessionHandler` and `NativeProxy` classes
910
* deprecated setting session save handlers that do not implement `\SessionHandlerInterface` in `NativeSessionStorage::setSaveHandler()`
1011
* deprecated using `MongoDbSessionHandler` with the legacy mongo extension; use it with the mongodb/mongodb package and ext-mongodb instead
1112
* deprecated `MemcacheSessionHandler`; use `MemcachedSessionHandler` instead

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

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\HttpFoundation\Session\Storage\Handler;
13+
14+
/**
15+
* This abstract session handler provides a generic implementation
16+
* of the PHP 7.0 SessionUpdateTimestampHandlerInterface,
17+
* enabling strict and lazy session handling.
18+
*
19+
* @author Nicolas Grekas <p@tchwork.com>
20+
*/
21+
abstract class AbstractSessionHandler implements \SessionHandlerInterface, \SessionUpdateTimestampHandlerInterface
22+
{
23+
private $sessionName;
24+
private $prefetchId;
25+
private $prefetchData;
26+
private $newSessionId;
27+
private $igbinaryEmptyData;
28+
29+
/**
30+
* {@inheritdoc}
31+
*/
32+
public function open($savePath, $sessionName)
33+
{
34+
$this->sessionName = $sessionName;
35+
36+
return true;
37+
}
38+
39+
/**
40+
* @param string $sessionId
41+
*
42+
* @return string
43+
*/
44+
abstract protected function doRead($sessionId);
45+
46+
/**
47+
* @param string $sessionId
48+
* @param string $data
49+
*
50+
* @return bool
51+
*/
52+
abstract protected function doWrite($sessionId, $data);
53+
54+
/**
55+
* @param string $sessionId
56+
*
57+
* @return bool
58+
*/
59+
abstract protected function doDestroy($sessionId);
60+
61+
/**
62+
* {@inheritdoc}
63+
*/
64+
public function validateId($sessionId)
65+
{
66+
$this->prefetchData = $this->read($sessionId);
67+
$this->prefetchId = $sessionId;
68+
69+
return '' !== $this->prefetchData;
70+
}
71+
72+
/**
73+
* {@inheritdoc}
74+
*/
75+
public function read($sessionId)
76+
{
77+
if (null !== $this->prefetchId) {
78+
$prefetchId = $this->prefetchId;
79+
$prefetchData = $this->prefetchData;
80+
$this->prefetchId = $this->prefetchData = null;
81+
82+
if ($prefetchId === $sessionId || '' === $prefetchData) {
83+
$this->newSessionId = '' === $prefetchData ? $sessionId : null;
84+
85+
return $prefetchData;
86+
}
87+
}
88+
89+
$data = $this->doRead($sessionId);
90+
$this->newSessionId = '' === $data ? $sessionId : null;
91+
if (\PHP_VERSION_ID < 70000) {
92+
$this->prefetchData = $data;
93+
}
94+
95+
return $data;
96+
}
97+
98+
/**
99+
* {@inheritdoc}
100+
*/
101+
public function write($sessionId, $data)
102+
{
103+
if (\PHP_VERSION_ID < 70000 && $this->prefetchData) {
104+
$readData = $this->prefetchData;
105+
$this->prefetchData = null;
106+
107+
if ($readData === $data) {
108+
return $this->updateTimestamp($sessionId, $data);
109+
}
110+
}
111+
if (null === $this->igbinaryEmptyData) {
112+
// see https://github.com/igbinary/igbinary/issues/146
113+
$this->igbinaryEmptyData = \function_exists('igbinary_serialize') ? igbinary_serialize(array()) : '';
114+
}
115+
if ('' === $data || $this->igbinaryEmptyData === $data) {
116+
return $this->destroy($sessionId);
117+
}
118+
$this->newSessionId = null;
119+
120+
return $this->doWrite($sessionId, $data);
121+
}
122+
123+
/**
124+
* {@inheritdoc}
125+
*/
126+
public function destroy($sessionId)
127+
{
128+
if (\PHP_VERSION_ID < 70000) {
129+
$this->prefetchData = null;
130+
}
131+
if (!headers_sent() && ini_get('session.use_cookies')) {
132+
if (!$this->sessionName) {
133+
throw new \LogicException(sprintf('Session name cannot be empty, did you forget to call "parent::open()" in "%s"?.', get_class($this)));
134+
}
135+
$sessionCookie = sprintf(' %s=', urlencode($this->sessionName));
136+
$sessionCookieWithId = sprintf('%s%s;', $sessionCookie, urlencode($sessionId));
137+
$sessionCookieFound = false;
138+
$otherCookies = array();
139+
foreach (headers_list() as $h) {
140+
if (0 !== stripos($h, 'Set-Cookie:')) {
141+
continue;
142+
}
143+
if (11 === strpos($h, $sessionCookie, 11)) {
144+
$sessionCookieFound = true;
145+
146+
if (11 !== strpos($h, $sessionCookieWithId, 11)) {
147+
$otherCookies[] = $h;
148+
}
149+
} else {
150+
$otherCookies[] = $h;
151+
}
152+
}
153+
if ($sessionCookieFound) {
154+
header_remove('Set-Cookie');
155+
foreach ($otherCookies as $h) {
156+
header('Set-Cookie:'.$h, false);
157+
}
158+
} else {
159+
setcookie($this->sessionName, '', 0, ini_get('session.cookie_path'), ini_get('session.cookie_domain'), ini_get('session.cookie_secure'), ini_get('session.cookie_httponly'));
160+
}
161+
}
162+
163+
return $this->newSessionId === $sessionId || $this->doDestroy($sessionId);
164+
}
165+
}

‎src/Symfony/Component/HttpFoundation/Session/Storage/Handler/MemcachedSessionHandler.php

Lines changed: 9 additions & 9 deletions
F438
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
*
2020
* @author Drak <drak@zikula.org>
2121
*/
22-
class MemcachedSessionHandler implements \SessionHandlerInterface
22+
class MemcachedSessionHandler extends AbstractSessionHandler
2323
{
2424
/**
2525
* @var \Memcached Memcached driver
@@ -39,7 +39,7 @@ class MemcachedSessionHandler implements \SessionHandlerInterface
3939
/**
4040
* List of available options:
4141
* * prefix: The prefix to use for the memcached keys in order to avoid collision
42-
* * expiretime: The time to live in seconds
42+
* * expiretime: The time to live in seconds.
4343
*
4444
* @param \Memcached $memcached A \Memcached instance
4545
* @param array $options An associative array of Memcached options
@@ -63,39 +63,39 @@ public function __construct(\Memcached $memcached, array $options = array())
6363
/**
6464
* {@inheritdoc}
6565
*/
66-
public function open($savePath, $sessionName)
66+
public function close()
6767
{
6868
return true;
6969
}
7070

7171
/**
7272
* {@inheritdoc}
7373
*/
74-
public function close()
74+
protected function doRead($sessionId)
7575
{
76-
return true;
76+
return $this->memcached->get($this->prefix.$sessionId) ?: '';
7777
}
7878

7979
/**
8080
* {@inheritdoc}
8181
*/
82-
public function read($sessionId)
82+
public function updateTimestamp($sessionId, $data)
8383
{
84-
return $this->memcached->get($this->prefix.$sessionId) ?: '';
84+
return $this->memcached->touch($this->prefix.$sessionId, time() + $this->ttl);
8585
}
8686

8787
/**
8888
* {@inheritdoc}
8989
*/
90-
public function write($sessionId, $data)
90+
protected function doWrite($sessionId, $data)
9191
{
9292
return $this->memcached->set($this->prefix.$sessionId, $data, time() + $this->ttl);
9393
}
9494

9595
/**
9696
* {@inheritdoc}
9797
*/
98-
public function destroy($sessionId)
98+
protected function doDestroy($sessionId)
9999
{
100100
$result = $this->memcached->delete($this->prefix.$sessionId);
101101

0 commit comments

Comments
 (0)
0