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

Commit 0b41c894 authored by Svet Ganov's avatar Svet Ganov Committed by Svetoslav Ganov
Browse files

[DO NOT MERGE] Don't drop restricted permissions on upgrade

Restricted permissions cannot be held until whitelisted. In
a P -> Q upgrade we grandfather all restricted permissions.
However, the whitelisting code runs after the internal update
of permission happens for the first time resulting in a
revocation of the restricted permissions we were about to
grandfather.

The fix is to not deal with restricted permission when updating
the permissions state until the permission controller has run
the grandfathering logic and once the latter happens we do run
the permission update logic again to properly handle the
restricted permissions.

Bug: 138263882

Test: atest CtsPermissionTestCases
      atest CtsPermission2TestCases
      atest CtsAppSecurityHostTestCases:android.appsecurity.cts.PermissionsHostTest
      P -> Q upgrade preserves grandfathered restricted permissions
      P -> Bad Q build -> Q fixes up broken fixed restricted permissions

Change-Id: Iaef80426bf50181df93d1380af1d0855340def8e
parent 000415be
Loading
Loading
Loading
Loading
+13 −0
Original line number Diff line number Diff line
@@ -319,6 +319,8 @@ import com.android.server.pm.permission.PermissionManagerService;
import com.android.server.pm.permission.PermissionManagerServiceInternal;
import com.android.server.pm.permission.PermissionManagerServiceInternal.PermissionCallback;
import com.android.server.pm.permission.PermissionsState;
import com.android.server.policy.PermissionPolicyInternal;
import com.android.server.policy.PermissionPolicyInternal.OnInitializedCallback;
import com.android.server.security.VerityUtils;
import com.android.server.storage.DeviceStorageMonitorInternal;
import com.android.server.wm.ActivityTaskManagerInternal;
@@ -21633,6 +21635,17 @@ public class PackageManagerService extends IPackageManager.Stub
            mPermissionManager.updateAllPermissions(
                    StorageManager.UUID_PRIVATE_INTERNAL, false, mPackages.values(),
                    mPermissionCallback);
            final PermissionPolicyInternal permissionPolicyInternal =
                    LocalServices.getService(PermissionPolicyInternal.class);
            permissionPolicyInternal.setOnInitializedCallback(userId -> {
                // The SDK updated case is already handled when we run during the ctor.
                synchronized (mPackages) {
                    mPermissionManager.updateAllPermissions(
                            StorageManager.UUID_PRIVATE_INTERNAL, false /*sdkUpdated*/,
                            mPackages.values(), mPermissionCallback);
                }
            });
        }
        // Watch for external volumes that come and go over time
