x/net/http2: call conn.Handshake before accessing ConnectionState

[sebsebmc]

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

$ go version
go1.12.7 windows/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
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\schlopeki\AppData\Local\go-build
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=C:\Users\schlopeki\go
set GOPROXY=
set GORACE=
set GOROOT=C:\Go
set GOTMPDIR=
set GOTOOLDIR=C:\Go\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=C:\Users\schlopeki\Projects\doh\go.mod
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\SCHLOP~1\AppData\Local\Temp\go-build767447546=/tmp/go-build -gno-record-gcc-switches

What did you do?

I attempted to make a simple http2 only client/server but the server rejects all connections as having "INADEQUATE_SECURITY, TLS version too low". When inspecting the TLS version connection claims to have a TLS version of 0 but in Wireshark I see that a TLS 1.2 negotiation has already started.

If I manually perform the TLS handshake the http2.Server.ServeConn no longer hangs up the connection.

Here is the minimal server:

package main

import (
	"crypto/tls"
	"log"
	"net/http"

	"golang.org/x/net/http2"
)

func main() {
	cert, err := tls.LoadX509KeyPair("server.crt", "server.key")
	if err != nil {
		panic("Cannot load cert")
	}
	tlsCfg := &tls.Config{
		Certificates: []tls.Certificate{cert},
		NextProtos:   []string{http2.NextProtoTLS},
	}

	srv := http2.Server{}
	srvOpt := &http2.ServeConnOpts{
		Handler:    http.HandlerFunc(handler),
		BaseConfig: &http.Server{TLSConfig: tlsCfg},
	}
	listener, err := tls.Listen("tcp", "127.0.0.1:8942", tlsCfg)
	if err != nil {
		panic("Cant listen")
	}

	for {
		conn, err := listener.Accept()
		if err != nil {
			panic("connection error")
		}
		// Uncommenting the next 2 lines make the server work
		// tlsConn := conn.(*tls.Conn)
		// tlsConn.Handshake()

		go func() { srv.ServeConn(conn, srvOpt) }()
	}
}

func handler(wrt http.ResponseWriter, req *http.Request) {
	if req.ProtoMajor != 2 {
		wrt.WriteHeader(500)
		flusher, _ := wrt.(http.Flusher)
		flusher.Flush()
		log.Printf("Not HTTP2")
		return
	}
	wrt.WriteHeader(200)
	flusher, _ := wrt.(http.Flusher)
	flusher.Flush()
}

Modern versions of Chrome/Firefox should be able to test this server. Testing server.crt and server.key need to be created and I used a host file to point test.local at the loopback address.

What did you expect to see?

The connection would have already completed the TLS handshake once listen.Accept returned it for use, or http2.Server.ServerConn would be able to see that the TLS version is at least negotiated.

What did you see instead?

When passing the connection directly from Accept to the http2.Server.ServeConn, the TLS state of the connection is not set. This results in ServeConn seeing a TLS version of 0 and therefore it sends GOAWAY and kills the connection.

[julieqiu]

/cc @FiloSottile

[sebsebmc]

The TLS handshake doesn't fail as far as I can tell, it just isn't complete in the tls.Conn I get from Accept.

ServeConn looks at the TLS ConnectionState and checks the HandshakeComplete, which is false and then ServeConn has to drop the connection according to the spec because the TLS version doesn't appear to be TLS 1.2 or higher.

If it would help I can provide a packet capture as well.

[MicOestergaard]

I have the same issue. It seems it is not related to http2, but has something to do with how tls connections are established. Here is a simple server program to demonstrate the issue.

package main

import (
	"crypto/tls"
	"encoding/base64"
	"log"
)

