From 9de59051f675d00c9fc8c984bf110cc0fe19ed24 Mon Sep 17 00:00:00 2001 From: Kobi Mizrachi Date: Tue, 19 May 2020 14:42:26 +0300 Subject: [PATCH 1/6] server: proxy: refactor --- server/proxy/modules/modules_api.h | 2 +- server/proxy/pf_client.c | 2 +- server/proxy/pf_context.c | 80 +++++++++++++----------------- server/proxy/pf_server.c | 8 +-- 4 files changed, 39 insertions(+), 53 deletions(-) diff --git a/server/proxy/modules/modules_api.h b/server/proxy/modules/modules_api.h index a99ebc492..bf380d303 100644 --- a/server/proxy/modules/modules_api.h +++ b/server/proxy/modules/modules_api.h @@ -106,7 +106,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 diff --git a/server/proxy/pf_client.c b/server/proxy/pf_client.c index 0db33f88a..56c722117 100644 --- a/server/proxy/pf_client.c +++ b/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; diff --git a/server/proxy/pf_context.c b/server/proxy/pf_context.c index 552eb7963..0ff8b5f15 100644 --- a/server/proxy/pf_context.c +++ b/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); - 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 */ + /* keep original ServerMode value */ dst->ServerMode = before_copy->ServerMode; /* revert some values that must not be changed */ - dst->ConfigPath = _strdup(before_copy->ConfigPath); - dst->PrivateKeyContent = _strdup(before_copy->PrivateKeyContent); - dst->RdpKeyContent = _strdup(before_copy->RdpKeyContent); - dst->RdpKeyFile = _strdup(before_copy->RdpKeyFile); - dst->PrivateKeyFile = _strdup(before_copy->PrivateKeyFile); - dst->CertificateFile = _strdup(before_copy->CertificateFile); - dst->CertificateName = _strdup(before_copy->CertificateName); - dst->CertificateContent = _strdup(before_copy->CertificateContent); + REVERT_STR_VALUE(ConfigPath); + REVERT_STR_VALUE(PrivateKeyContent); + REVERT_STR_VALUE(RdpKeyContent); + REVERT_STR_VALUE(RdpKeyFile); + REVERT_STR_VALUE(PrivateKeyFile); + REVERT_STR_VALUE(CertificateFile); + REVERT_STR_VALUE(CertificateName); + 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))) - { - proxy_data_free(pdata); - return NULL; - } + goto error; if (!(pdata->gfx_server_ready = CreateEvent(NULL, TRUE, FALSE, NULL))) - { - proxy_data_free(pdata); - return NULL; - } + goto error; winpr_RAND((BYTE*)&temp, 16); if (!(pdata->session_id = winpr_BinToHexString(temp, 16, FALSE))) - { - proxy_data_free(pdata); - return NULL; - } + goto error; if (!(pdata->modules_info = HashTable_New(FALSE))) - { - proxy_data_free(pdata); - return NULL; - } + goto error; /* 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 */ diff --git a/server/proxy/pf_server.c b/server/proxy/pf_server.c index 95165e0ba..251e70a72 100644 --- a/server/proxy/pf_server.c +++ b/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) - { - /* no routing token */ + if (!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; } From 215e41b4eeb252c8acbd337a841761cd9da96ff1 Mon Sep 17 00:00:00 2001 From: Kobi Mizrachi Date: Tue, 19 May 2020 14:42:59 +0300 Subject: [PATCH 2/6] server: proxy: config: fix comma separated list parsing --- server/proxy/pf_config.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/server/proxy/pf_config.c b/server/proxy/pf_config.c index ff78b70c8..babe6270e 100644 --- a/server/proxy/pf_config.c +++ b/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; } From 715c3f293afc8bb1173a3589ed8729831cc99e64 Mon Sep 17 00:00:00 2001 From: Kobi Mizrachi Date: Tue, 19 May 2020 14:43:30 +0300 Subject: [PATCH 3/6] server: proxy: vc hook: check if proxy client is not null --- server/proxy/pf_server.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/server/proxy/pf_server.c b/server/proxy/pf_server.c index 251e70a72..e0a64bed6 100644 --- a/server/proxy/pf_server.c +++ b/server/proxy/pf_server.c @@ -200,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) @@ -227,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); } From 7d48a587d2ecbf8c4e2bef12bf7e6e515c60c459 Mon Sep 17 00:00:00 2001 From: Kobi Mizrachi Date: Tue, 19 May 2020 14:57:15 +0300 Subject: [PATCH 4/6] server: proxy: prepare for exporting gfx capture to a module --- server/proxy/modules/demo/demo.cpp | 3 +++ server/proxy/modules/modules_api.h | 3 +++ server/proxy/pf_client.c | 3 +++ server/proxy/pf_modules.c | 16 ++++++++++++++-- server/proxy/pf_modules.h | 3 +++ server/proxy/pf_server.c | 1 + server/proxy/pf_update.c | 4 ++++ 7 files changed, 31 insertions(+), 2 deletions(-) diff --git a/server/proxy/modules/demo/demo.cpp b/server/proxy/modules/demo/demo.cpp index f50789a91..617cda037 100644 --- a/server/proxy/modules/demo/demo.cpp +++ b/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 */ diff --git a/server/proxy/modules/modules_api.h b/server/proxy/modules/modules_api.h index bf380d303..4b9de1d58 100644 --- a/server/proxy/modules/modules_api.h +++ b/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; diff --git a/server/proxy/pf_client.c b/server/proxy/pf_client.c index 56c722117..fd33e26c2 100644 --- a/server/proxy/pf_client.c +++ b/server/proxy/pf_client.c @@ -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)) diff --git a/server/proxy/pf_modules.c b/server/proxy/pf_modules.c index d8f32896d..daa81451e 100644 --- a/server/proxy/pf_modules.c +++ b/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", - "SERVER_CHANNELS_INIT", "SERVER_CHANNELS_FREE", + "CLIENT_PRE_CONNECT", "CLIENT_POST_CONNECT", "CLIENT_LOGIN_FAILURE", "CLIENT_END_PAINT", + "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"); } diff --git a/server/proxy/pf_modules.h b/server/proxy/pf_modules.h index a8efd7f32..bb188c46d 100644 --- a/server/proxy/pf_modules.h +++ b/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 }; diff --git a/server/proxy/pf_server.c b/server/proxy/pf_server.c index e0a64bed6..ba70952d0 100644 --- a/server/proxy/pf_server.c +++ b/server/proxy/pf_server.c @@ -420,6 +420,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"); diff --git a/server/proxy/pf_update.c b/server/proxy/pf_update.c index 83fed48ef..9debc0341 100644 --- a/server/proxy/pf_update.c +++ b/server/proxy/pf_update.c @@ -24,6 +24,7 @@ #include #include +#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; From 8d72051ab157d89af3f919a441170ca57bf67e84 Mon Sep 17 00:00:00 2001 From: Kobi Mizrachi Date: Tue, 19 May 2020 15:06:22 +0300 Subject: [PATCH 5/6] codec: fix typo in progressive codec log --- libfreerdp/codec/progressive.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libfreerdp/codec/progressive.c b/libfreerdp/codec/progressive.c index bfa2dc5da..d74cff39f 100644 --- a/libfreerdp/codec/progressive.c +++ b/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; } From 62692430919d45ce326e25a2dcf2a4e06e93a3a4 Mon Sep 17 00:00:00 2001 From: Kobi Mizrachi Date: Wed, 20 May 2020 10:36:42 +0300 Subject: [PATCH 6/6] server: proxy: print server host ip on new conn (for clustering info) --- server/proxy/pf_server.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/proxy/pf_server.c b/server/proxy/pf_server.c index ba70952d0..78bf7c138 100644 --- a/server/proxy/pf_server.c +++ b/server/proxy/pf_server.c @@ -339,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);