Skip to content

Commit

Permalink
Merge pull request #17144 from wangkx/h29189
Browse files Browse the repository at this point in the history 
HPCC-29189 Check Dropzone scope access before spraying/despraying

Reviewed-by: Jake Smith <[email protected]>
Reviewed-by: Gavin Halliday <[email protected]>
Merged-by: Gavin Halliday <[email protected]>
  • Loading branch information
@ghalliday
ghalliday authored Apr 17, 2023
2 parents d43611c + 4939327 commit eaaffad
Show file tree
Hide file tree
Showing 7 changed files with 127 additions and 114 deletions.
40 changes: 0 additions & 40 deletions esp/services/ws_fileio/ws_fileioservice.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,46 +35,6 @@ void CWsFileIOEx::init(IPropertyTree *cfg, const char *process, const char *serv
tpWrapper.getTpDropZones(9999, nullptr, false, allTpDropZones); //version 9999: get the latest information about dropzone
}

void CWsFileIOEx::validateDropZoneAccess(IEspContext &context, const char *targetDZNameOrHost, const char *hostReq, SecAccessFlags permissionReq,
const char *fileNameWithRelPath, CDfsLogicalFileName &dlfn)
{
if (containsRelPaths(fileNameWithRelPath)) //Detect a path like: a/../../../f
throw makeStringExceptionV(ECLWATCH_INVALID_INPUT, "Invalid file path %s", fileNameWithRelPath);

Owned<IPropertyTree> dropZone = getDropZonePlane(targetDZNameOrHost);
if (!dropZone) //The targetDZNameOrHost could be a dropzone host.
{
dropZone.setown(findDropZonePlane(nullptr, targetDZNameOrHost, true, true));
}
else if (!isEmptyString(hostReq))
{
if (!isHostInPlane(dropZone, hostReq, true))
throw makeStringExceptionV(ECLWATCH_INVALID_INPUT, "Host %s is not valid DropZone plane %s", hostReq, targetDZNameOrHost);
}

//make sure a relative dropzone path
StringBuffer s;
const char* prefix = dropZone->queryProp("@prefix");
if (hasPrefix(fileNameWithRelPath, prefix, true))
{
const char* p = fileNameWithRelPath + strlen(prefix);
if (!*p || !isPathSepChar(p[0]))
addPathSepChar(s);
s.append(p);
fileNameWithRelPath = s.str();
}

const char *dropZoneName = dropZone->queryProp("@name");
dlfn.setPlaneExternal(dropZoneName, fileNameWithRelPath);

Owned<IUserDescriptor> userDesc = createUserDescriptor();
userDesc->set(context.queryUserId(), context.queryPassword(), context.querySignature());
SecAccessFlags permission = queryDistributedFileDirectory().getDLFNPermissions(dlfn, userDesc);
if (permission < permissionReq)
throw makeStringExceptionV(ECLWATCH_INVALID_INPUT, "Access DropZone Scope %s %s not allowed for user %s (permission:%s). %s Access Required.",
dropZoneName, fileNameWithRelPath, context.queryUserId(), getSecAccessFlagName(permission), getSecAccessFlagName(permissionReq));
}

bool CWsFileIOEx::onCreateFile(IEspContext &context, IEspCreateFileRequest &req, IEspCreateFileResponse &resp)
{
context.ensureFeatureAccess(FILE_IO_URL, SecAccess_Write, ECLWATCH_ACCESS_TO_FILE_DENIED, "WsFileIO::CreateFile: Permission denied");
Expand Down
4 changes: 0 additions & 4 deletions esp/services/ws_fileio/ws_fileioservice.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,6 @@ class CWsFileIOEx : public CWsFileIO
virtual bool onCreateFile(IEspContext &context, IEspCreateFileRequest &req, IEspCreateFileResponse &resp);
virtual bool onWriteFileData(IEspContext &context, IEspWriteFileDataRequest &req, IEspWriteFileDataResponse &resp);
virtual bool onReadFileData(IEspContext &context, IEspReadFileDataRequest &req, IEspReadFileDataResponse &resp);

protected:
void validateDropZoneAccess(IEspContext &context, const char *targetDZNameOrHost, const char *hostReq,
SecAccessFlags permissionReq, const char *fileNameWithRelPath, CDfsLogicalFileName &dlfn);
};

