From d00ddf567235e20bcbdc527ae20824fa944c3a64 Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Tue, 8 Aug 2017 13:08:06 +0900 Subject: [PATCH] IpManager: use InitialConfiguration for provisioning This patch changes IpManager to take into account static provisioning information specified in the InitialConfiguration for IPv6 static configuration. When a valid InitialConfiguration with IPv6 content is specified, IpManager will do the following things: - at start(), it will push the IPv6 addresses in the config to netd - it will observe all addresses be notified via Netlink - when all addresses are there, it will patch in the associated IPv6 routes in the config, so that they get passed to ConnectivityService through the usual mechanism The logic triggering onProvisioningSuccess is also changed to take into account InitialConfiguration: when all addresses and all routes in the config are seen the provisioning is successful. Bug: 62988545 Test: runtest frameworks-net, with newly added tests Change-Id: I77ed7c576c4b198de7a4726be70c78b74689e98b --- .../net/java/android/net/ip/IpManager.java | 196 ++++++++++++---- .../java/android/net/ip/IpManagerTest.java | 211 +++++++++++++++++- 2 files changed, 361 insertions(+), 46 deletions(-) diff --git a/services/net/java/android/net/ip/IpManager.java b/services/net/java/android/net/ip/IpManager.java index 6a46b5b1d09a9..b487192c2b379 100644 --- a/services/net/java/android/net/ip/IpManager.java +++ b/services/net/java/android/net/ip/IpManager.java @@ -36,6 +36,7 @@ import android.net.dhcp.DhcpClient; import android.net.metrics.IpConnectivityLog; import android.net.metrics.IpManagerEvent; import android.net.util.MultinetworkPolicyTracker; +import android.net.util.NetdService; import android.net.util.NetworkConstants; import android.net.util.SharedLog; import android.os.INetworkManagementService; @@ -66,9 +67,11 @@ import java.net.Inet6Address; import java.net.InetAddress; import java.net.NetworkInterface; import java.net.SocketException; +import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; import java.util.Objects; +import java.util.List; import java.util.Set; import java.util.StringJoiner; import java.util.function.Predicate; @@ -423,6 +426,10 @@ public class IpManager extends StateMachine { } public boolean isValid() { + if (ipAddresses.isEmpty()) { + return false; + } + // For every IP address, there must be at least one prefix containing that address. for (LinkAddress addr : ipAddresses) { if (!any(directlyConnectedRoutes, (p) -> p.contains(addr.getAddress()))) { @@ -459,14 +466,47 @@ public class IpManager extends StateMachine { return true; } + /** + * @return true if the given list of addressess and routes satisfies provisioning for this + * InitialConfiguration. LinkAddresses and RouteInfo objects are not compared with equality + * because addresses and routes seen by Netlink will contain additional fields like flags, + * interfaces, and so on. If this InitialConfiguration has no IP address specified, the + * provisioning check always fails. + * + * If the given list of routes is null, only addresses are taken into considerations. + */ + public boolean isProvisionedBy(List addresses, List routes) { + if (ipAddresses.isEmpty()) { + return false; + } + + for (LinkAddress addr : ipAddresses) { + if (!any(addresses, (addrSeen) -> addr.isSameAddressAs(addrSeen))) { + return false; + } + } + + if (routes != null) { + for (IpPrefix prefix : directlyConnectedRoutes) { + if (!any(routes, (routeSeen) -> isDirectlyConnectedRoute(routeSeen, prefix))) { + return false; + } + } + } + + return true; + } + + private static boolean isDirectlyConnectedRoute(RouteInfo route, IpPrefix prefix) { + return !route.hasGateway() && prefix.equals(route.getDestination()); + } + private static boolean isPrefixLengthCompliant(LinkAddress addr) { - return (addr.getAddress() instanceof Inet4Address) - || isCompliantIPv6PrefixLength(addr.getPrefixLength()); + return addr.isIPv4() || isCompliantIPv6PrefixLength(addr.getPrefixLength()); } private static boolean isPrefixLengthCompliant(IpPrefix prefix) { - return (prefix.getAddress() instanceof Inet4Address) - || isCompliantIPv6PrefixLength(prefix.getPrefixLength()); + return prefix.isIPv4() || isCompliantIPv6PrefixLength(prefix.getPrefixLength()); } private static boolean isCompliantIPv6PrefixLength(int prefixLength) { @@ -479,28 +519,7 @@ public class IpManager extends StateMachine { } private static boolean isIPv6GUA(LinkAddress addr) { - return (addr.getAddress() instanceof Inet6Address) && addr.isGlobalPreferred(); - } - - private static boolean any(Iterable coll, Predicate fn) { - for (T t : coll) { - if (fn.test(t)) { - return true; - } - } - return false; - } - - private static boolean all(Iterable coll, Predicate fn) { - return !any(coll, not(fn)); - } - - private static Predicate not(Predicate fn) { - return (t) -> !fn.test(t); - } - - private static String join(String delimiter, Collection coll) { - return coll.stream().map(Object::toString).collect(Collectors.joining(delimiter)); + return addr.isIPv6() && addr.isGlobalPreferred(); } } @@ -549,6 +568,7 @@ public class IpManager extends StateMachine { private final LocalLog mConnectivityPacketLog; private final MessageHandlingLogger mMsgStateLogger; private final IpConnectivityLog mMetricsLog = new IpConnectivityLog(); + private final INetd mNetd; private NetworkInterface mNetworkInterface; @@ -568,14 +588,22 @@ public class IpManager extends StateMachine { public IpManager(Context context, String ifName, Callback callback) { this(context, ifName, callback, INetworkManagementService.Stub.asInterface( - ServiceManager.getService(Context.NETWORKMANAGEMENT_SERVICE))); + ServiceManager.getService(Context.NETWORKMANAGEMENT_SERVICE)), + NetdService.getInstance()); } /** * An expanded constructor, useful for dependency injection. + * TODO: migrate all test users to mock IpManager directly and remove this ctor. */ public IpManager(Context context, String ifName, Callback callback, INetworkManagementService nwService) { + this(context, ifName, callback, nwService, NetdService.getInstance()); + } + + @VisibleForTesting + IpManager(Context context, String ifName, Callback callback, + INetworkManagementService nwService, INetd netd) { super(IpManager.class.getSimpleName() + "." + ifName); mTag = getName(); @@ -584,6 +612,7 @@ public class IpManager extends StateMachine { mClatInterfaceName = CLAT_PREFIX + ifName; mCallback = new LoggingCallbackWrapper(callback); mNwService = nwService; + mNetd = netd; mLog = new SharedLog(MAX_LOG_RECORDS, mTag); mConnectivityPacketLog = new LocalLog(MAX_PACKET_RECORDS); @@ -886,10 +915,20 @@ public class IpManager extends StateMachine { } // For now: use WifiStateMachine's historical notion of provisioned. - private static boolean isProvisioned(LinkProperties lp) { + @VisibleForTesting + static boolean isProvisioned(LinkProperties lp, InitialConfiguration config) { // For historical reasons, we should connect even if all we have is // an IPv4 address and nothing else. - return lp.isProvisioned() || lp.hasIPv4Address(); + if (lp.hasIPv4Address() || lp.isProvisioned()) { + return true; + } + if (config == null) { + return false; + } + + // When an InitialConfiguration is specified, ignore any difference with previous + // properties and instead check if properties observed match the desired properties. + return config.isProvisionedBy(lp.getLinkAddresses(), lp.getRoutes()); } // TODO: Investigate folding all this into the existing static function @@ -898,12 +937,11 @@ public class IpManager extends StateMachine { // object that is a correct and complete assessment of what changed, taking // account of the asymmetries described in the comments in this function. // Then switch to using it everywhere (IpReachabilityMonitor, etc.). - private ProvisioningChange compareProvisioning( - LinkProperties oldLp, LinkProperties newLp) { + private ProvisioningChange compareProvisioning(LinkProperties oldLp, LinkProperties newLp) { ProvisioningChange delta; - - final boolean wasProvisioned = isProvisioned(oldLp); - final boolean isProvisioned = isProvisioned(newLp); + InitialConfiguration config = mConfiguration != null ? mConfiguration.mInitialConfig : null; + final boolean wasProvisioned = isProvisioned(oldLp, config); + final boolean isProvisioned = isProvisioned(newLp, config); if (!wasProvisioned && isProvisioned) { delta = ProvisioningChange.GAINED_PROVISIONING; @@ -1016,10 +1054,6 @@ public class IpManager extends StateMachine { return delta; } - private boolean linkPropertiesUnchanged(LinkProperties newLp) { - return Objects.equals(newLp, mLinkProperties); - } - private LinkProperties assembleLinkProperties() { // [1] Create a new LinkProperties object to populate. LinkProperties newLp = new LinkProperties(); @@ -1066,9 +1100,26 @@ public class IpManager extends StateMachine { newLp.setHttpProxy(mHttpProxy); } - if (VDBG) { - Log.d(mTag, "newLp{" + newLp + "}"); + // [5] Add data from InitialConfiguration + if (mConfiguration != null && mConfiguration.mInitialConfig != null) { + InitialConfiguration config = mConfiguration.mInitialConfig; + // Add InitialConfiguration routes and dns server addresses once all addresses + // specified in the InitialConfiguration have been observed with Netlink. + if (config.isProvisionedBy(newLp.getLinkAddresses(), null)) { + for (IpPrefix prefix : config.directlyConnectedRoutes) { + newLp.addRoute(new RouteInfo(prefix, null, mInterfaceName)); + } + } + addAllReachableDnsServers(newLp, config.dnsServers); } + final LinkProperties oldLp = mLinkProperties; + if (VDBG) { + Log.d(mTag, String.format("Netlink-seen LPs: %s, new LPs: %s; old LPs: %s", + netlinkLinkProperties, newLp, oldLp)); + } + + // TODO: also learn via netlink routes specified by an InitialConfiguration and specified + // from a static IP v4 config instead of manually patching them in in steps [3] and [5]. return newLp; } @@ -1087,7 +1138,7 @@ public class IpManager extends StateMachine { // Returns false if we have lost provisioning, true otherwise. private boolean handleLinkPropertiesUpdate(boolean sendCallbacks) { final LinkProperties newLp = assembleLinkProperties(); - if (linkPropertiesUnchanged(newLp)) { + if (Objects.equals(newLp, mLinkProperties)) { return true; } final ProvisioningChange delta = setLinkProperties(newLp); @@ -1218,6 +1269,26 @@ public class IpManager extends StateMachine { return true; } + private boolean applyInitialConfig(InitialConfiguration config) { + if (mNetd == null) { + logError("tried to add %s to %s but INetd was null", config, mInterfaceName); + return false; + } + + // TODO: also support specifying a static IPv4 configuration in InitialConfiguration. + for (LinkAddress addr : findAll(config.ipAddresses, LinkAddress::isIPv6)) { + try { + mNetd.interfaceAddAddress( + mInterfaceName, addr.getAddress().getHostAddress(), addr.getPrefixLength()); + } catch (ServiceSpecificException | RemoteException e) { + logError("failed to add %s to %s: %s", addr, mInterfaceName, e); + return false; + } + } + + return true; + } + private boolean startIpReachabilityMonitor() { try { mIpReachabilityMonitor = new IpReachabilityMonitor( @@ -1446,6 +1517,14 @@ public class IpManager extends StateMachine { return; } + InitialConfiguration config = mConfiguration.mInitialConfig; + if ((config != null) && !applyInitialConfig(config)) { + // TODO introduce a new IpManagerEvent constant to distinguish this error case. + doImmediateProvisioningFailure(IpManagerEvent.ERROR_INVALID_PROVISIONING); + transitionTo(mStoppingState); + return; + } + if (mConfiguration.mUsingIpReachabilityMonitor && !startIpReachabilityMonitor()) { doImmediateProvisioningFailure( IpManagerEvent.ERROR_STARTING_IPREACHABILITYMONITOR); @@ -1652,4 +1731,39 @@ public class IpManager extends StateMachine { receivedInState, processedInState); } } + + // TODO: extract out into CollectionUtils. + static boolean any(Iterable coll, Predicate fn) { + for (T t : coll) { + if (fn.test(t)) { + return true; + } + } + return false; + } + + static boolean all(Iterable coll, Predicate fn) { + return !any(coll, not(fn)); + } + + static Predicate not(Predicate fn) { + return (t) -> !fn.test(t); + } + + static String join(String delimiter, Collection coll) { + return coll.stream().map(Object::toString).collect(Collectors.joining(delimiter)); + } + + static T find(Iterable coll, Predicate fn) { + for (T t: coll) { + if (fn.test(t)) { + return t; + } + } + return null; + } + + static List findAll(Collection coll, Predicate fn) { + return coll.stream().filter(fn).collect(Collectors.toList()); + } } diff --git a/tests/net/java/android/net/ip/IpManagerTest.java b/tests/net/java/android/net/ip/IpManagerTest.java index dc77e22082baa..541f91adf7478 100644 --- a/tests/net/java/android/net/ip/IpManagerTest.java +++ b/tests/net/java/android/net/ip/IpManagerTest.java @@ -17,6 +17,8 @@ package android.net.ip; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.Mockito.any; import static org.mockito.Mockito.eq; @@ -32,8 +34,11 @@ import android.app.AlarmManager; import android.content.ContentResolver; import android.content.Context; import android.content.res.Resources; +import android.net.INetd; import android.net.IpPrefix; import android.net.LinkAddress; +import android.net.LinkProperties; +import android.net.RouteInfo; import android.net.ip.IpManager.Callback; import android.net.ip.IpManager.InitialConfiguration; import android.net.ip.IpManager.ProvisioningConfiguration; @@ -45,16 +50,20 @@ import android.test.mock.MockContentResolver; import com.android.internal.util.test.FakeSettingsProvider; import com.android.internal.R; +import com.android.server.net.BaseNetworkObserver; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.MockitoAnnotations; import java.net.Inet4Address; import java.net.Inet6Address; import java.net.InetAddress; +import java.util.Arrays; +import java.util.List; import java.util.HashSet; import java.util.Set; @@ -71,11 +80,14 @@ public class IpManagerTest { @Mock private Context mContext; @Mock private INetworkManagementService mNMService; + @Mock private INetd mNetd; @Mock private Resources mResources; @Mock private Callback mCb; @Mock private AlarmManager mAlarm; private MockContentResolver mContentResolver; + BaseNetworkObserver mObserver; + @Before public void setUp() throws Exception { MockitoAnnotations.initMocks(this); @@ -91,9 +103,13 @@ public class IpManagerTest { } private IpManager makeIpManager(String ifname) throws Exception { - final IpManager ipm = new IpManager(mContext, ifname, mCb, mNMService); + final IpManager ipm = new IpManager(mContext, ifname, mCb, mNMService, mNetd); verify(mNMService, timeout(100).times(1)).disableIpv6(ifname); verify(mNMService, timeout(100).times(1)).clearInterfaceAddresses(ifname); + ArgumentCaptor arg = + ArgumentCaptor.forClass(BaseNetworkObserver.class); + verify(mNMService, times(1)).registerObserver(arg.capture()); + mObserver = arg.getValue(); reset(mNMService); return ipm; } @@ -130,6 +146,134 @@ public class IpManagerTest { verify(mNMService, timeout(100).times(1)).clearInterfaceAddresses(iface); } + @Test + public void testProvisioningWithInitialConfiguration() throws Exception { + final String iface = "test_wlan0"; + final IpManager ipm = makeIpManager(iface); + + String[] addresses = { + "fe80::a4be:f92:e1f7:22d1/64", + "fe80::f04a:8f6:6a32:d756/64", + "fd2c:4e57:8e3c:0:548d:2db2:4fcf:ef75/64" + }; + String[] prefixes = { "fe80::/64", "fd2c:4e57:8e3c::/64" }; + + ProvisioningConfiguration config = new ProvisioningConfiguration.Builder() + .withoutIPv4() + .withoutIpReachabilityMonitor() + .withInitialConfiguration(conf(links(addresses), prefixes(prefixes), ips())) + .build(); + + ipm.startProvisioning(config); + verify(mCb, times(1)).setNeighborDiscoveryOffload(true); + verify(mCb, timeout(100).times(1)).setFallbackMulticastFilter(false); + verify(mCb, never()).onProvisioningFailure(any()); + + for (String addr : addresses) { + String[] parts = addr.split("/"); + verify(mNetd, timeout(100).times(1)) + .interfaceAddAddress(iface, parts[0], Integer.parseInt(parts[1])); + } + + final int lastAddr = addresses.length - 1; + + // Add N - 1 addresses + for (int i = 0; i < lastAddr; i++) { + mObserver.addressUpdated(iface, new LinkAddress(addresses[i])); + verify(mCb, timeout(100).times(1)).onLinkPropertiesChange(any()); + } + + // Add Nth address + mObserver.addressUpdated(iface, new LinkAddress(addresses[lastAddr])); + LinkProperties want = linkproperties(links(addresses), routes(prefixes)); + want.setInterfaceName(iface); + verify(mCb, timeout(100).times(1)).onProvisioningSuccess(eq(want)); + + ipm.stop(); + verify(mNMService, timeout(100).times(1)).disableIpv6(iface); + verify(mNMService, timeout(100).times(1)).clearInterfaceAddresses(iface); + } + + @Test + public void testIsProvisioned() throws Exception { + InitialConfiguration empty = conf(links(), prefixes()); + IsProvisionedTestCase[] testcases = { + // nothing + notProvisionedCase(links(), routes(), dns(), null), + notProvisionedCase(links(), routes(), dns(), empty), + + // IPv4 + provisionedCase(links("192.0.2.12/24"), routes(), dns(), empty), + + // IPv6 + notProvisionedCase( + links("fe80::a4be:f92:e1f7:22d1/64", "fd2c:4e57:8e3c:0:548d:2db2:4fcf:ef75/64"), + routes(), dns(), empty), + notProvisionedCase( + links("fe80::a4be:f92:e1f7:22d1/64", "fd2c:4e57:8e3c:0:548d:2db2:4fcf:ef75/64"), + routes("fe80::/64", "fd2c:4e57:8e3c::/64"), dns("fd00:1234:5678::1000"), empty), + provisionedCase( + links("2001:db8:dead:beef:f00::a0/64", "fe80::1/64"), + routes("::/0"), + dns("2001:db8:dead:beef:f00::02"), empty), + + // Initial configuration + provisionedCase( + links("fe80::e1f7:22d1/64", "fd2c:4e57:8e3c:0:548d:2db2:4fcf:ef75/64"), + routes("fe80::/64", "fd2c:4e57:8e3c::/64"), + dns(), + conf(links("fe80::e1f7:22d1/64", "fd2c:4e57:8e3c:0:548d:2db2:4fcf:ef75/64"), + prefixes( "fe80::/64", "fd2c:4e57:8e3c::/64"), ips())) + }; + + for (IsProvisionedTestCase testcase : testcases) { + if (IpManager.isProvisioned(testcase.lp, testcase.config) != testcase.isProvisioned) { + fail(testcase.errorMessage()); + } + } + } + + static class IsProvisionedTestCase { + boolean isProvisioned; + LinkProperties lp; + InitialConfiguration config; + + String errorMessage() { + return String.format("expected %s with config %s to be %s, but was %s", + lp, config, provisioned(isProvisioned), provisioned(!isProvisioned)); + } + + static String provisioned(boolean isProvisioned) { + return isProvisioned ? "provisioned" : "not provisioned"; + } + } + + static IsProvisionedTestCase provisionedCase(Set lpAddrs, Set lpRoutes, + Set lpDns, InitialConfiguration config) { + return provisioningTest(true, lpAddrs, lpRoutes, lpDns, config); + } + + static IsProvisionedTestCase notProvisionedCase(Set lpAddrs, + Set lpRoutes, Set lpDns, InitialConfiguration config) { + return provisioningTest(false, lpAddrs, lpRoutes, lpDns, config); + } + + static IsProvisionedTestCase provisioningTest(boolean isProvisioned, Set lpAddrs, + Set lpRoutes, Set lpDns, InitialConfiguration config) { + IsProvisionedTestCase testcase = new IsProvisionedTestCase(); + testcase.isProvisioned = isProvisioned; + testcase.lp = new LinkProperties(); + testcase.lp.setLinkAddresses(lpAddrs); + for (RouteInfo route : lpRoutes) { + testcase.lp.addRoute(route); + } + for (InetAddress dns : lpDns) { + testcase.lp.addDnsServer(dns); + } + testcase.config = config; + return testcase; + } + @Test public void testInitialConfigurations() throws Exception { InitialConfigurationTestCase[] testcases = { @@ -152,6 +296,7 @@ public class IpManagerTest { prefixes("fd00:1234:5678::/48"), dns("fd00:1234:5678::1000")), + invalidConf("empty configuration", links(), prefixes(), dns()), invalidConf("v4 addr and dns not in any prefix", links("192.0.2.12/24"), prefixes("198.51.100.0/24"), dns("192.0.2.2")), invalidConf("v4 addr not in any prefix", @@ -189,10 +334,9 @@ public class IpManagerTest { return String.format("%s: expected configuration %s to be %s, but was %s", descr, config, validString(isValid), validString(!isValid)); } - } - - static String validString(boolean isValid) { - return isValid ? VALID : INVALID; + static String validString(boolean isValid) { + return isValid ? VALID : INVALID; + } } static InitialConfigurationTestCase validConf(String descr, Set links, @@ -214,6 +358,19 @@ public class IpManagerTest { return testcase; } + static LinkProperties linkproperties(Set addresses, Set routes) { + LinkProperties lp = new LinkProperties(); + lp.setLinkAddresses(addresses); + for (RouteInfo route : routes) { + lp.addRoute(route); + } + return lp; + } + + static InitialConfiguration conf(Set links, Set prefixes) { + return conf(links, prefixes, new HashSet<>()); + } + static InitialConfiguration conf( Set links, Set prefixes, Set dns) { InitialConfiguration conf = new InitialConfiguration(); @@ -223,6 +380,10 @@ public class IpManagerTest { return conf; } + static Set routes(String... routes) { + return mapIntoSet(routes, (r) -> new RouteInfo(new IpPrefix(r))); + } + static Set prefixes(String... prefixes) { return mapIntoSet(prefixes, IpPrefix::new); } @@ -254,4 +415,44 @@ public class IpManagerTest { interface Fn { B call(A a) throws Exception; } + + @Test + public void testAll() { + List list1 = Arrays.asList(); + List list2 = Arrays.asList("foo"); + List list3 = Arrays.asList("bar", "baz"); + List list4 = Arrays.asList("foo", "bar", "baz"); + + assertTrue(IpManager.all(list1, (x) -> false)); + assertFalse(IpManager.all(list2, (x) -> false)); + assertTrue(IpManager.all(list3, (x) -> true)); + assertTrue(IpManager.all(list2, (x) -> x.charAt(0) == 'f')); + assertFalse(IpManager.all(list4, (x) -> x.charAt(0) == 'f')); + } + + @Test + public void testAny() { + List list1 = Arrays.asList(); + List list2 = Arrays.asList("foo"); + List list3 = Arrays.asList("bar", "baz"); + List list4 = Arrays.asList("foo", "bar", "baz"); + + assertFalse(IpManager.any(list1, (x) -> true)); + assertTrue(IpManager.any(list2, (x) -> true)); + assertTrue(IpManager.any(list2, (x) -> x.charAt(0) == 'f')); + assertFalse(IpManager.any(list3, (x) -> x.charAt(0) == 'f')); + assertTrue(IpManager.any(list4, (x) -> x.charAt(0) == 'f')); + } + + @Test + public void testFindAll() { + List list1 = Arrays.asList(); + List list2 = Arrays.asList("foo"); + List list3 = Arrays.asList("foo", "bar", "baz"); + + assertEquals(list1, IpManager.findAll(list1, (x) -> true)); + assertEquals(list1, IpManager.findAll(list3, (x) -> false)); + assertEquals(list3, IpManager.findAll(list3, (x) -> true)); + assertEquals(list2, IpManager.findAll(list3, (x) -> x.charAt(0) == 'f')); + } }