Merge pull request #17528 from wangkx/h29246

HPCC-29246 Check Plane scope in DFU server when spraying/despraying Reviewed-by: Jake Smith <[email protected]> Merged-by: Gavin Halliday <[email protected]>
hpcc-systems · Aug 11, 2023 · 37e23b6 · 37e23b6
2 parents 04e4592 + e3e40de
commit 37e23b6
Show file tree

Hide file tree

Showing 5 changed files with 95 additions and 30 deletions.
diff --git a/dali/base/dautils.cpp b/dali/base/dautils.cpp
@@ -1138,13 +1138,12 @@ static void convertPosixPathToLfn(StringBuffer &str,const char *path)
 
 void CDfsLogicalFileName::setPlaneExternal(const char *plane,const char *path)
 {
-    if (isEmptyString(path))
-        return;
-    if (isPathSepChar(path[0])&&(path[0]==path[1]))
+    if (!isEmptyString(path)&&isPathSepChar(path[0])&&(path[0]==path[1]))
         throw makeStringExceptionV(-1,"Invalid path %s.",path);
     StringBuffer str(PLANE_SCOPE "::");
     str.append(plane);
-    convertPosixPathToLfn(str,path);
+    if (!isEmptyString(path))
+        convertPosixPathToLfn(str,path);
     set(str.str());
 }
 

diff --git a/dali/dfu/dfurun.cpp b/dali/dfu/dfurun.cpp
@@ -533,6 +533,24 @@ class CDFUengine: public CInterface, implements IDFUengine
         }
     }
 
+    StringBuffer & getFDescName(IFileDescriptor * fd, StringBuffer & result)
+    {
+        fd->getTraceName(result);
+        elideString(result, 255);
+        return result;
+    }
+
+    void ensureFilePermissions(const char * fileName, SecAccessFlags perm, bool write)
+    {
+        if ((write && !HASWRITEPERMISSION(perm)) || (!write && !HASREADPERMISSION(perm)))
+        {
+            if (write)
+                throw makeStringExceptionV(DFSERR_CreateAccessDenied, "Create permission denied for physical file(s): %s", fileName);
+            else
+                throw makeStringExceptionV(DFSERR_LookupAccessDenied, "Lookup permission denied for physical file(s): %s", fileName);
+        }
+    }
+
     Linked<const IPropertyTree> config;
     Owned<IScheduleEventPusher> eventpusher;
     IArrayOf<cDFUlistener> listeners;
@@ -595,16 +613,8 @@ class CDFUengine: public CInterface, implements IDFUengine
             auditflags |= DALI_LDAP_WRITE_WANTED;
 
         SecAccessFlags perm = queryDistributedFileDirectory().getFDescPermissions(fd,user,auditflags);
-        if ((write && !HASWRITEPERMISSION(perm)) || (!write && !HASREADPERMISSION(perm)))
-        {
-            StringBuffer traceName;
-            fd->getTraceName(traceName);
-            elideString(traceName, 255);
-            if (write)
-                throw makeStringExceptionV(DFSERR_CreateAccessDenied, "Create permission denied for physical file(s): %s",traceName.str());
-            else
-                throw makeStringExceptionV(DFSERR_LookupAccessDenied, "Lookup permission denied for physical file(s): %s",traceName.str());
-        }
+        StringBuffer name;
+        ensureFilePermissions(getFDescName(fd,name),perm,write);
     }
 
     void checkForeignFilePermissions(IConstDFUfileSpec *fSpec,IFileDescriptor *fd,IUserDescriptor *user,bool write)
@@ -638,14 +648,55 @@ class CDFUengine: public CInterface, implements IDFUengine
         bool checkLegacyPhysicalPerms = getGlobalConfigSP()->getPropBool("expert/@failOverToLegacyPhysicalPerms",!isContainerized());
         if (((!write&&!HASREADPERMISSION(perm)) || (write&&!HASWRITEPERMISSION(perm))) && checkLegacyPhysicalPerms)
             perm = queryDistributedFileDirectory().getFDescPermissions(fd,user,auditflags);
+        ensureFilePermissions(logicalName,perm,write);
+    }
 
