Merge pull request #6193 from kubistika/proxy_fixes_

server: proxy: code refactor

libfreerdp/codec/progressive.c

@@ -1995,7 +1995,7 @@ static INLINE INT32 progressive_wb_read_region_header(PROGRESSIVE_CONTEXT* progr
 	len -= region->tileDataSize;
 	if (len > 0)
 		WLog_Print(progressive->log, WLOG_DEBUG,
-		           "Unused byes detected, %" PRIuz " bytes not processed", len);
+		           "Unused bytes detected, %" PRIuz " bytes not processed", len);
 	return 0;
 }
 

server/proxy/modules/demo/demo.cpp

@@ -55,10 +55,13 @@ static proxyPlugin demo_plugin = {
 	plugin_desc,                /* description */
 	demo_plugin_unload,         /* PluginUnload */
 	NULL,                       /* ClientPreConnect */
+	NULL,                       /* ClientPostConnect */
 	NULL,                       /* ClientLoginFailure */
+	NULL,                       /* ClientEndPaint */
 	NULL,                       /* ServerPostConnect */
 	NULL,                       /* ServerChannelsInit */
 	NULL,                       /* ServerChannelsFree */
+	NULL,                       /* ServerSessionEnd */
 	demo_filter_keyboard_event, /* KeyboardEvent */
 	NULL,                       /* MouseEvent */
 	NULL,                       /* ClientChannelData */

server/proxy/modules/modules_api.h

@@ -48,10 +48,13 @@ typedef struct proxy_plugin
 
 	/* proxy hooks. a module can set these function pointers to register hooks */
 	proxyHookFn ClientPreConnect;
+	proxyHookFn ClientPostConnect;
 	proxyHookFn ClientLoginFailure;
+	proxyHookFn ClientEndPaint;
 	proxyHookFn ServerPostConnect;
 	proxyHookFn ServerChannelsInit;
 	proxyHookFn ServerChannelsFree;
+	proxyHookFn ServerSessionEnd;
 
 	/* proxy filters. a module can set these function pointers to register filters */
 	proxyFilterFn KeyboardEvent;
@@ -106,7 +109,7 @@ typedef struct channel_data_event_info
 
 	/* actual data */
 	const BYTE* data;
-	int data_len;
+	size_t data_len;
 } proxyChannelDataEventInfo;
 #define WINPR_PACK_POP
 #include <winpr/pack.h>

server/proxy/pf_client.c

@@ -177,7 +177,7 @@ static BOOL pf_client_pre_connect(freerdp* instance)
 	 * Also, OrderSupport need to be zeroed, because it is currently not supported.
 	 */
 	settings->GlyphSupportLevel = GLYPH_SUPPORT_NONE;
-	ZeroMemory(instance->settings->OrderSupport, 32);
+	ZeroMemory(settings->OrderSupport, 32);
 
 	settings->SupportDynamicChannels = TRUE;
 
@@ -298,6 +298,9 @@ static BOOL pf_client_post_connect(freerdp* instance)
 	ps = (rdpContext*)pc->pdata->ps;
 	config = pc->pdata->config;
 
