From 8171508fe077325781ce83ff849a1d162ab8eff5 Mon Sep 17 00:00:00 2001 From: Rubin Xu Date: Thu, 4 Mar 2021 15:42:09 +0000 Subject: [PATCH] Return copy of pending token list Make SyntheticPasswordManager.getPendingTokensForUser() return a copy of the token list, as the underlying list might change if a pending token is activated or removed. This fixes the NPE in LockSettingsService.activateEscrowTokens() where it tries to iterate through the list and activate tokens. Bug: 178560581 Test: atest com.android.server.locksettings Merged-In: I59d159d792a3d9e2a79ea521d6f01a5cb7c65695 Change-Id: I59d159d792a3d9e2a79ea521d6f01a5cb7c65695 --- .../SyntheticPasswordManager.java | 3 +- .../locksettings/SyntheticPasswordTests.java | 42 +++++++++++++++++-- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java b/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java index d644b1dc6ca08..e2943f0a35bba 100644 --- a/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java +++ b/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java @@ -35,6 +35,7 @@ import android.security.Scrypt; import android.service.gatekeeper.GateKeeperResponse; import android.service.gatekeeper.IGateKeeperService; import android.util.ArrayMap; +import android.util.ArraySet; import android.util.Slog; import com.android.internal.annotations.VisibleForTesting; @@ -904,7 +905,7 @@ public class SyntheticPasswordManager { if (!tokenMap.containsKey(userId)) { return Collections.emptySet(); } - return tokenMap.get(userId).keySet(); + return new ArraySet<>(tokenMap.get(userId).keySet()); } public boolean removePendingToken(long handle, int userId) { 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 2c2fdcaab340f..f33f9ed1337c7 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java @@ -246,7 +246,8 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { assertFalse(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); assertTrue(mService.hasPendingEscrowToken(PRIMARY_USER_ID)); - mService.verifyCredential(password, 0, PRIMARY_USER_ID).getResponseCode(); + assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( + password, 0, PRIMARY_USER_ID).getResponseCode()); assertTrue(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); assertFalse(mService.hasPendingEscrowToken(PRIMARY_USER_ID)); @@ -275,7 +276,8 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { long handle = mLocalService.addEscrowToken(token, PRIMARY_USER_ID, null); assertFalse(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); - mService.verifyCredential(password, 0, PRIMARY_USER_ID).getResponseCode(); + assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( + password, 0, PRIMARY_USER_ID).getResponseCode()); assertTrue(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); mLocalService.setLockCredentialWithToken(nonePassword(), handle, token, PRIMARY_USER_ID); @@ -301,7 +303,8 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { long handle = mLocalService.addEscrowToken(token, PRIMARY_USER_ID, null); assertFalse(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); - mService.verifyCredential(password, 0, PRIMARY_USER_ID).getResponseCode(); + assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( + password, 0, PRIMARY_USER_ID).getResponseCode()); assertTrue(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); mService.setLockCredential(pattern, password, PRIMARY_USER_ID); @@ -376,6 +379,36 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { } catch (SecurityException expected) { } } + @Test + public void testActivateMultipleEscrowTokens() throws Exception { + byte[] token0 = "some-high-entropy-secure-token-0".getBytes(); + byte[] token1 = "some-high-entropy-secure-token-1".getBytes(); + byte[] token2 = "some-high-entropy-secure-token-2".getBytes(); + + LockscreenCredential password = newPassword("password"); + LockscreenCredential pattern = newPattern("123654"); + initializeCredentialUnderSP(password, PRIMARY_USER_ID); + + long handle0 = mLocalService.addEscrowToken(token0, PRIMARY_USER_ID, null); + long handle1 = mLocalService.addEscrowToken(token1, PRIMARY_USER_ID, null); + long handle2 = mLocalService.addEscrowToken(token2, PRIMARY_USER_ID, null); + + // Activate token + assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( + password, 0, PRIMARY_USER_ID).getResponseCode()); + + // Verify tokens work + assertTrue(mLocalService.isEscrowTokenActive(handle0, PRIMARY_USER_ID)); + assertTrue(mLocalService.setLockCredentialWithToken( + pattern, handle0, token0, PRIMARY_USER_ID)); + assertTrue(mLocalService.isEscrowTokenActive(handle1, PRIMARY_USER_ID)); + assertTrue(mLocalService.setLockCredentialWithToken( + pattern, handle1, token1, PRIMARY_USER_ID)); + assertTrue(mLocalService.isEscrowTokenActive(handle2, PRIMARY_USER_ID)); + assertTrue(mLocalService.setLockCredentialWithToken( + pattern, handle2, token2, PRIMARY_USER_ID)); + } + @Test public void testSetLockCredentialWithTokenFailsWithoutLockScreen() throws Exception { LockscreenCredential password = newPassword("password"); @@ -503,7 +536,8 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { reset(mDevicePolicyManager); long handle = mLocalService.addEscrowToken(token, PRIMARY_USER_ID, null); - mService.verifyCredential(password, 0, PRIMARY_USER_ID).getResponseCode(); + assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( + password, 0, PRIMARY_USER_ID).getResponseCode()); assertTrue(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); mService.onCleanupUser(PRIMARY_USER_ID);