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

Change-Id: If170c7dd860aa20eb58fe32990fc93af832742b6
Reviewed-on: https://go-review.googlesource.com/c/go/+/511155
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>

diff --git a/src/net/http/request.go b/src/net/http/request.go
index a2e8373dd5119ac302a5db75056d427fe0641b84..d1fbd5df907c334eccd9010ee7f366daae6a7344 100644 (file)
--- a/src/net/http/request.go
+++ b/src/net/http/request.go
@@ -591,8 +591,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/src/net/http/request_test.go b/src/net/http/request_test.go
index 0892bc255f9164e50baff7748b3aaa7606dacf68..a32b583c116a1517caed4c43c1e61f7a24b26efb 100644 (file)
--- a/src/net/http/request_test.go
+++ b/src/net/http/request_test.go
@@ -767,16 +767,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)
        }
 }