From 813f32c92cd915e41011c960940d50f28f0f58f2 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Wed, 21 Jun 2023 05:28:11 +0000 Subject: [PATCH] Add and use PasswordMetrics#validateCredential() Add a method PasswordMetrics#validateCredential() which takes a LockscreenCredential argument and honors the "invalid chars" flag in it. This should be used instead of PasswordMetrics#validatePassword() which does not honor the "invalid chars" flag (which it does not have access to) and only works for PASSWORD and PIN, not PATTERN and NONE. Convert all callers of validatePassword() in frameworks/base to use validateCredential(). After this, two callers remain: one in packages/apps/Settings which I'll convert right away too, and several in packages/apps/Car/Settings which will take a bit longer to get rid of. Bug: 219511761 Bug: 232900169 Bug: 243881358 Test: atest PasswordMetricsTest Test: Verified that 'locksettings set-password' no longer accepts a non-ASCII char that was accepted before Test: see I5f1822a34688473cb103eb64dca56e4c19d4dd08 Change-Id: Ib8f43aef5b5aa5bb059780707582d0419f9ddf9a --- core/java/android/app/KeyguardManager.java | 9 ++-- .../android/app/admin/PasswordMetrics.java | 41 +++++++++++++--- .../app/admin/PasswordMetricsTest.java | 47 ++++++++++++++++++- .../LockSettingsShellCommand.java | 14 +----- .../DevicePolicyManagerService.java | 25 ++++------ 5 files changed, 96 insertions(+), 40 deletions(-) diff --git a/core/java/android/app/KeyguardManager.java b/core/java/android/app/KeyguardManager.java index e7a2b61ecaf9e..f467200fab85f 100644 --- a/core/java/android/app/KeyguardManager.java +++ b/core/java/android/app/KeyguardManager.java @@ -883,17 +883,18 @@ public class KeyguardManager { if (!checkInitialLockMethodUsage()) { return false; } + Objects.requireNonNull(password, "Password cannot be null."); complexity = PasswordMetrics.sanitizeComplexityLevel(complexity); // TODO: b/131755827 add devicePolicyManager support for Auto DevicePolicyManager devicePolicyManager = (DevicePolicyManager) mContext.getSystemService(Context.DEVICE_POLICY_SERVICE); PasswordMetrics adminMetrics = devicePolicyManager.getPasswordMinimumMetrics(mContext.getUserId()); - // Check if the password fits the mold of a pin or pattern. - boolean isPinOrPattern = lockType != PASSWORD; - return PasswordMetrics.validatePassword( - adminMetrics, complexity, isPinOrPattern, password).size() == 0; + try (LockscreenCredential credential = createLockscreenCredential(lockType, password)) { + return PasswordMetrics.validateCredential(adminMetrics, complexity, + credential).size() == 0; + } } /** diff --git a/core/java/android/app/admin/PasswordMetrics.java b/core/java/android/app/admin/PasswordMetrics.java index ab48791d43eff..251c5d83fbb9c 100644 --- a/core/java/android/app/admin/PasswordMetrics.java +++ b/core/java/android/app/admin/PasswordMetrics.java @@ -513,16 +513,45 @@ public final class PasswordMetrics implements Parcelable { } /** - * Validates password against minimum metrics and complexity. + * Validates a proposed lockscreen credential against minimum metrics and complexity. * - * @param adminMetrics - minimum metrics to satisfy admin requirements. - * @param minComplexity - minimum complexity imposed by the requester. - * @param isPin - whether it is PIN that should be only digits - * @param password - password to validate. - * @return a list of password validation errors. An empty list means the password is OK. + * @param adminMetrics minimum metrics to satisfy admin requirements + * @param minComplexity minimum complexity imposed by the requester + * @param credential the proposed lockscreen credential + * + * @return a list of validation errors. An empty list means the credential is OK. * * TODO: move to PasswordPolicy */ + public static List validateCredential( + PasswordMetrics adminMetrics, int minComplexity, LockscreenCredential credential) { + if (credential.hasInvalidChars()) { + return Collections.singletonList( + new PasswordValidationError(CONTAINS_INVALID_CHARACTERS, 0)); + } + if (credential.isPassword() || credential.isPin()) { + return validatePassword(adminMetrics, minComplexity, credential.isPin(), + credential.getCredential()); + } else { + return validatePasswordMetrics(adminMetrics, minComplexity, + new PasswordMetrics(credential.getType())); + } + } + + /** + * Validates a proposed lockscreen credential against minimum metrics and complexity. + * + * @param adminMetrics minimum metrics to satisfy admin requirements + * @param minComplexity minimum complexity imposed by the requester + * @param isPin whether to validate as a PIN (true) or password (false) + * @param password the proposed lockscreen credential as a byte[]. Must be the value from + * {@link LockscreenCredential#getCredential()}. + * + * @return a list of validation errors. An empty list means the credential is OK. + * + * TODO: merge this into validateCredential() and remove the redundant hasInvalidCharacters(), + * once all external callers are removed + */ public static List validatePassword( PasswordMetrics adminMetrics, int minComplexity, boolean isPin, byte[] password) { diff --git a/core/tests/coretests/src/android/app/admin/PasswordMetricsTest.java b/core/tests/coretests/src/android/app/admin/PasswordMetricsTest.java index c9e02f8a998d4..84c54006fc67b 100644 --- a/core/tests/coretests/src/android/app/admin/PasswordMetricsTest.java +++ b/core/tests/coretests/src/android/app/admin/PasswordMetricsTest.java @@ -25,6 +25,7 @@ import static android.app.admin.DevicePolicyManager.PASSWORD_QUALITY_SOMETHING; import static android.app.admin.DevicePolicyManager.PASSWORD_QUALITY_UNSPECIFIED; import static android.app.admin.PasswordMetrics.complexityLevelToMinQuality; import static android.app.admin.PasswordMetrics.sanitizeComplexityLevel; +import static android.app.admin.PasswordMetrics.validateCredential; import static android.app.admin.PasswordMetrics.validatePasswordMetrics; import static com.android.internal.widget.LockPatternUtils.CREDENTIAL_TYPE_NONE; @@ -41,6 +42,7 @@ import android.platform.test.annotations.Presubmit; import androidx.test.ext.junit.runners.AndroidJUnit4; import androidx.test.filters.SmallTest; +import com.android.internal.widget.LockscreenCredential; import com.android.internal.widget.PasswordValidationError; import org.junit.Test; @@ -374,8 +376,51 @@ public class PasswordMetricsTest { PasswordValidationError.NOT_ENOUGH_NON_DIGITS, 1); } + @Test + public void testValidateCredential_none() { + PasswordMetrics adminMetrics; + LockscreenCredential none = LockscreenCredential.createNone(); + + adminMetrics = new PasswordMetrics(CREDENTIAL_TYPE_NONE); + assertValidationErrors( + validateCredential(adminMetrics, PASSWORD_COMPLEXITY_NONE, none)); + + adminMetrics = new PasswordMetrics(CREDENTIAL_TYPE_PIN); + assertValidationErrors( + validateCredential(adminMetrics, PASSWORD_COMPLEXITY_NONE, none), + PasswordValidationError.WEAK_CREDENTIAL_TYPE, 0); + } + + @Test + public void testValidateCredential_password() { + PasswordMetrics adminMetrics; + LockscreenCredential password; + + adminMetrics = new PasswordMetrics(CREDENTIAL_TYPE_NONE); + password = LockscreenCredential.createPassword("password"); + assertValidationErrors( + validateCredential(adminMetrics, PASSWORD_COMPLEXITY_LOW, password)); + + // Test that validateCredential() checks LockscreenCredential#hasInvalidChars(). + adminMetrics = new PasswordMetrics(CREDENTIAL_TYPE_NONE); + password = LockscreenCredential.createPassword("™™™™"); + assertTrue(password.hasInvalidChars()); + assertValidationErrors( + validateCredential(adminMetrics, PASSWORD_COMPLEXITY_LOW, password), + PasswordValidationError.CONTAINS_INVALID_CHARACTERS, 0); + + // Test one more case where validateCredential() should reject the password. Beyond this, + // the unit tests for the lower-level method validatePasswordMetrics() should be sufficient. + adminMetrics = new PasswordMetrics(CREDENTIAL_TYPE_NONE); + adminMetrics.length = 6; + password = LockscreenCredential.createPassword("pass"); + assertValidationErrors( + validateCredential(adminMetrics, PASSWORD_COMPLEXITY_LOW, password), + PasswordValidationError.TOO_SHORT, 6); + } + /** - * @param expected sequense of validation error codes followed by requirement values, must have + * @param expected sequence of validation error codes followed by requirement values, must have * even number of elements. Empty means no errors. */ private void assertValidationErrors( diff --git a/services/core/java/com/android/server/locksettings/LockSettingsShellCommand.java b/services/core/java/com/android/server/locksettings/LockSettingsShellCommand.java index f107d0bf99327..df95c69e7271c 100644 --- a/services/core/java/com/android/server/locksettings/LockSettingsShellCommand.java +++ b/services/core/java/com/android/server/locksettings/LockSettingsShellCommand.java @@ -16,8 +16,6 @@ package com.android.server.locksettings; -import static com.android.internal.widget.LockPatternUtils.CREDENTIAL_TYPE_NONE; -import static com.android.internal.widget.LockPatternUtils.CREDENTIAL_TYPE_PATTERN; import static com.android.internal.widget.LockPatternUtils.StrongAuthTracker.STRONG_AUTH_REQUIRED_AFTER_USER_LOCKDOWN; import android.app.ActivityManager; @@ -313,16 +311,8 @@ class LockSettingsShellCommand extends ShellCommand { mLockPatternUtils.getRequestedPasswordMetrics(mCurrentUserId); final int requiredComplexity = mLockPatternUtils.getRequestedPasswordComplexity(mCurrentUserId); - final List errors; - if (credential.isPassword() || credential.isPin()) { - errors = PasswordMetrics.validatePassword(requiredMetrics, requiredComplexity, - credential.isPin(), credential.getCredential()); - } else { - PasswordMetrics metrics = new PasswordMetrics( - credential.isPattern() ? CREDENTIAL_TYPE_PATTERN : CREDENTIAL_TYPE_NONE); - errors = PasswordMetrics.validatePasswordMetrics( - requiredMetrics, requiredComplexity, metrics); - } + final List errors = + PasswordMetrics.validateCredential(requiredMetrics, requiredComplexity, credential); if (!errors.isEmpty()) { getOutPrintWriter().println( "New credential doesn't satisfy admin policies: " + errors.get(0)); diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index a452328599e34..6ea71e382a716 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -5728,20 +5728,17 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { final int callingUid = caller.getUid(); final int userHandle = UserHandle.getUserId(callingUid); final boolean isPin = PasswordMetrics.isNumericOnly(password); + final LockscreenCredential newCredential; + if (isPin) { + newCredential = LockscreenCredential.createPin(password); + } else { + newCredential = LockscreenCredential.createPasswordOrNone(password); + } synchronized (getLockObject()) { final PasswordMetrics minMetrics = getPasswordMinimumMetricsUnchecked(userHandle); - final List validationErrors; final int complexity = getAggregatedPasswordComplexityLocked(userHandle); - // TODO: Consider changing validation API to take LockscreenCredential. - if (password.isEmpty()) { - validationErrors = PasswordMetrics.validatePasswordMetrics( - minMetrics, complexity, new PasswordMetrics(CREDENTIAL_TYPE_NONE)); - } else { - // TODO(b/120484642): remove getBytes() below - validationErrors = PasswordMetrics.validatePassword( - minMetrics, complexity, isPin, password.getBytes()); - } - + final List validationErrors = + PasswordMetrics.validateCredential(minMetrics, complexity, newCredential); if (!validationErrors.isEmpty()) { Slogf.w(LOG_TAG, "Failed to reset password due to constraint violation: %s", validationErrors.get(0)); @@ -5765,12 +5762,6 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { // Don't do this with the lock held, because it is going to call // back in to the service. final long ident = mInjector.binderClearCallingIdentity(); - final LockscreenCredential newCredential; - if (isPin) { - newCredential = LockscreenCredential.createPin(password); - } else { - newCredential = LockscreenCredential.createPasswordOrNone(password); - } try { if (tokenHandle == 0 || token == null) { if (!mLockPatternUtils.setLockCredential(newCredential,