0
0
mirror of https://github.com/OpenVPN/openvpn3.git synced 2024-09-20 12:12:15 +02:00
Commit Graph

2890 Commits

Author SHA1 Message Date
Arne Schwabe
e1037508e3
Add minimal FreeBSD support
Signed-off-by: Arne Schwabe <arne@openvpn.net>
2023-06-22 19:07:20 +02:00
Arne Schwabe
966515737d
Add missing includes in cli.cpp
The code for browser opening on Unix platforms assumes some includes to
be always present but nulltun + FreeBSD actually do not have include them.
So include them explicitly. And since this break windows, ensure that
process.hpp does nothing on that platform.

Signed-off-by: Arne Schwabe <arne@openvpn.net>
2023-06-22 19:07:19 +02:00
Antonio Quartulli
699bcb455a
numeric_cast.hpp: add missing include
Including cstdint is required by uintmax_t.

Fixes the following:

openvpn/common/numeric_cast.hpp:66:25: error: ‘uintmax_t’ does not name a type

Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
2023-06-08 17:30:40 +02:00
David Sommerseth
5f1f207bfd
Merging changes from releaseprep/3.8
Signed-off-by: David Sommerseth <davids@openvpn.net>
2023-05-22 14:51:27 +02:00
Mark Deric
55967d3f70
Reduce mssfix logging noise
The proto test output is overwhelmed with mssfix log messages.

Ideally, this PR should be accompanied by a mechanism to change the
debug level at runtime.

Signed-off-by: Mark Deric <jmark@openvpn.net>
2023-05-22 14:47:36 +02:00
Lev Stipakov
9fd9c6834f remotelist.hpp: do not add address family for cached remotes
We agreed that we display address family based on profile or config
override, not the result of resolve.

Signed-off-by: Lev Stipakov <lev@openvpn.net>
2023-05-15 10:47:06 +03:00
Lev Stipakov
6225707fab Fix compilation errors causes by release branch merge
Signed-off-by: Lev Stipakov <lev@openvpn.net>
2023-05-11 10:59:30 +03:00
David Sommerseth
0bcdeff84a
Merging changes from releaseprep/3.8 2023-05-10 19:57:23 +02:00
Frank Lichtenheld
ad21590268
ssl/proto: Fix unaligned access
Reported by UBSAN.

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
2023-05-10 19:43:26 +02:00
Lev Stipakov
de01e278cc
Display configured family address for UDP and DCO
Instead if displaying resolved family address (v4/v6),
display the one which is configured - either in ovpn profile
or config override options.

This is already the case for TCP.

Signed-off-by: Lev Stipakov <lev@openvpn.net>
2023-05-10 18:22:59 +02:00
Arne Schwabe
d6d4d547b5 Probably reference count the EPKI object and ensure right order
In order to ensure that the xkey provider certificate objects do not
use a dangling pointer, use a shared_ptr to manage the lifetime of
ExternalPKIImpl. Since we need to pass it into a raw void* pointer
we need this raw pointer to a shared_ptr magic here.

Also ensure the right order of epki and config on destruction.

Signed-off-by: Arne Schwabe <arne@openvpn.net>
2023-05-10 17:23:33 +02:00
Arne Schwabe
9e5de78347 Document behaviour of Signal class a bit more
Signed-off-by: Arne Schwabe <arne@openvpn.net>
2023-05-10 17:23:33 +02:00
Arne Schwabe
4823208513 Destroy library context after other related object have been destroyed
Theorectically this should not matter as this is internally reference
counted in OpenSSL but it is better to be safe than sorry.

Signed-off-by: Arne Schwabe <arne@openvpn.net>
2023-05-10 17:23:33 +02:00
Arne Schwabe
d2d8a60740 Use unique_ptr instead of raw pointers in xkey class
Signed-off-by: Arne Schwabe <arne@openvpn.net>
2023-05-10 17:23:33 +02:00
Arne Schwabe
f5bcc99b30 Convert OpenSSL SSL context class to use unique_ptr
Use a unique_ptr for the SSL_CTX instead of relying on managing it
ourselves.

Signed-off-by: Arne Schwabe <arne@openvpn.net>
2023-05-10 17:23:33 +02:00
Arne Schwabe
50334952bc Convert OpenSSL X509 class to use unique_ptr instead of a raw pointer
Signed-off-by: Arne Schwabe <arne@openvpn.net>
2023-05-10 17:23:33 +02:00
Lev Stipakov
84cf8f45cd dco: check for options/config DCO compatibility
When parsing config, check DCO compatibility. Following
options break DCO compatibility:

 - http-proxy
 - compress
 - comp-lzo

