Refactor Telephony phone number access checks to LegacyPermissionMgr

The TelephonyPermissions phone number access check can require several
interactions with the system_server to obtain the targetSdkVersion
and check the required permissions / appops for the requesting
package. This commit refactors all of these checks into the
LegacyPermissionManager (similar to what was previously done for the
device identifier access checks), requiring only a single request
to the system_server to check all non-subscriber access requirements.

Fixes: 159662444
Test: atest TelephonyPermissionsTest
Test: atest LegacyPermissionManagerServiceTest
Test: atest SmsManagerTest
Test: atest PhoneNumberTest
Test: atest SubscriptionControllerTest
Test: atest TelephonyManagerTest
Change-Id: I6c5cdbeecc2c4a2e200dcc33eedcb9213376f1ad

5 files changed, 385 insertions, 82 deletions

diff --git a/core/java/android/permission/ILegacyPermissionManager.aidl b/core/java/android/permission/ILegacyPermissionManager.aidl
index 3bd4bf549df0..f1f083668711 100644
--- a/core/java/android/permission/ILegacyPermissionManager.aidl
+++ b/core/java/android/permission/ILegacyPermissionManager.aidl
@@ -30,7 +30,11 @@ import android.permission.IOnPermissionsChangeListener;
  * @hide
  */
 interface ILegacyPermissionManager {
-    int checkDeviceIdentifierAccess(String packageName, String callingFeatureId, String message, int pid, int uid);
+    int checkDeviceIdentifierAccess(String packageName, String message, String callingFeatureId,
+            int pid, int uid);
+
+    int checkPhoneNumberAccess(String packageName, String message, String callingFeatureId,
+            int pid, int uid);
 
     void grantDefaultPermissionsToEnabledCarrierApps(in String[] packageNames, int userId);
 
diff --git a/core/java/android/permission/LegacyPermissionManager.java b/core/java/android/permission/LegacyPermissionManager.java
index b66dd82965d3..a4fa11b5121b 100644
--- a/core/java/android/permission/LegacyPermissionManager.java
+++ b/core/java/android/permission/LegacyPermissionManager.java
@@ -90,6 +90,36 @@ public final class LegacyPermissionManager {
     }
 
     /**
+     * Checks whether the package with the given pid/uid can read the device phone number.
+     *
+     * @param packageName      the name of the package to be checked for phone number access
+     * @param message          the message to be used for logging during phone number access
+     *                         verification
+     * @param callingFeatureId the feature in the package
+     * @param pid              the process id of the package to be checked
+     * @param uid              the uid of the package to be checked
+     * @return <ul>
+     *     <li>{@link PackageManager#PERMISSION_GRANTED} if the package is allowed phone number
+     *     access</li>
+     *     <li>{@link android.app.AppOpsManager#MODE_IGNORED} if the package does not have phone
+     *     number access but for appcompat reasons this should be a silent failure (ie return empty
+     *     or null data)</li>
+     *     <li>{@link PackageManager#PERMISSION_DENIED} if the package does not have phone number
+     *     access</li>
+     * </ul>
+     * @hide
+     */
+    public int checkPhoneNumberAccess(@Nullable String packageName, @Nullable String message,
+            @Nullable String callingFeatureId, int pid, int uid) {
+        try {
+            return mLegacyPermissionManager.checkPhoneNumberAccess(packageName, message,
+                    callingFeatureId, pid, uid);
+        } catch (RemoteException e) {
+            throw e.rethrowFromSystemServer();
+        }
+    }
+
+    /**
      * Grant default permissions to currently active LUI app
      * @param packageName The package name for the LUI app
      * @param user The user handle
diff --git a/services/core/java/com/android/server/pm/permission/LegacyPermissionManagerService.java b/services/core/java/com/android/server/pm/permission/LegacyPermissionManagerService.java
index fd9aa3e07b25..b1676d0e545f 100644
--- a/services/core/java/com/android/server/pm/permission/LegacyPermissionManagerService.java
+++ b/services/core/java/com/android/server/pm/permission/LegacyPermissionManagerService.java
@@ -21,9 +21,11 @@ import android.annotation.Nullable;
 import android.app.AppOpsManager;
 import android.app.admin.DevicePolicyManager;
 import android.content.Context;
+import android.content.pm.ApplicationInfo;
 import android.content.pm.PackageManager;
 import android.content.pm.PackageManagerInternal;
 import android.os.Binder;
+import android.os.Build;
 import android.os.Process;
 import android.os.ServiceManager;
 import android.os.UserHandle;
@@ -87,19 +89,7 @@ public class LegacyPermissionManagerService extends ILegacyPermissionManager.Stu
     @Override
     public int checkDeviceIdentifierAccess(@Nullable String packageName, @Nullable String message,
             @Nullable String callingFeatureId, int pid, int uid) {
-        // If the check is being requested by an app then only allow the app to query its own
-        // access status.
-        int callingUid = mInjector.getCallingUid();
-        int callingPid = mInjector.getCallingPid();
-        if (UserHandle.getAppId(callingUid) >= Process.FIRST_APPLICATION_UID && (callingUid != uid
-                || callingPid != pid)) {
-            String response = String.format(
-                    "Calling uid %d, pid %d cannot check device identifier access for package %s "
-                            + "(uid=%d, pid=%d)",
-                    callingUid, callingPid, packageName, uid, pid);
-            Log.w(TAG, response);
-            throw new SecurityException(response);
-        }
+        verifyCallerCanCheckAccess(packageName, message, pid, uid);
         // Allow system and root access to the device identifiers.
         final int appId = UserHandle.getAppId(uid);
         if (appId == Process.SYSTEM_UID || appId == Process.ROOT_UID) {
@@ -138,6 +128,110 @@ public class LegacyPermissionManagerService extends ILegacyPermissionManager.Stu
     }
 
     @Override
+    public int checkPhoneNumberAccess(@Nullable String packageName, @Nullable String message,
+            @Nullable String callingFeatureId, int pid, int uid) {
+        verifyCallerCanCheckAccess(packageName, message, pid, uid);
+        if (mInjector.checkPermission(android.Manifest.permission.READ_PRIVILEGED_PHONE_STATE, pid,
+                uid) == PackageManager.PERMISSION_GRANTED) {
+            // Skip checking for runtime permission since caller has privileged permission
+            return PackageManager.PERMISSION_GRANTED;
+        }
+        // if the packageName is null then just return now as the rest of the checks require a
+        // valid package name.
+        if (packageName == null) {
+            return PackageManager.PERMISSION_DENIED;
+        }
+        // If the target SDK version is below R then also check for READ_PHONE_STATE; prior to R
+        // the phone number was accessible with the READ_PHONE_STATE permission granted.
+        boolean preR = false;
+        int result = PackageManager.PERMISSION_DENIED;
+        try {
+            ApplicationInfo info = mInjector.getApplicationInfo(packageName, uid);
+            preR = info.targetSdkVersion <= Build.VERSION_CODES.Q;
+        } catch (PackageManager.NameNotFoundException nameNotFoundException) {
+        }
+        if (preR) {
+            // For target SDK < R if the READ_PHONE_STATE permission is granted but the appop
+            // is not granted then the caller should receive null / empty data instead of a
+            // potentially crashing SecurityException. Save the result of the READ_PHONE_STATE
+            // permission / appop check; if both do not pass then first check if the app meets
+            // any of the other requirements for access, if not then return the result of this
+            // check.
+            result = checkPermissionAndAppop(packageName,
+                    android.Manifest.permission.READ_PHONE_STATE,
+                    AppOpsManager.OPSTR_READ_PHONE_STATE, callingFeatureId, message, pid, uid);
+            if (result == PackageManager.PERMISSION_GRANTED) {
+                return result;
+            }
+        }
+        // Default SMS app can always read it.
+        if (checkPermissionAndAppop(packageName, null, AppOpsManager.OPSTR_WRITE_SMS,
+                callingFeatureId, message, pid, uid) == PackageManager.PERMISSION_GRANTED) {
+            return PackageManager.PERMISSION_GRANTED;
+        }
+        // Can be read with READ_PHONE_NUMBERS too.
+        if (checkPermissionAndAppop(packageName, android.Manifest.permission.READ_PHONE_NUMBERS,
+                AppOpsManager.OPSTR_READ_PHONE_NUMBERS, callingFeatureId, message, pid, uid)
+                == PackageManager.PERMISSION_GRANTED) {
+            return PackageManager.PERMISSION_GRANTED;
+        }
+        // Can be read with READ_SMS too.
+        if (checkPermissionAndAppop(packageName, android.Manifest.permission.READ_SMS,
+                AppOpsManager.OPSTR_READ_SMS, callingFeatureId, message, pid, uid)
+                == PackageManager.PERMISSION_GRANTED) {
+            return PackageManager.PERMISSION_GRANTED;
+        }
+        return result;
+    }
+
+    private void verifyCallerCanCheckAccess(String packageName, String message, int pid, int uid) {
+        // If the check is being requested by an app then only allow the app to query its own
+        // access status.
+        int callingUid = mInjector.getCallingUid();
+        int callingPid = mInjector.getCallingPid();
+        if (UserHandle.getAppId(callingUid) >= Process.FIRST_APPLICATION_UID && (callingUid != uid
+                || callingPid != pid)) {
+            String response = String.format(
+                    "Calling uid %d, pid %d cannot access for package %s (uid=%d, pid=%d): %s",
+                    callingUid, callingPid, packageName, uid, pid, message);
+            Log.w(TAG, response);
+            throw new SecurityException(response);
+        }
+    }
+
+    /**
+     * Returns whether the specified {@code packageName} with {@code pid} and {@code uid} has been
+     * granted the provided {@code permission} and {@code appop}, using the {@code callingFeatureId}
+     * and {@code message} for the {@link
+     * AppOpsManager#noteOpNoThrow(int, int, String, String, String)} call.
+
+     * @return <ul>
+     *     <li>{@link PackageManager#PERMISSION_GRANTED} if both the permission and the appop
+     *     are granted to the package</li>
+     *     <li>{@link android.app.AppOpsManager#MODE_IGNORED} if the permission is granted to the
+     *     package but the appop is not</li>
+     *     <li>{@link PackageManager#PERMISSION_DENIED} if the permission is not granted to the
+     *     package</li>
+     * </ul>
+     */
+    private int checkPermissionAndAppop(String packageName, String permission, String appop,
+            String callingFeatureId, String message, int pid, int uid) {
+        if (permission != null) {
+            if (mInjector.checkPermission(permission, pid, uid)
+                    != PackageManager.PERMISSION_GRANTED) {
+                return PackageManager.PERMISSION_DENIED;
+            }
+        }
+        AppOpsManager appOpsManager = (AppOpsManager) mInjector.getSystemService(
+                Context.APP_OPS_SERVICE);
+        if (appOpsManager.noteOpNoThrow(appop, uid, packageName, callingFeatureId, message)
+                != AppOpsManager.MODE_ALLOWED) {
+            return AppOpsManager.MODE_IGNORED;
+        }
+        return PackageManager.PERMISSION_GRANTED;
+    }
+
+    @Override
     public void grantDefaultPermissionsToActiveLuiApp(String packageName, int userId) {
         final int callingUid = Binder.getCallingUid();
         PackageManagerServiceUtils.enforceSystemOrPhoneCaller(
@@ -348,5 +442,16 @@ public class LegacyPermissionManagerService extends ILegacyPermissionManager.Stu
         public Object getSystemService(@NonNull String name) {
             return mContext.getSystemService(name);
         }
+
+        /**
+         * Returns the {@link ApplicationInfo} for the specified {@code packageName} under the
+         * provided {@code uid}.
+         */
+        public ApplicationInfo getApplicationInfo(@Nullable String packageName, int uid)
+                throws PackageManager.NameNotFoundException {
+
+            return mContext.getPackageManager().getApplicationInfoAsUser(packageName, 0,
+                    UserHandle.getUserHandleForUid(uid));
+        }
     }
 }
diff --git a/services/tests/servicestests/src/com/android/server/pm/permission/LegacyPermissionManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/pm/permission/LegacyPermissionManagerServiceTest.java
index 4f6441ff4e62..acd3fcab5e52 100644
--- a/services/tests/servicestests/src/com/android/server/pm/permission/LegacyPermissionManagerServiceTest.java
+++ b/services/tests/servicestests/src/com/android/server/pm/permission/LegacyPermissionManagerServiceTest.java
@@ -27,7 +27,9 @@ import static org.testng.Assert.assertThrows;
 import android.app.AppOpsManager;
 import android.app.admin.DevicePolicyManager;
 import android.content.Context;
+import android.content.pm.ApplicationInfo;
 import android.content.pm.PackageManager;
+import android.os.Build;
 import android.os.Process;
 
 import androidx.test.InstrumentationRegistry;
@@ -46,8 +48,12 @@ public class LegacyPermissionManagerServiceTest {
     private static final int APP_UID = Process.FIRST_APPLICATION_UID;
     private static final int APP_PID = 5678;
 
+    private static final String CHECK_DEVICE_IDENTIFIER_MESSAGE = "testCheckDeviceIdentifierAccess";
+    private static final String CHECK_PHONE_NUMBER_MESSAGE = "testCheckPhoneNumberAccess";
+
     private LegacyPermissionManagerService mLegacyPermissionManagerService;
     private Context mContext;
+    private String mPackageName;
 
     @Mock
     private LegacyPermissionManagerService.Injector mInjector;
@@ -63,6 +69,7 @@ public class LegacyPermissionManagerServiceTest {
         MockitoAnnotations.initMocks(this);
 
         mContext = InstrumentationRegistry.getContext();
+        mPackageName = mContext.getPackageName();
         mLegacyPermissionManagerService = new LegacyPermissionManagerService(mContext, mInjector);
     }
 
@@ -74,7 +81,7 @@ public class LegacyPermissionManagerServiceTest {
 
         assertThrows(SecurityException.class,
                 () -> mLegacyPermissionManagerService.checkDeviceIdentifierAccess(
-                        mContext.getPackageName(), "testCheckDeviceIdentifierAccess", null,
+                        mPackageName, CHECK_DEVICE_IDENTIFIER_MESSAGE, null,
                         APP_PID, SYSTEM_UID));
     }
 
@@ -86,7 +93,7 @@ public class LegacyPermissionManagerServiceTest {
 
         assertThrows(SecurityException.class,
                 () -> mLegacyPermissionManagerService.checkDeviceIdentifierAccess(
-                        mContext.getPackageName(), "testCheckDeviceIdentifierAccess", null,
+                        mPackageName, CHECK_DEVICE_IDENTIFIER_MESSAGE, null,
                         SYSTEM_PID, APP_UID));
     }
 
@@ -97,7 +104,7 @@ public class LegacyPermissionManagerServiceTest {
         setupCheckDeviceIdentifierAccessTest(APP_PID, APP_UID);
 
         int result = mLegacyPermissionManagerService.checkDeviceIdentifierAccess(
-                mContext.getPackageName(), "testCheckDeviceIdentifierAccess", null, APP_PID,
+                mPackageName, CHECK_DEVICE_IDENTIFIER_MESSAGE, null, APP_PID,
                 APP_UID);
 
         assertEquals(PackageManager.PERMISSION_DENIED, result);
@@ -108,7 +115,7 @@ public class LegacyPermissionManagerServiceTest {
         // The system UID should always have access to device identifiers.
         setupCheckDeviceIdentifierAccessTest(SYSTEM_PID, SYSTEM_UID);
         int result = mLegacyPermissionManagerService.checkDeviceIdentifierAccess(
-                mContext.getPackageName(), "testCheckDeviceIdentifierAccess", null, SYSTEM_PID,
+                mPackageName, CHECK_DEVICE_IDENTIFIER_MESSAGE, null, SYSTEM_PID,
                 SYSTEM_UID);
 
         assertEquals(PackageManager.PERMISSION_GRANTED, result);
@@ -123,7 +130,7 @@ public class LegacyPermissionManagerServiceTest {
                 APP_PID, APP_UID)).thenReturn(PackageManager.PERMISSION_GRANTED);
 
         int result = mLegacyPermissionManagerService.checkDeviceIdentifierAccess(
-                mContext.getPackageName(), "testCheckDeviceIdentifierAccess", null, APP_PID,
+                mPackageName, CHECK_DEVICE_IDENTIFIER_MESSAGE, null, APP_PID,
                 APP_UID);
 
         assertEquals(PackageManager.PERMISSION_GRANTED, result);
@@ -135,11 +142,11 @@ public class LegacyPermissionManagerServiceTest {
         // device identifiers.
         setupCheckDeviceIdentifierAccessTest(SYSTEM_PID, SYSTEM_UID);
         when(mAppOpsManager.noteOpNoThrow(eq(AppOpsManager.OPSTR_READ_DEVICE_IDENTIFIERS),
-                eq(APP_UID), eq(mContext.getPackageName()), any(), any())).thenReturn(
+                eq(APP_UID), eq(mPackageName), any(), any())).thenReturn(
                 AppOpsManager.MODE_ALLOWED);
 
         int result = mLegacyPermissionManagerService.checkDeviceIdentifierAccess(
-                mContext.getPackageName(), "testCheckDeviceIdentifierAccess", null, APP_PID,
+                mPackageName, CHECK_DEVICE_IDENTIFIER_MESSAGE, null, APP_PID,
                 APP_UID);
 
         assertEquals(PackageManager.PERMISSION_GRANTED, result);
@@ -150,32 +157,228 @@ public class LegacyPermissionManagerServiceTest {
         // Apps that pass a DevicePolicyManager device / profile owner check should have access to
         // device identifiers.
         setupCheckDeviceIdentifierAccessTest(SYSTEM_PID, SYSTEM_UID);
-        when(mDevicePolicyManager.hasDeviceIdentifierAccess(mContext.getPackageName(), APP_PID,
+        when(mDevicePolicyManager.hasDeviceIdentifierAccess(mPackageName, APP_PID,
                 APP_UID)).thenReturn(true);
 
         int result = mLegacyPermissionManagerService.checkDeviceIdentifierAccess(
-                mContext.getPackageName(), "testCheckDeviceIdentifierAccess", null, APP_PID,
+                mPackageName, CHECK_DEVICE_IDENTIFIER_MESSAGE, null, APP_PID,
                 APP_UID);
 
         assertEquals(PackageManager.PERMISSION_GRANTED, result);
     }
 
+    @Test
+    public void checkPhoneNumberAccess_callingAppUidMismatch_throwsException() throws Exception {
+        // An app can check its own phone number access, but an exception should be
+        // thrown if an app attempts to check the phone number access of another app's UID.
+        setupCheckPhoneNumberAccessTest(APP_PID, APP_UID);
+
+        assertThrows(SecurityException.class,
+                () -> mLegacyPermissionManagerService.checkPhoneNumberAccess(
+                        mPackageName, CHECK_PHONE_NUMBER_MESSAGE, null, APP_PID,
+                        SYSTEM_UID));
+    }
+
+    @Test
+    public void checkPhoneNumberAccess_callingAppPidMismatch_throwsException() throws Exception {
+        // An app can check its own phone number access, but an exception should be
+        // thrown if an app attempts to check the phone number access of another app's PID.
+        setupCheckPhoneNumberAccessTest(APP_PID, APP_UID);
+
+        assertThrows(SecurityException.class,
+                () -> mLegacyPermissionManagerService.checkPhoneNumberAccess(
+                        mPackageName, CHECK_PHONE_NUMBER_MESSAGE, null, SYSTEM_PID,
+                        APP_UID));
+    }
+
+    @Test
+    public void checkPhoneNumberAccess_callingAppIdWithoutAccess_returnsDenied() throws Exception {
+        // Since an app can check its own phone number access, this test verifies all checks
+        // are performed and the expected result is returned when an app does not have access.
+        setupCheckPhoneNumberAccessTest(APP_PID, APP_UID);
+
+        int result = mLegacyPermissionManagerService.checkPhoneNumberAccess(
+                mPackageName, CHECK_PHONE_NUMBER_MESSAGE, null, APP_PID, APP_UID);
+
+        assertEquals(PackageManager.PERMISSION_DENIED, result);
+    }
+
+    @Test
+    public void checkPhoneNumberAccess_nullPackageName_returnsDenied() throws Exception {
+        // While a null value can be provided for the package name only the privileged
+        // permission test would be able to proceed. Verify that a null value results in a
+        // denied response instead of a NullPointerException.
+        setupCheckPhoneNumberAccessTest(APP_PID, APP_UID);
+
+        int result = mLegacyPermissionManagerService.checkPhoneNumberAccess(mPackageName,
+                CHECK_PHONE_NUMBER_MESSAGE, null, APP_PID, APP_UID);
+
+        assertEquals(PackageManager.PERMISSION_DENIED, result);
+    }
+
+    @Test
+    public void checkPhoneNumberAccess_hasPrivilegedPermission_returnsGranted() throws Exception {
+        // An app with the READ_PRIVILEGED_PHONE_STATE should have access to the phone number.
+        setupCheckPhoneNumberAccessTest(SYSTEM_PID, SYSTEM_UID);
+        when(mInjector.checkPermission(android.Manifest.permission.READ_PRIVILEGED_PHONE_STATE,
+                APP_PID, APP_UID)).thenReturn(PackageManager.PERMISSION_GRANTED);
+
+        int result = mLegacyPermissionManagerService.checkPhoneNumberAccess(
+                null, CHECK_PHONE_NUMBER_MESSAGE, null, APP_PID, APP_UID);
+
+        assertEquals(PackageManager.PERMISSION_GRANTED, result);
+    }
+
+    @Test
+    public void checkPhoneNumberAccess_targetPreRWithReadPhoneState_returnsGranted()
+            throws Exception {
+        // Prior to R an app could access the phone number with the READ_PHONE_STATE permission, but
+        // both the permission and the appop must be granted. If the permission is granted but the
+        // appop is not then AppOpsManager#MODE_IGNORED should be returned to indicate that this
+        // should be a silent failure.
+        setupCheckPhoneNumberAccessTest(SYSTEM_PID, SYSTEM_UID);
+        setPackageTargetSdk(Build.VERSION_CODES.Q);
+
+        grantPermissionAndAppop(android.Manifest.permission.READ_PHONE_STATE, null);
+        int resultWithoutAppop = mLegacyPermissionManagerService.checkPhoneNumberAccess(
+                mPackageName, CHECK_PHONE_NUMBER_MESSAGE, null, APP_PID, APP_UID);
+        grantPermissionAndAppop(android.Manifest.permission.READ_PHONE_STATE,
+                AppOpsManager.OPSTR_READ_PHONE_STATE);
+        int resultWithAppop = mLegacyPermissionManagerService.checkPhoneNumberAccess(mPackageName,
+                CHECK_PHONE_NUMBER_MESSAGE, null, APP_PID, APP_UID);
+
+        assertEquals(AppOpsManager.MODE_IGNORED, resultWithoutAppop);
+        assertEquals(PackageManager.PERMISSION_GRANTED, resultWithAppop);
+    }
+
+    @Test
+    public void checkPhoneNumberAccess_targetRWithReadPhoneState_returnsDenied() throws Exception {
+        // Apps targeting R+ with just the READ_PHONE_STATE permission granted should not have
+        // access to the phone number; PERMISSION_DENIED should be returned both with and without
+        // the appop granted since this check should be skipped for target SDK R+.
+        setupCheckPhoneNumberAccessTest(SYSTEM_PID, SYSTEM_UID);
+
+        grantPermissionAndAppop(android.Manifest.permission.READ_PHONE_STATE, null);
+        int resultWithoutAppop = mLegacyPermissionManagerService.checkPhoneNumberAccess(
+                mPackageName, CHECK_PHONE_NUMBER_MESSAGE, null, APP_PID, APP_UID);
+        grantPermissionAndAppop(android.Manifest.permission.READ_PHONE_STATE,
+                AppOpsManager.OPSTR_READ_PHONE_STATE);
+        int resultWithAppop = mLegacyPermissionManagerService.checkPhoneNumberAccess(mPackageName,
+                CHECK_PHONE_NUMBER_MESSAGE, null, APP_PID, APP_UID);
+
+        assertEquals(PackageManager.PERMISSION_DENIED, resultWithoutAppop);
+        assertEquals(PackageManager.PERMISSION_DENIED, resultWithAppop);
+    }
+
+    @Test
+    public void checkPhoneNumberAccess_smsHandler_returnsGranted() throws Exception {
+        // The default SMS handler should have the WRITE_SMS appop granted and have access to the
+        // phone number.
+        setupCheckPhoneNumberAccessTest(APP_PID, APP_UID);
+        grantPermissionAndAppop(null, AppOpsManager.OPSTR_WRITE_SMS);
+
+        int result = mLegacyPermissionManagerService.checkPhoneNumberAccess(mPackageName,
+                CHECK_PHONE_NUMBER_MESSAGE, null, APP_PID, APP_UID);
+
+        assertEquals(PackageManager.PERMISSION_GRANTED, result);
+    }
+
+    @Test
+    public void checkPhoneNumberAccess_readPhoneNumbersGranted_returnsGranted()
+            throws Exception {
+        // Access to the phone number should be granted to an app with the READ_PHONE_NUMBERS
+        // permission and appop set.
+        setupCheckPhoneNumberAccessTest(APP_PID, APP_UID);
+
+        grantPermissionAndAppop(android.Manifest.permission.READ_PHONE_NUMBERS, null);
+        int resultWithoutAppop = mLegacyPermissionManagerService.checkPhoneNumberAccess(
+                mPackageName, CHECK_PHONE_NUMBER_MESSAGE, null, APP_PID, APP_UID);
+        grantPermissionAndAppop(android.Manifest.permission.READ_PHONE_NUMBERS,
+                AppOpsManager.OPSTR_READ_PHONE_NUMBERS);
+        int resultWithAppop = mLegacyPermissionManagerService.checkPhoneNumberAccess(mPackageName,
+                CHECK_PHONE_NUMBER_MESSAGE, null, APP_PID, APP_UID);
+
+        assertEquals(PackageManager.PERMISSION_DENIED, resultWithoutAppop);
+        assertEquals(PackageManager.PERMISSION_GRANTED, resultWithAppop);
+    }
+
+    @Test
+    public void checkPhoneNumberAccess_readSmsGranted_returnsGranted() throws Exception {
+        // Access to the phone number should be granted to an app with the READ_SMS permission and
+        // appop set.
+        setupCheckPhoneNumberAccessTest(APP_PID, APP_UID);
+
+        grantPermissionAndAppop(android.Manifest.permission.READ_SMS, null);
+        int resultWithoutAppop = mLegacyPermissionManagerService.checkPhoneNumberAccess(
+                mPackageName, CHECK_PHONE_NUMBER_MESSAGE, null, APP_PID, APP_UID);
+        grantPermissionAndAppop(android.Manifest.permission.READ_SMS, AppOpsManager.OPSTR_READ_SMS);
+        int resultWithAppop = mLegacyPermissionManagerService.checkPhoneNumberAccess(mPackageName,
+                CHECK_PHONE_NUMBER_MESSAGE, null, APP_PID, APP_UID);
+
+        assertEquals(PackageManager.PERMISSION_DENIED, resultWithoutAppop);
+        assertEquals(PackageManager.PERMISSION_GRANTED, resultWithAppop);
+    }
+
+    /**
+     * Configures device identifier access tests to fail; tests verifying access should individually
+     * set an access check to succeed to verify access when that condition is met.
+     */
     private void setupCheckDeviceIdentifierAccessTest(int callingPid, int callingUid) {
+        setupAccessTest(callingPid, callingUid);
+
+        when(mDevicePolicyManager.hasDeviceIdentifierAccess(anyString(), anyInt(),
+                anyInt())).thenReturn(false);
+        when(mInjector.getSystemService(eq(Context.DEVICE_POLICY_SERVICE))).thenReturn(
+                mDevicePolicyManager);
+    }
+
+    /**
+     * Configures phone number access tests to fail; tests verifying access should individually set
+     * an access check to succeed to verify access when that condition is met.
+     */
+    private void setupCheckPhoneNumberAccessTest(int callingPid, int callingUid) throws Exception {
+        setupAccessTest(callingPid, callingUid);
+        setPackageTargetSdk(Build.VERSION_CODES.R);
+    }
+
+    /**
+     * Configures the common mocks for any access tests using the provided {@code callingPid}
+     * and {@code callingUid}.
+     */
+    private void setupAccessTest(int callingPid, int callingUid) {
         when(mInjector.getCallingPid()).thenReturn(callingPid);
         when(mInjector.getCallingUid()).thenReturn(callingUid);
 
-        // Configure the checkDeviceIdentifierAccess tests to fail all access checks, then each test
-        // can individually set the access check to pass for verification.
         when(mInjector.checkPermission(anyString(), anyInt(), anyInt())).thenReturn(
                 PackageManager.PERMISSION_DENIED);
 
         when(mAppOpsManager.noteOpNoThrow(anyString(), anyInt(), anyString(), any(),
                 any())).thenReturn(AppOpsManager.MODE_DEFAULT);
         when(mInjector.getSystemService(eq(Context.APP_OPS_SERVICE))).thenReturn(mAppOpsManager);
+    }
 
-        when(mDevicePolicyManager.hasDeviceIdentifierAccess(anyString(), anyInt(),
-                anyInt())).thenReturn(false);
-        when(mInjector.getSystemService(eq(Context.DEVICE_POLICY_SERVICE))).thenReturn(
-                mDevicePolicyManager);
+    /**
+     * Sets the returned {@code targetSdkVersion} for the package under test.
+     */
+    private void setPackageTargetSdk(int targetSdkVersion) throws Exception {
+        ApplicationInfo appInfo = new ApplicationInfo();
+        appInfo.targetSdkVersion = targetSdkVersion;
+        when(mInjector.getApplicationInfo(eq(mPackageName), eq(APP_UID))).thenReturn(appInfo);
+    }
+
+    /**
+     * Configures the mocks to return {@code PackageManager.PERMISSION_GRANTED} for the specified
+     * {@code permission} and {@code AppOpsManager.MODE_ALLOWED} for the provided {@code appop}
+     * when queried for the package under test.
+     */
+    private void grantPermissionAndAppop(String permission, String appop) {
+        if (permission != null) {
+            when(mInjector.checkPermission(permission, APP_PID, APP_UID)).thenReturn(
+                    PackageManager.PERMISSION_GRANTED);
+        }
+        if (appop != null) {
+            when(mAppOpsManager.noteOpNoThrow(eq(appop), eq(APP_UID), eq(mPackageName), any(),
+                    any())).thenReturn(AppOpsManager.MODE_ALLOWED);
+        }
     }
 }
diff --git a/telephony/common/com/android/internal/telephony/TelephonyPermissions.java b/telephony/common/com/android/internal/telephony/TelephonyPermissions.java
index d250297e6f64..d361db2e9ee5 100644
--- a/telephony/common/com/android/internal/telephony/TelephonyPermissions.java
+++ b/telephony/common/com/android/internal/telephony/TelephonyPermissions.java
@@ -482,64 +482,25 @@ public final class TelephonyPermissions {
     public static boolean checkReadPhoneNumber(
             Context context, int subId, int pid, int uid,
             String callingPackage, @Nullable String callingFeatureId, String message) {
-        // First, check if the SDK version is below R
-        boolean preR = false;
-        try {
-            ApplicationInfo info = context.getPackageManager().getApplicationInfoAsUser(
-                    callingPackage, 0, UserHandle.getUserHandleForUid(Binder.getCallingUid()));
-            preR = info.targetSdkVersion <= Build.VERSION_CODES.Q;
-        } catch (PackageManager.NameNotFoundException nameNotFoundException) {
-        }
-        if (preR) {
-            // SDK < R allows READ_PHONE_STATE, READ_PRIVILEGED_PHONE_STATE, or carrier privilege
-            try {
-                return checkReadPhoneState(
-                        context, subId, pid, uid, callingPackage, callingFeatureId, message);
-            } catch (SecurityException readPhoneStateException) {
-            }
-        } else {
-            // SDK >= R allows READ_PRIVILEGED_PHONE_STATE or carrier privilege
-            try {
-                context.enforcePermission(
-                        android.Manifest.permission.READ_PRIVILEGED_PHONE_STATE, pid, uid, message);
-                // Skip checking for runtime permission since caller has privileged permission
-                return true;
-            } catch (SecurityException readPrivilegedPhoneStateException) {
-                if (SubscriptionManager.isValidSubscriptionId(subId)) {
-                    try {
-                        enforceCarrierPrivilege(context, subId, uid, message);
-                        // Skip checking for runtime permission since caller has carrier privilege
-                        return true;
-                    } catch (SecurityException carrierPrivilegeException) {
-                    }
-                }
-            }
-        }
-
-        // Default SMS app can always read it.
-        AppOpsManager appOps = (AppOpsManager) context.getSystemService(Context.APP_OPS_SERVICE);
-        if (appOps.noteOp(AppOpsManager.OPSTR_WRITE_SMS, uid, callingPackage, callingFeatureId,
-                null) == AppOpsManager.MODE_ALLOWED) {
+        LegacyPermissionManager permissionManager = (LegacyPermissionManager)
+                context.getSystemService(Context.LEGACY_PERMISSION_SERVICE);
+        // Apps with target SDK version < R can have the READ_PHONE_STATE permission granted with
+        // the appop denied. If PERMISSION_GRANTED is not received then check if the caller has
+        // carrier privileges; if not and the permission result is MODE_IGNORED then return false
+        // to return null data to the caller.
+        int permissionResult = permissionManager.checkPhoneNumberAccess(callingPackage, message,
+                callingFeatureId, pid, uid);
+        if (permissionResult == PackageManager.PERMISSION_GRANTED) {
             return true;
         }
-        // Can be read with READ_SMS too.
-        try {
-            context.enforcePermission(android.Manifest.permission.READ_SMS, pid, uid, message);
-            if (appOps.noteOp(AppOpsManager.OPSTR_READ_SMS, uid, callingPackage,
-                    callingFeatureId, null) == AppOpsManager.MODE_ALLOWED) {
+        if (SubscriptionManager.isValidSubscriptionId(subId)) {
+            if (TelephonyPermissions.getCarrierPrivilegeStatus(context, subId, uid)
+                    == TelephonyManager.CARRIER_PRIVILEGE_STATUS_HAS_ACCESS) {
                 return true;
             }
-        } catch (SecurityException readSmsSecurityException) {
         }
-        // Can be read with READ_PHONE_NUMBERS too.
-        try {
-            context.enforcePermission(android.Manifest.permission.READ_PHONE_NUMBERS, pid, uid,
-                    message);
-            if (appOps.noteOp(AppOpsManager.OPSTR_READ_PHONE_NUMBERS, uid, callingPackage,
-                    callingFeatureId, null) == AppOpsManager.MODE_ALLOWED) {
-                return true;
-            }
-        } catch (SecurityException readPhoneNumberSecurityException) {
+        if (permissionResult == AppOpsManager.MODE_IGNORED) {
+            return false;
         }
 
         throw new SecurityException(message + ": Neither user " + uid