diff --git a/app/Config/Security.php b/app/Config/Security.php index 635f8b77b9b7..d596174a3c78 100644 --- a/app/Config/Security.php +++ b/app/Config/Security.php @@ -17,6 +17,25 @@ class Security extends BaseConfig */ public string $csrfProtection = 'cookie'; + /** + * -------------------------------------------------------------------------- + * CSRF Fetch Metadata + * -------------------------------------------------------------------------- + * + * Whether to use Fetch Metadata request headers as a first-line CSRF check. + */ + public bool $csrfUseFetchMetadata = true; + + /** + * -------------------------------------------------------------------------- + * CSRF Allow Same Site + * -------------------------------------------------------------------------- + * + * Whether requests with the Sec-Fetch-Site: same-site header should pass + * Fetch Metadata verification. + */ + public bool $csrfAllowSameSite = false; + /** * -------------------------------------------------------------------------- * CSRF Token Randomization diff --git a/system/Filters/CSRF.php b/system/Filters/CSRF.php index ea9a9939f2de..e4a2035bec82 100644 --- a/system/Filters/CSRF.php +++ b/system/Filters/CSRF.php @@ -14,11 +14,13 @@ namespace CodeIgniter\Filters; use CodeIgniter\HTTP\IncomingRequest; +use CodeIgniter\HTTP\Method; use CodeIgniter\HTTP\RedirectResponse; use CodeIgniter\HTTP\RequestInterface; use CodeIgniter\HTTP\ResponseInterface; use CodeIgniter\Security\Exceptions\SecurityException; use CodeIgniter\Security\Security; +use Config\Security as SecurityConfig; /** * CSRF filter. @@ -52,12 +54,19 @@ public function before(RequestInterface $request, $arguments = null) $security->verify($request); } catch (SecurityException $e) { if ($security->shouldRedirect() && ! $request->isAJAX()) { - return redirect()->back()->with('error', $e->getMessage()); + $response = redirect()->back()->with('error', $e->getMessage()); + $this->addFetchMetadataVaryHeader($request, $response); + + return $response; } + $this->addFetchMetadataVaryHeader($request, service('response')); + throw $e; } + $this->addFetchMetadataVaryHeader($request, service('response')); + return null; } @@ -70,4 +79,15 @@ public function after(RequestInterface $request, ResponseInterface $response, $a { return null; } + + private function addFetchMetadataVaryHeader(IncomingRequest $request, ResponseInterface $response): void + { + $config = config(SecurityConfig::class); + $useFetchMetadata = ($config->csrfUseFetchMetadata ?? false) === true; // @phpstan-ignore nullCoalesce.property + $isUnsafeMethod = in_array($request->getMethod(), [Method::POST, Method::PUT, Method::DELETE, Method::PATCH], true); + + if ($useFetchMetadata && $isUnsafeMethod) { + $response->appendHeader('Vary', 'Sec-Fetch-Site'); + } + } } diff --git a/system/Security/Security.php b/system/Security/Security.php index c1bfb856995a..9a2dff36e795 100644 --- a/system/Security/Security.php +++ b/system/Security/Security.php @@ -35,8 +35,11 @@ */ class Security implements SecurityInterface { - public const CSRF_PROTECTION_COOKIE = 'cookie'; - public const CSRF_PROTECTION_SESSION = 'session'; + public const CSRF_PROTECTION_COOKIE = 'cookie'; + public const CSRF_PROTECTION_SESSION = 'session'; + private const FETCH_METADATA_ALLOW = 'allow'; + private const FETCH_METADATA_FALLBACK = 'fallback'; + private const FETCH_METADATA_REJECT = 'reject'; /** * CSRF hash length in bytes. @@ -118,6 +121,27 @@ public function verify(RequestInterface $request): static assert($request instanceof IncomingRequest); + $decision = $this->fetchMetadataDecision($request); + + if ($decision === self::FETCH_METADATA_ALLOW) { + $this->removeTokenInRequest($request); + + log_message('info', 'CSRF Fetch Metadata verified.'); + + return $this; + } + + if ($decision === self::FETCH_METADATA_REJECT) { + throw SecurityException::forDisallowedAction(); + } + + $this->verifyToken($request); + + return $this; + } + + private function verifyToken(IncomingRequest $request): void + { $postedToken = $this->getPostedToken($request); try { @@ -139,8 +163,6 @@ public function verify(RequestInterface $request): static } log_message('info', 'CSRF token verified.'); - - return $this; } public function getHash(): ?string @@ -229,6 +251,34 @@ private function isCsrfCookie(): bool return $this->config->csrfProtection === self::CSRF_PROTECTION_COOKIE; } + /** + * @return self::FETCH_METADATA_* + */ + private function fetchMetadataDecision(IncomingRequest $request): string + { + if (! ($this->config->csrfUseFetchMetadata ?? false)) { // @phpstan-ignore nullCoalesce.initializedProperty + return self::FETCH_METADATA_FALLBACK; + } + + $fetchSite = strtolower($request->getHeaderLine('Sec-Fetch-Site')); + + if ($fetchSite === 'same-origin') { + return self::FETCH_METADATA_ALLOW; + } + + if ($fetchSite === 'cross-site') { + return self::FETCH_METADATA_REJECT; + } + + if ($fetchSite === 'same-site') { + return $this->config->csrfAllowSameSite ?? false // @phpstan-ignore nullCoalesce.initializedProperty + ? self::FETCH_METADATA_ALLOW + : self::FETCH_METADATA_REJECT; + } + + return self::FETCH_METADATA_FALLBACK; + } + /** * @phpstan-assert SessionInterface $this->session */ @@ -285,6 +335,10 @@ private function removeTokenInRequest(IncomingRequest $request): void // If the token is found in form-encoded data, we can safely remove it. parse_str($body, $result); + if (! array_key_exists($tokenName, $result)) { + return; + } + unset($result[$tokenName]); $request->setBody(http_build_query($result)); } diff --git a/tests/system/Filters/CSRFTest.php b/tests/system/Filters/CSRFTest.php index ce9d3fdfe263..42b3e7bde2a1 100644 --- a/tests/system/Filters/CSRFTest.php +++ b/tests/system/Filters/CSRFTest.php @@ -13,10 +13,14 @@ namespace CodeIgniter\Filters; +use CodeIgniter\Config\Factories; use CodeIgniter\HTTP\CLIRequest; use CodeIgniter\HTTP\IncomingRequest; +use CodeIgniter\HTTP\RedirectResponse; use CodeIgniter\HTTP\Response; +use CodeIgniter\Security\Exceptions\SecurityException; use CodeIgniter\Test\CIUnitTestCase; +use Config\Security as SecurityConfig; use PHPUnit\Framework\Attributes\BackupGlobals; use PHPUnit\Framework\Attributes\Group; @@ -37,6 +41,14 @@ protected function setUp(): void $this->config = new \Config\Filters(); } + protected function tearDown(): void + { + parent::tearDown(); + + $this->resetServices(); + Factories::reset('config'); + } + public function testDoNotCheckCliRequest(): void { $this->config->globals = [ @@ -73,4 +85,83 @@ public function testPassGetRequest(): void // GET request is not protected, so no SecurityException will be thrown. $this->assertSame($this->request, $request); } + + public function testBeforeAddsVaryHeaderForFetchMetadataVerification(): void + { + $filter = new CSRF(); + $request = single_service('incomingrequest', null) + ->withMethod('POST') + ->setHeader('Sec-Fetch-Site', 'same-origin'); + + $filter->before($request); + + $this->assertSame('Sec-Fetch-Site', service('response')->getHeaderLine('Vary')); + } + + public function testBeforeAppendsVaryHeaderForFetchMetadataVerification(): void + { + $filter = new CSRF(); + $request = single_service('incomingrequest', null) + ->withMethod('POST') + ->setHeader('Sec-Fetch-Site', 'same-origin'); + service('response')->setHeader('Vary', 'Accept-Language'); + + $filter->before($request); + + $this->assertSame('Accept-Language, Sec-Fetch-Site', service('response')->getHeaderLine('Vary')); + } + + public function testBeforeAddsVaryHeaderToRedirectResponseForFetchMetadataVerification(): void + { + $config = new SecurityConfig(); + $config->redirect = true; + Factories::injectMock('config', 'Security', $config); + + $filter = new CSRF(); + $request = single_service('incomingrequest', null) + ->withMethod('POST') + ->setHeader('Sec-Fetch-Site', 'cross-site'); + + $response = $filter->before($request); + + $this->assertInstanceOf(RedirectResponse::class, $response); + $this->assertSame('Sec-Fetch-Site', $response->getHeaderLine('Vary')); + } + + public function testBeforeThrowsExceptionForRejectedFetchMetadataVerification(): void + { + $filter = new CSRF(); + $request = single_service('incomingrequest', null) + ->withMethod('POST') + ->setHeader('Sec-Fetch-Site', 'cross-site'); + + try { + $filter->before($request); + + $this->fail('Expected SecurityException was not thrown.'); + } catch (SecurityException) { + $this->assertSame('Sec-Fetch-Site', service('response')->getHeaderLine('Vary')); + } + } + + public function testBeforeDoesNotAddVaryHeaderForTokenVerification(): void + { + service('superglobals') + ->setServer('REQUEST_METHOD', 'POST') + ->setPost('csrf_test_name', '8b9218a55906f9dcc1dc263dce7f005a') + ->setCookie('csrf_cookie_name', '8b9218a55906f9dcc1dc263dce7f005a'); + + $config = new SecurityConfig(); + $config->csrfUseFetchMetadata = false; + Factories::injectMock('config', 'Security', $config); + + $filter = new CSRF(); + $request = single_service('incomingrequest', null) + ->withMethod('POST') + ->setHeader('Sec-Fetch-Site', 'same-origin'); + + $filter->before($request); + + $this->assertSame('', service('response')->getHeaderLine('Vary')); + } } diff --git a/tests/system/Security/SecurityTest.php b/tests/system/Security/SecurityTest.php index bc3f13148f2f..f20488d27286 100644 --- a/tests/system/Security/SecurityTest.php +++ b/tests/system/Security/SecurityTest.php @@ -237,6 +237,154 @@ public function testCsrfVerifyHeaderWithJsonBodyStripsTokenFromBody(): void $this->assertSame('{"foo":"bar"}', $request->getBody()); } + public function testCsrfVerifyFetchMetadataSameOriginReturnsSelf(): void + { + service('superglobals')->setServer('REQUEST_METHOD', 'POST'); + + $security = $this->createMockSecurity(); + $request = $this->createIncomingRequest()->setHeader('Sec-Fetch-Site', 'same-origin'); + + $this->assertInstanceOf(Security::class, $security->verify($request)); + $this->assertLogged('info', 'CSRF Fetch Metadata verified.'); + } + + public function testCsrfVerifyFetchMetadataRemovesTokenButDoesNotRegenerate(): void + { + service('superglobals') + ->setServer('REQUEST_METHOD', 'POST') + ->setPost('foo', 'bar') + ->setPost('csrf_test_name', self::CORRECT_CSRF_HASH) + ->setCookie('csrf_cookie_name', self::CORRECT_CSRF_HASH); + + $security = $this->createMockSecurity(); + $request = $this->createIncomingRequest()->setHeader('Sec-Fetch-Site', 'same-origin'); + + $oldHash = $security->getHash(); + $security->verify($request); + $newHash = $security->getHash(); + + $this->assertSame($oldHash, $newHash); + $this->assertSame(['foo' => 'bar'], service('superglobals')->getPostArray()); + } + + public function testCsrfVerifyFetchMetadataPreservesRawBodyWithoutToken(): void + { + service('superglobals')->setServer('REQUEST_METHOD', 'POST'); + + $security = $this->createMockSecurity(); + $request = $this->createIncomingRequest() + ->setHeader('Sec-Fetch-Site', 'same-origin') + ->setBody('unchanged'); + + $security->verify($request); + + $this->assertSame('unchanged', $request->getBody()); + } + + public function testCsrfVerifyFetchMetadataSameSiteThrowsExceptionByDefault(): void + { + service('superglobals') + ->setServer('REQUEST_METHOD', 'POST') + ->setPost('csrf_test_name', self::CORRECT_CSRF_HASH) + ->setCookie('csrf_cookie_name', self::CORRECT_CSRF_HASH); + + $security = $this->createMockSecurity(); + $request = $this->createIncomingRequest()->setHeader('Sec-Fetch-Site', 'same-site'); + + $this->expectException(SecurityException::class); + $security->verify($request); + } + + public function testCsrfVerifyFetchMetadataSameSiteReturnsSelfWhenAllowed(): void + { + service('superglobals')->setServer('REQUEST_METHOD', 'POST'); + + $config = new SecurityConfig(); + $config->csrfAllowSameSite = true; + $security = $this->createMockSecurity($config); + $request = $this->createIncomingRequest()->setHeader('Sec-Fetch-Site', 'same-site'); + + $this->assertInstanceOf(Security::class, $security->verify($request)); + } + + public function testCsrfVerifyFetchMetadataCrossSiteThrowsExceptionEvenWithValidToken(): void + { + service('superglobals') + ->setServer('REQUEST_METHOD', 'POST') + ->setPost('csrf_test_name', self::CORRECT_CSRF_HASH) + ->setCookie('csrf_cookie_name', self::CORRECT_CSRF_HASH); + + $security = $this->createMockSecurity(); + $request = $this->createIncomingRequest()->setHeader('Sec-Fetch-Site', 'cross-site'); + + $this->expectException(SecurityException::class); + $security->verify($request); + } + + #[DataProvider('provideCsrfVerifyFetchMetadataFallsBackToToken')] + public function testCsrfVerifyFetchMetadataFallsBackToToken(?string $fetchSite): void + { + service('superglobals') + ->setServer('REQUEST_METHOD', 'POST') + ->setPost('csrf_test_name', self::CORRECT_CSRF_HASH) + ->setCookie('csrf_cookie_name', self::CORRECT_CSRF_HASH); + + $security = $this->createMockSecurity(); + $request = $this->createIncomingRequest(); + + if ($fetchSite !== null) { + $request->setHeader('Sec-Fetch-Site', $fetchSite); + } + + $this->assertInstanceOf(Security::class, $security->verify($request)); + $this->assertLogged('info', 'CSRF token verified.'); + } + + /** + * @return iterable + */ + public static function provideCsrfVerifyFetchMetadataFallsBackToToken(): iterable + { + yield 'missing' => [null]; + + yield 'none' => ['none']; + + yield 'unknown' => ['future-value']; + } + + public function testCsrfVerifyTokenOnlyIgnoresFetchMetadata(): void + { + service('superglobals') + ->setServer('REQUEST_METHOD', 'POST') + ->setPost('csrf_test_name', self::CORRECT_CSRF_HASH) + ->setCookie('csrf_cookie_name', self::CORRECT_CSRF_HASH); + + $config = new SecurityConfig(); + $config->csrfUseFetchMetadata = false; + $security = $this->createMockSecurity($config); + $request = $this->createIncomingRequest()->setHeader('Sec-Fetch-Site', 'cross-site'); + + $this->assertInstanceOf(Security::class, $security->verify($request)); + $this->assertLogged('info', 'CSRF token verified.'); + } + + public function testCsrfVerifyMissingFetchMetadataConfigFallsBackToTokenOnly(): void + { + service('superglobals') + ->setServer('REQUEST_METHOD', 'POST') + ->setPost('csrf_test_name', self::CORRECT_CSRF_HASH) + ->setCookie('csrf_cookie_name', self::CORRECT_CSRF_HASH); + + $config = new SecurityConfig(); + unset($config->csrfUseFetchMetadata); + + $security = $this->createMockSecurity($config); + $request = $this->createIncomingRequest()->setHeader('Sec-Fetch-Site', 'cross-site'); + + $this->assertInstanceOf(Security::class, $security->verify($request)); + $this->assertLogged('info', 'CSRF token verified.'); + } + public function testCsrfVerifyPutBodyThrowsExceptionOnNoMatch(): void { service('superglobals') diff --git a/user_guide_src/source/changelogs/v4.8.0.rst b/user_guide_src/source/changelogs/v4.8.0.rst index 7670598cde1f..90d6c9543f2a 100644 --- a/user_guide_src/source/changelogs/v4.8.0.rst +++ b/user_guide_src/source/changelogs/v4.8.0.rst @@ -248,6 +248,7 @@ Libraries - **Locks:** Added :doc:`Atomic Locks ` for owner-aware, cross-process mutual exclusion backed by supported cache handlers: **File**, **Redis**, **Predis**, and **Memcached**. Memcached support has driver-specific release limitations because Memcached has no atomic compare-and-delete command. - **Logging:** Log handlers now receive the full context array as a third argument to ``handle()``. When ``$logGlobalContext`` is enabled, the CI global context is available under the ``HandlerInterface::GLOBAL_CONTEXT_KEY`` key. Built-in handlers append it to the log output; custom handlers can use it for structured logging. - **Logging:** Added :ref:`per-call context logging ` with three new ``Config\Logger`` options (``$logContext``, ``$logContextTrace``, ``$logContextUsedKeys``). Per PSR-3, a ``Throwable`` in the ``exception`` context key is automatically normalized to a meaningful array. All options default to ``false``. +- **Security:** Added :ref:`Fetch Metadata based CSRF protection ` with token fallback. Helpers and Functions ===================== diff --git a/user_guide_src/source/installation/upgrade_480.rst b/user_guide_src/source/installation/upgrade_480.rst index d2738db27158..de5d728c0e2b 100644 --- a/user_guide_src/source/installation/upgrade_480.rst +++ b/user_guide_src/source/installation/upgrade_480.rst @@ -75,6 +75,8 @@ Config - app/Config/Mimes.php - ``Config\Mimes::$mimes`` added a new key ``md`` for Markdown files. +- app/Config/Security.php + - ``Config\Security::$csrfUseFetchMetadata`` and ``Config\Security::$csrfAllowSameSite`` were added for Fetch Metadata based CSRF protection. All Changes =========== diff --git a/user_guide_src/source/libraries/security.rst b/user_guide_src/source/libraries/security.rst index 8fc49d646b1a..1252e0f2c7f9 100644 --- a/user_guide_src/source/libraries/security.rst +++ b/user_guide_src/source/libraries/security.rst @@ -89,6 +89,47 @@ You can set to use the Session based CSRF protection by editing the following co .. literalinclude:: security/002.php +.. _csrf-fetch-metadata: + +Fetch Metadata CSRF Protection +------------------------------ + +.. versionadded:: 4.8.0 + +CodeIgniter can use Fetch Metadata request headers as a first-line CSRF check for unsafe browser requests. +Fetch Metadata is a browser-supplied signal that helps the framework allow same-origin requests and reject +cross-site requests before checking the CSRF token. + +When CSRF protection is enabled, new applications use Fetch Metadata first by default. You can configure +this behavior in **app/Config/Security.php**: + +.. literalinclude:: security/011.php + +When it is enabled, unsafe requests with ``Sec-Fetch-Site: same-origin`` are allowed without a token. +Requests with ``Sec-Fetch-Site: cross-site`` are rejected. Requests with a missing ``Sec-Fetch-Site`` header, +``Sec-Fetch-Site: none``, or an unknown value fall back to token verification. This keeps protection working +for browsers or clients that do not send Fetch Metadata headers. Requests with ``Sec-Fetch-Site: same-site`` +are rejected unless same-site requests are explicitly allowed. + +Upgraded applications without this config value continue to use token verification. You may also disable +Fetch Metadata protection by setting ``$csrfUseFetchMetadata`` to ``false``. + +When an unsafe request passes with Fetch Metadata, the CSRF token is not regenerated. Token regeneration only +runs when token verification is used. + +.. warning:: Fetch Metadata protects browser-based requests. Browsers only send these headers for + potentially trustworthy URLs, and non-browser clients can send their own headers. It is not API + authentication. + +.. warning:: Same-site is not the same as same-origin, because sibling subdomains are considered same-site. + Same-site requests are rejected by default. If all same-site origins are trusted, you may allow them + in **app/Config/Security.php**: + + .. literalinclude:: security/012.php + +If your application receives legitimate cross-origin POST requests, such as webhooks, callbacks, or +OAuth/SAML responses, use :ref:`CSRF route exclusions ` and verify those routes another way. + Token Randomization ------------------- @@ -162,6 +203,8 @@ and enabling the `csrf` filter globally: .. literalinclude:: security/006.php +.. _csrf-exclude-uris: + Select URIs can be whitelisted from CSRF protection (for example API endpoints expecting externally POSTed content). You can add these URIs by adding them as exceptions in the filter: diff --git a/user_guide_src/source/libraries/security/011.php b/user_guide_src/source/libraries/security/011.php new file mode 100644 index 000000000000..9718501446f7 --- /dev/null +++ b/user_guide_src/source/libraries/security/011.php @@ -0,0 +1,12 @@ +