From 7d0fcbb0d37999e6b9b2f1737a0e2b140ae1f10d Mon Sep 17 00:00:00 2001 From: Sarup Dalwani Date: Tue, 18 Feb 2025 19:08:04 +0000 Subject: [PATCH] Adding stacked link addresses to local_net_access map Iterating over stacked link address to ensure correct mapping for interface to local network ip prefixes. Bug: 369815823 Test: atest ConnectivityCoverageTests:android.net.connectivity.com.android.server.BpfNetMapsTest Test: atest ConnectivityCoverageTests:android.net.connectivity.com.android.server.CSLocalNetworkProtectionTest Change-Id: I3b2ed906caf43584ffb0e8041f9302c760099f7b --- .../android/server/ConnectivityService.java | 73 ++++++++----- .../CSLocalNetworkProtectionTest.kt | 100 ++++++++++++++++++ 2 files changed, 146 insertions(+), 27 deletions(-) diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 5b415c835c..a9d993cfc5 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -9791,42 +9791,61 @@ public class ConnectivityService extends IConnectivityManager.Stub removeLocalAddressesFromBpfMap(iface, MULTICAST_AND_BROADCAST_PREFIXES, oldLp); } - final CompareResult linkAddressDiff = new CompareResult<>( - oldLp != null ? oldLp.getLinkAddresses() : null, - newLp != null ? newLp.getLinkAddresses() : null); + // The both list contain current link properties + stacked links for new and old LP. + List newLinkProperties = new ArrayList<>(); + List oldLinkProperties = new ArrayList<>(); - List unicastLocalPrefixesToBeAdded = new ArrayList<>(); - List unicastLocalPrefixesToBeRemoved = new ArrayList<>(); - - // Finding the list of local network prefixes that needs to be added if (newLp != null) { - for (LinkAddress linkAddress : newLp.getLinkAddresses()) { + newLinkProperties.add(newLp); + newLinkProperties.addAll(newLp.getStackedLinks()); + } + if (oldLp != null) { + oldLinkProperties.add(oldLp); + oldLinkProperties.addAll(oldLp.getStackedLinks()); + } + + // map contains interface name to list of local network prefixes added because of change + // in link properties + Map> prefixesAddedForInterface = new ArrayMap<>(); + + final CompareResult linkPropertiesDiff = new CompareResult<>( + oldLinkProperties, newLinkProperties); + + for (LinkProperties linkProperty : linkPropertiesDiff.added) { + List unicastLocalPrefixesToBeAdded = new ArrayList<>(); + for (LinkAddress linkAddress : linkProperty.getLinkAddresses()) { unicastLocalPrefixesToBeAdded.addAll( getLocalNetworkPrefixesForAddress(linkAddress)); } - } + addLocalAddressesToBpfMap(linkProperty.getInterfaceName(), + unicastLocalPrefixesToBeAdded, linkProperty); - for (LinkAddress linkAddress : linkAddressDiff.removed) { - unicastLocalPrefixesToBeRemoved.addAll(getLocalNetworkPrefixesForAddress(linkAddress)); - } - - // If newLp is not null, adding local network prefixes using interface name of newLp - if (newLp != null) { - addLocalAddressesToBpfMap(newLp.getInterfaceName(), - new ArrayList<>(unicastLocalPrefixesToBeAdded), newLp); - } - if (oldLp != null) { - // excluding removal of ip prefixes that needs to be added for newLp, but also - // removed for oldLp. - if (newLp != null && Objects.equals(oldLp.getInterfaceName(), - newLp.getInterfaceName())) { - unicastLocalPrefixesToBeRemoved.removeAll(unicastLocalPrefixesToBeAdded); + // adding iterface name -> ip prefixes that we added to map + if (!prefixesAddedForInterface.containsKey(linkProperty.getInterfaceName())) { + prefixesAddedForInterface.put(linkProperty.getInterfaceName(), new ArrayList<>()); } - // removing ip local network prefixes because of change in link addresses. - removeLocalAddressesFromBpfMap(oldLp.getInterfaceName(), - new ArrayList<>(unicastLocalPrefixesToBeRemoved), oldLp); + prefixesAddedForInterface.get(linkProperty.getInterfaceName()) + .addAll(unicastLocalPrefixesToBeAdded); } + for (LinkProperties linkProperty : linkPropertiesDiff.removed) { + List unicastLocalPrefixesToBeRemoved = new ArrayList<>(); + List unicastLocalPrefixesAdded = prefixesAddedForInterface.getOrDefault( + linkProperty.getInterfaceName(), new ArrayList<>()); + + for (LinkAddress linkAddress : linkProperty.getLinkAddresses()) { + unicastLocalPrefixesToBeRemoved.addAll( + getLocalNetworkPrefixesForAddress(linkAddress)); + } + + // 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 + unicastLocalPrefixesToBeRemoved.removeAll(unicastLocalPrefixesAdded); + + removeLocalAddressesFromBpfMap(linkProperty.getInterfaceName(), + new ArrayList<>(unicastLocalPrefixesToBeRemoved), linkProperty); + } } /** diff --git a/tests/unit/java/com/android/server/connectivityservice/CSLocalNetworkProtectionTest.kt b/tests/unit/java/com/android/server/connectivityservice/CSLocalNetworkProtectionTest.kt index 73e7515fcb..5bf6e0445c 100644 --- a/tests/unit/java/com/android/server/connectivityservice/CSLocalNetworkProtectionTest.kt +++ b/tests/unit/java/com/android/server/connectivityservice/CSLocalNetworkProtectionTest.kt @@ -189,6 +189,106 @@ class CSLocalNetworkProtectionTest : CSTest() { ) } + @Test + fun testStackedLinkPropertiesWithDifferentLinkAddresses_AddressAddedInBpfMap() { + 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) + // Adding 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()) + } + + @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) + wifiAgent.sendLinkProperties(wifiLp_3) + cb.expect(wifiAgent.network) + + // As both stacked links is removed, 10.0.0.0/8 should be removed from local_net_access map. + verify(bpfNetMaps).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)