Add check for same-origin requests for unsafe methods#3576
Open
takluyver wants to merge 2 commits intotornadoweb:masterfrom
Open
Add check for same-origin requests for unsafe methods#3576takluyver wants to merge 2 commits intotornadoweb:masterfrom
takluyver wants to merge 2 commits intotornadoweb:masterfrom
Conversation
Contributor
Author
|
Docs still TBD - I figured it was best to agree on how it should work, setting names etc. before writing that. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Simple XSRF protection based - primarily - on the Sec-Fetch-Site header, as described in #3495.
Sec-Fetch-Siteis pretty simple where it is used. The only question is whether to allowsame-site, i.e. a different subdomain or port within the same registrable domain. For now, the implementation disallows it, but this could be its own setting.The fallback, if there's no Sec-Fetch-Site header, is more complicated.
Sec-Fetch-Sitemight be missing if the request:Sec-Fetch-*are only sent for 'potentially trustworthy' URLs)curl, etc.Sec-Fetch-Site: noneto pass the check.In the fallback, we aim to compare the domain from which the request originated (
OriginorReferrerheader) with the domain we're serving. Origin & Referrer can both be missing - OWASP recommends blocking such requests, but I suspect this is overly cautious as a default in the framework. E.g. non-browser clients probably won't send either of these.Knowing what domain & port we're actually exposed on is also tricky. In simple scenarios, the
Hostheader gives this, but it can be replaced by proxies. Well behaved proxies can setX-Forwarded-Host, but they won't all be well-behaved, and if you use this, you have to work out how many layers ofX-Forwardedto trust. Probably the best approach is to tell the application explicitly what its domain should be.So I think the fallback should require a separate setting. I hope that the
Sec-Fetch-Sitecheck can be on by default in some future version, but the fallback probably can't be.