commit 81eb4e04d20d819c28a97627e2ab3b98919d4c1a
author dschinazi <dschinazi@google.com> Fri Sep 27 17:12:17 2019 -0700
committer Copybara-Service <copybara-worker@google.com> Tue Oct 01 12:56:12 2019 -0700
tree 512549d8606e8e8c6a693de485de0ca8d8cd1ba6
parent c1bbfe64c68dfc9db2dcf760122cfbf701cfe97a

Allow parsing PROX packets with length prefix

The PROX version is special in that it has custom data after the first 9 bytes of the connection IDs. That means that trying to parse it with length prefix as an unknown QUIC version means that a random payload byte can be interpreted as the source connection ID length. This CL makes parsing succeed in that case, ignoring the source connection ID. This allows a previously existing test to pass.

gfe-relnote: allow parsing PROX packets with length prefix, protected by gfe2_reloadable_flag_quic_parse_prox_source_connection_id

Startblock:
  after 2019-09-24 08:00 in US/Pacific
PiperOrigin-RevId: 271678159
Change-Id: Ib079697029d7378cd7297e2400cefe8be33fad43

quic/core/quic_framer.cc

diff --git a/quic/core/quic_framer.cc b/quic/core/quic_framer.cc
index 84dd00a..a6a663b 100644
--- a/quic/core/quic_framer.cc
+++ b/quic/core/quic_framer.cc
@@ -6366,6 +6366,8 @@
 
 namespace {
 
+const QuicVersionLabel kProxVersionLabel = 0x50524F58;  // "PROX"
+
 inline bool PacketHasLengthPrefixedConnectionIds(
     const QuicDataReader& reader,
     ParsedQuicVersion parsed_version,
@@ -6396,7 +6398,7 @@
 
   // Check for munged packets with version tag PROX.
   if ((connection_id_length_byte & 0x0f) == 0 &&
-      connection_id_length_byte >= 0x20 && version_label == 0x50524F58) {
+      connection_id_length_byte >= 0x20 && version_label == kProxVersionLabel) {
     return false;
   }
 
@@ -6406,6 +6408,7 @@
 inline bool ParseLongHeaderConnectionIds(
     QuicDataReader* reader,
     bool has_length_prefix,
+    QuicVersionLabel version_label,
     QuicConnectionId* destination_connection_id,
     QuicConnectionId* source_connection_id,
     std::string* detailed_error) {
@@ -6415,6 +6418,16 @@
       return false;
     }
     if (!reader->ReadLengthPrefixedConnectionId(source_connection_id)) {
+      if (GetQuicReloadableFlag(quic_parse_prox_source_connection_id) &&
+          version_label == kProxVersionLabel) {
+        QUIC_RELOADABLE_FLAG_COUNT(quic_parse_prox_source_connection_id);
+        // The "PROX" version does not follow the length-prefixed invariants,
+        // and can therefore attempt to read a payload byte and interpret it
+        // as the source connection ID length, which could fail to parse.
+        // In that scenario we keep the source connection ID empty but mark
+        // parsing as successful.
+        return true;
+      }
       *detailed_error = "Unable to read source connection ID.";
       return false;
     }
@@ -6525,7 +6538,7 @@
       *reader, *parsed_version, *version_label, *first_byte);
 
   // Parse connection IDs.
-  if (!ParseLongHeaderConnectionIds(reader, *has_length_prefix,
+  if (!ParseLongHeaderConnectionIds(reader, *has_length_prefix, *version_label,
                                     destination_connection_id,
                                     source_connection_id, detailed_error)) {
     return QUIC_INVALID_PACKET_HEADER;