+	if (!pf_modules_run_hook(HOOK_TYPE_CLIENT_POST_CONNECT, pc->pdata))
+		return FALSE;
+
 	if (config->SessionCapture)
 	{
 		if (!pf_capture_create_session_directory(pc))

server/proxy/pf_config.c

@@ -38,6 +38,20 @@
 #define CONFIG_PRINT_UINT16(config, key) WLog_INFO(TAG, "\t\t%s: %" PRIu16 "", #key, config->key)
 #define CONFIG_PRINT_UINT32(config, key) WLog_INFO(TAG, "\t\t%s: %" PRIu32 "", #key, config->key)
 
+static char** pf_config_parse_comma_separated_list(const char* list, size_t* count)
+{
+	if (!list || !count)
+		return NULL;
+
+	if (strlen(list) == 0)
+	{
+		*count = 0;
+		return NULL;
+	}
+
+	return CommandLineParseCommaSeparatedValues(list, count);
+}
+
 BOOL pf_config_get_uint16(wIniFile* ini, const char* section, const char* key, UINT16* result)
 {
 	int val;
@@ -154,7 +168,7 @@ static BOOL pf_config_load_channels(wIniFile* ini, proxyConfig* config)
 	config->Clipboard = pf_config_get_bool(ini, "Channels", "Clipboard");
 	config->AudioOutput = pf_config_get_bool(ini, "Channels", "AudioOutput");
 	config->RemoteApp = pf_config_get_bool(ini, "Channels", "RemoteApp");
-	config->Passthrough = CommandLineParseCommaSeparatedValues(
+	config->Passthrough = pf_config_parse_comma_separated_list(
 	    pf_config_get_str(ini, "Channels", "Passthrough"), &config->PassthroughCount);
 
 	{
@@ -212,10 +226,10 @@ static BOOL pf_config_load_modules(wIniFile* ini, proxyConfig* config)
 	modules_to_load = IniFile_GetKeyValueString(ini, "Plugins", "Modules");
 	required_modules = IniFile_GetKeyValueString(ini, "Plugins", "Required");
 
-	config->Modules = CommandLineParseCommaSeparatedValues(modules_to_load, &config->ModulesCount);
+	config->Modules = pf_config_parse_comma_separated_list(modules_to_load, &config->ModulesCount);
 
 	config->RequiredPlugins =
-	    CommandLineParseCommaSeparatedValues(required_modules, &config->RequiredPluginsCount);
+	    pf_config_parse_comma_separated_list(required_modules, &config->RequiredPluginsCount);
 	return TRUE;
 }
 

server/proxy/pf_context.c

@@ -112,49 +112,46 @@ BOOL pf_context_init_server_context(freerdp_peer* client)
 	return freerdp_peer_context_new(client);
 }
 
-/*
- * pf_context_copy_settings copies settings from `src` to `dst`.
- * when using this function, is_dst_server must be set to TRUE if the destination
- * settings are server's settings. otherwise, they must be set to FALSE.
- */
 BOOL pf_context_copy_settings(rdpSettings* dst, const rdpSettings* src)
 {
-	rdpSettings* before_copy = freerdp_settings_clone(dst);
+	BOOL rc = FALSE;
+	rdpSettings* before_copy;
 
+	if (!dst || !src)
+		return FALSE;
+
+	before_copy = freerdp_settings_clone(dst);
 	if (!before_copy)
 		return FALSE;
 
+#define REVERT_STR_VALUE(name)                                          \
+	free(dst->name);                                                    \
+	dst->name = NULL;                                                   \
+	if (before_copy->name && !(dst->name = _strdup(before_copy->name))) \
+	goto out_fail
+
 	if (!freerdp_settings_copy(dst, src))
 	{
 		freerdp_settings_free(before_copy);
 		return FALSE;
 	}
 
-	free(dst->ConfigPath);
+	/* keep original ServerMode value */
-	free(dst->PrivateKeyContent);
-	free(dst->RdpKeyContent);
-	free(dst->RdpKeyFile);
-	free(dst->PrivateKeyFile);
-	free(dst->CertificateFile);
-	free(dst->CertificateName);
-	free(dst->CertificateContent);
-
-	/* adjust pointer to instance pointer */
 	dst->ServerMode = before_copy->ServerMode;
 
 	/* revert some values that must not be changed */
-	dst->ConfigPath = _strdup(before_copy->ConfigPath);
+	REVERT_STR_VALUE(ConfigPath);
-	dst->PrivateKeyContent = _strdup(before_copy->PrivateKeyContent);
+	REVERT_STR_VALUE(PrivateKeyContent);
-	dst->RdpKeyContent = _strdup(before_copy->RdpKeyContent);
+	REVERT_STR_VALUE(RdpKeyContent);
-	dst->RdpKeyFile = _strdup(before_copy->RdpKeyFile);
+	REVERT_STR_VALUE(RdpKeyFile);
-	dst->PrivateKeyFile = _strdup(before_copy->PrivateKeyFile);
+	REVERT_STR_VALUE(PrivateKeyFile);
-	dst->CertificateFile = _strdup(before_copy->CertificateFile);
+	REVERT_STR_VALUE(CertificateFile);
-	dst->CertificateName = _strdup(before_copy->CertificateName);
+	REVERT_STR_VALUE(CertificateName);
-	dst->CertificateContent = _strdup(before_copy->CertificateContent);
+	REVERT_STR_VALUE(CertificateContent);
 
 	if (!dst->ServerMode)
 	{
-		/* adjust instance pointer for client's context */
+		/* adjust instance pointer */
 		dst->instance = before_copy->instance;
 
 		/*
@@ -168,8 +165,11 @@ BOOL pf_context_copy_settings(rdpSettings* dst, const rdpSettings* src)
 		dst->RdpServerRsaKey = NULL;
 	}
 
+	rc = TRUE;
+
+out_fail:
 	freerdp_settings_free(before_copy);
-	return TRUE;
+	return rc;
 }
 
 pClientContext* pf_context_create_client_context(rdpSettings* clientSettings)
@@ -203,42 +203,32 @@ proxyData* proxy_data_new(void)
 	BYTE temp[16];
 	proxyData* pdata = calloc(1, sizeof(proxyData));
 
-	if (pdata == NULL)
+	if (!pdata)
-	{
 		return NULL;
-	}
 
 	if (!(pdata->abort_event = CreateEvent(NULL, TRUE, FALSE, NULL)))
-	{
+		goto error;
-		proxy_data_free(pdata);
-		return NULL;
-	}
 
 	if (!(pdata->gfx_server_ready = CreateEvent(NULL, TRUE, FALSE, NULL)))
-	{
+		goto error;
-		proxy_data_free(pdata);
-		return NULL;
-	}
 
 	winpr_RAND((BYTE*)&temp, 16);
 	if (!(pdata->session_id = winpr_BinToHexString(temp, 16, FALSE)))
-	{
+		goto error;
-		proxy_data_free(pdata);
-		return NULL;
-	}
 
 	if (!(pdata->modules_info = HashTable_New(FALSE)))
-	{
+		goto error;
-		proxy_data_free(pdata);
-		return NULL;
-	}
 
 	/* modules_info maps between plugin name to custom data */
 	pdata->modules_info->hash = HashTable_StringHash;
 	pdata->modules_info->keyCompare = HashTable_StringCompare;
 	pdata->modules_info->keyClone = HashTable_StringClone;
 	pdata->modules_info->keyFree = HashTable_StringFree;
+
 	return pdata;
+error:
+	proxy_data_free(pdata);
+	return NULL;
 }
 
 /* updates circular pointers between proxyData and pClientContext instances */

server/proxy/pf_modules.c

@@ -47,8 +47,8 @@ static const char* FILTER_TYPE_STRINGS[] = {
 };
 
 static const char* HOOK_TYPE_STRINGS[] = {
-	"CLIENT_PRE_CONNECT",   "CLIENT_LOGIN_FAILURE", "SERVER_POST_CONNECT",
+	"CLIENT_PRE_CONNECT",  "CLIENT_POST_CONNECT",  "CLIENT_LOGIN_FAILURE", "CLIENT_END_PAINT",
-	"SERVER_CHANNELS_INIT", "SERVER_CHANNELS_FREE",
+	"SERVER_POST_CONNECT", "SERVER_CHANNELS_INIT", "SERVER_CHANNELS_FREE", "SERVER_SESSION_END",
 };
 
 static const char* pf_modules_get_filter_type_string(PF_FILTER_TYPE result)
@@ -89,10 +89,18 @@ BOOL pf_modules_run_hook(PF_HOOK_TYPE type, proxyData* pdata)
 				IFCALLRET(plugin->ClientPreConnect, ok, pdata);
 				break;
 
+			case HOOK_TYPE_CLIENT_POST_CONNECT:
+				IFCALLRET(plugin->ClientPostConnect, ok, pdata);
+				break;
+
 			case HOOK_TYPE_CLIENT_LOGIN_FAILURE:
 				IFCALLRET(plugin->ClientLoginFailure, ok, pdata);
 				break;
 
+			case HOOK_TYPE_CLIENT_END_PAINT:
+				IFCALLRET(plugin->ClientEndPaint, ok, pdata);
+				break;
+
 			case HOOK_TYPE_SERVER_POST_CONNECT:
 				IFCALLRET(plugin->ServerPostConnect, ok, pdata);
 				break;
@@ -105,6 +113,10 @@ BOOL pf_modules_run_hook(PF_HOOK_TYPE type, proxyData* pdata)
 				IFCALLRET(plugin->ServerChannelsFree, ok, pdata);
 				break;
 
+			case HOOK_TYPE_SERVER_SESSION_END:
+				IFCALLRET(plugin->ServerSessionEnd, ok, pdata);
+				break;
+
 			default:
 				WLog_ERR(TAG, "invalid hook called");
 		}

server/proxy/pf_modules.h

@@ -41,11 +41,14 @@ typedef enum _PF_HOOK_TYPE PF_HOOK_TYPE;
 enum _PF_HOOK_TYPE
 {
 	HOOK_TYPE_CLIENT_PRE_CONNECT,
+	HOOK_TYPE_CLIENT_POST_CONNECT,
 	HOOK_TYPE_CLIENT_LOGIN_FAILURE,
+	HOOK_TYPE_CLIENT_END_PAINT,
 
 	HOOK_TYPE_SERVER_POST_CONNECT,
 	HOOK_TYPE_SERVER_CHANNELS_INIT,
 	HOOK_TYPE_SERVER_CHANNELS_FREE,
+	HOOK_TYPE_SERVER_SESSION_END,
 
 	HOOK_LAST
 };

server/proxy/pf_server.c

@@ -60,11 +60,8 @@ static BOOL pf_server_parse_target_from_routing_token(rdpContext* context, char*
 	const char* routing_token = freerdp_nego_get_routing_token(context, &routing_token_length);
 	pServerContext* ps = (pServerContext*)context;
 
-	if (routing_token == NULL)
+	if (!routing_token)
-	{
-		/* no routing token */
 		return FALSE;
-	}
 
 	if ((routing_token_length <= prefix_len) || (routing_token_length >= TARGET_MAX))
 	{
@@ -149,7 +146,7 @@ static BOOL pf_server_post_connect(freerdp_peer* peer)
 	pc = pf_context_create_client_context(peer->settings);
 	if (pc == NULL)
 	{
-		LOG_ERR(TAG, ps, "[%s]: pf_context_create_client_context failed!");
+		LOG_ERR(TAG, ps, "failed to create client context!");
 		return FALSE;
 	}
 
@@ -160,7 +157,6 @@ static BOOL pf_server_post_connect(freerdp_peer* peer)
 
 	if (!pf_server_get_target_info(peer->context, client_settings, pdata->config))
 	{
-
 		LOG_INFO(TAG, ps, "pf_server_get_target_info failed!");
 		return FALSE;
 	}
@@ -204,11 +200,19 @@ static BOOL pf_server_receive_channel_data_hook(freerdp_peer* peer, UINT16 chann
 {
 	pServerContext* ps = (pServerContext*)peer->context;
 	pClientContext* pc = ps->pdata->pc;
-	proxyData* pdata = pc->pdata;
+	proxyData* pdata = ps->pdata;
 	proxyConfig* config = pdata->config;
 	size_t i;
 	const char* channel_name = WTSChannelGetName(peer, channelId);
 
+	/*
+	 * client side is not initialized yet, call original callback.
+	 * this is probably a drdynvc message between peer and proxy server,
+	 * which doesn't need to be proxied.
+	 */
+	if (!pc)
+		goto original_cb;
+
 	for (i = 0; i < config->PassthroughCount; i++)
 	{
 		if (strncmp(channel_name, config->Passthrough[i], CHANNEL_NAME_LEN) == 0)
@@ -231,6 +235,7 @@ static BOOL pf_server_receive_channel_data_hook(freerdp_peer* peer, UINT16 chann
 		}
 	}
 
+original_cb:
 	return server_receive_channel_data_original(peer, channelId, data, size, flags, totalSize);
 }
 
@@ -334,7 +339,8 @@ static DWORD WINAPI pf_server_handle_peer(LPVOID arg)
 	pdata = ps->pdata;
 
 	client->Initialize(client);
-	LOG_INFO(TAG, ps, "peer connected: %s", client->hostname);
+	LOG_INFO(TAG, ps, "new connection: proxy address: %s, client address: %s", pdata->config->Host,
+	         client->hostname);
 	/* Main client event handling loop */
 	ChannelEvent = WTSVirtualChannelManagerGetEventHandle(ps->vcm);
 
@@ -415,6 +421,7 @@ fail:
 	LOG_INFO(TAG, ps, "starting shutdown of connection");
 	LOG_INFO(TAG, ps, "stopping proxy's client");
 	freerdp_client_stop(pc);
+	pf_modules_run_hook(HOOK_TYPE_SERVER_SESSION_END, pdata);
 	LOG_INFO(TAG, ps, "freeing server's channels");
 	pf_server_channels_free(ps);
 	LOG_INFO(TAG, ps, "freeing proxy data");

server/proxy/pf_update.c

@@ -24,6 +24,7 @@
 #include <winpr/image.h>
 #include <winpr/sysinfo.h>
 
+#include "pf_modules.h"
 #include "pf_update.h"
 #include "pf_capture.h"
 #include "pf_context.h"
@@ -78,6 +79,9 @@ static BOOL pf_client_end_paint(rdpContext* context)
 	if (!ps->update->EndPaint(ps))
 		return FALSE;
 
+	if (!pf_modules_run_hook(HOOK_TYPE_CLIENT_END_PAINT, pdata))
+		return FALSE;
+
 	if (!pdata->config->SessionCapture)
 		return TRUE;