From bbda98be5e94950b8fa760740f7ecff050338fb8 Mon Sep 17 00:00:00 2001 From: Konrad Kollnig <5175206+kasnder@users.noreply.github.com> Date: Wed, 5 Aug 2026 10:28:29 +0200 Subject: [PATCH] Warn about local destinations named by host, not just by address MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit isConfigured() classifies the stored configuration, so it only sees addresses: a DoH endpoint at https://pi.hole/dns-query, or a WireGuard peer on a dynamic DNS name, resolves to the LAN but reads as remote. Those users get no banner and no prompt — just a resolver that stopped answering. Classifying them up front would mean resolving a name on whichever thread asked, including the main one, which is exactly what the address-only rule exists to avoid. Take the address from the code that already resolved it instead. OkHttp resolves the DoH endpoint anyway, so hang a Dns off the client and read what it got; WgEgress.resolveEndpoint() likewise has the peer's address in hand. Both hand it to LocalNetworkAccess.reportDestination(), which latches if it is local, and isMissing() ors that in. The destination is what matters, not whether the connection failed: if it is local and the permission is missing, that traffic is blocked. The latch is in-memory only — a stale observation dies with the process, and anything still pointing at the LAN re-reports on next use. Changing a relevant setting clears it, since the new value may point elsewhere. Co-Authored-By: Claude Opus 5 --- .../faircode/netguard/ActivitySettings.java | 5 ++ .../missioncontrol/LocalNetworkAccess.java | 51 ++++++++++++++++++- .../dns/DnsOverHttpsClient.java | 14 +++++ .../net/kollnig/missioncontrol/wg/WgEgress.kt | 5 ++ .../LocalNetworkAccessTest.java | 40 +++++++++++++++ 5 files changed, 114 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/eu/faircode/netguard/ActivitySettings.java b/app/src/main/java/eu/faircode/netguard/ActivitySettings.java index 09e3d65a2..e2b3fbc2f 100644 --- a/app/src/main/java/eu/faircode/netguard/ActivitySettings.java +++ b/app/src/main/java/eu/faircode/netguard/ActivitySettings.java @@ -978,6 +978,11 @@ else if ("domain_based_blocking".equals(name)) { // Asked at most once per visit: writing back a trimmed value re-enters // this listener, and a second request while the dialog is up is dropped // by the framework. + if (LocalNetworkAccess.isRelevantSetting(name)) + // The new value may point somewhere else entirely; what we saw the + // old one reach says nothing about it. + LocalNetworkAccess.forgetObservations(); + if (!requestedLocalNetwork && LocalNetworkAccess.isRelevantSetting(name) && LocalNetworkAccess.isMissing(this)) { requestedLocalNetwork = true; diff --git a/app/src/main/java/net/kollnig/missioncontrol/LocalNetworkAccess.java b/app/src/main/java/net/kollnig/missioncontrol/LocalNetworkAccess.java index b7602e99b..c23104581 100644 --- a/app/src/main/java/net/kollnig/missioncontrol/LocalNetworkAccess.java +++ b/app/src/main/java/net/kollnig/missioncontrol/LocalNetworkAccess.java @@ -121,7 +121,56 @@ public static boolean isGranted(Context context) { public static boolean isMissing(Context context) { // Cheapest checks first: nothing to do below Android 17, and parsing the // WireGuard config is pointless once the permission is granted. - return isEnforced() && !isGranted(context) && isConfigured(context); + return isEnforced() && !isGranted(context) + && (observedLocalDestination || isConfigured(context)); + } + + /** + * A destination we resolved to a local network address, seen while running. + * Covers what {@link #isConfigured(SharedPreferences)} structurally cannot: + * a configuration that names a host rather than an address — + * {@code https://pi.hole/dns-query}, a WireGuard endpoint on a dynamic DNS + * name — where classifying it up front would mean resolving a name on + * whichever thread asked, including the main one. + * + *

