Produce correct ICMP6 Echo Reply when pinging magic link-local address
Bonnet's implementation of IcmpReachableInterface internally produces
ICMP6 Echo Requests directed at a link-local address that is
replied-to by the server directly, without relying on other network
elements. However, we were previously constructing the ICMP6 Echo
Reply incorrectly, putting in the entire packet (as is expected for
ICMP errors) rather than its body.
This would produce non-sensical output when used with `ping6`, namely
```
# ping6 -i 0.1 fe80::71:626f:6e65%qbone0 -c 1
PING fe80::71:626f:6e65%qbone0(fe80::71:626f:6e65) 56 data bytes
Warning: time of day goes back (-3758166514911134782us), taking countermeasures.
Warning: time of day goes back (-3758166514911134731us), taking countermeasures.
112 bytes from 2002:a17:51a:2b52:b0:79c:f9be:5968: icmp_seq=1 ttl=255 time=0.000 ms
wrong data byte #16 should be 0x10 but was 0x0
#16 0 0 0 0 0 0 0 0 fe 80 0 0 0 0 0 0 0 0 0 71 62 6f 6e 65 80 0 fa c3 3a fb 0 1
#48 4f 53 8a 63 0 0 0 0
--- fe80::71:626f:6e65%qbone0 ping statistics ---
1 packets transmitted, 1 received, 0% packet loss, time 0ms
rtt min/avg/max/mdev = 0.000/0.000/0.000/0.000 ms
```
This changes corrects the `body` passed through to actually be that of
the ICMP6 Echo Request, which includes both the UNIX timeval and the
counter of padding when using standard iputils ping6.
This is really an entirely cosmetic issue, but it should also fix the
confusion Wireshark has when looking at packet captures over QBONE,
where it is not able to line up the request and reply since the data
don't match.
PiperOrigin-RevId: 492531951
diff --git a/quiche/quic/qbone/qbone_packet_processor.cc b/quiche/quic/qbone/qbone_packet_processor.cc
index abf77d5..be09ef2 100644
--- a/quiche/quic/qbone/qbone_packet_processor.cc
+++ b/quiche/quic/qbone/qbone_packet_processor.cc
@@ -96,7 +96,16 @@
stats_->OnPacketDeferred(direction);
break;
case ProcessingResult::ICMP:
- SendIcmpResponse(&icmp_header, *packet, direction);
+ if (icmp_header.icmp6_type == ICMP6_ECHO_REPLY) {
+ // If this is an ICMP6 ECHO REPLY, the payload should be the same as the
+ // ICMP6 ECHO REQUEST that this came from, not the entire packet. So we
+ // need to take off both the IPv6 header and the ICMP6 header.
+ auto icmp_body = absl::string_view(*packet).substr(sizeof(ip6_hdr) +
+ sizeof(icmp6_hdr));
+ SendIcmpResponse(&icmp_header, icmp_body, direction);
+ } else {
+ SendIcmpResponse(&icmp_header, *packet, direction);
+ }
stats_->OnPacketDroppedWithIcmp(direction);
break;
case ProcessingResult::ICMP_AND_TCP_RESET:
diff --git a/quiche/quic/qbone/qbone_packet_processor_test.cc b/quiche/quic/qbone/qbone_packet_processor_test.cc
index e11fd5c..e416bb9 100644
--- a/quiche/quic/qbone/qbone_packet_processor_test.cc
+++ b/quiche/quic/qbone/qbone_packet_processor_test.cc
@@ -9,6 +9,7 @@
#include "absl/strings/string_view.h"
#include "quiche/quic/platform/api/quic_test.h"
#include "quiche/quic/qbone/qbone_packet_processor_test_tools.h"
+#include "quiche/common/quiche_text_utils.h"
namespace quic::test {
namespace {
@@ -17,7 +18,9 @@
using ProcessingResult = QbonePacketProcessor::ProcessingResult;
using OutputInterface = QbonePacketProcessor::OutputInterface;
using ::testing::_;
+using ::testing::Invoke;
using ::testing::Return;
+using ::testing::WithArgs;
// clang-format off
static const char kReferenceClientPacketData[] = {
@@ -99,17 +102,56 @@
0x00, 0x00,
};
+static const char kReferenceEchoRequestData[] = {
+ // IPv6 with zero TOS and flow label.
+ 0x60, 0x00, 0x00, 0x00,
+ // Payload size is 64 bytes.
+ 0x00, 64,
+ // Next header is ICMP
+ 58,
+ // TTL is 127.
+ 127,
+ // IP address of the sender is fd00:0:0:1::1
+ 0xfd, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
+ // IP address of the receiver is fe80::71:626f:6e6f
+ 0xfe, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x71, 0x62, 0x6f, 0x6e, 0x6f,
+ // ICMP Type ping
+ 128,
+ // ICMP Code 0
+ 0,
+ // Checksum is not actually checked in any of the tests, so we leave it as
+ // zero
+ 0x00, 0x00,
+ // ICMP Identifier (0xcafe to be memorable)
+ 0xca, 0xfe,
+ // Sequence number
+ 0x00, 0x01,
+ // Data, starting with unix timeval then 0x10..0x37
+ 0x67, 0x37, 0x8a, 0x63, 0x00, 0x00, 0x00, 0x00,
+ 0x96, 0x58, 0x0c, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17,
+ 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f,
+ 0x20, 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27,
+ 0x28, 0x29, 0x2a, 0x2b, 0x2c, 0x2d, 0x2e, 0x2f,
+ 0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37,
+};
+
// clang-format on
static const absl::string_view kReferenceClientPacket(
- kReferenceClientPacketData, arraysize(kReferenceClientPacketData));
+ kReferenceClientPacketData, ABSL_ARRAYSIZE(kReferenceClientPacketData));
static const absl::string_view kReferenceNetworkPacket(
- kReferenceNetworkPacketData, arraysize(kReferenceNetworkPacketData));
+ kReferenceNetworkPacketData, ABSL_ARRAYSIZE(kReferenceNetworkPacketData));
static const absl::string_view kReferenceClientSubnetPacket(
kReferenceClientSubnetPacketData,
- arraysize(kReferenceClientSubnetPacketData));
+ ABSL_ARRAYSIZE(kReferenceClientSubnetPacketData));
+
+static const absl::string_view kReferenceEchoRequest(
+ kReferenceEchoRequestData, ABSL_ARRAYSIZE(kReferenceEchoRequestData));
MATCHER_P(IsIcmpMessage, icmp_type,
"Checks whether the argument is an ICMP message of supplied type") {
@@ -276,5 +318,30 @@
ASSERT_EQ(1, filter->called());
}
+TEST_F(QbonePacketProcessorTest, Icmp6EchoResponseHasRightPayload) {
+ auto filter = std::make_unique<MockPacketFilter>();
+ EXPECT_CALL(*filter, FilterPacket(_, _, _, _, _))
+ .WillOnce(WithArgs<2, 3>(
+ Invoke([](absl::string_view payload, icmp6_hdr* icmp_header) {
+ icmp_header->icmp6_type = ICMP6_ECHO_REPLY;
+ icmp_header->icmp6_code = 0;
+ auto* request_header =
+ reinterpret_cast<const icmp6_hdr*>(payload.data());
+ icmp_header->icmp6_id = request_header->icmp6_id;
+ icmp_header->icmp6_seq = request_header->icmp6_seq;
+ return ProcessingResult::ICMP;
+ })));
+ processor_->set_filter(std::move(filter));
+
+ EXPECT_CALL(stats_, OnPacketDroppedWithIcmp(Direction::FROM_OFF_NETWORK));
+ EXPECT_CALL(output_, SendPacketToClient(_))
+ .WillOnce(Invoke([](absl::string_view packet) {
+ EXPECT_EQ(packet.size(), kReferenceEchoRequest.size());
+ QUIC_LOG(INFO) << "ICMP response:\n"
+ << quiche::QuicheTextUtils::HexDump(packet);
+ }));
+ SendPacketFromClient(kReferenceEchoRequest);
+}
+
} // namespace
} // namespace quic::test
