[jakarta-commons-httpclient/f16: 7/7] Add missing connection hostname check against X.509 certificate name
Mikolaj Izdebski
mizdebsk at fedoraproject.org
Mon Jan 21 04:45:25 UTC 2013
commit 6ca5676c7d4390c7c6530484030815d4db6f0165
Author: Mikolaj Izdebski <mizdebsk at redhat.com>
Date: Mon Jan 21 05:26:48 2013 +0100
Add missing connection hostname check against X.509 certificate name
- Resolves: CVE-2012-5783
jakarta-commons-httpclient-CVE-2012-5783.patch | 373 ++++++++++++++++++++++++
jakarta-commons-httpclient.spec | 12 +-
2 files changed, 383 insertions(+), 2 deletions(-)
---
diff --git a/jakarta-commons-httpclient-CVE-2012-5783.patch b/jakarta-commons-httpclient-CVE-2012-5783.patch
new file mode 100644
index 0000000..9fe040b
--- /dev/null
+++ b/jakarta-commons-httpclient-CVE-2012-5783.patch
@@ -0,0 +1,373 @@
+Index: oac.hc3x/trunk/src/java/org/apache/commons/httpclient/protocol/SSLProtocolSocketFactory.java
+===================================================================
+diff -u -N -r608014 -r1422573
+--- oac.hc3x/trunk/src/java/org/apache/commons/httpclient/protocol/SSLProtocolSocketFactory.java (.../SSLProtocolSocketFactory.java) (revision 608014)
++++ oac.hc3x/trunk/src/java/org/apache/commons/httpclient/protocol/SSLProtocolSocketFactory.java (.../SSLProtocolSocketFactory.java) (revision 1422573)
+@@ -31,10 +31,25 @@
+ 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 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 java.util.Locale;
++import java.util.StringTokenizer;
++import java.util.regex.Pattern;
+
++import javax.net.ssl.SSLException;
++import javax.net.ssl.SSLSession;
++import javax.net.ssl.SSLSocket;
+ import javax.net.ssl.SSLSocketFactory;
+
+ import org.apache.commons.httpclient.ConnectTimeoutException;
+@@ -55,6 +70,11 @@
+ */
+ 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 +99,14 @@
+ 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 +146,19 @@
+ }
+ 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 +167,12 @@
+ */
+ 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,15 +184,273 @@
+ 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(Locale.US), (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(Locale.US), 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) && !isIPAddress(host)) {
++ 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
++ String parts[] = cn.split("\\.");
++ return parts.length >= 3 &&
++ parts[0].endsWith("*") &&
++ acceptableCountryWildcard(cn) &&
++ !isIPAddress(cn);
++ }
++
++
++ private static final Pattern IPV4_PATTERN =
++ Pattern.compile("^(25[0-5]|2[0-4]\\d|[0-1]?\\d?\\d)(\\.(25[0-5]|2[0-4]\\d|[0-1]?\\d?\\d)){3}$");
++
++ private static final Pattern IPV6_STD_PATTERN =
++ Pattern.compile("^(?:[0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}$");
++
++ private static final Pattern IPV6_HEX_COMPRESSED_PATTERN =
++ Pattern.compile("^((?:[0-9A-Fa-f]{1,4}(?::[0-9A-Fa-f]{1,4})*)?)::((?:[0-9A-Fa-f]{1,4}(?::[0-9A-Fa-f]{1,4})*)?)$");
++
++
++ private static boolean isIPAddress(final String hostname) {
++ return hostname != null
++ && (
++ IPV4_PATTERN.matcher(hostname).matches()
++ || IPV6_STD_PATTERN.matcher(hostname).matches()
++ || IPV6_HEX_COMPRESSED_PATTERN.matcher(hostname).matches()
++ );
++
++ }
++
++ 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;
++ String firstpart = parts[0];
++ if (firstpart.length() > 1) {
++ // server∗
++ // e.g. server
++ String prefix = firstpart.substring(0, firstpart.length() - 1);
++ // skipwildcard part from cn
++ String suffix = cn.substring(firstpart.length());
++ // skip wildcard part from host
++ String hostSuffix = hostName.substring(prefix.length());
++ match = hostName.startsWith(prefix) && hostSuffix.endsWith(suffix);
++ } else {
++ match = hostName.endsWith(cn.substring(1));
++ }
++ if (match) {
++ // I f we're in strict 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=juliusdavies at cucbc.com
++ String subjectPrincipal = cert.getSubjectX500Principal().toString();
++
++ return getCN(subjectPrincipal);
++
++ }
++ private static String getCN(String subjectPrincipal) {
++ StringTokenizer st = new StringTokenizer(subjectPrincipal, ",");
++ while(st.hasMoreTokens()) {
++ String tok = st.nextToken().trim();
++ if (tok.length() > 3) {
++ if (tok.substring(0, 3).equalsIgnoreCase("CN=")) {
++ return tok.substring(3);
++ }
++ }
++ }
++ return null;
++ }
++
++ /**
+ * All instances of SSLProtocolSocketFactory are the same.
+ */
+ public boolean equals(Object obj) {
diff --git a/jakarta-commons-httpclient.spec b/jakarta-commons-httpclient.spec
index f6a127a..e9eb703 100644
--- a/jakarta-commons-httpclient.spec
+++ b/jakarta-commons-httpclient.spec
@@ -32,7 +32,7 @@
Name: jakarta-commons-httpclient
Version: 3.1
-Release: 11%{?dist}
+Release: 12%{?dist}
Epoch: 1
Summary: Jakarta Commons HTTPClient implements the client side of HTTP standards
License: ASL 2.0 and (ASL 2.0 or LGPLv2+)
@@ -42,6 +42,9 @@ Patch0: %{name}-disablecryptotests.patch
# Add OSGi MANIFEST.MF bits
Patch1: %{name}-addosgimanifest.patch
Patch2: %{name}-encoding.patch
+# CVE-2012-5783: missing connection hostname check against X.509 certificate name
+# https://fisheye6.atlassian.com/changelog/httpcomponents?cs=1422573
+Patch3: %{name}-CVE-2012-5783.patch
URL: http://jakarta.apache.org/commons/httpclient/
Group: Development/Libraries
BuildArch: noarch
@@ -120,6 +123,7 @@ pushd src/conf
popd
%patch2
+%patch3 -p2
# Use javax classes, not com.sun ones
# assume no filename contains spaces
@@ -168,7 +172,7 @@ ln -s %{_javadocdir}/%{name}-%{version} dist/docs/apidocs
# maven POM and depmap
install -d -m 755 $RPM_BUILD_ROOT%{_mavenpomdir}
install -p -m 644 %{SOURCE1} $RPM_BUILD_ROOT%{_mavenpomdir}/JPP-%{name}.pom
-%add_maven_depmap
+%add_maven_depmap JPP-%{name}.pom %{name}.jar
%files
%doc LICENSE NOTICE
@@ -191,6 +195,10 @@ install -p -m 644 %{SOURCE1} $RPM_BUILD_ROOT%{_mavenpomdir}/JPP-%{name}.pom
%changelog
+* Mon Jan 21 2013 Mikolaj Izdebski <mizdebsk at redhat.com> - 1:3.1-12
+- Add missing connection hostname check against X.509 certificate name
+- Resolves: CVE-2012-5783
+
* Thu Nov 1 2012 Mikolaj Izdebski <mizdebsk at redhat.com> - 1:3.1-11
- Add maven POM
More information about the scm-commits
mailing list