(storm) 01/01: Hardening: clean up TlsTransportPlugin and surface unverified peers

[rz...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

rzo1 pushed a commit to branch hardening-tls-transport-plugin
in repository https://gitbox.apache.org/repos/asf/storm.git

commit 1b9dbbc759740cede009b6c222b52751318c7857
Author: Richard Zowalla <rz...@apache.org>
AuthorDate: Mon Apr 13 20:14:22 2026 +0200

    Hardening: clean up TlsTransportPlugin and surface unverified peers
    
    Three small cleanups in TlsTransportPlugin, none of which change
    runtime behavior on the live path:
    
    - Extract "CN=ANONYMOUS" into an ANONYMOUS_PRINCIPAL_NAME constant
      so it can be grepped for when auditing authorizer rules.
    - Raise the SSLPeerUnverifiedException log from debug to warn. The
      branch is only reachable when client auth is disabled at the
      transport layer (nimbus/supervisor.thrift.tls.client.auth.required
      = false), but when it does fire it is worth seeing in production
      logs.
    - Remove the dead TSSLTransportParameters wiring in getServer().
      The params object was built with keystore/truststore settings and
      requireClientAuth(true), but never passed to
      ReloadableTsslTransportFactory.getServerSocket — the real SSL
      context is built inside the factory from the ThriftConnectionType
      and conf. Keep the eager keystore/truststore presence checks so
      misconfiguration still fails fast with a clear message, and prune
      the now-unused TSSLTransportFactory and SecurityUtils imports.
    
    C
---
 .../security/auth/tls/TlsTransportPlugin.java      | 24 +++++++---------------
 1 file changed, 7 insertions(+), 17 deletions(-)

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 665d9f1fc..e260c4d0f 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
@@ -37,18 +37,17 @@ import org.apache.storm.thrift.protocol.TBinaryProtocol;
 import org.apache.storm.thrift.protocol.TProtocol;
 import org.apache.storm.thrift.server.TServer;
 import org.apache.storm.thrift.server.TThreadPoolServer;
-import org.apache.storm.thrift.transport.TSSLTransportFactory;
 import org.apache.storm.thrift.transport.TServerSocket;
 import org.apache.storm.thrift.transport.TSocket;
 import org.apache.storm.thrift.transport.TTransport;
 import org.apache.storm.thrift.transport.TTransportException;
 import org.apache.storm.utils.ExtendedThreadPoolExecutor;
-import org.apache.storm.utils.SecurityUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 public class TlsTransportPlugin implements ITransportPlugin {
     private static final Logger LOG = LoggerFactory.getLogger(TlsTransportPlugin.class);
+    private static final String ANONYMOUS_PRINCIPAL_NAME = "CN=ANONYMOUS";
     protected ThriftConnectionType type;
     protected Map<String, Object> conf;
     private int port;
@@ -71,22 +70,13 @@ public class TlsTransportPlugin implements ITransportPlugin {
         int configuredPort = type.getPort(conf);
         Integer socketTimeout = type.getSocketTimeOut(conf);
 
-        TSSLTransportFactory.TSSLTransportParameters params = new TSSLTransportFactory.TSSLTransportParameters();
-        if (type.getServerKeyStorePath(conf) != null && type.getServerKeyStorePassword(conf) != null) {
-            params.setKeyStore(type.getServerKeyStorePath(conf), type.getServerKeyStorePassword(conf), null,
-                    SecurityUtils.inferKeyStoreTypeFromPath(type.getServerKeyStorePath(conf)));
-        } else {
+        if (type.getServerKeyStorePath(conf) == null || type.getServerKeyStorePassword(conf) == null) {
             throw new IllegalArgumentException("The server keystore is not configured properly");
         }
 
-        if (type.isClientAuthRequired(conf)) {
-            if (type.getServerTrustStorePath(conf) != null && type.getServerTrustStorePassword(conf) != null) {
-                params.setTrustStore(type.getServerTrustStorePath(conf), type.getServerTrustStorePassword(conf), null,
-                        SecurityUtils.inferKeyStoreTypeFromPath(type.getServerTrustStorePath(conf)));
-                params.requireClientAuth(true);
-            } else {
-                throw new IllegalArgumentException("The server truststore is not configured properly");
-            }
+        if (type.isClientAuthRequired(conf)
+                && (type.getServerTrustStorePath(conf) == null || type.getServerTrustStorePassword(conf) == null)) {
+            throw new IllegalArgumentException("The server truststore is not configured properly");
         }
 
         int clientTimeout = (socketTimeout == null ? 0 : socketTimeout);
@@ -152,7 +142,7 @@ public class TlsTransportPlugin implements ITransportPlugin {
             TSocket tsocket = (TSocket) trans;
             SSLSocket socket = (SSLSocket) tsocket.getSocket();
 
-            String principalName = "CN=ANONYMOUS";
+            String principalName = ANONYMOUS_PRINCIPAL_NAME;
             try {
                 for (X509Certificate cert: socket.getSession().getPeerCertificateChain()) {
                     Principal principal = cert.getSubjectDN();
@@ -160,7 +150,7 @@ public class TlsTransportPlugin implements ITransportPlugin {
                     break;
                 }
             } catch (SSLPeerUnverifiedException e) {
-                LOG.debug("Client cert is not verified. Set principalName={}.", principalName, e);
+                LOG.warn("Client cert is not verified. Set principalName={}.", principalName, e);
             }
             LOG.debug("principalName : {} ", principalName);
             ReqContext reqContext = ReqContext.context();