diff --git a/core/java/com/android/internal/widget/LockPatternUtils.java b/core/java/com/android/internal/widget/LockPatternUtils.java index 12fd6c5531497..fe5850a59903f 100644 --- a/core/java/com/android/internal/widget/LockPatternUtils.java +++ b/core/java/com/android/internal/widget/LockPatternUtils.java @@ -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); diff --git a/core/java/com/android/internal/widget/LockscreenCredential.java b/core/java/com/android/internal/widget/LockscreenCredential.java index 6bc1a43b8c206..65e5e2965527e 100644 --- a/core/java/com/android/internal/widget/LockscreenCredential.java +++ b/core/java/com/android/internal/widget/LockscreenCredential.java @@ -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()) { - if (size() < LockPatternUtils.MIN_LOCK_PATTERN_SIZE) { - throw new IllegalArgumentException("pattern must not be null and at least " - + LockPatternUtils.MIN_LOCK_PATTERN_SIZE + " dots long."); - } - return; - } - 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); - } - return; + public void validateBasicRequirements() { + switch (getType()) { + case CREDENTIAL_TYPE_PATTERN: + if (size() < LockPatternUtils.MIN_LOCK_PATTERN_SIZE) { + throw new IllegalArgumentException("pattern must be at least " + + LockPatternUtils.MIN_LOCK_PATTERN_SIZE + " dots long."); + } + 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 (size() < LockPatternUtils.MIN_LOCK_PASSWORD_SIZE) { + throw new IllegalArgumentException("password must be at least " + + LockPatternUtils.MIN_LOCK_PASSWORD_SIZE + " characters long."); + } + break; } } diff --git a/core/tests/coretests/src/com/android/internal/widget/LockscreenCredentialTest.java b/core/tests/coretests/src/com/android/internal/widget/LockscreenCredentialTest.java index 2ad367cb939f7..12abaa90b9dd8 100644 --- a/core/tests/coretests/src/com/android/internal/widget/LockscreenCredentialTest.java +++ b/core/tests/coretests/src/com/android/internal/widget/LockscreenCredentialTest.java @@ -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) { } } } diff --git a/services/core/java/com/android/server/locksettings/LockSettingsService.java b/services/core/java/com/android/server/locksettings/LockSettingsService.java index 7bb0489467b40..97dc062006694 100644 --- a/services/core/java/com/android/server/locksettings/LockSettingsService.java +++ b/services/core/java/com/android/server/locksettings/LockSettingsService.java @@ -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"); diff --git a/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java b/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java index 60a033fde4270..5a62d92e8e127 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java @@ -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()); diff --git a/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java b/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java index ce0347dbe4acf..dee77806e4f37 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java @@ -215,12 +215,12 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { @Test public void testChangeCredentialKeepsAuthSecret() throws RemoteException { LockscreenCredential password = newPassword("password"); - LockscreenCredential badPassword = newPassword("new"); + LockscreenCredential newPassword = newPassword("newPassword"); initSpAndSetCredential(PRIMARY_USER_ID, password); - mService.setLockCredential(badPassword, password, PRIMARY_USER_ID); + mService.setLockCredential(newPassword, password, PRIMARY_USER_ID); assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( - badPassword, PRIMARY_USER_ID, 0 /* flags */).getResponseCode()); + newPassword, PRIMARY_USER_ID, 0 /* flags */).getResponseCode()); // Check the same secret was passed each time ArgumentCaptor secret = ArgumentCaptor.forClass(byte[].class);