From 637a302d8517f64dbded4ef8eaf232367ae25e0c Mon Sep 17 00:00:00 2001 From: Jay Modi Date: Thu, 19 Nov 2020 10:46:19 -0700 Subject: [PATCH] HttpsServer can use TLSv1.3 on JDK16+ (#64496) This commit changes code that previously pinned to TLSv1.2 when running on JDK 12+ to allow the use of TLSv1.3 if on JDK 16 or newer. There was a bug in the HttpsServer code that has finally been fixed, which prevented the use of TLSv1.3 as the HttpsServer would endlessly loop. The JDK bug is JDK-8254967. Closes #38646 --- .../reindex/ReindexRestClientSslTests.java | 51 ++++++++++++------- 1 file changed, 34 insertions(+), 17 deletions(-) diff --git a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ReindexRestClientSslTests.java b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ReindexRestClientSslTests.java index 6dd989ffafdb5..5f4bc6883fe0c 100644 --- a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ReindexRestClientSslTests.java +++ b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/ReindexRestClientSslTests.java @@ -23,6 +23,7 @@ import com.sun.net.httpserver.HttpsExchange; import com.sun.net.httpserver.HttpsParameters; import com.sun.net.httpserver.HttpsServer; +import org.elasticsearch.bootstrap.JavaVersion; import org.elasticsearch.client.Request; import org.elasticsearch.client.Response; import org.elasticsearch.client.RestClient; @@ -102,7 +103,7 @@ public static void shutdownHttpServer() { } private static SSLContext buildServerSslContext() throws Exception { - final SSLContext sslContext = SSLContext.getInstance("TLSv1.2"); + final SSLContext sslContext = SSLContext.getInstance(isHttpsServerBrokenWithTLSv13() ? "TLSv1.2" : "TLS"); final char[] password = "http-password".toCharArray(); final Path cert = PathUtils.get(ReindexRestClientSslTests.class.getResource("http/http.crt").toURI()); @@ -119,10 +120,12 @@ private static SSLContext buildServerSslContext() throws Exception { public void testClientFailsWithUntrustedCertificate() throws IOException { assumeFalse("https://github.com/elastic/elasticsearch/issues/49094", inFipsJvm()); final List threads = new ArrayList<>(); - final Settings settings = Settings.builder() - .put("path.home", createTempDir()) - .put("reindex.ssl.supported_protocols", "TLSv1.2") - .build(); + final Settings.Builder builder = Settings.builder() + .put("path.home", createTempDir()); + if (isHttpsServerBrokenWithTLSv13()) { + builder.put("reindex.ssl.supported_protocols", "TLSv1.2"); + } + final Settings settings = builder.build(); final Environment environment = TestEnvironment.newEnvironment(settings); final ReindexSslConfig ssl = new ReindexSslConfig(settings, environment, mock(ResourceWatcherService.class)); try (RestClient client = Reindexer.buildRestClient(getRemoteInfo(), ssl, 1L, threads)) { @@ -133,11 +136,13 @@ public void testClientFailsWithUntrustedCertificate() throws IOException { public void testClientSucceedsWithCertificateAuthorities() throws IOException { final List threads = new ArrayList<>(); final Path ca = getDataPath("ca.pem"); - final Settings settings = Settings.builder() + final Settings.Builder builder = Settings.builder() .put("path.home", createTempDir()) - .putList("reindex.ssl.certificate_authorities", ca.toString()) - .put("reindex.ssl.supported_protocols", "TLSv1.2") - .build(); + .putList("reindex.ssl.certificate_authorities", ca.toString()); + if (isHttpsServerBrokenWithTLSv13()) { + builder.put("reindex.ssl.supported_protocols", "TLSv1.2"); + } + final Settings settings = builder.build(); final Environment environment = TestEnvironment.newEnvironment(settings); final ReindexSslConfig ssl = new ReindexSslConfig(settings, environment, mock(ResourceWatcherService.class)); try (RestClient client = Reindexer.buildRestClient(getRemoteInfo(), ssl, 1L, threads)) { @@ -149,11 +154,13 @@ public void testClientSucceedsWithCertificateAuthorities() throws IOException { public void testClientSucceedsWithVerificationDisabled() throws IOException { assumeFalse("Cannot disable verification in FIPS JVM", inFipsJvm()); final List threads = new ArrayList<>(); - final Settings settings = Settings.builder() + final Settings.Builder builder = Settings.builder() .put("path.home", createTempDir()) - .put("reindex.ssl.verification_mode", "NONE") - .put("reindex.ssl.supported_protocols", "TLSv1.2") - .build(); + .put("reindex.ssl.verification_mode", "NONE"); + if (isHttpsServerBrokenWithTLSv13()) { + builder.put("reindex.ssl.supported_protocols", "TLSv1.2"); + } + final Settings settings = builder.build(); final Environment environment = TestEnvironment.newEnvironment(settings); final ReindexSslConfig ssl = new ReindexSslConfig(settings, environment, mock(ResourceWatcherService.class)); try (RestClient client = Reindexer.buildRestClient(getRemoteInfo(), ssl, 1L, threads)) { @@ -167,14 +174,16 @@ public void testClientPassesClientCertificate() throws IOException { final Path ca = getDataPath("ca.pem"); final Path cert = getDataPath("client/client.crt"); final Path key = getDataPath("client/client.key"); - final Settings settings = Settings.builder() + final Settings.Builder builder = Settings.builder() .put("path.home", createTempDir()) .putList("reindex.ssl.certificate_authorities", ca.toString()) .put("reindex.ssl.certificate", cert) .put("reindex.ssl.key", key) - .put("reindex.ssl.key_passphrase", "client-password") - .put("reindex.ssl.supported_protocols", "TLSv1.2") - .build(); + .put("reindex.ssl.key_passphrase", "client-password"); + if (isHttpsServerBrokenWithTLSv13()) { + builder.put("reindex.ssl.supported_protocols", "TLSv1.2"); + } + final Settings settings = builder.build(); AtomicReference clientCertificates = new AtomicReference<>(); handler = https -> { try { @@ -216,4 +225,12 @@ public void configure(HttpsParameters params) { params.setWantClientAuth(true); } } + + /** + * Checks whether the JVM this test is run under is affected by JDK-8254967, which causes these + * tests to fail if a TLSv1.3 SSLContext is used. + */ + private static boolean isHttpsServerBrokenWithTLSv13() { + return JavaVersion.current().compareTo(JavaVersion.parse("16.0.0")) < 0; + } }