From 1b9dbbc759740cede009b6c222b52751318c7857 Mon Sep 17 00:00:00 2001 From: Richard Zowalla Date: Mon, 13 Apr 2026 20:14:22 +0200 Subject: [PATCH] Hardening: clean up TlsTransportPlugin and surface unverified peers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 665d9f1fcb8..e260c4d0fe9 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.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 conf; private int port; @@ -71,22 +70,13 @@ public TServer getServer(TProcessor processor) throws IOException, TTransportExc 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 void process(final TProtocol inProt, final TProtocol outProt) throws TExc 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 void process(final TProtocol inProt, final TProtocol outProt) throws TExc 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();