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

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

Merge changes Ib48f05fb,I7451a7d3 

* changes:
  Remove unused and insecure fallback to legacy password history hash
  Avoid the suppression of an errorprone warning
parents b4d6af43 bd355874

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

+6 −16
Original line number Diff line number Diff line
@@ -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;

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

+0 −6
Original line number Diff line number Diff line
@@ -223,14 +223,10 @@ 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()))
@@ -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();

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

+17 −15
Original line number Diff line number Diff line
@@ -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;

    }

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

+50 −3
Original line number Diff line number Diff line
@@ -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));

        synchronized (mService.mSpManager) {

            mService.initializeSyntheticPasswordLocked(credential, userId);

        }

        if (credential.isNone()) {

            assertEquals(0, mGateKeeperService.getSecureUserId(userId));

        } else {

            assertNotEquals(0, mGateKeeperService.getSecureUserId(userId));

        }

    }

}