[Openvpn-devel] [PATCH v8] test_user_pass: add basic tests for static/dynamic challenges

2024-02-07 Thread Frank Lichtenheld 
Change-Id: I8b5570f6314e917f92dce072279efe415d79b22a
Signed-off-by: Frank Lichtenheld 
Acked-by: Arne Schwabe 
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/475
This mail reflects revision 8 of this Change.
Acked-by according to Gerrit (reflected above):
Arne Schwabe 


diff --git a/tests/unit_tests/openvpn/test_user_pass.c 
b/tests/unit_tests/openvpn/test_user_pass.c
index bd4eb1f..5d3f9b6 100644
--- a/tests/unit_tests/openvpn/test_user_pass.c
+++ b/tests/unit_tests/openvpn/test_user_pass.c
@@ -267,12 +267,73 @@
 assert_string_equal(up.password, "fuser");
 }
 
+#ifdef ENABLE_MANAGEMENT
+static void
+test_get_user_pass_dynamic_challenge(void **state)
+{
+struct user_pass up = { 0 };
+reset_user_pass();
+const char *challenge = 
"CRV1:R,E:Om01u7Fh4LrGBS7uh0SWmzwabUiGiW6l:Y3Ix:Please enter token PIN";
+unsigned int flags = GET_USER_PASS_DYNAMIC_CHALLENGE;
+
+expect_string(query_user_exec_builtin, query_user[i].prompt, "CHALLENGE: 
Please enter token PIN");
+will_return(query_user_exec_builtin, "challenge_response");
+will_return(query_user_exec_builtin, true);
+assert_true(get_user_pass_cr(, NULL, "UT", flags, challenge));
+assert_true(up.defined);
+assert_string_equal(up.username, "cr1");
+assert_string_equal(up.password, 
"CRV1::Om01u7Fh4LrGBS7uh0SWmzwabUiGiW6l::challenge_response");
+}
+
+static void
+test_get_user_pass_static_challenge(void **state)
+{
+struct user_pass up = { 0 };
+reset_user_pass();
+const char *challenge = "Please enter token PIN";
+unsigned int flags = GET_USER_PASS_STATIC_CHALLENGE;
+
+expect_string(query_user_exec_builtin, query_user[i].prompt, "Enter UT 
Username:");
+will_return(query_user_exec_builtin, "cuser");
+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);
+expect_string(query_user_exec_builtin, query_user[i].prompt, "CHALLENGE: 
Please enter token PIN");
+will_return(query_user_exec_builtin, "challenge_response");
+will_return(query_user_exec_builtin, true);
+assert_true(get_user_pass_cr(, NULL, "UT", flags, challenge));
+assert_true(up.defined);
+assert_string_equal(up.username, "cuser");
+/* SCRV1:cpassword:challenge_response but base64-encoded */
+assert_string_equal(up.password, 
"SCRV1:Y3Bhc3N3b3Jk:Y2hhbGxlbmdlX3Jlc3BvbnNl");
+
+reset_user_pass();
+
+flags |= GET_USER_PASS_INLINE_CREDS;
+
+/*FIXME: query_user_exec() called even though nothing queued */
+will_return(query_user_exec_builtin, true);
+expect_string(query_user_exec_builtin, query_user[i].prompt, "CHALLENGE: 
Please enter token PIN");
+will_return(query_user_exec_builtin, "challenge_response");
+will_return(query_user_exec_builtin, true);
+assert_true(get_user_pass_cr(, "iuser\nipassword", "UT", flags, 
challenge));
+assert_true(up.defined);
+assert_string_equal(up.username, "iuser");
+/* SCRV1:ipassword:challenge_response but base64-encoded */
+assert_string_equal(up.password, 
"SCRV1:aXBhc3N3b3Jk:Y2hhbGxlbmdlX3Jlc3BvbnNl");
+}
+#endif /* ENABLE_MANAGEMENT */
+
 const struct CMUnitTest user_pass_tests[] = {
 cmocka_unit_test(test_get_user_pass_defined),
 cmocka_unit_test(test_get_user_pass_needok),
 cmocka_unit_test(test_get_user_pass_inline_creds),
 cmocka_unit_test(test_get_user_pass_authfile_stdin),
 cmocka_unit_test(test_get_user_pass_authfile_file),
+#ifdef ENABLE_MANAGEMENT
+cmocka_unit_test(test_get_user_pass_dynamic_challenge),
+cmocka_unit_test(test_get_user_pass_static_challenge),
+#endif /* ENABLE_MANAGEMENT */
 };
 
 int


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [M] Change in openvpn[master]: test_user_pass: add basic tests for static/dynamic challenges

2024-02-07 Thread plaisthos (Code Review) 
Attention is currently required from: flichtenheld.

plaisthos has posted comments on this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/475?usp=email )

Change subject: test_user_pass: add basic tests for static/dynamic challenges
..


Patch Set 8: Code-Review+2


--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/475?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I8b5570f6314e917f92dce072279efe415d79b22a
Gerrit-Change-Number: 475
Gerrit-PatchSet: 8
Gerrit-Owner: flichtenheld 
Gerrit-Reviewer: plaisthos 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: flichtenheld 
Gerrit-Comment-Date: Wed, 07 Feb 2024 15:07:25 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [M] Change in openvpn[master]: test_user_pass: add basic tests for static/dynamic challenges

2024-02-07 Thread flichtenheld (Code Review) 
Attention is currently required from: flichtenheld, plaisthos.

Hello plaisthos,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#8).

The following approvals got outdated and were removed:
Code-Review+2 by plaisthos


Change subject: test_user_pass: add basic tests for static/dynamic challenges
..

test_user_pass: add basic tests for static/dynamic challenges

Change-Id: I8b5570f6314e917f92dce072279efe415d79b22a
Signed-off-by: Frank Lichtenheld 
---
M tests/unit_tests/openvpn/test_user_pass.c
1 file changed, 61 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/75/475/8

diff --git a/tests/unit_tests/openvpn/test_user_pass.c 
b/tests/unit_tests/openvpn/test_user_pass.c
index bd4eb1f..5d3f9b6 100644
--- a/tests/unit_tests/openvpn/test_user_pass.c
+++ b/tests/unit_tests/openvpn/test_user_pass.c
@@ -267,12 +267,73 @@
 assert_string_equal(up.password, "fuser");
 }

+#ifdef ENABLE_MANAGEMENT
+static void
+test_get_user_pass_dynamic_challenge(void **state)
+{
+struct user_pass up = { 0 };
+reset_user_pass();
+const char *challenge = 
"CRV1:R,E:Om01u7Fh4LrGBS7uh0SWmzwabUiGiW6l:Y3Ix:Please enter token PIN";
+unsigned int flags = GET_USER_PASS_DYNAMIC_CHALLENGE;
+
+expect_string(query_user_exec_builtin, query_user[i].prompt, "CHALLENGE: 
Please enter token PIN");
+will_return(query_user_exec_builtin, "challenge_response");
+will_return(query_user_exec_builtin, true);
+assert_true(get_user_pass_cr(, NULL, "UT", flags, challenge));
+assert_true(up.defined);
+assert_string_equal(up.username, "cr1");
+assert_string_equal(up.password, 
"CRV1::Om01u7Fh4LrGBS7uh0SWmzwabUiGiW6l::challenge_response");
+}
+
+static void
+test_get_user_pass_static_challenge(void **state)
+{
+struct user_pass up = { 0 };
+reset_user_pass();
+const char *challenge = "Please enter token PIN";
+unsigned int flags = GET_USER_PASS_STATIC_CHALLENGE;
+
+expect_string(query_user_exec_builtin, query_user[i].prompt, "Enter UT 
Username:");
+will_return(query_user_exec_builtin, "cuser");
+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);
+expect_string(query_user_exec_builtin, query_user[i].prompt, "CHALLENGE: 
Please enter token PIN");
+will_return(query_user_exec_builtin, "challenge_response");
+will_return(query_user_exec_builtin, true);
+assert_true(get_user_pass_cr(, NULL, "UT", flags, challenge));
+assert_true(up.defined);
+assert_string_equal(up.username, "cuser");
+/* SCRV1:cpassword:challenge_response but base64-encoded */
+assert_string_equal(up.password, 
"SCRV1:Y3Bhc3N3b3Jk:Y2hhbGxlbmdlX3Jlc3BvbnNl");
+
+reset_user_pass();
+
+flags |= GET_USER_PASS_INLINE_CREDS;
+
+/*FIXME: query_user_exec() called even though nothing queued */
+will_return(query_user_exec_builtin, true);
+expect_string(query_user_exec_builtin, query_user[i].prompt, "CHALLENGE: 
Please enter token PIN");
+will_return(query_user_exec_builtin, "challenge_response");
+will_return(query_user_exec_builtin, true);
+assert_true(get_user_pass_cr(, "iuser\nipassword", "UT", flags, 
challenge));
+assert_true(up.defined);
+assert_string_equal(up.username, "iuser");
+/* SCRV1:ipassword:challenge_response but base64-encoded */
+assert_string_equal(up.password, 
"SCRV1:aXBhc3N3b3Jk:Y2hhbGxlbmdlX3Jlc3BvbnNl");
+}
+#endif /* ENABLE_MANAGEMENT */
+
 const struct CMUnitTest user_pass_tests[] = {
 cmocka_unit_test(test_get_user_pass_defined),
 cmocka_unit_test(test_get_user_pass_needok),
 cmocka_unit_test(test_get_user_pass_inline_creds),
 cmocka_unit_test(test_get_user_pass_authfile_stdin),
 cmocka_unit_test(test_get_user_pass_authfile_file),
+#ifdef ENABLE_MANAGEMENT
+cmocka_unit_test(test_get_user_pass_dynamic_challenge),
+cmocka_unit_test(test_get_user_pass_static_challenge),
+#endif /* ENABLE_MANAGEMENT */
 };

 int

--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/475?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I8b5570f6314e917f92dce072279efe415d79b22a
Gerrit-Change-Number: 475
Gerrit-PatchSet: 8
Gerrit-Owner: flichtenheld 
Gerrit-Reviewer: plaisthos 
Gerrit-CC: openvpn-devel 
Gerrit-Attention: plaisthos 
Gerrit-Attention: flichtenheld 
Gerrit-MessageType: newpatchset
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

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

2024-02-07 Thread flichtenheld (Code Review) 
Attention is currently required from: plaisthos.

Hello plaisthos,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#8).

The following approvals got outdated and were removed:
Code-Review-1 by plaisthos


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.

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.

Change-Id: Icabc8acf75638c86c8c395e9ffecba7a7226cd97
Signed-off-by: Frank Lichtenheld 
---
M CMakeLists.txt
M tests/unit_tests/openvpn/Makefile.am
A tests/unit_tests/openvpn/input/appears_empty.txt
A tests/unit_tests/openvpn/input/empty.txt
A tests/unit_tests/openvpn/input/leak_suppr.txt
M tests/unit_tests/openvpn/mock_msg.c
M tests/unit_tests/openvpn/test_user_pass.c
7 files changed, 110 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/74/474/8

diff --git a/CMakeLists.txt b/CMakeLists.txt
index fdd2b01..a96733e 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -666,7 +666,7 @@
 # for compat with autotools make check
 set(_UT_SOURCE_DIR 
${CMAKE_CURRENT_SOURCE_DIR}/tests/unit_tests/openvpn)
 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 88a694d..7b0a086 100644
--- a/tests/unit_tests/openvpn/Makefile.am
+++ b/tests/unit_tests/openvpn/Makefile.am
@@ -4,6 +4,8 @@

 AM_TESTSUITE_SUMMARY_HEADER = ' for $(PACKAGE_STRING) Unit-Tests'

+AM_TESTS_ENVIRONMENT = export 
LSAN_OPTIONS=suppressions=$(srcdir)/input/leak_suppr.txt;
+
 test_binaries=

 if HAVE_LD_WRAP_SUPPORT
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 000..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 000..e69de29
--- /dev/null
+++ b/tests/unit_tests/openvpn/input/empty.txt
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 000..72ebfe0
--- /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 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 5d3f9b6..4506571 100644
--- a/tests/unit_tests/openvpn/test_user_pass.c
+++ b/tests/unit_tests/openvpn/test_user_pass.c
@@ -165,6 +165,16 @@

 reset_user_pass();

+/*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(, "\t\n\t", "UT", flags, NULL));
+assert_true(up.defined);
+assert_string_equal(up.username, "");
+assert_string_equal(up.password, "");
+
+reset_user_pass();
+
 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);
@@ -197,6 +207,25 @@
 assert_string_equal(up.password, "cpassword");
 }

+/* NOTE: expect_assert_failure does not seem to work with

[Openvpn-devel] [PATCH applied] Re: dco-freebsd: dynamically re-allocate buffer if it's too small

2024-02-07 Thread Gert Doering 
Acked-by: Gert Doering 

Stare-at-code looks good :-) - there's one possible catch should realloc()
return NULL - in that case we'd pass drv.ifd_data = NULL to the kernel - but
I'm reasonably sure the kernel will then not crash but return EINVAL.

I do not have a sufficient number of clients on a FreeBSD 14 system today
to trigger the original problem, but I can attest that the new dynamic
code will do the right thing - I tested this by reducing the start allocation
to 1 byte and see it grow to 1024 (test with 2 clients, 512 sufficient
for 1 client).

(Also, master+patch passes the full server test rig on FreeBSD 14)

Your patch has been applied to the master and release/2.6 branch (bugfix)

commit 62676935d738f74908845ca96819a36a8c0c230e (master)
commit d8faf568d237667c66141e2c3f6df3f999aa02bd (release/2.6)
Author: Kristof Provost
Date:   Wed Jan 24 16:27:39 2024 +0100

 dco-freebsd: dynamically re-allocate buffer if it's too small

 Signed-off-by: Kristof Provost 
 Acked-by: Gert Doering 
 Message-Id: <20240124152739.28248-1-kprov...@netgate.com>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28128.html
 Signed-off-by: Gert Doering 


--
kind regards,

Gert Doering



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel