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/http2: Add DialTLSContext to http2.Transport #50392

Open
francislavoie opened this issue Dec 30, 2021 · 1 comment
Open

x/net/http2: Add DialTLSContext to http2.Transport #50392

francislavoie opened this issue Dec 30, 2021 · 1 comment

Comments

@francislavoie
Copy link

@francislavoie francislavoie commented Dec 30, 2021

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

$ go version
go version go1.17.5 windows/amd64

Does this issue reproduce with the latest release?

Yes -- see https://pkg.go.dev/golang.org/x/net@v0.0.0-20211216030914-fe4d6282115f/http2#Transport which is missing a DialTLSContext field

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

go env Output
$ go env
Not relevant

What did you do?

Related issue caddyserver/caddy#4498

In Caddy, we're using a decently common pattern found in a few blog posts (for example https://www.mailgun.com/blog/http-2-cleartext-h2c-client-example-go/) to hack http2.Transport with a custom DialTLS function to skip actually doing TLS, so that we can make an H2C request to an upstream. We use this in the reverse_proxy module.

For non-H2C traffic, we use DialContext and grab the configured dial info from ctx. We do this because the upstream to dial is chosen dynamically, using load balancing selection policies etc.

We use a special config string <network>/<addr> (for example tcp/127.0.0.1:8080 or unix//path/to/unix.sock) so we can embed the network type in a single string.

A user reported that they want to use H2C with a unix socket. The issue is that the network type isn't retained, because the network/address is parsed on each request, and that would require the context so that we can grab a ctx.Value() with the current request's dial info... so it defaults to tcp instead.

We could hack in a thing to make sure the right network is used if the upstream is not dynamic, but that's... hacky and limits usecases.

The better fix would be to actually have access to the context via a new DialTLSContext field on the http2.Transport.

I expect this would have similar semantics to http.Transport's DialContext where if DialContext is non-nil, then it's used, otherwise Dial is used.

At a glance, this looks like it would a trivial change to func (t *Transport) dialTLS(ctx context.Context) right at the top of that function:

func (t *Transport) dialTLS(ctx context.Context) func(string, string, *tls.Config) (net.Conn, error) {
	if t.DialTLSContext != nil {
		return func(network, addr string, cfg *tls.Config) (net.Conn, error) {
			tlsCn, err := t.DialTLSContext(ctx, network, addr, cfg)
			return tlsCn, err
		}
	}
	...

But that might be a naive fix. I'm not comfortable enough with Go internals to attempt a patch.

@cagedmantis cagedmantis added this to the Backlog milestone Jan 7, 2022
@cagedmantis
Copy link
Contributor

@cagedmantis cagedmantis commented Jan 7, 2022

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants