Fix nullability issues in SslConfiguration (#3953)

Co-authored-by: Volkan Yazıcı <volkan@yazi.ci>

Author: jvz

log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/TlsSyslogAppenderTest.java

@@ -83,7 +83,9 @@ private void initServerSocketFactory() throws StoreConfigurationException {
         final TrustStoreConfiguration tsc = new TrustStoreConfiguration(
                 SslKeyStoreConstants.TRUSTSTORE_LOCATION, SslKeyStoreConstants::TRUSTSTORE_PWD, null, null);
         sslConfiguration = SslConfiguration.createSSLConfiguration(null, ksc, tsc);
-        serverSocketFactory = sslConfiguration.getSslContext().getServerSocketFactory();
+        serverSocketFactory = sslConfiguration.getSslContext() != null
+                ? sslConfiguration.getSslContext().getServerSocketFactory()
+                : (SSLServerSocketFactory) SSLServerSocketFactory.getDefault();
     }
 
     private void initTlsTestEnvironment(final int numberOfMessages, final TlsSyslogMessageFormat messageFormat)

log4j-core-test/src/test/java/org/apache/logging/log4j/core/net/ssl/SslSocketManagerTest.java

@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.logging.log4j.core.net.ssl;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+
+import org.apache.logging.log4j.core.layout.PatternLayout;
+import org.apache.logging.log4j.core.net.SslSocketManager;
+import org.junit.jupiter.api.Test;
+import org.junitpioneer.jupiter.Issue;
+
+class SslSocketManagerTest {
+    @Issue("https://github.com/apache/logging-log4j2/issues/3947")
+    @Test
+    void shouldNotThrowExceptionWhenConfiguringTrustStore() {
+        final TrustStoreConfiguration trustStoreConfiguration = assertDoesNotThrow(() -> new TrustStoreConfiguration(
+                SslKeyStoreConstants.TRUSTSTORE_LOCATION,
+                SslKeyStoreConstants::TRUSTSTORE_PWD,
+                SslKeyStoreConstants.TRUSTSTORE_TYPE,
+                null));
+        final SslConfiguration sslConfiguration =
+                SslConfiguration.createSSLConfiguration(null, null, trustStoreConfiguration);
+        assertDoesNotThrow(() -> {
+            // noinspection EmptyTryBlock (try-with-resources to close `SslSocketManager`, even on failure
+            try (final SslSocketManager ignored = SslSocketManager.getSocketManager(
+                    sslConfiguration, "localhost", 0, 0, 0, true, PatternLayout.createDefaultLayout(), 8192, null)) {
+                // Do nothing
+            }
+        });
+    }
+}

log4j-core/src/main/java/org/apache/logging/log4j/core/appender/HttpURLConnectionManager.java

@@ -23,8 +23,10 @@
 import java.net.HttpURLConnection;
 import java.net.URL;
 import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
 import java.util.Objects;
 import javax.net.ssl.HttpsURLConnection;
+import javax.net.ssl.SSLContext;
 import org.apache.logging.log4j.core.Layout;
 import org.apache.logging.log4j.core.LogEvent;
 import org.apache.logging.log4j.core.LoggerContext;
@@ -37,7 +39,7 @@
 
 public class HttpURLConnectionManager extends HttpManager {
 
-    private static final Charset CHARSET = Charset.forName("US-ASCII");
+    private static final Charset CHARSET = StandardCharsets.US_ASCII;
 
     private final URL url;
     private final boolean isHttps;
@@ -100,8 +102,10 @@ public void send(final Layout<?> layout, final LogEvent event) throws IOExceptio
                     header.getName(), header.evaluate(getConfiguration().getStrSubstitutor()));
         }
         if (sslConfiguration != null) {
-            ((HttpsURLConnection) urlConnection)
-                    .setSSLSocketFactory(sslConfiguration.getSslContext().getSocketFactory());
+            final SSLContext sslContext = sslConfiguration.getSslContext();
+            if (sslContext != null) {
+                ((HttpsURLConnection) urlConnection).setSSLSocketFactory(sslContext.getSocketFactory());
+            }
         }
         if (isHttps && !verifyHostname) {
             ((HttpsURLConnection) urlConnection).setHostnameVerifier(LaxHostnameVerifier.INSTANCE);

log4j-core/src/main/java/org/apache/logging/log4j/core/net/SmtpManager.java

@@ -35,6 +35,7 @@
 import javax.mail.internet.MimeMultipart;
 import javax.mail.internet.MimeUtility;
 import javax.mail.util.ByteArrayDataSource;
+import javax.net.ssl.SSLContext;
 import javax.net.ssl.SSLSocketFactory;
 import org.apache.logging.log4j.LoggingException;
 import org.apache.logging.log4j.core.Layout;
@@ -308,9 +309,11 @@ public SmtpManager createManager(final String name, final FactoryData data) {
             if (smtpProtocol.equals("smtps")) {
                 final SslConfiguration sslConfiguration = data.getSslConfiguration();
                 if (sslConfiguration != null) {
-                    final SSLSocketFactory sslSocketFactory =
-                            sslConfiguration.getSslContext().getSocketFactory();
-                    properties.put(prefix + ".ssl.socketFactory", sslSocketFactory);
+                    final SSLContext sslContext = sslConfiguration.getSslContext();
+                    if (sslContext != null) {
+                        final SSLSocketFactory sslSocketFactory = sslContext.getSocketFactory();
+                        properties.put(prefix + ".ssl.socketFactory", sslSocketFactory);
+                    }
                     properties.setProperty(
                             prefix + ".ssl.checkserveridentity", Boolean.toString(sslConfiguration.isVerifyHostName()));
                 }

log4j-core/src/main/java/org/apache/logging/log4j/core/net/SslSocketManager.java

@@ -27,8 +27,10 @@
 import java.util.Collections;
 import java.util.Enumeration;
 import java.util.List;
+import java.util.Objects;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
+import javax.net.ssl.SSLContext;
 import javax.net.ssl.SSLSocket;
 import javax.net.ssl.SSLSocketFactory;
 import org.apache.logging.log4j.core.Layout;
@@ -245,6 +247,7 @@ public static SslSocketManager getSocketManager(
      */
     private static String createSslConfigurationId(final SslConfiguration sslConfig) {
         return String.valueOf(Stream.of(sslConfig.getKeyStoreConfig(), sslConfig.getTrustStoreConfig())
+                .filter(Objects::nonNull)
                 .flatMap(keyStoreConfig -> {
                     final Enumeration<String> aliases;
                     try {
@@ -289,15 +292,13 @@ protected Socket createSocket(final InetSocketAddress socketAddress) throws IOEx
     }
 
     private static SSLSocketFactory createSslSocketFactory(final SslConfiguration sslConf) {
-        SSLSocketFactory socketFactory;
-
         if (sslConf != null) {
-            socketFactory = sslConf.getSslContext().getSocketFactory();
-        } else {
-            socketFactory = (SSLSocketFactory) SSLSocketFactory.getDefault();
+            final SSLContext sslContext = sslConf.getSslContext();
+            if (sslContext != null) {
+                return sslContext.getSocketFactory();
+            }
         }
-
-        return socketFactory;
+        return (SSLSocketFactory) SSLSocketFactory.getDefault();
     }
 
     private static class SslSocketManagerFactory extends TcpSocketManagerFactory<SslSocketManager, SslFactoryData> {

log4j-core/src/main/java/org/apache/logging/log4j/core/net/UrlConnectionFactory.java

@@ -28,6 +28,7 @@
 import java.util.Arrays;
 import java.util.List;
 import javax.net.ssl.HttpsURLConnection;
+import javax.net.ssl.SSLContext;
 import org.apache.logging.log4j.core.config.ConfigurationFactory;
 import org.apache.logging.log4j.core.net.ssl.LaxHostnameVerifier;
 import org.apache.logging.log4j.core.net.ssl.SslConfiguration;
@@ -120,10 +121,13 @@ public static <T extends URLConnection> T createConnection(
                 httpURLConnection.setIfModifiedSince(lastModifiedMillis);
             }
             if (url.getProtocol().equals(HTTPS) && sslConfiguration != null) {
-                ((HttpsURLConnection) httpURLConnection)
-                        .setSSLSocketFactory(sslConfiguration.getSslContext().getSocketFactory());
+                final SSLContext sslContext = sslConfiguration.getSslContext();
+                final HttpsURLConnection httpsURLConnection = (HttpsURLConnection) httpURLConnection;
+                if (sslContext != null) {
+                    httpsURLConnection.setSSLSocketFactory(sslContext.getSocketFactory());
+                }
                 if (!sslConfiguration.isVerifyHostName()) {
-                    ((HttpsURLConnection) httpURLConnection).setHostnameVerifier(LaxHostnameVerifier.INSTANCE);
+                    httpsURLConnection.setHostnameVerifier(LaxHostnameVerifier.INSTANCE);
                 }
             }
             urlConnection = httpURLConnection;

log4j-core/src/main/java/org/apache/logging/log4j/core/net/ssl/SslConfiguration.java

@@ -54,6 +54,7 @@ public class SslConfiguration {
     @Nullable
     private final TrustStoreConfiguration trustStoreConfig;
 
+    @Nullable
     private final transient SSLContext sslContext;
 
     private SslConfiguration(
@@ -88,8 +89,9 @@ public void clearSecrets() {
      * @deprecated Use {@link SSLContext#getSocketFactory()} on {@link #getSslContext()}
      */
     @Deprecated
+    @Nullable
     public SSLSocketFactory getSslSocketFactory() {
-        return sslContext.getSocketFactory();
+        return sslContext != null ? sslContext.getSocketFactory() : null;
     }
 
     /**
@@ -99,10 +101,12 @@ public SSLSocketFactory getSslSocketFactory() {
      * @deprecated Use {@link SSLContext#getServerSocketFactory()} on {@link #getSslContext()}
      */
     @Deprecated
+    @Nullable
     public SSLServerSocketFactory getSslServerSocketFactory() {
-        return sslContext.getServerSocketFactory();
+        return sslContext != null ? sslContext.getServerSocketFactory() : null;
     }
 
+    @Nullable
     private static SSLContext createDefaultSslContext(final String protocol) {
         try {
             return SSLContext.getDefault();
@@ -121,6 +125,7 @@ private static SSLContext createDefaultSslContext(final String protocol) {
         }
     }
 
+    @Nullable
     private static SSLContext createSslContext(
             final String protocol,
             @Nullable final KeyStoreConfiguration keyStoreConfig,
@@ -242,14 +247,17 @@ public boolean isVerifyHostName() {
         return verifyHostName;
     }
 
+    @Nullable
     public KeyStoreConfiguration getKeyStoreConfig() {
         return keyStoreConfig;
     }
 
+    @Nullable
     public TrustStoreConfiguration getTrustStoreConfig() {
         return trustStoreConfig;
     }
 
+    @Nullable
     public SSLContext getSslContext() {
         return sslContext;
     }

log4j-jakarta-smtp/src/main/java/org/apache/logging/log4j/smtp/SmtpManager.java

@@ -36,6 +36,7 @@
 import java.io.OutputStream;
 import java.util.Date;
 import java.util.Properties;
+import javax.net.ssl.SSLContext;
 import javax.net.ssl.SSLSocketFactory;
 import org.apache.logging.log4j.LoggingException;
 import org.apache.logging.log4j.core.Layout;
@@ -262,9 +263,11 @@ public SmtpManager createManager(final String name, final FactoryData data) {
             if (smtpProtocol.equals("smtps")) {
                 final SslConfiguration sslConfiguration = data.getSslConfiguration();
                 if (sslConfiguration != null) {
-                    final SSLSocketFactory sslSocketFactory =
-                            sslConfiguration.getSslContext().getSocketFactory();
-                    properties.put(prefix + ".ssl.socketFactory", sslSocketFactory);
+                    final SSLContext sslContext = sslConfiguration.getSslContext();
+                    if (sslContext != null) {
+                        final SSLSocketFactory sslSocketFactory = sslContext.getSocketFactory();
+                        properties.put(prefix + ".ssl.socketFactory", sslSocketFactory);
+                    }
                     properties.setProperty(
                             prefix + ".ssl.checkserveridentity", Boolean.toString(sslConfiguration.isVerifyHostName()));
                 }

src/changelog/.2.x.x/3947_fix_SslSocketManager_null_keystore.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="fixed">
+    <issue id="3947" link="https://github.com/apache/logging-log4j2/issues/3947"/>
+    <issue id="3953" link="https://github.com/apache/logging-log4j2/pull/3953"/>
+    <description format="asciidoc">
+        Fix failures caused by null `SslConfiguration`
+    </description>
+</entry>