Update PermissionChecker usages to avoid unnecessary attribution.

We had accidental usages of the PermissionChecker for cases where no
private data was provided to the app but the checkPermission API on
the latter also did blame data access on the app. The PermissionChecker
was designed to handle IPC calls and not for generic API checks.

To avoid future accidental incorrect PermissionChecker usages this
change renames the existing APIs of the latter to clearly indicate
that they should be used for data delivery and also adds sibling
methods for doing the same permission checks for preflight purposes.
Also the documentation is improved to furhter assist developers.

In addition, this change fixes accidental permission checker usages
that blame when they should not by using the new preflight flavor
of the permission check APIs.

Test:
    atest com.android.settingslib.location.RecentLocationAppsTest
    atest CtsPermissionTestCases
    added: LocationAccessCheckTest#notificationOnlyForAccessesSinceFeatureWasEnabled
    added: LocationAccessCheckTest#noNotificationIfFeatureDisabled
    added: LocationAccessCheckTest#noNotificationIfBlamerNotSystemOrLocationProvider
    added: LocationAccessCheckTest#testOpeningLocationSettingsDoesNotTriggerAccess

bug:141028068
Merged-In: I902a0ab049783a222ac2acdfac93ecd49f34dece
Change-Id: I902a0ab049783a222ac2acdfac93ecd49f34dece
(cherry picked from commit 625b69e00869d041b3114346965cd72a5d5c3c6a)

2 files changed, 63 insertions, 6 deletions

diff --git a/src/com/android/packageinstaller/Constants.java b/src/com/android/packageinstaller/Constants.java
index fa25bebc..38a2e205 100644
--- a/src/com/android/packageinstaller/Constants.java
+++ b/src/com/android/packageinstaller/Constants.java
@@ -74,6 +74,13 @@ public class Constants {
     public static final String PREFERENCES_FILE = "preferences";
 
     /**
+     * Key in the generic shared preferences that stores when the location access feature
+     * was enabled, specifically when it was picked up by the code managing the feature.
+     */
+    public static final String KEY_LOCATION_ACCESS_CHECK_ENABLED_TIME =
+            "location_access_check_enabled_time";
+
+    /**
      * Key in the generic shared preferences that stores when the last notification was shown by
      * {@link com.android.packageinstaller.permission.service.LocationAccessCheck}
      */
diff --git a/src/com/android/packageinstaller/permission/service/LocationAccessCheck.java b/src/com/android/packageinstaller/permission/service/LocationAccessCheck.java
index 727808bc..5d6425a3 100644
--- a/src/com/android/packageinstaller/permission/service/LocationAccessCheck.java
+++ b/src/com/android/packageinstaller/permission/service/LocationAccessCheck.java
@@ -18,7 +18,7 @@ package com.android.packageinstaller.permission.service;
 
 import static android.Manifest.permission.ACCESS_FINE_LOCATION;
 import static android.app.AppOpsManager.OPSTR_FINE_LOCATION;
-import static android.app.NotificationManager.IMPORTANCE_HIGH;
+import static android.app.NotificationManager.IMPORTANCE_LOW;
 import static android.app.PendingIntent.FLAG_ONE_SHOT;
 import static android.app.PendingIntent.FLAG_UPDATE_CURRENT;
 import static android.app.PendingIntent.getBroadcast;
@@ -42,6 +42,7 @@ import static android.provider.Settings.Secure.LOCATION_ACCESS_CHECK_INTERVAL_MI
 import static com.android.packageinstaller.Constants.EXTRA_SESSION_ID;
 import static com.android.packageinstaller.Constants.INVALID_SESSION_ID;
 import static com.android.packageinstaller.Constants.KEY_LAST_LOCATION_ACCESS_NOTIFICATION_SHOWN;
+import static com.android.packageinstaller.Constants.KEY_LOCATION_ACCESS_CHECK_ENABLED_TIME;
 import static com.android.packageinstaller.Constants.LOCATION_ACCESS_CHECK_ALREADY_NOTIFIED_FILE;
 import static com.android.packageinstaller.Constants.LOCATION_ACCESS_CHECK_JOB_ID;
 import static com.android.packageinstaller.Constants.LOCATION_ACCESS_CHECK_NOTIFICATION_ID;
@@ -57,7 +58,6 @@ import static com.android.packageinstaller.permission.utils.Utils.getParcelableE
 import static com.android.packageinstaller.permission.utils.Utils.getParentUserContext;
 import static com.android.packageinstaller.permission.utils.Utils.getStringExtraSafe;
 import static com.android.packageinstaller.permission.utils.Utils.getSystemServiceSafe;
-import static com.android.packageinstaller.permission.utils.Utils.isLocationAccessCheckEnabled;
 
 import static java.lang.System.currentTimeMillis;
 import static java.util.concurrent.TimeUnit.DAYS;
@@ -103,6 +103,7 @@ import androidx.core.util.Preconditions;
 import com.android.packageinstaller.PermissionControllerStatsLog;
 import com.android.packageinstaller.permission.model.AppPermissionGroup;
 import com.android.packageinstaller.permission.ui.AppPermissionActivity;
+import com.android.packageinstaller.permission.utils.Utils;
 import com.android.permissioncontroller.R;
 
 import java.io.BufferedReader;
@@ -297,7 +298,7 @@ public class LocationAccessCheck {
 
         NotificationChannel permissionReminderChannel = new NotificationChannel(
                 PERMISSION_REMINDER_CHANNEL_ID, mContext.getString(R.string.permission_reminders),
-                IMPORTANCE_HIGH);
+                IMPORTANCE_LOW);
         notificationManager.createNotificationChannel(permissionReminderChannel);
     }
 
