Fix: VPNs update caps upon underlying network disconnect.
Bug: 79748782
Test: ConnectivityServiceTests still pass
Change-Id: Ic8231b18a17e6feb5ebafe8d5688fb59f9d4d58e
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index 797cb4b..f729cb8 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -2503,6 +2503,9 @@
ensureNetworkTransitionWakelock(nai.name());
}
mLegacyTypeTracker.remove(nai, wasDefault);
+ if (!nai.networkCapabilities.hasTransport(TRANSPORT_VPN)) {
+ updateAllVpnsCapabilities();
+ }
rematchAllNetworksAndRequests(null, 0);
mLingerMonitor.noteDisconnect(nai);
if (nai.created) {
@@ -3778,6 +3781,26 @@
}
}
+ /**
+ * Ask all VPN objects to recompute and update their capabilities.
+ *
+ * When underlying networks change, VPNs may have to update capabilities to reflect things
+ * like the metered bit, their transports, and so on. This asks the VPN objects to update
+ * their capabilities, and as this will cause them to send messages to the ConnectivityService
+ * 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() {
+ synchronized (mVpns) {
+ for (int i = 0; i < mVpns.size(); i++) {
+ final Vpn vpn = mVpns.valueAt(i);
+ vpn.updateCapabilities();
+ }
+ }
+ }
+
@Override
public boolean updateLockdownVpn() {
if (Binder.getCallingUid() != Process.SYSTEM_UID) {
@@ -4575,7 +4598,7 @@
// Note: if mDefaultRequest is changed, NetworkMonitor needs to be updated.
private final NetworkRequest mDefaultRequest;
-
+
// Request used to optionally keep mobile data active even when higher
// priority networks like Wi-Fi are active.
private final NetworkRequest mDefaultMobileDataRequest;
@@ -4935,12 +4958,7 @@
if (!newNc.hasTransport(TRANSPORT_VPN)) {
// Tell VPNs about updated capabilities, since they may need to
// bubble those changes through.
- synchronized (mVpns) {
- for (int i = 0; i < mVpns.size(); i++) {
- final Vpn vpn = mVpns.valueAt(i);
- vpn.updateCapabilities();
- }
- }
+ updateAllVpnsCapabilities();
}
}
diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java
index bbb4b7b..cf7ab42 100644
--- a/tests/net/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java
@@ -4405,24 +4405,22 @@
&& !caps.hasCapability(NET_CAPABILITY_NOT_METERED),
vpnNetworkAgent);
- if (false) { // TODO : reactivate this ; in the current state, the below fail.
- // Disconnect cell. Receive update without even removing the dead network from the
- // underlying networks – it's dead anyway. Not metered any more.
- mCellNetworkAgent.disconnect();
- vpnNetworkCallback.expectCapabilitiesLike((caps) -> caps.hasTransport(TRANSPORT_VPN)
- && !caps.hasTransport(TRANSPORT_CELLULAR) && caps.hasTransport(TRANSPORT_WIFI)
- && caps.hasCapability(NET_CAPABILITY_NOT_METERED),
- vpnNetworkAgent);
+ // Disconnect cell. Receive update without even removing the dead network from the
+ // underlying networks – it's dead anyway. Not metered any more.
+ mCellNetworkAgent.disconnect();
+ vpnNetworkCallback.expectCapabilitiesLike((caps) -> caps.hasTransport(TRANSPORT_VPN)
+ && !caps.hasTransport(TRANSPORT_CELLULAR) && caps.hasTransport(TRANSPORT_WIFI)
+ && caps.hasCapability(NET_CAPABILITY_NOT_METERED),
+ vpnNetworkAgent);
- // Disconnect wifi too. No underlying networks should mean this is now metered,
- // unfortunately a discrepancy in the current implementation has this unmetered.
- // TODO : fix this.
- mWiFiNetworkAgent.disconnect();
- vpnNetworkCallback.expectCapabilitiesLike((caps) -> caps.hasTransport(TRANSPORT_VPN)
- && !caps.hasTransport(TRANSPORT_CELLULAR) && !caps.hasTransport(TRANSPORT_WIFI)
- && caps.hasCapability(NET_CAPABILITY_NOT_METERED),
- vpnNetworkAgent);
- }
+ // Disconnect wifi too. No underlying networks should mean this is now metered,
+ // unfortunately a discrepancy in the current implementation has this unmetered.
+ // TODO : fix this.
+ 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();
}