Gert Doering [Sun, 6 Dec 2020 12:57:11 +0000 (13:57 +0100)]
Fix line number reporting on config file errors after <inline> segments
<inline> segments neglected to increment the "current line number
in config file" variable (line_num), so after the first <inline>,
errors reported have the wrong line number.
Fix by introducing an extra argument to read_inline_file() function:
"so many lines in the inline block", and changing the return values of
the "check_inline*()" functions to "int", changing this from "false/true"
to "0 = no inline, 1...N = inline with <N> lines".
On calling add_options() this is implicitly converted back to bool.
v2: use int return value, not extra call-by-reference parameter
Trac: #1325 Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20201206125711.12071-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21334.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Steffan Karger [Thu, 3 Dec 2020 15:49:51 +0000 (16:49 +0100)]
tls-crypt-v2: also preload tls-crypt-v2 keys (if --persist-key)
This allows tls-crypt-v2 servers to drop privileges after reading the
keys. Without it, the server would try to read the key file for each
connecting client. (And clients for each reconnect.)
As with the previous patch, the pre-loading was developed in parallel
with tls-crypt-v2, and the tls-crypt-v2 patches were never amended to
implement the pre-loading.
Also as with the previous patch, it would be nicer if servers would not
reload the tls-crypt-v2 server key for each connecting client. But let's
first fix the issue, and see if we can improve later.
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20201203154951.29382-2-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21307.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Steffan Karger [Thu, 3 Dec 2020 18:22:30 +0000 (19:22 +0100)]
tls-crypt-v2: fix server memory leak
tls-crypt-v2 was developed in parallel with the changes that allowed to
use tls-auth/tls-crypt in connection blocks. The tls-crypt-v2 patch set
was never updated to the new reality after commit 5817b49b, causing a
memory leak of about 600 bytes for each connecting client.
It would be nicer to not reload the tls-crypt-v2 server key for each
connecting client, but that requires more refactoring (and thus more time
to get right). So for now just plug the leak by free'ing the memory when
we close a client connection.
To test this easily, compile openvpn with -fsanity=address, run a server
with tls-crypt-v2, connect a client, stop the server.
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20201203182230.33552-1-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21310.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Wed, 2 Dec 2020 11:59:28 +0000 (12:59 +0100)]
Remove auth_user_pass.wait_for_push variable
This variable was first introduce in earlier attempt to fix the
auth-token problems with auth-nocache before user_password and
auth_token were split into two variables. The idea of the variable it
is being set if --pull is in use. However the variable was not always
set correctly, especially if username/password are queried after an
expired auth-token. Instead using that variable use session->opt->pull
directly.
Patch V2: rename delayed_auth_pass_purge to ssl_clean_user_pass to give
a more fitting name since this function is not only used in
the delayed code path and also the new name aligns with
ssl_clean_auth_token. Also fix a leftover wait_for_push
in that function
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20201202115928.16615-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21297.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Mon, 30 Nov 2020 12:39:28 +0000 (13:39 +0100)]
Fix auth-token not being updated if auth-nocache is set
This fixes the auth-token not being updated if auth-nocache is set. Our
set_auth_token method ensures that the auth-token always has a username
but is a little bit too strict in the check.
Also add doxygen documentation and remove null checks. We use this function
only with non-null pointers and it makes it a bit nicer to read.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20201130123928.21837-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21291.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 23 Oct 2020 12:02:59 +0000 (14:02 +0200)]
Make any auth failure tls_authentication_status return auth failed
Previously tls_authentication_status only return
TLS_AUTHENTICATION_FAILED if there is no usable key at all. This
behaviour allows continuing using the still valid keys
(see --tran-window). However, the OpenVPN protocol lacks a way of
communicating that key is not useable to client once it reached
the TLS authenticated status (eg cert checks pass but connect or
user-pass verify fail). To avoid these desynchronisation issues
during deferred auth and renegotiation OpenVPN quietly only starts
using a new key after the hand-window has passed.
With this change any failure on a renogiation will lead to a
deauthentication of a client. This also fixes a number of bugs that
expiring auth-token and failed deferred auth is leading to key desync
or unexpected continuation of the VPN session.
The behaviour of deauthentication of all keys on deferred auth failure
has been already been used for years if authentication is done via
management interface. This commit also aligns the code paths for both.
A side effect might be that we also deauth clients earlier in some
other corner cases but the behaviour of continuing using an old
authenticated session while we already a failed authentication for the
client is most times unexpected behaviour from the user (admin).
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20201023120259.29783-7-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21223.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 23 Oct 2020 12:02:58 +0000 (14:02 +0200)]
Send AUTH_FAILED message to clients on renegotiation failures
This changes the exit in server mode on renegotiation to an exit that
also sends an AUTH_FAILED to the client. Any previously set failed auth
reason is passed to the client.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20201023120259.29783-6-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21222.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 23 Oct 2020 12:02:57 +0000 (14:02 +0200)]
Rename DECRYPT_KEY_ENABLED to TLS_AUTHENTICATED
The macro's name suggests that the key is enabled and being used. But
the macro actually something different but similar enough that the name
was probably right at some point.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20201023120259.29783-5-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21221.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 23 Oct 2020 12:02:56 +0000 (14:02 +0200)]
Clean up tls_authentication_status and document it
The gain of the used optimisation approach of using a array with a
calculated index in favour of simple ifs is questionable with modern
compilers and the readability of the function suffers.
Also change the return type from simple int to an enum and add comments
and doxygen documentation.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20201023120259.29783-4-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21224.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 23 Oct 2020 12:02:55 +0000 (14:02 +0200)]
Improve keys out of sync message
The current message basically lacks the information to actually figure
out why the keys are out of sync. This adds the missing information to
that diagnostic message.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20201023120259.29783-3-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21226.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 23 Oct 2020 12:02:53 +0000 (14:02 +0200)]
Replace key_scan array of static pointers with inline function
The key_scan array is (was) an array that is setup as a reference to
members of itself that have static offsets. Replace this pointer
indirection with an inline function. This has also the advantage
that the compiler can inline the function and just just a direct
offset into the struct.
Replacing the implicit indirection with the pointer array with an
explicit indirection with the inline function also makes the code a
bit easier to follow.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20201023120259.29783-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21225.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
David Sommerseth [Thu, 29 Oct 2020 21:32:59 +0000 (22:32 +0100)]
build: Fix missing install of man page in certain environments
It turns out the logic for dist_man_MANS was incorrectly put inside the
HAVE_PYDOCUTILS block. This results in the man page being installed
only if python-docutils is installed and available.
The solution is simple, move the dist_man_MANS part outside the
python-docutils block. The openvpn.8 file is prebuilt in source
tarballs and will thus be available.
Reported-By: Philip Brown <philip@pbdigital.org> Tested-By: Philip Brown <philip@pbdigital.org> Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20201029213259.1636-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21236.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 23 Oct 2020 11:32:44 +0000 (13:32 +0200)]
Remove --disable-def-auth configure argument
With scripts, plugin and management interface now all supporting
deferred auth, maintaining support of --disbale-def-auth becomes more
of a burden and the few kilobyte in potential binary size do not
outweigh this. Also the code in ssl_verify is hard to hard because
all the ifdefs.
Especially for management interface there are so many features not
directly related to deferred that depend on MANAGEMENT_DEF_AUTH
(like client-kill) that supporting management without deferred auth
is not worth it anymore. And removing this remover a high number of
ifdefs in manage.c/h
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20201023113244.26295-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21214.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Tue, 13 Oct 2020 20:47:58 +0000 (22:47 +0200)]
Avoid passing NULL to argv_printf_cat() in temp_file error case.
To pass username + password to verify_user_pass_script(), OpenVPN
can either put both into environment, or create a temp file, and
pass that file name to the "user-pass-verify" script. The file
name is initialized as "", so if no file is desired, it's well
defined - but if the file can not be created, the pointer is NULL
afterwards.
Change the sequence of events, setting up the argv before the
"if (file)" conditional, and add the file name only inside that
clause, if creating the temp file succeeded.
commit a4eeef17b2 did not create the problem, but modified the
code enough so that the static analyzer in gcc 9.2.0 *now* noticed
and issued a warning.
Arne Schwabe [Mon, 5 Oct 2020 11:16:14 +0000 (13:16 +0200)]
Add function for common env setting of verify user/pass calls
This removes the code duplication in verify_user_pass_script,
verify_user_pass_plugin and verify_user_pass_management.
This also fixes a bug that username is not set if auth-gen-token is
used without the external-auth flag as without calling any external auth
method, the environment would not be setup for connect-client calls.
This patch also removes an indentation level in most of touched functions
so diffing without whitespaces is recommended for review.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20201005111614.29325-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21174.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Mon, 5 Oct 2020 09:18:05 +0000 (11:18 +0200)]
Ignore deprecation warning for daemon on macOS
macOS warns that we should posix_spawn instead. However posix_spawn
would require a major redesign of code to daemonise or drop the --daemon
feature on macOS. Ignore the clang warning in order to allow -Werror
compile on macOS.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20201005091805.17260-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21171.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Sat, 10 Oct 2020 08:14:35 +0000 (10:14 +0200)]
Fix compilation on pre-EKM mbedTLS libraries.
commit f0734e49956217 simplified key_state_export_keying_material(),
changing the function prototype. For older mbedTLS versions, there
is an "always fail" dummy function which was overlooked in that change.
Fix prototype.
v2: also adjust function return (NULL -> false)
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Steffan Karger <steffan@karger.me>
Message-Id: <20201010081435.2154-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21198.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Steffan Karger [Fri, 9 Oct 2020 14:47:55 +0000 (16:47 +0200)]
Simplify key material exporter backend API
Just pass pointer and length, instead of a gc and return (possibly)
allocated memory. Saves us some gc instantiations and memcpy()s. Exact
same functionality, 19 lines less code.
(Didn't want to delay the TLS EKM reviews for this, so submitted as a
patch afterwards.)
Signed-off-by: Steffan Karger <steffan@karger.me> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20201009144755.39719-1-steffan@karger.me>
URL: https://www.mail-archive.com/search?l=mid&q=20201009144755.39719-1-steffan@karger.me Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 9 Oct 2020 11:54:53 +0000 (13:54 +0200)]
Implement generating data channel keys via EKM/RFC 5705
OpenVPN currently uses its own (based on TLS 1.0) key derivation
mechanism to generate the 256 bytes key data in key2 struct that
are then used used to generate encryption/hmac/iv vectors. While
this mechanism is still secure, it is not state of the art.
Instead of modernising our own approach, this commit implements
key derivation using the Keying Material Exporters API introduced
by RFC 5705.
We also use an opportunistic approach of negotiating the use of
EKM (exported key material) through an IV_PROTO flag and prefer
EKM to our own PRF if both client and server support it. The
use of EKM is pushed to the client as part of NCP as
key-derivation tls-ekm.
We still exchange the random data (112 bytes from client to server
and 64 byte from server to client) for the OpenVPN PRF but
do not use it. Removing that exchange would break the handshake
and make a key-method 3 or similar necessary.
As a side effect, this makes a little bit easier to have a FIPS compatible
version of OpenVPN since we do not rely on calling MD5 anymore.
Side note: this commit breaks the (not yet merged) WolfSSL support as it
claims to support EKM in the OpenSSL compat API but always returns an error
if you try to use it.
Patch v2: rebase/change to V2 of EKM refactoring
Patch v3: add Changes.rst
Patch v4: Rebase on master.
Patch v5: Refuse internal label to be used with --keying-material-exporter,
polishing/fixes suggested by Steffan integrated
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Steffan Karger <steffan.karger@foxcrypto.com>
Message-Id: <20201009115453.4279-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21187.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Thu, 8 Oct 2020 11:59:59 +0000 (13:59 +0200)]
Allow 'none' cipher being specified in --data-ciphers
Although we want to get rid of none as cipher, we still have not
deprecated it. In order to use it currently you need
--ncp-disable together with --cipher none to use the none cipher
otherwise OpenVPN will spit out an error about an unrecognised
cipher in --data-ciphers.
In our current situation allowing none to be specified in data-ciphers
is the lesser evil.
This commit also fixes that we use '[null-cipher]' instead 'none' when
setting remote_cipher.
Note that negotiating to cipher 'none' can the same the same problems
with frame size calculation as any other non AEAD cipher. If
--cipher none is also specified in the configuration, the workaround
of commit e539c95dc will also apply to cipher none.
Patch V2: Also work correctly if remote_cipher is NULL.
Patch V3: fix unit tests, add note about corner case
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20201008115959.21151-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21181.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
OpenVPN has the ability to choose different X509 field in case "CN" can
not be use used to be unique connected username since commit 935c62be9c0c8a256112df818bfb8470586a23b6 "Choose a different field in
X509 to be username".
Unfortunately it's not enough in case when client has multiple and
valid certificates from PKI for different devices (ex. laptop,
mobile, etc) with the same CN/UID.
Having --duplicate-cn as a workaround helps only partially: clients can
be connected, but it breaks coexistance with --ifconfig-pool-persist,
--client-config-dir and opens doors to DoS possibility since same client
device (with the same cert) being reconnected no more replaces previously
connected session, so it can exhaust server resources (ex. address pool)
and can prevent other clients to be connected.
With this patch, multiple X509 fields incl. "serialNumber" can be chosen
to be username with --x509-username-field parameters, they will be
concatened into the one username using '_' separator. As long as the
resulting username is unique, --duplicate-cn will not be required.
Default field is preserved as "CN".
Openssl backend is the only supported, since so far MbedTLS has no
--x509-username-field support at all.
v2: conform C99, man update, fix typos
v3: reuse buffer methods, drop delimiter define, use memcpy
v4: man update, change separator "_" to avoid path issues on windows
v5: mention collision possibility with "_" separator in man
capitalize hex serialNumber value
Signed-off-by: Vladislav Grishenko <themiron@yandex-team.ru> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20201005005114.13619-1-themiron@yandex-team.ru>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21168.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Fri, 2 Oct 2020 17:57:36 +0000 (19:57 +0200)]
Fix redirecting of IPv4 default gateway if connecting over IPv6.
Commit aa34684972eb0 fixed a long-standing bug in setting the
"route-list" flag RTSA_REMOTE_HOST for IPv4 ("we have a well-defined
remote_host == VPN server IP address") even if connecting over IPv6.
Unfortunately the logic in redirect_default_route_to_vpn() was also
wrong, and refused cooperation if that flag is not set, triggering
the message
"NOTE: unable to redirect IPv4 default gateway -- Cannot
obtain current remote host address"
Correct operation: if RTSA_REMOTE_HOST is not set, or remote_host
is IPV4_INVALID_ADDR (= 255.255.255.255), do not try to install a
host route for continued connectivity to the VPN server - which is
not needed when connecting over IPv6. But the actual *routes*
(/0 or 2 x /1) can be installed just fine.
There is a second bug here, which hits if there is no IPv4 gateway
at all. In that case, the same function triggers the message
"NOTE: unable to redirect IPv4 default gateway -- Cannot
read current default gateway from system"
This is caused by using "IPV4_INVALID_ADDR" as a flag for "do we
know the remote_host?" - which worked before, but after the commit
referenced above, the "remote_host" field is not well-defined unless
RTSA_REMOTE_HOST is set. So, change the condition to check that.
Reported-By: François Kooman <fkooman@tuxed.net> Reported-By: Thomas Schäfer <tschaefer@t-online.de>
Trac: #1332
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <20201002175736.82609-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21152.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
For non-blocking TCP/Unix connection, OpenVPN checks was it established in
loop and if not - sleeps or handles management for next one second. Since
the first check is made right after the connection attempt, it will likely
be always unsuccessful, causing redundant wait for one or more seconds:
After this patch openvpn_connect() will perform blocking wait for
connection
establishment (if possible) and just check for management events once in
one
second (if management enabled) w/o sleep. This speedups TCP/Unix connection
establishment and provides almost real connection time that can be used for
detection of the fastest remote server in subsequent patches:
v2: cosmetics, decrease connection_timeout to avoid wait more than it
v3: teach management_sleep() to handle zero timeout and reject negative
use 1s timeout for connection and 0s timeout for management events
Signed-off-by: Vladislav Grishenko <themiron@yandex-team.ru> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20201001225319.25125-1-themiron@yandex-team.ru>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21139.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Per https://community.openvpn.net/openvpn/wiki/CodeStyle the maximum line
length is 80 characters. This patch allows to split upcoming changes into
CodeStyle-conformant (w/o real code change) and more feature-specific.
Upcoming changes adds new PROTO_AUTO, so existing proto_names array is
reformatted as well.
v7: prefer line breaks before long string parameters
reformat proto_names array
Signed-off-by: Vladislav Grishenko <themiron@yandex-team.ru> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20200924091004.29065-1-themiron@yandex-team.ru>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21083.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
It's a long while since the bundled lz4 library has received an update.
It pulls in a lot of various fixes and enhancements, some of the changes
fixes compiler warnings and hardens the code a bit too.
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20201001154658.9798-1-davids@openvpn.net>
URL: https://www.mail-archive.com/search?l=mid&q=20201001154658.9798-1-davids@openvpn.net Signed-off-by: Gert Doering <gert@greenie.muc.de>
Fix update_time() and openvpn_gettimeofday() coexistence
With TIME_BACKTRACK_PROTECTION defined, openvpn_gettimeofday() uses and
updates global variable "now_usec" along with "now" only if current time
is ahead of the previsouly stored, taking nanoseconds into account.
But, update_time() function updates only "now" leaving "now_usec" as
is with any previously value stored.
This breaks openvpn_gettimeofday() results and leads to time jumps in the
future within one second, can affect shaper and user timers.
Example:
100.900 openvpn_gettimeofday():
now set to 100s, now_usec set to 900ns, stored time is 100.900
101.300 update_time():
now set to 101s, but now_usec is not updated and still 900ns, stored
time jumps to the future 101.900
101.600 openvpn_gettimeofday():
current time 101.600 is in the past relatively stored time 101.900,
now & now_usec variables are not updated, returned time 101.900 is
still and again incorrect
102.100 openvpn_gettimeofday():
current time 102.100 is no longer in the past relatively stored time
101.900, so now & now_usec get updated with wrong time delta from
previous openvpn_gettimeofday() call or now/now_usec math
Since update_time() and openvpn_gettimeofday() calls are mixed in runtime,
there're several options to fix the things:
1. Allow update_time() to reset "now_usec" value backward to 0, since it's
used directly only in time ajusting and always invalidate it in
openvpn_gettimeofday() unless time has drifted backwards.
Quick solution that only fixes openvpn_gettimeofday() and keeps current
level of time performance and backward-protection handling way.
2. Switch update_time() to gettimeofday() not only for windows, but for all
platforms: "now_usec" will be updated accordingly. As a disadvantage,
gettimeofday() may have performance penalty on older or platforms w/o
VDSO
where expensive kernel syscall will be made. And it will still need time
adjusting code, doubt it's feasible.
3. Switch update_time() and openvpn_gettimeofday() to clock_gettime() on
Linux/BSD platforms with CLOCK_REALTIME_FAST/CLOCK_REALTIME_COARSE
clock sources. According tests it'll be faster with VDSO than
gettimeofday()
or CLOCK_REALTIME/CLOCK_REALTIME_PRECISE, but still may require
adjusting
code to protect from time jumps on devices with no RTC (ex. routers)
where
NTP is the only way to get correct time after boot. Since not every
*libc
have clock_gettime() and corresponding CLOCK_* defines and/or running
kernel may have no VDSO/corresponding CLOCK_* support - related
autotools
checks and fallback code can still be necessary.
4. Switch update_time() and openvpn_gettimeofday() to clock_gettime() on
Linux/BSD platforms with CLOCK_MONOTONIC_FAST/CLOCK_MONOTONIC_COARSE
clock sources. This may allow to get rid of time adjusting code at all
with the acceptable performance on modern systems, but may still require
to fallback to gettimeofday() with adj friends on older platforms (most
likely to be Linux CPE/routers). From the effort point of view,
splitting
the whole OpenVPN code into realtime/monotonic is most significant and
desired task among the above, several parts still needs to use realtime
due API or storage or output reasons.
This patch implements the first stage only.
v2: move from gettimeofday() (1st way) back to time(), don't check previous
value of "now_usec" in update_usec() instead
v3: recover "now_usec" checks against time jumps within one second, zero it
in update_time() calls instead to pass the check.
Signed-off-by: Vladislav Grishenko <themiron@yandex-team.ru> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200922170841.13729-1-themiron@yandex-team.ru>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21070.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Use wmic instead of directly editing the registry
as the former does not take full effect unless the dns
client service is restarted.
Editing the registry appears to work erratically depending
on whether its followed with a dchp renew or ipconfig /registerdns
etc.
DOMAIN-SEARCH is not handled here as wmic only supports
setting the global search list which will over-ride all
interface specific values. Editing the registry directly
combined with a wmic command to reset the global SearchList
is an option that could be considered in a separate patch.
Trac # 1209, 1331
v2 changes
- Separate DNS domain setting from DNS server setting and call
only once either during IPv4 processing or IPv6 processing
if the former is not active. (file changed: tun.c)
- Null terminate domain and interface_name received from the
client. (file changed: interactive.c)
Its done using a const cast-away of msg in a limited scope.
Not pretty, but alternatives are no better.
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <1601085886-10351-1-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21097.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
openvpnmsica: Simplify find_adapters() to void return
As the find_adapters() failure is not critical and FindSystemInfo()
should continue regardless, the find_adapters() has been simplified not
to return result code. It still logs any error though.
Signed-off-by: Simon Rozman <simon@rozman.si> Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20200924065519.1839-1-simon@rozman.si>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21077.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Mon, 14 Sep 2020 07:08:43 +0000 (09:08 +0200)]
Fix combination of --dev tap and --topology subnet across multiple platforms.
--topology should have no effect in tap mode (tap is always "subnet"),
but due to the way options are checked, setting "topology subnet" caught
an improper branch on all non-linux and non-win32 platforms.
Easily tested by adding "--topology subnet" to a "--dev tap" t_client
test.
Tested, verified, and fixed on FreeBSD 13.3, NetBSD 8.1, OpenBSD 6.5,
OpenIndiana 2019 (Solaris) and MacOS X Mojave.
This is a forward-port of commit 6c13e24e5709 - the original intent
for "master" was to restructure tun.c in a larger way and clean up
these if() blocks more nicely... which has not happened yet, so this
patch is basically applying exactly the same changes to context that
has changed too much for git to be able to do this automatically.
Trac: #1085
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <20200914070843.51678-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20987.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Thu, 17 Sep 2020 16:19:09 +0000 (18:19 +0200)]
Add demo plugin that excercises "CLIENT_CONNECT" and "CLIENT_CONNECT_V2" paths
This is a new "samples" plugin which does not do many useful things,
besides
- show how a plugin is programmed
- how the various messages get dispatched
- how to pass back information from a client-connect/v2 plugin
- how to do async-cc plugins [not yet implemented]
the operation of the plugin is controlled by UV_WANT_* environment
variables
controlled by the client ("--setenv UV_WANT_CC_FAIL 1 --push-peer-info"),
to "fail CLIENT_CONNECT" or "use async-cc for CLIENT_CONNECT_V2" or
"send 'disable' back from ...") - which is useful for automated testing
of server success/defer/fail code paths for the CLIENT_CONNECT_* functions.
See samples/sample-plugins/client-connect/README for details how to do
this.
v2:
- implement async / deferred operation both for CLIENT_CONNECT and
CLIENT_CONNECT_V2 plugin calls
- implement returning openvpn-controlled (setenv) config snippets
(so the client side can verify in automated testing that the plugin
operated correctly, without hard-coding something in the plugin code)
v3:
- remove -Wno-unused-variable from Makefile
- remove unused "char ** argv" (commented out, but kept as reference)
v4:
- upgrade to use the build infra brought by commit 0b5141d8f946
- remove local Makefile
- include "config.h" to get what is needed to get rid of the strdup()
warning Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20200917161909.11573-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21047.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Thu, 17 Sep 2020 08:59:41 +0000 (10:59 +0200)]
If IPv6 pool specification sets pool start to ::0 address, increment.
The first IPv6 address in a subnet is not usable (IPv6 anycast address),
but our pool code ignored this.
Instead of assigning an unusable address or erroring out, just log the
fact, and increment the pool start to <pool_base>::1
NOTE: this is a bit simplistic. A pool that is larger than /96 and
has non-0 bits in the "uppermost bits" will still get the increment
as we only look at the lowermost 32 bits.
NOTE2: if the pool is specified with "--server-ipv6 $base/$bits", this
is a non-issue, as the address for the pool start will be incremented
anyway.
v2: make comment more explicit about "we're only talking about the
host part here" and "base sees only only 32 bit of the host part"
Reported-by: NicolaF_ in Trac
Trac: #1282
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <20200917085941.20972-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21039.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
If remote server has been resolved to multiple addresses, at
least one connection attempt has been made and connection to
the last address was skipped by management - resolved earlier
link socket addrinfo objects will not be cleared neither on
instance close nor in the next connection entry loop.
This causes fatal error assert:
David Sommerseth [Wed, 16 Sep 2020 19:56:16 +0000 (21:56 +0200)]
build: Fix make distclean/distcheck
In commit 0b5141d8f94 the sample-plugins got partially migrated to
automake. But since it was not fully integrated within the full
standard build, the sample/sample-plugins/Makefile was not removed
by 'make distclean', which annoys 'make distcheck'.
The simplest way is just to explicitly enlist this Makefile in the list
of files 'make distclean' should remove.
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200916195616.30633-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21026.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
David Sommerseth [Wed, 16 Sep 2020 14:19:56 +0000 (16:19 +0200)]
sample-plugins: Partially autotoolize the sample-plugins build
The sample-plugins have their own set of build/winbuild scripts in each
of these plugin directories. This does not give a good way to reuse
various macros the autoconf/automake/configure process enables; which
can contain important macros to make some code build without errors or
warnings.
Normally we would embrace the full autoconf/automake approach. But this
is sample code which we only want to build per request and the built
code should not be installed anywhere via 'make install'. But since we
do use libtool other plug-ins being installed and automake gets kind of
cranky when it comes to define certain build targets not following the
expected use cases, we try to only embrace just enough of automake to
get our main goals achieved.
This changeset kicks out the build scripts and replaces them with a
single Makefile.plugins file, which defines the plugins we want to build
by default when running 'make from the sample-plugins directory.
Neither of these plugins are otherwise built by default. No sample-plugins
are being installed. But we have enough strings attached to automake
to grab the CFLAGS and LDFLAGS used by the rest of the code. This also
makes it easy to use #include "config.h" in sample code, to also get
various macros defined by the ./configure run.
This patch does not touch the winbuild scripts, as it seems building
these sample-plugins on Windows requires a bit different compile and
linking steps than *nix systems in general.
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200916141956.1277-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21020.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Tue, 15 Sep 2020 09:41:01 +0000 (11:41 +0200)]
Fix netbits setting (in TAP mode) for IPv6 on Windows.
For TUN interfaces, the IPv6 address needs to be configured with
"address/128" and a local subnet route is needed, pointing to our
fake gateway fe80::8. There is no ethernet headers or ND outside
the tun/tap interface, so anything but fe80::8 is not resolvable.
For TAP interfaces, the proper subnet mask (netbits) must be configured,
and no connected route to "our local host address" must be configured,
to make make IPv6 ND work inside the local subnet.
Our code was nicely consistent in doing the same thing in tun.c
("gui/openvpn running with admin privileges") and in the requests
to the interactive service ("gui running with user privs"). Fix in
both places.
On tun close, symmetric to addition, remove the on-link subnet route only
for "tun" interfaces. Address removal works without specifying netbits.
While at it, extend do_address_service() to actually log both IPv4
and IPv6 addresses requested via it.
Tested on Win10/64.
v2:
- change logging to use D_IFCONFIG
- fix whitespace on "?" operator
Allow --dhcp-option in config file when windows-driver is wintun
When wintun is in use we mutate ip_win32_type to NETSH
and then complain that ip-win32 option should be dynamic or adaptive
if any --dhcp-option directive is present in the config file. This
causes a fatal error.
How to reproduce: specify a --dhcp-option in the config and change the
--windows-driver to wintun.
Fix this behaviour. A typo in the message is also corrected.
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <1600126181-16364-1-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21005.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
The --remote entry had a syntax mistake in the argument examples, which
was introduced during the .rst conversion.
In addition this section did not have a good flow. So the text was
regrouped and re-organized a bit so related text pieces are now gathered
in the same context instead of being more spread out.
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200909183012.7504-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20935.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Wed, 9 Sep 2020 12:22:23 +0000 (14:22 +0200)]
socks.c: fix alen for DOMAIN type addresses, bump up buffer sizes
When a SOCKS5 server sends back a reply, it encodes an "address",
which can be IPv4 (4 bytes), IPv6 (16 bytes) or "a domain name",
which has a lenght (1 byte) and "a string of length <length>" - so
when copying bytes, we need to hande "length +1" bytes.
Our code totally doesn't use this variant of addresses on reception,
but since this has been pointed out by "tpw_rules" in Trac, fix it,
so if/when someone works on this again, the foundation is correct.
While at it, increase buffer size used for sending to handle domain
names longer than 122 characters (length was already checked, so a
longer name would not overflow but just "not work").
v2: increase buf[] len in recv_socks_reply() from 22 to 270 so it
is large enough to actually copy a domain name
v3: increase buf[] len in establish_socks_proxy_passthru() from 128 to
270, to handle long domain names in queries
Reported-By: tpw_rules in Trac
Trac: #848
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <20200909122223.9222-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20928.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Sun, 13 Sep 2020 14:56:21 +0000 (16:56 +0200)]
Fix --show-gateway for IPv6 on NetBSD/i386.
Our ROUNDUP() macro to achieve the required system-specific alignment
for data structures sent to the routing socket was wrong for NetBSD -
unlike OpenBSD/FreeBSD, NetBSD is not using "long" (32/64 bit depending
on OS architecture), and not "uint32_t" either (32/32) like MacOS, but
uint64_t.
So our use of "long" always worked on NetBSD/amd64 and stopped working
on NetBSD/i386 when this was changed on the OS side...
NetBSD conveniently exports a RT_ROUNDUP() macro from <net/route.h> - use
that, and avoid trying to second-guess OS requirements.
While at it, add M_ERRNO to ominous "GDG6: problem writing to routing
socket"
error message to differenciate between "EINVAL" and other errors.
Trac: #734
Signed-off-by: Gert Doering <gert@greenie.net> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20200913145621.12125-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20983.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Wed, 9 Sep 2020 10:48:37 +0000 (12:48 +0200)]
Handle NULL returns from calloc() in sample plugins.
This is basic housekeeping, adding NULL checks to context initialization
of the sample plugin collection which are missing it. Realistically,
this can never happen, but since these are supposed to be "good examples",
not checking calloc() return isn't one.
Trac: #587
Reported-By: Dogbert (in Trac) Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20200909104837.6123-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20922.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
David Sommerseth [Fri, 11 Sep 2020 15:42:59 +0000 (17:42 +0200)]
man: Add missing --server-ipv6
During the conversion from .8 to .rst and further reorganizing of the
content into separate files, the --server-ipv6 entry got lost. This
resurrects it again.
Trac: #1324
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200911154259.13837-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20970.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Wed, 9 Sep 2020 12:29:26 +0000 (14:29 +0200)]
Fix description of --client-disconnect calling convention in manpage.
The man page claimed that --client-disconnect "is passed the same
pathname as the corresponding --client-connect command", which is
not what the code does. Fix.
Reported-By: hvenev in Trac
Trac: #884
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <20200909122926.9523-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20929.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Fri, 11 Sep 2020 08:59:07 +0000 (10:59 +0200)]
Fix handling of 'route remote_host' for IPv6 transport case.
If we connect to a VPN server over IPv6, and the config has a
route like this:
route remote_host default net_gateway
OpenVPN would try to install a route to "255.255.255.255", which
is obviously bogus.
The bug is twofold: init_route_list() should not set RTSA_REMOTE_HOST
for an "IPV4_INVALID_ADDR" remote_host (wrong condition, this is not
a pointer but an integer, and "invalid" is "-1" numerically here),
and init_route() must not ignore "status = false" returns from
get_special_addr().
I have just added the "if (!status)" check, not done refactoring for
init_route() to see whether I could make it "more pretty".
Trac: #1247
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20200911085907.26004-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20958.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Netlink route request with NLM_F_DUMP flag set means to return
all entries matching criteria passed in message content -
matching supplied family & dst address in our case.
So, gateway from the first ipv4 route was always used.
On kernels earlier than 2.6.38 default routes are the last ones,
so arbitrary host/net route w/o gateway is likely be returned as
first, causing gateway to be invalid or empty.
After refactoring in 2.6.38 kernel default routes are on top, so
the problem with older kernels was hidden.
Fix this behavior by selecting first 0.0.0.0/0 if dst was not set
or empty. For IPv6, no behavior is changed - request ::/128 route,
so just clarify the sizes via netlink route api.
Tested on 5.4.0, 4.1.51, 2.6.36 and 2.6.22 kernels.
Signed-off-by: Vladislav Grishenko <themiron@yandex-team.ru> Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <20200908123625.23179-1-themiron@yandex-team.ru>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20900.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Tue, 8 Sep 2020 10:51:30 +0000 (12:51 +0200)]
Fix error detection / abort in --inetd corner case.
Calling "openvpn --inetd" from the CLI (= no socket on stdin) will
lead to endless looping in the accept(4) loop.
Instead of cluttering that function further, detect failure to call
getsockame() in phase2_inetd() already, and trigger a M_FATAL abort
on "errno == ENOTSOCK" ("The argument s is a file, not a socket").
While at it, uncrustify the --bind-dev code (whitespace only).
Trac: #350
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20200908105130.24171-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20897.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Tue, 8 Sep 2020 11:15:11 +0000 (13:15 +0200)]
Document that --push-remove is generally more suitable than --push-reset
It's a long-standing and well-known problem that --push-reset removes
"critical" options from the push list (like "topology subnet") which
will then lead to non-working client configs. This can not be
reasonably fixed, because the list of "critical" options depends on
overall server config.
So just document the fact, and point people towards --push-remove as
a more selective tool.
Trac: #29
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Arne Schwabe <arne@rfc2549.org> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20200908111511.9271-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20899.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
In tap.c use DiInstallDevice to install the driver on a new adapter
As reported in Trac 1321, additional adapter installation
by tapctl.exe fails to fully setup the device node (some registry
keys missing, error in setapi.dev.log etc.).
Although the exact cause of this failure is unclear,
letting the Plug and Play subsystem handle the installation
by calling DiInstallDevice() avoids it.
We let the system automatically choose the best driver
by passing NULL for driverinfo to DiInstallDevice().
This also eliminates the need for enumerating all drivers
in the Net class and selecting a matching one.
Somehow mingw-w64 fails to find DiInstallDriver() in
newdev.lib although the header does define it. Use LoadLibrary()
to locate it at run time (available in Vista and above).
Built using mingw and tested both the msi installer (code shared
with libopenvpnmscia.dll) and tapctl.exe on Windows 10 64 bit.
Fixes: Trac #1321 Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <1599177404-29996-1-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20880.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Sun, 30 Aug 2020 13:14:40 +0000 (15:14 +0200)]
Fix client NCP OCC fallback when server and client cipher are identical
If we do not get a cipher pushed we call tls_poor_mans_ncp to determine
whether we can use the server's cipher. Inherited from OpenVPN
2.4's code we only did this check when the ciphers were different.
Since OpenVPN 2.5 does not assume that our cipher we report in OCC
(options->ciphername) is always a valid cipher we always need to perform
this check.
V2: Only call tls_item_in_cipher_list if remote_cipher is non-null to
avoid calling strcmp with NULL.
Reported-By: Rafael Gava <gava100@gmail.com> Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200830131440.10933-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20843.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Tue, 25 Aug 2020 04:16:47 +0000 (06:16 +0200)]
Fix compilation with older mbed TLS versions (mbedtls_tls_prf_types undefined)
The usage of the new keying material methods was not properly guarded.
To avoid a number of ifdefs this commit uses a dummy struct and function.
When we eventually drop support for non-EKM mbed TLS version we can remove
these.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200825041647.26235-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20812.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Thu, 23 Jul 2020 12:19:49 +0000 (14:19 +0200)]
Workaround FreeBSD 12+ race condition on tun/tap open with IPv6.
On FreeBSD 12 (tested and verified on 12.1-RELEASE-p2), after "ifconfig
inet6" for a tun/tap interface, there sometimes is a race condition
where the "IFDISABLED" flag shows up after a short time frame, under
a second, and never clears itself. This disables use of the configured
IPv6 address on the interface, breaking IPv6 over tun/tap operation.
This only happens if ipv6_activate_all_interfaces="YES" is not
set in /etc/rc.conf - but there might be reasons why this is not so.
As a workaround until this can be fixed on the FreeBSD side (or a
better workaround is found), sleep(1) after ifconfig, then call
"ifconfig $dev inet6 -ifdisable".
Yes, this is massively ugly but makes the problem completely go
away for my test systems.
(The same effect can be achieved with an --up script that does this,
but it's even less pretty - see trac ticket)
This refactors the common code between mbed SSL and OpenSSL into
export_user_keying_material and also prepares the backend functions
to export more than one key.
Also fix checking the return value of SSL_export_keying_material
only 1 is a success, -1 is also an error.
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Patch V2: Cache secrets for mbed TLS instead generating all ekms
in the call back function
Patch V3: comment is no longer a lie. (fixed doxygen)
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Steffan Karger <steffan@karger.me>
Message-Id: <20200814145153.12895-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20739.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 14 Aug 2020 08:06:19 +0000 (10:06 +0200)]
Fix client's poor man NCP fallback
This commit fixes two separate issues which are closely linked.
First, a 2.5 client cannot connect to a server which does not support NCP
and is not using one of the default --data-ciphers (AES-*-GCM).
This is because the 2.5 client does not use its configured --data-ciphers
cipher in the "fall back to OCC based cipher negotiation" case. Fix this.
Second, do not allow the 2.5 client to use --data-ciphers-fallback in the
above situation because that is not it's intended use (only to be used if
there is no pushed cipher [NCP] and no OCC provided cipher).
To reproduce the error use a client with only --data-ciphers set against
a server without NCP.
OPTIONS ERROR: failed to negotiate cipher with server.
Add the server's cipher ('AES-256-CBC') to --data-ciphers
(currently 'AES-256-CBC') if you want to connect to this server.
Reported by: Richard Bonhomme <tincanteksup@gmail.com>
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Steffan Karger <steffan@karger.me>
Message-Id: <20200814080619.2108-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20734.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Thu, 13 Aug 2020 10:13:01 +0000 (12:13 +0200)]
Fix stack overflow in OpenSolaris NEXTADDR()
Commit 5fde831c5807 fixed NEXTADDR() for all *BSDs and MacOS.
OpenSolaris has to use a slightly different macro due to lack of
sockaddr->sa_len - but it has the same problem, first rounding up,
then memmove()'ing. Switch order.
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20200813101301.12720-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20731.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Wed, 12 Aug 2020 08:54:12 +0000 (10:54 +0200)]
Improve sections about older OpenVPN clients in cipher-negotiation.rst
- Explain the IV_NCP=2 client situation in 2.4 a bit better.
- Make more clear what exact versions are meant in the old client section
- add a missing - in a heading
Thanks to Richard Bohnhomme for initial proof reading.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200812085412.19178-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20714.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Tue, 11 Aug 2020 11:02:48 +0000 (13:02 +0200)]
Document comp-lzo no and compress being incompatible
Most of the new compress but not v2 version do use swap operation. For
'compress lzo' the swap option is not used for backwards compatibility.
For lz4 the swap option is also not a problem since there is no version
without swap. Unfortunately, compress introduced a second stub format
with swap, contrary to the one in 'comp-lzo no' that does not use swap.
Document this weirdness to let not others fall into this trap.
v2: redo patch for rst man pages
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200811110248.3396-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20708.html
Arne Schwabe [Mon, 10 Aug 2020 14:37:03 +0000 (16:37 +0200)]
Remove S_OP_NORMAL key state.
The key state is virtually identical S_ACTIVE and we only did the state
state transition form S_ACTIVE to S_OP_NORMAL at the point where we
normally would have timed out the TLS negotiation. This is not a very
useful information to have and indeed we never use it anywhere.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200810143707.5834-14-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20674.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Mon, 10 Aug 2020 14:37:02 +0000 (16:37 +0200)]
Merge check_coarse_timers and check_coarse_timers_dowork
This simplifies the code a bit and makes the code flow clearer as
it only adds three curly brackets in check_coarse_timers. Merging the
resulting check_coarse_timers_dowork function into the caller and
called function as with the other function does not make sense here
since it does more than similar function.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200810143707.5834-13-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20671.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Mon, 10 Aug 2020 14:37:00 +0000 (16:37 +0200)]
Eliminate check_incoming_control_channel wrapper function
Move the check that calls this function into the calling function.
Also eliminate the if (len) check in the
check_incoming_control_channel_dowork function as it is only called
if len is > 0 anyway and replace it with a ASSERT.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200810143707.5834-11-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20680.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Mon, 10 Aug 2020 14:36:57 +0000 (16:36 +0200)]
Split pf_check_reload check and check timer in process_coarse_timers
This moves the timer check into process_coarse_timers and makes it
in line with the other functions. The the pf.enabled check is also moved
into process_coarse_timers to make it more clear this only is used if
pf is enabled at all.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200810143707.5834-8-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20664.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Mon, 10 Aug 2020 14:36:56 +0000 (16:36 +0200)]
Remove a number of check/do_work wrapper calls from coarse_timers
This indirection is not very helpful in understanding the code
flow. Move the check to process_coarse_timers, remove the
check function, rename the do_work function to the "real" thing
and then drop the do_work wrapper as it does no longer serve a
purpose.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200810143707.5834-7-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20668.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Mon, 10 Aug 2020 14:36:55 +0000 (16:36 +0200)]
Remove buf argument from link_socket_set_outgoing_addr
This was only used in a check that is better suited in the calling
functions. This also removes passing the buf argument to
link_socket_connection_initiated that also does not use that
parameter at all.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200810143707.5834-6-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20677.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
projects / thirdparty / openvpn.git / log
