openvpn-devel Mailing List for OpenVPN

Hi Antonio, I think I need to test another platform. It may give us more information. I will choose one arm board to have a try when I'm free. Tony Antonio Quartulli <a...@un...> 于2024年1月30日周二 19:02写道： > > Hi, > > On 29/01/2024 05:25, Tony He wrote: > > Hi Antonio, > > > > I'm using ovpn-dco which is backported to v4.14 based on your latest > > code. My topology is: > > > > LAN PC -- openwrt router running openvpn server -- WAN PC running openvpn client > > Router is with two mips64 cores. > > > > I use the iperf3 to test speed between LAN PC and WAN PC. > > The result is sometime the performance is good(~280Mbps） while > > sometimes is bad(~140Mbps). > > When the performance is bad, one of two CPUs is 100% idle. When the > > performance is good, two CPUs are busy. However， I don't see the issue > > when ipsec is tested in the same test env. Two cores are always used > > for ipsec. So , can ovpn-dco use all cpu cores to get max performance? > > ovpn-dco uses more than core in order to perform different operations, > but more parallelism on traffic processing can definitely be implemented > (patches are welcome ;)). > > Now it's hard to tell if what you are seeing is the result of this > implementation detail or something else, especially because in some > cases you get higher throughput. > > > Cheers, > > -- > Antonio Quartulli 

Hi, On 29/01/2024 05:25, Tony He wrote: > Hi Antonio, > > I'm using ovpn-dco which is backported to v4.14 based on your latest > code. My topology is: > > LAN PC -- openwrt router running openvpn server -- WAN PC running openvpn client > Router is with two mips64 cores. > > I use the iperf3 to test speed between LAN PC and WAN PC. > The result is sometime the performance is good(~280Mbps） while > sometimes is bad(~140Mbps). > When the performance is bad, one of two CPUs is 100% idle. When the > performance is good, two CPUs are busy. However， I don't see the issue > when ipsec is tested in the same test env. Two cores are always used > for ipsec. So , can ovpn-dco use all cpu cores to get max performance? ovpn-dco uses more than core in order to perform different operations, but more parallelism on traffic processing can definitely be implemented (patches are welcome ;)). Now it's hard to tell if what you are seeing is the result of this implementation detail or something else, especially because in some cases you get higher throughput. Cheers, -- Antonio Quartulli 

Fixes issues with UTF8 in Files (user_pass UT test). For added fanciness, could set charset header in resulting mail, so "git send-email" wouldn't have to ask... but that's less nuisance than "python explodes because UTF8" :-) Your patch has been applied to the master branch. commit e1f8c599aeb840909f5ea8e9ae0bc4dab5bc7deb Author: Frank Lichtenheld Date: Mon Jan 29 15:57:56 2024 +0100 gerrit-send-mail: Make output consistent across systems Signed-off-by: Frank Lichtenheld <fr...@li...> Acked-by: Arne Schwabe <arn...@rf...> Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg28153.html Signed-off-by: Gert Doering <ge...@gr...> -- kind regards, Gert Doering 

cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/508?usp=email ) Change subject: gerrit-send-mail: Make output consistent across systems ...................................................................... gerrit-send-mail: Make output consistent across systems When writing the file specify encoding and newline, so that the local settings (like locale) do not change the output. Change-Id: Id7b4bda38adfbb446bdac635ac5d5207ef3f2f40 Signed-off-by: Frank Lichtenheld <fr...@li...> Acked-by: Arne Schwabe <arn...@rf...> Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg28153.html Signed-off-by: Gert Doering <ge...@gr...> --- M dev-tools/gerrit-send-mail.py 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-tools/gerrit-send-mail.py b/dev-tools/gerrit-send-mail.py index 5429aef..67a2cf1 100755 --- a/dev-tools/gerrit-send-mail.py +++ b/dev-tools/gerrit-send-mail.py @@ -109,7 +109,7 @@ ) filename = f"gerrit-{args.changeid}-{details['revision']}.patch" patch_text_final = patch_text_mod.replace("Subject: [PATCH]", f"Subject: [PATCH v{details['revision']}]") - with open(filename, "w") as patch_file: + with open(filename, "w", encoding="utf-8", newline="\n") as patch_file: patch_file.write(patch_text_final) print("send with:") print(f"git send-email --in-reply-to {details['msg_id']} {filename}") -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/508?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: Id7b4bda38adfbb446bdac635ac5d5207ef3f2f40 Gerrit-Change-Number: 508 Gerrit-PatchSet: 2 Gerrit-Owner: flichtenheld <fr...@li...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-MessageType: merged 

cron2 has uploaded a new patch set (#2) to the change originally created by flichtenheld. ( http://gerrit.openvpn.net/c/openvpn/+/508?usp=email ) The following approvals got outdated and were removed: Code-Review+2 by plaisthos Change subject: gerrit-send-mail: Make output consistent across systems ...................................................................... gerrit-send-mail: Make output consistent across systems When writing the file specify encoding and newline, so that the local settings (like locale) do not change the output. Change-Id: Id7b4bda38adfbb446bdac635ac5d5207ef3f2f40 Signed-off-by: Frank Lichtenheld <fr...@li...> Acked-by: Arne Schwabe <arn...@rf...> Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg28153.html Signed-off-by: Gert Doering <ge...@gr...> --- M dev-tools/gerrit-send-mail.py 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/08/508/2 diff --git a/dev-tools/gerrit-send-mail.py b/dev-tools/gerrit-send-mail.py index 5429aef..67a2cf1 100755 --- a/dev-tools/gerrit-send-mail.py +++ b/dev-tools/gerrit-send-mail.py @@ -109,7 +109,7 @@ ) filename = f"gerrit-{args.changeid}-{details['revision']}.patch" patch_text_final = patch_text_mod.replace("Subject: [PATCH]", f"Subject: [PATCH v{details['revision']}]") - with open(filename, "w") as patch_file: + with open(filename, "w", encoding="utf-8", newline="\n") as patch_file: patch_file.write(patch_text_final) print("send with:") print(f"git send-email --in-reply-to {details['msg_id']} {filename}") -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/508?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: Id7b4bda38adfbb446bdac635ac5d5207ef3f2f40 Gerrit-Change-Number: 508 Gerrit-PatchSet: 2 Gerrit-Owner: flichtenheld <fr...@li...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-MessageType: newpatchset 

From: Frank Lichtenheld <fr...@li...> When writing the file specify encoding and newline, so that the local settings (like locale) do not change the output. Change-Id: Id7b4bda38adfbb446bdac635ac5d5207ef3f2f40 Signed-off-by: Frank Lichtenheld <fr...@li...> Acked-by: Arne Schwabe <arn...@rf...> --- 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/+/508 This mail reflects revision 1 of this Change. Acked-by according to Gerrit (reflected above): Arne Schwabe <arn...@rf...> diff --git a/dev-tools/gerrit-send-mail.py b/dev-tools/gerrit-send-mail.py index 5429aef..67a2cf1 100755 --- a/dev-tools/gerrit-send-mail.py +++ b/dev-tools/gerrit-send-mail.py @@ -109,7 +109,7 @@ ) filename = f"gerrit-{args.changeid}-{details['revision']}.patch" patch_text_final = patch_text_mod.replace("Subject: [PATCH v1]", f"Subject: [PATCH v{details['revision']}]") - with open(filename, "w") as patch_file: + with open(filename, "w", encoding="utf-8", newline="\n") as patch_file: patch_file.write(patch_text_final) print("send with:") print(f"git send-email --in-reply-to {details['msg_id']} {filename}") 

From: Frank Lichtenheld <fr...@li...> Change-Id: I8b5570f6314e917f92dce072279efe415d79b22a Signed-off-by: Frank Lichtenheld <fr...@li...> Acked-by: Arne Schwabe <arn...@rf...> --- 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 7 of this Change. Acked-by according to Gerrit (reflected above): Arne Schwabe <arn...@rf...> diff --git a/tests/unit_tests/openvpn/test_user_pass.c b/tests/unit_tests/openvpn/test_user_pass.c index d6e5650..743a006 100644 --- a/tests/unit_tests/openvpn/test_user_pass.c +++ b/tests/unit_tests/openvpn/test_user_pass.c @@ -364,6 +364,63 @@ } #endif /* ifndef _MSC_VER */ +#ifdef ENABLE_MANAGEMENT +static void +test_get_user_pass_dynamic_challenge(void **state) +{ + struct user_pass up = { 0 }; + reset_user_pass(&up); + 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(&up, 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(&up); + 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(&up, 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(&up); + + 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(&up, "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), @@ -375,6 +432,10 @@ cmocka_unit_test(test_get_user_pass_authfile_stdin_assertions), cmocka_unit_test(test_get_user_pass_authfile_file_assertions), #endif +#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 

cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/473?usp=email ) Change subject: test_user_pass: Add UTs for character filtering ...................................................................... test_user_pass: Add UTs for character filtering For simplicity I implemented them only with the inline method, but they actually apply to all methods. Change-Id: Ie8d2d5f6f58679baaf5eb817a7e2ca1afcb8c4db Signed-off-by: Frank Lichtenheld <fr...@li...> Acked-by: Arne Schwabe <arn...@rf...> Acked-by: Gert Doering <ge...@gr...> Message-Id: <202...@gr...> URL: https://www.mail-archive.com/search?l=mid&q=2...@gr... Signed-off-by: Gert Doering <ge...@gr...> --- M tests/unit_tests/openvpn/test_user_pass.c 1 file changed, 23 insertions(+), 0 deletions(-) diff --git a/tests/unit_tests/openvpn/test_user_pass.c b/tests/unit_tests/openvpn/test_user_pass.c index ab4dfe4..277cb1d 100644 --- a/tests/unit_tests/openvpn/test_user_pass.c +++ b/tests/unit_tests/openvpn/test_user_pass.c @@ -141,6 +141,29 @@ reset_user_pass(&up); + /* Test various valid characters */ + /*FIXME: query_user_exec() called even though nothing queued */ + will_return(query_user_exec_builtin, true); + /* FIXME? content after first two lines just ignored */ + assert_true(get_user_pass_cr(&up, "#iuser and 커뮤니티\n//ipasswörd!\nsome other content\nnot relevant", "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, "#iuser and 커뮤니티"); + assert_string_equal(up.password, "//ipasswörd!"); + + reset_user_pass(&up); + + /* Test various invalid characters */ + /*FIXME: query_user_exec() called even though nothing queued */ + will_return(query_user_exec_builtin, true); + /*FIXME? allows arbitrary crap if c > 127 */ + /*FIXME? silently removes control characters */ + assert_true(get_user_pass_cr(&up, "\tiuser\r\nipass\xffwo\x1erd", "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, "iuser"); + assert_string_equal(up.password, "ipass\xffword"); + + 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); -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/473?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: Ie8d2d5f6f58679baaf5eb817a7e2ca1afcb8c4db Gerrit-Change-Number: 473 Gerrit-PatchSet: 7 Gerrit-Owner: flichtenheld <fr...@li...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-MessageType: merged 

