-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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
net/http: insufficient sanitization of Host header #60374
Comments
Change https://go.dev/cl/506995 mentions this issue: |
Change https://go.dev/cl/506996 mentions this issue: |
Verify that the Host header we send is valid. Avoids sending a request that the server will reject, possibly sending us into a retry loop. No test in this CL, but this will be covered by the net/http test added in CL 506996. For golang/go#60374 Change-Id: I78867eb05293ad8ca1b02bc22fb626760949d4b8 Reviewed-on: https://go-review.googlesource.com/c/net/+/506995 TryBot-Result: Gopher Robot <[email protected]> Run-TryBot: Damien Neil <[email protected]> Reviewed-by: Tatiana Bradley <[email protected]>
Verify that the Host header we send is valid. Avoids surprising behavior such as a Host of "go.dev\r\nX-Evil:oops" adding an X-Evil header to HTTP/1 requests. Add a test, skip the test for HTTP/2. HTTP/2 is not vulnerable to header injection in the way HTTP/1 is, but x/net/http2 doesn't validate the header and will go into a retry loop when the server rejects it. CL 506995 adds the necessary validation to x/net/http2. For #60374 Change-Id: I05cb6866a9bead043101954dfded199258c6dd04 Reviewed-on: https://go-review.googlesource.com/c/go/+/506996 Reviewed-by: Tatiana Bradley <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Run-TryBot: Damien Neil <[email protected]>
@gopherbot please open backport issues. This is a (fairly minor) security issue. |
Backport issue(s) opened: #61075 (for 1.19), #61076 (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. |
Change https://go.dev/cl/507357 mentions this issue: |
Change https://go.dev/cl/507358 mentions this issue: |
This is CVE-2023-29406. |
@neild CL 506995 and CL 506996 both say "For". Is the only thing left to resolve this issue to backport CL 506995 to internal-branch.go1.21-vendor and regenerate src/net/http/h2_bundle.go, or is there more that needs to be done for Go 1.21? This is being backported to the next 1.20 and 1.19 minor releases, so adding release-blocker here since Go 1.21 shouldn't be released while this issue is still open. (CC @joedian.) |
Change https://go.dev/cl/507905 mentions this issue: |
Change https://go.dev/cl/507906 mentions this issue: |
Verify that the Host header we send is valid. Avoids surprising behavior such as a Host of "go.dev\r\nX-Evil:oops" adding an X-Evil header to HTTP/1 requests. Add a test, skip the test for HTTP/2. HTTP/2 is not vulnerable to header injection in the way HTTP/1 is, but x/net/http2 doesn't validate the header and will go into a retry loop when the server rejects it. CL 506995 adds the necessary validation to x/net/http2. Updates #60374 Fixes #61075 For CVE-2023-29406 Change-Id: I05cb6866a9bead043101954dfded199258c6dd04 Reviewed-on: https://go-review.googlesource.com/c/go/+/506996 Reviewed-by: Tatiana Bradley <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Run-TryBot: Damien Neil <[email protected]> (cherry picked from commit 499458f) Reviewed-on: https://go-review.googlesource.com/c/go/+/507358 Run-TryBot: Tatiana Bradley <[email protected]> Reviewed-by: Roland Shoemaker <[email protected]>
Verify that the Host header we send is valid. Avoids surprising behavior such as a Host of "go.dev\r\nX-Evil:oops" adding an X-Evil header to HTTP/1 requests. Add a test, skip the test for HTTP/2. HTTP/2 is not vulnerable to header injection in the way HTTP/1 is, but x/net/http2 doesn't validate the header and will go into a retry loop when the server rejects it. CL 506995 adds the necessary validation to x/net/http2. For #60374 Fixes #61076 For CVE-2023-29406 Change-Id: I05cb6866a9bead043101954dfded199258c6dd04 Reviewed-on: https://go-review.googlesource.com/c/go/+/506996 Reviewed-by: Tatiana Bradley <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Run-TryBot: Damien Neil <[email protected]> (cherry picked from commit 499458f) Reviewed-on: https://go-review.googlesource.com/c/go/+/507357 Reviewed-by: Damien Neil <[email protected]> Run-TryBot: Tatiana Bradley <[email protected]> Reviewed-by: Roland Shoemaker <[email protected]>
@neild what's the status here? |
Only thing left to resolve is the backports, so I think we can close this one. |
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.
Thank you for this change. This does cause an issue for unix sockets that have hostnames starting with a slash. I think we are seeing the community that uses sockets now have to react to this change and put in conditional logic to check their host and if it begins with a slash or ends with |
@jmurret I will dare to say that the security fix went too hard and broke a feature which many of us relied on and now there's really no right (clean) way to proceed, if the validation gets baked, workarounds will need to remain because there will be always people running some go release which doesn't have the thing baked. I'm sorry I know I'm not really commenting anything useful, I'll just move on with my life 😄 and let you guys think about what you just did. |
Verify that the Host header we send is valid. Avoids surprising behavior such as a Host of "go.dev\r\nX-Evil:oops" adding an X-Evil header to HTTP/1 requests. Add a test, skip the test for HTTP/2. HTTP/2 is not vulnerable to header injection in the way HTTP/1 is, but x/net/http2 doesn't validate the header and will go into a retry loop when the server rejects it. CL 506995 adds the necessary validation to x/net/http2. For golang#60374 Change-Id: I05cb6866a9bead043101954dfded199258c6dd04 Reviewed-on: https://go-review.googlesource.com/c/go/+/506996 Reviewed-by: Tatiana Bradley <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Run-TryBot: Damien Neil <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]> Reviewed-by: Roland Shoemaker <[email protected]> Run-TryBot: Damien Neil <[email protected]>
Change https://go.dev/cl/518855 mentions this issue: |
Change https://go.dev/cl/518756 mentions this issue: |
Change https://go.dev/cl/518856 mentions this issue: |
…eaders 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 #61904 Change-Id: If170c7dd860aa20eb58fe32990fc93af832742b6 Reviewed-on: https://go-review.googlesource.com/c/go/+/511155 TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Roland Shoemaker <[email protected]> Run-TryBot: Damien Neil <[email protected]> (cherry picked from commit b9153f6) Reviewed-on: https://go-review.googlesource.com/c/go/+/518856 Auto-Submit: Dmitri Shuralyov <[email protected]> Run-TryBot: Roland Shoemaker <[email protected]> Reviewed-by: Russ Cox <[email protected]>
…eaders 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 <[email protected]> Reviewed-by: Roland Shoemaker <[email protected]> Run-TryBot: Damien Neil <[email protected]> (cherry picked from commit b9153f6) Reviewed-on: https://go-review.googlesource.com/c/go/+/518756 Auto-Submit: Dmitri Shuralyov <[email protected]> Reviewed-by: Russ Cox <[email protected]> Run-TryBot: Roland Shoemaker <[email protected]>
…eaders 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 #61825 Change-Id: If170c7dd860aa20eb58fe32990fc93af832742b6 Reviewed-on: https://go-review.googlesource.com/c/go/+/511155 TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Roland Shoemaker <[email protected]> Run-TryBot: Damien Neil <[email protected]> (cherry picked from commit b9153f6) Reviewed-on: https://go-review.googlesource.com/c/go/+/518855 Auto-Submit: Dmitri Shuralyov <[email protected]> Run-TryBot: Roland Shoemaker <[email protected]> Reviewed-by: Russ Cox <[email protected]>
…onnections to UNIX domain sockets. As of golang/go#60374 (in the latest Go releases at the time of this writing), we must set it to something that looks like a hostname or requests will fail.
…22523) The Host part of the URL doesn't actually get used when we initiate connections to UNIX domain sockets. As of golang/go#60374 (in the latest Go releases at the time of this writing), we must set it to something that looks like a hostname or requests will fail.
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 <[email protected]> Tested-by: TIAN Yuanhao <[email protected]> Signed-off-by: Peter Korsgaard <[email protected]>
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 <[email protected]> Tested-by: TIAN Yuanhao <[email protected]> Signed-off-by: Peter Korsgaard <[email protected]>
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 <[email protected]> Tested-by: TIAN Yuanhao <[email protected]> Signed-off-by: Peter Korsgaard <[email protected]>
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 <[email protected]> Tested-by: TIAN Yuanhao <[email protected]> Signed-off-by: Peter Korsgaard <[email protected]>
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 <[email protected]> Tested-by: TIAN Yuanhao <[email protected]> Signed-off-by: Peter Korsgaard <[email protected]>
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 <[email protected]> Tested-by: TIAN Yuanhao <[email protected]> Signed-off-by: Peter Korsgaard <[email protected]>
The
net/http
client does not sufficiently sanitize or check the validity of theRequest.Host
field. A maliciously-craftedHost
field can inject request headers or entire new requests into the sent request. For example, settingRequest.Host
to"hostname\r\nX-Header: oops"
adds anX-Header: oops
header to the request.Whether this is a vulnerability or just a bug depends on whether the
Request.Host
field is expected to be secured against untrusted inputs. We don't document this one way or the other. Exploiting this in practice seems difficult, since it requires requests to be sent using an unsanitized and untrustedRequest.Host
value, so if this is a vulnerability it seems reasonable to treat it as PUBLIC track. Either way, we should fix it.This is a continuation of #11206, which reports the same issue but had an incomplete fix.
Thanks to @bartekn for reporting this issue.
The text was updated successfully, but these errors were encountered: