net/http: go 1.20.6 host validation breaks setting Host to a unix socket address

[nicks]

What version of Go are you using (go version)?

$ go version
go version go1.20.6 linux/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

can include this if requested

What did you do?

Create a request where the Host header is a unix socket address. Here's some runnable sample code:

package main

import (
    "context"
    "log"
    "net"
    "net/http"
    "os"
    "time"
)

func handler(w http.ResponseWriter, r *http.Request) {
    w.WriteHeader(204)
}

func main() {
    _ = os.Remove("/tmp/mysocket.sock")
    socket, err := net.Listen("unix", "/tmp/mysocket.sock")
    if err != nil {
        panic(err)
    }
    s := &http.Server{
        Handler:        http.HandlerFunc(handler),
    }

    go func() {
        c := &http.Client{
            Transport: &http.Transport{
                DialContext: func(ctx context.Context, _, _ string) (net.Conn, error) {
                    return net.Dial("unix", "/tmp/mysocket.sock")
                },
            },
        }
        req, _ := http.NewRequest("GET", "http://localhost.com", nil)
        req.Host = "unix:///tmp/mysocket.sock"
        resp, err := c.Do(req)
        if err != nil {
            log.Fatal(err)
        }
        log.Fatal("STATUS ", resp.Status)
    }()
    log.Fatal(s.Serve(socket))
}

What did you expect to see?

In Go 1.20.5, this prints:

2023/07/18 20:19:59 STATUS 204 No Content
exit status 1

What did you see instead?

In Go 1.20.6, this prints:

2023/07/18 20:20:29 Get "http://localhost.com": http: invalid Host header
exit status 1

Additional Info

Related discussion: https://github.com/golang/go/issues/60374

My understanding is that this is an intentional change, to fix a security bug where the Host header contains newline characters. Here's the CVE: https://nvd.nist.gov/vuln/detail/CVE-2023-29406

Unforuntately, this also breaks CLIs in the Go ecosystem that set the Host header to a unix socket, for example : https://github.com/moby/moby/issues/45935

Many projects silently upgrade from Go 1.20.5 -> 1.20.6, so we're starting to see this change break tons of projects in the go ecosystem.

My humble request is that the security fix on the Go 1.20 release branch could be more narrowly targeted at the security issue, and allow this Host header format, to unbreak the ecosystem. The Go 1.21 release line can more safely rollout the backwards-incompatible part of the change.

[ianlancetaylor]

CC @neild @golang/security

[rolandshoemaker]

Not entirely sure how common this behavior is, but I will note that RFC 2616 Section 14.23 does seem to explicitly disallow this:

A client MUST include a Host header field in all HTTP/1.1 request messages . If the requested URI does not include an Internet host name for the service being requested, then the Host header field MUST be given with an empty value.

[seankhliao]

This looks like misusing the Host header for what is arguably proxy/dialer functionality?

Aside: the file transport from net/http.NewFileTransport processes requests with URLs like file:///tmp/my.sock with an empty Host.

[ChristianSch]

I feel that discussing if this is allowed or not is not the most important aspect. As it worked before, it created an API and created expectations (see relevant xkcd). The fallout it created relates to a lot of docker-based technologies which make heavy use of sockets.

Examples I could find/affect me are:

https://github.com/k3d-io/k3d/issues/1321 https://github.com/testcontainers/testcontainers-go/issues/1359

As a lot of people are scrambling to fix the fallout, I think the good outcome will be a better understanding of potential downstream issues of „overcorrections“ and a better understanding of the standards in any case.

[bcmills]

Per #56986, the new validation should probably at least have a GODEBUG setting that allows (part or all of) the old behavior for incremental migration.

[rolandshoemaker]

It sounds like given the scope of those relying on this behavior we'll need to at least partial revert this. I'd like to keep the extremely strict behavior, at least behind a GODEBUG, so that we can work on pushing the ecosystem away from relying on this.

It seems like the main offender here is Docker?

[cpuguy83]

It seems like the main offender here is Docker?

I would say any client connecting to an HTTP server over a unix socket. So to answer the question, possibly given the breadth of use and the fact that it uses http with UDS. But really Docker is just trying to set a meaningful value AND not setting req.Host does not work.

https://go.dev/play/p/JAHc0RFCMRy

[neild]

I don't think we need a GODEBUG; we can reduce the validation of outgoing Host headers to just checking that it's a valid header value, not that it's a valid Host header specifically. That's enough to ensure the outbound request is, at worst, something the server will reject.

[gopherbot]

Change https://go.dev/cl/511155 mentions this issue: net/http: restore truncation of Host headers after / or space

[neild]

Ugh, it's worse than I thought. Our previous behavior was to silently truncate Host headers in HTTP/1 requests at the first ' ' or '/' character.

