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

Commit 7380bf8d authored by Philip P. Moltmann's avatar Philip P. Moltmann
Browse files

Only kill when needed when changing permissions 

Only kill app when an app-op actually changed.

Do not call revokePermission if permission is already revoked. Even
when the permission is alrady revoked, calling this method will kill the
app the permission belongs to.

Also, apply all device policy state and then perist the state for the
whole app at one. Before we persisted after every individual permission
which caused permissions to be temporarily revoked and then re-granted.

Test: Set up device with device-owner, set up work-profile
Fixes: 126185337, 126326421
Change-Id: Id28caebd36d0c66e83ee6ab4299f0ccb3bc6b488
parent 3af151de

src/com/android/packageinstaller/permission/model/AppPermissionGroup.java

+69 −28
Original line number Diff line number Diff line
@@ -26,7 +26,6 @@ import static android.Manifest.permission.WRITE_EXTERNAL_STORAGE; 
import static android.app.AppOpsManager.MODE_ALLOWED;

import static android.app.AppOpsManager.MODE_FOREGROUND;

import static android.app.AppOpsManager.MODE_IGNORED;

import static android.content.pm.PackageManager.FLAG_PERMISSION_SYSTEM_FIXED;

import static android.content.pm.PackageManager.PERMISSION_GRANTED;



