Repository navigation
Refuse cross-site state changes on the admin API - #15
Merged
Merged
Conversation
The admin API authenticates with HTTP Basic, and browsers attach Basic credentials to any request for that origin regardless of which page started it. That made every mutating route a target for a page on another website: POST /api/users/:id/reset needs no request body and no unusual method, so a plain cross-site form reaches it with the admin's credentials attached. Requests arriving with a foreign Origin, or with Sec-Fetch-Site: cross-site, are refused. A request with no Origin at all did not come from a browser page and is left alone, so curl and scripts keep working as they did -- this is a gate on browsers, not on the API. Origin is the precise check and Sec-Fetch-Site only rules requests out, because the latter names a site rather than an origin: refusing everything that was not same-origin would have broken a dashboard served from another subdomain, which Origin settles correctly on its own. Reads are untouched. A cross-origin GET changes nothing here, and there is no reason for it to start failing.
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configuration
📒 Files selected for processing: 3
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
The gap
The admin API authenticates with HTTP Basic, and browsers attach Basic credentials to any request for that origin regardless of which page started it. SameSite does not apply to Basic auth. So every mutating route was reachable from a page on any website.
The worst case needed no cleverness at all:
POST /api/users/:id/resettakes no request body and uses a simple method, so a plain cross-site<form>reaches it with the admin's credentials attached. Same for revoking a VLESS UUID or a Telegram secret.The fix
A middleware on the admin handler refuses state-changing requests carrying a foreign
Origin, orSec-Fetch-Site: cross-site. A request with noOriginat all did not come from a browser page and is left alone, socurland scripts keep working exactly as before — this is a gate on browsers, not a redesign of the API.Originis the precise check andSec-Fetch-Siteonly rules requests out. That split matters:Sec-Fetch-Sitenames a site, not an origin, so my first version — refusing anything that was notsame-origin— would have broken a dashboard served from a second subdomain.Originsettles that case correctly on its own. A test coverssame-siteexplicitly.Reads are untouched: a cross-origin
GETchanges nothing here, and there is no reason for it to start failing.Tests
Four, run against the real handler chain:
DELETEof a VLESS UUID is refused, and the client still exists afterwards — the request was refused, not partially honoured.Sec-Fetch-Site: cross-siteis refused even withOriginstripped.Sec-Fetch-Site: cross-site,Sec-Fetch-Mode: cors, formContent-Type, a foreignOrigin— never reaches the route.none, and no-Origin-at-all all still reach the route.gofmt/go vetclean, full suite green.Summary by CodeRabbit
Originremain allowed.