Changes following own code review

Signed-off-by: Gavin Halliday <[email protected]>

roxie/ccd/ccdmain.cpp

@@ -46,6 +46,10 @@
 #include "hpccconfig.hpp"
 #include "udpsha.hpp"
 
+#ifdef _USE_OPENSSL
+#include "securesocket.hpp"
+#endif
+
 #if defined (__linux__)
 #include <sys/syscall.h>
 #include "ioprio.h"
@@ -1450,7 +1454,7 @@ int CCD_API roxie_main(int argc, const char *argv[], const char * defaultYaml)
             else
             {
                 Owned<IHpccProtocolPlugin> protocolPlugin = loadHpccProtocolPlugin(protocolCtx, NULL);
-                Owned<IHpccProtocolListener> roxieServer = protocolPlugin->createListener("runOnce", createRoxieProtocolMsgSink(myNode.getIpAddress(), 0, 1, false), 0, 0, nullptr, nullptr, nullptr, nullptr, nullptr);
+                Owned<IHpccProtocolListener> roxieServer = protocolPlugin->createListener("runOnce", createRoxieProtocolMsgSink(myNode.getIpAddress(), 0, 1, false), 0, 0, nullptr, nullptr);
                 try
                 {
                     const char *format = topology->queryProp("@format");
@@ -1513,18 +1517,20 @@ int CCD_API roxie_main(int argc, const char *argv[], const char * defaultYaml)
                         StringBuffer certFileName;
                         StringBuffer keyFileName;
                         StringBuffer passPhraseStr;
-                        Owned<const IPropertyTree> tlsConfig;
+                        Owned<const ISyncedPropertyTree> tlsConfig;
                         if (serviceTLS)
                         {
                             protocol = "ssl";
 #ifdef _USE_OPENSSL
                             if (isContainerized())
                             {
+                                //MORE: Why is this containerized only?  Should it check for this in both, and fall back to the
+                                //old config if none found and non-containerized?
                                 const char *certIssuer = roxieFarm.queryProp("@issuer");
                                 if (isEmptyString(certIssuer))
                                     certIssuer = roxieFarm.getPropBool("@public", true) ? "public" : "local";
-                                tlsConfig.setown(getIssuerTlsServerConfigWithTrustedPeers(certIssuer, roxieFarm.queryProp("trusted_peers")));
-                                if (!tlsConfig)
+                                tlsConfig.setown(getIssuerTlsSyncedConfig(certIssuer, roxieFarm.queryProp("trusted_peers"), false));
+                                if (!tlsConfig || !tlsConfig->isValid())
                                     throw MakeStringException(ROXIE_FILE_ERROR, "TLS secret for issuer %s not found", certIssuer);
                                 DBGLOG("Roxie service, port(%d) TLS issuer (%s)", port, certIssuer);
                             }
@@ -1555,6 +1561,9 @@ int CCD_API roxie_main(int argc, const char *argv[], const char * defaultYaml)
 
                                 if (!checkFileExists(keyFileName.str()))
                                     throw MakeStringException(ROXIE_FILE_ERROR, "Roxie SSL Farm Listener on port %d missing privateKeyFile (%s)", port, keyFileName.str());
+
+                                Owned<IPropertyTree> staticConfig = createSecureSocketConfig(certFileName, keyFileName, passPhraseStr);
+                                tlsConfig.setown(createSyncedPropertyTree(staticConfig));
                             }
 
 #else
@@ -1566,7 +1575,7 @@ int CCD_API roxie_main(int argc, const char *argv[], const char * defaultYaml)
                         const char *config  = roxieFarm.queryProp("@config");
                         // NB: leaks - until we fix bug in ensureProtocolPlugin() whereby some paths return a linked object and others do not
                         IHpccProtocolPlugin *protocolPlugin = ensureProtocolPlugin(*protocolCtx, soname);
-                        roxieServer.setown(protocolPlugin->createListener(protocol ? protocol : "native", createRoxieProtocolMsgSink(ip, port, numThreads, suspended), port, listenQueue, config, tlsConfig, certFileName, keyFileName, passPhraseStr));
+                        roxieServer.setown(protocolPlugin->createListener(protocol ? protocol : "native", createRoxieProtocolMsgSink(ip, port, numThreads, suspended), port, listenQueue, config, tlsConfig));
                     }
                     else
                         roxieServer.setown(createRoxieWorkUnitListener(numThreads, suspended));

roxie/ccd/ccdprotocol.cpp

@@ -29,7 +29,7 @@
 
 //================================================================================================================================
 
-IHpccProtocolListener *createProtocolListener(const char *protocol, IHpccProtocolMsgSink *sink, unsigned port, unsigned listenQueue, const IPropertyTree *tlsConfig, const char *certFile, const char *keyFile, const char *passPhrase);
+IHpccProtocolListener *createProtocolListener(const char *protocol, IHpccProtocolMsgSink *sink, unsigned port, unsigned listenQueue, const ISyncedPropertyTree *tlsConfig);
 
 class CHpccProtocolPlugin : implements IHpccProtocolPlugin, public CInterface
 {
@@ -60,9 +60,9 @@ class CHpccProtocolPlugin : implements IHpccProtocolPlugin, public CInterface
         maxHttpConnectionRequests = ctx.ctxGetPropInt("@maxHttpConnectionRequests", 0);
         maxHttpKeepAliveWait = ctx.ctxGetPropInt("@maxHttpKeepAliveWait", 5000); // In milliseconds
     }
-    IHpccProtocolListener *createListener(const char *protocol, IHpccProtocolMsgSink *sink, unsigned port, unsigned listenQueue, const char *config, const IPropertyTree *tlsConfig, const char *certFile, const char *keyFile, const char *passPhrase)
+    IHpccProtocolListener *createListener(const char *protocol, IHpccProtocolMsgSink *sink, unsigned port, unsigned listenQueue, const char *config, const ISyncedPropertyTree *tlsConfig)
     {
-        return createProtocolListener(protocol, sink, port, listenQueue, tlsConfig, certFile, keyFile, passPhrase);
+        return createProtocolListener(protocol, sink, port, listenQueue, tlsConfig);
     }
 public:
     StringArray targetNames;
@@ -224,35 +224,23 @@ class ProtocolSocketListener : public ProtocolListener
     Owned<ISocket> socket;
     SocketEndpoint ep;
     StringAttr protocol;
-    StringAttr certFile;
-    StringAttr keyFile;
-    StringAttr passPhrase;
     Owned<ISecureSocketContext> secureContext;
     bool isSSL = false;
 
 public:
-    ProtocolSocketListener(IHpccProtocolMsgSink *_sink, unsigned _port, unsigned _listenQueue, const char *_protocol, const IPropertyTree *_tlsConfig, const char *_certFile, const char *_keyFile, const char *_passPhrase)
+    ProtocolSocketListener(IHpccProtocolMsgSink *_sink, unsigned _port, unsigned _listenQueue, const char *_protocol, const ISyncedPropertyTree *_tlsConfig)
       : ProtocolListener(_sink)
     {
         port = _port;
         listenQueue = _listenQueue;
         ep.set(port, queryHostIP());
         protocol.set(_protocol);
-        certFile.set(_certFile);
-        keyFile.set(_keyFile);
-        passPhrase.set(_passPhrase);
         isSSL = streq(protocol.str(), "ssl");
 
 #ifdef _USE_OPENSSL
         if (isSSL)
         {
-            Owned<IPropertyTree> config;
-            if (!_tlsConfig)
-            {
-                config.setown(createSecureSocketConfig(certFile.get(), keyFile.get(), passPhrase.get()));
-                _tlsConfig = config;
-            }
-            secureContext.setown(createSecureSocketContextEx2(_tlsConfig, ServerSocket));
+            secureContext.setown(createSecureSocketContextSynced(_tlsConfig, ServerSocket));
         }
 #endif
     }
@@ -2215,16 +2203,17 @@ void ProtocolSocketListener::runOnce(const char *query)
     p->runOnce(query);
 }
 
