diff --git a/assets/server/codes/issue.html b/assets/server/codes/issue.html index 19b7844c3..7618a7d25 100644 --- a/assets/server/codes/issue.html +++ b/assets/server/codes/issue.html @@ -391,7 +391,7 @@

{{t $.locale "codes.issue.header"}}

contentType: 'application/json', data: JSON.stringify(data), headers: { - 'X-CSRF-Token': '{{.csrfToken}}', + 'X-CSRF-Token': getCSRFToken(), }, success: function(result) { if(result.error && result.error != "") { diff --git a/assets/server/login/_loginscripts.html b/assets/server/login/_loginscripts.html index 58a2a6d3f..525357d35 100644 --- a/assets/server/login/_loginscripts.html +++ b/assets/server/login/_loginscripts.html @@ -15,7 +15,7 @@ data: { idToken: idToken, }, - headers: { 'X-CSRF-Token': '{{.csrfToken}}' }, + headers: { 'X-CSRF-Token': getCSRFToken() }, contentType: 'application/x-www-form-urlencoded', {{if not .currentUser}} success: function(returnData) { diff --git a/assets/server/login/register-phone.html b/assets/server/login/register-phone.html index 03b9463ea..b993b3981 100644 --- a/assets/server/login/register-phone.html +++ b/assets/server/login/register-phone.html @@ -319,7 +319,7 @@ idToken: idToken, factorCount: factorCount, }, - headers: { 'X-CSRF-Token': '{{.csrfToken}}' }, + headers: { 'X-CSRF-Token': getCSRFToken() }, contentType: 'application/x-www-form-urlencoded', success: function(returnData) { flash.clear(); diff --git a/assets/server/static/js/application.js b/assets/server/static/js/application.js index 1804e04a9..9a2f36f8b 100644 --- a/assets/server/static/js/application.js +++ b/assets/server/static/js/application.js @@ -34,7 +34,7 @@ $(function() { } } - let csrfToken = $("meta[name=csrf-token]").attr("content"); + let csrfToken = getCSRFToken(); let $csrfField = $("") .attr("type", "hidden") .attr("name", "gorilla.csrf.Token") @@ -238,6 +238,10 @@ $(function() { window.flash = flash; }); +function getCSRFToken() { + return document.querySelector('meta[name="csrf-token"]').content; +} + function setCookie(cname, cvalue, exdays) { let d = new Date(); d.setTime(d.getTime() + (exdays * 24 * 60 * 60 * 1000)); diff --git a/assets/server/users/import.html b/assets/server/users/import.html index 0ff21c416..77da4b26a 100644 --- a/assets/server/users/import.html +++ b/assets/server/users/import.html @@ -207,7 +207,7 @@

Import users

"users": data, "sendInvites": sendInvites, }), - headers: { 'X-CSRF-Token': '{{.csrfToken}}' }, + headers: { 'X-CSRF-Token': getCSRFToken() }, contentType: 'application/json', success: function(result) { totalUsersAdded += result.newUsers.length; diff --git a/docs/development.md b/docs/development.md index 0028ce6dd..c557d8cb9 100644 --- a/docs/development.md +++ b/docs/development.md @@ -75,12 +75,6 @@ represent best practices. # Disable local observability. export OBSERVABILITY_EXPORTER="NOOP" - # Configure CSRF for preventing request forgery. Create your own with: - # - # openssl rand -base64 32 - # - export CSRF_AUTH_KEY="RcCNhTkS9tSDMSGcl4UCa1FUg9GmctkJpdI+eqZ+3v4=" - # Configure cookie encryption, the first is 64 bytes, the second is 32. # Create your own values with: # diff --git a/docs/production.md b/docs/production.md index 27591e31d..3f11a1ea8 100644 --- a/docs/production.md +++ b/docs/production.md @@ -184,22 +184,6 @@ provide its _reference_ in the environment. If you are not using a secret manager, provide this value directly in the environment. -### Cross-site request forgery (CSRF) keys - -**Recommended frequency:** 90 days, on breach - -To rotate the key, generate a new 32-byte key. You can use `openssl` or similar: - -```sh -openssl rand -base64 32 | tr -d "\n" -``` - -Update the `CSRF_AUTH_KEY` environment variable and re-deploy. The system [only -supports a single key for CSRF](https://github.com/gorilla/csrf/issues/65). When -you deploy the new key, any existing open HTML forms will fail to submit as an -invalid request. - - ### Database encryption keys **Recommend frequency:** 30 days, on breach diff --git a/go.mod b/go.mod index 9ec20c258..fff85fb48 100644 --- a/go.mod +++ b/go.mod @@ -20,7 +20,6 @@ require ( github.com/google/exposure-notifications-server v0.24.1-0.20210322233555-ecf9dee504c1 github.com/google/go-cmp v0.5.5 github.com/google/uuid v1.2.0 - github.com/gorilla/csrf v1.7.0 github.com/gorilla/handlers v1.5.1 github.com/gorilla/mux v1.8.0 github.com/gorilla/schema v1.2.0 diff --git a/go.sum b/go.sum index aca701acb..46a19985e 100644 --- a/go.sum +++ b/go.sum @@ -430,8 +430,6 @@ github.com/googleapis/gax-go/v2 v2.0.5 h1:sjZBwGj9Jlw33ImPtvFviGYvseOtDM7hkSKB7+ github.com/googleapis/gax-go/v2 v2.0.5/go.mod h1:DWXyrwAJ9X0FpwwEdw+IPEYBICEFu5mhpdKc/us6bOk= 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/css v1.0.0 h1:BQqNyPTi50JCFMTw/b67hByjMVXZRwGha6wxVGkeihY= github.com/gorilla/css v1.0.0/go.mod h1:Dn721qIggHpt4+EFCcTLTU/vk5ySda2ReITrtgBl60c= github.com/gorilla/handlers v1.4.2/go.mod h1:Qkdc/uu4tH4g6mTK6auzZ766c4CA0Ng8+o/OAirnOIQ= diff --git a/internal/envstest/server.go b/internal/envstest/server.go index aa52c6eab..188deb282 100644 --- a/internal/envstest/server.go +++ b/internal/envstest/server.go @@ -232,8 +232,7 @@ func NewServerConfig(tb testing.TB, testDatabaseInstance *database.TestInstance) EnableAuthenticatedSMS: true, }, - CookieKeys: config.Base64ByteSlice{randomBytes(tb, 64), randomBytes(tb, 32)}, - CSRFAuthKey: randomBytes(tb, 32), + CookieKeys: config.Base64ByteSlice{randomBytes(tb, 64), randomBytes(tb, 32)}, CertificateSigning: config.CertificateSigningConfig{ Keys: *harness.KeyManagerConfig, diff --git a/internal/routes/server.go b/internal/routes/server.go index 669d57cca..d2a59c00e 100644 --- a/internal/routes/server.go +++ b/internal/routes/server.go @@ -125,10 +125,6 @@ func Server( processDebug := middleware.ProcessDebug() r.Use(processDebug) - // Install the CSRF protection middleware. - configureCSRF := middleware.ConfigureCSRF(cfg, h) - r.Use(configureCSRF) - // Sessions requireSession := middleware.RequireSession(sessions, h) r.Use(requireSession) @@ -137,6 +133,10 @@ func Server( currentPath := middleware.InjectCurrentPath() r.Use(currentPath) + // Install the CSRF protection middleware. + handleCSRF := middleware.HandleCSRF(h) + r.Use(handleCSRF) + // Create common middleware requireAuth := middleware.RequireAuth(cacher, authProvider, db, h, cfg.SessionIdleTimeout, cfg.SessionDuration) checkIdleNoAuth := middleware.CheckSessionIdleNoAuth(h, cfg.SessionIdleTimeout) diff --git a/pkg/api/api.go b/pkg/api/api.go index 9a66a91d2..2978011d6 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -172,15 +172,6 @@ func (p *Padding) UnmarshalJSON(b []byte) error { return nil } -// CSRFResponse is the return type when requesting an AJAX CSRF token. -type CSRFResponse struct { - Padding Padding `json:"padding"` - - CSRFToken string `json:"csrftoken"` - Error string `json:"error"` - ErrorCode string `json:"errorCode"` -} - // UserBatchRequest is a request for bulk creation of users. // This is called by the Web frontend. // API is served at /users/import/userbatch diff --git a/pkg/config/server_config.go b/pkg/config/server_config.go index a60f793b8..f67e6f558 100644 --- a/pkg/config/server_config.go +++ b/pkg/config/server_config.go @@ -17,6 +17,7 @@ package config import ( "context" "fmt" + "os" "strings" "time" @@ -27,6 +28,7 @@ import ( "github.com/microcosm-cc/bluemonday" "github.com/russross/blackfriday/v2" + "github.com/google/exposure-notifications-server/pkg/logging" "github.com/google/exposure-notifications-server/pkg/observability" firebase "firebase.google.com/go" @@ -85,10 +87,6 @@ type ServerConfig struct { // CookieDomain is the domain for which cookie should be valid. CookieDomain string `env:"COOKIE_DOMAIN"` - // CSRFAuthKey is the authentication key. It must be 32-bytes and can be - // generated with tools/gen-secret. The value's should be base64 encoded. - CSRFAuthKey envconfig.Base64Bytes `env:"CSRF_AUTH_KEY,required"` - // Application Config ServerName string `env:"SERVER_NAME,default=Diagnosis Verification Server"` @@ -113,6 +111,12 @@ func NewServerConfig(ctx context.Context) (*ServerConfig, error) { return nil, err } + // Deprecations + logger := logging.FromContext(ctx).Named("config.ServerConfig") + if v := os.Getenv("CSRF_AUTH_KEY"); v != "" { + logger.Warnw("CSRF_AUTH_KEY is no longer used, please remove it from your configuration") + } + // Parse system notice - do this once since it's displayed on every page. if v := project.TrimSpace(config.SystemNotice); v != "" { raw := blackfriday.Run([]byte(v)) diff --git a/pkg/controller/middleware/csrf.go b/pkg/controller/middleware/csrf.go index 763d1c92d..76ea2dda6 100644 --- a/pkg/controller/middleware/csrf.go +++ b/pkg/controller/middleware/csrf.go @@ -1,4 +1,4 @@ -// Copyright 2020 Google LLC +// Copyright 2021 Google LLC // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -15,58 +15,194 @@ package middleware import ( + "crypto/subtle" + "encoding/base64" + "fmt" "html/template" "net/http" "github.com/google/exposure-notifications-server/pkg/logging" - "github.com/google/exposure-notifications-verification-server/pkg/config" + "github.com/google/exposure-notifications-verification-server/internal/project" "github.com/google/exposure-notifications-verification-server/pkg/controller" "github.com/google/exposure-notifications-verification-server/pkg/render" - - "github.com/gorilla/csrf" "github.com/gorilla/mux" ) -// ConfigureCSRF injects the CSRF handling and populates the global template map -// with the csrfToken and csrfTemplate. -func ConfigureCSRF(config *config.ServerConfig, h *render.Renderer) mux.MiddlewareFunc { - protect := csrf.Protect(config.CSRFAuthKey, - csrf.Secure(!config.DevMode), - csrf.ErrorHandler(handleCSRFError(h)), - ) +const ( + // CSRFHeaderField is the name of the header where the CSRF token resides. + CSRFHeaderField = "X-CSRF-Token" + + // CSRFFormField is the form field name. + CSRFFormField = "csrf_token" + CSRFFormFieldTemplate = `` + + // CSRFMetaTagName is the meta tag name (used by Javascript). + CSRFMetaTagName = "csrf-token" + CSRFMetaTagTemplate = `` + + // TokenLength is the length of the token (in bytes). + TokenLength = 64 +) + +const ( + ErrMissingExistingToken = Error("missing existing csrf token in session") + ErrMissingIncomingToken = Error("missing csrf token in request") + ErrInvalidToken = Error("invalid csrf token") +) + +type Error string +func (e Error) Error() string { return string(e) } + +// HandleCSRF first extracts an existing CSRF token from the session (if one +// exists). Then, it generates a unique, per-request CSRF token and stores it in +// the session. This must come after RequireSession to ensure the session has +// been populated. +func HandleCSRF(h *render.Renderer) mux.MiddlewareFunc { return func(next http.Handler) http.Handler { - return protect(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctx := r.Context() - // Save csrf configuration on the template map. - m := controller.TemplateMapFromContext(ctx) - m["csrfField"] = csrf.TemplateField(r) - m["csrfToken"] = csrf.Token(r) - m["csrfMeta"] = template.HTML( - ``) + logger := logging.FromContext(ctx).Named("middleware.HandleCSRF") - // Save the template map on the context. + session := controller.SessionFromContext(ctx) + if session == nil { + controller.MissingSession(w, r, h) + return + } + + // Get the existing CSRF token from the session, if one exists. + existingToken := controller.CSRFTokenFromSession(session) + + // If the existing token is invalid or missing, generate and store a new + // token on the session. + if l := len(existingToken); l != TokenLength { + newToken, err := project.RandomBytes(TokenLength) + if err != nil { + controller.InternalError(w, r, h, err) + return + } + controller.StoreSessionCSRFToken(session, newToken) + + // Set the existing token to this new token. The only way we got into + // this block was if the existing token was missing or invalid. That + // means it's not safe to pass to the template or request helpers or + // else we risk future requests failing. The new token will fail + // validation, if applicable, below. + existingToken = newToken + } + + // Save the csrf functions on the template map for use in HTML templates. + masked, err := mask(existingToken) + if err != nil { + controller.InternalError(w, r, h, err) + return + } + m := controller.TemplateMapFromContext(ctx) + m["csrfHeaderField"] = CSRFHeaderField + m["csrfMetaTagName"] = CSRFMetaTagName + m["csrfToken"] = template.HTML(masked) + m["csrfField"] = template.HTML(fmt.Sprintf(CSRFFormFieldTemplate, CSRFFormField, masked)) + m["csrfMeta"] = template.HTML(fmt.Sprintf(CSRFMetaTagTemplate, CSRFMetaTagName, masked)) ctx = controller.WithTemplateMap(ctx, m) r = r.Clone(ctx) + // Only mutating methods need CSRF verification. + if canSkipCSRFCheck(r) { + next.ServeHTTP(w, r) + return + } + + // Grab the incoming token from the request. + incomingToken, err := tokenFromRequest(r) + if err != nil { + // Warn, but don't return an error here. The token comparison below will + // correctly fail and we can return the correct response there. This is + // almost certainly user error with the provided token in the request. + logger.Warnw("invalid csrf token from request", "error", err) + } + + if subtle.ConstantTimeCompare(existingToken, incomingToken) != 1 { + controller.Unauthorized(w, r, h) + return + } + next.ServeHTTP(w, r) - })) + }) } } -// handleCSRFError is an http.HandlerFunc that can be installed in the gorilla -// csrf protect middleware. It will respond w/ a JSON object containing error: -// on API requests and a signout redirect to other requests. -func handleCSRFError(h *render.Renderer) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - ctx := r.Context() - logger := logging.FromContext(ctx).Named("middleware.handleCSRFError") +// tokenFromRequest extracts the provided token from the request, then the form, +// then the multi-part form. The token must be a valid base64 URL-encoded string +// generated from the csrf middleware. If the token is missing, it returns +// ErrMissingIncomingToken. If the token is present but cannot be decoded, it +// returns ErrInvalidToken. +func tokenFromRequest(r *http.Request) ([]byte, error) { + str := r.Header.Get(CSRFHeaderField) - reason := csrf.FailureReason(r) - logger.Debugw("invalid csrf", "reason", reason) + if str == "" { + str = r.PostFormValue(CSRFFormField) + } - controller.Unauthorized(w, r, h) - return - }) + if str == "" && r.MultipartForm != nil { + vals, ok := r.MultipartForm.Value[CSRFFormField] + if ok && len(vals) > 0 { + str = vals[0] + } + } + + if str == "" { + return nil, ErrMissingIncomingToken + } + + b, err := base64.RawURLEncoding.DecodeString(str) + if err != nil { + return nil, ErrInvalidToken + } + + raw, err := unmask(b) + if err != nil { + return nil, err + } + return raw, nil +} + +// canSkipCSRFCheck returns true if the request does not need CSRF validation, +// false otherwise. Only mutating methods need CSRF verification. +func canSkipCSRFCheck(r *http.Request) bool { + return r.Method == http.MethodGet || r.Method == http.MethodHead || + r.Method == http.MethodOptions || r.Method == http.MethodTrace +} + +// mask xors the raw token with random bytes and prepends those random bytes to +// the result. +func mask(raw []byte) (string, error) { + b, err := project.RandomBytes(len(raw)) + if err != nil { + return "", fmt.Errorf("maskToken: %w", err) + } + + b = append(b, xor(b, raw)...) + return base64.RawURLEncoding.EncodeToString(b), nil +} + +// unmask splits the raw token into its otp and token parts and XORs the values +// again to get back the original token. It returns an error if the slice is too +// small or of an uneven length. +func unmask(raw []byte) ([]byte, error) { + if len(raw) < 2 || len(raw)%2 != 0 { + return nil, ErrInvalidToken + } + + half := len(raw) / 2 + return xor(raw[half:], raw[:half]), nil +} + +// xor takes the XOR of x of y. len(y) must be >= len(x). +func xor(x, y []byte) []byte { + result := make([]byte, len(x)) + for i := 0; i < len(x); i++ { + result[i] = x[i] ^ y[i] + } + return result } diff --git a/pkg/controller/middleware/csrf_test.go b/pkg/controller/middleware/csrf_test.go index c286060e9..9f73257ee 100644 --- a/pkg/controller/middleware/csrf_test.go +++ b/pkg/controller/middleware/csrf_test.go @@ -20,7 +20,6 @@ import ( "testing" "github.com/google/exposure-notifications-verification-server/internal/project" - "github.com/google/exposure-notifications-verification-server/pkg/config" "github.com/google/exposure-notifications-verification-server/pkg/controller" "github.com/google/exposure-notifications-verification-server/pkg/controller/middleware" "github.com/google/exposure-notifications-verification-server/pkg/render" @@ -35,15 +34,7 @@ func TestConfigureCSRF(t *testing.T) { t.Fatal(err) } - key, err := project.RandomBytes(32) - if err != nil { - t.Fatal(err) - } - cfg := &config.ServerConfig{ - CSRFAuthKey: key, - } - - configureCSRF := middleware.ConfigureCSRF(cfg, h) + handleCSRF := middleware.HandleCSRF(h) r := httptest.NewRequest(http.MethodGet, "/", nil) r.Header.Set("Accept", "application/json") @@ -51,10 +42,16 @@ func TestConfigureCSRF(t *testing.T) { w := httptest.NewRecorder() // Verify the proper fields are added to the template map. - configureCSRF(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + handleCSRF(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctx := r.Context() m := controller.TemplateMapFromContext(ctx) + if _, ok := m["csrfHeaderField"]; !ok { + t.Errorf("expected csrfHeaderField to be populated in template map") + } + if _, ok := m["csrfMetaTagName"]; !ok { + t.Errorf("expected csrfMetaTagName to be populated in template map") + } if _, ok := m["csrfField"]; !ok { t.Errorf("expected csrfField to be populated in template map") } diff --git a/pkg/controller/session.go b/pkg/controller/session.go index df8624f3a..4194fb383 100644 --- a/pkg/controller/session.go +++ b/pkg/controller/session.go @@ -29,12 +29,42 @@ type sessionKey string const ( emailVerificationPrompted = sessionKey("emailVerificationPrompted") mfaPrompted = sessionKey("mfaPrompted") + passwordExpireWarned = sessionKey("passwordExpireWarned") + sessionKeyCSRFToken = sessionKey("csrfToken") sessionKeyLastActivity = sessionKey("lastActivity") sessionKeyRealmID = sessionKey("realmID") sessionKeyWelcomeMessageDisplayed = sessionKey("welcomeMessageDisplayed") - passwordExpireWarned = sessionKey("passwordExpireWarned") ) +// StoreSessionCSRFToken stores the CSRF token on the session. +func StoreSessionCSRFToken(session *sessions.Session, token []byte) { + if session == nil || len(token) == 0 { + return + } + session.Values[sessionKeyCSRFToken] = token +} + +// ClearSessionCSRFToken clears the CSRF token from the session. +func ClearSessionCSRFToken(session *sessions.Session) { + sessionClear(session, sessionKeyCSRFToken) +} + +// CSRFTokenFromSession extracts the CSRF token from the session. +func CSRFTokenFromSession(session *sessions.Session) []byte { + v := sessionGet(session, sessionKeyCSRFToken) + if v == nil { + return nil + } + + t, ok := v.([]byte) + if !ok { + delete(session.Values, sessionKeyCSRFToken) + return nil + } + + return t +} + // StoreSessionRealm stores the realm's ID in the session. func StoreSessionRealm(session *sessions.Session, realm *database.Realm) { if session == nil || realm == nil { diff --git a/terraform/main.tf b/terraform/main.tf index fcb897c1d..6a85642c4 100644 --- a/terraform/main.tf +++ b/terraform/main.tf @@ -175,7 +175,6 @@ resource "local_file" "env" { export PROJECT_ID="${var.project}" export REGION="${var.region}" -export CSRF_AUTH_KEY="secret://${google_secret_manager_secret_version.csrf-token-version.id}" export COOKIE_KEYS="secret://${google_secret_manager_secret_version.cookie-hmac-key-version.id},secret://${google_secret_manager_secret_version.cookie-encryption-key-version.id}" # Note: these configurations assume you're using the Cloud SQL proxy! diff --git a/terraform/services.tf b/terraform/services.tf index dd83b09ea..d7b7fc2c5 100644 --- a/terraform/services.tf +++ b/terraform/services.tf @@ -30,6 +30,7 @@ locals { BACKUP_DATABASE_NAME = google_sql_database.db.name } + # TODO(sethvargo): Remove in 0.26+ csrf_config = { CSRF_AUTH_KEY = "secret://${google_secret_manager_secret_version.csrf-token-version.id}" } diff --git a/terraform/sessions.tf b/terraform/sessions.tf index ea59c2304..8a1dc25a3 100644 --- a/terraform/sessions.tf +++ b/terraform/sessions.tf @@ -14,12 +14,13 @@ locals { session_secrets = [ - google_secret_manager_secret.csrf-token.id, + google_secret_manager_secret.csrf-token.id, # TODO(sethvargo): Remove in 0.26+ google_secret_manager_secret.cookie-hmac-key.id, google_secret_manager_secret.cookie-encryption-key.id, ] } +# TODO(sethvargo): Remove in 0.26+ resource "random_id" "csrf-token" { byte_length = 32 }