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>

ssl/test/runner/conn.go

@@ -734,13 +734,17 @@ func (hc *halfConn) splitBlock(b *block, n int) (*block, *block) {
 	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 @@ Again:
 			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 @@ func (c *Conn) handlePostHandshakeMessage() error {
 		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 @@ func (c *Conn) ReadKeyUpdateACK() error {
 		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 {

ssl/test/runner/handshake_client.go

@@ -754,7 +754,9 @@ func (hs *clientHandshakeState) doTLS13Handshake() error {
 	// 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 @@ func (hs *clientHandshakeState) doTLS13Handshake() error {
 
 	// 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.

ssl/test/runner/handshake_server.go

@@ -648,8 +648,7 @@ ResendHelloRetryRequest:
 		// 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 @@ ResendHelloRetryRequest:
 		}
 		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 @@ ResendHelloRetryRequest:
 	}
 
 	// 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 @@ ResendHelloRetryRequest:
 	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)