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

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

Make LockSettingsService enforce basic requirements for new credentials 

Currently all LSKF requirements are enforced by
PasswordMetrics#validateCredential().  The standard minimum length of 4
is also checked again in LockPatternUtils#setLockCredential().

These are both at the caller's option, though.  These requirements could
be circumvented by calling ILockSettings#setLockCredential() directly.

Therefore, to provide higher assurance that at least the standard
requirements are met, this CL moves the standard length check into
LockSettingsService and also adds the invalid chars check alongside it.

Bug: 219511761
Bug: 232900169
Bug: 243881358
Test: atest LockscreenCredentialTest
Test: atest com.android.server.locksettings
Change-Id: Icc48a0d6caac0884bf3e3a9181828e8dfffff7e4
parent 813f32c9

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

+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);

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

+29 −19
Original line number Diff line number Diff line
@@ -253,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;

        }

    }

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

+9 −1
Original line number Diff line number Diff line
@@ -47,6 +47,7 @@ public class LockscreenCredentialTest { 
        assertFalse(none.isPassword());

        assertFalse(none.isPattern());

        assertFalse(none.hasInvalidChars());

        none.validateBasicRequirements();

    }



    @Test
@@ -61,6 +62,7 @@ public class LockscreenCredentialTest { 
        assertFalse(pin.isPassword());

        assertFalse(pin.isPattern());

        assertFalse(pin.hasInvalidChars());

        pin.validateBasicRequirements();

    }



    @Test
@@ -75,6 +77,7 @@ public class LockscreenCredentialTest { 
        assertFalse(password.isPin());

        assertFalse(password.isPattern());

        assertFalse(password.hasInvalidChars());

        password.validateBasicRequirements();

    }



    @Test
@@ -95,6 +98,7 @@ public class LockscreenCredentialTest { 
        assertFalse(pattern.isPin());

        assertFalse(pattern.isPassword());

        assertFalse(pattern.hasInvalidChars());

        pattern.validateBasicRequirements();

    }



    // Constructing a LockscreenCredential with a too-short length, even 0, should not throw an
@@ -136,7 +140,7 @@ public class LockscreenCredentialTest { 
    }



    // Test that passwords containing invalid characters that were incorrectly allowed in

    // Android 10–14 are still interpreted in the same way.

    // Android 10–14 are still interpreted in the same way, but are not allowed for new passwords.

    @Test

    public void testPasswordWithInvalidChars() {

        // ™ is U+2122, which was truncated to ASCII 0x22 which is double quote.
@@ -146,6 +150,10 @@ public class LockscreenCredentialTest { 
            LockscreenCredential credential = LockscreenCredential.createPassword(passwords[i]);

            assertTrue(credential.hasInvalidChars());

            assertArrayEquals(equivalentAsciiPasswords[i].getBytes(), credential.getCredential());

            try {

                credential.validateBasicRequirements();

                fail("should not be able to set password with invalid chars");

            } catch (IllegalArgumentException expected) { }

        }

    }

services/core/java/com/android/server/locksettings/LockSettingsService.java

+2 −0
Original line number Diff line number Diff line
@@ -1634,6 +1634,7 @@ public class LockSettingsService extends ILockSettings.Stub { 
                                + PERMISSION);

            }

        }

        credential.validateBasicRequirements();



        final long identity = Binder.clearCallingIdentity();

        try {
@@ -3069,6 +3070,7 @@ public class LockSettingsService extends ILockSettings.Stub { 
    private boolean setLockCredentialWithToken(LockscreenCredential credential, long tokenHandle,

            byte[] token, int userId) {

        boolean result;

        credential.validateBasicRequirements();

        synchronized (mSpManager) {

            if (!mSpManager.hasEscrowData(userId)) {

                throw new SecurityException("Escrow token is disabled on the current user");

services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java

+22 −2
Original line number Diff line number Diff line
@@ -77,6 +77,26 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { 
        testSetCredentialFailsWithoutLockScreen(PRIMARY_USER_ID, newPassword("password"));

    }



    @Test(expected = IllegalArgumentException.class)

    public void testSetTooShortPatternFails() throws RemoteException {

        mService.setLockCredential(newPattern("123"), nonePassword(), PRIMARY_USER_ID);

    }



    @Test(expected = IllegalArgumentException.class)

    public void testSetTooShortPinFails() throws RemoteException {

        mService.setLockCredential(newPin("123"), nonePassword(), PRIMARY_USER_ID);

    }



    @Test(expected = IllegalArgumentException.class)

    public void testSetTooShortPassword() throws RemoteException {

        mService.setLockCredential(newPassword("123"), nonePassword(), PRIMARY_USER_ID);

    }



    @Test(expected = IllegalArgumentException.class)

    public void testSetPasswordWithInvalidChars() throws RemoteException {

        mService.setLockCredential(newPassword("§µ¿¶¥£"), nonePassword(), PRIMARY_USER_ID);

    }



    @Test

    public void testSetPatternPrimaryUser() throws RemoteException {

        setAndVerifyCredential(PRIMARY_USER_ID, newPattern("123456789"));
@@ -94,7 +114,7 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { 


    @Test

    public void testChangePatternPrimaryUser() throws RemoteException {

        testChangeCredential(PRIMARY_USER_ID, newPassword("!£$%^&*(())"), newPattern("1596321"));

        testChangeCredential(PRIMARY_USER_ID, newPassword("password"), newPattern("1596321"));

    }



    @Test
@@ -185,7 +205,7 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { 
        assertNotNull(mGateKeeperService.getAuthToken(MANAGED_PROFILE_USER_ID));

        assertEquals(profileSid, mGateKeeperService.getSecureUserId(MANAGED_PROFILE_USER_ID));



        setCredential(PRIMARY_USER_ID, newPassword("pwd"), primaryPassword);

        setCredential(PRIMARY_USER_ID, newPassword("password"), primaryPassword);

        assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential(

                profilePassword, MANAGED_PROFILE_USER_ID, 0 /* flags */)

                .getResponseCode());