Skip to content

Commit

Permalink
HPCC-30037 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.

Revise based on review
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().
6. 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.
7. Rename the validateDZFileScopePermissions() to
   validateDZFileScopePermsAndLegacyPhysicalPerms()
8. Consoliate the code in FileSprayEx::onFileList() by using the
getDropZoneAndValidateHostAndPath().
9. Add validateDZPathScopePermsAndLegacyPhysicalPerms() which contains the common
code for the validateDropZoneReq() and WsFileSprayEx.
10. Revise the code in validateDropZoneReq().
11. Move the addPathSepChar() line, fix an intent bug, and fix a message bug.
12. Add more comments.

Signed-off-by: wangkx <[email protected]>
  • Loading branch information
wangkx committed Sep 22, 2023
1 parent a4ec440 commit f2e53c0
Show file tree
Hide file tree
Showing 7 changed files with 175 additions and 81 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
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
17 changes: 3 additions & 14 deletions esp/services/ws_fs/ws_fsBinding.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -410,16 +410,11 @@ int CFileSpraySoapBindingEx::downloadFile(IEspContext &context, CHttpRequest* re
if (!osStr.isEmpty() && (atoi(osStr.str())== OS_WINDOWS))
pathSep = '\\';
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;
addPathSepChar(pathStr);
fullName.appendf("%s%s", pathStr.str(), nameStr.str());

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

if (!sourcePathReq.isEmpty())
Expand Down Expand Up @@ -2585,10 +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);
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));
validateDZFileScopePermsAndLegacyPhysicalPerms(context, destPlane, destfileWithPath, destip, SecAccess_Write);
}
else
destfileWithPath.append(destfile).trim();
Expand Down Expand Up @@ -3029,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 = 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));
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 @@ -3055,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 @@ -3145,7 +3132,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 (getDZPathScopePermsAndLegacyPhysicalPerms(context, dropZoneName, dir, server, SecAccess_Read) < SecAccess_Read)
return false;

RemoteFilename rfn;
Expand Down Expand Up @@ -3366,7 +3353,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 (getDZPathScopePermsAndLegacyPhysicalPerms(context, dropZoneName, fullPath, host, SecAccess_Read) < SecAccess_Read)
continue;
}

Expand Down Expand Up @@ -3477,7 +3464,7 @@ 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)
if (!isEmptyString(dzName) && (getDZPathScopePermsAndLegacyPhysicalPerms(context, dzName, directoryStr, netAddress, SecAccess_Read) < SecAccess_Read))
return false;

bool directoryOnly = req.getDirectoryOnly();
Expand Down Expand Up @@ -3627,12 +3614,7 @@ void CFileSprayEx::checkDropZoneFileScopeAccess(IEspContext &context, const char
if (isEmptyString(dropZoneName))
dropZoneName = findDropZonePlaneName(netAddress, dropZonePath, dzName);
if (!isEmptyString(dropZoneName))
{
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, context.queryUserId(), getSecAccessFlagName(permission), accessReqName);
}
validateDZPathScopePermsAndLegacyPhysicalPerms(context, dropZoneName, dropZonePath, netAddress, accessReq);

RemoteFilename rfn;
SocketEndpoint ep(netAddress);
Expand Down Expand Up @@ -3674,7 +3656,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
: getDZPathScopePermsAndLegacyPhysicalPerms(context, dropZoneName, fullPath, netAddress, accessReq);
if (permission < accessReq)
{
uniquePath.setValue(pathToCheck.str(), false); //add a path denied
Expand Down
Loading

0 comments on commit f2e53c0

Please sign in to comment.