Remove Android support for out-of-process KeyStores

chrome/android/java/src/org/chromium/chrome/browser/SSLClientCertificateRequest.java

Index: chrome/android/java/src/org/chromium/chrome/browser/SSLClientCertificateRequest.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/SSLClientCertificateRequest.java b/chrome/android/java/src/org/chromium/chrome/browser/SSLClientCertificateRequest.java
index 6d9fcf3fdd05b7a5f97c76d6d9b90ca950ab4214..e3d68901bf0a05f65459132f96af81fc9ba76511 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/SSLClientCertificateRequest.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/SSLClientCertificateRequest.java
@@ -21,7 +21,6 @@ import org.chromium.base.VisibleForTesting;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
 import org.chromium.chrome.R;
-import org.chromium.chrome.browser.smartcard.PKCS11AuthenticationManager;
 import org.chromium.net.AndroidPrivateKey;
 import org.chromium.net.DefaultAndroidKeyStore;
 import org.chromium.ui.base.WindowAndroid;
@@ -49,12 +48,12 @@ public class SSLClientCertificateRequest {
             new DefaultAndroidKeyStore();

[Yaron, 2015/11/25 16:24:31]

Any reason not to remove the AndroidKeyStore interface? Nothing takes advantage
of it (e.g. for testing)

[davidben, 2015/11/25 17:48:14]

+1

I believe this is the CL that introduced it:
https://codereview.chromium.org/166143002 The code's changed in the meantime, so
there'll probably be some unraveling to do, but one less layer of indirection.

[Changwan Ryu, 2015/11/26 02:52:11]

No need to keep this. Thanks for the pointer. Done as suggested.

 
     /**
-     * Common implementation for anynchronous task of handling the certificate request. This
-     * AsyncTask uses the abstract methods to retrieve the authentication material from a
-     * generalized key store. The key store is accessed in background, as the APIs being exercised
+     * Implementation for anynchronous task of handling the certificate request. This
+     * AsyncTask retrieve the authentication material from the system key store.
+     * The key store is accessed in background, as the APIs being exercised
      * may be blocking. The results are posted back to native on the UI thread.
      */
-    abstract static class CertAsyncTask extends AsyncTask<Void, Void, Void> {
+    private static class CertAsyncTaskKeyChain extends AsyncTask<Void, Void, Void> {
         // These fields will store the results computed in doInBackground so that they can be posted
         // back in onPostExecute.
         private byte[][] mEncodedChain;
@@ -63,15 +62,16 @@ public class SSLClientCertificateRequest {
         // Pointer to the native certificate request needed to return the results.
         private final long mNativePtr;
 
-        CertAsyncTask(long nativePtr) {
+        final Context mContext;
+        final String mAlias;
+
+        CertAsyncTaskKeyChain(Context context, long nativePtr, String alias) {
             mNativePtr = nativePtr;
+            mContext = context;
+            assert alias != null;
+            mAlias = alias;
         }
 
-        // These overriden methods will be used to access the key store.
-        abstract String getAlias();
-        abstract AndroidPrivateKey getPrivateKey(String alias);
-        abstract X509Certificate[] getCertificateChain(String alias);
-
         @Override
         protected Void doInBackground(Void... params) {
             String alias = getAlias();
@@ -106,27 +106,12 @@ public class SSLClientCertificateRequest {
             ThreadUtils.assertOnUiThread();
             nativeOnSystemRequestCompletion(mNativePtr, mEncodedChain, mAndroidPrivateKey);
         }
-    }
-
-    /** Implementation of CertAsyncTask for the system KeyChain API. */
-    private static class CertAsyncTaskKeyChain extends CertAsyncTask {
-        final Context mContext;
-        final String mAlias;
 
-        CertAsyncTaskKeyChain(Context context, long nativePtr, String alias) {
-            super(nativePtr);
-            mContext = context;
-            assert alias != null;
-            mAlias = alias;
-        }
-
-        @Override
-        String getAlias() {
+        private String getAlias() {
             return mAlias;
         }
 
-        @Override
-        AndroidPrivateKey getPrivateKey(String alias) {
+        private AndroidPrivateKey getPrivateKey(String alias) {
             try {
                 return sLocalKeyStore.createKey(KeyChain.getPrivateKey(mContext, alias));
             } catch (KeyChainException e) {
@@ -138,8 +123,7 @@ public class SSLClientCertificateRequest {
             }
         }
 
-        @Override
-        X509Certificate[] getCertificateChain(String alias) {
+        private X509Certificate[] getCertificateChain(String alias) {
             try {
                 return KeyChain.getCertificateChain(mContext, alias);
             } catch (KeyChainException e) {
@@ -152,36 +136,6 @@ public class SSLClientCertificateRequest {
         }
     }
 
-    /** Implementation of CertAsyncTask for use with a PKCS11-backed KeyStore. */
-    private static class CertAsyncTaskPKCS11 extends CertAsyncTask {
-        private final PKCS11AuthenticationManager mPKCS11AuthManager;
-        private final String mHostName;
-        private final int mPort;
-
-        CertAsyncTaskPKCS11(long nativePtr, String hostName, int port,
-                PKCS11AuthenticationManager pkcs11CardAuthManager) {
-            super(nativePtr);
-            mHostName = hostName;
-            mPort = port;
-            mPKCS11AuthManager = pkcs11CardAuthManager;
-        }
-
-        @Override
-        String getAlias() {
-            return mPKCS11AuthManager.getClientCertificateAlias(mHostName, mPort);
-        }
-
-        @Override
-        AndroidPrivateKey getPrivateKey(String alias) {
-            return mPKCS11AuthManager.getPrivateKey(alias);
-        }
-
-        @Override
-        X509Certificate[] getCertificateChain(String alias) {
-            return mPKCS11AuthManager.getCertificateChain(alias);
-        }
-    }
-
     /**
      * The system KeyChain API will call us back on the alias() method, passing the alias of the
      * certificate selected by the user.
@@ -275,6 +229,7 @@ public class SSLClientCertificateRequest {
                     .setMessage(R.string.client_cert_unsupported_message)
                     .setNegativeButton(R.string.close,
                             new OnClickListener() {
+                                @Override
                                 public void onClick(DialogInterface dialog, int which) {
                                     // Do nothing
                                 }
@@ -321,50 +276,13 @@ public class SSLClientCertificateRequest {
             }
         }
 
-        final Principal[] principalsForCallback = principals;
-        // Certificate for client authentication can be obtained either from the system store of
-        // from a smart card (if available).
-        Runnable useSystemStore = new Runnable() {
-            @Override
-            public void run() {
-                KeyChainCertSelectionCallback callback =
-                        new KeyChainCertSelectionCallback(activity.getApplicationContext(),
-                            nativePtr);
-                KeyChainCertSelectionWrapper keyChain = new KeyChainCertSelectionWrapper(activity,
-                        callback, keyTypes, principalsForCallback, hostName, port, null);
-                maybeShowCertSelection(keyChain, callback,
-                        new CertSelectionFailureDialog(activity));
-            }
-        };
-
-        final Context appContext = activity.getApplicationContext();
-        final PKCS11AuthenticationManager smartCardAuthManager =
-                ((ChromeApplication) appContext).getPKCS11AuthenticationManager();
-        if (smartCardAuthManager.isPKCS11AuthEnabled()) {
-            // Smart card support is available, prompt the user whether to use it or Android system
-            // store.
-            Runnable useSmartCard = new Runnable() {
-                @Override
-                public void run() {
-                    new CertAsyncTaskPKCS11(nativePtr, hostName, port,
-                            smartCardAuthManager).execute();
-                }
-            };
-            Runnable cancelRunnable = new Runnable() {
-                @Override
-                public void run() {
-                    // We took ownership of the request, need to delete it.
-                    nativeOnSystemRequestCompletion(nativePtr, null, null);
-                }
-            };
-
-            KeyStoreSelectionDialog selectionDialog = new KeyStoreSelectionDialog(

[Yaron, 2015/11/25 16:24:31]

You can remove this class too and the strings added in
https://codereview.chromium.org/133373009

[Changwan Ryu, 2015/11/26 02:52:11]

Done.

-                    useSystemStore, useSmartCard, cancelRunnable);
-            selectionDialog.show(activity.getFragmentManager(), null);
-        } else {
-            // Smart card support is not available, use the system store unconditionally.
-            useSystemStore.run();
-        }
+        KeyChainCertSelectionCallback callback =
+                new KeyChainCertSelectionCallback(activity.getApplicationContext(),
+                    nativePtr);
+        KeyChainCertSelectionWrapper keyChain = new KeyChainCertSelectionWrapper(activity,
+                callback, keyTypes, principals, hostName, port, null);
+        maybeShowCertSelection(keyChain, callback,
+                new CertSelectionFailureDialog(activity));
 
         // We've taken ownership of the native ssl request object.
         return true;