From 5321abef41e5130340c9612518014dd9d7664661 Mon Sep 17 00:00:00 2001 From: Rhed Jao Date: Tue, 3 May 2022 19:12:46 +0800 Subject: [PATCH 1/3] Fix cross user package visibility leakage for PackageManager (6/n) APIs: - PackageManager#checkUidSignatures - PackageManager#hasUidSigningCertificate - PackageManager#getNameForUid - PackageManager#getNamesForUids - IPackageManager#getUidForSharedUser - IPackageManager#getFlagsForUid - IPackageManager#getPrivateFlagsForUid Bug: 229684723 Test: atest CrossUserPackageVisibilityTests Change-Id: I3f4585e4dbe563dd513ed30b63dcfbfb435ecd3e --- .../java/com/android/server/pm/Computer.java | 20 +++ .../com/android/server/pm/ComputerEngine.java | 133 ++++++++---------- .../CrossUserPackageVisibilityTests.java | 62 +++++++- ...oidManifest-crossUserPackageVisibility.xml | 2 +- 4 files changed, 138 insertions(+), 79 deletions(-) diff --git a/services/core/java/com/android/server/pm/Computer.java b/services/core/java/com/android/server/pm/Computer.java index 8e8ca8605343b..bcffd65d34240 100644 --- a/services/core/java/com/android/server/pm/Computer.java +++ b/services/core/java/com/android/server/pm/Computer.java @@ -227,8 +227,28 @@ public interface Computer extends PackageDataSnapshot { int userId); boolean shouldFilterApplication(@NonNull SharedUserSetting sus, int callingUid, int userId); + /** + * Different form {@link #shouldFilterApplication(PackageStateInternal, int, int)}, the function + * returns {@code true} if the target package is not found in the device or uninstalled in the + * current user. Unless the caller's function needs to handle the package's uninstalled state + * by itself, using this function to keep the consistent behavior between conditions of package + * uninstalled and visibility not allowed to avoid the side channel leakage of package + * existence. + *

