Merge "Protect Device Identifiers behind priv permission and DO/PO checks" (be3ffbf7) · Commits · e / os / android_frameworks_opt_telephony

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

+18 −10

Original line number	Diff line number	Diff line
		@@ -60,7 +60,7 @@ public class PhoneSubInfoController extends IPhoneSubInfo.Stub {
		}

		public String getDeviceIdForPhone(int phoneId, String callingPackage) {
		return callPhoneMethodForPhoneIdWithReadCheck(phoneId, callingPackage,
		return callPhoneMethodForPhoneIdWithReadDeviceIdentifierCheck(phoneId, callingPackage,
		"getDeviceId", (phone)-> phone.getDeviceId());
		}

		@@ -122,8 +122,8 @@ public class PhoneSubInfoController extends IPhoneSubInfo.Stub {
		}

		public String getSubscriberIdForSubscriber(int subId, String callingPackage) {
		return callPhoneMethodForSubIdWithReadCheck(subId, callingPackage, "getSubscriberId",
		(phone)-> phone.getSubscriberId());
		return callPhoneMethodForSubIdWithReadSubscriberIdentifiersCheck(subId, callingPackage,
		"getSubscriberId", (phone) -> phone.getSubscriberId());
		}

		/**
		@@ -134,8 +134,8 @@ public class PhoneSubInfoController extends IPhoneSubInfo.Stub {
		}

		public String getIccSerialNumberForSubscriber(int subId, String callingPackage) {
		return callPhoneMethodForSubIdWithReadCheck(subId, callingPackage, "getIccSerialNumber",
		(phone)-> phone.getIccSerialNumber());
		return callPhoneMethodForSubIdWithReadSubscriberIdentifiersCheck(subId, callingPackage,
		"getIccSerialNumber", (phone) -> phone.getIccSerialNumber());
		}

		public String getLine1Number(String callingPackage) {
		@@ -392,6 +392,15 @@ public class PhoneSubInfoController extends IPhoneSubInfo.Stub {
		aContext, aSubId, aCallingPackage, aMessage));
		}

		private <T> T callPhoneMethodForSubIdWithReadSubscriberIdentifiersCheck(int subId,
		String callingPackage, String message, CallPhoneMethodHelper<T> callMethodHelper) {
		return callPhoneMethodWithPermissionCheck(subId, callingPackage, message, callMethodHelper,
		(aContext, aSubId, aCallingPackage, aMessage)->
		TelephonyPermissions.checkCallingOrSelfReadSubscriberIdentifiers(
		aContext, aSubId, aCallingPackage, aMessage));
		}


		private <T> T callPhoneMethodForSubIdWithPrivilegedCheck(
		int subId, String message, CallPhoneMethodHelper<T> callMethodHelper) {
		return callPhoneMethodWithPermissionCheck(subId, null, message, callMethodHelper,
		@@ -418,8 +427,8 @@ public class PhoneSubInfoController extends IPhoneSubInfo.Stub {
		aContext, aSubId, aCallingPackage, aMessage));
		}

		private <T> T callPhoneMethodForPhoneIdWithReadCheck(int phoneId, String callingPackage,
		String message, CallPhoneMethodHelper<T> callMethodHelper) {
		private <T> T callPhoneMethodForPhoneIdWithReadDeviceIdentifierCheck(int phoneId,
		String callingPackage, String message, CallPhoneMethodHelper<T> callMethodHelper) {
		// Getting subId before doing permission check.
		if (!SubscriptionManager.isValidPhoneId(phoneId)) {
		phoneId = 0;
		@@ -428,9 +437,8 @@ public class PhoneSubInfoController extends IPhoneSubInfo.Stub {
		if (phone == null) {
		return null;
		}

		if (!TelephonyPermissions.checkCallingOrSelfReadPhoneState(
		mContext, phone.getSubId(), callingPackage, message)) {
		if (!TelephonyPermissions.checkCallingOrSelfReadDeviceIdentifiers(mContext,
		phone.getSubId(), callingPackage, message)) {
		return null;
		}

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

+98 −16

Original line number	Diff line number	Diff line
		@@ -82,6 +82,9 @@ public class PhoneSubInfoControllerTest extends TelephonyTest {
		@Test
		@SmallTest
		public void testGetDeviceIdWithOutPermission() {
		// The READ_PRIVILEGED_PHONE_STATE permission or passing a device / profile owner access
		// check is required to access device identifiers. Since neither of those are true for this
		// test each case will result in a SecurityException being thrown.
		doReturn("353626073736741").when(mPhone).getDeviceId();
		doReturn("353626073736742").when(mSecondPhone).getDeviceId();

		@@ -107,16 +110,42 @@ public class PhoneSubInfoControllerTest extends TelephonyTest {
		mContextFixture.addCallingOrSelfPermission(READ_PHONE_STATE);
		doReturn(AppOpsManager.MODE_ERRORED).when(mAppOsMgr).noteOp(
		eq(AppOpsManager.OP_READ_PHONE_STATE), anyInt(), eq(TAG));
		try {
		mPhoneSubInfoControllerUT.getDeviceIdForPhone(0, TAG);
		Assert.fail("expected Security Exception Thrown");
		} catch (Exception ex) {
		assertTrue(ex instanceof SecurityException);
		assertTrue(ex.getMessage().contains("getDeviceId"));
		}

		assertNull(mPhoneSubInfoControllerUT.getDeviceIdForPhone(0, TAG));
		assertNull(mPhoneSubInfoControllerUT.getDeviceIdForPhone(1, TAG));
		try {
		mPhoneSubInfoControllerUT.getDeviceIdForPhone(1, TAG);
		Assert.fail("expected Security Exception Thrown");
		} catch (Exception ex) {
		assertTrue(ex instanceof SecurityException);
		assertTrue(ex.getMessage().contains("getDeviceId"));
		}

		//case 3: no READ_PRIVILEGED_PHONE_STATE
		// The READ_PRIVILEGED_PHONE_STATE permission is now required to get device identifiers.
		mContextFixture.addCallingOrSelfPermission(READ_PHONE_STATE);
		doReturn(AppOpsManager.MODE_ALLOWED).when(mAppOsMgr).noteOp(
		eq(AppOpsManager.OP_READ_PHONE_STATE), anyInt(), eq(TAG));
		assertEquals("353626073736741", mPhoneSubInfoControllerUT.getDeviceIdForPhone(0, TAG));
		assertEquals("353626073736742", mPhoneSubInfoControllerUT.getDeviceIdForPhone(1, TAG));
		try {
		mPhoneSubInfoControllerUT.getDeviceIdForPhone(0, TAG);
		Assert.fail("expected Security Exception Thrown");
		} catch (Exception ex) {
		assertTrue(ex instanceof SecurityException);
		assertTrue(ex.getMessage().contains("getDeviceId"));
		}

		try {
		mPhoneSubInfoControllerUT.getDeviceIdForPhone(1, TAG);
		Assert.fail("expected Security Exception Thrown");
		} catch (Exception ex) {
		assertTrue(ex instanceof SecurityException);
		assertTrue(ex.getMessage().contains("getDeviceId"));
		}
		}

		@Test
		@@ -285,6 +314,9 @@ public class PhoneSubInfoControllerTest extends TelephonyTest {
		@Test
		@SmallTest
		public void testGetSubscriberIdWithOutPermission() {
		// The READ_PRIVILEGED_PHONE_STATE permission, carrier privileges, or passing a device /
		// profile owner access check is required to access subscriber identifiers. Since none of
		// those are true for this test each case will result in a SecurityException being thrown.
		doReturn("310260426283121").when(mPhone).getSubscriberId();
		doReturn("310260426283122").when(mSecondPhone).getSubscriberId();

		@@ -310,18 +342,42 @@ public class PhoneSubInfoControllerTest extends TelephonyTest {
		mContextFixture.addCallingOrSelfPermission(READ_PHONE_STATE);
		doReturn(AppOpsManager.MODE_ERRORED).when(mAppOsMgr).noteOp(
		eq(AppOpsManager.OP_READ_PHONE_STATE), anyInt(), eq(TAG));
		try {
		mPhoneSubInfoControllerUT.getSubscriberIdForSubscriber(0, TAG);
		Assert.fail("expected Security Exception Thrown");
		} catch (Exception ex) {
		assertTrue(ex instanceof SecurityException);
		assertTrue(ex.getMessage().contains("getSubscriberId"));
		}

		assertNull(mPhoneSubInfoControllerUT.getSubscriberIdForSubscriber(0, TAG));
		assertNull(mPhoneSubInfoControllerUT.getSubscriberIdForSubscriber(1, TAG));
		try {
		mPhoneSubInfoControllerUT.getSubscriberIdForSubscriber(1, TAG);
		Assert.fail("expected Security Exception Thrown");
		} catch (Exception ex) {
		assertTrue(ex instanceof SecurityException);
		assertTrue(ex.getMessage().contains("getSubscriberId"));
		}

		//case 3: no READ_PRIVILEGED_PHONE_STATE
		// The READ_PRIVILEGED_PHONE_STATE permission is now required to get device identifiers.
		mContextFixture.addCallingOrSelfPermission(READ_PHONE_STATE);
		doReturn(AppOpsManager.MODE_ALLOWED).when(mAppOsMgr).noteOp(
		eq(AppOpsManager.OP_READ_PHONE_STATE), anyInt(), eq(TAG));
		assertEquals("310260426283121",
		mPhoneSubInfoControllerUT.getSubscriberIdForSubscriber(0, TAG));
		assertEquals("310260426283122",
		mPhoneSubInfoControllerUT.getSubscriberIdForSubscriber(1, TAG));
		try {
		mPhoneSubInfoControllerUT.getSubscriberIdForSubscriber(0, TAG);
		Assert.fail("expected Security Exception Thrown");
		} catch (Exception ex) {
		assertTrue(ex instanceof SecurityException);
		assertTrue(ex.getMessage().contains("getSubscriberId"));
		}

		try {
		mPhoneSubInfoControllerUT.getSubscriberIdForSubscriber(1, TAG);
		Assert.fail("expected Security Exception Thrown");
		} catch (Exception ex) {
		assertTrue(ex instanceof SecurityException);
		assertTrue(ex.getMessage().contains("getSubscriberId"));
		}
		}

		@Test
		@@ -340,6 +396,9 @@ public class PhoneSubInfoControllerTest extends TelephonyTest {
		@Test
		@SmallTest
		public void testGetIccSerialNumberWithOutPermission() {
		// The READ_PRIVILEGED_PHONE_STATE permission, carrier privileges, or passing a device /
		// profile owner access check is required to access subscriber identifiers. Since none of
		// those are true for this test each case will result in a SecurityException being thrown.
		doReturn("8991101200003204510").when(mPhone).getIccSerialNumber();
		doReturn("8991101200003204511").when(mSecondPhone).getIccSerialNumber();

		@@ -365,18 +424,41 @@ public class PhoneSubInfoControllerTest extends TelephonyTest {
		mContextFixture.addCallingOrSelfPermission(READ_PHONE_STATE);
		doReturn(AppOpsManager.MODE_ERRORED).when(mAppOsMgr).noteOp(
		eq(AppOpsManager.OP_READ_PHONE_STATE), anyInt(), eq(TAG));
		try {
		mPhoneSubInfoControllerUT.getIccSerialNumberForSubscriber(0, TAG);
		Assert.fail("expected Security Exception Thrown");
		} catch (Exception ex) {
		assertTrue(ex instanceof SecurityException);
		assertTrue(ex.getMessage().contains("getIccSerialNumber"));
		}

		assertNull(mPhoneSubInfoControllerUT.getIccSerialNumberForSubscriber(0, TAG));
		assertNull(mPhoneSubInfoControllerUT.getIccSerialNumberForSubscriber(1, TAG));
		try {
		mPhoneSubInfoControllerUT.getIccSerialNumberForSubscriber(1, TAG);
		Assert.fail("expected Security Exception Thrown");
		} catch (Exception ex) {
		assertTrue(ex instanceof SecurityException);
		assertTrue(ex.getMessage().contains("getIccSerialNumber"));
		}

		//case 3: no READ_PRIVILEGED_PHONE_STATE
		mContextFixture.addCallingOrSelfPermission(READ_PHONE_STATE);
		doReturn(AppOpsManager.MODE_ALLOWED).when(mAppOsMgr).noteOp(
		eq(AppOpsManager.OP_READ_PHONE_STATE), anyInt(), eq(TAG));
		assertEquals("8991101200003204510", mPhoneSubInfoControllerUT
		.getIccSerialNumberForSubscriber(0, TAG));
		assertEquals("8991101200003204511", mPhoneSubInfoControllerUT
		.getIccSerialNumberForSubscriber(1, TAG));
		try {
		mPhoneSubInfoControllerUT.getIccSerialNumberForSubscriber(0, TAG);
		Assert.fail("expected Security Exception Thrown");
		} catch (Exception ex) {
		assertTrue(ex instanceof SecurityException);
		assertTrue(ex.getMessage().contains("getIccSerialNumber"));
		}

		try {
		mPhoneSubInfoControllerUT.getIccSerialNumberForSubscriber(1, TAG);
		Assert.fail("expected Security Exception Thrown");
		} catch (Exception ex) {
		assertTrue(ex instanceof SecurityException);
		assertTrue(ex.getMessage().contains("getIccSerialNumber"));
		}
		}

		@Test