[Openvpn-devel] [M] Change in openvpn[master]: t_client.sh: Allow to skip tests

2024-03-08 Thread cron2 (Code Review) 
cron2 has submitted this change. ( 
http://gerrit.openvpn.net/c/openvpn/+/521?usp=email )

Change subject: t_client.sh: Allow to skip tests
..

t_client.sh: Allow to skip tests

Individual tests can define a script to run to test
whether they should be skipped.

Included in this commit is an example check which
checks whether we can do NTLM checks. This fails
e.g. on recent versions of Fedora with mbedTLS
(tested with Fedora 39) or when NTLM support is not
compiled in.

v2:
 - ntlm_support:
   - support OpenSSL 3
   - allow to build without cmocka
v3:
 - add example to t_client.rc-sample
 - t_client.sh code style
 - use syshead.h in error.h
v5:
 - rename SKIP_x to CHECK_SKIP_x

Change-Id: I13ea6752c8d102eabcc579e391828c05d5322899
Signed-off-by: Frank Lichtenheld 
Acked-by: Gert Doering 
Message-Id: <20240308102818.9249-1-g...@greenie.muc.de>
URL: 
https://www.mail-archive.com/search?l=mid=20240308102818.9249-1-g...@greenie.muc.de
Signed-off-by: Gert Doering 
---
M src/openvpn/error.h
M tests/Makefile.am
A tests/ntlm_support.c
M tests/t_client.rc-sample
M tests/t_client.sh.in
M tests/unit_tests/openvpn/mock_msg.c
6 files changed, 119 insertions(+), 15 deletions(-)




diff --git a/src/openvpn/error.h b/src/openvpn/error.h
index 1225b13..be3484d 100644
--- a/src/openvpn/error.h
+++ b/src/openvpn/error.h
@@ -25,16 +25,10 @@
 #define ERROR_H

 #include "basic.h"
-
-#include 
-#include 
+#include "syshead.h"

 #include 

-#if _WIN32
-#include 
-#endif
-
 /* #define ABORT_ON_ERROR */

 #if defined(ENABLE_PKCS11) || defined(ENABLE_MANAGEMENT)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 6c71067..6bc02b4 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -18,6 +18,8 @@

 if !WIN32
 test_scripts = t_client.sh t_lpback.sh t_cltsrv.sh
+
+check_PROGRAMS = ntlm_support
 if HAVE_SITNL
 test_scripts += t_net.sh
 endif
@@ -35,3 +37,15 @@

 dist_noinst_DATA = \
t_client.rc-sample
+
+ntlm_support_CFLAGS  = -I$(top_srcdir)/src/openvpn -I$(top_srcdir)/src/compat 
-I$(top_srcdir)/tests/unit_tests/openvpn -DNO_CMOCKA @TEST_CFLAGS@
+ntlm_support_LDFLAGS = @TEST_LDFLAGS@ -L$(top_srcdir)/src/openvpn 
$(OPTIONAL_CRYPTO_LIBS)
+ntlm_support_SOURCES = ntlm_support.c \
+   unit_tests/openvpn/mock_msg.c unit_tests/openvpn/mock_msg.h \
+   $(top_srcdir)/src/openvpn/buffer.c \
+   $(top_srcdir)/src/openvpn/crypto.c \
+   $(top_srcdir)/src/openvpn/crypto_openssl.c \
+   $(top_srcdir)/src/openvpn/crypto_mbedtls.c \
+   $(top_srcdir)/src/openvpn/otime.c \
+   $(top_srcdir)/src/openvpn/packet_id.c \
+   $(top_srcdir)/src/openvpn/platform.c
diff --git a/tests/ntlm_support.c b/tests/ntlm_support.c
new file mode 100644
index 000..2d7da86
--- /dev/null
+++ b/tests/ntlm_support.c
@@ -0,0 +1,52 @@
+/*
+ *  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 
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2
+ *  as published by the Free Software Foundation.
+ *
+ *  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
+
+#include "syshead.h"
+
+#include "crypto.h"
+#include "error.h"
+
+int
+main(void)
+{
+#if defined(ENABLE_CRYPTO_OPENSSL)
+crypto_load_provider("legacy");
+crypto_load_provider("default");
+#endif
+#ifdef NTLM
+if (!md_valid("MD4"))
+{
+msg(M_FATAL, "MD4 not supported");
+}
+if (!md_valid("MD5"))
+{
+msg(M_FATAL, "MD5 not supported");
+}
+#else  /* ifdef NTLM */
+msg(M_FATAL, "NTLM support not compiled in");
+#endif
+}
diff --git a/tests/t_client.rc-sample b/tests/t_client.rc-sample
index 355e8bb..d61ecc4 100644
--- a/tests/t_client.rc-sample
+++ b/tests/t_client.rc-sample
@@ -27,7 +27,7 @@
 #
 # tests to run (list suffixes for config stanzas below)
 #
-TEST_RUN_LIST="1 2"
+TEST_RUN_LIST="1 2 2n"

 #
 # use "sudo" (etc) to give openvpn the necessary privileges
@@ -53,14 +53,24 @@
 #
 # if something is not defined here, the corresponding test is not run
 #
-# possible test options:
+# common test options:
 #
-# RUN_TITLE_x="what is being tested on here" (purely informational)
-# OPENVPN_CONF_x = "how to call ./openvpn"

[Openvpn-devel] [M] Change in openvpn[master]: t_client.sh: Allow to skip tests

