From 8a8a267fc3ff16b1ad373ca29d439f82691f9766 Mon Sep 17 00:00:00 2001 From: Konrad Kollnig <5175206+kasnder@users.noreply.github.com> Date: Sat, 22 Aug 2026 16:53:31 +0200 Subject: [PATCH 1/2] Document the fragmented-traffic drop on the direct path The IPv6 extension-header walk stops at Fragment (44) and ESP (50), so those packets match no L4 dispatch branch in handle_ip(): an allowed packet is silently blackholed on the direct path. IPv4 fragments (IP_MF, first fragments included) are dropped even earlier -- before the WireGuard hijack, unlike their IPv6 counterparts. Decide the issue's option B: keep dropping (reassembly rejected as a battery/memory cost against traffic PMTUD keeps rare; dispatching first fragments alone would strand mid-flow continuations), document it at both drop sites and in the codebase map, state that ESP stays a permanent limitation, and log the previously silent drop instead of vanishing. See #779. --- agents/docs/codebase-map.md | 12 ++++++++++++ app/src/main/jni/netguard/ip.c | 26 ++++++++++++++++++++++++++ app/src/main/jni/netguard/ip6_ext.h | 3 ++- 3 files changed, 40 insertions(+), 1 deletion(-) diff --git a/agents/docs/codebase-map.md b/agents/docs/codebase-map.md index 90dc270d..da497e0b 100644 --- a/agents/docs/codebase-map.md +++ b/agents/docs/codebase-map.md @@ -69,3 +69,15 @@ which drives future block decisions recorded in `DatabaseHelper`. onboarding); *Standard* loads Disconnect + X-Ray + DDG and **allows** ambiguous shared-IP hosts; *Strict* **blocks** those ambiguous shared-IP hosts. Shared-IP ambiguity + UID-global DNS evidence is the root of a whole cluster of reports. + +## Known limitations (deliberate) + +* **Fragmented traffic is dropped on the direct path.** IPv4 packets with + `IP_MF` set (first fragments included) drop up front; IPv6 packets whose + extension-header chain stops at a Fragment (44) or ESP (50) header never + reach an L4 dispatch branch, so they drop too -- even when the destination + would be allowed. No reassembly engine, on purpose: memory/battery cost + outweighs traffic PMTUD keeps rare. IPv4 fragments drop before the + WireGuard hijack as well, while IPv6 fragmented flows pass through when + WireGuard-routed (raw forward). ESP stays a permanent limitation. + See issue #779. diff --git a/app/src/main/jni/netguard/ip.c b/app/src/main/jni/netguard/ip.c index 1f80b8fb..78828823 100644 --- a/app/src/main/jni/netguard/ip.c +++ b/app/src/main/jni/netguard/ip.c @@ -309,6 +309,13 @@ void handle_ip(const struct arguments *args, saddr = &ip4hdr->saddr; daddr = &ip4hdr->daddr; + // Deliberate: every IPv4 fragment (IP_MF set, first fragments + // included) is dropped on the direct path -- the L4 state machines + // have no reassembly, and a non-first fragment has no header they + // could parse. This return also fires before the WireGuard hijack + // below, so unlike IPv6 fragments, IPv4 ones die even when + // WireGuard-routed. Same standing limitation as the IPv6 + // Fragment/ESP stop further down; see issue #779. if (ip4hdr->frag_off & IP_MF) { log_android(ANDROID_LOG_ERROR, "IP fragment offset %u", (ip4hdr->frag_off & IP_OFFMASK) * 8); @@ -351,6 +358,17 @@ void handle_ip(const struct arguments *args, if (!ip6_skip_ext_headers(pkt, length, &protocol, &payload_off)) log_android(ANDROID_LOG_WARN, "IP6 extension %d not walkable", protocol); + // A stopped walk leaves protocol at the stopping header type -- + // Fragment (44) or ESP (50) in practice -- which matches none of + // the dispatch branches below. On the direct path such a packet is + // therefore dropped even when the destination passes the allow + // check: there is no L4 header to parse, no session to attach to, + // and no raw-forward path in this userspace stack. WireGuard-routed + // flows are unaffected: the WG hijack below forwards them raw + // before dispatch. Fragment reassembly was considered and rejected + // (memory and battery cost against traffic PMTUD keeps rare); ESP + // stays a documented limitation permanently. See issue #779. + saddr = &ip6hdr->ip6_src; daddr = &ip6hdr->ip6_dst; @@ -720,6 +738,14 @@ void handle_ip(const struct arguments *args, handle_udp(args, pkt, length, payload, uid, redirect, epoll_fd); else if (protocol == IPPROTO_TCP) handle_tcp(args, pkt, length, payload, uid, allowed, redirect, epoll_fd); + else { + // Allowed but undispatchable: no L4 handler for this protocol + // (in practice a Fragment/ESP-stopped ext-header walk, see + // above). Drop visibly rather than vanish silently. + log_android(ANDROID_LOG_WARN, + "Protocol %d allowed but not forwardable, dropping", + protocol); + } } else { if (protocol == IPPROTO_UDP) block_udp(args, pkt, length, payload, uid); diff --git a/app/src/main/jni/netguard/ip6_ext.h b/app/src/main/jni/netguard/ip6_ext.h index 78dcd304..ff30d8be 100644 --- a/app/src/main/jni/netguard/ip6_ext.h +++ b/app/src/main/jni/netguard/ip6_ext.h @@ -74,7 +74,8 @@ extern "C" { * - Fragment (44): ip6e_len is a reserved field for this header, * not a length, so it cannot be walked; the packet may also not * be first-fragment, so there may be no upper-layer header here - * at all. + * at all. The resulting direct-path drop of fragmented flows is + * a documented limitation (issue #779). * - ESP (50): the payload is encrypted; nothing after it is * parseable in plaintext. * - An unrecognised/non-walkable next-header value. From 299251c2fc75d45dd90d01f695ddf542e1db6f04 Mon Sep 17 00:00:00 2001 From: Konrad Kollnig <5175206+kasnder@users.noreply.github.com> Date: Sat, 22 Aug 2026 15:19:21 +0000 Subject: [PATCH 2/2] Fix the IPv4 fragment test the #779 comment describes frag_off is network byte order but IP_MF and IP_OFFMASK are host-order constants, and the test had no ntohs. On little-endian Android that never looked at More Fragments at all: it tested bit 13 of the byte-swapped value, i.e. bit 0x20 of the real field's low byte, so the drop fired on an arbitrary slice of fragment offsets (256-511 bytes, 768-1023, ...). True first fragments (MF set, offset 0) fell through and were parsed as whole packets; last fragments (MF clear, offset non-zero) fell through too and had their payload read as a TCP/UDP header, yielding garbage ports. The ERROR log printed the same swapped value, which is why the offsets in it never looked wrong enough to chase. Convert once and test both halves, so first, middle and last fragments alike drop, and the comment added alongside it is true of the code. Also rate-limit the new undispatchable-protocol WARN the way the two WireGuard drop paths in this function already do -- a single ESP flow hits it every packet -- and keep HOPOPTS/IGMP/ESP out of that log, mirroring the exemption at the "Unknown protocol" site above. The throttle counts on its own counter: sharing one with the exempt protocols would let steady ESP traffic eat every slot and silence the protocols worth seeing. --- agents/docs/codebase-map.md | 3 ++- app/src/main/jni/netguard/ip.c | 46 +++++++++++++++++++++++++--------- 2 files changed, 36 insertions(+), 13 deletions(-) diff --git a/agents/docs/codebase-map.md b/agents/docs/codebase-map.md index da497e0b..a67df225 100644 --- a/agents/docs/codebase-map.md +++ b/agents/docs/codebase-map.md @@ -73,7 +73,8 @@ ambiguity + UID-global DNS evidence is the root of a whole cluster of reports. ## Known limitations (deliberate) * **Fragmented traffic is dropped on the direct path.** IPv4 packets with - `IP_MF` set (first fragments included) drop up front; IPv6 packets whose + `IP_MF` set or a non-zero fragment offset (first, middle and last + fragments alike) drop up front; IPv6 packets whose extension-header chain stops at a Fragment (44) or ESP (50) header never reach an L4 dispatch branch, so they drop too -- even when the destination would be allowed. No reassembly engine, on purpose: memory/battery cost diff --git a/app/src/main/jni/netguard/ip.c b/app/src/main/jni/netguard/ip.c index 78828823..897ba4dc 100644 --- a/app/src/main/jni/netguard/ip.c +++ b/app/src/main/jni/netguard/ip.c @@ -29,6 +29,8 @@ extern _Atomic int wg_required; static atomic_long wg_drop_count = 0; static atomic_long wg_gap_drop_count = 0; +static atomic_long undispatchable_drop_count = 0; +static atomic_long undispatchable_log_count = 0; // Skip tunneling for addresses WireGuard cannot meaningfully forward // (multicast, link-local, loopback). Apps targeting these wouldn't gain @@ -309,16 +311,20 @@ void handle_ip(const struct arguments *args, saddr = &ip4hdr->saddr; daddr = &ip4hdr->daddr; - // Deliberate: every IPv4 fragment (IP_MF set, first fragments - // included) is dropped on the direct path -- the L4 state machines - // have no reassembly, and a non-first fragment has no header they - // could parse. This return also fires before the WireGuard hijack + uint16_t frag_off = ntohs(ip4hdr->frag_off); + + // Deliberate: every IPv4 fragment -- MF set or a non-zero offset, + // so first, middle and last alike -- is dropped on the direct path. + // The L4 state machines have no reassembly, and a non-first + // fragment has no header they could parse. This return also fires before the WireGuard hijack // below, so unlike IPv6 fragments, IPv4 ones die even when // WireGuard-routed. Same standing limitation as the IPv6 - // Fragment/ESP stop further down; see issue #779. - if (ip4hdr->frag_off & IP_MF) { - log_android(ANDROID_LOG_ERROR, "IP fragment offset %u", - (ip4hdr->frag_off & IP_OFFMASK) * 8); + // Fragment/ESP stop further down; see issue #779. The ntohs above + // is load-bearing: frag_off is network byte order but IP_MF and + // IP_OFFMASK are host-order constants; don't simplify it away. + if ((frag_off & IP_MF) || (frag_off & IP_OFFMASK)) { + log_android(ANDROID_LOG_ERROR, "IP fragment MF %d offset %u", + (frag_off & IP_MF) != 0, (frag_off & IP_OFFMASK) * 8); return; } @@ -741,10 +747,26 @@ void handle_ip(const struct arguments *args, else { // Allowed but undispatchable: no L4 handler for this protocol // (in practice a Fragment/ESP-stopped ext-header walk, see - // above). Drop visibly rather than vanish silently. - log_android(ANDROID_LOG_WARN, - "Protocol %d allowed but not forwardable, dropping", - protocol); + // above). Drop visibly rather than vanish silently, but + // rate-limit: a single ESP/IPsec flow can hit this every + // packet and would otherwise spam logcat. + long drops = atomic_fetch_add_explicit( + &undispatchable_drop_count, 1, memory_order_relaxed) + 1; + // HOPOPTS/IGMP/ESP are exempted from the log here too, mirroring + // the "Unknown protocol" exemption above -- still counted so the + // running total stays honest, just never logged. The throttle + // runs off its own counter: sharing one with the exempt + // protocols would let a steady ESP flow eat every slot and + // silence the protocols worth seeing. + if (protocol != IPPROTO_HOPOPTS && protocol != IPPROTO_IGMP && + protocol != IPPROTO_ESP) { + long logged = atomic_fetch_add_explicit( + &undispatchable_log_count, 1, memory_order_relaxed) + 1; + if ((logged & 1023L) == 1) + log_android(ANDROID_LOG_WARN, + "Protocol %d allowed but not forwardable, dropped %ld packets (%ld total)", + protocol, logged, drops); + } } } else { if (protocol == IPPROTO_UDP)