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

Commit 49e179cb authored by Hai Zhang's avatar Hai Zhang
Browse files

Fix allowlisting restricted permissions for secondary users on app 

upgrade.

restorePermissionState() runs for all users and drops any restricted
permission grant without an allowlist, however we may not have the
allowlist ready for other users when package manager notifies us of a
package install for one of the installed users, as the callback API
is (and should be) per-user. To be more specific about this issue:

The way this used to work is very fragile and relies on the
implementation detail that things happen in restorePermissionState()
with the following order:

1. Implicit permissions are considered new if there wasn't any
permission state for it.
2. Then permission restrictions are applied and since a new permission
state won't have any allowlist, the APPLY_RESTRICTION flag is added
and the permission is made revoked.
3. setInitialGrantForNewImplicitPermissionsLocked() is called after
permission restrictions are applied, and doesn't respect the
APPLY_RESTRICTION flag, so that the new implicit permission is
granted.

And if the allowlist is added in time before the next
restorePermissionState() call, the permission restriction mechanism
will see the allowlist and not revoke the implicit permission grant.

However since our callback API from package manager is per-user, and
we want to properly support adding instead of replacing the allowlist
for each user (the existing allowlist for each user can be different
so addition has to be done with get-add-set per-user), the allowlist
addition has to be per-user, and this would introduce a
restorePermissionState() call that's too early for the other users.

Thanks to the refactoring that happened in S, the permission state
objects are now per-UID instead of per-package, so that the updates
are actually being perform per-user now, and we just need to expose
the capability for restorePermissionState() to be per-user instead of
hard-coded to run for all users. Then we can prepare the permission
state and allowlist for each user independently and fix this issue.

Fixes: 191381905
Test: atest SplitPermissionTest --user-type secondary_user
Test: atest CtsPermission2TestCases
Test: atest CtsPermission2TestCases --user-type secondary_user
Test: atest CtsPermission3TestCases
Test: atest CtsPermission3TestCases --user-type secondary_user
Change-Id: I6973fd54281a3b15e5206db286eab8b918709a19
parent 80b02a06

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

