From 0bf2f50bc9e17aea39c1341476eafd9eaaf4727a Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Tue, 8 May 2018 16:32:07 -0400 Subject: [PATCH] allow configurable cipher suites disallow 3DES and RC4 ciphers add documentation for tls_cipher_suites --- client/client.go | 9 +- command/agent/config_parse.go | 6 ++ helper/tlsutil/config.go | 69 ++++++++++++++- helper/tlsutil/config_test.go | 83 +++++++++++++++++++ nomad/server.go | 6 +- nomad/structs/config/tls.go | 4 + nomad/structs/config/tls_test.go | 3 + .../docs/agent/configuration/tls.html.md | 6 ++ 8 files changed, 179 insertions(+), 7 deletions(-) diff --git a/client/client.go b/client/client.go index a3e8dad6b6dd..2db927e6d73e 100644 --- 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 index 5239e8cd511e..37ea60dc609b 100644 --- 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 index 8f6b1f01df3e..dfad04b4bef8 100644 --- 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 index ffc3f54ec6c1..6b3ec95a6dc2 100644 --- 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 index 235e2988a3df..0aac5c60397c 100644 --- 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 index f40866c015b3..eb5300123ec1 100644 --- 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 index 698855e08083..b3c40d87c6f6 100644 --- a/nomad/structs/config/tls_test.go +++ b/nomad/structs/config/tls_test.go @@ -174,6 +174,9 @@ func TestTLS_Copy(t *testing.T) { CAFile: cafile, CertFile: foocert, KeyFile: fookey, + CipherSuites: []string{ + "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 index b816f7f21ce0..bb62c2bbb112 100644 --- 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.