allow configurable cipher suites

chelseakomlo · chelseakomlo · commit eb587d67e030 · 2018-05-08T19:07:52.000-04:00
disallow 3DES and RC4 ciphers

add documentation for tls_cipher_suites
diff --git a/client/client.go b/client/client.go
@@ -399,11 +399,16 @@ 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).OutgoingTLSWrapper()
+		tw, err := tlsutil.NewTLSConfiguration(newConfig)
 		if err != nil {
 			return err
 		}
-		tlsWrap = tw
+
+		twWrap, err := tw.OutgoingTLSWrapper()
+		if err != nil {
+			return err
+		}
+		tlsWrap = twWrap
 	}
 
 	// Store the new tls wrapper.
diff --git a/command/agent/config_parse.go b/command/agent/config_parse.go
@@ -13,6 +13,7 @@ import (
 	"github.com/hashicorp/hcl"
 	"github.com/hashicorp/hcl/hcl/ast"
 	"github.com/hashicorp/nomad/helper"
+	"github.com/hashicorp/nomad/helper/tlsutil"
 	"github.com/hashicorp/nomad/nomad/structs/config"
 	"github.com/mitchellh/mapstructure"
 )
@@ -760,6 +761,7 @@ func parseTLSConfig(result **config.TLSConfig, list *ast.ObjectList) error {
 		"cert_file",
 		"key_file",
 		"verify_https_client",
+		"tls_cipher_suites",
 	}
 
 	if err := helper.CheckHCLKeys(listVal, valid); err != nil {
@@ -776,6 +778,10 @@ func parseTLSConfig(result **config.TLSConfig, list *ast.ObjectList) error {
 		return err
 	}
 
+	if _, err := tlsutil.ParseCiphers(tlsConfig.TLSCipherSuites); err != nil {
+		return err
+	}
+
 	*result = &tlsConfig
 	return nil
 }
diff --git a/helper/tlsutil/config.go b/helper/tlsutil/config.go
@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net"
+	"strings"
 	"time"
 
 	"github.com/hashicorp/nomad/nomad/structs/config"
@@ -65,9 +66,18 @@ type Config struct {
 
 	// KeyLoader dynamically reloads TLS configuration.
 	KeyLoader *config.KeyLoader
+
+	// CipherSuites have a default safe configuration, or operators can override
+	// these values for acceptable safe alternatives.
+	CipherSuites []uint16
 }
 