Same for config settings:

 - non-preferred-algorithms
 - legacy-algorithms
 - proxyHost

DCO compatibility could be checked with

 - bool EvalConfig::dcoCompatible
 - std::string dcoIncompatibilityReason

If client nevertheless tries to connect, an exception
will be thrown:

  connect error: option_error: dco_compatibility: config/options are not
compatible with dco

Fixes OVPN3-960.

Signed-off-by: Lev Stipakov <lev@openvpn.net>
2023-05-08 13:51:34 +03:00
James Yonan
cdc62e9a9c WS::Client::AltRoutingShimFactory: make local_ip() plural and remove error_expire()
* local_ip() has been renamed to local_addrs() and now
  returns a vector of addresses.  This is because the
  user may want to specify local addresses for both
  IPv4 and IPv6.

* error_expire() has been removed because it currently
  has no users.

Signed-off-by: James Yonan <james@openvpn.net>
2023-05-05 15:06:01 -06:00
James Yonan
493a5d8d15 RCCopyable: added move methods for construction/assignment
Copy/move construction/assignment are no-ops because
we always want to default-construct the refcount from
scratch.  We never want to actually copy or move
the refcount because that would break any
referencing smart pointers.

Signed-off-by: James Yonan <james@openvpn.net>
2023-05-01 11:48:16 -06:00
James Yonan
f3a861d54c jsonhelper: provide rvalue reference overloads for all methods that return a reference to a passed object
Signed-off-by: James Yonan <james@openvpn.net>
2023-05-01 11:48:09 -06:00
James Yonan
993875b6f8 WS::Client: added websocket_timeout parameter
Previously, when WS::Client entered websocket streaming
mode, it would cancel the general_timeout.

Now when websocket_timeout is nonzero, general_timeout will
be reset to the websocket_timeout value when the websocket
begins streaming.  This allows websocket use cases
to retain a general_timeout even during streaming.

By default, websocket_timeout is set to zero, which will
retain the pre-existing behavior of canceling the
general_timeout when streaming begins.

Signed-off-by: James Yonan <james@openvpn.net>
2023-04-28 09:34:01 -06:00
James Yonan
8654108234 WS::Server: added a comment about ksm.is_internal() usage
Signed-off-by: James Yonan <james@openvpn.net>
2023-04-28 00:00:02 -06:00
James Yonan
55dd04c6a6 WS::Client: in AltRoutingShimFactory, added virtual method local_ip()
When WS::Client is operating with a AltRoutingShimFactory
object, bind to the local address provided by the local_ip()
method if defined.

Signed-off-by: James Yonan <james@openvpn.net>
2023-04-28 00:00:02 -06:00
James Yonan
d7e5bd5397 WS::Client: in AltRoutingShimFactory, added virtual method alt_routing_debug_level()
Previously the debug message "ALT_ROUTING HTTP CONNECT to ..."
was conditional on debug_level in WS::Client::Config being
>= 2.

Instead, we now consult alt_routing_debug_level()
defined by AltRoutingShimFactory for the debug level.

This makes it more straightforward to debug
AltRoutingShimFactory because the relevant debug levels
have been consolidated within the class being debugged.

Signed-off-by: James Yonan <james@openvpn.net>
2023-04-28 00:00:02 -06:00
Arne Schwabe
0d0e3809c1
Remove OPENVPN_EXTERN
with the new C++17 capability of inline for variables, we can avoid
having to ifdef tricks to only include the variables into one compilation
unit. Also remove the extern.hpp that serves no purpose now anymore.

Signed-off-by: Arne Schwabe <arne@openvpn.net>
2023-04-27 23:35:41 +02:00
Frank Lichtenheld
b8ae379dd5 Make all C++ source code files have LF (Unix) line endings
For consistency. Some of the Windows-specific files, but not
all of them, had CRLF file endings.

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
2023-04-24 17:16:58 +02:00
Frank Lichtenheld
823a5d44f6 time.hpp: some cleanups
- Use C++-17 static inline to get rid of explicit
  definitions for static class variables.
- Convert file comment to a proper doxygen comment
- Change Windows code to use ULONGLONG now that we
  do not support GetTickerCount() anymore.

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
2023-04-24 16:02:33 +02:00
Frank Lichtenheld
5a84a32f9f Add some preprocessor checks for Windows ARM64
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
2023-04-24 14:37:44 +02:00
Lev Stipakov
f544e04df7
Bring back "allow local DNS resolvers" functionality
This was introduced in commit

  613aa6bf ("Win: support for local DNS resolvers")

