diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 2a66e4bff0..2c6390f1a0 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -9810,8 +9810,8 @@ public class ConnectivityService extends IConnectivityManager.Stub { } // The both list contain current link properties + stacked links for new and old LP. - List newLinkProperties = new ArrayList<>(); - List oldLinkProperties = new ArrayList<>(); + final List newLinkProperties = new ArrayList<>(); + final List oldLinkProperties = new ArrayList<>(); if (newLp != null) { newLinkProperties.add(newLp); @@ -9824,13 +9824,13 @@ public class ConnectivityService extends IConnectivityManager.Stub { // map contains interface name to list of local network prefixes added because of change // in link properties - Map> prefixesAddedForInterface = new ArrayMap<>(); + final Map> prefixesAddedForInterface = new ArrayMap<>(); final CompareResult linkPropertiesDiff = new CompareResult<>( oldLinkProperties, newLinkProperties); for (LinkProperties linkProperty : linkPropertiesDiff.added) { - List unicastLocalPrefixesToBeAdded = new ArrayList<>(); + final List unicastLocalPrefixesToBeAdded = new ArrayList<>(); for (LinkAddress linkAddress : linkProperty.getLinkAddresses()) { unicastLocalPrefixesToBeAdded.addAll( getLocalNetworkPrefixesForAddress(linkAddress)); @@ -9838,7 +9838,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { addLocalAddressesToBpfMap(linkProperty.getInterfaceName(), unicastLocalPrefixesToBeAdded, linkProperty); - // adding iterface name -> ip prefixes that we added to map + // populating interface name -> ip prefixes which were added to local_net_access map. if (!prefixesAddedForInterface.containsKey(linkProperty.getInterfaceName())) { prefixesAddedForInterface.put(linkProperty.getInterfaceName(), new ArrayList<>()); } @@ -9847,9 +9847,9 @@ public class ConnectivityService extends IConnectivityManager.Stub { } for (LinkProperties linkProperty : linkPropertiesDiff.removed) { - List unicastLocalPrefixesToBeRemoved = new ArrayList<>(); - List unicastLocalPrefixesAdded = prefixesAddedForInterface.getOrDefault( - linkProperty.getInterfaceName(), new ArrayList<>()); + final List unicastLocalPrefixesToBeRemoved = new ArrayList<>(); + final List unicastLocalPrefixesAdded = prefixesAddedForInterface.getOrDefault( + linkProperty.getInterfaceName(), Collections.emptyList()); for (LinkAddress linkAddress : linkProperty.getLinkAddresses()) { unicastLocalPrefixesToBeRemoved.addAll( @@ -9857,8 +9857,8 @@ public class ConnectivityService extends IConnectivityManager.Stub { } // This is to ensure if 10.0.10.0/24 was added and 10.0.11.0/24 was removed both will - // still populate the same prefix of 10.0.0.0/8, which mean we should not remove the - // prefix because of removal of 10.0.11.0/24 + // still populate the same prefix of 10.0.0.0/8, which mean 10.0.0.0/8 should not be + // removed due to removal of 10.0.11.0/24 unicastLocalPrefixesToBeRemoved.removeAll(unicastLocalPrefixesAdded); removeLocalAddressesFromBpfMap(linkProperty.getInterfaceName(), diff --git a/tests/unit/java/com/android/server/connectivityservice/CSLocalNetworkProtectionTest.kt b/tests/unit/java/com/android/server/connectivityservice/CSLocalNetworkProtectionTest.kt index 5bf6e0445c..84c98359de 100644 --- a/tests/unit/java/com/android/server/connectivityservice/CSLocalNetworkProtectionTest.kt +++ b/tests/unit/java/com/android/server/connectivityservice/CSLocalNetworkProtectionTest.kt @@ -38,6 +38,7 @@ import org.junit.Test import org.junit.runner.RunWith import org.mockito.ArgumentMatchers.eq import org.mockito.Mockito.never +import org.mockito.Mockito.times import org.mockito.Mockito.verify private const val LONG_TIMEOUT_MS = 5_000 @@ -45,6 +46,7 @@ private const val PREFIX_LENGTH_IPV4 = 32 + 96 private const val PREFIX_LENGTH_IPV6 = 32 private const val WIFI_IFNAME = "wlan0" private const val WIFI_IFNAME_2 = "wlan1" +private const val WIFI_IFNAME_3 = "wlan2" private val wifiNc = NetworkCapabilities.Builder() .addTransportType(TRANSPORT_WIFI) @@ -78,6 +80,20 @@ class CSLocalNetworkProtectionTest : CSTest() { LOCAL_IPV6_IP_ADDRESS_PREFIX.getPrefixLength() ) + private val LOCAL_IPV6_IP_ADDRESS_2_PREFIX = + IpPrefix("2601:19b:67f:e220:1cf1:35ff:fe8c:db87/64") + private val LOCAL_IPV6_LINK_ADDRESS_2 = LinkAddress( + LOCAL_IPV6_IP_ADDRESS_2_PREFIX.getAddress(), + LOCAL_IPV6_IP_ADDRESS_2_PREFIX.getPrefixLength() + ) + + private val LOCAL_IPV6_IP_ADDRESS_3_PREFIX = + IpPrefix("fe80::/10") + private val LOCAL_IPV6_LINK_ADDRESS_3 = LinkAddress( + LOCAL_IPV6_IP_ADDRESS_3_PREFIX.getAddress(), + LOCAL_IPV6_IP_ADDRESS_3_PREFIX.getPrefixLength() + ) + private val LOCAL_IPV4_IP_ADDRESS_PREFIX_1 = IpPrefix("10.0.0.184/24") private val LOCAL_IPV4_LINK_ADDRRESS_1 = LinkAddress( @@ -190,7 +206,7 @@ class CSLocalNetworkProtectionTest : CSTest() { } @Test - fun testStackedLinkPropertiesWithDifferentLinkAddresses_AddressAddedInBpfMap() { + fun testAddingThenRemovingStackedLinkProperties_AddressAddedThenRemovedInBpfMap() { val nr = nr(TRANSPORT_WIFI) val cb = TestableNetworkCallback() cm.requestNetwork(nr, cb) @@ -230,49 +246,6 @@ class CSLocalNetworkProtectionTest : CSTest() { ) // As both addresses are in stacked links, so no address should be removed from the map. verify(bpfNetMaps, never()).removeLocalNetAccess(any(), any(), any(), any(), any()) - } - - @Test - fun testRemovingStackedLinkProperties_AddressRemovedInBpfMap() { - val nr = nr(TRANSPORT_WIFI) - val cb = TestableNetworkCallback() - cm.requestNetwork(nr, cb) - - val wifiLp = lp(WIFI_IFNAME, LOCAL_IPV6_LINK_ADDRESS) - val wifiLp2 = lp(WIFI_IFNAME_2, LOCAL_IPV4_LINK_ADDRRESS_1) - // populating stacked link - wifiLp.addStackedLink(wifiLp2) - val wifiAgent = Agent(nc = wifiNc, lp = wifiLp) - wifiAgent.connect() - cb.expectAvailableCallbacks(wifiAgent.network, validated = false) - - // Multicast and Broadcast address should always be populated in local_net_access map - verifyPopulationOfMulticastAndBroadcastAddress() - // Verifying IPv6 address should be populated in local_net_access map - verify(bpfNetMaps).addLocalNetAccess( - eq(PREFIX_LENGTH_IPV6 + LOCAL_IPV6_IP_ADDRESS_PREFIX.getPrefixLength()), - eq(WIFI_IFNAME), - eq(LOCAL_IPV6_IP_ADDRESS_PREFIX.getAddress()), - eq(0), - eq(0), - eq(false) - ) - - // Multicast and Broadcast address should always be populated on stacked link - // in local_net_access map - verifyPopulationOfMulticastAndBroadcastAddress(WIFI_IFNAME_2) - // Verifying IPv4 matching prefix(10.0.0.0/8) should be populated as part of stacked link - // in local_net_access map - verify(bpfNetMaps).addLocalNetAccess( - eq(PREFIX_LENGTH_IPV4 + 8), - eq(WIFI_IFNAME_2), - eq(InetAddresses.parseNumericAddress("10.0.0.0")), - eq(0), - eq(0), - eq(false) - ) - // As both addresses are in stacked links, so no address should be removed from the map. - verify(bpfNetMaps, never()).removeLocalNetAccess(any(), any(), any(), any(), any()) // replacing link properties without stacked links val wifiLp_3 = lp(WIFI_IFNAME, LOCAL_IPV6_LINK_ADDRESS) @@ -289,6 +262,107 @@ class CSLocalNetworkProtectionTest : CSTest() { ) } + @Test + fun testChangeStackedLinkProperties_AddressReplacedBpfMap() { + val nr = nr(TRANSPORT_WIFI) + val cb = TestableNetworkCallback() + cm.requestNetwork(nr, cb) + + val wifiLp = lp(WIFI_IFNAME, LOCAL_IPV6_LINK_ADDRESS) + val wifiLp2 = lp(WIFI_IFNAME_2, LOCAL_IPV4_LINK_ADDRRESS_1) + // populating stacked link + wifiLp.addStackedLink(wifiLp2) + val wifiAgent = Agent(nc = wifiNc, lp = wifiLp) + wifiAgent.connect() + cb.expectAvailableCallbacks(wifiAgent.network, validated = false) + + // Multicast and Broadcast address should always be populated in local_net_access map + verifyPopulationOfMulticastAndBroadcastAddress() + // Verifying IPv6 address should be populated in local_net_access map + verify(bpfNetMaps).addLocalNetAccess( + eq(PREFIX_LENGTH_IPV6 + LOCAL_IPV6_IP_ADDRESS_PREFIX.getPrefixLength()), + eq(WIFI_IFNAME), + eq(LOCAL_IPV6_IP_ADDRESS_PREFIX.getAddress()), + eq(0), + eq(0), + eq(false) + ) + + // Multicast and Broadcast address should always be populated on stacked link + // in local_net_access map + verifyPopulationOfMulticastAndBroadcastAddress(WIFI_IFNAME_2) + // Verifying IPv4 matching prefix(10.0.0.0/8) should be populated as part of stacked link + // in local_net_access map + verify(bpfNetMaps).addLocalNetAccess( + eq(PREFIX_LENGTH_IPV4 + 8), + eq(WIFI_IFNAME_2), + eq(InetAddresses.parseNumericAddress("10.0.0.0")), + eq(0), + eq(0), + eq(false) + ) + // As both addresses are in stacked links, so no address should be removed from the map. + verify(bpfNetMaps, never()).removeLocalNetAccess(any(), any(), any(), any(), any()) + + // replacing link properties multiple stacked links + val wifiLp_3 = lp(WIFI_IFNAME, LOCAL_IPV6_LINK_ADDRESS_2) + val wifiLp_4 = lp(WIFI_IFNAME_2, LOCAL_IPV4_LINK_ADDRRESS_2) + val wifiLp_5 = lp(WIFI_IFNAME_3, LOCAL_IPV6_LINK_ADDRESS_3) + wifiLp_3.addStackedLink(wifiLp_4) + wifiLp_3.addStackedLink(wifiLp_5) + wifiAgent.sendLinkProperties(wifiLp_3) + cb.expect(wifiAgent.network) + + // Multicast and Broadcast address should always be populated on stacked link + // in local_net_access map + verifyPopulationOfMulticastAndBroadcastAddress(WIFI_IFNAME_3) + // Verifying new base IPv6 address should be populated in local_net_access map + verify(bpfNetMaps).addLocalNetAccess( + eq(PREFIX_LENGTH_IPV6 + LOCAL_IPV6_IP_ADDRESS_2_PREFIX.getPrefixLength()), + eq(WIFI_IFNAME), + eq(LOCAL_IPV6_IP_ADDRESS_2_PREFIX.getAddress()), + eq(0), + eq(0), + eq(false) + ) + // Verifying IPv4 matching prefix(10.0.0.0/8) should be populated as part of stacked link + // in local_net_access map + verify(bpfNetMaps, times(2)).addLocalNetAccess( + eq(PREFIX_LENGTH_IPV4 + 8), + eq(WIFI_IFNAME_2), + eq(InetAddresses.parseNumericAddress("10.0.0.0")), + eq(0), + eq(0), + eq(false) + ) + // Verifying newly stacked IPv6 address should be populated in local_net_access map + verify(bpfNetMaps).addLocalNetAccess( + eq(PREFIX_LENGTH_IPV6 + LOCAL_IPV6_IP_ADDRESS_3_PREFIX.getPrefixLength()), + eq(WIFI_IFNAME_3), + eq(LOCAL_IPV6_IP_ADDRESS_3_PREFIX.getAddress()), + eq(0), + eq(0), + eq(false) + ) + // Verifying old base IPv6 address should be removed from local_net_access map + verify(bpfNetMaps).removeLocalNetAccess( + eq(PREFIX_LENGTH_IPV6 + LOCAL_IPV6_IP_ADDRESS_PREFIX.getPrefixLength()), + eq(WIFI_IFNAME), + eq(LOCAL_IPV6_IP_ADDRESS_PREFIX.getAddress()), + eq(0), + eq(0) + ) + // As both stacked links is had same prefix, 10.0.0.0/8 should not be removed from + // local_net_access map. + verify(bpfNetMaps, never()).removeLocalNetAccess( + eq(PREFIX_LENGTH_IPV4 + 8), + eq(WIFI_IFNAME_2), + eq(InetAddresses.parseNumericAddress("10.0.0.0")), + eq(0), + eq(0) + ) + } + @Test fun testChangeLinkPropertiesWithLinkAddressesInSameRange_AddressIntactInBpfMap() { val nr = nr(TRANSPORT_WIFI)