From cbe48d94962ff5addc11cb1e4fcd8eb0805b4470 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 Change-Id: I59d159d792a3d9e2a79ea521d6f01a5cb7c65695 --- .../SyntheticPasswordManager.java | 3 +- .../locksettings/SyntheticPasswordTests.java | 46 +++++++++++++++---- 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java b/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java index 7ab8b27075b7c..a5763aee6336b 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; @@ -906,7 +907,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 2d6605ae222f3..c0a38b8749143 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java @@ -239,8 +239,8 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { assertFalse(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); assertTrue(mService.hasPendingEscrowToken(PRIMARY_USER_ID)); - mService.verifyCredential(password, PRIMARY_USER_ID, 0 /* flags */) - .getResponseCode(); + assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( + password, PRIMARY_USER_ID, 0 /* flags */).getResponseCode()); assertTrue(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); assertFalse(mService.hasPendingEscrowToken(PRIMARY_USER_ID)); @@ -268,8 +268,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, PRIMARY_USER_ID, 0 /* flags */) - .getResponseCode(); + assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( + password, PRIMARY_USER_ID, 0 /* flags */).getResponseCode()); assertTrue(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); mLocalService.setLockCredentialWithToken(nonePassword(), handle, token, PRIMARY_USER_ID); @@ -294,8 +294,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, PRIMARY_USER_ID, 0 /* flags */) - .getResponseCode(); + assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( + password, PRIMARY_USER_ID, 0 /* flags */).getResponseCode()); assertTrue(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); mService.setLockCredential(pattern, password, PRIMARY_USER_ID); @@ -368,6 +368,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, PRIMARY_USER_ID, 0 /* flags */).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"); @@ -494,8 +524,8 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { reset(mDevicePolicyManager); long handle = mLocalService.addEscrowToken(token, PRIMARY_USER_ID, null); - mService.verifyCredential(password, PRIMARY_USER_ID, 0 /* flags */) - .getResponseCode(); + assertEquals(VerifyCredentialResponse.RESPONSE_OK, mService.verifyCredential( + password, PRIMARY_USER_ID, 0 /* flags */).getResponseCode()); assertTrue(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID)); mService.onCleanupUser(PRIMARY_USER_ID);