[go] net/http: validate Host header before sending
103 views
Skip to first unread message
Damien Neil (Gerrit)
Jun 28, 2023, 4:23:31 PM6/28/23
to goph...@pubsubhelper.golang.org, Tatiana Bradley, golang-co...@googlegroups.com
Attention is currently required from: Tatiana Bradley.
To view, visit change 506996. To unsubscribe, or for help writing mail filters, visit settings.
Damien Neil (Gerrit)
Jun 28, 2023, 4:50:26 PM6/28/23
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Damien Neil, Tatiana Bradley.
Damien Neil uploaded patch set #2 to this change.
The following approvals got outdated and were removed: Run-TryBot+1 by Damien Neil, TryBot-Result-1 by Gopher Robot
net/http: validate Host header before sending
Verify that the Host header we send is valid.
Avoids surprising behavior such as a Host of "go.dev\r\nX-Evil:oops"
adding an X-Evil header to HTTP/1 requests.
Add a test, skip the test for HTTP/2. HTTP/2 is not vulnerable to
header injection in the way HTTP/1 is, but x/net/http2 doesn't validate
the header and will go into a retry loop when the server rejects it.
CL 506995 adds the necessary validation to x/net/http2.
For #60374
Change-Id: I05cb6866a9bead043101954dfded199258c6dd04
---
M src/net/http/http_test.go
M src/net/http/request.go
M src/net/http/request_test.go
M src/net/http/transport_test.go
4 files changed, 31 insertions(+), 75 deletions(-)
To view, visit change 506996. To unsubscribe, or for help writing mail filters, visit settings.
Tatiana Bradley (Gerrit)
Jun 29, 2023, 11:41:49 AM6/29/23
to Damien Neil, goph...@pubsubhelper.golang.org, Gopher Robot, golang-co...@googlegroups.com
Damien Neil (Gerrit)
Jun 29, 2023, 1:00:10 PM6/29/23
to goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Tatiana Bradley, Gopher Robot, golang-co...@googlegroups.com
Damien Neil submitted this change.
Approvals:
Tatiana Bradley: Looks good to me, approved
Gopher Robot: TryBots succeeded
Damien Neil: Run TryBots
net/http: validate Host header before sending
Verify that the Host header we send is valid.
Avoids surprising behavior such as a Host of "go.dev\r\nX-Evil:oops"
adding an X-Evil header to HTTP/1 requests.
Add a test, skip the test for HTTP/2. HTTP/2 is not vulnerable to
header injection in the way HTTP/1 is, but x/net/http2 doesn't validate
the header and will go into a retry loop when the server rejects it.
CL 506995 adds the necessary validation to x/net/http2.
For #60374
Change-Id: I05cb6866a9bead043101954dfded199258c6dd04
Reviewed-on: https://go-review.googlesource.com/c/go/+/506996
Reviewed-by: Tatiana Bradley <tatiana...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
Run-TryBot: Damien Neil <dn...@google.com>
---
M src/net/http/http_test.go
M src/net/http/request.go
M src/net/http/request_test.go
M src/net/http/transport_test.go
4 files changed, 31 insertions(+), 75 deletions(-)
diff --git a/src/net/http/http_test.go b/src/net/http/http_test.go
index 1c9fb33..91bb1b2 100644
--- a/src/net/http/http_test.go
+++ b/src/net/http/http_test.go
@@ -48,35 +48,6 @@
}
}
-func TestCleanHost(t *testing.T) {
- tests := []struct {
- in, want string
- }{
- {"www.google.com", "www.google.com"},
- {"www.google.com foo", "www.google.com"},
- {"www.google.com/foo", "www.google.com"},
- {" first character is a space", ""},
- {"[1::6]:8080", "[1::6]:8080"},
-
- // Punycode:
- {"гофер.рф/foo", "xn--c1ae0ajs.xn--p1ai"},
- {"bücher.de", "xn--bcher-kva.de"},
- {"bücher.de:8080", "xn--bcher-kva.de:8080"},
- // Verify we convert to lowercase before punycode:
- {"BÜCHER.de", "xn--bcher-kva.de"},
- {"BÜCHER.de:8080", "xn--bcher-kva.de:8080"},
- // Verify we normalize to NFC before punycode:
- {"gophér.nfc", "xn--gophr-esa.nfc"}, // NFC input; no work needed
- {"goph\u0065\u0301r.nfd", "xn--gophr-esa.nfd"}, // NFD input
- }
- for _, tt := range tests {
- got := cleanHost(tt.in)
- if tt.want != got {
- t.Errorf("cleanHost(%q) = %q, want %q", tt.in, got, tt.want)
- }
- }
-}
-
// Test that cmd/go doesn't link in the HTTP server.
//
// This catches accidental dependencies between the HTTP transport and
diff --git a/src/net/http/request.go b/src/net/http/request.go
index 4e91904..bd86837 100644
--- a/src/net/http/request.go
+++ b/src/net/http/request.go
@@ -17,7 +17,6 @@
"io"
"mime"
"mime/multipart"
- "net"
"net/http/httptrace"
"net/http/internal/ascii"
"net/textproto"
@@ -27,6 +26,7 @@
"strings"
"sync"
+ "golang.org/x/net/http/httpguts"
"golang.org/x/net/idna"
)
@@ -580,12 +580,19 @@
// is not given, use the host from the request URL.
//
// Clean the host, in case it arrives with unexpected stuff in it.
- host := cleanHost(r.Host)
+ host := r.Host
if host == "" {
if r.URL == nil {
return errMissingHost
}
- host = cleanHost(r.URL.Host)
+ host = r.URL.Host
+ }
+ host, err = httpguts.PunycodeHostPort(host)
+ if err != nil {
+ return err
+ }
+ if !httpguts.ValidHostHeader(host) {
+ return errors.New("http: invalid Host header")
}
// According to RFC 6874, an HTTP client, proxy, or other
@@ -742,40 +749,6 @@
return idna.Lookup.ToASCII(v)
}
-// cleanHost cleans up the host sent in request's Host header.
-//
-// It both strips anything after '/' or ' ', and puts the value
-// into Punycode form, if necessary.
-//
-// Ideally we'd clean the Host header according to the spec:
-//
-// https://tools.ietf.org/html/rfc7230#section-5.4 (Host = uri-host [ ":" port ]")
-// https://tools.ietf.org/html/rfc7230#section-2.7 (uri-host -> rfc3986's host)
-// https://tools.ietf.org/html/rfc3986#section-3.2.2 (definition of host)
-//
-// But practically, what we are trying to avoid is the situation in
-// issue 11206, where a malformed Host header used in the proxy context
-// would create a bad request. So it is enough to just truncate at the
-// first offending character.
-func cleanHost(in string) string {
- if i := strings.IndexAny(in, " /"); i != -1 {
- in = in[:i]
- }
- host, port, err := net.SplitHostPort(in)
- if err != nil { // input was just a host
- a, err := idnaASCII(in)
- if err != nil {
- return in // garbage in, garbage out
- }
- return a
- }
- a, err := idnaASCII(host)
- if err != nil {
- return in // garbage in, garbage out
- }
- return net.JoinHostPort(a, port)
-}
-
// removeZone removes IPv6 zone identifier from host.
// E.g., "[fe80::1%en0]:8080" to "[fe80::1]:8080"
func removeZone(host string) string {
diff --git a/src/net/http/request_test.go b/src/net/http/request_test.go
index 78b968f..0892bc2 100644
--- a/src/net/http/request_test.go
+++ b/src/net/http/request_test.go
@@ -775,15 +775,8 @@
}
req.Host = "foo.com with spaces"
req.URL.Host = "foo.com with spaces"
- req.Write(logWrites{t, &got})
- want := []string{
- "GET /after HTTP/1.1\r\n",
- "Host: foo.com\r\n",
- "User-Agent: " + DefaultUserAgent + "\r\n",
- "\r\n",
- }
- if !reflect.DeepEqual(got, want) {
- t.Errorf("Writes = %q\n Want = %q", got, want)
+ if err := req.Write(logWrites{t, &got}); err == nil {
+ t.Errorf("Writing request with invalid Host: succeded, want error")
}
}
diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go
index 172aba6..028fecc 100644
--- a/src/net/http/transport_test.go
+++ b/src/net/http/transport_test.go
@@ -6731,3 +6731,22 @@
}
wg.Wait()
}
+
+func TestRequestSanitization(t *testing.T) { run(t, testRequestSanitization) }
+func testRequestSanitization(t *testing.T, mode testMode) {
+ if mode == http2Mode {
+ // Remove this after updating x/net.
+ t.Skip("https://go.dev/issue/60374 test fails when run with HTTP/2")
+ }
+ ts := newClientServerTest(t, mode, HandlerFunc(func(rw ResponseWriter, req *Request) {
+ if h, ok := req.Header["X-Evil"]; ok {
+ t.Errorf("request has X-Evil header: %q", h)
+ }
+ })).ts
+ req, _ := NewRequest("GET", ts.URL, nil)
+ req.Host = "go.dev\r\nX-Evil:evil"
+ resp, _ := ts.Client().Do(req)
+ if resp != nil {
+ resp.Body.Close()
+ }
+}
To view, visit change 506996. To unsubscribe, or for help writing mail filters, visit settings.
Tatiana Bradley (Gerrit)
Jun 30, 2023, 2:05:15 PM6/30/23
to goph...@pubsubhelper.golang.org, Damien Neil, golang-co...@googlegroups.com
Tatiana Bradley has uploaded this change for review.
net/http: validate Host header before sending
Verify that the Host header we send is valid.
Avoids surprising behavior such as a Host of "go.dev\r\nX-Evil:oops"
adding an X-Evil header to HTTP/1 requests.
Add a test, skip the test for HTTP/2. HTTP/2 is not vulnerable to
header injection in the way HTTP/1 is, but x/net/http2 doesn't validate
the header and will go into a retry loop when the server rejects it.
CL 506995 adds the necessary validation to x/net/http2.
For #60374
For #61076
For CVE-2023-29406
Change-Id: I05cb6866a9bead043101954dfded199258c6dd04
Reviewed-on: https://go-review.googlesource.com/c/go/+/506996
Reviewed-by: Tatiana Bradley <tatiana...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
Run-TryBot: Damien Neil <dn...@google.com>
(cherry picked from commit 499458f7ca04087958987a33c2703c3ef03e27e2)
---
M src/net/http/http_test.go
M src/net/http/request.go
M src/net/http/request_test.go
M src/net/http/transport_test.go
4 files changed, 31 insertions(+), 75 deletions(-)
diff --git a/src/net/http/http_test.go b/src/net/http/http_test.go
index 0d92fe5..f03272a 100644
index a45c9e3..9c888b3 100644
--- a/src/net/http/request.go
+++ b/src/net/http/request.go
@@ -17,7 +17,6 @@
"io"
"mime"
"mime/multipart"
- "net"
"net/http/httptrace"
"net/http/internal/ascii"
"net/textproto"
@@ -27,6 +26,7 @@
"strings"
"sync"
+ "golang.org/x/net/http/httpguts"
"golang.org/x/net/idna"
)
@@ -575,12 +575,19 @@
// is not given, use the host from the request URL.
//
// Clean the host, in case it arrives with unexpected stuff in it.
- host := cleanHost(r.Host)
+ host := r.Host
if host == "" {
if r.URL == nil {
return errMissingHost
}
- host = cleanHost(r.URL.Host)
+ host = r.URL.Host
+ }
+ host, err = httpguts.PunycodeHostPort(host)
+ if err != nil {
+ return err
+ }
+ if !httpguts.ValidHostHeader(host) {
+ return errors.New("http: invalid Host header")
}
// According to RFC 6874, an HTTP client, proxy, or other
@@ -737,40 +744,6 @@index 23e49d6..86c68e4 100644
--- a/src/net/http/request_test.go
+++ b/src/net/http/request_test.go
@@ -774,15 +774,8 @@
}
req.Host = "foo.com with spaces"
req.URL.Host = "foo.com with spaces"
- req.Write(logWrites{t, &got})
- want := []string{
- "GET /after HTTP/1.1\r\n",
- "Host: foo.com\r\n",
- "User-Agent: " + DefaultUserAgent + "\r\n",
- "\r\n",
- }
- if !reflect.DeepEqual(got, want) {
- t.Errorf("Writes = %q\n Want = %q", got, want)
+ if err := req.Write(logWrites{t, &got}); err == nil {
+ t.Errorf("Writing request with invalid Host: succeded, want error")
}
}
diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go
index 245f73b..f4896c5 100644
--- a/src/net/http/transport_test.go
+++ b/src/net/http/transport_test.go
@@ -6654,3 +6654,22 @@
}
wg.Wait()
}
+
+func TestRequestSanitization(t *testing.T) { run(t, testRequestSanitization) }
+func testRequestSanitization(t *testing.T, mode testMode) {
+ if mode == http2Mode {
+ // Remove this after updating x/net.
+ t.Skip("https://go.dev/issue/60374 test fails when run with HTTP/2")
+ }
+ ts := newClientServerTest(t, mode, HandlerFunc(func(rw ResponseWriter, req *Request) {
+ if h, ok := req.Header["X-Evil"]; ok {
+ t.Errorf("request has X-Evil header: %q", h)
+ }
+ })).ts
+ req, _ := NewRequest("GET", ts.URL, nil)
+ req.Host = "go.dev\r\nX-Evil:evil"
+ resp, _ := ts.Client().Do(req)
+ if resp != nil {
+ resp.Body.Close()
+ }
+}
To view, visit change 507357. To unsubscribe, or for help writing mail filters, visit settings.
Tatiana Bradley (Gerrit)
Jun 30, 2023, 2:06:57 PM6/30/23
to goph...@pubsubhelper.golang.org, Damien Neil, golang-co...@googlegroups.com
Tatiana Bradley has uploaded this change for review.
net/http: validate Host header before sending
Verify that the Host header we send is valid.
Avoids surprising behavior such as a Host of "go.dev\r\nX-Evil:oops"
adding an X-Evil header to HTTP/1 requests.
Add a test, skip the test for HTTP/2. HTTP/2 is not vulnerable to
header injection in the way HTTP/1 is, but x/net/http2 doesn't validate
the header and will go into a retry loop when the server rejects it.
CL 506995 adds the necessary validation to x/net/http2.
For #60374
For #61075
index cead91d..3100037 100644
--- a/src/net/http/request.go
+++ b/src/net/http/request.go
@@ -17,7 +17,6 @@
"io"
"mime"
"mime/multipart"
- "net"
"net/http/httptrace"
"net/http/internal/ascii"
"net/textproto"
@@ -27,6 +26,7 @@
"strings"
"sync"
+ "golang.org/x/net/http/httpguts"
"golang.org/x/net/idna"
)
@@ -571,12 +571,19 @@
// is not given, use the host from the request URL.
//
// Clean the host, in case it arrives with unexpected stuff in it.
- host := cleanHost(r.Host)
+ host := r.Host
if host == "" {
if r.URL == nil {
return errMissingHost
}
- host = cleanHost(r.URL.Host)
+ host = r.URL.Host
+ }
+ host, err = httpguts.PunycodeHostPort(host)
+ if err != nil {
+ return err
+ }
+ if !httpguts.ValidHostHeader(host) {
+ return errors.New("http: invalid Host header")
}
// According to RFC 6874, an HTTP client, proxy, or other
@@ -733,40 +740,6 @@index 0ec8f24..fddc85d 100644
--- a/src/net/http/request_test.go
+++ b/src/net/http/request_test.go
@@ -778,15 +778,8 @@
}
req.Host = "foo.com with spaces"
req.URL.Host = "foo.com with spaces"
- req.Write(logWrites{t, &got})
- want := []string{
- "GET /after HTTP/1.1\r\n",
- "Host: foo.com\r\n",
- "User-Agent: " + DefaultUserAgent + "\r\n",
- "\r\n",
- }
- if !reflect.DeepEqual(got, want) {
- t.Errorf("Writes = %q\n Want = %q", got, want)
+ if err := req.Write(logWrites{t, &got}); err == nil {
+ t.Errorf("Writing request with invalid Host: succeded, want error")
}
}
diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go
index cba35db..8b306eb 100644
--- a/src/net/http/transport_test.go
+++ b/src/net/http/transport_test.go
@@ -6566,3 +6566,22 @@
}
wg.Wait()
}
+
+func TestRequestSanitization(t *testing.T) { run(t, testRequestSanitization) }
+func testRequestSanitization(t *testing.T, mode testMode) {
+ if mode == http2Mode {
+ // Remove this after updating x/net.
+ t.Skip("https://go.dev/issue/60374 test fails when run with HTTP/2")
+ }
+ ts := newClientServerTest(t, mode, HandlerFunc(func(rw ResponseWriter, req *Request) {
+ if h, ok := req.Header["X-Evil"]; ok {
+ t.Errorf("request has X-Evil header: %q", h)
+ }
+ })).ts
+ req, _ := NewRequest("GET", ts.URL, nil)
+ req.Host = "go.dev\r\nX-Evil:evil"
+ resp, _ := ts.Client().Do(req)
+ if resp != nil {
+ resp.Body.Close()
+ }
+}
To view, visit change 507358. To unsubscribe, or for help writing mail filters, visit settings.
Tatiana Bradley (Gerrit)
Jun 30, 2023, 2:13:40 PM6/30/23
to Damien Neil, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Tatiana Bradley uploaded patch set #2 to this change.
[release-branch.go.1.19] net/http: validate Host header before sending
Verify that the Host header we send is valid.
Avoids surprising behavior such as a Host of "go.dev\r\nX-Evil:oops"
adding an X-Evil header to HTTP/1 requests.
Add a test, skip the test for HTTP/2. HTTP/2 is not vulnerable to
header injection in the way HTTP/1 is, but x/net/http2 doesn't validate
the header and will go into a retry loop when the server rejects it.
CL 506995 adds the necessary validation to x/net/http2.
For #60374
Fixes #61075
For CVE-2023-29406
Change-Id: I05cb6866a9bead043101954dfded199258c6dd04
Reviewed-on: https://go-review.googlesource.com/c/go/+/506996
Reviewed-by: Tatiana Bradley <tatiana...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
Run-TryBot: Damien Neil <dn...@google.com>
(cherry picked from commit 499458f7ca04087958987a33c2703c3ef03e27e2)
---
M src/net/http/http_test.go
M src/net/http/request.go
M src/net/http/request_test.go
M src/net/http/transport_test.go
4 files changed, 31 insertions(+), 75 deletions(-)
To view, visit change 507358. To unsubscribe, or for help writing mail filters, visit settings.
Tatiana Bradley (Gerrit)
Jun 30, 2023, 2:14:57 PM6/30/23
to Damien Neil, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Tatiana Bradley uploaded patch set #2 to this change.
[release-branch.go.1.20] net/http: validate Host header before sending
Verify that the Host header we send is valid.
Avoids surprising behavior such as a Host of "go.dev\r\nX-Evil:oops"
adding an X-Evil header to HTTP/1 requests.
Add a test, skip the test for HTTP/2. HTTP/2 is not vulnerable to
header injection in the way HTTP/1 is, but x/net/http2 doesn't validate
the header and will go into a retry loop when the server rejects it.
CL 506995 adds the necessary validation to x/net/http2.
For #60374
Fixes #61076
For CVE-2023-29406
Change-Id: I05cb6866a9bead043101954dfded199258c6dd04
Reviewed-on: https://go-review.googlesource.com/c/go/+/506996
Reviewed-by: Tatiana Bradley <tatiana...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
Run-TryBot: Damien Neil <dn...@google.com>
(cherry picked from commit 499458f7ca04087958987a33c2703c3ef03e27e2)
---
M src/net/http/http_test.go
M src/net/http/request.go
M src/net/http/request_test.go
M src/net/http/transport_test.go
4 files changed, 31 insertions(+), 75 deletions(-)
To view, visit change 507357. To unsubscribe, or for help writing mail filters, visit settings.
Tatiana Bradley (Gerrit)
Jun 30, 2023, 2:15:57 PM6/30/23
to Damien Neil, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Patch set 2:Run-TryBot +1
To view, visit change 507358. To unsubscribe, or for help writing mail filters, visit settings.
Tatiana Bradley (Gerrit)
Jun 30, 2023, 2:16:22 PM6/30/23
to Damien Neil, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Tatiana Bradley (Gerrit)
Jun 30, 2023, 4:06:04 PM6/30/23
to Damien Neil, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Damien Neil, Roland Shoemaker, Tatiana Bradley.
Tatiana Bradley uploaded patch set #3 to this change.
[release-branch.go1.19] net/http: validate Host header before sending
Verify that the Host header we send is valid.
Avoids surprising behavior such as a Host of "go.dev\r\nX-Evil:oops"
adding an X-Evil header to HTTP/1 requests.
Add a test, skip the test for HTTP/2. HTTP/2 is not vulnerable to
header injection in the way HTTP/1 is, but x/net/http2 doesn't validate
the header and will go into a retry loop when the server rejects it.
CL 506995 adds the necessary validation to x/net/http2.
For #60374
Fixes #61075
For CVE-2023-29406
Change-Id: I05cb6866a9bead043101954dfded199258c6dd04
Reviewed-on: https://go-review.googlesource.com/c/go/+/506996
Reviewed-by: Tatiana Bradley <tatiana...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
Run-TryBot: Damien Neil <dn...@google.com>
(cherry picked from commit 499458f7ca04087958987a33c2703c3ef03e27e2)
---
M src/net/http/http_test.go
M src/net/http/request.go
M src/net/http/request_test.go
M src/net/http/transport_test.go
4 files changed, 31 insertions(+), 75 deletions(-)
To view, visit change 507358. To unsubscribe, or for help writing mail filters, visit settings.
Tatiana Bradley (Gerrit)
Jun 30, 2023, 4:06:54 PM6/30/23
to Damien Neil, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Damien Neil, Roland Shoemaker.
Tatiana Bradley uploaded patch set #3 to this change.
[release-branch.go1.20] net/http: validate Host header before sending
Verify that the Host header we send is valid.
Avoids surprising behavior such as a Host of "go.dev\r\nX-Evil:oops"
adding an X-Evil header to HTTP/1 requests.
Add a test, skip the test for HTTP/2. HTTP/2 is not vulnerable to
header injection in the way HTTP/1 is, but x/net/http2 doesn't validate
the header and will go into a retry loop when the server rejects it.
CL 506995 adds the necessary validation to x/net/http2.
For #60374
Fixes #61076
For CVE-2023-29406
Change-Id: I05cb6866a9bead043101954dfded199258c6dd04
Reviewed-on: https://go-review.googlesource.com/c/go/+/506996
Reviewed-by: Tatiana Bradley <tatiana...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
Run-TryBot: Damien Neil <dn...@google.com>
(cherry picked from commit 499458f7ca04087958987a33c2703c3ef03e27e2)
---
M src/net/http/http_test.go
M src/net/http/request.go
M src/net/http/request_test.go
M src/net/http/transport_test.go
4 files changed, 31 insertions(+), 75 deletions(-)
To view, visit change 507357. To unsubscribe, or for help writing mail filters, visit settings.
Tatiana Bradley (Gerrit)
Jul 5, 2023, 11:03:44 AM7/5/23
to Damien Neil, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Roland Shoemaker, Tatiana Bradley.
Tatiana Bradley uploaded patch set #4 to this change.
The following approvals got outdated and were removed: Run-TryBot+1 by Tatiana Bradley, TryBot-Result-1 by Gopher Robot
[release-branch.go1.19] net/http: validate Host header before sending
Verify that the Host header we send is valid.
Avoids surprising behavior such as a Host of "go.dev\r\nX-Evil:oops"
adding an X-Evil header to HTTP/1 requests.
Add a test, skip the test for HTTP/2. HTTP/2 is not vulnerable to
header injection in the way HTTP/1 is, but x/net/http2 doesn't validate
the header and will go into a retry loop when the server rejects it.
CL 506995 adds the necessary validation to x/net/http2.
For #60374
Fixes #61075
For CVE-2023-29406
Change-Id: I05cb6866a9bead043101954dfded199258c6dd04
Reviewed-on: https://go-review.googlesource.com/c/go/+/506996
Reviewed-by: Tatiana Bradley <tatiana...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
Run-TryBot: Damien Neil <dn...@google.com>
(cherry picked from commit 499458f7ca04087958987a33c2703c3ef03e27e2)
---
M src/net/http/http_test.go
M src/net/http/request.go
M src/net/http/request_test.go
M src/net/http/transport_test.go
M src/runtime/runtime2.go
5 files changed, 35 insertions(+), 79 deletions(-)
To view, visit change 507358. To unsubscribe, or for help writing mail filters, visit settings.
Roland Shoemaker (Gerrit)
Jul 5, 2023, 11:43:14 AM7/5/23
to Damien Neil, Tatiana Bradley, goph...@pubsubhelper.golang.org, Gopher Robot, golang-co...@googlegroups.com
Attention is currently required from: Tatiana Bradley.
Patch set 4:Code-Review +2
1 comment:
File src/runtime/runtime2.go:
Patch Set #4, Line 521:
freeMStack = 0 // M done, free stack and reference.These diffs seem unintentional?
To view, visit change 507358. To unsubscribe, or for help writing mail filters, visit settings.
Roland Shoemaker (Gerrit)
Jul 5, 2023, 11:45:32 AM7/5/23
to Damien Neil, Tatiana Bradley, goph...@pubsubhelper.golang.org, Gopher Robot, golang-co...@googlegroups.com
Attention is currently required from: Damien Neil, Tatiana Bradley.
Patch set 3:Code-Review +2
To view, visit change 507357. To unsubscribe, or for help writing mail filters, visit settings.
Tatiana Bradley (Gerrit)
Jul 5, 2023, 12:12:21 PM7/5/23
to Damien Neil, goph...@pubsubhelper.golang.org, Roland Shoemaker, Gopher Robot, golang-co...@googlegroups.com
Patch set 4:Code-Review +2
To view, visit change 507358. To unsubscribe, or for help writing mail filters, visit settings.
Tatiana Bradley (Gerrit)
Jul 5, 2023, 12:32:29 PM7/5/23
to Damien Neil, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Tatiana Bradley.
Tatiana Bradley uploaded patch set #10 to this change.
The following approvals got outdated and were removed: Run-TryBot+1 by Tatiana Bradley, TryBot-Result+1 by Gopher Robot
[release-branch.go1.19] net/http: validate Host header before sending
Verify that the Host header we send is valid.
Avoids surprising behavior such as a Host of "go.dev\r\nX-Evil:oops"
adding an X-Evil header to HTTP/1 requests.
Add a test, skip the test for HTTP/2. HTTP/2 is not vulnerable to
header injection in the way HTTP/1 is, but x/net/http2 doesn't validate
the header and will go into a retry loop when the server rejects it.
CL 506995 adds the necessary validation to x/net/http2.
For #60374
Fixes #61075
For CVE-2023-29406
Change-Id: I05cb6866a9bead043101954dfded199258c6dd04
Reviewed-on: https://go-review.googlesource.com/c/go/+/506996
Reviewed-by: Tatiana Bradley <tatiana...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
Run-TryBot: Damien Neil <dn...@google.com>
(cherry picked from commit 499458f7ca04087958987a33c2703c3ef03e27e2)
---
M src/net/http/http_test.go
M src/net/http/request.go
M src/net/http/request_test.go
M src/net/http/transport_test.go
4 files changed, 31 insertions(+), 75 deletions(-)
To view, visit change 507358. To unsubscribe, or for help writing mail filters, visit settings.
Tatiana Bradley (Gerrit)
Jul 5, 2023, 12:32:45 PM7/5/23
to Damien Neil, goph...@pubsubhelper.golang.org, Roland Shoemaker, Gopher Robot, golang-co...@googlegroups.com
Patch Set #4, Line 521:
freeMStack = 0 // M done, free stack and reference.These diffs seem unintentional?
Done
To view, visit change 507358. To unsubscribe, or for help writing mail filters, visit settings.
Joedian Reid (Gerrit)
Jul 5, 2023, 3:08:43 PM7/5/23
to Damien Neil, Tatiana Bradley, goph...@pubsubhelper.golang.org, Roland Shoemaker, Gopher Robot, golang-co...@googlegroups.com
Attention is currently required from: Tatiana Bradley.
Patch set 10:Run-TryBot +1
Dmitri Shuralyov (Gerrit)
Jul 5, 2023, 3:56:28 PM7/5/23
to Damien Neil, Tatiana Bradley, goph...@pubsubhelper.golang.org, Dmitri Shuralyov, Roland Shoemaker, Gopher Robot, golang-co...@googlegroups.com
Attention is currently required from: Damien Neil, Tatiana Bradley.
The change is no longer submittable: No-Unresolved-Comments is unsatisfied now.
1 comment:
Commit Message:
Patch Set #3, Line 19:
Fixes #61076Is the change in CL 506995 also a part of the fix that needs to be [cherry-picked](https://go.dev/wiki/MinorReleases#cherry-pick-cls-for-vendored-golangorgx-packages) to resolve #61076?
To view, visit change 507357. To unsubscribe, or for help writing mail filters, visit settings.
Tatiana Bradley (Gerrit)
Jul 5, 2023, 4:49:02 PM7/5/23
to Damien Neil, goph...@pubsubhelper.golang.org, Gopher Robot, Joedian Reid, Roland Shoemaker, golang-co...@googlegroups.com
Tatiana Bradley (Gerrit)
Jul 5, 2023, 4:54:47 PM7/5/23
to Damien Neil, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Tatiana Bradley uploaded patch set #11 to this change.
[release-branch.go1.19] net/http: validate Host header before sending
Verify that the Host header we send is valid.
Avoids surprising behavior such as a Host of "go.dev\r\nX-Evil:oops"
adding an X-Evil header to HTTP/1 requests.
Add a test, skip the test for HTTP/2. HTTP/2 is not vulnerable to
header injection in the way HTTP/1 is, but x/net/http2 doesn't validate
the header and will go into a retry loop when the server rejects it.
CL 506995 adds the necessary validation to x/net/http2.
For #60374
For #61075
For CVE-2023-29406
Change-Id: I05cb6866a9bead043101954dfded199258c6dd04
Reviewed-on: https://go-review.googlesource.com/c/go/+/506996
Reviewed-by: Tatiana Bradley <tatiana...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
Run-TryBot: Damien Neil <dn...@google.com>
(cherry picked from commit 499458f7ca04087958987a33c2703c3ef03e27e2)
---
M src/net/http/http_test.go
M src/net/http/request.go
M src/net/http/request_test.go
M src/net/http/transport_test.go
4 files changed, 31 insertions(+), 75 deletions(-)
To view, visit change 507358. To unsubscribe, or for help writing mail filters, visit settings.
Tatiana Bradley (Gerrit)
Jul 5, 2023, 4:54:58 PM7/5/23
to Damien Neil, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Damien Neil, Tatiana Bradley.
Tatiana Bradley uploaded patch set #4 to this change.
[release-branch.go1.20] net/http: validate Host header before sending
Verify that the Host header we send is valid.
Avoids surprising behavior such as a Host of "go.dev\r\nX-Evil:oops"
adding an X-Evil header to HTTP/1 requests.
Add a test, skip the test for HTTP/2. HTTP/2 is not vulnerable to
header injection in the way HTTP/1 is, but x/net/http2 doesn't validate
the header and will go into a retry loop when the server rejects it.
CL 506995 adds the necessary validation to x/net/http2.
For #60374
For #61076
For CVE-2023-29406
Change-Id: I05cb6866a9bead043101954dfded199258c6dd04
Reviewed-on: https://go-review.googlesource.com/c/go/+/506996
Reviewed-by: Tatiana Bradley <tatiana...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
Run-TryBot: Damien Neil <dn...@google.com>
(cherry picked from commit 499458f7ca04087958987a33c2703c3ef03e27e2)
---
M src/net/http/http_test.go
M src/net/http/request.go
M src/net/http/request_test.go
M src/net/http/transport_test.go
4 files changed, 31 insertions(+), 75 deletions(-)
To view, visit change 507357. To unsubscribe, or for help writing mail filters, visit settings.
Tatiana Bradley (Gerrit)
Jul 5, 2023, 5:01:57 PM7/5/23
to Damien Neil, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Damien Neil, Tatiana Bradley.
Tatiana Bradley uploaded patch set #5 to this change.
[release-branch.go1.20] net/http: validate Host header before sending
Verify that the Host header we send is valid.
Avoids surprising behavior such as a Host of "go.dev\r\nX-Evil:oops"
adding an X-Evil header to HTTP/1 requests.
Add a test, skip the test for HTTP/2. HTTP/2 is not vulnerable to
header injection in the way HTTP/1 is, but x/net/http2 doesn't validate
the header and will go into a retry loop when the server rejects it.
CL 506995 adds the necessary validation to x/net/http2.
For #60374
Fixes #61076
For CVE-2023-29406
Change-Id: I05cb6866a9bead043101954dfded199258c6dd04
Reviewed-on: https://go-review.googlesource.com/c/go/+/506996
Reviewed-by: Tatiana Bradley <tatiana...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
Run-TryBot: Damien Neil <dn...@google.com>
(cherry picked from commit 499458f7ca04087958987a33c2703c3ef03e27e2)
---
M src/net/http/http_test.go
M src/net/http/request.go
M src/net/http/request_test.go
M src/net/http/transport_test.go
4 files changed, 31 insertions(+), 75 deletions(-)
To view, visit change 507357. To unsubscribe, or for help writing mail filters, visit settings.
Tatiana Bradley (Gerrit)
Jul 5, 2023, 5:02:07 PM7/5/23
to Damien Neil, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Tatiana Bradley uploaded patch set #12 to this change.
[release-branch.go1.19] net/http: validate Host header before sending
Verify that the Host header we send is valid.
Avoids surprising behavior such as a Host of "go.dev\r\nX-Evil:oops"
adding an X-Evil header to HTTP/1 requests.
Add a test, skip the test for HTTP/2. HTTP/2 is not vulnerable to
header injection in the way HTTP/1 is, but x/net/http2 doesn't validate
the header and will go into a retry loop when the server rejects it.
CL 506995 adds the necessary validation to x/net/http2.
For #60374
Fixes #61075
For CVE-2023-29406
Change-Id: I05cb6866a9bead043101954dfded199258c6dd04
Reviewed-on: https://go-review.googlesource.com/c/go/+/506996
Reviewed-by: Tatiana Bradley <tatiana...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
Run-TryBot: Damien Neil <dn...@google.com>
(cherry picked from commit 499458f7ca04087958987a33c2703c3ef03e27e2)
---
M src/net/http/http_test.go
M src/net/http/request.go
M src/net/http/request_test.go
M src/net/http/transport_test.go
4 files changed, 31 insertions(+), 75 deletions(-)
To view, visit change 507358. To unsubscribe, or for help writing mail filters, visit settings.
Tatiana Bradley (Gerrit)
Jul 5, 2023, 5:03:16 PM7/5/23
to Damien Neil, goph...@pubsubhelper.golang.org, Gopher Robot, Joedian Reid, Roland Shoemaker, golang-co...@googlegroups.com
Patch set 12:Run-TryBot +1Code-Review +2
1 comment:
Patchset:
1 of 47 TryBots failed. […]
Unrelated failure
To view, visit change 507358. To unsubscribe, or for help writing mail filters, visit settings.
Damien Neil (Gerrit)
Jul 5, 2023, 5:03:43 PM7/5/23
to Tatiana Bradley, goph...@pubsubhelper.golang.org, Dmitri Shuralyov, Roland Shoemaker, Gopher Robot, golang-co...@googlegroups.com
Attention is currently required from: Dmitri Shuralyov, Tatiana Bradley.
1 comment:
Commit Message:
Patch Set #3, Line 19:
Fixes #61076
Is the change in CL 506995 also a part of the fix that needs to be [cherry-picked](https://go. […]
That change fixes a related issue with HTTP/2, which is (unlike the issue this fixes) not something we classified as a vulnerability.
We could cherry-pick that one as well to get both fixes for Host header handling, but this is the only one required to resolve #61076.
To view, visit change 507357. To unsubscribe, or for help writing mail filters, visit settings.
Tatiana Bradley (Gerrit)
Jul 5, 2023, 5:08:33 PM7/5/23
to Damien Neil, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Tatiana Bradley uploaded patch set #13 to this change.
[release-branch.go1.19] net/http: validate Host header before sending
Verify that the Host header we send is valid.
Avoids surprising behavior such as a Host of "go.dev\r\nX-Evil:oops"
adding an X-Evil header to HTTP/1 requests.
Add a test, skip the test for HTTP/2. HTTP/2 is not vulnerable to
header injection in the way HTTP/1 is, but x/net/http2 doesn't validate
the header and will go into a retry loop when the server rejects it.
CL 506995 adds the necessary validation to x/net/http2.
Updates #60374
Fixes #61075
For CVE-2023-29406
Change-Id: I05cb6866a9bead043101954dfded199258c6dd04
Reviewed-on: https://go-review.googlesource.com/c/go/+/506996
Reviewed-by: Tatiana Bradley <tatiana...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
Run-TryBot: Damien Neil <dn...@google.com>
(cherry picked from commit 499458f7ca04087958987a33c2703c3ef03e27e2)
---
M src/net/http/http_test.go
M src/net/http/request.go
M src/net/http/request_test.go
M src/net/http/transport_test.go
4 files changed, 31 insertions(+), 75 deletions(-)
To view, visit change 507358. To unsubscribe, or for help writing mail filters, visit settings.
Tatiana Bradley (Gerrit)
Jul 5, 2023, 5:09:17 PM7/5/23
to Damien Neil, goph...@pubsubhelper.golang.org, Gopher Robot, Joedian Reid, Roland Shoemaker, golang-co...@googlegroups.com
Patch set 13:-Run-TryBotCode-Review +2
Tatiana Bradley (Gerrit)
Jul 5, 2023, 5:09:28 PM7/5/23
to Damien Neil, goph...@pubsubhelper.golang.org, Gopher Robot, Joedian Reid, Roland Shoemaker, golang-co...@googlegroups.com
Tatiana Bradley removed a vote from this change.
Removed TryBot-Result-1 by Gopher Robot <go...@golang.org>
To view, visit change 507358. To unsubscribe, or for help writing mail filters, visit settings.
Tatiana Bradley (Gerrit)
Jul 5, 2023, 5:09:34 PM7/5/23
to Damien Neil, goph...@pubsubhelper.golang.org, Gopher Robot, Joedian Reid, Roland Shoemaker, golang-co...@googlegroups.com
Patch set 13:Run-TryBot +1
Tatiana Bradley (Gerrit)
Jul 5, 2023, 5:25:31 PM7/5/23
to Damien Neil, goph...@pubsubhelper.golang.org, Gopher Robot, Joedian Reid, Roland Shoemaker, golang-co...@googlegroups.com
Attention is currently required from: Tatiana Bradley.
Tatiana Bradley removed a vote from this change.
Removed TryBot-Result-1 by Gopher Robot <go...@golang.org>
To view, visit change 507358. To unsubscribe, or for help writing mail filters, visit settings.
Tatiana Bradley (Gerrit)
Jul 5, 2023, 6:27:32 PM7/5/23
to Damien Neil, goph...@pubsubhelper.golang.org, Gopher Robot, Joedian Reid, Roland Shoemaker, golang-co...@googlegroups.com
Tatiana Bradley (Gerrit)
Jul 6, 2023, 10:11:49 AM7/6/23
to Damien Neil, goph...@pubsubhelper.golang.org, Gopher Robot, Joedian Reid, Roland Shoemaker, golang-co...@googlegroups.com
Patch set 13:-Run-TryBot
Tatiana Bradley (Gerrit)
Jul 6, 2023, 10:11:59 AM7/6/23
to Damien Neil, goph...@pubsubhelper.golang.org, Gopher Robot, Joedian Reid, Roland Shoemaker, golang-co...@googlegroups.com
Patch set 13:Run-TryBot +1
Tatiana Bradley (Gerrit)
Jul 6, 2023, 10:15:45 AM7/6/23
to Damien Neil, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Joedian Reid, Tatiana Bradley.
Tatiana Bradley uploaded patch set #14 to this change.
The following approvals got outdated and were removed: Run-TryBot+1 by Joedian Reid, Run-TryBot+1 by Tatiana Bradley
[release-branch.go1.19] net/http: validate Host header before sending
Verify that the Host header we send is valid.
Avoids surprising behavior such as a Host of "go.dev\r\nX-Evil:oops"
adding an X-Evil header to HTTP/1 requests.
Add a test, skip the test for HTTP/2. HTTP/2 is not vulnerable to
header injection in the way HTTP/1 is, but x/net/http2 doesn't validate
the header and will go into a retry loop when the server rejects it.
CL 506995 adds the necessary validation to x/net/http2.
Updates #60374
Fixes #61075
For CVE-2023-29406
Change-Id: I05cb6866a9bead043101954dfded199258c6dd04
Reviewed-on: https://go-review.googlesource.com/c/go/+/506996
Reviewed-by: Tatiana Bradley <tatiana...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
Run-TryBot: Damien Neil <dn...@google.com>
(cherry picked from commit 499458f7ca04087958987a33c2703c3ef03e27e2)
---
M src/net/http/http_test.go
M src/net/http/request.go
M src/net/http/request_test.go
M src/net/http/transport_test.go
4 files changed, 30 insertions(+), 75 deletions(-)
To view, visit change 507358. To unsubscribe, or for help writing mail filters, visit settings.
Tatiana Bradley (Gerrit)
Jul 6, 2023, 10:16:06 AM7/6/23
to Damien Neil, goph...@pubsubhelper.golang.org, Gopher Robot, Joedian Reid, Roland Shoemaker, golang-co...@googlegroups.com
Attention is currently required from: Joedian Reid.
Patch set 14:Run-TryBot +1Code-Review +2
To view, visit change 507358. To unsubscribe, or for help writing mail filters, visit settings.
Dmitri Shuralyov (Gerrit)
Jul 6, 2023, 11:33:52 AM7/6/23
to Damien Neil, Tatiana Bradley, goph...@pubsubhelper.golang.org, Dmitri Shuralyov, Roland Shoemaker, Gopher Robot, golang-co...@googlegroups.com
Attention is currently required from: Damien Neil, Tatiana Bradley.
1 comment:
Commit Message:
Patch Set #3, Line 19:
Fixes #61076
That change fixes a related issue with HTTP/2, which is (unlike the issue this fixes) not something […]
Thanks for confirming. That means this Fixes line is correct and there's nothing more to do here. (If you decide to also include the CL for the related issue with HTTP/2, that can happen independently and just use the same backport issue number even after it's closed.)
To view, visit change 507357. To unsubscribe, or for help writing mail filters, visit settings.
Damien Neil (Gerrit)
Jul 6, 2023, 2:34:55 PM7/6/23
to Tatiana Bradley, goph...@pubsubhelper.golang.org, Dmitri Shuralyov, Roland Shoemaker, Gopher Robot, golang-co...@googlegroups.com
Joedian Reid (Gerrit)
Jul 6, 2023, 3:41:24 PM7/6/23
to Damien Neil, Tatiana Bradley, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Dmitri Shuralyov, Roland Shoemaker, Gopher Robot, golang-co...@googlegroups.com
Joedian Reid submitted this change.
Approvals:
Tatiana Bradley: Run TryBots
Gopher Robot: TryBots succeeded
Damien Neil: Looks good to me, approved
Roland Shoemaker: Looks good to me, approved
[release-branch.go1.20] net/http: validate Host header before sending
Verify that the Host header we send is valid.
Avoids surprising behavior such as a Host of "go.dev\r\nX-Evil:oops"
adding an X-Evil header to HTTP/1 requests.
Add a test, skip the test for HTTP/2. HTTP/2 is not vulnerable to
header injection in the way HTTP/1 is, but x/net/http2 doesn't validate
the header and will go into a retry loop when the server rejects it.
CL 506995 adds the necessary validation to x/net/http2.
For #60374
Fixes #61076
For CVE-2023-29406
Change-Id: I05cb6866a9bead043101954dfded199258c6dd04
Reviewed-on: https://go-review.googlesource.com/c/go/+/506996
Reviewed-by: Tatiana Bradley <tatiana...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
Run-TryBot: Damien Neil <dn...@google.com>
(cherry picked from commit 499458f7ca04087958987a33c2703c3ef03e27e2)
Reviewed-on: https://go-review.googlesource.com/c/go/+/507357
Reviewed-by: Damien Neil <dn...@google.com>
Run-TryBot: Tatiana Bradley <tatiana...@google.com>
Reviewed-by: Roland Shoemaker <rol...@golang.org>
---
M src/net/http/http_test.go
M src/net/http/request.go
M src/net/http/request_test.go
M src/net/http/transport_test.go
4 files changed, 31 insertions(+), 75 deletions(-)
diff --git a/src/net/http/http_test.go b/src/net/http/http_test.go
index 0d92fe5..f03272a 100644
--- a/src/net/http/http_test.go
+++ b/src/net/http/http_test.go
@@ -48,35 +48,6 @@
}
}
-func TestCleanHost(t *testing.T) {
- tests := []struct {
- in, want string
- }{
- {"www.google.com", "www.google.com"},
- {"www.google.com foo", "www.google.com"},
- {"www.google.com/foo", "www.google.com"},
- {" first character is a space", ""},
- {"[1::6]:8080", "[1::6]:8080"},
-
- // Punycode:
- {"гофер.рф/foo", "xn--c1ae0ajs.xn--p1ai"},
- {"bücher.de", "xn--bcher-kva.de"},
- {"bücher.de:8080", "xn--bcher-kva.de:8080"},
- // Verify we convert to lowercase before punycode:
- {"BÜCHER.de", "xn--bcher-kva.de"},
- {"BÜCHER.de:8080", "xn--bcher-kva.de:8080"},
- // Verify we normalize to NFC before punycode:
- {"gophér.nfc", "xn--gophr-esa.nfc"}, // NFC input; no work needed
- {"goph\u0065\u0301r.nfd", "xn--gophr-esa.nfd"}, // NFD input
- }
- for _, tt := range tests {
- got := cleanHost(tt.in)
- if tt.want != got {
- t.Errorf("cleanHost(%q) = %q, want %q", tt.in, got, tt.want)
- }
- }
-}
-
// Test that cmd/go doesn't link in the HTTP server.
//
// This catches accidental dependencies between the HTTP transport and
diff --git a/src/net/http/request.go b/src/net/http/request.go
index a45c9e3..9c888b3 100644
--- a/src/net/http/request.go
+++ b/src/net/http/request.go
@@ -17,7 +17,6 @@
"io"
"mime"
"mime/multipart"
- "net"
"net/http/httptrace"
"net/http/internal/ascii"
"net/textproto"
@@ -27,6 +26,7 @@
"strings"
"sync"
+ "golang.org/x/net/http/httpguts"
"golang.org/x/net/idna"
)
@@ -575,12 +575,19 @@
// is not given, use the host from the request URL.
//
// Clean the host, in case it arrives with unexpected stuff in it.
- host := cleanHost(r.Host)
+ host := r.Host
if host == "" {
if r.URL == nil {
return errMissingHost
}
- host = cleanHost(r.URL.Host)
+ host = r.URL.Host
+ }
+ host, err = httpguts.PunycodeHostPort(host)
+ if err != nil {
+ return err
+ }
+ if !httpguts.ValidHostHeader(host) {
+ return errors.New("http: invalid Host header")
}
// According to RFC 6874, an HTTP client, proxy, or other
@@ -737,40 +744,6 @@
return idna.Lookup.ToASCII(v)
}
-// cleanHost cleans up the host sent in request's Host header.
-//
-// It both strips anything after '/' or ' ', and puts the value
-// into Punycode form, if necessary.
-//
-// Ideally we'd clean the Host header according to the spec:
-//
-// https://tools.ietf.org/html/rfc7230#section-5.4 (Host = uri-host [ ":" port ]")
-// https://tools.ietf.org/html/rfc7230#section-2.7 (uri-host -> rfc3986's host)
-// https://tools.ietf.org/html/rfc3986#section-3.2.2 (definition of host)
-//
-// But practically, what we are trying to avoid is the situation in
-// issue 11206, where a malformed Host header used in the proxy context
-// would create a bad request. So it is enough to just truncate at the
-// first offending character.
-func cleanHost(in string) string {
- if i := strings.IndexAny(in, " /"); i != -1 {
- in = in[:i]
- }
- host, port, err := net.SplitHostPort(in)
- if err != nil { // input was just a host
- a, err := idnaASCII(in)
- if err != nil {
- return in // garbage in, garbage out
- }
- return a
- }
- a, err := idnaASCII(host)
- if err != nil {
- return in // garbage in, garbage out
- }
- return net.JoinHostPort(a, port)
-}
-
// removeZone removes IPv6 zone identifier from host.
// E.g., "[fe80::1%en0]:8080" to "[fe80::1]:8080"
func removeZone(host string) string {
diff --git a/src/net/http/request_test.go b/src/net/http/request_test.go
index 23e49d6..86c68e4 100644
--- a/src/net/http/request_test.go
+++ b/src/net/http/request_test.go
@@ -774,15 +774,8 @@
}
req.Host = "foo.com with spaces"
req.URL.Host = "foo.com with spaces"
- req.Write(logWrites{t, &got})
- want := []string{
- "GET /after HTTP/1.1\r\n",
- "Host: foo.com\r\n",
- "User-Agent: " + DefaultUserAgent + "\r\n",
- "\r\n",
- }
- if !reflect.DeepEqual(got, want) {
- t.Errorf("Writes = %q\n Want = %q", got, want)
+ if err := req.Write(logWrites{t, &got}); err == nil {
+ t.Errorf("Writing request with invalid Host: succeded, want error")
}
}
diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go
index 245f73b..f4896c5 100644
--- a/src/net/http/transport_test.go
+++ b/src/net/http/transport_test.go
@@ -6654,3 +6654,22 @@
}
wg.Wait()
}
+
+func TestRequestSanitization(t *testing.T) { run(t, testRequestSanitization) }
+func testRequestSanitization(t *testing.T, mode testMode) {
+ if mode == http2Mode {
+ // Remove this after updating x/net.
+ t.Skip("https://go.dev/issue/60374 test fails when run with HTTP/2")
+ }
+ ts := newClientServerTest(t, mode, HandlerFunc(func(rw ResponseWriter, req *Request) {
+ if h, ok := req.Header["X-Evil"]; ok {
+ t.Errorf("request has X-Evil header: %q", h)
+ }
+ })).ts
+ req, _ := NewRequest("GET", ts.URL, nil)
+ req.Host = "go.dev\r\nX-Evil:evil"
+ resp, _ := ts.Client().Do(req)
+ if resp != nil {
+ resp.Body.Close()
+ }
+}
To view, visit change 507357. To unsubscribe, or for help writing mail filters, visit settings.
Joedian Reid (Gerrit)
Jul 6, 2023, 3:41:25 PM7/6/23
to Damien Neil, Tatiana Bradley, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Gopher Robot, Roland Shoemaker, golang-co...@googlegroups.com
Joedian Reid submitted this change.
Approvals:
Tatiana Bradley: Looks good to me, approved; Run TryBots
Gopher Robot: TryBots succeeded
Roland Shoemaker: Looks good to me, approved
[release-branch.go1.19] net/http: validate Host header before sending
Verify that the Host header we send is valid.
Avoids surprising behavior such as a Host of "go.dev\r\nX-Evil:oops"
adding an X-Evil header to HTTP/1 requests.
Add a test, skip the test for HTTP/2. HTTP/2 is not vulnerable to
header injection in the way HTTP/1 is, but x/net/http2 doesn't validate
the header and will go into a retry loop when the server rejects it.
CL 506995 adds the necessary validation to x/net/http2.
Updates #60374
Fixes #61075
For CVE-2023-29406
Change-Id: I05cb6866a9bead043101954dfded199258c6dd04
Reviewed-on: https://go-review.googlesource.com/c/go/+/506996
Reviewed-by: Tatiana Bradley <tatiana...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
Run-TryBot: Damien Neil <dn...@google.com>
(cherry picked from commit 499458f7ca04087958987a33c2703c3ef03e27e2)
Reviewed-on: https://go-review.googlesource.com/c/go/+/507358
Run-TryBot: Tatiana Bradley <tatiana...@google.com>
Reviewed-by: Roland Shoemaker <rol...@golang.org>
---
M src/net/http/http_test.go
M src/net/http/request.go
M src/net/http/request_test.go
M src/net/http/transport_test.go
4 files changed, 30 insertions(+), 75 deletions(-)
index cead91d..3100037 100644
--- a/src/net/http/request.go
+++ b/src/net/http/request.go
@@ -17,7 +17,6 @@
"io"
"mime"
"mime/multipart"
- "net"
"net/http/httptrace"
"net/http/internal/ascii"
"net/textproto"
@@ -27,6 +26,7 @@
"strings"
"sync"
+ "golang.org/x/net/http/httpguts"
"golang.org/x/net/idna"
)
@@ -571,12 +571,19 @@
// is not given, use the host from the request URL.
//
// Clean the host, in case it arrives with unexpected stuff in it.
- host := cleanHost(r.Host)
+ host := r.Host
if host == "" {
if r.URL == nil {
return errMissingHost
}
- host = cleanHost(r.URL.Host)
+ host = r.URL.Host
+ }
+ host, err = httpguts.PunycodeHostPort(host)
+ if err != nil {
+ return err
+ }
+ if !httpguts.ValidHostHeader(host) {
+ return errors.New("http: invalid Host header")
}
// According to RFC 6874, an HTTP client, proxy, or other
@@ -733,40 +740,6 @@index 0ec8f24..fddc85d 100644
--- a/src/net/http/request_test.go
+++ b/src/net/http/request_test.go
@@ -778,15 +778,8 @@
}
req.Host = "foo.com with spaces"
req.URL.Host = "foo.com with spaces"
- req.Write(logWrites{t, &got})
- want := []string{
- "GET /after HTTP/1.1\r\n",
- "Host: foo.com\r\n",
- "User-Agent: " + DefaultUserAgent + "\r\n",
- "\r\n",
- }
- if !reflect.DeepEqual(got, want) {
- t.Errorf("Writes = %q\n Want = %q", got, want)
+ if err := req.Write(logWrites{t, &got}); err == nil {
+ t.Errorf("Writing request with invalid Host: succeded, want error")
}
}
diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go
index cba35db..985d062 100644
--- a/src/net/http/transport_test.go
+++ b/src/net/http/transport_test.go
@@ -6566,3 +6566,21 @@
}
wg.Wait()
}
+
+func TestRequestSanitization(t *testing.T) {
+ setParallel(t)
+ defer afterTest(t)
+
+ ts := newClientServerTest(t, h1Mode, HandlerFunc(func(rw ResponseWriter, req *Request) {
+ if h, ok := req.Header["X-Evil"]; ok {
+ t.Errorf("request has X-Evil header: %q", h)
+ }
+ })).ts
+ defer ts.Close()
+ req, _ := NewRequest("GET", ts.URL, nil)
+ req.Host = "go.dev\r\nX-Evil:evil"
+ resp, _ := ts.Client().Do(req)
+ if resp != nil {
+ resp.Body.Close()
+ }
+}
To view, visit change 507358. To unsubscribe, or for help writing mail filters, visit settings.
Reply all
Reply to author
Forward
0 new messages