From 925f026cc190d4e6822d1728846fd0f940a618f8 Mon Sep 17 00:00:00 2001 From: Dmitry Dementyev Date: Thu, 12 Apr 2018 12:10:50 -0700 Subject: [PATCH] Don't create recovery snapshot until it contains at least one key. Bug: 77931409 Test: atest RecoveryControllerHostTest Change-Id: Ibd239cbd21a756a00c33e3cd0100b389b88d38b0 --- .../RecoverableKeyStoreManager.java | 24 ++++--- .../storage/RecoverableKeyStoreDb.java | 15 ++++- .../RecoverableKeyStoreManagerTest.java | 64 +++++++++++++++++-- 3 files changed, 85 insertions(+), 18 deletions(-) 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 c484251cc1fc9..335da50b020da 100644 --- a/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java +++ b/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java @@ -232,9 +232,6 @@ public class RecoverableKeyStoreManager { throw new ServiceSpecificException(ERROR_INVALID_CERTIFICATE, e.getMessage()); } - boolean wasInitialized = mDatabase.getRecoveryServiceCertPath(userId, uid, - rootCertificateAlias) != null; - // Save the chosen and validated certificate into database try { Log.d(TAG, "Saving the randomly chosen endpoint certificate to database"); @@ -242,9 +239,11 @@ public class RecoverableKeyStoreManager { certPath) > 0) { mDatabase.setRecoveryServiceCertSerial(userId, uid, rootCertificateAlias, newSerial); - if (wasInitialized) { - Log.i(TAG, "This is a certificate change. Snapshot pending."); + if (mDatabase.getSnapshotVersion(userId, uid) != null) { mDatabase.setShouldCreateSnapshot(userId, uid, true); + Log.i(TAG, "This is a certificate change. Snapshot must be updated"); + } else { + Log.i(TAG, "This is a certificate change. Snapshot didn't exist"); } mDatabase.setCounterId(userId, uid, new SecureRandom().nextLong()); } @@ -350,8 +349,12 @@ public class RecoverableKeyStoreManager { return; } - Log.i(TAG, "Updated server params. Snapshot pending."); - mDatabase.setShouldCreateSnapshot(userId, uid, true); + if (mDatabase.getSnapshotVersion(userId, uid) != null) { + mDatabase.setShouldCreateSnapshot(userId, uid, true); + Log.i(TAG, "Updated server params. Snapshot must be updated"); + } else { + Log.i(TAG, "Updated server params. Snapshot didn't exist"); + } } /** @@ -407,7 +410,12 @@ public class RecoverableKeyStoreManager { } Log.i(TAG, "Updated secret types. Snapshot pending."); - mDatabase.setShouldCreateSnapshot(userId, uid, true); + if (mDatabase.getSnapshotVersion(userId, uid) != null) { + mDatabase.setShouldCreateSnapshot(userId, uid, true); + Log.i(TAG, "Updated secret types. Snapshot must be updated"); + } else { + Log.i(TAG, "Updated secret types. Snapshot didn't exist"); + } } /** diff --git a/services/core/java/com/android/server/locksettings/recoverablekeystore/storage/RecoverableKeyStoreDb.java b/services/core/java/com/android/server/locksettings/recoverablekeystore/storage/RecoverableKeyStoreDb.java index 7c4360e5813c3..e69f73df01848 100644 --- a/services/core/java/com/android/server/locksettings/recoverablekeystore/storage/RecoverableKeyStoreDb.java +++ b/services/core/java/com/android/server/locksettings/recoverablekeystore/storage/RecoverableKeyStoreDb.java @@ -789,11 +789,20 @@ public class RecoverableKeyStoreDb { } /** - * Updates the snapshot version. + * Updates a flag indicating that a new snapshot should be created. + * It will be {@code false} until the first application key is added. + * After that, the flag will be set to true, if one of the following values is updated: + * * * @param userId The userId of the profile the application is running under. * @param uid The uid of the application. - * @param pending The server parameters. + * @param pending Should create snapshot flag. * @return The primary key of the inserted row, or -1 if failed. * * @hide @@ -809,7 +818,7 @@ public class RecoverableKeyStoreDb { * * @param userId The userId of the profile the application is running under. * @param uid The uid of the application who initialized the local recovery components. - * @return snapshot outdated flag. + * @return should create snapshot flag * * @hide */ diff --git a/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManagerTest.java b/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManagerTest.java index db910f2494f86..e82478fb68ba7 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManagerTest.java @@ -301,6 +301,33 @@ public class RecoverableKeyStoreManagerTest { assertThat(mRecoverableKeyStoreDb.getRecoveryServicePublicKey(userId, uid)).isNull(); } + @Test + public void initRecoveryService_updatesShouldCreatesnapshotOnCertUpdate() throws Exception { + int uid = Binder.getCallingUid(); + int userId = UserHandle.getCallingUserId(); + long certSerial = 1000L; + mRecoverableKeyStoreDb.setShouldCreateSnapshot(userId, uid, false); + + mRecoverableKeyStoreManager.initRecoveryService(ROOT_CERTIFICATE_ALIAS, + TestData.getCertXmlWithSerial(certSerial)); + + assertThat(mRecoverableKeyStoreDb.getShouldCreateSnapshot(userId, uid)).isFalse(); + + mRecoverableKeyStoreManager.initRecoveryService(ROOT_CERTIFICATE_ALIAS, + TestData.getCertXmlWithSerial(certSerial + 1)); + + // Since there were no recoverable keys, new snapshot will not be created. + assertThat(mRecoverableKeyStoreDb.getShouldCreateSnapshot(userId, uid)).isFalse(); + + generateKeyAndSimulateSync(userId, uid, 10); + + mRecoverableKeyStoreManager.initRecoveryService(ROOT_CERTIFICATE_ALIAS, + TestData.getCertXmlWithSerial(certSerial + 2)); + + // Since there were a recoverable key, new serial number triggers snapshot creation + assertThat(mRecoverableKeyStoreDb.getShouldCreateSnapshot(userId, uid)).isTrue(); + } + @Test public void initRecoveryService_triesToFilterRootAlias() throws Exception { int uid = Binder.getCallingUid(); @@ -405,7 +432,8 @@ public class RecoverableKeyStoreManagerTest { assertThat(mRecoverableKeyStoreDb.getRecoveryServiceCertSerial(userId, uid, DEFAULT_ROOT_CERT_ALIAS)).isEqualTo(certSerial + 1); - assertThat(mRecoverableKeyStoreDb.getShouldCreateSnapshot(userId, uid)).isTrue(); + // There were no keys. + assertThat(mRecoverableKeyStoreDb.getShouldCreateSnapshot(userId, uid)).isFalse(); } @Test @@ -479,10 +507,12 @@ public class RecoverableKeyStoreManagerTest { mRecoverableKeyStoreManager.initRecoveryService(ROOT_CERTIFICATE_ALIAS, TestData.getCertXmlWithSerial(certSerial)); + + generateKeyAndSimulateSync(userId, uid, 10); + mRecoverableKeyStoreManager.initRecoveryService(ROOT_CERTIFICATE_ALIAS, TestData.getCertXmlWithSerial(certSerial)); - // If the second update succeeds, getShouldCreateSnapshot() will return true. assertThat(mRecoverableKeyStoreDb.getShouldCreateSnapshot(userId, uid)).isFalse(); } @@ -935,7 +965,6 @@ public class RecoverableKeyStoreManagerTest { assertThat(recoveredKeys).hasSize(1); assertThat(recoveredKeys).containsKey(TEST_ALIAS); - // TODO(76083050) Test the grant mechanism for the keys. } @Test @@ -974,7 +1003,6 @@ public class RecoverableKeyStoreManagerTest { assertThat(recoveredKeys).hasSize(1); assertThat(recoveredKeys).containsKey(TEST_ALIAS2); - // TODO(76083050) Test the grant mechanism for the keys. } @Test @@ -1016,6 +1044,9 @@ public class RecoverableKeyStoreManagerTest { byte[] serverParams = new byte[] { 1 }; mRecoverableKeyStoreManager.setServerParams(serverParams); + + generateKeyAndSimulateSync(userId, uid, 10); + mRecoverableKeyStoreManager.setServerParams(serverParams); assertThat(mRecoverableKeyStoreDb.getShouldCreateSnapshot(userId, uid)).isFalse(); @@ -1027,6 +1058,9 @@ public class RecoverableKeyStoreManagerTest { int userId = UserHandle.getCallingUserId(); mRecoverableKeyStoreManager.setServerParams(new byte[] { 1 }); + + generateKeyAndSimulateSync(userId, uid, 10); + mRecoverableKeyStoreManager.setServerParams(new byte[] { 2 }); assertThat(mRecoverableKeyStoreDb.getShouldCreateSnapshot(userId, uid)).isTrue(); @@ -1059,6 +1093,7 @@ public class RecoverableKeyStoreManagerTest { mRecoverableKeyStoreManager.setRecoverySecretTypes(secretTypes); + // There were no keys. assertThat(mRecoverableKeyStoreDb.getShouldCreateSnapshot(userId, uid)).isFalse(); } @@ -1070,6 +1105,9 @@ public class RecoverableKeyStoreManagerTest { int[] secretTypes = new int[] { 101 }; mRecoverableKeyStoreManager.setRecoverySecretTypes(secretTypes); + + generateKeyAndSimulateSync(userId, uid, 10); + mRecoverableKeyStoreManager.setRecoverySecretTypes(secretTypes); assertThat(mRecoverableKeyStoreDb.getShouldCreateSnapshot(userId, uid)).isFalse(); @@ -1081,6 +1119,11 @@ public class RecoverableKeyStoreManagerTest { int userId = UserHandle.getCallingUserId(); mRecoverableKeyStoreManager.setRecoverySecretTypes(new int[] { 101 }); + + assertThat(mRecoverableKeyStoreDb.getShouldCreateSnapshot(userId, uid)).isFalse(); + + generateKeyAndSimulateSync(userId, uid, 10); + mRecoverableKeyStoreManager.setRecoverySecretTypes(new int[] { 102 }); assertThat(mRecoverableKeyStoreDb.getShouldCreateSnapshot(userId, uid)).isTrue(); @@ -1102,9 +1145,8 @@ public class RecoverableKeyStoreManagerTest { int userId = UserHandle.getCallingUserId(); mRecoverableKeyStoreManager.setRecoverySecretTypes(new int[] { 1 }); - mRecoverableKeyStoreManager.generateKey(TEST_ALIAS); - // Pretend that key was synced - mRecoverableKeyStoreDb.setShouldCreateSnapshot(userId, uid, false); + generateKeyAndSimulateSync(userId, uid, 10); + mRecoverableKeyStoreManager.setRecoverySecretTypes(new int[] { 2 }); assertThat(mRecoverableKeyStoreDb.getShouldCreateSnapshot(userId, uid)).isTrue(); @@ -1175,6 +1217,14 @@ public class RecoverableKeyStoreManagerTest { return bytes; } + private void generateKeyAndSimulateSync(int userId, int uid, int snapshotVersion) + throws Exception{ + mRecoverableKeyStoreManager.generateKey(TEST_ALIAS); + // Simulate key sync. + mRecoverableKeyStoreDb.setSnapshotVersion(userId, uid, snapshotVersion); + mRecoverableKeyStoreDb.setShouldCreateSnapshot(userId, uid, false); + } + private AndroidKeyStoreSecretKey generateAndroidKeyStoreKey() throws Exception { KeyGenerator keyGenerator = KeyGenerator.getInstance( KEY_ALGORITHM,