From 32796cd24bf5b392d5e823d4c6abc4e2f1dfe4a2 Mon Sep 17 00:00:00 2001 From: Winson Chiu Date: Tue, 14 Dec 2021 18:58:30 +0000 Subject: [PATCH] Verify querent for printing app links state Verifies that printing domain verification state asserts that the caller is an approved querent, to match the getDomainVerificationInfo Java API. Also allows callers with the DUMP permission to be a querent, so that dumpsys package still works. Bug: 200035185 Test: atest DomainVerificationEnforcerTest Merged-In: I9ad409ba15ce1a358ca8c0e047444a2fa6d2140b Change-Id: I9ad409ba15ce1a358ca8c0e047444a2fa6d2140b (cherry picked from commit e466493f72c05a607fc3de1fac9f8d943e5f6729) --- .../server/pm/PackageManagerService.java | 2 +- .../domain/DomainVerificationEnforcer.java | 7 +++-- .../domain/DomainVerificationService.java | 1 + .../domain/DomainVerificationEnforcerTest.kt | 29 +++++++++++++++++-- 4 files changed, 33 insertions(+), 6 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 7a70bfe912486..c0486443b97ec 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -4792,7 +4792,7 @@ public class PackageManagerService extends IPackageManager.Stub try { mDomainVerificationManager.printState(writer, packageName, UserHandle.USER_ALL, mSettings::getPackageLPr); - } catch (PackageManager.NameNotFoundException e) { + } catch (Exception e) { pw.println("Failure printing domain verification information"); Slog.e(TAG, "Failure printing domain verification information", e); } 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 0b48b5c6dd705..2208e3c02c4d1 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 @@ -70,8 +70,11 @@ public class DomainVerificationEnforcer { break; default: if (!proxy.isCallerVerifier(callingUid)) { - throw new SecurityException( - "Caller is not allowed to query domain verification state"); + mContext.enforcePermission(android.Manifest.permission.DUMP, + Binder.getCallingPid(), callingUid, + "Caller " + callingUid + + " is not allowed to query domain verification state"); + break; } mContext.enforcePermission(android.Manifest.permission.QUERY_ALL_PACKAGES, 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 4f190093e0d04..2270df3752452 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 @@ -1193,6 +1193,7 @@ public class DomainVerificationService extends SystemService @Nullable @UserIdInt Integer userId, @NonNull Function pkgSettingFunction) throws NameNotFoundException { + mEnforcer.assertApprovedQuerent(mConnection.getCallingUid(), mProxy); synchronized (mLock) { mDebug.printState(writer, packageName, userId, pkgSettingFunction, mAttachedPkgStates); } 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 886b2e03a1042..347952bcebb02 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 @@ -19,6 +19,7 @@ package com.android.server.pm.test.verify.domain import android.content.Context import android.content.Intent import android.content.pm.PackageManager +import android.content.pm.PackageParser.SigningDetails import android.content.pm.PackageUserState import android.content.pm.parsing.component.ParsedActivity import android.content.pm.parsing.component.ParsedIntentInfo @@ -27,6 +28,7 @@ import android.content.pm.verify.domain.DomainVerificationState import android.os.Build import android.os.Process import android.util.ArraySet +import android.util.IndentingPrintWriter import android.util.SparseArray import androidx.test.platform.app.InstrumentationRegistry import com.android.server.pm.PackageSetting @@ -47,6 +49,7 @@ import org.mockito.Mockito.anyInt import org.mockito.Mockito.anyLong import org.mockito.Mockito.anyString import org.mockito.Mockito.eq +import org.mockito.Mockito.mock import org.mockito.Mockito.verifyNoMoreInteractions import java.io.File import java.util.UUID @@ -206,6 +209,14 @@ class DomainVerificationEnforcerTest { service(Type.QUERENT, "getInfo") { getDomainVerificationInfo(it.targetPackageName) }, + service(Type.QUERENT, "printState") { + printState(mock(IndentingPrintWriter::class.java), null, null) + }, + service(Type.QUERENT, "printStateInternal") { + printState(mock(IndentingPrintWriter::class.java), null, null) { + mockPkgSetting(it, UUID.randomUUID()) + } + }, service(Type.VERIFIER, "setStatus") { setDomainVerificationStatus( it.targetDomainSetId, @@ -311,6 +322,7 @@ class DomainVerificationEnforcerTest { } ) } + whenever(signingDetails) { SigningDetails.UNKNOWN } } fun mockPkgSetting(packageName: String, domainSetId: UUID) = spyThrowOnUnmocked( @@ -339,6 +351,7 @@ class DomainVerificationEnforcerTest { whenever(readUserState(1)) { PackageUserState() } whenever(getInstantApp(anyInt())) { false } whenever(isSystem()) { false } + whenever(signingDetails) { SigningDetails.UNKNOWN } } } @@ -385,6 +398,7 @@ class DomainVerificationEnforcerTest { val allowUserState = AtomicBoolean(false) val allowPreferredApps = AtomicBoolean(false) val allowQueryAll = AtomicBoolean(false) + val allowDump = AtomicBoolean(false) val context: Context = mockThrowOnUnmocked { initPermission( allowUserState, @@ -395,6 +409,7 @@ class DomainVerificationEnforcerTest { android.Manifest.permission.SET_PREFERRED_APPLICATIONS ) initPermission(allowQueryAll, android.Manifest.permission.QUERY_ALL_PACKAGES) + initPermission(allowDump, android.Manifest.permission.DUMP) } val target = params.construct(context) @@ -421,6 +436,10 @@ class DomainVerificationEnforcerTest { allowQueryAll.set(true) assertFails { runMethod(target, NON_VERIFIER_UID) } + + allowDump.set(true) + + runMethod(target, NON_VERIFIER_UID) } private fun approvedVerifier() { @@ -806,8 +825,12 @@ class DomainVerificationEnforcerTest { } val valueAsInt = value as? Int - if (valueAsInt != null && valueAsInt == DomainVerificationManager.STATUS_OK) { - throw AssertionError("Expected call to return false, was $value") + if (valueAsInt != null) { + if (valueAsInt == DomainVerificationManager.STATUS_OK) { + throw AssertionError("Expected call to return false, was $value") + } + } else { + throw AssertionError("Expected call to fail") } } catch (e: SecurityException) { } catch (e: PackageManager.NameNotFoundException) { @@ -819,7 +842,7 @@ class DomainVerificationEnforcerTest { // System/shell only INTERNAL, - // INTERNAL || non-legacy domain verification agent + // INTERNAL || non-legacy domain verification agent || DUMP permission QUERENT, // INTERNAL || domain verification agent