Keystore.java: Make apps tolerant to keystore crashes

Since keystore accepts asynchronous requests it may be that apps wait
forever if keystore dies. This patch adds a death recipient to all
keystore promises so that Keystore.java gets notified when keystore
dies.

Test: atest android.keystore.cts
Bug: 111443219
Bug: 128991260
Change-Id: Ie6e4a4f371287f83a2cdf4069d5686c67d8aebc1

1 files changed, 115 insertions, 43 deletions

diff --git a/keystore/java/android/security/KeyStore.java b/keystore/java/android/security/KeyStore.java
index bfce17c3e6ab..f21fa5ef1486 100644
--- a/keystore/java/android/security/KeyStore.java
+++ b/keystore/java/android/security/KeyStore.java
@@ -17,7 +17,6 @@
 package android.security;
 
 import android.annotation.UnsupportedAppUsage;
-import android.app.ActivityManager;
 import android.app.ActivityThread;
 import android.app.Application;
 import android.app.KeyguardManager;
@@ -45,24 +44,24 @@ import android.security.keystore.KeyExpiredException;
 import android.security.keystore.KeyNotYetValidException;
 import android.security.keystore.KeyPermanentlyInvalidatedException;
 import android.security.keystore.KeyProperties;
-import android.security.keystore.KeyProtection;
 import android.security.keystore.KeystoreResponse;
-import android.security.keystore.StrongBoxUnavailableException;
 import android.security.keystore.UserNotAuthenticatedException;
 import android.util.Log;
+
 import com.android.org.bouncycastle.asn1.ASN1InputStream;
 import com.android.org.bouncycastle.asn1.pkcs.PrivateKeyInfo;
-import java.math.BigInteger;
+
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
+import java.math.BigInteger;
 import java.security.InvalidKeyException;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Date;
 import java.util.List;
 import java.util.Locale;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ExecutionException;
+
 import sun.security.util.ObjectIdentifier;
 import sun.security.x509.AlgorithmId;
 
