KeyChainKeyManager modifications

The constructor now saves the certificate chain, so the code to retrieve
it again or to perform any additional error checking in
getCertificateChain() is no longer needed.

The constructor now retrieves and saves the private key so that any
resulting errors are detected sooner.

Methods that retrieve the alias perform checks to assure that the client
cert. satisfies the requested issuers and key type.  It's known that
Sendmail may provide a list of issuers in its certificate request, but
then may authenticate against a much larger set of CAs, but then later
reject the mail because the client certificate was not acceptable.
Vetting against the issuer list helps detect such certificate problems
sooner (upon connection) rather than later (upon transmission of mail).
Earlier error detection is necessary so that errors may be presented to
the user during account setup.

Portions of these modifications are based on code from KeyManagerImpl:
https://android.googlesource.com/platform/external/conscrypt/+/master/src/main/java/org/conscrypt/KeyManagerImpl.java

src/com/fsck/k9/net/ssl/KeyChainKeyManager.java

@@ -5,8 +5,13 @@ import java.net.Socket;
 import java.security.Principal;
 import java.security.PrivateKey;
 import java.security.cert.X509Certificate;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Locale;
 
+import javax.net.ssl.SSLEngine;
 import javax.net.ssl.X509ExtendedKeyManager;
+import javax.security.auth.x500.X500Principal;
 
 import android.os.Build;
 import android.security.KeyChain;
