From: Adam Langley Date: Thu, 26 Sep 2013 21:09:56 +0000 (-0400) Subject: crypto/tls: don't select TLS 1.2 cipher suites in prior versions. X-Git-Tag: go1.2rc2~112 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=f752484c7493bd55e19174418f5cd5abcb46e0e4;p=gostls13.git 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 --- diff --git a/src/pkg/crypto/tls/cipher_suites.go b/src/pkg/crypto/tls/cipher_suites.go index 9a95cf82a9..39a51459d2 100644 --- a/src/pkg/crypto/tls/cipher_suites.go +++ b/src/pkg/crypto/tls/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/src/pkg/crypto/tls/handshake_client.go b/src/pkg/crypto/tls/handshake_client.go index bfd70a2ecc..9cd57c55f4 100644 --- a/src/pkg/crypto/tls/handshake_client.go +++ b/src/pkg/crypto/tls/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/src/pkg/crypto/tls/handshake_server.go b/src/pkg/crypto/tls/handshake_server.go index 7c318555ba..1db10f289d 100644 --- a/src/pkg/crypto/tls/handshake_server.go +++ b/src/pkg/crypto/tls/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/src/pkg/crypto/tls/handshake_server_test.go b/src/pkg/crypto/tls/handshake_server_test.go index 31bcc785f5..c08eba7f17 100644 --- a/src/pkg/crypto/tls/handshake_server_test.go +++ b/src/pkg/crypto/tls/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() {