@@ -492,8 +491,9 @@ public class KeyStore {
     }
 
     public boolean addRngEntropy(byte[] data, int flags) {
+        KeystoreResultPromise promise = new KeystoreResultPromise();
         try {
-            KeystoreResultPromise promise = new KeystoreResultPromise();
+            mBinder.asBinder().linkToDeath(promise, 0);
             int errorCode = mBinder.addRngEntropy(promise, data, flags);
             if (errorCode == NO_ERROR) {
                 return promise.getFuture().get().getErrorCode() == NO_ERROR;
@@ -506,6 +506,8 @@ public class KeyStore {
         } catch (ExecutionException | InterruptedException e) {
             Log.e(TAG, "AddRngEntropy completed with exception", e);
             return false;
+        } finally {
+            mBinder.asBinder().unlinkToDeath(promise, 0);
         }
     }
 
@@ -537,7 +539,8 @@ public class KeyStore {
     }
 
     private class KeyCharacteristicsPromise
-    extends android.security.keystore.IKeystoreKeyCharacteristicsCallback.Stub {
+            extends android.security.keystore.IKeystoreKeyCharacteristicsCallback.Stub
+            implements IBinder.DeathRecipient {
         final private CompletableFuture<KeyCharacteristicsCallbackResult> future =
                 new CompletableFuture<KeyCharacteristicsCallbackResult>();
         @Override
@@ -550,19 +553,30 @@ public class KeyStore {
         public final CompletableFuture<KeyCharacteristicsCallbackResult> getFuture() {
             return future;
         }
+        @Override
+        public void binderDied() {
+            future.completeExceptionally(new RemoteException("Keystore died"));
+        }
     };
 
     private int generateKeyInternal(String alias, KeymasterArguments args, byte[] entropy, int uid,
             int flags, KeyCharacteristics outCharacteristics)
                     throws RemoteException, ExecutionException, InterruptedException {
         KeyCharacteristicsPromise promise = new KeyCharacteristicsPromise();
-        int error = mBinder.generateKey(promise, alias, args, entropy, uid, flags);
-        if (error != NO_ERROR) {
-            Log.e(TAG, "generateKeyInternal failed on request " + error);
-            return error;
+        int error = NO_ERROR;
+        KeyCharacteristicsCallbackResult result = null;
+        try {
+            mBinder.asBinder().linkToDeath(promise, 0);
+            error = mBinder.generateKey(promise, alias, args, entropy, uid, flags);
+            if (error != NO_ERROR) {
+                Log.e(TAG, "generateKeyInternal failed on request " + error);
+                return error;
+            }
+            result = promise.getFuture().get();
+        } finally {
+            mBinder.asBinder().unlinkToDeath(promise, 0);
         }
 
-        KeyCharacteristicsCallbackResult result = promise.getFuture().get();
         error = result.getKeystoreResponse().getErrorCode();
         if (error != NO_ERROR) {
             Log.e(TAG, "generateKeyInternal failed on response " + error);
@@ -604,10 +618,12 @@ public class KeyStore {
 
     public int getKeyCharacteristics(String alias, KeymasterBlob clientId, KeymasterBlob appId,
             int uid, KeyCharacteristics outCharacteristics) {
+        KeyCharacteristicsPromise promise = new KeyCharacteristicsPromise();
         try {
+            mBinder.asBinder().linkToDeath(promise, 0);
             clientId = clientId != null ? clientId : new KeymasterBlob(new byte[0]);
             appId = appId != null ? appId : new KeymasterBlob(new byte[0]);
-            KeyCharacteristicsPromise promise = new KeyCharacteristicsPromise();
+
             int error = mBinder.getKeyCharacteristics(promise, alias, clientId, appId, uid);
             if (error != NO_ERROR) return error;
 
@@ -625,6 +641,8 @@ public class KeyStore {
         } catch (ExecutionException | InterruptedException e) {
             Log.e(TAG, "GetKeyCharacteristics completed with exception", e);
             return SYSTEM_ERROR;
+        } finally {
+            mBinder.asBinder().unlinkToDeath(promise, 0);
         }
     }
 
@@ -637,17 +655,23 @@ public class KeyStore {
             int uid, int flags, KeyCharacteristics outCharacteristics)
                     throws RemoteException, ExecutionException, InterruptedException {
         KeyCharacteristicsPromise promise = new KeyCharacteristicsPromise();
-        int error = mBinder.importKey(promise, alias, args, format, keyData, uid, flags);
-        if (error != NO_ERROR) return error;
+        mBinder.asBinder().linkToDeath(promise, 0);
+        try {
+            int error = mBinder.importKey(promise, alias, args, format, keyData, uid, flags);
+            if (error != NO_ERROR) return error;
 
-        KeyCharacteristicsCallbackResult result = promise.getFuture().get();
-        error = result.getKeystoreResponse().getErrorCode();
-        if (error != NO_ERROR) return error;
+            KeyCharacteristicsCallbackResult result = promise.getFuture().get();
 
-        KeyCharacteristics characteristics = result.getKeyCharacteristics();
-        if (characteristics == null) return SYSTEM_ERROR;
-        outCharacteristics.shallowCopyFrom(characteristics);
-        return NO_ERROR;
+            error = result.getKeystoreResponse().getErrorCode();
+            if (error != NO_ERROR) return error;
+
+            KeyCharacteristics characteristics = result.getKeyCharacteristics();
+            if (characteristics == null) return SYSTEM_ERROR;
+            outCharacteristics.shallowCopyFrom(characteristics);
+            return NO_ERROR;
+        } finally {
+            mBinder.asBinder().unlinkToDeath(promise, 0);
+        }
     }
 
     public int importKey(String alias, KeymasterArguments args, int format, byte[] keyData,
@@ -738,18 +762,25 @@ public class KeyStore {
             KeyCharacteristics outCharacteristics)
                     throws RemoteException, ExecutionException, InterruptedException {
         KeyCharacteristicsPromise promise = new KeyCharacteristicsPromise();
-        int error = mBinder.importWrappedKey(promise, wrappedKeyAlias, wrappedKey, wrappingKeyAlias,
-                maskingKey, args, rootSid, fingerprintSid);
-        if (error != NO_ERROR) return error;
+        mBinder.asBinder().linkToDeath(promise, 0);
+        try {
+            KeyCharacteristicsCallbackResult result = null;
+            int error = mBinder.importWrappedKey(promise, wrappedKeyAlias, wrappedKey,
+                    wrappingKeyAlias, maskingKey, args, rootSid, fingerprintSid);
+            if (error != NO_ERROR) return error;
 
-        KeyCharacteristicsCallbackResult result = promise.getFuture().get();
-        error = result.getKeystoreResponse().getErrorCode();
-        if (error != NO_ERROR) return error;
+            KeyCharacteristicsCallbackResult esult = promise.getFuture().get();
 
-        KeyCharacteristics characteristics = result.getKeyCharacteristics();
-        if (characteristics == null) return SYSTEM_ERROR;
-        outCharacteristics.shallowCopyFrom(characteristics);
-        return NO_ERROR;
+            error = result.getKeystoreResponse().getErrorCode();
+            if (error != NO_ERROR) return error;
+
+            KeyCharacteristics characteristics = result.getKeyCharacteristics();
+            if (characteristics == null) return SYSTEM_ERROR;
+            outCharacteristics.shallowCopyFrom(characteristics);
+            return NO_ERROR;
+        } finally {
+            mBinder.asBinder().unlinkToDeath(promise, 0);
+        }
     }
 
     public int importWrappedKey(String wrappedKeyAlias, byte[] wrappedKey,
@@ -776,7 +807,8 @@ public class KeyStore {
     }
 
     private class ExportKeyPromise
-    extends android.security.keystore.IKeystoreExportKeyCallback.Stub {
+            extends android.security.keystore.IKeystoreExportKeyCallback.Stub
+            implements IBinder.DeathRecipient {
         final private CompletableFuture<ExportResult> future = new CompletableFuture<ExportResult>();
         @Override
         public void onFinished(ExportResult exportKeyResult) throws android.os.RemoteException {
@@ -785,14 +817,19 @@ public class KeyStore {
         public final CompletableFuture<ExportResult> getFuture() {
             return future;
         }
+        @Override
+        public void binderDied() {
+            future.completeExceptionally(new RemoteException("Keystore died"));
+        }
     };
 
     public ExportResult exportKey(String alias, int format, KeymasterBlob clientId,
             KeymasterBlob appId, int uid) {
+        ExportKeyPromise promise = new ExportKeyPromise();
         try {
+            mBinder.asBinder().linkToDeath(promise, 0);
             clientId = clientId != null ? clientId : new KeymasterBlob(new byte[0]);
             appId = appId != null ? appId : new KeymasterBlob(new byte[0]);
-            ExportKeyPromise promise = new ExportKeyPromise();
             int error = mBinder.exportKey(promise, alias, format, clientId, appId, uid);
             if (error == NO_ERROR) {
                 return promise.getFuture().get();
@@ -805,6 +842,8 @@ public class KeyStore {
         } catch (ExecutionException | InterruptedException e) {
             Log.e(TAG, "ExportKey completed with exception", e);
             return null;
+        } finally {
+            mBinder.asBinder().unlinkToDeath(promise, 0);
         }
     }
     public ExportResult exportKey(String alias, int format, KeymasterBlob clientId,
@@ -813,7 +852,8 @@ public class KeyStore {
     }
 
     private class OperationPromise
-    extends android.security.keystore.IKeystoreOperationResultCallback.Stub {
+            extends android.security.keystore.IKeystoreOperationResultCallback.Stub
+            implements IBinder.DeathRecipient {
         final private CompletableFuture<OperationResult> future = new CompletableFuture<OperationResult>();
         @Override
         public void onFinished(OperationResult operationResult) throws android.os.RemoteException {
@@ -822,14 +862,19 @@ public class KeyStore {
         public final CompletableFuture<OperationResult> getFuture() {
             return future;
         }
+        @Override
+        public void binderDied() {
+            future.completeExceptionally(new RemoteException("Keystore died"));
+        }
     };
 
     public OperationResult begin(String alias, int purpose, boolean pruneable,
             KeymasterArguments args, byte[] entropy, int uid) {
+        OperationPromise promise = new OperationPromise();
         try {
+            mBinder.asBinder().linkToDeath(promise, 0);
             args = args != null ? args : new KeymasterArguments();
             entropy = entropy != null ? entropy : new byte[0];
-            OperationPromise promise = new OperationPromise();
             int errorCode =  mBinder.begin(promise, getToken(), alias, purpose, pruneable, args,
                                            entropy, uid);
             if (errorCode == NO_ERROR) {
@@ -843,6 +888,8 @@ public class KeyStore {
         } catch (ExecutionException | InterruptedException e) {
             Log.e(TAG, "Begin completed with exception", e);
             return null;
+        } finally {
+            mBinder.asBinder().unlinkToDeath(promise, 0);
         }
     }
 
@@ -854,10 +901,11 @@ public class KeyStore {
     }
 
     public OperationResult update(IBinder token, KeymasterArguments arguments, byte[] input) {
+        OperationPromise promise = new OperationPromise();
         try {
+            mBinder.asBinder().linkToDeath(promise, 0);
             arguments = arguments != null ? arguments : new KeymasterArguments();
             input = input != null ? input : new byte[0];
-            OperationPromise promise = new OperationPromise();
             int errorCode =  mBinder.update(promise, token, arguments, input);
             if (errorCode == NO_ERROR) {
                 return promise.getFuture().get();
@@ -870,16 +918,19 @@ public class KeyStore {
         } catch (ExecutionException | InterruptedException e) {
             Log.e(TAG, "Update completed with exception", e);
             return null;
+        } finally {
+            mBinder.asBinder().unlinkToDeath(promise, 0);
         }
     }
 
     public OperationResult finish(IBinder token, KeymasterArguments arguments, byte[] signature,
             byte[] entropy) {
+        OperationPromise promise = new OperationPromise();
         try {
+            mBinder.asBinder().linkToDeath(promise, 0);
             arguments = arguments != null ? arguments : new KeymasterArguments();
             entropy = entropy != null ? entropy : new byte[0];
             signature = signature != null ? signature : new byte[0];
-            OperationPromise promise = new OperationPromise();
             int errorCode = mBinder.finish(promise, token, arguments, signature, entropy);
             if (errorCode == NO_ERROR) {
                 return promise.getFuture().get();
@@ -892,6 +943,8 @@ public class KeyStore {
         } catch (ExecutionException | InterruptedException e) {
             Log.e(TAG, "Finish completed with exception", e);
             return null;
+        } finally {
+            mBinder.asBinder().unlinkToDeath(promise, 0);
         }
     }
 
@@ -900,7 +953,8 @@ public class KeyStore {
     }
 
     private class KeystoreResultPromise
-    extends android.security.keystore.IKeystoreResponseCallback.Stub {
+            extends android.security.keystore.IKeystoreResponseCallback.Stub
+            implements IBinder.DeathRecipient {
         final private CompletableFuture<KeystoreResponse> future = new CompletableFuture<KeystoreResponse>();
         @Override
         public void onFinished(KeystoreResponse keystoreResponse) throws android.os.RemoteException {
@@ -909,11 +963,16 @@ public class KeyStore {
         public final CompletableFuture<KeystoreResponse> getFuture() {
             return future;
         }
+        @Override
+        public void binderDied() {
+            future.completeExceptionally(new RemoteException("Keystore died"));
+        }
     };
 
     public int abort(IBinder token) {
+        KeystoreResultPromise promise = new KeystoreResultPromise();
         try {
-            KeystoreResultPromise promise = new KeystoreResultPromise();
+            mBinder.asBinder().linkToDeath(promise, 0);
             int errorCode = mBinder.abort(promise, token);
             if (errorCode == NO_ERROR) {
                 return promise.getFuture().get().getErrorCode();
@@ -926,6 +985,8 @@ public class KeyStore {
         } catch (ExecutionException | InterruptedException e) {
             Log.e(TAG, "Abort completed with exception", e);
             return SYSTEM_ERROR;
+        } finally {
+            mBinder.asBinder().unlinkToDeath(promise, 0);
         }
     }
 
@@ -1035,7 +1096,8 @@ public class KeyStore {
     }
 
     private class CertificateChainPromise
-    extends android.security.keystore.IKeystoreCertificateChainCallback.Stub {
+            extends android.security.keystore.IKeystoreCertificateChainCallback.Stub
+            implements IBinder.DeathRecipient {
         final private CompletableFuture<KeyAttestationCallbackResult> future = new CompletableFuture<KeyAttestationCallbackResult>();
         @Override
         public void onFinished(KeystoreResponse keystoreResponse,
@@ -1045,19 +1107,24 @@ public class KeyStore {
         public final CompletableFuture<KeyAttestationCallbackResult> getFuture() {
             return future;
         }
+        @Override
+        public void binderDied() {
+            future.completeExceptionally(new RemoteException("Keystore died"));
+        }
     };
 
 
     public int attestKey(
             String alias, KeymasterArguments params, KeymasterCertificateChain outChain) {
+        CertificateChainPromise promise = new CertificateChainPromise();
         try {
+            mBinder.asBinder().linkToDeath(promise, 0);
             if (params == null) {
                 params = new KeymasterArguments();
             }
             if (outChain == null) {
                 outChain = new KeymasterCertificateChain();
             }
-            CertificateChainPromise promise = new CertificateChainPromise();
             int error = mBinder.attestKey(promise, alias, params);
             if (error != NO_ERROR) return error;
             KeyAttestationCallbackResult result = promise.getFuture().get();
@@ -1072,18 +1139,21 @@ public class KeyStore {
         } catch (ExecutionException | InterruptedException e) {
             Log.e(TAG, "AttestKey completed with exception", e);
             return SYSTEM_ERROR;
+        } finally {
+            mBinder.asBinder().unlinkToDeath(promise, 0);
         }
     }
 
     public int attestDeviceIds(KeymasterArguments params, KeymasterCertificateChain outChain) {
+        CertificateChainPromise promise = new CertificateChainPromise();
         try {
+            mBinder.asBinder().linkToDeath(promise, 0);
             if (params == null) {
                 params = new KeymasterArguments();
             }
             if (outChain == null) {
                 outChain = new KeymasterCertificateChain();
             }
-            CertificateChainPromise promise = new CertificateChainPromise();
             int error = mBinder.attestDeviceIds(promise, params);
             if (error != NO_ERROR) return error;
             KeyAttestationCallbackResult result = promise.getFuture().get();
@@ -1098,6 +1168,8 @@ public class KeyStore {
         } catch (ExecutionException | InterruptedException e) {
             Log.e(TAG, "AttestDevicdeIds completed with exception", e);
             return SYSTEM_ERROR;
+        } finally {
+            mBinder.asBinder().unlinkToDeath(promise, 0);
         }
     }