From 573b3791ce82052db970e59fcb59e4d814f36341 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Thu, 15 Dec 2022 05:00:46 +0000 Subject: [PATCH] Remove isSyntheticPasswordBasedCredentialLocked() As an additional cleanup, now that all users are guaranteed to have a synthetic password (except for new users during early boot), remove the isSyntheticPasswordBasedCredentialLocked() method and its callers. Considering each case: - In migrateFrpCredential(), the user is guaranteed to have an SP, since they also have an LSKF. This was true even before "SPs on creation". - In onThirdPartyAppsStarted(), any users in mEarlyCreatedUsers are guaranteed to not have an SP yet, since their SP creation was delayed, and the code that did on-demand SP creation has been removed (as it should not have been reachable anyway). - In getCredentialTypeInternal(), the LSKF-based protector ID is being looked up anyway. It's more efficient to check that value for NULL_PROTECTOR_ID, instead of doing a redundant lookup. - In doVerifyCredential(), the check for an SP was redundant with later checks. So, removing it doesn't change the behavior (other than the log messages); VerifyCredentialResponse.ERROR is still returned. Also, the SP should always exist here anyway. - Similarly, in getHashFactor(), the check for an SP is redundant with the check for NULL_PROTECTOR_ID in unlockLskfBasedProtector(). - In disableEscrowTokenOnNonManagedDevicesIfNeeded(), calling destroyEscrowData() is harmless if there is no SP. But there should always be an SP here anyway. Test: atest com.android.server.locksettings Bug: 232452368 Change-Id: I39ad1bdf84db745db85d4d8fcaaa1d989511d0e1 --- .../locksettings/LockSettingsService.java | 63 ++++++------------- 1 file changed, 19 insertions(+), 44 deletions(-) diff --git a/services/core/java/com/android/server/locksettings/LockSettingsService.java b/services/core/java/com/android/server/locksettings/LockSettingsService.java index 5f39a523b3aca..33a7ed41d68e0 100644 --- a/services/core/java/com/android/server/locksettings/LockSettingsService.java +++ b/services/core/java/com/android/server/locksettings/LockSettingsService.java @@ -883,7 +883,6 @@ public class LockSettingsService extends ILockSettings.Stub { * Migrate the credential for the FRP credential owner user if the following are satisfied: * - the user has a secure credential * - the FRP credential is not set up - * - the credential is based on a synthetic password. */ private void migrateFrpCredential() { if (mStorage.readPersistentDataBlock() != PersistentData.NONE) { @@ -892,15 +891,13 @@ public class LockSettingsService extends ILockSettings.Stub { for (UserInfo userInfo : mUserManager.getUsers()) { if (userOwnsFrpCredential(mContext, userInfo) && isUserSecure(userInfo.id)) { synchronized (mSpManager) { - if (isSyntheticPasswordBasedCredentialLocked(userInfo.id)) { - int actualQuality = (int) getLong(LockPatternUtils.PASSWORD_TYPE_KEY, - DevicePolicyManager.PASSWORD_QUALITY_UNSPECIFIED, userInfo.id); + int actualQuality = (int) getLong(LockPatternUtils.PASSWORD_TYPE_KEY, + DevicePolicyManager.PASSWORD_QUALITY_UNSPECIFIED, userInfo.id); - mSpManager.migrateFrpPasswordLocked( - getCurrentLskfBasedProtectorId(userInfo.id), - userInfo, - redactActualQualityToMostLenientEquivalentQuality(actualQuality)); - } + mSpManager.migrateFrpPasswordLocked( + getCurrentLskfBasedProtectorId(userInfo.id), + userInfo, + redactActualQualityToMostLenientEquivalentQuality(actualQuality)); } return; } @@ -941,13 +938,9 @@ public class LockSettingsService extends ILockSettings.Stub { int serialNumber = mEarlyCreatedUsers.valueAt(i); removeStateForReusedUserIdIfNecessary(userId, serialNumber); - synchronized (mSpManager) { - if (!isSyntheticPasswordBasedCredentialLocked(userId)) { - Slogf.i(TAG, "Creating locksettings state for user %d now that boot " - + "is complete", userId); - initializeSyntheticPassword(userId); - } - } + Slogf.i(TAG, "Creating locksettings state for user %d now that boot is complete", + userId); + initializeSyntheticPassword(userId); } mEarlyCreatedUsers = null; // no longer needed @@ -1234,16 +1227,17 @@ public class LockSettingsService extends ILockSettings.Stub { return getFrpCredentialType(); } synchronized (mSpManager) { - if (isSyntheticPasswordBasedCredentialLocked(userId)) { - final long protectorId = getCurrentLskfBasedProtectorId(userId); - int rawType = mSpManager.getCredentialType(protectorId, userId); - if (rawType != CREDENTIAL_TYPE_PASSWORD_OR_PIN) { - return rawType; - } - return pinOrPasswordQualityToCredentialType(getKeyguardStoredQuality(userId)); + final long protectorId = getCurrentLskfBasedProtectorId(userId); + if (protectorId == SyntheticPasswordManager.NULL_PROTECTOR_ID) { + // Only possible for new users during early boot (before onThirdPartyAppsStarted()) + return CREDENTIAL_TYPE_NONE; } + int rawType = mSpManager.getCredentialType(protectorId, userId); + if (rawType != CREDENTIAL_TYPE_PASSWORD_OR_PIN) { + return rawType; + } + return pinOrPasswordQualityToCredentialType(getKeyguardStoredQuality(userId)); } - return CREDENTIAL_TYPE_NONE; } private int getFrpCredentialType() { @@ -2167,10 +2161,6 @@ public class LockSettingsService extends ILockSettings.Stub { VerifyCredentialResponse response; synchronized (mSpManager) { - if (!isSyntheticPasswordBasedCredentialLocked(userId)) { - Slog.wtf(TAG, "Unexpected credential type, should be SP based."); - return VerifyCredentialResponse.ERROR; - } if (userId == USER_FRP) { return mSpManager.verifyFrpCredential(getGateKeeperService(), credential, progressCallback); @@ -2672,15 +2662,6 @@ public class LockSettingsService extends ILockSettings.Stub { setLong(LSKF_LAST_CHANGED_TIME_KEY, System.currentTimeMillis(), userId); } - private boolean isSyntheticPasswordBasedCredentialLocked(int userId) { - if (userId == USER_FRP) { - final int type = mStorage.readPersistentDataBlock().type; - return type == PersistentData.TYPE_SP || type == PersistentData.TYPE_SP_WEAVER; - } - long protectorId = getCurrentLskfBasedProtectorId(userId); - return protectorId != SyntheticPasswordManager.NULL_PROTECTOR_ID; - } - /** * Stores the gatekeeper password temporarily. * @param gatekeeperPassword unlocked upon successful Synthetic Password @@ -2888,10 +2869,6 @@ public class LockSettingsService extends ILockSettings.Stub { } } synchronized (mSpManager) { - if (!isSyntheticPasswordBasedCredentialLocked(userId)) { - Slog.w(TAG, "Synthetic password not enabled"); - return null; - } long protectorId = getCurrentLskfBasedProtectorId(userId); AuthenticationResult auth = mSpManager.unlockLskfBasedProtector( getGateKeeperService(), protectorId, currentCredential, userId, null); @@ -3218,9 +3195,7 @@ public class LockSettingsService extends ILockSettings.Stub { // Disable escrow token permanently on all other device/user types. Slog.i(TAG, "Disabling escrow token on user " + userId); - if (isSyntheticPasswordBasedCredentialLocked(userId)) { - mSpManager.destroyEscrowData(userId); - } + mSpManager.destroyEscrowData(userId); } /**