From 0e657973bec47020bdaf5e0eddfaf3c5ac71b348 Mon Sep 17 00:00:00 2001 From: hc-github-team-consul-core Date: Fri, 14 Jul 2023 15:58:58 -0500 Subject: [PATCH] Backport of [NET-4897] net/http host header is now verified and request.host that contains socked now error into release/1.13.x (#18142) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Backport This PR is auto-generated from #18129 to be assessed for backporting due to the inclusion of the label backport/1.13. :rotating_light: >**Warning** automatic cherry-pick of commits failed. If the first commit failed, you will see a blank no-op commit below. If at least one commit succeeded, you will see the cherry-picked commits up to, _not including_, the commit where the merge conflict occurred. The person who merged in the original PR is: @jmurret This person should manually cherry-pick the original PR into a new backport PR, and close this one when the manual backport PR is merged in. > merge conflict error: POST https://api.github.com/repos/hashicorp/consul/merges: 409 Merge conflict [] The below text is copied from the body of the original PR. --- ### Description This is related to https://github.com/hashicorp/consul/pull/18124 where we pinned the go versions in CI to 1.20.5 and 1.19.10. go 1.20.6 and 1.19.11 now validate request host headers for validity, including the hostname cannot be prefixed with slashes. For local communications (npipe://, unix://), the hostname is not used, but we need valid and meaningful hostname. 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. Around the community we are seeing that others are intercepting the req.host and if it starts with a slash or ends with .sock, they changing the host to localhost or another dummy value. [client: define a "dummy" hostname to use for local connections by thaJeztah · Pull Request #45942 · moby/moby](https://github.com/moby/moby/pull/45942) ### Testing & Reproduction steps Check CI tests. ### Links * [ ] updated test coverage * [ ] external facing docs updated * [ ] appropriate backport labels added * [ ] not a security concern ---
Overview of commits - 747195f7aaf291305681bb7d8ae070761a2aef55 - 516492420bf43427f1cf89adce4d4e222bbb5aaa - f4d6ca19f8e543048e167b9c47528eeb0bdb656f - a47407115e086bb5eff6b34a08839989534b505f - 8c03b36e00719b65a87d277012dea2ac08b67442 - c50b17c46ec64dfea20f61d242e1998c804eb8f7 - cc8eaf8213471674843e4a8f6f2f05b98786af4c - ce10138d074ee4fc92418a5d1ae9f595cc7420da - 133c7ecbf5fdd6e6423a40d758800528cf2147c4 - b0bd440d8f7e3912a8af3d9aff6c6cde8bcdd3b7 - 8f223080c027847cd4b3363fa32a66780e5cca5c - f8578b0749fe3df10906b53f91416e9f2421bc29 - 4452224d6ac786d1d1a723490d08aa7038088f31 - 19634a4b3be9a6e466acfb93df769c17eecafe43
--------- Co-authored-by: temp Co-authored-by: John Murret --- .github/workflows/go-tests.yml | 10 +++------- api/api.go | 11 +++++++++++ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/.github/workflows/go-tests.yml b/.github/workflows/go-tests.yml index 557d609bf33d..232d6a181447 100644 --- a/.github/workflows/go-tests.yml +++ b/.github/workflows/go-tests.yml @@ -290,7 +290,7 @@ jobs: runs-on: ${{ needs.setup.outputs.compute-xl }} repository-name: ${{ github.repository }} go-tags: "${{ github.event.repository.name == 'consul-enterprise' && 'consulent consulprem consuldev' || '' }}" - go-version: "1.19.10" + go-version: "1.19" permissions: id-token: write # NOTE: this permission is explicitly required for Vault auth. contents: read @@ -309,12 +309,7 @@ jobs: runs-on: ${{ needs.setup.outputs.compute-xl }} repository-name: ${{ github.repository }} go-tags: "${{ github.event.repository.name == 'consul-enterprise' && 'consulent consulprem consuldev' || '' }}" - # pinning this to 1.20.5 because this issue in go-testcontainers occurs - # in 1.20.6 with the error "http: invalid Host header, host port waiting failed" - # https://github.com/testcontainers/testcontainers-go/issues/1359 - # remove setting this when the above issue is fixed so that the reusable - # job will just get the go version from go.mod. - go-version: "1.20.5" + go-version: "1.20" permissions: id-token: write # NOTE: this permission is explicitly required for Vault auth. contents: read @@ -352,6 +347,7 @@ jobs: runs-on: ${{ needs.setup.outputs.compute-xl }} repository-name: ${{ github.repository }} go-tags: "${{ github.event.repository.name == 'consul-enterprise' && 'consulent consulprem consuldev' || '' }}" + go-version: "1.20" permissions: id-token: write # NOTE: this permission is explicitly required for Vault auth. contents: read diff --git a/api/api.go b/api/api.go index 84b850eb30c9..2fce4db9fbbf 100644 --- a/api/api.go +++ b/api/api.go @@ -954,6 +954,17 @@ func (r *request) toHTTP() (*http.Request, error) { return nil, err } + // validate that socket communications that do not use the host, detect + // slashes in the host name and replace it with local host. + // this is required since go started validating req.host in 1.20.6 and 1.19.11. + // prior to that they would strip out the slashes for you. They removed that + // behavior and added more strict validation as part of a CVE. + // https://github.com/golang/go/issues/60374 + // the hope is that + if strings.HasPrefix(r.url.Host, "/") { + r.url.Host = "localhost" + } + req.URL.Host = r.url.Host req.URL.Scheme = r.url.Scheme req.Host = r.url.Host