From 8bd04bdd3a6ceca64dfd04b49035cc16cbe2b2e1 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 29 Jan 2025 20:44:01 +0000 Subject: [PATCH] go.mod: bump gorilla/csrf for security fix (#14822) For https://github.com/gorilla/csrf/commit/9dd6af1f6d30fc79fb0d972394deebdabad6b5eb Update client/web and safeweb to correctly signal to the csrf middleware whether the request is being served over TLS. This determines whether Origin and Referer header checks are strictly enforced. The gorilla library previously did not enforce these checks due to a logic bug based on erroneous use of the net/http.Request API. The patch to fix this also inverts the library behavior to presume that every request is being served over TLS, necessitating these changes. Updates tailscale/corp#25340 Signed-off-by: Patrick O'Doherty Co-authored-by: Patrick O'Doherty --- client/web/web.go | 16 +++++++++++++--- go.mod | 2 +- go.sum | 4 ++-- safeweb/http.go | 6 ++++++ 4 files changed, 22 insertions(+), 6 deletions(-) diff --git a/client/web/web.go b/client/web/web.go index 4e4866923..3a7feea40 100644 --- a/client/web/web.go +++ b/client/web/web.go @@ -211,15 +211,25 @@ func NewServer(opts ServerOpts) (s *Server, err error) { // The client is secured by limiting the interface it listens on, // or by authenticating requests before they reach the web client. csrfProtect := csrf.Protect(s.csrfKey(), csrf.Secure(false)) + + // signal to the CSRF middleware that the request is being served over + // plaintext HTTP to skip TLS-only header checks. + withSetPlaintext := func(h http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + r = csrf.PlaintextHTTPRequest(r) + h.ServeHTTP(w, r) + }) + } + switch s.mode { case LoginServerMode: - s.apiHandler = csrfProtect(http.HandlerFunc(s.serveLoginAPI)) + s.apiHandler = csrfProtect(withSetPlaintext(http.HandlerFunc(s.serveLoginAPI))) metric = "web_login_client_initialization" case ReadOnlyServerMode: - s.apiHandler = csrfProtect(http.HandlerFunc(s.serveLoginAPI)) + s.apiHandler = csrfProtect(withSetPlaintext(http.HandlerFunc(s.serveLoginAPI))) metric = "web_readonly_client_initialization" case ManageServerMode: - s.apiHandler = csrfProtect(http.HandlerFunc(s.serveAPI)) + s.apiHandler = csrfProtect(withSetPlaintext(http.HandlerFunc(s.serveAPI))) metric = "web_client_initialization" } diff --git a/go.mod b/go.mod index 8e52a9ab3..6a5080585 100644 --- a/go.mod +++ b/go.mod @@ -265,7 +265,7 @@ require ( github.com/gordonklaus/ineffassign v0.1.0 // indirect github.com/goreleaser/chglog v0.5.0 // indirect github.com/goreleaser/fileglob v1.3.0 // indirect - github.com/gorilla/csrf v1.7.2 + github.com/gorilla/csrf v1.7.3-0.20250123201450-9dd6af1f6d30 github.com/gostaticanalysis/analysisutil v0.7.1 // indirect github.com/gostaticanalysis/comment v1.4.2 // indirect github.com/gostaticanalysis/forcetypeassert v0.1.0 // indirect diff --git a/go.sum b/go.sum index c1c82ad77..c38c96029 100644 --- a/go.sum +++ b/go.sum @@ -529,8 +529,8 @@ github.com/goreleaser/fileglob v1.3.0 h1:/X6J7U8lbDpQtBvGcwwPS6OpzkNVlVEsFUVRx9+ github.com/goreleaser/fileglob v1.3.0/go.mod h1:Jx6BoXv3mbYkEzwm9THo7xbr5egkAraxkGorbJb4RxU= github.com/goreleaser/nfpm/v2 v2.33.1 h1:EkdAzZyVhAI9JC1vjmjjbmnNzyH1J6Cu4JCsA7YcQuc= github.com/goreleaser/nfpm/v2 v2.33.1/go.mod h1:8wwWWvJWmn84xo/Sqiv0aMvEGTHlHZTXTEuVSgQpkIM= -github.com/gorilla/csrf v1.7.2 h1:oTUjx0vyf2T+wkrx09Trsev1TE+/EbDAeHtSTbtC2eI= -github.com/gorilla/csrf v1.7.2/go.mod h1:F1Fj3KG23WYHE6gozCmBAezKookxbIvUJT+121wTuLk= +github.com/gorilla/csrf v1.7.3-0.20250123201450-9dd6af1f6d30 h1:fiJdrgVBkjZ5B1HJ2WQwNOaXB+QyYcNXTA3t1XYLz0M= +github.com/gorilla/csrf v1.7.3-0.20250123201450-9dd6af1f6d30/go.mod h1:F1Fj3KG23WYHE6gozCmBAezKookxbIvUJT+121wTuLk= github.com/gorilla/securecookie v1.1.2 h1:YCIWL56dvtr73r6715mJs5ZvhtnY73hBvEF8kXD8ePA= github.com/gorilla/securecookie v1.1.2/go.mod h1:NfCASbcHqRSY+3a8tlWJwsQap2VX5pwzwo4h3eOamfo= github.com/gostaticanalysis/analysisutil v0.7.1 h1:ZMCjoue3DtDWQ5WyU16YbjbQEQ3VuzwxALrpYd+HeKk= diff --git a/safeweb/http.go b/safeweb/http.go index 983ff2fad..143c4dcee 100644 --- a/safeweb/http.go +++ b/safeweb/http.go @@ -318,6 +318,12 @@ func checkHandlerType(apiPattern, browserPattern string) handlerType { } func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { + // if we are not in a secure context, signal to the CSRF middleware that + // TLS-only header checks should be skipped + if !s.Config.SecureContext { + r = csrf.PlaintextHTTPRequest(r) + } + _, bp := s.BrowserMux.Handler(r) _, ap := s.APIMux.Handler(r) switch {