Merge "Check permissions and carrier privilege in notifyActiveDataSubIdChanged"

3 files changed, 83 insertions, 9 deletions

diff --git a/services/core/java/com/android/server/TelephonyRegistry.java b/services/core/java/com/android/server/TelephonyRegistry.java
index 9c5c152726dd..7a8d23a2295c 100644
--- a/services/core/java/com/android/server/TelephonyRegistry.java
+++ b/services/core/java/com/android/server/TelephonyRegistry.java
@@ -77,6 +77,7 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.NoSuchElementException;
+import java.util.stream.Collectors;
 
 /**
  * Since phone process can be restarted, this class provides a centralized place
@@ -260,8 +261,7 @@ public class TelephonyRegistry extends ITelephonyRegistry.Stub {
     static final int ENFORCE_PHONE_STATE_PERMISSION_MASK =
                 PhoneStateListener.LISTEN_CALL_FORWARDING_INDICATOR
                         | PhoneStateListener.LISTEN_MESSAGE_WAITING_INDICATOR
-                        | PhoneStateListener.LISTEN_EMERGENCY_NUMBER_LIST
-                        | PhoneStateListener.LISTEN_ACTIVE_DATA_SUBSCRIPTION_ID_CHANGE;
+                        | PhoneStateListener.LISTEN_EMERGENCY_NUMBER_LIST;
 
     static final int PRECISE_PHONE_STATE_PERMISSION_MASK =
                 PhoneStateListener.LISTEN_PRECISE_CALL_STATE |
@@ -822,7 +822,10 @@ public class TelephonyRegistry extends ITelephonyRegistry.Stub {
                         }
                     }
                     if ((events & PhoneStateListener
-                            .LISTEN_ACTIVE_DATA_SUBSCRIPTION_ID_CHANGE) != 0) {
+                            .LISTEN_ACTIVE_DATA_SUBSCRIPTION_ID_CHANGE) != 0
+                            && TelephonyPermissions.checkReadPhoneStateOnAnyActiveSub(
+                                    r.context, r.callerPid, r.callerUid, r.callingPackage,
+                            "listen_active_data_subid_change")) {
                         try {
                             r.callback.onActiveDataSubIdChanged(mActiveDataSubId);
                         } catch (RemoteException ex) {
@@ -1753,12 +1756,23 @@ public class TelephonyRegistry extends ITelephonyRegistry.Stub {
             log("notifyActiveDataSubIdChanged: activeDataSubId=" + activeDataSubId);
         }
 
+        // Create a copy to prevent the IPC call while checking carrier privilege under the lock.
+        List<Record> copiedRecords;
         synchronized (mRecords) {
-            mActiveDataSubId = activeDataSubId;
+            copiedRecords = new ArrayList<>(mRecords);
+        }
+        mActiveDataSubId = activeDataSubId;
 
-            for (Record r : mRecords) {
-                if (r.matchPhoneStateListenerEvent(
-                        PhoneStateListener.LISTEN_ACTIVE_DATA_SUBSCRIPTION_ID_CHANGE)) {
+        // Filter the record that does not listen to this change or does not have the permission.
+        copiedRecords = copiedRecords.stream().filter(r -> r.matchPhoneStateListenerEvent(
+                PhoneStateListener.LISTEN_ACTIVE_DATA_SUBSCRIPTION_ID_CHANGE)
+                && TelephonyPermissions.checkReadPhoneStateOnAnyActiveSub(
+                        mContext, r.callerPid, r.callerUid, r.callingPackage,
+                "notifyActiveDataSubIdChanged")).collect(Collectors.toCollection(ArrayList::new));
+
+        synchronized (mRecords) {
+            for (Record r : copiedRecords) {
+                if (mRecords.contains(r)) {
                     try {
                         r.callback.onActiveDataSubIdChanged(activeDataSubId);
                     } catch (RemoteException ex) {
diff --git a/telephony/java/android/telephony/PhoneStateListener.java b/telephony/java/android/telephony/PhoneStateListener.java
index 918bf60c9fa7..373c5d27eec8 100644
--- a/telephony/java/android/telephony/PhoneStateListener.java
+++ b/telephony/java/android/telephony/PhoneStateListener.java
@@ -297,8 +297,11 @@ public class PhoneStateListener {
      *  it could be the current active opportunistic subscription in use, or the
      *  subscription user selected as default data subscription in DSDS mode.
      *
-     *  Requires Permission: {@link android.Manifest.permission#READ_PHONE_STATE
-     *  READ_PHONE_STATE}
+     *  Requires Permission: No permission is required to listen, but notification requires
+     *  {@link android.Manifest.permission#READ_PHONE_STATE READ_PHONE_STATE} or the calling
+     *  app has carrier privileges (see {@link TelephonyManager#hasCarrierPrivileges})
+     *  on any active subscription.
+     *
      *  @see #onActiveDataSubscriptionIdChanged
      */
     public static final int LISTEN_ACTIVE_DATA_SUBSCRIPTION_ID_CHANGE = 0x00400000;
