Commit f752484c authored by Adam Langley's avatar Adam Langley

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
parent 649a2a9b
......@@ -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},
......
......@@ -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)
......
......@@ -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
}
}
......
......@@ -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() {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment