From b36a3131d1b05efa176f79d318197499b2c45380 Mon Sep 17 00:00:00 2001 From: Erik Kline Date: Fri, 26 Jun 2015 19:21:34 +0900 Subject: [PATCH] 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 --- core/java/android/net/LinkProperties.java | 39 ++++++++++- .../src/android/net/LinkPropertiesTest.java | 79 ++++++++++++++++++++++ .../com/android/server/ConnectivityService.java | 45 +----------- 3 files changed, 118 insertions(+), 45 deletions(-) diff --git a/core/java/android/net/LinkProperties.java b/core/java/android/net/LinkProperties.java index 31aedad77116..cfd5bf159671 100644 --- 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 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 5c55efb837fa..ea444a45e182 100644 --- 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 47971a1dc081..8fba98c87720 100644 --- 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 getLikelyReachableDnsServers(LinkProperties lp) { - final ArrayList dnsServers = new ArrayList(); - final List 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 dnses = getLikelyReachableDnsServers(newLp); + Collection dnses = newLp.getDnsServers(); if (dnses.size() == 0 && mDefaultDns != null && useDefaultDns) { dnses = new ArrayList(); dnses.add(mDefaultDns); -- 2.11.0