diff --git a/public/index.php b/public/index.php index 4a5b104..ee886f8 100644 --- a/public/index.php +++ b/public/index.php @@ -43,16 +43,28 @@ if ($secret === false || $secret === '') { } } -// 2. Ensure the __csrf cookie is set. If absent, issue a fresh token. -// The double-submit pattern stores the raw token in a separate readable -// cookie so client code (meta tag, JS) can echo it back as the -// X-CSRF-Token header. The HMAC cookie is the verification oracle; -// the raw-token cookie rides parallel and is readable by document.cookie -// or the equivalent server-side read. +// 2. Ensure the __csrf cookie is set. If absent OR stale, issue a fresh +// token. A cookie pair is "stale" when both cookies are present but +// the HMAC (__csrf) does not match the raw token (__csrf_token) under +// the current secret. This can happen when a browser session predates +// a server restart with a fresh secret.key: the browser still has the +// old __csrf cookie, the new server's secret does not match it, and +// every CSRF check fails. By re-minting on first request, we recover +// the session without requiring the user to clear cookies manually. +// The double-submit pattern stores the raw token in a separate +// readable cookie so client code (meta tag, JS) can echo it back as +// the X-CSRF-Token header. The HMAC cookie is the verification +// oracle; the raw-token cookie rides parallel and is readable by +// document.cookie or the equivalent server-side read. $cookies = $_COOKIE; $existingCookie = (string) ($cookies['__csrf'] ?? ''); $existingRawToken = (string) ($cookies['__csrf_token'] ?? ''); -if ($existingCookie === '' || $existingRawToken === '') { +$pairIsFresh = false; +if ($existingCookie !== '' && $existingRawToken !== '') { + $expectedHmac = hash_hmac('sha256', $existingRawToken, $secret); + $pairIsFresh = hash_equals($expectedHmac, $existingCookie); +} +if ($existingCookie === '' || $existingRawToken === '' || !$pairIsFresh) { [$token, $cookie] = CsrfToken::issue($secret); setcookie('__csrf', $cookie, [ 'expires' => time() + 86400, diff --git a/tests/Integration/StaleCookieRotationTest.php b/tests/Integration/StaleCookieRotationTest.php new file mode 100644 index 0000000..e4a106e --- /dev/null +++ b/tests/Integration/StaleCookieRotationTest.php @@ -0,0 +1,175 @@ +primeGet('/'); + $bodyA = $this->runFrontController(); + self::assertSame(200, $this->lastStatus); + $tokenA = $this->extractMetaToken($bodyA); + + // 2. Now simulate a server restart with SECRET_B. The browser + // still has the old cookies (minted under SECRET_A). We capture + // them from the first request's response by re-running a request + // with the same env: but since we cannot read Set-Cookie headers + // from a captured PHP test, we re-derive them using the same + // secret + the raw token the controller issued. + $cookieA = hash_hmac('sha256', $tokenA, self::SECRET_A); + + // 3. Send a second request with the stale cookies but SECRET_B + // active. The front controller should detect the mismatch and + // re-mint a fresh pair. + putenv('BATTLEFORGE_SECRET=' . self::SECRET_B); + $_COOKIE['__csrf'] = $cookieA; + $_COOKIE['__csrf_token'] = $tokenA; + $this->primeGet('/'); + $bodyB = $this->runFrontController(); + + // 4. The response should now carry a meta tag with a different + // raw token, and that token should HMAC under SECRET_B. + $tokenB = $this->extractMetaToken($bodyB); + self::assertNotSame($tokenA, $tokenB, 'raw token should be re-minted under SECRET_B'); + + $hmacUnderB = hash_hmac('sha256', $tokenB, self::SECRET_B); + self::assertSame(64, strlen($hmacUnderB), 'HMAC under SECRET_B should be 64 hex chars'); + } + + public function testValidCookiesAreNotReIssuedOnNextRequest(): void + { + // Mint a fresh pair under SECRET_A and capture both the raw token + // and the corresponding HMAC. The test sends these as the browser's + // cookie jar on the second request; the controller should recognise + // them as fresh and re-use the existing pair (no re-mint). + putenv('BATTLEFORGE_SECRET=' . self::SECRET_A); + [$tokenA, $cookieA] = CsrfToken::issue(self::SECRET_A); + + // 1. First request: no cookies, controller mints a fresh pair. + $this->primeGet('/'); + $bodyA = $this->runFrontController(); + self::assertSame(200, $this->lastStatus); + $tokenAController = $this->extractMetaToken($bodyA); + $cookieAController = hash_hmac('sha256', $tokenAController, self::SECRET_A); + + // 2. Second request: pre-mint the cookies that match the test's + // expectation. The controller should recognise the pair is fresh + // and re-use it (meta tag carries the same token, no re-mint). + $_COOKIE['__csrf'] = $cookieA; + $_COOKIE['__csrf_token'] = $tokenA; + $this->primeGet('/'); + $bodyB = $this->runFrontController(); + $tokenB = $this->extractMetaToken($bodyB); + + // Both bodies have valid tokens; the test's "no re-mint" claim is + // that $tokenA (the test's mint) is the same as $tokenB (the + // controller's reuse). The controller's mint and the test's mint + // are different only when the controller re-mints, so the + // identity proves the controller re-used the existing pair. + self::assertSame($tokenA, $tokenB, 'controller should re-use the existing token when cookies are valid'); + } + + public function testMissingTokenCookieIsFilledInOnNextRequest(): void + { + // 1. Boot under SECRET_A so the controller mints a pair. + putenv('BATTLEFORGE_SECRET=' . self::SECRET_A); + $this->primeGet('/'); + $bodyA = $this->runFrontController(); + $tokenA = $this->extractMetaToken($bodyA); + + // 2. Browser has the __csrf cookie but is missing __csrf_token + // (a partial-clear scenario, e.g. user cleared just one). + putenv('BATTLEFORGE_SECRET=' . self::SECRET_B); + $_COOKIE['__csrf'] = hash_hmac('sha256', $tokenA, self::SECRET_A); + // __csrf_token is intentionally missing. + $this->primeGet('/'); + $bodyB = $this->runFrontController(); + $tokenB = $this->extractMetaToken($bodyB); + self::assertNotSame($tokenA, $tokenB, 'missing __csrf_token should trigger re-mint'); + } + + private function primeGet(string $path): void + { + $_SERVER['REQUEST_METHOD'] = 'GET'; + $_SERVER['REQUEST_URI'] = $path; + $_SERVER['CONTENT_TYPE'] = null; + } + + private function extractMetaToken(string $body): string + { + $regex = '//'; + if (!preg_match($regex, $body, $m)) { + self::fail('response body did not contain a 64-char hex ; first 500 chars: ' . substr($body, 0, 500)); + } + return $m[1]; + } + + private function runFrontController(): string + { + $this->lastStatus = 0; + $body = $this->captureOutput(function (): void { + $this->lastStatus = (require __DIR__ . '/../../public/index.php') ?? http_response_code(); + }); + return $body; + } + + /** @param callable(): void $fn */ + private function captureOutput(callable $fn): string + { + ob_start(); + try { + $fn(); + } finally { + $output = (string) ob_get_clean(); + } + return $output; + } +}