commit 963d57eaa1dd0174c9c67638d21c4a775408b651
author QUICHE team <quiche-dev@google.com> Thu Mar 21 10:58:47 2019 -0700
committer Copybara-Service <copybara-worker@google.com> Thu Mar 21 11:27:01 2019 -0700
tree a9a296b10666d065a82a6a51f6cfdac8846389e5
parent f03456ceb0db4f11696f304325e55fddc9567797

Make QuicDispatcher reject packets with invalid short connection IDs

This CLs enforces a MUST in the IETF spec that dictates that clients cannot send initial connection IDs under 8 bytes. The QuicDispatcher will reject (and close the connection of) any packet whose connection ID is shorter than 8 (or what it was configured for). The behavior is disabled by quartc. This only impacts v99 because connection IDs of any length other than 8 are already currently dropped when using versions < 99.

gfe-relnote: v99 only, not flag protected
PiperOrigin-RevId: 239629063
Change-Id: I85cee11d84566073e8cbb3569ba3e88e91192f2a

quic/core/quic_dispatcher_test.cc

diff --git a/quic/core/quic_dispatcher_test.cc b/quic/core/quic_dispatcher_test.cc
index 3f4ef0e..1e646af 100644
--- a/quic/core/quic_dispatcher_test.cc
+++ b/quic/core/quic_dispatcher_test.cc
@@ -158,6 +158,7 @@
   using QuicDispatcher::current_client_address;
   using QuicDispatcher::current_peer_address;
   using QuicDispatcher::current_self_address;
+  using QuicDispatcher::SetAllowShortInitialConnectionIds;
   using QuicDispatcher::writer;
 };
 
@@ -639,7 +640,7 @@
 }
 
 // Makes sure nine-byte connection IDs are replaced by 8-byte ones.
-TEST_F(QuicDispatcherTest, BadConnectionIdLengthReplaced) {
+TEST_F(QuicDispatcherTest, LongConnectionIdLengthReplaced) {
   if (!QuicUtils::VariableLengthConnectionIdAllowedForVersion(
           CurrentSupportedVersions()[0].transport_version)) {
     // When variable length connection IDs are not supported, the connection
@@ -672,6 +673,45 @@
   EXPECT_EQ(server_address_, dispatcher_->current_self_address());
 }
 
+// Makes sure zero-byte connection IDs are replaced by 8-byte ones.
+TEST_F(QuicDispatcherTest, InvalidShortConnectionIdLengthReplaced) {
+  if (!QuicUtils::VariableLengthConnectionIdAllowedForVersion(
+          CurrentSupportedVersions()[0].transport_version)) {
+    // When variable length connection IDs are not supported, the connection
+    // fails. See StrayPacketTruncatedConnectionId.
+    return;
+  }
+  QuicSocketAddress client_address(QuicIpAddress::Loopback4(), 1);
+
+  QuicConnectionId bad_connection_id = EmptyQuicConnectionId();
+  QuicConnectionId fixed_connection_id =
+      QuicUtils::CreateRandomConnectionId(mock_helper_.GetRandomGenerator());
+
+  // Disable validation of invalid short connection IDs.
+  dispatcher_->SetAllowShortInitialConnectionIds(true);
+  // Note that StrayPacketTruncatedConnectionId covers the case where the
+  // validation is still enabled.
+
+  EXPECT_CALL(*dispatcher_,
+              CreateQuicSession(fixed_connection_id, client_address,
+                                QuicStringPiece("hq"), _))
+      .WillOnce(testing::Return(CreateSession(
+          dispatcher_.get(), config_, fixed_connection_id, client_address,
+          &mock_helper_, &mock_alarm_factory_, &crypto_config_,
+          QuicDispatcherPeer::GetCache(dispatcher_.get()), &session1_)));
+  EXPECT_CALL(*reinterpret_cast<MockQuicConnection*>(session1_->connection()),
+              ProcessUdpPacket(_, _, _))
+      .WillOnce(WithArg<2>(
+          Invoke([this, bad_connection_id](const QuicEncryptedPacket& packet) {
+            ValidatePacket(bad_connection_id, packet);
+          })));
+  EXPECT_CALL(*dispatcher_,
+              ShouldCreateOrBufferPacketForConnection(bad_connection_id, _));
+  ProcessPacket(client_address, bad_connection_id, true, SerializeCHLO());
+  EXPECT_EQ(client_address, dispatcher_->current_peer_address());
+  EXPECT_EQ(server_address_, dispatcher_->current_self_address());
+}
+
 // Makes sure TestConnectionId(1) creates a new connection and
 // TestConnectionIdNineBytesLong(2) gets replaced.
 TEST_F(QuicDispatcherTest, MixGoodAndBadConnectionIdLengthPackets) {
@@ -1323,20 +1363,15 @@
 // Packets with truncated connection IDs should be dropped.
 TEST_F(QuicDispatcherTestStrayPacketConnectionId,
        StrayPacketTruncatedConnectionId) {
-  if (QuicUtils::VariableLengthConnectionIdAllowedForVersion(
-          CurrentSupportedVersions()[0].transport_version)) {
-    // When variable length connection IDs are supported, the server
-    // transparently replaces empty connection IDs with one it chooses.
-    // See BadConnectionIdLengthReplaced.
-    return;
-  }
   CreateTimeWaitListManager();
 
   QuicSocketAddress client_address(QuicIpAddress::Loopback4(), 1);
   QuicConnectionId connection_id = TestConnectionId(1);
   EXPECT_CALL(*dispatcher_, CreateQuicSession(_, _, QuicStringPiece("hq"), _))
       .Times(0);
-  if (CurrentSupportedVersions()[0].transport_version > QUIC_VERSION_43) {
+  if (CurrentSupportedVersions()[0].transport_version > QUIC_VERSION_43 &&
+      !QuicUtils::VariableLengthConnectionIdAllowedForVersion(
+          CurrentSupportedVersions()[0].transport_version)) {
     // This IETF packet has invalid connection ID length.
     EXPECT_CALL(*time_wait_list_manager_, ProcessPacket(_, _, _, _, _))
         .Times(0);