Merge "Throw an exception if the given root alias is unknown" into pi-dev
diff --git a/core/java/android/security/keystore/recovery/TrustedRootCertificates.java b/core/java/android/security/keystore/recovery/TrustedRootCertificates.java
index 63faac3..a96dab6 100644
--- a/core/java/android/security/keystore/recovery/TrustedRootCertificates.java
+++ b/core/java/android/security/keystore/recovery/TrustedRootCertificates.java
@@ -40,7 +40,7 @@
/**
* Certificate used for client-side end-to-end encryption tests.
* When recovery controller is initialized with the certificate, recovery snapshots will only
- * contain application keys started with {@link INSECURE_KEY_ALIAS}.
+ * contain application keys started with {@link #INSECURE_KEY_ALIAS_PREFIX}.
* Recovery snapshot will only be created if device is unlocked with password started with
* {@link #INSECURE_PASSWORD_PREFIX}.
*
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 b0afac2..4c4176a 100644
--- a/services/core/java/com/android/server/locksettings/recoverablekeystore/KeySyncTask.java
+++ b/services/core/java/com/android/server/locksettings/recoverablekeystore/KeySyncTask.java
@@ -19,12 +19,10 @@
import static android.security.keystore.recovery.KeyChainProtectionParams.TYPE_LOCKSCREEN;
import android.annotation.Nullable;
-import android.annotation.NonNull;
import android.content.Context;
import android.security.keystore.recovery.KeyChainProtectionParams;
import android.security.keystore.recovery.KeyChainSnapshot;
import android.security.keystore.recovery.KeyDerivationParams;
-import android.security.keystore.recovery.TrustedRootCertificates;
import android.security.keystore.recovery.WrappedApplicationKey;
import android.util.Log;
@@ -218,10 +216,10 @@
return;
}
- if (mTestOnlyInsecureCertificateHelper.isTestOnlyCertificate(rootCertAlias)) {
+ if (mTestOnlyInsecureCertificateHelper.isTestOnlyCertificateAlias(rootCertAlias)) {
Log.w(TAG, "Insecure root certificate is used by recovery agent "
+ recoveryAgentUid);
- if (mTestOnlyInsecureCertificateHelper.doesCredentailSupportInsecureMode(
+ if (mTestOnlyInsecureCertificateHelper.doesCredentialSupportInsecureMode(
mCredentialType, mCredential)) {
Log.w(TAG, "Whitelisted credential is used to generate snapshot by "
+ "recovery agent "+ recoveryAgentUid);
@@ -252,7 +250,7 @@
}
// Only include insecure key material for test
- if (mTestOnlyInsecureCertificateHelper.isTestOnlyCertificate(rootCertAlias)) {
+ if (mTestOnlyInsecureCertificateHelper.isTestOnlyCertificateAlias(rootCertAlias)) {
rawKeys = mTestOnlyInsecureCertificateHelper.keepOnlyWhitelistedInsecureKeys(rawKeys);
}
SecretKey recoveryKey;
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 9f6ac10..a64a559 100644
--- a/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java
+++ b/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java
@@ -174,9 +174,13 @@
checkRecoverKeyStorePermission();
int userId = UserHandle.getCallingUserId();
int uid = Binder.getCallingUid();
+
rootCertificateAlias
= mTestCertHelper.getDefaultCertificateAliasIfEmpty(rootCertificateAlias);
-
+ if (!mTestCertHelper.isValidRootCertificateAlias(rootCertificateAlias)) {
+ throw new ServiceSpecificException(
+ ERROR_INVALID_CERTIFICATE, "Invalid root certificate alias");
+ }
// Always set active alias to the argument of the last call to initRecoveryService method,
// even if cert file is incorrect.
String activeRootAlias = mDatabase.getActiveRootOfTrust(userId, uid);
diff --git a/services/core/java/com/android/server/locksettings/recoverablekeystore/TestOnlyInsecureCertificateHelper.java b/services/core/java/com/android/server/locksettings/recoverablekeystore/TestOnlyInsecureCertificateHelper.java
index 490f733..5ba3cce 100644
--- a/services/core/java/com/android/server/locksettings/recoverablekeystore/TestOnlyInsecureCertificateHelper.java
+++ b/services/core/java/com/android/server/locksettings/recoverablekeystore/TestOnlyInsecureCertificateHelper.java
@@ -51,7 +51,7 @@
public @NonNull X509Certificate
getRootCertificate(String rootCertificateAlias) throws RemoteException {
rootCertificateAlias = getDefaultCertificateAliasIfEmpty(rootCertificateAlias);
- if (isTestOnlyCertificate(rootCertificateAlias)) {
+ if (isTestOnlyCertificateAlias(rootCertificateAlias)) {
return TrustedRootCertificates.getTestOnlyInsecureCertificate();
}
@@ -74,12 +74,17 @@
return rootCertificateAlias;
}
- public boolean isTestOnlyCertificate(String rootCertificateAlias) {
+ public boolean isTestOnlyCertificateAlias(String rootCertificateAlias) {
return TrustedRootCertificates.TEST_ONLY_INSECURE_CERTIFICATE_ALIAS
.equals(rootCertificateAlias);
}
- public boolean doesCredentailSupportInsecureMode(int credentialType, String credential) {
+ public boolean isValidRootCertificateAlias(String rootCertificateAlias) {
+ return TrustedRootCertificates.getRootCertificates().containsKey(rootCertificateAlias)
+ || isTestOnlyCertificateAlias(rootCertificateAlias);
+ }
+
+ public boolean doesCredentialSupportInsecureMode(int credentialType, String credential) {
return (credentialType == LockPatternUtils.CREDENTIAL_TYPE_PASSWORD)
&& (credential != null)
&& credential.startsWith(TrustedRootCertificates.INSECURE_PASSWORD_PREFIX);
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 b8d2c3e..7ce5904 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
@@ -301,8 +301,8 @@
TEST_USER_ID, TEST_RECOVERY_AGENT_UID, TEST_ROOT_CERT_ALIAS, TestData.CERT_PATH_1);
// Enter test mode with whitelisted credentials
- when(mTestOnlyInsecureCertificateHelper.isTestOnlyCertificate(any())).thenReturn(true);
- when(mTestOnlyInsecureCertificateHelper.doesCredentailSupportInsecureMode(anyInt(), any()))
+ when(mTestOnlyInsecureCertificateHelper.isTestOnlyCertificateAlias(any())).thenReturn(true);
+ when(mTestOnlyInsecureCertificateHelper.doesCredentialSupportInsecureMode(anyInt(), any()))
.thenReturn(true);
mKeySyncTask.run();
@@ -311,7 +311,7 @@
// run whitelist checks
verify(mTestOnlyInsecureCertificateHelper)
- .doesCredentailSupportInsecureMode(anyInt(), any());
+ .doesCredentialSupportInsecureMode(anyInt(), any());
verify(mTestOnlyInsecureCertificateHelper)
.keepOnlyWhitelistedInsecureKeys(any());
@@ -331,8 +331,8 @@
TEST_USER_ID, TEST_RECOVERY_AGENT_UID, TEST_ROOT_CERT_ALIAS, TestData.CERT_PATH_1);
// Enter test mode with non whitelisted credentials
- when(mTestOnlyInsecureCertificateHelper.isTestOnlyCertificate(any())).thenReturn(true);
- when(mTestOnlyInsecureCertificateHelper.doesCredentailSupportInsecureMode(anyInt(), any()))
+ when(mTestOnlyInsecureCertificateHelper.isTestOnlyCertificateAlias(any())).thenReturn(true);
+ when(mTestOnlyInsecureCertificateHelper.doesCredentialSupportInsecureMode(anyInt(), any()))
.thenReturn(false);
mKeySyncTask.run();
@@ -340,7 +340,7 @@
verify(mTestOnlyInsecureCertificateHelper)
.getDefaultCertificateAliasIfEmpty(eq(TEST_ROOT_CERT_ALIAS));
verify(mTestOnlyInsecureCertificateHelper)
- .doesCredentailSupportInsecureMode(anyInt(), any());
+ .doesCredentialSupportInsecureMode(anyInt(), any());
}
@Test
@@ -358,11 +358,11 @@
verify(mTestOnlyInsecureCertificateHelper)
.getDefaultCertificateAliasIfEmpty(eq(TEST_ROOT_CERT_ALIAS));
verify(mTestOnlyInsecureCertificateHelper, atLeast(1))
- .isTestOnlyCertificate(eq(TEST_ROOT_CERT_ALIAS));
+ .isTestOnlyCertificateAlias(eq(TEST_ROOT_CERT_ALIAS));
// no whitelists check
verify(mTestOnlyInsecureCertificateHelper, never())
- .doesCredentailSupportInsecureMode(anyInt(), any());
+ .doesCredentialSupportInsecureMode(anyInt(), any());
verify(mTestOnlyInsecureCertificateHelper, never())
.keepOnlyWhitelistedInsecureKeys(any());
}
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 5efe5d2..e65c31e 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
@@ -456,6 +456,17 @@
}
@Test
+ public void initRecoveryService_throwsIfUnknownRootCertAlias() throws Exception {
+ try {
+ mRecoverableKeyStoreManager.initRecoveryService(
+ "unknown-root-cert-alias", TestData.getCertXml());
+ fail("should have thrown");
+ } catch (ServiceSpecificException e) {
+ assertThat(e.errorCode).isEqualTo(ERROR_INVALID_CERTIFICATE);
+ }
+ }
+
+ @Test
public void initRecoveryServiceWithSigFile_succeeds() throws Exception {
int uid = Binder.getCallingUid();
int userId = UserHandle.getCallingUserId();
@@ -563,8 +574,6 @@
eq(Manifest.permission.RECOVER_KEYSTORE), any());
}
- // TODO: Add tests for non-existing cert alias
-
@Test
public void startRecoverySessionWithCertPath_storesTheSessionInfo() throws Exception {
mRecoverableKeyStoreManager.startRecoverySessionWithCertPath(
diff --git a/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/TestOnlyInsecureCertificateHelperTest.java b/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/TestOnlyInsecureCertificateHelperTest.java
index bc50c9e..67436cc 100644
--- a/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/TestOnlyInsecureCertificateHelperTest.java
+++ b/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/TestOnlyInsecureCertificateHelperTest.java
@@ -2,12 +2,9 @@
import static com.google.common.truth.Truth.assertThat;
-import android.os.RemoteException;
-import android.os.ServiceSpecificException;
import android.security.keystore.recovery.TrustedRootCertificates;
import android.support.test.filters.SmallTest;
import android.support.test.runner.AndroidJUnit4;
-import android.util.Log;
import com.android.internal.widget.LockPatternUtils;
@@ -15,7 +12,6 @@
import org.junit.runner.RunWith;
import java.util.HashMap;
-import java.security.cert.X509Certificate;
import java.util.Map;
import javax.crypto.SecretKey;
@@ -27,40 +23,40 @@
@Test
public void testDoesCredentailSupportInsecureMode_forNonWhitelistedPassword() throws Exception {
- assertThat(mHelper.doesCredentailSupportInsecureMode(
+ assertThat(mHelper.doesCredentialSupportInsecureMode(
LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, "secret12345")).isFalse();
- assertThat(mHelper.doesCredentailSupportInsecureMode(
+ assertThat(mHelper.doesCredentialSupportInsecureMode(
LockPatternUtils.CREDENTIAL_TYPE_PASSWORD, "1234")).isFalse();
}
@Test
public void testDoesCredentailSupportInsecureMode_forWhitelistedPassword() throws Exception {
- assertThat(mHelper.doesCredentailSupportInsecureMode(
+ assertThat(mHelper.doesCredentialSupportInsecureMode(
LockPatternUtils.CREDENTIAL_TYPE_PASSWORD,
TrustedRootCertificates.INSECURE_PASSWORD_PREFIX)).isTrue();
- assertThat(mHelper.doesCredentailSupportInsecureMode(
+ assertThat(mHelper.doesCredentialSupportInsecureMode(
LockPatternUtils.CREDENTIAL_TYPE_PASSWORD,
TrustedRootCertificates.INSECURE_PASSWORD_PREFIX + "12")).isTrue();
}
@Test
public void testDoesCredentailSupportInsecureMode_Pattern() throws Exception {
- assertThat(mHelper.doesCredentailSupportInsecureMode(
+ assertThat(mHelper.doesCredentialSupportInsecureMode(
LockPatternUtils.CREDENTIAL_TYPE_PATTERN,
TrustedRootCertificates.INSECURE_PASSWORD_PREFIX)).isFalse();
- assertThat(mHelper.doesCredentailSupportInsecureMode(
+ assertThat(mHelper.doesCredentialSupportInsecureMode(
LockPatternUtils.CREDENTIAL_TYPE_NONE,
TrustedRootCertificates.INSECURE_PASSWORD_PREFIX)).isFalse();
}
@Test
public void testIsTestOnlyCertificate() throws Exception {
- assertThat(mHelper.isTestOnlyCertificate(
+ assertThat(mHelper.isTestOnlyCertificateAlias(
TrustedRootCertificates.GOOGLE_CLOUD_KEY_VAULT_SERVICE_V1_ALIAS)).isFalse();
- assertThat(mHelper.isTestOnlyCertificate(
+ assertThat(mHelper.isTestOnlyCertificateAlias(
TrustedRootCertificates.TEST_ONLY_INSECURE_CERTIFICATE_ALIAS)).isTrue();
- assertThat(mHelper.isTestOnlyCertificate(
+ assertThat(mHelper.isTestOnlyCertificateAlias(
"UNKNOWN_ALIAS")).isFalse();
}
@@ -125,4 +121,31 @@
assertThat(filteredKeys.entrySet()).containsExactlyElementsIn(expectedResult.entrySet());
assertThat(rawKeys.entrySet()).containsAllIn(filteredKeys.entrySet());
}
+
+ @Test
+ public void testIsValidRootCertificateAlias_googleCertAlias() {
+ assertThat(mHelper.isValidRootCertificateAlias(
+ TrustedRootCertificates.GOOGLE_CLOUD_KEY_VAULT_SERVICE_V1_ALIAS)).isTrue();
+ }
+
+ @Test
+ public void testIsValidRootCertificateAlias_testOnlyCertAlias() {
+ assertThat(mHelper.isValidRootCertificateAlias(
+ TrustedRootCertificates.TEST_ONLY_INSECURE_CERTIFICATE_ALIAS)).isTrue();
+ }
+
+ @Test
+ public void testIsValidRootCertificateAlias_emptyCertAlias() {
+ assertThat(mHelper.isValidRootCertificateAlias("")).isFalse();
+ }
+
+ @Test
+ public void testIsValidRootCertificateAlias_nullCertAlias() {
+ assertThat(mHelper.isValidRootCertificateAlias(null)).isFalse();
+ }
+
+ @Test
+ public void testIsValidRootCertificateAlias_unknownCertAlias() {
+ assertThat(mHelper.isValidRootCertificateAlias("unknown-root-certifiate-alias")).isFalse();
+ }
}