#endif //_ESPWIZ_WsFileIO_HPP__
Expand Down
4 changes: 2 additions & 2 deletions esp/services/ws_fs/ws_fsBinding.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ int CFileSpraySoapBindingEx::downloadFile(IEspContext &context, CHttpRequest* re
request->getParameter("Name", nameStr);
request->getParameter("DropZoneName", dropZoneName);

SecAccessFlags permission = getDropZoneScopePermissions(context, dropZoneName, pathStr, netAddressStr);
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));
Expand Down Expand Up @@ -490,7 +490,7 @@ int CFileSpraySoapBindingEx::onStartUpload(IEspContext& ctx, CHttpRequest* reque
request->getParameter("DropZoneName", dropZoneName);
if (!validateDropZonePath(nullptr, netAddress, path)) //The path should be the absolute path for the dropzone.
throw makeStringExceptionV(ECLWATCH_INVALID_INPUT, "Invalid Landing Zone path %s", path.str());
SecAccessFlags permission = getDropZoneScopePermissions(ctx, dropZoneName, path, netAddress);
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));
Expand Down
115 changes: 65 additions & 50 deletions esp/services/ws_fs/ws_fsService.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1882,7 +1882,7 @@ IPropertyTree *CFileSprayEx::getAndValidateDropZone(const char *path, const char
}

