Make LocationAccessQuery's builder safer to use

Add new methods on LocationAccessQuery.Builder to make it harder to
screw up when using. Changes include:
* Mandatory specification of min sdk levels for enforcing both fine and
coarse permission
* Mandatory "are-you-sure" check when both fine and coarse are above the
base level (i.e. when there are api levels that are left unprotected)

Also adapt TelephonyRegistry's permission checks to follow these new
requirements on the builder.

Fixes: 157170257
Test: CTS, unit tests, manual
Change-Id: Ica8779dff4c671352b917a3ccc02cdd0e1c5856d

2 files changed, 81 insertions, 8 deletions

diff --git a/services/core/java/com/android/server/TelephonyRegistry.java b/services/core/java/com/android/server/TelephonyRegistry.java
index 913d1fb6713e..c99bf33162cb 100644
--- a/services/core/java/com/android/server/TelephonyRegistry.java
+++ b/services/core/java/com/android/server/TelephonyRegistry.java
@@ -2623,14 +2623,19 @@ public class TelephonyRegistry extends ITelephonyRegistry.Stub {
                 .setCallingUid(Binder.getCallingUid());
 
         boolean shouldCheckLocationPermissions = false;
-        if ((events & ENFORCE_COARSE_LOCATION_PERMISSION_MASK) != 0) {
-            locationQueryBuilder.setMinSdkVersionForCoarse(0);
-            shouldCheckLocationPermissions = true;
-        }
 
         if ((events & ENFORCE_FINE_LOCATION_PERMISSION_MASK) != 0) {
             // Everything that requires fine location started in Q. So far...
             locationQueryBuilder.setMinSdkVersionForFine(Build.VERSION_CODES.Q);
+            // If we're enforcing fine starting in Q, we also want to enforce coarse starting in Q.
+            locationQueryBuilder.setMinSdkVersionForCoarse(Build.VERSION_CODES.Q);
+            locationQueryBuilder.setMinSdkVersionForEnforcement(Build.VERSION_CODES.Q);
+            shouldCheckLocationPermissions = true;
+        }
+
+        if ((events & ENFORCE_COARSE_LOCATION_PERMISSION_MASK) != 0) {
+            locationQueryBuilder.setMinSdkVersionForCoarse(0);
+            locationQueryBuilder.setMinSdkVersionForEnforcement(0);
             shouldCheckLocationPermissions = true;
         }
 
@@ -2740,6 +2745,19 @@ public class TelephonyRegistry extends ITelephonyRegistry.Stub {
         }
     }
 
