options.c: fix format security error when compiling without optimization
error: format not a string literal and no format arguments
[-Werror=format-security]
2309 | msg(M_USAGE, str);
Found by accident, since it only happens without optimization.
Seems the compiler can figure out that this is harmless when
thinking a bit harder about it. Fix anyway.
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20221228110752.34060-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25848.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Tue, 27 Dec 2022 20:26:14 +0000 (21:26 +0100)]
bandaid fix for TCP multipoint server crash with Linux-DCO
TCP multipoint servers with Linux-DCO can crash under yet-unknown
circumstances where a TCP socket gets handed to the kernel (= userland
shall not acceess it again) but the socket still lands in the event
polling mechanism, and is passed to link_socket_read() with
sock->fd being "-1" (SOCKET_UNDEFINED).
This is a bug, but it happens very unfrequently so not fixed yet.
When this happens, the server gets stuck in an endless loop of
"trying recvfrom(-1, ..), getting an error, looging that error,
continue" until the server's disk is full.
The situation is being made a bit more complex by the dco-win
approach of treating "all kernel sockets as UDP", so the Linux
implementation tries to access the -1 socket as UDP, confusing
the picture more.
As a bandaid to avoid the crash, this patch changes
- socket.h: only do the "if dco_installed, treat as UDP" for WIN32
(link_socket_read())
- socket.c: add ASSERT(sock->fd >= 0); checks to all UDP socket paths
(we should never even hit those as this is a TCP specific problem,
but in the "sock->fd = -1" case, doing a clean server abort is
preferred to "the disk is full with non-helpful logfiles, and then
the server crashes anyway")
- socket.c: in the TCP read function, link_socket_read_tcp(),
check for sock->fd < 0 and trigger "sock->stream_reset = true"
(+ write to the log what happened).
This change will kill this particular TCP client instance (SIGTERM),
but leave the rest of the server running fine - and given that
in our tests this issue seems to be triggered by inbound TCP RST
in just the wrong moment, it seems to be "a properly-sized bandaid".
v2: rebase on top of "move dco_installed back to link_socket"
v3: move sock->fd check inside !residual_fully_formed clause (so
we can still handle already-read packets)
Github: OpenVPN/openvpn#190
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20221227202614.2114971-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25844.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Tue, 27 Dec 2022 14:02:45 +0000 (15:02 +0100)]
Replace realloc with new gc_realloc function
The realloc logic has the problem that it relies on the memory being
deallocated by uninit_options rather than by freeing the gc. This
does not always happen in all code path. Especially the crypto selftest
run by make check will not call uninit_options.
This introduces a gc_realloc function that ensures that the pointer is
instead freed when gc_free is called.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221227140249.3524943-2-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25829.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Allow skipping multple remotes via management interface
The mamangement command "remote SKIP" is extended with an
optional parameter 'count' > 0. If count is greater than
number of connection entries (len), count % len is used.
On going past the index of the last connection entry,
counting is restarted from the first connection entry.
Without this, use of management-query-remote from a UI is
virtually impractical except when there are only a handful
of remote entries. Skipping the entries one by one takes
a long time when there are many entries to be skipped
(~ 1 second per entry). Use of "remote MOD" is not an
option as change of protocol is not supported.
Management clients can determine the availability of this
feature by checking that the management interface version
is > 3. Older versions will ignore the count parameter and
behave identically to using count = 1.
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20210907223614.8574-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22817.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Use a template for 'unsupported management commands' error
The message
"ERROR: The 'foo' commmand is not supported by current daemon mode"
is repeatedly used in manage.c. Move it to a function for uniformity
in messaging.
v3, v3: no change Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20210907223126.8440-3-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22814.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Currently we allow a max of 64 connection entries and remotes.
A larger number would allow users with 100's of independent
config files for different end points of same provider to
consolidate them to connection entries.
v2,v3: no change
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20210907223126.8440-2-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22816.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Add remote-count and remote-entry query via management
Selecting the remote host via the management interface
(management-query-remote) provides a restrictive user
experience as there is no easy way to tabulate all available
remote entries and show a list to the user to choose from.
Fix that.
Two new commands for querying the management interface are added:
(i) remote-entry-count : returns the number of remotes specified
in the config file. Example result:
10
END
(ii) remote-entry-get i [j]: returns the remote entry at index i
in the form index,host,port,protocol. Or, if j is present
all entries from index i to j-1 are returned, one per line.
Example result for i = 2:
2,ovpn.example.com,1194,udp
END
Example result for i = 2, j = 4
2,ovpn.example.com,1194,udp
3,ovpn.example.com,443,tcp-client
END
remote-entry-get all: returns all remote entries.
v2: use independent callback functions for the two commands
v3: return results as 0 or more lines terminated by END, as done
for all other similar commands. v1 was fashioned after
pkcs11-id-count and pkcs11-id-get which uses a format not
consistent with the rest of the management commands.
See also management-notes.txt
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20210907223126.8440-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22815.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Sat, 24 Dec 2022 19:42:50 +0000 (20:42 +0100)]
Do not set nl socket buffer size
libnl increases the sizes we pass to 8192 anyway. Currently when we have
a lot of events queued we might run into a NLE_NOMEM message and that
terminates the server. So rather let the kernel decide the buffer sizes.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221224194253.3202231-7-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25789.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Sat, 24 Dec 2022 19:42:47 +0000 (20:42 +0100)]
Move dco_installed back to link_socket from link_socket.info.actual
this change was done in order to be able to differentiate when needing to
use dco and when to use normal socket sendto. Since we want to eventually
completely use the userspace sockets for sending/receiving, we just switch
to always use UDP sendto even if the socket is already installed in the
kernel.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221224194253.3202231-4-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25792.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Sat, 24 Dec 2022 19:42:45 +0000 (20:42 +0100)]
Rename TM_UNTRUSTED to TM_INITIAL, always start session in TM_INITIAL rather than TM_ACTIVE or TM_INITIAL
Currently we start new session in TM_ACTIVE or TM_INITIAL depending if
we already have an active session in TM_ACTIVE or not.
With this change, all session will be started in TM_INITIAL both initiated
by a peer but also session by ourselves. This simplifies state transitions
and eliminates the wacky state transition that when we have a failed
reneogitiation (and move TM_ACTIVE to TM_LAME_DUCK) that a new session of
a peer starts in TM_ACTIVE rather than TM_INITIAL
This is a squash of two mailing list patches:
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221224194253.3202231-2-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25798.html Signed-off-by: Gert Doering <gert@greenie.muc.de> Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221224194253.3202231-3-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25795.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Tue, 20 Dec 2022 14:04:58 +0000 (15:04 +0100)]
Make management password check constant time
This changes the password check on the management interface to be constant
time. Normally the management port should not be exposed in a way that
allows an attacker to even interact with it but making the check constant
time as an additional layer of security is always good.
Patch v2: include NUL byte in comparison
Reported-by: Connor Edwards <cedw@pm.me> Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221220140458.2666637-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25784.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Selva Nair [Mon, 19 Dec 2022 14:04:05 +0000 (09:04 -0500)]
Do not include auth-token in pulled option digest
As change in auth-token is common on restart and does not
require tun-reopen, exclude it from the "pulled options digest"
calculation. Without this tun is always re-opened on SIGUSR1
if auth-token is in use which breaks persist-tun.
Github: Fixes OpenVPN/openvpn#200
v2: explcitly filter auth-token and auth-token-user
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221219140405.1221341-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25768.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Mon, 19 Dec 2022 17:21:41 +0000 (18:21 +0100)]
Use include "buffer.h" instead of include <buffer.h>
My own non-standard cmake based build system found this one. But
even if this is not a problem with the normal autoconf based system
we should still be consistent.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221219172141.2565798-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25777.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Thu, 15 Dec 2022 19:01:43 +0000 (20:01 +0100)]
Deprecate NTLMv1 proxy auth method.
NTLMv1 is ancient and not considered secure anymore and we are not
aware of any users or software still requiring this feature.
Additionally it currently depends on our "doing single DES using
3DES" workaround for OpenSSL (cipher_des_encrypt_ecb). So removing
NTLMv1 will also allow us to remove that workaround.
Reported-By: Trial of Bits (TOB-OVPN-7) Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221215190143.2107896-9-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25731.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Thu, 15 Dec 2022 19:01:42 +0000 (20:01 +0100)]
Fix corner case that might lead to leaked file descriptor
Reported-By: Trail of Bits Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221215190143.2107896-8-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25730.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Thu, 15 Dec 2022 19:01:41 +0000 (20:01 +0100)]
Remove unused gc_arena
Reported-By: Trail of Bits Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221215190143.2107896-7-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25736.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Thu, 15 Dec 2022 19:01:40 +0000 (20:01 +0100)]
Eliminate or comment empty blocks and switch fallthrough
These empty blocks are intentional but trigger code checkers and
were pointed out by Trail of Bits in the security audits. Add comments
to them or eliminate them whatever makes more sense.
For fallthrough C23 [1] has a standard way to signal that but we not
adding a C23 feature to our codebase, so use a comment for now.
Arne Schwabe [Thu, 15 Dec 2022 19:01:38 +0000 (20:01 +0100)]
Ensure that argument to parse_line has always space for final sentinel
This fixes two places were we do not have enough space in the array
of parameters given to parse_line for the final NULL parameter that
signal the end of the parsed argument errors.
Both these cases can lead to a buffer overflow. But both of these
cases require root/admin access to OpenVPN:
- parse_argv, only able to trigger if starting openvpn from the command
line, at this point you cannot gain more privileges than you already
have.
Way to reproduce, compile with ASAN and run:
openvpn --tls-verify a a a a a a a a a a a a a a a
- remove_iroutes_from_push_route_list
This operates on the list of pushed entries that is generated
by the server itself. So trigger this, you need to have control
over config, management interface, a plugin or cdd files.
The parse_argv problem was found by Trial of Bits. I found the
remove_iroutes_from_push_route_list problem by looking for similar
problems.
Reported-By: Trial of Bits (TOB-OVPN-4) Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221215190143.2107896-4-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25734.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
David Sommerseth [Thu, 15 Dec 2022 19:01:37 +0000 (20:01 +0100)]
ssl_verify: Fix memleak if creating deferred auth control files fails
If the key_state_gen_auth_control_files() call fails, the code would
just return without freeing the argv container. Instead the code should
jump to an appropriate exit point where memory is being released.
Also adjust the related comment, to indicate that these deferred auth
control files are really pre-created.
Signed-off-by: David Sommerseth <davids@openvpn.net> Reported-by: Trail of Bits (TOB-OVPN-2) Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221215190143.2107896-3-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25737.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Lev Stipakov [Wed, 14 Dec 2022 22:42:20 +0000 (00:42 +0200)]
management: add timer to output BYTECOUNT
BYTECOUNT on management interface is used to display client stats,
for example by openvpn-gui. At the moment BYTECOUNT is sent if
there is a traffic. With DCO, userspace process doesn't see data
channel traffic, BYTECOUNT is not sent and therefore stats
are not updated.
Fix displaying DCO client stats by adding a timer, which is triggerd
every n seconds, where n is set by existing management command
bytecount <n>. Output stats, taking into account stats from DCO,
when timer is triggered.
While on it, simplify bytecount routines call chains - inlining
functions which are used only once.
DCO stats fetching is not yet implemented.
Stats for the server mode (BYTECOUNT_CLI) are unaffected
by this change - to output those in timer callback we would need to
enumerate all peers, and I am not sure we want to output stats
for all peers every <n> seconds.
Signed-off-by: Lev Stipakov <lev@openvpn.net> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20221214224220.307-1-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25707.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Marc Becker [Sun, 11 Dec 2022 20:01:08 +0000 (21:01 +0100)]
special handling for PKCS11 providers on win32
Change win32 dynamic loader behavior when supplying an absolute path.
The DLL location is considered/preferred to resolve dependencies.
Support in pkcs11-helper for loader flag is detected at compile time.
3rd party DLLs and additional dependencies do no longer need to be moved
to the OpenVPN directory or require changes to %PATH% configuration.
Signed-off-by: Marc Becker <marc.becker@astos.de> Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20221211200108.1402-1-marc.becker@astos.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25646.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Marc Becker [Sun, 11 Dec 2022 19:14:03 +0000 (20:14 +0100)]
use new pkcs11-helper interface to add providers
The new interface in pkcs11-helper 1.28 allows decoupling of provider
registration and initialization.
This allows modifying more (and future) properties apart from the
6 fixed ones supported as arguments to pkcs11h_addProvider().
With the new interface it is easier to see (from a code perspective)
which option is set to which value.
It's also not necessary to supply values for built-in defaults:
- slot_event_method=PKCS11H_SLOTEVENT_METHOD_AUTO
- slot_poll_interval=0
Signed-off-by: Marc Becker <marc.becker@astos.de> Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20221211191403.805-1-marc.becker@astos.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25643.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Max Fillinger [Wed, 14 Dec 2022 15:34:14 +0000 (16:34 +0100)]
Fix message for too long tls-crypt-v2 metadata
The current code only checks if the base64-encoded metadata is at most
980 characters. However, that can encode up to 735 bytes of data, while
only up to 733 bytes are allowed. When passing 734 or 735 bytes, openvpn
prints a misleading error message saying that the base64 cannot be
decoded.
This patch checks the decoded length to show an accurate error message.
v2: Remove now-unused macro and fix an off-by-one error.
Signed-off-by: Max Fillinger <maximilian.fillinger@foxcrypto.com> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20221214153414.12671-1-maximilian.fillinger@foxcrypto.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25694.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Lev Stipakov [Wed, 14 Dec 2022 13:28:35 +0000 (15:28 +0200)]
Rename dco_get_peer_stats to dco_get_peer_stats_multi
Existing API and implementation (FreeBSD only) are designed for
server usage. Rename it to *_multi to indicate that and not to mix
with upcoming client API/implementation.
Signed-off-by: Lev Stipakov <lev@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221214132835.1010-1-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25690.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Kristof Provost [Mon, 5 Dec 2022 16:41:02 +0000 (17:41 +0100)]
Read the peer deletion reason from the kernel
Recent FreeBSD kernels supply a reason for the OVPN_NOTIF_DEL_PEER
notification. Parse this from the nvlist so we can distinguish
user-requested removals from timeouts.
Signed-off-by: Kristof Provost <kprovost@netgate.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221205164103.9190-4-kprovost@netgate.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25617.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Kristof Provost [Mon, 5 Dec 2022 16:41:01 +0000 (17:41 +0100)]
dco: Update counters when a client disconnects
When the kernel module (Linux or FreeBSD) notifies us that a peer has
disconnected we'd like to get a final count of the in/out bytes for that
peer.
We can't request that information any more, because the kernel has
already removed the peer at that point.
Have the kernel send that information as part of the "delete peer"
notification, and update the counters a final time.
This implements the FreeBSD-specific DCO code, but not the
Linux-specific code. It will simply add 0 to the count on Linux.
Signed-off-by: Kristof Provost <kprovost@netgate.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221205164103.9190-3-kprovost@netgate.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25614.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Tue, 13 Dec 2022 22:54:30 +0000 (23:54 +0100)]
Set DCO_NOT_INSTALLED also for keys not in the get_key_scan range
We have 6 key slots but normally only consider 3 of them to be
active/valid keys. Especially the secondary key of TM_LAME_DUCK can
in rare corner cases have a key that is still installed in the kernel.
While this should not cause any issues since I do not see way for this
key to become active ever again, it is better to keep the state correctly.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20221213225430.1892940-3-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25681.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Tue, 13 Dec 2022 22:54:29 +0000 (23:54 +0100)]
Trigger a USR1 if dco_update_keys fails
When dco_update_keys fails, we are in some weird state that we are
unlikely to recover since what userspace and kernel space think of
the keys is very likely to not in sync anymore. So abandon the
connection if this happens.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20221213225430.1892940-2-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25679.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Kristof Provost [Mon, 5 Dec 2022 16:41:00 +0000 (17:41 +0100)]
Read DCO traffic stats from the kernel
When DCO is active userspace doesn't see all of the traffic, so when we
access these stats we must update them.
Retrieve kernel statistics every time we access the
link_(read|write)_bytes values.
Introduce a dco_(read|write)_bytes so that we don't clobber the existing
statistics, which still count control packets, sent or received directly
through the socket.
Signed-off-by: Kristof Provost <kprovost@netgate.com> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20221205164103.9190-2-kprovost@netgate.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25618.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Thu, 8 Dec 2022 15:31:29 +0000 (16:31 +0100)]
Ignore connection attempts while server is shutting down
Currently we still allow clients to connect while the server is waiting
to shut down. This window is very small (2s) and is only used when
explicit-exit-notify is enabled on the server side.
The chance of a client connecting during this time period is very low
unless someone puts something stupid like --connect-retry 1 3 into his/her
client config and forces the client to reconnect during this time period.
Github: OpenVPN/openvpn#189
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221208153129.1207228-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25638.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Max Fillinger [Sat, 26 Nov 2022 16:26:47 +0000 (17:26 +0100)]
Correct tls-crypt-v2 metadata length in man page
The manual page claims that the client metadata can be up to 735 bytes
(encoded as upt to 980 characters base64), but the actual maximum length
is 733 bytes which is also encoded as 980 characters in base64.
Signed-off-by: Max Fillinger <maximilian.fillinger@foxcrypto.com> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20221126162648.150678-1-maximilian.fillinger@foxcrypto.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25546.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
So Linux basically has a 16 bit uint16 instead of two uint8_t. Because
s390x is big endian, this happens to be same in memory layout as on all
BSDs with first byte being 0 and second byte being the family.
Introduce a second array to check against, if we are on little endian
Linux.
This is a bit fragile but this is also just a unit test.
This also fixes compiling test_pkt with windows.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221207140259.1083577-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25633.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
P2P mode with pre-shared key is deprecated, unsecure and should NOT be
used. This said we still carry it around for a bit and we have to make
sure it does not fight with DCO.
Disable DCO at all when --secret is specified.
Github: OpenVPN/openvpn#188
Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221207100201.6467-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25629.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Tue, 6 Dec 2022 13:36:47 +0000 (14:36 +0100)]
Fix connection cookie not including address and fix endianness in test
We accidentially checked the adress family size instead of the address
family.
For unit test checks we need to consider endianess to ensure the hmac
for the adress is always the same. The real code does not care about
endian since it only needs it to be same on the same architecture.
Converting the session to endianess is strictly speaking unecessary
for the actual function of the function but is almost no overhead
and makes the unit testing more robust.
Reported by David trying to the package on Red Hat/s390x and painfully
debugged by setting up a s390x qemu machine that takes 40s just to
run ./configure.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221206133647.954724-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25619.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Thu, 1 Dec 2022 11:01:28 +0000 (12:01 +0100)]
Allow reconnecting in p2p mode work under FreeBSD
This commit consists of two parts.
- explicitly removing an existing peer in p2p mode
- ignoring the ping timeout notification that is generated by the first
part
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221201110128.271064-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25602.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Wed, 30 Nov 2022 16:57:12 +0000 (17:57 +0100)]
Signal USR1 when connection initialising fails
When we fail initialisation the connection (e.g. P2P cipher NCP), we have
a non-working connection. Even though previous version would then stay in
this state, it does not really make sense to be in this state until the
keepalive timeout expires and triggers a USR1 anyway.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221130165712.159683-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25596.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Wed, 30 Nov 2022 16:57:05 +0000 (17:57 +0100)]
Introduce connection state for reconnecting peer in p2p
We introduce this state to make the reconnecting of a client more
obvious and what is called again instead of making it implicit. The
new state CAS_RECONNECT_PENDING is between CAS_WAITING_OPTIONS_IMPORT and
CAS_CONNECT_DONE as we need to redo some of the steps of the connection
setup, so this new state is going a "half step" back in the state machine.
We also do no longer generate data channel keys for untrusted session. This
is done for clarity but also to allow them being generated after the
session has become actually active.
These changes allow a reconnect in p2p mode with DCO to work as the initial
reconnect working.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221130165705.159610-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25595.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Tue, 29 Nov 2022 11:30:31 +0000 (12:30 +0100)]
Add section about common error with OpenVPN 2.6 and OpenSSL 3.0
We expect a number of configurations to no longer work with OpenVPN
2.6 and OpenSSL 3.0. This section tries to explain the most common
errors that will come up and how to work around them.
Patch V2: several mistakes highlighed and suggestions made by Frank
included.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20221129113031.3735598-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25571.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Selva Nair [Wed, 30 Nov 2022 10:55:02 +0000 (05:55 -0500)]
pull-filter: ignore leading "spaces" in option names
It seems sometimes comma-separated pulled options have
an offending leading space. Not sure whether that is an error,
but the change here matches the behaviour of option parsing.
v2: fix typo in commit message
v3: space() --> isspace()
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221130105502.662374-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25582.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Mon, 28 Nov 2022 11:16:42 +0000 (12:16 +0100)]
Update PORTS
Instead of fully removing PORTS, keep "this is what you want to do for
porting OpenVPN to a new platform" section, and update the PLATFORMS
part to better reflect current status.
v2:
drop "2.2+" from Linux, and name the fruitish thing "macOS"
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20221128111642.3483-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25558.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Sun, 27 Nov 2022 09:07:42 +0000 (10:07 +0100)]
Use dedicated multi->dco_peer_id for DCO instead of multi->peer_id
The lifetime and state machine of multi->peer_id does not exactly the
lifetime/state of DCO. This is especially for p2p NCP where a reconnection
can change the peer id. Also use this new field with value -1 to mean
not installed, replacing the dco_peer_added field.
Also ensure that we have a failure adding a new peer, we don't try to
set options for that peer or generating keys for it.
Patch v2: fix one comparison checking for 0 instead of -1
Patch v3: make recovery after failing dco_add_peer more robust
and the comparison that lead to not deleting a peer.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221127090742.3487997-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/search?l=mid&q=20221127090742.3487997-1-arne@rfc2549.org Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Thu, 24 Nov 2022 16:26:42 +0000 (17:26 +0100)]
Move dco_installed from sock->info to sock->info.lsa.actual
For tcp this makes no difference as the remote address of the
socket never changes. For udp this allows OpenVPN to differentiate
if a reconnecting client is using the same address as before or
from a different one. This allow sending via the normal userspace
socket in that case.
Patch v2: fix windows code path
Patch v3: fix mtcp server code path
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <20221124162642.3173118-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/search?l=mid&q=20221124162642.3173118-1-arne@rfc2549.org Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Tue, 15 Nov 2022 12:29:40 +0000 (13:29 +0100)]
Fix logic error in checking early negotiation support check
We want to check if EARLY_NEG_START is set and reserve the other bits
for future expansions. Right now we also check if all reserved bits are
zero. oops.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221115122940.1947284-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25519.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Wed, 9 Nov 2022 15:48:09 +0000 (16:48 +0100)]
Allow tun-mtu to be pushed
This allows tun-mtu to pushed but only up to the size of the preallocated
buffers. This is not a perfect solution but should allow most of the use
cases where the mtu is close enough to 1500 (or smaller).
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Patch v4: rebase for check_session_cipher name change
Patch v5: remove mention of change of default mtu, remove leftover code
from an earlier approach.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221109154810.1268403-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25498.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
There is no way to detect whether this information
is outdated in nmake itself. So leave it up to the
Python script to decide.
While here, change some leading whitespace to tabs as
expected in Makefile.
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Signed-off-by: Lev Stipakov <lev@openvpn.net> Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20221111121212.25167-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25508.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Tue, 8 Nov 2022 13:45:23 +0000 (14:45 +0100)]
Improve documentation for --dev and --dev-node.
During the research for commit a5cf4cfb77f745 it turned out that
OpenVPN's behaviour regarding "--dev arbitrary-name" is very
platform-specific and not very well documented.
The referenced commit fixed DCO behaviour to be in line with non-DCO
linux behaviour, this commit catches up on the documentation.
v2: disambiguate Linux ("all drivers") and FreeBSD ("only DCO"), add
comment about --dev-type being necessary for devices not starting with
tun* or tap*
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20221108134523.2325-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25488.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Wed, 9 Nov 2022 11:52:08 +0000 (12:52 +0100)]
Fix md_kt_size in mbed TLS when queried for size of "none"
Previously this would error out with a M_FATAL message about cipher
not known. Align the mbed TLS version to OpenSSL version and also remove
unreachable code. This manifested in key_print2() running into this
M_FATAL message when used with an AEAD cipher and verb 7.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221109115208.1248948-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25494.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Wed, 14 Sep 2022 17:25:27 +0000 (19:25 +0200)]
Improve data key id not found error message
With delayed data key generation now with deferred auth, NCP and similar
mechanism the "TLS Error: local/remote TLS keys are out of sync" is shown
much too frequent and confuses a lot of people.
This also removes the dead code of printing multi not ready keys and
replace it with an assert.
Factor out printing of error messages into an extra function to make
the code easier to understand and also to only call into that function
in the case that a key is not found and avoid the overhead.
Patch v2: fix comparing key_id to state value, improve message
Patch v3: also take key_id into account
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20220914172527.2661529-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25212.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Wed, 31 Aug 2022 13:41:40 +0000 (15:41 +0200)]
Add workaround for Softether server dropping P_ACK_V1 with >= 5 acks
Softether had the number of ACKs in ANY OpenVPN packet limited to 4 and
dropped packets with more than 4 ACKs. This leads to Softether dropping
P_ACK_V1 packets with more than 4 ACKs as invalid. As the recent change
of always acking as many packets as possible, this leads to Softether
server not being able to successfully establish a connection anymore as
it never registers the ACKs.
This behaviour has been fixed on the Softether side with commit 37aa1ba5
but in order to allow clients to connect to older Softether servers, this
commit implements a workaround for the case that the peer might be a
Softether server (no tls-auth/tls-crypt and no other advanced protocol
feature) and limits ACKs to 4 in this case.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20220831134140.913337-2-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25142.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Wed, 31 Aug 2022 13:41:39 +0000 (15:41 +0200)]
Always include ACKs for the last seen control packets
This adds an MRU cache for the last seen packets from the peer to send acks
to all recently recently packets. This allows packets to be acknowledged
even if a single P_ACK_V1 gets lost, avoiding retransmissions. The downside
is that we add up to 28 byte to an P_ACK_V1 (7* packet_id) and up to 24
bytes to other control channel packets (4* packet_id + peer session id).
However these small increases in packet size are a small price to pay for
increased reliability.
Currently OpenVPN will only send the absolute minimum of ACK messages. A
single lost ACK message will trigger a resend from the peer and another
ACK message.
Patch v2: fix multiple typos/grammar. Change lru to mru (this is really an
MRU cache), add more unit test cases
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20220831134140.913337-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25143.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 4 Nov 2022 12:56:55 +0000 (13:56 +0100)]
Allow setting control channel packet size with max-packet-size
Currently control packet size is controlled by tun-mtu in a very
non-obvious way since the control overhead is not taken into account
and control channel packet will end up with a different size than
data channel packet.
Instead we decouple this and introduce max-packet-size. Control packet size
defaults to 1250 if max-packet-size is not set.
Patch v2: rebase on latest patch set
Patch v3: Introduce TLS_CHANNEL_MTU_MIN define and give explaination
of its value.
Patch v4: introduce max-packet-size instead of tls-mtu
Patch v5: improve documentation
Patch v6: Rebase, lower lower limit, add warning message for
when wrapped tls-crypt-v2 keys will ignore max-packet-size
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20221104125655.656150-2-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25477.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 4 Nov 2022 12:56:54 +0000 (13:56 +0100)]
Refactor/optimise code sending TLS control channel messages
This commit originally tried to solve a problem that the SSL library
might split up a control frame into multiple TLS records when doing
multiple reads. However, this does not seem to be actually the case.
OpenVPN will consider a control message packet complete
when the TLS record is complete, we have to ensure that the SSL library
will still write one record, so the receiving side will only be able
to get/read the control message content when a TLS record is
complete.
To improve handling of large control channel messages, this
commit does:
- Split one read from TLS library into multiple control
channel packets, splitting one TLS record into multiple
control packets.
- increase allowed number of outstanding packets to 6 from 4 on the
sender side. This is still okay with older implementations as
receivers will have room for 8. This allows transmitting larger
control message more quickly.
- take the wrapped key length into account when sending packets
This is especially important for the tls-crypt-v2 P_CONTROL_WKC_V1
message
- calculate the overhead for control channel message to allow
staying below that threshold.
- remove maxlen from key_state_read_ciphertext and related functions.
We now always give the function a correctly sized buffer.
If we end up needing to send a packet larger than max-packet-size, we
warn about it but still do it as it might still work, while refusing to
send will never work.
Patch v2: avoid assertion about to large buffer by sticking to 1250 max
control size in this commit and leaving larger sizes for the
--max-packet-size commit. Also fix
various other small problems and grammar fixes.
Patch v3: grammar fixes
Patch v4: adjust tls-mtu to max-packet-size in message.
Patch v6: no longer make the assumption that multiple reads from the SSL
library split a control frame into multiple TLS records.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221104125655.656150-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25478.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Selva Nair [Thu, 27 Oct 2022 16:06:19 +0000 (12:06 -0400)]
Do not copy auth_token username to itself
- Fixes a potential mis-behaviour (strncpy with
dest == src) introduced by commits ecad4839c (2.6)
and 3d792ae955 (2.5).
Reported by: Gert Doering <gert@greenie.muc.de>
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221027160619.11894-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/search?l=mid&q=20221027160619.11894-1-selva.nair@gmail.com Signed-off-by: Gert Doering <gert@greenie.muc.de>
Selva Nair [Wed, 26 Oct 2022 18:55:43 +0000 (14:55 -0400)]
Purge auth-token as well while purging passwords
Starting from commit e61b401a auth-token is saved in a separate struct
from auth-user-pass and is not cleared when ssl_purge_auth() is called.
This makes "forget-passwords" sent to the management
interface or "--management-forget-disconnect" option not to work
as expected.
Purging caused by --auth-nocache is not affected
(auth-token is retained in that case as it should be).
Use case:
For Pre-Logon access and persistent connections on Windows, use of
"forget-passwords" before disconnect is probably the only way to
ensure that no credentials are left behind. Note that openvpn.exe
continues to run after disconnect in these cases.
Also, the original intent of "forget-passwords" appears to be to
clear all "passwords" that can be used to reconnect.
v2:
- call ssl_clean_auth_token() directly from manage.c instead
of amending ssl_purge_auth()
- Add a comment that ssl_purge_auth() does not clear auth-token
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221026185543.5378-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25460.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Selva Nair [Sun, 23 Oct 2022 19:51:05 +0000 (15:51 -0400)]
Ensure --auth-nocache is handled during renegotiation
Currently, clearing auth_user_pass struct is delayed until
push-reply processing to support auth-token. This results in
username/password not purged after renegotiations that may
not accompany any pushed tokens -- say, when auth-token is not
in use.
Fix by always clearing auth_user_pass soon after it is used,
instead of delaying the purge as in pre-token days. But, when
"pull" is true, retain the username in auth_token in anticipation
of a token that may or may not arrive later.
Remove ssl_clean_user_pass() as there is no delayed purge any
longer -- auth-nocache handling is now done immediately after
writing username/password to the send-buffer.
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20221023195105.31714-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25452.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
close_tun: print interface type consistently in message
When closing the tunnel interface we know if we were using DCO or not.
for this reason we can customize the closing message and make it
consistent with the opening one.
Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221022205521.29406-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25449.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Wed, 19 Oct 2022 13:36:27 +0000 (15:36 +0200)]
Fix regression of ignoring --user
Commit facb6fffb changed a call in the style of if(a() | b())
to if(a() || b()). While this looks identical, it is not. The first
statement always executes b() while the second only executes b() if
a() returns false. This lead to to the platform_state_user never to
set as side effect and thus --user being ignored. Rewrite the code
to make this more explicit.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221019133627.2918110-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25430.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Wed, 12 Oct 2022 14:59:15 +0000 (16:59 +0200)]
FreeBSD DCO: introduce real subnet mode
To be able to configure a FreeBSD interface to "subnet" mode
(as opposed to point-to-point mode), it needs to have its
if_iflags set to IFF_BROADCAST. For tun(4) interface this is
done with the TUNSIFMODE ioctl(), but this does not work for
more modern interfaces like ovpn(4) which communicate over
a common SIOCSDRVSPEC ioctl() that contains a "cmd" and a
"parameter list".
Introduce OVPN_SET_IFMODE cmd, add dco_set_ifmode() function
to put kernel interface into IFF_BROADCAST or IFF_POINTOPOINT
as needed.
NOTE: this needs a FreeBSD kernel that includes commit 2e797555f701c38d9d
to add the OVPN_SET_IFMODE on the kernel side - with an older kernel,
OpenVPN + ovpn(4) will log an error, and "topology subnet" setups
will not work.
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Kristof Provost <kprovost@netgate.com>
Message-Id: <20221012145915.25810-2-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25395.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Wed, 12 Oct 2022 14:59:14 +0000 (16:59 +0200)]
FreeBSD: for topology subnet, put tun interface into IFF_BROADCAST mode
For reasons unknown, OpenVPN has always put FreeBSD tun(4) interfaces
into point-to-point mode (IFF_POINTOPOINT), which means "local and
remote address, no on-link subnet".
"--topology subnet" was emulated by adding a subnet-route to the "remote"
(which was just picking a free address from the subnet).
This works well enough for classic tun(4) interfaces that have no
next-hop resolution, and routes pointing to "that fake remote" only
(because all routing is done inside OpenVPN and it does not matter how
packets get there) - but for ovpn(4) interfaces, it breaks iroute setup,
where the route next-hop must be an on-link address.
Thus, change interface to IFF_BROADCAST mode, and get rid of all the
special code needed to "fake" subnet mode.
Tested with tun(4) and ovpn(4) on FreeBSD 14, client and server, and
with tun(4) on FreeBSD 12 and 7.4
To actually work with ovpn(4) / FreeBSD DCO, a followup patch for
kernel ovpn(4) and OpenVPN dco_freebsd.c is needed.
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Kristof Provost <kprovost@netgate.com>
Message-Id: <20221012145915.25810-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25396.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Tue, 18 Oct 2022 14:37:04 +0000 (16:37 +0200)]
Fix renewal spelling and actually allow external-auth with renewal time
The previous commit 9a516170 forgot to change to allow more than 2
parameters to auth-gen-token, so you could either have renewal time
or external-auth but not both. Also fix two instances of misspelled
"renewal".
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221018143704.2759522-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25418.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 9 Sep 2022 19:59:00 +0000 (21:59 +0200)]
Allows renegotiation only to start if session is fully established
This change makes the state machine more strict in terms of transaction
that are allowed. The benefit of this change are twofold:
- only allow renegotiations after pushed option handling is done,
to ensure that pushed options which might affect renegotiation
have been processed on both sides
This is a prerequisite for the upcoming secure renegotiation patch set
- avoids corner cases of a peer (or an attacker) trying to renegotiate the
session while the original session is not fully setup. Currently there
there are no problems known with this but it is better to avoid the
corner case in the first time.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Heiko Hund <heiko@ist.eigentlich.net>
Message-Id: <20220909195902.2011798-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25162.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Mon, 17 Oct 2022 09:51:45 +0000 (11:51 +0200)]
Allow Authtoken lifetime to be short than renegotiation time
Currently the life time of the auth-token is tied to the renegotiation
time. While this is fine for many setups, some setups prefer a user
to be no longer authenticated when the user disconnects from the VPN
for a certain amount of time.
This commit allows to shorten the renewal time of the auth-token and
ensures that the server resends the auth-token often enough over the
existing control channel. This way of updating the auth token is a lot
more lightweight than the alternative (frequent renegotiations).
Patch v2: fix grammar mistakes (thanks Gert), fix unit tests
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221017095145.2580186-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25407.html Signed-off-by: Gert Doering <gert@greenie.muc.de>