HPCC-30259 Use secrets for dafilesrv client connections

dali/base/dafdesc.cpp

@@ -1040,6 +1040,7 @@ class CFileDescriptor:  public CFileDescriptorBase, implements ISuperFileDescrip
 
     SocketEndpointArray *pending;   // for constructing cluster group
     Owned<IStoragePlane> remoteStoragePlane;
+    std::vector<std::string> dafileSrvEndpoints;
     bool setupdone;
     byte version;
 
@@ -1398,6 +1399,32 @@ class CFileDescriptor:  public CFileDescriptorBase, implements ISuperFileDescrip
         }
     }
 
+    // mapDafileSrvSecrets is a CFileDescriptor is created if it is associated with a remoteStoragePlane.
+    // Identify the target dafilesrv location urls a secret based connections in the dafilesrv hook
+    // NB: the expectation is that they'll only be 1 target service dafilesrv URL
+    // These will remain associated in the hook, until this CFileDescriptor object is destroyed, and removeMappedDafileSrvSecrets is called.
+    void mapDafileSrvSecrets(IClusterInfo &cluster)
+    {
+        Owned<INodeIterator> groupIter = cluster.queryGroup()->getIterator();
+
+        ForEach(*groupIter)
+        {
+            INode &node = groupIter->query();
+            StringBuffer endpointString;
+            node.endpoint().getEndpointHostText(endpointString);
+            auto it = std::find(dafileSrvEndpoints.begin(), dafileSrvEndpoints.end(), endpointString.str());
+            if (it == dafileSrvEndpoints.end())
+                dafileSrvEndpoints.push_back(endpointString.str());
+        }
+        for (auto &dafileSrvEp: dafileSrvEndpoints)
+            queryDaFileSrvHook()->addSecretUrl(dafileSrvEp.c_str());
+    }
+    void removeMappedDafileSrvSecrets()
+    {
+        for (auto &dafileSrvEp: dafileSrvEndpoints)
+            queryDaFileSrvHook()->removeSecretUrl(dafileSrvEp.c_str());
+    }
+
 public:
     IMPLEMENT_IINTERFACE;
 
@@ -1482,6 +1509,8 @@ class CFileDescriptor:  public CFileDescriptorBase, implements ISuperFileDescrip
             {
                 assertex(1 == clusters.ordinality()); // only one cluster per logical remote file supported/will have resolved to 1
                 remoteStoragePlane.setown(createStoragePlane(remoteStoragePlaneMeta));
+                if (attr->getPropBool("@_remoteSecure"))
+                    mapDafileSrvSecrets(clusters.item(0));
             }
         }
         else
@@ -1613,6 +1642,8 @@ class CFileDescriptor:  public CFileDescriptorBase, implements ISuperFileDescrip
             assertex(1 == clusters.ordinality()); // only one cluster per logical remote file supported/will have resolved to 1
             remoteStoragePlane.setown(createStoragePlane(remoteStoragePlaneMeta));
             clusters.item(0).applyPlane(remoteStoragePlane);
+            if (attr->getPropBool("@_remoteSecure"))
+                mapDafileSrvSecrets(clusters.item(0));
         }
     }
 
@@ -1707,6 +1738,7 @@ class CFileDescriptor:  public CFileDescriptorBase, implements ISuperFileDescrip
 
     virtual ~CFileDescriptor()
     {
+        removeMappedDafileSrvSecrets();
         closePending();             // not sure strictly needed
         ForEachItemInRev(p, parts)
             delpart(p);

esp/clients/ws_dfsclient/ws_dfsclient.cpp

@@ -321,9 +321,9 @@ class CServiceDistributedFile : public CServiceDistributedFileBase<IDistributedF
         IPropertyTree *file = dfsFile->queryFileMeta()->queryPropTree("File");
 
         const char *remoteName = dfsFile->queryRemoteName(); // NB: null if local
-        IPropertyTree *dafileSrvRemoteFilePlane = nullptr;
         if (!isEmptyString(remoteName))
         {
+            IPropertyTree *dafileSrvRemoteFilePlane = nullptr;
             Owned<IPropertyTree> remoteStorage = getRemoteStorage(remoteName);
             if (!remoteStorage)
                 throw makeStringExceptionV(0, "Remote storage '%s' not found", remoteName);
@@ -394,10 +394,19 @@ class CServiceDistributedFile : public CServiceDistributedFileBase<IDistributedF
                     file->setProp("@directory", newPath.str());
                 }
             }
