Merge "Give apps with NETWORK_SETTINGS right to see any VPN." am: faed85f2b0 am: 331e50f601

am: ec7c68cb2b

Change-Id: I9a9d0eb212f6de105abc070a1eb5ff698b4e67bc

core/java/android/net/NetworkCapabilities.java

@@ -891,7 +891,16 @@ public final class NetworkCapabilities implements Parcelable {
     /**
      * List of UIDs this network applies to. No restriction if null.
      * <p>
-     * 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.
+     * <p>
+     * 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 == nc.mUids || null == mUids) return true; // The network satisfies everything.
-        if (null == mUids) return false; // Not everything allowed but requires everything
         for (UidRange requiredRange : mUids) {
             if (requiredRange.contains(nc.mEstablishingVpnAppUid)) return true;
             if (!nc.appliesToUidRange(requiredRange)) {

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 android.util.proto.ProtoOutputStream;
 
@@ -132,12 +133,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.

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;
@@ -1359,9 +1360,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
+                    return networkCapabilitiesWithoutUidsUnlessAllowed(nai.networkCapabilities,
-                    // that have the NETWORK_SETTINGS permission.
+                            Binder.getCallingPid(), Binder.getCallingUid());
-                    return networkCapabilitiesWithoutUids(nai.networkCapabilities);
                 }
             }
         }
@@ -1374,10 +1374,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
@@ -1577,6 +1585,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,
@@ -4258,13 +4276,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
+        restrictRequestUidsForCaller(networkCapabilities);
-        // that have the NETWORK_SETTINGS permission.
-        networkCapabilities.setSingleUid(Binder.getCallingUid());
 
         if (timeoutMs < 0) {
             throw new IllegalArgumentException("Bad timeout specified");
@@ -4338,9 +4355,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
         enforceMeteredApnPolicy(networkCapabilities);
         ensureRequestableCapabilities(networkCapabilities);
         ensureValidNetworkSpecifier(networkCapabilities);
-        // TODO : don't forcefully set the UID when communicating with processes
+        restrictRequestUidsForCaller(networkCapabilities);
-        // that have the NETWORK_SETTINGS permission.
-        networkCapabilities.setSingleUid(Binder.getCallingUid());
 
         NetworkRequest networkRequest = new NetworkRequest(networkCapabilities, TYPE_NONE,
                 nextNetworkRequestId(), NetworkRequest.Type.REQUEST);
@@ -4394,9 +4409,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
         }
 
         NetworkCapabilities nc = new NetworkCapabilities(networkCapabilities);
-        // TODO : don't forcefully set the UIDs when communicating with processes
+        restrictRequestUidsForCaller(nc);
-        // that have the NETWORK_SETTINGS permission.
-        nc.setSingleUid(Binder.getCallingUid());
         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
@@ -4426,9 +4439,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
+        restrictRequestUidsForCaller(nc);
-        // that have the NETWORK_SETTINGS permission.
-        nc.setSingleUid(Binder.getCallingUid());
 
         NetworkRequest networkRequest = new NetworkRequest(nc, TYPE_NONE, nextNetworkRequestId(),
                 NetworkRequest.Type.LISTEN);
@@ -4992,8 +5003,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 =
+                final NetworkCapabilities nc = networkCapabilitiesWithoutUidsUnlessAllowed(
-                        networkCapabilitiesWithoutUids(networkAgent.networkCapabilities);
+                        networkAgent.networkCapabilities, nri.mPid, nri.mUid);
                 putParcelable(bundle, nc);
                 break;
             }

tests/net/java/android/net/NetworkCapabilitiesTest.java

@@ -214,7 +214,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));
@@ -231,7 +233,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)));

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:
@@ -3748,14 +3749,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);
 
@@ -3763,6 +3769,7 @@ public class ConnectivityServiceTest {
         mWiFiNetworkAgent.connect(false);
 
         genericNetworkCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent);
+        genericNotVpnNetworkCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent);
         wifiNetworkCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent);
         vpnNetworkCallback.assertNoCallback();
 
@@ -3777,16 +3784,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);
 
@@ -3794,18 +3804,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);