From 8020fb2098f7eeed58a84f2f8369ced2bf5396bb Mon Sep 17 00:00:00 2001 From: Mike Morris Date: Thu, 24 Mar 2022 15:32:25 -0400 Subject: [PATCH] agent: convert listener config to TLS types (#12522) * tlsutil: initial implementation of types/TLSVersion tlsutil: add test for parsing deprecated agent TLS version strings tlsutil: return TLSVersionInvalid with error tlsutil: start moving tlsutil cipher suite lookups over to types/tls tlsutil: rename tlsLookup to ParseTLSVersion, add cipherSuiteLookup agent: attempt to use types in runtime config agent: implement b.tlsVersion validation in config builder agent: fix tlsVersion nil check in builder tlsutil: update to renamed ParseTLSVersion and goTLSVersions tlsutil: fixup TestConfigurator_CommonTLSConfigTLSMinVersion tlsutil: disable invalid config parsing tests tlsutil: update tests auto_config: lookup old config strings from base.TLSMinVersion auto_config: update endpoint tests to use TLS types agent: update runtime_test to use TLS types agent: update TestRuntimeCinfig_Sanitize.golden agent: update config runtime tests to expect TLS types * website: update Consul agent tls_min_version values * agent: fixup TLS parsing and compilation errors * test: fixup lint issues in agent/config_runtime_test and tlsutil/config_test * tlsutil: add CHACHA20_POLY1305 cipher suites to goTLSCipherSuites * test: revert autoconfig tls min version fixtures to old format * types: add TLSVersions public function * agent: add warning for deprecated TLS version strings * agent: move agent config specific logic from tlsutil.ParseTLSVersion into agent config builder * tlsutil(BREAKING): change default TLS min version to TLS 1.2 * agent: move ParseCiphers logic from tlsutil into agent config builder * tlsutil: remove unused CipherString function * agent: fixup import for types package * Revert "tlsutil: remove unused CipherString function" This reverts commit 6ca7f6f58d268e617501b7db9500113c13bae70c. * agent: fixup config builder and runtime tests * tlsutil: fixup one remaining ListenerConfig -> ProtocolConfig * test: move TLS cipher suites parsing test from tlsutil into agent config builder tests * agent: remove parseCiphers helper from auto_config_endpoint_test * test: remove unused imports from tlsutil * agent: remove resolved FIXME comment * tlsutil: remove TODO and FIXME in cipher suite validation * agent: prevent setting inherited cipher suite config when TLS 1.3 is specified * changelog: add entry for converting agent config to TLS types * agent: remove FIXME in runtime test, this is covered in builder tests with invalid tls9 value now * tlsutil: remove config tests for values checked at agent config builder boundary * tlsutil: remove tls version check from loadProtocolConfig * tlsutil: remove tests and TODOs for logic checked in TestBuilder_tlsVersion and TestBuilder_tlsCipherSuites * website: update search link for supported Consul agent cipher suites * website: apply review suggestions for tls_min_version description * website: attempt to clean up markdown list formatting for tls_min_version * website: moar linebreaks to fix tls_min_version formatting * Revert "website: moar linebreaks to fix tls_min_version formatting" This reverts commit 38585927422f73ebf838a7663e566ac245f2a75c. * autoconfig: translate old values for TLSMinVersion * agent: rename var for translated value of deprecated TLS version value * Update agent/config/deprecated.go Co-authored-by: Dan Upton * agent: fix lint issue * agent: fixup deprecated config test assertions for updated warning Co-authored-by: Dan Upton --- .changelog/12522.txt | 15 ++ agent/auto-config/config_translate.go | 13 +- agent/auto-config/config_translate_test.go | 2 +- agent/config/builder.go | 76 ++++++++- agent/config/builder_test.go | 57 +++++++ agent/config/default.go | 2 +- agent/config/deprecated.go | 13 +- agent/config/deprecated_test.go | 12 +- agent/config/runtime_test.go | 35 ++-- .../TestRuntimeConfig_Sanitize.golden | 2 +- agent/config/testdata/full-config.hcl | 19 ++- agent/config/testdata/full-config.json | 19 ++- agent/consul/auto_config_endpoint.go | 1 + agent/consul/auto_config_endpoint_test.go | 26 ++- tlsutil/config.go | 151 +++++++----------- tlsutil/config_test.go | 79 ++------- types/tls.go | 13 +- website/content/docs/agent/options.mdx | 25 ++- 18 files changed, 318 insertions(+), 242 deletions(-) create mode 100644 .changelog/12522.txt diff --git a/.changelog/12522.txt b/.changelog/12522.txt new file mode 100644 index 000000000..a9d8ec5fc --- /dev/null +++ b/.changelog/12522.txt @@ -0,0 +1,15 @@ +```release-note:deprecation +agent: deprecate older syntax for specifying TLS min version values +``` +```release-note:deprecation +agent: remove support for specifying insecure TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 and TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 cipher suites +``` +```release-note:enhancement +agent: add additional validation to TLS config +``` +```release-note:enhancement +agent: bump default min version for connections to TLS 1.2 +``` +```release-note:enhancement +agent: add support for specifying TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256 and TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256 cipher suites +``` diff --git a/agent/auto-config/config_translate.go b/agent/auto-config/config_translate.go index 4b7f7e663..c5c3606a9 100644 --- a/agent/auto-config/config_translate.go +++ b/agent/auto-config/config_translate.go @@ -6,6 +6,7 @@ import ( "github.com/hashicorp/consul/proto/pbautoconf" "github.com/hashicorp/consul/proto/pbconfig" "github.com/hashicorp/consul/proto/pbconnect" + "github.com/hashicorp/consul/types" ) // translateAgentConfig is meant to take in a proto/pbconfig.Config type @@ -83,9 +84,19 @@ func translateConfig(c *pbconfig.Config) config.Config { if t := c.TLS; t != nil { result.TLS.Defaults = config.TLSProtocolConfig{ VerifyOutgoing: &t.VerifyOutgoing, - TLSMinVersion: stringPtrOrNil(t.MinVersion), TLSCipherSuites: stringPtrOrNil(t.CipherSuites), } + + // NOTE: This inner check for deprecated values should eventually be + // removed, and possibly replaced with a versioning scheme for autoconfig + // or a proper integration with the deprecated config handling in + // agent/config/deprecated.go + if v, ok := types.DeprecatedConsulAgentTLSVersions[t.MinVersion]; ok { + result.TLS.Defaults.TLSMinVersion = stringPtrOrNil(v.String()) + } else { + result.TLS.Defaults.TLSMinVersion = stringPtrOrNil(t.MinVersion) + } + result.TLS.InternalRPC.VerifyServerHostname = &t.VerifyServerHostname } diff --git a/agent/auto-config/config_translate_test.go b/agent/auto-config/config_translate_test.go index ed605672d..e48eebf31 100644 --- a/agent/auto-config/config_translate_test.go +++ b/agent/auto-config/config_translate_test.go @@ -108,7 +108,7 @@ func TestTranslateConfig(t *testing.T) { Defaults: config.TLSProtocolConfig{ VerifyOutgoing: boolPointer(true), TLSCipherSuites: stringPointer("stuff"), - TLSMinVersion: stringPointer("tls13"), + TLSMinVersion: stringPointer("TLSv1_3"), }, InternalRPC: config.TLSProtocolConfig{ VerifyServerHostname: boolPointer(true), diff --git a/agent/config/builder.go b/agent/config/builder.go index 478540db8..d9686254b 100644 --- a/agent/config/builder.go +++ b/agent/config/builder.go @@ -1971,15 +1971,63 @@ func (b *builder) cidrsVal(name string, v []string) (nets []*net.IPNet) { return } -func (b *builder) tlsCipherSuites(name string, v *string) []uint16 { +func (b *builder) tlsVersion(name string, v *string) types.TLSVersion { + // Handles unspecified config and empty string case. + // + // This check is not inside types.ValidateTLSVersionString because Envoy config + // distinguishes between an unset empty string which inherits parent config and + // an explicit TLS_AUTO which allows overriding parent config with the proxy + // defaults. + if v == nil || *v == "" { + return types.TLSVersionAuto + } + + a := types.TLSVersion(*v) + + err := types.ValidateTLSVersion(a) + if err != nil { + b.err = multierror.Append(b.err, fmt.Errorf("%s: invalid TLS version: %s", name, err)) + return types.TLSVersionInvalid + } + return a +} + +// validateTLSVersionCipherSuitesCompat checks that the specified TLS version supports +// specifying cipher suites +func validateTLSVersionCipherSuitesCompat(tlsMinVersion types.TLSVersion) error { + if tlsMinVersion == types.TLSv1_3 { + return fmt.Errorf("TLS 1.3 cipher suites are not configurable") + } + return nil +} + +// tlsCipherSuites parses cipher suites from a comma-separated string into a +// recognized slice +func (b *builder) tlsCipherSuites(name string, v *string, tlsMinVersion types.TLSVersion) []types.TLSCipherSuite { if v == nil { return nil } - var a []uint16 - a, err := tlsutil.ParseCiphers(*v) + if err := validateTLSVersionCipherSuitesCompat(tlsMinVersion); err != nil { + b.err = multierror.Append(b.err, fmt.Errorf("%s: %s", name, err)) + return nil + } + + *v = strings.TrimSpace(*v) + if *v == "" { + return []types.TLSCipherSuite{} + } + ciphers := strings.Split(*v, ",") + + a := make([]types.TLSCipherSuite, len(ciphers)) + for i, cipher := range ciphers { + a[i] = types.TLSCipherSuite(cipher) + } + + err := types.ValidateConsulAgentCipherSuites(a) if err != nil { - b.err = multierror.Append(b.err, fmt.Errorf("%s: invalid tls cipher suites: %s", name, err)) + b.err = multierror.Append(b.err, fmt.Errorf("%s: invalid TLS cipher suites: %s", name, err)) + return []types.TLSCipherSuite{} } return a } @@ -2477,14 +2525,14 @@ func (b *builder) buildTLSConfig(rt RuntimeConfig, t TLS) (tlsutil.Config, error b.warn("tls.grpc was provided but TLS will NOT be enabled on the gRPC listener without an HTTPS listener configured (e.g. via ports.https)") } - defaultCipherSuites := b.tlsCipherSuites("tls.defaults.tls_cipher_suites", t.Defaults.TLSCipherSuites) + defaultTLSMinVersion := b.tlsVersion("tls.defaults.tls_min_version", t.Defaults.TLSMinVersion) + defaultCipherSuites := b.tlsCipherSuites("tls.defaults.tls_cipher_suites", t.Defaults.TLSCipherSuites, defaultTLSMinVersion) mapCommon := func(name string, src TLSProtocolConfig, dst *tlsutil.ProtocolConfig) { dst.CAPath = stringValWithDefault(src.CAPath, stringVal(t.Defaults.CAPath)) dst.CAFile = stringValWithDefault(src.CAFile, stringVal(t.Defaults.CAFile)) dst.CertFile = stringValWithDefault(src.CertFile, stringVal(t.Defaults.CertFile)) dst.KeyFile = stringValWithDefault(src.KeyFile, stringVal(t.Defaults.KeyFile)) - dst.TLSMinVersion = stringValWithDefault(src.TLSMinVersion, stringVal(t.Defaults.TLSMinVersion)) dst.VerifyIncoming = boolValWithDefault(src.VerifyIncoming, boolVal(t.Defaults.VerifyIncoming)) // We prevent this from being set explicity in the tls.grpc stanza above, but @@ -2494,12 +2542,26 @@ func (b *builder) buildTLSConfig(rt RuntimeConfig, t TLS) (tlsutil.Config, error dst.VerifyOutgoing = boolValWithDefault(src.VerifyOutgoing, boolVal(t.Defaults.VerifyOutgoing)) } + if src.TLSMinVersion == nil { + dst.TLSMinVersion = defaultTLSMinVersion + } else { + dst.TLSMinVersion = b.tlsVersion( + fmt.Sprintf("tls.%s.tls_min_version", name), + src.TLSMinVersion, + ) + } + if src.TLSCipherSuites == nil { - dst.CipherSuites = defaultCipherSuites + // If cipher suite config incompatible with a specified TLS min version + // would be inherited, omit it but don't return an error in the builder. + if validateTLSVersionCipherSuitesCompat(dst.TLSMinVersion) == nil { + dst.CipherSuites = defaultCipherSuites + } } else { dst.CipherSuites = b.tlsCipherSuites( fmt.Sprintf("tls.%s.tls_cipher_suites", name), src.TLSCipherSuites, + dst.TLSMinVersion, ) } } diff --git a/agent/config/builder_test.go b/agent/config/builder_test.go index 80ad7b368..58fd922fc 100644 --- a/agent/config/builder_test.go +++ b/agent/config/builder_test.go @@ -12,6 +12,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/hashicorp/consul/types" ) func TestLoad(t *testing.T) { @@ -327,3 +329,58 @@ func TestBuilder_ServiceVal_MultiError(t *testing.T) { func intPtr(v int) *int { return &v } + +func TestBuilder_tlsVersion(t *testing.T) { + b := builder{} + + validTLSVersion := "TLSv1_3" + b.tlsVersion("tls.defaults.tls_min_version", &validTLSVersion) + + deprecatedTLSVersion := "tls11" + b.tlsVersion("tls.defaults.tls_min_version", &deprecatedTLSVersion) + + invalidTLSVersion := "tls9" + b.tlsVersion("tls.defaults.tls_min_version", &invalidTLSVersion) + + require.Error(t, b.err) + require.Contains(t, b.err.Error(), "2 errors") + require.Contains(t, b.err.Error(), deprecatedTLSVersion) + require.Contains(t, b.err.Error(), invalidTLSVersion) +} + +func TestBuilder_tlsCipherSuites(t *testing.T) { + b := builder{} + + validCipherSuites := strings.Join([]string{ + "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256", + "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA", + "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", + "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA", + "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384", + "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256", + "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA", + "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", + "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA", + "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384", + }, ",") + b.tlsCipherSuites("tls.defaults.tls_cipher_suites", &validCipherSuites, types.TLSv1_2) + require.NoError(t, b.err) + + unsupportedCipherSuites := strings.Join([]string{ + "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256", + }, ",") + b.tlsCipherSuites("tls.defaults.tls_cipher_suites", &unsupportedCipherSuites, types.TLSv1_2) + + invalidCipherSuites := strings.Join([]string{ + "cipherX", + }, ",") + b.tlsCipherSuites("tls.defaults.tls_cipher_suites", &invalidCipherSuites, types.TLSv1_2) + + b.tlsCipherSuites("tls.defaults.tls_cipher_suites", &validCipherSuites, types.TLSv1_3) + + require.Error(t, b.err) + require.Contains(t, b.err.Error(), "3 errors") + require.Contains(t, b.err.Error(), unsupportedCipherSuites) + require.Contains(t, b.err.Error(), invalidCipherSuites) + require.Contains(t, b.err.Error(), "cipher suites are not configurable") +} diff --git a/agent/config/default.go b/agent/config/default.go index 3f40766ff..c0c1bd9b9 100644 --- a/agent/config/default.go +++ b/agent/config/default.go @@ -58,7 +58,7 @@ func DefaultSource() Source { tls = { defaults = { - tls_min_version = "tls12" + tls_min_version = "TLSv1_2" } } diff --git a/agent/config/deprecated.go b/agent/config/deprecated.go index d2649d61d..b27150b51 100644 --- a/agent/config/deprecated.go +++ b/agent/config/deprecated.go @@ -2,6 +2,8 @@ package config import ( "fmt" + + "github.com/hashicorp/consul/types" ) type DeprecatedConfig struct { @@ -219,7 +221,16 @@ func applyDeprecatedTLSConfig(dep DeprecatedConfig, cfg *Config) []string { if v := dep.TLSMinVersion; v != nil { if defaults.TLSMinVersion == nil { - defaults.TLSMinVersion = v + // NOTE: This inner check for deprecated values should eventually be + // removed + if version, ok := types.DeprecatedConsulAgentTLSVersions[*v]; ok { + // Log warning about deprecated config values + warns = append(warns, fmt.Sprintf("'tls_min_version' value '%s' is deprecated, please specify '%s' instead", *v, version)) + versionString := version.String() + defaults.TLSMinVersion = &versionString + } else { + defaults.TLSMinVersion = v + } } warns = append(warns, deprecationWarning("tls_min_version", "tls.defaults.tls_min_version")) } diff --git a/agent/config/deprecated_test.go b/agent/config/deprecated_test.go index ce32a3ccd..36dd86907 100644 --- a/agent/config/deprecated_test.go +++ b/agent/config/deprecated_test.go @@ -1,7 +1,7 @@ package config import ( - "crypto/tls" + "fmt" "sort" "testing" "time" @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/require" "github.com/hashicorp/consul/tlsutil" + "github.com/hashicorp/consul/types" ) func TestLoad_DeprecatedConfig(t *testing.T) { @@ -33,8 +34,8 @@ ca_file = "some-ca-file" ca_path = "some-ca-path" cert_file = "some-cert-file" key_file = "some-key-file" -tls_cipher_suites = "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256" -tls_min_version = "some-tls-version" +tls_cipher_suites = "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA" +tls_min_version = "tls11" verify_incoming = true verify_incoming_https = false verify_incoming_rpc = false @@ -61,6 +62,7 @@ tls_prefer_server_cipher_suites = true deprecationWarning("cert_file", "tls.defaults.cert_file"), deprecationWarning("key_file", "tls.defaults.key_file"), deprecationWarning("tls_cipher_suites", "tls.defaults.tls_cipher_suites"), + fmt.Sprintf("'tls_min_version' value 'tls11' is deprecated, please specify 'TLSv1_1' instead"), deprecationWarning("tls_min_version", "tls.defaults.tls_min_version"), deprecationWarning("verify_incoming", "tls.defaults.verify_incoming"), deprecationWarning("verify_incoming_https", "tls.https.verify_incoming"), @@ -90,8 +92,8 @@ tls_prefer_server_cipher_suites = true require.Equal(t, "some-ca-path", l.CAPath) require.Equal(t, "some-cert-file", l.CertFile) require.Equal(t, "some-key-file", l.KeyFile) - require.Equal(t, "some-tls-version", l.TLSMinVersion) - require.Equal(t, []uint16{tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256}, l.CipherSuites) + require.Equal(t, types.TLSVersion("TLSv1_1"), l.TLSMinVersion) + require.Equal(t, []types.TLSCipherSuite{types.TLSCipherSuite("TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA")}, l.CipherSuites) } require.False(t, rt.TLS.InternalRPC.VerifyIncoming) diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index 7c24e71d3..b239b1ed4 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -2,7 +2,6 @@ package config import ( "bytes" - "crypto/tls" "encoding/base64" "encoding/json" "errors" @@ -5395,8 +5394,8 @@ func TestLoad_IntegrationWithFlags(t *testing.T) { ca_file = "default_ca_file" ca_path = "default_ca_path" cert_file = "default_cert_file" - tls_min_version = "tls12" - tls_cipher_suites = "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256" + tls_min_version = "TLSv1_2" + tls_cipher_suites = "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256" verify_incoming = true } @@ -5406,7 +5405,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) { https { cert_file = "https_cert_file" - tls_min_version = "tls13" + tls_min_version = "TLSv1_3" } grpc { @@ -5425,8 +5424,8 @@ func TestLoad_IntegrationWithFlags(t *testing.T) { "ca_file": "default_ca_file", "ca_path": "default_ca_path", "cert_file": "default_cert_file", - "tls_min_version": "tls12", - "tls_cipher_suites": "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256", + "tls_min_version": "TLSv1_2", + "tls_cipher_suites": "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256", "verify_incoming": true }, "internal_rpc": { @@ -5434,7 +5433,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) { }, "https": { "cert_file": "https_cert_file", - "tls_min_version": "tls13" + "tls_min_version": "TLSv1_3" }, "grpc": { "verify_incoming": false, @@ -5455,22 +5454,21 @@ func TestLoad_IntegrationWithFlags(t *testing.T) { rt.TLS.InternalRPC.CAFile = "internal_rpc_ca_file" rt.TLS.InternalRPC.CAPath = "default_ca_path" rt.TLS.InternalRPC.CertFile = "default_cert_file" - rt.TLS.InternalRPC.TLSMinVersion = "tls12" - rt.TLS.InternalRPC.CipherSuites = []uint16{tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256} + rt.TLS.InternalRPC.TLSMinVersion = "TLSv1_2" + rt.TLS.InternalRPC.CipherSuites = []types.TLSCipherSuite{types.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256} rt.TLS.InternalRPC.VerifyIncoming = true rt.TLS.HTTPS.CAFile = "default_ca_file" rt.TLS.HTTPS.CAPath = "default_ca_path" rt.TLS.HTTPS.CertFile = "https_cert_file" - rt.TLS.HTTPS.TLSMinVersion = "tls13" - rt.TLS.HTTPS.CipherSuites = []uint16{tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256} + rt.TLS.HTTPS.TLSMinVersion = "TLSv1_3" rt.TLS.HTTPS.VerifyIncoming = true rt.TLS.GRPC.CAFile = "default_ca_file" rt.TLS.GRPC.CAPath = "default_ca_path" rt.TLS.GRPC.CertFile = "default_cert_file" - rt.TLS.GRPC.TLSMinVersion = "tls12" - rt.TLS.GRPC.CipherSuites = []uint16{tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA} + rt.TLS.GRPC.TLSMinVersion = "TLSv1_2" + rt.TLS.GRPC.CipherSuites = []types.TLSCipherSuite{types.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA} rt.TLS.GRPC.VerifyIncoming = false }, }) @@ -6310,8 +6308,8 @@ func TestLoad_FullConfig(t *testing.T) { CAPath: "lOp1nhPa", CertFile: "dfJ4oPln", KeyFile: "aL1Knkpo", - TLSMinVersion: "lPo1MklP", - CipherSuites: []uint16{tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256}, + TLSMinVersion: types.TLSv1_1, + CipherSuites: []types.TLSCipherSuite{types.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256, types.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA}, VerifyOutgoing: true, VerifyServerHostname: true, }, @@ -6321,8 +6319,8 @@ func TestLoad_FullConfig(t *testing.T) { CAPath: "fLponKpl", CertFile: "a674klPn", KeyFile: "1y4prKjl", - TLSMinVersion: "lPo4fNkl", - CipherSuites: []uint16{tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256}, + TLSMinVersion: types.TLSv1_0, + CipherSuites: []types.TLSCipherSuite{types.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256, types.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA}, VerifyOutgoing: false, }, HTTPS: tlsutil.ProtocolConfig{ @@ -6331,8 +6329,7 @@ func TestLoad_FullConfig(t *testing.T) { CAPath: "nu4PlHzn", CertFile: "1yrhPlMk", KeyFile: "1bHapOkL", - TLSMinVersion: "mK14iOpz", - CipherSuites: []uint16{tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256}, + TLSMinVersion: types.TLSv1_3, VerifyOutgoing: true, }, NodeName: "otlLxGaI", diff --git a/agent/config/testdata/TestRuntimeConfig_Sanitize.golden b/agent/config/testdata/TestRuntimeConfig_Sanitize.golden index 0f8d66b8b..a8e2f46ee 100644 --- a/agent/config/testdata/TestRuntimeConfig_Sanitize.golden +++ b/agent/config/testdata/TestRuntimeConfig_Sanitize.golden @@ -453,4 +453,4 @@ "Version": "", "VersionPrerelease": "", "Watches": [] -} \ No newline at end of file +} diff --git a/agent/config/testdata/full-config.hcl b/agent/config/testdata/full-config.hcl index bb68055cf..48b6e9a1a 100644 --- a/agent/config/testdata/full-config.hcl +++ b/agent/config/testdata/full-config.hcl @@ -653,8 +653,8 @@ tls { ca_path = "bN63LpXu" cert_file = "hB4PoxkL" key_file = "Po0hB1tY" - tls_cipher_suites = "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256" - tls_min_version = "yU0uIp1A" + tls_cipher_suites = "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256" + tls_min_version = "TLSv1_2" verify_incoming = true verify_outgoing = true } @@ -663,8 +663,8 @@ tls { ca_path = "lOp1nhPa" cert_file = "dfJ4oPln" key_file = "aL1Knkpo" - tls_cipher_suites = "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256" - tls_min_version = "lPo1MklP" + tls_cipher_suites = "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256,TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA" + tls_min_version = "TLSv1_1" verify_incoming = true verify_outgoing = true verify_server_hostname = true @@ -674,8 +674,7 @@ tls { ca_path = "nu4PlHzn" cert_file = "1yrhPlMk" key_file = "1bHapOkL" - tls_cipher_suites = "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256" - tls_min_version = "mK14iOpz" + tls_min_version = "TLSv1_3" verify_incoming = true verify_outgoing = true } @@ -684,13 +683,13 @@ tls { ca_path = "fLponKpl" cert_file = "a674klPn" key_file = "1y4prKjl" - tls_cipher_suites = "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256" - tls_min_version = "lPo4fNkl" + tls_cipher_suites = "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256,TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA" + tls_min_version = "TLSv1_0" verify_incoming = true } } -tls_cipher_suites = "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256" -tls_min_version = "pAOWafkR" +tls_cipher_suites = "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256" +tls_min_version = "tls11" tls_prefer_server_cipher_suites = true translate_wan_addrs = true ui_config { diff --git a/agent/config/testdata/full-config.json b/agent/config/testdata/full-config.json index 574c715f4..1c92d6d02 100644 --- a/agent/config/testdata/full-config.json +++ b/agent/config/testdata/full-config.json @@ -650,8 +650,8 @@ "ca_path": "bN63LpXu", "cert_file": "hB4PoxkL", "key_file": "Po0hB1tY", - "tls_cipher_suites": "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256", - "tls_min_version": "yU0uIp1A", + "tls_cipher_suites": "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", + "tls_min_version": "TLSv1_2", "verify_incoming": true, "verify_outgoing": true }, @@ -660,8 +660,8 @@ "ca_path": "lOp1nhPa", "cert_file": "dfJ4oPln", "key_file": "aL1Knkpo", - "tls_cipher_suites": "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256", - "tls_min_version": "lPo1MklP", + "tls_cipher_suites": "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256,TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA", + "tls_min_version": "TLSv1_1", "verify_incoming": true, "verify_outgoing": true }, @@ -670,8 +670,7 @@ "ca_path": "nu4PlHzn", "cert_file": "1yrhPlMk", "key_file": "1bHapOkL", - "tls_cipher_suites": "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256", - "tls_min_version": "mK14iOpz", + "tls_min_version": "TLSv1_3", "verify_incoming": true, "verify_outgoing": true }, @@ -680,13 +679,13 @@ "ca_path": "fLponKpl", "cert_file": "a674klPn", "key_file": "1y4prKjl", - "tls_cipher_suites": "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256", - "tls_min_version": "lPo4fNkl", + "tls_cipher_suites": "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256,TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA", + "tls_min_version": "TLSv1_0", "verify_incoming": true } }, - "tls_cipher_suites": "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256", - "tls_min_version": "pAOWafkR", + "tls_cipher_suites": "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", + "tls_min_version": "tls11", "tls_prefer_server_cipher_suites": true, "translate_wan_addrs": true, "ui_config": { diff --git a/agent/consul/auto_config_endpoint.go b/agent/consul/auto_config_endpoint.go index 781192d6e..5ca15f33b 100644 --- a/agent/consul/auto_config_endpoint.go +++ b/agent/consul/auto_config_endpoint.go @@ -281,6 +281,7 @@ func (ac *AutoConfig) updateTLSSettingsInConfig(_ AutoConfigOptions, resp *pbaut } var err error + resp.Config.TLS, err = ac.tlsConfigurator.AutoConfigTLSSettings() return err } diff --git a/agent/consul/auto_config_endpoint_test.go b/agent/consul/auto_config_endpoint_test.go index 00873b615..f81461bbb 100644 --- a/agent/consul/auto_config_endpoint_test.go +++ b/agent/consul/auto_config_endpoint_test.go @@ -25,6 +25,7 @@ import ( "github.com/hashicorp/consul/proto/pbconnect" "github.com/hashicorp/consul/sdk/testutil" "github.com/hashicorp/consul/tlsutil" + "github.com/hashicorp/consul/types" "gopkg.in/square/go-jose.v2/jwt" ) @@ -173,7 +174,7 @@ func TestAutoConfigInitialConfiguration(t *testing.T) { c.TLSConfig.InternalRPC.VerifyOutgoing = true c.TLSConfig.InternalRPC.VerifyIncoming = true c.TLSConfig.InternalRPC.VerifyServerHostname = true - c.TLSConfig.InternalRPC.TLSMinVersion = "tls12" + c.TLSConfig.InternalRPC.TLSMinVersion = types.TLSv1_2 c.ConnectEnabled = true c.AutoEncryptAllowTLS = true @@ -391,13 +392,6 @@ func TestAutoConfig_baseConfig(t *testing.T) { } } -func parseCiphers(t *testing.T, cipherStr string) []uint16 { - t.Helper() - ciphers, err := tlsutil.ParseCiphers(cipherStr) - require.NoError(t, err) - return ciphers -} - func TestAutoConfig_updateTLSSettingsInConfig(t *testing.T) { _, _, cacert, err := testTLSCertificates("server.dc1.consul") require.NoError(t, err) @@ -418,9 +412,9 @@ func TestAutoConfig_updateTLSSettingsInConfig(t *testing.T) { InternalRPC: tlsutil.ProtocolConfig{ VerifyServerHostname: true, VerifyOutgoing: true, - TLSMinVersion: "tls12", + TLSMinVersion: types.TLSv1_2, CAFile: cafile, - CipherSuites: parseCiphers(t, "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"), + CipherSuites: []types.TLSCipherSuite{"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384", "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"}, }, }, expected: pbautoconf.AutoConfigResponse{ @@ -439,9 +433,9 @@ func TestAutoConfig_updateTLSSettingsInConfig(t *testing.T) { InternalRPC: tlsutil.ProtocolConfig{ VerifyServerHostname: false, VerifyOutgoing: true, - TLSMinVersion: "tls10", + TLSMinVersion: types.TLSv1_0, CAFile: cafile, - CipherSuites: parseCiphers(t, "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"), + CipherSuites: []types.TLSCipherSuite{"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"}, }, }, expected: pbautoconf.AutoConfigResponse{ @@ -635,9 +629,9 @@ func TestAutoConfig_updateTLSCertificatesInConfig(t *testing.T) { InternalRPC: tlsutil.ProtocolConfig{ VerifyServerHostname: true, VerifyOutgoing: true, - TLSMinVersion: "tls12", + TLSMinVersion: types.TLSv1_2, CAFile: cafile, - CipherSuites: parseCiphers(t, "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"), + CipherSuites: []types.TLSCipherSuite{"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384", "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"}, }, }, expected: pbautoconf.AutoConfigResponse{ @@ -654,9 +648,9 @@ func TestAutoConfig_updateTLSCertificatesInConfig(t *testing.T) { InternalRPC: tlsutil.ProtocolConfig{ VerifyServerHostname: true, VerifyOutgoing: true, - TLSMinVersion: "tls12", + TLSMinVersion: types.TLSv1_2, CAFile: cafile, - CipherSuites: parseCiphers(t, "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"), + CipherSuites: []types.TLSCipherSuite{"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384", "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"}, }, }, opts: AutoConfigOptions{ diff --git a/tlsutil/config.go b/tlsutil/config.go index c6157da93..bf4e9f6c6 100644 --- a/tlsutil/config.go +++ b/tlsutil/config.go @@ -8,7 +8,6 @@ import ( "net" "os" "path/filepath" - "sort" "strings" "sync" "sync/atomic" @@ -19,6 +18,7 @@ import ( "github.com/hashicorp/consul/logging" "github.com/hashicorp/consul/proto/pbconfig" + "github.com/hashicorp/consul/types" ) // ALPNWrapper is a function that is used to wrap a non-TLS connection and @@ -36,13 +36,13 @@ type DCWrapper func(dc string, conn net.Conn) (net.Conn, error) // a constant value. This is usually done by currying DCWrapper. type Wrapper func(conn net.Conn) (net.Conn, error) -// tlsLookup maps the tls_min_version configuration to the internal value -var tlsLookup = map[string]uint16{ - "": tls.VersionTLS10, // default in golang - "tls10": tls.VersionTLS10, - "tls11": tls.VersionTLS11, - "tls12": tls.VersionTLS12, - "tls13": tls.VersionTLS13, +// goTLSVersions maps types.TLSVersion to the Go internal value +var goTLSVersions = map[types.TLSVersion]uint16{ + types.TLSVersionAuto: tls.VersionTLS12, + types.TLSv1_0: tls.VersionTLS10, + types.TLSv1_1: tls.VersionTLS11, + types.TLSv1_2: tls.VersionTLS12, + types.TLSv1_3: tls.VersionTLS13, } // ProtocolConfig contains configuration for a given protocol. @@ -71,27 +71,16 @@ type ProtocolConfig struct { KeyFile string // TLSMinVersion is the minimum accepted TLS version that can be used. - TLSMinVersion string + + TLSMinVersion types.TLSVersion // CipherSuites is the list of TLS cipher suites to use. // - // The values should be a list of the following values: - // - // TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA - // TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 - // TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 - // TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA - // TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384 - // TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA - // TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 - // TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 - // TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA - // TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 - // - // todo(fs): IMHO, we should also support the raw 0xNNNN values from - // todo(fs): https://golang.org/pkg/crypto/tls/#pkg-constants - // todo(fs): since they are standardized by IANA. - CipherSuites []uint16 + // We don't support the raw 0xNNNN values from + // https://golang.org/pkg/crypto/tls/#pkg-constants + // even though they are standardized by IANA because it would increase + // the likelihood of an operator inadvertently setting an insecure configuration + CipherSuites []types.TLSCipherSuite // VerifyOutgoing is used to verify the authenticity of outgoing // connections. This means that TLS requests are used, and TCP @@ -148,17 +137,6 @@ type Config struct { AutoTLS bool } -func tlsVersions() []string { - versions := []string{} - for v := range tlsLookup { - if v != "" { - versions = append(versions, v) - } - } - sort.Strings(versions) - return versions -} - // SpecificDC is used to invoke a static datacenter // and turns a DCWrapper into a Wrapper type. func SpecificDC(dc string, tlsWrap DCWrapper) Wrapper { @@ -289,19 +267,12 @@ func (c *Configurator) Update(config Config) error { // loadProtocolConfig loads the certificates etc. for a given ProtocolConfig // and performs validation. -func (c *Configurator) loadProtocolConfig(base Config, lc ProtocolConfig) (*protocolConfig, error) { - if min := lc.TLSMinVersion; min != "" { - if _, ok := tlsLookup[min]; !ok { - versions := strings.Join(tlsVersions(), ", ") - return nil, fmt.Errorf("TLSMinVersion: value %s not supported, please specify one of [%s]", min, versions) - } - } - - cert, err := loadKeyPair(lc.CertFile, lc.KeyFile) +func (c *Configurator) loadProtocolConfig(base Config, pc ProtocolConfig) (*protocolConfig, error) { + cert, err := loadKeyPair(pc.CertFile, pc.KeyFile) if err != nil { return nil, err } - pems, err := LoadCAs(lc.CAFile, lc.CAPath) + pems, err := LoadCAs(pc.CAFile, pc.CAPath) if err != nil { return nil, err } @@ -314,7 +285,7 @@ func (c *Configurator) loadProtocolConfig(base Config, lc ProtocolConfig) (*prot return nil, err } - if lc.VerifyIncoming { + if pc.VerifyIncoming { // Both auto-config and auto-encrypt require verifying the connection from the // client to the server for secure operation. In order to be able to verify the // server's certificate we must have some CA certs already provided. Therefore, @@ -336,7 +307,7 @@ func (c *Configurator) loadProtocolConfig(base Config, lc ProtocolConfig) (*prot } // Ensure we have a CA if VerifyOutgoing is set. - if lc.VerifyOutgoing && combinedPool == nil { + if pc.VerifyOutgoing && combinedPool == nil { return nil, fmt.Errorf("VerifyOutgoing set but no CA certificates were provided") } @@ -572,7 +543,11 @@ func (c *Configurator) commonTLSConfig(state protocolConfig, cfg ProtocolConfig, // Set the cipher suites if len(cfg.CipherSuites) != 0 { - tlsConfig.CipherSuites = cfg.CipherSuites + // TLS cipher suites are validated on input in agent config builder, + // so it's safe to ignore the error case here. + + cipherSuites, _ := cipherSuiteLookup(cfg.CipherSuites) + tlsConfig.CipherSuites = cipherSuites } // GetCertificate is used when acting as a server and responding to @@ -607,10 +582,11 @@ func (c *Configurator) commonTLSConfig(state protocolConfig, cfg ProtocolConfig, tlsConfig.ClientCAs = state.combinedCAPool tlsConfig.RootCAs = state.combinedCAPool - // This is possible because tlsLookup also contains "" with golang's - // default (tls10). And because the initial check makes sure the - // version correctly matches. - tlsConfig.MinVersion = tlsLookup[cfg.TLSMinVersion] + // Error handling is not needed here because agent config builder handles "" + // or a nil value as TLSVersionAuto with goTLSVersions mapping TLSVersionAuto + // to TLS 1.2 and because the initial check makes sure a specified version is + // not invalid. + tlsConfig.MinVersion = goTLSVersions[cfg.TLSMinVersion] // Set ClientAuth if necessary if verifyIncoming { @@ -736,7 +712,7 @@ func (c *Configurator) AutoConfigTLSSettings() (*pbconfig.TLS, error) { return &pbconfig.TLS{ VerifyOutgoing: cfg.VerifyOutgoing, VerifyServerHostname: cfg.VerifyServerHostname || c.autoTLS.verifyServerHostname, - MinVersion: cfg.TLSMinVersion, + MinVersion: types.ConsulAutoConfigTLSVersionStrings[cfg.TLSMinVersion], CipherSuites: cipherString, }, nil } @@ -1080,32 +1056,32 @@ func (c *Configurator) AuthorizeServerConn(dc string, conn TLSConn) error { } -// ParseCiphers parse ciphersuites from the comma-separated string into -// recognized slice -func ParseCiphers(cipherStr string) ([]uint16, error) { +// NOTE: any new cipher suites will also need to be added in types/tls.go +// TODO: should this be moved into types/tls.go? Would importing Go's tls +// package in there be acceptable? +var goTLSCipherSuites = map[types.TLSCipherSuite]uint16{ + types.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256: tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256, + types.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA: tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, + types.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256: tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, + types.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA: tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA, + types.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384: tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, + + types.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256: tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, + types.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA: tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, + types.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256: tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, + types.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA: tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, + types.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384: tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, +} + +func cipherSuiteLookup(ciphers []types.TLSCipherSuite) ([]uint16, error) { suites := []uint16{} - cipherStr = strings.TrimSpace(cipherStr) - if cipherStr == "" { + if len(ciphers) == 0 { return []uint16{}, nil } - ciphers := strings.Split(cipherStr, ",") - // Note: this needs to be kept up to date with the cipherMap in CipherString - cipherMap := map[string]uint16{ - "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA": tls.TLS_ECDHE_ECDSA_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_GCM_SHA256": tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, - "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA": tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA, - "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384": tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, - "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA": tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, - "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256": tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256, - "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256": tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, - "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA": tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, - "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384": tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, - } for _, cipher := range ciphers { - if v, ok := cipherMap[cipher]; ok { + if v, ok := goTLSCipherSuites[cipher]; ok { suites = append(suites, v) } else { return suites, fmt.Errorf("unsupported cipher %q", cipher) @@ -1115,29 +1091,16 @@ func ParseCiphers(cipherStr string) ([]uint16, error) { return suites, nil } -// CipherString performs the inverse operation of ParseCiphers -func CipherString(ciphers []uint16) (string, error) { - // Note: this needs to be kept up to date with the cipherMap in ParseCiphers - cipherMap := map[uint16]string{ - tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA: "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA", - tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256: "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256", - tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256: "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", - tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA: "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA", - tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384: "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384", - tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA: "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA", - tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256: "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256", - tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256: "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", - tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA: "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA", - tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384: "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384", +// CipherString performs the inverse operation of types.ParseCiphers +func CipherString(ciphers []types.TLSCipherSuite) (string, error) { + err := types.ValidateConsulAgentCipherSuites(ciphers) + if err != nil { + return "", err } cipherStrings := make([]string, len(ciphers)) for i, cipher := range ciphers { - if v, ok := cipherMap[cipher]; ok { - cipherStrings[i] = v - } else { - return "", fmt.Errorf("unsupported cipher %d", cipher) - } + cipherStrings[i] = string(cipher) } return strings.Join(cipherStrings, ","), nil diff --git a/tlsutil/config_test.go b/tlsutil/config_test.go index abcade402..b49bd66bc 100644 --- a/tlsutil/config_test.go +++ b/tlsutil/config_test.go @@ -8,8 +8,6 @@ import ( "io/ioutil" "net" "path/filepath" - "reflect" - "strings" "testing" "github.com/google/go-cmp/cmp" @@ -19,6 +17,7 @@ import ( "github.com/stretchr/testify/require" "github.com/hashicorp/consul/sdk/testutil" + "github.com/hashicorp/consul/types" ) func TestConfigurator_IncomingConfig_Common(t *testing.T) { @@ -46,7 +45,7 @@ func TestConfigurator_IncomingConfig_Common(t *testing.T) { t.Run(desc, func(t *testing.T) { t.Run("MinTLSVersion", func(t *testing.T) { cfg := ProtocolConfig{ - TLSMinVersion: "tls13", + TLSMinVersion: "TLSv1_3", CertFile: "../test/hostname/Alice.crt", KeyFile: "../test/hostname/Alice.key", } @@ -69,7 +68,7 @@ func TestConfigurator_IncomingConfig_Common(t *testing.T) { t.Run("CipherSuites", func(t *testing.T) { cfg := ProtocolConfig{ - CipherSuites: []uint16{tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384}, + CipherSuites: []types.TLSCipherSuite{types.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384}, CertFile: "../test/hostname/Alice.crt", KeyFile: "../test/hostname/Alice.key", } @@ -760,45 +759,6 @@ func TestConfigurator_outgoingWrapperALPN_serverHasNoNodeNameInSAN(t *testing.T) <-errc } -func TestConfig_ParseCiphers(t *testing.T) { - testOk := strings.Join([]string{ - "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA", - "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256", - "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", - "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA", - "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384", - "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA", - "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256", - "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", - "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA", - "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384", - }, ",") - ciphers := []uint16{ - tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, - tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256, - tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, - tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA, - tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, - tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, - tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256, - tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, - tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, - tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, - } - v, err := ParseCiphers(testOk) - require.NoError(t, err) - if got, want := v, ciphers; !reflect.DeepEqual(got, want) { - t.Fatalf("got ciphers %#v want %#v", got, want) - } - - _, err = ParseCiphers("TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,cipherX") - require.Error(t, err) - - v, err = ParseCiphers("") - require.NoError(t, err) - require.Equal(t, []uint16{}, v) -} - func TestLoadKeyPair(t *testing.T) { type variant struct { cert, key string @@ -866,14 +826,6 @@ func TestConfigurator_Validation(t *testing.T) { } testCases := map[string]testCase{ - "invalid TLSMinVersion": { - ProtocolConfig{TLSMinVersion: "tls9"}, - false, - }, - "default TLSMinVersion": { - ProtocolConfig{TLSMinVersion: ""}, - true, - }, "invalid CAFile": { ProtocolConfig{CAFile: "bogus"}, false, @@ -986,13 +938,6 @@ func TestConfigurator_Validation(t *testing.T) { }, } - for _, v := range tlsVersions() { - testCases[fmt.Sprintf("MinTLSVersion(%s)", v)] = testCase{ - ProtocolConfig{TLSMinVersion: v}, - true, - } - } - for desc, tc := range testCases { for _, p := range []string{"internal", "grpc", "https"} { info := fmt.Sprintf("%s => %s", p, desc) @@ -1229,7 +1174,7 @@ func TestConfigurator_OutgoingTLSConfigForCheck(t *testing.T) { conf: func() (*Configurator, error) { return NewConfigurator(Config{ InternalRPC: ProtocolConfig{ - TLSMinVersion: "tls12", + TLSMinVersion: types.TLSv1_2, }, EnableAgentTLSForChecks: false, }, nil) @@ -1242,7 +1187,7 @@ func TestConfigurator_OutgoingTLSConfigForCheck(t *testing.T) { conf: func() (*Configurator, error) { return NewConfigurator(Config{ InternalRPC: ProtocolConfig{ - TLSMinVersion: "tls12", + TLSMinVersion: types.TLSv1_2, }, EnableAgentTLSForChecks: false, ServerName: "servername", @@ -1257,7 +1202,7 @@ func TestConfigurator_OutgoingTLSConfigForCheck(t *testing.T) { conf: func() (*Configurator, error) { return NewConfigurator(Config{ InternalRPC: ProtocolConfig{ - TLSMinVersion: "tls12", + TLSMinVersion: types.TLSv1_2, }, EnableAgentTLSForChecks: false, ServerName: "servername", @@ -1275,7 +1220,7 @@ func TestConfigurator_OutgoingTLSConfigForCheck(t *testing.T) { conf: func() (*Configurator, error) { return NewConfigurator(Config{ InternalRPC: ProtocolConfig{ - TLSMinVersion: "tls12", + TLSMinVersion: types.TLSv1_2, }, EnableAgentTLSForChecks: true, NodeName: "nodename", @@ -1292,7 +1237,7 @@ func TestConfigurator_OutgoingTLSConfigForCheck(t *testing.T) { conf: func() (*Configurator, error) { return NewConfigurator(Config{ InternalRPC: ProtocolConfig{ - TLSMinVersion: "tls12", + TLSMinVersion: types.TLSv1_2, }, EnableAgentTLSForChecks: true, NodeName: "nodename", @@ -1310,7 +1255,7 @@ func TestConfigurator_OutgoingTLSConfigForCheck(t *testing.T) { conf: func() (*Configurator, error) { return NewConfigurator(Config{ InternalRPC: ProtocolConfig{ - TLSMinVersion: "tls12", + TLSMinVersion: types.TLSv1_2, }, EnableAgentTLSForChecks: true, ServerName: "servername", @@ -1517,12 +1462,6 @@ func TestConfigurator_AuthorizeInternalRPCServerConn(t *testing.T) { }) } -func TestConfig_tlsVersions(t *testing.T) { - require.Equal(t, []string{"tls10", "tls11", "tls12", "tls13"}, tlsVersions()) - expected := "tls10, tls11, tls12, tls13" - require.Equal(t, expected, strings.Join(tlsVersions(), ", ")) -} - func TestConfigurator_GRPCTLSConfigured(t *testing.T) { t.Run("certificate manually configured", func(t *testing.T) { c := makeConfigurator(t, Config{ diff --git a/types/tls.go b/types/tls.go index 9c50f498b..198d4052d 100644 --- a/types/tls.go +++ b/types/tls.go @@ -2,6 +2,7 @@ package types import ( "fmt" + "sort" "strings" ) @@ -92,9 +93,19 @@ func (a TLSVersion) LessThan(b TLSVersion) (error, bool) { return nil, tlsVersionComparison[a] < tlsVersionComparison[b] } +func TLSVersions() string { + versions := []string{} + for v := range tlsVersions { + versions = append(versions, string(v)) + } + sort.Strings(versions) + + return strings.Join(versions, ", ") +} + func ValidateTLSVersion(v TLSVersion) error { if _, ok := tlsVersions[v]; !ok { - return fmt.Errorf("no matching TLS version found for %s", v.String()) + return fmt.Errorf("no matching TLS version found for %s, please specify one of [%s]", v.String(), TLSVersions()) } return nil diff --git a/website/content/docs/agent/options.mdx b/website/content/docs/agent/options.mdx index 11eae98ae..b35e964ff 100644 --- a/website/content/docs/agent/options.mdx +++ b/website/content/docs/agent/options.mdx @@ -2495,15 +2495,30 @@ specially crafted certificate signed by the CA can be used to gain full access t [`cert_file`](#tls_defaults_cert_file). - `tls_min_version` ((#tls_defaults_tls_min_version)) This specifies the - minimum supported version of TLS. Accepted values are "tls10", "tls11", - "tls12", or "tls13". This defaults to "tls12". **WARNING: TLS 1.1 and - lower are generally considered less secure; avoid using these if - possible.** + minimum supported version of TLS. The following values are accepted: + * `TLSv1_0` + * `TLSv1_1` + * `TLSv1_2` (default) + * `TLSv1_3` + + **WARNING: TLS 1.1 and lower are generally considered less secure and + should not be used if possible.** + + The following values are also valid, but only when using the + [deprecated top-level `tls_min_version` config](#tls_deprecated_options), + and will be removed in a future release: + + * `tls10` + * `tls11` + * `tls12` + * `tls13` + + A warning message will appear if a deprecated value is specified. - `tls_cipher_suites` ((#tls_defaults_tls_cipher_suites)) This specifies the list of supported ciphersuites as a comma-separated-list. Applicable to TLS 1.2 and below only. The list of all supported ciphersuites is - available through [this search](https://github.com/hashicorp/consul/search?q=cipherMap+%3A%3D+map&unscoped_q=cipherMap+%3A%3D+map). + available through [this search](https://github.com/hashicorp/consul/search?q=goTLSCipherSuites+%3D+map). ~> **Note:** The ordering of cipher suites will not be guaranteed from Consul 1.11 onwards. See this [post](https://go.dev/blog/tls-cipher-suites)