Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 1 commit into from
Aug 11, 2023
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions dali/base/dautils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

Expand Down
83 changes: 67 additions & 16 deletions dali/dfu/dfurun.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,24 @@ class CDFUengine: public CInterface, implements IDFUengine
}
}

StringBuffer & getFDescName(IFileDescriptor * fd, StringBuffer & result)
jakesmith marked this conversation as resolved.
Show resolved Hide resolved
{
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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
jakesmith marked this conversation as resolved.
Show resolved Hide resolved
{
//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)
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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()) {
Expand Down
16 changes: 6 additions & 10 deletions esp/smc/SMCLib/TpCommon.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks the code below breaks this rule ?
Doesn't it need to conditionally call getRelativePath ? (if isAbsolute..)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will check if isAbsolute ...

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)
Expand Down
18 changes: 18 additions & 0 deletions system/jlib/jfile.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions system/jlib/jfile.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading