From 95889a81ddc58cba17b7c28536edd1e5037952c9 Mon Sep 17 00:00:00 2001 From: Danny Reidenbach Date: Fri, 24 Jul 2020 22:33:59 +0000 Subject: [PATCH 01/20] Copied in AOSP OKHostnameVerifier and used it in TrustManagerImpl --- .../java/org/conscrypt/TrustManagerImpl.java | 7 +- .../org/conscrypt/OkHostnameVerifier.java | 268 ++++++++++++++++++ 2 files changed, 272 insertions(+), 3 deletions(-) create mode 100644 openjdk/src/main/java/org/conscrypt/OkHostnameVerifier.java diff --git a/common/src/main/java/org/conscrypt/TrustManagerImpl.java b/common/src/main/java/org/conscrypt/TrustManagerImpl.java index bbbd45f0c..2e4b9bc2f 100644 --- a/common/src/main/java/org/conscrypt/TrustManagerImpl.java +++ b/common/src/main/java/org/conscrypt/TrustManagerImpl.java @@ -998,7 +998,7 @@ void setHostnameVerifier(ConscryptHostnameVerifier verifier) { * * @see #setHostnameVerifier(ConscryptHostnameVerifier) */ - ConscryptHostnameVerifier getHostnameVerifier() { + ConscryptHostnameVerifier getHostnameVerifier() { // doggo return hostnameVerifier; } @@ -1011,7 +1011,7 @@ public boolean verify(String hostname, SSLSession session) { } } - private ConscryptHostnameVerifier getHttpsVerifier() { + private ConscryptHostnameVerifier getHttpsVerifier() { // doggo if (hostnameVerifier != null) { return hostnameVerifier; } @@ -1019,7 +1019,8 @@ private ConscryptHostnameVerifier getHttpsVerifier() { if (defaultVerifier != null) { return defaultVerifier; } - return GlobalHostnameVerifierAdapter.INSTANCE; +// return GlobalHostnameVerifierAdapter.INSTANCE; + return OkHostnameVerifier.strictInstance(); } public void setCTEnabledOverride(boolean enabled) { diff --git a/openjdk/src/main/java/org/conscrypt/OkHostnameVerifier.java b/openjdk/src/main/java/org/conscrypt/OkHostnameVerifier.java new file mode 100644 index 000000000..ea277e548 --- /dev/null +++ b/openjdk/src/main/java/org/conscrypt/OkHostnameVerifier.java @@ -0,0 +1,268 @@ +/* + * 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. + */ + +package org.conscrypt; + +import java.security.cert.Certificate; +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.regex.Pattern; +import javax.net.ssl.HostnameVerifier; +import javax.net.ssl.SSLException; +import javax.net.ssl.SSLSession; + +/** + * A HostnameVerifier consistent with RFC 2818. + */ +public final class OkHostnameVerifier implements ConscryptHostnameVerifier { + // Android-changed: Add a mode which disallows top-level domain wildcards. b/144694112 + // public static final OkHostnameVerifier INSTANCE = new OkHostnameVerifier(); + public static final OkHostnameVerifier INSTANCE = new OkHostnameVerifier(true); + + /** + * Quick and dirty pattern to differentiate IP addresses from hostnames. This + * is an approximation of Android's private InetAddress#isNumeric API. + * + *

This matches IPv6 addresses as a hex string containing at least one + * colon, and possibly including dots after the first colon. It matches IPv4 + * addresses as strings containing only decimal digits and dots. This pattern + * matches strings like "a:.23" and "54" that are neither IP addresses nor + * hostnames; they will be verified as IP addresses (which is a more strict + * verification). + */ + private static final Pattern VERIFY_AS_IP_ADDRESS = Pattern.compile( + "([0-9a-fA-F]*:[0-9a-fA-F:.]*)|([\\d.]+)"); + + private static final int ALT_DNS_NAME = 2; + private static final int ALT_IPA_NAME = 7; + + // BEGIN Android-changed: Add a mode which disallows top-level domain wildcards. b/144694112 + // private OkHostnameVerifier() { + // } + private final boolean strictWildcardMode; + + private OkHostnameVerifier(boolean strictWildcardMode) { + this.strictWildcardMode = strictWildcardMode; + } + + public static OkHostnameVerifier strictInstance() { + return new OkHostnameVerifier(true); + } + // END Android-changed: Add a mode which disallows top-level domain wildcards. b/144694112 + + @Override + public boolean verify(String host, SSLSession session) { + try { + Certificate[] certificates = session.getPeerCertificates(); + return verify(host, (X509Certificate) certificates[0]); + } catch (SSLException e) { + return false; + } + } + + public boolean verify(String host, X509Certificate certificate) { + return verifyAsIpAddress(host) + ? verifyIpAddress(host, certificate) + : verifyHostName(host, certificate); + } + + static boolean verifyAsIpAddress(String host) { + return VERIFY_AS_IP_ADDRESS.matcher(host).matches(); + } + + /** + * Returns true if {@code certificate} matches {@code ipAddress}. + */ + private boolean verifyIpAddress(String ipAddress, X509Certificate certificate) { + List altNames = getSubjectAltNames(certificate, ALT_IPA_NAME); + for (int i = 0, size = altNames.size(); i < size; i++) { + if (ipAddress.equalsIgnoreCase(altNames.get(i))) { + return true; + } + } + return false; + } + + /** + * Returns true if {@code certificate} matches {@code hostName}. + */ + private boolean verifyHostName(String hostName, X509Certificate certificate) { + hostName = hostName.toLowerCase(Locale.US); + boolean hasDns = false; + List altNames = getSubjectAltNames(certificate, ALT_DNS_NAME); + for (int i = 0, size = altNames.size(); i < size; i++) { + hasDns = true; + if (verifyHostName(hostName, altNames.get(i))) { + return true; + } + } + return false; + } + + public static List allSubjectAltNames(X509Certificate certificate) { + List altIpaNames = getSubjectAltNames(certificate, ALT_IPA_NAME); + List altDnsNames = getSubjectAltNames(certificate, ALT_DNS_NAME); + List result = new ArrayList<>(altIpaNames.size() + altDnsNames.size()); + result.addAll(altIpaNames); + result.addAll(altDnsNames); + return result; + } + + private static List getSubjectAltNames(X509Certificate certificate, int type) { + List result = new ArrayList<>(); + try { + Collection subjectAltNames = certificate.getSubjectAlternativeNames(); + if (subjectAltNames == null) { + return Collections.emptyList(); + } + for (Object subjectAltName : subjectAltNames) { + List entry = (List) subjectAltName; + if (entry == null || entry.size() < 2) { + continue; + } + Integer altNameType = (Integer) entry.get(0); + if (altNameType == null) { + continue; + } + if (altNameType == type) { + String altName = (String) entry.get(1); + if (altName != null) { + result.add(altName); + } + } + } + return result; + } catch (CertificateParsingException e) { + return Collections.emptyList(); + } + } + + /** + * Returns {@code true} iff {@code hostName} matches the domain name {@code pattern}. + * + * @param hostName lower-case host name. + * @param pattern domain name pattern from certificate. May be a wildcard pattern such as + * {@code *.android.com}. + */ + private boolean verifyHostName(String hostName, String pattern) { + // Basic sanity checks + // Check length == 0 instead of .isEmpty() to support Java 5. + if ((hostName == null) || (hostName.length() == 0) || (hostName.startsWith(".")) + || (hostName.endsWith(".."))) { + // Invalid domain name + return false; + } + if ((pattern == null) || (pattern.length() == 0) || (pattern.startsWith(".")) + || (pattern.endsWith(".."))) { + // Invalid pattern/domain name + return false; + } + + // Normalize hostName and pattern by turning them into absolute domain names if they are not + // yet absolute. This is needed because server certificates do not normally contain absolute + // names or patterns, but they should be treated as absolute. At the same time, any hostName + // presented to this method should also be treated as absolute for the purposes of matching + // to the server certificate. + // www.android.com matches www.android.com + // www.android.com matches www.android.com. + // www.android.com. matches www.android.com. + // www.android.com. matches www.android.com + if (!hostName.endsWith(".")) { + hostName += '.'; + } + if (!pattern.endsWith(".")) { + pattern += '.'; + } + // hostName and pattern are now absolute domain names. + + pattern = pattern.toLowerCase(Locale.US); + // hostName and pattern are now in lower case -- domain names are case-insensitive. + + if (!pattern.contains("*")) { + // Not a wildcard pattern -- hostName and pattern must match exactly. + return hostName.equals(pattern); + } + // Wildcard pattern + + // WILDCARD PATTERN RULES: + // 1. Asterisk (*) is only permitted in the left-most domain name label and must be the + // only character in that label (i.e., must match the whole left-most label). + // For example, *.example.com is permitted, while *a.example.com, a*.example.com, + // a*b.example.com, a.*.example.com are not permitted. + // 2. Asterisk (*) cannot match across domain name labels. + // For example, *.example.com matches test.example.com but does not match + // sub.test.example.com. + // 3. Wildcard patterns for single-label domain names are not permitted. + // 4. Android-added: if strictWildcardMode is true then wildcards matching top-level domains, + // e.g. *.com, are not permitted. + + if ((!pattern.startsWith("*.")) || (pattern.indexOf('*', 1) != -1)) { + // Asterisk (*) is only permitted in the left-most domain name label and must be the only + // character in that label + return false; + } + + // Optimization: check whether hostName is too short to match the pattern. hostName must be at + // least as long as the pattern because asterisk must match the whole left-most label and + // hostName starts with a non-empty label. Thus, asterisk has to match one or more characters. + if (hostName.length() < pattern.length()) { + // hostName too short to match the pattern. + return false; + } + + if ("*.".equals(pattern)) { + // Wildcard pattern for single-label domain name -- not permitted. + return false; + } + + // BEGIN Android-added: Disallow top-level wildcards in strict mode. http://b/144694112 + if (strictWildcardMode) { + // By this point we know the pattern has been normalised and starts with a wildcard, + // i.e. "*.domainpart." + String domainPart = pattern.substring(2, pattern.length() - 1); + // If the domain part contains no dots then this pattern will match top level domains. + if (domainPart.indexOf('.') < 0) { + return false; + } + } + // END Android-added: Disallow top-level wildcards in strict mode. http://b/144694112 + + // hostName must end with the region of pattern following the asterisk. + String suffix = pattern.substring(1); + if (!hostName.endsWith(suffix)) { + // hostName does not end with the suffix + return false; + } + + // Check that asterisk did not match across domain name labels. + int suffixStartIndexInHostName = hostName.length() - suffix.length(); + if ((suffixStartIndexInHostName > 0) + && (hostName.lastIndexOf('.', suffixStartIndexInHostName - 1) != -1)) { + // Asterisk is matching across domain name labels -- not permitted. + return false; + } + + // hostName matches pattern + return true; + } +} \ No newline at end of file From db7d7daa58dfa5f7f526c77910979f91371a3e99 Mon Sep 17 00:00:00 2001 From: Danny Reidenbach Date: Fri, 24 Jul 2020 23:14:13 +0000 Subject: [PATCH 02/20] Connected Platforms and removed default initialization --- android/src/main/java/org/conscrypt/Platform.java | 4 ++++ common/src/main/java/org/conscrypt/TrustManagerImpl.java | 2 +- openjdk/src/main/java/org/conscrypt/Platform.java | 4 ++++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/android/src/main/java/org/conscrypt/Platform.java b/android/src/main/java/org/conscrypt/Platform.java index d4fdd1665..c158f5d22 100644 --- a/android/src/main/java/org/conscrypt/Platform.java +++ b/android/src/main/java/org/conscrypt/Platform.java @@ -1019,4 +1019,8 @@ private static boolean serverNamePermittedInternal( } return false; } + + public static HostNameVerifier getDefaultHostNameVerifier() { + return HttpsURLConnection.getDefaultHostnameVerifier(); + } } diff --git a/common/src/main/java/org/conscrypt/TrustManagerImpl.java b/common/src/main/java/org/conscrypt/TrustManagerImpl.java index 2e4b9bc2f..3299d5cff 100644 --- a/common/src/main/java/org/conscrypt/TrustManagerImpl.java +++ b/common/src/main/java/org/conscrypt/TrustManagerImpl.java @@ -1020,7 +1020,7 @@ private ConscryptHostnameVerifier getHttpsVerifier() { // doggo return defaultVerifier; } // return GlobalHostnameVerifierAdapter.INSTANCE; - return OkHostnameVerifier.strictInstance(); + return Platform.getDefaultHostnameVerifier(); } public void setCTEnabledOverride(boolean enabled) { diff --git a/openjdk/src/main/java/org/conscrypt/Platform.java b/openjdk/src/main/java/org/conscrypt/Platform.java index 7e54f8af5..ed94163b3 100644 --- a/openjdk/src/main/java/org/conscrypt/Platform.java +++ b/openjdk/src/main/java/org/conscrypt/Platform.java @@ -785,4 +785,8 @@ public ClassLoader run() { }); } } + + public static ConscryptHostnameVerifier getDefaultHostnameVerifier() { + return OkHostnameVerifier.strictInstance(); + } } From 6cf7ad7851dec83ebde0f98792caac0bfd0cc48a Mon Sep 17 00:00:00 2001 From: Danny Reidenbach Date: Fri, 24 Jul 2020 23:38:22 +0000 Subject: [PATCH 03/20] Cleaned up code --- common/src/main/java/org/conscrypt/TrustManagerImpl.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/common/src/main/java/org/conscrypt/TrustManagerImpl.java b/common/src/main/java/org/conscrypt/TrustManagerImpl.java index 3299d5cff..aedd828c7 100644 --- a/common/src/main/java/org/conscrypt/TrustManagerImpl.java +++ b/common/src/main/java/org/conscrypt/TrustManagerImpl.java @@ -1015,11 +1015,10 @@ private ConscryptHostnameVerifier getHttpsVerifier() { // doggo if (hostnameVerifier != null) { return hostnameVerifier; } - ConscryptHostnameVerifier defaultVerifier = getDefaultHostnameVerifier(); + ConscryptHostnameVerifier defaultVerifier = getDefaultHostnameVerifier(); // this is still needed for current tests. Will I have to remove this and update teh tests to work with default or are teh predefined tests still valid? if (defaultVerifier != null) { return defaultVerifier; } -// return GlobalHostnameVerifierAdapter.INSTANCE; return Platform.getDefaultHostnameVerifier(); } From b5c6d875bd0a813cfe9c62ae8961d6907da22267 Mon Sep 17 00:00:00 2001 From: Danny Reidenbach Date: Fri, 24 Jul 2020 23:57:48 +0000 Subject: [PATCH 04/20] Fixed Typos --- android/src/main/java/org/conscrypt/Platform.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/android/src/main/java/org/conscrypt/Platform.java b/android/src/main/java/org/conscrypt/Platform.java index c158f5d22..0e1b2d5fa 100644 --- a/android/src/main/java/org/conscrypt/Platform.java +++ b/android/src/main/java/org/conscrypt/Platform.java @@ -48,6 +48,7 @@ import java.util.Collection; import java.util.Collections; import java.util.List; +import javax.net.ssl.HttpsURLConnection; import javax.net.ssl.SNIHostName; import javax.net.ssl.SNIMatcher; import javax.net.ssl.SNIServerName; @@ -1020,7 +1021,7 @@ private static boolean serverNamePermittedInternal( return false; } - public static HostNameVerifier getDefaultHostNameVerifier() { + public static HostnameVerifier getDefaultHostnameVerifier() { return HttpsURLConnection.getDefaultHostnameVerifier(); } } From ed0bc29c1726b43d56bfebf26feb48f717f831be Mon Sep 17 00:00:00 2001 From: Danny Reidenbach Date: Mon, 27 Jul 2020 20:48:47 +0000 Subject: [PATCH 05/20] Added TrustManagerTesting as well as third non bound platform --- .../src/main/java/org/conscrypt/Platform.java | 6 +- .../main/java/org/conscrypt/Conscrypt.java | 30 ++-- .../org/conscrypt/OkHostnameVerifier.java | 13 ++ .../java/org/conscrypt/TrustManagerImpl.java | 9 +- .../org/conscrypt/TrustManagerImplTest.java | 144 +++++++++++------- .../src/main/java/org/conscrypt/Platform.java | 2 +- .../src/main/java/org/conscrypt/Platform.java | 7 +- 7 files changed, 138 insertions(+), 73 deletions(-) rename {openjdk => common}/src/main/java/org/conscrypt/OkHostnameVerifier.java (95%) diff --git a/android/src/main/java/org/conscrypt/Platform.java b/android/src/main/java/org/conscrypt/Platform.java index 0e1b2d5fa..c9e0f0e9d 100644 --- a/android/src/main/java/org/conscrypt/Platform.java +++ b/android/src/main/java/org/conscrypt/Platform.java @@ -48,7 +48,7 @@ import java.util.Collection; import java.util.Collections; import java.util.List; -import javax.net.ssl.HttpsURLConnection; +import javax.net.ssl.HttpsURLConnection.HostnameVerifier; import javax.net.ssl.SNIHostName; import javax.net.ssl.SNIMatcher; import javax.net.ssl.SNIServerName; @@ -1021,7 +1021,7 @@ private static boolean serverNamePermittedInternal( return false; } - public static HostnameVerifier getDefaultHostnameVerifier() { - return HttpsURLConnection.getDefaultHostnameVerifier(); + public static ConscryptHostnameVerifier getDefaultHostnameVerifier() { + return OkHostnameVerifier.strictInstance(); } } diff --git a/common/src/main/java/org/conscrypt/Conscrypt.java b/common/src/main/java/org/conscrypt/Conscrypt.java index 42440fdd2..419140cc6 100644 --- a/common/src/main/java/org/conscrypt/Conscrypt.java +++ b/common/src/main/java/org/conscrypt/Conscrypt.java @@ -22,18 +22,10 @@ import java.security.PrivateKey; import java.security.Provider; import java.util.Properties; -import javax.net.ssl.HttpsURLConnection; -import javax.net.ssl.SSLContext; -import javax.net.ssl.SSLContextSpi; -import javax.net.ssl.SSLEngine; -import javax.net.ssl.SSLEngineResult; -import javax.net.ssl.SSLException; -import javax.net.ssl.SSLServerSocketFactory; -import javax.net.ssl.SSLSessionContext; -import javax.net.ssl.SSLSocket; -import javax.net.ssl.SSLSocketFactory; -import javax.net.ssl.TrustManager; -import javax.net.ssl.X509TrustManager; +//import javax.net.ssl.HttpsURLConnection.HostnameVerifier; THIS IS GIVING ISSUES FOR SOME REASON +import javax.net.ssl.*; +import javax.net.ssl.HostnameVerifier; + import org.conscrypt.io.IoUtils; /** @@ -781,5 +773,17 @@ public static ConscryptHostnameVerifier getHostnameVerifier(TrustManager trustMa return toConscrypt(trustManager).getHostnameVerifier(); } - + /** + * Wraps the HttpsURLConnection.HostnameVerifier into a ConscryptHostnameVerifier + */ + public static ConscryptHostnameVerifier wrapHostnameVerifier(final HostnameVerifier verifier) { + // needed to add final due to : error: local variable verifier is accessed from within inner class; needs to be declared final +// Cannot find HttpsURLConnection.HostnameVerifier + return new ConscryptHostnameVerifier() { + @Override + public boolean verify(String hostname, SSLSession session) { + return verifier.verify(hostname, session); + } + }; + } } diff --git a/openjdk/src/main/java/org/conscrypt/OkHostnameVerifier.java b/common/src/main/java/org/conscrypt/OkHostnameVerifier.java similarity index 95% rename from openjdk/src/main/java/org/conscrypt/OkHostnameVerifier.java rename to common/src/main/java/org/conscrypt/OkHostnameVerifier.java index ea277e548..ba525bca1 100644 --- a/openjdk/src/main/java/org/conscrypt/OkHostnameVerifier.java +++ b/common/src/main/java/org/conscrypt/OkHostnameVerifier.java @@ -119,6 +119,19 @@ private boolean verifyHostName(String hostName, X509Certificate certificate) { return false; } + // BEGIN Android-removed: Ignore common name in hostname verification. http://b/70278814 + /* + if (!hasDns) { + X500Principal principal = certificate.getSubjectX500Principal(); + // RFC 2818 advises using the most specific name for matching. + String cn = new DistinguishedNameParser(principal).findMostSpecific("cn"); + if (cn != null) { + return verifyHostName(hostName, cn); + } + } + */ + // END Android-removed: Ignore common name in hostname verification. http://b/70278814 + public static List allSubjectAltNames(X509Certificate certificate) { List altIpaNames = getSubjectAltNames(certificate, ALT_IPA_NAME); List altDnsNames = getSubjectAltNames(certificate, ALT_DNS_NAME); diff --git a/common/src/main/java/org/conscrypt/TrustManagerImpl.java b/common/src/main/java/org/conscrypt/TrustManagerImpl.java index aedd828c7..c865e22dc 100644 --- a/common/src/main/java/org/conscrypt/TrustManagerImpl.java +++ b/common/src/main/java/org/conscrypt/TrustManagerImpl.java @@ -1015,10 +1015,11 @@ private ConscryptHostnameVerifier getHttpsVerifier() { // doggo if (hostnameVerifier != null) { return hostnameVerifier; } - ConscryptHostnameVerifier defaultVerifier = getDefaultHostnameVerifier(); // this is still needed for current tests. Will I have to remove this and update teh tests to work with default or are teh predefined tests still valid? - if (defaultVerifier != null) { - return defaultVerifier; - } +// // remove +// ConscryptHostnameVerifier defaultVerifier = getDefaultHostnameVerifier(); // this is still needed for current tests. Will I have to remove this and update the tests to work with default or are teh predefined tests still valid? +// if (defaultVerifier != null) { +// return defaultVerifier; +// } return Platform.getDefaultHostnameVerifier(); } diff --git a/common/src/test/java/org/conscrypt/TrustManagerImplTest.java b/common/src/test/java/org/conscrypt/TrustManagerImplTest.java index 3d444713d..17920fa4e 100644 --- a/common/src/test/java/org/conscrypt/TrustManagerImplTest.java +++ b/common/src/test/java/org/conscrypt/TrustManagerImplTest.java @@ -123,6 +123,91 @@ public void testGetFullChain() throws Exception { assertEquals(Arrays.asList(chain3), certs); } +// @Test +// public void testHttpsEndpointIdentification() throws Exception { +// TestUtils.assumeExtendedTrustManagerAvailable(); +// +// KeyStore.PrivateKeyEntry pke = TestKeyStore.getServerHostname().getPrivateKey("RSA", "RSA"); +// X509Certificate[] chain = (X509Certificate[]) pke.getCertificateChain(); +// X509Certificate root = chain[2]; +// TrustManagerImpl tmi = (TrustManagerImpl) trustManager(root); +// +// String goodHostname = TestKeyStore.CERT_HOSTNAME; +// String badHostname = "definitelywrong.nopenopenope"; +// +// // The default hostname verifier on OpenJDK rejects all hostnames, so use our own +// javax.net.ssl.HostnameVerifier oldDefault = HttpsURLConnection.getDefaultHostnameVerifier(); +// try { +// HttpsURLConnection.setDefaultHostnameVerifier(new TestHostnameVerifier()); +// +// SSLParameters params = new SSLParameters(); +// +// // Without endpoint identification this should pass despite the mismatched hostname +// params.setEndpointIdentificationAlgorithm(null); +// +// List certs = tmi.getTrustedChainForServer(chain, "RSA", +// new FakeSSLSocket(new FakeSSLSession(badHostname, chain), params)); +// assertEquals(Arrays.asList(chain), certs); +// +// // Turn on endpoint identification +// params.setEndpointIdentificationAlgorithm("HTTPS"); +// +// try { +// tmi.getTrustedChainForServer(chain, "RSA", +// new FakeSSLSocket(new FakeSSLSession(badHostname, chain), params)); +// fail(); +// } catch (CertificateException expected) { +// } +// +// certs = tmi.getTrustedChainForServer(chain, "RSA", +// new FakeSSLSocket(new FakeSSLSession(goodHostname, chain), params)); +// assertEquals(Arrays.asList(chain), certs); +// +// // Override the global default hostname verifier with a Conscrypt-specific one that +// // always passes. Both scenarios should pass. +// Conscrypt.setDefaultHostnameVerifier(new ConscryptHostnameVerifier() { +// @Override public boolean verify(String s, SSLSession sslSession) { return true; } +// }); +// +// certs = tmi.getTrustedChainForServer(chain, "RSA", +// new FakeSSLSocket(new FakeSSLSession(badHostname, chain), params)); +// assertEquals(Arrays.asList(chain), certs); +// +// certs = tmi.getTrustedChainForServer(chain, "RSA", +// new FakeSSLSocket(new FakeSSLSession(goodHostname, chain), params)); +// assertEquals(Arrays.asList(chain), certs); +// +// // Now set an instance-specific verifier on the trust manager. The bad hostname should +// // fail again. +// Conscrypt.setHostnameVerifier(tmi, new TestHostnameVerifier()); +// +// try { +// tmi.getTrustedChainForServer(chain, "RSA", +// new FakeSSLSocket(new FakeSSLSession(badHostname, chain), params)); +// fail(); +// } catch (CertificateException expected) { +// } +// +// certs = tmi.getTrustedChainForServer(chain, "RSA", +// new FakeSSLSocket(new FakeSSLSession(goodHostname, chain), params)); +// assertEquals(Arrays.asList(chain), certs); +// +// // Remove the instance-specific verifier, and both should pass again. +// Conscrypt.setHostnameVerifier(tmi, null); +// +// certs = tmi.getTrustedChainForServer(chain, "RSA", +// new FakeSSLSocket(new FakeSSLSession(badHostname, chain), params)); +// assertEquals(Arrays.asList(chain), certs); +// +// certs = tmi.getTrustedChainForServer(chain, "RSA", +// new FakeSSLSocket(new FakeSSLSession(goodHostname, chain), params)); +// assertEquals(Arrays.asList(chain), certs); +// } finally { +// Conscrypt.setDefaultHostnameVerifier(null); +// HttpsURLConnection.setDefaultHostnameVerifier(oldDefault); +// } +// } + @Test public void testHttpsEndpointIdentification() throws Exception { TestUtils.assumeExtendedTrustManagerAvailable(); @@ -135,76 +220,33 @@ public void testHttpsEndpointIdentification() throws Exception { String goodHostname = TestKeyStore.CERT_HOSTNAME; String badHostname = "definitelywrong.nopenopenope"; - // The default hostname verifier on OpenJDK rejects all hostnames, so use our own - javax.net.ssl.HostnameVerifier oldDefault = HttpsURLConnection.getDefaultHostnameVerifier(); + // The default hostname verifier on OpenJDK no longer rejects all hostnames +// javax.net.ssl.HostnameVerifier oldDefault = HttpsURLConnection.getDefaultHostnameVerifier(); try { - HttpsURLConnection.setDefaultHostnameVerifier(new TestHostnameVerifier()); - SSLParameters params = new SSLParameters(); // Without endpoint identification this should pass despite the mismatched hostname params.setEndpointIdentificationAlgorithm(null); List certs = tmi.getTrustedChainForServer(chain, "RSA", - new FakeSSLSocket(new FakeSSLSession(badHostname, chain), params)); + new FakeSSLSocket(new FakeSSLSession(badHostname, chain), params)); assertEquals(Arrays.asList(chain), certs); // Turn on endpoint identification params.setEndpointIdentificationAlgorithm("HTTPS"); - try { - tmi.getTrustedChainForServer(chain, "RSA", + try { // this should fail + certs = tmi.getTrustedChainForServer(chain, "RSA", new FakeSSLSocket(new FakeSSLSession(badHostname, chain), params)); + assertEquals(Arrays.asList(chain), certs); fail(); - } catch (CertificateException expected) { - } - - certs = tmi.getTrustedChainForServer(chain, "RSA", - new FakeSSLSocket(new FakeSSLSession(goodHostname, chain), params)); - assertEquals(Arrays.asList(chain), certs); - - // Override the global default hostname verifier with a Conscrypt-specific one that - // always passes. Both scenarios should pass. - Conscrypt.setDefaultHostnameVerifier(new ConscryptHostnameVerifier() { - @Override public boolean verify(String s, SSLSession sslSession) { return true; } - }); - - certs = tmi.getTrustedChainForServer(chain, "RSA", - new FakeSSLSocket(new FakeSSLSession(badHostname, chain), params)); - assertEquals(Arrays.asList(chain), certs); - - certs = tmi.getTrustedChainForServer(chain, "RSA", - new FakeSSLSocket(new FakeSSLSession(goodHostname, chain), params)); - assertEquals(Arrays.asList(chain), certs); - // Now set an instance-specific verifier on the trust manager. The bad hostname should - // fail again. - Conscrypt.setHostnameVerifier(tmi, new TestHostnameVerifier()); - - try { - tmi.getTrustedChainForServer(chain, "RSA", - new FakeSSLSocket(new FakeSSLSession(badHostname, chain), params)); - fail(); } catch (CertificateException expected) { } - - certs = tmi.getTrustedChainForServer(chain, "RSA", - new FakeSSLSocket(new FakeSSLSession(goodHostname, chain), params)); - assertEquals(Arrays.asList(chain), certs); - - // Remove the instance-specific verifier, and both should pass again. - Conscrypt.setHostnameVerifier(tmi, null); - - certs = tmi.getTrustedChainForServer(chain, "RSA", - new FakeSSLSocket(new FakeSSLSession(badHostname, chain), params)); - assertEquals(Arrays.asList(chain), certs); - certs = tmi.getTrustedChainForServer(chain, "RSA", - new FakeSSLSocket(new FakeSSLSession(goodHostname, chain), params)); + new FakeSSLSocket(new FakeSSLSession(goodHostname, chain), params)); assertEquals(Arrays.asList(chain), certs); - } finally { - Conscrypt.setDefaultHostnameVerifier(null); - HttpsURLConnection.setDefaultHostnameVerifier(oldDefault); + } catch (Exception e) { } } diff --git a/openjdk/src/main/java/org/conscrypt/Platform.java b/openjdk/src/main/java/org/conscrypt/Platform.java index ed94163b3..76e57d43a 100644 --- a/openjdk/src/main/java/org/conscrypt/Platform.java +++ b/openjdk/src/main/java/org/conscrypt/Platform.java @@ -787,6 +787,6 @@ public ClassLoader run() { } public static ConscryptHostnameVerifier getDefaultHostnameVerifier() { - return OkHostnameVerifier.strictInstance(); + return OkHostnameVerifier.strictInstance(); } } diff --git a/platform/src/main/java/org/conscrypt/Platform.java b/platform/src/main/java/org/conscrypt/Platform.java index 102bcaa53..a3c41d0d2 100644 --- a/platform/src/main/java/org/conscrypt/Platform.java +++ b/platform/src/main/java/org/conscrypt/Platform.java @@ -48,6 +48,7 @@ import java.util.Collections; import java.util.List; import javax.crypto.spec.GCMParameterSpec; +import javax.net.ssl.HttpsURLConnection.HostnameVerifier; import javax.net.ssl.SNIHostName; import javax.net.ssl.SNIMatcher; import javax.net.ssl.SNIServerName; @@ -64,7 +65,7 @@ import org.conscrypt.ct.CTPolicy; import org.conscrypt.ct.CTPolicyImpl; import sun.security.x509.AlgorithmId; - +// use https one here final class Platform { private static class NoPreloadHolder { public static final Platform MAPPER = new Platform(); } @@ -530,4 +531,8 @@ static boolean serverNamePermitted(SSLParametersImpl parameters, String serverNa } return false; } + + public static ConscryptHostnameVerifier getDefaultHostnameVerifier() { + return Conscrypt.wrapHostnameVerifier(HttpsURLConnection.getDefaultHostnameVerifier()); + } } From 6003260a0750a110179f48844da7319d3e77ba21 Mon Sep 17 00:00:00 2001 From: Danny Reidenbach Date: Mon, 27 Jul 2020 21:10:16 +0000 Subject: [PATCH 06/20] removed import for ubuntu integration test --- android/src/main/java/org/conscrypt/Platform.java | 1 - 1 file changed, 1 deletion(-) diff --git a/android/src/main/java/org/conscrypt/Platform.java b/android/src/main/java/org/conscrypt/Platform.java index c9e0f0e9d..3c4486452 100644 --- a/android/src/main/java/org/conscrypt/Platform.java +++ b/android/src/main/java/org/conscrypt/Platform.java @@ -48,7 +48,6 @@ import java.util.Collection; import java.util.Collections; import java.util.List; -import javax.net.ssl.HttpsURLConnection.HostnameVerifier; import javax.net.ssl.SNIHostName; import javax.net.ssl.SNIMatcher; import javax.net.ssl.SNIServerName; From 64d055a56e7186567d105e9fb3e2647db1b56427 Mon Sep 17 00:00:00 2001 From: Danny Reidenbach Date: Tue, 28 Jul 2020 18:18:12 +0000 Subject: [PATCH 07/20] Added HostnameVerifierTest --- .../main/java/org/conscrypt/Conscrypt.java | 18 +- .../org/conscrypt/OkHostnameVerifier.java | 4 +- .../java/org/conscrypt/FakeSSLSession.java | 120 ++++ .../org/conscrypt/HostnameVerifierTest.java | 621 ++++++++++++++++++ .../org/conscrypt/TrustManagerImplTest.java | 6 +- 5 files changed, 760 insertions(+), 9 deletions(-) create mode 100644 common/src/test/java/org/conscrypt/FakeSSLSession.java create mode 100644 common/src/test/java/org/conscrypt/HostnameVerifierTest.java diff --git a/common/src/main/java/org/conscrypt/Conscrypt.java b/common/src/main/java/org/conscrypt/Conscrypt.java index 419140cc6..f8bc2f4a1 100644 --- a/common/src/main/java/org/conscrypt/Conscrypt.java +++ b/common/src/main/java/org/conscrypt/Conscrypt.java @@ -22,10 +22,20 @@ import java.security.PrivateKey; import java.security.Provider; import java.util.Properties; -//import javax.net.ssl.HttpsURLConnection.HostnameVerifier; THIS IS GIVING ISSUES FOR SOME REASON -import javax.net.ssl.*; +import javax.net.ssl.HttpsURLConnection; import javax.net.ssl.HostnameVerifier; - +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLContextSpi; +import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLEngineResult; +import javax.net.ssl.SSLException; +import javax.net.ssl.SSLServerSocketFactory; +import javax.net.ssl.SSLSession; +import javax.net.ssl.SSLSessionContext; +import javax.net.ssl.SSLSocket; +import javax.net.ssl.SSLSocketFactory; +import javax.net.ssl.TrustManager; +import javax.net.ssl.X509TrustManager; import org.conscrypt.io.IoUtils; /** @@ -778,7 +788,7 @@ public static ConscryptHostnameVerifier getHostnameVerifier(TrustManager trustMa */ public static ConscryptHostnameVerifier wrapHostnameVerifier(final HostnameVerifier verifier) { // needed to add final due to : error: local variable verifier is accessed from within inner class; needs to be declared final -// Cannot find HttpsURLConnection.HostnameVerifier + // Cannot find HttpsURLConnection.HostnameVerifier return new ConscryptHostnameVerifier() { @Override public boolean verify(String hostname, SSLSession session) { diff --git a/common/src/main/java/org/conscrypt/OkHostnameVerifier.java b/common/src/main/java/org/conscrypt/OkHostnameVerifier.java index ba525bca1..f5b987008 100644 --- a/common/src/main/java/org/conscrypt/OkHostnameVerifier.java +++ b/common/src/main/java/org/conscrypt/OkHostnameVerifier.java @@ -174,8 +174,8 @@ private static List getSubjectAltNames(X509Certificate certificate, int * Returns {@code true} iff {@code hostName} matches the domain name {@code pattern}. * * @param hostName lower-case host name. - * @param pattern domain name pattern from certificate. May be a wildcard pattern such as - * {@code *.android.com}. + * @param pattern domain name pattern from certificate. May be a wildcard pattern such as + * {@code *.android.com}. */ private boolean verifyHostName(String hostName, String pattern) { // Basic sanity checks diff --git a/common/src/test/java/org/conscrypt/FakeSSLSession.java b/common/src/test/java/org/conscrypt/FakeSSLSession.java new file mode 100644 index 000000000..86372414e --- /dev/null +++ b/common/src/test/java/org/conscrypt/FakeSSLSession.java @@ -0,0 +1,120 @@ +/* + * 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. + */ +package org.conscrypt; + +import java.security.Principal; +import java.security.cert.Certificate; +import javax.net.ssl.SSLPeerUnverifiedException; +import javax.net.ssl.SSLSession; +import javax.net.ssl.SSLSessionContext; +import javax.security.cert.X509Certificate; + +final class FakeSSLSession implements SSLSession { + private final Certificate[] certificates; + + public FakeSSLSession(Certificate... certificates) throws Exception { + this.certificates = certificates; + } + + public int getApplicationBufferSize() { + throw new UnsupportedOperationException(); + } + + public String getCipherSuite() { + throw new UnsupportedOperationException(); + } + + public long getCreationTime() { + throw new UnsupportedOperationException(); + } + + public byte[] getId() { + throw new UnsupportedOperationException(); + } + + public long getLastAccessedTime() { + throw new UnsupportedOperationException(); + } + + public Certificate[] getLocalCertificates() { + throw new UnsupportedOperationException(); + } + + public Principal getLocalPrincipal() { + throw new UnsupportedOperationException(); + } + + public int getPacketBufferSize() { + throw new UnsupportedOperationException(); + } + + public Certificate[] getPeerCertificates() throws SSLPeerUnverifiedException { + if (certificates.length == 0) { + throw new SSLPeerUnverifiedException("peer not authenticated"); + } else { + return certificates; + } + } + + public X509Certificate[] getPeerCertificateChain() throws SSLPeerUnverifiedException { + throw new UnsupportedOperationException(); + } + + public String getPeerHost() { + throw new UnsupportedOperationException(); + } + + public int getPeerPort() { + throw new UnsupportedOperationException(); + } + + public Principal getPeerPrincipal() throws SSLPeerUnverifiedException { + throw new UnsupportedOperationException(); + } + + public String getProtocol() { + throw new UnsupportedOperationException(); + } + + public SSLSessionContext getSessionContext() { + throw new UnsupportedOperationException(); + } + + public void putValue(String s, Object obj) { + throw new UnsupportedOperationException(); + } + + public void removeValue(String s) { + throw new UnsupportedOperationException(); + } + + public Object getValue(String s) { + throw new UnsupportedOperationException(); + } + + public String[] getValueNames() { + throw new UnsupportedOperationException(); + } + + public void invalidate() { + throw new UnsupportedOperationException(); + } + + public boolean isValid() { + throw new UnsupportedOperationException(); + } +} diff --git a/common/src/test/java/org/conscrypt/HostnameVerifierTest.java b/common/src/test/java/org/conscrypt/HostnameVerifierTest.java new file mode 100644 index 000000000..a8fceb14e --- /dev/null +++ b/common/src/test/java/org/conscrypt/HostnameVerifierTest.java @@ -0,0 +1,621 @@ +/* + * 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. + */ + +package org.conscrypt; + +import java.io.ByteArrayInputStream; +import java.nio.charset.Charset; +import java.security.cert.Certificate; +import java.security.cert.CertificateFactory; +import java.security.cert.X509Certificate; +import java.util.Arrays; +import java.util.Collection; +import javax.net.ssl.HostnameVerifier; +import javax.net.ssl.SSLSession; +import javax.security.auth.x500.X500Principal; +import org.junit.Ignore; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +/** + * Tests for our hostname verifier. Most of these tests are from AOSP, which + * itself includes tests from the Apache HTTP Client test suite. + */ +@RunWith(Parameterized.class) +public final class HostnameVerifierTest { + + private static final Charset UTF_8 = Charset.forName("UTF-8"); + // BEGIN Android-changed: Run tests for both default and strict verifiers. http://b/144694112 + // private HostnameVerifier verifier = OkHostnameVerifier.INSTANCE; + @Parameters() + public static Collection data() { + // Both verifiers should behave the same in all tests except for + // subjectAltNameWithToplevelWildcard(), and that test is not parameterized for clarity. + return Arrays.asList(new Object[][] { + { OkHostnameVerifier.INSTANCE }, + { OkHostnameVerifier.strictInstance() } + }); + } + + @Parameter + public HostnameVerifier verifier; + // END Android-changed: Run tests for both default and strict verifiers. http://b/144694112 + + @Test public void verify() throws Exception { + FakeSSLSession session = new FakeSSLSession(); + assertFalse(verifier.verify("localhost", session)); + assertEquals(1,0); + } + + @Test public void verifyCn() throws Exception { + // CN=foo.com + SSLSession session = session("" + + "-----BEGIN CERTIFICATE-----\n" + + "MIIERjCCAy6gAwIBAgIJAIz+EYMBU6aQMA0GCSqGSIb3DQEBBQUAMIGiMQswCQYD\n" + + "VQQGEwJDQTELMAkGA1UECBMCQkMxEjAQBgNVBAcTCVZhbmNvdXZlcjEWMBQGA1UE\n" + + "ChMNd3d3LmN1Y2JjLmNvbTEUMBIGA1UECxQLY29tbW9uc19zc2wxHTAbBgNVBAMU\n" + + "FGRlbW9faW50ZXJtZWRpYXRlX2NhMSUwIwYJKoZIhvcNAQkBFhZqdWxpdXNkYXZp\n" + + "ZXNAZ21haWwuY29tMB4XDTA2MTIxMTE1MzE0MVoXDTI4MTEwNTE1MzE0MVowgaQx\n" + + "CzAJBgNVBAYTAlVTMREwDwYDVQQIEwhNYXJ5bGFuZDEUMBIGA1UEBxMLRm9yZXN0\n" + + "IEhpbGwxFzAVBgNVBAoTDmh0dHBjb21wb25lbnRzMRowGAYDVQQLExF0ZXN0IGNl\n" + + "cnRpZmljYXRlczEQMA4GA1UEAxMHZm9vLmNvbTElMCMGCSqGSIb3DQEJARYWanVs\n" + + "aXVzZGF2aWVzQGdtYWlsLmNvbTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoC\n" + + "ggEBAMhjr5aCPoyp0R1iroWAfnEyBMGYWoCidH96yGPFjYLowez5aYKY1IOKTY2B\n" + + "lYho4O84X244QrZTRl8kQbYtxnGh4gSCD+Z8gjZ/gMvLUlhqOb+WXPAUHMB39GRy\n" + + "zerA/ZtrlUqf+lKo0uWcocxeRc771KN8cPH3nHZ0rV0Hx4ZAZy6U4xxObe4rtSVY\n" + + "07hNKXAb2odnVqgzcYiDkLV8ilvEmoNWMWrp8UBqkTcpEhYhCYp3cTkgJwMSuqv8\n" + + "BqnGd87xQU3FVZI4tbtkB+KzjD9zz8QCDJAfDjZHR03KNQ5mxOgXwxwKw6lGMaiV\n" + + "JTxpTKqym93whYk93l3ocEe55c0CAwEAAaN7MHkwCQYDVR0TBAIwADAsBglghkgB\n" + + "hvhCAQ0EHxYdT3BlblNTTCBHZW5lcmF0ZWQgQ2VydGlmaWNhdGUwHQYDVR0OBBYE\n" + + "FJ8Ud78/OrbKOIJCSBYs2tDLXofYMB8GA1UdIwQYMBaAFHua2o+QmU5S0qzbswNS\n" + + "yoemDT4NMA0GCSqGSIb3DQEBBQUAA4IBAQC3jRmEya6sQCkmieULcvx8zz1euCk9\n" + + "fSez7BEtki8+dmfMXe3K7sH0lI8f4jJR0rbSCjpmCQLYmzC3NxBKeJOW0RcjNBpO\n" + + "c2JlGO9auXv2GDP4IYiXElLJ6VSqc8WvDikv0JmCCWm0Zga+bZbR/EWN5DeEtFdF\n" + + "815CLpJZNcYwiYwGy/CVQ7w2TnXlG+mraZOz+owr+cL6J/ZesbdEWfjoS1+cUEhE\n" + + "HwlNrAu8jlZ2UqSgskSWlhYdMTAP9CPHiUv9N7FcT58Itv/I4fKREINQYjDpvQcx\n" + + "SaTYb9dr5sB4WLNglk7zxDtM80H518VvihTcP7FHL+Gn6g4j5fkI98+S\n" + + "-----END CERTIFICATE-----\n"); + // Android-changed: Ignore common name in hostname verification. http://b/70278814 + // assertTrue(verifier.verify("foo.com", session)); + assertFalse(verifier.verify("foo.com", session)); + assertFalse(verifier.verify("a.foo.com", session)); + assertFalse(verifier.verify("bar.com", session)); + } + + @Test public void verifyNonAsciiCn() throws Exception { + // CN=花子.co.jp + SSLSession session = session("" + + "-----BEGIN CERTIFICATE-----\n" + + "MIIESzCCAzOgAwIBAgIJAIz+EYMBU6aTMA0GCSqGSIb3DQEBBQUAMIGiMQswCQYD\n" + + "VQQGEwJDQTELMAkGA1UECBMCQkMxEjAQBgNVBAcTCVZhbmNvdXZlcjEWMBQGA1UE\n" + + "ChMNd3d3LmN1Y2JjLmNvbTEUMBIGA1UECxQLY29tbW9uc19zc2wxHTAbBgNVBAMU\n" + + "FGRlbW9faW50ZXJtZWRpYXRlX2NhMSUwIwYJKoZIhvcNAQkBFhZqdWxpdXNkYXZp\n" + + "ZXNAZ21haWwuY29tMB4XDTA2MTIxMTE1NDIxNVoXDTI4MTEwNTE1NDIxNVowgakx\n" + + "CzAJBgNVBAYTAlVTMREwDwYDVQQIDAhNYXJ5bGFuZDEUMBIGA1UEBwwLRm9yZXN0\n" + + "IEhpbGwxFzAVBgNVBAoMDmh0dHBjb21wb25lbnRzMRowGAYDVQQLDBF0ZXN0IGNl\n" + + "cnRpZmljYXRlczEVMBMGA1UEAwwM6Iqx5a2QLmNvLmpwMSUwIwYJKoZIhvcNAQkB\n" + + "FhZqdWxpdXNkYXZpZXNAZ21haWwuY29tMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8A\n" + + "MIIBCgKCAQEAyGOvloI+jKnRHWKuhYB+cTIEwZhagKJ0f3rIY8WNgujB7PlpgpjU\n" + + "g4pNjYGViGjg7zhfbjhCtlNGXyRBti3GcaHiBIIP5nyCNn+Ay8tSWGo5v5Zc8BQc\n" + + "wHf0ZHLN6sD9m2uVSp/6UqjS5ZyhzF5FzvvUo3xw8fecdnStXQfHhkBnLpTjHE5t\n" + + "7iu1JVjTuE0pcBvah2dWqDNxiIOQtXyKW8Sag1YxaunxQGqRNykSFiEJindxOSAn\n" + + "AxK6q/wGqcZ3zvFBTcVVkji1u2QH4rOMP3PPxAIMkB8ONkdHTco1DmbE6BfDHArD\n" + + "qUYxqJUlPGlMqrKb3fCFiT3eXehwR7nlzQIDAQABo3sweTAJBgNVHRMEAjAAMCwG\n" + + "CWCGSAGG+EIBDQQfFh1PcGVuU1NMIEdlbmVyYXRlZCBDZXJ0aWZpY2F0ZTAdBgNV\n" + + "HQ4EFgQUnxR3vz86tso4gkJIFiza0Mteh9gwHwYDVR0jBBgwFoAUe5raj5CZTlLS\n" + + "rNuzA1LKh6YNPg0wDQYJKoZIhvcNAQEFBQADggEBALJ27i3okV/KvlDp6KMID3gd\n" + + "ITl68PyItzzx+SquF8gahMh016NX73z/oVZoVUNdftla8wPUB1GwIkAnGkhQ9LHK\n" + + "spBdbRiCj0gMmLCsX8SrjFvr7cYb2cK6J/fJe92l1tg/7Y4o7V/s4JBe/cy9U9w8\n" + + "a0ctuDmEBCgC784JMDtT67klRfr/2LlqWhlOEq7pUFxRLbhpquaAHSOjmIcWnVpw\n" + + "9BsO7qe46hidgn39hKh1WjKK2VcL/3YRsC4wUi0PBtFW6ScMCuMhgIRXSPU55Rae\n" + + "UIlOdPjjr1SUNWGId1rD7W16Scpwnknn310FNxFMHVI0GTGFkNdkilNCFJcIoRA=\n" + + "-----END CERTIFICATE-----\n"); + // Android-changed: Ignore common name in hostname verification. http://b/70278814 + // assertTrue(verifier.verify("\u82b1\u5b50.co.jp", session)); + assertFalse(verifier.verify("\u82b1\u5b50.co.jp", session)); + assertFalse(verifier.verify("a.\u82b1\u5b50.co.jp", session)); + } + + @Test public void verifySubjectAlt() throws Exception { + // CN=foo.com, subjectAlt=bar.com + SSLSession session = session("" + + "-----BEGIN CERTIFICATE-----\n" + + "MIIEXDCCA0SgAwIBAgIJAIz+EYMBU6aRMA0GCSqGSIb3DQEBBQUAMIGiMQswCQYD\n" + + "VQQGEwJDQTELMAkGA1UECBMCQkMxEjAQBgNVBAcTCVZhbmNvdXZlcjEWMBQGA1UE\n" + + "ChMNd3d3LmN1Y2JjLmNvbTEUMBIGA1UECxQLY29tbW9uc19zc2wxHTAbBgNVBAMU\n" + + "FGRlbW9faW50ZXJtZWRpYXRlX2NhMSUwIwYJKoZIhvcNAQkBFhZqdWxpdXNkYXZp\n" + + "ZXNAZ21haWwuY29tMB4XDTA2MTIxMTE1MzYyOVoXDTI4MTEwNTE1MzYyOVowgaQx\n" + + "CzAJBgNVBAYTAlVTMREwDwYDVQQIEwhNYXJ5bGFuZDEUMBIGA1UEBxMLRm9yZXN0\n" + + "IEhpbGwxFzAVBgNVBAoTDmh0dHBjb21wb25lbnRzMRowGAYDVQQLExF0ZXN0IGNl\n" + + "cnRpZmljYXRlczEQMA4GA1UEAxMHZm9vLmNvbTElMCMGCSqGSIb3DQEJARYWanVs\n" + + "aXVzZGF2aWVzQGdtYWlsLmNvbTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoC\n" + + "ggEBAMhjr5aCPoyp0R1iroWAfnEyBMGYWoCidH96yGPFjYLowez5aYKY1IOKTY2B\n" + + "lYho4O84X244QrZTRl8kQbYtxnGh4gSCD+Z8gjZ/gMvLUlhqOb+WXPAUHMB39GRy\n" + + "zerA/ZtrlUqf+lKo0uWcocxeRc771KN8cPH3nHZ0rV0Hx4ZAZy6U4xxObe4rtSVY\n" + + "07hNKXAb2odnVqgzcYiDkLV8ilvEmoNWMWrp8UBqkTcpEhYhCYp3cTkgJwMSuqv8\n" + + "BqnGd87xQU3FVZI4tbtkB+KzjD9zz8QCDJAfDjZHR03KNQ5mxOgXwxwKw6lGMaiV\n" + + "JTxpTKqym93whYk93l3ocEe55c0CAwEAAaOBkDCBjTAJBgNVHRMEAjAAMCwGCWCG\n" + + "SAGG+EIBDQQfFh1PcGVuU1NMIEdlbmVyYXRlZCBDZXJ0aWZpY2F0ZTAdBgNVHQ4E\n" + + "FgQUnxR3vz86tso4gkJIFiza0Mteh9gwHwYDVR0jBBgwFoAUe5raj5CZTlLSrNuz\n" + + "A1LKh6YNPg0wEgYDVR0RBAswCYIHYmFyLmNvbTANBgkqhkiG9w0BAQUFAAOCAQEA\n" + + "dQyprNZBmVnvuVWjV42sey/PTfkYShJwy1j0/jcFZR/ypZUovpiHGDO1DgL3Y3IP\n" + + "zVQ26uhUsSw6G0gGRiaBDe/0LUclXZoJzXX1qpS55OadxW73brziS0sxRgGrZE/d\n" + + "3g5kkio6IED47OP6wYnlmZ7EKP9cqjWwlnvHnnUcZ2SscoLNYs9rN9ccp8tuq2by\n" + + "88OyhKwGjJfhOudqfTNZcDzRHx4Fzm7UsVaycVw4uDmhEHJrAsmMPpj/+XRK9/42\n" + + "2xq+8bc6HojdtbCyug/fvBZvZqQXSmU8m8IVcMmWMz0ZQO8ee3QkBHMZfCy7P/kr\n" + + "VbWx/uETImUu+NZg22ewEw==\n" + + "-----END CERTIFICATE-----\n"); + assertFalse(verifier.verify("foo.com", session)); + assertFalse(verifier.verify("a.foo.com", session)); + assertTrue(verifier.verify("bar.com", session)); + assertFalse(verifier.verify("a.bar.com", session)); + } + + /** + * Ignored due to incompatibilities between Android and Java on how non-ASCII + * subject alt names are parsed. Android fails to parse these, which means we + * fall back to the CN. The RI does parse them, so the CN is unused. + */ + @Test @Ignore public void verifyNonAsciiSubjectAlt() throws Exception { + // CN=foo.com, subjectAlt=bar.com, subjectAlt=花子.co.jp + // (hanako.co.jp in kanji) + SSLSession session = session("" + + "-----BEGIN CERTIFICATE-----\n" + + "MIIEajCCA1KgAwIBAgIJAIz+EYMBU6aSMA0GCSqGSIb3DQEBBQUAMIGiMQswCQYD\n" + + "VQQGEwJDQTELMAkGA1UECBMCQkMxEjAQBgNVBAcTCVZhbmNvdXZlcjEWMBQGA1UE\n" + + "ChMNd3d3LmN1Y2JjLmNvbTEUMBIGA1UECxQLY29tbW9uc19zc2wxHTAbBgNVBAMU\n" + + "FGRlbW9faW50ZXJtZWRpYXRlX2NhMSUwIwYJKoZIhvcNAQkBFhZqdWxpdXNkYXZp\n" + + "ZXNAZ21haWwuY29tMB4XDTA2MTIxMTE1MzgxM1oXDTI4MTEwNTE1MzgxM1owgaQx\n" + + "CzAJBgNVBAYTAlVTMREwDwYDVQQIEwhNYXJ5bGFuZDEUMBIGA1UEBxMLRm9yZXN0\n" + + "IEhpbGwxFzAVBgNVBAoTDmh0dHBjb21wb25lbnRzMRowGAYDVQQLExF0ZXN0IGNl\n" + + "cnRpZmljYXRlczEQMA4GA1UEAxMHZm9vLmNvbTElMCMGCSqGSIb3DQEJARYWanVs\n" + + "aXVzZGF2aWVzQGdtYWlsLmNvbTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoC\n" + + "ggEBAMhjr5aCPoyp0R1iroWAfnEyBMGYWoCidH96yGPFjYLowez5aYKY1IOKTY2B\n" + + "lYho4O84X244QrZTRl8kQbYtxnGh4gSCD+Z8gjZ/gMvLUlhqOb+WXPAUHMB39GRy\n" + + "zerA/ZtrlUqf+lKo0uWcocxeRc771KN8cPH3nHZ0rV0Hx4ZAZy6U4xxObe4rtSVY\n" + + "07hNKXAb2odnVqgzcYiDkLV8ilvEmoNWMWrp8UBqkTcpEhYhCYp3cTkgJwMSuqv8\n" + + "BqnGd87xQU3FVZI4tbtkB+KzjD9zz8QCDJAfDjZHR03KNQ5mxOgXwxwKw6lGMaiV\n" + + "JTxpTKqym93whYk93l3ocEe55c0CAwEAAaOBnjCBmzAJBgNVHRMEAjAAMCwGCWCG\n" + + "SAGG+EIBDQQfFh1PcGVuU1NMIEdlbmVyYXRlZCBDZXJ0aWZpY2F0ZTAdBgNVHQ4E\n" + + "FgQUnxR3vz86tso4gkJIFiza0Mteh9gwHwYDVR0jBBgwFoAUe5raj5CZTlLSrNuz\n" + + "A1LKh6YNPg0wIAYDVR0RBBkwF4IHYmFyLmNvbYIM6Iqx5a2QLmNvLmpwMA0GCSqG\n" + + "SIb3DQEBBQUAA4IBAQBeZs7ZIYyKtdnVxVvdLgwySEPOE4pBSXii7XYv0Q9QUvG/\n" + + "++gFGQh89HhABzA1mVUjH5dJTQqSLFvRfqTHqLpxSxSWqMHnvRM4cPBkIRp/XlMK\n" + + "PlXadYtJLPTgpbgvulA1ickC9EwlNYWnowZ4uxnfsMghW4HskBqaV+PnQ8Zvy3L0\n" + + "12c7Cg4mKKS5pb1HdRuiD2opZ+Hc77gRQLvtWNS8jQvd/iTbh6fuvTKfAOFoXw22\n" + + "sWIKHYrmhCIRshUNohGXv50m2o+1w9oWmQ6Dkq7lCjfXfUB4wIbggJjpyEtbNqBt\n" + + "j4MC2x5rfsLKKqToKmNE7pFEgqwe8//Aar1b+Qj+\n" + + "-----END CERTIFICATE-----\n"); + assertTrue(verifier.verify("foo.com", session)); + assertFalse(verifier.verify("a.foo.com", session)); + // these checks test alternative subjects. The test data contains an + // alternative subject starting with a japanese kanji character. This is + // not supported by Android because the underlying implementation from + // harmony follows the definition from rfc 1034 page 10 for alternative + // subject names. This causes the code to drop all alternative subjects. + // assertTrue(verifier.verify("bar.com", session)); + // assertFalse(verifier.verify("a.bar.com", session)); + // assertFalse(verifier.verify("a.\u82b1\u5b50.co.jp", session)); + } + + @Test public void verifySubjectAltOnly() throws Exception { + // subjectAlt=foo.com + SSLSession session = session("" + + "-----BEGIN CERTIFICATE-----\n" + + "MIIESjCCAzKgAwIBAgIJAIz+EYMBU6aYMA0GCSqGSIb3DQEBBQUAMIGiMQswCQYD\n" + + "VQQGEwJDQTELMAkGA1UECBMCQkMxEjAQBgNVBAcTCVZhbmNvdXZlcjEWMBQGA1UE\n" + + "ChMNd3d3LmN1Y2JjLmNvbTEUMBIGA1UECxQLY29tbW9uc19zc2wxHTAbBgNVBAMU\n" + + "FGRlbW9faW50ZXJtZWRpYXRlX2NhMSUwIwYJKoZIhvcNAQkBFhZqdWxpdXNkYXZp\n" + + "ZXNAZ21haWwuY29tMB4XDTA2MTIxMTE2MjYxMFoXDTI4MTEwNTE2MjYxMFowgZIx\n" + + "CzAJBgNVBAYTAlVTMREwDwYDVQQIDAhNYXJ5bGFuZDEUMBIGA1UEBwwLRm9yZXN0\n" + + "IEhpbGwxFzAVBgNVBAoMDmh0dHBjb21wb25lbnRzMRowGAYDVQQLDBF0ZXN0IGNl\n" + + "cnRpZmljYXRlczElMCMGCSqGSIb3DQEJARYWanVsaXVzZGF2aWVzQGdtYWlsLmNv\n" + + "bTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAMhjr5aCPoyp0R1iroWA\n" + + "fnEyBMGYWoCidH96yGPFjYLowez5aYKY1IOKTY2BlYho4O84X244QrZTRl8kQbYt\n" + + "xnGh4gSCD+Z8gjZ/gMvLUlhqOb+WXPAUHMB39GRyzerA/ZtrlUqf+lKo0uWcocxe\n" + + "Rc771KN8cPH3nHZ0rV0Hx4ZAZy6U4xxObe4rtSVY07hNKXAb2odnVqgzcYiDkLV8\n" + + "ilvEmoNWMWrp8UBqkTcpEhYhCYp3cTkgJwMSuqv8BqnGd87xQU3FVZI4tbtkB+Kz\n" + + "jD9zz8QCDJAfDjZHR03KNQ5mxOgXwxwKw6lGMaiVJTxpTKqym93whYk93l3ocEe5\n" + + "5c0CAwEAAaOBkDCBjTAJBgNVHRMEAjAAMCwGCWCGSAGG+EIBDQQfFh1PcGVuU1NM\n" + + "IEdlbmVyYXRlZCBDZXJ0aWZpY2F0ZTAdBgNVHQ4EFgQUnxR3vz86tso4gkJIFiza\n" + + "0Mteh9gwHwYDVR0jBBgwFoAUe5raj5CZTlLSrNuzA1LKh6YNPg0wEgYDVR0RBAsw\n" + + "CYIHZm9vLmNvbTANBgkqhkiG9w0BAQUFAAOCAQEAjl78oMjzFdsMy6F1sGg/IkO8\n" + + "tF5yUgPgFYrs41yzAca7IQu6G9qtFDJz/7ehh/9HoG+oqCCIHPuIOmS7Sd0wnkyJ\n" + + "Y7Y04jVXIb3a6f6AgBkEFP1nOT0z6kjT7vkA5LJ2y3MiDcXuRNMSta5PYVnrX8aZ\n" + + "yiqVUNi40peuZ2R8mAUSBvWgD7z2qWhF8YgDb7wWaFjg53I36vWKn90ZEti3wNCw\n" + + "qAVqixM+J0qJmQStgAc53i2aTMvAQu3A3snvH/PHTBo+5UL72n9S1kZyNCsVf1Qo\n" + + "n8jKTiRriEM+fMFlcgQP284EBFzYHyCXFb9O/hMjK2+6mY9euMB1U1aFFzM/Bg==\n" + + "-----END CERTIFICATE-----\n"); + assertTrue(verifier.verify("foo.com", session)); + assertFalse(verifier.verify("a.foo.com", session)); + assertTrue(verifier.verify("foo.com", session)); + assertFalse(verifier.verify("a.foo.com", session)); + } + + @Test public void verifyMultipleCn() throws Exception { + // CN=foo.com, CN=bar.com, CN=花子.co.jp + SSLSession session = session("" + + "-----BEGIN CERTIFICATE-----\n" + + "MIIEbzCCA1egAwIBAgIJAIz+EYMBU6aXMA0GCSqGSIb3DQEBBQUAMIGiMQswCQYD\n" + + "VQQGEwJDQTELMAkGA1UECBMCQkMxEjAQBgNVBAcTCVZhbmNvdXZlcjEWMBQGA1UE\n" + + "ChMNd3d3LmN1Y2JjLmNvbTEUMBIGA1UECxQLY29tbW9uc19zc2wxHTAbBgNVBAMU\n" + + "FGRlbW9faW50ZXJtZWRpYXRlX2NhMSUwIwYJKoZIhvcNAQkBFhZqdWxpdXNkYXZp\n" + + "ZXNAZ21haWwuY29tMB4XDTA2MTIxMTE2MTk0NVoXDTI4MTEwNTE2MTk0NVowgc0x\n" + + "CzAJBgNVBAYTAlVTMREwDwYDVQQIDAhNYXJ5bGFuZDEUMBIGA1UEBwwLRm9yZXN0\n" + + "IEhpbGwxFzAVBgNVBAoMDmh0dHBjb21wb25lbnRzMRowGAYDVQQLDBF0ZXN0IGNl\n" + + "cnRpZmljYXRlczEQMA4GA1UEAwwHZm9vLmNvbTEQMA4GA1UEAwwHYmFyLmNvbTEV\n" + + "MBMGA1UEAwwM6Iqx5a2QLmNvLmpwMSUwIwYJKoZIhvcNAQkBFhZqdWxpdXNkYXZp\n" + + "ZXNAZ21haWwuY29tMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAyGOv\n" + + "loI+jKnRHWKuhYB+cTIEwZhagKJ0f3rIY8WNgujB7PlpgpjUg4pNjYGViGjg7zhf\n" + + "bjhCtlNGXyRBti3GcaHiBIIP5nyCNn+Ay8tSWGo5v5Zc8BQcwHf0ZHLN6sD9m2uV\n" + + "Sp/6UqjS5ZyhzF5FzvvUo3xw8fecdnStXQfHhkBnLpTjHE5t7iu1JVjTuE0pcBva\n" + + "h2dWqDNxiIOQtXyKW8Sag1YxaunxQGqRNykSFiEJindxOSAnAxK6q/wGqcZ3zvFB\n" + + "TcVVkji1u2QH4rOMP3PPxAIMkB8ONkdHTco1DmbE6BfDHArDqUYxqJUlPGlMqrKb\n" + + "3fCFiT3eXehwR7nlzQIDAQABo3sweTAJBgNVHRMEAjAAMCwGCWCGSAGG+EIBDQQf\n" + + "Fh1PcGVuU1NMIEdlbmVyYXRlZCBDZXJ0aWZpY2F0ZTAdBgNVHQ4EFgQUnxR3vz86\n" + + "tso4gkJIFiza0Mteh9gwHwYDVR0jBBgwFoAUe5raj5CZTlLSrNuzA1LKh6YNPg0w\n" + + "DQYJKoZIhvcNAQEFBQADggEBAGuZb8ai1NO2j4v3y9TLZvd5s0vh5/TE7n7RX+8U\n" + + "y37OL5k7x9nt0mM1TyAKxlCcY+9h6frue8MemZIILSIvMrtzccqNz0V1WKgA+Orf\n" + + "uUrabmn+CxHF5gpy6g1Qs2IjVYWA5f7FROn/J+Ad8gJYc1azOWCLQqSyfpNRLSvY\n" + + "EriQFEV63XvkJ8JrG62b+2OT2lqT4OO07gSPetppdlSa8NBSKP6Aro9RIX1ZjUZQ\n" + + "SpQFCfo02NO0uNRDPUdJx2huycdNb+AXHaO7eXevDLJ+QnqImIzxWiY6zLOdzjjI\n" + + "VBMkLHmnP7SjGSQ3XA4ByrQOxfOUTyLyE7NuemhHppuQPxE=\n" + + "-----END CERTIFICATE-----\n"); + assertFalse(verifier.verify("foo.com", session)); + assertFalse(verifier.verify("a.foo.com", session)); + assertFalse(verifier.verify("bar.com", session)); + assertFalse(verifier.verify("a.bar.com", session)); + // Android-changed: Ignore common name in hostname verification. http://b/70278814 + // assertTrue(verifier.verify("\u82b1\u5b50.co.jp", session)); + assertFalse(verifier.verify("\u82b1\u5b50.co.jp", session)); + assertFalse(verifier.verify("a.\u82b1\u5b50.co.jp", session)); + } + + @Test public void verifyWilcardCn() throws Exception { + // CN=*.foo.com + SSLSession session = session("" + + "-----BEGIN CERTIFICATE-----\n" + + "MIIESDCCAzCgAwIBAgIJAIz+EYMBU6aUMA0GCSqGSIb3DQEBBQUAMIGiMQswCQYD\n" + + "VQQGEwJDQTELMAkGA1UECBMCQkMxEjAQBgNVBAcTCVZhbmNvdXZlcjEWMBQGA1UE\n" + + "ChMNd3d3LmN1Y2JjLmNvbTEUMBIGA1UECxQLY29tbW9uc19zc2wxHTAbBgNVBAMU\n" + + "FGRlbW9faW50ZXJtZWRpYXRlX2NhMSUwIwYJKoZIhvcNAQkBFhZqdWxpdXNkYXZp\n" + + "ZXNAZ21haWwuY29tMB4XDTA2MTIxMTE2MTU1NVoXDTI4MTEwNTE2MTU1NVowgaYx\n" + + "CzAJBgNVBAYTAlVTMREwDwYDVQQIEwhNYXJ5bGFuZDEUMBIGA1UEBxMLRm9yZXN0\n" + + "IEhpbGwxFzAVBgNVBAoTDmh0dHBjb21wb25lbnRzMRowGAYDVQQLExF0ZXN0IGNl\n" + + "cnRpZmljYXRlczESMBAGA1UEAxQJKi5mb28uY29tMSUwIwYJKoZIhvcNAQkBFhZq\n" + + "dWxpdXNkYXZpZXNAZ21haWwuY29tMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIB\n" + + "CgKCAQEAyGOvloI+jKnRHWKuhYB+cTIEwZhagKJ0f3rIY8WNgujB7PlpgpjUg4pN\n" + + "jYGViGjg7zhfbjhCtlNGXyRBti3GcaHiBIIP5nyCNn+Ay8tSWGo5v5Zc8BQcwHf0\n" + + "ZHLN6sD9m2uVSp/6UqjS5ZyhzF5FzvvUo3xw8fecdnStXQfHhkBnLpTjHE5t7iu1\n" + + "JVjTuE0pcBvah2dWqDNxiIOQtXyKW8Sag1YxaunxQGqRNykSFiEJindxOSAnAxK6\n" + + "q/wGqcZ3zvFBTcVVkji1u2QH4rOMP3PPxAIMkB8ONkdHTco1DmbE6BfDHArDqUYx\n" + + "qJUlPGlMqrKb3fCFiT3eXehwR7nlzQIDAQABo3sweTAJBgNVHRMEAjAAMCwGCWCG\n" + + "SAGG+EIBDQQfFh1PcGVuU1NMIEdlbmVyYXRlZCBDZXJ0aWZpY2F0ZTAdBgNVHQ4E\n" + + "FgQUnxR3vz86tso4gkJIFiza0Mteh9gwHwYDVR0jBBgwFoAUe5raj5CZTlLSrNuz\n" + + "A1LKh6YNPg0wDQYJKoZIhvcNAQEFBQADggEBAH0ipG6J561UKUfgkeW7GvYwW98B\n" + + "N1ZooWX+JEEZK7+Pf/96d3Ij0rw9ACfN4bpfnCq0VUNZVSYB+GthQ2zYuz7tf/UY\n" + + "A6nxVgR/IjG69BmsBl92uFO7JTNtHztuiPqBn59pt+vNx4yPvno7zmxsfI7jv0ww\n" + + "yfs+0FNm7FwdsC1k47GBSOaGw38kuIVWqXSAbL4EX9GkryGGOKGNh0qvAENCdRSB\n" + + "G9Z6tyMbmfRY+dLSh3a9JwoEcBUso6EWYBakLbq4nG/nvYdYvG9ehrnLVwZFL82e\n" + + "l3Q/RK95bnA6cuRClGusLad0e6bjkBzx/VQ3VarDEpAkTLUGVAa0CLXtnyc=\n" + + "-----END CERTIFICATE-----\n"); + assertFalse(verifier.verify("foo.com", session)); + // Android-changed: Ignore common name in hostname verification. http://b/70278814 + // assertTrue(verifier.verify("www.foo.com", session)); + assertFalse(verifier.verify("www.foo.com", session)); + // Android-changed: Ignore common name in hostname verification. http://b/70278814 + // assertTrue(verifier.verify("\u82b1\u5b50.foo.com", session)); + assertFalse(verifier.verify("\u82b1\u5b50.foo.com", session)); + assertFalse(verifier.verify("a.b.foo.com", session)); + } + + @Test public void verifyWilcardCnOnTld() throws Exception { + // It's the CA's responsibility to not issue broad-matching certificates! + // CN=*.co.jp + SSLSession session = session("" + + "-----BEGIN CERTIFICATE-----\n" + + "MIIERjCCAy6gAwIBAgIJAIz+EYMBU6aVMA0GCSqGSIb3DQEBBQUAMIGiMQswCQYD\n" + + "VQQGEwJDQTELMAkGA1UECBMCQkMxEjAQBgNVBAcTCVZhbmNvdXZlcjEWMBQGA1UE\n" + + "ChMNd3d3LmN1Y2JjLmNvbTEUMBIGA1UECxQLY29tbW9uc19zc2wxHTAbBgNVBAMU\n" + + "FGRlbW9faW50ZXJtZWRpYXRlX2NhMSUwIwYJKoZIhvcNAQkBFhZqdWxpdXNkYXZp\n" + + "ZXNAZ21haWwuY29tMB4XDTA2MTIxMTE2MTYzMFoXDTI4MTEwNTE2MTYzMFowgaQx\n" + + "CzAJBgNVBAYTAlVTMREwDwYDVQQIEwhNYXJ5bGFuZDEUMBIGA1UEBxMLRm9yZXN0\n" + + "IEhpbGwxFzAVBgNVBAoTDmh0dHBjb21wb25lbnRzMRowGAYDVQQLExF0ZXN0IGNl\n" + + "cnRpZmljYXRlczEQMA4GA1UEAxQHKi5jby5qcDElMCMGCSqGSIb3DQEJARYWanVs\n" + + "aXVzZGF2aWVzQGdtYWlsLmNvbTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoC\n" + + "ggEBAMhjr5aCPoyp0R1iroWAfnEyBMGYWoCidH96yGPFjYLowez5aYKY1IOKTY2B\n" + + "lYho4O84X244QrZTRl8kQbYtxnGh4gSCD+Z8gjZ/gMvLUlhqOb+WXPAUHMB39GRy\n" + + "zerA/ZtrlUqf+lKo0uWcocxeRc771KN8cPH3nHZ0rV0Hx4ZAZy6U4xxObe4rtSVY\n" + + "07hNKXAb2odnVqgzcYiDkLV8ilvEmoNWMWrp8UBqkTcpEhYhCYp3cTkgJwMSuqv8\n" + + "BqnGd87xQU3FVZI4tbtkB+KzjD9zz8QCDJAfDjZHR03KNQ5mxOgXwxwKw6lGMaiV\n" + + "JTxpTKqym93whYk93l3ocEe55c0CAwEAAaN7MHkwCQYDVR0TBAIwADAsBglghkgB\n" + + "hvhCAQ0EHxYdT3BlblNTTCBHZW5lcmF0ZWQgQ2VydGlmaWNhdGUwHQYDVR0OBBYE\n" + + "FJ8Ud78/OrbKOIJCSBYs2tDLXofYMB8GA1UdIwQYMBaAFHua2o+QmU5S0qzbswNS\n" + + "yoemDT4NMA0GCSqGSIb3DQEBBQUAA4IBAQA0sWglVlMx2zNGvUqFC73XtREwii53\n" + + "CfMM6mtf2+f3k/d8KXhLNySrg8RRlN11zgmpPaLtbdTLrmG4UdAHHYr8O4y2BBmE\n" + + "1cxNfGxxechgF8HX10QV4dkyzp6Z1cfwvCeMrT5G/V1pejago0ayXx+GPLbWlNeZ\n" + + "S+Kl0m3p+QplXujtwG5fYcIpaGpiYraBLx3Tadih39QN65CnAh/zRDhLCUzKyt9l\n" + + "UGPLEUDzRHMPHLnSqT1n5UU5UDRytbjJPXzF+l/+WZIsanefWLsxnkgAuZe/oMMF\n" + + "EJMryEzOjg4Tfuc5qM0EXoPcQ/JlheaxZ40p2IyHqbsWV4MRYuFH4bkM\n" + + "-----END CERTIFICATE-----\n"); + // Android-changed: Ignore common name in hostname verification. http://b/70278814 + // assertTrue(verifier.verify("foo.co.jp", session)); + assertFalse(verifier.verify("foo.co.jp", session)); + // Android-changed: Ignore common name in hostname verification. http://b/70278814 + // assertTrue(verifier.verify("\u82b1\u5b50.co.jp", session)); + assertFalse(verifier.verify("\u82b1\u5b50.co.jp", session)); + } + + /** + * Ignored due to incompatibilities between Android and Java on how non-ASCII + * subject alt names are parsed. Android fails to parse these, which means we + * fall back to the CN. The RI does parse them, so the CN is unused. + */ + @Test @Ignore public void testWilcardNonAsciiSubjectAlt() throws Exception { + // CN=*.foo.com, subjectAlt=*.bar.com, subjectAlt=*.花子.co.jp + // (*.hanako.co.jp in kanji) + SSLSession session = session("" + + "-----BEGIN CERTIFICATE-----\n" + + "MIIEcDCCA1igAwIBAgIJAIz+EYMBU6aWMA0GCSqGSIb3DQEBBQUAMIGiMQswCQYD\n" + + "VQQGEwJDQTELMAkGA1UECBMCQkMxEjAQBgNVBAcTCVZhbmNvdXZlcjEWMBQGA1UE\n" + + "ChMNd3d3LmN1Y2JjLmNvbTEUMBIGA1UECxQLY29tbW9uc19zc2wxHTAbBgNVBAMU\n" + + "FGRlbW9faW50ZXJtZWRpYXRlX2NhMSUwIwYJKoZIhvcNAQkBFhZqdWxpdXNkYXZp\n" + + "ZXNAZ21haWwuY29tMB4XDTA2MTIxMTE2MTczMVoXDTI4MTEwNTE2MTczMVowgaYx\n" + + "CzAJBgNVBAYTAlVTMREwDwYDVQQIEwhNYXJ5bGFuZDEUMBIGA1UEBxMLRm9yZXN0\n" + + "IEhpbGwxFzAVBgNVBAoTDmh0dHBjb21wb25lbnRzMRowGAYDVQQLExF0ZXN0IGNl\n" + + "cnRpZmljYXRlczESMBAGA1UEAxQJKi5mb28uY29tMSUwIwYJKoZIhvcNAQkBFhZq\n" + + "dWxpdXNkYXZpZXNAZ21haWwuY29tMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIB\n" + + "CgKCAQEAyGOvloI+jKnRHWKuhYB+cTIEwZhagKJ0f3rIY8WNgujB7PlpgpjUg4pN\n" + + "jYGViGjg7zhfbjhCtlNGXyRBti3GcaHiBIIP5nyCNn+Ay8tSWGo5v5Zc8BQcwHf0\n" + + "ZHLN6sD9m2uVSp/6UqjS5ZyhzF5FzvvUo3xw8fecdnStXQfHhkBnLpTjHE5t7iu1\n" + + "JVjTuE0pcBvah2dWqDNxiIOQtXyKW8Sag1YxaunxQGqRNykSFiEJindxOSAnAxK6\n" + + "q/wGqcZ3zvFBTcVVkji1u2QH4rOMP3PPxAIMkB8ONkdHTco1DmbE6BfDHArDqUYx\n" + + "qJUlPGlMqrKb3fCFiT3eXehwR7nlzQIDAQABo4GiMIGfMAkGA1UdEwQCMAAwLAYJ\n" + + "YIZIAYb4QgENBB8WHU9wZW5TU0wgR2VuZXJhdGVkIENlcnRpZmljYXRlMB0GA1Ud\n" + + "DgQWBBSfFHe/Pzq2yjiCQkgWLNrQy16H2DAfBgNVHSMEGDAWgBR7mtqPkJlOUtKs\n" + + "27MDUsqHpg0+DTAkBgNVHREEHTAbggkqLmJhci5jb22CDiou6Iqx5a2QLmNvLmpw\n" + + "MA0GCSqGSIb3DQEBBQUAA4IBAQBobWC+D5/lx6YhX64CwZ26XLjxaE0S415ajbBq\n" + + "DK7lz+Rg7zOE3GsTAMi+ldUYnhyz0wDiXB8UwKXl0SDToB2Z4GOgqQjAqoMmrP0u\n" + + "WB6Y6dpkfd1qDRUzI120zPYgSdsXjHW9q2H77iV238hqIU7qCvEz+lfqqWEY504z\n" + + "hYNlknbUnR525ItosEVwXFBJTkZ3Yw8gg02c19yi8TAh5Li3Ad8XQmmSJMWBV4XK\n" + + "qFr0AIZKBlg6NZZFf/0dP9zcKhzSriW27bY0XfzA6GSiRDXrDjgXq6baRT6YwgIg\n" + + "pgJsDbJtZfHnV1nd3M6zOtQPm1TIQpNmMMMd/DPrGcUQerD3\n" + + "-----END CERTIFICATE-----\n"); + // try the foo.com variations + assertTrue(verifier.verify("foo.com", session)); + assertTrue(verifier.verify("www.foo.com", session)); + assertTrue(verifier.verify("\u82b1\u5b50.foo.com", session)); + assertFalse(verifier.verify("a.b.foo.com", session)); + // these checks test alternative subjects. The test data contains an + // alternative subject starting with a japanese kanji character. This is + // not supported by Android because the underlying implementation from + // harmony follows the definition from rfc 1034 page 10 for alternative + // subject names. This causes the code to drop all alternative subjects. + // assertFalse(verifier.verify("bar.com", session)); + // assertTrue(verifier.verify("www.bar.com", session)); + // assertTrue(verifier.verify("\u82b1\u5b50.bar.com", session)); + // assertTrue(verifier.verify("a.b.bar.com", session)); + } + + @Test public void subjectAltUsesLocalDomainAndIp() throws Exception { + // cat cert.cnf + // [req] + // distinguished_name=distinguished_name + // req_extensions=req_extensions + // x509_extensions=x509_extensions + // [distinguished_name] + // [req_extensions] + // [x509_extensions] + // subjectAltName=DNS:localhost.localdomain,DNS:localhost,IP:127.0.0.1 + // + // $ openssl req -x509 -nodes -days 36500 -subj '/CN=localhost' -config ./cert.cnf \ + // -newkey rsa:512 -out cert.pem + X509Certificate certificate = certificate("" + + "-----BEGIN CERTIFICATE-----\n" + + "MIIBWDCCAQKgAwIBAgIJANS1EtICX2AZMA0GCSqGSIb3DQEBBQUAMBQxEjAQBgNV\n" + + "BAMTCWxvY2FsaG9zdDAgFw0xMjAxMDIxOTA4NThaGA8yMTExMTIwOTE5MDg1OFow\n" + + "FDESMBAGA1UEAxMJbG9jYWxob3N0MFwwDQYJKoZIhvcNAQEBBQADSwAwSAJBAPpt\n" + + "atK8r4/hf4hSIs0os/BSlQLbRBaK9AfBReM4QdAklcQqe6CHsStKfI8pp0zs7Ptg\n" + + "PmMdpbttL0O7mUboBC8CAwEAAaM1MDMwMQYDVR0RBCowKIIVbG9jYWxob3N0Lmxv\n" + + "Y2FsZG9tYWlugglsb2NhbGhvc3SHBH8AAAEwDQYJKoZIhvcNAQEFBQADQQD0ntfL\n" + + "DCzOCv9Ma6Lv5o5jcYWVxvBSTsnt22hsJpWD1K7iY9lbkLwl0ivn73pG2evsAn9G\n" + + "X8YKH52fnHsCrhSD\n" + + "-----END CERTIFICATE-----"); + assertEquals(new X500Principal("CN=localhost"), certificate.getSubjectX500Principal()); + + FakeSSLSession session = new FakeSSLSession(certificate); + assertTrue(verifier.verify("localhost", session)); + assertTrue(verifier.verify("localhost.localdomain", session)); + assertFalse(verifier.verify("local.host", session)); + + assertTrue(verifier.verify("127.0.0.1", session)); + assertFalse(verifier.verify("127.0.0.2", session)); + } + + @Test public void wildcardsCannotMatchIpAddresses() throws Exception { + // openssl req -x509 -nodes -days 36500 -subj '/CN=*.0.0.1' -newkey rsa:512 -out cert.pem + SSLSession session = session("" + + "-----BEGIN CERTIFICATE-----\n" + + "MIIBkjCCATygAwIBAgIJAMdemqOwd/BEMA0GCSqGSIb3DQEBBQUAMBIxEDAOBgNV\n" + + "BAMUByouMC4wLjEwIBcNMTAxMjIwMTY0NDI1WhgPMjExMDExMjYxNjQ0MjVaMBIx\n" + + "EDAOBgNVBAMUByouMC4wLjEwXDANBgkqhkiG9w0BAQEFAANLADBIAkEAqY8c9Qrt\n" + + "YPWCvb7lclI+aDHM6fgbJcHsS9Zg8nUOh5dWrS7AgeA25wyaokFl4plBbbHQe2j+\n" + + "cCjsRiJIcQo9HwIDAQABo3MwcTAdBgNVHQ4EFgQUJ436TZPJvwCBKklZZqIvt1Yt\n" + + "JjEwQgYDVR0jBDswOYAUJ436TZPJvwCBKklZZqIvt1YtJjGhFqQUMBIxEDAOBgNV\n" + + "BAMUByouMC4wLjGCCQDHXpqjsHfwRDAMBgNVHRMEBTADAQH/MA0GCSqGSIb3DQEB\n" + + "BQUAA0EAk9i88xdjWoewqvE+iMC9tD2obMchgFDaHH0ogxxiRaIKeEly3g0uGxIt\n" + + "fl2WRY8hb4x+zRrwsFaLEpdEvqcjOQ==\n" + + "-----END CERTIFICATE-----"); + assertFalse(verifier.verify("127.0.0.1", session)); + } + + /** + * Earlier implementations of Android's hostname verifier required that + * wildcard names wouldn't match "*.com" or similar. This was a nonstandard + * check that we've since dropped. It is the CA's responsibility to not hand + * out certificates that match so broadly. + */ + @Test public void wildcardsDoesNotNeedTwoDots() throws Exception { + // openssl req -x509 -nodes -days 36500 -subj '/CN=*.com' -newkey rsa:512 -out cert.pem + SSLSession session = session("" + + "-----BEGIN CERTIFICATE-----\n" + + "MIIBjDCCATagAwIBAgIJAOVulXCSu6HuMA0GCSqGSIb3DQEBBQUAMBAxDjAMBgNV\n" + + "BAMUBSouY29tMCAXDTEwMTIyMDE2NDkzOFoYDzIxMTAxMTI2MTY0OTM4WjAQMQ4w\n" + + "DAYDVQQDFAUqLmNvbTBcMA0GCSqGSIb3DQEBAQUAA0sAMEgCQQDJd8xqni+h7Iaz\n" + + "ypItivs9kPuiJUqVz+SuJ1C05SFc3PmlRCvwSIfhyD67fHcbMdl+A/LrIjhhKZJe\n" + + "1joO0+pFAgMBAAGjcTBvMB0GA1UdDgQWBBS4Iuzf5w8JdCp+EtBfdFNudf6+YzBA\n" + + "BgNVHSMEOTA3gBS4Iuzf5w8JdCp+EtBfdFNudf6+Y6EUpBIwEDEOMAwGA1UEAxQF\n" + + "Ki5jb22CCQDlbpVwkruh7jAMBgNVHRMEBTADAQH/MA0GCSqGSIb3DQEBBQUAA0EA\n" + + "U6LFxmZr31lFyis2/T68PpjAppc0DpNQuA2m/Y7oTHBDi55Fw6HVHCw3lucuWZ5d\n" + + "qUYo4ES548JdpQtcLrW2sA==\n" + + "-----END CERTIFICATE-----"); + // Android-changed: Ignore common name in hostname verification. http://b/70278814 + // assertTrue(verifier.verify("google.com", session)); + assertFalse(verifier.verify("google.com", session)); + } + + @Test public void subjectAltName() throws Exception { + // $ cat ./cert.cnf + // [req] + // distinguished_name=distinguished_name + // req_extensions=req_extensions + // x509_extensions=x509_extensions + // [distinguished_name] + // [req_extensions] + // [x509_extensions] + // subjectAltName=DNS:bar.com,DNS:baz.com + // + // $ openssl req -x509 -nodes -days 36500 -subj '/CN=foo.com' -config ./cert.cnf \ + // -newkey rsa:512 -out cert.pem + SSLSession session = session("" + + "-----BEGIN CERTIFICATE-----\n" + + "MIIBPTCB6KADAgECAgkA7zoHaaqNGHQwDQYJKoZIhvcNAQEFBQAwEjEQMA4GA1UE\n" + + "AxMHZm9vLmNvbTAgFw0xMDEyMjAxODM5MzZaGA8yMTEwMTEyNjE4MzkzNlowEjEQ\n" + + "MA4GA1UEAxMHZm9vLmNvbTBcMA0GCSqGSIb3DQEBAQUAA0sAMEgCQQC+gmoSxF+8\n" + + "hbV+rgRQqHIJd50216OWQJbU3BvdlPbca779NYO4+UZWTFdBM8BdQqs3H4B5Agvp\n" + + "y7HeSff1F7XRAgMBAAGjHzAdMBsGA1UdEQQUMBKCB2Jhci5jb22CB2Jhei5jb20w\n" + + "DQYJKoZIhvcNAQEFBQADQQBXpZZPOY2Dy1lGG81JTr8L4or9jpKacD7n51eS8iqI\n" + + "oTznPNuXHU5bFN0AAGX2ij47f/EahqTpo5RdS95P4sVm\n" + + "-----END CERTIFICATE-----"); + assertFalse(verifier.verify("foo.com", session)); + assertTrue(verifier.verify("bar.com", session)); + assertTrue(verifier.verify("baz.com", session)); + assertFalse(verifier.verify("a.foo.com", session)); + assertFalse(verifier.verify("quux.com", session)); + } + + @Test public void subjectAltNameWithWildcard() throws Exception { + // $ cat ./cert.cnf + // [req] + // distinguished_name=distinguished_name + // req_extensions=req_extensions + // x509_extensions=x509_extensions + // [distinguished_name] + // [req_extensions] + // [x509_extensions] + // subjectAltName=DNS:bar.com,DNS:*.baz.com + // + // $ openssl req -x509 -nodes -days 36500 -subj '/CN=foo.com' -config ./cert.cnf \ + // -newkey rsa:512 -out cert.pem + SSLSession session = session("" + + "-----BEGIN CERTIFICATE-----\n" + + "MIIBPzCB6qADAgECAgkAnv/7Jv5r7pMwDQYJKoZIhvcNAQEFBQAwEjEQMA4GA1UE\n" + + "AxMHZm9vLmNvbTAgFw0xMDEyMjAxODQ2MDFaGA8yMTEwMTEyNjE4NDYwMVowEjEQ\n" + + "MA4GA1UEAxMHZm9vLmNvbTBcMA0GCSqGSIb3DQEBAQUAA0sAMEgCQQDAz2YXnyog\n" + + "YdYLSFr/OEgSumtwqtZKJTB4wqTW/eKbBCEzxnyUMxWZIqUGu353PzwfOuWp2re3\n" + + "nvVV+QDYQlh9AgMBAAGjITAfMB0GA1UdEQQWMBSCB2Jhci5jb22CCSouYmF6LmNv\n" + + "bTANBgkqhkiG9w0BAQUFAANBAB8yrSl8zqy07i0SNYx2B/FnvQY734pxioaqFWfO\n" + + "Bqo1ZZl/9aPHEWIwBrxYNVB0SGu/kkbt/vxqOjzzrkXukmI=\n" + + "-----END CERTIFICATE-----"); + assertFalse(verifier.verify("foo.com", session)); + assertTrue(verifier.verify("bar.com", session)); + assertTrue(verifier.verify("a.baz.com", session)); + assertFalse(verifier.verify("baz.com", session)); + assertFalse(verifier.verify("a.foo.com", session)); + assertFalse(verifier.verify("a.bar.com", session)); + assertFalse(verifier.verify("quux.com", session)); + } + + // BEGIN Android-added: Verify behaviour with top level wildcard SAN. http://b/144694112 + @Test + public void subjectAltNameWithToplevelWildcard() throws Exception { + // Default OkHostnameVerifier instance should allow SANs which + // have wildcards for top-level domains. The strict instance should not. + // + // Certificate generated using:- + // openssl req -x509 -nodes -days 36500 -subj "/CN=Google Inc" \ + // -addext "subjectAltName=DNS:*.com" -newkey rsa:512 + SSLSession session = session("" + + "-----BEGIN CERTIFICATE-----\n" + + "MIIBlTCCAT+gAwIBAgIUe1RB6C61ZW/SEQpKiywSEJOEOUMwDQYJKoZIhvcNAQEL\n" + + "BQAwFTETMBEGA1UEAwwKR29vZ2xlIEluYzAgFw0xOTExMjExMjE1NTBaGA8yMTE5\n" + + "MTAyODEyMTU1MFowFTETMBEGA1UEAwwKR29vZ2xlIEluYzBcMA0GCSqGSIb3DQEB\n" + + "AQUAA0sAMEgCQQCu24jT8hktpvnmcde4dqC6e7G5F4cNNLUFnTi3Ay9BzPH1r7sN\n" + + "v2lHTIQLKSlvjxa48mpeRBlOjDQigv7c+rfRAgMBAAGjZTBjMB0GA1UdDgQWBBQd\n" + + "myvYKfluxb0+kNEJoh1ZER2wUTAfBgNVHSMEGDAWgBQdmyvYKfluxb0+kNEJoh1Z\n" + + "ER2wUTAPBgNVHRMBAf8EBTADAQH/MBAGA1UdEQQJMAeCBSouY29tMA0GCSqGSIb3\n" + + "DQEBCwUAA0EAK710g2hQpXSmpbOQH4dHG61fkVDtM/kR/4/R61vDDqVkgOuyHqXl\n" + + "GUZFKHMeOZ8peQLT8b+5ik6pIO7Vu2pF6w==\n" + + "-----END CERTIFICATE-----\n"); + assertTrue(OkHostnameVerifier.INSTANCE.verify("google.com", session)); + assertFalse(OkHostnameVerifier.strictInstance().verify("google.com", session)); + } + // END Android-added: Verify behaviour with top level wildcard SAN. http://b/144694112 + + @Test public void verifyAsIpAddress() { + // IPv4 + assertTrue(OkHostnameVerifier.verifyAsIpAddress("127.0.0.1")); + assertTrue(OkHostnameVerifier.verifyAsIpAddress("1.2.3.4")); + + // IPv6 + assertTrue(OkHostnameVerifier.verifyAsIpAddress("::1")); + assertTrue(OkHostnameVerifier.verifyAsIpAddress("2001:db8::1")); + assertTrue(OkHostnameVerifier.verifyAsIpAddress("::192.168.0.1")); + assertTrue(OkHostnameVerifier.verifyAsIpAddress("::ffff:192.168.0.1")); + assertTrue(OkHostnameVerifier.verifyAsIpAddress("FEDC:BA98:7654:3210:FEDC:BA98:7654:3210")); + assertTrue(OkHostnameVerifier.verifyAsIpAddress("1080:0:0:0:8:800:200C:417A")); + assertTrue(OkHostnameVerifier.verifyAsIpAddress("1080::8:800:200C:417A")); + assertTrue(OkHostnameVerifier.verifyAsIpAddress("FF01::101")); + assertTrue(OkHostnameVerifier.verifyAsIpAddress("0:0:0:0:0:0:13.1.68.3")); + assertTrue(OkHostnameVerifier.verifyAsIpAddress("0:0:0:0:0:FFFF:129.144.52.38")); + assertTrue(OkHostnameVerifier.verifyAsIpAddress("::13.1.68.3")); + assertTrue(OkHostnameVerifier.verifyAsIpAddress("::FFFF:129.144.52.38")); + + // Hostnames + assertFalse(OkHostnameVerifier.verifyAsIpAddress("go")); + assertFalse(OkHostnameVerifier.verifyAsIpAddress("localhost")); + assertFalse(OkHostnameVerifier.verifyAsIpAddress("squareup.com")); + assertFalse(OkHostnameVerifier.verifyAsIpAddress("www.nintendo.co.jp")); + } + + private X509Certificate certificate(String certificate) throws Exception { + return (X509Certificate) CertificateFactory.getInstance("X.509").generateCertificate( + new ByteArrayInputStream(certificate.getBytes(UTF_8))); + } + + private SSLSession session(String certificate) throws Exception { + return new FakeSSLSession(certificate(certificate)); + } +} \ No newline at end of file diff --git a/common/src/test/java/org/conscrypt/TrustManagerImplTest.java b/common/src/test/java/org/conscrypt/TrustManagerImplTest.java index 17920fa4e..0f5a2d813 100644 --- a/common/src/test/java/org/conscrypt/TrustManagerImplTest.java +++ b/common/src/test/java/org/conscrypt/TrustManagerImplTest.java @@ -518,7 +518,7 @@ public boolean getEnableSessionCreation() { } } - private static class TestHostnameVerifier - extends org.conscrypt.javax.net.ssl.TestHostnameVerifier - implements ConscryptHostnameVerifier {} +// private static class TestHostnameVerifier +// extends org.conscrypt.javax.net.ssl.TestHostnameVerifier +// implements ConscryptHostnameVerifier {} } From a3d93d3f68a51053a9407dad02f2fda1df70d91f Mon Sep 17 00:00:00 2001 From: Danny Reidenbach Date: Tue, 28 Jul 2020 20:46:29 +0000 Subject: [PATCH 08/20] Fixed AOSP style and added HostnameVerifierTest --- .../org/conscrypt/OkHostnameVerifier.java | 12 +- .../java/org/conscrypt/TrustManagerImpl.java | 16 +-- .../java/org/conscrypt/ConscryptSuite.java | 1 + .../java/org/conscrypt/FakeSSLSession.java | 120 ------------------ .../org/conscrypt/HostnameVerifierTest.java | 22 +++- .../javax/net/ssl/FakeSSLSession.java | 3 +- 6 files changed, 37 insertions(+), 137 deletions(-) delete mode 100644 common/src/test/java/org/conscrypt/FakeSSLSession.java diff --git a/common/src/main/java/org/conscrypt/OkHostnameVerifier.java b/common/src/main/java/org/conscrypt/OkHostnameVerifier.java index f5b987008..5e97cb1af 100644 --- a/common/src/main/java/org/conscrypt/OkHostnameVerifier.java +++ b/common/src/main/java/org/conscrypt/OkHostnameVerifier.java @@ -37,7 +37,7 @@ public final class OkHostnameVerifier implements ConscryptHostnameVerifier { // Android-changed: Add a mode which disallows top-level domain wildcards. b/144694112 // public static final OkHostnameVerifier INSTANCE = new OkHostnameVerifier(); - public static final OkHostnameVerifier INSTANCE = new OkHostnameVerifier(true); + public static final OkHostnameVerifier INSTANCE = new OkHostnameVerifier(false); /** * Quick and dirty pattern to differentiate IP addresses from hostnames. This @@ -180,13 +180,13 @@ private static List getSubjectAltNames(X509Certificate certificate, int private boolean verifyHostName(String hostName, String pattern) { // Basic sanity checks // Check length == 0 instead of .isEmpty() to support Java 5. - if ((hostName == null) || (hostName.length() == 0) || (hostName.startsWith(".")) - || (hostName.endsWith(".."))) { + if (hostName == null || hostName.length() == 0 || hostName.startsWith(".") + || hostName.endsWith("..")) { // Invalid domain name return false; } - if ((pattern == null) || (pattern.length() == 0) || (pattern.startsWith(".")) - || (pattern.endsWith(".."))) { + if (pattern == null || pattern.length() == 0 || pattern.startsWith(".") + || pattern.endsWith("..")) { // Invalid pattern/domain name return false; } @@ -229,7 +229,7 @@ private boolean verifyHostName(String hostName, String pattern) { // 4. Android-added: if strictWildcardMode is true then wildcards matching top-level domains, // e.g. *.com, are not permitted. - if ((!pattern.startsWith("*.")) || (pattern.indexOf('*', 1) != -1)) { + if (!pattern.startsWith("*.") || pattern.indexOf('*', 1) != -1) { // Asterisk (*) is only permitted in the left-most domain name label and must be the only // character in that label return false; diff --git a/common/src/main/java/org/conscrypt/TrustManagerImpl.java b/common/src/main/java/org/conscrypt/TrustManagerImpl.java index c865e22dc..a4ffaf2e2 100644 --- a/common/src/main/java/org/conscrypt/TrustManagerImpl.java +++ b/common/src/main/java/org/conscrypt/TrustManagerImpl.java @@ -1002,14 +1002,14 @@ ConscryptHostnameVerifier getHostnameVerifier() { // doggo return hostnameVerifier; } - private enum GlobalHostnameVerifierAdapter implements ConscryptHostnameVerifier { - INSTANCE; - - @Override - public boolean verify(String hostname, SSLSession session) { - return HttpsURLConnection.getDefaultHostnameVerifier().verify(hostname, session); - } - } +// private enum GlobalHostnameVerifierAdapter implements ConscryptHostnameVerifier { +// INSTANCE; +// +// @Override +// public boolean verify(String hostname, SSLSession session) { +// return HttpsURLConnection.getDefaultHostnameVerifier().verify(hostname, session); +// } +// } private ConscryptHostnameVerifier getHttpsVerifier() { // doggo if (hostnameVerifier != null) { diff --git a/common/src/test/java/org/conscrypt/ConscryptSuite.java b/common/src/test/java/org/conscrypt/ConscryptSuite.java index 8591a6840..77379e1de 100644 --- a/common/src/test/java/org/conscrypt/ConscryptSuite.java +++ b/common/src/test/java/org/conscrypt/ConscryptSuite.java @@ -76,6 +76,7 @@ CertPinManagerTest.class, ChainStrengthAnalyzerTest.class, TrustManagerImplTest.class, + HostnameVerifierTest.class, // org.conscrypt.ct tests CTVerifierTest.class, SerializationTest.class, diff --git a/common/src/test/java/org/conscrypt/FakeSSLSession.java b/common/src/test/java/org/conscrypt/FakeSSLSession.java deleted file mode 100644 index 86372414e..000000000 --- a/common/src/test/java/org/conscrypt/FakeSSLSession.java +++ /dev/null @@ -1,120 +0,0 @@ -/* - * 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. - */ -package org.conscrypt; - -import java.security.Principal; -import java.security.cert.Certificate; -import javax.net.ssl.SSLPeerUnverifiedException; -import javax.net.ssl.SSLSession; -import javax.net.ssl.SSLSessionContext; -import javax.security.cert.X509Certificate; - -final class FakeSSLSession implements SSLSession { - private final Certificate[] certificates; - - public FakeSSLSession(Certificate... certificates) throws Exception { - this.certificates = certificates; - } - - public int getApplicationBufferSize() { - throw new UnsupportedOperationException(); - } - - public String getCipherSuite() { - throw new UnsupportedOperationException(); - } - - public long getCreationTime() { - throw new UnsupportedOperationException(); - } - - public byte[] getId() { - throw new UnsupportedOperationException(); - } - - public long getLastAccessedTime() { - throw new UnsupportedOperationException(); - } - - public Certificate[] getLocalCertificates() { - throw new UnsupportedOperationException(); - } - - public Principal getLocalPrincipal() { - throw new UnsupportedOperationException(); - } - - public int getPacketBufferSize() { - throw new UnsupportedOperationException(); - } - - public Certificate[] getPeerCertificates() throws SSLPeerUnverifiedException { - if (certificates.length == 0) { - throw new SSLPeerUnverifiedException("peer not authenticated"); - } else { - return certificates; - } - } - - public X509Certificate[] getPeerCertificateChain() throws SSLPeerUnverifiedException { - throw new UnsupportedOperationException(); - } - - public String getPeerHost() { - throw new UnsupportedOperationException(); - } - - public int getPeerPort() { - throw new UnsupportedOperationException(); - } - - public Principal getPeerPrincipal() throws SSLPeerUnverifiedException { - throw new UnsupportedOperationException(); - } - - public String getProtocol() { - throw new UnsupportedOperationException(); - } - - public SSLSessionContext getSessionContext() { - throw new UnsupportedOperationException(); - } - - public void putValue(String s, Object obj) { - throw new UnsupportedOperationException(); - } - - public void removeValue(String s) { - throw new UnsupportedOperationException(); - } - - public Object getValue(String s) { - throw new UnsupportedOperationException(); - } - - public String[] getValueNames() { - throw new UnsupportedOperationException(); - } - - public void invalidate() { - throw new UnsupportedOperationException(); - } - - public boolean isValid() { - throw new UnsupportedOperationException(); - } -} diff --git a/common/src/test/java/org/conscrypt/HostnameVerifierTest.java b/common/src/test/java/org/conscrypt/HostnameVerifierTest.java index a8fceb14e..d424bf27c 100644 --- a/common/src/test/java/org/conscrypt/HostnameVerifierTest.java +++ b/common/src/test/java/org/conscrypt/HostnameVerifierTest.java @@ -25,6 +25,7 @@ import java.util.Arrays; import java.util.Collection; import javax.net.ssl.HostnameVerifier; +import javax.net.ssl.SSLPeerUnverifiedException; import javax.net.ssl.SSLSession; import javax.security.auth.x500.X500Principal; import org.junit.Ignore; @@ -45,6 +46,24 @@ */ @RunWith(Parameterized.class) public final class HostnameVerifierTest { + public final class FakeSSLSession extends org.conscrypt.javax.net.ssl.FakeSSLSession { + + private final Certificate[] certificates; + + public FakeSSLSession(Certificate... certificates) throws Exception { + super("FakeHost"); + this.certificates = certificates; + } + + @Override + public Certificate[] getPeerCertificates() throws SSLPeerUnverifiedException { + if (certificates.length == 0) { + throw new SSLPeerUnverifiedException("peer not authenticated"); + } else { + return certificates; + } + } + } private static final Charset UTF_8 = Charset.forName("UTF-8"); // BEGIN Android-changed: Run tests for both default and strict verifiers. http://b/144694112 @@ -60,13 +79,12 @@ public static Collection data() { } @Parameter - public HostnameVerifier verifier; + public OkHostnameVerifier verifier; // END Android-changed: Run tests for both default and strict verifiers. http://b/144694112 @Test public void verify() throws Exception { FakeSSLSession session = new FakeSSLSession(); assertFalse(verifier.verify("localhost", session)); - assertEquals(1,0); } @Test public void verifyCn() throws Exception { diff --git a/testing/src/main/java/org/conscrypt/javax/net/ssl/FakeSSLSession.java b/testing/src/main/java/org/conscrypt/javax/net/ssl/FakeSSLSession.java index f83b62e71..f001db258 100644 --- a/testing/src/main/java/org/conscrypt/javax/net/ssl/FakeSSLSession.java +++ b/testing/src/main/java/org/conscrypt/javax/net/ssl/FakeSSLSession.java @@ -19,6 +19,7 @@ import java.nio.charset.Charset; import java.security.Principal; import java.security.cert.Certificate; +import javax.net.ssl.SSLPeerUnverifiedException; import javax.net.ssl.SSLSession; import javax.net.ssl.SSLSessionContext; @@ -76,7 +77,7 @@ public javax.security.cert.X509Certificate[] getPeerCertificateChain() { } @Override - public Certificate[] getPeerCertificates() { + public Certificate[] getPeerCertificates() throws SSLPeerUnverifiedException { throw new UnsupportedOperationException(); } From 566b507c6de559063b8902ddb1db0c1de4e9f46b Mon Sep 17 00:00:00 2001 From: Danny Reidenbach Date: Tue, 28 Jul 2020 22:06:12 +0000 Subject: [PATCH 09/20] Fixed Style for travis ci --- common/src/main/java/org/conscrypt/Conscrypt.java | 2 +- .../main/java/org/conscrypt/OkHostnameVerifier.java | 1 - .../test/java/org/conscrypt/HostnameVerifierTest.java | 10 ++++------ .../test/java/org/conscrypt/TrustManagerImplTest.java | 1 - platform/src/main/java/org/conscrypt/Platform.java | 3 +-- 5 files changed, 6 insertions(+), 11 deletions(-) diff --git a/common/src/main/java/org/conscrypt/Conscrypt.java b/common/src/main/java/org/conscrypt/Conscrypt.java index f8bc2f4a1..b87336d04 100644 --- a/common/src/main/java/org/conscrypt/Conscrypt.java +++ b/common/src/main/java/org/conscrypt/Conscrypt.java @@ -22,8 +22,8 @@ import java.security.PrivateKey; import java.security.Provider; import java.util.Properties; -import javax.net.ssl.HttpsURLConnection; import javax.net.ssl.HostnameVerifier; +import javax.net.ssl.HttpsURLConnection; import javax.net.ssl.SSLContext; import javax.net.ssl.SSLContextSpi; import javax.net.ssl.SSLEngine; diff --git a/common/src/main/java/org/conscrypt/OkHostnameVerifier.java b/common/src/main/java/org/conscrypt/OkHostnameVerifier.java index 5e97cb1af..f5b9f771e 100644 --- a/common/src/main/java/org/conscrypt/OkHostnameVerifier.java +++ b/common/src/main/java/org/conscrypt/OkHostnameVerifier.java @@ -26,7 +26,6 @@ import java.util.List; import java.util.Locale; import java.util.regex.Pattern; -import javax.net.ssl.HostnameVerifier; import javax.net.ssl.SSLException; import javax.net.ssl.SSLSession; diff --git a/common/src/test/java/org/conscrypt/HostnameVerifierTest.java b/common/src/test/java/org/conscrypt/HostnameVerifierTest.java index d424bf27c..f552292f5 100644 --- a/common/src/test/java/org/conscrypt/HostnameVerifierTest.java +++ b/common/src/test/java/org/conscrypt/HostnameVerifierTest.java @@ -17,6 +17,10 @@ package org.conscrypt; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + import java.io.ByteArrayInputStream; import java.nio.charset.Charset; import java.security.cert.Certificate; @@ -24,22 +28,16 @@ import java.security.cert.X509Certificate; import java.util.Arrays; import java.util.Collection; -import javax.net.ssl.HostnameVerifier; import javax.net.ssl.SSLPeerUnverifiedException; import javax.net.ssl.SSLSession; import javax.security.auth.x500.X500Principal; import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; import org.junit.runners.Parameterized; import org.junit.runners.Parameterized.Parameter; import org.junit.runners.Parameterized.Parameters; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; - /** * Tests for our hostname verifier. Most of these tests are from AOSP, which * itself includes tests from the Apache HTTP Client test suite. diff --git a/common/src/test/java/org/conscrypt/TrustManagerImplTest.java b/common/src/test/java/org/conscrypt/TrustManagerImplTest.java index 0f5a2d813..7a7183c69 100644 --- a/common/src/test/java/org/conscrypt/TrustManagerImplTest.java +++ b/common/src/test/java/org/conscrypt/TrustManagerImplTest.java @@ -29,7 +29,6 @@ import java.util.Arrays; import java.util.List; import javax.net.ssl.HandshakeCompletedListener; -import javax.net.ssl.HttpsURLConnection; import javax.net.ssl.SSLParameters; import javax.net.ssl.SSLPeerUnverifiedException; import javax.net.ssl.SSLSession; diff --git a/platform/src/main/java/org/conscrypt/Platform.java b/platform/src/main/java/org/conscrypt/Platform.java index a3c41d0d2..b1596b615 100644 --- a/platform/src/main/java/org/conscrypt/Platform.java +++ b/platform/src/main/java/org/conscrypt/Platform.java @@ -48,7 +48,6 @@ import java.util.Collections; import java.util.List; import javax.crypto.spec.GCMParameterSpec; -import javax.net.ssl.HttpsURLConnection.HostnameVerifier; import javax.net.ssl.SNIHostName; import javax.net.ssl.SNIMatcher; import javax.net.ssl.SNIServerName; @@ -65,7 +64,7 @@ import org.conscrypt.ct.CTPolicy; import org.conscrypt.ct.CTPolicyImpl; import sun.security.x509.AlgorithmId; -// use https one here + final class Platform { private static class NoPreloadHolder { public static final Platform MAPPER = new Platform(); } From 3d21617d26c5ef250c80c07678d208f319589857 Mon Sep 17 00:00:00 2001 From: Danny Reidenbach Date: Fri, 31 Jul 2020 22:14:59 +0000 Subject: [PATCH 10/20] Updated verify method to allow certificates to be passed in directly --- .../main/java/org/conscrypt/Conscrypt.java | 3 +- .../conscrypt/ConscryptHostnameVerifier.java | 3 +- .../org/conscrypt/OkHostnameVerifier.java | 16 ++- .../java/org/conscrypt/TrustManagerImpl.java | 20 +-- .../org/conscrypt/HostnameVerifierTest.java | 130 ++++++++++-------- .../org/conscrypt/TrustManagerImplTest.java | 2 +- 6 files changed, 92 insertions(+), 82 deletions(-) diff --git a/common/src/main/java/org/conscrypt/Conscrypt.java b/common/src/main/java/org/conscrypt/Conscrypt.java index b87336d04..3eb3c736f 100644 --- a/common/src/main/java/org/conscrypt/Conscrypt.java +++ b/common/src/main/java/org/conscrypt/Conscrypt.java @@ -21,6 +21,7 @@ import java.security.KeyManagementException; import java.security.PrivateKey; import java.security.Provider; +import java.security.cert.X509Certificate; import java.util.Properties; import javax.net.ssl.HostnameVerifier; import javax.net.ssl.HttpsURLConnection; @@ -791,7 +792,7 @@ public static ConscryptHostnameVerifier wrapHostnameVerifier(final HostnameVerif // Cannot find HttpsURLConnection.HostnameVerifier return new ConscryptHostnameVerifier() { @Override - public boolean verify(String hostname, SSLSession session) { + public boolean verify(X509Certificate[] certs, String hostname, SSLSession session) { return verifier.verify(hostname, session); } }; diff --git a/common/src/main/java/org/conscrypt/ConscryptHostnameVerifier.java b/common/src/main/java/org/conscrypt/ConscryptHostnameVerifier.java index e09ac1ebc..ffdd762f7 100644 --- a/common/src/main/java/org/conscrypt/ConscryptHostnameVerifier.java +++ b/common/src/main/java/org/conscrypt/ConscryptHostnameVerifier.java @@ -17,6 +17,7 @@ package org.conscrypt; import javax.net.ssl.SSLSession; +import java.security.cert.X509Certificate; /** * This interface is used to implement hostname verification in Conscrypt. Unlike with @@ -29,6 +30,6 @@ public interface ConscryptHostnameVerifier { * Returns whether the given hostname is allowable given the peer's authentication information * from the given session. */ - boolean verify(String hostname, SSLSession session); + boolean verify(X509Certificate[] certs, String hostname, SSLSession session); } diff --git a/common/src/main/java/org/conscrypt/OkHostnameVerifier.java b/common/src/main/java/org/conscrypt/OkHostnameVerifier.java index f5b9f771e..57f12f483 100644 --- a/common/src/main/java/org/conscrypt/OkHostnameVerifier.java +++ b/common/src/main/java/org/conscrypt/OkHostnameVerifier.java @@ -70,12 +70,16 @@ public static OkHostnameVerifier strictInstance() { // END Android-changed: Add a mode which disallows top-level domain wildcards. b/144694112 @Override - public boolean verify(String host, SSLSession session) { - try { - Certificate[] certificates = session.getPeerCertificates(); - return verify(host, (X509Certificate) certificates[0]); - } catch (SSLException e) { - return false; + public boolean verify(X509Certificate[] certs, String host, SSLSession session) { + if (certs.length > 0) { + return verify(host, certs[0]); + } else { + try { + Certificate[] certificates = session.getPeerCertificates(); + return verify(host, (X509Certificate) certificates[0]); + } catch (SSLException e) { + return false; + } } } diff --git a/common/src/main/java/org/conscrypt/TrustManagerImpl.java b/common/src/main/java/org/conscrypt/TrustManagerImpl.java index a4ffaf2e2..c5e6243e4 100644 --- a/common/src/main/java/org/conscrypt/TrustManagerImpl.java +++ b/common/src/main/java/org/conscrypt/TrustManagerImpl.java @@ -404,7 +404,7 @@ private List checkTrusted(X509Certificate[] certs, String authT String identificationAlgorithm = parameters.getEndpointIdentificationAlgorithm(); if ("HTTPS".equalsIgnoreCase(identificationAlgorithm)) { ConscryptHostnameVerifier verifier = getHttpsVerifier(); - if (!verifier.verify(hostname, session)) { + if (!verifier.verify(certs, hostname, session)) { throw new CertificateException("No subjectAltNames on the certificate match"); } } @@ -998,28 +998,14 @@ void setHostnameVerifier(ConscryptHostnameVerifier verifier) { * * @see #setHostnameVerifier(ConscryptHostnameVerifier) */ - ConscryptHostnameVerifier getHostnameVerifier() { // doggo + ConscryptHostnameVerifier getHostnameVerifier() { return hostnameVerifier; } -// private enum GlobalHostnameVerifierAdapter implements ConscryptHostnameVerifier { -// INSTANCE; -// -// @Override -// public boolean verify(String hostname, SSLSession session) { -// return HttpsURLConnection.getDefaultHostnameVerifier().verify(hostname, session); -// } -// } - - private ConscryptHostnameVerifier getHttpsVerifier() { // doggo + private ConscryptHostnameVerifier getHttpsVerifier() { if (hostnameVerifier != null) { return hostnameVerifier; } -// // remove -// ConscryptHostnameVerifier defaultVerifier = getDefaultHostnameVerifier(); // this is still needed for current tests. Will I have to remove this and update the tests to work with default or are teh predefined tests still valid? -// if (defaultVerifier != null) { -// return defaultVerifier; -// } return Platform.getDefaultHostnameVerifier(); } diff --git a/common/src/test/java/org/conscrypt/HostnameVerifierTest.java b/common/src/test/java/org/conscrypt/HostnameVerifierTest.java index f552292f5..5ed130283 100644 --- a/common/src/test/java/org/conscrypt/HostnameVerifierTest.java +++ b/common/src/test/java/org/conscrypt/HostnameVerifierTest.java @@ -28,6 +28,7 @@ import java.security.cert.X509Certificate; import java.util.Arrays; import java.util.Collection; +import javax.net.ssl.SSLException; import javax.net.ssl.SSLPeerUnverifiedException; import javax.net.ssl.SSLSession; import javax.security.auth.x500.X500Principal; @@ -57,9 +58,8 @@ public FakeSSLSession(Certificate... certificates) throws Exception { public Certificate[] getPeerCertificates() throws SSLPeerUnverifiedException { if (certificates.length == 0) { throw new SSLPeerUnverifiedException("peer not authenticated"); - } else { - return certificates; } + return certificates; } } @@ -82,7 +82,8 @@ public static Collection data() { @Test public void verify() throws Exception { FakeSSLSession session = new FakeSSLSession(); - assertFalse(verifier.verify("localhost", session)); + X509Certificate[] certs = {}; + assertFalse(verifier.verify(certs,"localhost", session)); } @Test public void verifyCn() throws Exception { @@ -115,9 +116,10 @@ public static Collection data() { + "-----END CERTIFICATE-----\n"); // Android-changed: Ignore common name in hostname verification. http://b/70278814 // assertTrue(verifier.verify("foo.com", session)); - assertFalse(verifier.verify("foo.com", session)); - assertFalse(verifier.verify("a.foo.com", session)); - assertFalse(verifier.verify("bar.com", session)); + X509Certificate[] certs = {}; + assertFalse(verifier.verify(certs, "foo.com", session)); + assertFalse(verifier.verify(certs, "a.foo.com", session)); + assertFalse(verifier.verify(certs, "bar.com", session)); } @Test public void verifyNonAsciiCn() throws Exception { @@ -150,8 +152,9 @@ public static Collection data() { + "-----END CERTIFICATE-----\n"); // Android-changed: Ignore common name in hostname verification. http://b/70278814 // assertTrue(verifier.verify("\u82b1\u5b50.co.jp", session)); - assertFalse(verifier.verify("\u82b1\u5b50.co.jp", session)); - assertFalse(verifier.verify("a.\u82b1\u5b50.co.jp", session)); + X509Certificate[] certs = {}; + assertFalse(verifier.verify(certs, "\u82b1\u5b50.co.jp", session)); + assertFalse(verifier.verify(certs, "a.\u82b1\u5b50.co.jp", session)); } @Test public void verifySubjectAlt() throws Exception { @@ -183,10 +186,11 @@ public static Collection data() { + "2xq+8bc6HojdtbCyug/fvBZvZqQXSmU8m8IVcMmWMz0ZQO8ee3QkBHMZfCy7P/kr\n" + "VbWx/uETImUu+NZg22ewEw==\n" + "-----END CERTIFICATE-----\n"); - assertFalse(verifier.verify("foo.com", session)); - assertFalse(verifier.verify("a.foo.com", session)); - assertTrue(verifier.verify("bar.com", session)); - assertFalse(verifier.verify("a.bar.com", session)); + X509Certificate[] certs = {}; + assertFalse(verifier.verify(certs, "foo.com", session)); + assertFalse(verifier.verify(certs, "a.foo.com", session)); + assertTrue(verifier.verify(certs, "bar.com", session)); + assertFalse(verifier.verify(certs, "a.bar.com", session)); } /** @@ -224,8 +228,9 @@ public static Collection data() { + "sWIKHYrmhCIRshUNohGXv50m2o+1w9oWmQ6Dkq7lCjfXfUB4wIbggJjpyEtbNqBt\n" + "j4MC2x5rfsLKKqToKmNE7pFEgqwe8//Aar1b+Qj+\n" + "-----END CERTIFICATE-----\n"); - assertTrue(verifier.verify("foo.com", session)); - assertFalse(verifier.verify("a.foo.com", session)); + X509Certificate[] certs = {}; + assertTrue(verifier.verify(certs, "foo.com", session)); + assertFalse(verifier.verify(certs, "a.foo.com", session)); // these checks test alternative subjects. The test data contains an // alternative subject starting with a japanese kanji character. This is // not supported by Android because the underlying implementation from @@ -264,10 +269,11 @@ public static Collection data() { + "qAVqixM+J0qJmQStgAc53i2aTMvAQu3A3snvH/PHTBo+5UL72n9S1kZyNCsVf1Qo\n" + "n8jKTiRriEM+fMFlcgQP284EBFzYHyCXFb9O/hMjK2+6mY9euMB1U1aFFzM/Bg==\n" + "-----END CERTIFICATE-----\n"); - assertTrue(verifier.verify("foo.com", session)); - assertFalse(verifier.verify("a.foo.com", session)); - assertTrue(verifier.verify("foo.com", session)); - assertFalse(verifier.verify("a.foo.com", session)); + X509Certificate[] certs = {}; + assertTrue(verifier.verify(certs, "foo.com", session)); + assertFalse(verifier.verify(certs, "a.foo.com", session)); + assertTrue(verifier.verify(certs, "foo.com", session)); + assertFalse(verifier.verify(certs, "a.foo.com", session)); } @Test public void verifyMultipleCn() throws Exception { @@ -299,14 +305,15 @@ public static Collection data() { + "SpQFCfo02NO0uNRDPUdJx2huycdNb+AXHaO7eXevDLJ+QnqImIzxWiY6zLOdzjjI\n" + "VBMkLHmnP7SjGSQ3XA4ByrQOxfOUTyLyE7NuemhHppuQPxE=\n" + "-----END CERTIFICATE-----\n"); - assertFalse(verifier.verify("foo.com", session)); - assertFalse(verifier.verify("a.foo.com", session)); - assertFalse(verifier.verify("bar.com", session)); - assertFalse(verifier.verify("a.bar.com", session)); + X509Certificate[] certs = {}; + assertFalse(verifier.verify(certs, "foo.com", session)); + assertFalse(verifier.verify(certs, "a.foo.com", session)); + assertFalse(verifier.verify(certs, "bar.com", session)); + assertFalse(verifier.verify(certs, "a.bar.com", session)); // Android-changed: Ignore common name in hostname verification. http://b/70278814 // assertTrue(verifier.verify("\u82b1\u5b50.co.jp", session)); - assertFalse(verifier.verify("\u82b1\u5b50.co.jp", session)); - assertFalse(verifier.verify("a.\u82b1\u5b50.co.jp", session)); + assertFalse(verifier.verify(certs, "\u82b1\u5b50.co.jp", session)); + assertFalse(verifier.verify(certs, "a.\u82b1\u5b50.co.jp", session)); } @Test public void verifyWilcardCn() throws Exception { @@ -337,14 +344,15 @@ public static Collection data() { + "G9Z6tyMbmfRY+dLSh3a9JwoEcBUso6EWYBakLbq4nG/nvYdYvG9ehrnLVwZFL82e\n" + "l3Q/RK95bnA6cuRClGusLad0e6bjkBzx/VQ3VarDEpAkTLUGVAa0CLXtnyc=\n" + "-----END CERTIFICATE-----\n"); - assertFalse(verifier.verify("foo.com", session)); + X509Certificate[] certs = {}; + assertFalse(verifier.verify(certs, "foo.com", session)); // Android-changed: Ignore common name in hostname verification. http://b/70278814 // assertTrue(verifier.verify("www.foo.com", session)); - assertFalse(verifier.verify("www.foo.com", session)); + assertFalse(verifier.verify(certs, "www.foo.com", session)); // Android-changed: Ignore common name in hostname verification. http://b/70278814 // assertTrue(verifier.verify("\u82b1\u5b50.foo.com", session)); - assertFalse(verifier.verify("\u82b1\u5b50.foo.com", session)); - assertFalse(verifier.verify("a.b.foo.com", session)); + assertFalse(verifier.verify(certs, "\u82b1\u5b50.foo.com", session)); + assertFalse(verifier.verify(certs, "a.b.foo.com", session)); } @Test public void verifyWilcardCnOnTld() throws Exception { @@ -376,12 +384,13 @@ public static Collection data() { + "UGPLEUDzRHMPHLnSqT1n5UU5UDRytbjJPXzF+l/+WZIsanefWLsxnkgAuZe/oMMF\n" + "EJMryEzOjg4Tfuc5qM0EXoPcQ/JlheaxZ40p2IyHqbsWV4MRYuFH4bkM\n" + "-----END CERTIFICATE-----\n"); + X509Certificate[] certs = {}; // Android-changed: Ignore common name in hostname verification. http://b/70278814 // assertTrue(verifier.verify("foo.co.jp", session)); - assertFalse(verifier.verify("foo.co.jp", session)); + assertFalse(verifier.verify(certs, "foo.co.jp", session)); // Android-changed: Ignore common name in hostname verification. http://b/70278814 // assertTrue(verifier.verify("\u82b1\u5b50.co.jp", session)); - assertFalse(verifier.verify("\u82b1\u5b50.co.jp", session)); + assertFalse(verifier.verify(certs, "\u82b1\u5b50.co.jp", session)); } /** @@ -419,11 +428,12 @@ public static Collection data() { + "qFr0AIZKBlg6NZZFf/0dP9zcKhzSriW27bY0XfzA6GSiRDXrDjgXq6baRT6YwgIg\n" + "pgJsDbJtZfHnV1nd3M6zOtQPm1TIQpNmMMMd/DPrGcUQerD3\n" + "-----END CERTIFICATE-----\n"); + X509Certificate[] certs = {}; // try the foo.com variations - assertTrue(verifier.verify("foo.com", session)); - assertTrue(verifier.verify("www.foo.com", session)); - assertTrue(verifier.verify("\u82b1\u5b50.foo.com", session)); - assertFalse(verifier.verify("a.b.foo.com", session)); + assertTrue(verifier.verify(certs, "foo.com", session)); + assertTrue(verifier.verify(certs, "www.foo.com", session)); + assertTrue(verifier.verify(certs, "\u82b1\u5b50.foo.com", session)); + assertFalse(verifier.verify(certs, "a.b.foo.com", session)); // these checks test alternative subjects. The test data contains an // alternative subject starting with a japanese kanji character. This is // not supported by Android because the underlying implementation from @@ -459,15 +469,18 @@ public static Collection data() { + "DCzOCv9Ma6Lv5o5jcYWVxvBSTsnt22hsJpWD1K7iY9lbkLwl0ivn73pG2evsAn9G\n" + "X8YKH52fnHsCrhSD\n" + "-----END CERTIFICATE-----"); - assertEquals(new X500Principal("CN=localhost"), certificate.getSubjectX500Principal()); + assertEquals(new X500Principal("CN=localhost"), certificate.getSubjectX500Principal()); FakeSSLSession session = new FakeSSLSession(certificate); - assertTrue(verifier.verify("localhost", session)); - assertTrue(verifier.verify("localhost.localdomain", session)); - assertFalse(verifier.verify("local.host", session)); - assertTrue(verifier.verify("127.0.0.1", session)); - assertFalse(verifier.verify("127.0.0.2", session)); + X509Certificate[] certs = {}; + + assertTrue(verifier.verify(certs, "localhost", session)); + assertTrue(verifier.verify(certs, "localhost.localdomain", session)); + assertFalse(verifier.verify(certs, "local.host", session)); + + assertTrue(verifier.verify(certs, "127.0.0.1", session)); + assertFalse(verifier.verify(certs, "127.0.0.2", session)); } @Test public void wildcardsCannotMatchIpAddresses() throws Exception { @@ -484,7 +497,8 @@ public static Collection data() { + "BQUAA0EAk9i88xdjWoewqvE+iMC9tD2obMchgFDaHH0ogxxiRaIKeEly3g0uGxIt\n" + "fl2WRY8hb4x+zRrwsFaLEpdEvqcjOQ==\n" + "-----END CERTIFICATE-----"); - assertFalse(verifier.verify("127.0.0.1", session)); + X509Certificate[] certs = {}; + assertFalse(verifier.verify(certs, "127.0.0.1", session)); } /** @@ -509,7 +523,8 @@ public static Collection data() { + "-----END CERTIFICATE-----"); // Android-changed: Ignore common name in hostname verification. http://b/70278814 // assertTrue(verifier.verify("google.com", session)); - assertFalse(verifier.verify("google.com", session)); + X509Certificate[] certs = {}; + assertFalse(verifier.verify(certs, "google.com", session)); } @Test public void subjectAltName() throws Exception { @@ -535,11 +550,12 @@ public static Collection data() { + "DQYJKoZIhvcNAQEFBQADQQBXpZZPOY2Dy1lGG81JTr8L4or9jpKacD7n51eS8iqI\n" + "oTznPNuXHU5bFN0AAGX2ij47f/EahqTpo5RdS95P4sVm\n" + "-----END CERTIFICATE-----"); - assertFalse(verifier.verify("foo.com", session)); - assertTrue(verifier.verify("bar.com", session)); - assertTrue(verifier.verify("baz.com", session)); - assertFalse(verifier.verify("a.foo.com", session)); - assertFalse(verifier.verify("quux.com", session)); + X509Certificate[] certs = {}; + assertFalse(verifier.verify(certs, "foo.com", session)); + assertTrue(verifier.verify(certs, "bar.com", session)); + assertTrue(verifier.verify(certs, "baz.com", session)); + assertFalse(verifier.verify(certs, "a.foo.com", session)); + assertFalse(verifier.verify(certs, "quux.com", session)); } @Test public void subjectAltNameWithWildcard() throws Exception { @@ -565,13 +581,14 @@ public static Collection data() { + "bTANBgkqhkiG9w0BAQUFAANBAB8yrSl8zqy07i0SNYx2B/FnvQY734pxioaqFWfO\n" + "Bqo1ZZl/9aPHEWIwBrxYNVB0SGu/kkbt/vxqOjzzrkXukmI=\n" + "-----END CERTIFICATE-----"); - assertFalse(verifier.verify("foo.com", session)); - assertTrue(verifier.verify("bar.com", session)); - assertTrue(verifier.verify("a.baz.com", session)); - assertFalse(verifier.verify("baz.com", session)); - assertFalse(verifier.verify("a.foo.com", session)); - assertFalse(verifier.verify("a.bar.com", session)); - assertFalse(verifier.verify("quux.com", session)); + X509Certificate[] certs = {}; + assertFalse(verifier.verify(certs, "foo.com", session)); + assertTrue(verifier.verify(certs, "bar.com", session)); + assertTrue(verifier.verify(certs, "a.baz.com", session)); + assertFalse(verifier.verify(certs, "baz.com", session)); + assertFalse(verifier.verify(certs, "a.foo.com", session)); + assertFalse(verifier.verify(certs, "a.bar.com", session)); + assertFalse(verifier.verify(certs, "quux.com", session)); } // BEGIN Android-added: Verify behaviour with top level wildcard SAN. http://b/144694112 @@ -595,8 +612,9 @@ public void subjectAltNameWithToplevelWildcard() throws Exception { + "DQEBCwUAA0EAK710g2hQpXSmpbOQH4dHG61fkVDtM/kR/4/R61vDDqVkgOuyHqXl\n" + "GUZFKHMeOZ8peQLT8b+5ik6pIO7Vu2pF6w==\n" + "-----END CERTIFICATE-----\n"); - assertTrue(OkHostnameVerifier.INSTANCE.verify("google.com", session)); - assertFalse(OkHostnameVerifier.strictInstance().verify("google.com", session)); + X509Certificate[] certs = {}; + assertTrue(OkHostnameVerifier.INSTANCE.verify(certs, "google.com", session)); + assertFalse(OkHostnameVerifier.strictInstance().verify(certs, "google.com", session)); } // END Android-added: Verify behaviour with top level wildcard SAN. http://b/144694112 diff --git a/common/src/test/java/org/conscrypt/TrustManagerImplTest.java b/common/src/test/java/org/conscrypt/TrustManagerImplTest.java index 7a7183c69..a20e362e6 100644 --- a/common/src/test/java/org/conscrypt/TrustManagerImplTest.java +++ b/common/src/test/java/org/conscrypt/TrustManagerImplTest.java @@ -243,7 +243,7 @@ public void testHttpsEndpointIdentification() throws Exception { } catch (CertificateException expected) { } certs = tmi.getTrustedChainForServer(chain, "RSA", - new FakeSSLSocket(new FakeSSLSession(goodHostname, chain), params)); + new FakeSSLSocket(new FakeSSLSession(goodHostname, chain), params)); assertEquals(Arrays.asList(chain), certs); } catch (Exception e) { } From 52a80433fadf9dd44c5d00a96563062e227a2b8e Mon Sep 17 00:00:00 2001 From: Danny Reidenbach Date: Thu, 23 Jul 2020 23:01:37 +0000 Subject: [PATCH 11/20] Cleaned up comments --- common/src/jni/main/cpp/conscrypt/native_crypto.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/common/src/jni/main/cpp/conscrypt/native_crypto.cc b/common/src/jni/main/cpp/conscrypt/native_crypto.cc index 6318492d5..163251191 100644 --- a/common/src/jni/main/cpp/conscrypt/native_crypto.cc +++ b/common/src/jni/main/cpp/conscrypt/native_crypto.cc @@ -3660,6 +3660,7 @@ static jint evp_aead_ctx_op_buf(JNIEnv* env, jlong evpAeadRef, jbyteArray keyArr size_t inSize = in_limit - in_position; uint8_t* outBufEnd = outBuf + out_limit - out_position; + uint8_t* inBufEnd = inBuf + inSize; std::unique_ptr inCopy; if (outBufEnd >= inBuf && inBufEnd >= outBuf) { // We have an overlap From e96142033a51df08181a49d34987f288a779f92f Mon Sep 17 00:00:00 2001 From: Danny Reidenbach Date: Tue, 4 Aug 2020 04:48:20 +0000 Subject: [PATCH 12/20] fixed merge conflict --- common/src/jni/main/cpp/conscrypt/native_crypto.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/common/src/jni/main/cpp/conscrypt/native_crypto.cc b/common/src/jni/main/cpp/conscrypt/native_crypto.cc index 163251191..6318492d5 100644 --- a/common/src/jni/main/cpp/conscrypt/native_crypto.cc +++ b/common/src/jni/main/cpp/conscrypt/native_crypto.cc @@ -3660,7 +3660,6 @@ static jint evp_aead_ctx_op_buf(JNIEnv* env, jlong evpAeadRef, jbyteArray keyArr size_t inSize = in_limit - in_position; uint8_t* outBufEnd = outBuf + out_limit - out_position; - uint8_t* inBufEnd = inBuf + inSize; std::unique_ptr inCopy; if (outBufEnd >= inBuf && inBufEnd >= outBuf) { // We have an overlap From 8345727d5f2d988d320cad1a1e4e54be716e6b1b Mon Sep 17 00:00:00 2001 From: Danny Reidenbach Date: Tue, 4 Aug 2020 05:03:54 +0000 Subject: [PATCH 13/20] Fixed style for travis --- .../src/main/java/org/conscrypt/ConscryptHostnameVerifier.java | 2 +- common/src/test/java/org/conscrypt/HostnameVerifierTest.java | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/common/src/main/java/org/conscrypt/ConscryptHostnameVerifier.java b/common/src/main/java/org/conscrypt/ConscryptHostnameVerifier.java index ffdd762f7..277226934 100644 --- a/common/src/main/java/org/conscrypt/ConscryptHostnameVerifier.java +++ b/common/src/main/java/org/conscrypt/ConscryptHostnameVerifier.java @@ -16,8 +16,8 @@ package org.conscrypt; -import javax.net.ssl.SSLSession; import java.security.cert.X509Certificate; +import javax.net.ssl.SSLSession; /** * This interface is used to implement hostname verification in Conscrypt. Unlike with diff --git a/common/src/test/java/org/conscrypt/HostnameVerifierTest.java b/common/src/test/java/org/conscrypt/HostnameVerifierTest.java index 5ed130283..90ec3f76d 100644 --- a/common/src/test/java/org/conscrypt/HostnameVerifierTest.java +++ b/common/src/test/java/org/conscrypt/HostnameVerifierTest.java @@ -28,7 +28,6 @@ import java.security.cert.X509Certificate; import java.util.Arrays; import java.util.Collection; -import javax.net.ssl.SSLException; import javax.net.ssl.SSLPeerUnverifiedException; import javax.net.ssl.SSLSession; import javax.security.auth.x500.X500Principal; From 878f0ceb8b8f6e3b99e005160784fe33ca640e22 Mon Sep 17 00:00:00 2001 From: Danny Reidenbach Date: Tue, 4 Aug 2020 05:32:02 +0000 Subject: [PATCH 14/20] update for warnings --- common/src/test/java/org/conscrypt/HostnameVerifierTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/src/test/java/org/conscrypt/HostnameVerifierTest.java b/common/src/test/java/org/conscrypt/HostnameVerifierTest.java index 90ec3f76d..a369ecd1e 100644 --- a/common/src/test/java/org/conscrypt/HostnameVerifierTest.java +++ b/common/src/test/java/org/conscrypt/HostnameVerifierTest.java @@ -44,7 +44,7 @@ */ @RunWith(Parameterized.class) public final class HostnameVerifierTest { - public final class FakeSSLSession extends org.conscrypt.javax.net.ssl.FakeSSLSession { + public static final class FakeSSLSession extends org.conscrypt.javax.net.ssl.FakeSSLSession { private final Certificate[] certificates; From faaf7b29d0b6ebd68b026aa7dc68471319403bb9 Mon Sep 17 00:00:00 2001 From: Danny Reidenbach Date: Tue, 4 Aug 2020 18:08:29 +0000 Subject: [PATCH 15/20] Fixed test and updated style. --- .../main/java/org/conscrypt/Conscrypt.java | 4 +- .../org/conscrypt/TrustManagerImplTest.java | 100 +----------------- 2 files changed, 6 insertions(+), 98 deletions(-) diff --git a/common/src/main/java/org/conscrypt/Conscrypt.java b/common/src/main/java/org/conscrypt/Conscrypt.java index 3eb3c736f..46fb22a24 100644 --- a/common/src/main/java/org/conscrypt/Conscrypt.java +++ b/common/src/main/java/org/conscrypt/Conscrypt.java @@ -788,11 +788,9 @@ public static ConscryptHostnameVerifier getHostnameVerifier(TrustManager trustMa * Wraps the HttpsURLConnection.HostnameVerifier into a ConscryptHostnameVerifier */ public static ConscryptHostnameVerifier wrapHostnameVerifier(final HostnameVerifier verifier) { - // needed to add final due to : error: local variable verifier is accessed from within inner class; needs to be declared final - // Cannot find HttpsURLConnection.HostnameVerifier return new ConscryptHostnameVerifier() { @Override - public boolean verify(X509Certificate[] certs, String hostname, SSLSession session) { + public boolean verify(X509Certificate[] certificates, String hostname, SSLSession session) { return verifier.verify(hostname, session); } }; diff --git a/common/src/test/java/org/conscrypt/TrustManagerImplTest.java b/common/src/test/java/org/conscrypt/TrustManagerImplTest.java index a20e362e6..45dac8807 100644 --- a/common/src/test/java/org/conscrypt/TrustManagerImplTest.java +++ b/common/src/test/java/org/conscrypt/TrustManagerImplTest.java @@ -122,91 +122,6 @@ public void testGetFullChain() throws Exception { assertEquals(Arrays.asList(chain3), certs); } -// @Test -// public void testHttpsEndpointIdentification() throws Exception { -// TestUtils.assumeExtendedTrustManagerAvailable(); -// -// KeyStore.PrivateKeyEntry pke = TestKeyStore.getServerHostname().getPrivateKey("RSA", "RSA"); -// X509Certificate[] chain = (X509Certificate[]) pke.getCertificateChain(); -// X509Certificate root = chain[2]; -// TrustManagerImpl tmi = (TrustManagerImpl) trustManager(root); -// -// String goodHostname = TestKeyStore.CERT_HOSTNAME; -// String badHostname = "definitelywrong.nopenopenope"; -// -// // The default hostname verifier on OpenJDK rejects all hostnames, so use our own -// javax.net.ssl.HostnameVerifier oldDefault = HttpsURLConnection.getDefaultHostnameVerifier(); -// try { -// HttpsURLConnection.setDefaultHostnameVerifier(new TestHostnameVerifier()); -// -// SSLParameters params = new SSLParameters(); -// -// // Without endpoint identification this should pass despite the mismatched hostname -// params.setEndpointIdentificationAlgorithm(null); -// -// List certs = tmi.getTrustedChainForServer(chain, "RSA", -// new FakeSSLSocket(new FakeSSLSession(badHostname, chain), params)); -// assertEquals(Arrays.asList(chain), certs); -// -// // Turn on endpoint identification -// params.setEndpointIdentificationAlgorithm("HTTPS"); -// -// try { -// tmi.getTrustedChainForServer(chain, "RSA", -// new FakeSSLSocket(new FakeSSLSession(badHostname, chain), params)); -// fail(); -// } catch (CertificateException expected) { -// } -// -// certs = tmi.getTrustedChainForServer(chain, "RSA", -// new FakeSSLSocket(new FakeSSLSession(goodHostname, chain), params)); -// assertEquals(Arrays.asList(chain), certs); -// -// // Override the global default hostname verifier with a Conscrypt-specific one that -// // always passes. Both scenarios should pass. -// Conscrypt.setDefaultHostnameVerifier(new ConscryptHostnameVerifier() { -// @Override public boolean verify(String s, SSLSession sslSession) { return true; } -// }); -// -// certs = tmi.getTrustedChainForServer(chain, "RSA", -// new FakeSSLSocket(new FakeSSLSession(badHostname, chain), params)); -// assertEquals(Arrays.asList(chain), certs); -// -// certs = tmi.getTrustedChainForServer(chain, "RSA", -// new FakeSSLSocket(new FakeSSLSession(goodHostname, chain), params)); -// assertEquals(Arrays.asList(chain), certs); -// -// // Now set an instance-specific verifier on the trust manager. The bad hostname should -// // fail again. -// Conscrypt.setHostnameVerifier(tmi, new TestHostnameVerifier()); -// -// try { -// tmi.getTrustedChainForServer(chain, "RSA", -// new FakeSSLSocket(new FakeSSLSession(badHostname, chain), params)); -// fail(); -// } catch (CertificateException expected) { -// } -// -// certs = tmi.getTrustedChainForServer(chain, "RSA", -// new FakeSSLSocket(new FakeSSLSession(goodHostname, chain), params)); -// assertEquals(Arrays.asList(chain), certs); -// -// // Remove the instance-specific verifier, and both should pass again. -// Conscrypt.setHostnameVerifier(tmi, null); -// -// certs = tmi.getTrustedChainForServer(chain, "RSA", -// new FakeSSLSocket(new FakeSSLSession(badHostname, chain), params)); -// assertEquals(Arrays.asList(chain), certs); -// -// certs = tmi.getTrustedChainForServer(chain, "RSA", -// new FakeSSLSocket(new FakeSSLSession(goodHostname, chain), params)); -// assertEquals(Arrays.asList(chain), certs); -// } finally { -// Conscrypt.setDefaultHostnameVerifier(null); -// HttpsURLConnection.setDefaultHostnameVerifier(oldDefault); -// } -// } - @Test public void testHttpsEndpointIdentification() throws Exception { TestUtils.assumeExtendedTrustManagerAvailable(); @@ -219,8 +134,6 @@ public void testHttpsEndpointIdentification() throws Exception { String goodHostname = TestKeyStore.CERT_HOSTNAME; String badHostname = "definitelywrong.nopenopenope"; - // The default hostname verifier on OpenJDK no longer rejects all hostnames -// javax.net.ssl.HostnameVerifier oldDefault = HttpsURLConnection.getDefaultHostnameVerifier(); try { SSLParameters params = new SSLParameters(); @@ -236,16 +149,17 @@ public void testHttpsEndpointIdentification() throws Exception { try { // this should fail certs = tmi.getTrustedChainForServer(chain, "RSA", - new FakeSSLSocket(new FakeSSLSession(badHostname, chain), params)); - assertEquals(Arrays.asList(chain), certs); + new FakeSSLSocket(new FakeSSLSession(badHostname, chain), params)); + assertEquals(Arrays.asList(chain), certs); fail(); } catch (CertificateException expected) { } certs = tmi.getTrustedChainForServer(chain, "RSA", - new FakeSSLSocket(new FakeSSLSession(goodHostname, chain), params)); + new FakeSSLSocket(new FakeSSLSession(goodHostname, chain), params)); assertEquals(Arrays.asList(chain), certs); - } catch (Exception e) { + } finally { + Conscrypt.setDefaultHostnameVerifier(null); } } @@ -516,8 +430,4 @@ public boolean getEnableSessionCreation() { throw new UnsupportedOperationException(); } } - -// private static class TestHostnameVerifier -// extends org.conscrypt.javax.net.ssl.TestHostnameVerifier -// implements ConscryptHostnameVerifier {} } From e2653a7756b48f37c4b93fd12eaafa0f266847d2 Mon Sep 17 00:00:00 2001 From: Danny Reidenbach Date: Thu, 6 Aug 2020 18:21:04 +0000 Subject: [PATCH 16/20] iffy integration test --- common/src/test/java/org/conscrypt/TrustManagerImplTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/src/test/java/org/conscrypt/TrustManagerImplTest.java b/common/src/test/java/org/conscrypt/TrustManagerImplTest.java index 45dac8807..0c7648804 100644 --- a/common/src/test/java/org/conscrypt/TrustManagerImplTest.java +++ b/common/src/test/java/org/conscrypt/TrustManagerImplTest.java @@ -158,7 +158,7 @@ public void testHttpsEndpointIdentification() throws Exception { certs = tmi.getTrustedChainForServer(chain, "RSA", new FakeSSLSocket(new FakeSSLSession(goodHostname, chain), params)); assertEquals(Arrays.asList(chain), certs); - } finally { + } finally { // Still need for protecting future tests Conscrypt.setDefaultHostnameVerifier(null); } } From 9634556957f8da6d6cf54d02cc65d11cd01b3db4 Mon Sep 17 00:00:00 2001 From: Danny Reidenbach Date: Thu, 6 Aug 2020 19:37:28 +0000 Subject: [PATCH 17/20] Iffy testing --- common/src/test/java/org/conscrypt/TrustManagerImplTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/src/test/java/org/conscrypt/TrustManagerImplTest.java b/common/src/test/java/org/conscrypt/TrustManagerImplTest.java index 0c7648804..a05241b01 100644 --- a/common/src/test/java/org/conscrypt/TrustManagerImplTest.java +++ b/common/src/test/java/org/conscrypt/TrustManagerImplTest.java @@ -158,7 +158,7 @@ public void testHttpsEndpointIdentification() throws Exception { certs = tmi.getTrustedChainForServer(chain, "RSA", new FakeSSLSocket(new FakeSSLSession(goodHostname, chain), params)); assertEquals(Arrays.asList(chain), certs); - } finally { // Still need for protecting future tests + } finally { // Still need for protecting future tests Conscrypt.setDefaultHostnameVerifier(null); } } From d61af15bce54465f1546690dcce81288c1324ecf Mon Sep 17 00:00:00 2001 From: Danny Reidenbach Date: Mon, 10 Aug 2020 17:56:37 +0000 Subject: [PATCH 18/20] Upgraded TrustManagerImplTest as well as suppressed warnings --- .../org/conscrypt/OkHostnameVerifier.java | 2 + .../org/conscrypt/TrustManagerImplTest.java | 54 +++++++++++++++++-- 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/common/src/main/java/org/conscrypt/OkHostnameVerifier.java b/common/src/main/java/org/conscrypt/OkHostnameVerifier.java index 57f12f483..c14e3719d 100644 --- a/common/src/main/java/org/conscrypt/OkHostnameVerifier.java +++ b/common/src/main/java/org/conscrypt/OkHostnameVerifier.java @@ -109,6 +109,7 @@ private boolean verifyIpAddress(String ipAddress, X509Certificate certificate) { /** * Returns true if {@code certificate} matches {@code hostName}. */ + @SuppressWarnings("UnusedVariable") private boolean verifyHostName(String hostName, X509Certificate certificate) { hostName = hostName.toLowerCase(Locale.US); boolean hasDns = false; @@ -144,6 +145,7 @@ public static List allSubjectAltNames(X509Certificate certificate) { return result; } + @SuppressWarnings("MixedMutabilityReturnType") private static List getSubjectAltNames(X509Certificate certificate, int type) { List result = new ArrayList<>(); try { diff --git a/common/src/test/java/org/conscrypt/TrustManagerImplTest.java b/common/src/test/java/org/conscrypt/TrustManagerImplTest.java index a05241b01..e3c7ad8d2 100644 --- a/common/src/test/java/org/conscrypt/TrustManagerImplTest.java +++ b/common/src/test/java/org/conscrypt/TrustManagerImplTest.java @@ -147,18 +147,64 @@ public void testHttpsEndpointIdentification() throws Exception { // Turn on endpoint identification params.setEndpointIdentificationAlgorithm("HTTPS"); - try { // this should fail - certs = tmi.getTrustedChainForServer(chain, "RSA", + try { + tmi.getTrustedChainForServer(chain, "RSA", new FakeSSLSocket(new FakeSSLSession(badHostname, chain), params)); - assertEquals(Arrays.asList(chain), certs); fail(); + } catch (CertificateException expected) { + } + + certs = tmi.getTrustedChainForServer(chain, "RSA", + new FakeSSLSocket(new FakeSSLSession(goodHostname, chain), params)); + assertEquals(Arrays.asList(chain), certs); + + // Override the global default hostname verifier with a Conscrypt-specific one that + // always passes. Both scenarios should pass. + Conscrypt.setHostnameVerifier(tmi, new ConscryptHostnameVerifier() { + @Override + public boolean verify(X509Certificate[] certificates, String s, SSLSession sslSession) { + return true; + } + }); + + certs = tmi.getTrustedChainForServer(chain, "RSA", + new FakeSSLSocket(new FakeSSLSession(badHostname, chain), params)); + assertEquals(Arrays.asList(chain), certs); + + certs = tmi.getTrustedChainForServer(chain, "RSA", + new FakeSSLSocket(new FakeSSLSession(goodHostname, chain), params)); + assertEquals(Arrays.asList(chain), certs); + // Now set an instance-specific verifier on the trust manager. The bad hostname should + // fail again. +// Conscrypt.setHostnameVerifier(tmi, new TestHostnameVerifier()); + Conscrypt.setHostnameVerifier(tmi, Conscrypt.wrapHostnameVerifier(new org.conscrypt.javax.net.ssl.TestHostnameVerifier())); + + try { + tmi.getTrustedChainForServer(chain, "RSA", + new FakeSSLSocket(new FakeSSLSession(badHostname, chain), params)); + fail(); + } catch (CertificateException expected) { + } + + certs = tmi.getTrustedChainForServer(chain, "RSA", + new FakeSSLSocket(new FakeSSLSession(goodHostname, chain), params)); + assertEquals(Arrays.asList(chain), certs); + + // Remove the instance-specific verifier, and both should pass again. + Conscrypt.setHostnameVerifier(tmi, null); + + try { + tmi.getTrustedChainForServer(chain, "RSA", + new FakeSSLSocket(new FakeSSLSession(badHostname, chain), params)); + fail(); } catch (CertificateException expected) { } + certs = tmi.getTrustedChainForServer(chain, "RSA", new FakeSSLSocket(new FakeSSLSession(goodHostname, chain), params)); assertEquals(Arrays.asList(chain), certs); - } finally { // Still need for protecting future tests + } finally { Conscrypt.setDefaultHostnameVerifier(null); } } From 840aa5e2f64239427fd7e5d2e75d7541b73e08e8 Mon Sep 17 00:00:00 2001 From: Pete Bentley <44170157+prbprbprb@users.noreply.github.com> Date: Mon, 10 Aug 2020 22:34:57 +0100 Subject: [PATCH 19/20] Remove commented out code and simplify slightly --- common/src/test/java/org/conscrypt/TrustManagerImplTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/src/test/java/org/conscrypt/TrustManagerImplTest.java b/common/src/test/java/org/conscrypt/TrustManagerImplTest.java index e3c7ad8d2..4d6a2736a 100644 --- a/common/src/test/java/org/conscrypt/TrustManagerImplTest.java +++ b/common/src/test/java/org/conscrypt/TrustManagerImplTest.java @@ -36,6 +36,7 @@ import javax.net.ssl.SSLSocket; import javax.net.ssl.X509TrustManager; import org.conscrypt.java.security.TestKeyStore; +import org.conscrypt.javax.net.ssl.TestHostnameVerifier; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -177,8 +178,7 @@ public boolean verify(X509Certificate[] certificates, String s, SSLSession sslSe // Now set an instance-specific verifier on the trust manager. The bad hostname should // fail again. -// Conscrypt.setHostnameVerifier(tmi, new TestHostnameVerifier()); - Conscrypt.setHostnameVerifier(tmi, Conscrypt.wrapHostnameVerifier(new org.conscrypt.javax.net.ssl.TestHostnameVerifier())); + Conscrypt.setHostnameVerifier(tmi, Conscrypt.wrapHostnameVerifier(new TestHostnameVerifier())); try { tmi.getTrustedChainForServer(chain, "RSA", From b45dbe8d558323df995dfc3780f442b2d5ce5ea1 Mon Sep 17 00:00:00 2001 From: Danny Reidenbach Date: Wed, 12 Aug 2020 17:52:03 +0000 Subject: [PATCH 20/20] Flaky Integration Commit --- common/src/main/java/org/conscrypt/TrustManagerImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/src/main/java/org/conscrypt/TrustManagerImpl.java b/common/src/main/java/org/conscrypt/TrustManagerImpl.java index c5e6243e4..b7e2b5034 100644 --- a/common/src/main/java/org/conscrypt/TrustManagerImpl.java +++ b/common/src/main/java/org/conscrypt/TrustManagerImpl.java @@ -942,7 +942,7 @@ private TrustAnchor findTrustAnchorBySubjectAndPublicKey(X509Certificate cert) { return trustAnchor; } if (trustedCertificateStore == null) { - // not trusted and no TrustedCertificateStore to check + // not trusted and no TrustedCertificateStore to check. return null; } // probe KeyStore for a cert. AndroidCAStore stores its