diff --git a/res/values/strings.xml b/res/values/strings.xml index b14762437..fa5c2d76b 100644 --- a/res/values/strings.xml +++ b/res/values/strings.xml @@ -1126,4 +1126,5 @@ Please submit bug reports, contribute new features and ask questions at Use client certificate No client certificate Remove client certificate selection + "Failed to retrieve client certificate for alias %s" diff --git a/src/com/fsck/k9/mail/CertificateValidationException.java b/src/com/fsck/k9/mail/CertificateValidationException.java index 8f04d13c3..54c3f19bf 100644 --- a/src/com/fsck/k9/mail/CertificateValidationException.java +++ b/src/com/fsck/k9/mail/CertificateValidationException.java @@ -5,6 +5,10 @@ import java.security.cert.CertPathValidatorException; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; +import javax.net.ssl.SSLHandshakeException; + +import android.security.KeyChainException; + public class CertificateValidationException extends MessagingException { public static final long serialVersionUID = -1; private X509Certificate[] mCertChain; @@ -23,16 +27,57 @@ public class CertificateValidationException extends MessagingException { private void scanForCause() { Throwable throwable = getCause(); - /* user attention is required if the certificate was deemed invalid */ + /* + * User attention is required if the server certificate was deemed + * invalid or if there was a problem with a client certificate. + * + * A CertificateException is known to be thrown by the default + * X509TrustManager.checkServerTrusted() if the server certificate + * doesn't validate. The cause of the CertificateException will be a + * CertPathValidatorException. However, it's unlikely those exceptions + * will be encountered here, because they are caught in + * SecureX509TrustManager.checkServerTrusted(), which throws a + * CertificateChainException instead (an extension of + * CertificateException). + * + * A CertificateChainException will likely result in (or, be the cause + * of) an SSLHandshakeException (an extension of SSLException). + * + * The various mail protocol handlers (IMAP, POP3, ...) will catch an + * SSLException and throw a CertificateValidationException (this class) + * with the SSLException as the cause. They may also throw a + * CertificateValidationException with a new CertificateException as the + * cause when STARTTLS is not available, just for the purpose of + * triggering a user notification. + * + * SSLHandshakeException is also known to occur if the *client* + * certificate was not accepted by the server (unknown CA, certificate + * expired, etc.). In this case, the SSLHandshakeException will not have + * a CertificateChainException as a cause. + * + * KeyChainException is known to occur if the device has no client + * certificate that's associated with the alias stored in the server + * settings. + */ while (throwable != null && !(throwable instanceof CertPathValidatorException) - && !(throwable instanceof CertificateException)) { + && !(throwable instanceof CertificateException) + && !(throwable instanceof KeyChainException) + && !(throwable instanceof SSLHandshakeException)) { throwable = throwable.getCause(); } if (throwable != null) { mNeedsUserAttention = true; - if (throwable instanceof CertificateChainException) { + + // See if there is a server certificate chain attached to the SSLHandshakeException + if (throwable instanceof SSLHandshakeException) { + while (throwable != null && !(throwable instanceof CertificateChainException)) { + throwable = throwable.getCause(); + } + } + + if (throwable != null && throwable instanceof CertificateChainException) { mCertChain = ((CertificateChainException) throwable).getCertChain(); } } diff --git a/src/com/fsck/k9/net/ssl/SslHelper.java b/src/com/fsck/k9/net/ssl/SslHelper.java index 12254c81c..af3723684 100644 --- a/src/com/fsck/k9/net/ssl/SslHelper.java +++ b/src/com/fsck/k9/net/ssl/SslHelper.java @@ -29,8 +29,13 @@ public class SslHelper { Log.d(K9.LOG_TAG, "createSslContext: Client certificate alias: " + clientCertificateAlias); - KeyManager[] keyManagers = new KeyManager[] { new KeyChainKeyManager( - clientCertificateAlias) }; + KeyManager[] keyManagers; + if (clientCertificateAlias == null || clientCertificateAlias.isEmpty()) { + keyManagers = null; + } else { + keyManagers = new KeyManager[] { new KeyChainKeyManager( + clientCertificateAlias) }; + } SSLContext sslContext = SSLContext.getInstance("TLS"); sslContext.init(keyManagers, diff --git a/src/com/fsck/k9/security/KeyChainKeyManager.java b/src/com/fsck/k9/security/KeyChainKeyManager.java index 29bacdd58..6de30c48d 100644 --- a/src/com/fsck/k9/security/KeyChainKeyManager.java +++ b/src/com/fsck/k9/security/KeyChainKeyManager.java @@ -14,6 +14,9 @@ import android.security.KeyChainException; import android.util.Log; import com.fsck.k9.K9; +import com.fsck.k9.R; +import com.fsck.k9.mail.CertificateValidationException; +import com.fsck.k9.mail.MessagingException; /** * For client certificate authentication! Provide private keys and certificates @@ -25,11 +28,24 @@ public class KeyChainKeyManager extends X509ExtendedKeyManager { private String mAlias; - public KeyChainKeyManager(String alias) { - if (alias == null || "".equals(alias)) { - mAlias = null; - } else { - mAlias = alias; + /** + * @param alias Must not be null nor empty + * @throws MessagingException + * Indicates an error in retrieving the certificate for the alias + * (likely because the alias is invalid or the certificate was deleted) + */ + 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); + } catch (KeyChainException e) { + throw new CertificateValidationException(K9.app.getString( + R.string.client_certificate_retrieval_failure, alias), e); + } catch (InterruptedException e) { + throw new MessagingException(K9.app.getString( + R.string.client_certificate_retrieval_failure, alias), e); } }