From e7fdde7cb8d39a51a4a38b0d16f23a18d086cfe2 Mon Sep 17 00:00:00 2001 From: Frank Lichtenheld Date: Fri, 10 Oct 2025 23:11:47 +0200 Subject: [PATCH] test_user_pass: Check fatal errors for empty username/password Required a fix to mock_msg to make tests of M_FATAL possible at all. This also tests some cases which arguably should throw a fatal error but do not. v2: - Suppress LeakSanitizer errors for fatal error tests. Due to aborting the function, the memory will not be cleaned up, but that is expected. v3: - Disable assert tests with MSVC. Does not seem to catch the error correctly. - Rebase on top of parallel-tests series to get AM_TESTS_ENVIRONMENT. v8: - Update srcdir handling according to master. v10: - Update mock_msg.c fatal handling to be compatible with NO_CMOCKA. Change-Id: Icabc8acf75638c86c8c395e9ffecba7a7226cd97 Signed-off-by: Frank Lichtenheld Acked-by: Gert Doering Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/474 Message-Id: <20251010211154.2780-1-gert@greenie.muc.de> URL: https://sourceforge.net/p/openvpn/mailman/message/59245149/ Signed-off-by: Gert Doering --- CMakeLists.txt | 2 +- tests/unit_tests/openvpn/Makefile.am | 2 + .../openvpn/input/appears_empty.txt | 3 + tests/unit_tests/openvpn/input/empty.txt | 0 tests/unit_tests/openvpn/input/leak_suppr.txt | 1 + tests/unit_tests/openvpn/mock_msg.c | 8 +- tests/unit_tests/openvpn/test_user_pass.c | 99 +++++++++++++++++++ 7 files changed, 112 insertions(+), 3 deletions(-) create mode 100644 tests/unit_tests/openvpn/input/appears_empty.txt create mode 100644 tests/unit_tests/openvpn/input/empty.txt create mode 100644 tests/unit_tests/openvpn/input/leak_suppr.txt diff --git a/CMakeLists.txt b/CMakeLists.txt index 9511bda4b..9e0b46eaa 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -729,7 +729,7 @@ if (BUILD_TESTING) add_test(${test_name} ${test_name}) # for compat with autotools make check set_tests_properties(${test_name} PROPERTIES - ENVIRONMENT "srcdir=${_UT_SOURCE_DIR}") + ENVIRONMENT "srcdir=${_UT_SOURCE_DIR};LSAN_OPTIONS=suppressions=${_UT_SOURCE_DIR}/input/leak_suppr.txt") endif () add_executable(${test_name} tests/unit_tests/openvpn/${test_name}.c diff --git a/tests/unit_tests/openvpn/Makefile.am b/tests/unit_tests/openvpn/Makefile.am index 05c0ea536..50f4a1109 100644 --- a/tests/unit_tests/openvpn/Makefile.am +++ b/tests/unit_tests/openvpn/Makefile.am @@ -4,6 +4,8 @@ EXTRA_DIST = input AM_TESTSUITE_SUMMARY_HEADER = ' for $(PACKAGE_STRING) Unit-Tests' +AM_TESTS_ENVIRONMENT = export LSAN_OPTIONS=suppressions=$(srcdir)/input/leak_suppr.txt; + test_binaries = argv_testdriver buffer_testdriver crypto_testdriver packet_id_testdriver auth_token_testdriver \ ncp_testdriver misc_testdriver options_parse_testdriver pkt_testdriver ssl_testdriver \ user_pass_testdriver push_update_msg_testdriver provider_testdriver socket_testdriver diff --git a/tests/unit_tests/openvpn/input/appears_empty.txt b/tests/unit_tests/openvpn/input/appears_empty.txt new file mode 100644 index 000000000..ffb749a63 --- /dev/null +++ b/tests/unit_tests/openvpn/input/appears_empty.txt @@ -0,0 +1,3 @@ + + +(contains \t\n\t\n) diff --git a/tests/unit_tests/openvpn/input/empty.txt b/tests/unit_tests/openvpn/input/empty.txt new file mode 100644 index 000000000..e69de29bb diff --git a/tests/unit_tests/openvpn/input/leak_suppr.txt b/tests/unit_tests/openvpn/input/leak_suppr.txt new file mode 100644 index 000000000..72ebfe08d --- /dev/null +++ b/tests/unit_tests/openvpn/input/leak_suppr.txt @@ -0,0 +1 @@ +leak:_assertions$ diff --git a/tests/unit_tests/openvpn/mock_msg.c b/tests/unit_tests/openvpn/mock_msg.c index a2aa3f9c5..2cd133662 100644 --- a/tests/unit_tests/openvpn/mock_msg.c +++ b/tests/unit_tests/openvpn/mock_msg.c @@ -41,7 +41,6 @@ msglvl_t x_debug_level = 0; /* Default to (almost) no debugging output */ msglvl_t print_x_debug_level = 0; -bool fatal_error_triggered = false; char mock_msg_buf[MOCK_MSG_BUF]; @@ -75,7 +74,6 @@ x_msg_va(const msglvl_t flags, const char *format, va_list arglist) { if (flags & M_FATAL) { - fatal_error_triggered = true; printf("FATAL ERROR:"); } CLEAR(mock_msg_buf); @@ -86,6 +84,12 @@ x_msg_va(const msglvl_t flags, const char *format, va_list arglist) printf("%s", mock_msg_buf); printf("\n"); } +#ifndef NO_CMOCKA + if (flags & M_FATAL) + { + mock_assert(false, "FATAL ERROR", __FILE__, __LINE__); + } +#endif } void diff --git a/tests/unit_tests/openvpn/test_user_pass.c b/tests/unit_tests/openvpn/test_user_pass.c index 32ec59f1c..f5dddd654 100644 --- a/tests/unit_tests/openvpn/test_user_pass.c +++ b/tests/unit_tests/openvpn/test_user_pass.c @@ -180,6 +180,16 @@ test_get_user_pass_inline_creds(void **state) reset_user_pass(&up); + /*FIXME: query_user_exec() called even though nothing queued */ + will_return(query_user_exec_builtin, true); + /*FIXME: silently removes control characters but does not error out */ + assert_true(get_user_pass_cr(&up, "\t\n\t", "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, ""); + assert_string_equal(up.password, ""); + + reset_user_pass(&up); + expect_string(query_user_exec_builtin, query_user[i].prompt, "Enter UT Password:"); will_return(query_user_exec_builtin, "cpassword"); will_return(query_user_exec_builtin, true); @@ -212,6 +222,25 @@ test_get_user_pass_inline_creds(void **state) assert_string_equal(up.password, "cpassword"); } +/* NOTE: expect_assert_failure does not seem to work with MSVC */ +#ifndef _MSC_VER +/* NOTE: leaks gc memory */ +static void +test_get_user_pass_inline_creds_assertions(void **state) +{ + struct user_pass up = { 0 }; + reset_user_pass(&up); + unsigned int flags = GET_USER_PASS_INLINE_CREDS; + + reset_user_pass(&up); + + /*FIXME: query_user_exec() called even though nothing queued */ + /*FIXME? username error thrown very late in stdin handling */ + will_return(query_user_exec_builtin, true); + expect_assert_failure(get_user_pass_cr(&up, "\nipassword\n", "UT", flags, NULL)); +} +#endif + static void test_get_user_pass_authfile_stdin(void **state) { @@ -239,8 +268,39 @@ test_get_user_pass_authfile_stdin(void **state) assert_true(up.defined); assert_string_equal(up.username, "user"); assert_string_equal(up.password, "cpassword"); + + reset_user_pass(&up); + + flags |= GET_USER_PASS_PASSWORD_ONLY; + expect_string(query_user_exec_builtin, query_user[i].prompt, "Enter UT Password:"); + will_return(query_user_exec_builtin, ""); + will_return(query_user_exec_builtin, true); + /*FIXME? does not error out on empty password */ + assert_true(get_user_pass_cr(&up, "stdin", "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, "user"); + assert_string_equal(up.password, ""); } +/* NOTE: expect_assert_failure does not seem to work with MSVC */ +#ifndef _MSC_VER +/* NOTE: leaks gc memory */ +static void +test_get_user_pass_authfile_stdin_assertions(void **state) +{ + struct user_pass up = { 0 }; + reset_user_pass(&up); + unsigned int flags = 0; + + expect_string(query_user_exec_builtin, query_user[i].prompt, "Enter UT Username:"); + expect_string(query_user_exec_builtin, query_user[i].prompt, "Enter UT Password:"); + will_return(query_user_exec_builtin, ""); + will_return(query_user_exec_builtin, "cpassword"); + will_return(query_user_exec_builtin, true); + expect_assert_failure(get_user_pass_cr(&up, "stdin", "UT", flags, NULL)); +} +#endif + static void test_get_user_pass_authfile_file(void **state) { @@ -260,6 +320,17 @@ test_get_user_pass_authfile_file(void **state) reset_user_pass(&up); + openvpn_test_get_srcdir_dir(authfile, PATH_MAX, "input/appears_empty.txt"); + /*FIXME: query_user_exec() called even though nothing queued */ + will_return(query_user_exec_builtin, true); + /*FIXME? does not error out */ + assert_true(get_user_pass_cr(&up, authfile, "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, ""); + assert_string_equal(up.password, ""); + + reset_user_pass(&up); + openvpn_test_get_srcdir_dir(authfile, PATH_MAX, "input/user_only.txt"); expect_string(query_user_exec_builtin, query_user[i].prompt, "Enter UT Password:"); will_return(query_user_exec_builtin, "cpassword"); @@ -361,6 +432,29 @@ test_get_user_pass_static_challenge(void **state) } #endif /* ENABLE_MANAGEMENT */ +/* NOTE: expect_assert_failure does not seem to work with MSVC */ +#ifndef _MSC_VER +/* NOTE: leaks gc memory */ +static void +test_get_user_pass_authfile_file_assertions(void **state) +{ + struct user_pass up = { 0 }; + reset_user_pass(&up); + unsigned int flags = 0; + + char authfile[PATH_MAX] = { 0 }; + + openvpn_test_get_srcdir_dir(authfile, PATH_MAX, "input/empty.txt"); + expect_assert_failure(get_user_pass_cr(&up, authfile, "UT", flags, NULL)); + + reset_user_pass(&up); + + flags |= GET_USER_PASS_PASSWORD_ONLY; + openvpn_test_get_srcdir_dir(authfile, PATH_MAX, "input/empty.txt"); + expect_assert_failure(get_user_pass_cr(&up, authfile, "UT", flags, NULL)); +} +#endif /* ifndef _MSC_VER */ + const struct CMUnitTest user_pass_tests[] = { cmocka_unit_test(test_get_user_pass_defined), cmocka_unit_test(test_get_user_pass_needok), @@ -371,6 +465,11 @@ const struct CMUnitTest user_pass_tests[] = { cmocka_unit_test(test_get_user_pass_dynamic_challenge), cmocka_unit_test(test_get_user_pass_static_challenge), #endif /* ENABLE_MANAGEMENT */ +#ifndef _MSC_VER + cmocka_unit_test(test_get_user_pass_inline_creds_assertions), + cmocka_unit_test(test_get_user_pass_authfile_stdin_assertions), + cmocka_unit_test(test_get_user_pass_authfile_file_assertions), +#endif }; int -- 2.47.3