client: define a "dummy" hostname to use for local connections by thaJeztah · Pull Request #45942 · moby/moby

thaJeztah · 2023-07-12T12:32:46Z

For local communications (npipe://, unix://), the hostname is not used, but we need valid and meaningful hostname.

The current code used the client's addr as hostname in some cases, which could contain the path for the unix-socket (/var/run/docker.sock), which gets rejected by go1.20.6 and go1.19.11 because of a security fix for CVE-2023-29406 , which was implemented in https://go.dev/issue/60374.

Prior versions go Go would clean the host header, and strip slashes in the process, but go1.20.6 and go1.19.11 no longer do, and reject the host header.

This patch introduces a DummyHost const, and uses this dummy host for cases where we don't need an actual hostname.

Before this patch (using go1.20.6):

make GO_VERSION=1.20.6 TEST_FILTER=TestAttach test-integration
=== RUN   TestAttachWithTTY
    attach_test.go:46: assertion failed: error is not nil: http: invalid Host header
--- FAIL: TestAttachWithTTY (0.11s)
=== RUN   TestAttachWithoutTTy
    attach_test.go:46: assertion failed: error is not nil: http: invalid Host header
--- FAIL: TestAttachWithoutTTy (0.02s)
FAIL

With this patch applied:

make GO_VERSION=1.20.6 TEST_FILTER=TestAttach test-integration
INFO: Testing against a local daemon
=== RUN   TestAttachWithTTY
--- PASS: TestAttachWithTTY (0.12s)
=== RUN   TestAttachWithoutTTy
--- PASS: TestAttachWithoutTTy (0.02s)
PASS

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

thaJeztah · 2023-07-12T12:35:54Z

I also opened #45943 to verify the combination of this patch with go1.20.6.

thaJeztah · 2023-07-12T12:46:39Z

Ah, looks like I overlooked a test;

=== FAIL: client TestSetHostHeader (0.00s)
    request_test.go:79: assertion failed: error is not nil: error during connect: Get "//%2Fvar%2Frun%2Fdocker.sock/test": Test Case #0: Expected host "docker", got "api.moby.localhost"

thaJeztah · 2023-07-12T12:56:45Z

Looks like we need more code updated;

[2023-07-12T12:54:46.838Z] === Failed
[2023-07-12T12:54:46.838Z] === FAIL: pkg/authorization TestAuthZRequestPluginError (15.01s)
[2023-07-12T12:54:46.838Z] time="2023-07-12T12:53:30Z" level=warning msg="Unable to connect to plugin: //tmp/authz4264769183/authz-test-plugin.sock/AuthZPlugin.AuthZReq: Post \"http://%2F%2Ftmp%2Fauthz4264769183%2Fauthz-test-plugin.sock/AuthZPlugin.AuthZReq\": http: invalid Host header, retrying in 1s"
[2023-07-12T12:54:46.838Z] time="2023-07-12T12:53:31Z" level=warning msg="Unable to connect to plugin: //tmp/authz4264769183/authz-test-plugin.sock/AuthZPlugin.AuthZReq: Post \"http://%2F%2Ftmp%2Fauthz4264769183%2Fauthz-test-plugin.sock/AuthZPlugin.AuthZReq\": http: invalid Host header, retrying in 2s"
[2023-07-12T12:54:46.838Z] time="2023-07-12T12:53:33Z" level=warning msg="Unable to connect to plugin: //tmp/authz4264769183/authz-test-plugin.sock/AuthZPlugin.AuthZReq: Post \"http://%2F%2Ftmp%2Fauthz4264769183%2Fauthz-test-plugin.sock/AuthZPlugin.AuthZReq\": http: invalid Host header, retrying in 4s"
[2023-07-12T12:54:46.838Z] time="2023-07-12T12:53:37Z" level=warning msg="Unable to connect to plugin: //tmp/authz4264769183/authz-test-plugin.sock/AuthZPlugin.AuthZReq: Post \"http://%2F%2Ftmp%2Fauthz4264769183%2Fauthz-test-plugin.sock/AuthZPlugin.AuthZReq\": http: invalid Host header, retrying in 8s"
[2023-07-12T12:54:46.838Z]     authz_unix_test.go:50: Failed to authorize request Post "http://%2F%2Ftmp%2Fauthz4264769183%2Fauthz-test-plugin.sock/AuthZPlugin.AuthZReq": http: invalid Host header
[2023-07-12T12:54:46.838Z] 
[2023-07-12T12:54:46.838Z] === FAIL: pkg/authorization TestAuthZRequestPlugin (15.01s)
[2023-07-12T12:54:46.838Z] time="2023-07-12T12:53:45Z" level=warning msg="Unable to connect to plugin: //tmp/authz2422457390/authz-test-plugin.sock/AuthZPlugin.AuthZReq: Post \"http://%2F%2Ftmp%2Fauthz2422457390%2Fauthz-test-plugin.sock/AuthZPlugin.AuthZReq\": http: invalid Host header, retrying in 1s"
[2023-07-12T12:54:46.838Z] time="2023-07-12T12:53:46Z" level=warning msg="Unable to connect to plugin: //tmp/authz2422457390/authz-test-plugin.sock/AuthZPlugin.AuthZReq: Post \"http://%2F%2Ftmp%2Fauthz2422457390%2Fauthz-test-plugin.sock/AuthZPlugin.AuthZReq\": http: invalid Host header, retrying in 2s"
[2023-07-12T12:54:46.838Z] time="2023-07-12T12:53:48Z" level=warning msg="Unable to connect to plugin: //tmp/authz2422457390/authz-test-plugin.sock/AuthZPlugin.AuthZReq: Post \"http://%2F%2Ftmp%2Fauthz2422457390%2Fauthz-test-plugin.sock/AuthZPlugin.AuthZReq\": http: invalid Host header, retrying in 4s"
[2023-07-12T12:54:46.838Z] time="2023-07-12T12:53:52Z" level=warning msg="Unable to connect to plugin: //tmp/authz2422457390/authz-test-plugin.sock/AuthZPlugin.AuthZReq: Post \"http://%2F%2Ftmp%2Fauthz2422457390%2Fauthz-test-plugin.sock/AuthZPlugin.AuthZReq\": http: invalid Host header, retrying in 8s"
[2023-07-12T12:54:46.838Z]     authz_unix_test.go:82: Failed to authorize request Post "http://%2F%2Ftmp%2Fauthz2422457390%2Fauthz-test-plugin.sock/AuthZPlugin.AuthZReq": http: invalid Host header
[2023-07-12T12:54:46.838Z] 
[2023-07-12T12:54:46.838Z] === FAIL: pkg/authorization TestAuthZResponsePlugin (15.01s)
[2023-07-12T12:54:46.838Z] time="2023-07-12T12:54:00Z" level=warning msg="Unable to connect to plugin: //tmp/authz3990016376/authz-test-plugin.sock/AuthZPlugin.AuthZRes: Post \"http://%2F%2Ftmp%2Fauthz3990016376%2Fauthz-test-plugin.sock/AuthZPlugin.AuthZRes\": http: invalid Host header, retrying in 1s"
[2023-07-12T12:54:46.838Z] time="2023-07-12T12:54:01Z" level=warning msg="Unable to connect to plugin: //tmp/authz3990016376/authz-test-plugin.sock/AuthZPlugin.AuthZRes: Post \"http://%2F%2Ftmp%2Fauthz3990016376%2Fauthz-test-plugin.sock/AuthZPlugin.AuthZRes\": http: invalid Host header, retrying in 2s"
[2023-07-12T12:54:46.838Z] time="2023-07-12T12:54:03Z" level=warning msg="Unable to connect to plugin: //tmp/authz3990016376/authz-test-plugin.sock/AuthZPlugin.AuthZRes: Post \"http://%2F%2Ftmp%2Fauthz3990016376%2Fauthz-test-plugin.sock/AuthZPlugin.AuthZRes\": http: invalid Host header, retrying in 4s"
[2023-07-12T12:54:46.838Z] time="2023-07-12T12:54:07Z" level=warning msg="Unable to connect to plugin: //tmp/authz3990016376/authz-test-plugin.sock/AuthZPlugin.AuthZRes: Post \"http://%2F%2Ftmp%2Fauthz3990016376%2Fauthz-test-plugin.sock/AuthZPlugin.AuthZRes\": http: invalid Host header, retrying in 8s"
[2023-07-12T12:54:46.838Z]     authz_unix_test.go:112: Failed to authorize request Post "http://%2F%2Ftmp%2Fauthz3990016376%2Fauthz-test-plugin.sock/AuthZPlugin.AuthZRes": http: invalid Host header
[2023-07-12T12:54:46.838Z] 
[2023-07-12T12:54:46.838Z] === FAIL: pkg/authorization TestMiddlewareWrapHandler/Positive_Test_Case_: (15.01s)
[2023-07-12T12:54:46.838Z] time="2023-07-12T12:54:30Z" level=warning msg="Unable to connect to plugin: //tmp/authz1534243050/authz-test-plugin.sock/AuthZPlugin.AuthZReq: Post \"http://%2F%2Ftmp%2Fauthz1534243050%2Fauthz-test-plugin.sock/AuthZPlugin.AuthZReq\": http: invalid Host header, retrying in 1s"
[2023-07-12T12:54:46.838Z] time="2023-07-12T12:54:31Z" level=warning msg="Unable to connect to plugin: //tmp/authz1534243050/authz-test-plugin.sock/AuthZPlugin.AuthZReq: Post \"http://%2F%2Ftmp%2Fauthz1534243050%2Fauthz-test-plugin.sock/AuthZPlugin.AuthZReq\": http: invalid Host header, retrying in 2s"
[2023-07-12T12:54:46.838Z] time="2023-07-12T12:54:33Z" level=warning msg="Unable to connect to plugin: //tmp/authz1534243050/authz-test-plugin.sock/AuthZPlugin.AuthZReq: Post \"http://%2F%2Ftmp%2Fauthz1534243050%2Fauthz-test-plugin.sock/AuthZPlugin.AuthZReq\": http: invalid Host header, retrying in 4s"
[2023-07-12T12:54:46.838Z] time="2023-07-12T12:54:37Z" level=warning msg="Unable to connect to plugin: //tmp/authz1534243050/authz-test-plugin.sock/AuthZPlugin.AuthZReq: Post \"http://%2F%2Ftmp%2Fauthz1534243050%2Fauthz-test-plugin.sock/AuthZPlugin.AuthZReq\": http: invalid Host header, retrying in 8s"
[2023-07-12T12:54:46.838Z] time="2023-07-12T12:54:45Z" level=error msg="AuthZRequest for GET www.example.com/auth returned error: plugin plugin failed with error: Post \"http://%2F%2Ftmp%2Fauthz1534243050%2Fauthz-test-plugin.sock/AuthZPlugin.AuthZReq\": http: invalid Host header"
[2023-07-12T12:54:46.838Z]     middleware_unix_test.go:60: assertion failed: error is not nil: plugin plugin failed with error: Post "http://%2F%2Ftmp%2Fauthz1534243050%2Fauthz-test-plugin.sock/AuthZPlugin.AuthZReq": http: invalid Host header
[2023-07-12T12:54:46.838Z] 
[2023-07-12T12:54:46.838Z] === FAIL: pkg/authorization TestMiddlewareWrapHandler (30.02s)

From #45943

thaJeztah · 2023-07-12T13:22:14Z

pkg/plugins/client.go

-	return transport.NewHTTPTransport(tr, scheme, socket), nil
+	return transport.NewHTTPTransport(tr, scheme, dummyHost), nil


Arf.. this needs more work;

=== FAIL: pkg/plugins TestFailOnce (15.02s) time="2023-07-12T13:13:54Z" level=warning msg="Unable to connect to plugin: plugin.moby.localhost/Test.FailOnce: Post \"http://plugin.moby.localhost/Test.FailOnce\": dial tcp: lookup plugin.moby.localhost on 10.100.0.2:53: no such host, retrying in 1s" time="2023-07-12T13:13:55Z" level=warning msg="Unable to connect to plugin: plugin.moby.localhost/Test.FailOnce: Post \"http://plugin.moby.localhost/Test.FailOnce\": dial tcp: lookup plugin.moby.localhost on 10.100.0.2:53: no such host, retrying in 2s" time="2023-07-12T13:13:57Z" level=warning msg="Unable to connect to plugin: plugin.moby.localhost/Test.FailOnce: Post \"http://plugin.moby.localhost/Test.FailOnce\": dial tcp: lookup plugin.moby.localhost on 10.100.0.2:53: no such host, retrying in 4s" time="2023-07-12T13:14:01Z" level=warning msg="Unable to connect to plugin: plugin.moby.localhost/Test.FailOnce: Post \"http://plugin.moby.localhost/Test.FailOnce\": dial tcp: lookup plugin.moby.localhost on 10.100.0.2:53: no such host, retrying in 8s" client_test.go:62: Post "http://plugin.moby.localhost/Test.FailOnce": dial tcp: lookup plugin.moby.localhost on 10.100.0.2:53: no such host

thaJeztah · 2023-07-12T14:17:27Z

And more to fix;

=== RUN   TestContainerInvalidJSON/exec/foobar/start/invalid_content_type
    container_test.go:43: assertion failed: error is not nil: Post "http://%2Fgo%2Fsrc%2Fgithub.com%2Fdocker%2Fdocker%2Fbundles%2Ftest-integration%2Fdocker.sock/exec/foobar/start": http: invalid Host header
        --- FAIL: TestContainerInvalidJSON/exec/foobar/start/invalid_content_type (0.00s)

thaJeztah · 2023-07-12T15:52:19Z

Some more;

=== FAIL: amd64.integration-cli TestDockerAPISuite/TestExecResizeImmediatelyAfterExecStart (0.34s)
    docker_api_exec_resize_test.go:111: failed to start the exec: client.Do: Post "http://%2Fgo%2Fsrc%2Fgithub.com%2Fdocker%2Fdocker%2Fbundles%2Ftest-integration%2Fdocker.sock/exec/ed61291b66b21a6d870e48d4956c03d6c79cb98e15097e4cadbbffc6f428e0e0/start": http: invalid Host header
    --- FAIL: TestDockerAPISuite/TestExecResizeImmediatelyAfterExecStart (0.34s)

=== FAIL: amd64.integration-cli TestDockerAPISuite/TestPostContainersAttach (0.37s)
    docker_api_attach_test.go:141: assertion failed: error is not nil: client.Do: Post "http://%2Fgo%2Fsrc%2Fgithub.com%2Fdocker%2Fdocker%2Fbundles%2Ftest-integration%2Fdocker.sock/containers/88656cbe3c6df1756854a082bee57b69b60044a4d1ebd877b02d7670df066947/attach?stream=1&stdin=1&stdout=1": http: invalid Host header
    --- FAIL: TestDockerAPISuite/TestPostContainersAttach (0.37s)

corhere

There is a more standards-compliant choice of Host header value for local communications: the empty string.

corhere · 2023-07-12T17:08:15Z

client/client.go

+// For local communications (npipe://, unix://), the hostname is not used,
+// but we need valid and meaningful hostname.


If the authority component is missing or undefined for the target URI, then a client MUST send a Host header field with an empty field-value.

RFC 7230, Section 5.4

The authority component is undefined for the unix scheme, therefore the empty string is the valid and meaningful host name to use.

client/request.go

corhere

Getting Go's HTTP client infrastructure to cooperate with sending an empty Host request header is going to be a big uphill battle which likely would require substituting a non-trivial amount of the machinery with custom implementations. Let's just stick with the *.localhost dummy strings for now. localhost is a reserved TLD, and UNIX domain sockets are only accessible on the local host, so it is a logical, future-proof and fairly self-documenting choice.

Go 1.20.6 and 1.19.11 include a security check of the http Host header: golang/go#60374 docker-cli does not satisfy this check: $ docker exec -it ctr bash http: invalid Host header This is a backported patch to fix this issue: Issue: moby/moby#45935 Upstream PR: moby/moby#45942 The upstream PR has been merged and will be included in v24.0.5. Signed-off-by: Christian Stewart <christian@aperture.us>

Docker daemon started to fail at handling most local connections when being compiled with Go 1.19.11, which addresses CVE-2023-29406 by blocking invalid host headers of HTTP/1. As a workaround, Docker started to define a dummy host header, and to use it for local connections. Backport the fixes to Flatcar to avoid failures. See also moby/moby#45935, moby/moby#45942.

Go 1.20.6 and 1.19.11 include a security check of the http Host header: golang/go#60374 docker-cli does not satisfy this check: $ docker exec -it ctr bash http: invalid Host header This is a backported patch to fix this issue: Issue: moby/moby#45935 Upstream PR: moby/moby#45942 The upstream PR has been merged and will be included in v24.0.5. Signed-off-by: Christian Stewart <christian@aperture.us>

Docker client and daemon started to fail at sending or handling most local connections when being compiled with Go 1.19.11, which addresses CVE-2023-29406 by blocking invalid host headers of HTTP/1. As a workaround, Docker started to define a dummy host header, and to use it for local connections. Backport the fixes to Flatcar to fix the runtime failures. See also moby/moby#45935, moby/moby#45942.

Go 1.20.6 and 1.19.11 include a security check of the http Host header: golang/go#60374 docker-cli does not satisfy this check: $ docker exec -it ctr bash http: invalid Host header This is a backported patch to fix this issue: Issue: moby/moby#45935 Upstream PR: moby/moby#45942 The upstream PR has been merged and will be included in v24.0.5. Signed-off-by: Christian Stewart <christian@aperture.us>

As a security fix, Go implemented stricter checks for the Host header in the http package. This breaks the Go Docker client library [1] resulting in "http: invalid Host header" errors in our integration tests. A fix [2] was merged to the 24.0 branch, but is not yet included in a release. Therefore, this commit updates the dependency to a specific commit (`go get -u github.com/docker/docker@24.0`). Once it's included in a release, this can be switched back to using a release version. [1] moby/moby#45935 [2] moby/moby#45942

see testcontainers/testcontainers-go#1359 and moby/moby#45942

* NR-100933 chore: upgrade to Go 1.20 * ci: set high severity threshold for snyk (#40) * fix: pin to 1.20.5 for test image see testcontainers/testcontainers-go#1359 and moby/moby#45942 --------- Co-authored-by: Roger Coll <rogercoll@protonmail.com>

thaJeztah added status/2-code-review impact/changelog process/cherry-pick kind/bugfix PR's that fix bugs process/cherry-pick/20.10 process/cherry-pick/23.0 labels Jul 12, 2023

thaJeztah added this to the 25.0.0 milestone Jul 12, 2023

This was referenced Jul 12, 2023

[do not merge] update to go1.20.6 and add client host fix #45943

Closed

update go to go1.20.6 #45941

Merged

Slijkhuis mentioned this pull request Jul 12, 2023

[Bug]: go 1.20.6: "http: invalid Host header, host port waiting failed" testcontainers/testcontainers-go#1359

Closed

thaJeztah force-pushed the fix_host_header branch from 9ae0fb8 to 859ef78 Compare July 12, 2023 12:48

thaJeztah commented Jul 12, 2023

View reviewed changes

thaJeztah force-pushed the fix_host_header branch from a8b2770 to 9d24ef4 Compare July 12, 2023 13:45

thaJeztah force-pushed the fix_host_header branch from 9d24ef4 to 61a41ba Compare July 12, 2023 15:37

thaJeztah force-pushed the fix_host_header branch from 61a41ba to 08aaec1 Compare July 12, 2023 16:06

corhere requested changes Jul 12, 2023

View reviewed changes

crazy-max mentioned this pull request Jul 12, 2023

ci(buildkit): match moby go version for buildkit tests #45947

Merged

thaJeztah force-pushed the fix_host_header branch from 08aaec1 to d565fe6 Compare July 12, 2023 20:43

thaJeztah marked this pull request as draft July 12, 2023 20:43

corhere reviewed Jul 12, 2023

View reviewed changes

client/request.go Show resolved Hide resolved

corhere reviewed Jul 12, 2023

View reviewed changes

thaJeztah force-pushed the fix_host_header branch 3 times, most recently from 2df6195 to c025300 Compare July 13, 2023 11:53

dongsupark mentioned this pull request Jul 18, 2023

Upgrade Go from 1.19.10 and 1.20.5 to 1.19.11 and 1.20.6 flatcar/scripts#988

Merged

thaJeztah mentioned this pull request Jul 18, 2023

vendor: github.com/docker/docker 8e51b8b59cb8 (master, v25.0.0-dev) moby/buildkit#4034

Merged

jacobwgillespie mentioned this pull request Jul 19, 2023

Pin Go to 1.20.5 depot/cli#151

Merged

thaJeztah mentioned this pull request Jul 20, 2023

[0.12 backport] vendor: github.com/docker/docker 8e51b8b59cb8 (master, v25.0.0-dev) moby/buildkit#4043

Merged

DmitriyLewen mentioned this pull request Jul 21, 2023

bug(test): http: invalid Host header aquasecurity/trivy#4848

Closed

julianbrost mentioned this pull request Jul 24, 2023

Make integration tests work with Go 1.20.6 Icinga/icingadb#619

Merged

DavSanchez added a commit to newrelic/nri-statsd that referenced this pull request Jul 25, 2023

fix: pin to 1.20.5 for test image

0b7588c

see testcontainers/testcontainers-go#1359 and moby/moby#45942

vidbina mentioned this pull request Jul 29, 2023

gitlab-runner docker executor broken since golang 1.20.6 NixOS/nixpkgs#245365

Closed

brandond mentioned this pull request Aug 1, 2023

GO 1.20.6 breaks cri-dockerd k3s-io/k3s#8089

Closed

mfrw mentioned this pull request Aug 4, 2023

moby-engine: introduce patch to fix http: invalid Host header microsoft/azurelinux#5936

Closed

12 tasks

mfrw mentioned this pull request Aug 18, 2023

golang: bump golang 1.19.12 -> 1.20.7 microsoft/azurelinux#6001

Merged

12 tasks

fumblehool mentioned this pull request Aug 21, 2023

Update docker to v24 digitalocean/doctl#1406

Merged

phroggyy mentioned this pull request Aug 22, 2023

Pack 0.30.0 fails to fetch builder with Docker engine 24.0.5 on remote host over SSH buildpacks/pack#1870

Closed

thaJeztah mentioned this pull request Dec 18, 2023

Replace use of httputil in client hijack #46920

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

client: define a "dummy" hostname to use for local connections#45942

client: define a "dummy" hostname to use for local connections#45942
thaJeztah merged 4 commits intomoby:masterfrom
thaJeztah:fix_host_header

thaJeztah commented Jul 12, 2023

Uh oh!

thaJeztah commented Jul 12, 2023

Uh oh!

thaJeztah commented Jul 12, 2023

Uh oh!

thaJeztah commented Jul 12, 2023

Uh oh!

thaJeztah Jul 12, 2023

Uh oh!

thaJeztah commented Jul 12, 2023

Uh oh!

thaJeztah commented Jul 12, 2023

Uh oh!

corhere left a comment

Uh oh!

corhere Jul 12, 2023

Uh oh!

Uh oh!

Uh oh!

corhere left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		return transport.NewHTTPTransport(tr, scheme, socket), nil
		return transport.NewHTTPTransport(tr, scheme, dummyHost), nil

		// For local communications (npipe://, unix://), the hostname is not used,
		// but we need valid and meaningful hostname.

Conversation

thaJeztah commented Jul 12, 2023

Uh oh!

thaJeztah commented Jul 12, 2023

Uh oh!

thaJeztah commented Jul 12, 2023

Uh oh!

thaJeztah commented Jul 12, 2023

Uh oh!

thaJeztah Jul 12, 2023

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Jul 12, 2023

Uh oh!

thaJeztah commented Jul 12, 2023

Uh oh!

corhere left a comment

Choose a reason for hiding this comment

Uh oh!

corhere Jul 12, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

corhere left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants