Skip to content

Commit

Permalink
HPCC-30259 Use secrets for dafilesrv client connections
Browse files Browse the repository at this point in the history 
Signed-off-by: Jake Smith <[email protected]>
  • Loading branch information
@jakesmith
jakesmith committed Sep 26, 2023
1 parent 2b038c1 commit 3931cd2
Show file tree
Hide file tree
Showing 6 changed files with 188 additions and 42 deletions.
32 changes: 32 additions & 0 deletions dali/base/dafdesc.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1040,6 +1040,7 @@ class CFileDescriptor: public CFileDescriptorBase, implements ISuperFileDescrip

SocketEndpointArray *pending; // for constructing cluster group
Owned<IStoragePlane> remoteStoragePlane;
std::vector<std::string> dafileSrvEndpoints;
bool setupdone;
byte version;

Expand Down Expand Up @@ -1398,6 +1399,32 @@ class CFileDescriptor: public CFileDescriptorBase, implements ISuperFileDescrip
}
}

// mapDafileSrvSecrets is a CFileDescriptor is created if it is associated with a remoteStoragePlane.
// Identify the target dafilesrv location urls a secret based connections in the dafilesrv hook
// NB: the expectation is that they'll only be 1 target service dafilesrv URL
// These will remain associated in the hook, until this CFileDescriptor object is destroyed, and removeMappedDafileSrvSecrets is called.
void mapDafileSrvSecrets(IClusterInfo &cluster)
{
Owned<INodeIterator> groupIter = cluster.queryGroup()->getIterator();

ForEach(*groupIter)
{
INode &node = groupIter->query();
StringBuffer endpointString;
node.endpoint().getEndpointHostText(endpointString);
auto it = std::find(dafileSrvEndpoints.begin(), dafileSrvEndpoints.end(), endpointString.str());
if (it == dafileSrvEndpoints.end())
dafileSrvEndpoints.push_back(endpointString.str());
}
for (auto &dafileSrvEp: dafileSrvEndpoints)
queryDaFileSrvHook()->addSecretUrl(dafileSrvEp.c_str());
}
void removeMappedDafileSrvSecrets()
{
for (auto &dafileSrvEp: dafileSrvEndpoints)
queryDaFileSrvHook()->removeSecretUrl(dafileSrvEp.c_str());
}

public:
IMPLEMENT_IINTERFACE;

Expand Down Expand Up @@ -1482,6 +1509,8 @@ class CFileDescriptor: public CFileDescriptorBase, implements ISuperFileDescrip
{
assertex(1 == clusters.ordinality()); // only one cluster per logical remote file supported/will have resolved to 1
remoteStoragePlane.setown(createStoragePlane(remoteStoragePlaneMeta));
if (attr->getPropBool("@_remoteSecure"))
mapDafileSrvSecrets(clusters.item(0));
}
}
else
Expand Down Expand Up @@ -1613,6 +1642,8 @@ class CFileDescriptor: public CFileDescriptorBase, implements ISuperFileDescrip
assertex(1 == clusters.ordinality()); // only one cluster per logical remote file supported/will have resolved to 1
remoteStoragePlane.setown(createStoragePlane(remoteStoragePlaneMeta));
clusters.item(0).applyPlane(remoteStoragePlane);
if (attr->getPropBool("@_remoteSecure"))
mapDafileSrvSecrets(clusters.item(0));
}
}

Expand Down Expand Up @@ -1707,6 +1738,7 @@ class CFileDescriptor: public CFileDescriptorBase, implements ISuperFileDescrip

