Skip to content

Commit

Permalink
HPCC-30037 Revise based on review
Browse files Browse the repository at this point in the history 
1. Rename the validateDropZoneHostAndPath() to getDropZoneAndValidateHostAndPath()
and revise it. It is called when uploading/downloading dropzone file and in
WsFileSprayEx::onFileList(). There are 2 issues in the existing code: a. some
code are duplicated with the code in FileSprayEx::onFileList(). b. the
isDropZoneRestrictionEnabled is not used.
2. Rename the validateDZFileScopePermissions() to
   validateDZFileScopePermsAndLegacyPhysicalPerms()
3. Consoliate the code in FileSprayEx::onFileList() by using the
getDropZoneAndValidateHostAndPath().
4. Add validateDZPathScopePermsAndLegacyPhysicalPerms() which contains the common
code for the validateDropZoneReq() and WsFileSprayEx.
5. Revise the code in validateDropZoneReq().
6. Move the addPathSepChar() line, fix an intent bug, and fix a message bug.
7. Add more comments.

Signed-off-by: wangkx <[email protected]>
  • Loading branch information
wangkx committed Sep 19, 2023
1 parent a1ce672 commit 00bebf5
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 63 deletions.
2 changes: 1 addition & 1 deletion esp/services/ws_fs/ws_fsBinding.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -410,11 +410,11 @@ int CFileSpraySoapBindingEx::downloadFile(IEspContext &context, CHttpRequest* re
if (!osStr.isEmpty() && (atoi(osStr.str())== OS_WINDOWS))
pathSep = '\\';
pathStr.replace(pathSep=='\\'?'/':'\\', pathSep);
addPathSepChar(pathStr);

validateDropZoneReq(context, dropZoneName, netAddressStr, pathStr, SecAccess_Read);

StringBuffer fullName;
addPathSepChar(pathStr);
fullName.appendf("%s%s", pathStr.str(), nameStr.str());

StringBuffer headerStr("attachment;");
Expand Down
36 changes: 12 additions & 24 deletions esp/services/ws_fs/ws_fsService.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2036,7 +2036,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.
validateDZFileScopePermissions(context, sourcePlaneReq, path, sourceIPReq, SecAccess_Read);
validateDZFileScopePermsAndLegacyPhysicalPerms(context, sourcePlaneReq, path, sourceIPReq, SecAccess_Read);
}

if (!sourcePathReq.isEmpty())
Expand Down Expand Up @@ -2582,7 +2582,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);
validateDZFileScopePermissions(context, destPlane, destfileWithPath, destip, SecAccess_Write);
validateDZFileScopePermsAndLegacyPhysicalPerms(context, destPlane, destfileWithPath, destip, SecAccess_Write);
}
else
destfileWithPath.append(destfile).trim();
Expand Down Expand Up @@ -3023,23 +3023,18 @@ bool CFileSprayEx::onFileList(IEspContext &context, IEspFileListRequest &req, IE
}
else
{
StringBuffer dzName;
if (isEmptyString(dropZoneName))
dropZoneName = findDropZonePlaneName(netaddr, sPath, dzName);
if (!isEmptyString(dropZoneName))
Owned<IPropertyTree> dropZone = getDropZoneAndValidateHostAndPath(dropZoneName, netaddr, sPath);
if (dropZone) // In bare-metal and isDropZoneRestrictionEnabled()==false, the dropZone may be nullptr.
{
SecAccessFlags permission = getDZPathScopePermsAndLegacyPhysicalPerms(context, dropZoneName, sPath, netaddr, SecAccess_Read);
if (permission < SecAccess_Read)
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));
if (isEmptyString(dropZoneName))
dropZoneName= dropZone->queryProp("@name");
validateDZPathScopePermsAndLegacyPhysicalPerms(context, dropZoneName, sPath, netaddr, SecAccess_Read);
}

