From 6ffc5a6eeed2b20edad0e88aa775ede3cf2e315e Mon Sep 17 00:00:00 2001 From: Todd Kennedy Date: Wed, 3 Jul 2019 09:35:31 -0700 Subject: [PATCH] Move add/remove permission Bug: 135279435 Test: atest android.content.pm.cts.PackageManagerTest Change-Id: I40a6d592031483a6dfe61906f6fa011c91910328 --- .../app/ApplicationPackageManager.java | 6 +- .../android/content/pm/IPackageManager.aidl | 18 +-- .../content/pm/PackageManagerInternal.java | 7 + .../permission/IPermissionManager.aidl | 4 + .../server/pm/PackageManagerService.java | 52 +++++--- .../permission/PermissionManagerService.java | 123 ++++++++---------- .../PermissionManagerServiceInternal.java | 4 - 7 files changed, 110 insertions(+), 104 deletions(-) diff --git a/core/java/android/app/ApplicationPackageManager.java b/core/java/android/app/ApplicationPackageManager.java index 2feede0b92bac..1641afb3d65ca 100644 --- a/core/java/android/app/ApplicationPackageManager.java +++ b/core/java/android/app/ApplicationPackageManager.java @@ -661,7 +661,7 @@ public class ApplicationPackageManager extends PackageManager { @Override public boolean addPermission(PermissionInfo info) { try { - return mPM.addPermission(info); + return mPermissionManager.addPermission(info, false); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } @@ -670,7 +670,7 @@ public class ApplicationPackageManager extends PackageManager { @Override public boolean addPermissionAsync(PermissionInfo info) { try { - return mPM.addPermissionAsync(info); + return mPermissionManager.addPermission(info, true); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } @@ -679,7 +679,7 @@ public class ApplicationPackageManager extends PackageManager { @Override public void removePermission(String name) { try { - mPM.removePermission(name); + mPermissionManager.removePermission(name); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } diff --git a/core/java/android/content/pm/IPackageManager.aidl b/core/java/android/content/pm/IPackageManager.aidl index 0a4f0a19b634e..277e41dc7e012 100644 --- a/core/java/android/content/pm/IPackageManager.aidl +++ b/core/java/android/content/pm/IPackageManager.aidl @@ -101,12 +101,6 @@ interface IPackageManager { int checkUidPermission(String permName, int uid); - @UnsupportedAppUsage - boolean addPermission(in PermissionInfo info); - - @UnsupportedAppUsage - void removePermission(String name); - @UnsupportedAppUsage void grantRuntimePermission(String packageName, String permissionName, int userId); @@ -614,9 +608,6 @@ interface IPackageManager { int movePackage(in String packageName, in String volumeUuid); int movePrimaryStorage(in String volumeUuid); - @UnsupportedAppUsage - boolean addPermissionAsync(in PermissionInfo info); - boolean setInstallLocation(int loc); @UnsupportedAppUsage int getInstallLocation(); @@ -771,4 +762,13 @@ interface IPackageManager { @UnsupportedAppUsage PermissionGroupInfo getPermissionGroupInfo(String name, int flags); + + @UnsupportedAppUsage + boolean addPermission(in PermissionInfo info); + + @UnsupportedAppUsage + boolean addPermissionAsync(in PermissionInfo info); + + @UnsupportedAppUsage + void removePermission(String name); } diff --git a/core/java/android/content/pm/PackageManagerInternal.java b/core/java/android/content/pm/PackageManagerInternal.java index 672994e791340..1c1d7095d9134 100644 --- a/core/java/android/content/pm/PackageManagerInternal.java +++ b/core/java/android/content/pm/PackageManagerInternal.java @@ -999,4 +999,11 @@ public abstract class PackageManagerInternal { * Migrates legacy obb data to its new location. */ public abstract void migrateLegacyObbData(); + + /** + * Writes all package manager settings to disk. If {@code async} is {@code true}, the + * settings are written at some point in the future. Otherwise, the call blocks until + * the settings have been written. + */ + public abstract void writeSettings(boolean async); } diff --git a/core/java/android/permission/IPermissionManager.aidl b/core/java/android/permission/IPermissionManager.aidl index f0ebd3a5607c7..3b69b122ca1d9 100644 --- a/core/java/android/permission/IPermissionManager.aidl +++ b/core/java/android/permission/IPermissionManager.aidl @@ -35,4 +35,8 @@ interface IPermissionManager { PermissionInfo getPermissionInfo(String permName, String packageName, int flags); ParceledListSlice queryPermissionsByGroup(String groupName, int flags); + + boolean addPermission(in PermissionInfo info, boolean async); + + void removePermission(String name); } diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 5f4cf5ff54557..232bca82aa9cd 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -5695,37 +5695,36 @@ public class PackageManagerService extends IPackageManager.Stub } } - private boolean addDynamicPermission(PermissionInfo info, final boolean async) { - return mPermissionManager.addDynamicPermission( - info, async, getCallingUid(), new PermissionCallback() { - @Override - public void onPermissionChanged() { - if (!async) { - mSettings.writeLPr(); - } else { - scheduleWriteSettingsLocked(); - } - } - }); - } - + // NOTE: Can't remove due to unsupported app usage @Override public boolean addPermission(PermissionInfo info) { - synchronized (mPackages) { - return addDynamicPermission(info, false); - } + try { + // Because this is accessed via the package manager service AIDL, + // go through the permission manager service AIDL + return mPermissionManagerService.addPermission(info, false); + } catch (RemoteException ignore) { } + return false; } + // NOTE: Can't remove due to unsupported app usage @Override public boolean addPermissionAsync(PermissionInfo info) { - synchronized (mPackages) { - return addDynamicPermission(info, true); - } + try { + // Because this is accessed via the package manager service AIDL, + // go through the permission manager service AIDL + return mPermissionManagerService.addPermission(info, true); + } catch (RemoteException ignore) { } + return false; } + // NOTE: Can't remove due to unsupported app usage @Override public void removePermission(String permName) { - mPermissionManager.removeDynamicPermission(permName, getCallingUid(), mPermissionCallback); + try { + // Because this is accessed via the package manager service AIDL, + // go through the permission manager service AIDL + mPermissionManagerService.removePermission(permName); + } catch (RemoteException ignore) { } } @Override @@ -25006,6 +25005,17 @@ public class PackageManagerService extends IPackageManager.Stub Slog.wtf(TAG, e); } } + + @Override + public void writeSettings(boolean async) { + synchronized (mPackages) { + if (async) { + scheduleWriteSettingsLocked(); + } else { + mSettings.writeLPr(); + } + } + } } @GuardedBy("mPackages") 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 712489fbe7ade..9d7bec5c8035b 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java @@ -399,6 +399,62 @@ public class PermissionManagerService extends IPermissionManager.Stub { } } + @Override + public boolean addPermission(PermissionInfo info, boolean async) { + final int callingUid = getCallingUid(); + if (mPackageManagerInt.getInstantAppPackageName(callingUid) != null) { + throw new SecurityException("Instant apps can't add permissions"); + } + if (info.labelRes == 0 && info.nonLocalizedLabel == null) { + throw new SecurityException("Label must be specified in permission"); + } + final BasePermission tree = mSettings.enforcePermissionTree(info.name, callingUid); + final boolean added; + final boolean changed; + synchronized (mLock) { + BasePermission bp = mSettings.getPermissionLocked(info.name); + added = bp == null; + int fixedLevel = PermissionInfo.fixProtectionLevel(info.protectionLevel); + if (added) { + enforcePermissionCapLocked(info, tree); + bp = new BasePermission(info.name, tree.getSourcePackageName(), + BasePermission.TYPE_DYNAMIC); + } else if (!bp.isDynamic()) { + throw new SecurityException("Not allowed to modify non-dynamic permission " + + info.name); + } + changed = bp.addToTree(fixedLevel, info, tree); + if (added) { + mSettings.putPermissionLocked(info.name, bp); + } + } + if (changed) { + mPackageManagerInt.writeSettings(async); + } + return added; + } + + @Override + public void removePermission(String permName) { + final int callingUid = getCallingUid(); + if (mPackageManagerInt.getInstantAppPackageName(callingUid) != null) { + throw new SecurityException("Instant applications don't have access to this method"); + } + final BasePermission tree = mSettings.enforcePermissionTree(permName, callingUid); + synchronized (mLock) { + final BasePermission bp = mSettings.getPermissionLocked(permName); + if (bp == null) { + return; + } + if (bp.isDynamic()) { + // TODO: switch this back to SecurityException + Slog.wtf(TAG, "Not allowed to modify non-dynamic permission " + + permName); + } + mSettings.removePermissionLocked(permName); + mPackageManagerInt.writeSettings(false); + } + } private int checkPermission(String permName, String pkgName, int callingUid, int userId) { if (!mUserManagerInt.exists(userId)) { @@ -843,63 +899,6 @@ public class PermissionManagerService extends IPermissionManager.Stub { } } - private boolean addDynamicPermission( - PermissionInfo info, int callingUid, PermissionCallback callback) { - if (mPackageManagerInt.getInstantAppPackageName(callingUid) != null) { - throw new SecurityException("Instant apps can't add permissions"); - } - if (info.labelRes == 0 && info.nonLocalizedLabel == null) { - throw new SecurityException("Label must be specified in permission"); - } - final BasePermission tree = mSettings.enforcePermissionTree(info.name, callingUid); - final boolean added; - final boolean changed; - synchronized (mLock) { - BasePermission bp = mSettings.getPermissionLocked(info.name); - added = bp == null; - int fixedLevel = PermissionInfo.fixProtectionLevel(info.protectionLevel); - if (added) { - enforcePermissionCapLocked(info, tree); - bp = new BasePermission(info.name, tree.getSourcePackageName(), - BasePermission.TYPE_DYNAMIC); - } else if (!bp.isDynamic()) { - throw new SecurityException("Not allowed to modify non-dynamic permission " - + info.name); - } - changed = bp.addToTree(fixedLevel, info, tree); - if (added) { - mSettings.putPermissionLocked(info.name, bp); - } - } - if (changed && callback != null) { - callback.onPermissionChanged(); - } - return added; - } - - private void removeDynamicPermission( - String permName, int callingUid, PermissionCallback callback) { - if (mPackageManagerInt.getInstantAppPackageName(callingUid) != null) { - throw new SecurityException("Instant applications don't have access to this method"); - } - final BasePermission tree = mSettings.enforcePermissionTree(permName, callingUid); - synchronized (mLock) { - final BasePermission bp = mSettings.getPermissionLocked(permName); - if (bp == null) { - return; - } - if (bp.isDynamic()) { - // TODO: switch this back to SecurityException - Slog.wtf(TAG, "Not allowed to modify non-dynamic permission " - + permName); - } - mSettings.removePermissionLocked(permName); - if (callback != null) { - callback.onPermissionRemoved(); - } - } - } - /** * Restore the permission state for a package. * @@ -3168,16 +3167,6 @@ public class PermissionManagerService extends IPermissionManager.Stub { PermissionManagerService.this.removeAllPermissions(pkg, chatty); } @Override - public boolean addDynamicPermission(PermissionInfo info, boolean async, int callingUid, - PermissionCallback callback) { - return PermissionManagerService.this.addDynamicPermission(info, callingUid, callback); - } - @Override - public void removeDynamicPermission(String permName, int callingUid, - PermissionCallback callback) { - PermissionManagerService.this.removeDynamicPermission(permName, callingUid, callback); - } - @Override public void grantRuntimePermission(String permName, String packageName, boolean overridePolicy, int callingUid, int userId, PermissionCallback callback) { 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 e4f48cc66a435..23d011439ccc7 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java @@ -148,10 +148,6 @@ public abstract class PermissionManagerServiceInternal extends PermissionManager public abstract void 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, - int callingUid, @Nullable PermissionCallback callback); - public abstract void removeDynamicPermission(@NonNull String permName, int callingUid, - @Nullable PermissionCallback callback); /** Retrieve the packages that have requested the given app op permission */ public abstract @Nullable String[] getAppOpPermissionPackages(