From d853228c1927d48fb1ec3181b50a6a481448a049 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Tue, 18 Jan 2022 16:30:52 +0000 Subject: [PATCH 1/8] const fixes for SSL calls --- common/ssl_calls.c | 4 ++-- common/ssl_calls.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/common/ssl_calls.c b/common/ssl_calls.c index b305815f..30640c51 100644 --- a/common/ssl_calls.c +++ b/common/ssl_calls.c @@ -140,7 +140,7 @@ ssl_rc4_info_delete(void *rc4_info) /*****************************************************************************/ void -ssl_rc4_set_key(void *rc4_info, char *key, int len) +ssl_rc4_set_key(void *rc4_info, const char *key, int len) { RC4_set_key((RC4_KEY *)rc4_info, len, (tui8 *)key); } @@ -214,7 +214,7 @@ ssl_md5_clear(void *md5_info) /*****************************************************************************/ void -ssl_md5_transform(void *md5_info, char *data, int len) +ssl_md5_transform(void *md5_info, const char *data, int len) { MD5_Update((MD5_CTX *)md5_info, data, len); } diff --git a/common/ssl_calls.h b/common/ssl_calls.h index 121cb390..78edd946 100644 --- a/common/ssl_calls.h +++ b/common/ssl_calls.h @@ -31,7 +31,7 @@ ssl_rc4_info_create(void); void ssl_rc4_info_delete(void *rc4_info); void -ssl_rc4_set_key(void *rc4_info, char *key, int len); +ssl_rc4_set_key(void *rc4_info, const char *key, int len); void ssl_rc4_crypt(void *rc4_info, char *data, int len); void * @@ -51,7 +51,7 @@ ssl_md5_info_delete(void *md5_info); void ssl_md5_clear(void *md5_info); void -ssl_md5_transform(void *md5_info, char *data, int len); +ssl_md5_transform(void *md5_info, const char *data, int len); void ssl_md5_complete(void *md5_info, char *data); void * From d02059d967f0bc1deaf915d3971d4aa8c41dbb4e Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Thu, 20 Jan 2022 16:43:00 +0000 Subject: [PATCH 2/8] Add missing ssl_sha1_clear()/ssl_md5_clear() calls --- sesman/env.c | 1 + vnc/vnc.c | 1 + xrdp/xrdp_bitmap.c | 1 + 3 files changed, 3 insertions(+) diff --git a/sesman/env.c b/sesman/env.c index cdac0f17..b8b2c030 100644 --- a/sesman/env.c +++ b/sesman/env.c @@ -55,6 +55,7 @@ env_check_password_file(const char *filename, const char *passwd) /* create password hash from password */ passwd_bytes = g_strlen(passwd); sha1 = ssl_sha1_info_create(); + ssl_sha1_clear(sha1); ssl_sha1_transform(sha1, "xrdp_vnc", 8); ssl_sha1_transform(sha1, passwd, passwd_bytes); ssl_sha1_transform(sha1, passwd, passwd_bytes); diff --git a/vnc/vnc.c b/vnc/vnc.c index 81e5ecd0..74861329 100644 --- a/vnc/vnc.c +++ b/vnc/vnc.c @@ -94,6 +94,7 @@ rfbHashEncryptBytes(char *bytes, const char *passwd) /* create password hash from password */ passwd_bytes = g_strlen(passwd); sha1 = ssl_sha1_info_create(); + ssl_sha1_clear(sha1); ssl_sha1_transform(sha1, "xrdp_vnc", 8); ssl_sha1_transform(sha1, passwd, passwd_bytes); ssl_sha1_transform(sha1, passwd, passwd_bytes); diff --git a/xrdp/xrdp_bitmap.c b/xrdp/xrdp_bitmap.c index efab0621..72aafff2 100644 --- a/xrdp/xrdp_bitmap.c +++ b/xrdp/xrdp_bitmap.c @@ -174,6 +174,7 @@ xrdp_bitmap_hash_crc(struct xrdp_bitmap *self) return 1; } hash = ssl_md5_info_create(); + ssl_md5_clear(hash); ssl_md5_transform(hash, self->data, bytes); ssl_md5_complete(hash, hash_data); ssl_md5_info_delete(hash); From fde161bac3eebc73b2e7c7bba4ab9381ef5327b9 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Thu, 20 Jan 2022 16:44:49 +0000 Subject: [PATCH 3/8] Add unit tests for SSL calls --- tests/common/Makefile.am | 3 +- tests/common/test_common.h | 1 + tests/common/test_common_main.c | 18 +- tests/common/test_ssl_calls.c | 417 ++++++++++++++++++++++++++++++++ 4 files changed, 437 insertions(+), 2 deletions(-) create mode 100644 tests/common/test_ssl_calls.c diff --git a/tests/common/Makefile.am b/tests/common/Makefile.am index 5a5ca8d5..88548b71 100644 --- a/tests/common/Makefile.am +++ b/tests/common/Makefile.am @@ -13,7 +13,8 @@ test_common_SOURCES = \ test_common.h \ test_common_main.c \ test_string_calls.c \ - test_os_calls.c + test_os_calls.c \ + test_ssl_calls.c test_common_CFLAGS = \ @CHECK_CFLAGS@ \ diff --git a/tests/common/test_common.h b/tests/common/test_common.h index b9db8373..3493c3bf 100644 --- a/tests/common/test_common.h +++ b/tests/common/test_common.h @@ -6,5 +6,6 @@ Suite *make_suite_test_string(void); Suite *make_suite_test_os_calls(void); +Suite *make_suite_test_ssl_calls(void); #endif /* TEST_COMMON_H */ diff --git a/tests/common/test_common_main.c b/tests/common/test_common_main.c index 694a827a..2674bea0 100644 --- a/tests/common/test_common_main.c +++ b/tests/common/test_common_main.c @@ -4,7 +4,10 @@ #endif #include -#include + +#include "log.h" +#include "ssl_calls.h" + #include "test_common.h" int main (void) @@ -14,11 +17,24 @@ int main (void) sr = srunner_create (make_suite_test_string()); srunner_add_suite(sr, make_suite_test_os_calls()); + srunner_add_suite(sr, make_suite_test_ssl_calls()); // srunner_add_suite(sr, make_list_suite()); srunner_set_tap(sr, "-"); + /* + * Set up console logging */ + struct log_config *lc = log_config_init_for_console(LOG_LEVEL_INFO, NULL); + log_start_from_param(lc); + log_config_free(lc); + + /* Initialise the ssl module */ + ssl_init(); + srunner_run_all (sr, CK_ENV); number_failed = srunner_ntests_failed(sr); srunner_free(sr); + + ssl_finish(); + log_end(); return (number_failed == 0) ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/common/test_ssl_calls.c b/tests/common/test_ssl_calls.c new file mode 100644 index 00000000..c5198fd9 --- /dev/null +++ b/tests/common/test_ssl_calls.c @@ -0,0 +1,417 @@ + +#if defined(HAVE_CONFIG_H) +#include "config_ac.h" +#endif + +#include "os_calls.h" +#include "string_calls.h" +#include "ssl_calls.h" + +#include "test_common.h" + +static +char *bin_to_hex(const char *input, int length) +{ + int i; + char *result = (char *)g_malloc(length * 2 + 1, 0); + if (result != NULL) + { + char *p = result; + const char *hexdigit = "0123456789abcdef"; + for (i = 0 ; i < length ; ++i) + { + int c = input[i]; + if (c < 0) + { + c += 256; + } + *p++ = hexdigit[ c / 16]; + *p++ = hexdigit[ c % 16]; + } + *p = '\0'; + } + + return result; +} + + +START_TEST(test_rc4_enc_ok) +{ + const char *key = "16_byte_key-----"; + char text[] = "xrdp-test-suite-rc4-encryption"; + char *result; + + void *info = ssl_rc4_info_create(); + ssl_rc4_set_key(info, key, g_strlen(key)); + ssl_rc4_crypt(info, text, sizeof(text) - 1); + ssl_rc4_info_delete(info); + result = bin_to_hex(text, sizeof(text) - 1); + ck_assert(result != NULL); + /* Result should be the same as + * echo -n '' | \ + * openssl rc4 -K -e [-provider legacy] | \ + * xxd -g0 + * + * where is the string above in hexadecimal */ + ck_assert_str_eq(result, "c080f175b2d85802dbf1042f07180ddc4be1d9bd4a44158f0aebf11c961b"); + g_free(result); +} +END_TEST + +START_TEST(test_rc4_enc_tv0_ok) +{ + /* + * This is one of the 5 original RC4 test vectors posted in response to + * the 'RC4 Algorithm revealed' sci.crypt usenet posting */ + unsigned char key[] = {0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef}; + unsigned char text[] = {0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef}; + const char *expected = "75b7878099e0c596"; + char *result; + + void *info = ssl_rc4_info_create(); + ssl_rc4_set_key(info, (char *)key, sizeof(key)); + ssl_rc4_crypt(info, (char *)text, sizeof(text)); + ssl_rc4_info_delete(info); + + result = bin_to_hex((char *)text, sizeof(text)); + ck_assert(result != NULL); + ck_assert_str_eq(result, expected); + g_free(result); +} +END_TEST + +START_TEST(test_rc4_enc_tv1_ok) +{ + /* + * This is one of the 5 original RC4 test vectors posted in response to + * the 'RC4 Algorithm revealed' sci.crypt usenet posting */ + unsigned char key[] = {0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef}; + unsigned char text[] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; + const char *expected = "7494c2e7104b0879"; + char *result; + + void *info = ssl_rc4_info_create(); + ssl_rc4_set_key(info, (char *)key, sizeof(key)); + ssl_rc4_crypt(info, (char *)text, sizeof(text)); + ssl_rc4_info_delete(info); + + result = bin_to_hex((char *)text, sizeof(text)); + ck_assert(result != NULL); + ck_assert_str_eq(result, expected); + g_free(result); +} +END_TEST + +START_TEST(test_rc4_enc_tv2_ok) +{ + /* + * This is one of the 5 original RC4 test vectors posted in response to + * the 'RC4 Algorithm revealed' sci.crypt usenet posting */ + unsigned char key[] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; + unsigned char text[] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; + const char *expected = "de188941a3375d3a"; + char *result; + + void *info = ssl_rc4_info_create(); + ssl_rc4_set_key(info, (char *)key, sizeof(key)); + ssl_rc4_crypt(info, (char *)text, sizeof(text)); + ssl_rc4_info_delete(info); + + result = bin_to_hex((char *)text, sizeof(text)); + ck_assert(result != NULL); + ck_assert_str_eq(result, expected); + g_free(result); +} +END_TEST + +START_TEST(test_rc4_enc_tv3_ok) +{ + /* + * This is one of the 5 original RC4 test vectors posted in response to + * the 'RC4 Algorithm revealed' sci.crypt usenet posting */ + unsigned char key[] = {0xef, 0x01, 0x23, 0x45}; + unsigned char text[] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; + const char *expected = "d6a141a7ec3c38dfbd61"; + char *result; + + void *info = ssl_rc4_info_create(); + ssl_rc4_set_key(info, (char *)key, sizeof(key)); + ssl_rc4_crypt(info, (char *)text, sizeof(text)); + ssl_rc4_info_delete(info); + + result = bin_to_hex((char *)text, sizeof(text)); + ck_assert(result != NULL); + ck_assert_str_eq(result, expected); + g_free(result); +} +END_TEST + +START_TEST(test_rc4_enc_tv4_ok) +{ + /* + * This is one of the 5 original RC4 test vectors posted in response to + * the 'RC4 Algorithm revealed' sci.crypt usenet posting */ + unsigned char key[] = {0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef}; + unsigned char text[] = + { + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x01, 0x01 + }; + const char *expected = + "7595c3e6114a09780c4ad452338e1ffd9a1be9498f813d76533449b6778dca" + "d8c78a8d2ba9ac66085d0e53d59c26c2d1c490c1ebbe0ce66d1b6b1b13b6" + "b919b847c25a91447a95e75e4ef16779cde8bf0a95850e32af9689444fd3" + "77108f98fdcbd4e726567500990bcc7e0ca3c4aaa304a387d20f3b8fbbcd" + "42a1bd311d7a4303dda5ab078896ae80c18b0af66dff319616eb784e495a" + "d2ce90d7f772a81747b65f62093b1e0db9e5ba532fafec47508323e67132" + "7df9444432cb7367cec82f5d44c0d00b67d650a075cd4b70dedd77eb9b10" + "231b6b5b741347396d62897421d43df9b42e446e358e9c11a9b2184ecbef" + "0cd8e7a877ef968f1390ec9b3d35a5585cb009290e2fcde7b5ec66d9084b" + "e44055a619d9dd7fc3166f9487f7cb272912426445998514c15d53a18c86" + "4ce3a2b7555793988126520eacf2e3066e230c91bee4dd5304f5fd0405b3" + "5bd99c73135d3d9bc335ee049ef69b3867bf2d7bd1eaa595d8bfc0066ff8" + "d31509eb0c6caa006c807a623ef84c3d33c195d23ee320c40de0558157c8" + "22d4b8c569d849aed59d4e0fd7f379586b4b7ff684ed6a189f7486d49b9c" + "4bad9ba24b96abf924372c8a8fffb10d55354900a77a3db5f205e1b99fcd" + "8660863a159ad4abe40fa48934163ddde542a6585540fd683cbfd8c00f12" + "129a284deacc4cdefe58be7137541c047126c8d49e2755ab181ab7e940b0c0"; + + char *result; + + void *info = ssl_rc4_info_create(); + ssl_rc4_set_key(info, (char *)key, sizeof(key)); + ssl_rc4_crypt(info, (char *)text, sizeof(text)); + ssl_rc4_info_delete(info); + + result = bin_to_hex((char *)text, sizeof(text)); + ck_assert(result != NULL); + ck_assert_str_eq(result, expected); + g_free(result); +} +END_TEST + +START_TEST(test_sha1_hash_ok) +{ + const char *hash_string = "xrdp-test-suite-sha1-hash"; + char digest[20]; + char *result1; + char *result2; + + void *info = ssl_sha1_info_create(); + ssl_sha1_clear(info); + ssl_sha1_transform(info, hash_string, g_strlen(hash_string)); + ssl_sha1_complete(info, digest); + result1 = bin_to_hex(digest, sizeof(digest)); + ck_assert(result1 != NULL); + /* Check result with echo -n '' | sha1sum */ + ck_assert_str_eq(result1, "3ea0ae84e97e6262c7cfe79ccd7ad2094c06885d"); + + /* Check a clear has the desired effect */ + ssl_sha1_clear(info); + ssl_sha1_transform(info, hash_string, g_strlen(hash_string)); + ssl_sha1_complete(info, digest); + result2 = bin_to_hex(digest, sizeof(digest)); + ck_assert(result2 != NULL); + ck_assert_str_eq(result1, result2); + + ssl_sha1_info_delete(info); + g_free(result1); + g_free(result2); +} +END_TEST + +START_TEST(test_md5_hash_ok) +{ + const char *hash_string = "xrdp-test-suite-md5-hash"; + char digest[16]; + char *result1; + char *result2; + + void *info = ssl_md5_info_create(); + + ssl_md5_clear(info); + ssl_md5_transform(info, hash_string, g_strlen(hash_string)); + ssl_md5_complete(info, digest); + result1 = bin_to_hex(digest, sizeof(digest)); + ck_assert(result1 != NULL); + /* Check result with echo -n '' | md5sum */ + ck_assert_str_eq(result1, "ddc599dc7ec62b8f78760b071704c007"); + + /* Check a clear has the desired effect */ + ssl_md5_clear(info); + ssl_md5_transform(info, hash_string, g_strlen(hash_string)); + ssl_md5_complete(info, digest); + result2 = bin_to_hex(digest, sizeof(digest)); + ck_assert(result2 != NULL); + ck_assert_str_eq(result1, result2); + + ssl_md5_info_delete(info); + g_free(result1); + g_free(result2); +} +END_TEST + +START_TEST(test_des3_enc_ok) +{ + const char *key = "24_byte_key-------------"; + char plaintext[] = "xrdp-test-suite-des3-encryption-must-be-multiple-of-8-chars-long--------"; + char ciphertext[sizeof(plaintext) - 1]; /* No terminator needed */ + char plaintext2[sizeof(plaintext)]; + + char *result; + + void *info = ssl_des3_encrypt_info_create(key, 0); + ssl_des3_encrypt(info, sizeof(plaintext) - 1, plaintext, ciphertext); + ssl_des3_info_delete(info); + result = bin_to_hex(ciphertext, sizeof(ciphertext)); + ck_assert(result != NULL); + /* Result should be the same as + * echo -n '' | \ + * openssl des3 -iv 0000000000000000 -K -e -nopad | \ + * od -t x1 + * + * where is the string above in hexadecimal */ + ck_assert_str_eq(result, + "856d70861827365e188781616e4f9dcc3009b2c5dc7785edcbc05fa825a4ea5e10b23735c0e971ca20f895f455b8845418963af6dd8e649719790eed6cbcee0fb97b743c60e32e8b"); + g_free(result); + + /* Let's go back again */ + info = ssl_des3_decrypt_info_create(key, 0); + ssl_des3_decrypt(info, sizeof(ciphertext), ciphertext, plaintext2); + ssl_des3_info_delete(info); + plaintext2[sizeof(plaintext2) - 1] = '\0'; + + ck_assert_str_eq(plaintext, plaintext2); +} +END_TEST + +START_TEST(test_hmac_sha1_dgst_ok) +{ + const char *key = "20_byte_key---------"; + const char *hash_string = "xrdp-test-suite-hmac-sha1-dgst"; + char hmac[20]; + + char *result; + + void *info = ssl_hmac_info_create(); + ssl_hmac_sha1_init(info, key, g_strlen(key)); + ssl_hmac_transform(info, hash_string, g_strlen(hash_string)); + ssl_hmac_complete(info, hmac, sizeof(hmac)); + ssl_hmac_info_delete(info); + result = bin_to_hex(hmac, sizeof(hmac)); + ck_assert(result != NULL); + /* Result should be the same as + * echo -n '' | openssl dgst -sha1 -hmac '' + * + * or:- + * + * echo -n '' | openssl mac -digest sha1 -macopt key:'' hmac + */ + ck_assert_str_eq(result, "af8c04e609e9f3cba53ad7815b60160dc69a9936"); + g_free(result); + +} +END_TEST + +START_TEST(test_gen_key_xrdp1) +{ +#define RSA_TEST_BITS 2048 + char modulus[RSA_TEST_BITS / 8] = {0}; + char private_key[RSA_TEST_BITS / 8] = {0}; + unsigned char exponent[4] = + { + 0x01, 0x00, 0x01, 0x00 /* 65537 in little-endian format */ + }; + + /* + * We can't do much here because of the nature of the call, but we can + * at least check it completes without error */ + int error; + error = ssl_gen_key_xrdp1(RSA_TEST_BITS, + (const char *)exponent, sizeof(exponent), + modulus, sizeof(modulus), + private_key, sizeof(private_key)); + ck_assert(error == 0); + + /* Both the modulus and the privatekey should be odd */ + ck_assert((modulus[0] & 1) == 1); + ck_assert((private_key[0] & 1) == 1); +#undef RSA_TEST_BITS +} +END_TEST + +/******************************************************************************/ +Suite * +make_suite_test_ssl_calls(void) +{ + Suite *s; + TCase *tc_ssl_calls; + + s = suite_create("SSL-Calls"); + + tc_ssl_calls = tcase_create("ssl_calls"); + suite_add_tcase(s, tc_ssl_calls); + tcase_add_test(tc_ssl_calls, test_rc4_enc_ok); + tcase_add_test(tc_ssl_calls, test_rc4_enc_tv0_ok); + tcase_add_test(tc_ssl_calls, test_rc4_enc_tv1_ok); + tcase_add_test(tc_ssl_calls, test_rc4_enc_tv2_ok); + tcase_add_test(tc_ssl_calls, test_rc4_enc_tv3_ok); + tcase_add_test(tc_ssl_calls, test_rc4_enc_tv4_ok); + tcase_add_test(tc_ssl_calls, test_sha1_hash_ok); + tcase_add_test(tc_ssl_calls, test_md5_hash_ok); + tcase_add_test(tc_ssl_calls, test_des3_enc_ok); + tcase_add_test(tc_ssl_calls, test_hmac_sha1_dgst_ok); + tcase_add_test(tc_ssl_calls, test_gen_key_xrdp1); + + return s; +} From 6cebade78e15ac5fdb5c807220291ef19ed41a5b Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Thu, 20 Jan 2022 16:45:25 +0000 Subject: [PATCH 4/8] OpenSSL 3.x compatibility --- common/ssl_calls.c | 406 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 370 insertions(+), 36 deletions(-) diff --git a/common/ssl_calls.c b/common/ssl_calls.c index 30640c51..792a9130 100644 --- a/common/ssl_calls.c +++ b/common/ssl_calls.c @@ -44,6 +44,16 @@ #define SSL_WANT_READ_WRITE_TIMEOUT 100 +/* + * Globals used by openssl 3 and later */ +#if OPENSSL_VERSION_NUMBER >= 0x30000000L +static EVP_MD *g_md_md5; /* MD5 message digest */ +static EVP_MD *g_md_sha1; /* SHA1 message digest */ +static EVP_CIPHER *g_cipher_des_ede3_cbc; /* DES3 CBC cipher */ +static EVP_MAC *g_mac_hmac; /* HMAC MAC */ +#endif + + #if OPENSSL_VERSION_NUMBER < 0x10100000L static inline HMAC_CTX * HMAC_CTX_new(void) @@ -106,12 +116,41 @@ DH_set0_pqg(DH *dh, BIGNUM *p, BIGNUM *q, BIGNUM *g) #endif /* OPENSSL_VERSION_NUMBER >= 0x10100000L */ + +/*****************************************************************************/ +static void +dump_error_stack(const char *prefix) +{ + /* Dump the error stack from the SSL library */ + unsigned long code; + char buff[256]; + while ((code = ERR_get_error()) != 0L) + { + ERR_error_string_n(code, buff, sizeof(buff)); + LOG(LOG_LEVEL_ERROR, "%s: %s", prefix, buff); + } +} + +/*****************************************************************************/ +/* As above, but used for TLS connection errors where only the first + error is logged */ +static void +dump_ssl_error_stack(struct ssl_tls *self) +{ + if (!self->error_logged) + { + dump_error_stack("SSL"); + self->error_logged = 1; + } +} + /*****************************************************************************/ int ssl_init(void) { SSL_load_error_strings(); SSL_library_init(); + return 0; } @@ -119,16 +158,44 @@ ssl_init(void) int ssl_finish(void) { +#if OPENSSL_VERSION_NUMBER >= 0x30000000L + /* De-allocate any allocated globals + * For OpenSSL 3, these can all safely be passed a NULL pointer */ + EVP_MD_free(g_md_md5); + EVP_MD_free(g_md_sha1); + EVP_CIPHER_free(g_cipher_des_ede3_cbc); + EVP_MAC_free(g_mac_hmac); +#endif return 0; } -/* rc4 stuff */ +/* rc4 stuff + * + * For OpenSSL 3.0, the rc4 encryption algorithm is only provided by the + * legacy provider (see crypto(7)). Since RC4 is so simple, we can implement + * it directly rather than having to load the legacy provider. This will + * avoids problems if running on a system where openssl has been built + * without the legacy provider */ + +#if OPENSSL_VERSION_NUMBER >= 0x30000000L +struct rc4_data +{ + /* See https://en.wikipedia.org/wiki/RC4 */ + unsigned char S[256]; + int i; + int j; +}; +#endif /*****************************************************************************/ void * ssl_rc4_info_create(void) { +#if OPENSSL_VERSION_NUMBER < 0x30000000L return g_malloc(sizeof(RC4_KEY), 1); +#else + return g_malloc(sizeof(struct rc4_data), 1); +#endif } /*****************************************************************************/ @@ -142,14 +209,75 @@ ssl_rc4_info_delete(void *rc4_info) void ssl_rc4_set_key(void *rc4_info, const char *key, int len) { +#if OPENSSL_VERSION_NUMBER < 0x30000000L RC4_set_key((RC4_KEY *)rc4_info, len, (tui8 *)key); +#else + unsigned char *S = ((struct rc4_data *)rc4_info)->S; + int i; + int j = 0; + unsigned char t; + for (i = 0 ; i < 256; ++i) + { + S[i] = i; + } + for (i = 0 ; i < 256; ++i) + { + j = (j + S[i] + key[i % len]) & 0xff; + t = S[i]; + S[i] = S[j]; + S[j] = t; + } + ((struct rc4_data *)rc4_info)->i = 0; + ((struct rc4_data *)rc4_info)->j = 0; +#endif } /*****************************************************************************/ void ssl_rc4_crypt(void *rc4_info, char *data, int len) { +#if OPENSSL_VERSION_NUMBER < 0x30000000L RC4((RC4_KEY *)rc4_info, len, (tui8 *)data, (tui8 *)data); +#else + unsigned char *S = ((struct rc4_data *)rc4_info)->S; + int i = ((struct rc4_data *)rc4_info)->i; + int j = ((struct rc4_data *)rc4_info)->j; + unsigned char *p = (unsigned char *)data; + unsigned char t; + unsigned char k; + + /* + * Do some loop-unrolling for performance. Here are the steps + * for each byte */ +#define RC4_ROUND \ + i = (i + 1) & 0xff; \ + j = (j + S[i]) & 0xff; \ + t = S[i]; \ + S[i] = S[j]; \ + S[j] = t; \ + k = S[(S[i] + S[j]) & 0xff]; \ + *p++ ^= k + + while (len >= 8) + { + RC4_ROUND; + RC4_ROUND; + RC4_ROUND; + RC4_ROUND; + RC4_ROUND; + RC4_ROUND; + RC4_ROUND; + RC4_ROUND; + len -= 8; + } + while (len-- > 0) + { + RC4_ROUND; + } + + ((struct rc4_data *)rc4_info)->i = i; + ((struct rc4_data *)rc4_info)->j = j; +#endif } /* sha1 stuff */ @@ -158,35 +286,78 @@ ssl_rc4_crypt(void *rc4_info, char *data, int len) void * ssl_sha1_info_create(void) { +#if OPENSSL_VERSION_NUMBER < 0x30000000L return g_malloc(sizeof(SHA_CTX), 1); +#else + /* + * If we can't get the digest loaded, there's a problem with the + * library providers, so there's no point in us returning anything useful. + * If we do load the digest, it's used later */ + if (g_md_sha1 == NULL) + { + if ((g_md_sha1 = EVP_MD_fetch(NULL, "sha1", NULL)) == NULL) + { + dump_error_stack("sha1"); + return NULL; + } + } + + return (void *)EVP_MD_CTX_new(); +#endif } /*****************************************************************************/ void ssl_sha1_info_delete(void *sha1_info) { +#if OPENSSL_VERSION_NUMBER < 0x30000000L g_free(sha1_info); +#else + EVP_MD_CTX_free((EVP_MD_CTX *)sha1_info); +#endif } /*****************************************************************************/ void ssl_sha1_clear(void *sha1_info) { +#if OPENSSL_VERSION_NUMBER < 0x30000000L SHA1_Init((SHA_CTX *)sha1_info); +#else + if (sha1_info != NULL) + { + EVP_DigestInit_ex(sha1_info, g_md_sha1, NULL); + } +#endif } /*****************************************************************************/ void ssl_sha1_transform(void *sha1_info, const char *data, int len) { +#if OPENSSL_VERSION_NUMBER < 0x30000000L SHA1_Update((SHA_CTX *)sha1_info, data, len); +#else + if (sha1_info != NULL) + { + EVP_DigestUpdate((EVP_MD_CTX *)sha1_info, data, len); + } +#endif } /*****************************************************************************/ void ssl_sha1_complete(void *sha1_info, char *data) { +#if OPENSSL_VERSION_NUMBER < 0x30000000L SHA1_Final((tui8 *)data, (SHA_CTX *)sha1_info); +#else + if (sha1_info != NULL) + { + EVP_DigestFinal_ex((EVP_MD_CTX *)sha1_info, (unsigned char *)data, + NULL); + } +#endif } /* md5 stuff */ @@ -195,35 +366,77 @@ ssl_sha1_complete(void *sha1_info, char *data) void * ssl_md5_info_create(void) { +#if OPENSSL_VERSION_NUMBER < 0x30000000L return g_malloc(sizeof(MD5_CTX), 1); +#else + /* + * If we can't get the digest loaded, there's a problem with the + * library providers, so there's no point in us returning anything useful. + * If we do load the digest, it's used later */ + if (g_md_md5 == NULL) + { + if ((g_md_md5 = EVP_MD_fetch(NULL, "md5", NULL)) == NULL) + { + dump_error_stack("md5"); + return NULL; + } + } + + return (void *)EVP_MD_CTX_new(); +#endif } /*****************************************************************************/ void ssl_md5_info_delete(void *md5_info) { +#if OPENSSL_VERSION_NUMBER < 0x30000000L g_free(md5_info); +#else + EVP_MD_CTX_free((EVP_MD_CTX *)md5_info); +#endif } /*****************************************************************************/ void ssl_md5_clear(void *md5_info) { +#if OPENSSL_VERSION_NUMBER < 0x30000000L MD5_Init((MD5_CTX *)md5_info); +#else + if (md5_info != NULL) + { + EVP_DigestInit_ex(md5_info, g_md_md5, NULL); + } +#endif } /*****************************************************************************/ void ssl_md5_transform(void *md5_info, const char *data, int len) { +#if OPENSSL_VERSION_NUMBER < 0x30000000L MD5_Update((MD5_CTX *)md5_info, data, len); +#else + if (md5_info != NULL) + { + EVP_DigestUpdate((EVP_MD_CTX *)md5_info, data, len); + } +#endif } /*****************************************************************************/ void ssl_md5_complete(void *md5_info, char *data) { +#if OPENSSL_VERSION_NUMBER < 0x30000000L MD5_Final((tui8 *)data, (MD5_CTX *)md5_info); +#else + if (md5_info != NULL) + { + EVP_DigestFinal_ex((EVP_MD_CTX *)md5_info, (unsigned char *)data, NULL); + } +#endif } /* FIPS stuff */ @@ -236,10 +449,30 @@ ssl_des3_encrypt_info_create(const char *key, const char *ivec) const tui8 *lkey; const tui8 *livec; +#if OPENSSL_VERSION_NUMBER >= 0x30000000L + /* + * For these versions of OpenSSL, there are no long-term guarantees the + * DES3 cipher will be available. We'll try to load it here so we + * can log any errors */ + if (g_cipher_des_ede3_cbc == NULL) + { + g_cipher_des_ede3_cbc = EVP_CIPHER_fetch(NULL, "des-ede3-cbc", NULL); + if (g_cipher_des_ede3_cbc == NULL) + { + dump_error_stack("DES-EDE3-CBC"); + return NULL; + } + } +#endif + des3_ctx = EVP_CIPHER_CTX_new(); lkey = (const tui8 *) key; livec = (const tui8 *) ivec; +#if OPENSSL_VERSION_NUMBER < 0x30000000L EVP_EncryptInit_ex(des3_ctx, EVP_des_ede3_cbc(), NULL, lkey, livec); +#else + EVP_EncryptInit_ex(des3_ctx, g_cipher_des_ede3_cbc, NULL, lkey, livec); +#endif EVP_CIPHER_CTX_set_padding(des3_ctx, 0); return des3_ctx; } @@ -252,10 +485,30 @@ ssl_des3_decrypt_info_create(const char *key, const char *ivec) const tui8 *lkey; const tui8 *livec; +#if OPENSSL_VERSION_NUMBER >= 0x30000000L + /* + * For these versions of OpenSSL, there are no long-term guarantees the + * DES3 cipher will be available. We'll try to load it here so we + * can log any errors */ + if (g_cipher_des_ede3_cbc == NULL) + { + g_cipher_des_ede3_cbc = EVP_CIPHER_fetch(NULL, "des-ede3-cbc", NULL); + if (g_cipher_des_ede3_cbc == NULL) + { + dump_error_stack("DES-EDE3-CBC"); + return NULL; + } + } +#endif + des3_ctx = EVP_CIPHER_CTX_new(); lkey = (const tui8 *) key; livec = (const tui8 *) ivec; +#if OPENSSL_VERSION_NUMBER < 0x30000000L EVP_DecryptInit_ex(des3_ctx, EVP_des_ede3_cbc(), NULL, lkey, livec); +#else + EVP_DecryptInit_ex(des3_ctx, g_cipher_des_ede3_cbc, NULL, lkey, livec); +#endif EVP_CIPHER_CTX_set_padding(des3_ctx, 0); return des3_ctx; } @@ -283,10 +536,13 @@ ssl_des3_encrypt(void *des3, int length, const char *in_data, char *out_data) tui8 *lout_data; des3_ctx = (EVP_CIPHER_CTX *) des3; - lin_data = (const tui8 *) in_data; - lout_data = (tui8 *) out_data; - len = 0; - EVP_EncryptUpdate(des3_ctx, lout_data, &len, lin_data, length); + if (des3_ctx != NULL) + { + lin_data = (const tui8 *) in_data; + lout_data = (tui8 *) out_data; + len = 0; + EVP_EncryptUpdate(des3_ctx, lout_data, &len, lin_data, length); + } return 0; } @@ -300,10 +556,13 @@ ssl_des3_decrypt(void *des3, int length, const char *in_data, char *out_data) tui8 *lout_data; des3_ctx = (EVP_CIPHER_CTX *) des3; - lin_data = (const tui8 *) in_data; - lout_data = (tui8 *) out_data; - len = 0; - EVP_DecryptUpdate(des3_ctx, lout_data, &len, lin_data, length); + if (des3_ctx != NULL) + { + lin_data = (const tui8 *) in_data; + lout_data = (tui8 *) out_data; + len = 0; + EVP_DecryptUpdate(des3_ctx, lout_data, &len, lin_data, length); + } return 0; } @@ -311,16 +570,28 @@ ssl_des3_decrypt(void *des3, int length, const char *in_data, char *out_data) void * ssl_hmac_info_create(void) { - HMAC_CTX *hmac_ctx; +#if OPENSSL_VERSION_NUMBER < 0x30000000L + return (HMAC_CTX *)HMAC_CTX_new(); +#else + /* Need a MAC algorithm loaded */ + if (g_mac_hmac == NULL) + { + if ((g_mac_hmac = EVP_MAC_fetch(NULL, "hmac", NULL)) == NULL) + { + dump_error_stack("hmac"); + return NULL; + } + } - hmac_ctx = HMAC_CTX_new(); - return hmac_ctx; + return (void *)EVP_MAC_CTX_new(g_mac_hmac); +#endif } /*****************************************************************************/ void ssl_hmac_info_delete(void *hmac) { +#if OPENSSL_VERSION_NUMBER < 0x30000000L HMAC_CTX *hmac_ctx; hmac_ctx = (HMAC_CTX *) hmac; @@ -328,34 +599,61 @@ ssl_hmac_info_delete(void *hmac) { HMAC_CTX_free(hmac_ctx); } +#else + EVP_MAC_CTX_free((EVP_MAC_CTX *)hmac); +#endif } /*****************************************************************************/ void ssl_hmac_sha1_init(void *hmac, const char *data, int len) { +#if OPENSSL_VERSION_NUMBER < 0x30000000L HMAC_CTX *hmac_ctx; hmac_ctx = (HMAC_CTX *) hmac; HMAC_Init_ex(hmac_ctx, data, len, EVP_sha1(), NULL); +#else + if (hmac != NULL) + { + char digest[] = "sha1"; + OSSL_PARAM params[3]; + size_t n = 0; + params[n++] = OSSL_PARAM_construct_utf8_string("digest", digest, 0); + params[n++] = OSSL_PARAM_construct_end(); + if (EVP_MAC_init((EVP_MAC_CTX *)hmac, (unsigned char *)data, + len, params) == 0) + { + dump_error_stack("hmac-sha1"); + } + } +#endif } /*****************************************************************************/ void ssl_hmac_transform(void *hmac, const char *data, int len) { +#if OPENSSL_VERSION_NUMBER < 0x30000000L HMAC_CTX *hmac_ctx; const tui8 *ldata; hmac_ctx = (HMAC_CTX *) hmac; ldata = (const tui8 *) data; HMAC_Update(hmac_ctx, ldata, len); +#else + if (hmac != NULL) + { + EVP_MAC_update((EVP_MAC_CTX *)hmac, (unsigned char *)data, len); + } +#endif } /*****************************************************************************/ void ssl_hmac_complete(void *hmac, char *data, int len) { +#if OPENSSL_VERSION_NUMBER < 0x30000000L HMAC_CTX *hmac_ctx; tui8 *ldata; tui32 llen; @@ -364,6 +662,12 @@ ssl_hmac_complete(void *hmac, char *data, int len) ldata = (tui8 *) data; llen = len; HMAC_Final(hmac_ctx, ldata, &llen); +#else + if (hmac != NULL) + { + EVP_MAC_final((EVP_MAC_CTX *)hmac, (unsigned char *)data, NULL, len); + } +#endif } /*****************************************************************************/ @@ -455,7 +759,6 @@ ssl_gen_key_xrdp1(int key_size_in_bits, const char *exp, int exp_len, char *mod, int mod_len, char *pri, int pri_len) { BIGNUM *my_e; - RSA *my_key; char *lexp; char *lmod; char *lpri; @@ -477,12 +780,44 @@ ssl_gen_key_xrdp1(int key_size_in_bits, const char *exp, int exp_len, ssl_reverse_it(lexp, exp_len); my_e = BN_new(); BN_bin2bn((tui8 *)lexp, exp_len, my_e); - my_key = RSA_new(); + +#if OPENSSL_VERSION_NUMBER < 0x30000000L + const BIGNUM *n = NULL; + const BIGNUM *d = NULL; + RSA *my_key = RSA_new(); error = RSA_generate_key_ex(my_key, key_size_in_bits, my_e, 0) == 0; - const BIGNUM *n; - const BIGNUM *d; + /* After this call, n and d point directly into my_key, and are valid + * until my_key is free'd */ RSA_get0_key(my_key, &n, NULL, &d); +#else + BIGNUM *n = NULL; + BIGNUM *d = NULL; + OSSL_PARAM params[] = + { + OSSL_PARAM_construct_int("bits", &key_size_in_bits), + OSSL_PARAM_construct_end() + }; + EVP_PKEY_CTX *pctx = EVP_PKEY_CTX_new_from_name(NULL, "RSA", NULL); + EVP_PKEY *pkey = NULL; + + if (pctx != NULL && + EVP_PKEY_keygen_init(pctx) > 0 && + EVP_PKEY_CTX_set_params(pctx, params) > 0 && + EVP_PKEY_generate(pctx, &pkey) > 0 && + EVP_PKEY_get_bn_param(pkey, "n", &n) > 0 && + EVP_PKEY_get_bn_param(pkey, "d", &d) > 0) + { + error = 0; + } + else + { + error = 1; + } + + EVP_PKEY_CTX_free(pctx); + EVP_PKEY_free(pkey); +#endif if (error == 0) { @@ -517,7 +852,12 @@ ssl_gen_key_xrdp1(int key_size_in_bits, const char *exp, int exp_len, } BN_free(my_e); +#if OPENSSL_VERSION_NUMBER < 0x30000000L RSA_free(my_key); +#else + BN_free(n); + BN_clear_free(d); +#endif g_free(lexp); g_free(lmod); g_free(lpri); @@ -529,7 +869,11 @@ ssl_gen_key_xrdp1(int key_size_in_bits, const char *exp, int exp_len, see also * https://wiki.openssl.org/index.php/Diffie-Hellman_parameters * https://wiki.openssl.org/index.php/Manual:SSL_CTX_set_tmp_dh_callback(3) + * + * We dont do this for OpenSSL 3 - we use SSL_CTX_set_dh_auto() instead, as this + * can cater for different key sizes on the certificate */ +#if OPENSSL_VERSION_NUMBER < 0x30000000L static DH *ssl_get_dh2236() { static unsigned char dh2236_p[] = @@ -591,6 +935,7 @@ static DH *ssl_get_dh2236() return dh; } +#endif /* OPENSSL_VERSION_NUMBER < 0x30000000L */ /*****************************************************************************/ struct ssl_tls * @@ -613,25 +958,6 @@ ssl_tls_create(struct trans *trans, const char *key, const char *cert) return self; } - -/*****************************************************************************/ -static void -dump_ssl_error_stack(struct ssl_tls *self) -{ - if (!self->error_logged) - { - /* Dump the error stack from the SSL library */ - unsigned long code; - char buff[256]; - while ((code = ERR_get_error()) != 0L) - { - ERR_error_string_n(code, buff, sizeof(buff)); - LOG(LOG_LEVEL_ERROR, "SSL: %s", buff); - } - self->error_logged = 1; - } -} - /*****************************************************************************/ static int ssl_tls_log_error(struct ssl_tls *self, const char *func, int value) @@ -752,6 +1078,7 @@ ssl_tls_accept(struct ssl_tls *self, long ssl_protocols, SSL_CTX_set_options(self->ctx, options); /* set DH parameters */ +#if OPENSSL_VERSION_NUMBER < 0x30000000L DH *dh = ssl_get_dh2236(); if (dh == NULL) { @@ -766,7 +1093,14 @@ ssl_tls_accept(struct ssl_tls *self, long ssl_protocols, return 1; } DH_free(dh); // ok to free, copied into ctx by SSL_CTX_set_tmp_dh() - +#else + if (!SSL_CTX_set_dh_auto(self->ctx, 1)) + { + LOG(LOG_LEVEL_ERROR, "TLS DHE auto failed to be enabled"); + dump_ssl_error_stack(self); + return 1; + } +#endif #if defined(SSL_CTX_set_ecdh_auto) if (!SSL_CTX_set_ecdh_auto(self->ctx, 1)) { From c894ba5b40011c36afdfb3d57547d2801fdf6409 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Fri, 28 Jan 2022 12:17:38 +0000 Subject: [PATCH 5/8] Better logging of classic connection security --- libxrdp/xrdp_rdp.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/libxrdp/xrdp_rdp.c b/libxrdp/xrdp_rdp.c index c2b23289..d08f068a 100644 --- a/libxrdp/xrdp_rdp.c +++ b/libxrdp/xrdp_rdp.c @@ -942,11 +942,21 @@ xrdp_rdp_incoming(struct xrdp_rdp *self) /* log non-TLS connections */ else { + int crypt_level = self->sec_layer->crypt_level; + const char *security_level = + (crypt_level == CRYPT_LEVEL_NONE) ? "none" : + (crypt_level == CRYPT_LEVEL_LOW) ? "low" : + (crypt_level == CRYPT_LEVEL_CLIENT_COMPATIBLE) ? "medium" : + (crypt_level == CRYPT_LEVEL_HIGH) ? "high" : + (crypt_level == CRYPT_LEVEL_FIPS) ? "fips" : + /* default */ "unknown"; + LOG(LOG_LEVEL_INFO, "Non-TLS connection established from %s port %s: " - "encrypted with standard RDP security", + "with security level : %s", self->client_info.client_addr, - self->client_info.client_port); + self->client_info.client_port, + security_level); } return 0; From e79bc7f1815b25390b4228378845a1f9028e74fc Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Thu, 27 Jan 2022 15:55:53 +0000 Subject: [PATCH 6/8] Fix banner comments in test results --- tests/libxrdp/Makefile.am | 2 ++ tests/memtest/Makefile.am | 2 ++ 2 files changed, 4 insertions(+) diff --git a/tests/libxrdp/Makefile.am b/tests/libxrdp/Makefile.am index e98e8d9c..30138cbf 100644 --- a/tests/libxrdp/Makefile.am +++ b/tests/libxrdp/Makefile.am @@ -6,6 +6,8 @@ AM_CPPFLAGS = \ LOG_DRIVER = env AM_TAP_AWK='$(AWK)' $(SHELL) \ $(top_srcdir)/tap-driver.sh +PACKAGE_STRING = "libxrdp" + TESTS = test_libxrdp check_PROGRAMS = test_libxrdp diff --git a/tests/memtest/Makefile.am b/tests/memtest/Makefile.am index 40669125..8660fc99 100644 --- a/tests/memtest/Makefile.am +++ b/tests/memtest/Makefile.am @@ -2,6 +2,8 @@ AM_CPPFLAGS = \ -I$(top_srcdir)/common +PACKAGE_STRING = "memtest" + check_PROGRAMS = \ memtest From 8b8cfbe1195bef6384674641225d40b5e98e88f4 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Thu, 27 Jan 2022 16:31:53 +0000 Subject: [PATCH 7/8] Improve wrapping of openssl module --- common/ssl_calls.c | 26 ++++++++++++++++++++++---- common/ssl_calls.h | 22 ++++++++-------------- common/trans.c | 12 ++++-------- 3 files changed, 34 insertions(+), 26 deletions(-) diff --git a/common/ssl_calls.c b/common/ssl_calls.c index 792a9130..168add76 100644 --- a/common/ssl_calls.c +++ b/common/ssl_calls.c @@ -53,6 +53,17 @@ static EVP_CIPHER *g_cipher_des_ede3_cbc; /* DES3 CBC cipher */ static EVP_MAC *g_mac_hmac; /* HMAC MAC */ #endif +/* definition of ssl_tls */ +struct ssl_tls +{ + SSL *ssl; /* SSL * */ + SSL_CTX *ctx; /* SSL_CTX * */ + char *cert; + char *key; + struct trans *trans; + tintptr rwo; /* wait obj */ + int error_logged; /* Error has already been logged */ +}; #if OPENSSL_VERSION_NUMBER < 0x10100000L static inline HMAC_CTX * @@ -1392,16 +1403,23 @@ ssl_tls_can_recv(struct ssl_tls *tls, int sck, int millis) /*****************************************************************************/ const char * -ssl_get_version(const struct ssl_st *ssl) +ssl_get_version(const struct ssl_tls *ssl) { - return SSL_get_version(ssl); + return SSL_get_version(ssl->ssl); } /*****************************************************************************/ const char * -ssl_get_cipher_name(const struct ssl_st *ssl) +ssl_get_cipher_name(const struct ssl_tls *ssl) { - return SSL_get_cipher_name(ssl); + return SSL_get_cipher_name(ssl->ssl); +} + +/*****************************************************************************/ +tintptr +ssl_get_rwo(const struct ssl_tls *ssl) +{ + return ssl->rwo; } /*****************************************************************************/ diff --git a/common/ssl_calls.h b/common/ssl_calls.h index 78edd946..01b67406 100644 --- a/common/ssl_calls.h +++ b/common/ssl_calls.h @@ -22,6 +22,10 @@ #include "arch.h" +/* Incomplete types */ +struct ssl_tls; +struct trans; + int ssl_init(void); int @@ -81,18 +85,6 @@ int ssl_gen_key_xrdp1(int key_size_in_bits, const char *exp, int exp_len, char *mod, int mod_len, char *pri, int pri_len); -/* ssl_tls */ -struct ssl_tls -{ - struct ssl_st *ssl; /* SSL * */ - struct ssl_ctx_st *ctx; /* SSL_CTX * */ - char *cert; - char *key; - struct trans *trans; - tintptr rwo; /* wait obj */ - int error_logged; /* Error has already been logged */ -}; - /* xrdp_tls.c */ struct ssl_tls * ssl_tls_create(struct trans *trans, const char *key, const char *cert); @@ -110,12 +102,14 @@ ssl_tls_write(struct ssl_tls *tls, const char *data, int length); int ssl_tls_can_recv(struct ssl_tls *tls, int sck, int millis); const char * -ssl_get_version(const struct ssl_st *ssl); +ssl_get_version(const struct ssl_tls *ssl); const char * -ssl_get_cipher_name(const struct ssl_st *ssl); +ssl_get_cipher_name(const struct ssl_tls *ssl); int ssl_get_protocols_from_string(const char *str, long *ssl_protocols); const char * get_openssl_version(); +tintptr +ssl_get_rwo(const struct ssl_tls *ssl); #endif diff --git a/common/trans.c b/common/trans.c index 3afae55d..55d2a638 100644 --- a/common/trans.c +++ b/common/trans.c @@ -179,13 +179,9 @@ trans_get_wait_objs(struct trans *self, tbus *objs, int *count) objs[*count] = self->sck; (*count)++; - if (self->tls != 0) + if (self->tls != NULL && (objs[*count] = ssl_get_rwo(self->tls)) != 0) { - if (self->tls->rwo != 0) - { - objs[*count] = self->tls->rwo; - (*count)++; - } + (*count)++; } return 0; @@ -995,8 +991,8 @@ trans_set_tls_mode(struct trans *self, const char *key, const char *cert, self->trans_send = trans_tls_send; self->trans_can_recv = trans_tls_can_recv; - self->ssl_protocol = ssl_get_version(self->tls->ssl); - self->cipher_name = ssl_get_cipher_name(self->tls->ssl); + self->ssl_protocol = ssl_get_version(self->tls); + self->cipher_name = ssl_get_cipher_name(self->tls); return 0; } From 4699dced141698bde452abb0b3ec817503e11c27 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Thu, 27 Jan 2022 16:23:51 +0000 Subject: [PATCH 8/8] Implement base64 without openssl --- common/base64.c | 246 +++++++++++++++++--- common/base64.h | 66 +++++- tests/common/Makefile.am | 5 +- tests/common/test_base64.c | 400 ++++++++++++++++++++++++++++++++ tests/common/test_common.h | 4 + tests/common/test_common_main.c | 31 +++ tests/common/test_ssl_calls.c | 26 --- xrdp/xrdp_login_wnd.c | 10 +- 8 files changed, 713 insertions(+), 75 deletions(-) create mode 100644 tests/common/test_base64.c diff --git a/common/base64.c b/common/base64.c index af01d7f5..0b8cfe67 100644 --- a/common/base64.c +++ b/common/base64.c @@ -1,5 +1,5 @@ /** - * Copyright (C) 2017 Koichiro Iwao, all xrdp contributors + * Copyright (C) 2022 Matt Burt, all xrdp contributors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -26,57 +26,229 @@ #include "string_calls.h" -#include -#include +#include "base64.h" -size_t -base64_decoded_bytes(const char *src) +/* + * Values for invalid and padding characters, used in the charmap + * for converting base64 to binary + * + * Thse value are specially chosen to make it easy to detect padding or + * invalid characters by or-ing together the values looked up in + * a base64 quantum */ +#define E_INVALID 0x40 +#define E_PAD 0x80 + +/* Determine the character set on this platform */ +#if ('a' == 0x61 && 'z' == 0x7a ) && \ + ('A' == 0x41 && 'Z' == 0x5a ) && \ + ('0' == 0x30 && '9' == 0x39 ) +# define PLATFORM_IS_ASCII 1 +#else +# error "Unrecognised character set on this platform" +#endif /* character set check */ + + +/* + * Define a table to map the base64 character values to bit values. + */ +#ifdef PLATFORM_IS_ASCII +#define CHARMAP_BASE 0x28 +#define E_IV E_INVALID /* For table alignment */ +const unsigned char charmap[] = { - size_t len; - size_t padding; + /* 0x28 */ E_IV, E_IV, E_IV, 0x3e, E_IV, E_IV, E_IV, 0x3f, + /* 0x30 */ 0x34, 0x35, 0x36, 0x37, 0x38, 0x39, 0x3a, 0x3b, + /* 0x38 */ 0x3c, 0x3d, E_IV, E_IV, E_IV, E_PAD, E_IV, E_IV, + /* 0x40 */ E_IV, 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, + /* 0x48 */ 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, + /* 0x50 */ 0x0f, 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, + /* 0x58 */ 0x17, 0x18, 0x19, E_IV, E_IV, E_IV, E_IV, E_IV, + /* 0x60 */ E_IV, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f, 0x20, + /* 0x68 */ 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, 0x28, + /* 0x70 */ 0x29, 0x2a, 0x2b, 0x2c, 0x2d, 0x2e, 0x2f, 0x30, + /* 0x78 */ 0x31, 0x32, 0x33 +}; +#undef E_IV +#endif /* PLATFORM_IS_ASCII */ - len = g_strlen(src); - padding = 0; - if (src[len - 1] == '=') +/** + * Lookup a value in the charmap + * + * @param x - byte to lookup. Only referenced once so can safely have + * side effects. + * @param dest - destination to assign result to. + */ +#define CM_LOOKUP(x,dest) \ + { \ + unsigned int t = (unsigned int)(x) - CHARMAP_BASE;\ + dest = (t < sizeof(charmap)) ? charmap[t] : E_INVALID; \ + } + +/*****************************************************************************/ + +int +base64_decode(const char *src, char *dst, size_t dst_len, size_t *actual_len) +{ + *actual_len = 0; + size_t src_len; + size_t src_i = 0; + size_t dst_i = 0; + unsigned int a; /* Four characters of base64 quantum */ + unsigned int b; + unsigned int c; + unsigned int d; + unsigned int v; + +#define OUTPUT_CHAR(x) \ + { \ + if (dst_i < dst_len) \ + { \ + dst[dst_i] = (x);\ + } \ + ++dst_i; \ + } + + src_len = g_strlen(src); + + while (src_i < src_len) { - padding++; - - if (src[len - 2] == '=') + if ((src_len - src_i) >= 4) { - padding++; + /* Usual case - full quantum */ + CM_LOOKUP(src[src_i++], a); + CM_LOOKUP(src[src_i++], b); + CM_LOOKUP(src[src_i++], c); + CM_LOOKUP(src[src_i++], d); + } + else + { + /* Add padding on the end to make up the full quantum */ + CM_LOOKUP(src[src_i++], a); + b = E_PAD; + c = E_PAD; + d = E_PAD; + if ((src_len - src_i) > 0) + { + CM_LOOKUP(src[src_i++], b); + } + if ((src_len - src_i) > 0) + { + CM_LOOKUP(src[src_i++], c); + } + } + + /* + * Bitwise-or the translated quantum values together, so that + * any invalid or padding characters can be detected with a + * single test */ + v = a | b | c | d; + + if ((v & E_INVALID) != 0) + { + return -1; /* At least one invalid character */ + } + + if ((v & E_PAD) == 0) + { + /* No padding - a full quantum */ + v = (a << 18) | (b << 12) | (c << 6) | d; + OUTPUT_CHAR(v >> 16); + OUTPUT_CHAR((v >> 8) & 0xff); + OUTPUT_CHAR(v & 0xff); + } + else if (((a | b | c) & E_PAD) == 0) + { + /* No padding in the first 3 chars, so the padding must + * be at the end */ + v = (a << 10) | (b << 4) | (c >> 2); + OUTPUT_CHAR(v >> 8); + OUTPUT_CHAR(v & 0xff); + } + else if (((a | b) & E_PAD) == 0 && c == d) + { + /* No padding in first two chars, so if the last two chars are + * equal, they must both be padding */ + v = (a << 2) | (b >> 4); + OUTPUT_CHAR(v); + } + else + { + /* Illegal padding */ + return -1; } } - return len * 3 / 4 - padding; + *actual_len = dst_i; + return 0; + +#undef OUTPUT_CHAR } /*****************************************************************************/ -char * -base64_decode(char *dst, const char *src, size_t len) +size_t +base64_encode(const char *src, size_t src_len, char *dst, size_t dst_len) { - BIO *b64; - BIO *bio; - char *b64str; - size_t estimated_decoded_bytes; - size_t decoded_bytes; + char *p = dst; + size_t src_i = 0; + size_t max_src_len; + unsigned int v; + static const char *b64chr = + "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + "abcdefghijklmnopqrstuvwxyz" + "0123456789+/="; - b64str = g_strdup(src); - estimated_decoded_bytes = base64_decoded_bytes(b64str); - dst[estimated_decoded_bytes] = '\0'; - - b64 = BIO_new(BIO_f_base64()); - bio = BIO_new_mem_buf(b64str, len); - bio = BIO_push(b64, bio); - BIO_set_flags(bio, BIO_FLAGS_BASE64_NO_NL); - decoded_bytes = BIO_read(bio, dst, len); - BIO_free_all(bio); - - /* if input is corrupt, return empty string */ - if (estimated_decoded_bytes != decoded_bytes) + /* Each three octets of the source results in four bytes at the output, + * plus we need a terminator. So we can work out the maximum number of + * source octets we can process */ + if (dst_len == 0) { - g_strncpy(dst, "", sizeof("")); + max_src_len = 0; + } + else + { + max_src_len = (dst_len - 1) / 4 * 3; } - return dst; + if (src_len > max_src_len) + { + src_len = max_src_len; + } + + while (src_i < src_len) + { + switch (src_len - src_i) + { + case 1: + v = (unsigned int)(unsigned char)src[src_i++] << 4; + *p++ = b64chr[v >> 6]; + *p++ = b64chr[v & 0x3f]; + *p++ = b64chr[64]; + *p++ = b64chr[64]; + break; + + case 2: + v = (unsigned int)(unsigned char)src[src_i++] << 10; + v |= (unsigned int)(unsigned char)src[src_i++] << 2; + *p++ = b64chr[v >> 12]; + *p++ = b64chr[(v >> 6) & 0x3f]; + *p++ = b64chr[v & 0x3f]; + *p++ = b64chr[64]; + break; + + default: + v = (unsigned int)(unsigned char)src[src_i++] << 16; + v |= (unsigned int)(unsigned char)src[src_i++] << 8; + v |= (unsigned int)(unsigned char)src[src_i++]; + *p++ = b64chr[v >> 18]; + *p++ = b64chr[(v >> 12) & 0x3f]; + *p++ = b64chr[(v >> 6) & 0x3f]; + *p++ = b64chr[v & 0x3f]; + break; + } + } + + *p = '\0'; + + return src_len; } diff --git a/common/base64.h b/common/base64.h index b53ed88f..1b341e9b 100644 --- a/common/base64.h +++ b/common/base64.h @@ -1,5 +1,5 @@ /** - * Copyright (C) 2017 Koichiro Iwao, all xrdp contributors + * Copyright (C) 2021 Koichiro Iwao, all xrdp contributors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,16 +17,66 @@ /** * @file common/base64.h * @brief Base64 encoder / decoder + * + * Base-64 is described in RFC4648. The following notes apply to this + * implementation:- + * - The only supported characters are [A-Za-z0-9+/=]. At present, + * embedded linefeeds and URL encodings are not supported. + * */ -#if !defined(SSL_CALLS_H) -#define SSL_CALLS_H +#if !defined(BASE64_CALLS_H) +#define BASE64_CALLS_H #include "arch.h" -size_t -base64_decoded_bytes(const char *src); -char * -base64_decode(char *dst, const char *src, size_t len); +/* + * Decodes a base64 string + * + * @param src Pointer to null-terminated source + * @param dst Pointer to output buffer + * @param dst_len Length of above. No more than this is written to the + * output buffer + * @param actual_len Pointer to value to receive actual length of decoded data + * @return 0 for success, 1 for an invalid input string + * + * The following notes apply to this implementation:- + * - Embedded padding is supported, provided it only occurs at the end + * of a quantum as described in RFC4648(4). This allows concatenated + * encodings to be fed into the decoder. + * - Padding of the last quantum is assumed if not provided. + * - Excess padding of the last quantum is ignored. + * + * Only dst_len bytes at most are written to the output. The length + * returned in actual_len however represents how much buffer is needed for + * a correct result. This may be more than dst_len, and enables the caller + * to detect a potential buffer overflow + */ +int +base64_decode(const char *src, char *dst, size_t dst_len, size_t *actual_len); -#endif +/* + * Encodes a buffer as base64 + * + * @param src Pointer to source + * @param src_len Length of above. + * @param dst Pointer to output buffer for null-terminated string + * @param dst_len Length of above. No more than this is written to the + * output buffer + * @return Number of source characters processed + * + * The following notes apply to this implementation:- + * - Padding of the last quantum is always written if the number of + * source bytes is not divisible by 3. + * + * The number of source characters processed is always returned. If + * the destination buffer is too small for all the output plus a + * terminator, the returned value from this procedure will be less than + * src_len. In this case no padding characters will have been written, + * and the remaining characters can be converted with more calls to + * this procedure. + */ +size_t +base64_encode(const char *src, size_t src_len, char *dst, size_t dst_len); + +#endif /* BASE64_CALLS_H */ diff --git a/tests/common/Makefile.am b/tests/common/Makefile.am index 88548b71..e6686e31 100644 --- a/tests/common/Makefile.am +++ b/tests/common/Makefile.am @@ -6,6 +6,8 @@ AM_CPPFLAGS = \ LOG_DRIVER = env AM_TAP_AWK='$(AWK)' $(SHELL) \ $(top_srcdir)/tap-driver.sh +PACKAGE_STRING = "libcommon" + TESTS = test_common check_PROGRAMS = test_common @@ -14,7 +16,8 @@ test_common_SOURCES = \ test_common_main.c \ test_string_calls.c \ test_os_calls.c \ - test_ssl_calls.c + test_ssl_calls.c \ + test_base64.c test_common_CFLAGS = \ @CHECK_CFLAGS@ \ diff --git a/tests/common/test_base64.c b/tests/common/test_base64.c new file mode 100644 index 00000000..4a70c921 --- /dev/null +++ b/tests/common/test_base64.c @@ -0,0 +1,400 @@ + +#if defined(HAVE_CONFIG_H) +#include "config_ac.h" +#endif + +#include "os_calls.h" +#include "string_calls.h" +#include "base64.h" + +#include "test_common.h" +/* +* These are the example test strings in RFC4648(10) +*/ +static const char *rfc4648_ex1_text = ""; +static const char *rfc4648_ex1_b64 = ""; +static const char *rfc4648_ex2_text = "f"; +static const char *rfc4648_ex2_b64 = "Zg=="; +static const char *rfc4648_ex3_text = "fo"; +static const char *rfc4648_ex3_b64 = "Zm8="; +static const char *rfc4648_ex4_text = "foo"; +static const char *rfc4648_ex4_b64 = "Zm9v"; +static const char *rfc4648_ex5_text = "foob"; +static const char *rfc4648_ex5_b64 = "Zm9vYg=="; +static const char *rfc4648_ex6_text = "fooba"; +static const char *rfc4648_ex6_b64 = "Zm9vYmE="; +static const char *rfc4648_ex7_text = "foobar"; +static const char *rfc4648_ex7_b64 = "Zm9vYmFy"; + +/* Every single valid base64 character, except padding */ +static const char *all_b64 = + "ABCDEFGHIJKL" + "MNOPQRSTUVWX" + "YZabcdefghij" + "klmnopqrstuv" + "wxyz01234567" + "89+/"; + +/* What we should get as binary if we decode this */ +static const char all_b64_decoded[] = +{ + '\x00', '\x10', '\x83', '\x10', '\x51', '\x87', '\x20', '\x92', '\x8b', + '\x30', '\xd3', '\x8f', '\x41', '\x14', '\x93', '\x51', '\x55', '\x97', + '\x61', '\x96', '\x9b', '\x71', '\xd7', '\x9f', '\x82', '\x18', '\xa3', + '\x92', '\x59', '\xa7', '\xa2', '\x9a', '\xab', '\xb2', '\xdb', '\xaf', + '\xc3', '\x1c', '\xb3', '\xd3', '\x5d', '\xb7', '\xe3', '\x9e', '\xbb', + '\xf3', '\xdf', '\xbf' +}; + +static void +test_rfc4648_to_b64(const char *plaintext, size_t len, const char *b64) +{ + char buff[256]; + size_t result; + + result = base64_encode(plaintext, len, buff, sizeof(buff)); + ck_assert_int_eq(result, len); + ck_assert_str_eq(buff, b64); + +} + +/* Text-only encoder wrapper */ +static void +test_rfc4648_to_b64_text(const char *plaintext, const char *b64) +{ + test_rfc4648_to_b64(plaintext, g_strlen(plaintext), b64); +} + +/* Text-only decoder wrapper for valid base64 */ +static void +test_rfc4648_from_b64_text(const char *b64, const char *text) +{ + char buff[256]; + size_t actual_len; + int result; + + result = base64_decode(b64, buff, sizeof(buff), &actual_len); + ck_assert_int_eq(result, 0); + ck_assert_int_lt(actual_len, sizeof(buff)); + buff[actual_len] = '\0'; + ck_assert_str_eq(buff, text); +} + +/******************************************************************************/ +START_TEST(test_b64_rfc4648_ex1_to) +{ + test_rfc4648_to_b64_text(rfc4648_ex1_text, rfc4648_ex1_b64); +} +END_TEST + +/******************************************************************************/ +START_TEST(test_b64_rfc4648_ex1_from) +{ + test_rfc4648_from_b64_text(rfc4648_ex1_b64, rfc4648_ex1_text); +} +END_TEST + +/******************************************************************************/ +START_TEST(test_b64_rfc4648_ex2_to) +{ + test_rfc4648_to_b64_text(rfc4648_ex2_text, rfc4648_ex2_b64); +} +END_TEST + +/******************************************************************************/ +START_TEST(test_b64_rfc4648_ex2_from) +{ + test_rfc4648_from_b64_text(rfc4648_ex2_b64, rfc4648_ex2_text); +} +END_TEST + +/******************************************************************************/ +START_TEST(test_b64_rfc4648_ex3_to) +{ + test_rfc4648_to_b64_text(rfc4648_ex3_text, rfc4648_ex3_b64); +} +END_TEST + +/******************************************************************************/ +START_TEST(test_b64_rfc4648_ex3_from) +{ + test_rfc4648_from_b64_text(rfc4648_ex3_b64, rfc4648_ex3_text); +} +END_TEST + +/******************************************************************************/ +START_TEST(test_b64_rfc4648_ex4_to) +{ + test_rfc4648_to_b64_text(rfc4648_ex4_text, rfc4648_ex4_b64); +} +END_TEST + +/******************************************************************************/ +START_TEST(test_b64_rfc4648_ex4_from) +{ + test_rfc4648_from_b64_text(rfc4648_ex4_b64, rfc4648_ex4_text); +} +END_TEST + +/******************************************************************************/ +START_TEST(test_b64_rfc4648_ex5_to) +{ + test_rfc4648_to_b64_text(rfc4648_ex5_text, rfc4648_ex5_b64); +} +END_TEST + +/******************************************************************************/ +START_TEST(test_b64_rfc4648_ex5_from) +{ + test_rfc4648_from_b64_text(rfc4648_ex5_b64, rfc4648_ex5_text); +} +END_TEST + +/******************************************************************************/ +START_TEST(test_b64_rfc4648_ex6_to) +{ + test_rfc4648_to_b64_text(rfc4648_ex6_text, rfc4648_ex6_b64); +} +END_TEST + +/******************************************************************************/ +START_TEST(test_b64_rfc4648_ex6_from) +{ + test_rfc4648_from_b64_text(rfc4648_ex6_b64, rfc4648_ex6_text); +} +END_TEST + +/******************************************************************************/ +START_TEST(test_b64_rfc4648_ex7_to) +{ + test_rfc4648_to_b64_text(rfc4648_ex7_text, rfc4648_ex7_b64); +} +END_TEST + +/******************************************************************************/ +START_TEST(test_b64_rfc4648_ex7_from) +{ + test_rfc4648_from_b64_text(rfc4648_ex7_b64, rfc4648_ex7_text); +} +END_TEST + +/******************************************************************************/ +START_TEST(test_b64_all_valid_from) +{ + char buff[256]; + size_t actual_len; + int result; + char *str_result; + char *str_expected; + + result = base64_decode(all_b64, buff, sizeof(buff), &actual_len); + ck_assert_int_eq(result, 0); + ck_assert_int_eq(actual_len, sizeof(all_b64_decoded)); + str_result = bin_to_hex(buff, actual_len); + str_expected = bin_to_hex(all_b64_decoded, sizeof(all_b64_decoded)); + ck_assert_str_eq(str_result, str_expected); + free(str_result); + free(str_expected); +} +END_TEST + +/******************************************************************************/ +START_TEST(test_b64_all_valid_to) +{ + char buff[256]; + size_t result; + + result = base64_encode(all_b64_decoded, sizeof(all_b64_decoded), + buff, sizeof(buff)); + ck_assert_int_eq(result, sizeof(all_b64_decoded)); + ck_assert_str_eq(buff, all_b64); +} +END_TEST + +/******************************************************************************/ +START_TEST(test_b64_all_invalid) +{ + char buff[256]; + size_t actual_len; + char valid[256] = {0}; + char encoded[5] = { "T0sh" }; /* Decodes to 'OK!' */ + int result; + + /* Make a note of all the valid b64 characters */ + unsigned int i = 0; + unsigned char c; + while ((c = all_b64[i]) != '\0') + { + valid[c] = 1; + ++i; + } + + /* Check the decoder's working on a simple string...*/ + result = base64_decode(encoded, buff, sizeof(buff), &actual_len); + ck_assert_int_eq(result, 0); + ck_assert_int_eq(actual_len, 3); + buff[actual_len] = '\0'; + ck_assert_str_eq(buff, "OK!"); + + /* Now replace the 1st character with all invalid characters in turn, + * and check they're rejected */ + for (i = 0 ; i < 256; ++i) + { + if (i != '\0' && !valid[i]) /* Don't try the string terminator char! */ + { + encoded[0] = i; + result = base64_decode(encoded, buff, sizeof(buff), &actual_len); + if (result == 0) + { + ck_abort_msg("Character 0x%02x was not rejected", i); + } + } + } +} +END_TEST + +/******************************************************************************/ +START_TEST(test_b64_small_buffer_encode) +{ + char buff[10 * 4 + 1]; /* Enough space for 10 quanta */ + + size_t result; + + result = base64_encode(all_b64_decoded, sizeof(all_b64_decoded), + buff, sizeof(buff)); + /* Should have read 10 lots of 24 bits from the input */ + ck_assert_int_eq(result, 10 * 3); + ck_assert_str_eq(buff, "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmn"); +} +END_TEST + +/******************************************************************************/ +START_TEST(test_b64_small_buffer_decode) +{ + char buff[10]; /* Enough space for 10 chars */ + + size_t actual_len; + int result; + char *str_result; + char *str_expected; + + result = base64_decode(all_b64, buff, sizeof(buff), &actual_len); + ck_assert_int_eq(result, 0); + ck_assert_int_eq(actual_len, sizeof(all_b64_decoded)); + str_result = bin_to_hex(buff, sizeof(buff)); + str_expected = bin_to_hex(all_b64_decoded, sizeof(buff)); + ck_assert_str_eq(str_result, str_expected); + free(str_result); + free(str_expected); +} +END_TEST + +/******************************************************************************/ +START_TEST(test_b64_add_pad_one) +{ + /* Check that a missing trailing '=' is added when decoding */ + test_rfc4648_from_b64_text("Zm8", "fo"); /* RFC4648 example 3 */ +} +END_TEST + +/******************************************************************************/ +START_TEST(test_b64_add_pad_two) +{ + /* Check that two missing trailing '=' chars are added when decoding */ + test_rfc4648_from_b64_text("Zg", "f"); /* RFC4648 example 2 */ +} +END_TEST + +/******************************************************************************/ +START_TEST(test_b64_bad_pad) +{ + char buff[16]; + size_t actual_len; + + /* Check all bad quanta with padding chars */ + static const char *bad_pad[] = + { + "=AAA", + "A=AA", + "AA=A", + "==AA", + "=A=A", + "=AA=", + "A==A", + "A=A=", + "===A", + "A===", + NULL + }; + const char **p; + + for (p = bad_pad ; *p != NULL ; ++p) + { + int result = base64_decode(*p, buff, sizeof(buff), &actual_len); + if (result == 0) + { + ck_abort_msg("Padding '%s' was not rejected", *p); + } + } +} +END_TEST + +/******************************************************************************/ +START_TEST(test_b64_concat_pad) +{ + const char *src = + "VGVzdA==" /* Test */ + "IA==" /* */ + "Y29uY2F0ZW5hdGVk" /* concatenated */ + "IA==" /* */ + "cGFkZGluZw=="; /*padding */ + const char *expected = "Test concatenated padding"; + char buff[64]; + size_t actual_len; + int result; + + result = base64_decode(src, buff, sizeof(buff), &actual_len); + ck_assert_int_eq(result, 0); + ck_assert_int_eq(actual_len, g_strlen(expected)); + buff[actual_len] = '\0'; + ck_assert_str_eq(buff, expected); +} +END_TEST + +/******************************************************************************/ +Suite * +make_suite_test_base64(void) +{ + Suite *s; + TCase *tc_b64; + + s = suite_create("base64"); + + tc_b64 = tcase_create("base64"); + suite_add_tcase(s, tc_b64); + tcase_add_test(tc_b64, test_b64_rfc4648_ex1_to); + tcase_add_test(tc_b64, test_b64_rfc4648_ex1_from); + tcase_add_test(tc_b64, test_b64_rfc4648_ex2_to); + tcase_add_test(tc_b64, test_b64_rfc4648_ex2_from); + tcase_add_test(tc_b64, test_b64_rfc4648_ex3_to); + tcase_add_test(tc_b64, test_b64_rfc4648_ex3_from); + tcase_add_test(tc_b64, test_b64_rfc4648_ex4_to); + tcase_add_test(tc_b64, test_b64_rfc4648_ex4_from); + tcase_add_test(tc_b64, test_b64_rfc4648_ex5_to); + tcase_add_test(tc_b64, test_b64_rfc4648_ex5_from); + tcase_add_test(tc_b64, test_b64_rfc4648_ex6_to); + tcase_add_test(tc_b64, test_b64_rfc4648_ex6_from); + tcase_add_test(tc_b64, test_b64_rfc4648_ex7_to); + tcase_add_test(tc_b64, test_b64_rfc4648_ex7_from); + tcase_add_test(tc_b64, test_b64_all_valid_from); + tcase_add_test(tc_b64, test_b64_all_valid_to); + tcase_add_test(tc_b64, test_b64_all_invalid); + tcase_add_test(tc_b64, test_b64_small_buffer_encode); + tcase_add_test(tc_b64, test_b64_small_buffer_decode); + tcase_add_test(tc_b64, test_b64_add_pad_one); + tcase_add_test(tc_b64, test_b64_add_pad_two); + tcase_add_test(tc_b64, test_b64_bad_pad); + tcase_add_test(tc_b64, test_b64_concat_pad); + + return s; +} diff --git a/tests/common/test_common.h b/tests/common/test_common.h index 3493c3bf..ed422ab4 100644 --- a/tests/common/test_common.h +++ b/tests/common/test_common.h @@ -4,8 +4,12 @@ #include +char * +bin_to_hex(const char *input, int length); + 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); #endif /* TEST_COMMON_H */ diff --git a/tests/common/test_common_main.c b/tests/common/test_common_main.c index 2674bea0..a08bc7f7 100644 --- a/tests/common/test_common_main.c +++ b/tests/common/test_common_main.c @@ -10,6 +10,36 @@ #include "test_common.h" +/** + * Converts a binary buffer to a hexadecimal representation + * + * Result must be free'd after use + */ +char * +bin_to_hex(const char *input, int length) +{ + int i; + char *result = (char *)malloc(length * 2 + 1); + if (result != NULL) + { + char *p = result; + const char *hexdigit = "0123456789abcdef"; + for (i = 0 ; i < length ; ++i) + { + int c = input[i]; + if (c < 0) + { + c += 256; + } + *p++ = hexdigit[ c / 16]; + *p++ = hexdigit[ c % 16]; + } + *p = '\0'; + } + + return result; +} + int main (void) { int number_failed; @@ -18,6 +48,7 @@ int main (void) sr = srunner_create (make_suite_test_string()); 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_list_suite()); srunner_set_tap(sr, "-"); diff --git a/tests/common/test_ssl_calls.c b/tests/common/test_ssl_calls.c index c5198fd9..56d8935b 100644 --- a/tests/common/test_ssl_calls.c +++ b/tests/common/test_ssl_calls.c @@ -9,32 +9,6 @@ #include "test_common.h" -static -char *bin_to_hex(const char *input, int length) -{ - int i; - char *result = (char *)g_malloc(length * 2 + 1, 0); - if (result != NULL) - { - char *p = result; - const char *hexdigit = "0123456789abcdef"; - for (i = 0 ; i < length ; ++i) - { - int c = input[i]; - if (c < 0) - { - c += 256; - } - *p++ = hexdigit[ c / 16]; - *p++ = hexdigit[ c % 16]; - } - *p = '\0'; - } - - return result; -} - - START_TEST(test_rc4_enc_ok) { const char *key = "16_byte_key-----"; diff --git a/xrdp/xrdp_login_wnd.c b/xrdp/xrdp_login_wnd.c index a1d7e9a5..da5917b2 100644 --- a/xrdp/xrdp_login_wnd.c +++ b/xrdp/xrdp_login_wnd.c @@ -346,6 +346,7 @@ xrdp_wm_show_edits(struct xrdp_wm *self, struct xrdp_bitmap *combo) struct xrdp_cfg_globals *globals; char resultIP[256]; char *plain; /* base64 decoded string */ + size_t plain_length; /* length of decoded base64 string */ size_t base64_length; /* length of base64 string */ globals = &self->xrdp_config->cfg_globals; @@ -379,8 +380,9 @@ xrdp_wm_show_edits(struct xrdp_wm *self, struct xrdp_bitmap *combo) { base64_length = g_strlen(value + BASE64PREFIX_LEN); plain = (char *)g_malloc(base64_length, 0); - base64_decode(plain, value + BASE64PREFIX_LEN, base64_length); - g_strncpy(value, plain, g_strlen(plain)); + base64_decode(value + BASE64PREFIX_LEN, + plain, base64_length, &plain_length); + g_strncpy(value, plain, plain_length); g_free(plain); } else if (g_strncmp(ASK, value, ASK_LEN) == 0) @@ -421,7 +423,9 @@ xrdp_wm_show_edits(struct xrdp_wm *self, struct xrdp_bitmap *combo) { base64_length = g_strlen(value + ASK_LEN + BASE64PREFIX_LEN); plain = (char *)g_malloc(base64_length, 0); - base64_decode(plain, value + ASK_LEN + BASE64PREFIX_LEN, base64_length); + base64_decode(value + ASK_LEN + BASE64PREFIX_LEN, + plain, base64_length, &plain_length); + plain[plain_length] = '\0'; g_strncpy(b->caption1, plain, 255); g_free(plain); }