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;

quic/core/quic_framer_test.cc

diff --git a/quic/core/quic_framer_test.cc b/quic/core/quic_framer_test.cc
index d343c0d..9bc4076 100644
--- a/quic/core/quic_framer_test.cc
+++ b/quic/core/quic_framer_test.cc
@@ -1283,6 +1283,69 @@
   }
 }
 
+TEST_P(QuicFramerTest, ParsePublicHeaderProxBadSourceConnectionIdLength) {
+  if (!framer_.version().HasLengthPrefixedConnectionIds()) {
+    return;
+  }
+  SetQuicReloadableFlag(quic_parse_prox_source_connection_id, true);
+  // clang-format off
+  unsigned char packet[] = {
+    // public flags (long header with packet type HANDSHAKE and
+    // 4-byte packet number)
+    0xE3,
+    // version
+    'P', 'R', 'O', 'X',
+    // destination connection ID length
+    0x08,
+    // destination connection ID
+    0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10,
+    // source connection ID length (bogus)
+    0xEE,
+    // long header packet length
+    0x05,
+    // packet number
+    0x12, 0x34, 0x56, 0x78,
+    // padding frame
+    0x00,
+  };
+  // clang-format on
+  unsigned char* p = packet;
+  size_t p_length = QUIC_ARRAYSIZE(packet);
+
+  uint8_t first_byte = 0x33;
+  PacketHeaderFormat format = GOOGLE_QUIC_PACKET;
+  bool version_present = false, has_length_prefix = false;
+  QuicVersionLabel version_label = 0;
+  ParsedQuicVersion parsed_version = UnsupportedQuicVersion();
+  QuicConnectionId destination_connection_id = EmptyQuicConnectionId(),
+                   source_connection_id = EmptyQuicConnectionId();
+  QuicLongHeaderType long_packet_type = INVALID_PACKET_TYPE;
+  QuicVariableLengthIntegerLength retry_token_length_length =
+      VARIABLE_LENGTH_INTEGER_LENGTH_4;
+  QuicStringPiece retry_token;
+  std::string detailed_error = "foobar";
+
+  QuicDataReader reader(AsChars(p), p_length);
+  const QuicErrorCode parse_error = QuicFramer::ParsePublicHeader(
+      &reader, kQuicDefaultConnectionIdLength,
+      /*ietf_format=*/true, &first_byte, &format, &version_present,
+      &has_length_prefix, &version_label, &parsed_version,
+      &destination_connection_id, &source_connection_id, &long_packet_type,
+      &retry_token_length_length, &retry_token, &detailed_error);
+  EXPECT_EQ(QUIC_NO_ERROR, parse_error);
+  EXPECT_EQ("", detailed_error);
+  EXPECT_EQ(p[0], first_byte);
+  EXPECT_TRUE(version_present);
+  EXPECT_TRUE(has_length_prefix);
+  EXPECT_EQ(0x50524F58u, version_label);  // "PROX"
+  EXPECT_EQ(UnsupportedQuicVersion(), parsed_version);
+  EXPECT_EQ(FramerTestConnectionId(), destination_connection_id);
+  EXPECT_EQ(EmptyQuicConnectionId(), source_connection_id);
+  EXPECT_EQ(VARIABLE_LENGTH_INTEGER_LENGTH_0, retry_token_length_length);
+  EXPECT_EQ(QuicStringPiece(), retry_token);
+  EXPECT_EQ(IETF_QUIC_LONG_HEADER_PACKET, format);
+}
+
 TEST_P(QuicFramerTest, ClientConnectionIdFromShortHeaderToClient) {
   if (!framer_.version().SupportsClientConnectionIds()) {
     return;