cron2 has uploaded a new patch set (#7) to the change originally created by flichtenheld. ( http://gerrit.openvpn.net/c/openvpn/+/473?usp=email ) The following approvals got outdated and were removed: Code-Review+2 by cron2, Code-Review+2 by plaisthos Change subject: test_user_pass: Add UTs for character filtering ...................................................................... test_user_pass: Add UTs for character filtering For simplicity I implemented them only with the inline method, but they actually apply to all methods. Change-Id: Ie8d2d5f6f58679baaf5eb817a7e2ca1afcb8c4db Signed-off-by: Frank Lichtenheld <fr...@li...> Acked-by: Arne Schwabe <arn...@rf...> Acked-by: Gert Doering <ge...@gr...> Message-Id: <202...@gr...> URL: https://www.mail-archive.com/search?l=mid&q=2...@gr... Signed-off-by: Gert Doering <ge...@gr...> --- M tests/unit_tests/openvpn/test_user_pass.c 1 file changed, 23 insertions(+), 0 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/73/473/7 diff --git a/tests/unit_tests/openvpn/test_user_pass.c b/tests/unit_tests/openvpn/test_user_pass.c index ab4dfe4..277cb1d 100644 --- a/tests/unit_tests/openvpn/test_user_pass.c +++ b/tests/unit_tests/openvpn/test_user_pass.c @@ -141,6 +141,29 @@ reset_user_pass(&up); + /* Test various valid characters */ + /*FIXME: query_user_exec() called even though nothing queued */ + will_return(query_user_exec_builtin, true); + /* FIXME? content after first two lines just ignored */ + assert_true(get_user_pass_cr(&up, "#iuser and 커뮤니티\n//ipasswörd!\nsome other content\nnot relevant", "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, "#iuser and 커뮤니티"); + assert_string_equal(up.password, "//ipasswörd!"); + + reset_user_pass(&up); + + /* Test various invalid characters */ + /*FIXME: query_user_exec() called even though nothing queued */ + will_return(query_user_exec_builtin, true); + /*FIXME? allows arbitrary crap if c > 127 */ + /*FIXME? silently removes control characters */ + assert_true(get_user_pass_cr(&up, "\tiuser\r\nipass\xffwo\x1erd", "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, "iuser"); + assert_string_equal(up.password, "ipass\xffword"); + + 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); -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/473?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: Ie8d2d5f6f58679baaf5eb817a7e2ca1afcb8c4db Gerrit-Change-Number: 473 Gerrit-PatchSet: 7 Gerrit-Owner: flichtenheld <fr...@li...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-MessageType: newpatchset 

Tested locally and with GHA. I expect the UTF8 codes in our source to create issues at some point (because software is so... helpful), but it's only the test code, and if that happens, we can move to \xbb\xa4 (etc.) in the strings... Your patch has been applied to the master branch. commit 55418bf62eaff1c4323d206181cd8a5f88e7c6c7 Author: Frank Lichtenheld Date: Mon Jan 29 11:53:57 2024 +0100 test_user_pass: Add UTs for character filtering Signed-off-by: Frank Lichtenheld <fr...@li...> Acked-by: Arne Schwabe <arn...@rf...> Acked-by: Gert Doering <ge...@gr...> Message-Id: <202...@gr...> URL: https://www.mail-archive.com/search?l=mid&q=2...@gr... Signed-off-by: Gert Doering <ge...@gr...> -- kind regards, Gert Doering 

Attention is currently required from: flichtenheld. Hello flichtenheld, I'd like you to do a code review. Please visit http://gerrit.openvpn.net/c/openvpn/+/509?usp=email to review the following change. Change subject: [CMake] Allow unit tests to fall back to hard coded location ...................................................................... [CMake] Allow unit tests to fall back to hard coded location Settings the environment variable required for running unit tests is tiresome in my IDE (Clion). So allow unit tests to fall back to a hard coded location in case the environment variable is not set. Change-Id: Ide72b81f497088dd0fd2cdcfff83cbce5b48f145 --- M CMakeLists.txt M tests/unit_tests/openvpn/test_common.h M tests/unit_tests/openvpn/test_user_pass.c 3 files changed, 37 insertions(+), 6 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/09/509/1 diff --git a/CMakeLists.txt b/CMakeLists.txt index be55c60..fdd2b01 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -682,6 +682,10 @@ target_include_directories(${test_name} PRIVATE src/openvpn) + # for compat with IDEs like Clion that ignore the tests properties + # for the environment variable srcdir when running tests as fallback + target_compile_definitions(${test_name} PRIVATE "-DUNIT_TEST_SOURCEDIR=\"${CMAKE_SOURCE_DIR}/tests/unit_tests/openvpn\"") + if (NOT ${test_name} STREQUAL "test_buffer") target_sources(${test_name} PRIVATE src/openvpn/buffer.c diff --git a/tests/unit_tests/openvpn/test_common.h b/tests/unit_tests/openvpn/test_common.h index 50e16d6..f219e93 100644 --- a/tests/unit_tests/openvpn/test_common.h +++ b/tests/unit_tests/openvpn/test_common.h @@ -32,9 +32,36 @@ * This has a openvpn prefix to avoid confusion with cmocka's unit_test_setup_* * methods */ -void +static inline void openvpn_unit_test_setup() { assert_int_equal(setvbuf(stdout, NULL, _IONBF, BUFSIZ), 0); assert_int_equal(setvbuf(stderr, NULL, _IONBF, BUFSIZ), 0); } + +/** + * Helper function to get a file path from the unit test directory to open it + * or pass its path to another function. This function will first look for + * an environment variable or if failing that, will fall back to a hardcoded + * value from compile time if compiled with CMake. + * + * @param buf buffer holding the path to the file + * @param bufsize size of buf + * @param filename name of the filename to retrieve relative to the + * unit test source directory + */ +void +openvpn_test_get_srcdir_dir(char *buf, size_t bufsize, const char *filename) +{ + const char *srcdir = getenv("srcdir"); + +#if defined(UNIT_TEST_SOURCEDIR) + if (!srcdir) + { + srcdir = UNIT_TEST_SOURCEDIR; + } +#endif + assert_non_null(srcdir); + + snprintf(buf, bufsize, "%s/%s", srcdir, filename); +} diff --git a/tests/unit_tests/openvpn/test_user_pass.c b/tests/unit_tests/openvpn/test_user_pass.c index ab4dfe4..fee5891 100644 --- a/tests/unit_tests/openvpn/test_user_pass.c +++ b/tests/unit_tests/openvpn/test_user_pass.c @@ -35,6 +35,7 @@ #include <string.h> #include <setjmp.h> #include <cmocka.h> +#include "test_common.h" #include "misc.c" @@ -209,11 +210,9 @@ reset_user_pass(&up); unsigned int flags = 0; - const char *srcdir = getenv("srcdir"); - assert_non_null(srcdir); char authfile[PATH_MAX] = { 0 }; + openvpn_test_get_srcdir_dir(authfile, PATH_MAX, "input/user_pass.txt" ); - snprintf(authfile, PATH_MAX, "%s/%s", srcdir, "input/user_pass.txt"); /*FIXME: query_user_exec() called even though nothing queued */ will_return(query_user_exec_builtin, true); assert_true(get_user_pass_cr(&up, authfile, "UT", flags, NULL)); @@ -223,7 +222,7 @@ reset_user_pass(&up); - snprintf(authfile, PATH_MAX, "%s/%s", srcdir, "input/user_only.txt"); + 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"); will_return(query_user_exec_builtin, true); @@ -236,7 +235,7 @@ reset_user_pass(&up); flags |= GET_USER_PASS_PASSWORD_ONLY; - snprintf(authfile, PATH_MAX, "%s/%s", srcdir, "input/user_only.txt"); + openvpn_test_get_srcdir_dir(authfile, PATH_MAX, "input/user_only.txt"); /*FIXME: query_user_exec() called even though nothing queued */ will_return(query_user_exec_builtin, true); assert_true(get_user_pass_cr(&up, authfile, "UT", flags, NULL)); @@ -256,5 +255,6 @@ int main(void) { + openvpn_unit_test_setup(); return cmocka_run_group_tests(user_pass_tests, NULL, NULL); } -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/509?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: Ide72b81f497088dd0fd2cdcfff83cbce5b48f145 Gerrit-Change-Number: 509 Gerrit-PatchSet: 1 Gerrit-Owner: plaisthos <arn...@rf...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-MessageType: newchange 

Attention is currently required from: plaisthos. Hello flichtenheld, I'd like you to reexamine a change. Please visit http://gerrit.openvpn.net/c/openvpn/+/446?usp=email to look at the new patch set (#3). Change subject: Turn dead list test code into unit test ...................................................................... Turn dead list test code into unit test Change-Id: I7511bc43cd6a0bcb89476f27d5822ab4a78d0d21 Signed-off-by: Arne Schwabe <ar...@rf...> --- M CMakeLists.txt M src/openvpn/init.c M src/openvpn/list.c M src/openvpn/list.h M tests/unit_tests/openvpn/Makefile.am M tests/unit_tests/openvpn/test_misc.c 6 files changed, 188 insertions(+), 193 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/46/446/3 diff --git a/CMakeLists.txt b/CMakeLists.txt index fdd2b01..3252a2d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -747,8 +747,11 @@ tests/unit_tests/openvpn/mock_get_random.c src/openvpn/options_util.c src/openvpn/ssl_util.c + src/openvpn/list.c ) + target_compile_definitions(test_misc PRIVATE "-DSOURCEDIR=\"${CMAKE_SOURCE_DIR}\"") + target_sources(test_ncp PRIVATE src/openvpn/crypto_mbedtls.c src/openvpn/crypto_openssl.c diff --git a/src/openvpn/init.c b/src/openvpn/init.c index c5cc154..52b4308 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -865,11 +865,6 @@ return false; #endif -#ifdef LIST_TEST - list_test(); - return false; -#endif - #ifdef IFCONFIG_POOL_TEST ifconfig_pool_test(0x0A010004, 0x0A0100FF); return false; diff --git a/src/openvpn/list.c b/src/openvpn/list.c index 480f39d..dc4b1df 100644 --- a/src/openvpn/list.c +++ b/src/openvpn/list.c @@ -326,185 +326,6 @@ } -#ifdef LIST_TEST - -/* - * Test the hash code by implementing a simple - * word frequency algorithm. - */ - -struct word -{ - const char *word; - int n; -}; - -static uint32_t -word_hash_function(const void *key, uint32_t iv) -{ - const char *str = (const char *) key; - const int len = strlen(str); - return hash_func((const uint8_t *)str, len, iv); -} - -static bool -word_compare_function(const void *key1, const void *key2) -{ - return strcmp((const char *)key1, (const char *)key2) == 0; -} - -static void -print_nhash(struct hash *hash) -{ - struct hash_iterator hi; - struct hash_element *he; - int count = 0; - - hash_iterator_init(hash, &hi, true); - - while ((he = hash_iterator_next(&hi))) - { - printf("%d ", (int) he->value); - ++count; - } - printf("\n"); - - hash_iterator_free(&hi); - ASSERT(count == hash_n_elements(hash)); -} - -static void -rmhash(struct hash *hash, const char *word) -{ - hash_remove(hash, word); -} - -void -list_test(void) -{ - openvpn_thread_init(); - - { - struct gc_arena gc = gc_new(); - struct hash *hash = hash_init(10000, get_random(), word_hash_function, word_compare_function); - struct hash *nhash = hash_init(256, get_random(), word_hash_function, word_compare_function); - - printf("hash_init n_buckets=%d mask=0x%08x\n", hash->n_buckets, hash->mask); - - /* parse words from stdin */ - while (true) - { - char buf[256]; - char wordbuf[256]; - int wbi; - int bi; - char c; - - if (!fgets(buf, sizeof(buf), stdin)) - { - break; - } - - bi = wbi = 0; - do - { - c = buf[bi++]; - if (isalnum(c) || c == '_') - { - ASSERT(wbi < (int) sizeof(wordbuf)); - wordbuf[wbi++] = c; - } - else - { - if (wbi) - { - struct word *w; - ASSERT(wbi < (int) sizeof(wordbuf)); - wordbuf[wbi++] = '\0'; - - /* word is parsed from stdin */ - - /* does it already exist in table? */ - w = (struct word *) hash_lookup(hash, wordbuf); - - if (w) - { - /* yes, increment count */ - ++w->n; - } - else - { - /* no, make a new object */ - ALLOC_OBJ_GC(w, struct word, &gc); - w->word = string_alloc(wordbuf, &gc); - w->n = 1; - ASSERT(hash_add(hash, w->word, w, false)); - ASSERT(hash_add(nhash, w->word, (void *) ((random() & 0x0F) + 1), false)); - } - } - wbi = 0; - } - } while (c); - } - -#if 1 - /* remove some words from the table */ - { - rmhash(hash, "true"); - rmhash(hash, "false"); - } -#endif - - /* output contents of hash table */ - { - int base; - int inc = 0; - int count = 0; - - for (base = 0; base < hash_n_buckets(hash); base += inc) - { - struct hash_iterator hi; - struct hash_element *he; - inc = (get_random() % 3) + 1; - hash_iterator_init_range(hash, &hi, true, base, base + inc); - - while ((he = hash_iterator_next(&hi))) - { - struct word *w = (struct word *) he->value; - printf("%6d '%s'\n", w->n, w->word); - ++count; - } - - hash_iterator_free(&hi); - } - ASSERT(count == hash_n_elements(hash)); - } - -#if 1 - /* test hash_remove_by_value function */ - { - int i; - for (i = 1; i <= 16; ++i) - { - printf("[%d] ***********************************\n", i); - print_nhash(nhash); - hash_remove_by_value(nhash, (void *) i, true); - } - printf("FINAL **************************\n"); - print_nhash(nhash); - } -#endif - - hash_free(hash); - hash_free(nhash); - gc_free(&gc); - } - - openvpn_thread_cleanup(); -} - -#endif /* ifdef LIST_TEST */ - /* * -------------------------------------------------------------------- * hash() -- hash a variable-length key into a 32-bit value diff --git a/src/openvpn/list.h b/src/openvpn/list.h index 94d14f2..18afc54 100644 --- a/src/openvpn/list.h +++ b/src/openvpn/list.h @@ -33,8 +33,6 @@ * client instances over various key spaces. */ -/* define this to enable special list test mode */ -/*#define LIST_TEST*/ #include "basic.h" #include "buffer.h" @@ -114,11 +112,6 @@ uint32_t hash_func(const uint8_t *k, uint32_t length, uint32_t initval); -#ifdef LIST_TEST -void list_test(void); - -#endif - static inline uint32_t hash_value(const struct hash *hash, const void *key) { diff --git a/tests/unit_tests/openvpn/Makefile.am b/tests/unit_tests/openvpn/Makefile.am index 88a694d..dfb9f6a 100644 --- a/tests/unit_tests/openvpn/Makefile.am +++ b/tests/unit_tests/openvpn/Makefile.am @@ -288,7 +288,8 @@	$(top_srcdir)/src/openvpn/ssl_util.c misc_testdriver_CFLAGS = @TEST_CFLAGS@ \ -	-I$(top_srcdir)/include -I$(top_srcdir)/src/compat -I$(top_srcdir)/src/openvpn +	-I$(top_srcdir)/include -I$(top_srcdir)/src/compat -I$(top_srcdir)/src/openvpn \ +	-DSOURCEDIR=\"$(top_srcdir)\" misc_testdriver_LDFLAGS = @TEST_LDFLAGS@ @@ -298,4 +299,6 @@	$(top_srcdir)/src/openvpn/options_util.c \	$(top_srcdir)/src/openvpn/ssl_util.c \	$(top_srcdir)/src/openvpn/win32-util.c \ -	$(top_srcdir)/src/openvpn/platform.c +	$(top_srcdir)/src/openvpn/platform.c \ +	$(top_srcdir)/src/openvpn/list.c + diff --git a/tests/unit_tests/openvpn/test_misc.c b/tests/unit_tests/openvpn/test_misc.c index 193f131..76de0d1 100644 --- a/tests/unit_tests/openvpn/test_misc.c +++ b/tests/unit_tests/openvpn/test_misc.c @@ -37,6 +37,7 @@ #include "ssl_util.h" #include "options_util.h" #include "test_common.h" +#include "list.h" static void test_compat_lzo_string(void **state) @@ -108,11 +109,190 @@ assert_int_equal(o.server_backoff_time, 77); } + + +struct word +{ + const char *word; + int n; +}; + + +static uint32_t +word_hash_function(const void *key, uint32_t iv) +{ + const char *str = (const char *) key; + const int len = strlen(str); + return hash_func((const uint8_t *)str, len, iv); +} + +static bool +word_compare_function(const void *key1, const void *key2) +{ + return strcmp((const char *)key1, (const char *)key2) == 0; +} + +static unsigned long +get_random(void) +{ + /* rand() is not very random, but it's C99 and this is just for testing */ + return rand(); +} + +static void +test_list(void **state) +{ + +/* + * Test the hash code by implementing a simple + * word frequency algorithm. + */ + + struct gc_arena gc = gc_new(); + struct hash *hash = hash_init(10000, get_random(), word_hash_function, word_compare_function); + struct hash *nhash = hash_init(256, get_random(), word_hash_function, word_compare_function); + + printf("hash_init n_buckets=%d mask=0x%08x\n", hash->n_buckets, hash->mask); + + char wordfile[PATH_MAX] = { 0 }; + openvpn_test_get_srcdir_dir(wordfile, PATH_MAX, "/../../../COPYRIGHT.GPL" ); + + FILE *words = fopen(wordfile, "r"); + assert_non_null(words); + + int wordcount = 0; + + /* parse words from file */ + while (true) + { + char buf[256]; + char wordbuf[256]; + + if (!fgets(buf, sizeof(buf), words)) + { + break; + } + + char c = 0; + int bi = 0, wbi = 0; + + do + { + c = buf[bi++]; + if (isalnum(c) || c == '_') + { + ASSERT(wbi < (int) sizeof(wordbuf)); + wordbuf[wbi++] = c; + } + else + { + if (wbi) + { + wordcount++; + + ASSERT(wbi < (int) sizeof(wordbuf)); + wordbuf[wbi++] = '\0'; + + /* word is parsed from stdin */ + + /* does it already exist in table? */ + struct word *w = (struct word *) hash_lookup(hash, wordbuf); + + if (w) + { + assert_string_equal(w->word, wordbuf); + /* yes, increment count */ + ++w->n; + } + else + { + /* no, make a new object */ + ALLOC_OBJ_GC(w, struct word, &gc); + w->word = string_alloc(wordbuf, &gc); + w->n = 1; + ASSERT(hash_add(hash, w->word, w, false)); + ASSERT(hash_add(nhash, w->word, (void *) ((ptr_type )(random() & 0x0F) + 1), false)); + } + } + wbi = 0; + } + } + while (c); + } + + assert_int_equal(wordcount, 2978); + + /* remove some words from the table */ + { + assert_true(hash_remove(hash, "DEFECTIVE")); + assert_false(hash_remove(hash, "false")); + } + + /* output contents of hash table */ + { + ptr_type inc = 0; + int count = 0; + + for (ptr_type base = 0; base < hash_n_buckets(hash); base += inc) + { + struct hash_iterator hi; + struct hash_element *he; + inc = (get_random() % 3) + 1; + hash_iterator_init_range(hash, &hi, base, base + inc); + + while ((he = hash_iterator_next(&hi))) + { + struct word *w = (struct word *) he->value; + /*printf("%6d '%s'\n", w->n, w->word); */ + ++count; + /* check a few words to match prior results */ + if (!strcmp(w->word, "is")) + { + assert_int_equal(w->n, 49); + } + else if (!strcmp(w->word, "redistribute")) + { + assert_int_equal(w->n, 5); + } + else if (!strcmp(w->word, "circumstances")) + { + assert_int_equal(w->n, 1); + } + else if (!strcmp(w->word, "so")) + { + assert_int_equal(w->n, 8); + } + else if (!strcmp(w->word, "BECAUSE")) + { + assert_int_equal(w->n, 1); + } + } + + hash_iterator_free(&hi); + } + ASSERT(count == hash_n_elements(hash)); + } + + /* test hash_remove_by_value function */ + { + for (ptr_type i = 1; i <= 16; ++i) + { + hash_remove_by_value(nhash, (void *) i); + } + } + + hash_free(hash); + hash_free(nhash); + gc_free(&gc); +} + + const struct CMUnitTest misc_tests[] = { cmocka_unit_test(test_compat_lzo_string), cmocka_unit_test(test_auth_fail_temp_no_flags), cmocka_unit_test(test_auth_fail_temp_flags), cmocka_unit_test(test_auth_fail_temp_flags_msg), + cmocka_unit_test(test_list) }; int -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/446?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: I7511bc43cd6a0bcb89476f27d5822ab4a78d0d21 Gerrit-Change-Number: 446 Gerrit-PatchSet: 3 Gerrit-Owner: plaisthos <arn...@rf...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-MessageType: newpatchset 

Attention is currently required from: cron2, flichtenheld. plaisthos has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/508?usp=email ) Change subject: gerrit-send-mail: Make output consistent across systems ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/508?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: Id7b4bda38adfbb446bdac635ac5d5207ef3f2f40 Gerrit-Change-Number: 508 Gerrit-PatchSet: 1 Gerrit-Owner: flichtenheld <fr...@li...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: cron2 <ge...@gr...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-Comment-Date: Mon, 29 Jan 2024 11:18:35 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment 