Not persisted. A stale observation survives no longer than the + * process, and anything still pointing at the LAN re-reports itself as soon + * as it is used again. + */ + private static volatile boolean observedLocalDestination; + + /** + * Report an address TrackerControl is about to talk to. Callers pass what + * they already resolved, so this never performs a lookup itself and is safe + * to call from any thread. + * + *

Only the destination matters, not whether the connection succeeded: if + * it is local and the permission is missing, that traffic is blocked. + */ + public static void reportDestination(String address) { + // Deliberately not gated on isEnforced(): isMissing() applies that gate + // anyway, and keeping this branch out makes the latch testable under + // Robolectric, which runs below the enforcement level. + if (observedLocalDestination) + return; + if (isLocalAddress(address)) { + Log.i(TAG, "Local network destination observed at runtime"); + observedLocalDestination = true; + } + } + + /** Whether a local destination has been seen since the last reset. */ + static boolean hasObservedLocalDestination() { + return observedLocalDestination; + } + + /** + * Drop what we observed, so a configuration that no longer points at the + * LAN stops warning. Called when a relevant setting changes; anything still + * local reports itself again on next use. + */ + public static void forgetObservations() { + observedLocalDestination = false; } /** Whether the current configuration makes TrackerControl talk to the LAN. */ diff --git a/app/src/main/java/net/kollnig/missioncontrol/dns/DnsOverHttpsClient.java b/app/src/main/java/net/kollnig/missioncontrol/dns/DnsOverHttpsClient.java index ac5759896..47879ba23 100644 --- a/app/src/main/java/net/kollnig/missioncontrol/dns/DnsOverHttpsClient.java +++ b/app/src/main/java/net/kollnig/missioncontrol/dns/DnsOverHttpsClient.java @@ -19,6 +19,7 @@ import androidx.preference.PreferenceManager; import net.kollnig.missioncontrol.BuildConfig; +import net.kollnig.missioncontrol.LocalNetworkAccess; import org.xbill.DNS.Message; import org.xbill.DNS.Record; @@ -27,6 +28,7 @@ import java.io.File; import java.io.IOException; +import java.net.InetAddress; import java.util.Arrays; import java.util.List; import java.util.Objects; @@ -36,6 +38,7 @@ import okhttp3.Cache; import okhttp3.ConnectionPool; +import okhttp3.Dns; import okhttp3.HttpUrl; import okhttp3.MediaType; import okhttp3.OkHttpClient; @@ -80,6 +83,17 @@ private DnsOverHttpsClient(Context context, String endpoint) { .connectionPool(new ConnectionPool(2, 30, TimeUnit.SECONDS)) .cache(getResponseCache(context)) .retryOnConnectionFailure(true) + // An endpoint given as a host name can still be a resolver on + // the user's own network, which Android 17 blocks us from + // reaching without ACCESS_LOCAL_NETWORK. OkHttp resolves it + // anyway, so note the address it got rather than looking it up + // a second time (#701). + .dns(hostname -> { + List addresses = Dns.SYSTEM.lookup(hostname); + for (InetAddress address : addresses) + LocalNetworkAccess.reportDestination(address.getHostAddress()); + return addresses; + }) .build(); Log.i(TAG, "DoH client initialized with endpoint: " + endpoint); 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 d6ce48dc0..5b92ff665 100644 --- a/app/src/main/java/net/kollnig/missioncontrol/wg/WgEgress.kt +++ b/app/src/main/java/net/kollnig/missioncontrol/wg/WgEgress.kt @@ -5,6 +5,7 @@ import android.os.Handler import android.os.Looper import android.os.ParcelFileDescriptor import android.util.Log +import net.kollnig.missioncontrol.LocalNetworkAccess import net.kollnig.missioncontrol.wgbridge.Logger as WgLogger import net.kollnig.missioncontrol.wgbridge.Protector as WgProtector import net.kollnig.missioncontrol.wgbridge.Tunnel as WgTunnel @@ -815,6 +816,10 @@ object WgEgress { val resolved = try { val addr = resolveHostBounded(host) val ip = addr.hostAddress ?: throw IllegalStateException("getHostAddress null for $host") + // A peer named by host name can still sit on the user's own network, + // which Android 17 blocks without ACCESS_LOCAL_NETWORK (#701). The + // address is in hand here, so no extra lookup is needed to notice. + LocalNetworkAccess.reportDestination(ip) if (addr is java.net.Inet6Address) "[$ip]:$port" else "$ip:$port" } catch (e: Exception) { // DNS often fails exactly when we resolve: the resolver runs over diff --git a/app/src/test/java/net/kollnig/missioncontrol/LocalNetworkAccessTest.java b/app/src/test/java/net/kollnig/missioncontrol/LocalNetworkAccessTest.java index 66bb4e6fb..edffeb69a 100644 --- a/app/src/test/java/net/kollnig/missioncontrol/LocalNetworkAccessTest.java +++ b/app/src/test/java/net/kollnig/missioncontrol/LocalNetworkAccessTest.java @@ -55,6 +55,46 @@ public class LocalNetworkAccessTest { public void setUp() { prefs = PreferenceManager.getDefaultSharedPreferences(RuntimeEnvironment.getApplication()); prefs.edit().clear().commit(); + LocalNetworkAccess.forgetObservations(); + } + + @Test + public void runtimeDestinationOnTheLanIsRemembered() { + assertFalse(LocalNetworkAccess.hasObservedLocalDestination()); + + // What a host name in a DoH URL or WireGuard endpoint resolved to. + LocalNetworkAccess.reportDestination("192.168.1.10"); + assertTrue(LocalNetworkAccess.hasObservedLocalDestination()); + } + + @Test + public void runtimeDestinationOffTheLanIsIgnored() { + LocalNetworkAccess.reportDestination("9.9.9.9"); + assertFalse(LocalNetworkAccess.hasObservedLocalDestination()); + + // Never resolved here — callers pass an address they already have. + LocalNetworkAccess.reportDestination("pi.hole"); + assertFalse(LocalNetworkAccess.hasObservedLocalDestination()); + + LocalNetworkAccess.reportDestination(null); + assertFalse(LocalNetworkAccess.hasObservedLocalDestination()); + } + + @Test + public void observationsAreForgottenOnReset() { + LocalNetworkAccess.reportDestination("fd00::1"); + assertTrue(LocalNetworkAccess.hasObservedLocalDestination()); + + LocalNetworkAccess.forgetObservations(); + assertFalse(LocalNetworkAccess.hasObservedLocalDestination()); + } + + @Test + public void observationDoesNotAffectConfigurationDetection() { + // isConfigured() answers "does the stored configuration name the LAN", + // which a runtime observation must not fake up. + LocalNetworkAccess.reportDestination("192.168.1.10"); + assertFalse(LocalNetworkAccess.isConfigured(prefs)); } @Test