Remove ClientCertificateRequiredException

With this commit, KeyChainKeyManager no longer throws the exception and
AccountSetupCheckSettings no longer catches it.

It was being thrown when the server requested a client certificate but no
client certificate alias had been configured for the server.

The code was making the incorrect assumption that the server would only
request a client certificate when such a certificate was *required*.
However, servers can be configured to accept multiple forms of
authentication, including both password authentication and client
certificate authentication.  So a server may request a certificate without
requiring it.  If a user has not configured a client certificate, then
that should not be treated as an error because the configuration may be
valid and the server may accept it.

The only indication that a certificate is *required* is when a
SSLProtocolException is thrown, caused by a SSLHandshakeException
resulting from a fatal handshake alert message received from the server.
Unfortunately, such a message is fairly generic and only "indicates that
the sender was unable to negotiate an acceptable set of security
parameters given the options available."  So there is no definitive way to
know that a client certificate is required.

Also, KeyChainKeyManager.getCertificateChain() and getPrivateKey() no
longer throw IllegalStateException().  These methods are permitted to
return null, and such a response is appropriate if the user has deleted
client certificates from the device.  Again, this may or may not cause the
server to abort the connection, depending on whether the server *requires*
a client certificate.

res/values/strings.xml

@@ -1124,7 +1124,6 @@ Please submit bug reports, contribute new features and ask questions at
 
     <!-- === Client certificates specific ================================================================== -->
     <string name="account_setup_basics_client_certificate">Use client certificate</string>
-    <string name="dialog_client_certificate_required">This server requires a valid client certificate to be selected.</string>
     <string name="client_certificate_spinner_empty">No client certificate</string>
     <string name="client_certificate_spinner_delete">Remove client certificate selection</string>
 </resources>

src/com/fsck/k9/activity/setup/AccountSetupCheckSettings.java

@@ -25,14 +25,10 @@ import com.fsck.k9.fragment.ConfirmationDialogFragment;
 import com.fsck.k9.fragment.ConfirmationDialogFragment.ConfirmationDialogFragmentListener;
 import com.fsck.k9.mail.AuthenticationFailedException;
 import com.fsck.k9.mail.CertificateValidationException;
-import com.fsck.k9.mail.ClientCertificateRequiredException;
-import com.fsck.k9.mail.ServerSettings;
 import com.fsck.k9.mail.Store;
 import com.fsck.k9.mail.Transport;
 import com.fsck.k9.mail.store.WebDavStore;
 import com.fsck.k9.mail.filter.Hex;
-import com.fsck.k9.security.KeyChainKeyManager;
-
 import java.security.cert.CertificateException;
 import java.security.cert.CertificateEncodingException;
 import java.security.cert.X509Certificate;
@@ -166,8 +162,6 @@ public class AccountSetupCheckSettings extends K9Activity implements OnClickList
                         afe.getMessage() == null ? "" : afe.getMessage());
                 } catch (final CertificateValidationException cve) {
                     handleCertificateValidationException(cve);
-                } catch (final ClientCertificateRequiredException ccr) {
-                    handleClientCertificateRequiredException(ccr);
                 } catch (final Throwable t) {
                     Log.e(K9.LOG_TAG, "Error while testing settings", t);
                     showErrorDialog(
@@ -196,70 +190,6 @@ public class AccountSetupCheckSettings extends K9Activity implements OnClickList
         }
     }
 
