openvpn-devel Mailing List for OpenVPN

Am 30.06.23 um 15:31 schrieb Maximilian Fillinger: > The grammar in the 3rd sentence in the comment below is messed up. (I think I understand it, but I'm not sure.) > >> + if (session->opt->verify_hash_no_ca) >> + { >> + /* >> + * If we decide to verify the peer certificate based on the fingerprint >> + * we ignore wrong dates and the certificate not being trusted. >> + * Any other problem with the certificate (wrong key, bad cert,...) >> + * will still trigger an error. >> + * Clearing these flags relies on verify_cert will later rejecting a >> + * certificate that has no matching fingerprint. >> + */ >> + uint32_t flags_ignore = MBEDTLS_X509_BADCERT_NOT_TRUSTED >> + | MBEDTLS_X509_BADCERT_EXPIRED >> + | MBEDTLS_X509_BADCERT_FUTURE; >> + *flags = *flags & ~flags_ignore; >> + } >> + > > Also, this comment is copied verbatim from Jason's commit 423ced962d which has been reverted. I'm not a lawyer, but since comments are relatively easy to rephrase, I think it's better to do that. My suggestion: The comment is already mine. Jason never included an mBed TLS implementation. I attributed the commit to Jason but some of the code and this comment is already written by me. Arne 

The grammar in the 3rd sentence in the comment below is messed up. (I think I understand it, but I'm not sure.) > + if (session->opt->verify_hash_no_ca) > + { > + /* > + * If we decide to verify the peer certificate based on the fingerprint > + * we ignore wrong dates and the certificate not being trusted. > + * Any other problem with the certificate (wrong key, bad cert,...) > + * will still trigger an error. > + * Clearing these flags relies on verify_cert will later rejecting a > + * certificate that has no matching fingerprint. > + */ > + uint32_t flags_ignore = MBEDTLS_X509_BADCERT_NOT_TRUSTED > + | MBEDTLS_X509_BADCERT_EXPIRED > + | MBEDTLS_X509_BADCERT_FUTURE; > + *flags = *flags & ~flags_ignore; > + } > + Also, this comment is copied verbatim from Jason's commit 423ced962d which has been reverted. I'm not a lawyer, but since comments are relatively easy to rephrase, I think it's better to do that. My suggestion: /* * If we verify the peer certificate based only on the fingerprint, * we ignore flags regarding the certificate's validity period and * the certificate being untrusted (because we don't have a CA to * check against). * Any other flags will still trigger an error. * * If the certificate's fingerprint doesn't match, it will be rejected * by verify_cert later. */ 

Use the casting variants of mock(). Using the mock_ptr_type fixes an existing bug where test_tls_crypt.c couldn't build in MinGW 32bit: test_tls_crypt.c:127:27: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast] 127 | const char *pem_str = (const char *) mock(); Change-Id: I6c03313b8677fa07c07e718b1f85f7efd3c4dea8 Signed-off-by: Frank Lichtenheld <fr...@li...> --- tests/unit_tests/openvpn/test_tls_crypt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit_tests/openvpn/test_tls_crypt.c b/tests/unit_tests/openvpn/test_tls_crypt.c index 8bed042f..ed7c7948 100644 --- a/tests/unit_tests/openvpn/test_tls_crypt.c +++ b/tests/unit_tests/openvpn/test_tls_crypt.c @@ -116,7 +116,7 @@ __wrap_buffer_write_file(const char *filename, const struct buffer *buf) check_expected(filename); check_expected(pem); - return mock(); + return mock_type(bool); } struct buffer @@ -124,7 +124,7 @@ __wrap_buffer_read_from_file(const char *filename, struct gc_arena *gc) { check_expected(filename); - const char *pem_str = (const char *) mock(); + const char *pem_str = mock_ptr_type(const char *); struct buffer ret = alloc_buf_gc(strlen(pem_str) + 1, gc); buf_write(&ret, pem_str, strlen(pem_str) + 1); -- 2.34.1 

On Thu, Jun 29, 2023 at 11:56:07PM +0200, Arne Schwabe wrote: > On my system python3 is the macOS system python3 while rst2html has > > #!/opt/homebrew/opt/python@3.9/bin/python3.9 > > as its first line. Running that with a different python results in missing > python modules. So directly execute the rst2html script instead. Acked-By: Frank Lichtenheld <fr...@li...> -- Frank Lichtenheld 

On Thu, Jun 29, 2023 at 11:56:11PM +0200, Arne Schwabe wrote: > the funktion is_on_link is not used on FreeBSD and triggers a > warning/error (-Werror) on FreeBSD. > > Change-Id: I6757d6509ff3ff522d6de417372a21e73ccca3ba > Signed-off-by: Arne Schwabe <ar...@rf...> > --- > src/openvpn/route.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/src/openvpn/route.c b/src/openvpn/route.c > index d18acd016..2180b7d1a 100644 > --- a/src/openvpn/route.c > +++ b/src/openvpn/route.c > @@ -1541,13 +1541,15 @@ local_route(in_addr_t network, > return LR_NOMATCH; > } > > -/* Return true if the "on-link" form of the route should be used. This is when the gateway for a > +/* Return true if the "on-link" form of the route should be used. This is when the gateway for > * a route is specified as an interface rather than an address. */ > +#ifndef TARGET_FREEBSD The actual condition seems to be #if defined(TARGET_LINUX) || defined(_WIN32) || defined(TARGET_DARWIN) > static inline bool > is_on_link(const int is_local_route, const unsigned int flags, const struct route_gateway_info *rgi) > { > return rgi && (is_local_route == LR_MATCH || ((flags & ROUTE_REF_GW) && (rgi->flags & RGI_ON_LINK))); > } > +#endif > > bool > add_route(struct route_ipv4 *r, Regards, -- Frank Lichtenheld 

On Thu, Jun 29, 2023 at 11:56:08PM +0200, Arne Schwabe wrote: > The -Wno-stringop-truncation flag is only supported by some GCC versions > and not by Clang (macOS, FreeBSD) at all. > > Move the includes to the top the file to have them available when running > the check_c_compiler_flag. Acked-by: Frank Lichtenheld <fr...@li...> -- Frank Lichtenheld 

On Thu, Jun 29, 2023 at 11:56:10PM +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. NAK. Breaks build on mingw. Will investigate why. Regards, -- Frank Lichtenheld 

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. Change-Id: Id26676bdc576c7d3d6726afa43fe6c7a397c579b Signed-off-by: Arne Schwabe <ar...@rf...> --- CMakeLists.txt | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index acebbb73c..d2445b414 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -16,6 +16,7 @@ find_package(PkgConfig REQUIRED) include(CheckSymbolExists) include(CheckIncludeFiles) include(CheckCCompilerFlag) +include(CheckLinkerFlag) include(CheckTypeSize) include(CheckStructHasMember) include(CTest) @@ -560,18 +561,20 @@ if (BUILD_TESTING) ) endif () - if (NOT MSVC) - # MSVC does not support --wrap + # MSVC and Apple's LLVM ld do not support --wrap + check_linker_flag(C -Wl,--wrap=parse_line LD_SUPPORTS_WRAP) + + 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) 

The patches to the cmake files did a lot of improvements but broke compiling on macOS and FreeBSD. This patch set restores the ability to compile again with these two platforms. Arne Schwabe (4): Do not blindly assume python3 is also the interpreter that runs rst2html [CMake] Only add -Wno-stringop-truncation on supported compilers Check if the -wrap argument is actually supported by the platform's ld Avoid unused function warning/error on FreeBSD CMakeLists.txt | 33 +++++++++++++++++++++------------ doc/CMakeLists.txt | 8 ++++---- src/openvpn/route.c | 4 +++- 3 files changed, 28 insertions(+), 17 deletions(-) -- 2.39.2 (Apple Git-143) 

The -Wno-stringop-truncation flag is only supported by some GCC versions and not by Clang (macOS, FreeBSD) at all. Move the includes to the top the file to have them available when running the check_c_compiler_flag. Change-Id: I452bc4ee935d13f8e9095d0a31805a3bbaff0cec Signed-off-by: Arne Schwabe <ar...@rf...> --- CMakeLists.txt | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 3cbba5a38..acebbb73c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -12,6 +12,14 @@ project(openvpn) # and OpenSSL having version 1.1.1+ and generally does not offer the same # configurability like autoconf +find_package(PkgConfig REQUIRED) +include(CheckSymbolExists) +include(CheckIncludeFiles) +include(CheckCCompilerFlag) +include(CheckTypeSize) +include(CheckStructHasMember) +include(CTest) + option(UNSUPPORTED_BUILDS "Allow unsupported builds" OFF) if (NOT WIN32 AND NOT ${UNSUPPORTED_BUILDS}) @@ -70,7 +78,12 @@ else () set(CMAKE_CXX_FLAGS_RELEASE "-O2") set(CMAKE_C_FLAGS_DEBUG "-g -O1") set(CMAKE_CXX_FLAGS_DEBUG "-g -O1") - add_compile_options(-Wall -Wuninitialized -Wno-stringop-truncation) + add_compile_options(-Wall -Wuninitialized) + check_c_compiler_flag(-Wno-stringop-truncation NoStringOpTruncation) + + if (${NoStringOpTruncation}) + add_compile_options(-Wno-stringop-truncation) + endif() # We are not ready for this #add_compile_options(-Wconversion -Wno-sign-conversion -Wsign-compare) if (USE_WERROR) @@ -78,13 +91,6 @@ else () endif () endif () -find_package(PkgConfig REQUIRED) -include(CheckSymbolExists) -include(CheckIncludeFiles) -include(CheckTypeSize) -include(CheckStructHasMember) -include(CTest) - find_program(PYTHON NAMES python3 python) execute_process( COMMAND ${PYTHON} ${CMAKE_CURRENT_SOURCE_DIR}/contrib/cmake/parse-version.m4.py ${CMAKE_CURRENT_SOURCE_DIR}/version.m4 -- 2.39.2 (Apple Git-143) 

the funktion is_on_link is not used on FreeBSD and triggers a warning/error (-Werror) on FreeBSD. Change-Id: I6757d6509ff3ff522d6de417372a21e73ccca3ba Signed-off-by: Arne Schwabe <ar...@rf...> --- src/openvpn/route.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/openvpn/route.c b/src/openvpn/route.c index d18acd016..2180b7d1a 100644 --- a/src/openvpn/route.c +++ b/src/openvpn/route.c @@ -1541,13 +1541,15 @@ local_route(in_addr_t network, return LR_NOMATCH; } -/* Return true if the "on-link" form of the route should be used. This is when the gateway for a +/* Return true if the "on-link" form of the route should be used. This is when the gateway for * a route is specified as an interface rather than an address. */ +#ifndef TARGET_FREEBSD static inline bool is_on_link(const int is_local_route, const unsigned int flags, const struct route_gateway_info *rgi) { return rgi && (is_local_route == LR_MATCH || ((flags & ROUTE_REF_GW) && (rgi->flags & RGI_ON_LINK))); } +#endif bool add_route(struct route_ipv4 *r, -- 2.39.2 (Apple Git-143) 

On my system python3 is the macOS system python3 while rst2html has #!/opt/homebrew/opt/python@3.9/bin/python3.9 as its first line. Running that with a different python results in missing python modules. So directly execute the rst2html script instead. Change-Id: I7e27ae031179c91cc1bca8122caf2453d6396ec0 Signed-off-by: Arne Schwabe <ar...@rf...> --- doc/CMakeLists.txt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/CMakeLists.txt b/doc/CMakeLists.txt index d38805513..2fba80bbd 100644 --- a/doc/CMakeLists.txt +++ b/doc/CMakeLists.txt @@ -50,13 +50,13 @@ if (_GENERATE_HTML_DOC) list(APPEND ALL_DOCS openvpn.8.html openvpn-examples.5.html) add_custom_command( OUTPUT openvpn.8.html - COMMAND ${PYTHON} ${RST2HTML} ${RST_FLAGS} ${CMAKE_CURRENT_SOURCE_DIR}/openvpn.8.rst ${CMAKE_CURRENT_BINARY_DIR}/openvpn.8.html + COMMAND ${RST2HTML} ${RST_FLAGS} ${CMAKE_CURRENT_SOURCE_DIR}/openvpn.8.rst ${CMAKE_CURRENT_BINARY_DIR}/openvpn.8.html MAIN_DEPENDENCY openvpn.8.rst DEPENDS ${OPENVPN_SECTIONS} ) add_custom_command( OUTPUT openvpn-examples.5.html - COMMAND ${PYTHON} ${RST2HTML} ${RST_FLAGS} ${CMAKE_CURRENT_SOURCE_DIR}/openvpn-examples.5.rst ${CMAKE_CURRENT_BINARY_DIR}/openvpn-examples.5.html + COMMAND ${RST2HTML} ${RST_FLAGS} ${CMAKE_CURRENT_SOURCE_DIR}/openvpn-examples.5.rst ${CMAKE_CURRENT_BINARY_DIR}/openvpn-examples.5.html MAIN_DEPENDENCY openvpn-examples.5.rst DEPENDS ${OPENVPN_EXAMPLES_SECTIONS} ) @@ -65,13 +65,13 @@ if (_GENERATE_MAN_DOC) list(APPEND ALL_DOCS openvpn.8 openvpn-examples.5) add_custom_command( OUTPUT openvpn.8 - COMMAND ${PYTHON} ${RST2MAN} ${RST_FLAGS} ${CMAKE_CURRENT_SOURCE_DIR}/openvpn.8.rst ${CMAKE_CURRENT_BINARY_DIR}/openvpn.8 + COMMAND ${RST2MAN} ${RST_FLAGS} ${CMAKE_CURRENT_SOURCE_DIR}/openvpn.8.rst ${CMAKE_CURRENT_BINARY_DIR}/openvpn.8 MAIN_DEPENDENCY openvpn.8.rst DEPENDS ${OPENVPN_SECTIONS} ) add_custom_command( OUTPUT openvpn-examples.5 - COMMAND ${PYTHON} ${RST2MAN} ${RST_FLAGS} ${CMAKE_CURRENT_SOURCE_DIR}/openvpn-examples.5.rst ${CMAKE_CURRENT_BINARY_DIR}/openvpn-examples.5 + COMMAND ${RST2MAN} ${RST_FLAGS} ${CMAKE_CURRENT_SOURCE_DIR}/openvpn-examples.5.rst ${CMAKE_CURRENT_BINARY_DIR}/openvpn-examples.5 MAIN_DEPENDENCY openvpn-examples.5.rst DEPENDS ${OPENVPN_EXAMPLES_SECTIONS} ) -- 2.39.2 (Apple Git-143) 

This is implements --peer-fingerprint command to support OpenVPN authentication without involving a PKI. The current implementation in OpenVPN for peer fingerprint has been already extensively rewritten from the original submission from Jason [1]. The commit preserved the original author since it was based on Jason code/idea. The current code uses two commits to prepare the --peer-fingerprint solution as which choose to use a simple to use --peer-fingerprint directive instead of using using a --tls-verify script like the v1 of the patch proposed. The two commit preparing this are: - Extend verify-hash to allow multiple hashes - Implement peer-fingerprint to check fingerprint of peer certificate This perparing patches make this actual patch quite short. There are some lines in this patch that bear some similarity to the ones like if (!preverify_ok && !session->opt->verify_hash_no_ca) vs if (!preverify_ok && !session->opt->ca_file_none) But these similarities are one line fragments and dictated by the surrounding style and program flow, so even a complete black box implementation will likely end up with the same lines. [1] https://www.mail-archive.com/ope...@li.../msg16781.html Change-Id: Ie74c3d606c5429455c293c367462244566a936e3 Signed-off-by: Arne Schwabe <ar...@rf...> --- src/openvpn/init.c | 1 + src/openvpn/options.c | 26 +++++++++++++------------- src/openvpn/options.h | 1 + src/openvpn/ssl_common.h | 1 + src/openvpn/ssl_verify_mbedtls.c | 16 ++++++++++++++++ src/openvpn/ssl_verify_openssl.c | 2 +- 6 files changed, 33 insertions(+), 14 deletions(-) diff --git a/src/openvpn/init.c b/src/openvpn/init.c index c023b33c6..d358ad003 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -3347,6 +3347,7 @@ do_init_crypto_tls(struct context *c, const unsigned int flags) to.verify_hash = options->verify_hash; to.verify_hash_algo = options->verify_hash_algo; to.verify_hash_depth = options->verify_hash_depth; + to.verify_hash_no_ca = options->verify_hash_no_ca; #ifdef ENABLE_X509ALTUSERNAME memcpy(to.x509_username_field, options->x509_username_field, sizeof(to.x509_username_field)); #else diff --git a/src/openvpn/options.c b/src/openvpn/options.c index fe9285384..e4c596b89 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -2991,21 +2991,11 @@ options_postprocess_verify_ce(const struct options *options, else { #ifdef ENABLE_CRYPTO_MBEDTLS - if (!(options->ca_file)) - { - msg(M_USAGE, "You must define CA file (--ca)"); - } - if (options->ca_path) { msg(M_USAGE, "Parameter --capath cannot be used with the mbed TLS version version of OpenVPN."); } -#else /* ifdef ENABLE_CRYPTO_MBEDTLS */ - if ((!(options->ca_file)) && (!(options->ca_path))) - { - msg(M_USAGE, "You must define CA file (--ca) or CA path (--capath)"); - } -#endif +#endif /* ifdef ENABLE_CRYPTO_MBEDTLS */ if (pull) { @@ -3737,6 +3727,13 @@ options_postprocess_mutate(struct options *o, struct env_set *es) options_postprocess_http_proxy_override(o); } #endif + if (!o->ca_file && !o->ca_path && o->verify_hash + && o->verify_hash_depth == 0) + { + msg(M_INFO, "Using certificate fingerprint to verify peer (no CA " + "option set). "); + o->verify_hash_no_ca = true; + } if (o->config && streq(o->config, "stdin") && o->remap_sigusr1 == SIGHUP) { @@ -4032,8 +4029,11 @@ options_postprocess_filechecks(struct options *options) errs |= check_file_access_inline(options->dh_file_inline, CHKACC_FILE, options->dh_file, R_OK, "--dh"); - errs |= check_file_access_inline(options->ca_file_inline, CHKACC_FILE, - options->ca_file, R_OK, "--ca"); + if (!options->verify_hash_no_ca) + { + errs |= check_file_access_inline(options->ca_file_inline, CHKACC_FILE, + options->ca_file, R_OK, "--ca"); + } errs |= check_file_access_chroot(options->chroot_dir, CHKACC_FILE, options->ca_path, R_OK, "--capath"); diff --git a/src/openvpn/options.h b/src/openvpn/options.h index 95f1158a4..f5890b90f 100644 --- a/src/openvpn/options.h +++ b/src/openvpn/options.h @@ -604,6 +604,7 @@ struct options struct verify_hash_list *verify_hash; hash_algo_type verify_hash_algo; int verify_hash_depth; + bool verify_hash_no_ca; unsigned int ssl_flags; /* set to SSLF_x flags from ssl.h */ #ifdef ENABLE_PKCS11 diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h index c0b3caa71..27b029479 100644 --- a/src/openvpn/ssl_common.h +++ b/src/openvpn/ssl_common.h @@ -345,6 +345,7 @@ struct tls_options const char *remote_cert_eku; struct verify_hash_list *verify_hash; int verify_hash_depth; + bool verify_hash_no_ca; hash_algo_type verify_hash_algo; #ifdef ENABLE_X509ALTUSERNAME char *x509_username_field[MAX_PARMS]; diff --git a/src/openvpn/ssl_verify_mbedtls.c b/src/openvpn/ssl_verify_mbedtls.c index c9ef7a171..e3437f740 100644 --- a/src/openvpn/ssl_verify_mbedtls.c +++ b/src/openvpn/ssl_verify_mbedtls.c @@ -62,6 +62,22 @@ verify_callback(void *session_obj, mbedtls_x509_crt *cert, int cert_depth, struct buffer cert_fingerprint = x509_get_sha256_fingerprint(cert, &gc); cert_hash_remember(session, cert_depth, &cert_fingerprint); + if (session->opt->verify_hash_no_ca) + { + /* + * If we decide to verify the peer certificate based on the fingerprint + * we ignore wrong dates and the certificate not being trusted. + * Any other problem with the certificate (wrong key, bad cert,...) + * will still trigger an error. + * Clearing these flags relies on verify_cert will later rejecting a + * certificate that has no matching fingerprint. + */ + uint32_t flags_ignore = MBEDTLS_X509_BADCERT_NOT_TRUSTED + | MBEDTLS_X509_BADCERT_EXPIRED + | MBEDTLS_X509_BADCERT_FUTURE; + *flags = *flags & ~flags_ignore; + } + /* did peer present cert which was signed by our root cert? */ if (*flags != 0) { diff --git a/src/openvpn/ssl_verify_openssl.c b/src/openvpn/ssl_verify_openssl.c index ac36f09db..e24ce4e4a 100644 --- a/src/openvpn/ssl_verify_openssl.c +++ b/src/openvpn/ssl_verify_openssl.c @@ -67,7 +67,7 @@ verify_callback(int preverify_ok, X509_STORE_CTX *ctx) cert_hash_remember(session, X509_STORE_CTX_get_error_depth(ctx), &cert_hash); /* did peer present cert which was signed by our root cert? */ - if (!preverify_ok) + if (!preverify_ok && !session->opt->verify_hash_no_ca) { /* get the X509 name */ char *subject = x509_get_subject(current_cert, &gc); -- 2.39.2 (Apple Git-143) 

Am 29.06.23 um 13:39 schrieb Arne Schwabe: > The -Wno-stringop-truncation flag is only supported by some GCC versions > and not by Clang (macOS, FreeBSD) at all. > > Change-Id: I452bc4ee935d13f8e9095d0a31805a3bbaff0cec Ingore this version. 

The -Wno-stringop-truncation flag is only supported by some GCC versions and not by Clang (macOS, FreeBSD) at all. Change-Id: I452bc4ee935d13f8e9095d0a31805a3bbaff0cec Signed-off-by: Arne Schwabe <ar...@rf...> --- CMakeLists.txt | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 3cbba5a38..ec0915bb0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -12,6 +12,14 @@ project(openvpn) # and OpenSSL having version 1.1.1+ and generally does not offer the same # configurability like autoconf +find_package(PkgConfig REQUIRED) +include(CheckSymbolExists) +include(CheckIncludeFiles) +include(CheckCCompilerFlag) +include(CheckTypeSize) +include(CheckStructHasMember) +include(CTest) + option(UNSUPPORTED_BUILDS "Allow unsupported builds" OFF) if (NOT WIN32 AND NOT ${UNSUPPORTED_BUILDS}) @@ -70,7 +78,10 @@ else () set(CMAKE_CXX_FLAGS_RELEASE "-O2") set(CMAKE_C_FLAGS_DEBUG "-g -O1") set(CMAKE_CXX_FLAGS_DEBUG "-g -O1") - add_compile_options(-Wall -Wuninitialized -Wno-stringop-truncation) + check_c_compiler_flag(-Wno-stringop-truncation NoStringOpTruncation) + if (${NoStringOpTruncation}) + add_compile_options(-Wall -Wuninitialized ) + endif() # We are not ready for this #add_compile_options(-Wconversion -Wno-sign-conversion -Wsign-compare) if (USE_WERROR) @@ -78,13 +89,6 @@ else () endif () endif () -find_package(PkgConfig REQUIRED) -include(CheckSymbolExists) -include(CheckIncludeFiles) -include(CheckTypeSize) -include(CheckStructHasMember) -include(CTest) - find_program(PYTHON NAMES python3 python) execute_process( COMMAND ${PYTHON} ${CMAKE_CURRENT_SOURCE_DIR}/contrib/cmake/parse-version.m4.py ${CMAKE_CURRENT_SOURCE_DIR}/version.m4 -- 2.39.2 (Apple Git-143) 

On Tue, Jun 27, 2023 at 08:35:33PM +0200, Gert Doering wrote: > I have tested this by pushing to GHA (as there are quite significant > changes to build.yaml) - it seems to still pass, which is good :-) > > I do wonder why all unit tests now produce differently-named .exe > files (packet_id_testdriver.exe -> test_packet_id.exe etc)? Is this > a side effect of a new cmocka version, or...? No, there is no technical reason for this. I think we can freely choose the name. We probably just applied the usual naming we use in all our CMake code. While the autotools buildsystem uses a different naming scheme. The old MSVC build had no support for the UTs at all. > This patch also adds bundling of the CMake related files to tarballs > (which I missed in 1/5). Good :-) - besides this, there should be no > change to "autoconf based building", and there are also no code changes. > > Your patch has been applied to the master branch. > > commit e8881ec6dd63bd80ce05202573eac54ab8657fcb > Author: Frank Lichtenheld > Date: Tue Jun 20 15:53:07 2023 +0200 > > CMake: Add complete MinGW and MSVC build > > Signed-off-by: Frank Lichtenheld <fr...@li...> > Acked-by: Lev Stipakov <lst...@gm...> > Message-Id: <202...@li...> > URL: https://www.mail-archive.com/ope...@li.../msg26754.html > Signed-off-by: Gert Doering <ge...@gr...> Regards, -- Frank Lichtenheld 

