From 1eb7ca924b184d06706cee78cf56d022ebb1fe5a Mon Sep 17 00:00:00 2001 From: Adam Langley Date: Mon, 28 Nov 2011 15:34:16 -0500 Subject: [PATCH] crypto/tls: don't rely on map iteration order. Previously we were using the map iteration order to set the order of the cipher suites in the ClientHello. R=bradfitz CC=golang-dev https://golang.org/cl/5440048 --- src/pkg/crypto/tls/cipher_suites.go | 28 ++++++++++++++++---------- src/pkg/crypto/tls/common.go | 6 ++---- src/pkg/crypto/tls/handshake_client.go | 4 ++-- src/pkg/crypto/tls/handshake_server.go | 17 +++++++++++----- 4 files changed, 33 insertions(+), 22 deletions(-) diff --git a/src/pkg/crypto/tls/cipher_suites.go b/src/pkg/crypto/tls/cipher_suites.go index 1134f362583..ff0ff2b09a1 100644 --- a/src/pkg/crypto/tls/cipher_suites.go +++ b/src/pkg/crypto/tls/cipher_suites.go @@ -37,6 +37,7 @@ type keyAgreement interface { // A cipherSuite is a specific combination of key agreement, cipher and MAC // function. All cipher suites currently assume RSA key agreement. type cipherSuite struct { + id uint16 // the lengths, in bytes, of the key material needed for each component. keyLen int macLen int @@ -50,13 +51,13 @@ type cipherSuite struct { mac func(version uint16, macKey []byte) macFunction } -var cipherSuites = map[uint16]*cipherSuite{ - TLS_RSA_WITH_RC4_128_SHA: &cipherSuite{16, 20, 0, rsaKA, false, cipherRC4, macSHA1}, - TLS_RSA_WITH_3DES_EDE_CBC_SHA: &cipherSuite{24, 20, 8, rsaKA, false, cipher3DES, macSHA1}, - TLS_RSA_WITH_AES_128_CBC_SHA: &cipherSuite{16, 20, 16, rsaKA, false, cipherAES, macSHA1}, - TLS_ECDHE_RSA_WITH_RC4_128_SHA: &cipherSuite{16, 20, 0, ecdheRSAKA, true, cipherRC4, macSHA1}, - TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA: &cipherSuite{24, 20, 8, ecdheRSAKA, true, cipher3DES, macSHA1}, - TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA: &cipherSuite{16, 20, 16, ecdheRSAKA, true, cipherAES, macSHA1}, +var cipherSuites = []*cipherSuite{ + &cipherSuite{TLS_RSA_WITH_RC4_128_SHA, 16, 20, 0, rsaKA, false, cipherRC4, macSHA1}, + &cipherSuite{TLS_RSA_WITH_3DES_EDE_CBC_SHA, 24, 20, 8, rsaKA, false, cipher3DES, macSHA1}, + &cipherSuite{TLS_RSA_WITH_AES_128_CBC_SHA, 16, 20, 16, rsaKA, false, cipherAES, macSHA1}, + &cipherSuite{TLS_ECDHE_RSA_WITH_RC4_128_SHA, 16, 20, 0, ecdheRSAKA, true, cipherRC4, macSHA1}, + &cipherSuite{TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA, 24, 20, 8, ecdheRSAKA, true, cipher3DES, macSHA1}, + &cipherSuite{TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, 16, 20, 16, ecdheRSAKA, true, cipherAES, macSHA1}, } func cipherRC4(key, iv []byte, isRead bool) interface{} { @@ -159,15 +160,20 @@ func ecdheRSAKA() keyAgreement { return new(ecdheRSAKeyAgreement) } -// mutualCipherSuite returns a cipherSuite and its id given a list of supported +// mutualCipherSuite returns a cipherSuite given a list of supported // ciphersuites and the id requested by the peer. -func mutualCipherSuite(have []uint16, want uint16) (suite *cipherSuite, id uint16) { +func mutualCipherSuite(have []uint16, want uint16) *cipherSuite { for _, id := range have { if id == want { - return cipherSuites[id], id + for _, suite := range cipherSuites { + if suite.id == want { + return suite + } + } + return nil } } - return + return nil } // A list of the possible cipher suite ids. Taken from diff --git a/src/pkg/crypto/tls/common.go b/src/pkg/crypto/tls/common.go index ea520859b82..e9042486828 100644 --- a/src/pkg/crypto/tls/common.go +++ b/src/pkg/crypto/tls/common.go @@ -315,9 +315,7 @@ var ( func initDefaultCipherSuites() { varDefaultCipherSuites = make([]uint16, len(cipherSuites)) - i := 0 - for id := range cipherSuites { - varDefaultCipherSuites[i] = id - i++ + for i, suite := range cipherSuites { + varDefaultCipherSuites[i] = suite.id } } diff --git a/src/pkg/crypto/tls/handshake_client.go b/src/pkg/crypto/tls/handshake_client.go index aed991ccd1b..0f41008bf43 100644 --- a/src/pkg/crypto/tls/handshake_client.go +++ b/src/pkg/crypto/tls/handshake_client.go @@ -72,7 +72,7 @@ func (c *Conn) clientHandshake() error { return errors.New("server advertised unrequested NPN") } - suite, suiteId := mutualCipherSuite(c.config.cipherSuites(), serverHello.cipherSuite) + suite := mutualCipherSuite(c.config.cipherSuites(), serverHello.cipherSuite) if suite == nil { return c.sendAlert(alertHandshakeFailure) } @@ -292,7 +292,7 @@ func (c *Conn) clientHandshake() error { } c.handshakeComplete = true - c.cipherSuite = suiteId + c.cipherSuite = suite.id return nil } diff --git a/src/pkg/crypto/tls/handshake_server.go b/src/pkg/crypto/tls/handshake_server.go index d5af084eda5..1fa4585189a 100644 --- a/src/pkg/crypto/tls/handshake_server.go +++ b/src/pkg/crypto/tls/handshake_server.go @@ -56,18 +56,25 @@ Curves: ellipticOk := supportedCurve && supportedPointFormat var suite *cipherSuite - var suiteId uint16 FindCipherSuite: for _, id := range clientHello.cipherSuites { for _, supported := range config.cipherSuites() { if id == supported { - suite = cipherSuites[id] + suite = nil + for _, s := range cipherSuites { + if s.id == id { + suite = s + break + } + } + if suite == nil { + continue + } // Don't select a ciphersuite which we can't // support for this client. if suite.elliptic && !ellipticOk { continue } - suiteId = id break FindCipherSuite } } @@ -87,7 +94,7 @@ FindCipherSuite: } hello.vers = vers - hello.cipherSuite = suiteId + hello.cipherSuite = suite.id t := uint32(config.time()) hello.random = make([]byte, 32) hello.random[0] = byte(t >> 24) @@ -296,7 +303,7 @@ FindCipherSuite: c.writeRecord(recordTypeHandshake, finished.marshal()) c.handshakeComplete = true - c.cipherSuite = suiteId + c.cipherSuite = suite.id return nil }