but got removed by mistake in commit

  fd065596 ("Merge release of OpenVPN Core library 3.6.4 to master")

Besides, this never worked for DCO, so fix that too.

Fixes OVPN3-964.

Signed-off-by: Lev Stipakov <lev@openvpn.net>
2023-04-24 13:07:46 +02:00
Lev Stipakov
41e96f96a6
Add IPv6 support to "get best gateway" Windows logic
The current implementation of "get best gateway"
is completely unaware of IPv6. Because of that
agent-enabled client is not able to connect to IPv6
server. This happens because the first call to agent
(add-bypass-route) fails, since we pass IPv6 address,
which agent tries to intrepret as IPv4 and fails.

Moreover, "add bypass route" logic looks for the best gateway
for the given remote, and the API we use (GetIpForwardTable and
GetBestGateway) doesn't work with IPv6.

This adds IPv6 support to BestGateway class and "add bypass route"
logic. For that we use IPv6-aware API such as GetIpForwardTable2
and own IP::Addr/4/6 absactions.

Fixes OVPN3-959.

Signed-off-by: Lev Stipakov <lev@openvpn.net>
2023-04-24 13:07:36 +02:00
Frank Lichtenheld
cb589b70f0 Remove support for pre-Vista Windows versions
We do not care about them anymore. So remove all
the support which is untested anyway.

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
2023-04-20 17:44:14 +02:00
Lev Stipakov
09be60d38d cli.cpp: implement get_password() on Windows
Password is not echoed and submitted when Enter is pressed.
This requires not removing ENABLE_PROCESSED_INPUT and ENABLE_LINE_INPUT
flags.

Signed-off-by: Lev Stipakov <lev@openvpn.net>
2023-04-19 18:47:48 +03:00
Lev Stipakov
7eba902d1c reg.hpp: support for INVALID_HANDLE_VALUE
We store an output of SetupDiOpenDevRegKey() in Win::RegKey. However,
this API returns INVALID_HANDLE_VALUE on error. In this case we should
not attempt to call RegCloseKey() on this handle, which we do in destructor
of Win::RegKey if handle is defined.

Signed-off-by: Lev Stipakov <lev@openvpn.net>
2023-04-19 18:47:39 +03:00
Lev Stipakov
9233d19209 Make session token problems non-fatal again
Commit 03917f ("add support for temporary authentication failures")
unintentionally changed client's behavior in a way that all auth
failures which do not start with "TEMP" (such as expired session token)
have become fatal - previously those were non-fatal and client reconnected.

Fix by adding omitted "else" statement. While on it, remove
duplicated comment.

Signed-off-by: Lev Stipakov <lev@openvpn.net>
2023-04-19 18:47:30 +03:00
David Sommerseth
037fda0521
Merge changes from releaseprep/3.8
Signed-off-by: David Sommerseth <davids@openvpn.net>
2023-04-14 13:44:00 +02:00
Lev Stipakov
93c961e169 dco-win: add missing ioctl code string for DEL_PEER
If ioctl call fails, we print failed code's string
representation, such as "OVPN_IOCTL_DEL_PEER".

Signed-off-by: Lev Stipakov <lev@openvpn.net>
2023-04-12 11:42:49 +03:00
David Sommerseth
a8729663dd
Merge changes from releaseprep/3.8
Signed-off-by: David Sommerseth <davids@openvpn.net>
2023-03-30 16:02:35 +02:00
Arne Schwabe
2d81118331
Cherry pick "Fix crash in xkey-provider in msvc builds" to xkey
Orginal commit message in OpenVPN 2.x (commit b2e4d0ad5c3):

Author: Selva Nair <selva.nair@gmail.com>
Date:   Wed Jul 6 23:51:51 2022 -0400

    Fix crash in xkey-provider in msvc builds

    The function signature for xkey_load_generic_key had
    function pointers defined as function types that seems
    to work in gcc but not in msvc.

    Fix it by changing the function signatures to what was intended.

    Also revert part of commit 627d1a3d28638... as that workaround
    should be no longer required.

    Reported by: Lev Stipakov https://github.com/lstipakov