2024-03-08 Thread cron2 (Code Review) 
cron2 has uploaded a new patch set (#6) to the change originally created by 
flichtenheld. ( http://gerrit.openvpn.net/c/openvpn/+/521?usp=email )

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


Change subject: t_client.sh: Allow to skip tests
..

t_client.sh: Allow to skip tests

Individual tests can define a script to run to test
whether they should be skipped.

Included in this commit is an example check which
checks whether we can do NTLM checks. This fails
e.g. on recent versions of Fedora with mbedTLS
(tested with Fedora 39) or when NTLM support is not
compiled in.

v2:
 - ntlm_support:
   - support OpenSSL 3
   - allow to build without cmocka
v3:
 - add example to t_client.rc-sample
 - t_client.sh code style
 - use syshead.h in error.h
v5:
 - rename SKIP_x to CHECK_SKIP_x

Change-Id: I13ea6752c8d102eabcc579e391828c05d5322899
Signed-off-by: Frank Lichtenheld 
Acked-by: Gert Doering 
Message-Id: <20240308102818.9249-1-g...@greenie.muc.de>
URL: 
https://www.mail-archive.com/search?l=mid=20240308102818.9249-1-g...@greenie.muc.de
Signed-off-by: Gert Doering 
---
M src/openvpn/error.h
M tests/Makefile.am
A tests/ntlm_support.c
M tests/t_client.rc-sample
M tests/t_client.sh.in
M tests/unit_tests/openvpn/mock_msg.c
6 files changed, 119 insertions(+), 15 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/21/521/6

diff --git a/src/openvpn/error.h b/src/openvpn/error.h
index 1225b13..be3484d 100644
--- a/src/openvpn/error.h
+++ b/src/openvpn/error.h
@@ -25,16 +25,10 @@
 #define ERROR_H

 #include "basic.h"
-
-#include 
-#include 
+#include "syshead.h"

 #include 

-#if _WIN32
-#include 
-#endif
-
 /* #define ABORT_ON_ERROR */

 #if defined(ENABLE_PKCS11) || defined(ENABLE_MANAGEMENT)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 6c71067..6bc02b4 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -18,6 +18,8 @@

 if !WIN32
 test_scripts = t_client.sh t_lpback.sh t_cltsrv.sh
+
+check_PROGRAMS = ntlm_support
 if HAVE_SITNL
 test_scripts += t_net.sh
 endif
@@ -35,3 +37,15 @@

 dist_noinst_DATA = \
t_client.rc-sample
+
+ntlm_support_CFLAGS  = -I$(top_srcdir)/src/openvpn -I$(top_srcdir)/src/compat 
-I$(top_srcdir)/tests/unit_tests/openvpn -DNO_CMOCKA @TEST_CFLAGS@
+ntlm_support_LDFLAGS = @TEST_LDFLAGS@ -L$(top_srcdir)/src/openvpn 
$(OPTIONAL_CRYPTO_LIBS)
+ntlm_support_SOURCES = ntlm_support.c \
+   unit_tests/openvpn/mock_msg.c unit_tests/openvpn/mock_msg.h \
+   $(top_srcdir)/src/openvpn/buffer.c \
+   $(top_srcdir)/src/openvpn/crypto.c \
+   $(top_srcdir)/src/openvpn/crypto_openssl.c \
+   $(top_srcdir)/src/openvpn/crypto_mbedtls.c \
+   $(top_srcdir)/src/openvpn/otime.c \
+   $(top_srcdir)/src/openvpn/packet_id.c \
+   $(top_srcdir)/src/openvpn/platform.c
diff --git a/tests/ntlm_support.c b/tests/ntlm_support.c
new file mode 100644
index 000..2d7da86
--- /dev/null
+++ b/tests/ntlm_support.c
@@ -0,0 +1,52 @@
+/*
+ *  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 
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2
+ *  as published by the Free Software Foundation.
+ *
+ *  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
+
+#include "syshead.h"
+
+#include "crypto.h"
+#include "error.h"
+
+int
+main(void)
+{
+#if defined(ENABLE_CRYPTO_OPENSSL)
+crypto_load_provider("legacy");
+crypto_load_provider("default");
+#endif
+#ifdef NTLM
+if (!md_valid("MD4"))
+{
+msg(M_FATAL, "MD4 not supported");
+}
+if (!md_valid("MD5"))
+{
+msg(M_FATAL, "MD5 not supported");
+}
+#else  /* ifdef NTLM */
+msg(M_FATAL, "NTLM support not compiled in");
+#endif
+}
diff --git a/tests/t_client.rc-sample b/tests/t_client.rc-sample
index 355e8bb..d61ecc4 100644
--- a/tests/t_client.rc-sample
+++ b/tests/t_client.rc-sample
@@ -27,7 +27,7 @@
 #
 # tests to run (list suffixes for config stanzas below)
 #
-TEST_RUN_LIST="1 2"
+TEST_RUN_LIST="1 2 2n"

 #
 # use "sudo" (etc) to give openvpn the necessary privileges
@@ -53,14 +53,24 @@
 #
 # if something is not defined here,

[Openvpn-devel] [M] Change in openvpn[master]: t_client.sh: Allow to skip tests

2024-03-08 Thread cron2 (Code Review) 
Attention is currently required from: flichtenheld, ordex, plaisthos.

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

Change subject: t_client.sh: Allow to skip tests
..


Patch Set 5: Code-Review+2

(1 comment)

Patchset:

PS5:
thanks.  t_client looks good, ntlm test we agreed on taking as it is and maybe 
someone will come up with good ideas how to improve later on.



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/521?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: I13ea6752c8d102eabcc579e391828c05d5322899
Gerrit-Change-Number: 521
Gerrit-PatchSet: 5
Gerrit-Owner: flichtenheld 
Gerrit-Reviewer: cron2 
Gerrit-Reviewer: plaisthos 
Gerrit-CC: openvpn-devel 
Gerrit-CC: ordex 
Gerrit-Attention: plaisthos 
Gerrit-Attention: flichtenheld 
Gerrit-Attention: ordex 
Gerrit-Comment-Date: Fri, 08 Mar 2024 10:27:55 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [M] Change in openvpn[master]: t_client.sh: Allow to skip tests

2024-03-06 Thread flichtenheld (Code Review) 
Attention is currently required from: cron2, ordex, plaisthos.

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

Change subject: t_client.sh: Allow to skip tests
..


Patch Set 5:

(1 comment)

Patchset:

PS4:
> Took me longer than expected after v4, but I do have some more wishes... […]
Acknowledged



-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/521?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: I13ea6752c8d102eabcc579e391828c05d5322899
Gerrit-Change-Number: 521
Gerrit-PatchSet: 5
Gerrit-Owner: flichtenheld 
Gerrit-Reviewer: cron2 
Gerrit-Reviewer: plaisthos 
Gerrit-CC: openvpn-devel 
Gerrit-CC: ordex 
Gerrit-Attention: plaisthos 
Gerrit-Attention: cron2 
Gerrit-Attention: ordex 
Gerrit-Comment-Date: Wed, 06 Mar 2024 13:55:06 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: cron2 
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [M] Change in openvpn[master]: t_client.sh: Allow to skip tests

2024-03-06 Thread flichtenheld (Code Review) 
Attention is currently required from: cron2, ordex, plaisthos.

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

Change subject: t_client.sh: Allow to skip tests
..


Patch Set 4:

(3 comments)

File tests/t_client.rc-sample:

http://gerrit.openvpn.net/c/openvpn/+/521/comment/fe9c7617_bd6d46f5 :
PS4, Line 67: # SKIP_x= "commands to execute before openvpn, skip 
test on failure"
> TBH, I do not like `SKIP_x` because it just tells me "well, yes, skip this 
> test". […]
Changed to CHECK_SKIP


http://gerrit.openvpn.net/c/openvpn/+/521/comment/70edeb0f_bee08576 :
PS4, Line 70: # CLEANUP_x = "commands to execute after the test"
> this is a very welcome addition to the last options that I forgot to properly 
> document. […]
Acknowledged


File tests/unit_tests/openvpn/mock_msg.c:

http://gerrit.openvpn.net/c/openvpn/+/521/comment/aeaf2450_604a1484 :
PS2, Line 94: endif
> Yes, in this PR it is now used in t_client context, which is not a unit test 
> context
Acknowledged



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/521?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: I13ea6752c8d102eabcc579e391828c05d5322899
Gerrit-Change-Number: 521
Gerrit-PatchSet: 4
Gerrit-Owner: flichtenheld 
Gerrit-Reviewer: cron2 
Gerrit-Reviewer: plaisthos 
Gerrit-CC: openvpn-devel 
Gerrit-CC: ordex 
Gerrit-Attention: plaisthos 
Gerrit-Attention: cron2 
Gerrit-Attention: ordex 
Gerrit-Comment-Date: Wed, 06 Mar 2024 13:54:26 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: cron2 
Comment-In-Reply-To: flichtenheld 
Comment-In-Reply-To: ordex 
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [M] Change in openvpn[master]: t_client.sh: Allow to skip tests

2024-03-06 Thread flichtenheld (Code Review) 
Attention is currently required from: cron2, flichtenheld, ordex, plaisthos.

Hello cron2, plaisthos,

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

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

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

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


Change subject: t_client.sh: Allow to skip tests
..

t_client.sh: Allow to skip tests

Individual tests can define a script to run to test
whether they should be skipped.

Included in this commit is an example check which
checks whether we can do NTLM checks. This fails
e.g. on recent versions of Fedora with mbedTLS
(tested with Fedora 39) or when NTLM support is not
compiled in.

v2:
 - ntlm_support:
   - support OpenSSL 3
   - allow to build without cmocka
v3:
 - add example to t_client.rc-sample
 - t_client.sh code style
 - use syshead.h in error.h
v5:
 - rename SKIP_x to CHECK_SKIP_x

Change-Id: I13ea6752c8d102eabcc579e391828c05d5322899
Signed-off-by: Frank Lichtenheld 
---
M src/openvpn/error.h
M tests/Makefile.am
A tests/ntlm_support.c
M tests/t_client.rc-sample
M tests/t_client.sh.in
M tests/unit_tests/openvpn/mock_msg.c
6 files changed, 119 insertions(+), 15 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/21/521/5

diff --git a/src/openvpn/error.h b/src/openvpn/error.h
index 1225b13..be3484d 100644
--- a/src/openvpn/error.h
+++ b/src/openvpn/error.h
@@ -25,16 +25,10 @@
 #define ERROR_H

 #include "basic.h"
-
-#include 
-#include 
+#include "syshead.h"

 #include 

-#if _WIN32
-#include 
-#endif
-
 /* #define ABORT_ON_ERROR */

 #if defined(ENABLE_PKCS11) || defined(ENABLE_MANAGEMENT)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index b3b2d74..13a1013 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -19,6 +19,8 @@

 if !WIN32
 test_scripts = t_client.sh t_lpback.sh t_cltsrv.sh
+
+check_PROGRAMS = ntlm_support
 if HAVE_SITNL
 test_scripts += t_net.sh
 endif
@@ -36,3 +38,15 @@

 dist_noinst_DATA = \
t_client.rc-sample
+
+ntlm_support_CFLAGS  = -I$(top_srcdir)/src/openvpn -I$(top_srcdir)/src/compat 
-I$(top_srcdir)/tests/unit_tests/openvpn -DNO_CMOCKA @TEST_CFLAGS@
+ntlm_support_LDFLAGS = @TEST_LDFLAGS@ -L$(top_srcdir)/src/openvpn 
$(OPTIONAL_CRYPTO_LIBS)
+ntlm_support_SOURCES = ntlm_support.c \
+   unit_tests/openvpn/mock_msg.c unit_tests/openvpn/mock_msg.h \
+   $(top_srcdir)/src/openvpn/buffer.c \
+   $(top_srcdir)/src/openvpn/crypto.c \
+   $(top_srcdir)/src/openvpn/crypto_openssl.c \
+   $(top_srcdir)/src/openvpn/crypto_mbedtls.c \
+   $(top_srcdir)/src/openvpn/otime.c \
+   $(top_srcdir)/src/openvpn/packet_id.c \
+   $(top_srcdir)/src/openvpn/platform.c
diff --git a/tests/ntlm_support.c b/tests/ntlm_support.c
new file mode 100644
index 000..2d7da86
--- /dev/null
+++ b/tests/ntlm_support.c
@@ -0,0 +1,52 @@
+/*
+ *  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 
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2
+ *  as published by the Free Software Foundation.
+ *
+ *  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
+
+#include "syshead.h"
+
+#include "crypto.h"
+#include "error.h"
+
+int
+main(void)
+{
+#if defined(ENABLE_CRYPTO_OPENSSL)
+crypto_load_provider("legacy");
+crypto_load_provider("default");
+#endif
+#ifdef NTLM
+if (!md_valid("MD4"))
+{
+msg(M_FATAL, "MD4 not supported");
+}
+if (!md_valid("MD5"))
+{
+msg(M_FATAL, "MD5 not supported");
+}
+#else  /* ifdef NTLM */
+msg(M_FATAL, "NTLM support not compiled in");
+#endif
+}
diff --git a/tests/t_client.rc-sample b/tests/t_client.rc-sample
index 355e8bb..d61ecc4 100644
--- a/tests/t_client.rc-sample
+++ b/tests/t_client.rc-sample
@@ -27,7 +27,7 @@
 #
 # tests to run (list suffixes for config stanzas below)
 #
