From a8dc606beb207e09ae511dc9481f94096be015db Mon Sep 17 00:00:00 2001 From: Victor Vrantchan Date: Wed, 22 Jul 2020 23:22:03 -0400 Subject: [PATCH] Add CSRF middleware to all pages The frontend server now runs a CSRF middleware on all paths except /assets/. To enable CSRF the -csrf_key flag must be set. Instead of enforcing the flag is set, I opted to log that CSRF is disabled. Not running CSRF means I can test with curl. In the future I might enforce that the flag must be set in release builds. --- cmd/micromdm/micromdm.go | 7 +++++ cmd/micromdm/server.go | 7 +++-- go.mod | 2 +- go.sum | 4 +++ pkg/frontend/frontend.go | 55 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 72 insertions(+), 3 deletions(-) diff --git a/cmd/micromdm/micromdm.go b/cmd/micromdm/micromdm.go index 28af5434..957645f1 100644 --- a/cmd/micromdm/micromdm.go +++ b/cmd/micromdm/micromdm.go @@ -35,6 +35,10 @@ type cliFlags struct { siteName string http string pidfile string + + csrfKey string + csrfCookieName string + csrfFieldName string } func micromdm(args []string, stdin io.Reader, stdout, stderr io.Writer) int { @@ -50,6 +54,9 @@ func micromdm(args []string, stdin io.Reader, stdout, stderr io.Writer) int { rootfs.BoolVar(&cli.debug, "debug", false, "Allow debug level") rootfs.StringVar(&cli.siteName, "site_name", "Acme", "Name of the site as it would appear in the top left of the HTML UI") rootfs.StringVar(&cli.http, "http", "localhost:9000", "HTTP service address") + rootfs.StringVar(&cli.csrfKey, "csrf_key", "", "32 byte long key") + rootfs.StringVar(&cli.csrfCookieName, "csrf_cookie_name", "micromdm_csrf", "Name of CSRF Cookie") + rootfs.StringVar(&cli.csrfFieldName, "csrf_field_name", "micromdm.csrf", "Name of CSRF field name in HTML input") // default output is os.Stderr. // setting the output and flag.ContinueOnError overrides allows testing usage. diff --git a/cmd/micromdm/server.go b/cmd/micromdm/server.go index 84f412a2..0862debc 100644 --- a/cmd/micromdm/server.go +++ b/cmd/micromdm/server.go @@ -11,8 +11,11 @@ type server struct { func ui(f *cliFlags, logger log.Logger) (*frontend.Server, error) { return frontend.New(frontend.Config{ - Logger: logger, - SiteName: f.siteName, + Logger: logger, + SiteName: f.siteName, + CSRFKey: []byte(f.csrfKey), + CSRFCookieName: f.csrfCookieName, + CSRFFieldName: f.csrfFieldName, }) } diff --git a/go.mod b/go.mod index da420e62..40595491 100644 --- a/go.mod +++ b/go.mod @@ -6,13 +6,13 @@ require ( crawshaw.io/sqlite v0.3.2 github.com/felixge/httpsnoop v1.0.1 github.com/go-kit/kit v0.10.0 + github.com/gorilla/csrf v1.7.0 github.com/gorilla/mux v1.7.4 github.com/jackc/pgx/v4 v4.7.2 github.com/oklog/run v1.1.0 github.com/oklog/ulid v1.3.1 // indirect github.com/oklog/ulid/v2 v2.0.2 github.com/peterbourgon/ff/v3 v3.0.0 - github.com/pkg/errors v0.9.1 // indirect golang.org/x/crypto v0.0.0-20200709230013-948cd5f35899 rsc.io/goversion v1.2.0 ) diff --git a/go.sum b/go.sum index f265bfcb..0cf24253 100644 --- a/go.sum +++ b/go.sum @@ -99,10 +99,14 @@ github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm4 github.com/google/uuid v1.0.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/gopherjs/gopherjs v0.0.0-20181017120253-0766667cb4d1/go.mod h1:wJfORRmW1u3UXTncJ5qlYoELFm8eSnnEO6hX4iZ3EWY= github.com/gorilla/context v1.1.1/go.mod h1:kBGZzfjB9CEq2AlWe17Uuf7NDRt0dE0s8S51q0aT7Yg= +github.com/gorilla/csrf v1.7.0 h1:mMPjV5/3Zd460xCavIkppUdvnl5fPXMpv2uz2Zyg7/Y= +github.com/gorilla/csrf v1.7.0/go.mod h1:+a/4tCmqhG6/w4oafeAZ9pEa3/NZOWYVbD9fV0FwIQA= github.com/gorilla/mux v1.6.2/go.mod h1:1lud6UwP+6orDFRuTfBEV8e9/aOM/c4fVVCaMa2zaAs= github.com/gorilla/mux v1.7.3/go.mod h1:1lud6UwP+6orDFRuTfBEV8e9/aOM/c4fVVCaMa2zaAs= github.com/gorilla/mux v1.7.4 h1:VuZ8uybHlWmqV03+zRzdwKL4tUnIp1MAQtp1mIFE1bc= github.com/gorilla/mux v1.7.4/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So= +github.com/gorilla/securecookie v1.1.1 h1:miw7JPhV+b/lAHSXz4qd/nN9jRiAFV5FwjeKyCS8BvQ= +github.com/gorilla/securecookie v1.1.1/go.mod h1:ra0sb63/xPlUeL+yeDciTfxMRAA+MP+HVt/4epWDjd4= github.com/gorilla/websocket v0.0.0-20170926233335-4201258b820c/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ= github.com/grpc-ecosystem/go-grpc-middleware v1.0.1-0.20190118093823-f849b5445de4/go.mod h1:FiyG127CGDf3tlThmgyCl78X/SZQqEOJBCDaAfeWzPs= github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0/go.mod h1:8NvIoxWQoOIhqOTXgfV/d3M/q6VIi02HzZEHgUlZvzk= diff --git a/pkg/frontend/frontend.go b/pkg/frontend/frontend.go index b7983dd5..bbe5fc5d 100644 --- a/pkg/frontend/frontend.go +++ b/pkg/frontend/frontend.go @@ -9,9 +9,11 @@ import ( "net/http" "path/filepath" "runtime/debug" + "strings" "sync" "text/template" + "github.com/gorilla/csrf" "github.com/gorilla/mux" "micromdm.io/v2/pkg/log" @@ -66,12 +68,20 @@ type Server struct { templates map[string]*template.Template siteName string + + csrfKey []byte + csrfCookieName string + csrfFieldName string } // Config parameters to create a new Server. type Config struct { Logger log.Logger SiteName string + + CSRFKey []byte + CSRFCookieName string + CSRFFieldName string } // New creates a Server. @@ -80,10 +90,15 @@ func New(config Config) (*Server, error) { r: mux.NewRouter(), templates: make(map[string]*template.Template), siteName: config.SiteName, + + csrfKey: config.CSRFKey, + csrfFieldName: config.CSRFFieldName, + csrfCookieName: config.CSRFCookieName, } srv.r.Use( log.HTTP(config.Logger), // HTTP logging middleware. + srv.csrf, // CSRF protection. srv.recoverPanic, // convert any panic into 500 errors. ) @@ -221,6 +236,46 @@ func (srv *Server) recoverPanic(next http.Handler) http.Handler { }) } +func csrfDisabled(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + logger := log.FromContext(r.Context()) + log.Info(logger).Log("msg", "CSRF Protection disabled", "reason", "CSRF key not set.") + next.ServeHTTP(w, r) + }) +} + +func (srv *Server) csrf(next http.Handler) http.Handler { + mw := csrfDisabled + if string(srv.csrfKey) != "" { + mw = csrf.Protect( + srv.csrfKey, + csrf.CookieName(srv.csrfCookieName), + csrf.FieldName(srv.csrfFieldName), + csrf.ErrorHandler(http.HandlerFunc(srv.csrfErrorHandler)), + ) + } + + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Excluding assets from setting the cookie. + // Refactor into a switch or callback? if adding other paths. + if strings.HasPrefix(r.URL.Path, "/assets/") { + next.ServeHTTP(w, r) + return + } + + mw(next).ServeHTTP(w, r) + }) +} + +func (srv *Server) csrfErrorHandler(w http.ResponseWriter, r *http.Request) { + switch err := csrf.FailureReason(r); { + // TODO: add 403 cases + default: + srv.Fail(r.Context(), w, err, "msg", "csrf.Protect encountered an error") + return + } +} + func (srv *Server) notFound(w http.ResponseWriter, r *http.Request) { srv.RenderTemplate(r.Context(), w, "404.tmpl", Data{}.WithCode(http.StatusNotFound)) }