From: Rubin Xu Date: Tue, 5 Sep 2017 17:40:49 +0000 (+0100) Subject: Always add local subnet routes to the interface's routing table X-Git-Tag: android-x86-8.1-r1~87^2~19^2 X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=3721305094b0175408bb42087b9f9ff59e2f0146;p=android-x86%2Fframeworks-base.git Always add local subnet routes to the interface's routing table For some networks such as mobile data connections, its LinkProperties does not contain routes for the local subnet so no such route is added to the interface's routing table. This can be problematic especially if the device is in VPN lockdown mode where there exists high-priority PROHIBIT routing rule which in turn blocks the network's default gateway route from being added (next hop address hitting the prohibit rule). We fix this by patching LinkProperties to always include direct connected routes when they are received by ConnectivityService. This has the added advantage that when apps get LinkProperties, they see the directly connected routes as well. Bug: 63662962 Test: runtest frameworks-core -c android.net.LinkPropertiesTest Test: runtest frameworks-services -c com.android.server.ConnectivityServiceTest Test: Start with device with mobile data, set up ics-OpenVPN in always-on lockdown mode. Turn off mobile data then turn it back on, observe mobile data connectivity is restored and VPN successfully reconnects. (cherry picked from commit 1bb5c0818f0c4fb426e13b65a3ba3db7f36c3d88) Change-Id: Ia14f88bcf49d37286519c26dff6b7180303e2cbe --- diff --git a/core/java/android/net/LinkProperties.java b/core/java/android/net/LinkProperties.java index 1bb0fbb74a53..f527f77ddd7c 100644 --- a/core/java/android/net/LinkProperties.java +++ b/core/java/android/net/LinkProperties.java @@ -18,14 +18,13 @@ package android.net; import android.annotation.NonNull; import android.annotation.Nullable; -import android.net.ProxyInfo; -import android.os.Parcelable; import android.os.Parcel; +import android.os.Parcelable; import android.text.TextUtils; -import java.net.InetAddress; import java.net.Inet4Address; import java.net.Inet6Address; +import java.net.InetAddress; import java.net.UnknownHostException; import java.util.ArrayList; import java.util.Collection; @@ -504,11 +503,22 @@ public final class LinkProperties implements Parcelable { } /** + * Make sure this LinkProperties instance contains routes that cover the local subnet + * of its link addresses. Add any route that is missing. + * @hide + */ + public void ensureDirectlyConnectedRoutes() { + for (LinkAddress addr: mLinkAddresses) { + addRoute(new RouteInfo(addr, null, mIfaceName)); + } + } + + /** * Returns all the routes on this link and all the links stacked above it. * @hide */ public List getAllRoutes() { - List routes = new ArrayList(); + List routes = new ArrayList<>(); routes.addAll(mRoutes); for (LinkProperties stacked: mStackedLinks.values()) { routes.addAll(stacked.getAllRoutes()); diff --git a/core/tests/coretests/src/android/net/LinkPropertiesTest.java b/core/tests/coretests/src/android/net/LinkPropertiesTest.java index d5f632190d06..9686dd9e57a0 100644 --- a/core/tests/coretests/src/android/net/LinkPropertiesTest.java +++ b/core/tests/coretests/src/android/net/LinkPropertiesTest.java @@ -24,10 +24,15 @@ import android.net.RouteInfo; import android.system.OsConstants; import android.test.suitebuilder.annotation.SmallTest; import android.test.suitebuilder.annotation.Suppress; +import android.util.ArraySet; + import junit.framework.TestCase; import java.net.InetAddress; -import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.Set; public class LinkPropertiesTest extends TestCase { @@ -678,4 +683,76 @@ public class LinkPropertiesTest extends TestCase { stacked.addRoute(new RouteInfo((IpPrefix) null, stackedAddress)); assertTrue(v6lp.isReachable(DNS1)); } + + @SmallTest + public void testLinkPropertiesEnsureDirectlyConnectedRoutes() { + // IPv4 case: no route added initially + LinkProperties rmnet0 = new LinkProperties(); + rmnet0.setInterfaceName("rmnet0"); + rmnet0.addLinkAddress(new LinkAddress("10.0.0.2/8")); + RouteInfo directRoute0 = new RouteInfo(new IpPrefix("10.0.0.0/8"), null, + rmnet0.getInterfaceName()); + + // Since no routes is added explicitly, getAllRoutes() should return empty. + assertTrue(rmnet0.getAllRoutes().isEmpty()); + rmnet0.ensureDirectlyConnectedRoutes(); + // ensureDirectlyConnectedRoutes() should have added the missing local route. + assertEqualRoutes(Collections.singletonList(directRoute0), rmnet0.getAllRoutes()); + + // IPv4 case: both direct and default routes added initially + LinkProperties rmnet1 = new LinkProperties(); + rmnet1.setInterfaceName("rmnet1"); + rmnet1.addLinkAddress(new LinkAddress("10.0.0.3/8")); + RouteInfo defaultRoute1 = new RouteInfo((IpPrefix) null, + NetworkUtils.numericToInetAddress("10.0.0.1"), rmnet1.getInterfaceName()); + RouteInfo directRoute1 = new RouteInfo(new IpPrefix("10.0.0.0/8"), null, + rmnet1.getInterfaceName()); + rmnet1.addRoute(defaultRoute1); + rmnet1.addRoute(directRoute1); + + // Check added routes + assertEqualRoutes(Arrays.asList(defaultRoute1, directRoute1), rmnet1.getAllRoutes()); + // ensureDirectlyConnectedRoutes() shouldn't change the routes since direct connected + // route is already part of the configuration. + rmnet1.ensureDirectlyConnectedRoutes(); + assertEqualRoutes(Arrays.asList(defaultRoute1, directRoute1), rmnet1.getAllRoutes()); + + // IPv6 case: only default routes added initially + LinkProperties rmnet2 = new LinkProperties(); + rmnet2.setInterfaceName("rmnet2"); + rmnet2.addLinkAddress(new LinkAddress("fe80::cafe/64")); + rmnet2.addLinkAddress(new LinkAddress("2001:db8::2/64")); + RouteInfo defaultRoute2 = new RouteInfo((IpPrefix) null, + NetworkUtils.numericToInetAddress("2001:db8::1"), rmnet2.getInterfaceName()); + RouteInfo directRoute2 = new RouteInfo(new IpPrefix("2001:db8::/64"), null, + rmnet2.getInterfaceName()); + RouteInfo linkLocalRoute2 = new RouteInfo(new IpPrefix("fe80::/64"), null, + rmnet2.getInterfaceName()); + rmnet2.addRoute(defaultRoute2); + + assertEqualRoutes(Arrays.asList(defaultRoute2), rmnet2.getAllRoutes()); + rmnet2.ensureDirectlyConnectedRoutes(); + assertEqualRoutes(Arrays.asList(defaultRoute2, directRoute2, linkLocalRoute2), + rmnet2.getAllRoutes()); + + // Corner case: no interface name + LinkProperties rmnet3 = new LinkProperties(); + rmnet3.addLinkAddress(new LinkAddress("192.168.0.2/24")); + RouteInfo directRoute3 = new RouteInfo(new IpPrefix("192.168.0.0/24"), null, + rmnet3.getInterfaceName()); + + assertTrue(rmnet3.getAllRoutes().isEmpty()); + rmnet3.ensureDirectlyConnectedRoutes(); + assertEqualRoutes(Collections.singletonList(directRoute3), rmnet3.getAllRoutes()); + + } + + private void assertEqualRoutes(Collection expected, Collection actual) { + Set expectedSet = new ArraySet<>(expected); + Set actualSet = new ArraySet<>(actual); + // Duplicated entries in actual routes are considered failures + assertEquals(actual.size(), actualSet.size()); + + assertEquals(expectedSet, actualSet); + } } diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 04b2112fae03..f1ea85374db3 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -4343,11 +4343,13 @@ public class ConnectivityService extends IConnectivityManager.Stub int currentScore, NetworkMisc networkMisc) { enforceConnectivityInternalPermission(); + LinkProperties lp = new LinkProperties(linkProperties); + lp.ensureDirectlyConnectedRoutes(); // TODO: Instead of passing mDefaultRequest, provide an API to determine whether a Network // satisfies mDefaultRequest. final NetworkAgentInfo nai = new NetworkAgentInfo(messenger, new AsyncChannel(), - new Network(reserveNetId()), new NetworkInfo(networkInfo), new LinkProperties( - linkProperties), new NetworkCapabilities(networkCapabilities), currentScore, + new Network(reserveNetId()), new NetworkInfo(networkInfo), lp, + new NetworkCapabilities(networkCapabilities), currentScore, mContext, mTrackerHandler, new NetworkMisc(networkMisc), mDefaultRequest, this); synchronized (this) { nai.networkMonitor.systemReady = mSystemReady; @@ -4657,6 +4659,8 @@ public class ConnectivityService extends IConnectivityManager.Stub synchronized (nai) { nai.linkProperties = newLp; } + // msg.obj is already a defensive copy. + nai.linkProperties.ensureDirectlyConnectedRoutes(); if (nai.everConnected) { updateLinkProperties(nai, oldLp); } diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index f6481cf59140..8816d43ef8de 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -64,6 +64,7 @@ import android.net.NetworkInfo.DetailedState; import android.net.NetworkMisc; import android.net.NetworkRequest; import android.net.NetworkSpecifier; +import android.net.NetworkUtils; import android.net.RouteInfo; import android.net.StringNetworkSpecifier; import android.net.metrics.IpConnectivityLog; @@ -88,6 +89,7 @@ import android.test.AndroidTestCase; import android.test.mock.MockContentResolver; import android.test.suitebuilder.annotation.SmallTest; import android.text.TextUtils; +import android.util.ArraySet; import android.util.Log; import android.util.LogPrinter; @@ -109,7 +111,10 @@ import org.mockito.Spy; import java.net.InetAddress; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; +import java.util.List; import java.util.Objects; +import java.util.Set; import java.util.concurrent.CountDownLatch; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.TimeUnit; @@ -304,6 +309,10 @@ public class ConnectivityServiceTest extends AndroidTestCase { private String mRedirectUrl; MockNetworkAgent(int transport) { + this(transport, new LinkProperties()); + } + + MockNetworkAgent(int transport, LinkProperties linkProperties) { final int type = transportToLegacyType(transport); final String typeName = ConnectivityManager.getNetworkTypeName(type); mNetworkInfo = new NetworkInfo(type, 0, typeName, "Mock"); @@ -329,7 +338,7 @@ public class ConnectivityServiceTest extends AndroidTestCase { mHandlerThread.start(); mNetworkAgent = new NetworkAgent(mHandlerThread.getLooper(), mServiceContext, "Mock-" + typeName, mNetworkInfo, mNetworkCapabilities, - new LinkProperties(), mScore, new NetworkMisc()) { + linkProperties, mScore, new NetworkMisc()) { @Override public void unwanted() { mDisconnected.open(); } @@ -3338,6 +3347,68 @@ public class ConnectivityServiceTest extends AndroidTestCase { assertException(() -> { mCm.requestRouteToHostAddress(TYPE_NONE, null); }, unsupported); } + @SmallTest + public void testLinkPropertiesEnsuresDirectlyConnectedRoutes() { + final NetworkRequest networkRequest = new NetworkRequest.Builder() + .addTransportType(TRANSPORT_WIFI).build(); + final TestNetworkCallback networkCallback = new TestNetworkCallback(); + mCm.registerNetworkCallback(networkRequest, networkCallback); + + LinkProperties lp = new LinkProperties(); + lp.setInterfaceName("wlan0"); + LinkAddress myIpv4Address = new LinkAddress("192.168.12.3/24"); + RouteInfo myIpv4DefaultRoute = new RouteInfo((IpPrefix) null, + NetworkUtils.numericToInetAddress("192.168.12.1"), lp.getInterfaceName()); + lp.addLinkAddress(myIpv4Address); + lp.addRoute(myIpv4DefaultRoute); + + // Verify direct routes are added when network agent is first registered in + // ConnectivityService. + MockNetworkAgent networkAgent = new MockNetworkAgent(TRANSPORT_WIFI, lp); + networkAgent.connect(true); + networkCallback.expectCallback(CallbackState.AVAILABLE, networkAgent); + networkCallback.expectCallback(CallbackState.NETWORK_CAPABILITIES, networkAgent); + CallbackInfo cbi = networkCallback.expectCallback(CallbackState.LINK_PROPERTIES, + networkAgent); + networkCallback.expectCapabilitiesWith(NET_CAPABILITY_VALIDATED, networkAgent); + networkCallback.assertNoCallback(); + checkDirectlyConnectedRoutes(cbi.arg, Arrays.asList(myIpv4Address), + Arrays.asList(myIpv4DefaultRoute)); + checkDirectlyConnectedRoutes(mCm.getLinkProperties(networkAgent.getNetwork()), + Arrays.asList(myIpv4Address), Arrays.asList(myIpv4DefaultRoute)); + + // Verify direct routes are added during subsequent link properties updates. + LinkProperties newLp = new LinkProperties(lp); + LinkAddress myIpv6Address1 = new LinkAddress("fe80::cafe/64"); + LinkAddress myIpv6Address2 = new LinkAddress("2001:db8::2/64"); + newLp.addLinkAddress(myIpv6Address1); + newLp.addLinkAddress(myIpv6Address2); + networkAgent.sendLinkProperties(newLp); + cbi = networkCallback.expectCallback(CallbackState.LINK_PROPERTIES, networkAgent); + networkCallback.assertNoCallback(); + checkDirectlyConnectedRoutes(cbi.arg, + Arrays.asList(myIpv4Address, myIpv6Address1, myIpv6Address2), + Arrays.asList(myIpv4DefaultRoute)); + mCm.unregisterNetworkCallback(networkCallback); + } + + private void checkDirectlyConnectedRoutes(Object callbackObj, + Collection linkAddresses, Collection otherRoutes) { + assertTrue(callbackObj instanceof LinkProperties); + LinkProperties lp = (LinkProperties) callbackObj; + + Set expectedRoutes = new ArraySet<>(); + expectedRoutes.addAll(otherRoutes); + for (LinkAddress address : linkAddresses) { + RouteInfo localRoute = new RouteInfo(address, null, lp.getInterfaceName()); + // Duplicates in linkAddresses are considered failures + assertTrue(expectedRoutes.add(localRoute)); + } + List observedRoutes = lp.getRoutes(); + assertEquals(expectedRoutes.size(), observedRoutes.size()); + assertTrue(observedRoutes.containsAll(expectedRoutes)); + } + private static void assertEmpty(T[] ts) { int length = ts.length; assertEquals("expected empty array, but length was " + length, 0, length);