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

Commit e2ba264d authored by TreeHugger Robot's avatar TreeHugger Robot Committed by Android (Google) Code Review
Browse files

Merge "Ensure pkg uid matches provided uid for device phone number check" into sc-qpr1-dev

parents 886d034f f4d8bd16
Loading
Loading
Loading
Loading
+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);
        }
    }
}
+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);