From 0c4110e09966e5dd225b1fc8eba56022f0ee1d7b Mon Sep 17 00:00:00 2001 From: Winson Date: Tue, 9 Mar 2021 10:26:28 -0800 Subject: [PATCH 1/7] Migrate approved/denied domain states and add tests Allows shell configured values to persist across package updates and validates the behavior with new unit tests. Also fixes a bug with failing to recalculate hasAutoVerifyDomains during an update. Bug: 180535047 Test: atest DomainVerificationPackageTest Change-Id: I793171491c8373ad67a38b5a02a57f46e4d5828a --- .../domain/DomainVerificationManager.java | 22 - .../domain/DomainVerificationState.java | 46 ++ .../domain/DomainVerificationService.java | 25 +- .../models/DomainVerificationPkgState.java | 10 +- .../unit/Android.bp | 2 +- .../domain/DomainVerificationPackageTest.kt | 429 ++++++++++++++++++ 6 files changed, 491 insertions(+), 43 deletions(-) create mode 100644 services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationPackageTest.kt diff --git a/core/java/android/content/pm/verify/domain/DomainVerificationManager.java b/core/java/android/content/pm/verify/domain/DomainVerificationManager.java index f7c81bcffda3f..ac7fdc557340e 100644 --- a/core/java/android/content/pm/verify/domain/DomainVerificationManager.java +++ b/core/java/android/content/pm/verify/domain/DomainVerificationManager.java @@ -162,28 +162,6 @@ public final class DomainVerificationManager { } } - /** - * For determine re-verify policy. This is hidden from the domain verification agent so that no - * behavior is made based on the result. - * - * @hide - */ - public static boolean isStateDefault(@DomainVerificationState.State int state) { - switch (state) { - case DomainVerificationState.STATE_NO_RESPONSE: - case DomainVerificationState.STATE_MIGRATED: - case DomainVerificationState.STATE_RESTORED: - return true; - case DomainVerificationState.STATE_SUCCESS: - case DomainVerificationState.STATE_APPROVED: - case DomainVerificationState.STATE_DENIED: - case DomainVerificationState.STATE_LEGACY_FAILURE: - case DomainVerificationState.STATE_SYS_CONFIG: - default: - return false; - } - } - /** * @hide */ diff --git a/core/java/android/content/pm/verify/domain/DomainVerificationState.java b/core/java/android/content/pm/verify/domain/DomainVerificationState.java index 17593ef2aeb1c..f02f2d9412dc1 100644 --- a/core/java/android/content/pm/verify/domain/DomainVerificationState.java +++ b/core/java/android/content/pm/verify/domain/DomainVerificationState.java @@ -97,4 +97,50 @@ public interface DomainVerificationState { * @see DomainVerificationManager#STATE_FIRST_VERIFIER_DEFINED */ int STATE_FIRST_VERIFIER_DEFINED = 0b10000000000; + + + /** + * For determining re-verify policy. This is hidden from the domain verification agent so that + * no behavior is made based on the result. + * + * @hide + */ + static boolean isDefault(@State int state) { + switch (state) { + case STATE_NO_RESPONSE: + case STATE_MIGRATED: + case STATE_RESTORED: + return true; + case STATE_SUCCESS: + case STATE_APPROVED: + case STATE_DENIED: + case STATE_LEGACY_FAILURE: + case STATE_SYS_CONFIG: + default: + return false; + } + } + + /** + * Whether the state is migrated when updating a package. Generally this is only for states + * that maintain verification state or were set by an explicit user or developer action. + * + * @hide + */ + static boolean shouldMigrate(@State int state) { + switch (state) { + case STATE_SUCCESS: + case STATE_MIGRATED: + case STATE_RESTORED: + case STATE_APPROVED: + case STATE_DENIED: + return true; + case STATE_NO_RESPONSE: + case STATE_LEGACY_FAILURE: + case STATE_SYS_CONFIG: + case STATE_FIRST_VERIFIER_DEFINED: + default: + return false; + } + } } 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 e85bbe41f7479..6c6df9ab3f1fd 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 @@ -793,19 +793,12 @@ public class DomainVerificationService extends SystemService Integer oldStateInteger = oldStateMap.get(domain); if (oldStateInteger != null) { int oldState = oldStateInteger; - switch (oldState) { - case DomainVerificationState.STATE_SUCCESS: - case DomainVerificationState.STATE_RESTORED: - case DomainVerificationState.STATE_MIGRATED: - newStateMap.put(domain, oldState); - break; - default: - // In all other cases, the state code is left unset - // (STATE_NO_RESPONSE) to signal to the verification agent that any - // existing error has been cleared and the domain should be - // re-attempted. This makes update of a package a signal to - // re-verify. - break; + // If the following case fails, the state code is left unset + // (STATE_NO_RESPONSE) to signal to the verification agent that any existing + // error has been cleared and the domain should be re-attempted. This makes + // update of a package a signal to re-verify. + if (DomainVerificationState.shouldMigrate(oldState)) { + newStateMap.put(domain, oldState); } } } @@ -872,7 +865,7 @@ public class DomainVerificationService extends SystemService boolean hasAutoVerifyDomains = !domains.isEmpty(); boolean isPendingOrRestored = pkgState != null; if (isPendingOrRestored) { - pkgState.setId(domainSetId); + pkgState = new DomainVerificationPkgState(pkgState, domainSetId, hasAutoVerifyDomains); } else { pkgState = new DomainVerificationPkgState(pkgName, domainSetId, hasAutoVerifyDomains); } @@ -1185,7 +1178,7 @@ public class DomainVerificationService extends SystemService /** * Determine whether or not a broadcast should be sent at boot for the given {@param pkgState}. * Sends only if the only states recorded are default as decided by {@link - * DomainVerificationManager#isStateDefault(int)}. + * DomainVerificationState#isDefault(int)}. * * If any other state is set, it's assumed that the domain verification agent is aware of the * package and has already scheduled future verification requests. @@ -1199,7 +1192,7 @@ public class DomainVerificationService extends SystemService int statesSize = stateMap.size(); for (int stateIndex = 0; stateIndex < statesSize; stateIndex++) { Integer state = stateMap.valueAt(stateIndex); - if (!DomainVerificationManager.isStateDefault(state)) { + if (!DomainVerificationState.isDefault(state)) { return false; } } diff --git a/services/core/java/com/android/server/pm/verify/domain/models/DomainVerificationPkgState.java b/services/core/java/com/android/server/pm/verify/domain/models/DomainVerificationPkgState.java index a089a60227352..40c70915f3cea 100644 --- a/services/core/java/com/android/server/pm/verify/domain/models/DomainVerificationPkgState.java +++ b/services/core/java/com/android/server/pm/verify/domain/models/DomainVerificationPkgState.java @@ -68,6 +68,12 @@ public class DomainVerificationPkgState { this(packageName, id, hasAutoVerifyDomains, new ArrayMap<>(0), new SparseArray<>(0)); } + public DomainVerificationPkgState(@NonNull DomainVerificationPkgState pkgState, + @NonNull UUID id, boolean hasAutoVerifyDomains) { + this(pkgState.getPackageName(), id, hasAutoVerifyDomains, pkgState.getStateMap(), + pkgState.getUserStates()); + } + @Nullable public DomainVerificationInternalUserState getUserState(@UserIdInt int userId) { return mUserStates.get(userId); @@ -84,10 +90,6 @@ public class DomainVerificationPkgState { return userState; } - public void setId(@NonNull UUID id) { - mId = id; - } - public void removeUser(@UserIdInt int userId) { mUserStates.remove(userId); } diff --git a/services/tests/PackageManagerServiceTests/unit/Android.bp b/services/tests/PackageManagerServiceTests/unit/Android.bp index 334e53a3aec71..eeea6618c08fd 100644 --- a/services/tests/PackageManagerServiceTests/unit/Android.bp +++ b/services/tests/PackageManagerServiceTests/unit/Android.bp @@ -28,9 +28,9 @@ android_test { "androidx.test.rules", "androidx.test.runner", "junit", + "kotlin-test", "services.core", "servicestests-utils", - "testng", "truth-prebuilt", ], platform_apis: true, 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 new file mode 100644 index 0000000000000..8d99005053c14 --- /dev/null +++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationPackageTest.kt @@ -0,0 +1,429 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.pm.test.verify.domain + +import android.content.Intent +import android.content.pm.PackageManager +import android.content.pm.parsing.component.ParsedActivity +import android.content.pm.parsing.component.ParsedIntentInfo +import android.content.pm.verify.domain.DomainVerificationState.STATE_APPROVED +import android.content.pm.verify.domain.DomainVerificationState.STATE_DENIED +import android.content.pm.verify.domain.DomainVerificationState.STATE_NO_RESPONSE +import android.content.pm.verify.domain.DomainVerificationState.STATE_SUCCESS +import android.content.pm.verify.domain.DomainVerificationUserState.DOMAIN_STATE_NONE +import android.content.pm.verify.domain.DomainVerificationUserState.DOMAIN_STATE_SELECTED +import android.content.pm.verify.domain.DomainVerificationUserState.DOMAIN_STATE_VERIFIED +import android.os.Build +import android.os.PatternMatcher +import android.os.Process +import android.util.ArraySet +import android.util.Xml +import com.android.server.pm.PackageSetting +import com.android.server.pm.parsing.pkg.AndroidPackage +import com.android.server.pm.verify.domain.DomainVerificationService +import com.android.server.testutils.mockThrowOnUnmocked +import com.android.server.testutils.whenever +import com.google.common.truth.Truth.assertThat +import org.junit.Test +import org.mockito.ArgumentMatchers +import org.mockito.ArgumentMatchers.any +import org.mockito.ArgumentMatchers.anyInt +import org.mockito.ArgumentMatchers.anyString +import java.util.UUID +import kotlin.test.assertFailsWith + +class DomainVerificationPackageTest { + + companion object { + private const val PKG_ONE = "com.test.one" + private const val PKG_TWO = "com.test.two" + private val UUID_ONE = UUID.fromString("1b041c96-8d37-4932-a858-561bfac5947c") + private val UUID_TWO = UUID.fromString("a3389c16-7f9f-4e86-85e3-500d1249c74c") + + private val DOMAIN_BASE = DomainVerificationPackageTest::class.java.packageName + private val DOMAIN_1 = "one.$DOMAIN_BASE" + private val DOMAIN_2 = "two.$DOMAIN_BASE" + private val DOMAIN_3 = "three.$DOMAIN_BASE" + private val DOMAIN_4 = "four.$DOMAIN_BASE" + + private const val USER_ID = 0 + } + + private val pkg1 = mockPkgSetting(PKG_ONE, UUID_ONE) + private val pkg2 = mockPkgSetting(PKG_TWO, UUID_TWO) + + @Test + fun addPackageFirstTime() { + val service = makeService(pkg1, pkg2) + service.addPackage(pkg1) + 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_NO_RESPONSE, + DOMAIN_2 to STATE_NO_RESPONSE, + )) + + val userState = service.getUserState(pkg1.getName()) + assertThat(userState.packageName).isEqualTo(pkg1.getName()) + assertThat(userState.identifier).isEqualTo(pkg1.domainSetId) + assertThat(userState.isLinkHandlingAllowed).isEqualTo(true) + assertThat(userState.user.identifier).isEqualTo(USER_ID) + assertThat(userState.hostToStateMap).containsExactlyEntriesIn(mapOf( + DOMAIN_1 to DOMAIN_STATE_NONE, + DOMAIN_2 to DOMAIN_STATE_NONE, + )) + + assertThat(service.queryValidVerificationPackageNames()) + .containsExactly(pkg1.getName()) + } + + @Test + fun addPackageActive() { + // language=XML + val xml = """ + + + + + + + + + + + + + + + + + + """.trimIndent() + + val service = makeService(pkg1, pkg2) + xml.byteInputStream().use { + service.readSettings(Xml.resolvePullParser(it)) + } + + service.addPackage(pkg1) + + 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, + DOMAIN_2 to STATE_NO_RESPONSE, + )) + + 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, + )) + + assertThat(service.queryValidVerificationPackageNames()) + .containsExactly(pkg1.getName()) + } + + @Test + fun migratePackageDropDomain() { + val pkgName = PKG_ONE + val pkgBefore = mockPkgSetting(pkgName, UUID_ONE, + listOf(DOMAIN_1, DOMAIN_2, DOMAIN_3, DOMAIN_4)) + val pkgAfter = mockPkgSetting(pkgName, UUID_TWO, + listOf(DOMAIN_1, DOMAIN_2)) + + // Test 4 domains: + // 1 will be approved and preserved, 2 will be selected and preserved, + // 3 will be denied and dropped, 4 will be selected and dropped + + val map = mutableMapOf() + val service = makeService { map[it] } + service.addPackage(pkgBefore) + + // Only insert the package after addPackage call to ensure the service doesn't access + // a live package inside the addPackage logic. It should only use the provided input. + map[pkgName] = pkgBefore + + // To test the approve/denial states, use the internal methods for this variant + service.setDomainVerificationStatusInternal(pkgName, STATE_APPROVED, + ArraySet(setOf(DOMAIN_1))) + service.setDomainVerificationStatusInternal(pkgName, STATE_DENIED, + ArraySet(setOf(DOMAIN_3))) + service.setDomainVerificationUserSelection( + UUID_ONE, setOf(DOMAIN_2, DOMAIN_4), true, USER_ID) + + // Check the verifier cannot change the shell approve/deny states + service.setDomainVerificationStatus(UUID_ONE, setOf(DOMAIN_1, DOMAIN_3), STATE_SUCCESS) + + assertThat(service.getInfo(pkgName).hostToStateMap).containsExactlyEntriesIn(mapOf( + DOMAIN_1 to STATE_APPROVED, + DOMAIN_2 to STATE_NO_RESPONSE, + DOMAIN_3 to STATE_DENIED, + DOMAIN_4 to STATE_NO_RESPONSE, + )) + assertThat(service.getUserState(pkgName).hostToStateMap).containsExactlyEntriesIn(mapOf( + DOMAIN_1 to DOMAIN_STATE_VERIFIED, + DOMAIN_2 to DOMAIN_STATE_SELECTED, + DOMAIN_3 to DOMAIN_STATE_NONE, + DOMAIN_4 to DOMAIN_STATE_SELECTED, + )) + + // Now remove the package because migrateState shouldn't use it either + map.remove(pkgName) + + map[pkgName] = pkgAfter + + service.migrateState(pkgBefore, pkgAfter) + + assertThat(service.getInfo(pkgName).hostToStateMap).containsExactlyEntriesIn(mapOf( + DOMAIN_1 to STATE_APPROVED, + DOMAIN_2 to STATE_NO_RESPONSE, + )) + assertThat(service.getUserState(pkgName).hostToStateMap).containsExactlyEntriesIn(mapOf( + DOMAIN_1 to DOMAIN_STATE_VERIFIED, + DOMAIN_2 to DOMAIN_STATE_SELECTED, + )) + assertThat(service.queryValidVerificationPackageNames()).containsExactly(pkgName) + } + + @Test + fun migratePackageDropAll() { + val pkgName = PKG_ONE + val pkgBefore = mockPkgSetting(pkgName, UUID_ONE, listOf(DOMAIN_1, DOMAIN_2)) + val pkgAfter = mockPkgSetting(pkgName, UUID_TWO, emptyList()) + + val map = mutableMapOf() + val service = makeService { map[it] } + service.addPackage(pkgBefore) + + // Only insert the package after addPackage call to ensure the service doesn't access + // a live package inside the addPackage logic. It should only use the provided input. + map[pkgName] = pkgBefore + + assertThat(service.getInfo(pkgName).hostToStateMap).containsExactlyEntriesIn(mapOf( + DOMAIN_1 to STATE_NO_RESPONSE, + DOMAIN_2 to STATE_NO_RESPONSE, + )) + assertThat(service.getUserState(pkgName).hostToStateMap).containsExactlyEntriesIn(mapOf( + DOMAIN_1 to DOMAIN_STATE_NONE, + DOMAIN_2 to DOMAIN_STATE_NONE, + )) + assertThat(service.queryValidVerificationPackageNames()).containsExactly(pkgName) + + // Now remove the package because migrateState shouldn't use it either + map.remove(pkgName) + + service.migrateState(pkgBefore, pkgAfter) + + map[pkgName] = pkgAfter + + assertFailsWith(IllegalArgumentException::class) { + service.setDomainVerificationStatus(UUID_ONE, mutableSetOf(DOMAIN_1), STATE_SUCCESS) + } + + assertFailsWith(IllegalArgumentException::class) { + service.setDomainVerificationUserSelection( + UUID_ONE, mutableSetOf(DOMAIN_2), true, USER_ID) + } + + assertThat(service.getDomainVerificationInfo(pkgName)).isNull() + assertThat(service.getUserState(pkgName).hostToStateMap).isEmpty() + assertThat(service.queryValidVerificationPackageNames()).isEmpty() + } + + @Test + fun migratePackageAddDomain() { + val pkgName = PKG_ONE + val pkgBefore = mockPkgSetting(pkgName, UUID_ONE, + listOf(DOMAIN_1, DOMAIN_2)) + val pkgAfter = mockPkgSetting(pkgName, UUID_TWO, + listOf(DOMAIN_1, DOMAIN_2, DOMAIN_3)) + + // Test 3 domains: + // 1 will be verified and preserved, 2 will be selected and preserved, + // 3 will be new and default + + val map = mutableMapOf() + val service = makeService { map[it] } + service.addPackage(pkgBefore) + + // Only insert the package after addPackage call to ensure the service doesn't access + // a live package inside the addPackage logic. It should only use the provided input. + map[pkgName] = pkgBefore + + service.setDomainVerificationStatus(UUID_ONE, setOf(DOMAIN_1), STATE_SUCCESS) + service.setDomainVerificationUserSelection(UUID_ONE, setOf(DOMAIN_2), true, USER_ID) + + assertThat(service.getInfo(pkgName).hostToStateMap).containsExactlyEntriesIn(mapOf( + DOMAIN_1 to STATE_SUCCESS, + DOMAIN_2 to STATE_NO_RESPONSE, + )) + assertThat(service.getUserState(pkgName).hostToStateMap).containsExactlyEntriesIn(mapOf( + DOMAIN_1 to DOMAIN_STATE_VERIFIED, + DOMAIN_2 to DOMAIN_STATE_SELECTED, + )) + + // Now remove the package because migrateState shouldn't use it either + map.remove(pkgName) + + service.migrateState(pkgBefore, pkgAfter) + + map[pkgName] = pkgAfter + + assertThat(service.getInfo(pkgName).hostToStateMap).containsExactlyEntriesIn(mapOf( + DOMAIN_1 to STATE_SUCCESS, + DOMAIN_2 to STATE_NO_RESPONSE, + DOMAIN_3 to STATE_NO_RESPONSE, + )) + assertThat(service.getUserState(pkgName).hostToStateMap).containsExactlyEntriesIn(mapOf( + DOMAIN_1 to DOMAIN_STATE_VERIFIED, + DOMAIN_2 to DOMAIN_STATE_SELECTED, + DOMAIN_3 to DOMAIN_STATE_NONE, + )) + assertThat(service.queryValidVerificationPackageNames()).containsExactly(pkgName) + } + + @Test + fun migratePackageAddAll() { + val pkgName = PKG_ONE + val pkgBefore = mockPkgSetting(pkgName, UUID_ONE, emptyList()) + val pkgAfter = mockPkgSetting(pkgName, UUID_TWO, listOf(DOMAIN_1, DOMAIN_2)) + + val map = mutableMapOf() + val service = makeService { map[it] } + service.addPackage(pkgBefore) + + // Only insert the package after addPackage call to ensure the service doesn't access + // a live package inside the addPackage logic. It should only use the provided input. + map[pkgName] = pkgBefore + + assertFailsWith(IllegalArgumentException::class) { + service.setDomainVerificationStatus(UUID_ONE, mutableSetOf(DOMAIN_1), STATE_SUCCESS) + } + + assertFailsWith(IllegalArgumentException::class) { + service.setDomainVerificationUserSelection( + UUID_ONE, mutableSetOf(DOMAIN_2), true, USER_ID) + } + + assertThat(service.getDomainVerificationInfo(pkgName)).isNull() + assertThat(service.getUserState(pkgName).hostToStateMap).isEmpty() + assertThat(service.queryValidVerificationPackageNames()).isEmpty() + + // Now remove the package because migrateState shouldn't use it either + map.remove(pkgName) + + service.migrateState(pkgBefore, pkgAfter) + + map[pkgName] = pkgAfter + + assertThat(service.getInfo(pkgName).hostToStateMap).containsExactlyEntriesIn(mapOf( + DOMAIN_1 to STATE_NO_RESPONSE, + DOMAIN_2 to STATE_NO_RESPONSE, + )) + assertThat(service.getUserState(pkgName).hostToStateMap).containsExactlyEntriesIn(mapOf( + DOMAIN_1 to DOMAIN_STATE_NONE, + DOMAIN_2 to DOMAIN_STATE_NONE, + )) + assertThat(service.queryValidVerificationPackageNames()).containsExactly(pkgName) + } + + private fun DomainVerificationService.getInfo(pkgName: String) = + getDomainVerificationInfo(pkgName) + .also { assertThat(it).isNotNull() }!! + + private fun DomainVerificationService.getUserState(pkgName: String) = + getDomainVerificationUserState(pkgName, USER_ID) + .also { assertThat(it).isNotNull() }!! + + private fun makeService(vararg pkgSettings: PackageSetting) = + makeService { pkgName -> pkgSettings.find { pkgName == it.getName()} } + + private fun makeService(pkgSettingFunction: (String) -> PackageSetting? = { null }) = + DomainVerificationService(mockThrowOnUnmocked { + // Assume the test has every permission necessary + whenever(enforcePermission(anyString(), anyInt(), anyInt(), anyString())) + whenever(checkPermission(anyString(), anyInt(), anyInt())) { + PackageManager.PERMISSION_GRANTED + } + }, mockThrowOnUnmocked { + whenever(linkedApps) { ArraySet() } + }, mockThrowOnUnmocked { + whenever(isChangeEnabled(ArgumentMatchers.anyLong(), any())) { true } + }).apply { + setConnection(mockThrowOnUnmocked { + whenever(filterAppAccess(anyString(), anyInt(), anyInt())) { false } + whenever(scheduleWriteSettings()) + + // Need to provide an internal UID so some permission checks are ignored + whenever(callingUid) { Process.ROOT_UID } + whenever(callingUserId) { 0 } + + whenever(getPackageSettingLocked(anyString())) { + pkgSettingFunction(arguments[0] as String)!! + } + whenever(getPackageLocked(anyString())) { + pkgSettingFunction(arguments[0] as String)!!.getPkg() + } + }) + } + + private fun mockPkgSetting(pkgName: String, domainSetId: UUID, domains: List = listOf( + DOMAIN_1, DOMAIN_2 + )) = mockThrowOnUnmocked { + val pkg = mockThrowOnUnmocked { + whenever(packageName) { pkgName } + whenever(targetSdkVersion) { Build.VERSION_CODES.S } + + val activityList = listOf( + ParsedActivity().apply { + domains.forEach { + addIntent( + ParsedIntentInfo().apply { + autoVerify = true + addAction(Intent.ACTION_VIEW) + addCategory(Intent.CATEGORY_BROWSABLE) + addCategory(Intent.CATEGORY_DEFAULT) + addDataScheme("http") + addDataScheme("https") + addDataPath("/sub", PatternMatcher.PATTERN_LITERAL) + addDataAuthority(it, null) + } + ) + } + }, + ) + + whenever(activities) { activityList } + } + + whenever(getPkg()) { pkg } + whenever(getName()) { pkgName } + whenever(this.domainSetId) { domainSetId } + whenever(getInstantApp(anyInt())) { false } + whenever(firstInstallTime) { 0L } + } +} From f9e1c3023981e90b5771bf621407190a013ebea7 Mon Sep 17 00:00:00 2001 From: Winson Date: Tue, 9 Mar 2021 12:59:17 -0800 Subject: [PATCH 2/7] Move domain state ints to DomainVerificationInfo Hides the internal values and exposes a smaller set of defined values for each specific case the domain verification agent might be interested in. These values are only for the public API output and all other classes should continue to use the internal DomainVerificationState. Bug: 181100856 Test: atest com.android.server.pm.test.verify.domain Change-Id: I2b16d14bbf3a70e4c2e3f4b358af8671aed71490 --- core/api/system-current.txt | 11 +- .../verify/domain/DomainVerificationInfo.java | 122 +++++++++++++----- .../domain/DomainVerificationManager.java | 105 +-------------- .../domain/DomainVerificationState.java | 97 ++++++++++++-- .../domain/DomainVerificationDebug.java | 3 +- .../domain/DomainVerificationService.java | 20 ++- .../domain/DomainVerificationCollectorTest.kt | 2 +- .../domain/DomainVerificationCoreApiTest.kt | 5 +- .../domain/DomainVerificationEnforcerTest.kt | 11 +- .../DomainVerificationLegacySettingsTest.kt | 2 +- .../DomainVerificationModelExtensions.kt | 2 +- .../domain/DomainVerificationPackageTest.kt | 18 +-- .../DomainVerificationPersistenceTest.kt | 12 +- .../domain/DomainVerificationProxyTest.kt | 14 +- .../DomainVerificationSettingsMutationTest.kt | 13 +- ...inVerificationUserSelectionOverrideTest.kt | 4 +- 16 files changed, 243 insertions(+), 198 deletions(-) diff --git a/core/api/system-current.txt b/core/api/system-current.txt index 39e259d7db9ab..e014716e71bd9 100644 --- a/core/api/system-current.txt +++ b/core/api/system-current.txt @@ -2835,21 +2835,22 @@ package android.content.pm.verify.domain { method @NonNull public String getPackageName(); method public void writeToParcel(@NonNull android.os.Parcel, int); field @NonNull public static final android.os.Parcelable.Creator CREATOR; + field public static final int STATE_FIRST_VERIFIER_DEFINED = 1024; // 0x400 + field public static final int STATE_MODIFIABLE_UNVERIFIED = 3; // 0x3 + field public static final int STATE_MODIFIABLE_VERIFIED = 4; // 0x4 + field public static final int STATE_NO_RESPONSE = 0; // 0x0 + field public static final int STATE_SUCCESS = 1; // 0x1 + field public static final int STATE_UNMODIFIABLE = 2; // 0x2 } public final class DomainVerificationManager { method @Nullable @RequiresPermission(anyOf={android.Manifest.permission.DOMAIN_VERIFICATION_AGENT, android.Manifest.permission.UPDATE_DOMAIN_VERIFICATION_USER_SELECTION}) public android.content.pm.verify.domain.DomainVerificationInfo getDomainVerificationInfo(@NonNull String) throws android.content.pm.PackageManager.NameNotFoundException; method @NonNull @RequiresPermission(android.Manifest.permission.UPDATE_DOMAIN_VERIFICATION_USER_SELECTION) public java.util.List getOwnersForDomain(@NonNull String); - method public static boolean isStateModifiable(int); - method public static boolean isStateVerified(int); method @NonNull @RequiresPermission(android.Manifest.permission.DOMAIN_VERIFICATION_AGENT) public java.util.List queryValidVerificationPackageNames(); method @RequiresPermission(android.Manifest.permission.UPDATE_DOMAIN_VERIFICATION_USER_SELECTION) public void setDomainVerificationLinkHandlingAllowed(@NonNull String, boolean) throws android.content.pm.PackageManager.NameNotFoundException; method @RequiresPermission(android.Manifest.permission.DOMAIN_VERIFICATION_AGENT) public void setDomainVerificationStatus(@NonNull java.util.UUID, @NonNull java.util.Set, int) throws android.content.pm.PackageManager.NameNotFoundException; method @RequiresPermission(android.Manifest.permission.UPDATE_DOMAIN_VERIFICATION_USER_SELECTION) public void setDomainVerificationUserSelection(@NonNull java.util.UUID, @NonNull java.util.Set, boolean) throws android.content.pm.PackageManager.NameNotFoundException; field public static final String EXTRA_VERIFICATION_REQUEST = "android.content.pm.verify.domain.extra.VERIFICATION_REQUEST"; - field public static final int STATE_FIRST_VERIFIER_DEFINED = 1024; // 0x400 - field public static final int STATE_NO_RESPONSE = 0; // 0x0 - field public static final int STATE_SUCCESS = 1; // 0x1 } public final class DomainVerificationRequest implements android.os.Parcelable { diff --git a/core/java/android/content/pm/verify/domain/DomainVerificationInfo.java b/core/java/android/content/pm/verify/domain/DomainVerificationInfo.java index 7c335b1d26dda..62277ef671e3a 100644 --- a/core/java/android/content/pm/verify/domain/DomainVerificationInfo.java +++ b/core/java/android/content/pm/verify/domain/DomainVerificationInfo.java @@ -43,9 +43,50 @@ import java.util.UUID; */ @SystemApi @DataClass(genAidl = true, genHiddenConstructor = true, genParcelable = true, genToString = true, - genEqualsHashCode = true) + genEqualsHashCode = true, genHiddenConstDefs = true) public final class DomainVerificationInfo implements Parcelable { + // Implementation note: the following states are OUTPUT only. Any value that is synonymous with + // a value in DomainVerificationState must be the EXACT same integer, so that state + // transformation does not have to occur when sending input into the system, assuming that the + // system only accepts those synonymous values. The public API values declared here are only + // used when exiting the system server to prepare this data object for consumption by the + // verification agent. These constants should only be referenced inside public API classes. + // The server must use DomainVerificationState. + + /** + * No response has been recorded by either the system or any verification agent. + */ + public static final int STATE_NO_RESPONSE = DomainVerificationState.STATE_NO_RESPONSE; + + /** + * The domain has been explicitly verified. + */ + public static final int STATE_SUCCESS = DomainVerificationState.STATE_SUCCESS; + + /** + * Indicates the host cannot be modified by the verification agent. + */ + public static final int STATE_UNMODIFIABLE = 2; + + /** + * Indicates the host can be modified by the verification agent and is not considered verified. + */ + public static final int STATE_MODIFIABLE_UNVERIFIED = 3; + + /** + * Indicates the host can be modified by the verification agent and is considered verified. + */ + public static final int STATE_MODIFIABLE_VERIFIED = 4; + + /** + * The first available custom response code. This and any greater integer, along with {@link + * #STATE_SUCCESS} are the only values settable by the verification agent. All custom values + * will be treated as if the domain is unverified. + */ + public static final int STATE_FIRST_VERIFIER_DEFINED = + DomainVerificationState.STATE_FIRST_VERIFIER_DEFINED; + /** * A domain verification ID for use in later API calls. This represents the snapshot of the * domains for a package on device, and will be invalidated whenever the package changes. @@ -74,16 +115,12 @@ public final class DomainVerificationInfo implements Parcelable { /** * Map of host names to their current state. State is an integer, which defaults to {@link - * DomainVerificationManager#STATE_NO_RESPONSE}. State can be modified by the domain - * verification agent (the intended consumer of this API), which can be equal to {@link - * DomainVerificationManager#STATE_SUCCESS} when verified, or equal to or greater than {@link - * DomainVerificationManager#STATE_FIRST_VERIFIER_DEFINED} for any unsuccessful response. + * #STATE_NO_RESPONSE}. State can be modified by the domain verification agent (the intended + * consumer of this API), which can be equal to {@link #STATE_SUCCESS} when verified, or equal + * to or greater than {@link #STATE_FIRST_VERIFIER_DEFINED} for any unsuccessful response. *

- * Any value non-inclusive between those 2 values are reserved for use by the system. The domain - * verification agent may be able to act on these reserved values, and this ability can be - * queried using {@link DomainVerificationManager#isStateModifiable(int)}. It is expected that - * the agent attempt to verify all domains that it can modify the state of, even if it does not - * understand the meaning of those values. + * Hosts which cannot be edited will be assigned {@link #STATE_UNMODIFIABLE}. It is expected + * that the agent attempt to verify all domains that it can modify the state of. */ @NonNull private final Map mHostToStateMap; @@ -112,6 +149,39 @@ public final class DomainVerificationInfo implements Parcelable { //@formatter:off + /** @hide */ + @android.annotation.IntDef(prefix = "STATE_", value = { + STATE_NO_RESPONSE, + STATE_SUCCESS, + STATE_UNMODIFIABLE, + STATE_MODIFIABLE_UNVERIFIED, + STATE_MODIFIABLE_VERIFIED, + STATE_FIRST_VERIFIER_DEFINED + }) + @java.lang.annotation.Retention(java.lang.annotation.RetentionPolicy.SOURCE) + @DataClass.Generated.Member + public @interface State {} + + /** @hide */ + @DataClass.Generated.Member + public static String stateToString(@State int value) { + switch (value) { + case STATE_NO_RESPONSE: + return "STATE_NO_RESPONSE"; + case STATE_SUCCESS: + return "STATE_SUCCESS"; + case STATE_UNMODIFIABLE: + return "STATE_UNMODIFIABLE"; + case STATE_MODIFIABLE_UNVERIFIED: + return "STATE_MODIFIABLE_UNVERIFIED"; + case STATE_MODIFIABLE_VERIFIED: + return "STATE_MODIFIABLE_VERIFIED"; + case STATE_FIRST_VERIFIER_DEFINED: + return "STATE_FIRST_VERIFIER_DEFINED"; + default: return Integer.toHexString(value); + } + } + /** * Creates a new DomainVerificationInfo. * @@ -134,16 +204,12 @@ public final class DomainVerificationInfo implements Parcelable { * The package name that this data corresponds to. * @param hostToStateMap * Map of host names to their current state. State is an integer, which defaults to {@link - * DomainVerificationManager#STATE_NO_RESPONSE}. State can be modified by the domain - * verification agent (the intended consumer of this API), which can be equal to {@link - * DomainVerificationManager#STATE_SUCCESS} when verified, or equal to or greater than {@link - * DomainVerificationManager#STATE_FIRST_VERIFIER_DEFINED} for any unsuccessful response. + * #STATE_NO_RESPONSE}. State can be modified by the domain verification agent (the intended + * consumer of this API), which can be equal to {@link #STATE_SUCCESS} when verified, or equal + * to or greater than {@link #STATE_FIRST_VERIFIER_DEFINED} for any unsuccessful response. *

- * Any value non-inclusive between those 2 values are reserved for use by the system. The domain - * verification agent may be able to act on these reserved values, and this ability can be - * queried using {@link DomainVerificationManager#isStateModifiable(int)}. It is expected that - * the agent attempt to verify all domains that it can modify the state of, even if it does not - * understand the meaning of those values. + * Hosts which cannot be edited will be assigned {@link #STATE_UNMODIFIABLE}. It is expected + * that the agent attempt to verify all domains that it can modify the state of. * @hide */ @DataClass.Generated.Member @@ -195,16 +261,12 @@ public final class DomainVerificationInfo implements Parcelable { /** * Map of host names to their current state. State is an integer, which defaults to {@link - * DomainVerificationManager#STATE_NO_RESPONSE}. State can be modified by the domain - * verification agent (the intended consumer of this API), which can be equal to {@link - * DomainVerificationManager#STATE_SUCCESS} when verified, or equal to or greater than {@link - * DomainVerificationManager#STATE_FIRST_VERIFIER_DEFINED} for any unsuccessful response. + * #STATE_NO_RESPONSE}. State can be modified by the domain verification agent (the intended + * consumer of this API), which can be equal to {@link #STATE_SUCCESS} when verified, or equal + * to or greater than {@link #STATE_FIRST_VERIFIER_DEFINED} for any unsuccessful response. *

- * Any value non-inclusive between those 2 values are reserved for use by the system. The domain - * verification agent may be able to act on these reserved values, and this ability can be - * queried using {@link DomainVerificationManager#isStateModifiable(int)}. It is expected that - * the agent attempt to verify all domains that it can modify the state of, even if it does not - * understand the meaning of those values. + * Hosts which cannot be edited will be assigned {@link #STATE_UNMODIFIABLE}. It is expected + * that the agent attempt to verify all domains that it can modify the state of. */ @DataClass.Generated.Member public @NonNull Map getHostToStateMap() { @@ -320,10 +382,10 @@ public final class DomainVerificationInfo implements Parcelable { }; @DataClass.Generated( - time = 1614721812023L, + time = 1615317187669L, codegenVersion = "1.0.22", sourceFile = "frameworks/base/core/java/android/content/pm/verify/domain/DomainVerificationInfo.java", - inputSignatures = "private final @android.annotation.NonNull @com.android.internal.util.DataClass.ParcelWith(com.android.internal.util.Parcelling.BuiltIn.ForUUID.class) java.util.UUID mIdentifier\nprivate final @android.annotation.NonNull java.lang.String mPackageName\nprivate final @android.annotation.NonNull java.util.Map mHostToStateMap\nprivate void parcelHostToStateMap(android.os.Parcel,int)\nprivate java.util.Map unparcelHostToStateMap(android.os.Parcel)\nclass DomainVerificationInfo extends java.lang.Object implements [android.os.Parcelable]\n@com.android.internal.util.DataClass(genAidl=true, genHiddenConstructor=true, genParcelable=true, genToString=true, genEqualsHashCode=true)") + inputSignatures = "public static final int STATE_NO_RESPONSE\npublic static final int STATE_SUCCESS\npublic static final int STATE_UNMODIFIABLE\npublic static final int STATE_MODIFIABLE_UNVERIFIED\npublic static final int STATE_MODIFIABLE_VERIFIED\npublic static final int STATE_FIRST_VERIFIER_DEFINED\nprivate final @android.annotation.NonNull @com.android.internal.util.DataClass.ParcelWith(com.android.internal.util.Parcelling.BuiltIn.ForUUID.class) java.util.UUID mIdentifier\nprivate final @android.annotation.NonNull java.lang.String mPackageName\nprivate final @android.annotation.NonNull java.util.Map mHostToStateMap\nprivate void parcelHostToStateMap(android.os.Parcel,int)\nprivate java.util.Map unparcelHostToStateMap(android.os.Parcel)\nclass DomainVerificationInfo extends java.lang.Object implements [android.os.Parcelable]\n@com.android.internal.util.DataClass(genAidl=true, genHiddenConstructor=true, genParcelable=true, genToString=true, genEqualsHashCode=true, genHiddenConstDefs=true)") @Deprecated private void __metadata() {} diff --git a/core/java/android/content/pm/verify/domain/DomainVerificationManager.java b/core/java/android/content/pm/verify/domain/DomainVerificationManager.java index ac7fdc557340e..c92f8cc525ecc 100644 --- a/core/java/android/content/pm/verify/domain/DomainVerificationManager.java +++ b/core/java/android/content/pm/verify/domain/DomainVerificationManager.java @@ -59,109 +59,6 @@ public final class DomainVerificationManager { public static final String EXTRA_VERIFICATION_REQUEST = "android.content.pm.verify.domain.extra.VERIFICATION_REQUEST"; - /** - * No response has been recorded by either the system or any verification agent. - * - * @hide - */ - @SystemApi - public static final int STATE_NO_RESPONSE = DomainVerificationState.STATE_NO_RESPONSE; - - /** - * The verification agent has explicitly verified the domain at some point. - * - * @hide - */ - @SystemApi - public static final int STATE_SUCCESS = DomainVerificationState.STATE_SUCCESS; - - /** - * The first available custom response code. This and any greater integer, along with {@link - * #STATE_SUCCESS} are the only values settable by the verification agent. All values will be - * treated as if the domain is unverified. - * - * @hide - */ - @SystemApi - public static final int STATE_FIRST_VERIFIER_DEFINED = - DomainVerificationState.STATE_FIRST_VERIFIER_DEFINED; - - /** - * @hide - */ - @NonNull - public static String stateToDebugString(@DomainVerificationState.State int state) { - switch (state) { - case DomainVerificationState.STATE_NO_RESPONSE: - return "none"; - case DomainVerificationState.STATE_SUCCESS: - return "verified"; - case DomainVerificationState.STATE_APPROVED: - return "approved"; - case DomainVerificationState.STATE_DENIED: - return "denied"; - case DomainVerificationState.STATE_MIGRATED: - return "migrated"; - case DomainVerificationState.STATE_RESTORED: - return "restored"; - case DomainVerificationState.STATE_LEGACY_FAILURE: - return "legacy_failure"; - case DomainVerificationState.STATE_SYS_CONFIG: - return "system_configured"; - default: - return String.valueOf(state); - } - } - - /** - * Checks if a state considers the corresponding domain to be successfully verified. The domain - * verification agent may use this to determine whether or not to re-verify a domain. - * - * @hide - */ - @SystemApi - public static boolean isStateVerified(@DomainVerificationState.State int state) { - switch (state) { - case DomainVerificationState.STATE_SUCCESS: - case DomainVerificationState.STATE_APPROVED: - case DomainVerificationState.STATE_MIGRATED: - case DomainVerificationState.STATE_RESTORED: - case DomainVerificationState.STATE_SYS_CONFIG: - return true; - case DomainVerificationState.STATE_NO_RESPONSE: - case DomainVerificationState.STATE_DENIED: - case DomainVerificationState.STATE_LEGACY_FAILURE: - default: - return false; - } - } - - /** - * Checks if a state is modifiable by the domain verification agent. This is useful as the - * platform may add new state codes in newer versions, and older verification agents can use - * this method to determine if a state can be changed without having to be aware of what the new - * state means. - * - * @hide - */ - @SystemApi - public static boolean isStateModifiable(@DomainVerificationState.State int state) { - switch (state) { - case DomainVerificationState.STATE_NO_RESPONSE: - case DomainVerificationState.STATE_SUCCESS: - case DomainVerificationState.STATE_MIGRATED: - case DomainVerificationState.STATE_RESTORED: - case DomainVerificationState.STATE_LEGACY_FAILURE: - return true; - case DomainVerificationState.STATE_APPROVED: - case DomainVerificationState.STATE_DENIED: - case DomainVerificationState.STATE_SYS_CONFIG: - return false; - default: - return state >= DomainVerificationState.STATE_FIRST_VERIFIER_DEFINED; - } - } - /** * @hide */ @@ -282,7 +179,7 @@ public final class DomainVerificationManager { @SystemApi @RequiresPermission(android.Manifest.permission.DOMAIN_VERIFICATION_AGENT) public void setDomainVerificationStatus(@NonNull UUID domainSetId, @NonNull Set domains, - @DomainVerificationState.State int state) throws NameNotFoundException { + int state) throws NameNotFoundException { try { mDomainVerificationManager.setDomainVerificationStatus(domainSetId.toString(), new DomainSet(domains), state); diff --git a/core/java/android/content/pm/verify/domain/DomainVerificationState.java b/core/java/android/content/pm/verify/domain/DomainVerificationState.java index f02f2d9412dc1..8e28042bf581a 100644 --- a/core/java/android/content/pm/verify/domain/DomainVerificationState.java +++ b/core/java/android/content/pm/verify/domain/DomainVerificationState.java @@ -17,15 +17,13 @@ package android.content.pm.verify.domain; import android.annotation.IntDef; +import android.annotation.NonNull; /** * @hide */ public interface DomainVerificationState { - /** - * @hide - */ @IntDef({ STATE_NO_RESPONSE, STATE_SUCCESS, @@ -42,12 +40,12 @@ public interface DomainVerificationState { // TODO(b/159952358): Document all the places that states need to be updated when one is added /** - * @see DomainVerificationManager#STATE_NO_RESPONSE + * @see DomainVerificationInfo#STATE_NO_RESPONSE */ int STATE_NO_RESPONSE = 0; /** - * @see DomainVerificationManager#STATE_SUCCESS + * @see DomainVerificationInfo#STATE_SUCCESS */ int STATE_SUCCESS = 1; @@ -94,16 +92,37 @@ public interface DomainVerificationState { int STATE_SYS_CONFIG = 7; /** - * @see DomainVerificationManager#STATE_FIRST_VERIFIER_DEFINED + * @see DomainVerificationInfo#STATE_FIRST_VERIFIER_DEFINED */ int STATE_FIRST_VERIFIER_DEFINED = 0b10000000000; + @NonNull + static String stateToDebugString(@DomainVerificationState.State int state) { + switch (state) { + case DomainVerificationState.STATE_NO_RESPONSE: + return "none"; + case DomainVerificationState.STATE_SUCCESS: + return "verified"; + case DomainVerificationState.STATE_APPROVED: + return "approved"; + case DomainVerificationState.STATE_DENIED: + return "denied"; + case DomainVerificationState.STATE_MIGRATED: + return "migrated"; + case DomainVerificationState.STATE_RESTORED: + return "restored"; + case DomainVerificationState.STATE_LEGACY_FAILURE: + return "legacy_failure"; + case DomainVerificationState.STATE_SYS_CONFIG: + return "system_configured"; + default: + return String.valueOf(state); + } + } /** * For determining re-verify policy. This is hidden from the domain verification agent so that * no behavior is made based on the result. - * - * @hide */ static boolean isDefault(@State int state) { switch (state) { @@ -121,11 +140,52 @@ public interface DomainVerificationState { } } + /** + * Checks if a state considers the corresponding domain to be successfully verified. The domain + * verification agent may use this to determine whether or not to re-verify a domain. + */ + static boolean isVerified(@DomainVerificationState.State int state) { + switch (state) { + case DomainVerificationState.STATE_SUCCESS: + case DomainVerificationState.STATE_APPROVED: + case DomainVerificationState.STATE_MIGRATED: + case DomainVerificationState.STATE_RESTORED: + case DomainVerificationState.STATE_SYS_CONFIG: + return true; + case DomainVerificationState.STATE_NO_RESPONSE: + case DomainVerificationState.STATE_DENIED: + case DomainVerificationState.STATE_LEGACY_FAILURE: + default: + return false; + } + } + + /** + * Checks if a state is modifiable by the domain verification agent. This is useful as the + * platform may add new state codes in newer versions, and older verification agents can use + * this method to determine if a state can be changed without having to be aware of what the new + * state means. + */ + static boolean isModifiable(@DomainVerificationState.State int state) { + switch (state) { + case DomainVerificationState.STATE_NO_RESPONSE: + case DomainVerificationState.STATE_SUCCESS: + case DomainVerificationState.STATE_MIGRATED: + case DomainVerificationState.STATE_RESTORED: + case DomainVerificationState.STATE_LEGACY_FAILURE: + return true; + case DomainVerificationState.STATE_APPROVED: + case DomainVerificationState.STATE_DENIED: + case DomainVerificationState.STATE_SYS_CONFIG: + return false; + default: + return state >= DomainVerificationState.STATE_FIRST_VERIFIER_DEFINED; + } + } + /** * Whether the state is migrated when updating a package. Generally this is only for states * that maintain verification state or were set by an explicit user or developer action. - * - * @hide */ static boolean shouldMigrate(@State int state) { switch (state) { @@ -143,4 +203,21 @@ public interface DomainVerificationState { return false; } } + + @DomainVerificationInfo.State + static int convertToInfoState(@State int internalState) { + if (internalState >= STATE_FIRST_VERIFIER_DEFINED) { + return internalState; + } else if (internalState == STATE_NO_RESPONSE) { + return DomainVerificationInfo.STATE_NO_RESPONSE; + } else if (internalState == STATE_SUCCESS) { + return DomainVerificationInfo.STATE_SUCCESS; + } else if (!isModifiable(internalState)) { + return DomainVerificationInfo.STATE_UNMODIFIABLE; + } else if (isVerified(internalState)) { + return DomainVerificationInfo.STATE_MODIFIABLE_VERIFIED; + } else { + return DomainVerificationInfo.STATE_MODIFIABLE_UNVERIFIED; + } + } } diff --git a/services/core/java/com/android/server/pm/verify/domain/DomainVerificationDebug.java b/services/core/java/com/android/server/pm/verify/domain/DomainVerificationDebug.java index b61fd8d633f61..39ed4882c69c1 100644 --- a/services/core/java/com/android/server/pm/verify/domain/DomainVerificationDebug.java +++ b/services/core/java/com/android/server/pm/verify/domain/DomainVerificationDebug.java @@ -21,7 +21,6 @@ import android.annotation.Nullable; import android.annotation.UserIdInt; import android.content.pm.PackageManager.NameNotFoundException; import android.content.pm.Signature; -import android.content.pm.verify.domain.DomainVerificationManager; import android.content.pm.verify.domain.DomainVerificationState; import android.os.UserHandle; import android.util.ArrayMap; @@ -151,7 +150,7 @@ public class DomainVerificationDebug { Integer state = reusedMap.valueAt(stateIndex); writer.print(domain); writer.print(": "); - writer.println(DomainVerificationManager.stateToDebugString(state)); + writer.println(DomainVerificationState.stateToDebugString(state)); } writer.decreaseIndent(); writer.decreaseIndent(); 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 6c6df9ab3f1fd..4c585e91f5a44 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 @@ -33,7 +33,6 @@ import android.content.pm.ResolveInfo; import android.content.pm.parsing.component.ParsedActivity; import android.content.pm.verify.domain.DomainOwner; import android.content.pm.verify.domain.DomainVerificationInfo; -import android.content.pm.verify.domain.DomainVerificationManager; import android.content.pm.verify.domain.DomainVerificationManager.InvalidDomainSetException; import android.content.pm.verify.domain.DomainVerificationState; import android.content.pm.verify.domain.DomainVerificationUserState; @@ -253,7 +252,7 @@ public class DomainVerificationService extends SystemService throw DomainVerificationUtils.throwPackageUnavailable(packageName); } - Map hostToStateMap = new ArrayMap<>(pkgState.getStateMap()); + ArrayMap hostToStateMap = new ArrayMap<>(pkgState.getStateMap()); // TODO(b/159952358): Should the domain list be cached? ArraySet domains = mCollector.collectValidAutoVerifyDomains(pkg); @@ -267,6 +266,13 @@ public class DomainVerificationService extends SystemService DomainVerificationState.STATE_NO_RESPONSE); } + final int mapSize = hostToStateMap.size(); + for (int index = 0; index < mapSize; index++) { + int internalValue = hostToStateMap.valueAt(index); + int publicValue = DomainVerificationState.convertToInfoState(internalValue); + hostToStateMap.setValueAt(index, publicValue); + } + // TODO(b/159952358): Do not return if no values are editable (all ignored states)? return new DomainVerificationInfo(pkgState.getId(), packageName, hostToStateMap); } @@ -300,11 +306,11 @@ public class DomainVerificationService extends SystemService for (String domain : domains) { Integer previousState = stateMap.get(domain); if (previousState != null - && !DomainVerificationManager.isStateModifiable(previousState)) { + && !DomainVerificationState.isModifiable(previousState)) { continue; } - if (DomainVerificationManager.isStateVerified(state)) { + if (DomainVerificationState.isVerified(state)) { verifiedDomains.add(domain); } @@ -659,7 +665,7 @@ public class DomainVerificationService extends SystemService Integer state = stateMap.get(host); int domainState; - if (state != null && DomainVerificationManager.isStateVerified(state)) { + if (state != null && DomainVerificationState.isVerified(state)) { domainState = DomainVerificationUserState.DOMAIN_STATE_VERIFIED; } else if (enabledHosts.contains(host)) { domainState = DomainVerificationUserState.DOMAIN_STATE_SELECTED; @@ -1535,7 +1541,7 @@ public class DomainVerificationService extends SystemService ArrayMap stateMap = pkgState.getStateMap(); // Check if the exact host matches Integer state = stateMap.get(host); - if (state != null && DomainVerificationManager.isStateVerified(state)) { + if (state != null && DomainVerificationState.isVerified(state)) { if (DEBUG_APPROVAL) { debugApproval(packageName, debugObject, userId, true, "host verified exactly"); @@ -1546,7 +1552,7 @@ public class DomainVerificationService extends SystemService // Otherwise see if the host matches a verified domain by wildcard int stateMapSize = stateMap.size(); for (int index = 0; index < stateMapSize; index++) { - if (!DomainVerificationManager.isStateVerified(stateMap.valueAt(index))) { + if (!DomainVerificationState.isVerified(stateMap.valueAt(index))) { continue; } diff --git a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationCollectorTest.kt b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationCollectorTest.kt index 0fa9a1def3812..d5eda203e42f6 100644 --- a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationCollectorTest.kt +++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationCollectorTest.kt @@ -25,8 +25,8 @@ import android.os.PatternMatcher import android.util.ArraySet import com.android.server.SystemConfig import com.android.server.compat.PlatformCompat -import com.android.server.pm.verify.domain.DomainVerificationCollector import com.android.server.pm.parsing.pkg.AndroidPackage +import com.android.server.pm.verify.domain.DomainVerificationCollector import com.android.server.testutils.mockThrowOnUnmocked import com.android.server.testutils.whenever import com.google.common.truth.Truth.assertThat diff --git a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationCoreApiTest.kt b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationCoreApiTest.kt index 8ef92393242a9..9693f3bab127d 100644 --- a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationCoreApiTest.kt +++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationCoreApiTest.kt @@ -19,6 +19,7 @@ package com.android.server.pm.test.verify.domain import android.content.pm.verify.domain.DomainSet import android.content.pm.verify.domain.DomainVerificationInfo import android.content.pm.verify.domain.DomainVerificationRequest +import android.content.pm.verify.domain.DomainVerificationState import android.content.pm.verify.domain.DomainVerificationUserState import android.os.Parcel import android.os.Parcelable @@ -74,7 +75,9 @@ class DomainVerificationCoreApiTest { DomainVerificationInfo( UUID.fromString("703f6d34-6241-4cfd-8176-2e1d23355811"), "com.test.pkg", - massiveSet.withIndex().associate { it.value to it.index } + massiveSet.withIndex().associate { + it.value to DomainVerificationState.convertToInfoState(it.index) + } ) }, unparcel = { DomainVerificationInfo.CREATOR.createFromParcel(it) }, 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 53f0ca20e787f..c9df5669ced27 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 @@ -20,20 +20,21 @@ import android.content.Context import android.content.Intent import android.content.pm.PackageManager import android.content.pm.PackageUserState -import android.content.pm.verify.domain.DomainVerificationManager import android.content.pm.parsing.component.ParsedActivity import android.content.pm.parsing.component.ParsedIntentInfo +import android.content.pm.verify.domain.DomainVerificationManager +import android.content.pm.verify.domain.DomainVerificationState import android.os.Build import android.os.Process import android.util.ArraySet import android.util.SparseArray import androidx.test.platform.app.InstrumentationRegistry import com.android.server.pm.PackageSetting +import com.android.server.pm.parsing.pkg.AndroidPackage import com.android.server.pm.verify.domain.DomainVerificationEnforcer import com.android.server.pm.verify.domain.DomainVerificationManagerInternal import com.android.server.pm.verify.domain.DomainVerificationService import com.android.server.pm.verify.domain.proxy.DomainVerificationProxy -import com.android.server.pm.parsing.pkg.AndroidPackage import com.android.server.testutils.mockThrowOnUnmocked import com.android.server.testutils.spyThrowOnUnmocked import com.android.server.testutils.whenever @@ -175,7 +176,7 @@ class DomainVerificationEnforcerTest { service(Type.INTERNAL, "setStatusInternalPackageName") { setDomainVerificationStatusInternal( it.targetPackageName, - DomainVerificationManager.STATE_SUCCESS, + DomainVerificationState.STATE_SUCCESS, ArraySet(setOf("example.com")) ) }, @@ -206,7 +207,7 @@ class DomainVerificationEnforcerTest { setDomainVerificationStatus( it.targetDomainSetId, setOf("example.com"), - DomainVerificationManager.STATE_SUCCESS + DomainVerificationState.STATE_SUCCESS ) }, service(Type.VERIFIER, "setStatusInternalUid") { @@ -214,7 +215,7 @@ class DomainVerificationEnforcerTest { it.callingUid, it.targetDomainSetId, setOf("example.com"), - DomainVerificationManager.STATE_SUCCESS + DomainVerificationState.STATE_SUCCESS ) }, service(Type.SELECTOR_USER, "setLinkHandlingAllowedUserId") { diff --git a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationLegacySettingsTest.kt b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationLegacySettingsTest.kt index 9a3bd994eac0c..509824d93512f 100644 --- a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationLegacySettingsTest.kt +++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationLegacySettingsTest.kt @@ -19,9 +19,9 @@ package com.android.server.pm.test.verify.domain import android.content.pm.IntentFilterVerificationInfo import android.content.pm.PackageManager import android.util.ArraySet -import com.android.server.pm.verify.domain.DomainVerificationLegacySettings import com.android.server.pm.test.verify.domain.DomainVerificationPersistenceTest.Companion.readXml import com.android.server.pm.test.verify.domain.DomainVerificationPersistenceTest.Companion.writeXml +import com.android.server.pm.verify.domain.DomainVerificationLegacySettings import com.google.common.truth.Truth.assertWithMessage import org.junit.Rule import org.junit.Test diff --git a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationModelExtensions.kt b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationModelExtensions.kt index 8c31c65e1b0a1..a5db3c5786700 100644 --- a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationModelExtensions.kt +++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationModelExtensions.kt @@ -16,8 +16,8 @@ package com.android.server.pm.test.verify.domain -import android.content.pm.verify.domain.DomainVerificationRequest import android.content.pm.verify.domain.DomainVerificationInfo +import android.content.pm.verify.domain.DomainVerificationRequest import android.content.pm.verify.domain.DomainVerificationUserState import com.android.server.pm.verify.domain.DomainVerificationPersistence 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 8d99005053c14..3b3dd61c44b16 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 @@ -20,10 +20,10 @@ import android.content.Intent import android.content.pm.PackageManager import android.content.pm.parsing.component.ParsedActivity import android.content.pm.parsing.component.ParsedIntentInfo -import android.content.pm.verify.domain.DomainVerificationState.STATE_APPROVED -import android.content.pm.verify.domain.DomainVerificationState.STATE_DENIED -import android.content.pm.verify.domain.DomainVerificationState.STATE_NO_RESPONSE -import android.content.pm.verify.domain.DomainVerificationState.STATE_SUCCESS +import android.content.pm.verify.domain.DomainVerificationInfo.STATE_NO_RESPONSE +import android.content.pm.verify.domain.DomainVerificationInfo.STATE_SUCCESS +import android.content.pm.verify.domain.DomainVerificationInfo.STATE_UNMODIFIABLE +import android.content.pm.verify.domain.DomainVerificationState import android.content.pm.verify.domain.DomainVerificationUserState.DOMAIN_STATE_NONE import android.content.pm.verify.domain.DomainVerificationUserState.DOMAIN_STATE_SELECTED import android.content.pm.verify.domain.DomainVerificationUserState.DOMAIN_STATE_VERIFIED @@ -168,9 +168,9 @@ class DomainVerificationPackageTest { map[pkgName] = pkgBefore // To test the approve/denial states, use the internal methods for this variant - service.setDomainVerificationStatusInternal(pkgName, STATE_APPROVED, + service.setDomainVerificationStatusInternal(pkgName, DomainVerificationState.STATE_APPROVED, ArraySet(setOf(DOMAIN_1))) - service.setDomainVerificationStatusInternal(pkgName, STATE_DENIED, + service.setDomainVerificationStatusInternal(pkgName, DomainVerificationState.STATE_DENIED, ArraySet(setOf(DOMAIN_3))) service.setDomainVerificationUserSelection( UUID_ONE, setOf(DOMAIN_2, DOMAIN_4), true, USER_ID) @@ -179,9 +179,9 @@ class DomainVerificationPackageTest { service.setDomainVerificationStatus(UUID_ONE, setOf(DOMAIN_1, DOMAIN_3), STATE_SUCCESS) assertThat(service.getInfo(pkgName).hostToStateMap).containsExactlyEntriesIn(mapOf( - DOMAIN_1 to STATE_APPROVED, + DOMAIN_1 to STATE_UNMODIFIABLE, DOMAIN_2 to STATE_NO_RESPONSE, - DOMAIN_3 to STATE_DENIED, + DOMAIN_3 to STATE_UNMODIFIABLE, DOMAIN_4 to STATE_NO_RESPONSE, )) assertThat(service.getUserState(pkgName).hostToStateMap).containsExactlyEntriesIn(mapOf( @@ -199,7 +199,7 @@ class DomainVerificationPackageTest { service.migrateState(pkgBefore, pkgAfter) assertThat(service.getInfo(pkgName).hostToStateMap).containsExactlyEntriesIn(mapOf( - DOMAIN_1 to STATE_APPROVED, + DOMAIN_1 to STATE_UNMODIFIABLE, DOMAIN_2 to STATE_NO_RESPONSE, )) assertThat(service.getUserState(pkgName).hostToStateMap).containsExactlyEntriesIn(mapOf( diff --git a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationPersistenceTest.kt b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationPersistenceTest.kt index 6597577cf14f3..f8fda12f806d8 100644 --- a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationPersistenceTest.kt +++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationPersistenceTest.kt @@ -16,7 +16,7 @@ package com.android.server.pm.test.verify.domain -import android.content.pm.verify.domain.DomainVerificationManager +import android.content.pm.verify.domain.DomainVerificationState import android.util.ArrayMap import android.util.TypedXmlPullParser import android.util.TypedXmlSerializer @@ -117,11 +117,11 @@ class DomainVerificationPersistenceTest { @Test fun readMalformed() { val stateZero = mockEmptyPkgState(0).apply { - stateMap["example.com"] = DomainVerificationManager.STATE_SUCCESS - stateMap["example.org"] = DomainVerificationManager.STATE_FIRST_VERIFIER_DEFINED + stateMap["example.com"] = DomainVerificationState.STATE_SUCCESS + stateMap["example.org"] = DomainVerificationState.STATE_FIRST_VERIFIER_DEFINED // A domain without a written state falls back to default - stateMap["missing-state.com"] = DomainVerificationManager.STATE_NO_RESPONSE + stateMap["missing-state.com"] = DomainVerificationState.STATE_NO_RESPONSE userStates[1] = DomainVerificationInternalUserState(1).apply { addHosts(setOf("example-user1.com", "example-user1.org")) @@ -159,9 +159,9 @@ class DomainVerificationPersistenceTest { > + DomainVerificationState.STATE_SUCCESS}"/> + DomainVerificationState.STATE_FIRST_VERIFIER_DEFINED}"/> diff --git a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationProxyTest.kt b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationProxyTest.kt index 91e5beccee090..62e3b73ec6ad1 100644 --- a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationProxyTest.kt +++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationProxyTest.kt @@ -21,20 +21,20 @@ import android.content.Context import android.content.Intent import android.content.IntentFilter import android.content.pm.PackageManager +import android.content.pm.verify.domain.DomainVerificationInfo import android.content.pm.verify.domain.DomainVerificationManager import android.content.pm.verify.domain.DomainVerificationRequest -import android.content.pm.verify.domain.DomainVerificationInfo import android.content.pm.verify.domain.DomainVerificationState import android.os.Bundle import android.os.UserHandle import android.util.ArraySet import com.android.server.DeviceIdleInternal +import com.android.server.pm.parsing.pkg.AndroidPackage import com.android.server.pm.verify.domain.DomainVerificationCollector import com.android.server.pm.verify.domain.DomainVerificationManagerInternal import com.android.server.pm.verify.domain.proxy.DomainVerificationProxy import com.android.server.pm.verify.domain.proxy.DomainVerificationProxyV1 import com.android.server.pm.verify.domain.proxy.DomainVerificationProxyV2 -import com.android.server.pm.parsing.pkg.AndroidPackage import com.android.server.testutils.mockThrowOnUnmocked import com.android.server.testutils.whenever import com.google.common.truth.Truth.assertThat @@ -106,14 +106,14 @@ class DomainVerificationProxyTest { when (val pkgName = arguments[0] as String) { TEST_PKG_NAME_TARGET_ONE -> DomainVerificationInfo( TEST_UUID_ONE, pkgName, mapOf( - "example1.com" to DomainVerificationManager.STATE_NO_RESPONSE, - "example2.com" to DomainVerificationManager.STATE_NO_RESPONSE + "example1.com" to DomainVerificationInfo.STATE_NO_RESPONSE, + "example2.com" to DomainVerificationInfo.STATE_NO_RESPONSE ) ) TEST_PKG_NAME_TARGET_TWO -> DomainVerificationInfo( TEST_UUID_TWO, pkgName, mapOf( - "example3.com" to DomainVerificationManager.STATE_NO_RESPONSE, - "example4.com" to DomainVerificationManager.STATE_NO_RESPONSE + "example3.com" to DomainVerificationInfo.STATE_NO_RESPONSE, + "example4.com" to DomainVerificationInfo.STATE_NO_RESPONSE ) ) else -> throw IllegalArgumentException("Unexpected package name $pkgName") @@ -316,7 +316,7 @@ class DomainVerificationProxyTest { eq(TEST_CALLING_UID_ACCEPT), idCaptor.capture(), hostCaptor.capture(), - eq(DomainVerificationManager.STATE_SUCCESS) + eq(DomainVerificationState.STATE_SUCCESS) ) assertThat(idCaptor.allValues).containsExactly(TEST_UUID_ONE, TEST_UUID_TWO) 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 0d8f275be09c7..ee87a5f67dd48 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 @@ -20,28 +20,27 @@ import android.content.Context import android.content.Intent import android.content.pm.PackageManager import android.content.pm.PackageUserState -import android.content.pm.verify.domain.DomainVerificationManager import android.content.pm.parsing.component.ParsedActivity import android.content.pm.parsing.component.ParsedIntentInfo +import android.content.pm.verify.domain.DomainVerificationState import android.os.Build import android.os.Process import android.util.ArraySet import android.util.SparseArray import com.android.server.pm.PackageSetting +import com.android.server.pm.parsing.pkg.AndroidPackage import com.android.server.pm.verify.domain.DomainVerificationManagerInternal import com.android.server.pm.verify.domain.DomainVerificationService import com.android.server.pm.verify.domain.proxy.DomainVerificationProxy -import com.android.server.pm.parsing.pkg.AndroidPackage import com.android.server.testutils.mockThrowOnUnmocked import com.android.server.testutils.spyThrowOnUnmocked import com.android.server.testutils.whenever import org.junit.Test import org.junit.runner.RunWith import org.junit.runners.Parameterized -import org.mockito.Mockito +import org.mockito.Mockito.any import org.mockito.Mockito.anyInt import org.mockito.Mockito.anyLong -import org.mockito.Mockito.any import org.mockito.Mockito.anyString import org.mockito.Mockito.eq import org.mockito.Mockito.verify @@ -129,13 +128,13 @@ class DomainVerificationSettingsMutationTest { setDomainVerificationStatus( TEST_UUID, setOf("example.com"), - DomainVerificationManager.STATE_SUCCESS + DomainVerificationState.STATE_SUCCESS ) }, service("setStatusInternalPackageName") { setDomainVerificationStatusInternal( TEST_PKG, - DomainVerificationManager.STATE_SUCCESS, + DomainVerificationState.STATE_SUCCESS, ArraySet(setOf("example.com")) ) }, @@ -144,7 +143,7 @@ class DomainVerificationSettingsMutationTest { TEST_UID, TEST_UUID, setOf("example.com"), - DomainVerificationManager.STATE_SUCCESS + DomainVerificationState.STATE_SUCCESS ) }, service("setLinkHandlingAllowedUserId") { 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 0576125748fb1..0f46629c08f72 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 @@ -20,7 +20,7 @@ import android.content.Intent import android.content.pm.PackageManager import android.content.pm.parsing.component.ParsedActivity import android.content.pm.parsing.component.ParsedIntentInfo -import android.content.pm.verify.domain.DomainVerificationManager +import android.content.pm.verify.domain.DomainVerificationState import android.content.pm.verify.domain.DomainVerificationUserState import android.os.Build import android.os.PatternMatcher @@ -160,7 +160,7 @@ class DomainVerificationUserStateOverrideTest { // Verify 1 to give it a higher approval level service.setDomainVerificationStatus(UUID_ONE, setOf(DOMAIN_ONE), - DomainVerificationManager.STATE_SUCCESS) + DomainVerificationState.STATE_SUCCESS) assertThat(service.stateFor(PKG_ONE, DOMAIN_ONE)).isEqualTo(STATE_VERIFIED) assertThat(service.getOwnersForDomain(DOMAIN_ONE, USER_ID).map { it.packageName }) .containsExactly(PKG_ONE) From 173ef94284f7031ffd56726ecbd6a0a52fd1fa6c Mon Sep 17 00:00:00 2001 From: Winson Date: Wed, 10 Mar 2021 13:55:00 -0800 Subject: [PATCH 3/7] Expose error codes for domain verification setters Removes the InvalidDomainSetException and exposes the reasons as API error codes. Implementers can check for a non-0 error code to tell if the failure is meant to indicate a cancelled request. Bug: 180991102 Test: atest com.android.server.pm.test.verify.domain Test: atest DomainVerificationManagerApiTest Change-Id: I19b88fb582fc7f633497d036a0590350ec463f3e --- core/api/system-current.txt | 11 +- .../domain/DomainVerificationManager.java | 218 +++++------ .../domain/IDomainVerificationManager.aidl | 4 +- .../DomainVerificationManagerInternal.java | 6 +- .../domain/DomainVerificationManagerStub.java | 20 +- .../domain/DomainVerificationService.java | 130 +++--- .../proxy/DomainVerificationProxyV1.java | 20 +- .../unit/Android.bp | 5 +- .../domain/DomainVerificationEnforcerTest.kt | 43 +- .../domain/DomainVerificationJavaUtil.java | 48 +++ .../DomainVerificationManagerApiTest.kt | 370 ++++++++++++++++++ .../domain/DomainVerificationPackageTest.kt | 32 +- .../domain/DomainVerificationProxyTest.kt | 4 +- .../domain/DomainVerificationServiceUtil.kt | 31 ++ ...inVerificationUserSelectionOverrideTest.kt | 6 +- 15 files changed, 715 insertions(+), 233 deletions(-) create mode 100644 services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationJavaUtil.java create mode 100644 services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationManagerApiTest.kt create mode 100644 services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationServiceUtil.kt diff --git a/core/api/system-current.txt b/core/api/system-current.txt index e014716e71bd9..7d6ffb6290193 100644 --- a/core/api/system-current.txt +++ b/core/api/system-current.txt @@ -2848,9 +2848,16 @@ package android.content.pm.verify.domain { method @NonNull @RequiresPermission(android.Manifest.permission.UPDATE_DOMAIN_VERIFICATION_USER_SELECTION) public java.util.List getOwnersForDomain(@NonNull String); method @NonNull @RequiresPermission(android.Manifest.permission.DOMAIN_VERIFICATION_AGENT) public java.util.List queryValidVerificationPackageNames(); method @RequiresPermission(android.Manifest.permission.UPDATE_DOMAIN_VERIFICATION_USER_SELECTION) public void setDomainVerificationLinkHandlingAllowed(@NonNull String, boolean) throws android.content.pm.PackageManager.NameNotFoundException; - method @RequiresPermission(android.Manifest.permission.DOMAIN_VERIFICATION_AGENT) public void setDomainVerificationStatus(@NonNull java.util.UUID, @NonNull java.util.Set, int) throws android.content.pm.PackageManager.NameNotFoundException; - method @RequiresPermission(android.Manifest.permission.UPDATE_DOMAIN_VERIFICATION_USER_SELECTION) public void setDomainVerificationUserSelection(@NonNull java.util.UUID, @NonNull java.util.Set, boolean) throws android.content.pm.PackageManager.NameNotFoundException; + method @RequiresPermission(android.Manifest.permission.DOMAIN_VERIFICATION_AGENT) public int setDomainVerificationStatus(@NonNull java.util.UUID, @NonNull java.util.Set, int) throws android.content.pm.PackageManager.NameNotFoundException; + method @RequiresPermission(android.Manifest.permission.UPDATE_DOMAIN_VERIFICATION_USER_SELECTION) public int setDomainVerificationUserSelection(@NonNull java.util.UUID, @NonNull java.util.Set, boolean) throws android.content.pm.PackageManager.NameNotFoundException; + field public static final int ERROR_DOMAIN_SET_ID_INVALID = 1; // 0x1 + field public static final int ERROR_DOMAIN_SET_ID_NULL = 2; // 0x2 + field public static final int ERROR_DOMAIN_SET_NULL_OR_EMPTY = 3; // 0x3 + field public static final int ERROR_INVALID_STATE_CODE = 6; // 0x6 + field public static final int ERROR_UNABLE_TO_APPROVE = 5; // 0x5 + field public static final int ERROR_UNKNOWN_DOMAIN = 4; // 0x4 field public static final String EXTRA_VERIFICATION_REQUEST = "android.content.pm.verify.domain.extra.VERIFICATION_REQUEST"; + field public static final int STATUS_OK = 0; // 0x0 } public final class DomainVerificationRequest implements android.os.Parcelable { diff --git a/core/java/android/content/pm/verify/domain/DomainVerificationManager.java b/core/java/android/content/pm/verify/domain/DomainVerificationManager.java index c92f8cc525ecc..55e19f2727bfc 100644 --- a/core/java/android/content/pm/verify/domain/DomainVerificationManager.java +++ b/core/java/android/content/pm/verify/domain/DomainVerificationManager.java @@ -60,29 +60,97 @@ public final class DomainVerificationManager { "android.content.pm.verify.domain.extra.VERIFICATION_REQUEST"; /** + * Default return code for when a method has succeeded. + * * @hide */ - public static final int ERROR_INVALID_DOMAIN_SET = 1; + @SystemApi + public static final int STATUS_OK = 0; + /** + * The provided domain set ID was invalid, probably due to the package being updated between + * the initial request that provided the ID and the method call that used it. This usually + * means the work being processed by the verification agent is outdated and a new request + * should be scheduled, which should already be in progress as part of the + * {@link Intent#ACTION_DOMAINS_NEED_VERIFICATION} broadcast. + * * @hide */ - public static final int ERROR_NAME_NOT_FOUND = 2; + @SystemApi + public static final int ERROR_DOMAIN_SET_ID_INVALID = 1; + + /** + * The provided domain set ID was null. This is a developer error. + * + * @hide + */ + @SystemApi + public static final int ERROR_DOMAIN_SET_ID_NULL = 2; + + /** + * The provided set of domains was null or empty. This is a developer error. + * + * @hide + */ + @SystemApi + public static final int ERROR_DOMAIN_SET_NULL_OR_EMPTY = 3; + + /** + * The provided set of domains contains a domain not declared by the target package. This + * usually means the work being processed by the verification agent is outdated and a new + * request should be scheduled, which should already be in progress as part of the + * {@link Intent#ACTION_DOMAINS_NEED_VERIFICATION} broadcast. + * + * @hide + */ + @SystemApi + public static final int ERROR_UNKNOWN_DOMAIN = 4; + + /** + * The system was unable to select the domain for approval. This indicates another application + * has been granted a higher approval, usually through domain verification, and the target + * package is unable to override it. + * + * @hide + */ + @SystemApi + public static final int ERROR_UNABLE_TO_APPROVE = 5; + + /** + * The provided state code is incorrect. The domain verification agent is only allowed to + * assign {@link DomainVerificationInfo#STATE_SUCCESS} or error codes equal to or greater than + * {@link DomainVerificationInfo#STATE_FIRST_VERIFIER_DEFINED}. + * + * @hide + */ + @SystemApi + public static final int ERROR_INVALID_STATE_CODE = 6; + + /** + * Used to communicate through {@link ServiceSpecificException}. Should not be exposed as API. + * + * @hide + */ + public static final int INTERNAL_ERROR_NAME_NOT_FOUND = 1; /** * @hide */ @IntDef(prefix = {"ERROR_"}, value = { - ERROR_INVALID_DOMAIN_SET, - ERROR_NAME_NOT_FOUND, + ERROR_DOMAIN_SET_ID_INVALID, + ERROR_DOMAIN_SET_ID_NULL, + ERROR_DOMAIN_SET_NULL_OR_EMPTY, + ERROR_UNKNOWN_DOMAIN, + ERROR_UNABLE_TO_APPROVE, + ERROR_INVALID_STATE_CODE }) - private @interface Error { + public @interface Error { } private final Context mContext; private final IDomainVerificationManager mDomainVerificationManager; - /** * System service to access the domain verification APIs. *

@@ -164,27 +232,24 @@ public final class DomainVerificationManager { * @param domainSetId See {@link DomainVerificationInfo#getIdentifier()}. * @param domains List of host names to change the state of. * @param state See {@link DomainVerificationInfo#getHostToStateMap()}. - * @throws IllegalArgumentException If the ID is invalidated or the {@param domains} are - * invalid. This usually means the work being processed by the - * verification agent is outdated and a new request should be - * scheduled, if one has not already been done as part of the - * {@link Intent#ACTION_DOMAINS_NEED_VERIFICATION} broadcast. * @throws NameNotFoundException If the ID is known to be good, but the package is * unavailable. This may be because the package is installed on * a volume that is no longer mounted. This error is * unrecoverable until the package is available again, and * should not be re-tried except on a time scheduled basis. + * @return error code or {@link #STATUS_OK} if successful + * * @hide */ @SystemApi @RequiresPermission(android.Manifest.permission.DOMAIN_VERIFICATION_AGENT) - public void setDomainVerificationStatus(@NonNull UUID domainSetId, @NonNull Set domains, + public int setDomainVerificationStatus(@NonNull UUID domainSetId, @NonNull Set domains, int state) throws NameNotFoundException { try { - mDomainVerificationManager.setDomainVerificationStatus(domainSetId.toString(), + return mDomainVerificationManager.setDomainVerificationStatus(domainSetId.toString(), new DomainSet(domains), state); } catch (Exception e) { - Exception converted = rethrow(e, domainSetId); + Exception converted = rethrow(e, null); if (converted instanceof NameNotFoundException) { throw (NameNotFoundException) converted; } else if (converted instanceof RuntimeException) { @@ -213,7 +278,7 @@ public final class DomainVerificationManager { mDomainVerificationManager.setDomainVerificationLinkHandlingAllowed(packageName, allowed, mContext.getUserId()); } catch (Exception e) { - Exception converted = rethrow(e, packageName); + Exception converted = rethrow(e, null); if (converted instanceof NameNotFoundException) { throw (NameNotFoundException) converted; } else if (converted instanceof RuntimeException) { @@ -247,24 +312,24 @@ public final class DomainVerificationManager { * @param domainSetId See {@link DomainVerificationInfo#getIdentifier()}. * @param domains The domains to toggle the state of. * @param enabled Whether or not the app should automatically open the domains specified. - * @throws IllegalArgumentException If the ID is invalidated or the {@param domains} are - * invalid. * @throws NameNotFoundException If the ID is known to be good, but the package is * unavailable. This may be because the package is installed on * a volume that is no longer mounted. This error is * unrecoverable until the package is available again, and * should not be re-tried except on a time scheduled basis. + * @return error code or {@link #STATUS_OK} if successful + * * @hide */ @SystemApi @RequiresPermission(android.Manifest.permission.UPDATE_DOMAIN_VERIFICATION_USER_SELECTION) - public void setDomainVerificationUserSelection(@NonNull UUID domainSetId, + public int setDomainVerificationUserSelection(@NonNull UUID domainSetId, @NonNull Set domains, boolean enabled) throws NameNotFoundException { try { - mDomainVerificationManager.setDomainVerificationUserSelection(domainSetId.toString(), - new DomainSet(domains), enabled, mContext.getUserId()); + return mDomainVerificationManager.setDomainVerificationUserSelection( + domainSetId.toString(), new DomainSet(domains), enabled, mContext.getUserId()); } catch (Exception e) { - Exception converted = rethrow(e, domainSetId); + Exception converted = rethrow(e, null); if (converted instanceof NameNotFoundException) { throw (NameNotFoundException) converted; } else if (converted instanceof RuntimeException) { @@ -322,123 +387,22 @@ public final class DomainVerificationManager { } } - private Exception rethrow(Exception exception, @Nullable UUID domainSetId) { - return rethrow(exception, domainSetId, null); - } - private Exception rethrow(Exception exception, @Nullable String packageName) { - return rethrow(exception, null, packageName); - } - - private Exception rethrow(Exception exception, @Nullable UUID domainSetId, - @Nullable String packageName) { if (exception instanceof ServiceSpecificException) { - int packedErrorCode = ((ServiceSpecificException) exception).errorCode; + int serviceSpecificErrorCode = ((ServiceSpecificException) exception).errorCode; if (packageName == null) { packageName = exception.getMessage(); } - @Error int managerErrorCode = packedErrorCode & 0xFFFF; - switch (managerErrorCode) { - case ERROR_INVALID_DOMAIN_SET: - int errorSpecificCode = packedErrorCode >> 16; - return new IllegalArgumentException(InvalidDomainSetException.buildMessage( - domainSetId, packageName, errorSpecificCode)); - case ERROR_NAME_NOT_FOUND: - return new NameNotFoundException(packageName); - default: - return exception; + if (serviceSpecificErrorCode == INTERNAL_ERROR_NAME_NOT_FOUND) { + return new NameNotFoundException(packageName); } + + return exception; } else if (exception instanceof RemoteException) { return ((RemoteException) exception).rethrowFromSystemServer(); } else { return exception; } } - - /** - * Thrown if a {@link DomainVerificationInfo#getIdentifier()}} or an associated set of domains - * provided by the caller is no longer valid. This may be recoverable, and the caller should - * re-query the package name associated with the ID using - * {@link #getDomainVerificationInfo(String)} - * in order to check. If that also fails, then the package is no longer known to the device and - * thus all pending work for it should be dropped. - * - * @hide - */ - public static class InvalidDomainSetException extends IllegalArgumentException { - - public static final int REASON_ID_NULL = 1; - public static final int REASON_ID_INVALID = 2; - public static final int REASON_SET_NULL_OR_EMPTY = 3; - public static final int REASON_UNKNOWN_DOMAIN = 4; - public static final int REASON_UNABLE_TO_APPROVE = 5; - - /** - * @hide - */ - @IntDef({ - REASON_ID_NULL, - REASON_ID_INVALID, - REASON_SET_NULL_OR_EMPTY, - REASON_UNKNOWN_DOMAIN, - REASON_UNABLE_TO_APPROVE - }) - public @interface Reason { - } - - public static String buildMessage(@Nullable UUID domainSetId, @Nullable String packageName, - @Reason int reason) { - switch (reason) { - case REASON_ID_NULL: - return "Domain set ID cannot be null"; - case REASON_ID_INVALID: - return "Domain set ID " + domainSetId + " has been invalidated"; - case REASON_SET_NULL_OR_EMPTY: - return "Domain set cannot be null or empty"; - case REASON_UNKNOWN_DOMAIN: - return "Domain set contains value that was not declared by the target package " - + packageName; - case REASON_UNABLE_TO_APPROVE: - return "Domain set contains value that was owned by another package"; - default: - return "Unknown failure"; - } - } - - @Reason - private final int mReason; - - @Nullable - private final UUID mDomainSetId; - - @Nullable - private final String mPackageName; - - /** - * @hide - */ - public InvalidDomainSetException(@Nullable UUID domainSetId, @Nullable String packageName, - @Reason int reason) { - super(buildMessage(domainSetId, packageName, reason)); - mDomainSetId = domainSetId; - mPackageName = packageName; - mReason = reason; - } - - @Nullable - public UUID getDomainSetId() { - return mDomainSetId; - } - - @Nullable - public String getPackageName() { - return mPackageName; - } - - @Reason - public int getReason() { - return mReason; - } - } } diff --git a/core/java/android/content/pm/verify/domain/IDomainVerificationManager.aidl b/core/java/android/content/pm/verify/domain/IDomainVerificationManager.aidl index 332b925445813..53205f3ea4707 100644 --- a/core/java/android/content/pm/verify/domain/IDomainVerificationManager.aidl +++ b/core/java/android/content/pm/verify/domain/IDomainVerificationManager.aidl @@ -40,10 +40,10 @@ interface IDomainVerificationManager { @nullable List getOwnersForDomain(String domain, int userId); - void setDomainVerificationStatus(String domainSetId, in DomainSet domains, int state); + int setDomainVerificationStatus(String domainSetId, in DomainSet domains, int state); void setDomainVerificationLinkHandlingAllowed(String packageName, boolean allowed, int userId); - void setDomainVerificationUserSelection(String domainSetId, in DomainSet domains, + int setDomainVerificationUserSelection(String domainSetId, in DomainSet domains, boolean enabled, int userId); } 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 b4bcde7261736..73182732cdaf6 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 @@ -333,10 +333,10 @@ public interface DomainVerificationManagerInternal { @Nullable UUID getDomainVerificationInfoId(@NonNull String packageName); + @DomainVerificationManager.Error @RequiresPermission(android.Manifest.permission.DOMAIN_VERIFICATION_AGENT) - void setDomainVerificationStatusInternal(int callingUid, @NonNull UUID domainSetId, - @NonNull Set domains, int state) - throws IllegalArgumentException, NameNotFoundException; + int setDomainVerificationStatusInternal(int callingUid, @NonNull UUID domainSetId, + @NonNull Set domains, int state) throws NameNotFoundException; interface Connection extends DomainVerificationEnforcer.Callback { diff --git a/services/core/java/com/android/server/pm/verify/domain/DomainVerificationManagerStub.java b/services/core/java/com/android/server/pm/verify/domain/DomainVerificationManagerStub.java index a7a52e0cd10c4..2a17c6d4cec52 100644 --- a/services/core/java/com/android/server/pm/verify/domain/DomainVerificationManagerStub.java +++ b/services/core/java/com/android/server/pm/verify/domain/DomainVerificationManagerStub.java @@ -24,7 +24,6 @@ import android.content.pm.verify.domain.DomainOwner; import android.content.pm.verify.domain.DomainSet; import android.content.pm.verify.domain.DomainVerificationInfo; import android.content.pm.verify.domain.DomainVerificationManager; -import android.content.pm.verify.domain.DomainVerificationManager.InvalidDomainSetException; import android.content.pm.verify.domain.DomainVerificationUserState; import android.content.pm.verify.domain.IDomainVerificationManager; import android.os.ServiceSpecificException; @@ -61,11 +60,12 @@ public class DomainVerificationManagerStub extends IDomainVerificationManager.St } } + @DomainVerificationManager.Error @Override - public void setDomainVerificationStatus(String domainSetId, @NonNull DomainSet domainSet, + public int setDomainVerificationStatus(String domainSetId, @NonNull DomainSet domainSet, int state) { try { - mService.setDomainVerificationStatus(UUID.fromString(domainSetId), + return mService.setDomainVerificationStatus(UUID.fromString(domainSetId), domainSet.getDomains(), state); } catch (Exception e) { throw rethrow(e); @@ -82,11 +82,12 @@ public class DomainVerificationManagerStub extends IDomainVerificationManager.St } } + @DomainVerificationManager.Error @Override - public void setDomainVerificationUserSelection(String domainSetId, @NonNull DomainSet domainSet, + public int setDomainVerificationUserSelection(String domainSetId, @NonNull DomainSet domainSet, boolean enabled, @UserIdInt int userId) { try { - mService.setDomainVerificationUserSelection(UUID.fromString(domainSetId), + return mService.setDomainVerificationUserSelection(UUID.fromString(domainSetId), domainSet.getDomains(), enabled, userId); } catch (Exception e) { throw rethrow(e); @@ -116,14 +117,9 @@ public class DomainVerificationManagerStub extends IDomainVerificationManager.St } private RuntimeException rethrow(Exception exception) throws RuntimeException { - if (exception instanceof InvalidDomainSetException) { - int packedErrorCode = DomainVerificationManager.ERROR_INVALID_DOMAIN_SET; - packedErrorCode |= ((InvalidDomainSetException) exception).getReason() << 16; - return new ServiceSpecificException(packedErrorCode, - ((InvalidDomainSetException) exception).getPackageName()); - } else if (exception instanceof NameNotFoundException) { + if (exception instanceof NameNotFoundException) { return new ServiceSpecificException( - DomainVerificationManager.ERROR_NAME_NOT_FOUND); + DomainVerificationManager.INTERNAL_ERROR_NAME_NOT_FOUND); } else if (exception instanceof RuntimeException) { return (RuntimeException) exception; } else { 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 4c585e91f5a44..6896a42376f4f 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 @@ -19,6 +19,7 @@ package com.android.server.pm.verify.domain; import static java.util.Collections.emptyList; import static java.util.Collections.emptySet; +import android.annotation.CheckResult; import android.annotation.NonNull; import android.annotation.Nullable; import android.annotation.UserIdInt; @@ -33,7 +34,7 @@ import android.content.pm.ResolveInfo; import android.content.pm.parsing.component.ParsedActivity; import android.content.pm.verify.domain.DomainOwner; import android.content.pm.verify.domain.DomainVerificationInfo; -import android.content.pm.verify.domain.DomainVerificationManager.InvalidDomainSetException; +import android.content.pm.verify.domain.DomainVerificationManager; import android.content.pm.verify.domain.DomainVerificationState; import android.content.pm.verify.domain.DomainVerificationUserState; import android.content.pm.verify.domain.IDomainVerificationManager; @@ -242,16 +243,16 @@ public class DomainVerificationService extends SystemService throws NameNotFoundException { mEnforcer.assertApprovedQuerent(mConnection.getCallingUid(), mProxy); synchronized (mLock) { - DomainVerificationPkgState pkgState = mAttachedPkgStates.get(packageName); - if (pkgState == null) { - return null; - } - AndroidPackage pkg = mConnection.getPackageLocked(packageName); if (pkg == null) { throw DomainVerificationUtils.throwPackageUnavailable(packageName); } + DomainVerificationPkgState pkgState = mAttachedPkgStates.get(packageName); + if (pkgState == null) { + return null; + } + ArrayMap hostToStateMap = new ArrayMap<>(pkgState.getStateMap()); // TODO(b/159952358): Should the domain list be cached? @@ -278,30 +279,35 @@ public class DomainVerificationService extends SystemService } } - public void setDomainVerificationStatus(@NonNull UUID domainSetId, @NonNull Set domains, - int state) throws InvalidDomainSetException, NameNotFoundException { + @DomainVerificationManager.Error + public int setDomainVerificationStatus(@NonNull UUID domainSetId, @NonNull Set domains, + int state) throws NameNotFoundException { if (state < DomainVerificationState.STATE_FIRST_VERIFIER_DEFINED) { if (state != DomainVerificationState.STATE_SUCCESS) { - throw new IllegalArgumentException( - "Verifier can only set STATE_SUCCESS or codes greater than or equal to " - + "STATE_FIRST_VERIFIER_DEFINED"); + return DomainVerificationManager.ERROR_INVALID_STATE_CODE; } } - setDomainVerificationStatusInternal(mConnection.getCallingUid(), domainSetId, domains, - state); + return setDomainVerificationStatusInternal(mConnection.getCallingUid(), domainSetId, + domains, state); } + @DomainVerificationManager.Error @Override - public void setDomainVerificationStatusInternal(int callingUid, @NonNull UUID domainSetId, + public int setDomainVerificationStatusInternal(int callingUid, @NonNull UUID domainSetId, @NonNull Set domains, int state) - throws InvalidDomainSetException, NameNotFoundException { + throws NameNotFoundException { mEnforcer.assertApprovedVerifier(callingUid, mProxy); synchronized (mLock) { List verifiedDomains = new ArrayList<>(); - DomainVerificationPkgState pkgState = getAndValidateAttachedLocked(domainSetId, domains, + GetAttachedResult result = getAndValidateAttachedLocked(domainSetId, domains, true /* forAutoVerify */, callingUid, null /* userId */); + if (result.isError()) { + return result.getErrorCode(); + } + + DomainVerificationPkgState pkgState = result.getPkgState(); ArrayMap stateMap = pkgState.getStateMap(); for (String domain : domains) { Integer previousState = stateMap.get(domain); @@ -324,6 +330,7 @@ public class DomainVerificationService extends SystemService } mConnection.scheduleWriteSettings(); + return DomainVerificationManager.STATUS_OK; } @Override @@ -474,9 +481,9 @@ public class DomainVerificationService extends SystemService mConnection.scheduleWriteSettings(); } - public void setDomainVerificationUserSelection(@NonNull UUID domainSetId, + public int setDomainVerificationUserSelection(@NonNull UUID domainSetId, @NonNull Set domains, boolean enabled, @UserIdInt int userId) - throws InvalidDomainSetException, NameNotFoundException { + throws NameNotFoundException { synchronized (mLock) { final int callingUid = mConnection.getCallingUid(); // Pass null for package name here and do the app visibility enforcement inside @@ -484,14 +491,17 @@ public class DomainVerificationService extends SystemService // ID reason if the target app is invisible if (!mEnforcer.assertApprovedUserSelector(callingUid, mConnection.getCallingUserId(), null /* packageName */, userId)) { - throw new InvalidDomainSetException(domainSetId, null, - InvalidDomainSetException.REASON_ID_INVALID); + return DomainVerificationManager.ERROR_DOMAIN_SET_ID_INVALID; } - DomainVerificationPkgState pkgState = getAndValidateAttachedLocked(domainSetId, domains, + GetAttachedResult result = getAndValidateAttachedLocked(domainSetId, domains, false /* forAutoVerify */, callingUid, userId); - DomainVerificationInternalUserState userState = - pkgState.getOrCreateUserState(userId); + if (result.isError()) { + return result.getErrorCode(); + } + + DomainVerificationPkgState pkgState = result.getPkgState(); + DomainVerificationInternalUserState userState = pkgState.getOrCreateUserState(userId); // Disable other packages if approving this one. Note that this check is only done for // enabling. This allows an escape hatch in case multiple packages somehow get selected. @@ -509,8 +519,7 @@ public class DomainVerificationService extends SystemService userId, APPROVAL_LEVEL_NONE + 1, mConnection::getPackageSettingLocked); int highestApproval = packagesToLevel.second; if (highestApproval > APPROVAL_LEVEL_SELECTION) { - throw new InvalidDomainSetException(domainSetId, null, - InvalidDomainSetException.REASON_UNABLE_TO_APPROVE); + return DomainVerificationManager.ERROR_UNABLE_TO_APPROVE; } domainToApprovedPackages.put(domain, packagesToLevel.first); @@ -550,6 +559,7 @@ public class DomainVerificationService extends SystemService } mConnection.scheduleWriteSettings(); + return DomainVerificationManager.STATUS_OK; } @Override @@ -642,16 +652,16 @@ public class DomainVerificationService extends SystemService throw DomainVerificationUtils.throwPackageUnavailable(packageName); } synchronized (mLock) { - DomainVerificationPkgState pkgState = mAttachedPkgStates.get(packageName); - if (pkgState == null) { - return null; - } - AndroidPackage pkg = mConnection.getPackageLocked(packageName); if (pkg == null) { throw DomainVerificationUtils.throwPackageUnavailable(packageName); } + DomainVerificationPkgState pkgState = mAttachedPkgStates.get(packageName); + if (pkgState == null) { + return null; + } + ArraySet webDomains = mCollector.collectAllWebDomains(pkg); int webDomainsSize = webDomains.size(); @@ -1096,28 +1106,25 @@ public class DomainVerificationService extends SystemService * @param userIdForFilter which user to filter app access to, or null if the caller has already * validated package visibility */ + @CheckResult @GuardedBy("mLock") - private DomainVerificationPkgState getAndValidateAttachedLocked(@NonNull UUID domainSetId, + private GetAttachedResult getAndValidateAttachedLocked(@NonNull UUID domainSetId, @NonNull Set domains, boolean forAutoVerify, int callingUid, - @Nullable Integer userIdForFilter) - throws InvalidDomainSetException, NameNotFoundException { + @Nullable Integer userIdForFilter) throws NameNotFoundException { if (domainSetId == null) { - throw new InvalidDomainSetException(null, null, - InvalidDomainSetException.REASON_ID_NULL); + return GetAttachedResult.error(DomainVerificationManager.ERROR_DOMAIN_SET_ID_NULL); } DomainVerificationPkgState pkgState = mAttachedPkgStates.get(domainSetId); if (pkgState == null) { - throw new InvalidDomainSetException(domainSetId, null, - InvalidDomainSetException.REASON_ID_INVALID); + return GetAttachedResult.error(DomainVerificationManager.ERROR_DOMAIN_SET_ID_INVALID); } String pkgName = pkgState.getPackageName(); if (userIdForFilter != null && mConnection.filterAppAccess(pkgName, callingUid, userIdForFilter)) { - throw new InvalidDomainSetException(domainSetId, null, - InvalidDomainSetException.REASON_ID_INVALID); + return GetAttachedResult.error(DomainVerificationManager.ERROR_DOMAIN_SET_ID_INVALID); } PackageSetting pkgSetting = mConnection.getPackageSettingLocked(pkgName); @@ -1126,8 +1133,8 @@ public class DomainVerificationService extends SystemService } if (CollectionUtils.isEmpty(domains)) { - throw new InvalidDomainSetException(domainSetId, pkgState.getPackageName(), - InvalidDomainSetException.REASON_SET_NULL_OR_EMPTY); + return GetAttachedResult.error( + DomainVerificationManager.ERROR_DOMAIN_SET_NULL_OR_EMPTY); } AndroidPackage pkg = pkgSetting.getPkg(); ArraySet declaredDomains = forAutoVerify @@ -1135,11 +1142,10 @@ public class DomainVerificationService extends SystemService : mCollector.collectAllWebDomains(pkg); if (domains.retainAll(declaredDomains)) { - throw new InvalidDomainSetException(domainSetId, pkgState.getPackageName(), - InvalidDomainSetException.REASON_UNKNOWN_DOMAIN); + return GetAttachedResult.error(DomainVerificationManager.ERROR_UNKNOWN_DOMAIN); } - return pkgState; + return GetAttachedResult.success(pkgState); } @Override @@ -1671,4 +1677,40 @@ public class DomainVerificationService extends SystemService Slog.d(TAG + "Approval", packageName + " was " + approvalString + " for " + debugObject + " for user " + userId + ": " + reason); } + + private static class GetAttachedResult { + + @Nullable + private DomainVerificationPkgState mPkgState; + + private int mErrorCode; + + GetAttachedResult(@Nullable DomainVerificationPkgState pkgState, int errorCode) { + mPkgState = pkgState; + mErrorCode = errorCode; + } + + @NonNull + static GetAttachedResult error(@DomainVerificationManager.Error int errorCode) { + return new GetAttachedResult(null, errorCode); + } + + @NonNull + static GetAttachedResult success(@NonNull DomainVerificationPkgState pkgState) { + return new GetAttachedResult(pkgState, DomainVerificationManager.STATUS_OK); + } + + @NonNull + DomainVerificationPkgState getPkgState() { + return mPkgState; + } + + boolean isError() { + return mErrorCode != DomainVerificationManager.STATUS_OK; + } + + public int getErrorCode() { + return mErrorCode; + } + } } diff --git a/services/core/java/com/android/server/pm/verify/domain/proxy/DomainVerificationProxyV1.java b/services/core/java/com/android/server/pm/verify/domain/proxy/DomainVerificationProxyV1.java index 18042af139a3b..fa36683e4aff7 100644 --- a/services/core/java/com/android/server/pm/verify/domain/proxy/DomainVerificationProxyV1.java +++ b/services/core/java/com/android/server/pm/verify/domain/proxy/DomainVerificationProxyV1.java @@ -207,20 +207,24 @@ public class DomainVerificationProxyV1 implements DomainVerificationProxy { int callingUid = response.callingUid; if (!successfulDomains.isEmpty()) { try { - mManager.setDomainVerificationStatusInternal(callingUid, domainSetId, - successfulDomains, DomainVerificationState.STATE_SUCCESS); - } catch (DomainVerificationManager.InvalidDomainSetException - | PackageManager.NameNotFoundException e) { + if (mManager.setDomainVerificationStatusInternal(callingUid, domainSetId, + successfulDomains, DomainVerificationState.STATE_SUCCESS) + != DomainVerificationManager.STATUS_OK) { + Slog.e(TAG, "Failure reporting successful domains for " + packageName); + } + } catch (Exception e) { Slog.e(TAG, "Failure reporting successful domains for " + packageName, e); } } if (!failedDomains.isEmpty()) { try { - mManager.setDomainVerificationStatusInternal(callingUid, domainSetId, - failedDomains, DomainVerificationState.STATE_LEGACY_FAILURE); - } catch (DomainVerificationManager.InvalidDomainSetException - | PackageManager.NameNotFoundException e) { + if (mManager.setDomainVerificationStatusInternal(callingUid, domainSetId, + failedDomains, DomainVerificationState.STATE_LEGACY_FAILURE) + != DomainVerificationManager.STATUS_OK) { + Slog.e(TAG, "Failure reporting failed domains for " + packageName); + } + } catch (Exception e) { Slog.e(TAG, "Failure reporting failed domains for " + packageName, e); } } diff --git a/services/tests/PackageManagerServiceTests/unit/Android.bp b/services/tests/PackageManagerServiceTests/unit/Android.bp index eeea6618c08fd..988c02bfb3db5 100644 --- a/services/tests/PackageManagerServiceTests/unit/Android.bp +++ b/services/tests/PackageManagerServiceTests/unit/Android.bp @@ -23,7 +23,10 @@ package { android_test { name: "PackageManagerServiceUnitTests", - srcs: ["src/**/*.kt"], + srcs: [ + "src/**/*.java", + "src/**/*.kt", + ], static_libs: [ "androidx.test.rules", "androidx.test.runner", 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 c9df5669ced27..6ac9907698312 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 @@ -643,12 +643,30 @@ class DomainVerificationEnforcerTest { } val target = params.construct(context) + // Legacy code can return PackageManager.INTENT_FILTER_DOMAIN_VERIFICATION_STATUS_UNDEFINED + // as an error code. This is distinct from the class level assertFails as unfortunately + // the same number, 0, is used in opposite contexts, where it does represent a failure + // for this legacy case, but not for the modern APIs. + fun assertFailsLegacy(block: () -> Any?) { + try { + val value = block() + if ((value as? Int) + != PackageManager.INTENT_FILTER_DOMAIN_VERIFICATION_STATUS_UNDEFINED + ) { + throw AssertionError("Expected call to return false, was $value") + } + } catch (e: SecurityException) { + } catch (e: PackageManager.NameNotFoundException) { + // Any of these 2 exceptions are considered failures, which is expected + } + } + fun runTestCases(callingUserId: Int, targetUserId: Int, throws: Boolean) { // Legacy makes no distinction by UID val allUids = INTERNAL_UIDS + VERIFIER_UID + NON_VERIFIER_UID if (throws) { allUids.forEach { - assertFails { + assertFailsLegacy { runMethod(target, it, visible = true, callingUserId, targetUserId) } } @@ -660,7 +678,7 @@ class DomainVerificationEnforcerTest { // Legacy doesn't use QUERY_ALL, so the invisible package should always fail allUids.forEach { - assertFails { + assertFailsLegacy { runMethod(target, it, visible = false, callingUserId, targetUserId) } } @@ -786,19 +804,20 @@ class DomainVerificationEnforcerTest { private fun assertFails(block: () -> Any?) { try { val value = block() - // Some methods return false rather than throwing, so check that as well - if ((value as? Boolean) != false) { - // Can also return default value if it's a legacy call - if ((value as? Int) - != PackageManager.INTENT_FILTER_DOMAIN_VERIFICATION_STATUS_UNDEFINED - ) { - throw AssertionError("Expected call to return false, was $value") - } + // Some methods return false or an error rather than throwing, so check that as well + val valueAsBoolean = value as? Boolean + if (valueAsBoolean == false) { + // Expected failure, do not throw + return + } + + val valueAsInt = value as? Int + if (valueAsInt != null && valueAsInt == DomainVerificationManager.STATUS_OK) { + throw AssertionError("Expected call to return false, was $value") } } catch (e: SecurityException) { } catch (e: PackageManager.NameNotFoundException) { - } catch (e: DomainVerificationManager.InvalidDomainSetException) { - // Any of these 3 exceptions are considered failures, which is expected + // Any of these 2 exceptions are considered failures, which is expected } } diff --git a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationJavaUtil.java b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationJavaUtil.java new file mode 100644 index 0000000000000..8ae4c5ae96a35 --- /dev/null +++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationJavaUtil.java @@ -0,0 +1,48 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.pm.test.verify.domain; + +import android.annotation.NonNull; +import android.annotation.Nullable; +import android.annotation.UserIdInt; +import android.content.pm.PackageManager; + +import com.android.server.pm.verify.domain.DomainVerificationService; + +import java.util.Set; +import java.util.UUID; + +/** + * Proxies Kotlin calls to the Java layer such that null values can be passed for {@link NonNull} + * marked parameters, as Kotlin disallows this at the compiler leveling, preventing the null error + * codes from being tested. + */ +class DomainVerificationJavaUtil { + + static int setStatusForceNullable(@NonNull DomainVerificationService service, + @Nullable UUID domainSetId, @Nullable Set domains, int state) + throws PackageManager.NameNotFoundException { + return service.setDomainVerificationStatus(domainSetId, domains, state); + } + + static int setUserSelectionForceNullable(@NonNull DomainVerificationService service, + @Nullable UUID domainSetId, @Nullable Set domains, boolean enabled, + @UserIdInt int userId) + throws PackageManager.NameNotFoundException { + return service.setDomainVerificationUserSelection(domainSetId, domains, enabled, userId); + } +} 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 new file mode 100644 index 0000000000000..ec4775260f33e --- /dev/null +++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationManagerApiTest.kt @@ -0,0 +1,370 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.pm.test.verify.domain + +import android.content.Intent +import android.content.pm.PackageManager +import android.content.pm.parsing.component.ParsedActivity +import android.content.pm.parsing.component.ParsedIntentInfo +import android.content.pm.verify.domain.DomainVerificationInfo +import android.content.pm.verify.domain.DomainVerificationManager +import android.content.pm.verify.domain.DomainVerificationUserState +import android.os.Build +import android.os.PatternMatcher +import android.os.Process +import android.util.ArraySet +import com.android.server.pm.PackageSetting +import com.android.server.pm.parsing.pkg.AndroidPackage +import com.android.server.pm.verify.domain.DomainVerificationService +import com.android.server.testutils.mockThrowOnUnmocked +import com.android.server.testutils.whenever +import com.google.common.truth.Truth.assertThat +import org.junit.Test +import org.mockito.ArgumentMatchers.any +import org.mockito.ArgumentMatchers.anyInt +import org.mockito.ArgumentMatchers.anyLong +import org.mockito.ArgumentMatchers.anyString +import java.util.UUID +import kotlin.test.assertFailsWith + +class DomainVerificationManagerApiTest { + + companion object { + private const val PKG_ONE = "com.test.one" + private const val PKG_TWO = "com.test.two" + private const val PKG_THREE = "com.test.three" + + private val UUID_ONE = UUID.fromString("1b041c96-8d37-4932-a858-561bfac5947c") + private val UUID_TWO = UUID.fromString("a3389c16-7f9f-4e86-85e3-500d1249c74c") + private val UUID_THREE = UUID.fromString("0b3260ed-07c4-4b45-840b-237f8fb8b433") + private val UUID_INVALID = UUID.fromString("ad33babc-490b-4965-9d78-7e91248b00f") + + private val DOMAIN_BASE = DomainVerificationManagerApiTest::class.java.packageName + private val DOMAIN_1 = "one.$DOMAIN_BASE" + private val DOMAIN_2 = "two.$DOMAIN_BASE" + private val DOMAIN_3 = "three.$DOMAIN_BASE" + private val DOMAIN_4 = "four.$DOMAIN_BASE" + } + + @Test + fun queryValidVerificationPackageNames() { + val pkgWithDomains = mockPkgSetting(PKG_ONE, UUID_ONE, listOf(DOMAIN_1, DOMAIN_2)) + val pkgWithoutDomains = mockPkgSetting(PKG_TWO, UUID_TWO, emptyList()) + + val service = makeService(pkgWithDomains, pkgWithoutDomains).apply { + addPackages(pkgWithDomains, pkgWithoutDomains) + } + + assertThat(service.queryValidVerificationPackageNames()) + .containsExactly(pkgWithDomains.getName()) + } + + @Test + fun getDomainVerificationInfoId() { + val pkgWithDomains = mockPkgSetting(PKG_ONE, UUID_ONE, listOf(DOMAIN_1, DOMAIN_2)) + val pkgWithoutDomains = mockPkgSetting(PKG_TWO, UUID_TWO, emptyList()) + + val service = makeService(pkgWithDomains, pkgWithoutDomains).apply { + addPackages(pkgWithDomains, pkgWithoutDomains) + } + + assertThat(service.getDomainVerificationInfoId(PKG_ONE)).isEqualTo(UUID_ONE) + assertThat(service.getDomainVerificationInfoId(PKG_TWO)).isEqualTo(UUID_TWO) + + assertThat(service.getDomainVerificationInfoId("invalid.pkg.name")).isEqualTo(null) + } + + @Test + fun getDomainVerificationInfo() { + val pkgWithDomains = mockPkgSetting(PKG_ONE, UUID_ONE, listOf(DOMAIN_1, DOMAIN_2)) + val pkgWithoutDomains = mockPkgSetting(PKG_TWO, UUID_TWO, emptyList()) + + val service = makeService(pkgWithDomains, pkgWithoutDomains).apply { + addPackages(pkgWithDomains, pkgWithoutDomains) + } + + val infoOne = service.getDomainVerificationInfo(pkgWithDomains.getName()) + assertThat(infoOne).isNotNull() + assertThat(infoOne!!.identifier).isEqualTo(pkgWithDomains.domainSetId) + assertThat(infoOne.packageName).isEqualTo(pkgWithDomains.getName()) + assertThat(infoOne.hostToStateMap).containsExactlyEntriesIn(mapOf( + DOMAIN_1 to DomainVerificationInfo.STATE_NO_RESPONSE, + DOMAIN_2 to DomainVerificationInfo.STATE_NO_RESPONSE, + )) + + assertThat(service.getDomainVerificationInfo(pkgWithoutDomains.getName())).isNull() + + assertFailsWith(PackageManager.NameNotFoundException::class) { + service.getDomainVerificationInfo("invalid.pkg.name") + } + } + + @Test + fun setStatus() { + val pkg1 = mockPkgSetting(PKG_ONE, UUID_ONE, listOf(DOMAIN_1, DOMAIN_2)) + val pkg2 = mockPkgSetting(PKG_TWO, UUID_TWO, listOf(DOMAIN_3, DOMAIN_4)) + + val map = mutableMapOf(pkg1.getName() to pkg1, pkg2.getName() to pkg2) + val service = makeService(map::get).apply { addPackages(pkg1, pkg2) } + + assertThat(service.setStatus(UUID_ONE, setOf(DOMAIN_2), 1100)) + .isEqualTo(DomainVerificationManager.STATUS_OK) + + assertThat(service.setStatus(UUID_INVALID, setOf(DOMAIN_1), 1100)) + .isEqualTo(DomainVerificationManager.ERROR_DOMAIN_SET_ID_INVALID) + + assertThat(DomainVerificationJavaUtil.setStatusForceNullable(service, null, + setOf(DOMAIN_1), 1100)) + .isEqualTo(DomainVerificationManager.ERROR_DOMAIN_SET_ID_NULL) + + assertThat(DomainVerificationJavaUtil.setStatusForceNullable(service, UUID_ONE, null, + 1100)).isEqualTo(DomainVerificationManager.ERROR_DOMAIN_SET_NULL_OR_EMPTY) + + assertThat(service.setStatus(UUID_ONE, emptySet(), 1100)) + .isEqualTo(DomainVerificationManager.ERROR_DOMAIN_SET_NULL_OR_EMPTY) + + assertThat(service.setStatus(UUID_ONE, setOf(DOMAIN_3), 1100)) + .isEqualTo(DomainVerificationManager.ERROR_UNKNOWN_DOMAIN) + + assertThat(service.setStatus(UUID_ONE, setOf(DOMAIN_1, DOMAIN_2, DOMAIN_3), 1100)) + .isEqualTo(DomainVerificationManager.ERROR_UNKNOWN_DOMAIN) + + assertThat(service.setStatus(UUID_ONE, setOf(DOMAIN_1), 15)) + .isEqualTo(DomainVerificationManager.ERROR_INVALID_STATE_CODE) + + map.clear() + assertFailsWith(PackageManager.NameNotFoundException::class){ + service.setStatus(UUID_ONE, setOf(DOMAIN_1), 1100) + } + } + + @Test + fun setDomainVerificationLinkHandlingAllowed() { + val pkg1 = mockPkgSetting(PKG_ONE, UUID_ONE, listOf(DOMAIN_1, DOMAIN_2)) + val pkg2 = mockPkgSetting(PKG_TWO, UUID_TWO, listOf(DOMAIN_3, DOMAIN_4)) + + val map = mutableMapOf(pkg1.getName() to pkg1, pkg2.getName() to pkg2) + val service = makeService(map::get).apply { addPackages(pkg1, pkg2) } + + service.setDomainVerificationLinkHandlingAllowed(PKG_ONE, false, 0); + + // Should edit same package, same user + assertThat(service.getDomainVerificationUserState(PKG_ONE, 0) + ?.isLinkHandlingAllowed).isEqualTo(false) + + // Shouldn't edit different user + assertThat(service.getDomainVerificationUserState(PKG_ONE, 1) + ?.isLinkHandlingAllowed).isEqualTo(true) + + // Shouldn't edit different package + assertThat(service.getDomainVerificationUserState(PKG_TWO, 0) + ?.isLinkHandlingAllowed).isEqualTo(true) + + assertFailsWith(PackageManager.NameNotFoundException::class){ + service.setDomainVerificationLinkHandlingAllowed("invalid.pkg.name", false, 0); + } + } + + @Test + fun setUserSelection() { + val pkg1 = mockPkgSetting(PKG_ONE, UUID_ONE, listOf(DOMAIN_1, DOMAIN_2)) + val pkg2 = mockPkgSetting(PKG_TWO, UUID_TWO, listOf(DOMAIN_3, DOMAIN_4)) + val pkg3 = mockPkgSetting(PKG_THREE, UUID_THREE, listOf(DOMAIN_1, DOMAIN_2)) + + val map = mutableMapOf( + pkg1.getName() to pkg1, + pkg2.getName() to pkg2, + pkg3.getName() to pkg3 + ) + val service = makeService(map::get).apply { addPackages(pkg1, pkg2, pkg3) } + + assertThat(service.setUserSelection(UUID_ONE, setOf(DOMAIN_2), true, 0)) + .isEqualTo(DomainVerificationManager.STATUS_OK) + + assertThat(service.setUserSelection(UUID_INVALID, setOf(DOMAIN_1), true, 0)) + .isEqualTo(DomainVerificationManager.ERROR_DOMAIN_SET_ID_INVALID) + + assertThat(DomainVerificationJavaUtil.setUserSelectionForceNullable(service, null, + setOf(DOMAIN_1), true, 0)) + .isEqualTo(DomainVerificationManager.ERROR_DOMAIN_SET_ID_NULL) + + assertThat(DomainVerificationJavaUtil.setUserSelectionForceNullable(service, UUID_ONE, null, + true, 0)).isEqualTo(DomainVerificationManager.ERROR_DOMAIN_SET_NULL_OR_EMPTY) + + assertThat(service.setUserSelection(UUID_ONE, emptySet(), true, 0)) + .isEqualTo(DomainVerificationManager.ERROR_DOMAIN_SET_NULL_OR_EMPTY) + + assertThat(service.setUserSelection(UUID_ONE, setOf(DOMAIN_3), true, 0)) + .isEqualTo(DomainVerificationManager.ERROR_UNKNOWN_DOMAIN) + + assertThat(service.setUserSelection(UUID_ONE, setOf(DOMAIN_1, DOMAIN_2, DOMAIN_3), true, 0)) + .isEqualTo(DomainVerificationManager.ERROR_UNKNOWN_DOMAIN) + + service.setStatus(UUID_ONE, setOf(DOMAIN_2), DomainVerificationInfo.STATE_SUCCESS) + + assertThat(service.setUserSelection(UUID_THREE, setOf(DOMAIN_2), true, 0)) + .isEqualTo(DomainVerificationManager.ERROR_UNABLE_TO_APPROVE) + + map.clear() + assertFailsWith(PackageManager.NameNotFoundException::class){ + service.setUserSelection(UUID_ONE, setOf(DOMAIN_1), true, 0) + } + } + + @Test + fun getDomainVerificationUserState() { + val pkgWithDomains = mockPkgSetting(PKG_ONE, UUID_ONE, listOf(DOMAIN_1, DOMAIN_2)) + val pkgWithoutDomains = mockPkgSetting(PKG_TWO, UUID_TWO, emptyList()) + + val service = makeService(pkgWithDomains, pkgWithoutDomains).apply { + addPackages(pkgWithDomains, pkgWithoutDomains) + } + + val infoOne = service.getDomainVerificationUserState(pkgWithDomains.getName(), 0) + assertThat(infoOne).isNotNull() + assertThat(infoOne!!.identifier).isEqualTo(pkgWithDomains.domainSetId) + assertThat(infoOne.packageName).isEqualTo(pkgWithDomains.getName()) + assertThat(infoOne.isLinkHandlingAllowed).isTrue() + assertThat(infoOne.hostToStateMap).containsExactlyEntriesIn(mapOf( + DOMAIN_1 to DomainVerificationUserState.DOMAIN_STATE_NONE, + DOMAIN_2 to DomainVerificationUserState.DOMAIN_STATE_NONE, + )) + + val infoTwo = service.getDomainVerificationUserState(pkgWithoutDomains.getName(), 0) + assertThat(infoTwo).isNotNull() + assertThat(infoTwo!!.identifier).isEqualTo(pkgWithoutDomains.domainSetId) + assertThat(infoTwo.packageName).isEqualTo(pkgWithoutDomains.getName()) + assertThat(infoOne.isLinkHandlingAllowed).isTrue() + assertThat(infoTwo.hostToStateMap).isEmpty() + + assertFailsWith(PackageManager.NameNotFoundException::class) { + service.getDomainVerificationUserState("invalid.pkg.name", 0) + } + } + + @Test + fun getOwnersForDomain() { + val pkg1 = mockPkgSetting(PKG_ONE, UUID_ONE, listOf(DOMAIN_1, DOMAIN_2)) + val pkg2 = mockPkgSetting(PKG_TWO, UUID_TWO, listOf(DOMAIN_1, DOMAIN_2)) + + val service = makeService(pkg1, pkg2).apply { + addPackages(pkg1, pkg2) + } + + 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) + + service.setUserSelection(pkg1.domainSetId, setOf(DOMAIN_2), true, 0) + + service.getOwnersForDomain(DOMAIN_1, 0).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.getOwnersForDomain(DOMAIN_2, 0).let { + assertThat(it).hasSize(1) + assertThat(it.single().packageName).isEqualTo(pkg1.getName()) + assertThat(it.single().isOverrideable).isEqualTo(true) + } + assertThat(service.getOwnersForDomain(DOMAIN_2, 1)).isEmpty() + + 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() + } + + private fun makeService(vararg pkgSettings: PackageSetting) = + makeService { pkgName -> pkgSettings.find { pkgName == it.getName() } } + + private fun makeService(pkgSettingFunction: (String) -> PackageSetting? = { null }) = + DomainVerificationService(mockThrowOnUnmocked { + // Assume the test has every permission necessary + whenever(enforcePermission(anyString(), anyInt(), anyInt(), anyString())) + whenever(checkPermission(anyString(), anyInt(), anyInt())) { + PackageManager.PERMISSION_GRANTED + } + }, mockThrowOnUnmocked { + whenever(linkedApps) { ArraySet() } + }, mockThrowOnUnmocked { + whenever(isChangeEnabled(anyLong(), any())) { true } + }).apply { + setConnection(mockThrowOnUnmocked { + whenever(filterAppAccess(anyString(), anyInt(), anyInt())) { false } + whenever(scheduleWriteSettings()) + + // Need to provide an internal UID so some permission checks are ignored + whenever(callingUid) { Process.ROOT_UID } + whenever(callingUserId) { 0 } + + whenever(getPackageSettingLocked(anyString())) { + pkgSettingFunction(arguments[0] as String) + } + whenever(getPackageLocked(anyString())) { + pkgSettingFunction(arguments[0] as String)?.getPkg() + } + }) + } + + private fun mockPkgSetting(pkgName: String, domainSetId: UUID, domains: List = listOf( + DOMAIN_1, DOMAIN_2 + )) = mockThrowOnUnmocked { + val pkg = mockThrowOnUnmocked { + whenever(packageName) { pkgName } + whenever(targetSdkVersion) { Build.VERSION_CODES.S } + + val activityList = listOf( + ParsedActivity().apply { + domains.forEach { + addIntent( + ParsedIntentInfo().apply { + autoVerify = true + addAction(Intent.ACTION_VIEW) + addCategory(Intent.CATEGORY_BROWSABLE) + addCategory(Intent.CATEGORY_DEFAULT) + addDataScheme("http") + addDataScheme("https") + addDataPath("/sub", PatternMatcher.PATTERN_LITERAL) + addDataAuthority(it, null) + } + ) + } + }, + ) + + whenever(activities) { activityList } + } + + whenever(getPkg()) { pkg } + whenever(getName()) { pkgName } + whenever(this.domainSetId) { domainSetId } + whenever(getInstantApp(anyInt())) { false } + whenever(firstInstallTime) { 0L } + } + + fun DomainVerificationService.addPackages(vararg pkgSettings: PackageSetting) = + pkgSettings.forEach(::addPackage) +} 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 3b3dd61c44b16..ae1e82823c834 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 @@ -23,6 +23,7 @@ import android.content.pm.parsing.component.ParsedIntentInfo import android.content.pm.verify.domain.DomainVerificationInfo.STATE_NO_RESPONSE import android.content.pm.verify.domain.DomainVerificationInfo.STATE_SUCCESS import android.content.pm.verify.domain.DomainVerificationInfo.STATE_UNMODIFIABLE +import android.content.pm.verify.domain.DomainVerificationManager import android.content.pm.verify.domain.DomainVerificationState import android.content.pm.verify.domain.DomainVerificationUserState.DOMAIN_STATE_NONE import android.content.pm.verify.domain.DomainVerificationUserState.DOMAIN_STATE_SELECTED @@ -44,7 +45,6 @@ import org.mockito.ArgumentMatchers.any import org.mockito.ArgumentMatchers.anyInt import org.mockito.ArgumentMatchers.anyString import java.util.UUID -import kotlin.test.assertFailsWith class DomainVerificationPackageTest { @@ -172,11 +172,11 @@ class DomainVerificationPackageTest { ArraySet(setOf(DOMAIN_1))) service.setDomainVerificationStatusInternal(pkgName, DomainVerificationState.STATE_DENIED, ArraySet(setOf(DOMAIN_3))) - service.setDomainVerificationUserSelection( + service.setUserSelection( UUID_ONE, setOf(DOMAIN_2, DOMAIN_4), true, USER_ID) // Check the verifier cannot change the shell approve/deny states - service.setDomainVerificationStatus(UUID_ONE, setOf(DOMAIN_1, DOMAIN_3), STATE_SUCCESS) + service.setStatus(UUID_ONE, setOf(DOMAIN_1, DOMAIN_3), STATE_SUCCESS) assertThat(service.getInfo(pkgName).hostToStateMap).containsExactlyEntriesIn(mapOf( DOMAIN_1 to STATE_UNMODIFIABLE, @@ -240,14 +240,11 @@ class DomainVerificationPackageTest { map[pkgName] = pkgAfter - assertFailsWith(IllegalArgumentException::class) { - service.setDomainVerificationStatus(UUID_ONE, mutableSetOf(DOMAIN_1), STATE_SUCCESS) - } + assertThat(service.setStatus(UUID_ONE, setOf(DOMAIN_1), STATE_SUCCESS)) + .isNotEqualTo(DomainVerificationManager.STATUS_OK) - assertFailsWith(IllegalArgumentException::class) { - service.setDomainVerificationUserSelection( - UUID_ONE, mutableSetOf(DOMAIN_2), true, USER_ID) - } + assertThat(service.setUserSelection(UUID_ONE, setOf(DOMAIN_2), true, USER_ID)) + .isNotEqualTo(DomainVerificationManager.STATUS_OK) assertThat(service.getDomainVerificationInfo(pkgName)).isNull() assertThat(service.getUserState(pkgName).hostToStateMap).isEmpty() @@ -274,8 +271,8 @@ class DomainVerificationPackageTest { // a live package inside the addPackage logic. It should only use the provided input. map[pkgName] = pkgBefore - service.setDomainVerificationStatus(UUID_ONE, setOf(DOMAIN_1), STATE_SUCCESS) - service.setDomainVerificationUserSelection(UUID_ONE, setOf(DOMAIN_2), true, USER_ID) + service.setStatus(UUID_ONE, setOf(DOMAIN_1), STATE_SUCCESS) + service.setUserSelection(UUID_ONE, setOf(DOMAIN_2), true, USER_ID) assertThat(service.getInfo(pkgName).hostToStateMap).containsExactlyEntriesIn(mapOf( DOMAIN_1 to STATE_SUCCESS, @@ -320,14 +317,11 @@ class DomainVerificationPackageTest { // a live package inside the addPackage logic. It should only use the provided input. map[pkgName] = pkgBefore - assertFailsWith(IllegalArgumentException::class) { - service.setDomainVerificationStatus(UUID_ONE, mutableSetOf(DOMAIN_1), STATE_SUCCESS) - } + assertThat(service.setStatus(UUID_ONE, setOf(DOMAIN_1), STATE_SUCCESS)) + .isNotEqualTo(DomainVerificationManager.STATUS_OK) - assertFailsWith(IllegalArgumentException::class) { - service.setDomainVerificationUserSelection( - UUID_ONE, mutableSetOf(DOMAIN_2), true, USER_ID) - } + assertThat(service.setUserSelection(UUID_ONE, setOf(DOMAIN_2), true, USER_ID)) + .isNotEqualTo(DomainVerificationManager.STATUS_OK) assertThat(service.getDomainVerificationInfo(pkgName)).isNull() assertThat(service.getUserState(pkgName).hostToStateMap).isEmpty() diff --git a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationProxyTest.kt b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationProxyTest.kt index 62e3b73ec6ad1..a9b77ea6d95b6 100644 --- a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationProxyTest.kt +++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationProxyTest.kt @@ -119,7 +119,9 @@ class DomainVerificationProxyTest { else -> throw IllegalArgumentException("Unexpected package name $pkgName") } } - whenever(setDomainVerificationStatusInternal(anyInt(), any(), any(), anyInt())) + whenever(setDomainVerificationStatusInternal(anyInt(), any(), any(), anyInt())) { + DomainVerificationManager.STATUS_OK + } } collector = mockThrowOnUnmocked { whenever(collectValidAutoVerifyDomains(any())) { diff --git a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationServiceUtil.kt b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationServiceUtil.kt new file mode 100644 index 0000000000000..6859b113298db --- /dev/null +++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationServiceUtil.kt @@ -0,0 +1,31 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.pm.test.verify.domain + +import android.annotation.UserIdInt +import com.android.server.pm.verify.domain.DomainVerificationService +import java.util.UUID + +fun DomainVerificationService.setStatus(domainSetId: UUID, domains: Set, state: Int) = + setDomainVerificationStatus(domainSetId, domains.toMutableSet(), state) + +fun DomainVerificationService.setUserSelection( + domainSetId: UUID, + domains: Set, + enabled: Boolean, + @UserIdInt userId: Int +) = setDomainVerificationUserSelection(domainSetId, domains.toMutableSet(), enabled, userId) 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 0f46629c08f72..f4ab182c6f98d 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 @@ -20,6 +20,7 @@ import android.content.Intent import android.content.pm.PackageManager import android.content.pm.parsing.component.ParsedActivity import android.content.pm.parsing.component.ParsedIntentInfo +import android.content.pm.verify.domain.DomainVerificationManager import android.content.pm.verify.domain.DomainVerificationState import android.content.pm.verify.domain.DomainVerificationUserState import android.os.Build @@ -154,7 +155,7 @@ class DomainVerificationUserStateOverrideTest { .containsExactly(PKG_TWO) } - @Test(expected = IllegalArgumentException::class) + @Test fun anotherPackageTakeoverFailure() { val service = makeService() @@ -166,7 +167,8 @@ class DomainVerificationUserStateOverrideTest { .containsExactly(PKG_ONE) // Attempt override by package 2 - service.setDomainVerificationUserSelection(UUID_TWO, setOf(DOMAIN_ONE), true, USER_ID) + assertThat(service.setDomainVerificationUserSelection(UUID_TWO, setOf(DOMAIN_ONE), true, + USER_ID)).isEqualTo(DomainVerificationManager.ERROR_UNABLE_TO_APPROVE) } private fun DomainVerificationService.stateFor(pkgName: String, host: String) = From 3780a17d96b416a4885aea51ef870f588df4c620 Mon Sep 17 00:00:00 2001 From: Winson Date: Thu, 11 Mar 2021 11:53:00 -0800 Subject: [PATCH 4/7] Check user ID for domain verification setters Validates the calling/target user IDs still exist, to handle race conditions. Since the data structures are not locked during the user check, also adds a call during user creation to wipe the user data if there's any stale values left. This should never practically happen, but it's a fairly fast call just to make sure. Bug: 182416431 Test: atest com.android.server.pm.test.verify.domain Change-Id: I6cc270fb0e85f0c920b3b71a52e059e818066fac --- .../server/pm/PackageManagerService.java | 6 + .../domain/DomainVerificationEnforcer.java | 32 +++ .../domain/DomainVerificationEnforcerTest.kt | 226 +++++++++--------- .../DomainVerificationManagerApiTest.kt | 2 + .../domain/DomainVerificationPackageTest.kt | 2 + .../DomainVerificationSettingsMutationTest.kt | 2 + ...inVerificationUserSelectionOverrideTest.kt | 2 + 7 files changed, 157 insertions(+), 115 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index dfe72b26f72ae..317effab37503 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -1768,6 +1768,11 @@ public class PackageManagerService extends IPackageManager.Stub public int[] getAllUserIds() { return mUserManager.getUserIds(); } + + @Override + public boolean doesUserExist(@UserIdInt int userId) { + return mUserManager.exists(userId); + } } /** @@ -25693,6 +25698,7 @@ public class PackageManagerService extends IPackageManager.Stub if (!convertedFromPreCreated || !readPermissionStateForUser(userId)) { mPermissionManager.onUserCreated(userId); mLegacyPermissionManager.grantDefaultPermissions(userId); + mDomainVerificationManager.clearUser(userId); } } diff --git a/services/core/java/com/android/server/pm/verify/domain/DomainVerificationEnforcer.java b/services/core/java/com/android/server/pm/verify/domain/DomainVerificationEnforcer.java index 1721a18f4f604..f4bcd3e65913d 100644 --- a/services/core/java/com/android/server/pm/verify/domain/DomainVerificationEnforcer.java +++ b/services/core/java/com/android/server/pm/verify/domain/DomainVerificationEnforcer.java @@ -141,6 +141,12 @@ public class DomainVerificationEnforcer { "Caller is not allowed to edit other users"); } + if (!mCallback.doesUserExist(callingUserId)) { + throw new SecurityException("User " + callingUserId + " does not exist"); + } else if (!mCallback.doesUserExist(targetUserId)) { + throw new SecurityException("User " + targetUserId + " does not exist"); + } + return !mCallback.filterAppAccess(packageName, callingUid, targetUserId); } @@ -161,6 +167,12 @@ public class DomainVerificationEnforcer { Binder.getCallingPid(), callingUid, "Caller is not allowed to edit user selections"); + if (!mCallback.doesUserExist(callingUserId)) { + throw new SecurityException("User " + callingUserId + " does not exist"); + } else if (!mCallback.doesUserExist(targetUserId)) { + throw new SecurityException("User " + targetUserId + " does not exist"); + } + if (packageName == null) { return true; } @@ -184,6 +196,12 @@ public class DomainVerificationEnforcer { } } + if (!mCallback.doesUserExist(callingUserId)) { + throw new SecurityException("User " + callingUserId + " does not exist"); + } else if (!mCallback.doesUserExist(targetUserId)) { + throw new SecurityException("User " + targetUserId + " does not exist"); + } + return !mCallback.filterAppAccess(packageName, callingUid, targetUserId); } @@ -197,6 +215,12 @@ public class DomainVerificationEnforcer { "Caller is not allowed to edit other users"); } + if (!mCallback.doesUserExist(callingUserId)) { + throw new SecurityException("User " + callingUserId + " does not exist"); + } else if (!mCallback.doesUserExist(targetUserId)) { + throw new SecurityException("User " + targetUserId + " does not exist"); + } + return !mCallback.filterAppAccess(packageName, callingUid, targetUserId); } @@ -221,6 +245,12 @@ public class DomainVerificationEnforcer { mContext.enforcePermission( android.Manifest.permission.UPDATE_DOMAIN_VERIFICATION_USER_SELECTION, callingPid, callingUid, "Caller is not allowed to query user selections"); + + if (!mCallback.doesUserExist(callingUserId)) { + throw new SecurityException("User " + callingUserId + " does not exist"); + } else if (!mCallback.doesUserExist(targetUserId)) { + throw new SecurityException("User " + targetUserId + " does not exist"); + } } public interface Callback { @@ -229,5 +259,7 @@ public class DomainVerificationEnforcer { * if the package was not installed */ boolean filterAppAccess(@NonNull String packageName, int callingUid, @UserIdInt int userId); + + boolean doesUserExist(@UserIdInt int 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 6ac9907698312..7e25901301aaa 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 @@ -51,6 +51,8 @@ import java.io.File import java.util.UUID import java.util.concurrent.atomic.AtomicBoolean import java.util.concurrent.atomic.AtomicInteger +import kotlin.test.assertFailsWith +import kotlin.test.fail @RunWith(Parameterized::class) class DomainVerificationEnforcerTest { @@ -82,47 +84,49 @@ class DomainVerificationEnforcerTest { whenever(filterAppAccess(eq(INVISIBLE_PKG), anyInt(), anyInt())) { true } + whenever(doesUserExist(anyInt())) { (arguments[0] as Int) <= 1 } }) } } - val makeService: (Context) -> Triple = - { - val callingUidInt = AtomicInteger(-1) - val callingUserIdInt = AtomicInteger(-1) + val makeService: (Context) -> Triple = { + val callingUidInt = AtomicInteger(-1) + val callingUserIdInt = AtomicInteger(-1) - val connection: DomainVerificationManagerInternal.Connection = - mockThrowOnUnmocked { - whenever(callingUid) { callingUidInt.get() } - whenever(callingUserId) { callingUserIdInt.get() } - whenever(getPackageSettingLocked(VISIBLE_PKG)) { visiblePkgSetting } - whenever(getPackageLocked(VISIBLE_PKG)) { visiblePkg } - whenever(getPackageSettingLocked(INVISIBLE_PKG)) { invisiblePkgSetting } - whenever(getPackageLocked(INVISIBLE_PKG)) { invisiblePkg } - whenever(schedule(anyInt(), any())) - whenever(scheduleWriteSettings()) - whenever(filterAppAccess(eq(VISIBLE_PKG), anyInt(), anyInt())) { false } - whenever(filterAppAccess(eq(INVISIBLE_PKG), anyInt(), anyInt())) { - true - } + val connection: DomainVerificationManagerInternal.Connection = + mockThrowOnUnmocked { + whenever(callingUid) { callingUidInt.get() } + whenever(callingUserId) { callingUserIdInt.get() } + whenever(getPackageSettingLocked(VISIBLE_PKG)) { visiblePkgSetting } + whenever(getPackageLocked(VISIBLE_PKG)) { visiblePkg } + whenever(getPackageSettingLocked(INVISIBLE_PKG)) { invisiblePkgSetting } + whenever(getPackageLocked(INVISIBLE_PKG)) { invisiblePkg } + whenever(schedule(anyInt(), any())) + whenever(scheduleWriteSettings()) + whenever(filterAppAccess(eq(VISIBLE_PKG), anyInt(), anyInt())) { false } + whenever(filterAppAccess(eq(INVISIBLE_PKG), anyInt(), anyInt())) { + true } - val service = DomainVerificationService( - it, - mockThrowOnUnmocked { whenever(linkedApps) { ArraySet() } }, - mockThrowOnUnmocked { - whenever( - isChangeEnabled( - anyLong(), - any() - ) - ) { true } - }).apply { - setConnection(connection) + whenever(doesUserExist(anyInt())) { (arguments[0] as Int) <= 1 } } - - Triple(callingUidInt, callingUserIdInt, service) + val service = DomainVerificationService( + it, + mockThrowOnUnmocked { whenever(linkedApps) { ArraySet() } }, + mockThrowOnUnmocked { + whenever( + isChangeEnabled( + anyLong(), + any() + ) + ) { true } + }).apply { + setConnection(connection) } + Triple(callingUidInt, callingUserIdInt, service) + } + fun enforcer( type: Type, name: String, @@ -476,24 +480,7 @@ class DomainVerificationEnforcerTest { fun runTestCases(callingUserId: Int, targetUserId: Int, throws: Boolean) { // User selector makes no distinction by UID val allUids = INTERNAL_UIDS + VERIFIER_UID + NON_VERIFIER_UID - if (throws) { - allUids.forEach { - assertFails { - runMethod(target, it, visible = true, callingUserId, targetUserId) - } - } - } else { - allUids.forEach { - runMethod(target, it, visible = true, callingUserId, targetUserId) - } - } - - // User selector doesn't use QUERY_ALL, so the invisible package should always fail - allUids.forEach { - assertFails { - runMethod(target, it, visible = false, callingUserId, targetUserId) - } - } + runCrossUserMethod(allUids, target, callingUserId, targetUserId, throws) } val callingUserId = 0 @@ -530,24 +517,7 @@ class DomainVerificationEnforcerTest { fun runTestCases(callingUserId: Int, targetUserId: Int, throws: Boolean) { // User selector makes no distinction by UID val allUids = INTERNAL_UIDS + VERIFIER_UID + NON_VERIFIER_UID - if (throws) { - allUids.forEach { - assertFails { - runMethod(target, it, visible = true, callingUserId, targetUserId) - } - } - } else { - allUids.forEach { - runMethod(target, it, visible = true, callingUserId, targetUserId) - } - } - - // User selector doesn't use QUERY_ALL, so the invisible package should always fail - allUids.forEach { - assertFails { - runMethod(target, it, visible = false, callingUserId, targetUserId) - } - } + runCrossUserMethod(allUids, target, callingUserId, targetUserId, throws) } val callingUserId = 0 @@ -591,24 +561,10 @@ class DomainVerificationEnforcerTest { fun runTestCases(callingUserId: Int, targetUserId: Int, throws: Boolean) { // Legacy makes no distinction by UID val allUids = INTERNAL_UIDS + VERIFIER_UID + NON_VERIFIER_UID - if (throws) { - allUids.forEach { - assertFails { - runMethod(target, it, visible = true, callingUserId, targetUserId) - } - } - } else { - allUids.forEach { - runMethod(target, it, visible = true, callingUserId, targetUserId) - } - } - - // Legacy doesn't use QUERY_ALL, so the invisible package should always fail - allUids.forEach { - assertFails { - runMethod(target, it, visible = false, callingUserId, targetUserId) - } - } + // The legacy selector does a silent failure when the user IDs don't match, so it + // cannot verify the non-existent user ID check, as it will not throw an Exception. + runCrossUserMethod(allUids, target, callingUserId, targetUserId, throws, + verifyUserIdCheck = false) } val callingUserId = 0 @@ -664,24 +620,8 @@ class DomainVerificationEnforcerTest { fun runTestCases(callingUserId: Int, targetUserId: Int, throws: Boolean) { // Legacy makes no distinction by UID val allUids = INTERNAL_UIDS + VERIFIER_UID + NON_VERIFIER_UID - if (throws) { - allUids.forEach { - assertFailsLegacy { - runMethod(target, it, visible = true, callingUserId, targetUserId) - } - } - } else { - allUids.forEach { - runMethod(target, it, visible = true, callingUserId, targetUserId) - } - } - - // Legacy doesn't use QUERY_ALL, so the invisible package should always fail - allUids.forEach { - assertFailsLegacy { - runMethod(target, it, visible = false, callingUserId, targetUserId) - } - } + runCrossUserMethod(allUids, target, callingUserId, targetUserId, throws, + assertFailsMethod = ::assertFailsLegacy) } val callingUserId = 0 @@ -723,17 +663,8 @@ class DomainVerificationEnforcerTest { fun runTestCases(callingUserId: Int, targetUserId: Int, throws: Boolean) { // Owner querent makes no distinction by UID val allUids = INTERNAL_UIDS + VERIFIER_UID + NON_VERIFIER_UID - if (throws) { - allUids.forEach { - assertFails { - runMethod(target, it, visible = true, callingUserId, targetUserId) - } - } - } else { - allUids.forEach { - runMethod(target, it, visible = true, callingUserId, targetUserId) - } - } + runCrossUserMethod(allUids, target, callingUserId, targetUserId, throws, + verifyInvisiblePkg = false) } val callingUserId = 0 @@ -801,6 +732,71 @@ class DomainVerificationEnforcerTest { return params.runMethod(target, callingUid, callingUserId, userId, packageName, uuid, proxy) } + private fun runCrossUserMethod( + allUids: Iterable, + target: Any, + callingUserId: Int, + targetUserId: Int, + throws: Boolean, + verifyUserIdCheck: Boolean = true, + verifyInvisiblePkg: Boolean = true, + assertFailsMethod: (() -> Any?) -> Unit = ::assertFails, + ) { + if (throws) { + allUids.forEach { + assertFailsMethod { + // When testing a non-user ID failure, send an invalid user ID. + // This ensures the failure occurs before the user ID check is run. + try { + runMethod(target, it, visible = true, callingUserId, 100) + } catch (e: SecurityException) { + if (verifyUserIdCheck) { + e.message?.let { + if (it.contains("user ID", ignoreCase = true) + || it.contains("100")) { + fail( + "Method should not check user existence before permissions" + ) + } + } + } + + // Rethrow to allow normal fail checking logic to run + throw e + } + } + } + } else { + allUids.forEach { + runMethod(target, it, visible = true, callingUserId, targetUserId) + } + } + + if (verifyInvisiblePkg) { + allUids.forEach { + assertFailsMethod { + runMethod(target, it, visible = false, callingUserId, targetUserId) + } + } + } + + if (verifyUserIdCheck) { + // An invalid target user ID should always fail + allUids.forEach { + assertFailsWith(SecurityException::class) { + runMethod(target, it, visible = true, callingUserId, 100) + } + } + + // An invalid calling user ID should always fail, although this cannot happen in prod + allUids.forEach { + assertFailsWith(SecurityException::class) { + runMethod(target, it, visible = true, 100, targetUserId) + } + } + } + } + private fun assertFails(block: () -> Any?) { try { val value = block() 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 ec4775260f33e..0e74b65d25d57 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 @@ -314,6 +314,8 @@ class DomainVerificationManagerApiTest { }).apply { setConnection(mockThrowOnUnmocked { whenever(filterAppAccess(anyString(), anyInt(), anyInt())) { false } + whenever(doesUserExist(0)) { true } + whenever(doesUserExist(1)) { true } whenever(scheduleWriteSettings()) // Need to provide an internal UID so some permission checks are ignored 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 ae1e82823c834..fe3672d06bc03 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 @@ -370,6 +370,8 @@ class DomainVerificationPackageTest { }).apply { setConnection(mockThrowOnUnmocked { whenever(filterAppAccess(anyString(), anyInt(), anyInt())) { false } + whenever(doesUserExist(0)) { true } + whenever(doesUserExist(1)) { true } whenever(scheduleWriteSettings()) // Need to provide an internal UID so some permission checks are ignored 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 ee87a5f67dd48..377bae15e2d52 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 @@ -265,5 +265,7 @@ class DomainVerificationSettingsMutationTest { // This doesn't check for visibility; that's done in the enforcer test whenever(filterAppAccess(anyString(), anyInt(), anyInt())) { false } + whenever(doesUserExist(0)) { true } + whenever(doesUserExist(10)) { true } } } 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 f4ab182c6f98d..44c1b8f3fbb95 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 @@ -75,6 +75,8 @@ class DomainVerificationUserStateOverrideTest { }).apply { setConnection(mockThrowOnUnmocked { whenever(filterAppAccess(anyString(), anyInt(), anyInt())) { false } + whenever(doesUserExist(0)) { true } + whenever(doesUserExist(1)) { true } whenever(scheduleWriteSettings()) // Need to provide an internal UID so some permission checks are ignored From 7a95ff7a546618c07c149b1dd2f05b75ef33c869 Mon Sep 17 00:00:00 2001 From: Winson Date: Mon, 15 Mar 2021 11:27:12 -0700 Subject: [PATCH 5/7] Remove pending/restored domain state when attaching Otherwise it's persisted beyond the package's uninstall. Bug: 182812601 Test: manual tested as part of writing CTS Change-Id: I35072d6e4bf142db463860d01e0101098e82a185 --- .../pm/verify/domain/DomainVerificationService.java | 4 ++-- .../pm/verify/domain/DomainVerificationSettings.java | 8 ++++---- 2 files changed, 6 insertions(+), 6 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 6896a42376f4f..022e7dbe79915 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 @@ -867,13 +867,13 @@ public class DomainVerificationService extends SystemService boolean sendBroadcast = true; DomainVerificationPkgState pkgState; - pkgState = mSettings.getPendingState(pkgName); + pkgState = mSettings.removePendingState(pkgName); if (pkgState != null) { // Don't send when attaching from pending read, which is usually boot scan. Re-send on // boot is handled in a separate method once all packages are added. sendBroadcast = false; } else { - pkgState = mSettings.getRestoredState(pkgName); + pkgState = mSettings.removeRestoredState(pkgName); } AndroidPackage pkg = newPkgSetting.getPkg(); 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 f3d1dbb1f6ada..8b59da7bb9445 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 @@ -256,16 +256,16 @@ class DomainVerificationSettings { } @Nullable - public DomainVerificationPkgState getPendingState(@NonNull String pkgName) { + public DomainVerificationPkgState removePendingState(@NonNull String pkgName) { synchronized (mLock) { - return mPendingPkgStates.get(pkgName); + return mPendingPkgStates.remove(pkgName); } } @Nullable - public DomainVerificationPkgState getRestoredState(@NonNull String pkgName) { + public DomainVerificationPkgState removeRestoredState(@NonNull String pkgName) { synchronized (mLock) { - return mRestoredPkgStates.get(pkgName); + return mRestoredPkgStates.remove(pkgName); } } } From 27f0a7d1032ddb49d1b90e9228eb91dbc7408489 Mon Sep 17 00:00:00 2001 From: Winson Date: Mon, 15 Mar 2021 12:10:37 -0700 Subject: [PATCH 6/7] Fix DomainVerificationShell user/allowed bugs The USER_ALL was not being applied to the specific package name branch and the set-app-links-allowed boolean parsing was incorrect. Bug: 180753593 Test: manual, diagnosed when writing CTS Change-Id: I3d62972218bca2b4c3b713e5eb7597977d7ce271 --- .../domain/DomainVerificationService.java | 11 ++- .../domain/DomainVerificationShell.java | 69 ++++++++++++------- 2 files changed, 55 insertions(+), 25 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 022e7dbe79915..76d5b7ba88b11 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 @@ -473,8 +473,15 @@ public class DomainVerificationService extends SystemService throw DomainVerificationUtils.throwPackageUnavailable(packageName); } - pkgState.getOrCreateUserState(userId) - .setLinkHandlingAllowed(allowed); + if (userId == UserHandle.USER_ALL) { + for (int aUserId : mConnection.getAllUserIds()) { + pkgState.getOrCreateUserState(aUserId) + .setLinkHandlingAllowed(allowed); + } + } else { + pkgState.getOrCreateUserState(userId) + .setLinkHandlingAllowed(allowed); + } } } diff --git a/services/core/java/com/android/server/pm/verify/domain/DomainVerificationShell.java b/services/core/java/com/android/server/pm/verify/domain/DomainVerificationShell.java index 94767f555574c..7e755fa384dbb 100644 --- a/services/core/java/com/android/server/pm/verify/domain/DomainVerificationShell.java +++ b/services/core/java/com/android/server/pm/verify/domain/DomainVerificationShell.java @@ -38,6 +38,8 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Locale; +import java.util.function.Function; public class DomainVerificationShell { @@ -226,22 +228,19 @@ public class DomainVerificationShell { userId = translateUserId(userId, "runSetAppLinksUserState"); - String enabledString = commandHandler.getNextArgRequired(); + String enabledArg = commandHandler.getNextArg(); + if (TextUtils.isEmpty(enabledArg)) { + commandHandler.getErrPrintWriter().println("Error: enabled param not specified"); + return false; + } - // Manually ensure that "true" and "false" are the only options, to ensure a domain isn't - // accidentally parsed as a boolean boolean enabled; - switch (enabledString) { - case "true": - enabled = true; - break; - case "false": - enabled = false; - break; - default: - commandHandler.getErrPrintWriter().println( - "Invalid enabled param: " + enabledString); - return false; + try { + enabled = parseEnabled(enabledArg); + } catch (IllegalArgumentException e) { + commandHandler.getErrPrintWriter() + .println("Error: invalid enabled param: " + e.getMessage()); + return false; } ArraySet domains = new ArraySet<>(getRemainingArgs(commandHandler)); @@ -255,8 +254,8 @@ public class DomainVerificationShell { } try { - mCallback.setDomainVerificationUserSelectionInternal(userId, - packageName, enabled, domains); + mCallback.setDomainVerificationUserSelectionInternal(userId, packageName, enabled, + domains); } catch (NameNotFoundException e) { commandHandler.getErrPrintWriter().println("Package not found: " + packageName); return false; @@ -362,15 +361,12 @@ public class DomainVerificationShell { private boolean runSetAppLinksAllowed(@NonNull BasicShellCommandHandler commandHandler) { String packageName = null; Integer userId = null; - Boolean allowed = null; String option; while ((option = commandHandler.getNextOption()) != null) { if (option.equals("--package")) { - packageName = commandHandler.getNextArgRequired(); - } if (option.equals("--user")) { + packageName = commandHandler.getNextArg(); + } else if (option.equals("--user")) { userId = UserHandle.parseUserArg(commandHandler.getNextArgRequired()); - } else if (allowed == null) { - allowed = Boolean.valueOf(option); } else { commandHandler.getErrPrintWriter().println("Error: unexpected option: " + option); return false; @@ -389,11 +385,21 @@ public class DomainVerificationShell { return false; } - if (allowed == null) { + String allowedArg = commandHandler.getNextArg(); + if (TextUtils.isEmpty(allowedArg)) { commandHandler.getErrPrintWriter().println("Error: allowed setting not specified"); return false; } + boolean allowed; + try { + allowed = parseEnabled(allowedArg); + } catch (IllegalArgumentException e) { + commandHandler.getErrPrintWriter() + .println("Error: invalid allowed setting: " + e.getMessage()); + return false; + } + userId = translateUserId(userId, "runSetAppLinksAllowed"); try { @@ -421,6 +427,22 @@ public class DomainVerificationShell { userId, true, true, logContext, "pm command"); } + /** + * Manually ensure that "true" and "false" are the only options, to ensure a domain isn't + * accidentally parsed as a boolean. + */ + @NonNull + private boolean parseEnabled(@NonNull String arg) throws IllegalArgumentException { + switch (arg.toLowerCase(Locale.US)) { + case "true": + return true; + case "false": + return false; + default: + throw new IllegalArgumentException(arg + " is not a valid boolean"); + } + } + /** * Separated interface from {@link DomainVerificationManagerInternal} to hide methods that are * even more internal, and so that testing is easier. @@ -498,7 +520,8 @@ public class DomainVerificationShell { void verifyPackages(@Nullable List packageNames, boolean reVerify); /** - * @see DomainVerificationManagerInternal#printState(IndentingPrintWriter, String, Integer) + * @see DomainVerificationManagerInternal#printState(IndentingPrintWriter, String, Integer, + * Function) */ void printState(@NonNull IndentingPrintWriter writer, @Nullable String packageName, @Nullable @UserIdInt Integer userId) throws NameNotFoundException; From 1122021b0c703dde22633d669f3afa6e63b1c32d Mon Sep 17 00:00:00 2001 From: Winson Date: Fri, 12 Mar 2021 10:43:11 -0800 Subject: [PATCH 7/7] Remove app links caller BROWSABLE requirement Does not require BROWSABLE category on the caller side anymore to apply app links logic. However, it is now enforced on the receiver side, such that all IntentFilter matches must contain BROWSABLE to be considered approved for a domain. Bug: 181890795 Change-Id: I8d5a10c3b8cc2370a4a69d4abe9ee74b1964f19e --- core/java/android/content/pm/ResolveInfo.java | 33 +++ .../android/server/pm/ComponentResolver.java | 2 +- .../server/pm/PackageManagerService.java | 3 +- .../domain/DomainVerificationService.java | 207 ++++++++++-------- .../domain/DomainVerificationUtils.java | 35 +-- 5 files changed, 155 insertions(+), 125 deletions(-) diff --git a/core/java/android/content/pm/ResolveInfo.java b/core/java/android/content/pm/ResolveInfo.java index fe8e4d7b1bb2a..6f07dd7a24e82 100644 --- a/core/java/android/content/pm/ResolveInfo.java +++ b/core/java/android/content/pm/ResolveInfo.java @@ -19,6 +19,7 @@ package android.content.pm; import android.annotation.SystemApi; import android.compat.annotation.UnsupportedAppUsage; import android.content.ComponentName; +import android.content.Intent; import android.content.IntentFilter; import android.graphics.drawable.Drawable; import android.os.Build; @@ -183,6 +184,17 @@ public class ResolveInfo implements Parcelable { @SystemApi public boolean handleAllWebDataURI; + /** + * Whether the resolved {@link IntentFilter} declares {@link Intent#CATEGORY_BROWSABLE} and is + * thus allowed to automatically resolve an {@link Intent} as it's assumed the action is safe + * for the user. + * + * Note that the above doesn't apply when this is the only result is returned in the candidate + * set, as the system will not prompt before opening the result. It only applies when there are + * multiple candidates. + */ + private final boolean mAutoResolutionAllowed; + /** {@hide} */ @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.R, trackingBug = 170729553) public ComponentInfo getComponentInfo() { @@ -364,8 +376,26 @@ public class ResolveInfo implements Parcelable { && INTENT_FORWARDER_ACTIVITY.equals(activityInfo.targetActivity); } + /** + * @see #mAutoResolutionAllowed + * @hide + */ + public boolean isAutoResolutionAllowed() { + return mAutoResolutionAllowed; + } + public ResolveInfo() { targetUserId = UserHandle.USER_CURRENT; + + // It's safer to assume that an unaware caller that constructs a ResolveInfo doesn't + // accidentally mark a result as auto resolveable. + mAutoResolutionAllowed = false; + } + + /** @hide */ + public ResolveInfo(boolean autoResolutionAllowed) { + targetUserId = UserHandle.USER_CURRENT; + mAutoResolutionAllowed = autoResolutionAllowed; } public ResolveInfo(ResolveInfo orig) { @@ -386,6 +416,7 @@ public class ResolveInfo implements Parcelable { system = orig.system; targetUserId = orig.targetUserId; handleAllWebDataURI = orig.handleAllWebDataURI; + mAutoResolutionAllowed = orig.mAutoResolutionAllowed; isInstantAppAvailable = orig.isInstantAppAvailable; } @@ -450,6 +481,7 @@ public class ResolveInfo implements Parcelable { dest.writeInt(noResourceId ? 1 : 0); dest.writeInt(iconResourceId); dest.writeInt(handleAllWebDataURI ? 1 : 0); + dest.writeInt(mAutoResolutionAllowed ? 1 : 0); dest.writeInt(isInstantAppAvailable ? 1 : 0); } @@ -498,6 +530,7 @@ public class ResolveInfo implements Parcelable { noResourceId = source.readInt() != 0; iconResourceId = source.readInt(); handleAllWebDataURI = source.readInt() != 0; + mAutoResolutionAllowed = source.readInt() != 0; isInstantAppAvailable = source.readInt() != 0; } diff --git a/services/core/java/com/android/server/pm/ComponentResolver.java b/services/core/java/com/android/server/pm/ComponentResolver.java index 4be509b3f4649..1d556fec31ea6 100644 --- a/services/core/java/com/android/server/pm/ComponentResolver.java +++ b/services/core/java/com/android/server/pm/ComponentResolver.java @@ -1492,7 +1492,7 @@ public class ComponentResolver { } return null; } - final ResolveInfo res = new ResolveInfo(); + final ResolveInfo res = new ResolveInfo(info.hasCategory(Intent.CATEGORY_BROWSABLE)); res.activityInfo = ai; if ((mFlags & PackageManager.GET_RESOLVED_FILTER) != 0) { res.filter = info; diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 317effab37503..1530e41917c7f 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -2644,8 +2644,7 @@ public class PackageManagerService extends IPackageManager.Stub // We'll want to include browser possibilities in a few cases boolean includeBrowser = false; - if (!DomainVerificationUtils.isDomainVerificationIntent(intent, candidates, - matchFlags)) { + if (!DomainVerificationUtils.isDomainVerificationIntent(intent, matchFlags)) { result.addAll(undefinedList); // Maybe add one for the other profile. if (xpDomainInfo != null && xpDomainInfo.highestApprovalLevel 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 76d5b7ba88b11..a0e252a8a28a6 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 @@ -1330,83 +1330,50 @@ public class DomainVerificationService extends SystemService @NonNull Function pkgSettingFunction) { String domain = intent.getData().getHost(); - // Collect package names - ArrayMap packageApprovals = new ArrayMap<>(); + // Collect valid infos + ArrayMap infoApprovals = new ArrayMap<>(); int infosSize = infos.size(); for (int index = 0; index < infosSize; index++) { - packageApprovals.put(infos.get(index).getComponentInfo().packageName, - APPROVAL_LEVEL_NONE); + final ResolveInfo info = infos.get(index); + // Only collect for intent filters that can auto resolve + if (info.isAutoResolutionAllowed()) { + infoApprovals.put(info, null); + } } // Find all approval levels - int highestApproval = fillMapWithApprovalLevels(packageApprovals, domain, userId, + int highestApproval = fillMapWithApprovalLevels(infoApprovals, domain, userId, pkgSettingFunction); if (highestApproval == APPROVAL_LEVEL_NONE) { return Pair.create(emptyList(), highestApproval); } - // Filter to highest, non-zero packages - ArraySet approvedPackages = new ArraySet<>(); - int approvalsSize = packageApprovals.size(); - for (int index = 0; index < approvalsSize; index++) { - if (packageApprovals.valueAt(index) == highestApproval) { - approvedPackages.add(packageApprovals.keyAt(index)); + // Filter to highest, non-zero infos + for (int index = infoApprovals.size() - 1; index >= 0; index--) { + if (infoApprovals.valueAt(index) != highestApproval) { + infoApprovals.removeAt(index); } } - ArraySet filteredPackages = new ArraySet<>(); - if (highestApproval == APPROVAL_LEVEL_LEGACY_ASK) { + if (highestApproval != APPROVAL_LEVEL_LEGACY_ASK) { // To maintain legacy behavior while the Settings API is not implemented, // show the chooser if all approved apps are marked ask, skipping the // last app, last declaration filtering. - filteredPackages.addAll(approvedPackages); - } else { - // Filter to last installed package - long latestInstall = Long.MIN_VALUE; - int approvedSize = approvedPackages.size(); - for (int index = 0; index < approvedSize; index++) { - String packageName = approvedPackages.valueAt(index); - PackageSetting pkgSetting = pkgSettingFunction.apply(packageName); - if (pkgSetting == null) { - continue; - } - long installTime = pkgSetting.getFirstInstallTime(); - if (installTime > latestInstall) { - latestInstall = installTime; - filteredPackages.clear(); - filteredPackages.add(packageName); - } else if (installTime == latestInstall) { - filteredPackages.add(packageName); - } - } + filterToLastFirstInstalled(infoApprovals, pkgSettingFunction); } - // Filter to approved ResolveInfos - ArrayMap> approvedInfos = new ArrayMap<>(); - for (int index = 0; index < infosSize; index++) { - ResolveInfo info = infos.get(index); - String packageName = info.getComponentInfo().packageName; - if (filteredPackages.contains(packageName)) { - List infosPerPackage = approvedInfos.get(packageName); - if (infosPerPackage == null) { - infosPerPackage = new ArrayList<>(); - approvedInfos.put(packageName, infosPerPackage); - } - infosPerPackage.add(info); - } + // Easier to transform into list as the filterToLastDeclared method + // requires swapping indexes, which doesn't work with ArrayMap keys + final int size = infoApprovals.size(); + List finalList = new ArrayList<>(size); + for (int index = 0; index < size; index++) { + finalList.add(infoApprovals.keyAt(index)); } - List finalList; - if (highestApproval == APPROVAL_LEVEL_LEGACY_ASK) { - // If legacy ask, skip the last declaration filtering - finalList = new ArrayList<>(); - int size = approvedInfos.size(); - for (int index = 0; index < size; index++) { - finalList.addAll(approvedInfos.valueAt(index)); - } - } else { + // If legacy ask, skip the last declaration filtering + if (highestApproval != APPROVAL_LEVEL_LEGACY_ASK) { // Find the last declared ResolveInfo per package - finalList = filterToLastDeclared(approvedInfos, pkgSettingFunction); + filterToLastDeclared(finalList, pkgSettingFunction); } return Pair.create(finalList, highestApproval); @@ -1415,68 +1382,127 @@ public class DomainVerificationService extends SystemService /** * @return highest approval level found */ - private int fillMapWithApprovalLevels(@NonNull ArrayMap inputMap, + @ApprovalLevel + private int fillMapWithApprovalLevels(@NonNull ArrayMap inputMap, @NonNull String domain, @UserIdInt int userId, @NonNull Function pkgSettingFunction) { int highestApproval = APPROVAL_LEVEL_NONE; int size = inputMap.size(); for (int index = 0; index < size; index++) { - String packageName = inputMap.keyAt(index); + if (inputMap.valueAt(index) != null) { + // Already filled by previous iteration + continue; + } + + ResolveInfo info = inputMap.keyAt(index); + final String packageName = info.getComponentInfo().packageName; PackageSetting pkgSetting = pkgSettingFunction.apply(packageName); if (pkgSetting == null) { - inputMap.setValueAt(index, APPROVAL_LEVEL_NONE); + fillInfoMapForSamePackage(inputMap, packageName, APPROVAL_LEVEL_NONE); continue; } int approval = approvalLevelForDomain(pkgSetting, domain, userId, domain); highestApproval = Math.max(highestApproval, approval); - inputMap.setValueAt(index, approval); + fillInfoMapForSamePackage(inputMap, packageName, approval); } return highestApproval; } - @NonNull - private List filterToLastDeclared( - @NonNull ArrayMap> inputMap, - @NonNull Function pkgSettingFunction) { - List finalList = new ArrayList<>(inputMap.size()); + private void fillInfoMapForSamePackage(@NonNull ArrayMap inputMap, + @NonNull String targetPackageName, @ApprovalLevel int level) { + final int size = inputMap.size(); + for (int index = 0; index < size; index++) { + final String packageName = inputMap.keyAt(index).getComponentInfo().packageName; + if (Objects.equals(targetPackageName, packageName)) { + inputMap.setValueAt(index, level); + } + } + } - int inputSize = inputMap.size(); - for (int inputIndex = 0; inputIndex < inputSize; inputIndex++) { - String packageName = inputMap.keyAt(inputIndex); - List infos = inputMap.valueAt(inputIndex); + @NonNull + private void filterToLastFirstInstalled(@NonNull ArrayMap inputMap, + @NonNull Function pkgSettingFunction) { + // First, find the package with the latest first install time + String targetPackageName = null; + long latestInstall = Long.MIN_VALUE; + final int size = inputMap.size(); + for (int index = 0; index < size; index++) { + ResolveInfo info = inputMap.keyAt(index); + String packageName = info.getComponentInfo().packageName; PackageSetting pkgSetting = pkgSettingFunction.apply(packageName); + if (pkgSetting == null) { + continue; + } + + long installTime = pkgSetting.getFirstInstallTime(); + if (installTime > latestInstall) { + latestInstall = installTime; + targetPackageName = packageName; + } + } + + // Then, remove all infos that don't match the package + for (int index = inputMap.size() - 1; index >= 0; index--) { + ResolveInfo info = inputMap.keyAt(index); + if (!Objects.equals(targetPackageName, info.getComponentInfo().packageName)) { + inputMap.removeAt(index); + } + } + } + + @NonNull + private void filterToLastDeclared(@NonNull List inputList, + @NonNull Function pkgSettingFunction) { + // Must call size each time as the size of the list will decrease + for (int index = 0; index < inputList.size(); index++) { + ResolveInfo info = inputList.get(index); + String targetPackageName = info.getComponentInfo().packageName; + PackageSetting pkgSetting = pkgSettingFunction.apply(targetPackageName); AndroidPackage pkg = pkgSetting == null ? null : pkgSetting.getPkg(); if (pkg == null) { continue; } - ResolveInfo result = null; - int highestIndex = -1; - int infosSize = infos.size(); - for (int infoIndex = 0; infoIndex < infosSize; infoIndex++) { - ResolveInfo info = infos.get(infoIndex); - List activities = pkg.getActivities(); - int activitiesSize = activities.size(); - for (int activityIndex = 0; activityIndex < activitiesSize; activityIndex++) { - if (Objects.equals(activities.get(activityIndex).getComponentName(), - info.getComponentInfo().getComponentName())) { - if (activityIndex > highestIndex) { - highestIndex = activityIndex; - result = info; - } - break; - } + ResolveInfo result = info; + int highestIndex = indexOfIntentFilterEntry(pkg, result); + + // Search backwards so that lower results can be removed as they're found + for (int searchIndex = inputList.size() - 1; searchIndex >= index + 1; searchIndex--) { + ResolveInfo searchInfo = inputList.get(searchIndex); + if (!Objects.equals(targetPackageName, searchInfo.getComponentInfo().packageName)) { + continue; } + + int entryIndex = indexOfIntentFilterEntry(pkg, searchInfo); + if (entryIndex > highestIndex) { + highestIndex = entryIndex; + result = searchInfo; + } + + // Always remove the entry so that the current index + // is left as the sole candidate of the target package + inputList.remove(searchIndex); } - // Shouldn't be null, but might as well be safe - if (result != null) { - finalList.add(result); + // Swap the current index for the result, leaving this as + // the only entry with the target package name + inputList.set(index, result); + } + } + + private int indexOfIntentFilterEntry(@NonNull AndroidPackage pkg, + @NonNull ResolveInfo target) { + List activities = pkg.getActivities(); + int activitiesSize = activities.size(); + for (int activityIndex = 0; activityIndex < activitiesSize; activityIndex++) { + if (Objects.equals(activities.get(activityIndex).getComponentName(), + target.getComponentInfo().getComponentName())) { + return activityIndex; } } - return finalList; + return -1; } @Override @@ -1484,8 +1510,7 @@ public class DomainVerificationService extends SystemService @NonNull List candidates, @PackageManager.ResolveInfoFlags int resolveInfoFlags, @UserIdInt int userId) { String packageName = pkgSetting.getName(); - if (!DomainVerificationUtils.isDomainVerificationIntent(intent, candidates, - resolveInfoFlags)) { + if (!DomainVerificationUtils.isDomainVerificationIntent(intent, resolveInfoFlags)) { if (DEBUG_APPROVAL) { debugApproval(packageName, intent, userId, false, "not valid intent"); } diff --git a/services/core/java/com/android/server/pm/verify/domain/DomainVerificationUtils.java b/services/core/java/com/android/server/pm/verify/domain/DomainVerificationUtils.java index 883bbad1bd2da..cb3b5c9db7e7c 100644 --- a/services/core/java/com/android/server/pm/verify/domain/DomainVerificationUtils.java +++ b/services/core/java/com/android/server/pm/verify/domain/DomainVerificationUtils.java @@ -22,7 +22,6 @@ import android.content.Intent; import android.content.pm.ApplicationInfo; import android.content.pm.PackageManager; import android.content.pm.PackageManager.NameNotFoundException; -import android.content.pm.ResolveInfo; import android.os.Binder; import com.android.internal.util.CollectionUtils; @@ -30,7 +29,6 @@ import com.android.server.compat.PlatformCompat; import com.android.server.pm.PackageManagerService; import com.android.server.pm.parsing.pkg.AndroidPackage; -import java.util.List; import java.util.Set; public final class DomainVerificationUtils { @@ -46,7 +44,6 @@ public final class DomainVerificationUtils { } public static boolean isDomainVerificationIntent(Intent intent, - @NonNull List candidates, @PackageManager.ResolveInfoFlags int resolveInfoFlags) { if (!intent.isWebIntent()) { return false; @@ -63,42 +60,18 @@ public final class DomainVerificationUtils { && intent.hasCategory(Intent.CATEGORY_BROWSABLE); } - // In cases where at least one browser is resolved and only one non-browser is resolved, - // the Intent is coerced into an app links intent, under the assumption the browser can - // be skipped if the app is approved at any level for the domain. - boolean foundBrowser = false; - boolean foundOneApp = false; - - final int candidatesSize = candidates.size(); - for (int index = 0; index < candidatesSize; index++) { - final ResolveInfo info = candidates.get(index); - if (info.handleAllWebDataURI) { - foundBrowser = true; - } else if (foundOneApp) { - // Already true, so duplicate app - foundOneApp = false; - break; - } else { - foundOneApp = true; - } - } - boolean matchDefaultByFlags = (resolveInfoFlags & PackageManager.MATCH_DEFAULT_ONLY) != 0; - boolean onlyOneNonBrowser = foundBrowser && foundOneApp; // Check if matches (BROWSABLE || none) && DEFAULT if (categoriesSize == 0) { - // No categories, run coerce case, matching DEFAULT by flags - return onlyOneNonBrowser && matchDefaultByFlags; - } else if (intent.hasCategory(Intent.CATEGORY_DEFAULT)) { - // Run coerce case, matching by explicit DEFAULT - return onlyOneNonBrowser; + // No categories, only allow matching DEFAULT by flags + return matchDefaultByFlags; } else if (intent.hasCategory(Intent.CATEGORY_BROWSABLE)) { // Intent matches BROWSABLE, must match DEFAULT by flags return matchDefaultByFlags; } else { - // Otherwise not matching any app link categories - return false; + // Otherwise only needs to have DEFAULT + return intent.hasCategory(Intent.CATEGORY_DEFAULT); } }