Randomize CSRF token to harden BREACH attacks
authorJérémy Derussé <jeremy@derusse.com>
Wed, 20 Jan 2021 23:20:38 +0000 (00:20 +0100)
committerJérémy Derussé <jeremy@derusse.com>
Thu, 21 Jan 2021 16:55:18 +0000 (17:55 +0100)
src/Symfony/Bundle/SecurityBundle/Tests/Functional/CsrfFormLoginTest.php
src/Symfony/Component/Security/CHANGELOG.md
src/Symfony/Component/Security/Csrf/CsrfTokenManager.php
src/Symfony/Component/Security/Csrf/Tests/CsrfTokenManagerTest.php

index 0302c8a..81a4227 100644 (file)
@@ -36,7 +36,6 @@ class CsrfFormLoginTest extends AbstractWebTestCase
         $logoutLinks = $crawler->selectLink('Log out')->links();
         $this->assertCount(2, $logoutLinks);
         $this->assertStringContainsString('_csrf_token=', $logoutLinks[0]->getUri());
-        $this->assertSame($logoutLinks[0]->getUri(), $logoutLinks[1]->getUri());
 
         $client->click($logoutLinks[0]);
 
index 794ff85..fa3eb5f 100644 (file)
@@ -4,6 +4,7 @@ CHANGELOG
 5.3
 ---
 
+ * Randomize CSRF tokens to harden BREACH attacks
  * Deprecated voters that do not return a valid decision when calling the `vote` method.
 
 5.2.0
index da28302..31a39b0 100644 (file)
@@ -77,7 +77,7 @@ class CsrfTokenManager implements CsrfTokenManagerInterface
             $this->storage->setToken($namespacedId, $value);
         }
 
-        return new CsrfToken($tokenId, $value);
+        return new CsrfToken($tokenId, $this->randomize($value));
     }
 
     /**
@@ -90,7 +90,7 @@ class CsrfTokenManager implements CsrfTokenManagerInterface
 
         $this->storage->setToken($namespacedId, $value);
 
-        return new CsrfToken($tokenId, $value);
+        return new CsrfToken($tokenId, $this->randomize($value));
     }
 
     /**
@@ -111,11 +111,40 @@ class CsrfTokenManager implements CsrfTokenManagerInterface
             return false;
         }
 
-        return hash_equals($this->storage->getToken($namespacedId), $token->getValue());
+        return hash_equals($this->storage->getToken($namespacedId), $this->derandomize($token->getValue()));
     }
 
     private function getNamespace(): string
     {
         return \is_callable($ns = $this->namespace) ? $ns() : $ns;
     }
+
+    private function randomize(string $value): string
+    {
+        $key = random_bytes(32);
+        $value = $this->xor($value, $key);
+
+        return sprintf('%s.%s.%s', substr(md5($key), 0, 1 + (\ord($key[0]) % 32)), rtrim(strtr(base64_encode($key), '+/', '-_'), '='), rtrim(strtr(base64_encode($value), '+/', '-_'), '='));
+    }
+
+    private function derandomize(string $value): string
+    {
+        $parts = explode('.', $value);
+        if (3 !== \count($parts)) {
+            return $value;
+        }
+        $key = base64_decode(strtr($parts[1], '-_', '+/'));
+        $value = base64_decode(strtr($parts[2], '-_', '+/'));
+
+        return $this->xor($value, $key);
+    }
+
+    private function xor(string $value, string $key): string
+    {
+        if (\strlen($value) > \strlen($key)) {
+            $key = str_repeat($key, ceil(\strlen($value) / \strlen($key)));
+        }
+
+        return $value ^ $key;
+    }
 }
index a3e7fad..4138555 100644 (file)
@@ -44,7 +44,7 @@ class CsrfTokenManagerTest extends TestCase
 
         $this->assertInstanceOf(CsrfToken::class, $token);
         $this->assertSame('token_id', $token->getId());
-        $this->assertSame('TOKEN', $token->getValue());
+        $this->assertNotSame('TOKEN', $token->getValue());
     }
 
     /**
@@ -66,7 +66,34 @@ class CsrfTokenManagerTest extends TestCase
 
         $this->assertInstanceOf(CsrfToken::class, $token);
         $this->assertSame('token_id', $token->getId());
-        $this->assertSame('TOKEN', $token->getValue());
+        $this->assertNotSame('TOKEN', $token->getValue());
+    }
+
+    /**
+     * @dataProvider getManagerGeneratorAndStorage
+     */
+    public function testRandomizeTheToken($namespace, $manager, $storage)
+    {
+        $storage->expects($this->any())
+            ->method('hasToken')
+            ->with($namespace.'token_id')
+            ->willReturn(true);
+
+        $storage->expects($this->any())
+            ->method('getToken')
+            ->with($namespace.'token_id')
+            ->willReturn('TOKEN');
+
+        $values = [];
+        $lengths = [];
+        for ($i = 0; $i < 10; ++$i) {
+            $token = $manager->getToken('token_id');
+            $values[] = $token->getValue();
+            $lengths[] = \strlen($token->getValue());
+        }
+
+        $this->assertCount(10, array_unique($values));
+        $this->assertGreaterThan(2, \count(array_unique($lengths)));
     }
 
     /**
@@ -89,13 +116,33 @@ class CsrfTokenManagerTest extends TestCase
 
         $this->assertInstanceOf(CsrfToken::class, $token);
         $this->assertSame('token_id', $token->getId());
-        $this->assertSame('TOKEN', $token->getValue());
+        $this->assertNotSame('TOKEN', $token->getValue());
     }
 
     /**
      * @dataProvider getManagerGeneratorAndStorage
      */
     public function testMatchingTokenIsValid($namespace, $manager, $storage)
+    {
+        $storage->expects($this->exactly(2))
+            ->method('hasToken')
+            ->with($namespace.'token_id')
+            ->willReturn(true);
+
+        $storage->expects($this->exactly(2))
+            ->method('getToken')
+            ->with($namespace.'token_id')
+            ->willReturn('TOKEN');
+
+        $token = $manager->getToken('token_id');
+        $this->assertNotSame('TOKEN', $token->getValue());
+        $this->assertTrue($manager->isTokenValid($token));
+    }
+
+    /**
+     * @dataProvider getManagerGeneratorAndStorage
+     */
+    public function testMatchingTokenIsValidWithLegacyToken($namespace, $manager, $storage)
     {
         $storage->expects($this->once())
             ->method('hasToken')
@@ -170,7 +217,6 @@ class CsrfTokenManagerTest extends TestCase
 
         $token = $manager->getToken('foo');
         $this->assertSame('foo', $token->getId());
-        $this->assertSame('random', $token->getValue());
     }
 
     public function getManagerGeneratorAndStorage()