Merge pull request #809 from csmith/certs

Use HostChecker in CertificateManager.

src/main/java/com/dmdirc/tls/CertificateManager.java

@@ -17,13 +17,12 @@
 
 package com.dmdirc.tls;
 
+import com.dmdirc.config.provider.AggregateConfigProvider;
+import com.dmdirc.config.provider.ConfigProvider;
 import com.dmdirc.events.ServerCertificateProblemEncounteredEvent;
 import com.dmdirc.events.ServerCertificateProblemResolvedEvent;
-import com.dmdirc.interfaces.Connection;
 import com.dmdirc.events.eventbus.EventBus;
-import com.dmdirc.config.provider.AggregateConfigProvider;
-import com.dmdirc.config.provider.ConfigProvider;
-
+import com.dmdirc.interfaces.Connection;
 import java.io.FileInputStream;
 import java.io.FileNotFoundException;
 import java.io.IOException;
@@ -32,7 +31,6 @@ import java.security.InvalidAlgorithmParameterException;
 import java.security.KeyStore;
 import java.security.KeyStoreException;
 import java.security.cert.CertificateException;
-import java.security.cert.CertificateParsingException;
 import java.security.cert.PKIXParameters;
 import java.security.cert.TrustAnchor;
 import java.security.cert.X509Certificate;
@@ -47,14 +45,12 @@ import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.Semaphore;
 import java.util.stream.Collectors;
-
 import javax.naming.InvalidNameException;
 import javax.naming.ldap.LdapName;
 import javax.naming.ldap.Rdn;
 import javax.net.ssl.KeyManager;
 import javax.net.ssl.KeyManagerFactory;
 import javax.net.ssl.X509TrustManager;
