Merge "Fix CaptivePortalDataTest#testParcelUnparcel on R"
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index 1b1b280..d67850c 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -3466,6 +3466,7 @@
// available until we've told netd to delete it below.
mNetworkForNetId.remove(nai.network.getNetId());
}
+ propagateUnderlyingNetworkCapabilities(nai.network);
// Remove all previously satisfied requests.
for (int i = 0; i < nai.numNetworkRequests(); i++) {
NetworkRequest request = nai.requestAt(i);
@@ -3478,7 +3479,9 @@
}
}
nai.clearLingerState();
- propagateUnderlyingNetworkCapabilities(nai.network);
+ // TODO: this loop, and the mLegacyTypeTracker.remove just below it, seem redundant given
+ // there's a full rematch right after. Currently, deleting it breaks tests that check for
+ // the default network disconnecting. Find out why, fix the rematch code, and delete this.
if (nai.isSatisfyingRequest(mDefaultRequest.requestId)) {
mDefaultNetworkNai = null;
updateDataActivityTracking(null /* newNetwork */, nai);
@@ -4995,6 +4998,8 @@
@Override
public boolean updateLockdownVpn() {
+ // Allow the system UID for the system server and for Settings.
+ // Also, for unit tests, allow the process that ConnectivityService is running in.
if (mDeps.getCallingUid() != Process.SYSTEM_UID
&& Binder.getCallingPid() != Process.myPid()) {
logw("Lockdown VPN only available to system process or AID_SYSTEM");
diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java
index 56afb40..f64ab60 100644
--- a/tests/net/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java
@@ -1389,9 +1389,6 @@
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 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 @@
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 @@
@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 @@
@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 @@
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 @@
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);