From 710d720db7a456c51f313f7d919424761e940ae1 Mon Sep 17 00:00:00 2001 From: Patrick Baumann Date: Thu, 9 Jan 2020 15:02:14 -0800 Subject: [PATCH] Grants visibility on uri permission grant When permission is granted to another app via URI, we implicitly grant visibility to that app of the app ID that that URI resolves to. Test: atest AppSecurityTests Fixes: 149781706 Fixes: 145677500 Exempt-From-Owner-Approval: Owner approved prior to cherry-pick Change-Id: I7c8967a4464fd821e4f95d8eb6c0bcfadadb912e --- .../content/pm/PackageManagerInternal.java | 20 +++++++++++----- .../server/am/ActivityManagerService.java | 4 ++-- .../com/android/server/pm/AppsFilter.java | 13 +++++------ .../android/server/pm/InstantAppRegistry.java | 8 +++---- .../server/pm/PackageManagerService.java | 23 +++++++++++-------- .../server/uri/UriGrantsManagerService.java | 2 ++ .../server/webkit/WebViewUpdateService.java | 5 ++-- .../android/server/wm/ActivityStarter.java | 5 ++-- .../server/wm/ActivityStarterTests.java | 2 +- 9 files changed, 48 insertions(+), 34 deletions(-) diff --git a/services/core/java/android/content/pm/PackageManagerInternal.java b/services/core/java/android/content/pm/PackageManagerInternal.java index 1230bd79ff976..1b1e06a9a92f7 100644 --- a/services/core/java/android/content/pm/PackageManagerInternal.java +++ b/services/core/java/android/content/pm/PackageManagerInternal.java @@ -458,8 +458,8 @@ public abstract class PackageManagerInternal { Bundle verificationBundle, int userId); /** - * Grants implicit access based on an interaction between two apps. This grants the target app - * access to the calling application's package metadata. + * Grants implicit access based on an interaction between two apps. This grants access to the + * from one application to the other's package metadata. *

* When an application explicitly tries to interact with another application [via an * activity, service or provider that is either declared in the caller's @@ -468,14 +468,22 @@ public abstract class PackageManagerInternal { * metadata about the calling app. If the calling application uses an implicit intent [ie * action VIEW, category BROWSABLE], it remains hidden from the launched app. *

+ * If an interaction is not explicit, the {@code direct} argument should be set to false as + * visibility should not be granted in some cases. This method handles that logic. + *

* @param userId the user * @param intent the intent that triggered the grant - * @param callingUid The uid of the calling application - * @param targetAppId The app ID of the target application + * @param recipientAppId The app ID of the application that is being given access to {@code + * visibleUid} + * @param visibleUid The uid of the application that is becoming accessible to {@code + * recipientAppId} + * @param direct true if the access is being made due to direct interaction between visibleUid + * and recipientAppId. */ public abstract void grantImplicitAccess( - @UserIdInt int userId, Intent intent, int callingUid, - @AppIdInt int targetAppId); + @UserIdInt int userId, Intent intent, + @AppIdInt int recipientAppId, int visibleUid, + boolean direct); public abstract boolean isInstantAppInstallerComponent(ComponentName component); /** diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index ca0b03dff259e..d4f97dc42fbcf 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -6285,9 +6285,9 @@ public class ActivityManagerService extends IActivityManager.Stub } @VisibleForTesting - public void grantImplicitAccess(int userId, Intent intent, int callingUid, int targetAppId) { + public void grantImplicitAccess(int userId, Intent intent, int visibleUid, int recipientAppId) { getPackageManagerInternalLocked(). - grantImplicitAccess(userId, intent, callingUid, targetAppId); + grantImplicitAccess(userId, intent, recipientAppId, visibleUid, true /*direct*/); } /** diff --git a/services/core/java/com/android/server/pm/AppsFilter.java b/services/core/java/com/android/server/pm/AppsFilter.java index ae6e058ee9313..0ad0b2373a797 100644 --- a/services/core/java/com/android/server/pm/AppsFilter.java +++ b/services/core/java/com/android/server/pm/AppsFilter.java @@ -354,14 +354,13 @@ public class AppsFilter { * Grants access based on an interaction between a calling and target package, granting * visibility of the caller from the target. * - * @param callingUid the uid initiating the interaction - * @param targetUid the uid being interacted with and thus gaining visibility of the - * initiating uid. + * @param recipientUid the uid gaining visibility of the {@code visibleUid}. + * @param visibleUid the uid becoming visible to the {@recipientUid} */ - public void grantImplicitAccess(int callingUid, int targetUid) { - if (targetUid != callingUid - && mImplicitlyQueryable.add(targetUid, callingUid) && DEBUG_LOGGING) { - Slog.wtf(TAG, "implicit access granted: " + targetUid + " -> " + callingUid); + public void grantImplicitAccess(int recipientUid, int visibleUid) { + if (recipientUid != visibleUid + && mImplicitlyQueryable.add(recipientUid, visibleUid) && DEBUG_LOGGING) { + Slog.wtf(TAG, "implicit access granted: " + recipientUid + " -> " + visibleUid); } } diff --git a/services/core/java/com/android/server/pm/InstantAppRegistry.java b/services/core/java/com/android/server/pm/InstantAppRegistry.java index cf85b0f3da7ad..0eaac4140c14b 100644 --- a/services/core/java/com/android/server/pm/InstantAppRegistry.java +++ b/services/core/java/com/android/server/pm/InstantAppRegistry.java @@ -403,7 +403,7 @@ class InstantAppRegistry { @GuardedBy("mService.mLock") public void grantInstantAccessLPw(@UserIdInt int userId, @Nullable Intent intent, - int instantAppId, int targetAppId) { + int recipientUid, int instantAppId) { if (mInstalledInstantAppUids == null) { return; // no instant apps installed; no need to grant } @@ -411,7 +411,7 @@ class InstantAppRegistry { if (instantAppList == null || !instantAppList.get(instantAppId)) { return; // instant app id isn't installed; no need to grant } - if (instantAppList.get(targetAppId)) { + if (instantAppList.get(recipientUid)) { return; // target app id is an instant app; no need to grant } if (intent != null && Intent.ACTION_VIEW.equals(intent.getAction())) { @@ -428,10 +428,10 @@ class InstantAppRegistry { targetAppList = new SparseArray<>(); mInstantGrants.put(userId, targetAppList); } - SparseBooleanArray instantGrantList = targetAppList.get(targetAppId); + SparseBooleanArray instantGrantList = targetAppList.get(recipientUid); if (instantGrantList == null) { instantGrantList = new SparseBooleanArray(); - targetAppList.put(targetAppId, instantGrantList); + targetAppList.put(recipientUid, instantGrantList); } instantGrantList.put(instantAppId, true /*granted*/); } diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 077fc6f014295..01de751a61728 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -23566,22 +23566,27 @@ public class PackageManagerService extends IPackageManager.Stub @Override public void grantImplicitAccess(int userId, Intent intent, - int callingUid, int targetAppId) { + int recipientAppId, int visibleUid, boolean direct) { synchronized (mLock) { - final AndroidPackage callingPackage = getPackage(callingUid); - final int targetUid = UserHandle.getUid(userId, targetAppId); - final AndroidPackage targetPackage = getPackage(targetUid); - if (callingPackage == null || targetPackage == null) { + final AndroidPackage visiblePackage = getPackage(visibleUid); + final int recipientUid = UserHandle.getUid(userId, recipientAppId); + if (visiblePackage == null || getPackage(recipientUid) == null) { return; } - final boolean instantApp = isInstantAppInternal(callingPackage.getPackageName(), - userId, callingUid); + final boolean instantApp = + isInstantAppInternal(visiblePackage.getPackageName(), userId, visibleUid); if (instantApp) { + if (!direct) { + // if the interaction that lead to this granting access to an instant app + // was indirect (i.e.: URI permission grant), do not actually execute the + // grant. + return; + } mInstantAppRegistry.grantInstantAccessLPw(userId, intent, - UserHandle.getAppId(callingUid), targetAppId); + recipientAppId, UserHandle.getAppId(visibleUid) /*instantAppId*/); } else { - mAppsFilter.grantImplicitAccess(callingUid, targetUid); + mAppsFilter.grantImplicitAccess(recipientUid, visibleUid); } } } diff --git a/services/core/java/com/android/server/uri/UriGrantsManagerService.java b/services/core/java/com/android/server/uri/UriGrantsManagerService.java index e3b7c0aae5072..fe34e86a27ae8 100644 --- a/services/core/java/com/android/server/uri/UriGrantsManagerService.java +++ b/services/core/java/com/android/server/uri/UriGrantsManagerService.java @@ -739,6 +739,8 @@ public class UriGrantsManagerService extends IUriGrantsManager.Stub { final UriPermission perm = findOrCreateUriPermission( pi.packageName, targetPkg, targetUid, grantUri); perm.grantModes(modeFlags, owner); + getPmInternal().grantImplicitAccess(UserHandle.getUserId(targetUid), null, + UserHandle.getAppId(targetUid), pi.applicationInfo.uid, false /*direct*/); } /** Like grantUriPermissionUnchecked, but takes an Intent. */ diff --git a/services/core/java/com/android/server/webkit/WebViewUpdateService.java b/services/core/java/com/android/server/webkit/WebViewUpdateService.java index 8130546e26990..9bbeb728ae33b 100644 --- a/services/core/java/com/android/server/webkit/WebViewUpdateService.java +++ b/services/core/java/com/android/server/webkit/WebViewUpdateService.java @@ -211,8 +211,9 @@ public class WebViewUpdateService extends SystemService { PackageManagerInternal.class); final int webviewUid = pmInternal.getPackageUidInternal( webViewPackageName, 0, UserHandle.getUserId(callingUid)); - pmInternal.grantImplicitAccess(UserHandle.getUserId(callingUid), null, webviewUid, - UserHandle.getAppId(callingUid)); + pmInternal.grantImplicitAccess(UserHandle.getUserId(callingUid), null, + UserHandle.getAppId(callingUid), webviewUid, + true /*direct*/); } /** diff --git a/services/core/java/com/android/server/wm/ActivityStarter.java b/services/core/java/com/android/server/wm/ActivityStarter.java index 600a125cdf43a..a46c30b7586ce 100644 --- a/services/core/java/com/android/server/wm/ActivityStarter.java +++ b/services/core/java/com/android/server/wm/ActivityStarter.java @@ -1558,9 +1558,8 @@ class ActivityStarter { mIntent, mStartActivity.getUriPermissionsLocked(), mStartActivity.mUserId); mService.getPackageManagerInternalLocked().grantImplicitAccess( mStartActivity.mUserId, mIntent, - mCallingUid, - UserHandle.getAppId(mStartActivity.info.applicationInfo.uid) - ); + UserHandle.getAppId(mStartActivity.info.applicationInfo.uid), mCallingUid, + true /*direct*/); if (newTask) { EventLogTags.writeWmCreateTask(mStartActivity.mUserId, mStartActivity.getTask().mTaskId); diff --git a/services/tests/wmtests/src/com/android/server/wm/ActivityStarterTests.java b/services/tests/wmtests/src/com/android/server/wm/ActivityStarterTests.java index b917e1b92fb91..83c09027f70d5 100644 --- a/services/tests/wmtests/src/com/android/server/wm/ActivityStarterTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/ActivityStarterTests.java @@ -383,7 +383,7 @@ public class ActivityStarterTests extends ActivityTestsBase { // Never review permissions doReturn(false).when(mMockPackageManager).isPermissionsReviewRequired(any(), anyInt()); doNothing().when(mMockPackageManager).grantImplicitAccess( - anyInt(), any(), anyInt(), anyInt()); + anyInt(), any(), anyInt(), anyInt(), anyBoolean()); doNothing().when(mMockPackageManager).notifyPackageUse(anyString(), anyInt()); final Intent intent = new Intent();