The host route to the VPN server disappeared when a mac client, using
the agent, was reconnecting. That was causing --redirect-gateway tunnels
to break because no traffic could be sent anymore. Cause for this was
some internal state in the agent not being reset when the utun device
is temporarily removed during the restart.
Signed-off-by: Heiko Hund <heiko@openvpn.net>
This is reusing the auth pending method as this is just another method where
the final authentication decision is pending on some results. In this case
custom messages going back and forth. This can be tested using
client-pending-auth 5 1 "ACC:1000 A:6 cck1:certcheck:cxa1:fortune" 60'
with OpenVPN 2.x servers easily.
Also correctly use ClientEvent::Base::Ptr with the Clientevents to avoid
problems with not correctly using RCPtr
Signed-off-by: Arne Schwabe <arne@openvpn.net>
Commit 0c5e356 ("Simplify client options classes") has
indeed simplified options handling by consolidating
options into the base class. However "disble_client_cert"
option was copied, not moved, from its original location.
As a consequence, it become broken.
Fix by removing unneeded copy of this option and use the
one from correct location in options base class.
Signed-off-by: Lev Stipakov <lev@openvpn.net>
Instead of silently ignoring errors in control channel message and removing
invalid characters, we should be more strict and reject these message.
A similar change has been also submitted to OpenVPN 2.x
Clients need access to the server VPN ca for whitelisting reasons
so it is now available inside the EvalConfig structure. Implemented
the change and added a unit test for same.
Signed-off-by: Charlie Vigue <charlie.vigue@openvpn.net>
The syslog.h UNIX header already #defines LOG_{DEBUG, INFO} as
log-level constants, which means that we can't have code that
includes both openvpn/log/logger.hpp and syslog.h.
This patch renames all the LOG_<LEVEL>() macros to
OVPN_LOG_<LEVEL>(), to hopefully eliminate "macro already defined"
conflicts in the future.
Signed-off-by: Razvan Cojocaru <razvan.cojocaru@openvpn.com>
This allow debugging the certificates the server sends. It did this as hack
two times before. I think it is now time to get this officially in.
Signed-off-by: Arne Schwabe <arne@openvpn.net>
Compilers will otherwise complain about unused function when this header
is included in a compilation unit that only uses a subset of the functions
(e.g. in a unit test).
Signed-off-by: Arne Schwabe <arne@openvpn.net>
Before this commit traffic to loopback was limited when only DNS
(port 53) was blocked, due to the "not loopback" match condition being
replaced instead of the match condition being made more specific.
This broke the client option to override access to DNS servers listening
on loopback.
To fix this three things are done:
1) do not add DNS block rules if the override option is active.
2) explicitly block port 53 on loopback, except when the override
option is active.
3) remove the implicit block of port 53 on loopback and instead let
the firewall rule for non-loopback devices only.
Signed-off-by: Heiko Hund <heiko@openvpn.net>
This also moves some of the reasonsibility from ClientProto to
OpenVPNClient. This plays better with the EPKI implementation and also
does not break the idea of the current certcheck implementation as we now
just give the certcheck in client protocol a preconfigured SSL Config instead
of all the certificates individually.
The implementation would previously return the alias the client library
requested to use but would not allow to have multiple different
external aliases be correctly used. This adds supports to have the
correct alias being used as part of the signature callback.
Instead of relying on passing an empty domain name into the NRPT class
for the '.' rule not to be created, skip calling the NRPT code
altogether. Since there's no rule generated in the case where local
resolvers should be used when no split DNS is to be configured, skipping
the NRPT call is more readable and less magic, when viewed from the
setup class. Also more effective during runtime.
Signed-off-by: Heiko Hund <heiko@openvpn.net>
This is meant to help with the following scenario: main thread
creates secondary threads, each secondary thread asks the context
store for its own context, then goes into a "forever" scheduling
loop. Main thread waits for the secondary threads to finish, and
owns the context store. It would be nice to be able to have yet
another thread (say, a SIGINT handler) be able to call .stop()
on all the contexts managed by the context store, thus being able
to end the loops in the secondary threads and allow the main
thread to exit.
Signed-off-by: Razvan Cojocaru <razvan.cojocaru@openvpn.com>
Allow to set a program to get root (e.g. "sudo").
This allows to run the sitnl tests via ctest.
Only required on Linux, since those tests are
Linux-only.
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Previously this logic was encoded in Jenkinsfile by
running the UTs manually. Much preferable to just
use ctest.
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
With DCO, userspace has no visibility of packets arriving
on data channel. To provide "last packet received" time,
update it when stats are pulled from DCO and there is a
difference in incoming transport bytes.
Signed-off-by: Lev Stipakov <lev@openvpn.net>
The AppControl feature provides an internal communications channel that
is described in detail in the relevant commits that implement it. This
change adds two intrinsic app control 'protocols' to the list of
supported handlers in the client. The new protocols are:
- cxa1: This is a request for the client to begin a TLS handshake via
the app control channel.
- cck1: This is the protocol that allows the exchange of the requested
TLS handshake data.
The 'cxa1' handler parses the request and initiates the handshake from
the client. This handshake is exchanged via the 'cck1' protocol and
serves to prove to the server that the client has access to the required
private key.
Signed-off-by: Charlie Vigue <charlie.vigue@openvpn.com>
- Some actions were not pinned at all.
- Note full version number in comment so that renovate
provides better changelog.
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
According to the notes in the issues and changelog GCC
has fixed some false-positives of this but basically
accepted the rest and will not fix them.
They introduced gnu::no_dangling attribute to allow people
to override this warning on a class/function basis. But
that doesn't help us since we also need to support older
compilers.
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
pool.hpp:158:13: error:
template-id not allowed for destructor in C++20
[-Werror=template-id-cdtor]
158 | virtual ~PoolType<ADDR>() = default;
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
In openvpn2 WFP block filters are added when the 'block-local' flag is
pushed to clients together with --redirect-{gateway|private}. That is
done in addition to adding routes to harden defense against attacks
collectively known as Tunnelcrack on Windows systems.
Since the openvpn3 library did not deal with the block-local flag at all
before this commit, on Windows it is sufficient to simply block traffic
to local interfaces by placing firewall rules. Traffic will only be
allowed originating from the OpenVPN process, on the VPN interface, and
loopback.
Note that previously WFP rules were already added to prevent access to
local DNS servers, when DNS servers were pushed. These are contained
within the ones added with 'block-local' and need not be set
additionally in that case.
Signed-off-by: Heiko Hund <heiko@openvpn.net>
Some classes are moved to subclasses of class WFP. Other things just got
a more descriptive name. Here is what this commit changes effectively:
* class WFPContext -> class WFP::Context
* class WFP::WFPEngine -> class WFP::EngineHandle (private)
* class ActionWFP is split into ActionBase and two derived classes
ActionBlock and ActionUnblock, so that the purpose is more visible
to the uninitiated observer (instead of just a bool making the
difference)
* instead of the 'tap_' prefix to names, use 'itf_' now, since we're
not only dealing with tap interfaces anymore
* INVALID_HANDLE_VALUE is used instead of NULL to mark a WIN32 handle
as uninitialized
Signed-off-by: Heiko Hund <heiko@openvpn.net>
We currently accept cipher none as pushed cipher when legacy ciphers are
enabled but do not announce support for it in IV_CIPHERS. This means we
currently display inconsistent behaviour. Servers that ignore IV_CIPHERS
can use none but server that are correctly working cannot.
Setting ADAPTER_DOMAIN_SUFFIX for non-DHCP adapters requires
registry modification. For that, we need adapter GUID.
This passes adapter GUID from agent to client via /tun-open call
and then from client to agent via /tun-setup call, when adapter
domain suffix is set.
Github: #304
Signed-off-by: Lev Stipakov <lev@openvpn.net>
Signed-off-by: Krasovskiy Saveliy Igorevich <skrasovskiy@ozon.ru>
