From 6adf5ac19b0277139a6d951a08fac181714e4f5d Mon Sep 17 00:00:00 2001 From: lucaslin Date: Tue, 19 Oct 2021 15:04:56 +0800 Subject: [PATCH] Add underlying networks into NetworkAgentInfo if any Now, VPN will set underlying networks into NetworkCapabilities directly. So the declaredUnderlyingNetworks can also be set directly when creating a NetworkAgentInfo. Bug: 191918368 Test: atest FrameworksNetTests:ConnectivityServiceTest Change-Id: I507072d00ae1eb0c391e5261ab93e359b9c4cb5c --- .../src/android/net/NetworkCapabilities.java | 7 +- .../android/server/ConnectivityService.java | 5 + .../server/connectivity/NetworkAgentInfo.java | 3 + .../server/ConnectivityServiceTest.java | 183 ++++++++++++++---- 4 files changed, 155 insertions(+), 43 deletions(-) diff --git a/framework/src/android/net/NetworkCapabilities.java b/framework/src/android/net/NetworkCapabilities.java index e9bcd95a5e..51c5585e87 100644 --- a/framework/src/android/net/NetworkCapabilities.java +++ b/framework/src/android/net/NetworkCapabilities.java @@ -2129,14 +2129,17 @@ public final class NetworkCapabilities implements Parcelable { sb.append(" SubscriptionIds: ").append(mSubIds); } - if (mUnderlyingNetworks != null && mUnderlyingNetworks.size() > 0) { - sb.append(" Underlying networks: ["); + sb.append(" UnderlyingNetworks: "); + if (mUnderlyingNetworks != null) { + sb.append("["); final StringJoiner joiner = new StringJoiner(","); for (int i = 0; i < mUnderlyingNetworks.size(); i++) { joiner.add(mUnderlyingNetworks.get(i).toString()); } sb.append(joiner.toString()); sb.append("]"); + } else { + sb.append("Null"); } sb.append("]"); diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 408dba3f9b..88cd876b2c 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -2032,6 +2032,7 @@ public class ConnectivityService extends IConnectivityManager.Stub if (!checkSettingsPermission(callerPid, callerUid)) { newNc.setUids(null); newNc.setSSID(null); + newNc.setUnderlyingNetworks(null); } if (newNc.getNetworkSpecifier() != null) { newNc.setNetworkSpecifier(newNc.getNetworkSpecifier().redact()); @@ -7305,7 +7306,9 @@ public class ConnectivityService extends IConnectivityManager.Stub boolean suspended = true; // suspended if all underlying are suspended boolean hadUnderlyingNetworks = false; + ArrayList newUnderlyingNetworks = null; if (null != underlyingNetworks) { + newUnderlyingNetworks = new ArrayList<>(); for (Network underlyingNetwork : underlyingNetworks) { final NetworkAgentInfo underlying = getNetworkAgentInfoForNetwork(underlyingNetwork); @@ -7335,6 +7338,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // 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); + newUnderlyingNetworks.add(underlyingNetwork); } } if (!hadUnderlyingNetworks) { @@ -7352,6 +7356,7 @@ public class ConnectivityService extends IConnectivityManager.Stub newNc.setCapability(NET_CAPABILITY_NOT_ROAMING, !roaming); newNc.setCapability(NET_CAPABILITY_NOT_CONGESTED, !congested); newNc.setCapability(NET_CAPABILITY_NOT_SUSPENDED, !suspended); + newNc.setUnderlyingNetworks(newUnderlyingNetworks); } /** diff --git a/service/src/com/android/server/connectivity/NetworkAgentInfo.java b/service/src/com/android/server/connectivity/NetworkAgentInfo.java index 6426f86070..b7f3ed98d2 100644 --- a/service/src/com/android/server/connectivity/NetworkAgentInfo.java +++ b/service/src/com/android/server/connectivity/NetworkAgentInfo.java @@ -377,6 +377,9 @@ public class NetworkAgentInfo implements Comparable, NetworkRa this.creatorUid = creatorUid; mLingerDurationMs = lingerDurationMs; mQosCallbackTracker = qosCallbackTracker; + declaredUnderlyingNetworks = (nc.getUnderlyingNetworks() != null) + ? nc.getUnderlyingNetworks().toArray(new Network[0]) + : null; } private class AgentDeathMonitor implements IBinder.DeathRecipient { diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index 352a468833..c54a11eb81 100644 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -151,6 +151,7 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.junit.Assume.assumeTrue; import static org.mockito.AdditionalMatchers.aryEq; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyLong; @@ -319,6 +320,7 @@ import com.android.internal.net.VpnProfile; import com.android.internal.util.WakeupMessage; import com.android.internal.util.test.BroadcastInterceptingContext; import com.android.internal.util.test.FakeSettingsProvider; +import com.android.modules.utils.build.SdkLevel; import com.android.net.module.util.ArrayTrackRecord; import com.android.net.module.util.CollectionUtils; import com.android.net.module.util.LocationPermissionChecker; @@ -7252,6 +7254,15 @@ public class ConnectivityServiceTest { initialCaps.addTransportType(TRANSPORT_VPN); initialCaps.addCapability(NET_CAPABILITY_INTERNET); initialCaps.removeCapability(NET_CAPABILITY_NOT_VPN); + final ArrayList emptyUnderlyingNetworks = new ArrayList(); + final ArrayList underlyingNetworksContainMobile = new ArrayList(); + underlyingNetworksContainMobile.add(mobile); + final ArrayList underlyingNetworksContainWifi = new ArrayList(); + underlyingNetworksContainWifi.add(wifi); + final ArrayList underlyingNetworksContainMobileAndMobile = + new ArrayList(); + underlyingNetworksContainMobileAndMobile.add(mobile); + underlyingNetworksContainMobileAndMobile.add(wifi); final NetworkCapabilities withNoUnderlying = new NetworkCapabilities(); withNoUnderlying.addCapability(NET_CAPABILITY_INTERNET); @@ -7260,17 +7271,20 @@ public class ConnectivityServiceTest { withNoUnderlying.addCapability(NET_CAPABILITY_NOT_SUSPENDED); withNoUnderlying.addTransportType(TRANSPORT_VPN); withNoUnderlying.removeCapability(NET_CAPABILITY_NOT_VPN); + withNoUnderlying.setUnderlyingNetworks(emptyUnderlyingNetworks); 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); + withMobileUnderlying.setUnderlyingNetworks(underlyingNetworksContainMobile); final NetworkCapabilities withWifiUnderlying = new NetworkCapabilities(withNoUnderlying); withWifiUnderlying.addTransportType(TRANSPORT_WIFI); withWifiUnderlying.addCapability(NET_CAPABILITY_NOT_METERED); withWifiUnderlying.setLinkUpstreamBandwidthKbps(20); + withWifiUnderlying.setUnderlyingNetworks(underlyingNetworksContainWifi); final NetworkCapabilities withWifiAndMobileUnderlying = new NetworkCapabilities(withNoUnderlying); @@ -7280,6 +7294,7 @@ public class ConnectivityServiceTest { withWifiAndMobileUnderlying.removeCapability(NET_CAPABILITY_NOT_ROAMING); withWifiAndMobileUnderlying.setLinkDownstreamBandwidthKbps(10); withWifiAndMobileUnderlying.setLinkUpstreamBandwidthKbps(20); + withWifiAndMobileUnderlying.setUnderlyingNetworks(underlyingNetworksContainMobileAndMobile); final NetworkCapabilities initialCapsNotMetered = new NetworkCapabilities(initialCaps); initialCapsNotMetered.addCapability(NET_CAPABILITY_NOT_METERED); @@ -7287,40 +7302,61 @@ public class ConnectivityServiceTest { NetworkCapabilities caps = new NetworkCapabilities(initialCaps); mService.applyUnderlyingCapabilities(new Network[]{}, initialCapsNotMetered, caps); assertEquals(withNoUnderlying, caps); + assertEquals(0, new ArrayList<>(caps.getUnderlyingNetworks()).size()); caps = new NetworkCapabilities(initialCaps); mService.applyUnderlyingCapabilities(new Network[]{null}, initialCapsNotMetered, caps); assertEquals(withNoUnderlying, caps); + assertEquals(0, new ArrayList<>(caps.getUnderlyingNetworks()).size()); caps = new NetworkCapabilities(initialCaps); mService.applyUnderlyingCapabilities(new Network[]{mobile}, initialCapsNotMetered, caps); assertEquals(withMobileUnderlying, caps); + assertEquals(1, new ArrayList<>(caps.getUnderlyingNetworks()).size()); + assertEquals(mobile, new ArrayList<>(caps.getUnderlyingNetworks()).get(0)); + caps = new NetworkCapabilities(initialCaps); mService.applyUnderlyingCapabilities(new Network[]{wifi}, initialCapsNotMetered, caps); assertEquals(withWifiUnderlying, caps); + assertEquals(1, new ArrayList<>(caps.getUnderlyingNetworks()).size()); + assertEquals(wifi, new ArrayList<>(caps.getUnderlyingNetworks()).get(0)); withWifiUnderlying.removeCapability(NET_CAPABILITY_NOT_METERED); caps = new NetworkCapabilities(initialCaps); mService.applyUnderlyingCapabilities(new Network[]{wifi}, initialCaps, caps); assertEquals(withWifiUnderlying, caps); + assertEquals(1, new ArrayList<>(caps.getUnderlyingNetworks()).size()); + assertEquals(wifi, new ArrayList<>(caps.getUnderlyingNetworks()).get(0)); caps = new NetworkCapabilities(initialCaps); mService.applyUnderlyingCapabilities(new Network[]{mobile, wifi}, initialCaps, caps); assertEquals(withWifiAndMobileUnderlying, caps); + assertEquals(2, new ArrayList<>(caps.getUnderlyingNetworks()).size()); + assertEquals(mobile, new ArrayList<>(caps.getUnderlyingNetworks()).get(0)); + assertEquals(wifi, new ArrayList<>(caps.getUnderlyingNetworks()).get(1)); withWifiUnderlying.addCapability(NET_CAPABILITY_NOT_METERED); caps = new NetworkCapabilities(initialCaps); mService.applyUnderlyingCapabilities(new Network[]{null, mobile, null, wifi}, initialCapsNotMetered, caps); assertEquals(withWifiAndMobileUnderlying, caps); + assertEquals(2, new ArrayList<>(caps.getUnderlyingNetworks()).size()); + assertEquals(mobile, new ArrayList<>(caps.getUnderlyingNetworks()).get(0)); + assertEquals(wifi, new ArrayList<>(caps.getUnderlyingNetworks()).get(1)); caps = new NetworkCapabilities(initialCaps); mService.applyUnderlyingCapabilities(new Network[]{null, mobile, null, wifi}, initialCapsNotMetered, caps); assertEquals(withWifiAndMobileUnderlying, caps); + assertEquals(2, new ArrayList<>(caps.getUnderlyingNetworks()).size()); + assertEquals(mobile, new ArrayList<>(caps.getUnderlyingNetworks()).get(0)); + assertEquals(wifi, new ArrayList<>(caps.getUnderlyingNetworks()).get(1)); + caps = new NetworkCapabilities(initialCaps); mService.applyUnderlyingCapabilities(null, initialCapsNotMetered, caps); assertEquals(withWifiUnderlying, caps); + assertEquals(1, new ArrayList<>(caps.getUnderlyingNetworks()).size()); + assertEquals(wifi, new ArrayList<>(caps.getUnderlyingNetworks()).get(0)); } @Test @@ -7329,51 +7365,78 @@ public class ConnectivityServiceTest { final NetworkRequest request = new NetworkRequest.Builder() .removeCapability(NET_CAPABILITY_NOT_VPN).build(); - mCm.registerNetworkCallback(request, callback); + runAsShell(NETWORK_SETTINGS, () -> { + mCm.registerNetworkCallback(request, callback); - // Bring up a VPN that specifies an underlying network that does not exist yet. - // Note: it's sort of meaningless for a VPN app to declare a network that doesn't exist yet, - // (and doing so is difficult without using reflection) but it's good to test that the code - // behaves approximately correctly. - mMockVpn.establishForMyUid(false, true, false); - assertUidRangesUpdatedForMyUid(true); - final Network wifiNetwork = new Network(mNetIdManager.peekNextNetId()); - mMockVpn.setUnderlyingNetworks(new Network[]{wifiNetwork}); - callback.expectAvailableCallbacksUnvalidated(mMockVpn); - assertTrue(mCm.getNetworkCapabilities(mMockVpn.getNetwork()) - .hasTransport(TRANSPORT_VPN)); - assertFalse(mCm.getNetworkCapabilities(mMockVpn.getNetwork()) - .hasTransport(TRANSPORT_WIFI)); + // Bring up a VPN that specifies an underlying network that does not exist yet. + // Note: it's sort of meaningless for a VPN app to declare a network that doesn't exist + // yet, (and doing so is difficult without using reflection) but it's good to test that + // the code behaves approximately correctly. + mMockVpn.establishForMyUid(false, true, false); + callback.expectAvailableCallbacksUnvalidated(mMockVpn); + assertUidRangesUpdatedForMyUid(true); + final Network wifiNetwork = new Network(mNetIdManager.peekNextNetId()); + mMockVpn.setUnderlyingNetworks(new Network[]{wifiNetwork}); + // onCapabilitiesChanged() should be called because + // NetworkCapabilities#mUnderlyingNetworks is updated. + CallbackEntry ce = callback.expectCallback(CallbackEntry.NETWORK_CAPS_UPDATED, + mMockVpn); + final NetworkCapabilities vpnNc1 = ((CallbackEntry.CapabilitiesChanged) ce).getCaps(); + // Since the wifi network hasn't brought up, + // ConnectivityService#applyUnderlyingCapabilities cannot find it. Update + // NetworkCapabilities#mUnderlyingNetworks to an empty array, and it will be updated to + // the correct underlying networks once the wifi network brings up. But this case + // shouldn't happen in reality since no one could get the network which hasn't brought + // up. For the empty array of underlying networks, it should be happened for 2 cases, + // the first one is that the VPN app declares an empty array for its underlying + // networks, the second one is that the underlying networks are torn down. + // + // It shouldn't be null since the null value means the underlying networks of this + // network should follow the default network. + final ArrayList underlyingNetwork = new ArrayList<>(); + assertEquals(underlyingNetwork, vpnNc1.getUnderlyingNetworks()); + // Since the wifi network isn't exist, applyUnderlyingCapabilities() + assertTrue(mCm.getNetworkCapabilities(mMockVpn.getNetwork()) + .hasTransport(TRANSPORT_VPN)); + assertFalse(mCm.getNetworkCapabilities(mMockVpn.getNetwork()) + .hasTransport(TRANSPORT_WIFI)); - // Make that underlying network connect, and expect to see its capabilities immediately - // reflected in the VPN's capabilities. - mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI); - assertEquals(wifiNetwork, mWiFiNetworkAgent.getNetwork()); - mWiFiNetworkAgent.connect(false); - // TODO: the callback for the VPN happens before any callbacks are called for the wifi - // network that has just connected. There appear to be two issues here: - // 1. The VPN code will accept an underlying network as soon as getNetworkCapabilities() for - // it returns non-null (which happens very early, during handleRegisterNetworkAgent). - // This is not correct because that that point the network is not connected and cannot - // pass any traffic. - // 2. When a network connects, updateNetworkInfo propagates underlying network capabilities - // before rematching networks. - // Given that this scenario can't really happen, this is probably fine for now. - callback.expectCallback(CallbackEntry.NETWORK_CAPS_UPDATED, mMockVpn); - callback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); - assertTrue(mCm.getNetworkCapabilities(mMockVpn.getNetwork()) - .hasTransport(TRANSPORT_VPN)); - assertTrue(mCm.getNetworkCapabilities(mMockVpn.getNetwork()) - .hasTransport(TRANSPORT_WIFI)); + // Make that underlying network connect, and expect to see its capabilities immediately + // reflected in the VPN's capabilities. + mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI); + assertEquals(wifiNetwork, mWiFiNetworkAgent.getNetwork()); + mWiFiNetworkAgent.connect(false); + // TODO: the callback for the VPN happens before any callbacks are called for the wifi + // network that has just connected. There appear to be two issues here: + // 1. The VPN code will accept an underlying network as soon as getNetworkCapabilities() + // for it returns non-null (which happens very early, during + // handleRegisterNetworkAgent). + // This is not correct because that that point the network is not connected and + // cannot pass any traffic. + // 2. When a network connects, updateNetworkInfo propagates underlying network + // capabilities before rematching networks. + // Given that this scenario can't really happen, this is probably fine for now. + ce = callback.expectCallback(CallbackEntry.NETWORK_CAPS_UPDATED, mMockVpn); + final NetworkCapabilities vpnNc2 = ((CallbackEntry.CapabilitiesChanged) ce).getCaps(); + // The wifi network is brought up, NetworkCapabilities#mUnderlyingNetworks is updated to + // it. + underlyingNetwork.add(wifiNetwork); + assertEquals(underlyingNetwork, vpnNc2.getUnderlyingNetworks()); + callback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); + assertTrue(mCm.getNetworkCapabilities(mMockVpn.getNetwork()) + .hasTransport(TRANSPORT_VPN)); + assertTrue(mCm.getNetworkCapabilities(mMockVpn.getNetwork()) + .hasTransport(TRANSPORT_WIFI)); - // Disconnect the network, and expect to see the VPN capabilities change accordingly. - mWiFiNetworkAgent.disconnect(); - callback.expectCallback(CallbackEntry.LOST, mWiFiNetworkAgent); - callback.expectCapabilitiesThat(mMockVpn, (nc) -> - nc.getTransportTypes().length == 1 && nc.hasTransport(TRANSPORT_VPN)); + // Disconnect the network, and expect to see the VPN capabilities change accordingly. + mWiFiNetworkAgent.disconnect(); + callback.expectCallback(CallbackEntry.LOST, mWiFiNetworkAgent); + callback.expectCapabilitiesThat(mMockVpn, (nc) -> + nc.getTransportTypes().length == 1 && nc.hasTransport(TRANSPORT_VPN)); - mMockVpn.disconnect(); - mCm.unregisterNetworkCallback(callback); + mMockVpn.disconnect(); + mCm.unregisterNetworkCallback(callback); + }); } private void assertGetNetworkInfoOfGetActiveNetworkIsConnected(boolean expectedConnectivity) { @@ -10697,6 +10760,14 @@ public class ConnectivityServiceTest { return fakeNai(wifiNc, info); } + private NetworkAgentInfo fakeVpnNai(NetworkCapabilities nc) { + final NetworkCapabilities vpnNc = new NetworkCapabilities.Builder(nc) + .addTransportType(TRANSPORT_VPN).build(); + final NetworkInfo info = new NetworkInfo(TYPE_VPN, 0 /* subtype */, + ConnectivityManager.getNetworkTypeName(TYPE_VPN), "" /* subtypeName */); + return fakeNai(vpnNc, info); + } + private NetworkAgentInfo fakeNai(NetworkCapabilities nc, NetworkInfo networkInfo) { return new NetworkAgentInfo(null, new Network(NET_ID), networkInfo, new LinkProperties(), nc, new NetworkScore.Builder().setLegacyInt(0).build(), @@ -10830,6 +10901,36 @@ public class ConnectivityServiceTest { mContext.getOpPackageName())); } + @Test + public void testUnderlyingNetworksWillBeSetInNetworkAgentInfoConstructor() throws Exception { + assumeTrue(SdkLevel.isAtLeastT()); + final Network network1 = new Network(100); + final Network network2 = new Network(101); + final List underlyingNetworks = new ArrayList<>(); + final NetworkCapabilities ncWithEmptyUnderlyingNetworks = new NetworkCapabilities.Builder() + .setUnderlyingNetworks(underlyingNetworks) + .build(); + final NetworkAgentInfo vpnNaiWithEmptyUnderlyingNetworks = + fakeVpnNai(ncWithEmptyUnderlyingNetworks); + assertEquals(underlyingNetworks, + Arrays.asList(vpnNaiWithEmptyUnderlyingNetworks.declaredUnderlyingNetworks)); + + underlyingNetworks.add(network1); + underlyingNetworks.add(network2); + final NetworkCapabilities ncWithUnderlyingNetworks = new NetworkCapabilities.Builder() + .setUnderlyingNetworks(underlyingNetworks) + .build(); + final NetworkAgentInfo vpnNaiWithUnderlyingNetwokrs = fakeVpnNai(ncWithUnderlyingNetworks); + assertEquals(underlyingNetworks, + Arrays.asList(vpnNaiWithUnderlyingNetwokrs.declaredUnderlyingNetworks)); + + final NetworkCapabilities ncWithoutUnderlyingNetworks = new NetworkCapabilities.Builder() + .build(); + final NetworkAgentInfo vpnNaiWithoutUnderlyingNetwokrs = + fakeVpnNai(ncWithoutUnderlyingNetworks); + assertNull(vpnNaiWithoutUnderlyingNetwokrs.declaredUnderlyingNetworks); + } + @Test public void testRegisterConnectivityDiagnosticsCallbackCallsOnConnectivityReport() throws Exception {