From 6e908fd487370aae46db4ca03aed3999485b5b04 Mon Sep 17 00:00:00 2001 From: Cassie Wang Date: Wed, 4 Aug 2021 16:18:10 +0000 Subject: [PATCH] Enable cross-user requests. Allow a cross-user request if the caller has INTERACT_ACROSS_USERS_FULL permission. Then, the request will be made in the targetUser's AppSearch instance. USERS_FULL is signature permission which is only given to apps which are signed with the same certificate as the platform. so we trust them more and usually we allow apps with full permission to interact with packages other than it's own on the other users. So switching to USERS_FULLFULL only was because it might be a smaller fix and that only very trusted apps could abuse the security risks we had. The general multi-user bug categories as 1:pre-existing bugs that blocked cross-user: FIXED, b/194413082, b/193902620, b/194939218. 2:enable cross-user: Current CL b/194332010. 3:restrict the cross-user use-cases: b/193423447, b/193423599 Bug: 194332010 Test: atest -m -c --rebuild-module-info CtsAppSearchTestCases FrameworksCoreTests:android.app.appsearch FrameworksServicesTests:com.android.server.appsearch CtsAppSearchHostTestCases Change-Id: I93c083d83612c21a3e6566d42c17db137e90ebe6 --- .../appsearch/AppSearchManagerService.java | 75 ++++++++++++++----- 1 file changed, 55 insertions(+), 20 deletions(-) diff --git a/apex/appsearch/service/java/com/android/server/appsearch/AppSearchManagerService.java b/apex/appsearch/service/java/com/android/server/appsearch/AppSearchManagerService.java index c33d5ecc5d164..710d6b546d9f2 100644 --- a/apex/appsearch/service/java/com/android/server/appsearch/AppSearchManagerService.java +++ b/apex/appsearch/service/java/com/android/server/appsearch/AppSearchManagerService.java @@ -18,6 +18,7 @@ package com.android.server.appsearch; import static android.app.appsearch.AppSearchResult.throwableToFailedResult; import static android.os.Process.INVALID_UID; +import android.Manifest; import android.annotation.ElapsedRealtimeLong; import android.annotation.NonNull; import android.app.appsearch.AppSearchBatchResult; @@ -331,6 +332,7 @@ public class AppSearchManagerService extends SystemService { Objects.requireNonNull(callback); long totalLatencyStartTimeMillis = SystemClock.elapsedRealtime(); + int callingPid = Binder.getCallingPid(); int callingUid = Binder.getCallingUid(); EXECUTOR.execute(() -> { @AppSearchResult.ResultCode int statusCode = AppSearchResult.RESULT_OK; @@ -343,7 +345,7 @@ public class AppSearchManagerService extends SystemService { // Obtain the user where the client wants to run the operations in. This should // end up being the same as userHandle, assuming it is not a special user and // the client is allowed to run operations in that user. - UserHandle targetUser = handleIncomingUser(userHandle, callingUid); + UserHandle targetUser = handleIncomingUser(userHandle, callingPid, callingUid); verifyUserUnlocked(targetUser); List schemas = new ArrayList<>(schemaBundles.size()); @@ -420,6 +422,7 @@ public class AppSearchManagerService extends SystemService { Objects.requireNonNull(userHandle); Objects.requireNonNull(callback); + int callingPid = Binder.getCallingPid(); int callingUid = Binder.getCallingUid(); EXECUTOR.execute(() -> { try { @@ -428,7 +431,7 @@ public class AppSearchManagerService extends SystemService { // Obtain the user where the client wants to run the operations in. This should // end up being the same as userHandle, assuming it is not a special user and // the client is allowed to run operations in that user. - UserHandle targetUser = handleIncomingUser(userHandle, callingUid); + UserHandle targetUser = handleIncomingUser(userHandle, callingPid, callingUid); verifyUserUnlocked(targetUser); AppSearchUserInstance instance = @@ -455,6 +458,7 @@ public class AppSearchManagerService extends SystemService { Objects.requireNonNull(userHandle); Objects.requireNonNull(callback); + int callingPid = Binder.getCallingPid(); int callingUid = Binder.getCallingUid(); EXECUTOR.execute(() -> { try { @@ -463,7 +467,7 @@ public class AppSearchManagerService extends SystemService { // Obtain the user where the client wants to run the operations in. This should // end up being the same as userHandle, assuming it is not a special user and // the client is allowed to run operations in that user. - UserHandle targetUser = handleIncomingUser(userHandle, callingUid); + UserHandle targetUser = handleIncomingUser(userHandle, callingPid, callingUid); verifyUserUnlocked(targetUser); AppSearchUserInstance instance = @@ -493,6 +497,7 @@ public class AppSearchManagerService extends SystemService { Objects.requireNonNull(callback); long totalLatencyStartTimeMillis = SystemClock.elapsedRealtime(); + int callingPid = Binder.getCallingPid(); int callingUid = Binder.getCallingUid(); EXECUTOR.execute(() -> { @AppSearchResult.ResultCode int statusCode = AppSearchResult.RESULT_OK; @@ -505,7 +510,7 @@ public class AppSearchManagerService extends SystemService { // Obtain the user where the client wants to run the operations in. This should // end up being the same as userHandle, assuming it is not a special user and // the client is allowed to run operations in that user. - UserHandle targetUser = handleIncomingUser(userHandle, callingUid); + UserHandle targetUser = handleIncomingUser(userHandle, callingPid, callingUid); verifyUserUnlocked(targetUser); AppSearchBatchResult.Builder resultBuilder = @@ -582,6 +587,7 @@ public class AppSearchManagerService extends SystemService { Objects.requireNonNull(callback); long totalLatencyStartTimeMillis = SystemClock.elapsedRealtime(); + int callingPid = Binder.getCallingPid(); int callingUid = Binder.getCallingUid(); EXECUTOR.execute(() -> { @AppSearchResult.ResultCode int statusCode = AppSearchResult.RESULT_OK; @@ -594,7 +600,7 @@ public class AppSearchManagerService extends SystemService { // Obtain the user where the client wants to run the operations in. This should // end up being the same as userHandle, assuming it is not a special user and // the client is allowed to run operations in that user. - UserHandle targetUser = handleIncomingUser(userHandle, callingUid); + UserHandle targetUser = handleIncomingUser(userHandle, callingPid, callingUid); verifyUserUnlocked(targetUser); AppSearchBatchResult.Builder resultBuilder = @@ -666,6 +672,7 @@ public class AppSearchManagerService extends SystemService { Objects.requireNonNull(callback); long totalLatencyStartTimeMillis = SystemClock.elapsedRealtime(); + int callingPid = Binder.getCallingPid(); int callingUid = Binder.getCallingUid(); EXECUTOR.execute(() -> { @AppSearchResult.ResultCode int statusCode = AppSearchResult.RESULT_OK; @@ -678,7 +685,7 @@ public class AppSearchManagerService extends SystemService { // Obtain the user where the client wants to run the operations in. This should // end up being the same as userHandle, assuming it is not a special user and // the client is allowed to run operations in that user. - UserHandle targetUser = handleIncomingUser(userHandle, callingUid); + UserHandle targetUser = handleIncomingUser(userHandle, callingPid, callingUid); verifyUserUnlocked(targetUser); instance = mAppSearchUserInstanceManager.getUserInstance(targetUser); @@ -735,6 +742,7 @@ public class AppSearchManagerService extends SystemService { Objects.requireNonNull(callback); long totalLatencyStartTimeMillis = SystemClock.elapsedRealtime(); + int callingPid = Binder.getCallingPid(); int callingUid = Binder.getCallingUid(); EXECUTOR.execute(() -> { @AppSearchResult.ResultCode int statusCode = AppSearchResult.RESULT_OK; @@ -747,7 +755,7 @@ public class AppSearchManagerService extends SystemService { // Obtain the user where the client wants to run the operations in. This should // end up being the same as userHandle, assuming it is not a special user and // the client is allowed to run operations in that user. - UserHandle targetUser = handleIncomingUser(userHandle, callingUid); + UserHandle targetUser = handleIncomingUser(userHandle, callingPid, callingUid); verifyUserUnlocked(targetUser); instance = mAppSearchUserInstanceManager.getUserInstance(targetUser); @@ -803,6 +811,7 @@ public class AppSearchManagerService extends SystemService { Objects.requireNonNull(userHandle); Objects.requireNonNull(callback); + int callingPid = Binder.getCallingPid(); int callingUid = Binder.getCallingUid(); EXECUTOR.execute(() -> { try { @@ -811,7 +820,7 @@ public class AppSearchManagerService extends SystemService { // Obtain the user where the client wants to run the operations in. This should // end up being the same as userHandle, assuming it is not a special user and // the client is allowed to run operations in that user. - UserHandle targetUser = handleIncomingUser(userHandle, callingUid); + UserHandle targetUser = handleIncomingUser(userHandle, callingPid, callingUid); verifyUserUnlocked(targetUser); AppSearchUserInstance instance = @@ -833,6 +842,7 @@ public class AppSearchManagerService extends SystemService { Objects.requireNonNull(packageName); Objects.requireNonNull(userHandle); + int callingPid = Binder.getCallingPid(); int callingUid = Binder.getCallingUid(); EXECUTOR.execute(() -> { try { @@ -841,7 +851,7 @@ public class AppSearchManagerService extends SystemService { // Obtain the user where the client wants to run the operations in. This should // end up being the same as userHandle, assuming it is not a special user and // the client is allowed to run operations in that user. - UserHandle targetUser = handleIncomingUser(userHandle, callingUid); + UserHandle targetUser = handleIncomingUser(userHandle, callingPid, callingUid); verifyUserUnlocked(targetUser); AppSearchUserInstance instance = @@ -870,6 +880,7 @@ public class AppSearchManagerService extends SystemService { Objects.requireNonNull(userHandle); Objects.requireNonNull(callback); + int callingPid = Binder.getCallingPid(); int callingUid = Binder.getCallingUid(); EXECUTOR.execute(() -> { try { @@ -878,7 +889,7 @@ public class AppSearchManagerService extends SystemService { // Obtain the user where the client wants to run the operations in. This should // end up being the same as userHandle, assuming it is not a special user and // the client is allowed to run operations in that user. - UserHandle targetUser = handleIncomingUser(userHandle, callingUid); + UserHandle targetUser = handleIncomingUser(userHandle, callingPid, callingUid); verifyUserUnlocked(targetUser); AppSearchUserInstance instance = @@ -922,6 +933,7 @@ public class AppSearchManagerService extends SystemService { Objects.requireNonNull(userHandle); Objects.requireNonNull(callback); + int callingPid = Binder.getCallingPid(); int callingUid = Binder.getCallingUid(); EXECUTOR.execute(() -> { try { @@ -930,7 +942,7 @@ public class AppSearchManagerService extends SystemService { // Obtain the user where the client wants to run the operations in. This should // end up being the same as userHandle, assuming it is not a special user and // the client is allowed to run operations in that user. - UserHandle targetUser = handleIncomingUser(userHandle, callingUid); + UserHandle targetUser = handleIncomingUser(userHandle, callingPid, callingUid); verifyUserUnlocked(targetUser); AppSearchUserInstance instance = @@ -987,6 +999,7 @@ public class AppSearchManagerService extends SystemService { Objects.requireNonNull(userHandle); Objects.requireNonNull(callback); + int callingPid = Binder.getCallingPid(); int callingUid = Binder.getCallingUid(); EXECUTOR.execute(() -> { try { @@ -995,7 +1008,7 @@ public class AppSearchManagerService extends SystemService { // Obtain the user where the client wants to run the operations in. This should // end up being the same as userHandle, assuming it is not a special user and // the client is allowed to run operations in that user. - UserHandle targetUser = handleIncomingUser(userHandle, callingUid); + UserHandle targetUser = handleIncomingUser(userHandle, callingPid, callingUid); verifyUserUnlocked(targetUser); AppSearchUserInstance instance = @@ -1037,6 +1050,7 @@ public class AppSearchManagerService extends SystemService { Objects.requireNonNull(callback); long totalLatencyStartTimeMillis = SystemClock.elapsedRealtime(); + int callingPid = Binder.getCallingPid(); int callingUid = Binder.getCallingUid(); EXECUTOR.execute(() -> { @AppSearchResult.ResultCode int statusCode = AppSearchResult.RESULT_OK; @@ -1049,7 +1063,7 @@ public class AppSearchManagerService extends SystemService { // Obtain the user where the client wants to run the operations in. This should // end up being the same as userHandle, assuming it is not a special user and // the client is allowed to run operations in that user. - UserHandle targetUser = handleIncomingUser(userHandle, callingUid); + UserHandle targetUser = handleIncomingUser(userHandle, callingPid, callingUid); verifyUserUnlocked(targetUser); AppSearchBatchResult.Builder resultBuilder = @@ -1126,6 +1140,7 @@ public class AppSearchManagerService extends SystemService { Objects.requireNonNull(callback); long totalLatencyStartTimeMillis = SystemClock.elapsedRealtime(); + int callingPid = Binder.getCallingPid(); int callingUid = Binder.getCallingUid(); EXECUTOR.execute(() -> { @AppSearchResult.ResultCode int statusCode = AppSearchResult.RESULT_OK; @@ -1138,7 +1153,7 @@ public class AppSearchManagerService extends SystemService { // Obtain the user where the client wants to run the operations in. This should // end up being the same as userHandle, assuming it is not a special user and // the client is allowed to run operations in that user. - UserHandle targetUser = handleIncomingUser(userHandle, callingUid); + UserHandle targetUser = handleIncomingUser(userHandle, callingPid, callingUid); verifyUserUnlocked(targetUser); instance = mAppSearchUserInstanceManager.getUserInstance(targetUser); @@ -1193,6 +1208,7 @@ public class AppSearchManagerService extends SystemService { Objects.requireNonNull(userHandle); Objects.requireNonNull(callback); + int callingPid = Binder.getCallingPid(); int callingUid = Binder.getCallingUid(); EXECUTOR.execute(() -> { try { @@ -1201,7 +1217,7 @@ public class AppSearchManagerService extends SystemService { // Obtain the user where the client wants to run the operations in. This should // end up being the same as userHandle, assuming it is not a special user and // the client is allowed to run operations in that user. - UserHandle targetUser = handleIncomingUser(userHandle, callingUid); + UserHandle targetUser = handleIncomingUser(userHandle, callingPid, callingUid); verifyUserUnlocked(targetUser); AppSearchUserInstance instance = @@ -1226,6 +1242,7 @@ public class AppSearchManagerService extends SystemService { Objects.requireNonNull(userHandle); long totalLatencyStartTimeMillis = SystemClock.elapsedRealtime(); + int callingPid = Binder.getCallingPid(); int callingUid = Binder.getCallingUid(); EXECUTOR.execute(() -> { @AppSearchResult.ResultCode int statusCode = AppSearchResult.RESULT_OK; @@ -1238,7 +1255,7 @@ public class AppSearchManagerService extends SystemService { // Obtain the user where the client wants to run the operations in. This should // end up being the same as userHandle, assuming it is not a special user and // the client is allowed to run operations in that user. - UserHandle targetUser = handleIncomingUser(userHandle, callingUid); + UserHandle targetUser = handleIncomingUser(userHandle, callingPid, callingUid); verifyUserUnlocked(targetUser); instance = mAppSearchUserInstanceManager.getUserInstance(targetUser); @@ -1281,6 +1298,7 @@ public class AppSearchManagerService extends SystemService { Objects.requireNonNull(callback); long totalLatencyStartTimeMillis = SystemClock.elapsedRealtime(); + int callingPid = Binder.getCallingPid(); int callingUid = Binder.getCallingUid(); EXECUTOR.execute(() -> { @@ -1294,7 +1312,7 @@ public class AppSearchManagerService extends SystemService { // Obtain the user where the client wants to run the operations in. This should // end up being the same as userHandle, assuming it is not a special user and // the client is allowed to run operations in that user. - UserHandle targetUser = handleIncomingUser(userHandle, callingUid); + UserHandle targetUser = handleIncomingUser(userHandle, callingPid, callingUid); verifyUserUnlocked(targetUser); Context targetUserContext = mContext.createContextAsUser(targetUser, @@ -1382,12 +1400,22 @@ public class AppSearchManagerService extends SystemService { /** * Helper for dealing with incoming user arguments to system service calls. * + *

Takes care of checking permissions and if the target is special user, this method will + * simply throw. + * * @param targetUserHandle The user which the caller is requesting to execute as. + * @param callingPid The actual pid of the caller as determined by Binder. * @param callingUid The actual uid of the caller as determined by Binder. + * * @return the user handle that the call should run as. Will always be a concrete user. + * + * @throws IllegalArgumentException if the target user is a special user. + * @throws SecurityException if caller trying to interact across user without + * {@link Manifest.permission#INTERACT_ACROSS_USERS_FULL} */ @NonNull - private UserHandle handleIncomingUser(@NonNull UserHandle targetUserHandle, int callingUid) { + private UserHandle handleIncomingUser(@NonNull UserHandle targetUserHandle, int callingPid, + int callingUid) { UserHandle callingUserHandle = UserHandle.getUserHandleForUid(callingUid); if (callingUserHandle.equals(targetUserHandle)) { return targetUserHandle; @@ -1399,9 +1427,16 @@ public class AppSearchManagerService extends SystemService { "Call does not support special user " + targetUserHandle); } + if (mContext.checkPermission( + Manifest.permission.INTERACT_ACROSS_USERS_FULL, + callingPid, + callingUid) == PackageManager.PERMISSION_GRANTED) { + return targetUserHandle; + } throw new SecurityException( - "Requested user, " + targetUserHandle + ", is not the same as the calling user, " - + callingUserHandle + "."); + "Permission denied while calling from uid " + callingUid + + " with " + targetUserHandle + "; Requires permission: " + + Manifest.permission.INTERACT_ACROSS_USERS_FULL); } /**