-func NewTLSConfiguration(newConf *config.TLSConfig) *Config {
+func NewTLSConfiguration(newConf *config.TLSConfig) (*Config, error) {
+	ciphers, err := ParseCiphers(newConf.TLSCipherSuites)
+	if err != nil {
+		return nil, err
+	}
+
 	return &Config{
 		VerifyIncoming:       true,
 		VerifyOutgoing:       true,
@@ -76,7 +86,8 @@ func NewTLSConfiguration(newConf *config.TLSConfig) *Config {
 		CertFile:             newConf.CertFile,
 		KeyFile:              newConf.KeyFile,
 		KeyLoader:            newConf.GetKeyLoader(),
-	}
+		CipherSuites:         ciphers,
+	}, nil
 }
 
 // AppendCA opens and parses the CA file and adds the certificates to
@@ -132,6 +143,7 @@ func (c *Config) OutgoingTLSConfig() (*tls.Config, error) {
 	tlsConfig := &tls.Config{
 		RootCAs:            x509.NewCertPool(),
 		InsecureSkipVerify: true,
+		CipherSuites:       c.CipherSuites,
 	}
 	if c.VerifyServerHostname {
 		tlsConfig.InsecureSkipVerify = false
@@ -248,8 +260,9 @@ func WrapTLSClient(conn net.Conn, tlsConfig *tls.Config) (net.Conn, error) {
 func (c *Config) IncomingTLSConfig() (*tls.Config, error) {
 	// Create the tlsConfig
 	tlsConfig := &tls.Config{
-		ClientCAs:  x509.NewCertPool(),
-		ClientAuth: tls.NoClientCert,
+		ClientCAs:    x509.NewCertPool(),
+		ClientAuth:   tls.NoClientCert,
+		CipherSuites: c.CipherSuites,
 	}
 
 	// Parse the CA cert if any
@@ -279,3 +292,51 @@ func (c *Config) IncomingTLSConfig() (*tls.Config, error) {
 
 	return tlsConfig, nil
 }
+
+// ParseCiphers parse ciphersuites from the comma-separated string into recognized slice
+func ParseCiphers(cipherStr string) ([]uint16, error) {
+	suites := []uint16{}
+
+	cipherStr = strings.TrimSpace(cipherStr)
+
+	var ciphers []string
+	if cipherStr == "" {
+		// Set good  default values
+		ciphers = []string{"TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305",
+			"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
+			"TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384",
+		}
+
+	} else {
+		ciphers = strings.Split(cipherStr, ",")
+	}
+
+	cipherMap := map[string]uint16{
+		"TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305":    tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,
+		"TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305":  tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305,
+		"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256":   tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
+		"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256": tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
+		"TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384":   tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
+		"TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384": tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
+		"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256":   tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256,
+		"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA":      tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA,
+		"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256": tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256,
+		"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA":    tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,
+		"TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA":      tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA,
+		"TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA":    tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA,
+		"TLS_RSA_WITH_AES_128_GCM_SHA256":         tls.TLS_RSA_WITH_AES_128_GCM_SHA256,
+		"TLS_RSA_WITH_AES_256_GCM_SHA384":         tls.TLS_RSA_WITH_AES_256_GCM_SHA384,
+		"TLS_RSA_WITH_AES_128_CBC_SHA256":         tls.TLS_RSA_WITH_AES_128_CBC_SHA256,
+		"TLS_RSA_WITH_AES_128_CBC_SHA":            tls.TLS_RSA_WITH_AES_128_CBC_SHA,
+		"TLS_RSA_WITH_AES_256_CBC_SHA":            tls.TLS_RSA_WITH_AES_256_CBC_SHA,
+	}
+	for _, cipher := range ciphers {
+		if v, ok := cipherMap[cipher]; ok {
+			suites = append(suites, v)
+		} else {
+			return suites, fmt.Errorf("unsupported cipher %q", cipher)
+		}
+	}
+
+	return suites, nil
+}
diff --git a/helper/tlsutil/config_test.go b/helper/tlsutil/config_test.go
@@ -3,14 +3,17 @@ package tlsutil
 import (
 	"crypto/tls"
 	"crypto/x509"
+	"fmt"
 	"io"
 	"io/ioutil"
 	"net"
+	"strings"
 	"testing"
 
 	"github.com/hashicorp/nomad/nomad/structs/config"
 	"github.com/hashicorp/yamux"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 const (
@@ -412,3 +415,83 @@ func TestConfig_IncomingTLS_NoVerify(t *testing.T) {
 		t.Fatalf("unexpected client cert")
 	}
 }
+
+func TestConfig_ParseCiphers_Valid(t *testing.T) {
+	require := require.New(t)
+
+	validCiphers := strings.Join([]string{
+		"TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305",
+		"TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305",
+		"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256",
+		"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
+		"TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384",
+		"TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384",
+		"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256",
+		"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA",
+		"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256",
+		"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA",
+		"TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA",
+		"TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA",
+		"TLS_RSA_WITH_AES_128_GCM_SHA256",
+		"TLS_RSA_WITH_AES_256_GCM_SHA384",
+		"TLS_RSA_WITH_AES_128_CBC_SHA256",
+		"TLS_RSA_WITH_AES_128_CBC_SHA",
+		"TLS_RSA_WITH_AES_256_CBC_SHA",
+	}, ",")
+
+	expectedCiphers := []uint16{
+		tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,
+		tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305,
+		tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
+		tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
+		tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
+		tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
+		tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256,
+		tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA,
+		tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256,
+		tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,
+		tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA,
+		tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA,
+		tls.TLS_RSA_WITH_AES_128_GCM_SHA256,
+		tls.TLS_RSA_WITH_AES_256_GCM_SHA384,
+		tls.TLS_RSA_WITH_AES_128_CBC_SHA256,
+		tls.TLS_RSA_WITH_AES_128_CBC_SHA,
+		tls.TLS_RSA_WITH_AES_256_CBC_SHA,
+	}
+
+	parsedCiphers, err := ParseCiphers(validCiphers)
+	require.Nil(err)
+	require.Equal(parsedCiphers, expectedCiphers)
+}
+
+func TestConfig_ParseCiphers_Default(t *testing.T) {
+	require := require.New(t)
+
+	expectedCiphers := []uint16{
+		tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305,
+		tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
+		tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
+	}
+
+	parsedCiphers, err := ParseCiphers("")
+	require.Nil(err)
+	require.Equal(parsedCiphers, expectedCiphers)
+}
+
+func TestConfig_ParseCiphers_Invalid(t *testing.T) {
+	require := require.New(t)
+
+	invalidCiphers := []string{"TLS_RSA_WITH_3DES_EDE_CBC_SHA",
+		"TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA",
+		"TLS_RSA_WITH_RC4_128_SHA",
+		"TLS_ECDHE_RSA_WITH_RC4_128_SHA",
+		"TLS_ECDHE_ECDSA_WITH_RC4_128_SHA",
+	}
+
+	for _, cipher := range invalidCiphers {
+		parsedCiphers, err := ParseCiphers(cipher)
+		require.NotNil(err)
+		require.Equal(fmt.Sprintf("unsupported cipher %q", cipher), err.Error())
+		require.Equal(0, len(parsedCiphers))
+	}
+}
diff --git a/nomad/server.go b/nomad/server.go
@@ -450,7 +450,11 @@ func (s *Server) reloadTLSConnections(newTLSConfig *config.TLSConfig) error {
 		return fmt.Errorf("can't reload uninitialized RPC listener")
 	}
 
-	tlsConf := tlsutil.NewTLSConfiguration(newTLSConfig)
+	tlsConf, err := tlsutil.NewTLSConfiguration(newTLSConfig)
+	if err != nil {
+		return err
+	}
+
 	incomingTLS, tlsWrap, err := getTLSConf(newTLSConfig.EnableRPC, tlsConf)
 	if err != nil {
 		s.logger.Printf("[ERR] nomad: unable to reset TLS context %s", err)
diff --git a/nomad/structs/config/tls.go b/nomad/structs/config/tls.go
@@ -55,6 +55,10 @@ type TLSConfig struct {
 	// Checksum is a MD5 hash of the certificate CA File, Certificate file, and
 	// key file.
 	Checksum string
+
+	// TLSCipherSuites are operator-defined ciphers to be used in Nomad TLS
+	// connections
+	TLSCipherSuites string `mapstructure:"tls_cipher_suites"`
 }
 
 type KeyLoader struct {
diff --git a/nomad/structs/config/tls_test.go b/nomad/structs/config/tls_test.go
@@ -171,9 +171,10 @@ func TestTLS_Copy(t *testing.T) {
 		fookey  = "../../../helper/tlsutil/testdata/nomad-foo-key.pem"
 	)
 	a := &TLSConfig{
-		CAFile:   cafile,
-		CertFile: foocert,
-		KeyFile:  fookey,
+		CAFile:          cafile,
+		CertFile:        foocert,
+		KeyFile:         fookey,
+		TLSCipherSuites: "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305",
 	}
 	a.SetChecksum()
 
diff --git a/website/source/docs/agent/configuration/tls.html.md b/website/source/docs/agent/configuration/tls.html.md
@@ -58,6 +58,12 @@ the [Agent's Gossip and RPC Encryption](/docs/agent/encryption.html).
   cluster is being upgraded to TLS, and removed after the migration is
   complete. This allows the agent to accept both TLS and plaintext traffic.
 
+- `tls_cipher_suites` - Specifies the TLS cipher suites that will be used by
+  the agent. Known insecure ciphers are disabled (3DES and RC4). By default,
+  an agent is configured to use TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305,
+  TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, and
+  TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384.
+
 - `verify_https_client` `(bool: false)` - Specifies agents should require
   client certificates for all incoming HTTPS requests. The client certificates
   must be signed by the same CA as Nomad.

Original file line number	Diff line number	Diff line change
`@@ -13,6 +13,7 @@ import (`
`13`	`13`	`"github.com/hashicorp/hcl"`
`14`	`14`	`"github.com/hashicorp/hcl/hcl/ast"`
`15`	`15`	`"github.com/hashicorp/nomad/helper"`
	`16`	`+ "github.com/hashicorp/nomad/helper/tlsutil"`
`16`	`17`	`"github.com/hashicorp/nomad/nomad/structs/config"`
`17`	`18`	`"github.com/mitchellh/mapstructure"`
`18`	`19`	`)`
`@@ -760,6 +761,7 @@ func parseTLSConfig(result *config.TLSConfig, list ast.ObjectList) error {`
`760`	`761`	`"cert_file",`
`761`	`762`	`"key_file",`
`762`	`763`	`"verify_https_client",`
	`764`	`+ "tls_cipher_suites",`
`763`	`765`	`}`
`764`	`766`
`765`	`767`	`if err := helper.CheckHCLKeys(listVal, valid); err != nil {`
`@@ -776,6 +778,10 @@ func parseTLSConfig(result *config.TLSConfig, list ast.ObjectList) error {`
`776`	`778`	`return err`
`777`	`779`	`}`
`778`	`780`
	`781`	`+ if _, err := tlsutil.ParseCiphers(tlsConfig.TLSCipherSuites); err != nil {`
	`782`	`+ return err`
	`783`	`+ }`
	`784`	`+`
`779`	`785`	`*result = &tlsConfig`
`780`	`786`	`return nil`
`781`	`787`	`}`
Original file line number	Diff line number	Diff line change
`@@ -171,9 +171,10 @@ func TestTLS_Copy(t *testing.T) {`
`171`	`171`	`fookey = "../../../helper/tlsutil/testdata/nomad-foo-key.pem"`
`172`	`172`	`)`
`173`	`173`	`a := &TLSConfig{`
`174`		`- CAFile: cafile,`
`175`		`- CertFile: foocert,`
`176`		`- KeyFile: fookey,`
	`174`	`+ CAFile: cafile,`
	`175`	`+ CertFile: foocert,`
	`176`	`+ KeyFile: fookey,`
	`177`	`+ TLSCipherSuites: "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305",`
`177`	`178`	`}`
`178`	`179`	`a.SetChecksum()`
`179`	`180`