+23 −14
Original line number Diff line number Diff line
@@ -2557,16 +2557,17 @@ public class PermissionManagerService extends IPermissionManager.Stub { 
     *     <li>During app update the state gets restored from the last version of the app</li>

     * </ul>

     *

     * <p>This restores the permission state for all users.

     *

     * @param pkg the package the permissions belong to

     * @param replace if the package is getting replaced (this might change the requested

     *                permissions of this package)

     * @param packageOfInterest If this is the name of {@code pkg} add extra logging

     * @param callback Result call back

     * @param filterUserId If not {@link UserHandle.USER_ALL}, only restore the permission state for

     *                     this particular user

     */

    private void restorePermissionState(@NonNull AndroidPackage pkg, boolean replace,

            @Nullable String packageOfInterest, @Nullable PermissionCallback callback) {

            @Nullable String packageOfInterest, @Nullable PermissionCallback callback,

            @UserIdInt int filterUserId) {

        // IMPORTANT: There are two types of permissions: install and runtime.

        // Install time permissions are granted when the app is installed to

        // all device users and users added in the future. Runtime permissions
@@ -2584,7 +2585,8 @@ public class PermissionManagerService extends IPermissionManager.Stub { 
            return;

        }



        final int[] userIds = getAllUserIds();

        final int[] userIds = filterUserId == UserHandle.USER_ALL ? getAllUserIds()

                : new int[] { filterUserId };



        boolean runtimePermissionsRevoked = false;

        int[] updatedUserIds = EMPTY_INT_ARRAY;
@@ -3874,7 +3876,8 @@ public class PermissionManagerService extends IPermissionManager.Stub { 


        if (updatePermissions) {

            // Update permission of this app to take into account the new allowlist state.

            restorePermissionState(pkg, false, pkg.getPackageName(), mDefaultPermissionCallback);

            restorePermissionState(pkg, false, pkg.getPackageName(), mDefaultPermissionCallback,

                    userId);



            // If this resulted in losing a permission we need to kill the app.

            if (oldGrantedRestrictedPermissions == null) {
@@ -4028,14 +4031,17 @@ public class PermissionManagerService extends IPermissionManager.Stub { 
     *

     * @param packageName The package that is updated

     * @param pkg The package that is updated, or {@code null} if package is deleted

     * @param filterUserId If not {@link UserHandle.USER_ALL}, only restore the permission state for

     *                     this particular user

     */

    private void updatePermissions(@NonNull String packageName, @Nullable AndroidPackage pkg) {

    private void updatePermissions(@NonNull String packageName, @Nullable AndroidPackage pkg,

                                   @UserIdInt int filterUserId) {

        // If the package is being deleted, update the permissions of all the apps

        final int flags =

                (pkg == null ? UPDATE_PERMISSIONS_ALL | UPDATE_PERMISSIONS_REPLACE_PKG

                        : UPDATE_PERMISSIONS_REPLACE_PKG);

        updatePermissions(

                packageName, pkg, getVolumeUuidForPackage(pkg), flags, mDefaultPermissionCallback);

        updatePermissions(packageName, pkg, getVolumeUuidForPackage(pkg), flags,

                mDefaultPermissionCallback, filterUserId);

    }



    /**
@@ -4057,7 +4063,8 @@ public class PermissionManagerService extends IPermissionManager.Stub { 
                    (fingerprintChanged

                            ? UPDATE_PERMISSIONS_REPLACE_PKG | UPDATE_PERMISSIONS_REPLACE_ALL

                            : 0);

            updatePermissions(null, null, volumeUuid, flags, mDefaultPermissionCallback);

            updatePermissions(null, null, volumeUuid, flags, mDefaultPermissionCallback,

                    UserHandle.USER_ALL);

        } finally {

            PackageManager.uncorkPackageInfoCache();

        }
@@ -4106,12 +4113,14 @@ public class PermissionManagerService extends IPermissionManager.Stub { 
     *                          all volumes

     * @param flags Control permission for which apps should be updated

     * @param callback Callback to call after permission changes

     * @param filterUserId If not {@link UserHandle.USER_ALL}, only restore the permission state for

     *                     this particular user

     */

    private void updatePermissions(final @Nullable String changingPkgName,

            final @Nullable AndroidPackage changingPkg,

            final @Nullable String replaceVolumeUuid,

            @UpdatePermissionFlags int flags,

            final @Nullable PermissionCallback callback) {

            final @Nullable PermissionCallback callback, @UserIdInt int filterUserId) {

        // TODO: Most of the methods exposing BasePermission internals [source package name,

        // etc..] shouldn't be needed. Instead, when we've parsed a permission that doesn't

        // have package settings, we should make note of it elsewhere [map between
@@ -4147,7 +4156,7 @@ public class PermissionManagerService extends IPermissionManager.Stub { 
                // Only replace for packages on requested volume

                final String volumeUuid = getVolumeUuidForPackage(pkg);

                final boolean replace = replaceAll && Objects.equals(replaceVolumeUuid, volumeUuid);

                restorePermissionState(pkg, replace, changingPkgName, callback);

                restorePermissionState(pkg, replace, changingPkgName, callback, filterUserId);

            });

        }
@@ -4156,7 +4165,7 @@ public class PermissionManagerService extends IPermissionManager.Stub { 
            final String volumeUuid = getVolumeUuidForPackage(changingPkg);

            final boolean replace = ((flags & UPDATE_PERMISSIONS_REPLACE_PKG) != 0)

                    && Objects.equals(replaceVolumeUuid, volumeUuid);

            restorePermissionState(changingPkg, replace, changingPkgName, callback);

            restorePermissionState(changingPkg, replace, changingPkgName, callback, filterUserId);

        }

        Trace.traceEnd(TRACE_TAG_PACKAGE_MANAGER);

    }
@@ -4824,7 +4833,7 @@ public class PermissionManagerService extends IPermissionManager.Stub { 
    private void onPackageInstalledInternal(@NonNull AndroidPackage pkg,

            @NonNull PermissionManagerServiceInternal.PackageInstalledParams params,

            @UserIdInt int userId) {

        updatePermissions(pkg.getPackageName(), pkg);

        updatePermissions(pkg.getPackageName(), pkg, userId);

        addAllowlistedRestrictedPermissionsInternal(pkg,

                params.getAllowlistedRestrictedPermissions(),

                FLAG_PERMISSION_WHITELIST_INSTALLER, userId);
@@ -4871,7 +4880,7 @@ public class PermissionManagerService extends IPermissionManager.Stub { 
            resetRuntimePermissionsInternal(pkg, userId);

            return;

        }

        updatePermissions(packageName, null);

        updatePermissions(packageName, null, userId);

        if (sharedUserPkgs.isEmpty()) {

            removeUidStateAndResetPackageInstallPermissionsFixed(appId, packageName, userId);

        } else {