Lev Stipakov [Thu, 17 Jun 2021 06:12:26 +0000 (09:12 +0300)]
crypto_openssl.c: disable explicit initialization on Windows (CVE-2121-3606)
Commit a4071b ("crypto_openssl: add initialization to pick up local
configuration") added openssl initialization to load configuration
file. However on Windows this file is loaded from user-writable
directory, such as c:\etc\ssl for mingw builds and (for example)
c:\vcpkg\packages\openssl_x64-windows\openvpn.cnf for vcpkg
builds. This could be a security risk.
CVE-2121-3606 has been assigned to acknowledge this risk.
Since aforementioned commit implements a niche feature which might
be better solved with CryptoAPI on Windows, make this code conditional
(for now).
CVE: 2121-3606 Signed-off-by: Lev Stipakov <lev@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210617061226.244-1-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22568.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit abd5ee9b7dc4ba85438da5d16bb7dfb31714dac7)
Selva Nair [Wed, 2 Jun 2021 19:47:39 +0000 (15:47 -0400)]
Apply the connect-retry backoff to only one side of a connection
p2p connections with both ends backing off seldom succeed
as their connection attempt durations becomes increasingly
unlikely to overlap when the retry wait time is long.
Avoid this by applying the backoff logic only on TCP clients
or the tls_client side for UDP.
Regression warning: shared secret setups are left out of the
backoff logic.
Trac: #1010, #1384
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210602194739.29488-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22485.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 063d55afeea723fc6df0af29a19df257a8ab6920)
Max Fillinger [Mon, 12 Apr 2021 17:46:17 +0000 (19:46 +0200)]
Fix build with mbedtls w/o SSL renegotiation support
In mbedtls, support for SSL renegotiation can be disabled at
compile-time. However, OpenVPN cannot be built with such a library
because it calls mbedtls_ssl_conf_renegotiation() to disable this
feature at runtime. This function doesn't exist when mbedtls was built
without support for SSL renegotiation.
This commit fixes the build by ifdef'ing out the function call when
mbedtls was built without support for SSL renegotiation.
Signed-off-by: Max Fillinger <maximilian.fillinger@foxcrypto.com> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <E1lW0eX-00012w-9n@sfs-ml-1.v29.lw.sourceforge.com>
URL: https://www.mail-archive.com/search?l=mid&q=E1lW0eX-00012w-9n@sfs-ml-1.v29.lw.sourceforge.com Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit e4bd17c86e01aaf6f809d9ea355419c86c4defdc)
Arne Schwabe [Thu, 20 May 2021 15:11:41 +0000 (17:11 +0200)]
Implement auth-token-user
When not using username and password (i.e. auth-user-pass) it can still
be desirable to provide the client with an auth-token, e.g. for allowing
a session to continue after a reconnect without requiring 2FA again.
However, without --auth-user-pass openvpn does not have a username and will
ignore any pushed auth-token command.
This patch adds support for auth-token-user to set the username that should
be used for auth-token
The spec of using auth-token-user base64-encoded-user are the ones that
OpenVPN3 already implements.
Patch V2: Improve style, fix comments and commit message
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210520151148.2565578-2-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22417.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit b398aa37ca309948b481401adf0074ea5589eb2d)
Arne Schwabe [Tue, 8 Jun 2021 15:24:34 +0000 (17:24 +0200)]
Add github actions
dummy0 gives strange errors on the Ubuntu 16 runner on github actions
because
dummy already exist, so use a more unique ovpn-dummy0 name instead.
Github actions are a good alternative to travis-ci, which futrure is
questionable
at the moment without payment. The github actions also allows building on
macOS
and Windows (not included in this commit). The matrix is a bit different
than Coverity and uses different Ubuntu version with their native OpenSSL
(1.0.2, 1.1.1)/mbed TLS instead of manually compiling different OpenSSL
versions on just Ubuntu 20.04.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20210608152434.1403999-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22511.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit b4f658dfb018dcb22962eaa8b45ddd45c55f3003)
Lev Stipakov [Tue, 15 Jun 2021 10:43:31 +0000 (13:43 +0300)]
vcpkg-ports: restore trailing whitespaces in .patch files
Commit 36b9aa (master) / 73fbeed493c632 (release/2.5)
"contrib/vcpkg-ports: add pkcs11-helper port" has trimmed trailing
white space in .patch files and thus corrupted them.
Restore trailing whitespaces to "uncorrupt" patches.
** applied with git am --whitespace=nowarn **
Signed-off-by: Lev Stipakov <lev@openvpn.net> Acked-By: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210615104331.210-1-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22553.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 742123498253e9a8aa0cbe12752e0e4a09b46f21)
Lev Stipakov [Mon, 7 Jun 2021 10:42:13 +0000 (13:42 +0300)]
contrib/vcpkg-ports: add pkcs11-helper port
pkcs11-helper is a dependency library used by OpenVPN.
So far it has been built only by mingw.
Since we're making MSVC build system a first class citizen,
we need to build depencencies with MSVC, which we do with vcpkg.
All dependencies are in vcpkg official repo, expect pkcs11-helper.
This provides vcpkg port for building pkcs11-helper.
To build for other arch, use x86-windows-ovpn or arm64-windows-ovpn.
Custom triplets are defines so that lz4 is linked statically and
other libraries dynamically.
Since we don't have CMake support yet, we cannot use vcpkg manifest
mode, since it doesn't work with overlay ports
(https://github.com/microsoft/vcpkg/issues/12289),
therefore this one-time manual dependency installation is required.
While on it, bump msvc-generate project target version to VS2019 and
cleanup leftovers from compat.vcxproj.filters.
Signed-off-by: Lev Stipakov <lev@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210514045704.686-1-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22389.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 56f775fe9dacab7804109e305d652057b0c7fd3c)
Matthias Andree [Thu, 3 Jun 2021 12:30:19 +0000 (14:30 +0200)]
Fix SIGSEGV (NULL deref) receiving push "echo"
A server pushing "echo" without arguments can crash the client.
In such a situation, the code in question receives p[1] == NULL
(which was CLEAR(p)'ed above), hands it strncmp, which then
dereferences the null pointer.
Original report and analysis here:
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=256331
Fixes: Trac #1409 Reported-by: peo@nethead.se (to FreeBSD) Signed-off-by: Matthias Andree <matthias.andree@gmx.de> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210603123019.422644-1-matthias.andree@gmx.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22486.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 0033811e0215af76f469d78912c95a2f59813454)
Lev Stipakov [Tue, 11 May 2021 07:57:39 +0000 (10:57 +0300)]
openvpnmsica: properly schedule reboot in the end of installation
A deferred custom action can not set the reboot status via MsiSetMode.
To schedule reboot:
- get user's temp directory in immediate custom action
(EvaluateTUNTAPAdapters) which is called in the beginning of
installation and pass it to the deffered action
- in deferred action (ProcessDeferredAction) create a special empty file
in user's temp directory if reboot is required
- in the end of installation call immediate custom action
(CheckAndScheduleReboot) which checks the existence of special file
and uses MsiSetMode() to schedule reboot
Signed-off-by: Lev Stipakov <lev@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210511075739.158-1-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22335.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 2f0e3fe1c6c57dd3989951d167e94fe4debe5405)
Arne Schwabe [Mon, 5 Apr 2021 22:14:47 +0000 (00:14 +0200)]
Ensure key state is authenticated before sending push reply
This ensures that the key state is authenticated when sending
a push reply.
This bug allows - under very specific circumstances - to trick a
server using delayed authentication (plugin or management) into
returning a PUSH_REPLY before the AUTH_FAILED message, which can
possibly be used to gather information about a VPN setup.
In combination with "--auth-gen-token" or user-specific token auth
solutions it can be possible to get access to a VPN with an
otherwise-invalid account.
CVE-2020-15078 has been assigned to acknowledge this risk.
CVE: 2020-15078 Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <d25ec73f-2ab0-31df-8cb6-7778000f4822@openvpn.net>
URL: non-public, embargoed Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Sat, 27 Mar 2021 18:35:44 +0000 (19:35 +0100)]
Ensure auth-token is only sent on a fully authenticated session
This fixes the problem that if client authentication is deferred, we
send an updated token before the authentication fully finished.
Calling the new ssl_session_fully_authenticated from the two places
that do the state transition to KS_AUTH_TRUE is a bit suboptimal but
a cleaner solution requires more refactoring of the involved methods
and state machines.
This bug allows - under very specific circumstances - to trick a
server using delayed authentication (plugin or management) *and*
"--auth-gen-token" into returning a PUSH_REPLY before the AUTH_FAILED
message, which can possibly be used to gather information about a
VPN setup or even get access to a VPN with an otherwise-invalid account.
CVE-2020-15078 has been assigned to acknowledge this risk.
CVE: 2020-15078 Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <d25ec73f-2ab0-31df-8cb6-7778000f4822@openvpn.net>
URL: non-public, embargoed Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Sat, 27 Mar 2021 17:47:24 +0000 (18:47 +0100)]
Move auth_token_state from multi to key_state
The auth-token check is tied to the username/password that is coming
via a specific SSL session, so keep the state also in the key_state
structure.
This also ensures the auth_token_state is always set to 0 on a new
session since we clear the key_state object at the start of a new
SSL session.
This is a prerequisite patch to fix 2020-15078 in the following two
commits.
2nd patch, squashed into the first one:
This also applies the changes to the auth_token_test.c. The change of
tls_session to a pointer is necessary since before that we had tls_session
not tied to the multi and had two tls_session used in the test. One
implicitly in tls_multi and one explicit one. Merge these to one.
CVE: 2020-15078 Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <d25ec73f-2ab0-31df-8cb6-7778000f4822@openvpn.net>
URL: non-public, embargoed Signed-off-by: Gert Doering <gert@greenie.muc.de>
Max Fillinger [Thu, 15 Apr 2021 09:34:54 +0000 (11:34 +0200)]
Abort if CRL file can't be stat-ed in ssl_init
Now that the path for the CRL file is handled correctly when using
chroot, there's no good reason for the file to be inaccessible during
ssl_init().
This commit ensures that the CRL file is accessed successfully at least
once, which fixes a bug where the mbedtls version of OpenVPN wouldn't
use a reloaded CRL if it initially failed to access the file.
Signed-off-by: Max Fillinger <maximilian.fillinger@foxcrypto.com> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210415093454.18324-1-maximilian.fillinger@foxcrypto.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22118.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 940619c88067d95a1c9865795624bc3822a89bd7)
Max Fillinger [Thu, 15 Apr 2021 09:12:48 +0000 (11:12 +0200)]
In init_ssl, open the correct CRL path pre-chroot
When using the chroot option, the init_ssl function can be called before
entering the chroot or, when OpenVPN receives a SIGHUP, afterwards. This
commit ensures that OpenVPN tries to open the correct path for the CRL
file in either situation.
This commit does not address key and certificate files. For these, the
--persist-key option should be used.
Signed-off-by: Max Fillinger <maximilian.fillinger@foxcrypto.com> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210415091248.18149-1-maximilian.fillinger@foxcrypto.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22117.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 21a0b2494e7f4f1c6325b2972743158acad4f394)
Arne Schwabe [Sun, 28 Mar 2021 12:02:41 +0000 (14:02 +0200)]
Fix condition to generate session keys
When OpenVPN sees a new (SSL) connection via HARD_RESET or SOFT_RESET with
the same port/ip as an existing session, it will give it the slot of the
renegotiation session (TM_UNTRUSTED). And when the authentication
succeeds it will replace the current session. In the case of a SOFT_RESET
this a renegotiation and we will generated data channel keys at the of
key_method_2_write function as key-id > 0.
For a HARD RESET the key-id is 0. Since we already have gone through
connect stages and set context_auth to CAS_SUCCEEDED, we don't
call all the connect stages again, and therefore also never call
multi_client_generate_tls_keys for this session.
This commit changes postponing the key generation to be done only if
the multi_connect has not yet been finished.
Patch V2: Explain better in the commit message why this change is done.
This is "sort of" a backport of commit a005044be9ca, except that the
master commit only got 1 of 3 hunks from the mailing list patch merged
while release/2.5 needs all 3. So this is exactly the patch as it was
sent to the list, URL below.
Trac: #1316
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Antonio Quartulli <antonio@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210328120241.27605-2-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21873.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Sun, 28 Mar 2021 12:02:40 +0000 (14:02 +0200)]
Move context_auth from context_2 to tls_multi and name it multi_state
context_2 and tls_multi have the same life cycle for TLS connections
but so this move does not affect behaviour of the variable.
OpenVPN TLS multi code has a grown a lot more complex and code that
handles multi objects needs to know the state that the object is in.
Since not all code has access to the context_2 struct, the code that
does not have access is often not checking the state directly but
checks other parts of multi that have been affected from a state
change.
This patch also renames it to multi_state as this variable represents
the multi state machine status rather than just the state of the connect
authentication (more upcoming patches will move other states
into this variable).
Patch V2: also rename context_auth to multi_state, explain a bit why this
change is done.
Patch V3: Add comments for c2->multi NULL check forwarding. Fix compile
with ENABLE_ASYNC_PUSH.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Antonio Quartulli <antonio@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210418160111.1494779-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22155.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(backported from commit 0767d5b447044e4cdcfd198058aef1f85f63bbe6)
Fix IPv4 default gateway with multiple route tables
Current default gateway selection for zero destination address just
dumps and parses all the routing tables. If any of non-main table
with default route comes first, wrong default gateway can be picked.
Since adding/removing routes currently handles only main table,
let's stick to RT_TABLE_MAIN while selecting default route too.
v2: keep gateway address unchanged on lookup error
v3: reduce ammout of gateway address copying
Reported-by: Donald Sharp <donaldsharp72@gmail.com> Signed-off-by: Vladislav Grishenko <themiron@yandex-team.ru> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210416120708.1532-1-themiron@yandex-team.ru>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22130.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit c7f95891a4a0aabb64e7d4f3200525c1a2fcf433)
Gert Doering [Fri, 2 Apr 2021 17:34:14 +0000 (19:34 +0200)]
Fix potential NULL ptr crash if compiled with DMALLOC
In the unlikely case that we are compiled with -DDMALLOC *and*
malloc() returns NULL, there is an uncaught memset() which would
crash then. Remove the memset(), as the right the next operation
after check_malloc_return() is a mempcy() which will overwrite
the whole memory block anyway.
Trac: #586
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210402173414.14216-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21981.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit e2acfad40c0d79ce7fd431c380d7466d383bcefa)
Arne Schwabe [Thu, 1 Apr 2021 11:00:03 +0000 (13:00 +0200)]
Always disable TLS renegotiations
Renegotiations have been troublesome in the past and also the recent
OpenSSL security problem (CVE-2021-3449) is only exploitable if
TLS renegotiation is enabled.
mbed TLS disables it by default and says in the documentation:
Warning: It is recommended to always disable renegotation unless you
know you need it and you know what you're doing. In the past, there
have been several issues associated with renegotiation or a poor
understanding of its properties.
TLS renegotiation can be used to restart a session with different
parameters (e.g. now with client certs). This something that OpenVPN does
not use.
For OpenSSL 1.0.2 the workaround to disable renegotiation is rather
cumbersome. So we keep this to 1.1.1 only since 1.0.2 is on its way to
deprecation anyway.
Furthermore because of all these problems, also TLS 1.3 completely
drops support for renegotiations.
Patch V2: Improve comments and commit message
Patch V3: Only disable renegotiation where the SSL_OP_NO_RENEGOTIATION
define is available. LibreSSL, wolfSSL and OpenSSL 1.0.2 are
lacking this macro. Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Antonio Quartulli <antonio@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210401110003.19689-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21939.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 9e702a5d0f1d8ca0443d95ba13fc821deaa81d48)
Arne Schwabe [Fri, 26 Mar 2021 17:57:50 +0000 (18:57 +0100)]
Cleanup print_details and add signature/ED certificate print
This commit cleans up the logic in the function a bit. It also makes it
more clear the the details printed in the second part of the message are
details about the peer certificate and not the TLS connection as such.
Also print the signature algorithm as this might help to identify
peer certificate that still use SHA1.
The new format with for TLS 1.3 and an EC certificate.
Control Channel: TLSv1.3, cipher TLSv1.3 TLS_AES_256_GCM_SHA384, peer
certificate: 384 bit EC, curve secp384r1, signature: ecdsa-with-SHA256
Using the more generic OpenSSL functions also allows use to correctly
print details about ED certificates:
Control Channel: TLSv1.3, cipher TLSv1.3 TLS_AES_256_GCM_SHA384, peer
certificate: 253 bit ED25519, signature: ED25519
Patch v2: Cleanup multiple calls to EVP_PKEY_id, minor code restructuring
Patch v3: Always initialise sig.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210326175750.4772-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21861.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit ca57070630a1b2935ee606cc1309005b56eb925f)
Arne Schwabe [Wed, 17 Mar 2021 16:00:38 +0000 (17:00 +0100)]
Restore also ping related options on a reconnect
This fixes the issue that if a client reconnects the next connection
entries inherits the keepalive settings that were pushed or set by
the previous entry. Since UDP+PULL entries have an implicit 120s
timeout, this timeout also got applied to a TCP session after an
UDP entry.
Patch v2: rebase on master
Reported-By: Jan Just Keijser <janjust@nikhef.nl> Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210317160038.25828-3-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21675.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 5a2ed714d14acb2312d27fe40e300be96d970c27)
Simon Rozman [Mon, 22 Mar 2021 10:39:57 +0000 (11:39 +0100)]
openvpnserv: Cache last error before it is overridden
FormatMessage() sets the last error according to its own success. This
looses the original error code leading to mismatched error message and
error number when sprintfted together resulting in confusing event log
message.
Signed-off-by: Simon Rozman <simon@rozman.si> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210322103957.1234-1-simon@rozman.si>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21789.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 1e938c50930f29124909e120a29fb116d4c46576)
Arne Schwabe [Tue, 16 Mar 2021 12:44:21 +0000 (13:44 +0100)]
Avoid generating unecessary mbed debug messages
The main motivation to make this change is to avoid a crash in mbed TLS
2.25 with --verb < 8.
mbed TLS 2.25 has a nasty bug that the print function for Montgomery style
EC curves (Curve25519 and Curve448) does segfault. See also the issue
reported here: https://github.com/ARMmbed/mbedtls/issues/4208
We request always debug level 3 from mbed TLS but filter out any debug
output of level 3 unless verb 8 or higher is set. This commeit sets
the debug level to 2 to avoid this problem by makeing mbed TLS not
generatin the problematic debug output.
For the affected version to still use --verb 8 with mbed TLS 2.25 is to
restrict the EC groups to ones that do not crash the print function
like with '--tls-groups secp521r1:secp384r1:secp256r1'.
This patch has no patch on user-visible behaviour on unaffected mbed TLS
versions.
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Patch V2: Replace magic constant with proper define. Highlight more this
avoding generating unessary debug output than crash workaround. Acked-by: Gert Doering <gert@greenie.muc.de> Acked-by: Steffan Karger <steffan@karger.me>
Message-Id: <20210316124421.1635-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21667.html
Greg Cox [Mon, 1 Feb 2021 07:09:49 +0000 (07:09 +0000)]
Explain structver usage in sample defer plugin.
sample-plugins/defer/simple.c uses OPENVPN_PLUGINv3_STRUCTVER settings
that may not be obvious to a new author. Add a comment to reduce
possible confusion. Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <1612163389-16421-1-git-send-email-gcox@mozilla.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21540.html
Greg Cox [Wed, 27 Jan 2021 20:21:49 +0000 (20:21 +0000)]
More explicit versioning compatibility in sample-plugins/defer/simple.c
While not required, adding openvpn_plugin_min_version_required_v1 helps
by making an example for others to copy, and helps to explicitly call
attention to the difference between the API version number and the
struct version number in v3 calls. Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <1611778909-20630-2-git-send-email-gcox@mozilla.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21508.html
Greg Cox [Wed, 27 Jan 2021 20:21:48 +0000 (20:21 +0000)]
Update openvpn_plugin_func_v2 to _v3 in sample-plugins/defer/simple.c
This isn't strictly required, but it modernizes the functions used.
This change makes _open the same parameter form as _func (for better
parallelism in function writing) and includes a check for the correct
struct version, as recommended by openvpn-plugin.h Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <1611778909-20630-1-git-send-email-gcox@mozilla.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21507.html
Greg Cox [Sun, 24 Jan 2021 23:46:13 +0000 (23:46 +0000)]
Documentation fixes around openvpn_plugin_func_v3 in openvpn-plugin.h.in
The comments refered to parameters found in openvpn_plugin_func_v2 but not
in v3 Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <1611531973-443-1-git-send-email-gcox@mozilla.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21481.html
Gert Doering [Thu, 21 Jan 2021 17:25:36 +0000 (18:25 +0100)]
clean up / rewrite sample-plugins/defer/simple.c
If we ship something that we consider a form of documentation
"this is how to write an OpenVPN plugin" it should meet our standards
for secure and modern code. This plugin did neither.
- get rid of system() calls, especially those that enabled a
remote-root exploit if this code was used "as is"
- change logging from printf() to OpenVPN's plugin_log()
- this requires changing to openvpn_plugin_open_v3() to get
to the function pointers
- change wacky "background and sleep in the shell call" to the
double-fork/waitpid model we use in plugins/auth-pam
(copy-paste code reuse)
- OpenVPN 2.5 and later react badly to OPENVPN_PLUGIN_FUNC_ERROR
returns to OPENVPN_PLUGIN_ENABLE_PF calls (SIGSEGV crash), so
always return SUCCESS. Only hook ENABLE_PF if that functionality
is actually requested ("setenv test_packet_filter NN").
- change deeply-nested functions auth_user_pass_verify() and
tls_final() to use early-return style
- actually make defered PF setup *work* with recent OpenVPNs
(pre-creating temp files broke this, so unlink() the pre-created
file in the ENABLE_PF hook, and re-create asyncronously later)
- add lots of comments explaining why we do things this way
Security issue reported by "oxr463" on HackerOne.
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20210121172536.32500-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21466.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 452e016cba977cb1c109e74977029b9c0de33de2)
Gert Doering [Thu, 21 Jan 2021 13:39:29 +0000 (14:39 +0100)]
Make OPENVPN_PLUGIN_ENABLE_PF failures FATAL
Without this patch, if openpn is using a plugin that provides
OPENVPN_PLUGIN_ENABLE_PF but then fails (returns OPENVPN_PLUGIN_FUNC_ERROR),
OpenVPN will crash on a NULL pointer reference.
The underlying cause is (likely) the refactoring work regarding
CAS_SUCCEEDED etc., and that nobody adjusted the pf.c code accordingly
(it tries to sent itself a SIGUSR1, which tries to tear down the
client MI instance, but since it is not fully set up yet at this
point, things explode). Full details on the call chain in Trac...
Since we intend to remove pf in 2.6, but we still do not want OpenVPN
to ever SIGSEGV, change the requirements for the plugins to "MUST SUCCEED",
so if the plugin ENABLE_PF call fails, abort openvpn with a M_FATAL
message.
Trac: #1377
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20210121133929.20186-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21464.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 6a0c51baaa4d2b329183601ec35d3d16f127519e)
Gert Doering [Mon, 18 Jan 2021 16:28:50 +0000 (17:28 +0100)]
Document common uses of 'echo' directive, re-enable logging for 'echo'.
The 'echo' command can be used to signal information to an OpenVPN
GUI driving the openvpn core via management interface. Which commands
exists and their syntax has so far been mostly undocumented.
Condense the long and good discussion between Selva Nair and
Jonathan K. Bullard into doc/gui-notes.txt (initial draft from
Jonathan, comments from Selva and Arne), with a pointer added
to doc/management-notes.txt.
Domagoj Pensa [Tue, 15 Dec 2020 17:30:04 +0000 (18:30 +0100)]
Skip DHCP renew with Wintun adapter
Wintun does not support DHCP.
Running DHCP renew with Wintun adapter fails with a logged warning.
Fixed so that DHCP renewing is called only for TAP-Windows6 adapters. Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20201215173004.26170-1-domagoj@pensa.hr>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21364.html
Domagoj Pensa [Thu, 24 Dec 2020 11:59:10 +0000 (12:59 +0100)]
Remove 1 second delay before running netsh
When running various netsh commands before each 1 second sleep is added.
As more netsh commands are run, especially for Wintun adapters, that can
add to a noticable delayed connecting time.
This should be safe. No problems were found in tests and all netsh
commands executed properly with delay removed. Also, no delays are used
in a similar code in interactive service and netsh command executions
are guarded with a semaphore.
Instead of removing management_sleep(1), management_sleep(0) is used as
a replacement to allow processing any pending actions on the management
interface without any wait.
Signed-off-by: Domagoj Pensa <domagoj@pensa.hr> Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20201224115910.10129-1-domagoj@pensa.hr>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21405.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit b1a8213ee3fe35a4617608ec7653e4dffea79207)
Gert Doering [Fri, 25 Dec 2020 16:42:14 +0000 (17:42 +0100)]
Clarify --block-ipv6 intent and direction.
--block-ipv6 is a fairly special-purpose option, and only blocks packet
in the client->server direction. This is implied by not ever mentioning
the other direction in the existing documentation, but not written down.
Make this explicit, avoid confusion.
Also, point why this option exist (avoid IPv6 leakage from dual-stacked
clients around IPv4-only VPN offerings).
Trac: #1351
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Richard Bonhomme <tincanteksup@gmail.com>
Message-Id: <20201225164214.22771-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21407.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 8a8ee283aa7a4b409a9dafc082a6c65b5539308b)
Domagoj Pensa [Tue, 15 Dec 2020 17:16:00 +0000 (18:16 +0100)]
Fix too early argv freeing when registering DNS
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>
(cherry picked from commit ab4688e3bd78d010ccc96adec66ab552bd009328)
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>
(cherry picked from commit a686f7e29af012783371f401f394ac1e62e5b75f)
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>
(cherry picked from commit 4d307ed431bf18d554f524ebaf111f5e136147fe)
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>
(cherry picked from commit fb169c3b8fdfa9792c0eee8441956f062dfd7982)
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>
(cherry picked from commit dfd624b52bce7ddd0eeaab516df9848e432f3242)
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>
(cherry picked from commit fb789947ab1eba3e68fb8e4b3551d095a53962bd)
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>
(cherry picked from commit fc25ca3a7cf720fbb53889fdba6ac0154c7c9c1a)
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>
(cherry picked from commit a4eeef17b20541a7afde0f1cbeae4a4e2b0c455a)
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>
(cherry picked from commit c018fc00be25aee5921d234531f87753a3a7aec7)
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>
(cherry picked from commit 23e11e591347080efa3b933beca7f620dd059d5c)
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>
(cherry picked from commit b68aa00603332357e6c711e91c5b4ba04d78294b)
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>
(cherry picked from commit a5409c0d34bf02cacdee61d61ba7b3e1f72e132f)
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>
(cherry picked from commit 0f44a9080530df70410106c244e9efc7f2d8a802)
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>
(cherry picked from commit e9e47f498674f6db8c3b88b32c877c5beb09a888)
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>
(cherry picked from commit 70882f3e40df1c70c553b8c22c747b468d5a0dc7)
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>
(cherry picked from commit 7f7b05395c3eb634e198d12e212360958c3ca8fb)
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>
(cherry picked from commit 860a7bc77ef515f1d042a2860f7e2bd9980e19be)
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>
(cherry picked from commit 94cebf8261d20a55b0260cce61ad892a98bc24d8)
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>
(cherry picked from commit 4dff236811a1ec9c97a27ad93182ad4beb12377f)
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>
(cherry picked from commit 6345cea86bde14b22a8024cad5026a7d302db25c)
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>
(cherry picked from commit 0b5141d8f946a274bf27b3592ac07dc9c6b0ee71)
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>
(cherry picked from commit b8625abbd5be21a810b648058e6e411a7ff19702)
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>
(cherry picked from commit 97ff64360745d27004e97043507ac788db23799c)
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>
(cherry picked from commit eebeaa02367d247fc2549df3edf8e598c58c3572)
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>
(cherry picked from commit 37aab49b083a9e385970e3ab2dd727ea1a95ff35)
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>
(cherry picked from commit a61c08a2c80d95dcc2bc30ddcb9a54a462e565ed)
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>
(cherry picked from commit 8120e1ad4f20d4ec1fa9e191f3b15740c4aec448)
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>
(cherry picked from commit 50c7700da09a1f83474e18f8709d59dbc4b509e2)
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>
(cherry picked from commit aa34684972eb01bfa5c355d1c8a8a9d384bf0175)
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>
(cherry picked from commit 505d5ad8fadcdc56bae07f4b95c05acd93a47c24)
projects / thirdparty / openvpn.git / log