-TEST_RUN_LIST="1 2"
+TEST_RUN_LIST="1 2 2n"

 #
 # use "sudo" (etc) to give openvpn the necessary privileges
@@ -53,14 +53,24 @@
 #
 # if something is not defined here, the corresponding test is not run
 #
-# possible test options:
+# common test options:
 #
-#

[Openvpn-devel] [M] Change in openvpn[master]: t_client.sh: Allow to skip tests

2024-03-06 Thread cron2 (Code Review) 
Attention is currently required from: flichtenheld, ordex, plaisthos.

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

Change subject: t_client.sh: Allow to skip tests
..


Patch Set 4: Code-Review-1

(4 comments)

Patchset:

PS4:
Took me longer than expected after v4, but I do have some more wishes... 
(thanks for bearing with me)


File tests/Makefile.am:

http://gerrit.openvpn.net/c/openvpn/+/521/comment/aabb9cc6_e1155271 :
PS4, Line 45:   unit_tests/openvpn/mock_msg.c unit_tests/openvpn/mock_msg.h \
OK, so now I do better understand the -DNO_CMOCKA code in 
unit_tests/openvpn/mock_msg.c - but I'm none the happier.  Unit test code is 
unit test code, and shouldn't be used by something else (= so when trying to 
understand code in unit_tests/ I don't really want to consider "oh, wait, this 
is also used by some other piece of code").

