- * This is typically (and at this time, only) used by VPN. This network is only available to + * For networks, mUids represent the list of network this applies to, and null means this + * network applies to all UIDs. + * For requests, mUids is the list of UIDs this network MUST apply to to match ; ALL UIDs + * must be included in a network so that they match. As an exception to the general rule, + * a null mUids field for requests mean "no requirements" rather than what the general rule + * would suggest ("must apply to all UIDs") : this is because this has shown to be what users + * of this API expect in practice. A network that must match all UIDs can still be + * expressed with a set ranging the entire set of possible UIDs. + *
+ * mUids is typically (and at this time, only) used by VPN. This network is only available to * the UIDs in this list, and it is their default network. Apps in this list that wish to * bypass the VPN can do so iff the VPN app allows them to or if they are privileged. If this * member is null, then the network is not restricted by app UID. If it's an empty list, then @@ -1013,8 +1022,7 @@ public final class NetworkCapabilities implements Parcelable { * @hide */ public boolean satisfiedByUids(NetworkCapabilities nc) { - if (null == nc.mUids) return true; // The network satisfies everything. - if (null == mUids) return false; // Not everything allowed but requires everything + if (null == nc.mUids || null == mUids) return true; // The network satisfies everything. for (UidRange requiredRange : mUids) { if (requiredRange.contains(nc.mEstablishingVpnAppUid)) return true; if (!nc.appliesToUidRange(requiredRange)) { diff --git a/core/java/android/net/NetworkRequest.java b/core/java/android/net/NetworkRequest.java index 97ded2d73b..f1dfbd1658 100644 --- a/core/java/android/net/NetworkRequest.java +++ b/core/java/android/net/NetworkRequest.java @@ -19,6 +19,7 @@ package android.net; import android.annotation.NonNull; import android.os.Parcel; import android.os.Parcelable; +import android.os.Process; import android.text.TextUtils; import java.util.Objects; @@ -131,12 +132,18 @@ public class NetworkRequest implements Parcelable { * needed in terms of {@link NetworkCapabilities} features */ public static class Builder { - private final NetworkCapabilities mNetworkCapabilities = new NetworkCapabilities(); + private final NetworkCapabilities mNetworkCapabilities; /** * Default constructor for Builder. */ - public Builder() {} + public Builder() { + // By default, restrict this request to networks available to this app. + // Apps can rescind this restriction, but ConnectivityService will enforce + // it for apps that do not have the NETWORK_SETTINGS permission. + mNetworkCapabilities = new NetworkCapabilities(); + mNetworkCapabilities.setSingleUid(Process.myUid()); + } /** * Build {@link NetworkRequest} give the current set of capabilities. diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index f2871e02a2..9c4feb49f6 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -17,6 +17,7 @@ package com.android.server; import static android.Manifest.permission.RECEIVE_DATA_ACTIVITY_CHANGE; +import static android.content.pm.PackageManager.PERMISSION_GRANTED; import static android.net.ConnectivityManager.CONNECTIVITY_ACTION; import static android.net.ConnectivityManager.NETID_UNSET; import static android.net.ConnectivityManager.TYPE_ETHERNET; @@ -1329,9 +1330,8 @@ public class ConnectivityService extends IConnectivityManager.Stub if (nai != null) { synchronized (nai) { if (nai.networkCapabilities != null) { - // TODO : don't remove the UIDs when communicating with processes - // that have the NETWORK_SETTINGS permission. - return networkCapabilitiesWithoutUids(nai.networkCapabilities); + return networkCapabilitiesWithoutUidsUnlessAllowed(nai.networkCapabilities, + Binder.getCallingPid(), Binder.getCallingUid()); } } } @@ -1344,10 +1344,18 @@ public class ConnectivityService extends IConnectivityManager.Stub return getNetworkCapabilitiesInternal(getNetworkAgentInfoForNetwork(network)); } - private NetworkCapabilities networkCapabilitiesWithoutUids(NetworkCapabilities nc) { + private NetworkCapabilities networkCapabilitiesWithoutUidsUnlessAllowed( + NetworkCapabilities nc, int callerPid, int callerUid) { + if (checkSettingsPermission(callerPid, callerUid)) return new NetworkCapabilities(nc); return new NetworkCapabilities(nc).setUids(null); } + private void restrictRequestUidsForCaller(NetworkCapabilities nc) { + if (!checkSettingsPermission()) { + nc.setSingleUid(Binder.getCallingUid()); + } + } + @Override public NetworkState[] getAllNetworkState() { // Require internal since we're handing out IMSI details @@ -1547,6 +1555,16 @@ public class ConnectivityService extends IConnectivityManager.Stub "ConnectivityService"); } + private boolean checkSettingsPermission() { + return PERMISSION_GRANTED == mContext.checkCallingOrSelfPermission( + android.Manifest.permission.NETWORK_SETTINGS); + } + + private boolean checkSettingsPermission(int pid, int uid) { + return PERMISSION_GRANTED == mContext.checkPermission( + android.Manifest.permission.NETWORK_SETTINGS, pid, uid); + } + private void enforceTetherAccessPermission() { mContext.enforceCallingOrSelfPermission( android.Manifest.permission.ACCESS_NETWORK_STATE, @@ -4214,13 +4232,12 @@ public class ConnectivityService extends IConnectivityManager.Stub enforceMeteredApnPolicy(networkCapabilities); } ensureRequestableCapabilities(networkCapabilities); - // Set the UID range for this request to the single UID of the requester. + // Set the UID range for this request to the single UID of the requester, or to an empty + // set of UIDs if the caller has the appropriate permission and UIDs have not been set. // This will overwrite any allowed UIDs in the requested capabilities. Though there // are no visible methods to set the UIDs, an app could use reflection to try and get // networks for other apps so it's essential that the UIDs are overwritten. - // TODO : don't forcefully set the UID when communicating with processes - // that have the NETWORK_SETTINGS permission. - networkCapabilities.setSingleUid(Binder.getCallingUid()); + restrictRequestUidsForCaller(networkCapabilities); if (timeoutMs < 0) { throw new IllegalArgumentException("Bad timeout specified"); @@ -4294,9 +4311,7 @@ public class ConnectivityService extends IConnectivityManager.Stub enforceMeteredApnPolicy(networkCapabilities); ensureRequestableCapabilities(networkCapabilities); ensureValidNetworkSpecifier(networkCapabilities); - // TODO : don't forcefully set the UID when communicating with processes - // that have the NETWORK_SETTINGS permission. - networkCapabilities.setSingleUid(Binder.getCallingUid()); + restrictRequestUidsForCaller(networkCapabilities); NetworkRequest networkRequest = new NetworkRequest(networkCapabilities, TYPE_NONE, nextNetworkRequestId(), NetworkRequest.Type.REQUEST); @@ -4350,9 +4365,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } NetworkCapabilities nc = new NetworkCapabilities(networkCapabilities); - // TODO : don't forcefully set the UIDs when communicating with processes - // that have the NETWORK_SETTINGS permission. - nc.setSingleUid(Binder.getCallingUid()); + restrictRequestUidsForCaller(nc); if (!ConnectivityManager.checkChangePermission(mContext)) { // Apps without the CHANGE_NETWORK_STATE permission can't use background networks, so // make all their listens include NET_CAPABILITY_FOREGROUND. That way, they will get @@ -4382,9 +4395,7 @@ public class ConnectivityService extends IConnectivityManager.Stub ensureValidNetworkSpecifier(networkCapabilities); final NetworkCapabilities nc = new NetworkCapabilities(networkCapabilities); - // TODO : don't forcefully set the UIDs when communicating with processes - // that have the NETWORK_SETTINGS permission. - nc.setSingleUid(Binder.getCallingUid()); + restrictRequestUidsForCaller(nc); NetworkRequest networkRequest = new NetworkRequest(nc, TYPE_NONE, nextNetworkRequestId(), NetworkRequest.Type.LISTEN); @@ -4948,8 +4959,8 @@ public class ConnectivityService extends IConnectivityManager.Stub } case ConnectivityManager.CALLBACK_CAP_CHANGED: { // networkAgent can't be null as it has been accessed a few lines above. - final NetworkCapabilities nc = - networkCapabilitiesWithoutUids(networkAgent.networkCapabilities); + final NetworkCapabilities nc = networkCapabilitiesWithoutUidsUnlessAllowed( + networkAgent.networkCapabilities, nri.mPid, nri.mUid); putParcelable(bundle, nc); break; } diff --git a/tests/net/java/android/net/NetworkCapabilitiesTest.java b/tests/net/java/android/net/NetworkCapabilitiesTest.java index 2414a8e22e..b32f0fd2be 100644 --- a/tests/net/java/android/net/NetworkCapabilitiesTest.java +++ b/tests/net/java/android/net/NetworkCapabilitiesTest.java @@ -223,7 +223,9 @@ public class NetworkCapabilitiesTest { assertFalse(netCap.appliesToUidRange(new UidRange(60, 3400))); NetworkCapabilities netCap2 = new NetworkCapabilities(); - assertFalse(netCap2.satisfiedByUids(netCap)); + // A new netcap object has null UIDs, so anything will satisfy it. + assertTrue(netCap2.satisfiedByUids(netCap)); + // Still not equal though. assertFalse(netCap2.equalsUids(netCap)); netCap2.setUids(uids); assertTrue(netCap2.satisfiedByUids(netCap)); @@ -240,7 +242,7 @@ public class NetworkCapabilitiesTest { assertTrue(netCap.appliesToUid(650)); assertFalse(netCap.appliesToUid(500)); - assertFalse(new NetworkCapabilities().satisfiedByUids(netCap)); + assertTrue(new NetworkCapabilities().satisfiedByUids(netCap)); netCap.combineCapabilities(new NetworkCapabilities()); assertTrue(netCap.appliesToUid(500)); assertTrue(netCap.appliesToUidRange(new UidRange(1, 100000))); diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 28f8122002..5ea21ea4f1 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -387,6 +387,7 @@ public class ConnectivityServiceTest { mScore = 20; break; case TRANSPORT_VPN: + mNetworkCapabilities.removeCapability(NET_CAPABILITY_NOT_VPN); mScore = ConnectivityConstants.VPN_DEFAULT_SCORE; break; default: @@ -3744,14 +3745,19 @@ public class ConnectivityServiceTest { final int uid = Process.myUid(); final TestNetworkCallback genericNetworkCallback = new TestNetworkCallback(); + final TestNetworkCallback genericNotVpnNetworkCallback = new TestNetworkCallback(); final TestNetworkCallback wifiNetworkCallback = new TestNetworkCallback(); final TestNetworkCallback vpnNetworkCallback = new TestNetworkCallback(); - final NetworkRequest genericRequest = new NetworkRequest.Builder().build(); + final NetworkRequest genericNotVpnRequest = new NetworkRequest.Builder().build(); + final NetworkRequest genericRequest = new NetworkRequest.Builder() + .removeCapability(NET_CAPABILITY_NOT_VPN).build(); final NetworkRequest wifiRequest = new NetworkRequest.Builder() .addTransportType(TRANSPORT_WIFI).build(); final NetworkRequest vpnNetworkRequest = new NetworkRequest.Builder() + .removeCapability(NET_CAPABILITY_NOT_VPN) .addTransportType(TRANSPORT_VPN).build(); mCm.registerNetworkCallback(genericRequest, genericNetworkCallback); + mCm.registerNetworkCallback(genericNotVpnRequest, genericNotVpnNetworkCallback); mCm.registerNetworkCallback(wifiRequest, wifiNetworkCallback); mCm.registerNetworkCallback(vpnNetworkRequest, vpnNetworkCallback); @@ -3759,6 +3765,7 @@ public class ConnectivityServiceTest { mWiFiNetworkAgent.connect(false); genericNetworkCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); + genericNotVpnNetworkCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); wifiNetworkCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); vpnNetworkCallback.assertNoCallback(); @@ -3773,16 +3780,19 @@ public class ConnectivityServiceTest { vpnNetworkAgent.connect(false); genericNetworkCallback.expectAvailableCallbacksUnvalidated(vpnNetworkAgent); + genericNotVpnNetworkCallback.assertNoCallback(); wifiNetworkCallback.assertNoCallback(); vpnNetworkCallback.expectAvailableCallbacksUnvalidated(vpnNetworkAgent); genericNetworkCallback.expectCallback(CallbackState.NETWORK_CAPABILITIES, vpnNetworkAgent); + genericNotVpnNetworkCallback.assertNoCallback(); vpnNetworkCallback.expectCapabilitiesLike(nc -> null == nc.getUids(), vpnNetworkAgent); ranges.clear(); vpnNetworkAgent.setUids(ranges); genericNetworkCallback.expectCallback(CallbackState.LOST, vpnNetworkAgent); + genericNotVpnNetworkCallback.assertNoCallback(); wifiNetworkCallback.assertNoCallback(); vpnNetworkCallback.expectCallback(CallbackState.LOST, vpnNetworkAgent); @@ -3790,18 +3800,21 @@ public class ConnectivityServiceTest { vpnNetworkAgent.setUids(ranges); genericNetworkCallback.expectAvailableCallbacksValidated(vpnNetworkAgent); + genericNotVpnNetworkCallback.assertNoCallback(); wifiNetworkCallback.assertNoCallback(); vpnNetworkCallback.expectAvailableCallbacksValidated(vpnNetworkAgent); mWiFiNetworkAgent.disconnect(); genericNetworkCallback.expectCallback(CallbackState.LOST, mWiFiNetworkAgent); + genericNotVpnNetworkCallback.expectCallback(CallbackState.LOST, mWiFiNetworkAgent); wifiNetworkCallback.expectCallback(CallbackState.LOST, mWiFiNetworkAgent); vpnNetworkCallback.assertNoCallback(); vpnNetworkAgent.disconnect(); genericNetworkCallback.expectCallback(CallbackState.LOST, vpnNetworkAgent); + genericNotVpnNetworkCallback.assertNoCallback(); wifiNetworkCallback.assertNoCallback(); vpnNetworkCallback.expectCallback(CallbackState.LOST, vpnNetworkAgent);