From 3482338bc1870a6f612dcc1d25c3a82791168daa Mon Sep 17 00:00:00 2001 From: Winson Date: Wed, 31 Mar 2021 14:04:26 -0700 Subject: [PATCH] Fix domain verify restore and add signature check Verifies that all signatures of the package being restored matches by SHA256 hash. And fixes a bug where the restored packages weren't being added to the internal structures. Bug: 170746586 Test: atest DomainVerificationPackageTest Change-Id: I2cf2e739a88396475599bc4c1ff27c36e09a8d45 --- .../domain/DomainVerificationService.java | 5 + .../domain/DomainVerificationSettings.java | 8 +- .../domain/DomainVerificationPackageTest.kt | 141 +++++++++++++++--- 3 files changed, 134 insertions(+), 20 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 b0ee46d54f269..4ae79a209524f 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 @@ -925,6 +925,11 @@ public class DomainVerificationService extends SystemService sendBroadcast = false; } else { pkgState = mSettings.removeRestoredState(pkgName); + if (pkgState != null && !Objects.equals(pkgState.getBackupSignatureHash(), + PackageUtils.computeSignaturesSha256Digest(newPkgSetting.getSignatures()))) { + // If restoring and the signatures don't match, drop the state + pkgState = null; + } } 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 44ffc5c146c57..c8e95b585b7a4 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 @@ -157,16 +157,20 @@ class DomainVerificationSettings { // mergePkgState runs, without the merge part. ArrayMap stateMap = newState.getStateMap(); int size = stateMap.size(); - for (int index = 0; index < size; index++) { + for (int index = size - 1; index >= 0; index--) { Integer stateInteger = stateMap.valueAt(index); if (stateInteger != null) { int state = stateInteger; if (state == DomainVerificationState.STATE_SUCCESS || state == DomainVerificationState.STATE_RESTORED) { - stateMap.setValueAt(index, state); + stateMap.setValueAt(index, DomainVerificationState.STATE_RESTORED); + } else { + stateMap.removeAt(index); } } } + + mRestoredPkgStates.put(pkgName, newState); } } } 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 8a549731af3a2..1097c45f8a9ff 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 @@ -19,8 +19,10 @@ package com.android.server.pm.test.verify.domain import android.content.Intent import android.content.pm.PackageManager import android.content.pm.PackageUserState +import android.content.pm.Signature import android.content.pm.parsing.component.ParsedActivity import android.content.pm.parsing.component.ParsedIntentInfo +import android.content.pm.verify.domain.DomainVerificationInfo.STATE_MODIFIABLE_VERIFIED import android.content.pm.verify.domain.DomainVerificationInfo.STATE_NO_RESPONSE import android.content.pm.verify.domain.DomainVerificationInfo.STATE_SUCCESS import android.content.pm.verify.domain.DomainVerificationInfo.STATE_UNMODIFIABLE @@ -42,7 +44,6 @@ 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.anyLong @@ -56,6 +57,10 @@ class DomainVerificationPackageTest { 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 const val SIGNATURE_ONE = "AA" + private const val DIGEST_ONE = + "BCEEF655B5A034911F1C3718CE056531B45EF03B4C7B1F15629E867294011A7D" + private const val SIGNATURE_TWO = "BB" private val DOMAIN_BASE = DomainVerificationPackageTest::class.java.packageName private val DOMAIN_1 = "one.$DOMAIN_BASE" @@ -66,8 +71,8 @@ class DomainVerificationPackageTest { private const val USER_ID = 0 } - private val pkg1 = mockPkgSetting(PKG_ONE, UUID_ONE) - private val pkg2 = mockPkgSetting(PKG_TWO, UUID_TWO) + private val pkg1 = mockPkgSetting(PKG_ONE, UUID_ONE, SIGNATURE_ONE) + private val pkg2 = mockPkgSetting(PKG_TWO, UUID_TWO, SIGNATURE_TWO) @Test fun addPackageFirstTime() { @@ -95,6 +100,104 @@ class DomainVerificationPackageTest { .containsExactly(pkg1.getName()) } + @Test + fun addPackageRestoredMatchingSignature() { + // language=XML + val xml = """ + + + + + + + + + + + """ + + val service = makeService(pkg1, pkg2) + service.restoreSettings(Xml.resolvePullParser(xml.byteInputStream())) + 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_MODIFIABLE_VERIFIED, + 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_VERIFIED, + DOMAIN_2 to DOMAIN_STATE_NONE, + ) + ) + + assertThat(service.queryValidVerificationPackageNames()) + .containsExactly(pkg1.getName()) + } + + @Test + fun addPackageRestoredMismatchSignature() { + // language=XML + val xml = """ + + + + + + + + + + + """ + + val service = makeService(pkg1, pkg2) + service.restoreSettings(Xml.resolvePullParser(xml.byteInputStream())) + 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 @@ -153,10 +256,9 @@ class DomainVerificationPackageTest { @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)) + val pkgBefore = mockPkgSetting(pkgName, UUID_ONE, SIGNATURE_ONE, + listOf(DOMAIN_1, DOMAIN_2, DOMAIN_3, DOMAIN_4)) + val pkgAfter = mockPkgSetting(pkgName, UUID_TWO, SIGNATURE_TWO, listOf(DOMAIN_1, DOMAIN_2)) // Test 4 domains: // 1 will be approved and preserved, 2 will be selected and preserved, @@ -215,8 +317,8 @@ class DomainVerificationPackageTest { @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 pkgBefore = mockPkgSetting(pkgName, UUID_ONE, SIGNATURE_ONE, listOf(DOMAIN_1, DOMAIN_2)) + val pkgAfter = mockPkgSetting(pkgName, UUID_TWO, SIGNATURE_TWO, emptyList()) val map = mutableMapOf() val service = makeService { map[it] } @@ -257,10 +359,9 @@ class DomainVerificationPackageTest { @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)) + val pkgBefore = mockPkgSetting(pkgName, UUID_ONE, SIGNATURE_ONE, listOf(DOMAIN_1, DOMAIN_2)) + val pkgAfter = mockPkgSetting(pkgName, UUID_TWO, SIGNATURE_TWO, + listOf(DOMAIN_1, DOMAIN_2, DOMAIN_3)) // Test 3 domains: // 1 will be verified and preserved, 2 will be selected and preserved, @@ -309,8 +410,8 @@ class DomainVerificationPackageTest { @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 pkgBefore = mockPkgSetting(pkgName, UUID_ONE, SIGNATURE_ONE, emptyList()) + val pkgAfter = mockPkgSetting(pkgName, UUID_TWO, SIGNATURE_TWO, listOf(DOMAIN_1, DOMAIN_2)) val map = mutableMapOf() val service = makeService { map[it] } @@ -387,9 +488,12 @@ class DomainVerificationPackageTest { }) } - private fun mockPkgSetting(pkgName: String, domainSetId: UUID, domains: List = listOf( - DOMAIN_1, DOMAIN_2 - )) = mockThrowOnUnmocked { + private fun mockPkgSetting( + pkgName: String, + domainSetId: UUID, + signature: String, + domains: List = listOf(DOMAIN_1, DOMAIN_2) + ) = mockThrowOnUnmocked { val pkg = mockThrowOnUnmocked { whenever(packageName) { pkgName } whenever(targetSdkVersion) { Build.VERSION_CODES.S } @@ -423,5 +527,6 @@ class DomainVerificationPackageTest { whenever(getInstantApp(anyInt())) { false } whenever(firstInstallTime) { 0L } whenever(readUserState(USER_ID)) { PackageUserState() } + whenever(signatures) { arrayOf(Signature(signature)) } } }