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

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

Merge changes from topic "12933478" into pi-dev

* changes:
  Revoke permission on non-runtime -> runtime upgrade
  Ensure permissions are revoked on state changes
parents f2972ed9 60c41ae4
Loading
Loading
Loading
Loading
+21 −4
Original line number Diff line number Diff line
@@ -284,6 +284,7 @@ import com.android.internal.os.SomeArgs;
import com.android.internal.os.Zygote;
import com.android.internal.telephony.CarrierAppUtils;
import com.android.internal.util.ArrayUtils;
import com.android.internal.util.CollectionUtils;
import com.android.internal.util.ConcurrentUtils;
import com.android.internal.util.DumpUtils;
import com.android.internal.util.FastXmlSerializer;
@@ -11638,11 +11639,16 @@ public class PackageManagerService extends IPackageManager.Stub
                mPermissionManager.addAllPermissionGroups(pkg, chatty);
            }
            // If a permission has had its defining app changed, or it has had its protection
            // upgraded, we need to revoke apps that hold it
            final List<String> permissionsWithChangedDefinition;
            // Don't allow ephemeral applications to define new permissions.
            if ((scanFlags & SCAN_AS_INSTANT_APP) != 0) {
                permissionsWithChangedDefinition = null;
                Slog.w(TAG, "Permissions from package " + pkg.packageName
                        + " ignored: instant apps cannot define new permissions.");
            } else {
                permissionsWithChangedDefinition =
                        mPermissionManager.addAllPermissions(pkg, chatty);
            }
@@ -11687,7 +11693,10 @@ public class PackageManagerService extends IPackageManager.Stub
                }
            }
            if (oldPkg != null) {
            boolean hasOldPkg = oldPkg != null;
            boolean hasPermissionDefinitionChanges =
                    !CollectionUtils.isEmpty(permissionsWithChangedDefinition);
            if (hasOldPkg || hasPermissionDefinitionChanges) {
                // We need to call revokeRuntimePermissionsIfGroupChanged async as permission
                // revoke callbacks from this method might need to kill apps which need the
                // mPackages lock on a different thread. This would dead lock.
@@ -11698,9 +11707,17 @@ public class PackageManagerService extends IPackageManager.Stub
                // won't be granted yet, hence new packages are no problem.
                final ArrayList<String> allPackageNames = new ArrayList<>(mPackages.keySet());
                AsyncTask.execute(() ->
                AsyncTask.execute(() -> {
                    if (hasOldPkg) {
                        mPermissionManager.revokeRuntimePermissionsIfGroupChanged(pkg, oldPkg,
                                allPackageNames, mPermissionCallback));
                                allPackageNames, mPermissionCallback);
                    }
                    if (hasPermissionDefinitionChanges) {
                        mPermissionManager.revokeRuntimePermissionsIfPermissionDefinitionChanged(
                                permissionsWithChangedDefinition, allPackageNames,
                                mPermissionCallback);
                    }
                });
            }
        }
