From 0ead37d55bacebfc88cacb57b52ceddaa61c9441 Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Thu, 10 May 2018 10:55:50 +0300 Subject: [PATCH 1/3] Make SSLContext reloadable In JVMs running in FIPS approved mode, only SunJSSE TrustManagers and KeyManagers can be used. This commit replaces all the custom KeyManagers and TrustManagers (ReloadableKeyManager, ReloadableTrustManager, EmptyKeyManager, EmptyTrustManager) with instances of X509ExtendedKeyManager and X509ExtendedTrustManager. Reloadability is now ensured by a volatile instance of SSLContext in SSLContectHolder. SSLConfigurationReloaderTests use the reloadable SSLContext to initialize HTTP Clients and Servers and use these for testing the key material and trust relations. --- .../xpack/core/ssl/SSLService.java | 303 ++-------- .../ssl/SSLConfigurationReloaderTests.java | 520 +++++++++--------- .../xpack/core/ssl/SSLServiceTests.java | 23 +- 3 files changed, 321 insertions(+), 525 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java index c59a2889c28db..8cf7cceeffdb6 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java @@ -8,7 +8,6 @@ import org.apache.http.conn.ssl.NoopHostnameVerifier; import org.apache.http.nio.conn.ssl.SSLIOSessionStrategy; import org.apache.lucene.util.SetOnce; -import org.bouncycastle.operator.OperatorCreationException; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.common.CheckedSupplier; import org.elasticsearch.common.Strings; @@ -21,28 +20,24 @@ import org.elasticsearch.xpack.core.ssl.cert.CertificateInfo; import javax.net.ssl.HostnameVerifier; +import javax.net.ssl.KeyManagerFactory; import javax.net.ssl.SSLContext; import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLParameters; import javax.net.ssl.SSLSessionContext; import javax.net.ssl.SSLSocket; import javax.net.ssl.SSLSocketFactory; +import javax.net.ssl.TrustManagerFactory; import javax.net.ssl.X509ExtendedKeyManager; import javax.net.ssl.X509ExtendedTrustManager; -import javax.security.auth.DestroyFailedException; import java.io.IOException; import java.net.InetAddress; import java.net.Socket; import java.security.GeneralSecurityException; import java.security.KeyManagementException; -import java.security.KeyStoreException; +import java.security.KeyStore; import java.security.NoSuchAlgorithmException; -import java.security.Principal; -import java.security.PrivateKey; -import java.security.UnrecoverableKeyException; -import java.security.cert.CertificateException; -import java.security.cert.X509Certificate; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -54,6 +49,7 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.Optional; import java.util.Set; /** @@ -71,8 +67,7 @@ public class SSLService extends AbstractComponent { * Create a new SSLService that parses the settings for the ssl contexts that need to be created, creates them, and then caches them * for use later */ - public SSLService(Settings settings, Environment environment) throws CertificateException, UnrecoverableKeyException, - NoSuchAlgorithmException, IOException, DestroyFailedException, KeyStoreException, OperatorCreationException { + public SSLService(Settings settings, Environment environment) { super(settings); this.env = environment; this.globalSSLConfiguration = new SSLConfiguration(settings.getByPrefix(XPackSettings.GLOBAL_SSL_PREFIX)); @@ -403,10 +398,10 @@ private SSLContextHolder createSslContext(SSLConfiguration sslConfiguration) { if (logger.isDebugEnabled()) { logger.debug("using ssl settings [{}]", sslConfiguration); } - ReloadableTrustManager trustManager = - new ReloadableTrustManager(sslConfiguration.trustConfig().createTrustManager(env), sslConfiguration.trustConfig()); - ReloadableX509KeyManager keyManager = - new ReloadableX509KeyManager(sslConfiguration.keyConfig().createKeyManager(env), sslConfiguration.keyConfig()); + X509ExtendedTrustManager trustManager = + sslConfiguration.trustConfig().createTrustManager(env); + X509ExtendedKeyManager keyManager = + sslConfiguration.keyConfig().createKeyManager(env); return createSslContext(keyManager, trustManager, sslConfiguration); } @@ -417,7 +412,7 @@ private SSLContextHolder createSslContext(SSLConfiguration sslConfiguration) { * @param trustManager the trust manager to use * @return the created SSLContext */ - private SSLContextHolder createSslContext(ReloadableX509KeyManager keyManager, ReloadableTrustManager trustManager, + private SSLContextHolder createSslContext(X509ExtendedKeyManager keyManager, X509ExtendedTrustManager trustManager, SSLConfiguration sslConfiguration) { // Initialize sslContext try { @@ -427,7 +422,7 @@ private SSLContextHolder createSslContext(ReloadableX509KeyManager keyManager, R // check the supported ciphers and log them here to prevent spamming logs on every call supportedCiphers(sslContext.getSupportedSSLParameters().getCipherSuites(), sslConfiguration.cipherSuites(), true); - return new SSLContextHolder(sslContext, trustManager, keyManager); + return new SSLContextHolder(sslContext, sslConfiguration); } catch (NoSuchAlgorithmException | KeyManagementException e) { throw new ElasticsearchException("failed to initialize the SSLContext", e); } @@ -436,9 +431,7 @@ private SSLContextHolder createSslContext(ReloadableX509KeyManager keyManager, R /** * Parses the settings to load all SSLConfiguration objects that will be used. */ - Map loadSSLConfigurations() throws CertificateException, - UnrecoverableKeyException, NoSuchAlgorithmException, IOException, DestroyFailedException, KeyStoreException, - OperatorCreationException { + Map loadSSLConfigurations() { Map sslConfigurations = new HashMap<>(); sslConfigurations.put(globalSSLConfiguration, createSslContext(globalSSLConfiguration)); @@ -560,258 +553,70 @@ private static SSLSocket createWithPermissions(CheckedSupplier sessionIds = sslSessionContext.getIds(); while (sessionIds.hasMoreElements()) { byte[] sessionId = sessionIds.nextElement(); sslSessionContext.getSession(sessionId).invalidate(); } } - } - - /** - * This is an empty key manager that is used in case a loaded key manager is null - */ - private static final class EmptyKeyManager extends X509ExtendedKeyManager { - - @Override - public String[] getClientAliases(String s, Principal[] principals) { - return new String[0]; - } - - @Override - public String chooseClientAlias(String[] strings, Principal[] principals, Socket socket) { - return null; - } - - @Override - public String[] getServerAliases(String s, Principal[] principals) { - return new String[0]; - } - - @Override - public String chooseServerAlias(String s, Principal[] principals, Socket socket) { - return null; - } - - @Override - public X509Certificate[] getCertificateChain(String s) { - return new X509Certificate[0]; - } - - @Override - public PrivateKey getPrivateKey(String s) { - return null; - } - } - - /** - * This is an empty trust manager that is used in case a loaded trust manager is null - */ - static final class EmptyX509TrustManager extends X509ExtendedTrustManager { - - @Override - public void checkClientTrusted(X509Certificate[] x509Certificates, String s, Socket socket) throws CertificateException { - throw new CertificateException("no certificates are trusted"); - } - - @Override - public void checkServerTrusted(X509Certificate[] x509Certificates, String s, Socket socket) throws CertificateException { - throw new CertificateException("no certificates are trusted"); - } - - @Override - public void checkClientTrusted(X509Certificate[] x509Certificates, String s, SSLEngine sslEngine) throws CertificateException { - throw new CertificateException("no certificates are trusted"); - } - - @Override - public void checkServerTrusted(X509Certificate[] x509Certificates, String s, SSLEngine sslEngine) throws CertificateException { - throw new CertificateException("no certificates are trusted"); - } - @Override - public void checkClientTrusted(X509Certificate[] x509Certificates, String s) throws CertificateException { - throw new CertificateException("no certificates are trusted"); + synchronized void reload() { + invalidateSessions(context.getClientSessionContext()); + invalidateSessions(context.getServerSessionContext()); + reloadSslContext(); + } + + private void reloadSslContext() { + try { + X509ExtendedKeyManager loadedKeyManager = Optional.ofNullable(keyConfig.createKeyManager(env)). + orElse(getEmptyKeyManager()); + X509ExtendedTrustManager loadedTrustManager = Optional.ofNullable(trustConfig.createTrustManager(env)). + orElse(getEmptyTrustManager()); + SSLContext loadedSslContext = SSLContext.getInstance(sslContextAlgorithm(sslConfiguration.supportedProtocols())); + loadedSslContext.init(new X509ExtendedKeyManager[]{loadedKeyManager}, + new X509ExtendedTrustManager[]{loadedTrustManager}, null); + supportedCiphers(loadedSslContext.getSupportedSSLParameters().getCipherSuites(), sslConfiguration.cipherSuites(), true); + this.context = loadedSslContext; + } catch (GeneralSecurityException | IOException e) { + throw new ElasticsearchException("failed to initialize the SSLContext", e); + } } - - @Override - public void checkServerTrusted(X509Certificate[] x509Certificates, String s) throws CertificateException { - throw new CertificateException("no certificates are trusted"); + X509ExtendedKeyManager getEmptyKeyManager() throws GeneralSecurityException, IOException { + KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType()); + keyStore.load(null, null); + KeyManagerFactory keyManagerFactory = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm()); + keyManagerFactory.init(keyStore, null); + return (X509ExtendedKeyManager) keyManagerFactory.getKeyManagers()[0]; } - @Override - public X509Certificate[] getAcceptedIssuers() { - return new X509Certificate[0]; + X509ExtendedTrustManager getEmptyTrustManager() throws GeneralSecurityException, IOException { + KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType()); + keyStore.load(null, null); + TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance("X509"); + trustManagerFactory.init(keyStore); + return (X509ExtendedTrustManager) trustManagerFactory.getTrustManagers()[0]; } } @@ -870,7 +675,7 @@ private static List getMonitoringExporterSettings(Settings settings) { * JCA Standard Algorithm Name * Documentation for Java 8. */ - private static String sslContextAlgorithm(List supportedProtocols) { + static String sslContextAlgorithm(List supportedProtocols) { if (supportedProtocols.isEmpty()) { return "TLSv1.2"; } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLConfigurationReloaderTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLConfigurationReloaderTests.java index f19f13d38b74a..e09de659c98b3 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLConfigurationReloaderTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLConfigurationReloaderTests.java @@ -5,25 +5,33 @@ */ package org.elasticsearch.xpack.core.ssl; -import org.apache.lucene.util.SetOnce; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClients; +import org.apache.http.ssl.SSLContextBuilder; import org.bouncycastle.openssl.jcajce.JcaPEMWriter; import org.bouncycastle.openssl.jcajce.JcePEMEncryptorBuilder; +import org.elasticsearch.common.CheckedRunnable; import org.elasticsearch.common.settings.MockSecureSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.env.TestEnvironment; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.http.MockResponse; +import org.elasticsearch.test.http.MockWebServer; import org.elasticsearch.threadpool.TestThreadPool; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.watcher.ResourceWatcherService; import org.junit.After; import org.junit.Before; -import javax.net.ssl.X509ExtendedKeyManager; -import javax.net.ssl.X509ExtendedTrustManager; +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLHandshakeException; import javax.security.auth.x500.X500Principal; +import java.io.BufferedWriter; import java.io.IOException; +import java.io.InputStream; import java.io.OutputStream; import java.io.OutputStreamWriter; import java.nio.charset.StandardCharsets; @@ -32,20 +40,23 @@ import java.nio.file.Path; import java.nio.file.StandardCopyOption; import java.nio.file.StandardOpenOption; -import java.nio.file.attribute.BasicFileAttributes; +import java.security.AccessController; +import java.security.KeyManagementException; import java.security.KeyPair; import java.security.KeyStore; -import java.security.PrivateKey; +import java.security.KeyStoreException; +import java.security.NoSuchAlgorithmException; +import java.security.PrivilegedActionException; +import java.security.PrivilegedExceptionAction; +import java.security.UnrecoverableKeyException; import java.security.cert.Certificate; +import java.security.cert.CertificateException; import java.security.cert.X509Certificate; import java.util.concurrent.CountDownLatch; -import java.util.function.BiConsumer; +import java.util.function.Consumer; import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.sameInstance; -import static org.hamcrest.core.Is.is; /** * Unit tests for the reloading of SSL configuration @@ -71,8 +82,7 @@ public void cleanup() throws Exception { } /** - * Tests reloading a keystore. The contents of the keystore is used for both keystore and truststore material, so both key - * config and trust config is checked. + * Tests reloading a keystore that is used in the KeyManager of SSLContext */ public void testReloadingKeyStore() throws Exception { final Path tempDir = createTempDir(); @@ -86,127 +96,127 @@ public void testReloadingKeyStore() throws Exception { .setSecureSettings(secureSettings) .build(); final Environment env = randomBoolean() ? null : TestEnvironment.newEnvironment(settings); - - final BiConsumer keyManagerPreChecks = (keyManager, config) -> { - // key manager checks - String[] aliases = keyManager.getServerAliases("RSA", null); - assertNotNull(aliases); - assertThat(aliases.length, is(1)); - assertThat(aliases[0], is("testnode")); - }; - - final SetOnce trustedCount = new SetOnce<>(); - final BiConsumer trustManagerPreChecks = (trustManager, config) -> { - // trust manager checks - Certificate[] certificates = trustManager.getAcceptedIssuers(); - trustedCount.set(certificates.length); - }; - - final Runnable modifier = () -> { - try { - // modify it - KeyStore keyStore = KeyStore.getInstance("jks"); - keyStore.load(null, null); - final KeyPair keyPair = CertUtils.generateKeyPair(512); - X509Certificate cert = CertUtils.generateSignedCertificate(new X500Principal("CN=testReloadingKeyStore"), null, keyPair, + //Load HTTPClient only once. Client uses the same store as a truststore + try(CloseableHttpClient client = getSSLClient(keystorePath, "testnode")) { + final Consumer keyMaterialPreChecks = (context) -> { + try (MockWebServer server = new MockWebServer(context, true)) { + server.enqueue(new MockResponse().setResponseCode(200).setBody("body")); + server.start(); + privilegedConnect(() -> client.execute(new HttpGet("https://localhost:" + server.getPort())).close()); + } catch (Exception e) { + throw new RuntimeException("Exception starting or connecting to the mock server", e); + } + }; + + final Runnable modifier = () -> { + try { + // modify the keystore that the KeyManager uses + KeyStore keyStore = KeyStore.getInstance("jks"); + keyStore.load(null, null); + final KeyPair keyPair = CertUtils.generateKeyPair(512); + X509Certificate cert = CertUtils.generateSignedCertificate(new X500Principal("CN=localhost"), null, keyPair, null, null, 365); - keyStore.setKeyEntry("key", keyPair.getPrivate(), "testnode".toCharArray(), new X509Certificate[] { cert }); - Path updated = tempDir.resolve("updated.jks"); - try (OutputStream out = Files.newOutputStream(updated)) { - keyStore.store(out, "testnode".toCharArray()); + keyStore.setKeyEntry("key", keyPair.getPrivate(), "testnode".toCharArray(), new X509Certificate[]{cert}); + Path updated = tempDir.resolve("updated.jks"); + try (OutputStream out = Files.newOutputStream(updated)) { + keyStore.store(out, "testnode".toCharArray()); + } + atomicMoveIfPossible(updated, keystorePath); + } catch (Exception e) { + throw new RuntimeException("modification failed", e); } - atomicMoveIfPossible(updated, keystorePath); - } catch (Exception e) { - throw new RuntimeException("modification failed", e); - } - }; - - final BiConsumer keyManagerPostChecks = (updatedKeyManager, config) -> { - String[] aliases = updatedKeyManager.getServerAliases("RSA", null); - assertNotNull(aliases); - assertThat(aliases.length, is(1)); - assertThat(aliases[0], is("key")); - }; - final BiConsumer trustManagerPostChecks = (updatedTrustManager, config) -> { - assertThat(trustedCount.get() - updatedTrustManager.getAcceptedIssuers().length, is(5)); - }; - validateSSLConfigurationIsReloaded(settings, env, keyManagerPreChecks, trustManagerPreChecks, modifier, keyManagerPostChecks, - trustManagerPostChecks); + }; + // The new server certificate is not in the client's truststore so SSLHandshake should fail + final Consumer keyMaterialPostChecks = (updatedContext) -> { + try (MockWebServer server = new MockWebServer(updatedContext, true)) { + server.enqueue(new MockResponse().setResponseCode(200).setBody("body")); + server.start(); + SSLHandshakeException sslException = expectThrows(SSLHandshakeException.class, () -> + privilegedConnect(() -> client.execute(new HttpGet("https://localhost:" + server.getPort())).close())); + assertThat(sslException.getCause().getMessage(), containsString("PKIX path building failed")); + } catch (Exception e) { + throw new RuntimeException("Exception starting or connecting to the mock server", e); + } + }; + validateSSLConfigurationIsReloaded(settings, env, keyMaterialPreChecks, modifier, keyMaterialPostChecks); + } } /** - * Tests the reloading of a PEM key config when the key is overwritten. The trust portion is not tested as it is not modified by this - * test. + * Tests the reloading of SSLContext when a PEM key and certificate are used. */ - public void testPEMKeyConfigReloading() throws Exception { - Path tempDir = createTempDir(); - Path keyPath = tempDir.resolve("testnode.pem"); - Path certPath = tempDir.resolve("testnode.crt"); - Path clientCertPath = tempDir.resolve("testclient.crt"); + public void testPEMKeyCertConfigReloading() throws Exception { + final Path tempDir = createTempDir(); + final Path keyPath = tempDir.resolve("testnode.pem"); + final Path certPath = tempDir.resolve("testnode.crt"); + final Path clientTruststorePath = tempDir.resolve("testnode.jks"); + Files.copy(getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.jks"), clientTruststorePath); Files.copy(getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.pem"), keyPath); Files.copy(getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.crt"), certPath); - Files.copy(getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testclient.crt"), clientCertPath); MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("xpack.ssl.secure_key_passphrase", "testnode"); final Settings settings = Settings.builder() - .put("path.home", createTempDir()) - .put("xpack.ssl.key", keyPath) - .put("xpack.ssl.certificate", certPath) - .putList("xpack.ssl.certificate_authorities", certPath.toString(), clientCertPath.toString()) - .setSecureSettings(secureSettings) - .build(); + .put("path.home", createTempDir()) + .put("xpack.ssl.key", keyPath) + .put("xpack.ssl.certificate", certPath) + .setSecureSettings(secureSettings) + .build(); final Environment env = randomBoolean() ? null : - TestEnvironment.newEnvironment(Settings.builder().put("path.home", createTempDir()).build()); - - final SetOnce privateKey = new SetOnce<>(); - final BiConsumer keyManagerPreChecks = (keyManager, config) -> { - String[] aliases = keyManager.getServerAliases("RSA", null); - assertNotNull(aliases); - assertThat(aliases.length, is(1)); - assertThat(aliases[0], is("key")); - privateKey.set(keyManager.getPrivateKey("key")); - assertNotNull(privateKey.get()); - }; - - final KeyPair keyPair = CertUtils.generateKeyPair(randomFrom(1024, 2048)); - final Runnable modifier = () -> { - try { - // make sure we wait long enough to see a change. if time is within a second the file may not be seen as modified since the - // size is the same! - assertTrue(awaitBusy(() -> { - try { - BasicFileAttributes attributes = Files.readAttributes(keyPath, BasicFileAttributes.class); - return System.currentTimeMillis() - attributes.lastModifiedTime().toMillis() >= 1000L; - } catch (IOException e) { - throw new RuntimeException("io exception while checking time", e); + TestEnvironment.newEnvironment(Settings.builder().put("path.home", createTempDir()).build()); + // Load HTTPClient once. Client uses a keystore containing testnode key/cert as a truststore + try (CloseableHttpClient client = getSSLClient(clientTruststorePath, "testnode")) { + final Consumer keyMaterialPreChecks = (context) -> { + try (MockWebServer server = new MockWebServer(context, false)) { + server.enqueue(new MockResponse().setResponseCode(200).setBody("body")); + server.start(); + privilegedConnect(() -> client.execute(new HttpGet("https://localhost:" + server.getPort())).close()); + } catch (Exception e) { + throw new RuntimeException("Exception starting or connecting to the mock server", e); + } + }; + final Runnable modifier = () -> { + try { + final KeyPair keyPair = CertUtils.generateKeyPair(512); + X509Certificate cert = CertUtils.generateSignedCertificate(new X500Principal("CN=localhost"), null, keyPair, + null, null, 365); + Path updatedKeyPath = tempDir.resolve("updated.pem"); + Path updatedCertPath = tempDir.resolve("updated.crt"); + try (OutputStream os = Files.newOutputStream(updatedKeyPath); + OutputStreamWriter osWriter = new OutputStreamWriter(os, StandardCharsets.UTF_8); + JcaPEMWriter writer = new JcaPEMWriter(osWriter)) { + writer.writeObject(keyPair, + new JcePEMEncryptorBuilder("DES-EDE3-CBC").setProvider(CertUtils.BC_PROV).build("testnode".toCharArray())); + } + try (BufferedWriter out = Files.newBufferedWriter(updatedCertPath); + JcaPEMWriter pemWriter = new JcaPEMWriter(out)) { + pemWriter.writeObject(cert); } - })); - Path updatedKeyPath = tempDir.resolve("updated.pem"); - try (OutputStream os = Files.newOutputStream(updatedKeyPath); - OutputStreamWriter osWriter = new OutputStreamWriter(os, StandardCharsets.UTF_8); - JcaPEMWriter writer = new JcaPEMWriter(osWriter)) { - writer.writeObject(keyPair, - new JcePEMEncryptorBuilder("DES-EDE3-CBC").setProvider(CertUtils.BC_PROV).build("testnode".toCharArray())); + atomicMoveIfPossible(updatedKeyPath, keyPath); + atomicMoveIfPossible(updatedCertPath, certPath); + } catch (Exception e) { + throw new RuntimeException("failed to modify file", e); } - atomicMoveIfPossible(updatedKeyPath, keyPath); - } catch (Exception e) { - throw new RuntimeException("failed to modify file", e); - } - }; + }; + // The new server certificate is not in the client's truststore so SSLHandshake should fail + final Consumer keyMaterialPostChecks = (updatedContext) -> { + try (MockWebServer server = new MockWebServer(updatedContext, false)) { + server.enqueue(new MockResponse().setResponseCode(200).setBody("body")); + server.start(); + SSLHandshakeException sslException = expectThrows(SSLHandshakeException.class, () -> + privilegedConnect(() -> client.execute(new HttpGet("https://localhost:" + server.getPort())).close())); + assertThat(sslException.getCause().getMessage(), containsString("PKIX path building failed")); + } catch (Exception e) { + throw new RuntimeException("Exception starting or connecting to the mock server", e); + } + }; - final BiConsumer keyManagerPostChecks = (keyManager, config) -> { - String[] aliases = keyManager.getServerAliases("RSA", null); - assertNotNull(aliases); - assertThat(aliases.length, is(1)); - assertThat(aliases[0], is("key")); - assertThat(keyManager.getPrivateKey(aliases[0]), not(equalTo(privateKey))); - assertThat(keyManager.getPrivateKey(aliases[0]), is(equalTo(keyPair.getPrivate()))); - }; - validateKeyConfigurationIsReloaded(settings, env, keyManagerPreChecks, modifier, keyManagerPostChecks); + validateSSLConfigurationIsReloaded(settings, env, keyMaterialPreChecks, modifier, keyMaterialPostChecks); + } } /** - * Tests the reloading of the trust config when the trust store is modified. The key config is not tested as part of this test. + * Tests the reloading of SSLContext when the trust store is modified. The same store is used as a TrustStore (for the + * reloadable SSLContext used in the HTTPClient) and as a KeyStore for the MockWebServer */ public void testReloadingTrustStore() throws Exception { Path tempDir = createTempDir(); @@ -215,80 +225,106 @@ public void testReloadingTrustStore() throws Exception { MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("xpack.ssl.truststore.secure_password", "testnode"); Settings settings = Settings.builder() - .put("xpack.ssl.truststore.path", trustStorePath) - .put("path.home", createTempDir()) - .setSecureSettings(secureSettings) - .build(); + .put("xpack.ssl.truststore.path", trustStorePath) + .put("path.home", createTempDir()) + .setSecureSettings(secureSettings) + .build(); Environment env = randomBoolean() ? null : TestEnvironment.newEnvironment(settings); - - final SetOnce trustedCount = new SetOnce<>(); - final BiConsumer trustManagerPreChecks = (trustManager, config) -> { - // trust manager checks - Certificate[] certificates = trustManager.getAcceptedIssuers(); - trustedCount.set(certificates.length); - }; - - - final Runnable modifier = () -> { - try { - Path updatedTruststore = tempDir.resolve("updated.jks"); - KeyStore keyStore = KeyStore.getInstance("jks"); - keyStore.load(null, null); - try (OutputStream out = Files.newOutputStream(updatedTruststore)) { - keyStore.store(out, "testnode".toCharArray()); + // Create the MockWebServer once for both pre and post checks + try(MockWebServer server = getSslServer(trustStorePath, "testnode")){ + final Consumer trustMaterialPreChecks = (context) -> { + try (CloseableHttpClient client = HttpClients.custom().setSSLContext(context).build()){ + privilegedConnect(() -> client.execute(new HttpGet("https://localhost:" + server.getPort())).close()); + } catch (Exception e) { + throw new RuntimeException("Error connecting to the mock server", e); } - atomicMoveIfPossible(updatedTruststore, trustStorePath); - } catch (Exception e) { - throw new RuntimeException("failed to modify file", e); - } - }; - - final BiConsumer trustManagerPostChecks = (updatedTrustManager, config) -> { - assertThat(trustedCount.get() - updatedTrustManager.getAcceptedIssuers().length, is(6)); - }; + }; + + final Runnable modifier = () -> { + try { + Path updatedTrustStore = tempDir.resolve("updated.jks"); + KeyStore keyStore = KeyStore.getInstance("jks"); + keyStore.load(null, null); + final KeyPair keyPair = CertUtils.generateKeyPair(512); + X509Certificate cert = CertUtils.generateSignedCertificate(new X500Principal("CN=localhost"), null, keyPair, + null, null, 365); + keyStore.setKeyEntry("newKey", keyPair.getPrivate(), "testnode".toCharArray(), new Certificate[]{cert}); + try (OutputStream out = Files.newOutputStream(updatedTrustStore)) { + keyStore.store(out, "testnode".toCharArray()); + } + atomicMoveIfPossible(updatedTrustStore, trustStorePath); + } catch (Exception e) { + throw new RuntimeException("failed to modify file", e); + } + }; + + // Client's truststore doesn't contain the server's certificate anymore so SSLHandshake should fail + final Consumer trustMaterialPostChecks = (updatedContext) -> { + try (CloseableHttpClient client = HttpClients.custom().setSSLContext(updatedContext).build()){ + SSLHandshakeException sslException = expectThrows(SSLHandshakeException.class, () -> + privilegedConnect(() -> client.execute(new HttpGet("https://localhost:" + server.getPort())).close())); + assertThat(sslException.getCause().getMessage(), containsString("PKIX path building failed")); + } catch (Exception e) { + throw new RuntimeException("Error closing CloseableHttpClient", e); + } + }; - validateTrustConfigurationIsReloaded(settings, env, trustManagerPreChecks, modifier, trustManagerPostChecks); + validateSSLConfigurationIsReloaded(settings, env, trustMaterialPreChecks, modifier, trustMaterialPostChecks); + } } /** - * Test the reloading of a trust config that is backed by PEM certificate files. The key config is not tested as we only care about the - * trust config in this test. + * Test the reloading of SSLContext whose trust config is backed by PEM certificate files. */ public void testReloadingPEMTrustConfig() throws Exception { Path tempDir = createTempDir(); - Path clientCertPath = tempDir.resolve("testclient.crt"); - Files.copy(getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testclient.crt"), clientCertPath); + Path clientCertPath = tempDir.resolve("testnode.crt"); + Path keyStorePath = tempDir.resolve("testnode.jks"); + Files.copy(getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.jks"), keyStorePath); + Files.copy(getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.crt"), clientCertPath); Settings settings = Settings.builder() .putList("xpack.ssl.certificate_authorities", clientCertPath.toString()) .put("path.home", createTempDir()) .build(); Environment env = randomBoolean() ? null : TestEnvironment.newEnvironment(settings); + // Create the MockWebServer once for both pre and post checks + try(MockWebServer server = getSslServer(keyStorePath, "testnode")){ + final Consumer trustMaterialPreChecks = (context) -> { + try (CloseableHttpClient client = HttpClients.custom().setSSLContext(context).build()){ + privilegedConnect(() -> client.execute(new HttpGet("https://localhost:" + server.getPort())).close()); + } catch (Exception e) { + throw new RuntimeException("Exception connecting to the mock server", e); + } + }; - final BiConsumer trustManagerPreChecks = (trustManager, config) -> { - // trust manager checks - Certificate[] certificates = trustManager.getAcceptedIssuers(); - assertThat(certificates.length, is(1)); - assertThat(((X509Certificate)certificates[0]).getSubjectX500Principal().getName(), containsString("Test Client")); - }; - - final Runnable modifier = () -> { - try { - Path updatedCert = tempDir.resolve("updated.crt"); - Files.copy(getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.crt"), updatedCert, - StandardCopyOption.REPLACE_EXISTING); - atomicMoveIfPossible(updatedCert, clientCertPath); - } catch (Exception e) { - throw new RuntimeException("failed to modify file", e); - } - }; - - final BiConsumer trustManagerPostChecks = (updatedTrustManager, config) -> { - Certificate[] updatedCerts = updatedTrustManager.getAcceptedIssuers(); - assertThat(updatedCerts.length, is(1)); - assertThat(((X509Certificate)updatedCerts[0]).getSubjectX500Principal().getName(), containsString("Test Node")); - }; + final Runnable modifier = () -> { + try { + final KeyPair keyPair = CertUtils.generateKeyPair(512); + X509Certificate cert = CertUtils.generateSignedCertificate(new X500Principal("CN=localhost"), null, keyPair, + null, null, 365); + Path updatedCertPath = tempDir.resolve("updated.crt"); + try (BufferedWriter out = Files.newBufferedWriter(updatedCertPath); + JcaPEMWriter pemWriter = new JcaPEMWriter(out)) { + pemWriter.writeObject(cert); + } + atomicMoveIfPossible(updatedCertPath, clientCertPath); + } catch (Exception e) { + throw new RuntimeException("failed to modify file", e); + } + }; + // Client doesn't trust the Server certificate anymore so SSLHandshake should fail + final Consumer trustMaterialPostChecks = (updatedContext) -> { + try (CloseableHttpClient client = HttpClients.custom().setSSLContext(updatedContext).build()){ + SSLHandshakeException sslException = expectThrows(SSLHandshakeException.class, () -> + privilegedConnect(() -> client.execute(new HttpGet("https://localhost:" + server.getPort())).close())); + assertThat(sslException.getCause().getMessage(), containsString("PKIX path building failed")); + } catch (Exception e) { + throw new RuntimeException("Error closing CloseableHttpClient", e); + } + }; - validateTrustConfigurationIsReloaded(settings, env, trustManagerPreChecks, modifier, trustManagerPostChecks); + validateSSLConfigurationIsReloaded(settings, env, trustMaterialPreChecks, modifier, trustMaterialPostChecks); + } } /** @@ -316,15 +352,14 @@ void reloadSSLContext(SSLConfiguration configuration) { } }; - // key manager checks - final X509ExtendedKeyManager keyManager = sslService.sslContextHolder(config).keyManager().getKeyManager(); + final SSLContext context = sslService.sslContextHolder(config).sslContext(); // truncate the keystore try (OutputStream out = Files.newOutputStream(keystorePath, StandardOpenOption.TRUNCATE_EXISTING)) { } // we intentionally don't wait here as we rely on concurrency to catch a failure - assertThat(sslService.sslContextHolder(config).keyManager().getKeyManager(), sameInstance(keyManager)); + assertThat(sslService.sslContextHolder(config).sslContext(), sameInstance(context)); } /** @@ -358,14 +393,14 @@ void reloadSSLContext(SSLConfiguration configuration) { } }; - final X509ExtendedKeyManager keyManager = sslService.sslContextHolder(config).keyManager().getKeyManager(); + final SSLContext context = sslService.sslContextHolder(config).sslContext(); // truncate the file try (OutputStream os = Files.newOutputStream(keyPath, StandardOpenOption.TRUNCATE_EXISTING)) { } // we intentionally don't wait here as we rely on concurrency to catch a failure - assertThat(sslService.sslContextHolder(config).keyManager().getKeyManager(), sameInstance(keyManager)); + assertThat(sslService.sslContextHolder(config).sslContext(), sameInstance(context)); } /** @@ -393,14 +428,14 @@ void reloadSSLContext(SSLConfiguration configuration) { } }; - final X509ExtendedTrustManager trustManager = sslService.sslContextHolder(config).trustManager().getTrustManager(); + final SSLContext context = sslService.sslContextHolder(config).sslContext(); // truncate the truststore try (OutputStream os = Files.newOutputStream(trustStorePath, StandardOpenOption.TRUNCATE_EXISTING)) { } // we intentionally don't wait here as we rely on concurrency to catch a failure - assertThat(sslService.sslContextHolder(config).trustManager().getTrustManager(), sameInstance(trustManager)); + assertThat(sslService.sslContextHolder(config).sslContext(), sameInstance(context)); } /** @@ -425,7 +460,7 @@ void reloadSSLContext(SSLConfiguration configuration) { } }; - final X509ExtendedTrustManager trustManager = sslService.sslContextHolder(config).trustManager().getTrustManager(); + final SSLContext context = sslService.sslContextHolder(config).sslContext(); // write bad file Path updatedCert = tempDir.resolve("updated.crt"); @@ -435,53 +470,13 @@ void reloadSSLContext(SSLConfiguration configuration) { atomicMoveIfPossible(updatedCert, clientCertPath); // we intentionally don't wait here as we rely on concurrency to catch a failure - assertThat(sslService.sslContextHolder(config).trustManager().getTrustManager(), sameInstance(trustManager)); + assertThat(sslService.sslContextHolder(config).sslContext(), sameInstance(context)); } - /** - * Validates the trust configuration aspect of the SSLConfiguration is reloaded - */ - private void validateTrustConfigurationIsReloaded(Settings settings, Environment env, - BiConsumer trustManagerPreChecks, - Runnable modificationFunction, - BiConsumer trustManagerPostChecks) - throws Exception { - validateSSLConfigurationIsReloaded(settings, env, false, true, null, trustManagerPreChecks, modificationFunction, null, - trustManagerPostChecks); - } - - /** - * Validates the trust configuration aspect of the SSLConfiguration is reloaded - */ - private void validateKeyConfigurationIsReloaded(Settings settings, Environment env, - BiConsumer keyManagerPreChecks, - Runnable modificationFunction, - BiConsumer keyManagerPostChecks) - throws Exception { - validateSSLConfigurationIsReloaded(settings, env, true, false, keyManagerPreChecks, null, modificationFunction, - keyManagerPostChecks, null); - } - - /** - * Validates that both the key and trust configuration aspects of the SSLConfiguration are reloaded - */ private void validateSSLConfigurationIsReloaded(Settings settings, Environment env, - BiConsumer keyManagerPreChecks, - BiConsumer trustManagerPreChecks, + Consumer preChecks, Runnable modificationFunction, - BiConsumer keyManagerPostChecks, - BiConsumer trustManagerPostChecks) - throws Exception { - validateSSLConfigurationIsReloaded(settings, env, true, true, keyManagerPreChecks, trustManagerPreChecks, modificationFunction, - keyManagerPostChecks, trustManagerPostChecks); - } - - private void validateSSLConfigurationIsReloaded(Settings settings, Environment env, boolean checkKeys, boolean checkTrust, - BiConsumer keyManagerPreChecks, - BiConsumer trustManagerPreChecks, - Runnable modificationFunction, - BiConsumer keyManagerPostChecks, - BiConsumer trustManagerPostChecks) + Consumer postChecks) throws Exception { final CountDownLatch reloadLatch = new CountDownLatch(1); @@ -494,50 +489,16 @@ void reloadSSLContext(SSLConfiguration configuration) { reloadLatch.countDown(); } }; - - final X509ExtendedKeyManager keyManager; - if (checkKeys) { - keyManager = sslService.sslContextHolder(config).keyManager().getKeyManager(); - } else { - keyManager = null; - } - - final X509ExtendedTrustManager trustManager; - if (checkTrust) { - trustManager = sslService.sslContextHolder(config).trustManager().getTrustManager(); - } else { - trustManager = null; - } - - // key manager checks - if (checkKeys) { - keyManagerPreChecks.accept(keyManager, config); - } - - // trust manager checks - if (checkTrust) { - trustManagerPreChecks.accept(trustManager, config); - } + // Baseline checks + preChecks.accept(sslService.sslContextHolder(config).sslContext()); assertEquals("nothing should have called reload", 1, reloadLatch.getCount()); // modify modificationFunction.run(); reloadLatch.await(); - - // check key manager - if (checkKeys) { - final X509ExtendedKeyManager updatedKeyManager = sslService.sslContextHolder(config).keyManager().getKeyManager(); - assertThat(updatedKeyManager, not(sameInstance(keyManager))); - keyManagerPostChecks.accept(updatedKeyManager, config); - } - - // check trust manager - if (checkTrust) { - final X509ExtendedTrustManager updatedTrustManager = sslService.sslContextHolder(config).trustManager().getTrustManager(); - assertThat(updatedTrustManager, not(sameInstance(trustManager))); - trustManagerPostChecks.accept(updatedTrustManager, config); - } + // checks after reload + postChecks.accept(sslService.sslContextHolder(config).sslContext()); } private static void atomicMoveIfPossible(Path source, Path target) throws IOException { @@ -547,4 +508,41 @@ private static void atomicMoveIfPossible(Path source, Path target) throws IOExce Files.move(source, target, StandardCopyOption.REPLACE_EXISTING); } } + + private static MockWebServer getSslServer(Path keyStorePath, String keyStorePass) throws KeyStoreException, CertificateException, + NoSuchAlgorithmException, IOException, KeyManagementException, UnrecoverableKeyException { + KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType()); + try(InputStream is = Files.newInputStream(keyStorePath)) { + keyStore.load(is, keyStorePass.toCharArray()); + } + final SSLContext sslContext = new SSLContextBuilder().loadKeyMaterial(keyStore, keyStorePass.toCharArray()) + .build(); + MockWebServer server = new MockWebServer(sslContext, false); + server.enqueue(new MockResponse().setResponseCode(200).setBody("body")); + server.start(); + return server; + } + + private static CloseableHttpClient getSSLClient(Path trustStorePath, String trustStorePass) throws KeyStoreException, + NoSuchAlgorithmException, + KeyManagementException, IOException, CertificateException { + KeyStore trustStore = KeyStore.getInstance(KeyStore.getDefaultType()); + try(InputStream is = Files.newInputStream(trustStorePath)) { + trustStore.load(is, trustStorePass.toCharArray()); + } + final SSLContext sslContext = new SSLContextBuilder().loadTrustMaterial(trustStore, null).build(); + return HttpClients.custom().setSSLContext(sslContext).build(); + } + + private static void privilegedConnect(CheckedRunnable runnable) throws Exception { + try { + AccessController.doPrivileged((PrivilegedExceptionAction) () -> { + runnable.run(); + return null; + }); + } catch (PrivilegedActionException e) { + throw (Exception) e.getCause(); + } + } + } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLServiceTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLServiceTests.java index 598a0f8a77ada..bcb4b63865432 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLServiceTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLServiceTests.java @@ -32,16 +32,20 @@ import org.mockito.ArgumentCaptor; import javax.net.ssl.HostnameVerifier; +import javax.net.ssl.KeyManagerFactory; import javax.net.ssl.SSLContext; import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLParameters; import javax.net.ssl.SSLSocket; import javax.net.ssl.SSLSocketFactory; +import javax.net.ssl.TrustManager; +import javax.net.ssl.TrustManagerFactory; import javax.net.ssl.X509ExtendedTrustManager; import java.net.Socket; import java.nio.file.Path; import java.security.AccessController; +import java.security.KeyStore; import java.security.PrivilegedActionException; import java.security.PrivilegedExceptionAction; import java.security.cert.CertificateException; @@ -446,22 +450,11 @@ public void testSSLStrategy() { } public void testEmptyTrustManager() throws Exception { - X509ExtendedTrustManager trustManager = new SSLService.EmptyX509TrustManager(); + Settings settings = Settings.builder().build(); + final SSLService sslService = new SSLService(settings, env); + SSLConfiguration sslConfig = new SSLConfiguration(settings); + X509ExtendedTrustManager trustManager = sslService.sslContextHolder(sslConfig).getEmptyTrustManager(); assertThat(trustManager.getAcceptedIssuers(), emptyArray()); - final String message = "no certificates are trusted"; - CertificateException ce = - expectThrows(CertificateException.class, () -> trustManager.checkClientTrusted(null, null, (Socket) null)); - assertEquals(message, ce.getMessage()); - ce = expectThrows(CertificateException.class, () -> trustManager.checkClientTrusted(null, null, (SSLEngine) null)); - assertEquals(message, ce.getMessage()); - ce = expectThrows(CertificateException.class, () -> trustManager.checkClientTrusted(null, null)); - assertEquals(message, ce.getMessage()); - ce = expectThrows(CertificateException.class, () -> trustManager.checkServerTrusted(null, null, (Socket) null)); - assertEquals(message, ce.getMessage()); - ce = expectThrows(CertificateException.class, () -> trustManager.checkServerTrusted(null, null, (SSLEngine) null)); - assertEquals(message, ce.getMessage()); - ce = expectThrows(CertificateException.class, () -> trustManager.checkServerTrusted(null, null)); - assertEquals(message, ce.getMessage()); } public void testReadCertificateInformation() throws Exception { From 9234de300f23d0a7ea89494c5ba77f2c1b1ac0f9 Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Thu, 10 May 2018 11:17:31 +0300 Subject: [PATCH 2/3] Reduce visibility, leftover from test --- .../main/java/org/elasticsearch/xpack/core/ssl/SSLService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java index 8cf7cceeffdb6..0ebbe9a71b850 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java @@ -675,7 +675,7 @@ private static List getMonitoringExporterSettings(Settings settings) { * JCA Standard Algorithm Name * Documentation for Java 8. */ - static String sslContextAlgorithm(List supportedProtocols) { + private static String sslContextAlgorithm(List supportedProtocols) { if (supportedProtocols.isEmpty()) { return "TLSv1.2"; } From 01fc7402fe06b5e00c1b5782c3a5efc46dbf6a16 Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Tue, 15 May 2018 08:14:42 +0300 Subject: [PATCH 3/3] Address feedback --- .../java/org/elasticsearch/xpack/core/ssl/SSLService.java | 8 +++----- .../xpack/core/ssl/SSLConfigurationReloaderTests.java | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java index 0ebbe9a71b850..e5150e3faadba 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java @@ -398,10 +398,8 @@ private SSLContextHolder createSslContext(SSLConfiguration sslConfiguration) { if (logger.isDebugEnabled()) { logger.debug("using ssl settings [{}]", sslConfiguration); } - X509ExtendedTrustManager trustManager = - sslConfiguration.trustConfig().createTrustManager(env); - X509ExtendedKeyManager keyManager = - sslConfiguration.keyConfig().createKeyManager(env); + X509ExtendedTrustManager trustManager = sslConfiguration.trustConfig().createTrustManager(env); + X509ExtendedKeyManager keyManager = sslConfiguration.keyConfig().createKeyManager(env); return createSslContext(keyManager, trustManager, sslConfiguration); } @@ -597,7 +595,7 @@ private void reloadSslContext() { SSLContext loadedSslContext = SSLContext.getInstance(sslContextAlgorithm(sslConfiguration.supportedProtocols())); loadedSslContext.init(new X509ExtendedKeyManager[]{loadedKeyManager}, new X509ExtendedTrustManager[]{loadedTrustManager}, null); - supportedCiphers(loadedSslContext.getSupportedSSLParameters().getCipherSuites(), sslConfiguration.cipherSuites(), true); + supportedCiphers(loadedSslContext.getSupportedSSLParameters().getCipherSuites(), sslConfiguration.cipherSuites(), false); this.context = loadedSslContext; } catch (GeneralSecurityException | IOException e) { throw new ElasticsearchException("failed to initialize the SSLContext", e); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLConfigurationReloaderTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLConfigurationReloaderTests.java index e09de659c98b3..2ccbd549105d9 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLConfigurationReloaderTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLConfigurationReloaderTests.java @@ -97,7 +97,7 @@ public void testReloadingKeyStore() throws Exception { .build(); final Environment env = randomBoolean() ? null : TestEnvironment.newEnvironment(settings); //Load HTTPClient only once. Client uses the same store as a truststore - try(CloseableHttpClient client = getSSLClient(keystorePath, "testnode")) { + try (CloseableHttpClient client = getSSLClient(keystorePath, "testnode")) { final Consumer keyMaterialPreChecks = (context) -> { try (MockWebServer server = new MockWebServer(context, true)) { server.enqueue(new MockResponse().setResponseCode(200).setBody("body"));