void CFileSprayEx::readAndCheckSpraySourceReq(MemoryBuffer& srcxml, const char* srcIP, const char* srcPath, const char* srcPlane,
StringBuffer& sourceIPReq, StringBuffer& sourcePathReq)
StringBuffer& sourcePlaneReq, StringBuffer& sourceIPReq, StringBuffer& sourcePathReq)
{
StringBuffer sourcePath(srcPath);
sourcePath.trim();
Expand Down Expand Up @@ -1910,13 +1910,15 @@ void CFileSprayEx::readAndCheckSpraySourceReq(MemoryBuffer& srcxml, const char*
sourcePath.append(s);
}
getDropZoneHost(srcPlane, dropZone, sourceIPReq);
sourcePlaneReq.append(srcPlane);
}
else
{
sourceIPReq.set(srcIP).trim();
if (sourceIPReq.isEmpty())
throw makeStringExceptionV(ECLWATCH_INVALID_INPUT, "Source network IP not specified.");
Owned<IPropertyTree> plane = getAndValidateDropZone(sourcePath, sourceIPReq);
sourcePlaneReq.append(plane->queryProp("@name"));
}
}
getStandardPosixPath(sourcePathReq, sourcePath.str());
Expand Down Expand Up @@ -1964,9 +1966,8 @@ bool CFileSprayEx::onSprayFixed(IEspContext &context, IEspSprayFixed &req, IEspS
throw makeStringExceptionV(ECLWATCH_INVALID_INPUT, "Destination node group not specified.");

MemoryBuffer& srcxml = (MemoryBuffer&)req.getSrcxml();
StringBuffer sourceIPReq, sourcePathReq;
readAndCheckSpraySourceReq(srcxml, req.getSourceIP(), req.getSourcePath(), req.getSourcePlane(), sourceIPReq, sourcePathReq);
const char* srcip = sourceIPReq.str();
StringBuffer sourcePlaneReq, sourceIPReq, sourcePathReq;
readAndCheckSpraySourceReq(srcxml, req.getSourceIP(), req.getSourcePath(), req.getSourcePlane(), sourcePlaneReq, sourceIPReq, sourcePathReq);
const char* srcfile = sourcePathReq.str();
const char* destname = req.getDestLogicalName();
if(isEmptyString(destname))
Expand Down Expand Up @@ -2005,24 +2006,7 @@ bool CFileSprayEx::onSprayFixed(IEspContext &context, IEspSprayFixed &req, IEspS
wu->setCommand(DFUcmd_import);

IDFUfileSpec *source = wu->queryUpdateSource();
if(srcxml.length() == 0)
{
RemoteMultiFilename rmfn;
SocketEndpoint ep(srcip);
if (ep.isNull())
throw makeStringExceptionV(ECLWATCH_INVALID_INPUT, "SprayFixed to %s: cannot resolve source network IP from %s.", destname, srcip);

rmfn.setEp(ep);
StringBuffer fnamebuf(srcfile);
fnamebuf.trim();
rmfn.append(fnamebuf.str()); // handles comma separated files
source->setMultiFilename(rmfn);
}
else
{
srcxml.append('\0');
source->setFromXML((const char*)srcxml.toByteArray());
}
checkDZScopeAccessAndSetSpraySourceDFUFileSpec(context, sourcePlaneReq, sourceIPReq, srcfile, srcxml, source);

IDFUfileSpec *destination = wu->queryUpdateDestination();
bool nosplit = req.getNosplit();
Expand Down Expand Up @@ -2147,9 +2131,8 @@ bool CFileSprayEx::onSprayVariable(IEspContext &context, IEspSprayVariable &req,
gName.append(destNodeGroup);

MemoryBuffer& srcxml = (MemoryBuffer&)req.getSrcxml();
StringBuffer sourceIPReq, sourcePathReq;
readAndCheckSpraySourceReq(srcxml, req.getSourceIP(), req.getSourcePath(), req.getSourcePlane(), sourceIPReq, sourcePathReq);
const char* srcip = sourceIPReq.str();
StringBuffer sourcePlaneReq, sourceIPReq, sourcePathReq;
readAndCheckSpraySourceReq(srcxml, req.getSourceIP(), req.getSourcePath(), req.getSourcePlane(), sourcePlaneReq, sourceIPReq, sourcePathReq);
const char* srcfile = sourcePathReq.str();
const char* destname = req.getDestLogicalName();
if(isEmptyString(destname))
Expand Down Expand Up @@ -2182,24 +2165,7 @@ bool CFileSprayEx::onSprayVariable(IEspContext &context, IEspSprayVariable &req,
IDFUfileSpec *destination = wu->queryUpdateDestination();
IDFUoptions *options = wu->queryUpdateOptions();

if(srcxml.length() == 0)
{
RemoteMultiFilename rmfn;
SocketEndpoint ep(srcip);
if (ep.isNull())
throw makeStringExceptionV(ECLWATCH_INVALID_INPUT, "SprayVariable to %s: cannot resolve source network IP from %s.", destname, srcip);

rmfn.setEp(ep);
StringBuffer fnamebuf(srcfile);
fnamebuf.trim();
rmfn.append(fnamebuf.str()); // handles comma separated files
source->setMultiFilename(rmfn);
}
else
{
srcxml.append('\0');
source->setFromXML((const char*)srcxml.toByteArray());
}
checkDZScopeAccessAndSetSpraySourceDFUFileSpec(context, sourcePlaneReq, sourceIPReq, srcfile, srcxml, source);
source->setMaxRecordSize(req.getSourceMaxRecordSize());
source->setFormat((DFUfileformat)req.getSourceFormat());

Expand Down Expand Up @@ -2306,6 +2272,49 @@ bool CFileSprayEx::onSprayVariable(IEspContext &context, IEspSprayVariable &req,
return true;
}

void CFileSprayEx::checkDZScopeAccessAndSetSpraySourceDFUFileSpec(IEspContext &context, const char *srcPlane, const char *srcHost,
const char *srcFile, MemoryBuffer &srcXML, IDFUfileSpec *srcDFUfileSpec)
{
if(srcXML.length() == 0)
{
//Both srcPlane and srcHost are validated in readAndCheckSpraySourceReq().
StringBuffer fnamebuf(srcFile);
fnamebuf.trim();
StringArray files;
files.appendList(fnamebuf, ","); // handles comma separated files
ForEachItemIn(i, files)
{
const char *file = files.item(i);
if (isEmptyString(file))
continue;

//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, srcPlane, file, srcHost);
if (permission < SecAccess_Read)
throw makeStringExceptionV(ECLWATCH_INVALID_INPUT, "Access DropZone Scope %s %s not allowed for user %s (permission:%s). Read Access Required.",
srcPlane, file, context.queryUserId(), getSecAccessFlagName(permission));
}

SocketEndpoint ep(srcHost);
if (ep.isNull())
throw makeStringExceptionV(ECLWATCH_INVALID_INPUT, "Cannot resolve source network IP from %s.", srcHost);

RemoteMultiFilename rmfn;
rmfn.setEp(ep);
rmfn.append(fnamebuf.str());
srcDFUfileSpec->setMultiFilename(rmfn);
}
else
{ //this block is copied from the code before PR https://github.com/hpcc-systems/HPCC-Platform/pull/17144.
//Not sure there exists any use case for spraying a file using the srcXML. JIRA-29339 is created to check
//whether this is completely legacy. If it is, it may be removed to make the code a lot clearer.
srcXML.append('\0');
srcDFUfileSpec->setFromXML((const char*)srcXML.toByteArray());
}
}

bool CFileSprayEx::onReplicate(IEspContext &context, IEspReplicate &req, IEspReplicateResponse &resp)
{
try
Expand Down Expand Up @@ -2458,6 +2467,11 @@ bool CFileSprayEx::onDespray(IEspContext &context, IEspDespray &req, IEspDespray
if (!isEmptyString(destPlane))
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.",
isEmptyString(destPlane) ? destip : destPlane, destfileWithPath.str(), context.queryUserId(), getSecAccessFlagName(permission));

RemoteFilename rfn;
SocketEndpoint ep(destip.str());
if (ep.isNull())
Expand All @@ -2473,7 +2487,8 @@ bool CFileSprayEx::onDespray(IEspContext &context, IEspDespray &req, IEspDespray
destination->setSingleFilename(rfn);
}
else
{
{ //Not sure there exists any use case for despraying a file using the dstxml. JIRA-29339 is created to check
//whether this is completely legacy. If it is, it may be removed to make the code a lot clearer.
dstxml.append('\0');
destination->setFromXML((const char*)dstxml.toByteArray());
}
Expand Down Expand Up @@ -2897,7 +2912,7 @@ bool CFileSprayEx::onFileList(IEspContext &context, IEspFileListRequest &req, IE
Owned<IPropertyTree> plane = findDropZonePlane(sPath, netaddr, true, true);
dropZoneName = plane->queryProp("@name");
}
SecAccessFlags permission = getDropZoneScopePermissions(context, dropZoneName, sPath, nullptr);
SecAccessFlags permission = getDZPathScopePermissions(context, dropZoneName, sPath, 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.",
dropZoneName, sPath.str(), context.queryUserId(), getSecAccessFlagName(permission));
Expand Down Expand Up @@ -3007,7 +3022,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 (getDropZoneScopePermissions(context, dropZoneName, dir, server) < SecAccess_Read)
if (getDZPathScopePermissions(context, dropZoneName, dir, server) < SecAccess_Read)
return false;

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

Expand Down Expand Up @@ -3340,7 +3355,7 @@ bool CFileSprayEx::onDropZoneFiles(IEspContext &context, IEspDropZoneFilesReques
Owned<IPropertyTree> plane = findDropZonePlane(directoryStr, netAddress, true, true);
dzName = plane->queryProp("@name");
}
if (getDropZoneScopePermissions(context, dzName, directoryStr, nullptr) < SecAccess_Read)
if (getDZPathScopePermissions(context, dzName, directoryStr, nullptr) < SecAccess_Read)
return false;

bool directoryOnly = req.getDirectoryOnly();
Expand Down Expand Up @@ -3482,7 +3497,7 @@ void CFileSprayEx::checkDropZoneFileScopeAccess(IEspContext &context, const char
Owned<IPropertyTree> plane = findDropZonePlane(dropZonePath, netAddress, true, true);
dropZoneName = plane->queryProp("@name");
}
SecAccessFlags permission = getDropZoneScopePermissions(context, dropZoneName, dropZonePath, nullptr);
SecAccessFlags permission = getDZPathScopePermissions(context, dropZoneName, dropZonePath, nullptr);
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, accessReqName, context.queryUserId(), getSecAccessFlagName(permission));
Expand Down Expand Up @@ -3526,7 +3541,7 @@ void CFileSprayEx::checkDropZoneFileScopeAccess(IEspContext &context, const char

StringBuffer fullPath(dropZonePath);
addPathSepChar(fullPath).append(pathToCheck);
SecAccessFlags permission = getDropZoneScopePermissions(context, dropZoneName, fullPath, nullptr);
SecAccessFlags permission = getDZPathScopePermissions(context, dropZoneName, fullPath, nullptr);
if (permission < accessReq)
{
uniquePath.setValue(pathToCheck.str(), false); //add a path denied
Expand Down
4 changes: 3 additions & 1 deletion esp/services/ws_fs/ws_fsService.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,14 @@ class Schedule : public Thread
class CFileSprayEx : public CFileSpray
{
void readAndCheckSpraySourceReq(MemoryBuffer& srcxml, const char* srcIP, const char* srcPath, const char* srcplane,
StringBuffer& sourceIPReq, StringBuffer& sourcePathReq);
StringBuffer& sourcePlaneReq, StringBuffer& sourceIPReq, StringBuffer& sourcePathReq);
void getServersInDropZone(const char* dropZoneName, IArrayOf<IConstTpDropZone>& dropZoneList,
bool isECLWatchVisibleOnly, StringArray& serverList);
IPropertyTree* getAndValidateDropZone(const char * path, const char * host);
IEspDFUWorkunit* createDFUWUFromSashaListResult(const char* result);
void setDFUCommand(const char* commandStr, IEspDFUWorkunit* dfuWU);
void checkDZScopeAccessAndSetSpraySourceDFUFileSpec(IEspContext& context, const char* srcPlane, const char * srcHost,
const char* srcFile, MemoryBuffer& srcXML, IDFUfileSpec* srcDFUfileSpec);

public:
virtual void init(IPropertyTree *cfg, const char *process, const char *service);
Expand Down
Loading

0 comments on commit eaaffad

Please sign in to comment.