runner: Store a cipherSuite object in sessionState We can avoid a bunch of needless error-handling if the ticket parser ensures sessionState always has a valid cipher ID. Change-Id: I824b35ad507dea7d303c02037a1b246af9b54eee Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/89467 Reviewed-by: Lily Chen <chlily@google.com> Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/ssl/test/runner/conn.go b/ssl/test/runner/conn.go index 6f11a56..ed00027 100644 --- a/ssl/test/runner/conn.go +++ b/ssl/test/runner/conn.go
@@ -2068,7 +2068,7 @@ state := sessionState{ vers: c.vers, - cipherSuite: c.cipherSuite.id, + cipherSuite: c.cipherSuite, secret: deriveSessionPSK(c.cipherSuite, c.wireVersion, c.resumptionSecret, nonce, c.isDTLS), certificates: peerCertificatesRaw, ticketCreationTime: c.config.time(),
diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go index 6cc0b00..0f0feab 100644 --- a/ssl/test/runner/handshake_server.go +++ b/ssl/test/runner/handshake_server.go
@@ -612,14 +612,9 @@ } if !config.Bugs.AcceptAnySession { - if sessionState.vers != c.vers { - continue - } - if sessionState.ticketExpiration.Before(c.config.time()) { - continue - } - sessionCipher := cipherSuiteFromID(sessionState.cipherSuite) - if sessionCipher == nil || sessionCipher.hash() != hs.suite.hash() { + if sessionState.vers != c.vers || + sessionState.ticketExpiration.Before(c.config.time()) || + sessionState.cipherSuite.hash() != hs.suite.hash() { continue } } @@ -914,7 +909,7 @@ // Decide whether or not to accept early data. if !sendHelloRetryRequest && hs.clientHello.hasEarlyData { if !config.Bugs.AlwaysRejectEarlyData && hs.sessionState != nil { - if hs.sessionState.cipherSuite == hs.suite.id && + if hs.sessionState.cipherSuite.id == hs.suite.id && c.clientProtocol == string(hs.sessionState.earlyALPN) && c.hasApplicationSettings == hs.sessionState.hasApplicationSettings && bytes.Equal(c.localApplicationSettings, hs.sessionState.localApplicationSettings) && @@ -938,8 +933,7 @@ encryptedExtensions.extensions.applicationSettingsOld = nil } - sessionCipher := cipherSuiteFromID(hs.sessionState.cipherSuite) - if err := c.useInTrafficSecret(uint16(encryptionEarlyData), c.wireVersion, sessionCipher, earlyTrafficSecret); err != nil { + if err := c.useInTrafficSecret(uint16(encryptionEarlyData), c.wireVersion, hs.sessionState.cipherSuite, earlyTrafficSecret); err != nil { return err } @@ -1803,21 +1797,18 @@ if c.config.Bugs.AcceptAnySession { // Replace the cipher suite with one known to work, to test // cross-version resumption attempts. - hs.sessionState.cipherSuite = TLS_RSA_WITH_AES_128_CBC_SHA + hs.sessionState.cipherSuite = cipherSuiteFromID(TLS_RSA_WITH_AES_128_CBC_SHA) } else { - // Never resume a session for a different SSL version. - if c.vers != hs.sessionState.vers { - return false - } - - // Check that the client is still offering the ciphersuite in the session. - if !slices.Contains(hs.clientHello.cipherSuites, hs.sessionState.cipherSuite) { + // Never resume a session for a different SSL version, and check + // that the client is still offering the ciphersuite in the session. + if c.vers != hs.sessionState.vers || + !slices.Contains(hs.clientHello.cipherSuites, hs.sessionState.cipherSuite.id) { return false } } // Check that we also support the ciphersuite from the session. - hs.suite = c.tryCipherSuite(hs.sessionState.cipherSuite, c.config.cipherSuites(), c.vers, hs.ellipticOk, hs.ecdsaOk) + hs.suite = c.tryCipherSuite(hs.sessionState.cipherSuite.id, c.config.cipherSuites(), c.vers, hs.ellipticOk, hs.ecdsaOk) if hs.suite == nil { return false @@ -2178,7 +2169,7 @@ c := hs.c state := sessionState{ vers: c.vers, - cipherSuite: hs.suite.id, + cipherSuite: hs.suite, secret: hs.masterSecret, certificates: hs.certsFromClient, handshakeHash: hs.finishedHash.Sum(), @@ -2403,12 +2394,7 @@ truncatedHello := clientHello.marshal() truncatedHello = truncatedHello[:len(truncatedHello)-binderLen] - pskCipherSuite := cipherSuiteFromID(sessionState.cipherSuite) - if pskCipherSuite == nil { - return errors.New("tls: Unknown cipher suite for PSK in session") - } - - binder := computePSKBinder(sessionState.secret, version, isDTLS, resumptionPSKBinderLabel, pskCipherSuite.hash(), firstClientHello, helloRetryRequest, truncatedHello) + binder := computePSKBinder(sessionState.secret, version, isDTLS, resumptionPSKBinderLabel, sessionState.cipherSuite.hash(), firstClientHello, helloRetryRequest, truncatedHello) if !bytes.Equal(binder, binderToVerify) { return errors.New("tls: PSK binder does not verify") }
diff --git a/ssl/test/runner/ticket.go b/ssl/test/runner/ticket.go index 51842d1..ec345a9 100644 --- a/ssl/test/runner/ticket.go +++ b/ssl/test/runner/ticket.go
@@ -21,7 +21,7 @@ // ticket in order to later resume a connection. type sessionState struct { vers uint16 - cipherSuite uint16 + cipherSuite *cipherSuite secret []byte handshakeHash []byte certificates [][]byte @@ -42,7 +42,7 @@ func (s *sessionState) marshal() []byte { msg := cryptobyte.NewBuilder(nil) msg.AddUint16(s.vers) - msg.AddUint16(s.cipherSuite) + msg.AddUint16(s.cipherSuite.id) addUint16LengthPrefixedBytes(msg, s.secret) addUint16LengthPrefixedBytes(msg, s.handshakeHash) msg.AddUint16(uint16(len(s.certificates))) @@ -102,15 +102,20 @@ func (s *sessionState) unmarshal(data []byte) bool { reader := cryptobyte.String(data) - var numCerts uint16 + var numCerts, cipherSuite uint16 if !reader.ReadUint16(&s.vers) || - !reader.ReadUint16(&s.cipherSuite) || + !reader.ReadUint16(&cipherSuite) || !readUint16LengthPrefixedBytes(&reader, &s.secret) || !readUint16LengthPrefixedBytes(&reader, &s.handshakeHash) || !reader.ReadUint16(&numCerts) { return false } + s.cipherSuite = cipherSuiteFromID(cipherSuite) + if s.cipherSuite == nil { + return false + } + s.certificates = make([][]byte, int(numCerts)) for i := range s.certificates { if !readUint24LengthPrefixedBytes(&reader, &s.certificates[i]) {