From 2d5a1c37d75bb1df20f4cee96f556fe5bdd5c337 Mon Sep 17 00:00:00 2001 From: Koichiro IWAO Date: Sat, 29 Oct 2022 01:49:45 +0900 Subject: [PATCH 1/7] gitignore: ignore some test artifacts --- .gitignore | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 8710e975c1..c5026d15c8 100644 --- a/.gitignore +++ b/.gitignore @@ -48,11 +48,14 @@ stamp-h1 tap-driver.sh test-driver tests/common/test_common +tests/libipm/test_libipm +tests/libxrdp/test_libxrdp tests/memtest/memtest +tests/xrdp/test_xrdp tools/devel/tcp_proxy/tcp_proxy *.trs xrdp/xrdp xrdp/xrdp.ini xrdp_configure_options.h xrdpapi/xrdp-xrdpapi-simple -.vscode/* \ No newline at end of file +.vscode/* From ece8fd2946f1183ac163ee51e6e5a6dddebf5782 Mon Sep 17 00:00:00 2001 From: Koichiro IWAO Date: Sat, 29 Oct 2022 01:45:36 +0900 Subject: [PATCH 2/7] Add CODEC_GUID_IGNORE ref. https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpbcgr/86507fed-a0ee-4242-b802-237534a8f65e --- common/ms-rdpbcgr.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/common/ms-rdpbcgr.h b/common/ms-rdpbcgr.h index 6129f98fa3..46e8975c00 100644 --- a/common/ms-rdpbcgr.h +++ b/common/ms-rdpbcgr.h @@ -384,7 +384,7 @@ #define XR_CODEC_GUID_IMAGE_REMOTEFX \ "\xD4\xCC\x44\x27\x8A\x9D\x74\x4E\x80\x3C\x0E\xCB\xEE\xA1\x9C\x54" -/* MFVideoFormat_H264 0x34363248-0000-0010-800000AA00389B71 */ +/* MFVideoFormat_H264 34363248-0000-0010-800000AA00389B71 */ #define XR_CODEC_GUID_H264 \ "\x48\x32\x36\x34\x00\x00\x10\x00\x80\x00\x00\xAA\x00\x38\x9B\x71" @@ -396,6 +396,10 @@ #define XR_CODEC_GUID_PNG \ "\x8D\x85\x0C\x0E\xE0\x28\xDB\x45\xAD\xAA\x0F\x83\xE5\x7C\xC5\x60" +/* CODEC_GUID_IGNORE 0C4351A6-3535-42AE-910CCDFCE5760B58 */ +#define XR_CODEC_GUID_IGNORE \ + "\xA6\x51\x43\x0C\x35\x35\xAE\x42\x91\x0C\xCD\xFC\xE5\x76\x0B\x58" + /* PDU Types (2.2.8.1.1.1.1) */ #define PDUTYPE_DEMANDACTIVEPDU 0x1 #define PDUTYPE_CONFIRMACTIVEPDU 0x3 From 791f055e18ff5cab29b1f27fa1c338ca49bf236a Mon Sep 17 00:00:00 2001 From: Koichiro IWAO Date: Sat, 29 Oct 2022 01:48:03 +0900 Subject: [PATCH 3/7] common: add function to convert from Microsoft's GUID to string --- common/guid.c | 18 ++++++++ common/guid.h | 13 +++++- tests/common/Makefile.am | 3 +- tests/common/test_common.h | 1 + tests/common/test_common_main.c | 1 + tests/common/test_guid.c | 71 ++++++++++++++++++++++++++++++++ tests/common/test_string_calls.c | 1 + 7 files changed, 106 insertions(+), 2 deletions(-) create mode 100644 tests/common/test_guid.c diff --git a/common/guid.c b/common/guid.c index 21611a478c..ae0386d420 100644 --- a/common/guid.c +++ b/common/guid.c @@ -70,3 +70,21 @@ const char *guid_to_str(const struct guid *guid, char *str) g_bytes_to_hexstr(guid->g, GUID_SIZE, str, GUID_STR_SIZE); return str; } + +const char *ms_guid_to_str(const char *src, char *dest) +{ + const unsigned char *guid = (const unsigned char *)src; + + /* + * Flipping integers into little-endian + * See also: https://devblogs.microsoft.com/oldnewthing/20220928-00/?p=107221 + */ + g_sprintf(dest, "%02X%02X%02X%02X-%02X%02X-%02X%02X-%02X%02X-%02X%02X%02X%02X%02X%02X", + guid[3], guid[2], guid[1], guid[0], + guid[5], guid[4], + guid[7], guid[6], + guid[8], guid[9], + guid[10], guid[11], guid[12], guid[13], guid[14], guid[15]); + + return dest; +} diff --git a/common/guid.h b/common/guid.h index 546788c660..060e9a0343 100644 --- a/common/guid.h +++ b/common/guid.h @@ -28,7 +28,8 @@ #include "arch.h" #define GUID_SIZE 16 /* bytes */ -#define GUID_STR_SIZE (GUID_SIZE * 2 + 1) /* Size for string representation */ +#define GUID_STR_SIZE (GUID_SIZE * 2 + 4 + 1) /* w/ 4 hyphens + null terminator */ + /** * Use a struct for the guid so we can easily copy by assignment @@ -72,4 +73,14 @@ guid_is_set(const struct guid *guid); */ const char *guid_to_str(const struct guid *guid, char *str); + +/** + * Converts a Microsoft's COM GUID to a string representation + * + * @param src GUID to represent + * @param dest pointer to at least GUID_STR_SIZE bytes to store the + * representation + */ +const char *ms_guid_to_str(const char *src, char *dest); #endif + diff --git a/tests/common/Makefile.am b/tests/common/Makefile.am index e6686e3124..f7dbbe28f6 100644 --- a/tests/common/Makefile.am +++ b/tests/common/Makefile.am @@ -17,7 +17,8 @@ test_common_SOURCES = \ test_string_calls.c \ test_os_calls.c \ test_ssl_calls.c \ - test_base64.c + test_base64.c \ + test_guid.c test_common_CFLAGS = \ @CHECK_CFLAGS@ \ diff --git a/tests/common/test_common.h b/tests/common/test_common.h index ed422ab419..1878bafb72 100644 --- a/tests/common/test_common.h +++ b/tests/common/test_common.h @@ -11,5 +11,6 @@ Suite *make_suite_test_string(void); Suite *make_suite_test_os_calls(void); Suite *make_suite_test_ssl_calls(void); Suite *make_suite_test_base64(void); +Suite *make_suite_test_guid(void); #endif /* TEST_COMMON_H */ diff --git a/tests/common/test_common_main.c b/tests/common/test_common_main.c index a08bc7f762..37adaa3fea 100644 --- a/tests/common/test_common_main.c +++ b/tests/common/test_common_main.c @@ -49,6 +49,7 @@ int main (void) srunner_add_suite(sr, make_suite_test_os_calls()); srunner_add_suite(sr, make_suite_test_ssl_calls()); srunner_add_suite(sr, make_suite_test_base64()); + srunner_add_suite(sr, make_suite_test_guid()); // srunner_add_suite(sr, make_list_suite()); srunner_set_tap(sr, "-"); diff --git a/tests/common/test_guid.c b/tests/common/test_guid.c new file mode 100644 index 0000000000..c39152deea --- /dev/null +++ b/tests/common/test_guid.c @@ -0,0 +1,71 @@ + +#if defined(HAVE_CONFIG_H) +#include "config_ac.h" +#endif + +#include "guid.h" +#include "string_calls.h" +#include "ms-rdpbcgr.h" + +#include "test_common.h" + +/******************************************************************************/ + +START_TEST(test_ms_guid_to_str_remotefx) +{ + /* setup */ + char dest[GUID_STR_SIZE]; + + /* test */ + ms_guid_to_str(XR_CODEC_GUID_REMOTEFX, dest); + + /* verify */ + ck_assert_str_eq(dest, "76772F12-BD72-4463-AFB3-B73C9C6F7886"); +} +END_TEST + +START_TEST(test_ms_guid_to_str_nscodec) +{ + + /* setup */ + char dest[GUID_STR_SIZE]; + + /* test */ + ms_guid_to_str(XR_CODEC_GUID_NSCODEC, dest); + + /* verify */ + ck_assert_str_eq(dest, "CA8D1BB9-000F-154F-589F-AE2D1A87E2D6"); +} +END_TEST + +START_TEST(test_ms_guid_to_str_ignore) +{ + /* setup */ + char dest[GUID_STR_SIZE]; + + /* test */ + ms_guid_to_str(XR_CODEC_GUID_IGNORE, dest); + + /* verify */ + ck_assert_str_eq(dest, "0C4351A6-3535-42AE-910C-CDFCE5760B58"); +} +END_TEST + +/******************************************************************************/ + +Suite * +make_suite_test_guid(void) +{ + Suite *s; + TCase *tc_guid; + + s = suite_create("GUID"); + + tc_guid = tcase_create("guid_to_str"); + suite_add_tcase(s, tc_guid); + tcase_add_test(tc_guid, test_ms_guid_to_str_remotefx); + tcase_add_test(tc_guid, test_ms_guid_to_str_nscodec); + tcase_add_test(tc_guid, test_ms_guid_to_str_ignore); + + return s; +} diff --git a/tests/common/test_string_calls.c b/tests/common/test_string_calls.c index 6dc14a7349..0f7d626859 100644 --- a/tests/common/test_string_calls.c +++ b/tests/common/test_string_calls.c @@ -4,6 +4,7 @@ #endif #include "string_calls.h" +#include "ms-rdpbcgr.h" #include "test_common.h" From fe14cb14a3d0afda1d9e24a4357b73b74068a093 Mon Sep 17 00:00:00 2001 From: Koichiro IWAO Date: Sat, 29 Oct 2022 01:59:40 +0900 Subject: [PATCH 4/7] libxrdp: record codec GUID to identify unknown codec --- libxrdp/xrdp_caps.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/libxrdp/xrdp_caps.c b/libxrdp/xrdp_caps.c index c953205342..035f45761c 100644 --- a/libxrdp/xrdp_caps.c +++ b/libxrdp/xrdp_caps.c @@ -25,6 +25,7 @@ #include +#include "guid.h" #include "libxrdp.h" #include "ms-rdpbcgr.h" #include "ms-rdperp.h" @@ -521,6 +522,7 @@ xrdp_caps_process_codecs(struct xrdp_rdp *self, struct stream *s, int len) int i1; char *codec_guid; char *next_guid; + char codec_guid_str[GUID_STR_SIZE]; if (len < 1) { @@ -533,6 +535,8 @@ xrdp_caps_process_codecs(struct xrdp_rdp *self, struct stream *s, int len) for (index = 0; index < codec_count; index++) { codec_guid = s->p; + ms_guid_to_str(codec_guid, codec_guid_str); + if (len < 16 + 1 + 2) { LOG(LOG_LEVEL_ERROR, "xrdp_caps_process_codecs: error"); @@ -552,8 +556,8 @@ xrdp_caps_process_codecs(struct xrdp_rdp *self, struct stream *s, int len) if (g_memcmp(codec_guid, XR_CODEC_GUID_NSCODEC, 16) == 0) { - LOG(LOG_LEVEL_INFO, "xrdp_caps_process_codecs: nscodec, codec id %d, properties len %d", - codec_id, codec_properties_length); + LOG(LOG_LEVEL_INFO, "xrdp_caps_process_codecs: NSCodec(%s), codec id [%d], properties len [%d]", + codec_guid_str, codec_id, codec_properties_length); self->client_info.ns_codec_id = codec_id; i1 = MIN(64, codec_properties_length); g_memcpy(self->client_info.ns_prop, s->p, i1); @@ -561,8 +565,8 @@ xrdp_caps_process_codecs(struct xrdp_rdp *self, struct stream *s, int len) } else if (g_memcmp(codec_guid, XR_CODEC_GUID_REMOTEFX, 16) == 0) { - LOG(LOG_LEVEL_INFO, "xrdp_caps_process_codecs: RemoteFX, codec id %d, properties len %d", - codec_id, codec_properties_length); + LOG(LOG_LEVEL_INFO, "xrdp_caps_process_codecs: RemoteFX(%s), codec id [%d], properties len [%d]", + codec_guid_str, codec_id, codec_properties_length); self->client_info.rfx_codec_id = codec_id; i1 = MIN(64, codec_properties_length); g_memcpy(self->client_info.rfx_prop, s->p, i1); @@ -570,8 +574,8 @@ xrdp_caps_process_codecs(struct xrdp_rdp *self, struct stream *s, int len) } else if (g_memcmp(codec_guid, XR_CODEC_GUID_JPEG, 16) == 0) { - LOG(LOG_LEVEL_INFO, "xrdp_caps_process_codecs: jpeg, codec id %d, properties len %d", - codec_id, codec_properties_length); + LOG(LOG_LEVEL_INFO, "xrdp_caps_process_codecs: JPEG(%s), codec id [%d], properties len [%d]", + codec_guid_str, codec_id, codec_properties_length); self->client_info.jpeg_codec_id = codec_id; i1 = MIN(64, codec_properties_length); g_memcpy(self->client_info.jpeg_prop, s->p, i1); @@ -587,8 +591,8 @@ xrdp_caps_process_codecs(struct xrdp_rdp *self, struct stream *s, int len) } else if (g_memcmp(codec_guid, XR_CODEC_GUID_H264, 16) == 0) { - LOG(LOG_LEVEL_INFO, "xrdp_caps_process_codecs: h264, codec id %d, properties len %d", - codec_id, codec_properties_length); + LOG(LOG_LEVEL_INFO, "xrdp_caps_process_codecs: H264(%s), codec id [%d], properties len [%d]", + codec_guid_str, codec_id, codec_properties_length); self->client_info.h264_codec_id = codec_id; i1 = MIN(64, codec_properties_length); g_memcpy(self->client_info.h264_prop, s->p, i1); @@ -596,7 +600,8 @@ xrdp_caps_process_codecs(struct xrdp_rdp *self, struct stream *s, int len) } else { - LOG(LOG_LEVEL_WARNING, "xrdp_caps_process_codecs: unknown codec id %d", codec_id); + LOG(LOG_LEVEL_WARNING, "xrdp_caps_process_codecs: unknown(%s), codec id [%d], properties len [%d]", + codec_guid_str, codec_id, codec_properties_length); } s->p = next_guid; From 44c977a7c2b590d0301a2a80caaf387e884704c8 Mon Sep 17 00:00:00 2001 From: Koichiro IWAO Date: Tue, 1 Nov 2022 22:57:07 +0900 Subject: [PATCH 5/7] Use 8-4-4-4-12 rather than 8-4-4-16 for GUID textual representation --- common/ms-rdpbcgr.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/common/ms-rdpbcgr.h b/common/ms-rdpbcgr.h index 46e8975c00..4e5a5e4a2f 100644 --- a/common/ms-rdpbcgr.h +++ b/common/ms-rdpbcgr.h @@ -372,11 +372,11 @@ /* Bitmap Codec: codecGUID (2.2.7.2.10.1.1) */ -/* CODEC_GUID_NSCODEC CA8D1BB9-000F-154F-589FAE2D1A87E2D6 */ +/* CODEC_GUID_NSCODEC CA8D1BB9-000F-154F-589F-AE2D1A87E2D6 */ #define XR_CODEC_GUID_NSCODEC \ "\xb9\x1b\x8d\xca\x0f\x00\x4f\x15\x58\x9f\xae\x2d\x1a\x87\xe2\xd6" -/* CODEC_GUID_REMOTEFX 76772F12-BD72-4463-AFB3B73C9C6F7886 */ +/* CODEC_GUID_REMOTEFX 76772F12-BD72-4463-AFB3-B73C9C6F7886 */ #define XR_CODEC_GUID_REMOTEFX \ "\x12\x2F\x77\x76\x72\xBD\x63\x44\xAF\xB3\xB7\x3C\x9C\x6F\x78\x86" @@ -384,19 +384,19 @@ #define XR_CODEC_GUID_IMAGE_REMOTEFX \ "\xD4\xCC\x44\x27\x8A\x9D\x74\x4E\x80\x3C\x0E\xCB\xEE\xA1\x9C\x54" -/* MFVideoFormat_H264 34363248-0000-0010-800000AA00389B71 */ +/* MFVideoFormat_H264 34363248-0000-0010-8000-00AA00389B71 */ #define XR_CODEC_GUID_H264 \ "\x48\x32\x36\x34\x00\x00\x10\x00\x80\x00\x00\xAA\x00\x38\x9B\x71" -/* CODEC_GUID_JPEG 1BAF4CE6-9EED-430C-869ACB8B37B66237 */ +/* CODEC_GUID_JPEG 1BAF4CE6-9EED-430C-869A-CB8B37B66237 */ #define XR_CODEC_GUID_JPEG \ "\xE6\x4C\xAF\x1B\xED\x9E\x0C\x43\x86\x9A\xCB\x8B\x37\xB6\x62\x37" -/* CODEC_GUID_PNG 0E0C858D-28E0-45DB-ADAA0F83E57CC560 */ +/* CODEC_GUID_PNG 0E0C858D-28E0-45DB-ADAA-0F83E57CC560 */ #define XR_CODEC_GUID_PNG \ "\x8D\x85\x0C\x0E\xE0\x28\xDB\x45\xAD\xAA\x0F\x83\xE5\x7C\xC5\x60" -/* CODEC_GUID_IGNORE 0C4351A6-3535-42AE-910CCDFCE5760B58 */ +/* CODEC_GUID_IGNORE 0C4351A6-3535-42AE-910C-CDFCE5760B58 */ #define XR_CODEC_GUID_IGNORE \ "\xA6\x51\x43\x0C\x35\x35\xAE\x42\x91\x0C\xCD\xFC\xE5\x76\x0B\x58" From 32da5a7ed638afbe81c775ed0900932a905712b0 Mon Sep 17 00:00:00 2001 From: Koichiro IWAO Date: Tue, 1 Nov 2022 23:40:09 +0900 Subject: [PATCH 6/7] Replace guid_to_str() with ms_guid_to_str() --- common/guid.c | 10 ++-------- common/guid.h | 11 +---------- libxrdp/xrdp_caps.c | 7 ++++++- tests/common/test_guid.c | 26 ++++++++++++++++---------- 4 files changed, 25 insertions(+), 29 deletions(-) diff --git a/common/guid.c b/common/guid.c index ae0386d420..cf863648a5 100644 --- a/common/guid.c +++ b/common/guid.c @@ -65,15 +65,9 @@ guid_is_set(const struct guid *guid) } -const char *guid_to_str(const struct guid *guid, char *str) +const char *guid_to_str(const struct guid *src, char *dest) { - g_bytes_to_hexstr(guid->g, GUID_SIZE, str, GUID_STR_SIZE); - return str; -} - -const char *ms_guid_to_str(const char *src, char *dest) -{ - const unsigned char *guid = (const unsigned char *)src; + const unsigned char *guid = (const unsigned char *)src->g; /* * Flipping integers into little-endian diff --git a/common/guid.h b/common/guid.h index 060e9a0343..c68823a28e 100644 --- a/common/guid.h +++ b/common/guid.h @@ -71,16 +71,7 @@ guid_is_set(const struct guid *guid); * representation * @return str is returned for convenience */ -const char *guid_to_str(const struct guid *guid, char *str); +const char *guid_to_str(const struct guid *guid, char *dest); - -/** - * Converts a Microsoft's COM GUID to a string representation - * - * @param src GUID to represent - * @param dest pointer to at least GUID_STR_SIZE bytes to store the - * representation - */ -const char *ms_guid_to_str(const char *src, char *dest); #endif diff --git a/libxrdp/xrdp_caps.c b/libxrdp/xrdp_caps.c index 035f45761c..2f3d8e756b 100644 --- a/libxrdp/xrdp_caps.c +++ b/libxrdp/xrdp_caps.c @@ -522,8 +522,11 @@ xrdp_caps_process_codecs(struct xrdp_rdp *self, struct stream *s, int len) int i1; char *codec_guid; char *next_guid; + struct guid guid; char codec_guid_str[GUID_STR_SIZE]; + guid_clear(&guid); + if (len < 1) { LOG(LOG_LEVEL_ERROR, "xrdp_caps_process_codecs: error"); @@ -535,7 +538,9 @@ xrdp_caps_process_codecs(struct xrdp_rdp *self, struct stream *s, int len) for (index = 0; index < codec_count; index++) { codec_guid = s->p; - ms_guid_to_str(codec_guid, codec_guid_str); + + g_memcpy(guid.g, s->p, GUID_SIZE); + guid_to_str(&guid, codec_guid_str); if (len < 16 + 1 + 2) { diff --git a/tests/common/test_guid.c b/tests/common/test_guid.c index c39152deea..7580bb9543 100644 --- a/tests/common/test_guid.c +++ b/tests/common/test_guid.c @@ -4,47 +4,53 @@ #endif #include "guid.h" -#include "string_calls.h" #include "ms-rdpbcgr.h" +#include "os_calls.h" #include "test_common.h" /******************************************************************************/ -START_TEST(test_ms_guid_to_str_remotefx) +START_TEST(test_guid_to_str_remotefx) { /* setup */ char dest[GUID_STR_SIZE]; + struct guid guid; + g_memcpy(guid.g, XR_CODEC_GUID_REMOTEFX, GUID_SIZE); /* test */ - ms_guid_to_str(XR_CODEC_GUID_REMOTEFX, dest); + guid_to_str(&guid, dest); /* verify */ ck_assert_str_eq(dest, "76772F12-BD72-4463-AFB3-B73C9C6F7886"); } END_TEST -START_TEST(test_ms_guid_to_str_nscodec) +START_TEST(test_guid_to_str_nscodec) { /* setup */ char dest[GUID_STR_SIZE]; + struct guid guid; + g_memcpy(guid.g, XR_CODEC_GUID_NSCODEC, GUID_SIZE); /* test */ - ms_guid_to_str(XR_CODEC_GUID_NSCODEC, dest); + guid_to_str(&guid, dest); /* verify */ ck_assert_str_eq(dest, "CA8D1BB9-000F-154F-589F-AE2D1A87E2D6"); } END_TEST -START_TEST(test_ms_guid_to_str_ignore) +START_TEST(test_guid_to_str_ignore) { /* setup */ char dest[GUID_STR_SIZE]; + struct guid guid; + g_memcpy(guid.g, XR_CODEC_GUID_IGNORE, GUID_SIZE); /* test */ - ms_guid_to_str(XR_CODEC_GUID_IGNORE, dest); + guid_to_str(&guid, dest); /* verify */ ck_assert_str_eq(dest, "0C4351A6-3535-42AE-910C-CDFCE5760B58"); @@ -63,9 +69,9 @@ make_suite_test_guid(void) tc_guid = tcase_create("guid_to_str"); suite_add_tcase(s, tc_guid); - tcase_add_test(tc_guid, test_ms_guid_to_str_remotefx); - tcase_add_test(tc_guid, test_ms_guid_to_str_nscodec); - tcase_add_test(tc_guid, test_ms_guid_to_str_ignore); + tcase_add_test(tc_guid, test_guid_to_str_remotefx); + tcase_add_test(tc_guid, test_guid_to_str_nscodec); + tcase_add_test(tc_guid, test_guid_to_str_ignore); return s; } From 9120dc9a66331fc6e0c2123797d2cd06b208b0a6 Mon Sep 17 00:00:00 2001 From: Koichiro IWAO Date: Fri, 4 Nov 2022 16:06:50 +0900 Subject: [PATCH 7/7] Update header comments --- common/guid.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/common/guid.h b/common/guid.h index c68823a28e..2951fa12d5 100644 --- a/common/guid.h +++ b/common/guid.h @@ -67,9 +67,9 @@ guid_is_set(const struct guid *guid); * Converts a GUID to a string representation * * @param guid GUID to represent - * @param str pointer to at least GUID_STR_SIZE bytes to store the - * representation - * @return str is returned for convenience + * @param dest destionation pointer to at least GUID_STR_SIZE + * bytes to store the representation + * @return dest is returned for convenience */ const char *guid_to_str(const struct guid *guid, char *dest);