Merge "Fix a bug where VPNs start out suspended on cellular" into rvc-dev am: 9941ef7930

Change-Id: If3e0353505e89ffb4b6ae2fe49e11639f20e293e

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

@@ -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(
+    public synchronized NetworkCapabilities updateCapabilities(@Nullable Network defaultNetwork) {
-            @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
+        boolean metered = isAlwaysMetered; // metered if any underlying is metered, or alwaysMetered
-        // networks.
+        boolean roaming = false; // roaming if any underlying is roaming
-        boolean metered = isAlwaysMetered;
+        boolean congested = false; // congested if any underlying is congested
-        boolean roaming = false;
+        boolean suspended = true; // suspended if all underlying are suspended
-        boolean congested = false;
 
         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
+                // Merge capabilities of this underlying network. For bandwidth, assume the
-                // worst-case link speed and restrictions.
+                // 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

@@ -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

@@ -5415,6 +5415,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();
@@ -5448,6 +5489,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(
@@ -5456,10 +5498,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(
@@ -5468,7 +5512,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(
@@ -5477,16 +5522,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));
 
-        // Use Wifi but not cell. Note the VPN is now unmetered.
+        // 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);
+
+        // 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(
@@ -5495,7 +5560,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

@@ -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));
     }
 
     /**