On Tue, Jun 27, 2023 at 10:08:34PM +0200, Gert Doering wrote: > This patch touches a large number of files, but all the source code > changes are trivial (remove the "config-msvc.h" include). > > The larger change "get rid of the evil MSVC xml files" is very > welcome :-) - one of the changes confuses me, though - this recent > "tapctl.exe.manifest" inclusion in EXTRA_DIST, and now removing all > of it again. Was that intentional? That was a "rebase" bug. Somehow I managed to introduce a second EXTRA_DIST. So make dist might be slightly broken in the CMake commit. But this commit then fixes it, so the end-state is correct. > Github says "it still builds fine", and Lev has tested the resulting > binaries, good enough for me :-) > > I'm a bit unsure how to proceed with 2.6 - people are (trying to) build > this with MSVC, so pulling up that changeset will break their ways of > doing things. And it's quite large... OTOH, *not* pulling it in > means we're stuck with MSVC building for 2.6 "until it reaches end of > life", including regular updates to these XML files. > > Your patch has been applied to the master branch. > > commit 7fbb9484116dc5d9d618d74f1ee253cf6fef4101 > Author: Frank Lichtenheld > Date: Tue Jun 20 15:53:08 2023 +0200 > > Remove all traces of the previous MSVC build system > > Signed-off-by: Frank Lichtenheld <fr...@li...> > Acked-by: Lev Stipakov <lst...@gm...> > Message-Id: <202...@li...> > URL: https://www.mail-archive.com/search?l=mid&q=2...@li... > Signed-off-by: Gert Doering <ge...@gr...> Regards, -- Frank Lichtenheld 

