HTTPCLIENT-1802: Do not attempt to match SSL host to subject CN if subject alternative name of any type are given

git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/branches/4.5.x@1779668 13f79535-47bb-0310-9956-ffa450edef68

httpclient/src/main/java/org/apache/http/conn/ssl/AbstractVerifier.java

@@ -31,6 +31,7 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.security.cert.Certificate;
 import java.security.cert.X509Certificate;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Locale;
@@ -132,15 +133,21 @@ public abstract class AbstractVerifier implements X509HostnameVerifier {
     @Override
     public final void verify(
             final String host, final X509Certificate cert) throws SSLException {
-        final int subjectType;
+        final List<SubjectName> allSubjectAltNames = DefaultHostnameVerifier.getSubjectAltNames(cert);
-        if (InetAddressUtils.isIPv4Address(host)) {
+        final List<String> subjectAlts = new ArrayList<String>();
-            subjectType = DefaultHostnameVerifier.HostNameType.IPv4.subjectType;
+        if (InetAddressUtils.isIPv4Address(host) || InetAddressUtils.isIPv6Address(host)) {
-        } else if (InetAddressUtils.isIPv6Address(host)) {
+            for (SubjectName subjectName: allSubjectAltNames) {
-            subjectType = DefaultHostnameVerifier.HostNameType.IPv6.subjectType;
+                if (subjectName.getType() == SubjectName.IP) {
-        } else {
+                    subjectAlts.add(subjectName.getValue());
-            subjectType = DefaultHostnameVerifier.HostNameType.DNS.subjectType;
+                }
+            }
+        } else {
+            for (SubjectName subjectName: allSubjectAltNames) {
+                if (subjectName.getType() == SubjectName.DNS) {
+                    subjectAlts.add(subjectName.getValue());
+                }
+            }
         }
-        final List<String> subjectAlts = DefaultHostnameVerifier.extractSubjectAlts(cert, subjectType);
         final X500Principal subjectPrincipal = cert.getSubjectX500Principal();
         final String cn = DefaultHostnameVerifier.extractCN(subjectPrincipal.getName(X500Principal.RFC2253));
         verify(host,
@@ -250,10 +257,17 @@ public abstract class AbstractVerifier implements X509HostnameVerifier {
      * @return Array of SubjectALT DNS names stored in the certificate.
      */
     public static String[] getDNSSubjectAlts(final X509Certificate cert) {
-        final List<String> subjectAlts = DefaultHostnameVerifier.extractSubjectAlts(
+        final List<SubjectName> subjectAltNames = DefaultHostnameVerifier.getSubjectAltNames(cert);
-                cert, DefaultHostnameVerifier.HostNameType.DNS.subjectType);
+        if (subjectAltNames == null) {
-        return subjectAlts != null && !subjectAlts.isEmpty() ?
+            return null;
-                subjectAlts.toArray(new String[subjectAlts.size()]) : null;
+        }
+        final List<String> dnsAlts = new ArrayList<String>();
+        for (SubjectName subjectName: subjectAltNames) {
+            if (subjectName.getType() == SubjectName.DNS) {
+                dnsAlts.add(subjectName.getValue());
+            }
+        }
+        return dnsAlts.isEmpty() ? dnsAlts.toArray(new String[dnsAlts.size()]) : null;
     }
 
     /**

httpclient/src/main/java/org/apache/http/conn/ssl/DefaultHostnameVerifier.java

@@ -34,6 +34,7 @@ import java.security.cert.CertificateParsingException;
 import java.security.cert.X509Certificate;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 import java.util.Locale;
 import java.util.NoSuchElementException;
@@ -108,7 +109,7 @@ public final class DefaultHostnameVerifier implements HostnameVerifier {
     public void verify(
             final String host, final X509Certificate cert) throws SSLException {
         final HostNameType hostType = determineHostFormat(host);
-        final List<String> subjectAlts = extractSubjectAlts(cert, hostType.subjectType);
+        final List<SubjectName> subjectAlts = getSubjectAltNames(cert);
         if (subjectAlts != null && !subjectAlts.isEmpty()) {
             switch (hostType) {
                 case IPv4:
@@ -133,40 +134,46 @@ public final class DefaultHostnameVerifier implements HostnameVerifier {
         }
     }
 
-    static void matchIPAddress(final String host, final List<String> subjectAlts) throws SSLException {
+    static void matchIPAddress(final String host, final List<SubjectName> subjectAlts) throws SSLException {
         for (int i = 0; i < subjectAlts.size(); i++) {
-            final String subjectAlt = subjectAlts.get(i);
+            final SubjectName subjectAlt = subjectAlts.get(i);
-            if (host.equals(subjectAlt)) {
+            if (subjectAlt.getType() == SubjectName.IP) {
+                if (host.equals(subjectAlt.getValue())) {
                     return;
                 }
             }
+        }
         throw new SSLPeerUnverifiedException("Certificate for <" + host + "> doesn't match any " +
                 "of the subject alternative names: " + subjectAlts);
     }
 
-    static void matchIPv6Address(final String host, final List<String> subjectAlts) throws SSLException {
+    static void matchIPv6Address(final String host, final List<SubjectName> subjectAlts) throws SSLException {
         final String normalisedHost = normaliseAddress(host);
         for (int i = 0; i < subjectAlts.size(); i++) {
-            final String subjectAlt = subjectAlts.get(i);
+            final SubjectName subjectAlt = subjectAlts.get(i);
-            final String normalizedSubjectAlt = normaliseAddress(subjectAlt);
+            if (subjectAlt.getType() == SubjectName.IP) {
+                final String normalizedSubjectAlt = normaliseAddress(subjectAlt.getValue());
                 if (normalisedHost.equals(normalizedSubjectAlt)) {
                     return;
                 }
             }
+        }
         throw new SSLPeerUnverifiedException("Certificate for <" + host + "> doesn't match any " +
                 "of the subject alternative names: " + subjectAlts);
     }
 
-    static void matchDNSName(final String host, final List<String> subjectAlts,
+    static void matchDNSName(final String host, final List<SubjectName> subjectAlts,
                              final PublicSuffixMatcher publicSuffixMatcher) throws SSLException {
         final String normalizedHost = host.toLowerCase(Locale.ROOT);
         for (int i = 0; i < subjectAlts.size(); i++) {
-            final String subjectAlt = subjectAlts.get(i);
+            final SubjectName subjectAlt = subjectAlts.get(i);
-            final String normalizedSubjectAlt = subjectAlt.toLowerCase(Locale.ROOT);
+            if (subjectAlt.getType() == SubjectName.DNS) {
+                final String normalizedSubjectAlt = subjectAlt.getValue().toLowerCase(Locale.ROOT);
                 if (matchIdentityStrict(normalizedHost, normalizedSubjectAlt, publicSuffixMatcher)) {
                     return;
                 }
             }
+        }
         throw new SSLPeerUnverifiedException("Certificate for <" + host + "> doesn't match any " +
                 "of the subject alternative names: " + subjectAlts);
     }
@@ -289,33 +296,24 @@ public final class DefaultHostnameVerifier implements HostnameVerifier {
         return HostNameType.DNS;
     }
 
-    static List<String> extractSubjectAlts(final X509Certificate cert, final int subjectType) {
+    static List<SubjectName> getSubjectAltNames(final X509Certificate cert) {
-        Collection<List<?>> c = null;
         try {
-            c = cert.getSubjectAlternativeNames();
+            final Collection<List<?>> entries = cert.getSubjectAlternativeNames();
+            if (entries == null) {
+                return Collections.emptyList();
+            }
+            final List<SubjectName> result = new ArrayList<SubjectName>();
+            for (List<?> entry: entries) {
+                final Integer type = entry.size() >= 2 ? (Integer) entry.get(0) : null;
+                if (type != null) {
+                    final String s = (String) entry.get(1);
+                    result.add(new SubjectName(s, type));
+                }
+            }
+            return result;
         } catch (final CertificateParsingException ignore) {
-            return null;
+            return Collections.emptyList();
         }
-        List<String> subjectAltList = null;
-        if (c != null) {
-            for (final List<?> aC : c) {
-                final List<?> list = aC;
-                final int type = ((Integer) list.get(0)).intValue();
-                if (type == subjectType) {
-                    final String s = (String) list.get(1);
-                    if (subjectAltList == null) {
-                        subjectAltList = new ArrayList<String>();
-                    }
-                    subjectAltList.add(s);
-                }
-            }
-        }
-        return subjectAltList;
-    }
-
-    static List<String> extractSubjectAlts(final String host, final X509Certificate cert) {
-        final HostNameType hostType = determineHostFormat(host);
-        return extractSubjectAlts(cert, hostType.subjectType);
     }
 
     /*

httpclient/src/main/java/org/apache/http/conn/ssl/SSLConnectionSocketFactory.java

@@ -463,7 +463,7 @@ public class SSLConnectionSocketFactory implements LayeredConnectionSocketFactor
             if (!this.hostnameVerifier.verify(hostname, session)) {
                 final Certificate[] certs = session.getPeerCertificates();
                 final X509Certificate x509 = (X509Certificate) certs[0];
-                final List<String> subjectAlts = DefaultHostnameVerifier.extractSubjectAlts(hostname, x509);
+                final List<SubjectName> subjectAlts = DefaultHostnameVerifier.getSubjectAltNames(x509);
                 throw new SSLPeerUnverifiedException("Certificate for <" + hostname + "> doesn't match any " +
                         "of the subject alternative names: " + subjectAlts);
             }

httpclient/src/main/java/org/apache/http/conn/ssl/SubjectName.java

@@ -0,0 +1,65 @@
+/*
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation.  For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ *
+ */
+package org.apache.http.conn.ssl;
+
+import org.apache.http.util.Args;
+
+final class SubjectName {
+
+    static final int DNS = 2;
+    static final int IP = 7;
+
+    private final String value;
+    private final int type;
+
+    static SubjectName IP(final String value) {
+        return new SubjectName(value, IP);
+    }
+
+    static SubjectName DNS(final String value) {
+        return new SubjectName(value, DNS);
+    }
+
+    SubjectName(final String value, final int type) {
+        this.value = Args.notNull(value, "Value");
+        this.type = Args.positive(type, "Type");
+    }
+
+    public int getType() {
+        return type;
+    }
+
+    public String getValue() {
+        return value;
+    }
+
+    @Override
+    public String toString() {
+        return value;
+    }
+
+}

httpclient/src/test/java/org/apache/http/conn/ssl/CertificatesToPlayWith.java

@@ -509,4 +509,45 @@ public class CertificatesToPlayWith {
         "I1mQmUs44cg2HZAqnFBpDyJQhNYy8M7yGVaRkbfuVaMqiPa+xDPR5v7NFB3kxRq2\n" +
         "Za2Snopi52eUxDEhJ0MNqFi3Jfj/ZSmJ+XHra5lU4R8lijCAq8SVLZCmIQ==\n" +
         "-----END CERTIFICATE-----").getBytes();
+
+    public final static byte[] S_GOOGLE_COM = (
+        "-----BEGIN CERTIFICATE-----\n" +
+        "MIICpzCCAY+gAwIBAgIBATANBgkqhkiG9w0BAQUFADAXMRUwEwYDVQQDDAwqLmdv\n" +
+        "b2dsZS5jb20wHhcNMTcwMTEzMjI0OTAzWhcNMTgwMTEzMjI0OTAzWjAXMRUwEwYD\n" +
+        "VQQDDAwqLmdvb2dsZS5jb20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIB\n" +
+        "AQDHuzznuHdJ5PH344xCyGYnUnIRhyLGBKN3WDLLrXWtr/5Sf3Q1qkiMiJ4BINsh\n" +
+        "3Xy0z7VvHmMFlntgHXtkofBUPvTihxsVIypRkCZb5hpsWLotR10AW2JpVl/oxLP2\n" +
+        "227/36X1zKh33fjImLJl9KzGWHLsbCBleQQJOn7YRsNR/QBZO0XGGkN/R2rRfLF3\n" +
+        "rseRfI5gJjZkO0WDxocnf/iieOe0XNR0NAZaY1aozzPmZ/pRrOKYB8OFH7F73WOC\n" +
+        "lPIUGai/byJ9SpbXdLUcMlGhml/4XzcnV/WVRD2P/mlY+xEFG3UEy3ufhNnKFJul\n" +
+        "yjZrOaKbagamqtOyktzkjnerAgMBAAEwDQYJKoZIhvcNAQEFBQADggEBADaMcwVs\n" +
+        "w5kbnoDJzMBJ01H16T4u8k78i/ybwz7u7krgkU0tABXCRj7S/4Dt3jqQ/rV6evj4\n" +
+        "gIJ/2kZUp/PHKkV7CxWI48XBTAQUu9LEpxj0Hut3AtNMD9y/J6cFn2978tWsHFHI\n" +
+        "mYgvclKUDE4WFMvuxfQVuX3RcGQ5i8khEMczY/KVhZYDcLU1PU0GTTJqqrQm59Z4\n" +
+        "T4UyI3OPBR7Nb/kaU1fcgQ083uxRXcNYRMMZnU6c2oFnR+c6pO6aGoXo0C6rgC4R\n" +
+        "pOj4hPvHCfZO2xg6HAdQ7UPALLX8pu5KGot7GRc8yiJ/Q1nBEuiPKKu0MIwQoFgP\n" +
+        "WUux/APTsgLR7Vc=\n" +
+        "-----END CERTIFICATE-----"
+        ).getBytes();
+
+    public final static byte[] IP_1_1_1_1 = (
+        "-----BEGIN CERTIFICATE-----\n" +
+        "MIICwjCCAaqgAwIBAgIBATANBgkqhkiG9w0BAQUFADAaMRgwFgYDVQQDEw9kdW1t\n" +
+        "eS12YWx1ZS5jb20wHhcNMTcwMTEzMjI1MTQ2WhcNMTgwMTEzMjI1MTQ2WjAaMRgw\n" +
+        "FgYDVQQDEw9kdW1teS12YWx1ZS5jb20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAw\n" +
+        "ggEKAoIBAQDfrapp3jHLp1RlElzpR/4sF9AcTYwMF1N+adkHRoVtmTlJV2lTIAjn\n" +
+        "QLauy0Kkzv8uxmbID3uROgrFNDQ5RxTTCe+kW/vE6Pyzr5Z5ayjSTKeycTE7mAC4\n" +
+        "6ntoCeEWiD593zlfqVo5PuRSp9Kusd+kexNVjC/BETDPa3yXctcH1ouW9GyGItgQ\n" +
+        "u4GhCE8cipKMuTltgfK+Gh/5e9lFG9/F2fD+wHUVBULLR3JOQoqwgk2zAwKDwLuS\n" +
+        "sEd1CBi35+W3apCKN0SEdTKIAxc/R+O/1j2hpOl9yXCCYyveGwJdFXVZtDcx+9/H\n" +
+        "7NXhOdmw/mTXC5fOQGKciEo2SXt8Wp89AgMBAAGjEzARMA8GA1UdEQQIMAaHBAEB\n" +
+        "AQEwDQYJKoZIhvcNAQEFBQADggEBAEAO6CE8twpcfdjk9oMjI5nX9GdC5Wt6+ujd\n" +
+        "tLj0SbXvMKzCLLkveT0xTEzXfyEo8KW2qYYvPP1h83BIxsbR/J3Swt35UQVofv+4\n" +
+        "JgO0FIdgB+iLEcjUh5+60xslylqWE+9bSWm4f06OXuv78tq5NYPZKku/3i4tqLRp\n" +
+        "gH2rTtjX7Q4olSS7GdAgfiA2AnDZAbMtxtsnTt/QFpYQqhlkqHVDwgkGP7C8aMBD\n" +
+        "RH0UIQCPxUkhwhtNmVyHO42r6oHXselZoVU6XRHuhogrGxPf/pzDUvrKBiJhsZQQ\n" +
+        "oEu+pZCwkFLiNwUoq1G2oDpkkdBWB0JcBXB2Txa536ezFFWZYc0=\n" +
+        "-----END CERTIFICATE-----"
+        ).getBytes();
+
 }

httpclient/src/test/java/org/apache/http/conn/ssl/TestDefaultHostnameVerifier.java

@@ -149,6 +149,21 @@ public class TestDefaultHostnameVerifier {
         in = new ByteArrayInputStream(CertificatesToPlayWith.X509_MULTIPLE_VALUE_AVA);
         x509 = (X509Certificate) cf.generateCertificate(in);
         impl.verify("repository.infonotary.com", x509);
+
+        in = new ByteArrayInputStream(CertificatesToPlayWith.S_GOOGLE_COM);
+        x509 = (X509Certificate) cf.generateCertificate(in);
+        impl.verify("*.google.com", x509);
+
+        in = new ByteArrayInputStream(CertificatesToPlayWith.S_GOOGLE_COM);
+        x509 = (X509Certificate) cf.generateCertificate(in);
+        impl.verify("*.Google.com", x509);
+
+        in = new ByteArrayInputStream(CertificatesToPlayWith.IP_1_1_1_1);
+        x509 = (X509Certificate) cf.generateCertificate(in);
+        impl.verify("1.1.1.1", x509);
+
+        exceptionPlease(impl, "1.1.1.2", x509);
+        exceptionPlease(impl, "dummy-value.com", x509);
     }
 
     @Test
@@ -259,18 +274,18 @@ public class TestDefaultHostnameVerifier {
     @Test // Check compressed IPv6 hostname matching
     public void testHTTPCLIENT_1316() throws Exception{
         final String host1 = "2001:0db8:aaaa:bbbb:cccc:0:0:0001";
-        DefaultHostnameVerifier.matchIPv6Address(host1, Arrays.asList("2001:0db8:aaaa:bbbb:cccc:0:0:0001"));
+        DefaultHostnameVerifier.matchIPv6Address(host1, Arrays.asList(SubjectName.IP("2001:0db8:aaaa:bbbb:cccc:0:0:0001")));
-        DefaultHostnameVerifier.matchIPv6Address(host1, Arrays.asList("2001:0db8:aaaa:bbbb:cccc::1"));
+        DefaultHostnameVerifier.matchIPv6Address(host1, Arrays.asList(SubjectName.IP("2001:0db8:aaaa:bbbb:cccc::1")));
         try {
-            DefaultHostnameVerifier.matchIPv6Address(host1, Arrays.asList("2001:0db8:aaaa:bbbb:cccc::10"));
+            DefaultHostnameVerifier.matchIPv6Address(host1, Arrays.asList(SubjectName.IP("2001:0db8:aaaa:bbbb:cccc::10")));
             Assert.fail("SSLException expected");
         } catch (final SSLException expected) {
         }
         final String host2 = "2001:0db8:aaaa:bbbb:cccc::1";
-        DefaultHostnameVerifier.matchIPv6Address(host2, Arrays.asList("2001:0db8:aaaa:bbbb:cccc:0:0:0001"));
+        DefaultHostnameVerifier.matchIPv6Address(host2, Arrays.asList(SubjectName.IP("2001:0db8:aaaa:bbbb:cccc:0:0:0001")));
-        DefaultHostnameVerifier.matchIPv6Address(host2, Arrays.asList("2001:0db8:aaaa:bbbb:cccc::1"));
+        DefaultHostnameVerifier.matchIPv6Address(host2, Arrays.asList(SubjectName.IP("2001:0db8:aaaa:bbbb:cccc::1")));
         try {
-            DefaultHostnameVerifier.matchIPv6Address(host2, Arrays.asList("2001:0db8:aaaa:bbbb:cccc::10"));
+            DefaultHostnameVerifier.matchIPv6Address(host2, Arrays.asList(SubjectName.IP("2001:0db8:aaaa:bbbb:cccc::10")));
             Assert.fail("SSLException expected");
         } catch (final SSLException expected) {
         }