From 049be2f16a585e45be4d8fe17660a537934488ee Mon Sep 17 00:00:00 2001 From: Jens Frederich Date: Tue, 7 Oct 2014 07:13:42 -0700 Subject: net/http: fix authentication info leakage in Referer header (potential security risk) http.Client calls URL.String() to fill in the Referer header, which may contain authentication info. This patch removes authentication info from the Referer header without introducing any API changes. A new test for net/http is also provided. This is the polished version of Alberto Garc?a Hierro's https://codereview.appspot.com/9766046/ It should handle https Referer right. Fixes issue 8417 LGTM=bradfitz R=golang-codereviews, gobot, bradfitz, mikioh.mikioh CC=golang-codereviews https://codereview.appspot.com/151430043 Committer: Brad Fitzpatrick --- src/net/http/client.go | 28 ++++++++++++++++++++++++++-- src/net/http/client_test.go | 37 +++++++++++++++++++++++++++++++++++++ src/net/http/export_test.go | 5 +++++ 3 files changed, 68 insertions(+), 2 deletions(-) (limited to 'src/net') diff --git a/src/net/http/client.go b/src/net/http/client.go index a5a3abe61..ce884d1f0 100644 --- a/src/net/http/client.go +++ b/src/net/http/client.go @@ -101,6 +101,30 @@ type RoundTripper interface { // return true if the string includes a port. func hasPort(s string) bool { return strings.LastIndex(s, ":") > strings.LastIndex(s, "]") } +// refererForURL returns a referer without any authentication info or +// an empty string if lastReq scheme is https and newReq scheme is http. +func refererForURL(lastReq, newReq *url.URL) string { + // https://tools.ietf.org/html/rfc7231#section-5.5.2 + // "Clients SHOULD NOT include a Referer header field in a + // (non-secure) HTTP request if the referring page was + // transferred with a secure protocol." + if lastReq.Scheme == "https" && newReq.Scheme == "http" { + return "" + } + referer := lastReq.String() + if lastReq.User != nil { + // This is not very efficient, but is the best we can + // do without: + // - introducing a new method on URL + // - creating a race condition + // - copying the URL struct manually, which would cause + // maintenance problems down the line + auth := lastReq.User.String() + "@" + referer = strings.Replace(referer, auth, "", 1) + } + return referer +} + // Used in Send to implement io.ReadCloser by bundling together the // bufio.Reader through which we read the response, and the underlying // network connection. @@ -324,8 +348,8 @@ func (c *Client) doFollowingRedirects(ireq *Request, shouldRedirect func(int) bo if len(via) > 0 { // Add the Referer header. lastReq := via[len(via)-1] - if lastReq.URL.Scheme != "https" { - nreq.Header.Set("Referer", lastReq.URL.String()) + if ref := refererForURL(lastReq.URL, nreq.URL); ref != "" { + nreq.Header.Set("Referer", ref) } err = redirectChecker(nreq, via) diff --git a/src/net/http/client_test.go b/src/net/http/client_test.go index 6392c1baf..56b6563c4 100644 --- a/src/net/http/client_test.go +++ b/src/net/http/client_test.go @@ -1036,3 +1036,40 @@ func TestClientTrailers(t *testing.T) { t.Errorf("Response trailers = %#v; want %#v", res.Trailer, want) } } + +func TestReferer(t *testing.T) { + tests := []struct { + lastReq, newReq string // from -> to URLs + want string + }{ + // don't send user: + {"http://gopher@test.com", "http://link.com", "http://test.com"}, + {"https://gopher@test.com", "https://link.com", "https://test.com"}, + + // don't send a user and password: + {"http://gopher:go@test.com", "http://link.com", "http://test.com"}, + {"https://gopher:go@test.com", "https://link.com", "https://test.com"}, + + // nothing to do: + {"http://test.com", "http://link.com", "http://test.com"}, + {"https://test.com", "https://link.com", "https://test.com"}, + + // https to http doesn't send a referer: + {"https://test.com", "http://link.com", ""}, + {"https://gopher:go@test.com", "http://link.com", ""}, + } + for _, tt := range tests { + l, err := url.Parse(tt.lastReq) + if err != nil { + t.Fatal(err) + } + n, err := url.Parse(tt.newReq) + if err != nil { + t.Fatal(err) + } + r := ExportRefererForURL(l, n) + if r != tt.want { + t.Errorf("refererForURL(%q, %q) = %q; want %q", tt.lastReq, tt.newReq, r, tt.want) + } + } +} diff --git a/src/net/http/export_test.go b/src/net/http/export_test.go index a6980b538..87b6c0773 100644 --- a/src/net/http/export_test.go +++ b/src/net/http/export_test.go @@ -9,6 +9,7 @@ package http import ( "net" + "net/url" "time" ) @@ -92,6 +93,10 @@ func ResetCachedEnvironment() { var DefaultUserAgent = defaultUserAgent +func ExportRefererForURL(lastReq, newReq *url.URL) string { + return refererForURL(lastReq, newReq) +} + // SetPendingDialHooks sets the hooks that run before and after handling // pending dials. func SetPendingDialHooks(before, after func()) { -- cgit v1.2.1