diff options
5 files changed, 325 insertions, 39 deletions
diff --git a/services/core/java/com/android/server/pm/PackageHandler.java b/services/core/java/com/android/server/pm/PackageHandler.java index 7f7a23419dda..83d2f6ae0e40 100644 --- a/services/core/java/com/android/server/pm/PackageHandler.java +++ b/services/core/java/com/android/server/pm/PackageHandler.java @@ -132,14 +132,15 @@ final class PackageHandler extends Handler { // Not found or complete. break; } - if (!streaming && state.timeoutExtended()) { + + final PackageVerificationResponse response = (PackageVerificationResponse) msg.obj; + if (!streaming && state.timeoutExtended(response.callerUid)) { // Timeout extended. break; } - final PackageVerificationResponse response = (PackageVerificationResponse) msg.obj; - VerificationUtils.processVerificationResponse(verificationId, state, response, - "Verification timed out", mPm); + VerificationUtils.processVerificationResponseOnTimeout(verificationId, state, + response, mPm); break; } @@ -195,8 +196,7 @@ final class PackageHandler extends Handler { } final PackageVerificationResponse response = (PackageVerificationResponse) msg.obj; - VerificationUtils.processVerificationResponse(verificationId, state, response, - "Install not allowed", mPm); + VerificationUtils.processVerificationResponse(verificationId, state, response, mPm); break; } diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 6bc876037cfb..a2bb743d78fe 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -4885,14 +4885,11 @@ public class PackageManagerService implements PackageSender, TestUtilityService mHandler.post(() -> { final int id = verificationId >= 0 ? verificationId : -verificationId; final PackageVerificationState state = mPendingVerification.get(id); - if (state == null || state.timeoutExtended() || !state.checkRequiredVerifierUid( - callingUid)) { - // Only allow calls from required verifiers. + if (state == null || !state.extendTimeout(callingUid)) { + // Invalid uid or already extended. return; } - state.extendTimeout(); - final PackageVerificationResponse response = new PackageVerificationResponse( verificationCodeAtTimeout, callingUid); diff --git a/services/core/java/com/android/server/pm/PackageVerificationState.java b/services/core/java/com/android/server/pm/PackageVerificationState.java index 929bc1e0b3c4..0b6ccc41d956 100644 --- a/services/core/java/com/android/server/pm/PackageVerificationState.java +++ b/services/core/java/com/android/server/pm/PackageVerificationState.java @@ -33,6 +33,8 @@ class PackageVerificationState { private final SparseBooleanArray mRequiredVerifierUids; private final SparseBooleanArray mUnrespondedRequiredVerifierUids; + private final SparseBooleanArray mExtendedTimeoutUids; + private boolean mSufficientVerificationComplete; private boolean mSufficientVerificationPassed; @@ -41,8 +43,6 @@ class PackageVerificationState { private boolean mRequiredVerificationPassed; - private boolean mExtendedTimeout; - private boolean mIntegrityVerificationComplete; /** @@ -54,9 +54,9 @@ class PackageVerificationState { mSufficientVerifierUids = new SparseBooleanArray(); mRequiredVerifierUids = new SparseBooleanArray(); mUnrespondedRequiredVerifierUids = new SparseBooleanArray(); + mExtendedTimeoutUids = new SparseBooleanArray(); mRequiredVerificationComplete = false; mRequiredVerificationPassed = true; - mExtendedTimeout = false; } VerifyingSession getVerifyingSession() { @@ -88,14 +88,27 @@ class PackageVerificationState { return mSufficientVerifierUids.get(uid, false); } + void setVerifierResponseOnTimeout(int uid, int code) { + if (!checkRequiredVerifierUid(uid)) { + return; + } + + // Timeout, not waiting for the sufficient verifiers anymore. + mSufficientVerifierUids.clear(); + + // Only if unresponded. + if (mUnrespondedRequiredVerifierUids.get(uid, false)) { + setVerifierResponse(uid, code); + } + } + /** * Should be called when a verification is received from an agent so the state of the package * verification can be tracked. * * @param uid user ID of the verifying agent - * @return {@code true} if the verifying agent actually exists in our list */ - boolean setVerifierResponse(int uid, int code) { + void setVerifierResponse(int uid, int code) { if (mRequiredVerifierUids.get(uid)) { switch (code) { case PackageManager.VERIFICATION_ALLOW_WITHOUT_SUFFICIENT: @@ -109,13 +122,19 @@ class PackageVerificationState { break; default: mRequiredVerificationPassed = false; + // Required verifier rejected, no need to wait for the rest. + mUnrespondedRequiredVerifierUids.clear(); + mSufficientVerifierUids.clear(); + mExtendedTimeoutUids.clear(); } + // Responded, no need to extend timeout. + mExtendedTimeoutUids.delete(uid); + mUnrespondedRequiredVerifierUids.delete(uid); if (mUnrespondedRequiredVerifierUids.size() == 0) { mRequiredVerificationComplete = true; } - return true; } else if (mSufficientVerifierUids.get(uid)) { if (code == PackageManager.VERIFICATION_ALLOW) { mSufficientVerificationPassed = true; @@ -126,11 +145,7 @@ class PackageVerificationState { if (mSufficientVerifierUids.size() == 0) { mSufficientVerificationComplete = true; } - - return true; } - - return false; } /** @@ -181,10 +196,12 @@ class PackageVerificationState { } /** Extend the timeout for this Package to be verified. */ - void extendTimeout() { - if (!mExtendedTimeout) { - mExtendedTimeout = true; + boolean extendTimeout(int uid) { + if (!checkRequiredVerifierUid(uid) || timeoutExtended(uid)) { + return false; } + mExtendedTimeoutUids.append(uid, true); + return true; } /** @@ -192,8 +209,8 @@ class PackageVerificationState { * * @return {@code true} if a timeout was already extended. */ - boolean timeoutExtended() { - return mExtendedTimeout; + boolean timeoutExtended(int uid) { + return mExtendedTimeoutUids.get(uid, false); } void setIntegrityVerificationResult(int code) { diff --git a/services/core/java/com/android/server/pm/VerificationUtils.java b/services/core/java/com/android/server/pm/VerificationUtils.java index 30f2132ce1f1..f0610180040c 100644 --- a/services/core/java/com/android/server/pm/VerificationUtils.java +++ b/services/core/java/com/android/server/pm/VerificationUtils.java @@ -18,6 +18,7 @@ package com.android.server.pm; import static android.os.Trace.TRACE_TAG_PACKAGE_MANAGER; +import static com.android.internal.annotations.VisibleForTesting.Visibility.PACKAGE; import static com.android.server.pm.PackageManagerService.PACKAGE_MIME_TYPE; import static com.android.server.pm.PackageManagerService.TAG; @@ -32,6 +33,8 @@ import android.os.UserHandle; import android.provider.Settings; import android.util.Slog; +import com.android.internal.annotations.VisibleForTesting; + final class VerificationUtils { /** * The default maximum time to wait for the verification agent to return in @@ -97,39 +100,63 @@ final class VerificationUtils { android.Manifest.permission.PACKAGE_VERIFICATION_AGENT); } + @VisibleForTesting(visibility = PACKAGE) + static void processVerificationResponseOnTimeout(int verificationId, + PackageVerificationState state, PackageVerificationResponse response, + PackageManagerService pms) { + state.setVerifierResponseOnTimeout(response.callerUid, response.code); + processVerificationResponse(verificationId, state, response.code, "Verification timed out", + pms); + } + + @VisibleForTesting(visibility = PACKAGE) static void processVerificationResponse(int verificationId, PackageVerificationState state, - PackageVerificationResponse response, String failureReason, PackageManagerService pms) { + PackageVerificationResponse response, PackageManagerService pms) { state.setVerifierResponse(response.callerUid, response.code); + processVerificationResponse(verificationId, state, response.code, "Install not allowed", + pms); + } + + private static void processVerificationResponse(int verificationId, + PackageVerificationState state, int verificationResult, String failureReason, + PackageManagerService pms) { if (!state.isVerificationComplete()) { return; } final VerifyingSession verifyingSession = state.getVerifyingSession(); - final Uri originUri = Uri.fromFile(verifyingSession.mOriginInfo.mResolvedFile); + final Uri originUri = verifyingSession != null ? Uri.fromFile( + verifyingSession.mOriginInfo.mResolvedFile) : null; final int verificationCode = - state.isInstallAllowed() ? response.code : PackageManager.VERIFICATION_REJECT; + state.isInstallAllowed() ? verificationResult : PackageManager.VERIFICATION_REJECT; - VerificationUtils.broadcastPackageVerified(verificationId, originUri, - verificationCode, null, - verifyingSession.getDataLoaderType(), verifyingSession.getUser(), - pms.mContext); + if (pms != null && verifyingSession != null) { + VerificationUtils.broadcastPackageVerified(verificationId, originUri, + verificationCode, null, + verifyingSession.getDataLoaderType(), verifyingSession.getUser(), + pms.mContext); + } if (state.isInstallAllowed()) { Slog.i(TAG, "Continuing with installation of " + originUri); } else { String errorMsg = failureReason + " for " + originUri; Slog.i(TAG, errorMsg); - verifyingSession.setReturnCode( - PackageManager.INSTALL_FAILED_VERIFICATION_FAILURE, errorMsg); + if (verifyingSession != null) { + verifyingSession.setReturnCode( + PackageManager.INSTALL_FAILED_VERIFICATION_FAILURE, errorMsg); + } } - if (state.areAllVerificationsComplete()) { + if (pms != null && state.areAllVerificationsComplete()) { pms.mPendingVerification.remove(verificationId); } Trace.asyncTraceEnd(TRACE_TAG_PACKAGE_MANAGER, "verification", verificationId); - verifyingSession.handleVerificationFinished(); + if (verifyingSession != null) { + verifyingSession.handleVerificationFinished(); + } } } diff --git a/services/tests/PackageManagerServiceTests/server/src/com/android/server/pm/PackageVerificationStateTest.java b/services/tests/PackageManagerServiceTests/server/src/com/android/server/pm/PackageVerificationStateTest.java index 8715afda5cee..a93e8ad93756 100644 --- a/services/tests/PackageManagerServiceTests/server/src/com/android/server/pm/PackageVerificationStateTest.java +++ b/services/tests/PackageManagerServiceTests/server/src/com/android/server/pm/PackageVerificationStateTest.java @@ -95,9 +95,13 @@ public class PackageVerificationStateTest extends AndroidTestCase { state.setVerifierResponse(REQUIRED_UID_1, PackageManager.VERIFICATION_REJECT); - assertFalse("Verification should not be marked as complete yet", + assertTrue("Verification should be considered complete now", state.isVerificationComplete()); + assertFalse("Installation should be marked as denied", + state.isInstallAllowed()); + + // Nothing changes. state.setVerifierResponse(REQUIRED_UID_2, PackageManager.VERIFICATION_REJECT); assertTrue("Verification should be considered complete now", @@ -117,9 +121,13 @@ public class PackageVerificationStateTest extends AndroidTestCase { state.setVerifierResponse(REQUIRED_UID_1, PackageManager.VERIFICATION_REJECT); - assertFalse("Verification should not be marked as complete yet", + assertTrue("Verification should be considered complete now", state.isVerificationComplete()); + assertFalse("Installation should be marked as denied", + state.isInstallAllowed()); + + // Nothing changes. state.setVerifierResponse(REQUIRED_UID_2, PackageManager.VERIFICATION_ALLOW); assertTrue("Verification should be considered complete now", @@ -151,6 +159,162 @@ public class PackageVerificationStateTest extends AndroidTestCase { state.isInstallAllowed()); } + public void testPackageVerificationState_TwoRequiredVerifiers_SecondTimesOut_DefaultAllow() { + PackageVerificationState state = new PackageVerificationState(null); + state.addRequiredVerifierUid(REQUIRED_UID_1); + state.addRequiredVerifierUid(REQUIRED_UID_2); + + state.addSufficientVerifier(SUFFICIENT_UID_1); + + assertFalse("Verification should not be marked as complete yet", + state.isVerificationComplete()); + + state.setVerifierResponse(REQUIRED_UID_1, PackageManager.VERIFICATION_ALLOW); + + assertFalse("Verification should not be marked as complete yet", + state.isVerificationComplete()); + + // Timeout with default ALLOW. + processOnTimeout(state, PackageManager.VERIFICATION_ALLOW, REQUIRED_UID_2, true); + } + + public void testPackageVerificationState_TwoRequiredVerifiers_SecondTimesOut_DefaultReject() { + PackageVerificationState state = new PackageVerificationState(null); + state.addRequiredVerifierUid(REQUIRED_UID_1); + state.addRequiredVerifierUid(REQUIRED_UID_2); + + state.addSufficientVerifier(SUFFICIENT_UID_1); + + assertFalse("Verification should not be marked as complete yet", + state.isVerificationComplete()); + + state.setVerifierResponse(REQUIRED_UID_1, PackageManager.VERIFICATION_ALLOW); + + assertFalse("Verification should not be marked as complete yet", + state.isVerificationComplete()); + + // Timeout with default REJECT. + processOnTimeout(state, PackageManager.VERIFICATION_REJECT, REQUIRED_UID_2, false); + } + + public void testPackageVerificationState_TwoRequiredVerifiers_FirstTimesOut_DefaultAllow() { + PackageVerificationState state = new PackageVerificationState(null); + state.addRequiredVerifierUid(REQUIRED_UID_1); + state.addRequiredVerifierUid(REQUIRED_UID_2); + + state.addSufficientVerifier(SUFFICIENT_UID_1); + + assertFalse("Verification should not be marked as complete yet", + state.isVerificationComplete()); + + // Timeout with default ALLOW. + processOnTimeout(state, PackageManager.VERIFICATION_ALLOW, REQUIRED_UID_1); + + assertFalse("Verification should not be marked as complete yet", + state.isVerificationComplete()); + + state.setVerifierResponse(REQUIRED_UID_2, PackageManager.VERIFICATION_ALLOW); + + assertTrue("Verification should be considered complete now", + state.isVerificationComplete()); + + assertTrue("Installation should be marked as allowed", + state.isInstallAllowed()); + } + + public void testPackageVerificationState_TwoRequiredVerifiers_FirstTimesOut_DefaultReject() { + PackageVerificationState state = new PackageVerificationState(null); + state.addRequiredVerifierUid(REQUIRED_UID_1); + state.addRequiredVerifierUid(REQUIRED_UID_2); + + state.addSufficientVerifier(SUFFICIENT_UID_1); + + assertFalse("Verification should not be marked as complete yet", + state.isVerificationComplete()); + + // Timeout with default REJECT. + processOnTimeout(state, PackageManager.VERIFICATION_REJECT, REQUIRED_UID_1); + + assertTrue("Verification should be considered complete now", + state.isVerificationComplete()); + + assertFalse("Installation should be marked as denied", + state.isInstallAllowed()); + + // Nothing changes. + state.setVerifierResponse(REQUIRED_UID_2, PackageManager.VERIFICATION_ALLOW); + + assertTrue("Verification should be considered complete now", + state.isVerificationComplete()); + + assertFalse("Installation should be marked as denied", + state.isInstallAllowed()); + } + + public void testPackageVerificationState_TwoRequiredVerifiers_FirstTimesOut_SecondExtends_DefaultAllow() { + PackageVerificationState state = new PackageVerificationState(null); + state.addRequiredVerifierUid(REQUIRED_UID_1); + state.addRequiredVerifierUid(REQUIRED_UID_2); + + state.addSufficientVerifier(SUFFICIENT_UID_1); + + assertFalse("Verification should not be marked as complete yet", + state.isVerificationComplete()); + + state.extendTimeout(REQUIRED_UID_2); + + // Timeout with default ALLOW. + processOnTimeout(state, PackageManager.VERIFICATION_ALLOW, REQUIRED_UID_1); + + assertFalse("Verification should not be marked as complete yet", + state.isVerificationComplete()); + + assertTrue("Timeout is extended", + state.timeoutExtended(REQUIRED_UID_2)); + + state.setVerifierResponse(REQUIRED_UID_2, PackageManager.VERIFICATION_ALLOW); + + assertTrue("Verification should be considered complete now", + state.isVerificationComplete()); + + assertTrue("Installation should be marked as allowed", + state.isInstallAllowed()); + } + + public void testPackageVerificationState_TwoRequiredVerifiers_FirstTimesOut_SecondExtends_DefaultReject() { + PackageVerificationState state = new PackageVerificationState(null); + state.addRequiredVerifierUid(REQUIRED_UID_1); + state.addRequiredVerifierUid(REQUIRED_UID_2); + + state.addSufficientVerifier(SUFFICIENT_UID_1); + + assertFalse("Verification should not be marked as complete yet", + state.isVerificationComplete()); + + state.extendTimeout(REQUIRED_UID_2); + + // Timeout with default REJECT. + processOnTimeout(state, PackageManager.VERIFICATION_REJECT, REQUIRED_UID_1); + + assertFalse("Timeout should not be extended for this verifier", + state.timeoutExtended(REQUIRED_UID_2)); + + assertTrue("Verification should be considered complete now", + state.isVerificationComplete()); + + assertFalse("Installation should be marked as denied", + state.isInstallAllowed()); + + // Nothing changes. + state.setVerifierResponse(REQUIRED_UID_2, PackageManager.VERIFICATION_ALLOW); + + assertTrue("Verification should be considered complete now", + state.isVerificationComplete()); + + assertFalse("Installation should be marked as denied", + state.isInstallAllowed()); + } + public void testPackageVerificationState_RequiredAndOneSufficient_RequiredDeniedInstall() { PackageVerificationState state = new PackageVerificationState(null); state.addRequiredVerifierUid(REQUIRED_UID_1); @@ -231,6 +395,66 @@ public class PackageVerificationStateTest extends AndroidTestCase { state.isInstallAllowed()); } + public void testPackageVerificationState_RequiredAllow_SufficientTimesOut_DefaultAllow() { + PackageVerificationState state = new PackageVerificationState(null); + state.addRequiredVerifierUid(REQUIRED_UID_1); + + assertFalse("Verification should not be marked as complete yet", + state.isVerificationComplete()); + + state.addSufficientVerifier(SUFFICIENT_UID_1); + + assertFalse("Verification should not be marked as complete yet", + state.isVerificationComplete()); + + // Required allows. + state.setVerifierResponse(REQUIRED_UID_1, PackageManager.VERIFICATION_ALLOW); + + // Timeout with default ALLOW. + processOnTimeout(state, PackageManager.VERIFICATION_ALLOW, REQUIRED_UID_1, true); + } + + public void testPackageVerificationState_RequiredExtendAllow_SufficientTimesOut_DefaultAllow() { + PackageVerificationState state = new PackageVerificationState(null); + state.addRequiredVerifierUid(REQUIRED_UID_1); + + assertFalse("Verification should not be marked as complete yet", + state.isVerificationComplete()); + + state.addSufficientVerifier(SUFFICIENT_UID_1); + + assertFalse("Verification should not be marked as complete yet", + state.isVerificationComplete()); + + // Extend first. + state.extendTimeout(REQUIRED_UID_1); + + // Required allows. + state.setVerifierResponse(REQUIRED_UID_1, PackageManager.VERIFICATION_ALLOW); + + // Timeout with default ALLOW. + processOnTimeout(state, PackageManager.VERIFICATION_ALLOW, REQUIRED_UID_1, true); + } + + public void testPackageVerificationState_RequiredAllow_SufficientTimesOut_DefaultReject() { + PackageVerificationState state = new PackageVerificationState(null); + state.addRequiredVerifierUid(REQUIRED_UID_1); + + assertFalse("Verification should not be marked as complete yet", + state.isVerificationComplete()); + + state.addSufficientVerifier(SUFFICIENT_UID_1); + + assertFalse("Verification should not be marked as complete yet", + state.isVerificationComplete()); + + // Required allows. + state.setVerifierResponse(REQUIRED_UID_1, PackageManager.VERIFICATION_ALLOW); + + // Timeout with default REJECT. + processOnTimeout(state, PackageManager.VERIFICATION_REJECT, REQUIRED_UID_1, true); + } + public void testPackageVerificationState_RequiredAndTwoSufficient_OneSufficientIsEnough() { PackageVerificationState state = new PackageVerificationState(null); state.addRequiredVerifierUid(REQUIRED_UID_1); @@ -400,4 +624,25 @@ public class PackageVerificationStateTest extends AndroidTestCase { assertFalse(state.areAllVerificationsComplete()); } + + private void processOnTimeout(PackageVerificationState state, int code, int uid) { + // CHECK_PENDING_VERIFICATION handler. + assertFalse("Verification should not be marked as complete yet", + state.isVerificationComplete()); + assertFalse("Timeout should not be extended for this verifier", + state.timeoutExtended(uid)); + + PackageVerificationResponse response = new PackageVerificationResponse(code, uid); + VerificationUtils.processVerificationResponseOnTimeout(-1, state, response, null); + } + + private void processOnTimeout(PackageVerificationState state, int code, int uid, + boolean expectAllow) { + processOnTimeout(state, code, uid); + + assertTrue("Verification should be considered complete now", + state.isVerificationComplete()); + assertEquals("Installation should be marked as " + (expectAllow ? "allowed" : "rejected"), + expectAllow, state.isInstallAllowed()); + } } |