Fix IPv6 extension header walk in handle_ip()

.github/workflows/test.yml

@@ -73,6 +73,13 @@ jobs:
             app/src/test/native/dns_frame_test.c app/src/main/jni/netguard/dns_frame.c
           /tmp/dns_frame_test
 
+      - name: Run IPv6 extension header walk host tests
+        run: |
+          cc -Wall -Wextra -Werror -Iapp/src/main/jni/netguard \
+            -o /tmp/ip6_ext_test \
+            app/src/test/native/ip6_ext_test.c app/src/main/jni/netguard/ip6_ext.c
+          /tmp/ip6_ext_test
+
       - name: Run unit tests
         run: ./gradlew testFdroidDebugUnitTest --offline
 

app/CMakeLists.txt

@@ -12,6 +12,7 @@ add_library( netguard
              src/main/jni/netguard/netguard.c
              src/main/jni/netguard/session.c
              src/main/jni/netguard/ip.c
+             src/main/jni/netguard/ip6_ext.c
              src/main/jni/netguard/policy.c
              src/main/jni/netguard/tls.c
              src/main/jni/netguard/tcp.c

app/src/main/jni/netguard/ip.c

@@ -19,6 +19,7 @@
 
 #include "netguard.h"
 #include "tls.h"
+#include "ip6_ext.h"
 #include <stdatomic.h>
 
 int max_tun_msg = 0;
@@ -267,27 +268,6 @@ int check_tun(const struct arguments *args,
     return 0;
 }
 
-// https://en.wikipedia.org/wiki/IPv6_packet#Extension_headers
-// http://www.iana.org/assignments/protocol-numbers/protocol-numbers.xhtml
-int is_lower_layer(int protocol) {
-    // No next header = 59
-    return (protocol == 0 || // Hop-by-Hop Options
-            protocol == 60 || // Destination Options (before routing header)
-            protocol == 43 || // Routing
-            protocol == 44 || // Fragment
-            protocol == 51 || // Authentication Header (AH)
-            protocol == 50 || // Encapsulating Security Payload (ESP)
-            protocol == 60 || // Destination Options (before upper-layer header)
-            protocol == 135); // Mobility
-}
-
-int is_upper_layer(int protocol) {
-    return (protocol == IPPROTO_TCP ||
-            protocol == IPPROTO_UDP ||
-            protocol == IPPROTO_ICMP ||
-            protocol == IPPROTO_ICMPV6);
-}
-
 // SNI extraction disabled by default: connecting to tracker IPs to read TLS
 // ClientHello leaks the user's IP address to the tracker server.
 // Can be enabled at runtime via jni_sni() for research purposes.
@@ -363,31 +343,18 @@ void handle_ip(const struct arguments *args,
 
         struct ip6_hdr *ip6hdr = (struct ip6_hdr *) pkt;
 
-        // Skip extension headers
-        uint16_t off = 0;
-        protocol = ip6hdr->ip6_nxt;
-        if (!is_upper_layer(protocol)) {
-            log_android(ANDROID_LOG_WARN, "IP6 extension %d", protocol);
-            off = sizeof(struct ip6_hdr);
-            struct ip6_ext *ext = (struct ip6_ext *) (pkt + off);
-            while (is_lower_layer(ext->ip6e_nxt) && !is_upper_layer(protocol)) {
-                protocol = ext->ip6e_nxt;
-                log_android(ANDROID_LOG_WARN, "IP6 extension %d", protocol);
-
-                off += (8 + ext->ip6e_len);
-                ext = (struct ip6_ext *) (pkt + off);
-            }
-            if (!is_upper_layer(protocol)) {
-                off = 0;
-                protocol = ip6hdr->ip6_nxt;
-                log_android(ANDROID_LOG_WARN, "IP6 final extension %d", protocol);
-            }
-        }
+        // Skip extension headers. ip6_skip_ext_headers() (ip6_ext.c) owns the
+        // walk -- RFC 8200 Hdr Ext Len arithmetic, hard bounds checks against
+        // `length`, and deciding which header types are walkable -- so it can
+        // be unit-tested on the host; see ip6_ext.h for the exact contract.
+        size_t payload_off;
+        if (!ip6_skip_ext_headers(pkt, length, &protocol, &payload_off))
+            log_android(ANDROID_LOG_WARN, "IP6 extension %d not walkable", protocol);
 
         saddr = &ip6hdr->ip6_src;
         daddr = &ip6hdr->ip6_dst;
 
-        payload = (uint8_t *) (pkt + sizeof(struct ip6_hdr) + off);
+        payload = (uint8_t *) (pkt + payload_off);
 
         // TODO checksum
     } else {

app/src/main/jni/netguard/ip6_ext.c

@@ -0,0 +1,136 @@
+/*
+    This file is part of NetGuard.
+
+    NetGuard is free software: you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation, either version 3 of the License, or
+    (at your option) any later version.
+
+    NetGuard is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with NetGuard.  If not, see <http://www.gnu.org/licenses/>.
+
+    Copyright 2015-2019 by Marcel Bokhorst (M66B)
+*/
+
+#include "ip6_ext.h"
+
+#include <netinet/in.h>
+
+/*
+ * Extension header "Next Header" values that are not IPPROTO_* upper-layer
+ * protocols and cannot be walked further, but that this engine still wants
+ * to name accurately in logs/decisions rather than lump in with a generic
+ * "unknown". Not all libc's define every one of these (IPPROTO_NONE and
+ * IPPROTO_DSTOPTS in particular vary), so they are given literal values
+ * straight from the IANA protocol-numbers registry rather than relying on
+ * <netinet/in.h> to supply them.
+ */
+#define IP6_EXT_HOPOPTS  0
+#define IP6_EXT_ROUTING  43
+#define IP6_EXT_FRAGMENT 44
+#define IP6_EXT_ESP      50
+#define IP6_EXT_AH       51
+#define IP6_EXT_DSTOPTS  60
+#define IP6_EXT_NONE     59 /* "No Next Header" */
+
+/*
+ * Defensive cap on the number of extension headers walked for a single
+ * packet. Legitimate chains are one or two headers long; this exists only
+ * to bound a maliciously (or corruptly) long chain, not to accommodate
+ * real traffic -- the `length` bound below already makes the loop
+ * terminate, but a packet can still carry many minimum-size (8-byte)
+ * extension headers, so an explicit cap keeps the walk cheap regardless.
+ */
+#define MAX_IP6_EXT_HEADERS 8
+
+static int is_upper_layer_protocol(uint8_t protocol) {
+    return protocol == IPPROTO_TCP ||
+           protocol == IPPROTO_UDP ||
+           protocol == IPPROTO_ICMP ||
+           protocol == IPPROTO_ICMPV6;
+}
+
+int ip6_skip_ext_headers(const uint8_t *pkt, size_t length,
+                          uint8_t *protocol_out, size_t *payload_off_out) {
+    if (pkt == NULL || protocol_out == NULL || payload_off_out == NULL)
+        return 0;
+
+    if (length < IP6_EXT_FIXED_HDR_LEN) {
+        /* Defensive only: handle_ip() already rejects a packet shorter than
+         * the fixed IPv6 header before ever calling this. Report a value
+         * that can never be mistaken for TCP/UDP/ICMP and never point
+         * payload_off_out past the buffer. */
+        *protocol_out = IP6_EXT_NONE;
+        *payload_off_out = length;
+        return 0;
+    }
+
+    /* The fixed IPv6 header's Next Header field is byte 6 (after the 4-byte
+     * Version/Traffic Class/Flow Label and the 2-byte Payload Length). */
+    uint8_t next = pkt[6];
+    size_t off = IP6_EXT_FIXED_HDR_LEN;
+
+    for (int hops = 0; ; hops++) {
+        if (is_upper_layer_protocol(next)) {
+            *protocol_out = next;
+            *payload_off_out = off;
+            return 1;
+        }
+
+        /* Headers that cannot be walked at all: report and stop right where
+         * we are, without trying to read a length field that (for Fragment)
+         * does not mean what it would for the walkable headers, or (for
+         * ESP/No-Next-Header/anything unrecognised) simply is not there to
+         * find plaintext structure behind. */
+        if (next == IP6_EXT_FRAGMENT || next == IP6_EXT_ESP || next == IP6_EXT_NONE ||
+            !(next == IP6_EXT_HOPOPTS || next == IP6_EXT_ROUTING ||
+              next == IP6_EXT_DSTOPTS || next == IP6_EXT_AH)) {
+            *protocol_out = next;
+            *payload_off_out = off;
+            return 0;
+        }
+
+        if (hops >= MAX_IP6_EXT_HEADERS) {
+            /* Chain too long -- bail out where we are rather than keep
+             * walking an attacker-controlled sequence of headers. */
+            *protocol_out = next;
+            *payload_off_out = off;
+            return 0;
+        }
+
+        /* Every walkable header (Hop-by-Hop, Routing, Destination Options,
+         * AH) starts with a 1-byte Next Header and a 1-byte length field;
+         * need both in bounds before reading either. */
+        if (off + 2 > length) {
+            *protocol_out = next;
+            *payload_off_out = off;
+            return 0;
+        }
+
+        uint8_t hdr_next = pkt[off];
+        uint8_t hdr_len = pkt[off + 1];
+
+        /* AH's length is in 4-octet units, not including the first 8
+         * octets, per RFC 4302 section 2.2: (Hdr Ext Len + 2) * 4. Every
+         * other walkable header uses RFC 8200's 8-octet units, not
+         * including the first 8 octets: 8 * (Hdr Ext Len + 1). */
+        size_t advance = (next == IP6_EXT_AH)
+                                 ? ((size_t) hdr_len + 2) * 4
+                                 : 8 * ((size_t) hdr_len + 1);
+
+        if (advance > length || off > length - advance) {
+            /* Declared length runs past the end of the buffer. */
+            *protocol_out = next;
+            *payload_off_out = off;
+            return 0;
+        }
+
+        off += advance;
+        next = hdr_next;
+    }
+}

app/src/main/jni/netguard/ip6_ext.h

@@ -0,0 +1,103 @@
+/*
+    This file is part of NetGuard.
+
+    NetGuard is free software: you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation, either version 3 of the License, or
+    (at your option) any later version.
+
+    NetGuard is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with NetGuard.  If not, see <http://www.gnu.org/licenses/>.
+
+    Copyright 2015-2019 by Marcel Bokhorst (M66B)
+*/
+
+#ifndef IP6_EXT_H
+#define IP6_EXT_H
+
+/*
+ * Pure IPv6 extension header walk, extracted out of handle_ip() (ip.c) so
+ * it can be unit-tested on the host without pulling in JNI/Android
+ * dependencies. This header and its implementation (ip6_ext.c) depend only
+ * on libc (<netinet/in.h> for the IPPROTO_* constants) -- no netguard.h,
+ * no JNI, no struct ip6_hdr/ip6_ext from <netinet/ip6.h>, so the fixed
+ * header size below is duplicated rather than taken from sizeof(struct
+ * ip6_hdr) (the two must and do agree: RFC 8200 defines the fixed IPv6
+ * header as exactly 40 bytes, with no options).
+ *
+ * See https://www.rfc-editor.org/rfc/rfc8200 (section 4) and
+ * https://www.iana.org/assignments/protocol-numbers/protocol-numbers.xhtml
+ */
+
+#include <stddef.h>
+#include <stdint.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/* Fixed IPv6 header length in bytes (RFC 8200 section 3). */
+#define IP6_EXT_FIXED_HDR_LEN 40
+
+/*
+ * Walks the IPv6 extension header chain that starts right after the fixed
+ * 40-byte IPv6 header, looking for an upper-layer protocol this engine can
+ * actually parse (TCP, UDP, ICMP, ICMPv6).
+ *
+ * pkt/length: the full IP packet buffer, exactly as received from the
+ *        tun device. length must be >= IP6_EXT_FIXED_HDR_LEN (the caller
+ *        already rejects shorter packets before calling this).
+ *
+ * On return, *protocol_out and *payload_off_out are always set, and
+ * *payload_off_out is always a valid offset into pkt (<= length):
+ *
+ *   - Return 1: an upper-layer protocol was found. *protocol_out is that
+ *     protocol (TCP/UDP/ICMP/ICMPv6) and *payload_off_out is the offset of
+ *     its header -- exactly what the pre-extraction code intended to
+ *     compute.
+ *
+ *   - Return 0: no upper-layer protocol was found before the walk had to
+ *     stop. *protocol_out is the extension-header type (or other next-
+ *     header value) the walk stopped on, and *payload_off_out is the
+ *     offset of that header (i.e. right after everything successfully
+ *     walked). *protocol_out in this case is guaranteed not to collide
+ *     with TCP/UDP/ICMP/ICMPv6, so a caller that only special-cases those
+ *     four values downstream (as handle_ip does) treats a stopped walk
+ *     exactly like an unparseable/unknown protocol -- it is never
+ *     misread as a real transport header. Reasons to stop:
+ *       - No Next Header (59): a legitimate, clean end of the chain.
+ *       - 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.
+ *       - ESP (50): the payload is encrypted; nothing after it is
+ *         parseable in plaintext.
+ *       - An unrecognised/non-walkable next-header value.
+ *       - The declared header length would run past `length`, or there
+ *         are not even enough bytes left to read the 2-byte extension
+ *         header itself (truncated packet).
+ *       - MAX_IP6_EXT_HEADERS extension headers were walked without
+ *         reaching an upper-layer protocol (a defensively small cap
+ *         against a maliciously long or looping chain).
+ *
+ * Hop-by-Hop (0), Routing (43) and Destination Options (60) are walked
+ * using the standard 8-octet-unit encoding: 8 * (Hdr Ext Len + 1) bytes.
+ * Authentication Header (51) is walked using its own 4-octet-unit
+ * encoding: (Hdr Ext Len + 2) * 4 bytes (RFC 4302 section 2.2) -- AH only
+ * authenticates, it does not encrypt, so the headers after it (including
+ * the real transport header, in AH transport mode) remain in plaintext
+ * and are worth walking into.
+ */
+int ip6_skip_ext_headers(const uint8_t *pkt, size_t length,
+                          uint8_t *protocol_out, size_t *payload_off_out);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* IP6_EXT_H */

app/src/main/jni/netguard/netguard.h

@@ -419,10 +419,6 @@ void check_tcp_socket(const struct arguments *args,
                       const struct epoll_event *ev,
                       const int epoll_fd);
 
-int is_lower_layer(int protocol);
-
-int is_upper_layer(int protocol);
-
 void handle_ip(const struct arguments *args,
                const uint8_t *buffer, size_t length,
                const int epoll_fd,