Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d7c23516 authored by Hall Liu's avatar Hall Liu
Browse files

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
parent 2168be0e

services/core/java/com/android/server/TelephonyRegistry.java

+31 −4
Original line number Diff line number Diff line
@@ -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(() -> {

telephony/common/android/telephony/LocationAccessPolicy.java

+50 −4
Original line number Diff line number Diff line
@@ -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) {
@@ -143,6 +158,17 @@ public final class LocationAccessPolicy { 
                return this;

            }



            /**

             * 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.

             */
@@ -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);