From ddaf1c8ce96a99da01a3915d5a84c6efd9c09bea Mon Sep 17 00:00:00 2001 From: sanjaypujare Date: Tue, 15 Oct 2019 17:32:45 -0400 Subject: [PATCH] xds: fix to use the resource based TestUtils.loadCert (#6281) --- .../grpc/xds/sds/trust/CertificateUtils.java | 10 +- xds/src/test/certs/client.pem | 18 --- xds/src/test/certs/server1.pem | 16 --- .../sds/trust/SdsX509TrustManagerTest.java | 120 ++++++++---------- 4 files changed, 61 insertions(+), 103 deletions(-) delete mode 100644 xds/src/test/certs/client.pem delete mode 100644 xds/src/test/certs/server1.pem diff --git a/xds/src/main/java/io/grpc/xds/sds/trust/CertificateUtils.java b/xds/src/main/java/io/grpc/xds/sds/trust/CertificateUtils.java index f51312df29c..805f2039a3b 100644 --- a/xds/src/main/java/io/grpc/xds/sds/trust/CertificateUtils.java +++ b/xds/src/main/java/io/grpc/xds/sds/trust/CertificateUtils.java @@ -17,6 +17,7 @@ package io.grpc.xds.sds.trust; import java.io.BufferedInputStream; +import java.io.File; import java.io.FileInputStream; import java.io.IOException; import java.security.cert.Certificate; @@ -38,10 +39,15 @@ private static synchronized void initInstance() throws CertificateException { } } - static synchronized X509Certificate[] toX509Certificates(String fileName) + /** + * Generates X509Certificate array from a file on disk. + * + * @param file a {@link File} containing the cert data + */ + static synchronized X509Certificate[] toX509Certificates(File file) throws CertificateException, IOException { initInstance(); - FileInputStream fis = new FileInputStream(fileName); + FileInputStream fis = new FileInputStream(file); BufferedInputStream bis = new BufferedInputStream(fis); try { Collection certs = factory.generateCertificates(bis); diff --git a/xds/src/test/certs/client.pem b/xds/src/test/certs/client.pem deleted file mode 100644 index 913649b97fb..00000000000 --- a/xds/src/test/certs/client.pem +++ /dev/null @@ -1,18 +0,0 @@ ------BEGIN CERTIFICATE----- -MIIC6TCCAlKgAwIBAgIBCjANBgkqhkiG9w0BAQsFADBWMQswCQYDVQQGEwJBVTET -MBEGA1UECBMKU29tZS1TdGF0ZTEhMB8GA1UEChMYSW50ZXJuZXQgV2lkZ2l0cyBQ -dHkgTHRkMQ8wDQYDVQQDEwZ0ZXN0Y2EwHhcNMTUxMTEwMDEwOTU4WhcNMjUxMTA3 -MDEwOTU4WjBaMQswCQYDVQQGEwJBVTETMBEGA1UECAwKU29tZS1TdGF0ZTEhMB8G -A1UECgwYSW50ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMRMwEQYDVQQDDAp0ZXN0Y2xp -ZW50MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDsVEfbob4W3lVCDLOVmx9K -cdJnoZdvurGaTY87xNiopmaR8zCR7pFR9BX5L4bNG/PkuVLfVTVAKndyDCQggBBr -UTaEITNbfWK9swHJEr20WnKfhS/wo/Xg5sqNNCrFRmnnnwOA4eDlvmYZEzSnJXV6 -pEro9bBH9uOCWWLqmaev7QIDAQABo4HCMIG/MAkGA1UdEwQCMAAwCwYDVR0PBAQD -AgXgMB0GA1UdDgQWBBQAdbW5Vml/CnYwqdP3mOHDARU+8zBwBgNVHSMEaTBnoVqk -WDBWMQswCQYDVQQGEwJBVTETMBEGA1UECBMKU29tZS1TdGF0ZTEhMB8GA1UEChMY -SW50ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMQ8wDQYDVQQDEwZ0ZXN0Y2GCCQCRxhke -HRoqBzAJBgNVHREEAjAAMAkGA1UdEgQCMAAwDQYJKoZIhvcNAQELBQADgYEAf4MM -k+sdzd720DfrQ0PF2gDauR3M9uBubozDuMuF6ufAuQBJSKGQEGibXbUelrwHmnql -UjTyfolVcxEBVaF4VFHmn7u6vP7S1NexIDdNUHcULqxIb7Tzl8JYq8OOHD2rQy4H -s8BXaVIzw4YcaCGAMS0iDX052Sy7e2JhP8Noxvo= ------END CERTIFICATE----- diff --git a/xds/src/test/certs/server1.pem b/xds/src/test/certs/server1.pem deleted file mode 100644 index f3d43fcc5be..00000000000 --- a/xds/src/test/certs/server1.pem +++ /dev/null @@ -1,16 +0,0 @@ ------BEGIN CERTIFICATE----- -MIICnDCCAgWgAwIBAgIBBzANBgkqhkiG9w0BAQsFADBWMQswCQYDVQQGEwJBVTET -MBEGA1UECBMKU29tZS1TdGF0ZTEhMB8GA1UEChMYSW50ZXJuZXQgV2lkZ2l0cyBQ -dHkgTHRkMQ8wDQYDVQQDEwZ0ZXN0Y2EwHhcNMTUxMTA0MDIyMDI0WhcNMjUxMTAx -MDIyMDI0WjBlMQswCQYDVQQGEwJVUzERMA8GA1UECBMISWxsaW5vaXMxEDAOBgNV -BAcTB0NoaWNhZ28xFTATBgNVBAoTDEV4YW1wbGUsIENvLjEaMBgGA1UEAxQRKi50 -ZXN0Lmdvb2dsZS5jb20wgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAOHDFSco -LCVJpYDDM4HYtIdV6Ake/sMNaaKdODjDMsux/4tDydlumN+fm+AjPEK5GHhGn1Bg -zkWF+slf3BxhrA/8dNsnunstVA7ZBgA/5qQxMfGAq4wHNVX77fBZOgp9VlSMVfyd -9N8YwbBYAckOeUQadTi2X1S6OgJXgQ0m3MWhAgMBAAGjazBpMAkGA1UdEwQCMAAw -CwYDVR0PBAQDAgXgME8GA1UdEQRIMEaCECoudGVzdC5nb29nbGUuZnKCGHdhdGVy -em9vaS50ZXN0Lmdvb2dsZS5iZYISKi50ZXN0LnlvdXR1YmUuY29thwTAqAEDMA0G -CSqGSIb3DQEBCwUAA4GBAJFXVifQNub1LUP4JlnX5lXNlo8FxZ2a12AFQs+bzoJ6 -hM044EDjqyxUqSbVePK0ni3w1fHQB5rY9yYC5f8G7aqqTY1QOhoUk8ZTSTRpnkTh -y4jjdvTZeLDVBlueZUTDRmy2feY5aZIU18vFDK08dTG0A87pppuv1LNIR3loveU8 ------END CERTIFICATE----- diff --git a/xds/src/test/java/io/grpc/xds/sds/trust/SdsX509TrustManagerTest.java b/xds/src/test/java/io/grpc/xds/sds/trust/SdsX509TrustManagerTest.java index d3e2e5c9728..e6276120628 100644 --- a/xds/src/test/java/io/grpc/xds/sds/trust/SdsX509TrustManagerTest.java +++ b/xds/src/test/java/io/grpc/xds/sds/trust/SdsX509TrustManagerTest.java @@ -19,13 +19,13 @@ import static com.google.common.truth.Truth.assertThat; import io.envoyproxy.envoy.api.v2.auth.CertificateValidationContext; +import io.grpc.internal.testing.TestUtils; import java.io.FileNotFoundException; import java.io.IOException; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; import javax.net.ssl.X509ExtendedTrustManager; import org.junit.Assert; -import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @@ -39,15 +39,12 @@ */ @RunWith(JUnit4.class) public class SdsX509TrustManagerTest { - /** - * server1 has 4 SANs. - */ - private static final String SERVER_1_PEM_FILE = "src/test/certs/server1.pem"; - /** - * client has no SANs. - */ - private static final String CLIENT_PEM_FILE = "src/test/certs/client.pem"; + /** server1 has 4 SANs. */ + private static final String SERVER_1_PEM_FILE = "server1.pem"; + + /** client has no SANs. */ + private static final String CLIENT_PEM_FILE = "client.pem"; @Rule public final MockitoRule mockitoRule = MockitoJUnit.rule(); @@ -55,86 +52,76 @@ public class SdsX509TrustManagerTest { @Mock private X509ExtendedTrustManager mockDelegate; - @Ignore("test fails on blaze") @Test public void nullCertContextTest() throws CertificateException, IOException { SdsX509TrustManager trustManager = new SdsX509TrustManager(null, mockDelegate); - X509Certificate[] certs = CertificateUtils.toX509Certificates(SERVER_1_PEM_FILE); + X509Certificate[] certs = + CertificateUtils.toX509Certificates(TestUtils.loadCert(SERVER_1_PEM_FILE)); trustManager.verifySubjectAltNameInChain(certs); } - @Ignore("test fails on blaze") @Test public void emptySanListContextTest() throws CertificateException, IOException { CertificateValidationContext certContext = CertificateValidationContext.getDefaultInstance(); SdsX509TrustManager trustManager = new SdsX509TrustManager(certContext, mockDelegate); - X509Certificate[] certs = CertificateUtils.toX509Certificates(SERVER_1_PEM_FILE); + X509Certificate[] certs = + CertificateUtils.toX509Certificates(TestUtils.loadCert(SERVER_1_PEM_FILE)); trustManager.verifySubjectAltNameInChain(certs); } @Test public void missingPeerCerts() throws CertificateException, FileNotFoundException { - CertificateValidationContext certContext = CertificateValidationContext - .newBuilder() - .addVerifySubjectAltName("foo.com") - .build(); + CertificateValidationContext certContext = + CertificateValidationContext.newBuilder().addVerifySubjectAltName("foo.com").build(); SdsX509TrustManager trustManager = new SdsX509TrustManager(certContext, mockDelegate); try { trustManager.verifySubjectAltNameInChain(null); Assert.fail("no exception thrown"); } catch (CertificateException expected) { - assertThat(expected).hasMessageThat() - .isEqualTo("Peer certificate(s) missing"); + assertThat(expected).hasMessageThat().isEqualTo("Peer certificate(s) missing"); } } @Test public void emptyArrayPeerCerts() throws CertificateException, FileNotFoundException { - CertificateValidationContext certContext = CertificateValidationContext - .newBuilder() - .addVerifySubjectAltName("foo.com") - .build(); + CertificateValidationContext certContext = + CertificateValidationContext.newBuilder().addVerifySubjectAltName("foo.com").build(); SdsX509TrustManager trustManager = new SdsX509TrustManager(certContext, mockDelegate); try { trustManager.verifySubjectAltNameInChain(new X509Certificate[0]); Assert.fail("no exception thrown"); } catch (CertificateException expected) { - assertThat(expected).hasMessageThat() - .isEqualTo("Peer certificate(s) missing"); + assertThat(expected).hasMessageThat().isEqualTo("Peer certificate(s) missing"); } } - @Ignore("test fails on blaze") @Test public void noSansInPeerCerts() throws CertificateException, IOException { - CertificateValidationContext certContext = CertificateValidationContext - .newBuilder() - .addVerifySubjectAltName("foo.com") - .build(); + CertificateValidationContext certContext = + CertificateValidationContext.newBuilder().addVerifySubjectAltName("foo.com").build(); SdsX509TrustManager trustManager = new SdsX509TrustManager(certContext, mockDelegate); - X509Certificate[] certs = CertificateUtils.toX509Certificates(CLIENT_PEM_FILE); + X509Certificate[] certs = + CertificateUtils.toX509Certificates(TestUtils.loadCert(CLIENT_PEM_FILE)); try { trustManager.verifySubjectAltNameInChain(certs); Assert.fail("no exception thrown"); } catch (CertificateException expected) { - assertThat(expected).hasMessageThat() - .isEqualTo("Peer certificate SAN check failed"); + assertThat(expected).hasMessageThat().isEqualTo("Peer certificate SAN check failed"); } } - @Ignore("test fails on blaze") @Test public void oneSanInPeerCertsVerifies() throws CertificateException, IOException { - CertificateValidationContext certContext = CertificateValidationContext - .newBuilder() + CertificateValidationContext certContext = + CertificateValidationContext.newBuilder() .addVerifySubjectAltName("waterzooi.test.google.be") .build(); SdsX509TrustManager trustManager = new SdsX509TrustManager(certContext, mockDelegate); - X509Certificate[] certs = CertificateUtils.toX509Certificates(SERVER_1_PEM_FILE); + X509Certificate[] certs = + CertificateUtils.toX509Certificates(TestUtils.loadCert(SERVER_1_PEM_FILE)); trustManager.verifySubjectAltNameInChain(certs); } - @Ignore("test fails on blaze") @Test public void oneSanInPeerCertsVerifiesMultipleVerifySans() throws CertificateException, IOException { @@ -144,18 +131,19 @@ public void oneSanInPeerCertsVerifiesMultipleVerifySans() .addVerifySubjectAltName("waterzooi.test.google.be") .build(); SdsX509TrustManager trustManager = new SdsX509TrustManager(certContext, mockDelegate); - X509Certificate[] certs = CertificateUtils.toX509Certificates(SERVER_1_PEM_FILE); + X509Certificate[] certs = + CertificateUtils.toX509Certificates(TestUtils.loadCert(SERVER_1_PEM_FILE)); trustManager.verifySubjectAltNameInChain(certs); } - @Ignore("test fails on blaze") @Test public void oneSanInPeerCertsNotFoundException() throws CertificateException, IOException { CertificateValidationContext certContext = CertificateValidationContext.newBuilder().addVerifySubjectAltName("x.foo.com").build(); SdsX509TrustManager trustManager = new SdsX509TrustManager(certContext, mockDelegate); - X509Certificate[] certs = CertificateUtils.toX509Certificates(SERVER_1_PEM_FILE); + X509Certificate[] certs = + CertificateUtils.toX509Certificates(TestUtils.loadCert(SERVER_1_PEM_FILE)); try { trustManager.verifySubjectAltNameInChain(certs); Assert.fail("no exception thrown"); @@ -164,85 +152,83 @@ public void oneSanInPeerCertsNotFoundException() } } - @Ignore("test fails on blaze") @Test public void wildcardSanInPeerCertsVerifiesMultipleVerifySans() throws CertificateException, IOException { - CertificateValidationContext certContext = CertificateValidationContext - .newBuilder() + CertificateValidationContext certContext = + CertificateValidationContext.newBuilder() .addVerifySubjectAltName("x.foo.com") - .addVerifySubjectAltName("abc.test.youtube.com") // should match *.test.youtube.com + .addVerifySubjectAltName("abc.test.youtube.com") // should match *.test.youtube.com .build(); SdsX509TrustManager trustManager = new SdsX509TrustManager(certContext, mockDelegate); - X509Certificate[] certs = CertificateUtils.toX509Certificates(SERVER_1_PEM_FILE); + X509Certificate[] certs = + CertificateUtils.toX509Certificates(TestUtils.loadCert(SERVER_1_PEM_FILE)); trustManager.verifySubjectAltNameInChain(certs); } - @Ignore("test fails on blaze") @Test public void wildcardSanInPeerCertsVerifiesMultipleVerifySans1() throws CertificateException, IOException { - CertificateValidationContext certContext = CertificateValidationContext - .newBuilder() + CertificateValidationContext certContext = + CertificateValidationContext.newBuilder() .addVerifySubjectAltName("x.foo.com") - .addVerifySubjectAltName("abc.test.google.fr") // should match *.test.google.fr + .addVerifySubjectAltName("abc.test.google.fr") // should match *.test.google.fr .build(); SdsX509TrustManager trustManager = new SdsX509TrustManager(certContext, mockDelegate); - X509Certificate[] certs = CertificateUtils.toX509Certificates(SERVER_1_PEM_FILE); + X509Certificate[] certs = + CertificateUtils.toX509Certificates(TestUtils.loadCert(SERVER_1_PEM_FILE)); trustManager.verifySubjectAltNameInChain(certs); } - @Ignore("test fails on blaze") @Test public void wildcardSanInPeerCertsSubdomainMismatch() throws CertificateException, IOException { // 2. Asterisk (*) cannot match across domain name labels. // For example, *.example.com matches test.example.com but does not match // sub.test.example.com. - CertificateValidationContext certContext = CertificateValidationContext - .newBuilder() + CertificateValidationContext certContext = + CertificateValidationContext.newBuilder() .addVerifySubjectAltName("sub.abc.test.youtube.com") .build(); SdsX509TrustManager trustManager = new SdsX509TrustManager(certContext, mockDelegate); - X509Certificate[] certs = CertificateUtils.toX509Certificates(SERVER_1_PEM_FILE); + X509Certificate[] certs = + CertificateUtils.toX509Certificates(TestUtils.loadCert(SERVER_1_PEM_FILE)); try { trustManager.verifySubjectAltNameInChain(certs); Assert.fail("no exception thrown"); } catch (CertificateException expected) { - assertThat(expected).hasMessageThat() - .isEqualTo("Peer certificate SAN check failed"); + assertThat(expected).hasMessageThat().isEqualTo("Peer certificate SAN check failed"); } } - @Ignore("test fails on blaze") @Test public void oneIpAddressInPeerCertsVerifies() throws CertificateException, IOException { - CertificateValidationContext certContext = CertificateValidationContext - .newBuilder() + CertificateValidationContext certContext = + CertificateValidationContext.newBuilder() .addVerifySubjectAltName("x.foo.com") .addVerifySubjectAltName("192.168.1.3") .build(); SdsX509TrustManager trustManager = new SdsX509TrustManager(certContext, mockDelegate); - X509Certificate[] certs = CertificateUtils.toX509Certificates(SERVER_1_PEM_FILE); + X509Certificate[] certs = + CertificateUtils.toX509Certificates(TestUtils.loadCert(SERVER_1_PEM_FILE)); trustManager.verifySubjectAltNameInChain(certs); } - @Ignore("test fails on blaze") @Test public void oneIpAddressInPeerCertsMismatch() throws CertificateException, IOException { - CertificateValidationContext certContext = CertificateValidationContext - .newBuilder() + CertificateValidationContext certContext = + CertificateValidationContext.newBuilder() .addVerifySubjectAltName("x.foo.com") .addVerifySubjectAltName("192.168.2.3") .build(); SdsX509TrustManager trustManager = new SdsX509TrustManager(certContext, mockDelegate); - X509Certificate[] certs = CertificateUtils.toX509Certificates(SERVER_1_PEM_FILE); + X509Certificate[] certs = + CertificateUtils.toX509Certificates(TestUtils.loadCert(SERVER_1_PEM_FILE)); try { trustManager.verifySubjectAltNameInChain(certs); Assert.fail("no exception thrown"); } catch (CertificateException expected) { - assertThat(expected).hasMessageThat() - .isEqualTo("Peer certificate SAN check failed"); + assertThat(expected).hasMessageThat().isEqualTo("Peer certificate SAN check failed"); } } }