Fixing the race condition in GenerateRkpKey
This file was written on the assumption that bindService was
synchronous, which it isn't. This change adds a CountDownLatch to force
the class to wait for the binding to finish.
Bug: 190222116
Test: atest RemoteProvisionerUnitTests
Change-Id: I917a61da612f21f9a0f783bea5d24270d4e1db42
diff --git a/keystore/java/android/security/GenerateRkpKey.java b/keystore/java/android/security/GenerateRkpKey.java
index a1a7aa8..cc1ec1b 100644
--- a/keystore/java/android/security/GenerateRkpKey.java
+++ b/keystore/java/android/security/GenerateRkpKey.java
@@ -22,6 +22,10 @@
import android.content.ServiceConnection;
import android.os.IBinder;
import android.os.RemoteException;
+import android.util.Log;
+
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
/**
* GenerateKey is a helper class to handle interactions between Keystore and the RemoteProvisioner
@@ -41,14 +45,25 @@
* @hide
*/
public class GenerateRkpKey {
+ private static final String TAG = "GenerateRkpKey";
+
+ private static final int NOTIFY_EMPTY = 0;
+ private static final int NOTIFY_KEY_GENERATED = 1;
+ private static final int TIMEOUT_MS = 1000;
private IGenerateRkpKeyService mBinder;
private Context mContext;
+ private CountDownLatch mCountDownLatch;
private ServiceConnection mConnection = new ServiceConnection() {
@Override
public void onServiceConnected(ComponentName className, IBinder service) {
mBinder = IGenerateRkpKeyService.Stub.asInterface(service);
+ mCountDownLatch.countDown();
+ }
+
+ @Override public void onBindingDied(ComponentName className) {
+ mCountDownLatch.countDown();
}
@Override
@@ -64,36 +79,51 @@
mContext = context;
}
- /**
- * Fulfills the use case of (2) described in the class documentation. Blocks until the
- * RemoteProvisioner application can get new attestation keys signed by the server.
- */
- public void notifyEmpty(int securityLevel) throws RemoteException {
+ private void bindAndSendCommand(int command, int securityLevel) throws RemoteException {
Intent intent = new Intent(IGenerateRkpKeyService.class.getName());
ComponentName comp = intent.resolveSystemService(mContext.getPackageManager(), 0);
+ if (comp == null) {
+ throw new RemoteException("Could not resolve GenerateRkpKeyService.");
+ }
intent.setComponent(comp);
- if (comp == null || !mContext.bindService(intent, mConnection, Context.BIND_AUTO_CREATE)) {
- throw new RemoteException("Failed to bind to GenerateKeyService");
+ mCountDownLatch = new CountDownLatch(1);
+ if (!mContext.bindService(intent, mConnection, Context.BIND_AUTO_CREATE)) {
+ throw new RemoteException("Failed to bind to GenerateRkpKeyService");
+ }
+ try {
+ mCountDownLatch.await(TIMEOUT_MS, TimeUnit.MILLISECONDS);
+ } catch (InterruptedException e) {
+ Log.e(TAG, "Interrupted: ", e);
}
if (mBinder != null) {
- mBinder.generateKey(securityLevel);
+ switch (command) {
+ case NOTIFY_EMPTY:
+ mBinder.generateKey(securityLevel);
+ break;
+ case NOTIFY_KEY_GENERATED:
+ mBinder.notifyKeyGenerated(securityLevel);
+ break;
+ default:
+ Log.e(TAG, "Invalid case for command");
+ }
+ } else {
+ Log.e(TAG, "Binder object is null; failed to bind to GenerateRkpKeyService.");
}
mContext.unbindService(mConnection);
}
/**
- * FUlfills the use case of (1) described in the class documentation. Non blocking call.
+ * Fulfills the use case of (2) described in the class documentation. Blocks until the
+ * RemoteProvisioner application can get new attestation keys signed by the server.
+ */
+ public void notifyEmpty(int securityLevel) throws RemoteException {
+ bindAndSendCommand(NOTIFY_EMPTY, securityLevel);
+ }
+
+ /**
+ * Fulfills the use case of (1) described in the class documentation. Non blocking call.
*/
public void notifyKeyGenerated(int securityLevel) throws RemoteException {
- Intent intent = new Intent(IGenerateRkpKeyService.class.getName());
- ComponentName comp = intent.resolveSystemService(mContext.getPackageManager(), 0);
- intent.setComponent(comp);
- if (comp == null || !mContext.bindService(intent, mConnection, Context.BIND_AUTO_CREATE)) {
- throw new RemoteException("Failed to bind to GenerateKeyService");
- }
- if (mBinder != null) {
- mBinder.notifyKeyGenerated(securityLevel);
- }
- mContext.unbindService(mConnection);
+ bindAndSendCommand(NOTIFY_KEY_GENERATED, securityLevel);
}
}
diff --git a/keystore/java/android/security/keystore2/AndroidKeyStoreKeyPairGeneratorSpi.java b/keystore/java/android/security/keystore2/AndroidKeyStoreKeyPairGeneratorSpi.java
index dc7f3dd..c048f3b 100644
--- a/keystore/java/android/security/keystore2/AndroidKeyStoreKeyPairGeneratorSpi.java
+++ b/keystore/java/android/security/keystore2/AndroidKeyStoreKeyPairGeneratorSpi.java
@@ -580,7 +580,7 @@
} catch (RemoteException e) {
// This is not really an error state, and necessarily does not apply to non RKP
// systems or hybrid systems where RKP is not currently turned on.
- Log.d(TAG, "Couldn't connect to the RemoteProvisioner backend.");
+ Log.d(TAG, "Couldn't connect to the RemoteProvisioner backend.", e);
}
success = true;
return new KeyPair(publicKey, publicKey.getPrivateKey());