diff --git a/common/xrdp_constants.h b/common/xrdp_constants.h index 8983d922..77398829 100644 --- a/common/xrdp_constants.h +++ b/common/xrdp_constants.h @@ -87,6 +87,9 @@ #define XRDP_MAX_BITMAP_CACHE_IDX 2000 #define XRDP_BITMAP_CACHE_ENTRIES 2048 +/* Max number of dynamic channels we support */ +#define DRDYNVC_CHANNEL_COUNT 256 + /* * Constants come from ITU-T Recommendations */ diff --git a/libxrdp/libxrdp.h b/libxrdp/libxrdp.h index 80593c4c..2663bf92 100644 --- a/libxrdp/libxrdp.h +++ b/libxrdp/libxrdp.h @@ -143,6 +143,7 @@ struct xrdp_drdynvc int status; /* see XRDP_DRDYNVC_STATUS_* */ int flags; int pad0; + struct dyn_dechunker *dc; // Use to dechunk fragments int (*open_response)(struct xrdp_process *id, int chan_id, int creation_status); int (*close_response)(struct xrdp_process *id, int chan_id); @@ -161,7 +162,7 @@ struct xrdp_channel int drdynvc_channel_id; int drdynvc_state; struct vc_dechunker *drdynvc_dc; - struct xrdp_drdynvc drdynvcs[256]; + struct xrdp_drdynvc drdynvcs[DRDYNVC_CHANNEL_COUNT]; }; /* rdp */ diff --git a/libxrdp/libxrdpinc.h b/libxrdp/libxrdpinc.h index 0b48d356..0745d594 100644 --- a/libxrdp/libxrdpinc.h +++ b/libxrdp/libxrdpinc.h @@ -91,6 +91,9 @@ struct xrdp_drdynvc_procs int (*open_response)(struct xrdp_process *id, int chan_id, int creation_status); int (*close_response)(struct xrdp_process *id, int chan_id); + // Only set data_first if you want to be responsible for + // defragging your own PDUs. Otherwise the channel + // process will do it for you. int (*data_first)(struct xrdp_process *id, int chan_id, struct stream *s, int total_bytes); int (*data)(struct xrdp_process *id, int chan_id, struct stream *s); diff --git a/libxrdp/xrdp_channel.c b/libxrdp/xrdp_channel.c index 46202b86..09926a57 100644 --- a/libxrdp/xrdp_channel.c +++ b/libxrdp/xrdp_channel.c @@ -74,11 +74,16 @@ xrdp_channel_create(struct xrdp_sec *owner, struct xrdp_mcs *mcs_layer) void xrdp_channel_delete(struct xrdp_channel *self) { + int i; if (self == 0) { return; } vc_dechunker_free(self->drdynvc_dc); + for (i = 0 ; i < DRDYNVC_CHANNEL_COUNT ; ++i) + { + dyn_dechunker_free(self->drdynvcs[i].dc); + } g_memset(self, 0, sizeof(struct xrdp_channel)); g_free(self); } @@ -338,10 +343,10 @@ drdynvc_process_open_channel_response(struct xrdp_channel *self, in_uint32_le(s, creation_status); /* CreationStatus */ LOG_DEVEL(LOG_LEVEL_TRACE, "Received [MS-RDPEDYC] DYNVC_CREATE_RSP " "ChannelId %d, CreationStatus %d", chan_id, creation_status); - if (chan_id > 255) + if (chan_id >= DRDYNVC_CHANNEL_COUNT) { - LOG(LOG_LEVEL_ERROR, "Received [MS-RDPEDYC] DYNVC_CREATE_RSP for an " - "invalid channel id. Max allowed 255, received %d", chan_id); + LOG(LOG_LEVEL_ERROR, "Received [MS-RDPEDYC] DYNVC_CREATE_RSP " + "for an invalid channel id %d", chan_id); return 1; } @@ -354,6 +359,8 @@ drdynvc_process_open_channel_response(struct xrdp_channel *self, else { drdynvc->status = XRDP_DRDYNVC_STATUS_CLOSED; + dyn_dechunker_free(drdynvc->dc); + drdynvc->dc = NULL; } LOG_DEVEL(LOG_LEVEL_DEBUG, "Dynamic Virtual Channel %s (%d) updated: status = %s", @@ -392,15 +399,28 @@ drdynvc_process_close_channel_response(struct xrdp_channel *self, LOG_DEVEL(LOG_LEVEL_TRACE, "Received [MS-RDPEDYC] DYNVC_CLOSE " "ChannelId %d", chan_id); session = self->sec_layer->rdp_layer->session; - if (chan_id > 255) + if (chan_id >= DRDYNVC_CHANNEL_COUNT) { LOG(LOG_LEVEL_ERROR, "Received message for an invalid " - "channel id. channel id %d", chan_id); + "channel id %d", chan_id); return 1; } drdynvc = self->drdynvcs + chan_id; + + if (dyn_dechunker_pending(drdynvc->dc)) + { + // The last PDU wasn't completed + LOG(LOG_LEVEL_WARNING, + "Dynamic Virtual Channel %s (%d) closing with outstanding data", + XRDP_DRDYNVC_CHANNEL_ID_TO_NAME(self, chan_id), + chan_id); + } + drdynvc->status = XRDP_DRDYNVC_STATUS_CLOSED; + dyn_dechunker_free(drdynvc->dc); + drdynvc->dc = NULL; + LOG_DEVEL(LOG_LEVEL_DEBUG, "Dynamic Virtual Channel %s (%d) updated: status = %s", XRDP_DRDYNVC_CHANNEL_ID_TO_NAME(self, chan_id), @@ -426,75 +446,113 @@ static int drdynvc_process_data_first(struct xrdp_channel *self, int cmd, struct stream *s) { - struct xrdp_session *session; uint32_t chan_id; - int len; - int bytes; - int total_bytes; - struct xrdp_drdynvc *drdynvc; - + int rv = 0; if (drdynvc_get_chan_id(s, cmd, &chan_id) != 0) /* ChannelId */ { LOG(LOG_LEVEL_ERROR, - "Parsing [MS-RDPEDYC] DYNVC_DATA_FIRST failed"); - return 1; - } - len = (cmd >> 2) & 0x03; - if (len == 0) - { - if (!s_check_rem_and_log(s, 1, "Parsing [MS-RDPEDYC] DYNVC_DATA_FIRST")) - { - return 1; - } - in_uint8(s, total_bytes); /* Length */ - } - else if (len == 1) - { - if (!s_check_rem_and_log(s, 2, "Parsing [MS-RDPEDYC] DYNVC_DATA_FIRST")) - { - return 1; - } - in_uint16_le(s, total_bytes); /* Length */ + "Parsing [MS-RDPEDYC] DYNVC_DATA_FIRST channel ID failed"); + rv = 1; } else { - if (!s_check_rem_and_log(s, 4, "Parsing [MS-RDPEDYC] DYNVC_DATA_FIRST")) + int len = (cmd >> 2) & 0x03; + int total_bytes; + switch (len) { - return 1; + case 0: + // Technically this can't happen as DATA_FIRST is only used for + // PDUs over 1590 bytes ([MS-RDPEDYC] 2.2.3) + if (!s_check_rem(s, 1)) + { + goto short_pdu; + } + in_uint8(s, total_bytes); /* Length */ + break; + + case 1: + if (!s_check_rem(s, 2)) + { + goto short_pdu; + } + in_uint16_le(s, total_bytes); /* Length */ + break; + + case 2: + if (!s_check_rem(s, 4)) + { + goto short_pdu; + } + in_uint32_le(s, total_bytes); /* Length */ + break; + + default: + LOG(LOG_LEVEL_ERROR, + "[MS-RDPEDYC] DYNVC_DATA_FIRST has bad Len field"); + return 1; } - in_uint32_le(s, total_bytes); /* Length */ - } - bytes = (int) (s->end - s->p); - LOG_DEVEL(LOG_LEVEL_TRACE, "Received [MS-RDPEDYC] DYNVC_DATA_FIRST " - "ChannelId %d, Length %d, Data (omitted from the log)", - chan_id, total_bytes); - // See [MS-RDPBCGR] 2.2.3 - if (total_bytes < 1590 || bytes > total_bytes) - { - LOG(LOG_LEVEL_ERROR, - "Badly formed DYNVC_DATA_FIRST PDU received on dynamic channel %d", - chan_id); - return 1; + LOG_DEVEL(LOG_LEVEL_TRACE, "Received [MS-RDPEDYC] DYNVC_DATA_FIRST " + "ChannelId %d, Length %d, Data (omitted from the log)", + chan_id, total_bytes); + + if (chan_id >= DRDYNVC_CHANNEL_COUNT) + { + LOG(LOG_LEVEL_ERROR, "Received [MS-RDPEDYC] DYNVC_DATA_FIRST " + "for an invalid channel id %d", chan_id); + rv = 1; + } + else + { + struct xrdp_drdynvc *drdynvc = self->drdynvcs + chan_id; + + if (drdynvc->data_first != NULL) + { + // Caller has requested to defragment PDUs themself + struct xrdp_session *session = + self->sec_layer->rdp_layer->session; + rv = drdynvc->data_first(session->id, chan_id, s, total_bytes); + } + else + { + // Get the dechunker working on the stream + enum dyn_dechunker_status status; + status = dyn_dechunker_process_first_chunk(drdynvc->dc, + s, total_bytes); + switch (status) + { + case E_DYN_INLINE_CHUNK: + // Pass through as data PDU + if (drdynvc->data != NULL) + { + struct xrdp_session *session = + self->sec_layer->rdp_layer->session; + rv = drdynvc->data(session->id, chan_id, s); + } + break; + + case E_DYN_IN_PROGRESS: + break; + + case E_DYN_ERROR: + rv = 1; + break; + + default: + LOG(LOG_LEVEL_ERROR, + "Dechunker returned unknown error %d", + (int)status); + rv = 1; + } + } + } } - session = self->sec_layer->rdp_layer->session; - if (chan_id > 255) - { - LOG(LOG_LEVEL_ERROR, "Received [MS-RDPEDYC] DYNVC_DATA_FIRST for an " - "invalid channel id. Max allowed 255, received %d", chan_id); - return 1; - } - drdynvc = self->drdynvcs + chan_id; - if (drdynvc->data_first != NULL) - { - return drdynvc->data_first(session->id, chan_id, s, total_bytes); - } - LOG_DEVEL(LOG_LEVEL_WARNING, "Dynamic Virtual Channel %s (%d): " - "callback 'data_first' is NULL", - XRDP_DRDYNVC_CHANNEL_ID_TO_NAME(self, chan_id), - chan_id); - return 0; + return rv; + +short_pdu: + LOG(LOG_LEVEL_ERROR, "[MS-RDPEDYC] DYNVC_DATA_FIRST is too short"); + return 1; } /*****************************************************************************/ @@ -505,37 +563,86 @@ static int drdynvc_process_data(struct xrdp_channel *self, int cmd, struct stream *s) { - struct xrdp_session *session; + int rv = 0; uint32_t chan_id; - int bytes; - struct xrdp_drdynvc *drdynvc; if (drdynvc_get_chan_id(s, cmd, &chan_id) != 0) /* ChannelId */ { - LOG(LOG_LEVEL_ERROR, "drdynvc_process_data: drdynvc_get_chan_id failed"); - return 1; + LOG(LOG_LEVEL_ERROR, + "Parsing [MS-RDPEDYC] DYNVC_DATA channel ID failed"); + rv = 1; } - bytes = (int) (s->end - s->p); - LOG_DEVEL(LOG_LEVEL_TRACE, "Received [MS-RDPEDYC] DYNVC_DATA " - "ChannelId %d, (re-assembled) Length %d, Data (omitted from the log)", - chan_id, bytes); - session = self->sec_layer->rdp_layer->session; - if (chan_id > 255) + else { - LOG(LOG_LEVEL_ERROR, "Received DYNVC_DATA PDU for an invalid " - "channel id. channel id %d", chan_id); - return 1; + LOG_DEVEL(LOG_LEVEL_TRACE, "Received [MS-RDPEDYC] DYNVC_DATA " + "ChannelId %d, Length %d, Data (omitted from the log)", + chan_id, s_rem(s)); + if (chan_id >= DRDYNVC_CHANNEL_COUNT) + { + LOG(LOG_LEVEL_ERROR, "Received DYNVC_DATA PDU for an invalid " + "channel id %d", chan_id); + rv = 1; + } + else + { + struct stream *ls = NULL; // Set if the application to be called + int free_ls = 0; // Set if we need to clear ls when we're done + struct xrdp_drdynvc *drdynvc = self->drdynvcs + chan_id; + if (drdynvc->data_first != NULL) + { + // Caller is processing all PDUs directly + ls = s; + } + else + { + // Pass the PDU to the dechunker + enum dyn_dechunker_status dechunker_status = + dyn_dechunker_process_data_chunk(drdynvc->dc, s); + switch (dechunker_status) + { + case E_DYN_INLINE_CHUNK: + ls = s; + break; + + case E_DYN_IN_PROGRESS: + break; + + case E_DYN_READY: + ls = dyn_dechunker_get_stream(drdynvc->dc); + // We now own the stream, so must delete it + free_ls = 1; + break; + + case E_DYN_ERROR: + // Error has been logged + rv = 1; + break; + + default: + LOG(LOG_LEVEL_ERROR, + "Dechunker returned unknown error %d", + (int)dechunker_status); + rv = 1; + } + } + + if (ls != NULL) + { + if (drdynvc->data != NULL) + { + struct xrdp_session *session = + self->sec_layer->rdp_layer->session; + rv = drdynvc->data(session->id, chan_id, ls); + } + + if (free_ls) + { + free_stream(ls); + } + } + } } - drdynvc = self->drdynvcs + chan_id; - if (drdynvc->data != NULL) - { - return drdynvc->data(session->id, chan_id, s); - } - LOG_DEVEL(LOG_LEVEL_WARNING, "Dynamic Virtual Channel %s (%d): " - "callback 'data' is NULL", - XRDP_DRDYNVC_CHANNEL_ID_TO_NAME(self, chan_id), - chan_id); - return 0; + return rv; } /*****************************************************************************/ @@ -819,6 +926,7 @@ xrdp_channel_drdynvc_open(struct xrdp_channel *self, const char *name, int total_data_len; int static_flags; char *cmd_ptr; + struct dyn_dechunker *dc = NULL; make_stream(s); init_stream(s, 8192); @@ -826,8 +934,18 @@ xrdp_channel_drdynvc_open(struct xrdp_channel *self, const char *name, { LOG(LOG_LEVEL_ERROR, "xrdp_channel_drdynvc_open: xrdp_channel_init failed"); - free_stream(s); - return 1; + goto cleanup; + } + + // If there's no 'data_first' proc, we need a dechunker for the + // channel + if (procs->data_first == NULL) + { + if ((dc = dyn_dechunker_init(name)) == NULL) + { + // Error logged + goto cleanup; + } } cmd_ptr = s->p; out_uint8(s, 0); /* set later */ @@ -835,14 +953,12 @@ xrdp_channel_drdynvc_open(struct xrdp_channel *self, const char *name, while (self->drdynvcs[ChId].status != XRDP_DRDYNVC_STATUS_CLOSED) { ChId++; - if (ChId > 255) + if (ChId >= DRDYNVC_CHANNEL_COUNT) { LOG(LOG_LEVEL_ERROR, "Attempting to create a new channel when the maximum " - "number of channels have already been created. " - "XRDP only supports 255 open channels."); - free_stream(s); - return 1; + "number of channels have already been created."); + goto cleanup; } } cbChId = drdynvc_insert_uint_124(s, ChId); /* ChannelId */ @@ -864,17 +980,30 @@ xrdp_channel_drdynvc_open(struct xrdp_channel *self, const char *name, { LOG(LOG_LEVEL_ERROR, "Sending [MS-RDPEDYC] DYNVC_CREATE_REQ failed"); - free_stream(s); - return 1; + goto cleanup; } - free_stream(s); + + if (procs->data == NULL) + { + // Not expected. Log this once. + LOG(LOG_LEVEL_WARNING, "Dynamic Virtual Channel %s (%d): " + "callback 'data' is NULL", name, ChId); + } + *chan_id = ChId; self->drdynvcs[ChId].open_response = procs->open_response; self->drdynvcs[ChId].close_response = procs->close_response; self->drdynvcs[ChId].data_first = procs->data_first; self->drdynvcs[ChId].data = procs->data; self->drdynvcs[ChId].status = XRDP_DRDYNVC_STATUS_OPEN_SENT; + self->drdynvcs[ChId].dc = dc; + free_stream(s); return 0; + +cleanup: + free_stream(s); + dyn_dechunker_free(dc); + return 1; } /*****************************************************************************/ @@ -892,9 +1021,9 @@ xrdp_channel_drdynvc_close(struct xrdp_channel *self, int chan_id) int static_flags; char *cmd_ptr; - if ((chan_id < 0) || (chan_id > 255)) + if ((chan_id < 0) || (chan_id >= DRDYNVC_CHANNEL_COUNT)) { - LOG(LOG_LEVEL_ERROR, "Attempting to close an invalid channel id. " + LOG(LOG_LEVEL_ERROR, "Attempting to close an invalid " "channel id %d", chan_id); return 1; } @@ -962,10 +1091,10 @@ xrdp_channel_drdynvc_data_first(struct xrdp_channel *self, int chan_id, int static_flags; char *cmd_ptr; - if ((chan_id < 0) || (chan_id > 255)) + if ((chan_id < 0) || (chan_id >= DRDYNVC_CHANNEL_COUNT)) { LOG(LOG_LEVEL_ERROR, "Attempting to send data to an invalid " - "channel id. channel id %d", chan_id); + "channel id %d", chan_id); return 1; } if (self->drdynvcs[chan_id].status != XRDP_DRDYNVC_STATUS_OPEN) @@ -1034,10 +1163,10 @@ xrdp_channel_drdynvc_data(struct xrdp_channel *self, int chan_id, int static_flags; char *cmd_ptr; - if ((chan_id < 0) || (chan_id > 255)) + if ((chan_id < 0) || (chan_id >= DRDYNVC_CHANNEL_COUNT)) { LOG(LOG_LEVEL_ERROR, "Attempting to send data to an invalid " - "channel id. channel id %d", chan_id); + "channel id %d", chan_id); return 1; } if (self->drdynvcs[chan_id].status != XRDP_DRDYNVC_STATUS_OPEN) diff --git a/xrdp/xrdp_egfx.c b/xrdp/xrdp_egfx.c index 6cc381ae..84f33790 100644 --- a/xrdp/xrdp_egfx.c +++ b/xrdp/xrdp_egfx.c @@ -942,37 +942,11 @@ xrdp_egfx_close_response(struct xrdp_process *id, int chan_id) return 0; } -/******************************************************************************/ -/* from client */ -static int -xrdp_egfx_data_first(struct xrdp_process *id, int chan_id, - struct stream *s, int total_bytes) -{ - struct xrdp_egfx *egfx; - int bytes = s_rem(s); - - LOG(LOG_LEVEL_TRACE, "xrdp_egfx_data_first: bytes %d" - " total_bytes %d", bytes, total_bytes); - egfx = id->wm->mm->egfx; - if (egfx->s != NULL) - { - LOG(LOG_LEVEL_ERROR, "DYNVC_DATA_FIRST PDU received while" - " another stream is active on channel %d", chan_id); - return 1; - } - make_stream(egfx->s); - // Caller has checked total_bytes is >= 0 and bytes is < total_bytes - init_stream(egfx->s, total_bytes); - out_uint8a(egfx->s, s->p, bytes); - return 0; -} - /******************************************************************************/ /* from client */ static int xrdp_egfx_data(struct xrdp_process *id, int chan_id, struct stream *s) { - int error; struct xrdp_wm *wm; struct xrdp_mm *mm; struct xrdp_egfx *egfx; @@ -1002,29 +976,7 @@ xrdp_egfx_data(struct xrdp_process *id, int chan_id, struct stream *s) return 0; } - if (egfx->s == NULL) - { - return xrdp_egfx_process(egfx, s); - } - int bytes = s_rem(s); - - if (!s_check_rem_out(egfx->s, bytes)) - { - LOG(LOG_LEVEL_ERROR, "DYNVC_DATA PDU data overflow on channel %d", - chan_id); - return 1; - } - out_uint8a(egfx->s, s->p, bytes); - if (!s_check_rem_out(egfx->s, 1)) - { - s_mark_end(egfx->s); - egfx->s->p = egfx->s->data; - error = xrdp_egfx_process(egfx, egfx->s); - free_stream(egfx->s); - egfx->s = NULL; - return error; - } - return 0; + return xrdp_egfx_process(egfx, s); } /******************************************************************************/ @@ -1045,7 +997,7 @@ xrdp_egfx_create(struct xrdp_mm *mm, struct xrdp_egfx **egfx) } procs.open_response = xrdp_egfx_open_response; procs.close_response = xrdp_egfx_close_response; - procs.data_first = xrdp_egfx_data_first; + procs.data_first = NULL; // Defragging handled elsewere procs.data = xrdp_egfx_data; process = mm->wm->pro_layer; error = libxrdp_drdynvc_open(process->session, diff --git a/xrdp/xrdp_egfx.h b/xrdp/xrdp_egfx.h index 18d32c90..455f15d2 100644 --- a/xrdp/xrdp_egfx.h +++ b/xrdp/xrdp_egfx.h @@ -127,7 +127,6 @@ struct xrdp_egfx int channel_id; int surface_id; int frame_id; - struct stream *s; void *user; struct xrdp_egfx_bulk *bulk; int (*caps_advertise)(void *user, int num_caps, int *version, int *flags); diff --git a/xrdp/xrdp_mm.c b/xrdp/xrdp_mm.c index def835df..4b6489db 100644 --- a/xrdp/xrdp_mm.c +++ b/xrdp/xrdp_mm.c @@ -1044,15 +1044,6 @@ dynamic_monitor_close_response(struct xrdp_process *id, int chan_id) return 0; } -/******************************************************************************/ -static int -dynamic_monitor_data_first(struct xrdp_process *id, int chan_id, - struct stream *s, int total_bytes) -{ - LOG_DEVEL(LOG_LEVEL_TRACE, "dynamic_monitor_data_first:"); - return 0; -} - /******************************************************************************/ int advance_resize_state_machine(struct xrdp_mm *mm, @@ -1967,7 +1958,7 @@ dynamic_monitor_initialize(struct xrdp_mm *self) g_memset(&d_procs, 0, sizeof(d_procs)); d_procs.open_response = dynamic_monitor_open_response; d_procs.close_response = dynamic_monitor_close_response; - d_procs.data_first = dynamic_monitor_data_first; + d_procs.data_first = NULL; // Defragging handled elsewhere d_procs.data = dynamic_monitor_data; flags = 0; error = libxrdp_drdynvc_open(self->wm->session,