From 16e67edb366ad9a65d67b473ecc52a2d9e22f060 Mon Sep 17 00:00:00 2001 From: Yohei Yukawa Date: Thu, 10 Nov 2016 00:40:56 -0800 Subject: Fix up content URI for different users Currently Commit Content API work only when the content URI provided by the IME is accessible to the target application. This is non-trivial if the target application is running as a different user (profile) and it is actually a possible scenario when managed profile is enabled. This CL takes care of such a situation, by fixing up the content URI when and only when InputContentInfo#getContentUri() is called from a different user than the owner of the content URI. This CL also makes it clear that we currently do not support content URIs that already have embedded user IDs. Since IActivityManager#grantUriPermissionFromOwner() does not support such URIs, we should have had a special handling for such a case, which will be addressed in a subsequent CL. Bug: 32427307 Bug: 32778718 Test: Made sure that Commit Content API works as expected on a managed profile created by https://github.com/googlesamples/android-testdpc Change-Id: I19d87fc19beea248f49b59ec5a5711b95bcbb466 --- .../android/view/inputmethod/InputContentInfo.java | 35 +++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/core/java/android/view/inputmethod/InputContentInfo.java b/core/java/android/view/inputmethod/InputContentInfo.java index b39705e0b1fa..001f6534c761 100644 --- a/core/java/android/view/inputmethod/InputContentInfo.java +++ b/core/java/android/view/inputmethod/InputContentInfo.java @@ -18,11 +18,14 @@ package android.view.inputmethod; import android.annotation.NonNull; import android.annotation.Nullable; +import android.annotation.UserIdInt; import android.content.ClipDescription; +import android.content.ContentProvider; import android.net.Uri; import android.os.Parcel; import android.os.Parcelable; import android.os.RemoteException; +import android.os.UserHandle; import com.android.internal.inputmethod.IInputContentUriToken; @@ -33,8 +36,21 @@ import java.security.InvalidParameterException; */ public final class InputContentInfo implements Parcelable { + /** + * The content URI that never has a user ID embedded by + * {@link ContentProvider#maybeAddUserId(Uri, int)}. + */ @NonNull private final Uri mContentUri; + /** + * The user ID to which {@link #mContentUri} belongs to. This is always determined based on + * the process ID when the constructor is called. + * + *

CAUTION: If you received {@link InputContentInfo} from a different process, there is no + * guarantee that this value is correct and valid. Never use this for any security purpose