import android.app.ActivityManager;
@@ -628,6 +627,25 @@ public final class AppPermissionGroup implements Comparable<AppPermissionGroup> 
        return grantRuntimePermissions(fixedByTheUser, null);

    }



    /**

     * Set mode of an app-op if needed.

     *

     * @param op The op to set

     * @param uid The uid the app-op belongs top

     * @param mode The new mode

     *

     * @return {@code true} iff app-op was changed

     */

    private boolean setAppOpMode(@NonNull String op, int uid, int mode) {

        int currentMode = mAppOps.unsafeCheckOpRaw(op, uid, mPackageInfo.packageName);

        if (currentMode == mode) {

            return false;

        }



        mAppOps.setUidMode(op, uid, mode);

        return true;

    }



    /**

     * Allow the app op for a permission/uid.

     *
@@ -649,8 +667,12 @@ public final class AppPermissionGroup implements Comparable<AppPermissionGroup> 
     *

     * @param permission The permission which has an appOps that should be allowed

     * @param uid        The uid of the process the app op if for

     *

     * @return {@code true} iff app-op was changed

     */

    private void allowAppOp(Permission permission, int uid) {

    private boolean allowAppOp(Permission permission, int uid) {

        boolean wasChanged = false;



        if (permission.isBackgroundPermission()) {

            ArrayList<Permission> foregroundPermissions = permission.getForegroundPermissions();
@@ -658,7 +680,7 @@ public final class AppPermissionGroup implements Comparable<AppPermissionGroup> 
            for (int i = 0; i < numForegroundPermissions; i++) {

                Permission foregroundPermission = foregroundPermissions.get(i);

                if (foregroundPermission.isAppOpAllowed()) {

                    mAppOps.setUidMode(foregroundPermission.getAppOp(), uid, MODE_ALLOWED);

                    wasChanged |= setAppOpMode(foregroundPermission.getAppOp(), uid, MODE_ALLOWED);

                }

            }

        } else {
@@ -669,18 +691,20 @@ public final class AppPermissionGroup implements Comparable<AppPermissionGroup> 
                    // The app requested a permission that has a background permission but it did

                    // not request the background permission, hence it can never get background

                    // access

                    mAppOps.setUidMode(permission.getAppOp(), uid, MODE_FOREGROUND);

                    wasChanged = setAppOpMode(permission.getAppOp(), uid, MODE_FOREGROUND);

                } else {

                    if (backgroundPermission.isAppOpAllowed()) {

                        mAppOps.setUidMode(permission.getAppOp(), uid, MODE_ALLOWED);

                        wasChanged = setAppOpMode(permission.getAppOp(), uid, MODE_ALLOWED);

                    } else {

                        mAppOps.setUidMode(permission.getAppOp(), uid, MODE_FOREGROUND);

                        wasChanged = setAppOpMode(permission.getAppOp(), uid, MODE_FOREGROUND);

                    }

                }

            } else {

                mAppOps.setUidMode(permission.getAppOp(), uid, MODE_ALLOWED);

                wasChanged = setAppOpMode(permission.getAppOp(), uid, MODE_ALLOWED);

            }

        }



        return wasChanged;

    }



    /**
@@ -846,8 +870,12 @@ public final class AppPermissionGroup implements Comparable<AppPermissionGroup> 
     *

     * @param permission The permission which has an appOps that should be disallowed

     * @param uid        The uid of the process the app op if for

     *

     * @return {@code true} iff app-op was changed

     */

    private void disallowAppOp(Permission permission, int uid) {

    private boolean disallowAppOp(Permission permission, int uid) {

        boolean wasChanged = false;



        if (permission.isBackgroundPermission()) {

            ArrayList<Permission> foregroundPermissions = permission.getForegroundPermissions();
@@ -855,12 +883,15 @@ public final class AppPermissionGroup implements Comparable<AppPermissionGroup> 
            for (int i = 0; i < numForegroundPermissions; i++) {

                Permission foregroundPermission = foregroundPermissions.get(i);

                if (foregroundPermission.isAppOpAllowed()) {

                    mAppOps.setUidMode(foregroundPermission.getAppOp(), uid, MODE_FOREGROUND);

                    wasChanged |= setAppOpMode(foregroundPermission.getAppOp(), uid,

                            MODE_FOREGROUND);

                }

            }

        } else {

            mAppOps.setUidMode(permission.getAppOp(), uid, MODE_IGNORED);

            wasChanged = setAppOpMode(permission.getAppOp(), uid, MODE_IGNORED);

        }



        return wasChanged;

    }



    /**
@@ -1134,7 +1165,9 @@ public final class AppPermissionGroup implements Comparable<AppPermissionGroup> 
     *                                     app ops change. If this is set to {@code false} the

     *                                     caller has to make sure to kill the app if needed.

     */

    public void persistChanges(boolean mayKillBecauseOfAppOpsChange) {

    void persistChanges(boolean mayKillBecauseOfAppOpsChange) {

        int uid = mPackageInfo.applicationInfo.uid;



        int numPermissions = mPermissions.size();

        boolean shouldKillApp = false;

        boolean shouldUpdateStorage = false;
@@ -1147,10 +1180,15 @@ public final class AppPermissionGroup implements Comparable<AppPermissionGroup> 
                    mPackageManager.grantRuntimePermission(mPackageInfo.packageName,

                            permission.getName(), mUserHandle);

                } else {

                    boolean isCurrentlyGranted = mContext.checkPermission(permission.getName(), -1,

                            uid) == PERMISSION_GRANTED;



                    if (isCurrentlyGranted) {

                        mPackageManager.revokeRuntimePermission(mPackageInfo.packageName,

                                permission.getName(), mUserHandle);

                    }

                }

            }



            int flags = (permission.isUserSet() ? PackageManager.FLAG_PERMISSION_USER_SET : 0)

                    | (permission.isUserFixed() ? PackageManager.FLAG_PERMISSION_USER_FIXED : 0)
@@ -1171,16 +1209,14 @@ public final class AppPermissionGroup implements Comparable<AppPermissionGroup> 


            if (permission.affectsAppOp()) {

                if (!permission.isSystemFixed()) {

                    // Enabling/Disabling an app op may put the app in a situation in which it has

                    // a handle to state it shouldn't have, so we have to kill the app. This matches

                    // the revoke runtime permission behavior.

                    if (permission.isAppOpAllowed()) {

                        allowAppOp(permission, mPackageInfo.applicationInfo.uid);

                        shouldKillApp |= allowAppOp(permission, uid);

                    } else {

                        disallowAppOp(permission, mPackageInfo.applicationInfo.uid);

                        shouldKillApp |= disallowAppOp(permission, uid);

                    }



                    // Enabling/Disabling an app op may put the app in a situation in which it has a

                    // handle to state it shouldn't have, so we have to kill the app. This matches

                    // the revoke runtime permission behavior.

                    shouldKillApp = true;

                }

            }
@@ -1201,12 +1237,12 @@ public final class AppPermissionGroup implements Comparable<AppPermissionGroup> 
        // case, whenever any of the new split permissions are granted to an

        // app, we also grant them the legacy "Storage" permission.

        if (StorageManager.hasIsolatedStorage() && shouldUpdateStorage) {

            boolean audioGranted = mPackageManager.checkPermission(READ_MEDIA_AUDIO,

                    mPackageInfo.packageName) == PERMISSION_GRANTED;

            boolean videoGranted = mPackageManager.checkPermission(READ_MEDIA_VIDEO,

                    mPackageInfo.packageName) == PERMISSION_GRANTED;

            boolean imagesGranted = mPackageManager.checkPermission(READ_MEDIA_IMAGES,

                    mPackageInfo.packageName) == PERMISSION_GRANTED;

            boolean audioGranted = mContext.checkPermission(READ_MEDIA_AUDIO,

                    -1, uid) == PERMISSION_GRANTED;

            boolean videoGranted = mContext.checkPermission(READ_MEDIA_VIDEO,

                    -1, uid) == PERMISSION_GRANTED;

            boolean imagesGranted = mContext.checkPermission(READ_MEDIA_IMAGES,

                    -1, uid) == PERMISSION_GRANTED;



            if (!ArrayUtils.isEmpty(mPackageInfo.requestedPermissions)) {

                for (String permission : mPackageInfo.requestedPermissions) {
@@ -1216,6 +1252,10 @@ public final class AppPermissionGroup implements Comparable<AppPermissionGroup> 
                            mPackageManager.grantRuntimePermission(mPackageInfo.packageName,

                                    permission, mUserHandle);

                        } else {

                            boolean isCurrentlyGranted = mContext.checkPermission(permission, -1,

                                    uid) == PERMISSION_GRANTED;



                            if (isCurrentlyGranted) {

                                mPackageManager.revokeRuntimePermission(mPackageInfo.packageName,

                                        permission, mUserHandle);

                            }
@@ -1223,6 +1263,7 @@ public final class AppPermissionGroup implements Comparable<AppPermissionGroup> 
                    }

                }

            }

        }



        if (mayKillBecauseOfAppOpsChange && shouldKillApp) {

            killApp(KILL_REASON_APP_OP_CHANGE);

src/com/android/packageinstaller/permission/model/AppPermissions.java

+7 −3
Original line number Diff line number Diff line
@@ -179,18 +179,22 @@ public final class AppPermissions { 


    /**

     * If the changes to the permission groups were delayed, persist them now.

     *

     * @param mayKillBecauseOfAppOpsChange If the app may be killed if app ops change. If this is

     *                                     set to {@code false} the caller has to make sure to kill

     *                                     the app if needed.

     */

    public void persistChanges() {

    public void persistChanges(boolean mayKillBecauseOfAppOpsChange) {

        if (mDelayChanges) {

            int numGroups = mGroups.size();



            for (int i = 0; i < numGroups; i++) {

                AppPermissionGroup group = mGroups.get(i);

                group.persistChanges(true);

                group.persistChanges(mayKillBecauseOfAppOpsChange);



                AppPermissionGroup backgroundGroup = group.getBackgroundPermissions();

                if (backgroundGroup != null) {

                    backgroundGroup.persistChanges(true);

                    backgroundGroup.persistChanges(mayKillBecauseOfAppOpsChange);

                }

            }

        }

src/com/android/packageinstaller/permission/service/BackupHelper.java

+1 −3
Original line number Diff line number Diff line
@@ -17,7 +17,6 @@ 
package com.android.packageinstaller.permission.service;



import static android.content.Context.MODE_PRIVATE;

import static android.content.pm.PackageManager.FLAG_PERMISSION_GRANTED_BY_DEFAULT;

import static android.content.pm.PackageManager.FLAG_PERMISSION_POLICY_FIXED;

import static android.content.pm.PackageManager.FLAG_PERMISSION_SYSTEM_FIXED;

import static android.content.pm.PackageManager.GET_PERMISSIONS;
@@ -53,7 +52,6 @@ import org.xmlpull.v1.XmlPullParser; 
import org.xmlpull.v1.XmlPullParserException;

import org.xmlpull.v1.XmlSerializer;



import java.io.File;

import java.io.FileInputStream;

import java.io.IOException;

import java.io.OutputStream;
@@ -750,7 +748,7 @@ public class BackupHelper { 
                }

            }



            appPerms.persistChanges();

            appPerms.persistChanges(true);

        }

    }

}

