Merge "Fix a bug where VPNs start out suspended on cellular" (bd3a7f99) · Commits · e / os / android_frameworks_base

services/core/java/com/android/server/connectivity/Vpn.java

+19 −9

Original line number	Diff line number	Diff line
		@@ -21,6 +21,7 @@ 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;

		@@ -317,8 +318,7 @@ public class Vpn {
		*
		* @param defaultNetwork underlying network for VPNs following platform's default
		*/
		public synchronized NetworkCapabilities updateCapabilities(
		@Nullable Network defaultNetwork) {
		public synchronized NetworkCapabilities updateCapabilities(@Nullable Network defaultNetwork) {
		if (mConfig == null) {
		// VPN is not running.
		return null;
		@@ -350,11 +350,10 @@ public class Vpn {
		int[] transportTypes = new int[] { NetworkCapabilities.TRANSPORT_VPN };
		int downKbps = NetworkCapabilities.LINK_BANDWIDTH_UNSPECIFIED;
		int upKbps = NetworkCapabilities.LINK_BANDWIDTH_UNSPECIFIED;
		// VPN's meteredness is OR'd with isAlwaysMetered and meteredness of its underlying
		// networks.
		boolean metered = isAlwaysMetered;
		boolean roaming = false;
		boolean congested = false;
		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) {
		@@ -367,15 +366,24 @@ public class Vpn {
		transportTypes = ArrayUtils.appendInt(transportTypes, underlyingType);
		}

		// When we have multiple networks, we have to assume the
		// worst-case link speed and restrictions.
		// 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) {
		@@ -383,6 +391,7 @@ public class Vpn {
		metered = true;
		roaming = false;
		congested = false;
		suspended = false;
		}

		caps.setTransportTypes(transportTypes);
		@@ -391,6 +400,7 @@ public class Vpn {
		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);
		}

		/**

tests/net/integration/util/com/android/server/NetworkAgentWrapper.java

+3 −0

Original line number	Diff line number	Diff line
		@@ -92,6 +92,9 @@ public class NetworkAgentWrapper implements TestableNetworkCallback.HasNetwork {
		break;
		case TRANSPORT_VPN:
		mNetworkCapabilities.removeCapability(NET_CAPABILITY_NOT_VPN);
		// VPNs deduce the SUSPENDED capability from their underlying networks and there
		// is no public API to let VPN services set it.
		mNetworkCapabilities.removeCapability(NET_CAPABILITY_NOT_SUSPENDED);
		mScore = ConnectivityConstants.VPN_DEFAULT_SCORE;
		break;
		default:

tests/net/java/com/android/server/ConnectivityServiceTest.java

+101 −6

Original line number	Diff line number	Diff line
		@@ -5422,6 +5422,47 @@ public class ConnectivityServiceTest {
		callback.expectAvailableCallbacksValidated(mEthernetNetworkAgent);
		}

		@Test
		public void testVpnStartsWithUnderlyingCaps() throws Exception {
		final int uid = Process.myUid();

		final TestNetworkCallback vpnNetworkCallback = new TestNetworkCallback();
		final NetworkRequest vpnNetworkRequest = new NetworkRequest.Builder()
		.removeCapability(NET_CAPABILITY_NOT_VPN)
		.addTransportType(TRANSPORT_VPN)
		.build();
		mCm.registerNetworkCallback(vpnNetworkRequest, vpnNetworkCallback);
		vpnNetworkCallback.assertNoCallback();

		// Connect cell. It will become the default network, and in the absence of setting
		// underlying networks explicitly it will become the sole underlying network for the vpn.
		mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR);
		mCellNetworkAgent.addCapability(NET_CAPABILITY_NOT_SUSPENDED);
		mCellNetworkAgent.connect(true);

		final TestNetworkAgentWrapper vpnNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_VPN);
		final ArraySet<UidRange> ranges = new ArraySet<>();
		ranges.add(new UidRange(uid, uid));
		mMockVpn.setNetworkAgent(vpnNetworkAgent);
		mMockVpn.connect();
		mMockVpn.setUids(ranges);
		vpnNetworkAgent.connect(true /* validated /, false / hasInternet */,
		false /* isStrictMode */);

		vpnNetworkCallback.expectAvailableCallbacks(vpnNetworkAgent.getNetwork(),
		false /* suspended /, false / validated /, false / blocked */, TIMEOUT_MS);
		vpnNetworkCallback.expectCapabilitiesThat(vpnNetworkAgent.getNetwork(), TIMEOUT_MS,
		nc -> nc.hasCapability(NET_CAPABILITY_VALIDATED));

		final NetworkCapabilities nc = mCm.getNetworkCapabilities(vpnNetworkAgent.getNetwork());
		assertTrue(nc.hasTransport(TRANSPORT_VPN));
		assertTrue(nc.hasTransport(TRANSPORT_CELLULAR));
		assertFalse(nc.hasTransport(TRANSPORT_WIFI));
		assertTrue(nc.hasCapability(NET_CAPABILITY_VALIDATED));
		assertFalse(nc.hasCapability(NET_CAPABILITY_NOT_METERED));
		assertTrue(nc.hasCapability(NET_CAPABILITY_NOT_SUSPENDED));
		}

		@Test
		public void testVpnSetUnderlyingNetworks() throws Exception {
		final int uid = Process.myUid();
		@@ -5455,6 +5496,7 @@ public class ConnectivityServiceTest {

		// Connect cell and use it as an underlying network.
		mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR);
		mCellNetworkAgent.addCapability(NET_CAPABILITY_NOT_SUSPENDED);
		mCellNetworkAgent.connect(true);

		mService.setUnderlyingNetworksForVpn(
		@@ -5463,10 +5505,12 @@ public class ConnectivityServiceTest {
		vpnNetworkCallback.expectCapabilitiesThat(vpnNetworkAgent,
		(caps) -> caps.hasTransport(TRANSPORT_VPN)
		&& caps.hasTransport(TRANSPORT_CELLULAR) && !caps.hasTransport(TRANSPORT_WIFI)
		&& !caps.hasCapability(NET_CAPABILITY_NOT_METERED));
		&& !caps.hasCapability(NET_CAPABILITY_NOT_METERED)
		&& caps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED));

		mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI);
		mWiFiNetworkAgent.addCapability(NET_CAPABILITY_NOT_METERED);
		mWiFiNetworkAgent.addCapability(NET_CAPABILITY_NOT_SUSPENDED);
		mWiFiNetworkAgent.connect(true);

		mService.setUnderlyingNetworksForVpn(
		@@ -5475,7 +5519,8 @@ public class ConnectivityServiceTest {
		vpnNetworkCallback.expectCapabilitiesThat(vpnNetworkAgent,
		(caps) -> caps.hasTransport(TRANSPORT_VPN)
		&& caps.hasTransport(TRANSPORT_CELLULAR) && caps.hasTransport(TRANSPORT_WIFI)
		&& !caps.hasCapability(NET_CAPABILITY_NOT_METERED));
		&& !caps.hasCapability(NET_CAPABILITY_NOT_METERED)
		&& caps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED));

		// Don't disconnect, but note the VPN is not using wifi any more.
		mService.setUnderlyingNetworksForVpn(
		@@ -5484,16 +5529,36 @@ public class ConnectivityServiceTest {
		vpnNetworkCallback.expectCapabilitiesThat(vpnNetworkAgent,
		(caps) -> caps.hasTransport(TRANSPORT_VPN)
		&& caps.hasTransport(TRANSPORT_CELLULAR) && !caps.hasTransport(TRANSPORT_WIFI)
		&& !caps.hasCapability(NET_CAPABILITY_NOT_METERED));
		&& !caps.hasCapability(NET_CAPABILITY_NOT_METERED)
		&& caps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED));

		// Remove NOT_SUSPENDED from the only network and observe VPN is now suspended.
		mCellNetworkAgent.removeCapability(NET_CAPABILITY_NOT_SUSPENDED);
		vpnNetworkCallback.expectCapabilitiesThat(vpnNetworkAgent,
		(caps) -> caps.hasTransport(TRANSPORT_VPN)
		&& caps.hasTransport(TRANSPORT_CELLULAR) && !caps.hasTransport(TRANSPORT_WIFI)
		&& !caps.hasCapability(NET_CAPABILITY_NOT_METERED)
		&& !caps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED));
		vpnNetworkCallback.expectCallback(CallbackEntry.SUSPENDED, vpnNetworkAgent);

		// Use Wifi but not cell. Note the VPN is now unmetered.
		// Add NOT_SUSPENDED again and observe VPN is no longer suspended.
		mCellNetworkAgent.addCapability(NET_CAPABILITY_NOT_SUSPENDED);
		vpnNetworkCallback.expectCapabilitiesThat(vpnNetworkAgent,
		(caps) -> caps.hasTransport(TRANSPORT_VPN)
		&& caps.hasTransport(TRANSPORT_CELLULAR) && !caps.hasTransport(TRANSPORT_WIFI)
		&& !caps.hasCapability(NET_CAPABILITY_NOT_METERED)
		&& caps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED));
		vpnNetworkCallback.expectCallback(CallbackEntry.RESUMED, vpnNetworkAgent);

		// Use Wifi but not cell. Note the VPN is now unmetered and not suspended.
		mService.setUnderlyingNetworksForVpn(
		new Network[] { mWiFiNetworkAgent.getNetwork() });

		vpnNetworkCallback.expectCapabilitiesThat(vpnNetworkAgent,
		(caps) -> caps.hasTransport(TRANSPORT_VPN)
		&& !caps.hasTransport(TRANSPORT_CELLULAR) && caps.hasTransport(TRANSPORT_WIFI)
		&& caps.hasCapability(NET_CAPABILITY_NOT_METERED));
		&& caps.hasCapability(NET_CAPABILITY_NOT_METERED)
		&& caps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED));

		// Use both again.
		mService.setUnderlyingNetworksForVpn(
		@@ -5502,7 +5567,37 @@ public class ConnectivityServiceTest {
		vpnNetworkCallback.expectCapabilitiesThat(vpnNetworkAgent,
		(caps) -> caps.hasTransport(TRANSPORT_VPN)
		&& caps.hasTransport(TRANSPORT_CELLULAR) && caps.hasTransport(TRANSPORT_WIFI)
		&& !caps.hasCapability(NET_CAPABILITY_NOT_METERED));
		&& !caps.hasCapability(NET_CAPABILITY_NOT_METERED)
		&& caps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED));

		// Cell is suspended again. As WiFi is not, this should not cause a callback.
		mCellNetworkAgent.removeCapability(NET_CAPABILITY_NOT_SUSPENDED);
		vpnNetworkCallback.assertNoCallback();

		// Stop using WiFi. The VPN is suspended again.
		mService.setUnderlyingNetworksForVpn(
		new Network[] { mCellNetworkAgent.getNetwork() });
		vpnNetworkCallback.expectCapabilitiesThat(vpnNetworkAgent,
		(caps) -> caps.hasTransport(TRANSPORT_VPN)
		&& caps.hasTransport(TRANSPORT_CELLULAR)
		&& !caps.hasCapability(NET_CAPABILITY_NOT_METERED)
		&& !caps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED));
		// While the SUSPENDED callback should in theory be sent here, it is not. This is
		// a bug in ConnectivityService, but as the SUSPENDED and RESUMED callbacks have never
		// been public and are deprecated and slated for removal, there is no sense in spending
		// resources fixing this bug now.

		// Use both again.
		mService.setUnderlyingNetworksForVpn(
		new Network[] { mCellNetworkAgent.getNetwork(), mWiFiNetworkAgent.getNetwork() });

		vpnNetworkCallback.expectCapabilitiesThat(vpnNetworkAgent,
		(caps) -> caps.hasTransport(TRANSPORT_VPN)
		&& caps.hasTransport(TRANSPORT_CELLULAR) && caps.hasTransport(TRANSPORT_WIFI)
		&& !caps.hasCapability(NET_CAPABILITY_NOT_METERED)
		&& caps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED));
		// As above, the RESUMED callback not being sent here is a bug, but not a bug that's
		// worth anybody's time to fix.

		// Disconnect cell. Receive update without even removing the dead network from the
		// underlying networks – it's dead anyway. Not metered any more.

tests/net/java/com/android/server/connectivity/VpnTest.java

+7 −0

Original line number	Diff line number	Diff line
		@@ -25,6 +25,7 @@ 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;
		@@ -606,6 +607,7 @@ public class VpnTest {
		.addCapability(NET_CAPABILITY_NOT_METERED)
		.addCapability(NET_CAPABILITY_NOT_ROAMING)
		.addCapability(NET_CAPABILITY_NOT_CONGESTED)
		.addCapability(NET_CAPABILITY_NOT_SUSPENDED)
		.setLinkUpstreamBandwidthKbps(20));
		setMockedNetworks(networks);

		@@ -621,6 +623,7 @@ public class VpnTest {
		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,
		@@ -635,6 +638,7 @@ public class VpnTest {
		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 */);
		@@ -646,6 +650,7 @@ public class VpnTest {
		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 */);
		@@ -657,6 +662,7 @@ public class VpnTest {
		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,
		@@ -671,6 +677,7 @@ public class VpnTest {
		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));
		}

		/**