Merge changes I475502fd,Ibf376a6f,Ia52f9caf
* changes: Address comments on aosp/1539753, aosp/1542487 and aosp/1547496. Fix propagating underlying caps when a network disconnects. Test for bugs with suspended VPN underlying networks.
This commit is contained in:
@@ -1389,9 +1389,6 @@ public class ConnectivityServiceTest {
|
||||
verify(mNetworkPolicyManager).registerListener(policyListenerCaptor.capture());
|
||||
mPolicyListener = policyListenerCaptor.getValue();
|
||||
|
||||
mServiceContext.setPermission(
|
||||
Manifest.permission.CONTROL_VPN, PERMISSION_GRANTED);
|
||||
|
||||
// Create local CM before sending system ready so that we can answer
|
||||
// getSystemService() correctly.
|
||||
mCm = new WrappedConnectivityManager(InstrumentationRegistry.getContext(), mService);
|
||||
@@ -1588,6 +1585,7 @@ public class ConnectivityServiceTest {
|
||||
}
|
||||
|
||||
public void expectNoBroadcast(int timeoutMs) throws Exception {
|
||||
waitForIdle();
|
||||
try {
|
||||
final Intent intent = get(timeoutMs, TimeUnit.MILLISECONDS);
|
||||
fail("Unexpected broadcast: " + intent.getAction() + " " + intent.getExtras());
|
||||
@@ -5898,6 +5896,131 @@ public class ConnectivityServiceTest {
|
||||
mCm.unregisterNetworkCallback(callback);
|
||||
}
|
||||
|
||||
private void assertGetNetworkInfoOfGetActiveNetworkIsConnected(boolean expectedConnectivity) {
|
||||
// What Chromium used to do before https://chromium-review.googlesource.com/2605304
|
||||
assertEquals("Unexpected result for getActiveNetworkInfo(getActiveNetwork())",
|
||||
expectedConnectivity, mCm.getNetworkInfo(mCm.getActiveNetwork()).isConnected());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testVpnUnderlyingNetworkSuspended() throws Exception {
|
||||
final TestNetworkCallback callback = new TestNetworkCallback();
|
||||
mCm.registerDefaultNetworkCallback(callback);
|
||||
|
||||
// Connect a VPN.
|
||||
mMockVpn.establishForMyUid(false /* validated */, true /* hasInternet */,
|
||||
false /* isStrictMode */);
|
||||
callback.expectAvailableCallbacksUnvalidated(mMockVpn);
|
||||
|
||||
// Connect cellular data.
|
||||
mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR);
|
||||
mCellNetworkAgent.connect(false /* validated */);
|
||||
callback.expectCapabilitiesThat(mMockVpn,
|
||||
nc -> nc.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)
|
||||
&& nc.hasTransport(TRANSPORT_CELLULAR));
|
||||
callback.assertNoCallback();
|
||||
|
||||
assertTrue(mCm.getNetworkCapabilities(mMockVpn.getNetwork())
|
||||
.hasCapability(NET_CAPABILITY_NOT_SUSPENDED));
|
||||
assertNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
|
||||
assertNetworkInfo(TYPE_WIFI, DetailedState.DISCONNECTED);
|
||||
assertNetworkInfo(TYPE_VPN, DetailedState.CONNECTED);
|
||||
assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
|
||||
assertGetNetworkInfoOfGetActiveNetworkIsConnected(true);
|
||||
|
||||
// Suspend the cellular network and expect the VPN to be suspended.
|
||||
mCellNetworkAgent.suspend();
|
||||
callback.expectCapabilitiesThat(mMockVpn,
|
||||
nc -> !nc.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)
|
||||
&& nc.hasTransport(TRANSPORT_CELLULAR));
|
||||
callback.expectCallback(CallbackEntry.SUSPENDED, mMockVpn);
|
||||
callback.assertNoCallback();
|
||||
|
||||
assertFalse(mCm.getNetworkCapabilities(mMockVpn.getNetwork())
|
||||
.hasCapability(NET_CAPABILITY_NOT_SUSPENDED));
|
||||
assertNetworkInfo(TYPE_MOBILE, DetailedState.SUSPENDED);
|
||||
assertNetworkInfo(TYPE_WIFI, DetailedState.DISCONNECTED);
|
||||
assertNetworkInfo(TYPE_VPN, DetailedState.SUSPENDED);
|
||||
assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.SUSPENDED);
|
||||
// VPN's main underlying network is suspended, so no connectivity.
|
||||
assertGetNetworkInfoOfGetActiveNetworkIsConnected(false);
|
||||
|
||||
// Switch to another network. The VPN should no longer be suspended.
|
||||
mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI);
|
||||
mWiFiNetworkAgent.connect(false /* validated */);
|
||||
callback.expectCapabilitiesThat(mMockVpn,
|
||||
nc -> nc.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)
|
||||
&& nc.hasTransport(TRANSPORT_WIFI));
|
||||
|
||||
// BUG: the VPN is no longer suspended, so a RESUMED callback should have been sent.
|
||||
// callback.expectCallback(CallbackEntry.RESUMED, mMockVpn);
|
||||
callback.assertNoCallback();
|
||||
|
||||
assertTrue(mCm.getNetworkCapabilities(mMockVpn.getNetwork())
|
||||
.hasCapability(NET_CAPABILITY_NOT_SUSPENDED));
|
||||
assertNetworkInfo(TYPE_MOBILE, DetailedState.DISCONNECTED);
|
||||
assertNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED);
|
||||
assertNetworkInfo(TYPE_VPN, DetailedState.SUSPENDED); // BUG: VPN caps have NOT_SUSPENDED.
|
||||
assertActiveNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED);
|
||||
// BUG: the device has connectivity, so this should return true.
|
||||
assertGetNetworkInfoOfGetActiveNetworkIsConnected(false);
|
||||
|
||||
// Unsuspend cellular and then switch back to it.
|
||||
// The same bug happens in the opposite direction: the VPN's capabilities correctly have
|
||||
// NOT_SUSPENDED, but the VPN's NetworkInfo is in state SUSPENDED.
|
||||
mCellNetworkAgent.resume();
|
||||
callback.assertNoCallback();
|
||||
mWiFiNetworkAgent.disconnect();
|
||||
callback.expectCapabilitiesThat(mMockVpn,
|
||||
nc -> nc.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)
|
||||
&& nc.hasTransport(TRANSPORT_CELLULAR));
|
||||
// Spurious double callback?
|
||||
callback.expectCapabilitiesThat(mMockVpn,
|
||||
nc -> nc.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)
|
||||
&& nc.hasTransport(TRANSPORT_CELLULAR));
|
||||
callback.assertNoCallback();
|
||||
|
||||
assertTrue(mCm.getNetworkCapabilities(mMockVpn.getNetwork())
|
||||
.hasCapability(NET_CAPABILITY_NOT_SUSPENDED));
|
||||
assertNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
|
||||
assertNetworkInfo(TYPE_WIFI, DetailedState.DISCONNECTED);
|
||||
assertNetworkInfo(TYPE_VPN, DetailedState.SUSPENDED); // BUG: VPN caps have NOT_SUSPENDED.
|
||||
assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
|
||||
// BUG: the device has connectivity, so this should return true.
|
||||
assertGetNetworkInfoOfGetActiveNetworkIsConnected(false);
|
||||
|
||||
// Re-suspending the current network fixes the problem.
|
||||
mCellNetworkAgent.suspend();
|
||||
callback.expectCapabilitiesThat(mMockVpn,
|
||||
nc -> !nc.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)
|
||||
&& nc.hasTransport(TRANSPORT_CELLULAR));
|
||||
callback.expectCallback(CallbackEntry.SUSPENDED, mMockVpn);
|
||||
callback.assertNoCallback();
|
||||
|
||||
assertFalse(mCm.getNetworkCapabilities(mMockVpn.getNetwork())
|
||||
.hasCapability(NET_CAPABILITY_NOT_SUSPENDED));
|
||||
assertNetworkInfo(TYPE_MOBILE, DetailedState.SUSPENDED);
|
||||
assertNetworkInfo(TYPE_WIFI, DetailedState.DISCONNECTED);
|
||||
assertNetworkInfo(TYPE_VPN, DetailedState.SUSPENDED);
|
||||
assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.SUSPENDED);
|
||||
assertGetNetworkInfoOfGetActiveNetworkIsConnected(false);
|
||||
|
||||
mCellNetworkAgent.resume();
|
||||
callback.expectCapabilitiesThat(mMockVpn,
|
||||
nc -> nc.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)
|
||||
&& nc.hasTransport(TRANSPORT_CELLULAR));
|
||||
callback.expectCallback(CallbackEntry.RESUMED, mMockVpn);
|
||||
callback.assertNoCallback();
|
||||
|
||||
assertTrue(mCm.getNetworkCapabilities(mMockVpn.getNetwork())
|
||||
.hasCapability(NET_CAPABILITY_NOT_SUSPENDED));
|
||||
assertNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
|
||||
assertNetworkInfo(TYPE_WIFI, DetailedState.DISCONNECTED);
|
||||
assertNetworkInfo(TYPE_VPN, DetailedState.CONNECTED);
|
||||
assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
|
||||
assertGetNetworkInfoOfGetActiveNetworkIsConnected(true);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testVpnNetworkActive() throws Exception {
|
||||
final int uid = Process.myUid();
|
||||
@@ -6454,9 +6577,13 @@ public class ConnectivityServiceTest {
|
||||
|
||||
@Test
|
||||
public void testLockdownVpnWithRestrictedProfiles() throws Exception {
|
||||
// NETWORK_SETTINGS is necessary to see the UID ranges in NetworkCapabilities.
|
||||
// For ConnectivityService#setAlwaysOnVpnPackage.
|
||||
mServiceContext.setPermission(
|
||||
Manifest.permission.CONTROL_ALWAYS_ON_VPN, PERMISSION_GRANTED);
|
||||
// For call Vpn#setAlwaysOnPackage.
|
||||
mServiceContext.setPermission(
|
||||
Manifest.permission.CONTROL_VPN, PERMISSION_GRANTED);
|
||||
// Necessary to see the UID ranges in NetworkCapabilities.
|
||||
mServiceContext.setPermission(
|
||||
Manifest.permission.NETWORK_SETTINGS, PERMISSION_GRANTED);
|
||||
|
||||
@@ -7019,6 +7146,9 @@ public class ConnectivityServiceTest {
|
||||
|
||||
@Test
|
||||
public void testLegacyLockdownVpn() throws Exception {
|
||||
mServiceContext.setPermission(
|
||||
Manifest.permission.CONTROL_VPN, PERMISSION_GRANTED);
|
||||
|
||||
final NetworkRequest request = new NetworkRequest.Builder().clearCapabilities().build();
|
||||
final TestNetworkCallback callback = new TestNetworkCallback();
|
||||
mCm.registerNetworkCallback(request, callback);
|
||||
@@ -7136,11 +7266,14 @@ public class ConnectivityServiceTest {
|
||||
mMockVpn.expectStopVpnRunnerPrivileged();
|
||||
mMockVpn.expectStartLegacyVpnRunner();
|
||||
|
||||
// TODO: why is wifi not blocked? Is this because something calls prepare()?
|
||||
// TODO: why is wifi not blocked? Is it because when this callback is sent, the VPN is still
|
||||
// connected, so the network is not considered blocked by the lockdown UID ranges? But the
|
||||
// fact that a VPN is connected should only result in the VPN itself being unblocked, not
|
||||
// any other network. Bug in isUidBlockedByVpn?
|
||||
callback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent);
|
||||
callback.expectCapabilitiesThat(mMockVpn, (nc) -> nc.hasTransport(TRANSPORT_WIFI));
|
||||
defaultCallback.expectCapabilitiesThat(mMockVpn, (nc) -> nc.hasTransport(TRANSPORT_WIFI));
|
||||
callback.expectCapabilitiesThat(mMockVpn, nc -> nc.hasTransport(TRANSPORT_WIFI));
|
||||
callback.expectCallback(CallbackEntry.LOST, mMockVpn);
|
||||
defaultCallback.expectCapabilitiesThat(mMockVpn, nc -> nc.hasTransport(TRANSPORT_WIFI));
|
||||
defaultCallback.expectCallback(CallbackEntry.LOST, mMockVpn);
|
||||
defaultCallback.expectAvailableCallbacksUnvalidatedAndBlocked(mWiFiNetworkAgent);
|
||||
|
||||
@@ -7183,7 +7316,7 @@ public class ConnectivityServiceTest {
|
||||
mWiFiNetworkAgent.disconnect();
|
||||
callback.expectCallback(CallbackEntry.LOST, mWiFiNetworkAgent);
|
||||
b1.expectBroadcast();
|
||||
callback.expectCapabilitiesThat(mMockVpn, (nc) -> !nc.hasTransport(TRANSPORT_WIFI));
|
||||
callback.expectCapabilitiesThat(mMockVpn, nc -> !nc.hasTransport(TRANSPORT_WIFI));
|
||||
b2 = expectConnectivityAction(TYPE_VPN, DetailedState.DISCONNECTED);
|
||||
mMockVpn.expectStopVpnRunnerPrivileged();
|
||||
callback.expectCallback(CallbackEntry.LOST, mMockVpn);
|
||||
|
||||
Reference in New Issue
Block a user