From 493b9859917ca5a8f3586995646b8dfa9f81500f Mon Sep 17 00:00:00 2001 From: Adam Langley Date: Thu, 26 Sep 2013 17:09:56 -0400 Subject: [PATCH] crypto/tls: don't select TLS 1.2 cipher suites in prior versions. AES-GCM cipher suites are only defined for TLS 1.2, although there's nothing really version specific about them. However, development versions of NSS (meaning Firefox and Chrome) have an issue where they'll advertise TLS 1.2-only cipher suites in a TLS 1.1 ClientHello but then balk when the server selects one. This change causes Go clients not to advertise TLS 1.2 cipher suites unless TLS 1.2 is being used, and prevents servers from selecting them unless TLS 1.2 has been negotiated. https://code.google.com/p/chromium/issues/detail?id=297151 https://bugzilla.mozilla.org/show_bug.cgi?id=919677 R=golang-dev, rsc CC=golang-dev https://golang.org/cl/13573047 --- cipher_suites.go | 7 ++++-- handshake_client.go | 20 ++++++++++++++++- handshake_server.go | 9 +++++--- handshake_server_test.go | 47 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 77 insertions(+), 6 deletions(-) diff --git a/cipher_suites.go b/cipher_suites.go index 9a95cf8..39a5145 100644 --- a/cipher_suites.go +++ b/cipher_suites.go @@ -45,6 +45,9 @@ const ( // certificate is ECDSA. If this is not set then the cipher suite is // RSA based. suiteECDSA + // suiteTLS12 indicates that the cipher suite should only be advertised + // and accepted when using TLS 1.2. + suiteTLS12 ) // A cipherSuite is a specific combination of key agreement, cipher and MAC @@ -66,8 +69,8 @@ type cipherSuite struct { var cipherSuites = []*cipherSuite{ // Ciphersuite order is chosen so that ECDHE comes before plain RSA // and RC4 comes before AES (because of the Lucky13 attack). - {TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, 16, 0, 4, ecdheRSAKA, suiteECDHE, nil, nil, aeadAESGCM}, - {TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, 16, 0, 4, ecdheECDSAKA, suiteECDHE | suiteECDSA, nil, nil, aeadAESGCM}, + {TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, 16, 0, 4, ecdheRSAKA, suiteECDHE | suiteTLS12, nil, nil, aeadAESGCM}, + {TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, 16, 0, 4, ecdheECDSAKA, suiteECDHE | suiteECDSA | suiteTLS12, nil, nil, aeadAESGCM}, {TLS_ECDHE_RSA_WITH_RC4_128_SHA, 16, 20, 0, ecdheRSAKA, suiteECDHE, cipherRC4, macSHA1, nil}, {TLS_ECDHE_ECDSA_WITH_RC4_128_SHA, 16, 20, 0, ecdheECDSAKA, suiteECDHE | suiteECDSA, cipherRC4, macSHA1, nil}, {TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, 16, 20, 16, ecdheRSAKA, suiteECDHE, cipherAES, macSHA1, nil}, diff --git a/handshake_client.go b/handshake_client.go index bfd70a2..9cd57c5 100644 --- a/handshake_client.go +++ b/handshake_client.go @@ -23,7 +23,6 @@ func (c *Conn) clientHandshake() error { hello := &clientHelloMsg{ vers: c.config.maxVersion(), - cipherSuites: c.config.cipherSuites(), compressionMethods: []uint8{compressionNone}, random: make([]byte, 32), ocspStapling: true, @@ -33,6 +32,25 @@ func (c *Conn) clientHandshake() error { nextProtoNeg: len(c.config.NextProtos) > 0, } + possibleCipherSuites := c.config.cipherSuites() + hello.cipherSuites = make([]uint16, 0, len(possibleCipherSuites)) + +NextCipherSuite: + for _, suiteId := range possibleCipherSuites { + for _, suite := range cipherSuites { + if suite.id != suiteId { + continue + } + // Don't advertise TLS 1.2-only cipher suites unless + // we're attempting TLS 1.2. + if hello.vers < VersionTLS12 && suite.flags&suiteTLS12 != 0 { + continue + } + hello.cipherSuites = append(hello.cipherSuites, suiteId) + continue NextCipherSuite + } + } + t := uint32(c.config.time().Unix()) hello.random[0] = byte(t >> 24) hello.random[1] = byte(t >> 16) diff --git a/handshake_server.go b/handshake_server.go index 7c31855..1db10f2 100644 --- a/handshake_server.go +++ b/handshake_server.go @@ -193,7 +193,7 @@ Curves: } for _, id := range preferenceList { - if hs.suite = c.tryCipherSuite(id, supportedList, hs.ellipticOk, hs.ecdsaOk); hs.suite != nil { + if hs.suite = c.tryCipherSuite(id, supportedList, c.vers, hs.ellipticOk, hs.ecdsaOk); hs.suite != nil { break } } @@ -234,7 +234,7 @@ func (hs *serverHandshakeState) checkForResumption() bool { } // Check that we also support the ciphersuite from the session. - hs.suite = c.tryCipherSuite(hs.sessionState.cipherSuite, c.config.cipherSuites(), hs.ellipticOk, hs.ecdsaOk) + hs.suite = c.tryCipherSuite(hs.sessionState.cipherSuite, c.config.cipherSuites(), hs.sessionState.vers, hs.ellipticOk, hs.ecdsaOk) if hs.suite == nil { return false } @@ -605,7 +605,7 @@ func (hs *serverHandshakeState) processCertsFromClient(certificates [][]byte) (c // tryCipherSuite returns a cipherSuite with the given id if that cipher suite // is acceptable to use. -func (c *Conn) tryCipherSuite(id uint16, supportedCipherSuites []uint16, ellipticOk, ecdsaOk bool) *cipherSuite { +func (c *Conn) tryCipherSuite(id uint16, supportedCipherSuites []uint16, version uint16, ellipticOk, ecdsaOk bool) *cipherSuite { for _, supported := range supportedCipherSuites { if id == supported { var candidate *cipherSuite @@ -627,6 +627,9 @@ func (c *Conn) tryCipherSuite(id uint16, supportedCipherSuites []uint16, ellipti if (candidate.flags&suiteECDSA != 0) != ecdsaOk { continue } + if version < VersionTLS12 && candidate.flags&suiteTLS12 != 0 { + continue + } return candidate } } diff --git a/handshake_server_test.go b/handshake_server_test.go index 31bcc78..c08eba7 100644 --- a/handshake_server_test.go +++ b/handshake_server_test.go @@ -104,6 +104,53 @@ func TestNoCompressionOverlap(t *testing.T) { testClientHelloFailure(t, clientHello, alertHandshakeFailure) } +func TestTLS12OnlyCipherSuites(t *testing.T) { + // Test that a Server doesn't select a TLS 1.2-only cipher suite when + // the client negotiates TLS 1.1. + var zeros [32]byte + + clientHello := &clientHelloMsg{ + vers: VersionTLS11, + random: zeros[:], + cipherSuites: []uint16{ + // The Server, by default, will use the client's + // preference order. So the GCM cipher suite + // will be selected unless it's excluded because + // of the version in this ClientHello. + TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, + TLS_RSA_WITH_RC4_128_SHA, + }, + compressionMethods: []uint8{compressionNone}, + supportedCurves: []uint16{curveP256, curveP384, curveP521}, + supportedPoints: []uint8{pointFormatUncompressed}, + } + + c, s := net.Pipe() + var reply interface{} + var clientErr error + go func() { + cli := Client(c, testConfig) + cli.vers = clientHello.vers + cli.writeRecord(recordTypeHandshake, clientHello.marshal()) + reply, clientErr = cli.readHandshake() + c.Close() + }() + config := *testConfig + config.CipherSuites = clientHello.cipherSuites + Server(s, &config).Handshake() + s.Close() + if clientErr != nil { + t.Fatal(clientErr) + } + serverHello, ok := reply.(*serverHelloMsg) + if !ok { + t.Fatalf("didn't get ServerHello message in reply. Got %v\n", reply) + } + if s := serverHello.cipherSuite; s != TLS_RSA_WITH_RC4_128_SHA { + t.Fatalf("bad cipher suite from server: %x", s) + } +} + func TestAlertForwarding(t *testing.T) { c, s := net.Pipe() go func() {