From ed09b3ce8ff6c0bb04220c5010c424b6e956addd Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 14 Jul 2026 22:45:08 +0000 Subject: [PATCH 1/4] Improve TLS ClientHello SNI parsing (research mode) Addresses the tls.c gaps raised in #654/#655 for the opt-in SNI extraction path (jni_sni / is_play). parse_tls_header: - Return status codes instead of void so the caller can tell an incomplete TLS record (buffer more) apart from a complete record with no SNI or a non-TLS payload. TLS_PARSE_INCOMPLETE is returned whenever the record length exceeds the bytes present. - Fix the off-by-one in parse_server_name_extension: the entry-header bound was `pos + 3 < data_len`, which dropped a server name ending exactly at the buffer boundary. It is now `pos + 3 <= data_len`. ip.c: - Fix a variable-shadowing bug: the TCP session found for a 443 flow was stored in a shadowed local, so the outer `cur` stayed NULL. As a result checkedHostname was never set and the block decision re-ran on every packet. The found session is now assigned to the single `cur`. - Reassemble a ClientHello that spans multiple TCP segments. When the first segment yields an incomplete record, buffer the payload per session (bounded by TLS_SNI_MAX_BUFFER) and re-parse as later segments arrive, instead of losing the SNI for good. The block decision is postponed while reassembling so it is not made on partial evidence. The buffer is allocated only for a split ClientHello and freed as soon as the record completes or the cap is reached, keeping the cost bounded and confined to research mode. tcp.c / netguard.h: - Initialise checkedHostname at session creation (previously read from uninitialised malloc memory) and add the reassembly buffer fields, freed in clear_tcp_data. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019fqmytQ1CBbvpXKrweEm7z --- app/src/main/jni/netguard/ip.c | 78 ++++++++++++++++--- app/src/main/jni/netguard/netguard.h | 2 + app/src/main/jni/netguard/tcp.c | 8 ++ app/src/main/jni/netguard/tls.c | 107 ++++++++++++++------------- app/src/main/jni/netguard/tls.h | 14 +++- 5 files changed, 147 insertions(+), 62 deletions(-) diff --git a/app/src/main/jni/netguard/ip.c b/app/src/main/jni/netguard/ip.c index dab033500..848ae3bd5 100644 --- a/app/src/main/jni/netguard/ip.c +++ b/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,51 @@ 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 (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,8 +443,10 @@ 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); diff --git a/app/src/main/jni/netguard/netguard.h b/app/src/main/jni/netguard/netguard.h index 3069416f4..7ec615db1 100644 --- a/app/src/main/jni/netguard/netguard.h +++ b/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 { diff --git a/app/src/main/jni/netguard/tcp.c b/app/src/main/jni/netguard/tcp.c index c2c4461ce..770ec224d 100644 --- a/app/src/main/jni/netguard/tcp.c +++ b/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) { diff --git a/app/src/main/jni/netguard/tls.c b/app/src/main/jni/netguard/tls.c index 384e7edd7..03f839140 100644 --- a/app/src/main/jni/netguard/tls.c +++ b/app/src/main/jni/netguard/tls.c @@ -34,6 +34,8 @@ #include /* strncpy() */ #include +#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 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); } \ No newline at end of file diff --git a/app/src/main/jni/netguard/tls.h b/app/src/main/jni/netguard/tls.h index 3a3d1bb9c..b6ae1caf4 100644 --- a/app/src/main/jni/netguard/tls.h +++ b/app/src/main/jni/netguard/tls.h @@ -27,7 +27,19 @@ #define TLS_H #include +#include -void parse_tls_header(const char *data, size_t data_len, char *hostname); +/* Maximum length of a fully-qualified domain name (server name). */ +#define FQDN_MAX 255 + +/* parse_tls_header() return codes (see tls.c for details) */ +#define TLS_PARSE_INCOMPLETE (-1) /* TLS record not fully received; buffer more and retry */ +#define TLS_PARSE_NO_SNI (-2) /* complete ClientHello, but no server name present */ +#define TLS_PARSE_INVALID (-5) /* not a TLS ClientHello handshake */ + +/* Returns the hostname length (>= 0) on success and writes it into *hostname + * (caller buffer of at least FQDN_MAX + 1 == 256 bytes), or one of the + * TLS_PARSE_* codes above. */ +int parse_tls_header(const char *data, size_t data_len, char *hostname); #endif //TLS_H From 6581201eccea33c35d33113d636a4fa730b92a3d Mon Sep 17 00:00:00 2001 From: Konrad Kollnig <5175206+kasnder@users.noreply.github.com> Date: Wed, 15 Jul 2026 09:34:00 +0200 Subject: [PATCH 2/4] Kill blocked research-mode sessions and skip buffering empty segments A blocked SNI verdict on an established 443 session only dropped the deciding segment; retransmissions then passed because the decision is once-per-session. Send an RST to the client instead, mirroring handle_tcp's handling of blocked new sessions. Also stop allocating the 16 KiB reassembly buffer on zero-length segments (the handshake ACK), deferring without buffering instead. Co-Authored-By: Claude Fable 5 --- app/src/main/jni/netguard/ip.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/app/src/main/jni/netguard/ip.c b/app/src/main/jni/netguard/ip.c index 848ae3bd5..3deb7336c 100644 --- a/app/src/main/jni/netguard/ip.c +++ b/app/src/main/jni/netguard/ip.c @@ -394,7 +394,12 @@ void handle_ip(const struct arguments *args, char hostname[FQDN_MAX + 1] = ""; int rc; - if (cur->tcp.tls_data == NULL) { + 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) { @@ -454,8 +459,16 @@ void handle_ip(const struct arguments *args, 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); + } } } From 0a109485c405d92163e00c7e8f0ccfd0411c68f0 Mon Sep 17 00:00:00 2001 From: Konrad Kollnig <5175206+kasnder@users.noreply.github.com> Date: Wed, 15 Jul 2026 09:38:42 +0200 Subject: [PATCH 3/4] Make ClientHello reassembly TCP-sequence-aware Continuation segments were appended to the reassembly buffer blindly, so a retransmitted segment (e.g. when the engine is slow to ACK) would duplicate bytes and corrupt the buffer, losing the SNI. Track the next expected sequence number: ignore pure retransmissions, append only the fresh tail of a partial overlap, and abandon reassembly on a forward gap rather than parse a buffer with a hole. Co-Authored-By: Claude Fable 5 --- app/src/main/jni/netguard/ip.c | 41 +++++++++++++++++++++------- app/src/main/jni/netguard/netguard.h | 1 + app/src/main/jni/netguard/tcp.c | 1 + 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/app/src/main/jni/netguard/ip.c b/app/src/main/jni/netguard/ip.c index 3deb7336c..8d3a4e05f 100644 --- a/app/src/main/jni/netguard/ip.c +++ b/app/src/main/jni/netguard/ip.c @@ -409,20 +409,41 @@ void handle_ip(const struct arguments *args, if (cur->tcp.tls_data != NULL) { memcpy(cur->tcp.tls_data, data, datalen); cur->tcp.tls_len = datalen; + cur->tcp.tls_seq = ntohl(tcphdr->seq) + 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 + // Continuation: append only in-order bytes and re-parse the + // accumulated record. The tun delivers the kernel's + // segments in order, so the only surprise is a + // retransmission, whose duplicate bytes would corrupt the + // buffer if appended again. A forward gap means we lost + // track: give up rather than parse a buffer with a hole. + uint32_t seq = ntohl(tcphdr->seq); + if ((int32_t) (seq - cur->tcp.tls_seq) > 0) { + // Gap: abandon reassembly; the decision below runs + // without SNI (defer_sni stays 0, buffer is freed). + } else { + uint32_t behind = cur->tcp.tls_seq - seq; + if (behind >= datalen) { + // Pure retransmission of already-buffered bytes. + defer_sni = 1; + } else { + // Skip the overlap, append only the new tail. + uint16_t skip = (uint16_t) behind; + uint16_t fresh = (uint16_t) (datalen - skip); + uint16_t space = (uint16_t) (TLS_SNI_MAX_BUFFER - cur->tcp.tls_len); + uint16_t copy = fresh < space ? fresh : space; + memcpy(cur->tcp.tls_data + cur->tcp.tls_len, data + skip, copy); + cur->tcp.tls_len += copy; + cur->tcp.tls_seq += 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) { diff --git a/app/src/main/jni/netguard/netguard.h b/app/src/main/jni/netguard/netguard.h index 7ec615db1..85ada1514 100644 --- a/app/src/main/jni/netguard/netguard.h +++ b/app/src/main/jni/netguard/netguard.h @@ -215,6 +215,7 @@ struct tcp_session { 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 + uint32_t tls_seq; // next expected TCP sequence number while reassembling }; struct ng_session { diff --git a/app/src/main/jni/netguard/tcp.c b/app/src/main/jni/netguard/tcp.c index 770ec224d..ad08ea71e 100644 --- a/app/src/main/jni/netguard/tcp.c +++ b/app/src/main/jni/netguard/tcp.c @@ -772,6 +772,7 @@ jboolean handle_tcp(const struct arguments *args, s->tcp.checkedHostname = 0; s->tcp.tls_data = NULL; s->tcp.tls_len = 0; + s->tcp.tls_seq = 0; s->next = NULL; if (datalen) { From 80420555107a18bb02f066d39139f57106f36630 Mon Sep 17 00:00:00 2001 From: Konrad Kollnig <5175206+kasnder@users.noreply.github.com> Date: Wed, 15 Jul 2026 09:40:57 +0200 Subject: [PATCH 4/4] Revert "Make ClientHello reassembly TCP-sequence-aware" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 0a109485c405d92163e00c7e8f0ccfd0411c68f0. Not worth the extra native code: the corruption it guards against (a retransmission during the few-ms reassembly window) is rare and already degrades safely — the TLS structure checks reject the buffer and the decision falls back to IP/DNS evidence. Blind best-effort append is the documented scope of research-mode reassembly. Co-Authored-By: Claude Fable 5 --- app/src/main/jni/netguard/ip.c | 41 +++++++--------------------- app/src/main/jni/netguard/netguard.h | 1 - app/src/main/jni/netguard/tcp.c | 1 - 3 files changed, 10 insertions(+), 33 deletions(-) diff --git a/app/src/main/jni/netguard/ip.c b/app/src/main/jni/netguard/ip.c index 8d3a4e05f..3deb7336c 100644 --- a/app/src/main/jni/netguard/ip.c +++ b/app/src/main/jni/netguard/ip.c @@ -409,41 +409,20 @@ void handle_ip(const struct arguments *args, if (cur->tcp.tls_data != NULL) { memcpy(cur->tcp.tls_data, data, datalen); cur->tcp.tls_len = datalen; - cur->tcp.tls_seq = ntohl(tcphdr->seq) + datalen; defer_sni = 1; } } } else { - // Continuation: append only in-order bytes and re-parse the - // accumulated record. The tun delivers the kernel's - // segments in order, so the only surprise is a - // retransmission, whose duplicate bytes would corrupt the - // buffer if appended again. A forward gap means we lost - // track: give up rather than parse a buffer with a hole. - uint32_t seq = ntohl(tcphdr->seq); - if ((int32_t) (seq - cur->tcp.tls_seq) > 0) { - // Gap: abandon reassembly; the decision below runs - // without SNI (defer_sni stays 0, buffer is freed). - } else { - uint32_t behind = cur->tcp.tls_seq - seq; - if (behind >= datalen) { - // Pure retransmission of already-buffered bytes. - defer_sni = 1; - } else { - // Skip the overlap, append only the new tail. - uint16_t skip = (uint16_t) behind; - uint16_t fresh = (uint16_t) (datalen - skip); - uint16_t space = (uint16_t) (TLS_SNI_MAX_BUFFER - cur->tcp.tls_len); - uint16_t copy = fresh < space ? fresh : space; - memcpy(cur->tcp.tls_data + cur->tcp.tls_len, data + skip, copy); - cur->tcp.tls_len += copy; - cur->tcp.tls_seq += 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 - } - } + // 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) { diff --git a/app/src/main/jni/netguard/netguard.h b/app/src/main/jni/netguard/netguard.h index 85ada1514..7ec615db1 100644 --- a/app/src/main/jni/netguard/netguard.h +++ b/app/src/main/jni/netguard/netguard.h @@ -215,7 +215,6 @@ struct tcp_session { 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 - uint32_t tls_seq; // next expected TCP sequence number while reassembling }; struct ng_session { diff --git a/app/src/main/jni/netguard/tcp.c b/app/src/main/jni/netguard/tcp.c index ad08ea71e..770ec224d 100644 --- a/app/src/main/jni/netguard/tcp.c +++ b/app/src/main/jni/netguard/tcp.c @@ -772,7 +772,6 @@ jboolean handle_tcp(const struct arguments *args, s->tcp.checkedHostname = 0; s->tcp.tls_data = NULL; s->tcp.tls_len = 0; - s->tcp.tls_seq = 0; s->next = NULL; if (datalen) {