But I can't really can come up with a nice alternative that will not boil down 
to "just add another copy of `msg()` mocking into ntlm_support.c" (because 
`crypto_load_provider()` needs it).


File tests/t_client.rc-sample:

http://gerrit.openvpn.net/c/openvpn/+/521/comment/7c7d3b68_b8417006 :
PS4, Line 67: # SKIP_x= "commands to execute before openvpn, skip 
test on failure"
TBH, I do not like `SKIP_x` because it just tells me "well, yes, skip this 
test".

I'd suggest to rename it to something like `SKIP_CONDITIONAL_x` or `PRE_TEST_x` 
or so which better transports the message "ah, this is a command to run, and 
depending on the outcome, things will be skipped".


http://gerrit.openvpn.net/c/openvpn/+/521/comment/34ed3c96_6462fbf2 :
PS4, Line 70: # CLEANUP_x = "commands to execute after the test"
this is a very welcome addition to the last options that I forgot to properly 
document.  Thanks ;-)



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/521?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: I13ea6752c8d102eabcc579e391828c05d5322899
Gerrit-Change-Number: 521
Gerrit-PatchSet: 4
Gerrit-Owner: flichtenheld 
Gerrit-Reviewer: cron2 
Gerrit-Reviewer: plaisthos 
Gerrit-CC: openvpn-devel 
Gerrit-CC: ordex 
Gerrit-Attention: plaisthos 
Gerrit-Attention: flichtenheld 
Gerrit-Attention: ordex 
Gerrit-Comment-Date: Wed, 06 Mar 2024 09:57:55 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [M] Change in openvpn[master]: t_client.sh: Allow to skip tests

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

Hello cron2, plaisthos,

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

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

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


Change subject: t_client.sh: Allow to skip tests
..

t_client.sh: Allow to skip tests

Individual tests can define a script to run to test
whether they should be skipped.

Included in this commit is an example check which
checks whether we can do NTLM checks. This fails
e.g. on recent versions of Fedora with mbedTLS
(tested with Fedora 39) or when NTLM support is not
compiled in.

v2:
 - ntlm_support:
   - support OpenSSL 3
   - allow to build without cmocka
v3:
 - add example to t_client.rc-sample
 - t_client.sh code style
 - use syshead.h in error.h

Change-Id: I13ea6752c8d102eabcc579e391828c05d5322899
Signed-off-by: Frank Lichtenheld 
---
M src/openvpn/error.h
M tests/Makefile.am
A tests/ntlm_support.c
M tests/t_client.rc-sample
M tests/t_client.sh.in
M tests/unit_tests/openvpn/mock_msg.c
6 files changed, 119 insertions(+), 15 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/21/521/4

diff --git a/src/openvpn/error.h b/src/openvpn/error.h
index 1225b13..be3484d 100644
--- a/src/openvpn/error.h
+++ b/src/openvpn/error.h
@@ -25,16 +25,10 @@
 #define ERROR_H

 #include "basic.h"
-
-#include 
-#include 
+#include "syshead.h"

 #include 

-#if _WIN32
-#include 
-#endif
-
 /* #define ABORT_ON_ERROR */

 #if defined(ENABLE_PKCS11) || defined(ENABLE_MANAGEMENT)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index b3b2d74..13a1013 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -19,6 +19,8 @@

 if !WIN32
 test_scripts = t_client.sh t_lpback.sh t_cltsrv.sh
+
+check_PROGRAMS = ntlm_support
 if HAVE_SITNL
 test_scripts += t_net.sh
 endif
@@ -36,3 +38,15 @@

 dist_noinst_DATA = \
