From 4b50158aba1b9683db9b198ddc61436713e778f8 Mon Sep 17 00:00:00 2001 From: Tom <33637037+tflyons@users.noreply.github.com> Date: Mon, 7 Oct 2019 19:23:26 -0700 Subject: [PATCH] feature: Support SameSite option (#123) * Add SameSite with build constraied * Update options test * Fix after feedback * Add docs --- README.md | 28 ++++++++ csrf.go | 18 +++++ options.go | 23 ++++++ options_test.go | 5 ++ store.go | 4 ++ store_legacy.go | 86 +++++++++++++++++++++++ store_legacy_test.go | 162 +++++++++++++++++++++++++++++++++++++++++++ store_test.go | 36 +++++++++- 8 files changed, 360 insertions(+), 2 deletions(-) create mode 100644 store_legacy.go create mode 100644 store_legacy_test.go diff --git a/README.md b/README.md index 2129446..3c7b533 100644 --- a/README.md +++ b/README.md @@ -42,6 +42,7 @@ go get github.com/gorilla/csrf - [HTML Forms](#html-forms) - [JavaScript Apps](#javascript-applications) - [Google App Engine](#google-app-engine) +- [Setting SameSite](#setting-samesite) - [Setting Options](#setting-options) gorilla/csrf is easy to use: add the middleware to your router with @@ -267,6 +268,30 @@ Note: You can ignore this if you're using the [second-generation](https://cloud.google.com/appengine/docs/go/) Go runtime on App Engine (Go 1.11 and above). +### Setting SameSite + +Go 1.11 introduced the option to set the SameSite attribute in cookies. This is +valuable if a developer wants to instruct a browser to not include cookies during +a cross site request. SameSiteStrictMode prevents all cross site requests from including +the cookie. SameSiteLaxMode prevents CSRF prone requests (POST) from including the cookie +but allows the cookie to be included in GET requests to support external linking. + +```go +func main() { + CSRF := csrf.Protect( + []byte("a-32-byte-long-key-goes-here"), + // instruct the browser to never send cookies during cross site requests + csrf.SameSite(csrf.SameSiteStrictMode), + ) + + r := mux.NewRouter() + r.HandleFunc("/signup", GetSignupForm) + r.HandleFunc("/signup/post", PostSignupForm) + + http.ListenAndServe(":8000", CSRF(r)) +} +``` + ### Setting Options What about providing your own error handler and changing the HTTP header the @@ -314,6 +339,9 @@ Getting CSRF protection right is important, so here's some background: - Cookies are authenticated and based on the [securecookie](https://github.com/gorilla/securecookie) library. They're also Secure (issued over HTTPS only) and are HttpOnly by default, because sane defaults are important. +- Cookie SameSite attribute (prevents cookies from being sent by a browser + during cross site requests) are not set by default to maintain backwards compatibility + for legacy systems. The SameSite attribute can be set with the SameSite option. - Go's `crypto/rand` library is used to generate the 32 byte (256 bit) tokens and the one-time-pad used for masking them. diff --git a/csrf.go b/csrf.go index 8f78653..76352de 100644 --- a/csrf.go +++ b/csrf.go @@ -52,6 +52,22 @@ var ( ErrBadToken = errors.New("CSRF token invalid") ) +// SameSiteMode allows a server to define a cookie attribute making it impossible for +// the browser to send this cookie along with cross-site requests. The main +// goal is to mitigate the risk of cross-origin information leakage, and provide +// some protection against cross-site request forgery attacks. +// +// See https://tools.ietf.org/html/draft-ietf-httpbis-cookie-same-site-00 for details. +type SameSiteMode int + +// SameSite options +const ( + SameSiteDefaultMode SameSiteMode = iota + 1 + SameSiteLaxMode + SameSiteStrictMode + SameSiteNoneMode +) + type csrf struct { h http.Handler sc *securecookie.SecureCookie @@ -68,6 +84,7 @@ type options struct { // http.Cookie field instead of the "correct" HTTPOnly name that golint suggests. HttpOnly bool Secure bool + SameSite SameSiteMode RequestHeader string FieldName string ErrorHandler http.Handler @@ -166,6 +183,7 @@ func Protect(authKey []byte, opts ...Option) func(http.Handler) http.Handler { maxAge: cs.opts.MaxAge, secure: cs.opts.Secure, httpOnly: cs.opts.HttpOnly, + sameSite: cs.opts.SameSite, path: cs.opts.Path, domain: cs.opts.Domain, sc: cs.sc, diff --git a/options.go b/options.go index 9219147..da82167 100644 --- a/options.go +++ b/options.go @@ -61,6 +61,26 @@ func HttpOnly(h bool) Option { } } +// SameSite sets the cookie SameSite attribute. Defaults to blank to maintain +// backwards compatibility, however, Strict is recommended. +// +// SameSite(SameSiteStrictMode) will prevent the cookie from being sent by the +// browser to the target site in all cross-site browsing context, even when +// following a regular link (GET request). +// +// SameSite(SameSiteLaxMode) provides a reasonable balance between security and +// usability for websites that want to maintain user's logged-in session after +// the user arrives from an external link. The session cookie would be allowed +// when following a regular link from an external website while blocking it in +// CSRF-prone request methods (e.g. POST). +// +// This option is only available for go 1.11+. +func SameSite(s SameSiteMode) Option { + return func(cs *csrf) { + cs.opts.SameSite = s + } +} + // ErrorHandler allows you to change the handler called when CSRF request // processing encounters an invalid token or request. A typical use would be to // provide a handler that returns a static HTML file with a HTTP 403 status. By @@ -132,6 +152,9 @@ func parseOptions(h http.Handler, opts ...Option) *csrf { cs.opts.Secure = true cs.opts.HttpOnly = true + // Default to blank to maintain backwards compatibility + cs.opts.SameSite = SameSiteDefaultMode + // Default; only override this if the package user explicitly calls MaxAge(0) cs.opts.MaxAge = defaultAge diff --git a/options_test.go b/options_test.go index 1fe40bf..d133fd1 100644 --- a/options_test.go +++ b/options_test.go @@ -24,6 +24,7 @@ func TestOptions(t *testing.T) { Path(path), HttpOnly(false), Secure(false), + SameSite(SameSiteStrictMode), RequestHeader(header), FieldName(field), ErrorHandler(http.HandlerFunc(errorHandler)), @@ -53,6 +54,10 @@ func TestOptions(t *testing.T) { t.Errorf("Secure not set correctly: got %v want %v", cs.opts.Secure, false) } + if cs.opts.SameSite != SameSiteStrictMode { + t.Errorf("SameSite not set correctly: got %v want %v", cs.opts.SameSite, SameSiteStrictMode) + } + if cs.opts.RequestHeader != header { t.Errorf("RequestHeader not set correctly: got %v want %v", cs.opts.RequestHeader, header) } diff --git a/store.go b/store.go index 39f47ad..f7997fc 100644 --- a/store.go +++ b/store.go @@ -1,3 +1,5 @@ +// +build go1.11 + package csrf import ( @@ -28,6 +30,7 @@ type cookieStore struct { path string domain string sc *securecookie.SecureCookie + sameSite SameSiteMode } // Get retrieves a CSRF token from the session cookie. It returns an empty token @@ -63,6 +66,7 @@ func (cs *cookieStore) Save(token []byte, w http.ResponseWriter) error { MaxAge: cs.maxAge, HttpOnly: cs.httpOnly, Secure: cs.secure, + SameSite: http.SameSite(cs.sameSite), Path: cs.path, Domain: cs.domain, } diff --git a/store_legacy.go b/store_legacy.go new file mode 100644 index 0000000..b211164 --- /dev/null +++ b/store_legacy.go @@ -0,0 +1,86 @@ +// +build !go1.11 +// file for compatibility with go versions prior to 1.11 + +package csrf + +import ( + "net/http" + "time" + + "github.com/gorilla/securecookie" +) + +// store represents the session storage used for CSRF tokens. +type store interface { + // Get returns the real CSRF token from the store. + Get(*http.Request) ([]byte, error) + // Save stores the real CSRF token in the store and writes a + // cookie to the http.ResponseWriter. + // For non-cookie stores, the cookie should contain a unique (256 bit) ID + // or key that references the token in the backend store. + // csrf.GenerateRandomBytes is a helper function for generating secure IDs. + Save(token []byte, w http.ResponseWriter) error +} + +// cookieStore is a signed cookie session store for CSRF tokens. +type cookieStore struct { + name string + maxAge int + secure bool + httpOnly bool + path string + domain string + sc *securecookie.SecureCookie + sameSite SameSiteMode +} + +// Get retrieves a CSRF token from the session cookie. It returns an empty token +// if decoding fails (e.g. HMAC validation fails or the named cookie doesn't exist). +func (cs *cookieStore) Get(r *http.Request) ([]byte, error) { + // Retrieve the cookie from the request + cookie, err := r.Cookie(cs.name) + if err != nil { + return nil, err + } + + token := make([]byte, tokenLength) + // Decode the HMAC authenticated cookie. + err = cs.sc.Decode(cs.name, cookie.Value, &token) + if err != nil { + return nil, err + } + + return token, nil +} + +// Save stores the CSRF token in the session cookie. +func (cs *cookieStore) Save(token []byte, w http.ResponseWriter) error { + // Generate an encoded cookie value with the CSRF token. + encoded, err := cs.sc.Encode(cs.name, token) + if err != nil { + return err + } + + cookie := &http.Cookie{ + Name: cs.name, + Value: encoded, + MaxAge: cs.maxAge, + HttpOnly: cs.httpOnly, + Secure: cs.secure, + Path: cs.path, + Domain: cs.domain, + } + + // Set the Expires field on the cookie based on the MaxAge + // If MaxAge <= 0, we don't set the Expires attribute, making the cookie + // session-only. + if cs.maxAge > 0 { + cookie.Expires = time.Now().Add( + time.Duration(cs.maxAge) * time.Second) + } + + // Write the authenticated cookie to the response. + http.SetCookie(w, cookie) + + return nil +} diff --git a/store_legacy_test.go b/store_legacy_test.go new file mode 100644 index 0000000..a3a4ac1 --- /dev/null +++ b/store_legacy_test.go @@ -0,0 +1,162 @@ +// +build !go1.11 +// file for compatibility with go versions prior to 1.11 + +package csrf + +import ( + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/pkg/errors" + + "github.com/gorilla/securecookie" +) + +// Check store implementations +var _ store = &cookieStore{} + +// brokenSaveStore is a CSRF store that cannot, well, save. +type brokenSaveStore struct { + store +} + +func (bs *brokenSaveStore) Get(*http.Request) ([]byte, error) { + // Generate an invalid token so we can progress to our Save method + return generateRandomBytes(24) +} + +func (bs *brokenSaveStore) Save(realToken []byte, w http.ResponseWriter) error { + return errors.New("test error") +} + +// Tests for failure if the middleware can't save to the Store. +func TestStoreCannotSave(t *testing.T) { + s := http.NewServeMux() + bs := &brokenSaveStore{} + s.HandleFunc("/", testHandler) + p := Protect(testKey, setStore(bs))(s) + + r, err := http.NewRequest("GET", "/", nil) + if err != nil { + t.Fatal(err) + } + + rr := httptest.NewRecorder() + p.ServeHTTP(rr, r) + + if rr.Code != http.StatusForbidden { + t.Fatalf("broken store did not set an error status: got %v want %v", + rr.Code, http.StatusForbidden) + } + + if c := rr.Header().Get("Set-Cookie"); c != "" { + t.Fatalf("broken store incorrectly set a cookie: got %v want %v", + c, "") + } + +} + +// TestCookieDecode tests that an invalid cookie store returns a decoding error. +func TestCookieDecode(t *testing.T) { + r, err := http.NewRequest("GET", "/", nil) + if err != nil { + t.Fatal(err) + } + + var age = 3600 + + // Test with a nil hash key + sc := securecookie.New(nil, nil) + sc.MaxAge(age) + st := &cookieStore{cookieName, age, true, true, "", "", sc, SameSiteDefaultMode} + + // Set a fake cookie value so r.Cookie passes. + r.Header.Set("Cookie", fmt.Sprintf("%s=%s", cookieName, "notacookie")) + + _, err = st.Get(r) + if err == nil { + t.Fatal("cookiestore did not report an invalid hashkey on decode") + } +} + +// TestCookieEncode tests that an invalid cookie store returns an encoding error. +func TestCookieEncode(t *testing.T) { + var age = 3600 + + // Test with a nil hash key + sc := securecookie.New(nil, nil) + sc.MaxAge(age) + st := &cookieStore{cookieName, age, true, true, "", "", sc, SameSiteDefaultMode} + + rr := httptest.NewRecorder() + + err := st.Save(nil, rr) + if err == nil { + t.Fatal("cookiestore did not report an invalid hashkey on encode") + } +} + +// TestMaxAgeZero tests that setting MaxAge(0) does not set the Expires +// attribute on the cookie. +func TestMaxAgeZero(t *testing.T) { + var age = 0 + + s := http.NewServeMux() + s.HandleFunc("/", testHandler) + + r, err := http.NewRequest("GET", "/", nil) + if err != nil { + t.Fatal(err) + } + + rr := httptest.NewRecorder() + p := Protect(testKey, MaxAge(age))(s) + p.ServeHTTP(rr, r) + + if rr.Code != http.StatusOK { + t.Fatalf("middleware failed to pass to the next handler: got %v want %v", + rr.Code, http.StatusOK) + } + + if rr.Header().Get("Set-Cookie") == "" { + t.Fatalf("cookie not set: got %q", rr.Header().Get("Set-Cookie")) + } + + cookie := rr.Header().Get("Set-Cookie") + if !strings.Contains(cookie, "HttpOnly") || strings.Contains(cookie, "Expires") { + t.Fatalf("cookie incorrectly has the Expires attribute set: got %q", cookie) + } +} + +// TestSameSizeSet tests that setting SameSite Option does not set the SameSite +// attribute on the cookie in legacy systems. +func TestSameSizeSet(t *testing.T) { + s := http.NewServeMux() + s.HandleFunc("/", testHandler) + + r, err := http.NewRequest("GET", "/", nil) + if err != nil { + t.Fatal(err) + } + + rr := httptest.NewRecorder() + p := Protect(testKey, SameSite(SameSiteStrictMode))(s) + p.ServeHTTP(rr, r) + + if rr.Code != http.StatusOK { + t.Fatalf("middleware failed to pass to the next handler: got %v want %v", + rr.Code, http.StatusOK) + } + + if rr.Header().Get("Set-Cookie") == "" { + t.Fatalf("cookie not set: got %q", rr.Header().Get("Set-Cookie")) + } + + cookie := rr.Header().Get("Set-Cookie") + if strings.Contains(cookie, "SameSite") { + t.Fatalf("cookie incorrectly has the SameSite attribute set: got %q", cookie) + } +} diff --git a/store_test.go b/store_test.go index e2836e5..cfa0534 100644 --- a/store_test.go +++ b/store_test.go @@ -1,3 +1,5 @@ +// +build go1.11 + package csrf import ( @@ -68,7 +70,7 @@ func TestCookieDecode(t *testing.T) { // Test with a nil hash key sc := securecookie.New(nil, nil) sc.MaxAge(age) - st := &cookieStore{cookieName, age, true, true, "", "", sc} + st := &cookieStore{cookieName, age, true, true, "", "", sc, SameSiteDefaultMode} // Set a fake cookie value so r.Cookie passes. r.Header.Set("Cookie", fmt.Sprintf("%s=%s", cookieName, "notacookie")) @@ -86,7 +88,7 @@ func TestCookieEncode(t *testing.T) { // Test with a nil hash key sc := securecookie.New(nil, nil) sc.MaxAge(age) - st := &cookieStore{cookieName, age, true, true, "", "", sc} + st := &cookieStore{cookieName, age, true, true, "", "", sc, SameSiteDefaultMode} rr := httptest.NewRecorder() @@ -127,3 +129,33 @@ func TestMaxAgeZero(t *testing.T) { t.Fatalf("cookie incorrectly has the Expires attribute set: got %q", cookie) } } + +// TestSameSizeSet tests that setting SameSite Option sets the SameSite +// attribute on the cookie in post go1.11 systems. +func TestSameSizeSet(t *testing.T) { + s := http.NewServeMux() + s.HandleFunc("/", testHandler) + + r, err := http.NewRequest("GET", "/", nil) + if err != nil { + t.Fatal(err) + } + + rr := httptest.NewRecorder() + p := Protect(testKey, SameSite(SameSiteStrictMode))(s) + p.ServeHTTP(rr, r) + + if rr.Code != http.StatusOK { + t.Fatalf("middleware failed to pass to the next handler: got %v want %v", + rr.Code, http.StatusOK) + } + + if rr.Header().Get("Set-Cookie") == "" { + t.Fatalf("cookie not set: got %q", rr.Header().Get("Set-Cookie")) + } + + cookie := rr.Header().Get("Set-Cookie") + if !strings.Contains(cookie, "SameSite") { + t.Fatalf("cookie incorrectly does not have the SameSite attribute set: got %q", cookie) + } +}