Consolidate Keystore alias prefixes.

Currently, the keystore SPI assigns different prefixes to user key
entries depending on the algorithm. Symmetric keys (secret keys) get
the prefix USERSKEY_ and asymmetric keys (private keys) get the
prefix USERPKEY_. This distinction is superfluous, as the information
can always be retrieved from the key characteristics. Also moving
forward it is desirable to be able to import keys the nature
of which is not known a priori. In these cases the prefix cannot be
chosen meaningfully.

This patch deprecates one of the prefixes (i.e. USERSKEY_) and uses
the other for both types of keys. Legacy keys with the old prefix
can still be used, but all new keys will have the prefix USERPKEY_.

Bug: 63931634
Test: CTS test and Manual upgrade test with KeyStoreTool app
      Also performed upgrade test with device PIN set
Change-Id: I5b4bb0b0d2b82c276659d55b862150326bb68d5d

5 files changed, 110 insertions, 76 deletions

diff --git a/keystore/java/android/security/Credentials.java b/keystore/java/android/security/Credentials.java
index 6830a7487dbc..57db20be1145 100644
--- a/keystore/java/android/security/Credentials.java
+++ b/keystore/java/android/security/Credentials.java
@@ -60,10 +60,12 @@ public class Credentials {
     /** Key prefix for user certificates. */
     public static final String USER_CERTIFICATE = "USRCERT_";
 
-    /** Key prefix for user private keys. */
+    /** Key prefix for user private and secret keys. */
     public static final String USER_PRIVATE_KEY = "USRPKEY_";
 
-    /** Key prefix for user secret keys. */
+    /** Key prefix for user secret keys.
+     *  @deprecated use {@code USER_PRIVATE_KEY} for this category instead.
+     */
     public static final String USER_SECRET_KEY = "USRSKEY_";
 
     /** Key prefix for VPN. */
@@ -235,8 +237,7 @@ public class Credentials {
          * Make sure every type is deleted. There can be all three types, so
          * don't use a conditional here.
          */
-        return deletePrivateKeyTypeForAlias(keystore, alias, uid)
-                & deleteSecretKeyTypeForAlias(keystore, alias, uid)
+        return deleteUserKeyTypeForAlias(keystore, alias, uid)
                 & deleteCertificateTypesForAlias(keystore, alias, uid);
     }
 
@@ -264,34 +265,27 @@ public class Credentials {
     }
 
     /**
-     * Delete private key for a particular {@code alias}.
-     * Returns {@code true} if the entry no longer exists.
-     */
-    static boolean deletePrivateKeyTypeForAlias(KeyStore keystore, String alias) {
-        return deletePrivateKeyTypeForAlias(keystore, alias, KeyStore.UID_SELF);
-    }
-
-    /**
-     * Delete private key for a particular {@code alias}.
+     * Delete user key for a particular {@code alias}.
      * Returns {@code true} if the entry no longer exists.
      */
-    static boolean deletePrivateKeyTypeForAlias(KeyStore keystore, String alias, int uid) {
-        return keystore.delete(Credentials.USER_PRIVATE_KEY + alias, uid);
+    public static boolean deleteUserKeyTypeForAlias(KeyStore keystore, String alias) {
+        return deleteUserKeyTypeForAlias(keystore, alias, KeyStore.UID_SELF);
     }
 
     /**
-     * Delete secret key for a particular {@code alias}.
+     * Delete user key for a particular {@code alias}.
      * Returns {@code true} if the entry no longer exists.
      */
-    public static boolean deleteSecretKeyTypeForAlias(KeyStore keystore, String alias) {
-        return deleteSecretKeyTypeForAlias(keystore, alias, KeyStore.UID_SELF);
+    public static boolean deleteUserKeyTypeForAlias(KeyStore keystore, String alias, int uid) {
+        return keystore.delete(Credentials.USER_PRIVATE_KEY + alias, uid) ||
+                keystore.delete(Credentials.USER_SECRET_KEY + alias, uid);
     }
 
     /**
-     * Delete secret key for a particular {@code alias}.
+     * Delete legacy prefixed entry for a particular {@code alias}
      * Returns {@code true} if the entry no longer exists.
      */
-    public static boolean deleteSecretKeyTypeForAlias(KeyStore keystore, String alias, int uid) {
+    public static boolean deleteLegacyKeyForAlias(KeyStore keystore, String alias, int uid) {
         return keystore.delete(Credentials.USER_SECRET_KEY + alias, uid);
     }
 }
diff --git a/keystore/java/android/security/keystore/AndroidKeyStoreKeyGeneratorSpi.java b/keystore/java/android/security/keystore/AndroidKeyStoreKeyGeneratorSpi.java
index 988e32cff069..f1d1e1665387 100644
--- a/keystore/java/android/security/keystore/AndroidKeyStoreKeyGeneratorSpi.java
+++ b/keystore/java/android/security/keystore/AndroidKeyStoreKeyGeneratorSpi.java
@@ -305,7 +305,7 @@ public abstract class AndroidKeyStoreKeyGeneratorSpi extends KeyGeneratorSpi {
                 KeyStoreCryptoOperationUtils.getRandomBytesToMixIntoKeystoreRng(
                         mRng, (mKeySizeBits + 7) / 8);
         int flags = 0;
-        String keyAliasInKeystore = Credentials.USER_SECRET_KEY + spec.getKeystoreAlias();
+        String keyAliasInKeystore = Credentials.USER_PRIVATE_KEY + spec.getKeystoreAlias();
         KeyCharacteristics resultingKeyCharacteristics = new KeyCharacteristics();
         boolean success = false;
         try {
diff --git a/keystore/java/android/security/keystore/AndroidKeyStoreProvider.java b/keystore/java/android/security/keystore/AndroidKeyStoreProvider.java
index f36c00ce8b86..55e6519d805d 100644
--- a/keystore/java/android/security/keystore/AndroidKeyStoreProvider.java
+++ b/keystore/java/android/security/keystore/AndroidKeyStoreProvider.java
@@ -196,7 +196,7 @@ public class AndroidKeyStoreProvider extends Provider {
     }
 
     @NonNull
-    public static AndroidKeyStorePrivateKey getAndroidKeyStorePrivateKey(
+    private static AndroidKeyStorePrivateKey getAndroidKeyStorePrivateKey(
             @NonNull AndroidKeyStorePublicKey publicKey) {
         String keyAlgorithm = publicKey.getAlgorithm();
         if (KeyProperties.KEY_ALGORITHM_EC.equalsIgnoreCase(keyAlgorithm)) {
@@ -212,17 +212,25 @@ public class AndroidKeyStoreProvider extends Provider {
     }
 
     @NonNull
-    public static AndroidKeyStorePublicKey loadAndroidKeyStorePublicKeyFromKeystore(
-            @NonNull KeyStore keyStore, @NonNull String privateKeyAlias, int uid)
+    private static KeyCharacteristics getKeyCharacteristics(@NonNull KeyStore keyStore,
+            @NonNull String alias, int uid)
             throws UnrecoverableKeyException {
         KeyCharacteristics keyCharacteristics = new KeyCharacteristics();
         int errorCode = keyStore.getKeyCharacteristics(
-                privateKeyAlias, null, null, uid, keyCharacteristics);
+                alias, null, null, uid, keyCharacteristics);
         if (errorCode != KeyStore.NO_ERROR) {
             throw (UnrecoverableKeyException)
-                    new UnrecoverableKeyException("Failed to obtain information about private key")
-                    .initCause(KeyStore.getKeyStoreException(errorCode));
+                    new UnrecoverableKeyException("Failed to obtain information about key")
+                            .initCause(KeyStore.getKeyStoreException(errorCode));
         }
+        return keyCharacteristics;
+    }
+
+    @NonNull
+    private static AndroidKeyStorePublicKey loadAndroidKeyStorePublicKeyFromKeystore(
+            @NonNull KeyStore keyStore, @NonNull String privateKeyAlias, int uid,
+            KeyCharacteristics keyCharacteristics)
+            throws UnrecoverableKeyException {
         ExportResult exportResult = keyStore.exportKey(
                 privateKeyAlias, KeymasterDefs.KM_KEY_FORMAT_X509, null, null, uid);
         if (exportResult.resultCode != KeyStore.NO_ERROR) {
@@ -252,37 +260,56 @@ public class AndroidKeyStoreProvider extends Provider {
     }
 
     @NonNull
-    public static KeyPair loadAndroidKeyStoreKeyPairFromKeystore(
+    public static AndroidKeyStorePublicKey loadAndroidKeyStorePublicKeyFromKeystore(
             @NonNull KeyStore keyStore, @NonNull String privateKeyAlias, int uid)
             throws UnrecoverableKeyException {
+        return loadAndroidKeyStorePublicKeyFromKeystore(keyStore, privateKeyAlias, uid,
+                getKeyCharacteristics(keyStore, privateKeyAlias, uid));
+    }
+
+    @NonNull
+    private static KeyPair loadAndroidKeyStoreKeyPairFromKeystore(
+            @NonNull KeyStore keyStore, @NonNull String privateKeyAlias, int uid,
+            @NonNull KeyCharacteristics keyCharacteristics)
+            throws UnrecoverableKeyException {
         AndroidKeyStorePublicKey publicKey =
-                loadAndroidKeyStorePublicKeyFromKeystore(keyStore, privateKeyAlias, uid);
+                loadAndroidKeyStorePublicKeyFromKeystore(keyStore, privateKeyAlias, uid,
+                        keyCharacteristics);
         AndroidKeyStorePrivateKey privateKey =
                 AndroidKeyStoreProvider.getAndroidKeyStorePrivateKey(publicKey);
         return new KeyPair(publicKey, privateKey);
     }
 
     @NonNull
-    public static AndroidKeyStorePrivateKey loadAndroidKeyStorePrivateKeyFromKeystore(
+    public static KeyPair loadAndroidKeyStoreKeyPairFromKeystore(
             @NonNull KeyStore keyStore, @NonNull String privateKeyAlias, int uid)
             throws UnrecoverableKeyException {
-        KeyPair keyPair = loadAndroidKeyStoreKeyPairFromKeystore(keyStore, privateKeyAlias, uid);
+        return loadAndroidKeyStoreKeyPairFromKeystore(keyStore, privateKeyAlias, uid,
+                getKeyCharacteristics(keyStore, privateKeyAlias, uid));
+    }
+
+    @NonNull
+    private static AndroidKeyStorePrivateKey loadAndroidKeyStorePrivateKeyFromKeystore(
+            @NonNull KeyStore keyStore, @NonNull String privateKeyAlias, int uid,
+            @NonNull KeyCharacteristics keyCharacteristics)
+            throws UnrecoverableKeyException {
+        KeyPair keyPair = loadAndroidKeyStoreKeyPairFromKeystore(keyStore, privateKeyAlias, uid,
+                keyCharacteristics);
         return (AndroidKeyStorePrivateKey) keyPair.getPrivate();
     }
 
     @NonNull
-    public static AndroidKeyStoreSecretKey loadAndroidKeyStoreSecretKeyFromKeystore(
-            @NonNull KeyStore keyStore, @NonNull String secretKeyAlias, int uid)
+    public static AndroidKeyStorePrivateKey loadAndroidKeyStorePrivateKeyFromKeystore(
+            @NonNull KeyStore keyStore, @NonNull String privateKeyAlias, int uid)
             throws UnrecoverableKeyException {
-        KeyCharacteristics keyCharacteristics = new KeyCharacteristics();
-        int errorCode = keyStore.getKeyCharacteristics(
-                secretKeyAlias, null, null, uid, keyCharacteristics);
-        if (errorCode != KeyStore.NO_ERROR) {
-            throw (UnrecoverableKeyException)
-                    new UnrecoverableKeyException("Failed to obtain information about key")
-                            .initCause(KeyStore.getKeyStoreException(errorCode));
-        }
+        return loadAndroidKeyStorePrivateKeyFromKeystore(keyStore, privateKeyAlias, uid,
+                getKeyCharacteristics(keyStore, privateKeyAlias, uid));
+    }
 
+    @NonNull
+    private static AndroidKeyStoreSecretKey loadAndroidKeyStoreSecretKeyFromKeystore(
+            @NonNull String secretKeyAlias, int uid, @NonNull KeyCharacteristics keyCharacteristics)
+            throws UnrecoverableKeyException {
         Integer keymasterAlgorithm = keyCharacteristics.getEnum(KeymasterDefs.KM_TAG_ALGORITHM);
         if (keymasterAlgorithm == null) {
             throw new UnrecoverableKeyException("Key algorithm unknown");
@@ -310,6 +337,29 @@ public class AndroidKeyStoreProvider extends Provider {
         return new AndroidKeyStoreSecretKey(secretKeyAlias, uid, keyAlgorithmString);
     }
 
+    public static AndroidKeyStoreKey loadAndroidKeyStoreKeyFromKeystore(
+            @NonNull KeyStore keyStore, @NonNull String userKeyAlias, int uid)
+            throws UnrecoverableKeyException  {
+        KeyCharacteristics keyCharacteristics = getKeyCharacteristics(keyStore, userKeyAlias, uid);
+
+        Integer keymasterAlgorithm = keyCharacteristics.getEnum(KeymasterDefs.KM_TAG_ALGORITHM);
+        if (keymasterAlgorithm == null) {
+            throw new UnrecoverableKeyException("Key algorithm unknown");
+        }
+
+        if (keymasterAlgorithm == KeymasterDefs.KM_ALGORITHM_HMAC ||
+                keymasterAlgorithm == KeymasterDefs.KM_ALGORITHM_AES) {
+            return loadAndroidKeyStoreSecretKeyFromKeystore(userKeyAlias, uid,
+                    keyCharacteristics);
+        } else if (keymasterAlgorithm == KeymasterDefs.KM_ALGORITHM_RSA ||
+                keymasterAlgorithm == KeymasterDefs.KM_ALGORITHM_EC) {
+            return loadAndroidKeyStorePrivateKeyFromKeystore(keyStore, userKeyAlias, uid,
+                    keyCharacteristics);
+        } else {
+            throw new UnrecoverableKeyException("Key algorithm unknown");
+        }
+    }
+
     /**
      * Returns an {@code AndroidKeyStore} {@link java.security.KeyStore}} of the specified UID.
      * The {@code KeyStore} contains keys and certificates owned by that UID. Such cross-UID
diff --git a/keystore/java/android/security/keystore/AndroidKeyStoreSecretKeyFactorySpi.java b/keystore/java/android/security/keystore/AndroidKeyStoreSecretKeyFactorySpi.java
index 0379863e8ca1..fdb885db6a18 100644
--- a/keystore/java/android/security/keystore/AndroidKeyStoreSecretKeyFactorySpi.java
+++ b/keystore/java/android/security/keystore/AndroidKeyStoreSecretKeyFactorySpi.java
@@ -64,7 +64,10 @@ public class AndroidKeyStoreSecretKeyFactorySpi extends SecretKeyFactorySpi {
         AndroidKeyStoreKey keystoreKey = (AndroidKeyStoreKey) key;
         String keyAliasInKeystore = keystoreKey.getAlias();
         String entryAlias;
-        if (keyAliasInKeystore.startsWith(Credentials.USER_SECRET_KEY)) {
+        if (keyAliasInKeystore.startsWith(Credentials.USER_PRIVATE_KEY)) {
+            entryAlias = keyAliasInKeystore.substring(Credentials.USER_PRIVATE_KEY.length());
+        } else if (keyAliasInKeystore.startsWith(Credentials.USER_SECRET_KEY)){
+            // key has legacy prefix
             entryAlias = keyAliasInKeystore.substring(Credentials.USER_SECRET_KEY.length());
         } else {
             throw new InvalidKeySpecException("Invalid key alias: " + keyAliasInKeystore);
diff --git a/keystore/java/android/security/keystore/AndroidKeyStoreSpi.java b/keystore/java/android/security/keystore/AndroidKeyStoreSpi.java
index bab4010b90e8..d73a9e29bb1b 100644
--- a/keystore/java/android/security/keystore/AndroidKeyStoreSpi.java
+++ b/keystore/java/android/security/keystore/AndroidKeyStoreSpi.java
@@ -89,18 +89,14 @@ public class AndroidKeyStoreSpi extends KeyStoreSpi {
     @Override
     public Key engineGetKey(String alias, char[] password) throws NoSuchAlgorithmException,
             UnrecoverableKeyException {
-        if (isPrivateKeyEntry(alias)) {
-            String privateKeyAlias = Credentials.USER_PRIVATE_KEY + alias;
-            return AndroidKeyStoreProvider.loadAndroidKeyStorePrivateKeyFromKeystore(
-                    mKeyStore, privateKeyAlias, mUid);
-        } else if (isSecretKeyEntry(alias)) {
-            String secretKeyAlias = Credentials.USER_SECRET_KEY + alias;
-            return AndroidKeyStoreProvider.loadAndroidKeyStoreSecretKeyFromKeystore(
-                    mKeyStore, secretKeyAlias, mUid);
-        } else {
-            // Key not found
-            return null;
+        String userKeyAlias = Credentials.USER_PRIVATE_KEY + alias;
+        if (!mKeyStore.contains(userKeyAlias, mUid)) {
+            // try legacy prefix for backward compatibility
+            userKeyAlias = Credentials.USER_SECRET_KEY + alias;
+            if (!mKeyStore.contains(userKeyAlias, mUid)) return null;
         }
+        return AndroidKeyStoreProvider.loadAndroidKeyStoreKeyFromKeystore(mKeyStore, userKeyAlias,
+                mUid);
     }
 
     @Override
@@ -540,7 +536,7 @@ public class AndroidKeyStoreSpi extends KeyStoreSpi {
             } else {
                 // Keep the stored private key around -- delete all other entry types
                 Credentials.deleteCertificateTypesForAlias(mKeyStore, alias, mUid);
-                Credentials.deleteSecretKeyTypeForAlias(mKeyStore, alias, mUid);
+                Credentials.deleteLegacyKeyForAlias(mKeyStore, alias, mUid);
             }
 
             // Store the leaf certificate
@@ -565,7 +561,7 @@ public class AndroidKeyStoreSpi extends KeyStoreSpi {
                     Credentials.deleteAllTypesForAlias(mKeyStore, alias, mUid);
                 } else {
                     Credentials.deleteCertificateTypesForAlias(mKeyStore, alias, mUid);
-                    Credentials.deleteSecretKeyTypeForAlias(mKeyStore, alias, mUid);
+                    Credentials.deleteLegacyKeyForAlias(mKeyStore, alias, mUid);
                 }
             }
         }
@@ -588,12 +584,17 @@ public class AndroidKeyStoreSpi extends KeyStoreSpi {
             if (keyAliasInKeystore == null) {
                 throw new KeyStoreException("KeyStore-backed secret key does not have an alias");
             }
-            if (!keyAliasInKeystore.startsWith(Credentials.USER_SECRET_KEY)) {
-                throw new KeyStoreException("KeyStore-backed secret key has invalid alias: "
-                        + keyAliasInKeystore);
+            String keyAliasPrefix = Credentials.USER_PRIVATE_KEY;
+            if (!keyAliasInKeystore.startsWith(keyAliasPrefix)) {
+                // try legacy prefix
+                keyAliasPrefix = Credentials.USER_SECRET_KEY;
+                if (!keyAliasInKeystore.startsWith(keyAliasPrefix)) {
+                    throw new KeyStoreException("KeyStore-backed secret key has invalid alias: "
+                            + keyAliasInKeystore);
+                }
             }
             String keyEntryAlias =
-                    keyAliasInKeystore.substring(Credentials.USER_SECRET_KEY.length());
+                    keyAliasInKeystore.substring(keyAliasPrefix.length());
             if (!entryAlias.equals(keyEntryAlias)) {
                 throw new KeyStoreException("Can only replace KeyStore-backed keys with same"
                         + " alias: " + entryAlias + " != " + keyEntryAlias);
@@ -728,7 +729,7 @@ public class AndroidKeyStoreSpi extends KeyStoreSpi {
         }
 
         Credentials.deleteAllTypesForAlias(mKeyStore, entryAlias, mUid);
-        String keyAliasInKeystore = Credentials.USER_SECRET_KEY + entryAlias;
+        String keyAliasInKeystore = Credentials.USER_PRIVATE_KEY + entryAlias;
         int errorCode = mKeyStore.importKey(
                 keyAliasInKeystore,
                 args,
@@ -827,24 +828,10 @@ public class AndroidKeyStoreSpi extends KeyStoreSpi {
     }
 
     private boolean isKeyEntry(String alias) {
-        return isPrivateKeyEntry(alias) || isSecretKeyEntry(alias);
-    }
-
-    private boolean isPrivateKeyEntry(String alias) {
-        if (alias == null) {
-            throw new NullPointerException("alias == null");
-        }
-
-        return mKeyStore.contains(Credentials.USER_PRIVATE_KEY + alias, mUid);
+        return mKeyStore.contains(Credentials.USER_PRIVATE_KEY + alias, mUid) ||
+                mKeyStore.contains(Credentials.USER_SECRET_KEY + alias, mUid);
     }
 
-    private boolean isSecretKeyEntry(String alias) {
-        if (alias == null) {
-            throw new NullPointerException("alias == null");
-        }
-
-        return mKeyStore.contains(Credentials.USER_SECRET_KEY + alias, mUid);
-    }
 
     private boolean isCertificateEntry(String alias) {
         if (alias == null) {