diff --git a/CHANGELOG-2.7.md b/CHANGELOG-2.7.md index 6903fb4ed455d..c343a8740748b 100644 --- a/CHANGELOG-2.7.md +++ b/CHANGELOG-2.7.md @@ -7,6 +7,14 @@ in 2.7 minor versions. To get the diff for a specific change, go to https://github.com/symfony/symfony/commit/XXX where XXX is the change hash To get the diff between two versions, go to https://github.com/symfony/symfony/compare/v2.7.0...v2.7.1 +* 2.7.48 (2018-05-25) + + * bug #27359 [HttpFoundation] Fix perf issue during MimeTypeGuesser intialization (nicolas-grekas) + * security #cve-2018-11408 [SecurityBundle] Fail if security.http_utils cannot be configured + * security #cve-2018-11406 clear CSRF tokens when the user is logged out + * security #cve-2018-11385 Adding session strategy to ALL listeners to avoid *any* possible fixation + * security #cve-2018-11386 [HttpFoundation] Break infinite loop in PdoSessionHandler when MySQL is in loose mode + * 2.7.47 (2018-05-21) * bug #26781 [Form] Fix precision of MoneyToLocalizedStringTransformer's divisions on transform() (syastrebov) diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index 672246e7f9da8..73af1c6a1cb79 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -127,6 +127,7 @@ Symfony is the result of the work of many people who made the code better - Tugdual Saunier (tucksaun) - Javier Spagnoletti (phansys) - Théo FIDRY (theofidry) + - gadelat (gadelat) - Robert Schönthal (digitalkaoz) - Florian Lonqueu-Brochard (florianlb) - Sebastiaan Stok (sstok) @@ -141,7 +142,6 @@ Symfony is the result of the work of many people who made the code better - Hidenori Goto (hidenorigoto) - Jérôme Vasseur (jvasseur) - Valentin Udaltsov (vudaltsov) - - gadelat (gadelat) - Guilherme Blanco (guilhermeblanco) - Pablo Godel (pgodel) - Jérémie Augustin (jaugustin) @@ -511,6 +511,7 @@ Symfony is the result of the work of many people who made the code better - Andrew Udvare (audvare) - alexpods - Arjen van der Meijden + - Adam Szaraniec (mimol) - Dariusz Ruminski - Erik Trapman (eriktrapman) - De Cock Xavier (xdecock) @@ -588,6 +589,7 @@ Symfony is the result of the work of many people who made the code better - Nahuel Cuesta (ncuesta) - Chris Boden (cboden) - Christophe Villeger (seragan) + - Bob van de Vijver (bobvandevijver) - Stefan Gehrig (sgehrig) - Hany el-Kerdany - Wang Jingyu @@ -691,6 +693,7 @@ Symfony is the result of the work of many people who made the code better - Alex Xandra Albert Sim - Craig Duncan (duncan3dc) - Carson Full + - Sergey Yastrebov - Trent Steel (trsteel88) - Yuen-Chi Lian - Besnik Br @@ -717,11 +720,11 @@ Symfony is the result of the work of many people who made the code better - Joschi Kuphal - John Bohn (jbohn) - Marc Morera (mmoreram) + - Smaine Milianni (ismail1432) - Andrew Hilobok (hilobok) - Noah Heck (myesain) - Christian Soronellas (theunic) - Johann Pardanaud - - Adam Szaraniec (mimol) - Yosmany Garcia (yosmanyga) - Wouter de Wild - Antoine M (amakdessi) @@ -934,7 +937,6 @@ Symfony is the result of the work of many people who made the code better - Máximo Cuadros (mcuadros) - tamirvs - julien.galenski - - Bob van de Vijver - Christian Neff - Oliver Hoff - Ole Rößner (basster) @@ -1017,6 +1019,7 @@ Symfony is the result of the work of many people who made the code better - Alex Bowers - Jeremy Bush - wizhippo + - Thomason, James - Viacheslav Sychov - Helmut Hummel (helhum) - Matt Brunt @@ -1158,7 +1161,6 @@ Symfony is the result of the work of many people who made the code better - Berat Doğan - Guillaume LECERF - Juanmi Rodriguez Cerón - - Sergey Yastrebov - Andy Raines - Anthony Ferrara - Klaas Cuvelier (kcuvelier) @@ -1209,6 +1211,7 @@ Symfony is the result of the work of many people who made the code better - Romain Geissler - Adrien Moiruad - Tomaz Ahlin + - Philip Ardery - Marcus Stöhr (dafish) - Emmanuel Vella (emmanuel.vella) - Jonathan Johnson (jrjohnson) @@ -1254,7 +1257,6 @@ Symfony is the result of the work of many people who made the code better - Ergie Gonzaga - Matthew J Mucklo - AnrDaemon - - Smaine Milianni (ismail1432) - fdgdfg (psampaz) - Stéphane Seng - Maxwell Vandervelde @@ -1765,6 +1767,7 @@ Symfony is the result of the work of many people who made the code better - Norman Soetbeer - zorn - Yuriy Potemkin + - Emilie Lorenzo - Benjamin Long - Matt Janssen - Ben Miller @@ -1929,6 +1932,7 @@ Symfony is the result of the work of many people who made the code better - fh-github@fholzhauer.de - AbdElKader Bouadjadja - DSeemiller + - Kyle - Jan Emrich - Mark Topper - Xavier REN diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddSessionDomainConstraintPass.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddSessionDomainConstraintPass.php index 3dd18944de9f3..ba523382b66ba 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddSessionDomainConstraintPass.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddSessionDomainConstraintPass.php @@ -26,7 +26,7 @@ class AddSessionDomainConstraintPass implements CompilerPassInterface */ public function process(ContainerBuilder $container) { - if (!$container->hasParameter('session.storage.options') || !$container->has('security.http_utils')) { + if (!$container->hasParameter('session.storage.options')) { return; } @@ -34,6 +34,7 @@ public function process(ContainerBuilder $container) $domainRegexp = empty($sessionOptions['cookie_domain']) ? '%s' : sprintf('(?:%%s|(?:.+\.)?%s)', preg_quote(trim($sessionOptions['cookie_domain'], '.'))); $domainRegexp = (empty($sessionOptions['cookie_secure']) ? 'https?://' : 'https://').$domainRegexp; + // if the service doesn't exist, an exception must be thrown - ignoring would put security at risk $container->findDefinition('security.http_utils')->addArgument(sprintf('{^%s$}i', $domainRegexp)); } } diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/RegisterCsrfTokenClearingLogoutHandlerPass.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/RegisterCsrfTokenClearingLogoutHandlerPass.php new file mode 100644 index 0000000000000..d4d28ecc4eb35 --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/RegisterCsrfTokenClearingLogoutHandlerPass.php @@ -0,0 +1,42 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler; + +use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; +use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Reference; + +/** + * @author Christian Flothmann + */ +class RegisterCsrfTokenClearingLogoutHandlerPass implements CompilerPassInterface +{ + public function process(ContainerBuilder $container) + { + if (!$container->has('security.logout_listener') || !$container->has('security.csrf.token_storage')) { + return; + } + + $csrfTokenStorage = $container->findDefinition('security.csrf.token_storage'); + $csrfTokenStorageClass = $container->getParameterBag()->resolveValue($csrfTokenStorage->getClass()); + + if (!is_subclass_of($csrfTokenStorageClass, 'Symfony\Component\Security\Csrf\TokenStorage\ClearableTokenStorageInterface')) { + return; + } + + $container->register('security.logout.handler.csrf_token_clearing', 'Symfony\Component\Security\Http\Logout\CsrfTokenClearingLogoutHandler') + ->addArgument(new Reference('security.csrf.token_storage')) + ->setPublic(false); + + $container->findDefinition('security.logout_listener')->addMethodCall('addHandler', array(new Reference('security.logout.handler.csrf_token_clearing'))); + } +} diff --git a/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php b/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php index 9bcd504a9c25e..137558569bc2d 100644 --- a/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php +++ b/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php @@ -11,6 +11,7 @@ namespace Symfony\Bundle\SecurityBundle; +use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\RegisterCsrfTokenClearingLogoutHandlerPass; use Symfony\Component\HttpKernel\Bundle\Bundle; use Symfony\Component\DependencyInjection\Compiler\PassConfig; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -49,6 +50,7 @@ public function build(ContainerBuilder $container) $extension->addUserProviderFactory(new InMemoryFactory()); $container->addCompilerPass(new AddSecurityVotersPass()); - $container->addCompilerPass(new AddSessionDomainConstraintPass(), PassConfig::TYPE_AFTER_REMOVING); + $container->addCompilerPass(new AddSessionDomainConstraintPass(), PassConfig::TYPE_BEFORE_REMOVING); + $container->addCompilerPass(new RegisterCsrfTokenClearingLogoutHandlerPass()); } } diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Compiler/AddSessionDomainConstraintPassTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Compiler/AddSessionDomainConstraintPassTest.php index 7d49ad3dd4ec6..e12f3a64f31fe 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Compiler/AddSessionDomainConstraintPassTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Compiler/AddSessionDomainConstraintPassTest.php @@ -96,6 +96,19 @@ public function testNoSession() $this->assertTrue($utils->createRedirectResponse($request, 'http://pirate.com/foo')->isRedirect('http://pirate.com/foo')); } + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException + * @expectedExceptionMessage You have requested a non-existent service "security.http_utils". + */ + public function testNoHttpUtils() + { + $container = new ContainerBuilder(); + $container->setParameter('session.storage.options', array()); + + $pass = new AddSessionDomainConstraintPass(); + $pass->process($container); + } + private function createContainer($sessionStorageOptions) { $container = new ContainerBuilder(); diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/LogoutTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/LogoutTest.php index 7eeb7c21171ce..d3c3b77fd5d61 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/LogoutTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/LogoutTest.php @@ -31,4 +31,22 @@ public function testSessionLessRememberMeLogout() $this->assertNull($cookieJar->get('REMEMBERME')); } + + public function testCsrfTokensAreClearedOnLogout() + { + $client = $this->createClient(array('test_case' => 'LogoutWithoutSessionInvalidation', 'root_config' => 'config.yml')); + $client->getContainer()->get('security.csrf.token_storage')->setToken('foo', 'bar'); + + $client->request('POST', '/login', array( + '_username' => 'johannes', + '_password' => 'test', + )); + + $this->assertTrue($client->getContainer()->get('security.csrf.token_storage')->hasToken('foo')); + $this->assertSame('bar', $client->getContainer()->get('security.csrf.token_storage')->getToken('foo')); + + $client->request('GET', '/logout'); + + $this->assertFalse($client->getContainer()->get('security.csrf.token_storage')->hasToken('foo')); + } } diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/LogoutWithoutSessionInvalidation/bundles.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/LogoutWithoutSessionInvalidation/bundles.php new file mode 100644 index 0000000000000..d90f774abde2b --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/LogoutWithoutSessionInvalidation/bundles.php @@ -0,0 +1,18 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +use Symfony\Bundle\SecurityBundle\SecurityBundle; +use Symfony\Bundle\FrameworkBundle\FrameworkBundle; + +return array( + new FrameworkBundle(), + new SecurityBundle(), +); diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/LogoutWithoutSessionInvalidation/config.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/LogoutWithoutSessionInvalidation/config.yml new file mode 100644 index 0000000000000..d3fd8d0339e86 --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/LogoutWithoutSessionInvalidation/config.yml @@ -0,0 +1,26 @@ +imports: + - { resource: ./../config/framework.yml } + +security: + encoders: + Symfony\Component\Security\Core\User\User: plaintext + + providers: + in_memory: + memory: + users: + johannes: { password: test, roles: [ROLE_USER] } + + firewalls: + default: + form_login: + check_path: login + remember_me: true + require_previous_session: false + remember_me: + always_remember_me: true + key: key + logout: + invalidate_session: false + anonymous: ~ + stateless: true diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/LogoutWithoutSessionInvalidation/routing.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/LogoutWithoutSessionInvalidation/routing.yml new file mode 100644 index 0000000000000..1dddfca2f8154 --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/LogoutWithoutSessionInvalidation/routing.yml @@ -0,0 +1,5 @@ +login: + path: /login + +logout: + path: /logout diff --git a/src/Symfony/Bundle/SecurityBundle/composer.json b/src/Symfony/Bundle/SecurityBundle/composer.json index 90edcf6e38cee..68b2c907cbf18 100644 --- a/src/Symfony/Bundle/SecurityBundle/composer.json +++ b/src/Symfony/Bundle/SecurityBundle/composer.json @@ -18,7 +18,7 @@ "require": { "php": ">=5.3.9", "ext-xml": "*", - "symfony/security": "~2.7.47|~2.8.40", + "symfony/security": "~2.7.48|~2.8.41", "symfony/security-acl": "~2.7", "symfony/http-kernel": "~2.7" }, @@ -26,7 +26,7 @@ "symfony/browser-kit": "~2.7", "symfony/console": "~2.7", "symfony/css-selector": "^2.7", - "symfony/dependency-injection": "^2.7.1", + "symfony/dependency-injection": "~2.7.48|^2.8.41", "symfony/dom-crawler": "^2.7", "symfony/form": "~2.7.15|^2.8.8", "symfony/framework-bundle": "~2.7.25|^2.8.18", diff --git a/src/Symfony/Component/HttpFoundation/File/MimeType/MimeTypeGuesser.php b/src/Symfony/Component/HttpFoundation/File/MimeType/MimeTypeGuesser.php index e3ef45ef672cf..d78c76068234f 100644 --- a/src/Symfony/Component/HttpFoundation/File/MimeType/MimeTypeGuesser.php +++ b/src/Symfony/Component/HttpFoundation/File/MimeType/MimeTypeGuesser.php @@ -80,13 +80,8 @@ public static function reset() */ private function __construct() { - if (FileBinaryMimeTypeGuesser::isSupported()) { - $this->register(new FileBinaryMimeTypeGuesser()); - } - - if (FileinfoMimeTypeGuesser::isSupported()) { - $this->register(new FileinfoMimeTypeGuesser()); - } + $this->register(new FileBinaryMimeTypeGuesser()); + $this->register(new FileinfoMimeTypeGuesser()); } /** @@ -125,18 +120,14 @@ public function guess($path) throw new AccessDeniedException($path); } - if (!$this->guessers) { - $msg = 'Unable to guess the mime type as no guessers are available'; - if (!FileinfoMimeTypeGuesser::isSupported()) { - $msg .= ' (Did you enable the php_fileinfo extension?)'; - } - throw new \LogicException($msg); - } - foreach ($this->guessers as $guesser) { if (null !== $mimeType = $guesser->guess($path)) { return $mimeType; } } + + if (2 === \count($this->guessers) && !FileBinaryMimeTypeGuesser::isSupported() && !FileinfoMimeTypeGuesser::isSupported()) { + throw new \LogicException('Unable to guess the mime type as no guessers are available (Did you enable the php_fileinfo extension?)'); + } } } diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php b/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php index 0825ee6ea9899..bb000f5c9a7a0 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php +++ b/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php @@ -494,6 +494,7 @@ private function doRead($sessionId) $selectSql = $this->getSelectSql(); $selectStmt = $this->pdo->prepare($selectSql); $selectStmt->bindParam(':id', $sessionId, \PDO::PARAM_STR); + $insertStmt = null; do { $selectStmt->execute(); @@ -509,6 +510,11 @@ private function doRead($sessionId) return is_resource($sessionRows[0][0]) ? stream_get_contents($sessionRows[0][0]) : $sessionRows[0][0]; } + if (null !== $insertStmt) { + $this->rollback(); + throw new \RuntimeException('Failed to read session: INSERT reported a duplicate id but next SELECT did not return any data.'); + } + if (self::LOCK_TRANSACTIONAL === $this->lockMode && 'sqlite' !== $this->driver) { // Exclusive-reading of non-existent rows does not block, so we need to do an insert to block // until other connections to the session are committed. diff --git a/src/Symfony/Component/HttpKernel/Kernel.php b/src/Symfony/Component/HttpKernel/Kernel.php index e0d221e2edce8..9ec779c8ce824 100644 --- a/src/Symfony/Component/HttpKernel/Kernel.php +++ b/src/Symfony/Component/HttpKernel/Kernel.php @@ -58,11 +58,11 @@ abstract class Kernel implements KernelInterface, TerminableInterface protected $startTime; protected $loadClassCache; - const VERSION = '2.7.47'; - const VERSION_ID = 20747; + const VERSION = '2.7.48'; + const VERSION_ID = 20748; const MAJOR_VERSION = 2; const MINOR_VERSION = 7; - const RELEASE_VERSION = 47; + const RELEASE_VERSION = 48; const EXTRA_VERSION = ''; const END_OF_MAINTENANCE = '05/2018'; diff --git a/src/Symfony/Component/Security/Csrf/Tests/TokenStorage/NativeSessionTokenStorageTest.php b/src/Symfony/Component/Security/Csrf/Tests/TokenStorage/NativeSessionTokenStorageTest.php index d7931c09b2099..89086e5c565c3 100644 --- a/src/Symfony/Component/Security/Csrf/Tests/TokenStorage/NativeSessionTokenStorageTest.php +++ b/src/Symfony/Component/Security/Csrf/Tests/TokenStorage/NativeSessionTokenStorageTest.php @@ -116,4 +116,32 @@ public function testRemoveExistingToken() $this->assertSame('TOKEN', $this->storage->removeToken('token_id')); $this->assertFalse($this->storage->hasToken('token_id')); } + + public function testClearRemovesAllTokensFromTheConfiguredNamespace() + { + $this->storage->setToken('foo', 'bar'); + $this->storage->clear(); + + $this->assertFalse($this->storage->hasToken('foo')); + $this->assertArrayNotHasKey(self::SESSION_NAMESPACE, $_SESSION); + } + + public function testClearDoesNotRemoveSessionValuesFromOtherNamespaces() + { + $_SESSION['foo']['bar'] = 'baz'; + $this->storage->clear(); + + $this->assertArrayHasKey('foo', $_SESSION); + $this->assertArrayHasKey('bar', $_SESSION['foo']); + $this->assertSame('baz', $_SESSION['foo']['bar']); + } + + public function testClearDoesNotRemoveNonNamespacedSessionValues() + { + $_SESSION['foo'] = 'baz'; + $this->storage->clear(); + + $this->assertArrayHasKey('foo', $_SESSION); + $this->assertSame('baz', $_SESSION['foo']); + } } diff --git a/src/Symfony/Component/Security/Csrf/Tests/TokenStorage/SessionTokenStorageTest.php b/src/Symfony/Component/Security/Csrf/Tests/TokenStorage/SessionTokenStorageTest.php index 306e19ad91bb9..7539852f13f3f 100644 --- a/src/Symfony/Component/Security/Csrf/Tests/TokenStorage/SessionTokenStorageTest.php +++ b/src/Symfony/Component/Security/Csrf/Tests/TokenStorage/SessionTokenStorageTest.php @@ -129,4 +129,31 @@ public function testRemoveExistingTokenFromActiveSession() $this->assertSame('TOKEN', $this->storage->removeToken('token_id')); } + + public function testClearRemovesAllTokensFromTheConfiguredNamespace() + { + $this->storage->setToken('foo', 'bar'); + $this->storage->clear(); + + $this->assertFalse($this->storage->hasToken('foo')); + $this->assertFalse($this->session->has(self::SESSION_NAMESPACE.'/foo')); + } + + public function testClearDoesNotRemoveSessionValuesFromOtherNamespaces() + { + $this->session->set('foo/bar', 'baz'); + $this->storage->clear(); + + $this->assertTrue($this->session->has('foo/bar')); + $this->assertSame('baz', $this->session->get('foo/bar')); + } + + public function testClearDoesNotRemoveNonNamespacedSessionValues() + { + $this->session->set('foo', 'baz'); + $this->storage->clear(); + + $this->assertTrue($this->session->has('foo')); + $this->assertSame('baz', $this->session->get('foo')); + } } diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/ClearableTokenStorageInterface.php b/src/Symfony/Component/Security/Csrf/TokenStorage/ClearableTokenStorageInterface.php new file mode 100644 index 0000000000000..0d6f16b68d0b6 --- /dev/null +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/ClearableTokenStorageInterface.php @@ -0,0 +1,23 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Csrf\TokenStorage; + +/** + * @author Christian Flothmann + */ +interface ClearableTokenStorageInterface extends TokenStorageInterface +{ + /** + * Removes all CSRF tokens. + */ + public function clear(); +} diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/NativeSessionTokenStorage.php b/src/Symfony/Component/Security/Csrf/TokenStorage/NativeSessionTokenStorage.php index e817fdb902af8..e57e98d542433 100644 --- a/src/Symfony/Component/Security/Csrf/TokenStorage/NativeSessionTokenStorage.php +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/NativeSessionTokenStorage.php @@ -18,7 +18,7 @@ * * @author Bernhard Schussek */ -class NativeSessionTokenStorage implements TokenStorageInterface +class NativeSessionTokenStorage implements ClearableTokenStorageInterface { /** * The namespace used to store values in the session. @@ -96,6 +96,14 @@ public function removeToken($tokenId) return $token; } + /** + * {@inheritdoc} + */ + public function clear() + { + unset($_SESSION[$this->namespace]); + } + private function startSession() { if (\PHP_VERSION_ID >= 50400) { diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/SessionTokenStorage.php b/src/Symfony/Component/Security/Csrf/TokenStorage/SessionTokenStorage.php index 7b00e3231b45a..d22b83e8d51de 100644 --- a/src/Symfony/Component/Security/Csrf/TokenStorage/SessionTokenStorage.php +++ b/src/Symfony/Component/Security/Csrf/TokenStorage/SessionTokenStorage.php @@ -19,7 +19,7 @@ * * @author Bernhard Schussek */ -class SessionTokenStorage implements TokenStorageInterface +class SessionTokenStorage implements ClearableTokenStorageInterface { /** * The namespace used to store values in the session. @@ -92,4 +92,16 @@ public function removeToken($tokenId) return $this->session->remove($this->namespace.'/'.$tokenId); } + + /** + * {@inheritdoc} + */ + public function clear() + { + foreach (array_keys($this->session->all()) as $key) { + if (0 === strpos($key, $this->namespace.'/')) { + $this->session->remove($key); + } + } + } } diff --git a/src/Symfony/Component/Security/Http/Firewall/AbstractPreAuthenticatedListener.php b/src/Symfony/Component/Security/Http/Firewall/AbstractPreAuthenticatedListener.php index 0065fe8237c3e..2054c4aa0774e 100644 --- a/src/Symfony/Component/Security/Http/Firewall/AbstractPreAuthenticatedListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/AbstractPreAuthenticatedListener.php @@ -82,6 +82,9 @@ final public function handle(GetResponseEvent $event) if (null !== $this->logger) { $this->logger->info('Pre-authentication successful.', array('token' => (string) $token)); } + + $this->migrateSession($request); + $this->tokenStorage->setToken($token); if (null !== $this->dispatcher) { @@ -114,4 +117,16 @@ private function clearToken(AuthenticationException $exception) * @return array An array composed of the user and the credentials */ abstract protected function getPreAuthenticatedData(Request $request); + + private function migrateSession(Request $request) + { + if (!$request->hasSession() || !$request->hasPreviousSession()) { + return; + } + + // Destroying the old session is broken in php 5.4.0 - 5.4.10 + // See https://bugs.php.net/63379 + $destroy = \PHP_VERSION_ID < 50400 || \PHP_VERSION_ID >= 50411; + $request->getSession()->migrate($destroy); + } } diff --git a/src/Symfony/Component/Security/Http/Firewall/BasicAuthenticationListener.php b/src/Symfony/Component/Security/Http/Firewall/BasicAuthenticationListener.php index 1ddc41643448e..63bd013c64e31 100644 --- a/src/Symfony/Component/Security/Http/Firewall/BasicAuthenticationListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/BasicAuthenticationListener.php @@ -11,6 +11,7 @@ namespace Symfony\Component\Security\Http\Firewall; +use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; use Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface; @@ -70,6 +71,9 @@ public function handle(GetResponseEvent $event) try { $token = $this->authenticationManager->authenticate(new UsernamePasswordToken($username, $request->headers->get('PHP_AUTH_PW'), $this->providerKey)); + + $this->migrateSession($request); + $this->tokenStorage->setToken($token); } catch (AuthenticationException $e) { $token = $this->tokenStorage->getToken(); @@ -88,4 +92,16 @@ public function handle(GetResponseEvent $event) $event->setResponse($this->authenticationEntryPoint->start($request, $e)); } } + + private function migrateSession(Request $request) + { + if (!$request->hasSession() || !$request->hasPreviousSession()) { + return; + } + + // Destroying the old session is broken in php 5.4.0 - 5.4.10 + // See https://bugs.php.net/63379 + $destroy = \PHP_VERSION_ID < 50400 || \PHP_VERSION_ID >= 50411; + $request->getSession()->migrate($destroy); + } } diff --git a/src/Symfony/Component/Security/Http/Firewall/DigestAuthenticationListener.php b/src/Symfony/Component/Security/Http/Firewall/DigestAuthenticationListener.php index 070d61a6740e4..91b46e4f8197a 100644 --- a/src/Symfony/Component/Security/Http/Firewall/DigestAuthenticationListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/DigestAuthenticationListener.php @@ -118,6 +118,8 @@ public function handle(GetResponseEvent $event) $this->logger->info('Digest authentication successful.', array('username' => $digestAuth->getUsername(), 'received' => $digestAuth->getResponse())); } + $this->migrateSession($request); + $this->tokenStorage->setToken(new UsernamePasswordToken($user, $user->getPassword(), $this->providerKey)); } @@ -134,6 +136,18 @@ private function fail(GetResponseEvent $event, Request $request, AuthenticationE $event->setResponse($this->authenticationEntryPoint->start($request, $authException)); } + + private function migrateSession(Request $request) + { + if (!$request->hasSession() || !$request->hasPreviousSession()) { + return; + } + + // Destroying the old session is broken in php 5.4.0 - 5.4.10 + // See https://bugs.php.net/63379 + $destroy = \PHP_VERSION_ID < 50400 || \PHP_VERSION_ID >= 50411; + $request->getSession()->migrate($destroy); + } } class DigestData diff --git a/src/Symfony/Component/Security/Http/Firewall/SimplePreAuthenticationListener.php b/src/Symfony/Component/Security/Http/Firewall/SimplePreAuthenticationListener.php index dd51869405547..23e517969f4e5 100644 --- a/src/Symfony/Component/Security/Http/Firewall/SimplePreAuthenticationListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/SimplePreAuthenticationListener.php @@ -11,6 +11,7 @@ namespace Symfony\Component\Security\Http\Firewall; +use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface; use Psr\Log\LoggerInterface; use Symfony\Component\HttpKernel\Event\GetResponseEvent; @@ -85,6 +86,9 @@ public function handle(GetResponseEvent $event) } $token = $this->authenticationManager->authenticate($token); + + $this->migrateSession($request); + $this->tokenStorage->setToken($token); if (null !== $this->dispatcher) { @@ -119,4 +123,16 @@ public function handle(GetResponseEvent $event) } } } + + private function migrateSession(Request $request) + { + if (!$request->hasSession() || !$request->hasPreviousSession()) { + return; + } + + // Destroying the old session is broken in php 5.4.0 - 5.4.10 + // See https://bugs.php.net/63379 + $destroy = \PHP_VERSION_ID < 50400 || \PHP_VERSION_ID >= 50411; + $request->getSession()->migrate($destroy); + } } diff --git a/src/Symfony/Component/Security/Http/Logout/CsrfTokenClearingLogoutHandler.php b/src/Symfony/Component/Security/Http/Logout/CsrfTokenClearingLogoutHandler.php new file mode 100644 index 0000000000000..ad6b888aad562 --- /dev/null +++ b/src/Symfony/Component/Security/Http/Logout/CsrfTokenClearingLogoutHandler.php @@ -0,0 +1,35 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Http\Logout; + +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Csrf\TokenStorage\ClearableTokenStorageInterface; + +/** + * @author Christian Flothmann + */ +class CsrfTokenClearingLogoutHandler implements LogoutHandlerInterface +{ + private $csrfTokenStorage; + + public function __construct(ClearableTokenStorageInterface $csrfTokenStorage) + { + $this->csrfTokenStorage = $csrfTokenStorage; + } + + public function logout(Request $request, Response $response, TokenInterface $token) + { + $this->csrfTokenStorage->clear(); + } +} diff --git a/src/Symfony/Component/Security/Http/Session/SessionAuthenticationStrategy.php b/src/Symfony/Component/Security/Http/Session/SessionAuthenticationStrategy.php index b431289392561..15e9b24bb9771 100644 --- a/src/Symfony/Component/Security/Http/Session/SessionAuthenticationStrategy.php +++ b/src/Symfony/Component/Security/Http/Session/SessionAuthenticationStrategy.php @@ -47,8 +47,11 @@ public function onAuthentication(Request $request, TokenInterface $token) return; case self::MIGRATE: + // Note: this logic is duplicated in several authentication listeners + // until Symfony 5.0 due to a security fix with BC compat + // Destroying the old session is broken in php 5.4.0 - 5.4.10 - // See php bug #63379 + // See https://bugs.php.net/63379 $destroy = \PHP_VERSION_ID < 50400 || \PHP_VERSION_ID >= 50411; $request->getSession()->migrate($destroy); diff --git a/src/Symfony/Component/Security/Http/Session/SessionAuthenticationStrategyInterface.php b/src/Symfony/Component/Security/Http/Session/SessionAuthenticationStrategyInterface.php index 9b05f151340ee..8de89b1868d16 100644 --- a/src/Symfony/Component/Security/Http/Session/SessionAuthenticationStrategyInterface.php +++ b/src/Symfony/Component/Security/Http/Session/SessionAuthenticationStrategyInterface.php @@ -27,8 +27,8 @@ interface SessionAuthenticationStrategyInterface /** * This performs any necessary changes to the session. * - * This method is called before the TokenStorage is populated with a - * Token, and only by classes inheriting from AbstractAuthenticationListener. + * This method should be called before the TokenStorage is populated with a + * Token. It should be used by authentication listeners when a session is used. */ public function onAuthentication(Request $request, TokenInterface $token); } diff --git a/src/Symfony/Component/Security/Http/Tests/Logout/CsrfTokenClearingLogoutHandlerTest.php b/src/Symfony/Component/Security/Http/Tests/Logout/CsrfTokenClearingLogoutHandlerTest.php new file mode 100644 index 0000000000000..fe34eaa6e5da3 --- /dev/null +++ b/src/Symfony/Component/Security/Http/Tests/Logout/CsrfTokenClearingLogoutHandlerTest.php @@ -0,0 +1,76 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Http\Tests\Logout; + +use PHPUnit\Framework\TestCase; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\HttpFoundation\Session\Session; +use Symfony\Component\HttpFoundation\Session\Storage\MockArraySessionStorage; +use Symfony\Component\Security\Csrf\TokenStorage\SessionTokenStorage; +use Symfony\Component\Security\Http\Logout\CsrfTokenClearingLogoutHandler; + +class CsrfTokenClearingLogoutHandlerTest extends TestCase +{ + private $session; + private $csrfTokenStorage; + private $csrfTokenClearingLogoutHandler; + + protected function setUp() + { + $this->session = new Session(new MockArraySessionStorage()); + $this->csrfTokenStorage = new SessionTokenStorage($this->session, 'foo'); + $this->csrfTokenStorage->setToken('foo', 'bar'); + $this->csrfTokenStorage->setToken('foobar', 'baz'); + $this->csrfTokenClearingLogoutHandler = new CsrfTokenClearingLogoutHandler($this->csrfTokenStorage); + } + + public function testCsrfTokenCookieWithSameNamespaceIsRemoved() + { + $this->assertSame('bar', $this->session->get('foo/foo')); + $this->assertSame('baz', $this->session->get('foo/foobar')); + + $this->csrfTokenClearingLogoutHandler->logout(new Request(), new Response(), $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock()); + + $this->assertFalse($this->csrfTokenStorage->hasToken('foo')); + $this->assertFalse($this->csrfTokenStorage->hasToken('foobar')); + + $this->assertFalse($this->session->has('foo/foo')); + $this->assertFalse($this->session->has('foo/foobar')); + } + + public function testCsrfTokenCookieWithDifferentNamespaceIsNotRemoved() + { + $barNamespaceCsrfSessionStorage = new SessionTokenStorage($this->session, 'bar'); + $barNamespaceCsrfSessionStorage->setToken('foo', 'bar'); + $barNamespaceCsrfSessionStorage->setToken('foobar', 'baz'); + + $this->assertSame('bar', $this->session->get('foo/foo')); + $this->assertSame('baz', $this->session->get('foo/foobar')); + $this->assertSame('bar', $this->session->get('bar/foo')); + $this->assertSame('baz', $this->session->get('bar/foobar')); + + $this->csrfTokenClearingLogoutHandler->logout(new Request(), new Response(), $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock()); + + $this->assertTrue($barNamespaceCsrfSessionStorage->hasToken('foo')); + $this->assertTrue($barNamespaceCsrfSessionStorage->hasToken('foobar')); + $this->assertSame('bar', $barNamespaceCsrfSessionStorage->getToken('foo')); + $this->assertSame('baz', $barNamespaceCsrfSessionStorage->getToken('foobar')); + $this->assertFalse($this->csrfTokenStorage->hasToken('foo')); + $this->assertFalse($this->csrfTokenStorage->hasToken('foobar')); + + $this->assertFalse($this->session->has('foo/foo')); + $this->assertFalse($this->session->has('foo/foobar')); + $this->assertSame('bar', $this->session->get('bar/foo')); + $this->assertSame('baz', $this->session->get('bar/foobar')); + } +} diff --git a/src/Symfony/Component/Security/Http/composer.json b/src/Symfony/Component/Security/Http/composer.json index 6e289031921d9..f04958bf431b8 100644 --- a/src/Symfony/Component/Security/Http/composer.json +++ b/src/Symfony/Component/Security/Http/composer.json @@ -24,9 +24,12 @@ }, "require-dev": { "symfony/routing": "~2.2", - "symfony/security-csrf": "~2.4", + "symfony/security-csrf": "~2.7.48 || ~2.8.41", "psr/log": "~1.0" }, + "conflict": { + "symfony/security-csrf": "<2.7.48 || >=2.8.0,<2.8.41 || >=3.0.0" + }, "suggest": { "symfony/security-csrf": "For using tokens to protect authentication/logout attempts", "symfony/routing": "For using the HttpUtils class to create sub-requests, redirect the user, and match URLs"