agent: enable reloading of tls config (#5419)

This PR introduces reloading tls configuration. Consul will now be able to reload the TLS configuration which previously required a restart. It is not yet possible to turn TLS ON or OFF with these changes. Only when TLS is already turned on, the configuration can be reloaded. Most importantly the certificates and CAs.
hashicorp · Mar 13, 2019 · 7e11dd8 · 7e11dd8
1 parent cd1aa9b
commit 7e11dd8
Show file tree

Hide file tree

Showing 12 changed files with 876 additions and 645 deletions.
diff --git a/agent/agent.go b/agent/agent.go
@@ -393,7 +393,11 @@ func (a *Agent) Start() error {
 	// waiting to discover a consul server
 	consulCfg.ServerUp = a.sync.SyncFull.Trigger
 
-	a.tlsConfigurator = tlsutil.NewConfigurator(c.ToTLSUtilConfig())
+	tlsConfigurator, err := tlsutil.NewConfigurator(c.ToTLSUtilConfig(), a.logger)
+	if err != nil {
+		return err
+	}
+	a.tlsConfigurator = tlsConfigurator
 
 	// Setup either the client or the server.
 	if c.ServerMode {
@@ -662,10 +666,7 @@ func (a *Agent) listenHTTP() ([]*HTTPServer, error) {
 			var tlscfg *tls.Config
 			_, isTCP := l.(*tcpKeepAliveListener)
 			if isTCP && proto == "https" {
-				tlscfg, err = a.tlsConfigurator.IncomingHTTPSConfig()
-				if err != nil {
-					return err
-				}
+				tlscfg = a.tlsConfigurator.IncomingHTTPSConfig()
 				l = tls.NewListener(l, tlscfg)
 			}
 			srv := &HTTPServer{
@@ -2232,11 +2233,7 @@ func (a *Agent) addCheck(check *structs.HealthCheck, chkType *structs.CheckType,
 				chkType.Interval = checks.MinInterval
 			}
 
-			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)
-			}
+			tlsClientConfig := a.tlsConfigurator.OutgoingTLSConfigForCheck(chkType.TLSSkipVerify)
 
 			http := &checks.CheckHTTP{
 				Notify:          a.State,
@@ -2287,12 +2284,7 @@ func (a *Agent) addCheck(check *structs.HealthCheck, chkType *structs.CheckType,
 
 			var tlsClientConfig *tls.Config
 			if chkType.GRPCUseTLS {
-				var err error
-				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)
-				}
+				tlsClientConfig = a.tlsConfigurator.OutgoingTLSConfigForCheck(chkType.TLSSkipVerify)
 			}
 
 			grpc := &checks.CheckGRPC{
@@ -2431,7 +2423,6 @@ func (a *Agent) removeCheckLocked(checkID types.CheckID, persist bool) error {
 		return fmt.Errorf("CheckID missing")
 	}
 
-	a.tlsConfigurator.RemoveCheck(string(checkID))
 	a.cancelCheckMonitors(checkID)
 	a.State.RemoveCheck(checkID)
 
@@ -3559,6 +3550,10 @@ func (a *Agent) ReloadConfig(newCfg *config.RuntimeConfig) error {
 	// the checks and service registrations.
 	a.loadTokens(newCfg)
 
+	if err := a.tlsConfigurator.Update(newCfg.ToTLSUtilConfig()); err != nil {
+		return fmt.Errorf("Failed reloading tls configuration: %s", err)
+	}
+
 	// Reload service/check definitions and metadata.
 	if err := a.loadServices(newCfg); err != nil {
 		return fmt.Errorf("Failed reloading services: %s", err)

diff --git a/agent/agent_test.go b/agent/agent_test.go
@@ -2,6 +2,7 @@ package agent
 
 import (
 	"bytes"
+	"crypto/tls"
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
@@ -3366,23 +3367,15 @@ func TestAgent_SetupProxyManager(t *testing.T) {
 		ports { http = -1 }
 		data_dir = "` + dataDir + `"
 	`
-	c := TestConfig(
-		// randomPortsSource(false),
-		config.Source{Name: t.Name(), Format: "hcl", Data: hcl},
-	)
-	a, err := New(c)
+	a, err := NewUnstartedAgent(t, t.Name(), hcl)
 	require.NoError(t, err)
 	require.Error(t, a.setupProxyManager(), "setupProxyManager should fail with invalid HTTP API config")
 
 	hcl = `
 		ports { http = 8001 }
 		data_dir = "` + dataDir + `"
 	`
-	c = TestConfig(
-		// randomPortsSource(false),
-		config.Source{Name: t.Name(), Format: "hcl", Data: hcl},
-	)
-	a, err = New(c)
+	a, err = NewUnstartedAgent(t, t.Name(), hcl)
 	require.NoError(t, err)
 	require.NoError(t, a.setupProxyManager())
 }
@@ -3543,3 +3536,107 @@ func TestAgent_loadTokens(t *testing.T) {
 		require.Equal("foxtrot", a.tokens.ReplicationToken())
 	})
 }
+
+func TestAgent_ReloadConfigOutgoingRPCConfig(t *testing.T) {
+	t.Parallel()
+	dataDir := testutil.TempDir(t, "agent") // we manage the data dir
+	defer os.RemoveAll(dataDir)
+	hcl := `
+		data_dir = "` + dataDir + `"
+		verify_outgoing = true
+		ca_file = "../test/ca/root.cer"
+		cert_file = "../test/key/ourdomain.cer"
+		key_file = "../test/key/ourdomain.key"
+		verify_server_hostname = false
+	`
+	a, err := NewUnstartedAgent(t, t.Name(), hcl)
+	require.NoError(t, err)
+	tlsConf := a.tlsConfigurator.OutgoingRPCConfig()
+	require.True(t, tlsConf.InsecureSkipVerify)
+	require.Len(t, tlsConf.ClientCAs.Subjects(), 1)
+	require.Len(t, tlsConf.RootCAs.Subjects(), 1)
+
+	hcl = `
+		data_dir = "` + dataDir + `"
+		verify_outgoing = true
+		ca_path = "../test/ca_path"
+		cert_file = "../test/key/ourdomain.cer"
+		key_file = "../test/key/ourdomain.key"
+		verify_server_hostname = true
+	`
+	c := TestConfig(config.Source{Name: t.Name(), Format: "hcl", Data: hcl})
+	require.NoError(t, a.ReloadConfig(c))
+	tlsConf = a.tlsConfigurator.OutgoingRPCConfig()
+	require.False(t, tlsConf.InsecureSkipVerify)
+	require.Len(t, tlsConf.RootCAs.Subjects(), 2)
+	require.Len(t, tlsConf.ClientCAs.Subjects(), 2)
+}
+
+func TestAgent_ReloadConfigIncomingRPCConfig(t *testing.T) {
+	t.Parallel()
+	dataDir := testutil.TempDir(t, "agent") // we manage the data dir
+	defer os.RemoveAll(dataDir)
+	hcl := `
+		data_dir = "` + dataDir + `"
+		verify_outgoing = true
+		ca_file = "../test/ca/root.cer"
+		cert_file = "../test/key/ourdomain.cer"
+		key_file = "../test/key/ourdomain.key"
+		verify_server_hostname = false
+	`
+	a, err := NewUnstartedAgent(t, t.Name(), hcl)
+	require.NoError(t, err)
+	tlsConf := a.tlsConfigurator.IncomingRPCConfig()
+	require.NotNil(t, tlsConf.GetConfigForClient)
+	tlsConf, err = tlsConf.GetConfigForClient(nil)
+	require.NoError(t, err)
+	require.NotNil(t, tlsConf)
+	require.True(t, tlsConf.InsecureSkipVerify)
+	require.Len(t, tlsConf.ClientCAs.Subjects(), 1)
+	require.Len(t, tlsConf.RootCAs.Subjects(), 1)
+
+	hcl = `
+		data_dir = "` + dataDir + `"
+		verify_outgoing = true
+		ca_path = "../test/ca_path"
+		cert_file = "../test/key/ourdomain.cer"
+		key_file = "../test/key/ourdomain.key"
+		verify_server_hostname = true
+	`
+	c := TestConfig(config.Source{Name: t.Name(), Format: "hcl", Data: hcl})
+	require.NoError(t, a.ReloadConfig(c))
+	tlsConf, err = tlsConf.GetConfigForClient(nil)
+	require.NoError(t, err)
+	require.False(t, tlsConf.InsecureSkipVerify)
+	require.Len(t, tlsConf.ClientCAs.Subjects(), 2)
+	require.Len(t, tlsConf.RootCAs.Subjects(), 2)
+}
+
+func TestAgent_ReloadConfigTLSConfigFailure(t *testing.T) {
+	t.Parallel()
+	dataDir := testutil.TempDir(t, "agent") // we manage the data dir
+	defer os.RemoveAll(dataDir)
+	hcl := `
+		data_dir = "` + dataDir + `"
+		verify_outgoing = true
+		ca_file = "../test/ca/root.cer"
+		cert_file = "../test/key/ourdomain.cer"
+		key_file = "../test/key/ourdomain.key"
+		verify_server_hostname = false
+	`
+	a, err := NewUnstartedAgent(t, t.Name(), hcl)
+	require.NoError(t, err)
+	tlsConf := a.tlsConfigurator.IncomingRPCConfig()
+
+	hcl = `
+		data_dir = "` + dataDir + `"
+		verify_incoming = true
+	`
+	c := TestConfig(config.Source{Name: t.Name(), Format: "hcl", Data: hcl})
+	require.Error(t, a.ReloadConfig(c))
+	tlsConf, err = tlsConf.GetConfigForClient(nil)
+	require.NoError(t, err)
+	require.Equal(t, tls.NoClientCert, tlsConf.ClientAuth)
+	require.Len(t, tlsConf.ClientCAs.Subjects(), 1)
+	require.Len(t, tlsConf.RootCAs.Subjects(), 1)
+}
diff --git a/agent/config/runtime.go b/agent/config/runtime.go
@@ -1580,12 +1580,13 @@ func (c *RuntimeConfig) Sanitized() map[string]interface{} {
 	return sanitize("rt", reflect.ValueOf(c)).Interface().(map[string]interface{})
 }
 
-func (c *RuntimeConfig) ToTLSUtilConfig() *tlsutil.Config {
-	return &tlsutil.Config{
+func (c *RuntimeConfig) ToTLSUtilConfig() tlsutil.Config {
+	return tlsutil.Config{
 		VerifyIncoming:           c.VerifyIncoming,
 		VerifyIncomingRPC:        c.VerifyIncomingRPC,
 		VerifyIncomingHTTPS:      c.VerifyIncomingHTTPS,
 		VerifyOutgoing:           c.VerifyOutgoing,
+		VerifyServerHostname:     c.VerifyServerHostname,
 		CAFile:                   c.CAFile,
 		CAPath:                   c.CAPath,
 		CertFile:                 c.CertFile,

diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go
@@ -5434,6 +5434,7 @@ func TestRuntime_ToTLSUtilConfig(t *testing.T) {
 		VerifyIncomingRPC:           true,
 		VerifyIncomingHTTPS:         true,
 		VerifyOutgoing:              true,
+		VerifyServerHostname:        true,
 		CAFile:                      "a",
 		CAPath:                      "b",
 		CertFile:                    "c",
@@ -5450,6 +5451,7 @@ func TestRuntime_ToTLSUtilConfig(t *testing.T) {
 	require.Equal(t, c.VerifyIncomingRPC, r.VerifyIncomingRPC)
 	require.Equal(t, c.VerifyIncomingHTTPS, r.VerifyIncomingHTTPS)
 	require.Equal(t, c.VerifyOutgoing, r.VerifyOutgoing)
+	require.Equal(t, c.VerifyServerHostname, r.VerifyServerHostname)
 	require.Equal(t, c.CAFile, r.CAFile)
 	require.Equal(t, c.CAPath, r.CAPath)
 	require.Equal(t, c.CertFile, r.CertFile)

diff --git a/agent/consul/client.go b/agent/consul/client.go
@@ -86,10 +86,16 @@ type Client struct {
 	EnterpriseClient
 }
 
-// NewClient is used to construct a new Consul client from the
-// configuration, potentially returning an error
+// NewClient is used to construct a new Consul client from the configuration,
+// potentially returning an error.
+// NewClient only used to help setting up a client for testing. Normal code
+// exercises NewClientLogger.
 func NewClient(config *Config) (*Client, error) {
-	return NewClientLogger(config, nil, tlsutil.NewConfigurator(config.ToTLSUtilConfig()))
+	c, err := tlsutil.NewConfigurator(config.ToTLSUtilConfig(), nil)
+	if err != nil {
+		return nil, err
+	}
+	return NewClientLogger(config, nil, c)
 }
 
 func NewClientLogger(config *Config, logger *log.Logger, tlsConfigurator *tlsutil.Configurator) (*Client, error) {
@@ -113,12 +119,6 @@ func NewClientLogger(config *Config, logger *log.Logger, tlsConfigurator *tlsuti
 		config.LogOutput = os.Stderr
 	}
 
-	// Create the tls Wrapper
-	tlsWrap, err := tlsConfigurator.OutgoingRPCWrapper()
-	if err != nil {
-		return nil, err
-	}
-
 	// Create a logger
 	if logger == nil {
 		logger = log.New(config.LogOutput, "", log.LstdFlags)
@@ -129,7 +129,7 @@ func NewClientLogger(config *Config, logger *log.Logger, tlsConfigurator *tlsuti
 		LogOutput:  config.LogOutput,
 		MaxTime:    clientRPCConnMaxIdle,
 		MaxStreams: clientMaxStreams,
-		TLSWrapper: tlsWrap,
+		TLSWrapper: tlsConfigurator.OutgoingRPCWrapper(),
 		ForceTLS:   config.VerifyOutgoing,
 	}
 
@@ -158,6 +158,7 @@ func NewClientLogger(config *Config, logger *log.Logger, tlsConfigurator *tlsuti
 		CacheConfig: clientACLCacheConfig,
 		Sentinel:    nil,
 	}
+	var err error
 	if c.acls, err = NewACLResolver(&aclConfig); err != nil {
 		c.Shutdown()
 		return nil, fmt.Errorf("Failed to create ACL resolver: %v", err)

diff --git a/agent/consul/config.go b/agent/consul/config.go
@@ -379,8 +379,8 @@ type Config struct {
 	CAConfig *structs.CAConfiguration
 }
 
-func (c *Config) ToTLSUtilConfig() *tlsutil.Config {
-	return &tlsutil.Config{
+func (c *Config) ToTLSUtilConfig() tlsutil.Config {
+	return tlsutil.Config{
 		VerifyIncoming:           c.VerifyIncoming,
 		VerifyOutgoing:           c.VerifyOutgoing,
 		CAFile:                   c.CAFile,

diff --git a/agent/consul/server.go b/agent/consul/server.go
@@ -252,11 +252,17 @@ type Server struct {
 	EnterpriseServer
 }
 
+// NewServer is only used to help setting up a server for testing. Normal code
+// exercises NewServerLogger.
 func NewServer(config *Config) (*Server, error) {
-	return NewServerLogger(config, nil, new(token.Store), tlsutil.NewConfigurator(config.ToTLSUtilConfig()))
+	c, err := tlsutil.NewConfigurator(config.ToTLSUtilConfig(), nil)
+	if err != nil {
+		return nil, err
+	}
+	return NewServerLogger(config, nil, new(token.Store), c)
 }
 
-// NewServer is used to construct a new Consul server from the
+// NewServerLogger is used to construct a new Consul server from the
 // configuration, potentially returning an error
 func NewServerLogger(config *Config, logger *log.Logger, tokens *token.Store, tlsConfigurator *tlsutil.Configurator) (*Server, error) {
 	// Check the protocol version.
@@ -296,18 +302,6 @@ func NewServerLogger(config *Config, logger *log.Logger, tokens *token.Store, tl
 		}
 	}
 
-	// Create the TLS wrapper for outgoing connections.
-	tlsWrap, err := tlsConfigurator.OutgoingRPCWrapper()
-	if err != nil {
-		return nil, err
-	}
-
-	// Get the incoming TLS config.
-	incomingTLS, err := tlsConfigurator.IncomingRPCConfig()
-	if err != nil {
-		return nil, err
-	}
-
 	// Create the tombstone GC.
 	gc, err := state.NewTombstoneGC(config.TombstoneTTL, config.TombstoneTTLGranularity)
 	if err != nil {
@@ -322,7 +316,7 @@ func NewServerLogger(config *Config, logger *log.Logger, tokens *token.Store, tl
 		LogOutput:  config.LogOutput,
 		MaxTime:    serverRPCCache,
 		MaxStreams: serverMaxStreams,
-		TLSWrapper: tlsWrap,
+		TLSWrapper: tlsConfigurator.OutgoingRPCWrapper(),
 		ForceTLS:   config.VerifyOutgoing,
 	}
 
@@ -338,7 +332,7 @@ func NewServerLogger(config *Config, logger *log.Logger, tokens *token.Store, tl
 		reconcileCh:      make(chan serf.Member, reconcileChSize),
 		router:           router.NewRouter(logger, config.Datacenter),
 		rpcServer:        rpc.NewServer(),
-		rpcTLS:           incomingTLS,
+		rpcTLS:           tlsConfigurator.IncomingRPCConfig(),
 		reassertLeaderCh: make(chan chan error),
 		segmentLAN:       make(map[string]*serf.Serf, len(config.Segments)),
 		sessionTimers:    NewSessionTimers(),
@@ -373,7 +367,7 @@ func NewServerLogger(config *Config, logger *log.Logger, tokens *token.Store, tl
 	}
 
 	// Initialize the RPC layer.
-	if err := s.setupRPC(tlsWrap); err != nil {
+	if err := s.setupRPC(tlsConfigurator.OutgoingRPCWrapper()); err != nil {
 		s.Shutdown()
 		return nil, fmt.Errorf("Failed to start RPC layer: %v", err)
 	}

diff --git a/agent/consul/server_test.go b/agent/consul/server_test.go
@@ -179,7 +179,11 @@ func newServer(c *Config) (*Server, error) {
 		w = os.Stderr
 	}
 	logger := log.New(w, c.NodeName+" - ", log.LstdFlags|log.Lmicroseconds)
-	srv, err := NewServerLogger(c, logger, new(token.Store), tlsutil.NewConfigurator(c.ToTLSUtilConfig()))
+	tlsConf, err := tlsutil.NewConfigurator(c.ToTLSUtilConfig(), logger)
+	if err != nil {
+		return nil, err
+	}
+	srv, err := NewServerLogger(c, logger, new(token.Store), tlsConf)
 	if err != nil {
 		return nil, err
 	}