Update OpenVPN3 to least version #3
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: David Sommerseth <davids@openvpn.net>
Earlier implementations just assumed that --client mode is always present in the config, which lead to config behaving different in OpenVPN 2.x and 3.x. This creates hard to debug corner cases. Additionally OpenVPN 3.x was not parsing the tls-client and pull options. This lead to OpenVPN 3.x erroring on a perfectly legal config with --pull in it. Note the original patch was by Merten Fermont <merten.fermont@gmail.com> but his patch got mangled in the email and when I started to apply it manually I instead wrote my own version of it since we need unit tests anyway.
redirect-gw is implemented by changing default route to a GW provided by VPN. For IPv4 before doing that we add a bypass route to a remote. This is needed only when remote is not on local network. The check "is remote on local network" has a wrong assumtion that remote is IPv4. This is obviously not always the case since remote could be IPv6. In this case if we want to redirect IPv4 traffic an exception is thrown inside BestGateway class while trying to convert IPv6 address to IPv4. Fix by specifying correct address family based on remote's "ipv6" flag. Later we add bypass route only if remote is IPv4. Fixes OVPN3-1004. Signed-off-by: Lev Stipakov <lev@openvpn.net>
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
This fixes deprecation warnings with latest CMake.
("Compatibility with CMake < 3.5 will be removed
from a future version of CMake.")
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Also run tests in GHA. Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Other Travis CI related files were already deleted in commit 4666c7f. Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Adapt to vcpkg changes. Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Since we use C++17 now, the attribute is guaranteed to be available. Should silence some Coverity warnings. Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
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 <frank@lichtenheld.com>
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 <frank@lichtenheld.com>
Reduce default set from /W3 to /W2 and disable two additional warnings that we do not care about. With these settings successful builds with /WX are possible. Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
This commit mostly reverts a previous commit: commit b7bc687 Avoid compiler warning with gcc by not using move semantics The previous commit changed the semantics of the client callback to use copy instead of move semantics on the filename string to placate a compiler warning which was later determined to be a false positive. We revert to calling the client-provided func() with move semantics on the filename parameter. We also retain the use of std::invoke to call the client-provided callback. Signed-off-by: James Yonan <james@openvpn.net>
The undefined behavior is unary negation of T:min of a signed type attempting to get a positive value of the same signed type. This commit adds a unit test that exposes the original bug and well as a fix for it. Signed-off-by: Mark Deric <jmark@openvpn.net>
The confusing overlapping structs and memory accesses with the struct lead to use missing a few bytes from being copied. Fix this by copying from the correct struct. Signed-off-by: Arne Schwabe <arne@openvpn.net>
- Used static_cast instead of direct type conversions in places where it's safe - Used numeric_cast where failure is possible - Changed types of arguments and locals when practical Signed-off-by: Charlie Vigue <charlie.vigue@openvpn.com>
We're specifically interested in the fix for the unit tests.
("Update test data to avoid failures of unit tests after
2023-08-07")
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Making it more explicit that the listed items is data being sent to the server. Signed-off-by: David Sommerseth <davids@openvpn.net>
Signed-off-by: Charlie Vigue <charlie.vigue@openvpn.com>
- Went back to a simple numeric_cast since the better way had issues on other paltforms Signed-off-by: Charlie Vigue <charlie.vigue@openvpn.com>
Signed-off-by: Charlie Vigue <charlie.vigue@openvpn.com>
Signed-off-by: Charlie Vigue <charlie.vigue@openvpn.com>
Using is_safe_conversion in places where it is a better fit than numeric_cast. Signed-off-by: Charlie Vigue <charlie.vigue@openvpn.com>
control_recv looks at the incoming messages and dispatches it to different handlers depending on the message. Split of these handlers into their own methods. Also rename/align process_halt_restart to recv_halt_rest to make all handlers consistent. Signed-off-by: Arne Schwabe <arne@openvpn.net>
Signed-off-by: Arne Schwabe <arne@openvpn.net>
The reauthentication logic differs from openvpn2 and the code is a bit hard to follow. Simplify the code and make it behave like in openvpn2. - password is cached by default - password is purged when auth-nocache is presented in a local config or pushed - when AUTH_FAILED is received and we have no session-id, throw a fatal error - when AUTH_FAILED is received and user interaction is required for authentication (MFA), throw a fatal error - when AUTH_FAILED is received, user interaction is not required for authentication and either we have a cached password OR password is not needed, we reconnect. Password is "needed" when non-empty password is provided. User interaction is required for static/dynamic challenge and SAML. Signed-off-by: Lev Stipakov <lev@openvpn.net>
OpenVPN uses a idiosyncrasy that all ciphers are uppercase but none is spelt lowercase and excepts this idiosyncrasy also in IV_CIPHERS Signed-off-by: Arne Schwabe <arne@openvpn.net>
Instead of using nullptr for uninitialized RegKey, use the value defined in WIN32 API for that. We need to check for it anyways, so unifying this makes the checks more straight forward. Signed-off-by: Heiko Hund <heiko@openvpn.net>
MinGW's g++ displays this warning when compiling: warning: the address of ‘IP_ADDRESS_STRING::String’ will never be NULL [-Waddress] since String is defined as a C array, it can never be nullptr, so the warning is correct and the check can be removed. Signed-off-by: Heiko Hund <heiko@openvpn.net>
Since C++17 the codecvt header is deprecated and scheduled for removal in C++26. MSVC warns about use of the deprecated API already, other will follow soon. It was decided to deprecate and remove it since it does not support the current UNICODE standard anymore. Also test for the _WIN32 define, instead of WIN32, so that this keeps working with MinGW headers, when cross-compiling. Signed-off-by: Heiko Hund <heiko@openvpn.net>
Signed-off-by: Heiko Hund <heiko@openvpn.net>
Functions from_utf8() and to_utf8() added one extra '\0' glyph to the output string, i.e. if the input basic_string::size() was 8 the output was 9. Normally this would not make a difference since for most string usage, as the extra NUL at the end would mostly be ignored. However if you used the output string to append to another string the extra NULs were actually also appended, resulting in a string with embedded NUL characters. Which is a problem with the next use case. The pack_string_vector() function failed to produce a wide MULTI_SZ string from a vector of strings, unlike advertised. The extra NUL actually led to the MULTI_SZ string always being terminated after the first string. Besides that, the function actually never terminated the MULTI_SZ in the first place and also failed to handle empty vectors gracefully. Signed-off-by: Heiko Hund <heiko@openvpn.net>
Create a struct Reg, which contains various setter and getter functions for different registry types and other operations that will be used. This is done so that these operations can be injected as a dependency and thus replaced with mock operation for the purpose of testing. Besides that it makes code more brief and less error prone, since there's now one implementation for converting C <-> C++ for each operation. Move existing class RegKey and class RegKeyEnumerator into struct Reg as well, so they are now known as Reg::Key and Reg::KeyEnumerator. Signed-off-by: Heiko Hund <heiko@openvpn.net>
Create a struct NetApi, which contains various network related functions that will be used. This is done so that these operations can be injected as a dependency and thus replaced with mock operation for the purpose of testing. There are also functions which operate solely on the Registry, those are left out of the NetApi since they can already be abstracted by struct Reg. Signed-off-by: Heiko Hund <heiko@openvpn.net>
Previous to this --dns and DNS related --dhcp-options shared the same code to apply the settings to Windows and macOS systems. So, both options were pretty much just aliases, with --dns offering more and finer grained settings that were mostly ignored. Now --dhcp-options are applied the way they have always been and --dns does it its own - the new - way. Reason for this behavioral change is foremost that we want it to be the same between openvpn version 2 and version 3. But there are also a few new features (e.g. DNSSEC), previously not present with the --dhcp-options. The name server and split-domain configuration is exclusively set via NRPT on Windows, since it overrules any other resolver setting. If there is no split DNS configured and all domains are resolved using the pushed name server, we make sure that local domain names are still resolvable by adding so called exclude NRPT rules, that make sure local domains get resolved by their local DNS resolvers. Since Windows does not know about alternative secure transports, the 'transport' and 'sni' settings are ignored. For macOS the 'dnssec' setting is ignored in addition to that. Besides that not much does change on that platform. In case of --dns options the explicit values are used now. The API in use may be changed at a later time. Signed-off-by: Heiko Hund <heiko@openvpn.net>
Incompatible changes to the --dns server address and --dns server exclude-domains options were introduced after the code for handling them was released. Add and send a new IV_PROTO flag, so servers which act on the flags set can differentiate between clients which have implemented --dns and those which just support the new option. This enables them to decide which variant of options to send to the client. Signed-off-by: Heiko Hund <heiko@openvpn.net>
The option is only enforced with the --dns option, since DNS settings coming in via --dhcp-option have always voluntarily blocked port 53. This behavior is kept for backwards compatibility. Since the --dns option allows local name servers to continue to work, even thought no split DNS is pushed, supporting the option makes sense. If admins do not want any DNS queries outside the tunnel, this is the option to push alongside the --dns options. Signed-off-by: Heiko Hund <heiko@openvpn.net>
Signed-off-by: Heiko Hund <heiko@openvpn.net>
This flag was introduced to allow clients to decide if they want to ignore non-split DNS option pushed to them. So, to be compatible with the previous behavior with --dhcp-option, we act on the flag as wenn when there are no resolve-domains specified. Signed-off-by: Heiko Hund <heiko@openvpn.net>
…envpn3 into ios-proj-refactor
outspace
force-pushed
the
ios-proj-refactor
branch
from
5cc205c to
e901077
Compare
yaroslavyaroslav
force-pushed
the
ios-proj-refactor
branch
2 times, most recently
from
e2d2855 to
b827c23
Compare
yaroslavyaroslav
force-pushed
the
ios-proj-refactor
branch
2 times, most recently
from
47482d2 to
881837b
Compare
yaroslavyaroslav
force-pushed
the
ios-proj-refactor
branch
from
881837b to
49ea4ee
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.