From: Frank Lichtenheld <fr...@li...> For simplicity I implemented them only with the inline method, but they actually apply to all methods. Change-Id: Ie8d2d5f6f58679baaf5eb817a7e2ca1afcb8c4db Signed-off-by: Frank Lichtenheld <fr...@li...> Acked-by: Arne Schwabe <arn...@rf...> Acked-by: Gert Doering <ge...@gr...> --- 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/+/473 This mail reflects revision 6 of this Change. Acked-by according to Gerrit (reflected above): Arne Schwabe <arn...@rf...> Gert Doering <ge...@gr...> diff --git a/tests/unit_tests/openvpn/test_user_pass.c b/tests/unit_tests/openvpn/test_user_pass.c index ab4dfe4..277cb1d 100644 --- a/tests/unit_tests/openvpn/test_user_pass.c +++ b/tests/unit_tests/openvpn/test_user_pass.c @@ -141,6 +141,29 @@ reset_user_pass(&up); + /* Test various valid characters */ + /*FIXME: query_user_exec() called even though nothing queued */ + will_return(query_user_exec_builtin, true); + /* FIXME? content after first two lines just ignored */ + assert_true(get_user_pass_cr(&up, "#iuser and 커뮤니티\n//ipasswörd!\nsome other content\nnot relevant", "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, "#iuser and 커뮤니티"); + assert_string_equal(up.password, "//ipasswörd!"); + + reset_user_pass(&up); + + /* Test various invalid characters */ + /*FIXME: query_user_exec() called even though nothing queued */ + will_return(query_user_exec_builtin, true); + /*FIXME? allows arbitrary crap if c > 127 */ + /*FIXME? silently removes control characters */ + assert_true(get_user_pass_cr(&up, "\tiuser\r\nipass\xffwo\x1erd", "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, "iuser"); + assert_string_equal(up.password, "ipass\xffword"); + + 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); 

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/+/508?usp=email to review the following change. Change subject: gerrit-send-mail: Make output consistent across systems ...................................................................... gerrit-send-mail: Make output consistent across systems When writing the file specify encoding and newline, so that the local settings (like locale) do not change the output. Change-Id: Id7b4bda38adfbb446bdac635ac5d5207ef3f2f40 Signed-off-by: Frank Lichtenheld <fr...@li...> --- M dev-tools/gerrit-send-mail.py 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/08/508/1 diff --git a/dev-tools/gerrit-send-mail.py b/dev-tools/gerrit-send-mail.py index 5429aef..67a2cf1 100755 --- a/dev-tools/gerrit-send-mail.py +++ b/dev-tools/gerrit-send-mail.py @@ -109,7 +109,7 @@ ) filename = f"gerrit-{args.changeid}-{details['revision']}.patch" patch_text_final = patch_text_mod.replace("Subject: [PATCH]", f"Subject: [PATCH v{details['revision']}]") - with open(filename, "w") as patch_file: + with open(filename, "w", encoding="utf-8", newline="\n") as patch_file: patch_file.write(patch_text_final) print("send with:") print(f"git send-email --in-reply-to {details['msg_id']} {filename}") -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/508?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: Id7b4bda38adfbb446bdac635ac5d5207ef3f2f40 Gerrit-Change-Number: 508 Gerrit-PatchSet: 1 Gerrit-Owner: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-MessageType: newchange 

Attention is currently required from: flichtenheld. cron2 has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/473?usp=email ) Change subject: test_user_pass: Add UTs for character filtering ...................................................................... Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/473?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: Ie8d2d5f6f58679baaf5eb817a7e2ca1afcb8c4db Gerrit-Change-Number: 473 Gerrit-PatchSet: 6 Gerrit-Owner: flichtenheld <fr...@li...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-Comment-Date: Mon, 29 Jan 2024 10:01:04 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment 

