This is an automated email from the ASF dual-hosted git repository.
rzo1 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/storm.git
The following commit(s) were added to refs/heads/master by this push:
new 1681a12db Fix TLS client auth socket configuration and modernize
certificate handling
1681a12db is described below
commit 1681a12dbe581d42dc5d7fcbc7cdfb0f29c0e4f0
Author: Richard Zowalla <[email protected]>
AuthorDate: Thu Apr 16 19:21:31 2026 +0200
Fix TLS client auth socket configuration and modernize certificate handling
- The SSLServerSocket setup called setWantClientAuth() after
setNeedClientAuth(), which per the JSSE specification clears the
needClientAuth flag. Remove the redundant setWantClientAuth() call so the
configured client-auth mode is applied correctly.
- Replace deprecated javax.security.cert.X509Certificate /
getPeerCertificateChain()
with java.security.cert.X509Certificate / getPeerCertificates()
- Make TTlsWrapProcessor aware of the client-auth configuration so it can
distinguish encryption-only from mutual-auth mode when handling peer
certificate lookup failures
- Add regression test for mutual-auth TLS handshake enforcement
---
.../auth/tls/ReloadableTsslTransportFactory.java | 1 -
.../security/auth/tls/TlsTransportPlugin.java | 29 +++++---
.../security/auth/TlsTransportPluginTest.java | 81 ++++++++++++++++++++++
3 files changed, 101 insertions(+), 10 deletions(-)
diff --git
a/storm-client/src/jvm/org/apache/storm/security/auth/tls/ReloadableTsslTransportFactory.java
b/storm-client/src/jvm/org/apache/storm/security/auth/tls/ReloadableTsslTransportFactory.java
index 0d128a456..34c9534ec 100644
---
a/storm-client/src/jvm/org/apache/storm/security/auth/tls/ReloadableTsslTransportFactory.java
+++
b/storm-client/src/jvm/org/apache/storm/security/auth/tls/ReloadableTsslTransportFactory.java
@@ -64,7 +64,6 @@ public class ReloadableTsslTransportFactory extends
TSSLTransportFactory {
serverSocket.setSoTimeout(timeout);
serverSocket.setNeedClientAuth(clientAuth);
- serverSocket.setWantClientAuth(clientAuth);
return new TServerSocket(new
TServerSocket.ServerSocketTransportArgs().serverSocket(serverSocket).clientTimeout(timeout));
} catch (Exception e) {
throw new TTransportException("Could not bind to port " + port, e);
diff --git
a/storm-client/src/jvm/org/apache/storm/security/auth/tls/TlsTransportPlugin.java
b/storm-client/src/jvm/org/apache/storm/security/auth/tls/TlsTransportPlugin.java
index e260c4d0f..211cd7bdc 100644
---
a/storm-client/src/jvm/org/apache/storm/security/auth/tls/TlsTransportPlugin.java
+++
b/storm-client/src/jvm/org/apache/storm/security/auth/tls/TlsTransportPlugin.java
@@ -16,7 +16,8 @@ import java.io.IOException;
import java.io.UnsupportedEncodingException;
import java.net.InetAddress;
import java.net.ServerSocket;
-import java.security.Principal;
+import java.security.cert.Certificate;
+import java.security.cert.X509Certificate;
import java.util.Map;
import java.util.concurrent.ArrayBlockingQueue;
import java.util.concurrent.BlockingQueue;
@@ -26,7 +27,6 @@ import java.util.concurrent.TimeUnit;
import javax.net.ssl.SSLPeerUnverifiedException;
import javax.net.ssl.SSLSocket;
import javax.security.auth.Subject;
-import javax.security.cert.X509Certificate;
import org.apache.storm.security.auth.ITransportPlugin;
import org.apache.storm.security.auth.ReqContext;
import org.apache.storm.security.auth.SingleUserPrincipal;
@@ -97,7 +97,7 @@ public class TlsTransportPlugin implements ITransportPlugin {
Integer queueSize = type.getQueueSize(conf);
TThreadPoolServer.Args serverArgs = new
TThreadPoolServer.Args(serverTransport)
- .processor(new TTlsWrapProcessor(processor))
+ .processor(new TTlsWrapProcessor(processor,
type.isClientAuthRequired(conf)))
.minWorkerThreads(numWorkerThreads)
.maxWorkerThreads(numWorkerThreads)
.protocolFactory(new TBinaryProtocol.Factory(false, true));
@@ -130,9 +130,11 @@ public class TlsTransportPlugin implements
ITransportPlugin {
private static class TTlsWrapProcessor implements TProcessor {
final TProcessor wrapped;
+ final boolean clientAuthRequired;
- TTlsWrapProcessor(TProcessor wrapped) {
+ TTlsWrapProcessor(TProcessor wrapped, boolean clientAuthRequired) {
this.wrapped = wrapped;
+ this.clientAuthRequired = clientAuthRequired;
}
@Override
@@ -144,13 +146,22 @@ public class TlsTransportPlugin implements
ITransportPlugin {
String principalName = ANONYMOUS_PRINCIPAL_NAME;
try {
- for (X509Certificate cert:
socket.getSession().getPeerCertificateChain()) {
- Principal principal = cert.getSubjectDN();
- principalName = principal.getName();
- break;
+ Certificate[] peers =
socket.getSession().getPeerCertificates();
+ if (peers.length > 0 && peers[0] instanceof X509Certificate) {
+ principalName = ((X509Certificate)
peers[0]).getSubjectX500Principal().getName();
+ } else if (clientAuthRequired) {
+ throw new TException("TLS peer presented no X.509
certificate");
}
} catch (SSLPeerUnverifiedException e) {
- LOG.warn("Client cert is not verified. Set principalName={}.",
principalName, e);
+ // Encryption-only mode (clientAuthRequired=false): fall
through with CN=ANONYMOUS.
+ // When client auth IS required this branch is
belt-and-suspenders against a
+ // misconfigured server socket — fail closed rather than
assign a default identity.
+ if (clientAuthRequired) {
+ LOG.warn("Rejecting TLS connection from {}: peer
certificate not verified",
+ socket.getInetAddress());
+ throw new TException("TLS peer not verified", e);
+ }
+ LOG.debug("Client cert not presented;
clientAuthRequired=false, using {}", principalName);
}
LOG.debug("principalName : {} ", principalName);
ReqContext reqContext = ReqContext.context();
diff --git
a/storm-client/test/jvm/org/apache/storm/security/auth/TlsTransportPluginTest.java
b/storm-client/test/jvm/org/apache/storm/security/auth/TlsTransportPluginTest.java
index 252b7cabf..5e66be3f6 100644
---
a/storm-client/test/jvm/org/apache/storm/security/auth/TlsTransportPluginTest.java
+++
b/storm-client/test/jvm/org/apache/storm/security/auth/TlsTransportPluginTest.java
@@ -13,13 +13,30 @@
package org.apache.storm.security.auth;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.InetAddress;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.security.KeyStore;
+import java.security.SecureRandom;
import java.util.HashMap;
import java.util.Map;
+import java.util.concurrent.atomic.AtomicReference;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLException;
+import javax.net.ssl.SSLHandshakeException;
+import javax.net.ssl.SSLServerSocket;
+import javax.net.ssl.SSLSocket;
+import javax.net.ssl.TrustManagerFactory;
import org.apache.storm.Config;
import org.apache.storm.generated.Nimbus;
+import org.apache.storm.security.auth.tls.ReloadableTsslTransportFactory;
import org.apache.storm.security.auth.tls.TlsTransportPlugin;
+import org.apache.storm.thrift.transport.TServerSocket;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@@ -35,6 +52,7 @@ class TlsTransportPluginTest {
@BeforeEach
void setUp() {
tlsTransportPlugin = new TlsTransportPlugin();
+ conf.clear();
conf.put(Config.STORM_THRIFT_TRANSPORT_PLUGIN,
TlsTransportPlugin.class.getName());
handler = mock(Nimbus.Iface.class);
}
@@ -65,4 +83,67 @@ class TlsTransportPluginTest {
fail("TLS setup failed: " + e.getMessage());
}
}
+
+ /**
+ * Regression test for VULN-14. With {@code
nimbus.thrift.tls.client.auth.required=true} the
+ * factory must leave the SSLServerSocket in true "need client auth" mode,
so that JSSE fails
+ * the handshake closed when the client presents no certificate. The
{@code CN=ANONYMOUS}
+ * fallback branch in {@link TlsTransportPlugin} must therefore be
unreachable.
+ */
+ @Test
+ void testClientAuthRequiredRejectsUnauthenticatedClient() throws Exception
{
+ conf.put(Config.NIMBUS_THRIFT_TLS_PORT, 0);
+ conf.put(Config.STORM_THRIFT_TLS_SOCKET_TIMEOUT_MS, 5000);
+ conf.put(Config.NIMBUS_THRIFT_TLS_SERVER_KEYSTORE_PATH, testDataPath +
"testKeyStore.jks");
+ conf.put(Config.NIMBUS_THRIFT_TLS_SERVER_KEYSTORE_PASSWORD,
"testpass");
+ conf.put(Config.NIMBUS_THRIFT_TLS_SERVER_TRUSTSTORE_PATH, testDataPath
+ "testTrustStore.jks");
+ conf.put(Config.NIMBUS_THRIFT_TLS_SERVER_TRUSTSTORE_PASSWORD,
"testpass");
+ conf.put(Config.NIMBUS_THRIFT_TLS_CLIENT_AUTH_REQUIRED, true);
+
+ TServerSocket serverTransport =
ReloadableTsslTransportFactory.getServerSocket(
+ 0, 5000, InetAddress.getLoopbackAddress(), type, conf);
+ SSLServerSocket serverSocket = (SSLServerSocket)
serverTransport.getServerSocket();
+ try {
+ assertTrue(serverSocket.getNeedClientAuth(),
+ "Factory must leave needClientAuth=true so JSSE fails the
handshake closed");
+
+ int port = serverSocket.getLocalPort();
+ AtomicReference<Throwable> serverError = new AtomicReference<>();
+ Thread acceptor = new Thread(() -> {
+ try (SSLSocket accepted = (SSLSocket) serverSocket.accept()) {
+ accepted.setSoTimeout(5000);
+ accepted.startHandshake();
+ } catch (Throwable t) {
+ serverError.set(t);
+ }
+ }, "tls-test-acceptor");
+ acceptor.setDaemon(true);
+ acceptor.start();
+
+ // Trust-only client context: no client keystore, so the client
cannot present a cert.
+ KeyStore ts = KeyStore.getInstance("JKS");
+ try (InputStream in = Files.newInputStream(Paths.get(testDataPath
+ "testTrustStore.jks"))) {
+ ts.load(in, "testpass".toCharArray());
+ }
+ TrustManagerFactory tmf =
TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
+ tmf.init(ts);
+ SSLContext sslContext = SSLContext.getInstance("TLSv1.2");
+ sslContext.init(null, tmf.getTrustManagers(), new SecureRandom());
+
+ try (SSLSocket client = (SSLSocket) sslContext.getSocketFactory()
+ .createSocket(InetAddress.getLoopbackAddress(), port)) {
+ client.setSoTimeout(5000);
+ // Depending on timing, JSSE surfaces the server-side
need-client-auth rejection
+ // as SSLException or as SocketException ("Connection reset")
— both prove the
+ // handshake failed closed.
+ assertThrows(IOException.class, client::startHandshake);
+ }
+
+ acceptor.join(5000);
+ assertTrue(serverError.get() instanceof IOException,
+ "Server-side handshake must fail; got: " +
serverError.get());
+ } finally {
+ serverTransport.close();
+ }
+ }
}
\ No newline at end of file
