diff options
| -rw-r--r-- | core/java/com/android/internal/widget/LockscreenCredential.java | 88 | ||||
| -rw-r--r-- | core/tests/coretests/src/com/android/internal/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 6e63f790ab98..6bc1a43b8c20 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<LockPatternView.Cell> 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<LockscreenCredential> 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 434a8953b19f..2ad367cb939f 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"); @@ -157,6 +175,10 @@ public class LockscreenCredentialTest { 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"); } catch (IllegalStateException expected) { } @@ -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 |