Hi I've backported the routine to validate certificate name, and I've made a patch (attached).
I'm not sure it's a good idea apply the patch, it can break programs that connect with "bad" hostnames (ips, host in /etc/hostname, etc)
Description: Validates the hostname requested is the same in the certificate in ssl-connections Fixes CVE-2012-5783, validates hostname certificate in SSL connections. Backported from http-client 4, and from Apache Synapse (plus some bugfixes). Author: Alberto Fernandez <inf...@gmail.com> Bug-Debian: http://bugs.debian.org/692442 Forwarded: no --- commons-httpclient-3.1.orig/src/java/org/apache/commons/httpclient/protocol/SSLProtocolSocketFactory.java +++ commons-httpclient-3.1/src/java/org/apache/commons/httpclient/protocol/SSLProtocolSocketFactory.java @@ -31,11 +31,23 @@ package org.apache.commons.httpclient.protocol; import java.io.IOException; +import java.io.InputStream; import java.net.InetAddress; import java.net.Socket; import java.net.UnknownHostException; +import javax.net.ssl.SSLException; +import javax.net.ssl.SSLSession; +import javax.net.ssl.SSLSocket; import javax.net.ssl.SSLSocketFactory; +import java.security.cert.Certificate; +import java.security.cert.CertificateParsingException; +import java.security.cert.X509Certificate; +import java.util.Arrays; +import java.util.Collection; +import java.util.Iterator; +import java.util.LinkedList; +import java.util.List; import org.apache.commons.httpclient.ConnectTimeoutException; import org.apache.commons.httpclient.params.HttpConnectionParams; @@ -55,6 +67,11 @@ public class SSLProtocolSocketFactory im */ private static final SSLProtocolSocketFactory factory = new SSLProtocolSocketFactory(); + // This is a a sorted list, if you insert new elements do it orderdered. + private final static String[] BAD_COUNTRY_2LDS = + {"ac", "co", "com", "ed", "edu", "go", "gouv", "gov", "info", + "lg", "ne", "net", "or", "org"}; + /** * Gets an singleton instance of the SSLProtocolSocketFactory. * @return a SSLProtocolSocketFactory @@ -79,12 +96,14 @@ public class SSLProtocolSocketFactory im InetAddress clientHost, int clientPort) throws IOException, UnknownHostException { - return SSLSocketFactory.getDefault().createSocket( + Socket sslSocket = SSLSocketFactory.getDefault().createSocket( host, port, clientHost, clientPort ); + verifyHostName(host, (SSLSocket) sslSocket); + return sslSocket; } /** @@ -124,16 +143,19 @@ public class SSLProtocolSocketFactory im } int timeout = params.getConnectionTimeout(); if (timeout == 0) { - return createSocket(host, port, localAddress, localPort); + Socket sslSocket = createSocket(host, port, localAddress, localPort); + verifyHostName(host, (SSLSocket) sslSocket); + return sslSocket; } else { // To be eventually deprecated when migrated to Java 1.4 or above - Socket socket = ReflectionSocketFactory.createSocket( + Socket sslSocket = ReflectionSocketFactory.createSocket( "javax.net.ssl.SSLSocketFactory", host, port, localAddress, localPort, timeout); - if (socket == null) { - socket = ControllerThreadSocketFactory.createSocket( + if (sslSocket == null) { + sslSocket = ControllerThreadSocketFactory.createSocket( this, host, port, localAddress, localPort, timeout); } - return socket; + verifyHostName(host, (SSLSocket) sslSocket); + return sslSocket; } } @@ -142,10 +164,12 @@ public class SSLProtocolSocketFactory im */ public Socket createSocket(String host, int port) throws IOException, UnknownHostException { - return SSLSocketFactory.getDefault().createSocket( + Socket sslSocket = SSLSocketFactory.getDefault().createSocket( host, port ); + verifyHostName(host, (SSLSocket) sslSocket); + return sslSocket; } /** @@ -157,14 +181,267 @@ public class SSLProtocolSocketFactory im int port, boolean autoClose) throws IOException, UnknownHostException { - return ((SSLSocketFactory) SSLSocketFactory.getDefault()).createSocket( + Socket sslSocket = ((SSLSocketFactory) SSLSocketFactory.getDefault()).createSocket( socket, host, port, autoClose ); + verifyHostName(host, (SSLSocket) sslSocket); + return sslSocket; + } + + + + + /** + * Verifies that the given hostname in certicifate is the hostname we are trying to connect to + * http://www.cvedetails.com/cve/CVE-2012-5783/ + * @param host + * @param ssl + * @throws IOException + */ + + private static void verifyHostName(String host, SSLSocket ssl) + throws IOException { + if (host == null) { + throw new IllegalArgumentException("host to verify was null"); + } + + SSLSession session = ssl.getSession(); + if (session == null) { + // In our experience this only happens under IBM 1.4.x when + // spurious (unrelated) certificates show up in the server's chain. + // Hopefully this will unearth the real problem: + InputStream in = ssl.getInputStream(); + in.available(); + /* + If you're looking at the 2 lines of code above because you're + running into a problem, you probably have two options: + + #1. Clean up the certificate chain that your server + is presenting (e.g. edit "/etc/apache2/server.crt" or + wherever it is your server's certificate chain is + defined). + + OR + + #2. Upgrade to an IBM 1.5.x or greater JVM, or switch to a + non-IBM JVM. + */ + + // If ssl.getInputStream().available() didn't cause an exception, + // maybe at least now the session is available? + session = ssl.getSession(); + if (session == null) { + // If it's still null, probably a startHandshake() will + // unearth the real problem. + ssl.startHandshake(); + + // Okay, if we still haven't managed to cause an exception, + // might as well go for the NPE. Or maybe we're okay now? + session = ssl.getSession(); + } + } + + Certificate[] certs = session.getPeerCertificates(); + verifyHostName(host.trim().toLowerCase(), (X509Certificate) certs[0]); + } + /** + * Extract the names from the certificate and tests host matches one of them + * @param host + * @param cert + * @throws SSLException + */ + + private static void verifyHostName(final String host, X509Certificate cert) + throws SSLException { + // I'm okay with being case-insensitive when comparing the host we used + // to establish the socket to the hostname in the certificate. + // Don't trim the CN, though. + + String cn = getCN(cert); + String[] subjectAlts = getDNSSubjectAlts(cert); + verifyHostName(host, cn.toLowerCase(), subjectAlts); + + } + + /** + * Extract all alternative names from a certificate. + * @param cert + * @return + */ + private static String[] getDNSSubjectAlts(X509Certificate cert) { + LinkedList subjectAltList = new LinkedList(); + Collection c = null; + try { + c = cert.getSubjectAlternativeNames(); + } catch (CertificateParsingException cpe) { + // Should probably log.debug() this? + cpe.printStackTrace(); + } + if (c != null) { + Iterator it = c.iterator(); + while (it.hasNext()) { + List list = (List) it.next(); + int type = ((Integer) list.get(0)).intValue(); + // If type is 2, then we've got a dNSName + if (type == 2) { + String s = (String) list.get(1); + subjectAltList.add(s); + } + } + } + if (!subjectAltList.isEmpty()) { + String[] subjectAlts = new String[subjectAltList.size()]; + subjectAltList.toArray(subjectAlts); + return subjectAlts; + } else { + return new String[0]; + } + + } + /** + * Verifies + * @param host + * @param cn + * @param subjectAlts + * @throws SSLException + */ + + private static void verifyHostName(final String host, String cn, String[] subjectAlts)throws SSLException{ + StringBuffer cnTested = new StringBuffer(); + + for (int i = 0; i < subjectAlts.length; i++){ + String name = subjectAlts[i]; + if (name != null) { + name = name.toLowerCase(); + if (verifyHostName(host, name)){ + return; + } + cnTested.append("/").append(name); + } + } + if (cn != null && verifyHostName(host, cn)){ + return; + } + cnTested.append("/").append(cn); + throw new SSLException("hostname in certificate didn't match: <" + + host + "> != <" + cnTested + ">"); + + } + + private static boolean verifyHostName(final String host, final String cn){ + if (doWildCard(cn)) { + return matchesWildCard(cn, host); + } + return host.equalsIgnoreCase(cn); + } + private static boolean doWildCard(String cn) { + // Contains a wildcard + // wildcard in the first block + // not an ipaddress (ip addres must explicitily be equal) + // not using 2nd level common tld : ex: not for *.co.uk + return + cn.indexOf("*.")>=0 && + cn.indexOf('.') > cn.indexOf("*.") && + !isIPAddress(cn) && + acceptableCountryWildcard(cn); } + private static boolean isIPAddress(final String cn) { + // IPv6 + if (cn.contains(":")) { + return true; + } + // IPb4 + boolean isIP4 = true; + String tld = cn; + int x = cn.lastIndexOf('.'); + // We only bother analyzing the characters after the final dot + // in the name. + if (x >= 0 && x + 1 < cn.length()) { + tld = cn.substring(x + 1); + } + for (int i = 0; i < tld.length(); i++) { + if (!Character.isDigit(tld.charAt(0))) { + isIP4 = false; + break; + } + } + return isIP4; + } + + private static boolean acceptableCountryWildcard(final String cn) { + // The CN better have at least two dots if it wants wildcard action, + // but can't be [*.co.uk] or [*.co.jp] or [*.org.uk], etc... + // The [*.co.uk] problem is an interesting one. Should we just + // hope that CA's would never foolishly allow such a + // certificate to happen? + + String[] parts = cn.split("\\."); + // Only checks for 3 levels, with country code of 2 letters. + if (parts.length > 3 || parts[parts.length - 1].length() != 2) { + return true; + } + String countryCode = parts[parts.length - 2]; + return Arrays.binarySearch(BAD_COUNTRY_2LDS, countryCode) < 0; + } + + private static boolean matchesWildCard(final String cn, + final String hostName) { + String parts[] = cn.split("\\."); + boolean match = false; + if (parts[0].length() > 1) { + // serverâ + String prefix = parts[0].substring(0, parts[0].length() - 2); + // e.g. server + String suffix = cn.substring(parts[0].length()); + // skipwildcard part from cn + String hostSuffix = hostName.substring(prefix.length()); + // skip wildcard part from host + match = hostName.startsWith(prefix) && hostSuffix.endsWith(suffix); + } else { + match = hostName.endsWith(cn.substring(1)); + } + if (match) { + // I f we â r e i n s t r i c t mode , + // [ â.foo.com] is not allowed to match [a.b.foo.com] + match = countDots(hostName) == countDots(cn); + } + return match; + } + + private static int countDots(final String data) { + int dots = 0; + for (int i = 0; i < data.length(); i++) { + if (data.charAt(i) == '.') { + dots += 1; + } + } + return dots; + } + + private static String getCN(X509Certificate cert) { + // Note: toString() seems to do a better job than getName() + // + // For example, getName() gives me this: + // 1.2.840.113549.1.9.1=#16166a756c6975736461766965734063756362632e636f6d + // + // whereas toString() gives me this: + // EMAILADDRESS=juliusdav...@cucbc.com + String subjectPrincipal = cert.getSubjectX500Principal().toString(); + int x = subjectPrincipal.indexOf("CN="); + if (x >= 0) { + int y = subjectPrincipal.indexOf(',', x); + // If there are no more commas, then CN= is the last entry. + y = (y >= 0) ? y : subjectPrincipal.length(); + return subjectPrincipal.substring(x + 3, y); + } else { + return null; + } + } + /** * All instances of SSLProtocolSocketFactory are the same. */
