Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d984e5fd authored by Eric Biggers's avatar Eric Biggers
Browse files

Make LockscreenCredential remember whether it has invalid chars 

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
parent 8066a758

core/java/com/android/internal/widget/LockscreenCredential.java

+70 −18
Original line number Diff line number Diff line
@@ -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);

core/tests/coretests/src/com/android/internal/widget/LockscreenCredentialTest.java

+44 −13
Original line number Diff line number Diff line
@@ -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