t_client.rc-sample
+
+ntlm_support_CFLAGS  = -I$(top_srcdir)/src/openvpn -I$(top_srcdir)/src/compat 
-I$(top_srcdir)/tests/unit_tests/openvpn -DNO_CMOCKA @TEST_CFLAGS@
+ntlm_support_LDFLAGS = @TEST_LDFLAGS@ -L$(top_srcdir)/src/openvpn 
$(OPTIONAL_CRYPTO_LIBS)
+ntlm_support_SOURCES = ntlm_support.c \
+   unit_tests/openvpn/mock_msg.c unit_tests/openvpn/mock_msg.h \
+   $(top_srcdir)/src/openvpn/buffer.c \
+   $(top_srcdir)/src/openvpn/crypto.c \
+   $(top_srcdir)/src/openvpn/crypto_openssl.c \
+   $(top_srcdir)/src/openvpn/crypto_mbedtls.c \
+   $(top_srcdir)/src/openvpn/otime.c \
+   $(top_srcdir)/src/openvpn/packet_id.c \
+   $(top_srcdir)/src/openvpn/platform.c
diff --git a/tests/ntlm_support.c b/tests/ntlm_support.c
new file mode 100644
index 000..2d7da86
--- /dev/null
+++ b/tests/ntlm_support.c
@@ -0,0 +1,52 @@
+/*
+ *  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 
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2
+ *  as published by the Free Software Foundation.
+ *
+ *  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
+
+#include "syshead.h"
+
+#include "crypto.h"
+#include "error.h"
+
+int
+main(void)
+{
+#if defined(ENABLE_CRYPTO_OPENSSL)
+crypto_load_provider("legacy");
+crypto_load_provider("default");
+#endif
+#ifdef NTLM
+if (!md_valid("MD4"))
+{
+msg(M_FATAL, "MD4 not supported");
+}
+if (!md_valid("MD5"))
+{
+msg(M_FATAL, "MD5 not supported");
+}
+#else  /* ifdef NTLM */
+msg(M_FATAL, "NTLM support not compiled in");
+#endif
+}
diff --git a/tests/t_client.rc-sample b/tests/t_client.rc-sample
index 355e8bb..608d614 100644
--- a/tests/t_client.rc-sample
+++ b/tests/t_client.rc-sample
@@ -27,7 +27,7 @@
 #
 # tests to run (list suffixes for config stanzas below)
 #
-TEST_RUN_LIST="1 2"
+TEST_RUN_LIST="1 2 2n"

 #
 # use "sudo" (etc) to give openvpn the necessary privileges
@@ -53,14 +53,24 @@
 #
 # if something is not defined here, the corresponding test is not run
 #
-# possible test options:
+# common test options:
 #
-# RUN_TITLE_x="what is being tested on here" (purely informational)
-# OPENVPN_CONF_x = "how to call ./openvpn" [mandatory]
+# RUN_TITLE_x

[Openvpn-devel] [M] Change in openvpn[master]: t_client.sh: Allow to skip tests

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

Hello cron2, plaisthos,

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

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

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

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


Change subject: t_client.sh: Allow to skip tests
..

t_client.sh: Allow to skip tests

Individual tests can define a script to run to test
whether they should be skipped.

Included in this commit is an example check which
checks whether we can do NTLM checks. This fails
e.g. on recent versions of Fedora with mbedTLS
(tested with Fedora 39) or when NTLM support is not
compiled in.

v2:
 - ntlm_support:
   - support OpenSSL 3
   - allow to build without cmocka
v3:
 - add example to t_client.rc-sample
 - t_client.sh code style
 - use syshead.h in error.h

Change-Id: I13ea6752c8d102eabcc579e391828c05d5322899
Signed-off-by: Frank Lichtenheld 
---
M src/openvpn/error.h
M tests/Makefile.am
A tests/ntlm_support.c
M tests/t_client.rc-sample
M tests/t_client.sh.in
M tests/unit_tests/openvpn/mock_msg.c
6 files changed, 119 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/21/521/3

diff --git a/src/openvpn/error.h b/src/openvpn/error.h
index 1225b13..b6963fb 100644
--- a/src/openvpn/error.h
+++ b/src/openvpn/error.h
@@ -25,15 +25,7 @@
 #define ERROR_H

 #include "basic.h"
-
-#include 
-#include 
-
-#include 
-
-#if _WIN32
-#include 
-#endif
+#include "syshead.h"

 /* #define ABORT_ON_ERROR */

diff --git a/tests/Makefile.am b/tests/Makefile.am
index b3b2d74..13a1013 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -19,6 +19,8 @@

 if !WIN32
 test_scripts = t_client.sh t_lpback.sh t_cltsrv.sh
+
+check_PROGRAMS = ntlm_support
 if HAVE_SITNL
 test_scripts += t_net.sh
 endif
@@ -36,3 +38,15 @@

 dist_noinst_DATA = \
t_client.rc-sample
+
+ntlm_support_CFLAGS  = -I$(top_srcdir)/src/openvpn -I$(top_srcdir)/src/compat 
-I$(top_srcdir)/tests/unit_tests/openvpn -DNO_CMOCKA @TEST_CFLAGS@
+ntlm_support_LDFLAGS = @TEST_LDFLAGS@ -L$(top_srcdir)/src/openvpn 
$(OPTIONAL_CRYPTO_LIBS)
+ntlm_support_SOURCES = ntlm_support.c \
+   unit_tests/openvpn/mock_msg.c unit_tests/openvpn/mock_msg.h \
+   $(top_srcdir)/src/openvpn/buffer.c \
+   $(top_srcdir)/src/openvpn/crypto.c \
+   $(top_srcdir)/src/openvpn/crypto_openssl.c \
+   $(top_srcdir)/src/openvpn/crypto_mbedtls.c \
+   $(top_srcdir)/src/openvpn/otime.c \
+   $(top_srcdir)/src/openvpn/packet_id.c \
+   $(top_srcdir)/src/openvpn/platform.c
diff --git a/tests/ntlm_support.c b/tests/ntlm_support.c
new file mode 100644
index 000..2d7da86
--- /dev/null
+++ b/tests/ntlm_support.c
@@ -0,0 +1,52 @@
+/*
+ *  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 
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2
+ *  as published by the Free Software Foundation.
+ *
+ *  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
+
+#include "syshead.h"
+
+#include "crypto.h"
+#include "error.h"
+
+int
+main(void)
+{
+#if defined(ENABLE_CRYPTO_OPENSSL)
+crypto_load_provider("legacy");
+crypto_load_provider("default");
+#endif
+#ifdef NTLM
+if (!md_valid("MD4"))
+{
+msg(M_FATAL, "MD4 not supported");
+}
+if (!md_valid("MD5"))
+{
+msg(M_FATAL, "MD5 not supported");
+}
+#else  /* ifdef NTLM */
+msg(M_FATAL, "NTLM support not compiled in");
+#endif
+}
diff --git a/tests/t_client.rc-sample b/tests/t_client.rc-sample
index 355e8bb..608d614 100644
--- a/tests/t_client.rc-sample
+++ b/tests/t_client.rc-sample
@@ -27,7 +27,7 @@
 #
 # tests to run (list suffixes for config stanzas below)
 #
