From 6b56e9e1c0b90868a8790ef3007f237d1ead1fb7 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Wed, 8 Jul 2015 12:49:04 +0900 Subject: [PATCH] Make immutable NetworkCapabilities more explicit. Bug: 21405941 Change-Id: Iafd738c31747b0f5f9356bed1c97f5f282830af1 --- .../java/android/net/NetworkCapabilities.java | 122 ++++++++++++++++-- .../android/server/ConnectivityService.java | 21 +-- 2 files changed, 119 insertions(+), 24 deletions(-) diff --git a/core/java/android/net/NetworkCapabilities.java b/core/java/android/net/NetworkCapabilities.java index 514d24abc5..84f1d103cc 100644 --- a/core/java/android/net/NetworkCapabilities.java +++ b/core/java/android/net/NetworkCapabilities.java @@ -38,10 +38,7 @@ public final class NetworkCapabilities implements Parcelable { */ public NetworkCapabilities() { clearAll(); - mNetworkCapabilities = - (1 << NET_CAPABILITY_NOT_RESTRICTED) | - (1 << NET_CAPABILITY_TRUSTED) | - (1 << NET_CAPABILITY_NOT_VPN); + mNetworkCapabilities = DEFAULT_CAPABILITIES; } 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. - * {@hide} + * Check if our requirements are satisfied by the given {@code NetworkCapabilities}. + * + * @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 && - satisfiedByNetCapabilities(nc) && - satisfiedByTransportTypes(nc) && - satisfiedByLinkBandwidths(nc) && - satisfiedBySpecifier(nc)); + return satisfiedByNetworkCapabilities(nc, false); + } + + /** + * 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 diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index eb74ab065d..9c767a55e8 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/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) { - 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"); + private void ensureRequestableCapabilities(NetworkCapabilities networkCapabilities) { + final String badCapability = networkCapabilities.describeFirstNonRequestableCapability(); + if (badCapability != null) { + throw new IllegalArgumentException("Cannot request network with " + badCapability); } } @@ -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());