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)); + } +}