cron2 has uploaded a new patch set (#9) to the change originally created by flichtenheld. ( http://gerrit.openvpn.net/c/openvpn/+/468?usp=email ) The following approvals got outdated and were removed: Code-Review+2 by plaisthos Change subject: test_user_pass: new UT for get_user_pass ...................................................................... test_user_pass: new UT for get_user_pass UTs for basic functionality, without management functions. v2: - add CMake support - add GHA support for both MSVC and mingw v3: - fix distcheck by adding input/ directory to dist Change-Id: I193aef06912f01426dd4ac298aadfab97dd75a35 Signed-off-by: Frank Lichtenheld <fr...@li...> Acked-by: Arne Schwabe <arn...@rf...> Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg28138.html Signed-off-by: Gert Doering <ge...@gr...> --- M .github/workflows/build.yaml M CMakeLists.txt M src/openvpn/misc.h M src/openvpn/syshead.h M tests/unit_tests/openvpn/Makefile.am A tests/unit_tests/openvpn/input/user_only.txt A tests/unit_tests/openvpn/input/user_pass.txt A tests/unit_tests/openvpn/test_user_pass.c 8 files changed, 327 insertions(+), 2 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/68/468/9 diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index cc98813..bc937e5 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -85,11 +85,13 @@ fail-fast: false matrix: arch: [x86, x64] - test: [argv, auth_token, buffer, cryptoapi, crypto, misc, ncp, packet_id, pkt, provider, ssl, tls_crypt] + test: [argv, auth_token, buffer, cryptoapi, crypto, misc, ncp, packet_id, pkt, provider, ssl, tls_crypt, user_pass] runs-on: windows-latest name: "mingw unittest ${{ matrix.test }} - ${{ matrix.arch }} - OSSL" steps: + - name: Checkout OpenVPN + uses: actions/checkout@v3 - name: Retrieve mingw unittest uses: actions/download-artifact@v3 with: @@ -97,6 +99,8 @@ path: unittests - name: Run ${{ matrix.test }} unit test run: ./unittests/test_${{ matrix.test }}.exe + env: + srcdir: "${{ github.workspace }}/tests/unit_tests/openvpn" ubuntu: strategy: @@ -279,6 +283,7 @@ configurePreset: win-${{ matrix.arch }}-release buildPreset: win-${{ matrix.arch }}-release testPreset: win-${{ matrix.arch }}-release + testPresetAdditionalArgs: "['--output-on-failure']" - uses: actions/upload-artifact@v3 with: diff --git a/CMakeLists.txt b/CMakeLists.txt index 5e07d50..be55c60 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -604,6 +604,7 @@ "test_pkt" "test_provider" "test_ssl" + "test_user_pass" ) if (WIN32) @@ -662,6 +663,10 @@ # test_networking needs special environment if (NOT ${test_name} STREQUAL "test_networking") add_test(${test_name} ${test_name}) + # 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}") endif () add_executable(${test_name} tests/unit_tests/openvpn/${test_name}.c @@ -782,6 +787,15 @@ src/openvpn/base64.c ) + target_sources(test_user_pass PRIVATE + tests/unit_tests/openvpn/mock_get_random.c + tests/unit_tests/openvpn/mock_win32_execve.c + src/openvpn/base64.c + src/openvpn/console.c + src/openvpn/env_set.c + src/openvpn/run_command.c + ) + if (TARGET test_argv) target_link_options(test_argv PRIVATE -Wl,--wrap=parse_line) target_sources(test_argv PRIVATE diff --git a/src/openvpn/misc.h b/src/openvpn/misc.h index d51609d..5ae61b5 100644 --- a/src/openvpn/misc.h +++ b/src/openvpn/misc.h @@ -118,12 +118,31 @@ #define GET_USER_PASS_INLINE_CREDS (1<<10) /* indicates that auth_file is actually inline creds */ +/** + * Retrieves the user credentials from various sources depending on the flags. + * + * @param up The user_pass structure to store the retrieved credentials. + * @param auth_file The path to the authentication file. Might be NULL. + * @param prefix The prefix to prepend to user prompts. + * @param flags Additional flags to control the behavior of the function. + * @param auth_challenge The authentication challenge string. + * @return true if the user credentials were successfully retrieved, false otherwise. + */ bool get_user_pass_cr(struct user_pass *up, const char *auth_file, const char *prefix, const unsigned int flags, const char *auth_challenge); +/** + * Retrieves the user credentials from various sources depending on the flags. + * + * @param up The user_pass structure to store the retrieved credentials. + * @param auth_file The path to the authentication file. Might be NULL. + * @param prefix The prefix to prepend to user prompts. + * @param flags Additional flags to control the behavior of the function. + * @return true if the user credentials were successfully retrieved, false otherwise. + */ static inline bool get_user_pass(struct user_pass *up, const char *auth_file, diff --git a/src/openvpn/syshead.h b/src/openvpn/syshead.h index a021c91..d2859fe 100644 --- a/src/openvpn/syshead.h +++ b/src/openvpn/syshead.h @@ -524,4 +524,10 @@ #define ENABLE_MEMSTATS #endif +#ifdef _MSC_VER +#ifndef PATH_MAX +#define PATH_MAX MAX_PATH +#endif +#endif + #endif /* ifndef SYSHEAD_H */ diff --git a/tests/unit_tests/openvpn/Makefile.am b/tests/unit_tests/openvpn/Makefile.am index f81a10f..88a694d 100644 --- a/tests/unit_tests/openvpn/Makefile.am +++ b/tests/unit_tests/openvpn/Makefile.am @@ -1,5 +1,7 @@ AUTOMAKE_OPTIONS = foreign +EXTRA_DIST = input + AM_TESTSUITE_SUMMARY_HEADER = ' for $(PACKAGE_STRING) Unit-Tests' test_binaries= @@ -9,7 +11,7 @@ endif test_binaries += crypto_testdriver packet_id_testdriver auth_token_testdriver ncp_testdriver misc_testdriver \ -	pkt_testdriver ssl_testdriver +	pkt_testdriver ssl_testdriver user_pass_testdriver if HAVE_LD_WRAP_SUPPORT if !WIN32 @@ -251,6 +253,22 @@	$(top_srcdir)/src/openvpn/base64.c +user_pass_testdriver_CFLAGS = @TEST_CFLAGS@ \ +	-I$(top_srcdir)/include -I$(top_srcdir)/src/compat -I$(top_srcdir)/src/openvpn +user_pass_testdriver_LDFLAGS = @TEST_LDFLAGS@ + +user_pass_testdriver_SOURCES = test_user_pass.c mock_msg.c \ +	$(top_srcdir)/src/openvpn/buffer.c \ +	$(top_srcdir)/src/openvpn/console.c \ +	$(top_srcdir)/src/openvpn/env_set.c \ +	mock_win32_execve.c \ +	$(top_srcdir)/src/openvpn/run_command.c \ +	mock_get_random.c \ +	$(top_srcdir)/src/openvpn/platform.c \ +	$(top_srcdir)/src/openvpn/win32-util.c \ +	$(top_srcdir)/src/openvpn/base64.c + + ncp_testdriver_CFLAGS = @TEST_CFLAGS@ \	-I$(top_srcdir)/include -I$(top_srcdir)/src/compat -I$(top_srcdir)/src/openvpn \	$(OPTIONAL_CRYPTO_CFLAGS) diff --git a/tests/unit_tests/openvpn/input/user_only.txt b/tests/unit_tests/openvpn/input/user_only.txt new file mode 100644 index 0000000..3ca0db3 --- /dev/null +++ b/tests/unit_tests/openvpn/input/user_only.txt @@ -0,0 +1 @@ +fuser diff --git a/tests/unit_tests/openvpn/input/user_pass.txt b/tests/unit_tests/openvpn/input/user_pass.txt new file mode 100644 index 0000000..31b38ef --- /dev/null +++ b/tests/unit_tests/openvpn/input/user_pass.txt @@ -0,0 +1,2 @@ +fuser +fpassword diff --git a/tests/unit_tests/openvpn/test_user_pass.c b/tests/unit_tests/openvpn/test_user_pass.c new file mode 100644 index 0000000..ab4dfe4 --- /dev/null +++ b/tests/unit_tests/openvpn/test_user_pass.c @@ -0,0 +1,260 @@ +/* + * OpenVPN -- An application to securely tunnel IP networks + * over a single UDP port, with support for SSL/TLS-based + * session authentication and key exchange, + * packet encryption, packet authentication, and + * packet compression. + * + * Copyright (C) 2023 OpenVPN Inc <sa...@op...> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by the + * Free Software Foundation, either version 2 of the License, + * or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#ifdef HAVE_CONFIG_H +#include "config.h" +#endif + +#undef ENABLE_SYSTEMD + +#include "syshead.h" +#include "manage.h" + +#include <stdlib.h> +#include <string.h> +#include <setjmp.h> +#include <cmocka.h> + +#include "misc.c" + +struct management *management; /* global */ + +/* mocking */ +bool +query_user_exec_builtin(void) +{ + /* Loop through configured query_user slots */ + for (int i = 0; i < QUERY_USER_NUMSLOTS && query_user[i].response != NULL; i++) + { + check_expected(query_user[i].prompt); + strncpy(query_user[i].response, mock_ptr_type(char *), query_user[i].response_len); + } + + return mock(); +} +void +management_auth_failure(struct management *man, const char *type, const char *reason) +{ + assert_true(0); +} +bool +management_query_user_pass(struct management *man, + struct user_pass *up, + const char *type, + const unsigned int flags, + const char *static_challenge) +{ + assert_true(0); + return false; +} +/* stubs for some unused functions instead of pulling in too many dependencies */ +int +parse_line(const char *line, char **p, const int n, const char *file, + const int line_num, int msglevel, struct gc_arena *gc) +{ + assert_true(0); + return 0; +} + +/* tooling */ +static void +reset_user_pass(struct user_pass *up) +{ + up->defined = false; + up->token_defined = false; + up->nocache = false; + strcpy(up->username, "user"); + strcpy(up->password, "password"); +} + +static void +test_get_user_pass_defined(void **state) +{ + struct user_pass up = { 0 }; + reset_user_pass(&up); + up.defined = true; + assert_true(get_user_pass_cr(&up, NULL, "UT", 0, NULL)); +} + +static void +test_get_user_pass_needok(void **state) +{ + struct user_pass up = { 0 }; + reset_user_pass(&up); + unsigned int flags = GET_USER_PASS_NEED_OK; + + expect_string(query_user_exec_builtin, query_user[i].prompt, "NEED-OK|UT|user:"); + will_return(query_user_exec_builtin, ""); + will_return(query_user_exec_builtin, true); + /*FIXME: query_user_exec() called even though nothing queued */ + will_return(query_user_exec_builtin, true); + assert_true(get_user_pass_cr(&up, NULL, "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.password, "ok"); + + reset_user_pass(&up); + + expect_string(query_user_exec_builtin, query_user[i].prompt, "NEED-OK|UT|user:"); + will_return(query_user_exec_builtin, "cancel"); + will_return(query_user_exec_builtin, true); + /*FIXME: query_user_exec() called even though nothing queued */ + will_return(query_user_exec_builtin, true); + assert_true(get_user_pass_cr(&up, NULL, "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.password, "cancel"); +} + +static void +test_get_user_pass_inline_creds(void **state) +{ + struct user_pass up = { 0 }; + reset_user_pass(&up); + unsigned int flags = GET_USER_PASS_INLINE_CREDS; + + /*FIXME: query_user_exec() called even though nothing queued */ + will_return(query_user_exec_builtin, true); + assert_true(get_user_pass_cr(&up, "iuser\nipassword", "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, "iuser"); + assert_string_equal(up.password, "ipassword"); + + 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); + /* will try to retrieve missing password from stdin */ + assert_true(get_user_pass_cr(&up, "iuser", "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, "iuser"); + assert_string_equal(up.password, "cpassword"); + + reset_user_pass(&up); + + flags |= GET_USER_PASS_PASSWORD_ONLY; + /*FIXME: query_user_exec() called even though nothing queued */ + will_return(query_user_exec_builtin, true); + assert_true(get_user_pass_cr(&up, "ipassword\n", "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, "user"); + assert_string_equal(up.password, "ipassword"); + + 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"); + will_return(query_user_exec_builtin, true); + /* will try to retrieve missing password from stdin */ + assert_true(get_user_pass_cr(&up, "", "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, "user"); + assert_string_equal(up.password, "cpassword"); +} + +static void +test_get_user_pass_authfile_stdin(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, "cuser"); + will_return(query_user_exec_builtin, "cpassword"); + will_return(query_user_exec_builtin, true); + assert_true(get_user_pass_cr(&up, "stdin", "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, "cuser"); + 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, "cpassword"); + will_return(query_user_exec_builtin, true); + 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, "cpassword"); +} + +static void +test_get_user_pass_authfile_file(void **state) +{ + struct user_pass up = { 0 }; + reset_user_pass(&up); + unsigned int flags = 0; + + const char *srcdir = getenv("srcdir"); + assert_non_null(srcdir); + char authfile[PATH_MAX] = { 0 }; + + snprintf(authfile, PATH_MAX, "%s/%s", srcdir, "input/user_pass.txt"); + /*FIXME: query_user_exec() called even though nothing queued */ + will_return(query_user_exec_builtin, true); + assert_true(get_user_pass_cr(&up, authfile, "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, "fuser"); + assert_string_equal(up.password, "fpassword"); + + 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"); + will_return(query_user_exec_builtin, true); + /* will try to retrieve missing password from stdin */ + assert_true(get_user_pass_cr(&up, authfile, "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, "fuser"); + assert_string_equal(up.password, "cpassword"); + + 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 */ + will_return(query_user_exec_builtin, true); + assert_true(get_user_pass_cr(&up, authfile, "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, "user"); + assert_string_equal(up.password, "fuser"); +} + +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), +}; + +int +main(void) +{ + return cmocka_run_group_tests(user_pass_tests, NULL, NULL); +} -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/468?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: I193aef06912f01426dd4ac298aadfab97dd75a35 Gerrit-Change-Number: 468 Gerrit-PatchSet: 9 Gerrit-Owner: flichtenheld <fr...@li...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-MessageType: newpatchset 

cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/468?usp=email ) Change subject: test_user_pass: new UT for get_user_pass ...................................................................... test_user_pass: new UT for get_user_pass UTs for basic functionality, without management functions. v2: - add CMake support - add GHA support for both MSVC and mingw v3: - fix distcheck by adding input/ directory to dist Change-Id: I193aef06912f01426dd4ac298aadfab97dd75a35 Signed-off-by: Frank Lichtenheld <fr...@li...> Acked-by: Arne Schwabe <arn...@rf...> Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg28138.html Signed-off-by: Gert Doering <ge...@gr...> --- M .github/workflows/build.yaml M CMakeLists.txt M src/openvpn/misc.h M src/openvpn/syshead.h M tests/unit_tests/openvpn/Makefile.am A tests/unit_tests/openvpn/input/user_only.txt A tests/unit_tests/openvpn/input/user_pass.txt A tests/unit_tests/openvpn/test_user_pass.c 8 files changed, 327 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index cc98813..bc937e5 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -85,11 +85,13 @@ fail-fast: false matrix: arch: [x86, x64] - test: [argv, auth_token, buffer, cryptoapi, crypto, misc, ncp, packet_id, pkt, provider, ssl, tls_crypt] + test: [argv, auth_token, buffer, cryptoapi, crypto, misc, ncp, packet_id, pkt, provider, ssl, tls_crypt, user_pass] runs-on: windows-latest name: "mingw unittest ${{ matrix.test }} - ${{ matrix.arch }} - OSSL" steps: + - name: Checkout OpenVPN + uses: actions/checkout@v3 - name: Retrieve mingw unittest uses: actions/download-artifact@v3 with: @@ -97,6 +99,8 @@ path: unittests - name: Run ${{ matrix.test }} unit test run: ./unittests/test_${{ matrix.test }}.exe + env: + srcdir: "${{ github.workspace }}/tests/unit_tests/openvpn" ubuntu: strategy: @@ -279,6 +283,7 @@ configurePreset: win-${{ matrix.arch }}-release buildPreset: win-${{ matrix.arch }}-release testPreset: win-${{ matrix.arch }}-release + testPresetAdditionalArgs: "['--output-on-failure']" - uses: actions/upload-artifact@v3 with: diff --git a/CMakeLists.txt b/CMakeLists.txt index 5e07d50..be55c60 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -604,6 +604,7 @@ "test_pkt" "test_provider" "test_ssl" + "test_user_pass" ) if (WIN32) @@ -662,6 +663,10 @@ # test_networking needs special environment if (NOT ${test_name} STREQUAL "test_networking") add_test(${test_name} ${test_name}) + # 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}") endif () add_executable(${test_name} tests/unit_tests/openvpn/${test_name}.c @@ -782,6 +787,15 @@ src/openvpn/base64.c ) + target_sources(test_user_pass PRIVATE + tests/unit_tests/openvpn/mock_get_random.c + tests/unit_tests/openvpn/mock_win32_execve.c + src/openvpn/base64.c + src/openvpn/console.c + src/openvpn/env_set.c + src/openvpn/run_command.c + ) + if (TARGET test_argv) target_link_options(test_argv PRIVATE -Wl,--wrap=parse_line) target_sources(test_argv PRIVATE diff --git a/src/openvpn/misc.h b/src/openvpn/misc.h index d51609d..5ae61b5 100644 --- a/src/openvpn/misc.h +++ b/src/openvpn/misc.h @@ -118,12 +118,31 @@ #define GET_USER_PASS_INLINE_CREDS (1<<10) /* indicates that auth_file is actually inline creds */ +/** + * Retrieves the user credentials from various sources depending on the flags. + * + * @param up The user_pass structure to store the retrieved credentials. + * @param auth_file The path to the authentication file. Might be NULL. + * @param prefix The prefix to prepend to user prompts. + * @param flags Additional flags to control the behavior of the function. + * @param auth_challenge The authentication challenge string. + * @return true if the user credentials were successfully retrieved, false otherwise. + */ bool get_user_pass_cr(struct user_pass *up, const char *auth_file, const char *prefix, const unsigned int flags, const char *auth_challenge); +/** + * Retrieves the user credentials from various sources depending on the flags. + * + * @param up The user_pass structure to store the retrieved credentials. + * @param auth_file The path to the authentication file. Might be NULL. + * @param prefix The prefix to prepend to user prompts. + * @param flags Additional flags to control the behavior of the function. + * @return true if the user credentials were successfully retrieved, false otherwise. + */ static inline bool get_user_pass(struct user_pass *up, const char *auth_file, diff --git a/src/openvpn/syshead.h b/src/openvpn/syshead.h index a021c91..d2859fe 100644 --- a/src/openvpn/syshead.h +++ b/src/openvpn/syshead.h @@ -524,4 +524,10 @@ #define ENABLE_MEMSTATS #endif +#ifdef _MSC_VER +#ifndef PATH_MAX +#define PATH_MAX MAX_PATH +#endif +#endif + #endif /* ifndef SYSHEAD_H */ diff --git a/tests/unit_tests/openvpn/Makefile.am b/tests/unit_tests/openvpn/Makefile.am index f81a10f..88a694d 100644 --- a/tests/unit_tests/openvpn/Makefile.am +++ b/tests/unit_tests/openvpn/Makefile.am @@ -1,5 +1,7 @@ AUTOMAKE_OPTIONS = foreign +EXTRA_DIST = input + AM_TESTSUITE_SUMMARY_HEADER = ' for $(PACKAGE_STRING) Unit-Tests' test_binaries= @@ -9,7 +11,7 @@ endif test_binaries += crypto_testdriver packet_id_testdriver auth_token_testdriver ncp_testdriver misc_testdriver \ -	pkt_testdriver ssl_testdriver +	pkt_testdriver ssl_testdriver user_pass_testdriver if HAVE_LD_WRAP_SUPPORT if !WIN32 @@ -251,6 +253,22 @@	$(top_srcdir)/src/openvpn/base64.c +user_pass_testdriver_CFLAGS = @TEST_CFLAGS@ \ +	-I$(top_srcdir)/include -I$(top_srcdir)/src/compat -I$(top_srcdir)/src/openvpn +user_pass_testdriver_LDFLAGS = @TEST_LDFLAGS@ + +user_pass_testdriver_SOURCES = test_user_pass.c mock_msg.c \ +	$(top_srcdir)/src/openvpn/buffer.c \ +	$(top_srcdir)/src/openvpn/console.c \ +	$(top_srcdir)/src/openvpn/env_set.c \ +	mock_win32_execve.c \ +	$(top_srcdir)/src/openvpn/run_command.c \ +	mock_get_random.c \ +	$(top_srcdir)/src/openvpn/platform.c \ +	$(top_srcdir)/src/openvpn/win32-util.c \ +	$(top_srcdir)/src/openvpn/base64.c + + ncp_testdriver_CFLAGS = @TEST_CFLAGS@ \	-I$(top_srcdir)/include -I$(top_srcdir)/src/compat -I$(top_srcdir)/src/openvpn \	$(OPTIONAL_CRYPTO_CFLAGS) diff --git a/tests/unit_tests/openvpn/input/user_only.txt b/tests/unit_tests/openvpn/input/user_only.txt new file mode 100644 index 0000000..3ca0db3 --- /dev/null +++ b/tests/unit_tests/openvpn/input/user_only.txt @@ -0,0 +1 @@ +fuser diff --git a/tests/unit_tests/openvpn/input/user_pass.txt b/tests/unit_tests/openvpn/input/user_pass.txt new file mode 100644 index 0000000..31b38ef --- /dev/null +++ b/tests/unit_tests/openvpn/input/user_pass.txt @@ -0,0 +1,2 @@ +fuser +fpassword diff --git a/tests/unit_tests/openvpn/test_user_pass.c b/tests/unit_tests/openvpn/test_user_pass.c new file mode 100644 index 0000000..ab4dfe4 --- /dev/null +++ b/tests/unit_tests/openvpn/test_user_pass.c @@ -0,0 +1,260 @@ +/* + * OpenVPN -- An application to securely tunnel IP networks + * over a single UDP port, with support for SSL/TLS-based + * session authentication and key exchange, + * packet encryption, packet authentication, and + * packet compression. + * + * Copyright (C) 2023 OpenVPN Inc <sa...@op...> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by the + * Free Software Foundation, either version 2 of the License, + * or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#ifdef HAVE_CONFIG_H +#include "config.h" +#endif + +#undef ENABLE_SYSTEMD + +#include "syshead.h" +#include "manage.h" + +#include <stdlib.h> +#include <string.h> +#include <setjmp.h> +#include <cmocka.h> + +#include "misc.c" + +struct management *management; /* global */ + +/* mocking */ +bool +query_user_exec_builtin(void) +{ + /* Loop through configured query_user slots */ + for (int i = 0; i < QUERY_USER_NUMSLOTS && query_user[i].response != NULL; i++) + { + check_expected(query_user[i].prompt); + strncpy(query_user[i].response, mock_ptr_type(char *), query_user[i].response_len); + } + + return mock(); +} +void +management_auth_failure(struct management *man, const char *type, const char *reason) +{ + assert_true(0); +} +bool +management_query_user_pass(struct management *man, + struct user_pass *up, + const char *type, + const unsigned int flags, + const char *static_challenge) +{ + assert_true(0); + return false; +} +/* stubs for some unused functions instead of pulling in too many dependencies */ +int +parse_line(const char *line, char **p, const int n, const char *file, + const int line_num, int msglevel, struct gc_arena *gc) +{ + assert_true(0); + return 0; +} + +/* tooling */ +static void +reset_user_pass(struct user_pass *up) +{ + up->defined = false; + up->token_defined = false; + up->nocache = false; + strcpy(up->username, "user"); + strcpy(up->password, "password"); +} + +static void +test_get_user_pass_defined(void **state) +{ + struct user_pass up = { 0 }; + reset_user_pass(&up); + up.defined = true; + assert_true(get_user_pass_cr(&up, NULL, "UT", 0, NULL)); +} + +static void +test_get_user_pass_needok(void **state) +{ + struct user_pass up = { 0 }; + reset_user_pass(&up); + unsigned int flags = GET_USER_PASS_NEED_OK; + + expect_string(query_user_exec_builtin, query_user[i].prompt, "NEED-OK|UT|user:"); + will_return(query_user_exec_builtin, ""); + will_return(query_user_exec_builtin, true); + /*FIXME: query_user_exec() called even though nothing queued */ + will_return(query_user_exec_builtin, true); + assert_true(get_user_pass_cr(&up, NULL, "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.password, "ok"); + + reset_user_pass(&up); + + expect_string(query_user_exec_builtin, query_user[i].prompt, "NEED-OK|UT|user:"); + will_return(query_user_exec_builtin, "cancel"); + will_return(query_user_exec_builtin, true); + /*FIXME: query_user_exec() called even though nothing queued */ + will_return(query_user_exec_builtin, true); + assert_true(get_user_pass_cr(&up, NULL, "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.password, "cancel"); +} + +static void +test_get_user_pass_inline_creds(void **state) +{ + struct user_pass up = { 0 }; + reset_user_pass(&up); + unsigned int flags = GET_USER_PASS_INLINE_CREDS; + + /*FIXME: query_user_exec() called even though nothing queued */ + will_return(query_user_exec_builtin, true); + assert_true(get_user_pass_cr(&up, "iuser\nipassword", "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, "iuser"); + assert_string_equal(up.password, "ipassword"); + + 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); + /* will try to retrieve missing password from stdin */ + assert_true(get_user_pass_cr(&up, "iuser", "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, "iuser"); + assert_string_equal(up.password, "cpassword"); + + reset_user_pass(&up); + + flags |= GET_USER_PASS_PASSWORD_ONLY; + /*FIXME: query_user_exec() called even though nothing queued */ + will_return(query_user_exec_builtin, true); + assert_true(get_user_pass_cr(&up, "ipassword\n", "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, "user"); + assert_string_equal(up.password, "ipassword"); + + 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"); + will_return(query_user_exec_builtin, true); + /* will try to retrieve missing password from stdin */ + assert_true(get_user_pass_cr(&up, "", "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, "user"); + assert_string_equal(up.password, "cpassword"); +} + +static void +test_get_user_pass_authfile_stdin(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, "cuser"); + will_return(query_user_exec_builtin, "cpassword"); + will_return(query_user_exec_builtin, true); + assert_true(get_user_pass_cr(&up, "stdin", "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, "cuser"); + 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, "cpassword"); + will_return(query_user_exec_builtin, true); + 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, "cpassword"); +} + +static void +test_get_user_pass_authfile_file(void **state) +{ + struct user_pass up = { 0 }; + reset_user_pass(&up); + unsigned int flags = 0; + + const char *srcdir = getenv("srcdir"); + assert_non_null(srcdir); + char authfile[PATH_MAX] = { 0 }; + + snprintf(authfile, PATH_MAX, "%s/%s", srcdir, "input/user_pass.txt"); + /*FIXME: query_user_exec() called even though nothing queued */ + will_return(query_user_exec_builtin, true); + assert_true(get_user_pass_cr(&up, authfile, "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, "fuser"); + assert_string_equal(up.password, "fpassword"); + + 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"); + will_return(query_user_exec_builtin, true); + /* will try to retrieve missing password from stdin */ + assert_true(get_user_pass_cr(&up, authfile, "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, "fuser"); + assert_string_equal(up.password, "cpassword"); + + 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 */ + will_return(query_user_exec_builtin, true); + assert_true(get_user_pass_cr(&up, authfile, "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, "user"); + assert_string_equal(up.password, "fuser"); +} + +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), +}; + +int +main(void) +{ + return cmocka_run_group_tests(user_pass_tests, NULL, NULL); +} -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/468?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: I193aef06912f01426dd4ac298aadfab97dd75a35 Gerrit-Change-Number: 468 Gerrit-PatchSet: 9 Gerrit-Owner: flichtenheld <fr...@li...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-MessageType: merged 

