From 5f5d4f6071e9f13b2b4deac77face295ce94aaf4 Mon Sep 17 00:00:00 2001 From: Winson Date: Fri, 19 Mar 2021 13:28:31 -0700 Subject: [PATCH] Check installed and enabled state for package domain approval If the package isn't installed or isn't enabled for a user, it cannot be approved for that user. Also hooks into package uninstall for a single user to remove the domain state for that package for that user. Bug: 183226822 Test: atest DomainVerificationManagerApiTest#getOwnersForDomain Test: atest com.android.server.pm.test.verify.domain Change-Id: I04942e1491d470fdd41e99f207bbf85baae87d4c --- .../android/content/pm/PackageUserState.java | 16 ++- .../server/pm/PackageManagerService.java | 1 + .../DomainVerificationManagerInternal.java | 5 + .../domain/DomainVerificationService.java | 32 ++++++ .../domain/DomainVerificationSettings.java | 42 ++++++-- .../domain/DomainVerificationEnforcerTest.kt | 8 +- .../DomainVerificationManagerApiTest.kt | 97 +++++++++++++++++-- .../domain/DomainVerificationPackageTest.kt | 3 + .../DomainVerificationSettingsMutationTest.kt | 8 +- ...inVerificationUserSelectionOverrideTest.kt | 4 + 10 files changed, 187 insertions(+), 29 deletions(-) diff --git a/core/java/android/content/pm/PackageUserState.java b/core/java/android/content/pm/PackageUserState.java index e115597865b3e..55a6ab708da5c 100644 --- a/core/java/android/content/pm/PackageUserState.java +++ b/core/java/android/content/pm/PackageUserState.java @@ -32,6 +32,7 @@ import android.annotation.Nullable; import android.compat.annotation.UnsupportedAppUsage; import android.content.ComponentName; import android.content.pm.overlay.OverlayPaths; +import android.content.pm.parsing.ParsingPackageRead; import android.content.pm.parsing.component.ParsedMainComponent; import android.os.BaseBundle; import android.os.Debug; @@ -53,7 +54,6 @@ import org.xmlpull.v1.XmlPullParserException; import org.xmlpull.v1.XmlSerializer; import java.io.IOException; -import java.util.Arrays; import java.util.Map; import java.util.Objects; @@ -310,6 +310,20 @@ public class PackageUserState { return result; } + public boolean isPackageEnabled(@NonNull ParsingPackageRead pkg) { + switch (this.enabled) { + case COMPONENT_ENABLED_STATE_ENABLED: + return true; + case COMPONENT_ENABLED_STATE_DISABLED: + case COMPONENT_ENABLED_STATE_DISABLED_USER: + case COMPONENT_ENABLED_STATE_DISABLED_UNTIL_USED: + return false; + default: + case COMPONENT_ENABLED_STATE_DEFAULT: + return pkg.isEnabled(); + } + } + public boolean isEnabled(ComponentInfo componentInfo, int flags) { return isEnabled(componentInfo.applicationInfo.enabled, componentInfo.enabled, componentInfo.name, flags); diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index cb68cc9d5e453..28d04b8689862 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -21759,6 +21759,7 @@ public class PackageManagerService extends IPackageManager.Stub clearPackagePreferredActivities(ps.name, nextUserId); mPermissionManager.onPackageUninstalled(ps.name, ps.appId, pkg, sharedUserPkgs, nextUserId); + mDomainVerificationManager.clearPackageForUser(ps.name, nextUserId); } if (outInfo != null) { 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 73182732cdaf6..5e57ccb9cee29 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 @@ -227,6 +227,11 @@ public interface DomainVerificationManagerInternal { */ void clearPackage(@NonNull String packageName); + /** + * Remove all state for the given package for the given user. + */ + void clearPackageForUser(@NonNull String packageName, @UserIdInt int userId); + /** * Delete all the state for a user. This can be because the user has been removed from the * device, or simply that the state for a user should be deleted. 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 d73f40f0deb3b..7e3fb53a4da4c 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 @@ -29,6 +29,7 @@ import android.content.Intent; import android.content.pm.IntentFilterVerificationInfo; import android.content.pm.PackageManager; import android.content.pm.PackageManager.NameNotFoundException; +import android.content.pm.PackageUserState; import android.content.pm.ResolveInfo; import android.content.pm.parsing.component.ParsedActivity; import android.content.pm.verify.domain.DomainOwner; @@ -1042,6 +1043,21 @@ public class DomainVerificationService extends SystemService public void clearPackage(@NonNull String packageName) { synchronized (mLock) { mAttachedPkgStates.remove(packageName); + mSettings.removePackage(packageName); + } + + mConnection.scheduleWriteSettings(); + } + + @Override + public void clearPackageForUser(@NonNull String packageName, @UserIdInt int userId) { + synchronized (mLock) { + final DomainVerificationPkgState pkgState = mAttachedPkgStates.get(packageName); + if (pkgState != null) { + pkgState.removeUser(userId); + } + + mSettings.removePackageForUser(packageName, userId); } mConnection.scheduleWriteSettings(); @@ -1544,6 +1560,22 @@ public class DomainVerificationService extends SystemService String packageName = pkgSetting.getName(); final AndroidPackage pkg = pkgSetting.getPkg(); + final PackageUserState pkgUserState = pkgSetting.readUserState(userId); + if (pkgUserState == null) { + if (DEBUG_APPROVAL) { + debugApproval(packageName, debugObject, userId, false, + "PackageUserState unavailable"); + } + return APPROVAL_LEVEL_NONE; + } + + if (!pkgUserState.installed || !pkgUserState.isPackageEnabled(pkg)) { + if (DEBUG_APPROVAL) { + debugApproval(packageName, debugObject, userId, false, "package not enabled"); + } + return APPROVAL_LEVEL_NONE; + } + // Should never be null, but if it is, skip this and assume that v2 is enabled if (pkg != null && !DomainVerificationUtils.isChangeEnabled(mPlatformCompat, pkg, SETTINGS_API_V2)) { diff --git a/services/core/java/com/android/server/pm/verify/domain/DomainVerificationSettings.java b/services/core/java/com/android/server/pm/verify/domain/DomainVerificationSettings.java index 8b59da7bb9445..d3cb0b33f45bf 100644 --- a/services/core/java/com/android/server/pm/verify/domain/DomainVerificationSettings.java +++ b/services/core/java/com/android/server/pm/verify/domain/DomainVerificationSettings.java @@ -241,17 +241,41 @@ class DomainVerificationSettings { } } - public void removeUser(@UserIdInt int userId) { - int pendingSize = mPendingPkgStates.size(); - for (int index = 0; index < pendingSize; index++) { - mPendingPkgStates.valueAt(index).removeUser(userId); + public void removePackage(@NonNull String packageName) { + synchronized (mLock) { + mPendingPkgStates.remove(packageName); + mRestoredPkgStates.remove(packageName); } + } - // TODO(b/170746586): Restored assumes user IDs match, which is probably not the case - // on a new device - int restoredSize = mRestoredPkgStates.size(); - for (int index = 0; index < restoredSize; index++) { - mRestoredPkgStates.valueAt(index).removeUser(userId); + public void removePackageForUser(@NonNull String packageName, @UserIdInt int userId) { + synchronized (mLock) { + final DomainVerificationPkgState pendingPkgState = mPendingPkgStates.get(packageName); + if (pendingPkgState != null) { + pendingPkgState.removeUser(userId); + } + // TODO(b/170746586): Restored assumes user IDs match, which is probably not the case + // on a new device + final DomainVerificationPkgState restoredPkgState = mRestoredPkgStates.get(packageName); + if (restoredPkgState != null) { + restoredPkgState.removeUser(userId); + } + } + } + + public void removeUser(@UserIdInt int userId) { + synchronized (mLock) { + int pendingSize = mPendingPkgStates.size(); + for (int index = 0; index < pendingSize; index++) { + mPendingPkgStates.valueAt(index).removeUser(userId); + } + + // TODO(b/170746586): Restored assumes user IDs match, which is probably not the case + // on a new device + int restoredSize = mRestoredPkgStates.size(); + for (int index = 0; index < restoredSize; index++) { + mRestoredPkgStates.valueAt(index).removeUser(userId); + } } } diff --git a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationEnforcerTest.kt b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationEnforcerTest.kt index 537a49e4adcda..01a76eea2b0d8 100644 --- a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationEnforcerTest.kt +++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationEnforcerTest.kt @@ -290,6 +290,7 @@ class DomainVerificationEnforcerTest { fun mockPkg(packageName: String) = mockThrowOnUnmocked { whenever(this.packageName) { packageName } whenever(targetSdkVersion) { Build.VERSION_CODES.S } + whenever(isEnabled) { true } whenever(activities) { listOf( ParsedActivity().apply { @@ -330,11 +331,8 @@ class DomainVerificationEnforcerTest { whenever(getName()) { packageName } whenever(getPkg()) { mockPkg(packageName) } whenever(this.domainSetId) { domainSetId } - whenever(userState) { - SparseArray().apply { - this[0] = PackageUserState() - } - } + whenever(readUserState(0)) { PackageUserState() } + whenever(readUserState(1)) { PackageUserState() } whenever(getInstantApp(anyInt())) { false } } } diff --git a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationManagerApiTest.kt b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationManagerApiTest.kt index 881604ffe6f60..f8e0c60c25df6 100644 --- a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationManagerApiTest.kt +++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationManagerApiTest.kt @@ -19,6 +19,7 @@ package com.android.server.pm.test.verify.domain import android.content.Context import android.content.Intent import android.content.pm.PackageManager +import android.content.pm.PackageUserState import android.content.pm.parsing.component.ParsedActivity import android.content.pm.parsing.component.ParsedIntentInfo import android.content.pm.verify.domain.DomainVerificationInfo @@ -41,6 +42,7 @@ import org.mockito.ArgumentMatchers.anyInt import org.mockito.ArgumentMatchers.anyLong import org.mockito.ArgumentMatchers.anyString import java.util.UUID +import java.util.concurrent.atomic.AtomicBoolean import kotlin.test.assertFailsWith class DomainVerificationManagerApiTest { @@ -267,7 +269,16 @@ class DomainVerificationManagerApiTest { @Test fun getOwnersForDomain() { - val pkg1 = mockPkgSetting(PKG_ONE, UUID_ONE, listOf(DOMAIN_1, DOMAIN_2)) + val pkg1User0Enabled = AtomicBoolean(true) + + val pkg1 = mockPkgSetting(PKG_ONE, UUID_ONE, listOf(DOMAIN_1, DOMAIN_2), pkgUserState0 = { + mockThrowOnUnmocked { + whenever(isPackageEnabled(any())) { + pkg1User0Enabled.get() + } + installed = true + } + }) val pkg2 = mockPkgSetting(PKG_TWO, UUID_TWO, listOf(DOMAIN_1, DOMAIN_2)) val service = makeService(pkg1, pkg2).apply { @@ -276,9 +287,20 @@ class DomainVerificationManagerApiTest { assertThat(service.getOwnersForDomain(DOMAIN_1, 0)).isEmpty() - service.setStatus(pkg1.domainSetId, setOf(DOMAIN_1), DomainVerificationInfo.STATE_SUCCESS) - - service.setStatus(pkg2.domainSetId, setOf(DOMAIN_1), DomainVerificationInfo.STATE_SUCCESS) + assertThat( + service.setStatus( + pkg1.domainSetId, + setOf(DOMAIN_1), + DomainVerificationInfo.STATE_SUCCESS + ) + ).isEqualTo(DomainVerificationManager.STATUS_OK) + assertThat( + service.setStatus( + pkg2.domainSetId, + setOf(DOMAIN_1), + DomainVerificationInfo.STATE_SUCCESS + ) + ).isEqualTo(DomainVerificationManager.STATUS_OK) service.setUserSelection(pkg1.domainSetId, setOf(DOMAIN_2), true, 0) @@ -295,15 +317,65 @@ class DomainVerificationManagerApiTest { assertThat(it.single().packageName).isEqualTo(pkg1.getName()) assertThat(it.single().isOverrideable).isEqualTo(true) } - assertThat(service.getOwnersForDomain(DOMAIN_2, 1)).isEmpty() + assertThat(service.getOwnersForDomain(DOMAIN_2, 1)).isEmpty() + service.setUserSelection(pkg1.domainSetId, setOf(DOMAIN_2), true, 1) + service.getOwnersForDomain(DOMAIN_2, 1).let { + assertThat(it).hasSize(1) + assertThat(it.single().packageName).isEqualTo(pkg1.getName()) + assertThat(it.single().isOverrideable).isEqualTo(true) + } + + // "Uninstall" the package from user 0 and ensure it's stripped from the results + pkg1User0Enabled.set(false) + service.clearPackageForUser(pkg1.getName(), 0) + + service.getOwnersForDomain(DOMAIN_1, 0).let { + assertThat(it).hasSize(1) + assertThat(it.single().packageName).isEqualTo(pkg2.getName()) + assertThat(it.single().isOverrideable).isEqualTo(false) + } + + // Domain 2 user selection gone for user 0 + assertThat(service.getOwnersForDomain(DOMAIN_2, 0)).isEmpty() + + // Domain 2 user selection still around for user 1 + service.getOwnersForDomain(DOMAIN_2, 1).let { + assertThat(it).hasSize(1) + assertThat(it.single().packageName).isEqualTo(pkg1.getName()) + assertThat(it.single().isOverrideable).isEqualTo(true) + } + + // Now assert for user 1 that it was unaffected by the change to user 0 + service.getOwnersForDomain(DOMAIN_1, 1).let { + assertThat(it).hasSize(2) + assertThat(it[0].packageName).isEqualTo(pkg1.getName()) + assertThat(it[0].isOverrideable).isEqualTo(false) + assertThat(it[1].packageName).isEqualTo(pkg2.getName()) + assertThat(it[1].isOverrideable).isEqualTo(false) + } + + service.setUserSelection(pkg1.domainSetId, setOf(DOMAIN_2), true, 0) + + service.getOwnersForDomain(DOMAIN_2, 1).let { + assertThat(it).hasSize(1) + assertThat(it.single().packageName).isEqualTo(pkg1.getName()) + assertThat(it.single().isOverrideable).isEqualTo(true) + } + + // "Reinstall" the package to user 0 + pkg1User0Enabled.set(false) + + // This state should have been cleared when the package was uninstalled + assertThat(service.getOwnersForDomain(DOMAIN_2, 0)).isEmpty() + + // Other package unaffected service.setUserSelection(pkg2.domainSetId, setOf(DOMAIN_2), true, 0) service.getOwnersForDomain(DOMAIN_2, 0).let { assertThat(it).hasSize(1) assertThat(it.single().packageName).isEqualTo(pkg2.getName()) assertThat(it.single().isOverrideable).isEqualTo(true) } - assertThat(service.getOwnersForDomain(DOMAIN_2, 1)).isEmpty() } @Test @@ -382,12 +454,17 @@ class DomainVerificationManagerApiTest { }) } - private fun mockPkgSetting(pkgName: String, domainSetId: UUID, domains: List = listOf( - DOMAIN_1, DOMAIN_2 - )) = mockThrowOnUnmocked { + private fun mockPkgSetting( + pkgName: String, + domainSetId: UUID, + domains: List = listOf(DOMAIN_1, DOMAIN_2), + pkgUserState0: PackageSetting.() -> PackageUserState = { PackageUserState() }, + pkgUserState1: PackageSetting.() -> PackageUserState = { PackageUserState() } + ) = mockThrowOnUnmocked { val pkg = mockThrowOnUnmocked { whenever(packageName) { pkgName } whenever(targetSdkVersion) { Build.VERSION_CODES.S } + whenever(isEnabled) { true } val activityList = listOf( ParsedActivity().apply { @@ -416,6 +493,8 @@ class DomainVerificationManagerApiTest { whenever(this.domainSetId) { domainSetId } whenever(getInstantApp(anyInt())) { false } whenever(firstInstallTime) { 0L } + whenever(readUserState(0)) { pkgUserState0() } + whenever(readUserState(1)) { pkgUserState1() } } fun DomainVerificationService.addPackages(vararg pkgSettings: PackageSetting) = diff --git a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationPackageTest.kt b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationPackageTest.kt index 0ce16e6b6af28..5f601e2648cb5 100644 --- a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationPackageTest.kt +++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationPackageTest.kt @@ -18,6 +18,7 @@ package com.android.server.pm.test.verify.domain import android.content.Intent import android.content.pm.PackageManager +import android.content.pm.PackageUserState import android.content.pm.parsing.component.ParsedActivity import android.content.pm.parsing.component.ParsedIntentInfo import android.content.pm.verify.domain.DomainVerificationInfo.STATE_NO_RESPONSE @@ -394,6 +395,7 @@ class DomainVerificationPackageTest { val pkg = mockThrowOnUnmocked { whenever(packageName) { pkgName } whenever(targetSdkVersion) { Build.VERSION_CODES.S } + whenever(isEnabled) { true } val activityList = listOf( ParsedActivity().apply { @@ -422,5 +424,6 @@ class DomainVerificationPackageTest { whenever(this.domainSetId) { domainSetId } whenever(getInstantApp(anyInt())) { false } whenever(firstInstallTime) { 0L } + whenever(readUserState(USER_ID)) { PackageUserState() } } } diff --git a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationSettingsMutationTest.kt b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationSettingsMutationTest.kt index b7c69226ded20..1cf57c41145a0 100644 --- a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationSettingsMutationTest.kt +++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationSettingsMutationTest.kt @@ -192,6 +192,7 @@ class DomainVerificationSettingsMutationTest { fun mockPkg() = mockThrowOnUnmocked { whenever(packageName) { TEST_PKG } whenever(targetSdkVersion) { Build.VERSION_CODES.S } + whenever(isEnabled) { true } whenever(activities) { listOf( ParsedActivity().apply { @@ -233,11 +234,8 @@ class DomainVerificationSettingsMutationTest { whenever(getName()) { TEST_PKG } whenever(getPkg()) { mockPkg() } whenever(domainSetId) { TEST_UUID } - whenever(userState) { - SparseArray().apply { - this[0] = PackageUserState() - } - } + whenever(readUserState(0)) { PackageUserState() } + whenever(readUserState(10)) { PackageUserState() } whenever(getInstantApp(anyInt())) { false } } } diff --git a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationUserSelectionOverrideTest.kt b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationUserSelectionOverrideTest.kt index 54648ab674229..474e8012d43bd 100644 --- a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationUserSelectionOverrideTest.kt +++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationUserSelectionOverrideTest.kt @@ -18,6 +18,7 @@ package com.android.server.pm.test.verify.domain import android.content.Intent import android.content.pm.PackageManager +import android.content.pm.PackageUserState import android.content.pm.parsing.component.ParsedActivity import android.content.pm.parsing.component.ParsedIntentInfo import android.content.pm.verify.domain.DomainVerificationManager @@ -100,6 +101,7 @@ class DomainVerificationUserStateOverrideTest { val pkg = mockThrowOnUnmocked { whenever(packageName) { pkgName } whenever(targetSdkVersion) { Build.VERSION_CODES.S } + whenever(isEnabled) { true } val activityList = listOf( ParsedActivity().apply { @@ -137,6 +139,8 @@ class DomainVerificationUserStateOverrideTest { whenever(this.domainSetId) { domainSetId } whenever(getInstantApp(anyInt())) { false } whenever(firstInstallTime) { 0L } + whenever(readUserState(0)) { PackageUserState() } + whenever(readUserState(1)) { PackageUserState() } } @Test