+19 −0
Original line number Diff line number Diff line
@@ -81,6 +81,8 @@ public final class BasePermission {

    final @PermissionType int type;

    private boolean mPermissionDefinitionChanged;

    String sourcePackageName;

    // TODO: Can we get rid of this? Seems we only use some signature info from the setting
@@ -133,6 +135,11 @@ public final class BasePermission {
    public Signature[] getSourceSignatures() {
        return sourcePackageSetting.getSignatures();
    }

    public boolean isPermissionDefinitionChanged() {
        return mPermissionDefinitionChanged;
    }

    public int getType() {
        return type;
    }
@@ -150,6 +157,10 @@ public final class BasePermission {
        this.sourcePackageSetting = sourcePackageSetting;
    }

    public void setPermissionDefinitionChanged(boolean shouldOverride) {
        mPermissionDefinitionChanged = shouldOverride;
    }

    public int[] computeGids(int userId) {
        if (perUser) {
            final int[] userGids = new int[gids.length];
@@ -286,6 +297,7 @@ public final class BasePermission {
            boolean chatty) {
        final PackageSettingBase pkgSetting = (PackageSettingBase) pkg.mExtras;
        // Allow system apps to redefine non-system permissions
        boolean ownerChanged = false;
        if (bp != null && !Objects.equals(bp.sourcePackageName, p.info.packageName)) {
            final boolean currentOwnerIsSystem = (bp.perm != null
                    && bp.perm.owner.isSystem());
@@ -301,6 +313,7 @@ public final class BasePermission {
                    String msg = "New decl " + p.owner + " of permission  "
                            + p.info.name + " is system; overriding " + bp.sourcePackageName;
                    PackageManagerService.reportSettingsProblem(Log.WARN, msg);
                    ownerChanged = true;
                    bp = null;
                }
            }
@@ -308,6 +321,7 @@ public final class BasePermission {
        if (bp == null) {
            bp = new BasePermission(p.info.name, p.info.packageName, TYPE_NORMAL);
        }
        boolean wasNonRuntime = !bp.isRuntime();
        StringBuilder r = null;
        if (bp.perm == null) {
            if (bp.sourcePackageName == null
@@ -351,6 +365,11 @@ public final class BasePermission {
        if (bp.perm == p) {
            bp.protectionLevel = p.info.protectionLevel;
        }
        if (bp.isRuntime() && (ownerChanged || wasNonRuntime)) {
            // If this is a runtime permission and the owner has changed, or this was a normal
            // permission, then permission state should be cleaned up
            bp.mPermissionDefinitionChanged = true;
        }
        if (PackageManagerService.DEBUG_PACKAGE_SCANNING && r != null) {
            Log.d(TAG, "  Permissions: " + r);
        }
+7 −2
Original line number Diff line number Diff line
@@ -107,13 +107,18 @@ public abstract class PermissionManagerInternal {
            @NonNull ArrayList<String> allPackageNames,
            @NonNull PermissionCallback permissionCallback);

    public abstract void revokeRuntimePermissionsIfPermissionDefinitionChanged(
            @NonNull List<String> permissionsToRevoke,
            @NonNull ArrayList<String> allPackageNames,
            @NonNull PermissionCallback permissionCallback);

    /**
     * Add all permissions in the given package.
     * <p>
     * NOTE: argument {@code groupTEMP} is temporary until mPermissionGroups is moved to
     * the permission settings.
     */
    public abstract void addAllPermissions(@NonNull PackageParser.Package pkg, boolean chatty);
    public abstract List<String> addAllPermissions(@NonNull PackageParser.Package pkg, boolean chatty);
    public abstract void addAllPermissionGroups(@NonNull PackageParser.Package pkg, boolean chatty);
    public abstract void removeAllPermissions(@NonNull PackageParser.Package pkg, boolean chatty);
    public abstract boolean addDynamicPermission(@NonNull PermissionInfo info, boolean async,
+87 −6
Original line number Diff line number Diff line
@@ -469,8 +469,74 @@ public class PermissionManagerService {
        }
    }

    private void addAllPermissions(PackageParser.Package pkg, boolean chatty) {
        final int N = pkg.permissions.size();
    /**
     * If permissions are upgraded to runtime, or their owner changes to the system, then any
     * granted permissions must be revoked.
     *
     * @param permissionsToRevoke A list of permission names to revoke
     * @param allPackageNames All package names
     * @param permissionCallback Callback for permission changed
     */
    private void revokeRuntimePermissionsIfPermissionDefinitionChanged(
            @NonNull List<String> permissionsToRevoke,
            @NonNull ArrayList<String> allPackageNames,
            @NonNull PermissionCallback permissionCallback) {

        final int[] userIds = mUserManagerInt.getUserIds();
        final int numPermissions = permissionsToRevoke.size();
        final int numUserIds = userIds.length;
        final int numPackages = allPackageNames.size();
        final int callingUid = Binder.getCallingUid();

        for (int permNum = 0; permNum < numPermissions; permNum++) {
            String permName = permissionsToRevoke.get(permNum);
            BasePermission bp = mSettings.getPermission(permName);
            if (bp == null || !bp.isRuntime()) {
                continue;
            }
            for (int userIdNum = 0; userIdNum < numUserIds; userIdNum++) {
                final int userId = userIds[userIdNum];
                for (int packageNum = 0; packageNum < numPackages; packageNum++) {
                    final String packageName = allPackageNames.get(packageNum);
                    final int uid = mPackageManagerInt.getPackageUid(packageName, 0, userId);
                    if (uid < Process.FIRST_APPLICATION_UID) {
                        // do not revoke from system apps
                        continue;
                    }
                    final int permissionState = checkPermission(permName, packageName,
                            callingUid, userId);
                    final int flags = getPermissionFlags(permName, packageName, callingUid,
                             userId);
                    final int flagMask = PackageManager.FLAG_PERMISSION_SYSTEM_FIXED
                            | PackageManager.FLAG_PERMISSION_POLICY_FIXED
                            | PackageManager.FLAG_PERMISSION_GRANTED_BY_DEFAULT;
                    if (permissionState == PackageManager.PERMISSION_GRANTED
                            && (flags & flagMask) == 0) {
                        EventLog.writeEvent(0x534e4554, "154505240", uid,
                                "Revoking permission " + permName + " from package "
                                        + packageName + " due to definition change");
                        EventLog.writeEvent(0x534e4554, "168319670", uid,
                                "Revoking permission " + permName + " from package "
                                        + packageName + " due to definition change");
                        Slog.e(TAG, "Revoking permission " + permName + " from package "
                                + packageName + " due to definition change");
                        try {
                            revokeRuntimePermission(permName, packageName,
                                    false, callingUid, userId, permissionCallback);
                        } catch (Exception e) {
                            Slog.e(TAG, "Could not revoke " + permName + " from "
                                    + packageName, e);
                        }
                    }
                }
            }
            bp.setPermissionDefinitionChanged(false);
        }
    }

    private List<String> addAllPermissions(PackageParser.Package pkg, boolean chatty) {
        final int N = ArrayUtils.size(pkg.permissions);
        ArrayList<String> definitionChangedPermissions = new ArrayList<>();
        for (int i=0; i<N; i++) {
            PackageParser.Permission p = pkg.permissions.get(i);

@@ -492,19 +558,24 @@ public class PermissionManagerService {
                    }
                }

                final BasePermission bp;
                if (p.tree) {
                    final BasePermission bp = BasePermission.createOrUpdate(
                    bp = BasePermission.createOrUpdate(
                            mSettings.getPermissionTreeLocked(p.info.name), p, pkg,
                            mSettings.getAllPermissionTreesLocked(), chatty);
                    mSettings.putPermissionTreeLocked(p.info.name, bp);
                } else {
                    final BasePermission bp = BasePermission.createOrUpdate(
                    bp = BasePermission.createOrUpdate(
                            mSettings.getPermissionLocked(p.info.name),
                            p, pkg, mSettings.getAllPermissionTreesLocked(), chatty);
                    mSettings.putPermissionLocked(p.info.name, bp);
                }
                if (bp.isPermissionDefinitionChanged()) {
                    definitionChangedPermissions.add(p.info.name);
                }
            }
        }
        return definitionChangedPermissions;
    }

    private void addAllPermissionGroups(PackageParser.Package pkg, boolean chatty) {
@@ -2064,9 +2135,19 @@ public class PermissionManagerService {
            PermissionManagerService.this.revokeRuntimePermissionsIfGroupChanged(newPackage,
                    oldPackage, allPackageNames, permissionCallback);
        }

        @Override
        public void revokeRuntimePermissionsIfPermissionDefinitionChanged(
                @NonNull List<String> permissionsToRevoke,
                @NonNull ArrayList<String> allPackageNames,
                @NonNull PermissionCallback permissionCallback) {
            PermissionManagerService.this.revokeRuntimePermissionsIfPermissionDefinitionChanged(
                    permissionsToRevoke, allPackageNames, permissionCallback);
        }

        @Override
        public void addAllPermissions(Package pkg, boolean chatty) {
            PermissionManagerService.this.addAllPermissions(pkg, chatty);
        public List<String> addAllPermissions(Package pkg, boolean chatty) {
            return PermissionManagerService.this.addAllPermissions(pkg, chatty);
        }
        @Override
        public void addAllPermissionGroups(Package pkg, boolean chatty) {