From 6dc7c7f36e9e377779a6a998b5807b33180e0020 Mon Sep 17 00:00:00 2001 From: Tobias Thierer Date: Wed, 10 Jan 2018 21:04:31 +0000 Subject: [RESTRICT AUTOMERGE] Track behavior change in default HostnameVerifier. Oreo MR1 will no longer support hostnames in the CN field of X.509 certificates after the next security bulletin, so this CL cherry-picks the relevant test changes from Pie. Original change description: The default HostnameVerifier now ignores any CommonName in the certificate provided by the server, even when no subjectAltName is present. As well as the cherrypick for the CommonName behaviour change, this also contains changes to match http://ag/13135278 which rejects non-ASCII names. Bug: 70278814 Bug: 171980069 Test: CtsLibcoreTestCases Change-Id: Ib6fa0c40d8903352e88d8812bf0c09ec1d8ef6be (cherry picked from commit 70bd0982aa6ed2603615df8a963f285b91872c87) --- .../tests/javax/net/ssl/HostnameVerifierTest.java | 31 ++++-- .../java/libcore/java/net/URLConnectionTest.java | 2 +- .../javax/net/ssl/DefaultHostnameVerifierTest.java | 114 +++++++++++++-------- .../java/libcore/java/security/TestKeyStore.java | 4 +- 4 files changed, 92 insertions(+), 59 deletions(-) diff --git a/harmony-tests/src/test/java/org/apache/harmony/tests/javax/net/ssl/HostnameVerifierTest.java b/harmony-tests/src/test/java/org/apache/harmony/tests/javax/net/ssl/HostnameVerifierTest.java index 2ca3f8bd05..44572ab3a5 100644 --- a/harmony-tests/src/test/java/org/apache/harmony/tests/javax/net/ssl/HostnameVerifierTest.java +++ b/harmony-tests/src/test/java/org/apache/harmony/tests/javax/net/ssl/HostnameVerifierTest.java @@ -94,18 +94,27 @@ public class HostnameVerifierTest extends TestCase implements * out certificates that match so broadly. */ public void testWildcardsDoesNotNeedTwoDots() throws Exception { - // openssl req -x509 -nodes -days 36500 -subj '/CN=*.com' -newkey rsa:512 -out cert.pem + /* + * $ cat ./cert.cnf + * [req] + * distinguished_name=distinguished_name + * req_extensions=req_extensions + * x509_extensions=x509_extensions + * [distinguished_name] + * [req_extensions] + * [x509_extensions] + * subjectAltName=DNS:*.com + */ + // openssl req -x509 -nodes -days 36500 -subj '/CN=CommonName' -config ./cert.cnf -newkey rsa:512 -out cert.pem String cert = "-----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-----"; + + "MIIBODCB46ADAgECAgkA5o09Q/EN/kMwDQYJKoZIhvcNAQELBQAwFTETMBEGA1UE\n" + + "AxMKQ29tbW9uTmFtZTAgFw0xODAxMTEwMDM1MDNaGA8yMTE3MTIxODAwMzUwM1ow\n" + + "FTETMBEGA1UEAxMKQ29tbW9uTmFtZTBcMA0GCSqGSIb3DQEBAQUAA0sAMEgCQQDE\n" + + "u2Yguj/n8mUvmEVIJeSxbtcK98yCkg07BIVPQaRBpBTjWk/lxRWlMGVAWTcls1El\n" + + "IvLn+/NsBLx5l4UFfkDFAgMBAAGjFDASMBAGA1UdEQQJMAeCBSouY29tMA0GCSqG\n" + + "SIb3DQEBCwUAA0EASyUpA60cGL8ePVO5XD4XGGIms5Dwd147+wiqKcYodnB8rlbF\n" + + "nxeiH6VZH3lBKJjrAXB0rOaBzb9jCuVxjYldew==\n" + + "-----END CERTIFICATE-----\n"; CertificateFactory cf = CertificateFactory.getInstance("X.509"); InputStream in = new ByteArrayInputStream(cert.getBytes("UTF-8")); X509Certificate x509 = (X509Certificate) cf.generateCertificate(in); diff --git a/luni/src/test/java/libcore/java/net/URLConnectionTest.java b/luni/src/test/java/libcore/java/net/URLConnectionTest.java index 750e73a56f..f8d82fc5cb 100644 --- a/luni/src/test/java/libcore/java/net/URLConnectionTest.java +++ b/luni/src/test/java/libcore/java/net/URLConnectionTest.java @@ -2058,7 +2058,7 @@ public final class URLConnectionTest extends TestCase { assertEquals(Arrays.asList("verify " + hostName), hostnameVerifier.calls); assertEquals(Arrays.asList("checkServerTrusted [" - + "CN=" + hostName + " 3, " + + "CN=Local Host 3, " + "CN=Test Intermediate Certificate Authority 2, " + "CN=Test Root Certificate Authority 1" + "] ECDHE_RSA"), diff --git a/luni/src/test/java/libcore/javax/net/ssl/DefaultHostnameVerifierTest.java b/luni/src/test/java/libcore/javax/net/ssl/DefaultHostnameVerifierTest.java index 07ecd12acf..3884a0e47e 100644 --- a/luni/src/test/java/libcore/javax/net/ssl/DefaultHostnameVerifierTest.java +++ b/luni/src/test/java/libcore/javax/net/ssl/DefaultHostnameVerifierTest.java @@ -45,18 +45,26 @@ import junit.framework.TestCase; */ public final class DefaultHostnameVerifierTest extends TestCase { private static final int ALT_UNKNOWN = 0; - private static final int ALT_DNS_NAME = 2; - private static final int ALT_IPA_NAME = 7; + private static final int ALT_DNS_NAME = 2; // DNS name + private static final int ALT_IPA_NAME = 7; // IP address private final HostnameVerifier verifier = HttpsURLConnection.getDefaultHostnameVerifier(); - public void testVerify() { - assertTrue(verifyWithServerCertificate( - "imap.g.com", new StubX509Certificate("cn=imap.g.com"))); + public void testVerify_wrongHost() { + assertFalse(verifyWithServerCertificate( + "imap.g.com", StubX509Certificate.dns("imap2.g.com"))); assertFalse(verifyWithServerCertificate( - "imap.g.com", new StubX509Certificate("cn=imap2.g.com"))); + "imap.g.com", StubX509Certificate.dns("sub.imap.g.com"))); + } + + public void testVerify_matchesAltNameButNotCommonName() { + assertTrue(verifyWithServerCertificate( + "imap.g.com", new StubX509Certificate("Common Name") + .addSubjectAlternativeName(ALT_DNS_NAME, "imap.g.com"))); + assertFalse(verifyWithServerCertificate( - "imap.g.com", new StubX509Certificate("cn=sub.imap.g.com"))); + "imap.g.com", new StubX509Certificate("imap.g.com") + .addSubjectAlternativeName(ALT_DNS_NAME, "example.com"))); } /** @@ -64,33 +72,28 @@ public final class DefaultHostnameVerifierTest extends TestCase { * be used as the identity and the CN should be ignored. */ public void testSubjectAltNameAndCn() { - assertFalse(verifyWithServerCertificate("imap.g.com", new StubX509Certificate("") + assertFalse(verifyWithServerCertificate("imap.g.com", new StubX509Certificate() .addSubjectAlternativeName(ALT_DNS_NAME, "a.y.com"))); - assertFalse( - verifyWithServerCertificate("imap.g.com", new StubX509Certificate("cn=imap.g.com") - .addSubjectAlternativeName(ALT_DNS_NAME, "a.y.com"))); - assertTrue(verifyWithServerCertificate("imap.g.com", new StubX509Certificate("") + assertFalse(verifyWithServerCertificate("imap.g.com", new StubX509Certificate("imap.g.com") + .addSubjectAlternativeName(ALT_DNS_NAME, "a.y.com"))); + assertTrue(verifyWithServerCertificate("imap.g.com", new StubX509Certificate() .addSubjectAlternativeName(ALT_DNS_NAME, "imap.g.com"))); } public void testSubjectAltNameWithWildcard() { - assertTrue(verifyWithServerCertificate("imap.g.com", new StubX509Certificate("") - .addSubjectAlternativeName(ALT_DNS_NAME, "*.g.com"))); + assertTrue(verifyWithServerCertificate("imap.g.com", StubX509Certificate.dns("*.g.com"))); } public void testSubjectAltNameWithIpAddress() { - assertTrue(verifyWithServerCertificate("1.2.3.4", new StubX509Certificate("") - .addSubjectAlternativeName(ALT_IPA_NAME, "1.2.3.4"))); - assertFalse(verifyWithServerCertificate("1.2.3.5", new StubX509Certificate("") - .addSubjectAlternativeName(ALT_IPA_NAME, "1.2.3.4"))); - assertTrue(verifyWithServerCertificate("192.168.100.1", new StubX509Certificate("") - .addSubjectAlternativeName(ALT_IPA_NAME, "1.2.3.4") - .addSubjectAlternativeName(ALT_IPA_NAME, "192.168.100.1"))); + assertTrue(verifyWithServerCertificate("1.2.3.4", StubX509Certificate.ipa("1.2.3.4"))); + assertFalse(verifyWithServerCertificate("1.2.3.5", StubX509Certificate.ipa("1.2.3.4"))); + assertTrue(verifyWithServerCertificate("192.168.100.1", + StubX509Certificate.ipa("1.2.3.4", "192.168.100.1"))); } public void testUnknownSubjectAltName() { // Has unknown subject alternative names - assertTrue(verifyWithServerCertificate("imap.g.com", new StubX509Certificate("") + assertTrue(verifyWithServerCertificate("imap.g.com", new StubX509Certificate() .addSubjectAlternativeName(ALT_UNKNOWN, "random string 1") .addSubjectAlternativeName(ALT_UNKNOWN, "random string 2") .addSubjectAlternativeName(ALT_DNS_NAME, "a.b.c.d") @@ -98,7 +101,7 @@ public final class DefaultHostnameVerifierTest extends TestCase { .addSubjectAlternativeName(ALT_DNS_NAME, "imap.g.com") .addSubjectAlternativeName(ALT_IPA_NAME, "2.33.44.55") .addSubjectAlternativeName(ALT_UNKNOWN, "random string 3"))); - assertTrue(verifyWithServerCertificate("2.33.44.55", new StubX509Certificate("") + assertTrue(verifyWithServerCertificate("2.33.44.55", new StubX509Certificate() .addSubjectAlternativeName(ALT_UNKNOWN, "random string 1") .addSubjectAlternativeName(ALT_UNKNOWN, "random string 2") .addSubjectAlternativeName(ALT_DNS_NAME, "a.b.c.d") @@ -106,7 +109,7 @@ public final class DefaultHostnameVerifierTest extends TestCase { .addSubjectAlternativeName(ALT_DNS_NAME, "imap.g.com") .addSubjectAlternativeName(ALT_IPA_NAME, "2.33.44.55") .addSubjectAlternativeName(ALT_UNKNOWN, "random string 3"))); - assertFalse(verifyWithServerCertificate("g.com", new StubX509Certificate("") + assertFalse(verifyWithServerCertificate("g.com", new StubX509Certificate() .addSubjectAlternativeName(ALT_UNKNOWN, "random string 1") .addSubjectAlternativeName(ALT_UNKNOWN, "random string 2") .addSubjectAlternativeName(ALT_DNS_NAME, "a.b.c.d") @@ -114,7 +117,7 @@ public final class DefaultHostnameVerifierTest extends TestCase { .addSubjectAlternativeName(ALT_DNS_NAME, "imap.g.com") .addSubjectAlternativeName(ALT_IPA_NAME, "2.33.44.55") .addSubjectAlternativeName(ALT_UNKNOWN, "random string 3"))); - assertFalse(verifyWithServerCertificate("2.33.44.1", new StubX509Certificate("") + assertFalse(verifyWithServerCertificate("2.33.44.1", new StubX509Certificate() .addSubjectAlternativeName(ALT_UNKNOWN, "random string 1") .addSubjectAlternativeName(ALT_UNKNOWN, "random string 2") .addSubjectAlternativeName(ALT_DNS_NAME, "a.b.c.d") @@ -125,14 +128,14 @@ public final class DefaultHostnameVerifierTest extends TestCase { } public void testWildcardsRejectedForIpAddress() { - assertFalse(verifyWithServerCertificate("1.2.3.4", new StubX509Certificate("cn=*.2.3.4"))); - assertFalse(verifyWithServerCertificate("1.2.3.4", new StubX509Certificate("cn=*.2.3.4") + assertFalse(verifyWithServerCertificate("1.2.3.4", new StubX509Certificate("*.2.3.4"))); + assertFalse(verifyWithServerCertificate("1.2.3.4", new StubX509Certificate("*.2.3.4") .addSubjectAlternativeName(ALT_IPA_NAME, "*.2.3.4") .addSubjectAlternativeName(ALT_DNS_NAME, "*.2.3.4"))); assertFalse(verifyWithServerCertificate( - "2001:1234::1", new StubX509Certificate("cn=*:1234::1"))); + "2001:1234::1", new StubX509Certificate("*:1234::1"))); assertFalse(verifyWithServerCertificate( - "2001:1234::1", new StubX509Certificate("cn=*:1234::1") + "2001:1234::1", new StubX509Certificate("*:1234::1") .addSubjectAlternativeName(ALT_IPA_NAME, "*:1234::1") .addSubjectAlternativeName(ALT_DNS_NAME, "*:1234::1"))); } @@ -270,7 +273,7 @@ public final class DefaultHostnameVerifierTest extends TestCase { + "rs2oQLwOLnuifH52ey9+tJguabo+brlYYigAuWWFEzJfBzikDkIwnE/L7wlrypIk\n" + "taXDWI4=\n" + "-----END CERTIFICATE-----"); - assertTrue(verifyWithServerCertificate("www.example.com", cert)); + assertFalse(verifyWithServerCertificate("www.example.com", cert)); assertFalse(verifyWithServerCertificate("www2.example.com", cert)); } @@ -330,7 +333,7 @@ public final class DefaultHostnameVerifierTest extends TestCase { public void testSubjectWithWildAltNamesCert() throws Exception { // subject: C=JP, CN=www.example.com // subject alt names: DNS:*.example2.com - // * Subject should be ignored, because it has subject alt names. + // CN should be ignored in all cases, only subject alt names should be considered. X509Certificate cert = parseCertificate("-----BEGIN CERTIFICATE-----\n" + "MIIC8DCCAdigAwIBAgIJAL/oWJ64VAdXMA0GCSqGSIb3DQEBBQUAMCcxCzAJBgNV\n" + "BAYTAkpQMRgwFgYDVQQDEw93d3cuZXhhbXBsZS5jb20wIBcNMTAwMTEyMjEwMDAx\n" @@ -359,6 +362,7 @@ public final class DefaultHostnameVerifierTest extends TestCase { public void testWildAltNameOnlyCert() throws Exception { // subject: C=JP // subject alt names: DNS:*.example.com + // CN should be ignored in all cases, only subject alt names should be considered. X509Certificate cert = parseCertificate("-----BEGIN CERTIFICATE-----\n" + "MIICuzCCAaOgAwIBAgIJAP82tgcvmAGxMA0GCSqGSIb3DQEBBQUAMA0xCzAJBgNV\n" + "BAYTAkpQMCAXDTEwMDExMjIxMDAyN1oYDzIwNjQxMDE1MjEwMDI3WjANMQswCQYD\n" @@ -384,6 +388,7 @@ public final class DefaultHostnameVerifierTest extends TestCase { public void testAltIpOnlyCert() throws Exception { // subject: C=JP // subject alt names: IP Address:192.168.10.1 + // CN should be ignored in all cases, only subject alt names should be considered. X509Certificate cert = parseCertificate("-----BEGIN CERTIFICATE-----\n" + "MIICsjCCAZqgAwIBAgIJALrC37YAXFIeMA0GCSqGSIb3DQEBBQUAMA0xCzAJBgNV\n" + "BAYTAkpQMCAXDTEwMDExMjIxMzk0NloYDzIwNjQxMDE1MjEzOTQ2WjANMQswCQYD\n" @@ -416,23 +421,14 @@ public final class DefaultHostnameVerifierTest extends TestCase { session.peerCertificates = new Certificate[] { new StubX509Certificate("cn=\"" + pattern + "\"") }; - boolean resultWhenPatternInCn = verifier.verify(hostname, session); + assertFalse("Verifier should ignore CN.", verifier.verify(hostname, session)); // Verify using a certificate where the pattern is in a DNS SubjectAltName session.peerCertificates = new Certificate[] { new StubX509Certificate("ou=test") .addSubjectAlternativeName(ALT_DNS_NAME, pattern) }; - boolean resultWhenPatternInSubjectAltName = verifier.verify(hostname, session); - - // Assert that in both cases the verifier gives the same result - if (resultWhenPatternInCn != resultWhenPatternInSubjectAltName) { - fail("Different results between pattern in CN and SubjectAltName." - + " hostname : " + hostname + ", pattern: " + pattern - + ", when pattern in CN: " + resultWhenPatternInCn - + ", when pattern in SubjectAltName: " + resultWhenPatternInSubjectAltName); - } - return resultWhenPatternInCn; + return verifier.verify(hostname, session); } /** @@ -565,12 +561,40 @@ public final class DefaultHostnameVerifierTest extends TestCase { private final X500Principal subjectX500Principal; private Collection> subjectAlternativeNames; - public StubX509Certificate(String subjectDn) { - subjectX500Principal = new X500Principal(subjectDn); + public StubX509Certificate() { + subjectX500Principal = new X500Principal(""); + subjectAlternativeNames = null; + } + + public StubX509Certificate(String commonName) { + subjectX500Principal = new X500Principal("cn=" + commonName); subjectAlternativeNames = null; } - public StubX509Certificate addSubjectAlternativeName(int type, String name) { + public static StubX509Certificate of(int type, String... altNames) { + StubX509Certificate result = new StubX509Certificate(); + for (String altName : altNames) { + result.addSubjectAlternativeName(type, altName); + } + return result; + } + + /** + * A StubX509Certificate with {@link #ALT_DNS_NAME} subjectAlternativeNames. + */ + public static StubX509Certificate dns(String... dnsNames) { + return of(ALT_DNS_NAME, dnsNames); + } + + /** + * A StubX509Certificate with {@link #ALT_IPA_NAME} subjectAlternativeNames. + */ + public static StubX509Certificate ipa(String... ipaNames) { + return of(ALT_IPA_NAME, ipaNames); + } + + + public final StubX509Certificate addSubjectAlternativeName(int type, String name) { if (subjectAlternativeNames == null) { subjectAlternativeNames = new ArrayList>(); } diff --git a/support/src/test/java/libcore/java/security/TestKeyStore.java b/support/src/test/java/libcore/java/security/TestKeyStore.java index 3829dc1e24..be9874f6e9 100644 --- a/support/src/test/java/libcore/java/security/TestKeyStore.java +++ b/support/src/test/java/libcore/java/security/TestKeyStore.java @@ -230,7 +230,7 @@ public final class TestKeyStore { .aliasPrefix("server") .signer(INTERMEDIATE_CA.getPrivateKey("RSA", "RSA")) .rootCa(INTERMEDIATE_CA.getRootCertificate("RSA")) - .addSubjectAltNameIpAddress(LOCAL_HOST_ADDRESS) + .addSubjectAltName(new GeneralName(GeneralName.dNSName, LOCAL_HOST_NAME)) .certificateSerialNumber(BigInteger.valueOf(3)) .build(); CLIENT = new TestKeyStore(createClient(INTERMEDIATE_CA.keyStore), null, null); @@ -641,7 +641,7 @@ public final class TestKeyStore { } private X500Principal localhost() { - return new X500Principal("CN=" + LOCAL_HOST_NAME); + return new X500Principal("CN=Local Host"); } } -- cgit v1.2.3