diff options
2 files changed, 220 insertions, 21 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 67aed4506b27..3cd7795005a7 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 @@ -317,7 +317,7 @@ public class DomainVerificationService extends SystemService mEnforcer.assertApprovedVerifier(callingUid, mProxy); final Computer snapshot = mConnection.snapshot(); synchronized (mLock) { - List<String> verifiedDomains = new ArrayList<>(); + List<String> newlyVerifiedDomains = new ArrayList<>(); GetAttachedResult result = getAndValidateAttachedLocked(domainSetId, domains, true /* forAutoVerify */, callingUid, null /* userId */, snapshot); @@ -329,21 +329,28 @@ public class DomainVerificationService extends SystemService ArrayMap<String, Integer> stateMap = pkgState.getStateMap(); for (String domain : domains) { Integer previousState = stateMap.get(domain); - if (previousState != null - && !DomainVerificationState.isModifiable(previousState)) { - continue; + // Skip if the state hasn't changed or can't be changed + if (previousState != null) { + if (previousState == state + || !DomainVerificationState.isModifiable(previousState)) { + continue; + } } if (DomainVerificationState.isVerified(state)) { - verifiedDomains.add(domain); + if (previousState == null + || !DomainVerificationState.isVerified(previousState)) { + newlyVerifiedDomains.add(domain); + } } stateMap.put(domain, state); } - int size = verifiedDomains.size(); + // For newly verified domains, revoke their user states across other packages + int size = newlyVerifiedDomains.size(); for (int index = 0; index < size; index++) { - removeUserStatesForDomain(verifiedDomains.get(index)); + removeUserStatesForDomain(pkgState, newlyVerifiedDomains.get(index)); } } @@ -367,7 +374,6 @@ public class DomainVerificationService extends SystemService "State must be one of NO_RESPONSE, SUCCESS, APPROVED, or DENIED"); } - ArraySet<String> verifiedDomains = new ArraySet<>(); if (packageName == null) { final Computer snapshot = mConnection.snapshot(); synchronized (mLock) { @@ -395,10 +401,6 @@ public class DomainVerificationService extends SystemService validDomains.retainAll(autoVerifyDomains); } - if (DomainVerificationState.isVerified(state)) { - verifiedDomains.addAll(validDomains); - } - setDomainVerificationStatusInternal(pkgState, state, validDomains); } } @@ -424,19 +426,33 @@ public class DomainVerificationService extends SystemService validDomains.retainAll(mCollector.collectValidAutoVerifyDomains(pkg)); } + ArraySet<String> newlyVerifiedDomains = null; if (DomainVerificationState.isVerified(state)) { - verifiedDomains.addAll(validDomains); + newlyVerifiedDomains = new ArraySet<>(); + ArrayMap<String, Integer> stateMap = pkgState.getStateMap(); + int domainsSize = validDomains.size(); + for (int domainIndex = 0; domainIndex < domainsSize; domainIndex++) { + String domain = validDomains.valueAt(domainIndex); + Integer oldState = stateMap.get(domain); + // Only remove if not previously verified + if (oldState == null || !DomainVerificationState.isVerified(oldState)) { + newlyVerifiedDomains.add(domain); + } + } } setDomainVerificationStatusInternal(pkgState, state, validDomains); - } - } - // Mirror SystemApi behavior of revoking user selection for approved domains. - if (DomainVerificationState.isVerified(state)) { - final int size = verifiedDomains.size(); - for (int index = 0; index < size; index++) { - removeUserStatesForDomain(verifiedDomains.valueAt(index)); + // Mirror SystemApi behavior of revoking user selection for approved domains. + // This is done in a second pass so that the previous state can be compared before + // the previous method overwrites it with the new state. + if (newlyVerifiedDomains != null) { + int domainsSize = newlyVerifiedDomains.size(); + for (int domainIndex = 0; domainIndex < domainsSize; domainIndex++) { + String domain = newlyVerifiedDomains.valueAt(domainIndex); + removeUserStatesForDomain(pkgState, domain); + } + } } } @@ -452,7 +468,10 @@ public class DomainVerificationService extends SystemService } } - private void removeUserStatesForDomain(@NonNull String domain) { + private void removeUserStatesForDomain(@NonNull DomainVerificationPkgState owningPkgState, + @NonNull String domain) { + SparseArray<DomainVerificationInternalUserState> owningUserStates = + owningPkgState.getUserStates(); synchronized (mLock) { final int size = mAttachedPkgStates.size(); for (int index = 0; index < size; index++) { @@ -460,6 +479,15 @@ public class DomainVerificationService extends SystemService SparseArray<DomainVerificationInternalUserState> array = pkgState.getUserStates(); int arraySize = array.size(); for (int arrayIndex = 0; arrayIndex < arraySize; arrayIndex++) { + int userId = array.keyAt(arrayIndex); + DomainVerificationInternalUserState owningUserState = + owningUserStates.get(userId); + if (owningUserState != null && !owningUserState.isLinkHandlingAllowed()) { + // Skip users where the owning package has their link handling disabled, + // since revoking those users would lead to no apps being able to handle + // the domain. + continue; + } array.valueAt(arrayIndex).removeHost(domain); } } @@ -527,6 +555,8 @@ public class DomainVerificationService extends SystemService mConnection.scheduleWriteSettings(); } + @CheckResult + @DomainVerificationManager.Error public int setDomainVerificationUserSelection(@NonNull UUID domainSetId, @NonNull Set<String> domains, boolean enabled, @UserIdInt int userId) throws NameNotFoundException { 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 40f37a7ee1f7..ed60c5064eb1 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 @@ -78,6 +78,8 @@ class DomainVerificationPackageTest { private val DOMAIN_4 = "four.$DOMAIN_BASE" private const val USER_ID = 0 + private const val USER_ID_SECONDARY = 10 + private val USER_IDS = listOf(USER_ID, USER_ID_SECONDARY) } private val pkg1 = mockPkgState(PKG_ONE, UUID_ONE, SIGNATURE_ONE) @@ -813,6 +815,173 @@ class DomainVerificationPackageTest { assertExpectedState(serviceAfter) } + @Test + fun verifiedUnapproved_unverifiedSelected_approvalCausesUnselect_systemApi() { + verifiedUnapproved_unverifiedSelected_approvalCausesUnselect { + setDomainVerificationStatus(it.domainSetId, setOf(DOMAIN_1, DOMAIN_2), STATE_SUCCESS) + } + } + + @Test + fun verifiedUnapproved_unverifiedSelected_approvalCausesUnselect_internalApi() { + verifiedUnapproved_unverifiedSelected_approvalCausesUnselect { + setDomainVerificationStatusInternal(it.packageName, STATE_SUCCESS, + ArraySet(setOf(DOMAIN_1, DOMAIN_2))) + } + } + + private fun verifiedUnapproved_unverifiedSelected_approvalCausesUnselect( + setStatusBlock: DomainVerificationService.(PackageStateInternal) -> Unit + ) { + /* + Domains tested: + 1: Becomes verified in package 1, but package 1 disabled in secondary user, only + disables selection for package 2 in main user + 2: Becomes verified in package 1, unselected by package 2, remains unselected + 3: Is autoVerify, but unverified, selected by package 2, remains selected + 4: Non-autoVerify, selected by package 2, remains selected + */ + + val pkg1 = mockPkgState( + PKG_ONE, + UUID_ONE, + SIGNATURE_ONE, + autoVerifyDomains = listOf(DOMAIN_1, DOMAIN_2, DOMAIN_3), + otherDomains = listOf(DOMAIN_4) + ) + val pkg2 = mockPkgState( + PKG_TWO, + UUID_TWO, + SIGNATURE_TWO, + autoVerifyDomains = emptyList(), + otherDomains = listOf(DOMAIN_1, DOMAIN_2, DOMAIN_3, DOMAIN_4) + ) + + val service = makeService(pkg1, pkg2) + service.addPackage(pkg1) + service.addPackage(pkg2) + + // Approve domain 1, 3, and 4 for package 2 for both users + USER_IDS.forEach { + assertThat( + service.setDomainVerificationUserSelection( + UUID_TWO, + setOf(DOMAIN_1, DOMAIN_3, DOMAIN_4), + true, + it + ) + ).isEqualTo(DomainVerificationManager.STATUS_OK) + } + + // But disable the owner package link handling in the secondary user + service.setDomainVerificationLinkHandlingAllowed(pkg1.packageName, false, + USER_ID_SECONDARY + ) + + service.assertState( + pkg1, + verifyState = listOf( + DOMAIN_1 to STATE_NO_RESPONSE, + DOMAIN_2 to STATE_NO_RESPONSE, + DOMAIN_3 to STATE_NO_RESPONSE, + ), + userState2LinkHandlingAllowed = false + ) + + service.assertState( + pkg2, + verifyState = null, + userState1DomainState1 = DOMAIN_STATE_SELECTED, + userState1DomainState3 = DOMAIN_STATE_SELECTED, + userState1DomainState4 = DOMAIN_STATE_SELECTED, + userState2DomainState1 = DOMAIN_STATE_SELECTED, + userState2DomainState3 = DOMAIN_STATE_SELECTED, + userState2DomainState4 = DOMAIN_STATE_SELECTED, + ) + + // Verify the owner package + service.setStatusBlock(pkg1) + + // Assert that package 1 is now verified, but link handling disabled in secondary user + service.assertState( + pkg1, + verifyState = listOf( + DOMAIN_1 to STATE_SUCCESS, + DOMAIN_2 to STATE_SUCCESS, + DOMAIN_3 to STATE_NO_RESPONSE, + ), + userState1DomainState1 = DOMAIN_STATE_VERIFIED, + userState1DomainState2 = DOMAIN_STATE_VERIFIED, + userState1DomainState3 = DOMAIN_STATE_NONE, + userState1DomainState4 = DOMAIN_STATE_NONE, + userState2LinkHandlingAllowed = false, + userState2DomainState1 = DOMAIN_STATE_VERIFIED, + userState2DomainState2 = DOMAIN_STATE_VERIFIED, + userState2DomainState3 = DOMAIN_STATE_NONE, + userState2DomainState4 = DOMAIN_STATE_NONE, + ) + + // Assert package 2 maintains selected in user where package 1 had link handling disabled + service.assertState( + pkg2, + verifyState = null, + userState1DomainState1 = DOMAIN_STATE_NONE, + userState1DomainState3 = DOMAIN_STATE_SELECTED, + userState1DomainState4 = DOMAIN_STATE_SELECTED, + userState2DomainState1 = DOMAIN_STATE_SELECTED, + userState2DomainState3 = DOMAIN_STATE_SELECTED, + userState2DomainState4 = DOMAIN_STATE_SELECTED, + ) + } + + fun DomainVerificationService.assertState( + pkg: PackageStateInternal, + verifyState: List<Pair<String, Int>>?, + userState1LinkHandlingAllowed: Boolean = true, + userState1DomainState1: Int = DOMAIN_STATE_NONE, + userState1DomainState2: Int = DOMAIN_STATE_NONE, + userState1DomainState3: Int = DOMAIN_STATE_NONE, + userState1DomainState4: Int = DOMAIN_STATE_NONE, + userState2LinkHandlingAllowed: Boolean = true, + userState2DomainState1: Int = DOMAIN_STATE_NONE, + userState2DomainState2: Int = DOMAIN_STATE_NONE, + userState2DomainState3: Int = DOMAIN_STATE_NONE, + userState2DomainState4: Int = DOMAIN_STATE_NONE, + ) { + if (verifyState == null) { + // If no auto verify domains, the info itself will be null + assertThat(getDomainVerificationInfo(pkg.packageName)).isNull() + } else { + getInfo(pkg.packageName).run { + assertThat(hostToStateMap).containsExactlyEntriesIn(verifyState.associate { it }) + } + } + + getUserState(pkg.packageName, USER_ID).run { + assertThat(isLinkHandlingAllowed).isEqualTo(userState1LinkHandlingAllowed) + assertThat(hostToStateMap).containsExactlyEntriesIn( + mapOf( + DOMAIN_1 to userState1DomainState1, + DOMAIN_2 to userState1DomainState2, + DOMAIN_3 to userState1DomainState3, + DOMAIN_4 to userState1DomainState4, + ) + ) + } + + getUserState(pkg.packageName, USER_ID_SECONDARY).run { + assertThat(isLinkHandlingAllowed).isEqualTo(userState2LinkHandlingAllowed) + assertThat(hostToStateMap).containsExactlyEntriesIn( + mapOf( + DOMAIN_1 to userState2DomainState1, + DOMAIN_2 to userState2DomainState2, + DOMAIN_3 to userState2DomainState3, + DOMAIN_4 to userState2DomainState4, + ) + ) + } + } + private fun DomainVerificationService.getInfo(pkgName: String) = getDomainVerificationInfo(pkgName) .also { assertThat(it).isNotNull() }!! |