mirrors/xrdp
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Remove xrdp_sec_in_mcs_data() function

Browse Source 
THe function xrdp_sec_in_mcs_data() parses data within the
TS_UD_CS_CORE struct which could just as easily be parsed
when xrdp_sec_process_mcs_data_CS_CORE() is called.

Currently the contents of the MSC Connect Initial PDU are stored within
the client_mcs_data member of the xrdp_sec struct. This stream is parsed
once by xrdp_sec_process_mcs_data() and then separately by
xrdp_sec_in_mcs_data(). There is no reason not to perform the parse in
a single pass through the stream.

This commit folds the functionality in xrdp_sec_in_mcs_data() into
xrdp_sec_process_mcs_data_CS_CORE() and removes xrdp_sec_in_mcs_data()
This commit is contained in:
matt335672 2024-09-30 14:50:00 +01:00
parent d4c30d5fc9
commit 10cf8f2d1c
1 changed files with 19 additions and 100 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

119
libxrdp/xrdp_sec.c
View File

@ -1488,7 +1488,6 @@ xrdp_sec_process_mcs_data_CS_CORE(struct xrdp_sec *self, struct stream *s)
int highColorDepth; int highColorDepth;
int supportedColorDepths; int supportedColorDepths;
int earlyCapabilityFlags; int earlyCapabilityFlags;
char clientName[INFO_CLIENT_NAME_BYTES / 2] = { '\0' };
UNUSED_VAR(version); UNUSED_VAR(version);
struct xrdp_client_info *client_info = &self->rdp_layer->client_info; struct xrdp_client_info *client_info = &self->rdp_layer->client_info;
@ -1516,8 +1515,8 @@ xrdp_sec_process_mcs_data_CS_CORE(struct xrdp_sec *self, struct stream *s)
break; break;
} }
in_uint8s(s, 2); /* SASSequence */ in_uint8s(s, 2); /* SASSequence */
in_uint8s(s, 4); /* keyboardLayout */ in_uint32_le(s, client_info->keylayout);
in_uint8s(s, 4); /* clientBuild */ in_uint32_le(s, client_info->build);
/* clientName /* clientName
* *
@ -1525,26 +1524,33 @@ xrdp_sec_process_mcs_data_CS_CORE(struct xrdp_sec *self, struct stream *s)
* isn't by ignoring the last two bytes and treating them as a * isn't by ignoring the last two bytes and treating them as a
* terminator anyway */ * terminator anyway */
in_utf16_le_fixed_as_utf8(s, (INFO_CLIENT_NAME_BYTES - 2) / 2, in_utf16_le_fixed_as_utf8(s, (INFO_CLIENT_NAME_BYTES - 2) / 2,
clientName, sizeof(clientName)); client_info->hostname,
sizeof(client_info->hostname));
in_uint8s(s, 2); /* See above */ in_uint8s(s, 2); /* See above */
LOG(LOG_LEVEL_INFO, "Connected client computer name: %s", clientName); LOG(LOG_LEVEL_INFO, "Connected client computer name: %s",
in_uint8s(s, 4); /* keyboardType */ client_info->hostname);
in_uint8s(s, 4); /* keyboardSubType */ in_uint32_le(s, client_info->keyboard_type); /* [MS-RDPBCGR] TS_UD_CS_CORE keyboardType */
in_uint32_le(s, client_info->keyboard_subtype); /* [MS-RDPBCGR] TS_UD_CS_CORE keyboardSubType */
in_uint8s(s, 4); /* keyboardFunctionKey */ in_uint8s(s, 4); /* keyboardFunctionKey */
in_uint8s(s, 64); /* imeFileName */ in_uint8s(s, 64); /* imeFileName */
LOG_DEVEL(LOG_LEVEL_TRACE, "Received [MS-RDPBCGR] TS_UD_CS_CORE " LOG_DEVEL(LOG_LEVEL_TRACE, "Received [MS-RDPBCGR] TS_UD_CS_CORE "
"<Required fields> version %08x, desktopWidth %d, " "<Required fields> version %08x, desktopWidth %d, "
"desktopHeight %d, colorDepth %s, SASSequence (ignored), " "desktopHeight %d, colorDepth %s, SASSequence (ignored), "
"keyboardLayout (ignored), clientBuild (ignored), " "keyboardLayout 0x%8.8x, clientBuild %d, "
"clientName %s, keyboardType (ignored), " "clientName %s, keyboardType 0x%8.8x, "
"keyboardSubType (ignored), keyboardFunctionKey (ignored), " "keyboardSubType 0x%8.8x, keyboardFunctionKey (ignored), "
"imeFileName (ignored)", "imeFileName (ignored)",
version, version,
client_info->display_sizes.session_width, client_info->display_sizes.session_width,
client_info->display_sizes.session_height, client_info->display_sizes.session_height,
(colorDepth == 0xca00 ? "RNS_UD_COLOR_4BPP" : (colorDepth == RNS_UD_COLOR_4BPP ? "RNS_UD_COLOR_4BPP" :
colorDepth == 0xca01 ? "RNS_UD_COLOR_8BPP" : "unknown"), colorDepth == RNS_UD_COLOR_8BPP ? "RNS_UD_COLOR_8BPP" :
clientName); "unknown"),
client_info->keylayout,
client_info->build,
client_info->hostname,
client_info->keyboard_type,
client_info->keyboard_subtype);
/* TS_UD_CS_CORE optional fields */ /* TS_UD_CS_CORE optional fields */
if (!s_check_rem(s, 2)) if (!s_check_rem(s, 2))
@ -2180,87 +2186,6 @@ xrdp_sec_process_mcs_data(struct xrdp_sec *self)
return 0; return 0;
} }
/*****************************************************************************/
/* Process the mcs client data [ITU T.124] ConferenceCreateRequest userData field
as a [MS-RDPBCGR] TS_UD_CS_CORE struct */
/* TODO: why does this method parse the strust from back to front (resetting
after each field) instead of from front to back like the rest of the parsing
code? */
/* TODO: why does this method exist when the same struct is parsed in
xrdp_sec_process_mcs_data_CS_CORE and there does not seem to be any
dependencies preventing a call to that function. */
/* TODO: this is a brittle function that assumes field offsets in the stream
instead of parsing the variable length fields of [ITU T.124] ConferenceCreateRequest */
static int
xrdp_sec_in_mcs_data(struct xrdp_sec *self)
{
struct stream *s = (struct stream *)NULL;
struct xrdp_client_info *client_info = (struct xrdp_client_info *)NULL;
client_info = &(self->rdp_layer->client_info);
s = &(self->client_mcs_data);
/* get hostname, it's unicode */
s->p = s->data;
if (!s_check_rem_and_log(s, 47, "Parsing [ITU T.124] ConferenceCreateRequest"))
{
return 1;
}
in_uint8s(s, 47); /* skip [ITU T.124] ConferenceCreateRequest up to the
userData field, and skip [MS-RDPBCGR] TS_UD_CS_CORE
up to the clientName field */
if (!s_check_rem_and_log(s, INFO_CLIENT_NAME_BYTES,
"Parsing [MS-RDPBCGR] TS_UD_CS_CORE clientName"))
{
return 1;
}
in_utf16_le_fixed_as_utf8(s, (INFO_CLIENT_NAME_BYTES - 2) / 2,
client_info->hostname,
sizeof(client_info->hostname));
in_uint8s(s, 2); /* Ignored - terminator for full-size clientName */
/* get build */
s->p = s->data;
if (!s_check_rem_and_log(s, 43 + 4, "Parsing [MS-RDPBCGR] TS_UD_CS_CORE clientBuild"))
{
return 1;
}
in_uint8s(s, 43);
in_uint32_le(s, client_info->build); /* [MS-RDPBCGR] TS_UD_CS_CORE clientBuild */
/* get keylayout */
s->p = s->data;
if (!s_check_rem_and_log(s, 39 + 4, "Parsing [MS-RDPBCGR] TS_UD_CS_CORE keyboardLayout"))
{
return 1;
}
in_uint8s(s, 39);
in_uint32_le(s, client_info->keylayout); /* [MS-RDPBCGR] TS_UD_CS_CORE keyboardLayout */
/* get keyboard type / subtype */
s->p = s->data;
if (!s_check_rem_and_log(s, 79 + 8, "Parsing [MS-RDPBCGR] TS_UD_CS_CORE keyboardType"))
{
return 1;
}
in_uint8s(s, 79);
in_uint32_le(s, client_info->keyboard_type); /* [MS-RDPBCGR] TS_UD_CS_CORE keyboardType */
in_uint32_le(s, client_info->keyboard_subtype); /* [MS-RDPBCGR] TS_UD_CS_CORE keyboardSubType */
LOG_DEVEL(LOG_LEVEL_TRACE, "Received [MS-RDPBCGR] TS_UD_CS_CORE "
"<Required fields> version (ignored), desktopWidth (ignored), "
"desktopHeight (ignored), colorDepth (ignored), SASSequence (ignored), "
"keyboardLayout 0x%8.8x, clientBuild %d, "
"clientName %s, keyboardType 0x%8.8x, "
"keyboardSubType 0x%8.8x, keyboardFunctionKey (ignored), "
"imeFileName (ignored)",
client_info->keylayout,
client_info->build,
client_info->hostname,
client_info->keyboard_type,
client_info->keyboard_subtype);
s->p = s->data;
return 0;
}
/*****************************************************************************/ /*****************************************************************************/
/* returns error */ /* returns error */
static int static int
@ -2445,12 +2370,6 @@ xrdp_sec_incoming(struct xrdp_sec *self)
self->server_mcs_data.data, self->server_mcs_data.data,
(int)(self->server_mcs_data.end - self->server_mcs_data.data)); (int)(self->server_mcs_data.end - self->server_mcs_data.data));
if (xrdp_sec_in_mcs_data(self) != 0)
{
LOG(LOG_LEVEL_ERROR, "xrdp_sec_incoming: xrdp_sec_in_mcs_data failed");
return 1;
}
return 0; return 0;
} }