Skip to content

Commit

Permalink
HPCC-30037 WIP Check legacy DZ physical permission in ESP services
Browse files Browse the repository at this point in the history 
1. In checkPlaneFilePermissions() (dfurun.cpp), check legacy
physical permission only if the plane name is set.
2. Add getLegacyDZPhysicalPerms() in ESP SMCLib.
3. Call getLegacyDZPhysicalPerms() in validateDropZoneAccess()
which is used by ESP WsFileIO service.
4. In ESP FileSpray service, check legacy physical permission
if plane exists.

TODO: rebase after PR 17676 is merged

Signed-off-by: wangkx <[email protected]>
  • Loading branch information
wangkx committed Aug 18, 2023
1 parent 5481a5c commit bdb9f0b
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 35 deletions.
11 changes: 6 additions & 5 deletions dali/dfu/dfurun.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -668,7 +668,6 @@ class CDFUengine: public CInterface, implements IDFUengine
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))
Expand All @@ -685,17 +684,19 @@ class CDFUengine: public CInterface, implements IDFUengine
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);
if (((!write&&!HASREADPERMISSION(perm))||(write&&!HASWRITEPERMISSION(perm))))
{
if (getGlobalConfigSP()->getPropBool("expert/@failOverToLegacyPhysicalPerms",!isContainerized()))
perm = queryDistributedFileDirectory().getFDescPermissions(fd,user,auditflags);
}
}
else
{
#ifndef _CONTAINERIZED
Owned<IEnvironmentFactory> factory = getEnvironmentFactory(true);
Owned<IConstEnvironment> env = factory->openEnvironment();
if (env->isDropZoneRestrictionEnabled()||!checkLegacyPhysicalPerms)
if (env->isDropZoneRestrictionEnabled())
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
Expand Down
28 changes: 15 additions & 13 deletions esp/services/ws_fs/ws_fsBinding.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,19 @@ void CFileSpraySoapBindingEx::xsltTransform(const char* xml, const char* sheet,
}
#endif

static void validateDropZoneReq(IEspContext& ctx, const char* dropZoneName, const char* host, const char* path, SecAccessFlags permissionReq)
{
if (!validateDropZoneHostAndPath(dropZoneName, host, path)) //The pathStr should be the absolute path for the dropzone.
throw makeStringException(ECLWATCH_INVALID_INPUT, "Invalid DropZoneName, NetAddress or Path.");

SecAccessFlags permission = getDZPathScopePermissions(ctx, dropZoneName, path, host);
if ((permission < permissionReq) && getGlobalConfigSP()->getPropBool("expert/@failOverToLegacyPhysicalPerms", !isContainerized()))
permission = getLegacyDZPhysicalPerms(ctx, dropZoneName, host, path, true);
if (permission < permissionReq)
throw makeStringExceptionV(ECLWATCH_INVALID_INPUT, "Access DropZone %s %s %s not allowed for user %s (permission:%s). %s permission required.",
dropZoneName, host, path, ctx.queryUserId(), getSecAccessFlagName(permission), getSecAccessFlagName(permissionReq));
}

int CFileSpraySoapBindingEx::downloadFile(IEspContext &context, CHttpRequest* request, CHttpResponse* response)
{
try
Expand All @@ -412,12 +425,7 @@ int CFileSpraySoapBindingEx::downloadFile(IEspContext &context, CHttpRequest* re
pathStr.replace(pathSep=='\\'?'/':'\\', pathSep);
addPathSepChar(pathStr);

if (!validateDropZoneHostAndPath(dropZoneName, netAddressStr, pathStr)) //The pathStr should be the absolute path for the dropzone.
throw makeStringException(ECLWATCH_INVALID_INPUT, "Invalid DropZoneName, NetAddress or Path.");
SecAccessFlags permission = getDZPathScopePermissions(context, dropZoneName, pathStr, netAddressStr);
if (permission < SecAccess_Read)
throw makeStringExceptionV(ECLWATCH_INVALID_INPUT, "Access DropZone Scope %s %s %s not allowed for user %s (permission:%s). Read Access Required.",
dropZoneName.str(), netAddressStr.str(), pathStr.str(), context.queryUserId(), getSecAccessFlagName(permission));
validateDropZoneReq(context, dropZoneName, netAddressStr, pathStr, SecAccess_Read);

StringBuffer fullName;
fullName.appendf("%s%s", pathStr.str(), nameStr.str());
Expand Down Expand Up @@ -463,13 +471,7 @@ int CFileSpraySoapBindingEx::onStartUpload(IEspContext& ctx, CHttpRequest* reque
request->getParameter("NetAddress", netAddress);
request->getParameter("Path", path);
request->getParameter("DropZoneName", dropZoneName);
if (!validateDropZoneHostAndPath(dropZoneName, netAddress, path)) //The path should be the absolute path for the dropzone.
throw makeStringException(ECLWATCH_INVALID_INPUT, "Invalid DropZoneName, NetAddress or Path.");
SecAccessFlags permission = getDZPathScopePermissions(ctx, dropZoneName, path, netAddress);
if (permission < SecAccess_Full)
throw makeStringExceptionV(ECLWATCH_INVALID_INPUT, "Access DropZone Scope %s %s %s not allowed for user %s (permission:%s). Full Access Required.",
dropZoneName.str(), netAddress.str(), path.str(), ctx.queryUserId(), getSecAccessFlagName(permission));

validateDropZoneReq(ctx, dropZoneName, netAddress, path, SecAccess_Full);
return EspHttpBinding::onStartUpload(ctx, request, response, serv, method);
}

Expand Down
46 changes: 30 additions & 16 deletions esp/services/ws_fs/ws_fsService.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1920,6 +1920,16 @@ static bool parseUNCPath(const char* sprayPath, StringBuffer& localPath, StringB
return true;
}

static void validateDZFilePermission(IEspContext& context, const char* dropZoneName, const char* path, const char* host, SecAccessFlags permissionReq)
{
SecAccessFlags permission = getDZFileScopePermissions(context, dropZoneName, path, host);
if ((permission < permissionReq) && getGlobalConfigSP()->getPropBool("expert/@failOverToLegacyPhysicalPerms", !isContainerized()))
permission = getLegacyDZPhysicalPerms(context, dropZoneName, host, path, false);
if (permission < permissionReq)
throw makeStringExceptionV(ECLWATCH_INVALID_INPUT, "Access DropZone %s %s not allowed for user %s (permission:%s). %s permission Required.",
dropZoneName, path, context.queryUserId(), getSecAccessFlagName(permission), getSecAccessFlagName(permissionReq));
}

void CFileSprayEx::readAndCheckSpraySourceReq(IEspContext& context, MemoryBuffer& srcxml, const char* srcIP, const char* srcPath, const char* srcPlane,
StringBuffer& sourcePlaneReq, StringBuffer& sourceIPReq, StringBuffer& sourcePathReq)
{
Expand Down Expand Up @@ -2036,10 +2046,7 @@ void CFileSprayEx::readAndCheckSpraySourceReq(IEspContext& context, MemoryBuffer
{
//Based on the tests, the dfuserver only supports the wildcard inside the file name, like '/path/f*'.
//The dfuserver throws an error if the wildcard is inside the path, like /p*ath/file.
SecAccessFlags permission = getDZFileScopePermissions(context, sourcePlaneReq, path, nullptr);
if (permission < SecAccess_Read)
throw makeStringExceptionV(ECLWATCH_INVALID_INPUT, "Access DropZone Scope %s %s not allowed for user %s (permission:%s). Read Access Required.",
sourcePlaneReq.str(), path, context.queryUserId(), getSecAccessFlagName(permission));
validateDZFilePermission(context, sourcePlaneReq, path, sourceIPReq, SecAccess_Read);
}

if (!sourcePathReq.isEmpty())
Expand Down Expand Up @@ -2585,10 +2592,7 @@ bool CFileSprayEx::onDespray(IEspContext &context, IEspDespray &req, IEspDespray
if (!isEmptyString(destPlane)) // must be true, unless bare-metal and isDropZoneRestrictionEnabled()==false
{
getDropZoneInfoByDestPlane(version, destPlane, destfile, destfileWithPath, umask, destip);
SecAccessFlags permission = getDZFileScopePermissions(context, destPlane, destfileWithPath, destip);
if (permission < SecAccess_Write)
throw makeStringExceptionV(ECLWATCH_INVALID_INPUT, "Access DropZone Scope %s %s not allowed for user %s (permission:%s). Write Access Required.",
destPlane, destfileWithPath.str(), context.queryUserId(), getSecAccessFlagName(permission));
validateDZFilePermission(context, destPlane, destfileWithPath, destip, SecAccess_Write);
}
else
destfileWithPath.append(destfile).trim();
Expand Down Expand Up @@ -2975,6 +2979,15 @@ bool CFileSprayEx::onDFUWUFile(IEspContext &context, IEspDFUWUFileRequest &req,
return true;
}

static SecAccessFlags getDZPathPermission(IEspContext &context, const char *dropZoneName, const char *path,
const char *host, SecAccessFlags permissionReq)
{
SecAccessFlags permission = getDZPathScopePermissions(context, dropZoneName, path, host);
if ((permission < permissionReq) && getGlobalConfigSP()->getPropBool("expert/@failOverToLegacyPhysicalPerms", !isContainerized()))
permission = getLegacyDZPhysicalPerms(context, dropZoneName, host, path, true);
return permission;
}

bool CFileSprayEx::onFileList(IEspContext &context, IEspFileListRequest &req, IEspFileListResponse &resp)
{
try
Expand Down Expand Up @@ -3034,9 +3047,9 @@ bool CFileSprayEx::onFileList(IEspContext &context, IEspFileListRequest &req, IE
dropZoneName = findDropZonePlaneName(netaddr, sPath, dzName);
if (!isEmptyString(dropZoneName))
{
SecAccessFlags permission = getDZPathScopePermissions(context, dropZoneName, sPath, nullptr);
SecAccessFlags permission = getDZPathPermission(context, dropZoneName, sPath, netaddr, SecAccess_Read);
if (permission < SecAccess_Read)
throw makeStringExceptionV(ECLWATCH_INVALID_INPUT, "Access DropZone Scope %s %s not allowed for user %s (permission:%s). Read Access Required.",
throw makeStringExceptionV(ECLWATCH_INVALID_INPUT, "Access DropZone %s %s not allowed for user %s (permission:%s). Read Access Required.",
dropZoneName, sPath.str(), context.queryUserId(), getSecAccessFlagName(permission));
}

Expand Down Expand Up @@ -3145,7 +3158,7 @@ void CFileSprayEx::addPhysicalFile(IEspContext& context, IDirectoryIterator* di,
bool CFileSprayEx::searchDropZoneFiles(IEspContext& context, const char* dropZoneName, const char* server,
const char* dir, const char* relDir, const char* nameFilter, IArrayOf<IConstPhysicalFileStruct>& files, unsigned& filesFound)
{
if (getDZPathScopePermissions(context, dropZoneName, dir, server) < SecAccess_Read)
if (getDZPathPermission(context, dropZoneName, dir, server, SecAccess_Read) < SecAccess_Read)
return false;

RemoteFilename rfn;
Expand Down Expand Up @@ -3366,7 +3379,7 @@ void CFileSprayEx::getPhysicalFiles(IEspContext &context, const char *dropZoneNa
if (dropZoneName && di->isDir())
{
VStringBuffer fullPath("%s%s", path, fileName.str());
if (getDZPathScopePermissions(context, dropZoneName, fullPath, nullptr) < SecAccess_Read)
if (getDZPathPermission(context, dropZoneName, fullPath, host, SecAccess_Read) < SecAccess_Read)
continue;
}

Expand Down Expand Up @@ -3477,8 +3490,8 @@ bool CFileSprayEx::onDropZoneFiles(IEspContext &context, IEspDropZoneFilesReques
StringBuffer planeName;
if (isEmptyString(dzName))
dzName = findDropZonePlaneName(netAddress, directoryStr, planeName);
if (!isEmptyString(dzName) && getDZPathScopePermissions(context, dzName, directoryStr, nullptr) < SecAccess_Read)
return false;
if (!isEmptyString(dzName) && (getDZPathPermission(context, dzName, directoryStr, netAddress, SecAccess_Read) < SecAccess_Read))
return false;

bool directoryOnly = req.getDirectoryOnly();
IArrayOf<IConstPhysicalFileStruct> &files = resp.getFiles();
Expand Down Expand Up @@ -3628,7 +3641,7 @@ void CFileSprayEx::checkDropZoneFileScopeAccess(IEspContext &context, const char
dropZoneName = findDropZonePlaneName(netAddress, dropZonePath, dzName);
if (!isEmptyString(dropZoneName))
{
SecAccessFlags permission = getDZPathScopePermissions(context, dropZoneName, dropZonePath, nullptr);
SecAccessFlags permission = getDZPathPermission(context, dropZoneName, dropZonePath, netAddress, accessReq);
if (permission < accessReq)
throw makeStringExceptionV(ECLWATCH_INVALID_INPUT, "Access DropZone Scope %s %s not allowed for user %s (permission:%s). %s Permission Required.",
dropZoneName, dropZonePath, context.queryUserId(), getSecAccessFlagName(permission), accessReqName);
Expand Down Expand Up @@ -3674,7 +3687,8 @@ void CFileSprayEx::checkDropZoneFileScopeAccess(IEspContext &context, const char
StringBuffer fullPath(dropZonePath);
addPathSepChar(fullPath).append(pathToCheck);
//If the dropzone name is not found, the DZPathScopePermissions cannot be validated.
SecAccessFlags permission = isEmptyString(dropZoneName) ? accessReq : getDZPathScopePermissions(context, dropZoneName, fullPath, nullptr);
SecAccessFlags permission = isEmptyString(dropZoneName) ? accessReq
: getDZPathPermission(context, dropZoneName, fullPath, netAddress, accessReq);
if (permission < accessReq)
{
uniquePath.setValue(pathToCheck.str(), false); //add a path denied
Expand Down
39 changes: 38 additions & 1 deletion esp/smc/SMCLib/TpCommon.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,37 @@ extern TPWRAPPER_API SecAccessFlags getDZFileScopePermissions(IEspContext& conte
return getDZPathScopePermissions(context, dropZoneName, dropZonePath, dropZoneHost);
}

extern TPWRAPPER_API SecAccessFlags getLegacyDZPhysicalPerms(IEspContext& context, const char* dropZoneName, const char* dropZoneHost,
const char* dropZoneFile, bool pathOnly)
{
if (isEmptyString(dropZoneHost) && isEmptyString(dropZoneName))
throw makeStringException(ECLWATCH_INVALID_INPUT, "Neither dropzone plane or host specified.");

Owned<IUserDescriptor> userDesc = createUserDescriptor();
userDesc->set(context.queryUserId(), context.queryPassword(), context.querySignature());

StringBuffer host;
if (isEmptyString(dropZoneHost))
{
Owned<IPropertyTree> plane = getDropZonePlane(dropZoneName);
if (getPlaneHost(host, plane, 0))
dropZoneHost = host.str();
else
dropZoneHost = "localhost";
}

//The setExternal() requests the path with a file name. The file name is not used by the getDLFNPermissions().
//If the dropZoneFile is a path only, the dropZoneScope + a faked file name is passed to the setExternal().
StringBuffer pathWithFileName(dropZoneFile);
if (pathOnly)
addNonEmptyPathSepChar(pathWithFileName).append("any");

CDfsLogicalFileName dlfn;
SocketEndpoint ep(dropZoneHost);
dlfn.setExternal(ep, pathWithFileName);
return queryDistributedFileDirectory().getDLFNPermissions(dlfn, userDesc);
}

extern TPWRAPPER_API void validateDropZoneAccess(IEspContext& context, const char* targetDZNameOrHost, const char* hostReq, SecAccessFlags permissionReq,
const char* fileNameWithRelPath, CDfsLogicalFileName& dlfn)
{
Expand All @@ -246,8 +277,14 @@ extern TPWRAPPER_API void validateDropZoneAccess(IEspContext& context, const cha
}
const char *dropZoneName = dropZone->queryProp("@name");
SecAccessFlags permission = getDZFileScopePermissions(context, dropZoneName, fileNameWithRelPath, hostReq);
if ((permission < permissionReq) && getGlobalConfigSP()->getPropBool("expert/@failOverToLegacyPhysicalPerms", !isContainerized()))
{
StringBuffer fullPath(dropZone->queryProp("@prefix"));
addNonEmptyPathSepChar(fullPath).append(fileNameWithRelPath);
permission = getLegacyDZPhysicalPerms(context, dropZoneName, hostReq, fullPath, false);
}
if (permission < permissionReq)
throw makeStringExceptionV(ECLWATCH_INVALID_INPUT, "Access DropZone Scope %s %s not allowed for user %s (permission:%s). %s Access Required.",
throw makeStringExceptionV(ECLWATCH_INVALID_INPUT, "Access DropZone %s %s not allowed for user %s (permission:%s). %s Access Required.",
dropZoneName, fileNameWithRelPath, context.queryUserId(), getSecAccessFlagName(permission), getSecAccessFlagName(permissionReq));
dlfn.setPlaneExternal(dropZoneName, fileNameWithRelPath);
}
1 change: 1 addition & 0 deletions esp/smc/SMCLib/TpWrapper.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ extern TPWRAPPER_API StringBuffer &findDropZonePlaneName(const char* host, const
extern TPWRAPPER_API bool validateDropZoneHostAndPath(const char* dropZoneName, const char* hostToCheck, const char* pathToCheck);
extern TPWRAPPER_API SecAccessFlags getDZPathScopePermissions(IEspContext& context, const char* dropZoneName, const char* dropZonePath, const char* dropZoneHost);
extern TPWRAPPER_API SecAccessFlags getDZFileScopePermissions(IEspContext& context, const char* dropZoneName, const char* dropZonePath, const char* dropZoneHost);
extern TPWRAPPER_API SecAccessFlags getLegacyDZPhysicalPerms(IEspContext& context, const char* dropZoneName, const char* dropZoneHost, const char* dropZoneFile, bool pathOnly);
extern TPWRAPPER_API void validateDropZoneAccess(IEspContext& context, const char* targetDZNameOrHost, const char* hostReq, SecAccessFlags permissionReq,
const char* fileNameWithRelPath, CDfsLogicalFileName& dlfn);

Expand Down

0 comments on commit bdb9f0b

Please sign in to comment.