src/com/android/packageinstaller/permission/service/PermissionControllerServiceImpl.java

+18 −10
Original line number Diff line number Diff line
@@ -286,7 +286,7 @@ public final class PermissionControllerServiceImpl extends PermissionControllerS 
        if (!doDryRun) {

            int numChangedApps = appsWithRevokedPerms.size();

            for (int i = 0; i < numChangedApps; i++) {

                appsWithRevokedPerms.get(i).persistChanges();

                appsWithRevokedPerms.get(i).persistChanges(true);

            }

        }
@@ -528,33 +528,41 @@ public final class PermissionControllerServiceImpl extends PermissionControllerS 
                Collections.singletonList(unexpandedPermission),

                callerPkgInfo.applicationInfo.targetSdkVersion);



        AppPermissions app = new AppPermissions(this, pkgInfo, false, null);



        int numPerms = expandedPermissions.size();

        for (int i = 0; i < numPerms; i++) {

            String perm = expandedPermissions.get(i);

            AppPermissionGroup group = AppPermissionGroup.create(this, pkgInfo, perm, true);

            String permName = expandedPermissions.get(i);

            AppPermissionGroup group = app.getGroupForPermission(permName);

            if (group == null || group.isSystemFixed()) {

                continue;

            }



            Permission perm = group.getPermission(permName);

            if (perm == null) {

                continue;

            }



            switch (grantState) {

                case PERMISSION_GRANT_STATE_GRANTED:

                    group.getPermission(perm).setPolicyFixed(true);

                    group.grantRuntimePermissions(false, new String[]{perm});

                    perm.setPolicyFixed(true);

                    group.grantRuntimePermissions(false, new String[]{permName});

                    break;

                case PERMISSION_GRANT_STATE_DENIED:

                    group.getPermission(perm).setPolicyFixed(true);

                    group.revokeRuntimePermissions(false, new String[]{perm});

                    perm.setPolicyFixed(true);

                    group.revokeRuntimePermissions(false, new String[]{permName});

                    break;

                case PERMISSION_GRANT_STATE_DEFAULT:

                    group.getPermission(perm).setPolicyFixed(false);

                    perm.setPolicyFixed(false);

                    break;

                default:

                    return false;

            }



            group.persistChanges(!callerPackageName.equals(packageName));

        }



        app.persistChanges(grantState == PERMISSION_GRANT_STATE_DENIED

                || !callerPackageName.equals(packageName));



        return true;

    }

}

src/com/android/packageinstaller/permission/ui/handheld/ReviewPermissionsFragment.java

+1 −1
Original line number Diff line number Diff line
@@ -211,7 +211,7 @@ public final class ReviewPermissionsFragment extends PreferenceFragmentCompat 
            }

        }



        mAppPermissions.persistChanges();

        mAppPermissions.persistChanges(true);

    }



    private void bindUi() {