Skip to content

Commit

Permalink
HPCC-30037 Revise based on review
Browse files Browse the repository at this point in the history 
1. Add queryDistributedFileDirectory().getFScopePermissions() and
call it in the getLegacyDZPhysicalPerms().
2. Move validateDropZoneReq(), validateDZFileScopePermission()
and getDZPathScopePermissionsEx()) into TpCommon.cpp.
Rename the getDZPathScopePermissionsEx() to
getDZPathScopePermsAndLegacyPhysicalPerms().
Rename the validateDZFileScopePermission() to
validateDZFileScopePermissions(). Also revise
them.
3. The following 3 files are not called from outside. Change them
from extern to static: getDZPathScopePermissions(),
getDZFileScopePermissions() and getLegacyDZPhysicalPerms().
4. Add comments.
5. Fix a bug in the message for the throwOrLogDropZoneLookUpError().

Signed-off-by: wangkx <[email protected]>
  • Loading branch information
wangkx committed Sep 12, 2023
1 parent cf5c6e1 commit a1ce672
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 56 deletions.
6 changes: 6 additions & 0 deletions dali/base/dadfs.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1181,6 +1181,7 @@ protected: friend class CDistributedFile;
IDistributedSuperFile *lookupSuperFile(const char *logicalname, IUserDescriptor *user, AccessMode accessMode, IDistributedFileTransaction *transaction, unsigned timeout=INFINITE);

SecAccessFlags getFilePermissions(const char *lname,IUserDescriptor *user,unsigned auditflags);
SecAccessFlags getFScopePermissions(const char *scope,IUserDescriptor *user,unsigned auditflags);
SecAccessFlags getNodePermissions(const IpAddress &ip,IUserDescriptor *user,unsigned auditflags);
SecAccessFlags getFDescPermissions(IFileDescriptor *,IUserDescriptor *user,unsigned auditflags=0);
SecAccessFlags getDLFNPermissions(CDfsLogicalFileName &dlfn,IUserDescriptor *user,unsigned auditflags=0);
Expand Down Expand Up @@ -11875,6 +11876,11 @@ SecAccessFlags CDistributedFileDirectory::getDropZoneScopePermissions(const char
return getScopePermissions(dlfn.get(),user,auditflags);
}

SecAccessFlags CDistributedFileDirectory::getFScopePermissions(const char *scope,IUserDescriptor *user,unsigned auditflags)
{
return getScopePermissions(scope,user,auditflags);
}

void CDistributedFileDirectory::setDefaultUser(IUserDescriptor *user)
{
if (user)
Expand Down
1 change: 1 addition & 0 deletions dali/base/dadfs.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,7 @@ interface IDistributedFileDirectory: extends IInterface
virtual void removeSuperFile(const char *_logicalname, bool delSubs=false, IUserDescriptor *user=NULL, IDistributedFileTransaction *transaction=NULL)=0;

virtual SecAccessFlags getFilePermissions(const char *lname,IUserDescriptor *user,unsigned auditflags=0)=0; // see dasess for auditflags values
virtual SecAccessFlags getFScopePermissions(const char *scope,IUserDescriptor *user,unsigned auditflags=0)=0; // see dasess for auditflags values
virtual void setDefaultUser(IUserDescriptor *user)=0;
virtual IUserDescriptor* queryDefaultUser()=0;
virtual SecAccessFlags getNodePermissions(const IpAddress &ip,IUserDescriptor *user,unsigned auditflags=0)=0;
Expand Down
13 changes: 0 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,19 +386,6 @@ 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 Down
35 changes: 8 additions & 27 deletions esp/services/ws_fs/ws_fsService.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1920,16 +1920,6 @@ static bool parseUNCPath(const char* sprayPath, StringBuffer& localPath, StringB
return true;
}

static void validateDZFileScopePermission(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 @@ -2046,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.
validateDZFileScopePermission(context, sourcePlaneReq, path, sourceIPReq, SecAccess_Read);
validateDZFileScopePermissions(context, sourcePlaneReq, path, sourceIPReq, SecAccess_Read);
}

if (!sourcePathReq.isEmpty())
Expand Down Expand Up @@ -2592,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);
validateDZFileScopePermission(context, destPlane, destfileWithPath, destip, SecAccess_Write);
validateDZFileScopePermissions(context, destPlane, destfileWithPath, destip, SecAccess_Write);
}
else
destfileWithPath.append(destfile).trim();
Expand Down Expand Up @@ -2979,15 +2969,6 @@ bool CFileSprayEx::onDFUWUFile(IEspContext &context, IEspDFUWUFileRequest &req,
return true;
}