On Wed, May 17, 2023 at 03:01:38PM +0200, Arne Schwabe wrote: > Am 15.02.23 um 13:31 schrieb David Sommerseth: > > > > OpenVPN 2.x is licensed under the GNU Public License v2.0 (GPL-2.0). > > This license has served us well in the past and we are not trying to > > change that.  However, changes in licenses of our dependencies put us in > > an unfortunate situation. > > So a good amount of time has passed and we got a lot of positive feedback to > the license agreement. We got 107 positive responses back contributors > including a positive one from Fox IT that covers all Fox It employers and a > positive one from OpenVPN Inc that cover all code that OpenVPN Inc owns. And > big thanks to everyone who said yes so far. > > > With that we are down to a small number of people (10) that are still > missing acknowledgment. > > > Note that the actions for the individual source are my opinion/suggestions. > [...] > - Josh Cepek <jos...@us...> > > No response at all to my emails. Commits were in 2013 and 2016 > > All seven commits look trivial to me. Need someone else to form an opinion > if this can be seen as trivial in total or not. > I have reviewed his patches and I agree that they are all individual bug fixes and are all individually trivial. Regards, -- Frank Lichtenheld 

From: Lev Stipakov <le...@op...> By default CMake links C runtime dynamically, which doesn't work on Windows 7, for example. This is not an issue with other openvpn binaries, since we bundle C runtime, but it is not yet available during installation. Change-Id: Ib2b014f075908e7db0d9115abaa2240e47fd27b9 Signed-off-by: Lev Stipakov <le...@op...> --- src/openvpnmsica/CMakeLists.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/openvpnmsica/CMakeLists.txt b/src/openvpnmsica/CMakeLists.txt index 683d38e0..6816b119 100644 --- a/src/openvpnmsica/CMakeLists.txt +++ b/src/openvpnmsica/CMakeLists.txt @@ -25,7 +25,10 @@ target_compile_options(openvpnmsica PRIVATE -D_UNICODE -UNTDDI_VERSION -D_WIN32_WINNT=_WIN32_WINNT_VISTA + "$<$<CONFIG:Release>:/MT>" + "$<$<CONFIG:Debug>:/MTd>" ) + target_link_libraries(openvpnmsica advapi32.lib ole32.lib msi.lib setupapi.lib iphlpapi.lib shell32.lib shlwapi.lib version.lib newdev.lib) if (MINGW) -- 2.23.0.windows.1 

