diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 006a46ac5d..b5fcde4bf2 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -919,7 +919,8 @@ public class ConnectivityService extends IConnectivityManager.Stub mPermissionMonitor = new PermissionMonitor(mContext, mNMS); - //set up the listener for user state for creating user VPNs + // Set up the listener for user state for creating user VPNs. + // Should run on mHandler to avoid any races. IntentFilter intentFilter = new IntentFilter(); intentFilter.addAction(Intent.ACTION_USER_STARTED); intentFilter.addAction(Intent.ACTION_USER_STOPPED); @@ -927,7 +928,11 @@ public class ConnectivityService extends IConnectivityManager.Stub intentFilter.addAction(Intent.ACTION_USER_REMOVED); intentFilter.addAction(Intent.ACTION_USER_UNLOCKED); mContext.registerReceiverAsUser( - mIntentReceiver, UserHandle.ALL, intentFilter, null, null); + mIntentReceiver, + UserHandle.ALL, + intentFilter, + null /* broadcastPermission */, + mHandler); mContext.registerReceiverAsUser(mUserPresentReceiver, UserHandle.SYSTEM, new IntentFilter(Intent.ACTION_USER_PRESENT), null, null); @@ -938,7 +943,11 @@ public class ConnectivityService extends IConnectivityManager.Stub intentFilter.addAction(Intent.ACTION_PACKAGE_REMOVED); intentFilter.addDataScheme("package"); mContext.registerReceiverAsUser( - mIntentReceiver, UserHandle.ALL, intentFilter, null, null); + mIntentReceiver, + UserHandle.ALL, + intentFilter, + null /* broadcastPermission */, + mHandler); try { mNMS.registerObserver(mTethering); @@ -4128,17 +4137,27 @@ public class ConnectivityService extends IConnectivityManager.Stub * handler thread through their agent, this is asynchronous. When the capabilities objects * are computed they will be up-to-date as they are computed synchronously from here and * this is running on the ConnectivityService thread. - * TODO : Fix this and call updateCapabilities inline to remove out-of-order events. */ private void updateAllVpnsCapabilities() { + Network defaultNetwork = getNetwork(getDefaultNetwork()); synchronized (mVpns) { for (int i = 0; i < mVpns.size(); i++) { final Vpn vpn = mVpns.valueAt(i); - vpn.updateCapabilities(); + NetworkCapabilities nc = vpn.updateCapabilities(defaultNetwork); + updateVpnCapabilities(vpn, nc); } } } + private void updateVpnCapabilities(Vpn vpn, @Nullable NetworkCapabilities nc) { + ensureRunningOnConnectivityServiceThread(); + NetworkAgentInfo vpnNai = getNetworkAgentInfoForNetId(vpn.getNetId()); + if (vpnNai == null || nc == null) { + return; + } + updateCapabilities(vpnNai.getCurrentScore(), vpnNai, nc); + } + @Override public boolean updateLockdownVpn() { if (Binder.getCallingUid() != Process.SYSTEM_UID) { @@ -4479,22 +4498,28 @@ public class ConnectivityService extends IConnectivityManager.Stub private void onUserAdded(int userId) { mPermissionMonitor.onUserAdded(userId); + Network defaultNetwork = getNetwork(getDefaultNetwork()); synchronized (mVpns) { final int vpnsSize = mVpns.size(); for (int i = 0; i < vpnsSize; i++) { Vpn vpn = mVpns.valueAt(i); vpn.onUserAdded(userId); + NetworkCapabilities nc = vpn.updateCapabilities(defaultNetwork); + updateVpnCapabilities(vpn, nc); } } } private void onUserRemoved(int userId) { mPermissionMonitor.onUserRemoved(userId); + Network defaultNetwork = getNetwork(getDefaultNetwork()); synchronized (mVpns) { final int vpnsSize = mVpns.size(); for (int i = 0; i < vpnsSize; i++) { Vpn vpn = mVpns.valueAt(i); vpn.onUserRemoved(userId); + NetworkCapabilities nc = vpn.updateCapabilities(defaultNetwork); + updateVpnCapabilities(vpn, nc); } } } @@ -4563,6 +4588,7 @@ public class ConnectivityService extends IConnectivityManager.Stub private BroadcastReceiver mIntentReceiver = new BroadcastReceiver() { @Override public void onReceive(Context context, Intent intent) { + ensureRunningOnConnectivityServiceThread(); final String action = intent.getAction(); final int userId = intent.getIntExtra(Intent.EXTRA_USER_HANDLE, UserHandle.USER_NULL); final int uid = intent.getIntExtra(Intent.EXTRA_UID, -1); @@ -5067,6 +5093,19 @@ public class ConnectivityService extends IConnectivityManager.Stub return getNetworkForRequest(mDefaultRequest.requestId); } + @Nullable + private Network getNetwork(@Nullable NetworkAgentInfo nai) { + return nai != null ? nai.network : null; + } + + private void ensureRunningOnConnectivityServiceThread() { + if (mHandler.getLooper().getThread() != Thread.currentThread()) { + throw new IllegalStateException( + "Not running on ConnectivityService thread: " + + Thread.currentThread().getName()); + } + } + private boolean isDefaultNetwork(NetworkAgentInfo nai) { return nai == getDefaultNetwork(); } @@ -5665,6 +5704,8 @@ public class ConnectivityService extends IConnectivityManager.Stub updateTcpBufferSizes(newNetwork.linkProperties.getTcpBufferSizes()); mDnsManager.setDefaultDnsSystemProperties(newNetwork.linkProperties.getDnsServers()); notifyIfacesChangedForNetworkStats(); + // Fix up the NetworkCapabilities of any VPNs that don't specify underlying networks. + updateAllVpnsCapabilities(); } private void processListenRequests(NetworkAgentInfo nai, boolean capabilitiesChanged) { @@ -6104,6 +6145,10 @@ public class ConnectivityService extends IConnectivityManager.Stub // doing. updateSignalStrengthThresholds(networkAgent, "CONNECT", null); + if (networkAgent.isVPN()) { + updateAllVpnsCapabilities(); + } + // Consider network even though it is not yet validated. final long now = SystemClock.elapsedRealtime(); rematchNetworkAndRequests(networkAgent, ReapUnvalidatedNetworks.REAP, now); @@ -6365,7 +6410,11 @@ public class ConnectivityService extends IConnectivityManager.Stub success = mVpns.get(user).setUnderlyingNetworks(networks); } if (success) { - mHandler.post(() -> notifyIfacesChangedForNetworkStats()); + mHandler.post(() -> { + // Update VPN's capabilities based on updated underlying network set. + updateAllVpnsCapabilities(); + notifyIfacesChangedForNetworkStats(); + }); } return success; } diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 1325e264aa..d1a06925a9 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -20,6 +20,7 @@ import static android.net.ConnectivityManager.CONNECTIVITY_ACTION; import static android.net.ConnectivityManager.PRIVATE_DNS_MODE_OFF; import static android.net.ConnectivityManager.PRIVATE_DNS_MODE_OPPORTUNISTIC; import static android.net.ConnectivityManager.PRIVATE_DNS_MODE_PROVIDER_HOSTNAME; +import static android.net.ConnectivityManager.NETID_UNSET; import static android.net.ConnectivityManager.TYPE_ETHERNET; import static android.net.ConnectivityManager.TYPE_MOBILE; import static android.net.ConnectivityManager.TYPE_MOBILE_FOTA; @@ -885,11 +886,14 @@ public class ConnectivityServiceTest { public void setUids(Set uids) { mNetworkCapabilities.setUids(uids); - updateCapabilities(); + updateCapabilities(null /* defaultNetwork */); } @Override public int getNetId() { + if (mMockNetworkAgent == null) { + return NETID_UNSET; + } return mMockNetworkAgent.getNetwork().netId; } @@ -911,12 +915,13 @@ public class ConnectivityServiceTest { } @Override - public void updateCapabilities() { - if (!mConnected) return; - super.updateCapabilities(); - // Because super.updateCapabilities will update the capabilities of the agent but not - // the mock agent, the mock agent needs to know about them. + public NetworkCapabilities updateCapabilities(Network defaultNetwork) { + if (!mConnected) return null; + super.updateCapabilities(defaultNetwork); + // Because super.updateCapabilities will update the capabilities of the agent but + // not the mock agent, the mock agent needs to know about them. copyCapabilitiesToNetworkAgent(); + return new NetworkCapabilities(mNetworkCapabilities); } private void copyCapabilitiesToNetworkAgent() { @@ -4669,6 +4674,7 @@ public class ConnectivityServiceTest { vpnNetworkAgent.connect(false); mMockVpn.connect(); + mMockVpn.setUnderlyingNetworks(new Network[0]); genericNetworkCallback.expectAvailableCallbacksUnvalidated(vpnNetworkAgent); genericNotVpnNetworkCallback.assertNoCallback(); @@ -4701,6 +4707,7 @@ public class ConnectivityServiceTest { ranges.add(new UidRange(uid, uid)); mMockVpn.setUids(ranges); + vpnNetworkAgent.setUids(ranges); genericNetworkCallback.expectAvailableCallbacksValidated(vpnNetworkAgent); genericNotVpnNetworkCallback.assertNoCallback(); @@ -4734,12 +4741,11 @@ public class ConnectivityServiceTest { } @Test - public void testVpnWithAndWithoutInternet() { + public void testVpnWithoutInternet() { final int uid = Process.myUid(); final TestNetworkCallback defaultCallback = new TestNetworkCallback(); mCm.registerDefaultNetworkCallback(defaultCallback); - defaultCallback.assertNoCallback(); mWiFiNetworkAgent = new MockNetworkAgent(TRANSPORT_WIFI); mWiFiNetworkAgent.connect(true); @@ -4761,11 +4767,30 @@ public class ConnectivityServiceTest { vpnNetworkAgent.disconnect(); defaultCallback.assertNoCallback(); - vpnNetworkAgent = new MockNetworkAgent(TRANSPORT_VPN); + mCm.unregisterNetworkCallback(defaultCallback); + } + + @Test + public void testVpnWithInternet() { + final int uid = Process.myUid(); + + final TestNetworkCallback defaultCallback = new TestNetworkCallback(); + mCm.registerDefaultNetworkCallback(defaultCallback); + + mWiFiNetworkAgent = new MockNetworkAgent(TRANSPORT_WIFI); + mWiFiNetworkAgent.connect(true); + + defaultCallback.expectAvailableThenValidatedCallbacks(mWiFiNetworkAgent); + assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); + + MockNetworkAgent vpnNetworkAgent = new MockNetworkAgent(TRANSPORT_VPN); + final ArraySet ranges = new ArraySet<>(); + ranges.add(new UidRange(uid, uid)); mMockVpn.setNetworkAgent(vpnNetworkAgent); mMockVpn.setUids(ranges); vpnNetworkAgent.connect(true /* validated */, true /* hasInternet */); mMockVpn.connect(); + defaultCallback.expectAvailableThenValidatedCallbacks(vpnNetworkAgent); assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); @@ -4773,14 +4798,6 @@ public class ConnectivityServiceTest { defaultCallback.expectCallback(CallbackState.LOST, vpnNetworkAgent); defaultCallback.expectAvailableCallbacksValidated(mWiFiNetworkAgent); - vpnNetworkAgent = new MockNetworkAgent(TRANSPORT_VPN); - ranges.clear(); - mMockVpn.setNetworkAgent(vpnNetworkAgent); - mMockVpn.setUids(ranges); - vpnNetworkAgent.connect(false /* validated */, true /* hasInternet */); - mMockVpn.connect(); - defaultCallback.assertNoCallback(); - mCm.unregisterNetworkCallback(defaultCallback); } @@ -4882,6 +4899,70 @@ public class ConnectivityServiceTest { mMockVpn.disconnect(); } + @Test + public void testNullUnderlyingNetworks() { + 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(); + NetworkCapabilities nc; + mCm.registerNetworkCallback(vpnNetworkRequest, vpnNetworkCallback); + vpnNetworkCallback.assertNoCallback(); + + final MockNetworkAgent vpnNetworkAgent = new MockNetworkAgent(TRANSPORT_VPN); + final ArraySet ranges = new ArraySet<>(); + ranges.add(new UidRange(uid, uid)); + mMockVpn.setNetworkAgent(vpnNetworkAgent); + mMockVpn.connect(); + mMockVpn.setUids(ranges); + vpnNetworkAgent.connect(true /* validated */, false /* hasInternet */); + + vpnNetworkCallback.expectAvailableThenValidatedCallbacks(vpnNetworkAgent); + nc = mCm.getNetworkCapabilities(vpnNetworkAgent.getNetwork()); + assertTrue(nc.hasTransport(TRANSPORT_VPN)); + assertFalse(nc.hasTransport(TRANSPORT_CELLULAR)); + assertFalse(nc.hasTransport(TRANSPORT_WIFI)); + // By default, VPN is set to track default network (i.e. its underlying networks is null). + // In case of no default network, VPN is considered metered. + assertFalse(nc.hasCapability(NET_CAPABILITY_NOT_METERED)); + + // Connect to Cell; Cell is the default network. + mCellNetworkAgent = new MockNetworkAgent(TRANSPORT_CELLULAR); + mCellNetworkAgent.connect(true); + + vpnNetworkCallback.expectCapabilitiesLike((caps) -> caps.hasTransport(TRANSPORT_VPN) + && caps.hasTransport(TRANSPORT_CELLULAR) && !caps.hasTransport(TRANSPORT_WIFI) + && !caps.hasCapability(NET_CAPABILITY_NOT_METERED), + vpnNetworkAgent); + + // Connect to WiFi; WiFi is the new default. + mWiFiNetworkAgent = new MockNetworkAgent(TRANSPORT_WIFI); + mWiFiNetworkAgent.addCapability(NET_CAPABILITY_NOT_METERED); + mWiFiNetworkAgent.connect(true); + + vpnNetworkCallback.expectCapabilitiesLike((caps) -> caps.hasTransport(TRANSPORT_VPN) + && !caps.hasTransport(TRANSPORT_CELLULAR) && caps.hasTransport(TRANSPORT_WIFI) + && caps.hasCapability(NET_CAPABILITY_NOT_METERED), + vpnNetworkAgent); + + // Disconnect Cell. The default network did not change, so there shouldn't be any changes in + // the capabilities. + mCellNetworkAgent.disconnect(); + + // Disconnect wifi too. Now we have no default network. + mWiFiNetworkAgent.disconnect(); + + vpnNetworkCallback.expectCapabilitiesLike((caps) -> caps.hasTransport(TRANSPORT_VPN) + && !caps.hasTransport(TRANSPORT_CELLULAR) && !caps.hasTransport(TRANSPORT_WIFI) + && !caps.hasCapability(NET_CAPABILITY_NOT_METERED), + vpnNetworkAgent); + + mMockVpn.disconnect(); + } + @Test public void testNetworkBlockedStatus() { final TestNetworkCallback cellNetworkCallback = new TestNetworkCallback(); diff --git a/tests/net/java/com/android/server/connectivity/VpnTest.java b/tests/net/java/com/android/server/connectivity/VpnTest.java index 46de3d0608..f169d6b5be 100644 --- a/tests/net/java/com/android/server/connectivity/VpnTest.java +++ b/tests/net/java/com/android/server/connectivity/VpnTest.java @@ -566,7 +566,7 @@ public class VpnTest { final NetworkCapabilities caps = new NetworkCapabilities(); - Vpn.updateCapabilities( + Vpn.applyUnderlyingCapabilities( mConnectivityManager, new Network[] {}, caps, false /* isAlwaysMetered */); assertTrue(caps.hasTransport(TRANSPORT_VPN)); assertFalse(caps.hasTransport(TRANSPORT_CELLULAR)); @@ -577,7 +577,7 @@ public class VpnTest { assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_ROAMING)); assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_CONGESTED)); - Vpn.updateCapabilities( + Vpn.applyUnderlyingCapabilities( mConnectivityManager, new Network[] {mobile}, caps, @@ -591,7 +591,7 @@ public class VpnTest { assertFalse(caps.hasCapability(NET_CAPABILITY_NOT_ROAMING)); assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_CONGESTED)); - Vpn.updateCapabilities( + Vpn.applyUnderlyingCapabilities( mConnectivityManager, new Network[] {wifi}, caps, false /* isAlwaysMetered */); assertTrue(caps.hasTransport(TRANSPORT_VPN)); assertFalse(caps.hasTransport(TRANSPORT_CELLULAR)); @@ -602,7 +602,7 @@ public class VpnTest { assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_ROAMING)); assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_CONGESTED)); - Vpn.updateCapabilities( + Vpn.applyUnderlyingCapabilities( mConnectivityManager, new Network[] {wifi}, caps, true /* isAlwaysMetered */); assertTrue(caps.hasTransport(TRANSPORT_VPN)); assertFalse(caps.hasTransport(TRANSPORT_CELLULAR)); @@ -613,7 +613,7 @@ public class VpnTest { assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_ROAMING)); assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_CONGESTED)); - Vpn.updateCapabilities( + Vpn.applyUnderlyingCapabilities( mConnectivityManager, new Network[] {mobile, wifi}, caps,