From 27e762d06f6b40fc0e7e958513db2af330161947 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Wed, 7 Apr 2021 17:13:48 -0700 Subject: [PATCH] GUACAMOLE-1283: Add synchronization around absolutely all outbound RDP messages. The FreeRDP library is intended to be threadsafe, but is not reliably so with respect to legacy RDP encryption and outbound messages. When outbound messages are sent by multiple threads, the encryption key used for legacy RDP encryption may not be updated correctly, resulting in a fatal connection error like: "ERRINFO_DECRYPT_FAILED (0x00001192):(a) Decryption using Standard RDP Security mechanisms (section 5.3.6) failed. (b) Session key creation using Standard RDP Security mechanisms (section 5.3.5) failed." --- .../rdp/channels/audio-input/audio-buffer.c | 5 +- .../rdp/channels/audio-input/audio-buffer.h | 34 ++++--- src/protocols/rdp/channels/cliprdr.c | 89 +++++++++++++------ src/protocols/rdp/channels/common-svc.c | 4 + src/protocols/rdp/channels/disp.c | 14 ++- src/protocols/rdp/channels/disp.h | 10 ++- src/protocols/rdp/channels/rail.c | 15 +++- src/protocols/rdp/channels/rdpei.c | 19 +++- src/protocols/rdp/channels/rdpei.h | 10 ++- src/protocols/rdp/client.c | 8 +- src/protocols/rdp/input.c | 29 +++--- src/protocols/rdp/keyboard.c | 13 +-- .../rdp/plugins/guacai/guacai-messages.c | 20 ++++- src/protocols/rdp/rdp.c | 15 ++-- src/protocols/rdp/rdp.h | 6 ++ 15 files changed, 217 insertions(+), 74 deletions(-) diff --git a/src/protocols/rdp/channels/audio-input/audio-buffer.c b/src/protocols/rdp/channels/audio-input/audio-buffer.c index 30513419..e7ecf3ec 100644 --- a/src/protocols/rdp/channels/audio-input/audio-buffer.c +++ b/src/protocols/rdp/channels/audio-input/audio-buffer.c @@ -31,9 +31,10 @@ #include #include -guac_rdp_audio_buffer* guac_rdp_audio_buffer_alloc() { +guac_rdp_audio_buffer* guac_rdp_audio_buffer_alloc(guac_client* client) { guac_rdp_audio_buffer* buffer = calloc(1, sizeof(guac_rdp_audio_buffer)); pthread_mutex_init(&(buffer->lock), NULL); + buffer->client = client; return buffer; } @@ -270,7 +271,7 @@ void guac_rdp_audio_buffer_write(guac_rdp_audio_buffer* audio_buffer, /* Only actually invoke if defined */ if (audio_buffer->flush_handler) - audio_buffer->flush_handler(audio_buffer->packet, + audio_buffer->flush_handler(audio_buffer, audio_buffer->packet, audio_buffer->bytes_written, audio_buffer->data); /* Reset buffer in all cases */ diff --git a/src/protocols/rdp/channels/audio-input/audio-buffer.h b/src/protocols/rdp/channels/audio-input/audio-buffer.h index 32f7def8..fb24b045 100644 --- a/src/protocols/rdp/channels/audio-input/audio-buffer.h +++ b/src/protocols/rdp/channels/audio-input/audio-buffer.h @@ -24,10 +24,21 @@ #include #include +/** + * A buffer of arbitrary audio data. Received audio data can be written to this + * buffer, and will automatically be flushed via a given handler once the + * internal buffer reaches capacity. + */ +typedef struct guac_rdp_audio_buffer guac_rdp_audio_buffer; + /** * Handler which is invoked when a guac_rdp_audio_buffer's internal packet * buffer has reached capacity and must be flushed. * + * @param audio_buffer + * The guac_rdp_audio_buffer that has reached capacity and is being + * flushed. + * * @param buffer * The buffer which needs to be flushed as an audio packet. * @@ -40,8 +51,8 @@ * The arbitrary data pointer provided when the audio buffer was * initialized. */ -typedef void guac_rdp_audio_buffer_flush_handler(char* buffer, int length, - void* data); +typedef void guac_rdp_audio_buffer_flush_handler(guac_rdp_audio_buffer* audio_buffer, + char* buffer, int length, void* data); /** * A description of an arbitrary PCM audio format. @@ -66,12 +77,7 @@ typedef struct guac_rdp_audio_format { } guac_rdp_audio_format; -/** - * A buffer of arbitrary audio data. Received audio data can be written to this - * buffer, and will automatically be flushed via a given handler once the - * internal buffer reaches capacity. - */ -typedef struct guac_rdp_audio_buffer { +struct guac_rdp_audio_buffer { /** * Lock which is acquired/released to ensure accesses to the audio buffer @@ -79,6 +85,11 @@ typedef struct guac_rdp_audio_buffer { */ pthread_mutex_t lock; + /** + * The guac_client instance handling the relevant RDP connection. + */ + guac_client* client; + /** * The user from which this audio buffer will receive data. If no user has * yet opened an associated audio stream, this will be NULL. @@ -145,17 +156,20 @@ typedef struct guac_rdp_audio_buffer { */ void* data; -} guac_rdp_audio_buffer; +}; /** * Allocates a new audio buffer. The new audio buffer will ignore any received * data until guac_rdp_audio_buffer_begin() is invoked, and will resume * ignoring received data once guac_rdp_audio_buffer_end() is invoked. * + * @param client + * The guac_client instance handling the relevant RDP connection. + * * @return * A newly-allocated audio buffer. */ -guac_rdp_audio_buffer* guac_rdp_audio_buffer_alloc(); +guac_rdp_audio_buffer* guac_rdp_audio_buffer_alloc(guac_client* client); /** * Associates the given audio buffer with the underlying audio stream which diff --git a/src/protocols/rdp/channels/cliprdr.c b/src/protocols/rdp/channels/cliprdr.c index f5fe2d75..b0e3345e 100644 --- a/src/protocols/rdp/channels/cliprdr.c +++ b/src/protocols/rdp/channels/cliprdr.c @@ -76,6 +76,9 @@ static UINT guac_rdp_cliprdr_send_format_list(CliprdrClientContext* cliprdr) { guac_rdp_clipboard* clipboard = (guac_rdp_clipboard*) cliprdr->custom; assert(clipboard != NULL); + guac_client* client = clipboard->client; + guac_rdp_client* rdp_client = (guac_rdp_client*) client->data; + /* We support CP-1252 and UTF-16 text */ CLIPRDR_FORMAT_LIST format_list = { .msgType = CB_FORMAT_LIST, @@ -86,10 +89,12 @@ static UINT guac_rdp_cliprdr_send_format_list(CliprdrClientContext* cliprdr) { .numFormats = 2 }; - guac_client_log(clipboard->client, GUAC_LOG_TRACE, "CLIPRDR: Sending " - "format list"); + guac_client_log(client, GUAC_LOG_TRACE, "CLIPRDR: Sending format list"); - return cliprdr->ClientFormatList(cliprdr, &format_list); + pthread_mutex_lock(&(rdp_client->message_lock)); + int retval = cliprdr->ClientFormatList(cliprdr, &format_list); + pthread_mutex_unlock(&(rdp_client->message_lock)); + return retval; } @@ -107,6 +112,17 @@ static UINT guac_rdp_cliprdr_send_format_list(CliprdrClientContext* cliprdr) { */ static UINT guac_rdp_cliprdr_send_capabilities(CliprdrClientContext* cliprdr) { + /* This function is only invoked within FreeRDP-specific handlers for + * CLIPRDR, which are not assigned, and thus not callable, until after the + * relevant guac_rdp_clipboard structure is allocated and associated with + * the CliprdrClientContext */ + guac_rdp_clipboard* clipboard = (guac_rdp_clipboard*) cliprdr->custom; + assert(clipboard != NULL); + + guac_client* client = clipboard->client; + guac_rdp_client* rdp_client = (guac_rdp_client*) client->data; + + /* We support CP-1252 and UTF-16 text */ CLIPRDR_GENERAL_CAPABILITY_SET cap_set = { .capabilitySetType = CB_CAPSTYPE_GENERAL, /* CLIPRDR specification requires that this is CB_CAPSTYPE_GENERAL, the only defined set type */ .capabilitySetLength = 12, /* The size of the capability set within the PDU - for CB_CAPSTYPE_GENERAL, this is ALWAYS 12 bytes */ @@ -119,7 +135,11 @@ static UINT guac_rdp_cliprdr_send_capabilities(CliprdrClientContext* cliprdr) { .capabilitySets = (CLIPRDR_CAPABILITY_SET*) &cap_set }; - return cliprdr->ClientCapabilities(cliprdr, &caps); + pthread_mutex_lock(&(rdp_client->message_lock)); + int retval = cliprdr->ClientCapabilities(cliprdr, &caps); + pthread_mutex_unlock(&(rdp_client->message_lock)); + + return retval; } @@ -190,6 +210,9 @@ static UINT guac_rdp_cliprdr_send_format_data_request( guac_rdp_clipboard* clipboard = (guac_rdp_clipboard*) cliprdr->custom; assert(clipboard != NULL); + guac_client* client = clipboard->client; + guac_rdp_client* rdp_client = (guac_rdp_client*) client->data; + /* Create new data request */ CLIPRDR_FORMAT_DATA_REQUEST data_request = { .requestedFormatId = format @@ -199,11 +222,14 @@ static UINT guac_rdp_cliprdr_send_format_data_request( * data is received via a Format Data Response PDU */ clipboard->requested_format = format; - guac_client_log(clipboard->client, GUAC_LOG_TRACE, "CLIPRDR: Sending " - "format data request."); + guac_client_log(client, GUAC_LOG_TRACE, "CLIPRDR: Sending format data request."); /* Send request */ - return cliprdr->ClientFormatDataRequest(cliprdr, &data_request); + pthread_mutex_lock(&(rdp_client->message_lock)); + int retval = cliprdr->ClientFormatDataRequest(cliprdr, &data_request); + pthread_mutex_unlock(&(rdp_client->message_lock)); + + return retval; } @@ -265,15 +291,19 @@ static UINT guac_rdp_cliprdr_format_list(CliprdrClientContext* cliprdr, guac_rdp_clipboard* clipboard = (guac_rdp_clipboard*) cliprdr->custom; assert(clipboard != NULL); - guac_client_log(clipboard->client, GUAC_LOG_TRACE, "CLIPRDR: Received " - "format list."); + guac_client* client = clipboard->client; + guac_rdp_client* rdp_client = (guac_rdp_client*) client->data; + + guac_client_log(client, GUAC_LOG_TRACE, "CLIPRDR: Received format list."); CLIPRDR_FORMAT_LIST_RESPONSE format_list_response = { .msgFlags = CB_RESPONSE_OK }; /* Report successful processing of format list */ + pthread_mutex_lock(&(rdp_client->message_lock)); cliprdr->ClientFormatListResponse(cliprdr, &format_list_response); + pthread_mutex_unlock(&(rdp_client->message_lock)); /* Prefer Unicode (in this case, UTF-16) */ if (guac_rdp_cliprdr_format_supported(format_list, CF_UNICODETEXT)) @@ -284,9 +314,9 @@ static UINT guac_rdp_cliprdr_format_list(CliprdrClientContext* cliprdr, return guac_rdp_cliprdr_send_format_data_request(cliprdr, CF_TEXT); /* Ignore any unsupported data */ - guac_client_log(clipboard->client, GUAC_LOG_DEBUG, "Ignoring unsupported " - "clipboard data. Only Unicode and text clipboard formats are " - "currently supported."); + guac_client_log(client, GUAC_LOG_DEBUG, "Ignoring unsupported clipboard " + "data. Only Unicode and text clipboard formats are currently " + "supported."); return CHANNEL_RC_OK; @@ -320,8 +350,10 @@ static UINT guac_rdp_cliprdr_format_data_request(CliprdrClientContext* cliprdr, guac_rdp_clipboard* clipboard = (guac_rdp_clipboard*) cliprdr->custom; assert(clipboard != NULL); - guac_client_log(clipboard->client, GUAC_LOG_TRACE, "CLIPRDR: Received " - "format data request."); + guac_client* client = clipboard->client; + guac_rdp_client* rdp_client = (guac_rdp_client*) client->data; + + guac_client_log(client, GUAC_LOG_TRACE, "CLIPRDR: Received format data request."); guac_iconv_write* writer; const char* input = clipboard->clipboard->buffer; @@ -341,11 +373,11 @@ static UINT guac_rdp_cliprdr_format_data_request(CliprdrClientContext* cliprdr, /* Warn if clipboard data cannot be sent as intended due to a violation * of the CLIPRDR spec */ default: - guac_client_log(clipboard->client, GUAC_LOG_WARNING, "Received " - "clipboard data cannot be sent to the RDP server because " - "the RDP server has requested a clipboard format which " - "was not declared as available. This violates the " - "specification for the CLIPRDR channel."); + guac_client_log(client, GUAC_LOG_WARNING, "Received clipboard " + "data cannot be sent to the RDP server because the RDP " + "server has requested a clipboard format which was not " + "declared as available. This violates the specification " + "for the CLIPRDR channel."); free(output); return CHANNEL_RC_OK; @@ -363,10 +395,12 @@ static UINT guac_rdp_cliprdr_format_data_request(CliprdrClientContext* cliprdr, .msgFlags = CB_RESPONSE_OK }; - guac_client_log(clipboard->client, GUAC_LOG_TRACE, "CLIPRDR: Sending " - "format data response."); + guac_client_log(client, GUAC_LOG_TRACE, "CLIPRDR: Sending format data response."); + pthread_mutex_lock(&(rdp_client->message_lock)); UINT result = cliprdr->ClientFormatDataResponse(cliprdr, &data_response); + pthread_mutex_unlock(&(rdp_client->message_lock)); + free(start); return result; @@ -407,9 +441,9 @@ static UINT guac_rdp_cliprdr_format_data_response(CliprdrClientContext* cliprdr, /* Ignore received data if copy has been disabled */ if (settings->disable_copy) { - guac_client_log(clipboard->client, GUAC_LOG_DEBUG, "Ignoring received " - "clipboard data as copying from within the remote desktop has " - "been explicitly disabled."); + guac_client_log(client, GUAC_LOG_DEBUG, "Ignoring received clipboard " + "data as copying from within the remote desktop has been " + "explicitly disabled."); return CHANNEL_RC_OK; } @@ -439,9 +473,8 @@ static UINT guac_rdp_cliprdr_format_data_response(CliprdrClientContext* cliprdr, * here. The values which may be stored within requested_format are * completely within our control. */ default: - guac_client_log(clipboard->client, GUAC_LOG_DEBUG, "Requested " - "clipboard data in unsupported format (0x%X).", - clipboard->requested_format); + guac_client_log(client, GUAC_LOG_DEBUG, "Requested clipboard data " + "in unsupported format (0x%X).", clipboard->requested_format); return CHANNEL_RC_OK; } @@ -453,7 +486,7 @@ static UINT guac_rdp_cliprdr_format_data_response(CliprdrClientContext* cliprdr, int length = strnlen(received_data, sizeof(received_data)); guac_common_clipboard_reset(clipboard->clipboard, "text/plain"); guac_common_clipboard_append(clipboard->clipboard, received_data, length); - guac_common_clipboard_send(clipboard->clipboard, clipboard->client); + guac_common_clipboard_send(clipboard->clipboard, client); } return CHANNEL_RC_OK; diff --git a/src/protocols/rdp/channels/common-svc.c b/src/protocols/rdp/channels/common-svc.c index 8b076341..ce3bf8f2 100644 --- a/src/protocols/rdp/channels/common-svc.c +++ b/src/protocols/rdp/channels/common-svc.c @@ -89,12 +89,16 @@ void guac_rdp_common_svc_write(guac_rdp_common_svc* svc, return; } + guac_rdp_client* rdp_client = (guac_rdp_client*) svc->client->data; + /* NOTE: The wStream sent via pVirtualChannelWriteEx will automatically be * freed later with a call to Stream_Free() when handling the * corresponding write cancel/completion event. */ + pthread_mutex_lock(&(rdp_client->message_lock)); svc->_entry_points.pVirtualChannelWriteEx(svc->_init_handle, svc->_open_handle, Stream_Buffer(output_stream), Stream_GetPosition(output_stream), output_stream); + pthread_mutex_unlock(&(rdp_client->message_lock)); } diff --git a/src/protocols/rdp/channels/disp.c b/src/protocols/rdp/channels/disp.c index a25f6fa9..95dc6d99 100644 --- a/src/protocols/rdp/channels/disp.c +++ b/src/protocols/rdp/channels/disp.c @@ -31,9 +31,10 @@ #include #include -guac_rdp_disp* guac_rdp_disp_alloc() { +guac_rdp_disp* guac_rdp_disp_alloc(guac_client* client) { guac_rdp_disp* disp = malloc(sizeof(guac_rdp_disp)); + disp->client = client; /* Not yet connected */ disp->disp = NULL; @@ -220,8 +221,17 @@ void guac_rdp_disp_update_size(guac_rdp_disp* disp, }}; /* Send display update notification if display channel is connected */ - if (disp->disp != NULL) + if (disp->disp != NULL) { + + guac_client* client = disp->client; + guac_rdp_client* rdp_client = (guac_rdp_client*) client->data; + + pthread_mutex_lock(&(rdp_client->message_lock)); disp->disp->SendMonitorLayout(disp->disp, 1, monitors); + pthread_mutex_unlock(&(rdp_client->message_lock)); + + } + } } diff --git a/src/protocols/rdp/channels/disp.h b/src/protocols/rdp/channels/disp.h index 2aff7c2d..4739c16c 100644 --- a/src/protocols/rdp/channels/disp.h +++ b/src/protocols/rdp/channels/disp.h @@ -48,6 +48,11 @@ */ typedef struct guac_rdp_disp { + /** + * The guac_client instance handling the relevant RDP connection. + */ + guac_client* client; + /** * Display control interface. */ @@ -81,10 +86,13 @@ typedef struct guac_rdp_disp { * Allocates a new display update module, which will ultimately control the * display update channel once connected. * + * @param client + * The guac_client instance handling the relevant RDP connection. + * * @return * A newly-allocated display update module. */ -guac_rdp_disp* guac_rdp_disp_alloc(); +guac_rdp_disp* guac_rdp_disp_alloc(guac_client* client); /** * Frees the resources associated with support for the RDP Display Update diff --git a/src/protocols/rdp/channels/rail.c b/src/protocols/rdp/channels/rail.c index 5f105c8e..6f8d7f7f 100644 --- a/src/protocols/rdp/channels/rail.c +++ b/src/protocols/rdp/channels/rail.c @@ -86,7 +86,10 @@ static UINT guac_rdp_rail_complete_handshake(RailClientContext* rail) { }; /* Send client handshake response */ + pthread_mutex_lock(&(rdp_client->message_lock)); status = rail->ClientHandshake(rail, &handshake); + pthread_mutex_unlock(&(rdp_client->message_lock)); + if (status != CHANNEL_RC_OK) return status; @@ -95,7 +98,10 @@ static UINT guac_rdp_rail_complete_handshake(RailClientContext* rail) { }; /* Send client status */ + pthread_mutex_lock(&(rdp_client->message_lock)); status = rail->ClientInformation(rail, &client_status); + pthread_mutex_unlock(&(rdp_client->message_lock)); + if (status != CHANNEL_RC_OK) return status; @@ -139,7 +145,10 @@ static UINT guac_rdp_rail_complete_handshake(RailClientContext* rail) { }; /* Send client system parameters */ + pthread_mutex_lock(&(rdp_client->message_lock)); status = rail->ClientSystemParam(rail, &sysparam); + pthread_mutex_unlock(&(rdp_client->message_lock)); + if (status != CHANNEL_RC_OK) return status; @@ -151,7 +160,11 @@ static UINT guac_rdp_rail_complete_handshake(RailClientContext* rail) { }; /* Execute desired RemoteApp command */ - return rail->ClientExecute(rail, &exec); + pthread_mutex_lock(&(rdp_client->message_lock)); + status = rail->ClientExecute(rail, &exec); + pthread_mutex_unlock(&(rdp_client->message_lock)); + + return status; } diff --git a/src/protocols/rdp/channels/rdpei.c b/src/protocols/rdp/channels/rdpei.c index f1c5d09e..af1771d4 100644 --- a/src/protocols/rdp/channels/rdpei.c +++ b/src/protocols/rdp/channels/rdpei.c @@ -32,9 +32,10 @@ #include #include -guac_rdp_rdpei* guac_rdp_rdpei_alloc() { +guac_rdp_rdpei* guac_rdp_rdpei_alloc(guac_client* client) { guac_rdp_rdpei* rdpei = malloc(sizeof(guac_rdp_rdpei)); + rdpei->client = client; /* Not yet connected */ rdpei->rdpei = NULL; @@ -107,6 +108,9 @@ void guac_rdp_rdpei_load_plugin(rdpContext* context) { int guac_rdp_rdpei_touch_update(guac_rdp_rdpei* rdpei, int id, int x, int y, double force) { + guac_client* client = rdpei->client; + guac_rdp_client* rdp_client = (guac_rdp_client*) client->data; + int contact_id; /* Ignored */ /* Track touches only if channel is connected */ @@ -149,20 +153,31 @@ int guac_rdp_rdpei_touch_update(guac_rdp_rdpei* rdpei, int id, int x, int y, if (!touch->active) return 1; + pthread_mutex_lock(&(rdp_client->message_lock)); context->TouchEnd(context, id, x, y, &contact_id); + pthread_mutex_unlock(&(rdp_client->message_lock)); + touch->active = 0; } /* Signal the start of a touch if this is the first we've seen it */ else if (!touch->active) { + + pthread_mutex_lock(&(rdp_client->message_lock)); context->TouchBegin(context, id, x, y, &contact_id); + pthread_mutex_unlock(&(rdp_client->message_lock)); + touch->active = 1; + } /* Established touches need only be updated */ - else + else { + pthread_mutex_lock(&(rdp_client->message_lock)); context->TouchUpdate(context, id, x, y, &contact_id); + pthread_mutex_unlock(&(rdp_client->message_lock)); + } return 0; diff --git a/src/protocols/rdp/channels/rdpei.h b/src/protocols/rdp/channels/rdpei.h index 5ca10c30..5b16d31a 100644 --- a/src/protocols/rdp/channels/rdpei.h +++ b/src/protocols/rdp/channels/rdpei.h @@ -66,6 +66,11 @@ typedef struct guac_rdp_rdpei_touch { */ typedef struct guac_rdp_rdpei { + /** + * The guac_client instance handling the relevant RDP connection. + */ + guac_client* client; + /** * RDPEI control interface. */ @@ -83,10 +88,13 @@ typedef struct guac_rdp_rdpei { * channel once connected. The RDPEI channel allows multi-touch input * events to be sent to the RDP server. * + * @param client + * The guac_client instance handling the relevant RDP connection. + * * @return * A newly-allocated RDPEI module. */ -guac_rdp_rdpei* guac_rdp_rdpei_alloc(); +guac_rdp_rdpei* guac_rdp_rdpei_alloc(guac_client* client); /** * Frees the resources associated with support for the RDPEI channel. Only diff --git a/src/protocols/rdp/client.c b/src/protocols/rdp/client.c index 50808dcf..4d8171f5 100644 --- a/src/protocols/rdp/client.c +++ b/src/protocols/rdp/client.c @@ -145,10 +145,10 @@ int guac_client_init(guac_client* client, int argc, char** argv) { rdp_client->clipboard = guac_rdp_clipboard_alloc(client); /* Init display update module */ - rdp_client->disp = guac_rdp_disp_alloc(); + rdp_client->disp = guac_rdp_disp_alloc(client); /* Init multi-touch support module (RDPEI) */ - rdp_client->rdpei = guac_rdp_rdpei_alloc(); + rdp_client->rdpei = guac_rdp_rdpei_alloc(client); /* Redirect FreeRDP log messages to guac_client_log() */ guac_rdp_redirect_wlog(client); @@ -158,8 +158,9 @@ int guac_client_init(guac_client* client, int argc, char** argv) { pthread_mutexattr_settype(&(rdp_client->attributes), PTHREAD_MUTEX_RECURSIVE); - /* Initalize the lock */ + /* Init required locks */ pthread_rwlock_init(&(rdp_client->lock), NULL); + pthread_mutex_init(&(rdp_client->message_lock), &(rdp_client->attributes)); /* Set handlers */ client->join_handler = guac_rdp_user_join_handler; @@ -226,6 +227,7 @@ int guac_rdp_client_free_handler(guac_client* client) { guac_rdp_audio_buffer_free(rdp_client->audio_input); pthread_rwlock_destroy(&(rdp_client->lock)); + pthread_mutex_destroy(&(rdp_client->message_lock)); /* Free client data */ free(rdp_client); diff --git a/src/protocols/rdp/input.c b/src/protocols/rdp/input.c index 0c36d614..54e99f15 100644 --- a/src/protocols/rdp/input.c +++ b/src/protocols/rdp/input.c @@ -54,8 +54,11 @@ int guac_rdp_user_mouse_handler(guac_user* user, int x, int y, int mask) { guac_common_recording_report_mouse(rdp_client->recording, x, y, mask); /* If button mask unchanged, just send move event */ - if (mask == rdp_client->mouse_button_mask) + if (mask == rdp_client->mouse_button_mask) { + pthread_mutex_lock(&(rdp_client->message_lock)); rdp_inst->input->MouseEvent(rdp_inst->input, PTR_FLAGS_MOVE, x, y); + pthread_mutex_unlock(&(rdp_client->message_lock)); + } /* Otherwise, send events describing button change */ else { @@ -75,7 +78,9 @@ int guac_rdp_user_mouse_handler(guac_user* user, int x, int y, int mask) { if (released_mask & 0x02) flags |= PTR_FLAGS_BUTTON3; if (released_mask & 0x04) flags |= PTR_FLAGS_BUTTON2; + pthread_mutex_lock(&(rdp_client->message_lock)); rdp_inst->input->MouseEvent(rdp_inst->input, flags, x, y); + pthread_mutex_unlock(&(rdp_client->message_lock)); } @@ -91,7 +96,9 @@ int guac_rdp_user_mouse_handler(guac_user* user, int x, int y, int mask) { if (pressed_mask & 0x10) flags |= PTR_FLAGS_WHEEL | PTR_FLAGS_WHEEL_NEGATIVE | 0x88; /* Send event */ + pthread_mutex_lock(&(rdp_client->message_lock)); rdp_inst->input->MouseEvent(rdp_inst->input, flags, x, y); + pthread_mutex_unlock(&(rdp_client->message_lock)); } @@ -99,18 +106,18 @@ int guac_rdp_user_mouse_handler(guac_user* user, int x, int y, int mask) { if (pressed_mask & 0x18) { /* Down */ - if (pressed_mask & 0x08) - rdp_inst->input->MouseEvent( - rdp_inst->input, - PTR_FLAGS_WHEEL | 0x78, - x, y); + if (pressed_mask & 0x08) { + pthread_mutex_lock(&(rdp_client->message_lock)); + rdp_inst->input->MouseEvent(rdp_inst->input, PTR_FLAGS_WHEEL | 0x78, x, y); + pthread_mutex_unlock(&(rdp_client->message_lock)); + } /* Up */ - if (pressed_mask & 0x10) - rdp_inst->input->MouseEvent( - rdp_inst->input, - PTR_FLAGS_WHEEL | PTR_FLAGS_WHEEL_NEGATIVE | 0x88, - x, y); + if (pressed_mask & 0x10) { + pthread_mutex_lock(&(rdp_client->message_lock)); + rdp_inst->input->MouseEvent(rdp_inst->input, PTR_FLAGS_WHEEL | PTR_FLAGS_WHEEL_NEGATIVE | 0x88, x, y); + pthread_mutex_unlock(&(rdp_client->message_lock)); + } } diff --git a/src/protocols/rdp/keyboard.c b/src/protocols/rdp/keyboard.c index b027bde5..a8a6f8b7 100644 --- a/src/protocols/rdp/keyboard.c +++ b/src/protocols/rdp/keyboard.c @@ -104,8 +104,9 @@ static void guac_rdp_send_key_event(guac_rdp_client* rdp_client, return; /* Send actual key */ - rdp_inst->input->KeyboardEvent(rdp_inst->input, - flags | pressed_flags, scancode); + pthread_mutex_lock(&(rdp_client->message_lock)); + rdp_inst->input->KeyboardEvent(rdp_inst->input, flags | pressed_flags, scancode); + pthread_mutex_unlock(&(rdp_client->message_lock)); } @@ -132,9 +133,9 @@ static void guac_rdp_send_unicode_event(guac_rdp_client* rdp_client, return; /* Send Unicode event */ - rdp_inst->input->UnicodeKeyboardEvent( - rdp_inst->input, - 0, codepoint); + pthread_mutex_lock(&(rdp_client->message_lock)); + rdp_inst->input->UnicodeKeyboardEvent(rdp_inst->input, 0, codepoint); + pthread_mutex_unlock(&(rdp_client->message_lock)); } @@ -161,7 +162,9 @@ static void guac_rdp_send_synchronize_event(guac_rdp_client* rdp_client, return; /* Synchronize lock key states */ + pthread_mutex_lock(&(rdp_client->message_lock)); rdp_inst->input->SynchronizeEvent(rdp_inst->input, flags); + pthread_mutex_unlock(&(rdp_client->message_lock)); } diff --git a/src/protocols/rdp/plugins/guacai/guacai-messages.c b/src/protocols/rdp/plugins/guacai/guacai-messages.c index eb9a79da..2bfedb44 100644 --- a/src/protocols/rdp/plugins/guacai/guacai-messages.c +++ b/src/protocols/rdp/plugins/guacai/guacai-messages.c @@ -247,6 +247,8 @@ static void guac_rdp_ai_send_formatchange(IWTSVirtualChannel* channel, void guac_rdp_ai_process_version(guac_client* client, IWTSVirtualChannel* channel, wStream* stream) { + guac_rdp_client* rdp_client = (guac_rdp_client*) client->data; + /* Verify we have at least 4 bytes available (UINT32) */ if (Stream_GetRemainingLength(stream) < 4) { guac_client_log(client, GUAC_LOG_WARNING, "Audio input Versoin PDU " @@ -269,8 +271,9 @@ void guac_rdp_ai_process_version(guac_client* client, Stream_Write_UINT32(response, 1); /* Version */ /* Send response */ - channel->Write(channel, (UINT32) Stream_GetPosition(response), - Stream_Buffer(response), NULL); + pthread_mutex_lock(&(rdp_client->message_lock)); + channel->Write(channel, (UINT32) Stream_GetPosition(response), Stream_Buffer(response), NULL); + pthread_mutex_unlock(&(rdp_client->message_lock)); Stream_Free(response, TRUE); } @@ -313,26 +316,35 @@ void guac_rdp_ai_process_formats(guac_client* client, format.channels, format.bps / 8); /* Accept single format */ + pthread_mutex_lock(&(rdp_client->message_lock)); guac_rdp_ai_send_incoming_data(channel); guac_rdp_ai_send_formats(channel, &format, 1); + pthread_mutex_unlock(&(rdp_client->message_lock)); return; } /* No formats available */ guac_client_log(client, GUAC_LOG_WARNING, "AUDIO_INPUT: No WAVE format."); + pthread_mutex_lock(&(rdp_client->message_lock)); guac_rdp_ai_send_incoming_data(channel); guac_rdp_ai_send_formats(channel, NULL, 0); + pthread_mutex_unlock(&(rdp_client->message_lock)); } -void guac_rdp_ai_flush_packet(char* buffer, int length, void* data) { +void guac_rdp_ai_flush_packet(guac_rdp_audio_buffer* audio_buffer, + char* buffer, int length, void* data) { + guac_client* client = audio_buffer->client; + guac_rdp_client* rdp_client = (guac_rdp_client*) client->data; IWTSVirtualChannel* channel = (IWTSVirtualChannel*) data; /* Send data over channel */ + pthread_mutex_lock(&(rdp_client->message_lock)); guac_rdp_ai_send_incoming_data(channel); guac_rdp_ai_send_data(channel, buffer, length); + pthread_mutex_unlock(&(rdp_client->message_lock)); } @@ -363,8 +375,10 @@ void guac_rdp_ai_process_open(guac_client* client, audio_buffer->out_format.bps); /* Success */ + pthread_mutex_lock(&(rdp_client->message_lock)); guac_rdp_ai_send_formatchange(channel, initial_format); guac_rdp_ai_send_open_reply(channel, 0); + pthread_mutex_unlock(&(rdp_client->message_lock)); /* Begin receiving audio data */ guac_rdp_audio_buffer_begin(audio_buffer, packet_frames, diff --git a/src/protocols/rdp/rdp.c b/src/protocols/rdp/rdp.c index 66537156..1c528e66 100644 --- a/src/protocols/rdp/rdp.c +++ b/src/protocols/rdp/rdp.c @@ -107,7 +107,7 @@ BOOL rdp_freerdp_pre_connect(freerdp* instance) { /* Load "AUDIO_INPUT" plugin for audio input*/ if (settings->enable_audio_input) { - rdp_client->audio_input = guac_rdp_audio_buffer_alloc(); + rdp_client->audio_input = guac_rdp_audio_buffer_alloc(client); guac_rdp_audio_load_plugin(instance->context); } @@ -564,13 +564,16 @@ static int guac_rdp_handle_connection(guac_client* client) { guac_timestamp frame_end; int frame_remaining; - /* Check the libfreerdp fds */ - if (!freerdp_check_event_handles(rdp_inst->context)) { + /* Handle any queued FreeRDP events (this may result in RDP + * messages being sent) */ + pthread_mutex_lock(&(rdp_client->message_lock)); + int event_result = freerdp_check_event_handles(rdp_inst->context); + pthread_mutex_unlock(&(rdp_client->message_lock)); - /* Flag connection failure */ + /* Abort if FreeRDP event handling fails */ + if (!event_result) { wait_result = -1; break; - } /* Calculate time remaining in frame */ @@ -634,7 +637,9 @@ static int guac_rdp_handle_connection(guac_client* client) { } /* Disconnect client and channels */ + pthread_mutex_lock(&(rdp_client->message_lock)); freerdp_disconnect(rdp_inst); + pthread_mutex_unlock(&(rdp_client->message_lock)); /* Clean up FreeRDP internal GDI implementation */ gdi_free(rdp_inst); diff --git a/src/protocols/rdp/rdp.h b/src/protocols/rdp/rdp.h index cc537966..ab6c3c52 100644 --- a/src/protocols/rdp/rdp.h +++ b/src/protocols/rdp/rdp.h @@ -172,6 +172,12 @@ typedef struct guac_rdp_client { */ pthread_rwlock_t lock; + /** + * Lock which synchronizes the sending of each RDP message, ensuring + * attempts to send RDP messages never overlap. + */ + pthread_mutex_t message_lock; + } guac_rdp_client; /**