From 22670f9b93fe220a32d72136c55b437dc2e48f9a Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Fri, 18 May 2018 22:02:56 +0900 Subject: [PATCH] Fix: VPNs update caps upon underlying network disconnect. Clean cherry-pick of ag/4083954 Bug: 79748782 Test: ConnectivityServiceTests still pass Change-Id: I21e866c723099e5c3dee54ff13e830d44427fc7a Merged-In: I12c948ebeb2b74290908f8320ff77220dc4a9fb9 --- .../android/server/ConnectivityService.java | 32 +++++++++++++++---- .../server/ConnectivityServiceTest.java | 32 +++++++++---------- 2 files changed, 40 insertions(+), 24 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index ff301487e2..fe0f6d23b1 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -2465,6 +2465,9 @@ public class ConnectivityService extends IConnectivityManager.Stub ensureNetworkTransitionWakelock(nai.name()); } mLegacyTypeTracker.remove(nai, wasDefault); + if (!nai.networkCapabilities.hasTransport(TRANSPORT_VPN)) { + updateAllVpnsCapabilities(); + } rematchAllNetworksAndRequests(null, 0); mLingerMonitor.noteDisconnect(nai); if (nai.created) { @@ -3734,6 +3737,26 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + /** + * 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) { @@ -4531,7 +4554,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // 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; @@ -4891,12 +4914,7 @@ public class ConnectivityService extends IConnectivityManager.Stub 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 572cb521ed..fef5417f1a 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -4401,24 +4401,22 @@ public class ConnectivityServiceTest { && !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(); }