+    void checkPlaneFilePermissions(IFileDescriptor *fd,IUserDescriptor *user,bool write)
+    {
+        //This function checks the scope permissions for a file or files that reside in a single directory on a single plane.
+        //The IFileDescriptor is used to discover the plane and directory.
+        //If the plane is not present, it implies that it is a bare-metal system and useDropZoneRestriction is off, and there
+        //is no matching dropzone in the environment. If this is the case, or the plane permissions are not found
+        //and @failOverToLegacyPhysicalPerms is configured, then the legacy physical file permissions will be checked.
+        unsigned auditflags = (DALI_LDAP_AUDIT_REPORT|DALI_LDAP_READ_WANTED);
         if (write)
+            auditflags |= DALI_LDAP_WRITE_WANTED;
+
+        SecAccessFlags perm;
+        bool checkLegacyPhysicalPerms = getGlobalConfigSP()->getPropBool("expert/@failOverToLegacyPhysicalPerms",!isContainerized());
+        IClusterInfo *iClusterInfo = fd->queryClusterNum(0);
+        const char *planeName = iClusterInfo->queryGroupName();
+        if (!isEmptyString(planeName))
         {
-            if (!HASWRITEPERMISSION(perm))
-                throw makeStringExceptionV(DFSERR_CreateAccessDenied,"Create permission denied for foreign file: %s",logicalName.str());
+            const char *dir = fd->queryDefaultDir();
+            if (isEmptyString(dir))
+                throw makeStringException(-1,"Empty default directory.");
+
+            Owned<IPropertyTree> dropZonePlane = getDropZonePlane(planeName);
+            if (!dropZonePlane)
+                throw makeStringExceptionV(-1,"DropZone %s not found.",planeName);
+            const char *relativePath = getRelativePath(dir,dropZonePlane->queryProp("@prefix"));
+            if (nullptr == relativePath)
+                throw makeStringExceptionV(-1,"Invalid DropZone directory %s.",dir);
+
+            perm = queryDistributedFileDirectory().getDropZoneScopePermissions(planeName,relativePath,user,auditflags);
+            if (((!write&&!HASREADPERMISSION(perm))||(write&&!HASWRITEPERMISSION(perm)))&&checkLegacyPhysicalPerms)
+                perm = queryDistributedFileDirectory().getFDescPermissions(fd,user,auditflags);
+        }
+        else
+        {
+#ifndef _CONTAINERIZED
+            Owned<IEnvironmentFactory> factory = getEnvironmentFactory(true);
+            Owned<IConstEnvironment> env = factory->openEnvironment();
+            if (env->isDropZoneRestrictionEnabled()||!checkLegacyPhysicalPerms)
+                throw makeStringException(-1,"Empty plane name.");
+            perm = queryDistributedFileDirectory().getFDescPermissions(fd,user,auditflags);
+#else
+            throw makeStringException(-1,"Unexpected empty plane name."); // should never be the case in containerized setups
+#endif
         }
-        else if (!HASREADPERMISSION(perm))
-            throw makeStringExceptionV(DFSERR_LookupAccessDenied,"Lookup permission denied for foreign file: %s",logicalName.str());
+        StringBuffer name;
+        ensureFilePermissions(getFDescName(fd,name),perm,write);
     }
 
     void monitorCycle(bool &cancelling)
