From a83c8635e82987001c66a172464a92df9ba99e3f Mon Sep 17 00:00:00 2001 From: Winson Date: Mon, 5 Apr 2021 12:52:47 -0700 Subject: [PATCH] Strip invalid domains in DVS#addPackage If a package is ever updated as part of an OTA, it have may removed domains that were included in the original set, and so that state should be evicted when it's re-attached. Bug: 184562304 Test: atest DomainVerificationPackageTest Change-Id: Ie90dc390afa89b0c26f73de64b1e513c6fa272a3 --- .../domain/DomainVerificationService.java | 21 ++- .../DomainVerificationInternalUserState.java | 5 + .../domain/DomainVerificationPackageTest.kt | 126 ++++++++++++++++-- 3 files changed, 134 insertions(+), 18 deletions(-) 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 e2c837f28520a..b1b4e2afeb497 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 @@ -956,17 +956,25 @@ public class DomainVerificationService extends SystemService } AndroidPackage pkg = newPkgSetting.getPkg(); - ArraySet domains = mCollector.collectValidAutoVerifyDomains(pkg); - boolean hasAutoVerifyDomains = !domains.isEmpty(); + ArraySet autoVerifyDomains = mCollector.collectValidAutoVerifyDomains(pkg); + boolean hasAutoVerifyDomains = !autoVerifyDomains.isEmpty(); boolean isPendingOrRestored = pkgState != null; if (isPendingOrRestored) { pkgState = new DomainVerificationPkgState(pkgState, domainSetId, hasAutoVerifyDomains); + pkgState.getStateMap().retainAll(autoVerifyDomains); + + Set webDomains = mCollector.collectAllWebDomains(pkg); + SparseArray userStates = pkgState.getUserStates(); + int size = userStates.size(); + for (int index = 0; index < size; index++) { + userStates.valueAt(index).retainHosts(webDomains); + } } else { pkgState = new DomainVerificationPkgState(pkgName, domainSetId, hasAutoVerifyDomains); } - boolean needsBroadcast = - applyImmutableState(newPkgSetting, pkgState.getStateMap(), domains); + boolean needsBroadcast = applyImmutableState(newPkgSetting, pkgState.getStateMap(), + autoVerifyDomains); if (needsBroadcast && !isPendingOrRestored) { // TODO(b/159952358): Test this behavior // Attempt to preserve user experience by automatically verifying all domains from @@ -997,9 +1005,10 @@ public class DomainVerificationService extends SystemService && legacyInfo.getStatus() == PackageManager.INTENT_FILTER_DOMAIN_VERIFICATION_STATUS_ALWAYS) { ArrayMap stateMap = pkgState.getStateMap(); - int domainsSize = domains.size(); + int domainsSize = autoVerifyDomains.size(); for (int index = 0; index < domainsSize; index++) { - stateMap.put(domains.valueAt(index), DomainVerificationState.STATE_MIGRATED); + stateMap.put(autoVerifyDomains.valueAt(index), + DomainVerificationState.STATE_MIGRATED); } } } diff --git a/services/core/java/com/android/server/pm/verify/domain/models/DomainVerificationInternalUserState.java b/services/core/java/com/android/server/pm/verify/domain/models/DomainVerificationInternalUserState.java index aa7407ce3fe84..41de3fc06d47f 100644 --- a/services/core/java/com/android/server/pm/verify/domain/models/DomainVerificationInternalUserState.java +++ b/services/core/java/com/android/server/pm/verify/domain/models/DomainVerificationInternalUserState.java @@ -73,6 +73,11 @@ public class DomainVerificationInternalUserState { return this; } + public DomainVerificationInternalUserState retainHosts(@NonNull Set hosts) { + mEnabledHosts.retainAll(hosts); + return this; + } + // Code below generated by codegen v1.0.22. 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 83c126842213b..6c2a8916617b2 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 @@ -22,6 +22,7 @@ import android.content.pm.PackageUserState import android.content.pm.Signature import android.content.pm.parsing.component.ParsedActivity import android.content.pm.parsing.component.ParsedIntentInfo +import android.content.pm.verify.domain.DomainOwner import android.content.pm.verify.domain.DomainVerificationInfo.STATE_MODIFIABLE_VERIFIED import android.content.pm.verify.domain.DomainVerificationInfo.STATE_NO_RESPONSE import android.content.pm.verify.domain.DomainVerificationInfo.STATE_SUCCESS @@ -297,11 +298,95 @@ class DomainVerificationPackageTest { service.addPackage(pkg1) + assertAddPackageActivePendingRestoredState(service) + } + + @Test + fun addPackagePendingStripInvalidDomains() { + val xml = addPackagePendingOrRestoredWithInvalidDomains() + val service = makeService(pkg1, pkg2) + xml.byteInputStream().use { + service.readSettings(Xml.resolvePullParser(it)) + } + + service.addPackage(pkg1) + + val userState = service.getUserState(pkg1.getName()) + assertThat(userState.packageName).isEqualTo(pkg1.getName()) + assertThat(userState.identifier).isEqualTo(pkg1.domainSetId) + assertThat(userState.isLinkHandlingAllowed).isEqualTo(false) + assertThat(userState.user.identifier).isEqualTo(USER_ID) + assertThat(userState.hostToStateMap).containsExactlyEntriesIn(mapOf( + DOMAIN_1 to DOMAIN_STATE_VERIFIED, + DOMAIN_2 to DOMAIN_STATE_SELECTED, + )) + + assertAddPackageActivePendingRestoredState(service) + } + + @Test + fun addPackageRestoredStripInvalidDomains() { + val xml = addPackagePendingOrRestoredWithInvalidDomains() + val service = makeService(pkg1, pkg2) + xml.byteInputStream().use { + service.restoreSettings(Xml.resolvePullParser(it)) + } + + service.addPackage(pkg1) + + assertAddPackageActivePendingRestoredState(service, expectRestore = true) + } + + /** + * Shared string that contains invalid [DOMAIN_3] and [DOMAIN_4] which should be stripped from + * the final state. + */ + private fun addPackagePendingOrRestoredWithInvalidDomains(): String = + // language=XML + """ + + + + + + + + + + + + + + + + + + + + + + + + + """.trimIndent() + + /** + * Shared method to assert the same output when testing adding pkg1. + */ + private fun assertAddPackageActivePendingRestoredState( + service: DomainVerificationService, + expectRestore: Boolean = false + ) { val info = service.getInfo(pkg1.getName()) assertThat(info.packageName).isEqualTo(pkg1.getName()) assertThat(info.identifier).isEqualTo(pkg1.domainSetId) assertThat(info.hostToStateMap).containsExactlyEntriesIn(mapOf( - DOMAIN_1 to STATE_SUCCESS, + // To share the majority of code, special case restoration to check a different int + DOMAIN_1 to if (expectRestore) STATE_MODIFIABLE_VERIFIED else STATE_SUCCESS, DOMAIN_2 to STATE_NO_RESPONSE, )) @@ -317,6 +402,23 @@ class DomainVerificationPackageTest { assertThat(service.queryValidVerificationPackageNames()) .containsExactly(pkg1.getName()) + + // Re-enable link handling to check that the 3/4 domains were stripped + service.setDomainVerificationLinkHandlingAllowed(pkg1.getName(), true, USER_ID) + + assertThat(service.getOwnersForDomain(DOMAIN_1, USER_ID)) + .containsExactly(DomainOwner(PKG_ONE, false)) + + assertThat(service.getOwnersForDomain(DOMAIN_2, USER_ID)) + .containsExactly(DomainOwner(PKG_ONE, true)) + + assertThat(service.getOwnersForDomain(DOMAIN_2, USER_ID + 10)).isEmpty() + + listOf(DOMAIN_3, DOMAIN_4).forEach { domain -> + listOf(USER_ID, USER_ID + 10).forEach { userId -> + assertThat(service.getOwnersForDomain(domain, userId)).isEmpty() + } + } } @Test @@ -528,9 +630,9 @@ class DomainVerificationPackageTest { serviceBefore.addPackage(pkg2) serviceBefore.setStatus(pkg1.domainSetId, setOf(DOMAIN_1), STATE_SUCCESS) - serviceBefore.setDomainVerificationLinkHandlingAllowed(pkg1.getName(), false, 1) + serviceBefore.setDomainVerificationLinkHandlingAllowed(pkg1.getName(), false, 10) serviceBefore.setUserSelection(pkg2.domainSetId, setOf(DOMAIN_2), true, 0) - serviceBefore.setUserSelection(pkg2.domainSetId, setOf(DOMAIN_3), true, 1) + serviceBefore.setUserSelection(pkg2.domainSetId, setOf(DOMAIN_3), true, 10) fun assertExpectedState(service: DomainVerificationService) { service.assertState( @@ -541,7 +643,7 @@ class DomainVerificationPackageTest { ) service.assertState( - pkg1, userId = 1, linkHandingAllowed = false, hostToStateMap = mapOf( + pkg1, userId = 10, linkHandingAllowed = false, hostToStateMap = mapOf( DOMAIN_1 to DOMAIN_STATE_VERIFIED, DOMAIN_2 to DOMAIN_STATE_NONE, ) @@ -556,7 +658,7 @@ class DomainVerificationPackageTest { ) service.assertState( - pkg2, userId = 1, hostToStateMap = mapOf( + pkg2, userId = 10, hostToStateMap = mapOf( DOMAIN_1 to DOMAIN_STATE_NONE, DOMAIN_2 to DOMAIN_STATE_NONE, DOMAIN_3 to DOMAIN_STATE_SELECTED, @@ -572,7 +674,7 @@ class DomainVerificationPackageTest { } val backupUser1 = ByteArrayOutputStream().use { - serviceBefore.writeSettings(Xml.resolveSerializer(it), true, 1) + serviceBefore.writeSettings(Xml.resolveSerializer(it), true, 10) it.toByteArray() } @@ -581,7 +683,7 @@ class DomainVerificationPackageTest { serviceAfter.addPackage(pkg2) // Check the state is default before the restoration applies - listOf(0, 1).forEach { + listOf(0, 10).forEach { serviceAfter.assertState( pkg1, userId = it, hostToStateMap = mapOf( DOMAIN_1 to DOMAIN_STATE_NONE, @@ -590,7 +692,7 @@ class DomainVerificationPackageTest { ) } - listOf(0, 1).forEach { + listOf(0, 10).forEach { serviceAfter.assertState( pkg2, userId = it, hostToStateMap = mapOf( DOMAIN_1 to DOMAIN_STATE_NONE, @@ -606,14 +708,14 @@ class DomainVerificationPackageTest { // Assert user 1 was restored serviceAfter.assertState( - pkg1, userId = 1, linkHandingAllowed = false, hostToStateMap = mapOf( + pkg1, userId = 10, linkHandingAllowed = false, hostToStateMap = mapOf( DOMAIN_1 to DOMAIN_STATE_VERIFIED, DOMAIN_2 to DOMAIN_STATE_NONE, ) ) serviceAfter.assertState( - pkg2, userId = 1, hostToStateMap = mapOf( + pkg2, userId = 10, hostToStateMap = mapOf( DOMAIN_1 to DOMAIN_STATE_NONE, DOMAIN_2 to DOMAIN_STATE_NONE, DOMAIN_3 to DOMAIN_STATE_SELECTED, @@ -679,7 +781,7 @@ class DomainVerificationPackageTest { setConnection(mockThrowOnUnmocked { whenever(filterAppAccess(anyString(), anyInt(), anyInt())) { false } whenever(doesUserExist(0)) { true } - whenever(doesUserExist(1)) { true } + whenever(doesUserExist(10)) { true } whenever(scheduleWriteSettings()) // Need to provide an internal UID so some permission checks are ignored @@ -732,7 +834,7 @@ class DomainVerificationPackageTest { whenever(getInstantApp(anyInt())) { false } whenever(firstInstallTime) { 0L } whenever(readUserState(0)) { PackageUserState() } - whenever(readUserState(1)) { PackageUserState() } + whenever(readUserState(10)) { PackageUserState() } whenever(signatures) { arrayOf(Signature(signature)) } whenever(isSystem) { isSystemApp } }