Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Golang client fails to attach to streams with "http: invalid Host header" with go1.20.6, go1.19.11 #45935

Closed
markusthoemmes opened this issue Jul 12, 2023 · 9 comments · Fixed by #45942 or #45962
Labels
kind/bug Bugs are bugs. The cause may or may not be known at triage time so debugging may be needed. status/needs-attention Calls for a collective discussion during a review session status/0-triage

Comments

@markusthoemmes
Copy link

Description

Since updating to Golang 1.20.6, the moby Golang client starts failing with http: invalid Host header errors when used against the default unix socket at least. Normal container operations seem to be fine, but ContainerAttach and ContainerExecAttach fail in particular.

I'm assuming this is related to Golang backporting Host header sanitization into 1.20 via golang/go#61076.

Reproduce

Create a container with the Golang client and try to attach to its streams. I can try to cook up a minimal code example if necessary.

Expected behavior

No response

docker version

github.com/docker/docker v24.0.4+incompatible

docker info

Server:
 Server Version: 23.0.4

Additional Info

No response

@markusthoemmes markusthoemmes added kind/bug Bugs are bugs. The cause may or may not be known at triage time so debugging may be needed. status/0-triage labels Jul 12, 2023
@markusthoemmes
Copy link
Author

Overriding the header manually by adding a client.WithHTTPHeaders(...) option didn't work. Pinning the respective Dockerfile (this component runs in a container) to be built by 1.20.5 fixes the issue reliably.

@thaJeztah
Copy link
Member

Yikes. Hm... yes we need to look into that; I didn't come round to updating the Go version in CI in this repository, so I guess we'll see things fail once we do 😬

I hope we don't have to add back a hack as we did for compatibility with go1.6;

@thaJeztah thaJeztah added the status/needs-attention Calls for a collective discussion during a review session label Jul 12, 2023
@ChristopherHX
Copy link

ChristopherHX commented Jul 12, 2023

I hope we don't have to add back a hack

Is there still a hack for non hijacked? Other api requests are still working in 1.20.6.

If I replace

req.Host = cli.addr

with req.Host = "docker" it works again (I saw docker as Host name for other requests in a debugger)

In my case was cli.addr = /var/run/docker.sock, which is rejected due to /

@thaJeztah
Copy link
Member

Looks like CI is already broken, because the BuildKit integration tests we run don't pin to a minor version of go, so have been silently updated to go1.19.11; #45937 (comment)

@thaJeztah thaJeztah changed the title Golang client fails to attach to streams with "http: invalid Host header" Golang client fails to attach to streams with "http: invalid Host header" with go1.20.6, go1.19.11 Jul 12, 2023
sluongng added a commit to buildbuddy-io/buildbuddy that referenced this issue Jul 12, 2023
In the latest Go version, the net/http client will validate Host header
stricter and will fail if it contains invalid characters.

For more info, see:
- golang/go#60374
- moby/moby#45935

Patch docker temporarily to set Host header to 'localhost' when a UDS
is used.
lbcjbb added a commit to leboncoin/moby that referenced this issue Jul 12, 2023
motoki317 added a commit to traPtitech/NeoShowcase that referenced this issue Jul 13, 2023
@kayvansol

This comment was marked as off-topic.

arnout pushed a commit to buildroot/buildroot that referenced this issue Sep 6, 2023
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>
Tested-by: TIAN Yuanhao <tianyuanhao3@163.com>
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
arnout pushed a commit to buildroot/buildroot that referenced this issue Sep 6, 2023
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>
Tested-by: TIAN Yuanhao <tianyuanhao3@163.com>
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
arnout pushed a commit to buildroot/buildroot that referenced this issue Sep 6, 2023
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>
Tested-by: TIAN Yuanhao <tianyuanhao3@163.com>
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
arnout pushed a commit to buildroot/buildroot that referenced this issue Sep 6, 2023
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>
Tested-by: TIAN Yuanhao <tianyuanhao3@163.com>
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
Grimler91 added a commit to Grimler91/termux-packages that referenced this issue Oct 15, 2023
20.10.24 does not work due to it being compiled with go1.19 or go1.20,
see moby/moby#45935.

Issue has been fixed in 20.10.26.
Grimler91 added a commit to Grimler91/termux-packages that referenced this issue Oct 15, 2023
20.10.24 does not work due to it being compiled with go1.19 or go1.20,
see moby/moby#45935.

Issue has been fixed in 20.10.26.
Grimler91 added a commit to Grimler91/termux-packages that referenced this issue Oct 23, 2023
20.10.24 does not work due to it being compiled with go1.19 or go1.20,
see moby/moby#45935.

Issue has been fixed in 20.10.26.
AbhiTheModder added a commit to AbhiTheModder/termux-packages that referenced this issue Oct 24, 2023
Hermes is a JavaScript engine optimized for React Native apps. It can improve performance, reduce memory usage, and enable faster startup times.

In this pull request, I've added a new package called `hermes` that builds the following tools in addition to Hermes:

