From d2400e07581134d5c5988a10ff5fab270537eec3 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Tue, 19 Aug 2014 14:09:30 +0000 Subject: [PATCH] Improved compliance with RFC 2818: default hostname verifier to ignore the common name of the certificate subject if alternative subject names (dNSName or iPAddress) are present git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@1618867 13f79535-47bb-0310-9956-ffa450edef68 --- RELEASE_NOTES.txt | 6 ++- .../http/conn/ssl/AbstractVerifier.java | 37 +++++++---------- .../conn/ssl/DefaultHostnameVerifier.java | 12 ++++-- .../http/conn/ssl/TestHostnameVerifier.java | 40 ++++++++----------- 4 files changed, 43 insertions(+), 52 deletions(-) diff --git a/RELEASE_NOTES.txt b/RELEASE_NOTES.txt index f77271ff8..3565e3d85 100644 --- a/RELEASE_NOTES.txt +++ b/RELEASE_NOTES.txt @@ -1,6 +1,10 @@ -Changes isnce 4.4 ALPHA1 +Changes since 4.4 ALPHA1 ------------------- +* Improved compliance with RFC 2818: default hostname verifier to ignore the common name of the + certificate subject if alternative subject names (dNSName or iPAddress) are present. + Contributed by Oleg Kalnichevski + * [HTTPCLIENT-1540] Support delegated credentials (ISC_REQ_DELEGATE) by Native windows Negotiate/NTLM auth schemes. Contributed by Ka-Lok Fung diff --git a/httpclient/src/main/java/org/apache/http/conn/ssl/AbstractVerifier.java b/httpclient/src/main/java/org/apache/http/conn/ssl/AbstractVerifier.java index 37ba0f57e..59526d6b2 100644 --- a/httpclient/src/main/java/org/apache/http/conn/ssl/AbstractVerifier.java +++ b/httpclient/src/main/java/org/apache/http/conn/ssl/AbstractVerifier.java @@ -140,20 +140,10 @@ public abstract class AbstractVerifier implements X509HostnameVerifier { final String cn = cns != null && cns.length > 0 ? cns[0] : null; final List subjectAltList = subjectAlts != null && subjectAlts.length > 0 ? Arrays.asList(subjectAlts) : null; - if (cn == null && subjectAltList == null) { - final String msg = "Certificate subject for <" + host + "> doesn't contain a common name " + - "and does not have alternative names"; - throw new SSLException(msg); - } + final String normalizedHost = InetAddressUtils.isIPv6Address(host) ? DefaultHostnameVerifier.normaliseAddress(host.toLowerCase(Locale.ROOT)) : host; - if (cn != null) { - final String normalizedCN = InetAddressUtils.isIPv6Address(cn) ? - DefaultHostnameVerifier.normaliseAddress(cn) : cn; - if (matchIdentity(normalizedHost, normalizedCN, strictWithSubDomains)) { - return; - } - } + if (subjectAltList != null) { for (String subjectAlt: subjectAltList) { final String normalizedAltSubject = InetAddressUtils.isIPv6Address(subjectAlt) ? @@ -162,19 +152,20 @@ public abstract class AbstractVerifier implements X509HostnameVerifier { return; } } - } - final StringBuilder buf = new StringBuilder(); - buf.append("Certificate for <").append(host).append("> doesn't match "); - if (cn != null) { - buf.append("common name of the certificate subject [").append(cn).append("]"); - } - if (subjectAltList != null) { - if (cn != null) { - buf.append(" or "); + throw new SSLException("Certificate for <" + host + "> doesn't match any " + + "of the subject alternative names: " + subjectAltList); + } else if (cn != null) { + final String normalizedCN = InetAddressUtils.isIPv6Address(cn) ? + DefaultHostnameVerifier.normaliseAddress(cn) : cn; + if (matchIdentity(normalizedHost, normalizedCN, strictWithSubDomains)) { + return; } - buf.append(" any of the subject alternative names").append(subjectAltList); + throw new SSLException("Certificate for <" + host + "> doesn't match " + + "common name of the certificate subject: " + cn); + } else { + throw new SSLException("Certificate subject for <" + host + "> doesn't contain " + + "a common name and does not have alternative names"); } - throw new SSLException(buf.toString()); } private static boolean matchIdentity(final String host, final String identity, final boolean strictWithSubDomains) { diff --git a/httpclient/src/main/java/org/apache/http/conn/ssl/DefaultHostnameVerifier.java b/httpclient/src/main/java/org/apache/http/conn/ssl/DefaultHostnameVerifier.java index f3f8af39f..9700f5466 100644 --- a/httpclient/src/main/java/org/apache/http/conn/ssl/DefaultHostnameVerifier.java +++ b/httpclient/src/main/java/org/apache/http/conn/ssl/DefaultHostnameVerifier.java @@ -123,6 +123,10 @@ public final class DefaultHostnameVerifier implements HostnameVerifier { // as fallback only when no subjectAlts are available final X500Principal subjectPrincipal = cert.getSubjectX500Principal(); final String cn = extractCN(subjectPrincipal.getName(X500Principal.RFC2253)); + if (cn == null) { + throw new SSLException("Certificate subject for <" + host + "> doesn't contain " + + "a common name and does not have alternative names"); + } matchCN(host, cn); } } @@ -135,7 +139,7 @@ public final class DefaultHostnameVerifier implements HostnameVerifier { } } throw new SSLException("Certificate for <" + host + "> doesn't match any " + - "of the subject alternative names " + subjectAlts); + "of the subject alternative names: " + subjectAlts); } static void matchIPv6Address(final String host, final List subjectAlts) throws SSLException { @@ -148,7 +152,7 @@ public final class DefaultHostnameVerifier implements HostnameVerifier { } } throw new SSLException("Certificate for <" + host + "> doesn't match any " + - "of the subject alternative names " + subjectAlts); + "of the subject alternative names: " + subjectAlts); } static void matchDNSName(final String host, final List subjectAlts) throws SSLException { @@ -159,13 +163,13 @@ public final class DefaultHostnameVerifier implements HostnameVerifier { } } throw new SSLException("Certificate for <" + host + "> doesn't match any " + - "of the subject alternative names " + subjectAlts); + "of the subject alternative names: " + subjectAlts); } static void matchCN(final String host, final String cn) throws SSLException { if (!matchIdentity(host, cn)) { throw new SSLException("Certificate for <" + host + "> doesn't match " + - "common name of the certificate subject [" + cn + "]"); + "common name of the certificate subject: " + cn); } } diff --git a/httpclient/src/test/java/org/apache/http/conn/ssl/TestHostnameVerifier.java b/httpclient/src/test/java/org/apache/http/conn/ssl/TestHostnameVerifier.java index 67388b5d8..a346a6eae 100644 --- a/httpclient/src/test/java/org/apache/http/conn/ssl/TestHostnameVerifier.java +++ b/httpclient/src/test/java/org/apache/http/conn/ssl/TestHostnameVerifier.java @@ -74,8 +74,8 @@ public class TestHostnameVerifier { in = new ByteArrayInputStream(CertificatesToPlayWith.X509_FOO_BAR); x509 = (X509Certificate) cf.generateCertificate(in); - DEFAULT.verify("foo.com", x509); - STRICT.verify("foo.com", x509); + exceptionPlease(DEFAULT, "foo.com", x509); + exceptionPlease(STRICT, "foo.com", x509); exceptionPlease(DEFAULT, "a.foo.com", x509); exceptionPlease(STRICT, "a.foo.com", x509); DEFAULT.verify("bar.com", x509); @@ -85,8 +85,8 @@ public class TestHostnameVerifier { in = new ByteArrayInputStream(CertificatesToPlayWith.X509_FOO_BAR_HANAKO); x509 = (X509Certificate) cf.generateCertificate(in); - DEFAULT.verify("foo.com", x509); - STRICT.verify("foo.com", x509); + exceptionPlease(DEFAULT, "foo.com", x509); + exceptionPlease(STRICT, "foo.com", x509); exceptionPlease(DEFAULT, "a.foo.com", x509); exceptionPlease(STRICT, "a.foo.com", x509); DEFAULT.verify("bar.com", x509); @@ -159,11 +159,11 @@ public class TestHostnameVerifier { // try the foo.com variations exceptionPlease(DEFAULT, "foo.com", x509); exceptionPlease(STRICT, "foo.com", x509); - DEFAULT.verify("www.foo.com", x509); - STRICT.verify("www.foo.com", x509); - DEFAULT.verify("\u82b1\u5b50.foo.com", x509); - STRICT.verify("\u82b1\u5b50.foo.com", x509); - DEFAULT.verify("a.b.foo.com", x509); + exceptionPlease(DEFAULT, "www.foo.com", x509); + exceptionPlease(STRICT, "www.foo.com", x509); + exceptionPlease(DEFAULT, "\u82b1\u5b50.foo.com", x509); + exceptionPlease(STRICT, "\u82b1\u5b50.foo.com", x509); + exceptionPlease(DEFAULT, "a.b.foo.com", x509); exceptionPlease(STRICT, "a.b.foo.com", x509); // try the bar.com variations exceptionPlease(DEFAULT, "bar.com", x509); @@ -174,19 +174,6 @@ public class TestHostnameVerifier { STRICT.verify("\u82b1\u5b50.bar.com", x509); DEFAULT.verify("a.b.bar.com", x509); exceptionPlease(STRICT, "a.b.bar.com", x509); - // try the \u82b1\u5b50.co.jp variations - /* - Java isn't extracting international subjectAlts properly. (Or - OpenSSL isn't storing them properly). - */ - //exceptionPlease( DEFAULT, "\u82b1\u5b50.co.jp", x509 ); - //exceptionPlease( STRICT, "\u82b1\u5b50.co.jp", x509 ); - //DEFAULT.verify("www.\u82b1\u5b50.co.jp", x509 ); - //STRICT.verify("www.\u82b1\u5b50.co.jp", x509 ); - //DEFAULT.verify("\u82b1\u5b50.\u82b1\u5b50.co.jp", x509 ); - //STRICT.verify("\u82b1\u5b50.\u82b1\u5b50.co.jp", x509 ); - //DEFAULT.verify("a.b.\u82b1\u5b50.co.jp", x509 ); - //exceptionPlease(STRICT,"a.b.\u82b1\u5b50.co.jp", x509 ); in = new ByteArrayInputStream(CertificatesToPlayWith.X509_MULTIPLE_VALUE_AVA); x509 = (X509Certificate) cf.generateCertificate(in); @@ -206,10 +193,15 @@ public class TestHostnameVerifier { Assert.assertEquals("CN=localhost, OU=Unknown, O=Unknown, L=Unknown, ST=Unknown, C=CH", x509.getSubjectDN().getName()); - verifier.verify("localhost", x509); verifier.verify("localhost.localdomain", x509); verifier.verify("127.0.0.1", x509); + try { + verifier.verify("localhost", x509); + Assert.fail("SSLException should have been thrown"); + } catch (final SSLException ex) { + // expected + } try { verifier.verify("local.host", x509); Assert.fail("SSLException should have been thrown"); @@ -296,7 +288,7 @@ public class TestHostnameVerifier { checkMatching(bhv, "s.a.gov.com", cns, alt, false); // OK, gov not 2TLD here checkMatching(shv, "s.a.gov.com", cns, alt, true); // no subdomain allowed - cns = new String []{"a*.gov.uk"}; // 2TLD check applies to wildcards + alt = new String []{"a*.gov.uk"}; // 2TLD check applies to wildcards checkMatching(bhv, "a.gov.uk", cns, alt, false); // OK checkMatching(shv, "a.gov.uk", cns, alt, true); // Bad 2TLD