diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 16cff7bb7f..479c885812 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -2812,6 +2812,7 @@ public class ConnectivityService extends IConnectivityManager.Stub break; } case NetworkAgent.EVENT_UNDERLYING_NETWORKS_CHANGED: { + // TODO: prevent loops, e.g., if a network declares itself as underlying. if (!nai.supportsUnderlyingNetworks()) { Log.wtf(TAG, "Non-virtual networks cannot have underlying networks"); break; @@ -3410,6 +3411,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } } nai.clearLingerState(); + propagateUnderlyingNetworkCapabilities(nai.network); if (nai.isSatisfyingRequest(mDefaultRequest.requestId)) { mDefaultNetworkNai = null; updateDataActivityTracking(null /* newNetwork */, nai); @@ -3417,9 +3419,6 @@ public class ConnectivityService extends IConnectivityManager.Stub ensureNetworkTransitionWakelock(nai.toShortString()); } mLegacyTypeTracker.remove(nai, wasDefault); - if (!nai.networkCapabilities.hasTransport(TRANSPORT_VPN)) { - propagateUnderlyingNetworkCapabilities(); - } rematchAllNetworksAndRequests(); mLingerMonitor.noteDisconnect(nai); if (nai.created) { @@ -4803,17 +4802,35 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + private Network[] underlyingNetworksOrDefault(Network[] underlyingNetworks) { + final Network defaultNetwork = getNetwork(getDefaultNetwork()); + if (underlyingNetworks == null && defaultNetwork != null) { + // null underlying networks means to track the default. + underlyingNetworks = new Network[] { defaultNetwork }; + } + return underlyingNetworks; + } + + // Returns true iff |network| is an underlying network of |nai|. + private boolean hasUnderlyingNetwork(NetworkAgentInfo nai, Network network) { + // TODO: support more than one level of underlying networks, either via a fixed-depth search + // (e.g., 2 levels of underlying networks), or via loop detection, or.... + if (!nai.supportsUnderlyingNetworks()) return false; + final Network[] underlying = underlyingNetworksOrDefault(nai.declaredUnderlyingNetworks); + return ArrayUtils.contains(underlying, network); + } + /** - * Ask all networks with underlying networks to recompute and update their capabilities. + * Recompute the capabilities for any networks that had a specific network as underlying. * * 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 propagateUnderlyingNetworkCapabilities() { + private void propagateUnderlyingNetworkCapabilities(Network updatedNetwork) { ensureRunningOnConnectivityServiceThread(); for (NetworkAgentInfo nai : mNetworkAgentInfos.values()) { - if (nai.supportsUnderlyingNetworks()) { + if (updatedNetwork == null || hasUnderlyingNetwork(nai, updatedNetwork)) { updateCapabilitiesForNetwork(nai); } } @@ -6351,27 +6368,28 @@ public class ConnectivityService extends IConnectivityManager.Stub * This method should never alter the agent's NetworkCapabilities, only store data in |nai|. */ private void processCapabilitiesFromAgent(NetworkAgentInfo nai, NetworkCapabilities nc) { - nai.declaredMetered = !nc.hasCapability(NET_CAPABILITY_NOT_METERED); + // Note: resetting the owner UID before storing the agent capabilities in NAI means that if + // the agent attempts to change the owner UID, then nai.declaredCapabilities will not + // actually be the same as the capabilities sent by the agent. Still, it is safer to reset + // the owner UID here and behave as if the agent had never tried to change it. if (nai.networkCapabilities.getOwnerUid() != nc.getOwnerUid()) { Log.e(TAG, nai.toShortString() + ": ignoring attempt to change owner from " + nai.networkCapabilities.getOwnerUid() + " to " + nc.getOwnerUid()); nc.setOwnerUid(nai.networkCapabilities.getOwnerUid()); } + nai.declaredCapabilities = new NetworkCapabilities(nc); } - /** Modifies |caps| based on the capabilities of the specified underlying networks. */ + /** Modifies |newNc| based on the capabilities of |underlyingNetworks| and |agentCaps|. */ @VisibleForTesting void applyUnderlyingCapabilities(@Nullable Network[] underlyingNetworks, - @NonNull NetworkCapabilities caps, boolean declaredMetered) { - final Network defaultNetwork = getNetwork(getDefaultNetwork()); - if (underlyingNetworks == null && defaultNetwork != null) { - // null underlying networks means to track the default. - underlyingNetworks = new Network[] { defaultNetwork }; - } - int[] transportTypes = new int[] { NetworkCapabilities.TRANSPORT_VPN }; + @NonNull NetworkCapabilities agentCaps, @NonNull NetworkCapabilities newNc) { + underlyingNetworks = underlyingNetworksOrDefault(underlyingNetworks); + int[] transportTypes = agentCaps.getTransportTypes(); int downKbps = NetworkCapabilities.LINK_BANDWIDTH_UNSPECIFIED; int upKbps = NetworkCapabilities.LINK_BANDWIDTH_UNSPECIFIED; - boolean metered = declaredMetered; // metered if any underlying is metered, or agentMetered + // metered if any underlying is metered, or originally declared metered by the agent. + boolean metered = !agentCaps.hasCapability(NET_CAPABILITY_NOT_METERED); boolean roaming = false; // roaming if any underlying is roaming boolean congested = false; // congested if any underlying is congested boolean suspended = true; // suspended if all underlying are suspended @@ -6417,13 +6435,13 @@ public class ConnectivityService extends IConnectivityManager.Stub suspended = false; } - caps.setTransportTypes(transportTypes); - caps.setLinkDownstreamBandwidthKbps(downKbps); - caps.setLinkUpstreamBandwidthKbps(upKbps); - caps.setCapability(NET_CAPABILITY_NOT_METERED, !metered); - caps.setCapability(NET_CAPABILITY_NOT_ROAMING, !roaming); - caps.setCapability(NET_CAPABILITY_NOT_CONGESTED, !congested); - caps.setCapability(NET_CAPABILITY_NOT_SUSPENDED, !suspended); + newNc.setTransportTypes(transportTypes); + newNc.setLinkDownstreamBandwidthKbps(downKbps); + newNc.setLinkUpstreamBandwidthKbps(upKbps); + newNc.setCapability(NET_CAPABILITY_NOT_METERED, !metered); + newNc.setCapability(NET_CAPABILITY_NOT_ROAMING, !roaming); + newNc.setCapability(NET_CAPABILITY_NOT_CONGESTED, !congested); + newNc.setCapability(NET_CAPABILITY_NOT_SUSPENDED, !suspended); } /** @@ -6480,7 +6498,8 @@ public class ConnectivityService extends IConnectivityManager.Stub } if (nai.supportsUnderlyingNetworks()) { - applyUnderlyingCapabilities(nai.declaredUnderlyingNetworks, newNc, nai.declaredMetered); + applyUnderlyingCapabilities(nai.declaredUnderlyingNetworks, nai.declaredCapabilities, + newNc); } return newNc; @@ -6559,11 +6578,8 @@ 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. - propagateUnderlyingNetworkCapabilities(); - } + // This network might have been underlying another network. Propagate its capabilities. + propagateUnderlyingNetworkCapabilities(nai.network); if (!newNc.equalsTransportTypes(prevNc)) { mDnsManager.updateTransportsForNetwork(nai.network.netId, newNc.getTransportTypes()); @@ -6885,8 +6901,10 @@ public class ConnectivityService extends IConnectivityManager.Stub updateTcpBufferSizes(null != newNetwork ? newNetwork.linkProperties.getTcpBufferSizes() : null); notifyIfacesChangedForNetworkStats(); - // Fix up the NetworkCapabilities of any VPNs that don't specify underlying networks. - propagateUnderlyingNetworkCapabilities(); + // Fix up the NetworkCapabilities of any networks that have this network as underlying. + if (newNetwork != null) { + propagateUnderlyingNetworkCapabilities(newNetwork.network); + } } private void processListenRequests(@NonNull final NetworkAgentInfo nai) { @@ -7342,13 +7360,11 @@ public class ConnectivityService extends IConnectivityManager.Stub networkAgent.networkCapabilities.addCapability(NET_CAPABILITY_FOREGROUND); if (!createNativeNetwork(networkAgent)) return; - if (networkAgent.isVPN()) { - // Initialize the VPN capabilities to their starting values according to the - // underlying networks. This will avoid a spurious callback to - // 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. - propagateUnderlyingNetworkCapabilities(); + if (networkAgent.supportsUnderlyingNetworks()) { + // Initialize the network's capabilities to their starting values according to the + // underlying networks. This ensures that the capabilities are correct before + // anything happens to the network. + updateCapabilitiesForNetwork(networkAgent); } networkAgent.created = true; } @@ -7390,10 +7406,6 @@ public class ConnectivityService extends IConnectivityManager.Stub // doing. updateSignalStrengthThresholds(networkAgent, "CONNECT", null); - if (networkAgent.supportsUnderlyingNetworks()) { - propagateUnderlyingNetworkCapabilities(); - } - // Consider network even though it is not yet validated. rematchAllNetworksAndRequests(); diff --git a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java index 3270dd5521..e189815c7b 100644 --- a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java +++ b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java @@ -138,9 +138,10 @@ public class NetworkAgentInfo implements Comparable { // 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; + // The capabilities originally announced by the NetworkAgent, regardless of any capabilities + // that were added or removed due to this network's underlying networks. + // Only set if #supportsUnderlyingNetworks is true. + public @Nullable NetworkCapabilities declaredCapabilities; // 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. diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index a4b6e31182..a7ba64b38e 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -5458,6 +5458,7 @@ public class ConnectivityServiceTest { final Network wifi = mWiFiNetworkAgent.getNetwork(); final NetworkCapabilities initialCaps = new NetworkCapabilities(); + initialCaps.addTransportType(TRANSPORT_VPN); initialCaps.addCapability(NET_CAPABILITY_INTERNET); initialCaps.removeCapability(NET_CAPABILITY_NOT_VPN); @@ -5489,44 +5490,45 @@ public class ConnectivityServiceTest { withWifiAndMobileUnderlying.setLinkDownstreamBandwidthKbps(10); withWifiAndMobileUnderlying.setLinkUpstreamBandwidthKbps(20); + final NetworkCapabilities initialCapsNotMetered = new NetworkCapabilities(initialCaps); + initialCapsNotMetered.addCapability(NET_CAPABILITY_NOT_METERED); + NetworkCapabilities caps = new NetworkCapabilities(initialCaps); - final boolean notDeclaredMetered = false; - mService.applyUnderlyingCapabilities(new Network[]{}, caps, notDeclaredMetered); + mService.applyUnderlyingCapabilities(new Network[]{}, initialCapsNotMetered, caps); assertEquals(withNoUnderlying, caps); caps = new NetworkCapabilities(initialCaps); - mService.applyUnderlyingCapabilities(new Network[]{null}, caps, notDeclaredMetered); + mService.applyUnderlyingCapabilities(new Network[]{null}, initialCapsNotMetered, caps); assertEquals(withNoUnderlying, caps); caps = new NetworkCapabilities(initialCaps); - mService.applyUnderlyingCapabilities(new Network[]{mobile}, caps, notDeclaredMetered); + mService.applyUnderlyingCapabilities(new Network[]{mobile}, initialCapsNotMetered, caps); assertEquals(withMobileUnderlying, caps); - mService.applyUnderlyingCapabilities(new Network[]{wifi}, caps, notDeclaredMetered); + mService.applyUnderlyingCapabilities(new Network[]{wifi}, initialCapsNotMetered, caps); assertEquals(withWifiUnderlying, caps); - final boolean isDeclaredMetered = true; withWifiUnderlying.removeCapability(NET_CAPABILITY_NOT_METERED); caps = new NetworkCapabilities(initialCaps); - mService.applyUnderlyingCapabilities(new Network[]{wifi}, caps, isDeclaredMetered); + mService.applyUnderlyingCapabilities(new Network[]{wifi}, initialCaps, caps); assertEquals(withWifiUnderlying, caps); caps = new NetworkCapabilities(initialCaps); - mService.applyUnderlyingCapabilities(new Network[]{mobile, wifi}, caps, isDeclaredMetered); + mService.applyUnderlyingCapabilities(new Network[]{mobile, wifi}, initialCaps, caps); assertEquals(withWifiAndMobileUnderlying, caps); withWifiUnderlying.addCapability(NET_CAPABILITY_NOT_METERED); caps = new NetworkCapabilities(initialCaps); mService.applyUnderlyingCapabilities(new Network[]{null, mobile, null, wifi}, - caps, notDeclaredMetered); + initialCapsNotMetered, caps); assertEquals(withWifiAndMobileUnderlying, caps); caps = new NetworkCapabilities(initialCaps); mService.applyUnderlyingCapabilities(new Network[]{null, mobile, null, wifi}, - caps, notDeclaredMetered); + initialCapsNotMetered, caps); assertEquals(withWifiAndMobileUnderlying, caps); - mService.applyUnderlyingCapabilities(null, caps, notDeclaredMetered); + mService.applyUnderlyingCapabilities(null, initialCapsNotMetered, caps); assertEquals(withWifiUnderlying, caps); }