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

Commit 05693166 authored by Eric Biggers's avatar Eric Biggers Committed by Automerger Merge Worker
Browse files

Make LockscreenCredential remember whether it has invalid chars am: 019ec7a6 

Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/2775376



Change-Id: Ibba38aa9c24046275b64b0b4ac253981968bb370
Signed-off-by: default avatarAutomerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
parents bfea325f 019ec7a6

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