runner: Remove remnants of the separate HelloRetryRequest message.

In early TLS 1.3 drafts, HelloRetryRequest was a dedicated message type.
Our HelloRetryRequest handling in runner is still based on this. Along
the way, remove the SendServerHelloAsHelloRetryRequest test, since
that's just a generic unexpected message type now.

Change-Id: Idd9c54d0ab66d962657af9a53849c3928f78ce5c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46585
Reviewed-by: Adam Langley <agl@google.com>

ssl/test/runner/common.go

diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index 8cc59d4..22257f0 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -74,7 +74,6 @@
 	typeHelloVerifyRequest    uint8 = 3
 	typeNewSessionTicket      uint8 = 4
 	typeEndOfEarlyData        uint8 = 5
-	typeHelloRetryRequest     uint8 = 6
 	typeEncryptedExtensions   uint8 = 8
 	typeCertificate           uint8 = 11
 	typeServerKeyExchange     uint8 = 12
@@ -1670,10 +1669,6 @@
 	// negotiated.
 	UseLegacySigningAlgorithm signatureAlgorithm
 
-	// SendServerHelloAsHelloRetryRequest, if true, causes the server to
-	// send ServerHello messages with a HelloRetryRequest type field.
-	SendServerHelloAsHelloRetryRequest bool
-
 	// RejectUnsolicitedKeyUpdate, if true, causes all unsolicited
 	// KeyUpdates from the peer to be rejected.
 	RejectUnsolicitedKeyUpdate bool

ssl/test/runner/conn.go

diff --git a/ssl/test/runner/conn.go b/ssl/test/runner/conn.go
index 3b1b7f4..8a065fa 100644
--- a/ssl/test/runner/conn.go
+++ b/ssl/test/runner/conn.go
@@ -1149,8 +1149,6 @@
 		msgType := data[0]
 		if c.config.Bugs.SendWrongMessageType != 0 && msgType == c.config.Bugs.SendWrongMessageType {
 			msgType += 42
-		} else if msgType == typeServerHello && c.config.Bugs.SendServerHelloAsHelloRetryRequest {
-			msgType = typeHelloRetryRequest
 		}
 		if msgType != data[0] {
 			data = append([]byte{msgType}, data[1:]...)
@@ -1390,8 +1388,6 @@
 		m = &serverHelloMsg{
 			isDTLS: c.isDTLS,
 		}
-	case typeHelloRetryRequest:
-		m = new(helloRetryRequestMsg)
 	case typeNewSessionTicket:
 		m = &newSessionTicketMsg{
 			vers:   c.wireVersion,
@@ -1452,7 +1448,6 @@
 		vers := uint16(data[4])<<8 | uint16(data[5])
 		if vers == VersionTLS12 && bytes.Equal(data[6:38], tls13HelloRetryRequest) {
 			m = new(helloRetryRequestMsg)
-			m.(*helloRetryRequestMsg).isServerHello = true
 		}
 	}
 

ssl/test/runner/handshake_messages.go

diff --git a/ssl/test/runner/handshake_messages.go b/ssl/test/runner/handshake_messages.go
index 443f5ab..1004cf1 100644
--- a/ssl/test/runner/handshake_messages.go
+++ b/ssl/test/runner/handshake_messages.go
@@ -1713,7 +1713,6 @@
 type helloRetryRequestMsg struct {
 	raw                 []byte
 	vers                uint16
-	isServerHello       bool
 	sessionID           []byte
 	cipherSuite         uint16
 	compressionMethod   uint8
@@ -1774,44 +1773,21 @@
 func (m *helloRetryRequestMsg) unmarshal(data []byte) bool {
 	m.raw = data
 	reader := byteReader(data[4:])
-	if !reader.readU16(&m.vers) {
-		return false
-	}
-	if m.isServerHello {
-		var random []byte
-		var compressionMethod byte
-		if !reader.readBytes(&random, 32) ||
-			!reader.readU8LengthPrefixedBytes(&m.sessionID) ||
-			!reader.readU16(&m.cipherSuite) ||
-			!reader.readU8(&compressionMethod) ||
-			compressionMethod != 0 {
-			return false
-		}
-	} else if !reader.readU16(&m.cipherSuite) {
-		return false
-	}
+	var legacyVers uint16
+	var random []byte
+	var compressionMethod byte
 	var extensions byteReader
-	if !reader.readU16LengthPrefixed(&extensions) || len(reader) != 0 {
+	if !reader.readU16(&legacyVers) ||
+		legacyVers != VersionTLS12 ||
+		!reader.readBytes(&random, 32) ||
+		!reader.readU8LengthPrefixedBytes(&m.sessionID) ||
+		!reader.readU16(&m.cipherSuite) ||
+		!reader.readU8(&compressionMethod) ||
+		compressionMethod != 0 ||
+		!reader.readU16LengthPrefixed(&extensions) ||
+		len(reader) != 0 {
 		return false
 	}
-	extensionsCopy := extensions
-	for len(extensionsCopy) > 0 {
-		var extension uint16
-		var body byteReader
-		if !extensionsCopy.readU16(&extension) ||
-			!extensionsCopy.readU16LengthPrefixed(&body) {
-			return false
-		}
-		switch extension {
-		case extensionSupportedVersions:
-			if !m.isServerHello ||
-				!body.readU16(&m.vers) ||
-				len(body) != 0 {
-				return false
-			}
-		default:
-		}
-	}
 	for len(extensions) > 0 {
 		var extension uint16
 		var body byteReader
@@ -1821,7 +1797,10 @@
 		}
 		switch extension {
 		case extensionSupportedVersions:
-			// Parsed above.
+			if !body.readU16(&m.vers) ||
+				len(body) != 0 {
+				return false
+			}
 		case extensionKeyShare:
 			var v uint16
 			if !body.readU16(&v) || len(body) != 0 {

ssl/test/runner/runner.go

diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index da66aa9..79f6f7d 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -13288,22 +13288,6 @@
 
 		testCases = append(testCases, t.test)
 	}
-
-	// The processing order for TLS 1.3 version negotiation is such that one
-	// may accidentally accept a HelloRetryRequest in lieu of ServerHello in
-	// TLS 1.2. Test that we do not do this.
-	testCases = append(testCases, testCase{
-		name: "SendServerHelloAsHelloRetryRequest",
-		config: Config{
-			MaxVersion: VersionTLS12,
-			Bugs: ProtocolBugs{
-				SendServerHelloAsHelloRetryRequest: true,
-			},
-		},
-		shouldFail:         true,
-		expectedError:      ":UNEXPECTED_MESSAGE:",
-		expectedLocalError: "remote error: unexpected message",
-	})
 }
 
 func addTrailingMessageDataTests() {