diff mbox series

[Openvpn-devel,M] Change in openvpn[master]: test_user_pass: Check fatal errors for empty username/password

Message ID c36ddcd3c6495a143a3f8aee876a1414868c474a-HTML@gerrit.openvpn.net
State Superseded
Headers show
Series [Openvpn-devel,M] Change in openvpn[master]: test_user_pass: Check fatal errors for empty username/password | expand

Commit Message

flichtenheld (Code Review) Dec. 8, 2023, 4:29 p.m. UTC 
Attention is currently required from: plaisthos.

Hello plaisthos,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/474?usp=email

to review the following change.


Change subject: test_user_pass: Check fatal errors for empty username/password
......................................................................

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.

Change-Id: Icabc8acf75638c86c8c395e9ffecba7a7226cd97
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
---
A tests/unit_tests/openvpn/input/appears_empty.txt
A tests/unit_tests/openvpn/input/empty.txt
M tests/unit_tests/openvpn/mock_msg.c
M tests/unit_tests/openvpn/test_user_pass.c
4 files changed, 68 insertions(+), 2 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/74/474/1
diff mbox series

Patch

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 0000000..ffb749a
--- /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 0000000..e69de29
--- /dev/null
+++ b/tests/unit_tests/openvpn/input/empty.txt
diff --git a/tests/unit_tests/openvpn/mock_msg.c b/tests/unit_tests/openvpn/mock_msg.c
index d74efaa..2fcad9d 100644
--- a/tests/unit_tests/openvpn/mock_msg.c
+++ b/tests/unit_tests/openvpn/mock_msg.c
@@ -38,7 +38,6 @@ 
 #include "error.h"
 
 unsigned int x_debug_level = 0; /* Default to (almost) no debugging output */
-bool fatal_error_triggered = false;
 
 void
 mock_set_debug_level(int level)
@@ -58,11 +57,14 @@ 
 {
     if (flags & M_FATAL)
     {
-        fatal_error_triggered = true;
         printf("FATAL ERROR:");
     }
     vprintf(format, arglist);
     printf("\n");
+    if (flags & M_FATAL)
+    {
+        mock_assert(false, "FATAL ERROR", __FILE__, __LINE__);
+    }
 }
 
 void
diff --git a/tests/unit_tests/openvpn/test_user_pass.c b/tests/unit_tests/openvpn/test_user_pass.c
index e468d3f..600ec80 100644
--- a/tests/unit_tests/openvpn/test_user_pass.c
+++ b/tests/unit_tests/openvpn/test_user_pass.c
@@ -172,6 +172,24 @@ 
 
     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);
+
+    /* Test Assertion */
+    /*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));
+
+    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);
@@ -223,6 +241,15 @@ 
 
     reset_user_pass(&up);
 
+    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));
+
+    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, "cpassword");
@@ -231,6 +258,18 @@ 
     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, "");
 }
 
 static void
@@ -254,6 +293,17 @@ 
 
     reset_user_pass(&up);
 
+    snprintf(authfile, PATH_MAX, "%s/%s", srcdir, "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);
+
     snprintf(authfile, PATH_MAX, "%s/%s", srcdir, "input/user_only.txt");
     expect_string(query_user_exec_builtin, query_user[i].prompt, "Enter UT Password:");
     will_return(query_user_exec_builtin, "cpassword");
@@ -266,6 +316,11 @@ 
 
     reset_user_pass(&up);
 
+    snprintf(authfile, PATH_MAX, "%s/%s", srcdir, "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;
     snprintf(authfile, PATH_MAX, "%s/%s", srcdir, "input/user_only.txt");
     /*FIXME: query_user_exec() called even though nothing queued */
@@ -274,6 +329,12 @@ 
     assert_true(up.defined);
     assert_string_equal(up.username, "user");
     assert_string_equal(up.password, "fuser");
+
+    reset_user_pass(&up);
+
+    flags |= GET_USER_PASS_PASSWORD_ONLY;
+    snprintf(authfile, PATH_MAX, "%s/%s", srcdir, "input/empty.txt");
+    expect_assert_failure(get_user_pass_cr(&up, authfile, "UT", flags, NULL));
 }
 
 const struct CMUnitTest user_pass_tests[] = {