Merge "Check the return values after updating Recoverable KeyStore Database." into pi-dev

This commit is contained in:
Dmitry Dementyev
2018-05-18 17:56:25 +00:00
committed by Android (Google) Code Review
6 changed files with 105 additions and 45 deletions

View File

@@ -33,6 +33,7 @@ import com.android.internal.widget.LockPatternUtils;
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 @@ public class KeySyncTask implements Runnable {
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 @@ public class KeySyncTask implements Runnable {
&& 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 @@ public class KeySyncTask implements Runnable {
}
}
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 @@ public class KeySyncTask implements Runnable {
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 @@ public class KeySyncTask implements Runnable {
Log.e(TAG,"Could not encrypt with recovery key", e);
return;
}
KeyDerivationParams keyDerivationParams;
if (useScryptToHashCredential) {
keyDerivationParams = KeyDerivationParams.createScryptParams(
@@ -330,7 +339,7 @@ public class KeySyncTask implements Runnable {
} 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 @@ public class KeySyncTask implements Runnable {
.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 @@ public class KeySyncTask implements Runnable {
}
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 @@ public class KeySyncTask implements Runnable {
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 @@ public class KeySyncTask implements Runnable {
*
* @return The salt.
*/
private byte[] generateSalt() {
private static byte[] generateSalt() {
byte[] salt = new byte[SALT_LENGTH_BYTES];
new SecureRandom().nextBytes(salt);
return salt;

View File

@@ -37,7 +37,7 @@ import javax.crypto.KeyGenerator;
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
*/

View File

@@ -157,11 +157,13 @@ public class PlatformKeyManager {
* @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 @@ public class PlatformKeyManager {
* @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 @@ public class PlatformKeyManager {
* @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 @@ public class PlatformKeyManager {
* @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 @@ public class PlatformKeyManager {
/**
* 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 @@ public class PlatformKeyManager {
* @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

View File

@@ -24,6 +24,7 @@ import java.security.InvalidKeyException;
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 @@ import javax.crypto.spec.SecretKeySpec;
*/
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 @@ public class RecoverableKeyGenerator {
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();
}

View File

@@ -57,6 +57,7 @@ import com.android.server.locksettings.recoverablekeystore.storage.RecoverableKe
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 @@ public class RecoverableKeyStoreManager {
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 @@ public class RecoverableKeyStoreManager {
// 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 @@ public class RecoverableKeyStoreManager {
}
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 class RecoverableKeyStoreManager {
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 @@ public class RecoverableKeyStoreManager {
}
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 @@ public class RecoverableKeyStoreManager {
} 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 @@ public class RecoverableKeyStoreManager {
} 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());

View File

@@ -64,7 +64,7 @@ class RecoverableKeyStoreDbContract {
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";
}