In the code base three different syntaxes for overriding virtual member
functions could be found:
1) virtual ... override
2) virtual ...
3) ... override
This converts all of them to the third syntax, as recommended by the ISO
C++ core guidelines in C.128
Signed-off-by: Heiko Hund <heiko@openvpn.net>
Without this fix, the openvpn3-linux build is broken whenever a
dependency enables -Wnon-virtual-dtor (which protobuf 27.3
currently does on Arch Linux). The openvpn3-linux build treats
warnings as errors.
Jira: OVPN3-1242
Signed-off-by: Razvan Cojocaru <razvan.cojocaru@openvpn.com>
This adds support for parsing PUSH_UPDATE
control command, which enables to update
options "on the fly", without reconnect.
The options presented in the PUSH_UPDATE list
overwrite current options with the name. To unset
an option, it has to be prefixed with the "-".
For example:
PUSH_UPDATE,route 10.10.10.0 255.255.255.0,-dns
Replaces all existing routes with this new one
and removes all "dns" options.
If the client doesn't support updating certain option,
it reconnects. Except when option is prefixed with "?" -
in this case option is considered "optional".
For example, this message
PUSH_UPDATE,?unsupported_option_a
does nothing, but this one:
PUSH_UPDATE,dns 0,block-ipv6,unsupported_option_b
makes client reconnect, since it contains mandatory unsupported option.
OVPN3-1234
Signed-off-by: Lev Stipakov <lev@openvpn.net>
Use empty braces to initalise the structs to zero since they
use sub structs and clang wants us to otherwise use {{ 0 }}
Ensure that methods with a return value do not return without a
value or exception by throwing an exception.
Add missing override in the 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>
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 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>
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 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>
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>
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>
When correcting conversion issues in RouteBase a to_string bug was
introduced which caused some characters to be escaped when inserted
to the string, for example a prefix_len of 0 would render as "\0"
rather than inserting '0'. The std::ios::binary flag does not seem
to prevent this for std::ostringstream so I have cast the data member
up to uint16_t which should be safe, and solves the issue.
Added a unit test to demonstrate the issue. Old code output was
"0.0.0.0/\0", now outputs "0.0.0.0/0" as expected.
Signed-off-by: Charlie Vigue <charlie.vigue@openvpn.com>
In commit 1b4f736bb9, an additional parentheses was added to
the MacGatewayInfo constructor. This results in code which cannot be
compiled.
Signed-off-by: David Sommerseth <davids@openvpn.net>
ERR_INVALID_OPTION_DNS -- invalid value for some of DNS\Domain options
ERR_INVALID_OPTION_CRYPTO -- invalid value for some of SSL\Crypto option
ERR_INVALID_CONFIG -- missing option or not supported option
ERR_INVALID_OPTION_PUSHED -- pushed to server option error
ERR_INVALID_OPTION_VAL -- invalid value for some general option
Signed-off-by: Illia Polishchuk <illia.polishchuk@openvpn.com>
Until now sitnl was just default to metric 0 when installing routes,
while ignoring any value that may have been passed by the user.
Extend logic to properly accept a user value.
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
When the user specifies no metric (i.e. value is -1), the TunBuilder
should pass the default value down the stack.
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
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 <arne@openvpn.net>
It was noted that the SITNL unit test is always failing for no clear
reason.
It turned out that commit 22ba196429
("SITNL: revert change of sitnl_send return type, return int"),
that was supposed to be a simple revert of
ae663c573a ("Using new numeric
conversion tools") is actually converting two "return ret" into
return -1 and return -EINVAL accordingly.
This accidental change results in two functions always returning
an error despite terminating succesfully.
This behaviour was obviously fooling the unitest which failed in result.
Fix both functions by properly returning "ret" as it was originally.
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
Out of all the suggestions by Coverity I picked
the ones that move non-Ptr objects into variables
or attributes.
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Currently PG only allows to either send or withhold the reason to the
client but there are certain circumstances where you want to have more
detailed internal reason but still want to send some reason to the
client.
Signed-off-by: Arne Schwabe <arne@openvpn.net>
This was changed in commit
ae663c573a ("Using new numeric
conversion tools") to avoid some conversion warnings. But
after understanding the workings of the function better, the
change turns out to have been wrong. Instead the function was
changed to use different intermediate variables for different
purposes.
This change ripples through the whole Netlink/SITNL interface.
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Past changes make it clear that the interface was not well
understood.
While here, clean up the code to make it easier to understand.
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
-- disambiguate new_obj(): new_man_obj(), new_tun_obj
-- remove obfuscatory typedef <class> Base; use <class>
-- in servproto.hpp typedef ProtoContext::ProtoConfig to ProtoConfig
since Arne's already disambiguated Config
-- disambiguate Link<>: TCPLink<>, UDPLink<>
Added TODO comment on unneeded version of control_net_recv()
Signed-off-by: Mark Deric <jmark@openvpn.net>
When setting IP address with "gateway" option,
Windows by some reasons creates 0.0.0.0/0 route
which we later remove. However for a few seconds
while this route exists it might interfer with routing.
To work around that, we initially set tun interface metric to
very high, which makes Windows create a rougue route with
high metric. After a few seconds we delete that route and
set metric to a lowest value.
Fixes https://github.com/OpenVPN/openvpn3/issues/281
Signed-off-by: Lev Stipakov <lev@openvpn.net>
For some reason RouteBase uses unsigned char for prefix_length
while all other code uses int or unsigned int. For now just
cast it, prefix_length should be <= 128 anyway.
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Unlike OpenVPN v2, v3 support split DNS already, so we need to make sure
that --dns options are added in a way that results in NRPT rules to be set. At
this time that means the --dns resolve-domains are added as search
domains and --dns search-domains (only the first one really) as an
adapter specific domain suffix.
Signed-off-by: Heiko Hund <heiko@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>
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>
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>
With ovpn-co-v2 logic, control packets do not flow through netlink
anymore but they are sent directly via the transport socket.
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
