From 720ccd2b3232bd3491070301067d1734845e1a4b Mon Sep 17 00:00:00 2001 From: Sewook Seo Date: Mon, 18 Nov 2024 12:46:12 +0000 Subject: [PATCH] Add a Map to NetworkAgent list for IP address Keeping a map from IP addresses to the list of NetworkAgentInfos that have that IP address in their LinkProperties. Bug: 373280981 Bug: 331139818i Flag: android.net.platform.flags.connectivity_service_destroy_socket Test: atest ConnectivityCoverageTests Test: device test Change-Id: I1e39ce00d8bc49252df8088efc768804ae9ee0a8 --- .../android/server/ConnectivityService.java | 69 ++++++++ .../net/module/util/LinkPropertiesUtils.java | 17 ++ .../CSDestroySocketTest.kt | 147 ++++++++++++++++-- .../base/CSAgentWrapper.kt | 142 ++++++++++++++++- .../server/connectivityservice/base/CSTest.kt | 2 + 5 files changed, 363 insertions(+), 14 deletions(-) diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 0a74857a6c..2f28265334 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -275,6 +275,7 @@ import android.net.netd.aidl.NativeUidRangeConfig; import android.net.networkstack.ModuleNetworkStackClient; import android.net.networkstack.NetworkStackClientBase; import android.net.networkstack.aidl.NetworkMonitorParameters; +import android.net.platform.flags.Flags; import android.net.resolv.aidl.DnsHealthEventParcel; import android.net.resolv.aidl.IDnsResolverUnsolicitedEventListener; import android.net.resolv.aidl.Nat64PrefixEventParcel; @@ -348,6 +349,7 @@ import com.android.net.module.util.CollectionUtils; import com.android.net.module.util.DeviceConfigUtils; import com.android.net.module.util.HandlerUtils; import com.android.net.module.util.InterfaceParams; +import com.android.net.module.util.LinkPropertiesUtils; import com.android.net.module.util.LinkPropertiesUtils.CompareOrUpdateResult; import com.android.net.module.util.LinkPropertiesUtils.CompareResult; import com.android.net.module.util.LocationPermissionChecker; @@ -409,6 +411,7 @@ import java.io.InterruptedIOException; import java.io.PrintWriter; import java.io.Writer; import java.net.Inet4Address; +import java.net.Inet6Address; import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.SocketException; @@ -1130,6 +1133,9 @@ public class ConnectivityService extends IConnectivityManager.Stub @Nullable private final QuicConnectionCloser mQuicConnectionCloser; + // A map from IP address to networks. Only the handler thread is allowed to access this field. + @Nullable @VisibleForTesting final Map> mIpToNetworksMap; + /** * Implements support for the legacy "one network per network type" model. * @@ -1859,6 +1865,11 @@ public class ConnectivityService extends IConnectivityManager.Stub final SparseArray networkForNetId, final Handler handler) { return new QuicConnectionCloser(networkForNetId, handler); } + + /** Whether the flag for connectivity service socket destroy is enabled or not. */ + public boolean flagConnectivityServiceDestroySocket() { + return Flags.connectivityServiceDestroySocket(); + } } public ConnectivityService(Context context) { @@ -1993,6 +2004,11 @@ public class ConnectivityService extends IConnectivityManager.Stub mSatelliteAccessController = null; } + if (mDeps.flagConnectivityServiceDestroySocket()) { + mIpToNetworksMap = new HashMap<>(); + } else { + mIpToNetworksMap = null; + } // To ensure uid state is synchronized with Network Policy, register for // NetworkPolicyManagerService events must happen prior to NetworkPolicyManagerService // reading existing policy from disk. @@ -4586,6 +4602,21 @@ public class ConnectivityService extends IConnectivityManager.Stub (mMulticastRoutingCoordinatorService != null)); pw.println("Background firewall chain enabled: " + mBackgroundFirewallChainEnabled); pw.println("IngressToVpnAddressFiltering: " + mIngressToVpnAddressFiltering); + + if (mIpToNetworksMap != null) { + pw.println(); + pw.println("mIpToNetworksMap:"); + pw.increaseIndent(); + for (Map.Entry> info : mIpToNetworksMap.entrySet()) { + StringBuilder sb = new StringBuilder(); + sb.append(info.getKey()).append(": "); + for (NetworkAgentInfo nai : info.getValue()) { + sb.append("{").append(nai.network).append("}"); + } + pw.println(sb); + } + pw.decreaseIndent(); + } } private void dumpNetworks(IndentingPrintWriter pw) { @@ -5732,6 +5763,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // for an unnecessarily long time. destroyNativeNetwork(nai); } + updateIpAddressesForNetwork(nai, nai.linkProperties, null); if (!nai.isCreated() && !mDeps.isAtLeastT()) { // Backwards compatibility: send onNetworkDestroyed even if network was never created. // This can never run if the code above runs because shouldDestroyNativeNetwork is @@ -9850,12 +9882,49 @@ public class ConnectivityService extends IConnectivityManager.Stub } networkAgent.networkMonitor().notifyLinkPropertiesChanged( new LinkProperties(newLp, true /* parcelSensitiveFields */)); + updateIpAddressesForNetwork(networkAgent, oldLp, newLp); notifyNetworkCallbacks(networkAgent, CALLBACK_IP_CHANGED); } mKeepaliveTracker.handleCheckKeepalivesStillValid(networkAgent); } + private void updateIpAddressesForNetwork( + NetworkAgentInfo nai, LinkProperties oldLp, LinkProperties newLp) { + ensureRunningOnConnectivityServiceThread(); + if (mIpToNetworksMap == null) { + return; + } + + final CompareResult result = + LinkPropertiesUtils.compareAllAddresses(oldLp, newLp); + + if (!result.added.isEmpty()) { + for (LinkAddress la : result.added) { + if (la.getAddress() instanceof Inet6Address + && la.getAddress().isLinkLocalAddress()) { + // Some NetworkAgents may not report IPv6 link local address to CS, thus socket + // destruction on the address will be done at handling RTM_DELADDR and the + // address doesn't need to be added to mIpToNetworksMap. + continue; + } + mIpToNetworksMap.computeIfAbsent(la.getAddress(), k -> new ArraySet<>()).add(nai); + } + } + + if (!result.removed.isEmpty()) { + for (LinkAddress la : result.removed) { + final Set networks = mIpToNetworksMap.get(la.getAddress()); + if (networks != null) { + networks.remove(nai); + if (networks.isEmpty()) { + mIpToNetworksMap.remove(la.getAddress()); + } + } + } + } + } + private void applyInitialLinkProperties(@NonNull NetworkAgentInfo nai) { updateLinkProperties(nai, new LinkProperties(nai.linkProperties), null); } diff --git a/staticlibs/framework/com/android/net/module/util/LinkPropertiesUtils.java b/staticlibs/framework/com/android/net/module/util/LinkPropertiesUtils.java index e271f64902..4760dfca5f 100644 --- a/staticlibs/framework/com/android/net/module/util/LinkPropertiesUtils.java +++ b/staticlibs/framework/com/android/net/module/util/LinkPropertiesUtils.java @@ -146,6 +146,23 @@ public final class LinkPropertiesUtils { right != null ? right.getLinkAddresses() : null); } + /** + * Compares the all addresses including stacked addresses in {@code left} LinkProperties with + * {@code right} LinkProperties, examining only addresses on the base link. + * + * @param left A LinkProperties with the old list of all addresses. + * @param right A LinkProperties with the new list of all addresses. + * @return the differences between the addresses. + */ + public static @NonNull CompareResult compareAllAddresses( + @Nullable LinkProperties left, @Nullable LinkProperties right) { + /* + * Same logic as compareAddresses() + */ + return new CompareResult<>(left != null ? left.getAllLinkAddresses() : null, + right != null ? right.getAllLinkAddresses() : null); + } + /** * Compares {@code left} {@code LinkProperties} allLinkAddresses against the {@code right}. * diff --git a/tests/unit/java/com/android/server/connectivityservice/CSDestroySocketTest.kt b/tests/unit/java/com/android/server/connectivityservice/CSDestroySocketTest.kt index 56d934bf51..ab666d2c33 100644 --- a/tests/unit/java/com/android/server/connectivityservice/CSDestroySocketTest.kt +++ b/tests/unit/java/com/android/server/connectivityservice/CSDestroySocketTest.kt @@ -24,13 +24,28 @@ import android.net.ConnectivityManager.BLOCKED_REASON_NONE import android.net.ConnectivityManager.FIREWALL_CHAIN_BACKGROUND import android.net.ConnectivityManager.FIREWALL_RULE_ALLOW import android.net.ConnectivityManager.FIREWALL_RULE_DENY +import android.net.LinkAddress import android.net.LinkProperties -import android.net.NetworkCapabilities +import android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET +import android.net.NetworkCapabilities.TRANSPORT_BLUETOOTH +import android.net.NetworkCapabilities.TRANSPORT_CELLULAR +import android.net.NetworkCapabilities.TRANSPORT_ETHERNET +import android.net.NetworkCapabilities.TRANSPORT_WIFI +import android.net.NetworkRequest import android.os.Build import com.android.net.module.util.BaseNetdUnsolicitedEventListener import com.android.server.connectivity.ConnectivityFlags.DELAY_DESTROY_SOCKETS import com.android.testutils.DevSdkIgnoreRule import com.android.testutils.DevSdkIgnoreRunner +import com.android.testutils.TestableNetworkAgent.Event.OnNetworkDestroyed +import com.android.testutils.TestableNetworkCallback +import com.android.testutils.TestableNetworkCallback.Event.LinkPropertiesChanged +import java.net.Inet6Address +import java.net.InetAddress +import junit.framework.Assert.assertFalse +import junit.framework.Assert.assertTrue +import kotlin.test.assertEquals +import kotlin.test.assertNotNull import org.junit.Test import org.junit.runner.RunWith import org.mockito.ArgumentCaptor @@ -45,17 +60,21 @@ private const val TEST_UID = 1234 private const val TEST_UID2 = 5678 private const val TEST_CELL_IFACE = "test_rmnet" -private fun cellNc() = NetworkCapabilities.Builder() - .addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR) - .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) - .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED) - .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VCN_MANAGED) - .build() +private fun cellNc() = makeNc(TRANSPORT_CELLULAR) -private fun cellLp() = LinkProperties().also{ - it.interfaceName = TEST_CELL_IFACE +private fun makeNc(transportType: Int) = nc(transportType, NET_CAPABILITY_INTERNET) + +private fun cellLp() = makeLp(TEST_CELL_IFACE) + +private fun makeLp(interfaceName: String) = LinkProperties().also{ + it.interfaceName = interfaceName } +private fun makeRequest(transportType: Int) = NetworkRequest.Builder() + .clearCapabilities() + .addTransportType(transportType) + .build() + @RunWith(DevSdkIgnoreRunner::class) @DevSdkIgnoreRule.IgnoreUpTo(Build.VERSION_CODES.UPSIDE_DOWN_CAKE) class CSDestroySocketTest : CSTest() { @@ -336,4 +355,114 @@ class CSDestroySocketTest : CSTest() { expectDestroySockets = false ) } + + private fun InetAddress.toLinkAddress() = + LinkAddress(this, if (this is Inet6Address) 64 else 24) + + private fun prepareNetworkAgent( + interfaceName: String, + addresses: List, + transportType: Int + ): Pair { + val callback = TestableNetworkCallback() + cm.registerNetworkCallback(makeRequest(transportType), callback) + val linkProperties = makeLp(interfaceName) + for (address in addresses) { + linkProperties.addLinkAddress(address) + } + val agent = Agent(nc = makeNc(transportType), lp = linkProperties) + agent.connect() + return agent to callback + } + + @Test + fun testIpToNetworksMap() { + val addressV6_1 = InetAddress.getByName("2001:DB8:0100::1111") + val addressV6_2 = InetAddress.getByName("2001:DB8:0200::2222") + val addressV6_3 = InetAddress.getByName("2001:DB8:0333::3333") + val addressV6LinkLocal = InetAddress.getByName("FE80::1234") + val addressV4_1 = InetAddress.getByName("192.0.2.10") + val addressV4_2 = InetAddress.getByName("192.0.2.11") + // Creates 3 NetworkAgent with various IP addresses(some are used for only one network + // agent some are used for multiple network agents.) + + val (cellAgent, cellCallback) = prepareNetworkAgent( + "rmnet1", + arrayListOf(addressV6_1.toLinkAddress(), addressV4_1.toLinkAddress()), + TRANSPORT_CELLULAR + ) + + val (wlanAgent, wlanCallback) = prepareNetworkAgent( + "wlan1", + arrayListOf(addressV6_2.toLinkAddress(), addressV4_1.toLinkAddress()), + TRANSPORT_WIFI + ) + val (ethAgent, ethCallback) = prepareNetworkAgent( + "eth2", + arrayListOf(addressV6_2.toLinkAddress(), addressV4_2.toLinkAddress()), + TRANSPORT_ETHERNET + ) + val ipToNetworksMap = assertNotNull(service.mIpToNetworksMap) + fun InetAddress.getNetworks() = ipToNetworksMap[this]!!.map{ it.network }.toSet() + + // Verify mIpToNetworksMap properly stores network agents based on IP addresses. + assertEquals(setOf(cellAgent.network), addressV6_1.getNetworks()) + assertEquals( + setOf(wlanAgent.network, ethAgent.network), + addressV6_2.getNetworks() + ) + assertEquals( + setOf(wlanAgent.network, cellAgent.network), + addressV4_1.getNetworks() + ) + assertEquals(setOf(ethAgent.network), addressV4_2.getNetworks()) + + // Disconnect network agent#2 & #3. + wlanAgent.disconnect() + ethAgent.disconnect() + // Update only interface name without IP address change for network agent #1. + val linkProperties = makeLp("ipsec1") + linkProperties.addLinkAddress(addressV4_1.toLinkAddress()) + linkProperties.addLinkAddress(addressV6_1.toLinkAddress()) + cellAgent.sendLinkProperties(linkProperties) + cellCallback.eventuallyExpect { + it.network == cellAgent.network && it.lp.interfaceName == "ipsec1" + } + + // Verify mIpToNetworksMap for the LinkPropertiesChanged + assertFalse(ipToNetworksMap.containsKey(addressV6_2)) + assertFalse(ipToNetworksMap.containsKey(addressV4_2)) + assertEquals(setOf(cellAgent.network), addressV6_1.getNetworks()) + assertEquals(setOf(cellAgent.network), addressV4_1.getNetworks()) + + // Change IP addresses for network agent #1. + val linkProperties2 = makeLp("rmnet1") + linkProperties2.addLinkAddress(addressV4_2.toLinkAddress()) + linkProperties2.addLinkAddress(addressV6_1.toLinkAddress()) + linkProperties2.addLinkAddress(addressV6_3.toLinkAddress()) + cellAgent.sendLinkProperties(linkProperties2) + // Add network agent#4 + val (agent4, callback4) = prepareNetworkAgent( + "bt1", + arrayListOf(addressV6LinkLocal.toLinkAddress(), addressV4_2.toLinkAddress()), + TRANSPORT_BLUETOOTH + ) + assertEquals(3, ipToNetworksMap.size) + assertEquals(setOf(cellAgent.network), addressV6_1.getNetworks()) + assertEquals( + setOf(cellAgent.network, agent4.network), + addressV4_2.getNetworks() + ) + assertEquals(setOf(cellAgent.network), addressV6_3.getNetworks()) + assertFalse(ipToNetworksMap.containsKey(addressV4_1)) + // This map doesn't handle IPv6 link local addresses. + assertFalse(ipToNetworksMap.containsKey(addressV6LinkLocal)) + + // Disconnect all remaining network agents. + cellAgent.disconnect() + agent4.disconnect() + agent4.eventuallyExpect() + // Verify mIpToNetworksMap is empty. + assertTrue(ipToNetworksMap.isEmpty()) + } } diff --git a/tests/unit/java/com/android/server/connectivityservice/base/CSAgentWrapper.kt b/tests/unit/java/com/android/server/connectivityservice/base/CSAgentWrapper.kt index 4dd6d82e08..990ef47392 100644 --- a/tests/unit/java/com/android/server/connectivityservice/base/CSAgentWrapper.kt +++ b/tests/unit/java/com/android/server/connectivityservice/base/CSAgentWrapper.kt @@ -22,6 +22,7 @@ import android.net.INetworkMonitor import android.net.INetworkMonitor.NETWORK_VALIDATION_PROBE_DNS import android.net.INetworkMonitor.NETWORK_VALIDATION_PROBE_HTTP import android.net.INetworkMonitorCallbacks +import android.net.KeepalivePacketData import android.net.LinkProperties import android.net.LocalNetworkConfig import android.net.Network @@ -35,13 +36,36 @@ import android.net.NetworkProvider import android.net.NetworkRequest import android.net.NetworkScore import android.net.NetworkTestResultParcelable +import android.net.QosFilter +import android.net.Uri import android.net.networkstack.NetworkStackClientBase import android.os.HandlerThread +import android.os.Looper +import com.android.net.module.util.ArrayTrackRecord +import com.android.testutils.TestableNetworkAgent.Event +import com.android.testutils.TestableNetworkAgent.Event.OnAddKeepalivePacketFilter +import com.android.testutils.TestableNetworkAgent.Event.OnAutomaticReconnectDisabled +import com.android.testutils.TestableNetworkAgent.Event.OnBandwidthUpdateRequested +import com.android.testutils.TestableNetworkAgent.Event.OnDscpPolicyStatusUpdated +import com.android.testutils.TestableNetworkAgent.Event.OnNetworkCreated +import com.android.testutils.TestableNetworkAgent.Event.OnNetworkDestroyed +import com.android.testutils.TestableNetworkAgent.Event.OnNetworkUnwanted +import com.android.testutils.TestableNetworkAgent.Event.OnRegisterQosCallback +import com.android.testutils.TestableNetworkAgent.Event.OnRemoveKeepalivePacketFilter +import com.android.testutils.TestableNetworkAgent.Event.OnSaveAcceptUnvalidated +import com.android.testutils.TestableNetworkAgent.Event.OnSignalStrengthThresholdsUpdated +import com.android.testutils.TestableNetworkAgent.Event.OnStartSocketKeepalive +import com.android.testutils.TestableNetworkAgent.Event.OnStopSocketKeepalive +import com.android.testutils.TestableNetworkAgent.Event.OnUnregisterQosCallback +import com.android.testutils.TestableNetworkAgent.Event.OnValidationStatus import com.android.testutils.TestableNetworkCallback import com.android.testutils.TestableNetworkCallback.Event.Available import com.android.testutils.TestableNetworkCallback.Event.Lost +import java.time.Duration import java.util.concurrent.atomic.AtomicInteger import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertTrue import kotlin.test.fail import org.mockito.ArgumentCaptor import org.mockito.ArgumentMatchers.any @@ -87,9 +111,104 @@ class CSAgentWrapper( private var nmValidationResult = NO_PROBE_RESULT private var nmProbesCompleted = NO_PROBE_RESULT private var nmProbesSucceeded = NO_PROBE_RESULT + val history = ArrayTrackRecord().newReadHead() + val DEFAULT_TIMEOUT_MS = 5000L override val network: Network get() = agent.network!! + inner class TestAgent : NetworkAgent { + constructor( + context: Context, + looper: Looper, + tag: String, + nc: NetworkCapabilities, + lp: LinkProperties, + lnc: LocalNetworkConfig?, + score: NetworkScore, + nac: NetworkAgentConfig, + provider: NetworkProvider? + ) : + super(context, looper, tag, nc, lp, lnc, score, nac, provider) { + } + + constructor( + context: Context, + looper: Looper, + tag: String, + nc: NetworkCapabilities, + lp: LinkProperties, + score: Int, + nac: NetworkAgentConfig, + provider: NetworkProvider? + ) : + super(context, looper, tag, nc, lp, score, nac, provider) { + } + + override fun onBandwidthUpdateRequested() { + history.add(OnBandwidthUpdateRequested) + } + + override fun onNetworkUnwanted() { + history.add(OnNetworkUnwanted) + } + + override fun onAddKeepalivePacketFilter(slot: Int, packet: KeepalivePacketData) { + history.add(OnAddKeepalivePacketFilter(slot, packet)) + } + + override fun onRemoveKeepalivePacketFilter(slot: Int) { + history.add(OnRemoveKeepalivePacketFilter(slot)) + } + + override fun onStartSocketKeepalive( + slot: Int, + interval: Duration, + packet: KeepalivePacketData + ) { + history.add(OnStartSocketKeepalive(slot, interval.seconds.toInt(), packet)) + } + + override fun onStopSocketKeepalive(slot: Int) { + history.add(OnStopSocketKeepalive(slot)) + } + + override fun onSaveAcceptUnvalidated(accept: Boolean) { + history.add(OnSaveAcceptUnvalidated(accept)) + } + + override fun onAutomaticReconnectDisabled() { + history.add(OnAutomaticReconnectDisabled) + } + + override fun onSignalStrengthThresholdsUpdated(thresholds: IntArray) { + history.add(OnSignalStrengthThresholdsUpdated(thresholds)) + } + + override fun onQosCallbackRegistered(qosCallbackId: Int, filter: QosFilter) { + history.add(OnRegisterQosCallback(qosCallbackId, filter)) + } + + override fun onQosCallbackUnregistered(qosCallbackId: Int) { + history.add(OnUnregisterQosCallback(qosCallbackId)) + } + + override fun onValidationStatus(status: Int, uri: Uri?) { + history.add(OnValidationStatus(status, uri)) + } + + override fun onNetworkCreated() { + history.add(OnNetworkCreated) + } + + override fun onNetworkDestroyed() { + history.add(OnNetworkDestroyed) + } + + override fun onDscpPolicyStatusUpdated(policyId: Int, status: Int) { + history.add(OnDscpPolicyStatusUpdated(policyId, status)) + } + } + init { // Capture network monitor callbacks and simulate network monitor val validateAnswer = Answer { @@ -108,12 +227,13 @@ class CSAgentWrapper( ) // Create the actual agent. NetworkAgent is abstract, so make an anonymous subclass. - if (deps.isAtLeastS()) { - agent = object : NetworkAgent(context, csHandlerThread.looper, TAG, - nc, lp, lnc?.value, score.value, nac, provider) {} + agent = if (deps.isAtLeastS()) { + TestAgent( + context, csHandlerThread.looper, TAG, nc, lp, lnc?.value, + score.value, nac, provider) } else { - agent = object : NetworkAgent(context, csHandlerThread.looper, TAG, - nc, lp, 50 /* score */, nac, provider) {} + TestAgent( + context, csHandlerThread.looper, TAG, nc, lp, 50 /* score */, nac, provider) } agent.register() assertEquals(agent.network!!.netId, nmNetworkCaptor.value.netId) @@ -225,4 +345,16 @@ class CSAgentWrapper( probesSucceeded = NO_PROBE_RESULT ) } + + inline fun expect(test: (T) -> Boolean = { true }): T { + val foundCallback = history.poll(DEFAULT_TIMEOUT_MS) + assertTrue(foundCallback is T, "Expected ${T::class} but found $foundCallback") + assertTrue(test(foundCallback), "Unexpected callback : $foundCallback") + return foundCallback + } + + inline fun eventuallyExpect() = + history.poll(DEFAULT_TIMEOUT_MS) { it is T }.also { + assertNotNull(it, "Callback ${T::class} not received") + } as T } diff --git a/tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt b/tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt index 0a915df77c..0f3e70c00b 100644 --- a/tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt +++ b/tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt @@ -427,6 +427,8 @@ open class CSTest { networkForNetId: SparseArray, handler: Handler ): QuicConnectionCloser = quicConnectionCloser + + override fun flagConnectivityServiceDestroySocket() = true } inner class PermDeps : PermissionMonitor.Dependencies() {