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 <[email protected]> Commit-Queue: Bob Beck <[email protected]> Auto-Submit: David Benjamin <[email protected]>
google · Feb 19, 2025 · 294ab97 · 294ab97
1 parent 084d0d1
commit 294ab97
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 9 deletions.
diff --git a/ssl/test/runner/conn.go b/ssl/test/runner/conn.go
@@ -1518,8 +1518,8 @@ func (c *Conn) readHandshake() (any, error) {
 
 	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
@@ -1940,14 +1940,19 @@ func (m *helloRetryRequestMsg) marshal() []byte {
 }
 
 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
 	var random []byte
 	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
@@ -711,7 +711,8 @@ static enum ssl_hs_wait_t do_send_hello_retry_request(SSL_HANDSHAKE *hs) {
   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 @@ static enum ssl_hs_wait_t do_send_server_hello(SSL_HANDSHAKE *hs) {
     }
   }
 
-  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) ||