HPCC-29246 Check Plane scope in DFU server when spraying/despraying

1. Merge the getPlaneFilePermissions() into the checkPlaneFilePermissions(). 2. Add option failOverToLegacyPhysicalPerms. 3. If no plane name, check the DropZoneRestrictionEnabled. 4. Check read permission only for read action 5. Change the ensureFilePermissions 6. Call the ensureFilePermissions in checkForeignFilePermissions 7. Check isAbsolute before calling getRelativePath 8. Allow empty relative path in setPlaneExternal() Signed-off-by: wangkx <[email protected]>
hpcc-systems · Aug 3, 2023 · 85e1288 · 85e1288
1 parent 9ad3eda
commit 85e1288
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