Merge "Check the return values after updating Recoverable KeyStore Database." into pi-dev
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 5d71cc7..f1951b1 100644
--- a/services/core/java/com/android/server/locksettings/recoverablekeystore/KeySyncTask.java
+++ b/services/core/java/com/android/server/locksettings/recoverablekeystore/KeySyncTask.java
@@ -33,6 +33,7 @@
import com.android.server.locksettings.recoverablekeystore.storage.RecoverableKeyStoreDb;
import com.android.server.locksettings.recoverablekeystore.storage.RecoverySnapshotStorage;
+import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.ByteOrder;
import java.nio.charset.StandardCharsets;
@@ -176,7 +177,11 @@
List<Integer> recoveryAgents = mRecoverableKeyStoreDb.getRecoveryAgents(mUserId);
for (int uid : recoveryAgents) {
- syncKeysForAgent(uid);
+ try {
+ syncKeysForAgent(uid);
+ } catch (IOException e) {
+ Log.e(TAG, "IOException during sync for agent " + uid, e);
+ }
}
if (recoveryAgents.isEmpty()) {
Log.w(TAG, "No recovery agent initialized for user " + mUserId);
@@ -189,13 +194,13 @@
&& mCredentialType != LockPatternUtils.CREDENTIAL_TYPE_PASSWORD;
}
- private void syncKeysForAgent(int recoveryAgentUid) {
- boolean recreateCurrentVersion = false;
+ private void syncKeysForAgent(int recoveryAgentUid) throws IOException {
+ boolean shouldRecreateCurrentVersion = false;
if (!shouldCreateSnapshot(recoveryAgentUid)) {
- recreateCurrentVersion =
+ shouldRecreateCurrentVersion =
(mRecoverableKeyStoreDb.getSnapshotVersion(mUserId, recoveryAgentUid) != null)
&& (mRecoverySnapshotStorage.get(recoveryAgentUid) == null);
- if (recreateCurrentVersion) {
+ if (shouldRecreateCurrentVersion) {
Log.d(TAG, "Recreating most recent snapshot");
} else {
Log.d(TAG, "Key sync not needed.");
@@ -203,12 +208,12 @@
}
}
- PublicKey publicKey;
String rootCertAlias =
mRecoverableKeyStoreDb.getActiveRootOfTrust(mUserId, recoveryAgentUid);
rootCertAlias = mTestOnlyInsecureCertificateHelper
.getDefaultCertificateAliasIfEmpty(rootCertAlias);
+ PublicKey publicKey;
CertPath certPath = mRecoverableKeyStoreDb.getRecoveryServiceCertPath(mUserId,
recoveryAgentUid, rootCertAlias);
if (certPath != null) {
@@ -260,19 +265,22 @@
Log.e(TAG, "Failed to load recoverable keys for sync", e);
return;
} catch (InsecureUserException e) {
- Log.wtf(TAG, "A screen unlock triggered the key sync flow, so user must have "
+ Log.e(TAG, "A screen unlock triggered the key sync flow, so user must have "
+ "lock screen. This should be impossible.", e);
return;
} catch (BadPlatformKeyException e) {
- Log.wtf(TAG, "Loaded keys for same generation ID as platform key, so "
+ Log.e(TAG, "Loaded keys for same generation ID as platform key, so "
+ "BadPlatformKeyException should be impossible.", e);
return;
+ } catch (IOException e) {
+ Log.e(TAG, "Local database error.", e);
+ return;
}
-
// Only include insecure key material for test
if (mTestOnlyInsecureCertificateHelper.isTestOnlyCertificateAlias(rootCertAlias)) {
rawKeys = mTestOnlyInsecureCertificateHelper.keepOnlyWhitelistedInsecureKeys(rawKeys);
}
+
SecretKey recoveryKey;
try {
recoveryKey = generateRecoveryKey();
@@ -323,6 +331,7 @@
Log.e(TAG,"Could not encrypt with recovery key", e);
return;
}
+
KeyDerivationParams keyDerivationParams;
if (useScryptToHashCredential) {
keyDerivationParams = KeyDerivationParams.createScryptParams(
@@ -330,7 +339,7 @@
} else {
keyDerivationParams = KeyDerivationParams.createSha256Params(salt);
}
- KeyChainProtectionParams metadata = new KeyChainProtectionParams.Builder()
+ KeyChainProtectionParams keyChainProtectionParams = new KeyChainProtectionParams.Builder()
.setUserSecretType(TYPE_LOCKSCREEN)
.setLockScreenUiFormat(getUiFormat(mCredentialType, mCredential))
.setKeyDerivationParams(keyDerivationParams)
@@ -338,13 +347,11 @@
.build();
ArrayList<KeyChainProtectionParams> metadataList = new ArrayList<>();
- metadataList.add(metadata);
-
- // If application keys are not updated, snapshot will not be created on next unlock.
- mRecoverableKeyStoreDb.setShouldCreateSnapshot(mUserId, recoveryAgentUid, false);
+ metadataList.add(keyChainProtectionParams);
KeyChainSnapshot.Builder keyChainSnapshotBuilder = new KeyChainSnapshot.Builder()
- .setSnapshotVersion(getSnapshotVersion(recoveryAgentUid, recreateCurrentVersion))
+ .setSnapshotVersion(
+ getSnapshotVersion(recoveryAgentUid, shouldRecreateCurrentVersion))
.setMaxAttempts(TRUSTED_HARDWARE_MAX_ATTEMPTS)
.setCounterId(counterId)
.setServerParams(vaultHandle)
@@ -360,25 +367,38 @@
}
mRecoverySnapshotStorage.put(recoveryAgentUid, keyChainSnapshotBuilder.build());
mSnapshotListenersStorage.recoverySnapshotAvailable(recoveryAgentUid);
+
+ mRecoverableKeyStoreDb.setShouldCreateSnapshot(mUserId, recoveryAgentUid, false);
}
@VisibleForTesting
- int getSnapshotVersion(int recoveryAgentUid, boolean recreateCurrentVersion) {
+ int getSnapshotVersion(int recoveryAgentUid, boolean shouldRecreateCurrentVersion)
+ throws IOException {
Long snapshotVersion = mRecoverableKeyStoreDb.getSnapshotVersion(mUserId, recoveryAgentUid);
- if (recreateCurrentVersion) {
+ if (shouldRecreateCurrentVersion) {
// version shouldn't be null at this moment.
snapshotVersion = snapshotVersion == null ? 1 : snapshotVersion;
} else {
snapshotVersion = snapshotVersion == null ? 1 : snapshotVersion + 1;
}
- mRecoverableKeyStoreDb.setSnapshotVersion(mUserId, recoveryAgentUid, snapshotVersion);
+
+ long updatedRows = mRecoverableKeyStoreDb.setSnapshotVersion(mUserId, recoveryAgentUid,
+ snapshotVersion);
+ if (updatedRows < 0) {
+ Log.e(TAG, "Failed to set the snapshot version in the local DB.");
+ throw new IOException("Failed to set the snapshot version in the local DB.");
+ }
return snapshotVersion.intValue();
}
- private long generateAndStoreCounterId(int recoveryAgentUid) {
+ private long generateAndStoreCounterId(int recoveryAgentUid) throws IOException {
long counter = new SecureRandom().nextLong();
- mRecoverableKeyStoreDb.setCounterId(mUserId, recoveryAgentUid, counter);
+ long updatedRows = mRecoverableKeyStoreDb.setCounterId(mUserId, recoveryAgentUid, counter);
+ if (updatedRows < 0) {
+ Log.e(TAG, "Failed to set the snapshot version in the local DB.");
+ throw new IOException("Failed to set counterId in the local DB.");
+ }
return counter;
}
@@ -388,7 +408,7 @@
private Map<String, SecretKey> getKeysToSync(int recoveryAgentUid)
throws InsecureUserException, KeyStoreException, UnrecoverableKeyException,
NoSuchAlgorithmException, NoSuchPaddingException, BadPlatformKeyException,
- InvalidKeyException, InvalidAlgorithmParameterException {
+ InvalidKeyException, InvalidAlgorithmParameterException, IOException {
PlatformDecryptionKey decryptKey = mPlatformKeyManager.getDecryptKey(mUserId);;
Map<String, WrappedKey> wrappedKeys = mRecoverableKeyStoreDb.getAllKeys(
mUserId, recoveryAgentUid, decryptKey.getGenerationId());
@@ -440,7 +460,7 @@
*
* @return The salt.
*/
- private byte[] generateSalt() {
+ private static byte[] generateSalt() {
byte[] salt = new byte[SALT_LENGTH_BYTES];
new SecureRandom().nextBytes(salt);
return salt;
diff --git a/services/core/java/com/android/server/locksettings/recoverablekeystore/KeySyncUtils.java b/services/core/java/com/android/server/locksettings/recoverablekeystore/KeySyncUtils.java
index 57fb74d..8e6f9cb 100644
--- a/services/core/java/com/android/server/locksettings/recoverablekeystore/KeySyncUtils.java
+++ b/services/core/java/com/android/server/locksettings/recoverablekeystore/KeySyncUtils.java
@@ -37,7 +37,7 @@
import javax.crypto.SecretKey;
/**
- * Utility functions for the flow where the RecoveryManager syncs keys with remote storage.
+ * Utility functions for the flow where the RecoveryController syncs keys with remote storage.
*
* @hide
*/
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 52394d2..9ca052b 100644
--- a/services/core/java/com/android/server/locksettings/recoverablekeystore/PlatformKeyManager.java
+++ b/services/core/java/com/android/server/locksettings/recoverablekeystore/PlatformKeyManager.java
@@ -157,11 +157,13 @@
* @throws NoSuchAlgorithmException if AES is unavailable - should never happen.
* @throws KeyStoreException if there is an error in AndroidKeyStore.
* @throws InsecureUserException if the user does not have a lock screen set.
+ * @throws IOException if there was an issue with local database update.
*
* @hide
*/
- public void regenerate(int userId)
- throws NoSuchAlgorithmException, KeyStoreException, InsecureUserException {
+ @VisibleForTesting
+ void regenerate(int userId)
+ throws NoSuchAlgorithmException, KeyStoreException, InsecureUserException, IOException {
if (!isAvailable(userId)) {
throw new InsecureUserException(String.format(
Locale.US, "%d does not have a lock screen set.", userId));
@@ -187,11 +189,12 @@
* @throws UnrecoverableKeyException if the key could not be recovered.
* @throws NoSuchAlgorithmException if AES is unavailable - should never occur.
* @throws InsecureUserException if the user does not have a lock screen set.
+ * @throws IOException if there was an issue with local database update.
*
* @hide
*/
public PlatformEncryptionKey getEncryptKey(int userId) throws KeyStoreException,
- UnrecoverableKeyException, NoSuchAlgorithmException, InsecureUserException {
+ UnrecoverableKeyException, NoSuchAlgorithmException, InsecureUserException, IOException {
init(userId);
try {
// Try to see if the decryption key is still accessible before using the encryption key.
@@ -239,11 +242,12 @@
* @throws UnrecoverableKeyException if the key could not be recovered.
* @throws NoSuchAlgorithmException if AES is unavailable - should never occur.
* @throws InsecureUserException if the user does not have a lock screen set.
+ * @throws IOException if there was an issue with local database update.
*
* @hide
*/
public PlatformDecryptionKey getDecryptKey(int userId) throws KeyStoreException,
- UnrecoverableKeyException, NoSuchAlgorithmException, InsecureUserException {
+ UnrecoverableKeyException, NoSuchAlgorithmException, InsecureUserException, IOException {
init(userId);
try {
return getDecryptKeyInternal(userId);
@@ -286,11 +290,12 @@
* @param userId The ID of the user to whose lock screen the platform key must be bound.
* @throws KeyStoreException if there was an error in AndroidKeyStore.
* @throws NoSuchAlgorithmException if AES is unavailable - should never happen.
+ * @throws IOException if there was an issue with local database update.
*
* @hide
*/
void init(int userId)
- throws KeyStoreException, NoSuchAlgorithmException, InsecureUserException {
+ throws KeyStoreException, NoSuchAlgorithmException, InsecureUserException, IOException {
if (!isAvailable(userId)) {
throw new InsecureUserException(String.format(
Locale.US, "%d does not have a lock screen set.", userId));
@@ -347,9 +352,13 @@
/**
* Sets the current generation ID to {@code generationId}.
+ * @throws IOException if there was an issue with local database update.
*/
- private void setGenerationId(int userId, int generationId) {
- mDatabase.setPlatformKeyGenerationId(userId, generationId);
+ private void setGenerationId(int userId, int generationId) throws IOException {
+ long updatedRows = mDatabase.setPlatformKeyGenerationId(userId, generationId);
+ if (updatedRows < 0) {
+ throw new IOException("Failed to set the platform key in the local DB.");
+ }
}
/**
@@ -370,9 +379,10 @@
* @throws NoSuchAlgorithmException if AES is unavailable. This should never happen, as it is
* available since API version 1.
* @throws KeyStoreException if there was an issue loading the keys into AndroidKeyStore.
+ * @throws IOException if there was an issue with local database update.
*/
private void generateAndLoadKey(int userId, int generationId)
- throws NoSuchAlgorithmException, KeyStoreException {
+ throws NoSuchAlgorithmException, KeyStoreException, IOException {
String encryptAlias = getEncryptAlias(userId, generationId);
String decryptAlias = getDecryptAlias(userId, generationId);
// SecretKey implementation doesn't provide reliable way to destroy the secret
diff --git a/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyGenerator.java b/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyGenerator.java
index 396862d..c249468 100644
--- a/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyGenerator.java
+++ b/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyGenerator.java
@@ -24,6 +24,7 @@
import java.security.KeyStoreException;
import java.security.NoSuchAlgorithmException;
import java.util.Locale;
+import android.util.Log;
import javax.crypto.KeyGenerator;
import javax.crypto.SecretKey;
@@ -40,6 +41,7 @@
*/
public class RecoverableKeyGenerator {
+ private static final String TAG = "PlatformKeyGen";
private static final int RESULT_CANNOT_INSERT_ROW = -1;
private static final String SECRET_KEY_ALGORITHM = "AES";
@@ -104,7 +106,11 @@
Locale.US, "Failed writing (%d, %s) to database.", uid, alias));
}
- mDatabase.setShouldCreateSnapshot(userId, uid, true);
+ long updatedRows = mDatabase.setShouldCreateSnapshot(userId, uid, true);
+ if (updatedRows < 0) {
+ Log.e(TAG, "Failed to set the shoudCreateSnapshot flag in the local DB.");
+ }
+
return key.getEncoded();
}
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 09906e4..fc5184d 100644
--- a/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java
+++ b/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java
@@ -57,6 +57,7 @@
import com.android.server.locksettings.recoverablekeystore.storage.RecoverySessionStorage;
import com.android.server.locksettings.recoverablekeystore.storage.RecoverySnapshotStorage;
+import java.io.IOException;
import java.security.InvalidKeyException;
import java.security.KeyFactory;
import java.security.KeyStoreException;
@@ -189,11 +190,14 @@
if (activeRootAlias == null) {
Log.d(TAG, "Root of trust for recovery agent + " + uid
+ " is assigned for the first time to " + rootCertificateAlias);
- mDatabase.setActiveRootOfTrust(userId, uid, rootCertificateAlias);
} else if (!activeRootAlias.equals(rootCertificateAlias)) {
Log.i(TAG, "Root of trust for recovery agent " + uid + " is changed to "
+ rootCertificateAlias + " from " + activeRootAlias);
- mDatabase.setActiveRootOfTrust(userId, uid, rootCertificateAlias);
+ }
+ long updatedRows = mDatabase.setActiveRootOfTrust(userId, uid, rootCertificateAlias);
+ if (updatedRows < 0) {
+ throw new ServiceSpecificException(ERROR_SERVICE_INTERNAL_ERROR,
+ "Failed to set the root of trust in the local DB.");
}
CertXml certXml;
@@ -236,17 +240,32 @@
// Save the chosen and validated certificate into database
try {
Log.d(TAG, "Saving the randomly chosen endpoint certificate to database");
- if (mDatabase.setRecoveryServiceCertPath(userId, uid, rootCertificateAlias,
- certPath) > 0) {
- mDatabase.setRecoveryServiceCertSerial(userId, uid, rootCertificateAlias,
- newSerial);
+ long updatedCertPathRows = mDatabase.setRecoveryServiceCertPath(userId, uid,
+ rootCertificateAlias, certPath);
+ if (updatedCertPathRows > 0) {
+ long updatedCertSerialRows = mDatabase.setRecoveryServiceCertSerial(userId, uid,
+ rootCertificateAlias, newSerial);
+ if (updatedCertSerialRows < 0) {
+ // Ideally CertPath and CertSerial should be updated together in single
+ // transaction, but since their mismatch doesn't create many problems
+ // extra complexity is unnecessary.
+ throw new ServiceSpecificException(ERROR_SERVICE_INTERNAL_ERROR,
+ "Failed to set the certificate serial number in the local DB.");
+ }
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());
+ long updatedCounterIdRows =
+ mDatabase.setCounterId(userId, uid, new SecureRandom().nextLong());
+ if (updatedCounterIdRows < 0) {
+ Log.e(TAG, "Failed to set the counter id in the local DB.");
+ }
+ } else if (updatedCertPathRows < 0) {
+ throw new ServiceSpecificException(ERROR_SERVICE_INTERNAL_ERROR,
+ "Failed to set the certificate path in the local DB.");
}
} catch (CertificateEncodingException e) {
Log.e(TAG, "Failed to encode CertPath", e);
@@ -340,7 +359,7 @@
}
long updatedRows = mDatabase.setServerParams(userId, uid, serverParams);
- if (updatedRows < 1) {
+ if (updatedRows < 0) {
throw new ServiceSpecificException(
ERROR_SERVICE_INTERNAL_ERROR, "Database failure trying to set server params.");
}
@@ -364,7 +383,12 @@
public void setRecoveryStatus(@NonNull String alias, int status) throws RemoteException {
checkRecoverKeyStorePermission();
Preconditions.checkNotNull(alias, "alias is null");
- mDatabase.setRecoveryStatus(Binder.getCallingUid(), alias, status);
+ long updatedRows = mDatabase.setRecoveryStatus(Binder.getCallingUid(), alias, status);
+ if (updatedRows < 0) {
+ throw new ServiceSpecificException(
+ ERROR_SERVICE_INTERNAL_ERROR,
+ "Failed to set the key recovery status in the local DB.");
+ }
}
/**
@@ -400,7 +424,7 @@
}
long updatedRows = mDatabase.setRecoverySecretTypes(userId, uid, secretTypes);
- if (updatedRows < 1) {
+ if (updatedRows < 0) {
throw new ServiceSpecificException(ERROR_SERVICE_INTERNAL_ERROR,
"Database error trying to set secret types.");
}
@@ -664,7 +688,7 @@
} catch (NoSuchAlgorithmException e) {
// Impossible: all algorithms must be supported by AOSP
throw new RuntimeException(e);
- } catch (KeyStoreException | UnrecoverableKeyException e) {
+ } catch (KeyStoreException | UnrecoverableKeyException | IOException e) {
throw new ServiceSpecificException(ERROR_SERVICE_INTERNAL_ERROR, e.getMessage());
} catch (InsecureUserException e) {
throw new ServiceSpecificException(ERROR_INSECURE_USER, e.getMessage());
@@ -713,7 +737,7 @@
} catch (NoSuchAlgorithmException e) {
// Impossible: all algorithms must be supported by AOSP
throw new RuntimeException(e);
- } catch (KeyStoreException | UnrecoverableKeyException e) {
+ } catch (KeyStoreException | UnrecoverableKeyException | IOException e) {
throw new ServiceSpecificException(ERROR_SERVICE_INTERNAL_ERROR, e.getMessage());
} catch (InsecureUserException e) {
throw new ServiceSpecificException(ERROR_INSECURE_USER, e.getMessage());
diff --git a/services/core/java/com/android/server/locksettings/recoverablekeystore/storage/RecoverableKeyStoreDbContract.java b/services/core/java/com/android/server/locksettings/recoverablekeystore/storage/RecoverableKeyStoreDbContract.java
index 7ee809a..22e77cc 100644
--- a/services/core/java/com/android/server/locksettings/recoverablekeystore/storage/RecoverableKeyStoreDbContract.java
+++ b/services/core/java/com/android/server/locksettings/recoverablekeystore/storage/RecoverableKeyStoreDbContract.java
@@ -64,7 +64,7 @@
static final String COLUMN_NAME_LAST_SYNCED_AT = "last_synced_at";
/**
- * Status of the key sync {@code RecoveryManager#setRecoveryStatus}
+ * Status of the key sync {@code RecoveryController#setRecoveryStatus}
*/
static final String COLUMN_NAME_RECOVERY_STATUS = "recovery_status";
}