Merge pull request #365 from zjw/Issue_5886_misleading_ssl_errors

Fix erroneous SSL certificate warnings

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

@@ -159,11 +159,12 @@ public class AccountSetupCheckSettings extends K9Activity implements OnClickList
                 } catch (final CertificateValidationException cve) {
                     Log.e(K9.LOG_TAG, "Error while testing settings", cve);
 
+                    X509Certificate[] chain = cve.getCertChain();
                     // Avoid NullPointerException in acceptKeyDialog()
-                    if (TrustManagerFactory.getLastCertChain() != null) {
+                    if (chain != null) {
                         acceptKeyDialog(
                             R.string.account_setup_failed_dlg_certificate_message_fmt,
-                            cve);
+                            cve, chain);
                     } else {
                         showErrorDialog(
                                 R.string.account_setup_failed_dlg_server_message_fmt,
@@ -232,16 +233,16 @@ public class AccountSetupCheckSettings extends K9Activity implements OnClickList
             }
         });
     }
-    private void acceptKeyDialog(final int msgResId, final Object... args) {
+
+    private void acceptKeyDialog(final int msgResId,
+            final CertificateValidationException ex, final X509Certificate[] chain) {
         mHandler.post(new Runnable() {
             public void run() {
                 if (mDestroyed) {
                     return;
                 }
-                final X509Certificate[] chain = TrustManagerFactory.getLastCertChain();
                 String exMessage = "Unknown Error";
 
-                Exception ex = ((Exception)args[0]);
                 if (ex != null) {
                     if (ex.getCause() != null) {
                         if (ex.getCause().getCause() != null) {

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

@@ -0,0 +1,34 @@
+package com.fsck.k9.mail;
+
+import java.security.cert.CertificateException;
+import java.security.cert.X509Certificate;
+
+/**
+ * A {@link CertificateException} extension that provides access to
+ * the pertinent certificate chain.
+ *
+ */
+public class CertificateChainException extends CertificateException {
+
+    private static final long serialVersionUID = 1103894512106650107L;
+    private X509Certificate[] mCertChain;
+
+    public CertificateChainException(String msg, X509Certificate[] chain) {
+        super(msg);
+        setCertChain(chain);
+    }
+
+    public CertificateChainException(CertificateException ce,
+            X509Certificate[] chain) {
+        super.initCause(ce);
+        setCertChain(chain);
+    }
+
+    public void setCertChain(X509Certificate[] chain) {
+        mCertChain = chain;
+    }
+    public X509Certificate[] getCertChain() {
+        return mCertChain;
+    }
+
+}

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

@@ -3,9 +3,11 @@ package com.fsck.k9.mail;
 
 import java.security.cert.CertPathValidatorException;
 import java.security.cert.CertificateException;
+import java.security.cert.X509Certificate;
 
 public class CertificateValidationException extends MessagingException {
     public static final long serialVersionUID = -1;
+    private X509Certificate[] mCertChain;
 
     public CertificateValidationException(String message) {
         super(message);
@@ -25,6 +27,22 @@ public class CertificateValidationException extends MessagingException {
             throwable = throwable.getCause();
         }
 
+        if (throwable instanceof CertificateChainException) {
+            mCertChain = ((CertificateChainException) throwable).getCertChain();
+        }
         return throwable != null;
     }
+
+    /**
+     * If the cause of this {@link CertificateValidationException} was a
+     * {@link CertificateChainException}, then the offending chain is available
+     * for return.
+     * 
+     * @return An {@link X509Certificate X509Certificate[]} containing the Cert.
+     *         chain, or else null.
+     */
+    public X509Certificate[] getCertChain() {
+        needsUserAttention();
+        return mCertChain;
+    }
 }

src/com/fsck/k9/mail/store/TrustManagerFactory.java

@@ -6,6 +6,8 @@ import android.content.Context;
 import android.util.Log;
 import com.fsck.k9.K9;
 import com.fsck.k9.helper.DomainNameChecker;
+import com.fsck.k9.mail.CertificateChainException;
+
 import org.apache.commons.io.IOUtils;
 
 import javax.net.ssl.TrustManager;
@@ -28,8 +30,6 @@ public final class TrustManagerFactory {
     private static X509TrustManager unsecureTrustManager;
     private static X509TrustManager localTrustManager;
 
-    private static X509Certificate[] lastCertChain = null;
-
     private static File keyStoreFile;
     private static KeyStore keyStore;
 
@@ -77,13 +77,15 @@ public final class TrustManagerFactory {
 
         public void checkServerTrusted(X509Certificate[] chain, String authType)
         throws CertificateException {
-            // FIXME: Using a static field to store the certificate chain is a bad idea. Instead
-            // create a CertificateException subclass and store the chain there.
-            TrustManagerFactory.setLastCertChain(chain);
             try {
                 defaultTrustManager.checkServerTrusted(chain, authType);
             } catch (CertificateException e) {
-                localTrustManager.checkServerTrusted(new X509Certificate[] {chain[0]}, authType);
+                try {
+                    localTrustManager.checkServerTrusted(
+                            new X509Certificate[] { chain[0] }, authType);
+                } catch (CertificateException ce) {
+                    throw new CertificateChainException(ce, chain);
+                }
             }
             if (!DomainNameChecker.match(chain[0], mHost)) {
                 try {
@@ -94,8 +96,9 @@ public final class TrustManagerFactory {
                 } catch (KeyStoreException e) {
                     throw new CertificateException("Certificate cannot be verified; KeyStore Exception: " + e);
                 }
-                throw new CertificateException("Certificate domain name does not match "
-                                               + mHost);
+                throw new CertificateChainException(
+                        "Certificate domain name does not match " + mHost,
+                        chain);
             }
         }
 
@@ -170,13 +173,6 @@ public final class TrustManagerFactory {
         return keyStore;
     }
 
-    public static void setLastCertChain(X509Certificate[] chain) {
-        lastCertChain = chain;
-    }
-    public static X509Certificate[] getLastCertChain() {
-        return lastCertChain;
-    }
-
     public static void addCertificateChain(String alias, X509Certificate[] chain) throws CertificateException {
         try {
             javax.net.ssl.TrustManagerFactory tmf = javax.net.ssl.TrustManagerFactory.getInstance("X509");