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/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 new file mode 100644 index 000000000..0ddd5241f --- /dev/null +++ b/app/src/main/java/eu/faircode/netguard/PhysicalNetworkState.java @@ -0,0 +1,166 @@ +package eu.faircode.netguard; + +import android.net.LinkProperties; +import android.net.Network; +import android.net.NetworkCapabilities; +import android.os.Build; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; + +/** Callback-owned snapshots: never query ConnectivityManager from its callbacks. */ +final class PhysicalNetworkState { + private static final class Entry { + List transports; + Boolean metered; + List routes; + List dns; + String privateDns; + boolean privateDnsActive; + } + + private final Map entries = new HashMap<>(); + private Network defaultNetwork; + private boolean defaultIsVpn; + private List vpnTransports = Collections.emptyList(); + private Network egress; + + synchronized String onPhysicalAvailable(Network network) { + 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) { + Entry entry = entries.get(network); + if (entry == null || caps == null || + !caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN)) return 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) { + 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 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.routes = routes; + entry.dns = dns; + entry.privateDns = privateDns; + entry.privateDnsActive = active; + 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 (entries.remove(network) == null) return null; + return selectEgress(); + } + + synchronized String onDefaultNetworkAvailable(Network 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; + 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) { + return network != null && network.equals(defaultNetwork) && !defaultIsVpn + ? onPhysicalLinkPropertiesChanged(network, props) : null; + } + + synchronized String onDefaultNetworkLost(Network network) { + 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(); + } + } + boolean changed = !Objects.equals(egress, selected); + egress = selected; + return changed ? NetworkReloadPolicy.REASON_NETWORK_CHANGED : null; + } + + @android.annotation.SuppressLint("InlinedApi") + 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); + return result; + } + + synchronized Network getDefaultNetwork() { return egress; } + + synchronized void reset() { + entries.clear(); + defaultNetwork = 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 e0e09f221..5f13316f9 100644 --- a/app/src/main/java/eu/faircode/netguard/ServiceSinkhole.java +++ b/app/src/main/java/eu/faircode/netguard/ServiceSinkhole.java @@ -180,7 +180,11 @@ 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 final Handler networkSnapshotHandler = new Handler(Looper.getMainLooper()); + private final Runnable defaultNetworkSnapshotRunnable = this::refreshDefaultNetworkSnapshot; private boolean registeredInteractiveState = false; private PhoneStateListener callStateListener = null; @@ -265,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, @@ -731,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: @@ -836,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. @@ -847,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); @@ -910,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. @@ -2137,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); @@ -2218,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); @@ -3882,59 +3890,23 @@ private void listenNetworkChanges() { 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)); + // Initial capability/link callbacks are only guaranteed from API 26. + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) + refreshPhysicalNetworkSnapshot(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,49 +3914,107 @@ 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)); } @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; + 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. @@ -4002,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(); @@ -4250,9 +4281,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 +4346,20 @@ public void onDestroy() { private void unlistenNetworkChanges() { ConnectivityManager cm = (ConnectivityManager) getSystemService(Context.CONNECTIVITY_SERVICE); - cm.unregisterNetworkCallback((ConnectivityManager.NetworkCallback) networkCallback); + networkSnapshotHandler.removeCallbacksAndMessages(null); + 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) { @@ -4964,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 1aca50ed9..4af7f06d8 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`. @@ -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 @@ -189,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 @@ -260,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 @@ -274,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() @@ -292,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 && @@ -714,84 +707,6 @@ object WgEgress { fun latestHandshakeMillisOrNull(): Long? = try { tunnel?.latestHandshakeMillis() } catch (_: Throwable) { null } - fun onUnderlyingNetworkChanged() { - 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() - } - 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 - } - } - } - } - /** * 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 new file mode 100644 index 000000000..da8458716 --- /dev/null +++ b/app/src/test/java/eu/faircode/netguard/PhysicalNetworkStateTest.java @@ -0,0 +1,207 @@ +package eu.faircode.netguard; + +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.lang.reflect.Field; +import java.net.InetAddress; +import java.util.Collections; +import java.util.Map; + +@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); + + 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 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 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; + } + + private static PhysicalNetworkState wifiWithStandbyCell() throws Exception { + PhysicalNetworkState state = new PhysicalNetworkState(); + 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, 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)); + 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 unvalidatedPhysicalDefaultSwitchIsDetected() throws Exception { + PhysicalNetworkState state = wifiWithStandbyCell(); + assertNull(state.onDefaultNetworkCapabilitiesChanged(WIFI, capabilities(NetworkCapabilities.TRANSPORT_WIFI))); + assertEquals(NetworkReloadPolicy.REASON_NETWORK_CHANGED, + state.onDefaultNetworkCapabilitiesChanged(CELL, capabilities(NetworkCapabilities.TRANSPORT_CELLULAR))); + } + + @Test + 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"))); + } + + @Test + @Config(sdk = 23) + public void suppliedDefaultSnapshotSelectsEgressOnApi23() throws Exception { + PhysicalNetworkState state = wifiWithStandbyCell(); + assertEquals(NetworkReloadPolicy.REASON_NETWORK_CHANGED, + state.onDefaultNetworkCapabilitiesChanged(VPN, vpn(NetworkCapabilities.TRANSPORT_CELLULAR))); + assertEquals(CELL, state.getDefaultNetwork()); + } + + @Test + 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 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 + @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 + @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 defaultOnlyNetworksCannotAccumulatePhysicalEntries() throws Exception { + PhysicalNetworkState state = new PhysicalNetworkState(); + 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 setField(Object object, String name, Object value) throws Exception { + Field field = object.getClass().getDeclaredField(name); + field.setAccessible(true); + field.set(object, value); + } + + 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 bd2c2cec9..29c65c590 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,27 @@ package net.kollnig.missioncontrol.wg; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +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 kotlin.Unit; +import kotlin.jvm.functions.Function0; + +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; + /** * Screen-state keepalive toggling for the WireGuard egress. * @@ -9,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() { @@ -21,6 +44,125 @@ public void interactiveStateChangeIsNoopWhenConfigIsMissing() { WgEgress.INSTANCE.onInteractiveStateChanged(true, "", true, false); } + @org.junit.Test + 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 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"); + + Object oldTunnel = tunnel.get(WgEgress.INSTANCE); + 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)); + currentConfig.set(WgEgress.INSTANCE, invalidConfig); + currentTunFd.setInt(WgEgress.INSTANCE, vpnFd.getFd()); + currentTunPfd.set(WgEgress.INSTANCE, vpnFd); + forceRestartPending.setBoolean(WgEgress.INSTANCE, false); + + 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); + + 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); + 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); + } + } + + 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 { + // 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); + } + private static String validConfig() { String key = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA="; return "[Interface]\n" + 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