diff options
| -rw-r--r-- | services/core/java/com/android/server/ConnectivityService.java | 43 | ||||
| -rw-r--r-- | tests/net/java/com/android/server/ConnectivityServiceTest.java | 70 |
2 files changed, 97 insertions, 16 deletions
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 709f5b2106f3..236380155467 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -1514,11 +1514,11 @@ public class ConnectivityService extends IConnectivityManager.Stub // but only exists if an app asks about them or requests them. Ensure the requesting app // gets the type it asks for. filtered.setType(type); - final DetailedState state = isNetworkWithCapabilitiesBlocked(nc, uid, ignoreBlocked) - ? DetailedState.BLOCKED - : filtered.getDetailedState(); - filtered.setDetailedState(getLegacyLockdownState(state), - "" /* reason */, null /* extraInfo */); + if (isNetworkWithCapabilitiesBlocked(nc, uid, ignoreBlocked)) { + filtered.setDetailedState(DetailedState.BLOCKED, null /* reason */, + null /* extraInfo */); + } + filterForLegacyLockdown(filtered); return filtered; } @@ -1594,8 +1594,8 @@ public class ConnectivityService extends IConnectivityManager.Stub final DetailedState state = isNetworkWithCapabilitiesBlocked(nc, uid, false) ? DetailedState.BLOCKED : DetailedState.DISCONNECTED; - info.setDetailedState(getLegacyLockdownState(state), - "" /* reason */, null /* extraInfo */); + info.setDetailedState(state, null /* reason */, null /* extraInfo */); + filterForLegacyLockdown(info); return info; } @@ -2389,9 +2389,7 @@ public class ConnectivityService extends IConnectivityManager.Stub mContext.enforceCallingOrSelfPermission(KeepaliveTracker.PERMISSION, "ConnectivityService"); } - // Public because it's used by mLockdownTracker. - public void sendConnectedBroadcast(NetworkInfo info) { - PermissionUtils.enforceNetworkStackPermission(mContext); + private void sendConnectedBroadcast(NetworkInfo info) { sendGeneralBroadcast(info, CONNECTIVITY_ACTION); } @@ -5036,8 +5034,8 @@ public class ConnectivityService extends IConnectivityManager.Stub // The legacy lockdown VPN always uses the default network. // If the VPN's underlying network is no longer the current default network, it means that // the default network has just switched, and the VPN is about to disconnect. - // Report that the VPN is not connected, so when the state of NetworkInfo objects - // overwritten by getLegacyLockdownState will be set to CONNECTING and not CONNECTED. + // Report that the VPN is not connected, so the state of NetworkInfo objects overwritten + // by filterForLegacyLockdown will be set to CONNECTING and not CONNECTED. final NetworkAgentInfo defaultNetwork = getDefaultNetwork(); if (defaultNetwork == null || !defaultNetwork.network.equals(underlying[0])) { return null; @@ -5046,6 +5044,9 @@ public class ConnectivityService extends IConnectivityManager.Stub return nai; }; + // TODO: move all callers to filterForLegacyLockdown and delete this method. + // This likely requires making sendLegacyNetworkBroadcast take a NetworkInfo object instead of + // just a DetailedState object. private DetailedState getLegacyLockdownState(DetailedState origState) { if (origState != DetailedState.CONNECTED) { return origState; @@ -5055,6 +5056,23 @@ public class ConnectivityService extends IConnectivityManager.Stub : DetailedState.CONNECTED; } + private void filterForLegacyLockdown(NetworkInfo ni) { + if (!mLockdownEnabled || !ni.isConnected()) return; + // The legacy lockdown VPN replaces the state of every network in CONNECTED state with the + // state of its VPN. This is to ensure that when an underlying network connects, apps will + // not see a CONNECTIVITY_ACTION broadcast for a network in state CONNECTED until the VPN + // comes up, at which point there is a new CONNECTIVITY_ACTION broadcast for the underlying + // network, this time with a state of CONNECTED. + // + // Now that the legacy lockdown code lives in ConnectivityService, and no longer has access + // to the internal state of the Vpn object, always replace the state with CONNECTING. This + // is not too far off the truth, since an always-on VPN, when not connected, is always + // trying to reconnect. + if (getLegacyLockdownNai() == null) { + ni.setDetailedState(DetailedState.CONNECTING, "", null); + } + } + @Override public void setProvisioningNotificationVisible(boolean visible, int networkType, String action) { @@ -7938,6 +7956,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // and is still connected. NetworkInfo info = new NetworkInfo(nai.networkInfo); info.setType(type); + filterForLegacyLockdown(info); if (state != DetailedState.DISCONNECTED) { info.setDetailedState(state, null, info.getExtraInfo()); sendConnectedBroadcast(info); diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 3b1c09e77aca..d2ea5770421d 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -1740,11 +1740,29 @@ public class ConnectivityServiceTest { return expected; } + private boolean extraInfoInBroadcastHasExpectedNullness(NetworkInfo ni) { + final DetailedState state = ni.getDetailedState(); + if (state == DetailedState.CONNECTED && ni.getExtraInfo() == null) return false; + // Expect a null extraInfo if the network is CONNECTING, because a CONNECTIVITY_ACTION + // broadcast with a state of CONNECTING only happens due to legacy VPN lockdown, which also + // nulls out extraInfo. + if (state == DetailedState.CONNECTING && ni.getExtraInfo() != null) return false; + // Can't make any assertions about DISCONNECTED broadcasts. When a network actually + // disconnects, disconnectAndDestroyNetwork sets its state to DISCONNECTED and its extraInfo + // to null. But if the DISCONNECTED broadcast is just simulated by LegacyTypeTracker due to + // a network switch, extraInfo will likely be populated. + // This is likely a bug in CS, but likely not one we can fix without impacting apps. + return true; + } + private ExpectedBroadcast expectConnectivityAction(int type, NetworkInfo.DetailedState state) { - return registerConnectivityBroadcastThat(1, intent -> - type == intent.getIntExtra(EXTRA_NETWORK_TYPE, -1) && state.equals( - ((NetworkInfo) intent.getParcelableExtra(EXTRA_NETWORK_INFO)) - .getDetailedState())); + return registerConnectivityBroadcastThat(1, intent -> { + final int actualType = intent.getIntExtra(EXTRA_NETWORK_TYPE, -1); + final NetworkInfo ni = intent.getParcelableExtra(EXTRA_NETWORK_INFO); + return type == actualType + && state == ni.getDetailedState() + && extraInfoInBroadcastHasExpectedNullness(ni); + }); } @Test @@ -7182,12 +7200,14 @@ public class ConnectivityServiceTest { assertEquals(mCellNetworkAgent.getNetwork(), mCm.getActiveNetwork()); assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED); assertNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED); + assertExtraInfoFromCmPresent(mCellNetworkAgent); setUidRulesChanged(RULE_REJECT_ALL); cellNetworkCallback.expectBlockedStatusCallback(true, mCellNetworkAgent); assertNull(mCm.getActiveNetwork()); assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED); assertNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED); + assertExtraInfoFromCmBlocked(mCellNetworkAgent); // ConnectivityService should cache it not to invoke the callback again. setUidRulesChanged(RULE_REJECT_METERED); @@ -7198,12 +7218,14 @@ public class ConnectivityServiceTest { assertEquals(mCellNetworkAgent.getNetwork(), mCm.getActiveNetwork()); assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED); assertNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED); + assertExtraInfoFromCmPresent(mCellNetworkAgent); setUidRulesChanged(RULE_REJECT_METERED); cellNetworkCallback.expectBlockedStatusCallback(true, mCellNetworkAgent); assertNull(mCm.getActiveNetwork()); assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED); assertNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED); + assertExtraInfoFromCmBlocked(mCellNetworkAgent); // Restrict the network based on UID rule and NOT_METERED capability change. mCellNetworkAgent.addCapability(NET_CAPABILITY_NOT_METERED); @@ -7212,6 +7234,7 @@ public class ConnectivityServiceTest { assertEquals(mCellNetworkAgent.getNetwork(), mCm.getActiveNetwork()); assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED); assertNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED); + assertExtraInfoFromCmPresent(mCellNetworkAgent); mCellNetworkAgent.removeCapability(NET_CAPABILITY_NOT_METERED); cellNetworkCallback.expectCapabilitiesWithout(NET_CAPABILITY_NOT_METERED, @@ -7220,12 +7243,14 @@ public class ConnectivityServiceTest { assertNull(mCm.getActiveNetwork()); assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED); assertNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED); + assertExtraInfoFromCmBlocked(mCellNetworkAgent); setUidRulesChanged(RULE_ALLOW_METERED); cellNetworkCallback.expectBlockedStatusCallback(false, mCellNetworkAgent); assertEquals(mCellNetworkAgent.getNetwork(), mCm.getActiveNetwork()); assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED); assertNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED); + assertExtraInfoFromCmPresent(mCellNetworkAgent); setUidRulesChanged(RULE_NONE); cellNetworkCallback.assertNoCallback(); @@ -7236,6 +7261,7 @@ public class ConnectivityServiceTest { assertNull(mCm.getActiveNetwork()); assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED); assertNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED); + assertExtraInfoFromCmBlocked(mCellNetworkAgent); setRestrictBackgroundChanged(true); cellNetworkCallback.assertNoCallback(); @@ -7243,12 +7269,14 @@ public class ConnectivityServiceTest { cellNetworkCallback.expectBlockedStatusCallback(false, mCellNetworkAgent); assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED); assertNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED); + assertExtraInfoFromCmPresent(mCellNetworkAgent); setRestrictBackgroundChanged(false); cellNetworkCallback.assertNoCallback(); assertEquals(mCellNetworkAgent.getNetwork(), mCm.getActiveNetwork()); assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED); assertNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED); + assertExtraInfoFromCmPresent(mCellNetworkAgent); mCm.unregisterNetworkCallback(cellNetworkCallback); } @@ -7307,6 +7335,15 @@ public class ConnectivityServiceTest { assertNotNull(ni); assertEquals(type, ni.getType()); assertEquals(ConnectivityManager.getNetworkTypeName(type), state, ni.getDetailedState()); + if (state == DetailedState.CONNECTED || state == DetailedState.SUSPENDED) { + assertNotNull(ni.getExtraInfo()); + } else { + // Technically speaking, a network that's in CONNECTING state will generally have a + // non-null extraInfo. This doesn't actually happen in this test because it never calls + // a legacy API while a network is connecting. When a network is in CONNECTING state + // because of legacy lockdown VPN, its extraInfo is always null. + assertNull(ni.getExtraInfo()); + } } private void assertActiveNetworkInfo(int type, DetailedState state) { @@ -7316,6 +7353,26 @@ public class ConnectivityServiceTest { checkNetworkInfo(mCm.getNetworkInfo(type), type, state); } + private void assertExtraInfoFromCm(TestNetworkAgentWrapper network, boolean present) { + final NetworkInfo niForNetwork = mCm.getNetworkInfo(network.getNetwork()); + final NetworkInfo niForType = mCm.getNetworkInfo(network.getLegacyType()); + if (present) { + assertEquals(network.getExtraInfo(), niForNetwork.getExtraInfo()); + assertEquals(network.getExtraInfo(), niForType.getExtraInfo()); + } else { + assertNull(niForNetwork.getExtraInfo()); + assertNull(niForType.getExtraInfo()); + } + } + + private void assertExtraInfoFromCmBlocked(TestNetworkAgentWrapper network) { + assertExtraInfoFromCm(network, false); + } + + private void assertExtraInfoFromCmPresent(TestNetworkAgentWrapper network) { + assertExtraInfoFromCm(network, true); + } + // Checks that each of the |agents| receive a blocked status change callback with the specified // |blocked| value, in any order. This is needed because when an event affects multiple // networks, ConnectivityService does not guarantee the order in which callbacks are fired. @@ -7630,6 +7687,7 @@ public class ConnectivityServiceTest { assertNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED); assertNetworkInfo(TYPE_WIFI, DetailedState.BLOCKED); assertNetworkInfo(TYPE_VPN, DetailedState.BLOCKED); + assertExtraInfoFromCmBlocked(mCellNetworkAgent); // TODO: it would be nice if we could simply rely on the production code here, and have // LockdownVpnTracker start the VPN, have the VPN code register its NetworkAgent with @@ -7658,6 +7716,7 @@ public class ConnectivityServiceTest { assertNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED); assertNetworkInfo(TYPE_WIFI, DetailedState.DISCONNECTED); assertNetworkInfo(TYPE_VPN, DetailedState.CONNECTED); + assertExtraInfoFromCmPresent(mCellNetworkAgent); assertTrue(vpnNc.hasTransport(TRANSPORT_VPN)); assertTrue(vpnNc.hasTransport(TRANSPORT_CELLULAR)); assertFalse(vpnNc.hasTransport(TRANSPORT_WIFI)); @@ -7700,6 +7759,7 @@ public class ConnectivityServiceTest { assertNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED); assertNetworkInfo(TYPE_WIFI, DetailedState.BLOCKED); assertNetworkInfo(TYPE_VPN, DetailedState.BLOCKED); + assertExtraInfoFromCmBlocked(mWiFiNetworkAgent); // The VPN comes up again on wifi. b1 = expectConnectivityAction(TYPE_VPN, DetailedState.CONNECTED); @@ -7714,6 +7774,7 @@ public class ConnectivityServiceTest { assertNetworkInfo(TYPE_MOBILE, DetailedState.DISCONNECTED); assertNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED); assertNetworkInfo(TYPE_VPN, DetailedState.CONNECTED); + assertExtraInfoFromCmPresent(mWiFiNetworkAgent); vpnNc = mCm.getNetworkCapabilities(mMockVpn.getNetwork()); assertTrue(vpnNc.hasTransport(TRANSPORT_VPN)); assertTrue(vpnNc.hasTransport(TRANSPORT_WIFI)); @@ -7730,6 +7791,7 @@ public class ConnectivityServiceTest { assertNetworkInfo(TYPE_MOBILE, DetailedState.DISCONNECTED); assertNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED); assertNetworkInfo(TYPE_VPN, DetailedState.CONNECTED); + assertExtraInfoFromCmPresent(mWiFiNetworkAgent); b1 = expectConnectivityAction(TYPE_WIFI, DetailedState.DISCONNECTED); mWiFiNetworkAgent.disconnect(); |