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

net/http: make Transport return a net.Conn Response.Body on successful CONNECT #32273

Open
krkhan opened this issue May 28, 2019 · 6 comments

Comments

@krkhan
Copy link

commented May 28, 2019

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

$ go version
go version
go version go1.12.5 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
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/kamran.khan/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/kamran.khan/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
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-build603500370=/tmp/go-build -gno-record-gcc-switches"

What did you do?

		req := &http.Request{
			Method: "CONNECT",
			URL: &url.URL{
				Scheme: "https",
				Host:   proxyHost,
				Opaque: addr,
			},
		}
		req.Host = proxyHost
		res, err := http.DefaultClient.Do(req)
		if err != nil {
			return errors.Annotate(err, "error connecting to https proxy")
		}
		if res.StatusCode < 200 || res.StatusCode > 299 {
			panic(fmt.Sprintf("error connecting to https proxy, status: %d", res.StatusCode))
		}
		conn, ok := res.Body.(net.Conn)
		if ok != true {
			return errors.New(fmt.Sprintf("res.Body is not a net.Conn, it has type %s", reflect.TypeOf(res.Body)))
		}
		_ = tls.Client(conn, tlsConfig)

What did you expect to see?

As per #17227, res.Body should be a net.Conn after successful CONNECT.

What did you see instead?

Instead it is of the type *http.bodyEOFSignal

@bradfitz can you please take a look?

@bradfitz

This comment has been minimized.

Copy link
Member

commented May 28, 2019

It looks like #17227 was closed prematurely. The code submitted only returns (and documents that it returns) net.Conn on a protocol upgrade (h2c, websockets, etc), but it never did so for CONNECT responses.

We can keep this bug open for finishing #17227, but it's too late for Go 1.13 at this point.

@bradfitz bradfitz changed the title res.Body is of type *http.bodyEOFSignal instead of net.Conn after successful CONNECT tunneling net/http: make Transport return a net.Conn Response.Body on successful CONNECT May 28, 2019
@bradfitz bradfitz added the NeedsFix label May 28, 2019
@bradfitz bradfitz added this to the Go1.14 milestone May 28, 2019
@krkhan

This comment has been minimized.

Copy link
Author

commented May 28, 2019

Thank you @bradfitz . What would be the recommended workaround/way to do a TLS client in golang through an HTTPS (and non-http2) proxy for CONNECT tunneling?

@bradfitz

This comment has been minimized.

Copy link
Member

commented May 28, 2019

The net/http.Transport already supports CONNECT. I don't know what io.Pipe you're talking about. See https://github.com/golang/go/wiki/Questions for forums where more people are available to help. I don't have much time this week.

@krkhan

This comment has been minimized.

Copy link
Author

commented May 28, 2019

@bradfitz sorry for not providing the full context. What I wanted to ask was if there is a way to get net.Conn out of net/http.Transport after a connect request. It doesn't look like that's possible in golang 1.12

// input:
// proxyAddr: string, address of the HTTPS proxy
// backendAddr:              string, address of the backend
// ---
// output:
// client.Conn        *tls.Conn: tls connection for reading/writing radius packets

u, err := url.Parse(proxyAddr)
if err != nil {
    return errors.Annotate(err, "error parsing https proxy url")
}

tr := &http.Transport{
    Proxy:           http.ProxyURL(u),
    TLSNextProto:    make(map[string]func(authority string, c *tls.Conn) http.RoundTripper),
    TLSClientConfig: tlsConfig,
}

req := &http.Request{
    Method: "CONNECT",
    URL: &url.URL{
        Scheme: "https",
        Host:   proxyAddr,
        Opaque: backendAddr,
    },
}
req.Header = make(http.Header)

resp, err := tr.RoundTrip(req)
if err != nil {
    return errors.Annotate(err, "error getting response from https proxy")
}
if resp.StatusCode != 200 {
    return errors.New(fmt.Sprintf("did not get 200 from https proxy, status code: %d", resp.StatusCode))
}

conn, ok := resp.Body.(net.Conn)
if ok != true {
    return errors.New(fmt.Sprintf("resp.Body is not a net.Conn, it has type: %s", reflect.TypeOf(resp.Body)))
}
tlsConn = tls.Client(conn, tlsConfig)

I can pass a io.Pipe to http.NewRequest and then write to the corresponding PipeWriter but that doesn't end up with a net.Conn interface (that can be upgraded to tls.Conn).

@krkhan

This comment has been minimized.

Copy link
Author

commented May 28, 2019

No worries, I'm able to do that by simulating a net.Conn

pr, pw := io.Pipe()
req, err := http.NewRequest("CONNECT", fmt.Sprintf("https://%s", proxyAddr), pr)
if err != nil {
    return errors.Annotate(err, "error creating https proxy request")
}
req.ContentLength = -1
req.URL.Opaque = backendAddr
res, err := http.DefaultClient.Do(req)
if err != nil {
    return errors.Annotate(err, "error connecting to https proxy")
}
if res.StatusCode < 200 || res.StatusCode > 299 {
    panic(fmt.Sprintf("error connecting to https proxy, status: %d", res.StatusCode))
}
pc := PipeConn{
    reader:     res.Body,
    writer:     pw,
}
tlsConn = tls.Client(pc, tlsConfig)
@bradfitz

This comment has been minimized.

Copy link
Member

commented May 28, 2019

@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@odeke-em odeke-em self-assigned this Oct 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.