Max Fillinger [Wed, 22 Jul 2026 16:37:55 +0000 (18:37 +0200)]
Remove --providers from --help output for Mbed TLS
This option is OpenSSL-only. When used with Mbed TLS, it prints warnings
and does not do anything. With this change, the option is still parsed,
to ensure compatiblity of configurations, but it is removed from the
--help text.
Change-Id: I0f56ca69efae0ca376eb9eae064a893c8768188e Signed-off-by: Max Fillinger <maximilian.fillinger@sentyron.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1799
Message-Id: <20260722163800.2474-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg37781.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Max Fillinger [Wed, 22 Jul 2026 15:25:14 +0000 (17:25 +0200)]
Make --x509-username-field work with Mbed TLS
In pre-2.7 versions, this option was not available in the Mbed TLS
build. This was changed in 2.7, but the option did not do anything when
Mbed TLS was selected as the crypto library. Regardless of the field
chosen with --x509-username-field, OpenVPN would always extract the CN
as username.
This could lead to a situation where an unintended certificate gets
accepted by OpenVPN: If we run with "--x509-username-field serialNumber"
and "--verify-x509-name 0x05 name", OpenVPN would accept a certificate
with CN=0x05 and an incorrect serial number, while a certificate with
the correct serial number would be rejected. (Though note that to
exploit this, an attacker needs to make the CA sign a certificate
with a hexadecimal number in the CN.)
This commit adds code to backend_x509_get_username to extract the
correct field values from X509 certificates. It also changes the
behavior of the function to match the OpenSSL version when the output
buffer is too small. (With Mbed TLS, the function would silently
truncate the output and return SUCCESS.)
It also adds unit tests for extracting the values of different fields.
Despite this commit fixing a CVE, we have decided not to keep it under
embargo until the release of the next version because it is unlikely to
be exploitable in practice: Someone has to run OpenVPN 2.7 with Mbed
TLS, use the --x509-username-field option even though it didn't exist in
Mbed TLS builds of earlier versions, not notice that the intended
certificate is *not* accepted, and then an attacker has to get the CA to
sign a certificate with a weird common name.
CVE: 2026-63650
Github: openvpn/openvpn-private-issues#144 Reported-By: Hcamael Reported-By: 章鱼哥 (www.aipyaipy.com)
Change-Id: Ic183f1f1f90561454b7b1128c95255368427cc4a Signed-off-by: Max Fillinger <maximilian.fillinger@sentyron.com> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1801
Message-Id: <20260722152521.22272-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg37773.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
This has also been reported twice as a security relevant bug, but
only later than the original finding - and it isn't.
While --client-nat would modify a 32bit integer "after the packet"
(the place where an IPv4 address would be, in a well-formed packet),
the underlying buffer is always max-frame sized, and we never look
at the "modified integer" afterwards, so there are no consequences
warranting allocation of a CVE ID.
Signed-off-by: rootvector2 <dxbnaveed.k@gmail.com> Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org> Acked-by: Antonio Quartulli <antonio@mandelbit.com>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1789 Reported-By: 章鱼哥 (www.aipyaipy.com) Reported-By: Yu Zhang Wong <wongyuzhang45@gmail.com>
Change-Id: I8219c6295acf28ff10ddb2fcc285f813c42fa8fe
Message-Id: <20260715202210.9010-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg37652.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Wed, 8 Jul 2026 15:31:07 +0000 (17:31 +0200)]
Replace strtok with strtok_r
This does not change anything in our source code but makes compiling
with newer Android NDKs (version 30 pre release) -Werror safe again
as it has started throwing warning on this.
The compat version is taken from current FreeBSD (commit dc36d6f9bb1)
src/lib/libc/string/strtok.c and reformatted to our clang standard.
Change-Id: I70560efd113308b7377424127eb2c1da4266371a Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1679
Message-Id: <20260708153107.60809-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg37538.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
- Exclude tests that use verify_script on Windows.
We currently do not have openvpn_execve available
for Windows tests. It would also need more work
to replace the hardcoded paths.
- Use platform_gen_path instead of hard-coding /
Do to the previous change that is not strictly
necessary, but it also doesn't hurt and it fixes
at least one issue if anyone ever decides to port
this to Windows).
Change-Id: I45ddc8e350d3948b3197ae725df102d8ce73fa77 Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Lev Stipakov <lstipakov@gmail.com>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1781
Message-Id: <20260714121238.13936-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg37593.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
t_client.sh.in: Do not run resolvectl if systemd is not running
In some cases we will have resolvectl installed but no
running systemd. E.g. on old Ubuntu distros resolvectl
is part of the main systemd package which we have installed
as a build-dependency.
Supress the warnings from resolvectl in this case.
Change-Id: I677d3206c24aad997889ed76fb8eca2e6f4b1259 Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1778
Message-Id: <20260713151816.22005-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg37566.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Lev Stipakov [Thu, 9 Jul 2026 14:42:40 +0000 (16:42 +0200)]
CMake: detect cmocka_version.h via include path, not by linking
check_include_files() was invoked with cmocka in CMAKE_REQUIRED_LIBRARIES,
which makes the probe link the (shared, imported) cmocka library. On some
platforms that link step fails inside the minimal probe, so the header check
reports failure and HAVE_CMOCKA_VERSION_H is left undefined even though
cmocka_version.h is present.
test_common.h then selects its cmocka 1.x compatibility shims and redefines
macros (check_expected_uint, expect_uint_value, ...) that cmocka 2.x already
provides, which breaks the -Werror build of every unit test driver.
Detecting a header only needs the include search path, so pass cmocka's
INTERFACE_INCLUDE_DIRECTORIES via CMAKE_REQUIRED_INCLUDES and drop the library
link requirement from the probe.
Change-Id: Icc14b286f2409738c73873aa7550dcc005ee8cbb Signed-off-by: Lev Stipakov <lev@openvpn.net> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1739
Message-Id: <20260709144240.61362-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg37552.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
ssl_pkt: Fix doxygen warning about read_control_auth
Commit 01ee1693f960a696a613e7b83f9960002264a379
("Ensure tls-crypt keys are not setup twice") removed
the argument but did not delete it from the doxygen
documentation, leading to warnings.
Change-Id: I20cc89a546b69aceadcd1630828de407d530d7c7 Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1765
Message-Id: <20260702100704.24692-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg37459.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Thu, 18 Jun 2026 14:33:43 +0000 (16:33 +0200)]
Ensure tls-crypt keys are not setup twice
Commit 82ee2fe4b42d already did this when the session id stayed the same
but forgot the other code path that could also lead to tls-crypt keys to be
setup.
This approach was a bit too fragile as it missed some other code path
that might trigger the same behaviour. This commit changes the logic
to directly infer if the key is already initialised instead of taking
a proxy (like key state as the previous commit did).
The fix in commit 7a6ab5773 (#121, #127) made sure that we do not try
to extract the tls-crypt-v2 key multiple times and made the unit test
basically not work as the extraction was skipped and then could also
not fail anymore. I could work around it in the unit test but improving
tls_wrap_free felt preferable.
CVE: 2026-13698 Reported-By: Max Fillinger <maximilian.fillinger@sentyron.com>
Github: OpenVPN/openvpn-private-issues#137
Github: OpenVPN/openvpn-private-issues#138
Change-Id: I3b5e4e84762aa253d46e69103f7b1e84ebefca1d Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-By: Max Fillinger <maximilian.fillinger@sentyron.com>
Lev Stipakov [Fri, 22 May 2026 09:05:43 +0000 (12:05 +0300)]
openvpnserv: fix DNS SearchList state pollution on (dis)connect
RemoveDnsSearchDomains used wcsstr on the comma-separated SearchList,
so removing a VPN-pushed domain like "vpn.corp.local" could match
inside an unrelated pre-existing entry like "test.vpn.corp.local"
and corrupt it. The wmemmove length was also too short, truncating
the tail of the list when the removed segment was shorter than what
followed. The wcsncmp against InitialSearchList was prefix-only too,
so a shortened list that happened to be a prefix of the initial one
triggered a spurious reset.
Replace the manual cut with two pure helpers that are unit-tested in
test_openvpnserv.c:
* AppendSearchList just appends the requested string, with a separator
comma if the list is non-empty. No deduplication: pushed domains are
always appended verbatim.
* RemoveSearchListTokens uses multiset semantics. For each comma-
separated token in the undo string it finds the last remaining
token in the list that fully matches (no substring) and splices it
out in place. If a domain appears K times in the list and L times in
the undo set, min(K, L) instances are removed.
Together these properties give the right behavior under overlap:
user has "corp.local"; VPN pushes "corp.local"
-> SearchList = "corp.local,corp.local"
-> on disconnect one occurrence is removed, user's entry preserved.
session A pushes "openvpn.com"; session B pushes "openvpn.com"
-> SearchList = "openvpn.com,openvpn.com"
-> A disconnects: one removed, B still sees its domain.
-> B disconnects: list returns to original.
AddDnsSearchDomains and RemoveDnsSearchDomains are now thin wrappers
around the helpers: read SearchList, call the helper, write back,
plus the StoreInitialDnsSearchList / ResetDnsSearchDomains dance.
Max Fillinger [Fri, 22 May 2026 14:55:13 +0000 (16:55 +0200)]
Fix ack_write_buf use after free
If the active TLS session has a pending dedicated ACK packet,
tls_multi_process sets to_link to ks->ack_write_buf. If in the same
execution of tls_multi_process, the initializing session reaches the
authenticated stage, the active session will be freed which leaves
to_link.data pointing to freed memory.
This commit extends check_session_buf_not_used so that it also checks
ks->ack_write_buf for all key states.
Max Fillinger [Fri, 22 May 2026 12:34:57 +0000 (14:34 +0200)]
Fix tls_wrap_reneg use after free
When dynamic tls-crypt is active, it is possible for tls_multi_process
to set to_link to session->tls_wrap_reneg.work and later free that
session, leaving to_link.data pointing to freed memory.
This is not caught by the function check_session_buf_not_used because it
checks only tls_wrap, not tls_wrap_reneg. This commit adds that check.
Arne Schwabe [Mon, 15 Jun 2026 18:35:07 +0000 (20:35 +0200)]
Clean up metadata handling in tls_crypt_v2_extract_client_key
This makes the metadata a local variable instead of a member of the
wrap_context struct. Also always ensure that this buffer is freed to
avoid any leak of the metadata buffer.
This touches the check methods. Ensure that they still work as
intended by adding unit tests for both script and age checks.
dco.c: fix argument order in dco_install_key() log string
The debug log in dco_install_key() has the epoch and dco_keys_installed
arguments swapped. So a line like epoch=1, currently 0 keys installed
actually means dco_keys_installed == 1 and epoch == false.
Use the correct argument ordering in the log string.
Lev Stipakov [Mon, 29 Jun 2026 12:48:34 +0000 (14:48 +0200)]
win32: fix plugin trusted-dir check prefix bypass
plugin_in_trusted_dir() validated the plugin path against the trusted
plugin/install directory (and the system directory fallback) using a raw
string prefix match via wcsnicmp(). When the trusted directory path does
not end in a separator (e.g. the plugin_dir registry value is set to
"C:\openvpn_plugins"), a sibling directory sharing the same prefix
("C:\openvpn_plugins_evil") also passes the check, allowing a plugin to
be loaded from outside the allow-listed directory.
Introduce win_path_in_dir() in win32-util.c which performs the prefix
match but additionally requires the match to end on a path-component
boundary, and use it for both the plugin/install directory and the
system directory checks. Add unit tests in test_misc.c.
Heiko Hund [Sun, 28 Jun 2026 13:02:50 +0000 (15:02 +0200)]
openvpnserv: rework ConvertItfDnsDomains and tests
Since there were issues with in-place modification of the buffer
repeatedly, re-implement ConvertItfDnsDomains() to use a internal
temporary buffer to prevent use of memmove and the length calculations
that come with it. Code should be easier to grasp since we're dealing
with one set of lengths (WCHARs) instead of two (WCHARs + octets) now.
The unit tests did not actually test the MULTI_SZs correctly, fixed
that and also added some more tests to cover more scenarios.
This was initially reported as a security vulnerability, as the previous
code could be tricked into overreading a buffer and accessing undefined
memory - but we decided against assigning a CVE ID because the data read
is subsequently never leaked anywhere, and it was not possible to trigger
a crash, so "just a bug, no security implications".
Arne Schwabe [Fri, 26 Jun 2026 15:29:46 +0000 (17:29 +0200)]
Make get_random return int64 instead of long
This avoids having get_ranomd being different on 32bit/Windows vs
64 bit Unix platform. Also adjust platform_create_temp_file to
create the same files on all platforms.
Change-Id: Ifefb3ad204c0c16cb4952dd6e8661fdc9136b125 Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1732
Message-Id: <20260626152951.29207-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg37322.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
options: fix use-after-free of DNS options on client connect
struct dns_options embeds its own gc_arena. When inherit_context_child()
/inherit_context_top() copy struct options by value, the child shares the
parent's DNS arena. options_detach() detached o->gc but not
o->dns_options.gc, so pre_connect_restore()'s gc_free() (and context
teardown) freed allocations the parent still referenced.
With one or more non-pushed --dhcp-option directives that yield a DNS
entry, a connecting client triggers this and the server crashes
(use-after-free in setenv_dns_options(), reported as a double free).
Detach o->dns_options.gc as well, mirroring the existing o->gc handling.
Change-Id: I49b37b5a90554fa2d4a83c8fc5608dad2a36b835
GitHub: closes openvpn/OpenVPN#1060 Signed-off-by: Antonio Quartulli <antonio@mandelbit.com> Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1715
Message-Id: <20260622120856.21586-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg37230.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
saddamr3e [Thu, 18 Jun 2026 06:18:28 +0000 (08:18 +0200)]
dns: Fix memory leak in dns_server_addr_parse
When the DNS server's address count is already full (exceeds the
limit of 8), the function returned early without freeing the
successfully resolved addrinfo struct. Fix this by checking the
limit before executing the DNS lookup.
udp_flags does not guarantee correct association with the socket being
processed. Use the rwflags delivered by the event engine along with the
event to ensure proper per-socket I/O handling.
Remove udp_flags entirely.
Replace the previous global-style flag computation with a per-socket
decision model in p2mp_iow_flags(), which derives I/O flags
directly from the current multi_context state and the specific socket
being processed. This ensures that read/write decisions are correctly
bound to the active socket rather than shared or implicit global state.
This change is based on an investigation triggered by a report from
Joshua Rogers using ZeroPath.
Change-Id: I6b303805a3688b6f6363140c76853a58badecd8f 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/+/1635
Message-Id: <20260616162129.28519-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg37198.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Tue, 9 Jun 2026 15:52:06 +0000 (17:52 +0200)]
AWS-LC: Use openssl_stack_size_t for declaring stack size
OpenSSL and AWS-LC disagree on the type that they use for
stack size. Instead of doing a lot of various casts, use
a typedef to avoid these casts and use the right type for
each library.
Change-Id: Ifd29485524674c64d56fc5f7ef8bdd1e00215fc9 Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1627
Message-Id: <20260609155211.30747-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg37144.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Since Github will switch to windows-2025 image to use
VS 2026 at some point in the near future (according to
https://github.blog/changelog/2026-02-05-github-actions-early-february-2026-updates/#windows-server-2025-with-visual-studio-2026-image-now-available-for-github-hosted-runners)
enforce that switch to avoid any build failures. Since
we hardcode the VS version in CMakePresets.json (and I
do not see an easy way to avoid this with CMake), we
need to override the generator value. We can switch the
default once our release infrastructure has been switched
to VS 2026.
Change-Id: I8a17d452e9e5690a5507d17129286142df6c88f1 Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1711
Message-Id: <20260609152123.27460-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg37139.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Max Fillinger [Tue, 9 Jun 2026 10:24:01 +0000 (12:24 +0200)]
Null-terminate tls-crypt client keys when testing
After generating a tls-crypt-v2 client key, OpenVPN will try to load the
generated key to verify that it was generated correctly. If the client
key is not written to disk but printed out on the command line, the PEM
encoded key is stored in memory and read_pem_key_file is called with
key_file_inline = true. However, this key is not a null-terminated
string, so we end up calling strlen on a buffer that isn't
null-terminated.
This commit adds a null-byte at the end of the key.
Change-Id: I2ca8bf90a796f2b757c2fde0ae24468ef3abc3b5 Signed-off-by: Max Fillinger <maximilian.fillinger@sentyron.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1701
Message-Id: <20260609102407.32590-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg37116.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Building a HARD_RESET reply was queueing the result into three
multi_context fields and deferring the send to the next event-loop
iteration, where multi_process_outgoing_link() flushed it:
1. hmac_reply_dest = &m->top.c2.from stored a pointer alias into
shared mutable state. Any subsequent read into m->top.c2.from
silently retargeted the pending reply to a different peer.
2. m->hmac_reply_ls = sock; at the top of multi_get_create_instance_udp()
was executed unconditionally for every UDP packet, including packets
that did not queue a reply. A stale queued reply could thus be sent
on the wrong listening socket.
3. hmac_reply.data pointed into m->top.c2.buffers->aux_buf (the only
slot). A second send_hmac_reset_packet() before the first flush
would overwrite the first reply's bytes.
These were latent on master because m->multi_io->udp_flags was consumed
and zeroed by the first event in each multi_io_process_io() loop, so at
most one UDP read ran per outer iteration.
Send the reply synchronously from within send_hmac_reset_packet() using
the sock that the read fired on (threaded through do_pre_decrypt_check).
The reply is small, stateless, and rate-limited by the existing
reflect_filter_rate_limit_check(); dropping on EAGAIN is acceptable
because the client retransmits its HARD_RESET. The three multi_context
fields and the deferred-flush block in multi_process_outgoing_link() are
gone; p2mp_iow_flags() no longer needs an IOW_TO_LINK branch for hmac
state.
Change-Id: I2df0fec786184b9fcf9b7c56c74816325cdb6942 Signed-off-by: Antonio Quartulli <antonio@mandelbit.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1702
Message-Id: <20260609075413.17380-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg37117.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
When port-share is used, enforce the presence
of a TCP listener by checking the local_list
entries insted of rely on the global
connection_entry proto field.
Github: fixes OpenVPN/openvpn#1027
Change-Id: Id4e21efebbe64b963cf7847ad77bc41339af7a37 Signed-off-by: Gianmarco De Gregori <gianmarco@mandelbit.com> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1680
Message-Id: <20260520101152.17453-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg36986.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Massively improve how we call cppcheck to cover
more code and identify more issues.
When specifying any -D argument all other defines
are ignored unless --force or --max-configs is
specified as well. I mistakenly assumed that this
was covered by --check-level=exhaustive. We need
to try finding a value for --max-configs so that
cppcheck doesn't spend hours scanning options.c
Add a library cfg for our code which for now
- identifies some printf-style functions
- adds some common macro defines
Use existing libraries.
Add a second call to cppcheck to separate the
Windows and Unixy code scans. This avoids some
very non-sensical define combinations.
Change-Id: I05720ccc3bcf706bbe62254afb74562580f5de56 Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1665
Message-Id: <20260607170713.4980-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg37078.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 5 Jun 2026 18:09:25 +0000 (20:09 +0200)]
Ensure pushed tun-mtu is no lower than TUN_MTU_MIN
The normal path ensure that the minimum tun mtu is set to at least
TUN_MTU_MIN. However, the pushed options path does not have this
restriction.
Check that the tun-mtu is within the limits of min/max mtu
in options.c. This ensure that the check is also correctly done
on the pushed variant.
Also add an extra check to keep the allowed payload for icmp6 packets
to be at least 64 bytes in the the block-ipv6 code path
(ipv6_send_icmp_unreachable) as extra layer of defence.
Pushing a low mtu like 1 and also block-ipv6 could trigger an
assertion in the ipv6_send_icmp_unreachable code path.
Lev Stipakov [Fri, 5 Jun 2026 14:18:02 +0000 (16:18 +0200)]
socket: assert buffer length before reading prepended sockaddr family
read_sockaddr_from_packet() inspected sa->sa_family before any check
on buf->len, so a short delivery from the dco-win driver would have
produced a garbage peer address from uninitialized buffer memory.
The driver always prepends a full sockaddr and validates the family
before writing, so reaching any of the size/family checks would mean
something is severely wrong on the driver side - assert the three
preconditions instead of M_FATAL'ing on them.
- Fix some uninitalised fields due to BUF_INIT_TRACKING
(found by cppcheck and the original reason for this
change).
- Fix "unused functions" if only BUF_INIT_TRACKING is
defined.
- Fix conversion error
Change-Id: I3ecb76d9022dcd7dae92eb5e9d62e5f018744883 Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1666
Message-Id: <20260513092251.28857-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg36901.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
dco_freebsd: Add check_malloc_return after realloc
cppcheck complained about a potential memleak
due to realloc failure. But trying to handle that
is probably not useful. Just abort like we do
for other malloc failures.
Change-Id: Icd8ea093dfe9f1888570f3d7b786b951b5262e47 Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1669
Message-Id: <20260507075321.25123-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg36842.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
t_client.sh: Do not use CA_CERT variable as indicator for good .rc file
t_client.sh doesn't actually care about this variable, and our
.rc files do not use it anymore. If you really want to dynamically
skip running t_client.sh just set TEST_RUN_LIST to empty in the .rc
file.
Change-Id: I7cf7146c3c4f1a01b0bcb57e03d09b32f8b59780 Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1664
Message-Id: <20260506145933.22301-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg36827.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
dns-scripts: Fix dnssec values in comments and Copyright statement format
- Fix the example value of dnssec to an actual valid one
- Fix the formatting of the Copyright statements to be
consistent with all other files in the project
Change-Id: Id6832e3f56420debc8b19d0144d53ca41abb678b Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1645
Message-Id: <20260430130354.25337-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg36800.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
- Updates GHA actions
- Switch clang-format job to archive: false supported
in new actions/upload-artifact version. This way the
file is not encapsulated in a zip
- Drop macos-14 builds. This runner is considered
deprecated and will be removed later this year.
Max Fillinger [Tue, 21 Apr 2026 05:53:50 +0000 (07:53 +0200)]
Mbed TLS: Error out if we have no valid tls-groups
Previously, when no valid groups were specified with the tls-groups
option, the Mbed TLS build of OpenVPN would start up and run, but fail
to complete a handshake, while the OpenSSL build would exit with an
error. This commit changes the behavior of the Mbed TLS build to match
the OpenSSL version.
Change-Id: Ica5f37e525c3812609021750ecd3986c1420e2a4 Signed-off-by: Max Fillinger <maximilian.fillinger@sentyron.com> Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1633
Message-Id: <20260421055357.21708-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg36699.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Commit b450414 added logic for reading password from management
when not in file or inline, but it was made conditional on
`response_from_stdin` which is always true! Fix by explicitly
checking for `password_from_stdin`.
Arne Schwabe [Fri, 10 Apr 2026 14:59:53 +0000 (16:59 +0200)]
Ensure that buffer of freed session are not used
In a race condition an old TLS session could still try to send a packet but
also get replaced by a new session. In this case, the buffer of the new
session is still referenced. Add the check_session_buf_not_used function
to mitigate this problem.
Also make the check if the to_link pointer is in one of the memory
regions a bit better even though this not make a difference with the
way we use these structs. But better safe than sorry.
A better solution to remove the TM_INITIAL state and handle reconnecting
session in their own complete tls_multi is a more involved fix that requires
a lot more refactoring.
CVE: 2026-40215 Reported-By: XlabAI Team of Tencent Xuanwu Lab (xlabai@tencent.com) Reported-By: Guannan Wang (wgnbuaa@gmail.com Reported-By: Zhanpeng Liu (pkugenuine@gmail.com) Reported-By: Guancheng Li (lgcpku@gmail.com) Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Change-Id: I7c5fa2a7a2563b7a8955d386411f3ceffe5b092f
Private-URL: https://github.com/OpenVPN/openvpn-private-issues/issues/112 Acked-by: Gert Doering <gert@greenie.muc.de> Signed-off-by: Gert Doering <gert@greenie.muc.de>
tls-crypt-v2: Avoid interpreting opcode as part of WKc
The buffer we pass to tls_crypt_v2_extract_client_key contains the
entire received control channel packet. We should skip the opcode before
trying to read WKC.
This logic error is a second bug behind the XlabAI finding, next too the
too-strict ASSERT in tls_crypt_unwrap.
Also remove a too strict ASSERT in tls_crypt_unwrap. We already check
a few lines later for a too short packet and return a proper error
("packet too short").
XlabAI found a way of triggering this ASSERT that requires a tls-crypt-v2
client key that has a specific property (a specific byte need to have a
specific value, about 1/256 probability). If an attacker can get hold of
such a tls-crypt-v2 client key or observe a handshake using such a key,
the attacker can trigger the ASSERT, crashing the server. Setups that do
not use tls-crypt-v2 are not affected.
Independently, Cisco Talos reported a way to trigger this ASSERT with any
tls-crypt-v2 key but this requires the attacker to be also in possession
of the private key part of the tls-crypt-v2 client key or to inject packet
into a live session of a client session.
CVE: 2026-35058 Reported-By: XlabAI Team of Tencent Xuanwu Lab (xlabai@tencent.com) Reported-By: Guannan Wang (wgnbuaa@gmail.com Reported-By: Zhanpeng Liu (pkugenuine@gmail.com) Reported-By: Guancheng Li (lgcpku@gmail.com) Reported-By: Emma Reuter of Cisco ASIG (TALOS-2026-2381) Signed-off-by: Steffan Karger <steffan@karger.me> Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Change-Id: I623733c0476c98f436d19009ee8990693c1579b5
Private-URL: https://github.com/OpenVPN/openvpn-private-issues/issues/111 Acked-by: Gert Doering <gert@greenie.muc.de> Signed-off-by: Gert Doering <gert@greenie.muc.de>
Inlined credentials: read missing password from management interface
When commit 39619b7fab added support for inlining username only,
fallback for password was from console. This is not ideal when
graphical UI is in use as there is no console. Instead, query the
management interface when possible.
This patch just extends a similar fix when username is read from
a file and password is missing. As before, any username read
from file or inlined is not peserved as we currently have no way
of locking the username in the management interface prompt.
David Benjamin [Thu, 16 Apr 2026 17:41:35 +0000 (19:41 +0200)]
ssl_openssl: Fix some CRL mixups
There are two ways to load CRLs in OpenSSL. They can be loaded at the
X509_STORE, shared across verifications, or loaded per verification at
the X509_STORE_CTX.
OpenVPN currently does the former. However, it also supports CRL
reloading, and tries to reload the CRL file before each connection.
OpenSSL does not really have a good way to unload objects from an
X509_STORE. OpenVPN currently does it by grabbing the
STACK_OF(X509_OBJECT) out of the X509_STORE and manually deleting all
the CRLs from it.
This mutates an OpenSSL internal object which bumps into problems if
OpenSSL ever switches to a more efficient representation. See
https://github.com/openssl/openssl/pull/28599
(It's also not thread-safe, though it doesn't look like that impacts
OpenVPN? Actually even reading that list doesn't work. See
CVE-2024-0397. This OpenSSL API was simply broken.)
Additionally, this seems to cause two OpenVPN features to not work
together. I gather backend_tls_ctx_reload_crl is trying to clear the
CRLs loaded from last time it ran. But tls_ctx_load_ca with a ca_file
can also load CRLs. tls_ctx_load_ca with ca_path will also pick up CRLs
and backend_tls_ctx_reload_crl actually ends up clobbering some state
X509_LOOKUP_hash_dir internally maintains on the X509_STORE. Likewise,
tls_verify_crl_missing can get confused between
backend_tls_ctx_reload_crl's crl_file-based CRLs and CRLs from
tls_ctx_load_ca.
Avoid all this by tracking the two CRLs separately. crl_file-based CRLs
now go onto a STACK_OF(X509_CRL) tracked on the tls_root_ctx. Now this
field can be freely reloaded by OpenVPN without reconfiguring OpenSSL.
Instead, pass the current value into OpenSSL at verification time. To
do so, we need to use the SSL_CTX_set_cert_verify_callback, which allows
swapping out the X509_verify_cert call, and also tweaking the
X509_STORE_CTX configuration before starting certificate verification.
Context: SSL_CTX_set_cert_verify_callback and the existing
verify_callback are not the same. SSL_CTX_set_cert_verify_callback wraps
the verification while verify_callback is called multiple times
throughout verification. It's too late to reconfigure X509_STORE_CTX in
verify_callback. verify_callback is usually not what you want.
Sometimes current_cert and error_depth don't quite line up, and
cert_hash_remember may end up called multiple times for a single
certificate.
I suspect some of the other verify_callback logic would also be better
done in the new callback, but I've left it alone to keep this change
minimal. verify_callback is really only usable for suppressing errors.
Application bookkeeping is better down elsewhere.
Add .clang-format section for STACK_OF since we otherwise format the
line as STACK_OF(X509_CRL) * crls
Github: see also openssl/openssl#28599 Signed-off-by: David Benjamin <davidben@google.com>
Change-Id: I31ac2a763209114267c35c4a9182a12d8d82f6fe Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org> Acked-by: MaxF <max@max-fillinger.net>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1289
Message-Id: <20260416174142.28918-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg36641.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
peer_id is mostly this already (except in DCO
context for some reason), and max_peerid was
defined as uint32_t as well. So changing max_clients
to uint32_t avoids many -Wsign-compare warnings.
While here fix limit for max_clients in options
parsing. It is not allowed to be MAX_PEER_ID
exactly.
Change-Id: I8d6b7bc1b7744dc6d57aaed3231b8901275752f2 Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1564
Message-Id: <20260407112434.5588-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg36535.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 13 Mar 2026 10:49:55 +0000 (11:49 +0100)]
Remove multi_context->iter
The multi_context->iter is basically a hash with only one bucket. This makes
m->iter a linear list. Instead of maintaining this extra list use
m->instances instead. This is a fixed sized continuous array, so iterating
over it should be very quick. When the number of connected clients
approaches max_clients, iterating over a static array should be faster than
a linked list, especially when considering cache locality.
Of the several places where m->iter is used only one is potentially on a
critical path: the usage of m->iter in multi_bcast.
However this performance difference would be only visible with a lightly
loaded server with very few clients. And even in this scenario I could
not manage to measure a difference.
Change-Id: Ibf8865e451866e1fffc8dbc8ad5ecf6bc5577ce4 Signed-off-by: Arne Schwabe <arne-openvpn@rfc2549.org> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1556
Message-Id: <20260313104955.16748-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg36087.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Luca Boccassi [Mon, 30 Mar 2026 18:08:54 +0000 (20:08 +0200)]
management: add base64 multi-line input for passwords
Allow management clients to send long passwords via the
usual multi-line base64 encoded protocol.
A client declares MCV 5 support and sends a 'password <type>'
line, followed by as many lines (each up to 1024 bytes) as
needed, in base64 encoded format, terminated by 'END'.
This is useful when a password is a JIT-generated use-once
token.
test_packet_id: Add a check after malloc to ensure value is valid
cppcheck complains about a potential null pointer dereference
in reliable_get_num_output_sequenced_available. That is mostly
theoretical, but still add a check.
Change-Id: I64da2328591ef2b9ee7502e574c878651cdf356a Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1621
Message-Id: <20260406074729.29903-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg36516.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
By default the first enum value is 0.
But we check whether we set the flag by doing
BOOL dnssec = (msg->flags & nrpt_dnssec) != 0;
This can't ever be true.
Found by cppcheck.
Change-Id: Iff5be978817bfc0cd4d78818e7be7b90bad71f3c Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1606
Message-Id: <20260405102209.31528-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg36487.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
projects / thirdparty / openvpn.git / log
