Allow passing ALPN next protocols down to connect services. Fixes #4466. (#9920)

* Allow passing ALPN next protocols down to connect services. Fixes #4466.

* Update connect/proxy/proxy_test.go

Co-authored-by: Paul Banks <banks@banksco.de>

Co-authored-by: Paul Banks <banks@banksco.de>

.changelog/9920.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+connect: The builtin connect proxy no longer advertises support for h2 via ALPN. [[GH-4466](https://github.com/hashicorp/consul/issues/4466)].
+```

connect/proxy/config.go

@@ -46,7 +46,7 @@ type Config struct {
 
 // Service returns the *connect.Service structure represented by this config.
 func (c *Config) Service(client *api.Client, logger hclog.Logger) (*connect.Service, error) {
-	return connect.NewServiceWithLogger(c.ProxiedServiceName, client, logger)
+	return connect.NewServiceWithConfig(c.ProxiedServiceName, connect.Config{Client: client, Logger: logger, ServerNextProtos: []string{}})
 }
 
 // PublicListenerConfig contains the parameters needed for the incoming mTLS

connect/proxy/proxy_test.go

@@ -2,6 +2,7 @@ package proxy
 
 import (
 	"context"
+	"crypto/tls"
 	"net"
 	"testing"
 
@@ -60,12 +61,16 @@ func TestProxy_public(t *testing.T) {
 	defer p.Close()
 	go p.Serve()
 
+	// We create this client with an explicit ServerNextProtos here which will use `h2`
+	// if the proxy supports it. This is so we can verify below that the proxy _doesn't_
+	// advertise `h2` support as it's only a L4 proxy.
+	svc, err := connect.NewServiceWithConfig("echo", connect.Config{Client: client, ServerNextProtos: []string{"h2"}})
+	require.NoError(err)
+
 	// Create a test connection to the proxy. We retry here a few times
 	// since this is dependent on the agent actually starting up and setting
 	// up the CA.
 	var conn net.Conn
-	svc, err := connect.NewService("echo", client)
-	require.NoError(err)
 	retry.Run(t, func(r *retry.R) {
 		conn, err = svc.Dial(context.Background(), &connect.StaticResolver{
 			Addr:    TestLocalAddr(ports[0]),
@@ -76,6 +81,10 @@ func TestProxy_public(t *testing.T) {
 		}
 	})
 
+	// Verify that we did not select h2 via ALPN since the proxy is layer 4 only
+	tlsConn := conn.(*tls.Conn)
+	require.Equal("", tlsConn.ConnectionState().NegotiatedProtocol)
+
 	// Connection works, test it is the right one
 	TestEchoConn(t, conn, "")
 }

connect/service.go

@@ -53,29 +53,34 @@ type Service struct {
 	logger hclog.Logger
 }
 
-// NewService creates and starts a Service. The caller must close the returned
+// Config represents the configuration options for a service.
-// service to free resources and allow the program to exit normally. This is
+type Config struct {
-// typically called in a signal handler.
+	// client is the mandatory Consul API client. Will panic if not set.
-//
+	Client *api.Client
-// Caller must provide client which is already configured to speak to the local
+	// Logger is the logger to use. If nil a default logger will be used.
-// Consul agent, and with an ACL token that has `service:write` privileges for
+	Logger hclog.Logger
-// the service specified.
+	// ServerNextProtos are the protocols advertised via ALPN. If nil, defaults to
-func NewService(serviceName string, client *api.Client) (*Service, error) {
+	// ["h2"] for backwards compatibility. Usually there is no need to change this,
-	logger := hclog.New(&hclog.LoggerOptions{})
+	// see https://github.com/hashicorp/consul/issues/4466 for some discussion on why
-
+	// this can be useful.
-	return NewServiceWithLogger(serviceName, client,
+	ServerNextProtos []string
-		logger)
 }
 
-// NewServiceWithLogger starts the service with a specified log.Logger.
+// NewServiceWithConfig starts a service with the specified Config.
-func NewServiceWithLogger(serviceName string, client *api.Client,
+func NewServiceWithConfig(serviceName string, config Config) (*Service, error) {
-	logger hclog.Logger) (*Service, error) {
+	if config.Logger == nil {
+		config.Logger = hclog.New(&hclog.LoggerOptions{})
+	}
+	tlsCfg := defaultTLSConfig()
+	if config.ServerNextProtos != nil {
+		tlsCfg.NextProtos = config.ServerNextProtos
+	}
 	s := &Service{
 		service:              serviceName,
-		client:               client,
+		client:               config.Client,
-		logger:               logger.Named(logging.Connect).With("service", serviceName),
+		logger:               config.Logger.Named(logging.Connect).With("service", serviceName),
-		tlsCfg:               newDynamicTLSConfig(defaultTLSConfig(), logger),
+		tlsCfg:               newDynamicTLSConfig(tlsCfg, config.Logger),
-		httpResolverFromAddr: ConsulResolverFromAddrFunc(client),
+		httpResolverFromAddr: ConsulResolverFromAddrFunc(config.Client),
 	}
 
 	// Set up root and leaf watches
@@ -98,12 +103,29 @@ func NewServiceWithLogger(serviceName string, client *api.Client,
 	s.leafWatch = p
 	s.leafWatch.HybridHandler = s.leafWatchHandler
 
-	go s.rootsWatch.RunWithClientAndHclog(client, s.logger)
+	go s.rootsWatch.RunWithClientAndHclog(config.Client, s.logger)
-	go s.leafWatch.RunWithClientAndHclog(client, s.logger)
+	go s.leafWatch.RunWithClientAndHclog(config.Client, s.logger)
 
 	return s, nil
 }
 
+// NewService creates and starts a Service. The caller must close the returned
+// service to free resources and allow the program to exit normally. This is
+// typically called in a signal handler.
+//
+// Caller must provide client which is already configured to speak to the local
+// Consul agent, and with an ACL token that has `service:write` privileges for
+// the service specified.
+func NewService(serviceName string, client *api.Client) (*Service, error) {
+	return NewServiceWithConfig(serviceName, Config{Client: client})
+}
+
+// NewServiceWithLogger starts the service with a specified log.Logger.
+func NewServiceWithLogger(serviceName string, client *api.Client,
+	logger hclog.Logger) (*Service, error) {
+	return NewServiceWithConfig(serviceName, Config{Client: client, Logger: logger})
+}
+
 // NewDevServiceFromCertFiles creates a Service using certificate and key files
 // passed instead of fetching them from the client.
 func NewDevServiceFromCertFiles(serviceID string, logger hclog.Logger,