From d2e0dfcfbc5bed57ec832aebfbfa271966655b69 Mon Sep 17 00:00:00 2001 From: Jack Yu Date: Wed, 5 Feb 2020 17:32:02 -0800 Subject: [PATCH 01/11] Fixed deadlock in IccSmsInterfaceManager The sync call in to RIL should run on a different thread than IccSmsInterfaceManager's handler thread, otherwise it will cause deadlock because the RIL response to unlock is also running on the same thread. Test: Manual boot up to make sure no ANR occurs. Bug: 148923425 Change-Id: I1f59f8af2134fb5e8b205dec0f728b29e24cbe5f (cherry picked from commit d34750b6dcf8a7733f0ba9dac7658be8cfa96330) --- .../telephony/IccSmsInterfaceManager.java | 34 +++++++++++++++---- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/src/java/com/android/internal/telephony/IccSmsInterfaceManager.java b/src/java/com/android/internal/telephony/IccSmsInterfaceManager.java index 3b1be0c986..0c65cf3aea 100644 --- a/src/java/com/android/internal/telephony/IccSmsInterfaceManager.java +++ b/src/java/com/android/internal/telephony/IccSmsInterfaceManager.java @@ -36,6 +36,7 @@ import android.os.AsyncResult; import android.os.Binder; import android.os.Handler; +import android.os.Looper; import android.os.Message; import android.os.UserManager; import android.provider.Telephony; @@ -199,13 +200,23 @@ public void onReceive(Context context, Intent intent) { if (mPhone.getPhoneId() == intent.getIntExtra( CarrierConfigManager.EXTRA_SLOT_INDEX, SubscriptionManager.INVALID_SIM_SLOT_INDEX)) { - mCellBroadcastRangeManager.updateRanges(); + new Thread(() -> { + log("Carrier config changed. Update ranges."); + mCellBroadcastRangeManager.updateRanges(); + }).start(); } } } }, new IntentFilter(CarrierConfigManager.ACTION_CARRIER_CONFIG_CHANGED)); } + private void enforceNotOnHandlerThread(String methodName) { + if (Looper.myLooper() == mHandler.getLooper()) { + throw new RuntimeException("This method " + methodName + " will deadlock if called from" + + " the handler's thread."); + } + } + protected void markMessagesAsRead(ArrayList messages) { if (messages == null) { return; @@ -275,6 +286,7 @@ private void enforceAccessMessageOnICC(String message) { "("+ Arrays.toString(pdu) + ")"); enforceReceiveAndSend("Updating message on Icc"); enforceAccessMessageOnICC("Updating message on Icc"); + enforceNotOnHandlerThread("updateMessageOnIccEf"); if (mAppOps.noteOp(AppOpsManager.OPSTR_WRITE_ICC_SMS, Binder.getCallingUid(), callingPackage) != AppOpsManager.MODE_ALLOWED) { return false; @@ -334,6 +346,7 @@ public boolean copyMessageToIccEf(String callingPackage, int status, byte[] pdu, "pdu=("+ Arrays.toString(pdu) + "), smsc=(" + Arrays.toString(smsc) +")"); enforceReceiveAndSend("Copying message to Icc"); + enforceNotOnHandlerThread("copyMessageToIccEf"); if (mAppOps.noteOp(AppOpsManager.OPSTR_WRITE_ICC_SMS, Binder.getCallingUid(), callingPackage) != AppOpsManager.MODE_ALLOWED) { return false; @@ -373,6 +386,7 @@ public List getAllMessagesFromIccEf(String callingPackage) { Manifest.permission.RECEIVE_SMS, "Reading messages from Icc"); enforceAccessMessageOnICC("Reading messages from Icc"); + enforceNotOnHandlerThread("getAllMessagesFromIccEf"); if (mAppOps.noteOp(AppOpsManager.OPSTR_READ_ICC_SMS, Binder.getCallingUid(), callingPackage) != AppOpsManager.MODE_ALLOWED) { return new ArrayList(); @@ -880,6 +894,7 @@ public String getSmscAddressFromIccEf(String callingPackage) { callingPackage, "getSmscAddressFromIccEf")) { return null; } + enforceNotOnHandlerThread("getSmscAddressFromIccEf"); synchronized (mLock) { mSmsc = null; Message response = mHandler.obtainMessage(EVENT_GET_SMSC_DONE); @@ -1155,9 +1170,10 @@ protected boolean finishUpdate() { @UnsupportedAppUsage private boolean setCellBroadcastConfig(SmsBroadcastConfigInfo[] configs) { - if (DBG) + if (DBG) { log("Calling setGsmBroadcastConfig with " + configs.length + " configurations"); - + } + enforceNotOnHandlerThread("setCellBroadcastConfig"); synchronized (mLock) { Message response = mHandler.obtainMessage(EVENT_SET_BROADCAST_CONFIG_DONE); @@ -1175,9 +1191,11 @@ private boolean setCellBroadcastConfig(SmsBroadcastConfigInfo[] configs) { } private boolean setCellBroadcastActivation(boolean activate) { - if (DBG) + if (DBG) { log("Calling setCellBroadcastActivation(" + activate + ')'); + } + enforceNotOnHandlerThread("setCellBroadcastConfig"); synchronized (mLock) { Message response = mHandler.obtainMessage(EVENT_SET_BROADCAST_ACTIVATION_DONE); @@ -1196,9 +1214,11 @@ private boolean setCellBroadcastActivation(boolean activate) { @UnsupportedAppUsage private boolean setCdmaBroadcastConfig(CdmaSmsBroadcastConfigInfo[] configs) { - if (DBG) + if (DBG) { log("Calling setCdmaBroadcastConfig with " + configs.length + " configurations"); + } + enforceNotOnHandlerThread("setCdmaBroadcastConfig"); synchronized (mLock) { Message response = mHandler.obtainMessage(EVENT_SET_BROADCAST_CONFIG_DONE); @@ -1216,9 +1236,11 @@ private boolean setCdmaBroadcastConfig(CdmaSmsBroadcastConfigInfo[] configs) { } private boolean setCdmaBroadcastActivation(boolean activate) { - if (DBG) + if (DBG) { log("Calling setCdmaBroadcastActivation(" + activate + ")"); + } + enforceNotOnHandlerThread("setCdmaBroadcastActivation"); synchronized (mLock) { Message response = mHandler.obtainMessage(EVENT_SET_BROADCAST_ACTIVATION_DONE); From c7f8cad9ea582ec507532465fedb0de6dcb16dfa Mon Sep 17 00:00:00 2001 From: Tyler Gunn Date: Thu, 6 Feb 2020 11:09:10 -0800 Subject: [PATCH 02/11] Fix potential call crashes in Telephony. There is a timing issue in the ImsPhoneCallTracker#dial method. Basically we assign the new outgoing call to mPendingMO, but it is possible for the dial to go out and the modem to signal onCallProgressing for that call. onCallProgressing will set mPendingMO to null, which means by the time we end the dial method, the return value we send back is null. This in turn causes TelephonyConnectionService to think that the dialed number was an MMI code and to bring up the MMI code dialog which itself crashes because _surprise_ the call wasn't actually an MMI code. Changing the dial method to cache the pending MO call as a local variable so we can still pass it back and allow the call to set up as expected. Test: Run telephony unit tests. Test: Manual phone call regression testing for normal numbers. Test: Manual phone call regression testing for test emergency numbers. Bug: 149005037 Change-Id: I959196ffbaf3843ba15369029fc640ee47df9d3a (cherry picked from commit c808925d6f16180da40b16bb4a46e13a26b7ef40) --- .../imsphone/ImsPhoneCallTracker.java | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/java/com/android/internal/telephony/imsphone/ImsPhoneCallTracker.java b/src/java/com/android/internal/telephony/imsphone/ImsPhoneCallTracker.java index db8defbec4..ac91475513 100644 --- a/src/java/com/android/internal/telephony/imsphone/ImsPhoneCallTracker.java +++ b/src/java/com/android/internal/telephony/imsphone/ImsPhoneCallTracker.java @@ -871,28 +871,33 @@ public Connection startConference(String[] participantsToDial, ImsPhone.ImsDialA boolean holdBeforeDial = prepareForDialing(dialArgs); mClirMode = clirMode; - + ImsPhoneConnection pendingConnection; synchronized (mSyncHold) { mLastDialArgs = dialArgs; - mPendingMO = new ImsPhoneConnection(mPhone, + pendingConnection = new ImsPhoneConnection(mPhone, participantsToDial, this, mForegroundCall, false); - mPendingMO.setVideoState(videoState); + // Don't rely on the mPendingMO in this method; if the modem calls back through + // onCallProgressing, we'll end up nulling out mPendingMO, which means that + // TelephonyConnectionService would treat this call as an MMI code, which it is not, + // which would mean that the MMI code dialog would crash. + mPendingMO = pendingConnection; + pendingConnection.setVideoState(videoState); if (dialArgs.rttTextStream != null) { log("startConference: setting RTT stream on mPendingMO"); - mPendingMO.setCurrentRttTextStream(dialArgs.rttTextStream); + pendingConnection.setCurrentRttTextStream(dialArgs.rttTextStream); } } - addConnection(mPendingMO); + addConnection(pendingConnection); if (!holdBeforeDial) { - dialInternal(mPendingMO, clirMode, videoState, dialArgs.intentExtras); + dialInternal(pendingConnection, clirMode, videoState, dialArgs.intentExtras); } updatePhoneState(); mPhone.notifyPreciseCallStateChanged(); - return mPendingMO; + return pendingConnection; } @UnsupportedAppUsage From a65c2c212020bac4aff59187ade4d65d2ac444f3 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Wed, 22 Jul 2020 14:12:30 +0900 Subject: [PATCH 03/11] Fix a bug where the subtype is not initialized correctly Note that the initial subtype is still not set correctly at register() time because there is no way to communicate this information. This only affects callers that look at the extra info before the agent is marked connected, so this should only be internal callers, and they don't actually rely on this information. We could fix this by adding a hidden method in NetworkAgentConfig but this is not strictly necessary, so at this time do the simplest change only. Test: FrameworksTelephonyTest Bug: 161653721 Bug: 161737783 Change-Id: I397d773ac9bb5532d72036d3cb3d37e52c79ee4e (cherry picked from commit 7403bd5462d052b38809297e2668d224c29a8d0a) --- .../internal/telephony/dataconnection/DcNetworkAgent.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/java/com/android/internal/telephony/dataconnection/DcNetworkAgent.java b/src/java/com/android/internal/telephony/dataconnection/DcNetworkAgent.java index 3be5b28a72..7fa1f97e8d 100644 --- a/src/java/com/android/internal/telephony/dataconnection/DcNetworkAgent.java +++ b/src/java/com/android/internal/telephony/dataconnection/DcNetworkAgent.java @@ -92,6 +92,7 @@ public class DcNetworkAgent extends NetworkAgent { mTransportType = transportType; mDataConnection = dc; mNetworkInfo = new NetworkInfo(ni); + setLegacySubtype(ni.getSubtype(), ni.getSubtypeName()); setLegacyExtraInfo(ni.getExtraInfo()); // TODO: Remove before R is released. if (dc.getLinkProperties() != null From 95c2a717d476d88cd0ef7c1b3e7be81dc86efa07 Mon Sep 17 00:00:00 2001 From: Shuo Qian Date: Fri, 31 Jul 2020 18:17:46 -0700 Subject: [PATCH 04/11] Add package checking with Uid in EuiccController#getEid EuiccController does not validate the calling package name (i.e. to ensure that it is owned by the calling UID). It is therefore possible for an app to effectively gain carrier privileges in the call to EuiccController#getEid by passing the package name of another app that does has carrier privileges to one or more subscriptions. Test: safe net log Bug: 159062405 Change-Id: I0bf7c8b267a0c9cd877328c4ff3169950e1ff64f (cherry picked from commit 1221ede9d8cdea7586ae98357726df3d80e0e448) --- .../android/internal/telephony/euicc/EuiccController.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/java/com/android/internal/telephony/euicc/EuiccController.java b/src/java/com/android/internal/telephony/euicc/EuiccController.java index 09e17c3dea..400c326713 100644 --- a/src/java/com/android/internal/telephony/euicc/EuiccController.java +++ b/src/java/com/android/internal/telephony/euicc/EuiccController.java @@ -47,6 +47,7 @@ import android.telephony.euicc.EuiccManager; import android.telephony.euicc.EuiccManager.OtaStatus; import android.text.TextUtils; +import android.util.EventLog; import android.util.Log; import android.util.Pair; @@ -191,6 +192,12 @@ public void continueOperation(int cardId, Intent resolutionIntent, Bundle resolu @Override public String getEid(int cardId, String callingPackage) { boolean callerCanReadPhoneStatePrivileged = callerCanReadPhoneStatePrivileged(); + try { + mAppOpsManager.checkPackage(Binder.getCallingUid(), callingPackage); + } catch (SecurityException e) { + EventLog.writeEvent(0x534e4554, "159062405", -1, "Missing UID checking"); + throw e; + } long token = Binder.clearCallingIdentity(); try { if (!callerCanReadPhoneStatePrivileged From a5b1af74477233d8f9ce1fa0a6b0340d028169a2 Mon Sep 17 00:00:00 2001 From: Shuo Qian Date: Tue, 18 May 2021 16:24:07 -0700 Subject: [PATCH 05/11] Check READ_PRIVILEGED_PHONE_STATE instead of READ_PHONE_STATE for getAvailableSubscriptionInfoList Test: atest SubscriptionManagerTest#testSubscriptionGroupingWithPermission; Safety net logging Bug: 185235454 Change-Id: Ideef8793ac3c42ab30ac3004071d6be19e15b5fe (cherry picked from commit 9f741b9a3f871cde331f0c0f06abaa42e74f87f8) --- .../telephony/SubscriptionController.java | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/java/com/android/internal/telephony/SubscriptionController.java b/src/java/com/android/internal/telephony/SubscriptionController.java index 557e52596b..e34d5acd40 100644 --- a/src/java/com/android/internal/telephony/SubscriptionController.java +++ b/src/java/com/android/internal/telephony/SubscriptionController.java @@ -58,6 +58,7 @@ import android.telephony.UiccSlotInfo; import android.telephony.euicc.EuiccManager; import android.text.TextUtils; +import android.util.EventLog; import android.util.LocalLog; import android.util.Log; @@ -1045,12 +1046,18 @@ public int getActiveSubInfoCountMax() { @Override public List getAvailableSubscriptionInfoList(String callingPackage, String callingFeatureId) { - // This API isn't public, so no need to provide a valid subscription ID - we're not worried - // about carrier-privileged callers not having access. - if (!TelephonyPermissions.checkCallingOrSelfReadPhoneState( - mContext, SubscriptionManager.INVALID_SUBSCRIPTION_ID, callingPackage, - callingFeatureId, "getAvailableSubscriptionInfoList")) { - throw new SecurityException("Need READ_PHONE_STATE to call " + try { + enforceReadPrivilegedPhoneState("getAvailableSubscriptionInfoList"); + } catch (SecurityException e) { + try { + mContext.enforceCallingOrSelfPermission(Manifest.permission.READ_PHONE_STATE, null); + // If caller doesn't have READ_PRIVILEGED_PHONE_STATE permission but only + // has READ_PHONE_STATE permission, log this event. + EventLog.writeEvent(0x534e4554, "185235454", Binder.getCallingUid()); + } catch (SecurityException ex) { + // Ignore + } + throw new SecurityException("Need READ_PRIVILEGED_PHONE_STATE to call " + " getAvailableSubscriptionInfoList"); } From fa47477dada3720b5c382ae8fc6a573aa83dfe31 Mon Sep 17 00:00:00 2001 From: Chen Xu Date: Thu, 13 May 2021 15:49:01 -0700 Subject: [PATCH 06/11] filter deviceIdentifiers for subscriptionInfo if callers without perm Fix a security issue that app can read iccId of sim card(s) without requiring READ_PRIVILEGED_PHONE_STATE permission when calling hidden API SubscriptionManager.getAllActiveSubscriptionInfoList. Apply deviceIdentifier filter to remove those info if the caller does not have proper permissions. Bug: 183612370 Test: Manual Merged-in: If7d243c40d187008f8cb314b162228cbad1702a4 Change-Id: If7d243c40d187008f8cb314b162228cbad1702a4 (cherry picked from commit f6bb9b20840c29e74a37ea2b880e63b3fc9470ff) (cherry picked from commit f24c432620af8a772fdf09ba960573e78f2a99cf) --- .../android/internal/telephony/SubscriptionController.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/java/com/android/internal/telephony/SubscriptionController.java b/src/java/com/android/internal/telephony/SubscriptionController.java index e34d5acd40..8cef166ce9 100644 --- a/src/java/com/android/internal/telephony/SubscriptionController.java +++ b/src/java/com/android/internal/telephony/SubscriptionController.java @@ -891,6 +891,10 @@ public List getAllSubInfoList(String callingPackage, subList = getSubInfo(null, null); if (subList != null) { if (VDBG) logd("[getAllSubInfoList]- " + subList.size() + " infos return"); + subList.stream().map( + subscriptionInfo -> conditionallyRemoveIdentifiers(subscriptionInfo, + callingPackage, callingFeatureId, "getAllSubInfoList")) + .collect(Collectors.toList()); } else { if (VDBG) logd("[getAllSubInfoList]- no info return"); } From db51ae566981ecd77c40588900ff8d3e47e7fa8c Mon Sep 17 00:00:00 2001 From: SongFerngWang Date: Wed, 5 May 2021 21:30:39 +0800 Subject: [PATCH 07/11] [security] SubscriptionGroup is exposed to unprivileged callers SubscriptionInfo.mGroupUUID is not cleared in conditionallyRemoveIdentifiers if the caller only has READ_PHONE_STATE (based on a check to checkReadPhoneState) and not READ_DEVICE_IDENTIFIERS. Bug: 181053462 Test: atest SubscriptionManagerTest Change-Id: I68d1edb4e7cc2ad6696363ea1dacb09e839a651e Merged-In: I68d1edb4e7cc2ad6696363ea1dacb09e839a651e (cherry picked from commit 1399361f2e553a673b4a0d90131c44ee2fe0e32a) --- .../com/android/internal/telephony/SubscriptionController.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/java/com/android/internal/telephony/SubscriptionController.java b/src/java/com/android/internal/telephony/SubscriptionController.java index 8cef166ce9..3d38cce63e 100644 --- a/src/java/com/android/internal/telephony/SubscriptionController.java +++ b/src/java/com/android/internal/telephony/SubscriptionController.java @@ -4011,6 +4011,7 @@ private SubscriptionInfo conditionallyRemoveIdentifiers(SubscriptionInfo subInfo if (!hasIdentifierAccess) { result.clearIccId(); result.clearCardString(); + result.clearGroupUuid(); } if (!hasPhoneNumberAccess) { result.clearNumber(); From f367815f7ffb139d1cf23d1de48a5a72caa130b1 Mon Sep 17 00:00:00 2001 From: Chen Xu Date: Thu, 24 Jun 2021 11:08:00 -0700 Subject: [PATCH 08/11] fix the issue that clearCallingIdentity before appops check we should restore CallingIndentity when do the appops check for access device/subscriber identifier Bug: 187147737 Bug: 183612370 Test: Manual test Change-Id: Id0abfee602823f56811799a6d5c2bbe8cd5e2cc1 (cherry picked from commit 020d831ced2393aca3b51eb2a9fb805f5757fd87) Merged-in: Id0abfee602823f56811799a6d5c2bbe8cd5e2cc1 (cherry picked from commit 02bb0cc34bd940cebf71902b1d2aef5e7ed5e580) --- .../telephony/SubscriptionController.java | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/java/com/android/internal/telephony/SubscriptionController.java b/src/java/com/android/internal/telephony/SubscriptionController.java index 3d38cce63e..3120eab24e 100644 --- a/src/java/com/android/internal/telephony/SubscriptionController.java +++ b/src/java/com/android/internal/telephony/SubscriptionController.java @@ -886,22 +886,22 @@ public List getAllSubInfoList(String callingPackage, // Now that all security checks passes, perform the operation as ourselves. final long identity = Binder.clearCallingIdentity(); + List subList; try { - List subList = null; subList = getSubInfo(null, null); - if (subList != null) { - if (VDBG) logd("[getAllSubInfoList]- " + subList.size() + " infos return"); - subList.stream().map( - subscriptionInfo -> conditionallyRemoveIdentifiers(subscriptionInfo, - callingPackage, callingFeatureId, "getAllSubInfoList")) - .collect(Collectors.toList()); - } else { - if (VDBG) logd("[getAllSubInfoList]- no info return"); - } - return subList; } finally { Binder.restoreCallingIdentity(identity); } + if (subList != null) { + if (VDBG) logd("[getAllSubInfoList]- " + subList.size() + " infos return"); + subList.stream().map( + subscriptionInfo -> conditionallyRemoveIdentifiers(subscriptionInfo, + callingPackage, callingFeatureId, "getAllSubInfoList")) + .collect(Collectors.toList()); + } else { + if (VDBG) logd("[getAllSubInfoList]- no info return"); + } + return subList; } @Deprecated From 4277230fb439d531947926d35658c003ad82adf4 Mon Sep 17 00:00:00 2001 From: Chen Xu Date: Thu, 24 Jun 2021 11:08:00 -0700 Subject: [PATCH 09/11] fix the issue that clearCallingIdentity before appops check we should restore CallingIndentity when do the appops check for access device/subscriber identifier Bug: 187147737 Bug: 183612370 Test: Manual test Change-Id: Id0abfee602823f56811799a6d5c2bbe8cd5e2cc1 (cherry picked from commit 020d831ced2393aca3b51eb2a9fb805f5757fd87) Merged-in: Id0abfee602823f56811799a6d5c2bbe8cd5e2cc1 (cherry picked from commit 5fc2375db75129ac458a618c35ede8fb045976c4) --- .../telephony/SubscriptionController.java | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/java/com/android/internal/telephony/SubscriptionController.java b/src/java/com/android/internal/telephony/SubscriptionController.java index 8a03431f99..42d918614e 100644 --- a/src/java/com/android/internal/telephony/SubscriptionController.java +++ b/src/java/com/android/internal/telephony/SubscriptionController.java @@ -887,22 +887,22 @@ public List getAllSubInfoList(String callingPackage, // Now that all security checks passes, perform the operation as ourselves. final long identity = Binder.clearCallingIdentity(); + List subList; try { - List subList = null; subList = getSubInfo(null, null); - if (subList != null) { - if (VDBG) logd("[getAllSubInfoList]- " + subList.size() + " infos return"); - subList.stream().map( - subscriptionInfo -> conditionallyRemoveIdentifiers(subscriptionInfo, - callingPackage, callingFeatureId, "getAllSubInfoList")) - .collect(Collectors.toList()); - } else { - if (VDBG) logd("[getAllSubInfoList]- no info return"); - } - return subList; } finally { Binder.restoreCallingIdentity(identity); } + if (subList != null) { + if (VDBG) logd("[getAllSubInfoList]- " + subList.size() + " infos return"); + subList.stream().map( + subscriptionInfo -> conditionallyRemoveIdentifiers(subscriptionInfo, + callingPackage, callingFeatureId, "getAllSubInfoList")) + .collect(Collectors.toList()); + } else { + if (VDBG) logd("[getAllSubInfoList]- no info return"); + } + return subList; } private List makeCacheListCopyWithLock(List cacheSubList) { From 8d84f8b34f8075bfa396d43b6ca07f5ef37fcf9c Mon Sep 17 00:00:00 2001 From: Chen Xu Date: Fri, 5 Nov 2021 23:52:37 -0700 Subject: [PATCH 10/11] filter deviceIdentifiers for subscriptionInfo if callers without perm Fix a security issue that app can read iccId of sim card(s) without requiring READ_PRIVILEGED_PHONE_STATE permission when calling hidden API SubscriptionManager.getAllActiveSubscriptionInfoList. Apply deviceIdentifier filter to remove those info if the caller does not have proper permissions. The previous fix forgot to reassign returned value with filtered result. Bug: 183612370 Test: Manual Change-Id: I592a100f274bfe8a9f1b17b9a4c54ae2aadd6fdb (cherry picked from commit 9845ef56ff9a397041026af9034f1c17d4e65d97) Merged-In:I592a100f274bfe8a9f1b17b9a4c54ae2aadd6fdb --- .../telephony/SubscriptionController.java | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/java/com/android/internal/telephony/SubscriptionController.java b/src/java/com/android/internal/telephony/SubscriptionController.java index 3120eab24e..b3a317cb21 100644 --- a/src/java/com/android/internal/telephony/SubscriptionController.java +++ b/src/java/com/android/internal/telephony/SubscriptionController.java @@ -874,6 +874,19 @@ public SubscriptionInfo getActiveSubscriptionInfoForSimSlotIndex(int slotIndex, @Override public List getAllSubInfoList(String callingPackage, String callingFeatureId) { + return getAllSubInfoList(callingPackage, callingFeatureId, false); + } + + /** + * @param callingPackage The package making the IPC. + * @param callingFeatureId The feature in the package + * @param skipConditionallyRemoveIdentifier if set, skip removing identifier conditionally + * @return List of all SubscriptionInfo records in database, + * include those that were inserted before, maybe empty but not null. + * @hide + */ + public List getAllSubInfoList(String callingPackage, + String callingFeatureId, boolean skipConditionallyRemoveIdentifier) { if (VDBG) logd("[getAllSubInfoList]+"); // This API isn't public, so no need to provide a valid subscription ID - we're not worried @@ -892,9 +905,9 @@ public List getAllSubInfoList(String callingPackage, } finally { Binder.restoreCallingIdentity(identity); } - if (subList != null) { + if (subList != null && !skipConditionallyRemoveIdentifier) { if (VDBG) logd("[getAllSubInfoList]- " + subList.size() + " infos return"); - subList.stream().map( + subList = subList.stream().map( subscriptionInfo -> conditionallyRemoveIdentifiers(subscriptionInfo, callingPackage, callingFeatureId, "getAllSubInfoList")) .collect(Collectors.toList()); @@ -3612,8 +3625,10 @@ public List getSubscriptionsInGroup(ParcelUuid groupUuid, List subInfoList; try { + // need to bypass removing identifier check because that will remove the subList without + // group id. subInfoList = getAllSubInfoList(mContext.getOpPackageName(), - mContext.getAttributionTag()); + mContext.getAttributionTag(), true); if (groupUuid == null || subInfoList == null || subInfoList.isEmpty()) { return new ArrayList<>(); } From f7248cd81fc0b9ee6283a5e6f42f2ff7ec4d72c8 Mon Sep 17 00:00:00 2001 From: Ling Ma Date: Tue, 3 May 2022 18:13:57 -0700 Subject: [PATCH 11/11] Enforce privileged phone state for getSubscriptionProperty(GROUP_UUID) Bug: 213457638 Test: atest Change-Id: I8d7cc836402a9a7695c972860d38035c4ec0fa44 Merged-In: I8d7cc836402a9a7695c972860d38035c4ec0fa44 Merged-In: Ie8017c39a495f93603aeb5d1a335fe2fe528cf77 (cherry picked from commit b0e3c5d17e44b6de4ddb5e1ad0018243d38e2cc4) (cherry picked from commit 7f3dd2afda9546b8a08897b016393cd5fd54c8b6) Merged-In: I8d7cc836402a9a7695c972860d38035c4ec0fa44 --- .../telephony/SubscriptionController.java | 16 ++++++++-- .../telephony/SubscriptionControllerTest.java | 31 +++++++++++++++++++ 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/src/java/com/android/internal/telephony/SubscriptionController.java b/src/java/com/android/internal/telephony/SubscriptionController.java index b3a317cb21..326c7b0245 100644 --- a/src/java/com/android/internal/telephony/SubscriptionController.java +++ b/src/java/com/android/internal/telephony/SubscriptionController.java @@ -2954,9 +2954,19 @@ private int setSubscriptionPropertyIntoContentResolver( @Override public String getSubscriptionProperty(int subId, String propKey, String callingPackage, String callingFeatureId) { - if (!TelephonyPermissions.checkCallingOrSelfReadPhoneState(mContext, subId, callingPackage, - callingFeatureId, "getSubscriptionProperty")) { - return null; + switch (propKey) { + case SubscriptionManager.GROUP_UUID: + if (mContext.checkCallingOrSelfPermission( + Manifest.permission.READ_PRIVILEGED_PHONE_STATE) != PERMISSION_GRANTED) { + EventLog.writeEvent(0x534e4554, "213457638", Binder.getCallingUid()); + return null; + } + break; + default: + if (!TelephonyPermissions.checkCallingOrSelfReadPhoneState(mContext, subId, + callingPackage, callingFeatureId, "getSubscriptionProperty")) { + return null; + } } final long identity = Binder.clearCallingIdentity(); diff --git a/tests/telephonytests/src/com/android/internal/telephony/SubscriptionControllerTest.java b/tests/telephonytests/src/com/android/internal/telephony/SubscriptionControllerTest.java index 57c9eb28cf..23e6f8ff31 100644 --- a/tests/telephonytests/src/com/android/internal/telephony/SubscriptionControllerTest.java +++ b/tests/telephonytests/src/com/android/internal/telephony/SubscriptionControllerTest.java @@ -701,6 +701,37 @@ public void testSetSubscriptionGroupWithModifyPermission() throws Exception { assertNotEquals(groupId, newGroupId); } + @Test + @SmallTest + public void testGetSubscriptionProperty() throws Exception { + testInsertSim(); + ContentValues values = new ContentValues(); + values.put(SubscriptionManager.GROUP_UUID, 1); + mFakeTelephonyProvider.update(SubscriptionManager.CONTENT_URI, values, + SubscriptionManager.UNIQUE_KEY_SUBSCRIPTION_ID + "=" + 1, null); + + mContextFixture.removeCallingOrSelfPermission(ContextFixture.PERMISSION_ENABLE_ALL); + mContextFixture.addCallingOrSelfPermission(Manifest.permission.READ_PHONE_STATE); + + // should succeed with read phone state permission + String prop = mSubscriptionControllerUT.getSubscriptionProperty(1, + SubscriptionManager.CB_EXTREME_THREAT_ALERT, mContext.getOpPackageName(), + mContext.getAttributionTag()); + + assertNotEquals(null, prop); + + // group UUID requires privileged phone state permission + prop = mSubscriptionControllerUT.getSubscriptionProperty(1, SubscriptionManager.GROUP_UUID, + mContext.getOpPackageName(), mContext.getAttributionTag()); + assertEquals(null, prop); + + // group UUID should succeed once privileged phone state permission is granted + mContextFixture.addCallingOrSelfPermission(Manifest.permission.READ_PRIVILEGED_PHONE_STATE); + prop = mSubscriptionControllerUT.getSubscriptionProperty(1, SubscriptionManager.GROUP_UUID, + mContext.getOpPackageName(), mContext.getAttributionTag()); + assertNotEquals(null, prop); + } + @Test @SmallTest public void testCreateSubscriptionGroupWithCarrierPrivilegePermission() throws Exception {