@@ -338,7 +339,7 @@ public class LocationAccessCheck {
     @WorkerThread
     private void addLocationNotificationIfNeeded(@NonNull JobParameters params,
             @NonNull LocationAccessCheckJobService service) {
-        if (!isLocationAccessCheckEnabled()) {
+        if (!checkLocationAccessCheckEnabledAndUpdateEnabledTime()) {
             service.jobFinished(params, false);
             return;
         }
@@ -480,9 +481,25 @@ public class LocationAccessCheck {
             for (int opNum = 0; opNum < numOps; opNum++) {
                 OpEntry entry = packageOps.getOps().get(opNum);
 
-                if (entry.getLastAccessBackgroundTime(AppOpsManager.OP_FLAGS_ALL_TRUSTED) > 0) {
-                    pkgsWithLocationAccess.add(userPkg);
+                // To protect against OEM apps that accidentally blame app ops on other packages
+                // since they can hold the privileged UPDATE_APP_OPS_STATS permission for location
+                // access in the background we trust only the OS and the location providers. Note
+                // that this mitigation only handles usage of AppOpsManager#noteProxyOp and not
+                // direct usage of AppOpsManager#noteOp, i.e. handles bad blaming and not bad
+                // attribution.
+                String proxyPackageName = entry.getProxyPackageName();
+                if (proxyPackageName != null && !proxyPackageName.equals(OS_PKG)
+                        && !lm.isProviderPackage(proxyPackageName)) {
+                    continue;
+                }
 
+                // We show only bg accesses since the location access check feature was enabled
+                // to handle cases where the feature is remotely toggled since we don't want to
+                // notify for accesses before the feature was turned on.
+                long featureEnabledTime = getLocationAccessCheckEnabledTime();
+                if (featureEnabledTime >= 0 && entry.getLastAccessBackgroundTime(
+                        AppOpsManager.OP_FLAGS_ALL_TRUSTED) > featureEnabledTime) {
+                    pkgsWithLocationAccess.add(userPkg);
                     break;
                 }
             }
@@ -498,6 +515,39 @@ public class LocationAccessCheck {
     }
 
     /**
+     * Checks whether the location access check feature is enabled and updates the
+     * time when the feature was first enabled. If the feature is enabled and no
+     * enabled time persisted we persist the current time as the enabled time. If
+     * the feature is disabled and an enabled time is persisted we delete the
+     * persisted time.
+     *
+     * @return Whether the location access feature is enabled.
+     */
+    private boolean checkLocationAccessCheckEnabledAndUpdateEnabledTime() {
+        final long enabledTime = getLocationAccessCheckEnabledTime();
+        if (Utils.isLocationAccessCheckEnabled()) {
+            if (enabledTime <= 0) {
+                mSharedPrefs.edit().putLong(KEY_LOCATION_ACCESS_CHECK_ENABLED_TIME,
+                        currentTimeMillis()).commit();
+            }
+            return true;
+        } else {
+            if (enabledTime > 0) {
+                mSharedPrefs.edit().remove(KEY_LOCATION_ACCESS_CHECK_ENABLED_TIME)
+                        .commit();
+            }
+            return false;
+        }
+    }
+
+    /**
+     * @return The time the location access check was enabled, or 0 if not enabled.
+     */
+    private long getLocationAccessCheckEnabledTime() {
+        return mSharedPrefs.getLong(KEY_LOCATION_ACCESS_CHECK_ENABLED_TIME, 0);
+    }
+
+    /**
      * Create a notification reminding the user that a package used the location. From this
      * notification the user can directly go to the screen that allows to change the permission.
      *