diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c32f51..4c3a6bc 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 (@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..c4cf52a 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 signed token for authenticated-only forms when it is +acceptable for a submitted token to stay valid until it expires. ```mermaid flowchart TD @@ -157,20 +157,20 @@ flowchart TD C -- No --> S C -- Yes --> D{Token replay within lifetime OK?} D -- No --> S - D -- Yes --> H[HMAC] + D -- Yes --> H[HMAC signed] ``` Detailed comparison: -| Factor | Synchronizer | HMAC | +| Factor | Synchronizer | HMAC signed | |--------|--------------|------| | I/O per request | Session read and write | No token storage I/O | | 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: +To switch to HMAC signed token: ```php use Yiisoft\Csrf\CsrfTokenInterface; @@ -205,12 +205,11 @@ 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 its signature is bound to the current identity. The token is added to a form. When the form is submitted, we verify +the token signature, check that it belongs to the current identity, 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 +234,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..3e5d4d6 100644 --- a/src/Hmac/HmacCsrfToken.php +++ b/src/Hmac/HmacCsrfToken.php @@ -4,37 +4,44 @@ namespace Yiisoft\Csrf\Hmac; +use InvalidArgumentException; +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\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 an expiration timestamp and is signed with + * an identity-bound key. It is added to forms. When the form is submitted, we verify the token signature, check that it + * belongs to the current 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 Hash length in bytes. + */ + private int $hashLength; + /** * @var int|null Number of seconds that the token is valid for. */ @@ -47,8 +54,9 @@ public function __construct( ?int $lifetime = null ) { $this->identityGenerator = $identityGenerator; - $this->mac = new Mac($algorithm); $this->secretKey = $secretKey; + $this->algorithm = $algorithm; + $this->hashLength = $this->calcHashLength(); $this->lifetime = $lifetime; } @@ -66,54 +74,74 @@ public function validate(string $token): bool return false; } - [$expiration, $identity] = $data; + [$expiration, $payload] = $data; if ($expiration !== null && time() > $expiration) { return false; } - return $identity === $this->identityGenerator->generate(); + $hash = StringHelper::byteSubstring($payload, 0, $this->hashLength); + $message = StringHelper::byteSubstring($payload, $this->hashLength, null); + + if (!hash_equals($hash, $this->generateHash($message))) { + 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; + + 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); + } catch (InvalidArgumentException $e) { return null; } - $chunks = explode('~', $raw, 2); - if (count($chunks) !== 2) { + if (StringHelper::byteLength($payload) < $this->hashLength) { return null; } - if ($chunks[0] === '') { + $message = StringHelper::byteSubstring($payload, $this->hashLength, null); + if ($message === '') { $expiration = null; } else { - $expiration = (int) $chunks[0]; - if ((string) $expiration !== $chunks[0]) { + $expiration = (int) $message; + if ((string) $expiration !== $message) { return null; } } - $identity = $chunks[1]; + return [$expiration, $payload]; + } + + 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; + } - return [$expiration, $identity]; + private function calcHashLength(): int + { + $hash = hash_hmac($this->algorithm, '', '', true); + if (!$hash) { + throw new RuntimeException("Failed to generate HMAC with hash algorithm: {$this->algorithm}."); + } + return StringHelper::byteLength($hash); } } diff --git a/tests/Hmac/HmacCsrfTokenTest.php b/tests/Hmac/HmacCsrfTokenTest.php index f25cafe..d3f39ee 100644 --- a/tests/Hmac/HmacCsrfTokenTest.php +++ b/tests/Hmac/HmacCsrfTokenTest.php @@ -6,8 +6,8 @@ use PHPUnit\Framework\TestCase; use Yiisoft\Csrf\Hmac\HmacCsrfToken; +use Yiisoft\Csrf\Hmac\IdentityGenerator\CsrfTokenIdentityGeneratorInterface; use Yiisoft\Csrf\Tests\Hmac\IdentityGenerator\MockCsrfTokenIdentityGenerator; -use Yiisoft\Security\Mac; use Yiisoft\Security\Random; use Yiisoft\Strings\StringHelper; @@ -38,6 +38,37 @@ public function testBase(): void $this->assertTrue($csrfToken->validate($token)); } + 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 testTokenPayloadContainsExpiration(): void + { + self::$timeResult = 300; + + $csrfToken = new HmacCsrfToken( + new MockCsrfTokenIdentityGenerator('user7'), + 'mySecretKey', + 'sha256', + 100, + ); + + $payload = StringHelper::base64UrlDecode($csrfToken->getValue()); + $message = StringHelper::byteSubstring($payload, $this->getHashLength(), null); + + $this->assertSame('400', $message); + } + public function testExpiration(): void { self::$timeResult = 300; @@ -68,18 +99,77 @@ public function testIncorrectToken(): void ); $this->assertFalse($csrfToken->validate(Random::string())); + $this->assertFalse($csrfToken->validate('*')); - $token = StringHelper::base64UrlEncode( - (new Mac('sha256'))->sign('a2~user1', 'mySecretKey', true), - ); + $token = $this->createToken('user7', 'a2'); $this->assertFalse($csrfToken->validate($token)); - $token = StringHelper::base64UrlEncode( - (new Mac('sha256'))->sign('hello', 'mySecretKey', true), - ); + $token = $this->createToken('user7', 'hello'); $this->assertFalse($csrfToken->validate($token)); } + public function testValidatesTokenSignedWithCurrentIdentity(): void + { + self::$timeResult = 300; + + $csrfToken = new HmacCsrfToken( + new MockCsrfTokenIdentityGenerator('user7'), + 'mySecretKey', + ); + + $this->assertTrue($csrfToken->validate($this->createToken('user7', '500'))); + } + + public function testRejectsSignedTokenWithMalformedMessage(): void + { + self::$timeResult = 300; + + $csrfToken = new HmacCsrfToken( + new MockCsrfTokenIdentityGenerator('user7'), + 'mySecretKey', + ); + + $this->assertFalse($csrfToken->validate($this->createToken('user7', '0500'))); + $this->assertFalse($csrfToken->validate($this->createToken('user7', 'not-a-timestamp'))); + $this->assertFalse($csrfToken->validate($this->createToken('user7', '500~extra'))); + } + + public function testInvalidTokenParsingDoesNotGenerateIdentity(): void + { + $identityGenerator = new class implements CsrfTokenIdentityGeneratorInterface { + public int $calls = 0; + + public function generate(): string + { + $this->calls++; + return 'user7'; + } + }; + $csrfToken = new HmacCsrfToken($identityGenerator, 'mySecretKey'); + + $this->assertFalse($csrfToken->validate(StringHelper::base64UrlEncode('short'))); + $this->assertSame(0, $identityGenerator->calls); + } + + public function testExpiredTokenDoesNotGenerateIdentity(): void + { + self::$timeResult = 300; + + $identityGenerator = new class implements CsrfTokenIdentityGeneratorInterface { + public int $calls = 0; + + public function generate(): string + { + $this->calls++; + return 'user7'; + } + }; + $csrfToken = new HmacCsrfToken($identityGenerator, 'mySecretKey'); + + $this->assertFalse($csrfToken->validate($this->createToken('user7', '299'))); + $this->assertSame(0, $identityGenerator->calls); + } + public function testIdentityWithTilda(): void { $csrfToken = new HmacCsrfToken( @@ -91,6 +181,22 @@ public function testIdentityWithTilda(): void $this->assertTrue($csrfToken->validate($token)); } + + private function createToken(string $identity, string $message): string + { + $signedMessage = StringHelper::byteLength($identity) . '~' . $identity . '~' . $message; + $hash = hash_hmac('sha256', $signedMessage, 'mySecretKey', true); + if (!$hash) { + self::fail('Failed to generate HMAC.'); + } + + return StringHelper::base64UrlEncode($hash . $message); + } + + private function getHashLength(): int + { + return StringHelper::byteLength(hash_hmac('sha256', '', '', true)); + } } namespace Yiisoft\Csrf\Hmac;