commit b78dea8e6fceed8098ec71df40649eaaafa93a17
author dschinazi <dschinazi@google.com> Tue Nov 09 14:52:02 2021 -0800
committer Copybara-Service <copybara-worker@google.com> Tue Nov 09 14:53:02 2021 -0800
tree 6847a7fafcfcea865b463c16c75c95af529adc3f
parent c2e481404dd68e8f142e3f02e199fa7ffbdc1a34

Refactor QUIC version downgrade prevention, part 2

This CL is part of a series of CLs aimed at supporting draft-ietf-quic-version-negotiation. Instead of filling in legacy_version_information with bogus values then checking those, this CL skips the check. This CL does not change any behavior, and is therefore not flag protected.

PiperOrigin-RevId: 408712468

quic/core/tls_client_handshaker.cc

diff --git a/quic/core/tls_client_handshaker.cc b/quic/core/tls_client_handshaker.cc
index b2abf33..b0bd516 100644
--- a/quic/core/tls_client_handshaker.cc
+++ b/quic/core/tls_client_handshaker.cc
@@ -268,36 +268,24 @@
   session()->connection()->OnTransportParametersReceived(
       *received_transport_params_);
 
-  // When interoperating with non-Google implementations that do not send
-  // the version extension, set it to what we expect.
-  if (!received_transport_params_->legacy_version_information.has_value()) {
-    received_transport_params_->legacy_version_information =
-        TransportParameters::LegacyVersionInformation();
-  }
-  if (received_transport_params_->legacy_version_information.value().version ==
-      0) {
-    received_transport_params_->legacy_version_information.value().version =
-        CreateQuicVersionLabel(session()->connection()->version());
-  }
-  if (received_transport_params_->legacy_version_information.value()
-          .supported_versions.empty()) {
-    received_transport_params_->legacy_version_information.value()
-        .supported_versions.push_back(
+  if (received_transport_params_->legacy_version_information.has_value()) {
+    if (received_transport_params_->legacy_version_information.value()
+            .version !=
+        CreateQuicVersionLabel(session()->connection()->version())) {
+      *error_details = "Version mismatch detected";
+      return false;
+    }
+    if (CryptoUtils::ValidateServerHelloVersions(
             received_transport_params_->legacy_version_information.value()
-                .version);
+                .supported_versions,
+            session()->connection()->server_supported_versions(),
+            error_details) != QUIC_NO_ERROR) {
+      QUICHE_DCHECK(!error_details->empty());
+      return false;
+    }
   }
 
-  if (received_transport_params_->legacy_version_information.value().version !=
-      CreateQuicVersionLabel(session()->connection()->version())) {
-    *error_details = "Version mismatch detected";
-    return false;
-  }
-  if (CryptoUtils::ValidateServerHelloVersions(
-          received_transport_params_->legacy_version_information.value()
-              .supported_versions,
-          session()->connection()->server_supported_versions(),
-          error_details) != QUIC_NO_ERROR ||
-      handshaker_delegate()->ProcessTransportParameters(
+  if (handshaker_delegate()->ProcessTransportParameters(
           *received_transport_params_, /* is_resumption = */ false,
           error_details) != QUIC_NO_ERROR) {
     QUICHE_DCHECK(!error_details->empty());

quic/core/tls_server_handshaker.cc

diff --git a/quic/core/tls_server_handshaker.cc b/quic/core/tls_server_handshaker.cc
index a36f97d..6566c8a 100644
--- a/quic/core/tls_server_handshaker.cc
+++ b/quic/core/tls_server_handshaker.cc
@@ -500,26 +500,20 @@
     }
   }
 
-  // When interoperating with non-Google implementations that do not send
-  // the version extension, set it to what we expect.
-  if (!client_params.legacy_version_information.has_value()) {
-    client_params.legacy_version_information =
-        TransportParameters::LegacyVersionInformation();
-  }
-  if (client_params.legacy_version_information.value().version == 0) {
-    client_params.legacy_version_information.value().version =
-        CreateQuicVersionLabel(session()->connection()->version());
-  }
-
-  if (CryptoUtils::ValidateClientHelloVersion(
+  if (client_params.legacy_version_information.has_value() &&
+      CryptoUtils::ValidateClientHelloVersion(
           client_params.legacy_version_information.value().version,
           session()->connection()->version(), session()->supported_versions(),
-          error_details) != QUIC_NO_ERROR ||
-      handshaker_delegate()->ProcessTransportParameters(
-          client_params, /* is_resumption = */ false, error_details) !=
-          QUIC_NO_ERROR) {
+          error_details) != QUIC_NO_ERROR) {
     return false;
   }
+
+  if (handshaker_delegate()->ProcessTransportParameters(
+          client_params, /* is_resumption = */ false, error_details) !=
+      QUIC_NO_ERROR) {
+    return false;
+  }
+
   ProcessAdditionalTransportParameters(client_params);
   if (!session()->user_agent_id().has_value() &&
       client_params.user_agent_id.has_value()) {