From 129c01eabf1dd4e95473ec10a79f55e949f8ec77 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Mon, 9 Nov 2020 10:32:56 +0900 Subject: [PATCH] Move applying underlying caps from Vpn to ConnectivityService. Add support to ConnectivityService to track underlying networks directly instead of through the Vpn class. 1. Communicate all information necessary to propagate underlying network capabilities to ConnectivityService via NetworkAgent. This includes: a. Underlying networks: - Add SystemApi for NetworkAgent to declare its underlying networks to ConnectivityService, and use it in Vpn. - Add a new declaredUnderlyingNetworks member to NetworkAgentInfo and store the underlying networks in it. Move propagation of underlying network capabilities to mixInCapabilities, which is a natural place for it. b. "Always metered" bit: - Communicate this to ConnectivityService via the existing NOT_METERED capability. Store it in a new declaredMetered boolean in NetworkAgentInfo to separate it cleanly from the NOT_METERED bit in the capabilities, which depends on whether the underlying networks are metered or not. In order to ensure that this is only ever changed when a NC update is received from a NetworkAgent, define a new processCapabilitiesFromAgent similar to the existing processLinkPropertiesFromAgent. 2. Ensure that propagating underlying network capabilities does not read the VPN's NetworkCapabilities. In order to do this, ensure that all relevant information on underlying networks and metering is sent to ConnectivityService at NetworkAgent registration time. CS still calls Vpn#updateCapabilities when a user is added/removed, but that is deleted in a future CL. 3. Slightly generalize propagating underlying network capabilities because there may be other network types that also have underlying networks that aren't VPNs (e.g., VCN). - Introduce a new supportsUnderlyingNetworks() boolean method in NetworkAgentInfo. - Rename updateAllVpnsCapabilities to propagateUnderlyingNetworkCapabilities. This commit does not move the actual logic of calculating the underlying capabilities out of Vpn.java. That can be done in a subsequent change once CS stops calling getUnderlyingNetworks(). This commit also does not modify any of the other code in CS that directly accesses VPNs' underlying networks. Bug: 173331190 Test: passes existing tests in ConnectivityServiceTest Test: CTS test in r.android.com/1511114 Test: atest CtsNetTestCases:Ikev2VpnTest HostsideVpnTests Change-Id: I5f76cb1aa4866efed3d5c4590e931fdb0e994f8d --- core/java/android/net/NetworkAgent.java | 44 +++++++- .../android/server/ConnectivityService.java | 103 +++++++++++++----- .../server/connectivity/NetworkAgentInfo.java | 16 +++ .../server/ConnectivityServiceTest.java | 9 +- 4 files changed, 143 insertions(+), 29 deletions(-) diff --git a/core/java/android/net/NetworkAgent.java b/core/java/android/net/NetworkAgent.java index 44ebff99f3..0676ad4e23 100644 --- a/core/java/android/net/NetworkAgent.java +++ b/core/java/android/net/NetworkAgent.java @@ -40,6 +40,7 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.time.Duration; import java.util.ArrayList; +import java.util.List; import java.util.Objects; import java.util.concurrent.atomic.AtomicBoolean; @@ -173,6 +174,14 @@ public abstract class NetworkAgent { */ public static final int EVENT_NETWORK_SCORE_CHANGED = BASE + 4; + /** + * Sent by the NetworkAgent to ConnectivityService to pass the current + * list of underlying networks. + * obj = array of Network objects + * @hide + */ + public static final int EVENT_UNDERLYING_NETWORKS_CHANGED = BASE + 5; + /** * Sent by ConnectivityService to the NetworkAgent to inform the agent of the * networks status - whether we could use the network or could not, due to @@ -217,7 +226,13 @@ public abstract class NetworkAgent { * The key for the redirect URL in the Bundle argument of {@code CMD_REPORT_NETWORK_STATUS}. * @hide */ - public static String REDIRECT_URL_KEY = "redirect URL"; + public static final String REDIRECT_URL_KEY = "redirect URL"; + + /** + * Bundle key for the underlying networks in {@code EVENT_UNDERLYING_NETWORKS_CHANGED}. + * @hide + */ + public static final String UNDERLYING_NETWORKS_KEY = "underlyingNetworks"; /** * Sent by the NetworkAgent to ConnectivityService to indicate this network was @@ -649,6 +664,33 @@ public abstract class NetworkAgent { queueOrSendMessage(EVENT_NETWORK_PROPERTIES_CHANGED, new LinkProperties(linkProperties)); } + /** + * Must be called by the agent when the network's underlying networks change. + * + *