+    private boolean checkFineLocationAccess(Record r) {
+        return checkFineLocationAccess(r, Build.VERSION_CODES.BASE);
+    }
+
+    private boolean checkCoarseLocationAccess(Record r) {
+        return checkCoarseLocationAccess(r, Build.VERSION_CODES.BASE);
+    }
+
+    /**
+     * Note -- this method should only be used at the site of a permission check if you need to
+     * explicitly allow apps below a certain SDK level access regardless of location permissions.
+     * If you don't need app compat logic, use {@link #checkFineLocationAccess(Record)}.
+     */
     private boolean checkFineLocationAccess(Record r, int minSdk) {
         LocationAccessPolicy.LocationPermissionQuery query =
                 new LocationAccessPolicy.LocationPermissionQuery.Builder()
@@ -2749,6 +2767,8 @@ public class TelephonyRegistry extends ITelephonyRegistry.Stub {
                         .setMethod("TelephonyRegistry push")
                         .setLogAsInfo(true) // we don't need to log an error every time we push
                         .setMinSdkVersionForFine(minSdk)
+                        .setMinSdkVersionForCoarse(minSdk)
+                        .setMinSdkVersionForEnforcement(minSdk)
                         .build();
 
         return Binder.withCleanCallingIdentity(() -> {
@@ -2758,6 +2778,11 @@ public class TelephonyRegistry extends ITelephonyRegistry.Stub {
         });
     }
 
+    /**
+     * Note -- this method should only be used at the site of a permission check if you need to
+     * explicitly allow apps below a certain SDK level access regardless of location permissions.
+     * If you don't need app compat logic, use {@link #checkCoarseLocationAccess(Record)}.
+     */
     private boolean checkCoarseLocationAccess(Record r, int minSdk) {
         LocationAccessPolicy.LocationPermissionQuery query =
                 new LocationAccessPolicy.LocationPermissionQuery.Builder()
@@ -2767,6 +2792,8 @@ public class TelephonyRegistry extends ITelephonyRegistry.Stub {
                         .setMethod("TelephonyRegistry push")
                         .setLogAsInfo(true) // we don't need to log an error every time we push
                         .setMinSdkVersionForCoarse(minSdk)
+                        .setMinSdkVersionForFine(Integer.MAX_VALUE)
+                        .setMinSdkVersionForEnforcement(minSdk)
                         .build();
 
         return Binder.withCleanCallingIdentity(() -> {
diff --git a/telephony/common/android/telephony/LocationAccessPolicy.java b/telephony/common/android/telephony/LocationAccessPolicy.java
index 3048ad7c1fb0..7d3fde61caa8 100644
--- a/telephony/common/android/telephony/LocationAccessPolicy.java
+++ b/telephony/common/android/telephony/LocationAccessPolicy.java
@@ -86,8 +86,9 @@ public final class LocationAccessPolicy {
             private String mCallingFeatureId;
             private int mCallingUid;
             private int mCallingPid;
-            private int mMinSdkVersionForCoarse = Integer.MAX_VALUE;
-            private int mMinSdkVersionForFine = Integer.MAX_VALUE;
+            private int mMinSdkVersionForCoarse = -1;
+            private int mMinSdkVersionForFine = -1;
+            private int mMinSdkVersionForEnforcement = -1;
             private boolean mLogAsInfo = false;
             private String mMethod;
 
@@ -125,7 +126,14 @@ public final class LocationAccessPolicy {
 
             /**
              * Apps that target at least this sdk version will be checked for coarse location
-             * permission. Defaults to INT_MAX (which means don't check)
+             * permission. This method MUST be called before calling {@link #build()}. Otherwise, an
+             * {@link IllegalArgumentException} will be thrown.
+             *
+             * Additionally, if both the argument to this method and
+             * {@link #setMinSdkVersionForFine} are greater than {@link Build.VERSION_CODES#BASE},
+             * you must call {@link #setMinSdkVersionForEnforcement} with the min of the two to
+             * affirm that you do not want any location checks below a certain SDK version.
+             * Otherwise, {@link #build} will throw an {@link IllegalArgumentException}.
              */
             public Builder setMinSdkVersionForCoarse(
                     int minSdkVersionForCoarse) {
@@ -135,7 +143,14 @@ public final class LocationAccessPolicy {
 
             /**
              * Apps that target at least this sdk version will be checked for fine location
-             * permission. Defaults to INT_MAX (which means don't check)
+             * permission.  This method MUST be called before calling {@link #build()}.
+             * Otherwise, an {@link IllegalArgumentException} will be thrown.
+             *
+             * Additionally, if both the argument to this method and
+             * {@link #setMinSdkVersionForCoarse} are greater than {@link Build.VERSION_CODES#BASE},
+             * you must call {@link #setMinSdkVersionForEnforcement} with the min of the two to
+             * affirm that you do not want any location checks below a certain SDK version.
+             * Otherwise, {@link #build} will throw an {@link IllegalArgumentException}.
              */
             public Builder setMinSdkVersionForFine(
                     int minSdkVersionForFine) {
@@ -144,6 +159,17 @@ public final class LocationAccessPolicy {
             }
 
             /**
+             * If both the argument to {@link #setMinSdkVersionForFine} and
+             * {@link #setMinSdkVersionForCoarse} are greater than {@link Build.VERSION_CODES#BASE},
+             * this method must be called with the min of the two to
+             * affirm that you do not want any location checks below a certain SDK version.
+             */
+            public Builder setMinSdkVersionForEnforcement(int minSdkVersionForEnforcement) {
+                mMinSdkVersionForEnforcement = minSdkVersionForEnforcement;
+                return this;
+            }
+
+            /**
              * Optional, for logging purposes only.
              */
             public Builder setMethod(String method) {
@@ -161,6 +187,26 @@ public final class LocationAccessPolicy {
 
             /** build LocationPermissionQuery */
             public LocationPermissionQuery build() {
+                if (mMinSdkVersionForCoarse < 0 || mMinSdkVersionForFine < 0) {
+                    throw new IllegalArgumentException("Must specify min sdk versions for"
+                            + " enforcement for both coarse and fine permissions");
+                }
+                if (mMinSdkVersionForFine > Build.VERSION_CODES.BASE
+                        && mMinSdkVersionForCoarse > Build.VERSION_CODES.BASE) {
+                    if (mMinSdkVersionForEnforcement != Math.min(
+                            mMinSdkVersionForCoarse, mMinSdkVersionForFine)) {
+                        throw new IllegalArgumentException("setMinSdkVersionForEnforcement must be"
+                                + " called.");
+                    }
+                }
+
+                if (mMinSdkVersionForFine < mMinSdkVersionForCoarse) {
+                    throw new IllegalArgumentException("Since fine location permission includes"
+                            + " access to coarse location, the min sdk level for enforcement of"
+                            + " the fine location permission must not be less than the min sdk"
+                            + " level for enforcement of the coarse location permission.");
+                }
+
                 return new LocationPermissionQuery(mCallingPackage, mCallingFeatureId,
                         mCallingUid, mCallingPid, mMinSdkVersionForCoarse, mMinSdkVersionForFine,
                         mLogAsInfo, mMethod);