Local tests & GHA are now happy. Ship it :-) Your patch has been applied to the master branch. commit b9696ff387c1754d057a3611531b681d14de9105 Author: Frank Lichtenheld Date: Sat Jan 27 21:07:16 2024 +0100 test_user_pass: new UT for get_user_pass Signed-off-by: Frank Lichtenheld <fr...@li...> Acked-by: Arne Schwabe <arn...@rf...> Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg28138.html Signed-off-by: Gert Doering <ge...@gr...> -- kind regards, Gert Doering 

Hi Antonio, I'm using ovpn-dco which is backported to v4.14 based on your latest code. My topology is: LAN PC -- openwrt router running openvpn server -- WAN PC running openvpn client Router is with two mips64 cores. I use the iperf3 to test speed between LAN PC and WAN PC. The result is sometime the performance is good(~280Mbps） while sometimes is bad(~140Mbps). When the performance is bad, one of two CPUs is 100% idle. When the performance is good, two CPUs are busy. However， I don't see the issue when ipsec is tested in the same test env. Two cores are always used for ipsec. So , can ovpn-dco use all cpu cores to get max performance? Thank you! Tony 

From: Frank Lichtenheld <fr...@li...> UTs for basic functionality, without management functions. v2: - add CMake support - add GHA support for both MSVC and mingw v3: - fix distcheck by adding input/ directory to dist Change-Id: I193aef06912f01426dd4ac298aadfab97dd75a35 Signed-off-by: Frank Lichtenheld <fr...@li...> Acked-by: Arne Schwabe <arn...@rf...> --- 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/+/468 This mail reflects revision 8 of this Change. Acked-by according to Gerrit (reflected above): Arne Schwabe <arn...@rf...> diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index cc98813..bc937e5 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -85,11 +85,13 @@ fail-fast: false matrix: arch: [x86, x64] - test: [argv, auth_token, buffer, cryptoapi, crypto, misc, ncp, packet_id, pkt, provider, ssl, tls_crypt] + test: [argv, auth_token, buffer, cryptoapi, crypto, misc, ncp, packet_id, pkt, provider, ssl, tls_crypt, user_pass] runs-on: windows-latest name: "mingw unittest ${{ matrix.test }} - ${{ matrix.arch }} - OSSL" steps: + - name: Checkout OpenVPN + uses: actions/checkout@v3 - name: Retrieve mingw unittest uses: actions/download-artifact@v3 with: @@ -97,6 +99,8 @@ path: unittests - name: Run ${{ matrix.test }} unit test run: ./unittests/test_${{ matrix.test }}.exe + env: + srcdir: "${{ github.workspace }}/tests/unit_tests/openvpn" ubuntu: strategy: @@ -279,6 +283,7 @@ configurePreset: win-${{ matrix.arch }}-release buildPreset: win-${{ matrix.arch }}-release testPreset: win-${{ matrix.arch }}-release + testPresetAdditionalArgs: "['--output-on-failure']" - uses: actions/upload-artifact@v3 with: diff --git a/CMakeLists.txt b/CMakeLists.txt index 5e07d50..be55c60 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -604,6 +604,7 @@ "test_pkt" "test_provider" "test_ssl" + "test_user_pass" ) if (WIN32) @@ -662,6 +663,10 @@ # test_networking needs special environment if (NOT ${test_name} STREQUAL "test_networking") add_test(${test_name} ${test_name}) + # 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}") endif () add_executable(${test_name} tests/unit_tests/openvpn/${test_name}.c @@ -782,6 +787,15 @@ src/openvpn/base64.c ) + target_sources(test_user_pass PRIVATE + tests/unit_tests/openvpn/mock_get_random.c + tests/unit_tests/openvpn/mock_win32_execve.c + src/openvpn/base64.c + src/openvpn/console.c + src/openvpn/env_set.c + src/openvpn/run_command.c + ) + if (TARGET test_argv) target_link_options(test_argv PRIVATE -Wl,--wrap=parse_line) target_sources(test_argv PRIVATE diff --git a/src/openvpn/misc.h b/src/openvpn/misc.h index d51609d..5ae61b5 100644 --- a/src/openvpn/misc.h +++ b/src/openvpn/misc.h @@ -118,12 +118,31 @@ #define GET_USER_PASS_INLINE_CREDS (1<<10) /* indicates that auth_file is actually inline creds */ +/** + * Retrieves the user credentials from various sources depending on the flags. + * + * @param up The user_pass structure to store the retrieved credentials. + * @param auth_file The path to the authentication file. Might be NULL. + * @param prefix The prefix to prepend to user prompts. + * @param flags Additional flags to control the behavior of the function. + * @param auth_challenge The authentication challenge string. + * @return true if the user credentials were successfully retrieved, false otherwise. + */ bool get_user_pass_cr(struct user_pass *up, const char *auth_file, const char *prefix, const unsigned int flags, const char *auth_challenge); +/** + * Retrieves the user credentials from various sources depending on the flags. + * + * @param up The user_pass structure to store the retrieved credentials. + * @param auth_file The path to the authentication file. Might be NULL. + * @param prefix The prefix to prepend to user prompts. + * @param flags Additional flags to control the behavior of the function. + * @return true if the user credentials were successfully retrieved, false otherwise. + */ static inline bool get_user_pass(struct user_pass *up, const char *auth_file, diff --git a/src/openvpn/syshead.h b/src/openvpn/syshead.h index a021c91..d2859fe 100644 --- a/src/openvpn/syshead.h +++ b/src/openvpn/syshead.h @@ -524,4 +524,10 @@ #define ENABLE_MEMSTATS #endif +#ifdef _MSC_VER +#ifndef PATH_MAX +#define PATH_MAX MAX_PATH +#endif +#endif + #endif /* ifndef SYSHEAD_H */ diff --git a/tests/unit_tests/openvpn/Makefile.am b/tests/unit_tests/openvpn/Makefile.am index f81a10f..88a694d 100644 --- a/tests/unit_tests/openvpn/Makefile.am +++ b/tests/unit_tests/openvpn/Makefile.am @@ -1,5 +1,7 @@ AUTOMAKE_OPTIONS = foreign +EXTRA_DIST = input + AM_TESTSUITE_SUMMARY_HEADER = ' for $(PACKAGE_STRING) Unit-Tests' test_binaries= @@ -9,7 +11,7 @@ endif test_binaries += crypto_testdriver packet_id_testdriver auth_token_testdriver ncp_testdriver misc_testdriver \ -	pkt_testdriver ssl_testdriver +	pkt_testdriver ssl_testdriver user_pass_testdriver if HAVE_LD_WRAP_SUPPORT if !WIN32 @@ -251,6 +253,22 @@	$(top_srcdir)/src/openvpn/base64.c +user_pass_testdriver_CFLAGS = @TEST_CFLAGS@ \ +	-I$(top_srcdir)/include -I$(top_srcdir)/src/compat -I$(top_srcdir)/src/openvpn +user_pass_testdriver_LDFLAGS = @TEST_LDFLAGS@ + +user_pass_testdriver_SOURCES = test_user_pass.c mock_msg.c \ +	$(top_srcdir)/src/openvpn/buffer.c \ +	$(top_srcdir)/src/openvpn/console.c \ +	$(top_srcdir)/src/openvpn/env_set.c \ +	mock_win32_execve.c \ +	$(top_srcdir)/src/openvpn/run_command.c \ +	mock_get_random.c \ +	$(top_srcdir)/src/openvpn/platform.c \ +	$(top_srcdir)/src/openvpn/win32-util.c \ +	$(top_srcdir)/src/openvpn/base64.c + + ncp_testdriver_CFLAGS = @TEST_CFLAGS@ \	-I$(top_srcdir)/include -I$(top_srcdir)/src/compat -I$(top_srcdir)/src/openvpn \	$(OPTIONAL_CRYPTO_CFLAGS) diff --git a/tests/unit_tests/openvpn/input/user_only.txt b/tests/unit_tests/openvpn/input/user_only.txt new file mode 100644 index 0000000..3ca0db3 --- /dev/null +++ b/tests/unit_tests/openvpn/input/user_only.txt @@ -0,0 +1 @@ +fuser diff --git a/tests/unit_tests/openvpn/input/user_pass.txt b/tests/unit_tests/openvpn/input/user_pass.txt new file mode 100644 index 0000000..31b38ef --- /dev/null +++ b/tests/unit_tests/openvpn/input/user_pass.txt @@ -0,0 +1,2 @@ +fuser +fpassword diff --git a/tests/unit_tests/openvpn/test_user_pass.c b/tests/unit_tests/openvpn/test_user_pass.c new file mode 100644 index 0000000..ab4dfe4 --- /dev/null +++ b/tests/unit_tests/openvpn/test_user_pass.c @@ -0,0 +1,260 @@ +/* + * OpenVPN -- An application to securely tunnel IP networks + * over a single UDP port, with support for SSL/TLS-based + * session authentication and key exchange, + * packet encryption, packet authentication, and + * packet compression. + * + * Copyright (C) 2023 OpenVPN Inc <sa...@op...> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by the + * Free Software Foundation, either version 2 of the License, + * or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#ifdef HAVE_CONFIG_H +#include "config.h" +#endif + +#undef ENABLE_SYSTEMD + +#include "syshead.h" +#include "manage.h" + +#include <stdlib.h> +#include <string.h> +#include <setjmp.h> +#include <cmocka.h> + +#include "misc.c" + +struct management *management; /* global */ + +/* mocking */ +bool +query_user_exec_builtin(void) +{ + /* Loop through configured query_user slots */ + for (int i = 0; i < QUERY_USER_NUMSLOTS && query_user[i].response != NULL; i++) + { + check_expected(query_user[i].prompt); + strncpy(query_user[i].response, mock_ptr_type(char *), query_user[i].response_len); + } + + return mock(); +} +void +management_auth_failure(struct management *man, const char *type, const char *reason) +{ + assert_true(0); +} +bool +management_query_user_pass(struct management *man, + struct user_pass *up, + const char *type, + const unsigned int flags, + const char *static_challenge) +{ + assert_true(0); + return false; +} +/* stubs for some unused functions instead of pulling in too many dependencies */ +int +parse_line(const char *line, char **p, const int n, const char *file, + const int line_num, int msglevel, struct gc_arena *gc) +{ + assert_true(0); + return 0; +} + +/* tooling */ +static void +reset_user_pass(struct user_pass *up) +{ + up->defined = false; + up->token_defined = false; + up->nocache = false; + strcpy(up->username, "user"); + strcpy(up->password, "password"); +} + +static void +test_get_user_pass_defined(void **state) +{ + struct user_pass up = { 0 }; + reset_user_pass(&up); + up.defined = true; + assert_true(get_user_pass_cr(&up, NULL, "UT", 0, NULL)); +} + +static void +test_get_user_pass_needok(void **state) +{ + struct user_pass up = { 0 }; + reset_user_pass(&up); + unsigned int flags = GET_USER_PASS_NEED_OK; + + expect_string(query_user_exec_builtin, query_user[i].prompt, "NEED-OK|UT|user:"); + will_return(query_user_exec_builtin, ""); + will_return(query_user_exec_builtin, true); + /*FIXME: query_user_exec() called even though nothing queued */ + will_return(query_user_exec_builtin, true); + assert_true(get_user_pass_cr(&up, NULL, "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.password, "ok"); + + reset_user_pass(&up); + + expect_string(query_user_exec_builtin, query_user[i].prompt, "NEED-OK|UT|user:"); + will_return(query_user_exec_builtin, "cancel"); + will_return(query_user_exec_builtin, true); + /*FIXME: query_user_exec() called even though nothing queued */ + will_return(query_user_exec_builtin, true); + assert_true(get_user_pass_cr(&up, NULL, "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.password, "cancel"); +} + +static void +test_get_user_pass_inline_creds(void **state) +{ + struct user_pass up = { 0 }; + reset_user_pass(&up); + unsigned int flags = GET_USER_PASS_INLINE_CREDS; + + /*FIXME: query_user_exec() called even though nothing queued */ + will_return(query_user_exec_builtin, true); + assert_true(get_user_pass_cr(&up, "iuser\nipassword", "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, "iuser"); + assert_string_equal(up.password, "ipassword"); + + 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); + /* will try to retrieve missing password from stdin */ + assert_true(get_user_pass_cr(&up, "iuser", "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, "iuser"); + assert_string_equal(up.password, "cpassword"); + + reset_user_pass(&up); + + flags |= GET_USER_PASS_PASSWORD_ONLY; + /*FIXME: query_user_exec() called even though nothing queued */ + will_return(query_user_exec_builtin, true); + assert_true(get_user_pass_cr(&up, "ipassword\n", "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, "user"); + assert_string_equal(up.password, "ipassword"); + + 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"); + will_return(query_user_exec_builtin, true); + /* will try to retrieve missing password from stdin */ + assert_true(get_user_pass_cr(&up, "", "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, "user"); + assert_string_equal(up.password, "cpassword"); +} + +static void +test_get_user_pass_authfile_stdin(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, "cuser"); + will_return(query_user_exec_builtin, "cpassword"); + will_return(query_user_exec_builtin, true); + assert_true(get_user_pass_cr(&up, "stdin", "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, "cuser"); + 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, "cpassword"); + will_return(query_user_exec_builtin, true); + 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, "cpassword"); +} + +static void +test_get_user_pass_authfile_file(void **state) +{ + struct user_pass up = { 0 }; + reset_user_pass(&up); + unsigned int flags = 0; + + const char *srcdir = getenv("srcdir"); + assert_non_null(srcdir); + char authfile[PATH_MAX] = { 0 }; + + snprintf(authfile, PATH_MAX, "%s/%s", srcdir, "input/user_pass.txt"); + /*FIXME: query_user_exec() called even though nothing queued */ + will_return(query_user_exec_builtin, true); + assert_true(get_user_pass_cr(&up, authfile, "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, "fuser"); + assert_string_equal(up.password, "fpassword"); + + 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"); + will_return(query_user_exec_builtin, true); + /* will try to retrieve missing password from stdin */ + assert_true(get_user_pass_cr(&up, authfile, "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, "fuser"); + assert_string_equal(up.password, "cpassword"); + + 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 */ + will_return(query_user_exec_builtin, true); + assert_true(get_user_pass_cr(&up, authfile, "UT", flags, NULL)); + assert_true(up.defined); + assert_string_equal(up.username, "user"); + assert_string_equal(up.password, "fuser"); +} + +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), +}; + +int +main(void) +{ + return cmocka_run_group_tests(user_pass_tests, NULL, NULL); +} 

