From 54d82637acbb9458caec4a384c75b84ce414224c Mon Sep 17 00:00:00 2001 From: Winson Date: Tue, 25 May 2021 16:20:24 -0700 Subject: [PATCH] Handle general and specific cross profile logic Previous resolve had a weird behavior where a specific IntentFilter configured cross profile filter configuration (as opposed to a general configuration requiring allow_parent_profile_app_linking to be toggled) would be added into the candidate set, which the code assumed was only for the user ID being queried. Instead, this specific cross profile resolution needs to be kept separate as a different user, and applied if when the general resolution is unavailable. This makes both specific and general branchs assign the same CrossProfileDomainInfo that domain verification was already checking, and so should allow the logic to work in cases where the specific info was previously dropped. Bug: 189222753 Test: CtsDomainVerificationDeviceMultiUserTestCases Change-Id: I0ac12d7125a7c5a9d4b9b35692d13928cbeb84e3 --- .../content/pm/verify/domain/TEST_MAPPING | 5 +- .../server/pm/PackageManagerService.java | 256 +++++++++++------- .../DomainVerificationManagerInternal.java | 1 - .../domain/DomainVerificationService.java | 22 +- .../server/pm/verify/domain/TEST_MAPPING | 5 +- 5 files changed, 191 insertions(+), 98 deletions(-) diff --git a/core/java/android/content/pm/verify/domain/TEST_MAPPING b/core/java/android/content/pm/verify/domain/TEST_MAPPING index 5fcf4118ddd1b..ba4a62cdbbf1d 100644 --- a/core/java/android/content/pm/verify/domain/TEST_MAPPING +++ b/core/java/android/content/pm/verify/domain/TEST_MAPPING @@ -9,7 +9,10 @@ ] }, { - "name": "CtsDomainVerificationDeviceTestCases" + "name": "CtsDomainVerificationDeviceStandaloneTestCases" + }, + { + "name": "CtsDomainVerificationDeviceMultiUserTestCases" }, { "name": "CtsDomainVerificationHostTestCases" diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index b84ca06e7ac9f..5388f3d86f3f0 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -2332,11 +2332,11 @@ public class PackageManagerService extends IPackageManager.Stub List matchingFilters = getMatchingCrossProfileIntentFilters(intent, resolvedType, userId); // Check for results that need to skip the current profile. - ResolveInfo xpResolveInfo = querySkipCurrentProfileIntents(matchingFilters, intent, - resolvedType, flags, userId); - if (xpResolveInfo != null) { + ResolveInfo skipProfileInfo = querySkipCurrentProfileIntents(matchingFilters, + intent, resolvedType, flags, userId); + if (skipProfileInfo != null) { List xpResult = new ArrayList<>(1); - xpResult.add(xpResolveInfo); + xpResult.add(skipProfileInfo); return new QueryIntentActivitiesResult( applyPostResolutionFilter( filterIfNotSystemUser(xpResult, userId), instantAppPkgName, @@ -2351,54 +2351,55 @@ public class PackageManagerService extends IPackageManager.Stub false /*skipPackageCheck*/, flags); // Check for cross profile results. boolean hasNonNegativePriorityResult = hasNonNegativePriority(result); - xpResolveInfo = queryCrossProfileIntents( + CrossProfileDomainInfo specificXpInfo = queryCrossProfileIntents( matchingFilters, intent, resolvedType, flags, userId, hasNonNegativePriorityResult); - if (xpResolveInfo != null && isUserEnabled(xpResolveInfo.targetUserId)) { - boolean isVisibleToUser = filterIfNotSystemUser( - Collections.singletonList(xpResolveInfo), userId).size() > 0; - if (isVisibleToUser) { - result.add(xpResolveInfo); - sortResult = true; - } - } if (intent.hasWebURI()) { - CrossProfileDomainInfo xpDomainInfo = null; + CrossProfileDomainInfo generalXpInfo = null; final UserInfo parent = getProfileParent(userId); if (parent != null) { - xpDomainInfo = getCrossProfileDomainPreferredLpr(intent, resolvedType, + generalXpInfo = getCrossProfileDomainPreferredLpr(intent, resolvedType, flags, userId, parent.id); } - if (xpDomainInfo != null) { - if (xpResolveInfo != null) { - // If we didn't remove it, the cross-profile ResolveInfo would be twice - // in the result. - result.remove(xpResolveInfo); - } - if (result.size() == 0 && !addInstant) { + + // Generalized cross profile intents take precedence over specific. + // Note that this is the opposite of the intuitive order. + CrossProfileDomainInfo prioritizedXpInfo = + generalXpInfo != null ? generalXpInfo : specificXpInfo; + + if (!addInstant) { + if (result.isEmpty() && prioritizedXpInfo != null) { // No result in current profile, but found candidate in parent user. // And we are not going to add ephemeral app, so we can return the // result straight away. - result.add(xpDomainInfo.resolveInfo); + result.add(prioritizedXpInfo.resolveInfo); + return new QueryIntentActivitiesResult( + applyPostResolutionFilter(result, instantAppPkgName, + allowDynamicSplits, filterCallingUid, resolveForStart, + userId, intent)); + } else if (result.size() <= 1 && prioritizedXpInfo == null) { + // No result in parent user and <= 1 result in current profile, and we + // are not going to add ephemeral app, so we can return the result + // without further processing. return new QueryIntentActivitiesResult( applyPostResolutionFilter(result, instantAppPkgName, allowDynamicSplits, filterCallingUid, resolveForStart, userId, intent)); } - } else if (result.size() <= 1 && !addInstant) { - // No result in parent user and <= 1 result in current profile, and we - // are not going to add ephemeral app, so we can return the result without - // further processing. - return new QueryIntentActivitiesResult( - applyPostResolutionFilter(result, instantAppPkgName, - allowDynamicSplits, filterCallingUid, resolveForStart, userId, - intent)); } + // We have more than one candidate (combining results from current and parent // profile), so we need filtering and sorting. result = filterCandidatesWithDomainPreferredActivitiesLPr( - intent, flags, result, xpDomainInfo, userId); + intent, flags, result, prioritizedXpInfo, userId); sortResult = true; + } else { + // If not web Intent, just add result to candidate set and let ResolverActivity + // figure it out. + if (specificXpInfo != null) { + result.add(specificXpInfo.resolveInfo); + sortResult = true; + } } } else { final PackageSetting setting = @@ -2830,15 +2831,17 @@ public class PackageManagerService extends IPackageManager.Stub if (ps == null) { continue; } - if (result == null) { - result = new CrossProfileDomainInfo(); - result.resolveInfo = createForwardingResolveInfoUnchecked( - new WatchedIntentFilter(), sourceUserId, parentUserId); - } - result.highestApprovalLevel = Math.max(mDomainVerificationManager - .approvalLevelForDomain(ps, intent, resultTargetUser, flags, - parentUserId), result.highestApprovalLevel); + int approvalLevel = mDomainVerificationManager + .approvalLevelForDomain(ps, intent, flags, parentUserId); + + if (result == null) { + result = new CrossProfileDomainInfo(createForwardingResolveInfoUnchecked( + new WatchedIntentFilter(), sourceUserId, parentUserId), approvalLevel); + } else { + result.highestApprovalLevel = + Math.max(approvalLevel, result.highestApprovalLevel); + } } if (result != null && result.highestApprovalLevel <= DomainVerificationManagerInternal.APPROVAL_LEVEL_NONE) { @@ -3085,8 +3088,8 @@ public class PackageManagerService extends IPackageManager.Stub final String packageName = info.activityInfo.packageName; final PackageSetting ps = mSettings.getPackageLPr(packageName); if (ps.getInstantApp(userId)) { - if (hasAnyDomainApproval(mDomainVerificationManager, ps, intent, - instantApps, flags, userId)) { + if (hasAnyDomainApproval(mDomainVerificationManager, ps, intent, flags, + userId)) { if (DEBUG_INSTANT) { Slog.v(TAG, "Instant app approved for intent; pkg: " + packageName); @@ -3413,28 +3416,59 @@ public class PackageManagerService extends IPackageManager.Stub } /** - * If the filter's target user can handle the intent and is enabled: returns a ResolveInfo - * that - * will forward the intent to the filter's target user. - * Otherwise, returns null. + * If the filter's target user can handle the intent and is enabled: a [ResolveInfo] that + * will forward the intent to the filter's target user, along with the highest approval of + * any handler in the target user. Otherwise, returns null. */ - private ResolveInfo createForwardingResolveInfo(CrossProfileIntentFilter filter, - Intent intent, - String resolvedType, int flags, int sourceUserId) { + @Nullable + private CrossProfileDomainInfo createForwardingResolveInfo( + @NonNull CrossProfileIntentFilter filter, @NonNull Intent intent, + @Nullable String resolvedType, int flags, int sourceUserId) { int targetUserId = filter.getTargetUserId(); + if (!isUserEnabled(targetUserId)) { + return null; + } + List resultTargetUser = mComponentResolver.queryActivities(intent, resolvedType, flags, targetUserId); - if (resultTargetUser != null && isUserEnabled(targetUserId)) { - // If all the matches in the target profile are suspended, return null. - for (int i = resultTargetUser.size() - 1; i >= 0; i--) { - if ((resultTargetUser.get(i).activityInfo.applicationInfo.flags - & ApplicationInfo.FLAG_SUSPENDED) == 0) { - return createForwardingResolveInfoUnchecked(filter, - sourceUserId, targetUserId); - } + if (CollectionUtils.isEmpty(resultTargetUser)) { + return null; + } + + ResolveInfo forwardingInfo = null; + for (int i = resultTargetUser.size() - 1; i >= 0; i--) { + ResolveInfo targetUserResolveInfo = resultTargetUser.get(i); + if ((targetUserResolveInfo.activityInfo.applicationInfo.flags + & ApplicationInfo.FLAG_SUSPENDED) == 0) { + forwardingInfo = createForwardingResolveInfoUnchecked(filter, sourceUserId, + targetUserId); + break; } } - return null; + + if (forwardingInfo == null) { + // If all the matches in the target profile are suspended, return null. + return null; + } + + int highestApprovalLevel = DomainVerificationManagerInternal.APPROVAL_LEVEL_NONE; + + int size = resultTargetUser.size(); + for (int i = 0; i < size; i++) { + ResolveInfo riTargetUser = resultTargetUser.get(i); + if (riTargetUser.handleAllWebDataURI) { + continue; + } + String packageName = riTargetUser.activityInfo.packageName; + PackageSetting ps = mSettings.getPackageLPr(packageName); + if (ps == null) { + continue; + } + highestApprovalLevel = Math.max(highestApprovalLevel, mDomainVerificationManager + .approvalLevelForDomain(ps, intent, flags, targetUserId)); + } + + return new CrossProfileDomainInfo(forwardingInfo, highestApprovalLevel); } public ResolveInfo createForwardingResolveInfoUnchecked(WatchedIntentFilter filter, @@ -3473,34 +3507,59 @@ public class PackageManagerService extends IPackageManager.Stub } // Return matching ResolveInfo in target user if any. - private ResolveInfo queryCrossProfileIntents( + @Nullable + private CrossProfileDomainInfo queryCrossProfileIntents( List matchingFilters, Intent intent, String resolvedType, int flags, int sourceUserId, boolean matchInCurrentProfile) { - if (matchingFilters != null) { - // Two {@link CrossProfileIntentFilter}s can have the same targetUserId and - // match the same intent. For performance reasons, it is better not to - // run queryIntent twice for the same userId - SparseBooleanArray alreadyTriedUserIds = new SparseBooleanArray(); - int size = matchingFilters.size(); - for (int i = 0; i < size; i++) { - CrossProfileIntentFilter filter = matchingFilters.get(i); - int targetUserId = filter.getTargetUserId(); - boolean skipCurrentProfile = - (filter.getFlags() & PackageManager.SKIP_CURRENT_PROFILE) != 0; - boolean skipCurrentProfileIfNoMatchFound = - (filter.getFlags() & PackageManager.ONLY_IF_NO_MATCH_FOUND) != 0; - if (!skipCurrentProfile && !alreadyTriedUserIds.get(targetUserId) - && (!skipCurrentProfileIfNoMatchFound || !matchInCurrentProfile)) { - // Checking if there are activities in the target user that can handle the - // intent. - ResolveInfo resolveInfo = createForwardingResolveInfo(filter, intent, - resolvedType, flags, sourceUserId); - if (resolveInfo != null) return resolveInfo; - alreadyTriedUserIds.put(targetUserId, true); + if (matchingFilters == null) { + return null; + } + // Two {@link CrossProfileIntentFilter}s can have the same targetUserId and + // match the same intent. For performance reasons, it is better not to + // run queryIntent twice for the same userId + SparseBooleanArray alreadyTriedUserIds = new SparseBooleanArray(); + + CrossProfileDomainInfo resultInfo = null; + + int size = matchingFilters.size(); + for (int i = 0; i < size; i++) { + CrossProfileIntentFilter filter = matchingFilters.get(i); + int targetUserId = filter.getTargetUserId(); + boolean skipCurrentProfile = + (filter.getFlags() & PackageManager.SKIP_CURRENT_PROFILE) != 0; + boolean skipCurrentProfileIfNoMatchFound = + (filter.getFlags() & PackageManager.ONLY_IF_NO_MATCH_FOUND) != 0; + if (!skipCurrentProfile && !alreadyTriedUserIds.get(targetUserId) + && (!skipCurrentProfileIfNoMatchFound || !matchInCurrentProfile)) { + // Checking if there are activities in the target user that can handle the + // intent. + CrossProfileDomainInfo info = createForwardingResolveInfo(filter, intent, + resolvedType, flags, sourceUserId); + if (info != null) { + resultInfo = info; + break; } + alreadyTriedUserIds.put(targetUserId, true); } } - return null; + + if (resultInfo == null) { + return null; + } + + ResolveInfo forwardingResolveInfo = resultInfo.resolveInfo; + if (!isUserEnabled(forwardingResolveInfo.targetUserId)) { + return null; + } + + List filteredResult = + filterIfNotSystemUser(Collections.singletonList(forwardingResolveInfo), + sourceUserId); + if (filteredResult.isEmpty()) { + return null; + } + + return resultInfo; } private ResolveInfo querySkipCurrentProfileIntents( @@ -3513,10 +3572,10 @@ public class PackageManagerService extends IPackageManager.Stub if ((filter.getFlags() & PackageManager.SKIP_CURRENT_PROFILE) != 0) { // Checking if there are activities in the target user that can handle the // intent. - ResolveInfo resolveInfo = createForwardingResolveInfo(filter, intent, + CrossProfileDomainInfo info = createForwardingResolveInfo(filter, intent, resolvedType, flags, sourceUserId); - if (resolveInfo != null) { - return resolveInfo; + if (info != null) { + return info.resolveInfo; } } } @@ -4026,8 +4085,8 @@ public class PackageManagerService extends IPackageManager.Stub if (ps != null) { // only check domain verification status if the app is not a browser if (!info.handleAllWebDataURI) { - if (hasAnyDomainApproval(mDomainVerificationManager, ps, intent, - resolvedActivities, flags, userId)) { + if (hasAnyDomainApproval(mDomainVerificationManager, ps, intent, flags, + userId)) { if (DEBUG_INSTANT) { Slog.v(TAG, "DENY instant app;" + " pkg: " + packageName + ", approved"); @@ -9525,7 +9584,7 @@ public class PackageManagerService extends IPackageManager.Stub final String packageName = ri.activityInfo.packageName; final PackageSetting ps = mSettings.getPackageLPr(packageName); if (ps != null && hasAnyDomainApproval(mDomainVerificationManager, ps, - intent, query, flags, userId)) { + intent, flags, userId)) { return ri; } } @@ -9582,10 +9641,10 @@ public class PackageManagerService extends IPackageManager.Stub */ private static boolean hasAnyDomainApproval( @NonNull DomainVerificationManagerInternal manager, @NonNull PackageSetting pkgSetting, - @NonNull Intent intent, @NonNull List candidates, - @PackageManager.ResolveInfoFlags int resolveInfoFlags, @UserIdInt int userId) { - return manager.approvalLevelForDomain(pkgSetting, intent, candidates, resolveInfoFlags, - userId) > DomainVerificationManagerInternal.APPROVAL_LEVEL_NONE; + @NonNull Intent intent, @PackageManager.ResolveInfoFlags int resolveInfoFlags, + @UserIdInt int userId) { + return manager.approvalLevelForDomain(pkgSetting, intent, resolveInfoFlags, userId) + > DomainVerificationManagerInternal.APPROVAL_LEVEL_NONE; } /** @@ -10018,7 +10077,20 @@ public class PackageManagerService extends IPackageManager.Stub private static class CrossProfileDomainInfo { /* ResolveInfo for IntentForwarderActivity to send the intent to the other profile */ ResolveInfo resolveInfo; - int highestApprovalLevel = DomainVerificationManagerInternal.APPROVAL_LEVEL_NONE; + int highestApprovalLevel; + + CrossProfileDomainInfo(ResolveInfo resolveInfo, int highestApprovalLevel) { + this.resolveInfo = resolveInfo; + this.highestApprovalLevel = highestApprovalLevel; + } + + @Override + public String toString() { + return "CrossProfileDomainInfo{" + + "resolveInfo=" + resolveInfo + + ", highestApprovalLevel=" + highestApprovalLevel + + '}'; + } } private CrossProfileDomainInfo getCrossProfileDomainPreferredLpr(Intent intent, diff --git a/services/core/java/com/android/server/pm/verify/domain/DomainVerificationManagerInternal.java b/services/core/java/com/android/server/pm/verify/domain/DomainVerificationManagerInternal.java index 65e4e95759b8c..262734fe18b41 100644 --- a/services/core/java/com/android/server/pm/verify/domain/DomainVerificationManagerInternal.java +++ b/services/core/java/com/android/server/pm/verify/domain/DomainVerificationManagerInternal.java @@ -389,7 +389,6 @@ public interface DomainVerificationManagerInternal { */ @ApprovalLevel int approvalLevelForDomain(@NonNull PackageSetting pkgSetting, @NonNull Intent intent, - @NonNull List candidates, @PackageManager.ResolveInfoFlags int resolveInfoFlags, @UserIdInt int userId); /** diff --git a/services/core/java/com/android/server/pm/verify/domain/DomainVerificationService.java b/services/core/java/com/android/server/pm/verify/domain/DomainVerificationService.java index b1b4e2afeb497..ba64d25178e76 100644 --- a/services/core/java/com/android/server/pm/verify/domain/DomainVerificationService.java +++ b/services/core/java/com/android/server/pm/verify/domain/DomainVerificationService.java @@ -1717,7 +1717,6 @@ public class DomainVerificationService extends SystemService @Override public int approvalLevelForDomain(@NonNull PackageSetting pkgSetting, @NonNull Intent intent, - @NonNull List candidates, @PackageManager.ResolveInfoFlags int resolveInfoFlags, @UserIdInt int userId) { String packageName = pkgSetting.getName(); if (!DomainVerificationUtils.isDomainVerificationIntent(intent, resolveInfoFlags)) { @@ -1783,9 +1782,26 @@ public class DomainVerificationService extends SystemService return APPROVAL_LEVEL_NONE; } - if (!pkgUserState.installed || !pkgUserState.isPackageEnabled(pkg)) { + if (!pkgUserState.installed) { if (DEBUG_APPROVAL) { - debugApproval(packageName, debugObject, userId, false, "package not enabled"); + debugApproval(packageName, debugObject, userId, false, + "package not installed for user"); + } + return APPROVAL_LEVEL_NONE; + } + + if (!pkgUserState.isPackageEnabled(pkg)) { + if (DEBUG_APPROVAL) { + debugApproval(packageName, debugObject, userId, false, + "package not enabled for user"); + } + return APPROVAL_LEVEL_NONE; + } + + if (pkgUserState.suspended) { + if (DEBUG_APPROVAL) { + debugApproval(packageName, debugObject, userId, false, + "package suspended for user"); } return APPROVAL_LEVEL_NONE; } diff --git a/services/core/java/com/android/server/pm/verify/domain/TEST_MAPPING b/services/core/java/com/android/server/pm/verify/domain/TEST_MAPPING index 5fcf4118ddd1b..ba4a62cdbbf1d 100644 --- a/services/core/java/com/android/server/pm/verify/domain/TEST_MAPPING +++ b/services/core/java/com/android/server/pm/verify/domain/TEST_MAPPING @@ -9,7 +9,10 @@ ] }, { - "name": "CtsDomainVerificationDeviceTestCases" + "name": "CtsDomainVerificationDeviceStandaloneTestCases" + }, + { + "name": "CtsDomainVerificationDeviceMultiUserTestCases" }, { "name": "CtsDomainVerificationHostTestCases"