Add useful app-op-note message to async Location deliveries

Test: atest CtsAppOpsTestCases Bug: 136505050 Change-Id: I1a34cd993132cc4fdf87c92625595240bb8ec4a7
2019-09-27 17:06:25 -07:00
parent e0d74cdf2e
commit bc8b48a97b
8 changed files with 118 additions and 60 deletions
--- a/location/java/android/location/ILocationManager.aidl
+++ b/location/java/android/location/ILocationManager.aidl
@@ -42,11 +42,11 @@ import com.android.internal.location.ProviderProperties;
 interface ILocationManager
 {
    void requestLocationUpdates(in LocationRequest request, in ILocationListener listener,
-            in PendingIntent intent, String packageName);
+            in PendingIntent intent, String packageName, String listenerIdentifier);
    void removeUpdates(in ILocationListener listener, in PendingIntent intent, String packageName);

    void requestGeofence(in LocationRequest request, in Geofence geofence,
-            in PendingIntent intent, String packageName);
+            in PendingIntent intent, String packageName, String listenerIdentifier);
    void removeGeofence(in Geofence fence, in PendingIntent intent, String packageName);

    Location getLastLocation(in LocationRequest request, String packageName);
@@ -64,22 +64,23 @@ interface ILocationManager

    boolean sendNiResponse(int notifId, int userResponse);

-    boolean addGnssMeasurementsListener(in IGnssMeasurementsListener listener, in String packageName);
+    boolean addGnssMeasurementsListener(in IGnssMeasurementsListener listener,
+             String packageName, String listenerIdentifier);
    void injectGnssMeasurementCorrections(in GnssMeasurementCorrections corrections,
            in String packageName);
    long getGnssCapabilities(in String packageName);
    void removeGnssMeasurementsListener(in IGnssMeasurementsListener listener);

-    boolean addGnssNavigationMessageListener(
-            in IGnssNavigationMessageListener listener,
-            in String packageName);
+    boolean addGnssNavigationMessageListener(in IGnssNavigationMessageListener listener,
+             String packageName, String listenerIdentifier);
    void removeGnssNavigationMessageListener(in IGnssNavigationMessageListener listener);

    int getGnssYearOfHardware();
    String getGnssHardwareModelName();

    int getGnssBatchSize(String packageName);
-    boolean addGnssBatchingCallback(in IBatchedLocationCallback callback, String packageName);
+    boolean addGnssBatchingCallback(in IBatchedLocationCallback callback, String packageName,
+             String listenerIdentifier);
    void removeGnssBatchingCallback();
    boolean startGnssBatch(long periodNanos, boolean wakeOnFifoFull, String packageName);
    void flushGnssBatch(String packageName);
--- a/location/java/android/location/LocationManager.java
+++ b/location/java/android/location/LocationManager.java
@@ -541,6 +541,23 @@ public class LocationManager {
        }
    }

