Remove Android support for out-of-process KeyStores

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

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.chrome.browser;
 
 import android.app.Activity;
 import android.content.ActivityNotFoundException;
 import android.content.Context;
 import android.content.DialogInterface;
 import android.content.DialogInterface.OnClickListener;
 import android.os.AsyncTask;
 import android.security.KeyChain;
 import android.security.KeyChainAliasCallback;
 import android.security.KeyChainException;
 import android.support.v7.app.AlertDialog;
 import android.util.Log;
 
 import org.chromium.base.ThreadUtils;
 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;
 
 import java.security.Principal;
 import java.security.cert.CertificateEncodingException;
 import java.security.cert.X509Certificate;
 
 import javax.security.auth.x500.X500Principal;
 
 /**
  * Handles selection of client certificate on the Java side. This class is responsible for selection
  * of the client certificate to be used for authentication and retrieval of the private key and full
  * certificate chain.
  *
  * The entry point is selectClientCertificate() and it will be called on the UI thread. Then the
  * class will construct and run an appropriate CertAsyncTask, that will run in background, and
  * finally pass the results back to the UI thread, which will return to the native code.
  */
 @JNINamespace("chrome::android")
 public class SSLClientCertificateRequest {
     static final String TAG = "SSLClientCertificateRequest";
 
     private static final DefaultAndroidKeyStore sLocalKeyStore =
             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;
         private AndroidPrivateKey mAndroidPrivateKey;
 
         // 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();
             if (alias == null) return null;
 
             AndroidPrivateKey key = getPrivateKey(alias);
             X509Certificate[] chain = getCertificateChain(alias);
 
             if (key == null || chain == null || chain.length == 0) {
                 Log.w(TAG, "Empty client certificate chain?");
@@ skipping 14 matching lines @@
             mEncodedChain = encodedChain;
             mAndroidPrivateKey = key;
             return null;
         }
 
         @Override
         protected void onPostExecute(Void result) {
             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) {
                 Log.w(TAG, "KeyChainException when looking for '" + alias + "' certificate");
                 return null;
             } catch (InterruptedException e) {
                 Log.w(TAG, "InterruptedException when looking for '" + alias + "'certificate");
                 return null;
             }
         }
 
-        @Override
-        X509Certificate[] getCertificateChain(String alias) {
+        private X509Certificate[] getCertificateChain(String alias) {
             try {
                 return KeyChain.getCertificateChain(mContext, alias);
             } catch (KeyChainException e) {
                 Log.w(TAG, "KeyChainException when looking for '" + alias + "' certificate");
                 return null;
             } catch (InterruptedException e) {
                 Log.w(TAG, "InterruptedException when looking for '" + alias + "'certificate");
                 return null;
             }
         }
     }
 
-    /** 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.
      */
     private static class KeyChainCertSelectionCallback implements KeyChainAliasCallback {
         private final long mNativePtr;
         private final Context mContext;
 
         KeyChainCertSelectionCallback(Context context, long nativePtr) {
             mContext = context;
@@ skipping 73 matching lines @@
         /**
          * Builds and shows the dialog.
          */
         public void show() {
             final AlertDialog.Builder builder =
                     new AlertDialog.Builder(mActivity, R.style.AlertDialogTheme);
             builder.setTitle(R.string.client_cert_unsupported_title)
                     .setMessage(R.string.client_cert_unsupported_message)
                     .setNegativeButton(R.string.close,
                             new OnClickListener() {
+                                @Override
                                 public void onClick(DialogInterface dialog, int which) {
                                     // Do nothing
                                 }
                             });
             builder.show();
         }
     }
 
     /**
      * Create a new asynchronous request to select a client certificate.
@@ skipping 26 matching lines @@
             try {
                 for (int n = 0; n < encodedPrincipals.length; n++) {
                     principals[n] = new X500Principal(encodedPrincipals[n]);
                 }
             } catch (Exception e) {
                 Log.w(TAG, "Exception while decoding issuers list: " + e);
                 return false;
             }
         }
 
-        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;
     }
 
     /**
      * Attempt to show the certificate selection dialog and shows the provided
      * CertSelectionFailureDialog if the platform's cert selection activity can't be found.
      */
     @VisibleForTesting
@@ skipping 16 matching lines @@
         Log.d(TAG, "ClientCertificatesChanged!");
         nativeNotifyClientCertificatesChangedOnIOThread();
     }
 
     private static native void nativeNotifyClientCertificatesChangedOnIOThread();
 
     // Called to pass request results to native side.
     private static native void nativeOnSystemRequestCompletion(
             long requestPtr, byte[][] certChain, AndroidPrivateKey androidKey);
 }