Dial broken by TLS configuration shared w/ net/http client

[Foosec]

Describe the bug

A clear and concise description of what the bug is.

http2: server: error reading preface from client 192.168.3.1:48056: bogus greeting "GET /uploadSocket HTTP/1"

The error of the client is either
2020/06/10 21:04:03 dial:malformed HTTP response "\x00\x00\x18\x04\x00\x00\x00\x00\x00\x00\x05\x00\x10\x00\x00\x00\x03\x00\x00\x00\xfa\x00\x06\x00\x10\x01@\x00\x04\x00\x10\x00\x00"

or EOF

My Go http server throws that error when connecting with gorilla websocket client,
this exact code used to work.

Clientside
I add custom root CAs and set them to the default http transport and gorillas default dialer.

       tlscfg := tls.Config{InsecureSkipVerify: false, RootCAs: rootCAs}

	http.DefaultTransport.(*http.Transport).TLSClientConfig = &tlscfg
	websocket.DefaultDialer.TLSClientConfig = &tlscfg

         u := url.URL{Scheme: "wss", Host: ServerIP, Path:UploadSocket}
         conn, _, err := websocket.DefaultDialer.Dial(u.String(), nil)
	if err != nil {
		log.Fatal("dial:", err)
	}
	defer conn.Close()

This is fixed if i force the server to only use http 1, however that breaks other things.

Versions

Go version:go version go1.14.4 linux/amd64
package version: b65e629

[Foosec]

Ive isolated it to only happen, if i dial after doing other http requests. Taking your example code for server / client and dropping my TLS code in doesn't break it. However if i first make a post request and then try to dial, im met with the same error. Im thinking it could be Go's http reuse of connections not switching to the correct http version?

[Foosec]

More information on the problem, i tried going back in go versions. I went directly to 1.9.7 and it works without issue there, so im guessing it must be something in Gos http thats causing it

[Foosec]

I managed to fix it by setting ForceAttemptHTTP2 on the default http transport to false

[Foosec]

the /uploadSocket part is mine but all insight i can give is that indeed, what i wrote fixed it.

[andreas-kupries]

I am running into the same issue, it seems. See x-reference to my ticket above.

Some more data points:

• Using InsecureSkipVerify in the tls config of the gorilla dialer, instead of custom certs, is ok also.
• My go is 1.14.15, i.e. somewhat beyond the 1.14.4 reported by @Foosec

I don't see how setting ForceAttemptHTTP2 can make a difference because this package does not interact with net/http transports.

@ghost I can.

OP mentioned in the original description

http2: server: error reading preface from client 192.168.3.1:48056: bogus greeting "GET /uploadSocket HTTP/1"

This is a message from the golang http server. I suspect that something in the ws setup with custom certs triggers a bug in the server somewhere, and the various workarounds simply manage to force the server around that place.

Searching a bit I found golang/go#21336 and golang/go#22481

Foosec also said

I'm thinking it could be Go's http reuse of connections not switching to the correct http version

And something like that could explain your confusion regarding

This package does not send the greeting ""GET /uploadSocket HTTP/1"

Because that greeting does not have to come from your package when the server is confused through reused connections.
It may very well come from some completely different request made outside gorilla, with a regular http connection.

I hope I am making sense.

I suspect finding and fixing the exact issue requires talking to the golang net/http maintainers.
And that gorilla is just the canary here.

[garyburd]

Both error reports use a TLS client configuration shared with a net/http Transport (see 1, 2). The net/http Transport sets NextProtos to []string{"h2", "http/1.1"} using a sync.Once field in the transport.

There are two problems:

• The websocket protocol does not use h2.
• There's a data race in environments where where websocket connections are dialed concurrently with HTTP requests.

Fix both of these problems by using a unique TLS client configuration for each protocol:

tlscfg := &tls.Config{InsecureSkipVerify: false, RootCAs: rootCAs}
http.DefaultTransport.(*http.Transport).TLSClientConfig = tlscfg
websocket.DefaultDialer.TLSClientConfig = tlscfg.Clone() // <-- note Clone

Here's a self-contained example that demonstrate the problem. The following test fails:

func TestNextProto(t *testing.T) {
	ts := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		var u Upgrader
		c, err := u.Upgrade(w, r, nil)
		if err == nil {
			c.Close()
		}
	}))
	ts.EnableHTTP2 = true
	ts.StartTLS()
	defer ts.Close()

	d := Dialer{
		TLSClientConfig:  ts.Client().Transport.(*http.Transport).TLSClientConfig,
	}

	r, err := ts.Client().Get(ts.URL)
	if err != nil {
		t.Fatalf("Get: %v", err)
	}
	r.Body.Close()

	c, _, err := d.Dial(makeWsProto(ts.URL), nil)
	if err != nil {
		t.Fatalf("Dial: %v", err)
	}
	defer c.Close()
}

The test succees when the dialer is configured as follows:

d := Dialer{
	TLSClientConfig:  ts.Client().Transport.(*http.Transport).TLSClientConfig.Clone(),
}