Improve TLS ClientHello SNI parsing (research mode)

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

@@ -153,6 +153,14 @@ int is_upper_layer(int protocol) {
 // Can be enabled at runtime via jni_sni() for research purposes.
 int is_play = 0;
 
+// Upper bound on bytes buffered while reassembling a ClientHello that spans
+// multiple TCP segments (research-mode SNI extraction). A single TLS record is
+// at most 2^14 + 5 bytes; real ClientHellos are well under this even with
+// post-quantum key shares. The buffer is per-session, allocated only for a
+// split ClientHello, and freed as soon as the record is complete or the cap is
+// reached, so the battery/memory cost stays bounded and confined to is_play.
+#define TLS_SNI_MAX_BUFFER 16384
+
 void handle_ip(const struct arguments *args,
                const uint8_t *pkt, const size_t length,
                const int epoll_fd,
@@ -345,6 +353,10 @@ void handle_ip(const struct arguments *args,
     else {
         struct ng_session *cur = NULL;
         char* packetdata = data;
+        // While a ClientHello is still being reassembled across TCP segments we
+        // let the segment through but postpone the block decision until the SNI
+        // is available (or we give up), so it is not made on partial evidence.
+        int defer_sni = 0;
 
         // Check if we have a CLIENT HELLO, and if so extract SNI
         if (protocol == IPPROTO_TCP && dport == 443 && !syn && is_play) {
@@ -358,8 +370,12 @@ void handle_ip(const struct arguments *args,
             const uint8_t *data = payload + sizeof(struct tcphdr) + tcpoptlen;
             const uint16_t datalen = (const uint16_t) (length - (data - pkt));
 
-            // Search existing TCP session
-            struct ng_session *cur = args->ctx->ng_session;
+            // Search existing TCP session (created on the SYN). Assign the
+            // outer cur so the block-decision below sees the same session
+            // instead of a shadowed local that stayed NULL (which meant
+            // checkedHostname was never set and the decision re-ran every
+            // packet).
+            cur = args->ctx->ng_session;
             while (cur != NULL &&
                    !(cur->protocol == IPPROTO_TCP &&
                      cur->tcp.version == version &&
@@ -370,13 +386,56 @@ void handle_ip(const struct arguments *args,
                                      memcmp(&cur->tcp.daddr.ip6, &ip6->ip6_dst, 16) == 0)))
                 cur = cur->next;
 
-            // Try to parse Server Name Extension
-            if (cur != NULL && cur->tcp.checkedHostname == 0) {                
-                char hostname[512] = "";
-                parse_tls_header((const char *) data, datalen, hostname);
-                if (strnlen(hostname, 512) > 0) {
-                    log_android(ANDROID_LOG_DEBUG, "Seen SNI: %s", hostname);
-                    packetdata = hostname;
+            // Try to parse the Server Name Indication once per session. A
+            // ClientHello can span several TCP segments; buffer up to
+            // TLS_SNI_MAX_BUFFER bytes and retry instead of losing the hostname
+            // when it does not fit in the first segment.
+            if (cur != NULL && cur->tcp.checkedHostname == 0) {
+                char hostname[FQDN_MAX + 1] = "";
+                int rc;
+
+                if (datalen == 0) {
+                    // Bare ACK (e.g. completing the TCP handshake) or FIN:
+                    // no ClientHello bytes to parse yet. Keep waiting for
+                    // data without spending the reassembly buffer.
+                    defer_sni = 1;
+                } else if (cur->tcp.tls_data == NULL) {
+                    // First segment: try to parse it on its own.
+                    rc = parse_tls_header((const char *) data, datalen, hostname);
+                    if (rc == TLS_PARSE_INCOMPLETE && datalen < TLS_SNI_MAX_BUFFER) {
+                        // ClientHello continues in later segments: start
+                        // bounded reassembly.
+                        cur->tcp.tls_data = ng_malloc(TLS_SNI_MAX_BUFFER, "tls sni");
+                        if (cur->tcp.tls_data != NULL) {
+                            memcpy(cur->tcp.tls_data, data, datalen);
+                            cur->tcp.tls_len = datalen;
+                            defer_sni = 1;
+                        }
+                    }
+                } else {
+                    // Continuation: append this segment and re-parse the
+                    // accumulated record.
+                    uint16_t space = (uint16_t) (TLS_SNI_MAX_BUFFER - cur->tcp.tls_len);
+                    uint16_t copy = datalen < space ? datalen : space;
+                    memcpy(cur->tcp.tls_data + cur->tcp.tls_len, data, copy);
+                    cur->tcp.tls_len += copy;
+                    rc = parse_tls_header((const char *) cur->tcp.tls_data,
+                                          cur->tcp.tls_len, hostname);
+                    if (rc == TLS_PARSE_INCOMPLETE && cur->tcp.tls_len < TLS_SNI_MAX_BUFFER)
+                        defer_sni = 1; // still need more segments
+                }
+
+                if (!defer_sni) {
+                    // Determined: SNI found, no SNI, invalid, or cap reached.
+                    if (strnlen(hostname, sizeof(hostname)) > 0) {
+                        log_android(ANDROID_LOG_DEBUG, "Seen SNI: %s", hostname);
+                        packetdata = hostname;
+                    }
+                    if (cur->tcp.tls_data != NULL) {
+                        ng_free(cur->tcp.tls_data, __FILE__, __LINE__);
+                        cur->tcp.tls_data = NULL;
+                        cur->tcp.tls_len = 0;
+                    }
                 }
             }
 
@@ -389,17 +448,27 @@ void handle_ip(const struct arguments *args,
             allowed = 1;
         }
 
-        // No existing TCP session, or unhandled TLS session?
-        if (cur == NULL || cur->tcp.checkedHostname == 0) {
+        // No existing TCP session, or unhandled TLS session? Skip while a
+        // ClientHello is still being reassembled (the segment is already
+        // allowed to pass; the decision waits for the SNI).
+        if (!defer_sni && (cur == NULL || cur->tcp.checkedHostname == 0)) {
             jobject objPacket = create_packet(
                     args, version, protocol, flags, source, sport, dest, dport, packetdata, uid, 0);
             redirect = is_address_allowed(args, objPacket);
             allowed = (redirect != NULL);
             if (redirect != NULL && (*redirect->raddr == 0 || redirect->rport == 0))
                 redirect = NULL;
 
-            if (cur != NULL)
+            if (cur != NULL) {
                 cur->tcp.checkedHostname = 1;
+                // A blocked verdict on an established session must reset the
+                // connection: dropping only this segment is undone by TCP
+                // retransmission, because later segments skip the
+                // once-per-session decision above and pass with allowed = 1.
+                // Mirrors handle_tcp's write_rst for blocked new sessions.
+                if (!allowed)
+                    write_rst(args, &cur->tcp);
+            }
         }
     }
 

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

@@ -213,6 +213,8 @@ struct tcp_session {
     struct segment *forward;
 
     int checkedHostname;
+    uint8_t *tls_data; // buffered ClientHello for cross-segment SNI reassembly (research mode)
+    uint16_t tls_len;  // bytes currently buffered in tls_data
 };
 
 struct ng_session {

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

@@ -34,6 +34,11 @@ void clear_tcp_data(struct tcp_session *cur) {
         ng_free(p->data, __FILE__, __LINE__);
         ng_free(p, __FILE__, __LINE__);
     }
+    if (cur->tls_data != NULL) {
+        ng_free(cur->tls_data, __FILE__, __LINE__);
+        cur->tls_data = NULL;
+        cur->tls_len = 0;
+    }
 }
 
 int get_tcp_timeout(const struct tcp_session *t, int sessions, int maxsessions) {
@@ -764,6 +769,9 @@ jboolean handle_tcp(const struct arguments *args,
             s->tcp.state = TCP_LISTEN;
             s->tcp.socks5 = SOCKS5_NONE;
             s->tcp.forward = NULL;
+            s->tcp.checkedHostname = 0;
+            s->tcp.tls_data = NULL;
+            s->tcp.tls_len = 0;
             s->next = NULL;
 
             if (datalen) {

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

@@ -34,6 +34,8 @@
 #include <string.h> /* strncpy() */
 #include <sys/socket.h>
 
+#include "tls.h"
+
 #define TLS_HEADER_LEN 5
 #define TLS_HANDSHAKE_CONTENT_TYPE 0x16
 #define TLS_HANDSHAKE_TYPE_CLIENT_HELLO 0x01
@@ -44,44 +46,49 @@
 
 
 /* Parse a TLS packet for the Server Name Indication extension in the client
- * hello handshake, returning the first servername found (pointer to static
- * array)
+ * hello handshake, writing the first server name found into *hostname (a
+ * caller-provided buffer of at least FQDN_MAX + 1 bytes).
  *
  * Returns:
- *  >=0  - length of the hostname and updates *hostname
- *         caller is responsible for freeing *hostname
- *  -1   - Incomplete request
- *  -2   - No Host header included in this request
- *  -3   - Invalid hostname pointer
- *  -4   - malloc failure
- *  < -4 - Invalid TLS client hello
+ *  >= 0                     - length of the hostname written to *hostname
+ *  TLS_PARSE_INCOMPLETE(-1) - the TLS record is not fully present yet; the
+ *                             caller may buffer more TCP payload and retry
+ *  TLS_PARSE_NO_SNI  (-2)   - a complete ClientHello with no server name
+ *  TLS_PARSE_INVALID (-5)   - not a TLS ClientHello handshake
  */
-#define FQDN_MAX	255
 
-static void parse_server_name_extension(const char *data, size_t data_len,
-                                        char *hostname)
+static int parse_server_name_extension(const char *data, size_t data_len,
+                                       char *hostname)
 {
     size_t pos = 2; /* skip server name list length */
 
-    while (pos + 3 < data_len) {
+    /*
+     * Each entry is a 3-byte header (1 byte name type + 2 byte length)
+     * followed by <length> bytes of name. Reading the header needs pos,
+     * pos + 1 and pos + 2 in range, i.e. pos + 3 <= data_len. The original
+     * "pos + 3 < data_len" dropped a name entry ending exactly at the buffer
+     * boundary.
+     */
+    while (pos + 3 <= data_len) {
         size_t len = ((unsigned char)data[pos + 1] << 8) +
                      (unsigned char)data[pos + 2];
 
         if (pos + 3 + len > data_len)
-            return;
+            return TLS_PARSE_NO_SNI;
 
         switch (data[pos]) { /* name type */
             case 0x00: /* host_name */
                 len = MIN(len, FQDN_MAX);
                 strncpy(hostname, data + pos + 3, len);
                 hostname[len] = '\0';
-                return;
+                return (int) len;
         }
         pos += 3 + len;
     }
+    return TLS_PARSE_NO_SNI;
 }
 
-static void parse_extensions(const char *data, size_t data_len, char *hostname)
+static int parse_extensions(const char *data, size_t data_len, char *hostname)
 {
     size_t pos = 0;
 
@@ -99,21 +106,21 @@ static void parse_extensions(const char *data, size_t data_len, char *hostname)
              * of the extension here
              */
             if (pos + 4 + len > data_len)
-                return;
-            parse_server_name_extension(data + pos + 4, len,
-                                        hostname);
-            return;
+                return TLS_PARSE_NO_SNI;
+            return parse_server_name_extension(data + pos + 4, len,
+                                               hostname);
         }
         pos += 4 + len; /* Advance to the next extension header */
     }
+    return TLS_PARSE_NO_SNI;
 }
 
 /*
  * Parse a TLS packet for the Server Name Indication extension in the client
- * hello handshake, returning the first servername found (pointer to static
- * array)
+ * hello handshake, writing the first server name found into *hostname.
+ * See the block comment above for return values.
  */
-void parse_tls_header(const char *data, size_t data_len, char *hostname)
+int parse_tls_header(const char *data, size_t data_len, char *hostname)
 {
     char tls_content_type;
     char tls_version_major;
@@ -126,7 +133,7 @@ void parse_tls_header(const char *data, size_t data_len, char *hostname)
      * TLS header
      */
     if (data_len < TLS_HEADER_LEN)
-        return;
+        return TLS_PARSE_INCOMPLETE;
 
     /*
      * SSL 2.0 compatible Client Hello
@@ -135,36 +142,37 @@ void parse_tls_header(const char *data, size_t data_len, char *hostname)
      *
      * See RFC5246 Appendix E.2
      */
-    if (data[0] & 0x80 && data[2] == 1) {
-        return;
-    }
+    if (data[0] & 0x80 && data[2] == 1)
+        return TLS_PARSE_INVALID;
 
     tls_content_type = data[0];
-    if (tls_content_type != TLS_HANDSHAKE_CONTENT_TYPE) {
-        return;
-    }
+    if (tls_content_type != TLS_HANDSHAKE_CONTENT_TYPE)
+        return TLS_PARSE_INVALID;
 
     tls_version_major = data[1];
     tls_version_minor = data[2];
-    if (tls_version_major < 3) {
-        return;
-    }
+    if (tls_version_major < 3)
+        return TLS_PARSE_INVALID;
 
-    /* TLS record length */
+    /* Full TLS record length (5-byte header + payload) */
     len = ((unsigned char)data[3] << 8) +
           (unsigned char)data[4] + TLS_HEADER_LEN;
-    data_len = MIN(data_len, len);
 
-    /* Check we received entire TLS record length */
+    /*
+     * A ClientHello can be split across several TCP segments (large extension
+     * sets, post-quantum key shares). If the whole record is not present yet,
+     * ask the caller to buffer more rather than silently giving up — which
+     * previously lost the SNI for good.
+     */
     if (data_len < len)
-        return;
+        return TLS_PARSE_INCOMPLETE;
+    data_len = len;
 
     /* Handshake */
     if (pos + 1 > data_len)
-        return;
-    if (data[pos] != TLS_HANDSHAKE_TYPE_CLIENT_HELLO) {
-        return;
-    }
+        return TLS_PARSE_NO_SNI;
+    if (data[pos] != TLS_HANDSHAKE_TYPE_CLIENT_HELLO)
+        return TLS_PARSE_INVALID;
 
     /*
      * Skip past fixed length records:
@@ -178,35 +186,34 @@ void parse_tls_header(const char *data, size_t data_len, char *hostname)
 
     /* Session ID */
     if (pos + 1 > data_len)
-        return;
+        return TLS_PARSE_NO_SNI;
     len = (unsigned char)data[pos];
     pos += 1 + len;
 
     /* Cipher Suites */
     if (pos + 2 > data_len)
-        return;
+        return TLS_PARSE_NO_SNI;
     len = ((unsigned char)data[pos] << 8) + (unsigned char)data[pos + 1];
     pos += 2 + len;
 
     /* Compression Methods */
     if (pos + 1 > data_len)
-        return;
+        return TLS_PARSE_NO_SNI;
     len = (unsigned char)data[pos];
     pos += 1 + len;
 
     if (pos == data_len && tls_version_major == 3 &&
-        tls_version_minor == 0) {
-        return;
-    }
+        tls_version_minor == 0)
+        return TLS_PARSE_NO_SNI;
 
     /* Extensions */
     if (pos + 2 > data_len)
-        return;
+        return TLS_PARSE_NO_SNI;
     len = ((unsigned char)data[pos] << 8) + (unsigned char)data[pos + 1];
     pos += 2;
 
     if (pos + len > data_len)
-        return;
+        return TLS_PARSE_NO_SNI;
 
-    parse_extensions(data + pos, len, hostname);
+    return parse_extensions(data + pos, len, hostname);
 }