From 998f77009ead1ab8a452e24180ec70a3d890e235 Mon Sep 17 00:00:00 2001 From: Peter Wu Date: Tue, 12 Sep 2017 19:50:24 +0100 Subject: [PATCH] crypto/tls: remove TLS13CipherSuites. To allow clients to advertise both TLS 1.2 and TLS 1.3 cipher suites, remove the distinction between both suites. TLS 1.3 suites are now always included in the default cipher list (and the client will send it if MaxVersion allows for it). Since TLS 1.3 is expected to become the default MaxVersion and applications might have set only TLS 1.2 cipher suites in their configuration, TLS 1.3 cipher suites are added when none are present. Alternatively, I considered disallowing overriding the TLS 1.3 suites, but that requires more complexity and has not much benefits. Provide a mechanism and do not dictate policy, application developers might want to fix a cipher suite for testing other implementations for example. Fixes https://github.com/cloudflare/tls-tris/pull/22 --- cipher_suites.go | 4 ++-- common.go | 35 +++++++++++++++++++++-------------- handshake_client.go | 2 +- handshake_server.go | 6 +++--- handshake_server_test.go | 13 ------------- tls_test.go | 1 - 6 files changed, 27 insertions(+), 34 deletions(-) diff --git a/cipher_suites.go b/cipher_suites.go index 3c872ed..6bb17a2 100644 --- a/cipher_suites.go +++ b/cipher_suites.go @@ -389,7 +389,7 @@ func mutualCipherSuite(have []uint16, want uint16) *cipherSuite { // // Taken from http://www.iana.org/assignments/tls-parameters/tls-parameters.xml const ( - // TLS 1.0 - 1.2 cipher suites. To be used in Config.CipherSuites. + // TLS 1.0 - 1.2 cipher suites. TLS_RSA_WITH_RC4_128_SHA uint16 = 0x0005 TLS_RSA_WITH_3DES_EDE_CBC_SHA uint16 = 0x000a TLS_RSA_WITH_AES_128_CBC_SHA uint16 = 0x002f @@ -413,7 +413,7 @@ const ( TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305 uint16 = 0xcca8 TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305 uint16 = 0xcca9 - // TLS 1.3+ cipher suites. To be used in Config.TLS13CipherSuites. + // TLS 1.3+ cipher suites. TLS_AES_128_GCM_SHA256 uint16 = 0x1301 TLS_AES_256_GCM_SHA384 uint16 = 0x1302 TLS_CHACHA20_POLY1305_SHA256 uint16 = 0x1303 diff --git a/common.go b/common.go index 52090b4..aead91f 100644 --- a/common.go +++ b/common.go @@ -510,10 +510,6 @@ type Config struct { // supported by the implementation. CipherSuites []uint16 - // TLS13CipherSuites is a list of supported cipher suites to be used in - // TLS 1.3. If nil, uses a list of suites supported by the implementation. - TLS13CipherSuites []uint16 - // PreferServerCipherSuites controls whether the server selects the // client's most preferred ciphersuite, or the server's most preferred // ciphersuite. If true then the server's preference, as expressed in @@ -659,7 +655,6 @@ func (c *Config) Clone() *Config { ClientCAs: c.ClientCAs, InsecureSkipVerify: c.InsecureSkipVerify, CipherSuites: c.CipherSuites, - TLS13CipherSuites: c.TLS13CipherSuites, PreferServerCipherSuites: c.PreferServerCipherSuites, SessionTicketsDisabled: c.SessionTicketsDisabled, SessionTicketKey: c.SessionTicketKey, @@ -756,17 +751,30 @@ func (c *Config) time() time.Time { return t() } -func (c *Config) cipherSuites(version uint16) []uint16 { - if version >= VersionTLS13 { - s := c.TLS13CipherSuites - if s == nil { - s = defaultTLS13CipherSuites() +func hasOverlappingCipherSuites(cs1, cs2 []uint16) bool { + for _, c1 := range cs1 { + for _, c2 := range cs2 { + if c1 == c2 { + return true + } } - return s } + return false +} + +func (c *Config) cipherSuites() []uint16 { s := c.CipherSuites if s == nil { s = defaultCipherSuites() + } else if c.maxVersion() >= VersionTLS13 { + // Ensure that TLS 1.3 suites are always present, but respect + // the application cipher suite preferences. + s13 := defaultTLS13CipherSuites() + if !hasOverlappingCipherSuites(s, s13) { + allSuites := make([]uint16, len(s13)+len(s)) + allSuites = append(allSuites, s13...) + s = append(allSuites, s...) + } } return s } @@ -1089,9 +1097,7 @@ func initDefaultCipherSuites() { } varDefaultTLS13CipherSuites = make([]uint16, 0, len(cipherSuites)) - for _, topCipher := range topTLS13CipherSuites { - varDefaultTLS13CipherSuites = append(varDefaultTLS13CipherSuites, topCipher) - } + varDefaultTLS13CipherSuites = append(varDefaultTLS13CipherSuites, topTLS13CipherSuites...) varDefaultCipherSuites = make([]uint16, 0, len(cipherSuites)) varDefaultCipherSuites = append(varDefaultCipherSuites, topCipherSuites...) @@ -1116,6 +1122,7 @@ NextCipherSuite: varDefaultCipherSuites = append(varDefaultCipherSuites, suite.id) } } + varDefaultCipherSuites = append(varDefaultTLS13CipherSuites, varDefaultCipherSuites...) } func unexpectedMessageError(wanted, got interface{}) error { diff --git a/handshake_client.go b/handshake_client.go index 4dce059..096471c 100644 --- a/handshake_client.go +++ b/handshake_client.go @@ -74,7 +74,7 @@ func (c *Conn) clientHandshake() error { hello.secureRenegotiation = c.clientFinished[:] } - possibleCipherSuites := c.config.cipherSuites(c.vers) + possibleCipherSuites := c.config.cipherSuites() hello.cipherSuites = make([]uint16, 0, len(possibleCipherSuites)) NextCipherSuite: diff --git a/handshake_server.go b/handshake_server.go index 0649336..5799ee7 100644 --- a/handshake_server.go +++ b/handshake_server.go @@ -321,11 +321,11 @@ Curves: var preferenceList, supportedList []uint16 if c.config.PreferServerCipherSuites { - preferenceList = c.config.cipherSuites(c.vers) + preferenceList = c.config.cipherSuites() supportedList = hs.clientHello.cipherSuites } else { preferenceList = hs.clientHello.cipherSuites - supportedList = c.config.cipherSuites(c.vers) + supportedList = c.config.cipherSuites() } for _, id := range preferenceList { @@ -387,7 +387,7 @@ func (hs *serverHandshakeState) checkForResumption() bool { } // Check that we also support the ciphersuite from the session. - if !hs.setCipherSuite(hs.sessionState.cipherSuite, c.config.cipherSuites(c.vers), hs.sessionState.vers) { + if !hs.setCipherSuite(hs.sessionState.cipherSuite, c.config.cipherSuites(), hs.sessionState.vers) { return false } diff --git a/handshake_server_test.go b/handshake_server_test.go index 5bdf707..d893414 100644 --- a/handshake_server_test.go +++ b/handshake_server_test.go @@ -49,18 +49,6 @@ func allCipherSuites() []uint16 { return ids } -func allTLS13CipherSuites() []uint16 { - var ids []uint16 - for _, suite := range cipherSuites { - if suite.flags&suiteTLS13 == 0 { - continue - } - ids = append(ids, suite.id) - } - - return ids -} - func init() { testConfig = &Config{ Time: func() time.Time { return time.Unix(0, 0) }, @@ -70,7 +58,6 @@ func init() { MinVersion: VersionSSL30, MaxVersion: VersionTLS12, CipherSuites: allCipherSuites(), - TLS13CipherSuites: allTLS13CipherSuites(), } testConfig.Certificates[0].Certificate = [][]byte{testRSACertificate} testConfig.Certificates[0].PrivateKey = testRSAPrivateKey diff --git a/tls_test.go b/tls_test.go index b9b3bf9..565b6ec 100644 --- a/tls_test.go +++ b/tls_test.go @@ -648,7 +648,6 @@ func TestCloneNonFuncFields(t *testing.T) { case "SessionTicketKey": f.Set(reflect.ValueOf([32]byte{})) case "CipherSuites": - case "TLS13CipherSuites": f.Set(reflect.ValueOf([]uint16{1, 2})) case "CurvePreferences": f.Set(reflect.ValueOf([]CurveID{CurveP256}))