runner: Check c.hand before changing ciphers.
This doesn't matter in so far as runner is not a real TLS
implementation, but it should enforce what there is to enforce just to
keep BoringSSL honest.
Bug: 80
Change-Id: I68940c33712d34a2437dc4dee31342e7f0f57c23
Reviewed-on: https://boringssl-review.googlesource.com/22069
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/ssl/test/runner/conn.go b/ssl/test/runner/conn.go
index 24edea9..d6f1ce6 100644
--- a/ssl/test/runner/conn.go
+++ b/ssl/test/runner/conn.go
@@ -734,13 +734,17 @@
return b, bb
}
-func (c *Conn) useInTrafficSecret(version uint16, suite *cipherSuite, secret []byte) {
+func (c *Conn) useInTrafficSecret(version uint16, suite *cipherSuite, secret []byte) error {
+ if c.hand.Len() != 0 {
+ return c.in.setErrorLocked(errors.New("tls: buffered handshake messages on cipher change"))
+ }
side := serverWrite
if !c.isClient {
side = clientWrite
}
c.in.useTrafficSecret(version, suite, secret, side)
c.seenHandshakePackEnd = false
+ return nil
}
func (c *Conn) useOutTrafficSecret(version uint16, suite *cipherSuite, secret []byte) {
@@ -961,8 +965,11 @@
break
}
if !isResumptionExperiment(c.wireVersion) {
- err := c.in.changeCipherSpec(c.config)
- if err != nil {
+ if c.hand.Len() != 0 {
+ c.in.setErrorLocked(errors.New("tls: buffered handshake messages on cipher change"))
+ break
+ }
+ if err := c.in.changeCipherSpec(c.config); err != nil {
c.in.setErrorLocked(c.sendAlert(err.(alert)))
}
}
@@ -1547,7 +1554,9 @@
if c.config.Bugs.RejectUnsolicitedKeyUpdate {
return errors.New("tls: unexpected KeyUpdate message")
}
- c.useInTrafficSecret(c.in.wireVersion, c.cipherSuite, updateTrafficSecret(c.cipherSuite.hash(), c.in.trafficSecret))
+ if err := c.useInTrafficSecret(c.in.wireVersion, c.cipherSuite, updateTrafficSecret(c.cipherSuite.hash(), c.in.trafficSecret)); err != nil {
+ return err
+ }
if keyUpdate.keyUpdateRequest == keyUpdateRequested {
c.keyUpdateRequested = true
}
@@ -1579,8 +1588,7 @@
return errors.New("tls: received invalid KeyUpdate message")
}
- c.useInTrafficSecret(c.in.wireVersion, c.cipherSuite, updateTrafficSecret(c.cipherSuite.hash(), c.in.trafficSecret))
- return nil
+ return c.useInTrafficSecret(c.in.wireVersion, c.cipherSuite, updateTrafficSecret(c.cipherSuite.hash(), c.in.trafficSecret))
}
func (c *Conn) Renegotiate() error {
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go
index 3656c2b..30105a5 100644
--- a/ssl/test/runner/handshake_client.go
+++ b/ssl/test/runner/handshake_client.go
@@ -754,7 +754,9 @@
// traffic key.
clientHandshakeTrafficSecret := hs.finishedHash.deriveSecret(clientHandshakeTrafficLabel)
serverHandshakeTrafficSecret := hs.finishedHash.deriveSecret(serverHandshakeTrafficLabel)
- c.useInTrafficSecret(c.wireVersion, hs.suite, serverHandshakeTrafficSecret)
+ if err := c.useInTrafficSecret(c.wireVersion, hs.suite, serverHandshakeTrafficSecret); err != nil {
+ return err
+ }
msg, err := c.readHandshake()
if err != nil {
@@ -888,7 +890,9 @@
// Switch to application data keys on read. In particular, any alerts
// from the client certificate are read over these keys.
- c.useInTrafficSecret(c.wireVersion, hs.suite, serverTrafficSecret)
+ if err := c.useInTrafficSecret(c.wireVersion, hs.suite, serverTrafficSecret); err != nil {
+ return err
+ }
// If we're expecting 0.5-RTT messages from the server, read them
// now.
diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go
index 251e91f..f50772e 100644
--- a/ssl/test/runner/handshake_server.go
+++ b/ssl/test/runner/handshake_server.go
@@ -648,8 +648,7 @@
// AcceptAnyBinder is set. See https://crbug.com/115.
if hs.sessionState != nil && !config.Bugs.AcceptAnySession {
binderToVerify := newClientHello.pskBinders[pskIndex]
- err := verifyPSKBinder(newClientHello, hs.sessionState, binderToVerify, append(oldClientHelloBytes, helloRetryRequest.marshal()...))
- if err != nil {
+ if err := verifyPSKBinder(newClientHello, hs.sessionState, binderToVerify, append(oldClientHelloBytes, helloRetryRequest.marshal()...)); err != nil {
return err
}
}
@@ -664,7 +663,9 @@
}
if encryptedExtensions.extensions.hasEarlyData {
earlyTrafficSecret := hs.finishedHash.deriveSecret(earlyTrafficLabel)
- c.useInTrafficSecret(c.wireVersion, hs.suite, earlyTrafficSecret)
+ if err := c.useInTrafficSecret(c.wireVersion, hs.suite, earlyTrafficSecret); err != nil {
+ return err
+ }
for _, expectedMsg := range config.Bugs.ExpectEarlyData {
if err := c.readRecord(recordTypeApplicationData); err != nil {
@@ -928,7 +929,9 @@
}
// Switch input stream to handshake traffic keys.
- c.useInTrafficSecret(c.wireVersion, hs.suite, clientHandshakeTrafficSecret)
+ if err := c.useInTrafficSecret(c.wireVersion, hs.suite, clientHandshakeTrafficSecret); err != nil {
+ return err
+ }
// If we requested a client certificate, then the client must send a
// certificate message, even if it's empty.
@@ -1032,7 +1035,9 @@
hs.writeClientHash(clientFinished.marshal())
// Switch to application data keys on read.
- c.useInTrafficSecret(c.wireVersion, hs.suite, clientTrafficSecret)
+ if err := c.useInTrafficSecret(c.wireVersion, hs.suite, clientTrafficSecret); err != nil {
+ return err
+ }
c.cipherSuite = hs.suite
c.resumptionSecret = hs.finishedHash.deriveSecret(resumptionLabel)
