OSDN Git Service

Always add local subnet routes to the interface's routing table
authorRubin Xu <rubinxu@google.com>
Tue, 5 Sep 2017 17:40:49 +0000 (18:40 +0100)
committerRubin Xu <rubinxu@google.com>
Thu, 7 Sep 2017 09:50:20 +0000 (10:50 +0100)
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.
Change-Id: I35b614eebccfd22c4a5270f40256f9be1e25abfb

core/java/android/net/LinkProperties.java
core/tests/coretests/src/android/net/LinkPropertiesTest.java
services/core/java/com/android/server/ConnectivityService.java
tests/net/java/com/android/server/ConnectivityServiceTest.java

index 1bb0fbb..f527f77 100644 (file)
@@ -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<RouteInfo> getAllRoutes() {
-        List<RouteInfo> routes = new ArrayList();
+        List<RouteInfo> routes = new ArrayList<>();
         routes.addAll(mRoutes);
         for (LinkProperties stacked: mStackedLinks.values()) {
             routes.addAll(stacked.getAllRoutes());
index d5f6321..9686dd9 100644 (file)
@@ -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<RouteInfo> expected, Collection<RouteInfo> actual) {
+        Set<RouteInfo> expectedSet = new ArraySet<>(expected);
+        Set<RouteInfo> actualSet = new ArraySet<>(actual);
+        // Duplicated entries in actual routes are considered failures
+        assertEquals(actual.size(), actualSet.size());
+
+        assertEquals(expectedSet, actualSet);
+    }
 }
index 874303d..135872b 100644 (file)
@@ -4316,11 +4316,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;
@@ -4630,6 +4632,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);
         }
index f6481cf..8816d43 100644 (file)
@@ -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<LinkAddress> linkAddresses, Collection<RouteInfo> otherRoutes) {
+        assertTrue(callbackObj instanceof LinkProperties);
+        LinkProperties lp = (LinkProperties) callbackObj;
+
+        Set<RouteInfo> 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<RouteInfo> observedRoutes = lp.getRoutes();
+        assertEquals(expectedRoutes.size(), observedRoutes.size());
+        assertTrue(observedRoutes.containsAll(expectedRoutes));
+    }
+
     private static <T> void assertEmpty(T[] ts) {
         int length = ts.length;
         assertEquals("expected empty array, but length was " + length, 0, length);