From cd51b41fac80843fe2defcb9d169598195ffa563 Mon Sep 17 00:00:00 2001 From: Hector Sanjuan Date: Mon, 6 Apr 2020 10:16:01 +0200 Subject: [PATCH 1/2] Fix: disallow POST without Origin nor Referer from specific user agents Addresses browsers being able to POST without control due to things like https://bugzilla.mozilla.org/show_bug.cgi?id=429594 --- http/config.go | 50 +++++++++++++++++++++++++++++++++++++++++++++ http/errors_test.go | 36 ++++++++++++++++++++++++++++++++ http/handler.go | 2 +- 3 files changed, 87 insertions(+), 1 deletion(-) diff --git a/http/config.go b/http/config.go index 0232922e..a3a25015 100644 --- a/http/config.go +++ b/http/config.go @@ -3,6 +3,7 @@ package http import ( "net/http" "net/url" + "strings" "sync" cors "github.com/rs/cors" @@ -14,6 +15,19 @@ const ( ACACredentials = "Access-Control-Allow-Credentials" ) +// disallowedUserAgents specifies a denylist of user agents that are not +// allowed to perform POST requests if they are not providing Origin +// and/or Referer headers. As mitigation for things like +// https://bugzilla.mozilla.org/show_bug.cgi?id=429594. Defaults to +// Firefox-related things. The matching against the user-agent string +// is made with strings.Contains(). +var disallowedUserAgents = []string{ + "Firefox", + "Focus", + "Klar", + "FxiOS", +} + type ServerConfig struct { // APIPath is the prefix of all request paths. // Example: host:port/api/v0/add. Here the APIPath is /api/v0 @@ -30,6 +44,14 @@ type ServerConfig struct { // websites to include resources from the API but not _read_ them. AllowGet bool + // DisallowUserAgents specifies a blacklist of user agents that are not + // allowed to perform POST requests if they are not providing Origin + // and/or Referer headers. As mitigation for things like + // https://bugzilla.mozilla.org/show_bug.cgi?id=429594. + // Defaults to ["Firefox"]. The matching against the user-agent + // string is made with strings.Contains(). + DisallowUserAgents []string + // corsOpts is a set of options for CORS headers. corsOpts *cors.Options @@ -150,3 +172,31 @@ func allowReferer(r *http.Request, cfg *ServerConfig) bool { return false } + +// allowUserAgent checks the request's user-agent against the list +// of DisallowUserAgents for requests with no origin nor referer set. +func allowUserAgent(r *http.Request, cfg *ServerConfig) bool { + // This check affects POST as we should never get POST requests from a + // browser without Origin or Referer, but we might: + // https://bugzilla.mozilla.org/show_bug.cgi?id=429594. + if r.Method != http.MethodPost { + return true + } + + origin := r.Header.Get("Origin") + referer := r.Referer() + + // If these are set, we leave up to CORS and CSRF checks. + if origin != "" || referer != "" { + return true + } + + // If not, check that request is not from a blacklisted UA. + ua := r.Header.Get("User-agent") + for _, forbiddenUA := range disallowedUserAgents { + if strings.Contains(ua, forbiddenUA) { + return false + } + } + return true +} diff --git a/http/errors_test.go b/http/errors_test.go index a320b7d7..c55d4237 100644 --- a/http/errors_test.go +++ b/http/errors_test.go @@ -170,3 +170,39 @@ func TestUnhandledMethod(t *testing.T) { } tc.test(t) } + +func TestDisallowedUserAgents(t *testing.T) { + tcs := []httpTestCase{ + { + // Block Firefox + Method: "POST", + AllowGet: false, + Code: http.StatusForbidden, + ReqHeaders: map[string]string{ + "User-Agent": "Mozilla/5.0 (X11; Linux x86_64; rv:10.0) Gecko/20100101 Firefox/10.0", + }, + }, + { + // Do not block on GETs + Method: "GET", + AllowGet: true, + Code: http.StatusOK, + ReqHeaders: map[string]string{ + "User-Agent": "Mozilla/5.0 (X11; Linux x86_64; rv:10.0) Gecko/20100101 Firefox/10.0", + }, + }, + { + // Do not block Chrome + Method: "POST", + AllowGet: false, + Code: http.StatusOK, + ReqHeaders: map[string]string{ + "User-Agent": "Mozilla/5.0 (Linux; U; Android 4.1.1; en-gb; Build/KLP) AppleWebKit/534.30 (KHTML, like Gecko) Version/4.0 Safari/534.30", + }, + }, + } + + for _, tc := range tcs { + tc.test(t) + } +} diff --git a/http/handler.go b/http/handler.go index b88e4e63..2152ea97 100644 --- a/http/handler.go +++ b/http/handler.go @@ -121,7 +121,7 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } - if !allowOrigin(r, h.cfg) || !allowReferer(r, h.cfg) { + if !allowOrigin(r, h.cfg) || !allowReferer(r, h.cfg) || !allowUserAgent(r, h.cfg) { http.Error(w, "403 - Forbidden", http.StatusForbidden) log.Warnf("API blocked request to %s. (possible CSRF)", r.URL) return From b00bc407c4e83444bf21a090f4170fd20717b7df Mon Sep 17 00:00:00 2001 From: Hector Sanjuan Date: Mon, 6 Apr 2020 13:31:49 +0200 Subject: [PATCH 2/2] http: block all ^Mozilla user agents without Origin nor Referer --- http/config.go | 36 +++++++++--------------------------- http/errors_test.go | 13 ++++++++----- 2 files changed, 17 insertions(+), 32 deletions(-) diff --git a/http/config.go b/http/config.go index a3a25015..6dc3877c 100644 --- a/http/config.go +++ b/http/config.go @@ -15,19 +15,6 @@ const ( ACACredentials = "Access-Control-Allow-Credentials" ) -// disallowedUserAgents specifies a denylist of user agents that are not -// allowed to perform POST requests if they are not providing Origin -// and/or Referer headers. As mitigation for things like -// https://bugzilla.mozilla.org/show_bug.cgi?id=429594. Defaults to -// Firefox-related things. The matching against the user-agent string -// is made with strings.Contains(). -var disallowedUserAgents = []string{ - "Firefox", - "Focus", - "Klar", - "FxiOS", -} - type ServerConfig struct { // APIPath is the prefix of all request paths. // Example: host:port/api/v0/add. Here the APIPath is /api/v0 @@ -44,14 +31,6 @@ type ServerConfig struct { // websites to include resources from the API but not _read_ them. AllowGet bool - // DisallowUserAgents specifies a blacklist of user agents that are not - // allowed to perform POST requests if they are not providing Origin - // and/or Referer headers. As mitigation for things like - // https://bugzilla.mozilla.org/show_bug.cgi?id=429594. - // Defaults to ["Firefox"]. The matching against the user-agent - // string is made with strings.Contains(). - DisallowUserAgents []string - // corsOpts is a set of options for CORS headers. corsOpts *cors.Options @@ -191,12 +170,15 @@ func allowUserAgent(r *http.Request, cfg *ServerConfig) bool { return true } - // If not, check that request is not from a blacklisted UA. + // Allow if the user agent does not start with Mozilla... (i.e. curl) ua := r.Header.Get("User-agent") - for _, forbiddenUA := range disallowedUserAgents { - if strings.Contains(ua, forbiddenUA) { - return false - } + if !strings.HasPrefix(ua, "Mozilla") { + return true } - return true + + // Disallow otherwise. + // + // This means the request probably came from a browser and thus, it + // should have included Origin or referer headers. + return false } diff --git a/http/errors_test.go b/http/errors_test.go index c55d4237..48b83593 100644 --- a/http/errors_test.go +++ b/http/errors_test.go @@ -174,7 +174,7 @@ func TestUnhandledMethod(t *testing.T) { func TestDisallowedUserAgents(t *testing.T) { tcs := []httpTestCase{ { - // Block Firefox + // Block Mozilla* browsers that do not provide origins. Method: "POST", AllowGet: false, Code: http.StatusForbidden, @@ -192,10 +192,13 @@ func TestDisallowedUserAgents(t *testing.T) { }, }, { - // Do not block Chrome - Method: "POST", - AllowGet: false, - Code: http.StatusOK, + // Do not block a Mozilla* browser that provides an + // allowed Origin + Method: "POST", + AllowGet: false, + AllowOrigins: []string{"*"}, + Origin: "null", + Code: http.StatusOK, ReqHeaders: map[string]string{ "User-Agent": "Mozilla/5.0 (Linux; U; Android 4.1.1; en-gb; Build/KLP) AppleWebKit/534.30 (KHTML, like Gecko) Version/4.0 Safari/534.30", },