Skip to content

Commit

Permalink
Updated verify method to allow certificates to be passed in directly
Browse files Browse the repository at this point in the history
  • Loading branch information
@d-reidenbach
d-reidenbach committed Jul 31, 2020
1 parent 2cf9e24 commit b8009c1
Show file tree
Hide file tree
Showing 6 changed files with 92 additions and 82 deletions.
3 changes: 2 additions & 1 deletion common/src/main/java/org/conscrypt/Conscrypt.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
};
Expand Down
3 changes: 2 additions & 1 deletion common/src/main/java/org/conscrypt/ConscryptHostnameVerifier.java
View file
Expand Up @@ -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
Expand All @@ -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);

}
16 changes: 10 additions & 6 deletions common/src/main/java/org/conscrypt/OkHostnameVerifier.java
View file
Expand Up @@ -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;
}
}
}

Expand Down
20 changes: 3 additions & 17 deletions common/src/main/java/org/conscrypt/TrustManagerImpl.java
View file
Expand Up @@ -404,7 +404,7 @@ private List<X509Certificate> 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");
}
}
Expand Down Expand Up @@ -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();
}

Expand Down
130 changes: 74 additions & 56 deletions common/src/test/java/org/conscrypt/HostnameVerifierTest.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
}

Expand All @@ -82,7 +82,8 @@ public static Collection<Object[]> 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 {
Expand Down Expand Up @@ -115,9 +116,10 @@ public static Collection<Object[]> 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 {
Expand Down Expand Up @@ -150,8 +152,9 @@ public static Collection<Object[]> 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 {
Expand Down Expand Up @@ -183,10 +186,11 @@ public static Collection<Object[]> 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));
}

/**
Expand Down Expand Up @@ -224,8 +228,9 @@ public static Collection<Object[]> 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
Expand Down Expand Up @@ -264,10 +269,11 @@ public static Collection<Object[]> 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 {
Expand Down Expand Up @@ -299,14 +305,15 @@ public static Collection<Object[]> 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 {
Expand Down Expand Up @@ -337,14 +344,15 @@ public static Collection<Object[]> 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 {
Expand Down Expand Up @@ -376,12 +384,13 @@ public static Collection<Object[]> 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));
}

/**
Expand Down Expand Up @@ -419,11 +428,12 @@ public static Collection<Object[]> 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
Expand Down Expand Up @@ -459,15 +469,18 @@ public static Collection<Object[]> 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 {
Expand All @@ -484,7 +497,8 @@ public static Collection<Object[]> 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));
}

/**
Expand All @@ -509,7 +523,8 @@ public static Collection<Object[]> 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 {
Expand All @@ -535,11 +550,12 @@ public static Collection<Object[]> 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 {
Expand All @@ -565,13 +581,14 @@ public static Collection<Object[]> 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
Expand All @@ -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

Expand Down
2 changes: 1 addition & 1 deletion common/src/test/java/org/conscrypt/TrustManagerImplTest.java
View file
Expand Up @@ -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) {
}
Expand Down

0 comments on commit b8009c1

Please sign in to comment.