Replace gorilla/csrf with justinas/nosurf

- replace dependency for less indirect dependencies
- remove unused configuration options

README.md

@@ -91,8 +91,6 @@ You can use the following table to find useful values for the parameters in `con
 | `client_ca_certificates`                  | File containing allowed client certificate CA certificates (production value is `cacert_class3.pem`)                                                       | use the shell code above                                                                                              |
 | `server_certificate`                      | X.509 certificate that is used to identify your server (i.e. `server.crt`)                                                                                 | use the filename used as `-out` parameter in the `openssl` invocation above                                           |
 | `server_key`                              | PEM encoded private key file (i.e. `server.key`)                                                                                                           | use the filename used as `-keyout` parameter in the `openssl` invocation above                                        |
-| `cookie_secret`                           | A base64 encoded random byte value of at least 32 bytes used to encrypt cookies                                                                            | see [Generating random byte values](#generating-random-byte-values) below                                             |
-| `csrf_key`                                | A base64 encoded random byte value of at least 32 bytes used to encrypt [CSRF](https://en.wikipedia.org/wiki/Cross-site_request_forgery#Prevention) tokens | see [Generating random byte values](#generating-random-byte-values) below                                             |
 | `mail_config.smtp_host`                   | Mail server host (production value is `localhost`)                                                                                                         | `localhost`                                                                                                           |
 | `mail_config.smtp_port`                   | Mail server TCP port (production value is `25`                                                                                                             | see [how to setup a debugging SMTP server](#debugging-smtp-server) below and choose the port of that (default `8025`) |
 | `mail_config.base_url`                    | The base URL of your application instance (production value is https://motions.cacert.org)                                                                 | use https://localhost:8443                                                                                            |

cmd/boardvoting/config.go

@@ -18,7 +18,6 @@ limitations under the License.
 package main
 
 import (
-	"encoding/base64"
 	"fmt"
 	"io/ioutil"
 	"time"
@@ -27,8 +26,6 @@ import (
 )
 
 const (
-	cookieSecretMinLen    = 32
-	csrfKeyLength         = 32
 	httpIdleTimeout       = time.Minute
 	httpReadHeaderTimeout = 5 * time.Second
 	httpReadTimeout       = 5 * time.Second
@@ -62,8 +59,6 @@ type Config struct {
 	HTTPSAddress         string             `yaml:"https_address,omitempty"`
 	MailConfig           *mailConfig        `yaml:"mail_config"`
 	Timeouts             *httpTimeoutConfig `yaml:"timeouts,omitempty"`
-	CookieSecret         []byte             `yaml:"-"`
-	CsrfKey              []byte             `yaml:"-"`
 }
 
 func parseConfig(configFile string) (*Config, error) {
@@ -87,23 +82,5 @@ func parseConfig(configFile string) (*Config, error) {
 		return nil, fmt.Errorf("could not parse configuration: %w", err)
 	}
 
-	if config.CookieSecret, err = base64.StdEncoding.DecodeString(config.CookieSecretStr); err != nil {
-		return nil, fmt.Errorf("could not decode cookie secret: %w", err)
-	}
-
-	if len(config.CookieSecret) < cookieSecretMinLen {
-		return nil, fmt.Errorf("cookie secret is less than the minimum require %d bytes long", cookieSecretMinLen)
-	}
-
-	if config.CsrfKey, err = base64.StdEncoding.DecodeString(config.CsrfKeyStr); err != nil {
-		return nil, fmt.Errorf("could not decode CSRF key: %w", err)
-	}
-
-	if len(config.CsrfKey) != csrfKeyLength {
-		return nil, fmt.Errorf(
-			"CSRF key must be exactly %d bytes long but is %d bytes long", csrfKeyLength, len(config.CsrfKey),
-		)
-	}
-
 	return config, nil
 }

cmd/boardvoting/handlers.go

@@ -18,24 +18,16 @@ limitations under the License.
 package main
 
 import (
-	"bytes"
 	"database/sql"
 	"errors"
 	"fmt"
-	"html/template"
-	"io/fs"
 	"net/http"
-	"path/filepath"
-	"strings"
 	"time"
 
-	"github.com/Masterminds/sprig/v3"
-	"github.com/gorilla/csrf"
 	"github.com/julienschmidt/httprouter"
 
 	"git.cacert.org/cacert-boardvoting/internal/models"
 	"git.cacert.org/cacert-boardvoting/internal/validator"
-	"git.cacert.org/cacert-boardvoting/ui"
 )
 
 func checkRole(v *models.User, roles []string) (bool, error) {
@@ -47,49 +39,6 @@ func checkRole(v *models.User, roles []string) (bool, error) {
 	return hasRole, nil
 }
 
-func newTemplateCache() (map[string]*template.Template, error) {
-	cache := map[string]*template.Template{}
-
-	pages, err := fs.Glob(ui.Files, "html/pages/*.html")
-	if err != nil {
-		return nil, fmt.Errorf("could not find page templates: %w", err)
-	}
-
-	funcMaps := sprig.FuncMap()
-	funcMaps["nl2br"] = func(text string) template.HTML {
-		// #nosec G203 input is sanitized
-		return template.HTML(strings.ReplaceAll(template.HTMLEscapeString(text), "\n", "<br>"))
-	}
-	funcMaps["canManageUsers"] = func(v *models.User) (bool, error) {
-		return checkRole(v, []string{models.RoleSecretary, models.RoleAdmin})
-	}
-	funcMaps["canVote"] = func(v *models.User) (bool, error) {
-		return checkRole(v, []string{models.RoleVoter})
-	}
-	funcMaps["canStartVote"] = func(v *models.User) (bool, error) {
-		return checkRole(v, []string{models.RoleVoter})
-	}
-	funcMaps[csrf.TemplateTag] = csrf.TemplateField
-
-	for _, page := range pages {
-		name := filepath.Base(page)
-
-		ts, err := template.New("").Funcs(funcMaps).ParseFS(
-			ui.Files,
-			"html/base.html",
-			"html/partials/*.html",
-			page,
-		)
-		if err != nil {
-			return nil, fmt.Errorf("could not parse base template: %w", err)
-		}
-
-		cache[name] = ts
-	}
-
-	return cache, nil
-}
-
 type topLevelNavItem string
 type subLevelNavItem string
 
@@ -102,61 +51,6 @@ const (
 	subLevelNavUsers          subLevelNavItem = "users"
 )
 
-type templateData struct {
-	PrevPage     string
-	NextPage     string
-	Motion       *models.MotionForDisplay
-	Motions      []*models.MotionForDisplay
-	User         *models.User
-	Users        []*models.User
-	Request      *http.Request
-	Flash        string
-	Form         any
-	ActiveNav    topLevelNavItem
-	ActiveSubNav subLevelNavItem
-}
-
-func (app *application) newTemplateData(
-	r *http.Request,
-	nav topLevelNavItem,
-	subNav subLevelNavItem,
-) *templateData {
-	user, err := app.GetUser(r)
-	if err != nil {
-		app.errorLog.Printf("error getting user for template data: %v", err)
-	}
-
-	return &templateData{
-		Request:      r,
-		User:         user,
-		ActiveNav:    nav,
-		ActiveSubNav: subNav,
-		Flash:        app.sessionManager.PopString(r.Context(), "flash"),
-	}
-}
-
-func (app *application) render(w http.ResponseWriter, status int, page string, data *templateData) {
-	ts, ok := app.templateCache[page]
-	if !ok {
-		app.serverError(w, fmt.Errorf("the template %s does not exist", page))
-
-		return
-	}
-
-	buf := new(bytes.Buffer)
-
-	err := ts.ExecuteTemplate(buf, "base", data)
-	if err != nil {
-		app.serverError(w, err)
-
-		return
-	}
-
-	w.WriteHeader(status)
-
-	_, _ = buf.WriteTo(w)
-}
-
 func (m *templateData) motionPaginationOptions(limit int, first, last *time.Time) error {
 	motions := m.Motions
 

cmd/boardvoting/helpers.go

@@ -18,12 +18,22 @@ limitations under the License.
 package main
 
 import (
+	"bytes"
 	"errors"
 	"fmt"
+	"html/template"
+	"io/fs"
 	"net/http"
+	"path/filepath"
 	"runtime/debug"
+	"strings"
 
+	"github.com/Masterminds/sprig/v3"
 	"github.com/go-playground/form/v4"
+	"github.com/justinas/nosurf"
+
+	"git.cacert.org/cacert-boardvoting/internal/models"
+	"git.cacert.org/cacert-boardvoting/ui"
 )
 
 func (app *application) serverError(w http.ResponseWriter, err error) {
@@ -61,3 +71,102 @@ func (app *application) decodePostForm(r *http.Request, dst any) error {
 
 	return nil
 }
+
+func newTemplateCache() (map[string]*template.Template, error) {
+	cache := map[string]*template.Template{}
+
+	pages, err := fs.Glob(ui.Files, "html/pages/*.html")
+	if err != nil {
+		return nil, fmt.Errorf("could not find page templates: %w", err)
+	}
+
+	funcMaps := sprig.FuncMap()
+	funcMaps["nl2br"] = func(text string) template.HTML {
+		// #nosec G203 input is sanitized
+		return template.HTML(strings.ReplaceAll(template.HTMLEscapeString(text), "\n", "<br>"))
+	}
+	funcMaps["canManageUsers"] = func(v *models.User) (bool, error) {
+		return checkRole(v, []string{models.RoleSecretary, models.RoleAdmin})
+	}
+	funcMaps["canVote"] = func(v *models.User) (bool, error) {
+		return checkRole(v, []string{models.RoleVoter})
+	}
+	funcMaps["canStartVote"] = func(v *models.User) (bool, error) {
+		return checkRole(v, []string{models.RoleVoter})
+	}
+
+	for _, page := range pages {
+		name := filepath.Base(page)
+
+		ts, err := template.New("").Funcs(funcMaps).ParseFS(
+			ui.Files,
+			"html/base.html",
+			"html/partials/*.html",
+			page,
+		)
+		if err != nil {
+			return nil, fmt.Errorf("could not parse base template: %w", err)
+		}
+
+		cache[name] = ts
+	}
+
+	return cache, nil
+}
+
+type templateData struct {
+	PrevPage     string
+	NextPage     string
+	Motion       *models.MotionForDisplay
+	Motions      []*models.MotionForDisplay
+	User         *models.User
+	Users        []*models.User
+	Request      *http.Request
+	Flash        string
+	Form         any
+	ActiveNav    topLevelNavItem
+	ActiveSubNav subLevelNavItem
+	CSRFToken    string
+}
+
+func (app *application) newTemplateData(
+	r *http.Request,
+	nav topLevelNavItem,
+	subNav subLevelNavItem,
+) *templateData {
+	user, err := app.GetUser(r)
+	if err != nil {
+		app.errorLog.Printf("error getting user for template data: %v", err)
+	}
+
+	return &templateData{
+		Request:      r,
+		User:         user,
+		ActiveNav:    nav,
+		ActiveSubNav: subNav,
+		Flash:        app.sessionManager.PopString(r.Context(), "flash"),
+		CSRFToken:    nosurf.Token(r),
+	}
+}
+
+func (app *application) render(w http.ResponseWriter, status int, page string, data *templateData) {
+	ts, ok := app.templateCache[page]
+	if !ok {
+		app.serverError(w, fmt.Errorf("the template %s does not exist", page))
+
+		return
+	}
+
+	buf := new(bytes.Buffer)
+
+	err := ts.ExecuteTemplate(buf, "base", data)
+	if err != nil {
+		app.serverError(w, err)
+
+		return
+	}
+
+	w.WriteHeader(status)
+
+	_, _ = buf.WriteTo(w)
+}

cmd/boardvoting/main.go

@@ -61,7 +61,6 @@ type application struct {
 	templateCache     map[string]*template.Template
 	sessionManager    *scs.SessionManager
 	formDecoder       *form.Decoder
-	csrfKey           []byte
 }
 
 func main() {
@@ -110,7 +109,6 @@ func main() {
 		mailConfig:     config.MailConfig,
 		templateCache:  templateCache,
 		sessionManager: sessionManager,
-		csrfKey:        config.CsrfKey,
 		formDecoder:    setupFormDecoder(),
 	}
 

cmd/boardvoting/middleware.go

@@ -24,6 +24,8 @@ import (
 	"net/http"
 	"strings"
 
+	"github.com/justinas/nosurf"
+
 	"git.cacert.org/cacert-boardvoting/internal/models"
 )
 
@@ -172,3 +174,15 @@ func (app *application) userCanEditVote(next http.Handler) http.Handler {
 func (app *application) userCanChangeVoters(next http.Handler) http.Handler {
 	return app.requireRole(next, []string{models.RoleSecretary, models.RoleAdmin})
 }
+
+func noSurf(next http.Handler) http.Handler {
+	csrfHandler := nosurf.New(next)
+	csrfHandler.SetBaseCookie(http.Cookie{
+		HttpOnly: true,
+		Path:     "/",
+		Secure:   true,
+		SameSite: http.SameSiteStrictMode,
+	})
+
+	return csrfHandler
+}

cmd/boardvoting/routes.go

@@ -22,7 +22,6 @@ import (
 	"io/fs"
 	"net/http"
 
-	"github.com/gorilla/csrf"
 	"github.com/julienschmidt/httprouter"
 	"github.com/justinas/alice"
 	"github.com/vearutop/statigz"
@@ -61,16 +60,14 @@ func (app *application) routes() http.Handler {
 	)
 	router.Handler(http.MethodGet, "/static/*filepath", http.StripPrefix("/static", fileServer))
 
-	csrfHandler := csrf.Protect(app.csrfKey, csrf.SameSite(csrf.SameSiteStrictMode))
-
 	dynamic := alice.New(
 		app.sessionManager.LoadAndSave,
 		app.tryAuthenticate,
 	)
 
-	canVote := dynamic.Append(app.userCanVote, csrfHandler)
-	canEditVote := dynamic.Append(app.userCanEditVote, csrfHandler)
-	canManageUsers := dynamic.Append(app.userCanChangeVoters, csrfHandler)
+	canVote := dynamic.Append(app.userCanVote, noSurf)
+	canEditVote := dynamic.Append(app.userCanEditVote, noSurf)
+	canManageUsers := dynamic.Append(app.userCanChangeVoters, noSurf)
 
 	router.Handler(http.MethodGet, "/motions/", dynamic.ThenFunc(app.motionList))
 	router.Handler(http.MethodGet, "/motions/:tag", dynamic.ThenFunc(app.motionDetails))

config.yaml.example

@@ -3,8 +3,6 @@ database_file: database.sqlite
 client_ca_certificates: cacert_class3.pem
 server_certificate: server.crt
 server_key: server.key
-cookie_secret: base64encoded_random_byte_value_of_at_least_32_bytes
-csrf_key: base64encoded_random_byte_value_of_at_least_32_bytes
 mail_server:
   smtp_host: localhost
   smtp_port: 25

go.mod

@@ -6,8 +6,6 @@ require (
 	github.com/Masterminds/sprig/v3 v3.2.2
 	github.com/golang-migrate/migrate/v4 v4.15.2
 	github.com/google/uuid v1.3.0 // indirect
-	github.com/gorilla/csrf v1.7.1
-	github.com/gorilla/sessions v1.2.1
 	github.com/hashicorp/errwrap v1.1.0 // indirect
 	github.com/hashicorp/go-multierror v1.1.1 // indirect
 	github.com/huandu/xstrings v1.3.2 // indirect
@@ -26,22 +24,23 @@ require (
 )
 
 require (
+	github.com/alexedwards/scs/sqlite3store v0.0.0-20220216073957-c252878bcf5a
+	github.com/alexedwards/scs/v2 v2.5.0
+	github.com/go-playground/form/v4 v4.2.0
+	github.com/gorilla/csrf v1.7.1
+	github.com/gorilla/sessions v1.2.1
+	github.com/julienschmidt/httprouter v1.3.0
 	github.com/justinas/alice v1.2.0
+	github.com/justinas/nosurf v1.1.1
 	github.com/stretchr/testify v1.7.0
 )
 
 require (
 	github.com/Masterminds/goutils v1.1.1 // indirect
 	github.com/Masterminds/semver/v3 v3.1.1 // indirect
-	github.com/alexedwards/scs v1.4.1 // indirect
-	github.com/alexedwards/scs/sqlite3store v0.0.0-20220216073957-c252878bcf5a // indirect
-	github.com/alexedwards/scs/v2 v2.5.0 // indirect
 	github.com/andybalholm/brotli v1.0.4 // indirect
 	github.com/davecgh/go-spew v1.1.1 // indirect
-	github.com/go-playground/form/v4 v4.2.0 // indirect
-	github.com/gorilla/schema v1.2.0 // indirect
 	github.com/gorilla/securecookie v1.1.1 // indirect
-	github.com/julienschmidt/httprouter v1.3.0 // indirect
 	github.com/mitchellh/reflectwalk v1.0.2 // indirect
 	github.com/pkg/errors v0.9.1 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect

go.sum

@@ -121,8 +121,6 @@ github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuy
 github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0=
 github.com/alecthomas/units v0.0.0-20190717042225-c3de453c63f4/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0=
 github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d/go.mod h1:rBZYJk541a8SKzHPHnH3zbiI+7dagKZ0cgpgrD7Fyho=
-github.com/alexedwards/scs v1.4.1 h1:/5L5a07IlqApODcEfZyMsu8Smd1S7Q4nBjEyKxIRTp0=
-github.com/alexedwards/scs v1.4.1/go.mod h1:JRIFiXthhMSivuGbxpzUa0/hT5rz2hpyw61Bmd+S1bg=
 github.com/alexedwards/scs/sqlite3store v0.0.0-20220216073957-c252878bcf5a h1:5SCXvM8hruEAoNdKHVte0v3uVKqWLjDQeq4KIfFGqpM=
 github.com/alexedwards/scs/sqlite3store v0.0.0-20220216073957-c252878bcf5a/go.mod h1:Iyk7S76cxGaiEX/mSYmTZzYehp4KfyylcLaV3OnToss=
 github.com/alexedwards/scs/v2 v2.5.0 h1:zgxOfNFmiJyXG7UPIuw1g2b9LWBeRLh3PjfB9BDmfL4=
@@ -462,6 +460,7 @@ github.com/go-openapi/swag v0.0.0-20160704191624-1d0bd113de87/go.mod h1:DXUve3Dp
 github.com/go-openapi/swag v0.19.2/go.mod h1:POnQmlKehdgb5mhVOsnJFsivZCEZ/vjK9gh66Z9tfKk=
 github.com/go-openapi/swag v0.19.5/go.mod h1:POnQmlKehdgb5mhVOsnJFsivZCEZ/vjK9gh66Z9tfKk=
 github.com/go-openapi/swag v0.19.14/go.mod h1:QYRuS/SOXUCsnplDa677K7+DxSOj6IPNl/eQntq43wQ=
+github.com/go-playground/assert/v2 v2.0.1 h1:MsBgLAaY856+nPRTKrp3/OZK38U/wa0CcBYNjji3q3A=
 github.com/go-playground/assert/v2 v2.0.1/go.mod h1:VDjEfimB/XKnb+ZQfWdccd7VUvScMdVu0Titje2rxJ4=
 github.com/go-playground/form/v4 v4.2.0 h1:N1wh+Goz61e6w66vo8vJkQt+uwZSoLz50kZPJWR8eic=
 github.com/go-playground/form/v4 v4.2.0/go.mod h1:q1a2BY+AQUUzhl6xA/6hBetay6dEIhMHjgvJiGo6K7U=
@@ -626,8 +625,6 @@ github.com/gorilla/mux v1.6.2/go.mod h1:1lud6UwP+6orDFRuTfBEV8e9/aOM/c4fVVCaMa2z
 github.com/gorilla/mux v1.7.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/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So=
-github.com/gorilla/schema v1.2.0 h1:YufUaxZYCKGFuAq3c96BOhjgd5nmXiOY9NGzF247Tsc=
-github.com/gorilla/schema v1.2.0/go.mod h1:kgLaKoK1FELgZqMAVxx/5cbj0kT+57qxUrAlIO2eleU=
 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/sessions v1.2.1 h1:DHd3rPN5lE3Ts3D8rKkQ8x/0kqfeNmBAaiSi+o7FsgI=
@@ -766,6 +763,8 @@ github.com/jung-kurt/gofpdf v1.0.0/go.mod h1:7Id9E/uU8ce6rXgefFLlgrJj/GYY22cpxn+
 github.com/jung-kurt/gofpdf v1.0.3-0.20190309125859-24315acbbda5/go.mod h1:7Id9E/uU8ce6rXgefFLlgrJj/GYY22cpxn+r32jIOes=
 github.com/justinas/alice v1.2.0 h1:+MHSA/vccVCF4Uq37S42jwlkvI2Xzl7zTPCN5BnZNVo=
 github.com/justinas/alice v1.2.0/go.mod h1:fN5HRH/reO/zrUflLfTN43t3vXvKzvZIENsNEe7i7qA=
+github.com/justinas/nosurf v1.1.1 h1:92Aw44hjSK4MxJeMSyDa7jwuI9GR2J/JCQiaKvXXSlk=
+github.com/justinas/nosurf v1.1.1/go.mod h1:ALpWdSbuNGy2lZWtyXdjkYv4edL23oSEgfBT1gPJ5BQ=
 github.com/k0kubun/colorstring v0.0.0-20150214042306-9440f1994b88/go.mod h1:3w7q1U84EfirKl04SVQ/s7nPm1ZPhiXd34z40TNz36k=
 github.com/k0kubun/pp v2.3.0+incompatible/go.mod h1:GWse8YhT0p8pT4ir3ZgBbfZild3tgzSScAn6HmfYukg=
 github.com/kardianos/osext v0.0.0-20190222173326-2bc1f35cddc0/go.mod h1:1NbS8ALrpOvjt0rHPNLyCIeMtbizbir8U//inJ+zuB8=

ui/html/pages/create_motion.html

@@ -3,7 +3,7 @@
 {{ define "main" }}
     <div class="ui raised segment">
         <form action="/newmotion/" method="post">
-            {{ csrfField .Request }}
+            <input type="hidden" name="csrf_token" value="{{ .CSRFToken }}">
             <div class="ui form{{ if .Form.FieldErrors }} error{{ end }}">
                 <div class="three fields">
                     <div class="field">