From b870dbf86bd47c750d2a5350a83ec245396578ee Mon Sep 17 00:00:00 2001 From: David Christie Date: Mon, 22 Jun 2015 12:42:53 -0700 Subject: [PATCH] Fix privacy leaks in LocationManager -Register for listener for permission changes and stop request immediately if client loses permission. -Also remove permission requirement to remove geofences and clean up permission annotations. Bug: 21903866 Change-Id: I7e028b6b2ca5b21f25fcbba5de86dfb55caff872 --- .../java/android/location/LocationManager.java | 4 -- .../com/android/server/LocationManagerService.java | 48 +++++++++++++++++----- 2 files changed, 38 insertions(+), 14 deletions(-) diff --git a/location/java/android/location/LocationManager.java b/location/java/android/location/LocationManager.java index b09f2169e8ad..2c1932439584 100644 --- a/location/java/android/location/LocationManager.java +++ b/location/java/android/location/LocationManager.java @@ -892,7 +892,6 @@ public class LocationManager { * @param listener listener object that no longer needs location updates * @throws IllegalArgumentException if listener is null */ - @RequiresPermission(anyOf = {ACCESS_COARSE_LOCATION, ACCESS_FINE_LOCATION}) public void removeUpdates(LocationListener listener) { checkListener(listener); String packageName = mContext.getPackageName(); @@ -1055,7 +1054,6 @@ public class LocationManager { * @throws SecurityException if {@link android.Manifest.permission#ACCESS_FINE_LOCATION} * permission is not present */ - @RequiresPermission(anyOf = {ACCESS_COARSE_LOCATION, ACCESS_FINE_LOCATION}) public void removeProximityAlert(PendingIntent intent) { checkPendingIntent(intent); String packageName = mContext.getPackageName(); @@ -1083,7 +1081,6 @@ public class LocationManager { * * @hide */ - @RequiresPermission(anyOf = {ACCESS_COARSE_LOCATION, ACCESS_FINE_LOCATION}) public void removeGeofence(Geofence fence, PendingIntent intent) { checkPendingIntent(intent); checkGeofence(fence); @@ -1107,7 +1104,6 @@ public class LocationManager { * * @hide */ - @RequiresPermission(anyOf = {ACCESS_COARSE_LOCATION, ACCESS_FINE_LOCATION}) public void removeAllGeofences(PendingIntent intent) { checkPendingIntent(intent); String packageName = mContext.getPackageName(); diff --git a/services/core/java/com/android/server/LocationManagerService.java b/services/core/java/com/android/server/LocationManagerService.java index 61bedf5d6bd7..cae060a8493e 100644 --- a/services/core/java/com/android/server/LocationManagerService.java +++ b/services/core/java/com/android/server/LocationManagerService.java @@ -270,6 +270,17 @@ public class LocationManagerService extends ILocationManager.Stub { }; mAppOps.startWatchingMode(AppOpsManager.OP_COARSE_LOCATION, null, callback); + PackageManager.OnPermissionsChangedListener permissionListener + = new PackageManager.OnPermissionsChangedListener() { + @Override + public void onPermissionsChanged(final int uid) { + synchronized (mLock) { + applyAllProviderRequirementsLocked(); + } + } + }; + mPackageManager.addOnPermissionsChangeListener(permissionListener); + mUserManager = (UserManager) mContext.getSystemService(Context.USER_SERVICE); updateUserProfiles(mCurrentUserId); @@ -1133,23 +1144,34 @@ public class LocationManagerService extends ILocationManager.Stub { return -1; } - boolean reportLocationAccessNoThrow(int uid, String packageName, int allowedResolutionLevel) { + boolean reportLocationAccessNoThrow( + int pid, int uid, String packageName, int allowedResolutionLevel) { int op = resolutionLevelToOp(allowedResolutionLevel); if (op >= 0) { if (mAppOps.noteOpNoThrow(op, uid, packageName) != AppOpsManager.MODE_ALLOWED) { return false; } } + + if (getAllowedResolutionLevel(pid, uid) < allowedResolutionLevel) { + return false; + } + return true; } - boolean checkLocationAccess(int uid, String packageName, int allowedResolutionLevel) { + boolean checkLocationAccess(int pid, int uid, String packageName, int allowedResolutionLevel) { int op = resolutionLevelToOp(allowedResolutionLevel); if (op >= 0) { if (mAppOps.checkOp(op, uid, packageName) != AppOpsManager.MODE_ALLOWED) { return false; } } + + if (getAllowedResolutionLevel(pid, uid) < allowedResolutionLevel) { + return false; + } + return true; } @@ -1347,7 +1369,10 @@ public class LocationManagerService extends ILocationManager.Stub { if (records != null) { for (UpdateRecord record : records) { if (isCurrentProfile(UserHandle.getUserId(record.mReceiver.mUid))) { - if (checkLocationAccess(record.mReceiver.mUid, record.mReceiver.mPackageName, + if (checkLocationAccess( + record.mReceiver.mPid, + record.mReceiver.mUid, + record.mReceiver.mPackageName, record.mReceiver.mAllowedResolutionLevel)) { LocationRequest locationRequest = record.mRequest; providerRequest.locationRequests.add(locationRequest); @@ -1583,7 +1608,7 @@ public class LocationManagerService extends ILocationManager.Stub { try { // We don't check for MODE_IGNORED here; we will do that when we go to deliver // a location. - checkLocationAccess(uid, packageName, allowedResolutionLevel); + checkLocationAccess(pid, uid, packageName, allowedResolutionLevel); synchronized (mLock) { Receiver recevier = checkListenerOrIntentLocked(listener, intent, pid, uid, @@ -1711,6 +1736,7 @@ public class LocationManagerService extends ILocationManager.Stub { request.getProvider()); // no need to sanitize this request, as only the provider name is used + final int pid = Binder.getCallingPid(); final int uid = Binder.getCallingUid(); final long identity = Binder.clearCallingIdentity(); try { @@ -1720,7 +1746,7 @@ public class LocationManagerService extends ILocationManager.Stub { return null; } - if (!reportLocationAccessNoThrow(uid, packageName, allowedResolutionLevel)) { + if (!reportLocationAccessNoThrow(pid, uid, packageName, allowedResolutionLevel)) { if (D) Log.d(TAG, "not returning last loc for no op app: " + packageName); return null; @@ -1794,7 +1820,6 @@ public class LocationManagerService extends ILocationManager.Stub { @Override public void removeGeofence(Geofence geofence, PendingIntent intent, String packageName) { - checkResolutionLevelIsSufficientForGeofenceUse(getCallerAllowedResolutionLevel()); checkPendingIntent(intent); checkPackageName(packageName); @@ -1816,10 +1841,11 @@ public class LocationManagerService extends ILocationManager.Stub { checkResolutionLevelIsSufficientForProviderUse(allowedResolutionLevel, LocationManager.GPS_PROVIDER); + final int pid = Binder.getCallingPid(); final int uid = Binder.getCallingUid(); final long ident = Binder.clearCallingIdentity(); try { - if (!checkLocationAccess(uid, packageName, allowedResolutionLevel)) { + if (!checkLocationAccess(pid, uid, packageName, allowedResolutionLevel)) { return false; } } finally { @@ -1859,11 +1885,12 @@ public class LocationManagerService extends ILocationManager.Stub { allowedResolutionLevel, LocationManager.GPS_PROVIDER); + int pid = Binder.getCallingPid(); int uid = Binder.getCallingUid(); long identity = Binder.clearCallingIdentity(); boolean hasLocationAccess; try { - hasLocationAccess = checkLocationAccess(uid, packageName, allowedResolutionLevel); + hasLocationAccess = checkLocationAccess(pid, uid, packageName, allowedResolutionLevel); } finally { Binder.restoreCallingIdentity(identity); } @@ -1890,11 +1917,12 @@ public class LocationManagerService extends ILocationManager.Stub { allowedResolutionLevel, LocationManager.GPS_PROVIDER); + int pid = Binder.getCallingPid(); int uid = Binder.getCallingUid(); long identity = Binder.clearCallingIdentity(); boolean hasLocationAccess; try { - hasLocationAccess = checkLocationAccess(uid, packageName, allowedResolutionLevel); + hasLocationAccess = checkLocationAccess(pid, uid, packageName, allowedResolutionLevel); } finally { Binder.restoreCallingIdentity(identity); } @@ -2209,7 +2237,7 @@ public class LocationManagerService extends ILocationManager.Stub { continue; } - if (!reportLocationAccessNoThrow(receiver.mUid, receiver.mPackageName, + if (!reportLocationAccessNoThrow(receiver.mPid, receiver.mUid, receiver.mPackageName, receiver.mAllowedResolutionLevel)) { if (D) Log.d(TAG, "skipping loc update for no op app: " + receiver.mPackageName); -- 2.11.0