@@ -1741,7 +1792,7 @@ class CDFUengine: public CInterface, implements IDFUengine
                     if (!replicating) {
                         runningconn.setown(setRunning(runningpath.str()));
                         Owned<IFileDescriptor> fdesc = source->getFileDescriptor();
-                        checkPhysicalFilePermissions(fdesc,userdesc,false);
+                        checkPlaneFilePermissions(fdesc,userdesc,false);
                         checkSourceTarget(fdesc);
                         bool needrep = options->getReplicate();
                         ClusterPartDiskMapSpec mspec;
@@ -1777,7 +1828,7 @@ class CDFUengine: public CInterface, implements IDFUengine
                 {
                     runningconn.setown(setRunning(runningpath.str()));
                     Owned<IFileDescriptor> fdesc = destination->getFileDescriptor(iskey);
-                    checkPhysicalFilePermissions(fdesc,userdesc,true);
+                    checkPlaneFilePermissions(fdesc,userdesc,true);
                     checkSourceTarget(fdesc);
                     fsys.exportFile(srcFile, fdesc, recovery, recoveryconn, filter, opttree, &feedback, &abortnotify, dfuwuid);
                     if (!abortnotify.abortRequested()) {

diff --git a/esp/smc/SMCLib/TpCommon.cpp b/esp/smc/SMCLib/TpCommon.cpp
@@ -183,21 +183,17 @@ static SecAccessFlags getDropZoneScopePermissions(IEspContext& context, const IP
         throw makeStringException(ECLWATCH_INVALID_CLUSTER_NAME, "getDropZoneScopePermissions(): DropZone path must be specified.");
 
     //If the dropZonePath is an absolute path, change it to a relative path.
-    StringBuffer s;
-    const char* prefix = dropZone->queryProp("@prefix");
-    const char* name = dropZone->queryProp("@name");
-    if (hasPrefix(dropZonePath, prefix, true))
+    if (isAbsolutePath(dropZonePath))
     {
-        const char* p = dropZonePath + strlen(prefix);
-        if (!*p || !isPathSepChar(p[0]))
-            addPathSepChar(s);
-        s.append(p);
-        dropZonePath = s.str();
+        const char* relativePath = getRelativePath(dropZonePath, dropZone->queryProp("@prefix"));
+        if (nullptr == relativePath)
+            throw makeStringExceptionV(-1, "Invalid DropZone path %s.", dropZonePath);
+        dropZonePath = relativePath;
     }
 
     Owned<IUserDescriptor> userDesc = createUserDescriptor();
     userDesc->set(context.queryUserId(), context.queryPassword(), context.querySignature());
-    return queryDistributedFileDirectory().getDropZoneScopePermissions(name, dropZonePath, userDesc);
+    return queryDistributedFileDirectory().getDropZoneScopePermissions(dropZone->queryProp("@name"), dropZonePath, userDesc);
 }
 
 extern TPWRAPPER_API SecAccessFlags getDZPathScopePermissions(IEspContext& context, const char* dropZoneName, const char* dropZonePath, const char* dropZoneHost)

diff --git a/system/jlib/jfile.cpp b/system/jlib/jfile.cpp
@@ -5706,6 +5706,24 @@ const char *splitRelativePath(const char *full,const char *basedir,StringBuffer
     return t;
 }
 
+const char *getRelativePath(const char *path,const char *leadingPath)
+{
+    size_t pathLen = strlen(path);
+    size_t leadingLen = strlen(leadingPath);
+    if ((pathLen==leadingLen-1)&&isPathSepChar(leadingPath[leadingLen-1]))
+        --leadingLen;
+    if (0 == strncmp(path,leadingPath,leadingLen))
+    {
+        const char *rel = path + leadingLen;
+        if ('\0' == *rel)
+            return rel;
+        if (isPathSepChar(*rel))
+            return rel+1;
+        return rel;
+    }
+    return nullptr;
+}
+
 const char *splitDirMultiTail(const char *multipath,StringBuffer &dir,StringBuffer &tail)
 {
     // the first directory is the significant one

diff --git a/system/jlib/jfile.hpp b/system/jlib/jfile.hpp
@@ -606,6 +606,7 @@ extern jlib_decl StringBuffer &makeAbsolutePath(StringBuffer &relpath,bool mustE
 extern jlib_decl StringBuffer &makeAbsolutePath(const char *relpath, const char *basedir, StringBuffer &out);
 extern jlib_decl StringBuffer &makePathUniversal(const char *path, StringBuffer &out);
 extern jlib_decl const char *splitRelativePath(const char *full,const char *basedir,StringBuffer &reldir); // removes basedir if matches, returns tail and relative dir
+extern jlib_decl const char *getRelativePath(const char *path, const char *leadingPath);
 extern jlib_decl const char *splitDirMultiTail(const char *multipath,StringBuffer &dir,StringBuffer &tail);
 extern jlib_decl StringBuffer &mergeDirMultiTail(const char *dir,const char *tail, StringBuffer &multipath);
 extern jlib_decl StringBuffer &removeRelativeMultiPath(const char *full,const char *dir,StringBuffer &reltail); // removes dir if matches, returns relative multipath