Repository: google/go-safeweb
Pull request #286 (2021-05-06 09:29:14)

xsrfhtml: relax action check to host.
Currently we validate CSRF tokens based on paths, but this makes
writing applications very hard.

For example a "logout" button cannot post to a "/logout" endpoint
from a "/profile" page or it will be blocked.

This protection mechanism is aimed at protecting from cross-site
forgery anyways, not cross-path forgery.


Patch:
diff --git a/safehttp/plugins/xsrf/xsrfhtml/xsrf.go b/safehttp/plugins/xsrf/xsrfhtml/xsrf.go
index 17b5400..d7b4ee9 100644
--- a/safehttp/plugins/xsrf/xsrfhtml/xsrf.go
+++ b/safehttp/plugins/xsrf/xsrfhtml/xsrf.go
@@ -84,7 +84,7 @@ func (it *Interceptor) Before(w safehttp.ResponseWriter, r *safehttp.IncomingReq
 		return w.WriteError(safehttp.StatusUnauthorized)
 	}
 
-	if ok := xsrftoken.Valid(tok, it.SecretAppKey, cookieID.Value(), r.URL.Path()); !ok {
+	if ok := xsrftoken.Valid(tok, it.SecretAppKey, cookieID.Value(), r.URL.Host()); !ok {
 		return w.WriteError(safehttp.StatusForbidden)
 	}
 
@@ -125,7 +125,7 @@ func (it *Interceptor) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.Inco
 		return
 	}
 
-	tok := xsrftoken.Generate(it.SecretAppKey, cookieID.Value(), r.URL.Path())
+	tok := xsrftoken.Generate(it.SecretAppKey, cookieID.Value(), r.URL.Host())
 	if tmplResp.FuncMap == nil {
 		tmplResp.FuncMap = map[string]interface{}{}
 	}


Test patch:
diff --git a/internal/requesttesting/headers/host_test.go b/internal/requesttesting/headers/host_test.go
index a1b0134..6846df5 100644
--- a/internal/requesttesting/headers/host_test.go
+++ b/internal/requesttesting/headers/host_test.go
@@ -17,7 +17,6 @@ package headers
 import (
 	"context"
 	"net/http"
-	"strings"
 	"testing"
 
 	"github.com/google/go-safeweb/internal/requesttesting"
@@ -101,7 +100,7 @@ func TestHostHeaderMultiple(t *testing.T) {
 		t.Fatalf("MakeRequest() got err: %v", err)
 	}
 
-	if got, want := extractStatus(resp), statusBadRequest; !strings.HasPrefix(got, want) {
+	if got, want := extractStatus(resp), statusTooManyHostHeaders; got != want {
 		t.Errorf("status code got: %q want: %q", got, want)
 	}
 }
diff --git a/internal/requesttesting/headers/statusmessages.go b/internal/requesttesting/headers/statusmessages.go
index 6c1ef6b..146b812 100644
--- a/internal/requesttesting/headers/statusmessages.go
+++ b/internal/requesttesting/headers/statusmessages.go
@@ -18,6 +18,7 @@ import "bytes"
 
 const (
 	statusOK                 = "HTTP/1.1 200 OK"
+	statusTooManyHostHeaders = "HTTP/1.1 400 Bad Request: too many Host headers"
 	statusNotImplemented     = "HTTP/1.1 501 Not Implemented"
 	statusBadRequest         = "HTTP/1.1 400 Bad Request"
 	statusInvalidHeaderName  = "HTTP/1.1 400 Bad Request: invalid header name"
diff --git a/safehttp/plugins/xsrf/xsrfhtml/xsrf_test.go b/safehttp/plugins/xsrf/xsrfhtml/xsrf_test.go
index 1220e55..ab94806 100644
--- a/safehttp/plugins/xsrf/xsrfhtml/xsrf_test.go
+++ b/safehttp/plugins/xsrf/xsrfhtml/xsrf_test.go
@@ -25,25 +25,25 @@ import (
 
 var (
 	formTokenTests = []struct {
-		name, cookieVal, actionID string
-		wantStatus                safehttp.StatusCode
+		name, cookieVal, host string
+		wantStatus            safehttp.StatusCode
 	}{
 		{
 			name:       "Valid token",
 			cookieVal:  "abcdef",
-			actionID:   "/pizza",
+			host:       "go.dev",
 			wantStatus: safehttp.StatusOK,
 		},
 		{
-			name:       "Invalid actionID in token generation",
+			name:       "Invalid host in token generation",
 			cookieVal:  "abcdef",
-			actionID:   "/spaghetti",
+			host:       "google.com",
 			wantStatus: safehttp.StatusForbidden,
 		},
 		{
 			name:       "Invalid cookie value in token generation",
 			cookieVal:  "evilvalue",
-			actionID:   "/pizza",
+			host:       "go.dev",
 			wantStatus: safehttp.StatusForbidden,
 		},
 	}
@@ -53,8 +53,8 @@ func TestTokenPost(t *testing.T) {
 	for _, test := range formTokenTests {
 		t.Run(test.name, func(t *testing.T) {
 			fakeRW, rr := safehttptest.NewFakeResponseWriter()
-			tok := xsrftoken.Generate("testSecretAppKey", test.cookieVal, test.actionID)
-			req := safehttptest.NewRequest(safehttp.MethodPost, "https://foo.com/pizza", strings.NewReader(TokenKey+"="+tok))
+			tok := xsrftoken.Generate("testSecretAppKey", test.cookieVal, test.host)
+			req := safehttptest.NewRequest(safehttp.MethodPost, "https://go.dev/", strings.NewReader(TokenKey+"="+tok))
 			req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
 			req.Header.Set("Cookie", cookieIDKey+"=abcdef")
 
@@ -72,13 +72,13 @@ func TestTokenMultipart(t *testing.T) {
 	for _, test := range formTokenTests {
 		t.Run(test.name, func(t *testing.T) {
 			fakeRW, rr := safehttptest.NewFakeResponseWriter()
-			tok := xsrftoken.Generate("testSecretAppKey", test.cookieVal, test.actionID)
+			tok := xsrftoken.Generate("testSecretAppKey", test.cookieVal, test.host)
 			b := "--123\r\n" +
 				"Content-Disposition: form-data; name=\"xsrf-token\"\r\n" +
 				"\r\n" +
 				tok + "\r\n" +
 				"--123--\r\n"
-			req := safehttptest.NewRequest(safehttp.MethodPost, "https://foo.com/pizza", strings.NewReader(b))
+			req := safehttptest.NewRequest(safehttp.MethodPost, "https://go.dev/", strings.NewReader(b))
 			req.Header.Set("Content-Type", `multipart/form-data; boundary="123"`)
 			req.Header.Set("Cookie", cookieIDKey+"=abcdef")