Merge "Add NonNull annotation to more parameters in Recovery Controller." into pi-dev

8 files changed, 45 insertions, 38 deletions

diff --git a/core/java/android/security/keystore/BackwardsCompat.java b/core/java/android/security/keystore/BackwardsCompat.java
index 69558c4d8aaf..cf5fe1f07266 100644
--- a/core/java/android/security/keystore/BackwardsCompat.java
+++ b/core/java/android/security/keystore/BackwardsCompat.java
@@ -61,8 +61,8 @@ class BackwardsCompat {
     static android.security.keystore.recovery.KeyDerivationParams fromLegacyKeyDerivationParams(
             KeyDerivationParams keyDerivationParams
     ) {
-        return new android.security.keystore.recovery.KeyDerivationParams(
-                keyDerivationParams.getAlgorithm(), keyDerivationParams.getSalt());
+        return android.security.keystore.recovery.KeyDerivationParams.createSha256Params(
+                keyDerivationParams.getSalt());
     }
 
     static android.security.keystore.recovery.WrappedApplicationKey fromLegacyWrappedApplicationKey(
diff --git a/core/java/android/security/keystore/recovery/KeyChainSnapshot.java b/core/java/android/security/keystore/recovery/KeyChainSnapshot.java
index 69b9123c3c3e..ccb627e1181d 100644
--- a/core/java/android/security/keystore/recovery/KeyChainSnapshot.java
+++ b/core/java/android/security/keystore/recovery/KeyChainSnapshot.java
@@ -84,8 +84,8 @@ public final class KeyChainSnapshot implements Parcelable {
     }
 
     /**
-     * Snapshot version for given account. It is incremented when user secret or list of application
-     * keys changes.
+     * Snapshot version for given recovery agent. It is incremented when user secret or list of
+     * application keys changes.
      */
     public int getSnapshotVersion() {
         return mSnapshotVersion;
@@ -178,7 +178,7 @@ public final class KeyChainSnapshot implements Parcelable {
         private KeyChainSnapshot mInstance = new KeyChainSnapshot();
 
         /**
-         * Snapshot version for given account.
+         * Snapshot version for the recovery agent.
          *
          * @param snapshotVersion The snapshot version
          * @return This builder.
diff --git a/core/java/android/security/keystore/recovery/KeyDerivationParams.java b/core/java/android/security/keystore/recovery/KeyDerivationParams.java
index 8cb8e5162f16..225b592d7595 100644
--- a/core/java/android/security/keystore/recovery/KeyDerivationParams.java
+++ b/core/java/android/security/keystore/recovery/KeyDerivationParams.java
@@ -22,7 +22,6 @@ import android.annotation.SystemApi;
 import android.os.Parcel;
 import android.os.Parcelable;
 
-
 import com.android.internal.util.Preconditions;
 
 import java.lang.annotation.Retention;
@@ -63,7 +62,7 @@ public final class KeyDerivationParams implements Parcelable {
      * salt + key_material_len + key_material, where salt_len and key_material_len are one-byte, and
      * denote the number of bytes for salt and key_material, respectively.
      */
-    public static KeyDerivationParams createSha256Params(@NonNull byte[] salt) {
+    public static @NonNull KeyDerivationParams createSha256Params(@NonNull byte[] salt) {
         return new KeyDerivationParams(ALGORITHM_SHA256, salt);
     }
 
@@ -76,7 +75,7 @@ public final class KeyDerivationParams implements Parcelable {
      * the parallelization parameter p is 1, the block size parameter r is 8, and the hashing output
      * length is 32-byte.
      */
-    public static KeyDerivationParams createScryptParams(
+    public static @NonNull KeyDerivationParams createScryptParams(
             @NonNull byte[] salt, int memoryDifficulty) {
         return new KeyDerivationParams(ALGORITHM_SCRYPT, salt, memoryDifficulty);
     }
@@ -84,8 +83,7 @@ public final class KeyDerivationParams implements Parcelable {
     /**
      * @hide
      */
-    // TODO: Make private once legacy API is removed
-    public KeyDerivationParams(@KeyDerivationAlgorithm int algorithm, @NonNull byte[] salt) {
+    private KeyDerivationParams(@KeyDerivationAlgorithm int algorithm, @NonNull byte[] salt) {
         this(algorithm, salt, /*memoryDifficulty=*/ -1);
     }
 
diff --git a/core/java/android/security/keystore/recovery/RecoveryCertPath.java b/core/java/android/security/keystore/recovery/RecoveryCertPath.java
index 1950947d34fd..f3604febaa87 100644
--- a/core/java/android/security/keystore/recovery/RecoveryCertPath.java
+++ b/core/java/android/security/keystore/recovery/RecoveryCertPath.java
@@ -45,7 +45,7 @@ public final class RecoveryCertPath implements Parcelable {
      * @param certPath The certificate path to be wrapped.
      * @throws CertificateException if the given certificate path cannot be encoded properly.
      */
-    public static RecoveryCertPath createRecoveryCertPath(@NonNull CertPath certPath)
+    public static @NonNull RecoveryCertPath createRecoveryCertPath(@NonNull CertPath certPath)
             throws CertificateException {
         // Perform the encoding here to avoid throwing exceptions in writeToParcel
         try {
@@ -61,7 +61,7 @@ public final class RecoveryCertPath implements Parcelable {
      * @return the wrapped certificate path.
      * @throws CertificateException if the wrapped certificate path cannot be decoded properly.
      */
-    public CertPath getCertPath() throws CertificateException {
+    public @NonNull CertPath getCertPath() throws CertificateException {
         // Perform the decoding here to avoid throwing exceptions in createFromParcel
         return decodeCertPath(mEncodedCertPath);
     }
diff --git a/core/java/android/security/keystore/recovery/RecoverySession.java b/core/java/android/security/keystore/recovery/RecoverySession.java
index cf8a9ddf257e..ff49ca3f005c 100644
--- a/core/java/android/security/keystore/recovery/RecoverySession.java
+++ b/core/java/android/security/keystore/recovery/RecoverySession.java
@@ -234,7 +234,7 @@ public class RecoverySession implements AutoCloseable {
      * @throws InternalRecoveryServiceException if an error occurs internal to the recovery service.
      */
     @RequiresPermission(Manifest.permission.RECOVER_KEYSTORE)
-    public Map<String, Key> recoverKeyChainSnapshot(
+    @NonNull public Map<String, Key> recoverKeyChainSnapshot(
             @NonNull byte[] recoveryKeyBlob,
             @NonNull List<WrappedApplicationKey> applicationKeys
     ) throws SessionExpiredException, DecryptionFailedException, InternalRecoveryServiceException {
@@ -257,7 +257,7 @@ public class RecoverySession implements AutoCloseable {
     }
 
     /** Given a map from alias to grant alias, returns a map from alias to a {@link Key} handle. */
-    private Map<String, Key> getKeysFromGrants(Map<String, String> grantAliases)
+    private @NonNull Map<String, Key> getKeysFromGrants(Map<String, String> grantAliases)
             throws InternalRecoveryServiceException {
         ArrayMap<String, Key> keysByAlias = new ArrayMap<>(grantAliases.size());
         for (String alias : grantAliases.keySet()) {
diff --git a/core/java/android/security/keystore/recovery/TrustedRootCertificates.java b/core/java/android/security/keystore/recovery/TrustedRootCertificates.java
index a65b40f7b10c..383af424989c 100644
--- a/core/java/android/security/keystore/recovery/TrustedRootCertificates.java
+++ b/core/java/android/security/keystore/recovery/TrustedRootCertificates.java
@@ -18,6 +18,7 @@ package android.security.keystore.recovery;
 
 import static android.security.keystore.recovery.X509CertificateParsingUtils.decodeBase64Cert;
 
+import android.annotation.NonNull;
 import android.util.ArrayMap;
 
 import java.security.cert.CertificateException;
@@ -83,7 +84,7 @@ public final class TrustedRootCertificates {
     /**
      * Returns all available root certificates, keyed by alias.
      */
-    public static Map<String, X509Certificate> getRootCertificates() {
+    public static @NonNull Map<String, X509Certificate> getRootCertificates() {
         return new ArrayMap(ALL_ROOT_CERTIFICATES);
     }
 
@@ -93,7 +94,7 @@ public final class TrustedRootCertificates {
      * @param alias the alias of the certificate
      * @return the certificate referenced by the alias, or null if such a certificate doesn't exist.
      */
-    public static X509Certificate getRootCertificate(String alias) {
+    public static @NonNull X509Certificate getRootCertificate(String alias) {
         return ALL_ROOT_CERTIFICATES.get(alias);
     }
 
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 d9c97aef4455..86b04f61016f 100644
--- a/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java
+++ b/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java
@@ -587,7 +587,7 @@ public class RecoverableKeyStoreManager {
      *     were wrapped with the recovery key.
      * @throws RemoteException if an error occurred recovering the keys.
      */
-    public Map<String, String> recoverKeyChainSnapshot(
+    public @NonNull Map<String, String> recoverKeyChainSnapshot(
             @NonNull String sessionId,
             @NonNull byte[] encryptedRecoveryKey,
             @NonNull List<WrappedApplicationKey> applicationKeys) throws RemoteException {
@@ -623,7 +623,7 @@ public class RecoverableKeyStoreManager {
      * @param keysByAlias The key materials, keyed by alias.
      * @throws KeyStoreException if an error occurs importing the key or getting the grant.
      */
-    private Map<String, String> importKeyMaterials(
+    private @NonNull Map<String, String> importKeyMaterials(
             int userId, int uid, Map<String, byte[]> keysByAlias) throws KeyStoreException {
         ArrayMap<String, String> grantAliasesByAlias = new ArrayMap<>(keysByAlias.size());
         for (String alias : keysByAlias.keySet()) {
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 0d6d525b7fc6..fc2da39da7eb 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
@@ -713,12 +713,14 @@ public class RecoverableKeyStoreManagerTest {
     @Test
     public void recoverKeyChainSnapshot_throwsIfNoSessionIsPresent() throws Exception {
         try {
+            WrappedApplicationKey applicationKey = new WrappedApplicationKey.Builder()
+                .setAlias(TEST_ALIAS)
+                .setEncryptedKeyMaterial(randomBytes(32))
+                .build();
             mRecoverableKeyStoreManager.recoverKeyChainSnapshot(
                     TEST_SESSION_ID,
                     /*recoveryKeyBlob=*/ randomBytes(32),
-                    /*applicationKeys=*/ ImmutableList.of(
-                            new WrappedApplicationKey("alias", randomBytes(32))
-                    ));
+                    /*applicationKeys=*/ ImmutableList.of(applicationKey));
             fail("should have thrown");
         } catch (ServiceSpecificException e) {
             // expected
@@ -766,10 +768,11 @@ public class RecoverableKeyStoreManagerTest {
         SecretKey recoveryKey = randomRecoveryKey();
         byte[] encryptedClaimResponse = encryptClaimResponse(
                 keyClaimant, TEST_SECRET, TEST_VAULT_PARAMS, recoveryKey);
-        WrappedApplicationKey badApplicationKey = new WrappedApplicationKey(
-                TEST_ALIAS,
-                encryptedApplicationKey(randomRecoveryKey(), randomBytes(32)));
-
+        WrappedApplicationKey badApplicationKey = new WrappedApplicationKey.Builder()
+                .setAlias(TEST_ALIAS)
+                .setEncryptedKeyMaterial(
+                            encryptedApplicationKey(randomRecoveryKey(), randomBytes(32)))
+                .build();
         try {
             mRecoverableKeyStoreManager.recoverKeyChainSnapshot(
                     TEST_SESSION_ID,
@@ -824,9 +827,11 @@ public class RecoverableKeyStoreManagerTest {
         byte[] encryptedClaimResponse = encryptClaimResponse(
                 keyClaimant, TEST_SECRET, TEST_VAULT_PARAMS, recoveryKey);
         byte[] applicationKeyBytes = randomBytes(32);
-        WrappedApplicationKey applicationKey = new WrappedApplicationKey(
-                TEST_ALIAS,
-                encryptedApplicationKey(recoveryKey, applicationKeyBytes));
+        WrappedApplicationKey applicationKey = new WrappedApplicationKey.Builder()
+                    .setAlias(TEST_ALIAS)
+                    .setEncryptedKeyMaterial(
+                            encryptedApplicationKey(recoveryKey, applicationKeyBytes))
+                    .build();
 
         Map<String, String> recoveredKeys = mRecoverableKeyStoreManager.recoverKeyChainSnapshot(
                 TEST_SESSION_ID,
@@ -858,14 +863,17 @@ public class RecoverableKeyStoreManagerTest {
 
         byte[] applicationKeyBytes1 = randomBytes(32);
         byte[] applicationKeyBytes2 = randomBytes(32);
-
-        WrappedApplicationKey applicationKey1 = new WrappedApplicationKey(
-                TEST_ALIAS,
-                // Use a different recovery key here, so the decryption will fail
-                encryptedApplicationKey(randomRecoveryKey(), applicationKeyBytes1));
-        WrappedApplicationKey applicationKey2 = new WrappedApplicationKey(
-                TEST_ALIAS2,
-                encryptedApplicationKey(recoveryKey, applicationKeyBytes2));
+        WrappedApplicationKey applicationKey1 = new WrappedApplicationKey.Builder()
+                    .setAlias(TEST_ALIAS)
+                     // Use a different recovery key here, so the decryption will fail
+                    .setEncryptedKeyMaterial(
+                            encryptedApplicationKey(randomRecoveryKey(), applicationKeyBytes1))
+                    .build();
+        WrappedApplicationKey applicationKey2 = new WrappedApplicationKey.Builder()
+                    .setAlias(TEST_ALIAS2)
+                    .setEncryptedKeyMaterial(
+                            encryptedApplicationKey(recoveryKey, applicationKeyBytes2))
+                    .build();
 
         Map<String, String> recoveredKeys = mRecoverableKeyStoreManager.recoverKeyChainSnapshot(
                 TEST_SESSION_ID,
@@ -963,8 +971,8 @@ public class RecoverableKeyStoreManagerTest {
     private static byte[] encryptedApplicationKey(
             SecretKey recoveryKey, byte[] applicationKey) throws Exception {
         return KeySyncUtils.encryptKeysWithRecoveryKey(recoveryKey, ImmutableMap.of(
-                "alias", new SecretKeySpec(applicationKey, "AES")
-        )).get("alias");
+                TEST_ALIAS, new SecretKeySpec(applicationKey, "AES")
+        )).get(TEST_ALIAS);
     }
 
     private static byte[] encryptClaimResponse(