From 304006bbb6ac02d47c8a280893e147af24e23cbf Mon Sep 17 00:00:00 2001 From: Konrad Kollnig <5175206+kasnder@users.noreply.github.com> Date: Fri, 11 Sep 2026 18:15:32 +0200 Subject: [PATCH 1/4] Verify and recover WireGuard data paths after network handovers --- .../netguard/PhysicalNetworkState.java | 341 ++++++++++++++++++ .../eu/faircode/netguard/ServiceSinkhole.java | 160 ++++---- .../wg/WgConnectivityMonitor.kt | 22 +- .../net/kollnig/missioncontrol/wg/WgEgress.kt | 334 +++++++++++------ .../missioncontrol/wg/WgHandoverVerifier.kt | 208 +++++++++++ .../missioncontrol/wgbridge/Tunnel.java | 16 +- .../missioncontrol/wgbridge/TunnelStats.java | 22 ++ .../netguard/PhysicalNetworkStateTest.java | 204 +++++++++++ .../wg/WgConnectivityCheckerTest.kt | 23 +- .../wg/WgConnectivityMonitorTest.kt | 21 ++ .../wg/WgHandoverVerifierTest.kt | 190 ++++++++++ wgbridge-rs/README.md | 19 + wgbridge-rs/src/jni_bindings.rs | 26 ++ wgbridge-rs/src/lib.rs | 1 + wgbridge-rs/src/probe.rs | 329 +++++++++++++++++ wgbridge-rs/src/transport/ip_recv.rs | 46 ++- wgbridge-rs/src/transport/ip_send.rs | 45 +++ wgbridge-rs/src/tunnel.rs | 45 ++- 18 files changed, 1855 insertions(+), 197 deletions(-) create mode 100644 app/src/main/java/eu/faircode/netguard/PhysicalNetworkState.java create mode 100644 app/src/main/java/net/kollnig/missioncontrol/wg/WgHandoverVerifier.kt create mode 100644 app/src/test/java/eu/faircode/netguard/PhysicalNetworkStateTest.java create mode 100644 app/src/test/java/net/kollnig/missioncontrol/wg/WgHandoverVerifierTest.kt create mode 100644 wgbridge-rs/src/probe.rs diff --git a/app/src/main/java/eu/faircode/netguard/PhysicalNetworkState.java b/app/src/main/java/eu/faircode/netguard/PhysicalNetworkState.java new file mode 100644 index 000000000..17fec3037 --- /dev/null +++ b/app/src/main/java/eu/faircode/netguard/PhysicalNetworkState.java @@ -0,0 +1,341 @@ +package eu.faircode.netguard; + +import android.net.LinkAddress; +import android.net.LinkProperties; +import android.net.Network; +import android.net.NetworkCapabilities; +import android.os.Build; +import android.os.Parcel; + +import java.net.InetAddress; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; + +/** + * Callback-owned state for physical networks. Connectivity callbacks can be + * reordered and do not make synchronous ConnectivityManager lookups safe, so + * the callback snapshots are the source of truth until the next callback. + */ +final class PhysicalNetworkState { + static final class Change { + private final String reason; + private final boolean privateDnsChanged; + + private Change(String reason, boolean privateDnsChanged) { + this.reason = reason; + this.privateDnsChanged = privateDnsChanged; + } + + static Change none() { + return new Change(null, false); + } + + static Change of(String reason, boolean privateDnsChanged) { + return new Change(reason, privateDnsChanged); + } + + String getReason() { + return reason; + } + + boolean isPrivateDnsChanged() { + return privateDnsChanged; + } + } + + private static final class Entry { + NetworkCapabilities capabilities; + LinkProperties linkProperties; + Fingerprint fingerprint; + String privateDns; + boolean privateDnsActive; + + Entry(NetworkCapabilities capabilities, LinkProperties linkProperties) { + updateCapabilities(capabilities); + updateLinkProperties(linkProperties); + } + + void updateCapabilities(NetworkCapabilities supplied) { + capabilities = supplied == null ? null : new NetworkCapabilities(supplied); + fingerprint = Fingerprint.from(capabilities, linkProperties); + } + + boolean updateLinkProperties(LinkProperties supplied) { + String oldPrivateDns = privateDns; + boolean oldPrivateDnsActive = privateDnsActive; + // Parcelable copying works on older Android releases too; public + // LinkProperties constructors and setters require API 29. + String newPrivateDns = null; + boolean newPrivateDnsActive = false; + if (supplied != null && Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) { + newPrivateDns = supplied.getPrivateDnsServerName(); + newPrivateDnsActive = supplied.isPrivateDnsActive(); + } + linkProperties = copyLinkProperties(supplied); + privateDns = newPrivateDns; + privateDnsActive = newPrivateDnsActive; + fingerprint = Fingerprint.from(capabilities, linkProperties); + return !Objects.equals(oldPrivateDns, privateDns) || + oldPrivateDnsActive != privateDnsActive; + } + + private static LinkProperties copyLinkProperties(LinkProperties supplied) { + if (supplied == null) + return null; + Parcel parcel = Parcel.obtain(); + try { + supplied.writeToParcel(parcel, 0); + parcel.setDataPosition(0); + return LinkProperties.CREATOR.createFromParcel(parcel); + } finally { + parcel.recycle(); + } + } + } + + private static final class Fingerprint { + // hasCapability safely returns false for capabilities an older OS + // does not know; these integer constants do not invoke newer APIs. + @android.annotation.SuppressLint("InlinedApi") + private static final int[] CAPABILITIES = new int[]{ + NetworkCapabilities.NET_CAPABILITY_INTERNET, + NetworkCapabilities.NET_CAPABILITY_VALIDATED, + NetworkCapabilities.NET_CAPABILITY_NOT_VPN, + NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED, + NetworkCapabilities.NET_CAPABILITY_NOT_METERED, + NetworkCapabilities.NET_CAPABILITY_TEMPORARILY_NOT_METERED + }; + + private final boolean[] capabilities; + private final int[] transports; + private final List linkAddresses; + private final List routes; + private final List dnsServers; + private final String domains; + + private Fingerprint(boolean[] capabilities, int[] transports, + List linkAddresses, List routes, + List dnsServers, String domains) { + this.capabilities = capabilities; + this.transports = transports; + this.linkAddresses = linkAddresses; + this.routes = routes; + this.dnsServers = dnsServers; + this.domains = domains; + } + + static Fingerprint from(NetworkCapabilities caps, LinkProperties props) { + boolean[] capabilities = new boolean[CAPABILITIES.length]; + if (caps != null) + for (int i = 0; i < CAPABILITIES.length; i++) + capabilities[i] = caps.hasCapability(CAPABILITIES[i]); + + List transportTypes = new ArrayList<>(); + if (caps != null) { + // hasTransport is available on the minimum supported API. Do + // not use getTransportTypes(), which is newer than API 24. + for (int transport = 0; transport < 32; transport++) + if (caps.hasTransport(transport)) + transportTypes.add(transport); + } + int[] transports = new int[transportTypes.size()]; + for (int i = 0; i < transportTypes.size(); i++) + transports[i] = transportTypes.get(i); + Arrays.sort(transports); + + List linkAddresses = new ArrayList<>(); + List routes = new ArrayList<>(); + List dnsServers = new ArrayList<>(); + String domains = null; + if (props != null) { + for (LinkAddress address : props.getLinkAddresses()) + linkAddresses.add(address.toString()); + for (Object route : props.getRoutes()) + routes.add(String.valueOf(route)); + for (InetAddress dns : props.getDnsServers()) + dnsServers.add(dns.getHostAddress()); + domains = props.getDomains(); + } + Collections.sort(linkAddresses); + Collections.sort(routes); + Collections.sort(dnsServers); + return new Fingerprint(capabilities, transports, linkAddresses, routes, + dnsServers, domains); + } + + @Override + public boolean equals(Object other) { + if (!(other instanceof Fingerprint)) + return false; + Fingerprint that = (Fingerprint) other; + return Arrays.equals(capabilities, that.capabilities) && + Arrays.equals(transports, that.transports) && + Objects.equals(linkAddresses, that.linkAddresses) && + Objects.equals(routes, that.routes) && + Objects.equals(dnsServers, that.dnsServers) && + Objects.equals(domains, that.domains); + } + + @Override + public int hashCode() { + int result = Arrays.hashCode(capabilities); + result = 31 * result + Arrays.hashCode(transports); + result = 31 * result + linkAddresses.hashCode(); + result = 31 * result + routes.hashCode(); + result = 31 * result + dnsServers.hashCode(); + result = 31 * result + Objects.hashCode(domains); + return result; + } + } + + private final Map entries = new HashMap<>(); + private Network defaultNetwork; + private boolean baselineEstablished; + private int[] defaultVpnTransports; + + synchronized Change onPhysicalAvailable(Network network) { + if (network == null || entries.containsKey(network)) + return Change.none(); + entries.put(network, new Entry(null, null)); + return Change.of(NetworkReloadPolicy.REASON_NETWORK_AVAILABLE, false); + } + + synchronized Change onPhysicalCapabilitiesChanged(Network network, NetworkCapabilities supplied) { + if (network == null || supplied == null || + !supplied.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN)) + return Change.none(); + Entry entry = entries.get(network); + if (entry == null) { + entry = new Entry(null, null); + entries.put(network, entry); + } + boolean changed = !entry.fingerprint.equals(Fingerprint.from(supplied, entry.linkProperties)); + entry.updateCapabilities(supplied); + if (changed) + return Change.of(NetworkReloadPolicy.REASON_NETWORK_CHANGED, false); + return Change.none(); + } + + synchronized Change onPhysicalLinkPropertiesChanged(Network network, LinkProperties supplied) { + if (network == null) + return Change.none(); + Entry entry = entries.get(network); + if (entry == null) { + entry = new Entry(null, null); + entries.put(network, entry); + } + Fingerprint oldFingerprint = entry.fingerprint; + boolean privateDnsChanged = entry.updateLinkProperties(supplied); + boolean changed = !oldFingerprint.equals(entry.fingerprint); + if (changed) + return Change.of(NetworkReloadPolicy.REASON_LINK_PROPERTIES_CHANGED, privateDnsChanged); + if (privateDnsChanged) + return Change.of(NetworkReloadPolicy.REASON_PRIVATE_DNS_CHANGED, true); + return Change.none(); + } + + synchronized Change onPhysicalLost(Network network) { + if (network == null || entries.remove(network) == null) + return Change.none(); + if (network.equals(defaultNetwork)) + defaultNetwork = null; + return Change.of(NetworkReloadPolicy.REASON_NETWORK_LOST, false); + } + + synchronized Change onDefaultNetworkAvailable(Network network) { + if (network == null) + return Change.none(); + return acceptDefaultIfPhysical(network); + } + + synchronized Change onDefaultNetworkCapabilitiesChanged(Network network, NetworkCapabilities supplied) { + if (network == null || supplied == null) + return Change.none(); + if (!supplied.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN)) { + // The VPN often remains the default Network across Wi-Fi/cellular + // handover. Its physical transport set changes even when both + // underlying networks were already available. Ignore VPN identity + // and link-property churn caused by our own establish/reload. + int[] transports = Fingerprint.from(supplied, null).transports; + int count = 0; + for (int transport : transports) + if (transport != NetworkCapabilities.TRANSPORT_VPN) + transports[count++] = transport; + if (count == 0) + return Change.none(); + transports = Arrays.copyOf(transports, count); + boolean changed = defaultVpnTransports != null && + !Arrays.equals(defaultVpnTransports, transports); + defaultVpnTransports = transports; + return changed ? Change.of(NetworkReloadPolicy.REASON_NETWORK_CHANGED, false) : Change.none(); + } + Change change = onPhysicalCapabilitiesChanged(network, supplied); + Change defaultChange = acceptDefaultIfPhysical(network); + if (defaultChange.getReason() != null) + return defaultChange; + return change; + } + + synchronized Change onDefaultNetworkLinkPropertiesChanged(Network network, LinkProperties supplied) { + Entry entry = entries.get(network); + if (entry == null || entry.capabilities == null || + !entry.capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN)) + return Change.none(); + Change change = onPhysicalLinkPropertiesChanged(network, supplied); + Change defaultChange = acceptDefaultIfPhysical(network); + if (defaultChange.getReason() != null) + return defaultChange; + return change; + } + + synchronized Change onDefaultNetworkLost(Network network) { + if (network == null || !network.equals(defaultNetwork)) + return Change.none(); + defaultNetwork = null; + return Change.none(); + } + + private Change acceptDefaultIfPhysical(Network network) { + Entry entry = entries.get(network); + if (entry == null || entry.capabilities == null || + !entry.capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN)) + return Change.none(); + if (!baselineEstablished) { + defaultNetwork = network; + baselineEstablished = true; + return Change.none(); + } + if (!network.equals(defaultNetwork)) { + defaultNetwork = network; + return Change.of("Network changed", false); + } + return Change.none(); + } + + synchronized Network getDefaultNetwork() { + return defaultNetwork; + } + + synchronized NetworkCapabilities getCapabilities(Network network) { + Entry entry = entries.get(network); + return entry == null || entry.capabilities == null ? null : + new NetworkCapabilities(entry.capabilities); + } + + synchronized LinkProperties getLinkProperties(Network network) { + Entry entry = entries.get(network); + return entry == null ? null : Entry.copyLinkProperties(entry.linkProperties); + } + + synchronized void reset() { + entries.clear(); + defaultNetwork = null; + baselineEstablished = false; + defaultVpnTransports = null; + } +} diff --git a/app/src/main/java/eu/faircode/netguard/ServiceSinkhole.java b/app/src/main/java/eu/faircode/netguard/ServiceSinkhole.java index e0e09f221..c1f4593c7 100644 --- a/app/src/main/java/eu/faircode/netguard/ServiceSinkhole.java +++ b/app/src/main/java/eu/faircode/netguard/ServiceSinkhole.java @@ -180,7 +180,9 @@ private static int getIntPref(SharedPreferences prefs, String key, int def) { clearWireGuardErrorNotification(); }; - private Object networkCallback = null; + private ConnectivityManager.NetworkCallback networkCallback = null; + private ConnectivityManager.NetworkCallback defaultNetworkCallback = null; + private final PhysicalNetworkState physicalNetworkState = new PhysicalNetworkState(); private boolean registeredInteractiveState = false; private PhoneStateListener callStateListener = null; @@ -2210,6 +2212,13 @@ public void onProviderRejected(String providerLabel, String message) { })); jni_wireguard_required(prefs.getBoolean("wg_enabled", false) && !TextUtils.isEmpty(prefs.getString("wg_config", ""))); + List probeSources = (last_builder == null + ? new ArrayList<>() : new ArrayList<>(last_builder.listAddress)); + List probeResolvers = new ArrayList<>(); + if (last_builder != null) + for (InetAddress dns : last_builder.listDns) + if (dns != null && dns.getHostAddress() != null) + probeResolvers.add(dns.getHostAddress()); boolean wgOk = net.kollnig.missioncontrol.wg.WgEgress.INSTANCE.startOrUpdate( prefs.getBoolean("wg_enabled", false), prefs.getString("wg_config", ""), @@ -2218,7 +2227,9 @@ public void onProviderRejected(String providerLabel, String message) { Util.isInteractive(ServiceSinkhole.this), prefs.getBoolean("wg_keepalive_when_screen_off", false), () -> jni_wireguard_start(), - () -> { jni_wireguard_stop(); return kotlin.Unit.INSTANCE; }); + () -> { jni_wireguard_stop(); return kotlin.Unit.INSTANCE; }, + probeSources, + probeResolvers); if (!wgOk) { String wgError = net.kollnig.missioncontrol.wg.WgEgress.INSTANCE.getLastError(); Log.w(TAG, "WireGuard egress failed to start; blocking traffic: " + wgError); @@ -3877,64 +3888,30 @@ public void onCreate() { } private void listenNetworkChanges() { + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.N) { + listenConnectivityChanges(); + return; + } + // Listen for network changes Log.i(TAG, "Starting listening to network changes"); ConnectivityManager cm = (ConnectivityManager) getSystemService(Context.CONNECTIVITY_SERVICE); NetworkRequest.Builder builder = new NetworkRequest.Builder(); builder.addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET); - builder.addCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED); + builder.addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN); ConnectivityManager.NetworkCallback nc = new ConnectivityManager.NetworkCallback() { - private Network last_active = null; - private Network last_network = null; - private Boolean last_connected = null; - private Boolean last_metered = null; - private List last_dns = null; - private String last_private_dns = null; - @Override public void onAvailable(Network network) { Log.i(TAG, "Available network=" + network); - if (!isActiveNetwork(network)) - return; - - last_active = network; - last_network = network; - last_connected = Util.isConnected(ServiceSinkhole.this); - last_metered = Util.isMeteredNetwork(ServiceSinkhole.this); - reloadAfterNetworkChange(NetworkReloadPolicy.onNetworkAvailable()); + handlePhysicalNetworkChange(physicalNetworkState.onPhysicalAvailable(network)); } @Override public void onLinkPropertiesChanged(Network network, LinkProperties linkProperties) { Log.i(TAG, "Changed properties=" + network + " props=" + linkProperties); - if (!isActiveNetwork(network)) - return; - - // Make sure the right DNS servers are being used - List dns = linkProperties.getDnsServers(); - // Non-null only when Private DNS is pinned to a hostname, which - // leaves the resolver list untouched — so this is the only part - // of the properties that reveals the change. - String private_dns = (Build.VERSION.SDK_INT < Build.VERSION_CODES.P - ? null : linkProperties.getPrivateDnsServerName()); - SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(ServiceSinkhole.this); - String reason = NetworkReloadPolicy.onLinkPropertiesChanged( - last_dns, - dns, - Build.VERSION.SDK_INT >= Build.VERSION_CODES.O, - prefs.getBoolean("reload_onconnectivity", false), - last_private_dns, - private_dns); - if (reason != null) { - Log.i(TAG, "Changed link properties=" + linkProperties + - "DNS cur=" + TextUtils.join(",", dns) + - "DNS prv=" + (last_dns == null ? null : TextUtils.join(",", last_dns)) + - " private DNS cur=" + private_dns + " prv=" + last_private_dns); - last_dns = dns; - last_private_dns = private_dns; - reloadAfterNetworkChange(reason); - } + handlePhysicalNetworkChange(physicalNetworkState.onPhysicalLinkPropertiesChanged( + network, linkProperties)); if (vpn != null) requestPrivateDnsWarningUpdate(); } @@ -3942,42 +3919,63 @@ public void onLinkPropertiesChanged(Network network, LinkProperties linkProperti @Override public void onCapabilitiesChanged(Network network, NetworkCapabilities networkCapabilities) { Log.i(TAG, "Changed capabilities=" + network + " caps=" + networkCapabilities); - if (!isActiveNetwork(network)) - return; + handlePhysicalNetworkChange(physicalNetworkState.onPhysicalCapabilitiesChanged( + network, networkCapabilities)); + } - boolean connected = Util.isConnected(ServiceSinkhole.this); - boolean metered = Util.isMeteredNetwork(ServiceSinkhole.this); - Log.i(TAG, "Connected=" + connected + "/" + last_connected + - " metered=" + metered + "/" + last_metered); + @Override + public void onLost(Network network) { + Log.i(TAG, "Lost network=" + network); + handlePhysicalNetworkChange(physicalNetworkState.onPhysicalLost(network)); + } + }; - String reason = NetworkReloadPolicy.onCapabilitiesChanged( - network, last_network, - last_connected, connected, - last_metered, metered); + ConnectivityManager.NetworkCallback dnc = new ConnectivityManager.NetworkCallback() { + @Override + public void onAvailable(Network network) { + Log.i(TAG, "Default network available=" + network); + handlePhysicalNetworkChange(physicalNetworkState.onDefaultNetworkAvailable(network)); + } - if (reason != null) - reloadAfterNetworkChange(reason); + @Override + public void onCapabilitiesChanged(Network network, NetworkCapabilities capabilities) { + Log.i(TAG, "Default network capabilities=" + network + " caps=" + capabilities); + handlePhysicalNetworkChange(physicalNetworkState.onDefaultNetworkCapabilitiesChanged( + network, capabilities)); + } - last_network = network; - last_connected = connected; - last_metered = metered; + @Override + public void onLinkPropertiesChanged(Network network, LinkProperties linkProperties) { + Log.i(TAG, "Default network properties=" + network + " props=" + linkProperties); + handlePhysicalNetworkChange(physicalNetworkState.onDefaultNetworkLinkPropertiesChanged( + network, linkProperties)); + if (vpn != null) + requestPrivateDnsWarningUpdate(); } @Override public void onLost(Network network) { - Log.i(TAG, "Lost network=" + network + " active=" + isActiveNetwork(network)); - if (last_active == null || !last_active.equals(network)) - return; - - String reason = NetworkReloadPolicy.onNetworkLost(network, last_active); - last_active = null; - last_connected = Util.isConnected(ServiceSinkhole.this); - if (reason != null) - reloadAfterNetworkChange(reason); + Log.i(TAG, "Default network lost=" + network); + handlePhysicalNetworkChange(physicalNetworkState.onDefaultNetworkLost(network)); } }; + cm.registerNetworkCallback(builder.build(), nc); networkCallback = nc; + try { + cm.registerDefaultNetworkCallback(dnc); + defaultNetworkCallback = dnc; + } catch (Throwable ex) { + cm.unregisterNetworkCallback(nc); + networkCallback = null; + physicalNetworkState.reset(); + throw ex; + } + } + + private void handlePhysicalNetworkChange(PhysicalNetworkState.Change change) { + if (change.getReason() != null) + reloadAfterNetworkChange(change.getReason()); } // Network flapping (Wi-Fi<->cellular handoffs, DHCP renewals) fires several @@ -4250,9 +4248,13 @@ public void onDestroy() { registeredPackageChanged = false; } - if (networkCallback != null) { - unlistenNetworkChanges(); - networkCallback = null; + if (networkCallback != null || defaultNetworkCallback != null) { + try { + unlistenNetworkChanges(); + } finally { + networkCallback = null; + defaultNetworkCallback = null; + } } if (registeredConnectivityChanged) { unregisterReceiver(connectivityChangedReceiver); @@ -4311,7 +4313,19 @@ public void onDestroy() { private void unlistenNetworkChanges() { ConnectivityManager cm = (ConnectivityManager) getSystemService(Context.CONNECTIVITY_SERVICE); - cm.unregisterNetworkCallback((ConnectivityManager.NetworkCallback) networkCallback); + try { + if (networkCallback != null) + cm.unregisterNetworkCallback(networkCallback); + } finally { + try { + if (defaultNetworkCallback != null) + cm.unregisterNetworkCallback(defaultNetworkCallback); + } finally { + networkCallback = null; + defaultNetworkCallback = null; + physicalNetworkState.reset(); + } + } } private Notification getEnforcingNotification(int allowed, int blocked, int hosts) { diff --git a/app/src/main/java/net/kollnig/missioncontrol/wg/WgConnectivityMonitor.kt b/app/src/main/java/net/kollnig/missioncontrol/wg/WgConnectivityMonitor.kt index 4e5718578..e6471e65b 100644 --- a/app/src/main/java/net/kollnig/missioncontrol/wg/WgConnectivityMonitor.kt +++ b/app/src/main/java/net/kollnig/missioncontrol/wg/WgConnectivityMonitor.kt @@ -10,7 +10,9 @@ data class WgStats( val latestHandshakeMillis: Long, val hasFreshHandshake: Boolean = false, val tunWriteFailuresTotal: Long = 0L, - val tunWriteFailuresStreak: Long = 0L + val tunWriteFailuresStreak: Long = 0L, + val deliveredRxBytes: Long = 0L, + val probeReplyToken: Long = 0L ) /** Outcome of a single connectivity poll. */ @@ -82,9 +84,10 @@ internal class WgConnectivityChecker(private val prod: () -> Unit) { private var tunWriteFailureStartedAt: Long? = null private var tunWriteFailureRunIdentity: Long? = null private var tunWriteFailuresSuspended = false + private var deliveredRxBytes = 0L /** - * True when the most recent [tick] observed the rx counter advancing — + * True when the most recent [tick] observed the delivered-IP counter advancing — * decrypted return traffic, the only signal that proves the data path * works end to end. A completed handshake is NOT such proof (a path can * pass handshakes yet drop transport packets), so recovery backoff resets @@ -97,6 +100,7 @@ internal class WgConnectivityChecker(private val prod: () -> Unit) { fun seed(now: Long, stats: WgStats) { state = ConnState.Connecting(now, false, stats.rxBytes, stats.txBytes) lastTickSawRx = false + deliveredRxBytes = stats.deliveredRxBytes resetProd() resetTunWriteFailures(stats.tunWriteFailuresTotal) tunWriteFailuresSuspended = false @@ -108,7 +112,8 @@ internal class WgConnectivityChecker(private val prod: () -> Unit) { if (stats == null) return WgVerdict.GONE val rxAdvanced = update(now, stats.rxBytes, stats.txBytes) - lastTickSawRx = rxAdvanced + lastTickSawRx = stats.deliveredRxBytes > deliveredRxBytes + deliveredRxBytes = stats.deliveredRxBytes // A completed handshake can coexist with a TUN fd that rejects every // decrypted packet. Evaluate the write-failure evidence first so the @@ -123,7 +128,6 @@ internal class WgConnectivityChecker(private val prod: () -> Unit) { } if (rxAdvanced) { - lastTickSawRx = true resetProd() return WgVerdict.HEALTHY } @@ -334,7 +338,11 @@ internal class WgConnectivityMonitor( // Test-only barrier between callback authorization and dispatch. Keeping // this seam here makes the stop/callback ordering deterministic without // adding any production scheduling. - private val beforeCallback: () -> Unit = {} + private val beforeCallback: () -> Unit = {}, + // A generation-gated sample stream used by handover verification. The + // callback runs on the monitor thread and must only enqueue JNI work. + private val onSample: (WgStats) -> Unit = {}, + private val onSuspended: () -> Unit = {} ) { internal companion object { /** How often the loop samples the tunnel counters while the screen is on. */ @@ -611,6 +619,7 @@ internal class WgConnectivityMonitor( // The device dozed; the elapsed gap is not evidence of a stall. if (isSuspendGap(slept, intervalMs)) { statsFailures = 0 + invokeCallback(generation, "onSuspended", onSuspended) checker.onSuspended(now) continue } @@ -642,6 +651,9 @@ internal class WgConnectivityMonitor( statsFailures = 0 if (!isCurrent()) return + invokeCallback(generation, "onSample") { onSample(stats) } + if (!isActive(generation)) return + when (checker.tick(now, stats)) { WgVerdict.HEALTHY, WgVerdict.WAITING -> {} WgVerdict.BROKEN -> { diff --git a/app/src/main/java/net/kollnig/missioncontrol/wg/WgEgress.kt b/app/src/main/java/net/kollnig/missioncontrol/wg/WgEgress.kt index 1aca50ed9..534e086a3 100644 --- a/app/src/main/java/net/kollnig/missioncontrol/wg/WgEgress.kt +++ b/app/src/main/java/net/kollnig/missioncontrol/wg/WgEgress.kt @@ -143,6 +143,11 @@ object WgEgress { // [reportProviderFailure]. @Volatile private var pendingProviderFailure: String? = null @Volatile private var verificationGeneration: Long = 0 + @Volatile private var pendingHandoverVerification: Boolean = false + @Volatile private var handoverUpdateInProgress: Boolean = false + @Volatile private var currentProbeSources: List = emptyList() + @Volatile private var currentProbeResolvers: List = emptyList() + private val handoverVerifier = WgHandoverVerifier() @Volatile private var currentConfig: String? = null private var currentTunFd: Int = -1 // The exact ParcelFileDescriptor the running tunnel was started with. A @@ -274,117 +279,134 @@ object WgEgress { interactive: Boolean, keepaliveAlwaysOn: Boolean, startSocketpair: () -> Int, - stopSocketpair: () -> Unit + stopSocketpair: () -> Unit, + probeSources: List = emptyList(), + probeResolvers: List = emptyList() ): Boolean { - verificationGeneration++ - val wantRunning = wgEnabled && !configText.isNullOrEmpty() - val desiredFd = vpnFd.fd - lastError = null + synchronized(tunnelLifecycleLock) { + handoverUpdateInProgress = true + pendingHandoverVerification = pendingHandoverVerification || handoverVerifier.isPending() + verificationGeneration++ + handoverVerifier.cancel() + } + try { + val wantRunning = wgEnabled && !configText.isNullOrEmpty() + val desiredFd = vpnFd.fd + lastError = null + + if (!wantRunning) { + clearRecoveryState() + pendingHandoverVerification = false + handoverVerifier.cancel() + clearAllEndpointState() + if (tunnel != null) { + Log.i(TAG, "WG disabled — tearing down tunnel") + stopInternal(stopSocketpair) + notifyStateChanged() + } + return true + } + + if (tunnel != null && currentConfig == configText && currentTunPfd === vpnFd && !forceRestartPending) { + val oldKeepaliveEnabled = currentInteractive || currentKeepaliveAlwaysOn + val newKeepaliveEnabled = interactive || keepaliveAlwaysOn + if (oldKeepaliveEnabled != newKeepaliveEnabled && + !updateKeepaliveOrError(configText!!, newKeepaliveEnabled, interactive, keepaliveAlwaysOn)) + return false + // The tunnel can outlive a monitor whose initial stats read raced + // tunnel startup (or which exited after a stale sample). Keep the + // idempotent tunnel path, but recreate a dead watchdog so a + // same-config start does not silently leave connectivity unwatched. + startMonitorIfDead() + currentProbeSources = probeSources.toList() + currentProbeResolvers = probeResolvers.toList() + Log.v(TAG, "startOrUpdate: same config + same TUN pfd, no-op") + return true + } - if (!wantRunning) { - clearRecoveryState() - clearAllEndpointState() if (tunnel != null) { - Log.i(TAG, "WG disabled — tearing down tunnel") + Log.i(TAG, "WG config, TUN fd, or recovery state changed — restarting") stopInternal(stopSocketpair) - notifyStateChanged() } - return true - } + forceRestartPending = false + val keepaliveEnabled = interactive || keepaliveAlwaysOn - if (tunnel != null && currentConfig == configText && currentTunPfd === vpnFd && !forceRestartPending) { - val oldKeepaliveEnabled = currentInteractive || currentKeepaliveAlwaysOn - val newKeepaliveEnabled = interactive || keepaliveAlwaysOn - if (oldKeepaliveEnabled != newKeepaliveEnabled && - !updateKeepaliveOrError(configText!!, newKeepaliveEnabled, interactive, keepaliveAlwaysOn)) + val parsed = try { + WgConfigParser.parse(configText!!) + } catch (e: Exception) { + lastError = "Invalid WireGuard config: ${e.message}" + Log.e(TAG, "config parse: ${e.message}") + notifyStateChanged() return false - // The tunnel can outlive a monitor whose initial stats read raced - // tunnel startup (or which exited after a stale sample). Keep the - // idempotent tunnel path, but recreate a dead watchdog so a - // same-config start does not silently leave connectivity unwatched. - startMonitorIfDead() - Log.v(TAG, "startOrUpdate: same config + same TUN pfd, no-op") - return true - } - - if (tunnel != null) { - Log.i(TAG, "WG config, TUN fd, or recovery state changed — restarting") - stopInternal(stopSocketpair) - } - forceRestartPending = false - val keepaliveEnabled = interactive || keepaliveAlwaysOn + } - val parsed = try { - WgConfigParser.parse(configText!!) - } catch (e: Exception) { - lastError = "Invalid WireGuard config: ${e.message}" - Log.e(TAG, "config parse: ${e.message}") - notifyStateChanged() - return false - } + val resolved = try { + withResolvedEndpoints(parsed) + } catch (e: Exception) { + lastError = "WireGuard endpoint resolution failed: ${e.message}" + Log.e(TAG, "endpoint resolve: ${e.message}") + notifyStateChanged() + return false + } - val resolved = try { - withResolvedEndpoints(parsed) - } catch (e: Exception) { - lastError = "WireGuard endpoint resolution failed: ${e.message}" - Log.e(TAG, "endpoint resolve: ${e.message}") - notifyStateChanged() - return false - } + val rxFd = startSocketpair() + if (rxFd < 0) { + lastError = "Could not create WireGuard packet socket" + Log.e(TAG, "jni_wireguard_start failed") + notifyStateChanged() + return false + } - val rxFd = startSocketpair() - if (rxFd < 0) { - lastError = "Could not create WireGuard packet socket" - Log.e(TAG, "jni_wireguard_start failed") - notifyStateChanged() - return false - } + val mtu = resolved.mtu ?: DEFAULT_MTU + val protector = object : WgProtector { + override fun protect(fd: Int): Boolean = vpnService.protect(fd) + } + val logger = object : WgLogger { + override fun verbosef(s: String) { Log.v(TAG, s) } + override fun errorf(s: String) { Log.e(TAG, s) } + } + val dnsRecorder = object : WgDnsRecorder { + override fun recordDns(qname: String, aname: String, resource: String, ttl: Int) { + if (vpnService is eu.faircode.netguard.ServiceSinkhole) + vpnService.wireGuardDnsResolved(qname, aname, resource, ttl) + } + } - val mtu = resolved.mtu ?: DEFAULT_MTU - val protector = object : WgProtector { - override fun protect(fd: Int): Boolean = vpnService.protect(fd) - } - val logger = object : WgLogger { - override fun verbosef(s: String) { Log.v(TAG, s) } - override fun errorf(s: String) { Log.e(TAG, s) } - } - val dnsRecorder = object : WgDnsRecorder { - override fun recordDns(qname: String, aname: String, resource: String, ttl: Int) { - if (vpnService is eu.faircode.netguard.ServiceSinkhole) - vpnService.wireGuardDnsResolved(qname, aname, resource, ttl) + val startedTunnel = try { + Wgbridge.startTunnel( + resolved.toUapi(keepaliveEnabled), rxFd, desiredFd, mtu, protector, logger, dnsRecorder + ) + } catch (e: Throwable) { + lastError = "WireGuard tunnel failed to start: ${e.message ?: e.javaClass.simpleName}" + Log.e(TAG, "Wgbridge.startTunnel failed", e) + stopSocketpair() + notifyStateChanged() + return false + } finally { + closeRawFd(rxFd) } - } - val startedTunnel = try { - Wgbridge.startTunnel( - resolved.toUapi(keepaliveEnabled), rxFd, desiredFd, mtu, protector, logger, dnsRecorder - ) - } catch (e: Throwable) { - lastError = "WireGuard tunnel failed to start: ${e.message ?: e.javaClass.simpleName}" - Log.e(TAG, "Wgbridge.startTunnel failed", e) - stopSocketpair() + currentConfig = configText + currentTunFd = desiredFd + currentTunPfd = vpnFd + currentInteractive = interactive + currentKeepaliveAlwaysOn = keepaliveAlwaysOn + currentProbeSources = probeSources.toList() + currentProbeResolvers = probeResolvers.toList() + synchronized(tunnelLifecycleLock) { + tunnelGeneration.incrementAndGet() + // Volatile publication happens only after all companion state has + // been installed above. + tunnel = startedTunnel + } + Log.i(TAG, "WG up: tunFd=$desiredFd mtu=$mtu peers=${resolved.peers.size}") notifyStateChanged() - return false + scheduleFreshHandshakeNotificationCheck() + startMonitor() + return true } finally { - closeRawFd(rxFd) + handoverUpdateInProgress = false } - - currentConfig = configText - currentTunFd = desiredFd - currentTunPfd = vpnFd - currentInteractive = interactive - currentKeepaliveAlwaysOn = keepaliveAlwaysOn - synchronized(tunnelLifecycleLock) { - tunnelGeneration.incrementAndGet() - // Volatile publication happens only after all companion state has - // been installed above. - tunnel = startedTunnel - } - Log.i(TAG, "WG up: tunFd=$desiredFd mtu=$mtu peers=${resolved.peers.size}") - notifyStateChanged() - scheduleFreshHandshakeNotificationCheck() - startMonitor() - return true } private fun startMonitor() { @@ -419,7 +441,9 @@ object WgEgress { // produces, so resetting on it would defeat the backoff. onRxAdvanced = { if (isCurrent(expected)) restartAttempts = 0 }, isInteractive = { currentInteractive }, - isCurrent = { isCurrent(expected) } + isCurrent = { isCurrent(expected) }, + onSample = { stats -> onHandoverSample(expected, stats) }, + onSuspended = { handoverVerifier.onSuspended() } ) monitorLifecycle.replace( candidate, @@ -428,6 +452,82 @@ object WgEgress { ) } + private fun beginHandoverVerification(expected: TunnelSnapshot, baseline: WgStats? = null) { + val generation = synchronized(tunnelLifecycleLock) { + if (!pendingHandoverVerification || handoverUpdateInProgress || forceRestartPending || + !isCurrentLocked(expected)) return + verificationGeneration + } + synchronized(rebindLock) { + if (rebindInFlight) return + } + val sample = baseline ?: statsOrNull(expected) ?: return + val config = currentConfig ?: return + val targets = try { + val parsed = WgConfigParser.parse(config) + WgProbeTargetSelector.select( + currentProbeSources, + currentProbeResolvers, + parsed.peers.flatMap { it.allowedIPs } + ) + } catch (e: Throwable) { + Log.w(TAG, "handover probe target selection failed", e) + emptyList() + } + val action = synchronized(tunnelLifecycleLock) { + if (!pendingHandoverVerification || handoverUpdateInProgress || forceRestartPending || !isCurrentLocked(expected) || + generation != verificationGeneration) return + pendingHandoverVerification = false + handoverVerifier.begin(generation, sample, targets, currentInteractive) + } + dispatchHandoverAction(expected, action) + } + + private fun onHandoverSample(expected: TunnelSnapshot, stats: WgStats) { + if (!isCurrent(expected)) return + if (pendingHandoverVerification) { + beginHandoverVerification(expected, stats) + return + } + dispatchHandoverAction( + expected, + handoverVerifier.onSample(verificationGeneration, stats, currentInteractive) + ) + } + + private fun dispatchHandoverAction(expected: TunnelSnapshot, action: WgHandoverVerifier.Action) { + when (action) { + WgHandoverVerifier.Action.None -> Unit + is WgHandoverVerifier.Action.Probe -> { + rebindExecutor.execute { + if (!currentInteractive || !isCurrent(expected) || + !handoverVerifier.isCurrent(action.generation, action.token)) return@execute + try { + if (!expected.tunnel.sendDnsProbe( + action.target.sourceIp, + action.target.resolverIp, + action.token + ) + ) Log.i(TAG, "handover probe enqueue was rejected; awaiting bounded retry") + } catch (e: Throwable) { + Log.w(TAG, "handover probe enqueue failed; awaiting bounded retry", e) + } + } + } + is WgHandoverVerifier.Action.Restart -> { + if (action.generation == verificationGeneration && isCurrent(expected)) { + requestFullRestart( + "WG handover verification failed", + notify = false, + expected = expected, + eligibleForFailover = false, + expectedVerificationGeneration = action.generation + ) + } + } + } + } + private fun stopMonitor() { monitorLifecycle.stop() } @@ -470,7 +570,8 @@ object WgEgress { reason: String, notify: Boolean, expected: TunnelSnapshot, - eligibleForFailover: Boolean + eligibleForFailover: Boolean, + expectedVerificationGeneration: Long? = null ) { val attempt: Int synchronized(tunnelLifecycleLock) { @@ -478,6 +579,12 @@ object WgEgress { // tunnel replacement. Otherwise a replacement can land between // them and inherit forceRestartPending from an obsolete failure. if (!isCurrentLocked(expected)) return + if (expectedVerificationGeneration != null && + (expectedVerificationGeneration != verificationGeneration || !currentInteractive)) return + // Verify the replacement too. A successful construction/handshake + // alone must not end recovery of a handover that lost its data path. + if (expectedVerificationGeneration != null) + pendingHandoverVerification = true clearEndpointCache() attempt = restartAttempts++ forceRestartPending = true @@ -672,7 +779,9 @@ object WgEgress { it.latestHandshakeMillis, it.latestHandshakeMillis > 0 && now() - it.latestHandshakeMillis < HANDSHAKE_DEAD_AFTER_MS, it.tunWriteFailuresTotal, - it.tunWriteFailuresStreak + it.tunWriteFailuresStreak, + it.deliveredRxBytes, + it.probeReplyToken ) } } catch (e: Throwable) { @@ -715,7 +824,11 @@ object WgEgress { try { tunnel?.latestHandshakeMillis() } catch (_: Throwable) { null } fun onUnderlyingNetworkChanged() { - verificationGeneration++ + synchronized(tunnelLifecycleLock) { + verificationGeneration++ + pendingHandoverVerification = tunnel != null + handoverVerifier.cancel() + } clearEndpointCache() if (tunnel == null) return // A full restart is already queued (and the accompanying reload() is @@ -754,7 +867,11 @@ object WgEgress { } when (tryCheapRecovery(expected)) { RecoveryResult.SUCCEEDED -> { - if (isCurrent(expected)) lastCheapRecoveryMs = now() + if (isCurrent(expected)) { + lastCheapRecoveryMs = now() + // The monitor starts verification after this + // rebind (and any dirty re-run) leaves the queue. + } } RecoveryResult.FAILED -> { if (!forceRestartPending) { @@ -810,6 +927,10 @@ object WgEgress { val oldKeepaliveEnabled = currentInteractive || currentKeepaliveAlwaysOn val newKeepaliveEnabled = interactive || keepaliveAlwaysOn + synchronized(tunnelLifecycleLock) { + currentInteractive = interactive + handoverVerifier.setInteractive(interactive) + } if (oldKeepaliveEnabled == newKeepaliveEnabled) { currentInteractive = interactive currentKeepaliveAlwaysOn = keepaliveAlwaysOn @@ -852,7 +973,8 @@ object WgEgress { currentTunPfd = null currentKeepaliveAlwaysOn = false lastCheapRecoveryMs = 0 - verificationGeneration++ + bumpVerificationGeneration() + handoverVerifier.cancel() // An error describes a tunnel that no longer exists. Listeners check // lastError before isRunning — deliberately, so a start that fails // without ever producing a tunnel still reports — so leaving it set @@ -936,7 +1058,9 @@ object WgEgress { } private fun clearRecoveryState() { - verificationGeneration++ + bumpVerificationGeneration() + pendingHandoverVerification = false + handoverVerifier.cancel() recoveryNotificationGeneration++ providerFailureReason = null pendingProviderFailure = null @@ -976,6 +1100,12 @@ object WgEgress { private fun now(): Long = System.currentTimeMillis() + private fun bumpVerificationGeneration() { + synchronized(tunnelLifecycleLock) { + verificationGeneration++ + } + } + private fun withResolvedEndpoints(config: WgConfig): WgConfig { return config.copy(peers = config.peers.map { peer -> val ep = peer.endpoint diff --git a/app/src/main/java/net/kollnig/missioncontrol/wg/WgHandoverVerifier.kt b/app/src/main/java/net/kollnig/missioncontrol/wg/WgHandoverVerifier.kt new file mode 100644 index 000000000..d99940e20 --- /dev/null +++ b/app/src/main/java/net/kollnig/missioncontrol/wg/WgHandoverVerifier.kt @@ -0,0 +1,208 @@ +package net.kollnig.missioncontrol.wg + +import java.net.InetAddress + +/** A numeric source/resolver pair for an in-tunnel DNS probe. */ +internal data class WgProbeTarget(val sourceIp: String, val resolverIp: String) + +/** + * Bounded, generation-scoped verification after an underlying-network change. + * It is deliberately clock-injected and side-effect free: the owner dispatches + * [Action.Probe] away from the main thread and decides how to restart. + */ +internal class WgHandoverVerifier( + private val clock: () -> Long = { android.os.SystemClock.elapsedRealtime() } +) { + companion object { + const val PROBE_INTERVAL_MS = 5_000L + const val VERIFY_TIMEOUT_MS = 15_000L + const val MAX_PROBES = 3 + } + + sealed class Action { + object None : Action() + data class Probe(val generation: Long, val target: WgProbeTarget, val token: Long) : Action() + data class Restart(val generation: Long) : Action() + } + + private data class Session( + val generation: Long, + val targets: List, + var baselineDelivered: Long, + var currentToken: Long, + var probesSent: Int, + var nextProbeAt: Long, + var deadline: Long, + var paused: Boolean + ) + + private var session: Session? = null + private var nextToken = 0L + + @Synchronized fun begin( + generation: Long, + baseline: WgStats, + targets: List, + interactive: Boolean, + now: Long = clock() + ): Action { + session = null + if (targets.isEmpty()) return Action.None + val next = Session( + generation = generation, + targets = targets.toList(), + baselineDelivered = baseline.deliveredRxBytes, + currentToken = 0L, + probesSent = 0, + nextProbeAt = now, + deadline = now + VERIFY_TIMEOUT_MS, + paused = !interactive + ) + session = next + return if (interactive) issueProbe(next, now) else Action.None + } + + @Synchronized fun onSample( + generation: Long, + stats: WgStats, + interactive: Boolean, + now: Long = clock() + ): Action { + val current = session ?: return Action.None + if (current.generation != generation) return Action.None + + if (!interactive) { + // Screen-off verification is deferred and rebased. This avoids + // probes and deadlines being driven by the idle monitor cadence. + current.paused = true + current.baselineDelivered = stats.deliveredRxBytes + current.currentToken = 0L + current.probesSent = 0 + return Action.None + } + + if (current.paused) { + current.paused = false + current.baselineDelivered = stats.deliveredRxBytes + current.currentToken = 0L + current.probesSent = 0 + current.nextProbeAt = now + current.deadline = now + VERIFY_TIMEOUT_MS + return issueProbe(current, now) + } + + if (stats.deliveredRxBytes > current.baselineDelivered || + (current.currentToken != 0L && stats.probeReplyToken == current.currentToken)) { + session = null + return Action.None + } + + if (now >= current.deadline) { + session = null + return Action.Restart(current.generation) + } + + return if (current.probesSent < MAX_PROBES && now >= current.nextProbeAt) + issueProbe(current, now) + else + Action.None + } + + @Synchronized fun isCurrent(generation: Long, token: Long): Boolean = + session?.let { it.generation == generation && it.currentToken == token } == true + + @Synchronized fun isPending(): Boolean = session != null + + @Synchronized fun cancel() { + session = null + } + + @Synchronized fun setInteractive(interactive: Boolean) { + if (!interactive) { + session?.apply { + paused = true + currentToken = 0L + probesSent = 0 + } + } + } + + @Synchronized fun onSuspended() { + setInteractive(false) + } + + @Synchronized fun cancelIfCurrent(generation: Long, token: Long) { + if (isCurrent(generation, token)) session = null + } + + private fun issueProbe(session: Session, now: Long): Action { + val token = ++nextToken + session.currentToken = token + session.probesSent++ + session.nextProbeAt = now + PROBE_INTERVAL_MS + val target = session.targets[(session.probesSent - 1) % session.targets.size] + return Action.Probe(session.generation, target, token) + } +} + +/** Select only numeric, same-family resolver targets covered by WireGuard routes. */ +internal object WgProbeTargetSelector { + private val ipv4 = Regex("^(?:[0-9]{1,3}\\.){3}[0-9]{1,3}$") + + fun select( + sourceAddresses: List, + resolverAddresses: List, + allowedIps: List + ): List { + val sources = sourceAddresses.mapNotNull { numericAddress(it.substringBefore('/')) } + val resolvers = resolverAddresses.mapNotNull { numericAddress(it) } + val targets = ArrayList() + for ((sourceText, source) in sources) { + for ((resolverText, resolver) in resolvers) { + if (source.size != resolver.size || !allowedIps.any { contains(it, resolver) }) + continue + val target = WgProbeTarget(sourceText, resolverText) + if (!targets.contains(target)) targets += target + } + } + return targets + } + + private fun numericAddress(text: String, rejectSpecial: Boolean = true): Pair? { + if (text.isEmpty()) return null + if (!text.contains(':') && !ipv4.matches(text)) return null + if (text.contains(':') && !WgConfigParser.isIpv6Literal(text)) return null + val bytes = try { InetAddress.getByName(text).address } catch (_: Exception) { return null } + val address = try { InetAddress.getByAddress(bytes) } catch (_: Exception) { return null } + if (rejectSpecial && + (address.isAnyLocalAddress || address.isMulticastAddress || address.isLoopbackAddress)) + return null + return if ((text.contains(':') && bytes.size == 16) || + (!text.contains(':') && bytes.size == 4)) text to bytes else null + } + + private fun contains(entry: String, address: ByteArray): Boolean { + val slash = entry.indexOf('/') + val prefixText = if (slash < 0) null else entry.substring(slash + 1) + val parsed = numericAddress( + if (slash < 0) entry else entry.substring(0, slash), + rejectSpecial = false + ) ?: return false + if (parsed.second.size != address.size) return false + val prefix = if (prefixText == null) parsed.second.size * 8 + else prefixText.toIntOrNull() ?: return false + if (prefix !in 0..parsed.second.size * 8) return false + var remaining = prefix + for (i in parsed.second.indices) { + if (remaining >= 8) { + if (parsed.second[i] != address[i]) return false + remaining -= 8 + } else if (remaining > 0) { + val mask = (0xff shl (8 - remaining)) and 0xff + if ((parsed.second[i].toInt() and mask) != (address[i].toInt() and mask)) return false + break + } else break + } + return true + } +} diff --git a/app/src/main/java/net/kollnig/missioncontrol/wgbridge/Tunnel.java b/app/src/main/java/net/kollnig/missioncontrol/wgbridge/Tunnel.java index 94ccfece8..e3eeee09c 100644 --- a/app/src/main/java/net/kollnig/missioncontrol/wgbridge/Tunnel.java +++ b/app/src/main/java/net/kollnig/missioncontrol/wgbridge/Tunnel.java @@ -21,15 +21,16 @@ public synchronized void setConfig(String uapiConfig) { /** * Snapshot of the device's transfer counters and newest handshake, summed - * across all peers. rx bytes count decrypted transport payload, so they - * only advance when the tunnel actually carries return traffic — that is - * the liveness signal the connectivity monitor is biased toward. + * across all peers. Engine rxBytes includes handshakes; deliveredRxBytes + * counts only complete decrypted IP packets successfully written to Android. */ public synchronized TunnelStats stats() { long[] values = nativeStats(handle); long totalFailures = values.length > 3 ? values[3] : 0L; long failureStreak = values.length > 4 ? values[4] : 0L; - return new TunnelStats(values[0], values[1], values[2], totalFailures, failureStreak); + long deliveredRx = values.length > 5 ? values[5] : 0L; + long probeReply = values.length > 6 ? values[6] : 0L; + return new TunnelStats(values[0], values[1], values[2], totalFailures, failureStreak, deliveredRx, probeReply); } /** @@ -56,6 +57,11 @@ public synchronized void rebind() { nativeRebind(handle); } + /** Queues a DNS reachability probe inside WireGuard. No direct socket send. */ + public synchronized boolean sendDnsProbe(String sourceIp, String resolverIp, long token) { + return nativeSendDnsProbe(handle, sourceIp, resolverIp, token); + } + /** * Moves a peer to a new endpoint ("ip:port" or "[ipv6]:port", already * resolved) without disturbing the session, e.g. after DNS re-resolution. @@ -92,6 +98,8 @@ public synchronized void stop() { private static native void nativeRebind(long handle); + private static native boolean nativeSendDnsProbe(long handle, String sourceIp, String resolverIp, long token); + private static native void nativeUpdateEndpoint(long handle, String peerPublicKeyBase64, String endpoint); private static native void nativeSetKeepalive(long handle, String peerPublicKeyBase64, int seconds); diff --git a/app/src/main/java/net/kollnig/missioncontrol/wgbridge/TunnelStats.java b/app/src/main/java/net/kollnig/missioncontrol/wgbridge/TunnelStats.java index 9480a8999..c1340e410 100644 --- a/app/src/main/java/net/kollnig/missioncontrol/wgbridge/TunnelStats.java +++ b/app/src/main/java/net/kollnig/missioncontrol/wgbridge/TunnelStats.java @@ -9,6 +9,8 @@ public final class TunnelStats { public final long latestHandshakeMillis; public final long tunWriteFailuresTotal; public final long tunWriteFailuresStreak; + public final long deliveredRxBytes; + public final long probeReplyToken; TunnelStats(long rxBytes, long txBytes, long latestHandshakeMillis) { this(rxBytes, txBytes, latestHandshakeMillis, 0L, 0L); @@ -16,17 +18,37 @@ public final class TunnelStats { TunnelStats(long rxBytes, long txBytes, long latestHandshakeMillis, long tunWriteFailuresTotal, long tunWriteFailuresStreak) { + this(rxBytes, txBytes, latestHandshakeMillis, tunWriteFailuresTotal, tunWriteFailuresStreak, 0L); + } + + TunnelStats(long rxBytes, long txBytes, long latestHandshakeMillis, + long tunWriteFailuresTotal, long tunWriteFailuresStreak, long deliveredRxBytes) { + this(rxBytes, txBytes, latestHandshakeMillis, tunWriteFailuresTotal, tunWriteFailuresStreak, deliveredRxBytes, 0L); + } + + TunnelStats(long rxBytes, long txBytes, long latestHandshakeMillis, + long tunWriteFailuresTotal, long tunWriteFailuresStreak, long deliveredRxBytes, long probeReplyToken) { this.rxBytes = rxBytes; this.txBytes = txBytes; this.latestHandshakeMillis = latestHandshakeMillis; this.tunWriteFailuresTotal = tunWriteFailuresTotal; this.tunWriteFailuresStreak = tunWriteFailuresStreak; + this.deliveredRxBytes = deliveredRxBytes; + this.probeReplyToken = probeReplyToken; } public long getRxBytes() { return rxBytes; } + public long getDeliveredRxBytes() { + return deliveredRxBytes; + } + + public long getProbeReplyToken() { + return probeReplyToken; + } + public long getTxBytes() { return txBytes; } diff --git a/app/src/test/java/eu/faircode/netguard/PhysicalNetworkStateTest.java b/app/src/test/java/eu/faircode/netguard/PhysicalNetworkStateTest.java new file mode 100644 index 000000000..7c941b730 --- /dev/null +++ b/app/src/test/java/eu/faircode/netguard/PhysicalNetworkStateTest.java @@ -0,0 +1,204 @@ +package eu.faircode.netguard; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import android.net.LinkProperties; +import android.net.Network; +import android.net.NetworkCapabilities; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.Shadows; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.shadows.ShadowNetwork; +import org.robolectric.shadows.ShadowNetworkCapabilities; + +import java.net.InetAddress; +import java.lang.reflect.Field; +import java.util.Collections; + +@RunWith(RobolectricTestRunner.class) +public class PhysicalNetworkStateTest { + private static final Network WIFI = ShadowNetwork.newInstance(101); + private static final Network CELL = ShadowNetwork.newInstance(102); + private static final Network VPN = ShadowNetwork.newInstance(103); + + @Test + @org.robolectric.annotation.Config(sdk = 24) + public void olderAndroidStoresIndependentCallbackSnapshots() throws Exception { + PhysicalNetworkState state = new PhysicalNetworkState(); + NetworkCapabilities caps = capabilities(NetworkCapabilities.TRANSPORT_WIFI); + state.onPhysicalCapabilitiesChanged(WIFI, caps); + LinkProperties props = linkProperties("9.9.9.9"); + state.onPhysicalLinkPropertiesChanged(WIFI, props); + props.setDnsServers(Collections.singleton(InetAddress.getByName("1.1.1.1"))); + assertEquals("9.9.9.9", state.getLinkProperties(WIFI).getDnsServers().get(0).getHostAddress()); + assertEquals(NetworkReloadPolicy.REASON_LINK_PROPERTIES_CHANGED, + state.onPhysicalLinkPropertiesChanged(WIFI, props).getReason()); + } + + private static NetworkCapabilities capabilities(int transport) { + NetworkCapabilities capabilities = ShadowNetworkCapabilities.newInstance(); + ShadowNetworkCapabilities shadow = Shadows.shadowOf(capabilities); + shadow.addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET); + shadow.addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN); + shadow.addTransportType(transport); + return capabilities; + } + + private static LinkProperties linkProperties(String dns) throws Exception { + LinkProperties properties = new LinkProperties(); + properties.setDnsServers(Collections.singleton(InetAddress.getByName(dns))); + return properties; + } + + @Test + public void vpnDefaultTransportHandoverReloadsWithoutVpnIdentityChurn() throws Exception { + PhysicalNetworkState state = new PhysicalNetworkState(); + NetworkCapabilities wifiVpn = capabilities(NetworkCapabilities.TRANSPORT_WIFI); + Shadows.shadowOf(wifiVpn).removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN); + Shadows.shadowOf(wifiVpn).addTransportType(NetworkCapabilities.TRANSPORT_VPN); + assertNull(state.onDefaultNetworkCapabilitiesChanged(VPN, wifiVpn).getReason()); + + NetworkCapabilities cellVpn = capabilities(NetworkCapabilities.TRANSPORT_CELLULAR); + Shadows.shadowOf(cellVpn).removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN); + Shadows.shadowOf(cellVpn).addTransportType(NetworkCapabilities.TRANSPORT_VPN); + assertEquals(NetworkReloadPolicy.REASON_NETWORK_CHANGED, + state.onDefaultNetworkCapabilitiesChanged(VPN, cellVpn).getReason()); + + Network replacement = ShadowNetwork.newInstance(104); + assertNull(state.onDefaultNetworkAvailable(replacement).getReason()); + assertNull(state.onDefaultNetworkCapabilitiesChanged(replacement, cellVpn).getReason()); + assertNull(state.onDefaultNetworkLinkPropertiesChanged(replacement, + linkProperties("9.9.9.9")).getReason()); + assertNull(state.onDefaultNetworkLost(VPN).getReason()); + } + + @Test + public void physicalCallbacksWorkWhileVpnIsDefault() { + PhysicalNetworkState state = new PhysicalNetworkState(); + + assertEquals(NetworkReloadPolicy.REASON_NETWORK_AVAILABLE, + state.onPhysicalAvailable(WIFI).getReason()); + assertEquals(NetworkReloadPolicy.REASON_NETWORK_CHANGED, + state.onPhysicalCapabilitiesChanged(WIFI, capabilities( + NetworkCapabilities.TRANSPORT_WIFI)).getReason()); + assertNotNull(state.getCapabilities(WIFI)); + assertNull(state.getDefaultNetwork()); + } + + @Test + public void defaultSwitchIsDetectedWhenBothPhysicalNetworksRemain() { + PhysicalNetworkState state = new PhysicalNetworkState(); + state.onPhysicalAvailable(WIFI); + state.onPhysicalCapabilitiesChanged(WIFI, capabilities(NetworkCapabilities.TRANSPORT_WIFI)); + state.onPhysicalAvailable(CELL); + state.onPhysicalCapabilitiesChanged(CELL, capabilities(NetworkCapabilities.TRANSPORT_CELLULAR)); + + assertNull(state.onDefaultNetworkAvailable(WIFI).getReason()); + assertNull(state.onDefaultNetworkCapabilitiesChanged(WIFI, + capabilities(NetworkCapabilities.TRANSPORT_WIFI)).getReason()); + assertEquals(NetworkReloadPolicy.REASON_NETWORK_CHANGED, + state.onDefaultNetworkAvailable(CELL).getReason()); + assertEquals(CELL, state.getDefaultNetwork()); + } + + @Test + public void defaultPhysicalTransportChangeReloads() { + PhysicalNetworkState state = new PhysicalNetworkState(); + state.onPhysicalAvailable(WIFI); + state.onPhysicalCapabilitiesChanged(WIFI, capabilities(NetworkCapabilities.TRANSPORT_WIFI)); + state.onDefaultNetworkAvailable(WIFI); + state.onDefaultNetworkCapabilitiesChanged(WIFI, + capabilities(NetworkCapabilities.TRANSPORT_WIFI)); + + assertEquals(NetworkReloadPolicy.REASON_NETWORK_CHANGED, + state.onDefaultNetworkCapabilitiesChanged(WIFI, + capabilities(NetworkCapabilities.TRANSPORT_CELLULAR)).getReason()); + assertEquals(WIFI, state.getDefaultNetwork()); + } + + @Test + public void signalAndBandwidthChatterDoesNotReload() throws Exception { + PhysicalNetworkState state = new PhysicalNetworkState(); + state.onPhysicalAvailable(CELL); + NetworkCapabilities initial = capabilities(NetworkCapabilities.TRANSPORT_CELLULAR); + state.onPhysicalCapabilitiesChanged(CELL, initial); + + NetworkCapabilities chatter = new NetworkCapabilities(initial); + ShadowNetworkCapabilities chatterShadow = Shadows.shadowOf(chatter); + chatterShadow.setLinkDownstreamBandwidthKbps(12000); + chatterShadow.setLinkUpstreamBandwidthKbps(3000); + setSignalStrength(chatter, -55); + assertNull(state.onPhysicalCapabilitiesChanged(CELL, chatter).getReason()); + } + + @Test + public void standbyLossIsTrackedAndStaleLossIsIgnored() { + PhysicalNetworkState state = new PhysicalNetworkState(); + state.onPhysicalAvailable(WIFI); + state.onPhysicalCapabilitiesChanged(WIFI, capabilities(NetworkCapabilities.TRANSPORT_WIFI)); + state.onPhysicalAvailable(CELL); + state.onPhysicalCapabilitiesChanged(CELL, capabilities(NetworkCapabilities.TRANSPORT_CELLULAR)); + + assertEquals(NetworkReloadPolicy.REASON_NETWORK_LOST, + state.onPhysicalLost(CELL).getReason()); + assertNull(state.onPhysicalLost(CELL).getReason()); + assertNotNull(state.getCapabilities(WIFI)); + } + + @Test + public void privateDnsOnlyChangeDoesNotRestartWireGuard() throws Exception { + PhysicalNetworkState state = new PhysicalNetworkState(); + state.onPhysicalAvailable(WIFI); + state.onPhysicalCapabilitiesChanged(WIFI, capabilities(NetworkCapabilities.TRANSPORT_WIFI)); + LinkProperties initial = linkProperties("9.9.9.9"); + state.onPhysicalLinkPropertiesChanged(WIFI, initial); + + LinkProperties pinned = linkProperties("9.9.9.9"); + setPrivateDns(pinned, "dns.example", true); + PhysicalNetworkState.Change change = state.onPhysicalLinkPropertiesChanged(WIFI, pinned); + assertEquals(NetworkReloadPolicy.REASON_PRIVATE_DNS_CHANGED, change.getReason()); + assertTrue(change.isPrivateDnsChanged()); + assertFalse(NetworkReloadPolicy.shouldRestartWireGuard(change.getReason())); + } + + @Test + public void vpnDefaultCallbacksDoNotCreatePhysicalDefault() throws Exception { + PhysicalNetworkState state = new PhysicalNetworkState(); + state.onPhysicalAvailable(WIFI); + state.onPhysicalCapabilitiesChanged(WIFI, capabilities(NetworkCapabilities.TRANSPORT_WIFI)); + state.onDefaultNetworkAvailable(WIFI); + state.onDefaultNetworkCapabilitiesChanged(WIFI, capabilities(NetworkCapabilities.TRANSPORT_WIFI)); + + NetworkCapabilities vpn = ShadowNetworkCapabilities.newInstance(); + ShadowNetworkCapabilities vpnShadow = Shadows.shadowOf(vpn); + vpnShadow.addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET); + vpnShadow.removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN); + vpnShadow.addTransportType(NetworkCapabilities.TRANSPORT_VPN); + assertNull(state.onDefaultNetworkAvailable(VPN).getReason()); + assertNull(state.onDefaultNetworkCapabilitiesChanged(VPN, vpn).getReason()); + assertNull(state.onDefaultNetworkLinkPropertiesChanged(VPN, + linkProperties("1.1.1.1")).getReason()); + assertNull(state.onDefaultNetworkLost(VPN).getReason()); + assertEquals(WIFI, state.getDefaultNetwork()); + } + + private static void setPrivateDns(LinkProperties properties, String name, boolean active) + throws Exception { + Field nameField = LinkProperties.class.getDeclaredField("mPrivateDnsServerName"); + nameField.setAccessible(true); + nameField.set(properties, name); + } + + private static void setSignalStrength(NetworkCapabilities capabilities, int strength) + throws Exception { + Field signalField = NetworkCapabilities.class.getDeclaredField("mSignalStrength"); + signalField.setAccessible(true); + signalField.setInt(capabilities, strength); + } +} diff --git a/app/src/test/java/net/kollnig/missioncontrol/wg/WgConnectivityCheckerTest.kt b/app/src/test/java/net/kollnig/missioncontrol/wg/WgConnectivityCheckerTest.kt index 0ae41130b..d038d5cc3 100644 --- a/app/src/test/java/net/kollnig/missioncontrol/wg/WgConnectivityCheckerTest.kt +++ b/app/src/test/java/net/kollnig/missioncontrol/wg/WgConnectivityCheckerTest.kt @@ -24,8 +24,9 @@ class WgConnectivityCheckerTest { tx: Long, freshHandshake: Boolean = false, tunFailuresTotal: Long = 0L, - tunFailuresStreak: Long = 0L - ) = WgStats(rx, tx, 0L, freshHandshake, tunFailuresTotal, tunFailuresStreak) + tunFailuresStreak: Long = 0L, + deliveredRxBytes: Long = rx + ) = WgStats(rx, tx, 0L, freshHandshake, tunFailuresTotal, tunFailuresStreak, deliveredRxBytes) // --- baseline / connecting ------------------------------------------ @@ -309,16 +310,24 @@ class WgConnectivityCheckerTest { c.tick(3000, stats(0, 20, freshHandshake = true)) assertEquals(false, c.lastTickSawRx) - // Return traffic arrives. - c.tick(4000, stats(50, 20)) + // Raw WireGuard rx bytes are not proof that decrypted traffic reached the TUN. + c.tick(4000, stats(50, 20, deliveredRxBytes = 0)) + assertEquals(false, c.lastTickSawRx) + + // A full TUN write advances the dedicated delivered counter. + c.tick(5000, stats(50, 20, deliveredRxBytes = 7)) assertEquals(true, c.lastTickSawRx) // Back to idle: the flag reflects the latest tick only. - c.tick(5000, stats(50, 20)) + c.tick(5500, stats(50, 20, deliveredRxBytes = 7)) + assertEquals(false, c.lastTickSawRx) + + // Raw rx and a fresh handshake alone still do not count. + c.tick(6000, stats(80, 30, freshHandshake = true, deliveredRxBytes = 7)) assertEquals(false, c.lastTickSawRx) - // rx advancing during a fresh-handshake tick still counts. - c.tick(6000, stats(80, 30, freshHandshake = true)) + // Delivered traffic during a fresh-handshake tick does count. + c.tick(7000, stats(80, 30, freshHandshake = true, deliveredRxBytes = 8)) assertEquals(true, c.lastTickSawRx) } diff --git a/app/src/test/java/net/kollnig/missioncontrol/wg/WgConnectivityMonitorTest.kt b/app/src/test/java/net/kollnig/missioncontrol/wg/WgConnectivityMonitorTest.kt index 9f6f5f43d..88c2c8b3a 100644 --- a/app/src/test/java/net/kollnig/missioncontrol/wg/WgConnectivityMonitorTest.kt +++ b/app/src/test/java/net/kollnig/missioncontrol/wg/WgConnectivityMonitorTest.kt @@ -38,6 +38,27 @@ class WgConnectivityMonitorTest { assertEquals(1_000L, WgConnectivityMonitor.pollIntervalMs(true)) } + @Test + fun sampleCallbackRunsThroughTheMonitorGenerationGate() { + var samples = 0 + var monitor: WgConnectivityMonitor? = null + monitor = WgConnectivityMonitor( + statsProvider = { stats() }, + prod = {}, + onBroken = { fail("sample callback test entered recovery") }, + onSample = { + samples++ + monitor!!.stop() + }, + sleep = {}, + clock = { 0L } + ) + + monitor.start() + awaitStopped(monitor) + assertEquals(1, samples) + } + @Test fun screenOffCadenceIsSlow() { val idle = WgConnectivityMonitor.pollIntervalMs(false) diff --git a/app/src/test/java/net/kollnig/missioncontrol/wg/WgHandoverVerifierTest.kt b/app/src/test/java/net/kollnig/missioncontrol/wg/WgHandoverVerifierTest.kt new file mode 100644 index 000000000..ff2692ac3 --- /dev/null +++ b/app/src/test/java/net/kollnig/missioncontrol/wg/WgHandoverVerifierTest.kt @@ -0,0 +1,190 @@ +package net.kollnig.missioncontrol.wg + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +class WgHandoverVerifierTest { + private val target = WgProbeTarget("10.0.0.2", "1.1.1.1") + + private fun stats(delivered: Long = 0L, token: Long = 0L) = WgStats( + rxBytes = 0L, + txBytes = 0L, + latestHandshakeMillis = 0L, + deliveredRxBytes = delivered, + probeReplyToken = token + ) + + @Test + fun startsWithProbeThenRetriesAtFiveSecondCadenceAndRestartsAtDeadline() { + val verifier = WgHandoverVerifier() + val first = verifier.begin(7, stats(10), listOf(target), true, now = 0L) + assertTrue(first is WgHandoverVerifier.Action.Probe) + assertTrue(verifier.onSample(7, stats(10), true, now = 4_999L) is WgHandoverVerifier.Action.None) + assertTrue(verifier.onSample(7, stats(10), true, now = 5_000L) is WgHandoverVerifier.Action.Probe) + assertTrue(verifier.onSample(7, stats(10), true, now = 10_000L) is WgHandoverVerifier.Action.Probe) + assertTrue(verifier.onSample(7, stats(10), true, now = 14_999L) is WgHandoverVerifier.Action.None) + assertTrue(verifier.onSample(7, stats(10), true, now = 15_000L) is WgHandoverVerifier.Action.Restart) + assertFalse(verifier.isPending()) + } + + @Test + fun deliveredTrafficConfirmsHandoverWithoutProbeReply() { + val verifier = WgHandoverVerifier() + verifier.begin(1, stats(10), listOf(target), true, now = 0L) + assertTrue(verifier.onSample(1, stats(11), true, now = 1_000L) is WgHandoverVerifier.Action.None) + assertFalse(verifier.isPending()) + } + + @Test + fun matchingProbeTokenConfirmsHandover() { + val verifier = WgHandoverVerifier() + val action = verifier.begin(2, stats(), listOf(target), true, now = 0L) + as WgHandoverVerifier.Action.Probe + assertTrue(verifier.onSample(2, stats(token = action.token), true, now = 1_000L) + is WgHandoverVerifier.Action.None) + assertFalse(verifier.isPending()) + } + + @Test + fun screenOffDefersAndRebasesVerification() { + val verifier = WgHandoverVerifier() + assertTrue(verifier.begin(3, stats(10), listOf(target), false, now = 0L) + is WgHandoverVerifier.Action.None) + assertTrue(verifier.onSample(3, stats(20), false, now = 20_000L) + is WgHandoverVerifier.Action.None) + assertTrue(verifier.onSample(3, stats(20), true, now = 21_000L) + is WgHandoverVerifier.Action.Probe) + assertTrue(verifier.onSample(3, stats(20), true, now = 26_000L) + is WgHandoverVerifier.Action.Probe) + assertTrue(verifier.onSample(3, stats(20), true, now = 31_000L) + is WgHandoverVerifier.Action.Probe) + assertTrue(verifier.onSample(3, stats(20), true, now = 35_999L) + is WgHandoverVerifier.Action.None) + assertTrue(verifier.onSample(3, stats(20), true, now = 36_000L) + is WgHandoverVerifier.Action.Restart) + } + + @Test + fun interactiveTransitionRebasesDeadline() { + val verifier = WgHandoverVerifier() + verifier.begin(8, stats(), listOf(target), true, now = 0L) + assertTrue(verifier.onSample(8, stats(), true, now = 1_000L) + is WgHandoverVerifier.Action.None) + + verifier.setInteractive(false) + // No monitor poll occurs while off: the explicit event must retain + // the suspension even after the screen comes back on. + verifier.setInteractive(true) + assertTrue(verifier.onSample(8, stats(), true, now = 11_000L) + is WgHandoverVerifier.Action.Probe) + assertTrue(verifier.onSample(8, stats(), true, now = 16_000L) + is WgHandoverVerifier.Action.Probe) + assertTrue(verifier.onSample(8, stats(), true, now = 21_000L) + is WgHandoverVerifier.Action.Probe) + assertTrue(verifier.onSample(8, stats(), true, now = 25_999L) + is WgHandoverVerifier.Action.None) + assertTrue(verifier.onSample(8, stats(), true, now = 26_000L) + is WgHandoverVerifier.Action.Restart) + } + + @Test + fun suspendedStateRebasesDeadlineOnResume() { + val verifier = WgHandoverVerifier() + verifier.begin(9, stats(), listOf(target), true, now = 0L) + assertTrue(verifier.onSample(9, stats(), true, now = 0L) + is WgHandoverVerifier.Action.None) + + verifier.onSuspended() + assertTrue(verifier.onSample(9, stats(), true, now = 100_000L) + is WgHandoverVerifier.Action.Probe) + assertTrue(verifier.onSample(9, stats(), true, now = 105_000L) + is WgHandoverVerifier.Action.Probe) + assertTrue(verifier.onSample(9, stats(), true, now = 110_000L) + is WgHandoverVerifier.Action.Probe) + assertTrue(verifier.onSample(9, stats(), true, now = 114_999L) + is WgHandoverVerifier.Action.None) + assertTrue(verifier.onSample(9, stats(), true, now = 115_000L) + is WgHandoverVerifier.Action.Restart) + } + + @Test + fun newBeginInvalidatesQueuedReplyFromPreviousGeneration() { + val verifier = WgHandoverVerifier() + val oldProbe = verifier.begin(10, stats(), listOf(target), true, now = 0L) + as WgHandoverVerifier.Action.Probe + val newProbe = verifier.begin(11, stats(), listOf(target), true, now = 100L) + as WgHandoverVerifier.Action.Probe + + assertFalse(oldProbe.token == newProbe.token) + assertTrue(verifier.onSample(11, stats(token = oldProbe.token), true, now = 101L) + is WgHandoverVerifier.Action.None) + assertTrue(verifier.isPending()) + assertTrue(verifier.onSample(11, stats(token = newProbe.token), true, now = 102L) + is WgHandoverVerifier.Action.None) + assertFalse(verifier.isPending()) + } + + @Test + fun staleGenerationCannotConfirmOrRestart() { + val verifier = WgHandoverVerifier() + verifier.begin(4, stats(), listOf(target), true, now = 0L) + assertTrue(verifier.onSample(5, stats(100), true, now = 20_000L) + is WgHandoverVerifier.Action.None) + assertTrue(verifier.isPending()) + } + + @Test + fun unsupportedTargetsLeavePassiveWatchdogAlone() { + val verifier = WgHandoverVerifier() + assertTrue(verifier.begin(6, stats(), emptyList(), true, now = 0L) + is WgHandoverVerifier.Action.None) + assertFalse(verifier.isPending()) + } + + @Test + fun selectorRejectsMixedFamilyAndUnroutedResolvers() { + val targets = WgProbeTargetSelector.select( + sourceAddresses = listOf("10.0.0.2/32", "2001:db8::2/128"), + resolverAddresses = listOf("1.1.1.1", "2001:4860:4860::8888", "resolver.example"), + allowedIps = listOf("0.0.0.0/0", "2001::/16") + ) + assertEquals( + listOf( + WgProbeTarget("10.0.0.2", "1.1.1.1"), + WgProbeTarget("2001:db8::2", "2001:4860:4860::8888") + ), + targets + ) + } + + @Test + fun selectorRejectsResolverOutsideAllowedIps() { + assertTrue( + WgProbeTargetSelector.select( + listOf("10.0.0.2/32"), + listOf("1.1.1.1"), + listOf("10.0.0.0/8") + ).isEmpty() + ) + } + + @Test + fun selectorRejectsSpecialDestinationsAndMalformedRoutes() { + assertTrue( + WgProbeTargetSelector.select( + listOf("0.0.0.0/32"), + listOf("224.0.0.1"), + listOf("0.0.0.0/0") + ).isEmpty() + ) + assertTrue( + WgProbeTargetSelector.select( + listOf("127.0.0.1/32"), + listOf("1.1.1.1"), + listOf("0.0.0.0/not-a-prefix") + ).isEmpty() + ) + } +} diff --git a/wgbridge-rs/README.md b/wgbridge-rs/README.md index 175ed2b89..b9b266a07 100644 --- a/wgbridge-rs/README.md +++ b/wgbridge-rs/README.md @@ -188,6 +188,7 @@ class Tunnel { long latestHandshakeMillis(); void sendKeepalive(); void rebind(); // re-bind + re-protect UDP sockets (roaming) + boolean sendDnsProbe(String sourceIp, String resolverIp, long token); void updateEndpoint(String peerPublicKeyBase64, String endpoint); void setKeepalive(String peerPublicKeyBase64, int seconds); // 0 disables void stop(); @@ -199,6 +200,24 @@ class Tunnel { endpoints must arrive as resolved IP literals (`WgEgress` resolves hostnames, and re-resolves them on network changes via `updateEndpoint`). +`TunnelStats.rxBytes` is the engine counter and includes handshakes. +`deliveredRxBytes` counts only decrypted IP packets successfully written to +Android; use it for data-path recovery evidence. `probeReplyToken` identifies +the last correlated internal DNS probe reply. Probes enter the same encrypted +IP transport as outbound application packets. Their sockets only reserve a +source port and never send on the physical network. Matching replies are +consumed before DNS policy and TUN delivery, so they do not inflate the +delivered-IP counter. Probe destinations must be covered by a peer's AllowedIPs. + +After an underlying-network change, Android first rebinds the protected UDP +sockets and refreshes endpoints. While interactive, it then verifies the path +with up to three root-NS DNS queries at five-second intervals. Delivered +application traffic or a correlated reply ends verification; fifteen seconds +without either requests a full restart through the existing backoff, followed +by verification of the replacement. Screen-off and suspend gaps defer/rebase +the check. Profiles without a same-family, routed numeric resolver retain the +ordinary watchdog. No probe opens a direct fallback path. + ## Potential improvements - **Split DNS-over-TCP rewriting**: inbound TCP DNS is recorded only when the diff --git a/wgbridge-rs/src/jni_bindings.rs b/wgbridge-rs/src/jni_bindings.rs index 53f7cbcaa..14339a1c1 100644 --- a/wgbridge-rs/src/jni_bindings.rs +++ b/wgbridge-rs/src/jni_bindings.rs @@ -336,6 +336,8 @@ pub extern "system" fn Java_net_kollnig_missioncontrol_wgbridge_Tunnel_nativeSta stats.latest_handshake_millis, stats.tun_write_failures_total, stats.tun_write_failures_streak, + stats.delivered_rx_bytes, + stats.probe_reply_token, ]; match env.new_long_array(values.len()) { Ok(array) => { @@ -371,6 +373,30 @@ pub extern "system" fn Java_net_kollnig_missioncontrol_wgbridge_Tunnel_nativeSen }) } +#[no_mangle] +pub extern "system" fn Java_net_kollnig_missioncontrol_wgbridge_Tunnel_nativeSendDnsProbe( + mut unowned_env: EnvUnowned, + _class: JClass, + handle: jlong, + source: JString, + resolver: JString, + token: jlong, +) -> jni::sys::jboolean { + with_native_env!(unowned_env, env, { + let Some(tunnel) = tunnel_from_handle(handle) else { return false; }; + let (Some(source), Some(resolver)) = (get_string(env, &source), get_string(env, &resolver)) else { + return false; + }; + match tunnel.send_dns_probe(&source, &resolver, token) { + Ok(sent) => sent, + Err(error) => { + log::warn!("could not queue WireGuard DNS probe: {error}"); + false + } + } + }) +} + #[no_mangle] pub extern "system" fn Java_net_kollnig_missioncontrol_wgbridge_Tunnel_nativeRebind( mut unowned_env: EnvUnowned, diff --git a/wgbridge-rs/src/lib.rs b/wgbridge-rs/src/lib.rs index 531820e1a..02aa49928 100644 --- a/wgbridge-rs/src/lib.rs +++ b/wgbridge-rs/src/lib.rs @@ -21,6 +21,7 @@ pub mod config; pub mod dns; pub mod keys; pub mod policy; +pub mod probe; // The C ABI the NetGuard engine links against. It lives in this crate, not // in tc-dns, so the cdylib itself defines the exported symbols rather than // inheriting them from an rlib dependency (see app/gradle/wgbridge.gradle). diff --git a/wgbridge-rs/src/probe.rs b/wgbridge-rs/src/probe.rs new file mode 100644 index 000000000..a07d1a901 --- /dev/null +++ b/wgbridge-rs/src/probe.rs @@ -0,0 +1,329 @@ +//! Bounded, in-tunnel DNS reachability probes. No socket sends bypass the VPN. +//! The UDP socket below only reserves a source port; the packet enters gotatun +//! through IpRecv and its correlated reply is consumed before reaching Android. + +use std::io; +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr, UdpSocket}; +use std::sync::atomic::{AtomicBool, AtomicI64, Ordering}; +use std::sync::Mutex; +use std::time::{Duration, Instant}; + +const QUESTION: [u8; 5] = [0, 0, 2, 0, 1]; // root NS IN; no user hostname +pub const PROBE_LIFETIME: Duration = Duration::from_secs(5); + +struct Pending { + source: IpAddr, + resolver: IpAddr, + port: u16, + id: [u8; 2], + token: i64, + expires: Instant, + _reservation: UdpSocket, +} + +#[derive(Default)] +pub struct ProbeTracker { + active: AtomicBool, + pending: Mutex>, + reply_token: AtomicI64, +} + +impl ProbeTracker { + pub fn reply_token(&self) -> i64 { + self.reply_token.load(Ordering::Acquire) + } + + pub fn prepare(&self, source: IpAddr, resolver: IpAddr, token: i64) -> io::Result> { + if source.is_ipv4() != resolver.is_ipv4() + || source.is_unspecified() + || source.is_multicast() + || resolver.is_unspecified() + || resolver.is_multicast() + || resolver.is_loopback() + || token <= 0 + { + return Err(io::Error::other("invalid DNS probe addresses or token")); + } + let bind_ip = match source { + IpAddr::V4(_) => IpAddr::V4(Ipv4Addr::UNSPECIFIED), + IpAddr::V6(_) => IpAddr::V6(Ipv6Addr::UNSPECIFIED), + }; + let reservation = UdpSocket::bind(SocketAddr::new(bind_ip, 0))?; + let port = reservation.local_addr()?.port(); + let mut id = [0; 2]; + getrandom::fill(&mut id).map_err(io::Error::other)?; + let packet = dns_packet(source, resolver, port, id); + let mut pending = self + .pending + .lock() + .map_err(|_| io::Error::other("probe lock poisoned"))?; + *pending = Some(Pending { + source, + resolver, + port, + id, + token, + expires: Instant::now() + PROBE_LIFETIME, + _reservation: reservation, + }); + self.active.store(true, Ordering::Release); + Ok(packet) + } + + pub fn expire(&self) { + if let Ok(mut pending) = self.pending.lock() { + if pending + .as_ref() + .is_some_and(|p| Instant::now() >= p.expires) + { + *pending = None; + self.active.store(false, Ordering::Release); + } + } + } + + /// Returns true only for our current, unexpired DNS transaction. Ordinary + /// application packets take one atomic load when no probe is outstanding. + pub fn consume_reply(&self, packet: &[u8]) -> bool { + if !self.active.load(Ordering::Acquire) { + return false; + } + let Ok(mut pending) = self.pending.lock() else { + return false; + }; + let Some(probe) = pending.as_ref() else { + return false; + }; + if Instant::now() >= probe.expires { + *pending = None; + self.active.store(false, Ordering::Release); + return false; + } + let Some((source, dest, udp)) = udp_payload(packet) else { + return false; + }; + if source != probe.resolver + || dest != probe.source + || udp.len() < 25 + || udp[0..2] != 53u16.to_be_bytes() + || udp[2..4] != probe.port.to_be_bytes() + || usize::from(u16::from_be_bytes([udp[4], udp[5]])) != udp.len() + { + return false; + } + let dns = &udp[8..]; + if dns[0..2] != probe.id + || dns[2] & 0xf8 != 0x80 + || dns[4..6] != [0, 1] + || dns[12..17] != QUESTION + { + return false; + } + // Any DNS rcode proves a round trip; this is not a resolver health test. + self.reply_token.store(probe.token, Ordering::Release); + *pending = None; + self.active.store(false, Ordering::Release); + true + } +} + +fn udp_payload(packet: &[u8]) -> Option<(IpAddr, IpAddr, &[u8])> { + match packet.first()? >> 4 { + 4 if packet.len() >= 20 => { + let header = usize::from(packet[0] & 15) * 4; + let len = usize::from(u16::from_be_bytes([packet[2], packet[3]])); + if header < 20 + || len != packet.len() + || header > len + || packet[9] != 17 + || u16::from_be_bytes([packet[6], packet[7]]) & 0x3fff != 0 + { + return None; + } + Some(( + Ipv4Addr::new(packet[12], packet[13], packet[14], packet[15]).into(), + Ipv4Addr::new(packet[16], packet[17], packet[18], packet[19]).into(), + &packet[header..], + )) + } + 6 if packet.len() >= 40 => { + if packet[6] != 17 + || usize::from(u16::from_be_bytes([packet[4], packet[5]])) + 40 != packet.len() + { + return None; + } + let source: [u8; 16] = packet[8..24].try_into().ok()?; + let dest: [u8; 16] = packet[24..40].try_into().ok()?; + Some(( + Ipv6Addr::from(source).into(), + Ipv6Addr::from(dest).into(), + &packet[40..], + )) + } + _ => None, + } +} + +fn checksum(bytes: &[u8]) -> u16 { + let sum = bytes.chunks(2).fold(0u32, |sum, pair| { + sum + (u32::from(pair[0]) << 8) + u32::from(*pair.get(1).unwrap_or(&0)) + }); + let sum = (sum & 0xffff) + (sum >> 16); + !((sum & 0xffff) + (sum >> 16)) as u16 +} + +fn dns_packet(source: IpAddr, resolver: IpAddr, port: u16, id: [u8; 2]) -> Vec { + let mut udp = vec![0u8; 25]; + udp[0..2].copy_from_slice(&port.to_be_bytes()); + udp[2..4].copy_from_slice(&53u16.to_be_bytes()); + udp[4..6].copy_from_slice(&25u16.to_be_bytes()); + udp[8..10].copy_from_slice(&id); + udp[10] = 1; // recursion desired + udp[13] = 1; // one question + udp[20..25].copy_from_slice(&QUESTION); + let mut pseudo = Vec::new(); + let mut ip = match (source, resolver) { + (IpAddr::V4(src), IpAddr::V4(dst)) => { + let mut ip = vec![0u8; 20]; + ip[0] = 0x45; + ip[2..4].copy_from_slice(&45u16.to_be_bytes()); + ip[8] = 64; + ip[9] = 17; + ip[12..16].copy_from_slice(&src.octets()); + ip[16..20].copy_from_slice(&dst.octets()); + let check = checksum(&ip); + ip[10..12].copy_from_slice(&check.to_be_bytes()); + pseudo.extend_from_slice(&src.octets()); + pseudo.extend_from_slice(&dst.octets()); + pseudo.extend_from_slice(&[0, 17, 0, 25]); + ip + } + (IpAddr::V6(src), IpAddr::V6(dst)) => { + let mut ip = vec![0u8; 40]; + ip[0] = 0x60; + ip[4..6].copy_from_slice(&25u16.to_be_bytes()); + ip[6] = 17; + ip[7] = 64; + ip[8..24].copy_from_slice(&src.octets()); + ip[24..40].copy_from_slice(&dst.octets()); + pseudo.extend_from_slice(&src.octets()); + pseudo.extend_from_slice(&dst.octets()); + pseudo.extend_from_slice(&[0, 0, 0, 25, 0, 0, 0, 17]); + ip + } + _ => return Vec::new(), // prepare rejects mixed families + }; + pseudo.extend_from_slice(&udp); + let check = checksum(&pseudo); + udp[6..8].copy_from_slice(&(if check == 0 { 0xffff } else { check }).to_be_bytes()); + ip.extend_from_slice(&udp); + ip +} + +#[cfg(test)] +pub(crate) fn test_reply(query: &[u8]) -> Vec { + let mut reply = query.to_vec(); + let header = if query[0] >> 4 == 4 { 20 } else { 40 }; + if header == 20 { + reply[12..16].copy_from_slice(&query[16..20]); + reply[16..20].copy_from_slice(&query[12..16]); + } else { + reply[8..24].copy_from_slice(&query[24..40]); + reply[24..40].copy_from_slice(&query[8..24]); + } + reply[header..header + 2].copy_from_slice(&query[header + 2..header + 4]); + reply[header + 2..header + 4].copy_from_slice(&query[header..header + 2]); + reply[header + 10] |= 0x80; + reply +} + +#[cfg(test)] +mod tests { + use super::*; + + fn addresses(v6: bool) -> (IpAddr, IpAddr) { + if v6 { + ( + "2001:db8::2".parse().unwrap(), + "2001:db8::53".parse().unwrap(), + ) + } else { + ("10.0.0.2".parse().unwrap(), "10.0.0.53".parse().unwrap()) + } + } + + #[test] + fn queries_have_valid_ip_and_udp_checksums_in_both_families() { + for v6 in [false, true] { + let (source, resolver) = addresses(v6); + let packet = dns_packet(source, resolver, 40000, [0x12, 0x34]); + let header = if v6 { 40 } else { 20 }; + let mut pseudo = Vec::new(); + if v6 { + pseudo.extend_from_slice(&packet[8..40]); + pseudo.extend_from_slice(&[0, 0, 0, 25, 0, 0, 0, 17]); + } else { + assert_eq!(checksum(&packet[..20]), 0); + pseudo.extend_from_slice(&packet[12..20]); + pseudo.extend_from_slice(&[0, 17, 0, 25]); + } + pseudo.extend_from_slice(&packet[header..]); + assert_eq!(checksum(&pseudo), 0); + assert_eq!(&packet[header + 20..], &QUESTION); + assert_eq!(&packet[header + 8..header + 10], &[0x12, 0x34]); + } + } + + #[test] + fn only_current_correlated_reply_is_consumed_once() { + for v6 in [false, true] { + let tracker = ProbeTracker::default(); + let (source, resolver) = addresses(v6); + let query = tracker.prepare(source, resolver, 11).unwrap(); + assert!(!tracker.consume_reply(&query)); + let reply = test_reply(&query); + let header = if v6 { 40 } else { 20 }; + for index in [header, header + 2, header + 4, header + 8, header + 20] { + let mut malformed = reply.clone(); + malformed[index] ^= 1; + assert!(!tracker.consume_reply(&malformed)); + } + for len in 0..reply.len() { + assert!(!tracker.consume_reply(&reply[..len])); + } + assert_eq!(tracker.reply_token(), 0); + assert!(tracker.consume_reply(&reply)); + assert_eq!(tracker.reply_token(), 11); + assert!(!tracker.consume_reply(&reply)); + } + } + + #[test] + fn replaced_and_expired_probes_cannot_confirm_new_attempts() { + let tracker = ProbeTracker::default(); + let (source, resolver) = addresses(false); + let old = tracker.prepare(source, resolver, 1).unwrap(); + let new = tracker.prepare(source, resolver, 2).unwrap(); + assert!(!tracker.consume_reply(&test_reply(&old))); + tracker.pending.lock().unwrap().as_mut().unwrap().expires = Instant::now(); + assert!(!tracker.consume_reply(&test_reply(&new))); + assert_eq!(tracker.reply_token(), 0); + assert!(!tracker.active.load(Ordering::Acquire)); + tracker.prepare(source, resolver, 3).unwrap(); + tracker.pending.lock().unwrap().as_mut().unwrap().expires = Instant::now(); + tracker.expire(); + assert!(tracker.pending.lock().unwrap().is_none()); + } + + #[test] + fn invalid_probe_addresses_are_rejected() { + let (source, resolver) = addresses(false); + let tracker = ProbeTracker::default(); + for invalid in ["::1", "0.0.0.0", "127.0.0.1", "224.0.0.1"] { + assert!(tracker + .prepare(source, invalid.parse().unwrap(), 1) + .is_err()); + } + assert!(tracker.prepare(source, resolver, 0).is_err()); + } +} diff --git a/wgbridge-rs/src/transport/ip_recv.rs b/wgbridge-rs/src/transport/ip_recv.rs index a83d2e154..63ef785b7 100644 --- a/wgbridge-rs/src/transport/ip_recv.rs +++ b/wgbridge-rs/src/transport/ip_recv.rs @@ -16,16 +16,23 @@ const MAX_BATCH: usize = 32; pub struct SocketpairRecv { afd: AsyncFd, mtu: MtuWatcher, + probes: tokio::sync::mpsc::Receiver>, } impl SocketpairRecv { /// Takes ownership of `fd` (already a private dup). Sets it non-blocking /// for use with the tokio reactor. pub fn new(fd: OwnedFd, mtu: u16) -> io::Result { + let (_, probes) = tokio::sync::mpsc::channel(1); + Self::with_probes(fd, mtu, probes) + } + + pub fn with_probes(fd: OwnedFd, mtu: u16, probes: tokio::sync::mpsc::Receiver>) -> io::Result { set_nonblocking(&fd)?; Ok(Self { afd: AsyncFd::with_interest(fd, Interest::READABLE)?, mtu: MtuWatcher::new(mtu), + probes, }) } } @@ -55,7 +62,15 @@ impl IpRecv for SocketpairRecv { pool: &mut PacketBufPool, ) -> io::Result> + Send + 'a> { loop { - let mut guard = self.afd.readable().await?; + let mut guard = tokio::select! { + Some(bytes) = self.probes.recv() => { + if let Ok(packet) = Packet::copy_from(bytes.as_slice()).try_into_ip() { + return Ok(vec![packet].into_iter()); + } + continue; + } + ready = self.afd.readable() => ready?, + }; let fd = self.afd.get_ref().as_raw_fd(); let mut packets: Vec> = Vec::new(); @@ -109,3 +124,32 @@ impl IpRecv for SocketpairRecv { self.mtu.clone() } } + +#[cfg(test)] +mod tests { + use super::*; + use std::os::unix::net::UnixDatagram; + use std::time::Duration; + + #[tokio::test] + async fn probes_and_normal_packets_share_recv_without_channel_close_stopping_it() { + let (writer, reader) = UnixDatagram::pair().unwrap(); + let (tx, rx) = tokio::sync::mpsc::channel(1); + let mut recv = SocketpairRecv::with_probes(reader.into(), 1280, rx).unwrap(); + let tracker = crate::probe::ProbeTracker::default(); + let query = tracker.prepare("10.0.0.2".parse().unwrap(), "10.0.0.53".parse().unwrap(), 1).unwrap(); + tx.send(query.clone()).await.unwrap(); + let mut pool = PacketBufPool::new(2); + { + let mut packets = tokio::time::timeout(Duration::from_secs(1), recv.recv(&mut pool)).await.unwrap().unwrap(); + let packet: Packet<[u8]> = packets.next().unwrap().into(); + assert_eq!(packet.as_ref(), query.as_slice()); + assert!(packets.next().is_none()); + } + drop(tx); + writer.send(&query).unwrap(); + let mut packets = tokio::time::timeout(Duration::from_secs(1), recv.recv(&mut pool)).await.unwrap().unwrap(); + let packet: Packet<[u8]> = packets.next().unwrap().into(); + assert_eq!(packet.as_ref(), query.as_slice()); + } +} diff --git a/wgbridge-rs/src/transport/ip_send.rs b/wgbridge-rs/src/transport/ip_send.rs index 6a1d9caab..3008ec8ef 100644 --- a/wgbridge-rs/src/transport/ip_send.rs +++ b/wgbridge-rs/src/transport/ip_send.rs @@ -18,6 +18,8 @@ pub struct TunFdSend { dns_inspector: DnsInspector, write_failures_total: Arc, write_failures_streak: Arc, + delivered_bytes: Arc, + probes: Option>, } impl TunFdSend { @@ -45,8 +47,20 @@ impl TunFdSend { dns_inspector: DnsInspector::default(), write_failures_total, write_failures_streak, + delivered_bytes: Arc::new(AtomicU64::new(0)), + probes: None, } } + + pub fn with_delivered_counter(mut self, counter: Arc) -> Self { + self.delivered_bytes = counter; + self + } + + pub fn with_probes(mut self, probes: Arc) -> Self { + self.probes = Some(probes); + self + } } fn write_fd(fd: i32, buf: &[u8]) -> isize { @@ -68,6 +82,9 @@ fn record_tun_write(total: &AtomicU64, streak: &AtomicU64, full_write: bool) -> impl IpSend for TunFdSend { async fn send(&mut self, packet: Packet) -> io::Result<()> { let mut packet: Packet<[u8]> = packet.into(); + if self.probes.as_ref().is_some_and(|p| p.consume_reply(packet.as_ref())) { + return Ok(()); + } if let Some(dns) = &self.dns { // The inspector records A/AAAA mappings before it blanks @@ -83,6 +100,9 @@ impl IpSend for TunFdSend { let data = packet.as_ref(); let n = write_fd(self.fd.as_raw_fd(), data); + if n == data.len() as isize { + self.delivered_bytes.fetch_add(data.len() as u64, Ordering::Relaxed); + } let (errors, streak) = record_tun_write( &self.write_failures_total, &self.write_failures_streak, @@ -154,18 +174,22 @@ mod tests { let writer = unsafe { OwnedFd::from_raw_fd(fds[1]) }; let total = Arc::new(AtomicU64::new(7)); let streak = Arc::new(AtomicU64::new(3)); + let delivered = Arc::new(AtomicU64::new(0)); let mut sender = TunFdSend::with_counters( writer, None, Arc::clone(&total), Arc::clone(&streak), ); + sender = sender.with_delivered_counter(Arc::clone(&delivered)); let (packet, expected) = minimal_ipv4_packet(); assert!(sender.send(packet).await.is_ok()); assert_eq!(total.load(Ordering::Relaxed), 7); assert_eq!(streak.load(Ordering::Relaxed), 0); + assert_eq!(delivered.load(Ordering::Relaxed), expected.len() as u64); + let reader = std::os::unix::net::UnixDatagram::from(reader); reader.set_read_timeout(Some(std::time::Duration::from_secs(1))).unwrap(); let mut received = [0; 20]; @@ -182,6 +206,7 @@ mod tests { let fd: OwnedFd = file.into(); let total = Arc::new(AtomicU64::new(5)); let streak = Arc::new(AtomicU64::new(2)); + let delivered = Arc::new(AtomicU64::new(0)); let mut sender = TunFdSend::with_counters( fd, None, @@ -189,6 +214,7 @@ mod tests { Arc::clone(&streak), ); + sender = sender.with_delivered_counter(Arc::clone(&delivered)); for _ in 0..3 { let (packet, _) = minimal_ipv4_packet(); assert!(sender.send(packet).await.is_ok()); @@ -196,5 +222,24 @@ mod tests { assert_eq!(total.load(Ordering::Relaxed), 8); assert_eq!(streak.load(Ordering::Relaxed), 5); + assert_eq!(delivered.load(Ordering::Relaxed), 0); + } + + #[tokio::test] + async fn probe_reply_is_consumed_without_tun_write_or_delivery_credit() { + let probes = Arc::new(crate::probe::ProbeTracker::default()); + let query = probes.prepare("10.0.0.2".parse().unwrap(), "10.0.0.53".parse().unwrap(), 42).unwrap(); + let reply = crate::probe::test_reply(&query); + // A write would fail on this read-only fd, making accidental delivery + // observable through the failure counter as well as delivered bytes. + let fd = OpenOptions::new().read(true).open("/dev/null").unwrap().into(); + let failures = Arc::new(AtomicU64::new(0)); + let delivered = Arc::new(AtomicU64::new(0)); + let mut sender = TunFdSend::with_counters(fd, None, Arc::clone(&failures), Arc::new(AtomicU64::new(0))) + .with_delivered_counter(Arc::clone(&delivered)).with_probes(Arc::clone(&probes)); + sender.send(Packet::copy_from(reply.as_slice()).try_into_ip().unwrap()).await.unwrap(); + assert_eq!(probes.reply_token(), 42); + assert_eq!(failures.load(Ordering::Relaxed), 0); + assert_eq!(delivered.load(Ordering::Relaxed), 0); } } diff --git a/wgbridge-rs/src/tunnel.rs b/wgbridge-rs/src/tunnel.rs index cb4e4592f..97642a130 100644 --- a/wgbridge-rs/src/tunnel.rs +++ b/wgbridge-rs/src/tunnel.rs @@ -27,6 +27,8 @@ pub struct TunnelStats { pub latest_handshake_millis: i64, pub tun_write_failures_total: i64, pub tun_write_failures_streak: i64, + pub delivered_rx_bytes: i64, + pub probe_reply_token: i64, } struct Inner { @@ -42,6 +44,9 @@ struct Inner { logger: Arc, tun_write_failures_total: Arc, tun_write_failures_streak: Arc, + delivered_rx_bytes: Arc, + probes: Arc, + probe_sender: tokio::sync::mpsc::Sender>, } pub struct Tunnel { @@ -95,6 +100,9 @@ pub fn start_tunnel( let tun_write_failures_streak = Arc::new(std::sync::atomic::AtomicU64::new(0)); let stats_total = Arc::clone(&tun_write_failures_total); let stats_streak = Arc::clone(&tun_write_failures_streak); + let delivered_rx_bytes = Arc::new(std::sync::atomic::AtomicU64::new(0)); + let probes = Arc::new(crate::probe::ProbeTracker::default()); + let (probe_sender, probe_receiver) = tokio::sync::mpsc::channel(1); let runtime = tokio::runtime::Builder::new_multi_thread() .worker_threads(2) @@ -106,13 +114,14 @@ pub fn start_tunnel( let device = runtime .block_on(async { - let ip_recv = SocketpairRecv::new(rx_fd, mtu)?; + let ip_recv = SocketpairRecv::with_probes(rx_fd, mtu, probe_receiver)?; let ip_send = TunFdSend::with_counters( tx_fd, dns, Arc::clone(&tun_write_failures_total), Arc::clone(&tun_write_failures_streak), - ); + ).with_delivered_counter(Arc::clone(&delivered_rx_bytes)) + .with_probes(Arc::clone(&probes)); gotatun::device::build() .with_udp(ProtectedUdpFactory::new(protector)) .with_ip_pair(ip_send, ip_recv) @@ -138,11 +147,35 @@ pub fn start_tunnel( logger, tun_write_failures_total: stats_total, tun_write_failures_streak: stats_streak, + delivered_rx_bytes, + probes, + probe_sender, }), }) } impl Tunnel { + /// Queue one DNS probe on the encrypted IP transport. Returning true means + /// queued, not healthy; only probe_reply_token confirms a correlated reply. + pub fn send_dns_probe(&self, source: &str, resolver: &str, token: i64) -> Result { + let source: std::net::IpAddr = source.parse().map_err(|_| "invalid probe source")?; + let resolver: std::net::IpAddr = resolver.parse().map_err(|_| "invalid probe resolver")?; + let peers = self.inner.peers.lock().map_err(|_| "peer lock poisoned")?; + if !peers.iter().any(|peer| peer.allowed_ips.iter().any(|net| net.contains(resolver))) { + return Ok(false); + } + drop(peers); + let Ok(permit) = self.inner.probe_sender.try_reserve() else { return Ok(false); }; + let packet = self.inner.probes.prepare(source, resolver, token).map_err(|e| e.to_string())?; + permit.send(packet); + let probes = Arc::clone(&self.inner.probes); + self.runtime.spawn(async move { + tokio::time::sleep(crate::probe::PROBE_LIFETIME).await; + probes.expire(); + }); + Ok(true) + } + /// Reapplies UAPI configuration to the running device without restarting /// it. The screen-state keepalive toggle goes through [`Tunnel::set_keepalive`] /// instead, which touches one field per peer. @@ -181,9 +214,8 @@ impl Tunnel { } /// Transfer counters and newest handshake, summed across all peers. - /// rx_bytes counts decrypted transport payload, so it only advances when - /// the tunnel actually carries return traffic — that is the liveness - /// signal the connectivity monitor is biased toward. + /// Engine rx_bytes includes handshake traffic. delivered_rx_bytes counts + /// only complete decrypted IP packets successfully written to Android. pub fn stats(&self) -> Result { let inner = Arc::clone(&self.inner); self.runtime.block_on(async move { @@ -199,6 +231,9 @@ impl Tunnel { rx_bytes: 0, tx_bytes: 0, latest_handshake_millis: 0, + probe_reply_token: inner.probes.reply_token(), + delivered_rx_bytes: inner.delivered_rx_bytes.load( + std::sync::atomic::Ordering::Relaxed) as i64, tun_write_failures_total: inner .tun_write_failures_total .load(std::sync::atomic::Ordering::Relaxed) From 3f4a2f2bd10dbfa9e7b945b0e0c057ffc4446e56 Mon Sep 17 00:00:00 2001 From: Konrad Kollnig <5175206+kasnder@users.noreply.github.com> Date: Fri, 11 Sep 2026 18:27:34 +0200 Subject: [PATCH 2/4] Narrow handover fix to debounced tunnel recreation --- .../netguard/PhysicalNetworkState.java | 397 +++++----------- .../eu/faircode/netguard/ServiceSinkhole.java | 17 +- .../wg/WgConnectivityMonitor.kt | 22 +- .../net/kollnig/missioncontrol/wg/WgEgress.kt | 427 +++++------------- .../missioncontrol/wg/WgHandoverVerifier.kt | 208 --------- .../missioncontrol/wgbridge/Tunnel.java | 16 +- .../missioncontrol/wgbridge/TunnelStats.java | 22 - .../netguard/PhysicalNetworkStateTest.java | 54 +-- .../wg/WgConnectivityCheckerTest.kt | 23 +- .../wg/WgConnectivityMonitorTest.kt | 21 - .../wg/WgEgressRecoveryTest.java | 89 ++++ .../wg/WgHandoverVerifierTest.kt | 190 -------- wgbridge-rs/README.md | 19 - wgbridge-rs/src/jni_bindings.rs | 26 -- wgbridge-rs/src/lib.rs | 1 - wgbridge-rs/src/probe.rs | 329 -------------- wgbridge-rs/src/transport/ip_recv.rs | 46 +- wgbridge-rs/src/transport/ip_send.rs | 45 -- wgbridge-rs/src/tunnel.rs | 45 +- 19 files changed, 358 insertions(+), 1639 deletions(-) delete mode 100644 app/src/main/java/net/kollnig/missioncontrol/wg/WgHandoverVerifier.kt delete mode 100644 app/src/test/java/net/kollnig/missioncontrol/wg/WgHandoverVerifierTest.kt delete mode 100644 wgbridge-rs/src/probe.rs diff --git a/app/src/main/java/eu/faircode/netguard/PhysicalNetworkState.java b/app/src/main/java/eu/faircode/netguard/PhysicalNetworkState.java index 17fec3037..0b3c999fe 100644 --- a/app/src/main/java/eu/faircode/netguard/PhysicalNetworkState.java +++ b/app/src/main/java/eu/faircode/netguard/PhysicalNetworkState.java @@ -1,13 +1,10 @@ package eu.faircode.netguard; -import android.net.LinkAddress; import android.net.LinkProperties; import android.net.Network; import android.net.NetworkCapabilities; import android.os.Build; -import android.os.Parcel; -import java.net.InetAddress; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -16,326 +13,150 @@ import java.util.Map; import java.util.Objects; -/** - * Callback-owned state for physical networks. Connectivity callbacks can be - * reordered and do not make synchronous ConnectivityManager lookups safe, so - * the callback snapshots are the source of truth until the next callback. - */ +/** Callback-owned snapshots: never query ConnectivityManager from its callbacks. */ final class PhysicalNetworkState { - static final class Change { - private final String reason; - private final boolean privateDnsChanged; - - private Change(String reason, boolean privateDnsChanged) { - this.reason = reason; - this.privateDnsChanged = privateDnsChanged; - } - - static Change none() { - return new Change(null, false); - } - - static Change of(String reason, boolean privateDnsChanged) { - return new Change(reason, privateDnsChanged); - } - - String getReason() { - return reason; - } - - boolean isPrivateDnsChanged() { - return privateDnsChanged; - } - } - private static final class Entry { - NetworkCapabilities capabilities; - LinkProperties linkProperties; - Fingerprint fingerprint; + List capabilities; + List links; String privateDns; boolean privateDnsActive; - - Entry(NetworkCapabilities capabilities, LinkProperties linkProperties) { - updateCapabilities(capabilities); - updateLinkProperties(linkProperties); - } - - void updateCapabilities(NetworkCapabilities supplied) { - capabilities = supplied == null ? null : new NetworkCapabilities(supplied); - fingerprint = Fingerprint.from(capabilities, linkProperties); - } - - boolean updateLinkProperties(LinkProperties supplied) { - String oldPrivateDns = privateDns; - boolean oldPrivateDnsActive = privateDnsActive; - // Parcelable copying works on older Android releases too; public - // LinkProperties constructors and setters require API 29. - String newPrivateDns = null; - boolean newPrivateDnsActive = false; - if (supplied != null && Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) { - newPrivateDns = supplied.getPrivateDnsServerName(); - newPrivateDnsActive = supplied.isPrivateDnsActive(); - } - linkProperties = copyLinkProperties(supplied); - privateDns = newPrivateDns; - privateDnsActive = newPrivateDnsActive; - fingerprint = Fingerprint.from(capabilities, linkProperties); - return !Objects.equals(oldPrivateDns, privateDns) || - oldPrivateDnsActive != privateDnsActive; - } - - private static LinkProperties copyLinkProperties(LinkProperties supplied) { - if (supplied == null) - return null; - Parcel parcel = Parcel.obtain(); - try { - supplied.writeToParcel(parcel, 0); - parcel.setDataPosition(0); - return LinkProperties.CREATOR.createFromParcel(parcel); - } finally { - parcel.recycle(); - } - } - } - - private static final class Fingerprint { - // hasCapability safely returns false for capabilities an older OS - // does not know; these integer constants do not invoke newer APIs. - @android.annotation.SuppressLint("InlinedApi") - private static final int[] CAPABILITIES = new int[]{ - NetworkCapabilities.NET_CAPABILITY_INTERNET, - NetworkCapabilities.NET_CAPABILITY_VALIDATED, - NetworkCapabilities.NET_CAPABILITY_NOT_VPN, - NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED, - NetworkCapabilities.NET_CAPABILITY_NOT_METERED, - NetworkCapabilities.NET_CAPABILITY_TEMPORARILY_NOT_METERED - }; - - private final boolean[] capabilities; - private final int[] transports; - private final List linkAddresses; - private final List routes; - private final List dnsServers; - private final String domains; - - private Fingerprint(boolean[] capabilities, int[] transports, - List linkAddresses, List routes, - List dnsServers, String domains) { - this.capabilities = capabilities; - this.transports = transports; - this.linkAddresses = linkAddresses; - this.routes = routes; - this.dnsServers = dnsServers; - this.domains = domains; - } - - static Fingerprint from(NetworkCapabilities caps, LinkProperties props) { - boolean[] capabilities = new boolean[CAPABILITIES.length]; - if (caps != null) - for (int i = 0; i < CAPABILITIES.length; i++) - capabilities[i] = caps.hasCapability(CAPABILITIES[i]); - - List transportTypes = new ArrayList<>(); - if (caps != null) { - // hasTransport is available on the minimum supported API. Do - // not use getTransportTypes(), which is newer than API 24. - for (int transport = 0; transport < 32; transport++) - if (caps.hasTransport(transport)) - transportTypes.add(transport); - } - int[] transports = new int[transportTypes.size()]; - for (int i = 0; i < transportTypes.size(); i++) - transports[i] = transportTypes.get(i); - Arrays.sort(transports); - - List linkAddresses = new ArrayList<>(); - List routes = new ArrayList<>(); - List dnsServers = new ArrayList<>(); - String domains = null; - if (props != null) { - for (LinkAddress address : props.getLinkAddresses()) - linkAddresses.add(address.toString()); - for (Object route : props.getRoutes()) - routes.add(String.valueOf(route)); - for (InetAddress dns : props.getDnsServers()) - dnsServers.add(dns.getHostAddress()); - domains = props.getDomains(); - } - Collections.sort(linkAddresses); - Collections.sort(routes); - Collections.sort(dnsServers); - return new Fingerprint(capabilities, transports, linkAddresses, routes, - dnsServers, domains); - } - - @Override - public boolean equals(Object other) { - if (!(other instanceof Fingerprint)) - return false; - Fingerprint that = (Fingerprint) other; - return Arrays.equals(capabilities, that.capabilities) && - Arrays.equals(transports, that.transports) && - Objects.equals(linkAddresses, that.linkAddresses) && - Objects.equals(routes, that.routes) && - Objects.equals(dnsServers, that.dnsServers) && - Objects.equals(domains, that.domains); - } - - @Override - public int hashCode() { - int result = Arrays.hashCode(capabilities); - result = 31 * result + Arrays.hashCode(transports); - result = 31 * result + linkAddresses.hashCode(); - result = 31 * result + routes.hashCode(); - result = 31 * result + dnsServers.hashCode(); - result = 31 * result + Objects.hashCode(domains); - return result; - } } private final Map entries = new HashMap<>(); private Network defaultNetwork; - private boolean baselineEstablished; - private int[] defaultVpnTransports; - - synchronized Change onPhysicalAvailable(Network network) { - if (network == null || entries.containsKey(network)) - return Change.none(); - entries.put(network, new Entry(null, null)); - return Change.of(NetworkReloadPolicy.REASON_NETWORK_AVAILABLE, false); + private boolean defaultSeen; + private List vpnTransports; + + synchronized String onPhysicalAvailable(Network network) { + if (network == null || entries.containsKey(network)) return null; + entries.put(network, new Entry()); + return NetworkReloadPolicy.REASON_NETWORK_AVAILABLE; + } + + synchronized String onPhysicalCapabilitiesChanged(Network network, NetworkCapabilities caps) { + if (network == null || caps == null || + !caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN)) return null; + Entry entry = entry(network); + List snapshot = capabilities(caps); + boolean changed = !snapshot.equals(entry.capabilities); + entry.capabilities = snapshot; + return changed ? NetworkReloadPolicy.REASON_NETWORK_CHANGED : null; + } + + synchronized String onPhysicalLinkPropertiesChanged(Network network, LinkProperties props) { + if (network == null || props == null) return null; + Entry entry = entry(network); + List snapshot = links(props); + String privateDns = Build.VERSION.SDK_INT >= Build.VERSION_CODES.P + ? props.getPrivateDnsServerName() : null; + boolean active = Build.VERSION.SDK_INT >= Build.VERSION_CODES.P && props.isPrivateDnsActive(); + boolean changed = !snapshot.equals(entry.links); + boolean privateChanged = !Objects.equals(privateDns, entry.privateDns) || + active != entry.privateDnsActive; + entry.links = snapshot; + entry.privateDns = privateDns; + entry.privateDnsActive = active; + if (changed) return NetworkReloadPolicy.REASON_LINK_PROPERTIES_CHANGED; + return privateChanged ? NetworkReloadPolicy.REASON_PRIVATE_DNS_CHANGED : null; + } + + synchronized String onPhysicalLost(Network network) { + if (network == null || entries.remove(network) == null) return null; + if (network.equals(defaultNetwork)) defaultNetwork = null; + return NetworkReloadPolicy.REASON_NETWORK_LOST; + } + + synchronized String onDefaultNetworkAvailable(Network network) { + return acceptDefaultIfPhysical(network); } - synchronized Change onPhysicalCapabilitiesChanged(Network network, NetworkCapabilities supplied) { - if (network == null || supplied == null || - !supplied.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN)) - return Change.none(); - Entry entry = entries.get(network); - if (entry == null) { - entry = new Entry(null, null); - entries.put(network, entry); + synchronized String onDefaultNetworkCapabilitiesChanged(Network network, NetworkCapabilities caps) { + if (network == null || caps == null) return null; + if (!caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN)) { + // A VPN can stay default while its underlying Wi-Fi/mobile transport + // changes. Ignore VPN identity churn caused by our own reloads. + List snapshot = transports(caps); + if (snapshot.isEmpty()) return null; + boolean changed = vpnTransports != null && !vpnTransports.equals(snapshot); + vpnTransports = snapshot; + return changed ? NetworkReloadPolicy.REASON_NETWORK_CHANGED : null; } - boolean changed = !entry.fingerprint.equals(Fingerprint.from(supplied, entry.linkProperties)); - entry.updateCapabilities(supplied); - if (changed) - return Change.of(NetworkReloadPolicy.REASON_NETWORK_CHANGED, false); - return Change.none(); + String change = onPhysicalCapabilitiesChanged(network, caps); + String defaultChange = acceptDefaultIfPhysical(network); + return defaultChange != null ? defaultChange : change; } - synchronized Change onPhysicalLinkPropertiesChanged(Network network, LinkProperties supplied) { - if (network == null) - return Change.none(); + synchronized String onDefaultNetworkLinkPropertiesChanged(Network network, LinkProperties props) { Entry entry = entries.get(network); - if (entry == null) { - entry = new Entry(null, null); - entries.put(network, entry); - } - Fingerprint oldFingerprint = entry.fingerprint; - boolean privateDnsChanged = entry.updateLinkProperties(supplied); - boolean changed = !oldFingerprint.equals(entry.fingerprint); - if (changed) - return Change.of(NetworkReloadPolicy.REASON_LINK_PROPERTIES_CHANGED, privateDnsChanged); - if (privateDnsChanged) - return Change.of(NetworkReloadPolicy.REASON_PRIVATE_DNS_CHANGED, true); - return Change.none(); + if (entry == null || entry.capabilities == null) return null; + String change = onPhysicalLinkPropertiesChanged(network, props); + String defaultChange = acceptDefaultIfPhysical(network); + return defaultChange != null ? defaultChange : change; } - synchronized Change onPhysicalLost(Network network) { - if (network == null || entries.remove(network) == null) - return Change.none(); - if (network.equals(defaultNetwork)) - defaultNetwork = null; - return Change.of(NetworkReloadPolicy.REASON_NETWORK_LOST, false); + synchronized String onDefaultNetworkLost(Network network) { + if (network != null && network.equals(defaultNetwork)) defaultNetwork = null; + return null; // The physical callback reports actual loss; VPN loss is self-generated. } - synchronized Change onDefaultNetworkAvailable(Network network) { - if (network == null) - return Change.none(); - return acceptDefaultIfPhysical(network); + private String acceptDefaultIfPhysical(Network network) { + Entry entry = entries.get(network); + if (entry == null || entry.capabilities == null) return null; + boolean changed = defaultSeen && !network.equals(defaultNetwork); + defaultNetwork = network; + defaultSeen = true; + return changed ? NetworkReloadPolicy.REASON_NETWORK_CHANGED : null; } - synchronized Change onDefaultNetworkCapabilitiesChanged(Network network, NetworkCapabilities supplied) { - if (network == null || supplied == null) - return Change.none(); - if (!supplied.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN)) { - // The VPN often remains the default Network across Wi-Fi/cellular - // handover. Its physical transport set changes even when both - // underlying networks were already available. Ignore VPN identity - // and link-property churn caused by our own establish/reload. - int[] transports = Fingerprint.from(supplied, null).transports; - int count = 0; - for (int transport : transports) - if (transport != NetworkCapabilities.TRANSPORT_VPN) - transports[count++] = transport; - if (count == 0) - return Change.none(); - transports = Arrays.copyOf(transports, count); - boolean changed = defaultVpnTransports != null && - !Arrays.equals(defaultVpnTransports, transports); - defaultVpnTransports = transports; - return changed ? Change.of(NetworkReloadPolicy.REASON_NETWORK_CHANGED, false) : Change.none(); + private Entry entry(Network network) { + Entry entry = entries.get(network); + if (entry == null) { + entry = new Entry(); + entries.put(network, entry); } - Change change = onPhysicalCapabilitiesChanged(network, supplied); - Change defaultChange = acceptDefaultIfPhysical(network); - if (defaultChange.getReason() != null) - return defaultChange; - return change; + return entry; } - synchronized Change onDefaultNetworkLinkPropertiesChanged(Network network, LinkProperties supplied) { - Entry entry = entries.get(network); - if (entry == null || entry.capabilities == null || - !entry.capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN)) - return Change.none(); - Change change = onPhysicalLinkPropertiesChanged(network, supplied); - Change defaultChange = acceptDefaultIfPhysical(network); - if (defaultChange.getReason() != null) - return defaultChange; - return change; + // Compare only route-relevant values, not signal strength or bandwidth. + // Unknown capability integers safely return false on older Android versions. + @android.annotation.SuppressLint("InlinedApi") + private static List capabilities(NetworkCapabilities caps) { + return Arrays.asList(transports(caps), + caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET), + caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED), + caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED), + caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED), + caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_TEMPORARILY_NOT_METERED)); } - synchronized Change onDefaultNetworkLost(Network network) { - if (network == null || !network.equals(defaultNetwork)) - return Change.none(); - defaultNetwork = null; - return Change.none(); + private static List transports(NetworkCapabilities caps) { + List result = new ArrayList<>(); + for (int transport = 0; transport < 32; transport++) + if (transport != NetworkCapabilities.TRANSPORT_VPN && caps.hasTransport(transport)) + result.add(transport); + return result; } - private Change acceptDefaultIfPhysical(Network network) { - Entry entry = entries.get(network); - if (entry == null || entry.capabilities == null || - !entry.capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN)) - return Change.none(); - if (!baselineEstablished) { - defaultNetwork = network; - baselineEstablished = true; - return Change.none(); - } - if (!network.equals(defaultNetwork)) { - defaultNetwork = network; - return Change.of("Network changed", false); - } - return Change.none(); + private static List links(LinkProperties props) { + // Retain immutable values rather than mutable callback objects. This + // also avoids LinkProperties constructors/setters that require API 29. + List result = new ArrayList<>(); + for (Object address : props.getLinkAddresses()) result.add("address:" + address); + for (Object route : props.getRoutes()) result.add("route:" + route); + for (java.net.InetAddress dns : props.getDnsServers()) result.add("dns:" + dns.getHostAddress()); + result.add("domains:" + props.getDomains()); + Collections.sort(result); + return result; } synchronized Network getDefaultNetwork() { return defaultNetwork; } - synchronized NetworkCapabilities getCapabilities(Network network) { - Entry entry = entries.get(network); - return entry == null || entry.capabilities == null ? null : - new NetworkCapabilities(entry.capabilities); - } - - synchronized LinkProperties getLinkProperties(Network network) { - Entry entry = entries.get(network); - return entry == null ? null : Entry.copyLinkProperties(entry.linkProperties); - } - synchronized void reset() { entries.clear(); defaultNetwork = null; - baselineEstablished = false; - defaultVpnTransports = null; + defaultSeen = false; + vpnTransports = null; } } diff --git a/app/src/main/java/eu/faircode/netguard/ServiceSinkhole.java b/app/src/main/java/eu/faircode/netguard/ServiceSinkhole.java index c1f4593c7..b9305b60f 100644 --- a/app/src/main/java/eu/faircode/netguard/ServiceSinkhole.java +++ b/app/src/main/java/eu/faircode/netguard/ServiceSinkhole.java @@ -2212,13 +2212,6 @@ public void onProviderRejected(String providerLabel, String message) { })); jni_wireguard_required(prefs.getBoolean("wg_enabled", false) && !TextUtils.isEmpty(prefs.getString("wg_config", ""))); - List probeSources = (last_builder == null - ? new ArrayList<>() : new ArrayList<>(last_builder.listAddress)); - List probeResolvers = new ArrayList<>(); - if (last_builder != null) - for (InetAddress dns : last_builder.listDns) - if (dns != null && dns.getHostAddress() != null) - probeResolvers.add(dns.getHostAddress()); boolean wgOk = net.kollnig.missioncontrol.wg.WgEgress.INSTANCE.startOrUpdate( prefs.getBoolean("wg_enabled", false), prefs.getString("wg_config", ""), @@ -2227,9 +2220,7 @@ public void onProviderRejected(String providerLabel, String message) { Util.isInteractive(ServiceSinkhole.this), prefs.getBoolean("wg_keepalive_when_screen_off", false), () -> jni_wireguard_start(), - () -> { jni_wireguard_stop(); return kotlin.Unit.INSTANCE; }, - probeSources, - probeResolvers); + () -> { jni_wireguard_stop(); return kotlin.Unit.INSTANCE; }); if (!wgOk) { String wgError = net.kollnig.missioncontrol.wg.WgEgress.INSTANCE.getLastError(); Log.w(TAG, "WireGuard egress failed to start; blocking traffic: " + wgError); @@ -3973,9 +3964,9 @@ public void onLost(Network network) { } } - private void handlePhysicalNetworkChange(PhysicalNetworkState.Change change) { - if (change.getReason() != null) - reloadAfterNetworkChange(change.getReason()); + private void handlePhysicalNetworkChange(String reason) { + if (reason != null) + reloadAfterNetworkChange(reason); } // Network flapping (Wi-Fi<->cellular handoffs, DHCP renewals) fires several diff --git a/app/src/main/java/net/kollnig/missioncontrol/wg/WgConnectivityMonitor.kt b/app/src/main/java/net/kollnig/missioncontrol/wg/WgConnectivityMonitor.kt index e6471e65b..4e5718578 100644 --- a/app/src/main/java/net/kollnig/missioncontrol/wg/WgConnectivityMonitor.kt +++ b/app/src/main/java/net/kollnig/missioncontrol/wg/WgConnectivityMonitor.kt @@ -10,9 +10,7 @@ data class WgStats( val latestHandshakeMillis: Long, val hasFreshHandshake: Boolean = false, val tunWriteFailuresTotal: Long = 0L, - val tunWriteFailuresStreak: Long = 0L, - val deliveredRxBytes: Long = 0L, - val probeReplyToken: Long = 0L + val tunWriteFailuresStreak: Long = 0L ) /** Outcome of a single connectivity poll. */ @@ -84,10 +82,9 @@ internal class WgConnectivityChecker(private val prod: () -> Unit) { private var tunWriteFailureStartedAt: Long? = null private var tunWriteFailureRunIdentity: Long? = null private var tunWriteFailuresSuspended = false - private var deliveredRxBytes = 0L /** - * True when the most recent [tick] observed the delivered-IP counter advancing — + * True when the most recent [tick] observed the rx counter advancing — * decrypted return traffic, the only signal that proves the data path * works end to end. A completed handshake is NOT such proof (a path can * pass handshakes yet drop transport packets), so recovery backoff resets @@ -100,7 +97,6 @@ internal class WgConnectivityChecker(private val prod: () -> Unit) { fun seed(now: Long, stats: WgStats) { state = ConnState.Connecting(now, false, stats.rxBytes, stats.txBytes) lastTickSawRx = false - deliveredRxBytes = stats.deliveredRxBytes resetProd() resetTunWriteFailures(stats.tunWriteFailuresTotal) tunWriteFailuresSuspended = false @@ -112,8 +108,7 @@ internal class WgConnectivityChecker(private val prod: () -> Unit) { if (stats == null) return WgVerdict.GONE val rxAdvanced = update(now, stats.rxBytes, stats.txBytes) - lastTickSawRx = stats.deliveredRxBytes > deliveredRxBytes - deliveredRxBytes = stats.deliveredRxBytes + lastTickSawRx = rxAdvanced // A completed handshake can coexist with a TUN fd that rejects every // decrypted packet. Evaluate the write-failure evidence first so the @@ -128,6 +123,7 @@ internal class WgConnectivityChecker(private val prod: () -> Unit) { } if (rxAdvanced) { + lastTickSawRx = true resetProd() return WgVerdict.HEALTHY } @@ -338,11 +334,7 @@ internal class WgConnectivityMonitor( // Test-only barrier between callback authorization and dispatch. Keeping // this seam here makes the stop/callback ordering deterministic without // adding any production scheduling. - private val beforeCallback: () -> Unit = {}, - // A generation-gated sample stream used by handover verification. The - // callback runs on the monitor thread and must only enqueue JNI work. - private val onSample: (WgStats) -> Unit = {}, - private val onSuspended: () -> Unit = {} + private val beforeCallback: () -> Unit = {} ) { internal companion object { /** How often the loop samples the tunnel counters while the screen is on. */ @@ -619,7 +611,6 @@ internal class WgConnectivityMonitor( // The device dozed; the elapsed gap is not evidence of a stall. if (isSuspendGap(slept, intervalMs)) { statsFailures = 0 - invokeCallback(generation, "onSuspended", onSuspended) checker.onSuspended(now) continue } @@ -651,9 +642,6 @@ internal class WgConnectivityMonitor( statsFailures = 0 if (!isCurrent()) return - invokeCallback(generation, "onSample") { onSample(stats) } - if (!isActive(generation)) return - when (checker.tick(now, stats)) { WgVerdict.HEALTHY, WgVerdict.WAITING -> {} WgVerdict.BROKEN -> { diff --git a/app/src/main/java/net/kollnig/missioncontrol/wg/WgEgress.kt b/app/src/main/java/net/kollnig/missioncontrol/wg/WgEgress.kt index 534e086a3..442d2870a 100644 --- a/app/src/main/java/net/kollnig/missioncontrol/wg/WgEgress.kt +++ b/app/src/main/java/net/kollnig/missioncontrol/wg/WgEgress.kt @@ -85,8 +85,8 @@ internal class WgMonitorLifecycle( * Lifecycle is driven by [startOrUpdate] from `ServiceSinkhole.startNative` * and [stop] from the actual VPN-shutdown path. Crucially, `stopNative` does * NOT call [stop] — when NetGuard does a "Native restart" reload (same - * builder, same TUN fd) we want WG to keep running so we don't redo the - * handshake on every DHCP/connectivity blip. + * builder, same TUN fd) ordinarily preserves WG. A debounced physical-network + * change explicitly requests a fresh tunnel through that same reload path. * * The wgbridge classes used here are hand-written JNI bindings to the Rust * crate in `wgbridge-rs/`; build instructions live in `wgbridge-rs/README.md`. @@ -143,11 +143,6 @@ object WgEgress { // [reportProviderFailure]. @Volatile private var pendingProviderFailure: String? = null @Volatile private var verificationGeneration: Long = 0 - @Volatile private var pendingHandoverVerification: Boolean = false - @Volatile private var handoverUpdateInProgress: Boolean = false - @Volatile private var currentProbeSources: List = emptyList() - @Volatile private var currentProbeResolvers: List = emptyList() - private val handoverVerifier = WgHandoverVerifier() @Volatile private var currentConfig: String? = null private var currentTunFd: Int = -1 // The exact ParcelFileDescriptor the running tunnel was started with. A @@ -194,19 +189,6 @@ object WgEgress { } } - // Single-thread executor for network-change rebinds: bounds the thread - // count on a flapping network (instead of one raw Thread per event). - // rebindInFlight means a rebind task is running; a network change arriving - // during that window sets rebindDirty so the task re-runs once with the - // latest network instead of being dropped — otherwise the sockets could - // stay bound to a network that has already gone away. - private val rebindExecutor = java.util.concurrent.Executors.newSingleThreadExecutor { - Thread(it, "wg-rebind").apply { isDaemon = true } - } - private val rebindLock = Any() - private var rebindInFlight: Boolean = false - private var rebindDirty: Boolean = false - @Volatile private var requestReloadCb: Runnable? = null @Volatile private var notifyBrokenCb: Runnable? = null // Provider-aware hook: tries to move the active profile to a different @@ -279,134 +261,117 @@ object WgEgress { interactive: Boolean, keepaliveAlwaysOn: Boolean, startSocketpair: () -> Int, - stopSocketpair: () -> Unit, - probeSources: List = emptyList(), - probeResolvers: List = emptyList() + stopSocketpair: () -> Unit ): Boolean { - synchronized(tunnelLifecycleLock) { - handoverUpdateInProgress = true - pendingHandoverVerification = pendingHandoverVerification || handoverVerifier.isPending() - verificationGeneration++ - handoverVerifier.cancel() - } - try { - val wantRunning = wgEnabled && !configText.isNullOrEmpty() - val desiredFd = vpnFd.fd - lastError = null - - if (!wantRunning) { - clearRecoveryState() - pendingHandoverVerification = false - handoverVerifier.cancel() - clearAllEndpointState() - if (tunnel != null) { - Log.i(TAG, "WG disabled — tearing down tunnel") - stopInternal(stopSocketpair) - notifyStateChanged() - } - return true - } - - if (tunnel != null && currentConfig == configText && currentTunPfd === vpnFd && !forceRestartPending) { - val oldKeepaliveEnabled = currentInteractive || currentKeepaliveAlwaysOn - val newKeepaliveEnabled = interactive || keepaliveAlwaysOn - if (oldKeepaliveEnabled != newKeepaliveEnabled && - !updateKeepaliveOrError(configText!!, newKeepaliveEnabled, interactive, keepaliveAlwaysOn)) - return false - // The tunnel can outlive a monitor whose initial stats read raced - // tunnel startup (or which exited after a stale sample). Keep the - // idempotent tunnel path, but recreate a dead watchdog so a - // same-config start does not silently leave connectivity unwatched. - startMonitorIfDead() - currentProbeSources = probeSources.toList() - currentProbeResolvers = probeResolvers.toList() - Log.v(TAG, "startOrUpdate: same config + same TUN pfd, no-op") - return true - } + verificationGeneration++ + val wantRunning = wgEnabled && !configText.isNullOrEmpty() + val desiredFd = vpnFd.fd + lastError = null + if (!wantRunning) { + clearRecoveryState() + clearAllEndpointState() if (tunnel != null) { - Log.i(TAG, "WG config, TUN fd, or recovery state changed — restarting") + Log.i(TAG, "WG disabled — tearing down tunnel") stopInternal(stopSocketpair) - } - forceRestartPending = false - val keepaliveEnabled = interactive || keepaliveAlwaysOn - - val parsed = try { - WgConfigParser.parse(configText!!) - } catch (e: Exception) { - lastError = "Invalid WireGuard config: ${e.message}" - Log.e(TAG, "config parse: ${e.message}") notifyStateChanged() - return false } + return true + } - val resolved = try { - withResolvedEndpoints(parsed) - } catch (e: Exception) { - lastError = "WireGuard endpoint resolution failed: ${e.message}" - Log.e(TAG, "endpoint resolve: ${e.message}") - notifyStateChanged() + if (tunnel != null && currentConfig == configText && currentTunPfd === vpnFd && !forceRestartPending) { + val oldKeepaliveEnabled = currentInteractive || currentKeepaliveAlwaysOn + val newKeepaliveEnabled = interactive || keepaliveAlwaysOn + if (oldKeepaliveEnabled != newKeepaliveEnabled && + !updateKeepaliveOrError(configText!!, newKeepaliveEnabled, interactive, keepaliveAlwaysOn)) return false - } + // The tunnel can outlive a monitor whose initial stats read raced + // tunnel startup (or which exited after a stale sample). Keep the + // idempotent tunnel path, but recreate a dead watchdog so a + // same-config start does not silently leave connectivity unwatched. + startMonitorIfDead() + Log.v(TAG, "startOrUpdate: same config + same TUN pfd, no-op") + return true + } - val rxFd = startSocketpair() - if (rxFd < 0) { - lastError = "Could not create WireGuard packet socket" - Log.e(TAG, "jni_wireguard_start failed") - notifyStateChanged() - return false - } + if (tunnel != null) { + Log.i(TAG, "WG config, TUN fd, or recovery state changed — restarting") + stopInternal(stopSocketpair) + } + forceRestartPending = false + val keepaliveEnabled = interactive || keepaliveAlwaysOn - val mtu = resolved.mtu ?: DEFAULT_MTU - val protector = object : WgProtector { - override fun protect(fd: Int): Boolean = vpnService.protect(fd) - } - val logger = object : WgLogger { - override fun verbosef(s: String) { Log.v(TAG, s) } - override fun errorf(s: String) { Log.e(TAG, s) } - } - val dnsRecorder = object : WgDnsRecorder { - override fun recordDns(qname: String, aname: String, resource: String, ttl: Int) { - if (vpnService is eu.faircode.netguard.ServiceSinkhole) - vpnService.wireGuardDnsResolved(qname, aname, resource, ttl) - } - } + val parsed = try { + WgConfigParser.parse(configText!!) + } catch (e: Exception) { + lastError = "Invalid WireGuard config: ${e.message}" + Log.e(TAG, "config parse: ${e.message}") + notifyStateChanged() + return false + } - val startedTunnel = try { - Wgbridge.startTunnel( - resolved.toUapi(keepaliveEnabled), rxFd, desiredFd, mtu, protector, logger, dnsRecorder - ) - } catch (e: Throwable) { - lastError = "WireGuard tunnel failed to start: ${e.message ?: e.javaClass.simpleName}" - Log.e(TAG, "Wgbridge.startTunnel failed", e) - stopSocketpair() - notifyStateChanged() - return false - } finally { - closeRawFd(rxFd) - } + val resolved = try { + withResolvedEndpoints(parsed) + } catch (e: Exception) { + lastError = "WireGuard endpoint resolution failed: ${e.message}" + Log.e(TAG, "endpoint resolve: ${e.message}") + notifyStateChanged() + return false + } - currentConfig = configText - currentTunFd = desiredFd - currentTunPfd = vpnFd - currentInteractive = interactive - currentKeepaliveAlwaysOn = keepaliveAlwaysOn - currentProbeSources = probeSources.toList() - currentProbeResolvers = probeResolvers.toList() - synchronized(tunnelLifecycleLock) { - tunnelGeneration.incrementAndGet() - // Volatile publication happens only after all companion state has - // been installed above. - tunnel = startedTunnel + val rxFd = startSocketpair() + if (rxFd < 0) { + lastError = "Could not create WireGuard packet socket" + Log.e(TAG, "jni_wireguard_start failed") + notifyStateChanged() + return false + } + + val mtu = resolved.mtu ?: DEFAULT_MTU + val protector = object : WgProtector { + override fun protect(fd: Int): Boolean = vpnService.protect(fd) + } + val logger = object : WgLogger { + override fun verbosef(s: String) { Log.v(TAG, s) } + override fun errorf(s: String) { Log.e(TAG, s) } + } + val dnsRecorder = object : WgDnsRecorder { + override fun recordDns(qname: String, aname: String, resource: String, ttl: Int) { + if (vpnService is eu.faircode.netguard.ServiceSinkhole) + vpnService.wireGuardDnsResolved(qname, aname, resource, ttl) } - Log.i(TAG, "WG up: tunFd=$desiredFd mtu=$mtu peers=${resolved.peers.size}") + } + + val startedTunnel = try { + Wgbridge.startTunnel( + resolved.toUapi(keepaliveEnabled), rxFd, desiredFd, mtu, protector, logger, dnsRecorder + ) + } catch (e: Throwable) { + lastError = "WireGuard tunnel failed to start: ${e.message ?: e.javaClass.simpleName}" + Log.e(TAG, "Wgbridge.startTunnel failed", e) + stopSocketpair() notifyStateChanged() - scheduleFreshHandshakeNotificationCheck() - startMonitor() - return true + return false } finally { - handoverUpdateInProgress = false + closeRawFd(rxFd) + } + + currentConfig = configText + currentTunFd = desiredFd + currentTunPfd = vpnFd + currentInteractive = interactive + currentKeepaliveAlwaysOn = keepaliveAlwaysOn + synchronized(tunnelLifecycleLock) { + tunnelGeneration.incrementAndGet() + // Volatile publication happens only after all companion state has + // been installed above. + tunnel = startedTunnel } + Log.i(TAG, "WG up: tunFd=$desiredFd mtu=$mtu peers=${resolved.peers.size}") + notifyStateChanged() + scheduleFreshHandshakeNotificationCheck() + startMonitor() + return true } private fun startMonitor() { @@ -441,9 +406,7 @@ object WgEgress { // produces, so resetting on it would defeat the backoff. onRxAdvanced = { if (isCurrent(expected)) restartAttempts = 0 }, isInteractive = { currentInteractive }, - isCurrent = { isCurrent(expected) }, - onSample = { stats -> onHandoverSample(expected, stats) }, - onSuspended = { handoverVerifier.onSuspended() } + isCurrent = { isCurrent(expected) } ) monitorLifecycle.replace( candidate, @@ -452,82 +415,6 @@ object WgEgress { ) } - private fun beginHandoverVerification(expected: TunnelSnapshot, baseline: WgStats? = null) { - val generation = synchronized(tunnelLifecycleLock) { - if (!pendingHandoverVerification || handoverUpdateInProgress || forceRestartPending || - !isCurrentLocked(expected)) return - verificationGeneration - } - synchronized(rebindLock) { - if (rebindInFlight) return - } - val sample = baseline ?: statsOrNull(expected) ?: return - val config = currentConfig ?: return - val targets = try { - val parsed = WgConfigParser.parse(config) - WgProbeTargetSelector.select( - currentProbeSources, - currentProbeResolvers, - parsed.peers.flatMap { it.allowedIPs } - ) - } catch (e: Throwable) { - Log.w(TAG, "handover probe target selection failed", e) - emptyList() - } - val action = synchronized(tunnelLifecycleLock) { - if (!pendingHandoverVerification || handoverUpdateInProgress || forceRestartPending || !isCurrentLocked(expected) || - generation != verificationGeneration) return - pendingHandoverVerification = false - handoverVerifier.begin(generation, sample, targets, currentInteractive) - } - dispatchHandoverAction(expected, action) - } - - private fun onHandoverSample(expected: TunnelSnapshot, stats: WgStats) { - if (!isCurrent(expected)) return - if (pendingHandoverVerification) { - beginHandoverVerification(expected, stats) - return - } - dispatchHandoverAction( - expected, - handoverVerifier.onSample(verificationGeneration, stats, currentInteractive) - ) - } - - private fun dispatchHandoverAction(expected: TunnelSnapshot, action: WgHandoverVerifier.Action) { - when (action) { - WgHandoverVerifier.Action.None -> Unit - is WgHandoverVerifier.Action.Probe -> { - rebindExecutor.execute { - if (!currentInteractive || !isCurrent(expected) || - !handoverVerifier.isCurrent(action.generation, action.token)) return@execute - try { - if (!expected.tunnel.sendDnsProbe( - action.target.sourceIp, - action.target.resolverIp, - action.token - ) - ) Log.i(TAG, "handover probe enqueue was rejected; awaiting bounded retry") - } catch (e: Throwable) { - Log.w(TAG, "handover probe enqueue failed; awaiting bounded retry", e) - } - } - } - is WgHandoverVerifier.Action.Restart -> { - if (action.generation == verificationGeneration && isCurrent(expected)) { - requestFullRestart( - "WG handover verification failed", - notify = false, - expected = expected, - eligibleForFailover = false, - expectedVerificationGeneration = action.generation - ) - } - } - } - } - private fun stopMonitor() { monitorLifecycle.stop() } @@ -570,8 +457,7 @@ object WgEgress { reason: String, notify: Boolean, expected: TunnelSnapshot, - eligibleForFailover: Boolean, - expectedVerificationGeneration: Long? = null + eligibleForFailover: Boolean ) { val attempt: Int synchronized(tunnelLifecycleLock) { @@ -579,12 +465,6 @@ object WgEgress { // tunnel replacement. Otherwise a replacement can land between // them and inherit forceRestartPending from an obsolete failure. if (!isCurrentLocked(expected)) return - if (expectedVerificationGeneration != null && - (expectedVerificationGeneration != verificationGeneration || !currentInteractive)) return - // Verify the replacement too. A successful construction/handshake - // alone must not end recovery of a handover that lost its data path. - if (expectedVerificationGeneration != null) - pendingHandoverVerification = true clearEndpointCache() attempt = restartAttempts++ forceRestartPending = true @@ -779,9 +659,7 @@ object WgEgress { it.latestHandshakeMillis, it.latestHandshakeMillis > 0 && now() - it.latestHandshakeMillis < HANDSHAKE_DEAD_AFTER_MS, it.tunWriteFailuresTotal, - it.tunWriteFailuresStreak, - it.deliveredRxBytes, - it.probeReplyToken + it.tunWriteFailuresStreak ) } } catch (e: Throwable) { @@ -824,88 +702,14 @@ object WgEgress { try { tunnel?.latestHandshakeMillis() } catch (_: Throwable) { null } fun onUnderlyingNetworkChanged() { - synchronized(tunnelLifecycleLock) { - verificationGeneration++ - pendingHandoverVerification = tunnel != null - handoverVerifier.cancel() - } + verificationGeneration++ clearEndpointCache() - if (tunnel == null) return - // A full restart is already queued (and the accompanying reload() is - // in flight); rebinding concurrently would just race it. - if (forceRestartPending) return - - // Rebind the protected UDP sockets onto the new default network and - // re-resolve the endpoint instead of tearing the tunnel down: the - // WireGuard session survives outer-address changes, so this recovers - // roaming (Wi-Fi <-> cellular, crossing borders) without a - // re-handshake. Runs off-thread because endpoint re-resolution does - // blocking DNS. Falls back to a full restart if the rebind fails. - synchronized(rebindLock) { - if (rebindInFlight) { - // A rebind is already running; the network changed again, so - // mark it for a re-run rather than dropping this event. - rebindDirty = true - Log.i(TAG, "underlying network changed; rebind in flight, scheduling re-run") - return - } - rebindInFlight = true - rebindDirty = false - } - - Log.i(TAG, "underlying network changed; rebinding WG sockets") - rebindExecutor.execute { - try { - while (true) { - val expected = captureTunnel() - if (expected == null) { - synchronized(rebindLock) { - rebindInFlight = false - rebindDirty = false - } - return@execute - } - when (tryCheapRecovery(expected)) { - RecoveryResult.SUCCEEDED -> { - if (isCurrent(expected)) { - lastCheapRecoveryMs = now() - // The monitor starts verification after this - // rebind (and any dirty re-run) leaves the queue. - } - } - RecoveryResult.FAILED -> { - if (!forceRestartPending) { - requestFullRestart( - "WG rebind failed after network change", - notify = false, - expected = expected, - // A rebind failure means the local network - // changed under us, not that the relay is - // dead — don't let it count toward - // switching relays. - eligibleForFailover = false - ) - } - } - RecoveryResult.STALE -> Unit - } - synchronized(rebindLock) { - if (!rebindDirty) { - rebindInFlight = false - return@execute - } - // Another network change landed mid-rebind; loop once - // more with the now-current default network. - rebindDirty = false - } - } - } catch (e: Throwable) { - Log.w(TAG, "WG rebind task failed", e) - synchronized(rebindLock) { - rebindInFlight = false - rebindDirty = false - } - } + // ServiceSinkhole calls this once per debounced network-change burst, + // immediately before reload. Recreate the tunnel in that reload even + // if its configuration and TUN are unchanged: a successful socket + // rebind does not prove the new path can carry application traffic. + synchronized(tunnelLifecycleLock) { + if (tunnel != null) forceRestartPending = true } } @@ -927,10 +731,6 @@ object WgEgress { val oldKeepaliveEnabled = currentInteractive || currentKeepaliveAlwaysOn val newKeepaliveEnabled = interactive || keepaliveAlwaysOn - synchronized(tunnelLifecycleLock) { - currentInteractive = interactive - handoverVerifier.setInteractive(interactive) - } if (oldKeepaliveEnabled == newKeepaliveEnabled) { currentInteractive = interactive currentKeepaliveAlwaysOn = keepaliveAlwaysOn @@ -973,8 +773,7 @@ object WgEgress { currentTunPfd = null currentKeepaliveAlwaysOn = false lastCheapRecoveryMs = 0 - bumpVerificationGeneration() - handoverVerifier.cancel() + verificationGeneration++ // An error describes a tunnel that no longer exists. Listeners check // lastError before isRunning — deliberately, so a start that fails // without ever producing a tunnel still reports — so leaving it set @@ -1058,9 +857,7 @@ object WgEgress { } private fun clearRecoveryState() { - bumpVerificationGeneration() - pendingHandoverVerification = false - handoverVerifier.cancel() + verificationGeneration++ recoveryNotificationGeneration++ providerFailureReason = null pendingProviderFailure = null @@ -1100,12 +897,6 @@ object WgEgress { private fun now(): Long = System.currentTimeMillis() - private fun bumpVerificationGeneration() { - synchronized(tunnelLifecycleLock) { - verificationGeneration++ - } - } - private fun withResolvedEndpoints(config: WgConfig): WgConfig { return config.copy(peers = config.peers.map { peer -> val ep = peer.endpoint diff --git a/app/src/main/java/net/kollnig/missioncontrol/wg/WgHandoverVerifier.kt b/app/src/main/java/net/kollnig/missioncontrol/wg/WgHandoverVerifier.kt deleted file mode 100644 index d99940e20..000000000 --- a/app/src/main/java/net/kollnig/missioncontrol/wg/WgHandoverVerifier.kt +++ /dev/null @@ -1,208 +0,0 @@ -package net.kollnig.missioncontrol.wg - -import java.net.InetAddress - -/** A numeric source/resolver pair for an in-tunnel DNS probe. */ -internal data class WgProbeTarget(val sourceIp: String, val resolverIp: String) - -/** - * Bounded, generation-scoped verification after an underlying-network change. - * It is deliberately clock-injected and side-effect free: the owner dispatches - * [Action.Probe] away from the main thread and decides how to restart. - */ -internal class WgHandoverVerifier( - private val clock: () -> Long = { android.os.SystemClock.elapsedRealtime() } -) { - companion object { - const val PROBE_INTERVAL_MS = 5_000L - const val VERIFY_TIMEOUT_MS = 15_000L - const val MAX_PROBES = 3 - } - - sealed class Action { - object None : Action() - data class Probe(val generation: Long, val target: WgProbeTarget, val token: Long) : Action() - data class Restart(val generation: Long) : Action() - } - - private data class Session( - val generation: Long, - val targets: List, - var baselineDelivered: Long, - var currentToken: Long, - var probesSent: Int, - var nextProbeAt: Long, - var deadline: Long, - var paused: Boolean - ) - - private var session: Session? = null - private var nextToken = 0L - - @Synchronized fun begin( - generation: Long, - baseline: WgStats, - targets: List, - interactive: Boolean, - now: Long = clock() - ): Action { - session = null - if (targets.isEmpty()) return Action.None - val next = Session( - generation = generation, - targets = targets.toList(), - baselineDelivered = baseline.deliveredRxBytes, - currentToken = 0L, - probesSent = 0, - nextProbeAt = now, - deadline = now + VERIFY_TIMEOUT_MS, - paused = !interactive - ) - session = next - return if (interactive) issueProbe(next, now) else Action.None - } - - @Synchronized fun onSample( - generation: Long, - stats: WgStats, - interactive: Boolean, - now: Long = clock() - ): Action { - val current = session ?: return Action.None - if (current.generation != generation) return Action.None - - if (!interactive) { - // Screen-off verification is deferred and rebased. This avoids - // probes and deadlines being driven by the idle monitor cadence. - current.paused = true - current.baselineDelivered = stats.deliveredRxBytes - current.currentToken = 0L - current.probesSent = 0 - return Action.None - } - - if (current.paused) { - current.paused = false - current.baselineDelivered = stats.deliveredRxBytes - current.currentToken = 0L - current.probesSent = 0 - current.nextProbeAt = now - current.deadline = now + VERIFY_TIMEOUT_MS - return issueProbe(current, now) - } - - if (stats.deliveredRxBytes > current.baselineDelivered || - (current.currentToken != 0L && stats.probeReplyToken == current.currentToken)) { - session = null - return Action.None - } - - if (now >= current.deadline) { - session = null - return Action.Restart(current.generation) - } - - return if (current.probesSent < MAX_PROBES && now >= current.nextProbeAt) - issueProbe(current, now) - else - Action.None - } - - @Synchronized fun isCurrent(generation: Long, token: Long): Boolean = - session?.let { it.generation == generation && it.currentToken == token } == true - - @Synchronized fun isPending(): Boolean = session != null - - @Synchronized fun cancel() { - session = null - } - - @Synchronized fun setInteractive(interactive: Boolean) { - if (!interactive) { - session?.apply { - paused = true - currentToken = 0L - probesSent = 0 - } - } - } - - @Synchronized fun onSuspended() { - setInteractive(false) - } - - @Synchronized fun cancelIfCurrent(generation: Long, token: Long) { - if (isCurrent(generation, token)) session = null - } - - private fun issueProbe(session: Session, now: Long): Action { - val token = ++nextToken - session.currentToken = token - session.probesSent++ - session.nextProbeAt = now + PROBE_INTERVAL_MS - val target = session.targets[(session.probesSent - 1) % session.targets.size] - return Action.Probe(session.generation, target, token) - } -} - -/** Select only numeric, same-family resolver targets covered by WireGuard routes. */ -internal object WgProbeTargetSelector { - private val ipv4 = Regex("^(?:[0-9]{1,3}\\.){3}[0-9]{1,3}$") - - fun select( - sourceAddresses: List, - resolverAddresses: List, - allowedIps: List - ): List { - val sources = sourceAddresses.mapNotNull { numericAddress(it.substringBefore('/')) } - val resolvers = resolverAddresses.mapNotNull { numericAddress(it) } - val targets = ArrayList() - for ((sourceText, source) in sources) { - for ((resolverText, resolver) in resolvers) { - if (source.size != resolver.size || !allowedIps.any { contains(it, resolver) }) - continue - val target = WgProbeTarget(sourceText, resolverText) - if (!targets.contains(target)) targets += target - } - } - return targets - } - - private fun numericAddress(text: String, rejectSpecial: Boolean = true): Pair? { - if (text.isEmpty()) return null - if (!text.contains(':') && !ipv4.matches(text)) return null - if (text.contains(':') && !WgConfigParser.isIpv6Literal(text)) return null - val bytes = try { InetAddress.getByName(text).address } catch (_: Exception) { return null } - val address = try { InetAddress.getByAddress(bytes) } catch (_: Exception) { return null } - if (rejectSpecial && - (address.isAnyLocalAddress || address.isMulticastAddress || address.isLoopbackAddress)) - return null - return if ((text.contains(':') && bytes.size == 16) || - (!text.contains(':') && bytes.size == 4)) text to bytes else null - } - - private fun contains(entry: String, address: ByteArray): Boolean { - val slash = entry.indexOf('/') - val prefixText = if (slash < 0) null else entry.substring(slash + 1) - val parsed = numericAddress( - if (slash < 0) entry else entry.substring(0, slash), - rejectSpecial = false - ) ?: return false - if (parsed.second.size != address.size) return false - val prefix = if (prefixText == null) parsed.second.size * 8 - else prefixText.toIntOrNull() ?: return false - if (prefix !in 0..parsed.second.size * 8) return false - var remaining = prefix - for (i in parsed.second.indices) { - if (remaining >= 8) { - if (parsed.second[i] != address[i]) return false - remaining -= 8 - } else if (remaining > 0) { - val mask = (0xff shl (8 - remaining)) and 0xff - if ((parsed.second[i].toInt() and mask) != (address[i].toInt() and mask)) return false - break - } else break - } - return true - } -} diff --git a/app/src/main/java/net/kollnig/missioncontrol/wgbridge/Tunnel.java b/app/src/main/java/net/kollnig/missioncontrol/wgbridge/Tunnel.java index e3eeee09c..94ccfece8 100644 --- a/app/src/main/java/net/kollnig/missioncontrol/wgbridge/Tunnel.java +++ b/app/src/main/java/net/kollnig/missioncontrol/wgbridge/Tunnel.java @@ -21,16 +21,15 @@ public synchronized void setConfig(String uapiConfig) { /** * Snapshot of the device's transfer counters and newest handshake, summed - * across all peers. Engine rxBytes includes handshakes; deliveredRxBytes - * counts only complete decrypted IP packets successfully written to Android. + * across all peers. rx bytes count decrypted transport payload, so they + * only advance when the tunnel actually carries return traffic — that is + * the liveness signal the connectivity monitor is biased toward. */ public synchronized TunnelStats stats() { long[] values = nativeStats(handle); long totalFailures = values.length > 3 ? values[3] : 0L; long failureStreak = values.length > 4 ? values[4] : 0L; - long deliveredRx = values.length > 5 ? values[5] : 0L; - long probeReply = values.length > 6 ? values[6] : 0L; - return new TunnelStats(values[0], values[1], values[2], totalFailures, failureStreak, deliveredRx, probeReply); + return new TunnelStats(values[0], values[1], values[2], totalFailures, failureStreak); } /** @@ -57,11 +56,6 @@ public synchronized void rebind() { nativeRebind(handle); } - /** Queues a DNS reachability probe inside WireGuard. No direct socket send. */ - public synchronized boolean sendDnsProbe(String sourceIp, String resolverIp, long token) { - return nativeSendDnsProbe(handle, sourceIp, resolverIp, token); - } - /** * Moves a peer to a new endpoint ("ip:port" or "[ipv6]:port", already * resolved) without disturbing the session, e.g. after DNS re-resolution. @@ -98,8 +92,6 @@ public synchronized void stop() { private static native void nativeRebind(long handle); - private static native boolean nativeSendDnsProbe(long handle, String sourceIp, String resolverIp, long token); - private static native void nativeUpdateEndpoint(long handle, String peerPublicKeyBase64, String endpoint); private static native void nativeSetKeepalive(long handle, String peerPublicKeyBase64, int seconds); diff --git a/app/src/main/java/net/kollnig/missioncontrol/wgbridge/TunnelStats.java b/app/src/main/java/net/kollnig/missioncontrol/wgbridge/TunnelStats.java index c1340e410..9480a8999 100644 --- a/app/src/main/java/net/kollnig/missioncontrol/wgbridge/TunnelStats.java +++ b/app/src/main/java/net/kollnig/missioncontrol/wgbridge/TunnelStats.java @@ -9,8 +9,6 @@ public final class TunnelStats { public final long latestHandshakeMillis; public final long tunWriteFailuresTotal; public final long tunWriteFailuresStreak; - public final long deliveredRxBytes; - public final long probeReplyToken; TunnelStats(long rxBytes, long txBytes, long latestHandshakeMillis) { this(rxBytes, txBytes, latestHandshakeMillis, 0L, 0L); @@ -18,37 +16,17 @@ public final class TunnelStats { TunnelStats(long rxBytes, long txBytes, long latestHandshakeMillis, long tunWriteFailuresTotal, long tunWriteFailuresStreak) { - this(rxBytes, txBytes, latestHandshakeMillis, tunWriteFailuresTotal, tunWriteFailuresStreak, 0L); - } - - TunnelStats(long rxBytes, long txBytes, long latestHandshakeMillis, - long tunWriteFailuresTotal, long tunWriteFailuresStreak, long deliveredRxBytes) { - this(rxBytes, txBytes, latestHandshakeMillis, tunWriteFailuresTotal, tunWriteFailuresStreak, deliveredRxBytes, 0L); - } - - TunnelStats(long rxBytes, long txBytes, long latestHandshakeMillis, - long tunWriteFailuresTotal, long tunWriteFailuresStreak, long deliveredRxBytes, long probeReplyToken) { this.rxBytes = rxBytes; this.txBytes = txBytes; this.latestHandshakeMillis = latestHandshakeMillis; this.tunWriteFailuresTotal = tunWriteFailuresTotal; this.tunWriteFailuresStreak = tunWriteFailuresStreak; - this.deliveredRxBytes = deliveredRxBytes; - this.probeReplyToken = probeReplyToken; } public long getRxBytes() { return rxBytes; } - public long getDeliveredRxBytes() { - return deliveredRxBytes; - } - - public long getProbeReplyToken() { - return probeReplyToken; - } - public long getTxBytes() { return txBytes; } diff --git a/app/src/test/java/eu/faircode/netguard/PhysicalNetworkStateTest.java b/app/src/test/java/eu/faircode/netguard/PhysicalNetworkStateTest.java index 7c941b730..1297e95c7 100644 --- a/app/src/test/java/eu/faircode/netguard/PhysicalNetworkStateTest.java +++ b/app/src/test/java/eu/faircode/netguard/PhysicalNetworkStateTest.java @@ -2,9 +2,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; import android.net.LinkProperties; import android.net.Network; @@ -36,9 +34,8 @@ public void olderAndroidStoresIndependentCallbackSnapshots() throws Exception { LinkProperties props = linkProperties("9.9.9.9"); state.onPhysicalLinkPropertiesChanged(WIFI, props); props.setDnsServers(Collections.singleton(InetAddress.getByName("1.1.1.1"))); - assertEquals("9.9.9.9", state.getLinkProperties(WIFI).getDnsServers().get(0).getHostAddress()); assertEquals(NetworkReloadPolicy.REASON_LINK_PROPERTIES_CHANGED, - state.onPhysicalLinkPropertiesChanged(WIFI, props).getReason()); + state.onPhysicalLinkPropertiesChanged(WIFI, props)); } private static NetworkCapabilities capabilities(int transport) { @@ -62,20 +59,20 @@ public void vpnDefaultTransportHandoverReloadsWithoutVpnIdentityChurn() throws E NetworkCapabilities wifiVpn = capabilities(NetworkCapabilities.TRANSPORT_WIFI); Shadows.shadowOf(wifiVpn).removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN); Shadows.shadowOf(wifiVpn).addTransportType(NetworkCapabilities.TRANSPORT_VPN); - assertNull(state.onDefaultNetworkCapabilitiesChanged(VPN, wifiVpn).getReason()); + assertNull(state.onDefaultNetworkCapabilitiesChanged(VPN, wifiVpn)); NetworkCapabilities cellVpn = capabilities(NetworkCapabilities.TRANSPORT_CELLULAR); Shadows.shadowOf(cellVpn).removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN); Shadows.shadowOf(cellVpn).addTransportType(NetworkCapabilities.TRANSPORT_VPN); assertEquals(NetworkReloadPolicy.REASON_NETWORK_CHANGED, - state.onDefaultNetworkCapabilitiesChanged(VPN, cellVpn).getReason()); + state.onDefaultNetworkCapabilitiesChanged(VPN, cellVpn)); Network replacement = ShadowNetwork.newInstance(104); - assertNull(state.onDefaultNetworkAvailable(replacement).getReason()); - assertNull(state.onDefaultNetworkCapabilitiesChanged(replacement, cellVpn).getReason()); + assertNull(state.onDefaultNetworkAvailable(replacement)); + assertNull(state.onDefaultNetworkCapabilitiesChanged(replacement, cellVpn)); assertNull(state.onDefaultNetworkLinkPropertiesChanged(replacement, - linkProperties("9.9.9.9")).getReason()); - assertNull(state.onDefaultNetworkLost(VPN).getReason()); + linkProperties("9.9.9.9"))); + assertNull(state.onDefaultNetworkLost(VPN)); } @Test @@ -83,11 +80,11 @@ public void physicalCallbacksWorkWhileVpnIsDefault() { PhysicalNetworkState state = new PhysicalNetworkState(); assertEquals(NetworkReloadPolicy.REASON_NETWORK_AVAILABLE, - state.onPhysicalAvailable(WIFI).getReason()); + state.onPhysicalAvailable(WIFI)); assertEquals(NetworkReloadPolicy.REASON_NETWORK_CHANGED, state.onPhysicalCapabilitiesChanged(WIFI, capabilities( - NetworkCapabilities.TRANSPORT_WIFI)).getReason()); - assertNotNull(state.getCapabilities(WIFI)); + NetworkCapabilities.TRANSPORT_WIFI))); + assertNull(state.onPhysicalAvailable(WIFI)); assertNull(state.getDefaultNetwork()); } @@ -99,11 +96,11 @@ public void defaultSwitchIsDetectedWhenBothPhysicalNetworksRemain() { state.onPhysicalAvailable(CELL); state.onPhysicalCapabilitiesChanged(CELL, capabilities(NetworkCapabilities.TRANSPORT_CELLULAR)); - assertNull(state.onDefaultNetworkAvailable(WIFI).getReason()); + assertNull(state.onDefaultNetworkAvailable(WIFI)); assertNull(state.onDefaultNetworkCapabilitiesChanged(WIFI, - capabilities(NetworkCapabilities.TRANSPORT_WIFI)).getReason()); + capabilities(NetworkCapabilities.TRANSPORT_WIFI))); assertEquals(NetworkReloadPolicy.REASON_NETWORK_CHANGED, - state.onDefaultNetworkAvailable(CELL).getReason()); + state.onDefaultNetworkAvailable(CELL)); assertEquals(CELL, state.getDefaultNetwork()); } @@ -118,7 +115,7 @@ public void defaultPhysicalTransportChangeReloads() { assertEquals(NetworkReloadPolicy.REASON_NETWORK_CHANGED, state.onDefaultNetworkCapabilitiesChanged(WIFI, - capabilities(NetworkCapabilities.TRANSPORT_CELLULAR)).getReason()); + capabilities(NetworkCapabilities.TRANSPORT_CELLULAR))); assertEquals(WIFI, state.getDefaultNetwork()); } @@ -134,7 +131,7 @@ public void signalAndBandwidthChatterDoesNotReload() throws Exception { chatterShadow.setLinkDownstreamBandwidthKbps(12000); chatterShadow.setLinkUpstreamBandwidthKbps(3000); setSignalStrength(chatter, -55); - assertNull(state.onPhysicalCapabilitiesChanged(CELL, chatter).getReason()); + assertNull(state.onPhysicalCapabilitiesChanged(CELL, chatter)); } @Test @@ -146,9 +143,9 @@ public void standbyLossIsTrackedAndStaleLossIsIgnored() { state.onPhysicalCapabilitiesChanged(CELL, capabilities(NetworkCapabilities.TRANSPORT_CELLULAR)); assertEquals(NetworkReloadPolicy.REASON_NETWORK_LOST, - state.onPhysicalLost(CELL).getReason()); - assertNull(state.onPhysicalLost(CELL).getReason()); - assertNotNull(state.getCapabilities(WIFI)); + state.onPhysicalLost(CELL)); + assertNull(state.onPhysicalLost(CELL)); + assertNull(state.onPhysicalAvailable(WIFI)); } @Test @@ -161,10 +158,9 @@ public void privateDnsOnlyChangeDoesNotRestartWireGuard() throws Exception { LinkProperties pinned = linkProperties("9.9.9.9"); setPrivateDns(pinned, "dns.example", true); - PhysicalNetworkState.Change change = state.onPhysicalLinkPropertiesChanged(WIFI, pinned); - assertEquals(NetworkReloadPolicy.REASON_PRIVATE_DNS_CHANGED, change.getReason()); - assertTrue(change.isPrivateDnsChanged()); - assertFalse(NetworkReloadPolicy.shouldRestartWireGuard(change.getReason())); + String change = state.onPhysicalLinkPropertiesChanged(WIFI, pinned); + assertEquals(NetworkReloadPolicy.REASON_PRIVATE_DNS_CHANGED, change); + assertFalse(NetworkReloadPolicy.shouldRestartWireGuard(change)); } @Test @@ -180,11 +176,11 @@ public void vpnDefaultCallbacksDoNotCreatePhysicalDefault() throws Exception { vpnShadow.addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET); vpnShadow.removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN); vpnShadow.addTransportType(NetworkCapabilities.TRANSPORT_VPN); - assertNull(state.onDefaultNetworkAvailable(VPN).getReason()); - assertNull(state.onDefaultNetworkCapabilitiesChanged(VPN, vpn).getReason()); + assertNull(state.onDefaultNetworkAvailable(VPN)); + assertNull(state.onDefaultNetworkCapabilitiesChanged(VPN, vpn)); assertNull(state.onDefaultNetworkLinkPropertiesChanged(VPN, - linkProperties("1.1.1.1")).getReason()); - assertNull(state.onDefaultNetworkLost(VPN).getReason()); + linkProperties("1.1.1.1"))); + assertNull(state.onDefaultNetworkLost(VPN)); assertEquals(WIFI, state.getDefaultNetwork()); } diff --git a/app/src/test/java/net/kollnig/missioncontrol/wg/WgConnectivityCheckerTest.kt b/app/src/test/java/net/kollnig/missioncontrol/wg/WgConnectivityCheckerTest.kt index d038d5cc3..0ae41130b 100644 --- a/app/src/test/java/net/kollnig/missioncontrol/wg/WgConnectivityCheckerTest.kt +++ b/app/src/test/java/net/kollnig/missioncontrol/wg/WgConnectivityCheckerTest.kt @@ -24,9 +24,8 @@ class WgConnectivityCheckerTest { tx: Long, freshHandshake: Boolean = false, tunFailuresTotal: Long = 0L, - tunFailuresStreak: Long = 0L, - deliveredRxBytes: Long = rx - ) = WgStats(rx, tx, 0L, freshHandshake, tunFailuresTotal, tunFailuresStreak, deliveredRxBytes) + tunFailuresStreak: Long = 0L + ) = WgStats(rx, tx, 0L, freshHandshake, tunFailuresTotal, tunFailuresStreak) // --- baseline / connecting ------------------------------------------ @@ -310,24 +309,16 @@ class WgConnectivityCheckerTest { c.tick(3000, stats(0, 20, freshHandshake = true)) assertEquals(false, c.lastTickSawRx) - // Raw WireGuard rx bytes are not proof that decrypted traffic reached the TUN. - c.tick(4000, stats(50, 20, deliveredRxBytes = 0)) - assertEquals(false, c.lastTickSawRx) - - // A full TUN write advances the dedicated delivered counter. - c.tick(5000, stats(50, 20, deliveredRxBytes = 7)) + // Return traffic arrives. + c.tick(4000, stats(50, 20)) assertEquals(true, c.lastTickSawRx) // Back to idle: the flag reflects the latest tick only. - c.tick(5500, stats(50, 20, deliveredRxBytes = 7)) - assertEquals(false, c.lastTickSawRx) - - // Raw rx and a fresh handshake alone still do not count. - c.tick(6000, stats(80, 30, freshHandshake = true, deliveredRxBytes = 7)) + c.tick(5000, stats(50, 20)) assertEquals(false, c.lastTickSawRx) - // Delivered traffic during a fresh-handshake tick does count. - c.tick(7000, stats(80, 30, freshHandshake = true, deliveredRxBytes = 8)) + // rx advancing during a fresh-handshake tick still counts. + c.tick(6000, stats(80, 30, freshHandshake = true)) assertEquals(true, c.lastTickSawRx) } diff --git a/app/src/test/java/net/kollnig/missioncontrol/wg/WgConnectivityMonitorTest.kt b/app/src/test/java/net/kollnig/missioncontrol/wg/WgConnectivityMonitorTest.kt index 88c2c8b3a..9f6f5f43d 100644 --- a/app/src/test/java/net/kollnig/missioncontrol/wg/WgConnectivityMonitorTest.kt +++ b/app/src/test/java/net/kollnig/missioncontrol/wg/WgConnectivityMonitorTest.kt @@ -38,27 +38,6 @@ class WgConnectivityMonitorTest { assertEquals(1_000L, WgConnectivityMonitor.pollIntervalMs(true)) } - @Test - fun sampleCallbackRunsThroughTheMonitorGenerationGate() { - var samples = 0 - var monitor: WgConnectivityMonitor? = null - monitor = WgConnectivityMonitor( - statsProvider = { stats() }, - prod = {}, - onBroken = { fail("sample callback test entered recovery") }, - onSample = { - samples++ - monitor!!.stop() - }, - sleep = {}, - clock = { 0L } - ) - - monitor.start() - awaitStopped(monitor) - assertEquals(1, samples) - } - @Test fun screenOffCadenceIsSlow() { val idle = WgConnectivityMonitor.pollIntervalMs(false) diff --git a/app/src/test/java/net/kollnig/missioncontrol/wg/WgEgressRecoveryTest.java b/app/src/test/java/net/kollnig/missioncontrol/wg/WgEgressRecoveryTest.java index bd2c2cec9..d35388511 100644 --- a/app/src/test/java/net/kollnig/missioncontrol/wg/WgEgressRecoveryTest.java +++ b/app/src/test/java/net/kollnig/missioncontrol/wg/WgEgressRecoveryTest.java @@ -1,5 +1,15 @@ package net.kollnig.missioncontrol.wg; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.lang.reflect.Constructor; +import java.lang.reflect.Field; +import java.util.concurrent.atomic.AtomicInteger; + +import net.kollnig.missioncontrol.wgbridge.Tunnel; + /** * Screen-state keepalive toggling for the WireGuard egress. * @@ -21,6 +31,85 @@ public void interactiveStateChangeIsNoopWhenConfigIsMissing() { WgEgress.INSTANCE.onInteractiveStateChanged(true, "", true, false); } + @org.junit.Test + public void underlyingNetworkChangeIsNoopWhenWireGuardIsIdle() throws Exception { + Field tunnel = field("tunnel"); + Field pending = field("forceRestartPending"); + Field attempts = field("restartAttempts"); + Field generation = field("verificationGeneration"); + Field reload = field("requestReloadCb"); + Object oldTunnel = tunnel.get(WgEgress.INSTANCE); + boolean oldPending = pending.getBoolean(WgEgress.INSTANCE); + int oldAttempts = attempts.getInt(WgEgress.INSTANCE); + long oldGeneration = generation.getLong(WgEgress.INSTANCE); + Object oldReload = reload.get(WgEgress.INSTANCE); + AtomicInteger reloads = new AtomicInteger(); + try { + tunnel.set(WgEgress.INSTANCE, null); + pending.setBoolean(WgEgress.INSTANCE, false); + attempts.setInt(WgEgress.INSTANCE, 7); + reload.set(WgEgress.INSTANCE, (Runnable) reloads::incrementAndGet); + + WgEgress.INSTANCE.onUnderlyingNetworkChanged(); + + assertFalse(pending.getBoolean(WgEgress.INSTANCE)); + assertEquals(7, attempts.getInt(WgEgress.INSTANCE)); + assertEquals(0, reloads.get()); + } finally { + tunnel.set(WgEgress.INSTANCE, oldTunnel); + pending.setBoolean(WgEgress.INSTANCE, oldPending); + attempts.setInt(WgEgress.INSTANCE, oldAttempts); + generation.setLong(WgEgress.INSTANCE, oldGeneration); + reload.set(WgEgress.INSTANCE, oldReload); + } + } + + @org.junit.Test + public void underlyingNetworkChangeMarksRunningTunnelForOneRestart() throws Exception { + Field tunnel = field("tunnel"); + Field pending = field("forceRestartPending"); + Field attempts = field("restartAttempts"); + Field generation = field("verificationGeneration"); + Field reload = field("requestReloadCb"); + Object oldTunnel = tunnel.get(WgEgress.INSTANCE); + boolean oldPending = pending.getBoolean(WgEgress.INSTANCE); + int oldAttempts = attempts.getInt(WgEgress.INSTANCE); + long oldGeneration = generation.getLong(WgEgress.INSTANCE); + Object oldReload = reload.get(WgEgress.INSTANCE); + AtomicInteger reloads = new AtomicInteger(); + try { + tunnel.set(WgEgress.INSTANCE, newTunnel(0L)); + pending.setBoolean(WgEgress.INSTANCE, false); + attempts.setInt(WgEgress.INSTANCE, 11); + reload.set(WgEgress.INSTANCE, (Runnable) reloads::incrementAndGet); + + WgEgress.INSTANCE.onUnderlyingNetworkChanged(); + WgEgress.INSTANCE.onUnderlyingNetworkChanged(); + + assertTrue(pending.getBoolean(WgEgress.INSTANCE)); + assertEquals(11, attempts.getInt(WgEgress.INSTANCE)); + assertEquals(0, reloads.get()); + } finally { + tunnel.set(WgEgress.INSTANCE, oldTunnel); + pending.setBoolean(WgEgress.INSTANCE, oldPending); + attempts.setInt(WgEgress.INSTANCE, oldAttempts); + generation.setLong(WgEgress.INSTANCE, oldGeneration); + reload.set(WgEgress.INSTANCE, oldReload); + } + } + + private static Field field(String name) throws Exception { + Field field = WgEgress.class.getDeclaredField(name); + field.setAccessible(true); + return field; + } + + private static Tunnel newTunnel(long handle) throws Exception { + Constructor constructor = Tunnel.class.getDeclaredConstructor(long.class); + constructor.setAccessible(true); + return constructor.newInstance(handle); + } + private static String validConfig() { String key = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA="; return "[Interface]\n" + diff --git a/app/src/test/java/net/kollnig/missioncontrol/wg/WgHandoverVerifierTest.kt b/app/src/test/java/net/kollnig/missioncontrol/wg/WgHandoverVerifierTest.kt deleted file mode 100644 index ff2692ac3..000000000 --- a/app/src/test/java/net/kollnig/missioncontrol/wg/WgHandoverVerifierTest.kt +++ /dev/null @@ -1,190 +0,0 @@ -package net.kollnig.missioncontrol.wg - -import org.junit.Assert.assertEquals -import org.junit.Assert.assertFalse -import org.junit.Assert.assertTrue -import org.junit.Test - -class WgHandoverVerifierTest { - private val target = WgProbeTarget("10.0.0.2", "1.1.1.1") - - private fun stats(delivered: Long = 0L, token: Long = 0L) = WgStats( - rxBytes = 0L, - txBytes = 0L, - latestHandshakeMillis = 0L, - deliveredRxBytes = delivered, - probeReplyToken = token - ) - - @Test - fun startsWithProbeThenRetriesAtFiveSecondCadenceAndRestartsAtDeadline() { - val verifier = WgHandoverVerifier() - val first = verifier.begin(7, stats(10), listOf(target), true, now = 0L) - assertTrue(first is WgHandoverVerifier.Action.Probe) - assertTrue(verifier.onSample(7, stats(10), true, now = 4_999L) is WgHandoverVerifier.Action.None) - assertTrue(verifier.onSample(7, stats(10), true, now = 5_000L) is WgHandoverVerifier.Action.Probe) - assertTrue(verifier.onSample(7, stats(10), true, now = 10_000L) is WgHandoverVerifier.Action.Probe) - assertTrue(verifier.onSample(7, stats(10), true, now = 14_999L) is WgHandoverVerifier.Action.None) - assertTrue(verifier.onSample(7, stats(10), true, now = 15_000L) is WgHandoverVerifier.Action.Restart) - assertFalse(verifier.isPending()) - } - - @Test - fun deliveredTrafficConfirmsHandoverWithoutProbeReply() { - val verifier = WgHandoverVerifier() - verifier.begin(1, stats(10), listOf(target), true, now = 0L) - assertTrue(verifier.onSample(1, stats(11), true, now = 1_000L) is WgHandoverVerifier.Action.None) - assertFalse(verifier.isPending()) - } - - @Test - fun matchingProbeTokenConfirmsHandover() { - val verifier = WgHandoverVerifier() - val action = verifier.begin(2, stats(), listOf(target), true, now = 0L) - as WgHandoverVerifier.Action.Probe - assertTrue(verifier.onSample(2, stats(token = action.token), true, now = 1_000L) - is WgHandoverVerifier.Action.None) - assertFalse(verifier.isPending()) - } - - @Test - fun screenOffDefersAndRebasesVerification() { - val verifier = WgHandoverVerifier() - assertTrue(verifier.begin(3, stats(10), listOf(target), false, now = 0L) - is WgHandoverVerifier.Action.None) - assertTrue(verifier.onSample(3, stats(20), false, now = 20_000L) - is WgHandoverVerifier.Action.None) - assertTrue(verifier.onSample(3, stats(20), true, now = 21_000L) - is WgHandoverVerifier.Action.Probe) - assertTrue(verifier.onSample(3, stats(20), true, now = 26_000L) - is WgHandoverVerifier.Action.Probe) - assertTrue(verifier.onSample(3, stats(20), true, now = 31_000L) - is WgHandoverVerifier.Action.Probe) - assertTrue(verifier.onSample(3, stats(20), true, now = 35_999L) - is WgHandoverVerifier.Action.None) - assertTrue(verifier.onSample(3, stats(20), true, now = 36_000L) - is WgHandoverVerifier.Action.Restart) - } - - @Test - fun interactiveTransitionRebasesDeadline() { - val verifier = WgHandoverVerifier() - verifier.begin(8, stats(), listOf(target), true, now = 0L) - assertTrue(verifier.onSample(8, stats(), true, now = 1_000L) - is WgHandoverVerifier.Action.None) - - verifier.setInteractive(false) - // No monitor poll occurs while off: the explicit event must retain - // the suspension even after the screen comes back on. - verifier.setInteractive(true) - assertTrue(verifier.onSample(8, stats(), true, now = 11_000L) - is WgHandoverVerifier.Action.Probe) - assertTrue(verifier.onSample(8, stats(), true, now = 16_000L) - is WgHandoverVerifier.Action.Probe) - assertTrue(verifier.onSample(8, stats(), true, now = 21_000L) - is WgHandoverVerifier.Action.Probe) - assertTrue(verifier.onSample(8, stats(), true, now = 25_999L) - is WgHandoverVerifier.Action.None) - assertTrue(verifier.onSample(8, stats(), true, now = 26_000L) - is WgHandoverVerifier.Action.Restart) - } - - @Test - fun suspendedStateRebasesDeadlineOnResume() { - val verifier = WgHandoverVerifier() - verifier.begin(9, stats(), listOf(target), true, now = 0L) - assertTrue(verifier.onSample(9, stats(), true, now = 0L) - is WgHandoverVerifier.Action.None) - - verifier.onSuspended() - assertTrue(verifier.onSample(9, stats(), true, now = 100_000L) - is WgHandoverVerifier.Action.Probe) - assertTrue(verifier.onSample(9, stats(), true, now = 105_000L) - is WgHandoverVerifier.Action.Probe) - assertTrue(verifier.onSample(9, stats(), true, now = 110_000L) - is WgHandoverVerifier.Action.Probe) - assertTrue(verifier.onSample(9, stats(), true, now = 114_999L) - is WgHandoverVerifier.Action.None) - assertTrue(verifier.onSample(9, stats(), true, now = 115_000L) - is WgHandoverVerifier.Action.Restart) - } - - @Test - fun newBeginInvalidatesQueuedReplyFromPreviousGeneration() { - val verifier = WgHandoverVerifier() - val oldProbe = verifier.begin(10, stats(), listOf(target), true, now = 0L) - as WgHandoverVerifier.Action.Probe - val newProbe = verifier.begin(11, stats(), listOf(target), true, now = 100L) - as WgHandoverVerifier.Action.Probe - - assertFalse(oldProbe.token == newProbe.token) - assertTrue(verifier.onSample(11, stats(token = oldProbe.token), true, now = 101L) - is WgHandoverVerifier.Action.None) - assertTrue(verifier.isPending()) - assertTrue(verifier.onSample(11, stats(token = newProbe.token), true, now = 102L) - is WgHandoverVerifier.Action.None) - assertFalse(verifier.isPending()) - } - - @Test - fun staleGenerationCannotConfirmOrRestart() { - val verifier = WgHandoverVerifier() - verifier.begin(4, stats(), listOf(target), true, now = 0L) - assertTrue(verifier.onSample(5, stats(100), true, now = 20_000L) - is WgHandoverVerifier.Action.None) - assertTrue(verifier.isPending()) - } - - @Test - fun unsupportedTargetsLeavePassiveWatchdogAlone() { - val verifier = WgHandoverVerifier() - assertTrue(verifier.begin(6, stats(), emptyList(), true, now = 0L) - is WgHandoverVerifier.Action.None) - assertFalse(verifier.isPending()) - } - - @Test - fun selectorRejectsMixedFamilyAndUnroutedResolvers() { - val targets = WgProbeTargetSelector.select( - sourceAddresses = listOf("10.0.0.2/32", "2001:db8::2/128"), - resolverAddresses = listOf("1.1.1.1", "2001:4860:4860::8888", "resolver.example"), - allowedIps = listOf("0.0.0.0/0", "2001::/16") - ) - assertEquals( - listOf( - WgProbeTarget("10.0.0.2", "1.1.1.1"), - WgProbeTarget("2001:db8::2", "2001:4860:4860::8888") - ), - targets - ) - } - - @Test - fun selectorRejectsResolverOutsideAllowedIps() { - assertTrue( - WgProbeTargetSelector.select( - listOf("10.0.0.2/32"), - listOf("1.1.1.1"), - listOf("10.0.0.0/8") - ).isEmpty() - ) - } - - @Test - fun selectorRejectsSpecialDestinationsAndMalformedRoutes() { - assertTrue( - WgProbeTargetSelector.select( - listOf("0.0.0.0/32"), - listOf("224.0.0.1"), - listOf("0.0.0.0/0") - ).isEmpty() - ) - assertTrue( - WgProbeTargetSelector.select( - listOf("127.0.0.1/32"), - listOf("1.1.1.1"), - listOf("0.0.0.0/not-a-prefix") - ).isEmpty() - ) - } -} diff --git a/wgbridge-rs/README.md b/wgbridge-rs/README.md index b9b266a07..175ed2b89 100644 --- a/wgbridge-rs/README.md +++ b/wgbridge-rs/README.md @@ -188,7 +188,6 @@ class Tunnel { long latestHandshakeMillis(); void sendKeepalive(); void rebind(); // re-bind + re-protect UDP sockets (roaming) - boolean sendDnsProbe(String sourceIp, String resolverIp, long token); void updateEndpoint(String peerPublicKeyBase64, String endpoint); void setKeepalive(String peerPublicKeyBase64, int seconds); // 0 disables void stop(); @@ -200,24 +199,6 @@ class Tunnel { endpoints must arrive as resolved IP literals (`WgEgress` resolves hostnames, and re-resolves them on network changes via `updateEndpoint`). -`TunnelStats.rxBytes` is the engine counter and includes handshakes. -`deliveredRxBytes` counts only decrypted IP packets successfully written to -Android; use it for data-path recovery evidence. `probeReplyToken` identifies -the last correlated internal DNS probe reply. Probes enter the same encrypted -IP transport as outbound application packets. Their sockets only reserve a -source port and never send on the physical network. Matching replies are -consumed before DNS policy and TUN delivery, so they do not inflate the -delivered-IP counter. Probe destinations must be covered by a peer's AllowedIPs. - -After an underlying-network change, Android first rebinds the protected UDP -sockets and refreshes endpoints. While interactive, it then verifies the path -with up to three root-NS DNS queries at five-second intervals. Delivered -application traffic or a correlated reply ends verification; fifteen seconds -without either requests a full restart through the existing backoff, followed -by verification of the replacement. Screen-off and suspend gaps defer/rebase -the check. Profiles without a same-family, routed numeric resolver retain the -ordinary watchdog. No probe opens a direct fallback path. - ## Potential improvements - **Split DNS-over-TCP rewriting**: inbound TCP DNS is recorded only when the diff --git a/wgbridge-rs/src/jni_bindings.rs b/wgbridge-rs/src/jni_bindings.rs index 14339a1c1..53f7cbcaa 100644 --- a/wgbridge-rs/src/jni_bindings.rs +++ b/wgbridge-rs/src/jni_bindings.rs @@ -336,8 +336,6 @@ pub extern "system" fn Java_net_kollnig_missioncontrol_wgbridge_Tunnel_nativeSta stats.latest_handshake_millis, stats.tun_write_failures_total, stats.tun_write_failures_streak, - stats.delivered_rx_bytes, - stats.probe_reply_token, ]; match env.new_long_array(values.len()) { Ok(array) => { @@ -373,30 +371,6 @@ pub extern "system" fn Java_net_kollnig_missioncontrol_wgbridge_Tunnel_nativeSen }) } -#[no_mangle] -pub extern "system" fn Java_net_kollnig_missioncontrol_wgbridge_Tunnel_nativeSendDnsProbe( - mut unowned_env: EnvUnowned, - _class: JClass, - handle: jlong, - source: JString, - resolver: JString, - token: jlong, -) -> jni::sys::jboolean { - with_native_env!(unowned_env, env, { - let Some(tunnel) = tunnel_from_handle(handle) else { return false; }; - let (Some(source), Some(resolver)) = (get_string(env, &source), get_string(env, &resolver)) else { - return false; - }; - match tunnel.send_dns_probe(&source, &resolver, token) { - Ok(sent) => sent, - Err(error) => { - log::warn!("could not queue WireGuard DNS probe: {error}"); - false - } - } - }) -} - #[no_mangle] pub extern "system" fn Java_net_kollnig_missioncontrol_wgbridge_Tunnel_nativeRebind( mut unowned_env: EnvUnowned, diff --git a/wgbridge-rs/src/lib.rs b/wgbridge-rs/src/lib.rs index 02aa49928..531820e1a 100644 --- a/wgbridge-rs/src/lib.rs +++ b/wgbridge-rs/src/lib.rs @@ -21,7 +21,6 @@ pub mod config; pub mod dns; pub mod keys; pub mod policy; -pub mod probe; // The C ABI the NetGuard engine links against. It lives in this crate, not // in tc-dns, so the cdylib itself defines the exported symbols rather than // inheriting them from an rlib dependency (see app/gradle/wgbridge.gradle). diff --git a/wgbridge-rs/src/probe.rs b/wgbridge-rs/src/probe.rs deleted file mode 100644 index a07d1a901..000000000 --- a/wgbridge-rs/src/probe.rs +++ /dev/null @@ -1,329 +0,0 @@ -//! Bounded, in-tunnel DNS reachability probes. No socket sends bypass the VPN. -//! The UDP socket below only reserves a source port; the packet enters gotatun -//! through IpRecv and its correlated reply is consumed before reaching Android. - -use std::io; -use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr, UdpSocket}; -use std::sync::atomic::{AtomicBool, AtomicI64, Ordering}; -use std::sync::Mutex; -use std::time::{Duration, Instant}; - -const QUESTION: [u8; 5] = [0, 0, 2, 0, 1]; // root NS IN; no user hostname -pub const PROBE_LIFETIME: Duration = Duration::from_secs(5); - -struct Pending { - source: IpAddr, - resolver: IpAddr, - port: u16, - id: [u8; 2], - token: i64, - expires: Instant, - _reservation: UdpSocket, -} - -#[derive(Default)] -pub struct ProbeTracker { - active: AtomicBool, - pending: Mutex>, - reply_token: AtomicI64, -} - -impl ProbeTracker { - pub fn reply_token(&self) -> i64 { - self.reply_token.load(Ordering::Acquire) - } - - pub fn prepare(&self, source: IpAddr, resolver: IpAddr, token: i64) -> io::Result> { - if source.is_ipv4() != resolver.is_ipv4() - || source.is_unspecified() - || source.is_multicast() - || resolver.is_unspecified() - || resolver.is_multicast() - || resolver.is_loopback() - || token <= 0 - { - return Err(io::Error::other("invalid DNS probe addresses or token")); - } - let bind_ip = match source { - IpAddr::V4(_) => IpAddr::V4(Ipv4Addr::UNSPECIFIED), - IpAddr::V6(_) => IpAddr::V6(Ipv6Addr::UNSPECIFIED), - }; - let reservation = UdpSocket::bind(SocketAddr::new(bind_ip, 0))?; - let port = reservation.local_addr()?.port(); - let mut id = [0; 2]; - getrandom::fill(&mut id).map_err(io::Error::other)?; - let packet = dns_packet(source, resolver, port, id); - let mut pending = self - .pending - .lock() - .map_err(|_| io::Error::other("probe lock poisoned"))?; - *pending = Some(Pending { - source, - resolver, - port, - id, - token, - expires: Instant::now() + PROBE_LIFETIME, - _reservation: reservation, - }); - self.active.store(true, Ordering::Release); - Ok(packet) - } - - pub fn expire(&self) { - if let Ok(mut pending) = self.pending.lock() { - if pending - .as_ref() - .is_some_and(|p| Instant::now() >= p.expires) - { - *pending = None; - self.active.store(false, Ordering::Release); - } - } - } - - /// Returns true only for our current, unexpired DNS transaction. Ordinary - /// application packets take one atomic load when no probe is outstanding. - pub fn consume_reply(&self, packet: &[u8]) -> bool { - if !self.active.load(Ordering::Acquire) { - return false; - } - let Ok(mut pending) = self.pending.lock() else { - return false; - }; - let Some(probe) = pending.as_ref() else { - return false; - }; - if Instant::now() >= probe.expires { - *pending = None; - self.active.store(false, Ordering::Release); - return false; - } - let Some((source, dest, udp)) = udp_payload(packet) else { - return false; - }; - if source != probe.resolver - || dest != probe.source - || udp.len() < 25 - || udp[0..2] != 53u16.to_be_bytes() - || udp[2..4] != probe.port.to_be_bytes() - || usize::from(u16::from_be_bytes([udp[4], udp[5]])) != udp.len() - { - return false; - } - let dns = &udp[8..]; - if dns[0..2] != probe.id - || dns[2] & 0xf8 != 0x80 - || dns[4..6] != [0, 1] - || dns[12..17] != QUESTION - { - return false; - } - // Any DNS rcode proves a round trip; this is not a resolver health test. - self.reply_token.store(probe.token, Ordering::Release); - *pending = None; - self.active.store(false, Ordering::Release); - true - } -} - -fn udp_payload(packet: &[u8]) -> Option<(IpAddr, IpAddr, &[u8])> { - match packet.first()? >> 4 { - 4 if packet.len() >= 20 => { - let header = usize::from(packet[0] & 15) * 4; - let len = usize::from(u16::from_be_bytes([packet[2], packet[3]])); - if header < 20 - || len != packet.len() - || header > len - || packet[9] != 17 - || u16::from_be_bytes([packet[6], packet[7]]) & 0x3fff != 0 - { - return None; - } - Some(( - Ipv4Addr::new(packet[12], packet[13], packet[14], packet[15]).into(), - Ipv4Addr::new(packet[16], packet[17], packet[18], packet[19]).into(), - &packet[header..], - )) - } - 6 if packet.len() >= 40 => { - if packet[6] != 17 - || usize::from(u16::from_be_bytes([packet[4], packet[5]])) + 40 != packet.len() - { - return None; - } - let source: [u8; 16] = packet[8..24].try_into().ok()?; - let dest: [u8; 16] = packet[24..40].try_into().ok()?; - Some(( - Ipv6Addr::from(source).into(), - Ipv6Addr::from(dest).into(), - &packet[40..], - )) - } - _ => None, - } -} - -fn checksum(bytes: &[u8]) -> u16 { - let sum = bytes.chunks(2).fold(0u32, |sum, pair| { - sum + (u32::from(pair[0]) << 8) + u32::from(*pair.get(1).unwrap_or(&0)) - }); - let sum = (sum & 0xffff) + (sum >> 16); - !((sum & 0xffff) + (sum >> 16)) as u16 -} - -fn dns_packet(source: IpAddr, resolver: IpAddr, port: u16, id: [u8; 2]) -> Vec { - let mut udp = vec![0u8; 25]; - udp[0..2].copy_from_slice(&port.to_be_bytes()); - udp[2..4].copy_from_slice(&53u16.to_be_bytes()); - udp[4..6].copy_from_slice(&25u16.to_be_bytes()); - udp[8..10].copy_from_slice(&id); - udp[10] = 1; // recursion desired - udp[13] = 1; // one question - udp[20..25].copy_from_slice(&QUESTION); - let mut pseudo = Vec::new(); - let mut ip = match (source, resolver) { - (IpAddr::V4(src), IpAddr::V4(dst)) => { - let mut ip = vec![0u8; 20]; - ip[0] = 0x45; - ip[2..4].copy_from_slice(&45u16.to_be_bytes()); - ip[8] = 64; - ip[9] = 17; - ip[12..16].copy_from_slice(&src.octets()); - ip[16..20].copy_from_slice(&dst.octets()); - let check = checksum(&ip); - ip[10..12].copy_from_slice(&check.to_be_bytes()); - pseudo.extend_from_slice(&src.octets()); - pseudo.extend_from_slice(&dst.octets()); - pseudo.extend_from_slice(&[0, 17, 0, 25]); - ip - } - (IpAddr::V6(src), IpAddr::V6(dst)) => { - let mut ip = vec![0u8; 40]; - ip[0] = 0x60; - ip[4..6].copy_from_slice(&25u16.to_be_bytes()); - ip[6] = 17; - ip[7] = 64; - ip[8..24].copy_from_slice(&src.octets()); - ip[24..40].copy_from_slice(&dst.octets()); - pseudo.extend_from_slice(&src.octets()); - pseudo.extend_from_slice(&dst.octets()); - pseudo.extend_from_slice(&[0, 0, 0, 25, 0, 0, 0, 17]); - ip - } - _ => return Vec::new(), // prepare rejects mixed families - }; - pseudo.extend_from_slice(&udp); - let check = checksum(&pseudo); - udp[6..8].copy_from_slice(&(if check == 0 { 0xffff } else { check }).to_be_bytes()); - ip.extend_from_slice(&udp); - ip -} - -#[cfg(test)] -pub(crate) fn test_reply(query: &[u8]) -> Vec { - let mut reply = query.to_vec(); - let header = if query[0] >> 4 == 4 { 20 } else { 40 }; - if header == 20 { - reply[12..16].copy_from_slice(&query[16..20]); - reply[16..20].copy_from_slice(&query[12..16]); - } else { - reply[8..24].copy_from_slice(&query[24..40]); - reply[24..40].copy_from_slice(&query[8..24]); - } - reply[header..header + 2].copy_from_slice(&query[header + 2..header + 4]); - reply[header + 2..header + 4].copy_from_slice(&query[header..header + 2]); - reply[header + 10] |= 0x80; - reply -} - -#[cfg(test)] -mod tests { - use super::*; - - fn addresses(v6: bool) -> (IpAddr, IpAddr) { - if v6 { - ( - "2001:db8::2".parse().unwrap(), - "2001:db8::53".parse().unwrap(), - ) - } else { - ("10.0.0.2".parse().unwrap(), "10.0.0.53".parse().unwrap()) - } - } - - #[test] - fn queries_have_valid_ip_and_udp_checksums_in_both_families() { - for v6 in [false, true] { - let (source, resolver) = addresses(v6); - let packet = dns_packet(source, resolver, 40000, [0x12, 0x34]); - let header = if v6 { 40 } else { 20 }; - let mut pseudo = Vec::new(); - if v6 { - pseudo.extend_from_slice(&packet[8..40]); - pseudo.extend_from_slice(&[0, 0, 0, 25, 0, 0, 0, 17]); - } else { - assert_eq!(checksum(&packet[..20]), 0); - pseudo.extend_from_slice(&packet[12..20]); - pseudo.extend_from_slice(&[0, 17, 0, 25]); - } - pseudo.extend_from_slice(&packet[header..]); - assert_eq!(checksum(&pseudo), 0); - assert_eq!(&packet[header + 20..], &QUESTION); - assert_eq!(&packet[header + 8..header + 10], &[0x12, 0x34]); - } - } - - #[test] - fn only_current_correlated_reply_is_consumed_once() { - for v6 in [false, true] { - let tracker = ProbeTracker::default(); - let (source, resolver) = addresses(v6); - let query = tracker.prepare(source, resolver, 11).unwrap(); - assert!(!tracker.consume_reply(&query)); - let reply = test_reply(&query); - let header = if v6 { 40 } else { 20 }; - for index in [header, header + 2, header + 4, header + 8, header + 20] { - let mut malformed = reply.clone(); - malformed[index] ^= 1; - assert!(!tracker.consume_reply(&malformed)); - } - for len in 0..reply.len() { - assert!(!tracker.consume_reply(&reply[..len])); - } - assert_eq!(tracker.reply_token(), 0); - assert!(tracker.consume_reply(&reply)); - assert_eq!(tracker.reply_token(), 11); - assert!(!tracker.consume_reply(&reply)); - } - } - - #[test] - fn replaced_and_expired_probes_cannot_confirm_new_attempts() { - let tracker = ProbeTracker::default(); - let (source, resolver) = addresses(false); - let old = tracker.prepare(source, resolver, 1).unwrap(); - let new = tracker.prepare(source, resolver, 2).unwrap(); - assert!(!tracker.consume_reply(&test_reply(&old))); - tracker.pending.lock().unwrap().as_mut().unwrap().expires = Instant::now(); - assert!(!tracker.consume_reply(&test_reply(&new))); - assert_eq!(tracker.reply_token(), 0); - assert!(!tracker.active.load(Ordering::Acquire)); - tracker.prepare(source, resolver, 3).unwrap(); - tracker.pending.lock().unwrap().as_mut().unwrap().expires = Instant::now(); - tracker.expire(); - assert!(tracker.pending.lock().unwrap().is_none()); - } - - #[test] - fn invalid_probe_addresses_are_rejected() { - let (source, resolver) = addresses(false); - let tracker = ProbeTracker::default(); - for invalid in ["::1", "0.0.0.0", "127.0.0.1", "224.0.0.1"] { - assert!(tracker - .prepare(source, invalid.parse().unwrap(), 1) - .is_err()); - } - assert!(tracker.prepare(source, resolver, 0).is_err()); - } -} diff --git a/wgbridge-rs/src/transport/ip_recv.rs b/wgbridge-rs/src/transport/ip_recv.rs index 63ef785b7..a83d2e154 100644 --- a/wgbridge-rs/src/transport/ip_recv.rs +++ b/wgbridge-rs/src/transport/ip_recv.rs @@ -16,23 +16,16 @@ const MAX_BATCH: usize = 32; pub struct SocketpairRecv { afd: AsyncFd, mtu: MtuWatcher, - probes: tokio::sync::mpsc::Receiver>, } impl SocketpairRecv { /// Takes ownership of `fd` (already a private dup). Sets it non-blocking /// for use with the tokio reactor. pub fn new(fd: OwnedFd, mtu: u16) -> io::Result { - let (_, probes) = tokio::sync::mpsc::channel(1); - Self::with_probes(fd, mtu, probes) - } - - pub fn with_probes(fd: OwnedFd, mtu: u16, probes: tokio::sync::mpsc::Receiver>) -> io::Result { set_nonblocking(&fd)?; Ok(Self { afd: AsyncFd::with_interest(fd, Interest::READABLE)?, mtu: MtuWatcher::new(mtu), - probes, }) } } @@ -62,15 +55,7 @@ impl IpRecv for SocketpairRecv { pool: &mut PacketBufPool, ) -> io::Result> + Send + 'a> { loop { - let mut guard = tokio::select! { - Some(bytes) = self.probes.recv() => { - if let Ok(packet) = Packet::copy_from(bytes.as_slice()).try_into_ip() { - return Ok(vec![packet].into_iter()); - } - continue; - } - ready = self.afd.readable() => ready?, - }; + let mut guard = self.afd.readable().await?; let fd = self.afd.get_ref().as_raw_fd(); let mut packets: Vec> = Vec::new(); @@ -124,32 +109,3 @@ impl IpRecv for SocketpairRecv { self.mtu.clone() } } - -#[cfg(test)] -mod tests { - use super::*; - use std::os::unix::net::UnixDatagram; - use std::time::Duration; - - #[tokio::test] - async fn probes_and_normal_packets_share_recv_without_channel_close_stopping_it() { - let (writer, reader) = UnixDatagram::pair().unwrap(); - let (tx, rx) = tokio::sync::mpsc::channel(1); - let mut recv = SocketpairRecv::with_probes(reader.into(), 1280, rx).unwrap(); - let tracker = crate::probe::ProbeTracker::default(); - let query = tracker.prepare("10.0.0.2".parse().unwrap(), "10.0.0.53".parse().unwrap(), 1).unwrap(); - tx.send(query.clone()).await.unwrap(); - let mut pool = PacketBufPool::new(2); - { - let mut packets = tokio::time::timeout(Duration::from_secs(1), recv.recv(&mut pool)).await.unwrap().unwrap(); - let packet: Packet<[u8]> = packets.next().unwrap().into(); - assert_eq!(packet.as_ref(), query.as_slice()); - assert!(packets.next().is_none()); - } - drop(tx); - writer.send(&query).unwrap(); - let mut packets = tokio::time::timeout(Duration::from_secs(1), recv.recv(&mut pool)).await.unwrap().unwrap(); - let packet: Packet<[u8]> = packets.next().unwrap().into(); - assert_eq!(packet.as_ref(), query.as_slice()); - } -} diff --git a/wgbridge-rs/src/transport/ip_send.rs b/wgbridge-rs/src/transport/ip_send.rs index 3008ec8ef..6a1d9caab 100644 --- a/wgbridge-rs/src/transport/ip_send.rs +++ b/wgbridge-rs/src/transport/ip_send.rs @@ -18,8 +18,6 @@ pub struct TunFdSend { dns_inspector: DnsInspector, write_failures_total: Arc, write_failures_streak: Arc, - delivered_bytes: Arc, - probes: Option>, } impl TunFdSend { @@ -47,20 +45,8 @@ impl TunFdSend { dns_inspector: DnsInspector::default(), write_failures_total, write_failures_streak, - delivered_bytes: Arc::new(AtomicU64::new(0)), - probes: None, } } - - pub fn with_delivered_counter(mut self, counter: Arc) -> Self { - self.delivered_bytes = counter; - self - } - - pub fn with_probes(mut self, probes: Arc) -> Self { - self.probes = Some(probes); - self - } } fn write_fd(fd: i32, buf: &[u8]) -> isize { @@ -82,9 +68,6 @@ fn record_tun_write(total: &AtomicU64, streak: &AtomicU64, full_write: bool) -> impl IpSend for TunFdSend { async fn send(&mut self, packet: Packet) -> io::Result<()> { let mut packet: Packet<[u8]> = packet.into(); - if self.probes.as_ref().is_some_and(|p| p.consume_reply(packet.as_ref())) { - return Ok(()); - } if let Some(dns) = &self.dns { // The inspector records A/AAAA mappings before it blanks @@ -100,9 +83,6 @@ impl IpSend for TunFdSend { let data = packet.as_ref(); let n = write_fd(self.fd.as_raw_fd(), data); - if n == data.len() as isize { - self.delivered_bytes.fetch_add(data.len() as u64, Ordering::Relaxed); - } let (errors, streak) = record_tun_write( &self.write_failures_total, &self.write_failures_streak, @@ -174,22 +154,18 @@ mod tests { let writer = unsafe { OwnedFd::from_raw_fd(fds[1]) }; let total = Arc::new(AtomicU64::new(7)); let streak = Arc::new(AtomicU64::new(3)); - let delivered = Arc::new(AtomicU64::new(0)); let mut sender = TunFdSend::with_counters( writer, None, Arc::clone(&total), Arc::clone(&streak), ); - sender = sender.with_delivered_counter(Arc::clone(&delivered)); let (packet, expected) = minimal_ipv4_packet(); assert!(sender.send(packet).await.is_ok()); assert_eq!(total.load(Ordering::Relaxed), 7); assert_eq!(streak.load(Ordering::Relaxed), 0); - assert_eq!(delivered.load(Ordering::Relaxed), expected.len() as u64); - let reader = std::os::unix::net::UnixDatagram::from(reader); reader.set_read_timeout(Some(std::time::Duration::from_secs(1))).unwrap(); let mut received = [0; 20]; @@ -206,7 +182,6 @@ mod tests { let fd: OwnedFd = file.into(); let total = Arc::new(AtomicU64::new(5)); let streak = Arc::new(AtomicU64::new(2)); - let delivered = Arc::new(AtomicU64::new(0)); let mut sender = TunFdSend::with_counters( fd, None, @@ -214,7 +189,6 @@ mod tests { Arc::clone(&streak), ); - sender = sender.with_delivered_counter(Arc::clone(&delivered)); for _ in 0..3 { let (packet, _) = minimal_ipv4_packet(); assert!(sender.send(packet).await.is_ok()); @@ -222,24 +196,5 @@ mod tests { assert_eq!(total.load(Ordering::Relaxed), 8); assert_eq!(streak.load(Ordering::Relaxed), 5); - assert_eq!(delivered.load(Ordering::Relaxed), 0); - } - - #[tokio::test] - async fn probe_reply_is_consumed_without_tun_write_or_delivery_credit() { - let probes = Arc::new(crate::probe::ProbeTracker::default()); - let query = probes.prepare("10.0.0.2".parse().unwrap(), "10.0.0.53".parse().unwrap(), 42).unwrap(); - let reply = crate::probe::test_reply(&query); - // A write would fail on this read-only fd, making accidental delivery - // observable through the failure counter as well as delivered bytes. - let fd = OpenOptions::new().read(true).open("/dev/null").unwrap().into(); - let failures = Arc::new(AtomicU64::new(0)); - let delivered = Arc::new(AtomicU64::new(0)); - let mut sender = TunFdSend::with_counters(fd, None, Arc::clone(&failures), Arc::new(AtomicU64::new(0))) - .with_delivered_counter(Arc::clone(&delivered)).with_probes(Arc::clone(&probes)); - sender.send(Packet::copy_from(reply.as_slice()).try_into_ip().unwrap()).await.unwrap(); - assert_eq!(probes.reply_token(), 42); - assert_eq!(failures.load(Ordering::Relaxed), 0); - assert_eq!(delivered.load(Ordering::Relaxed), 0); } } diff --git a/wgbridge-rs/src/tunnel.rs b/wgbridge-rs/src/tunnel.rs index 97642a130..cb4e4592f 100644 --- a/wgbridge-rs/src/tunnel.rs +++ b/wgbridge-rs/src/tunnel.rs @@ -27,8 +27,6 @@ pub struct TunnelStats { pub latest_handshake_millis: i64, pub tun_write_failures_total: i64, pub tun_write_failures_streak: i64, - pub delivered_rx_bytes: i64, - pub probe_reply_token: i64, } struct Inner { @@ -44,9 +42,6 @@ struct Inner { logger: Arc, tun_write_failures_total: Arc, tun_write_failures_streak: Arc, - delivered_rx_bytes: Arc, - probes: Arc, - probe_sender: tokio::sync::mpsc::Sender>, } pub struct Tunnel { @@ -100,9 +95,6 @@ pub fn start_tunnel( let tun_write_failures_streak = Arc::new(std::sync::atomic::AtomicU64::new(0)); let stats_total = Arc::clone(&tun_write_failures_total); let stats_streak = Arc::clone(&tun_write_failures_streak); - let delivered_rx_bytes = Arc::new(std::sync::atomic::AtomicU64::new(0)); - let probes = Arc::new(crate::probe::ProbeTracker::default()); - let (probe_sender, probe_receiver) = tokio::sync::mpsc::channel(1); let runtime = tokio::runtime::Builder::new_multi_thread() .worker_threads(2) @@ -114,14 +106,13 @@ pub fn start_tunnel( let device = runtime .block_on(async { - let ip_recv = SocketpairRecv::with_probes(rx_fd, mtu, probe_receiver)?; + let ip_recv = SocketpairRecv::new(rx_fd, mtu)?; let ip_send = TunFdSend::with_counters( tx_fd, dns, Arc::clone(&tun_write_failures_total), Arc::clone(&tun_write_failures_streak), - ).with_delivered_counter(Arc::clone(&delivered_rx_bytes)) - .with_probes(Arc::clone(&probes)); + ); gotatun::device::build() .with_udp(ProtectedUdpFactory::new(protector)) .with_ip_pair(ip_send, ip_recv) @@ -147,35 +138,11 @@ pub fn start_tunnel( logger, tun_write_failures_total: stats_total, tun_write_failures_streak: stats_streak, - delivered_rx_bytes, - probes, - probe_sender, }), }) } impl Tunnel { - /// Queue one DNS probe on the encrypted IP transport. Returning true means - /// queued, not healthy; only probe_reply_token confirms a correlated reply. - pub fn send_dns_probe(&self, source: &str, resolver: &str, token: i64) -> Result { - let source: std::net::IpAddr = source.parse().map_err(|_| "invalid probe source")?; - let resolver: std::net::IpAddr = resolver.parse().map_err(|_| "invalid probe resolver")?; - let peers = self.inner.peers.lock().map_err(|_| "peer lock poisoned")?; - if !peers.iter().any(|peer| peer.allowed_ips.iter().any(|net| net.contains(resolver))) { - return Ok(false); - } - drop(peers); - let Ok(permit) = self.inner.probe_sender.try_reserve() else { return Ok(false); }; - let packet = self.inner.probes.prepare(source, resolver, token).map_err(|e| e.to_string())?; - permit.send(packet); - let probes = Arc::clone(&self.inner.probes); - self.runtime.spawn(async move { - tokio::time::sleep(crate::probe::PROBE_LIFETIME).await; - probes.expire(); - }); - Ok(true) - } - /// Reapplies UAPI configuration to the running device without restarting /// it. The screen-state keepalive toggle goes through [`Tunnel::set_keepalive`] /// instead, which touches one field per peer. @@ -214,8 +181,9 @@ impl Tunnel { } /// Transfer counters and newest handshake, summed across all peers. - /// Engine rx_bytes includes handshake traffic. delivered_rx_bytes counts - /// only complete decrypted IP packets successfully written to Android. + /// rx_bytes counts decrypted transport payload, so it only advances when + /// the tunnel actually carries return traffic — that is the liveness + /// signal the connectivity monitor is biased toward. pub fn stats(&self) -> Result { let inner = Arc::clone(&self.inner); self.runtime.block_on(async move { @@ -231,9 +199,6 @@ impl Tunnel { rx_bytes: 0, tx_bytes: 0, latest_handshake_millis: 0, - probe_reply_token: inner.probes.reply_token(), - delivered_rx_bytes: inner.delivered_rx_bytes.load( - std::sync::atomic::Ordering::Relaxed) as i64, tun_write_failures_total: inner .tun_write_failures_total .load(std::sync::atomic::Ordering::Relaxed) From c6f8d5842ec34a92c93b2df0e9a04b28f37ec9a5 Mon Sep 17 00:00:00 2001 From: Konrad Kollnig <5175206+kasnder@users.noreply.github.com> Date: Mon, 14 Sep 2026 13:27:02 +0200 Subject: [PATCH 3/4] Scope WireGuard handover recovery to the selected path --- .../faircode/netguard/ActivitySettings.java | 12 +- .../netguard/NetworkReloadPolicy.java | 76 +---- .../netguard/PhysicalNetworkState.java | 180 ++++++------ .../eu/faircode/netguard/ServiceSinkhole.java | 101 +++++-- .../net/kollnig/missioncontrol/wg/WgEgress.kt | 28 +- .../netguard/NetworkReloadPolicyTest.java | 197 +------------ .../netguard/PhysicalNetworkStateTest.java | 277 +++++++++--------- .../ServiceSinkholeNetworkReloadTest.java | 167 +++++++++++ .../wg/WgEgressRecoveryTest.java | 165 +++++++---- 9 files changed, 613 insertions(+), 590 deletions(-) create mode 100644 app/src/test/java/eu/faircode/netguard/ServiceSinkholeNetworkReloadTest.java diff --git a/app/src/main/java/eu/faircode/netguard/ActivitySettings.java b/app/src/main/java/eu/faircode/netguard/ActivitySettings.java index acab9bcb3..3e1f99b1b 100644 --- a/app/src/main/java/eu/faircode/netguard/ActivitySettings.java +++ b/app/src/main/java/eu/faircode/netguard/ActivitySettings.java @@ -277,13 +277,11 @@ protected void onPostExecute(Throwable ex) { }); } - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { - TwoStatePreference pref_reload_onconnectivity = (TwoStatePreference) screen - .findPreference("reload_onconnectivity"); - if (pref_reload_onconnectivity != null) { - pref_reload_onconnectivity.setChecked(true); - pref_reload_onconnectivity.setEnabled(false); - } + TwoStatePreference pref_reload_onconnectivity = (TwoStatePreference) screen + .findPreference("reload_onconnectivity"); + if (pref_reload_onconnectivity != null) { + pref_reload_onconnectivity.setChecked(true); + pref_reload_onconnectivity.setEnabled(false); } // Handle port forwarding diff --git a/app/src/main/java/eu/faircode/netguard/NetworkReloadPolicy.java b/app/src/main/java/eu/faircode/netguard/NetworkReloadPolicy.java index ded42060b..a2c7ea06b 100644 --- a/app/src/main/java/eu/faircode/netguard/NetworkReloadPolicy.java +++ b/app/src/main/java/eu/faircode/netguard/NetworkReloadPolicy.java @@ -1,92 +1,22 @@ package eu.faircode.netguard; -import java.util.List; -import java.util.Objects; - final class NetworkReloadPolicy { - static final String REASON_NETWORK_AVAILABLE = "network available"; - static final String REASON_NETWORK_LOST = "network lost"; static final String REASON_NETWORK_CHANGED = "Network changed"; - static final String REASON_CONNECTED_CHANGED = "Connected state changed"; static final String REASON_LINK_PROPERTIES_CHANGED = "link properties changed"; static final String REASON_PRIVATE_DNS_CHANGED = "private DNS changed"; static final String REASON_METERED_CHANGED = "Metered state changed"; + static final String REASON_DNS_CHANGED = "DNS servers changed"; static final String REASON_CONNECTIVITY_CHANGED = "connectivity changed"; - private NetworkReloadPolicy() { - } - - static String onNetworkAvailable() { - return REASON_NETWORK_AVAILABLE; - } - - static String onNetworkLost(Object lostNetwork, Object lastActiveNetwork) { - return lastActiveNetwork != null && Objects.equals(lastActiveNetwork, lostNetwork) - ? REASON_NETWORK_LOST - : null; - } + private NetworkReloadPolicy() { } static String onConnectivityChanged() { return REASON_CONNECTIVITY_CHANGED; } - static String onLinkPropertiesChanged(List lastDns, List currentDns, - boolean compareDns, boolean reloadOnConnectivity, - String lastPrivateDns, String currentPrivateDns) { - if (compareDns ? !same(lastDns, currentDns) : reloadOnConnectivity) - return REASON_LINK_PROPERTIES_CHANGED; - - // Pinning Private DNS to a hostname leaves the resolver list alone, so - // the comparison above never sees it — yet it decides whether blocking - // DoT stops name resolution outright, which the user has to be told. - if (!Objects.equals(lastPrivateDns, currentPrivateDns)) - return REASON_PRIVATE_DNS_CHANGED; - - return null; - } - - static String onCapabilitiesChanged(Object network, Object lastNetwork, - Boolean lastConnected, boolean connected, - Boolean lastMetered, boolean metered) { - if (!Objects.equals(network, lastNetwork)) - return REASON_NETWORK_CHANGED; - - if (lastConnected != null && !lastConnected.equals(connected)) - return REASON_CONNECTED_CHANGED; - - if (lastMetered != null && !lastMetered.equals(metered)) - return REASON_METERED_CHANGED; - - return null; - } - static boolean shouldRestartWireGuard(String reason) { - return REASON_NETWORK_AVAILABLE.equals(reason) || - REASON_NETWORK_LOST.equals(reason) || - REASON_NETWORK_CHANGED.equals(reason) || - REASON_CONNECTED_CHANGED.equals(reason) || + return REASON_NETWORK_CHANGED.equals(reason) || REASON_LINK_PROPERTIES_CHANGED.equals(reason) || - REASON_METERED_CHANGED.equals(reason) || REASON_CONNECTIVITY_CHANGED.equals(reason); } - - /** - * The same decision across a coalesced burst of callbacks, which keeps only - * the last reason. The need for a rebind is sticky: once any reason in the - * burst required one, a later reason that does not must not cancel it. - */ - static boolean shouldRestartWireGuard(boolean pendingRestart, String reason) { - return pendingRestart || shouldRestartWireGuard(reason); - } - - static boolean same(List last, List current) { - if (last == null || current == null || last.size() != current.size()) - return false; - - for (int i = 0; i < current.size(); i++) - if (!Objects.equals(last.get(i), current.get(i))) - return false; - - return true; - } } diff --git a/app/src/main/java/eu/faircode/netguard/PhysicalNetworkState.java b/app/src/main/java/eu/faircode/netguard/PhysicalNetworkState.java index 0b3c999fe..0ddd5241f 100644 --- a/app/src/main/java/eu/faircode/netguard/PhysicalNetworkState.java +++ b/app/src/main/java/eu/faircode/netguard/PhysicalNetworkState.java @@ -6,7 +6,6 @@ import android.os.Build; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -16,147 +15,152 @@ /** Callback-owned snapshots: never query ConnectivityManager from its callbacks. */ final class PhysicalNetworkState { private static final class Entry { - List capabilities; - List links; + List transports; + Boolean metered; + List routes; + List dns; String privateDns; boolean privateDnsActive; } private final Map entries = new HashMap<>(); private Network defaultNetwork; - private boolean defaultSeen; - private List vpnTransports; + private boolean defaultIsVpn; + private List vpnTransports = Collections.emptyList(); + private Network egress; synchronized String onPhysicalAvailable(Network network) { - if (network == null || entries.containsKey(network)) return null; - entries.put(network, new Entry()); - return NetworkReloadPolicy.REASON_NETWORK_AVAILABLE; + if (network != null && !entries.containsKey(network)) entries.put(network, new Entry()); + return null; // Availability alone says nothing about the selected egress. } synchronized String onPhysicalCapabilitiesChanged(Network network, NetworkCapabilities caps) { - if (network == null || caps == null || + Entry entry = entries.get(network); + if (entry == null || caps == null || !caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN)) return null; - Entry entry = entry(network); - List snapshot = capabilities(caps); - boolean changed = !snapshot.equals(entry.capabilities); - entry.capabilities = snapshot; - return changed ? NetworkReloadPolicy.REASON_NETWORK_CHANGED : null; + List previous = entry.transports; + Boolean previousMetered = entry.metered; + entry.transports = transports(caps); + entry.metered = isMetered(caps); + String change = selectEgress(); + if (change != null || !network.equals(egress)) return change; + if (previous != null && !previous.equals(entry.transports)) + return NetworkReloadPolicy.REASON_NETWORK_CHANGED; + return previousMetered != null && !previousMetered.equals(entry.metered) + ? NetworkReloadPolicy.REASON_METERED_CHANGED : null; } synchronized String onPhysicalLinkPropertiesChanged(Network network, LinkProperties props) { - if (network == null || props == null) return null; - Entry entry = entry(network); - List snapshot = links(props); + Entry entry = entries.get(network); + if (entry == null || props == null) return null; + List routes = new ArrayList<>(); + for (Object address : props.getLinkAddresses()) routes.add("address:" + address); + for (Object route : props.getRoutes()) routes.add("route:" + route); + Collections.sort(routes); + List dns = new ArrayList<>(); + for (java.net.InetAddress server : props.getDnsServers()) dns.add(server.getHostAddress()); + dns.add("domains:" + props.getDomains()); + Collections.sort(dns); String privateDns = Build.VERSION.SDK_INT >= Build.VERSION_CODES.P ? props.getPrivateDnsServerName() : null; boolean active = Build.VERSION.SDK_INT >= Build.VERSION_CODES.P && props.isPrivateDnsActive(); - boolean changed = !snapshot.equals(entry.links); + boolean routeChanged = entry.routes != null && !entry.routes.equals(routes); + boolean dnsChanged = entry.dns != null && !entry.dns.equals(dns); boolean privateChanged = !Objects.equals(privateDns, entry.privateDns) || active != entry.privateDnsActive; - entry.links = snapshot; + entry.routes = routes; + entry.dns = dns; entry.privateDns = privateDns; entry.privateDnsActive = active; - if (changed) return NetworkReloadPolicy.REASON_LINK_PROPERTIES_CHANGED; + if (!network.equals(egress)) return null; + if (routeChanged) return NetworkReloadPolicy.REASON_LINK_PROPERTIES_CHANGED; + if (dnsChanged) return NetworkReloadPolicy.REASON_DNS_CHANGED; return privateChanged ? NetworkReloadPolicy.REASON_PRIVATE_DNS_CHANGED : null; } synchronized String onPhysicalLost(Network network) { - if (network == null || entries.remove(network) == null) return null; - if (network.equals(defaultNetwork)) defaultNetwork = null; - return NetworkReloadPolicy.REASON_NETWORK_LOST; + if (entries.remove(network) == null) return null; + return selectEgress(); } synchronized String onDefaultNetworkAvailable(Network network) { - return acceptDefaultIfPhysical(network); + // onCapabilitiesChanged identifies physical versus VPN. Guessing here + // would treat our own replacement VPN as a physical handover. + return null; } synchronized String onDefaultNetworkCapabilitiesChanged(Network network, NetworkCapabilities caps) { if (network == null || caps == null) return null; - if (!caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN)) { - // A VPN can stay default while its underlying Wi-Fi/mobile transport - // changes. Ignore VPN identity churn caused by our own reloads. - List snapshot = transports(caps); - if (snapshot.isEmpty()) return null; - boolean changed = vpnTransports != null && !vpnTransports.equals(snapshot); - vpnTransports = snapshot; - return changed ? NetworkReloadPolicy.REASON_NETWORK_CHANGED : null; - } - String change = onPhysicalCapabilitiesChanged(network, caps); - String defaultChange = acceptDefaultIfPhysical(network); - return defaultChange != null ? defaultChange : change; + boolean vpn = !caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN); + List snapshot = transports(caps); + if (vpn && snapshot.isEmpty()) return null; // Replacement VPN has not inherited transports yet. + boolean transportChanged = vpn && defaultIsVpn && !vpnTransports.equals(snapshot); + defaultNetwork = network; + defaultIsVpn = vpn; + vpnTransports = vpn ? snapshot : Collections.emptyList(); + // Never create physical entries from this unfiltered callback: only + // the physical registration guarantees a matching onLost later. + String change = vpn ? selectEgress() : onPhysicalCapabilitiesChanged(network, caps); + if (change == null) change = selectEgress(); + return change != null ? change : transportChanged ? NetworkReloadPolicy.REASON_NETWORK_CHANGED : null; } synchronized String onDefaultNetworkLinkPropertiesChanged(Network network, LinkProperties props) { - Entry entry = entries.get(network); - if (entry == null || entry.capabilities == null) return null; - String change = onPhysicalLinkPropertiesChanged(network, props); - String defaultChange = acceptDefaultIfPhysical(network); - return defaultChange != null ? defaultChange : change; + return network != null && network.equals(defaultNetwork) && !defaultIsVpn + ? onPhysicalLinkPropertiesChanged(network, props) : null; } synchronized String onDefaultNetworkLost(Network network) { - if (network != null && network.equals(defaultNetwork)) defaultNetwork = null; - return null; // The physical callback reports actual loss; VPN loss is self-generated. - } - - private String acceptDefaultIfPhysical(Network network) { - Entry entry = entries.get(network); - if (entry == null || entry.capabilities == null) return null; - boolean changed = defaultSeen && !network.equals(defaultNetwork); - defaultNetwork = network; - defaultSeen = true; - return changed ? NetworkReloadPolicy.REASON_NETWORK_CHANGED : null; - } - - private Entry entry(Network network) { - Entry entry = entries.get(network); - if (entry == null) { - entry = new Entry(); - entries.put(network, entry); + if (!Objects.equals(network, defaultNetwork) || defaultIsVpn) return null; + defaultNetwork = null; + return selectEgress(); + } + + private String selectEgress() { + Network selected = null; + if (!defaultIsVpn) { + Entry entry = entries.get(defaultNetwork); + if (entry != null && entry.transports != null) selected = defaultNetwork; + } else { + // VPN capabilities expose physical transports, not necessarily an + // underlying Network identity. Retain a still-matching selection; + // otherwise select only an unambiguous candidate, never a standby + // merely because its validation/metered state changed. + Entry current = entries.get(egress); + if (current != null && current.transports != null && + !Collections.disjoint(current.transports, vpnTransports)) selected = egress; + else for (Map.Entry candidate : entries.entrySet()) { + List transports = candidate.getValue().transports; + if (transports == null || Collections.disjoint(transports, vpnTransports)) continue; + if (selected != null) { selected = null; break; } + selected = candidate.getKey(); + } } - return entry; + boolean changed = !Objects.equals(egress, selected); + egress = selected; + return changed ? NetworkReloadPolicy.REASON_NETWORK_CHANGED : null; } - // Compare only route-relevant values, not signal strength or bandwidth. - // Unknown capability integers safely return false on older Android versions. @android.annotation.SuppressLint("InlinedApi") - private static List capabilities(NetworkCapabilities caps) { - return Arrays.asList(transports(caps), - caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET), - caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED), - caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED), - caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED), - caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_TEMPORARILY_NOT_METERED)); + private static boolean isMetered(NetworkCapabilities caps) { + return !caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED) && + !caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_TEMPORARILY_NOT_METERED); } private static List transports(NetworkCapabilities caps) { List result = new ArrayList<>(); for (int transport = 0; transport < 32; transport++) - if (transport != NetworkCapabilities.TRANSPORT_VPN && caps.hasTransport(transport)) - result.add(transport); + if (transport != NetworkCapabilities.TRANSPORT_VPN && caps.hasTransport(transport)) result.add(transport); return result; } - private static List links(LinkProperties props) { - // Retain immutable values rather than mutable callback objects. This - // also avoids LinkProperties constructors/setters that require API 29. - List result = new ArrayList<>(); - for (Object address : props.getLinkAddresses()) result.add("address:" + address); - for (Object route : props.getRoutes()) result.add("route:" + route); - for (java.net.InetAddress dns : props.getDnsServers()) result.add("dns:" + dns.getHostAddress()); - result.add("domains:" + props.getDomains()); - Collections.sort(result); - return result; - } - - synchronized Network getDefaultNetwork() { - return defaultNetwork; - } + synchronized Network getDefaultNetwork() { return egress; } synchronized void reset() { entries.clear(); defaultNetwork = null; - defaultSeen = false; - vpnTransports = null; + defaultIsVpn = false; + vpnTransports = Collections.emptyList(); + egress = null; } } diff --git a/app/src/main/java/eu/faircode/netguard/ServiceSinkhole.java b/app/src/main/java/eu/faircode/netguard/ServiceSinkhole.java index b9305b60f..5f13316f9 100644 --- a/app/src/main/java/eu/faircode/netguard/ServiceSinkhole.java +++ b/app/src/main/java/eu/faircode/netguard/ServiceSinkhole.java @@ -183,6 +183,8 @@ private static int getIntPref(SharedPreferences prefs, String key, int def) { private ConnectivityManager.NetworkCallback networkCallback = null; private ConnectivityManager.NetworkCallback defaultNetworkCallback = null; private final PhysicalNetworkState physicalNetworkState = new PhysicalNetworkState(); + private final Handler networkSnapshotHandler = new Handler(Looper.getMainLooper()); + private final Runnable defaultNetworkSnapshotRunnable = this::refreshDefaultNetworkSnapshot; private boolean registeredInteractiveState = false; private PhoneStateListener callStateListener = null; @@ -267,6 +269,7 @@ private static int getIntPref(SharedPreferences prefs, String key, int def) { private static final long WG_STARTUP_RECOVERY_INITIAL_DELAY_MS = 1_000L; private static final long WG_STARTUP_RECOVERY_STABLE_WINDOW_MS = 2 * 60_000L; private static final String EXTRA_WG_STARTUP_RETRY = "WireGuardStartupRetry"; + private static final String EXTRA_WG_NETWORK_CHANGED = "WireGuardNetworkChanged"; private final WireGuardStartupRecoveryPolicy wgStartupRecoveryPolicy = new WireGuardStartupRecoveryPolicy( WG_STARTUP_RECOVERY_MAX_RETRIES, @@ -733,7 +736,8 @@ public void onCallStateChanged(int state, String incomingNumber) { cancelWireGuardStartupRecovery(false); if (!intent.getBooleanExtra(EXTRA_REPLACEMENT_RETRY, false)) cancelVpnReplacementRecovery(false); - reload(intent.getBooleanExtra(EXTRA_INTERACTIVE, false)); + reload(intent.getBooleanExtra(EXTRA_INTERACTIVE, false), + intent.getBooleanExtra(EXTRA_WG_NETWORK_CHANGED, false)); break; case stop: @@ -838,7 +842,7 @@ private void start() { if (vpn == null) throw new StartFailedException(getString((R.string.msg_start_failed))); - if (!startNative(vpn, listAllowed, listRule)) + if (!startNative(vpn, listAllowed, listRule, false)) return; // Start DoH proxy if enabled and not superseded by WireGuard DNS. @@ -849,7 +853,7 @@ private void start() { } } - private void reload(boolean interactive) { + private void reload(boolean interactive, boolean networkChanged) { List listRule = Rule.getRules(true, ServiceSinkhole.this); SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(ServiceSinkhole.this); @@ -912,7 +916,7 @@ private void reload(boolean interactive) { if (vpn == null) throw new StartFailedException(getString((R.string.msg_start_failed))); - if (!startNative(vpn, listAllowed, listRule)) + if (!startNative(vpn, listAllowed, listRule, networkChanged)) return; // Update DoH proxy state based on current settings. @@ -2139,7 +2143,8 @@ private Builder getBlockingBuilder(List listRule) { return builder; } - private boolean startNative(final ParcelFileDescriptor vpn, List listAllowed, List listRule) { + private boolean startNative(final ParcelFileDescriptor vpn, List listAllowed, + List listRule, boolean networkChanged) { SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(ServiceSinkhole.this); boolean log = prefs.getBoolean("log", false); boolean log_app = prefs.getBoolean("log_app", true); @@ -2220,7 +2225,8 @@ public void onProviderRejected(String providerLabel, String message) { Util.isInteractive(ServiceSinkhole.this), prefs.getBoolean("wg_keepalive_when_screen_off", false), () -> jni_wireguard_start(), - () -> { jni_wireguard_stop(); return kotlin.Unit.INSTANCE; }); + () -> { jni_wireguard_stop(); return kotlin.Unit.INSTANCE; }, + networkChanged); if (!wgOk) { String wgError = net.kollnig.missioncontrol.wg.WgEgress.INSTANCE.getLastError(); Log.w(TAG, "WireGuard egress failed to start; blocking traffic: " + wgError); @@ -3879,11 +3885,6 @@ public void onCreate() { } private void listenNetworkChanges() { - if (Build.VERSION.SDK_INT < Build.VERSION_CODES.N) { - listenConnectivityChanges(); - return; - } - // Listen for network changes Log.i(TAG, "Starting listening to network changes"); ConnectivityManager cm = (ConnectivityManager) getSystemService(Context.CONNECTIVITY_SERVICE); @@ -3896,6 +3897,9 @@ private void listenNetworkChanges() { public void onAvailable(Network network) { Log.i(TAG, "Available network=" + network); handlePhysicalNetworkChange(physicalNetworkState.onPhysicalAvailable(network)); + // Initial capability/link callbacks are only guaranteed from API 26. + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) + refreshPhysicalNetworkSnapshot(network); } @Override @@ -3940,8 +3944,6 @@ public void onLinkPropertiesChanged(Network network, LinkProperties linkProperti Log.i(TAG, "Default network properties=" + network + " props=" + linkProperties); handlePhysicalNetworkChange(physicalNetworkState.onDefaultNetworkLinkPropertiesChanged( network, linkProperties)); - if (vpn != null) - requestPrivateDnsWarningUpdate(); } @Override @@ -3953,27 +3955,66 @@ public void onLost(Network network) { cm.registerNetworkCallback(builder.build(), nc); networkCallback = nc; - try { - cm.registerDefaultNetworkCallback(dnc); - defaultNetworkCallback = dnc; - } catch (Throwable ex) { - cm.unregisterNetworkCallback(nc); - networkCallback = null; - physicalNetworkState.reset(); - throw ex; + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + try { + cm.registerDefaultNetworkCallback(dnc); + defaultNetworkCallback = dnc; + } catch (Throwable ex) { + cm.unregisterNetworkCallback(nc); + networkCallback = null; + physicalNetworkState.reset(); + throw ex; + } } } private void handlePhysicalNetworkChange(String reason) { + handlePhysicalNetworkChange(reason, true); + } + + private void handlePhysicalNetworkChange(String reason, boolean refreshDefault) { if (reason != null) reloadAfterNetworkChange(reason); + if (refreshDefault) + scheduleDefaultNetworkSnapshot(); + } + + private void scheduleDefaultNetworkSnapshot() { + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.N) { + networkSnapshotHandler.removeCallbacks(defaultNetworkSnapshotRunnable); + networkSnapshotHandler.post(defaultNetworkSnapshotRunnable); + } + } + + private void refreshDefaultNetworkSnapshot() { + ConnectivityManager cm = (ConnectivityManager) getSystemService(Context.CONNECTIVITY_SERVICE); + if (cm == null) + return; + Network active = cm.getActiveNetwork(); + NetworkCapabilities capabilities = active == null ? null : cm.getNetworkCapabilities(active); + String reason = physicalNetworkState.onDefaultNetworkCapabilitiesChanged(active, capabilities); + handlePhysicalNetworkChange(reason, false); + } + + private void refreshPhysicalNetworkSnapshot(final Network network) { + networkSnapshotHandler.post(() -> { + ConnectivityManager cm = (ConnectivityManager) getSystemService(Context.CONNECTIVITY_SERVICE); + if (cm == null) + return; + NetworkCapabilities capabilities = cm.getNetworkCapabilities(network); + LinkProperties properties = cm.getLinkProperties(network); + handlePhysicalNetworkChange(physicalNetworkState.onPhysicalCapabilitiesChanged( + network, capabilities)); + handlePhysicalNetworkChange(physicalNetworkState.onPhysicalLinkPropertiesChanged( + network, properties)); + }); } // Network flapping (Wi-Fi<->cellular handoffs, DHCP renewals) fires several // ConnectivityManager callbacks within milliseconds of each other. Each // reload is a foreground-service update + wakelock + native VPN restart + - // WireGuard rebind, so bursts are coalesced into a single reload using the - // last reason once the burst settles. Not every reason needs the rebind, so + // WireGuard restart, so bursts are coalesced into a single reload using the + // last reason once the burst settles. Not every reason needs the restart, so // the need for one is accumulated across the burst rather than read off the // surviving reason: a reason that does not need it must not cancel one that // did, or the tunnel keeps a socket bound to a network that is gone. @@ -3991,14 +4032,15 @@ private void reloadAfterNetworkChange(final String reason) { networkReloadDebounceHandler.postAtTime(new Runnable() { @Override public void run() { - if (pendingWireGuardRestart.getAndSet(false)) - net.kollnig.missioncontrol.wg.WgEgress.INSTANCE.onUnderlyingNetworkChanged(); - reload(reason, ServiceSinkhole.this, false); + boolean networkChanged = pendingWireGuardRestart.getAndSet(false); + reload(reason, ServiceSinkhole.this, false, networkChanged); } }, NETWORK_RELOAD_TOKEN, SystemClock.uptimeMillis() + NETWORK_RELOAD_DEBOUNCE_MS); } private void listenConnectivityChanges() { + if (registeredConnectivityChanged) + return; // Listen for connectivity updates Log.i(TAG, "Starting listening to connectivity changes"); IntentFilter ifConnectivity = new IntentFilter(); @@ -4304,6 +4346,7 @@ public void onDestroy() { private void unlistenNetworkChanges() { ConnectivityManager cm = (ConnectivityManager) getSystemService(Context.CONNECTIVITY_SERVICE); + networkSnapshotHandler.removeCallbacksAndMessages(null); try { if (networkCallback != null) cm.unregisterNetworkCallback(networkCallback); @@ -4969,12 +5012,18 @@ public static void start(String reason, Context context, boolean userInitiated) } public static void reload(String reason, Context context, boolean interactive) { + reload(reason, context, interactive, false); + } + + private static void reload(String reason, Context context, boolean interactive, + boolean networkChanged) { SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(context); if (prefs.getBoolean("enabled", false)) { Intent intent = new Intent(context, ServiceSinkhole.class); intent.putExtra(EXTRA_COMMAND, Command.reload); intent.putExtra(EXTRA_REASON, reason); intent.putExtra(EXTRA_INTERACTIVE, interactive); + intent.putExtra(EXTRA_WG_NETWORK_CHANGED, networkChanged); try { ContextCompat.startForegroundService(context, intent); } catch (Throwable ex) { diff --git a/app/src/main/java/net/kollnig/missioncontrol/wg/WgEgress.kt b/app/src/main/java/net/kollnig/missioncontrol/wg/WgEgress.kt index 442d2870a..4af7f06d8 100644 --- a/app/src/main/java/net/kollnig/missioncontrol/wg/WgEgress.kt +++ b/app/src/main/java/net/kollnig/missioncontrol/wg/WgEgress.kt @@ -119,8 +119,8 @@ object WgEgress { // Bounds the relay-list fetch + config rewrite so a stalled network call // can never withhold a restart for longer than the caller would have // waited anyway. forceRestartPending is already set by the time this - // runs, which makes onMonitorBroken/onUnderlyingNetworkChanged no-op - // until a restart is scheduled — without a bound, a hung call would + // runs, which makes onMonitorBroken no-op until a restart is scheduled — + // without a bound, a hung call would // silently stall recovery instead of merely skipping the relay switch. private const val FAILOVER_TIMEOUT_MS = 15_000L @@ -247,7 +247,9 @@ object WgEgress { /** * Bring the tunnel up, take it down, or leave it alone — whichever the * desired state requires. Idempotent: same config + same TUN fd is a - * no-op so reload-induced restarts don't re-handshake. + * no-op so ordinary reload-induced restarts don't re-handshake. A + * debounced physical-network change can set [networkChanged] to request + * a fresh tunnel on that same reload path. * * Returns true on success or already-correct state. Returns false if * WG was supposed to start but failed; in that case the caller must keep @@ -261,12 +263,15 @@ object WgEgress { interactive: Boolean, keepaliveAlwaysOn: Boolean, startSocketpair: () -> Int, - stopSocketpair: () -> Unit + stopSocketpair: () -> Unit, + networkChanged: Boolean = false ): Boolean { verificationGeneration++ val wantRunning = wgEnabled && !configText.isNullOrEmpty() val desiredFd = vpnFd.fd lastError = null + if (networkChanged) + clearEndpointCache() if (!wantRunning) { clearRecoveryState() @@ -279,7 +284,8 @@ object WgEgress { return true } - if (tunnel != null && currentConfig == configText && currentTunPfd === vpnFd && !forceRestartPending) { + if (tunnel != null && currentConfig == configText && currentTunPfd === vpnFd && + !forceRestartPending && !networkChanged) { val oldKeepaliveEnabled = currentInteractive || currentKeepaliveAlwaysOn val newKeepaliveEnabled = interactive || keepaliveAlwaysOn if (oldKeepaliveEnabled != newKeepaliveEnabled && @@ -701,18 +707,6 @@ object WgEgress { fun latestHandshakeMillisOrNull(): Long? = try { tunnel?.latestHandshakeMillis() } catch (_: Throwable) { null } - fun onUnderlyingNetworkChanged() { - verificationGeneration++ - clearEndpointCache() - // ServiceSinkhole calls this once per debounced network-change burst, - // immediately before reload. Recreate the tunnel in that reload even - // if its configuration and TUN are unchanged: a successful socket - // rebind does not prove the new path can carry application traffic. - synchronized(tunnelLifecycleLock) { - if (tunnel != null) forceRestartPending = true - } - } - /** * Apply the screen-state keepalive policy (PersistentKeepalive is dropped * while the screen is off to save battery). Tunnel liveness and recovery diff --git a/app/src/test/java/eu/faircode/netguard/NetworkReloadPolicyTest.java b/app/src/test/java/eu/faircode/netguard/NetworkReloadPolicyTest.java index 905a97f86..5ca9e6a68 100644 --- a/app/src/test/java/eu/faircode/netguard/NetworkReloadPolicyTest.java +++ b/app/src/test/java/eu/faircode/netguard/NetworkReloadPolicyTest.java @@ -1,196 +1,17 @@ package eu.faircode.netguard; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; - +import static org.junit.Assert.*; import org.junit.Test; -import java.util.Arrays; -import java.util.Collections; - public class NetworkReloadPolicyTest { @Test - public void activeNetworkAvailableReloads() { - assertEquals("network available", NetworkReloadPolicy.onNetworkAvailable()); - } - - @Test - public void activeNetworkLostReloads() { - assertEquals("network lost", NetworkReloadPolicy.onNetworkLost("wifi", "wifi")); - } - - @Test - public void inactiveNetworkLostDoesNotReload() { - assertNull(NetworkReloadPolicy.onNetworkLost("mobile", "wifi")); - } - - @Test - public void activeNetworkIdentityChangeReloads() { - assertEquals("Network changed", - NetworkReloadPolicy.onCapabilitiesChanged( - "mobile", "wifi", - true, true, - false, false)); - } - - @Test - public void firstCapabilitiesCallbackReloadsAsNetworkChange() { - assertEquals("Network changed", - NetworkReloadPolicy.onCapabilitiesChanged( - "wifi", null, - null, true, - null, false)); - } - - @Test - public void connectedStateChangeReloads() { - assertEquals("Connected state changed", - NetworkReloadPolicy.onCapabilitiesChanged( - "wifi", "wifi", - false, true, - false, false)); - } - - @Test - public void meteredStateChangeReloads() { - assertEquals("Metered state changed", - NetworkReloadPolicy.onCapabilitiesChanged( - "wifi", "wifi", - true, true, - false, true)); - } - - @Test - public void sameCapabilitiesDoNotReload() { - assertNull(NetworkReloadPolicy.onCapabilitiesChanged( - "mobile", "mobile", - true, true, - true, true)); - } - - @Test - public void dnsChangeReloadsOnModernAndroid() { - assertEquals("link properties changed", - NetworkReloadPolicy.onLinkPropertiesChanged( - Collections.singletonList("9.9.9.9"), - Collections.singletonList("1.1.1.1"), - true, - false, - null, null)); - } - - @Test - public void sameDnsDoesNotReloadOnModernAndroid() { - assertNull(NetworkReloadPolicy.onLinkPropertiesChanged( - Arrays.asList("9.9.9.9", "149.112.112.112"), - Arrays.asList("9.9.9.9", "149.112.112.112"), - true, - false, - null, null)); - } - - @Test - public void preOConnectivityPreferenceControlsLinkPropertyReload() { - assertEquals("link properties changed", - NetworkReloadPolicy.onLinkPropertiesChanged( - Collections.singletonList("9.9.9.9"), - Collections.singletonList("9.9.9.9"), - false, - true, - null, null)); - - assertNull(NetworkReloadPolicy.onLinkPropertiesChanged( - Collections.singletonList("9.9.9.9"), - Collections.singletonList("1.1.1.1"), - false, - false, - null, null)); - } - - /** - * Pinning Private DNS to a hostname leaves the resolver list untouched, so - * comparing DNS servers alone never notices it and the warning that DoT is - * blocked would not appear until some unrelated network change. - */ - @Test - public void privateDnsPinnedReloads() { - assertEquals("private DNS changed", - NetworkReloadPolicy.onLinkPropertiesChanged( - Collections.singletonList("9.9.9.9"), - Collections.singletonList("9.9.9.9"), - true, - false, - null, "dns.google")); - } - - @Test - public void privateDnsClearedReloads() { - assertEquals("private DNS changed", - NetworkReloadPolicy.onLinkPropertiesChanged( - Collections.singletonList("9.9.9.9"), - Collections.singletonList("9.9.9.9"), - true, - false, - "dns.google", null)); - } - - @Test - public void samePrivateDnsDoesNotReload() { - assertNull(NetworkReloadPolicy.onLinkPropertiesChanged( - Collections.singletonList("9.9.9.9"), - Collections.singletonList("9.9.9.9"), - true, - false, - "dns.google", "dns.google")); - } - - /** - * The tunnel is unaffected by a resolver being pinned, so this reload must - * not cost a WireGuard rebind and re-handshake. - */ - @Test - public void privateDnsChangeDoesNotRestartWireGuard() { - assertFalse(NetworkReloadPolicy.shouldRestartWireGuard("private DNS changed")); - } - - /** - * A burst of callbacks is collapsed to its last reason, but the rebind it - * needs is not a property of that reason alone: a private DNS change - * landing right after a genuine network change must not cancel the rebind - * that change required, or the tunnel keeps a socket bound to a gone - * network until some later event. - */ - @Test - public void privateDnsChangeDoesNotCancelAPendingRestart() { - boolean pending = NetworkReloadPolicy.shouldRestartWireGuard(false, "Network changed"); - assertTrue(pending); - assertTrue(NetworkReloadPolicy.shouldRestartWireGuard(pending, "private DNS changed")); - } - - @Test - public void privateDnsChangeAloneStillDoesNotRestartWireGuard() { - assertFalse(NetworkReloadPolicy.shouldRestartWireGuard(false, "private DNS changed")); - } - - @Test - public void physicalConnectivityReloadsRestartWireGuard() { - assertTrue(NetworkReloadPolicy.shouldRestartWireGuard("network available")); - assertTrue(NetworkReloadPolicy.shouldRestartWireGuard("network lost")); - assertTrue(NetworkReloadPolicy.shouldRestartWireGuard("Network changed")); - assertTrue(NetworkReloadPolicy.shouldRestartWireGuard("Connected state changed")); - assertTrue(NetworkReloadPolicy.shouldRestartWireGuard("Metered state changed")); - } - - @Test - public void linkPropertyReloadRestartsWireGuard() { - assertTrue(NetworkReloadPolicy.shouldRestartWireGuard("link properties changed")); - } - - @Test - public void fallbackConnectivityReloadsRestartWireGuard() { - assertEquals("connectivity changed", NetworkReloadPolicy.onConnectivityChanged()); - assertTrue(NetworkReloadPolicy.shouldRestartWireGuard("connectivity changed")); + public void onlyPathChangesRequestWireGuardRecreation() { + assertTrue(NetworkReloadPolicy.shouldRestartWireGuard(NetworkReloadPolicy.REASON_NETWORK_CHANGED)); + assertTrue(NetworkReloadPolicy.shouldRestartWireGuard(NetworkReloadPolicy.REASON_LINK_PROPERTIES_CHANGED)); + assertTrue(NetworkReloadPolicy.shouldRestartWireGuard(NetworkReloadPolicy.onConnectivityChanged())); + assertFalse(NetworkReloadPolicy.shouldRestartWireGuard(NetworkReloadPolicy.REASON_METERED_CHANGED)); + assertFalse(NetworkReloadPolicy.shouldRestartWireGuard(NetworkReloadPolicy.REASON_DNS_CHANGED)); + assertFalse(NetworkReloadPolicy.shouldRestartWireGuard(NetworkReloadPolicy.REASON_PRIVATE_DNS_CHANGED)); + assertFalse(NetworkReloadPolicy.shouldRestartWireGuard(null)); } } diff --git a/app/src/test/java/eu/faircode/netguard/PhysicalNetworkStateTest.java b/app/src/test/java/eu/faircode/netguard/PhysicalNetworkStateTest.java index 1297e95c7..da8458716 100644 --- a/app/src/test/java/eu/faircode/netguard/PhysicalNetworkStateTest.java +++ b/app/src/test/java/eu/faircode/netguard/PhysicalNetworkStateTest.java @@ -1,23 +1,22 @@ package eu.faircode.netguard; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; +import static org.junit.Assert.*; +import android.net.LinkAddress; import android.net.LinkProperties; import android.net.Network; import android.net.NetworkCapabilities; - import org.junit.Test; import org.junit.runner.RunWith; import org.robolectric.Shadows; import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; import org.robolectric.shadows.ShadowNetwork; import org.robolectric.shadows.ShadowNetworkCapabilities; - -import java.net.InetAddress; import java.lang.reflect.Field; +import java.net.InetAddress; import java.util.Collections; +import java.util.Map; @RunWith(RobolectricTestRunner.class) public class PhysicalNetworkStateTest { @@ -25,176 +24,184 @@ public class PhysicalNetworkStateTest { private static final Network CELL = ShadowNetwork.newInstance(102); private static final Network VPN = ShadowNetwork.newInstance(103); - @Test - @org.robolectric.annotation.Config(sdk = 24) - public void olderAndroidStoresIndependentCallbackSnapshots() throws Exception { - PhysicalNetworkState state = new PhysicalNetworkState(); - NetworkCapabilities caps = capabilities(NetworkCapabilities.TRANSPORT_WIFI); - state.onPhysicalCapabilitiesChanged(WIFI, caps); - LinkProperties props = linkProperties("9.9.9.9"); - state.onPhysicalLinkPropertiesChanged(WIFI, props); - props.setDnsServers(Collections.singleton(InetAddress.getByName("1.1.1.1"))); - assertEquals(NetworkReloadPolicy.REASON_LINK_PROPERTIES_CHANGED, - state.onPhysicalLinkPropertiesChanged(WIFI, props)); + private static NetworkCapabilities capabilities(int transport) { + NetworkCapabilities caps = ShadowNetworkCapabilities.newInstance(); + Shadows.shadowOf(caps).addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET); + Shadows.shadowOf(caps).addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN); + Shadows.shadowOf(caps).addTransportType(transport); + return caps; } - private static NetworkCapabilities capabilities(int transport) { - NetworkCapabilities capabilities = ShadowNetworkCapabilities.newInstance(); - ShadowNetworkCapabilities shadow = Shadows.shadowOf(capabilities); - shadow.addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET); - shadow.addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN); - shadow.addTransportType(transport); - return capabilities; + private static NetworkCapabilities vpn(int transport) { + NetworkCapabilities caps = capabilities(transport); + Shadows.shadowOf(caps).removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN); + Shadows.shadowOf(caps).addTransportType(NetworkCapabilities.TRANSPORT_VPN); + return caps; } - private static LinkProperties linkProperties(String dns) throws Exception { - LinkProperties properties = new LinkProperties(); - properties.setDnsServers(Collections.singleton(InetAddress.getByName(dns))); - return properties; + private static LinkProperties links(String address, String dns) throws Exception { + LinkProperties props = new LinkProperties(); + props.setLinkAddresses(Collections.singleton(linkAddress(address))); + props.setDnsServers(Collections.singleton(InetAddress.getByName(dns))); + return props; } - @Test - public void vpnDefaultTransportHandoverReloadsWithoutVpnIdentityChurn() throws Exception { + private static PhysicalNetworkState wifiWithStandbyCell() throws Exception { PhysicalNetworkState state = new PhysicalNetworkState(); - NetworkCapabilities wifiVpn = capabilities(NetworkCapabilities.TRANSPORT_WIFI); - Shadows.shadowOf(wifiVpn).removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN); - Shadows.shadowOf(wifiVpn).addTransportType(NetworkCapabilities.TRANSPORT_VPN); - assertNull(state.onDefaultNetworkCapabilitiesChanged(VPN, wifiVpn)); - - NetworkCapabilities cellVpn = capabilities(NetworkCapabilities.TRANSPORT_CELLULAR); - Shadows.shadowOf(cellVpn).removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN); - Shadows.shadowOf(cellVpn).addTransportType(NetworkCapabilities.TRANSPORT_VPN); + state.onPhysicalAvailable(WIFI); + state.onPhysicalCapabilitiesChanged(WIFI, capabilities(NetworkCapabilities.TRANSPORT_WIFI)); + state.onPhysicalLinkPropertiesChanged(WIFI, links("192.0.2.2/24", "9.9.9.9")); + state.onPhysicalAvailable(CELL); + state.onPhysicalCapabilitiesChanged(CELL, capabilities(NetworkCapabilities.TRANSPORT_CELLULAR)); + state.onPhysicalLinkPropertiesChanged(CELL, links("198.51.100.2/24", "1.1.1.1")); assertEquals(NetworkReloadPolicy.REASON_NETWORK_CHANGED, - state.onDefaultNetworkCapabilitiesChanged(VPN, cellVpn)); + state.onDefaultNetworkCapabilitiesChanged(VPN, vpn(NetworkCapabilities.TRANSPORT_WIFI))); + assertEquals(WIFI, state.getDefaultNetwork()); + return state; + } + + @Test + public void standbyChatterAndLossNeverReloadActiveWifi() throws Exception { + PhysicalNetworkState state = wifiWithStandbyCell(); + NetworkCapabilities cell = capabilities(NetworkCapabilities.TRANSPORT_CELLULAR); + Shadows.shadowOf(cell).addCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED); + Shadows.shadowOf(cell).addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED); + Shadows.shadowOf(cell).addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED); + assertNull(state.onPhysicalCapabilitiesChanged(CELL, cell)); + assertNull(state.onPhysicalLinkPropertiesChanged(CELL, links("198.51.100.3/24", "8.8.8.8"))); + assertNull(state.onPhysicalLost(CELL)); + assertNull(state.onPhysicalLost(CELL)); + assertEquals(WIFI, state.getDefaultNetwork()); + } + @Test + public void vpnTransportHandoverSelectsCellAndIgnoresLateWifiLoss() throws Exception { + PhysicalNetworkState state = wifiWithStandbyCell(); + assertEquals(NetworkReloadPolicy.REASON_NETWORK_CHANGED, + state.onDefaultNetworkCapabilitiesChanged(VPN, vpn(NetworkCapabilities.TRANSPORT_CELLULAR))); + assertEquals(CELL, state.getDefaultNetwork()); + assertNull(state.onPhysicalLost(WIFI)); Network replacement = ShadowNetwork.newInstance(104); assertNull(state.onDefaultNetworkAvailable(replacement)); - assertNull(state.onDefaultNetworkCapabilitiesChanged(replacement, cellVpn)); - assertNull(state.onDefaultNetworkLinkPropertiesChanged(replacement, - linkProperties("9.9.9.9"))); + NetworkCapabilities uninitialised = ShadowNetworkCapabilities.newInstance(); + Shadows.shadowOf(uninitialised).removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN); + Shadows.shadowOf(uninitialised).addTransportType(NetworkCapabilities.TRANSPORT_VPN); + assertNull(state.onDefaultNetworkCapabilitiesChanged(replacement, uninitialised)); + assertNull(state.onDefaultNetworkCapabilitiesChanged(replacement, vpn(NetworkCapabilities.TRANSPORT_CELLULAR))); + assertNull(state.onDefaultNetworkLinkPropertiesChanged(replacement, links("10.0.0.2/32", "10.0.0.1"))); assertNull(state.onDefaultNetworkLost(VPN)); } @Test - public void physicalCallbacksWorkWhileVpnIsDefault() { - PhysicalNetworkState state = new PhysicalNetworkState(); - - assertEquals(NetworkReloadPolicy.REASON_NETWORK_AVAILABLE, - state.onPhysicalAvailable(WIFI)); + public void unvalidatedPhysicalDefaultSwitchIsDetected() throws Exception { + PhysicalNetworkState state = wifiWithStandbyCell(); + assertNull(state.onDefaultNetworkCapabilitiesChanged(WIFI, capabilities(NetworkCapabilities.TRANSPORT_WIFI))); assertEquals(NetworkReloadPolicy.REASON_NETWORK_CHANGED, - state.onPhysicalCapabilitiesChanged(WIFI, capabilities( - NetworkCapabilities.TRANSPORT_WIFI))); - assertNull(state.onPhysicalAvailable(WIFI)); - assertNull(state.getDefaultNetwork()); + state.onDefaultNetworkCapabilitiesChanged(CELL, capabilities(NetworkCapabilities.TRANSPORT_CELLULAR))); } @Test - public void defaultSwitchIsDetectedWhenBothPhysicalNetworksRemain() { - PhysicalNetworkState state = new PhysicalNetworkState(); - state.onPhysicalAvailable(WIFI); - state.onPhysicalCapabilitiesChanged(WIFI, capabilities(NetworkCapabilities.TRANSPORT_WIFI)); - state.onPhysicalAvailable(CELL); - state.onPhysicalCapabilitiesChanged(CELL, capabilities(NetworkCapabilities.TRANSPORT_CELLULAR)); + public void sameTransportStandbyDoesNotDisplaceLiveEgress() throws Exception { + PhysicalNetworkState state = wifiWithStandbyCell(); + Network otherWifi = ShadowNetwork.newInstance(104); + assertNull(state.onPhysicalAvailable(otherWifi)); + assertNull(state.onPhysicalCapabilitiesChanged(otherWifi, capabilities(NetworkCapabilities.TRANSPORT_WIFI))); + assertEquals(WIFI, state.getDefaultNetwork()); + assertEquals(NetworkReloadPolicy.REASON_NETWORK_CHANGED, state.onPhysicalLost(WIFI)); + assertEquals(otherWifi, state.getDefaultNetwork()); + assertNull(state.onPhysicalCapabilitiesChanged(WIFI, capabilities(NetworkCapabilities.TRANSPORT_WIFI))); + assertNull(state.onPhysicalLinkPropertiesChanged(WIFI, links("192.0.2.2/24", "9.9.9.9"))); + } - assertNull(state.onDefaultNetworkAvailable(WIFI)); - assertNull(state.onDefaultNetworkCapabilitiesChanged(WIFI, - capabilities(NetworkCapabilities.TRANSPORT_WIFI))); + @Test + @Config(sdk = 23) + public void suppliedDefaultSnapshotSelectsEgressOnApi23() throws Exception { + PhysicalNetworkState state = wifiWithStandbyCell(); assertEquals(NetworkReloadPolicy.REASON_NETWORK_CHANGED, - state.onDefaultNetworkAvailable(CELL)); + state.onDefaultNetworkCapabilitiesChanged(VPN, vpn(NetworkCapabilities.TRANSPORT_CELLULAR))); assertEquals(CELL, state.getDefaultNetwork()); } @Test - public void defaultPhysicalTransportChangeReloads() { - PhysicalNetworkState state = new PhysicalNetworkState(); - state.onPhysicalAvailable(WIFI); - state.onPhysicalCapabilitiesChanged(WIFI, capabilities(NetworkCapabilities.TRANSPORT_WIFI)); - state.onDefaultNetworkAvailable(WIFI); - state.onDefaultNetworkCapabilitiesChanged(WIFI, - capabilities(NetworkCapabilities.TRANSPORT_WIFI)); - - assertEquals(NetworkReloadPolicy.REASON_NETWORK_CHANGED, - state.onDefaultNetworkCapabilitiesChanged(WIFI, - capabilities(NetworkCapabilities.TRANSPORT_CELLULAR))); - assertEquals(WIFI, state.getDefaultNetwork()); + public void activeValidationSuspensionAndSignalChangesAreIgnored() throws Exception { + PhysicalNetworkState state = wifiWithStandbyCell(); + NetworkCapabilities wifi = capabilities(NetworkCapabilities.TRANSPORT_WIFI); + Shadows.shadowOf(wifi).addCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED); + Shadows.shadowOf(wifi).addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED); + Shadows.shadowOf(wifi).setLinkDownstreamBandwidthKbps(12000); + // There is no public setter on all tested SDKs. Fail loudly if the + // AOSP field changes when updating Robolectric's Android runtime. + setField(wifi, "mSignalStrength", -55); + assertNull(state.onPhysicalCapabilitiesChanged(WIFI, wifi)); } @Test - public void signalAndBandwidthChatterDoesNotReload() throws Exception { - PhysicalNetworkState state = new PhysicalNetworkState(); - state.onPhysicalAvailable(CELL); - NetworkCapabilities initial = capabilities(NetworkCapabilities.TRANSPORT_CELLULAR); - state.onPhysicalCapabilitiesChanged(CELL, initial); - - NetworkCapabilities chatter = new NetworkCapabilities(initial); - ShadowNetworkCapabilities chatterShadow = Shadows.shadowOf(chatter); - chatterShadow.setLinkDownstreamBandwidthKbps(12000); - chatterShadow.setLinkUpstreamBandwidthKbps(3000); - setSignalStrength(chatter, -55); - assertNull(state.onPhysicalCapabilitiesChanged(CELL, chatter)); + public void activeMeteredAndDnsChangesReloadPolicyWithoutForcingWireGuard() throws Exception { + PhysicalNetworkState state = wifiWithStandbyCell(); + NetworkCapabilities wifi = capabilities(NetworkCapabilities.TRANSPORT_WIFI); + Shadows.shadowOf(wifi).addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED); + String metered = state.onPhysicalCapabilitiesChanged(WIFI, wifi); + assertEquals(NetworkReloadPolicy.REASON_METERED_CHANGED, metered); + assertFalse(NetworkReloadPolicy.shouldRestartWireGuard(metered)); + String dns = state.onPhysicalLinkPropertiesChanged(WIFI, links("192.0.2.2/24", "8.8.8.8")); + assertEquals(NetworkReloadPolicy.REASON_DNS_CHANGED, dns); + assertFalse(NetworkReloadPolicy.shouldRestartWireGuard(dns)); } @Test - public void standbyLossIsTrackedAndStaleLossIsIgnored() { - PhysicalNetworkState state = new PhysicalNetworkState(); - state.onPhysicalAvailable(WIFI); - state.onPhysicalCapabilitiesChanged(WIFI, capabilities(NetworkCapabilities.TRANSPORT_WIFI)); - state.onPhysicalAvailable(CELL); - state.onPhysicalCapabilitiesChanged(CELL, capabilities(NetworkCapabilities.TRANSPORT_CELLULAR)); - - assertEquals(NetworkReloadPolicy.REASON_NETWORK_LOST, - state.onPhysicalLost(CELL)); - assertNull(state.onPhysicalLost(CELL)); - assertNull(state.onPhysicalAvailable(WIFI)); + @Config(sdk = 24) + public void activeAddressChangesUseIndependentSnapshotsOnOlderAndroid() throws Exception { + PhysicalNetworkState state = wifiWithStandbyCell(); + LinkProperties props = links("192.0.2.2/24", "9.9.9.9"); + assertNull(state.onPhysicalLinkPropertiesChanged(WIFI, props)); + props.setLinkAddresses(Collections.singleton(linkAddress("192.0.2.3/24"))); + String change = state.onPhysicalLinkPropertiesChanged(WIFI, props); + assertEquals(NetworkReloadPolicy.REASON_LINK_PROPERTIES_CHANGED, change); + assertTrue(NetworkReloadPolicy.shouldRestartWireGuard(change)); } @Test - public void privateDnsOnlyChangeDoesNotRestartWireGuard() throws Exception { - PhysicalNetworkState state = new PhysicalNetworkState(); - state.onPhysicalAvailable(WIFI); - state.onPhysicalCapabilitiesChanged(WIFI, capabilities(NetworkCapabilities.TRANSPORT_WIFI)); - LinkProperties initial = linkProperties("9.9.9.9"); - state.onPhysicalLinkPropertiesChanged(WIFI, initial); - - LinkProperties pinned = linkProperties("9.9.9.9"); - setPrivateDns(pinned, "dns.example", true); - String change = state.onPhysicalLinkPropertiesChanged(WIFI, pinned); - assertEquals(NetworkReloadPolicy.REASON_PRIVATE_DNS_CHANGED, change); - assertFalse(NetworkReloadPolicy.shouldRestartWireGuard(change)); + @Config(sdk = 28) + public void privateDnsActiveAndHostnameChangesAreBothPolicyOnly() throws Exception { + PhysicalNetworkState state = wifiWithStandbyCell(); + LinkProperties props = links("192.0.2.2/24", "9.9.9.9"); + // Private DNS setters are hidden framework APIs; use reflection only + // in the fixture, keeping production on the public getters. + setField(props, "mUsePrivateDns", true); + String active = state.onPhysicalLinkPropertiesChanged(WIFI, props); + assertEquals(NetworkReloadPolicy.REASON_PRIVATE_DNS_CHANGED, active); + assertFalse(NetworkReloadPolicy.shouldRestartWireGuard(active)); + setField(props, "mPrivateDnsServerName", "dns.example"); + assertEquals(NetworkReloadPolicy.REASON_PRIVATE_DNS_CHANGED, + state.onPhysicalLinkPropertiesChanged(WIFI, props)); + assertNull(state.onPhysicalLinkPropertiesChanged(WIFI, props)); } @Test - public void vpnDefaultCallbacksDoNotCreatePhysicalDefault() throws Exception { + public void defaultOnlyNetworksCannotAccumulatePhysicalEntries() throws Exception { PhysicalNetworkState state = new PhysicalNetworkState(); - state.onPhysicalAvailable(WIFI); - state.onPhysicalCapabilitiesChanged(WIFI, capabilities(NetworkCapabilities.TRANSPORT_WIFI)); - state.onDefaultNetworkAvailable(WIFI); - state.onDefaultNetworkCapabilitiesChanged(WIFI, capabilities(NetworkCapabilities.TRANSPORT_WIFI)); - - NetworkCapabilities vpn = ShadowNetworkCapabilities.newInstance(); - ShadowNetworkCapabilities vpnShadow = Shadows.shadowOf(vpn); - vpnShadow.addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET); - vpnShadow.removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN); - vpnShadow.addTransportType(NetworkCapabilities.TRANSPORT_VPN); - assertNull(state.onDefaultNetworkAvailable(VPN)); - assertNull(state.onDefaultNetworkCapabilitiesChanged(VPN, vpn)); - assertNull(state.onDefaultNetworkLinkPropertiesChanged(VPN, - linkProperties("1.1.1.1"))); - assertNull(state.onDefaultNetworkLost(VPN)); - assertEquals(WIFI, state.getDefaultNetwork()); + for (int id = 200; id < 220; id++) { + Network network = ShadowNetwork.newInstance(id); + state.onDefaultNetworkAvailable(network); + state.onDefaultNetworkCapabilitiesChanged(network, capabilities(NetworkCapabilities.TRANSPORT_WIFI)); + state.onDefaultNetworkLost(network); + } + Field entries = PhysicalNetworkState.class.getDeclaredField("entries"); + entries.setAccessible(true); + assertTrue(((Map) entries.get(state)).isEmpty()); + assertNull(state.onPhysicalLost(WIFI)); } - private static void setPrivateDns(LinkProperties properties, String name, boolean active) - throws Exception { - Field nameField = LinkProperties.class.getDeclaredField("mPrivateDnsServerName"); - nameField.setAccessible(true); - nameField.set(properties, name); + private static void setField(Object object, String name, Object value) throws Exception { + Field field = object.getClass().getDeclaredField(name); + field.setAccessible(true); + field.set(object, value); } - private static void setSignalStrength(NetworkCapabilities capabilities, int strength) - throws Exception { - Field signalField = NetworkCapabilities.class.getDeclaredField("mSignalStrength"); - signalField.setAccessible(true); - signalField.setInt(capabilities, strength); + private static LinkAddress linkAddress(String address) throws Exception { + // LinkAddress's value constructor is hidden in the public SDK stub. + String[] parts = address.split("/"); + return LinkAddress.class.getDeclaredConstructor(InetAddress.class, int.class) + .newInstance(InetAddress.getByName(parts[0]), Integer.parseInt(parts[1])); } } diff --git a/app/src/test/java/eu/faircode/netguard/ServiceSinkholeNetworkReloadTest.java b/app/src/test/java/eu/faircode/netguard/ServiceSinkholeNetworkReloadTest.java new file mode 100644 index 000000000..61b63e2e6 --- /dev/null +++ b/app/src/test/java/eu/faircode/netguard/ServiceSinkholeNetworkReloadTest.java @@ -0,0 +1,167 @@ +package eu.faircode.netguard; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import android.content.Context; +import android.content.Intent; +import android.content.SharedPreferences; +import android.os.Looper; + +import androidx.preference.PreferenceManager; + +import net.kollnig.missioncontrol.wg.WgEgress; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.RuntimeEnvironment; +import org.robolectric.Shadows; +import org.robolectric.shadows.ShadowApplication; +import org.robolectric.shadows.ShadowLooper; + +import java.lang.reflect.Constructor; +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.util.List; +import java.util.concurrent.TimeUnit; + +@RunWith(RobolectricTestRunner.class) +public class ServiceSinkholeNetworkReloadTest { + // Invoke the private debounce/command seams without onCreate(), which starts + // the native VPN. Reflection keeps these test seams out of the service API. + private Context context; + private SharedPreferences prefs; + private ShadowApplication shadowApplication; + + private static class TestService extends ServiceSinkhole { + void attach(Context context) { + attachBaseContext(context); + } + } + + @Before + public void setUp() { + context = RuntimeEnvironment.getApplication(); + prefs = PreferenceManager.getDefaultSharedPreferences(context); + prefs.edit().clear().commit(); + shadowApplication = Shadows.shadowOf(RuntimeEnvironment.getApplication()); + shadowApplication.clearStartedServices(); + } + + @Test + public void networkChangeSurvivesPrivateDnsReasonInDebouncedBurst() throws Exception { + prefs.edit().putBoolean("enabled", true).commit(); + TestService service = newService(); + + invokeReloadAfterNetworkChange(service, NetworkReloadPolicy.REASON_NETWORK_CHANGED); + invokeReloadAfterNetworkChange(service, NetworkReloadPolicy.REASON_PRIVATE_DNS_CHANGED); + idleDebounce(); + + List intents = shadowApplication.getAllStartedServices(); + assertEquals(1, intents.size()); + Intent dispatched = shadowApplication.getNextStartedService(); + assertTrue(dispatched.getBooleanExtra(networkChangedExtra(), false)); + assertNull(shadowApplication.getNextStartedService()); + } + + @Test + public void privateDnsChangeDoesNotMarkWireGuardNetworkChanged() throws Exception { + prefs.edit().putBoolean("enabled", true).commit(); + invokeReloadAfterNetworkChange(newService(), NetworkReloadPolicy.REASON_PRIVATE_DNS_CHANGED); + idleDebounce(); + + List intents = shadowApplication.getAllStartedServices(); + assertEquals(1, intents.size()); + Intent dispatched = shadowApplication.getNextStartedService(); + assertFalse(dispatched.getBooleanExtra(networkChangedExtra(), false)); + assertNull(shadowApplication.getNextStartedService()); + } + + @Test + public void disabledReloadDoesNotDispatchOrMarkWireGuard() throws Exception { + boolean oldPending = forceRestartPending(); + try { + setForceRestartPending(false); + invokeReloadAfterNetworkChange(newService(), NetworkReloadPolicy.REASON_NETWORK_CHANGED); + idleDebounce(); + + assertTrue(shadowApplication.getAllStartedServices().isEmpty()); + assertFalse(forceRestartPending()); + } finally { + setForceRestartPending(oldPending); + } + } + + @Test + public void droppedReloadDoesNotMarkWireGuard() throws Exception { + TestService service = newService(); + Field foreground = field(ServiceSinkhole.class, "user_foreground"); + boolean oldForeground = foreground.getBoolean(service); + boolean oldPending = forceRestartPending(); + try { + foreground.setBoolean(service, false); + setForceRestartPending(false); + + Class handlerClass = Class.forName( + "eu.faircode.netguard.ServiceSinkhole$CommandHandler"); + Constructor constructor = handlerClass.getDeclaredConstructor( + ServiceSinkhole.class, Looper.class); + constructor.setAccessible(true); + Object handler = constructor.newInstance(service, Looper.getMainLooper()); + Method handleIntent = handlerClass.getDeclaredMethod("handleIntent", Intent.class); + handleIntent.setAccessible(true); + Intent intent = new Intent(context, ServiceSinkhole.class); + intent.putExtra(ServiceSinkhole.EXTRA_COMMAND, ServiceSinkhole.Command.reload); + intent.putExtra(networkChangedExtra(), true); + handleIntent.invoke(handler, intent); + + assertFalse(forceRestartPending()); + assertTrue(shadowApplication.getAllStartedServices().isEmpty()); + } finally { + foreground.setBoolean(service, oldForeground); + setForceRestartPending(oldPending); + } + } + + private TestService newService() { + TestService service = new TestService(); + service.attach(context); + return service; + } + + private static void invokeReloadAfterNetworkChange(ServiceSinkhole service, String reason) + throws Exception { + Method reload = ServiceSinkhole.class.getDeclaredMethod( + "reloadAfterNetworkChange", String.class); + reload.setAccessible(true); + reload.invoke(service, reason); + } + + private static void idleDebounce() { + ShadowLooper shadowLooper = Shadows.shadowOf(Looper.getMainLooper()); + shadowLooper.idleFor(1600, TimeUnit.MILLISECONDS); + } + + private static String networkChangedExtra() throws Exception { + Field field = field(ServiceSinkhole.class, "EXTRA_WG_NETWORK_CHANGED"); + return (String) field.get(null); + } + + private static boolean forceRestartPending() throws Exception { + return field(WgEgress.class, "forceRestartPending").getBoolean(WgEgress.INSTANCE); + } + + private static void setForceRestartPending(boolean value) throws Exception { + field(WgEgress.class, "forceRestartPending").setBoolean(WgEgress.INSTANCE, value); + } + + private static Field field(Class type, String name) throws Exception { + Field field = type.getDeclaredField(name); + field.setAccessible(true); + return field; + } +} diff --git a/app/src/test/java/net/kollnig/missioncontrol/wg/WgEgressRecoveryTest.java b/app/src/test/java/net/kollnig/missioncontrol/wg/WgEgressRecoveryTest.java index d35388511..29c65c590 100644 --- a/app/src/test/java/net/kollnig/missioncontrol/wg/WgEgressRecoveryTest.java +++ b/app/src/test/java/net/kollnig/missioncontrol/wg/WgEgressRecoveryTest.java @@ -2,13 +2,25 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertNull; + +import android.net.VpnService; +import android.os.ParcelFileDescriptor; + +import net.kollnig.missioncontrol.wgbridge.Tunnel; import java.lang.reflect.Constructor; import java.lang.reflect.Field; +import java.util.HashMap; +import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicLong; -import net.kollnig.missioncontrol.wgbridge.Tunnel; +import kotlin.Unit; +import kotlin.jvm.functions.Function0; + +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; /** * Screen-state keepalive toggling for the WireGuard egress. @@ -19,6 +31,7 @@ * {@link WgEgress#onInteractiveStateChanged} now only re-applies the keepalive * interval and must be a safe no-op when there is no running tunnel. */ +@RunWith(RobolectricTestRunner.class) public class WgEgressRecoveryTest { @org.junit.Test public void interactiveStateChangeIsNoopWhenWireGuardIsDisabled() { @@ -32,69 +45,107 @@ public void interactiveStateChangeIsNoopWhenConfigIsMissing() { } @org.junit.Test - public void underlyingNetworkChangeIsNoopWhenWireGuardIsIdle() throws Exception { + public void networkChangedBypassesSameConfigShortcutBeforeNativeStart() throws Exception { + // No production test seam exists for private lifecycle state. These + // fields seed the exact same config/PFD identity and are restored + // below so this singleton cannot affect later tests. Field tunnel = field("tunnel"); - Field pending = field("forceRestartPending"); - Field attempts = field("restartAttempts"); - Field generation = field("verificationGeneration"); - Field reload = field("requestReloadCb"); - Object oldTunnel = tunnel.get(WgEgress.INSTANCE); - boolean oldPending = pending.getBoolean(WgEgress.INSTANCE); - int oldAttempts = attempts.getInt(WgEgress.INSTANCE); - long oldGeneration = generation.getLong(WgEgress.INSTANCE); - Object oldReload = reload.get(WgEgress.INSTANCE); - AtomicInteger reloads = new AtomicInteger(); - try { - tunnel.set(WgEgress.INSTANCE, null); - pending.setBoolean(WgEgress.INSTANCE, false); - attempts.setInt(WgEgress.INSTANCE, 7); - reload.set(WgEgress.INSTANCE, (Runnable) reloads::incrementAndGet); - - WgEgress.INSTANCE.onUnderlyingNetworkChanged(); + Field currentConfig = field("currentConfig"); + Field currentTunFd = field("currentTunFd"); + Field currentTunPfd = field("currentTunPfd"); + Field currentKeepaliveAlwaysOn = field("currentKeepaliveAlwaysOn"); + Field forceRestartPending = field("forceRestartPending"); + Field lastCheapRecoveryMs = field("lastCheapRecoveryMs"); + Field verificationGeneration = field("verificationGeneration"); + Field tunnelGeneration = field("tunnelGeneration"); + Field recoveryNotificationGeneration = field("recoveryNotificationGeneration"); + Field lastError = field("lastError"); + Field providerFailureReason = field("providerFailureReason"); + Field pendingProviderFailure = field("pendingProviderFailure"); + Field pendingRestartTunnel = field("pendingRestartTunnel"); + Field pendingRestartTunnelGeneration = field("pendingRestartTunnelGeneration"); + Field endpointCache = field("endpointCache"); - assertFalse(pending.getBoolean(WgEgress.INSTANCE)); - assertEquals(7, attempts.getInt(WgEgress.INSTANCE)); - assertEquals(0, reloads.get()); - } finally { - tunnel.set(WgEgress.INSTANCE, oldTunnel); - pending.setBoolean(WgEgress.INSTANCE, oldPending); - attempts.setInt(WgEgress.INSTANCE, oldAttempts); - generation.setLong(WgEgress.INSTANCE, oldGeneration); - reload.set(WgEgress.INSTANCE, oldReload); - } - } - - @org.junit.Test - public void underlyingNetworkChangeMarksRunningTunnelForOneRestart() throws Exception { - Field tunnel = field("tunnel"); - Field pending = field("forceRestartPending"); - Field attempts = field("restartAttempts"); - Field generation = field("verificationGeneration"); - Field reload = field("requestReloadCb"); Object oldTunnel = tunnel.get(WgEgress.INSTANCE); - boolean oldPending = pending.getBoolean(WgEgress.INSTANCE); - int oldAttempts = attempts.getInt(WgEgress.INSTANCE); - long oldGeneration = generation.getLong(WgEgress.INSTANCE); - Object oldReload = reload.get(WgEgress.INSTANCE); - AtomicInteger reloads = new AtomicInteger(); + String oldConfig = (String) currentConfig.get(WgEgress.INSTANCE); + int oldTunFd = currentTunFd.getInt(WgEgress.INSTANCE); + Object oldTunPfd = currentTunPfd.get(WgEgress.INSTANCE); + boolean oldKeepaliveAlwaysOn = currentKeepaliveAlwaysOn.getBoolean(WgEgress.INSTANCE); + boolean oldForceRestartPending = forceRestartPending.getBoolean(WgEgress.INSTANCE); + long oldLastCheapRecoveryMs = lastCheapRecoveryMs.getLong(WgEgress.INSTANCE); + long oldVerificationGeneration = verificationGeneration.getLong(WgEgress.INSTANCE); + AtomicLong generations = (AtomicLong) tunnelGeneration.get(WgEgress.INSTANCE); + long oldTunnelGeneration = generations.get(); + long oldRecoveryNotificationGeneration = + recoveryNotificationGeneration.getLong(WgEgress.INSTANCE); + Object oldLastError = lastError.get(WgEgress.INSTANCE); + Object oldProviderFailureReason = providerFailureReason.get(WgEgress.INSTANCE); + Object oldPendingProviderFailure = pendingProviderFailure.get(WgEgress.INSTANCE); + Object oldPendingRestartTunnel = pendingRestartTunnel.get(WgEgress.INSTANCE); + long oldPendingRestartTunnelGeneration = + pendingRestartTunnelGeneration.getLong(WgEgress.INSTANCE); + Map oldEndpointCache = new HashMap<>((Map) endpointCache.get(WgEgress.INSTANCE)); + + ParcelFileDescriptor[] pipe = ParcelFileDescriptor.createPipe(); + ParcelFileDescriptor vpnFd = pipe[0]; + AtomicInteger starts = new AtomicInteger(); + AtomicInteger stops = new AtomicInteger(); + String invalidConfig = "malformed"; try { tunnel.set(WgEgress.INSTANCE, newTunnel(0L)); - pending.setBoolean(WgEgress.INSTANCE, false); - attempts.setInt(WgEgress.INSTANCE, 11); - reload.set(WgEgress.INSTANCE, (Runnable) reloads::incrementAndGet); + currentConfig.set(WgEgress.INSTANCE, invalidConfig); + currentTunFd.setInt(WgEgress.INSTANCE, vpnFd.getFd()); + currentTunPfd.set(WgEgress.INSTANCE, vpnFd); + forceRestartPending.setBoolean(WgEgress.INSTANCE, false); - WgEgress.INSTANCE.onUnderlyingNetworkChanged(); - WgEgress.INSTANCE.onUnderlyingNetworkChanged(); + boolean result = WgEgress.INSTANCE.startOrUpdate( + true, + invalidConfig, + new VpnService(), + vpnFd, + false, + false, + new Function0() { + @Override + public Integer invoke() { + starts.incrementAndGet(); + return -1; + } + }, + new Function0() { + @Override + public Unit invoke() { + stops.incrementAndGet(); + return Unit.INSTANCE; + } + }, + true); - assertTrue(pending.getBoolean(WgEgress.INSTANCE)); - assertEquals(11, attempts.getInt(WgEgress.INSTANCE)); - assertEquals(0, reloads.get()); + assertFalse("invalid config must fail after the existing tunnel is stopped", result); + assertEquals("same-config handover must stop the old tunnel", 1, stops.get()); + assertEquals("config parsing must fail before JNI socket setup", 0, starts.get()); + assertNull(tunnel.get(WgEgress.INSTANCE)); } finally { + vpnFd.close(); + pipe[1].close(); tunnel.set(WgEgress.INSTANCE, oldTunnel); - pending.setBoolean(WgEgress.INSTANCE, oldPending); - attempts.setInt(WgEgress.INSTANCE, oldAttempts); - generation.setLong(WgEgress.INSTANCE, oldGeneration); - reload.set(WgEgress.INSTANCE, oldReload); + currentConfig.set(WgEgress.INSTANCE, oldConfig); + currentTunFd.setInt(WgEgress.INSTANCE, oldTunFd); + currentTunPfd.set(WgEgress.INSTANCE, oldTunPfd); + currentKeepaliveAlwaysOn.setBoolean(WgEgress.INSTANCE, oldKeepaliveAlwaysOn); + forceRestartPending.setBoolean(WgEgress.INSTANCE, oldForceRestartPending); + lastCheapRecoveryMs.setLong(WgEgress.INSTANCE, oldLastCheapRecoveryMs); + verificationGeneration.setLong(WgEgress.INSTANCE, oldVerificationGeneration); + generations.set(oldTunnelGeneration); + recoveryNotificationGeneration.setLong(WgEgress.INSTANCE, oldRecoveryNotificationGeneration); + lastError.set(WgEgress.INSTANCE, oldLastError); + providerFailureReason.set(WgEgress.INSTANCE, oldProviderFailureReason); + pendingProviderFailure.set(WgEgress.INSTANCE, oldPendingProviderFailure); + pendingRestartTunnel.set(WgEgress.INSTANCE, oldPendingRestartTunnel); + pendingRestartTunnelGeneration.setLong(WgEgress.INSTANCE, oldPendingRestartTunnelGeneration); + Map cache = (Map) endpointCache.get(WgEgress.INSTANCE); + cache.clear(); + cache.putAll((Map) oldEndpointCache); } } @@ -105,6 +156,8 @@ private static Field field(String name) throws Exception { } private static Tunnel newTunnel(long handle) throws Exception { + // Tunnel.stop() checks the handle before entering nativeStop(), so a + // zero-handle fake exercises lifecycle cleanup without JNI. Constructor constructor = Tunnel.class.getDeclaredConstructor(long.class); constructor.setAccessible(true); return constructor.newInstance(handle); From 81c6a67dc57c6c0c9b57de96e51aa447442b2847 Mon Sep 17 00:00:00 2001 From: Konrad Kollnig <5175206+kasnder@users.noreply.github.com> Date: Thu, 17 Sep 2026 11:01:53 +0200 Subject: [PATCH 4/4] Bump Android NDK to 29.0.14206865 (r29) Verified libwgbridge.so cross-compile (all 4 ABIs), assembleGithubDebug (native CMake + Rust builds), and testGithubDebugUnitTest all pass. Co-Authored-By: Claude Sonnet 5 --- README.md | 2 +- agents/docs/build-and-test.md | 2 +- app/gradle/wgbridge.gradle | 2 +- wgbridge-rs/README.md | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 57fdb0fd2..debb372d9 100644 --- a/README.md +++ b/README.md @@ -133,7 +133,7 @@ In combination with F-Droid, this repository uses automated builds and follows a You need: - Android Studio (with the Android SDK and build tools) -- Android NDK 27.2.12479018 (r27c) +- Android NDK 29.0.14206865 (r29) - Rust via [rustup](https://rustup.rs), for the WireGuard engine ([gotatun](https://github.com/mullvad/gotatun), built from source in `wgbridge-rs/`). The compiler, Android targets, and `cargo-ndk` version are pinned; install them and pre-fetch locked crates with: ```bash ./scripts/setup_rust_android.sh diff --git a/agents/docs/build-and-test.md b/agents/docs/build-and-test.md index 591a162a7..facd53134 100644 --- a/agents/docs/build-and-test.md +++ b/agents/docs/build-and-test.md @@ -5,7 +5,7 @@ flavour matrix, the native builds, and the reproducibility flags. ## Prerequisites -JDK 17, Android SDK (compile/target SDK 37, min SDK 23), NDK `27.2.12479018`, +JDK 17, Android SDK (compile/target SDK 37, min SDK 23), NDK `29.0.14206865`, CMake. Native builds also need Rust ≥ 1.95 with the four Android targets; the WireGuard bridge additionally needs `cargo-ndk`. Gradle wires both Rust builds in but deliberately does not install tools or fetch crates. See diff --git a/app/gradle/wgbridge.gradle b/app/gradle/wgbridge.gradle index 51340e059..4a0e3f801 100644 --- a/app/gradle/wgbridge.gradle +++ b/app/gradle/wgbridge.gradle @@ -12,7 +12,7 @@ import org.gradle.process.ExecOperations def wgbridgeSrcDir = file("$rootDir/wgbridge-rs") def wgbridgeOutDir = layout.buildDirectory.dir("rustJniLibs").get().asFile def wgbridgeAbis = ['armeabi-v7a', 'arm64-v8a', 'x86', 'x86_64'] -ext.wgbridgeNdkVersion = '27.2.12479018' // keep in sync with defaultConfig.ndkVersion +ext.wgbridgeNdkVersion = '29.0.14206865' // keep in sync with defaultConfig.ndkVersion def wgbridgeNdkVersion = ext.wgbridgeNdkVersion def wgbridgeCargoNdkVersion = '4.1.2' // keep in sync with scripts/setup_rust_android.sh def wgbridgeExecOperations = project.services.get(ExecOperations) diff --git a/wgbridge-rs/README.md b/wgbridge-rs/README.md index 175ed2b89..77435894a 100644 --- a/wgbridge-rs/README.md +++ b/wgbridge-rs/README.md @@ -64,7 +64,7 @@ runtime code in the APK. - **Rust 1.95.0** via [rustup](https://rustup.rs). The version and Android targets are pinned in the repository's `rust-toolchain.toml`. - **cargo-ndk 4.1.2**. -- **Android NDK 27.2.12479018 (r27c)** (the Gradle task points cargo-ndk at +- **Android NDK 29.0.14206865 (r29)** (the Gradle task points cargo-ndk at the NDK configured for the app module). Install the pinned Rust prerequisites and pre-fetch the locked crates with: @@ -109,7 +109,7 @@ sudo: - apt-get install -y rustup gcc libc-dev prebuild: - ../scripts/setup_rust_android.sh -ndk: r27c +ndk: r29 ``` The existing `gradle: [fdroid]` setting remains unchanged. The prebuild step