Arne Schwabe [Thu, 8 Apr 2021 12:00:27 +0000 (14:00 +0200)]
Always save/restore pull options
The makes the code path for pull and non-pull more aligned and even
though this might do extra work for non-pull scenarios, saving the
few bytes of memory is not a worthwhile optimisation here.
Additionally with the upcoming P2P mode NCP, the client needs to
save/restore a subset of these options anyway.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210408120029.19438-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22079.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Tue, 6 Apr 2021 16:25:18 +0000 (18:25 +0200)]
Remove OpenSSL configure checks
These checks for the functions take a lot of time in configure call and
also having these checks make it more blurry for which of the supported
OpenSSL versions (and libraries claiming to be OpenSSL) are actually
needed.
Tested with OpenSSL 1.1.1(Ubuntu 20, macOS), 1.0.2 (CentOS7),
1.1.0 (Debian stretch), LibreSSL (OpenBSD 6.8) and wolfSSL
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210406162518.4075-5-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22051.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
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>
Arne Schwabe [Tue, 6 Apr 2021 16:25:16 +0000 (18:25 +0200)]
Remove a number of checks for functions/headers that are always present
For the unlink function we actually have code that just ignores
the unlink call if the unlink function is not present. But all
platforms should have an unlink function.
This also removes all conditionals check for the headers that
belong to the C99 standard library header list
(https://en.cppreference.com/w/c/header).
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210406162518.4075-3-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22053.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Sun, 4 Apr 2021 11:06:02 +0000 (13:06 +0200)]
Remove conditionals compilation for P2MP, ENABLE_SHAPER and TIME_BACKTRACK_PROTECTION
Using OpenVPN without P2MP support (pull, TLS) is unrealistic and
building a binary without it is not something we realistically want
to support anyway. Building P2MP support currently only depended
on HAVE_GETTIMEOFDAY or _WIN32, which has a compat function for it.
So we basically can assume that gettimeofday is always availabe,
either natively or through our compat function.
Remove all the #ifdef P2MP logic, simplify code and reduce maintenance
effort.
This also removes the ENABLE_SHAPER and TIME_BACKTRACK_PROTECTION
defines, which also depended only on the HAVE_GETTIMEOFDAY or _WIN32.
I kept the configure.ac check and ifdef in compat since mingw actually
provides a gettimeofday and we will use that instead of our own compat
function.
Patch V2: Remove dco parts that slipped into the patch, mention the
other removed defines that are always enabled.
Patch V3: Also remove the TIME_BACKTRACK_PROTECTION defines from otime.h
Message-Id: <20210403184626.23067-1-arne@rfc2549.org> Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210404110602.20374-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22030.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Fixes:
tun.c: In function ‘do_ifconfig_ipv4’:
tun.c:1217:17: warning: variable ‘ifconfig_remote_netmask’ set but not
used [-Wunused-but-set-variable]
const char *ifconfig_remote_netmask = NULL;
tun.c:1213:10: warning: unused variable ‘tun’ [-Wunused-variable]
bool tun = is_tun_p2p(tt);
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210403172403.9452-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22019.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
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>
Arne Schwabe [Sat, 3 Apr 2021 12:24:44 +0000 (14:24 +0200)]
Fix async push broken after auth deferred refactor
Commit c5fec838e moved the auth control file related
states into its own struct. Unfortunately I forgot
to also do the part inside #if defined(ENABLE_ASYNC_PUSH)
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210403122444.17090-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22007.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Sat, 3 Apr 2021 12:30:00 +0000 (14:30 +0200)]
log file descriptor in more socket related error messages
This add the fd to the epoll event error message and the x_check_status
message. This helps debugging when thing go wrong with event handling.
Also add logging when ep_del fails to remove a socket from the structure.
In constract to ep_ctl that has this as a FATAL message (M_ERR), we only
log here since the code has been ignoring the status forever there might
be corner cases where a FATAL message could trigger an unintened
regression.
PATCH v2: Fix wrong order of fd,code in printed message.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210403123000.17688-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22008.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Thu, 1 Apr 2021 12:37:51 +0000 (14:37 +0200)]
Remove deprecated option '--keysize'
This option has been deprecated in OpenVPN 2.4 and the ciphers that allow
using this option fall all into the SWEET32 category of ciphers with
64 bit block size.
Patch V2: Remove superflous check in OpenSSL codepath to check keysize
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210401123751.31756-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21943.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Sun, 28 Mar 2021 09:05:30 +0000 (11:05 +0200)]
Deprecate non TLS mode in OpenVPN
The non-TLS mode is a relict from OpenVPN 1.x or 2.0. When TLS mode was
introduced the advantages of TLS over non-TLS were small but TLS mode
evolved to include a lot more features (NCP, multipeer, AEAD ciphers to
name a few).
Today VPN setups that use --secret are mainly used because this mode is
easier to setup and does not require setting up a PKI. This shortcoming
of TLS mode should be addressed now with the peer-fingerprint option.
The primary reason to deprecate --secret is that it is not secure enough
anymore for modern environments. This mode uses a fixed pre-shared key and
no session keys. Thus, no forward secrecy is possible, which means that
any captured VPN traffic can be decrypted later should the --secret key
get into the wrong hands. The cryptography overall used here was okay
when --secret was introduced but is not acceptable by today's standard
anymore.
Finally, modern hardware-accelerated crypto modes like AES-GCM can only
be used in TLS mode (due to IV requirements).
Patch V2: Improve commit message
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210328090530.10653-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21868.html
Arne Schwabe [Fri, 19 Feb 2021 16:52:52 +0000 (17:52 +0100)]
Allow running a default configuration with TLS libraries without BF-CBC
Modern TLS libraries might drop Blowfish by default or distributions
might disable Blowfish in OpenSSL/mbed TLS. We still signal OCC
options with BF-CBC compatible strings. To avoid requiring BF-CBC
for this, special this one usage of BF-CBC enough to avoid a hard
requirement on Blowfish in the default configuration.
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Patch v2: add more clarifying comment, do not warn about OCC only insecure
ciphers, code improvements
Patch V3: Put ciphername resolution via ciper_kt_name in the right branch
Patch V4: Fix cornercase of BF-CBC in data-ciphers not itialising cipher.
Patch v5: I accidently resend v3 as v4. So v5 is just a resend of the real
v4 Acked-by: Antonio Quartulli <antonio@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210219165252.4562-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21577.html
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>
Steffan Karger [Wed, 31 Mar 2021 18:03:23 +0000 (20:03 +0200)]
reliable: retransmit if 3 follow-up ACKs are received
To improve the control channel performance under packet loss conditions,
add a more aggressive retransmit policy similar to what many TCP
implementations do: retransmit a packet if the ACK timeout expires (like
we already do), *or* if three ACKs for follow-up packets are received.
The rationale behind this is that if follow-up packets *are* received, the
connection is apparently functional and we should be able to retransmit
immediately. This significantly improves performance for connections with
low (up to a few percent) packet loss. Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <E1lRfW3-0001sy-VM@sfs-ml-4.v29.lw.sourceforge.com>
URL: https://www.mail-archive.com/search?l=mid&q=E1lRfW3-0001sy-VM@sfs-ml-4.v29.lw.sourceforge.com
Arne Schwabe [Thu, 1 Apr 2021 13:13:37 +0000 (15:13 +0200)]
Remove do_init_socket_2 and do_init_socket_1 wrapper function
These two function basically just pass a number of fields of context to
the linit_socket_init1/2 functions. This wrapper add little to no value
in understanding the code, especially since the linit_socket_init1 will
just copy them to yet another structure.
Remove these wrapper functions and pass context directly to the called
function.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210401131337.3684-15-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21954.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Thu, 1 Apr 2021 13:13:35 +0000 (15:13 +0200)]
Extract multi_assign_peer_id into its own function
This makes multi_get_create_instance_udp a bit shorter and better
structured and also prepares this method to be called from the
mutlti TCP context with DCO which will also need to assign unique peer
ids to instances.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210401131337.3684-13-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21959.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Fri, 2 Apr 2021 13:45:29 +0000 (15:45 +0200)]
Fix 'compress migrate' for 2.2 clients.
Commit 8fa8a17528c001a introduces "compress migrate" to move old clients
that have "compress" or "comp-lzo" in their config towards a connection
without compression. This is done by looking at incoming OCC strings
to see if the client has compression enabled, and at incoming IV_
strings to see whether it can do "compress stub-v2" or needs to be sent
"comp-lzo no".
That check fails for 2.2 clients that do not send *any* peer-info by
default, so the server will not push back any "disable compression"
command. It works if the client connects with "--push-peer-info".
Fix: turn around the order of checks, treat "no peer_info" the same
as "peer_info does not contain IV_COMP_STUBv2".
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20210402134529.27866-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21974.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Wed, 24 Mar 2021 22:08:53 +0000 (23:08 +0100)]
Implement '--compress migrate' to migrate to non-compression setup
This option allow migration to a non compression server config while
still retraining compatibility with client that have a compression
setting in their config.
For existing setups that used to have comp-lzo no or another
compression setting in their configs it is a difficult to migrate to
a setup without compression without replacing all client configs at
once especially if OpenVPN 2.3 or earlier clients are in the mix that
do not support pushing stub-v2. Even with OpenVPN 2.4 and later clients
that support pushing this is not a satisfying solution as the clients
log occ mismatches and the "push stub-v2" needs to be in the server
config "forever".
If the new migrate option to compress is set and a client is detected
that indicates that compression is used (via OCC), the server will
automatically add ``--push compress stub-v2`` to the client specific
configuration if stub-v2 is supported by the client and otherwise
switch to ``comp-lzo no`` and add ``--push comp-lzo`` to the client
specific configuration.
Gert Doering [Thu, 1 Apr 2021 08:29:34 +0000 (10:29 +0200)]
Get rid of last PLUGIN_DEF_AUTH #ifdef
Commit 99d217b200 attempted to get rid of all #ifdef related to
--disable-def-auth but one of them managed to hide. Remove.
The effect of this is that the "openvpn_acf_...tmp" files get not
removed after when an async auth plugin is in use. This is can
get very annoying on a busy server.
Trac: #1186
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210401082934.29922-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21933.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Tõivo Leedjärv [Sun, 28 Mar 2021 17:11:51 +0000 (17:11 +0000)]
Stop using deprecated getpass()
The getpass() function is present in SUSv2, but marked LEGACY. It is
removed in POSIX.1-2001. Additionally, on Solaris getpass() returns
maximum 9 bytes. This will make longer passwords fail with no
possibility for user to know what is happening.
This patch removes usage of getpass() completely and replaces it with
direct implementation of what getpass() does: opens tty (existing code),
outputs the prompt (existing code), turns off echoing (new code), reads
one line (existing code shared with echoed mode), restores tty state
(new code) and closes tty (existing code).
Patch v2: incorporate review feedback, incl. style fixes, merge
termios.h check in configure.ac with an existing
AC_CHECK_HEADERS, add error check and logging after
tcsettattr() when restoring tty settings
Signed-off-by: Tõivo Leedjärv <toivol@gmail.com> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210328171151.12056-1-toivol@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21889.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Selva Nair [Mon, 29 Mar 2021 04:23:18 +0000 (00:23 -0400)]
Remove automatic service
This has been replaced by openvpnserv2 since 2.4.0 and we have
stopped setting up this service in the installer since 2.5.0.
Get rid of the unused code. The mechanics of supporting multiple
services with the same executable is retained for possible future use.
For backwards compatibility, the command line option -instance
is unchanged as "-instance <name> id" although <name>="interactive"
is the only supported value now.
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <1616991798-7179-1-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21890.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Sun, 28 Mar 2021 14:20:38 +0000 (16:20 +0200)]
Remove support for non ISO C99 vararg support
We require ISO C99 as minimum support for our source code and all compilers
should support the ISO C99 macros. Especially gcc does not need
the gcc extensions anymore. Also MSVC has support for it (as defined
in the config-msvc.h but also double checked)
LCLINT seems to be a C analyzer that history has forgotten about. I could
only find https://splint.org/release1.3.html and an similarly old research
paper.
Patch V2: Also remove AX_ macros from configure.ac
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Steffan Karger <steffan.karger@foxcrypto.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210328142038.8826-2-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21883.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Sun, 28 Mar 2021 14:20:37 +0000 (16:20 +0200)]
Remove flexible array member autoconf check
This is configure macro that tries out how to declare a variable array
at the end of struct. This has been standardised in C99, so there is
no more need for non C99 magic. See also this stackoverflow discussion:
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>
Arne Schwabe [Wed, 24 Mar 2021 22:23:30 +0000 (23:23 +0100)]
Use correct types for OpenSSL and Windows APIs
The error code of OpenSSL is a long. On most Unics systems
(mac, Linux...) this happens to be the same as size_t. But on Windows
as LP64, long is a 32 bit type and size_t is a 64 bit type. So use the
same type as OpenSSL.
When calling the Windows API use DWORD for the functions that want a
DWORD.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210324222330.455-4-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21803.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Wed, 24 Mar 2021 22:23:27 +0000 (23:23 +0100)]
Make buffer related function conversion explicit when narrowing
Clang and gcc do report many of the narrowing conversion that MSVC
reports, like these:
warning C4267: 'function': conversion from 'size_t' to 'int', possible
loss of data
This commit changes int to size_t where it is safe
(e.g. checked by buf_size_valid) and add casts where necessary.
In the function buffer_read_from_file the return value of fread is
size_t (at least on Linux/Windows/macOS and cppreference), so fix the
check to actually make sense.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210324222330.455-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21805.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
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>
Arne Schwabe [Wed, 17 Mar 2021 16:00:37 +0000 (17:00 +0100)]
Move NCP saving and restore to the prepush restore code
This unifies save/restoring options that might be changed by a push
from the server. It also removes using the context_1 to store something
that is not related to a SIGHUP lifetime.
Patch v2: rebase on master.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210317160038.25828-2-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21674.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Wed, 17 Mar 2021 16:00:36 +0000 (17:00 +0100)]
Move restoring pre pull options to initialising of c2 context
We currently delay restoring these options until we actually must
restore them. Since there is no reason to do so apart from the very
minor saving to not have to execute that code when a connection fails,
move them it into the general context_2 initialisation.
Patch V2: rebase on master.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210317160038.25828-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21676.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
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>
Arne Schwabe [Mon, 22 Mar 2021 10:21:19 +0000 (11:21 +0100)]
Remove empty dummy functions
These functions seem to have been added to avoid MSVC compiler warnigns.
However nowadays, they trigger compiler warnings from Clang (e.g. when
using --disable-lzo and --disable-lz4):
Arne Schwabe [Mon, 22 Mar 2021 09:16:21 +0000 (10:16 +0100)]
Deprecate the --verify-hash option
Despite trying to figure out with multiple people what the use case for
this option is, we could not come up with a good one. Checking that only
a specific CA is used can be also done by only using that CA in the --ca
directive.
Although it feels a bit strange to deprecate the option after improving
it with peer-fingerprint patches, all the improvements are needed for
--peer-fingerprint and making them specify to --peer-fingerprint would
have added more (unecessary) changes.
Patch v3: rebased on v3 version of other patches.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210322091621.7864-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21779.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Support fingerprint authentication without CA certificate
OpenVPN traditionally works around CAs. However many TLS-based protocols
also
allow an alternative simpler mode in which rather than verify certificates
against CAs, the certificate itself is hashed and compared against a
pre-known set of acceptable hashes. This is usually referred to as
"fingerprint verification". It's popular across SMTP servers, IRC servers,
XMPP servers, and even in the context of HTTP with pinning.
* Allow not specifying the --ca parameter, to specify that
certificates should not be checked against a CA.
I've included some instructions on how to use all of this.
Record our fingerprint in an environment variable for the client to use
later:
$ server_fingerprint="$(openssl x509 -in servercert.pem -noout -sha256
-fingerprint | sed 's/.*=//;s/\(.*\)/\1/')"
Record our fingerprint in an environment variable for the server to use
later:
$ client_fingerprint="$(openssl x509 -in clientcert.pem -noout -sha256
-fingerprint | sed 's/.*=//;s/\(.*\)/\1/')"
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Patch V2: Changes in V2 (by Arne Schwabe):
- Only check peer certificates, not all cert levels, if you need
multiple levels of certificate you should use a real CA
- Use peer-fingerprint instead tls-verify on server side in
example.
- rename variable ca_file_none to verify_hash_no_ca
- do no require --ca none but allow --ca simply
to be absent when --peer-fingprint is present
- adjust warnings/errors messages to also point to
peer-fingerprint as valid verification method.
- Fix mbed TLS version of not requiring CA
not working
Patch v3: Fix minor style. Remove unessary check of verify_hash_no_ca in
ssl.c.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210322091414.7533-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/search?l=mid&q=20210322091414.7533-1-arne@rfc2549.org Signed-off-by: Gert Doering <gert@greenie.muc.de>
wcsncat() was declared unsafe in favour of wcsncat_s(). However, the
string concatenation follows the string length check, making wcsncat()
safe too. Code analysis is just not smart enough (yet) to detect this.
The code was refactored to use wcscat_s() MSVC is considering as "safe".
Signed-off-by: Simon Rozman <simon@rozman.si> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210322074359.527-1-simon@rozman.si>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21774.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Sun, 21 Mar 2021 14:33:53 +0000 (15:33 +0100)]
Implement peer-fingerprint to check fingerprint of peer certificate
This option allows to pin one or more more peer certificates. It also
prepares for doing TLS authentication without a CA and just
self-signed certificates.
Patch V2: Allow peer-fingerprint to be specified multiple times
to allow multiple peers without needing to use inline
syntax. (e.g. on command line).
Patch V3: rebase on v3 of 1/4, reword message of verify-hash and
peer-fingerpring incompatibility
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210321143353.2677-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/search?l=mid&q=20210321143353.2677-1-arne@rfc2549.org Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Sun, 21 Mar 2021 14:25:38 +0000 (15:25 +0100)]
Extend verify-hash to allow multiple hashes
This patch introduces support for verify-hash inlining.
When inlined, this options now allows to specify multiple fingerprints,
one per line.
Since this is a new syntax, there is no backwards compatibility to take
care of, therefore we can drop support for SHA1. Inlined fingerprints
are assumed be to SHA-256 only.
Also print a warning about SHA1 hash being deprecated to verify
certificates as it is not "industry standard" anymore.
Patch v2: fix/clarify various comments, fix a few minor problems, allow
the option to be specified multiple times and have that
added to the list.
Simon Rozman [Sun, 21 Mar 2021 14:46:27 +0000 (15:46 +0100)]
iservice: Resolve MSVC C4996 warnings
Lots of string functions were declared unsafe in favor of ..._s()
counterparts. However, the code already is careful about the buffer
size. Code analysis is just not smart enough (yet) to detect this.
The code was refactored to use ..._s() variants MSVC is considering as
"safe".
Signed-off-by: Simon Rozman <simon@rozman.si> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20210321144627.1621-5-simon@rozman.si>
URL: https://www.mail-archive.com/search?l=mid&q=20210321144627.1621-5-simon@rozman.si Signed-off-by: Gert Doering <gert@greenie.muc.de>
It's about using a standard recommended alias for the wcsdup():
> warning C4996: 'wcsdup': The POSIX name for this item is deprecated.
> Instead, use the ISO C and C++ conformant name: _wcsdup. See online
> help for details.
And the documentation says:
> The Microsoft-implemented POSIX function names strdup and wcsdup are
> deprecated aliases for the _strdup and _wcsdup functions. By default,
> they generate Compiler warning (level 3) C4996. The names are
> deprecated because they don't follow the Standard C rules for
> implementation-specific names. However, the functions are still
> supported.
>
> We recommend you use _strdup and _wcsdup instead. Or, you can continue
> to use these function names, and disable the warning. For more
> information, see Turn off the warning and POSIX function names.
Reference:
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/strdup-wcs
dup Signed-off-by: Simon Rozman <simon@rozman.si> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20210321144627.1621-3-simon@rozman.si>
URL: https://www.mail-archive.com/search?l=mid&q=20210321144627.1621-3-simon@rozman.si Signed-off-by: Gert Doering <gert@greenie.muc.de>
Max Fillinger [Fri, 19 Mar 2021 21:54:48 +0000 (22:54 +0100)]
Wipe Socks5 credentials after use
Plaintext authentication is not exactly high security, but we might as
well memzero the credentials before leaving the function. Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210319215448.38350-1-max@max-fillinger.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21738.html
Arne Schwabe [Fri, 19 Mar 2021 11:46:31 +0000 (12:46 +0100)]
Fix multiple problems when compiling with LLVM/Windows (clang-cl)
When using the LLVM clang compiler instead the MSVC cl.exe but with
the same build environment as MSVC, clang encounters a few errors:
src\openvpn\socket.c(3550,23): warning: assigning to 'CHAR *' (aka 'char
*') from 'uint8_t *' (aka 'unsigned char *') converts between pointers to
integer types with different sign [-Wpointer-sign]
wsabuf[0].buf = BPTR(&sock->reads.buf);
^ ~~~~~~~~~~~~~~~~~~~~~~
src\openvpn\socket.c(3670,23): warning: assigning to 'CHAR *' (aka 'char
*') from 'uint8_t *' (aka 'unsigned char *') converts between pointers to
integer types with different sign [-Wpointer-sign]
wsabuf[0].buf = BPTR(&sock->writes.buf);
^ ~~~~~~~~~~~~~~~~~~~~~~~
Use BSTR instead of BPTR, which casts to the correct type that is
expected.
src\compat\compat-gettimeofday.c(105,18): error: assignment to cast is
illegal, lvalue casts are not supported
tv->tv_sec = (long)last_sec = (long)sec;
Split into two assignments to avoid the illegal cast
include\stdint.h(18,28): error: typedef redefinition with different types
('signed char' vs 'char')
typedef signed char int8_t;
^
openvpn\config-msvc.h(162,16): note: previous definition is here
typedef __int8 int8_t;
Removes our custom int type typdefs from config-msvc.h and replace it
with an include of inttypes.h.
C:\Program Files (x86)\Windows
Kits\10\include\10.0.19041.0\shared\tcpmib.h(56,3): error: typedef
redefinition with different types ('enum MIB_TCP_STATE' vs 'int')
} MIB_TCP_STATE;
^
C:\Users\User\source\repos\openvpn\src\openvpn/syshead.h(369,13): note:
previous definition is here
typedef int MIB_TCP_STATE;
^
1 error generated.
This seems to be for mingw32 only, so guard this with a mingw32
compiler guard.
\src\openvpn\tun.c(3727,34): warning: passing 'char [256]' to parameter of
type 'LPBYTE' (aka 'unsigned char *') converts between pointers to integer
types with different sign [-Wpointer-sign]
net_cfg_instance_id,
^~~~~~~~~~~~~~~~~~~
C:\Program Files (x86)\Windows
Kits\10\include\10.0.19041.0\um\winreg.h(955,88): note: passing argument
to parameter 'lpData' here
This is windows specific code, use the Windows LPBTYE in the
definitions. (long pointer to BYTE (long pointer as far/near pointer
relict from windows 16 bit times, in moddern words (unsigned char *))
Fix also a few other char vs uint8/unisgned char/BYTE issues in tun.c
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210319114631.20459-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21719.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Updates for the wolfSSL README file:
- fix typos
- correct wolfSSL company spelling
- add a point of contact for users having problems using OpenVPN + wolfSSL
Signed-off-by: Juliusz Sosinowicz <juliusz@wolfssl.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210319134322.131905-1-juliusz@wolfssl.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21722.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
build: Add support for pkg-config < 0.28 for old autoconf versions
The PKG_CHECK_VAR() macro is not available on versions of pkgconfig before
0.28, which breaks configure on RHEL-7, Ubuntu 16, and others.
This patch copies the definition generated by newer versions of autoconf
to be used for compatibility with older versions. Tested with automake
1.14.1-2ubuntu1 and autoconf 2.69-6 on Ubuntu 14.
Signed-off-by: Juliusz Sosinowicz <juliusz@wolfssl.com> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20210318181258.89704-1-juliusz@wolfssl.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21708.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
This patch adds support for wolfSSL in OpenVPN. Support is added by using
wolfSSL's OpenSSL compatibility layer. Function calls are left unchanged
and instead the OpenSSL includes point to wolfSSL headers and OpenVPN is
linked against the wolfSSL library. The wolfSSL installation directory is
detected using pkg-config.
As requested by OpenVPN maintainers, this patch does not include
wolfssl/options.h on its own. By defining the macro EXTERNAL_OPTS_OPENVPN
in the configure script wolfSSL will include wolfssl/options.h on its own
(change added in https://github.com/wolfSSL/wolfssl/pull/2825). The patch
adds an option `--disable-wolfssl-options-h` in case the user would like
to supply their own settings file for wolfSSL.
wolfSSL:
Support added in: https://github.com/wolfSSL/wolfssl/pull/2503
```
git clone https://github.com/wolfSSL/wolfssl.git
cd wolfssl
./autogen.sh
./configure --enable-openvpn
make
sudo make install
```
OpenVPN:
```
autoreconf -i -v -f
./configure --with-crypto-library=wolfssl
make
make check
sudo make install
```
Signed-off-by: Juliusz Sosinowicz <juliusz@wolfssl.com> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20210317181153.83716-1-juliusz@wolfssl.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21686.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
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
David Sommerseth [Wed, 17 Mar 2021 22:06:42 +0000 (23:06 +0100)]
build: Remove compat-lz4
Since 2014, the OpenVPN project has shipped an adopted LZ4 library to be
enabled if no LZ4 libraries was found on the system. This was due to
the LZ4 library not being available on all platforms and it was vastly
better than the older LZO compression algorithm. But this was years
before VORACLE and related attack vectors affecting VPN connections,
where compression is considered a vulnerability.
The OpenVPN project is gradually moving away from supporting compression,
so shipping our own LZ4 library is no longer wanted. It will now only
use the LZ4 compression libraries found on the host, and can otherwise
be disabled completely with ./configure --disable-lz4.
Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210317220642.38741-1-openvpn@sf.lists.topphemmelig.net>
URL: https://www.mail-archive.com/search?l=mid&q=20210317220642.38741-1-openvpn@sf.lists.topphemmelig.net Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Wed, 10 Mar 2021 12:48:08 +0000 (13:48 +0100)]
Require at least 100MB of mlock()-able memory if --mlock is used.
If --mlock is used, the amount of memory OpenVPN can use is guarded
by the RLIMIT_MEMLOCK value (see mlockall(2)). The OS default for this
is usually 64 Kbyte, which is enough for OpenVPN to initialize, but
as soon as the first TLS handshake comes it, OpenVPN will crash due
to "ouf of memory", and might even end up in a crash loop.
Steady-state OpenVPN requires between 8 MB and 30-50 MB (servers with
many concurrent clients) of memory. TLS renegotiation with EC keys
requires up to 90 MB of transient memory.
So: with this patch, we check if getrlimit() is available, and if yes,
log the amount of mlock'able memory. If the amount is below 100 MB,
which is an arbitrary value "large enough for most smaller deployments",
we try to increase the limits to 100 MB, and abort if this fails.
v2:
change arbitrary number to 100 MB, introduce #define for it
not only check but also increase with setrlimit()
uncrustify fixes
v3:
OpenSolaris has mlockall() and getrlimit(), but no RLIMIT_MEMLOCK -
make code conditional on HAVE_GETRLIMIT *and* RLIMIT_MEMLOCK
add Changes.rst entry
Trac: #1390
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20210310124808.14741-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21657.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Wed, 10 Mar 2021 10:28:23 +0000 (11:28 +0100)]
Change parameter of send_auth_pending_messages from context to tls_multi
This prepares send_auth_pending_messages to be used a in context that
does not have context c available but also does not need to schedule
an immediate sending of the message (auth plugin/script)
Patch V2: Adjust the comment of reschedule_multi_process to actually fit a
function.
Patch V3: Rebase needed because v3 of 3/11
Patch V4: Send with push.h prototype
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Lev Stipakov <lstipakov@gmail.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210310102823.29508-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/search?l=mid&q=20210310102823.29508-1-arne@rfc2549.org Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 26 Feb 2021 11:10:12 +0000 (12:10 +0100)]
Refactor extract_var_peer_info into standalone function and add ssl_util.c
Our "natural" place for this function would be ssl.c but ssl.c has a lot of
dependencies on all kinds of other compilation units so including ssl.c
into
unit tests is near impossible currently. Instead create a new file
ssl_util.c
that holds small utility functions like this one.
Patch v2: add newline add the end of sll_util.h and ssl_util.c
Patch v3: Refactor/clean up the function even more as suggested by Gert.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Lev Stipakov <lstipakov@gmail.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210226111012.21269-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21585.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Wed, 3 Mar 2021 12:38:18 +0000 (13:38 +0100)]
Implement server side of AUTH_PENDING with extending timeout
Patch V2: eliminate parse_kid function, fix style
Patch V3: adding missing parameter in function, this was added
by a later patch in the original series
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20210303123818.16012-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21596.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 5 Mar 2021 14:13:52 +0000 (15:13 +0100)]
Prefer TLS libraries TLS PRF function, fix OpenVPN in FIPS mode
This moves from using our own copy of the TLS1 PRF function to using
TLS library provided function where possible. This includes currently
OpenSSL 1.1.0+ and mbed TLS 2.18+.
For the libraries where it is not possible to use the library's own
function, we still use our own implementation. mbed TLS will continue
to use our own old PRF function while for OpenSSL we will use a
adapted version from OpenSSL 1.0.2t code. The version allows to be
used in a FIPS enabled environment.
The old OpenSSL and mbed TLS implementation could have shared some
more code but as we will eventually drop support for older TLS
libraries, the separation makes it easier it remove that code
invdidually.
In FIPS mode MD5 is normally forbidden, the TLS1 PRF1 function we
use, makes uses of MD5, which in the past has caused OpenVPN to segfault.
The new implementation for OpenSSL version of our custom implementation
has added the special flags that tell OpenSSL that this specific use
of MD5 is allowed in FIPS mode.
No FIPS conformitiy testing etc has been done, this is only about
allowing OpenVPN on a system where FIPS mode has been enabled system
wide (e.g. on RHEL derivates).
Patch v4: Handle the unlikely case that PRF generation fails. More
formatting
fixes.
Patch v5: v4 with the formatting fixes actually commited. sigh.
Patch v6: More formatting fixes, make OpenSSL fucntion return bool instead
of int.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210305141352.21847-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21612.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Mon, 25 Jan 2021 12:56:21 +0000 (13:56 +0100)]
Introduce management client state for AUTH_PENDING notifications
This allows a UI client to display the correct state. Technically the
client is still waiting for PUSH_REPLY but for every practical concern
this is a different state as we are waiting for the pending
authentication to finish.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20210125125628.30364-5-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21498.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Mon, 25 Jan 2021 12:56:19 +0000 (13:56 +0100)]
Implement client side handling of AUTH_PENDING message
This allows a client to extend the timeout of pull-request response
while waiting for the user to complete a pending authentication. A
timeout of 60s for a normal authentication might still works for a
simple 2FA (but still challenging). With a sophisticated (or overly
complicated) web based authentication 60s are quite short.
To avoid not detecting network problem in this phase, we use the
constant sending of PUSH_REQUEST/AUTH_PENDING as keepalive signal
and still timeout the session after the handshake window time.
patch v2: typo fixes, invert if for sscanf
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20210125125628.30364-3-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21491.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
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
Arne Schwabe [Mon, 25 Jan 2021 12:56:18 +0000 (13:56 +0100)]
Change pull request timeout use a timeout rather than a number
This commit changes the count n_sent_push_requests to time_t based
push_request_timeout. This is more in line to our other timeouts which
are also time based instead of number retries based.
This does not change the behaviour but it prepares allowing to extend
the pull request timeout during a pending authentication. As a user
visible change we print the the time we waited for a timeout instead
Also update the man page to actually document that hand-window controls
this timeout.
Patch V2: grammar fix in manual page
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20210125125628.30364-2-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21490.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
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>
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>
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>
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>