+    /**
+     * Create a string that allows an app to identify a listener
+     *
+     * @param listener The listener
+     *
+     * @return A identifying string
+     */
+    private static String getListenerIdentifier(@NonNull Object listener) {
+        StringBuilder sb = new StringBuilder();
+
+        sb.append(listener.getClass().getName());
+        sb.append('@');
+        sb.append(Integer.toHexString(System.identityHashCode(listener)));
+
+        return sb.toString();
+    }
+
    /**
     * Register for a single location update using the named provider and
     * a callback.
@@ -981,7 +998,7 @@ public class LocationManager {
            boolean registered = false;
            try {
                mService.requestLocationUpdates(locationRequest, transport, null,
-                        mContext.getPackageName());
+                        mContext.getPackageName(), getListenerIdentifier(listener));
                registered = true;
            } catch (RemoteException e) {
                throw e.rethrowFromSystemServer();
@@ -1026,7 +1043,7 @@ public class LocationManager {

        try {
            mService.requestLocationUpdates(locationRequest, null, pendingIntent,
-                    mContext.getPackageName());
+                    mContext.getPackageName(), getListenerIdentifier(pendingIntent));
        } catch (RemoteException e) {
            throw e.rethrowFromSystemServer();
        }
@@ -1471,7 +1488,8 @@ public class LocationManager {
        Geofence fence = Geofence.createCircle(latitude, longitude, radius);
        LocationRequest request = new LocationRequest().setExpireIn(expiration);
        try {
-            mService.requestGeofence(request, fence, intent, mContext.getPackageName());
+            mService.requestGeofence(request, fence, intent, mContext.getPackageName(),
+                    getListenerIdentifier(intent));
        } catch (RemoteException e) {
            throw e.rethrowFromSystemServer();
        }
@@ -1548,7 +1566,8 @@ public class LocationManager {
        Preconditions.checkArgument(fence != null, "invalid null geofence");

        try {
-            mService.requestGeofence(request, fence, intent, mContext.getPackageName());
+            mService.requestGeofence(request, fence, intent, mContext.getPackageName(),
+                    getListenerIdentifier(intent));
        } catch (RemoteException e) {
            throw e.rethrowFromSystemServer();
        }
@@ -2538,7 +2557,7 @@ public class LocationManager {

            mListenerTransport = new GnssMeasurementsListener();
            return mService.addGnssMeasurementsListener(mListenerTransport,
-                    mContext.getPackageName());
+                    mContext.getPackageName(), "gnss measurement callback");
        }

        @Override
@@ -2574,7 +2593,7 @@ public class LocationManager {

            mListenerTransport = new GnssNavigationMessageListener();
            return mService.addGnssNavigationMessageListener(mListenerTransport,
-                    mContext.getPackageName());
+                    mContext.getPackageName(), "gnss navigation callback");
        }

        @Override
@@ -2609,7 +2628,8 @@ public class LocationManager {
            Preconditions.checkState(mListenerTransport == null);

            mListenerTransport = new BatchedLocationCallback();
-            return mService.addGnssBatchingCallback(mListenerTransport, mContext.getPackageName());
+            return mService.addGnssBatchingCallback(mListenerTransport, mContext.getPackageName(),
+                     "batched location callback");
        }

        @Override
--- a/services/core/java/com/android/server/GnssManagerService.java
+++ b/services/core/java/com/android/server/GnssManagerService.java
@@ -17,6 +17,7 @@
 package com.android.server;

 import android.Manifest;
+import android.annotation.NonNull;
 import android.app.ActivityManager;
 import android.app.AppOpsManager;
 import android.content.Context;
@@ -298,7 +299,8 @@ public class GnssManagerService {
     * @param packageName name of requesting package
     * @return true if callback is successfully added, false otherwise
     */
