Merge changes from topic "ImproveSubControllerPermChecks" into rvc-dev am: 0670127ac2 (f5e77caf) · Commits · e / os / android_frameworks_opt_telephony

src/java/com/android/internal/telephony/SubscriptionController.java

+78 −21

Original line number	Diff line number	Diff line
		@@ -3903,20 +3903,62 @@ public class SubscriptionController extends ISub.Stub {
		// They are doing similar things except operating on different cache.
		private List<SubscriptionInfo> getSubscriptionInfoListFromCacheHelper(
		String callingPackage, String callingFeatureId, List<SubscriptionInfo> cacheSubList) {
		synchronized (mSubInfoListLock) {
		// Filter the list to only include subscriptions which the caller can manage.
		return cacheSubList.stream()
		.filter(subscriptionInfo -> {
		boolean canReadPhoneState = false;
		boolean canReadIdentifiers = false;
		boolean canReadPhoneNumber = false;
		try {
		return TelephonyPermissions.checkCallingOrSelfReadPhoneState(mContext,
		subscriptionInfo.getSubscriptionId(), callingPackage,
		canReadPhoneState = TelephonyPermissions.checkReadPhoneState(mContext,
		SubscriptionManager.INVALID_SUBSCRIPTION_ID, Binder.getCallingPid(),
		Binder.getCallingUid(), callingPackage, callingFeatureId,
		"getSubscriptionInfoList");
		// If the calling package has the READ_PHONE_STATE permission then check if the caller
		// also has access to subscriber identifiers and the phone number to ensure that the ICC
		// ID and any other unique identifiers are removed if the caller should not have access.
		if (canReadPhoneState) {
		canReadIdentifiers = hasSubscriberIdentifierAccess(
		SubscriptionManager.INVALID_SUBSCRIPTION_ID, callingPackage,
		callingFeatureId, "getSubscriptionInfoList");
		canReadPhoneNumber = hasPhoneNumberAccess(
		SubscriptionManager.INVALID_SUBSCRIPTION_ID, callingPackage,
		callingFeatureId, "getSubscriptionInfoList");
		}
		} catch (SecurityException e) {
		return false;
		// If a SecurityException is thrown during the READ_PHONE_STATE check then the only way
		// to access a subscription is to have carrier privileges for its subId; an app with
		// carrier privileges for a subscription is also granted access to all identifiers so
		// the identifier and phone number access checks are not required.
		}
		}).map(subscriptionInfo -> conditionallyRemoveIdentifiers(subscriptionInfo,
		callingPackage, callingFeatureId, "getSubscriptionInfoList"))
		.collect(Collectors.toList());

		synchronized (mSubInfoListLock) {
		// If the caller can read all phone state, just return the full list.
		if (canReadIdentifiers && canReadPhoneNumber) {
		return new ArrayList<>(cacheSubList);
		}
		// Filter the list to only include subscriptions which the caller can manage.
		List<SubscriptionInfo> subscriptions = new ArrayList<>(cacheSubList.size());
		for (SubscriptionInfo subscriptionInfo : cacheSubList) {
		int subId = subscriptionInfo.getSubscriptionId();
		boolean hasCarrierPrivileges = TelephonyPermissions.checkCarrierPrivilegeForSubId(
		mContext, subId);
		// If the caller does not have the READ_PHONE_STATE permission nor carrier
		// privileges then they cannot access the current subscription.
		if (!canReadPhoneState && !hasCarrierPrivileges) {
		continue;
		}
		// If the caller has carrier privileges then they are granted access to all
		// identifiers for their subscription.
		if (hasCarrierPrivileges) {
		subscriptions.add(subscriptionInfo);
		} else {
		// The caller does not have carrier privileges for this subId, filter the
		// identifiers in the subscription based on the results of the initial
		// permission checks.
		subscriptions.add(
		conditionallyRemoveIdentifiers(subscriptionInfo, canReadIdentifiers,
		canReadPhoneNumber));
		}
		}
		return subscriptions;
		}
		}

		@@ -3937,8 +3979,24 @@ public class SubscriptionController extends ISub.Stub {
		callingFeatureId, message);
		boolean hasPhoneNumberAccess = hasPhoneNumberAccess(subId, callingPackage, callingFeatureId,
		message);
		if (!hasIdentifierAccess \|\| !hasPhoneNumberAccess) {
		result = new SubscriptionInfo(subInfo);
		return conditionallyRemoveIdentifiers(subInfo, hasIdentifierAccess, hasPhoneNumberAccess);
		}

		/**
		* Conditionally removes identifiers from the provided {@code subInfo} based on if the calling
		* package {@code hasIdentifierAccess} and {@code hasPhoneNumberAccess} and returns the
		* potentially modified object.
		*
		* <p>If the caller specifies the package does not have identifier or phone number access
		* a clone of the provided SubscriptionInfo is created and modified to avoid altering
		* SubscriptionInfo objects in a cache.
		*/
		private SubscriptionInfo conditionallyRemoveIdentifiers(SubscriptionInfo subInfo,
		boolean hasIdentifierAccess, boolean hasPhoneNumberAccess) {
		if (hasIdentifierAccess && hasPhoneNumberAccess) {
		return subInfo;
		}
		SubscriptionInfo result = new SubscriptionInfo(subInfo);
		if (!hasIdentifierAccess) {
		result.clearIccId();
		result.clearCardString();
		@@ -3946,7 +4004,6 @@ public class SubscriptionController extends ISub.Stub {
		if (!hasPhoneNumberAccess) {
		result.clearNumber();
		}
		}
		return result;
		}

tests/telephonytests/src/com/android/internal/telephony/SubscriptionControllerTest.java

+55 −3

Original line number	Diff line number	Diff line
		@@ -1273,6 +1273,54 @@ public class SubscriptionControllerTest extends TelephonyTest {
		}
		}

		@Test
		public void testGetActiveSubscriptionInfoListWithCarrierPrivilegesOnOneSubId()
		throws Exception {
		// If an app does not have the READ_PHONE_STATE permission but has carrier privileges on one
		// out of multiple sub IDs then the SubscriptionInfo for that subId should be returned with
		// the ICC ID and phone number.
		testInsertSim();
		doReturn(2).when(mTelephonyManager).getPhoneCount();
		mSubscriptionControllerUT.addSubInfoRecord("test2", 1);
		int firstSubId = getFirstSubId();
		int secondSubId = getSubIdAtIndex(1);
		mSubscriptionControllerUT.setDisplayNumber(DISPLAY_NUMBER, secondSubId);
		setupIdentifierCarrierPrivilegesTest();
		mContextFixture.removeCallingOrSelfPermission(Manifest.permission.READ_PHONE_STATE);
		setCarrierPrivilegesForSubId(false, firstSubId);
		setCarrierPrivilegesForSubId(true, secondSubId);

		List<SubscriptionInfo> subInfoList =
		mSubscriptionControllerUT.getActiveSubscriptionInfoList(mCallingPackage,
		mCallingFeature);

		assertEquals(1, subInfoList.size());
		SubscriptionInfo subInfo = subInfoList.get(0);
		assertEquals("test2", subInfo.getIccId());
		assertEquals(DISPLAY_NUMBER, subInfo.getNumber());
		}

		@Test
		public void testGetActiveSubscriptionInfoListWithIdentifierAccessWithoutNumberAccess()
		throws Exception {
		// An app with access to device identifiers may not have access to the device phone number
		// (ie an app that passes the device / profile owner check or an app that has been granted
		// the device identifiers appop); this test verifies that an app with identifier access
		// can read the ICC ID but does not receive the phone number.
		testInsertSim();
		setupReadPhoneNumbersTest();
		setIdentifierAccess(true);

		List<SubscriptionInfo> subInfoList =
		mSubscriptionControllerUT.getActiveSubscriptionInfoList(mCallingPackage,
		mCallingFeature);

		assertEquals(1, subInfoList.size());
		SubscriptionInfo subInfo = subInfoList.get(0);
		assertEquals("test", subInfo.getIccId());
		assertEquals(UNAVAILABLE_NUMBER, subInfo.getNumber());
		}

		@Test
		public void testGetActiveSubscriptionInfoListWithPrivilegedPermission() throws Exception {
		// If the calling package has the READ_PRIVILEGED_PHONE_STATE permission or carrier
		@@ -1403,9 +1451,13 @@ public class SubscriptionControllerTest extends TelephonyTest {
		}

		private int getFirstSubId() throws Exception {
		int[] subIds = mSubscriptionControllerUT.getActiveSubIdList(/visibleOnly/false);
		assertTrue(subIds != null && subIds.length != 0);
		return subIds[0];
		return getSubIdAtIndex(0);
		}

		private int getSubIdAtIndex(int index) throws Exception {
		int[] subIds = mSubscriptionControllerUT.getActiveSubIdList(/visibileOnly/false);
		assertTrue(subIds != null && subIds.length > index);
		return subIds[index];
		}

		@Test

tests/telephonytests/src/com/android/internal/telephony/TelephonyTest.java

+9 −0

Original line number	Diff line number	Diff line
		@@ -102,6 +102,7 @@ import com.android.server.pm.PackageManagerService;
		import com.android.server.pm.permission.PermissionManagerService;

		import org.mockito.Mock;
		import org.mockito.Mockito;
		import org.mockito.MockitoAnnotations;
		import org.mockito.invocation.InvocationOnMock;
		import org.mockito.stubbing.Answer;
		@@ -839,6 +840,14 @@ public abstract class TelephonyTest {
		mTelephonyManager).getCarrierPrivilegeStatus(anyInt());
		}

		protected void setCarrierPrivilegesForSubId(boolean hasCarrierPrivileges, int subId) {
		TelephonyManager mockTelephonyManager = Mockito.mock(TelephonyManager.class);
		doReturn(mockTelephonyManager).when(mTelephonyManager).createForSubscriptionId(subId);
		doReturn(hasCarrierPrivileges ? TelephonyManager.CARRIER_PRIVILEGE_STATUS_HAS_ACCESS
		: TelephonyManager.CARRIER_PRIVILEGE_STATUS_NO_ACCESS).when(
		mockTelephonyManager).getCarrierPrivilegeStatus(anyInt());
		}

		protected final void waitForHandlerAction(Handler h, long timeoutMillis) {
		final CountDownLatch lock = new CountDownLatch(1);
		h.post(lock::countDown);