Merge pull request #4328 from hashicorp/r-single-tls-config-constructor

Refactor to prefer using NewTLSConfiguration constructor
hashicorp · May 24, 2018 · c42a07b · c42a07b
2 parents 5803932 + 6733d76
commit c42a07b
Show file tree

Hide file tree

Showing 9 changed files with 45 additions and 45 deletions.
diff --git a/client/client.go b/client/client.go
@@ -197,11 +197,14 @@ func NewClient(cfg *config.Config, consulCatalog consul.CatalogAPI, consulServic
 	// Create the tls wrapper
 	var tlsWrap tlsutil.RegionWrapper
 	if cfg.TLSConfig.EnableRPC {
-		tw, err := cfg.TLSConfiguration().OutgoingTLSWrapper()
+		tw, err := tlsutil.NewTLSConfiguration(cfg.TLSConfig, true, true)
+		if err != nil {
+			return nil, err
+		}
+		tlsWrap, err = tw.OutgoingTLSWrapper()
 		if err != nil {
 			return nil, err
 		}
-		tlsWrap = tw
 	}
 
 	// Create the client
@@ -399,7 +402,7 @@ func (c *Client) init() error {
 func (c *Client) reloadTLSConnections(newConfig *nconfig.TLSConfig) error {
 	var tlsWrap tlsutil.RegionWrapper
 	if newConfig != nil && newConfig.EnableRPC {
-		tw, err := tlsutil.NewTLSConfiguration(newConfig)
+		tw, err := tlsutil.NewTLSConfiguration(newConfig, true, true)
 		if err != nil {
 			return err
 		}

diff --git a/client/config/config.go b/client/config/config.go
@@ -9,7 +9,6 @@ import (
 	"time"
 
 	"github.com/hashicorp/nomad/helper"
-	"github.com/hashicorp/nomad/helper/tlsutil"
 	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/hashicorp/nomad/nomad/structs/config"
 	"github.com/hashicorp/nomad/version"
@@ -358,17 +357,3 @@ func (c *Config) ReadStringListToMapDefault(key, defaultValue string) map[string
 	}
 	return list
 }
-
-// TLSConfiguration returns a TLSUtil Config based on the existing client
-// configuration
-func (c *Config) TLSConfiguration() *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(),
-	}
-}
diff --git a/command/agent/http.go b/command/agent/http.go
@@ -70,15 +70,11 @@ func NewHTTPServer(agent *Agent, config *Config) (*HTTPServer, error) {
 
 	// If TLS is enabled, wrap the listener with a TLS listener
 	if config.TLSConfig.EnableHTTP {
-		tlsConf := &tlsutil.Config{
-			VerifyIncoming:       config.TLSConfig.VerifyHTTPSClient,
-			VerifyOutgoing:       true,
-			VerifyServerHostname: config.TLSConfig.VerifyServerHostname,
-			CAFile:               config.TLSConfig.CAFile,
-			CertFile:             config.TLSConfig.CertFile,
-			KeyFile:              config.TLSConfig.KeyFile,
-			KeyLoader:            config.TLSConfig.GetKeyLoader(),
+		tlsConf, err := tlsutil.NewTLSConfiguration(config.TLSConfig, config.TLSConfig.VerifyHTTPSClient, true)
+		if err != nil {
+			return nil, err
 		}
+
 		tlsConfig, err := tlsConf.IncomingTLSConfig()
 		if err != nil {
 			return nil, err

diff --git a/helper/tlsutil/config.go b/helper/tlsutil/config.go
@@ -109,7 +109,7 @@ type Config struct {
 	MinVersion uint16
 }
 
-func NewTLSConfiguration(newConf *config.TLSConfig) (*Config, error) {
+func NewTLSConfiguration(newConf *config.TLSConfig, verifyIncoming, verifyOutgoing bool) (*Config, error) {
 	ciphers, err := ParseCiphers(newConf.TLSCipherSuites)
 	if err != nil {
 		return nil, err
@@ -121,8 +121,8 @@ func NewTLSConfiguration(newConf *config.TLSConfig) (*Config, error) {
 	}
 
 	return &Config{
-		VerifyIncoming:       true,
-		VerifyOutgoing:       true,
+		VerifyIncoming:       verifyIncoming,
+		VerifyOutgoing:       verifyOutgoing,
 		VerifyServerHostname: newConf.VerifyServerHostname,
 		CAFile:               newConf.CAFile,
 		CertFile:             newConf.CertFile,

diff --git a/helper/tlsutil/config_test.go b/helper/tlsutil/config_test.go
@@ -531,3 +531,22 @@ func TestConfig_ParseMinVersion_Invalid(t *testing.T) {
 		require.Equal(uint16(0), parsedVersion)
 	}
 }
+
+func TestConfig_NewTLSConfiguration(t *testing.T) {
+	require := require.New(t)
+
+	conf := &config.TLSConfig{
+		TLSCipherSuites: "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256",
+	}
+
+	tlsConf, err := NewTLSConfiguration(conf, true, true)
+	require.Nil(err)
+	require.True(tlsConf.VerifyIncoming)
+	require.True(tlsConf.VerifyOutgoing)
+
+	expectedCiphers := []uint16{
+		tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,
+		tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
+	}
+	require.Equal(tlsConf.CipherSuites, expectedCiphers)
+}
diff --git a/nomad/config.go b/nomad/config.go
@@ -9,7 +9,6 @@ import (
 	"time"
 
 	"github.com/hashicorp/memberlist"
-	"github.com/hashicorp/nomad/helper/tlsutil"
 	"github.com/hashicorp/nomad/helper/uuid"
 	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/hashicorp/nomad/nomad/structs/config"
@@ -388,16 +387,3 @@ func DefaultConfig() *Config {
 
 	return c
 }
-
-// tlsConfig returns a TLSUtil Config based on the server configuration
-func (c *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(),
-	}
-}
diff --git a/nomad/server.go b/nomad/server.go
@@ -273,7 +273,10 @@ func NewServer(config *Config, consulCatalog consul.CatalogAPI, logger *log.Logg
 	}
 
 	// Configure TLS
-	tlsConf := config.tlsConfig()
+	tlsConf, err := tlsutil.NewTLSConfiguration(config.TLSConfig, true, true)
+	if err != nil {
+		return nil, err
+	}
 	incomingTLS, tlsWrap, err := getTLSConf(config.TLSConfig.EnableRPC, tlsConf)
 	if err != nil {
 		return nil, err
@@ -450,7 +453,7 @@ func (s *Server) reloadTLSConnections(newTLSConfig *config.TLSConfig) error {
 		return fmt.Errorf("can't reload uninitialized RPC listener")
 	}
 
-	tlsConf, err := tlsutil.NewTLSConfiguration(newTLSConfig)
+	tlsConf, err := tlsutil.NewTLSConfiguration(newTLSConfig, true, true)
 	if err != nil {
 		s.logger.Printf("[ERR] nomad: unable to create TLS configuration %s", err)
 		return err

diff --git a/nomad/structs/config/tls.go b/nomad/structs/config/tls.go
@@ -205,6 +205,12 @@ func (t *TLSConfig) Merge(b *TLSConfig) *TLSConfig {
 	if b.RPCUpgradeMode {
 		result.RPCUpgradeMode = true
 	}
+	if b.TLSCipherSuites != "" {
+		result.TLSCipherSuites = b.TLSCipherSuites
+	}
+	if b.TLSMinVersion != "" {
+		result.TLSMinVersion = b.TLSMinVersion
+	}
 	return result
 }
 

diff --git a/nomad/structs/config/tls_test.go b/nomad/structs/config/tls_test.go
@@ -21,6 +21,8 @@ func TestTLSConfig_Merge(t *testing.T) {
 		CAFile:               "test-ca-file-2",
 		CertFile:             "test-cert-file-2",
 		RPCUpgradeMode:       true,
+		TLSCipherSuites:      "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
+		TLSMinVersion:        "tls12",
 	}
 
 	new := a.Merge(b)