OSDN Git Service

Allow carrier-privileged apps to register telephony listeners.
authorJeff Davidson <jpd@google.com>
Thu, 1 Mar 2018 01:50:16 +0000 (17:50 -0800)
committerJeff Davidson <jpd@google.com>
Thu, 15 Mar 2018 20:47:34 +0000 (13:47 -0700)
For TelephonyManager#listen, we check carrier privileges if
READ_PHONE_STATE is missing for any calls which enforce the
permission. For calls which check it and behave differently (by
redacting sensitive info), we defer the permission check until the
actual event occurs, at which point it is checked based on the current
state of the device.

For SubscriptionManager#addOnSubscriptionsChangedListener, we remove
the existing permission check for READ_PHONE_STATE altogether. The
event itself contains no information, and reading subscriptions still
requires either READ_PHONE_STATE or carrier privileges on the
subscription in question.

Also updates incorrect Javadoc on
PhoneStateListener#LISTEN_SIGNAL_STRENGTH, which does not actually
check any permissions by design.

Bug: 70041899
Fixes: 74034127
Test: TreeHugger + E2E test w/ a carrier-privileged app
Change-Id: I84a56ad3972b9edcfdefcbb43ef174c54cdcac00
Merged-In: I84a56ad3972b9edcfdefcbb43ef174c54cdcac00
(cherry pick from commit 62b994b3cf0f05801bd8b58a5874118c404d656b)

services/core/java/com/android/server/TelephonyRegistry.java
telephony/java/android/telephony/PhoneStateListener.java
telephony/java/android/telephony/SubscriptionManager.java

