Skip to content

Commit

Permalink
Add configuration option to enable more secure CSRF token (#16481)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@markstory
markstory committed May 2, 2022
1 parent 477eff8 commit 309c899
Show file tree
Hide file tree
Showing 3 changed files with 187 additions and 7 deletions.
87 changes: 83 additions & 4 deletions src/Http/Middleware/CsrfProtectionMiddleware.php
View file
Expand Up @@ -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
*/
Expand All @@ -59,6 +64,7 @@ class CsrfProtectionMiddleware
'httpOnly' => false,
'samesite' => null,
'field' => '_csrfToken',
'verifyTokenSource' => false,
];

/**
Expand All @@ -77,6 +83,11 @@ class CsrfProtectionMiddleware
*/
protected $whitelistCallback;

/**
* @var int
*/
const TOKEN_VALUE_LENGTH = 16;

/**
* Constructor
*
Expand Down Expand Up @@ -115,7 +126,7 @@ public function __invoke(ServerRequest $request, Response $response, $next)

$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);

Expand Down Expand Up @@ -169,7 +180,22 @@ protected function _validateAndUnsetTokenField(ServerRequest $request)
*/
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());
}

/**
Expand Down Expand Up @@ -231,8 +257,61 @@ protected function _validateToken(ServerRequest $request)
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;
}
}
8 changes: 5 additions & 3 deletions src/TestSuite/IntegrationTestTrait.php
View file
Expand Up @@ -15,7 +15,7 @@

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;
Expand Down Expand Up @@ -51,7 +51,6 @@
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;
Expand Down Expand Up @@ -701,8 +700,11 @@ protected function _addTokens($url, $data)
}

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'];
Expand Down
99 changes: 99 additions & 0 deletions tests/TestCase/Http/Middleware/CsrfProtectionMiddlewareTest.php
View file
Expand Up @@ -189,6 +189,105 @@ public function testValidTokenRequestData($method)
$middleware($request, $response, $closure);
}

/**
* 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.
*
Expand Down

0 comments on commit 309c899

Please sign in to comment.