diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c32f51..a14b513 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## 2.2.4 under development +- Bug #32: Stop exposing CSRF HMAC token identity in token payload and update OWASP documentation link (@samdark) - Enh #82: Explicitly import classes and functions in "use" section (@mspirkov) - Enh #83: Remove unnecessary files from Composer package (@mspirkov) diff --git a/README.md b/README.md index ba66755..2ede656 100644 --- a/README.md +++ b/README.md @@ -144,8 +144,8 @@ return [ In case Yii framework is used along with config plugin, the package is [configured](./config/di-web.php) automatically to use synchronizer token and masked decorator. You can change that depending on your needs. -Use synchronizer token for sensitive anonymous forms; use HMAC token for authenticated-only forms when a submitted -token may stay valid for a few minutes. +Use synchronizer token for sensitive anonymous forms; use HMAC token for authenticated-only forms when it is acceptable +for a submitted token to stay valid until it expires. ```mermaid flowchart TD @@ -168,7 +168,7 @@ Detailed comparison: | File based session GC | May scan session files | Not triggered by CSRF token storage | | Token storage growth | Depends on session storage | Nothing to store | | Token revocation | Possible by removing stored token | Not possible before token expiration | -| Replay within lifetime | Prevented by storage policy | Possible until the token expires | +| Replay within lifetime | Prevented by storage policy | Possible until expiration | To switch token to HMAC: @@ -205,12 +205,12 @@ Package provides `RandomCsrfTokenGenerator` that generates a random token and To learn more about the synchronizer token pattern, [check OWASP CSRF cheat sheet](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#synchronizer-token-pattern). -### HMAC based token +### HMAC signed token -HMAC based token is a stateless CSRF token that does not require any storage. The token is a hash from session ID and -a timestamp used to prevent replay attacks. The token is added to a form. When the form is submitted, we re-generate -the token from the current session ID and a timestamp from the original token. If two hashes match, we check that the -timestamp is less than the token lifetime. +HMAC signed token is a stateless CSRF token that does not require any storage. The token contains expiration timestamp +and random value, and its signature is bound to the current session ID. The token is added to a form. When the form is +submitted, we verify the token signature, check that it belongs to the current session ID, and check that it has not +expired. `HmacCsrfToken` requires implementation of `CsrfTokenIdentityGeneratorInterface` for generating an identity. The package provides `SessionCsrfTokenIdentityGenerator` that is using session ID thus making the session a token scope. @@ -235,8 +235,8 @@ return [ ]; ``` -To learn more about HMAC based token pattern -[check OWASP CSRF cheat sheet](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#hmac-based-token-pattern). +To learn more about employing HMAC CSRF tokens, check the +[OWASP CSRF cheat sheet](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#employing-hmac-csrf-tokens). ### Stub CSRF token diff --git a/src/Hmac/HmacCsrfToken.php b/src/Hmac/HmacCsrfToken.php index a8a5487..704abd5 100644 --- a/src/Hmac/HmacCsrfToken.php +++ b/src/Hmac/HmacCsrfToken.php @@ -4,37 +4,40 @@ namespace Yiisoft\Csrf\Hmac; +use RuntimeException; use Yiisoft\Csrf\CsrfTokenInterface; use Yiisoft\Csrf\Hmac\IdentityGenerator\CsrfTokenIdentityGeneratorInterface; -use Yiisoft\Security\DataIsTamperedException; -use Yiisoft\Security\Mac; -use Yiisoft\Strings\StringHelper; use Yiisoft\Csrf\MaskedCsrfToken; +use Yiisoft\Security\Random; +use Yiisoft\Strings\StringHelper; use function count; +use function hash_equals; +use function hash_hmac; /** - * Stateless CSRF token that does not require any storage. The token is a hash from session ID and a timestamp - * (to prevent replay attacks). It is added to forms. When the form is submitted, we re-generate the token from - * the current session ID and a timestamp from the original token. If two hashes match, we check that timestamp is - * less than {@see HmacCsrfToken::$lifetime}. - * - * The algorithm is also known as "HMAC Based Token". + * Stateless CSRF token that does not require any storage. The token contains expiration timestamp and random value, + * and is signed with a session-bound identity. It is added to forms. When the form is submitted, we verify the token + * signature, check that it belongs to the current session identity, and check that it has not expired. * * Do not forget to decorate the token with {@see MaskedCsrfToken} to prevent BREACH attack. * - * @link https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#hmac-based-token-pattern + * @link https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#employing-hmac-csrf-tokens */ final class HmacCsrfToken implements CsrfTokenInterface { private CsrfTokenIdentityGeneratorInterface $identityGenerator; - private Mac $mac; /** * @var string Shared secret key used to generate the hash. */ private string $secretKey; + /** + * @var string Hash algorithm for message authentication. + */ + private string $algorithm; + /** * @var int|null Number of seconds that the token is valid for. */ @@ -47,8 +50,8 @@ public function __construct( ?int $lifetime = null ) { $this->identityGenerator = $identityGenerator; - $this->mac = new Mac($algorithm); $this->secretKey = $secretKey; + $this->algorithm = $algorithm; $this->lifetime = $lifetime; } @@ -66,39 +69,43 @@ public function validate(string $token): bool return false; } - [$expiration, $identity] = $data; + [$expiration, $payload] = $data; - if ($expiration !== null && time() > $expiration) { + $hashLength = $this->getHashLength(); + $hash = StringHelper::byteSubstring($payload, 0, $hashLength); + $message = StringHelper::byteSubstring($payload, $hashLength, null); + + if (!hash_equals($hash, $this->generateHash($message))) { return false; } - return $identity === $this->identityGenerator->generate(); + if ($expiration !== null && time() > $expiration) { + return false; + } + return true; } private function generateToken(?int $expiration): string { - return StringHelper::base64UrlEncode( - $this->mac->sign( - (string) $expiration . '~' . $this->identityGenerator->generate(), - $this->secretKey, - true, - ), - ); + $message = (string) $expiration . '~' . Random::string(32); + + return StringHelper::base64UrlEncode($this->generateHash($message) . $message); } + /** + * @return array{0: int|null, 1: string}|null + */ private function extractData(string $token): ?array { - try { - $raw = $this->mac->getMessage( - StringHelper::base64UrlDecode($token), - $this->secretKey, - true, - ); - } catch (DataIsTamperedException $e) { + $payload = StringHelper::base64UrlDecode($token); + $hashLength = $this->getHashLength(); + + if (StringHelper::byteLength($payload) <= $hashLength) { return null; } - $chunks = explode('~', $raw, 2); + $message = StringHelper::byteSubstring($payload, $hashLength, null); + $chunks = explode('~', $message, 2); if (count($chunks) !== 2) { return null; } @@ -112,8 +119,22 @@ private function extractData(string $token): ?array } } - $identity = $chunks[1]; + return [$expiration, $payload]; + } - return [$expiration, $identity]; + private function generateHash(string $message): string + { + $identity = $this->identityGenerator->generate(); + $message = StringHelper::byteLength($identity) . '~' . $identity . '~' . $message; + $hash = hash_hmac($this->algorithm, $message, $this->secretKey, true); + if (!$hash) { + throw new RuntimeException("Failed to generate HMAC with hash algorithm: {$this->algorithm}."); + } + return $hash; + } + + private function getHashLength(): int + { + return StringHelper::byteLength($this->generateHash('')); } } diff --git a/tests/Hmac/HmacCsrfTokenTest.php b/tests/Hmac/HmacCsrfTokenTest.php index f25cafe..859a84a 100644 --- a/tests/Hmac/HmacCsrfTokenTest.php +++ b/tests/Hmac/HmacCsrfTokenTest.php @@ -38,6 +38,30 @@ public function testBase(): void $this->assertTrue($csrfToken->validate($token)); } + public function testTokenValueChanges(): void + { + $csrfToken = new HmacCsrfToken( + new MockCsrfTokenIdentityGenerator('user7'), + 'mySecretKey', + ); + + $this->assertNotSame($csrfToken->getValue(), $csrfToken->getValue()); + } + + public function testTokenDoesNotExposeIdentity(): void + { + $identity = 'session-id-that-must-not-be-in-token'; + $csrfToken = new HmacCsrfToken( + new MockCsrfTokenIdentityGenerator($identity), + 'mySecretKey', + ); + + $token = $csrfToken->getValue(); + + $this->assertStringNotContainsString($identity, StringHelper::base64UrlDecode($token)); + $this->assertTrue($csrfToken->validate($token)); + } + public function testExpiration(): void { self::$timeResult = 300;