From 762bcd679e7a1fb9b19aced8528623ab22265eed Mon Sep 17 00:00:00 2001 From: Bowgo Tsai Date: Thu, 13 Aug 2020 06:31:44 +0000 Subject: [PATCH] Revert "Move PermissionsState into PermissionManagerService." This reverts commit 7e8c82cdfe769b0beaa25ec1621ae2cc3f8e61eb. Reason for revert: Causes lots of test failure Bug: 163971610 Change-Id: I8529e451a7b5310ce0f6beb9d5b37e82701c61e1 --- .../server/pm/PackageManagerService.java | 96 ++++--- .../server/pm/PackageManagerServiceUtils.java | 16 ++ .../java/com/android/server/pm/Settings.java | 108 +++++++ .../server/pm/permission/BasePermission.java | 4 +- .../permission/PermissionManagerService.java | 271 ++++++------------ .../PermissionManagerServiceInternal.java | 31 -- 6 files changed, 273 insertions(+), 253 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 572892bd6ff50..0d1c00dfe0355 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -103,6 +103,7 @@ import static android.os.incremental.IncrementalManager.isIncrementalPath; import static android.os.storage.StorageManager.FLAG_STORAGE_CE; import static android.os.storage.StorageManager.FLAG_STORAGE_DE; import static android.os.storage.StorageManager.FLAG_STORAGE_EXTERNAL; +import static android.permission.PermissionManager.KILL_APP_REASON_GIDS_CHANGED; import static com.android.internal.annotations.VisibleForTesting.Visibility; import static com.android.internal.app.IntentForwarderActivity.FORWARD_INTENT_TO_MANAGED_PROFILE; @@ -364,6 +365,7 @@ import com.android.server.pm.parsing.pkg.ParsedPackage; import com.android.server.pm.permission.BasePermission; import com.android.server.pm.permission.PermissionManagerService; import com.android.server.pm.permission.PermissionManagerServiceInternal; +import com.android.server.pm.permission.PermissionsState; import com.android.server.policy.PermissionPolicyInternal; import com.android.server.rollback.RollbackManagerInternal; import com.android.server.security.VerityUtils; @@ -1805,7 +1807,7 @@ public class PackageManagerService extends IPackageManager.Stub synchronized (mLock) { removeMessages(WRITE_SETTINGS); removeMessages(WRITE_PACKAGE_RESTRICTIONS); - writeSettingsLPrTEMP(); + mSettings.writeLPr(); mDirtyUsers.clear(); } Process.setThreadPriority(Process.THREAD_PRIORITY_BACKGROUND); @@ -1825,7 +1827,6 @@ public class PackageManagerService extends IPackageManager.Stub Process.setThreadPriority(Process.THREAD_PRIORITY_DEFAULT); synchronized (mLock) { removeMessages(WRITE_PACKAGE_LIST); - mPermissionManager.writePermissionsStateToPackageSettingsTEMP(); mSettings.writePackageListLPr(msg.arg1); } Process.setThreadPriority(Process.THREAD_PRIORITY_BACKGROUND); @@ -2506,7 +2507,7 @@ public class PackageManagerService extends IPackageManager.Stub } mSettings.onVolumeForgotten(fsUuid); - writeSettingsLPrTEMP(); + mSettings.writeLPr(); } } }; @@ -3426,7 +3427,6 @@ public class PackageManagerService extends IPackageManager.Stub + ((SystemClock.uptimeMillis()-startTime)/1000f) + " seconds"); - mPermissionManager.readPermissionsStateFromPackageSettingsTEMP(); // If the platform SDK has changed since the last time we booted, // we need to re-grant app permission to catch any new ones that // appear. This is really a hack, and means that apps can in some @@ -3544,7 +3544,7 @@ public class PackageManagerService extends IPackageManager.Stub // can downgrade to reader t.traceBegin("write settings"); - writeSettingsLPrTEMP(); + mSettings.writeLPr(); t.traceEnd(); EventLog.writeEvent(EventLogTags.BOOT_PROGRESS_PMS_READY, SystemClock.uptimeMillis()); @@ -3748,7 +3748,7 @@ public class PackageManagerService extends IPackageManager.Stub Slog.e(TAG, "updateAllSharedLibrariesLPw failed: ", e); } mPermissionManager.updatePermissions(pkg.getPackageName(), pkg); - writeSettingsLPrTEMP(); + mSettings.writeLPr(); } } catch (PackageManagerException e) { // Whoops! Something went very wrong; roll back to the stub and disable the package @@ -3759,8 +3759,9 @@ public class PackageManagerService extends IPackageManager.Stub // If we don't, installing the system package fails during scan enableSystemPackageLPw(stubPkg); } - installPackageFromSystemLIF(stubPkg.getCodePath(), null /*allUserHandles*/, - null /*origUserHandles*/, true /*writeSettings*/); + installPackageFromSystemLIF(stubPkg.getCodePath(), + null /*allUserHandles*/, null /*origUserHandles*/, + null /*origPermissionsState*/, true /*writeSettings*/); } catch (PackageManagerException pme) { // Serious WTF; we have to be able to install the stub Slog.wtf(TAG, "Failed to restore system package:" + stubPkg.getPackageName(), @@ -3774,7 +3775,7 @@ public class PackageManagerService extends IPackageManager.Stub stubPs.setEnabled(COMPONENT_ENABLED_STATE_DISABLED, UserHandle.USER_SYSTEM, "android"); } - writeSettingsLPrTEMP(); + mSettings.writeLPr(); } } return false; @@ -16262,7 +16263,7 @@ public class PackageManagerService extends IPackageManager.Stub res.setReturnCode(PackageManager.INSTALL_SUCCEEDED); //to update install status Trace.traceBegin(TRACE_TAG_PACKAGE_MANAGER, "writeSettings"); - writeSettingsLPrTEMP(); + mSettings.writeLPr(); Trace.traceEnd(TRACE_TAG_PACKAGE_MANAGER); } @@ -18862,11 +18863,29 @@ public class PackageManagerService extends IPackageManager.Stub if (outInfo != null) { outInfo.removedAppId = removedAppId; } - if ((deletedPs.sharedUser == null || deletedPs.sharedUser.packages.size() == 0) - && !isUpdatedSystemApp(deletedPs)) { - mPermissionManager.removePermissionsStateTEMP(removedAppId); - } mPermissionManager.updatePermissions(deletedPs.name, null); + if (deletedPs.sharedUser != null) { + // Remove permissions associated with package. Since runtime + // permissions are per user we have to kill the removed package + // or packages running under the shared user of the removed + // package if revoking the permissions requested only by the removed + // package is successful and this causes a change in gids. + boolean shouldKill = false; + for (int userId : UserManagerService.getInstance().getUserIds()) { + final int userIdToKill = mSettings.updateSharedUserPermsLPw(deletedPs, + userId); + shouldKill |= userIdToKill == UserHandle.USER_ALL + || userIdToKill >= UserHandle.USER_SYSTEM; + } + // If gids changed, kill all affected packages. + if (shouldKill) { + mHandler.post(() -> { + // This has to happen with no lock held. + killApplication(deletedPs.name, deletedPs.appId, + KILL_APP_REASON_GIDS_CHANGED); + }); + } + } clearPackagePreferredActivitiesLPw( deletedPs.name, changedUsers, UserHandle.USER_ALL); } @@ -18900,7 +18919,7 @@ public class PackageManagerService extends IPackageManager.Stub // can downgrade to reader if (writeSettings) { // Save settings now - writeSettingsLPrTEMP(); + mSettings.writeLPr(); } if (installedStateChanged) { mSettings.writeKernelMappingLPr(deletedPs); @@ -18987,7 +19006,8 @@ public class PackageManagerService extends IPackageManager.Stub if (DEBUG_REMOVE) Slog.d(TAG, "Re-installing system package: " + disabledPs); try { installPackageFromSystemLIF(disabledPs.getCodePathString(), allUserHandles, - outInfo == null ? null : outInfo.origUsers, writeSettings); + outInfo == null ? null : outInfo.origUsers, deletedPs.getPermissionsState(), + writeSettings); } catch (PackageManagerException e) { Slog.w(TAG, "Failed to restore system package:" + deletedPkg.getPackageName() + ": " + e.getMessage()); @@ -19018,8 +19038,9 @@ public class PackageManagerService extends IPackageManager.Stub * Installs a package that's already on the system partition. */ private AndroidPackage installPackageFromSystemLIF(@NonNull String codePathString, - @Nullable int[] allUserHandles, @Nullable int[] origUserHandles, boolean writeSettings) - throws PackageManagerException { + @Nullable int[] allUserHandles, @Nullable int[] origUserHandles, + @Nullable PermissionsState origPermissionState, boolean writeSettings) + throws PackageManagerException { final File codePath = new File(codePathString); @ParseFlags int parseFlags = mDefParseFlags @@ -19056,8 +19077,12 @@ public class PackageManagerService extends IPackageManager.Stub synchronized (mLock) { PackageSetting ps = mSettings.mPackages.get(pkg.getPackageName()); - // The update permissions method below will take care of removing obsolete permissions - // and granting install permissions. + // Propagate the permissions state as we do not want to drop on the floor + // runtime permissions. The update permissions method below will take + // care of removing obsolete permissions and grant install permissions. + if (origPermissionState != null) { + ps.getPermissionsState().copyFrom(origPermissionState); + } mPermissionManager.updatePermissions(pkg.getPackageName(), pkg); final boolean applyUserRestrictions @@ -19091,7 +19116,7 @@ public class PackageManagerService extends IPackageManager.Stub } // can downgrade to reader here if (writeSettings) { - writeSettingsLPrTEMP(); + mSettings.writeLPr(); } } return pkg; @@ -19165,7 +19190,7 @@ public class PackageManagerService extends IPackageManager.Stub } else { ps.pkgPrivateFlags &= ~ApplicationInfo.PRIVATE_FLAG_REQUIRED_FOR_SYSTEM_USER; } - writeSettingsLPrTEMP(); + mSettings.writeLPr(); } return true; } @@ -20357,7 +20382,7 @@ public class PackageManagerService extends IPackageManager.Stub (parser1, userId1) -> { synchronized (mLock) { mSettings.readAllDomainVerificationsLPr(parser1, userId1); - writeSettingsLPrTEMP(); + mSettings.writeLPr(); } }); } catch (Exception e) { @@ -21708,8 +21733,6 @@ public class PackageManagerService extends IPackageManager.Stub protected void dump(FileDescriptor fd, PrintWriter pw, String[] args) { if (!DumpUtils.checkDumpAndUsageStatsPermission(mContext, TAG, pw)) return; - mPermissionManager.writePermissionsStateToPackageSettingsTEMP(); - DumpState dumpState = new DumpState(); boolean fullPreferred = false; boolean checkin = false; @@ -21905,7 +21928,7 @@ public class PackageManagerService extends IPackageManager.Stub dumpState.setDump(DumpState.DUMP_SERVICE_PERMISSIONS); } else if ("write".equals(cmd)) { synchronized (mLock) { - writeSettingsLPrTEMP(); + mSettings.writeLPr(); pw.println("Settings written."); return; } @@ -22623,7 +22646,7 @@ public class PackageManagerService extends IPackageManager.Stub // Yay, everything is now upgraded ver.forceCurrent(); - writeSettingsLPrTEMP(); + mSettings.writeLPr(); } for (PackageFreezer freezer : freezers) { @@ -22673,7 +22696,7 @@ public class PackageManagerService extends IPackageManager.Stub AttributeCache.instance().removePackage(ps.name); } - writeSettingsLPrTEMP(); + mSettings.writeLPr(); } } @@ -23552,8 +23575,6 @@ public class PackageManagerService extends IPackageManager.Stub synchronized (mLock) { mDirtyUsers.remove(userId); mUserNeedsBadging.delete(userId); - mPermissionManager.onUserRemoved(userId); - mPermissionManager.writePermissionsStateToPackageSettingsTEMP(); mSettings.removeUserLPw(userId); mPendingBroadcasts.remove(userId); mInstantAppRegistry.onUserRemovedLPw(userId); @@ -25110,7 +25131,7 @@ public class PackageManagerService extends IPackageManager.Stub if (async) { scheduleWriteSettingsLocked(); } else { - writeSettingsLPrTEMP(); + mSettings.writeLPr(); } } } @@ -25157,7 +25178,7 @@ public class PackageManagerService extends IPackageManager.Stub return; } mSettings.mReadExternalStorageEnforced = enforced ? Boolean.TRUE : Boolean.FALSE; - writeSettingsLPrTEMP(); + mSettings.writeLPr(); } } @@ -25671,17 +25692,6 @@ public class PackageManagerService extends IPackageManager.Stub public List getMimeGroup(String packageName, String mimeGroup) { return mSettings.mPackages.get(packageName).getMimeGroup(mimeGroup); } - - /** - * Temporary method that wraps mSettings.writeLPr() and calls - * mPermissionManager.writePermissionsStateToPackageSettingsTEMP() beforehand. - * - * TODO(zhanghai): This should be removed once we finish migration of permission storage. - */ - private void writeSettingsLPrTEMP() { - mPermissionManager.writePermissionsStateToPackageSettingsTEMP(); - mSettings.writeLPr(); - } } interface PackageSender { diff --git a/services/core/java/com/android/server/pm/PackageManagerServiceUtils.java b/services/core/java/com/android/server/pm/PackageManagerServiceUtils.java index 491b4fc515cec..de0e4b53adabf 100644 --- a/services/core/java/com/android/server/pm/PackageManagerServiceUtils.java +++ b/services/core/java/com/android/server/pm/PackageManagerServiceUtils.java @@ -34,6 +34,7 @@ import android.content.Context; import android.content.Intent; import android.content.pm.PackageInfoLite; import android.content.pm.PackageManager; +import android.content.pm.PackageManagerInternal; import android.content.pm.PackageParser; import android.content.pm.PackageParser.PackageParserException; import android.content.pm.ResolveInfo; @@ -67,6 +68,7 @@ import com.android.server.EventLogTags; import com.android.server.pm.dex.DexManager; import com.android.server.pm.dex.PackageDexUsage; import com.android.server.pm.parsing.pkg.AndroidPackage; +import com.android.server.pm.permission.PermissionsState; import dalvik.system.VMRuntime; @@ -965,6 +967,20 @@ public class PackageManagerServiceUtils { } } + /** + * Returns the {@link PermissionsState} for the given package. If the {@link PermissionsState} + * could not be found, {@code null} will be returned. + */ + public static PermissionsState getPermissionsState( + PackageManagerInternal packageManagerInternal, AndroidPackage pkg) { + final PackageSetting packageSetting = packageManagerInternal.getPackageSetting( + pkg.getPackageName()); + if (packageSetting == null) { + return null; + } + return packageSetting.getPermissionsState(); + } + /** * Recursively create target directory */ diff --git a/services/core/java/com/android/server/pm/Settings.java b/services/core/java/com/android/server/pm/Settings.java index 2cf411d1ed7ae..3e3e3c5904917 100644 --- a/services/core/java/com/android/server/pm/Settings.java +++ b/services/core/java/com/android/server/pm/Settings.java @@ -954,6 +954,93 @@ public final class Settings { } } + /* + * Update the shared user setting when a package with a shared user id is removed. The gids + * associated with each permission of the deleted package are removed from the shared user' + * gid list only if its not in use by other permissions of packages in the shared user setting. + * + * @return the affected user id + */ + @UserIdInt + int updateSharedUserPermsLPw(PackageSetting deletedPs, int userId) { + if ((deletedPs == null) || (deletedPs.pkg == null)) { + Slog.i(PackageManagerService.TAG, + "Trying to update info for null package. Just ignoring"); + return UserHandle.USER_NULL; + } + + // No sharedUserId + if (deletedPs.sharedUser == null) { + return UserHandle.USER_NULL; + } + + SharedUserSetting sus = deletedPs.sharedUser; + + int affectedUserId = UserHandle.USER_NULL; + // Update permissions + for (String eachPerm : deletedPs.pkg.getRequestedPermissions()) { + BasePermission bp = mPermissions.getPermission(eachPerm); + if (bp == null) { + continue; + } + + // Check if another package in the shared user needs the permission. + boolean used = false; + for (PackageSetting pkg : sus.packages) { + if (pkg.pkg != null + && !pkg.pkg.getPackageName().equals(deletedPs.pkg.getPackageName()) + && pkg.pkg.getRequestedPermissions().contains(eachPerm)) { + used = true; + break; + } + } + if (used) { + continue; + } + + PermissionsState permissionsState = sus.getPermissionsState(); + PackageSetting disabledPs = getDisabledSystemPkgLPr(deletedPs.pkg.getPackageName()); + + // If the package is shadowing is a disabled system package, + // do not drop permissions that the shadowed package requests. + if (disabledPs != null) { + boolean reqByDisabledSysPkg = false; + for (String permission : disabledPs.pkg.getRequestedPermissions()) { + if (permission.equals(eachPerm)) { + reqByDisabledSysPkg = true; + break; + } + } + if (reqByDisabledSysPkg) { + continue; + } + } + + // Try to revoke as an install permission which is for all users. + // The package is gone - no need to keep flags for applying policy. + permissionsState.updatePermissionFlags(bp, userId, + PackageManager.MASK_PERMISSION_FLAGS_ALL, 0); + + if (permissionsState.revokeInstallPermission(bp) == + PermissionsState.PERMISSION_OPERATION_SUCCESS_GIDS_CHANGED) { + affectedUserId = UserHandle.USER_ALL; + } + + // Try to revoke as an install permission which is per user. + if (permissionsState.revokeRuntimePermission(bp, userId) == + PermissionsState.PERMISSION_OPERATION_SUCCESS_GIDS_CHANGED) { + if (affectedUserId == UserHandle.USER_NULL) { + affectedUserId = userId; + } else if (affectedUserId != userId) { + // Multiple users affected. + affectedUserId = UserHandle.USER_ALL; + } + } + } + + return affectedUserId; + } + int removePackageLPw(String name) { final PackageSetting p = mPackages.get(name); if (p != null) { @@ -5457,11 +5544,32 @@ public final class Settings { // Make sure we do not mHandler.removeMessages(userId); + for (SettingBase sb : mPackages.values()) { + revokeRuntimePermissionsAndClearFlags(sb, userId); + } + + for (SettingBase sb : mSharedUsers.values()) { + revokeRuntimePermissionsAndClearFlags(sb, userId); + } + mPermissionUpgradeNeeded.delete(userId); mVersions.delete(userId); mFingerprints.remove(userId); } + private void revokeRuntimePermissionsAndClearFlags(SettingBase sb, int userId) { + PermissionsState permissionsState = sb.getPermissionsState(); + for (PermissionState permissionState + : permissionsState.getRuntimePermissionStates(userId)) { + BasePermission bp = mPermissions.getPermission(permissionState.getName()); + if (bp != null) { + permissionsState.revokeRuntimePermission(bp, userId); + permissionsState.updatePermissionFlags(bp, userId, + PackageManager.MASK_PERMISSION_FLAGS_ALL, 0); + } + } + } + public void deleteUserRuntimePermissionsFile(int userId) { mPersistence.deleteForUser(UserHandle.of(userId)); } diff --git a/services/core/java/com/android/server/pm/permission/BasePermission.java b/services/core/java/com/android/server/pm/permission/BasePermission.java index 962638b4f63c0..cfa0449aaf336 100644 --- a/services/core/java/com/android/server/pm/permission/BasePermission.java +++ b/services/core/java/com/android/server/pm/permission/BasePermission.java @@ -38,6 +38,7 @@ import android.util.Slog; import com.android.server.pm.DumpState; import com.android.server.pm.PackageManagerService; +import com.android.server.pm.PackageSetting; import com.android.server.pm.PackageSettingBase; import com.android.server.pm.parsing.PackageInfoUtils; import com.android.server.pm.parsing.pkg.AndroidPackage; @@ -419,7 +420,8 @@ public final class BasePermission { } public void enforceDeclaredUsedAndRuntimeOrDevelopment(AndroidPackage pkg, - PermissionsState permsState) { + PackageSetting pkgSetting) { + final PermissionsState permsState = pkgSetting.getPermissionsState(); int index = pkg.getRequestedPermissions().indexOf(name); if (!permsState.hasRequestedPermission(name) && index == -1) { throw new SecurityException("Package " + pkg.getPackageName() 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 5b4a630260395..63aa80e8269a0 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java @@ -142,6 +142,7 @@ import com.android.server.Watchdog; import com.android.server.pm.ApexManager; import com.android.server.pm.PackageManagerServiceUtils; import com.android.server.pm.PackageSetting; +import com.android.server.pm.SharedUserSetting; import com.android.server.pm.UserManagerService; import com.android.server.pm.parsing.PackageInfoUtils; import com.android.server.pm.parsing.pkg.AndroidPackage; @@ -226,9 +227,6 @@ public class PermissionManagerService extends IPermissionManager.Stub { /** Internal connection to the user manager */ private final UserManagerInternal mUserManagerInt; - /** Maps from App ID to PermissionsState */ - private final SparseArray mAppIdStates = new SparseArray<>(); - /** Permission controller: User space permission management */ private PermissionControllerManager mPermissionControllerManager; @@ -673,6 +671,11 @@ public class PermissionManagerService extends IPermissionManager.Stub { if (pkg == null) { return 0; } + final PackageSetting ps = (PackageSetting) mPackageManagerInt.getPackageSetting( + pkg.getPackageName()); + if (ps == null) { + return 0; + } synchronized (mLock) { if (mSettings.getPermissionLocked(permName) == null) { return 0; @@ -681,11 +684,7 @@ public class PermissionManagerService extends IPermissionManager.Stub { if (mPackageManagerInt.filterAppAccess(pkg, callingUid, userId)) { return 0; } - final PermissionsState permissionsState = getPermissionsState(pkg); - if (permissionsState == null) { - Slog.e(TAG, "Missing permissions state for " + packageName); - return 0; - } + PermissionsState permissionsState = ps.getPermissionsState(); return permissionsState.getPermissionFlags(permName, userId); } @@ -772,7 +771,9 @@ public class PermissionManagerService extends IPermissionManager.Stub { } final AndroidPackage pkg = mPackageManagerInt.getPackage(packageName); - if (pkg == null) { + final PackageSetting ps = (PackageSetting) mPackageManagerInt.getPackageSetting( + packageName); + if (pkg == null || ps == null) { Log.e(TAG, "Unknown package: " + packageName); return; } @@ -788,12 +789,7 @@ public class PermissionManagerService extends IPermissionManager.Stub { throw new IllegalArgumentException("Unknown permission: " + permName); } - final PermissionsState permissionsState = getPermissionsState(pkg); - if (permissionsState == null) { - Slog.e(TAG, "Missing permissions state for " + packageName); - return; - } - + final PermissionsState permissionsState = ps.getPermissionsState(); final boolean hadState = permissionsState.getRuntimePermissionState(permName, userId) != null; if (!hadState) { @@ -868,11 +864,12 @@ public class PermissionManagerService extends IPermissionManager.Stub { final boolean[] changed = new boolean[1]; mPackageManagerInt.forEachPackage(pkg -> { - final PermissionsState permissionsState = getPermissionsState(pkg); - if (permissionsState == null) { - Slog.e(TAG, "Missing permissions state for " + pkg.getPackageName()); + final PackageSetting ps = (PackageSetting) mPackageManagerInt.getPackageSetting( + pkg.getPackageName()); + if (ps == null) { return; } + final PermissionsState permissionsState = ps.getPermissionsState(); changed[0] |= permissionsState.updatePermissionFlagsForAllPermissions( userId, effectiveFlagMask, effectiveFlagValues); mOnPermissionChangeListeners.onPermissionsChanged(pkg.getUid()); @@ -926,11 +923,12 @@ public class PermissionManagerService extends IPermissionManager.Stub { } final int uid = UserHandle.getUid(userId, pkg.getUid()); - final PermissionsState permissionsState = getPermissionsState(pkg); - if (permissionsState == null) { - Slog.e(TAG, "Missing permissions state for " + pkg.getPackageName()); + final PackageSetting ps = (PackageSetting) mPackageManagerInt.getPackageSetting( + pkg.getPackageName()); + if (ps == null) { return PackageManager.PERMISSION_DENIED; } + final PermissionsState permissionsState = ps.getPermissionsState(); if (checkSinglePermissionInternal(uid, permissionsState, permissionName)) { return PackageManager.PERMISSION_GRANTED; @@ -1141,9 +1139,9 @@ public class PermissionManagerService extends IPermissionManager.Stub { final long identity = Binder.clearCallingIdentity(); try { - final PermissionsState permissionsState = getPermissionsState(pkg); + final PermissionsState permissionsState = + PackageManagerServiceUtils.getPermissionsState(mPackageManagerInt, pkg); if (permissionsState == null) { - Slog.e(TAG, "Missing permissions state for " + packageName); return null; } @@ -1453,13 +1451,7 @@ public class PermissionManagerService extends IPermissionManager.Stub { throw new IllegalArgumentException("Unknown package: " + packageName); } - final PermissionsState permissionsState = getPermissionsState(pkg); - if (permissionsState == null) { - Slog.e(TAG, "Missing permissions state for " + pkg.getPackageName()); - return; - } - - bp.enforceDeclaredUsedAndRuntimeOrDevelopment(pkg, permissionsState); + bp.enforceDeclaredUsedAndRuntimeOrDevelopment(pkg, ps); // If a permission review is required for legacy apps we represent // their permissions as always granted runtime ones since we need @@ -1472,6 +1464,8 @@ public class PermissionManagerService extends IPermissionManager.Stub { final int uid = UserHandle.getUid(userId, UserHandle.getAppId(pkg.getUid())); + final PermissionsState permissionsState = ps.getPermissionsState(); + final int flags = permissionsState.getPermissionFlags(permName, userId); if ((flags & PackageManager.FLAG_PERMISSION_SYSTEM_FIXED) != 0) { Log.e(TAG, "Cannot grant system fixed permission " @@ -1605,7 +1599,9 @@ public class PermissionManagerService extends IPermissionManager.Stub { "revokeRuntimePermission"); final AndroidPackage pkg = mPackageManagerInt.getPackage(packageName); - if (pkg == null) { + final PackageSetting ps = (PackageSetting) mPackageManagerInt.getPackageSetting( + packageName); + if (pkg == null || ps == null) { Log.e(TAG, "Unknown package: " + packageName); return; } @@ -1617,13 +1613,7 @@ public class PermissionManagerService extends IPermissionManager.Stub { throw new IllegalArgumentException("Unknown permission: " + permName); } - final PermissionsState permissionsState = getPermissionsState(pkg); - if (permissionsState == null) { - Slog.e(TAG, "Missing permissions state for " + pkg.getPackageName()); - return; - } - - bp.enforceDeclaredUsedAndRuntimeOrDevelopment(pkg, permissionsState); + bp.enforceDeclaredUsedAndRuntimeOrDevelopment(pkg, ps); // If a permission review is required for legacy apps we represent // their permissions as always granted runtime ones since we need @@ -1634,6 +1624,8 @@ public class PermissionManagerService extends IPermissionManager.Stub { return; } + final PermissionsState permissionsState = ps.getPermissionsState(); + final int flags = permissionsState.getPermissionFlags(permName, userId); // Only the system may revoke SYSTEM_FIXED permissions. if ((flags & PackageManager.FLAG_PERMISSION_SYSTEM_FIXED) != 0 @@ -2464,36 +2456,14 @@ public class PermissionManagerService extends IPermissionManager.Stub { } } - private void onUserRemoved(@UserIdInt int userId) { - synchronized (mLock) { - final int appIdStatesSize = mAppIdStates.size(); - for (int i = 0; i < appIdStatesSize; i++) { - PermissionsState permissionsState = mAppIdStates.valueAt(i); - for (PermissionState permissionState - : permissionsState.getRuntimePermissionStates(userId)) { - BasePermission bp = mSettings.getPermission(permissionState.getName()); - if (bp != null) { - permissionsState.revokeRuntimePermission(bp, userId); - permissionsState.updatePermissionFlags(bp, userId, - PackageManager.MASK_PERMISSION_FLAGS_ALL, 0); - } - } - } - } - } - @NonNull private Set getGrantedPermissions(@NonNull String packageName, @UserIdInt int userId) { - final AndroidPackage pkg = mPackageManagerInt.getPackage(packageName); - if (pkg == null) { - return null; - } - final PermissionsState permissionsState = getPermissionsState(pkg); - if (permissionsState == null) { - Slog.e(TAG, "Missing permissions state for " + pkg.getPackageName()); + final PackageSetting ps = mPackageManagerInt.getPackageSetting(packageName); + if (ps == null) { return null; } + final PermissionsState permissionsState = ps.getPermissionsState(); return permissionsState.getPermissions(userId); } @@ -2508,15 +2478,11 @@ public class PermissionManagerService extends IPermissionManager.Stub { @Nullable private int[] getPackageGids(@NonNull String packageName, @UserIdInt int userId) { - final AndroidPackage pkg = mPackageManagerInt.getPackage(packageName); - if (pkg == null) { - return null; - } - final PermissionsState permissionsState = getPermissionsState(pkg); - if (permissionsState == null) { - Slog.e(TAG, "Missing permissions state for " + pkg.getPackageName()); + final PackageSetting ps = mPackageManagerInt.getPackageSetting(packageName); + if (ps == null) { return null; } + final PermissionsState permissionsState = ps.getPermissionsState(); return permissionsState.computeGids(userId); } @@ -2554,7 +2520,8 @@ public class PermissionManagerService extends IPermissionManager.Stub { if (ps == null) { return; } - final PermissionsState permissionsState = getOrCreatePermissionsState(pkg); + + final PermissionsState permissionsState = ps.getPermissionsState(); final int[] userIds = getAllUserIds(); @@ -2626,8 +2593,8 @@ public class PermissionManagerService extends IPermissionManager.Stub { // changed runtime permissions here are promotion of an install to // runtime and revocation of a runtime from a shared user. synchronized (mLock) { - updatedUserIds = revokeUnusedSharedUserPermissionsLocked( - ps.getSharedUser().getPackages(), permissionsState, userIds); + updatedUserIds = revokeUnusedSharedUserPermissionsLocked(ps.getSharedUser(), + userIds); if (!ArrayUtils.isEmpty(updatedUserIds)) { runtimePermissionsRevoked = true; } @@ -3103,8 +3070,6 @@ public class PermissionManagerService extends IPermissionManager.Stub { updatedUserIds); } - // TODO: Kill UIDs whose GIDs or runtime permissions changed. This might be more important - // for shared users. // Persist the runtime permissions state for users with changes. If permissions // were revoked because no app in the shared user declares them we have to // write synchronously to avoid losing runtime permissions state. @@ -3568,15 +3533,37 @@ public class PermissionManagerService extends IPermissionManager.Stub { final PackageSetting disabledPs = mPackageManagerInt .getDisabledSystemPackage(pkg.getPackageName()); final AndroidPackage disabledPkg = disabledPs == null ? null : disabledPs.pkg; - if (disabledPkg != null && isPackageRequestingPermission(disabledPkg, perm) - && ((privilegedPermission && disabledPkg.isPrivileged()) - || (oemPermission && canGrantOemPermission(disabledPkg, - perm)))) { - allowed = true; + if (disabledPs != null + && disabledPs.getPermissionsState().hasInstallPermission(perm)) { + // If the original was granted this permission, we take + // that grant decision as read and propagate it to the + // update. + if ((privilegedPermission && disabledPs.isPrivileged()) + || (oemPermission && disabledPs.isOem() + && canGrantOemPermission(disabledPs, perm))) { + allowed = true; + } + } else { + // The system apk may have been updated with an older + // version of the one on the data partition, but which + // granted a new system permission that it didn't have + // before. In this case we do want to allow the app to + // now get the new permission if the ancestral apk is + // privileged to get it. + if (disabledPs != null && disabledPkg != null + && isPackageRequestingPermission(disabledPkg, perm) + && ((privilegedPermission && disabledPs.isPrivileged()) + || (oemPermission && disabledPs.isOem() + && canGrantOemPermission(disabledPs, perm)))) { + allowed = true; + } } } else { + final PackageSetting ps = (PackageSetting) mPackageManagerInt.getPackageSetting( + pkg.getPackageName()); allowed = (privilegedPermission && pkg.isPrivileged()) - || (oemPermission && canGrantOemPermission(pkg, perm)); + || (oemPermission && pkg.isOem() + && canGrantOemPermission(ps, perm)); } // In any case, don't grant a privileged permission to privileged vendor apps, if // the permission's protectionLevel does not have the extra 'vendorPrivileged' @@ -3727,16 +3714,16 @@ public class PermissionManagerService extends IPermissionManager.Stub { return false; } - private static boolean canGrantOemPermission(AndroidPackage pkg, String permission) { - if (!pkg.isOem()) { + private static boolean canGrantOemPermission(PackageSetting ps, String permission) { + if (!ps.isOem()) { return false; } // all oem permissions must explicitly be granted or denied final Boolean granted = - SystemConfig.getInstance().getOemPermissions(pkg.getPackageName()).get(permission); + SystemConfig.getInstance().getOemPermissions(ps.name).get(permission); if (granted == null) { throw new IllegalStateException("OEM permission" + permission + " requested by package " - + pkg.getPackageName() + " must be explicitly declared granted or not"); + + ps.name + " must be explicitly declared granted or not"); } return Boolean.TRUE == granted; } @@ -3749,11 +3736,12 @@ public class PermissionManagerService extends IPermissionManager.Stub { } // Legacy apps have the permission and get user consent on launch. - final PermissionsState permissionsState = getPermissionsState(pkg); - if (permissionsState == null) { - Slog.e(TAG, "Missing permissions state for " + pkg.getPackageName()); + final PackageSetting ps = (PackageSetting) mPackageManagerInt.getPackageSetting( + pkg.getPackageName()); + if (ps == null) { return false; } + final PermissionsState permissionsState = ps.getPermissionsState(); return permissionsState.isPermissionReviewRequired(userId); } @@ -3778,12 +3766,14 @@ public class PermissionManagerService extends IPermissionManager.Stub { private void grantRequestedRuntimePermissionsForUser(AndroidPackage pkg, int userId, String[] grantedPermissions, int callingUid, PermissionCallback callback) { - final PermissionsState permissionsState = getPermissionsState(pkg); - if (permissionsState == null) { - Slog.e(TAG, "Missing permissions state for " + pkg.getPackageName()); + PackageSetting ps = (PackageSetting) mPackageManagerInt.getPackageSetting( + pkg.getPackageName()); + if (ps == null) { return; } + PermissionsState permissionsState = ps.getPermissionsState(); + final int immutableFlags = PackageManager.FLAG_PERMISSION_SYSTEM_FIXED | PackageManager.FLAG_PERMISSION_POLICY_FIXED; @@ -3827,9 +3817,9 @@ public class PermissionManagerService extends IPermissionManager.Stub { private void setWhitelistedRestrictedPermissionsForUsers(@NonNull AndroidPackage pkg, @UserIdInt int[] userIds, @Nullable List permissions, int callingUid, @PermissionWhitelistFlags int whitelistFlags, PermissionCallback callback) { - final PermissionsState permissionsState = getPermissionsState(pkg); + final PermissionsState permissionsState = + PackageManagerServiceUtils.getPermissionsState(mPackageManagerInt, pkg); if (permissionsState == null) { - Slog.e(TAG, "Missing permissions state for " + pkg.getPackageName()); return; } @@ -3947,11 +3937,9 @@ public class PermissionManagerService extends IPermissionManager.Stub { for (int j = 0; j < oldGrantedCount; j++) { final String permission = oldPermsForUser.valueAt(j); // Sometimes we create a new permission state instance during update. - final PermissionsState newPermissionsState = getPermissionsState(pkg); - if (permissionsState == null) { - Slog.e(TAG, "Missing permissions state for " + pkg.getPackageName()); - continue; - } + final PermissionsState newPermissionsState = + PackageManagerServiceUtils.getPermissionsState(mPackageManagerInt, + pkg); if (!newPermissionsState.hasPermission(permission, userId)) { callback.onPermissionRevoked(pkg.getUid(), userId, null); break; @@ -3963,9 +3951,10 @@ public class PermissionManagerService extends IPermissionManager.Stub { @GuardedBy("mLock") private int[] revokeUnusedSharedUserPermissionsLocked( - List pkgList, PermissionsState permissionsState, int[] allUserIds) { + SharedUserSetting suSetting, int[] allUserIds) { // Collect all used permissions in the UID final ArraySet usedPermissions = new ArraySet<>(); + final List pkgList = suSetting.getPackages(); if (pkgList == null || pkgList.size() == 0) { return EmptyArray.INT; } @@ -3983,6 +3972,7 @@ public class PermissionManagerService extends IPermissionManager.Stub { } } + PermissionsState permissionsState = suSetting.getPermissionsState(); // Prune install permissions List installPermStates = permissionsState.getInstallPermissionStates(); final int installPermCount = installPermStates.size(); @@ -4268,11 +4258,12 @@ public class PermissionManagerService extends IPermissionManager.Stub { } } else { mPackageManagerInt.forEachPackage(p -> { - final PermissionsState permissionsState = getPermissionsState(p); - if (permissionsState == null) { - Slog.e(TAG, "Missing permissions state for " + p.getPackageName()); + PackageSetting ps = mPackageManagerInt.getPackageSetting( + p.getPackageName()); + if (ps == null) { return; } + PermissionsState permissionsState = ps.getPermissionsState(); if (permissionsState.getInstallPermissionState(bp.getName()) != null) { permissionsState.revokeInstallPermission(bp); permissionsState.updatePermissionFlags(bp, UserHandle.USER_ALL, @@ -4679,66 +4670,6 @@ public class PermissionManagerService extends IPermissionManager.Stub { return mBackgroundPermissions; } - @Nullable - private PermissionsState getPermissionsState(@NonNull AndroidPackage pkg) { - return getPermissionsState(pkg.getUid()); - } - - @Nullable - private PermissionsState getPermissionsState(int appId) { - synchronized (mLock) { - return mAppIdStates.get(appId); - } - } - - @Nullable - private PermissionsState getOrCreatePermissionsState(@NonNull AndroidPackage pkg) { - return getOrCreatePermissionsState(pkg.getUid()); - } - - @Nullable - private PermissionsState getOrCreatePermissionsState(int appId) { - synchronized (mLock) { - PermissionsState state = mAppIdStates.get(appId); - if (state == null) { - state = new PermissionsState(); - mAppIdStates.put(appId, state); - } - return state; - } - } - - private void removePermissionsState(int appId) { - synchronized (mLock) { - mAppIdStates.remove(appId); - } - } - - private void readPermissionsStateFromPackageSettings() { - mPackageManagerInt.forEachPackageSetting(ps -> { - synchronized (mLock) { - final int appId = ps.pkg.getUid(); - if (!mAppIdStates.contains(appId)) { - mAppIdStates.put(appId, new PermissionsState(ps.getPermissionsState())); - } - } - }); - } - - private void writePermissionsStateToPackageSettings() { - mPackageManagerInt.forEachPackageSetting(ps -> { - synchronized (mLock) { - final int appId = ps.pkg.getUid(); - final PermissionsState permissionsState = mAppIdStates.get(appId); - if (permissionsState == null) { - Slog.e(TAG, "Missing permissions state for " + ps.pkg.getPackageName()); - return; - } - ps.getPermissionsState().copyFrom(permissionsState); - } - }); - } - private class PermissionManagerServiceInternalImpl extends PermissionManagerServiceInternal { @Override public void systemReady() { @@ -4770,22 +4701,6 @@ public class PermissionManagerService extends IPermissionManager.Stub { public void removeAllPermissions(AndroidPackage pkg, boolean chatty) { PermissionManagerService.this.removeAllPermissions(pkg, chatty); } - @Override - public void readPermissionsStateFromPackageSettingsTEMP() { - PermissionManagerService.this.readPermissionsStateFromPackageSettings(); - } - @Override - public void writePermissionsStateToPackageSettingsTEMP() { - PermissionManagerService.this.writePermissionsStateToPackageSettings(); - } - @Override - public void onUserRemoved(@UserIdInt int userId) { - PermissionManagerService.this.onUserRemoved(userId); - } - @Override - public void removePermissionsStateTEMP(int appId) { - PermissionManagerService.this.removePermissionsState(appId); - } @NonNull @Override public Set getGrantedPermissions(@NonNull String packageName, diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java index 5c7ff1f9a1ad0..cfa371ddbad3f 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java @@ -264,37 +264,6 @@ public abstract class PermissionManagerServiceInternal extends PermissionManager public abstract void addAllPermissionGroups(@NonNull AndroidPackage pkg, boolean chatty); public abstract void removeAllPermissions(@NonNull AndroidPackage pkg, boolean chatty); - /** - * Read {@code PermissionsState} from package settings. - * - * TODO(zhanghai): This is a temporary method because we should not expose - * {@code PackageSetting} which is a implementation detail that permission should not know. - * Instead, it should retrieve the legacy state via a defined API. - */ - public abstract void readPermissionsStateFromPackageSettingsTEMP(); - - /** - * Write {@code PermissionsState} from to settings. - * - * TODO(zhanghai): This is a temporary method and should be removed once we migrated persistence - * for permission. - */ - public abstract void writePermissionsStateToPackageSettingsTEMP(); - - /** - * Notify that a user has been removed and its permission state should be removed as well. - */ - public abstract void onUserRemoved(@UserIdInt int userId); - - /** - * Remove the {@code PermissionsState} associated with an app ID, called the same time as the - * removal of a {@code PackageSetitng}. - * - * TODO(zhanghai): This is a temporary method before we figure out a way to get notified of app - * ID removal via API. - */ - public abstract void removePermissionsStateTEMP(int appId); - /** * Get all the permissions granted to a package. */