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 <bradfitz@golang.org>

3 files changed, 68 insertions, 2 deletions

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()) {