- `hdb`: A command line debugger for JavaScript code running on hermes
- `hbcdump`: A tool to disassemble hermes bytecode files
-`hermesc`: A standalone compiler that can convert JavaScript to hermes bytecode, but cannot execute it
- `hvm`: A standalone virtual machine that can execute hermes bytecode, but cannot compile it

These tools can be useful for debugging & testing on termux. You can find more information about them here: https://github.com/facebook/hermes/blob/main/doc/BuildingAndRunning.md

bump(main/asciinema): 2.4.0

This commit has been automatically submitted by Github Actions.

bump(main/curlie): 1.7.2

This commit has been automatically submitted by Github Actions.

bump(main/flyctl): 0.1.112

This commit has been automatically submitted by Github Actions.

bump(main/jfrog-cli): 2.50.4

This commit has been automatically submitted by Github Actions.

bump(main/neomutt): 20231023

This commit has been automatically submitted by Github Actions.

chore(root/docker): set src archive names when downloading

So that we are able to cache them.

chore(root/docker): mv termux_setup_golang to pre_configure

updpkg(root/docker): update to 20.10.26

20.10.24 does not work due to it being compiled with go1.19 or go1.20,
see moby/moby#45935.

Issue has been fixed in 20.10.26.

chore(root/docker): put socket and other files in $PREFIX

While it is nice to not pollute $PREFIX with root owned files I think
we should stick to just using our prefix, other apps might also try to
use /data/docker/, which could result in disaster.

chore(root/docker): replace sed commands with patches

The sed commands makes life a lot easier when updating the
package (less patches to update), but they also make build process a
lot less transparent.  Let's try to stick to patches to make it easier
to understand what modifications we actually do.

fix(root/docker): patch /etc/resolv.conf as well

enhance(root/docker): add service script

So that we can run `sv up dockerd` and `sv down dockerd` to control
the docker daemon.

updpkg(root/docker): update to 24.0.6

chore(root/containerd): load config from and put stuff in $TERMUX_PREFIX

Has been changed in docker package, so needs to be changed in
containerd as well.

chore(root/containerd): clean up patches

Seems newer versions of containerd does not work in termux, trying to
run docker with containerd 1.6.24 or 1.7.7 we get:

$ sudo docker run -it ubuntu bash
docker: Error response from daemon: failed to create task for container: failed to start shim: start failed: io.containerd.runc.v2: create new shim socket: listen unix /data/data/com.termux/files/usr/var/run/containerd/s/3f71828f1d6c1ead43fded842abc9c3cf5857c74c3e0704cd83ab177e17cfe6c: bind: invalid argument: exit status 1: unknown.

bump(main/glab-cli): 1.34.0

This commit has been automatically submitted by Github Actions.

bump(main/parallel): 20231022

This commit has been automatically submitted by Github Actions.

bump(main/teleport-tsh): 14.1.1

This commit has been automatically submitted by Github Actions.

bump(main/wasmtime): 14.0.1

This commit has been automatically submitted by Github Actions.

add readline
AaronFriel added a commit to pulumi/pulumi-docker that referenced this issue Oct 31, 2023
Adding additional logging to `sess.Run()` call here, and fixing moby/moby#45935,
as a prerequisite to debugging #812.

While looking into #812, when using a locally built provider, updates to the
Image resource would not result in a new image built and a corresponding new
`RepoDigest` output value. This was root caused to the Moby issue below after
adding a log statement following `sess.Run()` in `provider/image.go`.
- moby/moby#45935

However, the behavior seen while reproducing is *different* from our issue. That
local repro was too consistent, it occurred in over 20 consecutive builds
following the repro steps at the bottom of this commit.

After updating the Moby dependencies, the image build was no longer skipped and
I was unable to reproduce #812. Neither [Docker v23 release
notes](https://docs.docker.com/engine/release-notes/23.0/) nor the [git diff
from 23.0.1 to 23.0.7](moby/moby@v23.0.1...v23.0.7)
contain any obvious changes that explain that.

The most likely conclusion here is:
1. There is still an unknown bug, likely a race condition, that can trigger
   #812, and I was unsuccessful in reproducing it.
2. The Moby dependency update fixed an issue with the session dialing that has
   the same symptoms as #812, which is fixed in the change to `client/client.go`
   in the Moby diff above.

Repro
=====

The Pulumi program in `examples/multi-stage-build-go` was run repeatedly to
trigger #812 via:

```console
$ uuidgen -r > ./app/example-lockfile.json && pulumi up --skip-preview --diff
```

When the symptoms occurred, verbose logging indicated the image build was
skipped entirely - `docker.ImageBuild` did not run.

provider/go.mod #	modified:   provider/go.sum #	modified:   provider/image.go #
examples/multi-stage-build-go/go.sum #	modified:
examples/multi-stage-build-go/main.go #	modified:
provider/pkg/docs-gen/examples/image.md #
examples/aws-container-registry/ts/pulumi-up.log #
examples/multi-stage-build-yaml/ #	go.work #	go.work.sum #
AaronFriel added a commit to pulumi/pulumi-docker that referenced this issue Oct 31, 2023
Adding additional logging to `sess.Run()` call here, and fixing moby/moby#45935,
as a prerequisite to debugging #812.

