From 9346e5338a4a30316b3da3f2d4b289cebc316427 Mon Sep 17 00:00:00 2001 From: Thomas Vannet Date: Fri, 28 Jan 2022 17:12:32 -0800 Subject: [PATCH] Add killed delay param to startOneTimePermissionSession This param controls how long to wait before revoking permission after every process has been killed. Deprecate previous API and update all known uses of the deprecated API. Use updated API for self-revocation feature. If multiple one-time permission sessions are started for the same package with different parameters, always use the shortest parameters. Test: atest android.permission.cts.RevokeOwnPermissionTest, atest android.permission.cts.OneTimePermissionTest Bug: 210387494 Change-Id: I0c0e21b3b48dd31f0c267d5c8b89336714835289 --- core/api/system-current.txt | 3 +- .../permission/IPermissionManager.aidl | 3 +- .../PermissionControllerManager.java | 6 +- .../PermissionControllerService.java | 2 +- .../android/permission/PermissionManager.java | 35 ++++++- .../OneTimePermissionUserManager.java | 91 ++++++++++--------- .../permission/PermissionManagerService.java | 18 +--- .../PermissionManagerServiceImpl.java | 7 +- .../PermissionManagerServiceInterface.java | 5 +- 9 files changed, 94 insertions(+), 76 deletions(-) diff --git a/core/api/system-current.txt b/core/api/system-current.txt index b2f57248402f5..ab3feeb9cd17b 100644 --- a/core/api/system-current.txt +++ b/core/api/system-current.txt @@ -9948,7 +9948,8 @@ package android.permission { method @IntRange(from=0) @RequiresPermission(anyOf={android.Manifest.permission.ADJUST_RUNTIME_PERMISSIONS_POLICY, android.Manifest.permission.UPGRADE_RUNTIME_PERMISSIONS}) public int getRuntimePermissionsVersion(); method @NonNull public java.util.List getSplitPermissions(); method @RequiresPermission(anyOf={android.Manifest.permission.ADJUST_RUNTIME_PERMISSIONS_POLICY, android.Manifest.permission.UPGRADE_RUNTIME_PERMISSIONS}) public void setRuntimePermissionsVersion(@IntRange(from=0) int); - method @RequiresPermission(android.Manifest.permission.MANAGE_ONE_TIME_PERMISSION_SESSIONS) public void startOneTimePermissionSession(@NonNull String, long, int, int); + method @Deprecated @RequiresPermission(android.Manifest.permission.MANAGE_ONE_TIME_PERMISSION_SESSIONS) public void startOneTimePermissionSession(@NonNull String, long, int, int); + method @RequiresPermission(android.Manifest.permission.MANAGE_ONE_TIME_PERMISSION_SESSIONS) public void startOneTimePermissionSession(@NonNull String, long, long, int, int); method @RequiresPermission(android.Manifest.permission.MANAGE_ONE_TIME_PERMISSION_SESSIONS) public void stopOneTimePermissionSession(@NonNull String); field @RequiresPermission(android.Manifest.permission.START_REVIEW_PERMISSION_DECISIONS) public static final String ACTION_REVIEW_PERMISSION_DECISIONS = "android.permission.action.REVIEW_PERMISSION_DECISIONS"; field public static final int PERMISSION_GRANTED = 0; // 0x0 diff --git a/core/java/android/permission/IPermissionManager.aidl b/core/java/android/permission/IPermissionManager.aidl index 1c0320e9a86e6..619c8705ddae8 100644 --- a/core/java/android/permission/IPermissionManager.aidl +++ b/core/java/android/permission/IPermissionManager.aidl @@ -79,7 +79,8 @@ interface IPermissionManager { void revokeOwnPermissionsOnKill(String packageName, in List permissions); void startOneTimePermissionSession(String packageName, int userId, long timeout, - int importanceToResetTimer, int importanceToKeepSessionAlive); + long revokeAfterKilledDelay, int importanceToResetTimer, + int importanceToKeepSessionAlive); void stopOneTimePermissionSession(String packageName, int userId); diff --git a/core/java/android/permission/PermissionControllerManager.java b/core/java/android/permission/PermissionControllerManager.java index 0cf06aa364ecd..a0c3c2be8dc76 100644 --- a/core/java/android/permission/PermissionControllerManager.java +++ b/core/java/android/permission/PermissionControllerManager.java @@ -913,15 +913,15 @@ public final class PermissionControllerManager { * * @param packageName The name of the package for which the permissions will be revoked. * @param permissions List of permissions to be revoked. - * @param callback Callback called when the revocation request has been completed. * - * @see Context#revokeOwnPermissionsOnKill(Collection) + * @see Context#revokeOwnPermissionsOnKill(java.util.Collection) * * @hide */ public void revokeOwnPermissionsOnKill(@NonNull String packageName, - @NonNull List permissions, AndroidFuture callback) { + @NonNull List permissions) { mRemoteService.postAsync(service -> { + AndroidFuture callback = new AndroidFuture<>(); service.revokeOwnPermissionsOnKill(packageName, permissions, callback); return callback; }).whenComplete((result, err) -> { diff --git a/core/java/android/permission/PermissionControllerService.java b/core/java/android/permission/PermissionControllerService.java index 8d9f82b04b545..0ce36f7a9a35d 100644 --- a/core/java/android/permission/PermissionControllerService.java +++ b/core/java/android/permission/PermissionControllerService.java @@ -291,7 +291,7 @@ public abstract class PermissionControllerService extends Service { /** * Called when a package is considered inactive based on the criteria given by - * {@link PermissionManager#startOneTimePermissionSession(String, long, int, int)}. + * {@link PermissionManager#startOneTimePermissionSession(String, long, long, int, int)}. * This method is called at the end of a one-time permission session * * @param packageName The package that has been inactive diff --git a/core/java/android/permission/PermissionManager.java b/core/java/android/permission/PermissionManager.java index 15f13eb89cb9e..12fa0ddfc648a 100644 --- a/core/java/android/permission/PermissionManager.java +++ b/core/java/android/permission/PermissionManager.java @@ -20,6 +20,7 @@ import static android.os.Build.VERSION_CODES.S; import android.Manifest; import android.annotation.CheckResult; +import android.annotation.DurationMillisLong; import android.annotation.IntRange; import android.annotation.NonNull; import android.annotation.Nullable; @@ -1281,6 +1282,22 @@ public final class PermissionManager { } } + /** + * Starts a one-time permission session for a given package. + * @see #startOneTimePermissionSession(String, long, long, int, int) + * @hide + * @deprecated Use {@link #startOneTimePermissionSession(String, long, long, int, int)} instead + */ + @Deprecated + @SystemApi + @RequiresPermission(Manifest.permission.MANAGE_ONE_TIME_PERMISSION_SESSIONS) + public void startOneTimePermissionSession(@NonNull String packageName, long timeoutMillis, + @ActivityManager.RunningAppProcessInfo.Importance int importanceToResetTimer, + @ActivityManager.RunningAppProcessInfo.Importance int importanceToKeepSessionAlive) { + startOneTimePermissionSession(packageName, timeoutMillis, -1, + importanceToResetTimer, importanceToKeepSessionAlive); + } + /** * Starts a one-time permission session for a given package. A one-time permission session is * ended if app becomes inactive. Inactivity is defined as the package's uid importance level @@ -1301,25 +1318,33 @@ public final class PermissionManager { * {@link PermissionControllerService#onOneTimePermissionSessionTimeout(String)} is invoked. *

*

- * Note that if there is currently an active session for a package a new one isn't created and - * the existing one isn't changed. + * Note that if there is currently an active session for a package a new one isn't created but + * each parameter of the existing one will be updated to the more aggressive of both sessions. + * This means that durations will be set to the shortest parameter and importances will be set + * to the lowest one. *

* @param packageName The package to start a one-time permission session for * @param timeoutMillis Number of milliseconds for an app to be in an inactive state + * @param revokeAfterKilledDelayMillis Number of milliseconds to wait before revoking on the + * event an app is terminated. Set to -1 to use default + * value for the device. * @param importanceToResetTimer The least important level to uid must be to reset the timer * @param importanceToKeepSessionAlive The least important level the uid must be to keep the - * session alive + * session alive * * @hide */ @SystemApi @RequiresPermission(Manifest.permission.MANAGE_ONE_TIME_PERMISSION_SESSIONS) - public void startOneTimePermissionSession(@NonNull String packageName, long timeoutMillis, + public void startOneTimePermissionSession(@NonNull String packageName, + @DurationMillisLong long timeoutMillis, + @DurationMillisLong long revokeAfterKilledDelayMillis, @ActivityManager.RunningAppProcessInfo.Importance int importanceToResetTimer, @ActivityManager.RunningAppProcessInfo.Importance int importanceToKeepSessionAlive) { try { mPermissionManager.startOneTimePermissionSession(packageName, mContext.getUserId(), - timeoutMillis, importanceToResetTimer, importanceToKeepSessionAlive); + timeoutMillis, revokeAfterKilledDelayMillis, importanceToResetTimer, + importanceToKeepSessionAlive); } catch (RemoteException e) { e.rethrowFromSystemServer(); } diff --git a/services/core/java/com/android/server/pm/permission/OneTimePermissionUserManager.java b/services/core/java/com/android/server/pm/permission/OneTimePermissionUserManager.java index d455be7e4a694..46fde4b59d8bd 100644 --- a/services/core/java/com/android/server/pm/permission/OneTimePermissionUserManager.java +++ b/services/core/java/com/android/server/pm/permission/OneTimePermissionUserManager.java @@ -108,14 +108,18 @@ public class OneTimePermissionUserManager { *

* @param packageName The package to start a one-time permission session for * @param timeoutMillis Number of milliseconds for an app to be in an inactive state + * @param revokeAfterKilledDelayMillis Number of milliseconds to wait after the process dies + * before ending the session. Set to -1 to use default value + * for the device. * @param importanceToResetTimer The least important level to uid must be to reset the timer * @param importanceToKeepSessionAlive The least important level the uid must be to keep the - * session alive + * session alive * * @hide */ void startPackageOneTimeSession(@NonNull String packageName, long timeoutMillis, - int importanceToResetTimer, int importanceToKeepSessionAlive) { + long revokeAfterKilledDelayMillis, int importanceToResetTimer, + int importanceToKeepSessionAlive) { int uid; try { uid = mContext.getPackageManager().getPackageUid(packageName, 0); @@ -126,11 +130,15 @@ public class OneTimePermissionUserManager { synchronized (mLock) { PackageInactivityListener listener = mListeners.get(uid); - if (listener == null) { - listener = new PackageInactivityListener(uid, packageName, timeoutMillis, + if (listener != null) { + listener.updateSessionParameters(timeoutMillis, revokeAfterKilledDelayMillis, importanceToResetTimer, importanceToKeepSessionAlive); - mListeners.put(uid, listener); + return; } + listener = new PackageInactivityListener(uid, packageName, timeoutMillis, + revokeAfterKilledDelayMillis, importanceToResetTimer, + importanceToKeepSessionAlive); + mListeners.put(uid, listener); } } @@ -158,18 +166,6 @@ public class OneTimePermissionUserManager { } } - /** - * The delay to wait before revoking on the event an app is terminated. Recommended to be long - * enough so that apps don't lose permission on an immediate restart - */ - private long getKilledDelayMillis(boolean isSelfRevokedPermissionSession) { - if (isSelfRevokedPermissionSession) { - return 0; - } - return DeviceConfig.getLong(DeviceConfig.NAMESPACE_PERMISSIONS, - PROPERTY_KILLED_DELAY_CONFIG_KEY, DEFAULT_KILLED_DELAY_MILLIS); - } - /** * Register to listen for Uids being uninstalled. This must be done outside of the * PermissionManagerService lock. @@ -178,18 +174,6 @@ public class OneTimePermissionUserManager { mContext.registerReceiver(mUninstallListener, new IntentFilter(Intent.ACTION_UID_REMOVED)); } - void setSelfRevokedPermissionSession(int uid) { - synchronized (mLock) { - PackageInactivityListener listener = mListeners.get(uid); - if (listener == null) { - Log.e(LOG_TAG, "Could not set session for uid " + uid - + " as self-revoke session: session not found"); - return; - } - listener.setSelfRevokedPermissionSession(); - } - } - /** * A class which watches a package for inactivity and notifies the permission controller when * the package becomes inactive @@ -200,11 +184,11 @@ public class OneTimePermissionUserManager { private final int mUid; private final @NonNull String mPackageName; - private final long mTimeout; - private final int mImportanceToResetTimer; - private final int mImportanceToKeepSessionAlive; + private long mTimeout; + private long mRevokeAfterKilledDelay; + private int mImportanceToResetTimer; + private int mImportanceToKeepSessionAlive; - private boolean mIsSelfRevokedPermissionSession; private boolean mIsAlarmSet; private boolean mIsFinished; @@ -218,16 +202,23 @@ public class OneTimePermissionUserManager { private final Object mToken = new Object(); private PackageInactivityListener(int uid, @NonNull String packageName, long timeout, - int importanceToResetTimer, int importanceToKeepSessionAlive) { + long revokeAfterkilledDelay, int importanceToResetTimer, + int importanceToKeepSessionAlive) { Log.i(LOG_TAG, "Start tracking " + packageName + ". uid=" + uid + " timeout=" + timeout + + " killedDelay=" + revokeAfterkilledDelay + " importanceToResetTimer=" + importanceToResetTimer + " importanceToKeepSessionAlive=" + importanceToKeepSessionAlive); mUid = uid; mPackageName = packageName; mTimeout = timeout; + mRevokeAfterKilledDelay = revokeAfterkilledDelay == -1 + ? DeviceConfig.getLong( + DeviceConfig.NAMESPACE_PERMISSIONS, PROPERTY_KILLED_DELAY_CONFIG_KEY, + DEFAULT_KILLED_DELAY_MILLIS) + : revokeAfterkilledDelay; mImportanceToResetTimer = importanceToResetTimer; mImportanceToKeepSessionAlive = importanceToKeepSessionAlive; @@ -247,6 +238,28 @@ public class OneTimePermissionUserManager { onImportanceChanged(mUid, mActivityManager.getPackageImportance(packageName)); } + public void updateSessionParameters(long timeoutMillis, long revokeAfterKilledDelayMillis, + int importanceToResetTimer, int importanceToKeepSessionAlive) { + synchronized (mInnerLock) { + mTimeout = Math.min(mTimeout, timeoutMillis); + mRevokeAfterKilledDelay = Math.min(mRevokeAfterKilledDelay, + revokeAfterKilledDelayMillis == -1 + ? DeviceConfig.getLong( + DeviceConfig.NAMESPACE_PERMISSIONS, + PROPERTY_KILLED_DELAY_CONFIG_KEY, DEFAULT_KILLED_DELAY_MILLIS) + : revokeAfterKilledDelayMillis); + mImportanceToResetTimer = Math.min(importanceToResetTimer, mImportanceToResetTimer); + mImportanceToKeepSessionAlive = Math.min(importanceToKeepSessionAlive, + mImportanceToKeepSessionAlive); + Log.v(LOG_TAG, + "Updated params for " + mPackageName + ". timeout=" + mTimeout + + " killedDelay=" + mRevokeAfterKilledDelay + + " importanceToResetTimer=" + mImportanceToResetTimer + + " importanceToKeepSessionAlive=" + mImportanceToKeepSessionAlive); + onImportanceChanged(mUid, mActivityManager.getPackageImportance(mPackageName)); + } + } + private void onImportanceChanged(int uid, int importance) { if (uid != mUid) { return; @@ -271,7 +284,7 @@ public class OneTimePermissionUserManager { } onImportanceChanged(mUid, imp); } - }, mToken, getKilledDelayMillis(mIsSelfRevokedPermissionSession)); + }, mToken, mRevokeAfterKilledDelay); return; } if (importance > mImportanceToResetTimer) { @@ -306,14 +319,6 @@ public class OneTimePermissionUserManager { } } - /** - * Marks the session as a self-revoke session, which does not delay the revocation when - * the app is restarting. - */ - public void setSelfRevokedPermissionSession() { - mIsSelfRevokedPermissionSession = true; - } - /** * Set the alarm which will callback when the package is inactive */ 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 edc0e3d64c423..695d6dd0bc766 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java @@ -68,7 +68,6 @@ import android.util.Slog; import android.util.SparseArray; import com.android.internal.annotations.GuardedBy; -import com.android.internal.infra.AndroidFuture; import com.android.internal.util.Preconditions; import com.android.internal.util.function.TriFunction; import com.android.server.LocalServices; @@ -388,7 +387,8 @@ public class PermissionManagerService extends IPermissionManager.Stub { @Override public void startOneTimePermissionSession(String packageName, @UserIdInt int userId, - long timeoutMillis, int importanceToResetTimer, int importanceToKeepSessionAlive) { + long timeoutMillis, long revokeAfterKilledDelayMillis, int importanceToResetTimer, + int importanceToKeepSessionAlive) { mContext.enforceCallingOrSelfPermission( Manifest.permission.MANAGE_ONE_TIME_PERMISSION_SESSIONS, "Must hold " + Manifest.permission.MANAGE_ONE_TIME_PERMISSION_SESSIONS @@ -398,7 +398,8 @@ public class PermissionManagerService extends IPermissionManager.Stub { final long token = Binder.clearCallingIdentity(); try { getOneTimePermissionUserManager(userId).startPackageOneTimeSession(packageName, - timeoutMillis, importanceToResetTimer, importanceToKeepSessionAlive); + timeoutMillis, revokeAfterKilledDelayMillis, importanceToResetTimer, + importanceToKeepSessionAlive); } finally { Binder.restoreCallingIdentity(token); } @@ -563,16 +564,7 @@ public class PermissionManagerService extends IPermissionManager.Stub { @Override public void revokeOwnPermissionsOnKill(@NonNull String packageName, @NonNull List permissions) { - final int callingUid = Binder.getCallingUid(); - final int callingUserId = UserHandle.getUserId(callingUid); - AndroidFuture future = new AndroidFuture<>(); - future.whenComplete((result, err) -> { - if (err == null) { - getOneTimePermissionUserManager(callingUserId) - .setSelfRevokedPermissionSession(callingUid); - } - }); - mPermissionManagerServiceImpl.revokeOwnPermissionsOnKill(packageName, permissions, future); + mPermissionManagerServiceImpl.revokeOwnPermissionsOnKill(packageName, permissions); } @Override diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceImpl.java b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceImpl.java index db9c1b56e4d87..ed351fd4aef91 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceImpl.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceImpl.java @@ -108,7 +108,6 @@ import android.util.SparseBooleanArray; import com.android.internal.annotations.GuardedBy; import com.android.internal.compat.IPlatformCompat; -import com.android.internal.infra.AndroidFuture; import com.android.internal.logging.MetricsLogger; import com.android.internal.logging.nano.MetricsProto; import com.android.internal.os.RoSystemProperties; @@ -1592,8 +1591,7 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt } @Override - public void revokeOwnPermissionsOnKill(String packageName, List permissions, - AndroidFuture callback) { + public void revokeOwnPermissionsOnKill(String packageName, List permissions) { final int callingUid = Binder.getCallingUid(); int callingUserId = UserHandle.getUserId(callingUid); int targetPackageUid = mPackageManagerInt.getPackageUid(packageName, 0, callingUserId); @@ -1608,8 +1606,7 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt + permName + " because it does not hold that permission"); } } - mPermissionControllerManager.revokeOwnPermissionsOnKill(packageName, permissions, - callback); + mPermissionControllerManager.revokeOwnPermissionsOnKill(packageName, permissions); } private boolean mayManageRolePermission(int uid) { diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInterface.java b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInterface.java index 91c558b2f35ee..b558e3dee6233 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInterface.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInterface.java @@ -27,7 +27,6 @@ import android.content.pm.permission.SplitPermissionInfoParcelable; import android.permission.IOnPermissionsChangeListener; import android.permission.PermissionManagerInternal; -import com.android.internal.infra.AndroidFuture; import com.android.server.pm.parsing.pkg.AndroidPackage; import java.io.FileDescriptor; @@ -344,10 +343,8 @@ public interface PermissionManagerServiceInterface extends PermissionManagerInte * * @param packageName The name of the package for which the permissions will be revoked. * @param permissions List of permissions to be revoked. - * @param callback Callback called when the revocation request has been completed. */ - void revokeOwnPermissionsOnKill(String packageName, List permissions, - AndroidFuture callback); + void revokeOwnPermissionsOnKill(String packageName, List permissions); /** * Get whether you should show UI with rationale for requesting a permission. You should do this