Apply version/wire mapping at a higher layer in runner.

This is in preparation for implementing the version extension and is
probably what we should have done from the beginning as it makes
intolerance bugs simpler.

This means knobs like SendClientVersion and SendServerVersion deal with
the wire values while knobs like NegotiateVersion and MaxVersion deal
with logical versions. (This matches how the bugs have always worked.
SendFoo is just a weird post-processing bit on the handshake messages
while NegotiateVersion actually changes how BoGo behaves.)

BUG=90

Change-Id: I7f359d798d0899fa2742107fb3d854be19e731a4
Reviewed-on: https://boringssl-review.googlesource.com/11300
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

6 files changed, 43 insertions, 44 deletions

diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index 02ab23b0..7aa984e7 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -597,8 +597,8 @@ type ProtocolBugs struct {
 	// send a NewSessionTicket message during an abbreviated handshake.
 	RenewTicketOnResume bool
 
-	// SendClientVersion, if non-zero, causes the client to send a different
-	// TLS version in the ClientHello than the maximum supported version.
+	// SendClientVersion, if non-zero, causes the client to send the
+	// specified value in the ClientHello version field.
 	SendClientVersion uint16
 
 	// NegotiateVersion, if non-zero, causes the server to negotiate the
@@ -1040,7 +1040,7 @@ type ProtocolBugs struct {
 	SecondHelloRetryRequest bool
 
 	// SendServerHelloVersion, if non-zero, causes the server to send the
-	// specified version in ServerHello rather than the true version.
+	// specified value in ServerHello version field.
 	SendServerHelloVersion uint16
 
 	// SkipHelloRetryRequest, if true, causes the TLS 1.3 server to not send
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go
index 05cf0c27..bdddfb42 100644
--- a/ssl/test/runner/handshake_client.go
+++ b/ssl/test/runner/handshake_client.go
@@ -57,9 +57,10 @@ func (c *Conn) clientHandshake() error {
 		return errors.New("tls: NextProtos values too large")
 	}
 
+	maxVersion := c.config.maxVersion(c.isDTLS)
 	hello := &clientHelloMsg{
 		isDTLS:                  c.isDTLS,
-		vers:                    c.config.maxVersion(c.isDTLS),
+		vers:                    versionToWire(maxVersion, c.isDTLS),
 		compressionMethods:      []uint8{compressionNone},
 		random:                  make([]byte, 32),
 		ocspStapling:            true,
@@ -110,7 +111,7 @@ func (c *Conn) clientHandshake() error {
 	}
 
 	var keyShares map[CurveID]ecdhCurve
-	if hello.vers >= VersionTLS13 {
+	if maxVersion >= VersionTLS13 {
 		keyShares = make(map[CurveID]ecdhCurve)
 		hello.hasKeyShares = true
 		hello.trailingKeyShareData = c.config.Bugs.TrailingKeyShareData
@@ -163,7 +164,7 @@ NextCipherSuite:
 			if !c.config.Bugs.EnableAllCiphers {
 				// Don't advertise TLS 1.2-only cipher suites unless
 				// we're attempting TLS 1.2.
-				if hello.vers < VersionTLS12 && suite.flags&suiteTLS12 != 0 {
+				if maxVersion < VersionTLS12 && suite.flags&suiteTLS12 != 0 {
 					continue
 				}
 				// Don't advertise non-DTLS cipher suites in DTLS.
@@ -190,7 +191,7 @@ NextCipherSuite:
 		return errors.New("tls: short read from Rand: " + err.Error())
 	}
 
-	if hello.vers >= VersionTLS12 && !c.config.Bugs.NoSignatureAlgorithms {
+	if maxVersion >= VersionTLS12 && !c.config.Bugs.NoSignatureAlgorithms {
 		hello.signatureAlgorithms = c.config.verifySignatureAlgorithms()
 	}
 
@@ -352,19 +353,19 @@ NextCipherSuite:
 		}
 	}
 
-	var serverVersion uint16
+	var serverWireVersion uint16
 	switch m := msg.(type) {
 	case *helloRetryRequestMsg:
-		serverVersion = m.vers
+		serverWireVersion = m.vers
 	case *serverHelloMsg:
-		serverVersion = m.vers
+		serverWireVersion = m.vers
 	default:
 		c.sendAlert(alertUnexpectedMessage)
 		return fmt.Errorf("tls: received unexpected message of type %T when waiting for HelloRetryRequest or ServerHello", msg)
 	}
 
 	var ok bool
-	c.vers, ok = c.config.mutualVersion(serverVersion, c.isDTLS)
+	c.vers, ok = c.config.mutualVersion(wireToVersion(serverWireVersion, c.isDTLS), c.isDTLS)
 	if !ok {
 		c.sendAlert(alertProtocolVersion)
 		return fmt.Errorf("tls: server selected unsupported protocol version %x", c.vers)
@@ -427,9 +428,9 @@ NextCipherSuite:
 		return unexpectedMessageError(serverHello, msg)
 	}
 
-	if c.vers != serverHello.vers {
+	if serverWireVersion != serverHello.vers {
 		c.sendAlert(alertProtocolVersion)
-		return fmt.Errorf("tls: server sent non-matching version %x vs %x", serverHello.vers, c.vers)
+		return fmt.Errorf("tls: server sent non-matching version %x vs %x", serverWireVersion, serverHello.vers)
 	}
 
 	// Check for downgrade signals in the server random, per
diff --git a/ssl/test/runner/handshake_messages.go b/ssl/test/runner/handshake_messages.go
index 8cec98b1..329be7f9 100644
--- a/ssl/test/runner/handshake_messages.go
+++ b/ssl/test/runner/handshake_messages.go
@@ -212,8 +212,7 @@ func (m *clientHelloMsg) marshal() []byte {
 	handshakeMsg := newByteBuilder()
 	handshakeMsg.addU8(typeClientHello)
 	hello := handshakeMsg.addU24LengthPrefixed()
-	vers := versionToWire(m.vers, m.isDTLS)
-	hello.addU16(vers)
+	hello.addU16(m.vers)
 	hello.addBytes(m.random)
 	sessionId := hello.addU8LengthPrefixed()
 	sessionId.addBytes(m.sessionId)
@@ -420,7 +419,7 @@ func (m *clientHelloMsg) unmarshal(data []byte) bool {
 		return false
 	}
 	m.raw = data
-	m.vers = wireToVersion(uint16(data[4])<<8|uint16(data[5]), m.isDTLS)
+	m.vers = uint16(data[4])<<8 | uint16(data[5])
 	m.random = data[6:38]
 	sessionIdLen := int(data[38])
 	if sessionIdLen > 32 || len(data) < 39+sessionIdLen {
@@ -740,21 +739,21 @@ func (m *serverHelloMsg) marshal() []byte {
 	handshakeMsg := newByteBuilder()
 	handshakeMsg.addU8(typeServerHello)
 	hello := handshakeMsg.addU24LengthPrefixed()
-	vers := versionToWire(m.vers, m.isDTLS)
-	hello.addU16(vers)
+	vers := wireToVersion(m.vers, m.isDTLS)
+	hello.addU16(m.vers)
 	hello.addBytes(m.random)
-	if m.vers < VersionTLS13 {
+	if vers < VersionTLS13 {
 		sessionId := hello.addU8LengthPrefixed()
 		sessionId.addBytes(m.sessionId)
 	}
 	hello.addU16(m.cipherSuite)
-	if m.vers < VersionTLS13 {
+	if vers < VersionTLS13 {
 		hello.addU8(m.compressionMethod)
 	}
 
 	extensions := hello.addU16LengthPrefixed()
 
-	if m.vers >= VersionTLS13 {
+	if vers >= VersionTLS13 {
 		if m.hasKeyShare {
 			extensions.addU16(extensionKeyShare)
 			keyShare := extensions.addU16LengthPrefixed()
@@ -772,7 +771,7 @@ func (m *serverHelloMsg) marshal() []byte {
 			extensions.addU16(0) // Length
 		}
 	} else {
-		m.extensions.marshal(extensions, m.vers)
+		m.extensions.marshal(extensions, vers)
 		if extensions.len() == 0 {
 			hello.discardChild()
 		}
@@ -787,10 +786,11 @@ func (m *serverHelloMsg) unmarshal(data []byte) bool {
 		return false
 	}
 	m.raw = data
-	m.vers = wireToVersion(uint16(data[4])<<8|uint16(data[5]), m.isDTLS)
+	m.vers = uint16(data[4])<<8 | uint16(data[5])
+	vers := wireToVersion(m.vers, m.isDTLS)
 	m.random = data[6:38]
 	data = data[38:]
-	if m.vers < VersionTLS13 {
+	if vers < VersionTLS13 {
 		sessionIdLen := int(data[0])
 		if sessionIdLen > 32 || len(data) < 1+sessionIdLen {
 			return false
@@ -803,7 +803,7 @@ func (m *serverHelloMsg) unmarshal(data []byte) bool {
 	}
 	m.cipherSuite = uint16(data[0])<<8 | uint16(data[1])
 	data = data[2:]
-	if m.vers < VersionTLS13 {
+	if vers < VersionTLS13 {
 		if len(data) < 1 {
 			return false
 		}
@@ -826,7 +826,7 @@ func (m *serverHelloMsg) unmarshal(data []byte) bool {
 		return false
 	}
 
-	if m.vers >= VersionTLS13 {
+	if vers >= VersionTLS13 {
 		for len(data) != 0 {
 			if len(data) < 4 {
 				return false
@@ -871,7 +871,7 @@ func (m *serverHelloMsg) unmarshal(data []byte) bool {
 				return false
 			}
 		}
-	} else if !m.extensions.unmarshal(data, m.vers) {
+	} else if !m.extensions.unmarshal(data, vers) {
 		return false
 	}
 
@@ -1873,7 +1873,7 @@ func (m *helloVerifyRequestMsg) marshal() []byte {
 	x[1] = uint8(length >> 16)
 	x[2] = uint8(length >> 8)
 	x[3] = uint8(length)
-	vers := versionToWire(m.vers, true)
+	vers := m.vers
 	x[4] = uint8(vers >> 8)
 	x[5] = uint8(vers)
 	x[6] = uint8(len(m.cookie))
@@ -1887,7 +1887,7 @@ func (m *helloVerifyRequestMsg) unmarshal(data []byte) bool {
 		return false
 	}
 	m.raw = data
-	m.vers = wireToVersion(uint16(data[4])<<8|uint16(data[5]), true)
+	m.vers = uint16(data[4])<<8 | uint16(data[5])
 	cookieLen := int(data[6])
 	if cookieLen > 32 || len(data) != 7+cookieLen {
 		return false
diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go
index 604fa140..55bc9cc1 100644
--- a/ssl/test/runner/handshake_server.go
+++ b/ssl/test/runner/handshake_server.go
@@ -205,14 +205,15 @@ func (hs *serverHandshakeState) readClientHello() error {
 		}
 	}
 	c.clientVersion = hs.clientHello.vers
+	clientVersion := wireToVersion(c.clientVersion, c.isDTLS)
 
 	// Reject < 1.2 ClientHellos with signature_algorithms.
-	if c.clientVersion < VersionTLS12 && len(hs.clientHello.signatureAlgorithms) > 0 {
+	if clientVersion < VersionTLS12 && len(hs.clientHello.signatureAlgorithms) > 0 {
 		return fmt.Errorf("tls: client included signature_algorithms before TLS 1.2")
 	}
 
 	// Check the client cipher list is consistent with the version.
-	if hs.clientHello.vers < VersionTLS12 {
+	if clientVersion < VersionTLS12 {
 		for _, id := range hs.clientHello.cipherSuites {
 			if isTLS12Cipher(id) {
 				return fmt.Errorf("tls: client offered TLS 1.2 cipher before TLS 1.2")
@@ -238,10 +239,10 @@ func (hs *serverHandshakeState) readClientHello() error {
 	} else if c.haveVers && config.Bugs.NegotiateVersionOnRenego != 0 {
 		c.vers = config.Bugs.NegotiateVersionOnRenego
 	} else {
-		c.vers, ok = config.mutualVersion(hs.clientHello.vers, c.isDTLS)
+		c.vers, ok = config.mutualVersion(clientVersion, c.isDTLS)
 		if !ok {
 			c.sendAlert(alertProtocolVersion)
-			return fmt.Errorf("tls: client offered an unsupported, maximum protocol version of %x", hs.clientHello.vers)
+			return fmt.Errorf("tls: client offered an unsupported, maximum protocol version of %x", clientVersion)
 		}
 	}
 	c.haveVers = true
@@ -311,7 +312,7 @@ func (hs *serverHandshakeState) doTLS13Handshake() error {
 
 	hs.hello = &serverHelloMsg{
 		isDTLS: c.isDTLS,
-		vers:   c.vers,
+		vers:   versionToWire(c.vers, c.isDTLS),
 	}
 
 	if config.Bugs.SendServerHelloVersion != 0 {
@@ -816,7 +817,7 @@ func (hs *serverHandshakeState) processClientHello() (isResume bool, err error)
 
 	hs.hello = &serverHelloMsg{
 		isDTLS:            c.isDTLS,
-		vers:              c.vers,
+		vers:              versionToWire(c.vers, c.isDTLS),
 		compressionMethod: compressionNone,
 	}
 
@@ -1711,5 +1712,5 @@ func isTLS12Cipher(id uint16) bool {
 }
 
 func isGREASEValue(val uint16) bool {
-	return val&0x0f0f == 0x0a0a && val&0xff == val >> 8
+	return val&0x0f0f == 0x0a0a && val&0xff == val>>8
 }
diff --git a/ssl/test/runner/key_agreement.go b/ssl/test/runner/key_agreement.go
index 6327c134..271a9d17 100644
--- a/ssl/test/runner/key_agreement.go
+++ b/ssl/test/runner/key_agreement.go
@@ -40,7 +40,7 @@ type rsaKeyAgreement struct {
 
 func (ka *rsaKeyAgreement) generateServerKeyExchange(config *Config, cert *Certificate, clientHello *clientHelloMsg, hello *serverHelloMsg) (*serverKeyExchangeMsg, error) {
 	// Save the client version for comparison later.
-	ka.clientVersion = versionToWire(clientHello.vers, clientHello.isDTLS)
+	ka.clientVersion = clientHello.vers
 
 	if !config.Bugs.RSAEphemeralKey {
 		return nil, nil
@@ -128,7 +128,7 @@ func (ka *rsaKeyAgreement) processClientKeyExchange(config *Config, cert *Certif
 	// RFC 4346.
 	vers := uint16(preMasterSecret[0])<<8 | uint16(preMasterSecret[1])
 	if ka.clientVersion != vers {
-		return nil, errors.New("tls: invalid version in RSA premaster")
+		return nil, fmt.Errorf("tls: invalid version in RSA premaster (got %04x, wanted %04x)", vers, ka.clientVersion)
 	}
 	return preMasterSecret, nil
 }
@@ -144,7 +144,6 @@ func (ka *rsaKeyAgreement) generateClientKeyExchange(config *Config, clientHello
 	if bad == RSABadValueWrongVersion {
 		vers ^= 1
 	}
-	vers = versionToWire(vers, clientHello.isDTLS)
 	preMasterSecret[0] = byte(vers >> 8)
 	preMasterSecret[1] = byte(vers)
 	_, err := io.ReadFull(config.rand(), preMasterSecret[2:])
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index ea660abe..ea29ef50 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -4194,7 +4194,7 @@ func addVersionNegotiationTests() {
 		name:     "MinorVersionTolerance-DTLS",
 		config: Config{
 			Bugs: ProtocolBugs{
-				SendClientVersion: 0x03ff,
+				SendClientVersion: 0xfe00,
 			},
 		},
 		expectedVersion: VersionTLS12,
@@ -4205,7 +4205,7 @@ func addVersionNegotiationTests() {
 		name:     "MajorVersionTolerance-DTLS",
 		config: Config{
 			Bugs: ProtocolBugs{
-				SendClientVersion: 0x0400,
+				SendClientVersion: 0xfdff,
 			},
 		},
 		expectedVersion: VersionTLS12,
@@ -4229,9 +4229,7 @@ func addVersionNegotiationTests() {
 		name:     "VersionTooLow-DTLS",
 		config: Config{
 			Bugs: ProtocolBugs{
-				// 0x0201 is the lowest version expressable in
-				// DTLS.
-				SendClientVersion: 0x0201,
+				SendClientVersion: 0xffff,
 			},
 		},
 		shouldFail:    true,