From 87cfc70b732422d6927ad9fe97f1ad7ab65fd508 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Mon, 27 Jul 2015 16:35:33 +0900 Subject: [PATCH 1/2] Always check off-link connectivity in NetworkDiagnostics. Currently, NetworkDiagnostics only checks off-link connectivity if one of the DNS servers is off-link. Make it check off-link connectivity in all cases by sending probes to Google Public DNS if off-link DNS servers are not specified. Bug: 22569331 Bug: 22641669 Bug: 22748900 Change-Id: I6cb0dd8491bc0c1a488631deca56722b9c1d2b3f --- core/java/android/net/LinkProperties.java | 13 +++++++++++- .../android/server/ConnectivityService.java | 2 +- .../connectivity/NetworkDiagnostics.java | 20 +++++++++++++++++++ 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/core/java/android/net/LinkProperties.java b/core/java/android/net/LinkProperties.java index cfd5bf1596715..c4de4a21e0930 100644 --- a/core/java/android/net/LinkProperties.java +++ b/core/java/android/net/LinkProperties.java @@ -661,6 +661,17 @@ public final class LinkProperties implements Parcelable { return false; } + /** + * Returns true if this link or any of its stacked interfaces has an IPv4 address. + * + * @return {@code true} if there is an IPv4 address, {@code false} otherwise. + */ + private boolean hasIPv4AddressOnInterface(String iface) { + return (mIfaceName.equals(iface) && hasIPv4Address()) || + (iface != null && mStackedLinks.containsKey(iface) && + mStackedLinks.get(iface).hasIPv4Address()); + } + /** * Returns true if this link has a global preferred IPv6 address. * @@ -792,7 +803,7 @@ public final class LinkProperties implements Parcelable { if (ip instanceof Inet4Address) { // For IPv4, it suffices for now to simply have any address. - return hasIPv4Address(); + return hasIPv4AddressOnInterface(bestRoute.getInterface()); } else if (ip instanceof Inet6Address) { if (ip.isLinkLocalAddress()) { // For now, just make sure link-local destinations have diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index eb74ab065d1a0..76d2258e0b858 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -1772,7 +1772,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // Start gathering diagnostic information. netDiags.add(new NetworkDiagnostics( nai.network, - new LinkProperties(nai.linkProperties), + new LinkProperties(nai.linkProperties), // Must be a copy. DIAG_TIME_MS)); } diff --git a/services/core/java/com/android/server/connectivity/NetworkDiagnostics.java b/services/core/java/com/android/server/connectivity/NetworkDiagnostics.java index 74ba404eb0ee6..aca699152e02a 100644 --- a/services/core/java/com/android/server/connectivity/NetworkDiagnostics.java +++ b/services/core/java/com/android/server/connectivity/NetworkDiagnostics.java @@ -20,6 +20,7 @@ import static android.system.OsConstants.*; import android.net.LinkProperties; import android.net.Network; +import android.net.NetworkUtils; import android.net.RouteInfo; import android.os.SystemClock; import android.system.ErrnoException; @@ -79,6 +80,10 @@ import libcore.io.IoUtils; public class NetworkDiagnostics { private static final String TAG = "NetworkDiagnostics"; + private static final InetAddress TEST_DNS4 = NetworkUtils.numericToInetAddress("8.8.8.8"); + private static final InetAddress TEST_DNS6 = NetworkUtils.numericToInetAddress( + "2001:4860:4860::8888"); + // For brevity elsewhere. private static final long now() { return SystemClock.elapsedRealtime(); @@ -156,6 +161,21 @@ public class NetworkDiagnostics { mStartTime = now(); mDeadlineTime = mStartTime + mTimeoutMs; + // Hardcode measurements to TEST_DNS4 and TEST_DNS6 in order to test off-link connectivity. + // We are free to modify mLinkProperties with impunity because ConnectivityService passes us + // a copy and not the original object. It's easier to do it this way because we don't need + // to check whether the LinkProperties already contains these DNS servers because + // LinkProperties#addDnsServer checks for duplicates. + if (mLinkProperties.isReachable(TEST_DNS4)) { + mLinkProperties.addDnsServer(TEST_DNS4); + } + // TODO: we could use mLinkProperties.isReachable(TEST_DNS6) here, because we won't set any + // DNS servers for which isReachable() is false, but since this is diagnostic code, be extra + // careful. + if (mLinkProperties.hasGlobalIPv6Address() || mLinkProperties.hasIPv6DefaultRoute()) { + mLinkProperties.addDnsServer(TEST_DNS6); + } + for (RouteInfo route : mLinkProperties.getRoutes()) { if (route.hasGateway()) { prepareIcmpMeasurement(route.getGateway()); From f28e62bdf607c9d257b90a7032f7193fc7de6d1e Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Mon, 27 Jul 2015 16:35:22 +0900 Subject: [PATCH 2/2] Add a test for public bugs 2111 and 2136. Bug: 22602137 Bug: 22104401 Change-Id: I5c994de53b5906416767a8a1abe38fe59afb7cc0 --- .../src/android/net/dhcp/DhcpPacketTest.java | 68 ++++++++++++++++++- 1 file changed, 67 insertions(+), 1 deletion(-) diff --git a/services/tests/servicestests/src/android/net/dhcp/DhcpPacketTest.java b/services/tests/servicestests/src/android/net/dhcp/DhcpPacketTest.java index ba1231f7530c5..da1df1accdb00 100644 --- a/services/tests/servicestests/src/android/net/dhcp/DhcpPacketTest.java +++ b/services/tests/servicestests/src/android/net/dhcp/DhcpPacketTest.java @@ -21,12 +21,13 @@ import android.net.DhcpResults; import android.net.LinkAddress; import android.system.OsConstants; import android.test.suitebuilder.annotation.SmallTest; -import junit.framework.TestCase; +import com.android.internal.util.HexDump; import java.net.Inet4Address; import java.nio.ByteBuffer; import java.util.ArrayList; +import junit.framework.TestCase; import libcore.util.HexEncoding; import static android.net.dhcp.DhcpPacket.*; @@ -370,4 +371,69 @@ public class DhcpPacketTest extends TestCase { assertDhcpResults("172.17.152.118/16", "172.17.1.1", "172.17.1.1", null, "1.1.1.1", null, 43200, false, dhcpResults); } + + @SmallTest + public void testBug2111() throws Exception { + final ByteBuffer packet = ByteBuffer.wrap(HexEncoding.decode(( + // IP header. + "4500014c00000000ff119beac3eaf3880a3f5d04" + + // UDP header. TODO: fix invalid checksum (due to MAC address obfuscation). + "0043004401387464" + + // BOOTP header. + "0201060002554812000a0000000000000a3f5d040000000000000000" + + // MAC address. + "00904c00000000000000000000000000" + + // Server name. + "0000000000000000000000000000000000000000000000000000000000000000" + + "0000000000000000000000000000000000000000000000000000000000000000" + + // File. + "0000000000000000000000000000000000000000000000000000000000000000" + + "0000000000000000000000000000000000000000000000000000000000000000" + + "0000000000000000000000000000000000000000000000000000000000000000" + + "0000000000000000000000000000000000000000000000000000000000000000" + + // Options. + "638253633501023604c00002fe33040000bfc60104fffff00003040a3f50010608c0000201c0000202" + + "0f0f646f6d61696e3132332e636f2e756b0000000000ff00000000" + ).toCharArray(), false)); + + DhcpPacket offerPacket = DhcpPacket.decodeFullPacket(packet, ENCAP_L3); + assertTrue(offerPacket instanceof DhcpOfferPacket); + DhcpResults dhcpResults = offerPacket.toDhcpResults(); + assertDhcpResults("10.63.93.4/20", "10.63.80.1", "192.0.2.1,192.0.2.2", + "domain123.co.uk", "192.0.2.254", null, 49094, false, dhcpResults); + } + + @SmallTest + public void testBug2136() throws Exception { + final ByteBuffer packet = ByteBuffer.wrap(HexEncoding.decode(( + // Ethernet header. + "bcf5ac000000d0c7890000000800" + + // IP header. + "4500014c00000000ff119beac3eaf3880a3f5d04" + + // UDP header. TODO: fix invalid checksum (due to MAC address obfuscation). + "0043004401387574" + + // BOOTP header. + "0201060163339a3000050000000000000a209ecd0000000000000000" + + // MAC address. + "bcf5ac00000000000000000000000000" + + // Server name. + "0000000000000000000000000000000000000000000000000000000000000000" + + "0000000000000000000000000000000000000000000000000000000000000000" + + // File. + "0000000000000000000000000000000000000000000000000000000000000000" + + "0000000000000000000000000000000000000000000000000000000000000000" + + "0000000000000000000000000000000000000000000000000000000000000000" + + "0000000000000000000000000000000000000000000000000000000000000000" + + // Options. + "6382536335010236040a20ff80330400001c200104fffff00003040a20900106089458413494584135" + + "0f0b6c616e63732e61632e756b000000000000000000ff00000000" + ).toCharArray(), false)); + + DhcpPacket offerPacket = DhcpPacket.decodeFullPacket(packet, ENCAP_L2); + assertTrue(offerPacket instanceof DhcpOfferPacket); + assertEquals("BCF5AC000000", HexDump.toHexString(offerPacket.getClientMac())); + DhcpResults dhcpResults = offerPacket.toDhcpResults(); + assertDhcpResults("10.32.158.205/20", "10.32.144.1", "148.88.65.52,148.88.65.53", + "lancs.ac.uk", "10.32.255.128", null, 7200, false, dhcpResults); + } }