Attention is currently required from: cron2, flichtenheld. plaisthos has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/468?usp=email ) Change subject: test_user_pass: new UT for get_user_pass ...................................................................... Patch Set 8: Code-Review+2 -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/468?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: I193aef06912f01426dd4ac298aadfab97dd75a35 Gerrit-Change-Number: 468 Gerrit-PatchSet: 8 Gerrit-Owner: flichtenheld <fr...@li...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: cron2 <ge...@gr...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-Comment-Date: Fri, 26 Jan 2024 15:44:10 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment 

Attention is currently required from: flichtenheld. Hello flichtenheld, I'd like you to do a code review. Please visit http://gerrit.openvpn.net/c/openvpn/+/506?usp=email to review the following change. Change subject: Implement support for AEAD tag at the end ...................................................................... Implement support for AEAD tag at the end Change-Id: I00821d75342daf3f813b829812d648fe298bea81 --- M src/openvpn/crypto.c M src/openvpn/crypto.h M src/openvpn/init.c M src/openvpn/options.c M src/openvpn/push.c M src/openvpn/ssl.h M tests/unit_tests/openvpn/test_ssl.c 7 files changed, 85 insertions(+), 26 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/06/506/1 diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c index 2fca131..9988ebe 100644 --- a/src/openvpn/crypto.c +++ b/src/openvpn/crypto.c @@ -104,14 +104,10 @@ ASSERT(cipher_ctx_reset(ctx->cipher, iv)); } - /* Reserve space for authentication tag */ - mac_out = buf_write_alloc(&work, mac_len); - ASSERT(mac_out); - dmsg(D_PACKET_CONTENT, "ENCRYPT FROM: %s", format_hex(BPTR(buf), BLEN(buf), 80, &gc)); /* Buffer overflow check */ - if (!buf_safe(&work, buf->len + cipher_ctx_block_size(ctx->cipher))) + if (!buf_safe(&work, buf->len + mac_len + cipher_ctx_block_size(ctx->cipher))) { msg(D_CRYPT_ERRORS, "ENCRYPT: buffer size error, bc=%d bo=%d bl=%d wc=%d wo=%d wl=%d", @@ -121,9 +117,16 @@ } /* For AEAD ciphers, authenticate Additional Data, including opcode */ - ASSERT(cipher_ctx_update_ad(ctx->cipher, BPTR(&work), BLEN(&work) - mac_len)); + ASSERT(cipher_ctx_update_ad(ctx->cipher, BPTR(&work), BLEN(&work))); dmsg(D_PACKET_CONTENT, "ENCRYPT AD: %s", - format_hex(BPTR(&work), BLEN(&work) - mac_len, 0, &gc)); + format_hex(BPTR(&work), BLEN(&work), 0, &gc)); + + if (!(opt->flags & CO_AEAD_TAG_AT_THE_END)) + { + /* Reserve space for authentication tag */ + mac_out = buf_write_alloc(&work, mac_len); + ASSERT(mac_out); + } /* Encrypt packet ID, payload */ ASSERT(cipher_ctx_update(ctx->cipher, BEND(&work), &outlen, BPTR(buf), BLEN(buf))); @@ -133,6 +136,14 @@ ASSERT(cipher_ctx_final(ctx->cipher, BEND(&work), &outlen)); ASSERT(buf_inc_len(&work, outlen)); + /* if the tag is at end the end, allocate it now */ + if (opt->flags & CO_AEAD_TAG_AT_THE_END) + { + /* Reserve space for authentication tag */ + mac_out = buf_write_alloc(&work, mac_len); + ASSERT(mac_out); + } + /* Write authentication tag */ ASSERT(cipher_ctx_get_tag(ctx->cipher, mac_out, mac_len)); @@ -353,7 +364,6 @@ static const char error_prefix[] = "AEAD Decrypt error"; struct packet_id_net pin = { 0 }; const struct key_ctx *ctx = &opt->key_ctx_bi.decrypt; - uint8_t *tag_ptr = NULL; int outlen; struct gc_arena gc; @@ -406,19 +416,29 @@ /* keep the tag value to feed in later */ const int tag_size = OPENVPN_AEAD_TAG_LENGTH; - if (buf->len < tag_size) + if (buf->len < tag_size + 1) { - CRYPT_ERROR("missing tag"); + CRYPT_ERROR("missing tag or no payload"); } - tag_ptr = BPTR(buf); - ASSERT(buf_advance(buf, tag_size)); + + const int ad_size = BPTR(buf) - ad_start; + + uint8_t *tag_ptr = NULL; + int data_len = 0; + + if (opt->flags & CO_AEAD_TAG_AT_THE_END) + { + data_len = BLEN(buf) - tag_size; + tag_ptr = BPTR(buf) + data_len; + } + else + { + tag_ptr = BPTR(buf); + ASSERT(buf_advance(buf, tag_size)); + data_len = BLEN(buf); + } + dmsg(D_PACKET_CONTENT, "DECRYPT MAC: %s", format_hex(tag_ptr, tag_size, 0, &gc)); - - if (buf->len < 1) - { - CRYPT_ERROR("missing payload"); - } - dmsg(D_PACKET_CONTENT, "DECRYPT FROM: %s", format_hex(BPTR(buf), BLEN(buf), 0, &gc)); /* Buffer overflow check (should never fail) */ @@ -427,20 +447,19 @@ CRYPT_ERROR("potential buffer overflow"); } - { - /* feed in tag and the authenticated data */ - const int ad_size = BPTR(buf) - ad_start - tag_size; - ASSERT(cipher_ctx_update_ad(ctx->cipher, ad_start, ad_size)); - dmsg(D_PACKET_CONTENT, "DECRYPT AD: %s", - format_hex(BPTR(buf) - ad_size - tag_size, ad_size, 0, &gc)); - } + + /* feed in tag and the authenticated data */ + ASSERT(cipher_ctx_update_ad(ctx->cipher, ad_start, ad_size)); + dmsg(D_PACKET_CONTENT, "DECRYPT AD: %s", + format_hex(ad_start, ad_size, 0, &gc)); /* Decrypt and authenticate packet */ if (!cipher_ctx_update(ctx->cipher, BPTR(&work), &outlen, BPTR(buf), - BLEN(buf))) + data_len)) { CRYPT_ERROR("cipher update failed"); } + ASSERT(buf_inc_len(&work, outlen)); if (!cipher_ctx_final_check_tag(ctx->cipher, BPTR(&work) + outlen, &outlen, tag_ptr, tag_size)) diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h index 4201524..95a5b31 100644 --- a/src/openvpn/crypto.h +++ b/src/openvpn/crypto.h @@ -279,6 +279,10 @@ /**< Bit-flag indicating that renegotiations are using tls-crypt * with a TLS-EKM derived key. */ +#define CO_AEAD_TAG_AT_THE_END (1<<8) + /**< Bit-flag indicating that the AEAD tag is at the end of the + * packet. + */ unsigned int flags; /**< Bit-flags determining behavior of * security operation functions. */ diff --git a/src/openvpn/init.c b/src/openvpn/init.c index c5cc154..cd37b36 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -2323,6 +2323,10 @@ { buf_printf(&out, " dyn-tls-crypt"); } + if (o->imported_protocol_flags & CO_AEAD_TAG_AT_THE_END) + { + buf_printf(&out, " aead-tag-end"); + } } if (buf_len(&out) > strlen(header)) diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 2c79a1e..39f00c0 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -8686,6 +8686,10 @@ options->imported_protocol_flags |= CO_USE_DYNAMIC_TLS_CRYPT; } #endif + else if (streq(p[j], "aead-tag-end")) + { + options->imported_protocol_flags |= CO_AEAD_TAG_AT_THE_END; + } else { msg(msglevel, "Unknown protocol-flags flag: %s", p[j]); diff --git a/src/openvpn/push.c b/src/openvpn/push.c index 2249434..e4c122c 100644 --- a/src/openvpn/push.c +++ b/src/openvpn/push.c @@ -29,6 +29,7 @@ #include "push.h" #include "options.h" +#include "crypto.h" #include "ssl.h" #include "ssl_verify.h" #include "ssl_ncp.h" @@ -686,6 +687,11 @@ buf_printf(&proto_flags, " dyn-tls-crypt"); } + if (o->imported_protocol_flags & CO_AEAD_TAG_AT_THE_END) + { + buf_printf(&proto_flags, " aead-tag-end"); + } + if (buf_len(&proto_flags) > 0) { push_option_fmt(gc, push_list, M_USAGE, "protocol-flags%s", buf_str(&proto_flags)); diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h index 71b99db..a1c67a2 100644 --- a/src/openvpn/ssl.h +++ b/src/openvpn/ssl.h @@ -107,6 +107,9 @@ /** Support to dynamic tls-crypt (renegotiation with TLS-EKM derived tls-crypt key) */ #define IV_PROTO_DYN_TLS_CRYPT (1<<9) +/** Suport for the AEAD tag at the end a larger peer id and IV */ +#define IV_PROTO_DATA_V3 (1<<10) + /* Default field in X509 to be username */ #define X509_USERNAME_FIELD_DEFAULT "CN" diff --git a/tests/unit_tests/openvpn/test_ssl.c b/tests/unit_tests/openvpn/test_ssl.c index 8c1fb5b..0ded052 100644 --- a/tests/unit_tests/openvpn/test_ssl.c +++ b/tests/unit_tests/openvpn/test_ssl.c @@ -266,6 +266,19 @@ } +/* This adds a few more methods that strictly necessary but this allows + * us to see which exact test was run from the backtrace of the test + * when it fails */ + +static void +run_data_channel_with_cipher_end(const char *cipher) +{ + struct crypto_options co = init_crypto_options(cipher, "none"); + co.flags |= CO_AEAD_TAG_AT_THE_END; + do_data_channel_round_trip(&co); + uninit_crypto_options(&co); +} + static void run_data_channel_with_cipher(const char *cipher, const char *auth) { @@ -274,21 +287,25 @@ uninit_crypto_options(&co); } + static void test_data_channel_roundtrip_aes_128_gcm(void **state) { + run_data_channel_with_cipher_end("AES-128-GCM"); run_data_channel_with_cipher("AES-128-GCM", "none"); } static void test_data_channel_roundtrip_aes_192_gcm(void **state) { + run_data_channel_with_cipher_end("AES-192-GCM"); run_data_channel_with_cipher("AES-192-GCM", "none"); } static void test_data_channel_roundtrip_aes_256_gcm(void **state) { + run_data_channel_with_cipher_end("AES-256-GCM"); run_data_channel_with_cipher("AES-256-GCM", "none"); } @@ -318,6 +335,8 @@ skip(); return; } + + run_data_channel_with_cipher_end("ChaCha20-Poly1305"); run_data_channel_with_cipher("ChaCha20-Poly1305", "none"); } -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/506?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: I00821d75342daf3f813b829812d648fe298bea81 Gerrit-Change-Number: 506 Gerrit-PatchSet: 1 Gerrit-Owner: plaisthos <arn...@rf...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-MessageType: newchange 

