Add configuration option to enable more secure CSRF token (#16481)

* Add configuration option to enable more secure CSRF token

Backport the fixes from #14431 to 3.x. This will resolve
GHSA-j33j-fg2g-mcv2 aka CVE-2020-15400 in a backwards compatible way.
I didn't initially make this change because of the backwards
compatibility risk. However, I think a setting based approach at least
gives application developers a way to have a clean security audit and
maintain backwards compatibility.

Fixes #16326

src/Http/Middleware/CsrfProtectionMiddleware.php

@@ -46,9 +46,14 @@ class CsrfProtectionMiddleware
      *    Defaults to browser session.
      *  - `secure` Whether or not the cookie will be set with the Secure flag. Defaults to false.
      *  - `httpOnly` Whether or not the cookie will be set with the HttpOnly flag. Defaults to false.
-     * - `samesite` Value for "SameSite" attribute. Default to null.
+     *  - `samesite` Value for "SameSite" attribute. Default to null.
      *  - `field` The form field to check. Changing this will also require configuring
      *    FormHelper.
+     *  - `verifyTokenSource` Generate and verify tokens that include the application salt
+     *    value. This prevents tokens from being manipulated by an attacker via XSS or physical
+     *    access. This behavior is disabled by default as it is not cross compatible with tokens
+     *    created in earlier versions of CakePHP. It is recommended that you enable this setting
+     *    if possible as it is the default in 4.x.
      *
      * @var array
      */
@@ -59,6 +64,7 @@ class CsrfProtectionMiddleware
         'httpOnly' => false,
         'samesite' => null,
         'field' => '_csrfToken',
+        'verifyTokenSource' => false,
     ];
 
     /**
@@ -78,6 +84,11 @@ class CsrfProtectionMiddleware
     protected $whitelistCallback;
 
     /**
+     * @var int
+     */
+    const TOKEN_VALUE_LENGTH = 16;
+
+    /**
      * Constructor
      *
      * @param array $config Config options. See $_defaultConfig for valid keys.
@@ -115,7 +126,7 @@ class CsrfProtectionMiddleware
 
         $method = $request->getMethod();
         if ($method === 'GET' && $cookieData === null) {
-            $token = $this->_createToken();
+            $token = $this->createToken();
             $request = $this->_addTokenToRequest($token, $request);
             $response = $this->_addTokenCookie($token, $request, $response);
 
@@ -169,7 +180,22 @@ class CsrfProtectionMiddleware
      */
     protected function _createToken()
     {
-        return hash('sha512', Security::randomBytes(16), false);
+        return $this->createToken();
+    }
+
+    /**
+     * Create a new token to be used for CSRF protection.
+     *
+     * @return string
+     */
+    public function createToken()
+    {
+        $value = Security::randomBytes(static::TOKEN_VALUE_LENGTH);
+        if (!$this->_config['verifyTokenSource']) {
+            return hash('sha512', $value, false);
+        }
+
+        return $value . hash_hmac('sha1', $value, Security::getSalt());
     }
 
     /**
@@ -231,8 +257,61 @@ class CsrfProtectionMiddleware
             throw new InvalidCsrfTokenException(__d('cake', 'Missing CSRF token cookie'));
         }
 
-        if (!Security::constantEquals($post, $cookie) && !Security::constantEquals($header, $cookie)) {
+        if ($this->_config['verifyTokenSource']) {
+            // This path validates that the token was generated by our application.
+            if ($this->_compareToken($post, $cookie) || $this->_compareToken($header, $cookie)) {
+                return;
+            }
+
             throw new InvalidCsrfTokenException(__d('cake', 'CSRF token mismatch.'));
         }
+
+        // Backwards compatibility mode. This path compares tokens as opaque strings.
+        if (Security::constantEquals($post, $cookie) || Security::constantEquals($header, $cookie)) {
+            return;
+        }
+
+        throw new InvalidCsrfTokenException(__d('cake', 'CSRF token mismatch.'));
+    }
+
+    /**
+     * Ensure that the request token matches the cookie value and that
+     * both were generated by us.
+     *
+     * @param mixed $post The request token.
+     * @param mixed $cookie The cookie token.
+     * @return bool
+     */
+    protected function _compareToken($post, $cookie)
+    {
+        if (!is_string($post)) {
+            $post = '';
+        }
+        if (!is_string($cookie)) {
+            $cookie = '';
+        }
+        $postKey = (string)substr($post, 0, static::TOKEN_VALUE_LENGTH);
+        $postHmac = (string)substr($post, static::TOKEN_VALUE_LENGTH);
+        $cookieKey = (string)substr($cookie, 0, static::TOKEN_VALUE_LENGTH);
+        $cookieHmac = (string)substr($cookie, static::TOKEN_VALUE_LENGTH);
+
+        // Put all checks in a list
+        // so they all burn time reducing timing attack window.
+        $checks = [
+            hash_equals($postKey, $cookieKey),
+            hash_equals($postHmac, $cookieHmac),
+            hash_equals(
+                $postHmac,
+                hash_hmac('sha1', $postKey, Security::getSalt())
+            ),
+        ];
+
+        foreach ($checks as $check) {
+            if ($check !== true) {
+                return false;
+            }
+        }
+
+        return true;
     }
 }

