From b63430f2dae18fc34adf8cd2949906e8937d851f Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Wed, 21 Jun 2023 05:28:08 +0000 Subject: [PATCH 1/5] Convert LockscreenCredentialTest to JUnit4 Convert LockscreenCredentialTest from the JUnit3-based AndroidTestCase, which is deprecated, to JUnit4. Bug: 219511761 Bug: 232900169 Bug: 243881358 Test: atest LockscreenCredentialTest Change-Id: Id8da1606b1f758ff91f67c25d37a36f36a72c2bf --- .../widget/LockscreenCredentialTest.java | 37 +++++++++++++++---- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/core/tests/coretests/src/com/android/internal/widget/LockscreenCredentialTest.java b/core/tests/coretests/src/com/android/internal/widget/LockscreenCredentialTest.java index a47868d0a5243..63d1a876bf40e 100644 --- a/core/tests/coretests/src/com/android/internal/widget/LockscreenCredentialTest.java +++ b/core/tests/coretests/src/com/android/internal/widget/LockscreenCredentialTest.java @@ -16,16 +16,27 @@ package com.android.internal.widget; - import static com.google.common.truth.Truth.assertThat; -import android.test.AndroidTestCase; +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import androidx.test.ext.junit.runners.AndroidJUnit4; + +import org.junit.Test; +import org.junit.runner.RunWith; import java.util.Arrays; +@RunWith(AndroidJUnit4.class) +public class LockscreenCredentialTest { -public class LockscreenCredentialTest extends AndroidTestCase { - + @Test public void testEmptyCredential() { LockscreenCredential empty = LockscreenCredential.createNone(); @@ -38,30 +49,33 @@ public class LockscreenCredentialTest extends AndroidTestCase { assertFalse(empty.isPattern()); } + @Test public void testPinCredential() { LockscreenCredential pin = LockscreenCredential.createPin("3456"); assertTrue(pin.isPin()); assertEquals(4, pin.size()); - assertTrue(Arrays.equals("3456".getBytes(), pin.getCredential())); + assertArrayEquals("3456".getBytes(), pin.getCredential()); assertFalse(pin.isNone()); assertFalse(pin.isPassword()); assertFalse(pin.isPattern()); } + @Test public void testPasswordCredential() { LockscreenCredential password = LockscreenCredential.createPassword("password"); assertTrue(password.isPassword()); assertEquals(8, password.size()); - assertTrue(Arrays.equals("password".getBytes(), password.getCredential())); + assertArrayEquals("password".getBytes(), password.getCredential()); assertFalse(password.isNone()); assertFalse(password.isPin()); assertFalse(password.isPattern()); } + @Test public void testPatternCredential() { LockscreenCredential pattern = LockscreenCredential.createPattern(Arrays.asList( LockPatternView.Cell.of(0, 0), @@ -73,13 +87,14 @@ public class LockscreenCredentialTest extends AndroidTestCase { assertTrue(pattern.isPattern()); assertEquals(5, pattern.size()); - assertTrue(Arrays.equals("12369".getBytes(), pattern.getCredential())); + assertArrayEquals("12369".getBytes(), pattern.getCredential()); assertFalse(pattern.isNone()); assertFalse(pattern.isPin()); assertFalse(pattern.isPassword()); } + @Test public void testPasswordOrNoneCredential() { assertEquals(LockscreenCredential.createNone(), LockscreenCredential.createPasswordOrNone(null)); @@ -89,6 +104,7 @@ public class LockscreenCredentialTest extends AndroidTestCase { LockscreenCredential.createPasswordOrNone("abcd")); } + @Test public void testPinOrNoneCredential() { assertEquals(LockscreenCredential.createNone(), LockscreenCredential.createPinOrNone(null)); @@ -98,6 +114,7 @@ public class LockscreenCredentialTest extends AndroidTestCase { LockscreenCredential.createPinOrNone("1357")); } + @Test public void testSanitize() { LockscreenCredential password = LockscreenCredential.createPassword("password"); password.zeroize(); @@ -128,6 +145,7 @@ public class LockscreenCredentialTest extends AndroidTestCase { } catch (IllegalStateException expected) { } } + @Test public void testEquals() { assertEquals(LockscreenCredential.createNone(), LockscreenCredential.createNone()); assertEquals(LockscreenCredential.createPassword("1234"), @@ -164,6 +182,7 @@ public class LockscreenCredentialTest extends AndroidTestCase { LockscreenCredential.createPin("5678")); } + @Test public void testDuplicate() { LockscreenCredential credential; @@ -177,6 +196,7 @@ public class LockscreenCredentialTest extends AndroidTestCase { assertEquals(credential, credential.duplicate()); } + @Test public void testPasswordToHistoryHash() { String password = "1234"; LockscreenCredential credential = LockscreenCredential.createPassword(password); @@ -193,6 +213,7 @@ public class LockscreenCredentialTest extends AndroidTestCase { .isEqualTo(expectedHash); } + @Test public void testPasswordToHistoryHashInvalidInput() { String password = "1234"; LockscreenCredential credential = LockscreenCredential.createPassword(password); @@ -221,6 +242,7 @@ public class LockscreenCredentialTest extends AndroidTestCase { .isNull(); } + @Test public void testLegacyPasswordToHash() { String password = "1234"; String salt = "6d5331dd120077a0"; @@ -233,6 +255,7 @@ public class LockscreenCredentialTest extends AndroidTestCase { .isEqualTo(expectedHash); } + @Test public void testLegacyPasswordToHashInvalidInput() { String password = "1234"; String salt = "6d5331dd120077a0"; From 8066a758fefa847b1a6fdb874ea63c7814e05abc Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Wed, 21 Jun 2023 05:28:09 +0000 Subject: [PATCH 2/5] Allow LockscreenCredential to represent any proposed credential Currently the ChooseLockPassword activity in Settings creates a LockscreenCredential object for the text the user has entered, every time a character is added or deleted to/from the text view. It then uses this LockscreenCredential to validate that requirements such as minimum length have been met. Thus, LockscreenCredential is expected to be able to represent any proposed credential, even an invalid one. Yet, currently the constructor of LockscreenCredential throws an exception if the length is 0 and the credential type is not NONE. For the empty text case, ChooseLockPassword currently works around this by constructing a LockscreenCredential of type NONE and then actually treating it as a PIN or PASSWORD when validating it. To make it possible to fix the bug where invalid characters are being allowed in passwords, I'm adding a new validation method that operates on LockscreenCredential. For ChooseLockPassword to be converted to this, though, it needs to start constructing a LockscreenCredential with the correct type for the proposed credential. Therefore, this CL allows non-none LockscreenCredential objects to be constructed with length 0. Bug: 219511761 Bug: 232900169 Bug: 243881358 Test: atest LockscreenCredentialTest Test: atest com.android.server.locksettings Change-Id: I6b951759aa7fabacc46c20bbfd31d3a0dafa4c1c --- .../internal/widget/LockscreenCredential.java | 18 ++++++---- .../widget/LockscreenCredentialTest.java | 35 ++++++++++++++----- 2 files changed, 38 insertions(+), 15 deletions(-) diff --git a/core/java/com/android/internal/widget/LockscreenCredential.java b/core/java/com/android/internal/widget/LockscreenCredential.java index 03e7fd1c74035..6e63f790ab982 100644 --- a/core/java/com/android/internal/widget/LockscreenCredential.java +++ b/core/java/com/android/internal/widget/LockscreenCredential.java @@ -60,7 +60,7 @@ import java.util.Objects; public class LockscreenCredential implements Parcelable, AutoCloseable { private final int mType; - // Stores raw credential bytes, or null if credential has been zeroized. An empty password + // Stores raw credential bytes, or null if credential has been zeroized. A none credential // is represented as a byte array of length 0. private byte[] mCredential; @@ -80,14 +80,20 @@ public class LockscreenCredential implements Parcelable, AutoCloseable { Preconditions.checkArgument(type == CREDENTIAL_TYPE_PIN || type == CREDENTIAL_TYPE_PASSWORD || type == CREDENTIAL_TYPE_PATTERN); - Preconditions.checkArgument(credential.length > 0); + // Do not validate credential.length yet. All non-none credentials have a minimum + // length requirement; however, one of the uses of LockscreenCredential is to represent + // a proposed credential that might be too short. For example, a LockscreenCredential + // with type CREDENTIAL_TYPE_PIN and length 0 represents an attempt to set an empty PIN. + // This differs from an actual attempt to set a none credential. We have to allow the + // LockscreenCredential object to be constructed so that the validation logic can run, + // even though the validation logic will ultimately reject the credential as too short. } mType = type; mCredential = credential; } /** - * Creates a LockscreenCredential object representing empty password. + * Creates a LockscreenCredential object representing a none credential. */ public static LockscreenCredential createNone() { return new LockscreenCredential(CREDENTIAL_TYPE_NONE, new byte[0]); @@ -130,7 +136,7 @@ public class LockscreenCredential implements Parcelable, AutoCloseable { /** * Creates a LockscreenCredential object representing the given alphabetic password. - * If the supplied password is empty, create an empty credential object. + * If the supplied password is empty, create a none credential object. */ public static LockscreenCredential createPasswordOrNone(@Nullable CharSequence password) { if (TextUtils.isEmpty(password)) { @@ -142,7 +148,7 @@ public class LockscreenCredential implements Parcelable, AutoCloseable { /** * Creates a LockscreenCredential object representing the given numeric PIN. - * If the supplied password is empty, create an empty credential object. + * If the supplied password is empty, create a none credential object. */ public static LockscreenCredential createPinOrNone(@Nullable CharSequence pin) { if (TextUtils.isEmpty(pin)) { @@ -175,7 +181,7 @@ public class LockscreenCredential implements Parcelable, AutoCloseable { return mCredential; } - /** Returns whether this is an empty credential */ + /** Returns whether this is a none credential */ public boolean isNone() { ensureNotZeroized(); return mType == CREDENTIAL_TYPE_NONE; diff --git a/core/tests/coretests/src/com/android/internal/widget/LockscreenCredentialTest.java b/core/tests/coretests/src/com/android/internal/widget/LockscreenCredentialTest.java index 63d1a876bf40e..434a8953b19f1 100644 --- a/core/tests/coretests/src/com/android/internal/widget/LockscreenCredentialTest.java +++ b/core/tests/coretests/src/com/android/internal/widget/LockscreenCredentialTest.java @@ -21,7 +21,6 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -37,16 +36,16 @@ import java.util.Arrays; public class LockscreenCredentialTest { @Test - public void testEmptyCredential() { - LockscreenCredential empty = LockscreenCredential.createNone(); + public void testNoneCredential() { + LockscreenCredential none = LockscreenCredential.createNone(); - assertTrue(empty.isNone()); - assertEquals(0, empty.size()); - assertNotNull(empty.getCredential()); + assertTrue(none.isNone()); + assertEquals(0, none.size()); + assertArrayEquals(new byte[0], none.getCredential()); - assertFalse(empty.isPin()); - assertFalse(empty.isPassword()); - assertFalse(empty.isPattern()); + assertFalse(none.isPin()); + assertFalse(none.isPassword()); + assertFalse(none.isPattern()); } @Test @@ -94,6 +93,24 @@ public class LockscreenCredentialTest { assertFalse(pattern.isPassword()); } + // Constructing a LockscreenCredential with a too-short length, even 0, should not throw an + // exception. This is because LockscreenCredential needs to be able to represent a request to + // set a credential that is too short. + @Test + public void testZeroLengthCredential() { + LockscreenCredential credential = LockscreenCredential.createPin(""); + assertTrue(credential.isPin()); + assertEquals(0, credential.size()); + + credential = createPattern(""); + assertTrue(credential.isPattern()); + assertEquals(0, credential.size()); + + credential = LockscreenCredential.createPassword(""); + assertTrue(credential.isPassword()); + assertEquals(0, credential.size()); + } + @Test public void testPasswordOrNoneCredential() { assertEquals(LockscreenCredential.createNone(), From d984e5fd6211e1b23c8847db4912ce89753e3e80 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Wed, 21 Jun 2023 05:28:10 +0000 Subject: [PATCH 3/5] Make LockscreenCredential remember whether it has invalid chars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit http://ag/6283443 ("Refactor passwords/pins/patterns to byte[]" in packages/apps/Settings), which went into Android 10, introduced a bug where the chars of the lockscreen password are truncated to bytes before the password is validated to contain only ASCII 32–127. This causes characters outside the intended range to be accepted. Specifically, any character U-XXXX where XXXX mod 256 is in 32–127 is accepted and is treated as equivalent to some ASCII character. This reduces the entropy of the password, but also it can make it impossible for the user to unlock the device after rebooting. This happens if the chosen password uses a character that can only be entered on a third-party keyboard (IME) that is not direct boot aware or was uninstalled later. (The potential dependence on a third-party keyboard is one of the reasons that non-ASCII characters were never intended to be allowed in lockscreen passwords in the first place.) Unfortunately, it's likely that some users managed to set a password containing non-ASCII character(s) and are happily using it. To allow fixing this bug without locking out such users, this CL updates LockscreenCredential to keep track of whether it was instantiated using any invalid characters or not, while still keeping the truncation bug in place. Later CLs will use this "invalid chars" flag to reject new passwords that contain any invalid characters. Bug: 219511761 Bug: 232900169 Bug: 243881358 Test: atest LockscreenCredentialTest Test: atest com.android.server.locksettings Change-Id: I5c3c55367c3a294578cd0f97ac0e315a11ed517e --- .../internal/widget/LockscreenCredential.java | 88 +++++++++++++++---- .../widget/LockscreenCredentialTest.java | 57 +++++++++--- 2 files changed, 114 insertions(+), 31 deletions(-) diff --git a/core/java/com/android/internal/widget/LockscreenCredential.java b/core/java/com/android/internal/widget/LockscreenCredential.java index 6e63f790ab982..6bc1a43b8c206 100644 --- a/core/java/com/android/internal/widget/LockscreenCredential.java +++ b/core/java/com/android/internal/widget/LockscreenCredential.java @@ -64,6 +64,20 @@ public class LockscreenCredential implements Parcelable, AutoCloseable { // is represented as a byte array of length 0. private byte[] mCredential; + // This indicates that the credential is a password that used characters outside ASCII 32–127. + // + // Such passwords were never intended to be allowed. However, Android 10–14 had a bug where + // conversion from the chars the user entered to the credential bytes used a simple truncation. + // Thus, any 'char' whose remainder mod 256 was in the range 32–127 was accepted and was + // equivalent to some ASCII character. For example, ™, which is U+2122, was truncated to ASCII + // 0x22 which is the double-quote character ". + // + // We have to continue to allow a LockscreenCredential to be constructed with this bug, so that + // existing devices can be unlocked if their password used this bug. However, we prevent new + // passwords that use this bug from being set. The boolean below keeps track of the information + // needed to do that check, since the conversion to mCredential may have been lossy. + private final boolean mHasInvalidChars; + /** * Private constructor, use static builder methods instead. * @@ -71,7 +85,7 @@ public class LockscreenCredential implements Parcelable, AutoCloseable { * LockscreenCredential will only store the reference internally without copying. This is to * minimize the number of extra copies introduced. */ - private LockscreenCredential(int type, byte[] credential) { + private LockscreenCredential(int type, byte[] credential, boolean hasInvalidChars) { Objects.requireNonNull(credential); if (type == CREDENTIAL_TYPE_NONE) { Preconditions.checkArgument(credential.length == 0); @@ -88,15 +102,21 @@ public class LockscreenCredential implements Parcelable, AutoCloseable { // LockscreenCredential object to be constructed so that the validation logic can run, // even though the validation logic will ultimately reject the credential as too short. } + Preconditions.checkArgument(!hasInvalidChars || type == CREDENTIAL_TYPE_PASSWORD); mType = type; mCredential = credential; + mHasInvalidChars = hasInvalidChars; + } + + private LockscreenCredential(int type, CharSequence credential) { + this(type, charsToBytesTruncating(credential), hasInvalidChars(credential)); } /** * Creates a LockscreenCredential object representing a none credential. */ public static LockscreenCredential createNone() { - return new LockscreenCredential(CREDENTIAL_TYPE_NONE, new byte[0]); + return new LockscreenCredential(CREDENTIAL_TYPE_NONE, new byte[0], false); } /** @@ -104,15 +124,14 @@ public class LockscreenCredential implements Parcelable, AutoCloseable { */ public static LockscreenCredential createPattern(@NonNull List pattern) { return new LockscreenCredential(CREDENTIAL_TYPE_PATTERN, - LockPatternUtils.patternToByteArray(pattern)); + LockPatternUtils.patternToByteArray(pattern), /* hasInvalidChars= */ false); } /** * Creates a LockscreenCredential object representing the given alphabetic password. */ public static LockscreenCredential createPassword(@NonNull CharSequence password) { - return new LockscreenCredential(CREDENTIAL_TYPE_PASSWORD, - charSequenceToByteArray(password)); + return new LockscreenCredential(CREDENTIAL_TYPE_PASSWORD, password); } /** @@ -123,15 +142,14 @@ public class LockscreenCredential implements Parcelable, AutoCloseable { */ public static LockscreenCredential createManagedPassword(@NonNull byte[] password) { return new LockscreenCredential(CREDENTIAL_TYPE_PASSWORD, - Arrays.copyOf(password, password.length)); + Arrays.copyOf(password, password.length), /* hasInvalidChars= */ false); } /** * Creates a LockscreenCredential object representing the given numeric PIN. */ public static LockscreenCredential createPin(@NonNull CharSequence pin) { - return new LockscreenCredential(CREDENTIAL_TYPE_PIN, - charSequenceToByteArray(pin)); + return new LockscreenCredential(CREDENTIAL_TYPE_PIN, pin); } /** @@ -211,10 +229,17 @@ public class LockscreenCredential implements Parcelable, AutoCloseable { return mCredential.length; } + /** Returns true if this credential was constructed with any chars outside the allowed range */ + public boolean hasInvalidChars() { + ensureNotZeroized(); + return mHasInvalidChars; + } + /** Create a copy of the credential */ public LockscreenCredential duplicate() { return new LockscreenCredential(mType, - mCredential != null ? Arrays.copyOf(mCredential, mCredential.length) : null); + mCredential != null ? Arrays.copyOf(mCredential, mCredential.length) : null, + mHasInvalidChars); } /** @@ -323,6 +348,7 @@ public class LockscreenCredential implements Parcelable, AutoCloseable { public void writeToParcel(Parcel dest, int flags) { dest.writeInt(mType); dest.writeByteArray(mCredential); + dest.writeBoolean(mHasInvalidChars); } public static final Parcelable.Creator CREATOR = @@ -330,7 +356,8 @@ public class LockscreenCredential implements Parcelable, AutoCloseable { @Override public LockscreenCredential createFromParcel(Parcel source) { - return new LockscreenCredential(source.readInt(), source.createByteArray()); + return new LockscreenCredential(source.readInt(), source.createByteArray(), + source.readBoolean()); } @Override @@ -352,7 +379,7 @@ public class LockscreenCredential implements Parcelable, AutoCloseable { @Override public int hashCode() { // Effective Java — Always override hashCode when you override equals - return Objects.hash(mType, Arrays.hashCode(mCredential)); + return Objects.hash(mType, Arrays.hashCode(mCredential), mHasInvalidChars); } @Override @@ -360,20 +387,45 @@ public class LockscreenCredential implements Parcelable, AutoCloseable { if (o == this) return true; if (!(o instanceof LockscreenCredential)) return false; final LockscreenCredential other = (LockscreenCredential) o; - return mType == other.mType && Arrays.equals(mCredential, other.mCredential); + return mType == other.mType && Arrays.equals(mCredential, other.mCredential) + && mHasInvalidChars == other.mHasInvalidChars; + } + + private static boolean hasInvalidChars(CharSequence chars) { + // + // Consider the password to have invalid characters if it contains any non-ASCII characters + // or control characters. There are multiple reasons for this restriction: + // + // - Non-ASCII characters might only be possible to enter on a third-party keyboard app + // (IME) that is available when setting the password but not when verifying it after a + // reboot. This can happen if the keyboard is not direct boot aware or gets uninstalled. + // + // - Unicode strings that look identical to the user can map to different byte[]. Yet, only + // one byte[] can be accepted. Unicode normalization can solve this problem to some + // extent, but still many Unicode characters look similar and could cause confusion. + // + // - For backwards compatibility reasons, the upper 8 bits of the 16-bit 'chars' are + // discarded by charsToBytesTruncating(). Thus, as-is passwords with characters above + // U+00FF (255) are not as secure as they should be. IMPORTANT: Do not change the below + // code to allow characters above U+00FF (255) without fixing this issue! + // + for (int i = 0; i < chars.length(); i++) { + char c = chars.charAt(i); + if (c < 32 || c > 127) { + return true; + } + } + return false; } /** - * Converts a CharSequence to a byte array without requiring a toString(), which creates an - * additional copy. + * Converts a CharSequence to a byte array, intentionally truncating chars greater than 255 for + * backwards compatibility reasons. See {@link #mHasInvalidChars}. * * @param chars The CharSequence to convert * @return A byte array representing the input */ - private static byte[] charSequenceToByteArray(CharSequence chars) { - if (chars == null) { - return new byte[0]; - } + private static byte[] charsToBytesTruncating(CharSequence chars) { byte[] bytes = new byte[chars.length()]; for (int i = 0; i < chars.length(); i++) { bytes[i] = (byte) chars.charAt(i); diff --git a/core/tests/coretests/src/com/android/internal/widget/LockscreenCredentialTest.java b/core/tests/coretests/src/com/android/internal/widget/LockscreenCredentialTest.java index 434a8953b19f1..2ad367cb939f7 100644 --- a/core/tests/coretests/src/com/android/internal/widget/LockscreenCredentialTest.java +++ b/core/tests/coretests/src/com/android/internal/widget/LockscreenCredentialTest.java @@ -21,7 +21,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -46,6 +46,7 @@ public class LockscreenCredentialTest { assertFalse(none.isPin()); assertFalse(none.isPassword()); assertFalse(none.isPattern()); + assertFalse(none.hasInvalidChars()); } @Test @@ -59,6 +60,7 @@ public class LockscreenCredentialTest { assertFalse(pin.isNone()); assertFalse(pin.isPassword()); assertFalse(pin.isPattern()); + assertFalse(pin.hasInvalidChars()); } @Test @@ -72,6 +74,7 @@ public class LockscreenCredentialTest { assertFalse(password.isNone()); assertFalse(password.isPin()); assertFalse(password.isPattern()); + assertFalse(password.hasInvalidChars()); } @Test @@ -91,6 +94,7 @@ public class LockscreenCredentialTest { assertFalse(pattern.isNone()); assertFalse(pattern.isPin()); assertFalse(pattern.isPassword()); + assertFalse(pattern.hasInvalidChars()); } // Constructing a LockscreenCredential with a too-short length, even 0, should not throw an @@ -131,6 +135,20 @@ public class LockscreenCredentialTest { LockscreenCredential.createPinOrNone("1357")); } + // Test that passwords containing invalid characters that were incorrectly allowed in + // Android 10–14 are still interpreted in the same way. + @Test + public void testPasswordWithInvalidChars() { + // ™ is U+2122, which was truncated to ASCII 0x22 which is double quote. + String[] passwords = new String[] { "foo™", "™™™™", "™foo" }; + String[] equivalentAsciiPasswords = new String[] { "foo\"", "\"\"\"\"", "\"foo" }; + for (int i = 0; i < passwords.length; i++) { + LockscreenCredential credential = LockscreenCredential.createPassword(passwords[i]); + assertTrue(credential.hasInvalidChars()); + assertArrayEquals(equivalentAsciiPasswords[i].getBytes(), credential.getCredential()); + } + } + @Test public void testSanitize() { LockscreenCredential password = LockscreenCredential.createPassword("password"); @@ -156,6 +174,10 @@ public class LockscreenCredentialTest { password.size(); fail("Sanitized credential still accessible"); } catch (IllegalStateException expected) { } + try { + password.hasInvalidChars(); + fail("Sanitized credential still accessible"); + } catch (IllegalStateException expected) { } try { password.getCredential(); fail("Sanitized credential still accessible"); @@ -171,32 +193,37 @@ public class LockscreenCredentialTest { LockscreenCredential.createPin("4321")); assertEquals(createPattern("1234"), createPattern("1234")); - assertNotSame(LockscreenCredential.createPassword("1234"), + assertNotEquals(LockscreenCredential.createPassword("1234"), LockscreenCredential.createNone()); - assertNotSame(LockscreenCredential.createPassword("1234"), + assertNotEquals(LockscreenCredential.createPassword("1234"), LockscreenCredential.createPassword("4321")); - assertNotSame(LockscreenCredential.createPassword("1234"), + assertNotEquals(LockscreenCredential.createPassword("1234"), createPattern("1234")); - assertNotSame(LockscreenCredential.createPassword("1234"), + assertNotEquals(LockscreenCredential.createPassword("1234"), LockscreenCredential.createPin("1234")); - assertNotSame(LockscreenCredential.createPin("1111"), + assertNotEquals(LockscreenCredential.createPin("1111"), LockscreenCredential.createNone()); - assertNotSame(LockscreenCredential.createPin("1111"), + assertNotEquals(LockscreenCredential.createPin("1111"), LockscreenCredential.createPin("2222")); - assertNotSame(LockscreenCredential.createPin("1111"), + assertNotEquals(LockscreenCredential.createPin("1111"), createPattern("1111")); - assertNotSame(LockscreenCredential.createPin("1111"), + assertNotEquals(LockscreenCredential.createPin("1111"), LockscreenCredential.createPassword("1111")); - assertNotSame(createPattern("5678"), + assertNotEquals(createPattern("5678"), LockscreenCredential.createNone()); - assertNotSame(createPattern("5678"), + assertNotEquals(createPattern("5678"), createPattern("1234")); - assertNotSame(createPattern("5678"), + assertNotEquals(createPattern("5678"), LockscreenCredential.createPassword("5678")); - assertNotSame(createPattern("5678"), + assertNotEquals(createPattern("5678"), LockscreenCredential.createPin("5678")); + + // Test that mHasInvalidChars is compared. To do this, compare two passwords that map to + // the same byte[] (due to the truncation bug) but different values of mHasInvalidChars. + assertNotEquals(LockscreenCredential.createPassword("™™™™"), + LockscreenCredential.createPassword("\"\"\"\"")); } @Test @@ -211,6 +238,10 @@ public class LockscreenCredentialTest { assertEquals(credential, credential.duplicate()); credential = createPattern("5678"); assertEquals(credential, credential.duplicate()); + + // Test that mHasInvalidChars is duplicated. + credential = LockscreenCredential.createPassword("™™™™"); + assertEquals(credential, credential.duplicate()); } @Test From 813f32c92cd915e41011c960940d50f28f0f58f2 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Wed, 21 Jun 2023 05:28:11 +0000 Subject: [PATCH 4/5] 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, From fe59a023e86bed6daba4bfcb8c9a964c7636127f Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Wed, 21 Jun 2023 05:28:11 +0000 Subject: [PATCH 5/5] Make LockSettingsService enforce basic requirements for new credentials Currently all LSKF requirements are enforced by PasswordMetrics#validateCredential(). The standard minimum length of 4 is also checked again in LockPatternUtils#setLockCredential(). These are both at the caller's option, though. These requirements could be circumvented by calling ILockSettings#setLockCredential() directly. Therefore, to provide higher assurance that at least the standard requirements are met, this CL moves the standard length check into LockSettingsService and also adds the invalid chars check alongside it. Bug: 219511761 Bug: 232900169 Bug: 243881358 Test: atest LockscreenCredentialTest Test: atest com.android.server.locksettings Change-Id: Icc48a0d6caac0884bf3e3a9181828e8dfffff7e4 --- .../internal/widget/LockPatternUtils.java | 5 +- .../internal/widget/LockscreenCredential.java | 48 +++++++++++-------- .../widget/LockscreenCredentialTest.java | 10 +++- .../locksettings/LockSettingsService.java | 2 + .../LockSettingsServiceTests.java | 24 +++++++++- .../locksettings/SyntheticPasswordTests.java | 6 +-- 6 files changed, 66 insertions(+), 29 deletions(-) diff --git a/core/java/com/android/internal/widget/LockPatternUtils.java b/core/java/com/android/internal/widget/LockPatternUtils.java index 12fd6c5531497..fe5850a59903f 100644 --- a/core/java/com/android/internal/widget/LockPatternUtils.java +++ b/core/java/com/android/internal/widget/LockPatternUtils.java @@ -97,7 +97,7 @@ public class LockPatternUtils { public static final int MIN_LOCK_PATTERN_SIZE = 4; /** - * The minimum size of a valid password. + * The minimum size of a valid password or PIN. */ public static final int MIN_LOCK_PASSWORD_SIZE = 4; @@ -780,7 +780,6 @@ public class LockPatternUtils { * and return false if the given credential is wrong. * @throws RuntimeException if password change encountered an unrecoverable error. * @throws UnsupportedOperationException secure lockscreen is not supported on this device. - * @throws IllegalArgumentException if new credential is too short. */ public boolean setLockCredential(@NonNull LockscreenCredential newCredential, @NonNull LockscreenCredential savedCredential, int userHandle) { @@ -788,7 +787,6 @@ public class LockPatternUtils { throw new UnsupportedOperationException( "This operation requires the lock screen feature."); } - newCredential.checkLength(); try { if (!getLockSettings().setLockCredential(newCredential, savedCredential, userHandle)) { @@ -1553,7 +1551,6 @@ public class LockPatternUtils { throw new UnsupportedOperationException( "This operation requires the lock screen feature."); } - credential.checkLength(); LockSettingsInternal localService = getLockSettingsInternal(); return localService.setLockCredentialWithToken(credential, tokenHandle, token, userHandle); diff --git a/core/java/com/android/internal/widget/LockscreenCredential.java b/core/java/com/android/internal/widget/LockscreenCredential.java index 6bc1a43b8c206..65e5e2965527e 100644 --- a/core/java/com/android/internal/widget/LockscreenCredential.java +++ b/core/java/com/android/internal/widget/LockscreenCredential.java @@ -253,27 +253,37 @@ public class LockscreenCredential implements Parcelable, AutoCloseable { } /** - * Check if the credential meets minimal length requirement. + * Checks whether the credential meets basic requirements for setting it as a new credential. * - * @throws IllegalArgumentException if the credential is too short. + * This is redundant if {@link android.app.admin.PasswordMetrics#validateCredential()}, which + * does more comprehensive checks, is correctly called first (which it should be). + * + * @throws IllegalArgumentException if the credential contains invalid characters or is too + * short */ - public void checkLength() { - if (isNone()) { - return; - } - if (isPattern()) { - if (size() < LockPatternUtils.MIN_LOCK_PATTERN_SIZE) { - throw new IllegalArgumentException("pattern must not be null and at least " - + LockPatternUtils.MIN_LOCK_PATTERN_SIZE + " dots long."); - } - return; - } - if (isPassword() || isPin()) { - if (size() < LockPatternUtils.MIN_LOCK_PASSWORD_SIZE) { - throw new IllegalArgumentException("password must not be null and at least " - + "of length " + LockPatternUtils.MIN_LOCK_PASSWORD_SIZE); - } - return; + public void validateBasicRequirements() { + switch (getType()) { + case CREDENTIAL_TYPE_PATTERN: + if (size() < LockPatternUtils.MIN_LOCK_PATTERN_SIZE) { + throw new IllegalArgumentException("pattern must be at least " + + LockPatternUtils.MIN_LOCK_PATTERN_SIZE + " dots long."); + } + break; + case CREDENTIAL_TYPE_PIN: + if (size() < LockPatternUtils.MIN_LOCK_PASSWORD_SIZE) { + throw new IllegalArgumentException("PIN must be at least " + + LockPatternUtils.MIN_LOCK_PASSWORD_SIZE + " digits long."); + } + break; + case CREDENTIAL_TYPE_PASSWORD: + if (mHasInvalidChars) { + throw new IllegalArgumentException("password contains invalid characters"); + } + if (size() < LockPatternUtils.MIN_LOCK_PASSWORD_SIZE) { + throw new IllegalArgumentException("password must be at least " + + LockPatternUtils.MIN_LOCK_PASSWORD_SIZE + " characters long."); + } + break; } } diff --git a/core/tests/coretests/src/com/android/internal/widget/LockscreenCredentialTest.java b/core/tests/coretests/src/com/android/internal/widget/LockscreenCredentialTest.java index 2ad367cb939f7..12abaa90b9dd8 100644 --- a/core/tests/coretests/src/com/android/internal/widget/LockscreenCredentialTest.java +++ b/core/tests/coretests/src/com/android/internal/widget/LockscreenCredentialTest.java @@ -47,6 +47,7 @@ public class LockscreenCredentialTest { assertFalse(none.isPassword()); assertFalse(none.isPattern()); assertFalse(none.hasInvalidChars()); + none.validateBasicRequirements(); } @Test @@ -61,6 +62,7 @@ public class LockscreenCredentialTest { assertFalse(pin.isPassword()); assertFalse(pin.isPattern()); assertFalse(pin.hasInvalidChars()); + pin.validateBasicRequirements(); } @Test @@ -75,6 +77,7 @@ public class LockscreenCredentialTest { assertFalse(password.isPin()); assertFalse(password.isPattern()); assertFalse(password.hasInvalidChars()); + password.validateBasicRequirements(); } @Test @@ -95,6 +98,7 @@ public class LockscreenCredentialTest { assertFalse(pattern.isPin()); assertFalse(pattern.isPassword()); assertFalse(pattern.hasInvalidChars()); + pattern.validateBasicRequirements(); } // Constructing a LockscreenCredential with a too-short length, even 0, should not throw an @@ -136,7 +140,7 @@ public class LockscreenCredentialTest { } // Test that passwords containing invalid characters that were incorrectly allowed in - // Android 10–14 are still interpreted in the same way. + // Android 10–14 are still interpreted in the same way, but are not allowed for new passwords. @Test public void testPasswordWithInvalidChars() { // ™ is U+2122, which was truncated to ASCII 0x22 which is double quote. @@ -146,6 +150,10 @@ public class LockscreenCredentialTest { LockscreenCredential credential = LockscreenCredential.createPassword(passwords[i]); assertTrue(credential.hasInvalidChars()); assertArrayEquals(equivalentAsciiPasswords[i].getBytes(), credential.getCredential()); + try { + credential.validateBasicRequirements(); + fail("should not be able to set password with invalid chars"); + } catch (IllegalArgumentException expected) { } } } diff --git a/services/core/java/com/android/server/locksettings/LockSettingsService.java b/services/core/java/com/android/server/locksettings/LockSettingsService.java index 7bb0489467b40..97dc062006694 100644 --- a/services/core/java/com/android/server/locksettings/LockSettingsService.java +++ b/services/core/java/com/android/server/locksettings/LockSettingsService.java @@ -1634,6 +1634,7 @@ public class LockSettingsService extends ILockSettings.Stub { + PERMISSION); } } + credential.validateBasicRequirements(); final long identity = Binder.clearCallingIdentity(); try { @@ -3069,6 +3070,7 @@ public class LockSettingsService extends ILockSettings.Stub { private boolean setLockCredentialWithToken(LockscreenCredential credential, long tokenHandle, byte[] token, int userId) { boolean result; + credential.validateBasicRequirements(); synchronized (mSpManager) { if (!mSpManager.hasEscrowData(userId)) { throw new SecurityException("Escrow token is disabled on the current user"); diff --git a/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java b/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java index 60a033fde4270..5a62d92e8e127 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java @@ -77,6 +77,26 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { testSetCredentialFailsWithoutLockScreen(PRIMARY_USER_ID, newPassword("password")); } + @Test(expected = IllegalArgumentException.class) + public void testSetTooShortPatternFails() throws RemoteException { + mService.setLockCredential(newPattern("123"), nonePassword(), PRIMARY_USER_ID); + } + + @Test(expected = IllegalArgumentException.class) + public void testSetTooShortPinFails() throws RemoteException { + mService.setLockCredential(newPin("123"), nonePassword(), PRIMARY_USER_ID); + } + + @Test(expected = IllegalArgumentException.class) + public void testSetTooShortPassword() throws RemoteException { + mService.setLockCredential(newPassword("123"), nonePassword(), PRIMARY_USER_ID); + } + + @Test(expected = IllegalArgumentException.class) + public void testSetPasswordWithInvalidChars() throws RemoteException { + mService.setLockCredential(newPassword("§µ¿¶¥£"), nonePassword(), PRIMARY_USER_ID); + } + @Test public void testSetPatternPrimaryUser() throws RemoteException { setAndVerifyCredential(PRIMARY_USER_ID, newPattern("123456789")); @@ -94,7 +114,7 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { @Test public void testChangePatternPrimaryUser() throws RemoteException { - testChangeCredential(PRIMARY_USER_ID, newPassword("!£$%^&*(())"), newPattern("1596321")); + testChangeCredential(PRIMARY_USER_ID, newPassword("password"), newPattern("1596321")); } @Test @@ -185,7 +205,7 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { assertNotNull(mGateKeeperService.getAuthToken(MANAGED_PROFILE_USER_ID)); assertEquals(profileSid, mGateKeeperService.getSecureUserId(MANAGED_PROFILE_USER_ID)); - setCredential(PRIMARY_USER_ID, newPassword("pwd"), primaryPassword); + setCredential(PRIMARY_USER_ID, newPassword("password"), primaryPassword); assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( profilePassword, MANAGED_PROFILE_USER_ID, 0 /* flags */) .getResponseCode()); diff --git a/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java b/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java index ce0347dbe4acf..dee77806e4f37 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java @@ -215,12 +215,12 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { @Test public void testChangeCredentialKeepsAuthSecret() throws RemoteException { LockscreenCredential password = newPassword("password"); - LockscreenCredential badPassword = newPassword("new"); + LockscreenCredential newPassword = newPassword("newPassword"); initSpAndSetCredential(PRIMARY_USER_ID, password); - mService.setLockCredential(badPassword, password, PRIMARY_USER_ID); + mService.setLockCredential(newPassword, password, PRIMARY_USER_ID); assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( - badPassword, PRIMARY_USER_ID, 0 /* flags */).getResponseCode()); + newPassword, PRIMARY_USER_ID, 0 /* flags */).getResponseCode()); // Check the same secret was passed each time ArgumentCaptor secret = ArgumentCaptor.forClass(byte[].class);