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

Commit 160508ed authored by Hai Zhang's avatar Hai Zhang
Browse files

Properly fix allowlisting restricted permissions for secondary users 

on app upgrade.

More context is available in the original fix ag/15166603 which is
being reverted due to b/193206356. The root cause of the regression is
that unlike one may assume, we do keep a per-user permission state
even if an app is never installed in that user, however ag/15166603
broken that assumption because it's only initializing the permission
state for the installed users.

We actually never respected whether a package is installed for a user
when checking or mutating a permission state, and networking has kernel
code that relies on the user 0 permission state, so we have to keep the
original behavior at least in S.

So to properly fix the original b/191381905, we should:

1. Still limit the restorePermissionState() call in
setAllowlistedRestrictedPermissionsInternal() to the user that it's
operating upon, so that it's not accidentally calling
restorePermissionState() before the allowlist is set.

2. Make the onPackageInstalled()/onPackageUninstalled() callbacks
called only once so that we do one updatePermissions() for all users
to initialize the permission state for all of them. Actually the
install/uninstall request can only happen for either one user or
USER_ALL, so we can keep the current callback signature and just allow
USER_ALL in addition to normal users.

BYPASS_INCLUSIVE_LANGUAGE_REASON=Touching space around old API names.

Bug: 191381905
Bug: 193206356
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
Test: atest MixedManagedProfileOwnerTest#testAlwaysOnVpn
Change-Id: I6f314a65247c35f86ea4b8e8bf9f458c1332e29c
parent e38dffeb

services/core/java/com/android/server/pm/PackageManagerService.java

+12 −22
Original line number Diff line number Diff line
@@ -7955,12 +7955,9 @@ public class PackageManagerService extends IPackageManager.Stub 
                    } catch (PackageManagerException e) {

                        Slog.w(TAG, "updateAllSharedLibrariesLPw failed: ", e);

                    }

                    final int[] userIds = mUserManager.getUserIds();

                    for (final int userId : userIds) {

                    mPermissionManager.onPackageInstalled(pkg,

                            PermissionManagerServiceInternal.PackageInstalledParams.DEFAULT,

                                userId);

                    }

                            UserHandle.USER_ALL);

                    writeSettingsLPrTEMP();

                }

            } catch (PackageManagerException e) {
@@ -19213,12 +19210,7 @@ public class PackageManagerService extends IPackageManager.Stub 
                }

                final int autoRevokePermissionsMode = installArgs.autoRevokePermissionsMode;

                permissionParamsBuilder.setAutoRevokePermissionsMode(autoRevokePermissionsMode);

                for (int currentUserId : allUsersList) {

                    if (ps.getInstalled(currentUserId)) {

                        mPermissionManager.onPackageInstalled(pkg, permissionParamsBuilder.build(),

                                currentUserId);

                    }

                }

                mPermissionManager.onPackageInstalled(pkg, permissionParamsBuilder.build(), userId);

            }

            res.name = pkgName;

            res.uid = pkg.getUid();
@@ -21862,10 +21854,8 @@ public class PackageManagerService extends IPackageManager.Stub 
                        if (sharedUserPkgs == null) {

                            sharedUserPkgs = Collections.emptyList();

                        }

                        for (final int userId : allUserHandles) {

                        mPermissionManager.onPackageUninstalled(packageName, deletedPs.appId,

                                    deletedPs.pkg, sharedUserPkgs, userId);

                        }

                                deletedPs.pkg, sharedUserPkgs, UserHandle.USER_ALL);

                    }

                    clearPackagePreferredActivitiesLPw(

                            deletedPs.name, changedUsers, UserHandle.USER_ALL);
@@ -22082,11 +22072,12 @@ public class PackageManagerService extends IPackageManager.Stub 
                }

            }













            for (final int userId : allUserHandles) {















            // The method below will take care of removing obsolete permissions and granting















            // install permissions.















            mPermissionManager.onPackageInstalled(pkg,













                        PermissionManagerServiceInternal.PackageInstalledParams.DEFAULT, userId);













                    PermissionManagerServiceInternal.PackageInstalledParams.DEFAULT,













                    UserHandle.USER_ALL);













            for (final int userId : allUserHandles) {















                if (applyUserRestrictions) {















                    mSettings.writePermissionStateForUserLPr(userId, false);















                }










@@ -22409,10 +22400,9 @@ public class PackageManagerService extends IPackageManager.Stub













            }















            removeKeystoreDataIfNeeded(mInjector.getUserManagerInternal(), nextUserId, ps.appId);















            clearPackagePreferredActivities(ps.name, nextUserId);













            mPermissionManager.onPackageUninstalled(ps.name, ps.appId, pkg, sharedUserPkgs,













                    nextUserId);















            mDomainVerificationManager.clearPackageForUser(ps.name, nextUserId);















        }













        mPermissionManager.onPackageUninstalled(ps.name, ps.appId, pkg, sharedUserPkgs, userId);





























        if (outInfo != null) {















            if ((flags & PackageManager.DELETE_KEEP_DATA) == 0) {

























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



+57
−40




























Original line number
Diff line number
Diff line








@@ -2564,16 +2564,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









@@ -2591,7 +2592,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;










@@ -3883,7 +3885,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) {










@@ -4156,7 +4159,8 @@ 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,













                        UserHandle.USER_ALL);















            });















        }

