+15 −1
Original line number Diff line number Diff line
@@ -1170,6 +1170,11 @@ public final class DefaultPermissionGrantPolicy {
                final int flags = mContext.getPackageManager().getPermissionFlags(
                        permission, pkg.packageName, user);

                // If we are trying to grant as system fixed and already system fixed
                // then the system can change the system fixed grant state.
                final boolean changingGrantForSystemFixed = systemFixed
                        && (flags & PackageManager.FLAG_PERMISSION_SYSTEM_FIXED) != 0;

                // Certain flags imply that the permission's current state by the system or
                // device/profile owner or the user. In these cases we do not want to clobber the
                // current state.
@@ -1177,7 +1182,8 @@ public final class DefaultPermissionGrantPolicy {
                // Unless the caller wants to override user choices. The override is
                // to make sure we can grant the needed permission to the default
                // sms and phone apps after the user chooses this in the UI.
                if (!isFixedOrUserSet(flags) || ignoreSystemPackage) {
                if (!isFixedOrUserSet(flags) || ignoreSystemPackage
                        || changingGrantForSystemFixed) {
                    // Never clobber policy fixed permissions.
                    // We must allow the grant of a system-fixed permission because
                    // system-fixed is sticky, but the permission itself may be revoked.
@@ -1196,6 +1202,14 @@ public final class DefaultPermissionGrantPolicy {
                                PackageManager.FLAG_PERMISSION_RESTRICTION_SYSTEM_EXEMPT, user);
                    }

                    // If the system tries to change a system fixed permission from one fixed
                    // state to another we need to drop the fixed flag to allow the grant.
                    if (changingGrantForSystemFixed) {
                        mContext.getPackageManager().updatePermissionFlags(permission,
                                pkg.packageName, flags,
                                flags & ~PackageManager.FLAG_PERMISSION_SYSTEM_FIXED, user);
                    }

                    if (pm.checkPermission(permission, pkg.packageName)
                            != PackageManager.PERMISSION_GRANTED) {
                        mContext.getPackageManager()
+51 −32
Original line number Diff line number Diff line
@@ -97,6 +97,7 @@ import com.android.server.pm.SharedUserSetting;
import com.android.server.pm.UserManagerService;
import com.android.server.pm.permission.PermissionManagerServiceInternal.PermissionCallback;
import com.android.server.pm.permission.PermissionsState.PermissionState;
import com.android.server.policy.PermissionPolicyInternal;
import com.android.server.policy.SoftRestrictedPermissionPolicy;

import libcore.util.EmptyArray;
@@ -197,6 +198,9 @@ public class PermissionManagerService {
    @GuardedBy("mLock")
    private boolean mSystemReady;

    @GuardedBy("mLock")
    private PermissionPolicyInternal mPermissionPolicyInternal;

    /**
     * For each foreground/background permission the mapping:
     * Background permission -> foreground permissions
@@ -1080,6 +1084,13 @@ public class PermissionManagerService {
                            boolean softRestricted = bp.isSoftRestricted();

                            for (int userId : currentUserIds) {
                                // If permission policy is not ready we don't deal with restricted
                                // permissions as the policy may whitelist some permissions. Once
                                // the policy is initialized we would re-evaluate permissions.
                                final boolean permissionPolicyInitialized =
                                        mPermissionPolicyInternal != null
                                                && mPermissionPolicyInternal.isInitialized(userId);

                                PermissionState permState = origPermissions
                                        .getRuntimePermissionState(perm, userId);
                                int flags = permState != null ? permState.getFlags() : 0;
@@ -1094,7 +1105,7 @@ public class PermissionManagerService {

                                if (appSupportsRuntimePermissions) {
                                    // If hard restricted we don't allow holding it
                                    if (hardRestricted) {
                                    if (permissionPolicyInitialized && hardRestricted) {
                                        if (!restrictionExempt) {
                                            if (permState != null && permState.isGranted()
                                                    && permissionsState.revokeRuntimePermission(
@@ -1107,7 +1118,7 @@ public class PermissionManagerService {
                                            }
                                        }
                                    // If soft restricted we allow holding in a restricted form
                                    } else if (softRestricted) {
                                    } else if (permissionPolicyInitialized && softRestricted) {
                                        // Regardless if granted set the restriction flag as it
                                        // may affect app treatment based on this permission.
                                        if (!restrictionExempt && !restrictionApplied) {
@@ -1126,7 +1137,8 @@ public class PermissionManagerService {
                                        flags &= ~FLAG_PERMISSION_REVOKE_ON_UPGRADE;
                                        wasChanged = true;
                                    // Hard restricted permissions cannot be held.
                                    } else if (!hardRestricted || restrictionExempt) {
                                    } else if (!permissionPolicyInitialized
                                            || (!hardRestricted || restrictionExempt)) {
                                        if (permState != null && permState.isGranted()) {
                                            if (permissionsState.grantRuntimePermission(bp, userId)
                                                    == PERMISSION_OPERATION_FAILURE) {
@@ -1155,7 +1167,8 @@ public class PermissionManagerService {

                                    // If legacy app always grant the permission but if restricted
                                    // and not exempt take a note a restriction should be applied.
                                    if ((hardRestricted || softRestricted)
                                    if (permissionPolicyInitialized
                                            && (hardRestricted || softRestricted)
                                                    && !restrictionExempt && !restrictionApplied) {
                                        flags |= FLAG_PERMISSION_APPLY_RESTRICTION;
                                        wasChanged = true;
@@ -1163,23 +1176,17 @@ public class PermissionManagerService {
                                }

                                // If unrestricted or restriction exempt, don't apply restriction.
                                if (permissionPolicyInitialized) {
                                    if (!(hardRestricted || softRestricted) || restrictionExempt) {
                                        if (restrictionApplied) {
                                            flags &= ~FLAG_PERMISSION_APPLY_RESTRICTION;
                                        // Dropping restriction on a legacy app requires a review.
                                            // Dropping restriction on a legacy app implies a review
                                            if (!appSupportsRuntimePermissions) {
                                                flags |= FLAG_PERMISSION_REVIEW_REQUIRED;
                                            }
                                            wasChanged = true;
                                        }
                                    }

                                if (hardRestricted && !restrictionExempt
                                        && (flags & FLAG_PERMISSION_SYSTEM_FIXED) != 0) {
                                    // Applying a hard restriction implies revoking it. This might
                                    // lead to a system-fixed, revoked permission.
                                    flags &= ~FLAG_PERMISSION_SYSTEM_FIXED;
                                    wasChanged = true;
                                }

                                if (wasChanged) {
@@ -1216,6 +1223,13 @@ public class PermissionManagerService {
                            boolean softRestricted = bp.isSoftRestricted();

                            for (int userId : currentUserIds) {
                                // If permission policy is not ready we don't deal with restricted
                                // permissions as the policy may whitelist some permissions. Once
                                // the policy is initialized we would re-evaluate permissions.
                                final boolean permissionPolicyInitialized =
                                        mPermissionPolicyInternal != null
                                                && mPermissionPolicyInternal.isInitialized(userId);

                                boolean wasChanged = false;

                                boolean restrictionExempt =
@@ -1226,7 +1240,7 @@ public class PermissionManagerService {

                                if (appSupportsRuntimePermissions) {
                                    // If hard restricted we don't allow holding it
                                    if (hardRestricted) {
                                    if (permissionPolicyInitialized && hardRestricted) {
                                        if (!restrictionExempt) {
                                            if (permState != null && permState.isGranted()
                                                    && permissionsState.revokeRuntimePermission(
@@ -1239,7 +1253,7 @@ public class PermissionManagerService {
                                            }
                                        }
                                    // If soft restricted we allow holding in a restricted form
                                    } else if (softRestricted) {
                                    } else if (permissionPolicyInitialized && softRestricted) {
                                        // Regardless if granted set the  restriction flag as it
                                        // may affect app treatment based on this permission.
                                        if (!restrictionExempt && !restrictionApplied) {
@@ -1258,7 +1272,8 @@ public class PermissionManagerService {
                                        flags &= ~FLAG_PERMISSION_REVOKE_ON_UPGRADE;
                                        wasChanged = true;
                                    // Hard restricted permissions cannot be held.
                                    } else if (!hardRestricted || restrictionExempt) {
                                    } else if (!permissionPolicyInitialized ||
                                            (!hardRestricted || restrictionExempt)) {
                                        if (permissionsState.grantRuntimePermission(bp, userId) !=
                                                PERMISSION_OPERATION_FAILURE) {
                                             wasChanged = true;
@@ -1274,7 +1289,8 @@ public class PermissionManagerService {

                                    // If legacy app always grant the permission but if restricted
                                    // and not exempt take a note a restriction should be applied.
                                    if ((hardRestricted || softRestricted)
                                    if (permissionPolicyInitialized
                                            && (hardRestricted || softRestricted)
                                                    && !restrictionExempt && !restrictionApplied) {
                                        flags |= FLAG_PERMISSION_APPLY_RESTRICTION;
                                        wasChanged = true;
@@ -1282,16 +1298,18 @@ public class PermissionManagerService {
                                }

                                // If unrestricted or restriction exempt, don't apply restriction.
                                if (permissionPolicyInitialized) {
                                    if (!(hardRestricted || softRestricted) || restrictionExempt) {
                                        if (restrictionApplied) {
                                            flags &= ~FLAG_PERMISSION_APPLY_RESTRICTION;
                                        // Dropping restriction on a legacy app requires a review.
                                            // Dropping restriction on a legacy app implies a review
                                            if (!appSupportsRuntimePermissions) {
                                                flags |= FLAG_PERMISSION_REVIEW_REQUIRED;
                                            }
                                            wasChanged = true;
                                        }
                                    }
                                }

                                if (wasChanged) {
                                    updatedUserIds = ArrayUtils.appendInt(updatedUserIds, userId);
@@ -2900,6 +2918,7 @@ public class PermissionManagerService {
        }

        mPermissionControllerManager = mContext.getSystemService(PermissionControllerManager.class);
        mPermissionPolicyInternal = LocalServices.getService(PermissionPolicyInternal.class);
    }

    private static String getVolumeUuidForPackage(PackageParser.Package pkg) {
+27 −0
Original line number Diff line number Diff line
@@ -18,6 +18,7 @@ package com.android.server.policy;

import android.annotation.NonNull;
import android.annotation.Nullable;
import android.annotation.UserIdInt;
import android.content.Intent;

/**
@@ -25,6 +26,19 @@ import android.content.Intent;
 */
public abstract class PermissionPolicyInternal {

    /**
     * Callback for initializing the permission policy service.
     */
    public interface OnInitializedCallback {

        /**
         * Called when initialized for the given user.
         *
         * @param userId The initialized user.
         */
        void onInitialized(@UserIdInt int userId);
    }

    /**
     * Check whether an activity should be started.
     *
@@ -36,4 +50,17 @@ public abstract class PermissionPolicyInternal {
     */
    public abstract boolean checkStartActivity(@NonNull Intent intent, int callingUid,
            @Nullable String callingPackage);

    /**
     * @return Whether the policy is initialized for a user.
     */
    public abstract boolean isInitialized(@UserIdInt int userId);

    /**
     * Set a callback for users being initialized. If the user is already
     * initialized the callback will not be invoked.
     *
     * @param callback The callback to register.
     */
    public abstract void setOnInitializedCallback(@NonNull OnInitializedCallback callback);
}
+25 −0
Original line number Diff line number Diff line
@@ -66,6 +66,7 @@ import com.android.server.LocalServices;
import com.android.server.SystemService;
import com.android.server.pm.permission.PermissionManagerServiceInternal;

import com.android.server.policy.PermissionPolicyInternal.OnInitializedCallback;
import java.util.ArrayList;
import java.util.concurrent.CountDownLatch;

@@ -86,6 +87,10 @@ public final class PermissionPolicyService extends SystemService {
    @GuardedBy("mLock")
    private final SparseBooleanArray mIsStarted = new SparseBooleanArray();

    /** Callbacks for when a user is initialized */
    @GuardedBy("mLock")
    private OnInitializedCallback mOnInitializedCallback;

    /**
     * Whether an async {@link #synchronizePackagePermissionsAndAppOpsForUser} is currently
     * scheduled for a package/user.
@@ -240,12 +245,20 @@ public final class PermissionPolicyService extends SystemService {

        grantOrUpgradeDefaultRuntimePermissionsIfNeeded(userId);

        final OnInitializedCallback callback;

        synchronized (mLock) {
            mIsStarted.put(userId, true);
            callback = mOnInitializedCallback;
        }

        // Force synchronization as permissions might have changed
        synchronizePermissionsAndAppOpsForUser(userId);

        // Tell observers we are initialized for this user.
        if (callback != null) {
            callback.onInitialized(userId);
        }
    }

    @Override
@@ -807,6 +820,18 @@ public final class PermissionPolicyService extends SystemService {
            return true;
        }

        @Override
        public boolean isInitialized(int userId) {
            return isStarted(userId);
        }

        @Override
        public void setOnInitializedCallback(@NonNull OnInitializedCallback callback) {
            synchronized (mLock) {
                mOnInitializedCallback = callback;
            }
        }

        /**
         * Check if the intent action is removed for the calling package (often based on target SDK
         * version). If the action is removed, we'll silently cancel the activity launch.
Loading