Skip to content

Commit

Permalink
Merge pull request #467 from RobertoRoos/feature/cookie_salt
Browse files Browse the repository at this point in the history
Added the default option to add the application salt to the cookie
  • Loading branch information
markstory authored Jan 4, 2022
2 parents 3131dd1 + 800e0e5 commit 463bd69
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 6 deletions.
4 changes: 4 additions & 0 deletions docs/en/authenticators.rst
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,10 @@ Configuration options:
``null`` and all pages will be checked.
- **passwordHasher**: Password hasher to use for token hashing. Default
is ``DefaultPasswordHasher::class``.
- **salt**: When ``false`` no salt is used. When a string is passed that value is used as a salt value.
When ``true`` the default Security.salt is used. Default is ``true``. When a salt is used, the cookie value
will contain `hash(username + password + hmac(username + password, salt))`. This helps harden tokens against possible
database leaks and enables cookie values to be invalidated by rotating the salt value.

Usage
-----
Expand Down
23 changes: 21 additions & 2 deletions src/Authenticator/CookieAuthenticator.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
use Authentication\UrlChecker\UrlCheckerTrait;
use Cake\Http\Cookie\Cookie;
use Cake\Http\Cookie\CookieInterface;
use Cake\Utility\Security;
use InvalidArgumentException;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use RuntimeException;
Expand Down Expand Up @@ -51,6 +53,7 @@ class CookieAuthenticator extends AbstractAuthenticator implements PersistenceIn
'name' => 'CookieAuth',
],
'passwordHasher' => 'Authentication.Default',
'salt' => true,
];

/**
Expand Down Expand Up @@ -144,7 +147,7 @@ public function persistIdentity(ServerRequestInterface $request, ResponseInterfa
/**
* Creates a plain part of a cookie token.
*
* Returns concatenated username and password hash.
* Returns concatenated username, password hash, and HMAC signature.
*
* @param array|\ArrayAccess $identity Identity data.
* @return string
Expand All @@ -154,7 +157,23 @@ protected function _createPlainToken($identity): string
$usernameField = $this->getConfig('fields.username');
$passwordField = $this->getConfig('fields.password');

return $identity[$usernameField] . $identity[$passwordField];
$salt = $this->getConfig('salt', '');

$value = $identity[$usernameField] . $identity[$passwordField];

if ($salt === false) {
return $value;
}
if ($salt === true) {
$salt = Security::getSalt();
} elseif (!is_string($salt) || $salt === '') {
throw new InvalidArgumentException('Salt must be a non-empty string.');
}

$hmac = hash_hmac('sha1', $value, $salt);
// Instead of appending the plain salt, we create a hash. This limits the chance of the salt being leaked.

return $value . $hmac;
}

/**
Expand Down
69 changes: 65 additions & 4 deletions tests/TestCase/Authenticator/CookieAuthenticatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,12 @@
use Authentication\Authenticator\CookieAuthenticator;
use Authentication\Authenticator\Result;
use Authentication\Identifier\IdentifierCollection;
use Cake\Core\Configure;
use Cake\Http\Cookie\Cookie;
use Cake\Http\Response;
use Cake\Http\ServerRequestFactory;
use Cake\TestSuite\TestCase;
use InvalidArgumentException;
use Psr\Http\Message\RequestInterface;
use Psr\Http\Message\ResponseInterface;

Expand All @@ -45,6 +47,8 @@ public function setUp(): void
{
$this->skipIf(!class_exists(Cookie::class));

// Note: security salt is written in tests/bootstrap.php

parent::setUp();
}

Expand All @@ -64,7 +68,7 @@ public function testAuthenticateInvalidTokenMissingUsername()
null,
null,
[
'CookieAuth' => '["$2y$10$1bE1SgasKoz9WmEvUfuZLeYa6pQgxUIJ5LAoS/KGmC1hNuWkUG7ES"]',
'CookieAuth' => '["$2y$10$O5VgLDfIqszzr0Q47Ygkc.LkoLIwlIjc/OzoGp6yJasQlxcHU4.ES"]',
]
);

Expand All @@ -91,7 +95,8 @@ public function testAuthenticateSuccess()
null,
null,
[
'CookieAuth' => '["mariano","$2y$10$1bE1SgasKoz9WmEvUfuZLeYa6pQgxUIJ5LAoS/KGmC1hNuWkUG7ES"]',
// hash(username . password . hmac(username . password, salt))
'CookieAuth' => '["mariano","$2y$10$RlCAFt3e/9l42f8SIaIbqejOg9/b/HklPo.fjXY.tFGuluafugssa"]',
]
);

Expand All @@ -118,7 +123,7 @@ public function testAuthenticateExpandedCookie()
null,
null,
[
'CookieAuth' => ['mariano', '$2y$10$1bE1SgasKoz9WmEvUfuZLeYa6pQgxUIJ5LAoS/KGmC1hNuWkUG7ES'],
'CookieAuth' => ['mariano', '$2y$10$RlCAFt3e/9l42f8SIaIbqejOg9/b/HklPo.fjXY.tFGuluafugssa'],
]
);

Expand All @@ -129,6 +134,62 @@ public function testAuthenticateExpandedCookie()
$this->assertSame(Result::SUCCESS, $result->getStatus());
}

/**
* testAuthenticateSuccessNoSalt
*
* @return void
*/
public function testAuthenticateNoSalt()
{
Configure::delete('Security.salt');

$identifiers = new IdentifierCollection([
'Authentication.Password',
]);

$request = ServerRequestFactory::fromGlobals(
['REQUEST_URI' => '/testpath'],
null,
null,
[
// hash(username . password)
'CookieAuth' => '["mariano","$2y$10$yq91zLgrlF0TUzPjFj49DOL44svGrOYxaBfB6QYWEvxVKzNkvcVom"]',
]
);

$authenticator = new CookieAuthenticator($identifiers, ['salt' => false]);
$result = $authenticator->authenticate($request);

$this->assertInstanceOf(Result::class, $result);
$this->assertSame(Result::SUCCESS, $result->getStatus());
}

/**
* testAuthenticateSuccessNoSalt
*
* @return void
*/
public function testAuthenticateInvalidSalt()
{
$identifiers = new IdentifierCollection([
'Authentication.Password',
]);

$request = ServerRequestFactory::fromGlobals(
['REQUEST_URI' => '/testpath'],
null,
null,
[
'CookieAuth' => '["mariano","some_hash"]',
]
);

$authenticator = new CookieAuthenticator($identifiers, ['salt' => '']);

$this->expectException(InvalidArgumentException::class);
$authenticator->authenticate($request);
}

/**
* testAuthenticateUnknownUser
*
Expand Down Expand Up @@ -241,7 +302,7 @@ public function testPersistIdentity()
$this->assertInstanceOf(RequestInterface::class, $result['request']);
$this->assertInstanceOf(ResponseInterface::class, $result['response']);
$this->assertStringContainsString(
'CookieAuth=%5B%22mariano%22%2C%22%242y%2410%24',
'CookieAuth=%5B%22mariano%22%2C%22%242y%2410%24', // `CookieAuth=["mariano","$2y$10$`
$result['response']->getHeaderLine('Set-Cookie')
);
$this->assertStringContainsString(
Expand Down

0 comments on commit 463bd69

Please sign in to comment.