Check location permission for ConnDiags last. (727856cc) · Commits · e / os / android_frameworks_base

packages/Connectivity/framework/src/android/net/ConnectivityDiagnosticsManager.java

+3 −1

Original line number	Original line	Diff line number	Diff line
	@@ -713,7 +713,9 @@ public class ConnectivityDiagnosticsManager {
	* <p>Callbacks registered by apps not meeting the above criteria will not be invoked.		* <p>Callbacks registered by apps not meeting the above criteria will not be invoked.
	*		*
	* <p>If a registering app loses its relevant permissions, any callbacks it registered will		* <p>If a registering app loses its relevant permissions, any callbacks it registered will
	* silently stop receiving callbacks.		* silently stop receiving callbacks. Note that registering apps must also have location
			* permissions to receive callbacks as some Networks may be location-bound (such as WiFi
			* networks).
	*		*
	* <p>Each register() call <b>MUST</b> use a ConnectivityDiagnosticsCallback instance that is		* <p>Each register() call <b>MUST</b> use a ConnectivityDiagnosticsCallback instance that is
	* not currently registered. If a ConnectivityDiagnosticsCallback instance is registered with		* not currently registered. If a ConnectivityDiagnosticsCallback instance is registered with

packages/Connectivity/service/src/com/android/server/ConnectivityService.java

+24 −11

Original line number	Original line	Diff line number	Diff line
	@@ -9175,36 +9175,49 @@ public class ConnectivityService extends IConnectivityManager.Stub
	return results;		return results;
	}		}

	@VisibleForTesting		private boolean hasLocationPermission(String packageName, int uid) {
	boolean checkConnectivityDiagnosticsPermissions(
	int callbackPid, int callbackUid, NetworkAgentInfo nai, String callbackPackageName) {
	if (checkNetworkStackPermission(callbackPid, callbackUid)) {
	return true;
	}

	// LocationPermissionChecker#checkLocationPermission can throw SecurityException if the uid		// LocationPermissionChecker#checkLocationPermission can throw SecurityException if the uid
	// and package name don't match. Throwing on the CS thread is not acceptable, so wrap the		// and package name don't match. Throwing on the CS thread is not acceptable, so wrap the
	// call in a try-catch.		// call in a try-catch.
	try {		try {
	if (!mLocationPermissionChecker.checkLocationPermission(		if (!mLocationPermissionChecker.checkLocationPermission(
	callbackPackageName, null /* featureId /, callbackUid, null / message */)) {		packageName, null /* featureId /, uid, null / message */)) {
	return false;		return false;
	}		}
	} catch (SecurityException e) {		} catch (SecurityException e) {
	return false;		return false;
	}		}

			return true;
			}

			private boolean ownsVpnRunningOverNetwork(int uid, Network network) {
	for (NetworkAgentInfo virtual : mNetworkAgentInfos) {		for (NetworkAgentInfo virtual : mNetworkAgentInfos) {
	if (virtual.supportsUnderlyingNetworks()		if (virtual.supportsUnderlyingNetworks()
	&& virtual.networkCapabilities.getOwnerUid() == callbackUid		&& virtual.networkCapabilities.getOwnerUid() == uid
	&& CollectionUtils.contains(virtual.declaredUnderlyingNetworks, nai.network)) {		&& CollectionUtils.contains(virtual.declaredUnderlyingNetworks, network)) {
	return true;		return true;
	}		}
	}		}

			return false;
			}

			@VisibleForTesting
			boolean checkConnectivityDiagnosticsPermissions(
			int callbackPid, int callbackUid, NetworkAgentInfo nai, String callbackPackageName) {
			if (checkNetworkStackPermission(callbackPid, callbackUid)) {
			return true;
			}

	// Administrator UIDs also contains the Owner UID		// Administrator UIDs also contains the Owner UID
	final int[] administratorUids = nai.networkCapabilities.getAdministratorUids();		final int[] administratorUids = nai.networkCapabilities.getAdministratorUids();
	return CollectionUtils.contains(administratorUids, callbackUid);		if (!CollectionUtils.contains(administratorUids, callbackUid)
			&& !ownsVpnRunningOverNetwork(callbackUid, nai.network)) {
			return false;
			}

			return hasLocationPermission(callbackPackageName, callbackUid);
	}		}

	@Override		@Override

packages/Connectivity/tests/unit/java/com/android/server/ConnectivityServiceTest.java

+10 −6

Original line number	Original line	Diff line number	Diff line
	@@ -9743,28 +9743,32 @@ public class ConnectivityServiceTest {

	@Test		@Test
	public void testCheckConnectivityDiagnosticsPermissionsWrongUidPackageName() throws Exception {		public void testCheckConnectivityDiagnosticsPermissionsWrongUidPackageName() throws Exception {
	final NetworkAgentInfo naiWithoutUid = fakeMobileNai(new NetworkCapabilities());		final int wrongUid = Process.myUid() + 1;

			final NetworkCapabilities nc = new NetworkCapabilities();
			nc.setAdministratorUids(new int[] {wrongUid});
			final NetworkAgentInfo naiWithUid = fakeMobileNai(nc);

	mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED);		mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED);

	assertFalse(		assertFalse(
	"Mismatched uid/package name should not pass the location permission check",		"Mismatched uid/package name should not pass the location permission check",
	mService.checkConnectivityDiagnosticsPermissions(		mService.checkConnectivityDiagnosticsPermissions(
	Process.myPid() + 1, Process.myUid() + 1, naiWithoutUid,		Process.myPid() + 1, wrongUid, naiWithUid, mContext.getOpPackageName()));
	mContext.getOpPackageName()));
	}		}

	@Test		@Test
	public void testCheckConnectivityDiagnosticsPermissionsNoLocationPermission() throws Exception {		public void testCheckConnectivityDiagnosticsPermissionsNoLocationPermission() throws Exception {
	final NetworkAgentInfo naiWithoutUid = fakeMobileNai(new NetworkCapabilities());		final NetworkCapabilities nc = new NetworkCapabilities();
			nc.setAdministratorUids(new int[] {Process.myUid()});
			final NetworkAgentInfo naiWithUid = fakeMobileNai(nc);

	mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED);		mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED);

	assertFalse(		assertFalse(
	"ACCESS_FINE_LOCATION permission necessary for Connectivity Diagnostics",		"ACCESS_FINE_LOCATION permission necessary for Connectivity Diagnostics",
	mService.checkConnectivityDiagnosticsPermissions(		mService.checkConnectivityDiagnosticsPermissions(
	Process.myPid(), Process.myUid(), naiWithoutUid,		Process.myPid(), Process.myUid(), naiWithUid, mContext.getOpPackageName()));
	mContext.getOpPackageName()));
	}		}

	@Test		@Test