Browse code

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 <frank@lichtenheld.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
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 <gert@greenie.muc.de>

Frank Lichtenheld authored on 2025/10/11 06:11:47
Showing 7 changed files
... ...
@@ -729,7 +729,7 @@ if (BUILD_TESTING)
729 729
             add_test(${test_name} ${test_name})
730 730
             # for compat with autotools make check
731 731
             set_tests_properties(${test_name} PROPERTIES
732
-                ENVIRONMENT "srcdir=${_UT_SOURCE_DIR}")
732
+                ENVIRONMENT "srcdir=${_UT_SOURCE_DIR};LSAN_OPTIONS=suppressions=${_UT_SOURCE_DIR}/input/leak_suppr.txt")
733 733
         endif ()
734 734
         add_executable(${test_name}
735 735
             tests/unit_tests/openvpn/${test_name}.c
... ...
@@ -4,6 +4,8 @@ EXTRA_DIST = input
4 4
 
5 5
 AM_TESTSUITE_SUMMARY_HEADER = ' for $(PACKAGE_STRING) Unit-Tests'
6 6
 
7
+AM_TESTS_ENVIRONMENT = export LSAN_OPTIONS=suppressions=$(srcdir)/input/leak_suppr.txt;
8
+
7 9
 test_binaries = argv_testdriver buffer_testdriver crypto_testdriver packet_id_testdriver auth_token_testdriver \
8 10
 	ncp_testdriver misc_testdriver options_parse_testdriver pkt_testdriver ssl_testdriver \
9 11
 	user_pass_testdriver push_update_msg_testdriver provider_testdriver socket_testdriver
10 12
new file mode 100644
... ...
@@ -0,0 +1,3 @@
0
+	
1
+	
2
+(contains \t\n\t\n)
0 3
new file mode 100644
1 4
new file mode 100644
... ...
@@ -0,0 +1 @@
0
+leak:_assertions$
... ...
@@ -41,7 +41,6 @@
41 41
 msglvl_t x_debug_level = 0; /* Default to (almost) no debugging output */
42 42
 msglvl_t print_x_debug_level = 0;
43 43
 
44
-bool fatal_error_triggered = false;
45 44
 
46 45
 char mock_msg_buf[MOCK_MSG_BUF];
47 46
 
... ...
@@ -75,7 +74,6 @@ x_msg_va(const msglvl_t flags, const char *format, va_list arglist)
75 75
 {
76 76
     if (flags & M_FATAL)
77 77
     {
78
-        fatal_error_triggered = true;
79 78
         printf("FATAL ERROR:");
80 79
     }
81 80
     CLEAR(mock_msg_buf);
... ...
@@ -86,6 +84,12 @@ x_msg_va(const msglvl_t flags, const char *format, va_list arglist)
86 86
         printf("%s", mock_msg_buf);
87 87
         printf("\n");
88 88
     }
89
+#ifndef NO_CMOCKA
90
+    if (flags & M_FATAL)
91
+    {
92
+        mock_assert(false, "FATAL ERROR", __FILE__, __LINE__);
93
+    }
94
+#endif
89 95
 }
90 96
 
91 97
 void
... ...
@@ -180,6 +180,16 @@ test_get_user_pass_inline_creds(void **state)
180 180
 
181 181
     reset_user_pass(&up);
182 182
 
183
+    /*FIXME: query_user_exec() called even though nothing queued */
184
+    will_return(query_user_exec_builtin, true);
185
+    /*FIXME: silently removes control characters but does not error out */
186
+    assert_true(get_user_pass_cr(&up, "\t\n\t", "UT", flags, NULL));
187
+    assert_true(up.defined);
188
+    assert_string_equal(up.username, "");
189
+    assert_string_equal(up.password, "");
190
+
191
+    reset_user_pass(&up);
192
+
183 193
     expect_string(query_user_exec_builtin, query_user[i].prompt, "Enter UT Password:");
184 194
     will_return(query_user_exec_builtin, "cpassword");
185 195
     will_return(query_user_exec_builtin, true);
... ...
@@ -212,6 +222,25 @@ test_get_user_pass_inline_creds(void **state)
212 212
     assert_string_equal(up.password, "cpassword");
213 213
 }
214 214
 
215
+/* NOTE: expect_assert_failure does not seem to work with MSVC */
216
+#ifndef _MSC_VER
217
+/* NOTE: leaks gc memory */
218
+static void
219
+test_get_user_pass_inline_creds_assertions(void **state)
220
+{
221
+    struct user_pass up = { 0 };
222
+    reset_user_pass(&up);
223
+    unsigned int flags = GET_USER_PASS_INLINE_CREDS;
224
+
225
+    reset_user_pass(&up);
226
+
227
+    /*FIXME: query_user_exec() called even though nothing queued */
228
+    /*FIXME? username error thrown very late in stdin handling */
229
+    will_return(query_user_exec_builtin, true);
230
+    expect_assert_failure(get_user_pass_cr(&up, "\nipassword\n", "UT", flags, NULL));
231
+}
232
+#endif
233
+
215 234
 static void
216 235
 test_get_user_pass_authfile_stdin(void **state)
