net/http: keep sensitive headers on redirects to the same host

Preserve sensitive headers on a redirect to a different port of the same host.

Fixes #35104

Change-Id: I5ab57c414ce92a70e688ee684b9ff02fb062b3c6
GitHub-Last-Rev: 8d53e71e2243c141d70d27a503d0f7e6dee64c3c
GitHub-Pull-Request: golang/go#54539
Reviewed-on: https://go-review.googlesource.com/c/go/+/424935
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>

diff --git a/src/net/http/client.go b/src/net/http/client.go
index 122e8d030d0cde6797ad26742737a0f27d9efa27..2f7d4f696b7ec39a2702629720df22b76fb1eb53 100644 (file)
--- a/src/net/http/client.go
+++ b/src/net/http/client.go
@@ -990,8 +990,8 @@ func shouldCopyHeaderOnRedirect(headerKey string, initial, dest *url.URL) bool {
                // directly, we don't know their scope, so we assume
                // it's for *.domain.com.
 
-               ihost := canonicalAddr(initial)
-               dhost := canonicalAddr(dest)
+               ihost := idnaASCIIFromURL(initial)
+               dhost := idnaASCIIFromURL(dest)
                return isDomainOrSubdomain(dhost, ihost)
        }
        // All other headers are copied:

diff --git a/src/net/http/client_test.go b/src/net/http/client_test.go
index 8b53c41687a3f15ef8fa665520e0893ebeedbe03..bb1ed6f10cd08d512eaa0fdbeaac602f9df01daf 100644 (file)
--- a/src/net/http/client_test.go
+++ b/src/net/http/client_test.go
@@ -1470,6 +1470,9 @@ func TestClientRedirectResponseWithoutRequest(t *testing.T) {
 }
 
 // Issue 4800: copy (some) headers when Client follows a redirect.
+// Issue 35104: Since both URLs have the same host (localhost)
+// but different ports, sensitive headers like Cookie and Authorization
+// are preserved.
 func TestClientCopyHeadersOnRedirect(t *testing.T) { run(t, testClientCopyHeadersOnRedirect) }
 func testClientCopyHeadersOnRedirect(t *testing.T, mode testMode) {
        const (
@@ -1483,6 +1486,8 @@ func testClientCopyHeadersOnRedirect(t *testing.T, mode testMode) {
                        "X-Foo":           []string{xfoo},
                        "Referer":         []string{ts2URL},
                        "Accept-Encoding": []string{"gzip"},
+                       "Cookie":          []string{"foo=bar"},
+                       "Authorization":   []string{"secretpassword"},
                }
                if !reflect.DeepEqual(r.Header, want) {
                        t.Errorf("Request.Header = %#v; want %#v", r.Header, want)
@@ -1501,9 +1506,11 @@ func testClientCopyHeadersOnRedirect(t *testing.T, mode testMode) {
        c := ts1.Client()
        c.CheckRedirect = func(r *Request, via []*Request) error {
                want := Header{
-                       "User-Agent": []string{ua},
-                       "X-Foo":      []string{xfoo},
-                       "Referer":    []string{ts2URL},
+                       "User-Agent":    []string{ua},
+                       "X-Foo":         []string{xfoo},
+                       "Referer":       []string{ts2URL},
+                       "Cookie":        []string{"foo=bar"},
+                       "Authorization": []string{"secretpassword"},
                }
                if !reflect.DeepEqual(r.Header, want) {
                        t.Errorf("CheckRedirect Request.Header = %#v; want %#v", r.Header, want)
@@ -1707,18 +1714,30 @@ func TestShouldCopyHeaderOnRedirect(t *testing.T) {
                {"cookie", "http://foo.com/", "http://bar.com/", false},
                {"cookie2", "http://foo.com/", "http://bar.com/", false},
                {"authorization", "http://foo.com/", "http://bar.com/", false},
+               {"authorization", "http://foo.com/", "https://foo.com/", true},
+               {"authorization", "http://foo.com:1234/", "http://foo.com:4321/", true},
                {"www-authenticate", "http://foo.com/", "http://bar.com/", false},
 
                // But subdomains should work:
                {"www-authenticate", "http://foo.com/", "http://foo.com/", true},
                {"www-authenticate", "http://foo.com/", "http://sub.foo.com/", true},
                {"www-authenticate", "http://foo.com/", "http://notfoo.com/", false},
-               {"www-authenticate", "http://foo.com/", "https://foo.com/", false},
+               {"www-authenticate", "http://foo.com/", "https://foo.com/", true},
                {"www-authenticate", "http://foo.com:80/", "http://foo.com/", true},
                {"www-authenticate", "http://foo.com:80/", "http://sub.foo.com/", true},
                {"www-authenticate", "http://foo.com:443/", "https://foo.com/", true},
                {"www-authenticate", "http://foo.com:443/", "https://sub.foo.com/", true},
-               {"www-authenticate", "http://foo.com:1234/", "http://foo.com/", false},
+               {"www-authenticate", "http://foo.com:1234/", "http://foo.com/", true},
+
+               {"authorization", "http://foo.com/", "http://foo.com/", true},
+               {"authorization", "http://foo.com/", "http://sub.foo.com/", true},
+               {"authorization", "http://foo.com/", "http://notfoo.com/", false},
+               {"authorization", "http://foo.com/", "https://foo.com/", true},
+               {"authorization", "http://foo.com:80/", "http://foo.com/", true},
+               {"authorization", "http://foo.com:80/", "http://sub.foo.com/", true},
+               {"authorization", "http://foo.com:443/", "https://foo.com/", true},
+               {"authorization", "http://foo.com:443/", "https://sub.foo.com/", true},
+               {"authorization", "http://foo.com:1234/", "http://foo.com/", true},
        }
        for i, tt := range tests {
                u0, err := url.Parse(tt.initialURL)

diff --git a/src/net/http/transport.go b/src/net/http/transport.go
index b8e4c4e97b26c68670aff8dd65971ae2dfb7bc9b..7561f7f5cb524a0875b0f3cf863585905b32b3dc 100644 (file)
--- a/src/net/http/transport.go
+++ b/src/net/http/transport.go
@@ -2750,17 +2750,21 @@ var portMap = map[string]string{
        "socks5": "1080",
 }
 
-// canonicalAddr returns url.Host but always with a ":port" suffix.
-func canonicalAddr(url *url.URL) string {
+func idnaASCIIFromURL(url *url.URL) string {
        addr := url.Hostname()
        if v, err := idnaASCII(addr); err == nil {
                addr = v
        }
+       return addr
+}
+
+// canonicalAddr returns url.Host but always with a ":port" suffix.
+func canonicalAddr(url *url.URL) string {
        port := url.Port()
        if port == "" {
                port = portMap[url.Scheme]
        }
-       return net.JoinHostPort(addr, port)
+       return net.JoinHostPort(idnaASCIIFromURL(url), port)
 }
 
 // bodyEOFSignal is used by the HTTP/1 transport when reading response