Block immutable PIs from location APIs

Bug: 171317480
Test: atest CtsLocationFineTestCases
Change-Id: Ifba03b853f0681a5949d89c4a8e7c2de9a5c6018

3 files changed, 66 insertions, 42 deletions

diff --git a/location/java/android/location/LocationManager.java b/location/java/android/location/LocationManager.java
index 3493693ac67e..b61b79e07736 100644
--- a/location/java/android/location/LocationManager.java
+++ b/location/java/android/location/LocationManager.java
@@ -76,12 +76,11 @@ import java.util.function.Consumer;
  * obtain periodic updates of the device's geographical location, or to be notified when the device
  * enters the proximity of a given geographical location.
  *
- * <p class="note">Unless noted, all Location API methods require the {@link
- * android.Manifest.permission#ACCESS_COARSE_LOCATION} or {@link
- * android.Manifest.permission#ACCESS_FINE_LOCATION} permissions. If your application only has the
- * coarse permission then it will not have access to fine location providers. Other providers will
- * still return location results, but the exact location will be obfuscated to a coarse level of
- * accuracy.
+ * <p class="note">Unless otherwise noted, all Location API methods require the
+ * {@link android.Manifest.permission#ACCESS_COARSE_LOCATION} or
+ * {@link android.Manifest.permission#ACCESS_FINE_LOCATION} permissions. If your application only
+ * has the coarse permission then providers will still return location results, but the exact
+ * location will be obfuscated to a coarse level of accuracy.
  */
 @SuppressWarnings({"deprecation"})
 @SystemService(Context.LOCATION_SERVICE)
@@ -89,6 +88,16 @@ import java.util.function.Consumer;
 public class LocationManager {
 
     /**
+     * For apps targeting Android S and above, immutable PendingIntents passed into location APIs
+     * will generate an IllegalArgumentException.
+     *
+     * @hide
+     */
+    @ChangeId
+    @EnabledAfter(targetSdkVersion = Build.VERSION_CODES.R)
+    public static final long BLOCK_IMMUTABLE_PENDING_INTENTS = 171317480L;
+
+    /**
      * For apps targeting Android S and above, LocationRequest system APIs may not be used with
      * PendingIntent location requests.
      *
@@ -96,7 +105,7 @@ public class LocationManager {
      */
     @ChangeId
     @EnabledAfter(targetSdkVersion = Build.VERSION_CODES.R)
-    public static final long PREVENT_PENDING_INTENT_SYSTEM_API_USAGE = 169887240L;
+    public static final long BLOCK_PENDING_INTENT_SYSTEM_API_USAGE = 169887240L;
 
     /**
      * For apps targeting Android S and above, location clients may receive historical locations
@@ -116,7 +125,7 @@ public class LocationManager {
      */
     @ChangeId
     @EnabledAfter(targetSdkVersion = Build.VERSION_CODES.Q)
-    private static final long GET_PROVIDER_SECURITY_EXCEPTIONS = 150935354L;
+    public static final long GET_PROVIDER_SECURITY_EXCEPTIONS = 150935354L;
 
     /**
      * For apps targeting Android K and above, supplied {@link PendingIntent}s must be targeted to a
@@ -126,7 +135,7 @@ public class LocationManager {
      */
     @ChangeId
     @EnabledAfter(targetSdkVersion = Build.VERSION_CODES.JELLY_BEAN)
-    private static final long TARGETED_PENDING_INTENT = 148963590L;
+    public static final long BLOCK_UNTARGETED_PENDING_INTENTS = 148963590L;
 
     /**
      * For apps targeting Android K and above, incomplete locations may not be passed to
@@ -136,7 +145,7 @@ public class LocationManager {
      */
     @ChangeId
     @EnabledAfter(targetSdkVersion = Build.VERSION_CODES.JELLY_BEAN)
-    private static final long INCOMPLETE_LOCATION = 148964793L;
+    public static final long BLOCK_INCOMPLETE_LOCATIONS = 148964793L;
 
     /**
      * For apps targeting Android S and above, all {@link GpsStatus} API usage must be replaced with
@@ -146,7 +155,7 @@ public class LocationManager {
      */
     @ChangeId
     @EnabledAfter(targetSdkVersion = Build.VERSION_CODES.R)
-    private static final long GPS_STATUS_USAGE = 144027538L;
+    public static final long BLOCK_GPS_STATUS_USAGE = 144027538L;
 
     /**
      * Name of the network location provider.
@@ -1372,11 +1381,16 @@ public class LocationManager {
         Preconditions.checkArgument(locationRequest != null, "invalid null location request");
         Preconditions.checkArgument(pendingIntent != null, "invalid null pending intent");
 
-        if (Compatibility.isChangeEnabled(TARGETED_PENDING_INTENT)) {
+        if (Compatibility.isChangeEnabled(BLOCK_UNTARGETED_PENDING_INTENTS)) {
             Preconditions.checkArgument(pendingIntent.isTargetedToPackage(),
                     "pending intent must be targeted to a package");
         }
 
+        if (Compatibility.isChangeEnabled(BLOCK_IMMUTABLE_PENDING_INTENTS)) {
+            Preconditions.checkArgument(!pendingIntent.isImmutable(),
+                    "pending intent must be mutable");
+        }
+
         try {
             mService.registerLocationPendingIntent(provider, locationRequest, pendingIntent,
                     mContext.getPackageName(), mContext.getAttributionTag());
@@ -1729,7 +1743,7 @@ public class LocationManager {
         Preconditions.checkArgument(provider != null, "invalid null provider");
         Preconditions.checkArgument(location != null, "invalid null location");
 
-        if (Compatibility.isChangeEnabled(INCOMPLETE_LOCATION)) {
+        if (Compatibility.isChangeEnabled(BLOCK_INCOMPLETE_LOCATIONS)) {
             Preconditions.checkArgument(location.isComplete(),
                     "incomplete location object, missing timestamp or accuracy?");
         } else {
@@ -1821,29 +1835,38 @@ public class LocationManager {
      * {@link android.Manifest.permission#ACCESS_COARSE_LOCATION}. From API version 17 and onwards,
      * this method requires {@link android.Manifest.permission#ACCESS_FINE_LOCATION} permission.
      *
-     * @param latitude   the latitude of the central point of the alert region
-     * @param longitude  the longitude of the central point of the alert region
-     * @param radius     the radius of the central point of the alert region in meters
-     * @param expiration expiration realtime for this proximity alert in milliseconds, or -1 to
-     *                   indicate no expiration
-     * @param intent     a {@link PendingIntent} that will sent when entry to or exit from the alert
-     *                   region is detected
+     * @param latitude      the latitude of the central point of the alert region
+     * @param longitude     the longitude of the central point of the alert region
+     * @param radius        the radius of the central point of the alert region in meters
+     * @param expiration    expiration realtime for this proximity alert in milliseconds, or -1 to
+     *                      indicate no expiration
+     * @param pendingIntent a {@link PendingIntent} that will sent when entry to or exit from the
+     *                      alert region is detected
      * @throws SecurityException if {@link android.Manifest.permission#ACCESS_FINE_LOCATION}
      *                           permission is not present
      */
     @RequiresPermission(anyOf = {ACCESS_COARSE_LOCATION, ACCESS_FINE_LOCATION})
     public void addProximityAlert(double latitude, double longitude, float radius, long expiration,
-            @NonNull PendingIntent intent) {
-        Preconditions.checkArgument(intent != null, "invalid null pending intent");
-        if (Compatibility.isChangeEnabled(TARGETED_PENDING_INTENT)) {
-            Preconditions.checkArgument(intent.isTargetedToPackage(),
+            @NonNull PendingIntent pendingIntent) {
+        Preconditions.checkArgument(pendingIntent != null, "invalid null pending intent");
+
+        if (Compatibility.isChangeEnabled(BLOCK_UNTARGETED_PENDING_INTENTS)) {
+            Preconditions.checkArgument(pendingIntent.isTargetedToPackage(),
                     "pending intent must be targeted to a package");
         }
-        if (expiration < 0) expiration = Long.MAX_VALUE;
+
+        if (Compatibility.isChangeEnabled(BLOCK_IMMUTABLE_PENDING_INTENTS)) {
+            Preconditions.checkArgument(!pendingIntent.isImmutable(),
+                    "pending intent must be mutable");
+        }
+
+        if (expiration < 0) {
+            expiration = Long.MAX_VALUE;
+        }
 
         try {
             Geofence fence = Geofence.createCircle(latitude, longitude, radius, expiration);
-            mService.requestGeofence(fence, intent, mContext.getPackageName(),
+            mService.requestGeofence(fence, pendingIntent, mContext.getPackageName(),
                     mContext.getAttributionTag());
         } catch (RemoteException e) {
             throw e.rethrowFromSystemServer();
@@ -1941,7 +1964,7 @@ public class LocationManager {
     @Deprecated
     @RequiresPermission(ACCESS_FINE_LOCATION)
     public @Nullable GpsStatus getGpsStatus(@Nullable GpsStatus status) {
-        if (Compatibility.isChangeEnabled(GPS_STATUS_USAGE)) {
+        if (Compatibility.isChangeEnabled(BLOCK_GPS_STATUS_USAGE)) {
             throw new UnsupportedOperationException(
                     "GpsStatus APIs not supported, please use GnssStatus APIs instead");
         }
@@ -1976,7 +1999,7 @@ public class LocationManager {
     @Deprecated
     @RequiresPermission(ACCESS_FINE_LOCATION)
     public boolean addGpsStatusListener(GpsStatus.Listener listener) {
-        if (Compatibility.isChangeEnabled(GPS_STATUS_USAGE)) {
+        if (Compatibility.isChangeEnabled(BLOCK_GPS_STATUS_USAGE)) {
             throw new UnsupportedOperationException(
                     "GpsStatus APIs not supported, please use GnssStatus APIs instead");
         }
@@ -1996,7 +2019,7 @@ public class LocationManager {
      */
     @Deprecated
     public void removeGpsStatusListener(GpsStatus.Listener listener) {
-        if (Compatibility.isChangeEnabled(GPS_STATUS_USAGE)) {
+        if (Compatibility.isChangeEnabled(BLOCK_GPS_STATUS_USAGE)) {
             throw new UnsupportedOperationException(
                     "GpsStatus APIs not supported, please use GnssStatus APIs instead");
         }
diff --git a/services/core/java/com/android/server/location/LocationManagerService.java b/services/core/java/com/android/server/location/LocationManagerService.java
index 9c48d23dade6..6874f23b35ac 100644
--- a/services/core/java/com/android/server/location/LocationManagerService.java
+++ b/services/core/java/com/android/server/location/LocationManagerService.java
@@ -21,10 +21,10 @@ import static android.app.compat.CompatChanges.isChangeEnabled;
 import static android.content.pm.PackageManager.MATCH_DIRECT_BOOT_AWARE;
 import static android.content.pm.PackageManager.MATCH_SYSTEM_ONLY;
 import static android.content.pm.PackageManager.PERMISSION_GRANTED;
+import static android.location.LocationManager.BLOCK_PENDING_INTENT_SYSTEM_API_USAGE;
 import static android.location.LocationManager.FUSED_PROVIDER;
 import static android.location.LocationManager.GPS_PROVIDER;
 import static android.location.LocationManager.NETWORK_PROVIDER;
-import static android.location.LocationManager.PREVENT_PENDING_INTENT_SYSTEM_API_USAGE;
 import static android.location.LocationRequest.LOW_POWER_EXCEPTIONS;
 
 import static com.android.server.location.LocationPermissions.PERMISSION_COARSE;
@@ -597,14 +597,15 @@ public class LocationManagerService extends ILocationManager.Stub {
         // simplest to ensure these apis are simply never set for pending intent requests. the same
         // does not apply for listener requests since those will have the process (including the
         // listener) killed on permission removal
-        boolean usesSystemApi = request.isLowPower()
-                || request.isHiddenFromAppOps()
-                || request.isLocationSettingsIgnored()
-                || !request.getWorkSource().isEmpty();
-        if (usesSystemApi
-                && isChangeEnabled(PREVENT_PENDING_INTENT_SYSTEM_API_USAGE, identity.getUid())) {
-            throw new SecurityException(
-                    "PendingIntent location requests may not use system APIs: " + request);
+        if (isChangeEnabled(BLOCK_PENDING_INTENT_SYSTEM_API_USAGE, identity.getUid())) {
+            boolean usesSystemApi = request.isLowPower()
+                    || request.isHiddenFromAppOps()
+                    || request.isLocationSettingsIgnored()
+                    || !request.getWorkSource().isEmpty();
+            if (usesSystemApi) {
+                throw new SecurityException(
+                        "PendingIntent location requests may not use system APIs: " + request);
+            }
         }
 
         request = validateLocationRequest(request, identity);
diff --git a/services/core/java/com/android/server/location/geofence/GeofenceManager.java b/services/core/java/com/android/server/location/geofence/GeofenceManager.java
index 7a59cba02dd9..c23bd85505da 100644
--- a/services/core/java/com/android/server/location/geofence/GeofenceManager.java
+++ b/services/core/java/com/android/server/location/geofence/GeofenceManager.java
@@ -296,15 +296,15 @@ public class GeofenceManager extends
             @Nullable String attributionTag) {
         LocationPermissions.enforceCallingOrSelfLocationPermission(mContext, PERMISSION_FINE);
 
-        CallerIdentity callerIdentity = CallerIdentity.fromBinder(mContext, packageName,
+        CallerIdentity identity = CallerIdentity.fromBinder(mContext, packageName,
                 attributionTag, AppOpsManager.toReceiverId(pendingIntent));
 
-        final long identity = Binder.clearCallingIdentity();
+        final long ident = Binder.clearCallingIdentity();
         try {
             putRegistration(new GeofenceKey(pendingIntent, geofence),
-                    new GeofenceRegistration(geofence, callerIdentity, pendingIntent));
+                    new GeofenceRegistration(geofence, identity, pendingIntent));
         } finally {
-            Binder.restoreCallingIdentity(identity);
+            Binder.restoreCallingIdentity(ident);
         }
     }