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

x/net/http/httproxy: empty Host should bypass proxy #52988

Open
tdihp opened this issue May 19, 2022 · 2 comments
Open

x/net/http/httproxy: empty Host should bypass proxy #52988

tdihp opened this issue May 19, 2022 · 2 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@tdihp
Copy link

tdihp commented May 19, 2022

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

$ go version
go version go1.18.2 linux/amd64

Does this issue reproduce with the latest release?

Yes

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/ubuntu/.cache/go-build"
GOENV="/home/ubuntu/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/ubuntu/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/ubuntu/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/ubuntu/.local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/ubuntu/.local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.18.2"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/ubuntu/hello/go.mod"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build736401068=/tmp/go-build -gno-record-gcc-switches"

What did you do?

We are simulating a behavior where CoreDNS health plugin sends http requests via http_proxy to the http://:8080/health which is listened by the health plugin itself. We found that if we specify http://127.0.0.1:8080/, it won't connect to proxy, but only when we specify http://:8080/health it will always connect to proxy.

Sample code below which uses net/http with default transport, that mimics CoreDNS overloaded function in health plugin, configure export http_proxy=http://127.0.0.1:8000 before run:

package main

import (
	"fmt"
	"net/http"
	"time"
)

func main() {
	timeout := time.Duration(3 * time.Second)
	client := http.Client{
		Timeout: timeout,
	}
	// url := "http://127.0.0.1:8080/health" // not using proxy
	url := "http://:8080/health" // using proxy

	resp, err := client.Get(url)
	if err != nil {
		fmt.Println(err)
		return
	}
	fmt.Println("succeed")
	resp.Body.Close()
}

What did you expect to see?

Get "http://:8080/health": dial tcp 127.0.0.1:8080: connect: connection refused should show since it shouldn't reach proxy

What did you see instead?

Get "http://:8080/health": proxyconnect tcp: dial tcp 127.0.0.1:8000: connect: connection refused

Note that useProxy function specifically checks "localhost" and loopback, does it make sense to also use proxy if hostname not provided?

See also: #1589 and #28866, note that in this scenario, it isn't possible to use no_proxy to exempt proxy usage, as no_proxy skips empty strings.

@gopherbot gopherbot added this to the Unreleased milestone May 19, 2022
@tdihp tdihp changed the title x/net/http/httproxy: x/net/http/httproxy: empty Host should bypass proxy May 19, 2022
@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 19, 2022
@mknyszek
Copy link
Contributor

CC @neild

@neild
Copy link
Contributor

neild commented May 20, 2022

I'm surprised we're willing to make a request with an empty host in the URL. That's unfortunate.

Since changing that behavior is risky (we've got a demonstrated case here of someone relying on it), the don't-proxy-localhost check should also look for an empty string. And probably 0.0.0.0 and [::] as well, since net.Dial treats those as loopback too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants