Skip to content

Commit 9d0819b

Browse files
committed
crypto/tls: make cipher suite preference ordering automatic
We now have a (well, two, depending on AES hardware support) universal cipher suite preference order, based on their security and performance. Peer and application lists are now treated as filters (and AES hardware support hints) that are applied to this universal order. This removes a complex and nuanced decision from the application's responsibilities, one which we are better equipped to make and which applications usually don't need to have an opinion about. It also lets us worry less about what suites we support or enable, because we can be confident that bad ones won't be selected over good ones. This also moves 3DES suites to InsecureCipherSuites(), even if they are not disabled by default. Just because we can keep them as a last resort it doesn't mean they are secure. Thankfully we had not promised that Insecure means disabled by default. Notable test changes: - TestCipherSuiteCertPreferenceECDSA was testing that we'd pick the right certificate regardless of CipherSuite ordering, which is now completely ignored, as tested by TestCipherSuitePreference. Removed. - The openssl command of TestHandshakeServerExportKeyingMaterial was broken for TLS 1.0 in CL 262857, but its golden file was not regenerated, so the test kept passing. It now broke because the selected suite from the ones in the golden file changed. - In TestAESCipherReordering, "server strongly prefers AES-GCM" is removed because there is no way for a server to express a strong preference anymore; "client prefers AES-GCM and AES-CBC over ChaCha" switched to ChaCha20 when the server lacks AES hardware; and finally "client supports multiple AES-GCM" changed to always prefer AES-128 per the universal preference list. * this is going back on an explicit decision from CL 262857, and while that client order is weird and does suggest a strong dislike for ChaCha20, we have a strong dislike for software AES, so it didn't feel worth making the logic more complex - All Client-* golden files had to be regenerated because the ClientHello cipher suites have changed. (Even when Config.CipherSuites was limited to one suite, the TLS 1.3 default order changed.) Fixes #45430 Fixes #41476 (as 3DES is now always the last resort) Change-Id: If5f5d356c0f8d1f1c7542fb06644a478d6bad1e5 Reviewed-on: https://go-review.googlesource.com/c/go/+/314609 Run-TryBot: Filippo Valsorda <filippo@golang.org> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org> Trust: Filippo Valsorda <filippo@golang.org>
1 parent 02ce411 commit 9d0819b

File tree

68 files changed

+3754
-3815
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

68 files changed

+3754
-3815
lines changed

src/crypto/tls/cipher_suites.go

+216-43
Large diffs are not rendered by default.

src/crypto/tls/common.go

