From: =?utf-8?q?K=C3=A9vin_Dunglas?= <dunglas@gmail.com>
Date: Mon, 6 Nov 2017 23:19:48 +0100
Subject: [Security] Namespace generated CSRF tokens depending of the current
 scheme

[CVE-2017-16653] https://symfony.com/blog/cve-2017-16653-csrf-protection-does-not-use-different-tokens-for-http-and-https

Origin: backport, https://github.com/symfony/symfony/commit/cdb4271975667ed4a31365ab086dec63f25c1e1c
---
 .../Resources/config/security_csrf.xml             |   1 +
 .../Component/Security/Csrf/CsrfTokenManager.php   |  58 +++++--
 .../Security/Csrf/Tests/CsrfTokenManagerTest.php   | 193 ++++++++++++++-------
 3 files changed, 172 insertions(+), 80 deletions(-)

diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/security_csrf.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/security_csrf.xml
index dc57fd6..4bab610 100644
--- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/security_csrf.xml
+++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/security_csrf.xml
@@ -20,6 +20,7 @@
         <service id="security.csrf.token_manager" class="%security.csrf.token_manager.class%">
             <argument type="service" id="security.csrf.token_generator" />
             <argument type="service" id="security.csrf.token_storage" />
+            <argument type="service" id="request_stack" on-invalid="ignore" />
         </service>
     </services>
 </container>
diff --git a/src/Symfony/Component/Security/Csrf/CsrfTokenManager.php b/src/Symfony/Component/Security/Csrf/CsrfTokenManager.php
index cdda543..35da1ca 100644
--- a/src/Symfony/Component/Security/Csrf/CsrfTokenManager.php
+++ b/src/Symfony/Component/Security/Csrf/CsrfTokenManager.php
@@ -11,6 +11,8 @@
 
 namespace Symfony\Component\Security\Csrf;
 
+use Symfony\Component\HttpFoundation\RequestStack;
+use Symfony\Component\Security\Core\Exception\InvalidArgumentException;
 use Symfony\Component\Security\Csrf\TokenGenerator\UriSafeTokenGenerator;
 use Symfony\Component\Security\Csrf\TokenGenerator\TokenGeneratorInterface;
 use Symfony\Component\Security\Csrf\TokenStorage\NativeSessionTokenStorage;
@@ -20,6 +22,7 @@ use Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface;
  * Default implementation of {@link CsrfTokenManagerInterface}.
  *
  * @author Bernhard Schussek <bschussek@gmail.com>