@@ -28,6 +33,10 @@ public class KeyChainKeyManager extends X509ExtendedKeyManager {
 
     private String mAlias;
 
+    private X509Certificate[] mChain;
+
+    private PrivateKey mPrivateKey;
+
     /**
      * @param alias  Must not be null nor empty
      * @throws MessagingException
@@ -37,10 +46,30 @@ public class KeyChainKeyManager extends X509ExtendedKeyManager {
     public KeyChainKeyManager(String alias) throws MessagingException {
         mAlias = alias;
 
-        // Check for invalid alias (the user may have deleted the certificate)
         try {
-            KeyChain.getCertificateChain(K9.app, alias);
+            mChain = KeyChain.getCertificateChain(K9.app, alias);
+            if (mChain == null || mChain.length == 0) {
+                throw new MessagingException("No certificate chain found for: " + alias);
+            }
+
+            /*
+             * We need to keep reference to the first private key retrieved so
+             * it won't get garbage collected. If it will then the whole app
+             * will crash on Android < 4.2 with "Fatal signal 11 code=1". See
+             * https://code.google.com/p/android/issues/detail?id=62319
+             */
+            if (sClientCertificateReferenceWorkaround == null
+                    && Build.VERSION.SDK_INT < Build.VERSION_CODES.JELLY_BEAN_MR1) {
+                mPrivateKey = retrieveFirstPrivateKey(alias);
+            } else {
+                mPrivateKey = KeyChain.getPrivateKey(K9.app, alias);
+            }
+
+            if (mPrivateKey == null) {
+                throw new MessagingException("No private key found for: " + alias);
+            }
         } catch (KeyChainException e) {
+            // The certificate was possibly deleted.  Notify user of error.
             throw new CertificateValidationException(K9.app.getString(
                     R.string.client_certificate_retrieval_failure, alias), e);
         } catch (InterruptedException e) {
@@ -51,61 +80,17 @@ public class KeyChainKeyManager extends X509ExtendedKeyManager {
 
     @Override
     public String chooseClientAlias(String[] keyTypes, Principal[] issuers, Socket socket) {
-        return mAlias;
+        return chooseAlias(keyTypes, issuers);
     }
 
     @Override
     public X509Certificate[] getCertificateChain(String alias) {
-        try {
+        return (mAlias.equals(alias) ? mChain : null);
-            if (K9.DEBUG)
-                Log.d(K9.LOG_TAG, "KeyChainKeyManager.getCertificateChain for " + alias);
-
-            X509Certificate[] chain = KeyChain.getCertificateChain(K9.app, alias);
-
-            if (chain == null || chain.length == 0) {
-                Log.w(K9.LOG_TAG, "No certificate chain found for: " + alias);
-            }
-
-            return chain;
-        } catch (KeyChainException e) {
-            throw new RuntimeException(e);
-        } catch (InterruptedException e) {
-            Thread.currentThread().interrupt();
-            return null;
-        }
     }
 
     @Override
     public PrivateKey getPrivateKey(String alias) {
-        try {
+        return (mAlias.equals(alias) ? mPrivateKey : null);
-            if (K9.DEBUG)
-                Log.d(K9.LOG_TAG, "KeyChainKeyManager.getPrivateKey for " + alias);
-
-            PrivateKey key;
-
-            /*
-             * We need to keep reference to the first private key retrieved so
-             * it won't get garbage collected. If it will then the whole app
-             * will crash on Android < 4.2 with "Fatal signal 11 code=1". See
-             * https://code.google.com/p/android/issues/detail?id=62319
-             */
-            if (sClientCertificateReferenceWorkaround == null
-                    && Build.VERSION.SDK_INT < Build.VERSION_CODES.JELLY_BEAN_MR1) {
-                key = retrieveFirstPrivateKey(alias);
-            } else {
-                key = KeyChain.getPrivateKey(K9.app, alias);
-            }
-
-            if (key == null) {
-                Log.w(K9.LOG_TAG, "No private key found for: " + alias);
-            }
-            return key;
-        } catch (KeyChainException e) {
-            throw new RuntimeException(e);
-        } catch (InterruptedException e) {
-            Thread.currentThread().interrupt();
-            return null;
-        }
     }
 
     private synchronized PrivateKey retrieveFirstPrivateKey(String alias)
@@ -119,19 +104,90 @@ public class KeyChainKeyManager extends X509ExtendedKeyManager {
 
     @Override
     public String chooseServerAlias(String keyType, Principal[] issuers, Socket socket) {
-        // not valid for client side
+        return chooseAlias(new String[] { keyType }, issuers);
-        throw new UnsupportedOperationException();
     }
 
     @Override
     public String[] getClientAliases(String keyType, Principal[] issuers) {
-        // not valid for client side
+        final String al = chooseAlias(new String[] { keyType }, issuers);
-        throw new UnsupportedOperationException();
+        return (al == null ? null : new String[] { al });
     }
 
     @Override
     public String[] getServerAliases(String keyType, Principal[] issuers) {
-        // not valid for client side
+        final String al = chooseAlias(new String[] { keyType }, issuers);
-        throw new UnsupportedOperationException();
+        return (al == null ? null : new String[] { al });
+    }
+
+    @Override
+    public String chooseEngineClientAlias(String[] keyTypes, Principal[] issuers, SSLEngine engine) {
+        return chooseAlias(keyTypes, issuers);
+    }
+
+    @Override
+    public String chooseEngineServerAlias(String keyType, Principal[] issuers, SSLEngine engine) {
+        return chooseAlias(new String[] { keyType }, issuers);
+    }
+
+    private String chooseAlias(String[] keyTypes, Principal[] issuers) {
+        if (keyTypes == null || keyTypes.length == 0) {
+            return null;
+        }
+        final X509Certificate cert = mChain[0];
+        final String certKeyAlg = cert.getPublicKey().getAlgorithm();
+        final String certSigAlg = cert.getSigAlgName().toUpperCase(Locale.US);
+        for (String keyAlgorithm : keyTypes) {
+            if (keyAlgorithm == null) {
+                continue;
+            }
+            final String sigAlgorithm;
+            // handle cases like EC_EC and EC_RSA
+            int index = keyAlgorithm.indexOf('_');
+            if (index == -1) {
+                sigAlgorithm = null;
+            } else {
+                sigAlgorithm = keyAlgorithm.substring(index + 1);
+                keyAlgorithm = keyAlgorithm.substring(0, index);
+            }
+            // key algorithm does not match
+            if (!certKeyAlg.equals(keyAlgorithm)) {
+                continue;
+            }
+            /*
+             * TODO find a more reliable test for signature
+             * algorithm. Unfortunately value varies with
+             * provider. For example for "EC" it could be
+             * "SHA1WithECDSA" or simply "ECDSA".
+             */
+            // sig algorithm does not match
+            if (sigAlgorithm != null && certSigAlg != null
+                    && !certSigAlg.contains(sigAlgorithm)) {
+                continue;
+            }
+            // no issuers to match
+            if (issuers == null || issuers.length == 0) {
+                return mAlias;
+            }
+            List<Principal> issuersList = Arrays.asList(issuers);
+            // check that a certificate in the chain was issued by one of the specified issuers
+            for (X509Certificate certFromChain : mChain) {
+                /*
+                 * Note use of X500Principal from
+                 * getIssuerX500Principal as opposed to Principal
+                 * from getIssuerDN. Principal.equals test does
+                 * not work in the case where
+                 * xcertFromChain.getIssuerDN is a bouncycastle
+                 * org.bouncycastle.jce.X509Principal.
+                 */
+                X500Principal issuerFromChain = certFromChain.getIssuerX500Principal();
+                if (issuersList.contains(issuerFromChain)) {
+                    return mAlias;
+                }
+            }
+            Log.w(K9.LOG_TAG, "Client certificate" + mAlias + "not issued by any of the requested issuers");
+            return null;
+        }
+        Log.w(K9.LOG_TAG, "Client certificate" + mAlias + "does not match any of the requested key types");
+        return null;
     }
 }