srdusr
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--PLAN.md42
-rw-r--r--include/packeteer/net/ipv4.hpp11
-rw-r--r--include/packeteer/net/ipv6.hpp53
-rw-r--r--include/packeteer/summarize.hpp28
-rw-r--r--tests/test_ipv6.cpp37
-rw-r--r--tests/test_net.cpp31
-rw-r--r--tests/test_summarize.cpp28
7 files changed, 224 insertions, 6 deletions
diff --git a/PLAN.md b/PLAN.md
index 7ad2649..64b49e3 100644
--- a/PLAN.md
+++ b/PLAN.md
@@ -685,3 +685,45 @@ None currently open.
real intermediate routers - this machine's own gateway, then real
ISP infrastructure several hops out - all correctly showing "[this
machine -> 8.8.8.8 proto=1]", matching traceroute's own hop output.
+- IPv4/IPv6 fragmentation: a real correctness fix, not just added
+ visibility. Before this, a non-first fragment's payload - pure
+ continuation data, no TCP/UDP/ICMP header present at all - was
+ handed to the transport parsers unconditionally, which could
+ misread arbitrary payload bytes as port numbers, sequence numbers,
+ etc. and print a plausible-looking but entirely fake decode. IPv4
+ gained identification/more_fragments/fragment_offset fields on
+ Ipv4Header; a nonzero fragment_offset now stops summarize_packet
+ before it ever reaches transport dispatch, reporting the fragment
+ itself instead. IPv6 expresses fragmentation as a Fragment
+ extension header instead of header fields, so the fix lives in
+ walk_ipv6_extension_headers(): it already walked past Fragment
+ headers, but never checked the offset before continuing on to
+ decode whatever followed as if it were a transport header --
+ exactly the same bug, just reached through the extension-header
+ path instead of a header field. Fixed by having the walk stop
+ immediately (like the existing ESP hard-stop) whenever the offset is
+ nonzero, and reporting that stop via two new
+ Ipv6ExtensionWalkResult fields.
+ Caught and fixed a real bug in this fix while writing it, before it
+ ever ran: the first version computed a fragment's Identification
+ field as a loop-local variable, so it was silently discarded the
+ moment the walk continued past a *first* fragment's header (offset
+ zero) to keep decoding the real transport protocol underneath --
+ every return path after that point reported no fragment id at all,
+ even though one genuinely applied. Fixed by hoisting it to a
+ variable that persists across loop iterations, the same category of
+ mistake (and the same fix) as this session's earlier IGMPv3 bug:
+ state that needs to survive past the specific branch that computed
+ it. Fuzzed afterward regardless (fuzz_ipv4, fuzz_ipv6,
+ fuzz_summarize, ~16.8M combined runs) - clean, no crashes.
+ Live-verified with genuine IPv4 fragmentation: a 4000-byte ping to
+ the real local gateway over the actual 1500-MTU wlp1s0 interface,
+ captured on the wire. Both directions fragmented into exactly 3
+ pieces each; the first fragment decoded normally (ICMP Echo
+ Request/Reply, correct id/seq) with a "(fragmented, ...)" note, and
+ the two continuation fragments correctly showed only fragment
+ metadata - no fake ICMP decode attempted on their payload. IPv6
+ fragmentation is unit-tested (including the exact byte pattern that
+ triggered the fragment-id bug above) but not live-verified: this
+ machine has no real global IPv6 connectivity to generate genuine
+ IPv6-fragmented traffic against, only link-local addresses.
diff --git a/include/packeteer/net/ipv4.hpp b/include/packeteer/net/ipv4.hpp
index c6cb656..bcc8fa6 100644
--- a/include/packeteer/net/ipv4.hpp
+++ b/include/packeteer/net/ipv4.hpp
@@ -27,6 +27,13 @@ struct Ipv4Header {
std::uint8_t protocol;
Ipv4Address src;
Ipv4Address dst;
+ std::uint16_t identification; // ties fragments of the same original datagram together
+ bool more_fragments;
+ // In 8-byte units (RFC 791) - 0 means this is either an unfragmented
+ // packet or the *first* fragment, either way the one that actually
+ // carries the transport header. Nonzero means a later fragment:
+ // pure payload continuation, no TCP/UDP/ICMP header present at all.
+ std::uint16_t fragment_offset;
};
struct Ipv4Packet {
@@ -46,6 +53,10 @@ inline std::optional<Ipv4Packet> parse_ipv4(std::span<const unsigned char> bytes
header.version = version;
header.ihl = ihl;
header.total_length = read_be16(bytes, 2);
+ header.identification = read_be16(bytes, 4);
+ std::uint16_t flags_and_offset = read_be16(bytes, 6);
+ header.more_fragments = (flags_and_offset & 0x2000) != 0;
+ header.fragment_offset = flags_and_offset & 0x1FFF;
header.ttl = bytes[8];
header.protocol = bytes[9];
std::copy_n(bytes.begin() + 12, 4, header.src.bytes.begin());
diff --git a/include/packeteer/net/ipv6.hpp b/include/packeteer/net/ipv6.hpp
index 8c048e8..d7ea0d5 100644
--- a/include/packeteer/net/ipv6.hpp
+++ b/include/packeteer/net/ipv6.hpp
@@ -70,6 +70,18 @@ struct Ipv6ExtensionWalkResult {
std::uint8_t final_next_header; // a transport protocol, or an extension type we stopped at
std::span<const unsigned char> payload; // bytes after every extension header walked
bool stopped_at_esp; // true if ESP was hit - see walk_ipv6_extension_headers()
+ // True when a Fragment header (RFC 8200 4.5) was walked with a
+ // nonzero fragment offset - a later fragment, not the first.
+ // final_next_header still correctly names the eventual transport
+ // protocol, but `payload` here is pure continuation data with no
+ // TCP/UDP/ICMPv6 header in it at all; callers must not hand it to
+ // a transport parser. fragment_id is that header's 4-byte
+ // Identification field, set whenever a Fragment header was seen
+ // at all (first fragment or not) - useful for correlating
+ // fragments of the same original datagram even when this flag is
+ // false for the first one.
+ bool is_non_first_fragment;
+ std::optional<std::uint32_t> fragment_id;
};
// Walks Hop-by-Hop, Routing, Destination Options, Fragment, and AH
@@ -96,35 +108,64 @@ struct Ipv6ExtensionWalkResult {
inline Ipv6ExtensionWalkResult walk_ipv6_extension_headers(std::uint8_t next_header,
std::span<const unsigned char> payload) {
constexpr int kMaxExtensionHeaders = 8;
+ // Persists across loop iterations, unlike a Fragment header's other
+ // fields: once a Fragment header is seen (even the first one, whose
+ // walk continues normally afterward), every return from here on
+ // should still report its Identification field, not lose it the
+ // moment the loop moves past that one header.
+ std::optional<std::uint32_t> seen_fragment_id;
for (int i = 0; i < kMaxExtensionHeaders; ++i) {
if (next_header == kNextHeaderEsp) {
- return {next_header, payload, /*stopped_at_esp=*/true};
+ return {next_header, payload, /*stopped_at_esp=*/true, false, seen_fragment_id};
}
std::size_t ext_len;
if (next_header == kNextHeaderFragment) {
- if (payload.size() < 8) return {next_header, payload, false};
+ if (payload.size() < 8) return {next_header, payload, false, false, seen_fragment_id};
ext_len = 8;
} else if (next_header == kNextHeaderAh) {
- if (payload.size() < 2) return {next_header, payload, false};
+ if (payload.size() < 2) return {next_header, payload, false, false, seen_fragment_id};
ext_len = (static_cast<std::size_t>(payload[1]) + 2) * 4;
} else if (next_header == kNextHeaderHopByHop || next_header == kNextHeaderRouting ||
next_header == kNextHeaderDestOptions) {
- if (payload.size() < 2) return {next_header, payload, false};
+ if (payload.size() < 2) return {next_header, payload, false, false, seen_fragment_id};
ext_len = (static_cast<std::size_t>(payload[1]) + 1) * 8;
} else {
break; // TCP/UDP/ICMPv6/anything else we don't chain through: stop here
}
- if (payload.size() < ext_len) return {next_header, payload, false}; // truncated: stop
+ if (payload.size() < ext_len) {
+ return {next_header, payload, false, false, seen_fragment_id}; // truncated: stop
+ }
+
+ if (next_header == kNextHeaderFragment) {
+ // Fragment Offset (13 bits) + Res (2 bits) + M flag (1 bit)
+ // at bytes[2:4]; Identification (4 bytes) at bytes[4:8].
+ std::uint16_t offset_res_m = read_be16(payload, 2);
+ std::uint16_t fragment_offset = (offset_res_m >> 3) & 0x1FFF;
+ seen_fragment_id = read_be32(payload, 4);
+ std::uint8_t this_next_header = payload[0];
+ std::span<const unsigned char> remaining = payload.subspan(ext_len);
+ if (fragment_offset != 0) {
+ // A later fragment: `remaining` is pure continuation
+ // data, not a transport header, regardless of what
+ // this_next_header names. Stop here rather than
+ // walking (and definitely rather than letting a
+ // caller hand this to a transport parser).
+ return {this_next_header, remaining, false, true, seen_fragment_id};
+ }
+ payload = remaining;
+ next_header = this_next_header;
+ continue;
+ }
std::uint8_t this_next_header = payload[0];
payload = payload.subspan(ext_len);
next_header = this_next_header;
}
- return {next_header, payload, false};
+ return {next_header, payload, false, false, seen_fragment_id};
}
// RFC 5952 canonical text form: lowercase hex, and the longest run of
diff --git a/include/packeteer/summarize.hpp b/include/packeteer/summarize.hpp
index 4126eb5..4bb2d24 100644
--- a/include/packeteer/summarize.hpp
+++ b/include/packeteer/summarize.hpp
@@ -319,9 +319,27 @@ inline std::string summarize_packet(std::span<const unsigned char> bytes, int da
out += " | IPv4 (truncated)";
return out;
}
+ bool is_fragment = ip->header.more_fragments || ip->header.fragment_offset != 0;
+ if (ip->header.fragment_offset != 0) {
+ // A later fragment: no TCP/UDP/ICMP header is present in
+ // this packet at all, only continuation payload - handing
+ // it to a transport parser would misread arbitrary payload
+ // bytes as port numbers/sequence numbers/etc. Report the
+ // fragment itself and stop, rather than decode garbage.
+ char buf[96];
+ std::snprintf(buf, sizeof(buf), " | IPv4 %s -> %s ttl=%u proto=%u fragment id=%u offset=%u%s",
+ ipv4_to_string(ip->header.src).c_str(), ipv4_to_string(ip->header.dst).c_str(),
+ ip->header.ttl, ip->header.protocol, ip->header.identification,
+ ip->header.fragment_offset * 8, ip->header.more_fragments ? " (more)" : "");
+ out += buf;
+ return out;
+ }
out += summarize_transport_and_above({"IPv4", ipv4_to_string(ip->header.src),
ipv4_to_string(ip->header.dst), ip->header.ttl,
ip->header.protocol, ip->payload});
+ if (is_fragment) {
+ out += " (fragmented, id=" + std::to_string(ip->header.identification) + ", more follow)";
+ }
} else if (version == 6) {
auto ip6 = net::parse_ipv6(ip_bytes);
if (!ip6) {
@@ -341,9 +359,19 @@ inline std::string summarize_packet(std::span<const unsigned char> bytes, int da
src_str.c_str(), dst_str.c_str(), ip6->header.hop_limit,
net::kNextHeaderEsp);
out += buf;
+ } else if (walked.is_non_first_fragment) {
+ char buf[128];
+ std::snprintf(buf, sizeof(buf),
+ " | IPv6 %s -> %s ttl=%u proto=%u fragment id=%u (not first)",
+ src_str.c_str(), dst_str.c_str(), ip6->header.hop_limit,
+ walked.final_next_header, *walked.fragment_id);
+ out += buf;
} else {
out += summarize_transport_and_above({"IPv6", src_str, dst_str, ip6->header.hop_limit,
walked.final_next_header, walked.payload});
+ if (walked.fragment_id) {
+ out += " (fragmented, id=" + std::to_string(*walked.fragment_id) + ")";
+ }
}
} else {
out += " | IP version " + std::to_string(version) + " (unsupported)";
diff --git a/tests/test_ipv6.cpp b/tests/test_ipv6.cpp
index 1cce85b..2d75e9a 100644
--- a/tests/test_ipv6.cpp
+++ b/tests/test_ipv6.cpp
@@ -129,6 +129,43 @@ TEST_CASE("walk_ipv6_extension_headers walks the fixed-size Fragment header") {
CHECK(result.payload[0] == 0xCA);
}
+TEST_CASE("walk_ipv6_extension_headers reports the fragment id even for the first fragment") {
+ // Same shape as the test above (fragment offset 0 - the first
+ // fragment), but this time checking that the walk continues on to
+ // TCP *and* still surfaces the Identification field, which a
+ // caller needs to correlate this with the fragments that follow.
+ std::vector<unsigned char> payload = {static_cast<unsigned char>(kProtoTcp), 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x30, 0x39, // id = 0x3039
+ 0xCA, 0xFE};
+ auto result = walk_ipv6_extension_headers(kNextHeaderFragment, payload);
+ CHECK_FALSE(result.is_non_first_fragment);
+ REQUIRE(result.fragment_id.has_value());
+ CHECK(*result.fragment_id == 0x3039);
+ CHECK(result.final_next_header == kProtoTcp);
+ REQUIRE(result.payload.size() == 2); // walk continued past the fragment header to real payload
+}
+
+TEST_CASE("walk_ipv6_extension_headers stops at a non-first fragment rather than walking into "
+ "continuation data") {
+ // Fragment offset field (13 bits, packed into the top of bytes[2:3])
+ // set to a nonzero value - 8 in units of 8 bytes, i.e. byte offset
+ // 64 into the original datagram. offset_res_m = 8 << 3 = 0x0040.
+ std::vector<unsigned char> payload = {
+ static_cast<unsigned char>(kProtoTcp), 0x00, 0x00, 0x40, 0x00, 0x00, 0x00, 0x2A,
+ 0xDE, 0xAD, 0xBE, 0xEF, // pure continuation data - NOT a TCP header
+ };
+ auto result = walk_ipv6_extension_headers(kNextHeaderFragment, payload);
+ CHECK(result.is_non_first_fragment);
+ REQUIRE(result.fragment_id.has_value());
+ CHECK(*result.fragment_id == 0x2A);
+ // final_next_header still names TCP (that's what the reassembled
+ // datagram eventually is), but the payload past it is untouched
+ // continuation data - callers must not decode it as TCP.
+ CHECK(result.final_next_header == kProtoTcp);
+ REQUIRE(result.payload.size() == 4);
+ CHECK(result.payload[0] == 0xDE);
+}
+
TEST_CASE("walk_ipv6_extension_headers applies AH's 4-byte-unit length formula") {
// AH: next_header(1)=TCP, payload_len(1)=1 -> total len (1+2)*4=12 bytes.
std::vector<unsigned char> payload(12, 0);
diff --git a/tests/test_net.cpp b/tests/test_net.cpp
index 2702758..64db07f 100644
--- a/tests/test_net.cpp
+++ b/tests/test_net.cpp
@@ -134,6 +134,37 @@ TEST_CASE("parse_ipv4 honors IHL > 5 (options present)") {
CHECK(ip->payload.empty());
}
+TEST_CASE("parse_ipv4 decodes an unfragmented packet with fragment_offset zero") {
+ std::vector<unsigned char> bytes(20, 0);
+ bytes[0] = 0x45;
+ auto ip = parse_ipv4(bytes);
+ REQUIRE(ip.has_value());
+ CHECK_FALSE(ip->header.more_fragments);
+ CHECK(ip->header.fragment_offset == 0);
+}
+
+TEST_CASE("parse_ipv4 decodes the identification field and More Fragments flag") {
+ std::vector<unsigned char> bytes(20, 0);
+ bytes[0] = 0x45;
+ bytes[4] = 0x12; bytes[5] = 0x34; // identification = 0x1234
+ bytes[6] = 0x20; bytes[7] = 0x00; // flags: MF=1, fragment_offset=0 (first fragment)
+ auto ip = parse_ipv4(bytes);
+ REQUIRE(ip.has_value());
+ CHECK(ip->header.identification == 0x1234);
+ CHECK(ip->header.more_fragments);
+ CHECK(ip->header.fragment_offset == 0);
+}
+
+TEST_CASE("parse_ipv4 decodes a nonzero fragment_offset in 8-byte units") {
+ std::vector<unsigned char> bytes(20, 0);
+ bytes[0] = 0x45;
+ bytes[6] = 0x00; bytes[7] = 0x08; // fragment_offset = 8 (i.e. byte offset 64), MF=0 (last fragment)
+ auto ip = parse_ipv4(bytes);
+ REQUIRE(ip.has_value());
+ CHECK_FALSE(ip->header.more_fragments);
+ CHECK(ip->header.fragment_offset == 8);
+}
+
TEST_CASE("parse_tcp decodes header fields and flags") {
std::vector<unsigned char> bytes(20, 0);
bytes[0] = 0x00; bytes[1] = 0x50; // src port 80
diff --git a/tests/test_summarize.cpp b/tests/test_summarize.cpp
index f180f87..baf4ed4 100644
--- a/tests/test_summarize.cpp
+++ b/tests/test_summarize.cpp
@@ -186,6 +186,34 @@ TEST_CASE("summarize_packet decodes an ARP request end to end") {
"ARP who-has 10.0.0.2 tell 10.0.0.1 (aa:bb:cc:dd:ee:ff)");
}
+TEST_CASE("summarize_packet reports a non-first IPv4 fragment without decoding fake TCP/UDP") {
+ // Payload here is arbitrary bytes - if this were mistakenly
+ // handed to a transport parser it would produce a plausible-
+ // looking but entirely fake TCP/UDP line. The point of this test
+ // is that it must not.
+ std::vector<unsigned char> fake_continuation_data = {0xDE, 0xAD, 0xBE, 0xEF, 0x00, 0x01, 0x02, 0x03};
+
+ std::vector<unsigned char> ip(20, 0);
+ ip[0] = 0x45;
+ ip[4] = 0x00; ip[5] = 0x7B; // identification = 123
+ ip[6] = 0x00; ip[7] = 0x08; // fragment_offset = 8 (byte offset 64), MF=0
+ ip[9] = packeteer::net::kProtoTcp;
+ ip[12] = 10; ip[13] = 0; ip[14] = 0; ip[15] = 1;
+ ip[16] = 10; ip[17] = 0; ip[18] = 0; ip[19] = 2;
+
+ std::vector<unsigned char> eth = {
+ 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF, 0x08, 0x00,
+ };
+
+ std::vector<unsigned char> frame = eth;
+ frame.insert(frame.end(), ip.begin(), ip.end());
+ frame.insert(frame.end(), fake_continuation_data.begin(), fake_continuation_data.end());
+
+ auto line = packeteer::summarize_packet(frame, DLT_EN10MB);
+ CHECK(line.find("fragment id=123 offset=64") != std::string::npos);
+ CHECK(line.find("TCP") == std::string::npos); // must not have decoded the fake continuation data
+}
+
TEST_CASE("summarize_packet falls back to the RTCP heuristic on an unmatched UDP port") {
std::vector<unsigned char> rtcp = {0x80, 0xC9, 0x00, 0x01, 0, 0, 0, 0}; // RR, len=1 -> 8 bytes
generated by cgit v1.2.3 (git 2.39.1) at 2026-10-06 08:31:38 +0000