commit ecad9644724746dd525dcf42e5736ea749b56ee4
author dschinazi <dschinazi@google.com> Tue Oct 01 10:44:17 2019 -0700
committer Copybara-Service <copybara-worker@google.com> Tue Oct 01 12:57:56 2019 -0700
tree bb3e2f1f052677117efa78717c687dfed5bbffc7
parent 20a9ad92587e07e29a2e1c3563365bc6dbcce03c

Prevent fuzzer from sending unsupported RETRY packets

This CL also adds some DCHECKs and QUIC_BUGs that helped debug this, and a trivial refactor in test code.

gfe-relnote: adds QUIC_BUG in send path, not flag protected
PiperOrigin-RevId: 272244952
Change-Id: Icb77bbb2e498ef02ddc13a839834ddf9d8ed7632

quic/core/quic_framer.cc

diff --git a/quic/core/quic_framer.cc b/quic/core/quic_framer.cc
index a6a663b..0d09199 100644
--- a/quic/core/quic_framer.cc
+++ b/quic/core/quic_framer.cc
@@ -863,6 +863,10 @@
                                    char* buffer,
                                    size_t packet_length,
                                    EncryptionLevel level) {
+  QUIC_BUG_IF(header.version_flag &&
+              VersionHasIetfInvariantHeader(transport_version()) &&
+              header.long_packet_type == RETRY && !frames.empty())
+      << "IETF RETRY packets cannot contain frames " << header;
   QuicDataWriter writer(packet_length, buffer);
   size_t length_field_offset = 0;
   if (!AppendPacketHeader(header, &writer, &length_field_offset)) {
@@ -2160,6 +2164,11 @@
   }
 
   if (header.version_flag) {
+    DCHECK_NE(VERSION_NEGOTIATION, header.long_packet_type)
+        << "QuicFramer::AppendIetfPacketHeader does not support sending "
+           "version negotiation packets, use "
+           "QuicFramer::BuildVersionNegotiationPacket instead "
+        << header;
     // Append version for long header.
     QuicVersionLabel version_label = CreateQuicVersionLabel(version_);
     if (!writer->WriteUInt32(version_label)) {
@@ -2186,6 +2195,10 @@
         GetClientConnectionIdAsSender(header, perspective_);
   }
 
+  // TODO(b/141924462) Remove this QUIC_BUG once we do support sending RETRY.
+  QUIC_BUG_IF(header.version_flag && header.long_packet_type == RETRY)
+      << "Sending IETF RETRY packets is not currently supported " << header;
+
   if (QuicVersionHasLongHeaderLengths(transport_version()) &&
       header.version_flag) {
     if (header.long_packet_type == INITIAL) {

quic/test_tools/quic_test_utils.cc

diff --git a/quic/test_tools/quic_test_utils.cc b/quic/test_tools/quic_test_utils.cc
index dae5857..89cf15d 100644
--- a/quic/test_tools/quic_test_utils.cc
+++ b/quic/test_tools/quic_test_utils.cc
@@ -922,7 +922,8 @@
   if (!versions) {
     versions = &supported_versions;
   }
-  if (QuicVersionHasLongHeaderLengths((*versions)[0].transport_version) &&
+  ParsedQuicVersion version = (*versions)[0];
+  if (QuicVersionHasLongHeaderLengths(version.transport_version) &&
       version_flag) {
     header.retry_token_length_length = VARIABLE_LENGTH_INTEGER_LENGTH_1;
     header.length_length = VARIABLE_LENGTH_INTEGER_LENGTH_2;
@@ -932,7 +933,6 @@
   QuicFramer framer(*versions, QuicTime::Zero(), perspective,
                     kQuicDefaultConnectionIdLength);
   framer.SetInitialObfuscators(destination_connection_id);
-  ParsedQuicVersion version = (*versions)[0];
   EncryptionLevel level =
       header.version_flag ? ENCRYPTION_INITIAL : ENCRYPTION_FORWARD_SECURE;
   if (level != ENCRYPTION_INITIAL) {