Update VPN capabilities when its underlying network set is null.
Previously, they were only updated when underlying network set was
non-null.
This change also ensures that all the calls b/w ConnectivityService and
Vpn that leads to updating capabilities are on ConnectivityService
handler thread.
Additionally, it also ensures that capabilities are propagated after VPN
enters connected state. This was previously done from establish which
could potentially lead to race between VPN getting connected and
capabilities getting updated.
This change also updates VPN capabilities inline from
ConnectivityService handler thread. Previously, there was an additional
loop where Vpn would update capabilities via NetworkAgent thru
AsyncChannel which posts back to CS handler thread, which could
potentially lead to delays in updating VPN capabilities.
Bug: 119129310
Bug: 118856062
Bug: 124268198
Test: atest FrameworksNetTests
Test: manual - verified VPNs capabilities are getting updated and
DownloadManager is working correctly.
Change-Id: Id0abc4d304bb096e92479a118168690ccce634ed
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index f2d4ae2..95f2a1f 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -917,7 +917,8 @@
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);
@@ -925,7 +926,11 @@
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);
@@ -936,7 +941,11 @@
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);
@@ -4088,17 +4097,27 @@
* 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) {
@@ -4439,22 +4458,28 @@
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);
}
}
}
@@ -4523,6 +4548,7 @@
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);
@@ -5027,6 +5053,19 @@
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();
}
@@ -5620,6 +5659,8 @@
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) {
@@ -6059,6 +6100,10 @@
// 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);
@@ -6320,7 +6365,11 @@
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 9141ccb..a7d16d8 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 @@
mNetworkCapabilities = new NetworkCapabilities();
mNetworkCapabilities.addTransportType(NetworkCapabilities.TRANSPORT_VPN);
mNetworkCapabilities.removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN);
- updateCapabilities();
+ updateCapabilities(null /* defaultNetwork */);
loadAlwaysOnPackage();
}
@@ -267,22 +267,44 @@
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.
+ *
+ * <p>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 @@
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 @@
}
/**
- * 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 @@
// 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 @@
return ranges;
}
+ /**
+ * Updates UID ranges for this VPN and also updates its internal capabilities.
+ *
+ * <p>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 @@
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 @@
}
}
+ /**
+ * Updates UID ranges for this VPN and also updates its capabilities.
+ *
+ * <p>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 @@
final List<UidRange> removedRanges =
uidRangesForUser(userHandle, existingRanges);
existingRanges.removeAll(removedRanges);
+ // ConnectivityService will call {@link #updateCapabilities} and
+ // apply those for VPN network.
mNetworkCapabilities.setUids(existingRanges);
- updateCapabilities();
} catch (Exception e) {
Log.wtf(TAG, "Failed to remove restricted user to owner", e);
}
@@ -1502,6 +1534,12 @@
return success;
}
+ /**
+ * Updates underlying network set.
+ *
+ * <p>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 @@
}
}
}
- updateCapabilities();
return true;
}