Re: [Openvpn-devel] [PATCH] show extra info for OpenSSL errors
Hi,
This looks good except that the format of the log could be kept closer to
the current one:
On Fri, Jul 7, 2023 at 2:59 PM Arne Schwabe wrote:
> This also shows the extra data from the OpenSSL error function that
> can contain extra information. For example, the command
>
> openvpn --providers vollbit
>
> will print out (on macOS):
>
> OpenSSLerror:12800067:DSO support routines::could not load the shared
> library:filename(/opt/homebrew/Cellar/openssl@3/3.1.1_1/lib/ossl-modules/vollbit.dylib):
> dlopen(/opt/homebrew/Cellar/openssl@3/3.1.1_1/lib/ossl-modules/vollbit.dylib,
> 0x0002): tried:
> '/opt/homebrew/Cellar/openssl@3/3.1.1_1/lib/ossl-modules/vollbit.dylib'
> (no such file),
> '/System/Volumes/Preboot/Cryptexes/OS/opt/homebrew/Cellar/openssl@3/3.1.1_1/lib/ossl-modules/vollbit.dylib'
> (no such file),
> '/opt/homebrew/Cellar/openssl@3/3.1.1_1/lib/ossl-modules/vollbit.dylib'
> (no such file)
>
> Change-Id: Ic2ee89937dcd85721bcacd1b700a20c640364f80
> Signed-off-by: Arne Schwabe
> ---
> src/openvpn/crypto_openssl.c | 20 ++--
> src/openvpn/openssl_compat.h | 12
> 2 files changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
> index b043bb95e..d6916fc9b 100644
> --- a/src/openvpn/crypto_openssl.c
> +++ b/src/openvpn/crypto_openssl.c
> @@ -238,9 +238,16 @@ void
> crypto_print_openssl_errors(const unsigned int flags)
> {
> unsigned long err = 0;
> +int line, errflags;
> +const char *file, *data, *func;
>
> -while ((err = ERR_get_error()))
> +while ((err = ERR_get_error_all(, , , ,
> )) != 0)
> {
> +if (!(errflags & ERR_TXT_STRING))
> +{
> +data = "";
> +}
> +
> /* Be more clear about frequently occurring "no shared cipher"
> error */
> if (ERR_GET_REASON(err) == SSL_R_NO_SHARED_CIPHER)
> {
> @@ -258,7 +265,16 @@ crypto_print_openssl_errors(const unsigned int flags)
> "tls-version-min 1.0 to the client configuration to use
> TLS 1.0+ "
> "instead of TLS 1.0 only");
> }
> -msg(flags, "OpenSSL: %s", ERR_error_string(err, NULL));
> +
> +/* print file and line if verb >=8 */
> +if (!check_debug_level(D_TLS_DEBUG_MED))
> +{
> +msg(flags, "OpenSSL%s:%s", ERR_error_string(err, NULL), data);
>
Better to retain the ':" after "OpenSSL" in case anyone is parsing the logs
msg(flags, "OpenSSL: %s:%s, ERR_error_string(err, NULL), data);
> +}
> +else
> +{
> +msg(flags, "OpenSSL (%s:%d %s): %s:%s", file, line, func,
> ERR_error_string(err, NULL), data);
>
Again, I think we could keep the same order as above with additional info
(file:line:func) moved to the end with a consistent use of the delimiter ':'
msg(flags, "OpenSSL: %s:%s:%s:%s:%s, ERR_error_string(err, NULL), data,
file, line, func);
> +}
> }
> }
>
> diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
> index ffb64adf6..736ce1bd5 100644
> --- a/src/openvpn/openssl_compat.h
> +++ b/src/openvpn/openssl_compat.h
> @@ -43,6 +43,7 @@
> #include
> #include
> #include
> +#include
>
> /* Functionality missing in 1.1.0 */
> #if OPENSSL_VERSION_NUMBER < 0x10101000L &&
> !defined(ENABLE_CRYPTO_WOLFSSL)
> @@ -799,6 +800,17 @@ EVP_MD_free(const EVP_MD *md)
> /* OpenSSL 1.1.1 and lower use only const EVP_MD, nothing to free */
> }
>
> +static inline unsigned long
> +ERR_get_error_all(const char **file, int *line,
> + const char **func,
> + const char **data, int *flags)
> +{
> +static const char *empty = "";
> +*func = empty;
> +long err = ERR_get_error_line_data(file, line, data, flags);
>
unsigned long err =
> +return err;
> +}
> +
> #endif /* OPENSSL_VERSION_NUMBER < 0x3000L */
>
> #endif /* OPENSSL_COMPAT_H_ */
> --
>
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 3/4] Check if the -wrap argument is actually supported by the platform's ld
On Wed, Jul 12, 2023 at 01:10:53PM +0200, Frank Lichtenheld wrote: > On Wed, Jul 12, 2023 at 11:55:29AM +0200, Arne Schwabe wrote: > > This avoids build errors on macOS. Also the test_tls_crypt command works > > just fine on FreeBSD with its linkers, so do not make that test Linux only. > > > > Patch v2: allow running with old cmake version (cmake 3 on RHEL7 with EPEL > > is only 3.17) > > Patch v3: add OPTIONAL keyword to Incldue required by some cmake versions > > Yeah, I also have no idea why you didn't need that for your CMake. Anyway, > it is correct according to the documentation. And it is definitely required > on Ubuntu 20.04. > > Tested on Ubuntu 20.04 (cmake 3.16.3), Ubuntu 22.04 (cmake 3.22.1), and > checked build results for GHA. > > Acked-by: Frank Lichtenheld Note: please merge after "Mock openvpn_exece on win32 also for test_tls_crypt" to avoid broken MinGW builds. Regards, -- Frank Lichtenheld ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v2] Mock openvpn_exece on win32 also for test_tls_crypt
On Wed, Jul 12, 2023 at 11:54:12AM +0200, Arne Schwabe wrote: > This function is needed to commpile on win32 as run_command.c defines it > on Unix Linux but on windows it is defined in win32.c which pulls in too > many other unresolvable symbols. > > Patch v2: Also add mock_win32_execve.c to automake files I checked "make distdir" and the file now appears in there. Please note that this should be merged BEFORE "Check if the -wrap argument is actually supported by the platform's ld" since it fixes the MinGW build issue that arises from that. Acked-by: Frank Lichtenheld -- Frank Lichtenheld ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 3/4] Check if the -wrap argument is actually supported by the platform's ld
On Wed, Jul 12, 2023 at 11:55:29AM +0200, Arne Schwabe wrote: > This avoids build errors on macOS. Also the test_tls_crypt command works > just fine on FreeBSD with its linkers, so do not make that test Linux only. > > Patch v2: allow running with old cmake version (cmake 3 on RHEL7 with EPEL > is only 3.17) > Patch v3: add OPTIONAL keyword to Incldue required by some cmake versions Yeah, I also have no idea why you didn't need that for your CMake. Anyway, it is correct according to the documentation. And it is definitely required on Ubuntu 20.04. Tested on Ubuntu 20.04 (cmake 3.16.3), Ubuntu 22.04 (cmake 3.22.1), and checked build results for GHA. Acked-by: Frank Lichtenheld > Change-Id: Id26676bdc576c7d3d6726afa43fe6c7a397c579b > Signed-off-by: Arne Schwabe > --- > CMakeLists.txt | 15 +++ > 1 file changed, 11 insertions(+), 4 deletions(-) -- Frank Lichtenheld ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v3 3/4] Check if the -wrap argument is actually supported by the platform's ld
This avoids build errors on macOS. Also the test_tls_crypt command works
just fine on FreeBSD with its linkers, so do not make that test Linux only.
Patch v2: allow running with old cmake version (cmake 3 on RHEL7 with EPEL
is only 3.17)
Patch v3: add OPTIONAL keyword to Incldue required by some cmake versions
Change-Id: Id26676bdc576c7d3d6726afa43fe6c7a397c579b
Signed-off-by: Arne Schwabe
---
CMakeLists.txt | 15 +++
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 2d0cd5dd0..7dae6655d 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -16,6 +16,7 @@ find_package(PkgConfig REQUIRED)
include(CheckSymbolExists)
include(CheckIncludeFiles)
include(CheckCCompilerFlag)
+include(CheckLinkerFlag OPTIONAL)
include(CheckTypeSize)
include(CheckStructHasMember)
include(CTest)
@@ -564,18 +565,24 @@ if (BUILD_TESTING)
)
endif ()
-if (NOT MSVC)
-# MSVC does not support --wrap
+# MSVC and Apple's LLVM ld do not support --wrap
+# This test requires cmake >= 3.18, so check if check_linker_flag is
+# available
+if (COMMAND check_linker_flag)
+check_linker_flag(C -Wl,--wrap=parse_line LD_SUPPORTS_WRAP)
+endif()
+
+if (${LD_SUPPORTS_WRAP})
list(APPEND unit_tests
"test_argv"
+"test_tls_crypt"
)
endif ()
-# These tests work on only on Linux since they depend on special linker
features
+# These tests work on only on Linux since they depend on special Linux
features
if (${CMAKE_SYSTEM_NAME} STREQUAL "Linux")
list(APPEND unit_tests
"test_networking"
-"test_tls_crypt"
)
endif ()
--
2.39.2 (Apple Git-143)
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v2] Mock openvpn_exece on win32 also for test_tls_crypt
This function is needed to commpile on win32 as run_command.c defines it
on Unix Linux but on windows it is defined in win32.c which pulls in too
many other unresolvable symbols.
Patch v2: Also add mock_win32_execve.c to automake files
Change-Id: I8c8fe298eb30e211279f3fc010584b9d3bc14b4a
Signed-off-by: Arne Schwabe
---
CMakeLists.txt | 2 ++
tests/unit_tests/openvpn/Makefile.am | 3 +-
tests/unit_tests/openvpn/mock_win32_execve.c | 37
tests/unit_tests/openvpn/test_pkt.c | 8 -
4 files changed, 41 insertions(+), 9 deletions(-)
create mode 100644 tests/unit_tests/openvpn/mock_win32_execve.c
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 597dc9074..2d0cd5dd0 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -679,6 +679,7 @@ if (BUILD_TESTING)
)
target_sources(test_pkt PRIVATE
+tests/unit_tests/openvpn/mock_win32_execve.c
src/openvpn/argv.c
src/openvpn/base64.c
src/openvpn/crypto_mbedtls.c
@@ -740,6 +741,7 @@ if (BUILD_TESTING)
-Wl,--wrap=buffer_write_file
-Wl,--wrap=rand_bytes)
target_sources(test_tls_crypt PRIVATE
+tests/unit_tests/openvpn/mock_win32_execve.c
src/openvpn/argv.c
src/openvpn/base64.c
src/openvpn/crypto_mbedtls.c
diff --git a/tests/unit_tests/openvpn/Makefile.am
b/tests/unit_tests/openvpn/Makefile.am
index dbf658ac7..6b56f8480 100644
--- a/tests/unit_tests/openvpn/Makefile.am
+++ b/tests/unit_tests/openvpn/Makefile.am
@@ -83,7 +83,7 @@ packet_id_testdriver_SOURCES = test_packet_id.c mock_msg.c
mock_msg.h \
pkt_testdriver_CFLAGS = @TEST_CFLAGS@ \
-I$(top_srcdir)/include -I$(top_srcdir)/src/compat
-I$(top_srcdir)/src/openvpn
pkt_testdriver_LDFLAGS = @TEST_LDFLAGS@
-pkt_testdriver_SOURCES = test_pkt.c mock_msg.c mock_msg.h \
+pkt_testdriver_SOURCES = test_pkt.c mock_msg.c mock_msg.h mock_win32_execve.c \
$(top_srcdir)/src/openvpn/argv.c \
$(top_srcdir)/src/openvpn/base64.c \
$(top_srcdir)/src/openvpn/buffer.c \
@@ -110,6 +110,7 @@ tls_crypt_testdriver_LDFLAGS = @TEST_LDFLAGS@ \
-Wl,--wrap=parse_line \
-Wl,--wrap=rand_bytes
tls_crypt_testdriver_SOURCES = test_tls_crypt.c mock_msg.c mock_msg.h \
+mock_win32_execve.c \
$(top_srcdir)/src/openvpn/argv.c \
$(top_srcdir)/src/openvpn/base64.c \
$(top_srcdir)/src/openvpn/buffer.c \
diff --git a/tests/unit_tests/openvpn/mock_win32_execve.c
b/tests/unit_tests/openvpn/mock_win32_execve.c
new file mode 100644
index 0..4d37ebe33
--- /dev/null
+++ b/tests/unit_tests/openvpn/mock_win32_execve.c
@@ -0,0 +1,37 @@
+/*
+ * OpenVPN -- An application to securely tunnel IP networks
+ * over a single TCP/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
+ * Copyright (C) 2023 Arne Schwabe
+ *
+ *
+ * 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.
+ */
+
+#include "config.h"
+#include "syshead.h"
+
+#include "win32.h"
+
+#ifdef _WIN32
+int
+openvpn_execve(const struct argv *a, const struct env_set *es, const unsigned
int flags)
+{
+ASSERT(0);
+}
+#endif
diff --git a/tests/unit_tests/openvpn/test_pkt.c
b/tests/unit_tests/openvpn/test_pkt.c
index 5a53f702e..9f49ee7bd 100644
--- a/tests/unit_tests/openvpn/test_pkt.c
+++ b/tests/unit_tests/openvpn/test_pkt.c
@@ -69,14 +69,6 @@ print_link_socket_actual(const struct link_socket_actual
*act, struct gc_arena *
return "dummy print_link_socket_actual from unit test";
}
-#ifdef _WIN32
-int
-openvpn_execve(const struct argv *a, const struct env_set *es, const unsigned
int flags)
-{
-ASSERT(0);
-}
-#endif
-
struct test_pkt_context {
struct tls_auth_standalone tas_tls_auth;
struct tls_auth_standalone tas_crypt;
--
2.39.2 (Apple Git-143)
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] Ignore Ipv6 route delete request on Android and set ipv4 verbosity to 7
Android has no facility nor need one to delete routes as routes are
automatically cleaned up when the tun interface is closed. Also adjust
the IPv4 message to be only shown and verb 7 and rephrase the message.
Change-Id: If8f920d378c31e9ea773ce1f56f3df50f1ec36cd
Signed-off-by: Arne Schwabe
---
src/openvpn/route.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/src/openvpn/route.c b/src/openvpn/route.c
index 0b369da44..6028de9c2 100644
--- a/src/openvpn/route.c
+++ b/src/openvpn/route.c
@@ -2302,8 +2302,9 @@ delete_route(struct route_ipv4 *r,
openvpn_execve_check(, es, 0, "ERROR: OpenBSD/NetBSD route delete
command failed");
#elif defined(TARGET_ANDROID)
-msg(M_NONFATAL, "Sorry, deleting routes on Android is not possible. The
VpnService API allows routes to be set on connect only.");
-
+msg(D_ROUTE_DEBUG, "Deleting routes on Android is not possible/not "
+ "needed. The VpnService API allows routes to be set "
+ "on connect only and will clean up automatically.");
#elif defined(TARGET_AIX)
{
@@ -2490,7 +2491,10 @@ delete_route_ipv6(const struct route_ipv6 *r6, const
struct tuntap *tt,
network, r6->netbits, gateway);
argv_msg(D_ROUTE, );
openvpn_execve_check(, es, 0, "ERROR: AIX route add command failed");
-
+#elif defined(TARGET_ANDROID)
+msg(D_ROUTE_DEBUG, "Deleting routes on Android is not possible/not "
+ "needed. The VpnService API allows routes to be set "
+ "on connect only and will clean up automatically.");
#else /* if defined(TARGET_LINUX) */
msg(M_FATAL, "Sorry, but I don't know how to do 'route ipv6' commands on
this operating system. Try putting your routes in a --route-down script");
#endif /* if defined(TARGET_LINUX) */
--
2.39.2 (Apple Git-143)
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
