From fe1f5cbf32750bd2bd7f4a782816f3d3a2314dfd Mon Sep 17 00:00:00 2001 From: Sewook Seo Date: Thu, 21 Nov 2024 01:11:04 +0000 Subject: [PATCH] Destroy sockets with nlmsg RTM_DELADDR Destroy sockets that matched to netId less than MIN_NET_ID for the non link local address in RTM_DELADDR. Destroy sockets on link local address with the corresponding interface ID. Bug: 373280981 Bug: 331139818 Flag: android.net.platform.flags.connectivity_service_destroy_socket Test: atest ConnectivityCoverageTests Test: device test Change-Id: I4d8127fc693f8b32f815c5cddbae0b64deeb23e5 --- .../android/server/ConnectivityService.java | 93 ++++++++++++++- .../CSDestroySocketTest.kt | 106 ++++++++++++++---- .../server/connectivityservice/base/CSTest.kt | 30 ++++- 3 files changed, 208 insertions(+), 21 deletions(-) diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 8d2c06e43a..98c8368168 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -313,6 +313,7 @@ import android.stats.connectivity.RequestType; import android.stats.connectivity.ValidatedState; import android.sysprop.NetworkProperties; import android.system.ErrnoException; +import android.system.OsConstants; import android.telephony.SubscriptionManager; import android.telephony.TelephonyManager; import android.text.TextUtils; @@ -358,8 +359,14 @@ import com.android.net.module.util.LocationPermissionChecker; import com.android.net.module.util.PerUidCounter; import com.android.net.module.util.PermissionUtils; import com.android.net.module.util.RoutingCoordinatorService; +import com.android.net.module.util.SharedLog; import com.android.net.module.util.TcUtils; +import com.android.net.module.util.ip.NetlinkMonitor; import com.android.net.module.util.netlink.InetDiagMessage; +import com.android.net.module.util.netlink.NetlinkConstants; +import com.android.net.module.util.netlink.NetlinkMessage; +import com.android.net.module.util.netlink.RtNetlinkAddressMessage; +import com.android.net.module.util.netlink.StructIfaddrMsg; import com.android.networkstack.apishim.BroadcastOptionsShimImpl; import com.android.networkstack.apishim.ConstantsShim; import com.android.networkstack.apishim.common.BroadcastOptionsShim; @@ -1138,6 +1145,8 @@ public class ConnectivityService extends IConnectivityManager.Stub // A map from IP address to networks. Only the handler thread is allowed to access this field. @Nullable @VisibleForTesting final Map> mIpToNetworksMap; + // NetlinkMonitor for ConnectivityService + @Nullable private final AddressUpdateMonitor mAddressUpdateMonitor; /** * Implements support for the legacy "one network per network type" model. @@ -1484,6 +1493,28 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + /** + * Simple NetlinkMonitor. Listen for address changed events from kernel. + * All methods except the constructor must be called on the handler thread. + */ + public static class AddressUpdateMonitor extends NetlinkMonitor { + private final Consumer mNetlinkMessageConsumer; + + AddressUpdateMonitor(Handler h, SharedLog log, String tag, + Consumer netlinkMessageConsumer) { + super(h, log, tag, OsConstants.NETLINK_ROUTE, + (NetlinkConstants.RTMGRP_IPV4_IFADDR + | NetlinkConstants.RTMGRP_IPV6_IFADDR)); + mNetlinkMessageConsumer = netlinkMessageConsumer; + } + + @Override + protected void processNetlinkMessage( + @NonNull NetlinkMessage nlMsg, long whenMs) { + mNetlinkMessageConsumer.accept(nlMsg); + } + } + /** * Dependencies of ConnectivityService, for injection in tests. */ @@ -1859,6 +1890,18 @@ public class ConnectivityService extends IConnectivityManager.Stub InetDiagMessage.destroyLiveTcpSocketsByLocalAddress(address, netIdRange, uidRanges); } + /** + * Destroy live tcp sockets for IP addresses with conditions. + * + * @param address a local address to destroy sockets + * @param interfaceId interface ID + */ + public void destroyLiveTcpSocketsByLocalAddress( + @NonNull InetAddress address, int interfaceId) + throws SocketException, InterruptedIOException, ErrnoException { + InetDiagMessage.destroyLiveTcpSocketsByLocalAddress(address, interfaceId); + } + /** * Schedule the evaluation timeout. * @@ -1888,6 +1931,15 @@ public class ConnectivityService extends IConnectivityManager.Stub public boolean flagConnectivityServiceDestroySocket() { return Flags.connectivityServiceDestroySocket(); } + + /** + * Create a AddressUpdateMonitor instance. + */ + public AddressUpdateMonitor makeAddressUpdateMonitor( + @NonNull Handler h, @NonNull SharedLog log, @NonNull String tag, + @NonNull Consumer consumer) { + return new AddressUpdateMonitor(h, log, tag, consumer); + } } public ConnectivityService(Context context) { @@ -2024,8 +2076,12 @@ public class ConnectivityService extends IConnectivityManager.Stub if (mDeps.flagConnectivityServiceDestroySocket()) { mIpToNetworksMap = new HashMap<>(); + mAddressUpdateMonitor = mDeps.makeAddressUpdateMonitor( + mHandler, new SharedLog(20, TAG), TAG, + this::processNetlinkAddressUpdateMessage); } else { mIpToNetworksMap = null; + mAddressUpdateMonitor = null; } // To ensure uid state is synchronized with Network Policy, register for // NetworkPolicyManagerService events must happen prior to NetworkPolicyManagerService @@ -4231,6 +4287,9 @@ public class ConnectivityService extends IConnectivityManager.Stub mBroadcastReceiveHelper.callOnUserAddedForExistingUsers(); permissionMonitorInitializeDone.open(); }); + if (mAddressUpdateMonitor != null) { + mHandler.post(() -> mAddressUpdateMonitor.start()); + } mProxyTracker.loadGlobalProxy(); registerDnsResolverUnsolicitedEventListener(); @@ -9993,7 +10052,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } try { mDeps.destroyLiveTcpSocketsByLocalAddress(address, netIdRangeSet, uidRangeSet); - } catch (Exception e) { + } catch (SocketException | InterruptedIOException | ErrnoException e) { loge("Exception destroy TCP sockets on local address: ", e); } } @@ -15478,4 +15537,36 @@ public class ConnectivityService extends IConnectivityManager.Stub mQuicConnectionCloser.unregisterQuicConnectionClosePayload(pfd); } + // Currently this only handles message type RTM_DELADDR to destroy sockets for the deleted + // address, we could make this a dispatcher method if we have more use cases in the future. + private void processNetlinkAddressUpdateMessage(NetlinkMessage nlMsg) { + if (!(nlMsg instanceof RtNetlinkAddressMessage msg)) { + return; + } + if (msg.getHeader().nlmsg_type != NetlinkConstants.RTM_DELADDR) { + return; + } + + final StructIfaddrMsg ifaddrMsg = msg.getIfaddrHeader(); + final InetAddress removedAddress = msg.getIpAddress(); + if (removedAddress instanceof Inet6Address && removedAddress.isLinkLocalAddress()) { + try { + mDeps.destroyLiveTcpSocketsByLocalAddress(removedAddress, ifaddrMsg.index); + } catch (SocketException | InterruptedIOException | ErrnoException e) { + loge("Exception destroying TCP sockets on local address with interfaceId: ", e); + } + } else { + // Range of netId for networks that ConnectivityService isn't aware of. OEM & legacy + // local networks are included in this scope. + try { + mDeps.destroyLiveTcpSocketsByLocalAddress( + removedAddress, + Set.of(Range.create(0, MIN_NET_ID - 1)), null /* uidRanges */ + ); + } catch (SocketException | InterruptedIOException | ErrnoException e) { + loge("Exception destroy TCP sockets on local address: ", e); + } + } + + } } diff --git a/tests/unit/java/com/android/server/connectivityservice/CSDestroySocketTest.kt b/tests/unit/java/com/android/server/connectivityservice/CSDestroySocketTest.kt index 4b37d86852..ee8c521055 100644 --- a/tests/unit/java/com/android/server/connectivityservice/CSDestroySocketTest.kt +++ b/tests/unit/java/com/android/server/connectivityservice/CSDestroySocketTest.kt @@ -43,6 +43,10 @@ import android.net.VpnTransportInfo import android.os.Build import android.util.Range import com.android.net.module.util.BaseNetdUnsolicitedEventListener +import com.android.net.module.util.netlink.NetlinkConstants +import com.android.net.module.util.netlink.RtNetlinkAddressMessage +import com.android.net.module.util.netlink.StructIfaddrMsg +import com.android.net.module.util.netlink.StructNlMsgHdr import com.android.server.NetIdManager.MAX_NET_ID import com.android.server.NetIdManager.MIN_NET_ID import com.android.server.connectivity.ConnectivityFlags.DELAY_DESTROY_SOCKETS @@ -61,9 +65,12 @@ import org.junit.Test import org.junit.runner.RunWith import org.mockito.ArgumentCaptor import org.mockito.Mockito.any +import org.mockito.Mockito.anyInt import org.mockito.Mockito.doReturn import org.mockito.Mockito.inOrder import org.mockito.Mockito.never +import org.mockito.Mockito.timeout +import org.mockito.Mockito.times import org.mockito.Mockito.verify private const val TIMESTAMP = 1234L @@ -505,7 +512,6 @@ class CSDestroySocketTest : CSTest() { @Test fun testDestroySocketForRemovedIpAddressFromSingleNetwork() { - val inOrder = inOrder(destroySocketsWrapper) val addressV6_1 = InetAddress.getByName("2100::1234") val addressV6_2 = InetAddress.getByName("2100:0207::4321:1234") // add a test network. @@ -523,7 +529,7 @@ class CSDestroySocketTest : CSTest() { it.network == agent1.network && !it.lp.addresses.contains(addressV6_1) } // verify destroy sockets on the removed IP address - inOrder.verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( + verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( addressV6_1, setOf(Range.create(MIN_NET_ID, MAX_NET_ID)), null @@ -533,17 +539,16 @@ class CSDestroySocketTest : CSTest() { agent1.disconnect() agent1.eventuallyExpect() // verify destroy sockets on the removed IP address - inOrder.verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( + verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( addressV6_2, setOf(Range.create(MIN_NET_ID, MAX_NET_ID)), null ) - inOrder.verifyNoMoreInteractions() + verify(destroySocketsWrapper, never()).destroyLiveTcpSocketsByLocalAddress(any(), anyInt()) } @Test fun testDestroySocketForRemovedIpAddressFromMultipleNonVpnNetworks() { - val inOrder = inOrder(destroySocketsWrapper) val addressV6_1 = InetAddress.getByName("2100::1234") val addressV6_2 = InetAddress.getByName("2100:0207::4321:1234") val addressV6LinkLocal = InetAddress.getByName("FE80::1234") @@ -588,7 +593,7 @@ class CSDestroySocketTest : CSTest() { } // verify destroy sockets on the removed IP address and exempt netId lists. - inOrder.verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( + verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( addressV6_1, setOf( Range.create(MIN_NET_ID, wlanAgent.network.netId - 1), @@ -598,7 +603,7 @@ class CSDestroySocketTest : CSTest() { null ) // verify destroy sockets on the removed IP address - inOrder.verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( + verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( addressV4_1, setOf(Range.create(MIN_NET_ID, MAX_NET_ID)), null @@ -611,7 +616,7 @@ class CSDestroySocketTest : CSTest() { it.network == wlanAgent.network && it.lp.interfaceName == "wlan3" } // verify destroy sockets on the removed IP address and exempt netId lists. - inOrder.verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( + verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( addressV6_1, setOf( Range.create(MIN_NET_ID, ethAgent.network.netId - 1), @@ -620,7 +625,7 @@ class CSDestroySocketTest : CSTest() { null ) // verify not to destroy sockets on the link local address at NetworkAgent update. - inOrder.verify(destroySocketsWrapper, never()).destroyLiveTcpSocketsByLocalAddress( + verify(destroySocketsWrapper, never()).destroyLiveTcpSocketsByLocalAddress( addressV6LinkLocal, setOf(Range.create(MIN_NET_ID, MAX_NET_ID)), null @@ -634,21 +639,21 @@ class CSDestroySocketTest : CSTest() { ethAgent.eventuallyExpect() // verify destroy sockets on the removed IP address - inOrder.verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( + verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( addressV6_2, setOf(Range.create(MIN_NET_ID, MAX_NET_ID)), null ) - inOrder.verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( + verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( addressV6_1, setOf(Range.create(MIN_NET_ID, MAX_NET_ID)), null ) + verify(destroySocketsWrapper, never()).destroyLiveTcpSocketsByLocalAddress(any(), anyInt()) } @Test fun testDestroySocketForRemovedIpAddressFromMultipleVpnNetworks() { - val inOrder = inOrder(destroySocketsWrapper) val addressV6_1 = InetAddress.getByName("2100::1234") val addressV6_2 = InetAddress.getByName("2100:0207::4321:1234") val addressV6_3 = InetAddress.getByName("2234:0207::1234:4321") @@ -684,7 +689,7 @@ class CSDestroySocketTest : CSTest() { it.network == vpnAgent1.network && it.lp.interfaceName == "ipsec21" } // verify destroy sockets on the removed IP address(TEST_IPV6_ADDRESS1) and VPN's uid range. - inOrder.verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( + verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( addressV6_1, setOf(Range.create(MIN_NET_ID, MAX_NET_ID)), vpnAgent1.nc.uids @@ -692,7 +697,7 @@ class CSDestroySocketTest : CSTest() { // verify destroy sockets on the removed IP address(TEST_IPV4_ADDRESS1). network#1 and // network#4 use TEST_IPV4_ADDRESS1, however network#4 is not VPN network, so we destroy // sockets only based on the removed IP address. - inOrder.verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( + verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( addressV4_1, setOf(Range.create(MIN_NET_ID, MAX_NET_ID)), null @@ -706,13 +711,13 @@ class CSDestroySocketTest : CSTest() { it.lp.interfaceName == "ipsec2" && it.lp.addresses.contains(addressV6_3) } // verify destroy sockets on the removed IP address(TEST_IPV6_ADDRESS1) and VPN's uid range. - inOrder.verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( + verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( addressV6_1, setOf(Range.create(MIN_NET_ID, MAX_NET_ID)), vpnAgent2.nc.uids ) // verify not to destroy sockets on the link local address at NetworkAgent update. - inOrder.verify(destroySocketsWrapper, never()).destroyLiveTcpSocketsByLocalAddress( + verify(destroySocketsWrapper, never()).destroyLiveTcpSocketsByLocalAddress( addressV6LinkLocal, setOf(Range.create(MIN_NET_ID, MAX_NET_ID)), null @@ -726,20 +731,83 @@ class CSDestroySocketTest : CSTest() { testAgent.eventuallyExpect() // verify destroy sockets on the removed IP address. - inOrder.verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( + verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( addressV6_3, setOf(Range.create(MIN_NET_ID, MAX_NET_ID)), null ) - inOrder.verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( + verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( addressV6_2, setOf(Range.create(MIN_NET_ID, MAX_NET_ID)), null ) - inOrder.verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( + verify(destroySocketsWrapper, times(2)).destroyLiveTcpSocketsByLocalAddress( addressV4_1, setOf(Range.create(MIN_NET_ID, MAX_NET_ID)), null ) + verify(destroySocketsWrapper, never()).destroyLiveTcpSocketsByLocalAddress(any(), anyInt()) + } + + @Test + fun testDestroySocketWithNetlinkMessageForLinkLocalAddress() { + val addressV6LinkLocal = InetAddress.getByName("FE80::1234") + val testInterfaceId = 10 + val nlMsgLinkLocalAddressRemoved = RtNetlinkAddressMessage( + StructNlMsgHdr( + 30 /* payload length */, + NetlinkConstants.RTM_DELADDR, + 0 /* flages */, + 0 /* seq */ + ), + StructIfaddrMsg( + 10 /* family */, + 64 /* prefix length */, + 0 /* flags */, + 0 /* scope */, + testInterfaceId /* interfaceId */ + ), + addressV6LinkLocal, + null, + 0 /* flags */ + ) + csHandler.post{deps.netlinkMessageUpdate?.accept(nlMsgLinkLocalAddressRemoved)} + + // verify destroy a socket on link local address with corresponding interface ID. + verify(destroySocketsWrapper, timeout(100)).destroyLiveTcpSocketsByLocalAddress( + addressV6LinkLocal, + testInterfaceId + ) + } + + @Test + fun testDestroySocketWithNetlinkMessageForOemNetwork() { + val addressV4 = InetAddress.getByName("203.0.113.10") + val nlMsgAddressRemoved = RtNetlinkAddressMessage( + StructNlMsgHdr( + 30 /* payload length */, + NetlinkConstants.RTM_DELADDR, + 0 /* flags */, + 0 /* seq */ + ), + StructIfaddrMsg( + 10 /* family */, + 64 /* prefix length */, + 0 /* flags */, + 0 /* scope */, + 5 /* interfaceId */ + ), + addressV4, + null, + 0 /* flags */ + ) + csHandler.post{deps.netlinkMessageUpdate?.accept(nlMsgAddressRemoved)} + + // verify destroy non platform socket(netId < MIN_NET_ID) with the RTM_DELADDR message + verify(destroySocketsWrapper, timeout(100)).destroyLiveTcpSocketsByLocalAddress( + addressV4, + setOf(Range.create(0, MIN_NET_ID - 1)), + null /* uidRanges */ + ) } } 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 ce2869669f..6fa420638d 100644 --- a/tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt +++ b/tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt @@ -68,6 +68,8 @@ import com.android.internal.app.IBatteryStats import com.android.internal.util.test.BroadcastInterceptingContext import com.android.modules.utils.build.SdkLevel import com.android.net.module.util.ArrayTrackRecord +import com.android.net.module.util.SharedLog +import com.android.net.module.util.netlink.NetlinkMessage import com.android.networkstack.apishim.common.UnsupportedApiLevelException import com.android.server.connectivity.AutomaticOnOffKeepaliveTracker import com.android.server.connectivity.CarrierPrivilegeAuthenticator @@ -90,6 +92,7 @@ import java.util.concurrent.Executors import java.util.concurrent.LinkedBlockingQueue import java.util.concurrent.TimeUnit import java.util.function.BiConsumer +import java.util.function.Consumer import kotlin.annotation.AnnotationRetention.RUNTIME import kotlin.annotation.AnnotationTarget.FUNCTION import kotlin.test.assertNotNull @@ -297,6 +300,10 @@ open class CSTest { netIdRange: Set>?, uidRanges: Set>? ) {} + open fun destroyLiveTcpSocketsByLocalAddress( + address: InetAddress?, + interfaceId: Int + ) {} } inner class CSDeps : ConnectivityService.Dependencies() { @@ -323,7 +330,6 @@ open class CSTest { listener: BiConsumer, handler: Handler ) = if (SdkLevel.isAtLeastT()) mock() else null - var satelliteNetworkFallbackUidUpdate = BiConsumer, Set> {_, _ -> } override fun makeSatelliteAccessController( context: Context, @@ -451,6 +457,28 @@ open class CSTest { uidRanges ) } + + override fun destroyLiveTcpSocketsByLocalAddress( + address: InetAddress, + interfaceId: Int + ) { + // Call mocked destroyLiveTcpSocketsByLocalAddress so that test can verify this method + // call + destroySocketsWrapper.destroyLiveTcpSocketsByLocalAddress( + address, + interfaceId + ) + } + var netlinkMessageUpdate = Consumer {_ -> } + override fun makeAddressUpdateMonitor( + h: Handler, + log: SharedLog, + tag: String, + consumer: Consumer + ): ConnectivityService.AddressUpdateMonitor { + netlinkMessageUpdate = consumer + return ConnectivityService.AddressUpdateMonitor(h, log, tag, consumer) + } } inner class PermDeps : PermissionMonitor.Dependencies() {