Disallow requesting networks with mutable NetworkCapabilities.

It's not clear what it means to request a network with a mutable
NetworkCapability like NET_CAPABILITY_VALIDATED or
NET_CAPABILITY_CAPTIVE_PORTAL.  Presently requesting such a network
would fail in a number of different ways:
1. The NetworkFactories would fail to match the request against their
   filter which doesn't include stateful NetworkCapabilities.
2. If the NetworkFactories did match, they'd bring up networks to try
   and satisfy the requests, but the networks would not have any
   mutable NetworkCapabilities initially so they'd be reaped.
Because of these problems it's safest to simply disallow these
requests.

Bug: 21343774
Change-Id: I56303242b81d39b370b8d5d1e32059bfcfc25949

3 files changed, 58 insertions, 0 deletions

diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java
index 04a29aa..80476ea 100644
--- a/core/java/android/net/ConnectivityManager.java
+++ b/core/java/android/net/ConnectivityManager.java
@@ -2381,6 +2381,14 @@ public class ConnectivityManager {
      * Status of the request can be followed by listening to the various
      * callbacks described in {@link NetworkCallback}.  The {@link Network}
      * can be used to direct traffic to the network.
+     * <p>It is presently unsupported to request a network with mutable
+     * {@link NetworkCapabilities} such as
+     * {@link NetworkCapabilities#NET_CAPABILITY_VALIDATED} or
+     * {@link NetworkCapabilities#NET_CAPABILITY_CAPTIVE_PORTAL}
+     * as these {@code NetworkCapabilities} represent states that a particular
+     * network may never attain, and whether a network will attain these states
+     * is unknown prior to bringing up the network so the framework does not
+     * know how to go about satisfing a request with these capabilities.
      * <p>This method requires the caller to hold the permission
      * {@link android.Manifest.permission#CHANGE_NETWORK_STATE}.
      *
@@ -2388,6 +2396,8 @@ public class ConnectivityManager {
      * @param networkCallback The {@link NetworkCallback} to be utilized for this
      *                        request.  Note the callback must not be shared - they
      *                        uniquely specify this request.
+     * @throws IllegalArgumentException if {@code request} specifies any mutable
+     *         {@code NetworkCapabilities}.
      */
     public void requestNetwork(NetworkRequest request, NetworkCallback networkCallback) {
         sendRequestForNetwork(request.networkCapabilities, networkCallback, 0,
@@ -2469,12 +2479,22 @@ public class ConnectivityManager {
      * <p>
      * The request may be released normally by calling
      * {@link #releaseNetworkRequest(android.app.PendingIntent)}.
+     * <p>It is presently unsupported to request a network with either
+     * {@link NetworkCapabilities#NET_CAPABILITY_VALIDATED} or
+     * {@link NetworkCapabilities#NET_CAPABILITY_CAPTIVE_PORTAL}
+     * as these {@code NetworkCapabilities} represent states that a particular
+     * network may never attain, and whether a network will attain these states
+     * is unknown prior to bringing up the network so the framework does not
+     * know how to go about satisfing a request with these capabilities.
      * <p>This method requires the caller to hold the permission
      * {@link android.Manifest.permission#CHANGE_NETWORK_STATE}.
      * @param request {@link NetworkRequest} describing this request.
      * @param operation Action to perform when the network is available (corresponds
      *                  to the {@link NetworkCallback#onAvailable} call.  Typically
      *                  comes from {@link PendingIntent#getBroadcast}. Cannot be null.
+     * @throws IllegalArgumentException if {@code request} contains either
+     *         {@link NetworkCapabilities#NET_CAPABILITY_VALIDATED} or
+     *         {@link NetworkCapabilities#NET_CAPABILITY_CAPTIVE_PORTAL}.
      */
     public void requestNetwork(NetworkRequest request, PendingIntent operation) {
         checkPendingIntent(operation);
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index 7692bb6..d9e5b75 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -3578,12 +3578,24 @@ public class ConnectivityService extends IConnectivityManager.Stub
         }
     }
 
+    private void ensureImmutableCapabilities(NetworkCapabilities networkCapabilities) {
+        if (networkCapabilities.hasCapability(NET_CAPABILITY_VALIDATED)) {
+            throw new IllegalArgumentException(
+                    "Cannot request network with NET_CAPABILITY_VALIDATED");
+        }
+        if (networkCapabilities.hasCapability(NET_CAPABILITY_CAPTIVE_PORTAL)) {
+            throw new IllegalArgumentException(
+                    "Cannot request network with NET_CAPABILITY_CAPTIVE_PORTAL");
+        }
+    }
+
     @Override
     public NetworkRequest requestNetwork(NetworkCapabilities networkCapabilities,
             Messenger messenger, int timeoutMs, IBinder binder, int legacyType) {
         networkCapabilities = new NetworkCapabilities(networkCapabilities);
         enforceNetworkRequestPermissions(networkCapabilities);
         enforceMeteredApnPolicy(networkCapabilities);
+        ensureImmutableCapabilities(networkCapabilities);
 
         if (timeoutMs < 0 || timeoutMs > ConnectivityManager.MAX_NETWORK_REQUEST_TIMEOUT_MS) {
             throw new IllegalArgumentException("Bad timeout specified");
@@ -3652,6 +3664,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
         networkCapabilities = new NetworkCapabilities(networkCapabilities);
         enforceNetworkRequestPermissions(networkCapabilities);
         enforceMeteredApnPolicy(networkCapabilities);
+        ensureImmutableCapabilities(networkCapabilities);
 
         NetworkRequest networkRequest = new NetworkRequest(networkCapabilities, TYPE_NONE,
                 nextNetworkRequestId());
diff --git a/services/tests/servicestests/src/com/android/server/ConnectivityServiceTest.java b/services/tests/servicestests/src/com/android/server/ConnectivityServiceTest.java
index 712db09..fb8a5bb 100644
--- a/services/tests/servicestests/src/com/android/server/ConnectivityServiceTest.java
+++ b/services/tests/servicestests/src/com/android/server/ConnectivityServiceTest.java
@@ -34,6 +34,7 @@ import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.verify;
 
+import android.app.PendingIntent;
 import android.content.BroadcastReceiver;
 import android.content.Context;
 import android.content.ContextWrapper;
@@ -792,6 +793,30 @@ public class ConnectivityServiceTest extends AndroidTestCase {
         handlerThread.quit();
     }
 
+    @LargeTest
+    public void testNoMutableNetworkRequests() throws Exception {
+        PendingIntent pendingIntent = PendingIntent.getBroadcast(mContext, 0, new Intent("a"), 0);
+        NetworkRequest.Builder builder = new NetworkRequest.Builder();
+        builder.addCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED);
+        try {
+            mCm.requestNetwork(builder.build(), new NetworkCallback());
+            fail();
+        } catch (IllegalArgumentException expected) {}
+        try {
+            mCm.requestNetwork(builder.build(), pendingIntent);
+            fail();
+        } catch (IllegalArgumentException expected) {}
+        builder = new NetworkRequest.Builder();
+        builder.addCapability(NetworkCapabilities.NET_CAPABILITY_CAPTIVE_PORTAL);
+        try {
+            mCm.requestNetwork(builder.build(), new NetworkCallback());
+            fail();
+        } catch (IllegalArgumentException expected) {}
+        try {
+            mCm.requestNetwork(builder.build(), pendingIntent);
+            fail();
+        } catch (IllegalArgumentException expected) {}
+    }
 
 //    @Override
 //    public void tearDown() throws Exception {