From 3fd4388757f6568db3a1041b4a40aacfd9ed5e12 Mon Sep 17 00:00:00 2001 From: Alex Johnston Date: Thu, 8 Apr 2021 15:51:22 +0000 Subject: [PATCH] PasswordPolicy.getMinMetrics PIN * If the password quality is NUMERIC or NUMERIC_COMPLEX, the credential type should be CREDENTIAL_TYPE_PIN Bug: 184103377 Test: atest com.android.server.devicepolicy.PasswordPolicyTest atest android.app.admin.PasswordMetricsTest atest com.android.cts.devicepolicy.MixedManagedProfileOwnerTest atest com.android.server.devicepolicy.DevicePolicyManagerTest atest com.android.cts.devicepolicy.ManagedProfilePasswordTest make RunSettingsRoboTests -j ROBOTEST_FILTER=com.android.settings.password CtsVerifier > Device owner tests > policy transparency test > set password quality Change-Id: Ibad35d22c34b6cd9ed34b2e671b38503becd7da5 --- .../android/app/admin/PasswordMetrics.java | 37 ++++++++++--------- .../android/app/admin/PasswordPolicy.java | 16 ++++++-- .../app/admin/PasswordMetricsTest.java | 36 +++++++++++------- .../android/app/admin/PasswordPolicyTest.java | 5 ++- .../devicepolicy/DevicePolicyManagerTest.java | 22 ++++++----- 5 files changed, 70 insertions(+), 46 deletions(-) diff --git a/core/java/android/app/admin/PasswordMetrics.java b/core/java/android/app/admin/PasswordMetrics.java index c95a74e0f333c..92375bf0b7a21 100644 --- a/core/java/android/app/admin/PasswordMetrics.java +++ b/core/java/android/app/admin/PasswordMetrics.java @@ -28,6 +28,7 @@ import static android.app.admin.DevicePolicyManager.PASSWORD_QUALITY_UNSPECIFIED import static com.android.internal.widget.LockPatternUtils.CREDENTIAL_TYPE_NONE; import static com.android.internal.widget.LockPatternUtils.CREDENTIAL_TYPE_PASSWORD; 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.PasswordValidationError.CONTAINS_INVALID_CHARACTERS; import static com.android.internal.widget.PasswordValidationError.CONTAINS_SEQUENCE; @@ -74,11 +75,8 @@ public final class PasswordMetrics implements Parcelable { // consider it a complex PIN/password. public static final int MAX_ALLOWED_SEQUENCE = 3; - // One of CREDENTIAL_TYPE_NONE, CREDENTIAL_TYPE_PATTERN or CREDENTIAL_TYPE_PASSWORD. - // Note that this class still uses CREDENTIAL_TYPE_PASSWORD to represent both numeric PIN - // and alphabetic password. This is OK as long as this definition is only used internally, - // and the value never gets mixed up with credential types from other parts of the framework. - // TODO: fix this (ideally after we move logic to PasswordPolicy) + // One of CREDENTIAL_TYPE_NONE, CREDENTIAL_TYPE_PATTERN, CREDENTIAL_TYPE_PIN or + // CREDENTIAL_TYPE_PASSWORD. public @CredentialType int credType; // Fields below only make sense when credType is PASSWORD. public int length = 0; @@ -192,13 +190,15 @@ public final class PasswordMetrics implements Parcelable { /** * Returns the {@code PasswordMetrics} for a given credential. * - * If the credential is a pin or a password, equivalent to {@link #computeForPassword(byte[])}. - * {@code credential} cannot be null when {@code type} is + * If the credential is a pin or a password, equivalent to + * {@link #computeForPasswordOrPin(byte[], boolean)}. {@code credential} cannot be null + * when {@code type} is * {@link com.android.internal.widget.LockPatternUtils#CREDENTIAL_TYPE_PASSWORD}. */ public static PasswordMetrics computeForCredential(LockscreenCredential credential) { if (credential.isPassword() || credential.isPin()) { - return PasswordMetrics.computeForPassword(credential.getCredential()); + return PasswordMetrics.computeForPasswordOrPin(credential.getCredential(), + credential.isPin()); } else if (credential.isPattern()) { return new PasswordMetrics(CREDENTIAL_TYPE_PATTERN); } else if (credential.isNone()) { @@ -209,9 +209,9 @@ public final class PasswordMetrics implements Parcelable { } /** - * Returns the {@code PasswordMetrics} for a given password + * Returns the {@code PasswordMetrics} for a given password or pin */ - public static PasswordMetrics computeForPassword(@NonNull byte[] password) { + public static PasswordMetrics computeForPasswordOrPin(byte[] password, boolean isPin) { // Analyse the characters used int letters = 0; int upperCase = 0; @@ -245,8 +245,9 @@ public final class PasswordMetrics implements Parcelable { } } + final int credType = isPin ? CREDENTIAL_TYPE_PIN : CREDENTIAL_TYPE_PASSWORD; final int seqLength = maxLengthSequence(password); - return new PasswordMetrics(CREDENTIAL_TYPE_PASSWORD, length, letters, upperCase, lowerCase, + return new PasswordMetrics(credType, length, letters, upperCase, lowerCase, numeric, symbols, nonLetter, nonNumeric, seqLength); } @@ -353,7 +354,7 @@ public final class PasswordMetrics implements Parcelable { */ public void maxWith(PasswordMetrics other) { credType = Math.max(credType, other.credType); - if (credType != CREDENTIAL_TYPE_PASSWORD) { + if (credType != CREDENTIAL_TYPE_PASSWORD && credType != CREDENTIAL_TYPE_PIN) { return; } length = Math.max(length, other.length); @@ -408,7 +409,7 @@ public final class PasswordMetrics implements Parcelable { @Override boolean allowsCredType(int credType) { - return credType == CREDENTIAL_TYPE_PASSWORD; + return credType == CREDENTIAL_TYPE_PASSWORD || credType == CREDENTIAL_TYPE_PIN; } }, BUCKET_MEDIUM(PASSWORD_COMPLEXITY_MEDIUM) { @@ -424,7 +425,7 @@ public final class PasswordMetrics implements Parcelable { @Override boolean allowsCredType(int credType) { - return credType == CREDENTIAL_TYPE_PASSWORD; + return credType == CREDENTIAL_TYPE_PASSWORD || credType == CREDENTIAL_TYPE_PIN; } }, BUCKET_LOW(PASSWORD_COMPLEXITY_LOW) { @@ -489,7 +490,7 @@ public final class PasswordMetrics implements Parcelable { if (!bucket.allowsCredType(credType)) { return false; } - if (credType != CREDENTIAL_TYPE_PASSWORD) { + if (credType != CREDENTIAL_TYPE_PASSWORD && credType != CREDENTIAL_TYPE_PIN) { return true; } return (bucket.canHaveSequence() || seqLength <= MAX_ALLOWED_SEQUENCE) @@ -529,7 +530,7 @@ public final class PasswordMetrics implements Parcelable { new PasswordValidationError(CONTAINS_INVALID_CHARACTERS, 0)); } - final PasswordMetrics enteredMetrics = computeForPassword(password); + final PasswordMetrics enteredMetrics = computeForPasswordOrPin(password, isPin); return validatePasswordMetrics(adminMetrics, minComplexity, isPin, enteredMetrics); } @@ -555,8 +556,8 @@ public final class PasswordMetrics implements Parcelable { || !bucket.allowsCredType(actualMetrics.credType)) { return Collections.singletonList(new PasswordValidationError(WEAK_CREDENTIAL_TYPE, 0)); } - // TODO: this needs to be modified if CREDENTIAL_TYPE_PIN is added. - if (actualMetrics.credType != CREDENTIAL_TYPE_PASSWORD) { + if (actualMetrics.credType != CREDENTIAL_TYPE_PASSWORD + && actualMetrics.credType != CREDENTIAL_TYPE_PIN) { return Collections.emptyList(); // Nothing to check for pattern or none. } diff --git a/core/java/android/app/admin/PasswordPolicy.java b/core/java/android/app/admin/PasswordPolicy.java index 13f11ad74d127..0544a36666966 100644 --- a/core/java/android/app/admin/PasswordPolicy.java +++ b/core/java/android/app/admin/PasswordPolicy.java @@ -20,6 +20,7 @@ import static android.app.admin.DevicePolicyManager.PASSWORD_QUALITY_ALPHABETIC; import static android.app.admin.DevicePolicyManager.PASSWORD_QUALITY_ALPHANUMERIC; import static android.app.admin.DevicePolicyManager.PASSWORD_QUALITY_BIOMETRIC_WEAK; import static android.app.admin.DevicePolicyManager.PASSWORD_QUALITY_COMPLEX; +import static android.app.admin.DevicePolicyManager.PASSWORD_QUALITY_NUMERIC; import static android.app.admin.DevicePolicyManager.PASSWORD_QUALITY_NUMERIC_COMPLEX; import static android.app.admin.DevicePolicyManager.PASSWORD_QUALITY_SOMETHING; import static android.app.admin.DevicePolicyManager.PASSWORD_QUALITY_UNSPECIFIED; @@ -27,6 +28,7 @@ import static android.app.admin.DevicePolicyManager.PASSWORD_QUALITY_UNSPECIFIED import static com.android.internal.widget.LockPatternUtils.CREDENTIAL_TYPE_NONE; import static com.android.internal.widget.LockPatternUtils.CREDENTIAL_TYPE_PASSWORD; import static com.android.internal.widget.LockPatternUtils.CREDENTIAL_TYPE_PATTERN; +import static com.android.internal.widget.LockPatternUtils.CREDENTIAL_TYPE_PIN; /** * {@hide} @@ -58,14 +60,20 @@ public class PasswordPolicy { } else if (quality == PASSWORD_QUALITY_BIOMETRIC_WEAK || quality == PASSWORD_QUALITY_SOMETHING) { return new PasswordMetrics(CREDENTIAL_TYPE_PATTERN); - } // quality is NUMERIC or stronger. + } else if (quality == PASSWORD_QUALITY_NUMERIC + || quality == PASSWORD_QUALITY_NUMERIC_COMPLEX) { + PasswordMetrics result = new PasswordMetrics(CREDENTIAL_TYPE_PIN); + result.length = length; + if (quality == PASSWORD_QUALITY_NUMERIC_COMPLEX) { + result.seqLength = PasswordMetrics.MAX_ALLOWED_SEQUENCE; + } + return result; + } // quality is ALPHABETIC or stronger. PasswordMetrics result = new PasswordMetrics(CREDENTIAL_TYPE_PASSWORD); result.length = length; - if (quality == PASSWORD_QUALITY_NUMERIC_COMPLEX) { - result.seqLength = PasswordMetrics.MAX_ALLOWED_SEQUENCE; - } else if (quality == PASSWORD_QUALITY_ALPHABETIC) { + if (quality == PASSWORD_QUALITY_ALPHABETIC) { result.nonNumeric = 1; } else if (quality == PASSWORD_QUALITY_ALPHANUMERIC) { result.numeric = 1; diff --git a/core/tests/coretests/src/android/app/admin/PasswordMetricsTest.java b/core/tests/coretests/src/android/app/admin/PasswordMetricsTest.java index fb0dd46a52f00..b2b9ab31282ca 100644 --- a/core/tests/coretests/src/android/app/admin/PasswordMetricsTest.java +++ b/core/tests/coretests/src/android/app/admin/PasswordMetricsTest.java @@ -93,8 +93,8 @@ public class PasswordMetricsTest { @Test public void testComputeForPassword_metrics() { - final PasswordMetrics metrics = - PasswordMetrics.computeForPassword("6B~0z1Z3*8A".getBytes()); + final PasswordMetrics metrics = PasswordMetrics.computeForPasswordOrPin( + "6B~0z1Z3*8A".getBytes(), /* isPin */ false); assertEquals(11, metrics.length); assertEquals(4, metrics.letters); assertEquals(3, metrics.upperCase); @@ -133,61 +133,71 @@ public class PasswordMetricsTest { @Test public void testDetermineComplexity_lowNumeric() { assertEquals(PASSWORD_COMPLEXITY_LOW, - PasswordMetrics.computeForPassword("1234".getBytes()).determineComplexity()); + PasswordMetrics.computeForPasswordOrPin("1234".getBytes(), + /* isPin */true).determineComplexity()); } @Test public void testDetermineComplexity_lowNumericComplex() { assertEquals(PASSWORD_COMPLEXITY_LOW, - PasswordMetrics.computeForPassword("124".getBytes()).determineComplexity()); + PasswordMetrics.computeForPasswordOrPin("124".getBytes(), + /* isPin */ true).determineComplexity()); } @Test public void testDetermineComplexity_lowAlphabetic() { assertEquals(PASSWORD_COMPLEXITY_LOW, - PasswordMetrics.computeForPassword("a!".getBytes()).determineComplexity()); + PasswordMetrics.computeForPasswordOrPin("a!".getBytes(), + /* isPin */ false).determineComplexity()); } @Test public void testDetermineComplexity_lowAlphanumeric() { assertEquals(PASSWORD_COMPLEXITY_LOW, - PasswordMetrics.computeForPassword("a!1".getBytes()).determineComplexity()); + PasswordMetrics.computeForPasswordOrPin("a!1".getBytes(), + /* isPin */ false).determineComplexity()); } @Test public void testDetermineComplexity_mediumNumericComplex() { assertEquals(PASSWORD_COMPLEXITY_MEDIUM, - PasswordMetrics.computeForPassword("1238".getBytes()).determineComplexity()); + PasswordMetrics.computeForPasswordOrPin("1238".getBytes(), + /* isPin */ true).determineComplexity()); } @Test public void testDetermineComplexity_mediumAlphabetic() { assertEquals(PASSWORD_COMPLEXITY_MEDIUM, - PasswordMetrics.computeForPassword("ab!c".getBytes()).determineComplexity()); + PasswordMetrics.computeForPasswordOrPin("ab!c".getBytes(), + /* isPin */ false).determineComplexity()); } @Test public void testDetermineComplexity_mediumAlphanumeric() { assertEquals(PASSWORD_COMPLEXITY_MEDIUM, - PasswordMetrics.computeForPassword("ab!1".getBytes()).determineComplexity()); + PasswordMetrics.computeForPasswordOrPin("ab!1".getBytes(), + /* isPin */ false).determineComplexity()); } @Test public void testDetermineComplexity_highNumericComplex() { assertEquals(PASSWORD_COMPLEXITY_HIGH, - PasswordMetrics.computeForPassword("12389647!".getBytes()).determineComplexity()); + PasswordMetrics.computeForPasswordOrPin("12389647!".getBytes(), + /* isPin */ true).determineComplexity()); } @Test public void testDetermineComplexity_highAlphabetic() { assertEquals(PASSWORD_COMPLEXITY_HIGH, - PasswordMetrics.computeForPassword("alphabetic!".getBytes()).determineComplexity()); + PasswordMetrics.computeForPasswordOrPin("alphabetic!".getBytes(), + /* isPin */ false).determineComplexity()); } @Test public void testDetermineComplexity_highAlphanumeric() { - assertEquals(PASSWORD_COMPLEXITY_HIGH, PasswordMetrics.computeForPassword( - "alphanumeric123!".getBytes()).determineComplexity()); + assertEquals(PASSWORD_COMPLEXITY_HIGH, + PasswordMetrics.computeForPasswordOrPin("alphanumeric123!".getBytes(), + /* isPin */ false).determineComplexity()); } @Test diff --git a/core/tests/coretests/src/android/app/admin/PasswordPolicyTest.java b/core/tests/coretests/src/android/app/admin/PasswordPolicyTest.java index e951054e6558d..f1be173b3677b 100644 --- a/core/tests/coretests/src/android/app/admin/PasswordPolicyTest.java +++ b/core/tests/coretests/src/android/app/admin/PasswordPolicyTest.java @@ -28,6 +28,7 @@ import static android.app.admin.DevicePolicyManager.PASSWORD_QUALITY_UNSPECIFIED import static com.android.internal.widget.LockPatternUtils.CREDENTIAL_TYPE_NONE; import static com.android.internal.widget.LockPatternUtils.CREDENTIAL_TYPE_PASSWORD; import static com.android.internal.widget.LockPatternUtils.CREDENTIAL_TYPE_PATTERN; +import static com.android.internal.widget.LockPatternUtils.CREDENTIAL_TYPE_PIN; import static org.junit.Assert.assertEquals; @@ -80,7 +81,7 @@ public class PasswordPolicyTest { public void testGetMinMetrics_numeric() { PasswordPolicy policy = testPolicy(PASSWORD_QUALITY_NUMERIC); PasswordMetrics minMetrics = policy.getMinMetrics(); - assertEquals(CREDENTIAL_TYPE_PASSWORD, minMetrics.credType); + assertEquals(CREDENTIAL_TYPE_PIN, minMetrics.credType); assertEquals(TEST_VALUE, minMetrics.length); assertEquals(0, minMetrics.numeric); // numeric can doesn't really require digits. assertEquals(0, minMetrics.letters); @@ -104,7 +105,7 @@ public class PasswordPolicyTest { public void testGetMinMetrics_numericComplex() { PasswordPolicy policy = testPolicy(PASSWORD_QUALITY_NUMERIC_COMPLEX); PasswordMetrics minMetrics = policy.getMinMetrics(); - assertEquals(CREDENTIAL_TYPE_PASSWORD, minMetrics.credType); + assertEquals(CREDENTIAL_TYPE_PIN, minMetrics.credType); assertEquals(TEST_VALUE, minMetrics.length); assertEquals(0, minMetrics.numeric); assertEquals(0, minMetrics.letters); diff --git a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java index 6113e4c8ff0fc..94adefdd12485 100644 --- a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java @@ -31,7 +31,7 @@ import static android.app.admin.DevicePolicyManager.PASSWORD_COMPLEXITY_LOW; import static android.app.admin.DevicePolicyManager.PASSWORD_COMPLEXITY_MEDIUM; import static android.app.admin.DevicePolicyManager.PASSWORD_COMPLEXITY_NONE; import static android.app.admin.DevicePolicyManager.WIPE_EUICC; -import static android.app.admin.PasswordMetrics.computeForPassword; +import static android.app.admin.PasswordMetrics.computeForPasswordOrPin; import static android.content.pm.ApplicationInfo.PRIVATE_FLAG_DIRECT_BOOT_AWARE; import static android.net.InetAddresses.parseNumericAddress; @@ -5103,7 +5103,8 @@ public class DevicePolicyManagerTest extends DpmTestBase { reset(mContext.spiedContext); - PasswordMetrics passwordMetricsNoSymbols = computeForPassword("abcdXYZ5".getBytes()); + PasswordMetrics passwordMetricsNoSymbols = computeForPasswordOrPin( + "abcdXYZ5".getBytes(), /* isPin */ false); setActivePasswordState(passwordMetricsNoSymbols); assertThat(dpm.isActivePasswordSufficient()).isTrue(); @@ -5130,7 +5131,8 @@ public class DevicePolicyManagerTest extends DpmTestBase { reset(mContext.spiedContext); assertThat(dpm.isActivePasswordSufficient()).isFalse(); - PasswordMetrics passwordMetricsWithSymbols = computeForPassword("abcd.XY5".getBytes()); + PasswordMetrics passwordMetricsWithSymbols = computeForPasswordOrPin( + "abcd.XY5".getBytes(), /* isPin */ false); setActivePasswordState(passwordMetricsWithSymbols); assertThat(dpm.isActivePasswordSufficient()).isTrue(); @@ -5184,7 +5186,7 @@ public class DevicePolicyManagerTest extends DpmTestBase { parentDpm.setRequiredPasswordComplexity(PASSWORD_COMPLEXITY_MEDIUM); when(getServices().lockSettingsInternal.getUserPasswordMetrics(UserHandle.USER_SYSTEM)) - .thenReturn(computeForPassword("184342".getBytes())); + .thenReturn(computeForPasswordOrPin("184342".getBytes(), /* isPin */ true)); // Numeric password is compliant with current requirement (QUALITY_NUMERIC set explicitly // on the parent admin) @@ -6307,7 +6309,7 @@ public class DevicePolicyManagerTest extends DpmTestBase { .thenReturn(CALLER_USER_HANDLE); when(getServices().lockSettingsInternal .getUserPasswordMetrics(CALLER_USER_HANDLE)) - .thenReturn(computeForPassword("asdf".getBytes())); + .thenReturn(computeForPasswordOrPin("asdf".getBytes(), /* isPin */ false)); assertThat(dpm.getPasswordComplexity()).isEqualTo(PASSWORD_COMPLEXITY_MEDIUM); } @@ -6327,10 +6329,10 @@ public class DevicePolicyManagerTest extends DpmTestBase { when(getServices().lockSettingsInternal .getUserPasswordMetrics(CALLER_USER_HANDLE)) - .thenReturn(computeForPassword("asdf".getBytes())); + .thenReturn(computeForPasswordOrPin("asdf".getBytes(), /* isPin */ false)); when(getServices().lockSettingsInternal .getUserPasswordMetrics(parentUser.id)) - .thenReturn(computeForPassword("parentUser".getBytes())); + .thenReturn(computeForPasswordOrPin("parentUser".getBytes(), /* isPin */ false)); assertThat(dpm.getPasswordComplexity()).isEqualTo(PASSWORD_COMPLEXITY_HIGH); } @@ -7006,13 +7008,15 @@ public class DevicePolicyManagerTest extends DpmTestBase { assertThat(dpm.getPasswordComplexity()).isEqualTo(PASSWORD_COMPLEXITY_NONE); reset(mContext.spiedContext); - PasswordMetrics passwordMetricsNoSymbols = computeForPassword("1234".getBytes()); + PasswordMetrics passwordMetricsNoSymbols = computeForPasswordOrPin( + "1234".getBytes(), /* isPin */ true); setActivePasswordState(passwordMetricsNoSymbols); assertThat(dpm.getPasswordComplexity()).isEqualTo(PASSWORD_COMPLEXITY_LOW); assertThat(dpm.isActivePasswordSufficient()).isFalse(); reset(mContext.spiedContext); - passwordMetricsNoSymbols = computeForPassword("84125312943a".getBytes()); + passwordMetricsNoSymbols = computeForPasswordOrPin( + "84125312943a".getBytes(), /* isPin */ false); setActivePasswordState(passwordMetricsNoSymbols); assertThat(dpm.getPasswordComplexity()).isEqualTo(PASSWORD_COMPLEXITY_HIGH); // using isActivePasswordSufficient