virtual ~CFileDescriptor()
{
removeMappedDafileSrvSecrets();
closePending(); // not sure strictly needed
ForEachItemInRev(p, parts)
delpart(p);
Expand Down
15 changes: 12 additions & 3 deletions esp/clients/ws_dfsclient/ws_dfsclient.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -321,9 +321,9 @@ class CServiceDistributedFile : public CServiceDistributedFileBase<IDistributedF
IPropertyTree *file = dfsFile->queryFileMeta()->queryPropTree("File");

const char *remoteName = dfsFile->queryRemoteName(); // NB: null if local
IPropertyTree *dafileSrvRemoteFilePlane = nullptr;
if (!isEmptyString(remoteName))
{
IPropertyTree *dafileSrvRemoteFilePlane = nullptr;
Owned<IPropertyTree> remoteStorage = getRemoteStorage(remoteName);
if (!remoteStorage)
throw makeStringExceptionV(0, "Remote storage '%s' not found", remoteName);
Expand Down Expand Up @@ -394,10 +394,19 @@ class CServiceDistributedFile : public CServiceDistributedFileBase<IDistributedF
file->setProp("@directory", newPath.str());
}
}
if (dafileSrvRemoteFilePlane)
{
file->setPropTree("Attr/_remoteStoragePlane", createPTreeFromIPT(dafileSrvRemoteFilePlane));
if (remoteStorage->hasProp("@secret"))
{
// if remote storage service is secure, dafilesrv connections must be also.
// this flag is used by consumers of this IFleDescriptor to tell whether they need to make
// secure secret based connections to the dafilesrv's
file->setPropBool("Attr/@_remoteSecure", true);
}
}
}
AccessMode accessMode = static_cast<AccessMode>(dfsFile->queryCommonMeta()->getPropInt("@accessMode"));
if (dafileSrvRemoteFilePlane)
file->setPropTree("Attr/_remoteStoragePlane", createPTreeFromIPT(dafileSrvRemoteFilePlane));
fileDesc.setown(deserializeFileDescriptorTree(file));
fileDesc->setTraceName(logicalName);
// NB: the accessMode is being defined by the client call, and has been stored in the IDFSFile common meta
Expand Down
98 changes: 65 additions & 33 deletions fs/dafsclient/rmtclient.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -667,7 +667,7 @@ void clientAddSocketToCache(SocketEndpoint &ep,ISocket *socket)

//---------------------------------------------------------------------------