2023-03-29 23:16:36 +02:00
Arne Schwabe
d5c09e2b08
Do not allow SHA1 cipher suites when using preferred tls-cert-profile
We do not allow SHA1 in other instances using this profile and while
SHA1 is still fine as HMAC in these situation, people freak out when
seeing SHA1 and also the description and documentation will state
that SHA1 is not allowed in other context (certificate signature),
causing confusion. So better not allow it in this context as well.

Signed-off-by: Arne Schwabe <arne@openvpn.net>
2023-03-29 23:14:30 +02:00
Arne Schwabe
0d337f900c
[OSSL] Treat the error of missing a common signature algorithm as fatal
When trying to connect to a very old OpenVPN server (TLS 1.0) that
supports only outdated signature algorithm but at the same time
requiring a tls-cert-profile of legacy or higher, you can run into
the issue of not allowing the outdated signature algorithm of the
server.

OpenSSL 3.0.8 has added a specific error code for this situation that
we treat as fatal error, similar to the way we treat no common cipher
or no common TLS version.

Signed-off-by: Arne Schwabe <arne@openvpn.net>
2023-03-29 23:14:29 +02:00
Arne Schwabe
48f5adce94
Ensure that tlsVersionMinOverride does not lower TLS version
This ensure that client that want to set a miminum level of TLS
version do not accidentially lower the version when the profile already
requires a higher version.

Also make the tls version enum an enum class for better type safety.

Signed-off-by: Arne Schwabe <arne@openvpn.net>
2023-03-29 23:14:15 +02:00
Lev Stipakov
71b3391dee dco-win: add support for peer stats
Make DCOTransportSource aware of tun stats.

Implemenent DCOTransportSource interface. Withing
stats delta callback, fetch peer stats and return delta
between last and current stats (same as in DCO Linux).

Fixes OVPN3-947.

Signed-off-by: Lev Stipakov <lev@openvpn.net>
2023-03-29 14:08:33 +03:00
Lev Stipakov
fe9df4f431 dco-win: use OS-assigned random local port
The same behavior is implemented in openvpn2
and openvpn3 non-dco cases.

This also fixes some reconnect issues to
openvpn2 server.

Fixes OVPN3-949.

Signed-off-by: Lev Stipakov <lev@openvpn.net>
2023-03-29 11:03:38 +03:00
Lev Stipakov
3e61e624d3 dco-win: add missing socket_protect call
This call is required to add bypass route, which
is required when client reconnects with seamless tunnel.

Fixes OVPN3-948

Signed-off-by: Lev Stipakov <lev@openvpn.net>
2023-03-27 14:27:04 +03:00
David Sommerseth
bc3b549ed6
Merge changes from releaseprep/3.8
Signed-off-by: David Sommerseth <davids@openvpn.net>
2023-03-08 17:24:24 +01:00
Arne Schwabe
171fd2f0af
Fix mbed TLS AEAD encrypt/decrypt with newer mbed TLS 2.x versions
Newer mbed TLS version changed the API. This fixes our usage of the API and
also removed the micro optimisation of reusing the buffer for plain and cipher
text.

It also adds a unit test to ensure the data is correctly encrypted/decrypted.

Signed-off-by: Arne Schwabe <arne@openvpn.net>
2023-03-08 17:06:24 +01:00
Arne Schwabe
8b13cdd7a1 Allow duplicate options without error in configuration files
we often have configuration files where a directive is duplicated and
the later one wins. This is quite common and should not rais an error. We
still warn about these as this might an error/oversight.

Signed-off-by: Arne Schwabe <arne@openvpn.net>
2023-03-08 16:52:51 +01:00
Charlie Vigue
f38e97e1c3 Eliminate some conversion warnings
- [ipv4.hpp, ipv6.hpp] In both v4 and v6 headers it is safe to cast the hex
so as to eliminate the spurious warnings.

- [lz4.hpp] Apply value clamp to the hint that is sent to the compressor
to prevent a potential conversion overflow.

- [zlib.hpp] In compress_gzip, zs.s.avail_in and zs.s.avail_out are
theoretically susceptable to overflow. To prevent this we use
numeric_cast. In decompress_gzip we do a similar thing for zs.s.avail_in
but only value clamp avail_out, since the read loop looks like it will
compensate

- [buffmt.hpp] It's safe to cast the result of the arithmentically caused
promotion back down to char.

- [base64.hpp] In Base64 CTOR, changed type of a couple variables to
match the type of the table they generate. In decode, perform a static
cast to the type of the template elements the function is
instantiated for.

- [core.hpp] Perform static cast long --> int on value representing
number of cores. If we run on systems where there are more cores than
int can represent this will behave oddly, but this circumstance
seems unlikely at the present time.

