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.example can serve the following HTML to a target
<form action="https://example.com/send-money" method="post">
<input type="hidden" name="to" value="filippo" />
<input type="hidden" name="amount" value="1000000" />
</form>
and the browser will send a POST request to https://example.com/send-money using 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 even http://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.com and http://app.example.com are not same-site:
- Cookies in general apply the schemeless definition (HTTP = HTTPS). There is a proposal to address this, Origin-Bound-Cookies (and specifically its lack of opt-out for scheme binding, which subsumes the earlier Scheme-Bound Cookies proposal), which however hasn't shipped yet.
- 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.
- Sec-Fetch-Site (and the HTML and Fetch specifications in general) applies the schemeful definition (HTTP ≠ HTTPS).
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:
- it may be different from the browser origin if a reverse proxy is involved;
- it does not include the scheme, so there is no way to know if an
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 null in a variety of cases, such as due to Referrer-Policy: no-referrer or following cross-origin redirects. null must 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/none and is now the recommended method to mitigate CSRF. (none means 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.Scheme which 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 with r.URL.Host which 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-origin or none, 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.
// CrossOriginForgeryHandler rejects with a 403 Forbidden any non-safe browser
// requests that were initiated from a different origin. It protects against
// [Cross-Site Request Forgery (CSRF)].
//
// Cross-origin requests are detected with the [Sec-Fetch-Site] header,
// available in all browsers since 2023, or by comparing the hostname of the
// [Origin] header with the Host header.
//
// The GET, HEAD, and OPTIONS methods are [safe methods] and are always allowed.
// It's important that applications do not perform any state changing actions
// due to requests with safe methods.
//
// Requests without Sec-Fetch-Site or Origin headers are assumed to be either
// same-origin or non-browser requests, and are allowed.
//
// [Sec-Fetch-Site]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Sec-Fetch-Site
// [Origin]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Origin
// [Cross-Site Request Forgery (CSRF)]: https://developer.mozilla.org/en-US/docs/Web/Security/Attacks/CSRF
// [safe methods]: https://developer.mozilla.org/en-US/docs/Glossary/Safe/HTTP
type CrossOriginForgeryHandler struct {
// Handler is invoked for same-origin or non-browser requests.
Handler Handler
// ErrorHandler is invoked for cross-origin requests.
// If nil, a 403 Forbidden response is returned.
ErrorHandler Handler
// BypassOrigins is a list of origins that are allowed to send cross-origin
// requests. The values in this list must be fully-formed origins, including
// the scheme, and are compared verbatim to the [Origin] header.
//
// More complex bypass rules cam be implemented with [UnsafeAllowCrossOrigin].
//
// [Origin]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Origin
BypassOrigins []string
}
// UnsafeAllowCrossOrigin disables [CrossOriginForgeryHandler] for the request.
// It is generally only useful when implementing single sign-on (SSO) flows.
func UnsafeAllowCrossOrigin(r *http.Request) *http.Request
func ExampleUnsafeAllowCrossOrigin() {
// This example shows how to use UnsafeAllowCrossOrigin to disable
// CrossOriginForgeryHandler for certain paths.
mux := NewServeMux()
// ...
csrfHandler := CrossOriginForgeryHandler{Handler: mux}
h := HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path == "/sso/redirect" {
// Disable CrossOriginForgeryHandler for this path.
r = UnsafeAllowCrossOrigin(r)
}
csrfHandler.ServeHTTP(w, r)
})
http.ListenAndServe(":8080", h)
}
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
Comment From: seankhliao
I take it the api looks like this?
func CrossOriginForgeryHandler(next http.Handler) http.Handler
Comment From: FiloSottile
I've added a concrete API (oops). It's a struct to accomodate the ErrorHandler and BypassOrigins parameter, with the option to add more. I don't really have a strong opinion on other ways to pass parameters.
Comment From: FiloSottile
Added a section surveying existing implementations, which helps motivate why we need a standard library solution.
Comment From: earthboundkid
It would be nice if ErrorHandler got the error so it could be logged.
Comment From: FiloSottile
It would be nice if ErrorHandler got the error so it could be logged.
gorilla/csrf has func FailureReason(*http.Request) error and we could do something similar, but I wonder how many useful bits that error would carry. I think it's just one: either Sec-Fetch-Site was present and not same-origin/none, or it was missing and Origin didn't match Host. Is it really that useful to tell them apart?
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.
Comment From: earthboundkid
I was thinking ErrorHandler could be func(ResponseWriter, *Request, error) instead of a Handler, but I take the point that all you really care about is how often it's being triggered.
Comment From: renthraysk
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.
Comment From: jub0bs
Thanks for putting this proposal together, @FiloSottile!
There's this widespread misconception that the changes to the SameSite attribute back in 2020 addressed all CSRF risks. If anything, the addition of a defence against CSRF in the standard library would help disabuse Gophers of this idea.
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.
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.
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.
Same site vs same origin
Thanks for stressing this crucial, yet often overlooked, distinction. Shameless plug, but I wrote a blog post on this topic a few years ago.
but really it’s just a special case of a same-site cross-origin CSRF attack.
In the modern sense of same-site (i.e. schemelessly same-site), which all major browsers understand, a request from http://example.com to https://example.com is considered cross-site, though.
same-site origins can set cookies on each other to an extent by “cookie tossing”.
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.
if the attacker can obtain a token from a cross-origin response, it can forge a valid request.
For an example of exactly this, see this post.
[...] precisely because “simple requests” means the ones produced by
<form>.
Close, but not quite: some HEAD requests are simple, but HTML forms can't send HEAD requests.
The risk of same-site HTTP origins is still present, too, and needs to be mitigated with HTTP Strict Transport Security (HSTS).
Not if "same-site" is understood as "schemelessly same-site"; see my comment further up.
the Sec-Fetch-Site header [...] is now the recommended method to mitigate CSRF
Citation needed 😄
github.com/gorilla/csrf [...]
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.
FWIW, I've had a similar experience. I think it's fair to consider that library unmaintained.
Comment From: jub0bs
go // origin. It protects against [Cross Site Request Forgery (CSRF)].
Hyphenate "cross site". 😇
go // The handler sets the Vary header to "Sec-Fetch-Site Origin" to ensure // that caches do not cache the response across origins.
Two minor problems with this passage of the doc comment: - Vary is a list-based header; as such, the elements listed in its value should be separated by a comma (possibly surrounded by optional whitespace). - The handler should add (rather than set) a Vary header; earlier middleware may well have already populated the response with a Vary header listing different header names, and clobbering that header could open the door to cache-poisoning attacks.
I suggest
// The middleware adds a Vary header with the value "Sec-Fetch-Site, Origin" to ensure
// that caching intermediaries do not get poisoned.
go // Non-browser requests and GET/HEAD/OPTIONS requests are not affected.
Perhaps a good opportunity for the doc to remind server developers of the concept of safe methods.
go type CrossOriginForgeryHandler struct { // Handler is invoked for same-origin or non-browser requests. Handler Handler
One disadvantage of this approach is that you cannot reuse a CrossOriginForgeryHandler for different handlers. Instead, I suggest renaming the type, dropping its Handler field, and equipping it with a Wrap(Handler) Handler method:
```go type CrossOriginForgeryMiddleware struct { // fields omitted }
func (m CrossOriginForgeryMiddleware) Wrap(h Handler) Handler { / ... */ } ```
In this connection, see - https://pkg.go.dev/github.com/jub0bs/cors#Middleware.Wrap - https://github.com/gorilla/handlers/pull/263
```go // ErrorHandler is invoked for cross-origin requests. // If nil, a 403 Forbidden response is returned. ErrorHandler Handler
// BypassOrigins is a list of origins that are allowed to send cross-origin // requests. The values in this list must be fully-formed origins, including // the scheme, and are compared verbatim to the [Origin] header. // // More complex bypass rules cam be implemented with [UnsafeAllowCrossOrigin]. // // [Origin]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Origin BypassOrigins []string } ```
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:
- You could validate the list of bypass origins. In my experience, many developers either lack familiarity with the concept of Web origin or are prone to mistakes when specifying origins.
- Via a modicum of defensive copying, you could make the type concurrency-safe, e.g. even when changes are made to the bypassOrigins parameter while the middleware is processing requests.
- You would have the option to later make the type reconfigurable even while it's processing requests.
// UnsafeAllowCrossOrigin disables [CrossOriginForgeryHandler] for the request.
// It is generally only useful when implementing single sign-on (SSO) flows.
func UnsafeAllowCrossOrigin(r *http.Request) *http.Request
How UnsafeAllowCrossOrigin would modify the request is unclear to me. Can you clarify?
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?
Comment From: FiloSottile
Thank you for commenting @jub0bs, you were one of the folks I most hoped would take a look!
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.
Added to the intro.
In the modern sense of same-site (i.e. schemelessly same-site), which all major browsers understand, a request from
http://example.comtohttps://example.comis considered cross-site, though.
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.
- https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Sec-Fetch-Site#same-site
- https://html.spec.whatwg.org/multipage/browsers.html#sites
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.
- https://web.dev/articles/schemeful-samesite
- https://bugzilla.mozilla.org/show_bug.cgi?id=1651119
- https://wpt.fyi/results/cookies/schemeful-same-site?label=master&label=experimental&aligned&q=schemeful
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.
- https://chromestatus.com/feature/4945698250293248
- https://github.com/sbingler/Origin-Bound-Cookies
- https://github.com/mikewest/scheming-cookies
- https://github.com/sbingler/schemeful-same-site#how-do-schemeful-same-site-and-scheme-bound-cookies-differ
Does this sound right? Edit: I updated the Background, let me know if I got anything wrong.
[...] precisely because “simple requests” means the ones produced by
<form>.Close, but not quite: some HEAD requests are simple, but HTML forms can't send HEAD requests.
Updated the text, thank you.
The risk of same-site HTTP origins is still present, too, and needs to be mitigated with HTTP Strict Transport Security (HSTS).
Not if "same-site" is understood as "schemelessly same-site"; see my comment further up.
See above, I believe that is only the case in Chromium engines so far, unfortunately.
the Sec-Fetch-Site header [...] is now the recommended method to mitigate CSRF
Citation needed 😄
Well, https://web.dev/articles/fetch-metadata is a recommendation :)
Comment From: FiloSottile
// origin. It protects against Cross Site Request Forgery (CSRF).
Hyphenate "cross site". 😇
Done!
// The handler sets the Vary header to "Sec-Fetch-Site Origin" to ensure // that caches do not cache the response across origins.
Two minor problems with this passage of the doc comment:
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.
- Cookie-personalized responses should carry
Cache-Control: privateanyway. - RFC 9111 is adamant that non-safe methods MUST not be cached.
Given (2) in particular, I have removed the Vary part of the proposal.
This also makes the Handler read-only for allowed requests.
// Non-browser requests and GET/HEAD/OPTIONS requests are not affected.
Perhaps a good opportunity for the doc to remind server developers of the concept of safe methods.
Done.
[...] renaming the type, dropping its
Handlerfield, and equipping it with aWrap(Handler) Handlermethod[...] those fields would be better left non-exported and the type should be instantiated via some factory function / "constructor"
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.
type CrossOriginProtection struct {
// contains filtered or unexported fields
}
func NewCrossOriginProtection() CrossOriginProtection
func (CrossOriginProtection) WithBypassOrigins([]string) (CrossOriginProtection, error)
func (CrossOriginProtection) WithErrorHandler(Handler) CrossOriginProtection
func (CrossOriginProtection) Handler(Handler) Handler
Then the most simple usage is just NewCrossOriginProtection().Handler(mux) but you can wrap all the handlers you like, and we can add options later. Still, it's clear that options don't apply to the previous values.
(Anyone has a better name or opinions on *CrossOriginProtection vs CrossOriginProtection?)
How
UnsafeAllowCrossOriginwould modify the request is unclear to me. Can you clarify?
It would set a private context value on the Request, which would be detected by the Handler later.
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?
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.
Comment From: FiloSottile
With the CrossOriginProtection API, and given it's now a read-only check, we could add a manual Check method, now or later, as suggested above.
func (CrossOriginProtection) Check(*Request) error
Unrelatedly, /cc @jba who might have ideas on routing integration.
Comment From: renthraysk
Yes, Check makes testing easier, as don't have to deal with http.Handlers.
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)
})
}
Comment From: jba
Unrelatedly, /cc @jba who might have ideas on routing integration.
I think @neild might have some thoughts.
Comment From: jub0bs
@FiloSottile
Thank you for commenting @jub0bs, you were one of the folks I most hoped would take a look!
I'm honoured! 🙇
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.
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".
Well, https://web.dev/articles/fetch-metadata is a recommendation :)
Indeed. Thanks!
Does this sound right? Edit: I updated the Background, let me know if I got anything wrong.
I've got no further objections about the Background section. 😇
Comment From: jub0bs
@FiloSottile
[...] if we realize we are missing an option, we can't add it to the constructor later.
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.
If we add a setter, then it leads to the question "what happens if you call
Wrapand thenSetFoo? does it apply retroactively?"
A valid concern.
go func (CrossOriginProtection) Handler(Handler) Handler
I would still prefer a name like Wrap, since that's what a func(http.Handler) http.Handler (a middleware, essentially) invokes in my mind, but I won't insist further; others can chime in about naming.
It would set a private context value on the
Request, which would be detected by the Handler later.
Ok, that makes sense. Thanks for clarifying.
[...] that feels more error-prone: if another path is added later it would be unprotected by default, unless the developer remembers to apply Wrap.
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)
}
}
Comment From: markuswustenberg
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?"
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 func(http.Handler) http.Handler) is a constructor function that takes an options struct, so something like:
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 Wrap wouldn't make sense. But I can see why it would in the std lib.
Thoughts?
Comment From: jub0bs
@markuswustenberg
```go type Middleware = func(http.Handler) http.Handler
func CrossOriginProtection(CrossOriginProtectionOpts) Middleware ```
This approach would preclude option validation.
Comment From: markuswustenberg
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)
}
}
}
Comment From: FiloSottile
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:
That's a nice pattern if you stick to it, but if you do mux.Handle("/api/newEndpoint", ...) by mistake it won't be protected.
I would still prefer a name like
Wrap, since that's what afunc(http.Handler) http.Handler(a middleware, essentially) invokes in my mind
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.
func CrossOriginProtection(opts CrossOriginProtectionOpts) Middleware
I would call this a traditional "config" pattern. To validate the config it'll need to return (Middleware, error) and then you can't pass it to chi.(*Mux).Use. Also, you'd need to check the error for every Handler you wrap.
The With API from https://github.com/golang/go/issues/73626#issuecomment-2870092637 works with chi.(*Mux).Use:
mux := chi.NewRouter()
mux.Use(NewCrossOriginProtection().Handler)
Also reads nicely IMHO. I'm starting to really like the With API.
Comment From: jub0bs
@markuswustenberg
Hmm. I might be misunderstanding you, but wouldn’t you do that here?
You're right; you could validate the options. But how would you communicate failure (invalid options) to the caller of CrossOriginProtection, then? Panicking doesn't feel right. As Filippo wrote, you'd need the following signature:
func (CrossOriginProtectionOpts) (Middleware, error)
Comment From: neild
To summarize my understanding of the proposal at a high-level:
Provide an opt-in mechanism for enabling CSRF defense using the Sec-Fetch-Site and Origin headers. This will:
- Allow GET/HEAD/OPTIONS requests.
- Deny other requests when
Sec-Fetch-Siteis present and notsame-originornone. - Deny other requests when
Originis present and is not exactly equal toHost.
This mechanism requires additional customization, such as permitting a specific list of Origin values.
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 http.Server more defensive, changing less-secure defaults to more-secure ones. Is this something that can/should be enabled by a setting like that? How would this work?
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) error
I don't like using *http.Request to pass options through different handler layers, as in the proposed UnsafeAllowCrossOrigin. I also don't see how this is any less mistake-prone than composed muxes as @jub0bs proposes; in both cases, you need to apply pattern matching both before and after the CSRF handler.
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)
Comment From: earthboundkid
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.
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.
The proposal requires that the CSRF handler reject some requests. What, exactly, does it do to reject a request?
ErrorHandler is called if present, otherwise it does some default, presumably just text saying Forbidden: Bad CORS request.
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)
I like this. It makes it more foolproof to just wrap everything in CORS protection. Maybe a better name than Allow is DisableFor.
Comment From: FiloSottile
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.
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 filippo.io/csrf.
The proposal requires that the CSRF handler reject some requests. What, exactly, does it do to reject a request?
It doesn't pass them to the wrapped handler and instead returns a 403 Forbidden response.
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
http.Servermore defensive, changing less-secure defaults to more-secure ones. Is this something that can/should be enabled by a setting like that? How would this work?
Yes, a future hardened http.Server could do this by default. However, there is empirical evidence that applications do opt-in (by importing the unmaintained modules), so I wouldn't hold this until we move forward with the broader approach.
It would also be harder to migrate from an existing module to hardened http.Server than to this.
A function which returns an error is more general purpose than a handler with an error callback.
I agree with adding a Check, see https://github.com/golang/go/issues/73626#issuecomment-2870406631. We should still have the Handler as it provides a better safe-by-default experience and migration path from existing modules.
I don't like using
*http.Requestto pass options through different handler layers, as in the proposedUnsafeAllowCrossOrigin. I also don't see how this is any less mistake-prone than composed muxes as @jub0bs proposes; in both cases, you need to apply pattern matching both before and after the CSRF handler.
The difference is that if next year a colleague adds a new endpoint, with UnsafeAllowCrossOrigin as used in the example it will be protected by default.
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 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?
Comment From: neild
Yes, a future hardened http.Server could do this by default.
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?
How do wildcards work? How easy is it to internally repurpose the pattern code?
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.
Comment From: FiloSottile
Yes, a future hardened http.Server could do this by default.
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?
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.
if handler, _ := h.mux.Handler(req); handler != nil {
TIL about ServeMux.Handler! Nice. However, I think this will need to be
if _, pattern := h.mux.Handler(req); pattern != "" {
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."
Comment From: jub0bs
@earthboundkid
This makes sense to me. x/net/cors or some such.
[...] wrap everything in CORS protection
"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 initiated by a HTML form or by the following JS code:
fetch('//example.com', {mode: 'no-cors', credentials: 'include'})
Comment From: aclements
This proposal has been added to the active column of the proposals project and will now be reviewed at the weekly proposal review meetings. — aclements for the proposal review group
Comment From: neild
How about something like this:
// CrossOriginProtection implements [Cross-Site Request Forgery (CSRF)]
// protections by rejecting non-safe browser requests initiated from
// a different origin.
//
// Cross-origin requests are detected with the [Sec-Fetch-Site] header,
// available in all browsers since 2023, or by comparing the hostname of the
// [Origin] header with the Host header.
//
// The GET, HEAD, and OPTIONS methods are [safe methods] and are always allowed.
// It's important that applications do not perform any state changing actions
// due to requests with safe methods.
//
// Requests without Sec-Fetch-Site or Origin headers are assumed to be either
// same-origin or non-browser requests, and are allowed.
//
// [Sec-Fetch-Site]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Sec-Fetch-Site
// [Origin]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Origin
// [Cross-Site Request Forgery (CSRF)]: https://developer.mozilla.org/en-US/docs/Web/Security/Attacks/CSRF
// [safe methods]: https://developer.mozilla.org/en-US/docs/Glossary/Safe/HTTP
type CrossOriginProtection struct {
}
// AllowOrigin permits all requests with an [Origin] header
// which exactly matches the given value.
//
// [Origin]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Origin
func (c *CrossOriginProtection) AllowOrigin(origin string)
// AllowPattern permits all requests that match the given pattern.
// The pattern syntax and precedence rules are the same as [ServeMux].
func (c *CrossOriginProtection) AllowPattern(pattern string)
// Check applies cross-origin checks to a request.
// It returns an error if the request should be rejected.
func (c *CrossOriginProtection) Check(req *http.Request) error
// Handler returns a handler that applies cross-origin checks
// before invoking the handler h.
//
// If a request fails cross-origin checks, the request is rejected
// with a 403 Forbidden status.
func (c *CrossOriginProtection) Handler(http.Handler) http.Handler
Comment From: jub0bs
@neild
go // AllowOrigin permits all requests with an [Origin] header // which exactly matches the given value. // // [Origin]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Origin func (c *CrossOriginProtection) AllowOrigin(origin string)
What if the specified value is not a valid Web origin (a frequent mistake, in my experience)? I'd favour returning an error:
func (c *CrossOriginProtection) AllowOrigin(origin string) error
Comment From: earthboundkid
https://github.com/golang/go/issues/73626#issuecomment-2877843341 is fine, but it would be more useful with a func (c *CrossOriginProtection) ErrorHandler(http.Handler) method that lets you invoke a custom handler on error. That way you can render your standard error page, log things, etc.
Comment From: neild
Why is ErrorHandler better than:
if err := csrf.Check(); err != nil {
// do absolutely whatever you want here
}
Comment From: FiloSottile
Yeah https://github.com/golang/go/issues/73626#issuecomment-2877843341 is also essentially what I was converging on while prototyping.
Why is
ErrorHandlerbetter than:
if err := csrf.Check(); err != nil { // do absolutely whatever you want here }
For the same convenience reasons we'd have CrossOriginProtection.Handler instead of just telling people to write their own Handler that runs Check, no?
It's nice to use CrossOriginProtection.Handler and put a non-security-sensitive Handler in ErrorHandler, instead of writing a security-sensitive Handler. (Don't forget the return at the end of that err != nil body!)
Comment From: neild
For the question of how we'd enable this behavior as part of a "hardened mode" for servers, one possibility might be to add a field to Server:
type Server struct {
// If CrossOriginProtection is non-nil, all requests are validated using it before Server.Handler is called.
CrossOriginProtection *CrossOriginProtection
}
Then the hardened mode switch could initialize this field, and users have a way to further configure the settings if needed.
(None of this is part of this proposal. I just want to have some sense that this can be made to work if/when we get around to adding that hardened mode switch.)
Comment From: aclements
@earthboundkid , I'm not sure exactly what API you're proposing. Would ErrorHandler(h) be a mutator method that sets an error handler to use in case the check in Handler fails?
In general, I'm weary of unnecessary statefulness. I think it's also a little odd that this would only affect the Handler method and not Check.
An alternative that's less stateful and clearly ties the error handler to Handler would be:
// Handler returns a handler that applies cross-origin checks
// before invoking the handler h.
//
// If a request fails cross-origin checks, this invokes the fail handler.
// The fail handler may be nil, in which case the request is rejected
// with a 403 Forbidden status.
func (c *CrossOriginProtection) Handler(h http.Handler, fail http.Handler) http.Handler
Comment From: earthboundkid
func (c *CrossOriginProtection) Handler(h http.Handler, fail http.Handler) http.Handler is good too, but it doesn't implement the func(http.Handler) http.Handler middleware pattern that a lot of third parties expect. Of course, neither does http.MaxBytesHandler (which I implemented) or http.TimeoutHandler, so maybe it doesn't matter.
Comment From: FiloSottile
Agreed that passing the failure handler as an argument is better than setting it on the CrossOriginProtection value.
I'm not a fan of anonymous nil arguments, especially in the most common default configuration, and I don't expect most sites will set the failure handler.
Let's just split them? Can anyone think of a better name than HandlerWithFailHandler?
func (c *CrossOriginProtection) Handler(h http.Handler) http.Handler
func (c *CrossOriginProtection) HandlerWithFailHandler(h http.Handler, fail http.Handler) http.Handler
Sounds like, naming aside, we've converged on the following API?
// CrossOriginProtection implements [Cross-Site Request Forgery (CSRF)]
// protections by rejecting non-safe browser requests initiated from a different
// origin.
//
// Cross-origin requests are detected with the [Sec-Fetch-Site] header,
// available in all browsers since 2023, or by comparing the hostname of the
// [Origin] header with the Host header.
//
// The GET, HEAD, and OPTIONS methods are [safe methods] and are always allowed.
// It's important that applications do not perform any state changing actions
// due to requests with safe methods.
//
// Requests without Sec-Fetch-Site or Origin headers are assumed to be either
// same-origin or non-browser requests, and are allowed.
//
// [Sec-Fetch-Site]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Sec-Fetch-Site
// [Origin]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Origin
// [Cross-Site Request Forgery (CSRF)]: https://developer.mozilla.org/en-US/docs/Web/Security/Attacks/CSRF
// [safe methods]: https://developer.mozilla.org/en-US/docs/Glossary/Safe/HTTP
type CrossOriginProtection struct {}
// NewCrossOriginProtection returns a new [CrossOriginProtection] value.
func NewCrossOriginProtection() *CrossOriginProtection
// AddTrustedOrigin allows all requests with an [Origin] header
// which exactly matches the given value.
//
// Origin header values are of the form "scheme://host[:port]".
//
// [Origin]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Origin
func (c *CrossOriginProtection) AddTrustedOrigin(origin string) error
// AddUnsafeBypassPattern permits all requests that match the given pattern.
// The pattern syntax and precedence rules are the same as [ServeMux].
func (c *CrossOriginProtection) AddUnsafeBypassPattern(pattern string)
// Check applies cross-origin checks to a request.
// It returns an error if the request should be rejected.
func (c *CrossOriginProtection) Check(req *http.Request) error
// Handler returns a handler that applies cross-origin checks
// before invoking the handler h.
//
// If a request fails cross-origin checks, the request is rejected
// with a 403 Forbidden status.
func (c *CrossOriginProtection) Handler(h http.Handler) http.Handler
// HandlerWithFailHandler returns a handler that applies cross-origin checks
// before invoking the handler h.
//
// If a request fails cross-origin checks, the request is handled with the given
// failure handler.
func (c *CrossOriginProtection) HandlerWithFailHandler(h http.Handler, fail http.Handler) http.Handler
I named the bypasses AddTrustedOrigin and AddUnsafeBypassPattern because while one adds an origin to the trust boundary, and requires no further care if the origin is under the application's control, the latter is an unsafe bypass that needs careful handling in the endpoint implementation. I like unsafe things to have ugly names, but I'm not strongly attached to these names.
I've prototyped this at https://pkg.go.dev/filippo.io/csrf and it seems to work fine.
Comment From: neild
I'll argue for a stateful error handler, although I don't have a strong objection to the other approach:
// SetErrorHandler sets a handler to invoke when a request is rejected.
// The default error handler responds with a 403 Forbidden status.
//
// Check does not call the error handler.
func (c *CrossOriginProtection) SetErrorHandler(h http.Handler)
If we add a CrossOriginProtection field to Server in the future (as I suggest in https://github.com/golang/go/issues/73626#issuecomment-2878127776), then a SetErrorHandler method can be used to easily set a server-wide error handler. I don't know if we'll want to do that or not, but keeping the option open seems good.
Comment From: aclements
// CrossOriginProtection implements [Cross-Site Request Forgery (CSRF)] // protections
Maybe this should read "CrossOriginProtection implements protection against ..., also known as Cross-Site Request Forgery (CSRF)". The current phrasing of "XProtection implements something-that-is-not-X protection" seems very odd!
func (c *CrossOriginProtection) AllowPattern(pattern string)
@bradfitz pointed out that this is purely a convenience. @neild provided the following comparison of wrapping a top-level server handler with CSRF that allows cross-site requests to /sso:
With AllowPattern:
csrf := &CrossOriginProtection{}
csrf.AllowPattern("/sso")
server.Handler = csrf.Handle(server.Handler)
And without:
csrf := &CrossOriginProtection{}
csrfMux := http.ServeMux{}
csrfMux.Handle("/", csrf)
csrf.Handle("/sso", server.Handler)
server.Handler = csrfMux
Not a huge difference in terms of code quantity, but in my opinion the ~latter is much more readable than the former~ former is much more readable than the latter.
Minor bikeshed: I'm not sure about the name AllowPattern. It reads like you're allowing CSRF on a pattern. BypassPattern? Likewise for AllowOrigin, though somehow I don't find that one as confusing.
Comment From: aclements
Minor bikeshed: I'm not sure about the name AllowPattern.
Sorry, GitHub hadn't loaded the replies from days ago when I posted this. Sounds like @FiloSottile is thinking along the same lines. :)
Comment From: daenney
Not a huge difference in terms of code quantity, but in my opinion the latter is much more readable than the former.
I find myself in the opposite camp, I think the first example is much easier to read.
However, I find both examples a little confusing. csrf.AllowPattern seems to suggest "allow CSRF on this endpoint" which feels a bit odd. It's intended to be "allow cross-site requests" but not the forgery part.
The second example, csrf.Handle("/endpoint") is somewhat confusing to me as well. Are we allowing cross-site requests to that endpoint, are we protecting that specific endpoint from CSRF etc. The intent isn't clear to me from the naming.
Comment From: smlx
Minor bikeshed: I'm not sure about the name AllowPattern. It reads like you're allowing CSRF on a pattern. BypassPattern? Likewise for AllowOrigin, though somehow I don't find that one as confusing.
This may be a result of the confusing variable naming. For an instantiation of CrossOriginProtection more natural variable naming might be:
cop := &CrossOriginProtection{}
cop.AllowPattern("/sso")
server.Handler = cop.Handle(server.Handler)
Comment From: FiloSottile
csrf := &CrossOriginProtection{} csrfMux := http.ServeMux{} csrfMux.Handle("/", csrf) csrf.Handle("/sso", server.Handler) server.Handler = csrfMuxNot a huge difference in terms of code quantity, but in my opinion the latter is much more readable than the former.
It might be me but I am failing to parse this. What does CrossOriginProtection.Handle do in this proposed API? What's the point of csrfMux which handles all requests with csrf?
In general I agree that bypass patterns are a convenience that can be reimplemented by selective wrapping, e.g. https://github.com/golang/go/issues/73626#issuecomment-2872696587. However, selective wrapping exposes the risk of attaching a new handler to the wrong unwrapped mux, implicitly disabling protection. Explicitly typing AddUnsafeBypassPattern is easier to catch in code review and with linting/static analysis for escalation.
Comment From: aclements
I find myself in the opposite camp, I think the first example is much easier to read.
Sorry, I made a typo. I meant to say the first is much easier to read!
This may be a result of the confusing variable naming. For an instantiation of CrossOriginProtection more natural variable naming might be (
cop):
That still reads to me like you're "allowing" cross-origin protection on "/sso", which is the reverse of what you're doing.
I'm not sure about AddUnsafeBypassPattern because it's not "unsafe" in the language sense. "Bypass" seems like a good word. Or spelling out "cross-origin". AddBypassPattern? AddCrossOriginPattern?
Comment From: FiloSottile
AddInsecureBypassPattern, like tls.Config.InsecureSkipVerify?
Aside from this name, I think the only other open question is how to specify a fail handler. https://github.com/golang/go/issues/73626#issuecomment-2887674563
Comment From: aclements
I'm good with AddInsecureBypassPattern.
For the error handler, the options seems to be:
- Do nothing (https://github.com/golang/go/issues/73626#issuecomment-2878101163). Simple, but inconvenient and forces mixing of security-sensitive and security-insensitive code.
func (c *CrossOriginProtection) SetErrorHandler(h http.Handler)(https://github.com/golang/go/issues/73626#issuecomment-2878014887 and https://github.com/golang/go/issues/73626#issuecomment-2887674563). Easy to set a default, server-wide handler and will mix well in the future if we add aCrossOriginProtectionfield toServer.- Modify
Handler:func (c *CrossOriginProtection) Handler(h http.Handler, fail http.Handler) http.Handler(https://github.com/golang/go/issues/73626#issuecomment-2880809718). Avoids statefulness, but doesn't follow thefunc(http.Handler) http.Handlermiddleware pattern. - Add
func (c *CrossOriginProtection) HandlerWithFailHandler(h http.Handler, fail http.Handler) http.Handler(https://github.com/golang/go/issues/73626#issuecomment-2887653166). Avoids statefulness and "optional nil" pattern and works with handler middlerware. But no way to set a default.
Comment From: gopherbot
Change https://go.dev/cl/674936 mentions this issue: net/http: add CrossOriginProtection
Comment From: FiloSottile
I mailed CL 674936 to try to get in for Go 1.25, since we still haven't heard back from gorills/csrf. It's extremely roll-backable, cleanly adding two files.
I implemented CrossOriginProtection.SetBlockedHandler (renamed since blocking is not really an error) because while I agree HandlerWithFailHandler is cleaner, being able to use CrossOriginProtection as a config field is a strong argument.
Comment From: aclements
I implemented CrossOriginProtection.SetBlockedHandler (renamed since blocking is not really an error)
"Blocked" is potentially confusing, especially in a networking context where it usually means "waiting". SetDenyHandler perhaps?
Comment From: aclements
In proposal review, we also agreed that SetWhateverHandler is probably the best approach because it opens the possibility of using CrossOriginProtection as a config field.
Comment From: aclements
Based on the discussion above, this proposal seems like a likely accept. — aclements for the proposal review group
The proposal is to add the following to package net/http:
// CrossOriginProtection implements [Cross-Site Request Forgery (CSRF)]
// protections by rejecting non-safe browser requests initiated from a different
// origin.
//
// Cross-origin requests are detected with the [Sec-Fetch-Site] header,
// available in all browsers since 2023, or by comparing the hostname of the
// [Origin] header with the Host header.
//
// The GET, HEAD, and OPTIONS methods are [safe methods] and are always allowed.
// It's important that applications do not perform any state changing actions
// due to requests with safe methods.
//
// Requests without Sec-Fetch-Site or Origin headers are assumed to be either
// same-origin or non-browser requests, and are allowed.
//
// [Sec-Fetch-Site]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Sec-Fetch-Site
// [Origin]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Origin
// [Cross-Site Request Forgery (CSRF)]: https://developer.mozilla.org/en-US/docs/Web/Security/Attacks/CSRF
// [safe methods]: https://developer.mozilla.org/en-US/docs/Glossary/Safe/HTTP
type CrossOriginProtection struct {}
// NewCrossOriginProtection returns a new [CrossOriginProtection] value.
func NewCrossOriginProtection() *CrossOriginProtection
// AddTrustedOrigin allows all requests with an [Origin] header
// which exactly matches the given value.
//
// Origin header values are of the form "scheme://host[:port]".
//
// [Origin]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Origin
func (c *CrossOriginProtection) AddTrustedOrigin(origin string) error
// AddInsecureBypassPattern permits all requests that match the given pattern.
// The pattern syntax and precedence rules are the same as [ServeMux].
func (c *CrossOriginProtection) AddInsecureBypassPattern(pattern string)
// Check applies cross-origin checks to a request.
// It returns an error if the request should be rejected.
func (c *CrossOriginProtection) Check(req *http.Request) error
// Handler returns a handler that applies cross-origin checks
// before invoking the handler h.
//
// If a request fails cross-origin checks, the request is handled
// using the deny handler, or rejected with a 403 Forbidden status
// if no deny handler is set.
func (c *CrossOriginProtection) Handler(h http.Handler) http.Handler
// SetDenyHandler sets the handler used for requests that
// fail cross-origin checks.
func (c *CrossOriginProtection) SetDenyHandler(h http.Handler) http.Handler
Comment From: aclements
(Edit 2025-06-04: Corrected a minor error in a function signature.)
No change in consensus, so accepted. 🎉 This issue now tracks the work of implementing the proposal. — aclements for the proposal review group
The proposal is to add the following to package net/http:
// CrossOriginProtection implements [Cross-Site Request Forgery (CSRF)]
// protections by rejecting non-safe browser requests initiated from a different
// origin.
//
// Cross-origin requests are detected with the [Sec-Fetch-Site] header,
// available in all browsers since 2023, or by comparing the hostname of the
// [Origin] header with the Host header.
//
// The GET, HEAD, and OPTIONS methods are [safe methods] and are always allowed.
// It's important that applications do not perform any state changing actions
// due to requests with safe methods.
//
// Requests without Sec-Fetch-Site or Origin headers are assumed to be either
// same-origin or non-browser requests, and are allowed.
//
// [Sec-Fetch-Site]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Sec-Fetch-Site
// [Origin]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Origin
// [Cross-Site Request Forgery (CSRF)]: https://developer.mozilla.org/en-US/docs/Web/Security/Attacks/CSRF
// [safe methods]: https://developer.mozilla.org/en-US/docs/Glossary/Safe/HTTP
type CrossOriginProtection struct {}
// NewCrossOriginProtection returns a new [CrossOriginProtection] value.
func NewCrossOriginProtection() *CrossOriginProtection
// AddTrustedOrigin allows all requests with an [Origin] header
// which exactly matches the given value.
//
// Origin header values are of the form "scheme://host[:port]".
//
// [Origin]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Origin
func (c *CrossOriginProtection) AddTrustedOrigin(origin string) error
// AddInsecureBypassPattern permits all requests that match the given pattern.
// The pattern syntax and precedence rules are the same as [ServeMux].
func (c *CrossOriginProtection) AddInsecureBypassPattern(pattern string)
// Check applies cross-origin checks to a request.
// It returns an error if the request should be rejected.
func (c *CrossOriginProtection) Check(req *http.Request) error
// Handler returns a handler that applies cross-origin checks
// before invoking the handler h.
//
// If a request fails cross-origin checks, the request is handled
// using the deny handler, or rejected with a 403 Forbidden status
// if no deny handler is set.
func (c *CrossOriginProtection) Handler(h http.Handler) http.Handler
// SetDenyHandler sets the handler used for requests that
// fail cross-origin checks.
func (c *CrossOriginProtection) SetDenyHandler(h http.Handler)
Comment From: earthboundkid
func (c *CrossOriginProtection) SetDenyHandler(h http.Handler) http.Handler
I think this should be func (c *CrossOriginProtection) SetDenyHandler(h http.Handler) (no return value).
Comment From: neild
I think this should be func (c *CrossOriginProtection) SetDenyHandler(h http.Handler) (no return value).
Correct: SetDenyHandler has no return value.
Comment From: neild
Marking as closed, as the implementation landed just before the freeze.
Comment From: aclements
Last minute question from API audit: Why do we have NewCrossOriginProtection instead of just making the zero value of CrossOriginProtection work? It would require a tiny amount of on-demand initialization, but that would be easy to do.
Comment From: FiloSottile
Last minute question from API audit: Why do we have
NewCrossOriginProtectioninstead of just making the zero value ofCrossOriginProtectionwork? It would require a tiny amount of on-demand initialization, but that would be easy to do.
Discussed here during code review: https://go-review.googlesource.com/c/go/+/674936/comment/2a5cee0c_7dc7c3fa/
Comment From: aclements
Thanks for the pointer to that discussion. I agree that there's some value to providing NewCrossOriginProtection for in-place initialization and to parallel NewServeMux(), but you pointed out that the zero ServeMux is usable, and right now the zero CrossOriginProtection is not usable. It seems like we should keep NewCrossOriginProtection, but also make the zero value work.
You pointed out that the constructor is useful for one-shot use like:
server.Handler = http.NewCrossOriginProtection.Handle(server.Handler)
But if the code needs to configure the CrossOriginProtection at all, I would expect this to work and it doesn't right now:
var csrf http.CrossOriginProtection
csrf.AddInsecureBypassPattern("/sso")
server.Handler = csrf.Handle(server.Handler)
Comment From: FiloSottile
I agree it'd be reasonably easy to make the zero value usable. Do we want to do it as a freeze exception or for Go 1.26?
Comment From: gopherbot
Change https://go.dev/cl/680396 mentions this issue: net/http: make the zero value of CrossOriginProtection work
Comment From: jub0bs
This addition to the standard library is worthy of a post on the Go blog. Just a thought.