From aad1184febca8937aae418f6f1629882eadcef60 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 7 Oct 2026 19:11:57 +0000 Subject: [PATCH] Keep tracker statistics truthful when DNS goes unseen The research preset turned off DoT blocking. With Android's Private DNS on "Automatic", Android then upgrades to DoT against the resolvers the VPN advertises, TrackerControl sees no plaintext DNS, and nothing is blocked in any mode while SNI still records trackers as allowed (#968). - Keep "Block Android's Private DNS" on in every onboarding preset. - Warn when Private DNS is active on the VPN network (LinkProperties.isPrivateDnsActive), without needing the full traffic log. Re-check 30 s after the VPN comes up and after block_dot changes, since Android validates DoT asynchronously. - In SNI attribution, store the tracker name that matched (the CNAME target for cloaked trackers) so the timeline and Protection screen find the row again, and keep the shared-IP mark that explains why standard mode allowed the contact. Claude-Session: https://claude.ai/code/session_01WgbtBAKqNxgmeXSPtUoXxp --- .../eu/faircode/netguard/ActivityMain.java | 7 +- .../eu/faircode/netguard/ServiceSinkhole.java | 100 ++++++++++++++---- .../main/java/eu/faircode/netguard/Util.java | 29 +++++ .../missioncontrol/ActivityOnboarding.java | 8 +- app/src/main/res/values/strings.xml | 1 + .../netguard/PrivateDnsWarningTest.java | 26 +++++ .../ServiceSinkholeSniAttributionTest.java | 63 +++++++++++ 7 files changed, 208 insertions(+), 26 deletions(-) create mode 100644 app/src/test/java/eu/faircode/netguard/ServiceSinkholeSniAttributionTest.java diff --git a/app/src/main/java/eu/faircode/netguard/ActivityMain.java b/app/src/main/java/eu/faircode/netguard/ActivityMain.java index 496e5429b..c63bd6ad4 100644 --- a/app/src/main/java/eu/faircode/netguard/ActivityMain.java +++ b/app/src/main/java/eu/faircode/netguard/ActivityMain.java @@ -525,8 +525,7 @@ public void onClick(View v) { @Override public void onClick(View v) { Intent settings; - if (privateDnsWarningState - == ServiceSinkhole.PRIVATE_DNS_WARNING_ALLOWED_DOT) + if (ServiceSinkhole.opensTrackerControlSettings(privateDnsWarningState)) settings = new Intent(ActivityMain.this, ActivitySettings.class); else { settings = new Intent(Settings.ACTION_WIRELESS_SETTINGS); @@ -704,7 +703,9 @@ private void updatePrivateDnsBypassBanner() { String resolver = TextUtils.isEmpty(specifier) ? getString(R.string.msg_private_dns_unknown_resolver) : specifier; tvPrivateDnsBypass.setText(getString(R.string.msg_private_dns_bypass_notify, resolver)); - } else if (state == ServiceSinkhole.PRIVATE_DNS_WARNING_ALLOWED_DOT) + } else if (state == ServiceSinkhole.PRIVATE_DNS_WARNING_ACTIVE) + tvPrivateDnsBypass.setText(R.string.msg_private_dns_bypass_active_notify); + else if (state == ServiceSinkhole.PRIVATE_DNS_WARNING_ALLOWED_DOT) tvPrivateDnsBypass.setText(R.string.msg_private_dns_bypass_allowed_notify); tvPrivateDnsBypass.setVisibility( state != ServiceSinkhole.PRIVATE_DNS_WARNING_NONE ? View.VISIBLE : View.GONE); diff --git a/app/src/main/java/eu/faircode/netguard/ServiceSinkhole.java b/app/src/main/java/eu/faircode/netguard/ServiceSinkhole.java index 5869aaaad..6f6312219 100644 --- a/app/src/main/java/eu/faircode/netguard/ServiceSinkhole.java +++ b/app/src/main/java/eu/faircode/netguard/ServiceSinkhole.java @@ -380,14 +380,21 @@ private static int getIntPref(SharedPreferences prefs, String key, int def) { static final int PRIVATE_DNS_WARNING_NONE = 0; static final int PRIVATE_DNS_WARNING_HOSTNAME = 1; static final int PRIVATE_DNS_WARNING_ALLOWED_DOT = 2; + static final int PRIVATE_DNS_WARNING_ACTIVE = 3; private static final long PRIVATE_DNS_WARNING_WINDOW_MS = 60 * 60 * 1000L; + // Android validates DoT against the VPN's resolvers some seconds after the + // VPN comes up, so the state read at establish time is usually still off. + private static final long PRIVATE_DNS_VALIDATION_DELAY_MS = 30 * 1000L; private int privateDnsWarningState = PRIVATE_DNS_WARNING_NONE; private final Handler privateDnsWarningExpiryHandler = new Handler(Looper.getMainLooper()); private final Runnable privateDnsWarningExpiryRunnable = this::requestPrivateDnsWarningUpdate; + private final Runnable privateDnsValidationRunnable = this::requestPrivateDnsWarningUpdate; private final SharedPreferences.OnSharedPreferenceChangeListener privateDnsPreferenceListener = (preferences, key) -> { if ("block_dot".equals(key) || "log".equals(key)) requestPrivateDnsWarningUpdate(); + if ("block_dot".equals(key)) + schedulePrivateDnsValidationCheck(); }; public static final String EXTRA_COMMAND = "Command"; @@ -1203,11 +1210,15 @@ private void log(Packet packet, int connection, boolean interactive) { if (prefs.getBoolean("sni_enabled", false) && packet.data != null && !packet.data.isEmpty()) { - uncertain = DatabaseHelper.ACCESS_UNCERTAIN_NONE; + uncertain = sniUncertainty(uncertain); if (!packet.data.equals(originalDname)) { Log.d(TAG, "Using SNI " + packet.data + " instead of originalDname " + originalDname); - dname = packet.data; - isTracker = getDecloakedTracker(dname, dh).first != null; + Pair sniTracker = getDecloakedTracker(packet.data, dh); + isTracker = sniTracker.first != null; + // Record the name that matched, which is the CNAME target + // for a cloaked tracker. The timeline and Protection screen + // look the stored name up again and drop rows that miss. + dname = isTracker ? sniTracker.second : packet.data; } } @@ -1247,20 +1258,7 @@ private Pair getDecloakedTracker(String qname, DatabaseHelper d } private Pair getDecloakedTracker(String qname, String aname) { - String foundDname = null; - Tracker t = TrackerList.findTracker(qname); - - if (t != null) { - foundDname = qname; - } else { // DNS uncloaking - if (aname != null) { - t = TrackerList.findTracker(aname); - if (t != null) - foundDname = aname; - } - } - - return new Pair<>(t, foundDname); + return decloakTracker(qname, aname); } private void usage(Usage usage) { @@ -1756,6 +1754,7 @@ private ParcelFileDescriptor startVPN(Builder builder) throws SecurityException updateUnderlyingNetworks(); vpnEstablished = true; markStoppedCleanly(false); + schedulePrivateDnsValidationCheck(); return pfd; } catch (SecurityException ex) { throw ex; @@ -1766,17 +1765,28 @@ private ParcelFileDescriptor startVPN(Builder builder) throws SecurityException } static int getPrivateDnsWarningState(Context context) { + SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(context); + if (prefs.getBoolean("block_dot", true)) + return PRIVATE_DNS_WARNING_NONE; + return getPrivateDnsWarningState(context, Util.isPrivateDnsActiveOnVpn(context)); + } + + static int getPrivateDnsWarningState(Context context, boolean privateDnsActiveOnVpn) { SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(context); boolean blockDot = prefs.getBoolean("block_dot", true); if (blockDot) return PRIVATE_DNS_WARNING_NONE; - // Case A takes precedence and avoids querying the log. Case B is only + // Case A takes precedence and avoids querying the log. Case B covers + // "automatic" mode, where Android upgrades to DoT on its own once the + // resolvers the VPN advertises answer on port 853. Case C is only // meaningful when the full traffic log is enabled: insertLog runs only // on that path. log_app defaults to true but records recognized tracker // contacts in access, not all flows, so it cannot support this warning. if (Util.isPrivateDnsDetectionDefeated(context)) return PRIVATE_DNS_WARNING_HOSTNAME; + if (privateDnsActiveOnVpn) + return PRIVATE_DNS_WARNING_ACTIVE; if (!prefs.getBoolean("log", false)) return PRIVATE_DNS_WARNING_NONE; @@ -1785,6 +1795,14 @@ static int getPrivateDnsWarningState(Context context) { ? PRIVATE_DNS_WARNING_ALLOWED_DOT : PRIVATE_DNS_WARNING_NONE; } + /** + * Whether the warning's fix is TrackerControl's own "Block Android's + * Private DNS" setting rather than Android's Private DNS setting. + */ + static boolean opensTrackerControlSettings(int state) { + return state == PRIVATE_DNS_WARNING_ALLOWED_DOT || state == PRIVATE_DNS_WARNING_ACTIVE; + } + static boolean shouldShowPrivateDnsWarning(int previousState, int currentState) { return currentState != PRIVATE_DNS_WARNING_NONE && previousState != currentState; } @@ -2754,6 +2772,35 @@ private void dnsResolved(ResourceRecord rr) { } } + static Pair decloakTracker(String qname, String aname) { + String foundDname = null; + Tracker t = TrackerList.findTracker(qname); + + if (t != null) { + foundDname = qname; + } else { // DNS uncloaking + if (aname != null) { + t = TrackerList.findTracker(aname); + if (t != null) + foundDname = aname; + } + } + + return new Pair<>(t, foundDname); + } + + /** + * Uncertainty of a contact attributed by its SNI. The SNI names the host, + * so it settles which of an IP's DNS questions the flow belongs to. The + * block decision is still made by IP, though, so an IP shared by a tracker + * and a non-tracker keeps its mark: it explains why standard mode let the + * contact through. + */ + static int sniUncertainty(int dnsUncertainty) { + return dnsUncertainty == DatabaseHelper.ACCESS_UNCERTAIN_MIXED_TRACKER_AND_NON_TRACKER + ? dnsUncertainty : DatabaseHelper.ACCESS_UNCERTAIN_NONE; + } + static final class DnsEvidence { final Set questions = new HashSet<>(); final Set trackerQuestions = new HashSet<>(); @@ -4209,6 +4256,7 @@ public void onDestroy() { // Cancel any debounced network-change reload so it doesn't fire post-teardown networkReloadDebounceHandler.removeCallbacksAndMessages(NETWORK_RELOAD_TOKEN); privateDnsWarningExpiryHandler.removeCallbacks(privateDnsWarningExpiryRunnable); + privateDnsWarningExpiryHandler.removeCallbacks(privateDnsValidationRunnable); PreferenceManager.getDefaultSharedPreferences(ServiceSinkhole.this) .unregisterOnSharedPreferenceChangeListener(privateDnsPreferenceListener); cancelNativeRecovery(true); @@ -4698,6 +4746,15 @@ private synchronized void schedulePrivateDnsWarningExpiry() { privateDnsWarningExpiryHandler.postDelayed(privateDnsWarningExpiryRunnable, delay); } + // One-shot, and only while DoT is allowed: with DoT blocked Android's + // validation cannot succeed, so there is nothing to wait for. + private void schedulePrivateDnsValidationCheck() { + privateDnsWarningExpiryHandler.removeCallbacks(privateDnsValidationRunnable); + if (!destroying && !PreferenceManager.getDefaultSharedPreferences(this).getBoolean("block_dot", true)) + privateDnsWarningExpiryHandler.postDelayed(privateDnsValidationRunnable, + PRIVATE_DNS_VALIDATION_DELAY_MS); + } + private synchronized void cancelPrivateDnsWarningExpiry() { privateDnsWarningExpiryHandler.removeCallbacks(privateDnsWarningExpiryRunnable); } @@ -4739,6 +4796,7 @@ private synchronized void updatePrivateDnsBypassWarning() { private synchronized void resetPrivateDnsBypassWarning() { cancelPrivateDnsWarningExpiry(); + privateDnsWarningExpiryHandler.removeCallbacks(privateDnsValidationRunnable); clearPrivateDnsBypassNotification(); privateDnsWarningState = PRIVATE_DNS_WARNING_NONE; publishPrivateDnsWarningState(privateDnsWarningState); @@ -4746,7 +4804,7 @@ private synchronized void resetPrivateDnsBypassWarning() { private void showPrivateDnsBypassNotification(int state) { Intent settings; - if (state == PRIVATE_DNS_WARNING_ALLOWED_DOT) + if (opensTrackerControlSettings(state)) settings = new Intent(this, ActivitySettings.class); else { settings = new Intent(Settings.ACTION_WIRELESS_SETTINGS); @@ -4762,7 +4820,9 @@ private void showPrivateDnsBypassNotification(int state) { String resolver = TextUtils.isEmpty(specifier) ? getString(R.string.msg_private_dns_unknown_resolver) : specifier; detail = getString(R.string.msg_private_dns_bypass_notify, resolver); - } else + } else if (state == PRIVATE_DNS_WARNING_ACTIVE) + detail = getString(R.string.msg_private_dns_bypass_active_notify); + else detail = getString(R.string.msg_private_dns_bypass_allowed_notify); NotificationCompat.Builder builder = new NotificationCompat.Builder(this, "notify"); diff --git a/app/src/main/java/eu/faircode/netguard/Util.java b/app/src/main/java/eu/faircode/netguard/Util.java index bfcde4826..ee67db1ab 100644 --- a/app/src/main/java/eu/faircode/netguard/Util.java +++ b/app/src/main/java/eu/faircode/netguard/Util.java @@ -37,6 +37,7 @@ import android.net.ConnectivityManager; import android.net.LinkProperties; import android.net.Network; +import android.net.NetworkCapabilities; import android.net.NetworkInfo; import android.net.wifi.WifiManager; import android.os.Build; @@ -600,6 +601,34 @@ public static boolean isPrivateDnsHostnameMode(Context context) { return "hostname".equals(dns_mode); } + /** + * Whether Android is resolving over DoT on a VPN network, which in + * "automatic" mode happens silently once a resolver the VPN advertises + * answers on port 853. TrackerControl then sees no plaintext DNS for the + * apps it routes, so it can neither detect nor block their trackers. + */ + @SuppressWarnings("deprecation") // getAllNetworks: the only way to reach the VPN network, which TC excludes itself from + public static boolean isPrivateDnsActiveOnVpn(Context context) { + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.P) + return false; + ConnectivityManager cm = (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE); + if (cm == null) + return false; + try { + for (Network network : cm.getAllNetworks()) { + NetworkCapabilities caps = cm.getNetworkCapabilities(network); + if (caps == null || !caps.hasTransport(NetworkCapabilities.TRANSPORT_VPN)) + continue; + LinkProperties lp = cm.getLinkProperties(network); + if (lp != null && lp.isPrivateDnsActive()) + return true; + } + } catch (SecurityException ex) { + Log.w(TAG, "Private DNS state unavailable: " + ex); + } + return false; + } + public static String getPrivateDnsSpecifier(Context context) { if (!isPrivateDnsHostnameMode(context)) return null; diff --git a/app/src/main/java/net/kollnig/missioncontrol/ActivityOnboarding.java b/app/src/main/java/net/kollnig/missioncontrol/ActivityOnboarding.java index 4c253ea5f..3224e4dd4 100644 --- a/app/src/main/java/net/kollnig/missioncontrol/ActivityOnboarding.java +++ b/app/src/main/java/net/kollnig/missioncontrol/ActivityOnboarding.java @@ -621,7 +621,6 @@ else if (BlockingMode.MODE_STRICT.equals(currentMode)) String mode = BlockingMode.MODE_STANDARD; boolean enableSni = false; boolean enableAdbLogging = false; - boolean enableDotBlocking = true; if (checkedId == R.id.rbMinimal) mode = BlockingMode.MODE_MINIMAL; @@ -631,7 +630,6 @@ else if (checkedId == R.id.rbResearch) { mode = BlockingMode.MODE_MINIMAL; enableSni = true; enableAdbLogging = true; - enableDotBlocking = false; } PreferenceManager.getDefaultSharedPreferences(holder.itemView.getContext()) @@ -640,7 +638,11 @@ else if (checkedId == R.id.rbResearch) { .putBoolean("filter", true) .putBoolean("sni_enabled", enableSni) .putBoolean("log_logcat", enableAdbLogging) - .putBoolean("block_dot", enableDotBlocking) + // Every preset blocks DoT. Without it Android's + // "Automatic" Private DNS encrypts DNS to the + // advertised resolvers, which leaves nothing to + // detect or block trackers by. + .putBoolean("block_dot", true) .apply(); BlockingMode.applyMode(holder.itemView.getContext()); diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 3ae21729b..2d8fe1ee4 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -746,6 +746,7 @@ Sincerely,\n\n]]> Private DNS bypasses tracker detection Tracker detection is off: DNS is encrypted to %1$s, so app connections can\'t be analysed + Tracker detection is off: Android\'s Private DNS is encrypting DNS, so app connections can\'t be analysed. Tap to turn on \"Block Android\'s Private DNS\". Some apps are using encrypted DNS; their tracker contacts are invisible to TrackerControl. Monitor system apps diff --git a/app/src/test/java/eu/faircode/netguard/PrivateDnsWarningTest.java b/app/src/test/java/eu/faircode/netguard/PrivateDnsWarningTest.java index 3f91e5d42..15efa6f80 100644 --- a/app/src/test/java/eu/faircode/netguard/PrivateDnsWarningTest.java +++ b/app/src/test/java/eu/faircode/netguard/PrivateDnsWarningTest.java @@ -57,6 +57,32 @@ public void hostnameAndBlockDotCombinations() { } } + @Test + public void activePrivateDnsOnVpnWarnsWithoutTrafficLog() { + setPrivateDns("opportunistic", null); + prefs.edit().putBoolean("block_dot", false).commit(); + + assertEquals(ServiceSinkhole.PRIVATE_DNS_WARNING_ACTIVE, + ServiceSinkhole.getPrivateDnsWarningState(context, true)); + assertEquals(ServiceSinkhole.PRIVATE_DNS_WARNING_NONE, + ServiceSinkhole.getPrivateDnsWarningState(context, false)); + assertTrue(ServiceSinkhole.opensTrackerControlSettings( + ServiceSinkhole.PRIVATE_DNS_WARNING_ACTIVE)); + + // Blocking DoT makes Android's validation fail, so nothing to warn about. + prefs.edit().putBoolean("block_dot", true).commit(); + assertEquals(ServiceSinkhole.PRIVATE_DNS_WARNING_NONE, + ServiceSinkhole.getPrivateDnsWarningState(context, true)); + + // A pinned resolver keeps its more specific warning. + setPrivateDns("hostname", "dns.google"); + prefs.edit().putBoolean("block_dot", false).commit(); + assertEquals(ServiceSinkhole.PRIVATE_DNS_WARNING_HOSTNAME, + ServiceSinkhole.getPrivateDnsWarningState(context, true)); + assertFalse(ServiceSinkhole.opensTrackerControlSettings( + ServiceSinkhole.PRIVATE_DNS_WARNING_HOSTNAME)); + } + @Test public void hostnameWarningTakesPrecedenceOverAllowedDotFlow() { long now = System.currentTimeMillis(); diff --git a/app/src/test/java/eu/faircode/netguard/ServiceSinkholeSniAttributionTest.java b/app/src/test/java/eu/faircode/netguard/ServiceSinkholeSniAttributionTest.java new file mode 100644 index 000000000..7e08f0a81 --- /dev/null +++ b/app/src/test/java/eu/faircode/netguard/ServiceSinkholeSniAttributionTest.java @@ -0,0 +1,63 @@ +package eu.faircode.netguard; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import android.content.Context; +import android.util.Pair; + +import androidx.preference.PreferenceManager; + +import net.kollnig.missioncontrol.data.BlockingMode; +import net.kollnig.missioncontrol.data.Tracker; +import net.kollnig.missioncontrol.data.TrackerList; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.RuntimeEnvironment; + +/** How research mode attributes a contact by its SNI. */ +@RunWith(RobolectricTestRunner.class) +public class ServiceSinkholeSniAttributionTest { + + @Test + public void cloakedTrackerIsStoredUnderTheMatchedName() { + Context context = RuntimeEnvironment.getApplication(); + PreferenceManager.getDefaultSharedPreferences(context).edit() + .putString(BlockingMode.PREF_BLOCKING_MODE, BlockingMode.MODE_STANDARD) + .commit(); + assertTrue(TrackerList.getInstance(context).loadTrackers(context)); + assertNull(TrackerList.findTracker("metrics.sni-example.test")); + + Pair cloaked = + ServiceSinkhole.decloakTracker("metrics.sni-example.test", "doubleclick.net"); + assertNotNull(cloaked.first); + // The stored name must be found again by the timeline's findTracker. + assertEquals("doubleclick.net", cloaked.second); + assertNotNull(TrackerList.findTracker(cloaked.second)); + + Pair direct = ServiceSinkhole.decloakTracker("doubleclick.net", null); + assertEquals("doubleclick.net", direct.second); + + Pair none = + ServiceSinkhole.decloakTracker("metrics.sni-example.test", null); + assertNull(none.first); + assertNull(none.second); + } + + @Test + public void sniKeepsOnlyTheSharedIpVerdictMark() { + assertEquals(DatabaseHelper.ACCESS_UNCERTAIN_MIXED_TRACKER_AND_NON_TRACKER, + ServiceSinkhole.sniUncertainty( + DatabaseHelper.ACCESS_UNCERTAIN_MIXED_TRACKER_AND_NON_TRACKER)); + assertEquals(DatabaseHelper.ACCESS_UNCERTAIN_NONE, + ServiceSinkhole.sniUncertainty(DatabaseHelper.ACCESS_UNCERTAIN_SHARED_IP)); + assertEquals(DatabaseHelper.ACCESS_UNCERTAIN_NONE, + ServiceSinkhole.sniUncertainty(DatabaseHelper.ACCESS_UNCERTAIN_MULTIPLE_TRACKERS)); + assertEquals(DatabaseHelper.ACCESS_UNCERTAIN_NONE, + ServiceSinkhole.sniUncertainty(DatabaseHelper.ACCESS_UNCERTAIN_NONE)); + } +}