From 10fe3b82d4a7918544ea332840343db86fe67209 Mon Sep 17 00:00:00 2001 From: Arne Schwabe Date: Wed, 14 Feb 2024 14:42:15 +0100 Subject: [PATCH 01/31] Change year 2018 in MD5 warning to "near future". With the change to OpenSSL 3 and introducing insecure as profile we actually allowed MD5 again. Update the warning to reflect this. Signed-off-by: Arne Schwabe --- openvpn/client/cliproto.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openvpn/client/cliproto.hpp b/openvpn/client/cliproto.hpp index 0bc13f4ea..d83973fda 100644 --- a/openvpn/client/cliproto.hpp +++ b/openvpn/client/cliproto.hpp @@ -1056,7 +1056,7 @@ class Session : ProtoContext, if (tls_warnings & SSLAPI::TLS_WARN_SIG_MD5) { - ClientEvent::Base::Ptr ev = new ClientEvent::Warn("TLS: received certificate signed with MD5. Please inform your admin to upgrade to a stronger algorithm. Support for MD5 will be dropped at end of Apr 2018"); + ClientEvent::Base::Ptr ev = new ClientEvent::Warn("TLS: received certificate signed with MD5. Please inform your admin to upgrade to a stronger algorithm. Support for MD5 will be dropped in the near future"); cli_events->add_event(std::move(ev)); } From cdcf942c24091e84f68d9dac0db1d296ff909980 Mon Sep 17 00:00:00 2001 From: Frank Lichtenheld Date: Mon, 13 Feb 2023 15:39:48 +0100 Subject: [PATCH 02/31] CMake: small improvements - Increase required version to 3.10. That is the version in Ubuntu Bionic and currently the oldest one we still want to support. - Enable CTest for test target Signed-off-by: Frank Lichtenheld (cherry picked from commit 50271ee02a4e4211051fd1d3339f1d7a847d0a6b) --- CMakeLists.txt | 4 +++- test/unittests/CMakeLists.txt | 3 +-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e32c4d871..25bc762bb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,4 +1,4 @@ -cmake_minimum_required(VERSION 3.5) +cmake_minimum_required(VERSION 3.10) cmake_policy(SET CMP0048 NEW) project(OpenVPN3-core VERSION 3) @@ -24,6 +24,8 @@ set(CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/cmake set(CMAKE_CXX_STANDARD 17) set(CMAKE_C_STANDARD 99) +include(CTest) + include(findcoredeps) include(ovpn-doxygen) diff --git a/test/unittests/CMakeLists.txt b/test/unittests/CMakeLists.txt index 95e94f080..8f151f678 100644 --- a/test/unittests/CMakeLists.txt +++ b/test/unittests/CMakeLists.txt @@ -105,8 +105,7 @@ else () endif () if (${CMAKE_SYSTEM_NAME} STREQUAL "Linux") - list(APPEND EXTRA_LIBS -lcap) - + target_link_libraries(coreUnitTests cap) target_sources(coreUnitTests PRIVATE test_sitnl.cpp) endif () From 7c9eeca1ee971d969208c94bc7a418f411ca160b Mon Sep 17 00:00:00 2001 From: Frank Lichtenheld Date: Mon, 13 Feb 2023 15:42:31 +0100 Subject: [PATCH 03/31] vcpkg.json: Allow to use on Linux - Fix PATCHES to work on Linux - While here, fix version number Signed-off-by: Frank Lichtenheld (cherry picked from commit fb9bee5ad698a4c1d5a86980c794096a1ed842bb) --- deps/vcpkg-ports/asio/portfile.cmake | 8 +++---- deps/vcpkg-ports/mbedtls/portfile.cmake | 29 +++++++++++++++++++++++++ scripts/mingw/build | 2 +- vcpkg.json | 2 +- 4 files changed, 35 insertions(+), 6 deletions(-) create mode 100644 deps/vcpkg-ports/mbedtls/portfile.cmake diff --git a/deps/vcpkg-ports/asio/portfile.cmake b/deps/vcpkg-ports/asio/portfile.cmake index 32c48630f..9b92e9da7 100644 --- a/deps/vcpkg-ports/asio/portfile.cmake +++ b/deps/vcpkg-ports/asio/portfile.cmake @@ -7,10 +7,10 @@ vcpkg_from_github( SHA512 a5d6e597e5611b7293375965f37c09cb73e27639ebdda6163557fab8bbff2ddbb301080ad86ff7f97e8ed8454da25176385cfc43103447a4a04e35a9c41aec3e HEAD_REF master PATCHES - ..\\..\\asio\\patches\\0001-Added-Apple-NAT64-support-when-both-ASIO_HAS_GETADDR.patch - ..\\..\\asio\\patches\\0002-Added-randomize-method-to-asio-ip-tcp-resolver-resul.patch - ..\\..\\asio\\patches\\0003-Added-user-code-hook-async_connect_post_open-to-be-c.patch - ..\\..\\asio\\patches\\0004-error_code.ipp-Use-English-for-Windows-error-message.patch + ../../asio/patches/0001-Added-Apple-NAT64-support-when-both-ASIO_HAS_GETADDR.patch + ../../asio/patches/0002-Added-randomize-method-to-asio-ip-tcp-resolver-resul.patch + ../../asio/patches/0003-Added-user-code-hook-async_connect_post_open-to-be-c.patch + ../../asio/patches/0004-error_code.ipp-Use-English-for-Windows-error-message.patch ) # Always use "ASIO_STANDALONE" to avoid boost dependency diff --git a/deps/vcpkg-ports/mbedtls/portfile.cmake b/deps/vcpkg-ports/mbedtls/portfile.cmake new file mode 100644 index 000000000..cbed83c4e --- /dev/null +++ b/deps/vcpkg-ports/mbedtls/portfile.cmake @@ -0,0 +1,29 @@ +include(vcpkg_common_functions) + +set(VCPKG_LIBRARY_LINKAGE static) + +vcpkg_from_github( + OUT_SOURCE_PATH SOURCE_PATH + REPO ARMmbed/mbedtls + REF mbedtls-2.7.12 + SHA512 bfad5588804e52827ecba81ca030fe570c9772f633fbf470d71a781db4366541da69b85ee10941bf500a987c4da825caa049afc2c0e6ec0ecc55d50efd74e5a6 + HEAD_REF master + PATCHES + ../../mbedtls/patches/0001-relax-x509-date-format-check.patch +) + +vcpkg_configure_cmake( + SOURCE_PATH ${SOURCE_PATH} + PREFER_NINJA + OPTIONS + -DENABLE_TESTING=OFF + -DENABLE_PROGRAMS=OFF +) + +vcpkg_install_cmake() + +file(REMOVE_RECURSE ${CURRENT_PACKAGES_DIR}/debug/include) + +file(INSTALL ${SOURCE_PATH}/LICENSE DESTINATION ${CURRENT_PACKAGES_DIR}/share/mbedtls RENAME copyright) + +vcpkg_copy_pdbs() diff --git a/scripts/mingw/build b/scripts/mingw/build index 232ae99d7..fa798447b 100755 --- a/scripts/mingw/build +++ b/scripts/mingw/build @@ -48,7 +48,7 @@ download_deps() gitref=$(grep -oP '\bREF\s+\S+' "$portfile" | cut -d' ' -f2) git clone --single-branch --branch "$gitref" https://github.com/chriskohlhoff/asio # apply asio patches - for patchfile in $(grep -o patches.* "$portfile" | cut -d '\' -f3); do + for patchfile in $(grep -o patches.* "$portfile" | cut -d '/' -f2); do echo applying patch $patchfile patch -d asio -p1 < "${CORE_DIR}/deps/asio/patches/$patchfile" done diff --git a/vcpkg.json b/vcpkg.json index dc17a5c6d..810ee338f 100644 --- a/vcpkg.json +++ b/vcpkg.json @@ -1,7 +1,7 @@ { "$schema": "https://raw.githubusercontent.com/microsoft/vcpkg/master/scripts/vcpkg.schema.json", "name": "openvpn3", - "version-string": "3.7", + "version-string": "3.8", "dependencies": [ "asio", "gtest", From 4e0de88c0393de4ce80feb60b0c53d166ddc3ede Mon Sep 17 00:00:00 2001 From: Frank Lichtenheld Date: Tue, 14 Feb 2023 14:54:07 +0100 Subject: [PATCH 04/31] CMake: Fix issues in FindmbedTLS Use add_library to define a target so that we do not need to apply all the setting manually. Use find_package_message() to avoid printing the message more than once. Signed-off-by: Frank Lichtenheld (cherry picked from commit 2fb5d08ea0a912704855a14a245e4e38b1d0ea6d) --- cmake/FindmbedTLS.cmake | 42 ++++++++++++++++++++++++++-------------- cmake/findcoredeps.cmake | 7 +------ 2 files changed, 29 insertions(+), 20 deletions(-) diff --git a/cmake/FindmbedTLS.cmake b/cmake/FindmbedTLS.cmake index a262eaf5e..93bf64dbf 100644 --- a/cmake/FindmbedTLS.cmake +++ b/cmake/FindmbedTLS.cmake @@ -10,6 +10,8 @@ # MBEDX509_LIBRARY - path to mbedTLS X.509 library # MBEDCRYPTO_LIBRARY - path to mbedTLS Crypto library +include(FindPackageMessage) + FIND_PATH(MBEDTLS_INCLUDE_DIR mbedtls/version.h) IF(MBEDTLS_INCLUDE_DIR AND MBEDTLS_LIBRARIES) @@ -29,24 +31,36 @@ ELSEIF(MBEDTLS_INCLUDE_DIR AND MBEDTLS_LIBRARY AND NOT MBEDX509_LIBRARY AND NOT ENDIF() IF(MBEDTLS_FOUND) + IF(NOT TARGET mbedTLS::mbedTLS AND EXISTS ${MBEDTLS_LIBRARY}) + add_library(mbedTLS::mbedTLS UNKNOWN IMPORTED) + set_target_properties(mbedTLS::mbedTLS PROPERTIES + INTERFACE_INCLUDE_DIRECTORIES "${MBEDTLS_INCLUDE_DIR}") + set_target_properties(mbedTLS::mbedTLS PROPERTIES + IMPORTED_LINK_INTERFACE_LANGUAGES "C" + IMPORTED_LOCATION "${MBEDTLS_LIBRARY}") + IF(HACKY_OVPN_MBEDTLS) - SET(MBEDTLS_LIBRARIES ${MBEDTLS_LIBRARY}) + SET(MBEDTLS_LIBRARIES ${MBEDTLS_LIBRARY}) ELSE() - SET(MBEDTLS_LIBRARIES ${MBEDTLS_LIBRARY} ${MBEDX509_LIBRARY} ${MBEDCRYPTO_LIBRARY}) - endif() + SET(MBEDTLS_LIBRARIES ${MBEDTLS_LIBRARY} ${MBEDX509_LIBRARY} ${MBEDCRYPTO_LIBRARY}) + set_property( TARGET mbedTLS::mbedTLS APPEND PROPERTY INTERFACE_LINK_LIBRARIES ${MBEDX509_LIBRARY} ) + set_property( TARGET mbedTLS::mbedTLS APPEND PROPERTY INTERFACE_LINK_LIBRARIES ${MBEDCRYPTO_LIBRARY} ) + ENDIF() IF(NOT MBEDTLS_FIND_QUIETLY) - MESSAGE(STATUS "Found mbedTLS:") - FILE(READ ${MBEDTLS_INCLUDE_DIR}/mbedtls/version.h MBEDTLSCONTENT) - STRING(REGEX MATCH "MBEDTLS_VERSION_STRING +\"[0-9|.]+\"" MBEDTLSMATCH ${MBEDTLSCONTENT}) - IF (MBEDTLSMATCH) - STRING(REGEX REPLACE "MBEDTLS_VERSION_STRING +\"([0-9|.]+)\"" "\\1" MBEDTLS_VERSION ${MBEDTLSMATCH}) - MESSAGE(STATUS " version ${MBEDTLS_VERSION}") - ENDIF(MBEDTLSMATCH) - MESSAGE(STATUS " TLS: ${MBEDTLS_LIBRARY}") - MESSAGE(STATUS " X509: ${MBEDX509_LIBRARY}") - MESSAGE(STATUS " Crypto: ${MBEDCRYPTO_LIBRARY}") + SET(MBEDTLS_MSG "Found mbedTLS:\n") + FILE(READ ${MBEDTLS_INCLUDE_DIR}/mbedtls/version.h MBEDTLSCONTENT) + STRING(REGEX MATCH "MBEDTLS_VERSION_STRING +\"[0-9|.]+\"" MBEDTLSMATCH ${MBEDTLSCONTENT}) + IF (MBEDTLSMATCH) + STRING(REGEX REPLACE "MBEDTLS_VERSION_STRING +\"([0-9|.]+)\"" "\\1" MBEDTLS_VERSION ${MBEDTLSMATCH}) + string(APPEND MBEDTLS_MSG " version ${MBEDTLS_VERSION}\n") + ENDIF(MBEDTLSMATCH) + string(APPEND MBEDTLS_MSG " TLS: ${MBEDTLS_LIBRARY}\n") + string(APPEND MBEDTLS_MSG " X509: ${MBEDX509_LIBRARY}\n") + string(APPEND MBEDTLS_MSG " Crypto: ${MBEDCRYPTO_LIBRARY}\n") + FIND_PACKAGE_MESSAGE(MBEDTLS "${MBEDTLS_MSG}" "[${MBEDTLS_INCLUDE_DIR}][${MBEDTLS_LIBRARY}][${MBEDX509_LIBRARY}][${MBEDCRYPTO_LIBRARY}]") ENDIF(NOT MBEDTLS_FIND_QUIETLY) + ENDIF(NOT TARGET mbedTLS::mbedTLS AND EXISTS ${MBEDTLS_LIBRARY}) ELSE(MBEDTLS_FOUND) IF(mbedTLS_FIND_REQUIRED) MESSAGE(FATAL_ERROR "Could not find mbedTLS") @@ -60,4 +74,4 @@ MARK_AS_ADVANCED( MBEDTLS_LIBRARY MBEDX509_LIBRARY MBEDCRYPTO_LIBRARY -) \ No newline at end of file +) diff --git a/cmake/findcoredeps.cmake b/cmake/findcoredeps.cmake index 817372b18..2741032d9 100644 --- a/cmake/findcoredeps.cmake +++ b/cmake/findcoredeps.cmake @@ -30,13 +30,8 @@ endif () function(add_ssl_library target) if (${USE_MBEDTLS}) find_package(mbedTLS REQUIRED) - - set(SSL_LIBRARY ${MBEDTLS_LIBRARIES}) - + set(SSL_LIBRARY mbedTLS::mbedTLS) target_compile_definitions(${target} PRIVATE -DUSE_MBEDTLS) - - # The findpackage function does not set these automatically :( - target_include_directories(${target} PRIVATE ${MBEDTLS_INCLUDE_DIR}) else () find_package(OpenSSL REQUIRED) SET(SSL_LIBRARY OpenSSL::SSL) From fa99c85cb92346d52c136e42ef7a13c8e63bf467 Mon Sep 17 00:00:00 2001 From: Frank Lichtenheld Date: Tue, 14 Feb 2023 14:55:33 +0100 Subject: [PATCH 05/31] CMake: Generalize add_json_library Make sure we find vcpkg and system packages on all platforms. Signed-off-by: Frank Lichtenheld (cherry picked from commit e720bf3abad3e6c0d2590747f1dec93d8c436464) --- cmake/findcoredeps.cmake | 38 +++++++++++++++----------------------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/cmake/findcoredeps.cmake b/cmake/findcoredeps.cmake index 2741032d9..d605edad5 100644 --- a/cmake/findcoredeps.cmake +++ b/cmake/findcoredeps.cmake @@ -1,4 +1,4 @@ -cmake_minimum_required(VERSION 3.5) +cmake_minimum_required(VERSION 3.10) set(CMAKE_CXX_STANDARD 17) @@ -131,27 +131,19 @@ function(add_core_dependencies target) endfunction() function (add_json_library target) - if (MSVC) - find_package(jsoncpp CONFIG REQUIRED) - target_link_libraries(${target} jsoncpp_lib) - target_compile_definitions(${target} PRIVATE -DHAVE_JSONCPP) - message("Adding jsoncpp to " ${target}) - elseif (APPLE) - find_package(PkgConfig REQUIRED) - pkg_check_modules(JSONCPP jsoncpp) - target_link_libraries(${target} ${JSONCPP_LDFLAGS}) - target_include_directories(${target} PRIVATE ${JSONCPP_INCLUDE_DIRS}) - target_compile_definitions(${target} PRIVATE -DHAVE_JSONCPP) - else () - find_package(PkgConfig REQUIRED) - if (MINGW) - # due to cmake bug, APPEND doesn't work for mingw - # https://github.com/Kitware/CMake/commit/f92a4b23994fa7516f16fbb5b3c02caa07534b3f - set(CMAKE_PREFIX_PATH ${DEP_DIR}) - endif () - pkg_check_modules(JSONCPP jsoncpp) - target_link_libraries(${target} ${JSONCPP_LDFLAGS}) - target_include_directories(${target} PRIVATE ${JSONCPP_INCLUDE_DIRS}) - target_compile_definitions(${target} PRIVATE -DHAVE_JSONCPP) + find_package(jsoncpp CONFIG) + if (jsoncpp_FOUND AND TARGET JsonCpp::JsonCpp) + target_link_libraries(${target} JsonCpp::JsonCpp) + target_compile_definitions(${target} PRIVATE -DHAVE_JSONCPP) + else() + find_package(PkgConfig REQUIRED) + if (MINGW) + # due to cmake bug, APPEND doesn't work for mingw + # https://github.com/Kitware/CMake/commit/f92a4b23994fa7516f16fbb5b3c02caa07534b3f + set(CMAKE_PREFIX_PATH ${DEP_DIR}) endif () + pkg_check_modules(JSONCPP REQUIRED IMPORTED_TARGET jsoncpp) + target_link_libraries(${target} PkgConfig::JSONCPP) + target_compile_definitions(${target} PRIVATE -DHAVE_JSONCPP) + endif () endfunction () From 0dcae2690b3e09e9df752c8577309f04350bf5db Mon Sep 17 00:00:00 2001 From: Frank Lichtenheld Date: Tue, 14 Feb 2023 17:19:52 +0000 Subject: [PATCH 06/31] CMake: support BUILD_TESTING option This is important since it allows us to avoid the JsonCPP dependency on non-Win/non-Apple systems. Signed-off-by: Frank Lichtenheld (cherry picked from commit a9570cb78084eb6aa7ee9a9aec5de82eb7b1af67) --- CMakeLists.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 25bc762bb..cf600bb37 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -30,7 +30,9 @@ include(findcoredeps) include(ovpn-doxygen) add_subdirectory(client) -add_subdirectory(test/unittests) +if (BUILD_TESTING) + add_subdirectory(test/unittests) +endif() add_subdirectory(test/ovpncli) add_subdirectory(test/ssl) From 3614c1a00497a3a1283ad0f9fc4332c2eb56e928 Mon Sep 17 00:00:00 2001 From: Frank Lichtenheld Date: Tue, 14 Feb 2023 17:20:50 +0000 Subject: [PATCH 07/31] CMake: move architecture detection closer to the actual executable This makes it easier to see what is going on when looking at individual CMakeLists. Signed-off-by: Frank Lichtenheld (cherry picked from commit 4c81069564b291f85e0df75abbe5e3cece156d6f) --- CMakeLists.txt | 17 ++++------------- openvpn/omi/CMakeLists.txt | 14 +++++++++----- openvpn/ovpnagent/mac/CMakeLists.txt | 4 ++++ openvpn/ovpnagent/win/CMakeLists.txt | 4 ++++ test/unittests/CMakeLists.txt | 3 +++ 5 files changed, 24 insertions(+), 18 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index cf600bb37..c4e03c93a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -30,22 +30,13 @@ include(findcoredeps) include(ovpn-doxygen) add_subdirectory(client) -if (BUILD_TESTING) - add_subdirectory(test/unittests) -endif() +add_subdirectory(test/unittests) add_subdirectory(test/ovpncli) add_subdirectory(test/ssl) - -if (WIN32) - add_subdirectory(openvpn/omi) - add_subdirectory(openvpn/ovpnagent/win) -endif () - -if (APPLE) - add_subdirectory(openvpn/ovpnagent/mac) -endif () - +add_subdirectory(openvpn/omi) +add_subdirectory(openvpn/ovpnagent/win) +add_subdirectory(openvpn/ovpnagent/mac) if (ENABLE_DOXYGEN) # Exclude some project specific directories diff --git a/openvpn/omi/CMakeLists.txt b/openvpn/omi/CMakeLists.txt index cd7cb8fdd..08dcd7abb 100644 --- a/openvpn/omi/CMakeLists.txt +++ b/openvpn/omi/CMakeLists.txt @@ -1,3 +1,7 @@ +if (NOT WIN32) + return() +endif() + option(CLI_OVPNDCOWIN "Build ovpncli with ovpn-dco-win driver support" OFF) add_executable(omicliagent openvpn.cpp) @@ -8,9 +12,9 @@ target_compile_definitions(omicliagent PRIVATE -DOPENVPN_COMMAND_AGENT -DOVPNAGE add_executable(omicli openvpn.cpp) add_core_dependencies(omicli) -if (WIN32 AND ${CLI_OVPNDCOWIN}) - target_compile_definitions(omicliagent PRIVATE ENABLE_OVPNDCOWIN) - target_compile_definitions(omicli PRIVATE ENABLE_OVPNDCOWIN) - target_link_libraries(omicliagent "bcrypt.lib") - target_link_libraries(omicli "bcrypt.lib") +if (CLI_OVPNDCOWIN) + target_compile_definitions(omicliagent PRIVATE ENABLE_OVPNDCOWIN) + target_compile_definitions(omicli PRIVATE ENABLE_OVPNDCOWIN) + target_link_libraries(omicliagent "bcrypt.lib") + target_link_libraries(omicli "bcrypt.lib") endif () diff --git a/openvpn/ovpnagent/mac/CMakeLists.txt b/openvpn/ovpnagent/mac/CMakeLists.txt index 19a3117e9..9f1b425bd 100644 --- a/openvpn/ovpnagent/mac/CMakeLists.txt +++ b/openvpn/ovpnagent/mac/CMakeLists.txt @@ -1,3 +1,7 @@ +if (NOT APPLE) + return() +endif() + add_executable(agent_macos ovpnagent.cpp) add_core_dependencies(agent_macos) add_json_library(agent_macos) diff --git a/openvpn/ovpnagent/win/CMakeLists.txt b/openvpn/ovpnagent/win/CMakeLists.txt index a947e3b49..682cfaef1 100644 --- a/openvpn/ovpnagent/win/CMakeLists.txt +++ b/openvpn/ovpnagent/win/CMakeLists.txt @@ -1,3 +1,7 @@ +if (NOT WIN32) + return() +endif() + add_executable(ovpnagent ovpnagent.cpp) add_core_dependencies(ovpnagent) add_json_library(ovpnagent) diff --git a/test/unittests/CMakeLists.txt b/test/unittests/CMakeLists.txt index 8f151f678..3b4469e0c 100644 --- a/test/unittests/CMakeLists.txt +++ b/test/unittests/CMakeLists.txt @@ -1,3 +1,6 @@ +if (NOT BUILD_TESTING) + return() +endif() # current latest GTEST version set(OVPN_GTEST_VERSION release-1.11.0) From ac01ae47e9e914d4fc286248a86baa7756d167f3 Mon Sep 17 00:00:00 2001 From: Frank Lichtenheld Date: Fri, 17 Feb 2023 18:50:56 +0100 Subject: [PATCH 08/31] mingw: disable VCPKG_APPLOCAL_DEPS in build-vcpkg We do not want to force a dependency on powershell. Copying the right dlls is rather trivial. Signed-off-by: Frank Lichtenheld (cherry picked from commit e9e49239ce3fd9d81f9f4df605f103d100462e97) --- scripts/mingw/build-vcpkg | 56 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100755 scripts/mingw/build-vcpkg diff --git a/scripts/mingw/build-vcpkg b/scripts/mingw/build-vcpkg new file mode 100755 index 000000000..254f5168f --- /dev/null +++ b/scripts/mingw/build-vcpkg @@ -0,0 +1,56 @@ +#!/usr/bin/env bash + +set -eux + +: ${VCPKG_ROOT?:VCPKG_ROOT needs to be set} +: ${ARCH:=i686 x86_64} +: ${DCO:=1} +CORE_DIR=$(dirname $(realpath -s $0))/../.. +CMAKE_C_COMPILER="w64-mingw32-gcc-posix" +CMAKE_CXX_COMPILER="w64-mingw32-g++-posix" + +build_core() +{ + local ARCH=$1 + local VCPKG_ARCH=$ARCH + if [ "$ARCH" = i686 ]; then + VCPKG_ARCH=x86 + fi + if [ "$ARCH" = x86_64 ]; then + VCPKG_ARCH=x64 + fi + + echo "Building core for $ARCH (vcpkg: $VCPKG_ARCH-mingw-dynamic)" + + rm -rf "build-$ARCH" + mkdir "build-$ARCH" + + [ -z "$DCO" ] || { + WITH_OVPNDCOWIN="-D CLI_OVPNDCOWIN=ON" + } + + pushd build-$ARCH + + cmake -D CMAKE_TOOLCHAIN_FILE="${VCPKG_ROOT}/scripts/buildsystems/vcpkg.cmake" \ + -D VCPKG_TARGET_TRIPLET="$VCPKG_ARCH-mingw-dynamic" \ + -D VCPKG_APPLOCAL_DEPS=OFF \ + -D VCPKG_OVERLAY_PORTS="$CORE_DIR/deps/vcpkg-ports" \ + -D CMAKE_C_COMPILER="$ARCH-$CMAKE_C_COMPILER" \ + -D CMAKE_CXX_COMPILER="$ARCH-$CMAKE_CXX_COMPILER" \ + -D CMAKE_SYSTEM_NAME=Windows \ + -D CMAKE_PREFIX_PATH="/usr/local/$ARCH-w64-mingw32" \ + -D CMAKE_BUILD_TYPE=Release \ + -D USE_WERROR=true \ + $WITH_OVPNDCOWIN \ + $CORE_DIR + + make + + popd +} + +for arch in $ARCH +do + echo "Building for $arch" + build_core $arch +done From f845f7dd952fbf9691a0731ea529d659ef0ef759 Mon Sep 17 00:00:00 2001 From: Frank Lichtenheld Date: Thu, 23 Feb 2023 18:20:08 +0100 Subject: [PATCH 09/31] vcpkg-ports/asio: copy update asio-config.cmake from vcpkg Fixes problems when calling find_package on asio multiple times. Originally fixed by commit cba75f1aa08374733dcc79abebeca262ae94118a in vcpkg#28299. Signed-off-by: Frank Lichtenheld (cherry picked from commit 71cf5f48fe52cbf06b022a58f34f9bb7e2ccb17a) --- deps/vcpkg-ports/asio/asio-config.cmake | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/deps/vcpkg-ports/asio/asio-config.cmake b/deps/vcpkg-ports/asio/asio-config.cmake index 6e5325003..dec682e17 100644 --- a/deps/vcpkg-ports/asio/asio-config.cmake +++ b/deps/vcpkg-ports/asio/asio-config.cmake @@ -1,6 +1,9 @@ include ("${CMAKE_CURRENT_LIST_DIR}/asio-targets.cmake") -add_library(asio::asio INTERFACE IMPORTED) -target_link_libraries(asio::asio INTERFACE asio) + +if(NOT TARGET asio::asio) + add_library(asio::asio INTERFACE IMPORTED) + target_link_libraries(asio::asio INTERFACE asio) +endif() get_target_property(_ASIO_INCLUDE_DIR asio INTERFACE_INCLUDE_DIRECTORIES) set(ASIO_INCLUDE_DIR "${_ASIO_INCLUDE_DIR}") From 25ca35d71d0adc470d1d6cd40cdbd031d2de59ce Mon Sep 17 00:00:00 2001 From: Frank Lichtenheld Date: Fri, 24 Feb 2023 11:24:39 +0100 Subject: [PATCH 10/31] CMake: add CMakePresets.json and switch GHA to use it For now define vcpkg builds for MSVC and MinGW Signed-off-by: Frank Lichtenheld (cherry picked from commit 63499ba7ac7b435db1f11322b0eb14a01ff40d6d) --- .github/workflows/msbuild.yml | 74 ++++++-- CMakePresets.json | 234 ++++++++++++++++++++++++ deps/vcpkg_manifests/mingw/vcpkg.json | 13 ++ deps/vcpkg_manifests/windows/vcpkg.json | 15 ++ 4 files changed, 316 insertions(+), 20 deletions(-) create mode 100644 CMakePresets.json create mode 100644 deps/vcpkg_manifests/mingw/vcpkg.json create mode 100644 deps/vcpkg_manifests/windows/vcpkg.json diff --git a/.github/workflows/msbuild.yml b/.github/workflows/msbuild.yml index 6d3236186..265bc39d3 100644 --- a/.github/workflows/msbuild.yml +++ b/.github/workflows/msbuild.yml @@ -11,7 +11,6 @@ jobs: env: VCPKG_ROOT: ${{ github.workspace }}/vcpkg BUILD_CONFIGURATION: Release - buildDir: '${{ github.workspace }}/build' runs-on: windows-latest steps: @@ -24,29 +23,64 @@ jobs: - name: Restore from cache and install vcpkg uses: lukka/run-vcpkg@v6 with: - setupOnly: true - vcpkgGitCommitId: 'd8d61c941c333a147edffdcbdc9964dc0c0962f5' - additionalCachedPaths: ${{ env.buildDir }}/vcpkg_installed - appendedCacheKey: ${{ matrix.arch }}-${{ hashFiles( '**/vcpkg.json' ) }} + vcpkgGitCommitId: '36fb23307e10cc6ffcec566c46c4bb3f567c82c6' + vcpkgJsonGlob: '**/windows/vcpkg.json' - name: Run CMake with vcpkg.json manifest - uses: lukka/run-cmake@v3 - env: - VCPKG_OVERLAY_PORTS: '${{ github.workspace }}/deps/vcpkg-ports' + uses: lukka/run-cmake@v10 with: - useVcpkgToolchainFile: true - buildDirectory: '${{ env.buildDir }}' - buildWithCMake: true - cmakeAppendedArgs: '-DCLI_OVPNDCOWIN=ON' - cmakeBuildType: ${{env.BUILD_CONFIGURATION}} + configurePreset: win-${{ matrix.arch }}-release + buildPreset: win-${{ matrix.arch }}-release - uses: actions/upload-artifact@v2 with: - name: openvpn3-${{ matrix.arch }} + name: openvpn3-msvc-${{ matrix.arch }} path: | - ${{ env.buildDir }}/**/*.exe - ${{ env.buildDir }}/**/*.dll - !${{ env.buildDir }}/test/ssl/** - !${{ env.buildDir }}/test/unittests/** - !${{ env.buildDir }}/CMakeFiles/** - !${{ env.buildDir }}/vcpkg_installed/** + ${{ github.workspace }}/build/**/*.exe + ${{ github.workspace }}/build/**/*.dll + !${{ github.workspace }}/build/**/test/ssl/** + !${{ github.workspace }}/build/**/test/unittests/** + !${{ github.workspace }}/build/**/CMakeFiles/** + !${{ github.workspace }}/build/**/vcpkg_installed/** + mingw: + strategy: + matrix: + arch: [x86, x64] + + env: + VCPKG_ROOT: ${{ github.workspace }}/vcpkg + + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: lukka/get-cmake@latest + + - name: Install dependencies + run: sudo apt update && sudo apt install -y mingw-w64 unzip cmake build-essential + + - name: Restore from cache and install vcpkg + uses: lukka/run-vcpkg@v10 + with: + vcpkgGitCommitId: '36fb23307e10cc6ffcec566c46c4bb3f567c82c6' + vcpkgJsonGlob: '**/mingw/vcpkg.json' + + - name: Run CMake with vcpkg.json manifest + uses: lukka/run-cmake@v10 + with: + configurePreset: mingw-${{ matrix.arch }}-release + buildPreset: mingw-${{ matrix.arch }}-release + + - name: List $RUNNER_WORKSPACE after build + run: find $RUNNER_WORKSPACE + shell: bash + + - uses: actions/upload-artifact@v2 + with: + name: openvpn3-mingw-${{ matrix.arch }} + path: | + ${{ github.workspace }}/build/**/*.exe + ${{ github.workspace }}/build/**/*.dll + !${{ github.workspace }}/build/**/test/ssl/** + !${{ github.workspace }}/build/**/test/unittests/** + !${{ github.workspace }}/build/**/CMakeFiles/** + !${{ github.workspace }}/build/**/vcpkg_installed/** diff --git a/CMakePresets.json b/CMakePresets.json new file mode 100644 index 000000000..70fc315ca --- /dev/null +++ b/CMakePresets.json @@ -0,0 +1,234 @@ +{ + "version": 3, + "configurePresets": [ + { + "name": "base", + "hidden": true, + "cacheVariables": { + "CMAKE_TOOLCHAIN_FILE": { + "value": "$env{VCPKG_ROOT}/scripts/buildsystems/vcpkg.cmake", + "type": "FILEPATH" + }, + "VCPKG_OVERLAY_TRIPLETS": { + "value": "${sourceDir}/deps/vcpkg-triplets", + "type": "FILEPATH" + }, + "VCPKG_OVERLAY_PORTS": { + "value": "${sourceDir}/deps/vcpkg-ports", + "type": "FILEPATH" + } + } + }, + { + "name": "base-windows", + "hidden": true, + "generator": "Visual Studio 17 2022", + "cacheVariables": { + "CLI_OVPNDCOWIN": { + "value": "ON", + "type": "BOOL" + }, + "VCPKG_MANIFEST_DIR": "${sourceDir}/deps/vcpkg_manifests/windows" + }, + "vendor": { "microsoft.com/VisualStudioSettings/CMake/1.0": { "hostOS": [ "Windows" ] } } + }, + { + "name": "base-mingw", + "hidden": true, + "generator": "Unix Makefiles", + "cacheVariables": { + "CMAKE_SYSTEM_NAME": { + "value": "Windows", + "type": "STRING" + }, + "CLI_OVPNDCOWIN": { + "value": "ON", + "type": "BOOL" + }, + "VCPKG_MANIFEST_DIR": "${sourceDir}/deps/vcpkg_manifests/mingw" + } + }, + { + "name": "x64", + "hidden": true, + "binaryDir": "build/msvc/amd64", + "architecture": { + "value": "x64" + } + }, + { + "name": "x64-mingw", + "hidden": true, + "binaryDir": "build/mingw/x64", + "cacheVariables": { + "CMAKE_C_COMPILER": { + "value": "x86_64-w64-mingw32-gcc-posix", + "type": "STRING" + }, + "CMAKE_CXX_COMPILER": { + "value": "x86_64-w64-mingw32-g++-posix", + "type": "STRING" + }, + "VCPKG_TARGET_TRIPLET": "x64-mingw-dynamic" + } + }, + { + "name": "arm64", + "hidden": true, + "binaryDir": "build/msvc/arm64", + "architecture": { + "value": "arm64" + } + }, + { + "name": "x86", + "hidden": true, + "binaryDir": "build/msvc/x86", + "architecture": { + "value": "Win32" + } + }, + { + "name": "i686-mingw", + "hidden": true, + "binaryDir": "build/mingw/x86", + "cacheVariables": { + "CMAKE_C_COMPILER": { + "value": "i686-w64-mingw32-gcc-posix", + "type": "STRING" + }, + "CMAKE_CXX_COMPILER": { + "value": "i686-w64-mingw32-g++-posix", + "type": "STRING" + }, + "VCPKG_TARGET_TRIPLET": "x86-mingw-dynamic" + } + }, + { + "name": "debug", + "hidden": true, + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Debug" + } + }, + { + "name": "release", + "hidden": true, + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Release" + } + }, + { + "name": "asan", + "hidden": true, + "cacheVariables": { + "CMAKE_BUILD_TYPE": "ASAN" + } + }, + { + "name": "mingw-x64-debug", + "inherits": [ "base", "base-mingw", "x64-mingw", "debug" ] + }, + { + "name": "mingw-x64-release", + "inherits": [ "base", "base-mingw", "x64-mingw", "release" ] + }, + { + "name": "mingw-x86-debug", + "inherits": [ "base", "base-mingw", "i686-mingw", "debug" ] + }, + { + "name": "mingw-x86-release", + "inherits": [ "base", "base-mingw", "i686-mingw", "release" ] + }, + { + "name": "win-amd64-debug", + "inherits": [ "base", "base-windows", "x64", "debug" ] + }, + { + "name": "win-arm64-debug", + "inherits": [ "base", "base-windows", "arm64", "debug" ] + }, + { + "name": "win-amd64_arm64-debug", + "inherits": [ "base", "base-windows", "arm64", "debug" ] + }, + { + "name": "win-x86-debug", + "inherits": [ "base", "base-windows", "x86", "debug" ] + }, + { + "name": "win-amd64-release", + "inherits": [ "base", "base-windows", "x64", "release" ] + }, + { + "name": "win-arm64-release", + "inherits": [ "base", "base-windows", "arm64", "release" ] + }, + { + "name": "win-amd64_arm64-release", + "inherits": [ "base", "base-windows", "arm64", "release" ] + }, + { + "name": "win-x86-release", + "inherits": [ "base", "base-windows", "x86", "release" ] + } + ], + "buildPresets": [ + { + "name": "mingw-x64-debug", + "configurePreset": "mingw-x64-debug", + "configuration": "Debug" + }, + { + "name": "mingw-x86-debug", + "configurePreset": "mingw-x86-debug", + "configuration": "Debug" + }, + { + "name": "mingw-x64-release", + "configurePreset": "mingw-x64-release", + "configuration": "Release" + }, + { + "name": "mingw-x86-release", + "configurePreset": "mingw-x86-release", + "configuration": "Release" + }, + { + "name": "win-amd64-debug", + "configurePreset": "win-amd64-debug", + "configuration": "Debug" + }, + { + "name": "win-arm64-debug", + "configurePreset": "win-arm64-debug", + "configuration": "Debug" + }, + { + "name": "win-x86-debug", + "configurePreset": "win-x86-debug", + "configuration": "Debug" + }, + { + "name": "win-amd64-release", + "configurePreset": "win-amd64-release", + "configuration": "Release" + }, + { + "name": "win-arm64-release", + "configurePreset": "win-arm64-release", + "configuration": "Release" + }, + { + "name": "win-amd64_arm64-release", + "configurePreset": "win-arm64-release", + "configuration": "Release" + }, + { + "name": "win-x86-release", + "configurePreset": "win-x86-release", + "configuration": "Release" + } + ] +} diff --git a/deps/vcpkg_manifests/mingw/vcpkg.json b/deps/vcpkg_manifests/mingw/vcpkg.json new file mode 100644 index 000000000..1d878b4c1 --- /dev/null +++ b/deps/vcpkg_manifests/mingw/vcpkg.json @@ -0,0 +1,13 @@ +{ + "$schema": "https://raw.githubusercontent.com/microsoft/vcpkg/master/scripts/vcpkg.schema.json", + "name": "openvpn3", + "version-string": "3.8", + "dependencies": [ + "asio", + "jsoncpp", + "lz4", + "openssl", + "tap-windows6", + "xxhash" + ] + } diff --git a/deps/vcpkg_manifests/windows/vcpkg.json b/deps/vcpkg_manifests/windows/vcpkg.json new file mode 100644 index 000000000..874683ca2 --- /dev/null +++ b/deps/vcpkg_manifests/windows/vcpkg.json @@ -0,0 +1,15 @@ +{ + "$schema": "https://raw.githubusercontent.com/microsoft/vcpkg/master/scripts/vcpkg.schema.json", + "name": "openvpn3", + "version-string": "3.8", + "dependencies": [ + "asio", + "gtest", + "jsoncpp", + "lz4", + "openssl", + "tap-windows6", + "xxhash" + ] +} + From 37fb7c2efce0ca55f0354321284cfcace6b80788 Mon Sep 17 00:00:00 2001 From: Frank Lichtenheld Date: Fri, 24 Feb 2023 15:39:15 +0100 Subject: [PATCH 11/31] CMake: clean up dependency handling Always use find_package for all libraries. Add missing Find*.cmake modules. Always define an IMPORTED library in Find* Signed-off-by: Frank Lichtenheld (cherry picked from commit d7b3419f8e756d11832aa4ddee3f50cd3c5f06c2) --- cmake/FindLZ4.cmake | 9 --------- cmake/FindLZO.cmake | 19 +++++++++++++++++++ cmake/Findasio.cmake | 15 +++++++++++++++ cmake/Findlz4.cmake | 19 +++++++++++++++++++ cmake/FindxxHash.cmake | 15 +++++++++++++++ cmake/findcoredeps.cmake | 21 +++++++++------------ test/unittests/CMakeLists.txt | 31 +++++++++++++++++-------------- 7 files changed, 94 insertions(+), 35 deletions(-) delete mode 100644 cmake/FindLZ4.cmake create mode 100644 cmake/FindLZO.cmake create mode 100644 cmake/Findasio.cmake create mode 100644 cmake/Findlz4.cmake create mode 100644 cmake/FindxxHash.cmake diff --git a/cmake/FindLZ4.cmake b/cmake/FindLZ4.cmake deleted file mode 100644 index 761b275f9..000000000 --- a/cmake/FindLZ4.cmake +++ /dev/null @@ -1,9 +0,0 @@ -find_path(LZ4_INCLUDE_DIR NAMES lz4.h) -find_library(LZ4_LIBRARY NAMES lz4) - -include(FindPackageHandleStandardArgs) -FIND_PACKAGE_HANDLE_STANDARD_ARGS( - LZ4 DEFAULT_MSG - LZ4_LIBRARY LZ4_INCLUDE_DIR) - -mark_as_advanced(LZ4_INCLUDE_DIR LZ4_LIBRARY) \ No newline at end of file diff --git a/cmake/FindLZO.cmake b/cmake/FindLZO.cmake new file mode 100644 index 000000000..bb7b2c22a --- /dev/null +++ b/cmake/FindLZO.cmake @@ -0,0 +1,19 @@ +FIND_PATH(LZO_INCLUDE_DIR NAMES lzo/lzo1x.h) +FIND_LIBRARY(LZO_LIBRARY NAMES lzo2) + +include(FindPackageHandleStandardArgs) +FIND_PACKAGE_HANDLE_STANDARD_ARGS( + LZO DEFAULT_MSG + LZO_LIBRARY LZO_INCLUDE_DIR + ) + +if(LZO_LIBRARY AND NOT TARGET lzo::lzo) + add_library(lzo::lzo UNKNOWN IMPORTED) + set_target_properties(lzo::lzo PROPERTIES + INTERFACE_INCLUDE_DIRECTORIES "${LZO_INCLUDE_DIR}") + set_target_properties(lzo::lzo PROPERTIES + IMPORTED_LINK_INTERFACE_LANGUAGES "C" + IMPORTED_LOCATION "${LZO_LIBRARY}") +endif() + +mark_as_advanced(LZO_INCLUDE_DIR LZO_LIBRARY) diff --git a/cmake/Findasio.cmake b/cmake/Findasio.cmake new file mode 100644 index 000000000..553d9ae56 --- /dev/null +++ b/cmake/Findasio.cmake @@ -0,0 +1,15 @@ +find_path(ASIO_INCLUDE_DIR NAMES asio.hpp) + +include(FindPackageHandleStandardArgs) +FIND_PACKAGE_HANDLE_STANDARD_ARGS( + asio DEFAULT_MSG + ASIO_INCLUDE_DIR + ) + +if(ASIO_INCLUDE_DIR AND NOT TARGET asio::asio) + add_library(asio::asio INTERFACE IMPORTED) + set_target_properties(asio::asio PROPERTIES + INTERFACE_INCLUDE_DIRECTORIES "${ASIO_INCLUDE_DIR}") +endif() + +mark_as_advanced(ASIO_INCLUDE_DIR) diff --git a/cmake/Findlz4.cmake b/cmake/Findlz4.cmake new file mode 100644 index 000000000..2cf12bab4 --- /dev/null +++ b/cmake/Findlz4.cmake @@ -0,0 +1,19 @@ +find_path(LZ4_INCLUDE_DIR NAMES lz4.h) +find_library(LZ4_LIBRARY NAMES lz4) + +include(FindPackageHandleStandardArgs) +FIND_PACKAGE_HANDLE_STANDARD_ARGS( + lz4 DEFAULT_MSG + LZ4_LIBRARY LZ4_INCLUDE_DIR + ) + +if(LZ4_LIBRARY AND NOT TARGET lz4::lz4) + add_library(lz4::lz4 UNKNOWN IMPORTED) + set_target_properties(lz4::lz4 PROPERTIES + INTERFACE_INCLUDE_DIRECTORIES "${LZ4_INCLUDE_DIR}") + set_target_properties(lz4::lz4 PROPERTIES + IMPORTED_LINK_INTERFACE_LANGUAGES "C" + IMPORTED_LOCATION "${LZ4_LIBRARY}") +endif() + +mark_as_advanced(LZ4_INCLUDE_DIR LZ4_LIBRARY) diff --git a/cmake/FindxxHash.cmake b/cmake/FindxxHash.cmake new file mode 100644 index 000000000..d6de2916f --- /dev/null +++ b/cmake/FindxxHash.cmake @@ -0,0 +1,15 @@ +find_path(XXHASH_INCLUDE_DIR NAMES xxhash.h) + +include(FindPackageHandleStandardArgs) +FIND_PACKAGE_HANDLE_STANDARD_ARGS( + xxHash DEFAULT_MSG + XXHASH_INCLUDE_DIR + ) + +if(XXHASH_INCLUDE_DIR AND NOT TARGET xxHash::xxhash) + add_library(xxHash::xxhash INTERFACE IMPORTED) + set_target_properties(xxHash::xxhash PROPERTIES + INTERFACE_INCLUDE_DIRECTORIES "${XXHASH_INCLUDE_DIR}") +endif() + +mark_as_advanced(XXHASH_INCLUDE_DIR) diff --git a/cmake/findcoredeps.cmake b/cmake/findcoredeps.cmake index d605edad5..6886743ea 100644 --- a/cmake/findcoredeps.cmake +++ b/cmake/findcoredeps.cmake @@ -69,32 +69,31 @@ function(add_core_dependencies target) if (MSVC) target_compile_options(${target} PRIVATE "/bigobj") - find_package(lz4 CONFIG REQUIRED) - set(LZ4_LIBRARY lz4::lz4) else () find_package(Threads REQUIRED) target_compile_options(${target} PRIVATE "-Wa,-mbig-obj") list(APPEND EXTRA_LIBS ws2_32 wsock32 ${CMAKE_THREAD_LIBS_INIT}) list(APPEND CMAKE_PREFIX_PATH - ${DEP_DIR} + ${DEP_DIR}/asio/asio + ${DEP_DIR} ) - find_package(LZ4 REQUIRED) - target_include_directories(${target} PRIVATE ${DEP_DIR}/asio/asio/include) endif () else () - target_include_directories(${target} PRIVATE ${DEP_DIR}/asio/asio/include - ) list(APPEND CMAKE_PREFIX_PATH ${DEP_DIR}/mbedtls/mbedtls-${PLAT} ${DEP_DIR}/lz4/lz4-${PLAT} + ${DEP_DIR}/asio/asio ) list(APPEND CMAKE_LIBRARY_PATH ${DEP_DIR}/mbedtls/mbedtls-${PLAT}/library ) - - find_package(LZ4 REQUIRED) endif () + find_package(lz4 REQUIRED) + find_package(asio REQUIRED) + target_link_libraries(${target} lz4::lz4) + target_link_libraries(${target} asio::asio) + add_ssl_library(${target}) if (APPLE) @@ -109,9 +108,7 @@ function(add_core_dependencies target) target_link_libraries(${target} pthread) endif() - target_include_directories(${target} PRIVATE ${LZ4_INCLUDE_DIR}) - - target_link_libraries(${target} ${EXTRA_LIBS} ${LZ4_LIBRARY}) + target_link_libraries(${target} ${EXTRA_LIBS}) if (USE_WERROR) if (MSVC) diff --git a/test/unittests/CMakeLists.txt b/test/unittests/CMakeLists.txt index 3b4469e0c..f0291a05b 100644 --- a/test/unittests/CMakeLists.txt +++ b/test/unittests/CMakeLists.txt @@ -27,19 +27,6 @@ endif() include(dlgoogletest) -# Extra includes/libraries that are currently only use by the core unit test -FIND_PATH(LZO_INCLUDE_DIR NAMES lzo/lzo1x.h) -FIND_LIBRARY(LZO_LIBRARIES NAMES lzo2) - -if (LZO_INCLUDE_DIR AND LZO_LIBRARIES) - list(APPEND CORE_TEST_DEFINES -DHAVE_LZO) - list(APPEND EXTRA_LIBS ${LZO_LIBRARIES}) - list(APPEND EXTRA_INCLUDES ${LZO_INCLUDE_DIR}) - message("lzo found, running lzo compression tests") -else () - message("lzo not found, skipping lzo compression tests") -endif () - set(CORE_TEST_DEFINES -DOPENVPN_FORCE_TUN_NULL -DUNIT_TEST @@ -136,7 +123,23 @@ add_json_library(coreUnitTests) # xxHash target_compile_definitions(coreUnitTests PRIVATE -DHAVE_XXHASH) -target_include_directories(coreUnitTests PRIVATE ${DEP_DIR}/xxHash) +if(NOT MSVC) + list(APPEND CMAKE_PREFIX_PATH + ${DEP_DIR}/xxHash + ${DEP_DIR} + ) +endif() +find_package(xxHash REQUIRED) +target_link_libraries(coreUnitTests xxHash::xxhash) + +find_package(LZO) +if (LZO_FOUND) + target_compile_definitions(coreUnitTests PRIVATE -DHAVE_LZO) + target_link_libraries(coreUnitTests lzo::lzo) + message("lzo found, running lzo compression tests") +else () + message("lzo not found, skipping lzo compression tests") +endif () target_link_libraries(coreUnitTests ${GTEST_LIB} ${EXTRA_LIBS}) From a830d1e09c2f7759dcc886965902225a1bae16f7 Mon Sep 17 00:00:00 2001 From: Frank Lichtenheld Date: Fri, 24 Feb 2023 21:39:54 +0100 Subject: [PATCH 12/31] lzo_asym_impl: fix unaligned access Helpfully the comment above the code actually provided a solution... Signed-off-by: Frank Lichtenheld (cherry picked from commit db7ea3d96a12c178fa1a08bf065e45b41d7ffb03) --- openvpn/compress/lzoasym_impl.hpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/openvpn/compress/lzoasym_impl.hpp b/openvpn/compress/lzoasym_impl.hpp index e0f7d63aa..86bb8e0ba 100644 --- a/openvpn/compress/lzoasym_impl.hpp +++ b/openvpn/compress/lzoasym_impl.hpp @@ -107,9 +107,7 @@ inline size_t ptr_diff(const T *a, const T *b) // read uint16_t from memory inline size_t get_u16(const unsigned char *p) { - // NOTE: assumes little-endian and unaligned 16-bit access is okay. - // For a slower alternative without these assumptions, try: p[0] | (p[1] << 8) - return get_mem(p); + return p[0] | (p[1] << 8); } /** From 38ef9f2786afc0cd6181956ee4f77fcb131ea79f Mon Sep 17 00:00:00 2001 From: Frank Lichtenheld Date: Wed, 1 Mar 2023 10:30:38 +0100 Subject: [PATCH 13/31] mingw: disable VCPKG_APPLOCAL_DEPS in mingw presets We do not want to force a dependency on powershell. Copying the right dlls is rather trivial. Same change as commit commit e9e49239ce3fd9d81f9f4df605f103d100462e97 for build-vcpkg script. Signed-off-by: Frank Lichtenheld (cherry picked from commit 1f5aa5822327058695013af080a1e236fa39686e) --- CMakePresets.json | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CMakePresets.json b/CMakePresets.json index 70fc315ca..1d36c1bdc 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -45,6 +45,10 @@ "value": "ON", "type": "BOOL" }, + "VCPKG_APPLOCAL_DEPS": { + "value": "OFF", + "type": "BOOL" + }, "VCPKG_MANIFEST_DIR": "${sourceDir}/deps/vcpkg_manifests/mingw" } }, From c9939d271b31b06efc6e770dabd51293721eed70 Mon Sep 17 00:00:00 2001 From: Frank Lichtenheld Date: Mon, 20 Mar 2023 13:37:05 +0100 Subject: [PATCH 14/31] CMake: Reorder includes to prefer asio By adding the asio includes first we have a better chance to force using "our" asio. This can be important since some parts of the code require a patched version. The actual "core" parts of the code work fine with upstream asio however, so I also do not want to force the patched asio by requiring a special header name or directory structure. So this is a compromise solution which hopefully works for most use-cases. Signed-off-by: Frank Lichtenheld (cherry picked from commit bc7f4be01b6249e8616988139a694908a132985c) --- cmake/findcoredeps.cmake | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/cmake/findcoredeps.cmake b/cmake/findcoredeps.cmake index 6886743ea..3d03a8c80 100644 --- a/cmake/findcoredeps.cmake +++ b/cmake/findcoredeps.cmake @@ -80,20 +80,24 @@ function(add_core_dependencies target) endif () else () list(APPEND CMAKE_PREFIX_PATH - ${DEP_DIR}/mbedtls/mbedtls-${PLAT} - ${DEP_DIR}/lz4/lz4-${PLAT} ${DEP_DIR}/asio/asio + ${DEP_DIR}/lz4/lz4-${PLAT} + ${DEP_DIR}/mbedtls/mbedtls-${PLAT} ) list(APPEND CMAKE_LIBRARY_PATH ${DEP_DIR}/mbedtls/mbedtls-${PLAT}/library ) endif () - find_package(lz4 REQUIRED) + # asio should go first since some of our code requires + # a patched version. So we want to prefer its include + # directories. find_package(asio REQUIRED) - target_link_libraries(${target} lz4::lz4) target_link_libraries(${target} asio::asio) + find_package(lz4 REQUIRED) + target_link_libraries(${target} lz4::lz4) + add_ssl_library(${target}) if (APPLE) From ce054c562c3d61139af36c8a0538aa2a7be34c8b Mon Sep 17 00:00:00 2001 From: Frank Lichtenheld Date: Mon, 24 Apr 2023 14:32:31 +0200 Subject: [PATCH 15/31] CMake: make doxygen generation work better - Use CURRENT source and binary dir to make this work even if used as a sub-directory in another project. - Make USE_MDFILE_AS_MAINPAGE actually work. It is only used when part of the INPUT and does not automatically add it to INPUT. - Make sure CMake uses the latest version of README.rst by using configure_file instead of file(COPY). - Improve EXCLUDE_PATTERNS. - Add NUM_PROC_THREADS. Signed-off-by: Frank Lichtenheld (cherry picked from commit 474de6c93ff900560b62100303cac95a98cba260) --- CMakeLists.txt | 13 +++++++------ cmake/ovpn-doxygen.cmake | 10 ++++++---- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index c4e03c93a..a661cb2a5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -41,16 +41,17 @@ add_subdirectory(openvpn/ovpnagent/mac) if (ENABLE_DOXYGEN) # Exclude some project specific directories set(DOXYGEN_EXCLUDE_PATTERNS - ${CMAKE_BINARY_DIR}/test/unittests/googletest-* - ${PROJECT_SOURCE_DIR}/deps/* - ${PROJECT_SOURCE_DIR}/test/unittests/googletest-*) + "*/vcpkg_installed/*" + "*/googletest-src/*" + "${PROJECT_SOURCE_DIR}/deps/*") # Use README.rst as the Doxygen main page # Due to some doxygen oddities, it rejects processing README.rst, but a .md file is fine # So we copy it into our build tree as a .md file and use that - file(COPY "${CMAKE_SOURCE_DIR}/README.rst" DESTINATION "${CMAKE_BINARY_DIR}/doxygen") - file(RENAME "${CMAKE_BINARY_DIR}/doxygen/README.rst" "${CMAKE_BINARY_DIR}/doxygen/mainpage.md") - set(DOXYGEN_USE_MDFILE_AS_MAINPAGE "${CMAKE_BINARY_DIR}/doxygen/mainpage.md") + configure_file("${CMAKE_CURRENT_SOURCE_DIR}/README.rst" + "${CMAKE_CURRENT_BINARY_DIR}/doxygen/mainpage.md" + COPYONLY) + set(DOXYGEN_USE_MDFILE_AS_MAINPAGE "${CMAKE_CURRENT_BINARY_DIR}/doxygen/mainpage.md") configure_doxygen("OpenVPN 3 Core Library" "doxygen/core") endif () diff --git a/cmake/ovpn-doxygen.cmake b/cmake/ovpn-doxygen.cmake index 7db874709..b321a4a63 100644 --- a/cmake/ovpn-doxygen.cmake +++ b/cmake/ovpn-doxygen.cmake @@ -7,22 +7,24 @@ function(configure_doxygen projname outputdir) # OpenVPN Logo file(WRITE - "${CMAKE_BINARY_DIR}/doxygen/openvpn-logo.svg" + "${CMAKE_CURRENT_BINARY_DIR}/doxygen/openvpn-logo.svg" "") set(DOXYGEN_PROJECT_NAME "${projname}") - set(DOXYGEN_PROJECT_LOGO "${CMAKE_BINARY_DIR}/doxygen/openvpn-logo.svg") + set(DOXYGEN_PROJECT_LOGO "${CMAKE_CURRENT_BINARY_DIR}/doxygen/openvpn-logo.svg") set(DOXYGEN_PROJECT_NUMBER "") set(DOXYGEN_OUTPUT_DIRECTORY "${outputdir}") set(DOXYGEN_GENERATE_HTML YES) set(DOXYGEN_EXTRACT_ALL YES) set(DOXYGEN_EXTRACT_PRIVATE YES) set(DOXYGEN_EXTRACT_STATIC YES) + set(DOXYGEN_NUM_PROC_THREADS 0) set(DOXYGEN_CALL_GRAPH "NO" CACHE STRING "Generate call graph images (value: YES/NO)") set(DOXYGEN_CALLER_GRAPH "YES" CACHE STRING "Generate caller graph images (value: YES/NO)") set(DOXYGEN_FILE_PATTERNS *.c *.h *.cc *.hh *.cpp *.hpp *.java *.i *.md *.rst) doxygen_add_docs(doxygen ALL - WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}" + "${DOXYGEN_USE_MDFILE_AS_MAINPAGE}" "${CMAKE_CURRENT_SOURCE_DIR}" + WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}" COMMENT "Generate Doxygen documentation") -endfunction () \ No newline at end of file +endfunction () From 14136ee923a98ee972972a62dd412b9a1789cfc7 Mon Sep 17 00:00:00 2001 From: Frank Lichtenheld Date: Fri, 28 Jul 2023 16:32:04 +0200 Subject: [PATCH 16/31] CMake: disable -Wmaybe-uninitialized for GCC builds This is very noisy with lots of false positives, especially in newer version of GCC. So for now disable this. Signed-off-by: Frank Lichtenheld (cherry picked from commit d7e8375fc5f6776e412ac8c5749c1e6c81485312) --- cmake/findcoredeps.cmake | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cmake/findcoredeps.cmake b/cmake/findcoredeps.cmake index 3d03a8c80..f1ee86b78 100644 --- a/cmake/findcoredeps.cmake +++ b/cmake/findcoredeps.cmake @@ -127,6 +127,10 @@ function(add_core_dependencies target) # target_compile_options(${target} PRIVATE /W4) else() target_compile_options(${target} PRIVATE -Wall -Wsign-compare) + if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU") + # disable noisy warnings + target_compile_options(${target} PRIVATE -Wno-maybe-uninitialized) + endif() endif() endfunction() From ef3b290de62c153a2f09997d2f73e090722bfcee Mon Sep 17 00:00:00 2001 From: Frank Lichtenheld Date: Mon, 31 Jul 2023 12:37:09 +0200 Subject: [PATCH 17/31] CMake: replace FindPythonIntp with FindPython3 The earlier were deprecated since CMake 3.12. Since CMake 3.27 this causes deprecation warnings. Should be safe nowadays to require CMake 3.12. Signed-off-by: Frank Lichtenheld (cherry picked from commit bb61350ae5c0175699b048d08a98e184447e6e2a) --- CMakeLists.txt | 2 +- client/CMakeLists.txt | 4 ++-- cmake/findswigdeps.cmake | 5 ++--- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a661cb2a5..293329166 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,4 +1,4 @@ -cmake_minimum_required(VERSION 3.10) +cmake_minimum_required(VERSION 3.12) cmake_policy(SET CMP0048 NEW) project(OpenVPN3-core VERSION 3) diff --git a/client/CMakeLists.txt b/client/CMakeLists.txt index 0987cfd40..e8921e8ef 100644 --- a/client/CMakeLists.txt +++ b/client/CMakeLists.txt @@ -16,8 +16,8 @@ if (${BUILD_SWIG_LIB}) ) add_core_dependencies(ovpnclilib) - target_link_libraries(ovpnclilib ${PYTHON_LIBRARIES}) - target_include_directories(ovpnclilib PRIVATE ${PYTHON_INCLUDE_DIRS} ${CMAKE_CURRENT_SOURCE_DIR}) + target_link_libraries(ovpnclilib ${Python3_LIBRARIES}) + target_include_directories(ovpnclilib PRIVATE ${Python3_INCLUDE_DIRS} ${CMAKE_CURRENT_SOURCE_DIR}) # Use proper python library name to generate _ovpncli.so/dylib/dll set_target_properties(ovpnclilib PROPERTIES OUTPUT_NAME "_ovpncli") diff --git a/cmake/findswigdeps.cmake b/cmake/findswigdeps.cmake index c2faec3f4..9bc3139b3 100644 --- a/cmake/findswigdeps.cmake +++ b/cmake/findswigdeps.cmake @@ -1,11 +1,10 @@ -find_package(PythonInterp) -find_package(PythonLibs) +find_package(Python3 COMPONENTS Interpreter Development) FIND_PACKAGE(SWIG 3.0) # We test building this library with python instead of java since that is easier to do and both languages should work -if (PYTHONLIBS_FOUND AND SWIG_FOUND) +if (Python3_Development_FOUND AND SWIG_FOUND) if (NOT WIN32) set(BUILD_SWIG_LIB TRUE) elseif("${CMAKE_EXE_LINKER_FLAGS}" MATCHES "x64") From 61c0ab7f6b583bf6f89b6bb26831084f6e80e4b5 Mon Sep 17 00:00:00 2001 From: Frank Lichtenheld Date: Fri, 13 Oct 2023 13:31:13 +0200 Subject: [PATCH 18/31] client: Switch to UseSWIG instead of manual custom command On modern CMake this gets us swig dependency management, which should reduce problems for incremental builds. Also it is just cleaner. Signed-off-by: Frank Lichtenheld (cherry picked from commit 72275db1d516cea1a37ef84efcdeb9636749d492) --- CMakeLists.txt | 2 +- client/CMakeLists.txt | 30 ++++++++++++++---------------- cmake/findswigdeps.cmake | 5 +++++ 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 293329166..d04f7317b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,4 +1,4 @@ -cmake_minimum_required(VERSION 3.12) +cmake_minimum_required(VERSION 3.13) cmake_policy(SET CMP0048 NEW) project(OpenVPN3-core VERSION 3) diff --git a/client/CMakeLists.txt b/client/CMakeLists.txt index e8921e8ef..e1ecf8f3d 100644 --- a/client/CMakeLists.txt +++ b/client/CMakeLists.txt @@ -1,19 +1,21 @@ include(findcoredeps) include(findswigdeps) -if (${BUILD_SWIG_LIB}) - add_custom_command( - OUTPUT ovpncli_wrap.cxx ovpncli_wrap.h - COMMENT "Generating ovpncli Python swig files" - COMMAND ${SWIG_EXECUTABLE} -c++ -python -threads -DSWIG_PYTHON_2_UNICODE -outcurrentdir -I${CORE_DIR} ${CMAKE_CURRENT_SOURCE_DIR}/ovpncli.i - DEPENDS ovpncli.i - ) +if (BUILD_SWIG_LIB) + set_property(SOURCE ovpncli.i PROPERTY CPLUSPLUS ON) + if (NOT MSVC) + # Swig generates code with deprecated python declarations + set_property(SOURCE ovpncli.i PROPERTY GENERATED_COMPILE_OPTIONS -Wno-deprecated-declarations -Wno-sometimes-uninitialized -Wno-class-memaccess -Wno-unknown-warning-option) + endif() - add_library(ovpnclilib SHARED - ovpncli.cpp - ovpncli_wrap.cxx - ovpncli_wrap.h - ) + swig_add_library(ovpnclilib + TYPE SHARED + LANGUAGE python + SOURCES ovpncli.cpp ovpncli.i + ) + set_property(TARGET ovpnclilib PROPERTY SWIG_COMPILE_DEFINITIONS SWIG_PYTHON_2_UNICODE) + set_property(TARGET ovpnclilib PROPERTY SWIG_COMPILE_OPTIONS -threads) + set_property(TARGET ovpnclilib PROPERTY SWIG_INCLUDE_DIRECTORIES ${CORE_DIR}) add_core_dependencies(ovpnclilib) target_link_libraries(ovpnclilib ${Python3_LIBRARIES}) @@ -23,8 +25,4 @@ if (${BUILD_SWIG_LIB}) set_target_properties(ovpnclilib PROPERTIES OUTPUT_NAME "_ovpncli") set_target_properties(ovpnclilib PROPERTIES PREFIX "") - if (NOT WIN32) - # Swig generates code with deprecated python declarations - set_source_files_properties(ovpncli_wrap.cxx PROPERTIES COMPILE_FLAGS "-Wno-deprecated-declarations -Wno-sometimes-uninitialized -Wno-class-memaccess -Wno-unknown-warning-option") - endif() endif () diff --git a/cmake/findswigdeps.cmake b/cmake/findswigdeps.cmake index 9bc3139b3..5b4dd4652 100644 --- a/cmake/findswigdeps.cmake +++ b/cmake/findswigdeps.cmake @@ -2,6 +2,11 @@ find_package(Python3 COMPONENTS Interpreter Development) FIND_PACKAGE(SWIG 3.0) +include(UseSWIG) +if (CMAKE_VERSION VERSION_GREATER "3.20") + set(SWIG_USE_SWIG_DEPENDENCIES TRUE) +endif() + # We test building this library with python instead of java since that is easier to do and both languages should work if (Python3_Development_FOUND AND SWIG_FOUND) From 82d8dbb975db5c18fd35ef11f99b0172aa3e2384 Mon Sep 17 00:00:00 2001 From: Frank Lichtenheld Date: Fri, 13 Oct 2023 15:18:45 +0200 Subject: [PATCH 19/31] CMake: accept all NEW policies in released CMake versions Using the argument to cmake_minimum_required will set all policies up to to NEW. We might need to fix some issues arising from that, but this means that modern CMake can already behave like it wants even with leaving so that we can support old distros (currently Debian 10). Signed-off-by: Frank Lichtenheld (cherry picked from commit 268bf42b9e7dbb2949fbf189378a0463f50a0360) --- CMakeLists.txt | 3 +-- cmake/findcoredeps.cmake | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d04f7317b..4aeeccea0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,5 +1,4 @@ -cmake_minimum_required(VERSION 3.13) -cmake_policy(SET CMP0048 NEW) +cmake_minimum_required(VERSION 3.13...3.28) project(OpenVPN3-core VERSION 3) diff --git a/cmake/findcoredeps.cmake b/cmake/findcoredeps.cmake index f1ee86b78..cfa21205d 100644 --- a/cmake/findcoredeps.cmake +++ b/cmake/findcoredeps.cmake @@ -1,4 +1,4 @@ -cmake_minimum_required(VERSION 3.10) +cmake_minimum_required(VERSION 3.13...3.28) set(CMAKE_CXX_STANDARD 17) From 53c35b10133b36680c09edb192dfb129b89e1d3a Mon Sep 17 00:00:00 2001 From: Frank Lichtenheld Date: Fri, 13 Oct 2023 15:20:47 +0200 Subject: [PATCH 20/31] CMake: Refine CXX_STANDARD setting - Set CXX_STANDARD_REQUIRED ON so that we error out early if CMake thinks that the compiler does not support the used standard. - Set CXX_EXTENSIONS OFF so that we get less compiler specific behavior. Signed-off-by: Frank Lichtenheld (cherry picked from commit 9b8797fe5e1aa5d42a6656e71c3c50b1ee8dc94d) --- CMakeLists.txt | 2 ++ cmake/findcoredeps.cmake | 5 ++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 4aeeccea0..b4ce8b854 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -21,6 +21,8 @@ set(CMAKE_LINKER_FLAGS_ASAN set(CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/cmake ${CMAKE_MODULE_PATH}) set(CMAKE_CXX_STANDARD 17) +set(CMAKE_CXX_STANDARD_REQUIRED ON) +set(CMAKE_CXX_EXTENSIONS OFF) set(CMAKE_C_STANDARD 99) include(CTest) diff --git a/cmake/findcoredeps.cmake b/cmake/findcoredeps.cmake index cfa21205d..6e9f65092 100644 --- a/cmake/findcoredeps.cmake +++ b/cmake/findcoredeps.cmake @@ -1,12 +1,11 @@ cmake_minimum_required(VERSION 3.13...3.28) set(CMAKE_CXX_STANDARD 17) - -#cmake_policy(SET CMP0079 NEW) +set(CMAKE_CXX_STANDARD_REQUIRED ON) +set(CMAKE_CXX_EXTENSIONS OFF) set(CORE_DIR ${CMAKE_CURRENT_LIST_DIR}/..) - set(DEP_DIR ${CORE_DIR}/../deps CACHE PATH "Dependencies") option(USE_MBEDTLS "Use mbed TLS instead of OpenSSL") From 3bd3915d0a6eebf05e4db7f2256b5f632e6e7768 Mon Sep 17 00:00:00 2001 From: Frank Lichtenheld Date: Tue, 17 Oct 2023 15:08:34 +0200 Subject: [PATCH 21/31] CMake: Add USE_WCONVERSION option Makes it easier to test with -Wconversion, e.g. in Jenkins. For now disable -Wsign-conversion. That is the default in g++, but not clang++. Once we have fixed all -Wsign-conversion warnings, we can enable it for both. For now disable -Wenum-enum-conversion. Only present in clang++. Not clear whether cleaning those up will be worth the effort. Disable -ferror-limit in clang++. This ensures that it always displays all errors. Signed-off-by: Frank Lichtenheld (cherry picked from commit 6e7a98b5f461c83356f04047b869b37264c6d04e) --- cmake/findcoredeps.cmake | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/cmake/findcoredeps.cmake b/cmake/findcoredeps.cmake index 6e9f65092..aa825b649 100644 --- a/cmake/findcoredeps.cmake +++ b/cmake/findcoredeps.cmake @@ -10,6 +10,7 @@ set(DEP_DIR ${CORE_DIR}/../deps CACHE PATH "Dependencies") option(USE_MBEDTLS "Use mbed TLS instead of OpenSSL") option(USE_WERROR "Treat compiler warnings as errors (-Werror)") +option(USE_WCONVERSION "Enable -Wconversion") if (DEFINED ENV{DEP_DIR}) message("Overriding DEP_DIR setting with environment variable $ENV{DEP_DIR}") @@ -126,10 +127,17 @@ function(add_core_dependencies target) # target_compile_options(${target} PRIVATE /W4) else() target_compile_options(${target} PRIVATE -Wall -Wsign-compare) + if (USE_WCONVERSION) + target_compile_options(${target} PRIVATE -Wconversion -Wno-sign-conversion) + endif() if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU") # disable noisy warnings target_compile_options(${target} PRIVATE -Wno-maybe-uninitialized) endif() + if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang") + # display all warnings + target_compile_options(${target} PRIVATE -ferror-limit=0 -Wno-enum-enum-conversion) + endif() endif() endfunction() From 458e5df1abcb75fc07eba16364c437f3e8dd59f2 Mon Sep 17 00:00:00 2001 From: Frank Lichtenheld Date: Wed, 13 Mar 2024 14:18:39 +0100 Subject: [PATCH 22/31] test_cliopt: Refactor by using parameterized tests This makes it easier to add other configuration variants to test. Signed-off-by: Frank Lichtenheld --- test/unittests/test_cliopt.cpp | 148 ++++++++++++++------------------- 1 file changed, 63 insertions(+), 85 deletions(-) diff --git a/test/unittests/test_cliopt.cpp b/test/unittests/test_cliopt.cpp index 1a881da06..a0dab0c66 100644 --- a/test/unittests/test_cliopt.cpp +++ b/test/unittests/test_cliopt.cpp @@ -38,6 +38,14 @@ std::string minimalConfig = certconfig + "\n" + "client\n" "remote wooden.box\n"; +class ValidConfigs : public testing::TestWithParam +{ +}; +typedef std::pair config_error; +class InvalidConfigs : public testing::TestWithParam +{ +}; + void load_client_config(const std::string &config_content) { OptionList options; @@ -58,45 +66,38 @@ void load_client_config(const std::string &config_content) ClientOptions cliopt(options, config); } -TEST(config, missingRequiredOption) +TEST_P(ValidConfigs, valid_config) { - ParseClientConfig conf = ParseClientConfig::parse("mode server"); - EXPECT_EQ(conf.error(), true); - EXPECT_EQ(conf.message(), "ERR_PROFILE_OPTION: option_error: remote option not specified"); + load_client_config(GetParam()); } -TEST(config, parse_missing_option) +TEST_P(InvalidConfigs, config_throws_option_error) { OVPN_EXPECT_THROW( - load_client_config(std::string("remote wooden.box\n") + "mode server" - + "\n\n" + dummysecp256cert + "\n"), + load_client_config(GetParam().first), option_error, - "option 'cert' not found"); + GetParam().second); } -TEST(config, parse_forbidden_option) +TEST(config, missingRequiredOption) { - OVPN_EXPECT_THROW( - load_client_config(minimalConfig + "mode"), - option_error, - "Only 'mode p2p' supported"); - - OVPN_EXPECT_THROW( - load_client_config(minimalConfig + "mode server"), - option_error, - "Only 'mode p2p' supported"); - - OVPN_EXPECT_THROW( - load_client_config(minimalConfig + "key-method 1"), - option_error, - "Only 'key-method 2' is supported"); - - OVPN_EXPECT_THROW( - load_client_config(minimalConfig + "fragment"), - option_error, - "sorry, 'fragment' directive is not supported"); + ParseClientConfig conf = ParseClientConfig::parse("mode server"); + EXPECT_EQ(conf.error(), true); + EXPECT_EQ(conf.message(), "ERR_PROFILE_OPTION: option_error: remote option not specified"); } +INSTANTIATE_TEST_SUITE_P( + optionError, + InvalidConfigs, + testing::Values( + config_error{std::string("remote wooden.box\n") + "mode server" + + "\n\n" + dummysecp256cert + "\n", + "option 'cert' not found"}, + config_error{minimalConfig + "mode", "Only 'mode p2p' supported"}, + config_error{minimalConfig + "mode server", "Only 'mode p2p' supported"}, + config_error{minimalConfig + "key-method 1", "Only 'key-method 2' is supported"}, + config_error{minimalConfig + "fragment", "sorry, 'fragment' directive is not supported"})); + TEST(config, parse_unknown_option) { OVPN_EXPECT_THROW( @@ -105,27 +106,20 @@ TEST(config, parse_unknown_option) "UNKNOWN/UNSUPPORTED OPTIONS"); } -TEST(config, duplicate_option) -{ - /* A duplicate option should not cause our parser to fail */ - load_client_config(minimalConfig + "cipher AES-192-CBC\ncipher AES-256-GCM\n"); -} - -TEST(config, parse_ignore_unkown) -{ - /* Bikeshed colour is ignored should throw no error */ - load_client_config(minimalConfig + "ignore-unknown-option bikeshed-colour bikeshed-color\n" - "ignore-unknown-option danish axe phk\n" - "bikeshed-colour green"); - - load_client_config(minimalConfig + "setenv opt bikeshed-paint silver with sparkling"); -} - -TEST(config, ignore_warning_option) -{ - load_client_config(minimalConfig + "tun-ipv6\n"); - load_client_config(minimalConfig + "opt-verify\n"); -} +INSTANTIATE_TEST_SUITE_P( + minimalConfigs, + ValidConfigs, + testing::Values( + /* A duplicate option should not cause our parser to fail */ + minimalConfig + "cipher AES-192-CBC\ncipher AES-256-GCM\n", + /* Bikeshed colour is ignored should throw no error */ + minimalConfig + "ignore-unknown-option bikeshed-colour bikeshed-color\n" + "ignore-unknown-option danish axe phk\n" + "bikeshed-colour green", + minimalConfig + "setenv opt bikeshed-paint silver with sparkling", + /* warnings, but no errors */ + minimalConfig + "tun-ipv6\n", + minimalConfig + "opt-verify\n")); TEST(config, parse_management) { @@ -238,42 +232,26 @@ TEST(config, multiple_option_errors) os.str()); } -TEST(config, client_missing_in_config) -{ - std::string configNoClient = certconfig + "\nremote 1.2.3.4\n"; - OVPN_EXPECT_THROW( - load_client_config(configNoClient), - option_error, - "option_error: Neither 'client' nor both 'tls-client' and 'pull' options declared. OpenVPN3 client only supports --client mode."); -} - -TEST(config, pull_and_tlsclient_in_config) -{ - std::string configNoClient = certconfig + "\nremote 1.2.3.4\ntls-client\npull\n"; - /* Should not trigger an error, even without --client in place */ - load_client_config(configNoClient); -} - -TEST(config, pull_and_client_and_tlsclient_in_config) -{ - std::string configNoClient = certconfig + "\nremote 1.2.3.4\ntls-client\npull\nclient\n"; - /* Should not trigger an error. Redundant options are no problem */ - load_client_config(configNoClient); -} - -TEST(config, onlypullortlsclient) -{ - std::array options{"tls-client", "pull"}; - - for (const auto &opt : options) - { - std::string configNoClient = certconfig + "\nremote 1.2.3.4\n" + opt + "\n"; - OVPN_EXPECT_THROW( - load_client_config(configNoClient), - option_error, - "option_error: Neither 'client' nor both 'tls-client' and 'pull' options declared. OpenVPN3 client only supports --client mode."); - } -} +INSTANTIATE_TEST_SUITE_P( + clientPull, + ValidConfigs, + testing::Values( + /* Should not trigger an error, even without --client in place */ + certconfig + "\nremote 1.2.3.4\ntls-client\npull\n", + /* Should not trigger an error. Redundant options are no problem */ + certconfig + "\nremote 1.2.3.4\ntls-client\npull\nclient\n", + certconfig + "\nremote 1.2.3.4\nclient\ntls-client\n")); + +INSTANTIATE_TEST_SUITE_P( + clientPull, + InvalidConfigs, + testing::Values( + config_error{certconfig + "\nremote 1.2.3.4\n", + "option_error: Neither 'client' nor both 'tls-client' and 'pull' options declared. OpenVPN3 client only supports --client mode."}, + config_error{certconfig + "\nremote 1.2.3.4\ntls-client\n", + "option_error: Neither 'client' nor both 'tls-client' and 'pull' options declared. OpenVPN3 client only supports --client mode."}, + config_error{certconfig + "\nremote 1.2.3.4\npull\n", + "option_error: Neither 'client' nor both 'tls-client' and 'pull' options declared. OpenVPN3 client only supports --client mode."})); TEST(config, meta_option_in_content) { From 763176ea7033e5f9fae41461a0e75d1bc1383db1 Mon Sep 17 00:00:00 2001 From: Frank Lichtenheld Date: Fri, 8 Mar 2024 14:28:30 +0100 Subject: [PATCH 23/31] Options: do not error out on client + pull Config client pull was not correctly handled like client + tls-client since the code short-circuited if tls-client wasn't set and so didn't touch pull option. Github: #277 Signed-off-by: Frank Lichtenheld --- openvpn/client/cliopthelper.hpp | 8 ++++++-- test/unittests/test_cliopt.cpp | 1 + 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/openvpn/client/cliopthelper.hpp b/openvpn/client/cliopthelper.hpp index 5bb3050ba..0d156a6fa 100644 --- a/openvpn/client/cliopthelper.hpp +++ b/openvpn/client/cliopthelper.hpp @@ -366,8 +366,12 @@ class ParseClientConfig // add in missing options bool added = false; - // client - if (options.exists("tls-client") && options.exists("pull")) + /* client + Ensure that we always look at both options, so they register as touched */ + const bool tls_client_exists = options.exists("tls-client"); + const bool pull_exists = options.exists("pull"); + + if (tls_client_exists && pull_exists) { Option opt; opt.push_back("client"); diff --git a/test/unittests/test_cliopt.cpp b/test/unittests/test_cliopt.cpp index a0dab0c66..6c97d85ab 100644 --- a/test/unittests/test_cliopt.cpp +++ b/test/unittests/test_cliopt.cpp @@ -240,6 +240,7 @@ INSTANTIATE_TEST_SUITE_P( certconfig + "\nremote 1.2.3.4\ntls-client\npull\n", /* Should not trigger an error. Redundant options are no problem */ certconfig + "\nremote 1.2.3.4\ntls-client\npull\nclient\n", + certconfig + "\nremote 1.2.3.4\npull\nclient\n", certconfig + "\nremote 1.2.3.4\nclient\ntls-client\n")); INSTANTIATE_TEST_SUITE_P( From 1b4f736bb90c05ba858d86b764c5a70e7b824492 Mon Sep 17 00:00:00 2001 From: Arne Schwabe Date: Wed, 14 Feb 2024 14:39:51 +0100 Subject: [PATCH 24/31] Make macOS gateway detection IPv6 aware and use actual server address This also move the building IV_HWADDR peer info variable to the point that the server address is actually available. This also avoids failing to connect when push-peer-info is enabled and there is no IPv4 default gateway. The new code will always pick the device that holds the route to the current remote. Signed-off-by: Arne Schwabe --- openvpn/addr/ip.hpp | 4 +- openvpn/addr/ipv6.hpp | 4 + openvpn/client/cliopt.hpp | 18 ++- openvpn/client/cliproto.hpp | 1 + openvpn/netconf/hwaddr.hpp | 6 +- openvpn/ssl/peerinfo.hpp | 7 ++ openvpn/ssl/proto.hpp | 34 +++++- openvpn/tun/mac/{gwv4.hpp => gw.hpp} | 164 +++++++++++++-------------- test/unittests/test_ip.cpp | 6 + 9 files changed, 150 insertions(+), 94 deletions(-) rename openvpn/tun/mac/{gwv4.hpp => gw.hpp} (62%) diff --git a/openvpn/addr/ip.hpp b/openvpn/addr/ip.hpp index 0946226e3..92beb5eac 100644 --- a/openvpn/addr/ip.hpp +++ b/openvpn/addr/ip.hpp @@ -335,9 +335,9 @@ class Addr static Addr from_sockaddr(const struct sockaddr *sa) { if (sa->sa_family == AF_INET) - return from_ipv4(IPv4::Addr::from_sockaddr((struct sockaddr_in *)sa)); + return from_ipv4(IPv4::Addr::from_sockaddr(reinterpret_cast(sa))); else if (sa->sa_family == AF_INET6) - return from_ipv6(IPv6::Addr::from_sockaddr((struct sockaddr_in6 *)sa)); + return from_ipv6(IPv6::Addr::from_sockaddr(reinterpret_cast(sa))); else return Addr(); } diff --git a/openvpn/addr/ipv6.hpp b/openvpn/addr/ipv6.hpp index bb6ce906b..b991d7490 100644 --- a/openvpn/addr/ipv6.hpp +++ b/openvpn/addr/ipv6.hpp @@ -109,6 +109,10 @@ class Addr // NOTE: must be union-legal, so default constructor does not initial ret.sin6_port = htons(port); host_to_network_order((union ipv6addr *)&ret.sin6_addr.s6_addr, &u); ret.sin6_scope_id = scope_id_; +#ifdef SIN6_LEN + /* This is defined on both macOS and FreeBSD that have the sin6_len member */ + ret.sin6_len = sizeof(sockaddr_in6); +#endif return ret; } diff --git a/openvpn/client/cliopt.hpp b/openvpn/client/cliopt.hpp index 81591549e..0c487b504 100644 --- a/openvpn/client/cliopt.hpp +++ b/openvpn/client/cliopt.hpp @@ -1040,14 +1040,24 @@ class ClientOptions : public RC if (!config.sso_methods.empty()) pi->emplace_back("IV_SSO", config.sso_methods); + return pi; + } + + PeerInfo::Set::Ptr build_peer_info_transport(const Config &config, const ParseClientConfig &pcc) + { + PeerInfo::Set::Ptr pi(new PeerInfo::Set); + // MAC address if (pcc.pushPeerInfo()) { - std::string hwaddr = get_hwaddr(); + /* If we override the HWADDR, we add it at this time statically. If we need to + * dynamically discover it from the transport it will be added in + * \c build_connect_time_peer_info_string instead */ if (!config.hw_addr_override.empty()) + { pi->emplace_back("IV_HWADDR", config.hw_addr_override); - else if (!hwaddr.empty()) - pi->emplace_back("IV_HWADDR", hwaddr); + } + pi->emplace_back("IV_SSL", get_ssl_library_version()); if (!config.platform_version.empty()) @@ -1056,6 +1066,7 @@ class ClientOptions : public RC return pi; } + void next(RemoteList::Advance type) { bool omit_next = false; @@ -1253,6 +1264,7 @@ class ClientOptions : public RC cp->load(opt, *proto_context_options, config.default_key_direction, false); cp->set_xmit_creds(!autologin || pcc.hasEmbeddedPassword() || autologin_sessions); cp->extra_peer_info = build_peer_info(config, pcc, autologin_sessions); + cp->extra_peer_info_push_peerinfo = pcc.pushPeerInfo(); cp->frame = frame; cp->now = &now_; cp->rng = rng; diff --git a/openvpn/client/cliproto.hpp b/openvpn/client/cliproto.hpp index d83973fda..ef1491dfe 100644 --- a/openvpn/client/cliproto.hpp +++ b/openvpn/client/cliproto.hpp @@ -494,6 +494,7 @@ class Session : ProtoContext, { try { + Base::conf().build_connect_time_peer_info_string(transport); OPENVPN_LOG("Connecting to " << server_endpoint_render()); Base::set_protocol(transport->transport_protocol()); Base::start(); diff --git a/openvpn/netconf/hwaddr.hpp b/openvpn/netconf/hwaddr.hpp index d0e27df0b..a9f3d32d5 100644 --- a/openvpn/netconf/hwaddr.hpp +++ b/openvpn/netconf/hwaddr.hpp @@ -32,13 +32,13 @@ #if defined(OPENVPN_PLATFORM_WIN) && !defined(OPENVPN_PLATFORM_UWP) #include #elif defined(OPENVPN_PLATFORM_MAC) -#include +#include #elif defined(TARGET_OS_IPHONE) #include #endif namespace openvpn { -inline std::string get_hwaddr() +inline std::string get_hwaddr([[maybe_unused]] IP::Addr server_addr) { #if defined(OPENVPN_PLATFORM_WIN) && !defined(OPENVPN_PLATFORM_UWP) const TunWin::Util::BestGateway dg{AF_INET}; @@ -53,7 +53,7 @@ inline std::string get_hwaddr() } } #elif defined(OPENVPN_PLATFORM_MAC) - const MacGatewayInfoV4 gw; + const MacGatewayInfo gw{server_addr}; if (gw.hwaddr_defined()) { const MACAddr &mac = gw.hwaddr(); diff --git a/openvpn/ssl/peerinfo.hpp b/openvpn/ssl/peerinfo.hpp index 170799e63..740d7461d 100644 --- a/openvpn/ssl/peerinfo.hpp +++ b/openvpn/ssl/peerinfo.hpp @@ -90,6 +90,13 @@ struct Set : public std::vector, public RCCopyable #include #include +#include #include #include #include @@ -82,6 +83,7 @@ #include #include #include +#include #if OPENVPN_DEBUG_PROTO >= 1 @@ -369,9 +371,17 @@ class ProtoContext Time::Duration keepalive_timeout; // timeout period after primary KeyContext reaches ACTIVE state Time::Duration keepalive_timeout_early; // timeout period before primary KeyContext reaches ACTIVE state - // extra peer info key/value pairs generated by client app + //! extra peer info key/value pairs generated by client app PeerInfo::Set::Ptr extra_peer_info; + /** extra peer information that depends on the state of the underlying transport and needs to be initialised + * after the transport is initialised but before the IV variables are sent */ + PeerInfo::Set::Ptr extra_peer_info_transport; + + /** When the extra_peer_info_transport is being built, we need to remember if it should include the more + * sensitive information that push-peer-info includes */ + bool extra_peer_info_push_peerinfo = false; + // op header bool enable_op32 = false; int remote_peer_id = -1; // -1 to disable @@ -934,6 +944,26 @@ class ProtoContext return initial_options; } + /** + * This method adds the parts of the peer info string that depend on the state of the + * connection, especially the remote that we are connecting to. + */ + void build_connect_time_peer_info_string(TransportClient::Ptr transport) + { + extra_peer_info_transport.reset(new PeerInfo::Set{}); + if (extra_peer_info_push_peerinfo) + { + /* check if the IV_HWADDR is already present in the extra_peer_info set as it has then been + * statically been overridden */ + if (!extra_peer_info->contains_key("IV_HWADDR")) + { + std::string hwaddr = get_hwaddr(transport->server_endpoint_addr()); + if (!hwaddr.empty()) + extra_peer_info_transport->emplace_back("IV_HWADDR", hwaddr); + } + } + } + // generate a string summarizing information about the client // including capabilities std::string peer_info_string() const @@ -992,6 +1022,8 @@ class ProtoContext out << compstr; if (extra_peer_info) out << extra_peer_info->to_string(); + if (extra_peer_info_transport) + out << extra_peer_info_transport->to_string(); if (is_bs64_cipher(dc.cipher())) out << "IV_BS64DL=1\n"; // indicate support for data limits when using 64-bit block-size ciphers, version 1 (CVE-2016-6329) if (relay_mode) diff --git a/openvpn/tun/mac/gwv4.hpp b/openvpn/tun/mac/gw.hpp similarity index 62% rename from openvpn/tun/mac/gwv4.hpp rename to openvpn/tun/mac/gw.hpp index 5cd1ffe0d..e7c08427b 100644 --- a/openvpn/tun/mac/gwv4.hpp +++ b/openvpn/tun/mac/gw.hpp @@ -31,6 +31,7 @@ #include #include #include +#include #include #include @@ -47,9 +48,10 @@ #include #include #include +#include namespace openvpn { -class MacGatewayInfoV4 +class MacGatewayInfo { struct rtmsg { @@ -60,12 +62,21 @@ class MacGatewayInfoV4 #define OPENVPN_ROUNDUP(a) \ ((a) > 0 ? (1 + (((a)-1) | (sizeof(std::uint32_t) - 1))) : sizeof(std::uint32_t)) -#define OPENVPN_NEXTADDR(w, u) \ - if (rtm_addrs & (w)) \ - { \ - l = OPENVPN_ROUNDUP(u.sa_len); \ - std::memmove(cp, &(u), l); \ - cp += l; \ +#define OPENVPN_NEXTADDR(w, u) \ + if (rtm_addrs & (w)) \ + { \ + l = OPENVPN_ROUNDUP(u.sin_len); \ + std::memmove(cp, &(u), l); \ + cp += l; \ + } + + +#define OPENVPN_NEXTADDR6(w, u) \ + if (rtm_addrs & (w)) \ + { \ + l = OPENVPN_ROUNDUP(u.sin6_len); \ + std::memmove(cp, &(u), l); \ + cp += l; \ } #define OPENVPN_ADVANCE(x, n) \ @@ -82,13 +93,13 @@ class MacGatewayInfoV4 IFACE_DEFINED = (1 << 3), /* set if iface is defined */ }; - MacGatewayInfoV4() - : flags_(0) + MacGatewayInfo(IP::Addr dest) { struct rtmsg m_rtmsg; ScopedFD sockfd; int seq, l, pid, rtm_addrs; - struct sockaddr so_dst, so_mask; + sockaddr_in so_dst{}, so_mask{}; + sockaddr_in6 so_dst6{}; char *cp = m_rtmsg.m_space; struct sockaddr *gate = nullptr, *ifp = nullptr, *sa; struct rt_msghdr *rtm_aux; @@ -99,8 +110,6 @@ class MacGatewayInfoV4 rtm_addrs = RTA_DST | RTA_NETMASK | RTA_IFP; std::memset(&m_rtmsg, 0, sizeof(m_rtmsg)); - std::memset(&so_dst, 0, sizeof(so_dst)); - std::memset(&so_mask, 0, sizeof(so_mask)); std::memset(&m_rtmsg.m_rtm, 0, sizeof(struct rt_msghdr)); m_rtmsg.m_rtm.rtm_type = RTM_GET; @@ -109,13 +118,24 @@ class MacGatewayInfoV4 m_rtmsg.m_rtm.rtm_seq = ++seq; m_rtmsg.m_rtm.rtm_addrs = rtm_addrs; - so_dst.sa_family = AF_INET; - so_dst.sa_len = sizeof(struct sockaddr_in); - so_mask.sa_family = AF_INET; - so_mask.sa_len = sizeof(struct sockaddr_in); + const bool ipv6 = dest.is_ipv6(); - OPENVPN_NEXTADDR(RTA_DST, so_dst); - OPENVPN_NEXTADDR(RTA_NETMASK, so_mask); + if (!ipv6) + { + so_dst = dest.to_ipv4().to_sockaddr(); + // 32 netmask to lookup the route to the destination + so_mask.sin_family = AF_INET; + so_mask.sin_addr.s_addr = 0xffffffff; + so_mask.sin_len = sizeof(struct sockaddr_in); + + OPENVPN_NEXTADDR(RTA_DST, so_dst); + OPENVPN_NEXTADDR(RTA_NETMASK, so_mask); + } + else + { + so_dst6 = dest.to_ipv6().to_sockaddr(); + OPENVPN_NEXTADDR6(RTA_DST, so_dst6); + } m_rtmsg.m_rtm.rtm_msglen = l = cp - (char *)&m_rtmsg; @@ -123,8 +143,12 @@ class MacGatewayInfoV4 sockfd.reset(socket(PF_ROUTE, SOCK_RAW, 0)); if (!sockfd.defined()) throw route_gateway_error("GDG: socket #1 failed"); - if (::write(sockfd(), (char *)&m_rtmsg, l) < 0) - throw route_gateway_error("GDG: problem writing to routing socket"); + + auto ret = ::write(sockfd(), (char *)&m_rtmsg, l); + if (ret < 0) + throw route_gateway_error("GDG: problem writing to routing socket: " + std::to_string(ret) + + " errno: " + std::to_string(errno) + " msg: " + ::strerror(errno)); + do { l = ::read(sockfd(), (char *)&m_rtmsg, sizeof(m_rtmsg)); @@ -156,7 +180,7 @@ class MacGatewayInfoV4 if (gate != nullptr) { /* get default gateway addr */ - gateway_.addr.reset_ipv4_from_uint32(ntohl(((struct sockaddr_in *)gate)->sin_addr.s_addr)); + gateway_.addr = IP::Addr::from_sockaddr(gate); if (!gateway_.addr.unspecified()) flags_ |= ADDR_DEFINED; @@ -174,94 +198,64 @@ class MacGatewayInfoV4 } } - /* get netmask of interface that owns default gateway */ - if (flags_ & IFACE_DEFINED) + /* get netmask of interface that owns default gateway. Querying the IPv6 netmask does not + * seem to work on my system (Arne), so it is disabled for now until we can figure out why it + * doesn't work */ + if (flags_ & IFACE_DEFINED && gateway_.addr.version() == IP::Addr::V4) { - struct ifreq ifr; + ifreq ifr{}; + sa_family_t sa_family; + + sa_family = AF_INET; + ifr.ifr_addr.sa_family = sa_family; + string::strncpynt(ifr.ifr_name, iface_, IFNAMSIZ); - sockfd.reset(socket(AF_INET, SOCK_DGRAM, 0)); + sockfd.reset(socket(sa_family, SOCK_DGRAM, 0)); if (!sockfd.defined()) throw route_gateway_error("GDG: socket #2 failed"); - std::memset(&ifr, 0, sizeof(ifr)); - ifr.ifr_addr.sa_family = AF_INET; - string::strncpynt(ifr.ifr_name, iface_, IFNAMSIZ); - if (::ioctl(sockfd(), SIOCGIFNETMASK, (char *)&ifr) < 0) - throw route_gateway_error("GDG: ioctl #1 failed"); - sockfd.close(); + throw route_gateway_error("GDG: ioctl SIOCGIFNETMASK failed"); - gateway_.netmask.reset_ipv4_from_uint32(ntohl(((struct sockaddr_in *)&ifr.ifr_addr)->sin_addr.s_addr)); + gateway_.netmask = IP::Addr::from_sockaddr(&ifr.ifr_addr)); flags_ |= NETMASK_DEFINED; + + sockfd.close(); } /* try to read MAC addr associated with interface that owns default gateway */ if (flags_ & IFACE_DEFINED) { - struct ifconf ifc; - const int bufsize = 4096; + struct ifaddrs *ifaddrp, *ifa; - const std::unique_ptr buffer(new char[bufsize]); - std::memset(buffer.get(), 0, bufsize); - sockfd.reset(socket(AF_INET, SOCK_DGRAM, 0)); - if (!sockfd.defined()) - throw route_gateway_error("GDG: socket #3 failed"); - - ifc.ifc_len = bufsize; - ifc.ifc_buf = buffer.get(); + if (getifaddrs(&ifaddrp) != 0) + { + throw route_gateway_error("GDG: getifaddrs failed errno: " + std::to_string(errno) + " msg: " + ::strerror(errno)); + } - if (::ioctl(sockfd(), SIOCGIFCONF, (char *)&ifc) < 0) - throw route_gateway_error("GDG: ioctl #2 failed"); - sockfd.close(); + /* put the pointer into a unique_ptr to have RAII (allow throwing etc) */ + std::unique_ptr<::ifaddrs, decltype(&::freeifaddrs)> ifap{ifaddrp, &::freeifaddrs}; - for (cp = buffer.get(); cp <= buffer.get() + ifc.ifc_len - sizeof(struct ifreq);) + for (ifa = ifap.get(); ifa != nullptr; ifa = ifa->ifa_next) { - ifreq ifr = {}; - std::memcpy(&ifr, cp, sizeof(ifr)); - const size_t len = sizeof(ifr.ifr_name) + std::max(sizeof(ifr.ifr_addr), size_t{ifr.ifr_addr.sa_len}); + if (ifa->ifa_addr == nullptr) + continue; - if (!ifr.ifr_addr.sa_family) - break; - if (!::strncmp(ifr.ifr_name, iface_, IFNAMSIZ)) + if (flags_ & IFACE_DEFINED + && ifa->ifa_addr->sa_family == AF_LINK + && !strncmp(ifa->ifa_name, iface_, IFNAMSIZ)) { - if (ifr.ifr_addr.sa_family == AF_LINK) - { - /* This is a confusing member access on multiple levels. - * - * struct sockaddr_dl is 20 bytes in size and has - * 12 bytes space for the hw address (6 bytes) - * and Ethernet interface name (max 16 bytes) - * - * So if the interface name is more than 6 byte, it - * extends beyond the struct. - * - * This struct is embedded into ifreq that has - * 16 bytes for a sockaddr and also expects this - * struct to potentially extend beyond the bounds of - * the struct. - * - * Since we only copied 32 bytes from cp to ifr but sdl - * might extend after ifr's end, we need to copy from - * cp directly to avoid missing out on extra bytes - * behind the struct - */ - const size_t sock_dl_len = std::max(sizeof(sockaddr_dl), size_t{ifr.ifr_addr.sa_len}); - - const std::unique_ptr sock_dl_buf(new char[sock_dl_len]); - std::memcpy(sock_dl_buf.get(), cp + offsetof(struct ifreq, ifr_addr), sock_dl_len); - - const struct sockaddr_dl *sockaddr_dl = reinterpret_cast(sock_dl_buf.get()); - - hwaddr_.reset(reinterpret_cast(LLADDR(sockaddr_dl))); - flags_ |= HWADDR_DEFINED; - } + const struct sockaddr_dl *sockaddr_dl = reinterpret_cast(ifa->ifa_addr); + + hwaddr_.reset(reinterpret_cast(LLADDR(sockaddr_dl))); + flags_ |= HWADDR_DEFINED; } - cp += len; } } } #undef OPENVPN_ROUNDUP #undef OPENVPN_NEXTADDR +#undef OPENVPN_NEXTADDR6 #undef OPENVPN_ADVANCE std::string info() const @@ -323,7 +317,7 @@ class MacGatewayInfoV4 } private: - unsigned int flags_; + unsigned int flags_ = 0; IP::AddrMaskPair gateway_; char iface_[16]; MACAddr hwaddr_; diff --git a/test/unittests/test_ip.cpp b/test/unittests/test_ip.cpp index 0aeded48d..f8f79b42b 100644 --- a/test/unittests/test_ip.cpp +++ b/test/unittests/test_ip.cpp @@ -8,6 +8,7 @@ #include #include #include +#include using namespace openvpn; @@ -134,6 +135,11 @@ void do_shift_tests(std::vector test_vectors, bool leftshift) auto ret = shifted_addr.to_sockaddr(); sockaddr_in6 cmp{}; +#if defined(SIN6_LEN) || defined(__APPLE__) || defined(__FreeBSD__) + /* Enable this test on the platforms that we know to have sin6_len + * to not only depend on SIN6_LEN */ + cmp.sin6_len = sizeof(sockaddr_in6); +#endif cmp.sin6_family = AF_INET6; for (int i = 0; i < 16; i++) { From 4f1f22159facf93220a8197ba900a0d1e9310338 Mon Sep 17 00:00:00 2001 From: Lev Stipakov Date: Fri, 23 Feb 2024 13:35:15 +0200 Subject: [PATCH 25/31] Improve handling of unknown options A few minor changes: - add ORGANIZATION meta option to ignore list - remove excessive OVPN_ACCESS_SERVER_ prefix from NO_WEB meta option - Increase status message length from 256 to 2048 to be able to show the full list of unsupported options Signed-off-by: Lev Stipakov --- client/ovpncli.cpp | 2 +- openvpn/client/cliopt.hpp | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/client/ovpncli.cpp b/client/ovpncli.cpp index 545f8d3ca..1b07af563 100644 --- a/client/ovpncli.cpp +++ b/client/ovpncli.cpp @@ -1156,7 +1156,7 @@ OPENVPN_CLIENT_EXPORT Status OpenVPNClient::status_from_exception(const std::exc { Status ret; ret.error = true; - ret.message = Unicode::utf8_printable(e.what(), 256 | Unicode::UTF8_PASS_FMT); + ret.message = Unicode::utf8_printable(e.what(), 2048 | Unicode::UTF8_PASS_FMT); // if exception is an ExceptionCode, translate the code // to return status string diff --git a/openvpn/client/cliopt.hpp b/openvpn/client/cliopt.hpp index 0c487b504..7f0226cd3 100644 --- a/openvpn/client/cliopt.hpp +++ b/openvpn/client/cliopt.hpp @@ -895,7 +895,8 @@ class ClientOptions : public RC "WSHOST", "WEB_CA_BUNDLE", "IS_OPENVPN_WEB_CA", - "OVPN_ACCESS_SERVER_NO_WEB", + "NO_WEB", + "ORGANIZATION" }; std::unordered_set ignore_unknown_option_list; From 7f3e61089b83217943053a6ddc90f36c31d22b3c Mon Sep 17 00:00:00 2001 From: David Sommerseth Date: Wed, 20 Mar 2024 16:58:05 +0100 Subject: [PATCH 26/31] mac/gw: Fix incorrect additional ) In commit 1b4f736bb90c05ba85, an additional parentheses was added to the MacGatewayInfo constructor. This results in code which cannot be compiled. Signed-off-by: David Sommerseth --- openvpn/tun/mac/gw.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openvpn/tun/mac/gw.hpp b/openvpn/tun/mac/gw.hpp index e7c08427b..4db0dc6ab 100644 --- a/openvpn/tun/mac/gw.hpp +++ b/openvpn/tun/mac/gw.hpp @@ -217,7 +217,7 @@ class MacGatewayInfo if (::ioctl(sockfd(), SIOCGIFNETMASK, (char *)&ifr) < 0) throw route_gateway_error("GDG: ioctl SIOCGIFNETMASK failed"); - gateway_.netmask = IP::Addr::from_sockaddr(&ifr.ifr_addr)); + gateway_.netmask = IP::Addr::from_sockaddr(&ifr.ifr_addr); flags_ |= NETMASK_DEFINED; sockfd.close(); From 43da4c8bc2fc09ed93be5e7ef3fe396533a066d6 Mon Sep 17 00:00:00 2001 From: David Sommerseth Date: Wed, 20 Mar 2024 17:02:20 +0100 Subject: [PATCH 27/31] unittest: Don't include sys/socket.h unconditionally The sys/socket.h header is not available on Windows. This issue was introduced in commit 1b4f736bb90c05ba85, so the same fencing used in that commit was also added around the #include statement. Signed-off-by: David Sommerseth --- test/unittests/test_ip.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/unittests/test_ip.cpp b/test/unittests/test_ip.cpp index f8f79b42b..de5c833e6 100644 --- a/test/unittests/test_ip.cpp +++ b/test/unittests/test_ip.cpp @@ -8,7 +8,10 @@ #include #include #include + +#if defined(SIN6_LEN) || defined(__APPLE__) || defined(__FreeBSD__) #include +#endif using namespace openvpn; From c07fb748a9d3f625c8be581fc28c471413bf7e9d Mon Sep 17 00:00:00 2001 From: Arne Schwabe Date: Wed, 27 Mar 2024 15:43:24 +0100 Subject: [PATCH 28/31] Fix IV_SSL and IV_HWADDR not reported The previous commit restructured the way how peer info was built and accidentally move those into its own method without calling the method. Signed-off-by: Arne Schwabe --- openvpn/client/cliopt.hpp | 37 +++++++++++++------------------------ 1 file changed, 13 insertions(+), 24 deletions(-) diff --git a/openvpn/client/cliopt.hpp b/openvpn/client/cliopt.hpp index 7f0226cd3..6a8c9845d 100644 --- a/openvpn/client/cliopt.hpp +++ b/openvpn/client/cliopt.hpp @@ -1007,6 +1007,19 @@ class ClientOptions : public RC if (pcc.pushPeerInfo()) { + /* If we override the HWADDR, we add it at this time statically. If we need to + * dynamically discover it from the transport it will be added in + * \c build_connect_time_peer_info_string instead */ + if (!config.hw_addr_override.empty()) + { + pi->emplace_back("IV_HWADDR", config.hw_addr_override); + } + + pi->emplace_back("IV_SSL", get_ssl_library_version()); + + if (!config.platform_version.empty()) + pi->emplace_back("IV_PLAT_VER", config.platform_version); + /* ensure that we use only one variable with the same name */ std::unordered_map extra_values; @@ -1044,30 +1057,6 @@ class ClientOptions : public RC return pi; } - PeerInfo::Set::Ptr build_peer_info_transport(const Config &config, const ParseClientConfig &pcc) - { - PeerInfo::Set::Ptr pi(new PeerInfo::Set); - - // MAC address - if (pcc.pushPeerInfo()) - { - /* If we override the HWADDR, we add it at this time statically. If we need to - * dynamically discover it from the transport it will be added in - * \c build_connect_time_peer_info_string instead */ - if (!config.hw_addr_override.empty()) - { - pi->emplace_back("IV_HWADDR", config.hw_addr_override); - } - - pi->emplace_back("IV_SSL", get_ssl_library_version()); - - if (!config.platform_version.empty()) - pi->emplace_back("IV_PLAT_VER", config.platform_version); - } - return pi; - } - - void next(RemoteList::Advance type) { bool omit_next = false; From 9d542454ea9ba7d5e1fe868b39cb30e69bbc8781 Mon Sep 17 00:00:00 2001 From: David Sommerseth Date: Fri, 1 Mar 2024 13:28:00 +0100 Subject: [PATCH 29/31] dco: Fix incorrect #ifdef vs #if usage for ENABLE_KOVPN The code in dco/dcocli.hpp used #if ENABLE_KOVPN, which will fail on newer compilers if the macro is defined in a source file. Compilers may not complain if the macro is defined on the command line, via -D. This type of checks should use either #ifdef or #if defined(...). The #if conditional expects a boolean expression. Since these code blocks also depended on #elif (also expects a boolean expression , the defined(...) approach was chosen throughout this file. Signed-off-by: David Sommerseth --- openvpn/dco/dcocli.hpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/openvpn/dco/dcocli.hpp b/openvpn/dco/dcocli.hpp index bf3e55887..abb1c8ba0 100644 --- a/openvpn/dco/dcocli.hpp +++ b/openvpn/dco/dcocli.hpp @@ -36,22 +36,22 @@ #include #include -#ifndef ENABLE_OVPNDCOWIN +#if !defined(ENABLE_OVPNDCOWIN) #include #endif -#ifdef ENABLE_KOVPN +#if defined(ENABLE_KOVPN) #include #include #include #include -#elif ENABLE_OVPNDCO +#elif defined(ENABLE_OVPNDCO) #include #include #include #include #include -#elif ENABLE_OVPNDCOWIN +#elif defined(ENABLE_OVPNDCOWIN) #include #include #include @@ -92,7 +92,7 @@ class ClientConfig : public DCO, virtual void finalize(const bool disconnected) override { -#ifdef ENABLE_OVPNDCOWIN +#if defined(ENABLE_OVPNDCOWIN) if (disconnected) tun.tun_persist.reset(); #endif @@ -286,7 +286,7 @@ class Client : public TransportClient, uint32_t peer_id; }; -#ifdef ENABLE_KOVPN +#if defined(ENABLE_KOVPN) #include inline DCO::Ptr new_controller(TunBuilderBase *) { @@ -298,7 +298,7 @@ ClientConfig::new_transport_client_obj(openvpn_io::io_context &io_context, { return TransportClient::Ptr(new KovpnClient(io_context, this, parent)); } -#elif ENABLE_OVPNDCO +#elif defined(ENABLE_OVPNDCO) #include inline DCO::Ptr new_controller(TunBuilderBase *tb) { @@ -317,7 +317,7 @@ ClientConfig::new_transport_client_obj(openvpn_io::io_context &io_context, { return TransportClient::Ptr(new OvpnDcoClient(io_context, this, parent)); } -#elif ENABLE_OVPNDCOWIN +#elif defined(ENABLE_OVPNDCOWIN) #include inline DCO::Ptr new_controller(TunBuilderBase *tb) { From b47ef50484e20ce27239ee04c33cf3642a291fee Mon Sep 17 00:00:00 2001 From: David Sommerseth Date: Fri, 1 Mar 2024 13:43:35 +0100 Subject: [PATCH 30/31] test/ovnpcli: Refactor to use GDBus++ D-Bus implementation The openvpn3-linux ships with a netcfg-cli client, which is essentially the same code as test/ovpncli/cli.cpp but it uses the net.openvpn.v3.netcfg D-Bus service to create the virtual network adapter and the related network and DNS configuration. This is a useful test client when only wanting to test the Network Configuration service openvpn3-linux ships with. As part of the refactoring of the D-Bus implementation in openvpn3-linux, the supporting D-Bus setup needs to be adjusted to the new D-Bus API. It has been considered to support both types of APIs, but the legacy D-Bus API is deprecated and will not be used any more after the release of OpenVPN 3 Linux v22_dev. Prior releases will depend on an older OpenVPN 3 Core library version, which contains the old API. Signed-off-by: David Sommerseth --- test/ovpncli/cli.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/ovpncli/cli.cpp b/test/ovpncli/cli.cpp index 9139feea4..2012a85fa 100644 --- a/test/ovpncli/cli.cpp +++ b/test/ovpncli/cli.cpp @@ -1321,9 +1321,8 @@ int openvpn_client(int argc, char *argv[], const std::string *profile_content) else { #if defined(USE_NETCFG) - DBus conn(G_BUS_TYPE_SYSTEM); - conn.Connect(); - NetCfgTunBuilder client(conn.GetConnection()); + auto dbus_system = DBus::Connection::Create(DBus::BusType::SYSTEM); + NetCfgTunBuilder client(dbus_system); #else Client client; #endif From 03236ed7bb59b4fa5678241f6119e0e13a088ebc Mon Sep 17 00:00:00 2001 From: David Sommerseth Date: Mon, 22 Apr 2024 13:18:11 +0200 Subject: [PATCH 31/31] Release: OpenVPN 3 Core Library, version 3.8.5 Signed-off-by: David Sommerseth --- openvpn/common/version.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openvpn/common/version.hpp b/openvpn/common/version.hpp index e31021da2..911fb83ff 100644 --- a/openvpn/common/version.hpp +++ b/openvpn/common/version.hpp @@ -24,5 +24,5 @@ #pragma once #ifndef OPENVPN_VERSION -#define OPENVPN_VERSION "3.8.4" +#define OPENVPN_VERSION "3.8.5" #endif