From e38292a29b4b8dbc08828fd8e0118f82243c0d8a Mon Sep 17 00:00:00 2001 From: Eran Messeri Date: Mon, 9 Nov 2020 23:12:17 +0000 Subject: [PATCH] DPMS: Fix password complexity comparison The method for validating whether a given password is sufficient for a given complexity was returning the wrong value in the general case. validatePasswordMetrics should return true when the actualMetrics of a password satisfy the minComplexity passed in. However, the isPin argument, skews the result such that an all-digit PIN is considered not sufficient, because the only credential type recognized in that method is the "password" type, and for that credential type a non-digit character is required. This worked for checking whether the password is sufficient in the UI code, where isPin indicated whether the user chose a PIN or password. However it does not work for the general case, for example for enforcing complexity. This change gets rid of the 'allowsNumericPassword()' criteria for complexity buckets. Instead, the minimal length is calculated based on whether the actualMetrics already have a non-numeric character, OR a non-numeric character is required by the admin-set password quality requirements (in the case when merging complexity and quality). Bug: 172312413 Bug: 165573442 Test: atest FrameworksServicesTests:DevicePolicyManagerTest Test: atest MixedDeviceOwnerTest MixedProfileOwnerTest OrgOwnedProfileOwnerTest MixedDeviceOwnerTestApi25 MixedManagedProfileOwnerTest Change-Id: I3227d4d8e6825b5c4ea525828d7e09f52702065b --- .../android/app/admin/PasswordMetrics.java | 46 +++++++------------ 1 file changed, 16 insertions(+), 30 deletions(-) diff --git a/core/java/android/app/admin/PasswordMetrics.java b/core/java/android/app/admin/PasswordMetrics.java index 20a60bbe09740..c95a74e0f333c 100644 --- a/core/java/android/app/admin/PasswordMetrics.java +++ b/core/java/android/app/admin/PasswordMetrics.java @@ -406,11 +406,6 @@ public final class PasswordMetrics implements Parcelable { return containsNonNumeric ? 6 : 8; } - @Override - boolean allowsNumericPassword() { - return false; - } - @Override boolean allowsCredType(int credType) { return credType == CREDENTIAL_TYPE_PASSWORD; @@ -427,11 +422,6 @@ public final class PasswordMetrics implements Parcelable { return 4; } - @Override - boolean allowsNumericPassword() { - return false; - } - @Override boolean allowsCredType(int credType) { return credType == CREDENTIAL_TYPE_PASSWORD; @@ -448,11 +438,6 @@ public final class PasswordMetrics implements Parcelable { return 0; } - @Override - boolean allowsNumericPassword() { - return true; - } - @Override boolean allowsCredType(int credType) { return credType != CREDENTIAL_TYPE_NONE; @@ -469,11 +454,6 @@ public final class PasswordMetrics implements Parcelable { return 0; } - @Override - boolean allowsNumericPassword() { - return true; - } - @Override boolean allowsCredType(int credType) { return true; @@ -484,7 +464,6 @@ public final class PasswordMetrics implements Parcelable { abstract boolean canHaveSequence(); abstract int getMinimumLength(boolean containsNonNumeric); - abstract boolean allowsNumericPassword(); abstract boolean allowsCredType(int credType); ComplexityBucket(int complexityLevel) { @@ -591,7 +570,14 @@ public final class PasswordMetrics implements Parcelable { result.add(new PasswordValidationError(TOO_LONG, MAX_PASSWORD_LENGTH)); } - final PasswordMetrics minMetrics = applyComplexity(adminMetrics, isPin, bucket); + // A flag indicating whether the provided password already has non-numeric characters in + // it or if the admin imposes the requirement of any non-numeric characters. + final boolean hasOrWouldNeedNonNumeric = + actualMetrics.nonNumeric > 0 || adminMetrics.nonNumeric > 0 + || adminMetrics.letters > 0 || adminMetrics.lowerCase > 0 + || adminMetrics.upperCase > 0 || adminMetrics.symbols > 0; + final PasswordMetrics minMetrics = + applyComplexity(adminMetrics, hasOrWouldNeedNonNumeric, bucket); // Clamp required length between maximum and minimum valid values. minMetrics.length = Math.min(MAX_PASSWORD_LENGTH, @@ -684,23 +670,23 @@ public final class PasswordMetrics implements Parcelable { * TODO: move to PasswordPolicy */ public static PasswordMetrics applyComplexity( - PasswordMetrics adminMetrics, boolean isPin, int complexity) { - return applyComplexity(adminMetrics, isPin, ComplexityBucket.forComplexity(complexity)); + PasswordMetrics adminMetrics, boolean withNonNumericCharacters, + int complexity) { + return applyComplexity(adminMetrics, withNonNumericCharacters, + ComplexityBucket.forComplexity(complexity)); } private static PasswordMetrics applyComplexity( - PasswordMetrics adminMetrics, boolean isPin, ComplexityBucket bucket) { + PasswordMetrics adminMetrics, boolean withNonNumericCharacters, + ComplexityBucket bucket) { final PasswordMetrics minMetrics = new PasswordMetrics(adminMetrics); if (!bucket.canHaveSequence()) { minMetrics.seqLength = Math.min(minMetrics.seqLength, MAX_ALLOWED_SEQUENCE); } - minMetrics.length = Math.max(minMetrics.length, bucket.getMinimumLength(!isPin)); - - if (!isPin && !bucket.allowsNumericPassword()) { - minMetrics.nonNumeric = Math.max(minMetrics.nonNumeric, 1); - } + minMetrics.length = Math.max(minMetrics.length, + bucket.getMinimumLength(withNonNumericCharacters)); return minMetrics; }