diff options
4 files changed, 73 insertions, 40 deletions
diff --git a/core/java/com/android/internal/widget/LockscreenCredential.java b/core/java/com/android/internal/widget/LockscreenCredential.java index 40164a45516e..c6125fb217a9 100644 --- a/core/java/com/android/internal/widget/LockscreenCredential.java +++ b/core/java/com/android/internal/widget/LockscreenCredential.java @@ -290,25 +290,15 @@ public class LockscreenCredential implements Parcelable, AutoCloseable { } /** - * Generate a hash for the given password. To avoid brute force attacks, we use a salted hash. - * Not the most secure, but it is at least a second level of protection. First level is that - * the file is in a location only readable by the system process. + * Hash the given password for the password history, using the legacy algorithm. * - * @return the hash of the pattern in a byte array. - */ - public String legacyPasswordToHash(byte[] salt) { - return legacyPasswordToHash(mCredential, salt); - } - - /** - * Generate a hash for the given password. To avoid brute force attacks, we use a salted hash. - * Not the most secure, but it is at least a second level of protection. First level is that - * the file is in a location only readable by the system process. - * - * @param password the gesture pattern. + * @deprecated This algorithm is insecure because the password can be easily bruteforced, given + * the hash and salt. Use {@link #passwordToHistoryHash(byte[], byte[], byte[])} + * instead, which incorporates an SP-derived secret into the hash. * - * @return the hash of the pattern in a byte array. + * @return the legacy password hash */ + @Deprecated public static String legacyPasswordToHash(byte[] password, byte[] salt) { if (password == null || password.length == 0 || salt == null) { return null; 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 9d77d16b4a1d..a47868d0a524 100644 --- a/core/tests/coretests/src/com/android/internal/widget/LockscreenCredentialTest.java +++ b/core/tests/coretests/src/com/android/internal/widget/LockscreenCredentialTest.java @@ -223,15 +223,11 @@ public class LockscreenCredentialTest extends AndroidTestCase { public void testLegacyPasswordToHash() { String password = "1234"; - LockscreenCredential credential = LockscreenCredential.createPassword(password); String salt = "6d5331dd120077a0"; String expectedHash = "2DD04348ADBF8F4CABD7F722DC2E2887FAD4B6020A0C3E02C831E09946F0554FDC13B155"; assertThat( - credential.legacyPasswordToHash(salt.getBytes())) - .isEqualTo(expectedHash); - assertThat( LockscreenCredential.legacyPasswordToHash( password.getBytes(), salt.getBytes())) .isEqualTo(expectedHash); @@ -239,10 +235,8 @@ public class LockscreenCredentialTest extends AndroidTestCase { public void testLegacyPasswordToHashInvalidInput() { String password = "1234"; - LockscreenCredential credential = LockscreenCredential.createPassword(password); String salt = "6d5331dd120077a0"; - assertThat(credential.legacyPasswordToHash(/* salt= */ null)).isNull(); assertThat(LockscreenCredential.legacyPasswordToHash( password.getBytes(), /* salt= */ null)).isNull(); diff --git a/services/core/java/com/android/server/locksettings/LockSettingsService.java b/services/core/java/com/android/server/locksettings/LockSettingsService.java index 804315cddabd..9b6d0e7c09e4 100644 --- a/services/core/java/com/android/server/locksettings/LockSettingsService.java +++ b/services/core/java/com/android/server/locksettings/LockSettingsService.java @@ -234,7 +234,8 @@ public class LockSettingsService extends ILockSettings.Stub { protected final UserManager mUserManager; private final IStorageManager mStorageManager; private final IActivityManager mActivityManager; - private final SyntheticPasswordManager mSpManager; + @VisibleForTesting + protected final SyntheticPasswordManager mSpManager; private final KeyStore mKeyStore; private final java.security.KeyStore mJavaKeyStore; @@ -1625,7 +1626,7 @@ public class LockSettingsService extends ILockSettings.Stub { } onSyntheticPasswordKnown(userId, sp); - setLockCredentialWithSpLocked(credential, sp, userId); + setLockCredentialWithSpLocked(credential, sp, userId, isLockTiedToParent); sendCredentialsOnChangeIfRequired(credential, userId, isLockTiedToParent); return true; } @@ -1639,15 +1640,15 @@ public class LockSettingsService extends ILockSettings.Stub { if (newCredential.isPattern()) { setBoolean(LockPatternUtils.PATTERN_EVER_CHOSEN_KEY, true, userHandle); } - updatePasswordHistory(newCredential, userHandle); mContext.getSystemService(TrustManager.class).reportEnabledTrustAgentsChanged(userHandle); } /** - * Store the hash of the *current* password in the password history list, if device policy - * enforces password history requirement. + * Store the hash of the new password in the password history list, if device policy enforces + * a password history requirement. */ - private void updatePasswordHistory(LockscreenCredential password, int userHandle) { + private void updatePasswordHistory(SyntheticPassword sp, LockscreenCredential password, + int userHandle, boolean isLockTiedToParent) { if (password.isNone()) { return; } @@ -1655,8 +1656,11 @@ public class LockSettingsService extends ILockSettings.Stub { // Do not keep track of historical patterns return; } - // Add the password to the password history. We assume all - // password hashes have the same length for simplicity of implementation. + if (isLockTiedToParent) { + // Do not keep track of historical auto-generated profile passwords + return; + } + // Add the password to the password history. String passwordHistory = getString( LockPatternUtils.PASSWORD_HISTORY_KEY, /* defaultValue= */ null, userHandle); if (passwordHistory == null) { @@ -1666,13 +1670,9 @@ public class LockSettingsService extends ILockSettings.Stub { if (passwordHistoryLength == 0) { passwordHistory = ""; } else { - final byte[] hashFactor = getHashFactor(password, userHandle); + final byte[] hashFactor = sp.derivePasswordHashFactor(); final byte[] salt = getSalt(userHandle).getBytes(); String hash = password.passwordToHistoryHash(salt, hashFactor); - if (hash == null) { - Slog.e(TAG, "Compute new style password hash failed, fallback to legacy style"); - hash = password.legacyPasswordToHash(salt); - } if (TextUtils.isEmpty(passwordHistory)) { passwordHistory = hash; } else { @@ -2650,7 +2650,7 @@ public class LockSettingsService extends ILockSettings.Stub { */ @GuardedBy("mSpManager") private long setLockCredentialWithSpLocked(LockscreenCredential credential, - SyntheticPassword sp, int userId) { + SyntheticPassword sp, int userId, boolean isLockTiedToParent) { if (DEBUG) Slog.d(TAG, "setLockCredentialWithSpLocked: user=" + userId); final int savedCredentialType = getCredentialTypeInternal(userId); final long oldProtectorId = getCurrentLskfBasedProtectorId(userId); @@ -2688,6 +2688,7 @@ public class LockSettingsService extends ILockSettings.Stub { LockPatternUtils.invalidateCredentialTypeCache(); synchronizeUnifiedWorkChallengeForProfiles(userId, profilePasswords); + updatePasswordHistory(sp, credential, userId, isLockTiedToParent); setUserPasswordMetrics(credential, userId); mManagedProfilePasswordCache.removePassword(userId); if (savedCredentialType != CREDENTIAL_TYPE_NONE) { @@ -2933,7 +2934,8 @@ public class LockSettingsService extends ILockSettings.Stub { return false; } onSyntheticPasswordKnown(userId, result.syntheticPassword); - setLockCredentialWithSpLocked(credential, result.syntheticPassword, userId); + setLockCredentialWithSpLocked(credential, result.syntheticPassword, userId, + /* isLockTiedToParent= */ false); return true; } 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 20cc42cd9c76..a4cccb320ff1 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java @@ -33,15 +33,18 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.never; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import android.app.PropertyInvalidatedCache; import android.os.RemoteException; import android.platform.test.annotations.Presubmit; import android.service.gatekeeper.GateKeeperResponse; +import android.text.TextUtils; import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; +import com.android.internal.widget.LockPatternUtils; import com.android.internal.widget.LockscreenCredential; import com.android.internal.widget.VerifyCredentialResponse; @@ -450,6 +453,45 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { PRIMARY_USER_ID)); } + @Test + public void testPasswordHistoryDisabledByDefault() throws Exception { + final int userId = PRIMARY_USER_ID; + checkPasswordHistoryLength(userId, 0); + initializeStorageWithCredential(userId, nonePassword()); + checkPasswordHistoryLength(userId, 0); + + assertTrue(mService.setLockCredential(newPassword("1234"), nonePassword(), userId)); + checkPasswordHistoryLength(userId, 0); + } + + @Test + public void testPasswordHistoryLengthHonored() throws Exception { + final int userId = PRIMARY_USER_ID; + when(mDevicePolicyManager.getPasswordHistoryLength(any(), eq(userId))).thenReturn(3); + checkPasswordHistoryLength(userId, 0); + initializeStorageWithCredential(userId, nonePassword()); + checkPasswordHistoryLength(userId, 0); + + assertTrue(mService.setLockCredential(newPassword("pass1"), nonePassword(), userId)); + checkPasswordHistoryLength(userId, 1); + + assertTrue(mService.setLockCredential(newPassword("pass2"), newPassword("pass1"), userId)); + checkPasswordHistoryLength(userId, 2); + + assertTrue(mService.setLockCredential(newPassword("pass3"), newPassword("pass2"), userId)); + checkPasswordHistoryLength(userId, 3); + + // maximum length should have been reached + assertTrue(mService.setLockCredential(newPassword("pass4"), newPassword("pass3"), userId)); + checkPasswordHistoryLength(userId, 3); + } + + private void checkPasswordHistoryLength(int userId, int expectedLen) { + String history = mService.getString(LockPatternUtils.PASSWORD_HISTORY_KEY, "", userId); + String[] hashes = TextUtils.split(history, LockPatternUtils.PASSWORD_HISTORY_DELIMITER); + assertEquals(expectedLen, hashes.length); + } + private void testCreateCredential(int userId, LockscreenCredential credential) throws RemoteException { assertTrue(mService.setLockCredential(credential, nonePassword(), userId)); @@ -504,11 +546,16 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { badCredential, userId, 0 /* flags */).getResponseCode()); } - @SuppressWarnings("GuardedBy") // for initializeSyntheticPasswordLocked private void initializeStorageWithCredential(int userId, LockscreenCredential credential) throws RemoteException { assertEquals(0, mGateKeeperService.getSecureUserId(userId)); - mService.initializeSyntheticPasswordLocked(credential, userId); - assertNotEquals(0, mGateKeeperService.getSecureUserId(userId)); + synchronized (mService.mSpManager) { + mService.initializeSyntheticPasswordLocked(credential, userId); + } + if (credential.isNone()) { + assertEquals(0, mGateKeeperService.getSecureUserId(userId)); + } else { + assertNotEquals(0, mGateKeeperService.getSecureUserId(userId)); + } } } |