index 3896871..539c001 100644 (file)
@@ -89,6 +89,8 @@ class TelephonyRegistry extends ITelephonyRegistry.Stub {
     private static final boolean VDBG = false; // STOPSHIP if true
 
     private static class Record {
+        Context context;
+
         String callingPackage;
 
         IBinder binder;
@@ -107,8 +109,6 @@ class TelephonyRegistry extends ITelephonyRegistry.Stub {
 
         int phoneId = SubscriptionManager.INVALID_PHONE_INDEX;
 
-        boolean canReadPhoneState;
-
         boolean matchPhoneStateListenerEvent(int events) {
             return (callback != null) && ((events & this.events) != 0);
         }
@@ -117,6 +117,15 @@ class TelephonyRegistry extends ITelephonyRegistry.Stub {
             return (onSubscriptionsChangedListenerCallback != null);
         }
 
+        boolean canReadPhoneState() {
+            try {
+                return TelephonyPermissions.checkReadPhoneState(
+                        context, subId, callerPid, callerUid, callingPackage, "listen");
+            } catch (SecurityException e) {
+                return false;
+            }
+        }
+
         @Override
         public String toString() {
             return "{callingPackage=" + callingPackage + " binder=" + binder
@@ -124,8 +133,7 @@ class TelephonyRegistry extends ITelephonyRegistry.Stub {
                     + " onSubscriptionsChangedListenererCallback="
                                             + onSubscriptionsChangedListenerCallback
                     + " callerUid=" + callerUid + " subId=" + subId + " phoneId=" + phoneId
-                    + " events=" + Integer.toHexString(events)
-                    + " canReadPhoneState=" + canReadPhoneState + "}";
+                    + " events=" + Integer.toHexString(events) + "}";
         }
     }
 
@@ -206,11 +214,6 @@ class TelephonyRegistry extends ITelephonyRegistry.Stub {
                 PhoneStateListener.LISTEN_MESSAGE_WAITING_INDICATOR |
                 PhoneStateListener.LISTEN_VOLTE_STATE;
 
-    static final int CHECK_PHONE_STATE_PERMISSION_MASK =
-                PhoneStateListener.LISTEN_CALL_STATE |
-                PhoneStateListener.LISTEN_DATA_ACTIVITY |
-                PhoneStateListener.LISTEN_DATA_CONNECTION_STATE;
-
     static final int PRECISE_PHONE_STATE_PERMISSION_MASK =
                 PhoneStateListener.LISTEN_PRECISE_CALL_STATE |
                 PhoneStateListener.LISTEN_PRECISE_DATA_CONNECTION_STATE;
@@ -376,22 +379,13 @@ class TelephonyRegistry extends ITelephonyRegistry.Stub {
     public void addOnSubscriptionsChangedListener(String callingPackage,
             IOnSubscriptionsChangedListener callback) {
         int callerUserId = UserHandle.getCallingUserId();
-        mContext.getSystemService(AppOpsManager.class)
-                .checkPackage(Binder.getCallingUid(), callingPackage);
+        mAppOps.checkPackage(Binder.getCallingUid(), callingPackage);
         if (VDBG) {
             log("listen oscl: E pkg=" + callingPackage + " myUserId=" + UserHandle.myUserId()
                 + " callerUserId="  + callerUserId + " callback=" + callback
                 + " callback.asBinder=" + callback.asBinder());
         }
 
-        // TODO(b/70041899): Find a way to make this work for carrier-privileged callers.
-        if (!TelephonyPermissions.checkCallingOrSelfReadPhoneState(
-                mContext, SubscriptionManager.INVALID_SUBSCRIPTION_ID, callingPackage,
-                "addOnSubscriptionsChangedListener")) {
-            return;
-        }
-
-
         synchronized (mRecords) {
             // register
             IBinder b = callback.asBinder();
@@ -401,12 +395,12 @@ class TelephonyRegistry extends ITelephonyRegistry.Stub {
                 return;
             }
 
+            r.context = mContext;
             r.onSubscriptionsChangedListenerCallback = callback;
             r.callingPackage = callingPackage;
             r.callerUid = Binder.getCallingUid();
             r.callerPid = Binder.getCallingPid();
             r.events = 0;
-            r.canReadPhoneState = true; // permission has been enforced above
             if (DBG) {
                 log("listen oscl:  Register r=" + r);
             }
@@ -475,8 +469,7 @@ class TelephonyRegistry extends ITelephonyRegistry.Stub {
     private void listen(String callingPackage, IPhoneStateListener callback, int events,
             boolean notifyNow, int subId) {
         int callerUserId = UserHandle.getCallingUserId();
-        mContext.getSystemService(AppOpsManager.class)
-                .checkPackage(Binder.getCallingUid(), callingPackage);
+        mAppOps.checkPackage(Binder.getCallingUid(), callingPackage);
         if (VDBG) {
             log("listen: E pkg=" + callingPackage + " events=0x" + Integer.toHexString(events)
                 + " notifyNow=" + notifyNow + " subId=" + subId + " myUserId="
@@ -487,7 +480,7 @@ class TelephonyRegistry extends ITelephonyRegistry.Stub {
             // Checks permission and throws SecurityException for disallowed operations. For pre-M
             // apps whose runtime permission has been revoked, we return immediately to skip sending
             // events to the app without crashing it.
-            if (!checkListenerPermission(events, callingPackage, "listen")) {
+            if (!checkListenerPermission(events, subId, callingPackage, "listen")) {
                 return;
             }
 
@@ -501,14 +494,11 @@ class TelephonyRegistry extends ITelephonyRegistry.Stub {
                     return;
                 }
 
+                r.context = mContext;
                 r.callback = callback;
                 r.callingPackage = callingPackage;
                 r.callerUid = Binder.getCallingUid();
                 r.callerPid = Binder.getCallingPid();
-                boolean isPhoneStateEvent = (events & (CHECK_PHONE_STATE_PERMISSION_MASK
-                        | ENFORCE_PHONE_STATE_PERMISSION_MASK)) != 0;
-                r.canReadPhoneState =
-                        isPhoneStateEvent && canReadPhoneState(callingPackage, "listen");
                 // Legacy applications pass SubscriptionManager.DEFAULT_SUB_ID,
                 // force all illegal subId to SubscriptionManager.DEFAULT_SUB_ID
                 if (!SubscriptionManager.isValidSubscriptionId(subId)) {
@@ -676,19 +666,9 @@ class TelephonyRegistry extends ITelephonyRegistry.Stub {
         }
     }
 
-    private boolean canReadPhoneState(String callingPackage, String message) {
-        try {
-            // TODO(b/70041899): Find a way to make this work for carrier-privileged callers.
-            return TelephonyPermissions.checkCallingOrSelfReadPhoneState(
-                    mContext, SubscriptionManager.INVALID_SUBSCRIPTION_ID, callingPackage, message);
-        } catch (SecurityException e) {
-            return false;
-        }
-    }
-
     private String getCallIncomingNumber(Record record, int phoneId) {
-        // Hide the number if record's process has no READ_PHONE_STATE permission
-        return record.canReadPhoneState ? mCallIncomingNumber[phoneId] : "";
+        // Hide the number if record's process can't currently read phone state.
+        return record.canReadPhoneState() ? mCallIncomingNumber[phoneId] : "";
     }
 
     private Record add(IBinder binder) {
@@ -763,7 +743,7 @@ class TelephonyRegistry extends ITelephonyRegistry.Stub {
                 if (r.matchPhoneStateListenerEvent(PhoneStateListener.LISTEN_CALL_STATE) &&
                         (r.subId == SubscriptionManager.DEFAULT_SUBSCRIPTION_ID)) {
                     try {
-                        String incomingNumberOrEmpty = r.canReadPhoneState ? incomingNumber : "";
+                        String incomingNumberOrEmpty = r.canReadPhoneState() ? incomingNumber : "";
                         r.callback.onCallStateChanged(state, incomingNumberOrEmpty);
                     } catch (RemoteException ex) {
                         mRemoveList.add(r.binder);
@@ -1690,7 +1670,8 @@ class TelephonyRegistry extends ITelephonyRegistry.Stub {
                 == PackageManager.PERMISSION_GRANTED;
     }
 
-    private boolean checkListenerPermission(int events, String callingPackage, String message) {
+    private boolean checkListenerPermission(
+            int events, int subId, String callingPackage, String message) {
         if ((events & ENFORCE_COARSE_LOCATION_PERMISSION_MASK) != 0) {
             mContext.enforceCallingOrSelfPermission(
                     android.Manifest.permission.ACCESS_COARSE_LOCATION, null);
@@ -1701,9 +1682,8 @@ class TelephonyRegistry extends ITelephonyRegistry.Stub {
         }
 
         if ((events & ENFORCE_PHONE_STATE_PERMISSION_MASK) != 0) {
-            // TODO(b/70041899): Find a way to make this work for carrier-privileged callers.
-            if (!TelephonyPermissions.checkCallingOrSelfReadPhoneState(mContext,
-                    SubscriptionManager.INVALID_SUBSCRIPTION_ID, callingPackage, message)) {
+            if (!TelephonyPermissions.checkCallingOrSelfReadPhoneState(
+                    mContext, subId, callingPackage, message)) {
                 return false;
             }
         }
index 4ed6883..7db83f6 100644 (file)
@@ -62,9 +62,6 @@ public class PhoneStateListener {
     /**
      * Listen for changes to the network signal strength (cellular).
      * {@more}
-     * Requires Permission: {@link android.Manifest.permission#READ_PHONE_STATE
-     * READ_PHONE_STATE}
-     * <p>
      *
      * @see #onSignalStrengthChanged
      *
@@ -77,7 +74,8 @@ public class PhoneStateListener {
      * Listen for changes to the message-waiting indicator.
      * {@more}
      * Requires Permission: {@link android.Manifest.permission#READ_PHONE_STATE
-     * READ_PHONE_STATE}
+     * READ_PHONE_STATE} or that the calling app has carrier privileges (see
+     * {@link TelephonyManager#hasCarrierPrivileges}).
      * <p>
      * Example: The status bar uses this to determine when to display the
      * voicemail icon.
@@ -90,7 +88,9 @@ public class PhoneStateListener {
      * Listen for changes to the call-forwarding indicator.
      * {@more}
      * Requires Permission: {@link android.Manifest.permission#READ_PHONE_STATE
-     * READ_PHONE_STATE}
+     * READ_PHONE_STATE} or that the calling app has carrier privileges (see
+     * {@link TelephonyManager#hasCarrierPrivileges}).
+     *
      * @see #onCallForwardingIndicatorChanged
      */
     public static final int LISTEN_CALL_FORWARDING_INDICATOR                = 0x00000008;
@@ -439,8 +439,9 @@ public class PhoneStateListener {
      *
      * @param state call state
      * @param phoneNumber call phone number. If application does not have
-     * {@link android.Manifest.permission#READ_PHONE_STATE READ_PHONE_STATE} permission, an empty
-     * string will be passed as an argument.
+     * {@link android.Manifest.permission#READ_PHONE_STATE READ_PHONE_STATE} permission or carrier
+     * privileges (see {@link TelephonyManager#hasCarrierPrivileges}), an empty string will be
+     * passed as an argument.
      */
     public void onCallStateChanged(@TelephonyManager.CallState int state, String phoneNumber) {
         // default implementation empty
index ef66ed7..472a6fb 100644 (file)
@@ -609,8 +609,6 @@ public class SubscriptionManager {
      * @param listener an instance of {@link OnSubscriptionsChangedListener} with
      *                 onSubscriptionsChanged overridden.
      */
-    // TODO(b/70041899): Find a way to extend this to carrier-privileged apps.
-    @RequiresPermission(android.Manifest.permission.READ_PHONE_STATE)
     public void addOnSubscriptionsChangedListener(OnSubscriptionsChangedListener listener) {
         String pkgName = mContext != null ? mContext.getOpPackageName() : "<unknown>";
         if (DBG) {