func main() {
	certPEMBlock, _ := base64.StdEncoding.DecodeString("LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUJ6ekNDQVZXZ0F3SUJBZ0lVUlRvOXBnMTI5TG50SFB4Wm4zZ0dNU2swaStJd0NnWUlLb1pJemowRUF3SXcKSWpFZ01CNEdBMVVFQXd3WFEyVnlkR2xtYVdOaGRHVWdRWFYwYUc5eWFYUjVJREF3SUJjTk1Ua3dPREl5TURndwpOREV6V2hnUE1qRXhPVEE0TWpNd09EQTBNVE5hTUF3eENqQUlCZ05WQkFNTUFUQXdkakFRQmdjcWhrak9QUUlCCkJnVXJnUVFBSWdOaUFBU2RQVUZKQytiUyt4RlJzQUFHT3BkS0k4bVdVZ1VRWVZjdjNnRjlwSExZd2QrR3RxS3AKdmJIdXR0eUhQSkZHcGxHdVpUbjMyakdPV3pMMDJhQklFQjdpc3dpSVptc3hmZUxSdkFzeTdHd1B0Wkc1ZTlnQgp3TUF5WW0wTFRadjZ0VzJqWURCZU1Bd0dBMVVkRXdFQi93UUNNQUF3RVFZSllJWklBWWI0UWdFQkJBUURBZ2JBCk1BNEdBMVVkRHdFQi93UUVBd0lENkRBZEJnTlZIU1VFRmpBVUJnZ3JCZ0VGQlFjREFnWUlLd1lCQlFVSEF3RXcKREFZRFZSMFJCQVV3QTRJQk1EQUtCZ2dxaGtqT1BRUURBZ05vQURCbEFqQWM1WlZtL0pJaVRoZTQ1Z29Va3J5NgpmeGh5Z0hjS0NyaFByQTJjZDl6ZWt0SnBwd0VXU3d5ejRRcHJRczNhTHJnQ01RRE5PZHp3b2JlUW45dDk5TmduCityYlBObC9BWmVtdElER3pQU3dFb3dXR0RONitCTms3ZmVRd2Mwbmxnb0VOV0NzPQotLS0tLUVORCBDRVJUSUZJQ0FURS0tLS0tCg==")
	keyPEMBlock, _ := base64.StdEncoding.DecodeString("LS0tLS1CRUdJTiBQUklWQVRFIEtFWS0tLS0tCk1JRzJBZ0VBTUJBR0J5cUdTTTQ5QWdFR0JTdUJCQUFpQklHZU1JR2JBZ0VCQkRDMG1zam11TGVNSlNXZjJ6QnQKNFhpVVVMNWRkODh5d3ZueVVtQ3NCR01kZDJWSnNzdThnK2svMkdFYnc4MnhlbnFoWkFOaUFBU2RQVUZKQytiUworeEZSc0FBR09wZEtJOG1XVWdVUVlWY3YzZ0Y5cEhMWXdkK0d0cUtwdmJIdXR0eUhQSkZHcGxHdVpUbjMyakdPCld6TDAyYUJJRUI3aXN3aUlabXN4ZmVMUnZBc3k3R3dQdFpHNWU5Z0J3TUF5WW0wTFRadjZ0VzA9Ci0tLS0tRU5EIFBSSVZBVEUgS0VZLS0tLS0K")
	cert, _ := tls.X509KeyPair(certPEMBlock, keyPEMBlock)

	tlsConfig := tls.Config{
		Certificates: []tls.Certificate{cert},
	}

	listener, _ := tls.Listen("tcp", ":8443", &tlsConfig)
	for {
		conn, _ := listener.Accept()
		defer conn.Close()
		log.Printf("server: accepted from %s", conn.RemoteAddr())

		tlsConn := *conn.(*tls.Conn)
		//If you uncomment the line below it works
		//tlsConn.Handshake()
		log.Printf("server: handshake complete: %t", tlsConn.ConnectionState().HandshakeComplete)
	}
}

[FiloSottile]

No reads can happen in Accept(), as that would block the accept loop. That means the Conn returned by Accept is not initialized: HandshakeComplete is false, Version is unset.

What looks like a problem here is that ServeConn should know to run a Read or call Handshake before reaching into ConnectionState.