Go: proposal: x/net/xsrftoken: add new, less error-prone API

[empijei]

See https://github.com/golang/go/issues/42166#issuecomment-732061886 for concrete API.

---

Preface

The only thing that Go provides to protect against CSRF is the x/net/xsrftoken package. This is per-se an issue but there is an additional problem.

The API of said package is the following:

const Timeout
func Generate(key, userID, actionID string) string
func Valid(token, key, userID, actionID string) bool

This is the whole API surface, it is very low-level and requires its users to have a somewhat advanced knowledge of web security to use it properly.
I would invite the reader to stop here for a handful of seconds and think how they would use this package to protect a web application, including how they would retrieve the required userID.

The issue

By looking at the naming here a programmer might be inclined to use the XSRF protection only for authenticated users, especially since userID is the name of one of the parameters for both Generate and Valid.

This means that users of this package will probably be vulnerable to Login CSRF. I say this because some colleagues of mine and I analyzed quite a lot of Go web services code we could access and found it consistently vulnerable to some form of CSRF (the maintainers have already been warned and have fixed the issues).

I propose to apply one or more of the following:

1. Provide a higher-level protection alongside the low-level one: something that works on http handlers and manages cookies/token injection transparently. This would remove the burden of understanding the internals of CSRF from the users and would require less code to be written. As an example there are NoSurf and csrf that have a quite small but significantly harder to misuse API
2. Add documentation to this package with examples and detailed explanation on how to use it, especially wrt pseudonymous tokens to pass as userID, which from our analysis was one of the most frequent mistakes.
3. Provide other functions: add some functions to this package. For example helpers to issue and validate csrf-related cookies and inject tokens in html templates. These would basically be the easy-to-use and slightly more versatile building blocks for the first point of this list.

I am willing to do the work for any of these, but I would like to discuss all alternatives and gather some consensus and more ideas before I do.

[empijei]

@rsc you asked (in #42168)

what did you have in mind as a new, less error-prone API?

My proposal would be in three parts

A higher-level protection:

func Protect(http.Handler) http.Handler

This would decorate the given handler with a few features:

• Validate CSRF tokens on state-changing methods
• Inject the CSRF token in the request context for handlers to use

The tokens and validation algorithms we can use are several.

Double-submit secret

A secret token needs to both be in the request cookie and in a form/header. This is easy to implement and to work with. The implementation would be completely application-agnostic since it wouldn't require the user or action parameters, the CSRF protection is applied per-session.

Using this protection would require users to protect the entire ServeMux with the decorator we provide and do one of the following:

• Add a hidden form input to all forms with the injected CSRF token value or set up the client-side code to add an additional header on requests (e.g. Angular does exactly this by default).
• Make sure GET, HEAD, OPTIONS and similar non-state-changing methods are indeed non-state-changing.

Going for this solution would mean completely dropping the current API of this package and use a different approach altogether.

An example implementation would look like this:

package xsrftoken

// Protect defends a handler from CSRF attacks.
// This should ideally be used on entire server muxes.
// Users should make sure form-submission handlers only accept POST or other state-changing methods and don't work with GET
func Protect(h http.Handler /*+ parameters for configuration like header vs form submission*/) http.Handler{
  return http.HandlerFunc(func(w http.ResponseWriter, r*http.Request)){
    token, err := getTokenFromCookie(r)
    if err != nil {
      token = genSecureRandomToken()
      setCookie(w) // we can make this expire after 24h to keep renewing the secret
    }
    r = r.WithContext(context.WithValue(r.Context(), ctxKey, token))
    if isStateChangingMethod(r) && !valid(r){
      http.Error(w, "Forbidden", 403)
      return
    }
    h.ServeHTTP(w, r)
  })
}

// GetToken retrieves the CSRF token from the current request context.
func GetToken(r *http.Request) (string, error)

We could potentially simplify this further to rely on SameSite=strict behavior of cookies but that would introduce some niche vulnerabilities that I would avoid if possible.

User-bound token

This is more tricky to use and it's what our current API suggests to do: basically the token is re-generated when received for the given (user,action,time) tuple and validated against the received one.
The issue with this approach is that this requires quite a lot of knowledge about the app being protected for virtually no additional security.

Using this protection would require users to protect every handler in a specific way:

• Somehow pipe a notion of "user" into the middleware
• Somehow provide a concept of "action" to the token generation, which will have to depend on the endpoint receiving the form submission not the one generating it.
• Add a hidden form input to all forms with the injected CSRF token value
• Make sure GET, HEAD, OPTIONS and similar non-state-changing methods are indeed non-state-changing.

I am not particularly fond of this solution. The threat model of protecting some actions when a token is leaked for other actions seems quite odd and I don't think this extra security bit it's worth the extra cost.

Documentation

I would provide clear examples on how to properly use the currently existing functions (with code) and discourage their use in favor of the higher-level ones we are going to implement.

Note

The issue with the GET vs POST part is that we have to allow some form submissions to work (namely the non-state-changing ones like a search action) but in Go (*http.Request).FormValue retrieves the form value regardless the location (body or url).
This means we have to be very clear to our users on the need for filtering the method they accept.

There is no CSRF protection mechanism that I am aware of that would work for GET without breaking the application.

[rsc]

Merged #42168 into this one.

[rsc]

Some kind of automatic func Protect(h http.Handler) http.Handler sounds great.
But where is the key?
Or does this only work for single-server services?

[empijei]

If we use double-submit strategies (e.g. a cookie must match a header, or a cookie must match a form value) AFAIK there is no need to do key management or have server-side secrets, it is sufficient to generate random tokens for the cookie on the first visit and generate all forms tokens to match that cookie.

This is, for example, how Google's Angular protects from XSRF.

This, as you say, also has the benefit of relieving users from the burden of protecting and rotating keys.

Note: with this any server that runs on the origin the cookie was emitted for will be able to validate requests, so multiple servers behind a load balancer will be able to validate requests and generate valid forms and sessions with no communication.

If instead you need to make servers belonging to different origins generate valid forms (odd, but not impossible) you need to create a CORS endpoint with Allow-Credentials set to true and Allow-Origin set to the trusted third party that reflects the XSRF cookie in the response.
If needed we can also implement that and have an empty-by-default allow-list.

[rsc]

@empijei I don't actually understand what you are saying in the details, but the outcome seems to be that you are saying

func Protect(h http.Handler) http.Handler

is a sufficient API to provide users something that works well even for large, multi-server services.

Assuming that's true, then does anyone object to this very simple API?