From b2a11d35e5f849db53b71172901e11b60560c996 Mon Sep 17 00:00:00 2001
From: Max Bires <jbires@google.com>
Date: Wed, 9 Jun 2021 19:54:36 +0000
Subject: [PATCH] Revert "Fixing the race condition in GenerateRkpKey"

This reverts commit 0f62195535139026d35e831ccf329f8204781430.

Reason for revert: Breaks non-RKP systems

Bug: 190222116
Change-Id: I7e31a3045caa7b8bb0e34c8d1e266c104a627324
Merged-In: I7e31a3045caa7b8bb0e34c8d1e266c104a627324
---
 .../java/android/security/GenerateRkpKey.java | 72 ++++++-------------
 .../AndroidKeyStoreKeyPairGeneratorSpi.java   |  2 +-
 2 files changed, 22 insertions(+), 52 deletions(-)

diff --git a/keystore/java/android/security/GenerateRkpKey.java b/keystore/java/android/security/GenerateRkpKey.java
index cc1ec1bada50..a1a7aa85519f 100644
--- a/keystore/java/android/security/GenerateRkpKey.java
+++ b/keystore/java/android/security/GenerateRkpKey.java
@@ -22,10 +22,6 @@ import android.content.Intent;
 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
@@ -45,25 +41,14 @@ import java.util.concurrent.TimeUnit;
  * @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
@@ -79,51 +64,36 @@ public class GenerateRkpKey {
         mContext = context;
     }
 
-    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);
-        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) {
-            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 (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);
+        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.generateKey(securityLevel);
+        }
+        mContext.unbindService(mConnection);
     }
 
     /**
-     * Fulfills the use case of (1) described in the class documentation. Non blocking call.
+     * FUlfills the use case of (1) described in the class documentation. Non blocking call.
      */
     public void notifyKeyGenerated(int securityLevel) throws RemoteException {
-        bindAndSendCommand(NOTIFY_KEY_GENERATED, securityLevel);
+        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);
     }
 }
diff --git a/keystore/java/android/security/keystore2/AndroidKeyStoreKeyPairGeneratorSpi.java b/keystore/java/android/security/keystore2/AndroidKeyStoreKeyPairGeneratorSpi.java
index c048f3bffc75..dc7f3dda35c0 100644
--- a/keystore/java/android/security/keystore2/AndroidKeyStoreKeyPairGeneratorSpi.java
+++ b/keystore/java/android/security/keystore2/AndroidKeyStoreKeyPairGeneratorSpi.java
@@ -580,7 +580,7 @@ public abstract class AndroidKeyStoreKeyPairGeneratorSpi extends KeyPairGenerato
             } 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.", e);
+                Log.d(TAG, "Couldn't connect to the RemoteProvisioner backend.");
             }
             success = true;
             return new KeyPair(publicKey, publicKey.getPrivateKey());