Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix app links verification resetting user state

Browse Source 
In cases where domain verification comes in and does not take over
verification from another app (the owning app has link handling
disabled or the domain was already previous verified), the user
selection for that domain should not be reset.

This preserves user approved apps for links which the user has
disabled the official verified app when that verified app update,
as this event kicks off a verification through StatementService,
which may have caused the reset with the buggy code.

Bug: 221059351

Test: atest DomainVerificationPackageTest
Test: atest DomainVerificationIntentStandaloneTests
Test: manual, reset verification using shell set-app-links
Test: manual, reset verification using verified app update

Change-Id: Ia8aa70be485b2f94920fd97275fd16178895ef33
This commit is contained in:
Winson Chiu
2022-06-03 22:49:17 +00:00
parent fda91f9578
commit 5697a4c37e
2 changed files with 220 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

72
services/core/java/com/android/server/pm/verify/domain/DomainVerificationService.java
View File

@@ -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 {

169
services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationPackageTest.kt
View File

@@ -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() }!!