From 2cdcd9837910e372a247907930deb50477186013 Mon Sep 17 00:00:00 2001 From: Nathan Harold Date: Mon, 25 Jun 2018 19:13:29 -0700 Subject: [PATCH] Allow Cell Location for SYSTEM_UID and ROOT_UID This change permits the system uid and root uid to access cellular location information via the binder. Previously this was restricted to the phone uid, but running with uid=system is a privileged situation, which makes me think this this wasn't intentional. Also add a few lines of debug code to make issues in LocationAccessPolicy easier to track down in the future. Bug: 110806860 Test: manual - ran with SL4A as SYSTEM_UID and verified access to getAllCellInfo. Change-Id: Ie18be2cd72c49f1859d1434428f82f164bed8756 --- .../telephony/LocationAccessPolicy.java | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/telephony/java/android/telephony/LocationAccessPolicy.java b/telephony/java/android/telephony/LocationAccessPolicy.java index 6db8e825dbf0d..53d69f447a56e 100644 --- a/telephony/java/android/telephony/LocationAccessPolicy.java +++ b/telephony/java/android/telephony/LocationAccessPolicy.java @@ -39,7 +39,8 @@ import java.util.List; * @hide */ public final class LocationAccessPolicy { - private static final String LOG_TAG = LocationAccessPolicy.class.getSimpleName(); + private static final String TAG = "LocationAccessPolicy"; + private static final boolean DBG = false; /** * API to determine if the caller has permissions to get cell location. @@ -52,12 +53,13 @@ public final class LocationAccessPolicy { */ public static boolean canAccessCellLocation(@NonNull Context context, @NonNull String pkgName, int uid, int pid, boolean throwOnDeniedPermission) throws SecurityException { - Trace.beginSection("TelephonyLohcationCheck"); + Trace.beginSection("TelephonyLocationCheck"); try { - // Always allow the phone process to access location. This avoid breaking legacy code - // that rely on public-facing APIs to access cell location, and it doesn't create a - // info leak risk because the cell location is stored in the phone process anyway. - if (uid == Process.PHONE_UID) { + // Always allow the phone process and system server to access location. This avoid + // breaking legacy code that rely on public-facing APIs to access cell location, and + // it doesn't create an info leak risk because the cell location is stored in the phone + // process anyway, and the system server already has location access. + if (uid == Process.PHONE_UID || uid == Process.SYSTEM_UID || uid == Process.ROOT_UID) { return true; } @@ -72,15 +74,18 @@ public final class LocationAccessPolicy { pid, uid, "canAccessCellLocation"); } else if (context.checkPermission(Manifest.permission.ACCESS_COARSE_LOCATION, pid, uid) == PackageManager.PERMISSION_DENIED) { + if (DBG) Log.w(TAG, "Permission checked failed (" + pid + "," + uid + ")"); return false; } final int opCode = AppOpsManager.permissionToOpCode( Manifest.permission.ACCESS_COARSE_LOCATION); if (opCode != AppOpsManager.OP_NONE && context.getSystemService(AppOpsManager.class) .noteOpNoThrow(opCode, uid, pkgName) != AppOpsManager.MODE_ALLOWED) { + if (DBG) Log.w(TAG, "AppOp check failed (" + uid + "," + pkgName + ")"); return false; } if (!isLocationModeEnabled(context, UserHandle.getUserId(uid))) { + if (DBG) Log.w(TAG, "Location disabled, failed, (" + uid + ")"); return false; } // If the user or profile is current, permission is granted. @@ -94,7 +99,7 @@ public final class LocationAccessPolicy { private static boolean isLocationModeEnabled(@NonNull Context context, @UserIdInt int userId) { LocationManager locationManager = context.getSystemService(LocationManager.class); if (locationManager == null) { - Log.w(LOG_TAG, "Couldn't get location manager, denying location access"); + Log.w(TAG, "Couldn't get location manager, denying location access"); return false; } return locationManager.isLocationEnabledForUser(UserHandle.of(userId));