diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 006a46ac5d2fd..b5fcde4bf2033 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/services/core/java/com/android/server/connectivity/Vpn.java b/services/core/java/com/android/server/connectivity/Vpn.java index 9141ccb387bde..a7d16d8cd6ecf 100644 --- a/services/core/java/com/android/server/connectivity/Vpn.java +++ b/services/core/java/com/android/server/connectivity/Vpn.java @@ -240,7 +240,7 @@ public class Vpn { mNetworkCapabilities = new NetworkCapabilities(); mNetworkCapabilities.addTransportType(NetworkCapabilities.TRANSPORT_VPN); mNetworkCapabilities.removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN); - updateCapabilities(); + updateCapabilities(null /* defaultNetwork */); loadAlwaysOnPackage(); } @@ -267,22 +267,44 @@ public class Vpn { updateAlwaysOnNotification(detailedState); } - public void updateCapabilities() { - final Network[] underlyingNetworks = (mConfig != null) ? mConfig.underlyingNetworks : null; - // Only apps targeting Q and above can explicitly declare themselves as metered. - final boolean isAlwaysMetered = - mIsPackageTargetingAtLeastQ && (mConfig == null || mConfig.isMetered); - updateCapabilities(mContext.getSystemService(ConnectivityManager.class), underlyingNetworks, - mNetworkCapabilities, isAlwaysMetered); - - if (mNetworkAgent != null) { - mNetworkAgent.sendNetworkCapabilities(mNetworkCapabilities); + /** + * Updates {@link #mNetworkCapabilities} based on current underlying networks and returns a + * defensive copy. + * + *
Does not propagate updated capabilities to apps. + * + * @param defaultNetwork underlying network for VPNs following platform's default + */ + public synchronized NetworkCapabilities updateCapabilities( + @Nullable Network defaultNetwork) { + if (mConfig == null) { + // VPN is not running. + return null; } + + Network[] underlyingNetworks = mConfig.underlyingNetworks; + if (underlyingNetworks == null && defaultNetwork != null) { + // null underlying networks means to track the default. + underlyingNetworks = new Network[] { defaultNetwork }; + } + // Only apps targeting Q and above can explicitly declare themselves as metered. + final boolean isAlwaysMetered = mIsPackageTargetingAtLeastQ && mConfig.isMetered; + + applyUnderlyingCapabilities( + mContext.getSystemService(ConnectivityManager.class), + underlyingNetworks, + mNetworkCapabilities, + isAlwaysMetered); + + return new NetworkCapabilities(mNetworkCapabilities); } @VisibleForTesting - public static void updateCapabilities(ConnectivityManager cm, Network[] underlyingNetworks, - NetworkCapabilities caps, boolean isAlwaysMetered) { + public static void applyUnderlyingCapabilities( + ConnectivityManager cm, + Network[] underlyingNetworks, + NetworkCapabilities caps, + boolean isAlwaysMetered) { int[] transportTypes = new int[] { NetworkCapabilities.TRANSPORT_VPN }; int downKbps = NetworkCapabilities.LINK_BANDWIDTH_UNSPECIFIED; int upKbps = NetworkCapabilities.LINK_BANDWIDTH_UNSPECIFIED; @@ -295,6 +317,7 @@ public class Vpn { boolean hadUnderlyingNetworks = false; if (null != underlyingNetworks) { for (Network underlying : underlyingNetworks) { + // TODO(b/124469351): Get capabilities directly from ConnectivityService instead. final NetworkCapabilities underlyingCaps = cm.getNetworkCapabilities(underlying); if (underlyingCaps == null) continue; hadUnderlyingNetworks = true; @@ -1005,9 +1028,8 @@ public class Vpn { } /** - * Establish a VPN network and return the file descriptor of the VPN - * interface. This methods returns {@code null} if the application is - * revoked or not prepared. + * Establish a VPN network and return the file descriptor of the VPN interface. This methods + * returns {@code null} if the application is revoked or not prepared. * * @param config The parameters to configure the network. * @return The file descriptor of the VPN interface. @@ -1099,8 +1121,6 @@ public class Vpn { // as rules are deleted. This prevents data leakage as the rules are moved over. agentDisconnect(oldNetworkAgent); } - // Set up VPN's capabilities such as meteredness. - updateCapabilities(); if (oldConnection != null) { mContext.unbindService(oldConnection); @@ -1256,6 +1276,11 @@ public class Vpn { return ranges; } + /** + * Updates UID ranges for this VPN and also updates its internal capabilities. + * + *
Should be called on primary ConnectivityService thread. + */ public void onUserAdded(int userHandle) { // If the user is restricted tie them to the parent user's VPN UserInfo user = UserManager.get(mContext).getUserInfo(userHandle); @@ -1266,8 +1291,9 @@ public class Vpn { try { addUserToRanges(existingRanges, userHandle, mConfig.allowedApplications, mConfig.disallowedApplications); + // ConnectivityService will call {@link #updateCapabilities} and apply + // those for VPN network. mNetworkCapabilities.setUids(existingRanges); - updateCapabilities(); } catch (Exception e) { Log.wtf(TAG, "Failed to add restricted user to owner", e); } @@ -1277,6 +1303,11 @@ public class Vpn { } } + /** + * Updates UID ranges for this VPN and also updates its capabilities. + * + *
Should be called on primary ConnectivityService thread.
+ */
public void onUserRemoved(int userHandle) {
// clean up if restricted
UserInfo user = UserManager.get(mContext).getUserInfo(userHandle);
@@ -1288,8 +1319,9 @@ public class Vpn {
final List Note: Does not updates capabilities. Call {@link #updateCapabilities} from
+ * ConnectivityService thread to get updated capabilities.
+ */
public synchronized boolean setUnderlyingNetworks(Network[] networks) {
if (!isCallerEstablishedOwnerLocked()) {
return false;
@@ -1518,7 +1556,6 @@ public class Vpn {
}
}
}
- updateCapabilities();
return true;
}
diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java
index 1325e264aa605..d1a06925a902d 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