-IHpccProtocolListener *createProtocolListener(const char *protocol, IHpccProtocolMsgSink *sink, unsigned port, unsigned listenQueue, const IPropertyTree *tlsConfig, const char *certFile, const char *keyFile, const char *passPhrase)
+IHpccProtocolListener *createProtocolListener(const char *protocol, IHpccProtocolMsgSink *sink, unsigned port, unsigned listenQueue, const ISyncedPropertyTree *tlsConfig)
 {
     if (traceLevel)
     {
         const char *certIssuer = "none";
-        if (tlsConfig && tlsConfig->hasProp("@issuer"))
-            certIssuer = tlsConfig->queryProp("@issuer");
+        Owned<const IPropertyTree> tlsInfo = tlsConfig->getTree();
+        if (tlsInfo && tlsInfo->hasProp("@issuer"))
+            certIssuer = tlsInfo->queryProp("@issuer");
         DBGLOG("Creating Roxie socket listener, protocol %s, issuer=%s, pool size %d, listen queue %d%s", protocol, certIssuer, sink->getPoolSize(), listenQueue, sink->getIsSuspended() ? " SUSPENDED":"");
     }
-    return new ProtocolSocketListener(sink, port, listenQueue, protocol, tlsConfig, certFile, keyFile, passPhrase);
+    return new ProtocolSocketListener(sink, port, listenQueue, protocol, tlsConfig);
 }
 
 extern IHpccProtocolPlugin *loadHpccProtocolPlugin(IHpccProtocolPluginContext *ctx, IActiveQueryLimiterFactory *_limiterFactory)

