diff options
author | Rubin Xu <rubinxu@google.com> | 2020-09-16 14:10:21 +0100 |
---|---|---|
committer | android-build-team Robot <android-build-team-robot@google.com> | 2020-10-13 01:07:50 +0000 |
commit | 1a6f1fb402b96df561b9672aef1e4fce8a13de80 (patch) | |
tree | 1af102c5314852bd04a96860c401d4921ec7373a | |
parent | cf9d5d571f97fdce3d100ece113694ec2cd4bd7a (diff) | |
download | base-1a6f1fb402b96df561b9672aef1e4fce8a13de80.tar.gz |
Do not re-initialize synthetic password
A bug was introduced in R where LSS ends up regenerating SP
when an escrow token is being auto-activated on unsecured user,
due to a logic error in shouldMigrateToSyntheticPasswordLocked().
Fix the bug and add some safeguards as well as unit test to
prevent future regressions.
Bug: 168692734
Test: atest com.android.server.locksettings
Change-Id: If35f2fd26b49faf6e3d0d75c10b1b3bb95f247c2
(cherry picked from commit efc1d53df3a2e7116d7ed83bca9bf8e384d32740)
(cherry picked from commit 2d51788b08aa85afdb27af4f4586ac40dc949097)
-rw-r--r-- | services/core/java/com/android/server/locksettings/LockSettingsService.java | 7 | ||||
-rw-r--r-- | services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java | 18 |
2 files changed, 22 insertions, 3 deletions
diff --git a/services/core/java/com/android/server/locksettings/LockSettingsService.java b/services/core/java/com/android/server/locksettings/LockSettingsService.java index 69b02ceb2411..f6308202ab68 100644 --- a/services/core/java/com/android/server/locksettings/LockSettingsService.java +++ b/services/core/java/com/android/server/locksettings/LockSettingsService.java @@ -113,6 +113,7 @@ import com.android.internal.messages.nano.SystemMessageProto.SystemMessage; import com.android.internal.notification.SystemNotificationChannels; import com.android.internal.util.DumpUtils; import com.android.internal.util.IndentingPrintWriter; +import com.android.internal.util.Preconditions; import com.android.internal.widget.ICheckCredentialProgressCallback; import com.android.internal.widget.ILockSettings; import com.android.internal.widget.LockPatternUtils; @@ -2618,6 +2619,10 @@ public class LockSettingsService extends ILockSettings.Stub { protected AuthenticationToken initializeSyntheticPasswordLocked(byte[] credentialHash, LockscreenCredential credential, int userId) { Slog.i(TAG, "Initialize SyntheticPassword for user: " + userId); + Preconditions.checkState( + getSyntheticPasswordHandleLocked(userId) == SyntheticPasswordManager.DEFAULT_HANDLE, + "Cannot reinitialize SP"); + final AuthenticationToken auth = mSpManager.newSyntheticPasswordAndSid( getGateKeeperService(), credentialHash, credential, userId); if (auth == null) { @@ -2678,7 +2683,7 @@ public class LockSettingsService extends ILockSettings.Stub { @VisibleForTesting protected boolean shouldMigrateToSyntheticPasswordLocked(int userId) { - return true; + return getSyntheticPasswordHandleLocked(userId) == SyntheticPasswordManager.DEFAULT_HANDLE; } private VerifyCredentialResponse spBasedDoVerifyCredential(LockscreenCredential userCredential, diff --git a/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java b/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java index ba851992cbad..2c2fdcaab340 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java @@ -519,10 +519,24 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { LockscreenCredential password = newPassword("password"); initializeCredentialUnderSP(password, PRIMARY_USER_ID); assertTrue(mService.setLockCredential(password, password, PRIMARY_USER_ID)); + assertNoOrphanedFilesLeft(PRIMARY_USER_ID); + } + + @Test + public void testAddingEscrowToken_NoOrphanedFilesLeft() throws Exception { + final byte[] token = "some-high-entropy-secure-token".getBytes(); + for (int i = 0; i < 16; i++) { + long handle = mLocalService.addEscrowToken(token, PRIMARY_USER_ID, null); + assertTrue(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); + mLocalService.removeEscrowToken(handle, PRIMARY_USER_ID); + } + assertNoOrphanedFilesLeft(PRIMARY_USER_ID); + } + private void assertNoOrphanedFilesLeft(int userId) { String handleString = String.format("%016x", - mService.getSyntheticPasswordHandleLocked(PRIMARY_USER_ID)); - File directory = mStorage.getSyntheticPasswordDirectoryForUser(PRIMARY_USER_ID); + mService.getSyntheticPasswordHandleLocked(userId)); + File directory = mStorage.getSyntheticPasswordDirectoryForUser(userId); for (File file : directory.listFiles()) { String[] parts = file.getName().split("\\."); if (!parts[0].equals(handleString) && !parts[0].equals("0000000000000000")) { |