-    public boolean addGnssBatchingCallback(IBatchedLocationCallback callback, String packageName) {
+    public boolean addGnssBatchingCallback(IBatchedLocationCallback callback, String packageName,
+            @NonNull String listenerIdentity) {
        mContext.enforceCallingPermission(
                android.Manifest.permission.LOCATION_HARDWARE,
                "Location Hardware permission not granted to access hardware batching");
@@ -316,7 +318,8 @@ public class GnssManagerService {
        }

        CallerIdentity callerIdentity =
-                new CallerIdentity(Binder.getCallingUid(), Binder.getCallingPid(), packageName);
+                new CallerIdentity(Binder.getCallingUid(), Binder.getCallingPid(), packageName,
+                        listenerIdentity);
        synchronized (mGnssBatchingLock) {
            mGnssBatchingCallback = callback;
            mGnssBatchingDeathCallback =
@@ -494,7 +497,7 @@ public class GnssManagerService {
    private <TListener extends IInterface> boolean addGnssDataListenerLocked(
            TListener listener,
            String packageName,
-            String listenerName,
+            @NonNull String listenerIdentifier,
            RemoteListenerHelper<TListener> gnssDataProvider,
            ArrayMap<IBinder,
                    LinkedListener<TListener>> gnssDataListeners,
@@ -513,10 +516,11 @@ public class GnssManagerService {
        }

        CallerIdentity callerIdentity =
-                new CallerIdentity(Binder.getCallingUid(), Binder.getCallingPid(), packageName);
+                new CallerIdentity(Binder.getCallingUid(), Binder.getCallingPid(), packageName,
+                        listenerIdentifier);
        LinkedListener<TListener> linkedListener =
                new LocationManagerServiceUtils.LinkedListener<>(
-                        listener, listenerName, callerIdentity, binderDeathCallback);
+                        listener, listenerIdentifier, callerIdentity, binderDeathCallback);
        IBinder binder = listener.asBinder();
        if (!linkedListener.linkToListenerDeathNotificationLocked(binder)) {
            return false;
@@ -606,7 +610,7 @@ public class GnssManagerService {
            return addGnssDataListenerLocked(
                    listener,
                    packageName,
-                    "GnssStatusListener",
+                    "Gnss status",
                    mGnssStatusProvider,
                    mGnssStatusListeners,
                    this::unregisterGnssStatusCallback);
@@ -632,12 +636,13 @@ public class GnssManagerService {
     * @return true if listener is successfully added, false otherwise
     */
    public boolean addGnssMeasurementsListener(
-            IGnssMeasurementsListener listener, String packageName) {
+            IGnssMeasurementsListener listener, String packageName,
+            @NonNull String listenerIdentifier) {
        synchronized (mGnssMeasurementsListeners) {
            return addGnssDataListenerLocked(
                    listener,
                    packageName,
-                    "GnssMeasurementsListener",
+                    listenerIdentifier,
                    mGnssMeasurementsProvider,
                    mGnssMeasurementsListeners,
                    this::removeGnssMeasurementsListener);
@@ -689,12 +694,13 @@ public class GnssManagerService {
     * @return true if listener is successfully added, false otherwise
     */
    public boolean addGnssNavigationMessageListener(
-            IGnssNavigationMessageListener listener, String packageName) {
+            IGnssNavigationMessageListener listener, String packageName,
+            @NonNull String listenerIdentifier) {
        synchronized (mGnssNavigationMessageListeners) {
            return addGnssDataListenerLocked(
                    listener,
                    packageName,
-                    "GnssNavigationMessageListener",
+                    listenerIdentifier,
                    mGnssNavigationMessageProvider,
                    mGnssNavigationMessageListeners,
                    this::removeGnssNavigationMessageListener);
--- a/services/core/java/com/android/server/LocationManagerService.java
+++ b/services/core/java/com/android/server/LocationManagerService.java
@@ -1223,8 +1223,10 @@ public class LocationManagerService extends ILocationManager.Stub {
        PowerManager.WakeLock mWakeLock;

        private Receiver(ILocationListener listener, PendingIntent intent, int pid, int uid,
-                String packageName, WorkSource workSource, boolean hideFromAppOps) {
-            super(new CallerIdentity(uid, pid, packageName), "LocationListener");
+                String packageName, WorkSource workSource, boolean hideFromAppOps,
+                @NonNull String listenerIdentifier) {
+            super(new CallerIdentity(uid, pid, packageName, listenerIdentifier),
+                    "LocationListener");
            mListener = listener;
            mPendingIntent = intent;
            if (listener != null) {
@@ -1532,9 +1534,12 @@ public class LocationManagerService extends ILocationManager.Stub {
    }

    @Override
-    public boolean addGnssBatchingCallback(IBatchedLocationCallback callback, String packageName) {
+    public boolean addGnssBatchingCallback(IBatchedLocationCallback callback, String packageName,
+            String listenerIdentifier) {
+        Preconditions.checkNotNull(listenerIdentifier);
+
        return mGnssManagerService == null ? false : mGnssManagerService.addGnssBatchingCallback(
-                callback, packageName);
+                callback, packageName, listenerIdentifier);
    }

    @Override
@@ -1696,11 +1701,12 @@ public class LocationManagerService extends ILocationManager.Stub {
        }
    }

-    private boolean reportLocationAccessNoThrow(
-            int pid, int uid, String packageName, int allowedResolutionLevel) {
+    private boolean reportLocationAccessNoThrow(int pid, int uid, String packageName,
+            int allowedResolutionLevel, @Nullable String message) {
        int op = resolutionLevelToOp(allowedResolutionLevel);
        if (op >= 0) {
-            if (mAppOps.noteOpNoThrow(op, uid, packageName) != AppOpsManager.MODE_ALLOWED) {
+            if (mAppOps.noteOpNoThrow(op, uid, packageName, message)
+                    != AppOpsManager.MODE_ALLOWED) {
                return false;
            }
        }
@@ -2133,12 +2139,13 @@ public class LocationManagerService extends ILocationManager.Stub {

    @GuardedBy("mLock")
    private Receiver getReceiverLocked(ILocationListener listener, int pid, int uid,
-            String packageName, WorkSource workSource, boolean hideFromAppOps) {
+            String packageName, WorkSource workSource, boolean hideFromAppOps,
+            @NonNull String listenerIdentifier) {
        IBinder binder = listener.asBinder();
        Receiver receiver = mReceivers.get(binder);
        if (receiver == null) {
            receiver = new Receiver(listener, null, pid, uid, packageName, workSource,
-                    hideFromAppOps);
+                    hideFromAppOps, listenerIdentifier);
            if (!receiver.linkToListenerDeathNotificationLocked(
                    receiver.getListener().asBinder())) {
                return null;
@@ -2150,11 +2157,11 @@ public class LocationManagerService extends ILocationManager.Stub {

    @GuardedBy("mLock")
    private Receiver getReceiverLocked(PendingIntent intent, int pid, int uid, String packageName,
-            WorkSource workSource, boolean hideFromAppOps) {
+            WorkSource workSource, boolean hideFromAppOps, @NonNull String listenerIdentifier) {
        Receiver receiver = mReceivers.get(intent);
        if (receiver == null) {
            receiver = new Receiver(null, intent, pid, uid, packageName, workSource,
-                    hideFromAppOps);
+                    hideFromAppOps, listenerIdentifier);
            mReceivers.put(intent, receiver);
        }
        return receiver;
@@ -2216,7 +2223,9 @@ public class LocationManagerService extends ILocationManager.Stub {

    @Override
    public void requestLocationUpdates(LocationRequest request, ILocationListener listener,
-            PendingIntent intent, String packageName) {
+            PendingIntent intent, String packageName, String listenerIdentifier) {
+        Preconditions.checkNotNull(listenerIdentifier);
+
        synchronized (mLock) {
            if (request == null) request = DEFAULT_LOCATION_REQUEST;
            checkPackageName(packageName);
@@ -2271,10 +2280,10 @@ public class LocationManagerService extends ILocationManager.Stub {
                Receiver receiver;
                if (intent != null) {
                    receiver = getReceiverLocked(intent, pid, uid, packageName, workSource,
-                            hideFromAppOps);
+                            hideFromAppOps, listenerIdentifier);
                } else {
                    receiver = getReceiverLocked(listener, pid, uid, packageName, workSource,
-                            hideFromAppOps);
+                            hideFromAppOps, listenerIdentifier);
                }
                requestLocationUpdatesLocked(sanitizedRequest, receiver, uid, packageName);
            } finally {
@@ -2343,9 +2352,9 @@ public class LocationManagerService extends ILocationManager.Stub {
        synchronized (mLock) {
            Receiver receiver;
            if (intent != null) {
-                receiver = getReceiverLocked(intent, pid, uid, packageName, null, false);
+                receiver = getReceiverLocked(intent, pid, uid, packageName, null, false, "");
            } else {
-                receiver = getReceiverLocked(listener, pid, uid, packageName, null, false);
+                receiver = getReceiverLocked(listener, pid, uid, packageName, null, false, "");
            }

            long identity = Binder.clearCallingIdentity();
@@ -2464,8 +2473,8 @@ public class LocationManagerService extends ILocationManager.Stub {
                }
                // Don't report location access if there is no last location to deliver.
                if (lastLocation != null) {
-                    if (!reportLocationAccessNoThrow(
-                            pid, uid, packageName, allowedResolutionLevel)) {
+                    if (!reportLocationAccessNoThrow(pid, uid, packageName, allowedResolutionLevel,
+                            null)) {
                        if (D) {
                            Log.d(TAG, "not returning last loc for no op app: " + packageName);
                        }
@@ -2519,7 +2528,9 @@ public class LocationManagerService extends ILocationManager.Stub {

    @Override
    public void requestGeofence(LocationRequest request, Geofence geofence, PendingIntent intent,
-            String packageName) {
+            String packageName, String listenerIdentifier) {
+        Preconditions.checkNotNull(listenerIdentifier);
+
        if (request == null) request = DEFAULT_LOCATION_REQUEST;
        int allowedResolutionLevel = getCallerAllowedResolutionLevel();
        checkResolutionLevelIsSufficientForGeofenceUse(allowedResolutionLevel);
@@ -2564,9 +2575,8 @@ public class LocationManagerService extends ILocationManager.Stub {
                        mActivityManager.getPackageImportance(packageName));
            }

-            mGeofenceManager.addFence(sanitizedRequest, geofence, intent,
-                    allowedResolutionLevel,
-                    uid, packageName);
+            mGeofenceManager.addFence(sanitizedRequest, geofence, intent, allowedResolutionLevel,
+                    uid, packageName, listenerIdentifier);
        } finally {
            Binder.restoreCallingIdentity(identity);
        }
@@ -2613,10 +2623,13 @@ public class LocationManagerService extends ILocationManager.Stub {
    }

    @Override
-    public boolean addGnssMeasurementsListener(
-            IGnssMeasurementsListener listener, String packageName) {
+    public boolean addGnssMeasurementsListener(IGnssMeasurementsListener listener,
+            String packageName, String listenerIdentifier) {
+        Preconditions.checkNotNull(listenerIdentifier);
+
        return mGnssManagerService == null ? false
-                : mGnssManagerService.addGnssMeasurementsListener(listener, packageName);
+                : mGnssManagerService.addGnssMeasurementsListener(listener, packageName,
+                        listenerIdentifier);
    }

    @Override
@@ -2643,10 +2656,13 @@ public class LocationManagerService extends ILocationManager.Stub {
    }

    @Override
-    public boolean addGnssNavigationMessageListener(
-            IGnssNavigationMessageListener listener, String packageName) {
+    public boolean addGnssNavigationMessageListener(IGnssNavigationMessageListener listener,
+            String packageName, String listenerIdentifier) {
+        Preconditions.checkNotNull(listenerIdentifier);
+
        return mGnssManagerService == null ? false
-                : mGnssManagerService.addGnssNavigationMessageListener(listener, packageName);
+                : mGnssManagerService.addGnssNavigationMessageListener(listener, packageName,
+                        listenerIdentifier);
    }

    @Override
@@ -2943,7 +2959,8 @@ public class LocationManagerService extends ILocationManager.Stub {
                            receiver.mCallerIdentity.mPid,
                            receiver.mCallerIdentity.mUid,
                            receiver.mCallerIdentity.mPackageName,
-                            receiver.mAllowedResolutionLevel)) {
+                            receiver.mAllowedResolutionLevel,
+                            "Location sent to " + receiver.mCallerIdentity.mListenerIdentifier)) {
                        if (D) {
                            Log.d(TAG, "skipping loc update for no op app: "
                                    + receiver.mCallerIdentity.mPackageName);
--- a/services/core/java/com/android/server/location/CallerIdentity.java
+++ b/services/core/java/com/android/server/location/CallerIdentity.java
@@ -16,6 +16,8 @@

 package com.android.server.location;

+import android.annotation.NonNull;
+
 /**
 * Represents the calling process's uid, pid, and package name.
 */
@@ -23,10 +25,13 @@ public class CallerIdentity {
    public final int mUid;
    public final int mPid;
    public final String mPackageName;
+    public final @NonNull String mListenerIdentifier;

-    public CallerIdentity(int uid, int pid, String packageName) {
+    public CallerIdentity(int uid, int pid, String packageName,
+            @NonNull String listenerIdentifier) {
        mUid = uid;
        mPid = pid;
        mPackageName = packageName;
+        mListenerIdentifier = listenerIdentifier;
    }
 }
--- a/services/core/java/com/android/server/location/GeofenceManager.java
+++ b/services/core/java/com/android/server/location/GeofenceManager.java
@@ -16,6 +16,7 @@

 package com.android.server.location;

+import android.annotation.NonNull;
 import android.app.AppOpsManager;
 import android.app.PendingIntent;
 import android.content.ContentResolver;
@@ -151,14 +152,16 @@ public class GeofenceManager implements LocationListener, PendingIntent.OnFinish
    }

    public void addFence(LocationRequest request, Geofence geofence, PendingIntent intent,
-            int allowedResolutionLevel, int uid, String packageName) {
+            int allowedResolutionLevel, int uid, String packageName,
+            @NonNull String listenerIdentifier) {
        if (D) {
            Slog.d(TAG, "addFence: request=" + request + ", geofence=" + geofence
                    + ", intent=" + intent + ", uid=" + uid + ", packageName=" + packageName);
        }

        GeofenceState state = new GeofenceState(geofence,
-                request.getExpireAt(), allowedResolutionLevel, uid, packageName, intent);
+                request.getExpireAt(), allowedResolutionLevel, uid, packageName, listenerIdentifier,
+                intent);
        synchronized (mLock) {
            // first make sure it doesn't already exist
            for (int i = mFences.size() - 1; i >= 0; i--) {
@@ -301,7 +304,8 @@ public class GeofenceManager implements LocationListener, PendingIntent.OnFinish
                int op = LocationManagerService.resolutionLevelToOp(state.mAllowedResolutionLevel);
                if (op >= 0) {
                    if (mAppOps.noteOpNoThrow(AppOpsManager.OP_FINE_LOCATION, state.mUid,
-                            state.mPackageName) != AppOpsManager.MODE_ALLOWED) {
+                            state.mPackageName, state.mListenerIdentifier)
+                            != AppOpsManager.MODE_ALLOWED) {
                        if (D) {
                            Slog.d(TAG, "skipping geofence processing for no op app: "
                                    + state.mPackageName);
--- a/services/core/java/com/android/server/location/GeofenceState.java
+++ b/services/core/java/com/android/server/location/GeofenceState.java
@@ -17,6 +17,7 @@

 package com.android.server.location;

+import android.annotation.NonNull;
 import android.app.PendingIntent;
 import android.location.Geofence;
 import android.location.Location;
@@ -38,13 +39,14 @@ public class GeofenceState {
    public final int mAllowedResolutionLevel;
    public final int mUid;
    public final String mPackageName;
+    public final @NonNull String mListenerIdentifier;
    public final PendingIntent mIntent;

    int mState;  // current state
    double mDistanceToCenter;  // current distance to center of fence

-    public GeofenceState(Geofence fence, long expireAt,
-            int allowedResolutionLevel, int uid, String packageName, PendingIntent intent) {
+    public GeofenceState(Geofence fence, long expireAt, int allowedResolutionLevel, int uid,
+            String packageName, @NonNull String listenerIdentifier, PendingIntent intent) {
        mState = STATE_UNKNOWN;
        mDistanceToCenter = Double.MAX_VALUE;

@@ -53,6 +55,7 @@ public class GeofenceState {
        mAllowedResolutionLevel = allowedResolutionLevel;
        mUid = uid;
        mPackageName = packageName;
+        mListenerIdentifier = listenerIdentifier;
        mIntent = intent;

        mLocation = new Location("");
--- a/services/core/java/com/android/server/location/RemoteListenerHelper.java
+++ b/services/core/java/com/android/server/location/RemoteListenerHelper.java
@@ -182,7 +182,9 @@ public abstract class RemoteListenerHelper<TListener extends IInterface> {
        }

        return mAppOps.noteOpNoThrow(AppOpsManager.OP_FINE_LOCATION, callerIdentity.mUid,
-                callerIdentity.mPackageName) == AppOpsManager.MODE_ALLOWED;
+                callerIdentity.mPackageName,
+                "Location sent to " + callerIdentity.mListenerIdentifier)
+                == AppOpsManager.MODE_ALLOWED;
    }

    protected void logPermissionDisabledEventNotReported(String tag, String packageName,