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>
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
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
}
}
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 {
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() {
