Merge "Always add local subnet routes to the interface's routing table"

am: 609dd56140

Change-Id: I0b1728fae18c891d91fafdfbb1905c05e0fea47f

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;
@@ -503,12 +502,23 @@ public final class LinkProperties implements Parcelable {
         return Collections.unmodifiableList(mRoutes);
     }
 
+    /**
+     * 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());

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<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);
+    }
 }

services/core/java/com/android/server/ConnectivityService.java

@@ -4335,11 +4335,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(
+                new Network(reserveNetId()), new NetworkInfo(networkInfo), lp,
-                linkProperties), new NetworkCapabilities(networkCapabilities), currentScore,
+                new NetworkCapabilities(networkCapabilities), currentScore,
                 mContext, mTrackerHandler, new NetworkMisc(networkMisc), mDefaultRequest, this);
         synchronized (this) {
             nai.networkMonitor.systemReady = mSystemReady;
@@ -4649,6 +4651,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);
         }

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<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);