From ef77c9b71fec96c56002498a8d3ed0b6a0ce91d1 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 22:03:19 +0000 Subject: [PATCH 1/2] Skip SNI research mode for tunnelled flows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SNI research mode reassembles a ClientHello on the ng_session that handle_tcp creates for a 443 flow. A flow routed through the WireGuard tunnel never gets one: handle_ip writes the packet to the WG bridge and returns before handle_tcp runs. So the reassembly guard always saw cur == NULL and no SNI was ever collected, while every later segment re-ran is_address_allowed() and a UID lookup, because the once-per-session shortcut lives inside a cur != NULL branch those flows never reach. Resolve the routing verdict before the 443 decision sites and, when the flow tunnels, take the ordinary non-research path for it: decide once on the SYN by IP, then allowed = 1 — exactly how WireGuard behaves without research mode. Nothing changes when WireGuard is off, or for flows that per-app routing keeps direct, which still collect SNI as before. The tunnel_uid resolution is factored into resolve_tunnel_uid() and shared with the routing fork, which reuses the early answer rather than resolving and re-storing it a second time. Also stop the blocking-mode summary from implying research mode still works: it now says SNI extraction does not apply to remotely routed traffic whenever SNI and a configured remote VPN are both on. Refs #735 --- .../faircode/netguard/ActivitySettings.java | 33 +++- app/src/main/jni/netguard/ip.c | 186 ++++++++++++------ app/src/main/res/values/strings.xml | 1 + 3 files changed, 156 insertions(+), 64 deletions(-) diff --git a/app/src/main/java/eu/faircode/netguard/ActivitySettings.java b/app/src/main/java/eu/faircode/netguard/ActivitySettings.java index b680e6c3..7f3e8ecd 100644 --- a/app/src/main/java/eu/faircode/netguard/ActivitySettings.java +++ b/app/src/main/java/eu/faircode/netguard/ActivitySettings.java @@ -726,10 +726,12 @@ else if ("log_logcat".equals(name)) { boolean research = prefs.getBoolean(name, false); if (prefs.getBoolean("sni_enabled", false) != research) prefs.edit().putBoolean("sni_enabled", research).apply(); + refreshBlockingModeSummary(); ServiceSinkhole.reload("changed " + name, this, false); } else if ("sni_enabled".equals(name)) { + refreshBlockingModeSummary(); ServiceSinkhole.reload("changed " + name, this, false); } @@ -891,6 +893,7 @@ else if ("socks5_addr".equals(name)) { } else if ("wg_enabled".equals(name)) { updateWireGuardStatus(); + refreshBlockingModeSummary(); ServiceSinkhole.reload("changed " + name, this, false); } else if ("wg_keepalive_when_screen_off".equals(name)) { @@ -925,6 +928,7 @@ else if ("socks5_addr".equals(name)) { new WgProfileManager(this).updateActiveProfileConfig(wg_config); configureWireGuardProfiles(getPreferenceScreen(), prefs); updateWireGuardStatus(); + refreshBlockingModeSummary(); ServiceSinkhole.reload("changed " + name, this, false); } else if ("pcap_record_size".equals(name) || "pcap_file_size".equals(name)) { @@ -1037,12 +1041,35 @@ private void updateWireGuardStatus() { } private void updateBlockingModeSummary(Preference pref, String mode) { + int summaryRes; if (BlockingMode.MODE_MINIMAL.equals(mode)) - pref.setSummary(R.string.summary_blocking_mode_minimal); + summaryRes = R.string.summary_blocking_mode_minimal; else if (BlockingMode.MODE_STRICT.equals(mode)) - pref.setSummary(R.string.summary_blocking_mode_strict); + summaryRes = R.string.summary_blocking_mode_strict; else - pref.setSummary(R.string.summary_blocking_mode_standard); + summaryRes = R.string.summary_blocking_mode_standard; + + // SNI research mode reassembles the ClientHello on a per-flow session + // that only exists for directly-routed traffic (see handle_ip in + // ip.c): a flow the remote VPN tunnels never gets one, so research + // mode silently collects nothing for it. Say so here rather than + // letting the blocking-mode summary imply it still works. + SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(this); + if (prefs.getBoolean("sni_enabled", false) && + prefs.getBoolean("wg_enabled", false) && + !TextUtils.isEmpty(prefs.getString("wg_config", null))) + pref.setSummary(getString(summaryRes) + " " + getString(R.string.summary_sni_wg_note)); + else + pref.setSummary(summaryRes); + } + + // Re-derives the blocking-mode summary after a preference other than + // blocking_mode itself changed the SNI-under-WireGuard note's condition + // (sni_enabled, wg_enabled, wg_config). + private void refreshBlockingModeSummary() { + Preference pref = getPreferenceScreen().findPreference("blocking_mode"); + if (pref != null) + updateBlockingModeSummary(pref, BlockingMode.getMode(this)); } private void setTrackerProtectionForAll(boolean enabled) { diff --git a/app/src/main/jni/netguard/ip.c b/app/src/main/jni/netguard/ip.c index f361f991..721ba744 100644 --- a/app/src/main/jni/netguard/ip.c +++ b/app/src/main/jni/netguard/ip.c @@ -123,6 +123,70 @@ static jint get_route_uid(const struct arguments *args, int version, int protoco return get_uid_q(args, version, protocol, source, sport, dest, dport); } +// Whether a flow's owning app should be routed through the tunnel. Shared by +// handle_ip's routing fork and, for SNI research mode, by the earlier check +// that decides whether a 443 flow will ever get an ng_session to reassemble +// a ClientHello into (a tunnelled flow never does: the WireGuard write below +// returns before handle_tcp creates one). uid is the already-known UID for +// this packet, or -1 when the caller has not resolved one yet. +static int resolve_tunnel_uid(const struct arguments *args, int version, uint8_t protocol, + const void *saddr, uint16_t sport, + const void *daddr, uint16_t dport, + const char *source, const char *dest, + const uint8_t *pkt, const uint8_t *payload, + jint uid) { + int tunnel_uid; + if (route_uid_relevant()) { + // A flow keeps the verdict its first packet was given. A fresh UID + // is authoritative even when a previous flow happened to reuse the + // same 5-tuple, so do not consult the flow cache in that case. + jint route_uid = uid; + if (route_uid >= 0) { + tunnel_uid = is_tunnel_uid(route_uid); + route_flow_store(version, protocol, saddr, sport, daddr, dport, + tunnel_uid); + } else if (route_flow_lookup(version, protocol, saddr, sport, daddr, dport, + &tunnel_uid)) { + // Established tunnelled flows never create an ng_session — the + // WireGuard write below returns first — so the cache preserves + // their first-packet answer without a per-packet UID lookup. + } else { + // A cache expiry or collision is rare, but falling back to the + // selected-mode default would divert an already-established + // tunnelled flow direct and can make TCP reset. Recover the UID + // from the native session table first, then from the + // authoritative Android/procfs lookup. + route_uid = get_session_uid(args, version, protocol, pkt, payload); + if (route_uid < 0) + route_uid = get_route_uid(args, version, protocol, + saddr, sport, daddr, dport, + source, dest); + + if (route_uid >= 0) { + tunnel_uid = is_tunnel_uid(route_uid); + route_flow_store(version, protocol, saddr, sport, daddr, dport, + tunnel_uid); + } else { + // Unknown ownership is privacy-sensitive: keep the packet + // in the remote tunnel rather than fail-open to direct + // routing in selected mode. Cache that explicit fail-closed + // verdict so the rest of this flow does not repeat a Binder + // or procfs lookup on every packet. A later flow with the + // same tuple still wins because a freshly resolved UID is + // handled before the cache above. + tunnel_uid = 1; + route_flow_store(version, protocol, saddr, sport, daddr, dport, + tunnel_uid); + log_android(ANDROID_LOG_WARN, + "Route UID unavailable for v%d p%d %s/%u > %s/%u; tunnelling", + version, protocol, source, sport, dest, dport); + } + } + } else + tunnel_uid = route_default_is_tunnel(); + return tunnel_uid; +} + uint16_t get_mtu() { return 10000; } @@ -396,23 +460,65 @@ void handle_ip(const struct arguments *args, } } - // Get uid + // Get uid. SNI research mode deliberately lets a 443 SYN through without + // one so the ClientHello can be reassembled first. That is fine for the + // block decision, but the routing fork needs the UID now: with no UID and + // no session, the SYN falls to the global default and every later packet + // of that flow inherits the answer from it. jint uid = -1; + int sni_candidate = (is_play && protocol == IPPROTO_TCP && dport == 443); if (protocol == IPPROTO_ICMP || protocol == IPPROTO_ICMPV6 || (protocol == IPPROTO_UDP && !has_udp_session(args, pkt, payload)) || - // SNI research mode lets a 443 SYN through without a UID so the - // ClientHello can be reassembled first. That is fine for the block - // decision, but the routing fork needs the UID now: with no UID and no - // session, the SYN falls to the global default and every later packet - // of that flow inherits the answer from it. (protocol == IPPROTO_TCP && syn && - (dport != 443 || !is_play || route_uid_relevant()))) { + (!sni_candidate || route_uid_relevant()))) { if (args->ctx->sdk <= 28) // Android 9 Pie uid = get_uid(version, protocol, saddr, sport, daddr, dport); else uid = get_uid_q(args, version, protocol, source, sport, dest, dport); } + // SNI research mode reassembles a ClientHello on the ng_session that + // handle_tcp creates for a 443 flow — but the WireGuard hijack below hands + // a tunnelled flow's packets to the WG bridge and returns before + // handle_tcp ever runs, so such a flow never gets a session to reassemble + // on. Left alone that means the reassembly guard always sees cur == NULL, + // so no SNI is ever collected, and every later segment re-runs the full + // is_address_allowed() upcall and a UID lookup forever, because the + // once-per-session shortcut lives inside a cur != NULL branch a tunnelled + // flow never reaches. Rather than build per-flow reassembly state for a + // tunnelled flow, resolve the routing verdict up front and, when it + // tunnels, drop sni_active so the packet takes the ordinary path below: + // decide once on the SYN by IP, then allowed = 1, exactly as WireGuard + // behaves without research mode. + int sni_active = sni_candidate; + int sni_tunnel_uid = 0; + int sni_tunnel_uid_known = 0; + int wg_is_required = atomic_load_explicit(&wg_required, memory_order_acquire); + if (wg_is_required && sni_candidate) { + // is_dns is 0 here by construction: this is only reached for dport + // 443. uid is unresolved for every packet but the SYN of a per-app + // routed flow, which is what the flow cache and the session-table + // fallback inside resolve_tunnel_uid are for. + sni_tunnel_uid = resolve_tunnel_uid(args, version, protocol, + saddr, sport, daddr, dport, + source, dest, pkt, payload, uid); + sni_tunnel_uid_known = 1; + if (route_wants_tunnel(is_local_dest(version, daddr), 0, + sni_tunnel_uid, route_dns_direct())) + sni_active = 0; + } + + // The ordinary path decides on the SYN, so a flow that just lost research + // mode still needs the UID the exemption above skipped — without it the + // decision would run unattributed, which WireGuard without research mode + // never does. + if (sni_candidate && !sni_active && syn && uid < 0) { + if (args->ctx->sdk <= 28) // Android 9 Pie + uid = get_uid(version, protocol, saddr, sport, daddr, dport); + else + uid = get_uid_q(args, version, protocol, source, sport, dest, dport); + } + log_android(ANDROID_LOG_DEBUG, "Packet v%d %s/%u > %s/%u proto %d flags %s uid %d", version, source, sport, dest, dport, protocol, flags, uid); @@ -422,9 +528,9 @@ void handle_ip(const struct arguments *args, struct allowed *redirect = NULL; if (protocol == IPPROTO_UDP && has_udp_session(args, pkt, payload)) allowed = 1; // could be a lingering/blocked session - else if (protocol == IPPROTO_TCP && ((!syn && (dport != 443 || !is_play)) // assume existing session + else if (protocol == IPPROTO_TCP && ((!syn && (dport != 443 || !sni_active)) // assume existing session || (uid == 0 && dport == 53) // assume existing session - || (dport == 443 && syn && is_play))) // let SYN pass by until SNI can be extracted + || (dport == 443 && syn && sni_active))) // let SYN pass by until SNI can be extracted allowed = 1; else { struct ng_session *cur = NULL; @@ -435,7 +541,7 @@ void handle_ip(const struct arguments *args, 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) { + if (protocol == IPPROTO_TCP && dport == 443 && !syn && sni_active) { // Get TCP headers const uint8_t version = (*pkt) >> 4; const struct iphdr *ip4 = (struct iphdr *) pkt; @@ -559,7 +665,6 @@ void handle_ip(const struct arguments *args, // user's physical network. int is_dns = (dport == 53 && (protocol == IPPROTO_UDP || protocol == IPPROTO_TCP)); - int wg_is_required = atomic_load_explicit(&wg_required, memory_order_acquire); // Which app this packet belongs to — but only when that can change the // answer. With no per-app override configured every UID routes the same @@ -567,56 +672,15 @@ void handle_ip(const struct arguments *args, // cost a lock and, for the established flows that arrive with uid == -1 // (existing UDP sessions, non-SYN TCP, i.e. most packets), a walk of the // whole session table, which grows with load. That is pure waste for - // everyone who has not opted in. - int tunnel_uid; - if (route_uid_relevant()) { - // A flow keeps the verdict its first packet was given. A fresh UID - // is authoritative even when a previous flow happened to reuse the - // same 5-tuple, so do not consult the flow cache in that case. - jint route_uid = uid; - if (route_uid >= 0) { - tunnel_uid = is_tunnel_uid(route_uid); - route_flow_store(version, protocol, saddr, sport, daddr, dport, - tunnel_uid); - } else if (route_flow_lookup(version, protocol, saddr, sport, daddr, dport, - &tunnel_uid)) { - // Established tunnelled flows never create an ng_session — the - // WireGuard write below returns first — so the cache preserves - // their first-packet answer without a per-packet UID lookup. - } else { - // A cache expiry or collision is rare, but falling back to the - // selected-mode default would divert an already-established - // tunnelled flow direct and can make TCP reset. Recover the UID - // from the native session table first, then from the - // authoritative Android/procfs lookup. - route_uid = get_session_uid(args, version, protocol, pkt, payload); - if (route_uid < 0) - route_uid = get_route_uid(args, version, protocol, - saddr, sport, daddr, dport, - source, dest); - - if (route_uid >= 0) { - tunnel_uid = is_tunnel_uid(route_uid); - route_flow_store(version, protocol, saddr, sport, daddr, dport, - tunnel_uid); - } else { - // Unknown ownership is privacy-sensitive: keep the packet - // in the remote tunnel rather than fail-open to direct - // routing in selected mode. Cache that explicit fail-closed - // verdict so the rest of this flow does not repeat a Binder - // or procfs lookup on every packet. A later flow with the - // same tuple still wins because a freshly resolved UID is - // handled before the cache above. - tunnel_uid = 1; - route_flow_store(version, protocol, saddr, sport, daddr, dport, - tunnel_uid); - log_android(ANDROID_LOG_WARN, - "Route UID unavailable for v%d p%d %s/%u > %s/%u; tunnelling", - version, protocol, source, sport, dest, dport); - } - } - } else - tunnel_uid = route_default_is_tunnel(); + // everyone who has not opted in. The SNI research-mode check above + // already resolved this (and stored it in the flow cache) for a + // candidate 443 flow while WireGuard is required; reuse that answer + // instead of resolving and re-storing it a second time. + int tunnel_uid = sni_tunnel_uid_known + ? sni_tunnel_uid + : resolve_tunnel_uid(args, version, protocol, + saddr, sport, daddr, dport, + source, dest, pkt, payload, uid); int wg_dest = route_wants_tunnel(is_local_dest(version, daddr), is_dns, tunnel_uid, route_dns_direct()); diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 87e15a8b..ab1f40eb 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -626,6 +626,7 @@ Sincerely,\n\n]]> Minimal: Only blocks trackers from the DuckDuckGo list that are safe to block. Browsers stay routed through the VPN, but tracker protection is off by default. Known incompatible apps are excluded. Limited app breakage. Standard: Blocks all known trackers from multiple lists (X-Ray, Disconnect, DuckDuckGo). Essential services are allowed. Some apps will break and may need to be manually excluded (see Troubleshooting in the app menu). Strict: Blocks all known trackers including essential services and ambiguous shared-IP domains. Many apps will break and need to be manually excluded (see Troubleshooting in the app menu). + Note: SNI extraction does not apply to traffic routed through the remote VPN. Minimal Standard Strict From 004c83bf75ee9a3df264d62c4d98e3769eeca303 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 22:25:21 +0000 Subject: [PATCH 2/2] Address PR #744 review: fix stale docs, note placement, extra lookup - AGENTS.md's privacy-preservation principle claimed SNI parsing runs the same whether WireGuard is on or off, which this PR makes false. Update it to describe the actual behaviour (research mode collects nothing for tunnelled flows, surfaced in the Research summary). - Move the SNI/WireGuard caveat from the blocking_mode summary to the Research (log_logcat) preference summary, since that is the control that actually toggles sni_enabled; blocking_mode's summary never mentioned SNI at all. - resolve_tunnel_uid() now takes an optional out_uid parameter so the SNI-attribution fallback in handle_ip can reuse a UID the routing resolution already looked up, instead of paying for a third lookup (flow-cache miss -> session table -> procfs/Binder) on the same packet. --- AGENTS.md | 10 ++-- .../faircode/netguard/ActivitySettings.java | 46 +++++++++++-------- app/src/main/jni/netguard/ip.c | 29 +++++++++--- 3 files changed, 53 insertions(+), 32 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 8165f5af..1273ea4c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -50,11 +50,11 @@ The non-negotiables that decide most changes: 2. **Privacy-preserving by construction.** No SSL/TLS interception, ever. Detection works off DNS metadata; SNI/TLS parsing is confined to an opt-in research mode because acting on it would leak the user's IP to the tracker first. This SNI - parsing happens in the native `handle_ip()` block/allow decision (`ip.c`) - before traffic is dispatched to any egress path, so it runs the same way - whether remote routing (WireGuard) is on or off — when it is on, the - subsequent connection to the tracker goes out through the tunnel, so only - the VPN provider's IP leaks, not the device's real IP. + parsing happens in the native `handle_ip()` block/allow decision (`ip.c`), but + only for directly-routed flows: a flow the remote VPN (WireGuard) tunnels + never gets the per-flow session state the reassembly needs, so research mode + collects nothing for it, and the app tells the user so in the Research + preference summary when WireGuard is on. 3. **Battery is a first-class constraint.** Anything periodic must be gated off idle/screen-off. Do not make DoH a stronger default until its screen-off cost is profiled and fixed. Battery is also frequently mis-attributed to the diff --git a/app/src/main/java/eu/faircode/netguard/ActivitySettings.java b/app/src/main/java/eu/faircode/netguard/ActivitySettings.java index 7f3e8ecd..03d723d1 100644 --- a/app/src/main/java/eu/faircode/netguard/ActivitySettings.java +++ b/app/src/main/java/eu/faircode/netguard/ActivitySettings.java @@ -473,6 +473,10 @@ public void onSure() { updateBlockingModeSummary(pref_blocking_mode, currentMode); } + Preference pref_log_logcat = screen.findPreference("log_logcat"); + if (pref_log_logcat != null) + updateResearchModeSummary(pref_log_logcat); + if (Util.isPlayStoreInstall(this) && cat_advanced != null) { Log.i(TAG, "Play store install"); if (pref_forwarding != null) @@ -726,12 +730,12 @@ else if ("log_logcat".equals(name)) { boolean research = prefs.getBoolean(name, false); if (prefs.getBoolean("sni_enabled", false) != research) prefs.edit().putBoolean("sni_enabled", research).apply(); - refreshBlockingModeSummary(); + refreshResearchModeSummary(); ServiceSinkhole.reload("changed " + name, this, false); } else if ("sni_enabled".equals(name)) { - refreshBlockingModeSummary(); + refreshResearchModeSummary(); ServiceSinkhole.reload("changed " + name, this, false); } @@ -893,7 +897,7 @@ else if ("socks5_addr".equals(name)) { } else if ("wg_enabled".equals(name)) { updateWireGuardStatus(); - refreshBlockingModeSummary(); + refreshResearchModeSummary(); ServiceSinkhole.reload("changed " + name, this, false); } else if ("wg_keepalive_when_screen_off".equals(name)) { @@ -928,7 +932,7 @@ else if ("socks5_addr".equals(name)) { new WgProfileManager(this).updateActiveProfileConfig(wg_config); configureWireGuardProfiles(getPreferenceScreen(), prefs); updateWireGuardStatus(); - refreshBlockingModeSummary(); + refreshResearchModeSummary(); ServiceSinkhole.reload("changed " + name, this, false); } else if ("pcap_record_size".equals(name) || "pcap_file_size".equals(name)) { @@ -1041,35 +1045,37 @@ private void updateWireGuardStatus() { } private void updateBlockingModeSummary(Preference pref, String mode) { - int summaryRes; if (BlockingMode.MODE_MINIMAL.equals(mode)) - summaryRes = R.string.summary_blocking_mode_minimal; + pref.setSummary(R.string.summary_blocking_mode_minimal); else if (BlockingMode.MODE_STRICT.equals(mode)) - summaryRes = R.string.summary_blocking_mode_strict; + pref.setSummary(R.string.summary_blocking_mode_strict); else - summaryRes = R.string.summary_blocking_mode_standard; + pref.setSummary(R.string.summary_blocking_mode_standard); + } - // SNI research mode reassembles the ClientHello on a per-flow session - // that only exists for directly-routed traffic (see handle_ip in - // ip.c): a flow the remote VPN tunnels never gets one, so research - // mode silently collects nothing for it. Say so here rather than - // letting the blocking-mode summary imply it still works. + // SNI research mode reassembles the ClientHello on a per-flow session that + // only exists for directly-routed traffic (see handle_ip in ip.c): a flow + // the remote VPN tunnels never gets one, so research mode silently + // collects nothing for it. Say so on the Research preference itself, + // where sni_enabled is actually toggled, rather than on blocking_mode. + private void updateResearchModeSummary(Preference pref) { SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(this); + String base = getString(R.string.summary_log_logcat); if (prefs.getBoolean("sni_enabled", false) && prefs.getBoolean("wg_enabled", false) && !TextUtils.isEmpty(prefs.getString("wg_config", null))) - pref.setSummary(getString(summaryRes) + " " + getString(R.string.summary_sni_wg_note)); + pref.setSummary(base + " " + getString(R.string.summary_sni_wg_note)); else - pref.setSummary(summaryRes); + pref.setSummary(base); } - // Re-derives the blocking-mode summary after a preference other than - // blocking_mode itself changed the SNI-under-WireGuard note's condition + // Re-derives the Research preference summary after a preference other than + // log_logcat itself changed the SNI-under-WireGuard note's condition // (sni_enabled, wg_enabled, wg_config). - private void refreshBlockingModeSummary() { - Preference pref = getPreferenceScreen().findPreference("blocking_mode"); + private void refreshResearchModeSummary() { + Preference pref = getPreferenceScreen().findPreference("log_logcat"); if (pref != null) - updateBlockingModeSummary(pref, BlockingMode.getMode(this)); + updateResearchModeSummary(pref); } private void setTrackerProtectionForAll(boolean enabled) { diff --git a/app/src/main/jni/netguard/ip.c b/app/src/main/jni/netguard/ip.c index 721ba744..1b7a4d7a 100644 --- a/app/src/main/jni/netguard/ip.c +++ b/app/src/main/jni/netguard/ip.c @@ -128,13 +128,17 @@ static jint get_route_uid(const struct arguments *args, int version, int protoco // that decides whether a 443 flow will ever get an ng_session to reassemble // a ClientHello into (a tunnelled flow never does: the WireGuard write below // returns before handle_tcp creates one). uid is the already-known UID for -// this packet, or -1 when the caller has not resolved one yet. +// this packet, or -1 when the caller has not resolved one yet. out_uid, when +// not NULL, is filled with any UID this call resolves from the session table +// or the authoritative lookup, so a caller that still needs a UID afterwards +// (SNI research mode attributing a flow that just lost its exemption) can +// reuse it instead of paying for the same lookup again. static int resolve_tunnel_uid(const struct arguments *args, int version, uint8_t protocol, const void *saddr, uint16_t sport, const void *daddr, uint16_t dport, const char *source, const char *dest, const uint8_t *pkt, const uint8_t *payload, - jint uid) { + jint uid, jint *out_uid) { int tunnel_uid; if (route_uid_relevant()) { // A flow keeps the verdict its first packet was given. A fresh UID @@ -166,6 +170,8 @@ static int resolve_tunnel_uid(const struct arguments *args, int version, uint8_t tunnel_uid = is_tunnel_uid(route_uid); route_flow_store(version, protocol, saddr, sport, daddr, dport, tunnel_uid); + if (out_uid != NULL) + *out_uid = route_uid; } else { // Unknown ownership is privacy-sensitive: keep the packet // in the remote tunnel rather than fail-open to direct @@ -493,15 +499,20 @@ void handle_ip(const struct arguments *args, int sni_active = sni_candidate; int sni_tunnel_uid = 0; int sni_tunnel_uid_known = 0; + jint sni_resolved_uid = -1; int wg_is_required = atomic_load_explicit(&wg_required, memory_order_acquire); if (wg_is_required && sni_candidate) { // is_dns is 0 here by construction: this is only reached for dport // 443. uid is unresolved for every packet but the SYN of a per-app // routed flow, which is what the flow cache and the session-table - // fallback inside resolve_tunnel_uid are for. + // fallback inside resolve_tunnel_uid are for. sni_resolved_uid, when + // filled in, lets the UID-attribution fallback below reuse whatever + // this call already paid a session-table/procfs lookup for, instead + // of resolving it a second time. sni_tunnel_uid = resolve_tunnel_uid(args, version, protocol, saddr, sport, daddr, dport, - source, dest, pkt, payload, uid); + source, dest, pkt, payload, uid, + &sni_resolved_uid); sni_tunnel_uid_known = 1; if (route_wants_tunnel(is_local_dest(version, daddr), 0, sni_tunnel_uid, route_dns_direct())) @@ -511,9 +522,13 @@ void handle_ip(const struct arguments *args, // The ordinary path decides on the SYN, so a flow that just lost research // mode still needs the UID the exemption above skipped — without it the // decision would run unattributed, which WireGuard without research mode - // never does. + // never does. Prefer whatever resolve_tunnel_uid already resolved above; + // only fall back to a fresh lookup when it did not (route_uid_relevant() + // was false, so no UID needed resolving for the routing verdict). if (sni_candidate && !sni_active && syn && uid < 0) { - if (args->ctx->sdk <= 28) // Android 9 Pie + if (sni_resolved_uid >= 0) + uid = sni_resolved_uid; + else if (args->ctx->sdk <= 28) // Android 9 Pie uid = get_uid(version, protocol, saddr, sport, daddr, dport); else uid = get_uid_q(args, version, protocol, source, sport, dest, dport); @@ -680,7 +695,7 @@ void handle_ip(const struct arguments *args, ? sni_tunnel_uid : resolve_tunnel_uid(args, version, protocol, saddr, sport, daddr, dport, - source, dest, pkt, payload, uid); + source, dest, pkt, payload, uid, NULL); int wg_dest = route_wants_tunnel(is_local_dest(version, daddr), is_dns, tunnel_uid, route_dns_direct());