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://golang.org/cl/9766046/

It should handle https Referer right.

Fixes #8417

LGTM=bradfitz
R=golang-codereviews, gobot, bradfitz, mikioh.mikioh
CC=golang-codereviews
https://golang.org/cl/151430043

diff --git a/src/net/http/client.go b/src/net/http/client.go
index a5a3abe6138a596fbae22bf7e64b50651b638add..ce884d1f07bf519580ae1321899e3466b35de87a 100644 (file)
--- 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 6392c1baf39cdf83852a51e0aca8feb9cd9a1e23..56b6563c486750edb2a3efc1f754a4d75efa7200 100644 (file)
--- 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 a6980b5389fd9dcc1db2e59b2bf5bc4c6f9e9a1c..87b6c0773aa5cce710fd9130796827deac53062b 100644 (file)
--- 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()) {