+16-145
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,8 @@ import (
1818
"crypto/x509"
1919
"errors"
2020
"fmt"
21-
"internal/cpu"
2221
"io"
2322
"net"
24-
"runtime"
25-
"sort"
2623
"strings"
2724
"sync"
2825
"time"
@@ -648,17 +645,21 @@ type Config struct {
648645
// testing or in combination with VerifyConnection or VerifyPeerCertificate.
649646
InsecureSkipVerify bool
650647

651-
// CipherSuites is a list of supported cipher suites for TLS versions up to
652-
// TLS 1.2. If CipherSuites is nil, a default list of secure cipher suites
653-
// is used, with a preference order based on hardware performance. The
654-
// default cipher suites might change over Go versions. Note that TLS 1.3
655-
// ciphersuites are not configurable.
648+
// CipherSuites is a list of enabled TLS 1.0–1.2 cipher suites. The order of
649+
// the list is ignored. Note that TLS 1.3 ciphersuites are not configurable.
650+
//
651+
// If CipherSuites is nil, a safe default list is used. The default cipher
652+
// suites might change over time.
656653
CipherSuites []uint16
657654

658-
// PreferServerCipherSuites controls whether the server selects the
659-
// client's most preferred ciphersuite, or the server's most preferred
660-
// ciphersuite. If true then the server's preference, as expressed in
661-
// the order of elements in CipherSuites, is used.
655+
// PreferServerCipherSuites is a legacy field and has no effect.
656+
//
657+
// It used to control whether the server would follow the client's or the
658+
// server's preference. Servers now select the best mutually supported
659+
// cipher suite based on logic that takes into account inferred client
660+
// hardware, server hardware, and security.
661+
//
662+
// Deprected: PreferServerCipherSuites is ignored.
662663
PreferServerCipherSuites bool
663664

664665
// SessionTicketsDisabled may be set to true to disable session ticket and
@@ -950,11 +951,10 @@ func (c *Config) time() time.Time {
950951
}
951952

952953
func (c *Config) cipherSuites() []uint16 {
953-
s := c.CipherSuites
954-
if s == nil {
955-
s = defaultCipherSuites()
954+
if c.CipherSuites != nil {
955+
return c.CipherSuites
956956
}
957-
return s
957+
return defaultCipherSuites
958958
}
959959

960960
var supportedVersions = []uint16{
@@ -1444,87 +1444,6 @@ func defaultConfig() *Config {
14441444
return &emptyConfig
14451445
}
14461446

1447-
var (
1448-
once sync.Once
1449-
varDefaultCipherSuites []uint16
1450-
varDefaultCipherSuitesTLS13 []uint16
1451-
)
1452-
1453-
func defaultCipherSuites() []uint16 {
1454-
once.Do(initDefaultCipherSuites)
1455-
return varDefaultCipherSuites
1456-
}
1457-
1458-
func defaultCipherSuitesTLS13() []uint16 {
1459-
once.Do(initDefaultCipherSuites)
1460-
return varDefaultCipherSuitesTLS13
1461-
}
1462-
1463-
var (
1464-
hasGCMAsmAMD64 = cpu.X86.HasAES && cpu.X86.HasPCLMULQDQ
1465-
hasGCMAsmARM64 = cpu.ARM64.HasAES && cpu.ARM64.HasPMULL
1466-
// Keep in sync with crypto/aes/cipher_s390x.go.
1467-
hasGCMAsmS390X = cpu.S390X.HasAES && cpu.S390X.HasAESCBC && cpu.S390X.HasAESCTR && (cpu.S390X.HasGHASH || cpu.S390X.HasAESGCM)
1468-
1469-
hasAESGCMHardwareSupport = runtime.GOARCH == "amd64" && hasGCMAsmAMD64 ||
1470-
runtime.GOARCH == "arm64" && hasGCMAsmARM64 ||
1471-
runtime.GOARCH == "s390x" && hasGCMAsmS390X
1472-
)
1473-
1474-
func initDefaultCipherSuites() {
1475-
var topCipherSuites []uint16
1476-
1477-
if hasAESGCMHardwareSupport {
1478-
// If AES-GCM hardware is provided then prioritise AES-GCM
1479-
// cipher suites.
1480-
topCipherSuites = []uint16{
1481-
TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
1482-
TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
1483-
TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
1484-
TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
1485-
TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,
1486-
TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305,
1487-
}
1488-
varDefaultCipherSuitesTLS13 = []uint16{
1489-
TLS_AES_128_GCM_SHA256,
1490-
TLS_CHACHA20_POLY1305_SHA256,
1491-
TLS_AES_256_GCM_SHA384,
1492-
}
1493-
} else {
1494-
// Without AES-GCM hardware, we put the ChaCha20-Poly1305
1495-
// cipher suites first.
1496-
topCipherSuites = []uint16{
1497-
TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,
1498-
TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305,
1499-
TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
1500-
TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
1501-
TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
1502-
TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
1503-
}
1504-
varDefaultCipherSuitesTLS13 = []uint16{
1505-
TLS_CHACHA20_POLY1305_SHA256,
1506-
TLS_AES_128_GCM_SHA256,
1507-
TLS_AES_256_GCM_SHA384,
1508-
}
1509-
}
1510-
1511-
varDefaultCipherSuites = make([]uint16, 0, len(cipherSuites))
1512-
varDefaultCipherSuites = append(varDefaultCipherSuites, topCipherSuites...)
1513-
1514-
NextCipherSuite:
1515-
for _, suite := range cipherSuites {
1516-
if suite.flags&suiteDefaultOff != 0 {
1517-
continue
1518-
}
1519-
for _, existing := range varDefaultCipherSuites {
1520-
if existing == suite.id {
1521-
continue NextCipherSuite
1522-
}
1523-
}
1524-
varDefaultCipherSuites = append(varDefaultCipherSuites, suite.id)
1525-
}
1526-
}
1527-
15281447
func unexpectedMessageError(wanted, got interface{}) error {
15291448
return fmt.Errorf("tls: received unexpected handshake message of type %T when waiting for %T", got, wanted)
15301449
}
@@ -1537,51 +1456,3 @@ func isSupportedSignatureAlgorithm(sigAlg SignatureScheme, supportedSignatureAlg
15371456
}
15381457
return false
15391458
}
1540-
1541-
var aesgcmCiphers = map[uint16]bool{
1542-
// 1.2
1543-
TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256: true,
1544-
TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384: true,
1545-
TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256: true,
1546-
TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384: true,
1547-
// 1.3
1548-
TLS_AES_128_GCM_SHA256: true,
1549-
TLS_AES_256_GCM_SHA384: true,
1550-
}
1551-
1552-
var nonAESGCMAEADCiphers = map[uint16]bool{
1553-
// 1.2
1554-
TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305: true,
1555-
TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305: true,
1556-
// 1.3
1557-
TLS_CHACHA20_POLY1305_SHA256: true,
1558-
}
1559-
1560-
// aesgcmPreferred returns whether the first valid cipher in the preference list
1561-
// is an AES-GCM cipher, implying the peer has hardware support for it.
1562-
func aesgcmPreferred(ciphers []uint16) bool {
1563-
for _, cID := range ciphers {
1564-
c := cipherSuiteByID(cID)
1565-
if c == nil {
1566-
c13 := cipherSuiteTLS13ByID(cID)
1567-
if c13 == nil {
1568-
continue
1569-
}
1570-
return aesgcmCiphers[cID]
1571-
}
1572-
return aesgcmCiphers[cID]
1573-
}
1574-
return false
1575-
}
1576-
1577-
// deprioritizeAES reorders cipher preference lists by rearranging
1578-
// adjacent AEAD ciphers such that AES-GCM based ciphers are moved
1579-
// after other AEAD ciphers. It returns a fresh slice.
1580-
func deprioritizeAES(ciphers []uint16) []uint16 {
1581-
reordered := make([]uint16, len(ciphers))
1582-
copy(reordered, ciphers)
1583-
sort.SliceStable(reordered, func(i, j int) bool {
1584-
return nonAESGCMAEADCiphers[reordered[i]] && aesgcmCiphers[reordered[j]]
1585-
})
1586-
return reordered
1587-
}

src/crypto/tls/handshake_client.go

+21-15
Original file line numberDiff line numberDiff line change
@@ -84,22 +84,24 @@ func (c *Conn) makeClientHello() (*clientHelloMsg, ecdheParameters, error) {
8484
hello.secureRenegotiation = c.clientFinished[:]
8585
}
8686

87-
possibleCipherSuites := config.cipherSuites()
88-
hello.cipherSuites = make([]uint16, 0, len(possibleCipherSuites))
87+
preferenceOrder := cipherSuitesPreferenceOrder
88+
if !hasAESGCMHardwareSupport {
89+
preferenceOrder = cipherSuitesPreferenceOrderNoAES
90+
}
91+
configCipherSuites := config.cipherSuites()
92+
hello.cipherSuites = make([]uint16, 0, len(configCipherSuites))
8993

90-
for _, suiteId := range possibleCipherSuites {
91-
for _, suite := range cipherSuites {
92-
if suite.id != suiteId {
93-
continue
94-
}
95-
// Don't advertise TLS 1.2-only cipher suites unless
96-
// we're attempting TLS 1.2.
97-
if hello.vers < VersionTLS12 && suite.flags&suiteTLS12 != 0 {
98-
break
99-
}
100-
hello.cipherSuites = append(hello.cipherSuites, suiteId)
101-
break
94+
for _, suiteId := range preferenceOrder {
95+
suite := mutualCipherSuite(configCipherSuites, suiteId)
96+
if suite == nil {
97+
continue
10298
}
99+
// Don't advertise TLS 1.2-only cipher suites unless
100+
// we're attempting TLS 1.2.
101+
if hello.vers < VersionTLS12 && suite.flags&suiteTLS12 != 0 {
102+
continue
103+
}
104+
hello.cipherSuites = append(hello.cipherSuites, suiteId)
103105
}
104106

105107
_, err := io.ReadFull(config.rand(), hello.random)
@@ -120,7 +122,11 @@ func (c *Conn) makeClientHello() (*clientHelloMsg, ecdheParameters, error) {
120122

121123
var params ecdheParameters
122124
if hello.supportedVersions[0] == VersionTLS13 {
123-
hello.cipherSuites = append(hello.cipherSuites, defaultCipherSuitesTLS13()...)
125+
if hasAESGCMHardwareSupport {
126+
hello.cipherSuites = append(hello.cipherSuites, defaultCipherSuitesTLS13...)
127+
} else {
128+
hello.cipherSuites = append(hello.cipherSuites, defaultCipherSuitesTLS13NoAES...)
129+
}
124130

125131
curveID := config.curvePreferences()[0]
126132
if _, ok := curveForCurveID(curveID); curveID != X25519 && !ok {

src/crypto/tls/handshake_server.go

+14-21
Original file line numberDiff line numberDiff line change
@@ -302,30 +302,23 @@ func supportsECDHE(c *Config, supportedCurves []CurveID, supportedPoints []uint8
302302
func (hs *serverHandshakeState) pickCipherSuite() error {
303303
c := hs.c
304304

305-
var preferenceList, supportedList []uint16
306-
if c.config.PreferServerCipherSuites {
307-
preferenceList = c.config.cipherSuites()
308-
supportedList = hs.clientHello.cipherSuites
309-
310-
// If the client does not seem to have hardware support for AES-GCM,
311-
// and the application did not specify a cipher suite preference order,
312-
// prefer other AEAD ciphers even if we prioritized AES-GCM ciphers
313-
// by default.
314-
if c.config.CipherSuites == nil && !aesgcmPreferred(hs.clientHello.cipherSuites) {
315-
preferenceList = deprioritizeAES(preferenceList)
316-
}
317-
} else {
318-
preferenceList = hs.clientHello.cipherSuites
319-
supportedList = c.config.cipherSuites()
320-
321-
// If we don't have hardware support for AES-GCM, prefer other AEAD
322-
// ciphers even if the client prioritized AES-GCM.
323-
if !hasAESGCMHardwareSupport {
324-
preferenceList = deprioritizeAES(preferenceList)
305+
preferenceOrder := cipherSuitesPreferenceOrder
306+
if !hasAESGCMHardwareSupport || !aesgcmPreferred(hs.clientHello.cipherSuites) {
307+
preferenceOrder = cipherSuitesPreferenceOrderNoAES
308+
}
309+
310+
configCipherSuites := c.config.cipherSuites()
311+
preferenceList := make([]uint16, 0, len(configCipherSuites))
312+
for _, suiteID := range preferenceOrder {
313+
for _, id := range configCipherSuites {
314+
if id == suiteID {
315+
preferenceList = append(preferenceList, id)
316+
break
317+
}
325318
}
326319
}
327320

328-
hs.suite = selectCipherSuite(preferenceList, supportedList, hs.cipherSuiteOk)
321+
hs.suite = selectCipherSuite(preferenceList, hs.clientHello.cipherSuites, hs.cipherSuiteOk)
329322
if hs.suite == nil {
330323
c.sendAlert(alertHandshakeFailure)
331324
return errors.New("tls: no cipher suite supported by both client and server")

0 commit comments

Comments
 (0)