From 631fbc049666353d74a21a8af80913add7aa42e4 Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Wed, 19 Jul 2023 10:30:46 -0700 Subject: [PATCH 1/4] [release-branch.go1.20] net/http: permit requests with invalid Host headers Historically, the Transport has silently truncated invalid Host headers at the first '/' or ' ' character. CL 506996 changed this behavior to reject invalid Host headers entirely. Unfortunately, Docker appears to rely on the previous behavior. When sending a HTTP/1 request with an invalid Host, send an empty Host header. This is safer than truncation: If you care about the Host, then you should get the one you set; if you don't care, then an empty Host should be fine. Continue to fully validate Host headers sent to a proxy, since proxies generally can't productively forward requests without a Host. For #60374 Fixes #61431 Fixes #61826 Change-Id: If170c7dd860aa20eb58fe32990fc93af832742b6 Reviewed-on: https://go-review.googlesource.com/c/go/+/511155 TryBot-Result: Gopher Robot Reviewed-by: Roland Shoemaker Run-TryBot: Damien Neil (cherry picked from commit b9153f6ef338baee5fe02a867c8fbc83a8b29dd1) Reviewed-on: https://go-review.googlesource.com/c/go/+/518756 Auto-Submit: Dmitri Shuralyov Reviewed-by: Russ Cox Run-TryBot: Roland Shoemaker --- request.go | 23 ++++++++++++++++++++++- request_test.go | 17 ++++++++++++----- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/request.go b/request.go index 9c888b37..2cf96b44 100644 --- a/request.go +++ b/request.go @@ -586,8 +586,29 @@ func (r *Request) write(w io.Writer, usingProxy bool, extraHeaders Header, waitF if err != nil { return err } + // Validate that the Host header is a valid header in general, + // but don't validate the host itself. This is sufficient to avoid + // header or request smuggling via the Host field. + // The server can (and will, if it's a net/http server) reject + // the request if it doesn't consider the host valid. if !httpguts.ValidHostHeader(host) { - return errors.New("http: invalid Host header") + // Historically, we would truncate the Host header after '/' or ' '. + // Some users have relied on this truncation to convert a network + // address such as Unix domain socket path into a valid, ignored + // Host header (see https://go.dev/issue/61431). + // + // We don't preserve the truncation, because sending an altered + // header field opens a smuggling vector. Instead, zero out the + // Host header entirely if it isn't valid. (An empty Host is valid; + // see RFC 9112 Section 3.2.) + // + // Return an error if we're sending to a proxy, since the proxy + // probably can't do anything useful with an empty Host header. + if !usingProxy { + host = "" + } else { + return errors.New("http: invalid Host header") + } } // According to RFC 6874, an HTTP client, proxy, or other diff --git a/request_test.go b/request_test.go index 86c68e47..a9343cd9 100644 --- a/request_test.go +++ b/request_test.go @@ -766,16 +766,23 @@ func TestRequestWriteBufferedWriter(t *testing.T) { } } -func TestRequestBadHost(t *testing.T) { +func TestRequestBadHostHeader(t *testing.T) { got := []string{} req, err := NewRequest("GET", "http://foo/after", nil) if err != nil { t.Fatal(err) } - req.Host = "foo.com with spaces" - req.URL.Host = "foo.com with spaces" - if err := req.Write(logWrites{t, &got}); err == nil { - t.Errorf("Writing request with invalid Host: succeded, want error") + req.Host = "foo.com\nnewline" + req.URL.Host = "foo.com\nnewline" + req.Write(logWrites{t, &got}) + want := []string{ + "GET /after HTTP/1.1\r\n", + "Host: \r\n", + "User-Agent: " + DefaultUserAgent + "\r\n", + "\r\n", + } + if !reflect.DeepEqual(got, want) { + t.Errorf("Writes = %q\n Want = %q", got, want) } } From 926efc6dec962f9eca1178a306f6e584c2b3ac76 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Wed, 4 Oct 2023 13:49:11 +0200 Subject: [PATCH 2/4] chore: prepare syncing with go1.20.8 See https://github.com/ooni/probe/issues/2524 --- UPSTREAM | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/UPSTREAM b/UPSTREAM index ab9045a5..2cea1db3 100644 --- a/UPSTREAM +++ b/UPSTREAM @@ -1 +1 @@ -go1.20.6 +go1.20.8 From 14f08bd57a2aabf4b6d5d0baee79227d665b2f54 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Wed, 4 Oct 2023 14:02:21 +0200 Subject: [PATCH 3/4] chore: encourage to make the check-list explicit --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index f7bd850b..8211fa28 100644 --- a/README.md +++ b/README.md @@ -248,7 +248,7 @@ the `Request` and `Response` fields; - [ ] commit the changes and push `merged-main` to gitub; -- [ ] open a PR and merge it *using a merge commit*; +- [ ] open a PR using this check-list as part of the PR text and merge it *using a merge commit*; - [ ] create a new working branch to update the examples; From 56b9280cbc6dd11a77ae600c2638662d1940437e Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Wed, 4 Oct 2023 14:08:48 +0200 Subject: [PATCH 4/4] chore: go get -u -v ./... && go mod tidy --- go.mod | 4 ++-- go.sum | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index 1cdff18f..0ca58cb4 100644 --- a/go.mod +++ b/go.mod @@ -2,6 +2,6 @@ module github.com/ooni/oohttp go 1.20 -require golang.org/x/net v0.12.0 +require golang.org/x/net v0.15.0 -require golang.org/x/text v0.11.0 // indirect +require golang.org/x/text v0.13.0 // indirect diff --git a/go.sum b/go.sum index 8569e63b..e1c506e6 100644 --- a/go.sum +++ b/go.sum @@ -1,4 +1,4 @@ -golang.org/x/net v0.12.0 h1:cfawfvKITfUsFCeJIHJrbSxpeu/E81khclypR0GVT50= -golang.org/x/net v0.12.0/go.mod h1:zEVYFnQC7m/vmpQFELhcD1EWkZlX69l4oqgmer6hfKA= -golang.org/x/text v0.11.0 h1:LAntKIrcmeSKERyiOh0XMV39LXS8IE9UL2yP7+f5ij4= -golang.org/x/text v0.11.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE= +golang.org/x/net v0.15.0 h1:ugBLEUaxABaB5AJqW9enI0ACdci2RUd4eP51NTBvuJ8= +golang.org/x/net v0.15.0/go.mod h1:idbUs1IY1+zTqbi8yxTbhexhEEk5ur9LInksu6HrEpk= +golang.org/x/text v0.13.0 h1:ablQoSUd0tRdKxZewP80B+BaqeKJuVhuRxj/dkrun3k= +golang.org/x/text v0.13.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE=