Skip to content

Add check for same-origin requests for unsafe methods#3576

Open
takluyver wants to merge 2 commits intotornadoweb:masterfrom
takluyver:sec-fetch
Open

Add check for same-origin requests for unsafe methods#3576
takluyver wants to merge 2 commits intotornadoweb:masterfrom
takluyver:sec-fetch

Conversation

@takluyver
Copy link
Contributor

Simple XSRF protection based - primarily - on the Sec-Fetch-Site header, as described in #3495.

Sec-Fetch-Site is pretty simple where it is used. The only question is whether to allow same-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-Site might be missing if the request:

  • is made over unencrypted HTTP and not to localhost (Sec-Fetch-* are only sent for 'potentially trustworthy' URLs)
  • comes from an old or niche browser which doesn't implement it (all major browsers have since ~2023)
  • comes from a program other than a browser - Python API clients, curl, etc.
    • New/updated code making HTTP requests can set Sec-Fetch-Site: none to pass the check.
  • comes via some kind of proxy which strips the header (OWASP says that "This kind of header stripping is problematic, but common").

In the fallback, we aim to compare the domain from which the request originated (Origin or Referrer header) 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 Host header gives this, but it can be replaced by proxies. Well behaved proxies can set X-Forwarded-Host, but they won't all be well-behaved, and if you use this, you have to work out how many layers of X-Forwarded to 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-Site check can be on by default in some future version, but the fallback probably can't be.

@takluyver
Copy link
Contributor Author

Docs still TBD - I figured it was best to agree on how it should work, setting names etc. before writing that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant