Merge "Fix for: timeout and timeout extention for verifiers." into udc-dev

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.processVerificationResponseOnTimeout(verificationId, state,
-                VerificationUtils.processVerificationResponse(verificationId, state, response,
+                        response, mPm);
-                        "Verification timed out", mPm);
 
                 break;
             }
@@ -195,8 +196,7 @@ final class PackageHandler extends Handler {
                 }
 
                 final PackageVerificationResponse response = (PackageVerificationResponse) msg.obj;
-                VerificationUtils.processVerificationResponse(verificationId, state, response,
+                VerificationUtils.processVerificationResponse(verificationId, state, response, mPm);
-                        "Install not allowed", mPm);
 
                 break;
             }

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(
+                if (state == null || !state.extendTimeout(callingUid)) {
-                        callingUid)) {
+                    // Invalid uid or already extended.
-                    // Only allow calls from required verifiers.
                     return;
                 }
 
-                state.extendTimeout();
-
                 final PackageVerificationResponse response = new PackageVerificationResponse(
                         verificationCodeAtTimeout, callingUid);
 

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() {
+    boolean extendTimeout(int uid) {
-        if (!mExtendedTimeout) {
+        if (!checkRequiredVerifierUid(uid) || timeoutExtended(uid)) {
-            mExtendedTimeout = true;
+            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() {
+    boolean timeoutExtended(int uid) {
-        return mExtendedTimeout;
+        return mExtendedTimeoutUids.get(uid, false);
     }
 
     void setIntegrityVerificationResult(int code) {

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,
+        if (pms != null && verifyingSession != null) {
-                verificationCode, null,
+            VerificationUtils.broadcastPackageVerified(verificationId, originUri,
-                verifyingSession.getDataLoaderType(), verifyingSession.getUser(),
+                    verificationCode, null,
-                pms.mContext);
+                    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(
+            if (verifyingSession != null) {
-                    PackageManager.INSTALL_FAILED_VERIFICATION_FAILURE, errorMsg);
+                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();
+        }
     }
 }

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());
+    }
 }