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: support adding client certificate also for http/2 #18017

Closed
balboah opened this issue Nov 22, 2016 · 3 comments
Closed

net/http: support adding client certificate also for http/2 #18017

balboah opened this issue Nov 22, 2016 · 3 comments
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@balboah
Copy link

balboah commented Nov 22, 2016

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

1.7.3

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

darwin amd64

What did you do?

My first attempt to use a TLS client certificate for authenticating towards a http/2 only server looked something like this:

	cert, err := tls.LoadX509KeyPair(pub, key)
	if err != nil {
		return err
	}
	config := &tls.Config{
		Certificates: []tls.Certificate{cert},
	}
	config.BuildNameToCertificate()

	tr := http.Transport{
		TLSClientConfig: config,
	}
	client := &http.Client{
		Transport: &tr,
	}

	res, err := client.Do(req)

This code results in a HTTP/1.1 transport.

My 2nd attempt was to also define the NextProto:

	config := &tls.Config{
		Certificates: []tls.Certificate{cert},
		NextProtos: []string{"h2"},
	}

What did you expect to see?

I expected the http/2 upgrade to work seamlessly even with my custom client certificate specified.
This is how I've been authenticating with previous HTTP versions, I didn't expect the API to change.

What did you see instead?

Since the targeted server only supports HTTP/2, the returned request error was rather confusing:
malformed HTTP status code "client".

After reading the source of how the transport handling works for h2, I found these lines in net/http/transport.go:

   200		if t.TLSClientConfig != nil || t.Dial != nil || t.DialTLS != nil {
   201			// Be conservative and don't automatically enable
   202			// http2 if they've specified a custom TLS config or
   203			// custom dialers. Let them opt-in themselves via
   204			// http2.ConfigureTransport so we don't surprise them
   205			// by modifying their tls.Config. Issue 14275.
   206			return
   207		}

It refers to http2.ConfigureTransport, which is part of the external import golang.org/x/net/http2.
Importing and using this on my transport indeed fixed my problem.

To my surprise, I could not find any exported functionality in the standard library like this. I feel that it should be there, to make the http/2 client support complete.

@quentinmit quentinmit added Documentation NeedsFix The path to resolution is known, but the work has not been done. labels Nov 22, 2016
@quentinmit quentinmit added this to the Go1.8Maybe milestone Nov 22, 2016
@quentinmit
Copy link
Contributor

We should document this better in the godoc, but otherwise this is WAI. If you explicitly want HTTP/2 you need to use the golang.org/x/net/http2 package.

@bradfitz
Copy link
Contributor

This actually was documented a bunch during the Go 1.8 cycle.

Notably: 0631f29 (document relation and interaction with golang.org/x/net/http2) and ab0ae44.

Let me know if something's still not clear at tip, otherwise I'm going to assume this is adequately documented for now.

@balboah
Copy link
Author

balboah commented Nov 23, 2016

@bradfitz Thanks, that documentation will surely help. I guess when it's mature, the http2 will be included into the standard lib as well.

@golang golang locked and limited conversation to collaborators Nov 23, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants