[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
(cherry picked from commit 0b41c8940a44a9eff4b277ce019a1ffdb3a44b7e)

6 files changed, 136 insertions, 38 deletions

diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java
index 7fce4453b768..d60fa2e5c110 100644
--- a/services/core/java/com/android/server/pm/PackageManagerService.java
+++ b/services/core/java/com/android/server/pm/PackageManagerService.java
@@ -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
diff --git a/services/core/java/com/android/server/pm/permission/DefaultPermissionGrantPolicy.java b/services/core/java/com/android/server/pm/permission/DefaultPermissionGrantPolicy.java
index f8c4f6b2cdba..4550446f88c5 100644
--- a/services/core/java/com/android/server/pm/permission/DefaultPermissionGrantPolicy.java
+++ b/services/core/java/com/android/server/pm/permission/DefaultPermissionGrantPolicy.java
@@ -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()
diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java
index d3e5df5d62d1..8884821c770e 100644
--- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java
+++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java
@@ -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,33 +1167,28 @@ 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)
-                                            && !restrictionExempt && !restrictionApplied) {
+                                    if (permissionPolicyInitialized
+                                            && (hardRestricted || softRestricted)
+                                                    && !restrictionExempt && !restrictionApplied) {
                                         flags |= FLAG_PERMISSION_APPLY_RESTRICTION;
                                         wasChanged = true;
                                     }
                                 }
 
                                 // If unrestricted or restriction exempt, don't apply restriction.
-                                if (!(hardRestricted || softRestricted) || restrictionExempt)  {
-                                    if (restrictionApplied) {
-                                        flags &= ~FLAG_PERMISSION_APPLY_RESTRICTION;
-                                        // Dropping restriction on a legacy app requires a review.
-                                        if (!appSupportsRuntimePermissions) {
-                                            flags |= FLAG_PERMISSION_REVIEW_REQUIRED;
+                                if (permissionPolicyInitialized) {
+                                    if (!(hardRestricted || softRestricted) || restrictionExempt) {
+                                        if (restrictionApplied) {
+                                            flags &= ~FLAG_PERMISSION_APPLY_RESTRICTION;
+                                            // Dropping restriction on a legacy app implies a review
+                                            if (!appSupportsRuntimePermissions) {
+                                                flags |= FLAG_PERMISSION_REVIEW_REQUIRED;
+                                            }
+                                            wasChanged = true;
                                         }
-                                        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) {
                                     updatedUserIds = ArrayUtils.appendInt(updatedUserIds, userId);
                                 }
@@ -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,22 +1289,25 @@ 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)
-                                            && !restrictionExempt && !restrictionApplied) {
+                                    if (permissionPolicyInitialized
+                                            && (hardRestricted || softRestricted)
+                                                    && !restrictionExempt && !restrictionApplied) {
                                         flags |= FLAG_PERMISSION_APPLY_RESTRICTION;
                                         wasChanged = true;
                                     }
                                 }
 
                                 // If unrestricted or restriction exempt, don't apply restriction.
-                                if (!(hardRestricted || softRestricted) || restrictionExempt)  {
-                                    if (restrictionApplied) {
-                                        flags &= ~FLAG_PERMISSION_APPLY_RESTRICTION;
-                                        // Dropping restriction on a legacy app requires a review.
-                                        if (!appSupportsRuntimePermissions) {
-                                            flags |= FLAG_PERMISSION_REVIEW_REQUIRED;
+                                if (permissionPolicyInitialized) {
+                                    if (!(hardRestricted || softRestricted) || restrictionExempt) {
+                                        if (restrictionApplied) {
+                                            flags &= ~FLAG_PERMISSION_APPLY_RESTRICTION;
+                                            // Dropping restriction on a legacy app implies a review
+                                            if (!appSupportsRuntimePermissions) {
+                                                flags |= FLAG_PERMISSION_REVIEW_REQUIRED;
+                                            }
+                                            wasChanged = true;
                                         }
-                                        wasChanged = true;
                                     }
                                 }
 
@@ -2900,6 +2918,7 @@ public class PermissionManagerService {
         }
 
         mPermissionControllerManager = mContext.getSystemService(PermissionControllerManager.class);
+        mPermissionPolicyInternal = LocalServices.getService(PermissionPolicyInternal.class);
     }
 
     private static String getVolumeUuidForPackage(PackageParser.Package pkg) {
diff --git a/services/core/java/com/android/server/policy/PermissionPolicyInternal.java b/services/core/java/com/android/server/policy/PermissionPolicyInternal.java
index 7760c1edd9e6..6084c6718577 100644
--- a/services/core/java/com/android/server/policy/PermissionPolicyInternal.java
+++ b/services/core/java/com/android/server/policy/PermissionPolicyInternal.java
@@ -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;
 
 /**
@@ -26,6 +27,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.
      *
      * @param intent the {@link Intent} for the activity start
@@ -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);
 }
diff --git a/services/core/java/com/android/server/policy/PermissionPolicyService.java b/services/core/java/com/android/server/policy/PermissionPolicyService.java
index a569bffef141..3d543c96aacc 100644
--- a/services/core/java/com/android/server/policy/PermissionPolicyService.java
+++ b/services/core/java/com/android/server/policy/PermissionPolicyService.java
@@ -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.
diff --git a/services/java/com/android/server/SystemServer.java b/services/java/com/android/server/SystemServer.java
index 3a7cbf8cc06a..133b82f72746 100644
--- a/services/java/com/android/server/SystemServer.java
+++ b/services/java/com/android/server/SystemServer.java
@@ -1973,6 +1973,11 @@ public final class SystemServer {
         }
         traceEnd();
 
+        // Permission policy service
+        traceBeginAndSlog("StartPermissionPolicyService");
+        mSystemServiceManager.startService(PermissionPolicyService.class);
+        traceEnd();
+
         traceBeginAndSlog("MakePackageManagerServiceReady");
         mPackageManagerService.systemReady();
         traceEnd();
@@ -2007,11 +2012,6 @@ public final class SystemServer {
         mSystemServiceManager.startBootPhase(SystemService.PHASE_DEVICE_SPECIFIC_SERVICES_READY);
         traceEnd();
 
-        // Permission policy service
-        traceBeginAndSlog("StartPermissionPolicyService");
-        mSystemServiceManager.startService(PermissionPolicyService.class);
-        traceEnd();
-
         // These are needed to propagate to the runnable below.
         final NetworkManagementService networkManagementF = networkManagement;
         final NetworkStatsService networkStatsF = networkStats;