Skip to content

Commit

Permalink
HPCC-32715 SSL_connect/accept should honor timeout provided
Browse files Browse the repository at this point in the history 
Signed-off-by: M Kelly <[email protected]>
  • Loading branch information
@mckellyln
mckellyln committed Sep 26, 2024
1 parent 64b2ec6 commit 1167a59
Show file tree
Hide file tree
Showing 15 changed files with 75 additions and 45 deletions.
2 changes: 1 addition & 1 deletion common/thorhelper/thorsoapcall.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2499,7 +2499,7 @@ class CWSCAsyncFor : implements IWSCAsyncFor, public CInterface, public CAsyncFo
if (ssock)
{
checkTimeLimitExceeded(&remainingMS);
int status = ssock->secure_connect();
int status = ssock->secure_connect(remainingMS);
if (status < 0)
{
StringBuffer err;
Expand Down
4 changes: 3 additions & 1 deletion esp/bindings/http/client/httpclient.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -310,12 +310,14 @@ int CHttpClient::connect(StringBuffer& errmsg, bool forceNewConnection)
m_isPersistentSocket = false;
try
{
CCycleTimer timer;
m_socket = ISocket::connect_timeout(ep, m_connectTimeoutMs);

if(strcmp(m_protocol.get(), "HTTPS") == 0)
{
ISecureSocket* securesocket = m_ssctx->createSecureSocket(m_socket, SSLogNormal, m_host.str());
int res = securesocket->secure_connect();
unsigned remainingMs = timer.remainingMs(m_connectTimeoutMs);
int res = securesocket->secure_connect(remainingMs);
if(res < 0)
{
close();
Expand Down
2 changes: 1 addition & 1 deletion esp/bindings/http/platform/httpprot.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ bool CHttpThread::onRequest()
try
{
ESPLOG(LogMax, "Accepting from secure socket");
res = secure_sock->secure_accept(logLevel);
res = secure_sock->secure_accept();
if(res < 0)
return false;
}
Expand Down
4 changes: 3 additions & 1 deletion esp/clients/roxiecontrol.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ IPropertyTree *sendRoxieControlAllNodes(const SocketEndpoint &ep, const char *ms
static ISocket *createRoxieControlSocket(ISmartSocketFactory *conn, unsigned wait, unsigned connect_wait)
{
const SocketEndpoint &ep = conn->nextEndpoint();
CCycleTimer timer;
Owned<ISocket> sock = ISocket::connect_timeout(ep, connect_wait);
if (conn->isTlsService())
{
Expand All @@ -137,7 +138,8 @@ static ISocket *createRoxieControlSocket(ISmartSocketFactory *conn, unsigned wai
if (!ssock)
throw makeStringException(SECURE_CONNECTION_FAILURE, "failed creating secure socket for roxie control message");

int status = ssock->secure_connect();
unsigned remainingMs = timer.remainingMs(connect_wait);
int status = ssock->secure_connect(remainingMs);
if (status < 0)
{
StringBuffer err;
Expand Down
25 changes: 18 additions & 7 deletions fs/dafsclient/rmtclient.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -727,13 +727,16 @@ void CRemoteBase::connectSocket(SocketEndpoint &ep, unsigned connectTimeoutMs, u
//PrintStackReport();
}
bool ok = true;
unsigned connecttimeoutMs = DEFAULT_CONNECT_TIME;
try
{
CCycleTimer timer;
if (tm.timemon)
{
unsigned remaining;
if (tm.timemon->timedout(&remaining))
THROWJSOCKEXCEPTION(JSOCKERR_connection_failed);
connecttimeoutMs = remaining;
socket.setown(ISocket::connect_timeout(ep,remaining));
}
else
Expand Down Expand Up @@ -767,7 +770,8 @@ void CRemoteBase::connectSocket(SocketEndpoint &ep, unsigned connectTimeoutMs, u
}
else
ssock.setown(createSecureSocket(socket.getClear(), nullptr));
int status = ssock->secure_connect();
unsigned remainingMs = timer.remainingMs(connecttimeoutMs);
int status = ssock->secure_connect(remainingMs);
if (status < 0)
throw createDafsException(DAFSERR_connection_failed, "Failure to establish secure connection");
socket.setown(ssock.getLink());
Expand Down Expand Up @@ -1128,7 +1132,7 @@ IDaFsConnection *createDaFsConnection(const SocketEndpoint &ep, DAFSConnectCfg c

/////////////////////////

ISocket *checkSocketSecure(ISocket *socket)
ISocket *checkSocketSecure(ISocket *socket, unsigned timeoutms = DEFAULT_CONNECT_TIME)
{
if (securitySettings.queryConnectMethod() == SSLNone)
return LINK(socket);
Expand All @@ -1144,7 +1148,7 @@ ISocket *checkSocketSecure(ISocket *socket)
try
{
ssock.setown(createSecureSocket(LINK(socket), nullptr));
int status = ssock->secure_connect();
int status = ssock->secure_connect(timeoutms);
if (status < 0)
throw createDafsException(DAFSERR_connection_failed, "Failure to establish secure connection");
return ssock.getClear();
Expand Down Expand Up @@ -1173,6 +1177,7 @@ ISocket *connectDafs(SocketEndpoint &ep, unsigned timeoutms, const IPropertyTree

if (isContainerized())
{
CCycleTimer timer;
socket.setown(ISocket::connect_timeout(ep, timeoutms));

if (service && service->getPropBool("@tls"))
Expand All @@ -1182,7 +1187,8 @@ ISocket *connectDafs(SocketEndpoint &ep, unsigned timeoutms, const IPropertyTree
try
{
ssock.setown(createSecureSocket(LINK(socket), service->queryProp("@issuer")));
int status = ssock->secure_connect();
unsigned remainingMs = timer.remainingMs(timeoutms);
int status = ssock->secure_connect(remainingMs);
if (status < 0)
throw createDafsException(DAFSERR_connection_failed, "Failure to establish secure connection to dafilesrv");
return ssock.getClear();
Expand Down Expand Up @@ -1214,12 +1220,15 @@ ISocket *connectDafs(SocketEndpoint &ep, unsigned timeoutms, const IPropertyTree
{
if ( (securitySettings.queryConnectMethod() == SSLNone) || (securitySettings.queryConnectMethod() == SSLOnly) || (securitySettings.queryConnectMethod() == UnsecureAndSSL))
{
CCycleTimer timer;
socket.setown(ISocket::connect_timeout(ep, timeoutms));
return checkSocketSecure(socket);
unsigned remainingMs = timer.remainingMs(timeoutms);
return checkSocketSecure(socket, remainingMs);
}

// SSLFirst or UnsecureFirst ...

unsigned remainingMs;
unsigned newtimeout = timeoutms;
if (newtimeout > 5000)
newtimeout = 5000;
Expand All @@ -1229,10 +1238,12 @@ ISocket *connectDafs(SocketEndpoint &ep, unsigned timeoutms, const IPropertyTree
{
conAttempts--;
bool connected = false;
CCycleTimer timer;
try
{
socket.setown(ISocket::connect_timeout(ep, newtimeout));
connected = true;
remainingMs = timer.remainingMs(newtimeout);
newtimeout = timeoutms;
}
catch (IJSOCK_Exception *e)
Expand All @@ -1257,7 +1268,7 @@ ISocket *connectDafs(SocketEndpoint &ep, unsigned timeoutms, const IPropertyTree
{
try
{
return checkSocketSecure(socket);
return checkSocketSecure(socket, remainingMs);
}
catch (IDAFS_Exception *e)
{
Expand Down Expand Up @@ -1343,7 +1354,7 @@ unsigned getRemoteVersion(ISocket *origSock, StringBuffer &ver)
if (!origSock)
return 0;

Owned<ISocket> socket = checkSocketSecure(origSock);
Owned<ISocket> socket = checkSocketSecure(origSock, 10000);

unsigned ret;
MemoryBuffer sendbuf;
Expand Down
2 changes: 1 addition & 1 deletion fs/dafsclient/rmtclient.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ extern DAFSCLIENT_API int getDafsInfo(ISocket * socket, unsigned level, StringBu
extern DAFSCLIENT_API void setDafsEndpointPort(SocketEndpoint &ep);
extern DAFSCLIENT_API void setDafsLocalMountRedirect(const IpAddress &ip,const char *dir,const char *mountdir);
extern DAFSCLIENT_API ISocket *connectDafs(SocketEndpoint &ep, unsigned timeoutms, const IPropertyTree *service); // NOTE: might alter ep.port if configured for multiple ports ...
extern DAFSCLIENT_API ISocket *checkSocketSecure(ISocket *socket);
extern DAFSCLIENT_API ISocket *checkSocketSecure(ISocket *socket, unsigned timeoutms);
extern DAFSCLIENT_API unsigned short getActiveDaliServixPort(const IpAddress &ip);
extern DAFSCLIENT_API unsigned getDaliServixVersion(const IpAddress &ip,StringBuffer &ver);
extern DAFSCLIENT_API unsigned getDaliServixVersion(const SocketEndpoint &ep,StringBuffer &ver);
Expand Down
2 changes: 1 addition & 1 deletion roxie/ccd/ccdlistener.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ class CascadeManager : public CInterface
if (!ssock)
throw makeStringException(ROXIE_TLS_ERROR, "Roxie CascadeManager failed creating secure socket for roxie control message");

int status = ssock->secure_connect();
int status = ssock->secure_connect(2000);
if (status < 0)
{
StringBuffer err;
Expand Down
4 changes: 2 additions & 2 deletions roxie/ccd/ccdprotocol.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -291,11 +291,11 @@ class ProtocolSocketListener : public ProtocolListener
Owned<ISecureSocket> ssock;
try
{
ssock.setown(secureContext->createSecureSocket(base));
int loglevel = SSLogMin;
if (doTrace(traceSockets))
loglevel = SSLogMax;
int status = ssock->secure_accept(loglevel);
ssock.setown(secureContext->createSecureSocket(base, loglevel));
int status = ssock->secure_accept();
if (status < 0)
{
// secure_accept may also DBGLOG() errors ...
Expand Down
2 changes: 0 additions & 2 deletions system/jlib/jsocket.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,6 @@
#define CONNECT_TIMEOUT_REFUSED_WAIT 1000 // maximum to sleep on connect_timeout
#define TRACE_SLOW_BLOCK_TRANSFER

#define DEFAULT_CONNECT_TIME (100*1000) // for connect_wait

#ifdef _DEBUG
// #define SIMULATE_LOST_UDP_PACKETS
#endif
Expand Down
2 changes: 2 additions & 0 deletions system/jlib/jsocket.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@
#define WAIT_FOREVER ((unsigned)-1)
#endif

#define DEFAULT_CONNECT_TIME (100*1000) // for connect_wait

enum JSOCKET_ERROR_CODES {
JSOCKERR_ok = 0,
JSOCKERR_not_opened = -1, // accept,name,peer_name,read,write
Expand Down
11 changes: 7 additions & 4 deletions system/mp/mpcomm.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1099,16 +1099,19 @@ protected: friend class CMPPacketReader;
}
if (remaining<10000)
remaining = 10000; // 10s min granularity for MP

CCycleTimer timer;
newsock.setown(ISocket::connect_timeout(remoteep,remaining));

#if defined(_USE_OPENSSL)
if (parent->useTLS)
{
Owned<ISecureSocket> ssock = secureContextClient->createSecureSocket(newsock.getClear());
int tlsTraceLevel = SSLogMin;
if (parent->mpTraceLevel >= MPVerboseMsgThreshold)
tlsTraceLevel = SSLogMax;
int status = ssock->secure_connect(tlsTraceLevel);
Owned<ISecureSocket> ssock = secureContextClient->createSecureSocket(newsock.getClear(), tlsTraceLevel);
tm.timedout(&remaining);
int status = ssock->secure_connect(remaining);
if (status < 0)
{
ssock->close();
Expand Down Expand Up @@ -2567,11 +2570,11 @@ int CMPConnectThread::run()
#if defined(_USE_OPENSSL)
if (parent->useTLS)
{
Owned<ISecureSocket> ssock = secureContextServer->createSecureSocket(sock.getClear());
int tlsTraceLevel = SSLogMin;
if (parent->mpTraceLevel >= MPVerboseMsgThreshold)
tlsTraceLevel = SSLogMax;
int status = ssock->secure_accept(tlsTraceLevel);
Owned<ISecureSocket> ssock = secureContextServer->createSecureSocket(sock.getClear(), tlsTraceLevel);
int status = ssock->secure_accept(10000);
if (status < 0)
{
ssock->close();
Expand Down
Loading

0 comments on commit 1167a59

Please sign in to comment.