From cda101be7d997a796b1839b36d687c9c726b0767 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Tue, 24 Nov 2020 21:45:25 +0900 Subject: [PATCH 1/2] Move applyUnderlyingCapabilities to ConnectivityService. This is essentially a straighforward move of code from Vpn to ConnectivityService, and from VpnTest to ConnectivityServiceTest. Bug: 173331190 Test: passes existing tests, moved tests pass Change-Id: I76daa3abcc777e9c3ba57efb750de0e2e2f3bb74 --- .../android/server/ConnectivityService.java | 67 +++++++++- .../server/ConnectivityServiceTest.java | 114 +++++++++++++++++- .../android/server/connectivity/VpnTest.java | 105 ---------------- 3 files changed, 175 insertions(+), 111 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index f0561177fc..6ef1b6bbd5 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -41,6 +41,7 @@ import static android.net.INetworkMonitor.NETWORK_VALIDATION_RESULT_VALID; import static android.net.NetworkCapabilities.NET_CAPABILITY_CAPTIVE_PORTAL; import static android.net.NetworkCapabilities.NET_CAPABILITY_FOREGROUND; import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET; +import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_CONGESTED; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_METERED; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_ROAMING; @@ -6354,6 +6355,67 @@ public class ConnectivityService extends IConnectivityManager.Stub nai.declaredMetered = !nc.hasCapability(NET_CAPABILITY_NOT_METERED); } + @VisibleForTesting + void applyUnderlyingCapabilities(Network[] underlyingNetworks, + NetworkCapabilities caps, boolean declaredMetered) { + int[] transportTypes = new int[] { NetworkCapabilities.TRANSPORT_VPN }; + int downKbps = NetworkCapabilities.LINK_BANDWIDTH_UNSPECIFIED; + int upKbps = NetworkCapabilities.LINK_BANDWIDTH_UNSPECIFIED; + boolean metered = declaredMetered; // metered if any underlying is metered, or agentMetered + boolean roaming = false; // roaming if any underlying is roaming + boolean congested = false; // congested if any underlying is congested + boolean suspended = true; // suspended if all underlying are suspended + + boolean hadUnderlyingNetworks = false; + if (null != underlyingNetworks) { + for (Network underlyingNetwork : underlyingNetworks) { + final NetworkAgentInfo underlying = + getNetworkAgentInfoForNetwork(underlyingNetwork); + if (underlying == null) continue; + + final NetworkCapabilities underlyingCaps = underlying.networkCapabilities; + hadUnderlyingNetworks = true; + for (int underlyingType : underlyingCaps.getTransportTypes()) { + transportTypes = ArrayUtils.appendInt(transportTypes, underlyingType); + } + + // Merge capabilities of this underlying network. For bandwidth, assume the + // worst case. + downKbps = NetworkCapabilities.minBandwidth(downKbps, + underlyingCaps.getLinkDownstreamBandwidthKbps()); + upKbps = NetworkCapabilities.minBandwidth(upKbps, + underlyingCaps.getLinkUpstreamBandwidthKbps()); + // If this underlying network is metered, the VPN is metered (it may cost money + // to send packets on this network). + metered |= !underlyingCaps.hasCapability(NET_CAPABILITY_NOT_METERED); + // If this underlying network is roaming, the VPN is roaming (the billing structure + // is different than the usual, local one). + roaming |= !underlyingCaps.hasCapability(NET_CAPABILITY_NOT_ROAMING); + // If this underlying network is congested, the VPN is congested (the current + // condition of the network affects the performance of this network). + congested |= !underlyingCaps.hasCapability(NET_CAPABILITY_NOT_CONGESTED); + // If this network is not suspended, the VPN is not suspended (the VPN + // is able to transfer some data). + suspended &= !underlyingCaps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED); + } + } + if (!hadUnderlyingNetworks) { + // No idea what the underlying networks are; assume reasonable defaults + metered = true; + roaming = false; + congested = false; + suspended = false; + } + + caps.setTransportTypes(transportTypes); + caps.setLinkDownstreamBandwidthKbps(downKbps); + caps.setLinkUpstreamBandwidthKbps(upKbps); + caps.setCapability(NET_CAPABILITY_NOT_METERED, !metered); + caps.setCapability(NET_CAPABILITY_NOT_ROAMING, !roaming); + caps.setCapability(NET_CAPABILITY_NOT_CONGESTED, !congested); + caps.setCapability(NET_CAPABILITY_NOT_SUSPENDED, !suspended); + } + /** Propagates to |nc| the capabilities declared by the underlying networks of |nai|. */ private void mixInUnderlyingCapabilities(NetworkAgentInfo nai, NetworkCapabilities nc) { Network[] underlyingNetworks = nai.declaredUnderlyingNetworks; @@ -6362,10 +6424,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // null underlying networks means to track the default. underlyingNetworks = new Network[] { defaultNetwork }; } - - // TODO(b/124469351): Get capabilities directly from ConnectivityService instead. - final ConnectivityManager cm = mContext.getSystemService(ConnectivityManager.class); - Vpn.applyUnderlyingCapabilities(cm, underlyingNetworks, nc, nai.declaredMetered); + applyUnderlyingCapabilities(underlyingNetworks, nc, nai.declaredMetered); } /** diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index c917e66ea4..978c09ef13 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -46,6 +46,7 @@ import static android.net.INetworkMonitor.NETWORK_VALIDATION_PROBE_HTTPS; import static android.net.INetworkMonitor.NETWORK_VALIDATION_PROBE_PRIVDNS; import static android.net.INetworkMonitor.NETWORK_VALIDATION_RESULT_PARTIAL; import static android.net.INetworkMonitor.NETWORK_VALIDATION_RESULT_VALID; +import static android.net.NetworkCapabilities.LINK_BANDWIDTH_UNSPECIFIED; import static android.net.NetworkCapabilities.NET_CAPABILITY_CAPTIVE_PORTAL; import static android.net.NetworkCapabilities.NET_CAPABILITY_CBS; import static android.net.NetworkCapabilities.NET_CAPABILITY_DUN; @@ -56,8 +57,10 @@ import static android.net.NetworkCapabilities.NET_CAPABILITY_IA; import static android.net.NetworkCapabilities.NET_CAPABILITY_IMS; import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET; import static android.net.NetworkCapabilities.NET_CAPABILITY_MMS; +import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_CONGESTED; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_METERED; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED; +import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_ROAMING; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_VPN; import static android.net.NetworkCapabilities.NET_CAPABILITY_PARTIAL_CONNECTIVITY; @@ -5397,6 +5400,102 @@ public class ConnectivityServiceTest { assertTrue(lp.getDnsServers().containsAll(dnsServers)); } + @Test + public void testApplyUnderlyingCapabilities() throws Exception { + mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR); + mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI); + mCellNetworkAgent.connect(false /* validated */); + mWiFiNetworkAgent.connect(false /* validated */); + + final NetworkCapabilities cellNc = new NetworkCapabilities() + .addTransportType(TRANSPORT_CELLULAR) + .addCapability(NET_CAPABILITY_INTERNET) + .addCapability(NET_CAPABILITY_NOT_CONGESTED) + .setLinkDownstreamBandwidthKbps(10); + final NetworkCapabilities wifiNc = new NetworkCapabilities() + .addTransportType(TRANSPORT_WIFI) + .addCapability(NET_CAPABILITY_INTERNET) + .addCapability(NET_CAPABILITY_NOT_METERED) + .addCapability(NET_CAPABILITY_NOT_ROAMING) + .addCapability(NET_CAPABILITY_NOT_CONGESTED) + .addCapability(NET_CAPABILITY_NOT_SUSPENDED) + .setLinkUpstreamBandwidthKbps(20); + mCellNetworkAgent.setNetworkCapabilities(cellNc, true /* sendToConnectivityService */); + mWiFiNetworkAgent.setNetworkCapabilities(wifiNc, true /* sendToConnectivityService */); + waitForIdle(); + + final Network mobile = mCellNetworkAgent.getNetwork(); + final Network wifi = mWiFiNetworkAgent.getNetwork(); + + final NetworkCapabilities caps = new NetworkCapabilities(); + + mService.applyUnderlyingCapabilities(new Network[]{}, caps, false); + assertTrue(caps.hasTransport(TRANSPORT_VPN)); + assertFalse(caps.hasTransport(TRANSPORT_CELLULAR)); + assertFalse(caps.hasTransport(TRANSPORT_WIFI)); + assertEquals(LINK_BANDWIDTH_UNSPECIFIED, caps.getLinkDownstreamBandwidthKbps()); + assertEquals(LINK_BANDWIDTH_UNSPECIFIED, caps.getLinkUpstreamBandwidthKbps()); + assertFalse(caps.hasCapability(NET_CAPABILITY_NOT_METERED)); + assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_ROAMING)); + assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_CONGESTED)); + assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)); + + NetworkCapabilities otherCaps = new NetworkCapabilities(caps); + final boolean notDeclaredMetered = false; + mService.applyUnderlyingCapabilities(new Network[]{null}, otherCaps, notDeclaredMetered); + assertEquals(caps, otherCaps); + + mService.applyUnderlyingCapabilities(new Network[]{mobile}, caps, notDeclaredMetered); + assertTrue(caps.hasTransport(TRANSPORT_VPN)); + assertTrue(caps.hasTransport(TRANSPORT_CELLULAR)); + assertFalse(caps.hasTransport(TRANSPORT_WIFI)); + assertEquals(10, caps.getLinkDownstreamBandwidthKbps()); + assertEquals(LINK_BANDWIDTH_UNSPECIFIED, caps.getLinkUpstreamBandwidthKbps()); + assertFalse(caps.hasCapability(NET_CAPABILITY_NOT_METERED)); + assertFalse(caps.hasCapability(NET_CAPABILITY_NOT_ROAMING)); + assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_CONGESTED)); + assertFalse(caps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)); + + mService.applyUnderlyingCapabilities(new Network[]{wifi}, caps, notDeclaredMetered); + assertTrue(caps.hasTransport(TRANSPORT_VPN)); + assertFalse(caps.hasTransport(TRANSPORT_CELLULAR)); + assertTrue(caps.hasTransport(TRANSPORT_WIFI)); + assertEquals(LINK_BANDWIDTH_UNSPECIFIED, caps.getLinkDownstreamBandwidthKbps()); + assertEquals(20, caps.getLinkUpstreamBandwidthKbps()); + assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_METERED)); + assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_ROAMING)); + assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_CONGESTED)); + assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)); + + final boolean isDeclaredMetered = true; + mService.applyUnderlyingCapabilities(new Network[]{wifi}, caps, isDeclaredMetered); + assertTrue(caps.hasTransport(TRANSPORT_VPN)); + assertFalse(caps.hasTransport(TRANSPORT_CELLULAR)); + assertTrue(caps.hasTransport(TRANSPORT_WIFI)); + assertEquals(LINK_BANDWIDTH_UNSPECIFIED, caps.getLinkDownstreamBandwidthKbps()); + assertEquals(20, caps.getLinkUpstreamBandwidthKbps()); + assertFalse(caps.hasCapability(NET_CAPABILITY_NOT_METERED)); + assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_ROAMING)); + assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_CONGESTED)); + assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)); + + mService.applyUnderlyingCapabilities(new Network[]{mobile, wifi}, caps, notDeclaredMetered); + assertTrue(caps.hasTransport(TRANSPORT_VPN)); + assertTrue(caps.hasTransport(TRANSPORT_CELLULAR)); + assertTrue(caps.hasTransport(TRANSPORT_WIFI)); + assertEquals(10, caps.getLinkDownstreamBandwidthKbps()); + assertEquals(20, caps.getLinkUpstreamBandwidthKbps()); + assertFalse(caps.hasCapability(NET_CAPABILITY_NOT_METERED)); + assertFalse(caps.hasCapability(NET_CAPABILITY_NOT_ROAMING)); + assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_CONGESTED)); + assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)); + + otherCaps = new NetworkCapabilities(caps); + mService.applyUnderlyingCapabilities(new Network[]{null, mobile, null, wifi}, + otherCaps, notDeclaredMetered); + assertEquals(otherCaps, caps); + } + @Test public void testVpnConnectDisconnectUnderlyingNetwork() throws Exception { final TestNetworkCallback callback = new TestNetworkCallback(); @@ -5947,17 +6046,28 @@ public class ConnectivityServiceTest { && caps.hasTransport(TRANSPORT_VPN) && caps.hasTransport(TRANSPORT_WIFI)); + // Change the VPN's capabilities somehow (specifically, disconnect wifi). + mWiFiNetworkAgent.disconnect(); + callback.expectCallback(CallbackEntry.LOST, mWiFiNetworkAgent); + callback.expectCapabilitiesThat(mMockVpn, (caps) + -> caps.getUids().size() == 2 + && caps.getUids().contains(new UidRange(uid, uid)) + && caps.getUids().contains(UidRange.createForUser(restrictedUserId)) + && caps.hasTransport(TRANSPORT_VPN) + && !caps.hasTransport(TRANSPORT_WIFI)); + // Send a USER_REMOVED broadcast and expect to lose the UID range for the restricted user. final Intent removedIntent = new Intent(ACTION_USER_REMOVED); removedIntent.putExtra(Intent.EXTRA_USER_HANDLE, restrictedUserId); handler.post(() -> mServiceContext.sendBroadcast(removedIntent)); - // Expect that the VPN gains the UID range for the restricted user. + // Expect that the VPN gains the UID range for the restricted user, and that the capability + // change made just before that (i.e., loss of TRANSPORT_WIFI) is preserved. callback.expectCapabilitiesThat(mMockVpn, (caps) -> caps.getUids().size() == 1 && caps.getUids().contains(new UidRange(uid, uid)) && caps.hasTransport(TRANSPORT_VPN) - && caps.hasTransport(TRANSPORT_WIFI)); + && !caps.hasTransport(TRANSPORT_WIFI)); } @Test diff --git a/tests/net/java/com/android/server/connectivity/VpnTest.java b/tests/net/java/com/android/server/connectivity/VpnTest.java index e1e0efa5cf..1c1b5e1c66 100644 --- a/tests/net/java/com/android/server/connectivity/VpnTest.java +++ b/tests/net/java/com/android/server/connectivity/VpnTest.java @@ -21,15 +21,6 @@ import static android.content.pm.UserInfo.FLAG_MANAGED_PROFILE; import static android.content.pm.UserInfo.FLAG_PRIMARY; import static android.content.pm.UserInfo.FLAG_RESTRICTED; import static android.net.ConnectivityManager.NetworkCallback; -import static android.net.NetworkCapabilities.LINK_BANDWIDTH_UNSPECIFIED; -import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET; -import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_CONGESTED; -import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_METERED; -import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_ROAMING; -import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED; -import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR; -import static android.net.NetworkCapabilities.TRANSPORT_VPN; -import static android.net.NetworkCapabilities.TRANSPORT_WIFI; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; @@ -621,102 +612,6 @@ public class VpnTest { order.verify(mNotificationManager).cancel(anyString(), anyInt()); } - @Test - public void testCapabilities() { - setMockedUsers(primaryUser); - - final Network mobile = new Network(1); - final Network wifi = new Network(2); - - final Map networks = new HashMap<>(); - networks.put( - mobile, - new NetworkCapabilities() - .addTransportType(TRANSPORT_CELLULAR) - .addCapability(NET_CAPABILITY_INTERNET) - .addCapability(NET_CAPABILITY_NOT_CONGESTED) - .setLinkDownstreamBandwidthKbps(10)); - networks.put( - wifi, - new NetworkCapabilities() - .addTransportType(TRANSPORT_WIFI) - .addCapability(NET_CAPABILITY_INTERNET) - .addCapability(NET_CAPABILITY_NOT_METERED) - .addCapability(NET_CAPABILITY_NOT_ROAMING) - .addCapability(NET_CAPABILITY_NOT_CONGESTED) - .addCapability(NET_CAPABILITY_NOT_SUSPENDED) - .setLinkUpstreamBandwidthKbps(20)); - setMockedNetworks(networks); - - final NetworkCapabilities caps = new NetworkCapabilities(); - - Vpn.applyUnderlyingCapabilities( - mConnectivityManager, new Network[] {}, caps, false /* isAlwaysMetered */); - assertTrue(caps.hasTransport(TRANSPORT_VPN)); - assertFalse(caps.hasTransport(TRANSPORT_CELLULAR)); - assertFalse(caps.hasTransport(TRANSPORT_WIFI)); - assertEquals(LINK_BANDWIDTH_UNSPECIFIED, caps.getLinkDownstreamBandwidthKbps()); - assertEquals(LINK_BANDWIDTH_UNSPECIFIED, caps.getLinkUpstreamBandwidthKbps()); - assertFalse(caps.hasCapability(NET_CAPABILITY_NOT_METERED)); - assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_ROAMING)); - assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_CONGESTED)); - assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)); - - Vpn.applyUnderlyingCapabilities( - mConnectivityManager, - new Network[] {mobile}, - caps, - false /* isAlwaysMetered */); - assertTrue(caps.hasTransport(TRANSPORT_VPN)); - assertTrue(caps.hasTransport(TRANSPORT_CELLULAR)); - assertFalse(caps.hasTransport(TRANSPORT_WIFI)); - assertEquals(10, caps.getLinkDownstreamBandwidthKbps()); - assertEquals(LINK_BANDWIDTH_UNSPECIFIED, caps.getLinkUpstreamBandwidthKbps()); - assertFalse(caps.hasCapability(NET_CAPABILITY_NOT_METERED)); - assertFalse(caps.hasCapability(NET_CAPABILITY_NOT_ROAMING)); - assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_CONGESTED)); - assertFalse(caps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)); - - Vpn.applyUnderlyingCapabilities( - mConnectivityManager, new Network[] {wifi}, caps, false /* isAlwaysMetered */); - assertTrue(caps.hasTransport(TRANSPORT_VPN)); - assertFalse(caps.hasTransport(TRANSPORT_CELLULAR)); - assertTrue(caps.hasTransport(TRANSPORT_WIFI)); - assertEquals(LINK_BANDWIDTH_UNSPECIFIED, caps.getLinkDownstreamBandwidthKbps()); - assertEquals(20, caps.getLinkUpstreamBandwidthKbps()); - assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_METERED)); - assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_ROAMING)); - assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_CONGESTED)); - assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)); - - Vpn.applyUnderlyingCapabilities( - mConnectivityManager, new Network[] {wifi}, caps, true /* isAlwaysMetered */); - assertTrue(caps.hasTransport(TRANSPORT_VPN)); - assertFalse(caps.hasTransport(TRANSPORT_CELLULAR)); - assertTrue(caps.hasTransport(TRANSPORT_WIFI)); - assertEquals(LINK_BANDWIDTH_UNSPECIFIED, caps.getLinkDownstreamBandwidthKbps()); - assertEquals(20, caps.getLinkUpstreamBandwidthKbps()); - assertFalse(caps.hasCapability(NET_CAPABILITY_NOT_METERED)); - assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_ROAMING)); - assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_CONGESTED)); - assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)); - - Vpn.applyUnderlyingCapabilities( - mConnectivityManager, - new Network[] {mobile, wifi}, - caps, - false /* isAlwaysMetered */); - assertTrue(caps.hasTransport(TRANSPORT_VPN)); - assertTrue(caps.hasTransport(TRANSPORT_CELLULAR)); - assertTrue(caps.hasTransport(TRANSPORT_WIFI)); - assertEquals(10, caps.getLinkDownstreamBandwidthKbps()); - assertEquals(20, caps.getLinkUpstreamBandwidthKbps()); - assertFalse(caps.hasCapability(NET_CAPABILITY_NOT_METERED)); - assertFalse(caps.hasCapability(NET_CAPABILITY_NOT_ROAMING)); - assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_CONGESTED)); - assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)); - } - /** * The profile name should NOT change between releases for backwards compatibility * From d7caf838c034acf212b89685297156227fface1f Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Tue, 1 Dec 2020 01:08:37 +0900 Subject: [PATCH 2/2] Refactor applyUnderlyingCapabilities and its test. This reduces verbose assertions and makes the test more compact. I'm not sure whether it's actually more valuable, since the current code, while more verbose, is probably more straightforward to understand. Also add a test for passing in a null underlying network (i.e., follow default network). This requires a minor refactoring in ConnectivityService because the applyUnderlyingCapabilities does not currently treat null specially. Bug: 173331190 Test: test-only change Change-Id: Ic5a3e16969ea9e1a529706850f148cb0d5fd8e09 --- .../android/server/ConnectivityService.java | 23 ++-- .../server/ConnectivityServiceTest.java | 113 +++++++++--------- 2 files changed, 67 insertions(+), 69 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 6ef1b6bbd5..9b006d050c 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -6355,9 +6355,15 @@ public class ConnectivityService extends IConnectivityManager.Stub nai.declaredMetered = !nc.hasCapability(NET_CAPABILITY_NOT_METERED); } + /** Modifies |caps| based on the capabilities of the specified underlying networks. */ @VisibleForTesting - void applyUnderlyingCapabilities(Network[] underlyingNetworks, - NetworkCapabilities caps, boolean declaredMetered) { + void applyUnderlyingCapabilities(@Nullable Network[] underlyingNetworks, + @NonNull NetworkCapabilities caps, boolean declaredMetered) { + final Network defaultNetwork = getNetwork(getDefaultNetwork()); + if (underlyingNetworks == null && defaultNetwork != null) { + // null underlying networks means to track the default. + underlyingNetworks = new Network[] { defaultNetwork }; + } int[] transportTypes = new int[] { NetworkCapabilities.TRANSPORT_VPN }; int downKbps = NetworkCapabilities.LINK_BANDWIDTH_UNSPECIFIED; int upKbps = NetworkCapabilities.LINK_BANDWIDTH_UNSPECIFIED; @@ -6416,17 +6422,6 @@ public class ConnectivityService extends IConnectivityManager.Stub caps.setCapability(NET_CAPABILITY_NOT_SUSPENDED, !suspended); } - /** Propagates to |nc| the capabilities declared by the underlying networks of |nai|. */ - private void mixInUnderlyingCapabilities(NetworkAgentInfo nai, NetworkCapabilities nc) { - Network[] underlyingNetworks = nai.declaredUnderlyingNetworks; - Network defaultNetwork = getNetwork(getDefaultNetwork()); - if (underlyingNetworks == null && defaultNetwork != null) { - // null underlying networks means to track the default. - underlyingNetworks = new Network[] { defaultNetwork }; - } - applyUnderlyingCapabilities(underlyingNetworks, nc, nai.declaredMetered); - } - /** * Augments the NetworkCapabilities passed in by a NetworkAgent with capabilities that are * maintained here that the NetworkAgent is not aware of (e.g., validated, captive portal, @@ -6481,7 +6476,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } if (nai.supportsUnderlyingNetworks()) { - mixInUnderlyingCapabilities(nai, newNc); + applyUnderlyingCapabilities(nai.declaredUnderlyingNetworks, newNc, nai.declaredMetered); } return newNc; diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 978c09ef13..878f6fa0f9 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -46,7 +46,6 @@ import static android.net.INetworkMonitor.NETWORK_VALIDATION_PROBE_HTTPS; import static android.net.INetworkMonitor.NETWORK_VALIDATION_PROBE_PRIVDNS; import static android.net.INetworkMonitor.NETWORK_VALIDATION_RESULT_PARTIAL; import static android.net.INetworkMonitor.NETWORK_VALIDATION_RESULT_VALID; -import static android.net.NetworkCapabilities.LINK_BANDWIDTH_UNSPECIFIED; import static android.net.NetworkCapabilities.NET_CAPABILITY_CAPTIVE_PORTAL; import static android.net.NetworkCapabilities.NET_CAPABILITY_CBS; import static android.net.NetworkCapabilities.NET_CAPABILITY_DUN; @@ -5427,73 +5426,77 @@ public class ConnectivityServiceTest { final Network mobile = mCellNetworkAgent.getNetwork(); final Network wifi = mWiFiNetworkAgent.getNetwork(); - final NetworkCapabilities caps = new NetworkCapabilities(); + final NetworkCapabilities initialCaps = new NetworkCapabilities(); + initialCaps.addCapability(NET_CAPABILITY_INTERNET); + initialCaps.removeCapability(NET_CAPABILITY_NOT_VPN); - mService.applyUnderlyingCapabilities(new Network[]{}, caps, false); - assertTrue(caps.hasTransport(TRANSPORT_VPN)); - assertFalse(caps.hasTransport(TRANSPORT_CELLULAR)); - assertFalse(caps.hasTransport(TRANSPORT_WIFI)); - assertEquals(LINK_BANDWIDTH_UNSPECIFIED, caps.getLinkDownstreamBandwidthKbps()); - assertEquals(LINK_BANDWIDTH_UNSPECIFIED, caps.getLinkUpstreamBandwidthKbps()); - assertFalse(caps.hasCapability(NET_CAPABILITY_NOT_METERED)); - assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_ROAMING)); - assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_CONGESTED)); - assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)); + final NetworkCapabilities withNoUnderlying = new NetworkCapabilities(); + withNoUnderlying.addCapability(NET_CAPABILITY_INTERNET); + withNoUnderlying.addCapability(NET_CAPABILITY_NOT_CONGESTED); + withNoUnderlying.addCapability(NET_CAPABILITY_NOT_ROAMING); + withNoUnderlying.addCapability(NET_CAPABILITY_NOT_SUSPENDED); + withNoUnderlying.addTransportType(TRANSPORT_VPN); + withNoUnderlying.removeCapability(NET_CAPABILITY_NOT_VPN); - NetworkCapabilities otherCaps = new NetworkCapabilities(caps); + final NetworkCapabilities withMobileUnderlying = new NetworkCapabilities(withNoUnderlying); + withMobileUnderlying.addTransportType(TRANSPORT_CELLULAR); + withMobileUnderlying.removeCapability(NET_CAPABILITY_NOT_ROAMING); + withMobileUnderlying.removeCapability(NET_CAPABILITY_NOT_SUSPENDED); + withMobileUnderlying.setLinkDownstreamBandwidthKbps(10); + + final NetworkCapabilities withWifiUnderlying = new NetworkCapabilities(withNoUnderlying); + withWifiUnderlying.addTransportType(TRANSPORT_WIFI); + withWifiUnderlying.addCapability(NET_CAPABILITY_NOT_METERED); + withWifiUnderlying.setLinkUpstreamBandwidthKbps(20); + + final NetworkCapabilities withWifiAndMobileUnderlying = + new NetworkCapabilities(withNoUnderlying); + withWifiAndMobileUnderlying.addTransportType(TRANSPORT_CELLULAR); + withWifiAndMobileUnderlying.addTransportType(TRANSPORT_WIFI); + withWifiAndMobileUnderlying.removeCapability(NET_CAPABILITY_NOT_METERED); + withWifiAndMobileUnderlying.removeCapability(NET_CAPABILITY_NOT_ROAMING); + withWifiAndMobileUnderlying.setLinkDownstreamBandwidthKbps(10); + withWifiAndMobileUnderlying.setLinkUpstreamBandwidthKbps(20); + + NetworkCapabilities caps = new NetworkCapabilities(initialCaps); final boolean notDeclaredMetered = false; - mService.applyUnderlyingCapabilities(new Network[]{null}, otherCaps, notDeclaredMetered); - assertEquals(caps, otherCaps); + mService.applyUnderlyingCapabilities(new Network[]{}, caps, notDeclaredMetered); + assertEquals(withNoUnderlying, caps); + caps = new NetworkCapabilities(initialCaps); + mService.applyUnderlyingCapabilities(new Network[]{null}, caps, notDeclaredMetered); + assertEquals(withNoUnderlying, caps); + + caps = new NetworkCapabilities(initialCaps); mService.applyUnderlyingCapabilities(new Network[]{mobile}, caps, notDeclaredMetered); - assertTrue(caps.hasTransport(TRANSPORT_VPN)); - assertTrue(caps.hasTransport(TRANSPORT_CELLULAR)); - assertFalse(caps.hasTransport(TRANSPORT_WIFI)); - assertEquals(10, caps.getLinkDownstreamBandwidthKbps()); - assertEquals(LINK_BANDWIDTH_UNSPECIFIED, caps.getLinkUpstreamBandwidthKbps()); - assertFalse(caps.hasCapability(NET_CAPABILITY_NOT_METERED)); - assertFalse(caps.hasCapability(NET_CAPABILITY_NOT_ROAMING)); - assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_CONGESTED)); - assertFalse(caps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)); + assertEquals(withMobileUnderlying, caps); mService.applyUnderlyingCapabilities(new Network[]{wifi}, caps, notDeclaredMetered); - assertTrue(caps.hasTransport(TRANSPORT_VPN)); - assertFalse(caps.hasTransport(TRANSPORT_CELLULAR)); - assertTrue(caps.hasTransport(TRANSPORT_WIFI)); - assertEquals(LINK_BANDWIDTH_UNSPECIFIED, caps.getLinkDownstreamBandwidthKbps()); - assertEquals(20, caps.getLinkUpstreamBandwidthKbps()); - assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_METERED)); - assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_ROAMING)); - assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_CONGESTED)); - assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)); + assertEquals(withWifiUnderlying, caps); final boolean isDeclaredMetered = true; + withWifiUnderlying.removeCapability(NET_CAPABILITY_NOT_METERED); + caps = new NetworkCapabilities(initialCaps); mService.applyUnderlyingCapabilities(new Network[]{wifi}, caps, isDeclaredMetered); - assertTrue(caps.hasTransport(TRANSPORT_VPN)); - assertFalse(caps.hasTransport(TRANSPORT_CELLULAR)); - assertTrue(caps.hasTransport(TRANSPORT_WIFI)); - assertEquals(LINK_BANDWIDTH_UNSPECIFIED, caps.getLinkDownstreamBandwidthKbps()); - assertEquals(20, caps.getLinkUpstreamBandwidthKbps()); - assertFalse(caps.hasCapability(NET_CAPABILITY_NOT_METERED)); - assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_ROAMING)); - assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_CONGESTED)); - assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)); + assertEquals(withWifiUnderlying, caps); - mService.applyUnderlyingCapabilities(new Network[]{mobile, wifi}, caps, notDeclaredMetered); - assertTrue(caps.hasTransport(TRANSPORT_VPN)); - assertTrue(caps.hasTransport(TRANSPORT_CELLULAR)); - assertTrue(caps.hasTransport(TRANSPORT_WIFI)); - assertEquals(10, caps.getLinkDownstreamBandwidthKbps()); - assertEquals(20, caps.getLinkUpstreamBandwidthKbps()); - assertFalse(caps.hasCapability(NET_CAPABILITY_NOT_METERED)); - assertFalse(caps.hasCapability(NET_CAPABILITY_NOT_ROAMING)); - assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_CONGESTED)); - assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)); + caps = new NetworkCapabilities(initialCaps); + mService.applyUnderlyingCapabilities(new Network[]{mobile, wifi}, caps, isDeclaredMetered); + assertEquals(withWifiAndMobileUnderlying, caps); - otherCaps = new NetworkCapabilities(caps); + withWifiUnderlying.addCapability(NET_CAPABILITY_NOT_METERED); + caps = new NetworkCapabilities(initialCaps); mService.applyUnderlyingCapabilities(new Network[]{null, mobile, null, wifi}, - otherCaps, notDeclaredMetered); - assertEquals(otherCaps, caps); + caps, notDeclaredMetered); + assertEquals(withWifiAndMobileUnderlying, caps); + + caps = new NetworkCapabilities(initialCaps); + mService.applyUnderlyingCapabilities(new Network[]{null, mobile, null, wifi}, + caps, notDeclaredMetered); + assertEquals(withWifiAndMobileUnderlying, caps); + + mService.applyUnderlyingCapabilities(null, caps, notDeclaredMetered); + assertEquals(withWifiUnderlying, caps); } @Test