-    private void handleClientCertificateRequiredException(ClientCertificateRequiredException ccr) {
-        if (K9.DEBUG)
-            Log.d(K9.LOG_TAG, "Client certificate alias required: " + ccr.getMessage());
-
-        String alias = null;
-        if (CheckDirection.INCOMING.equals(mDirection)) {
-            ServerSettings storeSettings = Store.decodeStoreUri(mAccount.getStoreUri());
-            alias = storeSettings.clientCertificateAlias;
-        } else if (CheckDirection.OUTGOING.equals(mDirection)) {
-            ServerSettings transportSettings = Transport.decodeTransportUri(mAccount.getTransportUri());
-            alias = transportSettings.clientCertificateAlias;
-        }
-
-        if (K9.DEBUG)
-            Log.d(K9.LOG_TAG, "Client certificate alias is: " + alias);
-
-        alias = KeyChainKeyManager.interactivelyChooseClientCertificateAlias(
-                AccountSetupCheckSettings.this,
-                ccr.getKeyTypes(),
-                ccr.getIssuers(),
-                ccr.getHostName(),
-                ccr.getPort(),
-                alias);
-
-        // Note: KeyChainKeyManager gives back "" on cancel
-        if (alias != null && alias.equals("")) {
-            alias = null;
-        }
-
-        // save client certificate alias
-        if (alias != null) {
-            if (CheckDirection.INCOMING.equals(mDirection)) {
-                if (K9.DEBUG)
-                    Log.d(K9.LOG_TAG, "Setting store client certificate alias to: " + alias);
-
-                // Set incoming server client certificate alias
-                String storeUri = mAccount.getStoreUri();
-                ServerSettings incoming = Store.decodeStoreUri(storeUri);
-                ServerSettings newIncoming = incoming.newClientCertificateAlias(alias);
-                String newStoreUri = Store.createStoreUri(newIncoming);
-                mAccount.setStoreUri(newStoreUri);
-            } else if (CheckDirection.OUTGOING.equals(mDirection)) {
-                if (K9.DEBUG)
-                    Log.d(K9.LOG_TAG, "Setting transport client certificate alias to: " + alias);
-
-                // Set outgoing server client certificate alias
-                String transportUri = mAccount.getTransportUri();
-                ServerSettings outgoing = Transport.decodeTransportUri(transportUri);
-                ServerSettings newOutgoing = outgoing.newClientCertificateAlias(alias);
-                String newTransportUri = Transport.createTransportUri(newOutgoing);
-                mAccount.setTransportUri(newTransportUri);
-            }
-
-            // Save the account settings
-            mAccount.save(Preferences.getPreferences(AccountSetupCheckSettings.this));
-
-            // try again
-            AccountSetupCheckSettings.actionCheckSettings(AccountSetupCheckSettings.this, mAccount,
-                    mDirection);
-        } else {
-            showErrorDialog(R.string.dialog_client_certificate_required);
-        }
-    }
-
     @Override
     public void onDestroy() {
         super.onDestroy();

src/com/fsck/k9/mail/ClientCertificateRequiredException.java

@@ -1,48 +0,0 @@
-
-package com.fsck.k9.mail;
-
-import java.security.Principal;
-
-/**
- * This exception is thrown when, during an SSL handshake, a client certificate
- * alias is requested but we want the user to select one instead of using the
- * previously selected one silently. This must be a RuntimeException because the
- * implemented interface of X509ExtendedKeyManager (where it is thrown) does not
- * allow anything else.
- */
-public class ClientCertificateRequiredException extends RuntimeException {
-    public static final long serialVersionUID = -1;
-
-    String[] mKeyTypes;
-    Principal[] mIssuers;
-    String mHostName;
-    int mPort;
-
-    public ClientCertificateRequiredException(String[] keyTypes,
-            Principal[] issuers,
-            String hostName,
-            int port) {
-        super("interactive client certificate alias choice required");
-        this.mKeyTypes = keyTypes;
-        this.mIssuers = issuers;
-        this.mHostName = hostName;
-        this.mPort = port;
-    }
-
-    public String[] getKeyTypes() {
-        return mKeyTypes;
-    }
-
-    public Principal[] getIssuers() {
-        return mIssuers;
-    }
-
-    public String getHostName() {
-        return mHostName;
-    }
-
-    public int getPort() {
-        return mPort;
-    }
-
-}

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

@@ -29,16 +29,8 @@ public class SslHelper {
             Log.d(K9.LOG_TAG, "createSslContext: Client certificate alias: "
                     + clientCertificateAlias);
 
-        KeyManager[] keyManagers = null;
+        KeyManager[] keyManagers = new KeyManager[] { new KeyChainKeyManager(
-        if (clientCertificateAlias != null) {
+                clientCertificateAlias) };
-            keyManagers = new KeyManager[] {
-                    new KeyChainKeyManager(clientCertificateAlias)
-            };
-        } else {
-            keyManagers = new KeyManager[] {
-                    new KeyChainKeyManager()
-            };
-        }
 
         SSLContext sslContext = SSLContext.getInstance("TLS");
         sslContext.init(keyManagers,

src/com/fsck/k9/security/KeyChainKeyManager.java

@@ -5,23 +5,19 @@ import java.net.Socket;
 import java.security.Principal;
 import java.security.PrivateKey;
 import java.security.cert.X509Certificate;
+
 import javax.net.ssl.X509ExtendedKeyManager;
 
-import android.app.Activity;
 import android.os.Build;
 import android.security.KeyChain;
-import android.security.KeyChainAliasCallback;
 import android.security.KeyChainException;
 import android.util.Log;
 
 import com.fsck.k9.K9;
-import com.fsck.k9.mail.ClientCertificateRequiredException;
 
 /**
  * For client certificate authentication! Provide private keys and certificates
- * during the TLS handshake using the Android 4.0 KeyChain API. If interactive
+ * during the TLS handshake using the Android 4.0 KeyChain API.
- * selection is requested, we harvest the parameters during the handshake and
- * abort with a custom (runtime) ClientCertificateRequiredException.
  */
 public class KeyChainKeyManager extends X509ExtendedKeyManager {
 
@@ -29,34 +25,16 @@ public class KeyChainKeyManager extends X509ExtendedKeyManager {
 
     private String mAlias;
 
-    public KeyChainKeyManager() {
-        mAlias = null;
-        if (K9.DEBUG)
-            Log.d(K9.LOG_TAG, "KeyChainKeyManager set to interactive prompting required");
-    }
-
     public KeyChainKeyManager(String alias) {
         if (alias == null || "".equals(alias)) {
-            throw new IllegalArgumentException(
+            mAlias = null;
-                    "KeyChainKeyManager: The provided alias is null or empty!");
+        } else {
+            mAlias = alias;
         }
-
-        mAlias = alias;
-        if (K9.DEBUG)
-            Log.d(K9.LOG_TAG, "KeyChainKeyManager set up with for auto-selected alias " + alias);
     }
 
     @Override
     public String chooseClientAlias(String[] keyTypes, Principal[] issuers, Socket socket) {
-        if (mAlias == null) {
-            throw new ClientCertificateRequiredException(keyTypes, issuers,
-                    socket.getInetAddress().getHostName(), socket.getPort());
-        }
-
-        if (K9.DEBUG)
-            Log.d(K9.LOG_TAG, "KeyChainKeyManager.chooseClientAlias returning preselected alias "
-                    + mAlias);
-
         return mAlias;
     }
 
@@ -69,7 +47,7 @@ public class KeyChainKeyManager extends X509ExtendedKeyManager {
             X509Certificate[] chain = KeyChain.getCertificateChain(K9.app, alias);
 
             if (chain == null || chain.length == 0) {
-                throw new IllegalStateException("No certificate chain found for: " + alias);
+                Log.w(K9.LOG_TAG, "No certificate chain found for: " + alias);
             }
 
             return chain;
@@ -103,7 +81,7 @@ public class KeyChainKeyManager extends X509ExtendedKeyManager {
             }
 
             if (key == null) {
-                throw new IllegalStateException("No private key found for: " + alias);
+                Log.w(K9.LOG_TAG, "No private key found for: " + alias);
             }
             return key;
         } catch (KeyChainException e) {
@@ -140,46 +118,4 @@ public class KeyChainKeyManager extends X509ExtendedKeyManager {
         // not valid for client side
         throw new UnsupportedOperationException();
     }
-
-    public static String interactivelyChooseClientCertificateAlias(Activity activity,
-            String[] keyTypes, Principal[] issuers, String hostName, int port,
-            String preSelectedAlias) {
-        // defined as array to be able to set it inside the callback
-        final String[] selectedAlias = new String[1];
-
-        KeyChain.choosePrivateKeyAlias(activity, new KeyChainAliasCallback() {
-            @Override
-            public void alias(String alias) {
-                synchronized (selectedAlias) {
-                    if (K9.DEBUG)
-                        Log.d(K9.LOG_TAG, "User has selected client certificate alias:" + alias);
-
-                    // see below. not null is condition for breaking out of loop
-                    if (alias == null) {
-                        alias = "";
-                    }
-                    selectedAlias[0] = alias;
-                    selectedAlias.notifyAll();
-                }
-
-            }
-        }, keyTypes, issuers, hostName, port, preSelectedAlias);
-
-        synchronized (selectedAlias) {
-            while (selectedAlias[0] == null) {
-                try {
-                    selectedAlias.wait();
-                } catch (InterruptedException e) {
-                    Thread.currentThread().interrupt();
-                    break;
-                }
-            }
-
-            if ("".equals(selectedAlias[0])) {
-                selectedAlias[0] = null;
-            }
-        }
-
-        return selectedAlias[0];
-    }
 }