+ * Package with {@link PackageManager#SYSTEM_APP_STATE_HIDDEN_UNTIL_INSTALLED_HIDDEN} is not + * treated as an uninstalled package for the carrier apps customization. + */ boolean shouldFilterApplicationIncludingUninstalled(@Nullable PackageStateInternal ps, int callingUid, int userId); + /** + * Different from {@link #shouldFilterApplication(SharedUserSetting, int, int)}, the function + * returns {@code true} if packages with the same shared user are all uninstalled in the current + * user. + * + * @see #shouldFilterApplicationIncludingUninstalled(PackageStateInternal, int, int) + */ + boolean shouldFilterApplicationIncludingUninstalled(@NonNull SharedUserSetting sus, + int callingUid, int userId); int checkUidPermission(String permName, int uid); int getPackageUidInternal(String packageName, long flags, int userId, int callingUid); long updateFlagsForApplication(long flags, int userId); diff --git a/services/core/java/com/android/server/pm/ComputerEngine.java b/services/core/java/com/android/server/pm/ComputerEngine.java index 24004bc47954f..edbaba5faa2d7 100644 --- a/services/core/java/com/android/server/pm/ComputerEngine.java +++ b/services/core/java/com/android/server/pm/ComputerEngine.java @@ -2784,6 +2784,26 @@ public class ComputerEngine implements Computer { ps, callingUid, null, TYPE_UNKNOWN, userId, true /* filterUninstall */); } + /** + * @see #shouldFilterApplication(PackageStateInternal, int, ComponentName, int, int, boolean) + */ + public final boolean shouldFilterApplicationIncludingUninstalled( + @NonNull SharedUserSetting sus, int callingUid, int userId) { + if (shouldFilterApplication(sus, callingUid, userId)) { + return true; + } + final ArraySet packageStates = + (ArraySet) sus.getPackageStates(); + for (int index = 0; index < packageStates.size(); index++) { + final PackageStateInternal ps = packageStates.valueAt(index); + if (ps.getUserStateOrDefault(userId).isInstalled() || ps.isHiddenUntilInstalled()) { + return false; + } + } + // Filter it, all packages with the same shared uid are uninstalled. + return true; + } + /** * Verification statuses are ordered from the worse to the best, except for * INTENT_FILTER_DOMAIN_VERIFICATION_STATUS_NEVER, which is the worse. @@ -4243,55 +4263,38 @@ public class ComputerEngine implements Computer { @Override public int checkUidSignatures(int uid1, int uid2) { final int callingUid = Binder.getCallingUid(); - final int callingUserId = UserHandle.getUserId(callingUid); - // Map to base uids. - final int appId1 = UserHandle.getAppId(uid1); - final int appId2 = UserHandle.getAppId(uid2); - SigningDetails p1SigningDetails; - SigningDetails p2SigningDetails; - Object obj = mSettings.getSettingBase(appId1); - if (obj != null) { - if (obj instanceof SharedUserSetting) { - final SharedUserSetting sus = (SharedUserSetting) obj; - if (shouldFilterApplication(sus, callingUid, callingUserId)) { - return PackageManager.SIGNATURE_UNKNOWN_PACKAGE; - } - p1SigningDetails = sus.signatures.mSigningDetails; - } else if (obj instanceof PackageSetting) { - final PackageSetting ps = (PackageSetting) obj; - if (shouldFilterApplication(ps, callingUid, callingUserId)) { - return PackageManager.SIGNATURE_UNKNOWN_PACKAGE; - } - p1SigningDetails = ps.getSigningDetails(); - } else { - return PackageManager.SIGNATURE_UNKNOWN_PACKAGE; - } - } else { - return PackageManager.SIGNATURE_UNKNOWN_PACKAGE; - } - obj = mSettings.getSettingBase(appId2); - if (obj != null) { - if (obj instanceof SharedUserSetting) { - final SharedUserSetting sus = (SharedUserSetting) obj; - if (shouldFilterApplication(sus, callingUid, callingUserId)) { - return PackageManager.SIGNATURE_UNKNOWN_PACKAGE; - } - p2SigningDetails = sus.signatures.mSigningDetails; - } else if (obj instanceof PackageSetting) { - final PackageSetting ps = (PackageSetting) obj; - if (shouldFilterApplication(ps, callingUid, callingUserId)) { - return PackageManager.SIGNATURE_UNKNOWN_PACKAGE; - } - p2SigningDetails = ps.getSigningDetails(); - } else { - return PackageManager.SIGNATURE_UNKNOWN_PACKAGE; - } - } else { + final SigningDetails p1SigningDetails = getSigningDetailsAndFilterAccess(uid1, callingUid); + final SigningDetails p2SigningDetails = getSigningDetailsAndFilterAccess(uid2, callingUid); + if (p1SigningDetails == null || p2SigningDetails == null) { return PackageManager.SIGNATURE_UNKNOWN_PACKAGE; } return checkSignaturesInternal(p1SigningDetails, p2SigningDetails); } + private SigningDetails getSigningDetailsAndFilterAccess(int uid, int callingUid) { + // Map to base uids. + final int appId = UserHandle.getAppId(uid); + final int callingUserId = UserHandle.getUserId(callingUid); + final Object obj = mSettings.getSettingBase(appId); + if (obj == null) { + return null; + } + if (obj instanceof SharedUserSetting) { + final SharedUserSetting sus = (SharedUserSetting) obj; + if (shouldFilterApplicationIncludingUninstalled(sus, callingUid, callingUserId)) { + return null; + } + return sus.signatures.mSigningDetails; + } else if (obj instanceof PackageSetting) { + final PackageSetting ps = (PackageSetting) obj; + if (shouldFilterApplicationIncludingUninstalled(ps, callingUid, callingUserId)) { + return null; + } + return ps.getSigningDetails(); + } + return null; + } + private int checkSignaturesInternal(SigningDetails p1SigningDetails, SigningDetails p2SigningDetails) { if (p1SigningDetails == null) { @@ -4353,28 +4356,8 @@ public class ComputerEngine implements Computer { public boolean hasUidSigningCertificate(int uid, @NonNull byte[] certificate, @PackageManager.CertificateInputType int type) { final int callingUid = Binder.getCallingUid(); - final int callingUserId = UserHandle.getUserId(callingUid); - // Map to base uids. - final int appId = UserHandle.getAppId(uid); - final SigningDetails signingDetails; - final Object obj = mSettings.getSettingBase(appId); - if (obj != null) { - if (obj instanceof SharedUserSetting) { - final SharedUserSetting sus = (SharedUserSetting) obj; - if (shouldFilterApplication(sus, callingUid, callingUserId)) { - return false; - } - signingDetails = sus.signatures.mSigningDetails; - } else if (obj instanceof PackageSetting) { - final PackageSetting ps = (PackageSetting) obj; - if (shouldFilterApplication(ps, callingUid, callingUserId)) { - return false; - } - signingDetails = ps.getSigningDetails(); - } else { - return false; - } - } else { + final SigningDetails signingDetails = getSigningDetailsAndFilterAccess(uid, callingUid); + if (signingDetails == null) { return false; } switch (type) { @@ -4437,13 +4420,13 @@ public class ComputerEngine implements Computer { final Object obj = mSettings.getSettingBase(appId); if (obj instanceof SharedUserSetting) { final SharedUserSetting sus = (SharedUserSetting) obj; - if (shouldFilterApplication(sus, callingUid, callingUserId)) { + if (shouldFilterApplicationIncludingUninstalled(sus, callingUid, callingUserId)) { return null; } return sus.name + ":" + sus.mAppId; } else if (obj instanceof PackageSetting) { final PackageSetting ps = (PackageSetting) obj; - if (shouldFilterApplication(ps, callingUid, callingUserId)) { + if (shouldFilterApplicationIncludingUninstalled(ps, callingUid, callingUserId)) { return null; } return ps.getPackageName(); @@ -4472,14 +4455,14 @@ public class ComputerEngine implements Computer { final Object obj = mSettings.getSettingBase(appId); if (obj instanceof SharedUserSetting) { final SharedUserSetting sus = (SharedUserSetting) obj; - if (shouldFilterApplication(sus, callingUid, callingUserId)) { + if (shouldFilterApplicationIncludingUninstalled(sus, callingUid, callingUserId)) { names[i] = null; } else { names[i] = "shared:" + sus.name; } } else if (obj instanceof PackageSetting) { final PackageSetting ps = (PackageSetting) obj; - if (shouldFilterApplication(ps, callingUid, callingUserId)) { + if (shouldFilterApplicationIncludingUninstalled(ps, callingUid, callingUserId)) { names[i] = null; } else { names[i] = ps.getPackageName(); @@ -4501,7 +4484,7 @@ public class ComputerEngine implements Computer { return Process.INVALID_UID; } final SharedUserSetting suid = mSettings.getSharedUserFromId(sharedUserName); - if (suid != null && !shouldFilterApplication(suid, callingUid, + if (suid != null && !shouldFilterApplicationIncludingUninstalled(suid, callingUid, UserHandle.getUserId(callingUid))) { return suid.mAppId; } @@ -4522,13 +4505,13 @@ public class ComputerEngine implements Computer { final Object obj = mSettings.getSettingBase(appId); if (obj instanceof SharedUserSetting) { final SharedUserSetting sus = (SharedUserSetting) obj; - if (shouldFilterApplication(sus, callingUid, callingUserId)) { + if (shouldFilterApplicationIncludingUninstalled(sus, callingUid, callingUserId)) { return 0; } return sus.getFlags(); } else if (obj instanceof PackageSetting) { final PackageSetting ps = (PackageSetting) obj; - if (shouldFilterApplication(ps, callingUid, callingUserId)) { + if (shouldFilterApplicationIncludingUninstalled(ps, callingUid, callingUserId)) { return 0; } return ps.getFlags(); @@ -4550,13 +4533,13 @@ public class ComputerEngine implements Computer { final Object obj = mSettings.getSettingBase(appId); if (obj instanceof SharedUserSetting) { final SharedUserSetting sus = (SharedUserSetting) obj; - if (shouldFilterApplication(sus, callingUid, callingUserId)) { + if (shouldFilterApplicationIncludingUninstalled(sus, callingUid, callingUserId)) { return 0; } return sus.getPrivateFlags(); } else if (obj instanceof PackageSetting) { final PackageSetting ps = (PackageSetting) obj; - if (shouldFilterApplication(ps, callingUid, callingUserId)) { + if (shouldFilterApplicationIncludingUninstalled(ps, callingUid, callingUserId)) { return 0; } return ps.getPrivateFlags(); diff --git a/services/tests/PackageManagerServiceTests/appenumeration/src/com/android/server/pm/test/appenumeration/CrossUserPackageVisibilityTests.java b/services/tests/PackageManagerServiceTests/appenumeration/src/com/android/server/pm/test/appenumeration/CrossUserPackageVisibilityTests.java index a7ba45f0c4c2b..d7b442c8eaf8a 100644 --- a/services/tests/PackageManagerServiceTests/appenumeration/src/com/android/server/pm/test/appenumeration/CrossUserPackageVisibilityTests.java +++ b/services/tests/PackageManagerServiceTests/appenumeration/src/com/android/server/pm/test/appenumeration/CrossUserPackageVisibilityTests.java @@ -27,6 +27,8 @@ import android.app.Instrumentation; import android.content.Context; import android.content.pm.IPackageManager; import android.content.pm.KeySet; +import android.content.pm.PackageManager; +import android.os.Process; import android.os.UserHandle; import androidx.test.platform.app.InstrumentationRegistry; @@ -56,8 +58,12 @@ public class CrossUserPackageVisibilityTests { private static final String TEST_DATA_DIR = "/data/local/tmp/appenumerationtests"; private static final String CROSS_USER_TEST_PACKAGE_NAME = "com.android.appenumeration.crossuserpackagevisibility"; + private static final String SHARED_USER_TEST_PACKAGE_NAME = + "com.android.appenumeration.shareduid"; private static final File CROSS_USER_TEST_APK_FILE = new File(TEST_DATA_DIR, "AppEnumerationCrossUserPackageVisibilityTestApp.apk"); + private static final File SHARED_USER_TEST_APK_FILE = + new File(TEST_DATA_DIR, "AppEnumerationSharedUserTestApp.apk"); @ClassRule @Rule @@ -66,6 +72,7 @@ public class CrossUserPackageVisibilityTests { private Instrumentation mInstrumentation; private IPackageManager mIPackageManager; private Context mContext; + private UserReference mCurrentUser; private UserReference mOtherUser; @Before @@ -77,17 +84,21 @@ public class CrossUserPackageVisibilityTests { // Get another user final UserReference primaryUser = sDeviceState.primaryUser(); if (primaryUser.id() == UserHandle.myUserId()) { + mCurrentUser = primaryUser; mOtherUser = sDeviceState.secondaryUser(); } else { + mCurrentUser = sDeviceState.secondaryUser(); mOtherUser = primaryUser; } uninstallPackage(CROSS_USER_TEST_PACKAGE_NAME); + uninstallPackage(SHARED_USER_TEST_PACKAGE_NAME); } @After public void tearDown() { uninstallPackage(CROSS_USER_TEST_PACKAGE_NAME); + uninstallPackage(SHARED_USER_TEST_PACKAGE_NAME); } @Test @@ -151,16 +162,61 @@ public class CrossUserPackageVisibilityTests { assertThat(e1.getMessage()).isEqualTo(e2.getMessage()); } + @Test + public void testGetFlagsForUid_cannotDetectCrossUserPkg() throws Exception { + installPackage(CROSS_USER_TEST_APK_FILE); + final int uid = mContext.getPackageManager().getPackageUid( + CROSS_USER_TEST_PACKAGE_NAME, PackageManager.PackageInfoFlags.of(0)); + + uninstallPackageForUser(CROSS_USER_TEST_PACKAGE_NAME, mCurrentUser); + + assertThat(mIPackageManager.getFlagsForUid(uid)).isEqualTo(0); + } + + @Test + public void testGetUidForSharedUser_cannotDetectSharedUserPkg() throws Exception { + assertThat(mIPackageManager.getUidForSharedUser(SHARED_USER_TEST_PACKAGE_NAME)) + .isEqualTo(Process.INVALID_UID); + + installPackageForUser(SHARED_USER_TEST_APK_FILE, mOtherUser, true /* forceQueryable */); + + assertThat(mIPackageManager.getUidForSharedUser(SHARED_USER_TEST_PACKAGE_NAME)) + .isEqualTo(Process.INVALID_UID); + } + + private static void installPackage(File apk) { + installPackageForUser(apk, null, false /* forceQueryable */); + } + private static void installPackageForUser(File apk, UserReference user) { + installPackageForUser(apk, user, false /* forceQueryable */); + } + + private static void installPackageForUser(File apk, UserReference user, + boolean forceQueryable) { assertThat(apk.exists()).isTrue(); - final StringBuilder cmd = new StringBuilder("pm install --user "); - cmd.append(user.id()).append(" "); + final StringBuilder cmd = new StringBuilder("pm install -t "); + if (forceQueryable) { + cmd.append("--force-queryable "); + } + if (user != null) { + cmd.append("--user ").append(user.id()).append(" "); + } cmd.append(apk.getPath()); final String result = runShellCommand(cmd.toString()); assertThat(result.trim()).contains("Success"); } private static void uninstallPackage(String packageName) { - runShellCommand("pm uninstall " + packageName); + uninstallPackageForUser(packageName, null /* user */); + } + + private static void uninstallPackageForUser(String packageName, UserReference user) { + final StringBuilder cmd = new StringBuilder("pm uninstall "); + if (user != null) { + cmd.append("--user ").append(user.id()).append(" "); + } + cmd.append(packageName); + runShellCommand(cmd.toString()); } } diff --git a/services/tests/PackageManagerServiceTests/appenumeration/test-apps/target/AndroidManifest-crossUserPackageVisibility.xml b/services/tests/PackageManagerServiceTests/appenumeration/test-apps/target/AndroidManifest-crossUserPackageVisibility.xml index 874a1fc5ff3e8..9d38ddfab5f10 100644 --- a/services/tests/PackageManagerServiceTests/appenumeration/test-apps/target/AndroidManifest-crossUserPackageVisibility.xml +++ b/services/tests/PackageManagerServiceTests/appenumeration/test-apps/target/AndroidManifest-crossUserPackageVisibility.xml @@ -17,6 +17,6 @@ - + From 83e91711f24c7704cfd84d863fc22c3fe83d15d4 Mon Sep 17 00:00:00 2001 From: Rhed Jao Date: Mon, 9 May 2022 19:24:07 +0800 Subject: [PATCH 2/3] Add package manager internal api checkUidSignaturesForAllUsers Starting from U, the PackageManager#checkUidSignatures does not support to check package signatures for different users. It returns false if packages cannot be found in the calling user. This cl adds an internal api checkUidSignaturesForAllUsers for system modules that need to check package signatures installed in any users. Bug: 229684723 Test: atest BlobStoreMultiUserTest Change-Id: Ib5b3c25dcafe664b31bd737bdb2718c045f845b4 --- .../android/server/blob/BlobAccessMode.java | 10 +++-- .../content/pm/PackageManagerInternal.java | 12 ++++++ .../server/firewall/IntentFirewall.java | 12 +++--- .../java/com/android/server/pm/Computer.java | 2 + .../com/android/server/pm/ComputerEngine.java | 37 +++++++++++++++---- .../server/pm/PackageManagerInternalBase.java | 5 +++ 6 files changed, 62 insertions(+), 16 deletions(-) diff --git a/apex/blobstore/service/java/com/android/server/blob/BlobAccessMode.java b/apex/blobstore/service/java/com/android/server/blob/BlobAccessMode.java index 83ef21e7528b5..b0c295c331d79 100644 --- a/apex/blobstore/service/java/com/android/server/blob/BlobAccessMode.java +++ b/apex/blobstore/service/java/com/android/server/blob/BlobAccessMode.java @@ -24,6 +24,7 @@ import android.annotation.IntDef; import android.annotation.NonNull; import android.content.Context; import android.content.pm.PackageManager; +import android.content.pm.PackageManagerInternal; import android.os.Binder; import android.os.UserHandle; import android.util.ArraySet; @@ -32,6 +33,7 @@ import android.util.DebugUtils; import android.util.IndentingPrintWriter; import com.android.internal.util.XmlUtils; +import com.android.server.LocalServices; import org.xmlpull.v1.XmlPullParser; import org.xmlpull.v1.XmlPullParserException; @@ -108,7 +110,7 @@ class BlobAccessMode { } if ((mAccessType & ACCESS_TYPE_SAME_SIGNATURE) != 0) { - if (checkSignatures(context, callingUid, committerUid)) { + if (checkSignatures(callingUid, committerUid)) { return true; } } @@ -133,11 +135,11 @@ class BlobAccessMode { /** * Compare signatures for two packages of different users. */ - private boolean checkSignatures(Context context, int uid1, int uid2) { + private boolean checkSignatures(int uid1, int uid2) { final long token = Binder.clearCallingIdentity(); try { - return context.getPackageManager().checkSignatures(uid1, uid2) - == PackageManager.SIGNATURE_MATCH; + return LocalServices.getService(PackageManagerInternal.class) + .checkUidSignaturesForAllUsers(uid1, uid2) == PackageManager.SIGNATURE_MATCH; } finally { Binder.restoreCallingIdentity(token); } diff --git a/services/core/java/android/content/pm/PackageManagerInternal.java b/services/core/java/android/content/pm/PackageManagerInternal.java index c45a871749247..9b2c10f6130c2 100644 --- a/services/core/java/android/content/pm/PackageManagerInternal.java +++ b/services/core/java/android/content/pm/PackageManagerInternal.java @@ -28,6 +28,7 @@ import android.content.ComponentName; import android.content.ContentResolver; import android.content.Intent; import android.content.IntentSender; +import android.content.pm.PackageManager.SignatureResult; import android.content.pm.SigningDetails.CertCapabilities; import android.content.pm.overlay.OverlayPaths; import android.os.Bundle; @@ -1279,4 +1280,15 @@ public abstract class PackageManagerInternal { public abstract void shutdown(); public abstract DynamicCodeLogger getDynamicCodeLogger(); + + /** + * Compare the signatures of two packages that are installed in different users. + * + * @param uid1 First UID whose signature will be compared. + * @param uid2 Second UID whose signature will be compared. + * @return {@link PackageManager#SIGNATURE_MATCH} if signatures are matched. + * @throws SecurityException if the caller does not hold the + * {@link android.Manifest.permission#INTERACT_ACROSS_USERS}. + */ + public abstract @SignatureResult int checkUidSignaturesForAllUsers(int uid1, int uid2); } diff --git a/services/core/java/com/android/server/firewall/IntentFirewall.java b/services/core/java/com/android/server/firewall/IntentFirewall.java index bb8a74493a16a..c8a3ee6b1be6e 100644 --- a/services/core/java/com/android/server/firewall/IntentFirewall.java +++ b/services/core/java/com/android/server/firewall/IntentFirewall.java @@ -25,6 +25,7 @@ import android.content.pm.ApplicationInfo; import android.content.pm.IPackageManager; import android.content.pm.PackageManager; import android.content.pm.PackageManagerInternal; +import android.os.Binder; import android.os.Environment; import android.os.FileObserver; import android.os.Handler; @@ -623,12 +624,13 @@ public class IntentFirewall { } boolean signaturesMatch(int uid1, int uid2) { + final long token = Binder.clearCallingIdentity(); try { - IPackageManager pm = AppGlobals.getPackageManager(); - return pm.checkUidSignatures(uid1, uid2) == PackageManager.SIGNATURE_MATCH; - } catch (RemoteException ex) { - Slog.e(TAG, "Remote exception while checking signatures", ex); - return false; + // Compare signatures of two packages for different users. + return LocalServices.getService(PackageManagerInternal.class) + .checkUidSignaturesForAllUsers(uid1, uid2) == PackageManager.SIGNATURE_MATCH; + } finally { + Binder.restoreCallingIdentity(token); } } diff --git a/services/core/java/com/android/server/pm/Computer.java b/services/core/java/com/android/server/pm/Computer.java index bcffd65d34240..2657be3a97d9f 100644 --- a/services/core/java/com/android/server/pm/Computer.java +++ b/services/core/java/com/android/server/pm/Computer.java @@ -397,6 +397,8 @@ public interface Computer extends PackageDataSnapshot { int checkUidSignatures(int uid1, int uid2); + int checkUidSignaturesForAllUsers(int uid1, int uid2); + boolean hasSigningCertificate(@NonNull String packageName, @NonNull byte[] certificate, @PackageManager.CertificateInputType int type); diff --git a/services/core/java/com/android/server/pm/ComputerEngine.java b/services/core/java/com/android/server/pm/ComputerEngine.java index edbaba5faa2d7..6e772b23bf1b6 100644 --- a/services/core/java/com/android/server/pm/ComputerEngine.java +++ b/services/core/java/com/android/server/pm/ComputerEngine.java @@ -4263,31 +4263,52 @@ public class ComputerEngine implements Computer { @Override public int checkUidSignatures(int uid1, int uid2) { final int callingUid = Binder.getCallingUid(); - final SigningDetails p1SigningDetails = getSigningDetailsAndFilterAccess(uid1, callingUid); - final SigningDetails p2SigningDetails = getSigningDetailsAndFilterAccess(uid2, callingUid); + final int callingUserId = UserHandle.getUserId(callingUid); + final SigningDetails p1SigningDetails = + getSigningDetailsAndFilterAccess(uid1, callingUid, callingUserId); + final SigningDetails p2SigningDetails = + getSigningDetailsAndFilterAccess(uid2, callingUid, callingUserId); if (p1SigningDetails == null || p2SigningDetails == null) { return PackageManager.SIGNATURE_UNKNOWN_PACKAGE; } return checkSignaturesInternal(p1SigningDetails, p2SigningDetails); } - private SigningDetails getSigningDetailsAndFilterAccess(int uid, int callingUid) { + @Override + public int checkUidSignaturesForAllUsers(int uid1, int uid2) { + final int callingUid = Binder.getCallingUid(); + final int userId1 = UserHandle.getUserId(uid1); + final int userId2 = UserHandle.getUserId(uid2); + enforceCrossUserPermission(callingUid, userId1, false /* requireFullPermission */, + false /* checkShell */, "checkUidSignaturesForAllUsers"); + enforceCrossUserPermission(callingUid, userId2, false /* requireFullPermission */, + false /* checkShell */, "checkUidSignaturesForAllUsers"); + final SigningDetails p1SigningDetails = + getSigningDetailsAndFilterAccess(uid1, callingUid, userId1); + final SigningDetails p2SigningDetails = + getSigningDetailsAndFilterAccess(uid2, callingUid, userId2); + if (p1SigningDetails == null || p2SigningDetails == null) { + return PackageManager.SIGNATURE_UNKNOWN_PACKAGE; + } + return checkSignaturesInternal(p1SigningDetails, p2SigningDetails); + } + + private SigningDetails getSigningDetailsAndFilterAccess(int uid, int callingUid, int userId) { // Map to base uids. final int appId = UserHandle.getAppId(uid); - final int callingUserId = UserHandle.getUserId(callingUid); final Object obj = mSettings.getSettingBase(appId); if (obj == null) { return null; } if (obj instanceof SharedUserSetting) { final SharedUserSetting sus = (SharedUserSetting) obj; - if (shouldFilterApplicationIncludingUninstalled(sus, callingUid, callingUserId)) { + if (shouldFilterApplicationIncludingUninstalled(sus, callingUid, userId)) { return null; } return sus.signatures.mSigningDetails; } else if (obj instanceof PackageSetting) { final PackageSetting ps = (PackageSetting) obj; - if (shouldFilterApplicationIncludingUninstalled(ps, callingUid, callingUserId)) { + if (shouldFilterApplicationIncludingUninstalled(ps, callingUid, userId)) { return null; } return ps.getSigningDetails(); @@ -4356,7 +4377,9 @@ public class ComputerEngine implements Computer { public boolean hasUidSigningCertificate(int uid, @NonNull byte[] certificate, @PackageManager.CertificateInputType int type) { final int callingUid = Binder.getCallingUid(); - final SigningDetails signingDetails = getSigningDetailsAndFilterAccess(uid, callingUid); + final int callingUserId = UserHandle.getUserId(callingUid); + final SigningDetails signingDetails = + getSigningDetailsAndFilterAccess(uid, callingUid, callingUserId); if (signingDetails == null) { return false; } diff --git a/services/core/java/com/android/server/pm/PackageManagerInternalBase.java b/services/core/java/com/android/server/pm/PackageManagerInternalBase.java index 4a3b79e41de14..a224b2258d548 100644 --- a/services/core/java/com/android/server/pm/PackageManagerInternalBase.java +++ b/services/core/java/com/android/server/pm/PackageManagerInternalBase.java @@ -721,6 +721,11 @@ abstract class PackageManagerInternalBase extends PackageManagerInternal { return snapshot().isUidPrivileged(uid); } + @Override + public int checkUidSignaturesForAllUsers(int uid1, int uid2) { + return snapshot().checkUidSignaturesForAllUsers(uid1, uid2); + } + @NonNull @Override @Deprecated From 87944e742cea0890f686126fd8e706a11c6155d2 Mon Sep 17 00:00:00 2001 From: Rhed Jao Date: Tue, 10 May 2022 15:50:51 +0800 Subject: [PATCH 3/3] Using isUidPrivileged instead of getPrivateFlagsForUid API Starting from U, the PackageManager#getPrivateFlagsForUid does not support to get private flags for different users. It returns nothing if the package cannot be found in the calling user. To avoid failing to get private flags in the IntentFirewall, this CL uses the internal API isUidPrivileged to check the privilege state of the caller. Bug: 229684723 Test: Build Change-Id: I8440cb51e4318d57a097efe651be626b9df2da0f --- .../server/firewall/IntentFirewall.java | 4 +-- .../android/server/firewall/SenderFilter.java | 27 +++++-------------- 2 files changed, 9 insertions(+), 22 deletions(-) diff --git a/services/core/java/com/android/server/firewall/IntentFirewall.java b/services/core/java/com/android/server/firewall/IntentFirewall.java index c8a3ee6b1be6e..2b95b11a09cd4 100644 --- a/services/core/java/com/android/server/firewall/IntentFirewall.java +++ b/services/core/java/com/android/server/firewall/IntentFirewall.java @@ -130,7 +130,7 @@ public class IntentFirewall { mObserver.startWatching(); } - private PackageManagerInternal getPackageManager() { + PackageManagerInternal getPackageManager() { if (mPackageManager == null) { mPackageManager = LocalServices.getService(PackageManagerInternal.class); } @@ -627,7 +627,7 @@ public class IntentFirewall { final long token = Binder.clearCallingIdentity(); try { // Compare signatures of two packages for different users. - return LocalServices.getService(PackageManagerInternal.class) + return getPackageManager() .checkUidSignaturesForAllUsers(uid1, uid2) == PackageManager.SIGNATURE_MATCH; } finally { Binder.restoreCallingIdentity(token); diff --git a/services/core/java/com/android/server/firewall/SenderFilter.java b/services/core/java/com/android/server/firewall/SenderFilter.java index 0074119623298..40684fab5c70a 100644 --- a/services/core/java/com/android/server/firewall/SenderFilter.java +++ b/services/core/java/com/android/server/firewall/SenderFilter.java @@ -16,14 +16,11 @@ package com.android.server.firewall; -import android.app.AppGlobals; import android.content.ComponentName; import android.content.Intent; -import android.content.pm.ApplicationInfo; -import android.content.pm.IPackageManager; +import android.content.pm.PackageManagerInternal; import android.os.Process; -import android.os.RemoteException; -import android.util.Slog; + import org.xmlpull.v1.XmlPullParser; import org.xmlpull.v1.XmlPullParserException; @@ -37,22 +34,12 @@ class SenderFilter { private static final String VAL_SYSTEM_OR_SIGNATURE = "system|signature"; private static final String VAL_USER_ID = "userId"; - static boolean isPrivilegedApp(int callerUid, int callerPid) { + static boolean isPrivilegedApp(PackageManagerInternal pmi, int callerUid, int callerPid) { if (callerUid == Process.SYSTEM_UID || callerUid == 0 || callerPid == Process.myPid() || callerPid == 0) { return true; } - - IPackageManager pm = AppGlobals.getPackageManager(); - try { - return (pm.getPrivateFlagsForUid(callerUid) & ApplicationInfo.PRIVATE_FLAG_PRIVILEGED) - != 0; - } catch (RemoteException ex) { - Slog.e(IntentFirewall.TAG, "Remote exception while retrieving uid flags", - ex); - } - - return false; + return pmi.isUidPrivileged(callerUid); } public static final FilterFactory FACTORY = new FilterFactory("sender") { @@ -89,7 +76,7 @@ class SenderFilter { @Override public boolean matches(IntentFirewall ifw, ComponentName resolvedComponent, Intent intent, int callerUid, int callerPid, String resolvedType, int receivingUid) { - return isPrivilegedApp(callerUid, callerPid); + return isPrivilegedApp(ifw.getPackageManager(), callerUid, callerPid); } }; @@ -97,8 +84,8 @@ class SenderFilter { @Override public boolean matches(IntentFirewall ifw, ComponentName resolvedComponent, Intent intent, int callerUid, int callerPid, String resolvedType, int receivingUid) { - return isPrivilegedApp(callerUid, callerPid) || - ifw.signaturesMatch(callerUid, receivingUid); + return isPrivilegedApp(ifw.getPackageManager(), callerUid, callerPid) + || ifw.signaturesMatch(callerUid, receivingUid); } };