static SecAccessFlags getDZPathScopePermissionsEx(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 @@ -3047,7 +3028,7 @@ bool CFileSprayEx::onFileList(IEspContext &context, IEspFileListRequest &req, IE
dropZoneName = findDropZonePlaneName(netaddr, sPath, dzName);
if (!isEmptyString(dropZoneName))
{
SecAccessFlags permission = getDZPathScopePermissionsEx(context, dropZoneName, sPath, netaddr, SecAccess_Read);
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));
Expand Down Expand Up @@ -3158,7 +3139,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 (getDZPathScopePermissionsEx(context, dropZoneName, dir, server, SecAccess_Read) < SecAccess_Read)
if (getDZPathScopePermsAndLegacyPhysicalPerms(context, dropZoneName, dir, server, SecAccess_Read) < SecAccess_Read)
return false;

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

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

bool directoryOnly = req.getDirectoryOnly();
Expand Down Expand Up @@ -3641,7 +3622,7 @@ void CFileSprayEx::checkDropZoneFileScopeAccess(IEspContext &context, const char
dropZoneName = findDropZonePlaneName(netAddress, dropZonePath, dzName);
if (!isEmptyString(dropZoneName))
{
SecAccessFlags permission = getDZPathScopePermissionsEx(context, dropZoneName, dropZonePath, netAddress, accessReq);
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);
Expand Down Expand Up @@ -3688,7 +3669,7 @@ void CFileSprayEx::checkDropZoneFileScopeAccess(IEspContext &context, const char
addPathSepChar(fullPath).append(pathToCheck);
//If the dropzone name is not found, the DZPathScopePermissions cannot be validated.
SecAccessFlags permission = isEmptyString(dropZoneName) ? accessReq
: getDZPathScopePermissionsEx(context, dropZoneName, fullPath, netAddress, accessReq);
: getDZPathScopePermsAndLegacyPhysicalPerms(context, dropZoneName, fullPath, netAddress, accessReq);
if (permission < accessReq)
{
uniquePath.setValue(pathToCheck.str(), false); //add a path denied
Expand Down
67 changes: 54 additions & 13 deletions esp/smc/SMCLib/TpCommon.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ static SecAccessFlags getDropZoneScopePermissions(IEspContext& context, const IP
return queryDistributedFileDirectory().getDropZoneScopePermissions(dropZone->queryProp("@name"), dropZonePath, userDesc);
}

extern TPWRAPPER_API SecAccessFlags getDZPathScopePermissions(IEspContext& context, const char* dropZoneName, const char* dropZonePath, const char* dropZoneHost)
static SecAccessFlags getDZPathScopePermissions(IEspContext& context, const char* dropZoneName, const char* dropZonePath, const char* dropZoneHost)
{
if (isEmptyString(dropZonePath))
throw makeStringException(ECLWATCH_INVALID_CLUSTER_NAME, "getDZPathScopePermissions(): DropZone path must be specified.");
Expand All @@ -207,7 +207,8 @@ extern TPWRAPPER_API SecAccessFlags getDZPathScopePermissions(IEspContext& conte
dropZone.setown(findDropZonePlane(dropZonePath, dropZoneHost, true, false));
if (!dropZone)
{
throwOrLogDropZoneLookUpError(ECLWATCH_INVALID_INPUT, "getDZPathScopePermissions(): DropZone %s not found.", dropZoneName);
throwOrLogDropZoneLookUpError(ECLWATCH_INVALID_INPUT, "getDZPathScopePermissions(): DropZone %s not found for host '%s' path '%s'.",
isEmptyString(dropZoneHost) ? "unspecified" : dropZoneHost, isEmptyString(dropZonePath) ? "unspecified" : dropZonePath);
return SecAccess_Full;
}
}
Expand All @@ -221,7 +222,7 @@ extern TPWRAPPER_API SecAccessFlags getDZPathScopePermissions(IEspContext& conte
return getDropZoneScopePermissions(context, dropZone, dropZonePath);
}

extern TPWRAPPER_API SecAccessFlags getDZFileScopePermissions(IEspContext& context, const char* dropZoneName, const char* dropZonePath,
static SecAccessFlags getDZFileScopePermissions(IEspContext& context, const char* dropZoneName, const char* dropZonePath,
const char* dropZoneHost)
{
StringBuffer dir, fileName;
Expand All @@ -230,8 +231,8 @@ 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)
static SecAccessFlags getLegacyDZPhysicalPerms(IEspContext& context, const char* dropZoneName, const char* dropZoneHost,
const char* dropZoneFile, bool isPath)
{
if (isEmptyString(dropZoneHost) && isEmptyString(dropZoneName))
throw makeStringException(ECLWATCH_INVALID_INPUT, "Neither dropzone plane or host specified.");
Expand All @@ -249,18 +250,58 @@ extern TPWRAPPER_API SecAccessFlags getLegacyDZPhysicalPerms(IEspContext& contex
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);
dlfn.setExternal(ep, dropZoneFile);

StringBuffer scopes;
const char *scopesToCheck = nullptr;
if (isPath)
scopesToCheck = dlfn.get();
else
{
dlfn.getScopes(scopes);
scopesToCheck = scopes;
}
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.
extern TPWRAPPER_API SecAccessFlags getDZPathScopePermsAndLegacyPhysicalPerms(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;
}

//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)
{
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);
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));
}

//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)
{
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));
}

//Validate dropzone file path and permission, Also set the dlfn.
extern TPWRAPPER_API void validateDropZoneAccess(IEspContext& context, const char* targetDZNameOrHost, const char* hostReq, SecAccessFlags permissionReq,
const char* fileNameWithRelPath, CDfsLogicalFileName& dlfn)
{
Expand Down
7 changes: 4 additions & 3 deletions esp/smc/SMCLib/TpWrapper.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,10 +241,11 @@ extern TPWRAPPER_API bool matchNetAddressRequest(const char* netAddressReg, bool

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 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);
extern TPWRAPPER_API SecAccessFlags getDZPathScopePermsAndLegacyPhysicalPerms(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);

#endif //_ESPWIZ_TpWrapper_HPP__

0 comments on commit a1ce672

Please sign in to comment.