StringArray hosts;
if (isEmptyString(netaddr))
{
Owned<IPropertyTree> dropZone = getDropZonePlane(dropZoneName);
if (!dropZone)
throw makeStringExceptionV(ECLWATCH_INVALID_INPUT, "Unknown landing zone: %s", dropZoneName);
{ // If the netaddr is empty, the dropZoneName will not be empty and the dropZone should be
// set by the getDropZoneAndValidateHostAndPath().
getPlaneHosts(hosts, dropZone);
if (!hosts.ordinality())
hosts.append("localhost");
Expand All @@ -3049,9 +3044,7 @@ bool CFileSprayEx::onFileList(IEspContext &context, IEspFileListRequest &req, IE

ForEachItemIn(i, hosts)
{
const char* host = hosts.item(i);
if (validateDropZoneHostAndPath(dropZoneName, host, sPath))
getPhysicalFiles(context, dropZoneName, host, sPath, fileNameMask, directoryOnly, files);
getPhysicalFiles(context, dropZoneName, hosts.item(i), sPath, fileNameMask, directoryOnly, files);
}
}

Expand Down Expand Up @@ -3472,7 +3465,7 @@ bool CFileSprayEx::onDropZoneFiles(IEspContext &context, IEspDropZoneFilesReques
if (isEmptyString(dzName))
dzName = findDropZonePlaneName(netAddress, directoryStr, planeName);
if (!isEmptyString(dzName) && (getDZPathScopePermsAndLegacyPhysicalPerms(context, dzName, directoryStr, netAddress, SecAccess_Read) < SecAccess_Read))
return false;
return false;

bool directoryOnly = req.getDirectoryOnly();
IArrayOf<IConstPhysicalFileStruct> &files = resp.getFiles();
Expand Down Expand Up @@ -3621,12 +3614,7 @@ void CFileSprayEx::checkDropZoneFileScopeAccess(IEspContext &context, const char
if (isEmptyString(dropZoneName))
dropZoneName = findDropZonePlaneName(netAddress, dropZonePath, dzName);
if (!isEmptyString(dropZoneName))
{
SecAccessFlags permission = getDZPathScopePermsAndLegacyPhysicalPerms(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);
}
validateDZPathScopePermsAndLegacyPhysicalPerms(context, dropZoneName, dropZonePath, netAddress, accessReq);

RemoteFilename rfn;
SocketEndpoint ep(netAddress);
Expand Down
104 changes: 68 additions & 36 deletions esp/smc/SMCLib/TpCommon.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,33 +148,38 @@ StringBuffer &findDropZonePlaneName(const char *host, const char *path, StringBu
return planeName;
}

extern TPWRAPPER_API bool validateDropZoneHostAndPath(const char* dropZoneName, const char* hostToCheck, const char* pathToCheck)
//If the dropZoneName is empty, find out the dropzone based on the host and path. In bare-metal and
//isDropZoneRestrictionEnabled()==false, the dropzone may not be found.
//If the dropZoneName is not empty, find out the dropzone based on the dropZoneName and validate the host and path.
extern TPWRAPPER_API IPropertyTree* getDropZoneAndValidateHostAndPath(const char* dropZoneName, const char* host, const char* path)
{
//Both hostToCheck and pathToCheck should not be empty. For backward compatibility, the dropZoneName may be empty.
if (isEmptyString(hostToCheck))
throw makeStringException(ECLWATCH_INVALID_INPUT, "Host not defined.");
if (isEmptyString(pathToCheck))
throw makeStringException(ECLWATCH_INVALID_INPUT, "Path not defined.");
if (isContainerized() && streq("localhost", hostToCheck))
hostToCheck = nullptr; // "localhost" is a placeholder for mounted dropzones that have no hosts.
if (isEmptyString(hostToCheck) && isEmptyString(dropZoneName))
throw makeStringException(ECLWATCH_INVALID_INPUT, "No dropzone or host provided.");
StringBuffer pathToCheck(path);
addPathSepChar(pathToCheck);

if (containsRelPaths(pathToCheck)) //Detect a path like: /home/lexis/runtime/var/lib/HPCCSystems/mydropzone/../../../
throw makeStringExceptionV(ECLWATCH_INVALID_INPUT, "Invalid path %s", pathToCheck);
const char* hostToCheck = nullptr;
if (isContainerized() && strsame(host, "localhost"))
hostToCheck = nullptr; // "localhost" is a placeholder for mounted dropzones that have no hosts.
else
hostToCheck = host;

StringBuffer path(pathToCheck);
addPathSepChar(path);
Owned<IPropertyTree> dropZone;
if (isEmptyString(dropZoneName))
{
Owned<IPropertyTree> plane = findDropZonePlane(path, hostToCheck, isIPAddress(hostToCheck), false);
return nullptr != plane;
dropZone.setown(findDropZonePlane(pathToCheck, hostToCheck, isIPAddress(hostToCheck), false));
if (!dropZone)
throwOrLogDropZoneLookUpError(ECLWATCH_INVALID_INPUT, "DropZone not found for host '%s' path '%s'.",
isEmptyString(host) ? "unspecified" : host, isEmptyString(path) ? "unspecified" : path);
}

Owned<IPropertyTree> plane = getDropZonePlane(dropZoneName);
if (nullptr == plane)
return false;
return validateDropZone(plane, path, hostToCheck, isIPAddress(hostToCheck));
else
{
dropZone.setown(getDropZonePlane(dropZoneName));
if (!dropZone)
throw makeStringExceptionV(ECLWATCH_INVALID_INPUT, "DropZone '%s' not found.", dropZoneName);
if (!validateDropZone(dropZone, pathToCheck, hostToCheck, isIPAddress(hostToCheck)))
throw makeStringExceptionV(ECLWATCH_INVALID_INPUT, "The host '%s' or path '%s' is not valid for dropzone %s.",
isEmptyString(host) ? "unspecified" : host, isEmptyString(path) ? "unspecified" : path, dropZoneName);
}
return dropZone.getClear();
}

static SecAccessFlags getDropZoneScopePermissions(IEspContext& context, const IPropertyTree* dropZone, const char* dropZonePath)
Expand Down Expand Up @@ -207,7 +212,7 @@ static SecAccessFlags getDZPathScopePermissions(IEspContext& context, const char
dropZone.setown(findDropZonePlane(dropZonePath, dropZoneHost, true, false));
if (!dropZone)
{
throwOrLogDropZoneLookUpError(ECLWATCH_INVALID_INPUT, "getDZPathScopePermissions(): DropZone %s not found for host '%s' path '%s'.",
throwOrLogDropZoneLookUpError(ECLWATCH_INVALID_INPUT, "getDZPathScopePermissions(): DropZone not found for host '%s' path '%s'.",
isEmptyString(dropZoneHost) ? "unspecified" : dropZoneHost, isEmptyString(dropZonePath) ? "unspecified" : dropZonePath);
return SecAccess_Full;
}
Expand Down Expand Up @@ -266,9 +271,10 @@ static SecAccessFlags getLegacyDZPhysicalPerms(IEspContext& context, const char*
return queryDistributedFileDirectory().getFScopePermissions(scopesToCheck, userDesc);
}

//Get dropzone file permission when the file path does not contain a file name. To get the permission,
//this function (and the validate functions below) calls the getDZPathScopePermissions() which handles
//isDropZoneRestrictionEnabled if a dropzone cannot be found based on host and path.
//Get dropzone file permission for a dropzone file path (the path does NOT contain a file name).
//This function (and the validate functions below) calls the getDZPathScopePermissions(). In the
//getDZPathScopePermissions(), the isDropZoneRestrictionEnabled() will be used for bare-metal if
//a dropzone cannot be found based on host and path.
extern TPWRAPPER_API SecAccessFlags getDZPathScopePermsAndLegacyPhysicalPerms(IEspContext &context, const char *dropZoneName, const char *path,
const char *host, SecAccessFlags permissionReq)
{
Expand All @@ -278,20 +284,19 @@ extern TPWRAPPER_API SecAccessFlags getDZPathScopePermsAndLegacyPhysicalPerms(IE
return permission;
}

//Validate dropzone host and file path. Also validate dropzone file permission when the file path does not contain a file name.
extern TPWRAPPER_API void validateDropZoneReq(IEspContext& ctx, const char* dropZoneName, const char* host, const char* path, SecAccessFlags permissionReq)
//Validate dropzone file permission for a dropzone file path (the path does NOT contain a file name)
extern TPWRAPPER_API void validateDZPathScopePermsAndLegacyPhysicalPerms(IEspContext &context, const char *dropZoneName, const char *path,
const char *host, 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 = getDZPathScopePermsAndLegacyPhysicalPerms(ctx, dropZoneName, path, host, permissionReq);
SecAccessFlags permission = getDZPathScopePermsAndLegacyPhysicalPerms(context, dropZoneName, path, host, permissionReq);
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));
throw makeStringExceptionV(ECLWATCH_INVALID_INPUT, "Access DropZone %s %s not allowed for user %s (permission:%s). Read Access Required.",
dropZoneName, path, context.queryUserId(), getSecAccessFlagName(permission));
}

//Validate dropzone file permission when the file path contains a file name.
extern TPWRAPPER_API void validateDZFileScopePermissions(IEspContext& context, const char* dropZoneName, const char* path, const char* host, SecAccessFlags permissionReq)
//Validate dropzone file permission for a dropzone file path (the path DOES contain a file name)
extern TPWRAPPER_API void validateDZFileScopePermsAndLegacyPhysicalPerms(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()))
Expand All @@ -301,7 +306,34 @@ extern TPWRAPPER_API void validateDZFileScopePermissions(IEspContext& context, c
dropZoneName, path, context.queryUserId(), getSecAccessFlagName(permission), getSecAccessFlagName(permissionReq));
}

//Validate dropzone file path and permission, Also set the dlfn.
//This function is called when uploading a file to a dropzone or downloading a file to a dropzone.
//1. both host and filePath should not be empty.
//2. the filePath should not contain bad file path.
//3. call the getDropZoneAndValidateHostAndPath().
//4. if the dropzone name is available, validate dropzone file permission. Should be available unless bare-metal and isDropZoneRestrictionEnabled()==false.
extern TPWRAPPER_API void validateDropZoneReq(IEspContext& ctx, const char* dropZoneName, const char* host, const char* filePath, SecAccessFlags permissionReq)
{
if (isEmptyString(host) || isEmptyString(filePath)) //The host and filePath must be specified for accessing a DropZone file.
throw makeStringException(ECLWATCH_INVALID_INPUT, "Host or file path not defined.");

if (containsRelPaths(filePath)) //Detect a path like: /home/lexis/runtime/var/lib/HPCCSystems/mydropzone/../../../
throw makeStringExceptionV(ECLWATCH_INVALID_INPUT, "Invalid path %s", filePath);

Owned<IPropertyTree> dropZone = getDropZoneAndValidateHostAndPath(dropZoneName, host, filePath);
if (dropZone)
dropZoneName= dropZone->queryProp("@name");

if (!isEmptyString(dropZoneName))
validateDZPathScopePermsAndLegacyPhysicalPerms(ctx, dropZoneName, filePath, host, permissionReq);
}

//This function is called when accessing a dropzone file from the WsFileIO service (the fileNameWithRelPath is a relative path).
//1. the targetDZNameOrHost could be a dropzone name or dropzone host. Find out the dropzone.
// If the targetDZNameOrHost is a dropzone name and the hostReq is not empty, validate the hostReq inside the dropzone.
// If the dropzone cannot be found based on the targetDZNameOrHost, throw an exception.
//2. the fileNameWithRelPath should not contain bad file path.
//3. validate dropzone file permission.
//4. set the dlfn using dropzone name and fileNameWithRelPath.
extern TPWRAPPER_API void validateDropZoneAccess(IEspContext& context, const char* targetDZNameOrHost, const char* hostReq, SecAccessFlags permissionReq,
const char* fileNameWithRelPath, CDfsLogicalFileName& dlfn)
{
Expand Down
6 changes: 4 additions & 2 deletions esp/smc/SMCLib/TpWrapper.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,12 +240,14 @@ extern TPWRAPPER_API bool validateDataPlaneName(const char *remoteDali, const ch
extern TPWRAPPER_API bool matchNetAddressRequest(const char* netAddressReg, bool ipReq, IConstTpMachine& tpMachine);

extern TPWRAPPER_API StringBuffer &findDropZonePlaneName(const char* host, const char* path, StringBuffer& planeName);
extern TPWRAPPER_API bool validateDropZoneHostAndPath(const char* dropZoneName, const char* hostToCheck, const char* pathToCheck);
extern TPWRAPPER_API IPropertyTree* getDropZoneAndValidateHostAndPath(const char* dropZoneName, const char* host, const char* path);
extern TPWRAPPER_API void validateDropZoneAccess(IEspContext& context, const char* targetDZNameOrHost, const char* hostReq, SecAccessFlags permissionReq,
const char* fileNameWithRelPath, CDfsLogicalFileName& dlfn);
extern TPWRAPPER_API SecAccessFlags getDZPathScopePermsAndLegacyPhysicalPerms(IEspContext &context, const char *dropZoneName, const char *path,
const char *host, SecAccessFlags permissionReq);
extern TPWRAPPER_API void validateDZPathScopePermsAndLegacyPhysicalPerms(IEspContext &context, const char *dropZoneName, const char *path,
const char *host, SecAccessFlags permissionReq);
extern TPWRAPPER_API void validateDropZoneReq(IEspContext& ctx, const char* dropZoneName, const char* host, const char* path, SecAccessFlags permissionReq);
extern TPWRAPPER_API void validateDZFileScopePermissions(IEspContext& context, const char* dropZoneName, const char* path, const char* host, SecAccessFlags permissionReq);
extern TPWRAPPER_API void validateDZFileScopePermsAndLegacyPhysicalPerms(IEspContext& context, const char* dropZoneName, const char* path, const char* host, SecAccessFlags permissionReq);

#endif //_ESPWIZ_TpWrapper_HPP__

0 comments on commit 00bebf5

Please sign in to comment.