You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2014/07/28 17:57:36 UTC

svn commit: r1614065 - in /httpcomponents/httpclient/trunk/httpclient/src: main/java/org/apache/http/conn/ssl/ test/java/org/apache/http/conn/ssl/

Author: olegk
Date: Mon Jul 28 15:57:36 2014
New Revision: 1614065

URL: http://svn.apache.org/r1614065
Log:
Improved cert subject parsing

Modified:
    httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/conn/ssl/AbstractCommonHostnameVerifier.java
    httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/conn/ssl/AbstractVerifier.java
    httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/conn/ssl/TestHostnameVerifier.java

Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/conn/ssl/AbstractCommonHostnameVerifier.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/conn/ssl/AbstractCommonHostnameVerifier.java?rev=1614065&r1=1614064&r2=1614065&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/conn/ssl/AbstractCommonHostnameVerifier.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/conn/ssl/AbstractCommonHostnameVerifier.java Mon Jul 28 15:57:36 2014
@@ -31,14 +31,20 @@ import java.net.InetAddress;
 import java.net.UnknownHostException;
 import java.security.cert.CertificateParsingException;
 import java.security.cert.X509Certificate;
+import java.util.ArrayList;
 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 javax.naming.InvalidNameException;
+import javax.naming.NamingException;
+import javax.naming.directory.Attribute;
+import javax.naming.directory.Attributes;
+import javax.naming.ldap.LdapName;
+import javax.naming.ldap.Rdn;
 import javax.net.ssl.SSLException;
 
 import org.apache.commons.logging.Log;
@@ -83,7 +89,8 @@ public abstract class AbstractCommonHost
     @Override
     public final void verify(final String host, final X509Certificate cert)
           throws SSLException {
-        final String[] cns = getCNs(cert);
+        final String subjectPrincipal = cert.getSubjectX500Principal().toString();
+        final String[] cns = extractCNs(subjectPrincipal);
         final String[] subjectAlts = getSubjectAlts(cert, host);
         verify(host, cns, subjectAlts);
     }
@@ -188,48 +195,32 @@ public abstract class AbstractCommonHost
         return Arrays.binarySearch(BAD_COUNTRY_2LDS, parts[1]) < 0;
     }
 
