Disable TLS 1.3 compatibility mode for QUIC.

Bug: 335
Change-Id: Ic22dafbc4ada3af56260bc7213f0078876e56c3d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/41244
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>

ssl/handshake_client.cc

diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc
index d77a971..9625b8e 100644
--- a/ssl/handshake_client.cc
+++ b/ssl/handshake_client.cc
@@ -416,17 +416,20 @@
     return ssl_hs_error;
   }
 
-  if (ssl->session != nullptr &&
-      !ssl->s3->initial_handshake_complete &&
-      ssl->session->session_id_length > 0) {
-    hs->session_id_len = ssl->session->session_id_length;
-    OPENSSL_memcpy(hs->session_id, ssl->session->session_id,
-                   hs->session_id_len);
-  } else if (hs->max_version >= TLS1_3_VERSION) {
-    // Initialize a random session ID.
-    hs->session_id_len = sizeof(hs->session_id);
-    if (!RAND_bytes(hs->session_id, hs->session_id_len)) {
-      return ssl_hs_error;
+  // Never send a session ID in QUIC. QUIC uses TLS 1.3 at a minimum and
+  // disables TLS 1.3 middlebox compatibility mode.
+  if (ssl->quic_method == nullptr) {
+    if (ssl->session != nullptr && !ssl->s3->initial_handshake_complete &&
+        ssl->session->session_id_length > 0) {
+      hs->session_id_len = ssl->session->session_id_length;
+      OPENSSL_memcpy(hs->session_id, ssl->session->session_id,
+                     hs->session_id_len);
+    } else if (hs->max_version >= TLS1_3_VERSION) {
+      // Initialize a random session ID.
+      hs->session_id_len = sizeof(hs->session_id);
+      if (!RAND_bytes(hs->session_id, hs->session_id_len)) {
+        return ssl_hs_error;
+      }
     }
   }
 

ssl/ssl_test.cc

diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 2aaff76..424f7d2 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -5318,6 +5318,26 @@
   ExpectHandshakeSuccess();
 }
 
+// Test that the client does not send a legacy_session_id in the ClientHello.
+TEST_F(QUICMethodTest, NoLegacySessionId) {
+  const SSL_QUIC_METHOD quic_method = DefaultQUICMethod();
+
+  ASSERT_TRUE(SSL_CTX_set_quic_method(client_ctx_.get(), &quic_method));
+  ASSERT_TRUE(SSL_CTX_set_quic_method(server_ctx_.get(), &quic_method));
+  // Check that the session ID length is 0 in an early callback.
+  SSL_CTX_set_select_certificate_cb(
+      server_ctx_.get(),
+      [](const SSL_CLIENT_HELLO *client_hello) -> ssl_select_cert_result_t {
+        EXPECT_EQ(client_hello->session_id_len, 0u);
+        return ssl_select_cert_success;
+      });
+
+  ASSERT_TRUE(CreateClientAndServer());
+  ASSERT_TRUE(CompleteHandshakesForQUIC());
+
+  ExpectHandshakeSuccess();
+}
+
 // Test that, even in a 1-RTT handshake, the server installs keys at the right
 // time. Half-RTT keys are available early, but 1-RTT read keys are deferred.
 TEST_F(QUICMethodTest, HalfRTTKeys) {

ssl/test/runner/handshake_server.go

diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go
index 3d13b9b..931ecca 100644
--- a/ssl/test/runner/handshake_server.go
+++ b/ssl/test/runner/handshake_server.go
@@ -299,6 +299,9 @@
 		}
 	}
 
+	if config.Bugs.MockQUICTransport != nil && len(hs.clientHello.sessionId) > 0 {
+		return fmt.Errorf("tls: QUIC client did not disable compatibility mode")
+	}
 	if config.Bugs.ExpectNoTLS12Session {
 		if len(hs.clientHello.sessionId) > 0 && c.vers >= VersionTLS13 {
 			return fmt.Errorf("tls: client offered an unexpected session ID")