From eac923e30cf043046040abd077367745a64fc6f7 Mon Sep 17 00:00:00 2001 From: Bookatz Date: Tue, 17 Dec 2019 12:30:20 -0800 Subject: [PATCH] UserSystemPackageInstaller: Implicit System Mode Previously, to ease local development, the SYSTEM user was always implicitly whitelisted (meaning that, if the system package whitelist didn't have an entry for a particular system package, it would *always* be implicitly whitelisted on user 0). Here, we turn this into a optional mode (implicitWhitelistSystemMode) set in config.xml. Test: atest UserSystemPackageInstallerTest Bug: 134605778 Fixes: 145611545 Change-Id: I5cf8eaa31969bafc4c3bbe15ecd7bd36f3a87384 --- core/res/res/values/config.xml | 21 ++++-- .../server/pm/UserSystemPackageInstaller.java | 75 ++++++++++--------- .../pm/UserSystemPackageInstallerTest.java | 45 +++++------ 3 files changed, 76 insertions(+), 65 deletions(-) diff --git a/core/res/res/values/config.xml b/core/res/res/values/config.xml index ced5deb66899b..d9669669422b7 100644 --- a/core/res/res/values/config.xml +++ b/core/res/res/values/config.xml @@ -2357,15 +2357,22 @@ - 13 - + 29 + false diff --git a/services/core/java/com/android/server/pm/UserSystemPackageInstaller.java b/services/core/java/com/android/server/pm/UserSystemPackageInstaller.java index c36b9938a8cd2..77bb48eadc41c 100644 --- a/services/core/java/com/android/server/pm/UserSystemPackageInstaller.java +++ b/services/core/java/com/android/server/pm/UserSystemPackageInstaller.java @@ -67,10 +67,11 @@ import java.util.Set; * then: * * *

NOTE: the {@code SystemConfig} state is only updated on first boot or after a system @@ -86,22 +87,24 @@ class UserSystemPackageInstaller { * System Property whether to only install system packages on a user if they're whitelisted for * that user type. These are flags and can be freely combined. *

* Note: This list must be kept current with config_userTypePackageWhitelistMode in * frameworks/base/core/res/res/values/config.xml */ static final String PACKAGE_WHITELIST_MODE_PROP = "persist.debug.user.package_whitelist_mode"; - static final int USER_TYPE_PACKAGE_WHITELIST_MODE_DISABLE = 0; - static final int USER_TYPE_PACKAGE_WHITELIST_MODE_ENFORCE = 0b0001; - static final int USER_TYPE_PACKAGE_WHITELIST_MODE_LOG = 0b0010; - static final int USER_TYPE_PACKAGE_WHITELIST_MODE_IMPLICIT_WHITELIST = 0b0100; - static final int USER_TYPE_PACKAGE_WHITELIST_MODE_IGNORE_OTA = 0b1000; + static final int USER_TYPE_PACKAGE_WHITELIST_MODE_DISABLE = 0x00; + static final int USER_TYPE_PACKAGE_WHITELIST_MODE_ENFORCE = 0x01; + static final int USER_TYPE_PACKAGE_WHITELIST_MODE_LOG = 0x02; + static final int USER_TYPE_PACKAGE_WHITELIST_MODE_IMPLICIT_WHITELIST = 0x04; + static final int USER_TYPE_PACKAGE_WHITELIST_MODE_IMPLICIT_WHITELIST_SYSTEM = 0x08; + static final int USER_TYPE_PACKAGE_WHITELIST_MODE_IGNORE_OTA = 0x10; static final int USER_TYPE_PACKAGE_WHITELIST_MODE_DEVICE_DEFAULT = -1; @IntDef(flag = true, prefix = "USER_TYPE_PACKAGE_WHITELIST_MODE_", value = { @@ -281,6 +284,14 @@ class UserSystemPackageInstaller { return isImplicitWhitelistMode(getWhitelistMode()); } + /** + * Whether to treat all packages that are not mentioned at all in the whitelist to be implicitly + * whitelisted for the SYSTEM user. + */ + boolean isImplicitWhitelistSystemMode() { + return isImplicitWhitelistSystemMode(getWhitelistMode()); + } + /** See {@link #isEnforceMode()}. */ private static boolean isEnforceMode(int whitelistMode) { return (whitelistMode & USER_TYPE_PACKAGE_WHITELIST_MODE_ENFORCE) != 0; @@ -301,6 +312,11 @@ class UserSystemPackageInstaller { return (whitelistMode & USER_TYPE_PACKAGE_WHITELIST_MODE_IMPLICIT_WHITELIST) != 0; } + /** See {@link #isImplicitWhitelistSystemMode()}. */ + private static boolean isImplicitWhitelistSystemMode(int whitelistMode) { + return (whitelistMode & USER_TYPE_PACKAGE_WHITELIST_MODE_IMPLICIT_WHITELIST_SYSTEM) != 0; + } + /** Gets the PackageWhitelistMode for use of {@link #mWhitelistedPackagesForUserTypes}. */ private @PackageWhitelistMode int getWhitelistMode() { final int runtimeMode = SystemProperties.getInt( @@ -332,8 +348,8 @@ class UserSystemPackageInstaller { if (!isEnforceMode(mode)) { return null; } - final boolean isSystemUser = mUm.isUserTypeSubtypeOfSystem(userType); - final boolean isImplicitWhitelistMode = isImplicitWhitelistMode(mode); + final boolean implicitlyWhitelist = isImplicitWhitelistMode(mode) + || (isImplicitWhitelistSystemMode(mode) && mUm.isUserTypeSubtypeOfSystem(userType)); final Set whitelistedPackages = getWhitelistedPackagesForUserType(userType); final Set installPackages = new ArraySet<>(); @@ -343,7 +359,7 @@ class UserSystemPackageInstaller { return; } if (shouldInstallPackage(pkg, mWhitelistedPackagesForUserTypes, - whitelistedPackages, isImplicitWhitelistMode, isSystemUser)) { + whitelistedPackages, implicitlyWhitelist)) { // Although the whitelist uses manifest names, this function returns packageNames. installPackages.add(pkg.getPackageName()); } @@ -360,31 +376,18 @@ class UserSystemPackageInstaller { * installed. This is only used for overriding the userWhitelist in * certain situations (based on its keyset). * @param userWhitelist set of package manifest names that should be installed on this - * particular user. This must be consistent with userTypeWhitelist, but is - * passed in separately to avoid repeatedly calculating it from + * particular user. This must be consistent with userTypeWhitelist, + * but is passed in separately to avoid repeatedly calculating it from * userTypeWhitelist. - * @param isImplicitWhitelistMode whether non-mentioned packages are implicitly whitelisted. - * @param isSystemUser whether the user is USER_SYSTEM (which gets special treatment). + * @param implicitlyWhitelist whether non-mentioned packages are implicitly whitelisted. */ @VisibleForTesting static boolean shouldInstallPackage(AndroidPackage sysPkg, @NonNull ArrayMap userTypeWhitelist, - @NonNull Set userWhitelist, boolean isImplicitWhitelistMode, - boolean isSystemUser) { - + @NonNull Set userWhitelist, boolean implicitlyWhitelist) { final String pkgName = sysPkg.getManifestPackageName(); - boolean install = (isImplicitWhitelistMode && !userTypeWhitelist.containsKey(pkgName)) + return (implicitlyWhitelist && !userTypeWhitelist.containsKey(pkgName)) || userWhitelist.contains(pkgName); - - // For the purposes of local development, any package that isn't even mentioned in the - // whitelist at all is implicitly treated as whitelisted for the SYSTEM user. - if (!install && isSystemUser && !userTypeWhitelist.containsKey(pkgName)) { - install = true; - Slog.e(TAG, "System package " + pkgName + " is not mentioned " - + "in SystemConfig's 'install-in-user-type' but we are " - + "implicitly treating it as whitelisted for the SYSTEM user."); - } - return install; } /** diff --git a/services/tests/servicestests/src/com/android/server/pm/UserSystemPackageInstallerTest.java b/services/tests/servicestests/src/com/android/server/pm/UserSystemPackageInstallerTest.java index 683278b699c03..3db832b242368 100644 --- a/services/tests/servicestests/src/com/android/server/pm/UserSystemPackageInstallerTest.java +++ b/services/tests/servicestests/src/com/android/server/pm/UserSystemPackageInstallerTest.java @@ -26,6 +26,7 @@ import static com.android.server.pm.UserSystemPackageInstaller.USER_TYPE_PACKAGE import static com.android.server.pm.UserSystemPackageInstaller.USER_TYPE_PACKAGE_WHITELIST_MODE_ENFORCE; import static com.android.server.pm.UserSystemPackageInstaller.USER_TYPE_PACKAGE_WHITELIST_MODE_IGNORE_OTA; import static com.android.server.pm.UserSystemPackageInstaller.USER_TYPE_PACKAGE_WHITELIST_MODE_IMPLICIT_WHITELIST; +import static com.android.server.pm.UserSystemPackageInstaller.USER_TYPE_PACKAGE_WHITELIST_MODE_IMPLICIT_WHITELIST_SYSTEM; import static com.android.server.pm.UserSystemPackageInstaller.USER_TYPE_PACKAGE_WHITELIST_MODE_LOG; import static org.junit.Assert.assertEquals; @@ -262,39 +263,25 @@ public class UserSystemPackageInstallerTest { // No implicit whitelist, so only install pkg1. boolean implicit = false; - boolean isSysUser = false; assertTrue(UserSystemPackageInstaller.shouldInstallPackage( - pkg1, pkgBitSetMap, userWhitelist, implicit, isSysUser)); + pkg1, pkgBitSetMap, userWhitelist, implicit)); assertFalse(UserSystemPackageInstaller.shouldInstallPackage( - pkg2, pkgBitSetMap, userWhitelist, implicit, isSysUser)); + pkg2, pkgBitSetMap, userWhitelist, implicit)); assertFalse(UserSystemPackageInstaller.shouldInstallPackage( - pkg3, pkgBitSetMap, userWhitelist, implicit, isSysUser)); + pkg3, pkgBitSetMap, userWhitelist, implicit)); assertFalse(UserSystemPackageInstaller.shouldInstallPackage( - pkg4, pkgBitSetMap, userWhitelist, implicit, isSysUser)); + pkg4, pkgBitSetMap, userWhitelist, implicit)); // Use implicit whitelist, so install pkg1 and pkg4 implicit = true; - isSysUser = false; assertTrue(UserSystemPackageInstaller.shouldInstallPackage( - pkg1, pkgBitSetMap, userWhitelist, implicit, isSysUser)); + pkg1, pkgBitSetMap, userWhitelist, implicit)); assertFalse(UserSystemPackageInstaller.shouldInstallPackage( - pkg2, pkgBitSetMap, userWhitelist, implicit, isSysUser)); + pkg2, pkgBitSetMap, userWhitelist, implicit)); assertFalse(UserSystemPackageInstaller.shouldInstallPackage( - pkg3, pkgBitSetMap, userWhitelist, implicit, isSysUser)); + pkg3, pkgBitSetMap, userWhitelist, implicit)); assertTrue(UserSystemPackageInstaller.shouldInstallPackage( - pkg4, pkgBitSetMap, userWhitelist, implicit, isSysUser)); - - // For user 0 specifically, we always implicitly whitelist. - implicit = false; - isSysUser = true; - assertTrue(UserSystemPackageInstaller.shouldInstallPackage( - pkg1, pkgBitSetMap, userWhitelist, implicit, isSysUser)); - assertFalse(UserSystemPackageInstaller.shouldInstallPackage( - pkg2, pkgBitSetMap, userWhitelist, implicit, isSysUser)); - assertFalse(UserSystemPackageInstaller.shouldInstallPackage( - pkg3, pkgBitSetMap, userWhitelist, implicit, isSysUser)); - assertTrue(UserSystemPackageInstaller.shouldInstallPackage( - pkg4, pkgBitSetMap, userWhitelist, implicit, isSysUser)); + pkg4, pkgBitSetMap, userWhitelist, implicit)); } /** @@ -400,30 +387,42 @@ public class UserSystemPackageInstallerTest { assertFalse(mUserSystemPackageInstaller.isLogMode()); assertFalse(mUserSystemPackageInstaller.isEnforceMode()); assertFalse(mUserSystemPackageInstaller.isImplicitWhitelistMode()); + assertFalse(mUserSystemPackageInstaller.isImplicitWhitelistSystemMode()); assertFalse(mUserSystemPackageInstaller.isIgnoreOtaMode()); setUserTypePackageWhitelistMode(USER_TYPE_PACKAGE_WHITELIST_MODE_LOG); assertTrue(mUserSystemPackageInstaller.isLogMode()); assertFalse(mUserSystemPackageInstaller.isEnforceMode()); assertFalse(mUserSystemPackageInstaller.isImplicitWhitelistMode()); + assertFalse(mUserSystemPackageInstaller.isImplicitWhitelistSystemMode()); assertFalse(mUserSystemPackageInstaller.isIgnoreOtaMode()); setUserTypePackageWhitelistMode(USER_TYPE_PACKAGE_WHITELIST_MODE_ENFORCE); assertFalse(mUserSystemPackageInstaller.isLogMode()); assertTrue(mUserSystemPackageInstaller.isEnforceMode()); assertFalse(mUserSystemPackageInstaller.isImplicitWhitelistMode()); + assertFalse(mUserSystemPackageInstaller.isImplicitWhitelistSystemMode()); assertFalse(mUserSystemPackageInstaller.isIgnoreOtaMode()); setUserTypePackageWhitelistMode(USER_TYPE_PACKAGE_WHITELIST_MODE_IMPLICIT_WHITELIST); assertFalse(mUserSystemPackageInstaller.isLogMode()); assertFalse(mUserSystemPackageInstaller.isEnforceMode()); assertTrue(mUserSystemPackageInstaller.isImplicitWhitelistMode()); + assertFalse(mUserSystemPackageInstaller.isImplicitWhitelistSystemMode()); + assertFalse(mUserSystemPackageInstaller.isIgnoreOtaMode()); + + setUserTypePackageWhitelistMode(USER_TYPE_PACKAGE_WHITELIST_MODE_IMPLICIT_WHITELIST_SYSTEM); + assertFalse(mUserSystemPackageInstaller.isLogMode()); + assertFalse(mUserSystemPackageInstaller.isEnforceMode()); + assertFalse(mUserSystemPackageInstaller.isImplicitWhitelistMode()); + assertTrue(mUserSystemPackageInstaller.isImplicitWhitelistSystemMode()); assertFalse(mUserSystemPackageInstaller.isIgnoreOtaMode()); setUserTypePackageWhitelistMode(USER_TYPE_PACKAGE_WHITELIST_MODE_IGNORE_OTA); assertFalse(mUserSystemPackageInstaller.isLogMode()); assertFalse(mUserSystemPackageInstaller.isEnforceMode()); assertFalse(mUserSystemPackageInstaller.isImplicitWhitelistMode()); + assertFalse(mUserSystemPackageInstaller.isImplicitWhitelistSystemMode()); assertTrue(mUserSystemPackageInstaller.isIgnoreOtaMode()); setUserTypePackageWhitelistMode( @@ -431,6 +430,7 @@ public class UserSystemPackageInstallerTest { assertTrue(mUserSystemPackageInstaller.isLogMode()); assertTrue(mUserSystemPackageInstaller.isEnforceMode()); assertFalse(mUserSystemPackageInstaller.isImplicitWhitelistMode()); + assertFalse(mUserSystemPackageInstaller.isImplicitWhitelistSystemMode()); assertFalse(mUserSystemPackageInstaller.isIgnoreOtaMode()); setUserTypePackageWhitelistMode(USER_TYPE_PACKAGE_WHITELIST_MODE_IMPLICIT_WHITELIST @@ -438,6 +438,7 @@ public class UserSystemPackageInstallerTest { assertFalse(mUserSystemPackageInstaller.isLogMode()); assertTrue(mUserSystemPackageInstaller.isEnforceMode()); assertTrue(mUserSystemPackageInstaller.isImplicitWhitelistMode()); + assertFalse(mUserSystemPackageInstaller.isImplicitWhitelistSystemMode()); assertFalse(mUserSystemPackageInstaller.isIgnoreOtaMode()); }