Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge "Fix a bug where VPNs start out suspended on cellular" am: bd3a7f994d am: c84f5a0713 am: 983f827366 am: c4e3c258b6

Browse Source 
Change-Id: Ib63e5ce0f1e986c7d4bd4895e36a34ef94f62a57
This commit is contained in:
Chalard Jean
2020-05-11 05:25:52 +00:00
committed by Automerger Merge Worker
parent e38d71d745 c4e3c258b6
commit 8a05c86ded
4 changed files with 130 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
services/core/java/com/android/server/connectivity/Vpn.java
View File

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

3
tests/net/integration/util/com/android/server/NetworkAgentWrapper.java
View File

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

107
tests/net/java/com/android/server/ConnectivityServiceTest.java
View File

@@ -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));
// 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(
@@ -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.

7
tests/net/java/com/android/server/connectivity/VpnTest.java
View File

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