From 430a95f07c5aed1370f5e8d3acd28ef1a1b36d6e Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Tue, 24 Nov 2020 21:45:25 +0900 Subject: 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 --- .../com/android/server/ConnectivityService.java | 67 +++++++++++- .../java/com/android/server/connectivity/Vpn.java | 97 ------------------ .../android/server/ConnectivityServiceTest.java | 114 ++++++++++++++++++++- .../com/android/server/connectivity/VpnTest.java | 105 ------------------- 4 files changed, 175 insertions(+), 208 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index f0561177fc9c..6ef1b6bbd59f 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/services/core/java/com/android/server/connectivity/Vpn.java b/services/core/java/com/android/server/connectivity/Vpn.java index 73125c144380..ed1f60fb0bc1 100644 --- a/services/core/java/com/android/server/connectivity/Vpn.java +++ b/services/core/java/com/android/server/connectivity/Vpn.java @@ -18,10 +18,7 @@ package com.android.server.connectivity; import static android.Manifest.permission.BIND_VPN_SERVICE; import static android.net.ConnectivityManager.NETID_UNSET; -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.RouteInfo.RTN_THROW; import static android.net.RouteInfo.RTN_UNREACHABLE; @@ -111,7 +108,6 @@ import com.android.internal.net.LegacyVpnInfo; import com.android.internal.net.VpnConfig; import com.android.internal.net.VpnInfo; import com.android.internal.net.VpnProfile; -import com.android.internal.util.ArrayUtils; import com.android.server.DeviceIdleInternal; import com.android.server.LocalServices; import com.android.server.net.BaseNetworkObserver; @@ -462,99 +458,6 @@ public class Vpn { updateAlwaysOnNotification(detailedState); } - /** - * Updates {@link #mNetworkCapabilities} based on current underlying networks and returns a - * defensive copy. - * - *

Does not propagate updated capabilities to apps. - * - * @param defaultNetwork underlying network for VPNs following platform's default - */ - public synchronized NetworkCapabilities updateCapabilities(@Nullable Network defaultNetwork) { - if (mConfig == null) { - // VPN is not running. - return null; - } - - Network[] underlyingNetworks = mConfig.underlyingNetworks; - if (underlyingNetworks == null && defaultNetwork != null) { - // null underlying networks means to track the default. - underlyingNetworks = new Network[] { defaultNetwork }; - } - // Only apps targeting Q and above can explicitly declare themselves as metered. - final boolean isAlwaysMetered = mIsPackageTargetingAtLeastQ && mConfig.isMetered; - - applyUnderlyingCapabilities( - mConnectivityManager, - underlyingNetworks, - mNetworkCapabilities, - isAlwaysMetered); - - return new NetworkCapabilities(mNetworkCapabilities); - } - - @VisibleForTesting - public static void applyUnderlyingCapabilities( - @NonNull final ConnectivityManager cm, - @Nullable final Network[] underlyingNetworks, - @NonNull final NetworkCapabilities caps, - final boolean isAlwaysMetered) { - int[] transportTypes = new int[] { NetworkCapabilities.TRANSPORT_VPN }; - int downKbps = NetworkCapabilities.LINK_BANDWIDTH_UNSPECIFIED; - int upKbps = NetworkCapabilities.LINK_BANDWIDTH_UNSPECIFIED; - boolean metered = isAlwaysMetered; // metered if any underlying is metered, or alwaysMetered - 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 underlying : underlyingNetworks) { - // TODO(b/124469351): Get capabilities directly from ConnectivityService instead. - final NetworkCapabilities underlyingCaps = cm.getNetworkCapabilities(underlying); - if (underlyingCaps == null) continue; - 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 the safer 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); - } - /** * Chooses whether to force all connections to go though VPN. * diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index c917e66ea49a..978c09ef13d4 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 e1e0efa5cf21..1c1b5e1c66f0 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 * -- cgit v1.2.3-59-g8ed1b From fa24282505f4f61f68d59e591d9778a70a221027 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Tue, 1 Dec 2020 01:08:37 +0900 Subject: 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 --- .../com/android/server/ConnectivityService.java | 23 ++-- .../android/server/ConnectivityServiceTest.java | 121 +++++++++++---------- 2 files changed, 71 insertions(+), 73 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 6ef1b6bbd59f..9b006d050c02 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 978c09ef13d4..878f6fa0f96a 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(); - - 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 NetworkCapabilities initialCaps = new NetworkCapabilities(); + initialCaps.addCapability(NET_CAPABILITY_INTERNET); + initialCaps.removeCapability(NET_CAPABILITY_NOT_VPN); + + 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); + + 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)); - - 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); + assertEquals(withWifiUnderlying, caps); + + caps = new NetworkCapabilities(initialCaps); + mService.applyUnderlyingCapabilities(new Network[]{mobile, wifi}, caps, isDeclaredMetered); + assertEquals(withWifiAndMobileUnderlying, caps); + + withWifiUnderlying.addCapability(NET_CAPABILITY_NOT_METERED); + caps = new NetworkCapabilities(initialCaps); + mService.applyUnderlyingCapabilities(new Network[]{null, mobile, null, wifi}, + caps, notDeclaredMetered); + assertEquals(withWifiAndMobileUnderlying, caps); + + caps = new NetworkCapabilities(initialCaps); mService.applyUnderlyingCapabilities(new Network[]{null, mobile, null, wifi}, - otherCaps, notDeclaredMetered); - assertEquals(otherCaps, caps); + caps, notDeclaredMetered); + assertEquals(withWifiAndMobileUnderlying, caps); + + mService.applyUnderlyingCapabilities(null, caps, notDeclaredMetered); + assertEquals(withWifiUnderlying, caps); } @Test -- cgit v1.2.3-59-g8ed1b