Handle issuer and key type restrictions.

The caller to KeyChain.choosePrivateKeyAlias can restrict the set of
aliases that are displayed to the user to select from by specifying the
issuers that the associated certificates should be issued by or the key
types that these certificates should contain.

Until now this functionality was not implemented. This was mostly
affecting Chrome
(https://bugs.chromium.org/p/chromium/issues/detail?id=753756).

Support this functionality by passing the issuers and key types into the
KeyChainActivity (from KeyChain) and, prior to displaying the aliases
associated with the certificates, check if each certificate adheres to
the criteria (key type, issues) specified.

Bug: 62910781
Test: m -j RunKeyChainRoboTests
Change-Id: I75e071545699891cfbd77d4f706fc5ef35b85516

1 files changed, 61 insertions, 21 deletions

diff --git a/keystore/java/android/security/KeyChain.java b/keystore/java/android/security/KeyChain.java
index 78dbb6ae0df3..5d5e40fd3ac3 100644
--- a/keystore/java/android/security/KeyChain.java
+++ b/keystore/java/android/security/KeyChain.java
@@ -40,6 +40,7 @@ import android.security.keystore.KeyProperties;
 
 import java.io.ByteArrayInputStream;
 import java.io.Closeable;
+import java.io.Serializable;
 import java.security.KeyPair;
 import java.security.Principal;
 import java.security.PrivateKey;
@@ -55,6 +56,8 @@ import java.util.Locale;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.LinkedBlockingQueue;
 
+import javax.security.auth.x500.X500Principal;
+
 import com.android.org.conscrypt.TrustedCertificateStore;
 
 /**
@@ -142,6 +145,18 @@ public final class KeyChain {
     public static final String EXTRA_SENDER = "sender";
 
     /**
+     * Extra for use with {@link #ACTION_CHOOSER}
+     * @hide Also used by KeyChainActivity implementation
+     */
+    public static final String EXTRA_KEY_TYPES = "key_types";
+
+    /**
+     * Extra for use with {@link #ACTION_CHOOSER}
+     * @hide Also used by KeyChainActivity implementation
+     */
+    public static final String EXTRA_ISSUERS = "issuers";
+
+    /**
      * Action to bring up the CertInstaller.
      */
     private static final String ACTION_INSTALL = "android.credentials.INSTALL";
@@ -365,9 +380,10 @@ public final class KeyChain {
      * onChoosePrivateKeyAlias}.
      *
      * <p>{@code keyTypes} and {@code issuers} may be used to
-     * highlight suggested choices to the user, although to cope with
-     * sometimes erroneous values provided by servers, the user may be
-     * able to override these suggestions.
+     * narrow down suggested choices to the user. If either {@code keyTypes}
+     * or {@code issuers} is specified and non-empty, and there are no
+     * matching certificates in the KeyChain, then the certificate
+     * selection prompt would be suppressed entirely.
      *
      * <p>{@code host} and {@code port} may be used to give the user
      * more context about the server requesting the credentials.
@@ -382,7 +398,7 @@ public final class KeyChain {
      * @param response Callback to invoke when the request completes;
      *     must not be null.
      * @param keyTypes The acceptable types of asymmetric keys such as
-     *     "RSA" or "DSA", or null.
+     *     "RSA", "EC" or null.
      * @param issuers The acceptable certificate issuers for the
      *     certificate matching the private key, or null.
      * @param host The host name of the server requesting the
@@ -419,9 +435,10 @@ public final class KeyChain {
      * onChoosePrivateKeyAlias}.
      *
      * <p>{@code keyTypes} and {@code issuers} may be used to
-     * highlight suggested choices to the user, although to cope with
-     * sometimes erroneous values provided by servers, the user may be
-     * able to override these suggestions.
+     * narrow down suggested choices to the user. If either {@code keyTypes}
+     * or {@code issuers} is specified and non-empty, and there are no
+     * matching certificates in the KeyChain, then the certificate
+     * selection prompt would be suppressed entirely.
      *
      * <p>{@code uri} may be used to give the user more context about
      * the server requesting the credentials.
@@ -436,13 +453,15 @@ public final class KeyChain {
      * @param response Callback to invoke when the request completes;
      *     must not be null.
      * @param keyTypes The acceptable types of asymmetric keys such as
-     *     "EC" or "RSA", or null.
+     *     "RSA", "EC" or null.
      * @param issuers The acceptable certificate issuers for the
      *     certificate matching the private key, or null.
      * @param uri The full URI the server is requesting the certificate
      *     for, or null if unavailable.
      * @param alias The alias to preselect if available, or null if
      *     unavailable.
+     * @throws IllegalArgumentException if the specified issuers are not
+     *     of type {@code X500Principal}.
      */
     public static void choosePrivateKeyAlias(@NonNull Activity activity,
             @NonNull KeyChainAliasCallback response,
@@ -450,20 +469,21 @@ public final class KeyChain {
             @Nullable Principal[] issuers,
             @Nullable Uri uri, @Nullable String alias) {
         /*
-         * TODO currently keyTypes, issuers are unused. They are meant
-         * to follow the semantics and purpose of X509KeyManager
-         * method arguments.
-         *
-         * keyTypes would allow the list to be filtered and typically
-         * will be set correctly by the server. In practice today,
-         * most all users will want only RSA or EC, and usually
-         * only a small number of certs will be available.
+         * Specifying keyTypes excludes certificates with different key types
+         * from the list of certificates presented to the user.
+         * In practice today, most servers would require RSA or EC
+         * certificates.
          *
-         * issuers is typically not useful. Some servers historically
-         * will send the entire list of public CAs known to the
-         * server. Others will send none. If this is used, if there
-         * are no matches after applying the constraint, it should be
-         * ignored.
+         * Specifying issuers narrows down the list by filtering out
+         * certificates with issuers which are not matching the provided ones.
+         * This has been reported to Chrome several times (crbug.com/731769).
+         * There's no concrete description on what to do when the client has no
+         * certificates that match the provided issuers.
+         * To be conservative, Android will not present the user with any
+         * certificates to choose from.
+         * If the list of issuers is empty then the client may send any
+         * certificate, see:
+         * https://tools.ietf.org/html/rfc5246#section-7.4.4
          */
         if (activity == null) {
             throw new NullPointerException("activity == null");
@@ -476,6 +496,26 @@ public final class KeyChain {
         intent.putExtra(EXTRA_RESPONSE, new AliasResponse(response));
         intent.putExtra(EXTRA_URI, uri);
         intent.putExtra(EXTRA_ALIAS, alias);
+        intent.putExtra(EXTRA_KEY_TYPES, keyTypes);
+        ArrayList<byte[]> issuersList = new ArrayList();
+        if (issuers != null) {
+            for (Principal issuer: issuers) {
+                // In a TLS client context (like Chrome), issuers would only
+                // be specified as X500Principals. No other use cases for
+                // specifying principals have been brought up. Under these
+                // circumstances, only allow issuers specified as
+                // X500Principals.
+                if (!(issuer instanceof X500Principal)) {
+                    throw new IllegalArgumentException(String.format(
+                            "Issuer %s is of type %s, not X500Principal",
+                            issuer.toString(), issuer.getClass()));
+                }
+                // Pass the DER-encoded issuer as that's the most accurate
+                // representation and what is sent over the wire.
+                issuersList.add(((X500Principal) issuer).getEncoded());
+            }
+        }
+        intent.putExtra(EXTRA_ISSUERS, (Serializable) issuersList);
         // the PendingIntent is used to get calling package name
         intent.putExtra(EXTRA_SENDER, PendingIntent.getActivity(activity, 0, new Intent(), 0));
         activity.startActivity(intent);