From 23174b7eaeb93918451c36bbbfad94bafd44bdd6 Mon Sep 17 00:00:00 2001 From: Aseem Kumar Date: Tue, 3 Apr 2018 11:35:51 -0700 Subject: [PATCH] Throw ServiceSpecificException if calling app tries to initialize certificates with lower version. Earlier, the code just returned silently, giving no indication that updating certs failed. Change-Id: I3eb1b9f423791a655b47b3e76c20a170e2b632c0 Bug: 77533356 Test: runtest frameworks-services -p com.android.server.locksettings.recoverablekeystore --- .../security/keystore/recovery/RecoveryController.java | 14 ++++++++++++++ .../recoverablekeystore/RecoverableKeyStoreManager.java | 3 +++ .../RecoverableKeyStoreManagerTest.java | 16 +++++++++------- 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/core/java/android/security/keystore/recovery/RecoveryController.java b/core/java/android/security/keystore/recovery/RecoveryController.java index 281822a342f9..f351c5afa579 100644 --- a/core/java/android/security/keystore/recovery/RecoveryController.java +++ b/core/java/android/security/keystore/recovery/RecoveryController.java @@ -250,6 +250,16 @@ public class RecoveryController { */ public static final int ERROR_INVALID_CERTIFICATE = 28; + + /** + * Failed because the provided certificate contained serial version which is lower that the + * version device is already initialized with. It is not possible to downgrade serial version of + * the provided certificate. + * + * @hide + */ + public static final int ERROR_DOWNGRADE_CERTIFICATE = 29; + private final ILockSettings mBinder; private final KeyStore mKeyStore; @@ -340,6 +350,10 @@ public class RecoveryController { || e.errorCode == ERROR_INVALID_CERTIFICATE) { throw new CertificateException("Invalid certificate for recovery service", e); } + if (e.errorCode == ERROR_DOWNGRADE_CERTIFICATE) { + throw new CertificateException( + "Downgrading certificate serial version isn't supported.", e); + } throw wrapUnexpectedServiceSpecificException(e); } } 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 c09d7259e6c2..a6ee7e13348b 100644 --- a/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java +++ b/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java @@ -18,6 +18,7 @@ package com.android.server.locksettings.recoverablekeystore; import static android.security.keystore.recovery.RecoveryController.ERROR_BAD_CERTIFICATE_FORMAT; import static android.security.keystore.recovery.RecoveryController.ERROR_DECRYPTION_FAILED; +import static android.security.keystore.recovery.RecoveryController.ERROR_DOWNGRADE_CERTIFICATE; import static android.security.keystore.recovery.RecoveryController.ERROR_INSECURE_USER; import static android.security.keystore.recovery.RecoveryController.ERROR_INVALID_KEY_FORMAT; import static android.security.keystore.recovery.RecoveryController.ERROR_INVALID_CERTIFICATE; @@ -212,6 +213,8 @@ public class RecoverableKeyStoreManager { Log.i(TAG, "The cert file serial number is the same, so skip updating."); } else { Log.e(TAG, "The cert file serial number is older than the one in database."); + throw new ServiceSpecificException(ERROR_DOWNGRADE_CERTIFICATE, + "The cert file serial number is older than the one in database."); } return; } 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 f0d799f36707..9376bbb3ef1d 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 @@ -19,6 +19,7 @@ package com.android.server.locksettings.recoverablekeystore; import static android.security.keystore.recovery.KeyChainProtectionParams.TYPE_LOCKSCREEN; import static android.security.keystore.recovery.KeyChainProtectionParams.UI_FORMAT_PASSWORD; import static android.security.keystore.recovery.RecoveryController.ERROR_BAD_CERTIFICATE_FORMAT; +import static android.security.keystore.recovery.RecoveryController.ERROR_DOWNGRADE_CERTIFICATE; import static android.security.keystore.recovery.RecoveryController.ERROR_INVALID_CERTIFICATE; import static com.google.common.truth.Truth.assertThat; @@ -409,19 +410,20 @@ public class RecoverableKeyStoreManagerTest { } @Test - public void initRecoveryService_ignoresSmallerSerial() throws Exception { + public void initRecoveryService_throwsExceptionOnSmallerSerial() throws Exception { int uid = Binder.getCallingUid(); int userId = UserHandle.getCallingUserId(); long certSerial = 1000L; mRecoverableKeyStoreManager.initRecoveryService(ROOT_CERTIFICATE_ALIAS, TestData.getCertXmlWithSerial(certSerial)); - mRecoverableKeyStoreManager.initRecoveryService(ROOT_CERTIFICATE_ALIAS, - TestData.getCertXmlWithSerial(certSerial - 1)); - - assertThat(mRecoverableKeyStoreDb.getRecoveryServiceCertSerial(userId, uid, - DEFAULT_ROOT_CERT_ALIAS)).isEqualTo(certSerial); - assertThat(mRecoverableKeyStoreDb.getShouldCreateSnapshot(userId, uid)).isFalse(); + try { + mRecoverableKeyStoreManager.initRecoveryService(ROOT_CERTIFICATE_ALIAS, + TestData.getCertXmlWithSerial(certSerial - 1)); + fail(); + } catch (ServiceSpecificException e) { + assertThat(e.errorCode).isEqualTo(ERROR_DOWNGRADE_CERTIFICATE); + } } @Test -- 2.11.0