When registering DNS on Windows, argv is freed after being used in first
ipconfig command (/flushdns).
Then same argv is used uninitialized in next ipconfig command
(/registerdns)
causing heap exception and subprocess crash.
As a consequence second command is never executed and locked netcmd
semaphore is not cleanly released.
Removing argv freeing between ipconfig calls solves the problem.
This issue was introduced in commit 870e240 (argv: do fewer memory
re-allocations). After a quick glance at commit no similar problem was
spotted in rest of the argv related changes.
Signed-off-by: Domagoj Pensa <domagoj@pensa.hr>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20201215171600.25534-1-domagoj@pensa.hr>
URL: https://www.mail-archive.com/search?l=mid&q=20201215171600.25534-1-domagoj@pensa.hr
Signed-off-by: Gert Doering <gert@greenie.muc.de>
<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>
The port-share option assumed that all openvpn initial reset packets
are between 14 and 255 bytes long. This is not true for tls-crypt-v2.
Patch V2: use correct length for TLS-Crypt v2, use length variable
non-tlscryptv2 test
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Steffan Karger <steffan.karger@foxcrypto.com>
Message-Id: <20201130123813.21388-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21290.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
Almost everywhere in OpenVPN we rely on zero initialisation to
initialise all bool attributes to false.
ret is cleared by ALLOC_OBJ_CLEAR(ret, struct tls_multi);
Having this one variable treated different is a bit confusing.
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20201023113431.26691-4-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21218.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
We (and OpenSSL) already use calling free on null pointers in a number
of places and also C99 standards says free(NULL) does nothing.
The if (x) free(x) calls more often make code harder to read, instead
of easier, remove these NULL checks in favour of directly calling
free(x).
The OpenSSL *_free methods are also safe to call with NULL and
pkcs11h_certificate_freeCertificateIdList is also safe to be called with
NULL.
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20201023113431.26691-5-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21216.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
The semantic of most free methods is to free a pointer and all its
contents and also free the pointer itself. Align reliable_free to this
semantic.
Also clean up the other free uses in key_state_free.
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20201023113431.26691-3-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21215.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
All other places in our code also directly access peer_info and this
function does not contribute to code clarity.
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20201023113431.26691-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21217.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
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.
ssl_verify.c:1132:5: warning: '%s' directive argument is null
1132 | argv_printf_cat(&argv, "%s", tmp_file);
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20201013204758.2472-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21204.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
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>
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>
commit f0734e4995 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>
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>
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>
ASAN yelled at me that someone forgot to call argv_free(). Fix that.
Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <20201009134603.36263-1-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21189.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
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
935c62be9c "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>
This moves the OpenVPN specific PRF into its own function also
simplifies the code a bit by passing tls_session directly instead of
5 of its fields.
Patch v2: Rebase
Patch v4: rewrite/fix comments,
fix potential not initialised before goto issue
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Steffan Karger <steffan@karger.me>
Message-Id: <20200825073643.15920-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20815.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Commit aa34684972 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:
00:00:00.667607 fcntl(5, F_SETFL, O_RDONLY|O_NONBLOCK) = 0
00:00:00.667713 connect(5, {...}, 16) = -1 EINPROGRESS (Operation now
in progress)
00:00:00.667832 poll([{fd=5, events=POLLOUT}], 1, 0) = 0 (Timeout)
00:00:00.667954 nanosleep({tv_sec=1, tv_nsec=0}, 0x7fff52450270) = 0
00:00:01.668608 poll([{fd=5, events=POLLOUT}], 1, 0) = 1 ([{fd=5,
revents=POLLOUT}])
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:
00:00:00.790510 fcntl(5, F_SETFL, O_RDONLY|O_NONBLOCK) = 0
00:00:00.790616 connect(5, {...}, 16) = -1 EINPROGRESS (Operation now
in progress)
00:00:00.790877 poll([{fd=5, events=POLLOUT}], 1, 1000) = 0 (Timeout)
00:00:01.792880 poll([{fd=5, events=POLLOUT}], 1, 1000) = 1 ([{fd=5,
revents=POLLOUT}])
Or, with management interface enabled:
00:00:00.906421 fcntl(5, F_SETFL, O_RDONLY|O_NONBLOCK) = 0
00:00:00.906527 connect(6, {...}, 16) = -1 EINPROGRESS (Operation now
in progress)
00:00:00.906779 poll([{fd=6, events=POLLOUT}], 1, 1000) = 0 (Timeout)
00:00:01.910418 poll([{fd=3, events=POLLIN|POLLPRI}], 1, 0) = 0
(Timeout)
00:00:01.911365 poll([{fd=6, events=POLLOUT}], 1, 1000) = 0 ([{fd=6,
revents=POLLOUT}])
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>
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>
ADAPTER_DOMAIN_SUFFIX is an openvpn3 replacement for
DOMAIN, which is used there for split-dns. This option is pushed
by modern Access Server.
This change improves compatibility between OpenVPN
community client and Access Server.
Signed-off-by: Lev Stipakov <lev@openvpn.net>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200922100021.20329-1-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21107.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>
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>
When there are no IPv6 DNS published, the adapter state is not
sanitized and might contain IPv6 DNS server from a previous session.
netsh_ifconfig_options() clears DNS servers for IPv4 already.
Signed-off-by: Simon Rozman <simon@rozman.si>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20200924064452.1001-2-simon@rozman.si>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21078.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
This is more efficient and less error prone.
Signed-off-by: Simon Rozman <simon@rozman.si>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20200924064452.1001-1-simon@rozman.si>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21076.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--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 6c13e24e57 - 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>
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 0b5141d8f9
- 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>
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:
>REMOTE:openvpn.net,1194,udp
remote ACCEPT
SUCCESS: remote command succeeded
>REMOTE:openvpn.net,1194,udp
remote SKIP
SUCCESS: remote command succeeded
>FATAL:Assertion failed at init.c:504
(c->c1.link_socket_addr.current_remote == NULL)
Fix this behaviour by cleaning stale addrinfo objects.
v2: better comment placement and too long length fix
Signed-off-by: Vladislav Grishenko <themiron@yandex-team.ru>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20200916141755.1923-1-themiron@yandex-team.ru>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21019.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
In commit 0b5141d8f9 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>
