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

Commit 2d992284 authored by Michael Groover's avatar Michael Groover
Browse files

Guard getImeiForSubscriber behind new device ID requirements 

TelephonyManager#getImei which invokes PhoneInterfaceManager is
already protected by the new device identifier access restrictions.
This changes applies the same access restrictions to the
PhoneSubInfoController's getImeiForSubscriber method. The previously
ignored tests in PhoneSubInfoControllerTest are also restored.

Bug: 132380377
Bug: 117781266
Test: atest PhoneSubInfoControllerTest
Change-Id: Ia71bbaddf3293e64be9159f6d58318add1f94d0b
parent 4a3ddadc

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

+12 −4
Original line number Diff line number Diff line
@@ -63,7 +63,7 @@ public class PhoneSubInfoController extends IPhoneSubInfo.Stub { 
    }



    public String getDeviceIdForPhone(int phoneId, String callingPackage) {

        return callPhoneMethodForPhoneIdWithReadDeviceIdentifierCheck(phoneId, callingPackage,

        return callPhoneMethodForPhoneIdWithReadDeviceIdentifiersCheck(phoneId, callingPackage,

                "getDeviceId", (phone)-> phone.getDeviceId());

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



    public String getImeiForSubscriber(int subId, String callingPackage) {

        return callPhoneMethodForSubIdWithReadCheck(subId, callingPackage, "getImei",

                (phone)-> phone.getImei());

        return callPhoneMethodForSubIdWithReadDeviceIdentifiersCheck(subId, callingPackage,

                "getImei", (phone)-> phone.getImei());

    }



    public ImsiEncryptionInfo getCarrierInfoForImsiEncryption(int subId, int keyType,
@@ -397,6 +397,14 @@ public class PhoneSubInfoController extends IPhoneSubInfo.Stub { 
                                aContext, aSubId, aCallingPackage, aMessage));

    }



    private <T> T callPhoneMethodForSubIdWithReadDeviceIdentifiersCheck(int subId,

            String callingPackage, String message, CallPhoneMethodHelper<T> callMethodHelper) {

        return callPhoneMethodWithPermissionCheck(subId, callingPackage, message, callMethodHelper,

                (aContext, aSubId, aCallingPackage, aMessage)->

                        TelephonyPermissions.checkCallingOrSelfReadDeviceIdentifiers(

                                aContext, aSubId, aCallingPackage, aMessage));

    }



    private <T> T callPhoneMethodForSubIdWithReadSubscriberIdentifiersCheck(int subId,

            String callingPackage, String message, CallPhoneMethodHelper<T> callMethodHelper) {

        return callPhoneMethodWithPermissionCheck(subId, callingPackage, message, callMethodHelper,
@@ -432,7 +440,7 @@ public class PhoneSubInfoController extends IPhoneSubInfo.Stub { 
                                aContext, aSubId, aCallingPackage, aMessage));

    }



    private <T> T callPhoneMethodForPhoneIdWithReadDeviceIdentifierCheck(int phoneId,

    private <T> T callPhoneMethodForPhoneIdWithReadDeviceIdentifiersCheck(int phoneId,

            String callingPackage, String message, CallPhoneMethodHelper<T> callMethodHelper) {

        // Getting subId before doing permission check.

        if (!SubscriptionManager.isValidPhoneId(phoneId)) {

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

+40 −12
Original line number Diff line number Diff line
@@ -34,7 +34,6 @@ import android.test.suitebuilder.annotation.SmallTest; 
import org.junit.After;

import org.junit.Assert;

import org.junit.Before;

import org.junit.Ignore;

import org.junit.Test;

import org.mockito.Mock;
@@ -63,6 +62,14 @@ public class PhoneSubInfoControllerTest extends TelephonyTest { 


        mPhoneSubInfoControllerUT = new PhoneSubInfoController(mContext,

                new Phone[]{mPhone, mSecondPhone});



        setupMocksForTelephonyPermissions();

        // TelephonyPermissions will query the READ_DEVICE_IDENTIFIERS op from AppOpManager to

        // determine if the calling package should be granted access to device identifiers. To

        // ensure this appop does not interfere with any of the tests always return its default

        // value.

        doReturn(AppOpsManager.MODE_ERRORED).when(mAppOsMgr).noteOpNoThrow(

                eq(AppOpsManager.OPSTR_READ_DEVICE_IDENTIFIERS), anyInt(), eq(TAG));

    }



    @After
@@ -82,8 +89,6 @@ public class PhoneSubInfoControllerTest extends TelephonyTest { 


    @Test

    @SmallTest

    @Ignore("Temporarily ignoring until original device identifier access behavior is restored "

            + "under b/117781266")

    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
@@ -189,7 +194,6 @@ 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));



        assertNull(mPhoneSubInfoControllerUT.getNaiForSubscriber(0, TAG));

        assertNull(mPhoneSubInfoControllerUT.getNaiForSubscriber(1, TAG));
@@ -214,6 +218,9 @@ public class PhoneSubInfoControllerTest extends TelephonyTest { 
    @Test

    @SmallTest

    public void testGetImeiWithOutPermission() {

        // The READ_PRIVILEGED_PHONE_STATE permission, carrier privileges, or passing a device /

        // profile owner access check is required to access device identifiers. Since none of

        // those are true for this test each case will result in a SecurityException being thrown.

        doReturn("990000862471854").when(mPhone).getImei();

        doReturn("990000862471855").when(mSecondPhone).getImei();
@@ -239,16 +246,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.getImeiForSubscriber(0, TAG);

            Assert.fail("expected Security Exception Thrown");

        } catch (Exception ex) {

            assertTrue(ex instanceof SecurityException);

            assertTrue(ex.getMessage().contains("getImei"));

        }



        assertNull(mPhoneSubInfoControllerUT.getImeiForSubscriber(0, TAG));

        assertNull(mPhoneSubInfoControllerUT.getImeiForSubscriber(1, TAG));

        try {

            mPhoneSubInfoControllerUT.getImeiForSubscriber(1, TAG);

            Assert.fail("expected Security Exception Thrown");

        } catch (Exception ex) {

            assertTrue(ex instanceof SecurityException);

            assertTrue(ex.getMessage().contains("getImei"));

        }



        //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("990000862471854", mPhoneSubInfoControllerUT.getImeiForSubscriber(0, TAG));

        assertEquals("990000862471855", mPhoneSubInfoControllerUT.getImeiForSubscriber(1, TAG));

        try {

            mPhoneSubInfoControllerUT.getImeiForSubscriber(0, TAG);

            Assert.fail("expected Security Exception Thrown");

        } catch (Exception ex) {

            assertTrue(ex instanceof SecurityException);

            assertTrue(ex.getMessage().contains("getImei"));

        }



        try {

            mPhoneSubInfoControllerUT.getImeiForSubscriber(1, TAG);

            Assert.fail("expected Security Exception Thrown");

        } catch (Exception ex) {

            assertTrue(ex instanceof SecurityException);

            assertTrue(ex.getMessage().contains("getImei"));

        }

    }



    @Test
@@ -316,8 +348,6 @@ public class PhoneSubInfoControllerTest extends TelephonyTest { 


    @Test

    @SmallTest

    @Ignore("Temporarily ignoring until original device identifier access behavior is restored "

            + "under b/117781266")

    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
@@ -400,8 +430,6 @@ public class PhoneSubInfoControllerTest extends TelephonyTest { 


    @Test

    @SmallTest

    @Ignore("Temporarily ignoring until original device identifier access behavior is restored "

            + "under b/117781266")

    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

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

+67 −0
Original line number Diff line number Diff line
@@ -33,8 +33,10 @@ import android.content.Context; 
import android.content.IIntentSender;

import android.content.Intent;

import android.content.SharedPreferences;

import android.content.pm.ApplicationInfo;

import android.content.pm.PackageInfo;

import android.content.pm.PackageManager;

import android.os.Build;

import android.os.Bundle;

import android.os.Handler;

import android.os.IBinder;
@@ -43,6 +45,7 @@ import android.os.Looper; 
import android.os.RegistrantList;

import android.os.ServiceManager;

import android.provider.BlockedNumberContract;

import android.provider.DeviceConfig;

import android.provider.Settings;

import android.telephony.AccessNetworkConstants;

import android.telephony.NetworkRegistrationInfo;
@@ -186,6 +189,8 @@ public abstract class TelephonyTest { 
    @Mock

    protected PackageInfo mPackageInfo;

    @Mock

    protected ApplicationInfo mApplicationInfo;

    @Mock

    protected EriManager mEriManager;

    @Mock

    protected IBinder mConnMetLoggerBinder;
@@ -601,6 +606,36 @@ public abstract class TelephonyTest { 
        }

    }



    public static class FakeSettingsConfigProvider extends MockContentProvider {

        private static final String PROPERTY_DEVICE_IDENTIFIER_ACCESS_RESTRICTIONS_DISABLED =

                DeviceConfig.NAMESPACE_PRIVACY + "/"

                        + "device_identifier_access_restrictions_disabled";



        @Override

        public Bundle call(String method, String arg, Bundle extras) {

            switch (method) {

                case Settings.CALL_METHOD_GET_CONFIG: {

                    switch (arg) {

                        case PROPERTY_DEVICE_IDENTIFIER_ACCESS_RESTRICTIONS_DISABLED: {

                            Bundle bundle = new Bundle();

                            bundle.putString(

                                    PROPERTY_DEVICE_IDENTIFIER_ACCESS_RESTRICTIONS_DISABLED,

                                    "0");

                            return bundle;

                        }

                        default: {

                            fail("arg not expected: " + arg);

                        }

                    }

                    break;

                }

                default:

                    fail("Method not expected: " + method);

            }

            return null;

        }

    }



    protected void setupMockPackagePermissionChecks() throws Exception {

        doReturn(new String[]{TAG}).when(mPackageManager).getPackagesForUid(anyInt());

        doReturn(mPackageInfo).when(mPackageManager).getPackageInfo(eq(TAG), anyInt());
@@ -608,6 +643,38 @@ public abstract class TelephonyTest { 
                eq(TAG), anyInt(), anyInt());

    }



    protected void setupMocksForTelephonyPermissions() throws Exception {

        // If the calling package does not meet the new requirements for device identifier access

        // TelephonyPermissions will query the PackageManager for the ApplicationInfo of the package

        // to determine the target SDK. For apps targeting Q a SecurityException is thrown

        // regardless of if the package satisfies the previous requirements for device ID access.

        mApplicationInfo.targetSdkVersion = Build.VERSION_CODES.Q;

        doReturn(mApplicationInfo).when(mPackageManager).getApplicationInfoAsUser(eq(TAG), anyInt(),

                anyInt());



        // TelephonyPermissions queries DeviceConfig to determine if the identifier access

        // restrictions should be enabled; this results in a NPE when DeviceConfig uses

        // Activity.currentActivity.getContentResolver as the resolver for Settings.Config.getString

        // since the IContentProvider in the NameValueCache's provider holder is null.

        Class c = Class.forName("android.provider.Settings$Config");

        Field field = c.getDeclaredField("sNameValueCache");

        field.setAccessible(true);

        Object cache = field.get(null);



        c = Class.forName("android.provider.Settings$NameValueCache");

        field = c.getDeclaredField("mProviderHolder");

        field.setAccessible(true);

        Object providerHolder = field.get(cache);



        FakeSettingsConfigProvider fakeSettingsProvider = new FakeSettingsConfigProvider();

        field = MockContentProvider.class.getDeclaredField("mIContentProvider");

        field.setAccessible(true);

        Object iContentProvider = field.get(fakeSettingsProvider);



        replaceInstance(Class.forName("android.provider.Settings$ContentProviderHolder"),

                "mContentProvider", providerHolder, iContentProvider);

    }



    protected final void waitForHandlerAction(Handler h, long timeoutMillis) {

        final CountDownLatch lock = new CountDownLatch(1);

        h.post(lock::countDown);