Merge "Sanitize owner UID iff owning app does not have location permissions." am: 239908298e (8b18b535) · Commits · e / os / android_frameworks_base

core/java/android/net/ConnectivityManager.java

+5 −6

Original line number	Diff line number	Diff line
		@@ -1279,7 +1279,8 @@ public class ConnectivityManager {
		@UnsupportedAppUsage
		public NetworkCapabilities[] getDefaultNetworkCapabilitiesForUser(int userId) {
		try {
		return mService.getDefaultNetworkCapabilitiesForUser(userId);
		return mService.getDefaultNetworkCapabilitiesForUser(
		userId, mContext.getOpPackageName());
		} catch (RemoteException e) {
		throw e.rethrowFromSystemServer();
		}
		@@ -1361,7 +1362,7 @@ public class ConnectivityManager {
		@Nullable
		public NetworkCapabilities getNetworkCapabilities(@Nullable Network network) {
		try {
		return mService.getNetworkCapabilities(network);
		return mService.getNetworkCapabilities(network, mContext.getOpPackageName());
		} catch (RemoteException e) {
		throw e.rethrowFromSystemServer();
		}
		@@ -4036,10 +4037,9 @@ public class ConnectivityManager {
		@NonNull PendingIntent operation) {
		printStackTrace();
		checkPendingIntentNotNull(operation);
		final String callingPackageName = mContext.getOpPackageName();
		try {
		mService.pendingRequestForNetwork(
		request.networkCapabilities, operation, callingPackageName);
		request.networkCapabilities, operation, mContext.getOpPackageName());
		} catch (RemoteException e) {
		throw e.rethrowFromSystemServer();
		} catch (ServiceSpecificException e) {
		@@ -4151,10 +4151,9 @@ public class ConnectivityManager {
		@NonNull PendingIntent operation) {
		printStackTrace();
		checkPendingIntentNotNull(operation);
		final String callingPackageName = mContext.getOpPackageName();
		try {
		mService.pendingListenForNetwork(
		request.networkCapabilities, operation, callingPackageName);
		request.networkCapabilities, operation, mContext.getOpPackageName());
		} catch (RemoteException e) {
		throw e.rethrowFromSystemServer();
		} catch (ServiceSpecificException e) {

core/java/android/net/IConnectivityManager.aidl

+3 −2

Original line number	Diff line number	Diff line
		@@ -59,7 +59,8 @@ interface IConnectivityManager
		NetworkInfo[] getAllNetworkInfo();
		Network getNetworkForType(int networkType);
		Network[] getAllNetworks();
		NetworkCapabilities[] getDefaultNetworkCapabilitiesForUser(int userId);
		NetworkCapabilities[] getDefaultNetworkCapabilitiesForUser(
		int userId, String callingPackageName);

		boolean isNetworkSupported(int networkType);

		@@ -68,7 +69,7 @@ interface IConnectivityManager
		LinkProperties getLinkPropertiesForType(int networkType);
		LinkProperties getLinkProperties(in Network network);

		NetworkCapabilities getNetworkCapabilities(in Network network);
		NetworkCapabilities getNetworkCapabilities(in Network network, String callingPackageName);

		@UnsupportedAppUsage
		NetworkState[] getAllNetworkState();

core/java/android/net/NetworkCapabilities.java

+29 −2

Original line number	Diff line number	Diff line
		@@ -830,6 +830,23 @@ public final class NetworkCapabilities implements Parcelable {
		* <p>This field keeps track of the UID of the app that created this network and is in charge of
		* its lifecycle. This could be the UID of apps such as the Wifi network suggestor, the running
		* VPN, or Carrier Service app managing a cellular data connection.
		*
		* <p>For NetworkCapability instances being sent from ConnectivityService, this value MUST be
		* reset to Process.INVALID_UID unless all the following conditions are met:
		*
		* <ol>
		* <li>The destination app is the network owner
		* <li>The destination app has the ACCESS_FINE_LOCATION permission granted
		* <li>The user's location toggle is on
		* </ol>
		*
		* This is because the owner UID is location-sensitive. The apps that request a network could
		* know where the device is if they can tell for sure the system has connected to the network
		* they requested.
		*
		* <p>This is populated by the network agents and for the NetworkCapabilities instance sent by
		* an app to the System Server, the value MUST be reset to Process.INVALID_UID by the system
		* server.
		*/
		private int mOwnerUid = Process.INVALID_UID;

		@@ -842,7 +859,16 @@ public final class NetworkCapabilities implements Parcelable {
		}

		/**
		* Retrieves the UID of the owner app.
		* Retrieves the UID of the app that owns this network.
		*
		* <p>For user privacy reasons, this field will only be populated if:
		*
		* <ol>
		* <li>The calling app is the network owner
		* <li>The calling app has the ACCESS_FINE_LOCATION permission granted
		* <li>The user's location toggle is on
		* </ol>
		*
		*/
		public int getOwnerUid() {
		return mOwnerUid;
		@@ -880,8 +906,9 @@ public final class NetworkCapabilities implements Parcelable {
		* @param administratorUids the UIDs to be set as administrators of this Network.
		* @hide
		*/
		@NonNull
		@SystemApi
		public @NonNull NetworkCapabilities setAdministratorUids(
		public NetworkCapabilities setAdministratorUids(
		@NonNull final List<Integer> administratorUids) {
		mAdministratorUids.clear();
		mAdministratorUids.addAll(administratorUids);

services/core/java/com/android/server/ConnectivityService.java

+63 −28

Original line number	Diff line number	Diff line
		@@ -1536,7 +1536,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
		}

		@Override
		public NetworkCapabilities[] getDefaultNetworkCapabilitiesForUser(int userId) {
		public NetworkCapabilities[] getDefaultNetworkCapabilitiesForUser(
		int userId, String callingPackageName) {
		// The basic principle is: if an app's traffic could possibly go over a
		// network, without the app doing anything multinetwork-specific,
		// (hence, by "default"), then include that network's capabilities in
		@@ -1558,7 +1559,10 @@ public class ConnectivityService extends IConnectivityManager.Stub
		NetworkAgentInfo nai = getDefaultNetwork();
		NetworkCapabilities nc = getNetworkCapabilitiesInternal(nai);
		if (nc != null) {
		result.put(nai.network, nc);
		result.put(
		nai.network,
		maybeSanitizeLocationInfoForCaller(
		nc, Binder.getCallingUid(), callingPackageName));
		}

		synchronized (mVpns) {
		@@ -1568,10 +1572,12 @@ public class ConnectivityService extends IConnectivityManager.Stub
		Network[] networks = vpn.getUnderlyingNetworks();
		if (networks != null) {
		for (Network network : networks) {
		nai = getNetworkAgentInfoForNetwork(network);
		nc = getNetworkCapabilitiesInternal(nai);
		nc = getNetworkCapabilitiesInternal(network);
		if (nc != null) {
		result.put(network, nc);
		result.put(
		network,
		maybeSanitizeLocationInfoForCaller(
		nc, Binder.getCallingUid(), callingPackageName));
		}
		}
		}
		@@ -1638,20 +1644,26 @@ public class ConnectivityService extends IConnectivityManager.Stub
		}
		}

		private NetworkCapabilities getNetworkCapabilitiesInternal(Network network) {
		return getNetworkCapabilitiesInternal(getNetworkAgentInfoForNetwork(network));
		}

		private NetworkCapabilities getNetworkCapabilitiesInternal(NetworkAgentInfo nai) {
		if (nai == null) return null;
		synchronized (nai) {
		if (nai.networkCapabilities == null) return null;
		return networkCapabilitiesRestrictedForCallerPermissions(
		nai.networkCapabilities,
		Binder.getCallingPid(), Binder.getCallingUid());
		nai.networkCapabilities, Binder.getCallingPid(), Binder.getCallingUid());
		}
		}

		@Override
		public NetworkCapabilities getNetworkCapabilities(Network network) {
		public NetworkCapabilities getNetworkCapabilities(Network network, String callingPackageName) {
		mAppOpsManager.checkPackage(Binder.getCallingUid(), callingPackageName);
		enforceAccessPermission();
		return getNetworkCapabilitiesInternal(getNetworkAgentInfoForNetwork(network));
		return maybeSanitizeLocationInfoForCaller(
		getNetworkCapabilitiesInternal(network),
		Binder.getCallingUid(), callingPackageName);
		}

		@VisibleForTesting
		@@ -1667,20 +1679,34 @@ public class ConnectivityService extends IConnectivityManager.Stub
		}
		newNc.setAdministratorUids(Collections.EMPTY_LIST);

		maybeSanitizeLocationInfoForCaller(newNc, callerUid);
		return newNc;
		}

		@VisibleForTesting
		@Nullable
		NetworkCapabilities maybeSanitizeLocationInfoForCaller(
		@Nullable NetworkCapabilities nc, int callerUid, @NonNull String callerPkgName) {
		if (nc == null) {
		return null;
		}
		final NetworkCapabilities newNc = new NetworkCapabilities(nc);
		if (callerUid != newNc.getOwnerUid()) {
		newNc.setOwnerUid(INVALID_UID);
		return newNc;
		}

		private void maybeSanitizeLocationInfoForCaller(
		NetworkCapabilities nc, int callerUid) {
		// TODO(b/142072839): Conditionally reset the owner UID if the following
		// conditions are not met:
		// 1. The destination app is the network owner
		// 2. The destination app has the ACCESS_COARSE_LOCATION permission granted
		// if target SDK<29 or otherwise has the ACCESS_FINE_LOCATION permission granted
		// 3. The user's location toggle is on
		nc.setOwnerUid(INVALID_UID);
		Binder.withCleanCallingIdentity(
		() -> {
		if (!mLocationPermissionChecker.checkLocationPermission(
		callerPkgName, null /* featureId /, callerUid, null / message */)) {
		// Caller does not have the requisite location permissions. Reset the
		// owner's UID in the NetworkCapabilities.
		newNc.setOwnerUid(INVALID_UID);
		}
		}
		);

		return newNc;
		}

		private LinkProperties linkPropertiesRestrictedForCallerPermissions(
		@@ -1755,7 +1781,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
		public boolean isActiveNetworkMetered() {
		enforceAccessPermission();

		final NetworkCapabilities caps = getNetworkCapabilities(getActiveNetwork());
		final NetworkCapabilities caps = getNetworkCapabilitiesInternal(getActiveNetwork());
		if (caps != null) {
		return !caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED);
		} else {
		@@ -5322,8 +5348,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
		}

		public String toString() {
		return "uid/pid:" + mUid + "/" + mPid + " " + request +
		(mPendingIntent == null ? "" : " to trigger " + mPendingIntent);
		return "uid/pid:" + mUid + "/" + mPid + " " + request
		+ (mPendingIntent == null ? "" : " to trigger " + mPendingIntent);
		}
		}

		@@ -6416,8 +6442,13 @@ public class ConnectivityService extends IConnectivityManager.Stub
		}
		switch (notificationType) {
		case ConnectivityManager.CALLBACK_AVAILABLE: {
		putParcelable(bundle, networkCapabilitiesRestrictedForCallerPermissions(
		networkAgent.networkCapabilities, nri.mPid, nri.mUid));
		final NetworkCapabilities nc =
		networkCapabilitiesRestrictedForCallerPermissions(
		networkAgent.networkCapabilities, nri.mPid, nri.mUid);
		putParcelable(
		bundle,
		maybeSanitizeLocationInfoForCaller(
		nc, nri.mUid, nri.request.getRequestorPackageName()));
		putParcelable(bundle, linkPropertiesRestrictedForCallerPermissions(
		networkAgent.linkProperties, nri.mPid, nri.mUid));
		// For this notification, arg1 contains the blocked status.
		@@ -6430,9 +6461,13 @@ 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 = networkCapabilitiesRestrictedForCallerPermissions(
		final NetworkCapabilities netCap =
		networkCapabilitiesRestrictedForCallerPermissions(
		networkAgent.networkCapabilities, nri.mPid, nri.mUid);
		putParcelable(bundle, nc);
		putParcelable(
		bundle,
		maybeSanitizeLocationInfoForCaller(
		netCap, nri.mUid, nri.request.getRequestorPackageName()));
		break;
		}
		case ConnectivityManager.CALLBACK_IP_CHANGED: {

tests/net/java/com/android/server/ConnectivityServiceTest.java

+85 −24

Original line number	Diff line number	Diff line
		@@ -1180,6 +1180,10 @@ public class ConnectivityServiceTest {
		Arrays.asList(new UserInfo[] {
		new UserInfo(VPN_USER, "", 0),
		}));
		final ApplicationInfo applicationInfo = new ApplicationInfo();
		applicationInfo.targetSdkVersion = Build.VERSION_CODES.Q;
		when(mPackageManager.getApplicationInfoAsUser(anyString(), anyInt(), any()))
		.thenReturn(applicationInfo);

		// InstrumentationTestRunner prepares a looper, but AndroidJUnitRunner does not.
		// http://b/25897652 .
		@@ -3041,7 +3045,7 @@ public class ConnectivityServiceTest {
		networkCapabilities.addTransportType(TRANSPORT_WIFI)
		.setNetworkSpecifier(new MatchAllNetworkSpecifier());
		mService.requestNetwork(networkCapabilities, null, 0, null,
		ConnectivityManager.TYPE_WIFI, TEST_PACKAGE_NAME);
		ConnectivityManager.TYPE_WIFI, mContext.getPackageName());
		});

		class NonParcelableSpecifier extends NetworkSpecifier {
		@@ -6438,17 +6442,89 @@ public class ConnectivityServiceTest {
		assertEquals(wifiLp, mService.getActiveLinkProperties());
		}

		private void setupLocationPermissions(
		int targetSdk, boolean locationToggle, String op, String perm) throws Exception {
		final ApplicationInfo applicationInfo = new ApplicationInfo();
		applicationInfo.targetSdkVersion = targetSdk;
		when(mPackageManager.getApplicationInfoAsUser(anyString(), anyInt(), any()))
		.thenReturn(applicationInfo);

		when(mLocationManager.isLocationEnabledForUser(any())).thenReturn(locationToggle);

		if (op != null) {
		when(mAppOpsManager.noteOp(eq(op), eq(Process.myUid()), eq(mContext.getPackageName())))
		.thenReturn(AppOpsManager.MODE_ALLOWED);
		}

		if (perm != null) {
		mServiceContext.setPermission(perm, PERMISSION_GRANTED);
		}
		}

		private int getOwnerUidNetCapsForCallerPermission(int ownerUid, int callerUid) {
		final NetworkCapabilities netCap = new NetworkCapabilities().setOwnerUid(ownerUid);

		return mService
		.maybeSanitizeLocationInfoForCaller(netCap, callerUid, mContext.getPackageName())
		.getOwnerUid();
		}

		@Test
		public void testMaybeSanitizeLocationInfoForCallerWithFineLocationAfterQ() throws Exception {
		setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION,
		Manifest.permission.ACCESS_FINE_LOCATION);

		final int myUid = Process.myUid();
		assertEquals(myUid, getOwnerUidNetCapsForCallerPermission(myUid, myUid));
		}

		@Test
		public void testMaybeSanitizeLocationInfoForCallerWithCoarseLocationPreQ() throws Exception {
		setupLocationPermissions(Build.VERSION_CODES.P, true, AppOpsManager.OPSTR_COARSE_LOCATION,
		Manifest.permission.ACCESS_COARSE_LOCATION);

		final int myUid = Process.myUid();
		assertEquals(myUid, getOwnerUidNetCapsForCallerPermission(myUid, myUid));
		}

		@Test
		public void testNetworkCapabilitiesRestrictedForCallerPermissions() {
		int callerUid = Process.myUid();
		final NetworkCapabilities originalNc = new NetworkCapabilities();
		originalNc.setOwnerUid(callerUid);
		public void testMaybeSanitizeLocationInfoForCallerLocationOff() throws Exception {
		// Test that even with fine location permission, and UIDs matching, the UID is sanitized.
		setupLocationPermissions(Build.VERSION_CODES.Q, false, AppOpsManager.OPSTR_FINE_LOCATION,
		Manifest.permission.ACCESS_FINE_LOCATION);

		final int myUid = Process.myUid();
		assertEquals(Process.INVALID_UID, getOwnerUidNetCapsForCallerPermission(myUid, myUid));
		}

		@Test
		public void testMaybeSanitizeLocationInfoForCallerWrongUid() throws Exception {
		// Test that even with fine location permission, not being the owner leads to sanitization.
		setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION,
		Manifest.permission.ACCESS_FINE_LOCATION);

		final int myUid = Process.myUid();
		assertEquals(Process.INVALID_UID, getOwnerUidNetCapsForCallerPermission(myUid + 1, myUid));
		}

		@Test
		public void testMaybeSanitizeLocationInfoForCallerWithCoarseLocationAfterQ() throws Exception {
		// Test that not having fine location permission leads to sanitization.
		setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_COARSE_LOCATION,
		Manifest.permission.ACCESS_COARSE_LOCATION);

		// Test that without the location permission, the owner field is sanitized.
		final int myUid = Process.myUid();
		assertEquals(Process.INVALID_UID, getOwnerUidNetCapsForCallerPermission(myUid, myUid));
		}

		final NetworkCapabilities newNc =
		mService.networkCapabilitiesRestrictedForCallerPermissions(
		originalNc, Process.myPid(), callerUid);
		@Test
		public void testMaybeSanitizeLocationInfoForCallerWithoutLocationPermission() throws Exception {
		setupLocationPermissions(Build.VERSION_CODES.Q, true, null /* op /, null / perm */);

		assertEquals(Process.INVALID_UID, newNc.getOwnerUid());
		// Test that without the location permission, the owner field is sanitized.
		final int myUid = Process.myUid();
		assertEquals(Process.INVALID_UID, getOwnerUidNetCapsForCallerPermission(myUid, myUid));
		}

		private void setupConnectionOwnerUid(int vpnOwnerUid, @VpnManager.VpnType int vpnType)
		@@ -6734,21 +6810,6 @@ public class ConnectivityServiceTest {
		mContext.getOpPackageName()));
		}

		private void setupLocationPermissions(
		int targetSdk, boolean locationToggle, String op, String perm) throws Exception {
		final ApplicationInfo applicationInfo = new ApplicationInfo();
		applicationInfo.targetSdkVersion = targetSdk;
		when(mPackageManager.getApplicationInfoAsUser(anyString(), anyInt(), any()))
		.thenReturn(applicationInfo);

		when(mLocationManager.isLocationEnabledForUser(any())).thenReturn(locationToggle);

		when(mAppOpsManager.noteOp(eq(op), eq(Process.myUid()), eq(mContext.getPackageName())))
		.thenReturn(AppOpsManager.MODE_ALLOWED);

		mServiceContext.setPermission(perm, PERMISSION_GRANTED);
		}

		private void setUpConnectivityDiagnosticsCallback() throws Exception {
		final NetworkRequest request = new NetworkRequest.Builder().build();
		when(mConnectivityDiagnosticsCallback.asBinder()).thenReturn(mIBinder);