Increase maximum size of allowed incoming packets and remove incorrect QUIC_BUG
The current code would trigger a QUIC_BUG if we received a packet over 1452 bytes long. This CL increases that limit to 1472 and makes sure we fail gracefully by dropping the packet with error QUIC_PACKET_TOO_LARGE instead of triggering a QUIC_BUG.
gfe-relnote: minor change to handling of large packets (which we do not send), not flag protected
PiperOrigin-RevId: 242197597
Change-Id: Ia42288b44f3e26e0e28ab60485da9e0b570f2048
diff --git a/quic/core/quic_constants.h b/quic/core/quic_constants.h
index 0f8b019..e0bf4ce 100644
--- a/quic/core/quic_constants.h
+++ b/quic/core/quic_constants.h
@@ -40,6 +40,8 @@
// The maximum packet size of any QUIC packet over IPv4.
// 1500(Ethernet) - 20(IPv4 header) - 8(UDP header) = 1472.
const QuicByteCount kMaxV4PacketSize = 1472;
+// The maximum incoming packet size allowed.
+const QuicByteCount kMaxIncomingPacketSize = kMaxV4PacketSize;
// ETH_MAX_MTU - MAX(sizeof(iphdr), sizeof(ip6_hdr)) - sizeof(udphdr).
const QuicByteCount kMaxGsoPacketSize = 65535 - 40 - 8;
// Default maximum packet size used in the Linux TCP implementation.
diff --git a/quic/core/quic_framer.cc b/quic/core/quic_framer.cc
index ca53ca4..f170cbc 100644
--- a/quic/core/quic_framer.cc
+++ b/quic/core/quic_framer.cc
@@ -27,6 +27,7 @@
#include "net/third_party/quiche/src/quic/core/quic_utils.h"
#include "net/third_party/quiche/src/quic/core/quic_versions.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_aligned.h"
+#include "net/third_party/quiche/src/quic/platform/api/quic_arraysize.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_bug_tracker.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_client_stats.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_endian.h"
@@ -1560,15 +1561,16 @@
rv = ProcessVersionNegotiationPacket(&reader, header);
} else if (header.reset_flag) {
rv = ProcessPublicResetPacket(&reader, header);
- } else if (packet.length() <= kMaxPacketSize) {
+ } else if (packet.length() <= kMaxIncomingPacketSize) {
// The optimized decryption algorithm implementations run faster when
// operating on aligned memory.
- QUIC_CACHELINE_ALIGNED char buffer[kMaxPacketSize];
+ QUIC_CACHELINE_ALIGNED char buffer[kMaxIncomingPacketSize];
if (packet_has_ietf_packet_header) {
rv = ProcessIetfDataPacket(&reader, &header, packet, buffer,
- kMaxPacketSize);
+ QUIC_ARRAYSIZE(buffer));
} else {
- rv = ProcessDataPacket(&reader, &header, packet, buffer, kMaxPacketSize);
+ rv = ProcessDataPacket(&reader, &header, packet, buffer,
+ QUIC_ARRAYSIZE(buffer));
}
} else {
std::unique_ptr<char[]> large_buffer(new char[packet.length()]);
@@ -1580,7 +1582,8 @@
packet.length());
}
QUIC_BUG_IF(rv) << "QUIC should never successfully process packets larger"
- << "than kMaxPacketSize. packet size:" << packet.length();
+ << "than kMaxIncomingPacketSize. packet size:"
+ << packet.length();
}
return rv;
}
@@ -1857,9 +1860,8 @@
return true;
}
- if (packet.length() > kMaxPacketSize) {
- // If the packet has gotten this far, it should not be too large.
- QUIC_BUG << "Packet too large:" << packet.length();
+ if (packet.length() > kMaxIncomingPacketSize) {
+ set_detailed_error("Packet too large.");
return RaiseError(QUIC_PACKET_TOO_LARGE);
}
@@ -1936,9 +1938,8 @@
return true;
}
- if (packet.length() > kMaxPacketSize) {
- // If the packet has gotten this far, it should not be too large.
- QUIC_BUG << "Packet too large:" << packet.length();
+ if (packet.length() > kMaxIncomingPacketSize) {
+ set_detailed_error("Packet too large.");
return RaiseError(QUIC_PACKET_TOO_LARGE);
}
diff --git a/quic/core/quic_framer_test.cc b/quic/core/quic_framer_test.cc
index e627579..09b2023 100644
--- a/quic/core/quic_framer_test.cc
+++ b/quic/core/quic_framer_test.cc
@@ -802,7 +802,7 @@
TEST_P(QuicFramerTest, LargePacket) {
// clang-format off
- unsigned char packet[kMaxPacketSize + 1] = {
+ unsigned char packet[kMaxIncomingPacketSize + 1] = {
// public flags (8 byte connection_id)
0x28,
// connection_id
@@ -812,7 +812,7 @@
// private flags
0x00,
};
- unsigned char packet44[kMaxPacketSize + 1] = {
+ unsigned char packet44[kMaxIncomingPacketSize + 1] = {
// type (short header 4 byte packet number)
0x32,
// connection_id
@@ -820,7 +820,7 @@
// packet number
0x78, 0x56, 0x34, 0x12,
};
- unsigned char packet46[kMaxPacketSize + 1] = {
+ unsigned char packet46[kMaxIncomingPacketSize + 1] = {
// type (short header 4 byte packet number)
0x43,
// connection_id
@@ -845,10 +845,10 @@
!kIncludeDiversificationNonce, PACKET_4BYTE_PACKET_NUMBER,
VARIABLE_LENGTH_INTEGER_LENGTH_0, 0, VARIABLE_LENGTH_INTEGER_LENGTH_0);
- memset(p + header_size, 0, kMaxPacketSize - header_size);
+ memset(p + header_size, 0, kMaxIncomingPacketSize - header_size);
QuicEncryptedPacket encrypted(AsChars(p), p_size, false);
- EXPECT_QUIC_BUG(framer_.ProcessPacket(encrypted), "Packet too large:1");
+ EXPECT_FALSE(framer_.ProcessPacket(encrypted));
ASSERT_TRUE(visitor_.header_.get());
// Make sure we've parsed the packet header, so we can send an error.
@@ -856,6 +856,7 @@
visitor_.header_->destination_connection_id);
// Make sure the correct error is propagated.
EXPECT_EQ(QUIC_PACKET_TOO_LARGE, framer_.error());
+ EXPECT_EQ("Packet too large.", framer_.detailed_error());
}
TEST_P(QuicFramerTest, PacketHeader) {
