From c7efbf5b8e2290c8c97ea4159ff246ef3cd7a937 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Tue, 17 Sep 2024 10:26:55 +0200 Subject: [PATCH] [core,proxy] fix nonblocking BIO reads * In case of non-blocking BIO layers the proxy read functions bailed out with an error. Retry reading in that case unless the TcpConnectTimeout is exceeded * Terminate proxy read operations if rdpContext::abortEvent is set --- libfreerdp/core/gateway/arm.c | 2 +- libfreerdp/core/gateway/rdg.c | 2 +- libfreerdp/core/gateway/rpc.c | 2 +- libfreerdp/core/gateway/wst.c | 2 +- libfreerdp/core/proxy.c | 67 ++++++++++++++++++++++++++++------- libfreerdp/core/proxy.h | 2 +- libfreerdp/core/transport.c | 4 +-- 7 files changed, 61 insertions(+), 20 deletions(-) diff --git a/libfreerdp/core/gateway/arm.c b/libfreerdp/core/gateway/arm.c index 7b023d518..886e3914e 100644 --- a/libfreerdp/core/gateway/arm.c +++ b/libfreerdp/core/gateway/arm.c @@ -124,7 +124,7 @@ static BOOL arm_tls_connect(rdpArm* arm, rdpTls* tls, int timeout) if (isProxyConnection) { - if (!proxy_connect(settings, bufferedBio, proxyUsername, proxyPassword, + if (!proxy_connect(arm->context, bufferedBio, proxyUsername, proxyPassword, freerdp_settings_get_string(settings, FreeRDP_GatewayHostname), (UINT16)freerdp_settings_get_uint32(settings, FreeRDP_GatewayPort))) { diff --git a/libfreerdp/core/gateway/rdg.c b/libfreerdp/core/gateway/rdg.c index c1046b6af..26a4d8672 100644 --- a/libfreerdp/core/gateway/rdg.c +++ b/libfreerdp/core/gateway/rdg.c @@ -1301,7 +1301,7 @@ static BOOL rdg_tls_connect(rdpRdg* rdg, rdpTls* tls, const char* peerAddress, i if (isProxyConnection) { - if (!proxy_connect(settings, bufferedBio, proxyUsername, proxyPassword, + if (!proxy_connect(rdg->context, bufferedBio, proxyUsername, proxyPassword, settings->GatewayHostname, (UINT16)settings->GatewayPort)) { BIO_free_all(bufferedBio); diff --git a/libfreerdp/core/gateway/rpc.c b/libfreerdp/core/gateway/rpc.c index 41e7a63da..53c139f6b 100644 --- a/libfreerdp/core/gateway/rpc.c +++ b/libfreerdp/core/gateway/rpc.c @@ -759,7 +759,7 @@ static BOOL rpc_channel_tls_connect(RpcChannel* channel, UINT32 timeout) if (channel->client->isProxy) { - if (!proxy_connect(settings, bufferedBio, proxyUsername, proxyPassword, + if (!proxy_connect(context, bufferedBio, proxyUsername, proxyPassword, settings->GatewayHostname, settings->GatewayPort)) { BIO_free_all(bufferedBio); diff --git a/libfreerdp/core/gateway/wst.c b/libfreerdp/core/gateway/wst.c index 6de52a26a..d7ab58dba 100644 --- a/libfreerdp/core/gateway/wst.c +++ b/libfreerdp/core/gateway/wst.c @@ -254,7 +254,7 @@ static BOOL wst_tls_connect(rdpWst* wst, rdpTls* tls, int timeout) if (isProxyConnection) { - if (!proxy_connect(settings, bufferedBio, proxyUsername, proxyPassword, wst->gwhostname, + if (!proxy_connect(wst->context, bufferedBio, proxyUsername, proxyPassword, wst->gwhostname, wst->gwport)) { BIO_free_all(bufferedBio); diff --git a/libfreerdp/core/proxy.c b/libfreerdp/core/proxy.c index 161d43e76..17fff988f 100644 --- a/libfreerdp/core/proxy.c +++ b/libfreerdp/core/proxy.c @@ -30,6 +30,7 @@ #include "tcp.h" #include +#include #include /* For GetEnvironmentVariableA */ #define CRLF "\r\n" @@ -70,9 +71,9 @@ static const char* rplstat[] = { "succeeded", "Command not supported", "Address type not supported" }; -static BOOL http_proxy_connect(BIO* bufferedBio, const char* proxyUsername, +static BOOL http_proxy_connect(rdpContext* context, BIO* bufferedBio, const char* proxyUsername, const char* proxyPassword, const char* hostname, UINT16 port); -static BOOL socks_proxy_connect(BIO* bufferedBio, const char* proxyUsername, +static BOOL socks_proxy_connect(rdpContext* context, BIO* bufferedBio, const char* proxyUsername, const char* proxyPassword, const char* hostname, UINT16 port); static void proxy_read_environment(rdpSettings* settings, char* envname); @@ -483,9 +484,12 @@ fail: return rc; } -BOOL proxy_connect(rdpSettings* settings, BIO* bufferedBio, const char* proxyUsername, +BOOL proxy_connect(rdpContext* context, BIO* bufferedBio, const char* proxyUsername, const char* proxyPassword, const char* hostname, UINT16 port) { + WINPR_ASSERT(context); + rdpSettings* settings = context->settings; + switch (freerdp_settings_get_uint32(settings, FreeRDP_ProxyType)) { case PROXY_TYPE_NONE: @@ -493,10 +497,12 @@ BOOL proxy_connect(rdpSettings* settings, BIO* bufferedBio, const char* proxyUse return TRUE; case PROXY_TYPE_HTTP: - return http_proxy_connect(bufferedBio, proxyUsername, proxyPassword, hostname, port); + return http_proxy_connect(context, bufferedBio, proxyUsername, proxyPassword, hostname, + port); case PROXY_TYPE_SOCKS: - return socks_proxy_connect(bufferedBio, proxyUsername, proxyPassword, hostname, port); + return socks_proxy_connect(context, bufferedBio, proxyUsername, proxyPassword, hostname, + port); default: WLog_ERR(TAG, "Invalid internal proxy configuration"); @@ -516,7 +522,7 @@ static const char* get_response_header(char* response) return response; } -static BOOL http_proxy_connect(BIO* bufferedBio, const char* proxyUsername, +static BOOL http_proxy_connect(rdpContext* context, BIO* bufferedBio, const char* proxyUsername, const char* proxyPassword, const char* hostname, UINT16 port) { BOOL rc = FALSE; @@ -532,8 +538,11 @@ static BOOL http_proxy_connect(BIO* bufferedBio, const char* proxyUsername, const char connect[] = "CONNECT "; const char httpheader[] = " HTTP/1.1" CRLF "Host: "; + WINPR_ASSERT(context); WINPR_ASSERT(bufferedBio); WINPR_ASSERT(hostname); + const UINT32 timeout = + freerdp_settings_get_uint32(context->settings, FreeRDP_TcpConnectTimeout); _itoa_s(port, port_str, sizeof(port_str), 10); @@ -601,6 +610,7 @@ static BOOL http_proxy_connect(BIO* bufferedBio, const char* proxyUsername, /* Read result until CR-LF-CR-LF. * Keep recv_buf a null-terminated string. */ + const UINT64 start = GetTickCount64(); while (strstr(recv_buf, CRLF CRLF) == NULL) { if (resultsize >= sizeof(recv_buf) - 1) @@ -616,7 +626,7 @@ static BOOL http_proxy_connect(BIO* bufferedBio, const char* proxyUsername, if (status < 0) { /* Error? */ - if (BIO_should_retry(bufferedBio)) + if (!freerdp_shall_disconnect_context(context) && BIO_should_retry(bufferedBio)) { USleep(100); continue; @@ -626,6 +636,18 @@ static BOOL http_proxy_connect(BIO* bufferedBio, const char* proxyUsername, goto fail; } else if (status == 0) + { + const UINT64 now = GetTickCount64(); + const UINT64 diff = now - start; + if (freerdp_shall_disconnect_context(context) || (now < start) || (diff > timeout)) + { + /* Error? */ + WLog_ERR(TAG, "Failed reading reply from HTTP proxy (BIO_read returned zero)"); + goto fail; + } + Sleep(10); + } + else { /* Error? */ WLog_ERR(TAG, "Failed reading reply from HTTP proxy (BIO_read returned zero)"); @@ -661,10 +683,16 @@ fail: return rc; } -static int recv_socks_reply(BIO* bufferedBio, BYTE* buf, int len, char* reason, BYTE checkVer) +static int recv_socks_reply(rdpContext* context, BIO* bufferedBio, BYTE* buf, int len, char* reason, + BYTE checkVer) { int status = 0; + WINPR_ASSERT(context); + + const UINT32 timeout = + freerdp_settings_get_uint32(context->settings, FreeRDP_TcpConnectTimeout); + const UINT64 start = GetTickCount64(); for (;;) { ERR_clear_error(); @@ -677,7 +705,7 @@ static int recv_socks_reply(BIO* bufferedBio, BYTE* buf, int len, char* reason, else if (status < 0) { /* Error? */ - if (BIO_should_retry(bufferedBio)) + if (!freerdp_shall_disconnect_context(context) && BIO_should_retry(bufferedBio)) { USleep(100); continue; @@ -686,6 +714,19 @@ static int recv_socks_reply(BIO* bufferedBio, BYTE* buf, int len, char* reason, WLog_ERR(TAG, "Failed reading %s reply from SOCKS proxy (Status %d)", reason, status); return -1; } + else if (status == 0) + { + const UINT64 now = GetTickCount64(); + const UINT64 diff = now - start; + if (freerdp_shall_disconnect_context(context) || (now < start) || (diff > timeout)) + { + /* Error? */ + WLog_ERR(TAG, "Failed reading %s reply from SOCKS proxy (BIO_read returned zero)", + reason); + return status; + } + Sleep(10); + } else // if (status == 0) { /* Error? */ @@ -710,7 +751,7 @@ static int recv_socks_reply(BIO* bufferedBio, BYTE* buf, int len, char* reason, return status; } -static BOOL socks_proxy_connect(BIO* bufferedBio, const char* proxyUsername, +static BOOL socks_proxy_connect(rdpContext* context, BIO* bufferedBio, const char* proxyUsername, const char* proxyPassword, const char* hostname, UINT16 port) { int status = 0; @@ -742,7 +783,7 @@ static BOOL socks_proxy_connect(BIO* bufferedBio, const char* proxyUsername, return FALSE; } - status = recv_socks_reply(bufferedBio, buf, 2, "AUTH REQ", 5); + status = recv_socks_reply(context, bufferedBio, buf, 2, "AUTH REQ", 5); if (status <= 0) return FALSE; @@ -786,7 +827,7 @@ static BOOL socks_proxy_connect(BIO* bufferedBio, const char* proxyUsername, return FALSE; } - status = recv_socks_reply(bufferedBio, buf, 2, "AUTH REQ", 1); + status = recv_socks_reply(context, bufferedBio, buf, 2, "AUTH REQ", 1); if (status < 2) return FALSE; @@ -824,7 +865,7 @@ static BOOL socks_proxy_connect(BIO* bufferedBio, const char* proxyUsername, return FALSE; } - status = recv_socks_reply(bufferedBio, buf, sizeof(buf), "CONN REQ", 5); + status = recv_socks_reply(context, bufferedBio, buf, sizeof(buf), "CONN REQ", 5); if (status < 4) return FALSE; diff --git a/libfreerdp/core/proxy.h b/libfreerdp/core/proxy.h index ac2341c3d..f5eb2cf7b 100644 --- a/libfreerdp/core/proxy.h +++ b/libfreerdp/core/proxy.h @@ -26,7 +26,7 @@ BOOL proxy_prepare(rdpSettings* settings, const char** lpPeerHostname, UINT16* lpPeerPort, const char** lpProxyUsername, const char** lpProxyPassword); -BOOL proxy_connect(rdpSettings* settings, BIO* bio, const char* proxyUsername, +BOOL proxy_connect(rdpContext* context, BIO* bio, const char* proxyUsername, const char* proxyPassword, const char* hostname, UINT16 port); #endif /* FREERDP_LIB_CORE_HTTP_PROXY_H */ diff --git a/libfreerdp/core/transport.c b/libfreerdp/core/transport.c index f83c24a16..e048dfaea 100644 --- a/libfreerdp/core/transport.c +++ b/libfreerdp/core/transport.c @@ -595,8 +595,8 @@ BOOL transport_connect(rdpTransport* transport, const char* hostname, UINT16 por if (isProxyConnection) { - if (!proxy_connect(settings, transport->frontBio, proxyUsername, proxyPassword, - hostname, port)) + if (!proxy_connect(context, transport->frontBio, proxyUsername, proxyPassword, hostname, + port)) return FALSE; }