From 9db22c7a89ca50c0a2e41df08baef56f4618efcd Mon Sep 17 00:00:00 2001 From: Michael Groover Date: Tue, 16 Oct 2018 17:53:17 -0700 Subject: [PATCH] Temporarily revert device identifier access check to previous behavior If a calling package does not meet the new requirements for device identifier access the calling package and method will be logged and the previous READ_PHONE_STATE permission check will be performed to grant access to the requested identifier. This is to prevent additional breakage for apps that currently require device identifiers but have not yet been granted the privileged permission or carrier privileges. Bug: 117585389 Test: cts-tradefed run cts -m CtsPermissionTestCases \ -t android.permission.cts.TelephonyManagerPermissionTest Test: Manually invoked an app targeting pre-Q and verified access to device identifiers with the READ_PHONE_STATE permission. Change-Id: I03339486a2d6971b93472479b79959c888beba1e --- core/java/android/provider/Settings.java | 22 +++++++++++ .../android/provider/SettingsBackupTest.java | 2 + .../telephony/TelephonyPermissions.java | 37 ++++++++++--------- 3 files changed, 44 insertions(+), 17 deletions(-) diff --git a/core/java/android/provider/Settings.java b/core/java/android/provider/Settings.java index acb75778e08ea..880e662cabfe0 100644 --- a/core/java/android/provider/Settings.java +++ b/core/java/android/provider/Settings.java @@ -12362,6 +12362,28 @@ public final class Settings { public static final String SMS_ACCESS_RESTRICTION_ENABLED = "sms_access_restriction_enabled"; + /** + * If set to 1, an app must have the READ_PRIVILEGED_PHONE_STATE permission (or be a device + * / profile owner with the READ_PHONE_STATE permission) to access device identifiers. + * + * STOPSHIP: Remove this once we ship with the new device identifier check enabled. + * + * @hide + */ + public static final String PRIVILEGED_DEVICE_IDENTIFIER_CHECK_ENABLED = + "privileged_device_identifier_check_enabled"; + + /** + * If set to 1, an app that is targeting Q and does not meet the new requirements to access + * device identifiers will receive a SecurityException. + * + * STOPSHIP: Remove this once we ship with the new device identifier check enabled. + * + * @hide + */ + public static final String PRIVILEGED_DEVICE_IDENTIFIER_TARGET_Q_BEHAVIOR_ENABLED = + "privileged_device_identifier_target_q_behavior_enabled"; + /** * If set to 1, SettingsProvider's restoreAnyVersion="true" attribute will be ignored * and restoring to lower version of platform API will be skipped. diff --git a/core/tests/coretests/src/android/provider/SettingsBackupTest.java b/core/tests/coretests/src/android/provider/SettingsBackupTest.java index 60abd94681796..a375097d2ea19 100644 --- a/core/tests/coretests/src/android/provider/SettingsBackupTest.java +++ b/core/tests/coretests/src/android/provider/SettingsBackupTest.java @@ -368,6 +368,8 @@ public class SettingsBackupTest { Settings.Global.PRIV_APP_OOB_ENABLED, Settings.Global.PRIV_APP_OOB_LIST, Settings.Global.PRIVATE_DNS_DEFAULT_MODE, + Settings.Global.PRIVILEGED_DEVICE_IDENTIFIER_CHECK_ENABLED, + Settings.Global.PRIVILEGED_DEVICE_IDENTIFIER_TARGET_Q_BEHAVIOR_ENABLED, Settings.Global.PROVISIONING_APN_ALARM_DELAY_IN_MS, Settings.Global.RADIO_BLUETOOTH, Settings.Global.RADIO_CELL, diff --git a/telephony/java/com/android/internal/telephony/TelephonyPermissions.java b/telephony/java/com/android/internal/telephony/TelephonyPermissions.java index 9730ebc57fcfa..eda8e77660549 100644 --- a/telephony/java/com/android/internal/telephony/TelephonyPermissions.java +++ b/telephony/java/com/android/internal/telephony/TelephonyPermissions.java @@ -29,6 +29,7 @@ import android.os.Process; import android.os.RemoteException; import android.os.ServiceManager; import android.os.UserHandle; +import android.provider.Settings; import android.telephony.Rlog; import android.telephony.SubscriptionManager; import android.telephony.TelephonyManager; @@ -44,10 +45,6 @@ public final class TelephonyPermissions { private static final boolean DBG = false; - // When set to true this flag will treat all apps that fail the device identifier check as - // though they are targeting pre-Q and return dummy data instead of throwing a SecurityException - private static final boolean RELAX_DEVICE_IDENTIFIER_CHECK = true; - private static final Supplier TELEPHONY_SUPPLIER = () -> ITelephony.Stub.asInterface(ServiceManager.getService(Context.TELEPHONY_SERVICE)); @@ -280,23 +277,29 @@ public final class TelephonyPermissions { */ private static boolean reportAccessDeniedToReadIdentifiers(Context context, int subId, int pid, int uid, String callingPackage, String message) { - // if the device identifier check is relaxed then just return false to return dummy data to - // the caller instead of throwing a SecurityException for apps targeting Q+. - if (RELAX_DEVICE_IDENTIFIER_CHECK) { - Log.wtf(LOG_TAG, - "reportAccessDeniedToReadIdentifiers:" + callingPackage + ":" + message); - return false; + Log.wtf(LOG_TAG, + "reportAccessDeniedToReadIdentifiers:" + callingPackage + ":" + message); + // if the device identifier check is relaxed then revert to the READ_PHONE_STATE permission + // check that was previously required to access device identifiers. + boolean relaxDeviceIdentifierCheck = Settings.Global.getInt(context.getContentResolver(), + Settings.Global.PRIVILEGED_DEVICE_IDENTIFIER_CHECK_ENABLED, 0) == 0; + if (relaxDeviceIdentifierCheck) { + return checkReadPhoneState(context, subId, pid, uid, callingPackage, message); } else { + boolean targetQBehaviorDisabled = Settings.Global.getInt(context.getContentResolver(), + Settings.Global.PRIVILEGED_DEVICE_IDENTIFIER_TARGET_Q_BEHAVIOR_ENABLED, 0) == 0; if (callingPackage != null) { try { - // if the target SDK is pre-Q then check if the calling package would have - // previously had access to device identifiers. + // if the target SDK is pre-Q or the target Q behavior is disabled then check if + // the calling package would have previously had access to device identifiers. ApplicationInfo callingPackageInfo = context.getPackageManager().getApplicationInfo( callingPackage, 0); - if (callingPackageInfo != null - && callingPackageInfo.targetSdkVersion < Build.VERSION_CODES.Q) { - if (context.checkPermission(android.Manifest.permission.READ_PHONE_STATE, + if (callingPackageInfo != null && ( + callingPackageInfo.targetSdkVersion < Build.VERSION_CODES.Q + || targetQBehaviorDisabled)) { + if (context.checkPermission( + android.Manifest.permission.READ_PHONE_STATE, pid, uid) == PackageManager.PERMISSION_GRANTED) { return false; @@ -312,8 +315,8 @@ public final class TelephonyPermissions { // default to throwing the SecurityException. } } - throw new SecurityException(message + ": The user " + uid + " does not have the " - + "READ_PRIVILEGED_PHONE_STATE permission to access the device identifiers"); + throw new SecurityException(message + ": The user " + uid + + " does not meet the requirements to access device identifiers."); } }