+            if (dafileSrvRemoteFilePlane)
+            {
+                file->setPropTree("Attr/_remoteStoragePlane", createPTreeFromIPT(dafileSrvRemoteFilePlane));
+                if (remoteStorage->hasProp("@secret"))
+                {
+                    // if remote storage service is secure, dafilesrv connections must be also.
+                    // this flag is used by consumers of this IFleDescriptor to tell whether they need to make
+                    // secure secret based connections to the dafilesrv's
+                    file->setPropBool("Attr/@_remoteSecure", true);
+                }
+            }
         }
         AccessMode accessMode = static_cast<AccessMode>(dfsFile->queryCommonMeta()->getPropInt("@accessMode"));
-        if (dafileSrvRemoteFilePlane)
-            file->setPropTree("Attr/_remoteStoragePlane", createPTreeFromIPT(dafileSrvRemoteFilePlane));
         fileDesc.setown(deserializeFileDescriptorTree(file));
         fileDesc->setTraceName(logicalName);
         // NB: the accessMode is being defined by the client call, and has been stored in the IDFSFile common meta

fs/dafsclient/rmtclient.cpp

@@ -667,7 +667,7 @@ void clientAddSocketToCache(SocketEndpoint &ep,ISocket *socket)
 
 //---------------------------------------------------------------------------
 
