Generalize support for underlying networks.

Currently, ConnectivityService assumes that only VPNs can have
underlying networks. Make the code decide this based only on the
return value of NetworkAgentInfo#supportsUnderlyingNetworks.
This allows non-VPN network types to support underlying networks
in the future.

This requires storing the original agent's capabilities in
NetworkAgentInfo so that applyUnderlyingCapabilities can mix in
the underlying network capabilities without overwriting the
capabilities of the network itself. Currently, the only
information that applyUnderlyingCapabilities takes from the
original agent's capabilities are the metered bit (stored in
NetworkAgentInfo#declaredMetered) and the transports (assumed to
be exactly {TRANSPORT_VPN}. Store the full capabilities instead.
This is more state than needed but it ensures that we do not need
to make any changes if in the future we want to propagate new
types of information from the underlying networks.

This should have no impact on current use cases (i.e., VPNs).

There is a change in ordering: in disconnectAndDestroyNetwork,
the new code propagates underlying network capabilities before
removing the network from LegacyTypeTracker, instead of after.

This is done to simplify the new code. When the new code
propagates underlying network capabilities in response to a
change for a particular network (e.g., connect, disconnect,
capabilities change), it only considers networks that have the
changed network as underlying. Because determining the
underlying networks requires knowing the default network,
the new code runs before the default network is changed and
LegacyTypeTracker is updated.

This shouldn't have app implications because the connectivity
broadcasts sent by LegacyTypeTracker and the callbacks cannot be
ordered, since they run on separate threads with unpredictable
delays. The capability change callbacks resulting from
propagation of underlying network capabilities were already
sent before the rematch, so the callbacks themselves are not
reordered in any way.

Bug: 173331190
Test: atest FrameworksNetTests \
            CtsNetTestCases:NetworkAgentTest \
	    CtsNetTestCases:Ikev2VpnTest \
	    CtsNetTestCases:VpnServiceTest \
	    CtsNetTestCases:android.net.cts.ConnectivityDiagnosticsManagerTest \
	    HostsideVpnTests com.android.server.connectivity.VpnTest
Change-Id: Ic5353a928a3a3541dcf953c35f47277c5e295db8

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) {
+            @NonNull NetworkCapabilities agentCaps, @NonNull NetworkCapabilities newNc) {
-        final Network defaultNetwork = getNetwork(getDefaultNetwork());
+        underlyingNetworks = underlyingNetworksOrDefault(underlyingNetworks);
-        if (underlyingNetworks == null && defaultNetwork != null) {
+        int[] transportTypes = agentCaps.getTransportTypes();
-            // null underlying networks means to track the default.
-            underlyingNetworks = new Network[] { defaultNetwork };
-        }
-        int[] transportTypes = new int[] { NetworkCapabilities.TRANSPORT_VPN };
         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);
+        newNc.setTransportTypes(transportTypes);
-        caps.setLinkDownstreamBandwidthKbps(downKbps);
+        newNc.setLinkDownstreamBandwidthKbps(downKbps);
-        caps.setLinkUpstreamBandwidthKbps(upKbps);
+        newNc.setLinkUpstreamBandwidthKbps(upKbps);
-        caps.setCapability(NET_CAPABILITY_NOT_METERED, !metered);
+        newNc.setCapability(NET_CAPABILITY_NOT_METERED, !metered);
-        caps.setCapability(NET_CAPABILITY_NOT_ROAMING, !roaming);
+        newNc.setCapability(NET_CAPABILITY_NOT_ROAMING, !roaming);
-        caps.setCapability(NET_CAPABILITY_NOT_CONGESTED, !congested);
+        newNc.setCapability(NET_CAPABILITY_NOT_CONGESTED, !congested);
-        caps.setCapability(NET_CAPABILITY_NOT_SUSPENDED, !suspended);
+        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)) {
+        // This network might have been underlying another network. Propagate its capabilities.
-            // Tell VPNs about updated capabilities, since they may need to
+        propagateUnderlyingNetworkCapabilities(nai.network);
-            // bubble those changes through.
-            propagateUnderlyingNetworkCapabilities();
-        }
 
         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.
+        // Fix up the NetworkCapabilities of any networks that have this network as underlying.
-        propagateUnderlyingNetworkCapabilities();
+        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()) {
+            if (networkAgent.supportsUnderlyingNetworks()) {
-                // Initialize the VPN capabilities to their starting values according to the
+                // Initialize the network's capabilities to their starting values according to the
-                // underlying networks. This will avoid a spurious callback to
+                // underlying networks. This ensures that the capabilities are correct before
-                // onCapabilitiesUpdated being sent in updateAllVpnCapabilities below as
+                // anything happens to the network.
-                // the VPN would switch from its default, blank capabilities to those
+                updateCapabilitiesForNetwork(networkAgent);
-                // that reflect the capabilities of its underlying networks.
-                propagateUnderlyingNetworkCapabilities();
             }
             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();
 

services/core/java/com/android/server/connectivity/NetworkAgentInfo.java

@@ -138,9 +138,10 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo> {
     // 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.
+    // The capabilities originally announced by the NetworkAgent, regardless of any capabilities
-    // Only relevant if #supportsUnderlyingNetworks is true.
+    // that were added or removed due to this network's underlying networks.
-    public boolean declaredMetered;
+    // 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.

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[]{}, initialCapsNotMetered, caps);
-        mService.applyUnderlyingCapabilities(new Network[]{}, caps, notDeclaredMetered);
         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);
     }