Unify package verification processing.

Change isInstallAllowed logic to be more clear around when it returns
true.
This also fixes that we used to never send VERIFICATION_REJECT broadcast for a non-timeout case.

Bug: 171907685
Test: atest PackageManagerShellCommandTest
Change-Id: I2356a60f4b0da22036473d7fab982b3d2a295a78

services/core/java/com/android/server/pm/PackageHandler.java

@@ -146,37 +146,9 @@ final class PackageHandler extends Handler {
                 }
 
                 final PackageVerificationResponse response = (PackageVerificationResponse) msg.obj;
+                VerificationUtils.processVerificationResponse(verificationId, state, response,
+                        "Verification timed out", mPm);
 
-                final VerifyingSession verifyingSession = state.getVerifyingSession();
-                final Uri originUri = Uri.fromFile(verifyingSession.mOriginInfo.mResolvedFile);
-
-                String errorMsg = "Verification timed out for " + originUri;
-                Slog.i(TAG, errorMsg);
-
-                final UserHandle user = verifyingSession.getUser();
-                if (response.code != PackageManager.VERIFICATION_REJECT) {
-                    Slog.i(TAG, "Continuing with installation of " + originUri);
-                    state.setVerifierResponse(response.callerUid, response.code);
-                    VerificationUtils.broadcastPackageVerified(verificationId, originUri,
-                            PackageManager.VERIFICATION_ALLOW, null,
-                            verifyingSession.mDataLoaderType, user, mPm.mContext);
-                } else {
-                    VerificationUtils.broadcastPackageVerified(verificationId, originUri,
-                            PackageManager.VERIFICATION_REJECT, null,
-                            verifyingSession.mDataLoaderType, user, mPm.mContext);
-                    verifyingSession.setReturnCode(
-                            PackageManager.INSTALL_FAILED_VERIFICATION_FAILURE, errorMsg);
-                    state.setVerifierResponse(response.callerUid, response.code);
-                }
-
-                if (state.areAllVerificationsComplete()) {
-                    mPm.mPendingVerification.remove(verificationId);
-                }
-
-                Trace.asyncTraceEnd(
-                        TRACE_TAG_PACKAGE_MANAGER, "verification", verificationId);
-
-                verifyingSession.handleVerificationFinished();
                 break;
             }
             case CHECK_PENDING_INTEGRITY_VERIFICATION: {
@@ -231,31 +203,8 @@ final class PackageHandler extends Handler {
                 }
 
                 final PackageVerificationResponse response = (PackageVerificationResponse) msg.obj;
-                state.setVerifierResponse(response.callerUid, response.code);
-
-                if (state.isVerificationComplete()) {
-                    final VerifyingSession verifyingSession = state.getVerifyingSession();
-                    final Uri originUri = Uri.fromFile(verifyingSession.mOriginInfo.mResolvedFile);
-
-                    if (state.isInstallAllowed()) {
-                        VerificationUtils.broadcastPackageVerified(verificationId, originUri,
-                                response.code, null, verifyingSession.mDataLoaderType,
-                                verifyingSession.getUser(), mPm.mContext);
-                    } else {
-                        verifyingSession.setReturnCode(
-                                PackageManager.INSTALL_FAILED_VERIFICATION_FAILURE,
-                                "Install not allowed");
-                    }
-
-                    if (state.areAllVerificationsComplete()) {
-                        mPm.mPendingVerification.remove(verificationId);
-                    }
-
-                    Trace.asyncTraceEnd(
-                            TRACE_TAG_PACKAGE_MANAGER, "verification", verificationId);
-
-                    verifyingSession.handleVerificationFinished();
-                }
+                VerificationUtils.processVerificationResponse(verificationId, state, response,
+                        "Install not allowed", mPm);
 
                 break;
             }

services/core/java/com/android/server/pm/PackageVerificationState.java

@@ -137,15 +137,15 @@ class PackageVerificationState {
      * @return {@code true} if installation should be allowed
      */
     boolean isInstallAllowed() {
-        if (!mRequiredVerificationPassed) {
+        if (!mRequiredVerificationComplete) {
             return false;
         }
 
         if (mSufficientVerificationComplete) {
-            return mSufficientVerificationPassed;
+            return mRequiredVerificationPassed && mSufficientVerificationPassed;
         }
 
-        return true;
+        return mRequiredVerificationPassed;
     }
 
     /** Extend the timeout for this Package to be verified. */

services/core/java/com/android/server/pm/VerificationUtils.java

@@ -16,7 +16,10 @@
 
 package com.android.server.pm;
 
+import static android.os.Trace.TRACE_TAG_PACKAGE_MANAGER;
+
 import static com.android.server.pm.PackageManagerService.PACKAGE_MIME_TYPE;
+import static com.android.server.pm.PackageManagerService.TAG;
 
 import android.annotation.Nullable;
 import android.content.Context;
@@ -24,8 +27,10 @@ import android.content.Intent;
 import android.content.pm.PackageInstaller;
 import android.content.pm.PackageManager;
 import android.net.Uri;
+import android.os.Trace;
 import android.os.UserHandle;
 import android.provider.Settings;
+import android.util.Slog;
 
 final class VerificationUtils {
     /**
@@ -91,4 +96,40 @@ final class VerificationUtils {
         context.sendBroadcastAsUser(intent, user,
                 android.Manifest.permission.PACKAGE_VERIFICATION_AGENT);
     }
+
+    static void processVerificationResponse(int verificationId, PackageVerificationState state,
+            PackageVerificationResponse response, String failureReason, PackageManagerService pms) {
+        state.setVerifierResponse(response.callerUid, response.code);
+        if (!state.isVerificationComplete()) {
+            return;
+        }
+
+        final VerifyingSession verifyingSession = state.getVerifyingSession();
+        final Uri originUri = Uri.fromFile(verifyingSession.mOriginInfo.mResolvedFile);
+
+        final int verificationCode =
+                state.isInstallAllowed() ? response.code : PackageManager.VERIFICATION_REJECT;
+
+        VerificationUtils.broadcastPackageVerified(verificationId, originUri,
+                verificationCode, null,
+                verifyingSession.mDataLoaderType, 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 (state.areAllVerificationsComplete()) {
+            pms.mPendingVerification.remove(verificationId);
+        }
+
+        Trace.asyncTraceEnd(TRACE_TAG_PACKAGE_MANAGER, "verification", verificationId);
+
+        verifyingSession.handleVerificationFinished();
+    }
 }

services/tests/servicestests/src/com/android/server/pm/PackageVerificationStateTest.java

@@ -109,7 +109,7 @@ public class PackageVerificationStateTest extends AndroidTestCase {
         assertTrue("Verification should be considered complete now",
                 state.isVerificationComplete());
 
-        assertFalse("Installation should be marked as allowed",
+        assertFalse("Installation should be marked as rejected",
                 state.isInstallAllowed());
     }