Adds HTTP/2 support to Consul's HTTPS server.

agent/agent.go

@@ -39,6 +39,7 @@ import (
 	"github.com/hashicorp/raft"
 	"github.com/hashicorp/serf/serf"
 	"github.com/shirou/gopsutil/host"
+	"golang.org/x/net/http2"
 )
 
 const (
@@ -340,17 +341,16 @@ func (a *Agent) Start() error {
 		return err
 	}
 
-	// create listeners and unstarted servers
-	// see comment on listenHTTP why we are doing this
-	httpln, err := a.listenHTTP()
+	// Create listeners and unstarted servers; see comment on listenHTTP why
+	// we are doing this.
+	servers, err := a.listenHTTP()
 	if err != nil {
 		return err
 	}
 
-	// start HTTP and HTTPS servers
-	for _, l := range httpln {
-		srv := NewHTTPServer(l.Addr(), a)
-		if err := a.serveHTTP(l, srv); err != nil {
+	// Start HTTP and HTTPS servers.
+	for _, srv := range servers {
+		if err := a.serveHTTP(srv); err != nil {
 			return err
 		}
 		a.httpServers = append(a.httpServers, srv)
@@ -418,12 +418,13 @@ func (a *Agent) listenAndServeDNS() error {
 //
 // This approach should ultimately be refactored to the point where we just
 // start the server and any error should trigger a proper shutdown of the agent.
-func (a *Agent) listenHTTP() ([]net.Listener, error) {
+func (a *Agent) listenHTTP() ([]*HTTPServer, error) {
 	var ln []net.Listener
-
+	var servers []*HTTPServer
 	start := func(proto string, addrs []net.Addr) error {
 		for _, addr := range addrs {
 			var l net.Listener
+			var tlscfg *tls.Config
 			var err error
 
 			switch x := addr.(type) {
@@ -438,11 +439,10 @@ func (a *Agent) listenHTTP() ([]net.Listener, error) {
 				if err != nil {
 					return err
 				}
-
 				l = &tcpKeepAliveListener{l.(*net.TCPListener)}
 
 				if proto == "https" {
-					tlscfg, err := a.config.IncomingHTTPSConfig()
+					tlscfg, err = a.config.IncomingHTTPSConfig()
 					if err != nil {
 						return err
 					}
@@ -453,6 +453,29 @@ func (a *Agent) listenHTTP() ([]net.Listener, error) {
 				return fmt.Errorf("unsupported address type %T", addr)
 			}
 			ln = append(ln, l)
+
+			srv := &HTTPServer{
+				Server: &http.Server{
+					Addr:      l.Addr().String(),
+					TLSConfig: tlscfg,
+				},
+				ln:        l,
+				agent:     a,
+				blacklist: NewBlacklist(a.config.HTTPBlockEndpoints),
+				proto:     proto,
+			}
+			srv.Server.Handler = srv.handler(a.config.EnableDebug)
+
+			// This will enable upgrading connections to HTTP/2 as
+			// part of TLS negotiation.
+			if proto == "https" {
+				err = http2.ConfigureServer(srv.Server, nil)
+				if err != nil {
+					return err
+				}
+			}
+
+			servers = append(servers, srv)
 		}
 		return nil
 	}
@@ -469,12 +492,11 @@ func (a *Agent) listenHTTP() ([]net.Listener, error) {
 		}
 		return nil, err
 	}
-	return ln, nil
+	return servers, nil
 }
 
 // tcpKeepAliveListener sets TCP keep-alive timeouts on accepted
-// connections. It's used by NewHttpServer so dead TCP connections
-// eventually go away.
+// connections. It's used so dead TCP connections eventually go away.
 type tcpKeepAliveListener struct {
 	*net.TCPListener
 }
@@ -507,28 +529,19 @@ func (a *Agent) listenSocket(path string) (net.Listener, error) {
 	return l, nil
 }
 
-func (a *Agent) serveHTTP(l net.Listener, srv *HTTPServer) error {
+func (a *Agent) serveHTTP(srv *HTTPServer) error {
 	// https://github.com/golang/go/issues/20239
 	//
 	// In go.8.1 there is a race between Serve and Shutdown. If
 	// Shutdown is called before the Serve go routine was scheduled then
 	// the Serve go routine never returns. This deadlocks the agent
 	// shutdown for some tests since it will wait forever.
-	//
-	// Since we need to check for an unexported type (*tls.listener)
-	// we cannot just perform a type check since the compiler won't let
-	// us. We might be able to use reflection but the fmt.Sprintf() hack
-	// works just as well.
-	srv.proto = "http"
-	if strings.Contains("*tls.listener", fmt.Sprintf("%T", l)) {
-		srv.proto = "https"
-	}
 	notif := make(chan net.Addr)
 	a.wgServers.Add(1)
 	go func() {
 		defer a.wgServers.Done()
-		notif <- l.Addr()
-		err := srv.Serve(l)
+		notif <- srv.ln.Addr()
+		err := srv.Serve(srv.ln)
 		if err != nil && err != http.ErrServerClosed {
 			a.logger.Print(err)
 		}
@@ -1210,12 +1223,12 @@ func (a *Agent) ShutdownEndpoints() {
 	a.dnsServers = nil
 
 	for _, srv := range a.httpServers {
-		a.logger.Printf("[INFO] agent: Stopping %s server %s (%s)", strings.ToUpper(srv.proto), srv.addr.String(), srv.addr.Network())
+		a.logger.Printf("[INFO] agent: Stopping %s server %s (%s)", strings.ToUpper(srv.proto), srv.ln.Addr().String(), srv.ln.Addr().Network())
 		ctx, cancel := context.WithTimeout(context.Background(), time.Second)
 		defer cancel()
 		srv.Shutdown(ctx)
 		if ctx.Err() == context.DeadlineExceeded {
-			a.logger.Printf("[WARN] agent: Timeout stopping %s server %s (%s)", strings.ToUpper(srv.proto), srv.addr.String(), srv.addr.Network())
+			a.logger.Printf("[WARN] agent: Timeout stopping %s server %s (%s)", strings.ToUpper(srv.proto), srv.ln.Addr().String(), srv.ln.Addr().Network())
 		}
 	}
 	a.httpServers = nil

agent/http.go

@@ -31,24 +31,12 @@ func (e MethodNotAllowedError) Error() string {
 // HTTPServer provides an HTTP api for an agent.
 type HTTPServer struct {
 	*http.Server
+	ln        net.Listener
 	agent     *Agent
 	blacklist *Blacklist
 
 	// proto is filled by the agent to "http" or "https".
 	proto string
-	addr  net.Addr
-}
-
-func NewHTTPServer(addr net.Addr, a *Agent) *HTTPServer {
-	s := &HTTPServer{
-		Server:    &http.Server{Addr: addr.String()},
-		agent:     a,
-		blacklist: NewBlacklist(a.config.HTTPBlockEndpoints),
-		addr:      addr,
-	}
-
-	s.Server.Handler = s.handler(a.config.EnableDebug)
-	return s
 }
 
 // handler is used to attach our handlers to the mux

agent/http_test.go

@@ -19,9 +19,11 @@ import (
 	"time"
 
 	"github.com/hashicorp/consul/agent/structs"
+	"github.com/hashicorp/consul/api"
 	"github.com/hashicorp/consul/logger"
 	"github.com/hashicorp/consul/testutil"
 	"github.com/hashicorp/go-cleanhttp"
+	"golang.org/x/net/http2"
 )
 
 func TestHTTPServer_UnixSocket(t *testing.T) {
@@ -122,6 +124,86 @@ func TestHTTPServer_UnixSocket_FileExists(t *testing.T) {
 	}
 }
 
+func TestHTTPServer_H2(t *testing.T) {
+	t.Parallel()
+
+	// Fire up an agent with TLS enabled.
+	a := &TestAgent{
+		Name:   t.Name(),
+		UseTLS: true,
+		HCL: `
+			key_file = "../test/client_certs/server.key"
+			cert_file = "../test/client_certs/server.crt"
+			ca_file = "../test/client_certs/rootca.crt"
+		`,
+	}
+	a.Start()
+	defer a.Shutdown()
+
+	// Make an HTTP/2-enabled client, using the API helpers to set
+	// up TLS to be as normal as possible for Consul.
+	tlscfg := &api.TLSConfig{
+		Address:  "consul.test",
+		KeyFile:  "../test/client_certs/client.key",
+		CertFile: "../test/client_certs/client.crt",
+		CAFile:   "../test/client_certs/rootca.crt",
+	}
+	tlsccfg, err := api.SetupTLSConfig(tlscfg)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	transport := api.DefaultConfig().Transport
+	transport.TLSClientConfig = tlsccfg
+	if err := http2.ConfigureTransport(transport); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	hc := &http.Client{
+		Transport: transport,
+	}
+
+	// Hook a handler that echoes back the protocol.
+	handler := func(resp http.ResponseWriter, req *http.Request) {
+		resp.WriteHeader(http.StatusOK)
+		fmt.Fprint(resp, req.Proto)
+	}
+	mux, ok := a.srv.Handler.(*http.ServeMux)
+	if !ok {
+		t.Fatalf("handler is not expected type")
+	}
+	mux.HandleFunc("/echo", handler)
+
+	// Call it and make sure we see HTTP/2.
+	url := fmt.Sprintf("https://%s/echo", a.srv.ln.Addr().String())
+	resp, err := hc.Get(url)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	defer resp.Body.Close()
+	body, err := ioutil.ReadAll(resp.Body)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	if !bytes.Equal(body, []byte("HTTP/2.0")) {
+		t.Fatalf("bad: %v", body)
+	}
+
+	// This doesn't have a closed-loop way to verify HTTP/2 support for
+	// some other endpoint, but configure an API client and make a call
+	// just as a sanity check.
+	cfg := &api.Config{
+		Address:    a.srv.ln.Addr().String(),
+		Scheme:     "https",
+		HttpClient: hc,
+	}
+	client, err := api.NewClient(cfg)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	if _, err := client.Agent().Self(); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+}
+
 func TestSetIndex(t *testing.T) {
 	t.Parallel()
 	resp := httptest.NewRecorder()

agent/testagent.go

@@ -65,6 +65,10 @@ type TestAgent struct {
 	// Key is the optional encryption key for the LAN and WAN keyring.
 	Key string
 
+	// UseTLS, if true, will disable the HTTP port and enable the HTTPS
+	// one.
+	UseTLS bool
+
 	// dns is a reference to the first started DNS endpoint.
 	// It is valid after Start().
 	dns *DNSServer
@@ -117,7 +121,7 @@ func (a *TestAgent) Start() *TestAgent {
 		a.Config = TestConfig(
 			config.Source{Name: a.Name, Format: "hcl", Data: a.HCL},
 			config.Source{Name: a.Name + ".data_dir", Format: "hcl", Data: hclDataDir},
-			randomPortsSource(),
+			randomPortsSource(a.UseTLS),
 		)
 
 		// write the keyring
@@ -284,19 +288,24 @@ func (a *TestAgent) consulConfig() *consul.Config {
 // chance of port conflicts for concurrently executed test binaries.
 // Instead of relying on one set of ports to be sufficient we retry
 // starting the agent with different ports on port conflict.
-func randomPortsSource() config.Source {
-	ports := freeport.Get(5)
+func randomPortsSource(tls bool) config.Source {
+	ports := freeport.Get(6)
+	if tls {
+		ports[1] = -1
+	} else {
+		ports[2] = -1
+	}
 	return config.Source{
 		Name:   "ports",
 		Format: "hcl",
 		Data: `
 			ports = {
 				dns = ` + strconv.Itoa(ports[0]) + `
 				http = ` + strconv.Itoa(ports[1]) + `
-				https = -1
-				serf_lan = ` + strconv.Itoa(ports[2]) + `
-				serf_wan = ` + strconv.Itoa(ports[3]) + `
-				server = ` + strconv.Itoa(ports[4]) + `
+				https = ` + strconv.Itoa(ports[2]) + `
+				serf_lan = ` + strconv.Itoa(ports[3]) + `
+				serf_wan = ` + strconv.Itoa(ports[4]) + `
+				server = ` + strconv.Itoa(ports[5]) + `
 			}
 		`,
 	}

api/api.go

@@ -477,6 +477,14 @@ func NewHttpClient(transport *http.Transport, tlsConf TLSConfig) (*http.Client,
 		Transport: transport,
 	}
 
+	// TODO (slackpad) - Once we get some run time on the HTTP/2 support we
+	// should turn it on by default if TLS is enabled. We would basically
+	// just need to call http2.ConfigureTransport(transport) here. We also
+	// don't want to introduce another external dependency on
+	// golang.org/x/net/http2 at this time. For a complete recipe for how
+	// to enable HTTP/2 support on a transport suitable for the API client
+	// library see agent/http_test.go:TestHTTPServer_H2.
+
 	if transport.TLSClientConfig == nil {
 		tlsClientConfig, err := SetupTLSConfig(&tlsConf)