diff --git a/.changelog/9920.txt b/.changelog/9920.txt new file mode 100644 index 000000000..7777a16ba --- /dev/null +++ b/.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)]. +``` diff --git a/connect/proxy/config.go b/connect/proxy/config.go index 5d0b8f597..b49ab9ff8 100644 --- a/connect/proxy/config.go +++ b/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 diff --git a/connect/proxy/proxy_test.go b/connect/proxy/proxy_test.go index 0c4f0aa2e..fc9243bf9 100644 --- a/connect/proxy/proxy_test.go +++ b/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, "") } diff --git a/connect/service.go b/connect/service.go index cf8531949..bad2d7fe5 100644 --- a/connect/service.go +++ b/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 -// 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) { - logger := hclog.New(&hclog.LoggerOptions{}) - - return NewServiceWithLogger(serviceName, client, - logger) +// Config represents the configuration options for a service. +type Config struct { + // client is the mandatory Consul API client. Will panic if not set. + Client *api.Client + // Logger is the logger to use. If nil a default logger will be used. + Logger hclog.Logger + // ServerNextProtos are the protocols advertised via ALPN. If nil, defaults to + // ["h2"] for backwards compatibility. Usually there is no need to change this, + // see https://github.com/hashicorp/consul/issues/4466 for some discussion on why + // this can be useful. + ServerNextProtos []string } -// NewServiceWithLogger starts the service with a specified log.Logger. -func NewServiceWithLogger(serviceName string, client *api.Client, - logger hclog.Logger) (*Service, error) { +// NewServiceWithConfig starts a service with the specified Config. +func NewServiceWithConfig(serviceName string, config Config) (*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, - logger: logger.Named(logging.Connect).With("service", serviceName), - tlsCfg: newDynamicTLSConfig(defaultTLSConfig(), logger), - httpResolverFromAddr: ConsulResolverFromAddrFunc(client), + client: config.Client, + logger: config.Logger.Named(logging.Connect).With("service", serviceName), + tlsCfg: newDynamicTLSConfig(tlsCfg, config.Logger), + 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.leafWatch.RunWithClientAndHclog(client, s.logger) + go s.rootsWatch.RunWithClientAndHclog(config.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,