roxie/ccd/hpccprotocol.hpp

@@ -134,7 +134,7 @@ interface IActiveQueryLimiterFactory : extends IInterface
 
 interface IHpccProtocolPlugin : extends IInterface
 {
-    virtual IHpccProtocolListener *createListener(const char *protocol, IHpccProtocolMsgSink *sink, unsigned port, unsigned listenQueue, const char *config, const IPropertyTree *tlsConfig, const char *certFile, const char *keyFile, const char *passPhrase)=0;
+    virtual IHpccProtocolListener *createListener(const char *protocol, IHpccProtocolMsgSink *sink, unsigned port, unsigned listenQueue, const char *config, const ISyncedPropertyTree *tlsConfig)=0;
 };
 
 extern IHpccProtocolPlugin *loadHpccProtocolPlugin(IHpccProtocolPluginContext *ctx, IActiveQueryLimiterFactory *limiterFactory);

system/jlib/jptree.hpp

@@ -443,7 +443,7 @@ interface ISyncedPropertyTree : extends IInterface
     //Return a sequence number which changes whenever the secret actually changes - so that a caller can determine
     //whether it needs to reload the certificates.
     virtual unsigned getVersion() const = 0;
-    //An indication that the property tree may be out of date because it couldn'tt be resynchronized.
+    //An indication that the property tree may be out of date because it couldn't be resynchronized.
     virtual bool isStale() const = 0;
     virtual bool isValid() const = 0; // Was an entry found for this secret?
 };

system/jlib/jsecrets.cpp

@@ -1348,7 +1348,7 @@ class CSyncedCertificateBase : public CInterfaceOf<ISyncedPropertyTree>
     virtual unsigned getVersion() const override final
     {
         //If information that is combined with the secret (e.g. trusted peers) can also change dynamically this would
-        //need to be a separate hash calculated from the condig tree
+        //need to be a separate hash calculated from the config tree
         return secretHash;
     }
 
@@ -1540,14 +1540,6 @@ bool hasIssuerTlsConfig(const char *issuer)
     return match && match->isValid();
 }
 
-const IPropertyTree *getIssuerTlsServerConfigWithTrustedPeers(const char *issuer, const char *trusted_peers)
-{
-    Owned<const ISyncedPropertyTree> match = getIssuerTlsSyncedConfig(issuer, trusted_peers, false);
-    if (!match)
-        return nullptr;
-    return match->getTree();
-}
-
 enum UseMTLS { UNINIT, DISABLED, ENABLED };
 static UseMTLS useMtls = UNINIT;
 

system/jlib/jsecrets.hpp

@@ -44,7 +44,6 @@ extern jlib_decl const ISyncedPropertyTree * getIssuerTlsSyncedConfig(const char
 inline const ISyncedPropertyTree * getIssuerTlsSyncedConfig(const char * issuer) { return getIssuerTlsSyncedConfig(issuer, nullptr, false); }
 
 extern jlib_decl bool hasIssuerTlsConfig(const char *issuer);
-extern jlib_decl const IPropertyTree *getIssuerTlsServerConfigWithTrustedPeers(const char *issuer, const char *trusted_peers);
 
 extern jlib_decl ISyncedPropertyTree * createIssuerTlsConfig(const char * issuer, const char * optTrustedPeers, bool isClientConnection, bool acceptSelfSigned, bool addCACert, bool disableMTLS);
 extern jlib_decl ISyncedPropertyTree * createStorageTlsConfig(const char * secretName, bool addCACert);

system/security/securesocket/securesocket.cpp

@@ -2014,7 +2014,7 @@ SECURESOCKET_API ISecureSocketContext* createSecureSocketContextSecretSrv(const
         throw makeStringException(-100, "TLS secure communication requested but not configured");
 
     Owned<const ISyncedPropertyTree> info = getIssuerTlsSyncedConfig(issuer);
-    if (!!info->isValid())
+    if (!info->isValid())
         throw makeStringException(-101, "TLS secure communication requested but not configured (2)");
 
     return createSecureSocketContextSynced(info, ServerSocket);
@@ -2037,7 +2037,7 @@ IPropertyTree * createSecureSocketConfig(const char* certFileOrBuf, const char*
 
 SECURESOCKET_API ISecureSocketContext* createSecureSocketContextSSF(ISmartSocketFactory* ssf)
 {
-    if (ssf == nullptr || !ssf->queryTlsConfig())
+    if (ssf == nullptr)
         return createSecureSocketContext(ClientSocket);
 
     return createSecureSocketContextSynced(ssf->queryTlsConfig(), ClientSocket);