Gert Doering [Thu, 27 Nov 2025 11:35:12 +0000 (12:35 +0100)]
extract_x509_field_ssl(): verify that X509_NAME is not NULL.
This seems to be unlikely to ever happen, but this check won't harm
- as a matter of coding convention, we do not ensure this inside
extract_x509_field_ssl(), but in the (single) caller.
While at it, fix pre-C99 local-variable indent block, and missing {}
block in else/#endif construction.
Reported-By: Joshua Rogers <contact@joshua.hu> Found-by: ZeroPath (https://zeropath.com/)
Change-Id: I1e9c7eee06bf5f2e8aed8cd2523684539294ac8b Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Antonio Quartulli <antonio@mandelbit.com>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1388
Message-Id: <20251127113517.1352-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg34748.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Some things were detected in configure.ac and just
hard-coded in CMake. Move the hard-coding to
syshead.h instead and remove the additional code
in configure.ac. These are really Windows-specific
so I think not dynamically detecting is okay.
Change-Id: Ic55e1288b0d3160e68eef07ff4edbb070a03dd31 Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1410
Message-Id: <20251127103710.29341-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg34736.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
error: Allow status argument to check_status to be ssize_t
We only check for < 0 so no reason not to allow bigger
signed values. This makes it easier to feed result
values of read/write to it. Fixes a conversion warning
in process_outgoing_tun.
Also changes register_activity to int64_t for similar
reasons.
Change-Id: I750a46246c0d2447a6691e5c25c4732e3d335a63 Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1357
Message-Id: <20251126114047.10280-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg34701.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Traditional OpenVPN ``--multihome`` behaviour is to send packets out the
same interface that they were received on (copy ipi_ifindex from ingress
to egress packet info). For some scenarios this makes sense, for other
scenarios it is breaking connectivity when there are no routes pointing
out the ingress interface (intentionally asymmetric traffic).
For 2.7.0, change the default(!) to always send out packets with
ipi_ifindex = 0, to follow normal system interface selection rules.
Add a flag ``--multihome same-interface`` to restore the pre-2.7 behavior
of copying ipi_ifindex from ingress to egress packets. There are use
cases for this, and we want to give users a chance to read the release
notes and adjust their setups to "not break after upgrading to 2.7.0".
Heiko Hund [Mon, 24 Nov 2025 17:00:50 +0000 (18:00 +0100)]
iservice: rename one_glyph to glyph_size
Throughout the function variables which deal with byte counts have a
_size postfix. one_glyph is the number of bytes in one character.
Reading the code is easier and more consistent this way.
Heiko Hund [Wed, 26 Nov 2025 10:49:32 +0000 (11:49 +0100)]
iservice: fix off by one error
In case there is more than one non-ignored interface domain, the start
of the next domain was off by one glyph. That meant that all but the
first domain was ignored when converting to MULTI_SZ.
Reported-by: Marc Heuse <marc@srlabs.de> Reported-by: stephan@srlabs.de
Change-Id: I3ebf30f6d9edc66eb54fb3f1b5634c96d156b1ca Signed-off-by: Heiko Hund <heiko@ist.eigentlich.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1395
Message-Id: <20251126104938.5612-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg34678.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Heiko Hund [Wed, 26 Nov 2025 10:47:01 +0000 (11:47 +0100)]
iservice: handle ignoring itf domains correctly
GetItfDnsDomains() does ignore domains, which are in the search domains
list. Handling of this was done wrong if there was more than one
interface domain. In any case the size returned to the caller was
calculated wrong.
Reported-by: Marc Heuse <marc@srlabs.de> Reported-by: stephan@srlabs.de
Change-Id: I02e2c7b27b5a39b11556e4753c648baa05344ffc Signed-off-by: Heiko Hund <heiko@ist.eigentlich.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1394
Message-Id: <20251126104706.5378-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg34677.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Heiko Hund [Wed, 26 Nov 2025 10:43:07 +0000 (11:43 +0100)]
iservice: return correct size when domains are truncated
In case not all domains can be converted in the available buffer, return
the size of the already converted domains size plus the second terminating
zero for the MULTI_SZ.
Reported-by: Marc Heuse <marc@srlabs.de> Reported-by: stephan@srlabs.de
Change-Id: Iabad046c930dd0f38ec50d1c43d8b3288816df7d Signed-off-by: Heiko Hund <heiko@ist.eigentlich.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1391
Message-Id: <20251126104315.4776-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg34680.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Heiko Hund [Wed, 26 Nov 2025 10:34:21 +0000 (11:34 +0100)]
iservice: fix calculation of converted domains size
To keep track of how much of the buffer is already used, the difference
of the current position and the start of the buffer needs to be
multiplied with the size of a character / glyph to bet the byte count,
with which calculations are done further down below.
Reported-by: Marc Heuse <marc@srlabs.de> Reported-by: stephan@srlabs.de
Change-Id: I16f9426e57f4802ba038ab51f5b70161464b9428 Signed-off-by: Heiko Hund <heiko@ist.eigentlich.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1390
Message-Id: <20251126103427.4085-1-gert@greenie.muc.de> Signed-off-by: Gert Doering <gert@greenie.muc.de>
multi-socket: do not return tuntap flags on server-side
Tuntap flags are already handled within
multi_io_process_io() so return them by
multi_io_process_flags() would be redoundant,
since we need them only for tun_set().
While at it, removed part of the I/O process
from multi_process_io_udp() since those are
also handled within multi_io_process_io(),
removed the FILE_CLOSED and FILE_SHIFT
defines since we now handle that kind of
event in multi_io_process_io() through
the MULTI_IO_FILE_CLOSE_WRITE define.
Change-Id: I6a5110a0583b8b33496b06d9c27c1084df38e842 Signed-off-by: Gianmarco De Gregori <gianmarco@mandelbit.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1364
Message-Id: <20251124175832.21352-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg34650.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Selva Nair [Mon, 24 Nov 2025 16:53:47 +0000 (17:53 +0100)]
Restrict access to the service pipe to SYSTEM and owner
Access is restricted to SYSTEM and pipe client user
(the user starting openvpn.exe). The default is
full access to Administrtors, owner, and read access
to everyone. This hardens the pipe further.
Heiko Hund [Sun, 23 Nov 2025 11:58:46 +0000 (12:58 +0100)]
iservice: make sure registry string is terminated
When reading the interface domains from the registry, check that the
string is zero terminated, since the code in GetItfDnsDomains depends
on the fact when doing size calculations during the conversion.
Reported-by: Marc Heuse <marc@srlabs.de> Reported-by: stephan@srlabs.de
Change-Id: Icaeca22bdbd8ead0cb12345b1bcc2b5c0f46236f Signed-off-by: Heiko Hund <heiko@ist.eigentlich.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1392
Message-Id: <20251123115851.19555-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg34610.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
tls_crypt: Fix Coverity complaint in tls_crypt_v2_check_client_key_age
Coverity complained about "overflow_before_widen" because
there is a theoretical overflow that can happen even though
the target value is wide enough. For useful values of max_days
this is irrelevant but Coverity is not wrong, so change the
code accordingly.
Change-Id: Ie7308d549182a95b86cd113e4a8cc65ff45ba3d7 Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1385
Message-Id: <20251122162553.12254-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg34585.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
GHA: Add macos-26 and remove OpenSSL 1.1 builds on macOS
The new runner doesn't have OpenSSL 1.1 available anymore
via homebrew. The formula is also marked as "deprecated"
for older macOS versions. So I think it is okay to drop
it.
Change-Id: I1633ee6207443e041434e9f0dda98afd5a22e1b4 Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Yuriy Darnobyt <yura.uddr@gmail.com>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1382
Message-Id: <20251119162934.15672-1-gert@greenie.muc.de> Signed-off-by: Gert Doering <gert@greenie.muc.de>
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>
projects / thirdparty / openvpn.git / log
