From 4bff428285321e0d79b671021c8c53047a50fe58 Mon Sep 17 00:00:00 2001
From: Malcolm Chen <refuhoo@google.com>
Date: Wed, 6 Nov 2019 18:54:55 -0800
Subject: [PATCH] Don't allow reading IMSI of one active sub if only has
 carrier privilege on the other.

Bug: 136123413
Test: unittest
Change-Id: I67a0121d3e965f193be9e827533646423502fe70
---
 .../android/telephony/TelephonyManager.java   | 19 +++---
 .../telephony/TelephonyPermissions.java       | 60 ++++++++++++-------
 2 files changed, 50 insertions(+), 29 deletions(-)

diff --git a/telephony/java/android/telephony/TelephonyManager.java b/telephony/java/android/telephony/TelephonyManager.java
index 8c2145ab15513..8a7125f76c7aa 100644
--- a/telephony/java/android/telephony/TelephonyManager.java
+++ b/telephony/java/android/telephony/TelephonyManager.java
@@ -1589,8 +1589,8 @@ public class TelephonyManager {
      *
      * <p>Requires Permission: READ_PRIVILEGED_PHONE_STATE, for the calling app to be the device or
      * profile owner and have the READ_PHONE_STATE permission, or that the calling app has carrier
-     * privileges (see {@link #hasCarrierPrivileges}). The profile owner is an app that owns a
-     * managed profile on the device; for more details see <a
+     * privileges (see {@link #hasCarrierPrivileges}) on any active subscription. The profile owner
+     * is an app that owns a managed profile on the device; for more details see <a
      * href="https://developer.android.com/work/managed-profiles">Work profiles</a>. Profile owner
      * access is deprecated and will be removed in a future release.
      *
@@ -1630,8 +1630,8 @@ public class TelephonyManager {
      *
      * <p>Requires Permission: READ_PRIVILEGED_PHONE_STATE, for the calling app to be the device or
      * profile owner and have the READ_PHONE_STATE permission, or that the calling app has carrier
-     * privileges (see {@link #hasCarrierPrivileges}). The profile owner is an app that owns a
-     * managed profile on the device; for more details see <a
+     * privileges (see {@link #hasCarrierPrivileges}) on any active subscription. The profile owner
+     * is an app that owns a managed profile on the device; for more details see <a
      * href="https://developer.android.com/work/managed-profiles">Work profiles</a>. Profile owner
      * access is deprecated and will be removed in a future release.
      *
@@ -1690,7 +1690,8 @@ public class TelephonyManager {
      *     <li>The caller holds the READ_PRIVILEGED_PHONE_STATE permission.</li>
      *     <li>If the caller is the device or profile owner, the caller holds the
      *     {@link Manifest.permission#READ_PHONE_STATE} permission.</li>
-     *     <li>The caller has carrier privileges (see {@link #hasCarrierPrivileges()}.</li>
+     *     <li>The caller has carrier privileges (see {@link #hasCarrierPrivileges()} on any
+     *     active subscription.</li>
      *     <li>The caller is the default SMS app for the device.</li>
      * </ul>
      * <p>The profile owner is an app that owns a managed profile on the device; for more details
@@ -1759,8 +1760,8 @@ public class TelephonyManager {
      *
      * <p>Requires Permission: READ_PRIVILEGED_PHONE_STATE, for the calling app to be the device or
      * profile owner and have the READ_PHONE_STATE permission, or that the calling app has carrier
-     * privileges (see {@link #hasCarrierPrivileges}). The profile owner is an app that owns a
-     * managed profile on the device; for more details see <a
+     * privileges (see {@link #hasCarrierPrivileges}) on any active subscription. The profile owner
+     * is an app that owns a managed profile on the device; for more details see <a
      * href="https://developer.android.com/work/managed-profiles">Work profiles</a>. Profile owner
      * access is deprecated and will be removed in a future release.
      *
@@ -1786,8 +1787,8 @@ public class TelephonyManager {
      *
      * <p>Requires Permission: READ_PRIVILEGED_PHONE_STATE, for the calling app to be the device or
      * profile owner and have the READ_PHONE_STATE permission, or that the calling app has carrier
-     * privileges (see {@link #hasCarrierPrivileges}). The profile owner is an app that owns a
-     * managed profile on the device; for more details see <a
+     * privileges (see {@link #hasCarrierPrivileges}) on any active subscription. The profile owner
+     * is an app that owns a managed profile on the device; for more details see <a
      * href="https://developer.android.com/work/managed-profiles">Work profiles</a>. Profile owner
      * access is deprecated and will be removed in a future release.
      *
diff --git a/telephony/java/com/android/internal/telephony/TelephonyPermissions.java b/telephony/java/com/android/internal/telephony/TelephonyPermissions.java
index 51c5d12dfe157..f6cf8c51a189a 100644
--- a/telephony/java/com/android/internal/telephony/TelephonyPermissions.java
+++ b/telephony/java/com/android/internal/telephony/TelephonyPermissions.java
@@ -237,9 +237,10 @@ public final class TelephonyPermissions {
      * <ul>
      *   <li>return true: if the caller has the READ_PRIVILEGED_PHONE_STATE permission, the calling
      *       package passes a DevicePolicyManager Device Owner / Profile Owner device identifier
-     *       access check, or the calling package has carrier privileges.
-     *   <li>throw SecurityException: if the caller does not meet any of the requirements and is
-     *       targeting Q or is targeting pre-Q and does not have the READ_PHONE_STATE permission.
+     *       access check, or the calling package has carrier privileges on any active subscription.
+    *   <li>throw SecurityException: if the caller does not meet any of the requirements and is
+     *       targeting Q or is targeting pre-Q and does not have the READ_PHONE_STATE permission
+     *       or carrier privileges of any active subscription.
      *   <li>return false: if the caller is targeting pre-Q and does have the READ_PHONE_STATE
      *       permission. In this case the caller would expect to have access to the device
      *       identifiers so false is returned instead of throwing a SecurityException to indicate
@@ -259,10 +260,10 @@ public final class TelephonyPermissions {
      * <ul>
      *   <li>return true: if the caller has the READ_PRIVILEGED_PHONE_STATE permission, the calling
      *       package passes a DevicePolicyManager Device Owner / Profile Owner device identifier
-     *       access check, or the calling package has carrier privileges.
+     *       access check, or the calling package has carrier privileges on any active subscription.
      *   <li>throw SecurityException: if the caller does not meet any of the requirements and is
      *       targeting Q or is targeting pre-Q and does not have the READ_PHONE_STATE permission
-     *       or carrier privileges.
+     *       or carrier privileges of any active subscription.
      *   <li>return false: if the caller is targeting pre-Q and does have the READ_PHONE_STATE
      *       permission or carrier privileges. In this case the caller would expect to have access
      *       to the device identifiers so false is returned instead of throwing a SecurityException
@@ -271,8 +272,8 @@ public final class TelephonyPermissions {
      */
     public static boolean checkCallingOrSelfReadDeviceIdentifiers(Context context, int subId,
             String callingPackage, String message) {
-        return checkReadDeviceIdentifiers(context, TELEPHONY_SUPPLIER, subId,
-                Binder.getCallingPid(), Binder.getCallingUid(), callingPackage, message);
+        return checkPrivilegedReadPermissionOrCarrierPrivilegePermission(
+                context, subId, callingPackage, message, true);
     }
 
     /**
@@ -282,7 +283,7 @@ public final class TelephonyPermissions {
      * <ul>
      *   <li>return true: if the caller has the READ_PRIVILEGED_PHONE_STATE permission, the calling
      *       package passes a DevicePolicyManager Device Owner / Profile Owner device identifier
-     *       access check, or the calling package has carrier privileges.
+     *       access check, or the calling package has carrier privileges on specified subscription.
      *   <li>throw SecurityException: if the caller does not meet any of the requirements and is
      *       targeting Q or is targeting pre-Q and does not have the READ_PHONE_STATE permission.
      *   <li>return false: if the caller is targeting pre-Q and does have the READ_PHONE_STATE
@@ -293,21 +294,33 @@ public final class TelephonyPermissions {
      */
     public static boolean checkCallingOrSelfReadSubscriberIdentifiers(Context context, int subId,
             String callingPackage, String message) {
-        return checkReadDeviceIdentifiers(context, TELEPHONY_SUPPLIER, subId,
-                Binder.getCallingPid(), Binder.getCallingUid(), callingPackage, message);
+        return checkPrivilegedReadPermissionOrCarrierPrivilegePermission(
+                context, subId, callingPackage, message, false);
     }
 
     /**
      * Checks whether the app with the given pid/uid can read device identifiers.
      *
-     * @returns true if the caller has the READ_PRIVILEGED_PHONE_STATE permission or the calling
-     * package passes a DevicePolicyManager Device Owner / Profile Owner device identifier access
-     * check.
+     * <p>This method behaves in one of the following ways:
+     * <ul>
+     *   <li>return true: if the caller has the READ_PRIVILEGED_PHONE_STATE permission, the calling
+     *       package passes a DevicePolicyManager Device Owner / Profile Owner device identifier
+     *       access check; or the calling package has carrier privileges on the specified
+     *       subscription; or allowCarrierPrivilegeOnAnySub is true and has carrier privilege on
+     *       any active subscription.
+     *   <li>throw SecurityException: if the caller does not meet any of the requirements and is
+     *       targeting Q or is targeting pre-Q and does not have the READ_PHONE_STATE permission.
+     *   <li>return false: if the caller is targeting pre-Q and does have the READ_PHONE_STATE
+     *       permission. In this case the caller would expect to have access to the device
+     *       identifiers so false is returned instead of throwing a SecurityException to indicate
+     *       the calling function should return dummy data.
+     * </ul>
      */
-    @VisibleForTesting
-    public static boolean checkReadDeviceIdentifiers(Context context,
-            Supplier<ITelephony> telephonySupplier, int subId, int pid, int uid,
-            String callingPackage, String message) {
+    private static boolean checkPrivilegedReadPermissionOrCarrierPrivilegePermission(
+            Context context, int subId, String callingPackage, String message,
+            boolean allowCarrierPrivilegeOnAnySub) {
+        int uid = Binder.getCallingUid();
+        int pid = Binder.getCallingPid();
         // Allow system and root access to the device identifiers.
         final int appId = UserHandle.getAppId(uid);
         if (appId == Process.SYSTEM_UID || appId == Process.ROOT_UID) {
@@ -318,10 +331,17 @@ public final class TelephonyPermissions {
                 uid) == PackageManager.PERMISSION_GRANTED) {
             return true;
         }
-        // If the calling package has carrier privileges for any subscription then allow access.
-        if (checkCarrierPrivilegeForAnySubId(context, telephonySupplier, uid)) {
+
+        // If the calling package has carrier privileges for specified sub, then allow access.
+        if (checkCarrierPrivilegeForSubId(subId)) return true;
+
+        // If the calling package has carrier privileges for any subscription
+        // and allowCarrierPrivilegeOnAnySub is set true, then allow access.
+        if (allowCarrierPrivilegeOnAnySub && checkCarrierPrivilegeForAnySubId(
+                context, TELEPHONY_SUPPLIER, uid)) {
             return true;
         }
+
         // if the calling package is not null then perform the DevicePolicyManager device /
         // profile owner and Appop checks.
         if (callingPackage != null) {
@@ -347,7 +367,7 @@ public final class TelephonyPermissions {
             }
         }
         return reportAccessDeniedToReadIdentifiers(context, subId, pid, uid, callingPackage,
-            message);
+                message);
     }
 
     /**