From 45163857b2ec3d512a5195b16594a569f04e01e3 Mon Sep 17 00:00:00 2001 From: Sewook Seo Date: Sun, 17 Nov 2024 22:38:48 +0000 Subject: [PATCH 1/5] Add socket destroying methods to InetDiagMessage. Add socket destroying methods by address and permission to InetDiagMessage to move that functions to ConnectivityService from Netd. Bug: 373280981 Bug: 331139818 Flag: android.net.platform.flags.connectivity_service_destroy_socket Test: atest NetworkStaticLibTests Test: device test Change-Id: I3c2136df5dfd618d1c757c07bce2b4211b807237 --- staticlibs/Android.bp | 3 + .../module/util/netlink/InetDiagMessage.java | 289 +++++++++++++++--- .../util/netlink/InetDiagSocketTest.java | 201 ++++++++++++ 3 files changed, 459 insertions(+), 34 deletions(-) diff --git a/staticlibs/Android.bp b/staticlibs/Android.bp index 00f2f9b338..9702715c92 100644 --- a/staticlibs/Android.bp +++ b/staticlibs/Android.bp @@ -253,6 +253,9 @@ java_library { "net-utils-device-common-struct", "net-utils-device-common-struct-base", ], + static_libs: [ + "netd_aidl_interface-lateststable-java", + ], apex_available: [ "com.android.tethering", "//apex_available:platform", diff --git a/staticlibs/device/com/android/net/module/util/netlink/InetDiagMessage.java b/staticlibs/device/com/android/net/module/util/netlink/InetDiagMessage.java index f776f4755f..100987910f 100644 --- a/staticlibs/device/com/android/net/module/util/netlink/InetDiagMessage.java +++ b/staticlibs/device/com/android/net/module/util/netlink/InetDiagMessage.java @@ -24,9 +24,9 @@ import static android.system.OsConstants.IPPROTO_TCP; import static android.system.OsConstants.IPPROTO_UDP; import static android.system.OsConstants.NETLINK_INET_DIAG; +import static com.android.net.module.util.netlink.NetlinkConstants.SOCKDIAG_MSG_HEADER_SIZE; import static com.android.net.module.util.netlink.NetlinkConstants.SOCK_DESTROY; import static com.android.net.module.util.netlink.NetlinkConstants.SOCK_DIAG_BY_FAMILY; -import static com.android.net.module.util.netlink.NetlinkConstants.SOCKDIAG_MSG_HEADER_SIZE; import static com.android.net.module.util.netlink.NetlinkConstants.stringForAddressFamily; import static com.android.net.module.util.netlink.NetlinkConstants.stringForProtocol; import static com.android.net.module.util.netlink.NetlinkUtils.DEFAULT_RECV_BUFSIZE; @@ -37,6 +37,7 @@ import static com.android.net.module.util.netlink.NetlinkUtils.connectToKernel; import static com.android.net.module.util.netlink.StructNlMsgHdr.NLM_F_DUMP; import static com.android.net.module.util.netlink.StructNlMsgHdr.NLM_F_REQUEST; +import android.net.INetd; import android.net.util.SocketUtils; import android.os.Process; import android.os.SystemClock; @@ -48,6 +49,9 @@ import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; +import com.android.net.module.util.CollectionUtils; +import com.android.net.module.util.InetAddressUtils; + import java.io.FileDescriptor; import java.io.IOException; import java.io.InterruptedIOException; @@ -61,6 +65,7 @@ import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.util.ArrayList; import java.util.List; +import java.util.Objects; import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; @@ -76,6 +81,13 @@ import java.util.function.Predicate; public class InetDiagMessage extends NetlinkMessage { public static final String TAG = "InetDiagMessage"; private static final int TIMEOUT_MS = 500; + /** Bitmask position for 'explicitlySelected' in Fwmark.h */ + public static final int EXPLICITLY_SELECTED_BIT_SHIFT = 16; + /** Bitmask position for 'Permission' in Fwmark.h */ + public static final int PERMISSION_SHIFT = 18; + + /** FWMARK_NET_ID_MASK in Fwmark.h */ + public static final int FWMARK_NET_ID_MASK = 0xFFFF; /** * Construct an inet_diag_req_v2 message. This method will throw @@ -342,20 +354,27 @@ public class InetDiagMessage extends NetlinkMessage { FileDescriptor destroyFd, int proto, Predicate filter) throws SocketException, InterruptedIOException, ErrnoException { AtomicInteger destroyedSockets = new AtomicInteger(0); - Consumer handleNlDumpMsg = (diagMsg) -> { - if (filter.test(diagMsg)) { - try { - sendNetlinkDestroyRequest(destroyFd, proto, diagMsg.inetDiagMsg.id, - diagMsg.inetDiagMsg.idiag_family, 1 << diagMsg.inetDiagMsg.idiag_state); - destroyedSockets.getAndIncrement(); - } catch (InterruptedIOException | ErrnoException e) { - if (!(e instanceof ErrnoException - && ((ErrnoException) e).errno == ENOENT)) { - Log.e(TAG, "Failed to destroy socket: diagMsg=" + diagMsg + ", " + e); + Consumer handleNlDumpMsg = + diagMsg -> { + if (filter.test(diagMsg)) { + try { + sendNetlinkDestroyRequest( + destroyFd, + proto, + diagMsg.inetDiagMsg.id, + diagMsg.inetDiagMsg.idiag_family, + 1 << diagMsg.inetDiagMsg.idiag_state); + destroyedSockets.getAndIncrement(); + } catch (InterruptedIOException | ErrnoException e) { + if (!(e instanceof ErrnoException + && ((ErrnoException) e).errno == ENOENT)) { + Log.e( + TAG, + "Failed to destroy socket: diagMsg=" + diagMsg + ", " + e); + } + } } - } - } - }; + }; NetlinkUtils.getAndProcessNetlinkDumpMessages(dumpReq, NETLINK_INET_DIAG, InetDiagMessage.class, handleNlDumpMsg); @@ -388,6 +407,26 @@ public class InetDiagMessage extends NetlinkMessage { return false; } + private static boolean containsMarkMatchedToNetId( + InetDiagMessage msg, @NonNull Set> netIdRanges) { + final StructNlAttr attr = findInetDiagMarkAttr(msg); + if (attr == null) return false; + + final Integer fwMark = attr.getValueAsInteger(); + if (fwMark == null) return false; + return CollectionUtils.any( + netIdRanges, range -> range.contains(fwMark & FWMARK_NET_ID_MASK)); + } + + private static StructNlAttr findInetDiagMarkAttr(InetDiagMessage msg) { + for (StructNlAttr attr : msg.nlAttrs) { + if (attr.nla_type == NetlinkUtils.INET_DIAG_MARK) { + return attr; + } + } + return null; + } + private static boolean isLoopbackAddress(InetAddress addr) { if (addr.isLoopbackAddress()) return true; if (!(addr instanceof Inet6Address)) return false; @@ -415,6 +454,17 @@ public class InetDiagMessage extends NetlinkMessage { || srcAddr.equals(dstAddr); } + /** This returns the time taken to perform socket destruction including time spent in sleep. */ + private static long destroyLiveTcpSockets(Predicate filter) + throws SocketException, InterruptedIOException, ErrnoException { + final long startTimeMs = SystemClock.elapsedRealtime(); + destroySockets( + IPPROTO_TCP, + TCP_ALIVE_STATE_FILTER, + diagMsg -> filter.test(diagMsg) && !isLoopback(diagMsg) && !isAdbSocket(diagMsg)); + return SystemClock.elapsedRealtime() - startTimeMs; + } + private static void destroySockets(int proto, int states, Predicate filter) throws ErrnoException, SocketException, InterruptedIOException { FileDescriptor destroyFd = null; @@ -425,7 +475,6 @@ public class InetDiagMessage extends NetlinkMessage { for (int family : List.of(AF_INET, AF_INET6)) { byte[] req = makeNetlinkDumpRequest(proto, states, family); - try { final int destroyedSockets = processNetlinkDumpAndDestroySockets( req, destroyFd, proto, filter); @@ -456,34 +505,27 @@ public class InetDiagMessage extends NetlinkMessage { */ public static void destroyLiveTcpSockets(Set> ranges, Set exemptUids) throws SocketException, InterruptedIOException, ErrnoException { - final long startTimeMs = SystemClock.elapsedRealtime(); - destroySockets(IPPROTO_TCP, TCP_ALIVE_STATE_FILTER, - (diagMsg) -> !exemptUids.contains(diagMsg.inetDiagMsg.idiag_uid) - && containsUid(diagMsg, ranges) - && !isLoopback(diagMsg) - && !isAdbSocket(diagMsg)); - final long durationMs = SystemClock.elapsedRealtime() - startTimeMs; + final long durationMs = + destroyLiveTcpSockets( + diagMsg -> + !exemptUids.contains(diagMsg.inetDiagMsg.idiag_uid) + && containsUid(diagMsg, ranges)); Log.d(TAG, "Destroyed live tcp sockets for uids=" + ranges + " exemptUids=" + exemptUids + " in " + durationMs + "ms"); } /** - * Close tcp sockets that match the following condition - * 1. TCP status is one of TCP_ESTABLISHED, TCP_SYN_SENT, and TCP_SYN_RECV - * 2. Owner uid of socket is in the targetUids - * 3. Socket is not loopback - * 4. Socket is not adb socket + * Close tcp sockets that match the following condition 1. TCP status is one of TCP_ESTABLISHED, + * TCP_SYN_SENT, and TCP_SYN_RECV 2. Owner uid of socket is in the targetUids 3. Socket is not + * loopback 4. Socket is not adb socket * * @param ownerUids target uids to close sockets */ - public static void destroyLiveTcpSocketsByOwnerUids(Set ownerUids) + public static void destroyLiveTcpSocketsByOwnerUids(@NonNull Set ownerUids) throws SocketException, InterruptedIOException, ErrnoException { - final long startTimeMs = SystemClock.elapsedRealtime(); - destroySockets(IPPROTO_TCP, TCP_ALIVE_STATE_FILTER, - (diagMsg) -> ownerUids.contains(diagMsg.inetDiagMsg.idiag_uid) - && !isLoopback(diagMsg) - && !isAdbSocket(diagMsg)); - final long durationMs = SystemClock.elapsedRealtime() - startTimeMs; + Objects.requireNonNull(ownerUids); + final long durationMs = + destroyLiveTcpSockets(diagMsg -> ownerUids.contains(diagMsg.inetDiagMsg.idiag_uid)); Log.d(TAG, "Destroyed live tcp sockets for uids=" + ownerUids + " in " + durationMs + "ms"); } @@ -511,6 +553,185 @@ public class InetDiagMessage extends NetlinkMessage { } } + /** + * Close tcp sockets that match the following condition 1. TCP status is one of TCP_ESTABLISHED, + * TCP_SYN_SENT, and TCP_SYN_RECV 2. Local address of socket is equal to the given address 3. + * Socket is not loopback 4. Socket is not adb socket 5. Sockets satisfying given + * conditions(netId ranges, UID ranges). + * + * @param address a local address to destroy sockets + * @param netIdRanges ranges of net IDs need to be matched + * @param uidRanges ranges of UIDs to destroy sockets + */ + public static void destroyLiveTcpSocketsByLocalAddress( + @NonNull InetAddress address, + @Nullable Set> netIdRanges, + @Nullable Set> uidRanges) + throws SocketException, InterruptedIOException, ErrnoException { + final long durationMs = + destroyLiveTcpSockets( + diagMsg -> matchesLocalAddressWithNetworkAndUser( + address, netIdRanges, uidRanges, diagMsg)); + Log.d( + TAG, + "Destroyed live tcp sockets for local address " + + address + + ", netIdRanges" + + netIdRanges + + ", uidRanges:" + + uidRanges + + " in " + + durationMs + + "ms"); + } + + /** + * Close tcp sockets that match the following condition 1. TCP status is one of TCP_ESTABLISHED, + * TCP_SYN_SENT, and TCP_SYN_RECV 2. Local address of socket is equal to the given address 3. + * Socket is not loopback 4. Socket is not adb socket 5. Sockets satisfying given + * conditions(interface ID). + * + * @param address a local address to destroy sockets + * @param interfaceId interface id needs to be referred for destroying sockets. + */ + public static void destroyLiveTcpSocketsByLocalAddress( + @NonNull InetAddress address, int interfaceId) + throws SocketException, InterruptedIOException, ErrnoException { + final long durationMs = + destroyLiveTcpSockets( + diagMsg -> + matchesLocalAddressWithInterfaceId(address, interfaceId, diagMsg)); + Log.d( + TAG, + "Destroyed live tcp sockets for local address " + + address + + ", interfaceId:" + + interfaceId + + " in " + + durationMs + + "ms"); + } + + /** + * Returns whether the |diagMsg| matches the following conditions : - is for a socket with the + * passed address - is on one of the passed netIds - belongs to an app with one of the passed + * UIDs + * + * @param address local address to compare + * @param netIdRanges a range of net IDs that needs to be matched. If null, netId is not + * considered in this match result. + * @param uidRanges ranges of UIDs to destroy sockets, If null, netId is not considered in this + * match result. + * @param diagMsg {@link InetDiagMessage} retrieved from kernel. + */ + @VisibleForTesting + public static boolean matchesLocalAddressWithNetworkAndUser( + @NonNull InetAddress address, + @Nullable Set> netIdRanges, + @Nullable Set> uidRanges, + @NonNull InetDiagMessage diagMsg) { + if (!matchesLocalAddress(address, diagMsg)) { + // Socket's local address should be matched to the given address. + return false; + } + if (netIdRanges != null && !containsMarkMatchedToNetId(diagMsg, netIdRanges)) { + return false; + } + return uidRanges == null || containsUid(diagMsg, uidRanges); + } + + /** + * Returns whether the address of InetDiagMessage matches the given address. + * + * @param address local address to compare + * @param interfaceId interface id to be referred. If it's 0, interface id is not considered in + * this match result. + * @param diagMsg {@link InetDiagMessage} retrieved from kernel. + */ + @VisibleForTesting + public static boolean matchesLocalAddressWithInterfaceId( + @NonNull InetAddress address, int interfaceId, @NonNull InetDiagMessage diagMsg) { + return (interfaceId == 0 || interfaceId == diagMsg.inetDiagMsg.id.ifIndex) + && matchesLocalAddress(address, diagMsg); + } + + private static boolean matchesLocalAddress( + @NonNull InetAddress address, @NonNull InetDiagMessage diagMsg) { + final InetAddress addressDiag = diagMsg.inetDiagMsg.id.locSocketAddress.getAddress(); + return address.equals(addressDiag) + || (address instanceof Inet4Address + && addressDiag instanceof Inet6Address + && addressDiag.equals( + // TODO: Improve performance for computing this. b/418877261 + InetAddressUtils.v4MappedV6Address((Inet4Address) address))); + } + + /** + * Close tcp sockets that match the following condition 1. TCP status is one of TCP_ESTABLISHED, + * TCP_SYN_SENT, and TCP_SYN_RECV 2. Socket is not loopback 3. Socket is not adb socket 4. + * Sockets on the specified netId where: - The opening app no longer has permission to use this + * network, or: - The opening app does have permission, but did not explicitly select this + * network. + * + * @param netId a network identifier needs to be referred for destroying sockets. + * @param permission network permission, available values are INetd.PERMISSION_NETWORK or + * INetd.PERMISSION_SYSTEM. + */ + public static void destroyLiveTcpSocketsLackingPermission(int netId, int permission) + throws SocketException, InterruptedIOException, ErrnoException { + if (permission != INetd.PERMISSION_NETWORK && permission != INetd.PERMISSION_SYSTEM) { + throw new IllegalArgumentException("Invalid permission"); + } + final long durationMs = + destroyLiveTcpSockets(diagMsg -> isLackingPermission(netId, permission, diagMsg)); + Log.d( + TAG, + "Destroyed live tcp sockets for lacking permission for netId:" + + netId + + " and permission:" + + permission + + " in " + + durationMs + + "ms"); + } + + /** + * Returns whether the opening app lacking permission or not. "isLacking" because an app is not + * lacking a permission on a mismatched netId, while it's not true that it would + * "holdPermission" + * + * @param netId network ID to compare + * @param permission network permission defined in INetd.aidl + * @param diagMsg {@link InetDiagMessage} retrieved from kernel. + */ + @VisibleForTesting + public static boolean isLackingPermission( + int netId, int permission, @NonNull InetDiagMessage diagMsg) { + final int netdPermission = convertToNetdPermission(permission); + StructNlAttr attr = findInetDiagMarkAttr(diagMsg); + if (attr == null) return false; + final Integer fwMark = attr.getValueAsInteger(); + if (fwMark == null) return false; + if ((fwMark & FWMARK_NET_ID_MASK) != netId) return false; + // Makes a mask to find matching sockets where: + // 1. The opening app no longer has permission to use this network, or: + // 2. The opening app does have permission, but did not explicitly select this network. + final int mask = + netdPermission << PERMISSION_SHIFT | 1 << EXPLICITLY_SELECTED_BIT_SHIFT; + return (fwMark & mask) != mask; + } + + private static int convertToNetdPermission(int permission) { + // CS will destroy sockets lacking permission instead of Netd, and it follows permission + // value defined in INetd. However socket's Fwmark value follows Permission.h, thus we need + // conversion. + return switch (permission) { + case INetd.PERMISSION_NETWORK -> INetd.PERMISSION_NETWORK; + case INetd.PERMISSION_SYSTEM -> INetd.PERMISSION_SYSTEM | INetd.PERMISSION_NETWORK; + default -> INetd.PERMISSION_NONE; + }; + } + @Override public String toString() { return "InetDiagMessage{ " diff --git a/staticlibs/tests/unit/src/com/android/net/module/util/netlink/InetDiagSocketTest.java b/staticlibs/tests/unit/src/com/android/net/module/util/netlink/InetDiagSocketTest.java index b44e428547..2662f8132e 100644 --- a/staticlibs/tests/unit/src/com/android/net/module/util/netlink/InetDiagSocketTest.java +++ b/staticlibs/tests/unit/src/com/android/net/module/util/netlink/InetDiagSocketTest.java @@ -36,10 +36,13 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import android.annotation.NonNull; +import android.net.INetd; import android.net.InetAddresses; import android.util.ArraySet; import android.util.Range; +import androidx.annotation.Nullable; import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; @@ -56,6 +59,7 @@ import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.util.List; import java.util.Set; +import java.util.function.Predicate; @RunWith(AndroidJUnit4.class) @SmallTest @@ -800,4 +804,201 @@ public class InetDiagSocketTest { doTestIsAdbSocket(ROOT_UID, false /* expected */); doTestIsAdbSocket(appUid, false /* expected */); } + + private static final String INET_DIAG_MSG_MAPPED6 = + // struct nlmsghdr + "6C000000" // length = 108 + + "1400" // type = SOCK_DIAG_BY_FAMILY + + "0000" // flags + + "00000000" // seqno + + "f5220000" // pid + // struct inet_diag_msg + + "0a" // family = AF_INET6 + + "02" // idiag_state = 2 + + "10" // idiag_timer = 16 + + "20" // idiag_retrans = 32 + // inet_diag_sockid + + "a845" // idiag_sport = 43077 + + "01bb" // idiag_dport = 443 + + "00000000000000000000ffffc0000201" // idiag_src = ::FFFF:192.0.2.1 + + "00000000000000000000ffffc0000202" // idiag_dst = ::FFFF:192.0.2.2 + + "08000000" // idiag_if = 8 + + "6300000000000000" // idiag_cookie = 99 + + "30000000" // idiag_expires = 48 + + "40000000" // idiag_rqueue = 64 + + "50000000" // idiag_wqueue = 80 + + "39300000" // idiag_uid = 12345 + + "851a0000" // idiag_inode = 6789 + + "0500" // len = 5 + + "0800" // type = 8 + + "00000000" // data + + "0800" // len = 8 + + "0F00" // type = 15(INET_DIAG_MARK) + + "65000500" // Fwmark netId:101, explicitlySelected: true, permission: network + + "0400" // len = 4 + + "0200"; // type = 2 + + private static final String INET_DIAG_MSG_MAPPED6_OEM_NET_ID = + // struct nlmsghdr + "6C000000" // length = 108 + + "1400" // type = SOCK_DIAG_BY_FAMILY + + "0000" // flags + + "00000000" // seqno + + "f5220000" // pid + // struct inet_diag_msg + + "0a" // family = AF_INET6 + + "02" // idiag_state = 2 + + "10" // idiag_timer = 16 + + "20" // idiag_retrans = 32 + // inet_diag_sockid + + "a845" // idiag_sport = 43077 + + "01bb" // idiag_dport = 443 + + "00000000000000000000ffffc0000201" // idiag_src = ::FFFF:192.0.2.1 + + "00000000000000000000ffffc0000202" // idiag_dst = ::FFFF:192.0.2.2 + + "08000000" // idiag_if = 8 + + "6300000000000000" // idiag_cookie = 99 + + "30000000" // idiag_expires = 48 + + "40000000" // idiag_rqueue = 64 + + "50000000" // idiag_wqueue = 80 + + "39300000" // idiag_uid = 12345 + + "851a0000" // idiag_inode = 6789 + + "0500" // len = 5 + + "0800" // type = 8 + + "00000000" // data + + "0800" // len = 8 + + "0F00" // type = 15(INET_DIAG_MARK) + + "23000500" // Fwmark netId:35, explicitlySelected: true, permission: network + + "0400" // len = 4 + + "0200"; // type = 2 + + private boolean matchesDestroyCondition( + @NonNull String nlmsg, @NonNull Predicate condition) { + final byte[] nlMsgByte = HexEncoding.decode(nlmsg.toCharArray(), false); + final ByteBuffer byteBuffer = ByteBuffer.wrap(nlMsgByte); + byteBuffer.order(ByteOrder.nativeOrder()); + final NetlinkMessage msg = NetlinkMessage.parse(byteBuffer, NETLINK_INET_DIAG); + final InetDiagMessage inetDiagMsg = (InetDiagMessage) msg; + return condition.test(inetDiagMsg); + } + + private void doTestMatchingLocalAddress( + boolean expected, + @NonNull InetAddress address, + @Nullable Set> netIdRanges, + @Nullable Set> uidRanges, + @NonNull String nlmsg) { + assertEquals( + expected, + matchesDestroyCondition( + nlmsg, + diagMsg -> + InetDiagMessage.matchesLocalAddressWithNetworkAndUser( + address, netIdRanges, uidRanges, diagMsg))); + } + + private void doTestMatchingLocalAddress( + boolean expected, + @NonNull InetAddress address, + int interfaceId, + @NonNull String nlmsg) { + assertEquals( + expected, + matchesDestroyCondition( + nlmsg, + diagMsg -> + InetDiagMessage.matchesLocalAddressWithInterfaceId( + address, interfaceId, diagMsg))); + } + + private void doTestIsLackingPermission( + boolean expected, int netId, int permission, @NonNull String nlmsg) { + assertEquals( + expected, + matchesDestroyCondition( + nlmsg, + diagMsg -> + InetDiagMessage.isLackingPermission(netId, permission, diagMsg))); + } + + @Test + public void testMatchingDestroyConditionAddressOnly() { + doTestMatchingLocalAddress( + true, /* expected */ + InetAddresses.parseNumericAddress("2001:db8::3"), + null, /* netIdRange */ + null, /* uidRanges */ + INET_DIAG_MSG_HEX2); + doTestMatchingLocalAddress( + true, /* expected */ + InetAddresses.parseNumericAddress("192.0.2.1"), + Set.of(new Range<>(100, 65535)), + null, /* uidRanges */ + INET_DIAG_MSG_MAPPED6); + } + + @Test + public void testMatchingDestroyConditionForInterfaceId() { + doTestMatchingLocalAddress( + false /* expected */, + InetAddresses.parseNumericAddress("192.0.2.1"), + 7 /* interfaceId */, + INET_DIAG_MSG_MAPPED6); + doTestMatchingLocalAddress( + true /* expected */, + InetAddresses.parseNumericAddress("192.0.2.1"), + 8 /* interfaceId */, + INET_DIAG_MSG_MAPPED6); + } + + @Test + public void testMatchingDestroyConditionForNetIdRange() { + doTestMatchingLocalAddress( + false /* expected */, + InetAddresses.parseNumericAddress("192.0.2.1"), + Set.of(new Range<>(0, 99)), + null /* uidRanges */, + INET_DIAG_MSG_MAPPED6); + doTestMatchingLocalAddress( + true /* expected */, + InetAddresses.parseNumericAddress("192.0.2.1"), + Set.of(new Range<>(0, 99)), + null /* uidRanges */, + INET_DIAG_MSG_MAPPED6_OEM_NET_ID); + doTestMatchingLocalAddress( + false /* expected */, + InetAddresses.parseNumericAddress("192.0.2.1"), + Set.of(new Range<>(100, 100), new Range<>(102, 65535)), + null /* uidRanges */, + INET_DIAG_MSG_MAPPED6); + doTestMatchingLocalAddress( + true /* expected */, + InetAddresses.parseNumericAddress("192.0.2.1"), + Set.of(new Range<>(0, 20), new Range<>(100, 65535)), + null /* uidRanges */, + INET_DIAG_MSG_MAPPED6); + } + + @Test + public void testMatchingDestroyConditionForUidRanges() { + doTestMatchingLocalAddress( + true /* expected */, + InetAddresses.parseNumericAddress("2001:db8::3"), + Set.of(new Range<>(100, 65535)), + Set.of(new Range<>(12000, 12346)), + INET_DIAG_MSG_HEX2); + } + + @Test + public void testIsLackingPermission() { + int systemPermission = INetd.PERMISSION_SYSTEM; + int netPermission = INetd.PERMISSION_NETWORK; + doTestIsLackingPermission( + true /* expected */, 101 /* netId */, systemPermission, INET_DIAG_MSG_MAPPED6); + doTestIsLackingPermission( + false /* expected */, 100 /* netId */, systemPermission, INET_DIAG_MSG_MAPPED6); + doTestIsLackingPermission( + false /* expected */, 101 /* netId */, netPermission, INET_DIAG_MSG_MAPPED6); + doTestIsLackingPermission( + true /* expected */, 2693 /* netId */, systemPermission, INET_DIAG_MSG_HEX2); + } } From 720ccd2b3232bd3491070301067d1734845e1a4b Mon Sep 17 00:00:00 2001 From: Sewook Seo Date: Mon, 18 Nov 2024 12:46:12 +0000 Subject: [PATCH 2/5] 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() { From 1af98cb493b62818e396d0894a8c4a0a61a040de Mon Sep 17 00:00:00 2001 From: Sewook Seo Date: Wed, 20 Nov 2024 06:23:27 +0000 Subject: [PATCH 3/5] Destroy sockets with NetworkAgent update event Destroy platform managed sockets on the removed IP address based on NetworkAgent update. Bug: 373280981 Bug: 331139818 Flag: android.net.platform.flags.connectivity_service_destroy_socket Test: atest ConnectivityCoverageTests Test: device test Change-Id: I6a1d2d0c25c130c2747d66bee203b0408e2fa0da --- .../android/server/ConnectivityService.java | 79 ++++- .../connectivity/IntegerRangeUtils.java | 61 ++++ .../server/ConnectivityServiceTest.java | 3 +- .../connectivity/IntegerRangeUtilsTest.java | 94 ++++++ .../CSDestroySocketTest.kt | 299 +++++++++++++++++- .../server/connectivityservice/base/CSTest.kt | 22 ++ 6 files changed, 542 insertions(+), 16 deletions(-) create mode 100644 service/src/com/android/server/connectivity/IntegerRangeUtils.java create mode 100644 tests/unit/java/com/android/server/connectivity/IntegerRangeUtilsTest.java diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 2f28265334..8d2c06e43a 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -156,6 +156,8 @@ import static com.android.net.module.util.PermissionUtils.enforceNetworkStackPer import static com.android.net.module.util.PermissionUtils.enforceNetworkStackPermissionOr; import static com.android.net.module.util.PermissionUtils.hasAnyPermissionOf; import static com.android.server.ConnectivityStatsLog.CONNECTIVITY_STATE_SAMPLE; +import static com.android.server.NetIdManager.MAX_NET_ID; +import static com.android.server.NetIdManager.MIN_NET_ID; import static com.android.server.connectivity.ConnectivityFlags.CELLULAR_DATA_INACTIVITY_TIMEOUT; import static com.android.server.connectivity.ConnectivityFlags.CLOSE_QUIC_CONNECTION; import static com.android.server.connectivity.ConnectivityFlags.DELAY_DESTROY_SOCKETS; @@ -375,6 +377,7 @@ import com.android.server.connectivity.DnsManager; import com.android.server.connectivity.DnsManager.PrivateDnsValidationUpdate; import com.android.server.connectivity.DscpPolicyTracker; import com.android.server.connectivity.FullScore; +import com.android.server.connectivity.IntegerRangeUtils; import com.android.server.connectivity.InterfaceTracker; import com.android.server.connectivity.InvalidTagException; import com.android.server.connectivity.KeepaliveResourceUtil; @@ -1841,6 +1844,21 @@ public class ConnectivityService extends IConnectivityManager.Stub InetDiagMessage.destroyLiveTcpSocketsByOwnerUids(ownerUids); } + /** + * Destroy live tcp sockets for IP addresses with conditions. + * + * @param address a local address to destroy sockets + * @param netIdRange range of net IDs to destroy sockets + * @param uidRanges ranges of UIDs to destroy sockets + */ + public void destroyLiveTcpSocketsByLocalAddress( + @NonNull InetAddress address, + @Nullable Set> netIdRange, + @Nullable Set> uidRanges) + throws SocketException, InterruptedIOException, ErrnoException { + InetDiagMessage.destroyLiveTcpSocketsByLocalAddress(address, netIdRange, uidRanges); + } + /** * Schedule the evaluation timeout. * @@ -5763,7 +5781,6 @@ 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 @@ -5874,6 +5891,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } catch (RemoteException | ServiceSpecificException e) { loge("Exception destroying network(networkDestroy): " + e); } + updateIpAddressesAndDestroySockets(nai, nai.linkProperties, null); try { mDnsResolver.destroyNetworkCache(nai.network.getNetId()); } catch (RemoteException | ServiceSpecificException e) { @@ -9882,14 +9900,14 @@ public class ConnectivityService extends IConnectivityManager.Stub } networkAgent.networkMonitor().notifyLinkPropertiesChanged( new LinkProperties(newLp, true /* parcelSensitiveFields */)); - updateIpAddressesForNetwork(networkAgent, oldLp, newLp); + updateIpAddressesAndDestroySockets(networkAgent, oldLp, newLp); notifyNetworkCallbacks(networkAgent, CALLBACK_IP_CHANGED); } mKeepaliveTracker.handleCheckKeepalivesStillValid(networkAgent); } - private void updateIpAddressesForNetwork( + private void updateIpAddressesAndDestroySockets( NetworkAgentInfo nai, LinkProperties oldLp, LinkProperties newLp) { ensureRunningOnConnectivityServiceThread(); if (mIpToNetworksMap == null) { @@ -9914,7 +9932,12 @@ public class ConnectivityService extends IConnectivityManager.Stub if (!result.removed.isEmpty()) { for (LinkAddress la : result.removed) { + if (la.getAddress() instanceof Inet6Address + && la.getAddress().isLinkLocalAddress()) { + continue; + } final Set networks = mIpToNetworksMap.get(la.getAddress()); + destroySocketsForRemovedAddress(la.getAddress(), nai, networks); if (networks != null) { networks.remove(nai); if (networks.isEmpty()) { @@ -9925,6 +9948,56 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + private void destroySocketsForRemovedAddress( + InetAddress address, + @NonNull NetworkAgentInfo targetNai, + @NonNull Set networks) { + // Logic to destroy sockets(MIN_NET_ID ~ MAX_NET_ID) on networks managed by this service + // when IP addresses are removed. Sockets(0 ~ MIN_NET_ID - 1) are for OEM network or legacy + // local network. Those are not tracked by this service, thus will be destroyed at the + // RTM_DELADDR event handling. + final Set> netIdRangeSet; + final Set> uidRangeSet; + if (networks.size() == 1) { + // Removed IP address was in single network. + netIdRangeSet = Set.of(Range.create(MIN_NET_ID, MAX_NET_ID)); + uidRangeSet = null; + } else if (CollectionUtils.all(networks, nai -> !nai.isVPN())) { + // If all networks are not VPNs, other networks with the same IP address can be exempted + // from this socket destruction. Since there may be remaining socket's which has Fwmark + // matched to previously disconnected VPN's netId, It's intended destroying sockets not + // matched to exempt networks rather than sockets matched to the target network. + final List exemptNetIds = new ArrayList<>(); + for (NetworkAgentInfo nai : networks) { + if (nai.network.netId != targetNai.network.netId) { + exemptNetIds.add(nai.network.netId); + } + } + netIdRangeSet = IntegerRangeUtils.rangeWithoutValues( + Range.create(MIN_NET_ID, MAX_NET_ID), exemptNetIds); + uidRangeSet = null; + } else if (CollectionUtils.all(networks, NetworkAgentInfo::isVPN)) { + // Since the UID ranges of VPN networks cannot overlap with each other, if all networks + // with the corresponding address were VPNs, sockets included in the UID range of the + // target network are selectively removed. + netIdRangeSet = Set.of(Range.create(MIN_NET_ID, MAX_NET_ID)); + uidRangeSet = targetNai.networkCapabilities.getUids(); + } else { + // If there is at least one VPN network among multiple networks with the same address, + // there may be sockets with Fwmark value which does not match the netId (e.g. Socket + // connections on non-bypassable VPN or socket connections fallen out of split tunnel + // for the bypassable VPN), and it is currently too complicated to distinguish them. + // Thus sockets in whole netId range will be destroyed based on local IP address. + netIdRangeSet = Set.of(Range.create(MIN_NET_ID, MAX_NET_ID)); + uidRangeSet = null; + } + try { + mDeps.destroyLiveTcpSocketsByLocalAddress(address, netIdRangeSet, uidRangeSet); + } catch (Exception e) { + loge("Exception destroy TCP sockets on local address: ", e); + } + } + private void applyInitialLinkProperties(@NonNull NetworkAgentInfo nai) { updateLinkProperties(nai, new LinkProperties(nai.linkProperties), null); } diff --git a/service/src/com/android/server/connectivity/IntegerRangeUtils.java b/service/src/com/android/server/connectivity/IntegerRangeUtils.java new file mode 100644 index 0000000000..d67db4649d --- /dev/null +++ b/service/src/com/android/server/connectivity/IntegerRangeUtils.java @@ -0,0 +1,61 @@ +/* + * Copyright (C) 2025 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.connectivity; + +import android.util.ArraySet; +import android.util.Range; + +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; + +import com.android.net.module.util.CollectionUtils; + +import java.util.Collections; +import java.util.List; +import java.util.Set; + +public final class IntegerRangeUtils { + /** + * Remove given Integer list from the source Integer range and return a set of ranges. + * @param source Integer range from which Integer list will be removed + * @param exempt Integer list to be removed from the source. + * @return a set of Integer ranges which doesn't have exempted Integer list. + */ + public static Set> rangeWithoutValues( + @NonNull Range source, @Nullable List exempt) { + final Set> rangeSet = new ArraySet<>(); + final List exemptInRange = exempt != null + ? CollectionUtils.filter(exempt, source::contains) : null; + if (exemptInRange == null || exemptInRange.isEmpty()) { + rangeSet.add(source); + return rangeSet; + } + Collections.sort(exemptInRange); + int start = source.getLower(); + for (int i = 0; i < exemptInRange.size(); i++) { + int end = exemptInRange.get(i); + if (end > start) { + rangeSet.add(Range.create(start, end - 1)); + } + start = end + 1; + } + if (start <= source.getUpper()) { + rangeSet.add(Range.create(start, source.getUpper())); + } + return rangeSet; + } +} diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index a8b95b7418..896399019f 100644 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -214,12 +214,12 @@ import static org.junit.Assume.assumeFalse; import static org.junit.Assume.assumeTrue; import static org.mockito.AdditionalMatchers.aryEq; import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isNull; -import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.Mockito.any; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.clearInvocations; @@ -405,7 +405,6 @@ import com.android.server.ConnectivityService.ConnectivityDiagnosticsCallbackInf import com.android.server.ConnectivityService.NetworkRequestInfo; import com.android.server.ConnectivityServiceTest.ConnectivityServiceDependencies.DestroySocketsWrapper; import com.android.server.ConnectivityServiceTest.ConnectivityServiceDependencies.ReportedInterfaces; -import com.android.server.L2capNetworkProvider; import com.android.server.connectivity.ApplicationSelfCertifiedNetworkCapabilities; import com.android.server.connectivity.AutomaticOnOffKeepaliveTracker; import com.android.server.connectivity.CarrierPrivilegeAuthenticator; diff --git a/tests/unit/java/com/android/server/connectivity/IntegerRangeUtilsTest.java b/tests/unit/java/com/android/server/connectivity/IntegerRangeUtilsTest.java new file mode 100644 index 0000000000..b290459446 --- /dev/null +++ b/tests/unit/java/com/android/server/connectivity/IntegerRangeUtilsTest.java @@ -0,0 +1,94 @@ +/* + * Copyright (C) 2025 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.connectivity; + +import static org.junit.Assert.assertEquals; + +import android.os.Build; +import android.util.Range; + +import com.android.testutils.DevSdkIgnoreRule; +import com.android.testutils.DevSdkIgnoreRunner; + +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.util.List; +import java.util.Set; + +/** + * Tests for IntegerRangeUtils. + */ +@RunWith(DevSdkIgnoreRunner.class) +@DevSdkIgnoreRule.IgnoreUpTo(Build.VERSION_CODES.R) +public class IntegerRangeUtilsTest { + @Test + @DevSdkIgnoreRule.IgnoreUpTo(Build.VERSION_CODES.R) + public void testRangeWithoutValues() { + final Range source1 = Range.create(100, 200); + // Basic separation after excluding exempt Integer list. + final List exempt1 = List.of(110, 170); + final Set> expected1 = Set.of( + Range.create(100, 109), + Range.create(111, 169), + Range.create(171, 200)); + // Check continuous exempt list. + final List exempt2 = List.of(110, 111, 112, 114); + final Set> expected2 = Set.of( + Range.create(100, 109), + Range.create(113, 113), + Range.create(115, 200)); + // Check handling exempted Integer on the edge of range. + final List exempt3 = List.of(100, 199, 200); + final Set> expected3 = Set.of( + Range.create(101, 198) + ); + final List exempt4 = List.of(100, 101, 102, 200); + final Set> expected4 = Set.of( + Range.create(103, 199) + ); + // Check handling exempted Integer list out of the source range. + final List exempt5 = List.of(99, 201); + final Set> expected5 = Set.of( + Range.create(100, 200) + ); + final List exempt6 = List.of(99, 150, 200, 201); + final Set> expected6 = Set.of( + Range.create(100, 149), + Range.create(151, 199) + ); + // Check handling unordered exempted Integer list. + final List exempt7 = List.of(200, 99, 201, 150, 117); + final Set> expected7 = Set.of( + Range.create(100, 116), + Range.create(118, 149), + Range.create(151, 199) + ); + assertEquals(expected1, IntegerRangeUtils.rangeWithoutValues(source1, exempt1)); + assertEquals(expected2, IntegerRangeUtils.rangeWithoutValues(source1, exempt2)); + assertEquals(expected3, IntegerRangeUtils.rangeWithoutValues(source1, exempt3)); + assertEquals(expected4, IntegerRangeUtils.rangeWithoutValues(source1, exempt4)); + assertEquals(expected5, IntegerRangeUtils.rangeWithoutValues(source1, exempt5)); + assertEquals(expected6, IntegerRangeUtils.rangeWithoutValues(source1, exempt6)); + assertEquals(expected7, IntegerRangeUtils.rangeWithoutValues(source1, exempt7)); + // Check the case that source range is included in exempted list. + final Range source2 = Range.create(100, 101); + final List exempt8 = List.of(99, 100, 101); + final Set> expected8 = Set.of(); + assertEquals(expected8, IntegerRangeUtils.rangeWithoutValues(source2, exempt8)); + } +} diff --git a/tests/unit/java/com/android/server/connectivityservice/CSDestroySocketTest.kt b/tests/unit/java/com/android/server/connectivityservice/CSDestroySocketTest.kt index ab666d2c33..4b37d86852 100644 --- a/tests/unit/java/com/android/server/connectivityservice/CSDestroySocketTest.kt +++ b/tests/unit/java/com/android/server/connectivityservice/CSDestroySocketTest.kt @@ -26,14 +26,25 @@ 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.NET_CAPABILITY_NOT_SUSPENDED +import android.net.NetworkCapabilities.NET_CAPABILITY_NOT_VCN_MANAGED +import android.net.NetworkCapabilities.NET_CAPABILITY_NOT_VPN import android.net.NetworkCapabilities.TRANSPORT_BLUETOOTH import android.net.NetworkCapabilities.TRANSPORT_CELLULAR import android.net.NetworkCapabilities.TRANSPORT_ETHERNET +import android.net.NetworkCapabilities.TRANSPORT_TEST +import android.net.NetworkCapabilities.TRANSPORT_VPN import android.net.NetworkCapabilities.TRANSPORT_WIFI import android.net.NetworkRequest +import android.net.VpnManager +import android.net.VpnTransportInfo import android.os.Build +import android.util.Range import com.android.net.module.util.BaseNetdUnsolicitedEventListener +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 import com.android.testutils.DevSdkIgnoreRule import com.android.testutils.DevSdkIgnoreRunner @@ -58,6 +69,9 @@ import org.mockito.Mockito.verify private const val TIMESTAMP = 1234L private const val TEST_UID = 1234 private const val TEST_UID2 = 5678 +private val TEST_UID_RANGE1 = Range(10000, 20000) +private val TEST_UID_RANGE2 = Range(21000, 30000) +private val TEST_UID_RANGE3 = Range(31000, 40000) private const val TEST_CELL_IFACE = "test_rmnet" private fun cellNc() = makeNc(TRANSPORT_CELLULAR) @@ -66,13 +80,36 @@ private fun makeNc(transportType: Int) = nc(transportType, NET_CAPABILITY_INTERN private fun cellLp() = makeLp(TEST_CELL_IFACE) +private fun vpnNc(uidRanges: Set>) = NetworkCapabilities.Builder() + .addTransportType(TRANSPORT_VPN) + .setTransportInfo( + VpnTransportInfo( + VpnManager.TYPE_VPN_PLATFORM, + null /* sessionId */, + false /* bypassable */, + false /* longLivedTcpConnectionsExpensive */ + ) + ) + .removeCapability(NET_CAPABILITY_NOT_VPN) + .addCapability(NET_CAPABILITY_INTERNET) + .addCapability(NET_CAPABILITY_NOT_SUSPENDED) + .addCapability(NET_CAPABILITY_NOT_VCN_MANAGED) + .setUids(uidRanges) + .build() + private fun makeLp(interfaceName: String) = LinkProperties().also{ it.interfaceName = interfaceName } -private fun makeRequest(transportType: Int) = NetworkRequest.Builder() +private fun makeRequest(nc: NetworkCapabilities) = NetworkRequest.Builder() .clearCapabilities() - .addTransportType(transportType) + .setCapabilities(nc) + .build() + +private fun makeVpnRequest(uidRanges: Set>) = NetworkRequest.Builder() + .clearCapabilities() + .addTransportType(TRANSPORT_VPN) + .setUids(uidRanges) .build() @RunWith(DevSdkIgnoreRunner::class) @@ -362,15 +399,15 @@ class CSDestroySocketTest : CSTest() { private fun prepareNetworkAgent( interfaceName: String, addresses: List, - transportType: Int + nc: NetworkCapabilities ): Pair { val callback = TestableNetworkCallback() - cm.registerNetworkCallback(makeRequest(transportType), callback) + cm.registerNetworkCallback(makeRequest(nc), callback) val linkProperties = makeLp(interfaceName) - for (address in addresses) { - linkProperties.addLinkAddress(address) + for (linkAddress in addresses) { + linkProperties.addLinkAddress(linkAddress) } - val agent = Agent(nc = makeNc(transportType), lp = linkProperties) + val agent = Agent(nc = nc, lp = linkProperties) agent.connect() return agent to callback } @@ -389,18 +426,18 @@ class CSDestroySocketTest : CSTest() { val (cellAgent, cellCallback) = prepareNetworkAgent( "rmnet1", arrayListOf(addressV6_1.toLinkAddress(), addressV4_1.toLinkAddress()), - TRANSPORT_CELLULAR + makeNc(transportType = TRANSPORT_CELLULAR) ) val (wlanAgent, wlanCallback) = prepareNetworkAgent( "wlan1", arrayListOf(addressV6_2.toLinkAddress(), addressV4_1.toLinkAddress()), - TRANSPORT_WIFI + makeNc(transportType = TRANSPORT_WIFI) ) val (ethAgent, ethCallback) = prepareNetworkAgent( "eth2", arrayListOf(addressV6_2.toLinkAddress(), addressV4_2.toLinkAddress()), - TRANSPORT_ETHERNET + makeNc(transportType = TRANSPORT_ETHERNET) ) val ipToNetworksMap = assertNotNull(service.mIpToNetworksMap) fun InetAddress.getNetworks() = ipToNetworksMap[this]!!.map{ it.network }.toSet() @@ -445,7 +482,7 @@ class CSDestroySocketTest : CSTest() { val (agent4, callback4) = prepareNetworkAgent( "bt1", arrayListOf(addressV6LinkLocal.toLinkAddress(), addressV4_2.toLinkAddress()), - TRANSPORT_BLUETOOTH + makeNc(transportType = TRANSPORT_BLUETOOTH) ) assertEquals(3, ipToNetworksMap.size) assertEquals(setOf(cellAgent.network), addressV6_1.getNetworks()) @@ -465,4 +502,244 @@ class CSDestroySocketTest : CSTest() { // Verify mIpToNetworksMap is empty. assertTrue(ipToNetworksMap.isEmpty()) } + + @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. + val (agent1, callback1) = prepareNetworkAgent( + "wlan2", + arrayListOf(addressV6_1.toLinkAddress()), + makeNc(transportType = TRANSPORT_WIFI) + ) + + // IP address changed + val linkProperties2 = makeLp("wlan2") + linkProperties2.addLinkAddress(LinkAddress(addressV6_2, 64)) + agent1.sendLinkProperties(linkProperties2) + callback1.eventuallyExpect { + it.network == agent1.network && !it.lp.addresses.contains(addressV6_1) + } + // verify destroy sockets on the removed IP address + inOrder.verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( + addressV6_1, + setOf(Range.create(MIN_NET_ID, MAX_NET_ID)), + null + ) + + // network disconnect + agent1.disconnect() + agent1.eventuallyExpect() + // verify destroy sockets on the removed IP address + inOrder.verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( + addressV6_2, + setOf(Range.create(MIN_NET_ID, MAX_NET_ID)), + null + ) + inOrder.verifyNoMoreInteractions() + } + + @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") + val addressV4_1 = InetAddress.getByName("203.0.113.10") + + // add 4 test networks(non VPN). 3 have a same IP address, and 1 network has different one. + val (cellAgent, cellCallback) = prepareNetworkAgent( + "rmnet1", + arrayListOf(addressV6_1.toLinkAddress(), addressV4_1.toLinkAddress()), + makeNc(transportType = TRANSPORT_CELLULAR) + ) + + val (wlanAgent, wlanCallback) = prepareNetworkAgent( + "wlan0", + arrayListOf(), + makeNc(transportType = TRANSPORT_WIFI) + ) + + val (testAgent, testCallback) = prepareNetworkAgent( + "test2", + arrayListOf(addressV6_2.toLinkAddress()), + makeNc(transportType = TRANSPORT_TEST) + ) + + val (ethAgent, ethCallback) = prepareNetworkAgent( + "eth1", + arrayListOf(addressV6_1.toLinkAddress()), + makeNc(transportType = TRANSPORT_ETHERNET) + ) + + // Add IP addresses on network#2 + val linkProperties = makeLp("wlan0") + linkProperties.addLinkAddress(addressV6LinkLocal.toLinkAddress()) + linkProperties.addLinkAddress(addressV6_1.toLinkAddress()) + wlanAgent.sendLinkProperties(linkProperties) + + // Remove IP addresses network#1 + val linkProperties1 = makeLp("rmnet1") + cellAgent.sendLinkProperties(linkProperties1) + cellCallback.eventuallyExpect { + it.network == cellAgent.network && it.lp.addresses.size == 0 + } + + // verify destroy sockets on the removed IP address and exempt netId lists. + inOrder.verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( + addressV6_1, + setOf( + Range.create(MIN_NET_ID, wlanAgent.network.netId - 1), + Range.create(wlanAgent.network.netId + 1, ethAgent.network.netId - 1), + Range.create(ethAgent.network.netId + 1, MAX_NET_ID) + ), + null + ) + // verify destroy sockets on the removed IP address + inOrder.verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( + addressV4_1, + setOf(Range.create(MIN_NET_ID, MAX_NET_ID)), + null + ) + + // Remove IP addresses from network#2 + val linkProperties2 = makeLp("wlan3") + wlanAgent.sendLinkProperties(linkProperties2) + wlanCallback.eventuallyExpect { + it.network == wlanAgent.network && it.lp.interfaceName == "wlan3" + } + // verify destroy sockets on the removed IP address and exempt netId lists. + inOrder.verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( + addressV6_1, + setOf( + Range.create(MIN_NET_ID, ethAgent.network.netId - 1), + Range.create(ethAgent.network.netId + 1, MAX_NET_ID) + ), + null + ) + // verify not to destroy sockets on the link local address at NetworkAgent update. + inOrder.verify(destroySocketsWrapper, never()).destroyLiveTcpSocketsByLocalAddress( + addressV6LinkLocal, + setOf(Range.create(MIN_NET_ID, MAX_NET_ID)), + null + ) + + // disconnect all test networks + cellAgent.disconnect() + wlanAgent.disconnect() + testAgent.disconnect() + ethAgent.disconnect() + ethAgent.eventuallyExpect() + + // verify destroy sockets on the removed IP address + inOrder.verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( + addressV6_2, + setOf(Range.create(MIN_NET_ID, MAX_NET_ID)), + null + ) + inOrder.verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( + addressV6_1, + setOf(Range.create(MIN_NET_ID, MAX_NET_ID)), + null + ) + } + + @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") + val addressV6LinkLocal = InetAddress.getByName("FE80::1234") + val addressV4_1 = InetAddress.getByName("203.0.113.10") + // add 3 VPN test networks and 1 non-VPN network. + + val (vpnAgent1, vpnCallback1) = prepareNetworkAgent( + "ipsec1", + arrayListOf(addressV6_1.toLinkAddress(), addressV4_1.toLinkAddress()), + vpnNc(setOf(TEST_UID_RANGE1)) + ) + + val (vpnAgent2, vpnCallback2) = prepareNetworkAgent( + "ipsec2", + arrayListOf(addressV6LinkLocal.toLinkAddress(), addressV6_1.toLinkAddress()), + vpnNc(setOf(TEST_UID_RANGE2)) + ) + val (vpnAgent3, vpnCallback3) = prepareNetworkAgent( + "ipsec3", + arrayListOf(addressV6_1.toLinkAddress()), + vpnNc(setOf(TEST_UID_RANGE3)) + ) + val (testAgent, testCallback) = prepareNetworkAgent( + "test4", + arrayListOf(addressV6_2.toLinkAddress(), addressV4_1.toLinkAddress()), + makeNc(TRANSPORT_TEST) + ) + // Remove IP addresses network#1 + val linkProperties1 = makeLp("ipsec21") + vpnAgent1.sendLinkProperties(linkProperties1) + vpnCallback1.eventuallyExpect { + 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( + addressV6_1, + setOf(Range.create(MIN_NET_ID, MAX_NET_ID)), + vpnAgent1.nc.uids + ) + // 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( + addressV4_1, + setOf(Range.create(MIN_NET_ID, MAX_NET_ID)), + null + ) + + // Change IP addresses network#2 + val linkProperties2 = makeLp("ipsec2") + linkProperties2.addLinkAddress(LinkAddress(addressV6_3, 64)) + vpnAgent2.sendLinkProperties(linkProperties2) + vpnCallback2.eventuallyExpect { it.network == vpnAgent2.network && + 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( + 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( + addressV6LinkLocal, + setOf(Range.create(MIN_NET_ID, MAX_NET_ID)), + null + ) + + // disconnect all test networks + vpnAgent1.disconnect() + vpnAgent2.disconnect() + vpnAgent3.disconnect() + testAgent.disconnect() + testAgent.eventuallyExpect() + + // verify destroy sockets on the removed IP address. + inOrder.verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( + addressV6_3, + setOf(Range.create(MIN_NET_ID, MAX_NET_ID)), + null + ) + inOrder.verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( + addressV6_2, + setOf(Range.create(MIN_NET_ID, MAX_NET_ID)), + null + ) + inOrder.verify(destroySocketsWrapper).destroyLiveTcpSocketsByLocalAddress( + addressV4_1, + setOf(Range.create(MIN_NET_ID, MAX_NET_ID)), + null + ) + } } 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 0f3e70c00b..ce2869669f 100644 --- a/tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt +++ b/tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt @@ -61,6 +61,7 @@ import android.permission.PermissionManager.PermissionResult import android.telephony.SubscriptionManager import android.telephony.TelephonyManager import android.testing.TestableContext +import android.util.Range import android.util.SparseArray import androidx.test.platform.app.InstrumentationRegistry import com.android.internal.app.IBatteryStats @@ -84,6 +85,7 @@ import com.android.server.connectivity.QuicConnectionCloser import com.android.server.connectivity.SatelliteAccessController import com.android.testutils.visibleOnHandlerThread import com.android.testutils.waitForIdle +import java.net.InetAddress import java.util.concurrent.Executors import java.util.concurrent.LinkedBlockingQueue import java.util.concurrent.TimeUnit @@ -287,8 +289,14 @@ open class CSTest { } // Class to be mocked and used to verify destroy sockets methods call + // TODO: Move to use TestableCallback-style object with a TrackRecord inside to check. open inner class DestroySocketsWrapper { open fun destroyLiveTcpSocketsByOwnerUids(ownerUids: Set) {} + open fun destroyLiveTcpSocketsByLocalAddress( + address: InetAddress, + netIdRange: Set>?, + uidRanges: Set>? + ) {} } inner class CSDeps : ConnectivityService.Dependencies() { @@ -429,6 +437,20 @@ open class CSTest { ): QuicConnectionCloser = quicConnectionCloser override fun flagConnectivityServiceDestroySocket() = true + + override fun destroyLiveTcpSocketsByLocalAddress( + address: InetAddress, + netIdRange: Set>?, + uidRanges: Set>? + ) { + // Call mocked destroyLiveTcpSocketsByLocalAddress so that test can verify this method + // call + destroySocketsWrapper.destroyLiveTcpSocketsByLocalAddress( + address, + netIdRange, + uidRanges + ) + } } inner class PermDeps : PermissionMonitor.Dependencies() { From fe1f5cbf32750bd2bd7f4a782816f3d3a2314dfd Mon Sep 17 00:00:00 2001 From: Sewook Seo Date: Thu, 21 Nov 2024 01:11:04 +0000 Subject: [PATCH 4/5] 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() { From 34ef7dbd1b3709c5db5aad0582bc01775b3b3f54 Mon Sep 17 00:00:00 2001 From: Sewook Seo Date: Thu, 21 Nov 2024 04:51:27 +0000 Subject: [PATCH 5/5] Destroy sockets lacking permission at CS. Move destroying socket logic for lacking permission to CS from netd Bug: 373280981 Bug: 331139818 Flag: android.net.platform.flags.connectivity_service_destroy_socket Test: atest ConnectivityCoverageTests Test: device test Change-Id: Id933d51d6175958a1b78443360858db577319622 --- .../android/server/ConnectivityService.java | 50 ++++++++++++ .../CSDestroySocketTest.kt | 80 ++++++++++++++++--- .../base/CSAgentWrapper.kt | 1 + .../server/connectivityservice/base/CSTest.kt | 15 ++++ 4 files changed, 135 insertions(+), 11 deletions(-) diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 98c8368168..ee58c70b9e 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -1902,6 +1902,18 @@ public class ConnectivityService extends IConnectivityManager.Stub InetDiagMessage.destroyLiveTcpSocketsByLocalAddress(address, interfaceId); } + /** + * Call {@link InetDiagMessage#destroyLiveTcpSocketsLackingPermission(int, int)} + * + * @param netId network ID + * @param permission network permission + */ + public void destroyLiveTcpSocketsLackingPermission( + int netId, int permission) + throws SocketException, InterruptedIOException, ErrnoException { + InetDiagMessage.destroyLiveTcpSocketsLackingPermission(netId, permission); + } + /** * Schedule the evaluation timeout. * @@ -5820,8 +5832,14 @@ public class ConnectivityService extends IConnectivityManager.Stub // Delayed teardown. if (nai.isCreated() && !nai.isDestroyed()) { + destroyLiveTcpSocketsLackingPermission( + nai, getNetworkPermission(nai.networkCapabilities), INetd.PERMISSION_SYSTEM); try { mNetd.networkSetPermissionForNetwork(nai.network.netId, INetd.PERMISSION_SYSTEM); + // Call again in case sockets were opened after the first time : see + // updateNetworkPermissions + destroyLiveTcpSocketsLackingPermission(nai, + getNetworkPermission(nai.networkCapabilities), INetd.PERMISSION_SYSTEM); } catch (RemoteException e) { Log.d(TAG, "Error marking network restricted during teardown: ", e); } @@ -10615,8 +10633,14 @@ public class ConnectivityService extends IConnectivityManager.Stub final int oldPermission = getNetworkPermission(nai.networkCapabilities); final int newPermission = getNetworkPermission(newNc); if (oldPermission != newPermission && nai.isCreated() && !nai.isVPN()) { + destroyLiveTcpSocketsLackingPermission(nai, oldPermission, newPermission); try { mNetd.networkSetPermissionForNetwork(nai.network.getNetId(), newPermission); + // Destroy sockets again in case any were opened after + // destroySocketsLackingPermission is called above and before the permissions is + // changed. These sockets won't be able to send any RST packets because they are now + // no longer routed, but at least the apps will get errors. + destroyLiveTcpSocketsLackingPermission(nai, oldPermission, newPermission); } catch (RemoteException | ServiceSpecificException e) { loge("Exception in networkSetPermissionForNetwork: " + e); } @@ -15567,6 +15591,32 @@ public class ConnectivityService extends IConnectivityManager.Stub loge("Exception destroy TCP sockets on local address: ", e); } } + } + private void destroyLiveTcpSocketsLackingPermission( + NetworkAgentInfo nai, int oldPermission, int newPermission) { + if (!mDeps.flagConnectivityServiceDestroySocket()) { + return; + } + // Virtual network doesn't have permission set. + if (!isPermissionMoreRestrictive(oldPermission, newPermission) || nai.isVPN()) { + return; + } + try { + mDeps.destroyLiveTcpSocketsLackingPermission( + nai.network.netId, newPermission); + } catch (SocketException | InterruptedIOException | ErrnoException e) { + loge("Exception destroy TCP sockets lacking permission: " + e); + } + } + + private boolean isPermissionMoreRestrictive(int oldPermission, int newPermission) { + return switch (newPermission) { + case INetd.PERMISSION_NONE -> false; + case INetd.PERMISSION_NETWORK -> oldPermission == INetd.PERMISSION_NONE; + case INetd.PERMISSION_SYSTEM -> oldPermission == INetd.PERMISSION_NONE + || oldPermission == INetd.PERMISSION_NETWORK; + default -> throw new IllegalArgumentException("Invalid permission"); + }; } } diff --git a/tests/unit/java/com/android/server/connectivityservice/CSDestroySocketTest.kt b/tests/unit/java/com/android/server/connectivityservice/CSDestroySocketTest.kt index ee8c521055..aeeced6579 100644 --- a/tests/unit/java/com/android/server/connectivityservice/CSDestroySocketTest.kt +++ b/tests/unit/java/com/android/server/connectivityservice/CSDestroySocketTest.kt @@ -24,6 +24,7 @@ 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.INetd import android.net.LinkAddress import android.net.LinkProperties import android.net.NetworkCapabilities @@ -54,7 +55,9 @@ 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.CapabilitiesChanged import com.android.testutils.TestableNetworkCallback.Event.LinkPropertiesChanged +import com.android.testutils.TestableNetworkCallback.Event.Lost import java.net.Inet6Address import java.net.InetAddress import junit.framework.Assert.assertFalse @@ -66,6 +69,7 @@ import org.junit.runner.RunWith import org.mockito.ArgumentCaptor import org.mockito.Mockito.any import org.mockito.Mockito.anyInt +import org.mockito.Mockito.doNothing import org.mockito.Mockito.doReturn import org.mockito.Mockito.inOrder import org.mockito.Mockito.never @@ -512,8 +516,8 @@ class CSDestroySocketTest : CSTest() { @Test fun testDestroySocketForRemovedIpAddressFromSingleNetwork() { - val addressV6_1 = InetAddress.getByName("2100::1234") - val addressV6_2 = InetAddress.getByName("2100:0207::4321:1234") + val addressV6_1 = InetAddress.getByName("2001:DB8:0100::1111") + val addressV6_2 = InetAddress.getByName("2001:DB8:0200::2222") // add a test network. val (agent1, callback1) = prepareNetworkAgent( "wlan2", @@ -523,7 +527,7 @@ class CSDestroySocketTest : CSTest() { // IP address changed val linkProperties2 = makeLp("wlan2") - linkProperties2.addLinkAddress(LinkAddress(addressV6_2, 64)) + linkProperties2.addLinkAddress(addressV6_2.toLinkAddress()) agent1.sendLinkProperties(linkProperties2) callback1.eventuallyExpect { it.network == agent1.network && !it.lp.addresses.contains(addressV6_1) @@ -549,10 +553,10 @@ class CSDestroySocketTest : CSTest() { @Test fun testDestroySocketForRemovedIpAddressFromMultipleNonVpnNetworks() { - val addressV6_1 = InetAddress.getByName("2100::1234") - val addressV6_2 = InetAddress.getByName("2100:0207::4321:1234") + val addressV6_1 = InetAddress.getByName("2001:DB8:0100::1111") + val addressV6_2 = InetAddress.getByName("2001:DB8:0200::2222") val addressV6LinkLocal = InetAddress.getByName("FE80::1234") - val addressV4_1 = InetAddress.getByName("203.0.113.10") + val addressV4_1 = InetAddress.getByName("192.0.2.10") // add 4 test networks(non VPN). 3 have a same IP address, and 1 network has different one. val (cellAgent, cellCallback) = prepareNetworkAgent( @@ -654,11 +658,11 @@ class CSDestroySocketTest : CSTest() { @Test fun testDestroySocketForRemovedIpAddressFromMultipleVpnNetworks() { - 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") + 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("203.0.113.10") + val addressV4_1 = InetAddress.getByName("192.0.2.10") // add 3 VPN test networks and 1 non-VPN network. val (vpnAgent1, vpnCallback1) = prepareNetworkAgent( @@ -705,7 +709,7 @@ class CSDestroySocketTest : CSTest() { // Change IP addresses network#2 val linkProperties2 = makeLp("ipsec2") - linkProperties2.addLinkAddress(LinkAddress(addressV6_3, 64)) + linkProperties2.addLinkAddress(addressV6_3.toLinkAddress()) vpnAgent2.sendLinkProperties(linkProperties2) vpnCallback2.eventuallyExpect { it.network == vpnAgent2.network && it.lp.interfaceName == "ipsec2" && it.lp.addresses.contains(addressV6_3) @@ -810,4 +814,58 @@ class CSDestroySocketTest : CSTest() { null /* uidRanges */ ) } + + @Test + fun testDestroySocketsLackingPermissionForPermissionUpdate() { + val inOrder = inOrder(destroySocketsWrapper) + val addressV6_1 = InetAddress.getByName("2001:DB8:0100::1111") + val addressV4_1 = InetAddress.getByName("192.0.2.10") + + doNothing().`when`(netd).networkSetPermissionForNetwork(anyInt(), anyInt()) + + val ncRestricted = + cellNc().removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED) + val ncNotRestricted = + cellNc().addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED) + + val (agent, callback) = prepareNetworkAgent( + "rmnet1", + arrayListOf(addressV6_1.toLinkAddress(), addressV4_1.toLinkAddress()), + ncRestricted + ) + + agent.sendNetworkCapabilities(ncNotRestricted) + callback.eventuallyExpect { + it.network == agent.network && + it.caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED) + } + inOrder.verify(destroySocketsWrapper, never()) + .destroyLiveTcpSocketsLackingPermission(anyInt(), anyInt()) + agent.sendNetworkCapabilities(ncRestricted) + callback.eventuallyExpect() + inOrder.verify(destroySocketsWrapper, times(2)) + .destroyLiveTcpSocketsLackingPermission( + agent.network.netId, + INetd.PERMISSION_SYSTEM + ) + } + + @Test + fun testDestroySocketsLackingPermissionForDelayedTeardown() { + val addressV6_1 = InetAddress.getByName("2001:DB8:0100::1111") + doNothing().`when`(netd).networkSetPermissionForNetwork(anyInt(), anyInt()) + val (agent, callback) = prepareNetworkAgent( + "rmnet1", + arrayListOf(addressV6_1.toLinkAddress()), + makeNc(TRANSPORT_CELLULAR) + ) + agent.sendTeardownDelayMs(1) + agent.disconnect() + agent.eventuallyExpect() + + verify(destroySocketsWrapper, times(2)).destroyLiveTcpSocketsLackingPermission( + agent.network.netId, + INetd.PERMISSION_SYSTEM + ) + } } 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 990ef47392..a285dd1103 100644 --- a/tests/unit/java/com/android/server/connectivityservice/base/CSAgentWrapper.kt +++ b/tests/unit/java/com/android/server/connectivityservice/base/CSAgentWrapper.kt @@ -320,6 +320,7 @@ class CSAgentWrapper( fun sendLocalNetworkConfig(lnc: LocalNetworkConfig) = agent.sendLocalNetworkConfig(lnc) fun sendNetworkCapabilities(nc: NetworkCapabilities) = agent.sendNetworkCapabilities(nc) fun sendLinkProperties(lp: LinkProperties) = agent.sendLinkProperties(lp) + fun sendTeardownDelayMs(delayMs: Int) = agent.setTeardownDelayMillis(delayMs) fun connectWithCaptivePortal(redirectUrl: String) { setCaptivePortal(redirectUrl) 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 6fa420638d..a8f0b58f2b 100644 --- a/tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt +++ b/tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt @@ -304,6 +304,10 @@ open class CSTest { address: InetAddress?, interfaceId: Int ) {} + open fun destroyLiveTcpSocketsLackingPermission( + netId: Int, + permission: Int + ) {} } inner class CSDeps : ConnectivityService.Dependencies() { @@ -469,6 +473,17 @@ open class CSTest { interfaceId ) } + + override fun destroyLiveTcpSocketsLackingPermission( + netId: Int, + permission: Int + ) { + destroySocketsWrapper.destroyLiveTcpSocketsLackingPermission( + netId, + permission + ) + } + var netlinkMessageUpdate = Consumer {_ -> } override fun makeAddressUpdateMonitor( h: Handler,