From ee52d2375fa885128567800483ab8a86cb63ec89 Mon Sep 17 00:00:00 2001 From: Paul Jensen Date: Tue, 16 Jun 2015 15:11:58 -0400 Subject: [PATCH] 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 --- .../java/android/net/ConnectivityManager.java | 20 +++++++++++++++ .../android/server/ConnectivityService.java | 13 ++++++++++ .../server/ConnectivityServiceTest.java | 25 +++++++++++++++++++ 3 files changed, 58 insertions(+) diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java index 04a29aafe7..80476ea6d8 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. + *

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. *

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 { *

* The request may be released normally by calling * {@link #releaseNetworkRequest(android.app.PendingIntent)}. + *

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. *

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 7692bb647f..d9e5b756f8 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 712db09558..fb8a5bb82b 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 {