+ */ + @UserIdInt + private final int mContentUriOwnerUserId; @NonNull private final ClipDescription mDescription; @Nullable @@ -73,6 +89,7 @@ public final class InputContentInfo implements Parcelable { @Nullable Uri linkUri) { validateInternal(contentUri, description, linkUri, true /* throwException */); mContentUri = contentUri; + mContentUriOwnerUserId = UserHandle.myUserId(); mDescription = description; mLinkUri = linkUri; } @@ -121,6 +138,13 @@ public final class InputContentInfo implements Parcelable { } return false; } + if (ContentProvider.uriHasUserId(contentUri)) { + if (throwException) { + throw new InvalidParameterException( + "contentUri with a user ID is not currently supported"); + } + return false; + } if (linkUri != null) { final String scheme = linkUri.getScheme(); if (scheme == null || @@ -139,7 +163,14 @@ public final class InputContentInfo implements Parcelable { * @return Content URI with which the content can be obtained. */ @NonNull - public Uri getContentUri() { return mContentUri; } + public Uri getContentUri() { + // Fix up the content URI when and only when the caller's user ID does not match the owner's + // user ID. + if (mContentUriOwnerUserId != UserHandle.myUserId()) { + return ContentProvider.maybeAddUserId(mContentUri, mContentUriOwnerUserId); + } + return mContentUri; + } /** * @return {@link ClipDescription} object that contains the metadata of {@code #getContentUri()} @@ -203,6 +234,7 @@ public final class InputContentInfo implements Parcelable { @Override public void writeToParcel(Parcel dest, int flags) { Uri.writeToParcel(dest, mContentUri); + dest.writeInt(mContentUriOwnerUserId); mDescription.writeToParcel(dest, flags); Uri.writeToParcel(dest, mLinkUri); if (mUriToken != null) { @@ -215,6 +247,7 @@ public final class InputContentInfo implements Parcelable { private InputContentInfo(@NonNull Parcel source) { mContentUri = Uri.CREATOR.createFromParcel(source); + mContentUriOwnerUserId = source.readInt(); mDescription = ClipDescription.CREATOR.createFromParcel(source); mLinkUri = Uri.CREATOR.createFromParcel(source); if (source.readInt() == 1) { -- cgit v1.2.3-59-g8ed1b From 3933a6e0c358238302e07bc8782d65f672b88e7b Mon Sep 17 00:00:00 2001 From: Yohei Yukawa Date: Thu, 10 Nov 2016 00:47:48 -0800 Subject: Support content URIs w/ userId in IC#commitContent With this CL, one can specify a content URI with an embedded user ID to InputContentInfo, like such a URI is supported in Context#grantUriPermission(). Note that such a scenario is actually possible when 1) an application running as User X sets a content URI to the system clipboard then 2) the IME runing as User Y who share the clipboard with User X obtains the content URI from the system and tries to create a new instance of InputContentInfo. Bug: 32427307 Bug: 32778718 Test: 'adb shell dumpsys activity permissions' with a custom IME that instantiates InputContentInfo from the content URI obtained from the clipboard. Change-Id: I7918c0a379b8f3e7e64b106447b42447876f9057 --- .../android/view/inputmethod/InputContentInfo.java | 21 +++++++++------------ .../android/server/InputMethodManagerService.java | 17 +++++++++++++++-- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/core/java/android/view/inputmethod/InputContentInfo.java b/core/java/android/view/inputmethod/InputContentInfo.java index 001f6534c761..7104a2871f21 100644 --- a/core/java/android/view/inputmethod/InputContentInfo.java +++ b/core/java/android/view/inputmethod/InputContentInfo.java @@ -37,14 +37,17 @@ import java.security.InvalidParameterException; public final class InputContentInfo implements Parcelable { /** - * The content URI that never has a user ID embedded by - * {@link ContentProvider#maybeAddUserId(Uri, int)}. + * The content URI that may or may not have a user ID embedded by + * {@link ContentProvider#maybeAddUserId(Uri, int)}. This always preserves the exact value + * specified to a constructor. In other words, if it had user ID embedded when it was passed + * to the constructor, it still has the same user ID no matter if it is valid or not. */ @NonNull private final Uri mContentUri; /** - * The user ID to which {@link #mContentUri} belongs to. This is always determined based on - * the process ID when the constructor is called. + * The user ID to which {@link #mContentUri} belongs to. If {@link #mContentUri} already + * embedded the user ID when it was specified then this fields has the same user ID. Otherwise + * the user ID is determined based on the process ID when the constructor is called. * *

CAUTION: If you received {@link InputContentInfo} from a different process, there is no * guarantee that this value is correct and valid. Never use this for any security purpose

@@ -89,7 +92,8 @@ public final class InputContentInfo implements Parcelable { @Nullable Uri linkUri) { validateInternal(contentUri, description, linkUri, true /* throwException */); mContentUri = contentUri; - mContentUriOwnerUserId = UserHandle.myUserId(); + mContentUriOwnerUserId = + ContentProvider.getUserIdFromUri(mContentUri, UserHandle.myUserId()); mDescription = description; mLinkUri = linkUri; } @@ -138,13 +142,6 @@ public final class InputContentInfo implements Parcelable { } return false; } - if (ContentProvider.uriHasUserId(contentUri)) { - if (throwException) { - throw new InvalidParameterException( - "contentUri with a user ID is not currently supported"); - } - return false; - } if (linkUri != null) { final String scheme = linkUri.getScheme(); if (scheme == null || diff --git a/services/core/java/com/android/server/InputMethodManagerService.java b/services/core/java/com/android/server/InputMethodManagerService.java index 2bf5866e22f5..d1f07a505359 100644 --- a/services/core/java/com/android/server/InputMethodManagerService.java +++ b/services/core/java/com/android/server/InputMethodManagerService.java @@ -60,6 +60,7 @@ import android.app.NotificationManager; import android.app.PendingIntent; import android.content.BroadcastReceiver; import android.content.ComponentName; +import android.content.ContentProvider; import android.content.ContentResolver; import android.content.Context; import android.content.DialogInterface; @@ -3969,10 +3970,22 @@ public class InputMethodManagerService extends IInputMethodManager.Stub + mCurAttribute.packageName + " packageName=" + packageName); return null; } + // This user ID can never bee spoofed. final int imeUserId = UserHandle.getUserId(uid); + // This user ID can never bee spoofed. final int appUserId = UserHandle.getUserId(mCurClient.uid); - return new InputContentUriTokenHandler(contentUri, uid, packageName, imeUserId, - appUserId); + // This user ID may be invalid if "contentUri" embedded an invalid user ID. + final int contentUriOwnerUserId = ContentProvider.getUserIdFromUri(contentUri, + imeUserId); + final Uri contentUriWithoutUserId = ContentProvider.getUriWithoutUserId(contentUri); + // Note: InputContentUriTokenHandler.take() checks whether the IME (specified by "uid") + // actually has the right to grant a read permission for "contentUriWithoutUserId" that + // is claimed to belong to "contentUriOwnerUserId". For example, specifying random + // content URI and/or contentUriOwnerUserId just results in a SecurityException thrown + // from InputContentUriTokenHandler.take() and can never be allowed beyond what is + // actually allowed to "uid", which is guaranteed to be the IME's one. + return new InputContentUriTokenHandler(contentUriWithoutUserId, uid, + packageName, contentUriOwnerUserId, appUserId); } } -- cgit v1.2.3-59-g8ed1b