void CRemoteBase::connectSocket(SocketEndpoint &ep, unsigned connectTimeoutMs, unsigned connectRetries)
void CRemoteBase::connectSocket(SocketEndpoint &ep, unsigned connectTimeoutMs, unsigned connectRetries, bool secure)
{
if (!connectTimeoutMs)
connectTimeoutMs = dafsConnectTimeoutMs;
Expand Down Expand Up @@ -696,10 +696,7 @@ void CRemoteBase::connectSocket(SocketEndpoint &ep, unsigned connectTimeoutMs, u
if (TF_TRACE_CLIENT_CONN)
{
ep.getEndpointHostText(eps);
if (ep.port == securitySettings.queryDaFileSrvSSLPort())
PROGLOG("Connecting SECURE to %s", eps.str());
else
PROGLOG("Connecting to %s", eps.str());
PROGLOG("Connecting %sto %s", secure?"SECURE ":"", eps.str());
//PrintStackReport();
}
bool ok = true;
Expand All @@ -714,13 +711,36 @@ void CRemoteBase::connectSocket(SocketEndpoint &ep, unsigned connectTimeoutMs, u
}
else
socket.setown(ISocket::connect(ep));
if (ep.port == securitySettings.queryDaFileSrvSSLPort())
if (secure)
{
#ifdef _USE_OPENSSL
Owned<ISecureSocket> ssock;
try
{
ssock.setown(createSecureSocket(socket.getClear(), nullptr));
// instead of creating CRemoteFile/CRemoteBase with a secret, it could instead lookup, and create the ISecureSocketContext and pass that
// into the ctor. That would avoid having to look the secret up, extract the certs and recreate the context on each connection.
// The context would be 'owned' by the hook, and would expire when the mappings are removed (when removeMappedDafileSrvSecrets is called).
if (storageSecret)
{
Owned<IPropertyTree> secretPTree = getSecret("storage", storageSecret);
if (!secretPTree)
throw makeStringExceptionV(-1, "secret %s.%s not found", "storage", storageSecret.str());

StringBuffer certSecretBuf;
getSecretKeyValue(certSecretBuf, secretPTree, "tls.crt");

StringBuffer privKeySecretBuf;
getSecretKeyValue(privKeySecretBuf, secretPTree, "tls.key");

Owned<ISecureSocketContext> secureContext = createSecureSocketContextEx(certSecretBuf, privKeySecretBuf, nullptr, ClientSocket);
int loglevel = SSLogNormal;
#ifdef _DEBUG
loglevel = SSLogMax;
#endif
ssock.setown(secureContext->createSecureSocket(socket.getClear(), loglevel));
}
else
ssock.setown(createSecureSocket(socket.getClear(), nullptr));
int status = ssock->secure_connect();
if (status < 0)
throw createDafsException(DAFSERR_connection_failed, "Failure to establish secure connection");
Expand Down Expand Up @@ -787,10 +807,7 @@ void CRemoteBase::connectSocket(SocketEndpoint &ep, unsigned connectTimeoutMs, u
if (!timeExpired)
{
Sleep(sleeptime); // prevent multiple retries beating
if (ep.port == securitySettings.queryDaFileSrvSSLPort())
PROGLOG("Retrying SECURE connect");
else
PROGLOG("Retrying connect");
PROGLOG("Retrying %sconnect", secure?"SECURE ":"");
}
}

Expand Down Expand Up @@ -879,35 +896,41 @@ void CRemoteBase::sendRemoteCommand(MemoryBuffer & src, MemoryBuffer & reply, bo

if (!socket)
{
bool doConnect = true;
if (connectMethod == SSLFirst || connectMethod == UnsecureFirst)
if (storageSecret)
connectSocket(tep, 0, INFINITE, true);
else
{
// MCK - could maintain a list of 100 or so previous endpoints and if connection failed
// then mark port down for a delay (like 15 min above) to avoid having to try every time ...
try
{
connectSocket(tep, 5000, 0);
doConnect = false;
}
catch (IDAFS_Exception *e)
bool doConnect = true;
if (connectMethod == SSLFirst || connectMethod == UnsecureFirst)
{
if (e->errorCode() == DAFSERR_connection_failed)
bool secure = tep.port == securitySettings.queryDaFileSrvSSLPort();
// MCK - could maintain a list of 100 or so previous endpoints and if connection failed
// then mark port down for a delay (like 15 min above) to avoid having to try every time ...
try
{
connectSocket(tep, 5000, 0);
doConnect = false;
}
catch (IDAFS_Exception *e)
{
unsigned prevPort = tep.port;
if (prevPort == securitySettings.queryDaFileSrvSSLPort())
tep.port = securitySettings.queryDaFileSrvPort();
if (e->errorCode() == DAFSERR_connection_failed)
{
unsigned prevPort = tep.port;
if (secure)
tep.port = securitySettings.queryDaFileSrvPort();
else
tep.port = securitySettings.queryDaFileSrvSSLPort();
WARNLOG("Connect failed on port %d, retrying on port %d", prevPort, tep.port);
doConnect = true;
e->Release();
}
else
tep.port = securitySettings.queryDaFileSrvSSLPort();
WARNLOG("Connect failed on port %d, retrying on port %d", prevPort, tep.port);
doConnect = true;
e->Release();
throw e;
}
else
throw e;
}
if (doConnect)
connectSocket(tep, 0, INFINITE, tep.port == securitySettings.queryDaFileSrvSSLPort());
}
if (doConnect)
connectSocket(tep);
}
}

Expand Down Expand Up @@ -969,6 +992,10 @@ CRemoteBase::CRemoteBase(const SocketEndpoint &_ep, const char * _filename)
: filename(_filename)
{
ep = _ep;

StringBuffer endpointStr;
ep.getEndpointHostText(endpointStr);

connectMethod = securitySettings.queryConnectMethod();
}

Expand All @@ -979,6 +1006,11 @@ CRemoteBase::CRemoteBase(const SocketEndpoint &_ep, DAFSConnectCfg _connectMetho
connectMethod = _connectMethod;
}

CRemoteBase::CRemoteBase(const SocketEndpoint &_ep, const char *_storageSecret, const char * _filename)
: ep(_ep), storageSecret(_storageSecret), filename(_filename)
{
}

void CRemoteBase::disconnect()
{
CriticalBlock block(crit);
Expand Down
7 changes: 5 additions & 2 deletions fs/dafsclient/rmtclient_impl.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,22 +153,25 @@ class CRemoteBase : public CSimpleInterfaceOf<IDaFsConnection>
static SocketEndpoint lastfailep;
static unsigned lastfailtime;
static CriticalSection lastFailEpCrit;
DAFSConnectCfg connectMethod;
DAFSConnectCfg connectMethod = SSLNone;

void connectSocket(SocketEndpoint &ep, unsigned connectTimeoutMs=0, unsigned connectRetries=INFINITE);
void connectSocket(SocketEndpoint &ep, unsigned connectTimeoutMs=0, unsigned connectRetries=INFINITE, bool secure=false);
void killSocket(SocketEndpoint &tep);

protected: friend class CRemoteFileIO;

StringAttr filename;
CriticalSection crit;
SocketEndpoint ep;
StringBuffer storageSecret;

void sendRemoteCommand(MemoryBuffer & src, MemoryBuffer & reply, bool retry=true, bool lengthy=false, bool handleErrCode=true);
void sendRemoteCommand(MemoryBuffer & src, bool retry);
public:
CRemoteBase(const SocketEndpoint &_ep, const char * _filename);
CRemoteBase(const SocketEndpoint &_ep, DAFSConnectCfg _connectMethod, const char * _filename);
CRemoteBase(const SocketEndpoint &_ep, const char *_storageSecret, const char * _filename);

void disconnect();
const char *queryLocalName()
{
Expand Down
Loading

0 comments on commit 3931cd2

Please sign in to comment.