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

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

Skip creating password metrics file for unsecured users 

As another optimization, skip creating the password metrics file in the
LSKF-based protector when the LSKF is empty.  In this case, the password
metrics file is unnecessary and contains no information.

This saves creating one file, which can be somewhat expensive due to the
use of synchronous writes and an fsync of the parent directory.  It also
saves one key derivation operation and one encryption operation.

Note: previously, it seems to have been possible for a PasswordMetrics
object to end up in the LSS.mUserPasswordMetrics map for an unsecured
user, specifically via LSS.unlockUserWithToken().  After this change,
that can no longer happen.  This is fine because the isUserSecure()
check in LSS.getUserPasswordMetrics() meant that any PasswordMetrics
that may have been cached for an unsecured users were never used anyway.

Bug: 232452368
Bug: 251131631
Bug: 251147505
Test: atest com.android.server.locksettings
Change-Id: I93780a20c69b3a1b0ba21d7c38e98923771fc114
parent 3510d130

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

+9 −7
Original line number Diff line number Diff line
@@ -256,10 +256,10 @@ public class LockSettingsService extends ILockSettings.Stub { 
    @GuardedBy("mUserCreationAndRemovalLock")

    private boolean mBootComplete;



    // Current password metric for all users on the device. Updated when user unlocks

    // the device or changes password. Removed when user is stopped.

    // Current password metrics for all secured users on the device. Updated when user unlocks the

    // device or changes password. Removed when user is stopped.

    @GuardedBy("this")

    final SparseArray<PasswordMetrics> mUserPasswordMetrics = new SparseArray<>();

    private final SparseArray<PasswordMetrics> mUserPasswordMetrics = new SparseArray<>();

    @VisibleForTesting

    protected boolean mHasSecureLockScreen;
@@ -2274,8 +2274,11 @@ public class LockSettingsService extends ILockSettings.Stub { 
        }

    }



    private PasswordMetrics loadPasswordMetrics(SyntheticPassword sp, int userHandle) {

    private @Nullable PasswordMetrics loadPasswordMetrics(SyntheticPassword sp, int userHandle) {

        synchronized (mSpManager) {

            if (!isUserSecure(userHandle)) {

                return null;

            }

            return mSpManager.getPasswordMetrics(sp, getCurrentLskfBasedProtectorId(userHandle),

                    userHandle);

        }
@@ -2703,14 +2706,13 @@ public class LockSettingsService extends ILockSettings.Stub { 
        return handle;

    }



    private void onCredentialVerified(SyntheticPassword sp, PasswordMetrics metrics, int userId) {

    private void onCredentialVerified(SyntheticPassword sp, @Nullable PasswordMetrics metrics,

            int userId) {



        if (metrics != null) {

            synchronized (this) {

                mUserPasswordMetrics.put(userId,  metrics);

            }

        } else {

            Slog.wtf(TAG, "Null metrics after credential verification");

        }



        unlockKeystore(sp.deriveKeyStorePassword(), userId);

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

+17 −6
Original line number Diff line number Diff line
@@ -99,7 +99,8 @@ import java.util.Set; 
 *       PASSWORD_DATA_NAME: Data used for LSKF verification, such as the scrypt salt and

 *                           parameters.  Only exists for LSKF-based protectors.

 *       PASSWORD_METRICS_NAME: Metrics about the LSKF, encrypted by a key derived from the SP.

 *                              Only exists for LSKF-based protectors.

 *                              Only exists for LSKF-based protectors.  Doesn't exist when the LSKF

 *                              is empty, except in old protectors.

 *       SECDISCARDABLE_NAME: A large number of random bytes that all need to be known in order to

 *                            decrypt SP_BLOB_NAME.  When the protector is deleted, this file is

 *                            overwritten and deleted as a "best-effort" attempt to support secure
@@ -838,7 +839,9 @@ public class SyntheticPasswordManager { 
            synchronizeFrpPassword(pwd, 0, userId);

        }

        saveState(PASSWORD_DATA_NAME, pwd.toBytes(), protectorId, userId);

        if (!credential.isNone()) {

            savePasswordMetrics(credential, sp, protectorId, userId);

        }

        createSyntheticPasswordBlob(protectorId, PROTECTOR_TYPE_LSKF_BASED, sp, protectorSecret,

                sid, userId);

        return protectorId;
@@ -1149,7 +1152,8 @@ public class SyntheticPasswordManager { 


        // Upgrade case: store the metrics if the device did not have stored metrics before, should

        // only happen once on old protectors.

        if (result.syntheticPassword != null && !hasPasswordMetrics(protectorId, userId)) {

        if (result.syntheticPassword != null && !credential.isNone() &&

                !hasPasswordMetrics(protectorId, userId)) {

            savePasswordMetrics(credential, result.syntheticPassword, protectorId, userId);

        }

        return result;
@@ -1422,10 +1426,16 @@ public class SyntheticPasswordManager { 
    public @Nullable PasswordMetrics getPasswordMetrics(SyntheticPassword sp, long protectorId,

            int userId) {

        final byte[] encrypted = loadState(PASSWORD_METRICS_NAME, protectorId, userId);

        if (encrypted == null) return null;

        if (encrypted == null) {

            Slogf.e(TAG, "Failed to read password metrics file for user %d", userId);

            return null;

        }

        final byte[] decrypted = SyntheticPasswordCrypto.decrypt(sp.deriveMetricsKey(),

                /* personalization= */ new byte[0], encrypted);

        if (decrypted == null) return null;

        if (decrypted == null) {

            Slogf.e(TAG, "Failed to decrypt password metrics file for user %d", userId);

            return null;

        }

        return VersionedPasswordMetrics.deserialize(decrypted).getMetrics();

    }
@@ -1437,7 +1447,8 @@ public class SyntheticPasswordManager { 
        saveState(PASSWORD_METRICS_NAME, encrypted, protectorId, userId);

    }



    private boolean hasPasswordMetrics(long protectorId, int userId) {

    @VisibleForTesting

    boolean hasPasswordMetrics(long protectorId, int userId) {

        return hasState(PASSWORD_METRICS_NAME, protectorId, userId);

    }

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

+2 −0
Original line number Diff line number Diff line
@@ -84,6 +84,7 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { 
        long protectorId = manager.createLskfBasedProtector(mGateKeeperService,

                LockscreenCredential.createNone(), sp, USER_ID);

        assertFalse(lskfGatekeeperHandleExists(USER_ID));

        assertFalse(manager.hasPasswordMetrics(protectorId, USER_ID));



        AuthenticationResult result = manager.unlockLskfBasedProtector(mGateKeeperService,

                protectorId, LockscreenCredential.createNone(), USER_ID, null);
@@ -103,6 +104,7 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { 
        long protectorId = manager.createLskfBasedProtector(mGateKeeperService, password, sp,

                USER_ID);

        assertTrue(lskfGatekeeperHandleExists(USER_ID));

        assertTrue(manager.hasPasswordMetrics(protectorId, USER_ID));



        AuthenticationResult result = manager.unlockLskfBasedProtector(mGateKeeperService,

                protectorId, password, USER_ID, null);