[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#695234: unblock: commons-httpclient/3.1-10.1



Hi again,

the same as for axis (it is actually also the same patch) applied
here.  Debdiff against version in testing attached as well.

Kind regards

        Andreas.

On Thu, Dec 06, 2012 at 09:25:00PM +0000, Adam D. Barratt wrote:
> Control: tags -1 + moreinfo
> 
> On Wed, 2012-12-05 at 23:21 +0100, Andreas Tille wrote:
> > Please unblock package commons-httpclient
> > 
> > Thanks to Alberto Fernández Martínez <infjaf@gmail.com>
> > CVE-2012-5783 (= bug #692442) was closed and I just uploaded
> > his NMU.  Debdiff is attached to the bug report.
> 
> Similarly to axis, it looks like the patch isn't quite ready yet. Please
> let us know (and -moreinfo the bug) once that's resolved in unstable.
> 
> Regards,
> 
> Adam
> 
> 

-- 
http://fam-tille.de
diff -Nru commons-httpclient-3.1/debian/changelog commons-httpclient-3.1/debian/changelog
--- commons-httpclient-3.1/debian/changelog	2011-08-30 19:13:26.000000000 +0200
+++ commons-httpclient-3.1/debian/changelog	2012-12-06 14:41:48.000000000 +0100
@@ -1,3 +1,19 @@
+commons-httpclient (3.1-10.2) unstable; urgency=low
+
+  * Non-maintainer upload.
+  * Fix CVE-2012-5783 (Closes: #692442)
+  * Fix CN extraction from DN of X500 principal.
+  * Fix wildcard validation on ssl connections
+
+ -- Alberto Fernández Martínez <infjaf@gmail.com>  Thu, 6 Dec 2012 14:28:00 +0100
+
+commons-httpclient (3.1-10.1) unstable; urgency=low
+
+  * Non-maintainer upload.
+  * Fix CVE-2012-5783 (Closes: #692442)
+
+ -- Alberto Fernández Martínez <infjaf@gmail.com>  Wed, 5 Dec 2012 17:28:00 +0100
+
 commons-httpclient (3.1-10) unstable; urgency=low
 
   [ Damien Raude-Morvan ]
diff -Nru commons-httpclient-3.1/debian/patches/06_fix_CVE-2012-5783.patch commons-httpclient-3.1/debian/patches/06_fix_CVE-2012-5783.patch
--- commons-httpclient-3.1/debian/patches/06_fix_CVE-2012-5783.patch	1970-01-01 01:00:00.000000000 +0100
+++ commons-httpclient-3.1/debian/patches/06_fix_CVE-2012-5783.patch	2012-12-06 14:37:04.000000000 +0100
@@ -0,0 +1,383 @@
+Description: Fixed CN extraction from DN of X500 principal and wildcard validation
+
+ commons-httpclient (3.1-10.2) unstable; urgency=low
+
+   * Fixed CN extraction from DN of X500 principal and wildcard validation
+
+
+Author: Alberto Fernández Martínez <infjaf@gmail.com>
+
+
+Origin: other
+Bug-Debian: http://bugs.debian.org/692442
+Forwarded: https://issues.apache.org/jira/browse/HTTPCLIENT-1265
+Last-Update: <2012-12-06>
+
+--- 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,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 @@ 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 +99,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 +146,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 +167,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,13 +184,271 @@ 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(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@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.
diff -Nru commons-httpclient-3.1/debian/patches/series commons-httpclient-3.1/debian/patches/series
--- commons-httpclient-3.1/debian/patches/series	2011-08-30 11:42:03.000000000 +0200
+++ commons-httpclient-3.1/debian/patches/series	2012-12-05 17:34:20.000000000 +0100
@@ -4,3 +4,4 @@
 03_upstream_qualify_ConnectionPool_declaration.patch
 04_fix_classpath.patch
 05_osgi_metadata
+06_fix_CVE-2012-5783.patch

Reply to: