Do not dismiss keyguard after SIM PUK unlock After PUK unlock, multiple calls to KeyguardSecurityContainerController#dismiss() were being called from the KeyguardSimPukViewController, which begins the transition to the next security screen, if any. At the same time, other parts of the system, also listening to SIM events, recognize the PUK unlock and call KeyguardSecurityContainer#showSecurityScreen, which updates which security method comes next. After boot, this should be one of PIN, Password, Pattern, assuming they have a security method. If one of the first dismiss() calls comes AFTER the security method changes, this is incorrectly recognized by the code as a successful PIN/pattern/password unlock. This causes the keyguard to be marked as done, causing screen flickers and incorrect system state. The solution: every call to dismiss() should include a new parameter for the security method used. If there is a difference between this parameter and the current value in KeyguardSecurityContainerCallback, ignore the request, as the system state has changed. Fixes: 238804980 Bug: 218500036 Test: atest KeyguardSecurityContainerTest AdminSecondaryLockScreenControllerTest KeyguardHostViewControllerTest KeyguardSecurityContainerControllerTest Change-Id: I7c8714a177bc85fbce92f6e8fe911f74ca2ac243 Merged-In: I7c8714a177bc85fbce92f6e8fe911f74ca2ac243
diff --git a/packages/SystemUI/src/com/android/keyguard/AdminSecondaryLockScreenController.java b/packages/SystemUI/src/com/android/keyguard/AdminSecondaryLockScreenController.java index 23195af..00f1c01 100644 --- a/packages/SystemUI/src/com/android/keyguard/AdminSecondaryLockScreenController.java +++ b/packages/SystemUI/src/com/android/keyguard/AdminSecondaryLockScreenController.java
@@ -33,6 +33,7 @@ import android.view.ViewGroup; import com.android.internal.annotations.VisibleForTesting; +import com.android.keyguard.KeyguardSecurityModel.SecurityMode; import com.android.keyguard.dagger.KeyguardBouncerScope; import com.android.systemui.dagger.qualifiers.Main; @@ -208,7 +209,7 @@ hide(); if (mKeyguardCallback != null) { mKeyguardCallback.dismiss(/* securityVerified= */ true, userId, - /* bypassSecondaryLockScreen= */true); + /* bypassSecondaryLockScreen= */true, SecurityMode.Invalid); } } }
diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardAbsKeyInputViewController.java b/packages/SystemUI/src/com/android/keyguard/KeyguardAbsKeyInputViewController.java index eb418ff..b8fcb10 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardAbsKeyInputViewController.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardAbsKeyInputViewController.java
@@ -179,7 +179,7 @@ if (dismissKeyguard) { mDismissing = true; mLatencyTracker.onActionStart(LatencyTracker.ACTION_LOCKSCREEN_UNLOCK); - getKeyguardSecurityCallback().dismiss(true, userId); + getKeyguardSecurityCallback().dismiss(true, userId, getSecurityMode()); } } else { if (isValidPassword) {
diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardHostViewController.java b/packages/SystemUI/src/com/android/keyguard/KeyguardHostViewController.java index d32219a..db64f05 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardHostViewController.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardHostViewController.java
@@ -90,7 +90,7 @@ Log.i(TAG, "TrustAgent dismissed Keyguard."); } mSecurityCallback.dismiss(false /* authenticated */, userId, - /* bypassSecondaryLockScreen */ false); + /* bypassSecondaryLockScreen */ false, SecurityMode.Invalid); } else { mViewMediatorCallback.playTrustedSound(); } @@ -102,9 +102,9 @@ @Override public boolean dismiss(boolean authenticated, int targetUserId, - boolean bypassSecondaryLockScreen) { + boolean bypassSecondaryLockScreen, SecurityMode expectedSecurityMode) { return mKeyguardSecurityContainerController.showNextSecurityScreenOrFinish( - authenticated, targetUserId, bypassSecondaryLockScreen); + authenticated, targetUserId, bypassSecondaryLockScreen, expectedSecurityMode); } @Override @@ -212,7 +212,8 @@ * @return True if the keyguard is done. */ public boolean dismiss(int targetUserId) { - return mSecurityCallback.dismiss(false, targetUserId, false); + return mSecurityCallback.dismiss(false, targetUserId, false, + getCurrentSecurityMode()); } /** @@ -360,10 +361,10 @@ } public boolean handleBackKey() { - if (mKeyguardSecurityContainerController.getCurrentSecurityMode() - != SecurityMode.None) { + SecurityMode securityMode = mKeyguardSecurityContainerController.getCurrentSecurityMode(); + if (securityMode != SecurityMode.None) { mKeyguardSecurityContainerController.dismiss( - false, KeyguardUpdateMonitor.getCurrentUser()); + false, KeyguardUpdateMonitor.getCurrentUser(), securityMode); return true; } return false;
diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardInputViewController.java b/packages/SystemUI/src/com/android/keyguard/KeyguardInputViewController.java index 98ac640..87300c3 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardInputViewController.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardInputViewController.java
@@ -59,10 +59,11 @@ return false; } @Override - public void dismiss(boolean securityVerified, int targetUserId) { } + public void dismiss(boolean securityVerified, int targetUserId, + SecurityMode expectedSecurityMode) { } @Override public void dismiss(boolean authenticated, int targetId, - boolean bypassSecondaryLockScreen) { } + boolean bypassSecondaryLockScreen, SecurityMode expectedSecurityMode) { } @Override public void onUserInput() { } @Override
diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardPatternViewController.java b/packages/SystemUI/src/com/android/keyguard/KeyguardPatternViewController.java index b97d9671..9aa6f03 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardPatternViewController.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardPatternViewController.java
@@ -171,7 +171,7 @@ if (dismissKeyguard) { mLockPatternView.setDisplayMode(LockPatternView.DisplayMode.Correct); mLatencyTracker.onActionStart(LatencyTracker.ACTION_LOCKSCREEN_UNLOCK); - getKeyguardSecurityCallback().dismiss(true, userId); + getKeyguardSecurityCallback().dismiss(true, userId, SecurityMode.Pattern); } } else { mLockPatternView.setDisplayMode(LockPatternView.DisplayMode.Wrong);
diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityCallback.java b/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityCallback.java index e384727..bc72f79 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityCallback.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityCallback.java
@@ -15,14 +15,17 @@ */ package com.android.keyguard; +import com.android.keyguard.KeyguardSecurityModel.SecurityMode; + public interface KeyguardSecurityCallback { /** * Dismiss the given security screen. * @param securityVerified true if the user correctly entered credentials for the given screen. * @param targetUserId a user that needs to be the foreground user at the dismissal completion. + * @param expectedSecurityMode The security mode that is invoking this dismiss. */ - void dismiss(boolean securityVerified, int targetUserId); + void dismiss(boolean securityVerified, int targetUserId, SecurityMode expectedSecurityMode); /** * Dismiss the given security screen. @@ -30,8 +33,10 @@ * @param targetUserId a user that needs to be the foreground user at the dismissal completion. * @param bypassSecondaryLockScreen true if the user can bypass the secondary lock screen, * if any, during this dismissal. + * @param expectedSecurityMode The security mode that is invoking this dismiss. */ - void dismiss(boolean securityVerified, int targetUserId, boolean bypassSecondaryLockScreen); + void dismiss(boolean securityVerified, int targetUserId, boolean bypassSecondaryLockScreen, + SecurityMode expectedSecurityMode); /** * Manually report user activity to keep the device awake.
diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainer.java b/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainer.java index 8fb622a..f697e25 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainer.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainer.java
@@ -236,7 +236,12 @@ // Used to notify the container when something interesting happens. public interface SecurityCallback { - boolean dismiss(boolean authenticated, int targetUserId, boolean bypassSecondaryLockScreen); + /** + * Potentially dismiss the current security screen, after validating that all device + * security has been unlocked. Otherwise show the next screen. + */ + boolean dismiss(boolean authenticated, int targetUserId, boolean bypassSecondaryLockScreen, + SecurityMode expectedSecurityMode); void userActivity();
diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainerController.java b/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainerController.java index 0b3fe46..e1957c0 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainerController.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainerController.java
@@ -156,14 +156,17 @@ } @Override - public void dismiss(boolean authenticated, int targetId) { - dismiss(authenticated, targetId, /* bypassSecondaryLockScreen */ false); + public void dismiss(boolean authenticated, int targetId, + SecurityMode expectedSecurityMode) { + dismiss(authenticated, targetId, /* bypassSecondaryLockScreen */ false, + expectedSecurityMode); } @Override public void dismiss(boolean authenticated, int targetId, - boolean bypassSecondaryLockScreen) { - mSecurityCallback.dismiss(authenticated, targetId, bypassSecondaryLockScreen); + boolean bypassSecondaryLockScreen, SecurityMode expectedSecurityMode) { + mSecurityCallback.dismiss(authenticated, targetId, bypassSecondaryLockScreen, + expectedSecurityMode); } public boolean isVerifyUnlockOnly() { @@ -385,8 +388,13 @@ return mCurrentSecurityMode; } - public void dismiss(boolean authenticated, int targetUserId) { - mKeyguardSecurityCallback.dismiss(authenticated, targetUserId); + /** + * Potentially dismiss the current security screen, after validating that all device + * security has been unlocked. Otherwise show the next screen. + */ + public void dismiss(boolean authenticated, int targetUserId, + SecurityMode expectedSecurityMode) { + mKeyguardSecurityCallback.dismiss(authenticated, targetUserId, expectedSecurityMode); } public void reset() { @@ -456,12 +464,21 @@ * completion. * @param bypassSecondaryLockScreen true if the user is allowed to bypass the secondary * secondary lock screen requirement, if any. + * @param expectedSecurityMode SecurityMode that is invoking this request. SecurityMode.Invalid + * indicates that no check should be done * @return true if keyguard is done */ public boolean showNextSecurityScreenOrFinish(boolean authenticated, int targetUserId, - boolean bypassSecondaryLockScreen) { + boolean bypassSecondaryLockScreen, SecurityMode expectedSecurityMode) { if (DEBUG) Log.d(TAG, "showNextSecurityScreenOrFinish(" + authenticated + ")"); + if (expectedSecurityMode != SecurityMode.Invalid + && expectedSecurityMode != getCurrentSecurityMode()) { + Log.w(TAG, "Attempted to invoke showNextSecurityScreenOrFinish with securityMode " + + expectedSecurityMode + ", but current mode is " + getCurrentSecurityMode()); + return false; + } + boolean finish = false; boolean strongAuth = false; int eventSubtype = -1;
diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardSimPinViewController.java b/packages/SystemUI/src/com/android/keyguard/KeyguardSimPinViewController.java index 2f6fa14..ecd88e6 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardSimPinViewController.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardSimPinViewController.java
@@ -170,7 +170,8 @@ mRemainingAttempts = -1; mShowDefaultMessage = true; getKeyguardSecurityCallback().dismiss( - true, KeyguardUpdateMonitor.getCurrentUser()); + true, KeyguardUpdateMonitor.getCurrentUser(), + SecurityMode.SimPin); } else { mShowDefaultMessage = false; if (result.getResult() == PinResult.PIN_RESULT_TYPE_INCORRECT) {
diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardSimPukViewController.java b/packages/SystemUI/src/com/android/keyguard/KeyguardSimPukViewController.java index 47aa43b..203f9b6 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardSimPukViewController.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardSimPukViewController.java
@@ -69,7 +69,8 @@ if (simState == TelephonyManager.SIM_STATE_READY) { mRemainingAttempts = -1; mShowDefaultMessage = true; - getKeyguardSecurityCallback().dismiss(true, KeyguardUpdateMonitor.getCurrentUser()); + getKeyguardSecurityCallback().dismiss(true, KeyguardUpdateMonitor.getCurrentUser(), + SecurityMode.SimPuk); } else { resetState(); } @@ -278,7 +279,8 @@ mShowDefaultMessage = true; getKeyguardSecurityCallback().dismiss( - true, KeyguardUpdateMonitor.getCurrentUser()); + true, KeyguardUpdateMonitor.getCurrentUser(), + SecurityMode.SimPuk); } else { mShowDefaultMessage = false; if (result.getResult() == PinResult.PIN_RESULT_TYPE_INCORRECT) {
diff --git a/packages/SystemUI/tests/src/com/android/keyguard/AdminSecondaryLockScreenControllerTest.java b/packages/SystemUI/tests/src/com/android/keyguard/AdminSecondaryLockScreenControllerTest.java index dffad6c..80385e6 100644 --- a/packages/SystemUI/tests/src/com/android/keyguard/AdminSecondaryLockScreenControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/keyguard/AdminSecondaryLockScreenControllerTest.java
@@ -44,6 +44,7 @@ import androidx.test.filters.SmallTest; +import com.android.keyguard.KeyguardSecurityModel.SecurityMode; import com.android.systemui.SysuiTestCase; import org.junit.After; @@ -190,7 +191,7 @@ private void verifyViewDismissed(SurfaceView v) throws Exception { verify(mKeyguardSecurityContainer).removeView(v); - verify(mKeyguardCallback).dismiss(true, TARGET_USER_ID, true); + verify(mKeyguardCallback).dismiss(true, TARGET_USER_ID, true, SecurityMode.Invalid); assertThat(mContext.isBound(mComponentName)).isFalse(); } }
diff --git a/packages/SystemUI/tests/src/com/android/keyguard/KeyguardSecurityContainerControllerTest.java b/packages/SystemUI/tests/src/com/android/keyguard/KeyguardSecurityContainerControllerTest.java index dc87a6a..aecec9d 100644 --- a/packages/SystemUI/tests/src/com/android/keyguard/KeyguardSecurityContainerControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/keyguard/KeyguardSecurityContainerControllerTest.java
@@ -21,7 +21,10 @@ import static com.android.keyguard.KeyguardSecurityContainer.MODE_DEFAULT; import static com.android.keyguard.KeyguardSecurityContainer.MODE_ONE_HANDED; +import static com.google.common.truth.Truth.assertThat; + import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.eq; @@ -76,6 +79,8 @@ @RunWith(AndroidTestingRunner.class) @TestableLooper.RunWithLooper() public class KeyguardSecurityContainerControllerTest extends SysuiTestCase { + private static final int TARGET_USER_ID = 100; + @Rule public MockitoRule mRule = MockitoJUnit.rule(); @@ -381,6 +386,44 @@ verify(mSidefpsController, never()).show(); } + @Test + public void showNextSecurityScreenOrFinish_setsSecurityScreenToPinAfterSimPinUnlock() { + // GIVEN the current security method is SimPin + when(mKeyguardUpdateMonitor.getUserHasTrust(anyInt())).thenReturn(false); + when(mKeyguardUpdateMonitor.getUserUnlockedWithBiometric(TARGET_USER_ID)).thenReturn(false); + mKeyguardSecurityContainerController.showSecurityScreen(SecurityMode.SimPin); + + // WHEN a request is made from the SimPin screens to show the next security method + when(mKeyguardSecurityModel.getSecurityMode(TARGET_USER_ID)).thenReturn(SecurityMode.PIN); + mKeyguardSecurityContainerController.showNextSecurityScreenOrFinish( + /* authenticated= */true, + TARGET_USER_ID, + /* bypassSecondaryLockScreen= */true, + SecurityMode.SimPin); + + // THEN the next security method of PIN is set, and the keyguard is not marked as done + verify(mSecurityCallback, never()).finish(anyBoolean(), anyInt()); + assertThat(mKeyguardSecurityContainerController.getCurrentSecurityMode()) + .isEqualTo(SecurityMode.PIN); + } + + @Test + public void showNextSecurityScreenOrFinish_ignoresCallWhenSecurityMethodHasChanged() { + //GIVEN current security mode has been set to PIN + mKeyguardSecurityContainerController.showSecurityScreen(SecurityMode.PIN); + + //WHEN a request comes from SimPin to dismiss the security screens + boolean keyguardDone = mKeyguardSecurityContainerController.showNextSecurityScreenOrFinish( + /* authenticated= */true, + TARGET_USER_ID, + /* bypassSecondaryLockScreen= */true, + SecurityMode.SimPin); + + //THEN no action has happened, which will not dismiss the security screens + assertThat(keyguardDone).isEqualTo(false); + verify(mKeyguardUpdateMonitor, never()).getUserHasTrust(anyInt()); + } + private void setupConditionsToEnableSideFpsHint() { attachView(); setSideFpsHintEnabledFromResources(true);