Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configurable TLS cipher suites and versions; disallow weak ciphers #4269

Merged
merged 4 commits into from
May 11, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
11 changes: 11 additions & 0 deletions command/agent/config_parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -760,6 +761,8 @@ func parseTLSConfig(result **config.TLSConfig, list *ast.ObjectList) error {
"cert_file",
"key_file",
"verify_https_client",
"tls_cipher_suites",
"tls_min_version",
}

if err := helper.CheckHCLKeys(listVal, valid); err != nil {
Expand All @@ -776,6 +779,14 @@ func parseTLSConfig(result **config.TLSConfig, list *ast.ObjectList) error {
return err
}

if _, err := tlsutil.ParseCiphers(tlsConfig.TLSCipherSuites); err != nil {
return err
}

if _, err := tlsutil.ParseMinVersion(tlsConfig.TLSMinVersion); err != nil {
return err
}

*result = &tlsConfig
return nil
}
Expand Down
105 changes: 101 additions & 4 deletions helper/tlsutil/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,46 @@ import (
"fmt"
"io/ioutil"
"net"
"strings"
"time"

"github.com/hashicorp/nomad/nomad/structs/config"
)

// supportedTLSVersions are the current TLS versions that Nomad supports
var supportedTLSVersions = map[string]uint16{
"tls10": tls.VersionTLS10,
"tls11": tls.VersionTLS11,
"tls12": tls.VersionTLS12,
}

// supportedTLSCiphers are the complete list of TLS ciphers supported by Nomad
var supportedTLSCiphers = 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,
}

// defaultTLSCiphers are the TLS Ciphers that are supported by default
var defaultTLSCiphers = []string{"TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305",
"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
"TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384",
}

// RegionSpecificWrapper is used to invoke a static Region and turns a
// RegionWrapper into a Wrapper type.
func RegionSpecificWrapper(region string, tlsWrap RegionWrapper) Wrapper {
Expand Down Expand Up @@ -65,9 +100,26 @@ 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

// MinVersion contains the minimum SSL/TLS version that is accepted.
MinVersion 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
}

minVersion, err := ParseMinVersion(newConf.TLSMinVersion)
if err != nil {
return nil, err
}

return &Config{
VerifyIncoming: true,
VerifyOutgoing: true,
Expand All @@ -76,7 +128,9 @@ func NewTLSConfiguration(newConf *config.TLSConfig) *Config {
CertFile: newConf.CertFile,
KeyFile: newConf.KeyFile,
KeyLoader: newConf.GetKeyLoader(),
}
CipherSuites: ciphers,
MinVersion: minVersion,
}, nil
}

// AppendCA opens and parses the CA file and adds the certificates to
Expand Down Expand Up @@ -132,6 +186,8 @@ func (c *Config) OutgoingTLSConfig() (*tls.Config, error) {
tlsConfig := &tls.Config{
RootCAs: x509.NewCertPool(),
InsecureSkipVerify: true,
CipherSuites: c.CipherSuites,
MinVersion: c.MinVersion,
}
if c.VerifyServerHostname {
tlsConfig.InsecureSkipVerify = false
Expand Down Expand Up @@ -248,8 +304,10 @@ 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,
MinVersion: c.MinVersion,
}

// Parse the CA cert if any
Expand Down Expand Up @@ -279,3 +337,42 @@ func (c *Config) IncomingTLSConfig() (*tls.Config, error) {

return tlsConfig, nil
}

// ParseCiphers parses 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 == "" {
ciphers = defaultTLSCiphers

} else {
ciphers = strings.Split(cipherStr, ",")
}
for _, cipher := range ciphers {
c, ok := supportedTLSCiphers[cipher]
if !ok {
return suites, fmt.Errorf("unsupported TLS cipher %q", cipher)
}
suites = append(suites, c)
}

return suites, nil
}

// ParseMinVersion parses the specified minimum TLS version for the Nomad agent
func ParseMinVersion(version string) (uint16, error) {
if version == "" {
return supportedTLSVersions["tls12"], nil
}

vers, ok := supportedTLSVersions[version]
if !ok {
return 0, fmt.Errorf("unsupported TLS version %q", version)
}

return vers, nil
}
119 changes: 119 additions & 0 deletions helper/tlsutil/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -412,3 +415,119 @@ 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 TLS cipher %q", cipher), err.Error())
require.Equal(0, len(parsedCiphers))
}
}

func TestConfig_ParseMinVersion_Valid(t *testing.T) {
require := require.New(t)

validVersions := []string{"tls10",
"tls11",
"tls12",
}

expected := map[string]uint16{
"tls10": tls.VersionTLS10,
"tls11": tls.VersionTLS11,
"tls12": tls.VersionTLS12,
}

for _, version := range validVersions {
parsedVersion, err := ParseMinVersion(version)
require.Nil(err)
require.Equal(expected[version], parsedVersion)
}
}

func TestConfig_ParseMinVersion_Invalid(t *testing.T) {
require := require.New(t)

invalidVersions := []string{"tls13",
"tls15",
}

for _, version := range invalidVersions {
parsedVersion, err := ParseMinVersion(version)
require.NotNil(err)
require.Equal(fmt.Sprintf("unsupported TLS version %q", version), err.Error())
require.Equal(uint16(0), parsedVersion)
}
}
6 changes: 5 additions & 1 deletion nomad/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the error is logged by the caller we should log here

}

incomingTLS, tlsWrap, err := getTLSConf(newTLSConfig.EnableRPC, tlsConf)
if err != nil {
s.logger.Printf("[ERR] nomad: unable to reset TLS context %s", err)
Expand Down
11 changes: 11 additions & 0 deletions nomad/structs/config/tls.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,14 @@ 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"`

// TLSMinVersion is used to set the minimum TLS version used for TLS
// connections. Should be either "tls10", "tls11", or "tls12".
TLSMinVersion string `mapstructure:"tls_min_version"`
}

type KeyLoader struct {
Expand Down Expand Up @@ -147,6 +155,9 @@ func (t *TLSConfig) Copy() *TLSConfig {
new.RPCUpgradeMode = t.RPCUpgradeMode
new.VerifyHTTPSClient = t.VerifyHTTPSClient

new.TLSCipherSuites = t.TLSCipherSuites
new.TLSMinVersion = t.TLSMinVersion

new.SetChecksum()

return new
Expand Down
7 changes: 4 additions & 3 deletions nomad/structs/config/tls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
Loading