diff options
6 files changed, 113 insertions, 33 deletions
diff --git a/services/core/java/com/android/server/locksettings/LockSettingsService.java b/services/core/java/com/android/server/locksettings/LockSettingsService.java index 31c20cbf82f9..cae8468f8ee2 100644 --- a/services/core/java/com/android/server/locksettings/LockSettingsService.java +++ b/services/core/java/com/android/server/locksettings/LockSettingsService.java @@ -1348,11 +1348,11 @@ public class LockSettingsService extends ILockSettings.Stub { .verifyChallenge(userId, 0, willStore.hash, credential.getBytes()); setUserKeyProtection(userId, credential, convertResponse(gkResponse)); fixateNewestUserKeyAuth(userId); - mRecoverableKeyStoreManager.lockScreenSecretChanged(credentialType, credential, - userId); // Refresh the auth token doVerifyCredential(credential, credentialType, true, 0, userId, null /* progressCallback */); synchronizeUnifiedWorkChallengeForProfiles(userId, null); + mRecoverableKeyStoreManager.lockScreenSecretChanged(credentialType, credential, + userId); } else { throw new RemoteException("Failed to enroll " + (credentialType == LockPatternUtils.CREDENTIAL_TYPE_PASSWORD ? "password" @@ -2494,6 +2494,7 @@ public class LockSettingsService extends ILockSettings.Stub { (response != null ? "rate limit exceeded" : "failed")); return; } + mRecoverableKeyStoreManager.lockScreenSecretChanged(credentialType, credential, userId); } @Override diff --git a/services/core/java/com/android/server/locksettings/recoverablekeystore/KeySyncTask.java b/services/core/java/com/android/server/locksettings/recoverablekeystore/KeySyncTask.java index db46c3d4df3d..819b62b20ee4 100644 --- a/services/core/java/com/android/server/locksettings/recoverablekeystore/KeySyncTask.java +++ b/services/core/java/com/android/server/locksettings/recoverablekeystore/KeySyncTask.java @@ -72,7 +72,7 @@ public class KeySyncTask implements Runnable { private final int mCredentialType; private final String mCredential; private final boolean mCredentialUpdated; - private final PlatformKeyManager.Factory mPlatformKeyManagerFactory; + private final PlatformKeyManager mPlatformKeyManager; private final RecoverySnapshotStorage mRecoverySnapshotStorage; private final RecoverySnapshotListenersStorage mSnapshotListenersStorage; @@ -94,7 +94,7 @@ public class KeySyncTask implements Runnable { credentialType, credential, credentialUpdated, - () -> PlatformKeyManager.getInstance(context, recoverableKeyStoreDb)); + PlatformKeyManager.getInstance(context, recoverableKeyStoreDb)); } /** @@ -105,9 +105,7 @@ public class KeySyncTask implements Runnable { * @param credentialType The type of credential as defined in {@code LockPatternUtils} * @param credential The credential, encoded as a {@link String}. * @param credentialUpdated signals weather credentials were updated. - * @param platformKeyManagerFactory Instantiates a {@link PlatformKeyManager} for the user. - * This is a factory to enable unit testing, as otherwise it would be impossible to test - * without a screen unlock occurring! + * @param platformKeyManager platform key manager */ @VisibleForTesting KeySyncTask( @@ -118,14 +116,14 @@ public class KeySyncTask implements Runnable { int credentialType, String credential, boolean credentialUpdated, - PlatformKeyManager.Factory platformKeyManagerFactory) { + PlatformKeyManager platformKeyManager) { mSnapshotListenersStorage = recoverySnapshotListenersStorage; mRecoverableKeyStoreDb = recoverableKeyStoreDb; mUserId = userId; mCredentialType = credentialType; mCredential = credential; mCredentialUpdated = credentialUpdated; - mPlatformKeyManagerFactory = platformKeyManagerFactory; + mPlatformKeyManager = platformKeyManager; mRecoverySnapshotStorage = snapshotStorage; } @@ -145,6 +143,8 @@ public class KeySyncTask implements Runnable { if (mCredentialType == LockPatternUtils.CREDENTIAL_TYPE_NONE) { // Application keys for the user will not be available for sync. Log.w(TAG, "Credentials are not set for user " + mUserId); + int generation = mPlatformKeyManager.getGenerationId(mUserId); + mPlatformKeyManager.invalidatePlatformKey(mUserId, generation); return; } @@ -304,8 +304,7 @@ public class KeySyncTask implements Runnable { throws InsecureUserException, KeyStoreException, UnrecoverableKeyException, NoSuchAlgorithmException, NoSuchPaddingException, BadPlatformKeyException, InvalidKeyException, InvalidAlgorithmParameterException { - PlatformKeyManager platformKeyManager = mPlatformKeyManagerFactory.newInstance(); - PlatformDecryptionKey decryptKey = platformKeyManager.getDecryptKey(mUserId);; + PlatformDecryptionKey decryptKey = mPlatformKeyManager.getDecryptKey(mUserId);; Map<String, WrappedKey> wrappedKeys = mRecoverableKeyStoreDb.getAllKeys( mUserId, recoveryAgentUid, decryptKey.getGenerationId()); return WrappedKey.unwrapKeys(decryptKey, wrappedKeys); diff --git a/services/core/java/com/android/server/locksettings/recoverablekeystore/PlatformKeyManager.java b/services/core/java/com/android/server/locksettings/recoverablekeystore/PlatformKeyManager.java index ee6a89312988..3a78f950675e 100644 --- a/services/core/java/com/android/server/locksettings/recoverablekeystore/PlatformKeyManager.java +++ b/services/core/java/com/android/server/locksettings/recoverablekeystore/PlatformKeyManager.java @@ -129,6 +129,26 @@ public class PlatformKeyManager { } /** + * Removes the platform key from Android KeyStore. + * It is triggered when user disables lock screen. + * + * @param userId The ID of the user to whose lock screen the platform key must be bound. + * @param generationId Generation id. + * + * @hide + */ + public void invalidatePlatformKey(int userId, int generationId) { + if (generationId != -1) { + try { + mKeyStore.deleteEntry(getEncryptAlias(userId, generationId)); + mKeyStore.deleteEntry(getDecryptAlias(userId, generationId)); + } catch (KeyStoreException e) { + // Ignore failed attempt to delete key. + } + } + } + + /** * Generates a new key and increments the generation ID. Should be invoked if the platform key * is corrupted and needs to be rotated. * Updates status of old keys to {@code RecoveryController.RECOVERY_STATUS_PERMANENT_FAILURE}. @@ -152,6 +172,7 @@ public class PlatformKeyManager { if (generationId == -1) { nextId = 1; } else { + invalidatePlatformKey(userId, generationId); nextId = generationId + 1; } generateAndLoadKey(userId, nextId); @@ -197,8 +218,12 @@ public class PlatformKeyManager { private PlatformEncryptionKey getEncryptKeyInternal(int userId) throws KeyStoreException, UnrecoverableKeyException, NoSuchAlgorithmException, InsecureUserException { int generationId = getGenerationId(userId); + String alias = getEncryptAlias(userId, generationId); + if (!mKeyStore.containsAlias(alias)) { + throw new UnrecoverableKeyException("KeyStore doesn't contain key " + alias); + } AndroidKeyStoreSecretKey key = (AndroidKeyStoreSecretKey) mKeyStore.getKey( - getEncryptAlias(userId, generationId), /*password=*/ null); + alias, /*password=*/ null); return new PlatformEncryptionKey(generationId, key); } @@ -242,8 +267,12 @@ public class PlatformKeyManager { private PlatformDecryptionKey getDecryptKeyInternal(int userId) throws KeyStoreException, UnrecoverableKeyException, NoSuchAlgorithmException, InsecureUserException { int generationId = getGenerationId(userId); + String alias = getDecryptAlias(userId, generationId); + if (!mKeyStore.containsAlias(alias)) { + throw new UnrecoverableKeyException("KeyStore doesn't contain key " + alias); + } AndroidKeyStoreSecretKey key = (AndroidKeyStoreSecretKey) mKeyStore.getKey( - getDecryptAlias(userId, generationId), /*password=*/ null); + alias, /*password=*/ null); return new PlatformDecryptionKey(generationId, key); } @@ -405,16 +434,4 @@ public class PlatformKeyManager { return keyStore; } - /** - * @hide - */ - public interface Factory { - /** - * New PlatformKeyManager instance. - * - * @hide - */ - PlatformKeyManager newInstance() - throws NoSuchAlgorithmException, InsecureUserException, KeyStoreException; - } } diff --git a/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java b/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java index 0d567d1e971d..f3c95e45721e 100644 --- a/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java +++ b/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java @@ -579,6 +579,7 @@ public class RecoverableKeyStoreManager { /** * This function can only be used inside LockSettingsService. + * * @param storedHashType from {@code CredentialHash} * @param credential - unencrypted String * @param userId for the user whose lock screen credentials were changed. @@ -604,7 +605,7 @@ public class RecoverableKeyStoreManager { } catch (KeyStoreException e) { Log.e(TAG, "Key store error encountered during recoverable key sync", e); } catch (InsecureUserException e) { - Log.wtf(TAG, "Impossible - insecure user, but user just entered lock screen", e); + Log.e(TAG, "InsecureUserException during lock screen secret update", e); } } diff --git a/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/KeySyncTaskTest.java b/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/KeySyncTaskTest.java index 6a3a260361c9..c94f227f6961 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/KeySyncTaskTest.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/KeySyncTaskTest.java @@ -121,7 +121,7 @@ public class KeySyncTaskTest { TEST_CREDENTIAL_TYPE, TEST_CREDENTIAL, /*credentialUpdated=*/ false, - () -> mPlatformKeyManager); + mPlatformKeyManager); mWrappingKey = generateAndroidKeyStoreKey(); mEncryptKey = new PlatformEncryptionKey(TEST_GENERATION_ID, mWrappingKey); @@ -352,7 +352,7 @@ public class KeySyncTaskTest { CREDENTIAL_TYPE_PASSWORD, "password", /*credentialUpdated=*/ false, - () -> mPlatformKeyManager); + mPlatformKeyManager); mRecoverableKeyStoreDb.setRecoveryServicePublicKey( TEST_USER_ID, TEST_RECOVERY_AGENT_UID, mKeyPair.getPublic()); @@ -378,7 +378,7 @@ public class KeySyncTaskTest { CREDENTIAL_TYPE_PASSWORD, /*credential=*/ "1234", /*credentialUpdated=*/ false, - () -> mPlatformKeyManager); + mPlatformKeyManager); mRecoverableKeyStoreDb.setRecoveryServicePublicKey( TEST_USER_ID, TEST_RECOVERY_AGENT_UID, mKeyPair.getPublic()); @@ -405,7 +405,7 @@ public class KeySyncTaskTest { CREDENTIAL_TYPE_PATTERN, "12345", /*credentialUpdated=*/ false, - () -> mPlatformKeyManager); + mPlatformKeyManager); mRecoverableKeyStoreDb.setRecoveryServicePublicKey( TEST_USER_ID, TEST_RECOVERY_AGENT_UID, mKeyPair.getPublic()); diff --git a/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/PlatformKeyManagerTest.java b/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/PlatformKeyManagerTest.java index 6fc9e082bb7c..f9ffccdd5d38 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/PlatformKeyManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/PlatformKeyManagerTest.java @@ -252,6 +252,10 @@ public class PlatformKeyManagerTest { @Test public void getDecryptKey_getsDecryptKeyWithCorrectAlias() throws Exception { + when(mKeyStoreProxy + .containsAlias("com.android.server.locksettings.recoverablekeystore/" + + "platform/42/1/decrypt")).thenReturn(true); + mPlatformKeyManager.getDecryptKey(USER_ID_FIXTURE); verify(mKeyStoreProxy).getKey( @@ -265,6 +269,13 @@ public class PlatformKeyManagerTest { eq("com.android.server.locksettings.recoverablekeystore/platform/42/1/encrypt"), any()); + when(mKeyStoreProxy + .containsAlias("com.android.server.locksettings.recoverablekeystore/" + + "platform/42/1/encrypt")).thenReturn(true); + when(mKeyStoreProxy + .containsAlias("com.android.server.locksettings.recoverablekeystore/" + + "platform/42/2/encrypt")).thenReturn(true); + mPlatformKeyManager.getEncryptKey(USER_ID_FIXTURE); verify(mKeyStoreProxy).getKey( @@ -282,19 +293,50 @@ public class PlatformKeyManagerTest { eq("com.android.server.locksettings.recoverablekeystore/platform/42/1/decrypt"), any()); + when(mKeyStoreProxy + .containsAlias("com.android.server.locksettings.recoverablekeystore/" + + "platform/42/1/decrypt")).thenReturn(false); // was removed. + + when(mKeyStoreProxy + .containsAlias("com.android.server.locksettings.recoverablekeystore/" + + "platform/42/2/decrypt")).thenReturn(true); // new version is available + mPlatformKeyManager.getDecryptKey(USER_ID_FIXTURE); + verify(mKeyStoreProxy).containsAlias( + eq("com.android.server.locksettings.recoverablekeystore/platform/42/1/decrypt")); + // Attempt to get regenerated key. verify(mKeyStoreProxy).getKey( - eq("com.android.server.locksettings.recoverablekeystore/platform/42/1/decrypt"), + eq("com.android.server.locksettings.recoverablekeystore/platform/42/2/decrypt"), any()); + } + + @Test + public void getEncryptKey_generatesNewKeyIfOldWasRemoved() throws Exception { + when(mKeyStoreProxy + .containsAlias("com.android.server.locksettings.recoverablekeystore/" + + "platform/42/1/encrypt")).thenReturn(false); // was removed. + + when(mKeyStoreProxy + .containsAlias("com.android.server.locksettings.recoverablekeystore/" + + "platform/42/2/encrypt")).thenReturn(true); // new version is available + + mPlatformKeyManager.getEncryptKey(USER_ID_FIXTURE); + + verify(mKeyStoreProxy).containsAlias( + eq("com.android.server.locksettings.recoverablekeystore/platform/42/1/encrypt")); // Attempt to get regenerated key. verify(mKeyStoreProxy).getKey( - eq("com.android.server.locksettings.recoverablekeystore/platform/42/2/decrypt"), + eq("com.android.server.locksettings.recoverablekeystore/platform/42/2/encrypt"), any()); } @Test - public void getEncryptKey_getsDecryptKeyWithCorrectAlias() throws Exception { + public void getEncryptKey_getsEndryptKeyWithCorrectAlias() throws Exception { + when(mKeyStoreProxy + .containsAlias("com.android.server.locksettings.recoverablekeystore/" + + "platform/42/1/encrypt")).thenReturn(true); + mPlatformKeyManager.getEncryptKey(USER_ID_FIXTURE); verify(mKeyStoreProxy).getKey( @@ -312,6 +354,26 @@ public class PlatformKeyManagerTest { } @Test + public void regenerate_deletesOldKeysFromKeystore() throws Exception { + mPlatformKeyManager.init(USER_ID_FIXTURE); + + mPlatformKeyManager.regenerate(USER_ID_FIXTURE); + + verify(mKeyStoreProxy).deleteEntry( + eq("com.android.server.locksettings.recoverablekeystore/platform/42/1/encrypt")); + verify(mKeyStoreProxy).deleteEntry( + eq("com.android.server.locksettings.recoverablekeystore/platform/42/1/decrypt")); + + mPlatformKeyManager.regenerate(USER_ID_FIXTURE); + + // Removes second generation keys. + verify(mKeyStoreProxy).deleteEntry( + eq("com.android.server.locksettings.recoverablekeystore/platform/42/2/encrypt")); + verify(mKeyStoreProxy).deleteEntry( + eq("com.android.server.locksettings.recoverablekeystore/platform/42/2/decrypt")); + } + + @Test public void regenerate_generatesANewEncryptKeyWithTheCorrectAlias() throws Exception { mPlatformKeyManager.init(USER_ID_FIXTURE); |