From c901307a470c7481f5e151e96866561382751336 Mon Sep 17 00:00:00 2001 From: Devin Canterberry Date: Tue, 13 Mar 2018 09:50:41 -0700 Subject: [PATCH 1/4] =?UTF-8?q?=F0=9F=94=92=20Update=20supported=20TLS=20c?= =?UTF-8?q?ipher=20suites?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The list of cipher suites included in this commit are consistent with the values and precedence in the [Golang TLS documentation](https://golang.org/src/crypto/tls/cipher_suites.go). > **Note:** Cipher suites with RC4 are still included within the list > of accepted values for compatibility, but **these cipher suites are > not safe to use** and should be deprecated with warnings and > subsequently removed. Support for RC4 ciphers has already been > removed or disabled by default in many prominent browsers and tools, > including Golang. > > **References:** > > * [RC4 on Wikipedia](https://en.wikipedia.org/wiki/RC4) > * [Mozilla Security Blog](https://blog.mozilla.org/security/2015/09/11/deprecating-the-rc4-cipher/) --- agent/config/runtime.go | 31 ++++++++++++--------- agent/config/runtime_test.go | 2 +- tlsutil/config.go | 31 ++++++++++++--------- tlsutil/config_test.go | 52 +++++++++++++++++++++++++----------- 4 files changed, 73 insertions(+), 43 deletions(-) diff --git a/agent/config/runtime.go b/agent/config/runtime.go index fd8e012e07..d7bafe57ee 100644 --- a/agent/config/runtime.go +++ b/agent/config/runtime.go @@ -1107,23 +1107,28 @@ type RuntimeConfig struct { // // The values should be a list of the following values: // - // TLS_RSA_WITH_RC4_128_SHA - // TLS_RSA_WITH_3DES_EDE_CBC_SHA - // TLS_RSA_WITH_AES_128_CBC_SHA - // TLS_RSA_WITH_AES_256_CBC_SHA - // TLS_RSA_WITH_AES_128_GCM_SHA256 - // TLS_RSA_WITH_AES_256_GCM_SHA384 - // TLS_ECDHE_ECDSA_WITH_RC4_128_SHA - // TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA - // TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA - // TLS_ECDHE_RSA_WITH_RC4_128_SHA - // TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA - // TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA - // TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA + // 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 + // TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA + // TLS_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 // // todo(fs): IMHO, we should also support the raw 0xNNNN values from // todo(fs): https://golang.org/pkg/crypto/tls/#pkg-constants diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index 4e1cd6d4a2..9c5ac40331 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -2581,7 +2581,7 @@ func TestFullConfig(t *testing.T) { "statsd_address": "drce87cy", "statsite_address": "HpFwKB8R" }, - "tls_cipher_suites": "TLS_RSA_WITH_RC4_128_SHA,TLS_RSA_WITH_3DES_EDE_CBC_SHA", + "tls_cipher_suites": "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384", "tls_min_version": "pAOWafkR", "tls_prefer_server_cipher_suites": true, "translate_wan_addrs": true, diff --git a/tlsutil/config.go b/tlsutil/config.go index a780d8c20c..73b8fa362b 100644 --- a/tlsutil/config.go +++ b/tlsutil/config.go @@ -361,23 +361,28 @@ func ParseCiphers(cipherStr string) ([]uint16, error) { ciphers := strings.Split(cipherStr, ",") cipherMap := map[string]uint16{ - "TLS_RSA_WITH_RC4_128_SHA": tls.TLS_RSA_WITH_RC4_128_SHA, - "TLS_RSA_WITH_3DES_EDE_CBC_SHA": tls.TLS_RSA_WITH_3DES_EDE_CBC_SHA, - "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, - "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_ECDHE_ECDSA_WITH_RC4_128_SHA": tls.TLS_ECDHE_ECDSA_WITH_RC4_128_SHA, - "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA": tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, - "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA": tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA, - "TLS_ECDHE_RSA_WITH_RC4_128_SHA": tls.TLS_ECDHE_RSA_WITH_RC4_128_SHA, - "TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA": tls.TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA, - "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA": tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, - "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA": tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, + "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, + "TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA": tls.TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA, + "TLS_RSA_WITH_3DES_EDE_CBC_SHA": tls.TLS_RSA_WITH_3DES_EDE_CBC_SHA, + "TLS_RSA_WITH_RC4_128_SHA": tls.TLS_RSA_WITH_RC4_128_SHA, + "TLS_ECDHE_RSA_WITH_RC4_128_SHA": tls.TLS_ECDHE_RSA_WITH_RC4_128_SHA, + "TLS_ECDHE_ECDSA_WITH_RC4_128_SHA": tls.TLS_ECDHE_ECDSA_WITH_RC4_128_SHA } for _, cipher := range ciphers { if v, ok := cipherMap[cipher]; ok { diff --git a/tlsutil/config_test.go b/tlsutil/config_test.go index efb73a2727..ab4a850468 100644 --- a/tlsutil/config_test.go +++ b/tlsutil/config_test.go @@ -513,32 +513,52 @@ func TestConfig_IncomingTLS_TLSMinVersion(t *testing.T) { func TestConfig_ParseCiphers(t *testing.T) { testOk := strings.Join([]string{ - "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_AES_128_CBC_SHA", + "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305", + "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305", "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", - "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA", + "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384", - "TLS_RSA_WITH_AES_128_CBC_SHA", + "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_CBC_SHA", "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", + "TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA", + "TLS_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" }, ",") ciphers := []uint16{ - tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, - 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_CHACHA20_POLY1305, + tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305, tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, - tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, + tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, - tls.TLS_RSA_WITH_AES_128_CBC_SHA, + 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_CBC_SHA, 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, + tls.TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA, + tls.TLS_RSA_WITH_3DES_EDE_CBC_SHA, + tls.TLS_RSA_WITH_RC4_128_SHA, + tls.TLS_ECDHE_RSA_WITH_RC4_128_SHA, + tls.TLS_ECDHE_ECDSA_WITH_RC4_128_SHA } v, err := ParseCiphers(testOk) if err != nil { From a61abcd931dec2bce21cd128642de686152991ee Mon Sep 17 00:00:00 2001 From: Devin Canterberry Date: Tue, 13 Mar 2018 10:30:18 -0700 Subject: [PATCH 2/4] =?UTF-8?q?=F0=9F=90=9B=20Formatting=20changes=20only;?= =?UTF-8?q?=20add=20missing=20trailing=20commas?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- agent/consul/server_test.go | 4 ++-- tlsutil/config.go | 2 +- tlsutil/config_test.go | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/agent/consul/server_test.go b/agent/consul/server_test.go index 3a02308df2..d681d85cfe 100644 --- a/agent/consul/server_test.go +++ b/agent/consul/server_test.go @@ -767,10 +767,10 @@ func TestServer_RevokeLeadershipIdempotent(t *testing.T) { dir1, s1 := testServer(t) defer os.RemoveAll(dir1) defer s1.Shutdown() - + testrpc.WaitForLeader(t, s1.RPC, "dc1") - err:= s1.revokeLeadership() + err := s1.revokeLeadership() if err != nil { t.Fatal(err) } diff --git a/tlsutil/config.go b/tlsutil/config.go index 73b8fa362b..62ad910382 100644 --- a/tlsutil/config.go +++ b/tlsutil/config.go @@ -382,7 +382,7 @@ func ParseCiphers(cipherStr string) ([]uint16, error) { "TLS_RSA_WITH_3DES_EDE_CBC_SHA": tls.TLS_RSA_WITH_3DES_EDE_CBC_SHA, "TLS_RSA_WITH_RC4_128_SHA": tls.TLS_RSA_WITH_RC4_128_SHA, "TLS_ECDHE_RSA_WITH_RC4_128_SHA": tls.TLS_ECDHE_RSA_WITH_RC4_128_SHA, - "TLS_ECDHE_ECDSA_WITH_RC4_128_SHA": tls.TLS_ECDHE_ECDSA_WITH_RC4_128_SHA + "TLS_ECDHE_ECDSA_WITH_RC4_128_SHA": tls.TLS_ECDHE_ECDSA_WITH_RC4_128_SHA, } for _, cipher := range ciphers { if v, ok := cipherMap[cipher]; ok { diff --git a/tlsutil/config_test.go b/tlsutil/config_test.go index ab4a850468..11e1a131f1 100644 --- a/tlsutil/config_test.go +++ b/tlsutil/config_test.go @@ -534,7 +534,7 @@ func TestConfig_ParseCiphers(t *testing.T) { "TLS_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" + "TLS_ECDHE_ECDSA_WITH_RC4_128_SHA", }, ",") ciphers := []uint16{ tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305, @@ -558,7 +558,7 @@ func TestConfig_ParseCiphers(t *testing.T) { tls.TLS_RSA_WITH_3DES_EDE_CBC_SHA, tls.TLS_RSA_WITH_RC4_128_SHA, tls.TLS_ECDHE_RSA_WITH_RC4_128_SHA, - tls.TLS_ECDHE_ECDSA_WITH_RC4_128_SHA + tls.TLS_ECDHE_ECDSA_WITH_RC4_128_SHA, } v, err := ParseCiphers(testOk) if err != nil { From 7236c95e11f970ffe6eb645fbd4286f98fd5a0b4 Mon Sep 17 00:00:00 2001 From: Devin Canterberry Date: Thu, 15 Mar 2018 09:37:32 -0700 Subject: [PATCH 3/4] =?UTF-8?q?=E2=9C=85=20Match=20expectation=20of=20TLSC?= =?UTF-8?q?ipherSuites=20to=20values=20of=20tls=5Fcipher=5Fsuites?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- agent/config/runtime_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index 9c5ac40331..a3b2e44516 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -3017,7 +3017,7 @@ func TestFullConfig(t *testing.T) { statsd_address = "drce87cy" statsite_address = "HpFwKB8R" } - tls_cipher_suites = "TLS_RSA_WITH_RC4_128_SHA,TLS_RSA_WITH_3DES_EDE_CBC_SHA" + tls_cipher_suites = "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384" tls_min_version = "pAOWafkR" tls_prefer_server_cipher_suites = true translate_wan_addrs = true @@ -3575,7 +3575,7 @@ func TestFullConfig(t *testing.T) { TelemetryMetricsPrefix: "ftO6DySn", TelemetryStatsdAddr: "drce87cy", TelemetryStatsiteAddr: "HpFwKB8R", - TLSCipherSuites: []uint16{tls.TLS_RSA_WITH_RC4_128_SHA, tls.TLS_RSA_WITH_3DES_EDE_CBC_SHA}, + TLSCipherSuites: []uint16{tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305, tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384}, TLSMinVersion: "pAOWafkR", TLSPreferServerCipherSuites: true, TaggedAddresses: map[string]string{ From 2187ab1e1caebfbef8266e2e923495a44c046398 Mon Sep 17 00:00:00 2001 From: Devin Canterberry Date: Thu, 15 Mar 2018 10:30:38 -0700 Subject: [PATCH 4/4] =?UTF-8?q?=F0=9F=8E=A8=20Formatting=20changes=20only;?= =?UTF-8?q?=20convert=20leading=20space=20to=20tabs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- agent/config/runtime_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index a3b2e44516..dc48be8f91 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -3017,7 +3017,7 @@ func TestFullConfig(t *testing.T) { statsd_address = "drce87cy" statsite_address = "HpFwKB8R" } - tls_cipher_suites = "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384" + tls_cipher_suites = "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384" tls_min_version = "pAOWafkR" tls_prefer_server_cipher_suites = true translate_wan_addrs = true