Skip to content

Commit 9c6e9cc

Browse files
committed
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
1 parent 866acd2 commit 9c6e9cc

1 file changed

Lines changed: 7 additions & 17 deletions

File tree

storm-client/src/jvm/org/apache/storm/security/auth/tls/TlsTransportPlugin.java

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -37,18 +37,17 @@
3737
import org.apache.storm.thrift.protocol.TProtocol;
3838
import org.apache.storm.thrift.server.TServer;
3939
import org.apache.storm.thrift.server.TThreadPoolServer;
40-
import org.apache.storm.thrift.transport.TSSLTransportFactory;
4140
import org.apache.storm.thrift.transport.TServerSocket;
4241
import org.apache.storm.thrift.transport.TSocket;
4342
import org.apache.storm.thrift.transport.TTransport;
4443
import org.apache.storm.thrift.transport.TTransportException;
4544
import org.apache.storm.utils.ExtendedThreadPoolExecutor;
46-
import org.apache.storm.utils.SecurityUtils;
4745
import org.slf4j.Logger;
4846
import org.slf4j.LoggerFactory;
4947

5048
public class TlsTransportPlugin implements ITransportPlugin {
5149
private static final Logger LOG = LoggerFactory.getLogger(TlsTransportPlugin.class);
50+
private static final String ANONYMOUS_PRINCIPAL_NAME = "CN=ANONYMOUS";
5251
protected ThriftConnectionType type;
5352
protected Map<String, Object> conf;
5453
private int port;
@@ -71,22 +70,13 @@ public TServer getServer(TProcessor processor) throws IOException, TTransportExc
7170
int configuredPort = type.getPort(conf);
7271
Integer socketTimeout = type.getSocketTimeOut(conf);
7372

74-
TSSLTransportFactory.TSSLTransportParameters params = new TSSLTransportFactory.TSSLTransportParameters();
75-
if (type.getServerKeyStorePath(conf) != null && type.getServerKeyStorePassword(conf) != null) {
76-
params.setKeyStore(type.getServerKeyStorePath(conf), type.getServerKeyStorePassword(conf), null,
77-
SecurityUtils.inferKeyStoreTypeFromPath(type.getServerKeyStorePath(conf)));
78-
} else {
73+
if (type.getServerKeyStorePath(conf) == null || type.getServerKeyStorePassword(conf) == null) {
7974
throw new IllegalArgumentException("The server keystore is not configured properly");
8075
}
8176

82-
if (type.isClientAuthRequired(conf)) {
83-
if (type.getServerTrustStorePath(conf) != null && type.getServerTrustStorePassword(conf) != null) {
84-
params.setTrustStore(type.getServerTrustStorePath(conf), type.getServerTrustStorePassword(conf), null,
85-
SecurityUtils.inferKeyStoreTypeFromPath(type.getServerTrustStorePath(conf)));
86-
params.requireClientAuth(true);
87-
} else {
88-
throw new IllegalArgumentException("The server truststore is not configured properly");
89-
}
77+
if (type.isClientAuthRequired(conf)
78+
&& (type.getServerTrustStorePath(conf) == null || type.getServerTrustStorePassword(conf) == null)) {
79+
throw new IllegalArgumentException("The server truststore is not configured properly");
9080
}
9181

9282
int clientTimeout = (socketTimeout == null ? 0 : socketTimeout);
@@ -152,15 +142,15 @@ public void process(final TProtocol inProt, final TProtocol outProt) throws TExc
152142
TSocket tsocket = (TSocket) trans;
153143
SSLSocket socket = (SSLSocket) tsocket.getSocket();
154144

155-
String principalName = "CN=ANONYMOUS";
145+
String principalName = ANONYMOUS_PRINCIPAL_NAME;
156146
try {
157147
for (X509Certificate cert: socket.getSession().getPeerCertificateChain()) {
158148
Principal principal = cert.getSubjectDN();
159149
principalName = principal.getName();
160150
break;
161151
}
162152
} catch (SSLPeerUnverifiedException e) {
163-
LOG.debug("Client cert is not verified. Set principalName={}.", principalName, e);
153+
LOG.warn("Client cert is not verified. Set principalName={}.", principalName, e);
164154
}
165155
LOG.debug("principalName : {} ", principalName);
166156
ReqContext reqContext = ReqContext.context();

0 commit comments

Comments
 (0)