From 5e0a46bff6d801a52d9c50d19e25286a2afc8b40 Mon Sep 17 00:00:00 2001 From: Victor Chang Date: Fri, 13 May 2016 17:06:59 +0100 Subject: [PATCH] Fix Continous loop in unifed screenlock when trying to Trust a CA cert Cause: with unified screenlock, ConfirmDeviceCredentialActivity didn't forward result with FLAG_ACTIVITY_FORWARD_RESULT Also, fixed that ConfirmDeviceCredentialActivity didn't allow fingerprint authenication in unified screenlock after keystore unlocked. In ChooseLockSettingsHelper, add one new util function to allow extra option to set returnCredentials to false while external to true. Set StrongAuth to "not required" when it has been successfully unlocked. Test: 1. PO Unified Screenlock/Work Challenge x fingerprint -> ok to trust cert (Also, no credential is returned in intent) 2. WorkMode off -> Reboot -> turn on Work mode -> no fingerprint option, PIN unlock successful to turn work mode on Bug: 28752364 Change-Id: I6dc8865e8f005545f8577d7731afb4495647062b --- .../settings/ChooseLockSettingsHelper.java | 63 +++++-- .../ConfirmDeviceCredentialActivity.java | 5 +- .../ConfirmDeviceCredentialBaseFragment.java | 22 ++- .../android/settings/ConfirmLockPassword.java | 12 +- .../android/settings/ConfirmLockPattern.java | 10 +- .../ChooseLockSettingsHelperTest.java | 157 ++++++++++++++++++ 6 files changed, 236 insertions(+), 33 deletions(-) create mode 100644 tests/unit/src/com/android/settings/ChooseLockSettingsHelperTest.java diff --git a/src/com/android/settings/ChooseLockSettingsHelper.java b/src/com/android/settings/ChooseLockSettingsHelper.java index ec7aa330ea6..6f0c453698b 100644 --- a/src/com/android/settings/ChooseLockSettingsHelper.java +++ b/src/com/android/settings/ChooseLockSettingsHelper.java @@ -24,12 +24,14 @@ import android.content.Intent; import android.content.IntentSender; import android.os.UserManager; +import com.android.internal.annotations.VisibleForTesting; import com.android.internal.widget.LockPatternUtils; public final class ChooseLockSettingsHelper { static final String EXTRA_KEY_TYPE = "type"; static final String EXTRA_KEY_PASSWORD = "password"; + public static final String EXTRA_KEY_RETURN_CREDENTIALS = "return_credentials"; public static final String EXTRA_KEY_HAS_CHALLENGE = "has_challenge"; public static final String EXTRA_KEY_CHALLENGE = "challenge"; public static final String EXTRA_KEY_CHALLENGE_TOKEN = "hw_auth_token"; @@ -37,7 +39,7 @@ public final class ChooseLockSettingsHelper { public static final String EXTRA_KEY_FOR_CHANGE_CRED_REQUIRED_FOR_BOOT = "for_cred_req_boot"; - private LockPatternUtils mLockPatternUtils; + @VisibleForTesting LockPatternUtils mLockPatternUtils; private Activity mActivity; private Fragment mFragment; @@ -104,7 +106,8 @@ public final class ChooseLockSettingsHelper { * @param returnCredentials if true, put credentials into intent. Note that if this is true, * this can only be called internally. * @param external specifies whether this activity is launched externally, meaning that it will - * get a dark theme and allow fingerprint authentication + * get a dark theme, allow fingerprint authentication and it will forward + * activity result. * @return true if one exists and we launched an activity to confirm it * @see Activity#onActivityResult(int, int, android.content.Intent) */ @@ -124,7 +127,8 @@ public final class ChooseLockSettingsHelper { * @param returnCredentials if true, put credentials into intent. Note that if this is true, * this can only be called internally. * @param external specifies whether this activity is launched externally, meaning that it will - * get a dark theme and allow fingerprint authentication + * get a dark theme, allow fingerprint authentication and it will forward + * activity result. * @param userId The userId for whom the lock should be confirmed. * @return true if one exists and we launched an activity to confirm it * @see Activity#onActivityResult(int, int, android.content.Intent) @@ -138,35 +142,58 @@ public final class ChooseLockSettingsHelper { /** * If a pattern, password or PIN exists, prompt the user before allowing them to change it. - * @param message optional message to display about the action about to be done - * @param details optional detail message to display + * + * @param title title of the confirmation screen; shown in the action bar + * @param header header of the confirmation screen; shown as large text + * @param description description of the confirmation screen * @param challenge a challenge to be verified against the device credential. - * This method can only be called internally. * @return true if one exists and we launched an activity to confirm it - * @see #onActivityResult(int, int, android.content.Intent) + * @see Activity#onActivityResult(int, int, android.content.Intent) */ public boolean launchConfirmationActivity(int request, @Nullable CharSequence title, @Nullable CharSequence header, @Nullable CharSequence description, long challenge) { return launchConfirmationActivity(request, title, header, description, - false, false, true, challenge, Utils.getCredentialOwnerUserId(mActivity)); + true, false, true, challenge, Utils.getCredentialOwnerUserId(mActivity)); } /** * If a pattern, password or PIN exists, prompt the user before allowing them to change it. - * @param message optional message to display about the action about to be done - * @param details optional detail message to display + * + * @param title title of the confirmation screen; shown in the action bar + * @param header header of the confirmation screen; shown as large text + * @param description description of the confirmation screen * @param challenge a challenge to be verified against the device credential. - * This method can only be called internally. * @param userId The userId for whom the lock should be confirmed. * @return true if one exists and we launched an activity to confirm it - * @see #onActivityResult(int, int, android.content.Intent) + * @see Activity#onActivityResult(int, int, android.content.Intent) */ public boolean launchConfirmationActivity(int request, @Nullable CharSequence title, @Nullable CharSequence header, @Nullable CharSequence description, long challenge, int userId) { return launchConfirmationActivity(request, title, header, description, - false, false, true, challenge, Utils.enforceSameOwner(mActivity, userId)); + true, false, true, challenge, Utils.enforceSameOwner(mActivity, userId)); + } + + /** + * If a pattern, password or PIN exists, prompt the user before allowing them to change it. + * + * @param title title of the confirmation screen; shown in the action bar + * @param header header of the confirmation screen; shown as large text + * @param description description of the confirmation screen + * @param external specifies whether this activity is launched externally, meaning that it will + * get a dark theme, allow fingerprint authentication and it will forward + * activity result. + * @param challenge a challenge to be verified against the device credential. + * @param userId The userId for whom the lock should be confirmed. + * @return true if one exists and we launched an activity to confirm it + * @see Activity#onActivityResult(int, int, android.content.Intent) + */ + public boolean launchConfirmationActivityWithExternalAndChallenge(int request, + @Nullable CharSequence title, @Nullable CharSequence header, + @Nullable CharSequence description, boolean external, long challenge, int userId) { + return launchConfirmationActivity(request, title, header, description, false, + external, true, challenge, Utils.enforceSameOwner(mActivity, userId)); } private boolean launchConfirmationActivity(int request, @Nullable CharSequence title, @@ -181,7 +208,7 @@ public final class ChooseLockSettingsHelper { launched = launchConfirmationActivity(request, title, header, description, returnCredentials || hasChallenge ? ConfirmLockPattern.InternalActivity.class - : ConfirmLockPattern.class, external, + : ConfirmLockPattern.class, returnCredentials, external, hasChallenge, challenge, userId); break; case DevicePolicyManager.PASSWORD_QUALITY_NUMERIC: @@ -193,7 +220,7 @@ public final class ChooseLockSettingsHelper { launched = launchConfirmationActivity(request, title, header, description, returnCredentials || hasChallenge ? ConfirmLockPassword.InternalActivity.class - : ConfirmLockPassword.class, external, + : ConfirmLockPassword.class, returnCredentials, external, hasChallenge, challenge, userId); break; } @@ -201,8 +228,9 @@ public final class ChooseLockSettingsHelper { } private boolean launchConfirmationActivity(int request, CharSequence title, CharSequence header, - CharSequence message, Class activityClass, boolean external, boolean hasChallenge, - long challenge, int userId) { + CharSequence message, Class activityClass, boolean returnCredentials, + boolean external, boolean hasChallenge, long challenge, + int userId) { final Intent intent = new Intent(); intent.putExtra(ConfirmDeviceCredentialBaseFragment.TITLE_TEXT, title); intent.putExtra(ConfirmDeviceCredentialBaseFragment.HEADER_TEXT, header); @@ -211,6 +239,7 @@ public final class ChooseLockSettingsHelper { intent.putExtra(ConfirmDeviceCredentialBaseFragment.DARK_THEME, external); intent.putExtra(ConfirmDeviceCredentialBaseFragment.SHOW_CANCEL_BUTTON, external); intent.putExtra(ConfirmDeviceCredentialBaseFragment.SHOW_WHEN_LOCKED, external); + intent.putExtra(ChooseLockSettingsHelper.EXTRA_KEY_RETURN_CREDENTIALS, returnCredentials); intent.putExtra(ChooseLockSettingsHelper.EXTRA_KEY_HAS_CHALLENGE, hasChallenge); intent.putExtra(ChooseLockSettingsHelper.EXTRA_KEY_CHALLENGE, challenge); // we should never have a drawer when confirming device credentials. diff --git a/src/com/android/settings/ConfirmDeviceCredentialActivity.java b/src/com/android/settings/ConfirmDeviceCredentialActivity.java index 18fc2e09ef0..e98b819a976 100644 --- a/src/com/android/settings/ConfirmDeviceCredentialActivity.java +++ b/src/com/android/settings/ConfirmDeviceCredentialActivity.java @@ -89,8 +89,9 @@ public class ConfirmDeviceCredentialActivity extends Activity { && !lockPatternUtils.isSeparateProfileChallengeEnabled(userId)) { // We set the challenge as 0L, so it will force to unlock managed profile when it // unlocks primary profile screen lock, by calling verifyTiedProfileChallenge() - launched = helper.launchConfirmationActivity(0 /* request code */, null /* title */, - title, details, 0L, userId); + launched = helper.launchConfirmationActivityWithExternalAndChallenge( + 0 /* request code */, null /* title */, title, details, true /* isExternal */, + 0L /* challenge */, userId); } else { launched = helper.launchConfirmationActivity(0 /* request code */, null /* title */, title, details, false /* returnCredentials */, true /* isExternal */, userId); diff --git a/src/com/android/settings/ConfirmDeviceCredentialBaseFragment.java b/src/com/android/settings/ConfirmDeviceCredentialBaseFragment.java index ae321e8d9d9..ef19c2a2d90 100644 --- a/src/com/android/settings/ConfirmDeviceCredentialBaseFragment.java +++ b/src/com/android/settings/ConfirmDeviceCredentialBaseFragment.java @@ -37,6 +37,7 @@ import android.os.Bundle; import android.os.Handler; import android.os.RemoteException; import android.os.UserManager; +import android.security.KeyStore; import android.view.View; import android.view.ViewGroup; import android.widget.Button; @@ -68,6 +69,7 @@ public abstract class ConfirmDeviceCredentialBaseFragment extends OptionsMenuFra private FingerprintUiHelper mFingerprintHelper; protected boolean mIsStrongAuthRequired; private boolean mAllowFpAuthentication; + protected boolean mReturnCredentials = false; protected Button mCancelButton; protected ImageView mFingerprintIcon; protected int mEffectiveUserId; @@ -81,15 +83,17 @@ public abstract class ConfirmDeviceCredentialBaseFragment extends OptionsMenuFra super.onCreate(savedInstanceState); mAllowFpAuthentication = getActivity().getIntent().getBooleanExtra( ALLOW_FP_AUTHENTICATION, false); + mReturnCredentials = getActivity().getIntent().getBooleanExtra( + ChooseLockSettingsHelper.EXTRA_KEY_RETURN_CREDENTIALS, false); // Only take this argument into account if it belongs to the current profile. Intent intent = getActivity().getIntent(); mUserId = Utils.getUserIdFromBundle(getActivity(), intent.getExtras()); final UserManager userManager = UserManager.get(getActivity()); mEffectiveUserId = userManager.getCredentialOwnerProfile(mUserId); - mIsStrongAuthRequired = isStrongAuthRequired(); - mAllowFpAuthentication = mAllowFpAuthentication && !isFingerprintDisabledByAdmin() - && !mIsStrongAuthRequired; mLockPatternUtils = new LockPatternUtils(getActivity()); + mIsStrongAuthRequired = isFingerprintDisallowedByStrongAuth(); + mAllowFpAuthentication = mAllowFpAuthentication && !isFingerprintDisabledByAdmin() + && !mReturnCredentials && !mIsStrongAuthRequired; } @Override @@ -126,8 +130,13 @@ public abstract class ConfirmDeviceCredentialBaseFragment extends OptionsMenuFra return (disabledFeatures & DevicePolicyManager.KEYGUARD_DISABLE_FINGERPRINT) != 0; } - private boolean isStrongAuthRequired() { - return !(UserManager.get(getContext()).isUserUnlocked(mEffectiveUserId)); + // User could be locked while Effective user is unlocked even though the effective owns the + // credential. Otherwise, fingerprint can't unlock fbe/keystore through + // verifyTiedProfileChallenge. In such case, we also wanna show the user message that + // fingerprint is disabled due to device restart. + private boolean isFingerprintDisallowedByStrongAuth() { + return !(mLockPatternUtils.isFingerprintAllowedForUser(mEffectiveUserId) + && KeyStore.getInstance().state(mUserId) == KeyStore.State.UNLOCKED); } @Override @@ -245,6 +254,9 @@ public abstract class ConfirmDeviceCredentialBaseFragment extends OptionsMenuFra protected void reportSuccessfullAttempt() { if (isProfileChallenge()) { mLockPatternUtils.reportSuccessfulPasswordAttempt(mEffectiveUserId); + // Keyguard is responsible to disable StrongAuth for primary user. Disable StrongAuth + // for work challenge only here. + mLockPatternUtils.userPresent(mEffectiveUserId); } } diff --git a/src/com/android/settings/ConfirmLockPassword.java b/src/com/android/settings/ConfirmLockPassword.java index 61cf2aeec05..4797012178f 100644 --- a/src/com/android/settings/ConfirmLockPassword.java +++ b/src/com/android/settings/ConfirmLockPassword.java @@ -187,7 +187,7 @@ public class ConfirmLockPassword extends ConfirmDeviceCredentialBaseActivity { boolean isProfile = Utils.isManagedProfile( UserManager.get(getActivity()), mEffectiveUserId); // Map boolean flags to an index by isStrongAuth << 2 + isProfile << 1 + isAlpha. - int index = ((mIsStrongAuthRequired ? 1 : 0) << 2) + ((isProfile ? 1 : 0 ) << 1) + int index = ((mIsStrongAuthRequired ? 1 : 0) << 2) + ((isProfile ? 1 : 0) << 1) + (mIsAlpha ? 1 : 0); return DETAIL_TEXTS[index]; } @@ -356,9 +356,11 @@ public class ConfirmLockPassword extends ConfirmDeviceCredentialBaseActivity { boolean matched = false; if (token != null) { matched = true; - intent.putExtra( - ChooseLockSettingsHelper.EXTRA_KEY_CHALLENGE_TOKEN, - token); + if (mReturnCredentials) { + intent.putExtra( + ChooseLockSettingsHelper.EXTRA_KEY_CHALLENGE_TOKEN, + token); + } } mCredentialCheckResultTracker.setResult(matched, intent, timeoutMs, localUserId); @@ -382,7 +384,7 @@ public class ConfirmLockPassword extends ConfirmDeviceCredentialBaseActivity { @Override public void onChecked(boolean matched, int timeoutMs) { mPendingLockCheck = null; - if (matched && isInternalActivity()) { + if (matched && isInternalActivity() && mReturnCredentials) { intent.putExtra(ChooseLockSettingsHelper.EXTRA_KEY_TYPE, mIsAlpha ? StorageManager.CRYPT_TYPE_PASSWORD : StorageManager.CRYPT_TYPE_PIN); diff --git a/src/com/android/settings/ConfirmLockPattern.java b/src/com/android/settings/ConfirmLockPattern.java index 262cdc880ac..dd8640c1998 100644 --- a/src/com/android/settings/ConfirmLockPattern.java +++ b/src/com/android/settings/ConfirmLockPattern.java @@ -439,9 +439,11 @@ public class ConfirmLockPattern extends ConfirmDeviceCredentialBaseActivity { boolean matched = false; if (token != null) { matched = true; - intent.putExtra( - ChooseLockSettingsHelper.EXTRA_KEY_CHALLENGE_TOKEN, - token); + if (mReturnCredentials) { + intent.putExtra( + ChooseLockSettingsHelper.EXTRA_KEY_CHALLENGE_TOKEN, + token); + } } mCredentialCheckResultTracker.setResult(matched, intent, timeoutMs, localEffectiveUserId); @@ -472,7 +474,7 @@ public class ConfirmLockPattern extends ConfirmDeviceCredentialBaseActivity { @Override public void onChecked(boolean matched, int timeoutMs) { mPendingLockCheck = null; - if (matched && isInternalActivity()) { + if (matched && isInternalActivity() && mReturnCredentials) { intent.putExtra(ChooseLockSettingsHelper.EXTRA_KEY_TYPE, StorageManager.CRYPT_TYPE_PATTERN); intent.putExtra(ChooseLockSettingsHelper.EXTRA_KEY_PASSWORD, diff --git a/tests/unit/src/com/android/settings/ChooseLockSettingsHelperTest.java b/tests/unit/src/com/android/settings/ChooseLockSettingsHelperTest.java new file mode 100644 index 00000000000..218a74ea7af --- /dev/null +++ b/tests/unit/src/com/android/settings/ChooseLockSettingsHelperTest.java @@ -0,0 +1,157 @@ +package com.android.settings; + + +import android.app.Activity; +import android.app.admin.DevicePolicyManager; +import android.content.ComponentName; +import android.content.Context; +import android.content.Intent; +import android.os.UserHandle; +import android.os.UserManager; +import android.test.AndroidTestCase; + +import com.android.internal.widget.LockPatternUtils; + +import org.mockito.ArgumentCaptor; + +import java.util.List; + +import static org.mockito.Matchers.anyInt; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class ChooseLockSettingsHelperTest extends AndroidTestCase { + + private static final String SYSTEM_PROPERTY_DEXMAKER_DEXCACHE = "dexmaker.dexcache"; + + @Override + protected void setUp() throws Exception { + super.setUp(); + System.setProperty(SYSTEM_PROPERTY_DEXMAKER_DEXCACHE, getContext().getCacheDir().getPath()); + } + + @Override + protected void tearDown() throws Exception { + super.tearDown(); + System.clearProperty(SYSTEM_PROPERTY_DEXMAKER_DEXCACHE); + } + + public void testlaunchConfirmationActivityWithExternalAndChallenge() { + + final int userId = UserHandle.myUserId(); + final int request = 100; + final long challenge = 10000L; + { + // Test external == true + final boolean external = true; + + final Activity mockActivity = getMockActivity(); + ChooseLockSettingsHelper helper = getChooseLockSettingsHelper(mockActivity); + helper.launchConfirmationActivityWithExternalAndChallenge( + request, // request + "title", + "header", + "description", + external, + challenge, + userId + ); + final ArgumentCaptor intentCaptor = ArgumentCaptor.forClass(Intent.class); + verify(mockActivity, times(1)).startActivity(intentCaptor.capture()); + Intent capturedIntent = getResultIntent(intentCaptor); + + assertEquals(new ComponentName("com.android.settings", + ConfirmLockPattern.InternalActivity.class.getName()), + capturedIntent.getComponent()); + assertFalse(capturedIntent.getBooleanExtra( + ChooseLockSettingsHelper.EXTRA_KEY_RETURN_CREDENTIALS, false)); + assertTrue(capturedIntent.getBooleanExtra( + ChooseLockSettingsHelper.EXTRA_KEY_HAS_CHALLENGE, false)); + assertEquals(challenge, capturedIntent.getLongExtra( + ChooseLockSettingsHelper.EXTRA_KEY_CHALLENGE, 0L)); + assertEquals(external, + (capturedIntent.getFlags() & Intent.FLAG_ACTIVITY_FORWARD_RESULT) != 0); + assertEquals(external, capturedIntent.getBooleanExtra( + ConfirmDeviceCredentialBaseFragment.ALLOW_FP_AUTHENTICATION, false)); + assertEquals(external, capturedIntent.getBooleanExtra( + ConfirmDeviceCredentialBaseFragment.DARK_THEME, false)); + assertEquals(external, capturedIntent.getBooleanExtra( + ConfirmDeviceCredentialBaseFragment.SHOW_CANCEL_BUTTON, false)); + assertEquals(external, capturedIntent.getBooleanExtra( + ConfirmDeviceCredentialBaseFragment.SHOW_WHEN_LOCKED, false)); + } + + { + // Test external == false + final boolean external = false; + + final Activity mockActivity = getMockActivity(); + ChooseLockSettingsHelper helper = getChooseLockSettingsHelper(mockActivity); + helper.launchConfirmationActivityWithExternalAndChallenge( + request, // request + "title", + "header", + "description", + external, + challenge, + userId + ); + final ArgumentCaptor intentCaptor = ArgumentCaptor.forClass(Intent.class); + verify(mockActivity, times(1)).startActivityForResult(intentCaptor.capture(), + eq(request)); + Intent capturedIntent = getResultIntent(intentCaptor); + + + assertEquals(new ComponentName("com.android.settings", + ConfirmLockPattern.InternalActivity.class.getName()), + capturedIntent.getComponent()); + assertFalse(capturedIntent.getBooleanExtra( + ChooseLockSettingsHelper.EXTRA_KEY_RETURN_CREDENTIALS, false)); + assertTrue(capturedIntent.getBooleanExtra( + ChooseLockSettingsHelper.EXTRA_KEY_HAS_CHALLENGE, false)); + assertEquals(challenge, capturedIntent.getLongExtra( + ChooseLockSettingsHelper.EXTRA_KEY_CHALLENGE, 0L)); + assertEquals(external, + (capturedIntent.getFlags() & Intent.FLAG_ACTIVITY_FORWARD_RESULT) != 0); + assertEquals(external, capturedIntent.getBooleanExtra( + ConfirmDeviceCredentialBaseFragment.ALLOW_FP_AUTHENTICATION, false)); + assertEquals(external, capturedIntent.getBooleanExtra( + ConfirmDeviceCredentialBaseFragment.DARK_THEME, false)); + assertEquals(external, capturedIntent.getBooleanExtra( + ConfirmDeviceCredentialBaseFragment.SHOW_CANCEL_BUTTON, false)); + assertEquals(external, capturedIntent.getBooleanExtra( + ConfirmDeviceCredentialBaseFragment.SHOW_WHEN_LOCKED, false)); + } + } + + + private ChooseLockSettingsHelper getChooseLockSettingsHelper(Activity mockActivity) { + LockPatternUtils mockLockPatternUtils = mock(LockPatternUtils.class); + when(mockLockPatternUtils.getKeyguardStoredPasswordQuality(anyInt())) + .thenReturn(DevicePolicyManager.PASSWORD_QUALITY_SOMETHING); + + ChooseLockSettingsHelper helper = new ChooseLockSettingsHelper(mockActivity); + helper.mLockPatternUtils = mockLockPatternUtils; + return helper; + } + + private Activity getMockActivity() { + Activity mockActivity = mock(Activity.class); + when(mockActivity.getSystemService(Context.USER_SERVICE)) + .thenReturn(getContext().getSystemService(UserManager.class)); + when(mockActivity.getContentResolver()).thenReturn(getContext().getContentResolver()); + when(mockActivity.getIntent()).thenReturn(new Intent()); + return mockActivity; + } + + + + private static Intent getResultIntent(ArgumentCaptor intentCaptor) { + List capturedIntents = intentCaptor.getAllValues(); + assertEquals(1, capturedIntents.size()); + return capturedIntents.get(0); + } +}