@@ -4165,7 +4169,8 @@ 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,













                    UserHandle.USER_ALL);















        }















        Trace.traceEnd(TRACE_TAG_PACKAGE_MANAGER);















    }










@@ -4832,8 +4837,9 @@ public class PermissionManagerService extends IPermissionManager.Stub {





























    private void onPackageInstalledInternal(@NonNull AndroidPackage pkg,















            @NonNull PermissionManagerServiceInternal.PackageInstalledParams params,













            @UserIdInt int userId) {













            @UserIdInt int[] userIds) {















        updatePermissions(pkg.getPackageName(), pkg);













        for (final int userId : userIds) {















            addAllowlistedRestrictedPermissionsInternal(pkg,















                    params.getAllowlistedRestrictedPermissions(),















                    FLAG_PERMISSION_WHITELIST_INSTALLER, userId);









@@ -4845,6 +4851,7 @@ public class PermissionManagerService extends IPermissionManager.Stub {













            }















            grantRequestedRuntimePermissionsInternal(pkg, params.getGrantedPermissions(), userId);















        }













    }































    private void addAllowlistedRestrictedPermissionsInternal(@NonNull AndroidPackage pkg,















            @NonNull List<String> allowlistedRestrictedPermissions,









@@ -4866,7 +4873,7 @@ public class PermissionManagerService extends IPermissionManager.Stub {





























    private void onPackageUninstalledInternal(@NonNull String packageName, int appId,















            @Nullable AndroidPackage pkg, @NonNull List<AndroidPackage> sharedUserPkgs,













            @UserIdInt int userId) {













            @UserIdInt int[] userIds) {















        // TODO: Move these checks to check PackageState to be more reliable.















        // System packages should always have an available APK.















        if (pkg != null && pkg.isSystem()









@@ -4877,10 +4884,13 @@ public class PermissionManagerService extends IPermissionManager.Stub {













            // If we are only marking a system package as uninstalled, we need to keep its















            // pregranted permission state so that it still works once it gets reinstalled, thus















            // only reset the user modifications to its permission state.













            for (final int userId : userIds) {















                resetRuntimePermissionsInternal(pkg, userId);













            }















            return;















        }















        updatePermissions(packageName, null);













        for (final int userId : userIds) {















            if (sharedUserPkgs.isEmpty()) {















                removeUidStateAndResetPackageInstallPermissionsFixed(appId, packageName, userId);















            } else {









@@ -4901,6 +4911,7 @@ public class PermissionManagerService extends IPermissionManager.Stub {













                }















            }















        }













    }































    @NonNull















    private List<LegacyPermission> getLegacyPermissions() {










@@ -5172,8 +5183,11 @@ public class PermissionManagerService extends IPermissionManager.Stub {













                @NonNull PackageInstalledParams params, @UserIdInt int userId) {















            Objects.requireNonNull(pkg, "pkg");















            Objects.requireNonNull(params, "params");













            Preconditions.checkArgumentNonNegative(userId, "userId");













            onPackageInstalledInternal(pkg, params, userId);













            Preconditions.checkArgument(userId >= UserHandle.USER_SYSTEM













                    || userId == UserHandle.USER_ALL, "userId");













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













                    : new int[] { userId };













            onPackageInstalledInternal(pkg, params, userIds);















        }































        @Override









@@ -5188,8 +5202,11 @@ public class PermissionManagerService extends IPermissionManager.Stub {













                @UserIdInt int userId) {















            Objects.requireNonNull(packageName, "packageName");















            Objects.requireNonNull(sharedUserPkgs, "sharedUserPkgs");













            Preconditions.checkArgumentNonNegative(userId, "userId");













            onPackageUninstalledInternal(packageName, appId, pkg, sharedUserPkgs, userId);













            Preconditions.checkArgument(userId >= UserHandle.USER_SYSTEM













                    || userId == UserHandle.USER_ALL, "userId");













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













                    : new int[] { userId };













            onPackageUninstalledInternal(packageName, appId, pkg, sharedUserPkgs, userIds);















        }































        @NonNull