-    public static String[] getCNs(final X509Certificate cert) {
-        final LinkedList<String> cnList = new LinkedList<String>();
-        /*
-          Sebastian Hauer's original StrictSSLProtocolSocketFactory used
-          getName() and had the following comment:
-
-                Parses a X.500 distinguished name for the value of the
-                "Common Name" field.  This is done a bit sloppy right
-                 now and should probably be done a bit more according to
-                <code>RFC 2253</code>.
-
-           I've noticed that toString() seems to do a better job than
-           getName() on these X500Principal objects, so I'm hoping that
-           addresses Sebastian's concern.
-
-           For example, getName() gives me this:
-           1.2.840.113549.1.9.1=#16166a756c6975736461766965734063756362632e636f6d
-
-           whereas toString() gives me this:
-           EMAILADDRESS=juliusdavies@cucbc.com
-
-           Looks like toString() even works with non-ascii domain names!
-           I tested it with "&#x82b1;&#x5b50;.co.jp" and it worked fine.
-        */
-
-        final String subjectPrincipal = cert.getSubjectX500Principal().toString();
-        final StringTokenizer st = new StringTokenizer(subjectPrincipal, ",+");
-        while(st.hasMoreTokens()) {
-            final String tok = st.nextToken().trim();
-            if (tok.length() > 3) {
-                if (tok.substring(0, 3).equalsIgnoreCase("CN=")) {
-                    cnList.add(tok.substring(3));
+    static String[] extractCNs(final String subjectPrincipal) throws SSLException {
+        if (subjectPrincipal == null) {
+            return null;
+        }
+        final List<String> cns = new ArrayList<String>();
+        try {
+            final LdapName subjectDN = new LdapName(subjectPrincipal);
+            final List<Rdn> rdns = subjectDN.getRdns();
+            for (int i = rdns.size() - 1; i >= 0; i--) {
+                final Rdn rds = rdns.get(i);
+                final Attributes attributes = rds.toAttributes();
+                final Attribute cn = attributes.get("cn");
+                if (cn != null) {
+                    try {
+                        final Object value = cn.get();
+                        if (value != null) {
+                            cns.add(value.toString());
+                        }
+                    } catch (NamingException ignore) {
+                    }
                 }
             }
+        } catch (InvalidNameException e) {
+            throw new SSLException(subjectPrincipal + " is not a valid X500 distinguished name");
         }
-        if(!cnList.isEmpty()) {
-            final String[] cns = new String[cnList.size()];
-            cnList.toArray(cns);
-            return cns;
-        } else {
-            return null;
-        }
+        return cns.isEmpty() ? null : cns.toArray(new String[ cns.size() ]);
     }
 
     /**

Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/conn/ssl/AbstractVerifier.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/conn/ssl/AbstractVerifier.java?rev=1614065&r1=1614064&r2=1614065&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/conn/ssl/AbstractVerifier.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/conn/ssl/AbstractVerifier.java Mon Jul 28 15:57:36 2014
@@ -27,6 +27,10 @@
 
 package org.apache.http.conn.ssl;
 
+import java.security.cert.X509Certificate;
+
+import javax.net.ssl.SSLException;
+
 /**
  * Abstract base class for all standard {@link X509HostnameVerifier}
  * implementations.
@@ -39,4 +43,13 @@ package org.apache.http.conn.ssl;
 @Deprecated
 public abstract class AbstractVerifier extends AbstractCommonHostnameVerifier {
 
+    public static String[] getCNs(final X509Certificate cert) {
+        final String subjectPrincipal = cert.getSubjectX500Principal().toString();
+        try {
+            return extractCNs(subjectPrincipal);
+        } catch (SSLException ex) {
+            return null;
+        }
+    }
+
 }

Modified: httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/conn/ssl/TestHostnameVerifier.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/conn/ssl/TestHostnameVerifier.java?rev=1614065&r1=1614064&r2=1614065&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/conn/ssl/TestHostnameVerifier.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/conn/ssl/TestHostnameVerifier.java Mon Jul 28 15:57:36 2014
@@ -29,7 +29,6 @@ package org.apache.http.conn.ssl;
 
 import java.io.ByteArrayInputStream;
 import java.io.InputStream;
-import java.security.Principal;
 import java.security.cert.CertificateFactory;
 import java.security.cert.X509Certificate;
 import java.util.Arrays;
@@ -38,7 +37,6 @@ import javax.net.ssl.SSLException;
 
 import org.junit.Assert;
 import org.junit.Test;
-import org.mockito.Mockito;
 
 /**
  * Unit tests for {@link X509HostnameVerifier}.
@@ -349,16 +347,27 @@ public class TestHostnameVerifier {
         checkMatching(shv, "mail.a.b.c.com", cns, alt, false); // OK
     }
 
-    public void testGetCNs() {
-        final Principal principal = Mockito.mock(Principal.class);
-        final X509Certificate cert = Mockito.mock(X509Certificate.class);
-        Mockito.when(cert.getSubjectDN()).thenReturn(principal);
-        Mockito.when(principal.toString()).thenReturn("bla,  bla, blah");
-        Assert.assertArrayEquals(new String[] {}, AbstractVerifier.getCNs(cert));
-        Mockito.when(principal.toString()).thenReturn("Cn=,  Cn=  , CN, OU=CN=");
-        Assert.assertArrayEquals(new String[] {}, AbstractVerifier.getCNs(cert));
-        Mockito.when(principal.toString()).thenReturn("  Cn=blah,  CN= blah , OU=CN=yada");
-        Assert.assertArrayEquals(new String[] {"blah", " blah"}, AbstractVerifier.getCNs(cert));
+    @Test
+    public void testExtractCN() throws Exception {
+        Assert.assertArrayEquals(new String[] {"blah"}, AbstractCommonHostnameVerifier.extractCNs("cn=blah, ou=blah, o=blah"));
+        Assert.assertArrayEquals(new String[] {"blah", "yada", "booh"}, AbstractCommonHostnameVerifier.extractCNs("cn=blah, cn=yada, cn=booh"));
+        Assert.assertArrayEquals(new String[] {"blah"}, AbstractCommonHostnameVerifier.extractCNs("c = pampa ,  cn  =    blah    , ou = blah , o = blah"));
+        Assert.assertArrayEquals(new String[] {"blah"}, AbstractCommonHostnameVerifier.extractCNs("cn=\"blah\", ou=blah, o=blah"));
+        Assert.assertArrayEquals(new String[] {"blah  blah"}, AbstractCommonHostnameVerifier.extractCNs("cn=\"blah  blah\", ou=blah, o=blah"));
+        Assert.assertArrayEquals(new String[] {"blah, blah"}, AbstractCommonHostnameVerifier.extractCNs("cn=\"blah, blah\", ou=blah, o=blah"));
+        Assert.assertArrayEquals(new String[] {"blah, blah"}, AbstractCommonHostnameVerifier.extractCNs("cn=blah\\, blah, ou=blah, o=blah"));
+        Assert.assertArrayEquals(new String[] {"blah"}, AbstractCommonHostnameVerifier.extractCNs("c = cn=uuh, cn=blah, ou=blah, o=blah"));
+        Assert.assertArrayEquals(new String[] {""}, AbstractCommonHostnameVerifier.extractCNs("cn=   , ou=blah, o=blah"));
+    }
+
+    @Test(expected = SSLException.class)
+    public void testExtractCNInvalid1() throws Exception {
+        AbstractCommonHostnameVerifier.extractCNs("blah,blah");
+    }
+
+    @Test(expected = SSLException.class)
+    public void testExtractCNInvalid2() throws Exception {
+        AbstractCommonHostnameVerifier.extractCNs("cn,o=blah");
     }
 
 }