From d87d678b1480e42c9c9d5b50d7b4b9a7585f5647 Mon Sep 17 00:00:00 2001 From: Sumedh Sen Date: Thu, 3 Oct 2024 18:35:13 +0000 Subject: [RESTRICT AUTOMERGE] Check cross user permissions for a given UID Instead of relying on Context#checkCallingOrSelfPermission, explicitly check permissions against a given UID. However, to maintain legacy behavior, replace custom UIDs with Binder.getCallingUid when enforcing permissions from a method. Also update tests afftected by this change - by adding methods to mocked objects Bug: 350456241 Test: sts-tradefed run sts-dynamic-develop -m CtsSecurityTestCases -t android.security.cts.ContentProviderMultiUserTests#testAccessFromInitialUser --user-type PRIMARY Test: sts-tradefed run sts-dynamic-develop -m CtsSecurityTestCases -t android.security.cts.ContentProviderMultiUserTests --user-type SECONDARY Change-Id: Ib31cabff5714500471bd397c743e127c85751a5c (cherry picked from commit 6775f07552f15f6e4b934bb9552f7a6abff8060b) --- .../java/com/android/server/pm/ComputerEngine.java | 19 ++++++++++++------- .../PackageManagerComponentLabelIconOverrideTest.kt | 5 +++++ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/services/core/java/com/android/server/pm/ComputerEngine.java b/services/core/java/com/android/server/pm/ComputerEngine.java index 78f1fa60b69f..27be468f3ca6 100644 --- a/services/core/java/com/android/server/pm/ComputerEngine.java +++ b/services/core/java/com/android/server/pm/ComputerEngine.java @@ -634,11 +634,11 @@ public class ComputerEngine implements Computer { String resolvedType, @PackageManager.ResolveInfoFlagsBits long flags, int userId, int callingUid, boolean includeInstantApps) { if (!mUserManager.exists(userId)) return Collections.emptyList(); - enforceCrossUserOrProfilePermission(callingUid, + enforceCrossUserOrProfilePermission(Binder.getCallingUid(), userId, false /*requireFullPermission*/, false /*checkShell*/, - "query intent receivers"); + "query intent services"); final String instantAppPkgName = getInstantAppPackageName(callingUid); flags = updateFlagsForResolve(flags, userId, callingUid, includeInstantApps, false /* isImplicitImageCaptureIntentAndNotSetByDpc */); @@ -2155,10 +2155,10 @@ public class ComputerEngine implements Computer { return true; } if (requireFullPermission) { - return hasPermission(Manifest.permission.INTERACT_ACROSS_USERS_FULL); + return hasPermission(Manifest.permission.INTERACT_ACROSS_USERS_FULL, callingUid); } - return hasPermission(android.Manifest.permission.INTERACT_ACROSS_USERS_FULL) - || hasPermission(Manifest.permission.INTERACT_ACROSS_USERS); + return hasPermission(android.Manifest.permission.INTERACT_ACROSS_USERS_FULL, callingUid) + || hasPermission(Manifest.permission.INTERACT_ACROSS_USERS, callingUid); } /** @@ -2174,6 +2174,11 @@ public class ComputerEngine implements Computer { == PackageManager.PERMISSION_GRANTED; } + private boolean hasPermission(String permission, int uid) { + return mContext.checkPermission(permission, Process.INVALID_PID, uid) + == PackageManager.PERMISSION_GRANTED; + } + public final boolean isCallerSameApp(String packageName, int uid) { return isCallerSameApp(packageName, uid, false /* resolveIsolatedUid */); } @@ -4590,7 +4595,7 @@ public class ComputerEngine implements Computer { final boolean listApex = (flags & MATCH_APEX) != 0; enforceCrossUserPermission( - callingUid, + Binder.getCallingUid(), userId, false /* requireFullPermission */, false /* checkShell */, @@ -5136,7 +5141,7 @@ public class ComputerEngine implements Computer { @Override public int getComponentEnabledSetting(@NonNull ComponentName component, int callingUid, @UserIdInt int userId) { - enforceCrossUserPermission(callingUid, userId, false /*requireFullPermission*/, + enforceCrossUserPermission(Binder.getCallingUid(), userId, false /*requireFullPermission*/, false /*checkShell*/, "getComponentEnabled"); return getComponentEnabledSettingInternal(component, callingUid, userId); } diff --git a/services/tests/PackageManagerComponentOverrideTests/src/com/android/server/pm/test/override/PackageManagerComponentLabelIconOverrideTest.kt b/services/tests/PackageManagerComponentOverrideTests/src/com/android/server/pm/test/override/PackageManagerComponentLabelIconOverrideTest.kt index f3ac7d55c5db..7c6f0943dd02 100644 --- a/services/tests/PackageManagerComponentOverrideTests/src/com/android/server/pm/test/override/PackageManagerComponentLabelIconOverrideTest.kt +++ b/services/tests/PackageManagerComponentOverrideTests/src/com/android/server/pm/test/override/PackageManagerComponentLabelIconOverrideTest.kt @@ -55,6 +55,7 @@ import org.junit.BeforeClass import org.junit.Test import org.junit.runner.RunWith import org.junit.runners.Parameterized +import org.mockito.ArgumentMatchers.eq import org.mockito.Mockito.any import org.mockito.Mockito.anyInt import org.mockito.Mockito.doReturn @@ -385,6 +386,10 @@ class PackageManagerComponentLabelIconOverrideTest { android.Manifest.permission.INTERACT_ACROSS_USERS_FULL)) { PackageManager.PERMISSION_GRANTED } + whenever(this.checkPermission( + eq(android.Manifest.permission.INTERACT_ACROSS_USERS_FULL), anyInt(), anyInt())) { + PackageManager.PERMISSION_GRANTED + } } val mockSharedLibrariesImpl: SharedLibrariesImpl = mock { whenever(this.snapshot()) { this@mock } -- cgit v1.2.3-59-g8ed1b From 97e34e5a6bfa57236995550c5d1d12f979c29ab6 Mon Sep 17 00:00:00 2001 From: Sumedh Sen Date: Thu, 19 Sep 2024 11:57:12 -0700 Subject: [RESTRICT AUTOMERGE] Parse authority to separate userId and non-user parts of it Callers may pass an authority of type `10@com.example` to this API. We must make sure to only find providers with authority `com.example` installed in user 10. Bug: 350456241 Test: sts-tradefed run sts-dynamic-develop -m CtsSecurityTestCases -t android.security.cts.ContentProviderMultiUserTests#testAccessFromInitialUser --user-type PRIMARY Test: sts-tradefed run sts-dynamic-develop -m CtsSecurityTestCases -t android.security.cts.ContentProviderMultiUserTests --user-type SECONDARY Flag: EXEMPT. Bug fix only Change-Id: I737a435795698bdc612dc3bf88c31e5c8f9c17a6 (cherry picked from commit d1ec2efc0b8941a0585712d5b4cec95fd9f12f17) --- .../core/java/com/android/server/pm/ComputerEngine.java | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/services/core/java/com/android/server/pm/ComputerEngine.java b/services/core/java/com/android/server/pm/ComputerEngine.java index 27be468f3ca6..f6253e1f5aae 100644 --- a/services/core/java/com/android/server/pm/ComputerEngine.java +++ b/services/core/java/com/android/server/pm/ComputerEngine.java @@ -67,6 +67,7 @@ import android.annotation.UserIdInt; import android.app.ActivityManager; import android.app.admin.DevicePolicyManagerInternal; import android.content.ComponentName; +import android.content.ContentProvider; import android.content.Context; import android.content.Intent; import android.content.IntentFilter; @@ -4672,8 +4673,14 @@ public class ComputerEngine implements Computer { int callingUid) { if (!mUserManager.exists(userId)) return null; flags = updateFlagsForComponent(flags, userId); - final ProviderInfo providerInfo = mComponentResolver.queryProvider(this, name, flags, - userId); + + // Callers of this API may not always separate the userID and authority. Let's parse it + // before resolving + String authorityWithoutUserId = ContentProvider.getAuthorityWithoutUserId(name); + userId = ContentProvider.getUserIdFromAuthority(name, userId); + + final ProviderInfo providerInfo = mComponentResolver.queryProvider(this, + authorityWithoutUserId, flags, userId); boolean checkedGrants = false; if (providerInfo != null) { // Looking for cross-user grants before enforcing the typical cross-users permissions @@ -4687,7 +4694,7 @@ public class ComputerEngine implements Computer { if (!checkedGrants) { boolean enforceCrossUser = true; - if (isAuthorityRedirectedForCloneProfile(name)) { + if (isAuthorityRedirectedForCloneProfile(authorityWithoutUserId)) { final UserManagerInternal umInternal = mInjector.getUserManagerInternal(); UserInfo userInfo = umInternal.getUserInfo(UserHandle.getUserId(callingUid)); -- cgit v1.2.3-59-g8ed1b