While looking into #812, when using a locally built provider, updates to the
Image resource would not result in a new image built and a corresponding new
`RepoDigest` output value. This was root caused to the Moby issue below after
adding a log statement following `sess.Run()` in `provider/image.go`.
- moby/moby#45935

However, the behavior seen while reproducing is *different* from our issue. That
local repro was too consistent, it occurred in over 20 consecutive builds
following the repro steps at the bottom of this commit.

After updating the Moby dependencies, the image build was no longer skipped and
I was unable to reproduce #812. Neither [Docker v23 release
notes](https://docs.docker.com/engine/release-notes/23.0/) nor the [git diff
from 23.0.1 to 23.0.7](moby/moby@v23.0.1...v23.0.7)
contain any obvious changes that explain that.

The most likely conclusion here is:
1. There is still an unknown bug, likely a race condition, that can trigger
   #812, and I was unsuccessful in reproducing it.
2. The Moby dependency update fixed an issue with the session dialing that has
   the same symptoms as #812, which is fixed in the change to `client/client.go`
   in the Moby diff above.

Repro
=====

The Pulumi program in `examples/multi-stage-build-go` was run repeatedly to
trigger #812 via:

```console
$ uuidgen -r > ./app/example-lockfile.json && pulumi up --skip-preview --diff
```

When the symptoms occurred, verbose logging indicated the image build was
skipped entirely - `docker.ImageBuild` did not run.

provider/go.mod #	modified:   provider/go.sum #	modified:   provider/image.go #
examples/multi-stage-build-go/go.sum #	modified:
examples/multi-stage-build-go/main.go #	modified:
provider/pkg/docs-gen/examples/image.md #
examples/aws-container-registry/ts/pulumi-up.log #
examples/multi-stage-build-yaml/ #	go.work #	go.work.sum #
TomaszAIR added a commit to 3mdeb/meta-balena-engine that referenced this issue Nov 2, 2023
Go used in kirkstone uses fix for CVE-2023-29406 which breaks
docker/balena engine.

see:
 - moby/moby#46614
 - moby/moby#45935
 - golang/go#61076

Signed-off-by: Tomasz Żyjewski <tomasz.zyjewski@3mdeb.com>
TomJo2000 pushed a commit to TomJo2000/termux-packages-prs that referenced this issue Nov 12, 2023
20.10.24 does not work due to it being compiled with go1.19 or go1.20,
see moby/moby#45935.

Issue has been fixed in 20.10.26.
AaronFriel added a commit to pulumi/pulumi-docker that referenced this issue Nov 13, 2023
Adding additional logging to `sess.Run()` call here, and fixing
moby/moby#45935, as a prerequisite to debugging #812.

While looking into #812, when using a locally built provider, updates to
the Image resource would not result in a new image built and a
corresponding new `RepoDigest` output value. This was root caused to the
Moby issue below after adding a log statement following `sess.Run()` in
`provider/image.go`.
- moby/moby#45935

However, the behavior seen while reproducing is *different* from our
issue. That local repro was too consistent, it occurred in over 20
consecutive builds following the repro steps at the bottom of this
commit.

After updating the Moby dependencies, the image build was no longer
skipped and I was unable to reproduce #812. Neither [Docker v23 release
notes](https://docs.docker.com/engine/release-notes/23.0/) nor the [git
diff from 23.0.1 to
23.0.7](moby/moby@v23.0.1...v23.0.7) contain
any obvious changes that explain that.

The most likely conclusion here is:
1. There is still an unknown bug, likely a race condition, that can
trigger issue 812, and I was unsuccessful in reproducing it.
2. The Moby dependency update fixed an issue with the session dialing
that has the same symptoms as issue 812, which is fixed in the change to
`client/client.go` in the Moby diff above.

Repro
=====

The Pulumi program in `examples/multi-stage-build-go` was run repeatedly
to trigger #812 via:

```console
$ uuidgen -r > ./app/example-lockfile.json && pulumi up --skip-preview --diff
```

When the symptoms occurred, verbose logging indicated the image build
was skipped entirely - `docker.ImageBuild` did not run.
aethernet added a commit to balena-os/meta-balena that referenced this issue Feb 9, 2024
as it prevent balena-engine to be built due to moby/moby#45935
this is not threat for our use cases
j1nx pushed a commit to OpenVoiceOS/buildroot that referenced this issue Mar 27, 2024
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>
Tested-by: TIAN Yuanhao <tianyuanhao3@163.com>
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
j1nx pushed a commit to OpenVoiceOS/buildroot that referenced this issue Mar 27, 2024
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>
Tested-by: TIAN Yuanhao <tianyuanhao3@163.com>
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Bugs are bugs. The cause may or may not be known at triage time so debugging may be needed. status/needs-attention Calls for a collective discussion during a review session status/0-triage
Projects
None yet
7 participants