[13.x] Improve origin verification in PreventRequestForgery#59198
[13.x] Improve origin verification in PreventRequestForgery#59198SanderMuller wants to merge 1 commit into
Conversation
|
Thanks for the PR. I like the idea of a helpful error message on HTTP in originOnly mode. For the Origin fallback, major browsers have supported Sec-Fetch-Site for over 3 years now. If we ever completely remove the token fallback I could see adding it. Kinda on the fence here though, would be curious what the community thinks. I do have some questions about handling Sec-Fetch-Site: none as a safe value though. Forms on file:// pages and browser extensions seem like exactly the kind of thing you'd want CSRF protection against. If they're first-party you can always exclude the middleware for that route. Also, you reference an OWASP Fetch Metadata policy that treats |
|
I wouldn't take 3 years support as granted - we still have a project that needs to work on Firefox 37. |
You're right that the OWASP reference is perhaps a bit more open ended than how I listed it. The OWASP section you linked is indeed what I meant, together with the example in https://web.dev/articles/fetch-metadata#step_5_reject_all_other_requests_that_are_cross-site_and_not_navigational using
On the Origin fallback: I'm open to pulling it out if the community feels it's not needed given broad Sec-Fetch-Site support. In default mode, requests without the header already fall through to token validation, so the fallback is mainly useful for originOnly edge cases. I'm looking forward to hear how the community feels! |
Summary
This PR makes three focused improvements to the
PreventRequestForgerymiddleware shipped in #58400:Handle
Sec-Fetch-Site: noneas a safe value. The browser setsnonewhen there is no external origin involved (e.g., forms onfile://pages, browser extensions). This is a bugfix fororiginOnlymode, which incorrectly throwsOriginMismatchExceptionfor these requests. Go 1.25 and the OWASP Fetch Metadata policy both treatnoneas safe.Fall back to
Originheader whenSec-Fetch-Siteis absent. Clients that don't sendSec-Fetch-Site(plain HTTP contexts, older browsers, some WebViews) are forced through token validation even when theOriginheader already proves the request is same-origin. This adds a lightweightOriginvsHostcomparison using Symfony'sgetSchemeAndHttpHost(). In default mode, mismatches fall through totokensMatch(). InoriginOnlymode, a matchingOriginnow correctly passes instead of throwing.Helpful error message on HTTP in
originOnlymode. Browsers don't sendSec-Fetch-Siteover insecure connections. WhenoriginOnlyis enabled on plain HTTP, the exception now says "Origin verification requires a secure connection" instead of the generic "Origin mismatch", so developers know exactly what to fix.Changes
hasValidOrigin(): acceptsnone, falls back to Origin-vs-Host, improved error on HTTPoriginMatchesHost(): newprotectedmethod (6 lines), comparesOriginheader against$request->getSchemeAndHttpHost()Precedent
CrossOriginProtection: same three-tier strategy (Sec-Fetch-Site → Origin → allow non-browser)noneas safe, recommends Origin fallback