src/TestSuite/IntegrationTestTrait.php

@@ -15,7 +15,7 @@ namespace Cake\TestSuite;
 
 use Cake\Core\Configure;
 use Cake\Database\Exception as DatabaseException;
-use Cake\Http\ServerRequest;
+use Cake\Http\Middleware\CsrfProtectionMiddleware;
 use Cake\Http\Session;
 use Cake\Routing\Router;
 use Cake\TestSuite\Constraint\Response\BodyContains;
@@ -51,7 +51,6 @@ use Cake\TestSuite\Stub\TestExceptionRenderer;
 use Cake\Utility\CookieCryptTrait;
 use Cake\Utility\Hash;
 use Cake\Utility\Security;
-use Cake\Utility\Text;
 use Cake\View\Helper\SecureFieldTokenTrait;
 use Exception;
 use Laminas\Diactoros\Uri;
@@ -701,8 +700,11 @@ trait IntegrationTestTrait
         }
 
         if ($this->_csrfToken === true) {
+            // While most applications will not be using verify tokens, we enable
+            // it for tests so that if applications upgrade they don't face testing failures.
+            $middleware = new CsrfProtectionMiddleware(['verifyTokenSource' => true]);
             if (!isset($this->_cookie['csrfToken'])) {
-                $this->_cookie['csrfToken'] = Text::uuid();
+                $this->_cookie['csrfToken'] = $middleware->createToken();
             }
             if (!isset($data['_csrfToken'])) {
                 $data['_csrfToken'] = $this->_cookie['csrfToken'];

tests/TestCase/Http/Middleware/CsrfProtectionMiddlewareTest.php

@@ -190,6 +190,105 @@ class CsrfProtectionMiddlewareTest extends TestCase
     }
 
     /**
+     * Test that the X-CSRF-Token works with the various http methods.
+     *
+     * @dataProvider httpMethodProvider
+     * @return void
+     */
+    public function testValidTokenInHeaderVerifySource($method)
+    {
+        $middleware = new CsrfProtectionMiddleware(['verifyTokenSource' => true]);
+        $token = $middleware->createToken();
+        $request = new ServerRequest([
+            'environment' => [
+                'REQUEST_METHOD' => $method,
+                'HTTP_X_CSRF_TOKEN' => $token,
+            ],
+            'post' => ['a' => 'b'],
+            'cookies' => ['csrfToken' => $token],
+        ]);
+        $response = new Response();
+
+        // No exception means the test is valid
+        $response = $middleware($request, $response, $this->_getNextClosure());
+        $this->assertInstanceOf(Response::class, $response);
+    }
+
+    /**
+     * Test that the X-CSRF-Token works with the various http methods.
+     *
+     * @dataProvider httpMethodProvider
+     * @return void
+     */
+    public function testInvalidTokenInHeaderVerifySource($method)
+    {
+        $this->expectException(\Cake\Http\Exception\InvalidCsrfTokenException::class);
+        $request = new ServerRequest([
+            'environment' => [
+                'REQUEST_METHOD' => $method,
+                // Even though the values match they are not signed.
+                'HTTP_X_CSRF_TOKEN' => 'nope',
+            ],
+            'post' => ['a' => 'b'],
+            'cookies' => ['csrfToken' => 'nope'],
+        ]);
+        $response = new Response();
+
+        $middleware = new CsrfProtectionMiddleware(['verifyTokenSource' => true]);
+        $middleware($request, $response, $this->_getNextClosure());
+    }
+
+    /**
+     * Test that request data works with the various http methods.
+     *
+     * @dataProvider httpMethodProvider
+     * @return void
+     */
+    public function testValidTokenRequestDataVerifySource($method)
+    {
+        $middleware = new CsrfProtectionMiddleware(['verifyTokenSource' => true]);
+        $token = $middleware->createToken();
+        $request = new ServerRequest([
+            'environment' => [
+                'REQUEST_METHOD' => $method,
+            ],
+            'post' => ['_csrfToken' => $token],
+            'cookies' => ['csrfToken' => $token],
+        ]);
+        $response = new Response();
+
+        $closure = function ($request, $response) {
+            $this->assertNull($request->getData('_csrfToken'));
+        };
+
+        // No exception means everything is OK
+        $middleware($request, $response, $closure);
+    }
+
+    /**
+     * Test that request data works with the various http methods.
+     *
+     * @dataProvider httpMethodProvider
+     * @return void
+     */
+    public function testInvalidTokenRequestDataVerifySource($method)
+    {
+        $this->expectException(\Cake\Http\Exception\InvalidCsrfTokenException::class);
+        $request = new ServerRequest([
+            'environment' => [
+                'REQUEST_METHOD' => $method,
+            ],
+            // Even though the tokens match they are not signed.
+            'post' => ['_csrfToken' => 'example-token'],
+            'cookies' => ['csrfToken' => 'example-token'],
+        ]);
+        $response = new Response();
+
+        $middleware = new CsrfProtectionMiddleware(['verifyTokenSource' => true]);
+        $middleware($request, $response, $this->_getNextClosure());
+    }
+
+    /**
      * Test that request data works with the various http methods.
      *
      * @dataProvider httpMethodProvider