From e4c9d2de89f206cf1586c30d551a61aa43f6cb37 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 27 Jun 2023 22:27:26 +0000 Subject: [PATCH 1/2] Enforce minimum pattern length in PasswordMetrics For patterns, currently PasswordMetrics#validateCredential() does nothing except validate that the pattern credential type is allowed. Pattern length is supposed to be >= MIN_LOCK_PATTERN_SIZE, but that is hardcoded into the ChooseLockPattern activity in Settings. Since there are other places that can set a pattern lockscreen credential, such as LockSettingsShellCommand, let's add the pattern length validation to PasswordMetrics#validatePasswordMetrics() so that it gets done in the same place as PIN and password validation. Note that this required starting to populate the length field of PasswordMetrics created for patterns. Bug: 219511761 Bug: 232900169 Bug: 243881358 Test: atest PasswordMetricsTest Change-Id: I9f33f86045ea5f674b8ab79f9fda62c988436c53 --- .../android/app/admin/PasswordMetrics.java | 29 ++++++++++++------- .../app/admin/PasswordMetricsTest.java | 16 ++++++++++ 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/core/java/android/app/admin/PasswordMetrics.java b/core/java/android/app/admin/PasswordMetrics.java index 251c5d83fbb9c..9b64d8fe84b4d 100644 --- a/core/java/android/app/admin/PasswordMetrics.java +++ b/core/java/android/app/admin/PasswordMetrics.java @@ -30,6 +30,7 @@ import static com.android.internal.widget.LockPatternUtils.CREDENTIAL_TYPE_PASSW import static com.android.internal.widget.LockPatternUtils.CREDENTIAL_TYPE_PATTERN; import static com.android.internal.widget.LockPatternUtils.CREDENTIAL_TYPE_PIN; import static com.android.internal.widget.LockPatternUtils.MIN_LOCK_PASSWORD_SIZE; +import static com.android.internal.widget.LockPatternUtils.MIN_LOCK_PATTERN_SIZE; import static com.android.internal.widget.PasswordValidationError.CONTAINS_INVALID_CHARACTERS; import static com.android.internal.widget.PasswordValidationError.CONTAINS_SEQUENCE; import static com.android.internal.widget.PasswordValidationError.NOT_ENOUGH_DIGITS; @@ -201,7 +202,9 @@ public final class PasswordMetrics implements Parcelable { return PasswordMetrics.computeForPasswordOrPin(credential.getCredential(), credential.isPin()); } else if (credential.isPattern()) { - return new PasswordMetrics(CREDENTIAL_TYPE_PATTERN); + PasswordMetrics metrics = new PasswordMetrics(CREDENTIAL_TYPE_PATTERN); + metrics.length = credential.size(); + return metrics; } else if (credential.isNone()) { return new PasswordMetrics(CREDENTIAL_TYPE_NONE); } else { @@ -529,13 +532,8 @@ public final class PasswordMetrics implements Parcelable { 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())); - } + PasswordMetrics actualMetrics = computeForCredential(credential); + return validatePasswordMetrics(adminMetrics, minComplexity, actualMetrics); } /** @@ -584,9 +582,18 @@ public final class PasswordMetrics implements Parcelable { || !bucket.allowsCredType(actualMetrics.credType)) { return Collections.singletonList(new PasswordValidationError(WEAK_CREDENTIAL_TYPE, 0)); } - if (actualMetrics.credType != CREDENTIAL_TYPE_PASSWORD - && actualMetrics.credType != CREDENTIAL_TYPE_PIN) { - return Collections.emptyList(); // Nothing to check for pattern or none. + if (actualMetrics.credType == CREDENTIAL_TYPE_PATTERN) { + // For pattern, only need to check the length against the hardcoded minimum. If the + // pattern length is unavailable (e.g., PasswordMetrics that was stored on-disk before + // the pattern length started being included in it), assume it is okay. + if (actualMetrics.length != 0 && actualMetrics.length < MIN_LOCK_PATTERN_SIZE) { + return Collections.singletonList(new PasswordValidationError(TOO_SHORT, + MIN_LOCK_PATTERN_SIZE)); + } + return Collections.emptyList(); + } + if (actualMetrics.credType == CREDENTIAL_TYPE_NONE) { + return Collections.emptyList(); // Nothing to check for none. } if (actualMetrics.credType == CREDENTIAL_TYPE_PIN && actualMetrics.nonNumeric > 0) { diff --git a/core/tests/coretests/src/android/app/admin/PasswordMetricsTest.java b/core/tests/coretests/src/android/app/admin/PasswordMetricsTest.java index 84c54006fc67b..3c476300cf6bd 100644 --- a/core/tests/coretests/src/android/app/admin/PasswordMetricsTest.java +++ b/core/tests/coretests/src/android/app/admin/PasswordMetricsTest.java @@ -42,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.LockPatternUtils; import com.android.internal.widget.LockscreenCredential; import com.android.internal.widget.PasswordValidationError; @@ -419,6 +420,21 @@ public class PasswordMetricsTest { PasswordValidationError.TOO_SHORT, 6); } + private LockscreenCredential createPattern(String patternString) { + return LockscreenCredential.createPattern(LockPatternUtils.byteArrayToPattern( + patternString.getBytes())); + } + + @Test + public void testValidateCredential_pattern() { + PasswordMetrics adminMetrics = new PasswordMetrics(CREDENTIAL_TYPE_NONE); + assertValidationErrors( + validateCredential(adminMetrics, PASSWORD_COMPLEXITY_NONE, createPattern("123")), + PasswordValidationError.TOO_SHORT, 4); + assertValidationErrors( + validateCredential(adminMetrics, PASSWORD_COMPLEXITY_NONE, createPattern("1234"))); + } + /** * @param expected sequence of validation error codes followed by requirement values, must have * even number of elements. Empty means no errors. From 25053e2043640d373b1960dae0b6a023a64e7117 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 27 Jun 2023 23:01:26 +0000 Subject: [PATCH 2/2] Remove PasswordMetrics#validatePassword() PasswordMetrics#validatePassword() is no longer used since all its callers were to converted to use PasswordMetrics#validateCredential() instead. So, remove it. Bug: 219511761 Bug: 232900169 Bug: 243881358 Test: build Change-Id: Ica7adff4f7171b24518cd916f1d4f6d4707851d6 --- .../android/app/admin/PasswordMetrics.java | 37 ------------------- 1 file changed, 37 deletions(-) diff --git a/core/java/android/app/admin/PasswordMetrics.java b/core/java/android/app/admin/PasswordMetrics.java index 9b64d8fe84b4d..93c0c8a4a47ae 100644 --- a/core/java/android/app/admin/PasswordMetrics.java +++ b/core/java/android/app/admin/PasswordMetrics.java @@ -135,17 +135,6 @@ public final class PasswordMetrics implements Parcelable { } } - private static boolean hasInvalidCharacters(byte[] password) { - // Allow non-control Latin-1 characters only. - for (byte b : password) { - char c = (char) b; - if (c < 32 || c > 127) { - return true; - } - } - return false; - } - @Override public int describeContents() { return 0; @@ -536,32 +525,6 @@ public final class PasswordMetrics implements Parcelable { return validatePasswordMetrics(adminMetrics, minComplexity, actualMetrics); } - /** - * 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) { - - if (hasInvalidCharacters(password)) { - return Collections.singletonList( - new PasswordValidationError(CONTAINS_INVALID_CHARACTERS, 0)); - } - - final PasswordMetrics enteredMetrics = computeForPasswordOrPin(password, isPin); - return validatePasswordMetrics(adminMetrics, minComplexity, enteredMetrics); - } - /** * Validates password metrics against minimum metrics and complexity *