-
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -90,6 +86,8 @@ public class CertificateManager implements X509TrustManager {
     private final ConfigProvider userSettings;
     /** Locator to use to find a system keystore. */
     private final KeyStoreLocator keyStoreLocator;
+    /** Checker to use for hostnames. */
+    private final CertificateHostChecker hostChecker;
 
     /**
      * Creates a new certificate manager for a client connecting to the specified server.
@@ -111,6 +109,7 @@ public class CertificateManager implements X509TrustManager {
         this.userSettings = userSettings;
         this.eventBus = eventBus;
         this.keyStoreLocator = new KeyStoreLocator();
+        this.hostChecker = new CertificateHostChecker();
 
         loadTrustedCAs();
     }
@@ -206,74 +205,16 @@ public class CertificateManager implements X509TrustManager {
         return TrustResult.UNTRUSTED_GENERAL;
     }
 
-    /**
-     * Determines whether the given certificate has a valid CN or alternate name for this server's
-     * hostname.
-     *
-     * @param certificate The certificate to be validated
-     *
-     * @return True if the certificate is valid for this server's host, false otherwise
-     */
-    public boolean isValidHost(final X509Certificate certificate) {
-        final Map<String, String> fields = getDNFieldsFromCert(certificate);
-        if (fields.containsKey("CN") && isMatchingServerName(fields.get("CN"))) {
-            return true;
-        }
-
-        try {
-            if (certificate.getSubjectAlternativeNames() != null) {
-                for (List<?> entry : certificate.getSubjectAlternativeNames()) {
-                    final int type = (Integer) entry.get(0);
-
-                    // DNS or IP
-                    if ((type == 2 || type == 7) && isMatchingServerName((String) entry.get(1))) {
-                        return true;
-                    }
-                }
-            }
-        } catch (CertificateParsingException ex) {
-            return false;
-        }
-
-        return false;
-    }
-
-    /**
-     * Checks whether the specified target matches the server name this certificate manager was
-     * initialised with.
-     *
-     * Target names may contain wildcards per RFC2818.
-     *
-     * @since 0.6.5
-     * @param target The target to compare to our server name
-     *
-     * @return True if the target matches, false otherwise
-     */
-    protected boolean isMatchingServerName(final String target) {
-        final String[] targetParts = target.split("\\.");
-        final String[] serverParts = serverName.split("\\.");
-
-        if (targetParts.length != serverParts.length) {
-            // Fail fast if they don't match
-            return false;
-        }
-
-        for (int i = 0; i < serverParts.length; i++) {
-            if (!serverParts[i].matches("\\Q" + targetParts[i].replace("*", "\\E.*\\Q") + "\\E")) {
-                return false;
-            }
-        }
-
-        return true;
-    }
-
     @Override
     public void checkServerTrusted(final X509Certificate[] chain, final String authType)
             throws CertificateException {
         this.chain = Arrays.copyOf(chain, chain.length);
         problems.clear();
 
-        checkHost(chain);
+        if (!hostChecker.isValidFor(chain[0], serverName)) {
+            problems.add(new CertificateDoesntMatchHostException(
+                    "Certificate was not issued to " + serverName));
+        }
 
         if (checkIssuer(chain)) {
             problems.clear();
@@ -343,19 +284,6 @@ public class CertificateManager implements X509TrustManager {
         return manual;
     }
 
-    /**
-     * Checks that the host of the leaf certificate is the same as the server we are connected to.
-     *
-     * @param chain The chain of certificates to check.
-     */
-    private void checkHost(final X509Certificate... chain) {
-        // Check that the cert is issued to the correct host
-        if (!isValidHost(chain[0])) {
-            problems.add(new CertificateDoesntMatchHostException(
-                    "Certificate was not issued to " + serverName));
-        }
-    }
-
     /**
      * Gets the chain of certificates currently being validated, if any.
      *

src/main/java/com/dmdirc/ui/core/dialogs/sslcertificate/SSLCertificateDialogModel.java

@@ -19,6 +19,7 @@ package com.dmdirc.ui.core.dialogs.sslcertificate;
 
 import com.dmdirc.tls.CertificateAction;
 import com.dmdirc.tls.CertificateDoesntMatchHostException;
+import com.dmdirc.tls.CertificateHostChecker;
 import com.dmdirc.tls.CertificateManager;
 import com.dmdirc.tls.CertificateNotTrustedException;
 
@@ -47,6 +48,8 @@ public class SSLCertificateDialogModel {
     private final CertificateManager manager;
     /** The list of problems found with the certs, if any. */
     private final Collection<CertificateException> problems;
+    /** Checker to use for hostnames. */
+    private final CertificateHostChecker hostChecker;
 
     /**
      * Creates a new SSLCertificateDialogModel for the specified chain.
@@ -61,6 +64,7 @@ public class SSLCertificateDialogModel {
         this.chain = chain;
         this.problems = problems;
         this.manager = manager;
+        this.hostChecker = new CertificateHostChecker();
     }
 
     /**
@@ -75,7 +79,7 @@ public class SSLCertificateDialogModel {
         boolean first = true;
 
         for (X509Certificate cert : chain) {
-            boolean invalid = first && !manager.isValidHost(cert);
+            boolean invalid = first && !hostChecker.isValidFor(cert, manager.getServerName());
             first = false;
 
             try {
@@ -123,7 +127,7 @@ public class SSLCertificateDialogModel {
                 cert.getNotAfter().toString(), tooOld, false));
         res.add(group);
 
-        final boolean wrongName = index == 0 && !manager.isValidHost(cert);
+        final boolean wrongName = index == 0 && !hostChecker.isValidFor(cert, manager.getServerName());
         final String names = getAlternateNames(cert);
         final Map<String, String> fields = CertificateManager.getDNFieldsFromCert(cert);
 
@@ -160,7 +164,7 @@ public class SSLCertificateDialogModel {
      *
      * @return A comma-separated list of alternate names
      */
-    protected String getAlternateNames(final X509Certificate cert) {
+    private String getAlternateNames(final X509Certificate cert) {
         final StringBuilder res = new StringBuilder();
 
         try {
@@ -196,11 +200,13 @@ public class SSLCertificateDialogModel {
      * @param field   The name of the field to look for
      * @param invalid Whether or not the field is a cause for concern
      */
-    protected void addCertField(final Map<String, String> fields,
-            final List<CertificateInformationEntry> group, final String title,
-            final String field, final boolean invalid) {
-        group.add(new CertificateInformationEntry(title,
-                fields.containsKey(field) ? fields.get(field) : NOTPRESENT, invalid,
+    private void addCertField(
+            final Map<String, String> fields,
+            final List<CertificateInformationEntry> group,
+            final String title,
+            final String field,
+            final boolean invalid) {
+        group.add(new CertificateInformationEntry(title, fields.getOrDefault(field, NOTPRESENT), invalid,
                 !fields.containsKey(field)));
     }
 
@@ -212,22 +218,22 @@ public class SSLCertificateDialogModel {
     public List<CertificateSummaryEntry> getSummary() {
         final List<CertificateSummaryEntry> res = new ArrayList<>();
 
-        boolean outofdate = false;
-        boolean wronghost = false;
-        boolean nottrusted = false;
+        boolean outOfDate = false;
+        boolean wrongHost = false;
+        boolean notTrusted = false;
 
         for (CertificateException ex : problems) {
             if (ex instanceof CertificateExpiredException
                     || ex instanceof CertificateNotYetValidException) {
-                outofdate = true;
+                outOfDate = true;
             } else if (ex instanceof CertificateDoesntMatchHostException) {
-                wronghost = true;
+                wrongHost = true;
             } else if (ex instanceof CertificateNotTrustedException) {
-                nottrusted = true;
+                notTrusted = true;
             }
         }
 
-        if (outofdate) {
+        if (outOfDate) {
             res.add(new CertificateSummaryEntry("One or more certificates are "
                     + "not within their validity period", false));
         } else {
@@ -235,7 +241,7 @@ public class SSLCertificateDialogModel {
                     + "within their validity period", true));
         }
 
-        if (nottrusted) {
+        if (notTrusted) {
             res.add(new CertificateSummaryEntry("The certificate is not issued "
                     + "by a trusted authority", false));
         } else {
@@ -243,7 +249,7 @@ public class SSLCertificateDialogModel {
                     + "trusted", true));
         }
 
-        if (wronghost) {
+        if (wrongHost) {
             res.add(new CertificateSummaryEntry("The certificate is not issued "
                     + "to the host you are connecting to", false));
         } else {