Centralise tls configuration part 2 (#5374)

This PR is based on #5366 and continues to centralise the tls configuration in order to be reloadable eventually!

This PR is another refactoring. No tests are changed, beyond calling other functions or cosmetic stuff. I added a bunch of tests, even though they might be redundant.

agent/agent.go

@@ -251,6 +251,8 @@ type Agent struct {
 	// Envoy.
 	grpcServer *grpc.Server
 
+	// tlsConfigurator is the central instance to provide a *tls.Config
+	// based on the current consul configuration.
 	tlsConfigurator *tlsutil.Configurator
 }
 
@@ -2108,7 +2110,8 @@ func (a *Agent) AddCheck(check *structs.HealthCheck, chkType *structs.CheckType,
 				chkType.Interval = checks.MinInterval
 			}
 
-			tlsClientConfig, err := a.setupTLSClientConfig(chkType.TLSSkipVerify)
+			a.tlsConfigurator.AddCheck(string(check.CheckID), chkType.TLSSkipVerify)
+			tlsClientConfig, err := a.tlsConfigurator.OutgoingTLSConfigForCheck(string(check.CheckID))
 			if err != nil {
 				return fmt.Errorf("Failed to set up TLS: %v", err)
 			}
@@ -2163,7 +2166,8 @@ func (a *Agent) AddCheck(check *structs.HealthCheck, chkType *structs.CheckType,
 			var tlsClientConfig *tls.Config
 			if chkType.GRPCUseTLS {
 				var err error
-				tlsClientConfig, err = a.setupTLSClientConfig(chkType.TLSSkipVerify)
+				a.tlsConfigurator.AddCheck(string(check.CheckID), chkType.TLSSkipVerify)
+				tlsClientConfig, err = a.tlsConfigurator.OutgoingTLSConfigForCheck(string(check.CheckID))
 				if err != nil {
 					return fmt.Errorf("Failed to set up TLS: %v", err)
 				}
@@ -2301,23 +2305,6 @@ func (a *Agent) AddCheck(check *structs.HealthCheck, chkType *structs.CheckType,
 	return nil
 }
 
-func (a *Agent) setupTLSClientConfig(skipVerify bool) (tlsClientConfig *tls.Config, err error) {
-	// We re-use the API client's TLS structure since it
-	// closely aligns with Consul's internal configuration.
-	tlsConfig := &api.TLSConfig{
-		InsecureSkipVerify: skipVerify,
-	}
-	if a.config.EnableAgentTLSForChecks {
-		tlsConfig.Address = a.config.ServerName
-		tlsConfig.KeyFile = a.config.KeyFile
-		tlsConfig.CertFile = a.config.CertFile
-		tlsConfig.CAFile = a.config.CAFile
-		tlsConfig.CAPath = a.config.CAPath
-	}
-	tlsClientConfig, err = api.SetupTLSConfig(tlsConfig)
-	return
-}
-
 // RemoveCheck is used to remove a health check.
 // The agent will make a best effort to ensure it is deregistered
 func (a *Agent) RemoveCheck(checkID types.CheckID, persist bool) error {
@@ -2328,6 +2315,7 @@ func (a *Agent) RemoveCheck(checkID types.CheckID, persist bool) error {
 
 	// Add to the local state for anti-entropy
 	a.State.RemoveCheck(checkID)
+	a.tlsConfigurator.RemoveCheck(string(checkID))
 
 	a.checkLock.Lock()
 	defer a.checkLock.Unlock()

tlsutil/config.go

@@ -4,9 +4,9 @@ import (
 	"crypto/tls"
 	"crypto/x509"
 	"fmt"
-	"io/ioutil"
 	"net"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/hashicorp/go-rootcerts"
@@ -33,8 +33,16 @@ type Config struct {
 	// VerifyIncoming is used to verify the authenticity of incoming connections.
 	// This means that TCP requests are forbidden, only allowing for TLS. TLS connections
 	// must match a provided certificate authority. This can be used to force client auth.
-	VerifyIncoming      bool
+	VerifyIncoming bool
-	VerifyIncomingRPC   bool
+
+	// VerifyIncomingRPC is used to verify the authenticity of incoming RPC connections.
+	// This means that TCP requests are forbidden, only allowing for TLS. TLS connections
+	// must match a provided certificate authority. This can be used to force client auth.
+	VerifyIncomingRPC bool
+
+	// VerifyIncomingHTTPS is used to verify the authenticity of incoming HTTPS connections.
+	// This means that TCP requests are forbidden, only allowing for TLS. TLS connections
+	// must match a provided certificate authority. This can be used to force client auth.
 	VerifyIncomingHTTPS bool
 
 	// VerifyOutgoing is used to verify the authenticity of outgoing connections.
@@ -90,29 +98,14 @@ type Config struct {
 	// over the client ciphersuites.
 	PreferServerCipherSuites bool
 
+	// EnableAgentTLSForChecks is used to apply the agent's TLS settings in
+	// order to configure the HTTP client used for health checks. Enabling
+	// this allows HTTP checks to present a client certificate and verify
+	// the server using the same TLS configuration as the agent (CA, cert,
+	// and key).
 	EnableAgentTLSForChecks bool
 }
 
-// AppendCA opens and parses the CA file and adds the certificates to
-// the provided CertPool.
-func (c *Config) AppendCA(pool *x509.CertPool) error {
-	if c.CAFile == "" {
-		return nil
-	}
-
-	// Read the file
-	data, err := ioutil.ReadFile(c.CAFile)
-	if err != nil {
-		return fmt.Errorf("Failed to read CA file: %v", err)
-	}
-
-	if !pool.AppendCertsFromPEM(data) {
-		return fmt.Errorf("Failed to parse any CA certificates")
-	}
-
-	return nil
-}
-
 // KeyPair is used to open and parse a certificate and key file
 func (c *Config) KeyPair() (*tls.Certificate, error) {
 	if c.CertFile == "" || c.KeyFile == "" {
@@ -198,22 +191,44 @@ func (c *Config) wrapTLSClient(conn net.Conn, tlsConfig *tls.Config) (net.Conn,
 	return tlsConn, err
 }
 
+// Configurator holds a Config and is responsible for generating all the
+// *tls.Config necessary for Consul. Except the one in the api package.
 type Configurator struct {
-	base *Config
+	sync.Mutex
+	base   *Config
+	checks map[string]bool
 }
 
+// NewConfigurator creates a new Configurator and sets the provided
+// configuration.
+// Todo (Hans): should config be a value instead a pointer to avoid side
+// effects?
 func NewConfigurator(config *Config) *Configurator {
-	return &Configurator{base: config}
+	return &Configurator{base: config, checks: map[string]bool{}}
 }
 
-func (c *Configurator) commonTLSConfig() (*tls.Config, error) {
+// Update updates the internal configuration which is used to generate
+// *tls.Config.
+func (c *Configurator) Update(config *Config) {
+	c.Lock()
+	defer c.Unlock()
+	c.base = config
+}
+
+// commonTLSConfig generates a *tls.Config from the base configuration the
+// Configurator has. It accepts an additional flag in case a config is needed
+// for incoming TLS connections.
+func (c *Configurator) commonTLSConfig(additionalVerifyIncomingFlag bool) (*tls.Config, error) {
 	if c.base == nil {
-		return nil, fmt.Errorf("No config")
+		return nil, fmt.Errorf("No base config")
 	}
 
 	tlsConfig := &tls.Config{
-		ServerName: c.base.ServerName,
+		ClientAuth:         tls.NoClientCert,
+		InsecureSkipVerify: c.base.skipBuiltinVerify(),
+		ServerName:         c.base.ServerName,
 	}
+
 	if tlsConfig.ServerName == "" {
 		tlsConfig.ServerName = c.base.NodeName
 	}
@@ -242,44 +257,12 @@ func (c *Configurator) commonTLSConfig() (*tls.Config, error) {
 		}
 		tlsConfig.MinVersion = tlsvers
 	}
-	return tlsConfig, nil
-}
-
-func (c *Configurator) outgoingTLSConfig() (*tls.Config, error) {
-	tlsConfig, err := c.commonTLSConfig()
-	if err != nil {
-		return nil, err
-	}
-
-	tlsConfig.RootCAs = x509.NewCertPool()
-	tlsConfig.InsecureSkipVerify = c.base.skipBuiltinVerify()
 
 	// Ensure we have a CA if VerifyOutgoing is set
 	if c.base.VerifyOutgoing && c.base.CAFile == "" && c.base.CAPath == "" {
 		return nil, fmt.Errorf("VerifyOutgoing set, and no CA certificate provided!")
 	}
 
-	// Parse the CA certs if any
-	rootConfig := &rootcerts.Config{
-		CAFile: c.base.CAFile,
-		CAPath: c.base.CAPath,
-	}
-	if err := rootcerts.ConfigureTLS(tlsConfig, rootConfig); err != nil {
-		return nil, err
-	}
-
-	return tlsConfig, nil
-}
-
-func (c *Configurator) incomingTLSConfig(verify bool) (*tls.Config, error) {
-	tlsConfig, err := c.commonTLSConfig()
-	if err != nil {
-		return nil, err
-	}
-
-	tlsConfig.ClientCAs = x509.NewCertPool()
-	tlsConfig.ClientAuth = tls.NoClientCert
-
 	// Parse the CA certs if any
 	if c.base.CAFile != "" {
 		pool, err := rootcerts.LoadCAFile(c.base.CAFile)
@@ -287,43 +270,73 @@ func (c *Configurator) incomingTLSConfig(verify bool) (*tls.Config, error) {
 			return nil, err
 		}
 		tlsConfig.ClientCAs = pool
+		tlsConfig.RootCAs = pool
 	} else if c.base.CAPath != "" {
 		pool, err := rootcerts.LoadCAPath(c.base.CAPath)
 		if err != nil {
 			return nil, err
 		}
 		tlsConfig.ClientCAs = pool
+		tlsConfig.RootCAs = pool
 	}
 
-	if verify {
+	// Set ClientAuth if necessary
-		tlsConfig.ClientAuth = tls.RequireAndVerifyClientCert
+	if c.base.VerifyIncoming || additionalVerifyIncomingFlag {
-
 		if c.base.CAFile == "" && c.base.CAPath == "" {
 			return nil, fmt.Errorf("VerifyIncoming set, and no CA certificate provided!")
 		}
 		if len(tlsConfig.Certificates) == 0 {
 			return nil, fmt.Errorf("VerifyIncoming set, and no Cert/Key pair provided!")
 		}
+
+		tlsConfig.ClientAuth = tls.RequireAndVerifyClientCert
 	}
+
 	return tlsConfig, nil
 }
 
+// IncomingRPCConfig generates a *tls.Config for incoming RPC connections.
 func (c *Configurator) IncomingRPCConfig() (*tls.Config, error) {
-	return c.incomingTLSConfig(c.base.VerifyIncoming || c.base.VerifyIncomingRPC)
+	return c.commonTLSConfig(c.base.VerifyIncomingRPC)
 }
 
+// IncomingHTTPSConfig generates a *tls.Config for incoming HTTPS connections.
 func (c *Configurator) IncomingHTTPSConfig() (*tls.Config, error) {
-	return c.incomingTLSConfig(c.base.VerifyIncoming || c.base.VerifyIncomingHTTPS)
+	return c.commonTLSConfig(c.base.VerifyIncomingHTTPS)
 }
 
+// IncomingTLSConfig generates a *tls.Config for outgoing TLS connections for
+// checks. This function is seperated because there is an extra flag to
+// consider for checks. EnableAgentTLSForChecks and InsecureSkipVerify has to
+// be checked for checks.
+func (c *Configurator) OutgoingTLSConfigForCheck(id string) (*tls.Config, error) {
+	if !c.base.EnableAgentTLSForChecks {
+		return &tls.Config{
+			InsecureSkipVerify: c.getSkipVerifyForCheck(id),
+		}, nil
+	}
+
+	tlsConfig, err := c.commonTLSConfig(false)
+	if err != nil {
+		return nil, err
+	}
+	tlsConfig.InsecureSkipVerify = c.getSkipVerifyForCheck(id)
+	return tlsConfig, nil
+}
+
+// OutgoingRPCConfig generates a *tls.Config for outgoing RPC connections. If
+// there is a CA or VerifyOutgoing is set, a *tls.Config will be provided,
+// otherwise we assume that no TLS should be used.
 func (c *Configurator) OutgoingRPCConfig() (*tls.Config, error) {
 	useTLS := c.base.CAFile != "" || c.base.CAPath != "" || c.base.VerifyOutgoing
 	if !useTLS {
 		return nil, nil
 	}
-	return c.outgoingTLSConfig()
+	return c.commonTLSConfig(false)
 }
 
+// OutgoingRPCWrapper wraps the result of OutgoingRPCConfig in a DCWrapper. It
+// decides if verify server hostname should be used.
 func (c *Configurator) OutgoingRPCWrapper() (DCWrapper, error) {
 	// Get the TLS config
 	tlsConfig, err := c.OutgoingRPCConfig()
@@ -350,7 +363,29 @@ func (c *Configurator) OutgoingRPCWrapper() (DCWrapper, error) {
 	return wrapper, nil
 }
 
-// ParseCiphers parse ciphersuites from the comma-separated string into recognized slice
+// AddCheck adds a check to the internal check map together with the skipVerify
+// value, which is used when generating a *tls.Config for this check.
+func (c *Configurator) AddCheck(id string, skipVerify bool) {
+	c.Lock()
+	defer c.Unlock()
+	c.checks[id] = skipVerify
+}
+
+// RemoveCheck removes a check from the internal check map.
+func (c *Configurator) RemoveCheck(id string) {
+	c.Lock()
+	defer c.Unlock()
+	delete(c.checks, id)
+}
+
+func (c *Configurator) getSkipVerifyForCheck(id string) bool {
+	c.Lock()
+	defer c.Unlock()
+	return c.checks[id]
+}
+
+// ParseCiphers parse ciphersuites from the comma-separated string into
+// recognized slice
 func ParseCiphers(cipherStr string) ([]uint16, error) {
 	suites := []uint16{}
 

tlsutil/config_test.go

@@ -14,32 +14,6 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestConfig_AppendCA_None(t *testing.T) {
-	conf := &Config{}
-	pool := x509.NewCertPool()
-	err := conf.AppendCA(pool)
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	if len(pool.Subjects()) != 0 {
-		t.Fatalf("bad: %v", pool.Subjects())
-	}
-}
-
-func TestConfig_CACertificate_Valid(t *testing.T) {
-	conf := &Config{
-		CAFile: "../test/ca/root.cer",
-	}
-	pool := x509.NewCertPool()
-	err := conf.AppendCA(pool)
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	if len(pool.Subjects()) == 0 {
-		t.Fatalf("expected cert")
-	}
-}
-
 func TestConfig_KeyPair_None(t *testing.T) {
 	conf := &Config{}
 	cert, err := conf.KeyPair()
@@ -94,11 +68,71 @@ func TestConfigurator_OutgoingTLS_VerifyOutgoing(t *testing.T) {
 	tlsConf, err := c.OutgoingRPCConfig()
 	require.NoError(t, err)
 	require.NotNil(t, tlsConf)
-	require.Equal(t, len(tlsConf.RootCAs.Subjects()), 1)
+	require.Len(t, tlsConf.RootCAs.Subjects(), 1)
 	require.Empty(t, tlsConf.ServerName)
 	require.True(t, tlsConf.InsecureSkipVerify)
 }
 
+func TestConfigurator_OutgoingTLS_ServerName(t *testing.T) {
+	conf := &Config{
+		VerifyOutgoing: true,
+		CAFile:         "../test/ca/root.cer",
+		ServerName:     "consul.example.com",
+	}
+	c := NewConfigurator(conf)
+	tlsConf, err := c.OutgoingRPCConfig()
+	require.NoError(t, err)
+	require.NotNil(t, tlsConf)
+	require.Len(t, tlsConf.RootCAs.Subjects(), 1)
+	require.Equal(t, tlsConf.ServerName, "consul.example.com")
+	require.False(t, tlsConf.InsecureSkipVerify)
+}
+
+func TestConfigurator_OutgoingTLS_VerifyHostname(t *testing.T) {
+	conf := &Config{
+		VerifyOutgoing:       true,
+		VerifyServerHostname: true,
+		CAFile:               "../test/ca/root.cer",
+	}
+	c := NewConfigurator(conf)
+	tlsConf, err := c.OutgoingRPCConfig()
+	require.NoError(t, err)
+	require.NotNil(t, tlsConf)
+	require.Len(t, tlsConf.RootCAs.Subjects(), 1)
+	require.False(t, tlsConf.InsecureSkipVerify)
+}
+
+func TestConfigurator_OutgoingTLS_WithKeyPair(t *testing.T) {
+	conf := &Config{
+		VerifyOutgoing: true,
+		CAFile:         "../test/ca/root.cer",
+		CertFile:       "../test/key/ourdomain.cer",
+		KeyFile:        "../test/key/ourdomain.key",
+	}
+	c := NewConfigurator(conf)
+	tlsConf, err := c.OutgoingRPCConfig()
+	require.NoError(t, err)
+	require.NotNil(t, tlsConf)
+	require.True(t, tlsConf.InsecureSkipVerify)
+	require.Len(t, tlsConf.Certificates, 1)
+}
+
+func TestConfigurator_OutgoingTLS_TLSMinVersion(t *testing.T) {
+	tlsVersions := []string{"tls10", "tls11", "tls12"}
+	for _, version := range tlsVersions {
+		conf := &Config{
+			VerifyOutgoing: true,
+			CAFile:         "../test/ca/root.cer",
+			TLSMinVersion:  version,
+		}
+		c := NewConfigurator(conf)
+		tlsConf, err := c.OutgoingRPCConfig()
+		require.NoError(t, err)
+		require.NotNil(t, tlsConf)
+		require.Equal(t, tlsConf.MinVersion, TLSLookup[version])
+	}
+}
+
 func TestConfig_SkipBuiltinVerify(t *testing.T) {
 	type variant struct {
 		config Config
@@ -116,66 +150,6 @@ func TestConfig_SkipBuiltinVerify(t *testing.T) {
 	}
 }
 
-func TestConfigurator_OutgoingTLS_ServerName(t *testing.T) {
-	conf := &Config{
-		VerifyOutgoing: true,
-		CAFile:         "../test/ca/root.cer",
-		ServerName:     "consul.example.com",
-	}
-	c := NewConfigurator(conf)
-	tlsConf, err := c.OutgoingRPCConfig()
-	require.NoError(t, err)
-	require.NotNil(t, tlsConf)
-	require.Equal(t, len(tlsConf.RootCAs.Subjects()), 1)
-	require.Equal(t, tlsConf.ServerName, "consul.example.com")
-	require.False(t, tlsConf.InsecureSkipVerify)
-}
-
-func TestConfigurator_OutgoingTLS_VerifyHostname(t *testing.T) {
-	conf := &Config{
-		VerifyOutgoing:       true,
-		VerifyServerHostname: true,
-		CAFile:               "../test/ca/root.cer",
-	}
-	c := NewConfigurator(conf)
-	tlsConf, err := c.OutgoingRPCConfig()
-	require.NoError(t, err)
-	require.NotNil(t, tlsConf)
-	require.Equal(t, len(tlsConf.RootCAs.Subjects()), 1)
-	require.False(t, tlsConf.InsecureSkipVerify)
-}
-
-func TestConfigurator_OutgoingTLS_WithKeyPair(t *testing.T) {
-	conf := &Config{
-		VerifyOutgoing: true,
-		CAFile:         "../test/ca/root.cer",
-		CertFile:       "../test/key/ourdomain.cer",
-		KeyFile:        "../test/key/ourdomain.key",
-	}
-	c := NewConfigurator(conf)
-	tlsConf, err := c.OutgoingRPCConfig()
-	require.NoError(t, err)
-	require.NotNil(t, tlsConf)
-	require.True(t, tlsConf.InsecureSkipVerify)
-	require.Equal(t, len(tlsConf.Certificates), 1)
-}
-
-func TestConfigurator_OutgoingTLS_TLSMinVersion(t *testing.T) {
-	tlsVersions := []string{"tls10", "tls11", "tls12"}
-	for _, version := range tlsVersions {
-		conf := &Config{
-			VerifyOutgoing: true,
-			CAFile:         "../test/ca/root.cer",
-			TLSMinVersion:  version,
-		}
-		c := NewConfigurator(conf)
-		tlsConf, err := c.OutgoingRPCConfig()
-		require.NoError(t, err)
-		require.NotNil(t, tlsConf)
-		require.Equal(t, tlsConf.MinVersion, TLSLookup[version])
-	}
-}
-
 func startTLSServer(config *Config) (net.Conn, chan error) {
 	errc := make(chan error, 1)
 
@@ -427,7 +401,7 @@ func TestConfigurator_IncomingHTTPSConfig_CA_PATH(t *testing.T) {
 	c := NewConfigurator(conf)
 	tlsConf, err := c.IncomingHTTPSConfig()
 	require.NoError(t, err)
-	require.Equal(t, len(tlsConf.ClientCAs.Subjects()), 2)
+	require.Len(t, tlsConf.ClientCAs.Subjects(), 2)
 }
 
 func TestConfigurator_IncomingHTTPS(t *testing.T) {
@@ -441,9 +415,9 @@ func TestConfigurator_IncomingHTTPS(t *testing.T) {
 	tlsConf, err := c.IncomingHTTPSConfig()
 	require.NoError(t, err)
 	require.NotNil(t, tlsConf)
-	require.Equal(t, len(tlsConf.ClientCAs.Subjects()), 1)
+	require.Len(t, tlsConf.ClientCAs.Subjects(), 1)
 	require.Equal(t, tlsConf.ClientAuth, tls.RequireAndVerifyClientCert)
-	require.Equal(t, len(tlsConf.Certificates), 1)
+	require.Len(t, tlsConf.Certificates, 1)
 }
 
 func TestConfigurator_IncomingHTTPS_MissingCA(t *testing.T) {
@@ -473,9 +447,9 @@ func TestConfigurator_IncomingHTTPS_NoVerify(t *testing.T) {
 	tlsConf, err := c.IncomingHTTPSConfig()
 	require.NoError(t, err)
 	require.NotNil(t, tlsConf)
-	require.Equal(t, len(tlsConf.ClientCAs.Subjects()), 0)
+	require.Nil(t, tlsConf.ClientCAs)
 	require.Equal(t, tlsConf.ClientAuth, tls.NoClientCert)
-	require.Equal(t, len(tlsConf.Certificates), 0)
+	require.Empty(t, tlsConf.Certificates)
 }
 
 func TestConfigurator_IncomingHTTPS_TLSMinVersion(t *testing.T) {
@@ -497,16 +471,241 @@ func TestConfigurator_IncomingHTTPS_TLSMinVersion(t *testing.T) {
 }
 
 func TestConfigurator_IncomingHTTPSCAPath_Valid(t *testing.T) {
-	conf := &Config{
-		CAPath: "../test/ca_path",
-	}
 
-	c := NewConfigurator(conf)
+	c := NewConfigurator(&Config{CAPath: "../test/ca_path"})
 	tlsConf, err := c.IncomingHTTPSConfig()
-	if err != nil {
+	require.NoError(t, err)
-		t.Fatalf("err: %v", err)
+	require.Len(t, tlsConf.ClientCAs.Subjects(), 2)
+}
+
+func TestConfigurator_CommonTLSConfigNoBaseConfig(t *testing.T) {
+	c := NewConfigurator(nil)
+	_, err := c.commonTLSConfig(false)
+	require.Error(t, err)
+}
+
+func TestConfigurator_CommonTLSConfigServerNameNodeName(t *testing.T) {
+	type variant struct {
+		config *Config
+		result string
 	}
-	if len(tlsConf.ClientCAs.Subjects()) != 2 {
+	variants := []variant{
-		t.Fatalf("expected certs")
+		{config: &Config{NodeName: "node", ServerName: "server"},
+			result: "server"},
+		{config: &Config{ServerName: "server"},
+			result: "server"},
+		{config: &Config{NodeName: "node"},
+			result: "node"},
+	}
+	for _, v := range variants {
+		c := NewConfigurator(v.config)
+		tlsConf, err := c.commonTLSConfig(false)
+		require.NoError(t, err)
+		require.Equal(t, v.result, tlsConf.ServerName)
 	}
 }
+
+func TestConfigurator_CommonTLSConfigCipherSuites(t *testing.T) {
+	c := NewConfigurator(&Config{})
+	tlsConfig, err := c.commonTLSConfig(false)
+	require.NoError(t, err)
+	require.Empty(t, tlsConfig.CipherSuites)
+
+	conf := &Config{CipherSuites: []uint16{tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305}}
+	c.Update(conf)
+	tlsConfig, err = c.commonTLSConfig(false)
+	require.NoError(t, err)
+	require.Equal(t, conf.CipherSuites, tlsConfig.CipherSuites)
+}
+
+func TestConfigurator_CommonTLSConfigCertKey(t *testing.T) {
+	c := NewConfigurator(&Config{})
+	tlsConf, err := c.commonTLSConfig(false)
+	require.NoError(t, err)
+	require.Empty(t, tlsConf.Certificates)
+
+	c.Update(&Config{CertFile: "/something/bogus", KeyFile: "/more/bogus"})
+	tlsConf, err = c.commonTLSConfig(false)
+	require.Error(t, err)
+
+	c.Update(&Config{CertFile: "../test/key/ourdomain.cer", KeyFile: "../test/key/ourdomain.key"})
+	tlsConf, err = c.commonTLSConfig(false)
+	require.NoError(t, err)
+	require.Len(t, tlsConf.Certificates, 1)
+}
+
+func TestConfigurator_CommonTLSConfigTLSMinVersion(t *testing.T) {
+	tlsVersions := []string{"tls10", "tls11", "tls12"}
+	for _, version := range tlsVersions {
+		c := NewConfigurator(&Config{TLSMinVersion: version})
+		tlsConf, err := c.commonTLSConfig(false)
+		require.NoError(t, err)
+		require.Equal(t, tlsConf.MinVersion, TLSLookup[version])
+	}
+
+	c := NewConfigurator(&Config{TLSMinVersion: "tlsBOGUS"})
+	_, err := c.commonTLSConfig(false)
+	require.Error(t, err)
+}
+
+func TestConfigurator_CommonTLSConfigValidateVerifyOutgoingCA(t *testing.T) {
+	c := NewConfigurator(&Config{VerifyOutgoing: true})
+	_, err := c.commonTLSConfig(false)
+	require.Error(t, err)
+}
+
+func TestConfigurator_CommonTLSConfigLoadCA(t *testing.T) {
+	c := NewConfigurator(&Config{})
+	tlsConf, err := c.commonTLSConfig(false)
+	require.NoError(t, err)
+	require.Nil(t, tlsConf.RootCAs)
+	require.Nil(t, tlsConf.ClientCAs)
+
+	c.Update(&Config{CAFile: "/something/bogus"})
+	_, err = c.commonTLSConfig(false)
+	require.Error(t, err)
+
+	c.Update(&Config{CAPath: "/something/bogus/"})
+	_, err = c.commonTLSConfig(false)
+	require.Error(t, err)
+
+	c.Update(&Config{CAFile: "../test/ca/root.cer"})
+	tlsConf, err = c.commonTLSConfig(false)
+	require.NoError(t, err)
+	require.Len(t, tlsConf.RootCAs.Subjects(), 1)
+	require.Len(t, tlsConf.ClientCAs.Subjects(), 1)
+
+	c.Update(&Config{CAPath: "../test/ca_path"})
+	tlsConf, err = c.commonTLSConfig(false)
+	require.NoError(t, err)
+	require.Len(t, tlsConf.RootCAs.Subjects(), 2)
+	require.Len(t, tlsConf.ClientCAs.Subjects(), 2)
+
+	c.Update(&Config{CAFile: "../test/ca/root.cer", CAPath: "../test/ca_path"})
+	tlsConf, err = c.commonTLSConfig(false)
+	require.NoError(t, err)
+	require.Len(t, tlsConf.RootCAs.Subjects(), 1)
+	require.Len(t, tlsConf.ClientCAs.Subjects(), 1)
+}
+
+func TestConfigurator_CommonTLSConfigVerifyIncoming(t *testing.T) {
+	c := NewConfigurator(&Config{})
+	tlsConf, err := c.commonTLSConfig(false)
+	require.NoError(t, err)
+	require.Equal(t, tls.NoClientCert, tlsConf.ClientAuth)
+
+	c.Update(&Config{VerifyIncoming: true})
+	tlsConf, err = c.commonTLSConfig(false)
+	require.Error(t, err)
+
+	c.Update(&Config{VerifyIncoming: true, CAFile: "../test/ca/root.cer"})
+	tlsConf, err = c.commonTLSConfig(false)
+	require.Error(t, err)
+
+	c.Update(&Config{VerifyIncoming: true, CAFile: "../test/ca/root.cer", CertFile: "../test/cert/ourdomain.cer"})
+	tlsConf, err = c.commonTLSConfig(false)
+	require.Error(t, err)
+
+	c.Update(&Config{VerifyIncoming: true, CAFile: "../test/ca/root.cer", CertFile: "../test/key/ourdomain.cer", KeyFile: "../test/key/ourdomain.key"})
+	tlsConf, err = c.commonTLSConfig(false)
+	require.NoError(t, err)
+	require.Equal(t, tls.RequireAndVerifyClientCert, tlsConf.ClientAuth)
+
+	c.Update(&Config{VerifyIncoming: false, CAFile: "../test/ca/root.cer", CertFile: "../test/key/ourdomain.cer", KeyFile: "../test/key/ourdomain.key"})
+	tlsConf, err = c.commonTLSConfig(true)
+	require.NoError(t, err)
+	require.Equal(t, tls.RequireAndVerifyClientCert, tlsConf.ClientAuth)
+}
+
+func TestConfigurator_IncomingRPCConfig(t *testing.T) {
+	c := NewConfigurator(&Config{})
+	tlsConf, err := c.IncomingRPCConfig()
+	require.NoError(t, err)
+	require.Equal(t, tls.NoClientCert, tlsConf.ClientAuth)
+
+	c.Update(&Config{VerifyIncoming: true, CAFile: "../test/ca/root.cer", CertFile: "../test/key/ourdomain.cer", KeyFile: "../test/key/ourdomain.key"})
+	tlsConf, err = c.IncomingRPCConfig()
+	require.NoError(t, err)
+	require.Equal(t, tls.RequireAndVerifyClientCert, tlsConf.ClientAuth)
+
+	c.Update(&Config{VerifyIncomingRPC: true, CAFile: "../test/ca/root.cer", CertFile: "../test/key/ourdomain.cer", KeyFile: "../test/key/ourdomain.key"})
+	tlsConf, err = c.IncomingRPCConfig()
+	require.NoError(t, err)
+	require.Equal(t, tls.RequireAndVerifyClientCert, tlsConf.ClientAuth)
+
+	c.Update(&Config{VerifyIncomingHTTPS: true, CAFile: "../test/ca/root.cer", CertFile: "../test/key/ourdomain.cer", KeyFile: "../test/key/ourdomain.key"})
+	tlsConf, err = c.IncomingRPCConfig()
+	require.NoError(t, err)
+	require.Equal(t, tls.NoClientCert, tlsConf.ClientAuth)
+}
+
+func TestConfigurator_IncomingHTTPSConfig(t *testing.T) {
+	c := NewConfigurator(&Config{})
+	tlsConf, err := c.IncomingHTTPSConfig()
+	require.NoError(t, err)
+	require.Equal(t, tls.NoClientCert, tlsConf.ClientAuth)
+
+	c.Update(&Config{VerifyIncoming: true, CAFile: "../test/ca/root.cer", CertFile: "../test/key/ourdomain.cer", KeyFile: "../test/key/ourdomain.key"})
+	tlsConf, err = c.IncomingHTTPSConfig()
+	require.NoError(t, err)
+	require.Equal(t, tls.RequireAndVerifyClientCert, tlsConf.ClientAuth)
+
+	c.Update(&Config{VerifyIncomingHTTPS: true, CAFile: "../test/ca/root.cer", CertFile: "../test/key/ourdomain.cer", KeyFile: "../test/key/ourdomain.key"})
+	tlsConf, err = c.IncomingHTTPSConfig()
+	require.NoError(t, err)
+	require.Equal(t, tls.RequireAndVerifyClientCert, tlsConf.ClientAuth)
+
+	c.Update(&Config{VerifyIncomingRPC: true, CAFile: "../test/ca/root.cer", CertFile: "../test/key/ourdomain.cer", KeyFile: "../test/key/ourdomain.key"})
+	tlsConf, err = c.IncomingHTTPSConfig()
+	require.NoError(t, err)
+	require.Equal(t, tls.NoClientCert, tlsConf.ClientAuth)
+}
+
+func TestConfigurator_OutgoingRPCConfig(t *testing.T) {
+	c := NewConfigurator(&Config{})
+	tlsConf, err := c.OutgoingRPCConfig()
+	require.NoError(t, err)
+	require.Nil(t, tlsConf)
+
+	c.Update(&Config{VerifyOutgoing: true})
+	tlsConf, err = c.OutgoingRPCConfig()
+	require.Error(t, err)
+
+	c.Update(&Config{VerifyOutgoing: true, CAFile: "../test/ca/root.cer"})
+	tlsConf, err = c.OutgoingRPCConfig()
+	require.NoError(t, err)
+
+	c.Update(&Config{VerifyOutgoing: true, CAPath: "../test/ca_path"})
+	tlsConf, err = c.OutgoingRPCConfig()
+	require.NoError(t, err)
+}
+
+func TestConfigurator_OutgoingTLSConfigForChecks(t *testing.T) {
+	c := NewConfigurator(&Config{})
+	tlsConf, err := c.OutgoingTLSConfigForCheck("")
+	require.NoError(t, err)
+	require.False(t, tlsConf.InsecureSkipVerify)
+
+	c.Update(&Config{EnableAgentTLSForChecks: true})
+	tlsConf, err = c.OutgoingTLSConfigForCheck("")
+	require.NoError(t, err)
+	require.False(t, tlsConf.InsecureSkipVerify)
+
+	c.AddCheck("c1", true)
+	c.Update(&Config{EnableAgentTLSForChecks: true})
+	tlsConf, err = c.OutgoingTLSConfigForCheck("c1")
+	require.NoError(t, err)
+	require.True(t, tlsConf.InsecureSkipVerify)
+
+	c.AddCheck("c1", false)
+	c.Update(&Config{EnableAgentTLSForChecks: true})
+	tlsConf, err = c.OutgoingTLSConfigForCheck("c1")
+	require.NoError(t, err)
+	require.False(t, tlsConf.InsecureSkipVerify)
+
+	c.AddCheck("c1", false)
+	c.Update(&Config{EnableAgentTLSForChecks: true})
+	tlsConf, err = c.OutgoingTLSConfigForCheck("c1")
+	require.NoError(t, err)
+	require.False(t, tlsConf.InsecureSkipVerify)
+}