Attention is currently required from: flichtenheld. Hello flichtenheld, I'd like you to do a code review. Please visit http://gerrit.openvpn.net/c/openvpn/+/507?usp=email to review the following change. Change subject: Implement support for larger packet counter sizes ...................................................................... Implement support for larger packet counter sizes With DCO and possible future hardware assisted OpenVPN acceleration we are approaching the point where 32 bit IVs are not cutting it any more. To illustrate the problem, some back of the envelope math here: If we want to keep the current 3600s renegotiation interval and have a safety margin of 25% (when we trigger renegotiation) we have about 3.2 million packets (2*32 * 0.7) to work with. That translates to about 835k packets per second. With 1300 Byte packets that translates into 8-9 Gbit/s. That is far from unrealistic any more. Current DCO implementations are already in spitting distance to that or might even reach (for a single client connection) that if you have extremely fast single core performance CPU. This introduces the 64bit packet counters for AEAD data channel ciphers in TLS mode ciphers. No effort has been made to support larger packet counters in any scenario since the other scenarios are all legacy. While we still keep the old --secret logic around we use the same weird unix timestamp + packet counter format to avoid refactoring the code now and again when we remove --secret code but DCO implementations are free to use just a single 64 bit counter. One other small downside of this approach is that when rollover happens and we get reordering all the older packets are thrown away since the distance between the packet before and after the rollover is quite large as we probably jump forward more than 1s (or more than 2^32 packet ids) forward. But this is an obscure edge that we can (currently) live with. Change-Id: I01e258e97351b5aa4b9e561f5b35ddc2318569e2 --- M src/openvpn/crypto.c M src/openvpn/crypto.h M src/openvpn/init.c M src/openvpn/multi.c M src/openvpn/options.c M src/openvpn/push.c M src/openvpn/ssl.c M src/openvpn/ssl_common.h M src/openvpn/ssl_ncp.c M tests/unit_tests/openvpn/test_ssl.c 10 files changed, 105 insertions(+), 20 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/07/507/1 diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c index 9988ebe..81b33fe 100644 --- a/src/openvpn/crypto.c +++ b/src/openvpn/crypto.c @@ -68,6 +68,7 @@ const struct key_ctx *ctx = &opt->key_ctx_bi.encrypt; uint8_t *mac_out = NULL; const int mac_len = OPENVPN_AEAD_TAG_LENGTH; + bool longiv = opt->flags & CO_64_BIT_PKT_ID; /* IV, packet-ID and implicit IV required for this mode. */ ASSERT(ctx->cipher); @@ -86,7 +87,7 @@ buf_set_write(&iv_buffer, iv, iv_len); /* IV starts with packet id to make the IV unique for packet */ - if (!packet_id_write(&opt->packet_id.send, &iv_buffer, false, false)) + if (!packet_id_write(&opt->packet_id.send, &iv_buffer, longiv, false)) { msg(D_CRYPT_ERRORS, "ENCRYPT ERROR: packet ID roll over"); goto err; @@ -384,6 +385,8 @@ /* IV and Packet ID required for this mode */ ASSERT(packet_id_initialized(&opt->packet_id)); + bool longiv = opt->flags & CO_64_BIT_PKT_ID; + /* Combine IV from explicit part from packet and implicit part from context */ { uint8_t iv[OPENVPN_MAX_IV_LENGTH] = { 0 }; @@ -409,7 +412,7 @@ } /* Read packet ID from packet */ - if (!packet_id_read(&pin, buf, false)) + if (!packet_id_read(&pin, buf, longiv)) { CRYPT_ERROR("error reading packet-id"); } diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h index 95a5b31..0ef13e0 100644 --- a/src/openvpn/crypto.h +++ b/src/openvpn/crypto.h @@ -283,6 +283,11 @@ /**< Bit-flag indicating that the AEAD tag is at the end of the * packet. */ +#define CO_64_BIT_PKT_ID (1<<9) + /**< Bit-flag indicating that we should use a 64 bit (8 byte) packet + * counter instead of the 32 bit that we normally use. + */ + unsigned int flags; /**< Bit-flags determining behavior of * security operation functions. */ diff --git a/src/openvpn/init.c b/src/openvpn/init.c index cd37b36..7db8d06 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -2327,6 +2327,10 @@ { buf_printf(&out, " aead-tag-end"); } + if (o->imported_protocol_flags & CO_64_BIT_PKT_ID) + { + buf_printf(&out, " pkt-id-64-bit"); + } } if (buf_len(&out) > strlen(header)) @@ -3297,6 +3301,16 @@ to.push_peer_info_detail = 1; } + /* Check if the DCO drivers support the new 64bit packet counter and + * AEAD tag at the end */ + if (dco_enabled(options)) + { + to.data_v3_features_supported = false; + } + else + { + to.data_v3_features_supported = true; + } /* should we not xmit any packets until we get an initial * response from client? */ diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index 4344126..a80b9f4 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -1851,6 +1851,13 @@ o->imported_protocol_flags |= CO_USE_CC_EXIT_NOTIFY; } + if (tls_multi->session[TM_ACTIVE].opt->data_v3_features_supported + && (proto & IV_PROTO_DATA_V3)) + { + o->imported_protocol_flags |= CO_AEAD_TAG_AT_THE_END; + o->imported_protocol_flags |= CO_64_BIT_PKT_ID; + } + /* Select cipher if client supports Negotiable Crypto Parameters */ /* if we have already created our key, we cannot *change* our own diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 39f00c0..3f8fccf 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -8690,6 +8690,10 @@ { options->imported_protocol_flags |= CO_AEAD_TAG_AT_THE_END; } + else if (streq(p[j], "pkt-id-64-bit")) + { + options->imported_protocol_flags |= CO_64_BIT_PKT_ID; + } else { msg(msglevel, "Unknown protocol-flags flag: %s", p[j]); diff --git a/src/openvpn/push.c b/src/openvpn/push.c index e4c122c..5766c97 100644 --- a/src/openvpn/push.c +++ b/src/openvpn/push.c @@ -691,6 +691,10 @@ { buf_printf(&proto_flags, " aead-tag-end"); } + if (o->imported_protocol_flags & CO_64_BIT_PKT_ID) + { + buf_printf(&proto_flags, " pkt-id-64-bit"); + } if (buf_len(&proto_flags) > 0) { diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 33c8670..6579ff9 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -105,9 +105,11 @@ * @param ctx Encrypt/decrypt key context * @param key HMAC key, used to calculate implicit IV * @param key_len HMAC key length + * @param long_pkt_id 64-bit packet counters are used */ static void -key_ctx_update_implicit_iv(struct key_ctx *ctx, uint8_t *key, size_t key_len); +key_ctx_update_implicit_iv(struct key_ctx *ctx, uint8_t *key, size_t key_len, + bool long_pkt_id); /** @@ -1369,13 +1371,15 @@ } else { + bool longiv = ks->crypto_options.flags & CO_64_BIT_PKT_ID; init_key_ctx_bi(key, key2, key_direction, key_type, "Data Channel"); /* Initialize implicit IVs */ - key_ctx_update_implicit_iv(&key->encrypt, key2->keys[(int)server].hmac, - MAX_HMAC_KEY_LENGTH); + key_ctx_update_implicit_iv(&key->encrypt, + key2->keys[(int)server].hmac, + MAX_HMAC_KEY_LENGTH, longiv); key_ctx_update_implicit_iv(&key->decrypt, key2->keys[1 - (int)server].hmac, - MAX_HMAC_KEY_LENGTH); + MAX_HMAC_KEY_LENGTH, longiv); } } @@ -1513,14 +1517,15 @@ } static void -key_ctx_update_implicit_iv(struct key_ctx *ctx, uint8_t *key, size_t key_len) +key_ctx_update_implicit_iv(struct key_ctx *ctx, uint8_t *key, + size_t key_len, bool longiv) { /* Only use implicit IV in AEAD cipher mode, where HMAC key is not used */ if (cipher_ctx_mode_aead(ctx->cipher)) { size_t impl_iv_len = 0; ASSERT(cipher_ctx_iv_length(ctx->cipher) >= OPENVPN_AEAD_MIN_IV_LEN); - impl_iv_len = cipher_ctx_iv_length(ctx->cipher) - sizeof(packet_id_type); + impl_iv_len = cipher_ctx_iv_length(ctx->cipher) - packet_id_size(longiv); ASSERT(impl_iv_len <= OPENVPN_MAX_IV_LENGTH); ASSERT(impl_iv_len <= key_len); memcpy(ctx->implicit_iv, key, impl_iv_len); @@ -1935,6 +1940,12 @@ iv_proto |= IV_PROTO_DYN_TLS_CRYPT; #endif + /* support for AEAD tag at the end and 8 byte IV */ + if (session->opt->data_v3_features_supported) + { + iv_proto |= IV_PROTO_DATA_V3; + } + buf_printf(&out, "IV_PROTO=%d\n", iv_proto); if (session->opt->push_peer_info_detail > 1) diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h index f085e0d..53bf763 100644 --- a/src/openvpn/ssl_common.h +++ b/src/openvpn/ssl_common.h @@ -311,7 +311,6 @@ /* from command line */ bool single_session; - bool disable_occ; int mode; bool pull; /** @@ -361,6 +360,8 @@ const char *config_ciphername; const char *config_ncp_ciphers; + bool data_v3_features_supported; /**< dco supports new data channel features */ + bool tls_crypt_v2; const char *tls_crypt_v2_verify_script; @@ -490,8 +491,6 @@ */ int key_id; - int limit_next; /* used for traffic shaping on the control channel */ - int verify_maxlevel; char *common_name; diff --git a/src/openvpn/ssl_ncp.c b/src/openvpn/ssl_ncp.c index 0ca6d42..0b4ad8a 100644 --- a/src/openvpn/ssl_ncp.c +++ b/src/openvpn/ssl_ncp.c @@ -430,6 +430,12 @@ session->opt->crypto_flags |= CO_USE_CC_EXIT_NOTIFY; } + if (session->opt->data_v3_features_supported && (iv_proto_peer & IV_PROTO_DATA_V3)) + { + session->opt->crypto_flags |= CO_AEAD_TAG_AT_THE_END; + session->opt->crypto_flags |= CO_64_BIT_PKT_ID; + } + #if defined(HAVE_EXPORT_KEYING_MATERIAL) if (iv_proto_peer & IV_PROTO_TLS_KEY_EXPORT) { diff --git a/tests/unit_tests/openvpn/test_ssl.c b/tests/unit_tests/openvpn/test_ssl.c index 0ded052..125d24b 100644 --- a/tests/unit_tests/openvpn/test_ssl.c +++ b/tests/unit_tests/openvpn/test_ssl.c @@ -128,9 +128,12 @@ { cipher_ctx_t *cipher = co->key_ctx_bi.encrypt.cipher; + if (cipher_ctx_mode_aead(cipher)) { - size_t impl_iv_len = cipher_ctx_iv_length(cipher) - sizeof(packet_id_type); + bool longiv = co->flags & CO_64_BIT_PKT_ID; + + size_t impl_iv_len = cipher_ctx_iv_length(cipher) - packet_id_size(longiv); ASSERT(cipher_ctx_iv_length(cipher) <= OPENVPN_MAX_IV_LENGTH); ASSERT(cipher_ctx_iv_length(cipher) >= OPENVPN_AEAD_MIN_IV_LEN); @@ -142,6 +145,11 @@ memcpy(co->key_ctx_bi.decrypt.implicit_iv, co->key_ctx_bi.encrypt.implicit_iv, OPENVPN_MAX_IV_LENGTH); co->key_ctx_bi.decrypt.implicit_iv_len = impl_iv_len; + + if (longiv) + { + co->flags |= CO_64_BIT_PKT_ID; + } } } @@ -280,6 +288,25 @@ } static void +run_data_channel_with_cipher_end_and_long_pkt_counter(const char *cipher) +{ + struct crypto_options co = init_crypto_options(cipher, "none"); + co.flags |= CO_AEAD_TAG_AT_THE_END; + do_data_channel_round_trip(&co); + uninit_crypto_options(&co); +} + +static void +run_data_channel_with_long_pkt_counter(const char *cipher) +{ + struct crypto_options co = init_crypto_options(cipher, "none"); + co.flags |= CO_64_BIT_PKT_ID; + do_data_channel_round_trip(&co); + uninit_crypto_options(&co); +} + + +static void run_data_channel_with_cipher(const char *cipher, const char *auth) { struct crypto_options co = init_crypto_options(cipher, auth); @@ -289,24 +316,30 @@ static void +run_aead_channel_tests(const char *cipher) +{ + run_data_channel_with_cipher_end(cipher); + run_data_channel_with_cipher(cipher, "none"); + run_data_channel_with_cipher_end_and_long_pkt_counter(cipher); + run_data_channel_with_long_pkt_counter(cipher); +} + +static void test_data_channel_roundtrip_aes_128_gcm(void **state) { - run_data_channel_with_cipher_end("AES-128-GCM"); - run_data_channel_with_cipher("AES-128-GCM", "none"); + run_aead_channel_tests("AES-128-GCM"); } static void test_data_channel_roundtrip_aes_192_gcm(void **state) { - run_data_channel_with_cipher_end("AES-192-GCM"); - run_data_channel_with_cipher("AES-192-GCM", "none"); + run_aead_channel_tests("AES-192-GCM"); } static void test_data_channel_roundtrip_aes_256_gcm(void **state) { - run_data_channel_with_cipher_end("AES-256-GCM"); - run_data_channel_with_cipher("AES-256-GCM", "none"); + run_aead_channel_tests("AES-256-GCM"); } static void @@ -336,8 +369,7 @@ return; } - run_data_channel_with_cipher_end("ChaCha20-Poly1305"); - run_data_channel_with_cipher("ChaCha20-Poly1305", "none"); + run_aead_channel_tests("ChaCha20-Poly1305"); } static void -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/507?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: I01e258e97351b5aa4b9e561f5b35ddc2318569e2 Gerrit-Change-Number: 507 Gerrit-PatchSet: 1 Gerrit-Owner: plaisthos <arn...@rf...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-MessageType: newchange 