Whatever it does... GHA and Lev agree that it's fine. (One thing I noticed is that the "msbuild" builds now also run the cmocka tests - this seems to be a new thing, and very welcome!) I have extended the commit message to include the cache thing. Your patch has been applied to the master branch. commit 66e33ee81d1d7fa3495ae3aad6e673766e296687 Author: Frank Lichtenheld Date: Tue Jun 20 15:53:10 2023 +0200 GHA: update to run-vcpkg@v11 Signed-off-by: Frank Lichtenheld <fr...@li...> Acked-by: Lev Stipakov <lst...@gm...> Message-Id: <202...@li...> URL: https://www.mail-archive.com/ope...@li.../msg26755.html Signed-off-by: Gert Doering <ge...@gr...> -- kind regards, Gert Doering 

Whatever this does, it only affecst Windows builds, and both GHA and Lev agree that it still works... Your patch has been applied to the master branch. commit 5e94e8de4bfaf6637124947a3489710b591e5e26 Author: Frank Lichtenheld Date: Tue Jun 20 15:53:09 2023 +0200 CMake: Add /Brepro to MSVC link options Signed-off-by: Frank Lichtenheld <fr...@li...> Acked-by: Lev Stipakov <lst...@gm...> Message-Id: <202...@li...> URL: https://www.mail-archive.com/ope...@li.../msg26757.html Signed-off-by: Gert Doering <ge...@gr...> -- kind regards, Gert Doering 