-TEST_RUN_LIST="1 2"
+TEST_RUN_LIST="1 2 2n"

 #
 # use "sudo" (etc) to give openvpn the necessary privileges
@@ -53,14 +53,24 @@
 #
 # if something is not defined here, the corresponding test is not run
 #
-# possible test options:
+# common test options:
 #
-# RUN_TITLE_x="what is being tested on here" (purely informational)
-# OPENVPN_CONF_x = "how to call ./openvpn"

[Openvpn-devel] [M] Change in openvpn[master]: t_client.sh: Allow to skip tests

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

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

Change subject: t_client.sh: Allow to skip tests
..


Patch Set 2:

(4 comments)

File src/openvpn/error.h:

http://gerrit.openvpn.net/c/openvpn/+/521/comment/79ea4054_64ee021d :
PS2, Line 39: #endif
> I don't like this, for two reasons - we protect unistd. […]
Acknowledged


File tests/t_client.sh.in:

http://gerrit.openvpn.net/c/openvpn/+/521/comment/d9c095ee_2955765a :
PS2, Line 301: SKIP_
> Having an example would certainly help people setting this up for their first 
> time.
Acknowledged


http://gerrit.openvpn.net/c/openvpn/+/521/comment/df742960_4d92cd9b :
PS2, Line 325: eval $test_check_skip || {
> while this certainly works, the `|| {` style is quite different from the very 
> traditional `if/then/f […]
Acknowledged


File tests/unit_tests/openvpn/mock_msg.c:

http://gerrit.openvpn.net/c/openvpn/+/521/comment/91fc1e95_73420afd :
PS2, Line 94: endif
> Not sure I understand why we have cmocka specific . […]
Yes, in this PR it is now used in t_client context, which is not a unit test 
context



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/521?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: I13ea6752c8d102eabcc579e391828c05d5322899
Gerrit-Change-Number: 521
Gerrit-PatchSet: 2
Gerrit-Owner: flichtenheld 
Gerrit-Reviewer: cron2 
Gerrit-Reviewer: plaisthos 
Gerrit-CC: openvpn-devel 
Gerrit-CC: ordex 
Gerrit-Attention: plaisthos 
Gerrit-Attention: cron2 
Gerrit-Attention: ordex 
Gerrit-Comment-Date: Wed, 28 Feb 2024 13:36:12 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: cron2 
Comment-In-Reply-To: flichtenheld 
Comment-In-Reply-To: ordex 
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [M] Change in openvpn[master]: t_client.sh: Allow to skip tests

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

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

Change subject: t_client.sh: Allow to skip tests
..


Patch Set 2: Code-Review-1

(5 comments)

Patchset:

PS2:
I think this is a useful patch but there seems to be a bit of friction to 
overcome :-)


File src/openvpn/error.h:

http://gerrit.openvpn.net/c/openvpn/+/521/comment/509f59c6_11cebc44 :
PS2, Line 39: #endif
I don't like this, for two reasons - we protect unistd.h via `HAVE_UNISTD_H`, 
not via "not windows", and also this is really a separate project, moving out 
includes from `"syshead.h"` to the parent C file because some compilers 
complain.


File tests/t_client.sh.in:

http://gerrit.openvpn.net/c/openvpn/+/521/comment/4d712547_a890cc8a :
PS2, Line 301: SKIP_
> Like all the other test settings this is defined in the t_client.rc. […]
Having an example would certainly help people setting this up for their first 
time.


http://gerrit.openvpn.net/c/openvpn/+/521/comment/71a3de91_936dd75e :
PS2, Line 325: eval $test_check_skip || {
while this certainly works, the `|| {` style is quite different from the very 
traditional `if/then/fi` style of the rest of the script.  So I'm a bit 
undecided what to think about it.

```
if eval $test_check_skip ; then :
else
output ...
...
fi
```
is not as compact, but might be easier to grok?


File tests/unit_tests/openvpn/mock_msg.c:

http://gerrit.openvpn.net/c/openvpn/+/521/comment/4daebdcf_1d3501b2 :
PS2, Line 94: endif
> assert_failed is defined in error. […]
Not sure I understand why we have cmocka specific .c files with #ifdef 
NO_CMOCKA?  Is this ever used outside the unit test context?



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/521?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: I13ea6752c8d102eabcc579e391828c05d5322899
Gerrit-Change-Number: 521
Gerrit-PatchSet: 2
Gerrit-Owner: flichtenheld 
Gerrit-Reviewer: cron2 
Gerrit-Reviewer: plaisthos 
Gerrit-CC: openvpn-devel 
Gerrit-CC: ordex 
Gerrit-Attention: plaisthos 
Gerrit-Attention: flichtenheld 
Gerrit-Attention: ordex 
Gerrit-Comment-Date: Wed, 28 Feb 2024 10:01:20 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: flichtenheld 
Comment-In-Reply-To: ordex 
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [M] Change in openvpn[master]: t_client.sh: Allow to skip tests

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

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

Change subject: t_client.sh: Allow to skip tests
..


Patch Set 2:

(2 comments)

File tests/t_client.sh.in:

http://gerrit.openvpn.net/c/openvpn/+/521/comment/c1be2335_8f5e8256 :
PS2, Line 301: SKIP_
> I am probably clueless about this, but where is $SKIP filled?
Like all the other test settings this is defined in the t_client.rc. I can add 
an example to t_client.rc-sample


File tests/unit_tests/openvpn/mock_msg.c:

http://gerrit.openvpn.net/c/openvpn/+/521/comment/0cb16f2d_9efcdae7 :
PS2, Line 94: endif
> I wonder if mock_msg.c is the right place for assert_failed(). […]
assert_failed is defined in error.c like all the other functions mocked here, 
so it seems like a natural place.

The alternatives I saw for my patch were:
- duplicate mock_msg.c outside of unit_tests/openvpn. But except for 
assert_failed this works perfectly fine.
- try to use original error.c. But that pulls in a lot of platform code. We 
could look into split out all the complex msg stuff from error.c into its own 
msg.c. Then we could use error.c in the tests and mock less stuff here.
- move assert_failed to its own mock_assert.c. The current solution seems 
slightly simpler. But that would obviously be a possible alternative if we 
don't like the NO_CMOCKA define.



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/521?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: I13ea6752c8d102eabcc579e391828c05d5322899
Gerrit-Change-Number: 521
Gerrit-PatchSet: 2
Gerrit-Owner: flichtenheld 
Gerrit-Reviewer: plaisthos 
Gerrit-CC: openvpn-devel 
Gerrit-CC: ordex 
Gerrit-Attention: plaisthos 
Gerrit-Attention: ordex 
Gerrit-Comment-Date: Wed, 21 Feb 2024 10:47:39 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: ordex 
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [M] Change in openvpn[master]: t_client.sh: Allow to skip tests

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

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

Change subject: t_client.sh: Allow to skip tests
..


Patch Set 2:

(3 comments)

Patchset:

PS2:
Feature ACK. I like having the possibility to run tests only when 
needed/possible.


File tests/t_client.sh.in:

http://gerrit.openvpn.net/c/openvpn/+/521/comment/d0f81f67_ff0f5ae9 :
PS2, Line 301: SKIP_
I am probably clueless about this, but where is $SKIP filled?


File tests/unit_tests/openvpn/mock_msg.c:

http://gerrit.openvpn.net/c/openvpn/+/521/comment/9ff035ac_ffa92d48 :
PS2, Line 94: endif
I wonder if mock_msg.c is the right place for assert_failed(). Maybe it should 
just be moved somewhere else. opinions?



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/521?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: I13ea6752c8d102eabcc579e391828c05d5322899
Gerrit-Change-Number: 521
Gerrit-PatchSet: 2
Gerrit-Owner: flichtenheld 
Gerrit-Reviewer: plaisthos 
Gerrit-CC: openvpn-devel 
Gerrit-CC: ordex 
Gerrit-Attention: plaisthos 
Gerrit-Attention: flichtenheld 
Gerrit-Comment-Date: Wed, 21 Feb 2024 09:16:03 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [M] Change in openvpn[master]: t_client.sh: Allow to skip tests

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

Hello plaisthos,

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

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

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


Change subject: t_client.sh: Allow to skip tests
..

t_client.sh: Allow to skip tests

Individual tests can define a script to run to test
whether they should be skipped.

Included in this commit is an example check which
checks whether we can do NTLM checks. This fails
e.g. on recent versions of Fedora with mbedTLS
(tested with Fedora 39) or when NTLM support is not
compiled in.

v2:
 - ntlm_support:
   - support OpenSSL 3
   - allow to build without cmocka

Change-Id: I13ea6752c8d102eabcc579e391828c05d5322899
Signed-off-by: Frank Lichtenheld 
---
M src/openvpn/error.h
M tests/Makefile.am
A tests/ntlm_support.c
M tests/t_client.sh.in
M tests/unit_tests/openvpn/mock_msg.c
5 files changed, 94 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/21/521/2

diff --git a/src/openvpn/error.h b/src/openvpn/error.h
index 1225b13..0ef3263 100644
--- a/src/openvpn/error.h
+++ b/src/openvpn/error.h
@@ -33,6 +33,9 @@

 #if _WIN32
 #include 
+#else
+/* for _exit() */
+#include 
 #endif

 /* #define ABORT_ON_ERROR */
diff --git a/tests/Makefile.am b/tests/Makefile.am
index b3b2d74..13a1013 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -19,6 +19,8 @@

 if !WIN32
 test_scripts = t_client.sh t_lpback.sh t_cltsrv.sh
+
+check_PROGRAMS = ntlm_support
 if HAVE_SITNL
 test_scripts += t_net.sh
 endif
@@ -36,3 +38,15 @@

 dist_noinst_DATA = \
t_client.rc-sample
+
+ntlm_support_CFLAGS  = -I$(top_srcdir)/src/openvpn -I$(top_srcdir)/src/compat 
-I$(top_srcdir)/tests/unit_tests/openvpn -DNO_CMOCKA @TEST_CFLAGS@
+ntlm_support_LDFLAGS = @TEST_LDFLAGS@ -L$(top_srcdir)/src/openvpn 
$(OPTIONAL_CRYPTO_LIBS)
+ntlm_support_SOURCES = ntlm_support.c \
+   unit_tests/openvpn/mock_msg.c unit_tests/openvpn/mock_msg.h \
+   $(top_srcdir)/src/openvpn/buffer.c \
+   $(top_srcdir)/src/openvpn/crypto.c \
+   $(top_srcdir)/src/openvpn/crypto_openssl.c \
+   $(top_srcdir)/src/openvpn/crypto_mbedtls.c \
+   $(top_srcdir)/src/openvpn/otime.c \
+   $(top_srcdir)/src/openvpn/packet_id.c \
+   $(top_srcdir)/src/openvpn/platform.c
diff --git a/tests/ntlm_support.c b/tests/ntlm_support.c
new file mode 100644
index 000..8d81257
--- /dev/null
+++ b/tests/ntlm_support.c
@@ -0,0 +1,51 @@
+/*
+ *  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 
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2
+ *  as published by the Free Software Foundation.
+ *
+ *  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
+
+#include "syshead.h"
+
+#include "crypto.h"
+
+int
+main(void)
+{
+#if defined(ENABLE_CRYPTO_OPENSSL)
+crypto_load_provider("legacy");
+crypto_load_provider("default");
+#endif
+#ifdef NTLM
+if (!md_valid("MD4"))
+{
+msg(M_FATAL, "MD4 not supported");
+}
+if (!md_valid("MD5"))
+{
+msg(M_FATAL, "MD5 not supported");
+}
+#else  /* ifdef NTLM */
+msg(M_FATAL, "NTLM support not compiled in");
+#endif
+}
diff --git a/tests/t_client.sh.in b/tests/t_client.sh.in
index 99e6f9c..7be1c8e 100755
--- a/tests/t_client.sh.in
+++ b/tests/t_client.sh.in
@@ -291,12 +291,14 @@
 # main test loop
 # --
 SUMMARY_OK=
+SUMMARY_SKIP=
 SUMMARY_FAIL=

 for SUF in $TEST_RUN_LIST
 do
 # get config variables
 eval test_prep=\"\$PREPARE_$SUF\"
+eval test_check_skip=\"\$SKIP_$SUF\"
 eval test_postinit=\"\$POSTINIT_CMD_$SUF\"
 eval test_cleanup=\"\$CLEANUP_$SUF\"
 eval test_run_title=\"\$RUN_TITLE_$SUF\"
@@ -318,6 +320,15 @@
 output_start "### test run $SUF: '$test_run_title' ###"
 fail_count=0

+if [ -n "$test_check_skip" ]; then
+output "check whether we need to skip: '$test_check_skip'"
+eval $test_check_skip || {
+output "skip check failed, SKIP test $SUF."
+   SUMMARY_SKIP="$SUMMARY_SKIP $SUF"
+   echo

[Openvpn-devel] [M] Change in openvpn[master]: t_client.sh: Allow to skip tests

2024-02-13 Thread flichtenheld (Code Review) 
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/+/521?usp=email

to review the following change.


Change subject: t_client.sh: Allow to skip tests
..

t_client.sh: Allow to skip tests

Individual tests can define a script to run to test
whether they should be skipped.

Included in this commit is an example check which
checks whether we can do NTLM checks. This fails
e.g. on recent versions of Fedora with mbedTLS
(tested with Fedora 39) or when NTLM support is not
compiled in.

Change-Id: I13ea6752c8d102eabcc579e391828c05d5322899
Signed-off-by: Frank Lichtenheld 
---
M tests/Makefile.am
A tests/ntlm_support.c
M tests/t_client.sh.in
3 files changed, 76 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/21/521/1

diff --git a/tests/Makefile.am b/tests/Makefile.am
index b3b2d74..5911260 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -19,6 +19,8 @@

 if !WIN32
 test_scripts = t_client.sh t_lpback.sh t_cltsrv.sh
+
+check_PROGRAMS = ntlm_support
 if HAVE_SITNL
 test_scripts += t_net.sh
 endif
@@ -36,3 +38,15 @@

 dist_noinst_DATA = \
t_client.rc-sample
+
+ntlm_support_CFLAGS  = -I$(top_srcdir)/src/openvpn -I$(top_srcdir)/src/compat 
-I$(top_srcdir)/tests/unit_tests/openvpn @TEST_CFLAGS@
+ntlm_support_LDFLAGS = @TEST_LDFLAGS@ -L$(top_srcdir)/src/openvpn 
$(OPTIONAL_CRYPTO_LIBS)
+ntlm_support_SOURCES = ntlm_support.c \
+   unit_tests/openvpn/mock_msg.c unit_tests/openvpn/mock_msg.h \
+   $(top_srcdir)/src/openvpn/buffer.c \
+   $(top_srcdir)/src/openvpn/crypto.c \
+   $(top_srcdir)/src/openvpn/crypto_openssl.c \
+   $(top_srcdir)/src/openvpn/crypto_mbedtls.c \
+   $(top_srcdir)/src/openvpn/otime.c \
+   $(top_srcdir)/src/openvpn/packet_id.c \
+   $(top_srcdir)/src/openvpn/platform.c
diff --git a/tests/ntlm_support.c b/tests/ntlm_support.c
new file mode 100644
index 000..73bd4a8
--- /dev/null
+++ b/tests/ntlm_support.c
@@ -0,0 +1,49 @@
+/*
+ *  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 
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2
+ *  as published by the Free Software Foundation.
+ *
+ *  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
+
+#include "syshead.h"
+
+#include "crypto.h"
+
+#include "mock_msg.h"
+
+int
+main(void)
+{
+#ifdef NTLM
+if (!md_valid("MD4"))
+{
+msg(M_FATAL, "MD4 not supported");
+}
+if (!md_valid("MD5"))
+{
+msg(M_FATAL, "MD5 not supported");
+}
+#else  /* ifdef NTLM */
+msg(M_FATAL, "NTLM support not compiled in");
+#endif
+}
diff --git a/tests/t_client.sh.in b/tests/t_client.sh.in
index 99e6f9c..7be1c8e 100755
--- a/tests/t_client.sh.in
+++ b/tests/t_client.sh.in
@@ -291,12 +291,14 @@
 # main test loop
 # --
 SUMMARY_OK=
+SUMMARY_SKIP=
 SUMMARY_FAIL=

 for SUF in $TEST_RUN_LIST
 do
 # get config variables
 eval test_prep=\"\$PREPARE_$SUF\"
+eval test_check_skip=\"\$SKIP_$SUF\"
 eval test_postinit=\"\$POSTINIT_CMD_$SUF\"
 eval test_cleanup=\"\$CLEANUP_$SUF\"
 eval test_run_title=\"\$RUN_TITLE_$SUF\"
@@ -318,6 +320,15 @@
 output_start "### test run $SUF: '$test_run_title' ###"
 fail_count=0

+if [ -n "$test_check_skip" ]; then
+output "check whether we need to skip: '$test_check_skip'"
+eval $test_check_skip || {
+output "skip check failed, SKIP test $SUF."
+   SUMMARY_SKIP="$SUMMARY_SKIP $SUF"
+   echo -e "$outbuf" ; continue
+}
+fi
+
 if [ -n "$test_prep" ]; then
 output "running preparation: '$test_prep'"
 eval $test_prep
@@ -455,8 +466,10 @@
 done

 if [ -z "$SUMMARY_OK" ] ; then SUMMARY_OK=" none"; fi
+if [ -z "$SUMMARY_SKIP" ] ; then SUMMARY_SKIP=" none"; fi
 if [ -z "$SUMMARY_FAIL" ] ; then SUMMARY_FAIL=" none"; fi
 echo "Test sets succeeded:$SUMMARY_OK."
+echo "Test sets skipped:$SUMMARY_SKIP."
 echo "Test sets failed:$SUMMARY_FAIL."

 # remove trap handler