proposal: net/http: add CrossOriginForgeryHandler #73626
Comments
|
I take it the api looks like this? func CrossOriginForgeryHandler(next http.Handler) http.Handler |
|
I've added a concrete API (oops). It's a struct to accomodate the |
gabyhelp
added
the
LibraryProposal
|
Added a section surveying existing implementations, which helps motivate why we need a standard library solution. |
|
It would be nice if ErrorHandler got the error so it could be logged. |
gorilla/csrf has Also note that ErrorHandler has access to r.Headers, so if you're curious about % of requests without Sec-Fetch-Site, it's arguably easier to look at the headers than it is to look at error logs. |
|
I was thinking ErrorHandler could be |
|
Could just have a func that returns nil or an error. Just need to ensure it is idempotent. func CheckCrossOrigin(w http.ResponseWriter, r *http.Request) error So if need to care in the ErrorHandler just call the func again. |
adapted from golang/go#73626
|
Thanks for putting this proposal together, @FiloSottile! There's this widespread misconception that the changes to the Below are some general comments about your Background section; as this comment was getting quite long, I'll post another comment about the proposal itself.
It's worth clarifying that CSRF goes beyond cookies. The attacker may abuse the victim's position within a less public network in order to target a server directly inaccessible to the attacker. Private Network Access is a mitigation against this category of CSRF attacks, but it's not fully implemented in all major browsers.
Thanks for stressing this crucial, yet often overlooked, distinction. Shameless plug, but I wrote a blog post on this topic a few years ago.
In the modern sense of same-site (i.e. schemelessly same-site), which all major browsers understand, a request from
Another shameless plug: for readers interested in an example of cookie tossing (which I reported years ago and, last time I checked, still had not been fixed), see this video.
For an example of exactly this, see this post.
Close, but not quite: some HEAD requests are simple, but HTML forms can't send HEAD requests.
Not if "same-site" is understood as "schemelessly same-site"; see my comment further up.
Citation needed 😄
FWIW, I've had a similar experience. I think it's fair to consider that library unmaintained. |
Hyphenate "cross site". 😇
Two minor problems with this passage of the doc comment:
I suggest // The middleware adds a Vary header with the value "Sec-Fetch-Site, Origin" to ensure
// that caching intermediaries do not get poisoned.
Perhaps a good opportunity for the doc to remind server developers of the concept of safe methods.
One disadvantage of this approach is that you cannot reuse a
In this connection, see
Maybe it's just me, but I feel that those fields would be better left non-exported and the type should be instantiated via some factory function / "constructor": type CrossOriginForgeryMiddleware struct {
// contains filtered or unexported fields
}
func NewCrossOriginForgeryMiddleware(errorHandler Handler, bypassOrigins []string) (*CrossOriginForgeryMiddleware, error) {
// ...
}This approach presents several advantages:
// UnsafeAllowCrossOrigin disables [CrossOriginForgeryHandler] for the request.
// It is generally only useful when implementing single sign-on (SSO) flows.
func UnsafeAllowCrossOrigin(r *http.Request) *http.RequestHow if r.URL.Path == "/sso/redirect" {Not a big fan of this approach, to be honest. This seems like a routing concern that would be better addressed outside any handler. Wouldn't it be better to apply the middleware to all but the "/sso/redirect" path at the router level? |
|
Thank you for commenting @jub0bs, you were one of the folks I most hoped would take a look!
Added to the intro.
Thank you for making me learn about the distinction. Unfortunately, it looks like the situation is murky. Sec-Fetch-Site applies the schemeful definition (HTTP ≠ HTTPS). We don't really care, since we will only allow same-origin.
The SameSite cookie attribute used to apply the schemeless definition (HTTP = HTTPS). Chrome changed that with Schemeful Same-Site in 2020, but Firefox and Safari never implemented it, so they will still send SameSite=Lax/Strict cookies from HTTP initiators to HTTPS origins.
Finally, for the purposes of cookie tossing, which is the generic sense of "same-site" I was using there, the schemeless definition (HTTP = HTTPS) applies, because HTTP origins can set (prefix-less) cookies on same-site HTTPS origins. There is a proposal to address this, Origin-Bound-Cookies (and specifically its lack of opt-out for scheme binding, which subsumes Scheme-Bound Cookies), which however hasn't shipped yet.
Does this sound right? Edit: I updated the Background, let me know if I got anything wrong.
Updated the text, thank you.
See above, I believe that is only the case in Chromium engines so far, unfortunately.
Well, https://web.dev/articles/fetch-metadata is a recommendation :) |
Done!
D'oh. I remember thinking "I need to check the docs for Vary" and then not doing it. However, I am having doubts on whether we should do this at all.
Given (2) in particular, I have removed the Vary part of the proposal. This also makes the Handler read-only for allowed requests.
Done.
I don't care that much about the color of the bikeshed, but if we realize we are missing an option, we can't add it to the constructor later. If we add a setter, then it leads to the question "what happens if you call Wrap and then SetFoo? does it apply retroactively?" Validating the bypass origins is very compelling, though. Here's an alternative API, taking a page out of context and log/slog. Then the most simple usage is just (Anyone has a better name or opinions on
It would set a private context value on the Request, which would be detected by the Handler later.
I thought about how this integrates with the router level and I agree this is a bit annoying, but that feels more error-prone: if another path is added later it would be unprotected by default, unless the developer remembers to apply Wrap. |
|
With the Unrelatedly, /cc @jba who might have ideas on routing integration. |
|
Yes, If have a manual Check, might be worth it creating an interface, and then have a single http.Handler type Checker interface {
Check(r *http.Request) error
}
func Filter(check Checker, next, err http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
h := err
if err := check.Check(r); err == nil {
h = next
}
h.ServeHTTP(w, r)
})
} |
I'm honoured! 🙇
Thank you for bringing this subtlety to my attention! I had incorrectly assumed that all browsers had caught up to the modern meaning of "site".
Indeed. Thanks!
I've got no further objections about the Background section. 😇 |
Indeed. With a factory function with parameters, we may paint ourselves into a corner. Another possibility is something like functional options (which @dsnet implemented a form of in encoding/json/v2), but the pattern remains controversial in the community.
A valid concern.
I would still prefer a name like
Ok, that makes sense. Thanks for clarifying.
Not necessarily. @earthboundkid once helped me realise that you can build your routes by composition and only selectively apply a middleware to some "roots"; I have something like the following in mind: func ExampleNewCrossOriginProtection_Wrap() {
mux := http.NewServeMux()
mux.HandleFunc("/sso/redirect", handleSSO) // note: not protected against CSRF (by design)
api := http.NewServeMux()
api.HandleFunc("GET /users", handleUsersGet)
api.HandleFunc("POST /users", handleUsersPost)
handler := http.NewCrossOriginProtection().Wrap(api)
mux.Handle("/api/", http.StripPrefix("/api", handler))
if err := http.ListenAndServe(":8080", mux); err != http.ErrServerClosed {
log.Fatal(err)
}
} |
I would very much like the color of the shed to be pink! :D Jokes aside, I think the nicest pattern I've seen in the wild for middleware (with signature type Middleware = func(http.Handler) http.Handler
type CrossOriginProtectionOpts struct{
// …
}
func CrossOriginProtection(opts CrossOriginProtectionOpts) Middleware {
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// Do the thing
next.ServeHTTP(w, r)
}
}
}It's perhaps a bit verbose, but very clear and future-proof. I'm biased, because I use Chi extensively, and this way, the user-facing API would be something like: func main() {
mux := chi.NewRouter()
mux.Use(http.CrossOriginProtection(CrossOriginProtectionOpts{
// …
}))
}In that context, calling it Thoughts? |
This approach would preclude option validation. |
Hmm. I might be misunderstanding you, but wouldn’t you do that here? type Middleware = func(http.Handler) http.Handler
type CrossOriginProtectionOpts struct{
// …
}
func CrossOriginProtection(opts CrossOriginProtectionOpts) Middleware {
// Validate opts here 👈
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// Do the thing
next.ServeHTTP(w, r)
}
}
} |
That's a nice pattern if you stick to it, but if you do
Calling it Wrap would be the first instance of "Wrap" in the net/http API surface, and there is a pattern in the stdlib of naming methods after what they return. I don't have a strong opinion, though.
I would call this a traditional "config" pattern. To validate the config it'll need to return The With API from #73626 (comment) works with Also reads nicely IMHO. I'm starting to really like the With API. |
You're right; you could validate the options. But how would you communicate failure (invalid options) to the caller of func (CrossOriginProtectionOpts) (Middleware, error) |
|
To summarize my understanding of the proposal at a high-level: Provide an opt-in mechanism for enabling CSRF defense using the
This mechanism requires additional customization, such as permitting a specific list of It must be possible to disable CSRF protection for some requests. That all seems fine to me. So far as I can tell, all of this can be easily implemented in a third-party package, and perhaps we should start with one. The proposal requires that the CSRF handler reject some requests. What, exactly, does it do to reject a request? A concern with any API, either in or out of std, is that this is all going to be necessarily opt-in. We've talked in the past about having a one-knob setting to make an A few general API thoughts: A function which returns an error is more general purpose than a handler with an error callback. For example: type CrossOriginProtectionOpts struct { ... }
// Implement http.Handler.
func (c CrossOriginProtectionOpts) ServeHTTP(ResponseWriter, *Request)
// Users can call Check directly to implement their own error handling.
func (c CrossOriginProtectionOpts) Check(req *Request) errorI don't like using A possible option here is to add pattern matching to the CSRF options: // Allow permits all requests to the given pattern.
// Patterns use the same syntax as [ServeMux].
//
// Example:
// csrf.Allow("/sso/")
func (c CrossOriginProtectionOpts) Allow(pattern string) |
This makes sense to me. x/net/cors or some such. It would give more time for flexibility before it's fully baked. I don't foresee this middleware gaining a lot of options, but some of the questions around should it use an options struct or functional options depend in part on how the API changes over time and whether people will need more options.
I like this. It makes it more foolproof to just wrap everything in CORS protection. Maybe a better name than |
I plan to prototype this as a third-party module, but I still think we should ship this in net/http anyway. There are two unmaintained modules out there imported by thousands (just in the open source sphere) with active vulnerabilities. Migrating to a standard library handler requires much less vetting and is an easier sell than
It doesn't pass them to the wrapped handler and instead returns a 403 Forbidden response.
Yes, a future hardened It would also be harder to migrate from an existing module to hardened
I agree with adding a Check, see #73626 (comment). We should still have the Handler as it provides a better safe-by-default experience and migration path from existing modules.
The difference is that if next year a colleague adds a new endpoint, with
This would definitely be more user-friendly, but I was worried about opening a can of worms. How do wildcards work? How easy is it to internally repurpose the pattern code? |
This comment has been minimized.
This comment has been minimized.
Mainly, I'm curious how we'd do this by default. If this was a Server configuration option (which I'm not suggesting, this is just a hypothetical), then we'd turn it on. How would we turn on a middleware handler by default? If there are handler configuration options, how does the user set them?
I'm thinking something along the lines of: type CSRFHandler struct {
mux ServeMux
}
type csrfAllowHandler struct { Handler }
func (h *CSRFHandler) Allow(pattern string) {
h.mux.Handle(pattern, csrfAllowHandler{})
}
func (h *CSRFHandler) Check(req *Request) error {
if handler, _ := h.mux.Handler(req); handler != nil {
return nil // always allow
}
// apply CSRF checks...
}The actual implementation could be more elegant if this is in net/http, but I think the above would work well enough. |
Not sure how we'd configure the http.Server, there are many possible answers and it feels a bit out of scope, but we don't even need to call this a middleware. It's about using a different Handler (the ErrorHandler or a default that 403's) if the request has some headers.
TIL about ServeMux.Handler! Nice. However, I think this will need to be otherwise we'll catch the 404 handler. This part might be a bit surprising, but maybe it will be more often than not what users expect: "If the path is not in its canonical form, the handler will be an internally-generated handler that redirects to the canonical path. [...] Handler also returns the registered pattern that matches the request or, in the case of internally-generated redirects, the path that will match after following the redirect." |
"cors" wouldn't be the right package name, though. This proposal has little to do with CORS, which is only a mechanism for servers to instruct browsers to relax the Same-Origin Policy for select clients. In particular, note that there are (potentially malicious) cross-origin requests that do not participate in the CORS protocol, such as one issued by a HTML form or by the following JS code: fetch('//example.com', {mode: 'no-cors', credentials: 'include'}) |
|
This proposal has been added to the active column of the proposals project |
Background
Cross Site Request Forgery (CSRF) is a confused deputy attack where the attacker causes the browser to send a request to a target using the ambient authority of the user’s cookies or network position (although the latter is being addressed by Private Network Access). For example,
attacker.examplecan serve the following HTML to a targetand the browser will send a POST request to
https://example.com/send-moneyusing the target’s cookies.Essentially all applications that use cookies for authentication need to protect against CSRF. Importantly, this is not about protecting against an attacker that can make arbitrary requests (as an attacker doesn't know the user's cookies), but about working with browsers to identify authenticated requests initiated from untrusted sources.
Unlike Cross-Origin Resource Sharing (CORS), which is about sharing responses across origins, CSRF is about accepting state-changing requests, even if the attacker will not see the response. Defending against leaks is significantly more complex and nuanced, especially in the age of Spectre.
Why do browsers allow these requests in the first place? At least in part because disabling these third-party cookies breaks important Single-Sign On (SSO) flows. Any solution we implement will need to support a bypass mechanism, but these endpoints are rare exceptions.
Same site vs same site vs same origin
https://app.example.com,https://marketing.example.com, and evenhttp://app.example.com(depending on the definition) are all same-site but not same-origin.It’s tempting to declare the goal as ensuring requests are simply from the same site, but different origins in the same site can actually sit at very different trust levels: for example it might be much easier to get XSS into an old marketing blog than in the admin panel.
The starkest difference in trust though is between an HTTPS and an HTTP origin, since a network attacker can serve anything it wants on the latter. This is sometimes referred to as the MitM CSRF bypass, but really it’s just a special case of a schemelessly same-site cross-origin CSRF attack.
Some parts of the Web platform apply a schemeful definition of same-site, where
https://app.example.comandhttp://app.example.comare not same-site:Using HTTP Strict Transport Security (HSTS), if possible, is a potential mitigation for HTTP→HTTPS issues.
Countermeasures
There are a number of potential countermeasures to CSRF, some of which have been available only for a few years.
Double submit or synchronized tokens
The “classic” countermeasure is a CSRF token, a large random value submitted in the request (e.g. as a hidden
<input>) and compared against a value stored in a cookie (double-submit) or in a stateful server-side session (synchronized tokens).Normally, double-submit is not a same-origin countermeasure, because same-site origins can set cookies on each other by “cookie tossing”. This can be mitigated with the
__Host-cookie prefix, or by binding the token to the session/user with signed metadata. The former makes it impossible for the attacker to set the cookie, the latter ensures the attacker doesn't know a valid value to set it to.Note that signing the cookies or tokens is unnecessary and ineffectual, unless it is binding the token to a user: an attacker that’s cookie tossing can otherwise obtain a valid signed pair by logging into the website and then use that for the attack.
This countermeasure turns a cross-origin forgery problem into a cross-origin leak problem: if the attacker can obtain a token from a cross-origin response, it can forge a valid request.
The token in the HTML body should be masked as a countermeasure against the BREACH compression attack.
The primary issue with CSRF tokens is that they require developers to instrument all their forms and other POST requests.
Origin header
Browsers send the source of a request in the Origin header, so CSRF can be mitigated by rejecting non-safe requests from other origins.
The main issue is knowing the application’s own origin. One option obviously is asking the developer to configure it, but that’s friction and might not always be easy (such as for open source projects and proxied setups).
The closest readily available approximation of the application’s own origin is the Host header. This has two issues:
http://Origin is a cross-origin HTTP→HTTPS request or a same-origin HTTP request.Some older (pre-2020) browsers didn’t send the Origin header for POST requests.
The value can be
nullin a variety of cases, such as due toReferrer-Policy: no-referreror following cross-origin redirects.nullmust be treated as an indication of a cross-origin request.Some privacy extensions remove the Origin header instead of setting it to
null. This should be considered a security vulnerability introduced by the extension, since it removes any reliable indication of a browser cross-origin request.SameSite cookies
If authentication cookies are explicitly set with the SameSite attribute Lax or Strict, they will not be sent with non-safe cross-site requests.
This is, by design, not a cross-origin protection, and it can’t be fixed with the
__Host-prefix (or Secure attribute), since that’s about who can set and read cookies, not about where the requests originate. (This difference is reflected in the difference between Scheme-Bound Cookies and Schemeful Same-Site.) The risk of same-site HTTP origins is still present, too, in browsers that don't implement Schemeful Same-Site.Note that the rollout of SameSite Lax by default has mostly failed due to widespread breakage, especially in SSO flows.
Some browsers now default to Lax-allowing-unsafe, while others default(ed) to None for the first two minutes after the cookie was set. These defaults are not effective CSRF countermeasures.
Non-simple requests
Although CORS is not designed to protect against CSRF, “non-simple requests” which for example set headers that a simple
<form>couldn’t set are preflighted by an OPTIONS request.An application could choose to allow only non-simple requests, but that is fairly limiting precisely because “simple requests” includes all the ones produced by
<form>.Fetch metadata
To provide a reliable cross-origin signal to websites, browsers introduced Fetch metadata. In particular, the Sec-Fetch-Site header is set to
cross-site/same-site/same-origin/noneand is now the recommended method to mitigate CSRF. (nonemeans the request was directly user-initiated, e.g. a bookmark.)The header has been available in all major browsers since 2023 (and earlier for all but Safari).
One limitation is that it is only sent to “trustworthy origins”, i.e. HTTPS and localhost. Note that this is not about the scheme of the initiator origin, but of the target, so it is sent for HTTP→HTTPS requests, but not for HTTPS→HTTP or HTTP→HTTP requests.
Existing libraries
I found two widely used Go libraries for protecting against CSRF.
github.com/gorilla/csrf
github.com/gorilla/csrf is primarily double-submit based, but then implements Origin header checks if the application is hosted on HTTPS, to protect against HTTP→HTTPS requests.
Until v1.7.3 (three weeks ago), HTTPS detection was broken, so only the same-site token checks were ever performed (GHSA-rq77-p4h8-4crw, https://attack.csrf.patrickod.com, see also #73151). In v1.7.3, the application is assumed to always be on HTTPS unless manually flagged otherwise, and the Origin header is checked against the Host header. This caused a number of new false positives for HTTP hosted applications (gorilla/csrf#188, gorilla/csrf#187, tailscale/tailscale#14872, tailscale/tailscale#15065).
The Handler allows GET, HEAD, OPTIONS, and TRACE.
If Origin is missing, the library checks that Referer has either no hostname or an allowed hostname. If both Origin and Referer are missing, the request is rejected (presumably rejecting most non-browser requests). These checks are effectively new in v1.7.3.
The checks can be bypassed on a per-Request basis with
func UnsafeSkipCheck(*http.Request) *http.Request.Note that in v1.7.3 the double-submit tokens are redundant and strictly weaker than the Origin checks. The library also asks the developer to manage a secret key to sign the cookie with the token. I discussed this with other experts and my conclusion is that the signature and secret key are ineffectual.
It is not clear how actively maintained the library is. I reported a bypass of the new same-origin protections three weeks ago (GHSA-rm6j-cg4g-v2xx) and have not heard back despite reaching out to the maintainers directly. https://attack.csrf.patrickod.com suggests four months passed between the HTTPS detection issue report and the next release.
github.com/justinas/nosurf
github.com/justinas/nosurf is similarly based on double-submit.
It also tries to do extra checks for HTTPS targets, but appears to have the same issue as GHSA-rq77-p4h8-4crw, since it checks
r.URL.Schemewhich is always empty. (I apologize for dropping effectively a 0-day, but this was just disclosed on a similar library, and anyway the intended security goal is not articulated in the docs AFAICT.) The check would be rejecting all requests if it weren’t broken, because it compares the Referer hostname withr.URL.Hostwhich is empty.https://github.com/justinas/nosurf/blob/e5c9c1fe2d4f69668ff78f872abf3b396a08673a/handler.go#L148-L168
It also allows GET, HEAD, OPTIONS, and TRACE.
The checks can be rejected based on the path (including globs and regexes) or by callback.
The library seems unmaintained. The latest commit is a CI fix a year ago, the previous commit a critical security vulnerability fix in 2020.
Proposal details
I propose we add a handler to net/http which rejects cross-origin non-safe browser requests using primarily Fetch metadata, which require no extra effort from the developer.
If the Sec-Fetch-Site header is present and is not
same-originornone, we block all non-GET/HEAD/OPTIONS requests. This already secures all major up-to-date browsers for sites hosted on trustworthy origins.We could stop there and fail open, but I suggest that as a fallback, if Sec-Fetch-Site is missing but Origin is present, we reject non-GET/HEAD/OPTIONS requests where the Origin header’s hostname doesn’t match the Host header. This will have a few false positives (missing Sec-Fetch-Site and a reverse proxy changes the Host) but eliminate most false negatives, and in particular it will protect HTTP sites. The only remaining false negatives will be HTTP→HTTPS requests sent by old browsers, which can be mitigated with HSTS (or by updating the probably anyway insecure browser).
We allow requests with neither Sec-Fetch-Site nor Origin, as they are not from browsers, and can’t be affected by CSRF.
Thanks to @empijei for helping with the design and the analysis, and to @patrickod for setting this in motion and testing the solution. /cc @golang/security
The text was updated successfully, but these errors were encountered: