From 032660573c5d82c0f44c9259a352e411606e1d2f Mon Sep 17 00:00:00 2001 From: Filippo Valsorda Date: Wed, 22 May 2024 11:39:41 +0200 Subject: [PATCH] crypto/tls: disable 3-DES by default Fixes #66214 Change-Id: Iba8006a17fc7cd33c7485ab1a1ef8f56531c0ed1 Reviewed-on: https://go-review.googlesource.com/c/go/+/587295 LUCI-TryBot-Result: Go LUCI Reviewed-by: Roland Shoemaker Reviewed-by: Dmitri Shuralyov Auto-Submit: Filippo Valsorda --- doc/godebug.md | 4 +++ .../6-stdlib/99-minor/crypto/tls/66214.md | 3 ++ src/crypto/tls/cipher_suites.go | 33 +++++++++++-------- src/crypto/tls/common.go | 11 +++---- src/crypto/tls/handshake_client.go | 4 +++ src/crypto/tls/handshake_server.go | 4 +++ src/crypto/tls/tls_test.go | 13 ++++++-- src/internal/godebugs/table.go | 1 + src/net/http/client_test.go | 4 +-- src/runtime/metrics/doc.go | 4 +++ 10 files changed, 55 insertions(+), 26 deletions(-) create mode 100644 doc/next/6-stdlib/99-minor/crypto/tls/66214.md diff --git a/doc/godebug.md b/doc/godebug.md index b3a00a0c2b..d5455e337c 100644 --- a/doc/godebug.md +++ b/doc/godebug.md @@ -197,6 +197,10 @@ Go 1.23 re-enabled support in html/template for ECMAScript 6 template literals b The [`jstmpllitinterp` setting](/pkg/html/template#hdr-Security_Model) no longer has any effect. +Go 1.23 changed the default TLS cipher suites used by clients and servers when +not explicitly configured, removing 3DES cipher suites. The default can be reverted +using the [`tls3des` setting](/pkg/crypto/tls/#Config.CipherSuites). + ### Go 1.22 Go 1.22 adds a configurable limit to control the maximum acceptable RSA key size diff --git a/doc/next/6-stdlib/99-minor/crypto/tls/66214.md b/doc/next/6-stdlib/99-minor/crypto/tls/66214.md new file mode 100644 index 0000000000..4a32ca9fc9 --- /dev/null +++ b/doc/next/6-stdlib/99-minor/crypto/tls/66214.md @@ -0,0 +1,3 @@ +3DES cipher suites were removed from the default list used when +[Config.CipherSuites] is nil. The default can be reverted adding `tls3des=1` to +the GODEBUG environment variable. diff --git a/src/crypto/tls/cipher_suites.go b/src/crypto/tls/cipher_suites.go index 6f5bc37197..622ad9b3e4 100644 --- a/src/crypto/tls/cipher_suites.go +++ b/src/crypto/tls/cipher_suites.go @@ -17,7 +17,9 @@ import ( "fmt" "hash" "internal/cpu" + "internal/godebug" "runtime" + "slices" "golang.org/x/crypto/chacha20poly1305" ) @@ -334,6 +336,8 @@ var disabledCipherSuites = map[uint16]bool{ TLS_RSA_WITH_RC4_128_SHA: true, } +var tlsrsakex = godebug.New("tlsrsakex") + // rsaKexCiphers contains the ciphers which use RSA based key exchange, // which we also disable by default unless a GODEBUG is set. var rsaKexCiphers = map[uint16]bool{ @@ -346,21 +350,22 @@ var rsaKexCiphers = map[uint16]bool{ TLS_RSA_WITH_AES_256_GCM_SHA384: true, } -var defaultCipherSuites []uint16 -var defaultCipherSuitesWithRSAKex []uint16 +var tls3des = godebug.New("tls3des") -func init() { - defaultCipherSuites = make([]uint16, 0, len(cipherSuitesPreferenceOrder)) - defaultCipherSuitesWithRSAKex = make([]uint16, 0, len(cipherSuitesPreferenceOrder)) - for _, c := range cipherSuitesPreferenceOrder { - if disabledCipherSuites[c] { - continue - } - if !rsaKexCiphers[c] { - defaultCipherSuites = append(defaultCipherSuites, c) - } - defaultCipherSuitesWithRSAKex = append(defaultCipherSuitesWithRSAKex, c) - } +// tdesCiphers contains 3DES ciphers, +// which we also disable by default unless a GODEBUG is set. +var tdesCiphers = map[uint16]bool{ + TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA: true, + TLS_RSA_WITH_3DES_EDE_CBC_SHA: true, +} + +func defaultCipherSuites() []uint16 { + suites := slices.Clone(cipherSuitesPreferenceOrder) + return slices.DeleteFunc(suites, func(c uint16) bool { + return disabledCipherSuites[c] || + tlsrsakex.Value() != "1" && rsaKexCiphers[c] || + tls3des.Value() != "1" && tdesCiphers[c] + }) } // defaultCipherSuitesTLS13 is also the preference order, since there are no diff --git a/src/crypto/tls/common.go b/src/crypto/tls/common.go index 1b0f19da9e..dcefa2ac9a 100644 --- a/src/crypto/tls/common.go +++ b/src/crypto/tls/common.go @@ -687,7 +687,9 @@ type Config struct { // If CipherSuites is nil, a safe default list is used. The default cipher // suites might change over time. In Go 1.22 RSA key exchange based cipher // suites were removed from the default list, but can be re-added with the - // GODEBUG setting tlsrsakex=1. + // GODEBUG setting tlsrsakex=1. In Go 1.23 3DES cipher suites were removed + // from the default list, but can be re-added with the GODEBUG setting + // tls3des=1. CipherSuites []uint16 // PreferServerCipherSuites is a legacy field and has no effect. @@ -1025,8 +1027,6 @@ func (c *Config) time() time.Time { return t() } -var tlsrsakex = godebug.New("tlsrsakex") - func (c *Config) cipherSuites() []uint16 { if needFIPS() { return fipsCipherSuites(c) @@ -1034,10 +1034,7 @@ func (c *Config) cipherSuites() []uint16 { if c.CipherSuites != nil { return c.CipherSuites } - if tlsrsakex.Value() == "1" { - return defaultCipherSuitesWithRSAKex - } - return defaultCipherSuites + return defaultCipherSuites() } var supportedVersions = []uint16{ diff --git a/src/crypto/tls/handshake_client.go b/src/crypto/tls/handshake_client.go index 1a17385911..d80b2326b3 100644 --- a/src/crypto/tls/handshake_client.go +++ b/src/crypto/tls/handshake_client.go @@ -551,6 +551,10 @@ func (hs *clientHandshakeState) pickCipherSuite() error { tlsrsakex.Value() // ensure godebug is initialized tlsrsakex.IncNonDefault() } + if hs.c.config.CipherSuites == nil && !needFIPS() && tdesCiphers[hs.suite.id] { + tls3des.Value() // ensure godebug is initialized + tls3des.IncNonDefault() + } hs.c.cipherSuite = hs.suite.id return nil diff --git a/src/crypto/tls/handshake_server.go b/src/crypto/tls/handshake_server.go index 09ca8a4e54..ac3d915d17 100644 --- a/src/crypto/tls/handshake_server.go +++ b/src/crypto/tls/handshake_server.go @@ -376,6 +376,10 @@ func (hs *serverHandshakeState) pickCipherSuite() error { tlsrsakex.Value() // ensure godebug is initialized tlsrsakex.IncNonDefault() } + if c.config.CipherSuites == nil && !needFIPS() && tdesCiphers[hs.suite.id] { + tls3des.Value() // ensure godebug is initialized + tls3des.IncNonDefault() + } for _, id := range hs.clientHello.cipherSuites { if id == TLS_FALLBACK_SCSV { diff --git a/src/crypto/tls/tls_test.go b/src/crypto/tls/tls_test.go index ce1c967c57..69b57de1e6 100644 --- a/src/crypto/tls/tls_test.go +++ b/src/crypto/tls/tls_test.go @@ -1458,6 +1458,16 @@ func TestCipherSuites(t *testing.T) { t.Errorf("%#04x: suite TLS 1.0-1.2, but SupportedVersions is %v", c.id, cc.SupportedVersions) } + if cc.Insecure { + if slices.Contains(defaultCipherSuites(), c.id) { + t.Errorf("%#04x: insecure suite in default list", c.id) + } + } else { + if !slices.Contains(defaultCipherSuites(), c.id) { + t.Errorf("%#04x: secure suite not in default list", c.id) + } + } + if got := CipherSuiteName(c.id); got != cc.Name { t.Errorf("%#04x: unexpected CipherSuiteName: got %q, expected %q", c.id, got, cc.Name) } @@ -1491,9 +1501,6 @@ func TestCipherSuites(t *testing.T) { if len(cipherSuitesPreferenceOrderNoAES) != len(cipherSuitesPreferenceOrder) { t.Errorf("cipherSuitesPreferenceOrderNoAES is not the same size as cipherSuitesPreferenceOrder") } - if len(defaultCipherSuites) >= len(defaultCipherSuitesWithRSAKex) { - t.Errorf("defaultCipherSuitesWithRSAKex should be longer than defaultCipherSuites") - } // Check that disabled suites are marked insecure. for _, badSuites := range []map[uint16]bool{disabledCipherSuites, rsaKexCiphers} { diff --git a/src/internal/godebugs/table.go b/src/internal/godebugs/table.go index 4ead2e09c6..eaa95d5aa9 100644 --- a/src/internal/godebugs/table.go +++ b/src/internal/godebugs/table.go @@ -47,6 +47,7 @@ var All = []Info{ {Name: "randautoseed", Package: "math/rand"}, {Name: "tarinsecurepath", Package: "archive/tar"}, {Name: "tls10server", Package: "crypto/tls", Changed: 22, Old: "1"}, + {Name: "tls3des", Package: "crypto/tls", Changed: 23, Old: "1"}, {Name: "tlskyber", Package: "crypto/tls", Changed: 23, Old: "0", Opaque: true}, {Name: "tlsmaxrsasize", Package: "crypto/tls"}, {Name: "tlsrsakex", Package: "crypto/tls", Changed: 22, Old: "1"}, diff --git a/src/net/http/client_test.go b/src/net/http/client_test.go index 33e69467c6..1faa151647 100644 --- a/src/net/http/client_test.go +++ b/src/net/http/client_test.go @@ -946,7 +946,7 @@ func testResponseSetsTLSConnectionState(t *testing.T, mode testMode) { c := ts.Client() tr := c.Transport.(*Transport) - tr.TLSClientConfig.CipherSuites = []uint16{tls.TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA} + tr.TLSClientConfig.CipherSuites = []uint16{tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256} tr.TLSClientConfig.MaxVersion = tls.VersionTLS12 // to get to pick the cipher suite tr.Dial = func(netw, addr string) (net.Conn, error) { return net.Dial(netw, ts.Listener.Addr().String()) @@ -959,7 +959,7 @@ func testResponseSetsTLSConnectionState(t *testing.T, mode testMode) { if res.TLS == nil { t.Fatal("Response didn't set TLS Connection State.") } - if got, want := res.TLS.CipherSuite, tls.TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA; got != want { + if got, want := res.TLS.CipherSuite, tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256; got != want { t.Errorf("TLS Cipher Suite = %d; want %d", got, want) } } diff --git a/src/runtime/metrics/doc.go b/src/runtime/metrics/doc.go index 8e99846e6d..2dd8ce261c 100644 --- a/src/runtime/metrics/doc.go +++ b/src/runtime/metrics/doc.go @@ -302,6 +302,10 @@ Below is the full list of supported metrics, ordered lexicographically. The number of non-default behaviors executed by the crypto/tls package due to a non-default GODEBUG=tls10server=... setting. + /godebug/non-default-behavior/tls3des:events + The number of non-default behaviors executed by the crypto/tls + package due to a non-default GODEBUG=tls3des=... setting. + /godebug/non-default-behavior/tlsmaxrsasize:events The number of non-default behaviors executed by the crypto/tls package due to a non-default GODEBUG=tlsmaxrsasize=... setting. -- 2.48.1