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..d25c9d2 100644 --- a/src/Hmac/HmacCsrfToken.php +++ b/src/Hmac/HmacCsrfToken.php @@ -6,35 +6,36 @@ use Yiisoft\Csrf\CsrfTokenInterface; use Yiisoft\Csrf\Hmac\IdentityGenerator\CsrfTokenIdentityGeneratorInterface; +use Yiisoft\Csrf\MaskedCsrfToken; use Yiisoft\Security\DataIsTamperedException; use Yiisoft\Security\Mac; use Yiisoft\Strings\StringHelper; -use Yiisoft\Csrf\MaskedCsrfToken; - -use function count; /** - * 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. + * @var string Shared secret key used to sign the token. */ private string $secretKey; + /** + * @var int Hash length in bytes. + */ + private int $hashLength; + /** * @var int|null Number of seconds that the token is valid for. */ @@ -49,6 +50,7 @@ public function __construct( $this->identityGenerator = $identityGenerator; $this->mac = new Mac($algorithm); $this->secretKey = $secretKey; + $this->hashLength = $this->calcHashLength(); $this->lifetime = $lifetime; } @@ -61,59 +63,61 @@ public function getValue(): string public function validate(string $token): bool { - $data = $this->extractData($token); - if ($data === null) { + $raw = $this->decode($token); + + $message = $this->extractMessage($raw); + if ($message === null) { return false; } - [$expiration, $identity] = $data; + if ($message !== '') { + $expiration = (int) $message; + if ((string) $expiration !== $message || time() > $expiration) { + return false; + } + } - if ($expiration !== null && time() > $expiration) { + try { + $this->mac->getMessage($raw, $this->generateActualSecretKey(), true); + return true; + } catch (DataIsTamperedException $e) { return false; } - - return $identity === $this->identityGenerator->generate(); } private function generateToken(?int $expiration): string { return StringHelper::base64UrlEncode( $this->mac->sign( - (string) $expiration . '~' . $this->identityGenerator->generate(), - $this->secretKey, + (string) $expiration, + $this->generateActualSecretKey(), true, ), ); } - private function extractData(string $token): ?array + private function decode(string $token): string { - try { - $raw = $this->mac->getMessage( - StringHelper::base64UrlDecode($token), - $this->secretKey, - true, - ); - } catch (DataIsTamperedException $e) { - return null; - } + return StringHelper::base64UrlDecode($token); + } - $chunks = explode('~', $raw, 2); - if (count($chunks) !== 2) { + private function extractMessage(string $raw): ?string + { + if (StringHelper::byteLength($raw) < $this->hashLength) { return null; } - if ($chunks[0] === '') { - $expiration = null; - } else { - $expiration = (int) $chunks[0]; - if ((string) $expiration !== $chunks[0]) { - return null; - } - } + return StringHelper::byteSubstring($raw, $this->hashLength, null); + } - $identity = $chunks[1]; + private function generateActualSecretKey(): string + { + $identity = $this->identityGenerator->generate(); + return $this->secretKey . '~' . $identity; + } - return [$expiration, $identity]; + private function calcHashLength(): int + { + return StringHelper::byteLength($this->mac->sign('', '', true)); } } diff --git a/tests/Hmac/HmacCsrfTokenTest.php b/tests/Hmac/HmacCsrfTokenTest.php index f25cafe..6abf35f 100644 --- a/tests/Hmac/HmacCsrfTokenTest.php +++ b/tests/Hmac/HmacCsrfTokenTest.php @@ -6,6 +6,7 @@ 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; @@ -38,6 +39,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 +100,89 @@ 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 testRejectsTokenSignedWithDifferentIdentity(): void + { + self::$timeResult = 300; + + $csrfToken = new HmacCsrfToken( + new MockCsrfTokenIdentityGenerator('user7'), + 'mySecretKey', + ); + + $this->assertFalse($csrfToken->validate($this->createToken('user8', '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 +194,16 @@ public function testIdentityWithTilda(): void $this->assertTrue($csrfToken->validate($token)); } + + private function createToken(string $identity, string $message): string + { + return StringHelper::base64UrlEncode((new Mac())->sign($message, 'mySecretKey~' . $identity, true)); + } + + private function getHashLength(): int + { + return StringHelper::byteLength((new Mac())->sign('', '', true)); + } } namespace Yiisoft\Csrf\Hmac;