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

Commit 813f32c9 authored by Eric Biggers's avatar Eric Biggers
Browse files

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
parent d984e5fd
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) {

+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(
+2 −12
Original line number Diff line number Diff line
@@ -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<PasswordValidationError> 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<PasswordValidationError> errors =
                PasswordMetrics.validateCredential(requiredMetrics, requiredComplexity, credential);
        if (!errors.isEmpty()) {
            getOutPrintWriter().println(
                    "New credential doesn't satisfy admin policies: " + errors.get(0));
+8 −17
Original line number Diff line number Diff line
@@ -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<PasswordValidationError> 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<PasswordValidationError> 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,