Make immutable NetworkCapabilities more explicit.

Bug: 21405941
Change-Id: Iafd738c31747b0f5f9356bed1c97f5f282830af1

core/java/android/net/NetworkCapabilities.java

@@ -38,10 +38,7 @@ public final class NetworkCapabilities implements Parcelable {
      */
     public NetworkCapabilities() {
         clearAll();
-        mNetworkCapabilities =
+        mNetworkCapabilities = DEFAULT_CAPABILITIES;
-                (1 << NET_CAPABILITY_NOT_RESTRICTED) |
-                (1 << NET_CAPABILITY_TRUSTED) |
-                (1 << NET_CAPABILITY_NOT_VPN);
     }
 
     public NetworkCapabilities(NetworkCapabilities nc) {
@@ -186,6 +183,36 @@ public final class NetworkCapabilities implements Parcelable {
     private static final int MIN_NET_CAPABILITY = NET_CAPABILITY_MMS;
     private static final int MAX_NET_CAPABILITY = NET_CAPABILITY_CAPTIVE_PORTAL;
 
+    /**
+     * Network capabilities that are expected to be mutable, i.e., can change while a particular
+     * network is connected.
+     */
+    private static final long MUTABLE_CAPABILITIES =
+            // TRUSTED can change when user explicitly connects to an untrusted network in Settings.
+            // http://b/18206275
+            (1 << NET_CAPABILITY_TRUSTED) |
+            (1 << NET_CAPABILITY_VALIDATED) |
+            (1 << NET_CAPABILITY_CAPTIVE_PORTAL);
+
+    /**
+     * Network capabilities that are not allowed in NetworkRequests. This exists because the
+     * NetworkFactory / NetworkAgent model does not deal well with the situation where a
+     * capability's presence cannot be known in advance. If such a capability is requested, then we
+     * can get into a cycle where the NetworkFactory endlessly churns out NetworkAgents that then
+     * get immediately torn down because they do not have the requested capability.
+     */
+    private static final long NON_REQUESTABLE_CAPABILITIES =
+            (1 << NET_CAPABILITY_VALIDATED) |
+            (1 << NET_CAPABILITY_CAPTIVE_PORTAL);
+
+    /**
+     * Capabilities that are set by default when the object is constructed.
+     */
+    private static final long DEFAULT_CAPABILITIES =
+            (1 << NET_CAPABILITY_NOT_RESTRICTED) |
+            (1 << NET_CAPABILITY_TRUSTED) |
+            (1 << NET_CAPABILITY_NOT_VPN);
+
     /**
      * Adds the given capability to this {@code NetworkCapability} instance.
      * Multiple capabilities may be applied sequentially.  Note that when searching
@@ -259,8 +286,30 @@ public final class NetworkCapabilities implements Parcelable {
         this.mNetworkCapabilities |= nc.mNetworkCapabilities;
     }
 
-    private boolean satisfiedByNetCapabilities(NetworkCapabilities nc) {
+    /**
-        return ((nc.mNetworkCapabilities & this.mNetworkCapabilities) == this.mNetworkCapabilities);
+     * Convenience function that returns a human-readable description of the first mutable
+     * capability we find. Used to present an error message to apps that request mutable
+     * capabilities.
+     *
+     * @hide
+     */
+    public String describeFirstNonRequestableCapability() {
+        if (hasCapability(NET_CAPABILITY_VALIDATED)) return "NET_CAPABILITY_VALIDATED";
+        if (hasCapability(NET_CAPABILITY_CAPTIVE_PORTAL)) return "NET_CAPABILITY_CAPTIVE_PORTAL";
+        // This cannot happen unless the preceding checks are incomplete.
+        if ((mNetworkCapabilities & NON_REQUESTABLE_CAPABILITIES) != 0) {
+            return "unknown non-requestable capabilities " + Long.toHexString(mNetworkCapabilities);
+        }
+        if (mLinkUpBandwidthKbps != 0 || mLinkDownBandwidthKbps != 0) return "link bandwidth";
+        return null;
+    }
+
+    private boolean satisfiedByNetCapabilities(NetworkCapabilities nc, boolean onlyImmutable) {
+        long networkCapabilities = this.mNetworkCapabilities;
+        if (onlyImmutable) {
+            networkCapabilities = networkCapabilities & ~MUTABLE_CAPABILITIES;
+        }
+        return ((nc.mNetworkCapabilities & networkCapabilities) == networkCapabilities);
     }
 
     /** @hide */
@@ -268,6 +317,11 @@ public final class NetworkCapabilities implements Parcelable {
         return (nc.mNetworkCapabilities == this.mNetworkCapabilities);
     }
 
+    private boolean equalsNetCapabilitiesImmutable(NetworkCapabilities that) {
+        return ((this.mNetworkCapabilities & ~MUTABLE_CAPABILITIES) ==
+                (that.mNetworkCapabilities & ~MUTABLE_CAPABILITIES));
+    }
+
     /**
      * Representing the transport type.  Apps should generally not care about transport.  A
      * request for a fast internet connection could be satisfied by a number of different
@@ -517,7 +571,7 @@ public final class NetworkCapabilities implements Parcelable {
 
     /**
      * Combine a set of Capabilities to this one.  Useful for coming up with the complete set
-     * {@hide}
+     * @hide
      */
     public void combineCapabilities(NetworkCapabilities nc) {
         combineNetCapabilities(nc);
@@ -527,15 +581,55 @@ public final class NetworkCapabilities implements Parcelable {
     }
 
     /**
-     * Check if our requirements are satisfied by the given Capabilities.
+     * Check if our requirements are satisfied by the given {@code NetworkCapabilities}.
-     * {@hide}
+     *
+     * @param nc the {@code NetworkCapabilities} that may or may not satisfy our requirements.
+     * @param onlyImmutable if {@code true}, do not consider mutable requirements such as link
+     *         bandwidth, signal strength, or validation / captive portal status.
+     *
+     * @hide
+     */
+    private boolean satisfiedByNetworkCapabilities(NetworkCapabilities nc, boolean onlyImmutable) {
+        return (nc != null &&
+                satisfiedByNetCapabilities(nc, onlyImmutable) &&
+                satisfiedByTransportTypes(nc) &&
+                (onlyImmutable || satisfiedByLinkBandwidths(nc)) &&
+                satisfiedBySpecifier(nc));
+    }
+
+    /**
+     * Check if our requirements are satisfied by the given {@code NetworkCapabilities}.
+     *
+     * @param nc the {@code NetworkCapabilities} that may or may not satisfy our requirements.
+     *
+     * @hide
      */
     public boolean satisfiedByNetworkCapabilities(NetworkCapabilities nc) {
-        return (nc != null &&
+        return satisfiedByNetworkCapabilities(nc, false);
-                satisfiedByNetCapabilities(nc) &&
+    }
-                satisfiedByTransportTypes(nc) &&
+
-                satisfiedByLinkBandwidths(nc) &&
+    /**
-                satisfiedBySpecifier(nc));
+     * Check if our immutable requirements are satisfied by the given {@code NetworkCapabilities}.
+     *
+     * @param nc the {@code NetworkCapabilities} that may or may not satisfy our requirements.
+     *
+     * @hide
+     */
+    public boolean satisfiedByImmutableNetworkCapabilities(NetworkCapabilities nc) {
+        return satisfiedByNetworkCapabilities(nc, true);
+    }
+
+    /**
+     * Checks that our immutable capabilities are the same as those of the given
+     * {@code NetworkCapabilities}.
+     *
+     * @hide
+     */
+    public boolean equalImmutableCapabilities(NetworkCapabilities nc) {
+        if (nc == null) return false;
+        return (equalsNetCapabilitiesImmutable(nc) &&
+                equalsTransportTypes(nc) &&
+                equalsSpecifier(nc));
     }
 
     @Override

services/core/java/com/android/server/ConnectivityService.java

@@ -1923,6 +1923,11 @@ public class ConnectivityService extends IConnectivityManager.Stub
                                 networkCapabilities.hasCapability(NET_CAPABILITY_VALIDATED)) {
                             Slog.wtf(TAG, "BUG: " + nai + " has stateful capability.");
                         }
+                        if (nai.created && !nai.networkCapabilities.equalImmutableCapabilities(
+                                networkCapabilities)) {
+                            Slog.wtf(TAG, "BUG: " + nai + " changed immutable capabilities: "
+                                    + nai.networkCapabilities + " -> " + networkCapabilities);
+                        }
                         updateCapabilities(nai, networkCapabilities);
                     }
                     break;
@@ -3550,14 +3555,10 @@ public class ConnectivityService extends IConnectivityManager.Stub
         }
     }
 
-    private void ensureImmutableCapabilities(NetworkCapabilities networkCapabilities) {
+    private void ensureRequestableCapabilities(NetworkCapabilities networkCapabilities) {
-        if (networkCapabilities.hasCapability(NET_CAPABILITY_VALIDATED)) {
+        final String badCapability = networkCapabilities.describeFirstNonRequestableCapability();
-            throw new IllegalArgumentException(
+        if (badCapability != null) {
-                    "Cannot request network with NET_CAPABILITY_VALIDATED");
+            throw new IllegalArgumentException("Cannot request network with " + badCapability);
-        }
-        if (networkCapabilities.hasCapability(NET_CAPABILITY_CAPTIVE_PORTAL)) {
-            throw new IllegalArgumentException(
-                    "Cannot request network with NET_CAPABILITY_CAPTIVE_PORTAL");
         }
     }
 
@@ -3567,7 +3568,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
         networkCapabilities = new NetworkCapabilities(networkCapabilities);
         enforceNetworkRequestPermissions(networkCapabilities);
         enforceMeteredApnPolicy(networkCapabilities);
-        ensureImmutableCapabilities(networkCapabilities);
+        ensureRequestableCapabilities(networkCapabilities);
 
         if (timeoutMs < 0 || timeoutMs > ConnectivityManager.MAX_NETWORK_REQUEST_TIMEOUT_MS) {
             throw new IllegalArgumentException("Bad timeout specified");
@@ -3636,7 +3637,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
         networkCapabilities = new NetworkCapabilities(networkCapabilities);
         enforceNetworkRequestPermissions(networkCapabilities);
         enforceMeteredApnPolicy(networkCapabilities);
-        ensureImmutableCapabilities(networkCapabilities);
+        ensureRequestableCapabilities(networkCapabilities);
 
         NetworkRequest networkRequest = new NetworkRequest(networkCapabilities, TYPE_NONE,
                 nextNetworkRequestId());