Marco Baffo [Wed, 19 Nov 2025 11:40:35 +0000 (12:40 +0100)]
route: handle default gateway (net_gateway) and nexthop towards VPN server separately
Right now there is the assumption that the gateway used for net_gateway is the same used to reach the VPN server.
However, these two gateways may be different (i.e. when there is a specific hostroute for the VPN server using a different nexthop).
For this reason we must adapt init_route_list() to fetch the two gateways separately.
Github: fixes OpenVPN/openvpn#890
Change-Id: I16d90221d0a75193035253817ff195f6da9dc0b3 Signed-off-by: Marco Baffo <marco@mandelbit.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1222
Message-Id: <20251119114041.17665-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg34529.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Sat, 15 Nov 2025 17:16:12 +0000 (18:16 +0100)]
OpenVPN Release 2.7_rc2
version.m4, ChangeLog, Changes.rst
Changes.rst has not received an "2.7_rc2" section - it has the
"highlevel" overview of what is new in 2.7, but for alpha/beta releases
it's better to look at git log to see what has been added/fixed.
Notable changes rc1 -> rc2 are:
- IPv6 address parsing: fix buffer overread on invalid input
(CVE-2025-12106)
- HMAC verification check: fix incorrect memcmp() call
(CVE-2025-13086)
- even more type conversion related warnings have been fixed
- DCO FreeBSD improvements:
improving debug messages (verb 6)
implement client-side counter handling
repair --inactive (and document shortcomings)
repair handling of DCO disconnection notifications in --client mode
- Windows/Service improvements, hardening, bugfixes
fix DNS address list generation (if 3 or more --dns addresses in use)
fix DNS server undo_list
disallow "stdin" as config name unless user has OpenVPN admin privs
fix compilation errors with MSVC v19
iservice: improve validation of config path (pathcc lib)
[NOTE: this breaks OpenVPN compatibility with Windows 7]
tapctl: refactor, improve output, change driver default to ovpn-dco
iservice: when restoring iface metrics, enforce correct ifindex
- improve cmocka unit test assert() handling
- PUSH_UPDATE server: fix reporting of client IPs in ``status`` output
after pushing a new IPv4/IPv6 address to client
- AEAD cipher safety margins: fix calculation of AEAD blocks in use
(old code would undercount blocks)
- fix invalid pointer creation / memory overread in tls_pre_decrypt
- deprecate ``--opt-verify`` (change into no-op + warning)
Arne Schwabe [Mon, 27 Oct 2025 09:05:55 +0000 (10:05 +0100)]
Fix memcmp check for the hmac verification in the 3way handshake being inverted
This is a stupid mistake but causes all hmac cookies to be accepted,
thus breaking source IP address validation. As a consequence, TLS
sessions can be openend and state can be consumed in the server from
IP addresses that did not initiate an initial connection.
While at it, fix check to only allow [t-2;t] timeslots, disallowing
HMACs coming in from a future timeslot.
Github: OpenVPN/openvpn-private-issues#56
CVE: 2025-13086
Reported-By: Joshua Rogers <contact@joshua.hu> Found-by: ZeroPath (https://zeropath.com/) Reported-By: stefan@srlabs.de
Change-Id: I9cbe2bf535575b47ddd7f34e985c5c1c6953a6fc Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Max Fillinger <max@max-fillinger.net>
ssl: Clean up type handling in export_user_keying_material()
For this we actually change the API of the
format_hex{,_ex} functions by changing int
to size_t for length parameters. While we
call this function with int paramters in
a lot of places (usually BLEN), this will
not produce warnings under
-Wno-sign-conversion. And we're sure those
values are positive since format_hex already
uses size_t internally.
Change-Id: Id7bacec23edc6dcd94465c308ea2144c7329a0c1 Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1301
Message-Id: <20251030145231.2792-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg34036.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
ssl: Change update argument of compute_earliest_wakeup to time_t
Since we usually input a diff of two time_t values here
the input value will be officially time_t. So avoid
conversion warnings at almost every caller site.
We can safely cast it to interval_t here because we
checked that it is smaller than the interval_t value
earliest. And all negative values are treated equal,
so exact value doesn't matter.
Change-Id: I6bc3147d10ca50291110335cd9fc3be961280c1b Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1299
Message-Id: <20251116183622.11727-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg34482.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Lev Stipakov [Sun, 16 Nov 2025 12:11:40 +0000 (13:11 +0100)]
tapctl: refactor 'create' command
Make default adapter name selection logic more robust -
sometimes renaming fails because the deleted adapter name
might present in the registry even though adapter is not shown
anymore in enumeration.
Ensure that adapter name doesn't contain disallowed symbols.
Use --hwid ovpn-dco by default and update documentation.
Change-Id: I270f679505c90ef78a5afbad1e05219f166be089 Signed-off-by: Lev Stipakov <lev@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1374
Message-Id: <20251116121146.4067-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg34455.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Lev Stipakov [Fri, 14 Nov 2025 11:50:22 +0000 (12:50 +0100)]
recursive routing: fixes and clean-ups
- get rid of atoi() for getting the remote transport port.
It doesn't change, so no point to do in on every packet.
In addition, atoi() breaks when we use service names as ports.
- ensure we correctly handle IPv4 headers with options
- directly use buf parameter in place of c->c2.buf
GitHub: closes OpenVPN/openvpn#902
Change-Id: I8a0a8029da02fc63adc918e8d98e9f676ff4ea0d Signed-off-by: Lev Stipakov <lev@openvpn.net> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1377
Message-Id: <20251114115029.17432-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg34415.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Heiko Hund [Wed, 12 Nov 2025 21:51:00 +0000 (22:51 +0100)]
iservice: use saved iface index to restore metric
When adding block rules, the interface metric of the VPN adapter is
temporarily modified so that an old version of Windows 10 would pick
it up first when looking up stuff via DNS. These metrics are reverted to
the old value when the block is removed.
When reverting them, instead of using the stored interface index where
the original values were read from, we were using the interface index
passed to the service with the wfp block message. That index could
theoretically be different from the one stored, which would result in
the metric being set to the wrong interface.
ssl: change return type of calc_control_channel_frame_overhead to size_t
This avoids dealing with conversion warnings inside
the function. Since we only add values that are
supposed to be positive this should be safe.
Note that we now cast the return value to int at
the caller side. There we actually substract it and
want to catch the case where the result gets negative.
Since all the involved values are quite small compared
to INT_MAX I decided to just cast it without further
checks.
Change-Id: I71e9d4a61d37483685723c16e98f59755694cadf Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1297
Message-Id: <20251111172437.7634-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg34326.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Wed, 12 Nov 2025 14:13:28 +0000 (15:13 +0100)]
Fix construction of invalid pointer in tls_pre_decrypt
In tls_pre_decrypt we construct a pointer ks with an invalid i if
i is TM_SIZE, doing a out-of-bounds read in multi->session.
This is a something that exists at least since 2.3.0 (I didn't go further
back but probalby exists in earlier version as well as the commits date
back to SVN beta21 branch).
So we construct the pointer but do not do anything with it if it is
invalid as we check i *after* we construct the pointer `ks`.
I suspect that the compiler optimises the bug away in any higher
optimisation level.
Assuming there is no optimisation, let's check what is possible.
Since we never use the value `ks` if it is invalid, we do not have
worry if it ends up invalid or not. The only thing that we have to
worry about is whether
`session + offsetof(struct tls_session, key[KS_PRIMARY])` is pointing
to memory that is valid to read to construct the `ks` pointer.
This is outside the tls_multi struct, so this is not guaranteed to be
allocated memory but at the same time it is also only few bytes (or few
tens/hundred) after the struct, so it the propability is very high that
it will be be in a memory region that will not cause a segfault on read.
Every time this condition is hit and we construct the invalid pointer,
the log message "TLS Error: Unroutable control packet received" is
printed at `verb 1` or higher. And this is a quite common log message,
which serves as indication as well that a crash is not something that
typically happens but either the optimisation fixes or the memory
region of the invalid access is valid to read from.
Based on this this was categorized as "bug, but no way to exploit
this, thus no CVE".
openssl_compat: Avoid conversion warning for SSL_get_negotiated_group
SSL_get_negotiated_group is documented to return
int and SSL_group_to_name definitely expects an int.
But SSL_get_negotiated_group is actually a macro
implemented by SSL_ctrl, which does return a long.
So to avoid the conversion warning we need the cast.
Change-Id: I31024f93d9d9d0f678fb39d4758a7e870bf00873 Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1359
Message-Id: <20251111153230.29865-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg34316.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
ssl: Change tls_send_payload size argument to size_t
There is only one caller of this function and it
wants it to be size_t. So move the size_t to int
conversion one step down in the call chain. Do not
switch key_state_write_plaintext_const, yet, since
that is a backend function and so needs way more
work.
Change-Id: Ic90c5a0e48bda4a02d5e11c4c161f388cc8805af Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1355
Message-Id: <20251111155239.31747-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg34320.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Wed, 12 Nov 2025 11:21:27 +0000 (12:21 +0100)]
Do not underestimate number of encrypted/decrypted AEAD blocks
Even though the current code typically counts all the encrypted/decrypted
traffic, this is only the case because of the specific implementation
of OpenSSL at the moment.
Instead of counting the length returned by one call only, count all
the encrypted/decrypted bytes.
Other implementations that use AES-GCM (like IPSec, MacSEC, TLS 1.2)
(currently) do not honour these usage limits at all. This is the reason that
I also currently do not consider the lack/improper validation in our code
to be a security vulnerability. In the current state implementations/protocol
that lack this feature altogether are not considered vulnerable.
Reported by: <stephan@srlabs.de>
Change-Id: I429d768fb33ef2c58484287d4091440ad8599053 Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1358
Message-Id: <20251112112133.1325-1-gert@greenie.muc.de> Signed-off-by: Gert Doering <gert@greenie.muc.de>
Heiko Hund [Sun, 9 Nov 2025 15:44:31 +0000 (16:44 +0100)]
iservice: make sure directories have trailing backslash
At least in the case of the config dir this matters, since the value is
used to validate input data for the interactive service. A missing \
at the end would allow a string compare to succeed, if the last element of
the path to compare starts with the same substring. The trailing slash
ensures that the last element of a path must match completely.
Heiko Hund [Wed, 12 Nov 2025 09:22:38 +0000 (10:22 +0100)]
iservice: validate config path better
Instead of just rejecting any path that contains ".." use some WIN32 API
functions to combine, canonicalize and then check if the resulting
path is located under the config directory. Makes the code prettier
and more correct.
Removes a few smaller instances:
- Fix return type check for socket() on Windows/Unixy
- Ignore a few instances related to WSAWaitForMultipleEvents.
The compiler says the check is currently useless, but
we follow the API documentation.
Change-Id: Iaabddb6f81cd94863291b193aae9d384a8f9d871 Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1207
Message-Id: <20251111154846.31360-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg34317.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Fri, 7 Nov 2025 17:48:05 +0000 (18:48 +0100)]
dco_freebsd.c: fix integer warnings
Fix all nvlist_get_number() related warnings by explicitly casting - these
are all messages coming from DCO, which we trust in this (nothing will
crash if a number is truncated, just "things will not work correctly").
Remove #pragmas.
Change-Id: Ief19ba87b0832baa6530ea8bf039d85115434e3e Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1361
Message-Id: <20251107174810.31851-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg34256.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Sun, 9 Nov 2025 08:42:31 +0000 (09:42 +0100)]
FreeBSD DCO: repair --inactive
--inactive on DCO requires a working DCO counters query function
(dco_get_peer_stats(), implemented in the previous commit) and
that the DCO implementation in use fills the "tun_{read,write}_bytes"
fields for the peer context.
FreeBSD DCO only fills the "dco_{read,write}_bytes" counters - which is
something we can't fix in OpenVPN, this needs kernel enhancements.
So, to make the feature (mostly) work, check the other set of counters
on FreeBSD. Caveat: this will count encryption overhead and keepalives,
so it will still not work for `--inactive <n>` without a byte count, or
for byte counts with too tight thresholds.
Adding the #ifdef to forward.c was considered the least bad alternative.
Gert Doering [Sun, 9 Nov 2025 08:41:23 +0000 (09:41 +0100)]
dco_freebsd: implement dco_get_peer_stats()
This is "fetch read/write statistics for a single peer", complementing
dco_get_peer_stats_multi() "... for all peers", and it is called in
--client mode, and (!) in p2mp mode to check if --inactive thresholds
are reached.
The FreeBSD DCO module has no "give me stats for a single peer" call, so
we just call dco_get_peer_stats_multi() to get all of them - and that
function is modified to handle p2p or p2mp mode by checking mode == CM_TOP.
(dco_linux does about the same in dco_get_peer*() -> ovpn_handle_peer(),
after a few iterations, except that it can query for "just one peer")
"--inactive" still does not work on FreeBSD, because the code in forward.c
looks at counters that are not set by FreeBSD DCO.
v2:
on AUTH_FAIL, 'dco' struct is not initialized yet -> SIGSEGV crash,
verify that dco_peer_id is >= 0 before calling dco_get_peer_stats_multi()
The only caller of this function uses a constant
for this parameter, so this is all quite safe. Add
an ASSERT for good measure anyway to make the assumption
explicit.
Change-Id: I6079bf9e7f6b37cb2e2d7f28851a77d0b08be995 Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1352
Message-Id: <20251106133936.30264-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg34209.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Fri, 7 Nov 2025 16:50:29 +0000 (17:50 +0100)]
dco_freebsd.c: add D_DCO_DEBUG messages for counters and notifications
Some of these debug messages only existed on Linux, and made debugging
DCO issues on FreeBSD more difficult. Add them, using the same style as
used for dco_linux.c
While at it, change all format strings for "peerid" to "%u" (wherever
appropriate, dco->dco_message_peer_id is an "int" today and changing
this to uint32_t is out of scope for "make better logging")
Change-Id: Ife55cb78401dad921b75f6c86d9bd0642f6a6e83 Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1349
Message-Id: <20251107165038.26171-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg34250.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
commit 4a48841da2 introduced a workaround for spurious DCO notifications
"with no useful content" on Linux - namely, ignoring dco_message_type==0
in forward.c, process_incoming_dco(), "because Linux has no message
type 0 anyway".
Each DCO platform uses its own enum for these notification messages
*inside* OpenVPN (which might not have the best design decision ever),
and FreeBSD had OVPN_CMD_DEL_PEER in the enum on "position 0"...
Fix by changing the enum to start with 1. Tested with DEL_PEER in p2p
client and DEL_PEER & FLOAT in p2mp server mode.
v2:
introduce OVPN_CMD_NO_MESSAGE in position 0, and a comment explaining why.
A future commit can then clean up forward.c and dco_linux.c to use the
constant, and not "magic 0 which happens to be in there after CLEAR()".
Github: fixes OpenVPN/openvpn#881
Change-Id: I991d6053776efed771bc1a3880acb80b55959cbc Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1353
Message-Id: <20251107141333.12056-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg34237.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Heiko Hund [Mon, 3 Nov 2025 11:59:40 +0000 (12:59 +0100)]
iservice: fix DNS address list generation
While generating the address list string for the DNS server addresses,
to be set in the registry, the offset is calculated the wrong way. This
results in gaps between addresses after the first two.
Gert Doering [Tue, 28 Oct 2025 19:04:58 +0000 (20:04 +0100)]
OpenVPN Release 2.7_rc1
version.m4, ChangeLog, Changes.rst
Changes.rst has not received an "2.7_rc1" section - it has the
"highlevel" overview of what is new in 2.7, but for alpha/beta releases
it's better to look at git log to see what has been added/fixed.
Notable changes beta3 -> rc1 are:
- even more type conversion related warnings have been fixed
- more bugfixes related to BYTECOUNT display on the management
interface and byte counters on DCO platforms in general
- numerous minibugs reported by ZeroPath AI have been fixed
(small memleaks, possible file descriptor leaks, improved
sanity checks, add ASSERT() on function contracts, etc.)
- add warning for unsupported combination of --push and --tls-server
- add warning for unsupported combination of --reneg-bytes or
--reneg-pkts with DCO
- remove perf_push()/perf_pop() infrastructure (because it did not
work anymore, and compiler profiling will give better results today)
- ensure compatibility with OpenSSL 3.6.0 - specifically, do not crash
in t_lpback.sh trying to use new encrypt-then-mac (ETM) ciphers
- improved PUSH_UPDATE server side support, which now handles changes
of pushed ifconfig/ifconfig-ipv6 addresses correctly (send packets
to new IP addresses to this client, stop sending packets to the old
addresses).
- improve CONTRIBUTING documentation
- add unit test for DHCP packet infrastructure
- freshen URLs all over the tree, and change to HTTPS where possible
- on DCO Linux/FreeBSD, add support for clients receiving an IPv4/IPv6
address that is not part of the --server/--server-ipv6 subnet
(= install extra on-interface host routes).
- Windows programs use a new API for path name canonicalization now
(PathCchCanonicalizeEx()) which will break building with MinGW on
Ubuntu 22.04 -> Upgrade to 24.04 to make builds work again.
- on Windows, when setting up WINS servers using netsh, use interface
index instead of adapter name now ("as for all other netsh calls")
- remove undocumented and unused --memstats feature
Max Fillinger [Fri, 31 Oct 2025 10:08:04 +0000 (11:08 +0100)]
Zeroize tls-crypt-v2 client keys
Joshua Rogers sent in a bug report generated with ZeroPath that the
tls-crypt-v2 client key is loaded before running the verify script. If
the verify script fails, the key is not zeroized.
While investigating this report, I found that free_tls_pre_decrypt_state
never zeroizes tls_wrap_tmp.original_wrap_keydata. So also when the
check is successful, key data will remain in memory when it is no longer
needed.
This commit moves the tls-crypt-v2-verify check before loading the key.
If it fails, original_wrap_keydata is zeroized. Also, in
free_tls_pre_decrypt_state, if a key has been loaded,
original_wrap_keydata is zeroized.
Reported-By: Joshua Rogers <contact@joshua.hu> Found-by: ZeroPath (https://zeropath.com/)
Change-Id: Icfcbf8ee20c1c0016eb98b570f24b9325b157c5c Signed-off-by: Max Fillinger <max@max-fillinger.net> Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1315
Message-Id: <20251031100819.24855-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg34103.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Marco Baffo [Thu, 30 Oct 2025 19:52:35 +0000 (20:52 +0100)]
PUSH_UPDATE server: invalid read bug-fix and unit-tests improvements
The number of messages calculated before the call to message_splitter(),
used in the memory allocation in the buffer array, could in certain
cases be less than one than the actual number of messages, thus causing
an override of the sentinel buffer in message_splitter() and therefore
an invalid read in send_single_push_update().
The case in question would be, for example, a sequence of three options
"A,B,C" with the size of B equal to safe_cap - 1 and the sum of the
sizes of A and C less than safe_cap - 2.
The buffer array was therefore replaced with a list of buffers to
completely avoid calculating the number of messages before it was
actually computed.
The test case in question has been added to the unit tests.
The unit tests have been improved using cmocka macros.
Change-Id: Idba419681fe3ccc4e6e2f6ce7592332dcff62cd9 Signed-off-by: Marco Baffo <marco@mandelbit.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1316
Message-Id: <20251030195244.2659-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg34073.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Note: the check for PATHCCH_ENSURE_TRAILING_SLASH in
configure.ac may be omitted if we build only using latest
mingw32-w64 toolchain. Ubuntu 24.04 is not new enough.
Heiko Hund [Thu, 30 Oct 2025 19:47:31 +0000 (20:47 +0100)]
iservice: check return value of MultiByteToWideChar
If the first call to MultiByteToWideChar returns 0, something must have
failed, because it returns the required buffer size including the
terminating zero. When it does return 0, just return NULL and indicate
that the call to utf8to16(_size) failed.
Arne Schwabe [Thu, 30 Oct 2025 19:29:57 +0000 (20:29 +0100)]
Ensure that get_sigtype always return non-NULL
There is a theoretical possibility that OpenSSL returns an NID that
OBJ_nid2sn cannot resolve and thus the function return NULL.
This is however extremely unlikely. But we still cover this case now
to make linters/code checker happy and avoid similar false positives
in the future.
Reported-by: Joshua Rogers <contact@joshua.hu> Found-by: ZeroPath (https://zeropath.com/)
Change-Id: I70e221ff5d9752fec17bad18fd41dcf188ae8fbc Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1325
Message-Id: <20251030193003.348-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg34060.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Thu, 30 Oct 2025 19:36:30 +0000 (20:36 +0100)]
Ensure return value of snprintf is correctly checked
Commit 130548fe4d change the usages of openvpn_snprintf to snprintf. When
doing that conversion I did not notice that, despite the function name,
openvpn_snprintf had a different semantic for the return value.
This commit goes through all the usages of snprintf and ensures that
the return is correctly checked for overruns.
Reported-by: Joshua Rogers <contact@joshua.hu> Found-by: ZeroPath (https://zeropath.com/)
Change-Id: I830b6b27fc3efe707e103ba629c4bfe3796a9cbe Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1330
Message-Id: <20251030193638.1010-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg34063.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Thu, 30 Oct 2025 19:43:56 +0000 (20:43 +0100)]
clean up environment variable handling in verify_user_pass_script
The username environment variable is already set by the
set_verify_user_pass_env function before the verify_user_pass_script
function is called, so this call is not doing anything but might erroneously
made people think that this needs to be cleaned up.
Also ensure that the password is clean from the env even in an error case.
Reported-by: Joshua Rogers <contact@joshua.hu> Found-by: ZeroPath (https://zeropath.com/)
Change-Id: I6c502508026c6b85bb092ada4d16d985b20dd41f Signed-off-by: Arne Schwabe <arne-openvpn@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1326
Message-Id: <20251030194402.1729-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg34069.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Wed, 29 Oct 2025 16:38:43 +0000 (17:38 +0100)]
Remove --memstats feature
The ``--mememstat`` was largely undocumented and there is no known
user of this feature. This feature provided very limited statistics
(number of users, link bytes read/written) and we do not except any
usage because of this.
The only documentation was a mention in --help without any mention of
the (binary) format of the mmap file or other usage instructions.
This deals also with issues reported by zeropath regarding potentially
insecure handling of the file permission of the memory mapped file.
Reported-by: Joshua Rogers <contact@joshua.hu> Found-by: ZeroPath (https://zeropath.com/)
Change-Id: I5f57e7bf52e3f6289462ef05e1f6e81ab0133d0d Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1329
Message-Id: <20251029163849.446-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg34021.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Wed, 29 Oct 2025 07:06:56 +0000 (08:06 +0100)]
Install host routes for out-of-subnet ifconfig-push addresses when DCO is enabled
ifconfig-push and ifconfig-ipv6-push can configure the IP address of a
client. If this IP address lies inside the network that is configured
on the ovpn/tun device this works as expected as the routing table point to
the ovpn/tun interface. However, if the IP address is outside that range,
the IP packets are not forwarded to the ovpn/tun interface and Linux
and FreeBSD DCO implementations need a "connected" route so kernel
routing knows that the IP in question is a peer VPN IP.
This patch adds logic to add host routes for these ifconfig-push +
ifconfig-ipv6-push addresses to ensure that traffic for these IP
addresses is also directed to the VPN.
For Linux it is important that these extra routes are routes using scope
link rather than static since otherwise indirect routes via these IP
addresses, like iroute, will not work. On FreeBSD we also use interface
routes as that works and routes that target interfaces instead of
next-hop IP addresses are less brittle.
This setups an ifconfig-push addresses outside the --server/--server-ipv6
network and additionally configures a iroute behind that client. The
setenv-safe configure lwipovpn to use that additional IP addresses to allow
testing via ping.
Windows behaves like the user space implementation. It does not require these
special routes but instead (like user space) needs static routes to redirect
IP traffic for these IP addresses to the tunnel interface. E.g. in the example
above the server config needs to have:
Gert Doering [Tue, 28 Oct 2025 20:31:50 +0000 (21:31 +0100)]
zeroize struct image in packet_id_persist_save() before writing to disk
while this really is only a debug function, ensuring that no uninitialized
heap content ends up in padding in the structure and thus to disk is good
practice.
Reported-by: Joshua Rogers <contact@joshua.hu> Found-by: ZeroPath (https://zeropath.com/)
Change-Id: I7f4c7b0ca748975defca1e5104e7077a761cd49c Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1323
Message-Id: <20251028203156.11697-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg33983.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Tue, 28 Oct 2025 20:32:10 +0000 (21:32 +0100)]
remove redundant PULL_DEFINED() macro definition
this seems to be a leftover of the time when we had conditional
compilation for "--disable-server" or thus. Commit d6a0cf599
removed PUSH_DEFINED() nearby but overlooked this one.
Change-Id: I9118333bb65cd5db0836abefa5d45a729f0142cc Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1322
Message-Id: <20251028203219.11737-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg33984.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Ralf Lici [Tue, 21 Oct 2025 07:08:20 +0000 (09:08 +0200)]
management: ensure consistent BYTECOUNT timing on server
The BYTECOUNT notification is expected to be emitted every N seconds
when a management client issues the 'bytecount N' command. However, the
server currently relies on timeouts from unrelated periodic operations,
resulting in irregular notification timing.
This issue is especially noticeable with low bytecount intervals and DCO
enabled, as openvpn handles less traffic in userspace, causing the main
loop to run less frequently.
To address this, refactor the timeout logic and pass the timeval
reference to management_check_bytecount_server so that the timeout is
correctly set and notifications adhere to the specified interval.
Change-Id: Ifb1c49fce75e671f699f5db5f6da7246f6e0b519 Signed-off-by: Ralf Lici <ralf@mandelbit.com> Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20251021070825.20773-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg33812.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Thu, 23 Oct 2025 15:56:08 +0000 (17:56 +0200)]
Warn if push is used without --mode server/--server/--server-bridge
This is not a supported configuration and will often work good enough
to get a connection working but will operate more in a weird pre P2P
negotiation compatibility way rather than actually negotiating
protocol features.
While at it, remove an unused macro (PUSH_DEFINED).
Change-Id: I82c7c61be07593ecd5bf2f854767dda74ab5170c Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1288
Message-Id: <20251023155614.20642-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg33856.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Joshua Rogers [Tue, 21 Oct 2025 17:20:52 +0000 (01:20 +0800)]
tcp: apply CLOEXEC to accepted socket, not listener
The accept path calls set_cloexec(sd) after accept(). That re-flags the
listening socket, which is already CLOEXEC from create_socket_tcp(), and
leaves new_sd inheritable. As a result, client-connect and auth scripts
spawned after accept can inherit the connected socket and read or write
the raw TCP stream. This defeats the stated intent to prevent scripts from
accessing the client socket.
This bug was found using ZeroPath.
Signed-off-by: Joshua Rogers <MegaManSec@users.noreply.github.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <-MNw5Hu8h0rHV18x36ISt7V0UHchIO4i-JoAeV_wlxS1AmDIAe7YVYNput3_r2hiu3HhwxkhGyUhv4-iH_E7mf7nGjvocmGXlDq7Tjly5cE=@joshua.hu>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg33823.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Since OpenVPN spawns various child processes, it is important
that sockets are closed after calling exec.
The sitnl socket didn't have the right flag set, resulting
in it surviving in, for example, connect/disconnect scripts
and giving the latter a chance to abuse the socket.
Ensure this doesn't happen by setting FD_CLOEXEC on
this socket right after creation.
Reported-by: Joshua Rogers <contact@joshua.hu> Found-by: ZeroPath (https://zeropath.com/)
Change-Id: I54845bf4dd17d06cfc3b402f188795f74f4b1d3e Signed-off-by: Antonio Quartulli <antonio@mandelbit.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1314
Message-Id: <20251028162843.18189-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg33952.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Ralf Lici [Tue, 28 Oct 2025 11:33:05 +0000 (12:33 +0100)]
dco-freebsd: fix peer stats storage on client instances
Commit bf01a96 introduced a bug in the dco-freebsd path by attempting to
store peer statistics in a structure that only exists on server
instances. This leads to a SIGSEGV on non-server instances due to a NULL
multi_context pointer.
Resolve this by checking what mode the current instance is running in
and storing peer stats accordingly.
Steffan Karger [Sun, 26 Oct 2025 14:20:52 +0000 (15:20 +0100)]
Remove perf.c/perf.h
This code was always disabled by ENABLE_PERFORMANCE_METRICS being
commented out in perf.h. There was no configure flag. None of the
active developers remembers using it, the git log shows no actual
code changes since at least the project structure overhaul of 2012,
and tools like gprof are nowadays the go-to tool for performance
profiling. So, out with our custom implementation.
This was triggered by a bug report submitted by Joshua Rogers, who
used ZeroPath to discover we missed a perf_pop() call in one of the
error paths of ssl_mbedtls.c. This commit resolves that using git rm.
Arne Schwabe [Thu, 23 Oct 2025 11:11:33 +0000 (13:11 +0200)]
Do not try to use the encrypt-then-mac ciphers from OpenSSL 3.6.0
These ciphers claim to be CBC but since they are also include an HMAC
are more a mix of AEAD and CBC. Nevertheless, we do not support these
and also have no (good) reason to support them.
This patch defines the flag if the SSL library does not define the flag
to also work when the SSL library is upgraded after OpenVPN has been compiled.
Change-Id: Iafe3c94b952cd3fbecf6f3d05816e5859f425e7d Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1294
Message-Id: <20251023111138.25245-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg33846.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Ralf Lici [Fri, 17 Oct 2025 20:58:31 +0000 (22:58 +0200)]
dco: remove dco_read/write_bytes from dco_context_t
Remove dco_read_bytes and dco_write_bytes from all dco_context_t
structures, as peer statistics are now stored directly in the
corresponding c2 fields across all DCO interfaces.
Ralf Lici [Sun, 19 Oct 2025 17:02:42 +0000 (19:02 +0200)]
dco-freebsd: store peer stats directly in c2
The dco_context_t structure includes a reference to the general context
structure c, which allows us to store dco_read_bytes and dco_write_bytes
directly as c2 fields. This aligns the FreeBSD implementation with how
we handle DCO peer stats on Linux and Windows.
Marco Baffo [Fri, 17 Oct 2025 20:19:12 +0000 (22:19 +0200)]
PUSH_UPDATE server: remove old IP(s) from vhash after sending a message containing ifconfig(-ipv6)
When sending a PUSH_UPDATE containing an ifconfig(-ipv6) option, we must add the new IP to the
multi_context vhash (hash table of the clients indexed by virtual IPs). Now in addition to
adding new client IPs, old IPs are also removed from vhash, allowing for a more complete update.
Change-Id: I07a8ddd9026eef64b6f5abde98702a9801616a5f Signed-off-by: Marco Baffo <marco@mandelbit.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1253
Message-Id: <20251017201916.21697-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg33412.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Ralf Lici [Fri, 17 Oct 2025 19:16:06 +0000 (21:16 +0200)]
options: warn and ignore --reneg-bytes/pkts when DCO is enabled
Thresholds specified by --reneg-bytes and --reneg-pkts cannot be
enforced when DCO is enabled, as it only provides global statistics.
Rather than adding complexity to support these options, ignore them when
DCO is enabled. Print a warning to inform users and update the manpage
accordingly.
win32: Change some APIs to use DWORD instead of size_t
This is what the Win32 APIs use. Since we put static
integers into this (e.g. sizeof()) this doesn't
result in new conversion warnings at the caller sites.
Change-Id: Ia836e3c05a868a7e8419c2bb2f547d968260783c Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1269
Message-Id: <20251013162221.2156-1-gert@greenie.muc.de>
URL: https://sourceforge.net/p/openvpn/mailman/message/59246222/ Signed-off-by: Gert Doering <gert@greenie.muc.de>