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

Commit f4d8bd16 authored by Michael Groover's avatar Michael Groover
Browse files

Ensure pkg uid matches provided uid for device phone number check 

An app on the device is able to directly interact with any of the
services that accepts a package name and can return a protected
device resource (phone number or identifier). The app is then able
to pass the name of another package targeting pre-R and determine
whether the app is installed on the device based on whether the
service method throws an Exception or not. While the app is able
to pass another package's name to the service method, the service
method will still use Binder#getCallingUid for the check. To prevent
leaking information about packages installed on the device, this
commit adds an additional check to verify the provided uid matches
that of the package; if not, a SecurityException is thrown that
only contains the provided package name, along with the uid / pid
of the calling app.

Bug: 193441322
Bug: 193445182
Test: atest LegacyPermissionManagerServiceTest
Change-Id: If9353b7cb697bd78ab18775aee7723e984d3c1db
parent 76d42927

services/core/java/com/android/server/pm/permission/LegacyPermissionManagerService.java

+25 −0
Original line number Diff line number Diff line
@@ -30,6 +30,7 @@ import android.os.Process; 
import android.os.ServiceManager;

import android.os.UserHandle;

import android.permission.ILegacyPermissionManager;

import android.util.EventLog;

import android.util.Log;



import com.android.internal.annotations.VisibleForTesting;
@@ -187,10 +188,25 @@ public class LegacyPermissionManagerService extends ILegacyPermissionManager.Stu 
    private void verifyCallerCanCheckAccess(String packageName, String message, int pid, int uid) {

        // If the check is being requested by an app then only allow the app to query its own

        // access status.

        boolean reportError = false;

        int callingUid = mInjector.getCallingUid();

        int callingPid = mInjector.getCallingPid();

        if (UserHandle.getAppId(callingUid) >= Process.FIRST_APPLICATION_UID && (callingUid != uid

                || callingPid != pid)) {

            reportError = true;

        }

        // If the query is against an app on the device, then the check should only be allowed if

        // the provided uid matches that of the specified package.

        if (packageName != null && UserHandle.getAppId(uid) >= Process.FIRST_APPLICATION_UID) {

            int packageUid = mInjector.getPackageUidForUser(packageName, UserHandle.getUserId(uid));

            if (uid != packageUid) {

                EventLog.writeEvent(0x534e4554, "193441322",

                        UserHandle.getAppId(callingUid) >= Process.FIRST_APPLICATION_UID

                                ? callingUid : uid, "Package uid mismatch");

                reportError = true;

            }

        }

        if (reportError) {

            String response = String.format(

                    "Calling uid %d, pid %d cannot access for package %s (uid=%d, pid=%d): %s",

                    callingUid, callingPid, packageName, uid, pid, message);
@@ -385,12 +401,14 @@ public class LegacyPermissionManagerService extends ILegacyPermissionManager.Stu 
    @VisibleForTesting

    public static class Injector {

        private final Context mContext;

        private final PackageManagerInternal mPackageManagerInternal;



        /**

         * Public constructor that accepts a {@code context} within which to operate.

         */

        public Injector(@NonNull Context context) {

            mContext = context;

            mPackageManagerInternal = LocalServices.getService(PackageManagerInternal.class);

        }



        /**
@@ -453,5 +471,12 @@ public class LegacyPermissionManagerService extends ILegacyPermissionManager.Stu 
            return mContext.getPackageManager().getApplicationInfoAsUser(packageName, 0,

                    UserHandle.getUserHandleForUid(uid));

        }



        /**

         * Returns the uid for the specified {@code packageName} under the provided {@code userId}.

         */

        public int getPackageUidForUser(String packageName, int userId) {

            return mPackageManagerInternal.getPackageUid(packageName, 0, userId);

        }

    }

}

services/tests/servicestests/src/com/android/server/pm/permission/LegacyPermissionManagerServiceTest.java

+92 −9
Original line number Diff line number Diff line
@@ -125,7 +125,7 @@ public class LegacyPermissionManagerServiceTest { 
    public void checkDeviceIdentifierAccess_hasPrivilegedPermission_returnsGranted() {

        // Apps with the READ_PRIVILEGED_PHONE_STATE permission should have access to device

        // identifiers.

        setupCheckDeviceIdentifierAccessTest(SYSTEM_PID, SYSTEM_UID);

        setupCheckDeviceIdentifierAccessTest(SYSTEM_PID, SYSTEM_UID, APP_UID);

        when(mInjector.checkPermission(android.Manifest.permission.READ_PRIVILEGED_PHONE_STATE,

                APP_PID, APP_UID)).thenReturn(PackageManager.PERMISSION_GRANTED);
@@ -140,7 +140,7 @@ public class LegacyPermissionManagerServiceTest { 
    public void checkDeviceIdentifierAccess_hasAppOp_returnsGranted() {

        // Apps that have been granted the READ_DEVICE_IDENTIFIERS appop should have access to

        // device identifiers.

        setupCheckDeviceIdentifierAccessTest(SYSTEM_PID, SYSTEM_UID);

        setupCheckDeviceIdentifierAccessTest(SYSTEM_PID, SYSTEM_UID, APP_UID);

        when(mAppOpsManager.noteOpNoThrow(eq(AppOpsManager.OPSTR_READ_DEVICE_IDENTIFIERS),

                eq(APP_UID), eq(mPackageName), any(), any())).thenReturn(

                AppOpsManager.MODE_ALLOWED);
@@ -156,7 +156,7 @@ public class LegacyPermissionManagerServiceTest { 
    public void checkDeviceIdentifierAccess_hasDpmAccess_returnsGranted() {

        // Apps that pass a DevicePolicyManager device / profile owner check should have access to

        // device identifiers.

        setupCheckDeviceIdentifierAccessTest(SYSTEM_PID, SYSTEM_UID);

        setupCheckDeviceIdentifierAccessTest(SYSTEM_PID, SYSTEM_UID, APP_UID);

        when(mDevicePolicyManager.hasDeviceIdentifierAccess(mPackageName, APP_PID,

                APP_UID)).thenReturn(true);
@@ -236,7 +236,7 @@ public class LegacyPermissionManagerServiceTest { 
        // both the permission and the appop must be granted. If the permission is granted but the

        // appop is not then AppOpsManager#MODE_IGNORED should be returned to indicate that this

        // should be a silent failure.

        setupCheckPhoneNumberAccessTest(SYSTEM_PID, SYSTEM_UID);

        setupCheckPhoneNumberAccessTest(SYSTEM_PID, SYSTEM_UID, APP_UID);

        setPackageTargetSdk(Build.VERSION_CODES.Q);



        grantPermissionAndAppop(android.Manifest.permission.READ_PHONE_STATE, null);
@@ -256,7 +256,7 @@ public class LegacyPermissionManagerServiceTest { 
        // Apps targeting R+ with just the READ_PHONE_STATE permission granted should not have

        // access to the phone number; PERMISSION_DENIED should be returned both with and without

        // the appop granted since this check should be skipped for target SDK R+.

        setupCheckPhoneNumberAccessTest(SYSTEM_PID, SYSTEM_UID);

        setupCheckPhoneNumberAccessTest(SYSTEM_PID, SYSTEM_UID, APP_UID);



        grantPermissionAndAppop(android.Manifest.permission.READ_PHONE_STATE, null);

        int resultWithoutAppop = mLegacyPermissionManagerService.checkPhoneNumberAccess(
@@ -319,12 +319,79 @@ public class LegacyPermissionManagerServiceTest { 
        assertEquals(PackageManager.PERMISSION_GRANTED, resultWithAppop);

    }



    @Test

    public void checkPhoneNumberAccess_providedUidDoesNotMatchPackageUid_throwsException()

            throws Exception {

        // An app can directly interact with one of the services that accepts a package name and

        // returns a protected resource via a direct binder transact. This app could then provide

        // the name of another app that targets pre-R, then determine if the app is installed based

        // on whether the service throws an exception or not. While the app can provide the package

        // name of another app, it cannot specify the package uid which is passed to the

        // LegacyPermissionManager using Binder#getCallingUid. Ultimately this uid should then be

        // compared against the actual uid of the package to ensure information about packages

        // installed on the device is not leaked.

        setupCheckPhoneNumberAccessTest(SYSTEM_PID, SYSTEM_UID, APP_UID + 1);



        assertThrows(SecurityException.class,

                () -> mLegacyPermissionManagerService.checkPhoneNumberAccess(mPackageName,

                        CHECK_PHONE_NUMBER_MESSAGE, null, APP_PID, APP_UID));

    }



    @Test

    public void checkPhoneNumberAccess_nullPackageNameSystemUid_returnsGranted() throws Exception {

        // The platform can pass a null package name when checking if the platform itself has

        // access to the device phone number(s) / identifier(s). This test ensures if a null package

        // is provided, then the package uid check is skipped and the test is based on whether the

        // the provided uid / pid has been granted the privileged permission.

        setupCheckPhoneNumberAccessTest(SYSTEM_PID, SYSTEM_UID, -1);

        when(mInjector.checkPermission(android.Manifest.permission.READ_PRIVILEGED_PHONE_STATE,

                SYSTEM_PID, SYSTEM_UID)).thenReturn(PackageManager.PERMISSION_GRANTED);



        int result = mLegacyPermissionManagerService.checkPhoneNumberAccess(null,

                CHECK_PHONE_NUMBER_MESSAGE, null, SYSTEM_PID, SYSTEM_UID);



        assertEquals(PackageManager.PERMISSION_GRANTED, result);

    }



    @Test

    public void checkPhoneNumberAccess_systemUidMismatchPackageUid_returnsGranted()

            throws Exception {

        // When the platform is checking device phone number / identifier access checks for other

        // components on the platform, a uid less than the first application UID is provided; this

        // test verifies the package uid check is skipped and access is still granted with the

        // privileged permission.

        int telephonyUid = SYSTEM_UID + 1;

        int telephonyPid = SYSTEM_PID + 1;

        setupCheckPhoneNumberAccessTest(SYSTEM_PID, SYSTEM_UID, -1);

        when(mInjector.checkPermission(android.Manifest.permission.READ_PRIVILEGED_PHONE_STATE,

                telephonyPid, telephonyUid)).thenReturn(PackageManager.PERMISSION_GRANTED);



        int result = mLegacyPermissionManagerService.checkPhoneNumberAccess(mPackageName,

                CHECK_PHONE_NUMBER_MESSAGE, null, telephonyPid, telephonyUid);



        assertEquals(PackageManager.PERMISSION_GRANTED, result);

    }



    /**

     * Configures device identifier access tests to fail; tests verifying access should individually

     * set an access check to succeed to verify access when that condition is met.

     */

    private void setupCheckDeviceIdentifierAccessTest(int callingPid, int callingUid) {

        setupAccessTest(callingPid, callingUid);

        setupCheckDeviceIdentifierAccessTest(callingPid, callingUid, callingUid);

    }



    /**

     * Configures device identifier access tests to fail; tests verifying access should individually

     * set an access check to succeed to verify access when that condition is met.

     *

     * <p>To prevent leaking package information, access checks for package UIDs >= {@link

     * android.os.Process#FIRST_APPLICATION_UID} must ensure the provided uid matches the uid of

     * the package being checked; to ensure this check is successful, this method accepts the

     * {@code packageUid} to be used for the package being checked.

     */

    public void setupCheckDeviceIdentifierAccessTest(int callingPid, int callingUid,

            int packageUid) {

        setupAccessTest(callingPid, callingUid, packageUid);



        when(mDevicePolicyManager.hasDeviceIdentifierAccess(anyString(), anyInt(),

                anyInt())).thenReturn(false);
@@ -332,12 +399,27 @@ public class LegacyPermissionManagerServiceTest { 
                mDevicePolicyManager);

    }



    /**

     * Configures phone number access tests to fail; tests verifying access should individually

     * set an access check to succeed to verify access when that condition is set.

     *

     */

    private void setupCheckPhoneNumberAccessTest(int callingPid, int callingUid) throws Exception {

        setupCheckPhoneNumberAccessTest(callingPid, callingUid, callingUid);

    }



    /**

     * Configures phone number access tests to fail; tests verifying access should individually set

     * an access check to succeed to verify access when that condition is met.

     *

     * <p>To prevent leaking package information, access checks for package UIDs >= {@link

     * android.os.Process#FIRST_APPLICATION_UID} must ensure the provided uid matches the uid of

     * the package being checked; to ensure this check is successful, this method accepts the

     * {@code packageUid} to be used for the package being checked.

     */

    private void setupCheckPhoneNumberAccessTest(int callingPid, int callingUid) throws Exception {

        setupAccessTest(callingPid, callingUid);

    private void setupCheckPhoneNumberAccessTest(int callingPid, int callingUid, int packageUid)

            throws Exception {

        setupAccessTest(callingPid, callingUid, packageUid);

        setPackageTargetSdk(Build.VERSION_CODES.R);

    }
@@ -345,9 +427,10 @@ public class LegacyPermissionManagerServiceTest { 
     * Configures the common mocks for any access tests using the provided {@code callingPid}

     * and {@code callingUid}.

     */

    private void setupAccessTest(int callingPid, int callingUid) {

    private void setupAccessTest(int callingPid, int callingUid, int packageUid) {

        when(mInjector.getCallingPid()).thenReturn(callingPid);

        when(mInjector.getCallingUid()).thenReturn(callingUid);

        when(mInjector.getPackageUidForUser(anyString(), anyInt())).thenReturn(packageUid);



        when(mInjector.checkPermission(anyString(), anyInt(), anyInt())).thenReturn(

                PackageManager.PERMISSION_DENIED);