From 00ec29d69669a9d957a88432590b6c0090cac6ca Mon Sep 17 00:00:00 2001 From: Drew Bednar Date: Sun, 9 Feb 2025 11:53:40 -0500 Subject: [PATCH] Added token based mitigation of CSRF --- README.md | 18 +++++++++++++++++- cmd/ratchetd/main.go | 8 ++++++-- go.mod | 1 + go.sum | 2 ++ internal/server/middleware.go | 14 ++++++++++++++ internal/server/routes.go | 18 +++++++++--------- internal/server/templates.go | 7 ++++++- ui/html/pages/create.go.tmpl | 2 ++ ui/html/pages/login.go.tmpl | 2 ++ ui/html/pages/signup.go.tmpl | 2 ++ ui/html/partials/nav.go.tmpl | 2 ++ 11 files changed, 63 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 148cada..0b5eeaa 100644 --- a/README.md +++ b/README.md @@ -263,4 +263,20 @@ It has: ## Local TLS Certs -https://github.com/FiloSottile/mkcert Can be used to create local certs with a trusted CA installed on the machine. \ No newline at end of file +https://github.com/FiloSottile/mkcert Can be used to create local certs with a trusted CA installed on the machine. + +## CSRF + +- http://www.gnucitizen.org/blog/csrf-demystified/ +- https://stackoverflow.com/questions/6412813/do-login-forms-need-tokens-against-csrf-attacks + +### Mitigations + +- **SameSite cookies**. In chapter 11 we had `Lax`. This means that the session cookie "won't be sent by the browser for POST, PUT, or DELETE requests. As long as you stick to only using POST, PUT, or DELETE for state changing requests (login, signup, create snippet) `Lax` will prevent CRSF attacks. + +- **Token-based mitigation** The [OWASP guidance](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#token-based-mitigation)is to use some form of token check if using TLS 1.2 or lower. Like session and password management you may be best served by using a known and tested third party package. This project uses `justinas/nosurf (`gorilla/csrf` is a valid alternative). Both use the [double submit cookie](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#double-submit-cookie) pattern. In this pattern a random CSRF token is generated and sent to the user in a CSRF cookie. This CSRF token is then added to a hidden field in each HTML form that is potentially vulnerable to CSRF. When the form is submitted, both packages use some middleware to check that the hidden field value and cookie value match. + +Due to the fact that no browser exists which supports TLS 1.3 and does not support SameSite cookies. If you only allow HTTPS requests to your application and enforce TLS 1.3 as the minimum TLS version, you don’t need to make any additional mitigation against CSRF attacks (like using the justinas/nosurf package). Just make sure that you always: + + Set SameSite=Lax or SameSite=Strict on the session cookie; and + Use the POST, PUT or DELETE HTTP methods for any state-changing requests. diff --git a/cmd/ratchetd/main.go b/cmd/ratchetd/main.go index 7f4b82a..f3d9aa0 100644 --- a/cmd/ratchetd/main.go +++ b/cmd/ratchetd/main.go @@ -56,8 +56,12 @@ func main() { // SessionManager sm := scs.New() sm.Store = sqlite3store.New(db) - // ratchet.Version = strings.TrimPrefix(version, "") - // ratchet.Commit = commit + // If you want to change the same sight cookie setting from Lax to Strict + // will block the session cookie being sent by the user’s browser for all cross-site usage + // including GET and HEAD. That means the cookie won't be sent when clicking on a link in another site + // for a GET request, so the user won't be treated as being "logged in" to the app even if they + // did in another tab + // sm.Cookie.SameSite = http.SameSiteStrictMode app := server.NewRatchetApp(logger, tc, db, sm) // these two elliptic curves have assembly implementations diff --git a/go.mod b/go.mod index b9f0978..9e780ff 100644 --- a/go.mod +++ b/go.mod @@ -8,5 +8,6 @@ require ( github.com/alexedwards/scs/sqlite3store v0.0.0-20240316134038-7e11d57e8885 // indirect github.com/alexedwards/scs/v2 v2.8.0 // indirect github.com/go-playground/form/v4 v4.2.1 // indirect + github.com/justinas/nosurf v1.1.1 // indirect golang.org/x/crypto v0.32.0 // indirect ) diff --git a/go.sum b/go.sum index 97670f1..ef2985e 100644 --- a/go.sum +++ b/go.sum @@ -5,6 +5,8 @@ github.com/alexedwards/scs/v2 v2.8.0/go.mod h1:ToaROZxyKukJKT/xLcVQAChi5k6+Pn1Gv github.com/go-playground/assert/v2 v2.0.1/go.mod h1:VDjEfimB/XKnb+ZQfWdccd7VUvScMdVu0Titje2rxJ4= github.com/go-playground/form/v4 v4.2.1 h1:HjdRDKO0fftVMU5epjPW2SOREcZ6/wLUzEobqUGJuPw= github.com/go-playground/form/v4 v4.2.1/go.mod h1:q1a2BY+AQUUzhl6xA/6hBetay6dEIhMHjgvJiGo6K7U= +github.com/justinas/nosurf v1.1.1 h1:92Aw44hjSK4MxJeMSyDa7jwuI9GR2J/JCQiaKvXXSlk= +github.com/justinas/nosurf v1.1.1/go.mod h1:ALpWdSbuNGy2lZWtyXdjkYv4edL23oSEgfBT1gPJ5BQ= github.com/mattn/go-sqlite3 v1.14.6/go.mod h1:NyWgC/yNuGj7Q9rpYnZvas74GogHl5/Z4A/KQRfk6bU= github.com/mattn/go-sqlite3 v1.14.24 h1:tpSp2G2KyMnnQu99ngJ47EIkWVmliIizyZBfPrBWDRM= github.com/mattn/go-sqlite3 v1.14.24/go.mod h1:Uh1q+B4BYcTPb+yiD3kU8Ct7aC0hY9fxUwlHK0RXw+Y= diff --git a/internal/server/middleware.go b/internal/server/middleware.go index e3541b1..57c2c2d 100644 --- a/internal/server/middleware.go +++ b/internal/server/middleware.go @@ -6,6 +6,7 @@ import ( "net/http" "github.com/alexedwards/scs/v2" + "github.com/justinas/nosurf" ) // https://owasp.org/www-project-secure-headers/ guidance @@ -122,3 +123,16 @@ func RequireAuthenticationMiddleware(next http.Handler, sm *scs.SessionManager) next.ServeHTTP(w, r) }) } + +// NoSurfMiddleware uses the noSurf package to create a customized CSRF cookie +// with the Secure, Path and HttpOnly attributes set +func NoSurfMiddleware(next http.Handler) http.Handler { + csrfHandler := nosurf.New(next) + csrfHandler.SetBaseCookie(http.Cookie{ + HttpOnly: true, + Path: "/", + Secure: true, + }) + + return csrfHandler +} diff --git a/internal/server/routes.go b/internal/server/routes.go index 6c0bf40..f25e6dc 100644 --- a/internal/server/routes.go +++ b/internal/server/routes.go @@ -23,21 +23,21 @@ func addRoutes(mux *http.ServeMux, // resulting in this route requiring an exact match on "/" only // You can only include one HTTP method in a route pattern if you choose // GET will match GET & HEAD http request methods - mux.Handle("GET /{$}", sm.LoadAndSave(handleHome(logger, tc, sm, snippetService))) // might be time to swith to github.com/justinas/alice dynamic chain - mux.Handle("GET /snippet/view/{id}", sm.LoadAndSave(handleSnippetView(logger, tc, sm, snippetService))) - mux.Handle("GET /snippet/create", sm.LoadAndSave(RequireAuthenticationMiddleware(handleSnippetCreateGet(tc, sm), sm))) - mux.Handle("POST /snippet/create", sm.LoadAndSave(RequireAuthenticationMiddleware(handleSnippetCreatePost(logger, tc, fd, sm, snippetService), sm))) + mux.Handle("GET /{$}", sm.LoadAndSave(NoSurfMiddleware(handleHome(logger, tc, sm, snippetService)))) // might be time to swith to github.com/justinas/alice dynamic chain + mux.Handle("GET /snippet/view/{id}", sm.LoadAndSave(NoSurfMiddleware(handleSnippetView(logger, tc, sm, snippetService)))) + mux.Handle("GET /snippet/create", sm.LoadAndSave(NoSurfMiddleware(RequireAuthenticationMiddleware(handleSnippetCreateGet(tc, sm), sm)))) + mux.Handle("POST /snippet/create", sm.LoadAndSave(NoSurfMiddleware(RequireAuthenticationMiddleware(handleSnippetCreatePost(logger, tc, fd, sm, snippetService), sm)))) // mux.Handle("/something", handleSomething(logger, config)) // mux.Handle("/healthz", handleHealthzPlease(logger)) // mux.Handle("/", http.NotFoundHandler()) - mux.Handle("GET /user/signup", sm.LoadAndSave(handleUserSignupGet(tc, sm))) - mux.Handle("POST /user/signup", sm.LoadAndSave(handleUserSignupPost(logger, tc, fd, sm, userService))) - mux.Handle("GET /user/login", sm.LoadAndSave(handleUserLoginGet(tc, sm))) - mux.Handle("POST /user/login", sm.LoadAndSave(handleUserLoginPost(logger, tc, sm, fd, userService))) + mux.Handle("GET /user/signup", sm.LoadAndSave(NoSurfMiddleware(handleUserSignupGet(tc, sm)))) + mux.Handle("POST /user/signup", sm.LoadAndSave(NoSurfMiddleware(handleUserSignupPost(logger, tc, fd, sm, userService)))) + mux.Handle("GET /user/login", sm.LoadAndSave(NoSurfMiddleware(handleUserLoginGet(tc, sm)))) + mux.Handle("POST /user/login", sm.LoadAndSave(NoSurfMiddleware(handleUserLoginPost(logger, tc, sm, fd, userService)))) // Requires auth - mux.Handle("POST /user/logout", sm.LoadAndSave(RequireAuthenticationMiddleware(handleUserLogoutPost(logger, sm), sm))) + mux.Handle("POST /user/logout", sm.LoadAndSave(NoSurfMiddleware(RequireAuthenticationMiddleware(handleUserLogoutPost(logger, sm), sm)))) return mux } diff --git a/internal/server/templates.go b/internal/server/templates.go index a9223dc..8650812 100644 --- a/internal/server/templates.go +++ b/internal/server/templates.go @@ -11,6 +11,7 @@ import ( "git.runcible.io/learning/ratchet/internal/model" "github.com/alexedwards/scs/v2" + "github.com/justinas/nosurf" ) // Define a templateData type to act as the holding structure for @@ -24,13 +25,17 @@ type templateData struct { Form any Flash string IsAuthenticated bool + CSRFToken string } // newTemplateData is useful to inject default values. Example CSRF tokens for forms. func newTemplateData(r *http.Request, sm *scs.SessionManager) templateData { return templateData{CurrentYear: time.Now().Year(), Flash: sm.PopString(r.Context(), "flash"), - IsAuthenticated: isAuthenticated(r, sm)} + IsAuthenticated: isAuthenticated(r, sm), + // added to every page because the form for logout can appear on every page + CSRFToken: nosurf.Token(r), + } } // TEMPLATE FUNCTIONS diff --git a/ui/html/pages/create.go.tmpl b/ui/html/pages/create.go.tmpl index e135aa4..8773ea0 100644 --- a/ui/html/pages/create.go.tmpl +++ b/ui/html/pages/create.go.tmpl @@ -2,6 +2,8 @@ {{define "main"}}
+ +
+ {{range .Form.NonFieldErrors}} diff --git a/ui/html/pages/signup.go.tmpl b/ui/html/pages/signup.go.tmpl index 13f0527..de9b8e6 100644 --- a/ui/html/pages/signup.go.tmpl +++ b/ui/html/pages/signup.go.tmpl @@ -2,6 +2,8 @@ {{define "main"}} + +
{{with .Form.FieldErrors.name}} diff --git a/ui/html/partials/nav.go.tmpl b/ui/html/partials/nav.go.tmpl index ed22ab2..6f121b8 100644 --- a/ui/html/partials/nav.go.tmpl +++ b/ui/html/partials/nav.go.tmpl @@ -9,6 +9,8 @@
{{ if .IsAuthenticated }} + + {{ else }}