From b20628a54b3d98447508797b609111373f9b2c19 Mon Sep 17 00:00:00 2001 From: Nikita Dubrovsky Date: Mon, 11 Jan 2021 17:46:40 -0800 Subject: [PATCH] Use ActivityRecord for URI permissions in autofill Previously URI permissions granted when autofilling content were tied to the autofill session lifecycle and managed manually in the autofill service in system server. With this change, permissions are now tied to the lifecycle of the activity being filled. Bug: 168341541 Test: atest CtsAutoFillServiceTestCases:InlineAugmentedContentTest Test: Manually using ReceiveContentDemo Change-Id: I6a1ec210985a5d58ab2ff3db8e6e4ffa3fdb0df4 --- .../autofill/AutofillUriGrantsManager.java | 185 +++--------------- .../RemoteAugmentedAutofillService.java | 20 +- .../com/android/server/autofill/Session.java | 12 +- .../wm/ActivityTaskManagerInternal.java | 8 + .../server/wm/ActivityTaskManagerService.java | 10 + 5 files changed, 56 insertions(+), 179 deletions(-) diff --git a/services/autofill/java/com/android/server/autofill/AutofillUriGrantsManager.java b/services/autofill/java/com/android/server/autofill/AutofillUriGrantsManager.java index 801be5e48d8ba..51e023d545faf 100644 --- a/services/autofill/java/com/android/server/autofill/AutofillUriGrantsManager.java +++ b/services/autofill/java/com/android/server/autofill/AutofillUriGrantsManager.java @@ -20,6 +20,8 @@ import static android.content.ContentResolver.SCHEME_CONTENT; import static com.android.server.autofill.Helper.sVerbose; +import static java.lang.Integer.toHexString; + import android.annotation.NonNull; import android.annotation.UserIdInt; import android.app.IUriGrantsManager; @@ -33,32 +35,16 @@ import android.os.Binder; import android.os.IBinder; import android.os.RemoteException; import android.os.UserHandle; -import android.util.ArrayMap; -import android.util.ArraySet; -import android.util.Pair; import android.util.Slog; -import com.android.internal.annotations.GuardedBy; import com.android.server.LocalServices; -import com.android.server.uri.UriGrantsManagerInternal; - -import java.io.PrintWriter; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Iterator; -import java.util.List; -import java.util.Set; +import com.android.server.wm.ActivityTaskManagerInternal; /** - * Grants and revokes URI permissions for content-based autofill suggestions. + * Grants URI permissions for content-based autofill suggestions. * - *

Note that the system cannot just hand out grants directly; it must always do so on behalf of - * an owner (see {@link com.android.server.uri.UriGrantsManagerService}). For autofill, the owner - * is the autofill service provider that creates a given autofill suggestion containing a content - * URI. Therefore, this manager class must be instantiated with the service uid of the provider for - * which it will manage URI grants. - * - *

To dump the state of this class, use {@code adb shell dumpsys autofill}. + *

URI permissions granted by this class are tied to the activity being filled. When the + * activity finishes, its URI grants are automatically revoked. * *

To dump all active URI permissions, use {@code adb shell dumpsys activity permissions}. */ @@ -69,26 +55,10 @@ final class AutofillUriGrantsManager { @UserIdInt private final int mSourceUserId; @NonNull - private final IBinder mPermissionOwner; - @NonNull - private final UriGrantsManagerInternal mUgmInternal; + private final ActivityTaskManagerInternal mActivityTaskMgrInternal; @NonNull private final IUriGrantsManager mUgm; - // We use a local lock here for simplicity, since the synchronized code does not depend on - // any other resources (the "hold and wait" condition required for deadlock is not present). - // If this changes in the future, instead of using a local lock this should be updated to - // use the shared lock from AutofillManagerServiceImpl. - @NonNull - private final Object mLock; - - // Tracks the URIs that have been granted to each package. For each URI, the map stores the - // activities that triggered the grant. This allows revoking permissions only once all - // activities that triggered the grant are finished. - @NonNull - @GuardedBy("mLock") - private final ArrayMap>> mActiveGrantsByPackage; - /** * Creates a new instance of the manager. * @@ -99,159 +69,60 @@ final class AutofillUriGrantsManager { AutofillUriGrantsManager(int serviceUid) { mSourceUid = serviceUid; mSourceUserId = UserHandle.getUserId(mSourceUid); - mUgmInternal = LocalServices.getService(UriGrantsManagerInternal.class); - mPermissionOwner = mUgmInternal.newUriPermissionOwner("autofill-" + serviceUid); + mActivityTaskMgrInternal = LocalServices.getService(ActivityTaskManagerInternal.class); mUgm = UriGrantsManager.getService(); - mLock = new Object(); - mActiveGrantsByPackage = new ArrayMap<>(0); } public void grantUriPermissions(@NonNull ComponentName targetActivity, - @UserIdInt int targetUserId, @NonNull ClipData clip) { - String targetPkg = targetActivity.getPackageName(); + @NonNull IBinder targetActivityToken, @UserIdInt int targetUserId, + @NonNull ClipData clip) { + final String targetPkg = targetActivity.getPackageName(); + final IBinder permissionOwner = + mActivityTaskMgrInternal.getUriPermissionOwnerForActivity(targetActivityToken); + if (permissionOwner == null) { + Slog.w(TAG, "Can't grant URI permissions, because the target activity token is invalid:" + + " clip=" + clip + + ", targetActivity=" + targetActivity + ", targetUserId=" + targetUserId + + ", targetActivityToken=" + toHexString(targetActivityToken.hashCode())); + return; + } for (int i = 0; i < clip.getItemCount(); i++) { ClipData.Item item = clip.getItemAt(i); Uri uri = item.getUri(); if (uri == null || !SCHEME_CONTENT.equals(uri.getScheme())) { continue; } - if (grantUriPermissions(targetPkg, targetUserId, uri)) { - addToActiveGrants(uri, targetActivity); - } + grantUriPermissions(uri, targetPkg, targetUserId, permissionOwner); } } - public void revokeUriPermissions(@NonNull ComponentName targetActivity, - @UserIdInt int targetUserId) { - String targetPkg = targetActivity.getPackageName(); - Set urisWhoseGrantsShouldBeRevoked = removeFromActiveGrants(targetActivity); - for (Uri uri : urisWhoseGrantsShouldBeRevoked) { - revokeUriPermissions(targetPkg, targetUserId, uri); - } - } - - private boolean grantUriPermissions(@NonNull String targetPkg, @UserIdInt int targetUserId, - @NonNull Uri uri) { + private void grantUriPermissions(@NonNull Uri uri, @NonNull String targetPkg, + @UserIdInt int targetUserId, @NonNull IBinder permissionOwner) { final int sourceUserId = ContentProvider.getUserIdFromUri(uri, mSourceUserId); if (sVerbose) { Slog.v(TAG, "Granting URI permissions: uri=" + uri + ", sourceUid=" + mSourceUid + ", sourceUserId=" + sourceUserId - + ", targetPkg=" + targetPkg + ", targetUserId=" + targetUserId); + + ", targetPkg=" + targetPkg + ", targetUserId=" + targetUserId + + ", permissionOwner=" + toHexString(permissionOwner.hashCode())); } final Uri uriWithoutUserId = ContentProvider.getUriWithoutUserId(uri); final long ident = Binder.clearCallingIdentity(); try { mUgm.grantUriPermissionFromOwner( - mPermissionOwner, + permissionOwner, mSourceUid, targetPkg, uriWithoutUserId, Intent.FLAG_GRANT_READ_URI_PERMISSION, sourceUserId, targetUserId); - return true; } catch (RemoteException e) { Slog.e(TAG, "Granting URI permissions failed: uri=" + uri + ", sourceUid=" + mSourceUid + ", sourceUserId=" + sourceUserId - + ", targetPkg=" + targetPkg + ", targetUserId=" + targetUserId, e); - return false; + + ", targetPkg=" + targetPkg + ", targetUserId=" + targetUserId + + ", permissionOwner=" + toHexString(permissionOwner.hashCode()), e); } finally { Binder.restoreCallingIdentity(ident); } } - - private void revokeUriPermissions(@NonNull String targetPkg, @UserIdInt int targetUserId, - @NonNull Uri uri) { - final int sourceUserId = ContentProvider.getUserIdFromUri(uri, mSourceUserId); - if (sVerbose) { - Slog.v(TAG, "Revoking URI permissions: uri=" + uri - + ", sourceUid=" + mSourceUid + ", sourceUserId=" + sourceUserId - + ", target=" + targetPkg + ", targetUserId=" + targetUserId); - } - final Uri uriWithoutUserId = ContentProvider.getUriWithoutUserId(uri); - final long ident = Binder.clearCallingIdentity(); - try { - mUgmInternal.revokeUriPermissionFromOwner( - mPermissionOwner, - uriWithoutUserId, - Intent.FLAG_GRANT_READ_URI_PERMISSION, - sourceUserId, - targetPkg, - targetUserId); - } finally { - Binder.restoreCallingIdentity(ident); - } - } - - private void addToActiveGrants(@NonNull Uri uri, @NonNull ComponentName targetActivity) { - synchronized (mLock) { - String packageName = targetActivity.getPackageName(); - List> uris = mActiveGrantsByPackage.computeIfAbsent(packageName, - k -> new ArrayList<>(1)); - uris.add(Pair.create(uri, targetActivity.getClassName())); - } - } - - private Set removeFromActiveGrants(@NonNull ComponentName targetActivity) { - synchronized (mLock) { - String targetPackageName = targetActivity.getPackageName(); - List> uris = mActiveGrantsByPackage.get(targetPackageName); - if (uris == null || uris.isEmpty()) { - return Collections.emptySet(); - } - - // Collect all URIs whose grant was triggered by the target activity. - String targetActivityClassName = targetActivity.getClassName(); - Set urisWhoseGrantsShouldBeRevoked = new ArraySet<>(1); - for (Iterator> iter = uris.iterator(); iter.hasNext(); ) { - Pair uriAndActivity = iter.next(); - if (uriAndActivity.second.equals(targetActivityClassName)) { - urisWhoseGrantsShouldBeRevoked.add(uriAndActivity.first); - iter.remove(); - } - } - - // A URI grant may have been triggered by more than one activity for the same package. - // We should not revoke a grant if it was triggered by multiple activities and one or - // more of those activities is still alive. Therefore we do a second pass and prune - // the set of URIs to be revoked if an additional activity that triggered its grant - // is still present. - for (Pair uriAndActivity : uris) { - urisWhoseGrantsShouldBeRevoked.remove(uriAndActivity.first); - } - - // If there are no remaining URIs granted to the package, drop the entry from the map. - if (uris.isEmpty()) { - mActiveGrantsByPackage.remove(targetPackageName); - } - return urisWhoseGrantsShouldBeRevoked; - } - } - - /** - * Dump the active URI grants. - */ - public void dump(@NonNull String prefix, @NonNull PrintWriter pw) { - synchronized (mLock) { - if (mActiveGrantsByPackage.isEmpty()) { - pw.print(prefix); pw.println("URI grants: none"); - return; - } - pw.print(prefix); pw.println("URI grants:"); - final String prefix2 = prefix + " "; - final String prefix3 = prefix2 + " "; - for (int i = mActiveGrantsByPackage.size() - 1; i >= 0; i--) { - String packageName = mActiveGrantsByPackage.keyAt(i); - pw.print(prefix2); pw.println(packageName); - List> uris = mActiveGrantsByPackage.valueAt(i); - if (uris == null || uris.isEmpty()) { - continue; - } - for (Pair uriAndActivity : uris) { - pw.print(prefix3); - pw.println(uriAndActivity.first + ": " + uriAndActivity.second); - } - } - } - } } diff --git a/services/autofill/java/com/android/server/autofill/RemoteAugmentedAutofillService.java b/services/autofill/java/com/android/server/autofill/RemoteAugmentedAutofillService.java index db5bc4d5d4b0c..8525e3634e3af 100644 --- a/services/autofill/java/com/android/server/autofill/RemoteAugmentedAutofillService.java +++ b/services/autofill/java/com/android/server/autofill/RemoteAugmentedAutofillService.java @@ -57,7 +57,6 @@ import com.android.internal.logging.nano.MetricsProto.MetricsEvent; import com.android.internal.os.IResultReceiver; import com.android.server.autofill.ui.InlineFillUi; -import java.io.PrintWriter; import java.util.ArrayList; import java.util.List; import java.util.concurrent.CancellationException; @@ -152,8 +151,8 @@ final class RemoteAugmentedAutofillService * Called by {@link Session} to request augmented autofill. */ public void onRequestAutofillLocked(int sessionId, @NonNull IAutoFillManagerClient client, - int taskId, @NonNull ComponentName activityComponent, @NonNull AutofillId focusedId, - @Nullable AutofillValue focusedValue, + int taskId, @NonNull ComponentName activityComponent, @NonNull IBinder activityToken, + @NonNull AutofillId focusedId, @Nullable AutofillValue focusedValue, @Nullable InlineSuggestionsRequest inlineSuggestionsRequest, @Nullable Function inlineSuggestionsCallback, @NonNull Runnable onErrorCallback, @@ -181,7 +180,8 @@ final class RemoteAugmentedAutofillService inlineSuggestionsRequest, inlineSuggestionsData, clientState, focusedId, focusedValue, inlineSuggestionsCallback, client, onErrorCallback, - remoteRenderService, userId, activityComponent); + remoteRenderService, userId, + activityComponent, activityToken); if (!showingFillWindow) { requestAutofill.complete(null); } @@ -253,7 +253,7 @@ final class RemoteAugmentedAutofillService @NonNull IAutoFillManagerClient client, @NonNull Runnable onErrorCallback, @Nullable RemoteInlineSuggestionRenderService remoteRenderService, int userId, - @NonNull ComponentName targetActivity) { + @NonNull ComponentName targetActivity, @NonNull IBinder targetActivityToken) { if (inlineSuggestionsData == null || inlineSuggestionsData.isEmpty() || inlineSuggestionsCallback == null || request == null || remoteRenderService == null) { @@ -307,8 +307,8 @@ final class RemoteAugmentedAutofillService final ArrayList fieldIds = dataset.getFieldIds(); final ClipData content = dataset.getFieldContent(); if (content != null) { - mUriGrantsManager.grantUriPermissions( - targetActivity, userId, content); + mUriGrantsManager.grantUriPermissions(targetActivity, + targetActivityToken, userId, content); final AutofillId fieldId = fieldIds.get(0); if (sDebug) { Slog.d(TAG, "Calling client autofillContent(): " @@ -368,12 +368,6 @@ final class RemoteAugmentedAutofillService + ComponentName.flattenToShortString(mComponentName) + "]"; } - @Override - public void dump(@NonNull String prefix, @NonNull PrintWriter pw) { - super.dump(prefix, pw); - mUriGrantsManager.dump(prefix, pw); - } - /** * Called by {@link Session} when it's time to destroy all augmented autofill requests. */ diff --git a/services/autofill/java/com/android/server/autofill/Session.java b/services/autofill/java/com/android/server/autofill/Session.java index f8b770b4ec840..3d2283665dceb 100644 --- a/services/autofill/java/com/android/server/autofill/Session.java +++ b/services/autofill/java/com/android/server/autofill/Session.java @@ -1684,7 +1684,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState if (content != null) { final AutofillUriGrantsManager autofillUgm = remoteAugmentedAutofillService.getAutofillUriGrantsManager(); - autofillUgm.grantUriPermissions(mComponentName, userId, content); + autofillUgm.grantUriPermissions(mComponentName, mActivityToken, userId, content); } // Fill the value into the field. @@ -3521,7 +3521,8 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState synchronized (mLock) { logAugmentedAutofillRequestLocked(mode, remoteService.getComponentName(), focusedId, isWhitelisted, inlineSuggestionsRequest != null); - remoteService.onRequestAutofillLocked(id, mClient, taskId, mComponentName, + remoteService.onRequestAutofillLocked(id, mClient, + taskId, mComponentName, mActivityToken, AutofillId.withoutSession(focusedId), currentValue, inlineSuggestionsRequest, inlineSuggestionsResponseCallback, /*onErrorCallback=*/ () -> { @@ -4147,13 +4148,6 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState if (remoteRenderService != null) { remoteRenderService.destroySuggestionViews(userId, id); } - final RemoteAugmentedAutofillService remoteAugmentedAutofillService = - mService.getRemoteAugmentedAutofillServiceIfCreatedLocked(); - if (remoteAugmentedAutofillService != null) { - final AutofillUriGrantsManager autofillUgm = - remoteAugmentedAutofillService.getAutofillUriGrantsManager(); - autofillUgm.revokeUriPermissions(mComponentName, userId); - } mDestroyed = true; diff --git a/services/core/java/com/android/server/wm/ActivityTaskManagerInternal.java b/services/core/java/com/android/server/wm/ActivityTaskManagerInternal.java index aa993bfeaf2e4..9178a8d16d165 100644 --- a/services/core/java/com/android/server/wm/ActivityTaskManagerInternal.java +++ b/services/core/java/com/android/server/wm/ActivityTaskManagerInternal.java @@ -560,6 +560,14 @@ public abstract class ActivityTaskManagerInternal { public abstract ActivityMetricsLaunchObserverRegistry getLaunchObserverRegistry(); + /** + * Returns the URI permission owner associated with the given activity (see + * {@link ActivityRecord#getUriPermissionsLocked()}). If the passed-in activity token is + * invalid, returns null. + */ + @Nullable + public abstract IBinder getUriPermissionOwnerForActivity(@NonNull IBinder activityToken); + /** * Gets bitmap snapshot of the provided task id. * diff --git a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java index a0beee4afcb95..ee8e2b713f38e 100644 --- a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java +++ b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java @@ -6233,6 +6233,16 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { } } + @Nullable + @Override + public IBinder getUriPermissionOwnerForActivity(@NonNull IBinder activityToken) { + ActivityTaskManagerService.enforceNotIsolatedCaller("getUriPermissionOwnerForActivity"); + synchronized (mGlobalLock) { + ActivityRecord r = ActivityRecord.isInRootTaskLocked(activityToken); + return (r == null) ? null : r.getUriPermissionsLocked().getExternalToken(); + } + } + @Override public TaskSnapshot getTaskSnapshotBlocking( int taskId, boolean isLowResolution) {