This patch touches a large number of files, but all the source code changes are trivial (remove the "config-msvc.h" include). The larger change "get rid of the evil MSVC xml files" is very welcome :-) - one of the changes confuses me, though - this recent "tapctl.exe.manifest" inclusion in EXTRA_DIST, and now removing all of it again. Was that intentional? Github says "it still builds fine", and Lev has tested the resulting binaries, good enough for me :-) I'm a bit unsure how to proceed with 2.6 - people are (trying to) build this with MSVC, so pulling up that changeset will break their ways of doing things. And it's quite large... OTOH, *not* pulling it in means we're stuck with MSVC building for 2.6 "until it reaches end of life", including regular updates to these XML files. Your patch has been applied to the master branch. commit 7fbb9484116dc5d9d618d74f1ee253cf6fef4101 Author: Frank Lichtenheld Date: Tue Jun 20 15:53:08 2023 +0200 Remove all traces of the previous MSVC build system Signed-off-by: Frank Lichtenheld <fr...@li...> Acked-by: Lev Stipakov <lst...@gm...> Message-Id: <202...@li...> URL: https://www.mail-archive.com/search?l=mid&q=2...@li... Signed-off-by: Gert Doering <ge...@gr...> -- kind regards, Gert Doering 

Acked-by: Gert Doering <ge...@gr...> Verified by building a tarball on 2.6, cert_data.h is now included. Your patch has been applied to the release/2.6 branch. commit 5eb84eb19749eb7e12f3becb36d6aaae89457b5b Author: Frank Lichtenheld Date: Wed Jun 21 14:58:42 2023 +0200 unit_tests: Add missing cert_data.h to source list for unit tests Signed-off-by: Frank Lichtenheld <fr...@li...> Acked-by: Gert Doering <ge...@gr...> Message-Id: <202...@li...> URL: https://www.mail-archive.com/ope...@li.../msg26765.html Signed-off-by: Gert Doering <ge...@gr...> -- kind regards, Gert Doering 

