Skip to content

Commit

Permalink
HPCC-29482 WsDfs should pass user to DFS::getFileTree
Browse files Browse the repository at this point in the history
When performing a ~remote file lookup, the WsDfs service looks
up the logical file in Dali (via
IDistributedFileDirectory::getFileTree).
It should pass the active esp context user, but passes null.
This can cause access denials.

Signed-off-by: Jake Smith <[email protected]>
  • Loading branch information
jakesmith committed Apr 5, 2024
1 parent 2f6621c commit 0f2fb3b
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 9 deletions.
6 changes: 3 additions & 3 deletions dali/base/dadfs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1256,7 +1256,7 @@ protected: friend class CDistributedFile;
return ret;
}
virtual bool removePhysicalPartFiles(const char *logicalName, IFileDescriptor *fileDesc, IMultiException *mexcept, unsigned numParallelDeletes=0) override;
virtual void setFileAccessed(const char *logicalName, const CDateTime &dt, const INode *foreigndali, unsigned foreigndalitimeout);
virtual void setFileAccessed(IUserDescriptor* udesc, const char *logicalName, const CDateTime &dt, const INode *foreigndali, unsigned foreigndalitimeout);
};


Expand Down Expand Up @@ -11635,11 +11635,11 @@ void CDistributedFileDirectory::setFileAccessed(CDfsLogicalFileName &dlfn,IUserD
checkDfsReplyException(mb);
}

void CDistributedFileDirectory::setFileAccessed(const char *logicalName, const CDateTime &dt, const INode *foreigndali, unsigned foreigndalitimeout)
void CDistributedFileDirectory::setFileAccessed(IUserDescriptor* udesc, const char *logicalName, const CDateTime &dt, const INode *foreigndali, unsigned foreigndalitimeout)
{
CDfsLogicalFileName dlfn;
dlfn.set(logicalName);
setFileAccessed(dlfn, nullptr, dt, foreigndali, foreigndalitimeout);
setFileAccessed(dlfn, udesc, dt, foreigndali, foreigndalitimeout);
}

void CDistributedFileDirectory::setFileProtect(CDfsLogicalFileName &dlfn,IUserDescriptor *user, const char *owner, bool set, const INode *foreigndali,unsigned foreigndalitimeout)
Expand Down
2 changes: 1 addition & 1 deletion dali/base/dadfs.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -731,7 +731,7 @@ interface IDistributedFileDirectory: extends IInterface
// useful to clearup after temporary unpublished file.
virtual bool removePhysicalPartFiles(const char *logicalName, IFileDescriptor *fileDesc, IMultiException *mexcept, unsigned numParallelDeletes=0) = 0;

virtual void setFileAccessed(const char *logicalName, const CDateTime &dt, const INode *foreigndali=nullptr, unsigned foreigndalitimeout=FOREIGN_DALI_TIMEOUT) = 0;
virtual void setFileAccessed(IUserDescriptor* udesc, const char *logicalName, const CDateTime &dt, const INode *foreigndali=nullptr, unsigned foreigndalitimeout=FOREIGN_DALI_TIMEOUT) = 0;
};


Expand Down
10 changes: 5 additions & 5 deletions esp/services/ws_dfsservice/ws_dfsservice.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ enum LfnMetaOpts : byte
LfnMOptTls = 0x02,
};
BITMASK_ENUM(LfnMetaOpts);
static void populateLFNMeta(const char *logicalName, unsigned __int64 leaseId, LfnMetaOpts opts, IPropertyTree *metaRoot, IPropertyTree *meta)
static void populateLFNMeta(IUserDescriptor *userDesc, const char *logicalName, unsigned __int64 leaseId, LfnMetaOpts opts, IPropertyTree *metaRoot, IPropertyTree *meta)
{
CDfsLogicalFileName lfn;
lfn.set(logicalName);
Expand All @@ -59,7 +59,7 @@ static void populateLFNMeta(const char *logicalName, unsigned __int64 leaseId, L

assertex(!lfn.isMulti()); // not supported, don't think needs to be/will be.

Owned<IPropertyTree> tree = queryDistributedFileDirectory().getFileTree(logicalName, nullptr);
Owned<IPropertyTree> tree = queryDistributedFileDirectory().getFileTree(logicalName, userDesc);
if (!tree)
return;
if (hasMask(opts, LfnMOptRemap))
Expand Down Expand Up @@ -111,7 +111,7 @@ static void populateLFNMeta(const char *logicalName, unsigned __int64 leaseId, L
{
IPropertyTree &sub = *(orderedSubFiles[f]);
sub.getProp("@name", subname.clear());
populateLFNMeta(subname, leaseId, opts, metaRoot, fileMeta);
populateLFNMeta(userDesc, subname, leaseId, opts, metaRoot, fileMeta);
}
}
}
Expand Down Expand Up @@ -182,7 +182,7 @@ bool CWsDfsEx::onDFSFileLookup(IEspContext &context, IEspDFSFileLookupRequest &r
opts |= LfnMOptTls;

Owned<IPropertyTree> responseTree = createPTree();
populateLFNMeta(logicalName, leaseId, opts, responseTree, responseTree);
populateLFNMeta(userDesc, logicalName, leaseId, opts, responseTree, responseTree);

// serialize response
MemoryBuffer respMb, compressedRespMb;
Expand All @@ -198,7 +198,7 @@ bool CWsDfsEx::onDFSFileLookup(IEspContext &context, IEspDFSFileLookupRequest &r
// Really this should be done at end (or at end as well), but this is same as existing DFS lookup.
CDateTime dt;
dt.setNow();
queryDistributedFileDirectory().setFileAccessed(logicalName, dt);
queryDistributedFileDirectory().setFileAccessed(userDesc, logicalName, dt);

LOG(MCauditInfo,",FileAccess,EspProcess,READ,%s,%u,%s", logicalName, timeoutSecs, userID.str());
}
Expand Down

0 comments on commit 0f2fb3b

Please sign in to comment.