reloading tls config should be atomic for clients/servers

hashicorp · Nov 29, 2017 · 8c3df88 · 8c3df88
1 parent c6ccbc9
commit 8c3df88
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 28 deletions.
diff --git a/client/client.go b/client/client.go
@@ -367,21 +367,21 @@ func (c *Client) init() error {
 // ReloadTLSConnections allows a client to reload RPC connections if the
 // client's TLS configuration changes from plaintext to TLS
 func (c *Client) ReloadTLSConnections(newConfig *nconfig.TLSConfig) error {
-	c.configLock.Lock()
-	c.config.TLSConfig = newConfig
-	c.configLock.Unlock()
+	var tlsWrap tlsutil.RegionWrapper
+	if newConfig.EnableRPC {
+		tw, err := c.config.NewTLSConfiguration(newConfig).OutgoingTLSWrapper()
 
-	if c.config.TLSConfig.EnableRPC {
-		tw, err := c.config.TLSConfiguration().OutgoingTLSWrapper()
 		if err != nil {
 			return err
 		}
-		c.connPool.ReloadTLS(tw)
-	} else {
-		c.connPool.ReloadTLS(nil)
+		tlsWrap = tw
 	}
 
-	time.Sleep(3 * time.Second)
+	c.connPool.ReloadTLS(tlsWrap)
+
+	c.configLock.Lock()
+	c.config.TLSConfig = newConfig
+	c.configLock.Unlock()
 
 	return nil
 }

diff --git a/client/config/config.go b/client/config/config.go
@@ -347,16 +347,23 @@ func (c *Config) ReadStringListToMapDefault(key, defaultValue string) map[string
 	return list
 }
 
-// TLSConfig returns a TLSUtil Config based on the client configuration
+// TLSConfiguration returns a TLSUtil Config based on the existing client
+// configuration
 func (c *Config) TLSConfiguration() *tlsutil.Config {
-	tlsConf := &tlsutil.Config{
+	return c.NewTLSConfiguration(c.TLSConfig)
+}
+
+// NewTLSConfiguration returns a TLSUtil Config for a new TLS config object
+// This allows a TLSConfig object to be created without first explicitely
+// setting it
+func (c *Config) NewTLSConfiguration(tlsConfig *config.TLSConfig) *tlsutil.Config {
+	return &tlsutil.Config{
 		VerifyIncoming:       true,
 		VerifyOutgoing:       true,
-		VerifyServerHostname: c.TLSConfig.VerifyServerHostname,
-		CAFile:               c.TLSConfig.CAFile,
-		CertFile:             c.TLSConfig.CertFile,
-		KeyFile:              c.TLSConfig.KeyFile,
-		KeyLoader:            c.TLSConfig.GetKeyLoader(),
+		VerifyServerHostname: tlsConfig.VerifyServerHostname,
+		CAFile:               tlsConfig.CAFile,
+		CertFile:             tlsConfig.CertFile,
+		KeyFile:              tlsConfig.KeyFile,
+		KeyLoader:            tlsConfig.GetKeyLoader(),
 	}
-	return tlsConf
 }
diff --git a/nomad/config.go b/nomad/config.go
@@ -348,13 +348,20 @@ func DefaultConfig() *Config {
 
 // tlsConfig returns a TLSUtil Config based on the server configuration
 func (c *Config) tlsConfig() *tlsutil.Config {
+	return c.newTLSConfig(c.TLSConfig)
+}
+
+// newTLSConfig  returns a TLSUtil Config based on the server configuration
+// This is useful for creating a TLSConfig object without explictely setting it
+// on the config.
+func (c *Config) newTLSConfig(newConf *config.TLSConfig) *tlsutil.Config {
 	return &tlsutil.Config{
 		VerifyIncoming:       true,
 		VerifyOutgoing:       true,
-		VerifyServerHostname: c.TLSConfig.VerifyServerHostname,
-		CAFile:               c.TLSConfig.CAFile,
-		CertFile:             c.TLSConfig.CertFile,
-		KeyFile:              c.TLSConfig.KeyFile,
-		KeyLoader:            c.TLSConfig.GetKeyLoader(),
+		VerifyServerHostname: newConf.VerifyServerHostname,
+		CAFile:               newConf.CAFile,
+		CertFile:             newConf.CertFile,
+		KeyFile:              newConf.KeyFile,
+		KeyLoader:            newConf.GetKeyLoader(),
 	}
 }
diff --git a/nomad/server.go b/nomad/server.go
@@ -379,12 +379,8 @@ func getTLSConf(enableRPC bool, tlsConf *tlsutil.Config) (*tls.Config, tlsutil.R
 func (s *Server) ReloadTLSConnections(newTLSConfig *config.TLSConfig) error {
 	s.logger.Printf("[INFO] nomad: reloading server connections due to configuration changes")
 
-	s.configLock.Lock()
-	s.config.TLSConfig = newTLSConfig
-	s.configLock.Unlock()
-
-	tlsConf := s.config.tlsConfig()
-	incomingTLS, tlsWrap, err := getTLSConf(s.config.TLSConfig.EnableRPC, tlsConf)
+	tlsConf := s.config.newTLSConfig(newTLSConfig)
+	incomingTLS, tlsWrap, err := getTLSConf(newTLSConfig.EnableRPC, tlsConf)
 	if err != nil {
 		s.logger.Printf("[ERR] nomad: unable to reset TLS context")
 		return err
@@ -397,6 +393,10 @@ func (s *Server) ReloadTLSConnections(newTLSConfig *config.TLSConfig) error {
 
 	s.rpcCancel()
 
+	s.configLock.Lock()
+	s.config.TLSConfig = newTLSConfig
+	s.configLock.Unlock()
+
 	s.rpcTLSLock.Lock()
 	s.rpcTLS = incomingTLS
 	s.rpcTLSLock.Unlock()