217 236
 {
... ...
@@ -239,8 +268,39 @@ test_get_user_pass_authfile_stdin(void **state)
239 239
     assert_true(up.defined);
240 240
     assert_string_equal(up.username, "user");
241 241
     assert_string_equal(up.password, "cpassword");
242
+
243
+    reset_user_pass(&up);
244
+
245
+    flags |= GET_USER_PASS_PASSWORD_ONLY;
246
+    expect_string(query_user_exec_builtin, query_user[i].prompt, "Enter UT Password:");
247
+    will_return(query_user_exec_builtin, "");
248
+    will_return(query_user_exec_builtin, true);
249
+    /*FIXME? does not error out on empty password */
250
+    assert_true(get_user_pass_cr(&up, "stdin", "UT", flags, NULL));
251
+    assert_true(up.defined);
252
+    assert_string_equal(up.username, "user");
253
+    assert_string_equal(up.password, "");
242 254
 }
243 255
 
256
+/* NOTE: expect_assert_failure does not seem to work with MSVC */
257
+#ifndef _MSC_VER
258
+/* NOTE: leaks gc memory */
259
+static void
260
+test_get_user_pass_authfile_stdin_assertions(void **state)
261
+{
262
+    struct user_pass up = { 0 };
263
+    reset_user_pass(&up);
264
+    unsigned int flags = 0;
265
+
266
+    expect_string(query_user_exec_builtin, query_user[i].prompt, "Enter UT Username:");
267
+    expect_string(query_user_exec_builtin, query_user[i].prompt, "Enter UT Password:");
268
+    will_return(query_user_exec_builtin, "");
269
+    will_return(query_user_exec_builtin, "cpassword");
270
+    will_return(query_user_exec_builtin, true);
271
+    expect_assert_failure(get_user_pass_cr(&up, "stdin", "UT", flags, NULL));
272
+}
273
+#endif
274
+
244 275
 static void
245 276
 test_get_user_pass_authfile_file(void **state)
246 277
 {
... ...
@@ -260,6 +320,17 @@ test_get_user_pass_authfile_file(void **state)
260 260
 
261 261
     reset_user_pass(&up);
262 262
 
263
+    openvpn_test_get_srcdir_dir(authfile, PATH_MAX, "input/appears_empty.txt");
264
+    /*FIXME: query_user_exec() called even though nothing queued */
265
+    will_return(query_user_exec_builtin, true);
266
+    /*FIXME? does not error out */
267
+    assert_true(get_user_pass_cr(&up, authfile, "UT", flags, NULL));
268
+    assert_true(up.defined);
269
+    assert_string_equal(up.username, "");
270
+    assert_string_equal(up.password, "");
271
+
272
+    reset_user_pass(&up);
273
+
263 274
     openvpn_test_get_srcdir_dir(authfile, PATH_MAX, "input/user_only.txt");
264 275
     expect_string(query_user_exec_builtin, query_user[i].prompt, "Enter UT Password:");
265 276
     will_return(query_user_exec_builtin, "cpassword");
... ...
@@ -361,6 +432,29 @@ test_get_user_pass_static_challenge(void **state)
361 361
 }
362 362
 #endif /* ENABLE_MANAGEMENT */
363 363
 
364
+/* NOTE: expect_assert_failure does not seem to work with MSVC */
365
+#ifndef _MSC_VER
366
+/* NOTE: leaks gc memory */
367
+static void
368
+test_get_user_pass_authfile_file_assertions(void **state)
369
+{
370
+    struct user_pass up = { 0 };
371
+    reset_user_pass(&up);
372
+    unsigned int flags = 0;
373
+
374
+    char authfile[PATH_MAX] = { 0 };
375
+
376
+    openvpn_test_get_srcdir_dir(authfile, PATH_MAX, "input/empty.txt");
377
+    expect_assert_failure(get_user_pass_cr(&up, authfile, "UT", flags, NULL));
378
+
379
+    reset_user_pass(&up);
380
+
381
+    flags |= GET_USER_PASS_PASSWORD_ONLY;
382
+    openvpn_test_get_srcdir_dir(authfile, PATH_MAX, "input/empty.txt");
383
+    expect_assert_failure(get_user_pass_cr(&up, authfile, "UT", flags, NULL));
384
+}
385
+#endif /* ifndef _MSC_VER */
386
+
364 387
 const struct CMUnitTest user_pass_tests[] = {
365 388
     cmocka_unit_test(test_get_user_pass_defined),
366 389
     cmocka_unit_test(test_get_user_pass_needok),
... ...
@@ -371,6 +465,11 @@ const struct CMUnitTest user_pass_tests[] = {
371 371
     cmocka_unit_test(test_get_user_pass_dynamic_challenge),
372 372
     cmocka_unit_test(test_get_user_pass_static_challenge),
373 373
 #endif /* ENABLE_MANAGEMENT */
374
+#ifndef _MSC_VER
375
+    cmocka_unit_test(test_get_user_pass_inline_creds_assertions),
376
+    cmocka_unit_test(test_get_user_pass_authfile_stdin_assertions),
377
+    cmocka_unit_test(test_get_user_pass_authfile_file_assertions),
378
+#endif
374 379
 };
375 380
 
376 381
 int