Merge "Handle issuer and key type restrictions."

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;
 
 /**
@@ -141,6 +144,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.
      */
@@ -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.
+         * 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.
          *
-         * 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.
-         *
-         * 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);