Fix legacy_version in DTLS 1.3 HelloRetryRequest Sergey Sukhanov noticed we were setting legacy_version to TLS 1.2 instead of DTLS 1.2. Bug: 323561277 Change-Id: I07e0fd8e5ac8f027ba8c46b39a7e06b700d1f5c7 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/76587 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: Bob Beck <bbe@google.com> Auto-Submit: David Benjamin <davidben@google.com>
diff --git a/ssl/test/runner/conn.go b/ssl/test/runner/conn.go index a3ca817..b75e48b 100644 --- a/ssl/test/runner/conn.go +++ b/ssl/test/runner/conn.go
@@ -1518,8 +1518,8 @@ if data[0] == typeServerHello && len(data) >= 38 { vers := uint16(data[4])<<8 | uint16(data[5]) - if vers == VersionTLS12 && bytes.Equal(data[6:38], tls13HelloRetryRequest) { - m = new(helloRetryRequestMsg) + if (vers == VersionDTLS12 || vers == VersionTLS12) && bytes.Equal(data[6:38], tls13HelloRetryRequest) { + m = &helloRetryRequestMsg{isDTLS: c.isDTLS} } }
diff --git a/ssl/test/runner/handshake_messages.go b/ssl/test/runner/handshake_messages.go index 5232581..3bffb5a 100644 --- a/ssl/test/runner/handshake_messages.go +++ b/ssl/test/runner/handshake_messages.go
@@ -1940,6 +1940,11 @@ } func (m *helloRetryRequestMsg) unmarshal(data []byte) bool { + expectedLegacyVers := uint16(VersionTLS12) + if m.isDTLS { + expectedLegacyVers = VersionDTLS12 + } + m.raw = data reader := cryptobyte.String(data[4:]) var legacyVers uint16 @@ -1947,7 +1952,7 @@ var compressionMethod byte var extensions cryptobyte.String if !reader.ReadUint16(&legacyVers) || - legacyVers != VersionTLS12 || + legacyVers != expectedLegacyVers || !reader.ReadBytes(&random, 32) || !readUint8LengthPrefixedBytes(&reader, &m.sessionID) || !reader.ReadUint16(&m.cipherSuite) ||
diff --git a/ssl/tls13_server.cc b/ssl/tls13_server.cc index e34ecf4..3e83e5f 100644 --- a/ssl/tls13_server.cc +++ b/ssl/tls13_server.cc
@@ -711,7 +711,8 @@ ScopedCBB cbb; CBB body, session_id, extensions; if (!ssl->method->init_message(ssl, cbb.get(), &body, SSL3_MT_SERVER_HELLO) || - !CBB_add_u16(&body, TLS1_2_VERSION) || + !CBB_add_u16(&body, + SSL_is_dtls(ssl) ? DTLS1_2_VERSION : TLS1_2_VERSION) || !CBB_add_bytes(&body, kHelloRetryRequest, SSL3_RANDOM_SIZE) || !CBB_add_u8_length_prefixed(&body, &session_id) || !CBB_add_bytes(&session_id, hs->session_id.data(), @@ -907,15 +908,12 @@ } } - uint16_t server_hello_version = TLS1_2_VERSION; - if (SSL_is_dtls(ssl)) { - server_hello_version = DTLS1_2_VERSION; - } Array<uint8_t> server_hello; ScopedCBB cbb; CBB body, extensions, session_id; if (!ssl->method->init_message(ssl, cbb.get(), &body, SSL3_MT_SERVER_HELLO) || - !CBB_add_u16(&body, server_hello_version) || + !CBB_add_u16(&body, + SSL_is_dtls(ssl) ? DTLS1_2_VERSION : TLS1_2_VERSION) || !CBB_add_bytes(&body, ssl->s3->server_random, sizeof(ssl->s3->server_random)) || !CBB_add_u8_length_prefixed(&body, &session_id) ||