- [environ.hpp] The casts seem to be safe but I have added a todo ticket
to evaluate this change further.

- [hexstr.hpp] In render_hex_char there were two conversion warnings
and a bug involving out of range input. Those are addressed.
In dump_hex the result of some math and logic is now clamped
to the range of acceptable input values for string::spaces
In parse_hex the result of converting from a hex string to an
integral value is cast to the template value_type

- [hostport.hpp] The static_cast should be safe because the value
produced by validate_port is range checked.

- [split.hpp] Applied numeric cast to ensure output of lex.get stays
within acceptable type limit.

- [stop.hpp] In Stop::Scope It's extremely unlikely but was possible for
the vector size to exceed the limit of int. The size now has a much lower
limit applied and will throw if it is exceeded.

- [string.hpp] Changed the call to toupper/tolower so they call the
locale function template instead of the cctype C function. This
eliminates the warning and the need for the cast.

- [cliproto.hpp] The computation of mss_fix is stored in a size_t and
then assigned to an unsigned short. We clamp this assignment
to the range of unsigned short.

- [tempfile.hpp] In TempFile CTOR suffixLen is computed as one type
and consumed as another. Since the CTOR is already throwing
for a couple other error conditions, I have added a
numeric_cast to the conversion that also throws in case of a
value overflow.

- [unicode.hpp] In an 8 --> 16 bit string conversion we mask and assign
in a way the compiler can't be certain is safe even though it is safe.
Added static cast to let the compiler know it's safe. In the second case
the class uses unsigned int to store a size, and then uses it in with size_t
which generates conversion warnings. I have changed the type of size
to size_t

- [logperiod.hpp] in log_period changed return type specification to
match the actual return type.

- [usergroup_retain_cap.hpp] In the unlikely event the caps size (size_t)
exceeds the range cap_set_flag can accept, an exception will be thrown.

- [crypto_aead.hpp] StaticKey::size provides a size_t where unsigned int
is required. We use numeric_cast to check the size() value in the
extremely unlikely event it is manipulated to exceed the allowed value.

- [packet_id.hpp] Code packs a time_t into a uint32_t for replay packet
ID protection purposes. The warning is supressed by a mask and cast
since the 32 bit limit is baked into the protocol and the overflow itself
does not cause a severe breakage.

- [headredact.hpp] Altered code such that the type that stores the find
result is compatible with the result from find. Additionally used the
npos constant instead of -1. There is a commented out code block that
claims to be dropped due to requiring C++ '14 - consider just using
that.

- [csum.hpp] in csum fold and cfold one has a mask and cast, the
second is just a cast to undo a promotion. Both appear safe.

- [ipv4.hpp] Values are masked and shifted so the cast should be safe.
Added cast.

- [ping4.hpp] ICMP ID and sequence number function arguments are
changed to the same type as needed by the structure. For
IPv4 header version_len 2nd arg is int but sizeof is not, so we
cast it. IPv4 tot_len is a uint16_t so we clamp to that value
range and compute it once.

- [ping6.hpp] Enforces a value constraint on the len argument to
csum_icmp, then checks the result of some math to ensure
the result will fit in the type it has to fit. In generate_echo_request
the ICMP ID and sequence args are changed to match the
type they are assigned to in the struct, and added
numeric_cast to range check payload_len.

 - [remotelist.hpp] In get_endpoint, endpoint.port is called with an
unsigned int where the function is expecting an unsigned short int.
Since parse_number_throw is a function template, we just ask it to
return the correct type now.

- [compress.hpp] In v2_push we accept an int value that is assigned to
an unsigned char we push to the buffer. I changed the function to
accept an unsigned char directly.

Added unit tests - thanks Mark Deric.

Signed-off-by: Charlie Vigue <charlie.vigue@openvpn.net>
2023-03-08 15:21:50 +00:00
Arne Schwabe
781931139b
Ensure NTLM phase 2 is always large enough
Reported-By: Trail of Bits (TOB-OVPN3-14)
Signed-off-by: Arne Schwabe <arne@openvpn.net>
(cherry picked from commit 156ea554e2)
2023-03-03 21:41:22 +01:00
Arne Schwabe
084e6a7e1d
Ensure members of openvpn::TunLinuxSetup::Config are initialised
Reported-By: Trail of Bits (TOB-OVPN3-13)
Signed-off-by: Arne Schwabe <arne@openvpn.net>
(cherry picked from commit da41e7cfdf)
2023-03-03 21:41:21 +01:00