Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 74c2b8e7 authored by Michael Groover's avatar Michael Groover
Browse files

Protect Device Identifiers behind priv permission and DO/PO checks 

Bug: 110099294
Test: cts-tradefed run cts -m CtsDevicePolicyManagerTestCases \
      -t com.android.cts.devicepolicy.DeviceOwnerTest.testDeviceOwnerCanGetDeviceIdentifiers
Test: cts-tradefed run cts -m CtsDevicePolicyManagerTestCases \
      -t com.android.cts.devicepolicy.ManagedProfileTest#testGetDeviceIdentifiers
Test: cts-tradefed run cts -m CtsTelephonyTestCases -t android.telephony.cts.TelephonyManagerTest
Test: cts-tradefed run cts -m CtsPermissionTestCases -t android.permission.cts.TelephonyManagerPermissionTest

Change-Id: I203ba3bd85c3015cb3041e85e59f3b6ddfe52922
parent 2edf938f

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