Fix use of reachable DNS server logic

Move reachable DNS server computation out of ConnectivityService
and split it into LinkProperties#isReachable() and a companion
change in WifiStateMachine's makeLinkProperties().

Restore previous ConnectivityService#updateDnses() behaviour, as
the pruning is done in WifiStateMachine now.

Bug: 19470192
Bug: 20733156
Bug: 22098233
Change-Id: I810ef74d504e5dc1ca2017d435cdadd6b82171e6

diff --git a/core/java/android/net/LinkProperties.java b/core/java/android/net/LinkProperties.java
index 31aedad..cfd5bf1 100644 (file)
--- a/core/java/android/net/LinkProperties.java
+++ b/core/java/android/net/LinkProperties.java
@@ -517,7 +517,7 @@ public final class LinkProperties implements Parcelable {
      * Note that Http Proxies are only a hint - the system recommends their use, but it does
      * not enforce it and applications may ignore them.
      *
-     * @param proxy A {@link ProxyInfo} defining the Http Proxy to use on this link.
+     * @param proxy A {@link ProxyInfo} defining the HTTP Proxy to use on this link.
      * @hide
      */
     public void setHttpProxy(ProxyInfo proxy) {
@@ -774,6 +774,43 @@ public final class LinkProperties implements Parcelable {
     }
 
     /**
+     * Evaluate whether the {@link InetAddress} is considered reachable.
+     *
+     * @return {@code true} if the given {@link InetAddress} is considered reachable,
+     *         {@code false} otherwise.
+     * @hide
+     */
+    public boolean isReachable(InetAddress ip) {
+        final List<RouteInfo> allRoutes = getAllRoutes();
+        // If we don't have a route to this IP address, it's not reachable.
+        final RouteInfo bestRoute = RouteInfo.selectBestRoute(allRoutes, ip);
+        if (bestRoute == null) {
+            return false;
+        }
+
+        // TODO: better source address evaluation for destination addresses.
+
+        if (ip instanceof Inet4Address) {
+            // For IPv4, it suffices for now to simply have any address.
+            return hasIPv4Address();
+        } else if (ip instanceof Inet6Address) {
+            if (ip.isLinkLocalAddress()) {
+                // For now, just make sure link-local destinations have
+                // scopedIds set, since transmits will generally fail otherwise.
+                // TODO: verify it matches the ifindex of one of the interfaces.
+                return (((Inet6Address)ip).getScopeId() != 0);
+            }  else {
+                // For non-link-local destinations check that either the best route
+                // is directly connected or that some global preferred address exists.
+                // TODO: reconsider all cases (disconnected ULA networks, ...).
+                return (!bestRoute.hasGateway() || hasGlobalIPv6Address());
+            }
+        }
+
+        return false;
+    }
+
+    /**
      * Compares this {@code LinkProperties} interface name against the target
      *
      * @param target LinkProperties to compare.

diff --git a/core/tests/coretests/src/android/net/LinkPropertiesTest.java b/core/tests/coretests/src/android/net/LinkPropertiesTest.java
index 5c55efb..ea444a4 100644 (file)
--- a/core/tests/coretests/src/android/net/LinkPropertiesTest.java
+++ b/core/tests/coretests/src/android/net/LinkPropertiesTest.java
@@ -16,6 +16,8 @@
 
 package android.net;
 
+import android.net.IpPrefix;
+import android.net.LinkAddress;
 import android.net.LinkProperties;
 import android.net.LinkProperties.ProvisioningChange;
 import android.net.RouteInfo;
@@ -26,6 +28,7 @@ import junit.framework.TestCase;
 import java.net.InetAddress;
 import java.util.ArrayList;
 
+
 public class LinkPropertiesTest extends TestCase {
     private static InetAddress ADDRV4 = NetworkUtils.numericToInetAddress("75.208.6.1");
     private static InetAddress ADDRV6 = NetworkUtils.numericToInetAddress(
@@ -567,4 +570,80 @@ public class LinkPropertiesTest extends TestCase {
         assertEquals(ProvisioningChange.STILL_PROVISIONED,
                 LinkProperties.compareProvisioning(v6lp, v6lp2));
     }
+
+    @SmallTest
+    public void testIsReachable() {
+        final LinkProperties v4lp = new LinkProperties();
+        assertFalse(v4lp.isReachable(DNS1));
+        assertFalse(v4lp.isReachable(DNS2));
+
+        // Add an on-link route, making the on-link DNS server reachable,
+        // but there is still no IPv4 address.
+        assertTrue(v4lp.addRoute(new RouteInfo(
+                new IpPrefix(NetworkUtils.numericToInetAddress("75.208.0.0"), 16))));
+        assertFalse(v4lp.isReachable(DNS1));
+        assertFalse(v4lp.isReachable(DNS2));
+
+        // Adding an IPv4 address (right now, any IPv4 address) means we use
+        // the routes to compute likely reachability.
+        assertTrue(v4lp.addLinkAddress(new LinkAddress(ADDRV4, 16)));
+        assertTrue(v4lp.isReachable(DNS1));
+        assertFalse(v4lp.isReachable(DNS2));
+
+        // Adding a default route makes the off-link DNS server reachable.
+        assertTrue(v4lp.addRoute(new RouteInfo(GATEWAY1)));
+        assertTrue(v4lp.isReachable(DNS1));
+        assertTrue(v4lp.isReachable(DNS2));
+
+        final LinkProperties v6lp = new LinkProperties();
+        final InetAddress kLinkLocalDns = NetworkUtils.numericToInetAddress("fe80::6:1");
+        final InetAddress kLinkLocalDnsWithScope = NetworkUtils.numericToInetAddress("fe80::6:2%43");
+        final InetAddress kOnLinkDns = NetworkUtils.numericToInetAddress("2001:db8:85a3::53");
+        assertFalse(v6lp.isReachable(kLinkLocalDns));
+        assertFalse(v6lp.isReachable(kLinkLocalDnsWithScope));
+        assertFalse(v6lp.isReachable(kOnLinkDns));
+        assertFalse(v6lp.isReachable(DNS6));
+
+        // Add a link-local route, making the link-local DNS servers reachable. Because
+        // we assume the presence of an IPv6 link-local address, link-local DNS servers
+        // are considered reachable, but only those with a non-zero scope identifier.
+        assertTrue(v6lp.addRoute(new RouteInfo(
+                new IpPrefix(NetworkUtils.numericToInetAddress("fe80::"), 64))));
+        assertFalse(v6lp.isReachable(kLinkLocalDns));
+        assertTrue(v6lp.isReachable(kLinkLocalDnsWithScope));
+        assertFalse(v6lp.isReachable(kOnLinkDns));
+        assertFalse(v6lp.isReachable(DNS6));
+
+        // Add a link-local address--nothing changes.
+        assertTrue(v6lp.addLinkAddress(LINKADDRV6LINKLOCAL));
+        assertFalse(v6lp.isReachable(kLinkLocalDns));
+        assertTrue(v6lp.isReachable(kLinkLocalDnsWithScope));
+        assertFalse(v6lp.isReachable(kOnLinkDns));
+        assertFalse(v6lp.isReachable(DNS6));
+
+        // Add a global route on link, but no global address yet. DNS servers reachable
+        // via a route that doesn't require a gateway: give them the benefit of the
+        // doubt and hope the link-local source address suffices for communication.
+        assertTrue(v6lp.addRoute(new RouteInfo(
+                new IpPrefix(NetworkUtils.numericToInetAddress("2001:db8:85a3::"), 64))));
+        assertFalse(v6lp.isReachable(kLinkLocalDns));
+        assertTrue(v6lp.isReachable(kLinkLocalDnsWithScope));
+        assertTrue(v6lp.isReachable(kOnLinkDns));
+        assertFalse(v6lp.isReachable(DNS6));
+
+        // Add a global address; the on-link global address DNS server is (still)
+        // presumed reachable.
+        assertTrue(v6lp.addLinkAddress(new LinkAddress(ADDRV6, 64)));
+        assertFalse(v6lp.isReachable(kLinkLocalDns));
+        assertTrue(v6lp.isReachable(kLinkLocalDnsWithScope));
+        assertTrue(v6lp.isReachable(kOnLinkDns));
+        assertFalse(v6lp.isReachable(DNS6));
+
+        // Adding a default route makes the off-link DNS server reachable.
+        assertTrue(v6lp.addRoute(new RouteInfo(GATEWAY62)));
+        assertFalse(v6lp.isReachable(kLinkLocalDns));
+        assertTrue(v6lp.isReachable(kLinkLocalDnsWithScope));
+        assertTrue(v6lp.isReachable(kOnLinkDns));
+        assertTrue(v6lp.isReachable(DNS6));
+    }
 }

diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index 47971a1..8fba98c 100644 (file)
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -140,14 +140,12 @@ import java.io.FileReader;
 import java.io.IOException;
 import java.io.PrintWriter;
 import java.net.Inet4Address;
-import java.net.Inet6Address;
 import java.net.InetAddress;
 import java.net.UnknownHostException;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -4014,51 +4012,10 @@ public class ConnectivityService extends IConnectivityManager.Stub
         return !routeDiff.added.isEmpty() || !routeDiff.removed.isEmpty();
     }
 
-    // TODO: investigate moving this into LinkProperties, if only to make more accurate
-    // the isProvisioned() checks.
-    private static Collection<InetAddress> getLikelyReachableDnsServers(LinkProperties lp) {
-        final ArrayList<InetAddress> dnsServers = new ArrayList<InetAddress>();
-        final List<RouteInfo> allRoutes = lp.getAllRoutes();
-        for (InetAddress nameserver : lp.getDnsServers()) {
-            // If the LinkProperties doesn't include a route to the nameserver, ignore it.
-            final RouteInfo bestRoute = RouteInfo.selectBestRoute(allRoutes, nameserver);
-            if (bestRoute == null) {
-                continue;
-            }
-
-            // TODO: better source address evaluation for destination addresses.
-            if (nameserver instanceof Inet4Address) {
-                if (!lp.hasIPv4Address()) {
-                    continue;
-                }
-            } else if (nameserver instanceof Inet6Address) {
-                if (nameserver.isLinkLocalAddress()) {
-                    if (((Inet6Address)nameserver).getScopeId() == 0) {
-                        // For now, just make sure link-local DNS servers have
-                        // scopedIds set, since DNS lookups will fail otherwise.
-                        // TODO: verify the scopeId matches that of lp's interface.
-                        continue;
-                    }
-                }  else {
-                    if (bestRoute.isIPv6Default() && !lp.hasGlobalIPv6Address()) {
-                        // TODO: reconsider all corner cases (disconnected ULA networks, ...).
-                        continue;
-                    }
-                }
-            }
-
-            dnsServers.add(nameserver);
-        }
-        return Collections.unmodifiableList(dnsServers);
-    }
-
     private void updateDnses(LinkProperties newLp, LinkProperties oldLp, int netId,
                              boolean flush, boolean useDefaultDns) {
-        // TODO: consider comparing the getLikelyReachableDnsServers() lists, in case the
-        // route to a DNS server has been removed (only really applicable in special cases
-        // where there is no default route).
         if (oldLp == null || (newLp.isIdenticalDnses(oldLp) == false)) {
-            Collection<InetAddress> dnses = getLikelyReachableDnsServers(newLp);
+            Collection<InetAddress> dnses = newLp.getDnsServers();
             if (dnses.size() == 0 && mDefaultDns != null && useDefaultDns) {
                 dnses = new ArrayList();
                 dnses.add(mDefaultDns);