Attention is currently required from: flichtenheld, plaisthos. Hello flichtenheld, I'd like you to reexamine a change. Please visit http://gerrit.openvpn.net/c/openvpn/+/365?usp=email to look at the new patch set (#9). The following approvals got outdated and were removed: Code-Review-1 by flichtenheld Change subject: Print SSL peer signature information in handshake debug details ...................................................................... Print SSL peer signature information in handshake debug details This is more SSL debug information that most people do not really need or care about. OpenSSL's own s_client also logs them: Peer signing digest: SHA256 Peer signature type: ECDSA The complete message looks like this: Control Channel: TLSv1.3, cipher TLSv1.3 TLS_AES_256_GCM_SHA384, peer certificate: 2048 bits RSA, signature: RSA-SHA256, server temp key: 253 bits X25519, peer signing digest/type: SHA256 RSASSA-PSS or when forcing a specific group via tls-groups X448 with a ECDSA server: Control Channel: TLSv1.3, cipher TLSv1.3 TLS_AES_256_GCM_SHA384, peer certificate: 384 bits ECsecp384r1, signature: ecdsa-with-SHA256, server temp key: 448 bits X448, peer signing digest/type: SHA384 ECDSA Change-Id: Ib5fc0c4b8f164596681ac5ad73002068ec6de1e5 Signed-off-by: Arne Schwabe <ar...@rf...> --- M src/openvpn/ssl_openssl.c 1 file changed, 81 insertions(+), 2 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/65/365/9 diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c index c30e6a9..9b6027c 100644 --- a/src/openvpn/ssl_openssl.c +++ b/src/openvpn/ssl_openssl.c @@ -2166,6 +2166,83 @@ EVP_PKEY_free(pkey); } +#if !defined(LIBRESSL_VERSION_NUMBER) && OPENSSL_VERSION_NUMBER >= 0x1010000fL +/** + * Translate an OpenSSL NID into a more human readable name + * @param nid + * @return + */ +static const char * +get_sigtype(int nid) +{ + /* Fix a few OpenSSL names to be better understandable */ + switch (nid) + { + case EVP_PKEY_RSA: + /* will otherwise say rsaEncryption */ + return "RSA"; + + case EVP_PKEY_DSA: + /* dsaEncryption otherwise */ + return "DSA"; + + case EVP_PKEY_EC: + /* will say id-ecPublicKey */ + return "ECDSA"; + + case -1: + return "(error getting name)"; + + default: + return OBJ_nid2sn(nid); + } +} +#endif /* ifndef LIBRESSL_VERSION_NUMBER */ + +/** + * Get the type of the signature that is used by the peer during the + * TLS handshake + */ +static void +print_peer_signature(SSL *ssl, char *buf, size_t buflen) +{ + int peer_sig_nid = NID_undef, peer_sig_type_nid = NID_undef; + const char *peer_sig = "unknown"; + const char *peer_sig_type = "unknown type"; + + /* Even though these methods use the deprecated NIDs instead of using + * string as new OpenSSL APIs do, there seem to be no API that replaces + * it yet */ +#if !defined(LIBRESSL_VERSION_NUMBER) || LIBRESSL_VERSION_NUMBER > 0x3050400fL + if (SSL_get_peer_signature_nid(ssl, &peer_sig_nid) + && peer_sig_nid != NID_undef) + { + peer_sig = OBJ_nid2sn(peer_sig_nid); + } +#endif + +#if (!defined(LIBRESSL_VERSION_NUMBER) && OPENSSL_VERSION_NUMBER >= 0x1010000fL) \ + || (defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER >= 0x3090000fL) + /* LibreSSL 3.7.x and 3.8.x implement this function but do not export it + * and fail linking with an unresolved symbol */ + if (SSL_get_peer_signature_type_nid(ssl, &peer_sig_type_nid) + && peer_sig_type_nid != NID_undef) + { + peer_sig_type = get_sigtype(peer_sig_type_nid); + } +#endif + + if (peer_sig_nid == NID_undef && peer_sig_type_nid == NID_undef) + { + return; + } + + openvpn_snprintf(buf, buflen, ", peer signing digest/type: %s %s", + peer_sig, peer_sig_type); +} + + + /* ************************************** * * Information functions @@ -2180,8 +2257,9 @@ char s1[256]; char s2[256]; char s3[256]; + char s4[256]; - s1[0] = s2[0] = s3[0] = 0; + s1[0] = s2[0] = s3[0] = s4[0] = 0; ciph = SSL_get_current_cipher(ks_ssl->ssl); openvpn_snprintf(s1, sizeof(s1), "%s %s, cipher %s %s", prefix, @@ -2196,8 +2274,9 @@ X509_free(cert); } print_server_tempkey(ks_ssl->ssl, s3, sizeof(s3)); + print_peer_signature(ks_ssl->ssl, s4, sizeof(s4)); - msg(D_HANDSHAKE, "%s%s%s", s1, s2, s3); + msg(D_HANDSHAKE, "%s%s%s%s", s1, s2, s3, s4); } void -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/365?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: Ib5fc0c4b8f164596681ac5ad73002068ec6de1e5 Gerrit-Change-Number: 365 Gerrit-PatchSet: 9 Gerrit-Owner: plaisthos <arn...@rf...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-MessageType: newpatchset