I have tested this by pushing to GHA (as there are quite significant changes to build.yaml) - it seems to still pass, which is good :-) I do wonder why all unit tests now produce differently-named .exe files (packet_id_testdriver.exe -> test_packet_id.exe etc)? Is this a side effect of a new cmocka version, or...? This patch also adds bundling of the CMake related files to tarballs (which I missed in 1/5). Good :-) - besides this, there should be no change to "autoconf based building", and there are also no code changes. Your patch has been applied to the master branch. commit e8881ec6dd63bd80ce05202573eac54ab8657fcb Author: Frank Lichtenheld Date: Tue Jun 20 15:53:07 2023 +0200 CMake: Add complete MinGW and MSVC build Signed-off-by: Frank Lichtenheld <fr...@li...> Acked-by: Lev Stipakov <lst...@gm...> Message-Id: <202...@li...> URL: https://www.mail-archive.com/ope...@li.../msg26754.html Signed-off-by: Gert Doering <ge...@gr...> -- kind regards, Gert Doering 

I understand that this is work in progress, does not change anything, and is not feature-complete either (but due to shared authorship, this is a separate commit). It is not packaged into tarballs either. According to one of Frank's latest mails, this patch series does not work with 2.6 - so I'm going to apply all of it to master only, and if we decide "we want this in 2.6", we can do a proper backport of the relevant bits, in useful chunks. Your patch has been applied to the master branch. commit 0134184012dd46ec44cbca7eb3ece39037ae0bfa Author: Arne Schwabe Date: Tue Jun 20 15:53:06 2023 +0200 add basic CMake based build Signed-off-by: Arne Schwabe <ar...@rf...> Signed-off-by: Frank Lichtenheld <fr...@li...> Acked-by: Lev Stipakov <lst...@gm...> Message-Id: <202...@li...> URL: https://www.mail-archive.com/ope...@li.../msg26758.html Signed-off-by: Gert Doering <ge...@gr...> -- kind regards, Gert Doering