-void CRemoteBase::connectSocket(SocketEndpoint &ep, unsigned connectTimeoutMs, unsigned connectRetries)
+void CRemoteBase::connectSocket(SocketEndpoint &ep, unsigned connectTimeoutMs, unsigned connectRetries, bool secure)
 {
     if (!connectTimeoutMs)
         connectTimeoutMs = dafsConnectTimeoutMs;
@@ -696,10 +696,7 @@ void CRemoteBase::connectSocket(SocketEndpoint &ep, unsigned connectTimeoutMs, u
         if (TF_TRACE_CLIENT_CONN)
         {
             ep.getEndpointHostText(eps);
-            if (ep.port == securitySettings.queryDaFileSrvSSLPort())
-                PROGLOG("Connecting SECURE to %s", eps.str());
-            else
-                PROGLOG("Connecting to %s", eps.str());
+            PROGLOG("Connecting %sto %s", secure?"SECURE ":"", eps.str());
             //PrintStackReport();
         }
         bool ok = true;
@@ -714,13 +711,36 @@ void CRemoteBase::connectSocket(SocketEndpoint &ep, unsigned connectTimeoutMs, u
             }
             else
                 socket.setown(ISocket::connect(ep));
-            if (ep.port == securitySettings.queryDaFileSrvSSLPort())
+            if (secure)
             {
 #ifdef _USE_OPENSSL
                 Owned<ISecureSocket> ssock;
                 try
                 {
-                    ssock.setown(createSecureSocket(socket.getClear(), nullptr));
+                    // instead of creating CRemoteFile/CRemoteBase with a secret, it could instead lookup, and create the ISecureSocketContext and pass that
+                    // into the ctor. That would avoid having to look the secret up, extract the certs and recreate the context on each connection.
+                    // The context would be 'owned' by the hook, and would expire when the mappings are removed (when removeMappedDafileSrvSecrets is called).
+                    if (storageSecret)
+                    {
+                        Owned<IPropertyTree> secretPTree = getSecret("storage", storageSecret);
+                        if (!secretPTree)
+                            throw makeStringExceptionV(-1, "secret %s.%s not found", "storage", storageSecret.str());
+
+                        StringBuffer certSecretBuf;
+                        getSecretKeyValue(certSecretBuf, secretPTree, "tls.crt");
+
+                        StringBuffer privKeySecretBuf;
+                        getSecretKeyValue(privKeySecretBuf, secretPTree, "tls.key");
+
+                        Owned<ISecureSocketContext> secureContext = createSecureSocketContextEx(certSecretBuf, privKeySecretBuf, nullptr, ClientSocket);
+                        int loglevel = SSLogNormal;
+#ifdef _DEBUG
+                        loglevel = SSLogMax;
+#endif
+                        ssock.setown(secureContext->createSecureSocket(socket.getClear(), loglevel));
+                    }
+                    else
+                        ssock.setown(createSecureSocket(socket.getClear(), nullptr));
                     int status = ssock->secure_connect();
                     if (status < 0)
                         throw createDafsException(DAFSERR_connection_failed, "Failure to establish secure connection");
@@ -787,10 +807,7 @@ void CRemoteBase::connectSocket(SocketEndpoint &ep, unsigned connectTimeoutMs, u
         if (!timeExpired)
         {
             Sleep(sleeptime);       // prevent multiple retries beating
-            if (ep.port == securitySettings.queryDaFileSrvSSLPort())
-                PROGLOG("Retrying SECURE connect");
-            else
-                PROGLOG("Retrying connect");
+            PROGLOG("Retrying %sconnect", secure?"SECURE ":"");
         }
     }
 
@@ -879,35 +896,41 @@ void CRemoteBase::sendRemoteCommand(MemoryBuffer & src, MemoryBuffer & reply, bo
 
         if (!socket)
         {
-            bool doConnect = true;
-            if (connectMethod == SSLFirst || connectMethod == UnsecureFirst)
+            if (storageSecret)
+                connectSocket(tep, 0, INFINITE, true);
+            else
             {
-                // MCK - could maintain a list of 100 or so previous endpoints and if connection failed
-                // then mark port down for a delay (like 15 min above) to avoid having to try every time ...
-                try
-                {
-                    connectSocket(tep, 5000, 0);
-                    doConnect = false;
-                }
-                catch (IDAFS_Exception *e)
+                bool doConnect = true;
+                if (connectMethod == SSLFirst || connectMethod == UnsecureFirst)
                 {
-                    if (e->errorCode() == DAFSERR_connection_failed)
+                    bool secure = tep.port == securitySettings.queryDaFileSrvSSLPort();
+                    // MCK - could maintain a list of 100 or so previous endpoints and if connection failed
+                    // then mark port down for a delay (like 15 min above) to avoid having to try every time ...
+                    try
                     {
-                        unsigned prevPort = tep.port;
-                        if (prevPort == securitySettings.queryDaFileSrvSSLPort())
-                            tep.port = securitySettings.queryDaFileSrvPort();
+                        connectSocket(tep, 5000, 0);
+                        doConnect = false;
+                    }
+                    catch (IDAFS_Exception *e)
+                    {
+                        if (e->errorCode() == DAFSERR_connection_failed)
+                        {
+                            unsigned prevPort = tep.port;
+                            if (secure)
+                                tep.port = securitySettings.queryDaFileSrvPort();
+                            else
+                                tep.port = securitySettings.queryDaFileSrvSSLPort();
+                            WARNLOG("Connect failed on port %d, retrying on port %d", prevPort, tep.port);
+                            doConnect = true;
+                            e->Release();
+                        }
                         else
-                            tep.port = securitySettings.queryDaFileSrvSSLPort();
-                        WARNLOG("Connect failed on port %d, retrying on port %d", prevPort, tep.port);
-                        doConnect = true;
-                        e->Release();
+                            throw e;
                     }
-                    else
-                        throw e;
                 }
+                if (doConnect)
+                    connectSocket(tep, 0, INFINITE, tep.port == securitySettings.queryDaFileSrvSSLPort());
             }
-            if (doConnect)
-                connectSocket(tep);
         }
     }
 
@@ -979,6 +1002,11 @@ CRemoteBase::CRemoteBase(const SocketEndpoint &_ep, DAFSConnectCfg _connectMetho
     connectMethod = _connectMethod;
 }
 
+CRemoteBase::CRemoteBase(const SocketEndpoint &_ep, const char *_storageSecret, const char * _filename)
+    : ep(_ep), storageSecret(_storageSecret), filename(_filename)
+{
+}
+
 void CRemoteBase::disconnect()
 {
     CriticalBlock block(crit);

fs/dafsclient/rmtclient_impl.hpp

@@ -153,22 +153,25 @@ class CRemoteBase : public CSimpleInterfaceOf<IDaFsConnection>
     static SocketEndpoint   lastfailep;
     static unsigned         lastfailtime;
     static CriticalSection  lastFailEpCrit;
-    DAFSConnectCfg          connectMethod;
+    DAFSConnectCfg          connectMethod = SSLNone;
 
-    void connectSocket(SocketEndpoint &ep, unsigned connectTimeoutMs=0, unsigned connectRetries=INFINITE);
+    void connectSocket(SocketEndpoint &ep, unsigned connectTimeoutMs=0, unsigned connectRetries=INFINITE, bool secure=false);
     void killSocket(SocketEndpoint &tep);
 
 protected: friend class CRemoteFileIO;
 
     StringAttr          filename;
     CriticalSection     crit;
     SocketEndpoint      ep;
+    StringBuffer        storageSecret;
 
     void sendRemoteCommand(MemoryBuffer & src, MemoryBuffer & reply, bool retry=true, bool lengthy=false, bool handleErrCode=true);
     void sendRemoteCommand(MemoryBuffer & src, bool retry);
 public:
     CRemoteBase(const SocketEndpoint &_ep, const char * _filename);
     CRemoteBase(const SocketEndpoint &_ep, DAFSConnectCfg _connectMethod, const char * _filename);
+    CRemoteBase(const SocketEndpoint &_ep, const char *_storageSecret, const char * _filename);
+
     void disconnect();
     const char *queryLocalName()
     {

fs/dafsclient/rmtfile.cpp

@@ -222,10 +222,13 @@ CDaliServixFilter *createDaliServixFilter(IPropertyTree &filterProps)
     return filter;
 }
 
+
 class CDaliServixIntercept: public CInterface, implements IDaFileSrvHook
 {
     CIArrayOf<CDaliServixFilter> filters;
     StringAttr forceRemotePattern;
+    CriticalSection secretCrit;
+    std::unordered_map<std::string, unsigned> urls;
 
     void addFilter(CDaliServixFilter *filter)
     {
@@ -244,6 +247,13 @@ class CDaliServixIntercept: public CInterface, implements IDaFileSrvHook
     virtual IFile * createIFile(const RemoteFilename & filename)
     {
         SocketEndpoint ep = filename.queryEndpoint();
+
+        // check 1st if this is a secret based url
+        StringBuffer storageSecret;
+        getSecretBased(storageSecret, filename);
+        if (storageSecret.length())
+            return createDaliServixFile(filename, storageSecret);
+
         bool noport = (ep.port==0);
         setDafsEndpointPort(ep);
         if (!filename.isLocal()||(ep.port!=DAFILESRV_PORT && ep.port!=SECURE_DAFILESRV_PORT)) // assume standard port is running on local machine
@@ -340,6 +350,35 @@ class CDaliServixIntercept: public CInterface, implements IDaFileSrvHook
     {
         filters.kill();
     }
+    virtual StringBuffer &getSecretBased(StringBuffer &storageSecret, const RemoteFilename & filename) override
+    {
+        const SocketEndpoint &ep = filename.queryEndpoint();
+
+        StringBuffer endpointStr;
+        ep.getEndpointHostText(endpointStr);
+
+        CriticalBlock b(secretCrit);
+        auto it = urls.find(endpointStr.str());
+        if (it != urls.end())
+        {
+            VStringBuffer secureUrl("https://%s", endpointStr.str());
+            generateDynamicUrlSecretName(storageSecret, secureUrl, nullptr);
+        }
+        return storageSecret;
+    }
+    virtual void addSecretUrl(const char *url) override
+    {
+        CriticalBlock b(secretCrit);
+        urls[url]++; // NB: if doesn't exist std::unordered_map will insert with default values, i.e. with an initial unsigned value of 0
+    }
+    virtual void removeSecretUrl(const char *url) override
+    {
+        CriticalBlock b(secretCrit);
+        auto it = urls.find(url);
+        assertex(it != urls.end());
+        if (--it->second == 0)
+            urls.erase(it);
+    }
 } *DaliServixIntercept = NULL;
 
 
@@ -663,11 +702,8 @@ class CRemoteFile : public CRemoteBase, implements IFile
     StringAttr remotefilename;
     unsigned flags;
     bool isShareSet;
-public:
-    IMPLEMENT_IINTERFACE_O_USING(CRemoteBase);
 
-    CRemoteFile(const SocketEndpoint &_ep, const char * _filename)
-        : CRemoteBase(_ep, _filename)
+    void commonInit()
     {
         flags = ((unsigned)IFSHread)|((S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH)<<16);
         isShareSet = false;
@@ -677,6 +713,20 @@ class CRemoteFile : public CRemoteBase, implements IFile
             filename.set(winDriveFilename);
         }
     }
+public:
+    IMPLEMENT_IINTERFACE_O_USING(CRemoteBase);
+
+    CRemoteFile(const SocketEndpoint &_ep, const char * _filename)
+        : CRemoteBase(_ep, _filename)
+    {
+        commonInit();
+    }
+
+    CRemoteFile(const SocketEndpoint &ep, const char *filename, const char *storageSecret)
+        : CRemoteBase(ep, storageSecret, filename)
+    {
+        commonInit();
+    }
 
     bool exists()
     {
@@ -1751,6 +1801,13 @@ IFile *createDaliServixFile(const RemoteFilename & file)
     return createRemoteFile(ep, path.str());
 }
 
+IFile *createDaliServixFile(const RemoteFilename & file, const char *storageSecret)
+{
+    StringBuffer path;
+    file.getLocalPath(path);
+    return new CRemoteFile(file.queryEndpoint(), path.str(), storageSecret);
+}
+
 void clientDisconnectRemoteIoOnExit(IFileIO *fileio, bool set)
 {
     CRemoteFileIO *cfileio = QUERYINTERFACE(fileio,CRemoteFileIO);
@@ -2169,6 +2226,11 @@ class CRemoteFilteredFileIOBase : public CRemoteBase, implements IRemoteFileIO
     CRemoteFilteredFileIOBase(SocketEndpoint &ep, const char *filename, IOutputMetaData *actual, IOutputMetaData *projected, const RowFilter &fieldFilters, unsigned __int64 chooseN)
         : CRemoteBase(ep, filename)
     {
+        // populate secret if there is one
+        RemoteFilename rfn;
+        rfn.setPath(ep, filename);
+        queryDaFileSrvHook()->getSecretBased(storageSecret, rfn);
+
         // NB: inputGrouped == outputGrouped for now, but may want output to be ungrouped
 
         openRequest();