diff --git a/telephony/java/com/android/internal/telephony/TelephonyPermissions.java b/telephony/java/com/android/internal/telephony/TelephonyPermissions.java
index 2c8b908abbec..7574a6e028f4 100644
--- a/telephony/java/com/android/internal/telephony/TelephonyPermissions.java
+++ b/telephony/java/com/android/internal/telephony/TelephonyPermissions.java
@@ -130,6 +130,63 @@ public final class TelephonyPermissions {
         // We have READ_PHONE_STATE permission, so return true as long as the AppOps bit hasn't been
         // revoked.
         AppOpsManager appOps = (AppOpsManager) context.getSystemService(Context.APP_OPS_SERVICE);
+        return appOps.noteOp(AppOpsManager.OP_READ_PHONE_STATE, uid, callingPackage)
+                == AppOpsManager.MODE_ALLOWED;
+    }
+
+    /**
+     * Check whether the app with the given pid/uid can read phone state, or has carrier
+     * privileges on any active subscription.
+     *
+     * <p>If the app does not have carrier privilege, this method will return {@code false} instead
+     * of throwing a SecurityException. Therefore, the callers cannot tell the difference
+     * between M+ apps which declare the runtime permission but do not have it, and pre-M apps
+     * which declare the static permission but had access revoked via AppOps. Apps in the former
+     * category expect SecurityExceptions; apps in the latter don't. So this method is suitable for
+     * use only if the behavior in both scenarios is meant to be identical.
+     *
+     * @return {@code true} if the app can read phone state or has carrier privilege;
+     *         {@code false} otherwise.
+     */
+    public static boolean checkReadPhoneStateOnAnyActiveSub(
+            Context context, int pid, int uid, String callingPackage, String message) {
+        return checkReadPhoneStateOnAnyActiveSub(context, TELEPHONY_SUPPLIER, pid, uid,
+                    callingPackage, message);
+    }
+
+    @VisibleForTesting
+    public static boolean checkReadPhoneStateOnAnyActiveSub(
+            Context context, Supplier<ITelephony> telephonySupplier, int pid, int uid,
+            String callingPackage, String message) {
+        try {
+            context.enforcePermission(
+                    android.Manifest.permission.READ_PRIVILEGED_PHONE_STATE, pid, uid, message);
+
+            // SKIP checking for run-time permission since caller has PRIVILEGED permission
+            return true;
+        } catch (SecurityException privilegedPhoneStateException) {
+            try {
+                context.enforcePermission(
+                        android.Manifest.permission.READ_PHONE_STATE, pid, uid, message);
+            } catch (SecurityException phoneStateException) {
+                SubscriptionManager sm = (SubscriptionManager) context.getSystemService(
+                        Context.TELEPHONY_SUBSCRIPTION_SERVICE);
+                int[] activeSubIds = sm.getActiveSubscriptionIdList();
+                for (int activeSubId : activeSubIds) {
+                    // If we don't have the runtime permission, but do have carrier privileges, that
+                    // suffices for reading phone state.
+                    if (getCarrierPrivilegeStatus(telephonySupplier, activeSubId, uid)
+                            == TelephonyManager.CARRIER_PRIVILEGE_STATUS_HAS_ACCESS) {
+                        return true;
+                    }
+                }
+                return false;
+            }
+        }
+
+        // We have READ_PHONE_STATE permission, so return true as long as the AppOps bit hasn't been
+        // revoked.
+        AppOpsManager appOps = (AppOpsManager) context.getSystemService(Context.APP_OPS_SERVICE);
         return appOps.noteOp(AppOpsManager.OP_READ_PHONE_STATE, uid, callingPackage) ==
                 AppOpsManager.MODE_ALLOWED;
     }