{@code networks} is one of the following: + *

+ * + * @param underlyingNetworks the new list of underlying networks. + * @see {@link VpnService.Builder#setUnderlyingNetworks(Network[])} + */ + public final void setUnderlyingNetworks(@Nullable List underlyingNetworks) { + final ArrayList underlyingArray = (underlyingNetworks != null) + ? new ArrayList<>(underlyingNetworks) : null; + final Bundle bundle = new Bundle(); + bundle.putParcelableArrayList(UNDERLYING_NETWORKS_KEY, underlyingArray); + queueOrSendMessage(EVENT_UNDERLYING_NETWORKS_CHANGED, bundle); + } + /** * Inform ConnectivityService that this agent has now connected. * Call {@link #unregister} to disconnect. diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index f7de5c023b..e96958eb32 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -2771,6 +2771,7 @@ public class ConnectivityService extends IConnectivityManager.Stub networkCapabilities = new NetworkCapabilities(networkCapabilities); networkCapabilities.restrictCapabilitesForTestNetwork(nai.creatorUid); } + processCapabilitiesFromAgent(nai, networkCapabilities); updateCapabilities(nai.getCurrentScore(), nai, networkCapabilities); break; } @@ -2809,6 +2810,31 @@ public class ConnectivityService extends IConnectivityManager.Stub mKeepaliveTracker.handleEventSocketKeepalive(nai, msg); break; } + case NetworkAgent.EVENT_UNDERLYING_NETWORKS_CHANGED: { + if (!nai.supportsUnderlyingNetworks()) { + Log.wtf(TAG, "Non-virtual networks cannot have underlying networks"); + break; + } + final ArrayList underlying; + try { + underlying = ((Bundle) msg.obj).getParcelableArrayList( + NetworkAgent.UNDERLYING_NETWORKS_KEY); + } catch (NullPointerException | ClassCastException e) { + break; + } + final Network[] oldUnderlying = nai.declaredUnderlyingNetworks; + nai.declaredUnderlyingNetworks = (underlying != null) + ? underlying.toArray(new Network[0]) : null; + + if (!Arrays.equals(oldUnderlying, nai.declaredUnderlyingNetworks)) { + if (DBG) { + log(nai.toShortString() + " changed underlying networks to " + + Arrays.toString(nai.declaredUnderlyingNetworks)); + } + updateCapabilities(nai.getCurrentScore(), nai, nai.networkCapabilities); + notifyIfacesChangedForNetworkStats(); + } + } } } @@ -3394,7 +3420,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } mLegacyTypeTracker.remove(nai, wasDefault); if (!nai.networkCapabilities.hasTransport(TRANSPORT_VPN)) { - updateAllVpnsCapabilities(); + propagateUnderlyingNetworkCapabilities(); } rematchAllNetworksAndRequests(); mLingerMonitor.noteDisconnect(nai); @@ -4774,22 +4800,17 @@ public class ConnectivityService extends IConnectivityManager.Stub } /** - * Ask all VPN objects to recompute and update their capabilities. + * Ask all networks with underlying networks 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. + * When underlying networks change, such networks may have to update capabilities to reflect + * things like the metered bit, their transports, and so on. The capabilities are calculated + * immediately. This method runs on the ConnectivityService thread. */ - private void updateAllVpnsCapabilities() { - Network defaultNetwork = getNetwork(getDefaultNetwork()); - synchronized (mVpns) { - for (int i = 0; i < mVpns.size(); i++) { - final Vpn vpn = mVpns.valueAt(i); - NetworkCapabilities nc = vpn.updateCapabilities(defaultNetwork); - updateVpnCapabilities(vpn, nc); + private void propagateUnderlyingNetworkCapabilities() { + ensureRunningOnConnectivityServiceThread(); + for (NetworkAgentInfo nai : mNetworkAgentInfos.values()) { + if (nai.supportsUnderlyingNetworks()) { + updateCapabilities(nai.getCurrentScore(), nai, nai.networkCapabilities); } } } @@ -5979,6 +6000,7 @@ public class ConnectivityService extends IConnectivityManager.Stub this, mNetd, mDnsResolver, mNMS, providerId, Binder.getCallingUid()); // Make sure the LinkProperties and NetworkCapabilities reflect what the agent info says. + processCapabilitiesFromAgent(nai, nc); nai.getAndSetNetworkCapabilities(mixInCapabilities(nai, nc)); processLinkPropertiesFromAgent(nai, nai.linkProperties); @@ -6019,6 +6041,12 @@ public class ConnectivityService extends IConnectivityManager.Stub updateUids(nai, null, nai.networkCapabilities); } + /** + * Called when receiving LinkProperties directly from a NetworkAgent. + * Stores into |nai| any data coming from the agent that might also be written to the network's + * LinkProperties by ConnectivityService itself. This ensures that the data provided by the + * agent is not lost when updateLinkProperties is called. + */ private void processLinkPropertiesFromAgent(NetworkAgentInfo nai, LinkProperties lp) { lp.ensureDirectlyConnectedRoutes(); nai.clatd.setNat64PrefixFromRa(lp.getNat64Prefix()); @@ -6314,6 +6342,30 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + /** + * Called when receiving NetworkCapabilities directly from a NetworkAgent. + * Stores into |nai| any data coming from the agent that might also be written to the network's + * NetworkCapabilities by ConnectivityService itself. This ensures that the data provided by the + * agent is not lost when updateCapabilities is called. + */ + private void processCapabilitiesFromAgent(NetworkAgentInfo nai, NetworkCapabilities nc) { + nai.declaredMetered = !nc.hasCapability(NET_CAPABILITY_NOT_METERED); + } + + /** Propagates to |nc| the capabilities declared by the underlying networks of |nai|. */ + private void mixInUnderlyingCapabilities(NetworkAgentInfo nai, NetworkCapabilities nc) { + Network[] underlyingNetworks = nai.declaredUnderlyingNetworks; + Network defaultNetwork = getNetwork(getDefaultNetwork()); + if (underlyingNetworks == null && defaultNetwork != null) { + // null underlying networks means to track the default. + underlyingNetworks = new Network[] { defaultNetwork }; + } + + // TODO(b/124469351): Get capabilities directly from ConnectivityService instead. + final ConnectivityManager cm = mContext.getSystemService(ConnectivityManager.class); + Vpn.applyUnderlyingCapabilities(cm, underlyingNetworks, nc, nai.declaredMetered); + } + /** * Augments the NetworkCapabilities passed in by a NetworkAgent with capabilities that are * maintained here that the NetworkAgent is not aware of (e.g., validated, captive portal, @@ -6367,6 +6419,10 @@ public class ConnectivityService extends IConnectivityManager.Stub newNc.addCapability(NET_CAPABILITY_NOT_ROAMING); } + if (nai.supportsUnderlyingNetworks()) { + mixInUnderlyingCapabilities(nai, newNc); + } + return newNc; } @@ -6446,7 +6502,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. - updateAllVpnsCapabilities(); + propagateUnderlyingNetworkCapabilities(); } if (!newNc.equalsTransportTypes(prevNc)) { @@ -6766,7 +6822,7 @@ public class ConnectivityService extends IConnectivityManager.Stub ? newNetwork.linkProperties.getTcpBufferSizes() : null); notifyIfacesChangedForNetworkStats(); // Fix up the NetworkCapabilities of any VPNs that don't specify underlying networks. - updateAllVpnsCapabilities(); + propagateUnderlyingNetworkCapabilities(); } private void processListenRequests(@NonNull final NetworkAgentInfo nai) { @@ -7228,7 +7284,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // onCapabilitiesUpdated being sent in updateAllVpnCapabilities below as // the VPN would switch from its default, blank capabilities to those // that reflect the capabilities of its underlying networks. - updateAllVpnsCapabilities(); + propagateUnderlyingNetworkCapabilities(); } networkAgent.created = true; } @@ -7270,8 +7326,8 @@ public class ConnectivityService extends IConnectivityManager.Stub // doing. updateSignalStrengthThresholds(networkAgent, "CONNECT", null); - if (networkAgent.isVPN()) { - updateAllVpnsCapabilities(); + if (networkAgent.supportsUnderlyingNetworks()) { + propagateUnderlyingNetworkCapabilities(); } // Consider network even though it is not yet validated. @@ -7528,13 +7584,6 @@ public class ConnectivityService extends IConnectivityManager.Stub throwIfLockdownEnabled(); success = mVpns.get(user).setUnderlyingNetworks(networks); } - if (success) { - 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/NetworkAgentInfo.java b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java index a9f62d9159..3270dd5521 100644 --- a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java +++ b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java @@ -132,6 +132,16 @@ public class NetworkAgentInfo implements Comparable { // TODO: make this private with a getter. public NetworkCapabilities networkCapabilities; public final NetworkAgentConfig networkAgentConfig; + + // Underlying networks declared by the agent. Only set if supportsUnderlyingNetworks is true. + // The networks in this list might be declared by a VPN app using setUnderlyingNetworks and are + // not guaranteed to be current or correct, or even to exist. + public @Nullable Network[] declaredUnderlyingNetworks; + + // Whether this network is always metered even if its underlying networks are unmetered. + // Only relevant if #supportsUnderlyingNetworks is true. + public boolean declaredMetered; + // Indicates if netd has been told to create this Network. From this point on the appropriate // routing rules are setup and routes are added so packets can begin flowing over the Network. // This is a sticky bit; once set it is never cleared. @@ -474,10 +484,16 @@ public class NetworkAgentInfo implements Comparable { networkCapabilities); } + /** Whether this network is a VPN. */ public boolean isVPN() { return networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_VPN); } + /** Whether this network might have underlying networks. Currently only true for VPNs. */ + public boolean supportsUnderlyingNetworks() { + return isVPN(); + } + private int getCurrentScore(boolean pretendValidated) { // TODO: We may want to refactor this into a NetworkScore class that takes a base score from // the NetworkAgent and signals from the NetworkAgent and uses those signals to modify the diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 99f1985ff6..561c6bab9c 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -1084,7 +1084,7 @@ public class ConnectivityServiceTest { throws Exception { if (mAgentRegistered) throw new IllegalStateException("already registered"); setUids(uids); - mConfig.isMetered = isAlwaysMetered; + if (!isAlwaysMetered) mNetworkCapabilities.addCapability(NET_CAPABILITY_NOT_METERED); mInterface = VPN_IFNAME; mMockNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_VPN, lp, mNetworkCapabilities); @@ -5053,6 +5053,13 @@ public class ConnectivityServiceTest { waitForIdle(); expectForceUpdateIfaces(wifiAndVpn, null); reset(mStatsService); + + // Passing in null again means follow the default network again. + mService.setUnderlyingNetworksForVpn(null); + waitForIdle(); + expectForceUpdateIfaces(wifiAndVpn, WIFI_IFNAME, Process.myUid(), VPN_IFNAME, + new String[]{WIFI_IFNAME}); + reset(mStatsService); } @Test