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

Commit 8057abea authored by Eric Biggers's avatar Eric Biggers Committed by Android (Google) Code Review
Browse files

Merge changes from topic "lskf-chars-fix"

* changes:
  Make LockSettingsService enforce basic requirements for new credentials
  Add and use PasswordMetrics#validateCredential()
  Make LockscreenCredential remember whether it has invalid chars
  Allow LockscreenCredential to represent any proposed credential
  Convert LockscreenCredentialTest to JUnit4
parents 173305cc fe59a023
Loading
Loading
Loading
Loading
+5 −4
Original line number Diff line number Diff line
@@ -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;
        }
    }

    /**
+35 −6
Original line number Diff line number Diff line
@@ -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<PasswordValidationError> 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<PasswordValidationError> validatePassword(
            PasswordMetrics adminMetrics, int minComplexity, boolean isPin, byte[] password) {

+1 −4
Original line number Diff line number Diff line
@@ -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);
+111 −43
Original line number Diff line number Diff line
@@ -60,10 +60,24 @@ 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;

    // 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);
@@ -80,17 +94,29 @@ 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.
        }
        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 empty password.
     * 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);
    }

    /**
@@ -98,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);
    }

    /**
@@ -117,20 +142,19 @@ 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);
    }

    /**
     * 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 +166,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 +199,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;
@@ -205,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);
    }

    /**
@@ -222,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()) {
    public void validateBasicRequirements() {
        switch (getType()) {
            case CREDENTIAL_TYPE_PATTERN:
                if (size() < LockPatternUtils.MIN_LOCK_PATTERN_SIZE) {
                throw new IllegalArgumentException("pattern must not be null and at least "
                    throw new IllegalArgumentException("pattern must be at least "
                            + LockPatternUtils.MIN_LOCK_PATTERN_SIZE + " dots long.");
                }
            return;
                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 (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);
                    throw new IllegalArgumentException("password must be at least "
                            + LockPatternUtils.MIN_LOCK_PASSWORD_SIZE + " characters long.");
                }
            return;
                break;
        }
    }

@@ -317,6 +358,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 =
@@ -324,7 +366,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
@@ -346,7 +389,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
@@ -354,20 +397,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);
+46 −1
Original line number Diff line number Diff line
@@ -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(
Loading