From eccb945d92ba5e7f88c6f4f0e810862588ebd688 Mon Sep 17 00:00:00 2001
From: Gustavo Falco <comfortablynumb84@gmail.com>
Date: Sun, 11 Dec 2022 02:39:20 +0000
Subject: [PATCH 2/4] [Backport] net/http, net/http/cookiejar: avoid subdomain
 matches on IPv6 zones

Offering: Cloud Core Network
CVE: CVE-2023-45289
Reference: https://go-review.googlesource.com/c/go/+/569340

When deciding whether to forward cookies or sensitive headers
across a redirect, do not attempt to interpret an IPv6 address
as a domain name.

Avoids a case where a maliciously-crafted redirect to an
IPv6 address with a scoped addressing zone could be
misinterpreted as a within-domain redirect. For example,
we could interpret "::1%.www.example.com" as a subdomain
of "www.example.com".

Thanks to Juho Nurminen of Mattermost for reporting this issue.

Note: The upstream does not submit this change to go1.16 according to the rules of MinorReleases.
Corego2.x are based on go1.16.5. Therefore, it need to submit the change to corego2.x.

Edited-by: zhaoshengwei z00581105

Fixes CVE-2023-45289
Fixes #65065

Change-Id: I8f463f59f0e700c8a18733d2b264a8bcb3a19599
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2131938
Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
Reviewed-by: Roland Shoemaker <bracewell@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/569340
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Signed-off-by: Zhao Sheng Wei zhaoshengwei@huawei.com

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

Offering: Cloud Core Network
Reference: https://go-review.googlesource.com/c/go/+/424935

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

Note: The upstream does not submit this change to go1.16 according to the rules of MinorReleases.
Corego2.x are based on go1.16.5. Therefore, it need to submit the change to corego2.x.

Edited-by: zhaoshengwei z00581105

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>
Signed-off-by: Zhao Sheng Wei zhaoshengwei@huawei.com
---
 src/net/http/client.go             | 10 ++++++++--
 src/net/http/client_test.go        | 30 +++++++++++++++++++++++++-----
 src/net/http/cookiejar/jar.go      |  7 +++++++
 src/net/http/cookiejar/jar_test.go | 10 ++++++++++
 src/net/http/transport.go          | 10 +++++++---
 5 files changed, 57 insertions(+), 10 deletions(-)

diff --git a/src/net/http/client.go b/src/net/http/client.go
index 3860d97d8f..5f24bfce7a 100644
--- a/src/net/http/client.go
+++ b/src/net/http/client.go
@@ -976,8 +976,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:
@@ -992,6 +992,12 @@ func isDomainOrSubdomain(sub, parent string) bool {
 	if sub == parent {
 		return true
 	}
+	// If sub contains a :, it's probably an IPv6 address (and is definitely not a hostname).
+	// Don't check the suffix in this case, to avoid matching the contents of a IPv6 zone.
+	// For example, "::1%.www.example.com" is not a subdomain of "www.example.com".
+	if strings.ContainsAny(sub, ":%") {
+		return false
+	}
 	// If sub is "foo.example.com" and parent is "example.com",
 	// that means sub must end in "."+parent.
 	// Do it without allocating.
diff --git a/src/net/http/client_test.go b/src/net/http/client_test.go
index 80807fae7a..ce476d1a34 100644
--- a/src/net/http/client_test.go
+++ b/src/net/http/client_test.go
@@ -1466,6 +1466,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) {
 	const (
 		ua   = "some-agent/1.2"
@@ -1478,6 +1481,8 @@ func TestClientCopyHeadersOnRedirect(t *testing.T) {
 			"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)
@@ -1498,9 +1503,11 @@ func TestClientCopyHeadersOnRedirect(t *testing.T) {
 	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)
@@ -1702,18 +1709,31 @@ 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},
+		{"authorization", "http://foo.com/", "http://[::1%25.foo.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/cookiejar/jar.go b/src/net/http/cookiejar/jar.go
index 9f19917084..18cbfc272d 100644
--- a/src/net/http/cookiejar/jar.go
+++ b/src/net/http/cookiejar/jar.go
@@ -356,6 +356,13 @@ func jarKey(host string, psl PublicSuffixList) string {
 
 // isIP reports whether host is an IP address.
 func isIP(host string) bool {
+	if strings.ContainsAny(host, ":%") {
+		// Probable IPv6 address.
+		// Hostnames can't contain : or %, so this is definitely not a valid host.
+		// Treating it as an IP is the more conservative option, and avoids the risk
+		// of interpeting ::1%.www.example.com as a subtomain of www.example.com.
+		return true
+	}
 	return net.ParseIP(host) != nil
 }
 
diff --git a/src/net/http/cookiejar/jar_test.go b/src/net/http/cookiejar/jar_test.go
index 47fb1abdaa..fd8d40ed1b 100644
--- a/src/net/http/cookiejar/jar_test.go
+++ b/src/net/http/cookiejar/jar_test.go
@@ -251,6 +251,7 @@ var isIPTests = map[string]bool{
 	"127.0.0.1":            true,
 	"1.2.3.4":              true,
 	"2001:4860:0:2001::68": true,
+	"::1%zone":             true,
 	"example.com":          false,
 	"1.1.1.300":            false,
 	"www.foo.bar.net":      false,
@@ -613,6 +614,15 @@ var basicsTests = [...]jarTest{
 			{"http://www.host.test:1234/", "a=1"},
 		},
 	},
+	{
+		"IPv6 zone is not treated as a host.",
+		"https://example.com/",
+		[]string{"a=1"},
+		"a=1",
+		[]query{
+			{"https://[::1%25.example.com]:80/", ""},
+		},
+	},
 }
 
 func TestBasics(t *testing.T) {
diff --git a/src/net/http/transport.go b/src/net/http/transport.go
index 88d15a5919..e48026b7be 100644
--- a/src/net/http/transport.go
+++ b/src/net/http/transport.go
@@ -2678,17 +2678,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
-- 
2.33.0