I suspect Docker is relying on that silent truncation, since otherwise the invalid Host: unix:///tmp/mysocket.sock header is going to be rejected on the server. (The net/http server already validates the header, that hasn't changed recently.)

So I think the thing to do is reduce the validation to just checking that we're sending a valid header value (which is the minimum required) and restore the old truncation behavior. Perhaps we should have a GODEBUG for the truncation, although unless we have a plan for how to change it in the future that's probably just useless complexity.

[gopherbot]

Change https://go.dev/cl/511156 mentions this issue: http2: relax Host header validation

[oleg-nenashev]

Any chance something was backported to go1.19.11 linux/amd64 too? We started seeing a similar behavior there on GitHub actions: https://github.com/wiremock/wiremock-testcontainers-go/issues/18

[bcmills]

@oleg-nenashev, yes, this change was backported to Go 1.19.11 as well. (See #61075.)

[oleg-nenashev]

@bcmills Yes, I confirm the regression: https://github.com/wiremock/wiremock-testcontainers-go/pull/19

P.S: Probably wasn't the greatest time to release the new WireMock module for Testcontainers Go :) Will add to Errata while we we wait for a fix/revert on one of the sides

[bcmills]

Perhaps we should have a GODEBUG for the truncation, although unless we have a plan for how to change it in the future that's probably just useless complexity.

I suggest that:

1. We restore the truncation on the 1.19, 1.20, and 1.21 release branches, and
2. we fix the truncation behavior (and pass the full Host string as long as it is valid to send in a header) for Go 1.22, guarded by a GODEBUG and enabled by default only at Go 1.22 and above.

(The “Go 1.22 and above” part can be implemented by adding an entry to internal/godebugs/table.go with Changed: 22.)

[cpuguy83]

That's my thought as well. It was getting this on a patch release that really caused the pain. We expect some things to break during major version updates.

[seankhliao]

instead of truncating, maybe it should blank out the host header?

[nicks]

does anyone from the Go team have thoughts/opinions on downgrading Homebrew's Go formula to 1.20.5 until the fix goes out? https://github.com/Homebrew/homebrew-core/issues/137413

the problem is that CLIs installed through Homebrew automatically get compiled with Go 1.20.6 and mysteriously break. I don't know what the pros/cons are of the other fixes in 1.20.6.

[cpuguy83]

For docker: docker 24.0.5 was just released and should work with go1.20.6

[srabraham]

For docker: docker 24.0.5 was just released and should work with go1.20.6

Thanks for the update @cpuguy83! The fix in Docker itself does resolve the problem, so it's probably reasonable to call this issue resolved. Go itself is still planning to do a partial revert, but I don't think that's relevant for testcontainers anymore. https://github.com/golang/go/issues/61431

[mholt]

If we are making HTTP requests to a unix socket, with this change, how are we supposed to stay compliant?

RFC 2616, Section 14.26:

A client MUST include a Host header field in all HTTP/1.1 request messages. If the requested URI does not include an Internet host name for the service being requested, then the Host header field MUST be given with an empty value.

Since there is no Internet host name, the Host header "MUST" be given with an empty value.

Previously, " " (space) worked by sheer luck and allowed us to stay compliant.

Now if we set the Host header to a non-empty value, we are not only violating the RFC but we also make CORS validation & DNS rebinding mitigation tricky, since those checks require accurate hostnames, if any. So if we invent one like localhost or foo.local, we risk enabling an CORS breach or DNS rebinding attack.

I don't know that we can use an IP address like 127.0.0.1 or ::1 because we don't know which, if any, IP versions a host supports (hence their use of Unix sockets).

Anyone know if there's something I'm missing/overlooking about this patch so that we can still achieve our objective?

[jub0bs]

@mholt Not quite sure I'm following your comment about CORS, since the Host header plays no part in the CORS protocol.

[mholt]

DNS rebinding mitigation does, which is also important; but all the same, we need a host (even if it's used in the Origin header), if we cannot leave it blank.

[neild]

https://go.dev/cl/511155 changes the transport to send an empty Host header when the host is invalid, except in the case of a request sent to a proxy. Returning an error seems more useful than sending a destination-free request to a proxy. Sending an empty Host offers less potential for request smuggling than truncating at the first invalid character.

Are there any scenarios that I'm missing that this won't cover?

[neild]

@gopherbot please open backport issues. This is a regression.

[gopherbot]

Backport issue(s) opened: #61825 (for 1.19), #61826 (for 1.20).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

[gopherbot]

Change https://go.dev/cl/518855 mentions this issue: [release-branch.go1.19] net/http: permit requests with invalid Host headers

[gopherbot]

Change https://go.dev/cl/518756 mentions this issue: [release-branch.go1.20] net/http: permit requests with invalid Host headers

[gopherbot]

Change https://go.dev/cl/518856 mentions this issue: [release-branch.go1.21] net/http: permit requests with invalid Host headers