+ * @author Kévin Dunglas <dunglas@gmail.com>
  */
 class CsrfTokenManager implements CsrfTokenManagerInterface
 {
@@ -32,18 +35,39 @@ class CsrfTokenManager implements CsrfTokenManagerInterface
      * @var TokenStorageInterface
      */
     private $storage;
+    private $namespace;
 
     /**
-     * Creates a new CSRF provider using PHP's native session storage.
-     *
-     * @param TokenGeneratorInterface|null $generator The token generator
-     * @param TokenStorageInterface|null   $storage   The storage for storing
-     *                                                generated CSRF tokens
+     * @param null|string|RequestStack|callable $namespace
+     *   * null: generates a namespace using $_SERVER['HTTPS']
+     *   * string: uses the given string
+     *   * RequestStack: generates a namespace using the current master request
+     *   * callable: uses the result of this callable (must return a string)
      */
-    public function __construct(TokenGeneratorInterface $generator = null, TokenStorageInterface $storage = null)
+    public function __construct(TokenGeneratorInterface $generator = null, TokenStorageInterface $storage = null, $namespace = null)
     {
         $this->generator = $generator ?: new UriSafeTokenGenerator();
         $this->storage = $storage ?: new NativeSessionTokenStorage();
+
+        $superGlobalNamespaceGenerator = function () {
+            return !empty($_SERVER['HTTPS']) && 'off' !== strtolower($_SERVER['HTTPS']) ? 'https-' : '';
+        };
+
+        if (null === $namespace) {
+            $this->namespace = $superGlobalNamespaceGenerator;
+        } elseif ($namespace instanceof RequestStack) {
+            $this->namespace = function () use ($namespace, $superGlobalNamespaceGenerator) {
+                if ($request = $namespace->getMasterRequest()) {
+                    return $request->isSecure() ? 'https-' : '';
+                }
+
+                return $superGlobalNamespaceGenerator();
+            };
+        } elseif (is_callable($namespace) || is_string($namespace)) {
+            $this->namespace = $namespace;
+        } else {
+            throw new InvalidArgumentException(sprintf('$namespace must be a string, a callable returning a string, null or an instance of "RequestStack". "%s" given.', gettype($namespace)));
+        }
     }
 
     /**
@@ -51,12 +75,13 @@ class CsrfTokenManager implements CsrfTokenManagerInterface
      */
     public function getToken($tokenId)
     {
-        if ($this->storage->hasToken($tokenId)) {
-            $value = $this->storage->getToken($tokenId);
+        $namespacedId = $this->getNamespace().$tokenId;
+        if ($this->storage->hasToken($namespacedId)) {
+            $value = $this->storage->getToken($namespacedId);
         } else {
             $value = $this->generator->generateToken();
 
-            $this->storage->setToken($tokenId, $value);
+            $this->storage->setToken($namespacedId, $value);
         }
 
         return new CsrfToken($tokenId, $value);
@@ -67,9 +92,10 @@ class CsrfTokenManager implements CsrfTokenManagerInterface
      */
     public function refreshToken($tokenId)
     {
+        $namespacedId = $this->getNamespace().$tokenId;
         $value = $this->generator->generateToken();
 
-        $this->storage->setToken($tokenId, $value);
+        $this->storage->setToken($namespacedId, $value);
 
         return new CsrfToken($tokenId, $value);
     }
@@ -79,7 +105,7 @@ class CsrfTokenManager implements CsrfTokenManagerInterface
      */
     public function removeToken($tokenId)
     {
-        return $this->storage->removeToken($tokenId);
+        return $this->storage->removeToken($this->getNamespace().$tokenId);
     }
 
     /**
@@ -87,10 +113,16 @@ class CsrfTokenManager implements CsrfTokenManagerInterface
      */
     public function isTokenValid(CsrfToken $token)
     {
-        if (!$this->storage->hasToken($token->getId())) {
+        $namespacedId = $this->getNamespace().$token->getId();
+        if (!$this->storage->hasToken($namespacedId)) {
             return false;
         }
 
-        return hash_equals($this->storage->getToken($token->getId()), $token->getValue());
+        return hash_equals($this->storage->getToken($namespacedId), $token->getValue());
+    }
+
+    private function getNamespace()
+    {
+        return is_callable($ns = $this->namespace) ? $ns() : $ns;
     }
 }
diff --git a/src/Symfony/Component/Security/Csrf/Tests/CsrfTokenManagerTest.php b/src/Symfony/Component/Security/Csrf/Tests/CsrfTokenManagerTest.php
index 3112038..59ab621 100644
--- a/src/Symfony/Component/Security/Csrf/Tests/CsrfTokenManagerTest.php
+++ b/src/Symfony/Component/Security/Csrf/Tests/CsrfTokenManagerTest.php
@@ -11,6 +11,8 @@
 
 namespace Symfony\Component\Security\Csrf\Tests;
 
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\Security\Csrf\CsrfToken;
 use Symfony\Component\Security\Csrf\CsrfTokenManager;
 
@@ -20,145 +22,202 @@ use Symfony\Component\Security\Csrf\CsrfTokenManager;
 class CsrfTokenManagerTest extends \PHPUnit_Framework_TestCase
 {
     /**
-     * @var \PHPUnit_Framework_MockObject_MockObject
+     * @dataProvider getManagerGeneratorAndStorage
      */
-    private $generator;
-
-    /**
-     * @var \PHPUnit_Framework_MockObject_MockObject
-     */
-    private $storage;
-
-    /**
-     * @var CsrfTokenManager
-     */
-    private $manager;
-
-    protected function setUp()
-    {
-        $this->generator = $this->getMock('Symfony\Component\Security\Csrf\TokenGenerator\TokenGeneratorInterface');
-        $this->storage = $this->getMock('Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface');
-        $this->manager = new CsrfTokenManager($this->generator, $this->storage);
-    }
-
-    protected function tearDown()
-    {
-        $this->generator = null;
-        $this->storage = null;
-        $this->manager = null;
-    }
-
-    public function testGetNonExistingToken()
+    public function testGetNonExistingToken($namespace, $manager, $storage, $generator)
     {
-        $this->storage->expects($this->once())
+        $storage->expects($this->once())
             ->method('hasToken')
-            ->with('token_id')
+            ->with($namespace.'token_id')
             ->will($this->returnValue(false));
 
-        $this->generator->expects($this->once())
+        $generator->expects($this->once())
             ->method('generateToken')
             ->will($this->returnValue('TOKEN'));
 
-        $this->storage->expects($this->once())
+        $storage->expects($this->once())
             ->method('setToken')
-            ->with('token_id', 'TOKEN');
+            ->with($namespace.'token_id', 'TOKEN');
 
-        $token = $this->manager->getToken('token_id');
+        $token = $manager->getToken('token_id');
 
         $this->assertInstanceOf('Symfony\Component\Security\Csrf\CsrfToken', $token);
         $this->assertSame('token_id', $token->getId());
         $this->assertSame('TOKEN', $token->getValue());
     }
 
-    public function testUseExistingTokenIfAvailable()
+    /**
+     * @dataProvider getManagerGeneratorAndStorage
+     */
+    public function testUseExistingTokenIfAvailable($namespace, $manager, $storage)
     {
-        $this->storage->expects($this->once())
+        $storage->expects($this->once())
             ->method('hasToken')
-            ->with('token_id')
+            ->with($namespace.'token_id')
             ->will($this->returnValue(true));
 
-        $this->storage->expects($this->once())
+        $storage->expects($this->once())
             ->method('getToken')
-            ->with('token_id')
+            ->with($namespace.'token_id')
             ->will($this->returnValue('TOKEN'));
 
-        $token = $this->manager->getToken('token_id');
+        $token = $manager->getToken('token_id');
 
         $this->assertInstanceOf('Symfony\Component\Security\Csrf\CsrfToken', $token);
         $this->assertSame('token_id', $token->getId());
         $this->assertSame('TOKEN', $token->getValue());
     }
 
-    public function testRefreshTokenAlwaysReturnsNewToken()
+    /**
+     * @dataProvider getManagerGeneratorAndStorage
+     */
+    public function testRefreshTokenAlwaysReturnsNewToken($namespace, $manager, $storage, $generator)
     {
-        $this->storage->expects($this->never())
+        $storage->expects($this->never())
             ->method('hasToken');
 
-        $this->generator->expects($this->once())
+        $generator->expects($this->once())
             ->method('generateToken')
             ->will($this->returnValue('TOKEN'));
 
-        $this->storage->expects($this->once())
+        $storage->expects($this->once())
             ->method('setToken')
-            ->with('token_id', 'TOKEN');
+            ->with($namespace.'token_id', 'TOKEN');
 
-        $token = $this->manager->refreshToken('token_id');
+        $token = $manager->refreshToken('token_id');
 
         $this->assertInstanceOf('Symfony\Component\Security\Csrf\CsrfToken', $token);
         $this->assertSame('token_id', $token->getId());
         $this->assertSame('TOKEN', $token->getValue());
     }
 
-    public function testMatchingTokenIsValid()
+    /**
+     * @dataProvider getManagerGeneratorAndStorage
+     */
+    public function testMatchingTokenIsValid($namespace, $manager, $storage)
     {
-        $this->storage->expects($this->once())
+        $storage->expects($this->once())
             ->method('hasToken')
-            ->with('token_id')
+            ->with($namespace.'token_id')
             ->will($this->returnValue(true));
 
-        $this->storage->expects($this->once())
+        $storage->expects($this->once())
             ->method('getToken')
-            ->with('token_id')
+            ->with($namespace.'token_id')
             ->will($this->returnValue('TOKEN'));
 
-        $this->assertTrue($this->manager->isTokenValid(new CsrfToken('token_id', 'TOKEN')));
+        $this->assertTrue($manager->isTokenValid(new CsrfToken('token_id', 'TOKEN')));
     }
 
-    public function testNonMatchingTokenIsNotValid()
+    /**
+     * @dataProvider getManagerGeneratorAndStorage
+     */
+    public function testNonMatchingTokenIsNotValid($namespace, $manager, $storage)
     {
-        $this->storage->expects($this->once())
+        $storage->expects($this->once())
             ->method('hasToken')
-            ->with('token_id')
+            ->with($namespace.'token_id')
             ->will($this->returnValue(true));
 
-        $this->storage->expects($this->once())
+        $storage->expects($this->once())
             ->method('getToken')
-            ->with('token_id')
+            ->with($namespace.'token_id')
             ->will($this->returnValue('TOKEN'));
 
-        $this->assertFalse($this->manager->isTokenValid(new CsrfToken('token_id', 'FOOBAR')));
+        $this->assertFalse($manager->isTokenValid(new CsrfToken('token_id', 'FOOBAR')));
     }
 
-    public function testNonExistingTokenIsNotValid()
+    /**
+     * @dataProvider getManagerGeneratorAndStorage
+     */
+    public function testNonExistingTokenIsNotValid($namespace, $manager, $storage)
     {
-        $this->storage->expects($this->once())
+        $storage->expects($this->once())
             ->method('hasToken')
-            ->with('token_id')
+            ->with($namespace.'token_id')
             ->will($this->returnValue(false));
 
-        $this->storage->expects($this->never())
+        $storage->expects($this->never())
             ->method('getToken');
 
-        $this->assertFalse($this->manager->isTokenValid(new CsrfToken('token_id', 'FOOBAR')));
+        $this->assertFalse($manager->isTokenValid(new CsrfToken('token_id', 'FOOBAR')));
     }
 
-    public function testRemoveToken()
+    /**
+     * @dataProvider getManagerGeneratorAndStorage
+     */
+    public function testRemoveToken($namespace, $manager, $storage)
     {
-        $this->storage->expects($this->once())
+        $storage->expects($this->once())
             ->method('removeToken')
-            ->with('token_id')
+            ->with($namespace.'token_id')
             ->will($this->returnValue('REMOVED_TOKEN'));
 
-        $this->assertSame('REMOVED_TOKEN', $this->manager->removeToken('token_id'));
+        $this->assertSame('REMOVED_TOKEN', $manager->removeToken('token_id'));
+    }
+
+    public function testNamespaced()
+    {
+        $generator = $this->getMockBuilder('Symfony\Component\Security\Csrf\TokenGenerator\TokenGeneratorInterface')->getMock();
+        $storage = $this->getMockBuilder('Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface')->getMock();
+
+        $requestStack = new RequestStack();
+        $requestStack->push(new Request(array(), array(), array(), array(), array(), array('HTTPS' => 'on')));
+
+        $manager = new CsrfTokenManager($generator, $storage, null, $requestStack);
+
+        $token = $manager->getToken('foo');
+        $this->assertSame('foo', $token->getId());
+    }
+
+    public function getManagerGeneratorAndStorage()
+    {
+        $data = array();
+
+        list($generator, $storage) = $this->getGeneratorAndStorage();
+        $data[] = array('', new CsrfTokenManager($generator, $storage, ''), $storage, $generator);
+
+        list($generator, $storage) = $this->getGeneratorAndStorage();
+        $data[] = array('https-', new CsrfTokenManager($generator, $storage), $storage, $generator);
+
+        list($generator, $storage) = $this->getGeneratorAndStorage();
+        $data[] = array('aNamespace-', new CsrfTokenManager($generator, $storage, 'aNamespace-'), $storage, $generator);
+
+        $requestStack = new RequestStack();
+        $requestStack->push(new Request(array(), array(), array(), array(), array(), array('HTTPS' => 'on')));
+        list($generator, $storage) = $this->getGeneratorAndStorage();
+        $data[] = array('https-', new CsrfTokenManager($generator, $storage, $requestStack), $storage, $generator);
+
+        list($generator, $storage) = $this->getGeneratorAndStorage();
+        $data[] = array('generated-', new CsrfTokenManager($generator, $storage, function () {
+            return 'generated-';
+        }), $storage, $generator);
+
+        $requestStack = new RequestStack();
+        $requestStack->push(new Request());
+        list($generator, $storage) = $this->getGeneratorAndStorage();
+        $data[] = array('', new CsrfTokenManager($generator, $storage, $requestStack), $storage, $generator);
+
+        return $data;
+    }
+
+    private function getGeneratorAndStorage()
+    {
+        return array(
+            $this->getMockBuilder('Symfony\Component\Security\Csrf\TokenGenerator\TokenGeneratorInterface')->getMock(),
+            $this->getMockBuilder('Symfony\Component\Security\Csrf\TokenStorage\TokenStorageInterface')->getMock(),
+        );
+    }
+
+    public function setUp()
+    {
+        $_SERVER['HTTPS'] = 'on';
+    }
+
+    public function tearDown()
+    {
+        parent::tearDown();
+
+        unset($_SERVER['HTTPS']);
     }
 }