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

Commit 9669549c authored by Eric Biggers's avatar Eric Biggers
Browse files

Split getHashFactorInternal() from getHashFactor() 

Move the core of getHashFactor() into a new method
getHashFactorInternal().  Make updatePasswordHistory() call
getHashFactorInternal() instead of getHashFactor().  This follows the
best practice for service implementations by not re-entering the Binder
interface as part of the implementation of a different method in the
interface.  Specifically, it avoids a redundant permission check that is
needed only for incoming Binder calls.

(updatePasswordHistory() is part of the implementation of
ILockSettings#setLockCredential() and
LockSettingsInternal#setLockCredentialWithToken().
ILockSettings#setLockCredential() already does its own permission check
and clears the calling identity.  LockSettingsInternal methods are
callable only by system_server itself and are not expected to check
permissions; indeed, the only caller of setLockCredentialWithToken()
does its own permission check and clears the calling identity.)

No functional change other than removing redundant work.

Test: atest FrameworksServicesTests:com.android.server.locksettings
Bug: 320392352
Bug: 416768837
Flag: EXEMPT refactor
Change-Id: Iba3687cbe3db9cbbcb23f029111aa3208d4bd5a5
parent 9133f2f0

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

+14 −10
Original line number Diff line number Diff line
@@ -2029,13 +2029,13 @@ public class LockSettingsService extends ILockSettings.Stub { 
            passwordHistory = "";

        } else {

            Slogf.d(TAG, "Adding new password to password history for user %d", userHandle);

            final byte[] hashFactor = getHashFactor(password, userHandle);

            final byte[] hashFactor = getHashFactorInternal(password, userHandle);

            final byte[] salt = getSalt(userHandle).getBytes();

            String hash = password.passwordToHistoryHash(salt, hashFactor);

            if (hash == null) {

                // This should never happen, as all information needed to compute the hash should be

                // available.  In particular, unwrapping the SP in getHashFactor() should always

                // succeed, as we're using the LSKF that was just set.

                // available. In particular, unwrapping the SP in getHashFactorInternal() should

                // always succeed, as we're using the LSKF that was just set.

                Slog.e(TAG, "Failed to compute password hash; password history won't be updated");

                return;

            }
@@ -3320,14 +3320,12 @@ public class LockSettingsService extends ILockSettings.Stub { 
    }



    /**

     * Returns a fixed pseudorandom byte string derived from the user's synthetic password.

     * This is used to salt the password history hash to protect the hash against offline

     * bruteforcing, since rederiving this value requires a successful authentication.

     * If user is a profile with unified challenge, currentCredential is ignored.

     * Returns a fixed pseudorandom byte string derived from the user's synthetic password. This is

     * used to salt the password history hash to protect the hash against offline bruteforcing,

     * since rederiving this value requires a successful authentication. If user is a profile with

     * unified challenge, currentCredential is ignored.

     */

    @Override

    public byte[] getHashFactor(LockscreenCredential currentCredential, int userId) {

        checkPasswordReadPermission();

    private byte[] getHashFactorInternal(LockscreenCredential currentCredential, int userId) {

        try {

            Slogf.d(TAG, "Getting password history hash factor for user %d", userId);

            if (isProfileWithUnifiedLock(userId)) {
@@ -3353,6 +3351,12 @@ public class LockSettingsService extends ILockSettings.Stub { 
        }

    }



    @Override

    public byte[] getHashFactor(LockscreenCredential currentCredential, int userId) {

        checkPasswordReadPermission();

        return getHashFactorInternal(currentCredential, userId);

    }



    private long addEscrowToken(@NonNull byte[] token, @TokenType int type, int userId,

            @NonNull EscrowTokenStateChangeCallback callback) {

        Slogf.i(TAG, "Adding escrow token for user %d", userId);