You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by ra...@apache.org on 2026/09/21 10:23:21 UTC
(logging-log4j2) 01/01: Fail TLS connections when an Ssl key store or trust store cannot be loaded
This is an automated email from the ASF dual-hosted git repository.
ramanathan1504 pushed a commit to branch fix/2.x/ssl-fail-closed
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit 20397d866ca8e40242038baa3eebe79695e78c66
Author: Ramanathan <ra...@gmail.com>
AuthorDate: Mon Sep 21 15:53:03 2026 +0530
Fail TLS connections when an Ssl key store or trust store cannot be loaded
---
.../core/net/ssl/KeyStoreConfigurationTest.java | 14 ++
.../core/net/ssl/SslConfigurationFactoryTest.java | 29 ++++
.../log4j/core/net/ssl/SslConfigurationTest.java | 162 +++++++++++++++++++++
.../core/net/ssl/TrustStoreConfigurationTest.java | 26 ++++
.../net/ssl/AbstractKeyStoreConfiguration.java | 26 ++++
.../log4j/core/net/ssl/KeyStoreConfiguration.java | 35 ++++-
.../log4j/core/net/ssl/SslConfiguration.java | 78 +++++++---
.../core/net/ssl/SslConfigurationFactory.java | 46 ++----
.../core/net/ssl/TrustStoreConfiguration.java | 41 +++++-
.../4332_fail_closed_on_invalid_ssl_store.xml | 13 ++
10 files changed, 414 insertions(+), 56 deletions(-)
diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/net/ssl/KeyStoreConfigurationTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/net/ssl/KeyStoreConfigurationTest.java
index 1889d5b2aa..c24dd9f6ca 100644
--- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/net/ssl/KeyStoreConfigurationTest.java
+++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/net/ssl/KeyStoreConfigurationTest.java
@@ -16,6 +16,7 @@
*/
package org.apache.logging.log4j.core.net.ssl;
+import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertSame;
@@ -183,4 +184,17 @@ class KeyStoreConfigurationTest {
}
});
}
+
+ @Test
+ void pluginFactoryRecordsLoadFailureWithEmptyKeyStore() throws Exception {
+ final KeyStoreConfiguration config = KeyStoreConfiguration.createOrRecordFailure(
+ SslKeyStoreConstants.KEYSTORE_LOCATION,
+ "wrongPassword!".toCharArray(),
+ null,
+ null,
+ SslKeyStoreConstants.KEYSTORE_TYPE,
+ null);
+ assertThat(config.getLoadFailure()).isNotNull();
+ assertThat(config.getKeyStore().size()).isZero();
+ }
}
diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/net/ssl/SslConfigurationFactoryTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/net/ssl/SslConfigurationFactoryTest.java
index 308f7546ef..94a9f12fa3 100644
--- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/net/ssl/SslConfigurationFactoryTest.java
+++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/net/ssl/SslConfigurationFactoryTest.java
@@ -16,11 +16,13 @@
*/
package org.apache.logging.log4j.core.net.ssl;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import java.util.Properties;
import java.util.stream.Stream;
+import javax.net.ssl.SSLHandshakeException;
import org.apache.logging.log4j.test.junit.UsingStatusListener;
import org.apache.logging.log4j.util.PropertiesUtil;
import org.junit.jupiter.api.Test;
@@ -131,4 +133,31 @@ class SslConfigurationFactoryTest {
assertNotNull(trustStoreConfig);
KeyStoreConfigurationTest.checkKeystoreConfiguration(trustStoreConfig);
}
+
+ @Test
+ void trustStoreThatFailsToLoadRejectsHandshake() {
+ final Properties props = new Properties();
+ props.setProperty(TRUSTSTORE_LOCATION_PROP_NAME, SslKeyStoreConstants.TRUSTSTORE_LOCATION + ".missing");
+ props.setProperty(TRUSTSTORE_TYPE_PROP_NAME, SslKeyStoreConstants.TRUSTSTORE_TYPE);
+ final SslConfiguration sslConfiguration =
+ SslConfigurationFactory.createSslConfiguration(new PropertiesUtil(props));
+ assertNotNull(sslConfiguration);
+ assertNotNull(sslConfiguration.getTrustStoreConfig());
+ assertThatThrownBy(() -> SslConfigurationTest.handshake(sslConfiguration))
+ .isInstanceOf(SSLHandshakeException.class);
+ }
+
+ @Test
+ void keyStoreThatFailsToLoadRejectsHandshake() {
+ final Properties props = new Properties();
+ addTruststoreConfiguration(props);
+ addKeystoreConfiguration(props);
+ props.setProperty(KEYSTORE_PASSWORD_PROP_NAME, "wrongPassword!");
+ final SslConfiguration sslConfiguration =
+ SslConfigurationFactory.createSslConfiguration(new PropertiesUtil(props));
+ assertNotNull(sslConfiguration);
+ assertNotNull(sslConfiguration.getKeyStoreConfig());
+ assertThatThrownBy(() -> SslConfigurationTest.handshake(sslConfiguration))
+ .isInstanceOf(SSLHandshakeException.class);
+ }
}
diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/net/ssl/SslConfigurationTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/net/ssl/SslConfigurationTest.java
index b916f4410e..c303125067 100644
--- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/net/ssl/SslConfigurationTest.java
+++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/net/ssl/SslConfigurationTest.java
@@ -17,15 +17,28 @@
package org.apache.logging.log4j.core.net.ssl;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatCode;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import java.io.IOException;
import java.io.OutputStream;
+import java.net.InetAddress;
import java.net.UnknownHostException;
+import java.security.cert.CertificateException;
+import java.security.cert.X509Certificate;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLHandshakeException;
+import javax.net.ssl.SSLServerSocket;
import javax.net.ssl.SSLSocket;
import javax.net.ssl.SSLSocketFactory;
+import javax.net.ssl.X509TrustManager;
+import org.apache.logging.log4j.core.config.Configuration;
import org.apache.logging.log4j.core.config.DefaultConfiguration;
import org.apache.logging.log4j.core.config.Node;
import org.apache.logging.log4j.core.config.plugins.util.PluginBuilder;
@@ -159,4 +172,153 @@ class SslConfigurationTest {
.build();
assertThat(sslConfiguration.isVerifyHostName()).isTrue();
}
+
+ static void handshake(final SslConfiguration clientConfiguration) throws Exception {
+ final SslConfiguration serverConfiguration = createTestSslConfigurationFiles();
+ try (final SSLServerSocket server = (SSLServerSocket) serverConfiguration
+ .getSslContext()
+ .getServerSocketFactory()
+ .createServerSocket(0, 1, InetAddress.getLoopbackAddress())) {
+ server.setSoTimeout(10_000);
+ final CompletableFuture<Void> serverHandshake = CompletableFuture.runAsync(() -> {
+ try (final SSLSocket socket = (SSLSocket) server.accept()) {
+ socket.startHandshake();
+ } catch (final IOException ignored) {
+ }
+ });
+ try (final SSLSocket client = (SSLSocket) clientConfiguration
+ .getSslContext()
+ .getSocketFactory()
+ .createSocket(server.getInetAddress(), server.getLocalPort())) {
+ client.startHandshake();
+ } finally {
+ serverHandshake.join();
+ }
+ }
+ }
+
+ private static SslConfiguration buildSslElement(
+ final Map<String, String> keyStoreAttributes, final Map<String, String> trustStoreAttributes) {
+ final PluginManager pluginManager = new PluginManager(Node.CATEGORY);
+ pluginManager.collectPlugins();
+ final Configuration configuration = new DefaultConfiguration();
+ final PluginType<?> sslType = pluginManager.getPluginType("Ssl");
+ final Node ssl = new Node(null, sslType.getElementName(), sslType);
+ addStoreElement(pluginManager, configuration, ssl, "KeyStore", keyStoreAttributes);
+ addStoreElement(pluginManager, configuration, ssl, "TrustStore", trustStoreAttributes);
+ return (SslConfiguration) new PluginBuilder(sslType)
+ .withConfigurationNode(ssl)
+ .withConfiguration(configuration)
+ .build();
+ }
+
+ private static void addStoreElement(
+ final PluginManager pluginManager,
+ final Configuration configuration,
+ final Node ssl,
+ final String elementName,
+ final Map<String, String> attributes) {
+ final PluginType<?> type = pluginManager.getPluginType(elementName);
+ final Node store = new Node(ssl, type.getElementName(), type);
+ store.getAttributes().putAll(attributes);
+ store.setObject(new PluginBuilder(type)
+ .withConfigurationNode(store)
+ .withConfiguration(configuration)
+ .build());
+ ssl.getChildren().add(store);
+ }
+
+ private static Map<String, String> attributes(final String... namesAndValues) {
+ final Map<String, String> attributes = new HashMap<>();
+ for (int i = 0; i < namesAndValues.length; i += 2) {
+ attributes.put(namesAndValues[i], namesAndValues[i + 1]);
+ }
+ return attributes;
+ }
+
+ private static Map<String, String> validKeyStore() {
+ return attributes(
+ "location",
+ SslKeyStoreConstants.KEYSTORE_LOCATION,
+ "password",
+ new String(SslKeyStoreConstants.KEYSTORE_PWD()),
+ "type",
+ SslKeyStoreConstants.KEYSTORE_TYPE);
+ }
+
+ private static Map<String, String> validTrustStore() {
+ return attributes(
+ "location",
+ SslKeyStoreConstants.TRUSTSTORE_LOCATION,
+ "password",
+ new String(SslKeyStoreConstants.TRUSTSTORE_PWD()),
+ "type",
+ SslKeyStoreConstants.TRUSTSTORE_TYPE);
+ }
+
+ @Test
+ void validSslElementCompletesHandshake() {
+ final SslConfiguration sslConfiguration = buildSslElement(validKeyStore(), validTrustStore());
+ assertThatCode(() -> handshake(sslConfiguration)).doesNotThrowAnyException();
+ }
+
+ @Test
+ @UsingStatusListener
+ void sslElementWithMissingTrustStoreRejectsHandshake() {
+ final SslConfiguration sslConfiguration = buildSslElement(
+ validKeyStore(),
+ attributes(
+ "location",
+ SslKeyStoreConstants.TRUSTSTORE_LOCATION + ".missing",
+ "type",
+ SslKeyStoreConstants.TRUSTSTORE_TYPE));
+ assertThat(sslConfiguration).isNotNull();
+ assertThat(sslConfiguration.getTrustStoreConfig()).isNotNull();
+ assertThatThrownBy(() -> handshake(sslConfiguration)).isInstanceOf(SSLHandshakeException.class);
+ }
+
+ @Test
+ @UsingStatusListener
+ void sslElementWithWrongKeyStorePasswordRejectsHandshake() {
+ final SslConfiguration sslConfiguration = buildSslElement(
+ attributes(
+ "location",
+ SslKeyStoreConstants.KEYSTORE_LOCATION,
+ "password",
+ "wrong",
+ "type",
+ SslKeyStoreConstants.KEYSTORE_TYPE),
+ validTrustStore());
+ assertThat(sslConfiguration).isNotNull();
+ assertThat(sslConfiguration.getKeyStoreConfig()).isNotNull();
+ assertThatThrownBy(() -> handshake(sslConfiguration)).isInstanceOf(SSLHandshakeException.class);
+ }
+
+ @Test
+ @UsingStatusListener
+ void sslContextInitializationFailureDoesNotFallBackToDefault() throws Exception {
+ final KeyStoreConfiguration ksc = new KeyStoreConfiguration(
+ SslKeyStoreConstants.KEYSTORE_LOCATION,
+ new MemoryPasswordProvider(SslKeyStoreConstants.KEYSTORE_PWD()),
+ SslKeyStoreConstants.KEYSTORE_TYPE,
+ "NoSuchKeyManagerFactoryAlgorithm");
+ final TrustStoreConfiguration tsc = new TrustStoreConfiguration(
+ SslKeyStoreConstants.TRUSTSTORE_LOCATION,
+ new MemoryPasswordProvider(SslKeyStoreConstants.TRUSTSTORE_PWD()),
+ SslKeyStoreConstants.TRUSTSTORE_TYPE,
+ null);
+ final SslConfiguration sslConfiguration = SslConfiguration.createSSLConfiguration(null, ksc, tsc);
+ assertThat(sslConfiguration.getSslContext()).isNotSameAs(SSLContext.getDefault());
+ assertThatThrownBy(() -> handshake(sslConfiguration)).isInstanceOf(SSLHandshakeException.class);
+ }
+
+ @Test
+ void rejectingTrustManagerRejectsEveryPeer() {
+ final X509TrustManager trustManager = SslConfiguration.RejectingTrustManager.INSTANCE;
+ assertThatThrownBy(() -> trustManager.checkServerTrusted(new X509Certificate[0], "RSA"))
+ .isInstanceOf(CertificateException.class);
+ assertThatThrownBy(() -> trustManager.checkClientTrusted(new X509Certificate[0], "RSA"))
+ .isInstanceOf(CertificateException.class);
+ assertThat(trustManager.getAcceptedIssuers()).isEmpty();
+ }
}
diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/net/ssl/TrustStoreConfigurationTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/net/ssl/TrustStoreConfigurationTest.java
index 72b605e22b..d476474d23 100644
--- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/net/ssl/TrustStoreConfigurationTest.java
+++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/net/ssl/TrustStoreConfigurationTest.java
@@ -16,6 +16,7 @@
*/
package org.apache.logging.log4j.core.net.ssl;
+import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -104,4 +105,29 @@ class TrustStoreConfigurationTest {
null,
null));
}
+
+ @Test
+ void pluginFactoryRecordsLoadFailureWithEmptyKeyStore() throws Exception {
+ final TrustStoreConfiguration config = TrustStoreConfiguration.createOrRecordFailure(
+ SslKeyStoreConstants.TRUSTSTORE_LOCATION + ".missing",
+ null,
+ null,
+ null,
+ SslKeyStoreConstants.TRUSTSTORE_TYPE,
+ null);
+ assertThat(config.getLoadFailure()).isNotNull();
+ assertThat(config.getKeyStore().size()).isZero();
+ }
+
+ @Test
+ void pluginFactoryRecordsConflictingPasswordSources() {
+ final TrustStoreConfiguration config = TrustStoreConfiguration.createOrRecordFailure(
+ SslKeyStoreConstants.TRUSTSTORE_LOCATION,
+ SslKeyStoreConstants.TRUSTSTORE_PWD(),
+ "LOG4J_TRUSTSTORE_PASSWORD",
+ "trustStore.password",
+ SslKeyStoreConstants.TRUSTSTORE_TYPE,
+ null);
+ assertThat(config.getLoadFailure()).hasCauseInstanceOf(IllegalStateException.class);
+ }
}
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/net/ssl/AbstractKeyStoreConfiguration.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/net/ssl/AbstractKeyStoreConfiguration.java
index a70b71edd3..5800b5595a 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/net/ssl/AbstractKeyStoreConfiguration.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/net/ssl/AbstractKeyStoreConfiguration.java
@@ -19,6 +19,7 @@ package org.apache.logging.log4j.core.net.ssl;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
+import java.security.GeneralSecurityException;
import java.security.KeyStore;
import java.security.KeyStoreException;
import java.security.NoSuchAlgorithmException;
@@ -37,12 +38,33 @@ public class AbstractKeyStoreConfiguration extends StoreConfiguration<KeyStore>
private final transient KeyStore keyStore;
+ private final StoreConfigurationException loadFailure;
+
public AbstractKeyStoreConfiguration(
final String location, final PasswordProvider passwordProvider, final String keyStoreType)
throws StoreConfigurationException {
super(location, passwordProvider);
this.keyStoreType = keyStoreType == null ? SslConfigurationDefaults.KEYSTORE_TYPE : keyStoreType;
this.keyStore = this.load();
+ this.loadFailure = null;
+ }
+
+ AbstractKeyStoreConfiguration(
+ final String location, final String keyStoreType, final StoreConfigurationException loadFailure) {
+ super(location, new MemoryPasswordProvider(null));
+ this.keyStoreType = keyStoreType == null ? SslConfigurationDefaults.KEYSTORE_TYPE : keyStoreType;
+ this.keyStore = createEmptyKeyStore();
+ this.loadFailure = Objects.requireNonNull(loadFailure, "loadFailure");
+ }
+
+ private static KeyStore createEmptyKeyStore() {
+ try {
+ final KeyStore keyStore = KeyStore.getInstance(SslConfigurationDefaults.KEYSTORE_TYPE);
+ keyStore.load(null, null);
+ return keyStore;
+ } catch (final GeneralSecurityException | IOException error) {
+ throw new IllegalStateException("Failed to create an empty key store", error);
+ }
}
/**
@@ -123,6 +145,10 @@ public class AbstractKeyStoreConfiguration extends StoreConfiguration<KeyStore>
return this.keyStore;
}
+ StoreConfigurationException getLoadFailure() {
+ return loadFailure;
+ }
+
@Override
public int hashCode() {
final int prime = 31;
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/net/ssl/KeyStoreConfiguration.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/net/ssl/KeyStoreConfiguration.java
index 205f7b0d9d..9b57538489 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/net/ssl/KeyStoreConfiguration.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/net/ssl/KeyStoreConfiguration.java
@@ -51,6 +51,17 @@ public class KeyStoreConfiguration extends AbstractKeyStoreConfiguration {
: keyManagerFactoryAlgorithm;
}
+ private KeyStoreConfiguration(
+ final String location,
+ final String keyStoreType,
+ final String keyManagerFactoryAlgorithm,
+ final StoreConfigurationException loadFailure) {
+ super(location, keyStoreType, loadFailure);
+ this.keyManagerFactoryAlgorithm = keyManagerFactoryAlgorithm == null
+ ? KeyManagerFactory.getDefaultAlgorithm()
+ : keyManagerFactoryAlgorithm;
+ }
+
/**
*
* @throws StoreConfigurationException Thrown if this instance cannot load the KeyStore.
@@ -102,7 +113,6 @@ public class KeyStoreConfiguration extends AbstractKeyStoreConfiguration {
* @return a new KeyStoreConfiguration
* @throws StoreConfigurationException Thrown if this call cannot load the KeyStore.
*/
- @PluginFactory
public static KeyStoreConfiguration createKeyStoreConfiguration(
// @formatter:off
@PluginAttribute("location") final String location,
@@ -136,6 +146,29 @@ public class KeyStoreConfiguration extends AbstractKeyStoreConfiguration {
}
}
+ @PluginFactory
+ static KeyStoreConfiguration createOrRecordFailure(
+ // @formatter:off
+ @PluginAttribute("location") final String location,
+ @PluginAttribute(value = "password", sensitive = true) final char[] password,
+ @PluginAttribute("passwordEnvironmentVariable") final String passwordEnvironmentVariable,
+ @PluginAttribute("passwordFile") final String passwordFile,
+ @PluginAttribute("type") final String keyStoreType,
+ @PluginAttribute("keyManagerFactoryAlgorithm") final String keyManagerFactoryAlgorithm) {
+ // @formatter:on
+ try {
+ return createKeyStoreConfiguration(
+ location,
+ password,
+ passwordEnvironmentVariable,
+ passwordFile,
+ keyStoreType,
+ keyManagerFactoryAlgorithm);
+ } catch (final StoreConfigurationException error) {
+ return new KeyStoreConfiguration(location, keyStoreType, keyManagerFactoryAlgorithm, error);
+ }
+ }
+
/**
* @deprecated use {@link #createKeyStoreConfiguration(String, char[], String, String, String, String)}
*/
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/net/ssl/SslConfiguration.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/net/ssl/SslConfiguration.java
index 1d4065f9af..611a234b97 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/net/ssl/SslConfiguration.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/net/ssl/SslConfiguration.java
@@ -16,7 +16,9 @@
*/
package org.apache.logging.log4j.core.net.ssl;
-import java.security.NoSuchAlgorithmException;
+import java.security.GeneralSecurityException;
+import java.security.cert.CertificateException;
+import java.security.cert.X509Certificate;
import java.util.Objects;
import javax.net.ssl.KeyManager;
import javax.net.ssl.KeyManagerFactory;
@@ -25,6 +27,7 @@ import javax.net.ssl.SSLServerSocketFactory;
import javax.net.ssl.SSLSocketFactory;
import javax.net.ssl.TrustManager;
import javax.net.ssl.TrustManagerFactory;
+import javax.net.ssl.X509TrustManager;
import org.apache.logging.log4j.core.Core;
import org.apache.logging.log4j.core.config.plugins.Plugin;
import org.apache.logging.log4j.core.config.plugins.PluginAttribute;
@@ -106,30 +109,14 @@ public class SslConfiguration {
return sslContext != null ? sslContext.getServerSocketFactory() : null;
}
- @Nullable
- private static SSLContext createDefaultSslContext(final String protocol) {
- try {
- return SSLContext.getDefault();
- } catch (final NoSuchAlgorithmException defaultContextError) {
- LOGGER.error(
- "Failed to create an `SSLContext` using the default configuration, falling back to creating an empty one",
- defaultContextError);
- try {
- final SSLContext emptyContext = SSLContext.getInstance(protocol);
- emptyContext.init(new KeyManager[0], new TrustManager[0], null);
- return emptyContext;
- } catch (final Exception emptyContextError) {
- LOGGER.error("Failed to create an empty `SSLContext`", emptyContextError);
- return null;
- }
- }
- }
-
@Nullable
private static SSLContext createSslContext(
final String protocol,
@Nullable final KeyStoreConfiguration keyStoreConfig,
@Nullable final TrustStoreConfiguration trustStoreConfig) {
+ if (hasLoadFailure(keyStoreConfig) || hasLoadFailure(trustStoreConfig)) {
+ return createRejectingSslContext();
+ }
try {
final SSLContext sslContext = SSLContext.getInstance(protocol);
@Nullable final KeyManager[] keyManagers = loadKeyManagers(keyStoreConfig);
@@ -138,9 +125,32 @@ public class SslConfiguration {
return sslContext;
} catch (final Exception error) {
LOGGER.error(
- "Failed to create an `SSLContext` using the provided configuration, falling back to a default instance",
+ "Failed to create an `SSLContext` using the provided configuration, all TLS connections using this configuration will be rejected",
error);
- return createDefaultSslContext(protocol);
+ return createRejectingSslContext();
+ }
+ }
+
+ private static boolean hasLoadFailure(@Nullable final AbstractKeyStoreConfiguration storeConfig) {
+ if (storeConfig == null || storeConfig.getLoadFailure() == null) {
+ return false;
+ }
+ LOGGER.error(
+ "Failed to load the store located at `{}`, all TLS connections using this configuration will be rejected",
+ storeConfig.getLocation(),
+ storeConfig.getLoadFailure());
+ return true;
+ }
+
+ @Nullable
+ private static SSLContext createRejectingSslContext() {
+ try {
+ final SSLContext sslContext = SSLContext.getInstance(SslConfigurationDefaults.PROTOCOL);
+ sslContext.init(new KeyManager[0], new TrustManager[] {RejectingTrustManager.INSTANCE}, null);
+ return sslContext;
+ } catch (final GeneralSecurityException error) {
+ LOGGER.error("Failed to create an `SSLContext` rejecting all connections", error);
+ return null;
}
}
@@ -263,4 +273,28 @@ public class SslConfiguration {
public SSLContext getSslContext() {
return sslContext;
}
+
+ static final class RejectingTrustManager implements X509TrustManager {
+
+ static final RejectingTrustManager INSTANCE = new RejectingTrustManager();
+
+ private static final String MESSAGE = "The `Ssl` configuration is invalid, see the status logger for the cause";
+
+ @Override
+ public void checkClientTrusted(final X509Certificate[] chain, final String authType)
+ throws CertificateException {
+ throw new CertificateException(MESSAGE);
+ }
+
+ @Override
+ public void checkServerTrusted(final X509Certificate[] chain, final String authType)
+ throws CertificateException {
+ throw new CertificateException(MESSAGE);
+ }
+
+ @Override
+ public X509Certificate[] getAcceptedIssuers() {
+ return new X509Certificate[0];
+ }
+ }
}
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/net/ssl/SslConfigurationFactory.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/net/ssl/SslConfigurationFactory.java
index 2da16b886d..20d7da861a 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/net/ssl/SslConfigurationFactory.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/net/ssl/SslConfigurationFactory.java
@@ -16,8 +16,6 @@
*/
package org.apache.logging.log4j.core.net.ssl;
-import org.apache.logging.log4j.Logger;
-import org.apache.logging.log4j.status.StatusLogger;
import org.apache.logging.log4j.util.PropertiesUtil;
import org.apache.logging.log4j.util.Strings;
@@ -26,8 +24,6 @@ import org.apache.logging.log4j.util.Strings;
*/
public class SslConfigurationFactory {
- private static final Logger LOGGER = StatusLogger.getLogger();
-
private static final String trustStorelocation = "log4j2.trustStoreLocation";
private static final String trustStorePassword = "log4j2.trustStorePassword";
private static final String trustStorePasswordFile = "log4j2.trustStorePasswordFile";
@@ -50,40 +46,26 @@ public class SslConfigurationFactory {
if (Strings.isNotEmpty(location) || storeType != null) {
final String password = props.getStringProperty(trustStorePassword);
final char[] passwordChars = getPassword(password, storeType);
- try {
- trustStoreConfiguration = TrustStoreConfiguration.createKeyStoreConfiguration(
- Strings.trimToNull(location),
- passwordChars,
- props.getStringProperty(trustStorePasswordEnvVar),
- props.getStringProperty(trustStorePasswordFile),
- storeType,
- props.getStringProperty(trustStoreKeyManagerFactoryAlgorithm));
- } catch (Exception ex) {
- LOGGER.warn(
- "Unable to create trust store configuration due to: {} {}",
- ex.getClass().getName(),
- ex.getMessage());
- }
+ trustStoreConfiguration = TrustStoreConfiguration.createOrRecordFailure(
+ Strings.trimToNull(location),
+ passwordChars,
+ props.getStringProperty(trustStorePasswordEnvVar),
+ props.getStringProperty(trustStorePasswordFile),
+ storeType,
+ props.getStringProperty(trustStoreKeyManagerFactoryAlgorithm));
}
location = props.getStringProperty(keyStoreLocation);
storeType = props.getStringProperty(keyStoreType);
if (Strings.isNotEmpty(location) || storeType != null) {
final String password = props.getStringProperty(keyStorePassword);
final char[] passwordChars = getPassword(password, storeType);
- try {
- keyStoreConfiguration = KeyStoreConfiguration.createKeyStoreConfiguration(
- Strings.trimToNull(location),
- passwordChars,
- props.getStringProperty(keyStorePasswordEnvVar),
- props.getStringProperty(keyStorePasswordFile),
- storeType,
- props.getStringProperty(keyStoreKeyManagerFactoryAlgorithm));
- } catch (Exception ex) {
- LOGGER.warn(
- "Unable to create key store configuration due to: {} {}",
- ex.getClass().getName(),
- ex.getMessage());
- }
+ keyStoreConfiguration = KeyStoreConfiguration.createOrRecordFailure(
+ Strings.trimToNull(location),
+ passwordChars,
+ props.getStringProperty(keyStorePasswordEnvVar),
+ props.getStringProperty(keyStorePasswordFile),
+ storeType,
+ props.getStringProperty(keyStoreKeyManagerFactoryAlgorithm));
}
if (trustStoreConfiguration != null || keyStoreConfiguration != null) {
final boolean isVerifyHostName = props.getBooleanProperty(verifyHostName, false);
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/net/ssl/TrustStoreConfiguration.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/net/ssl/TrustStoreConfiguration.java
index 5b27b883b0..74d70ef077 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/net/ssl/TrustStoreConfiguration.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/net/ssl/TrustStoreConfiguration.java
@@ -46,6 +46,17 @@ public class TrustStoreConfiguration extends AbstractKeyStoreConfiguration {
: trustManagerFactoryAlgorithm;
}
+ private TrustStoreConfiguration(
+ final String location,
+ final String keyStoreType,
+ final String trustManagerFactoryAlgorithm,
+ final StoreConfigurationException loadFailure) {
+ super(location, keyStoreType, loadFailure);
+ this.trustManagerFactoryAlgorithm = trustManagerFactoryAlgorithm == null
+ ? TrustManagerFactory.getDefaultAlgorithm()
+ : trustManagerFactoryAlgorithm;
+ }
+
/**
* @deprecated Use {@link #TrustStoreConfiguration(String, PasswordProvider, String, String)} instead
*/
@@ -93,7 +104,6 @@ public class TrustStoreConfiguration extends AbstractKeyStoreConfiguration {
* @return a new TrustStoreConfiguration
* @throws StoreConfigurationException Thrown if this instance cannot load the KeyStore.
*/
- @PluginFactory
public static TrustStoreConfiguration createKeyStoreConfiguration(
// @formatter:off
@PluginAttribute("location") final String location,
@@ -127,6 +137,35 @@ public class TrustStoreConfiguration extends AbstractKeyStoreConfiguration {
}
}
+ @PluginFactory
+ static TrustStoreConfiguration createOrRecordFailure(
+ // @formatter:off
+ @PluginAttribute("location") final String location,
+ @PluginAttribute(value = "password", sensitive = true) final char[] password,
+ @PluginAttribute("passwordEnvironmentVariable") final String passwordEnvironmentVariable,
+ @PluginAttribute("passwordFile") final String passwordFile,
+ @PluginAttribute("type") final String keyStoreType,
+ @PluginAttribute("trustManagerFactoryAlgorithm") final String trustManagerFactoryAlgorithm) {
+ // @formatter:on
+ try {
+ return createKeyStoreConfiguration(
+ location,
+ password,
+ passwordEnvironmentVariable,
+ passwordFile,
+ keyStoreType,
+ trustManagerFactoryAlgorithm);
+ } catch (final StoreConfigurationException error) {
+ return new TrustStoreConfiguration(location, keyStoreType, trustManagerFactoryAlgorithm, error);
+ } catch (final IllegalStateException error) {
+ return new TrustStoreConfiguration(
+ location,
+ keyStoreType,
+ trustManagerFactoryAlgorithm,
+ new StoreConfigurationException(error.getMessage(), error));
+ }
+ }
+
/**
* @deprecated Use {@link #createKeyStoreConfiguration(String, char[], String, String, String, String)}
*/
diff --git a/src/changelog/.2.x.x/4332_fail_closed_on_invalid_ssl_store.xml b/src/changelog/.2.x.x/4332_fail_closed_on_invalid_ssl_store.xml
new file mode 100644
index 0000000000..0a5326be3f
--- /dev/null
+++ b/src/changelog/.2.x.x/4332_fail_closed_on_invalid_ssl_store.xml
@@ -0,0 +1,13 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns="https://logging.apache.org/xml/ns"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="
+ https://logging.apache.org/xml/ns
+ https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
+ type="changed">
+ <issue id="4332" link="https://github.com/apache/logging-log4j2/issues/4332"/>
+ <issue id="4337" link="https://github.com/apache/logging-log4j2/pull/4337"/>
+ <description format="asciidoc">
+ Fail TLS connections when an `Ssl` key store or trust store cannot be loaded.
+ </description>
+</entry>