Lev Stipakov [Tue, 24 Dec 2024 17:42:33 +0000 (18:42 +0100)]
repair DNS address option
Commit
6f2d222 ("dns: store IPv4 addresses in network byte order")
changed the internal representation of IPv4 address within DNS
settings to network byte order, however later this value is copied into
tuntap_options, where IPv4 addresses are assumed to be in host byte
order (see lots of occurences of "htonl(tt->" in tun.c). As a
consequence, DNS server address is set incorrectly, like 4.4.8.8 instead
of 8.8.4.4
Fix by converting address to host byte order when copying from DNS
options to tuntap_options.
Change-Id: I87e4593e6a548bacd40b840cd241950019fa457d Signed-off-by: Lev Stipakov <lev@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20241224174233.13005-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg30195.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Sun, 22 Dec 2024 21:45:41 +0000 (22:45 +0100)]
Move initialisation of implicit IVs to init_key_ctx_bi methods
This is really more a function of initialising the data cipher and key
context and putting it into the init_key_ctx_bi makes more sense.
It will allow calling init_key_ctx_bi to fully initialise a
data channel key without calling some extra functions after that
which will make the (upcoming) epoch key implementation cleaner.
Also ensure that free_ctx_bi actually also sets initialized to false.
Change-Id: Id223612c7bcab91d49c013fb775024bd64ab0836 Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20241222214541.11021-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg30170.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Sat, 21 Dec 2024 22:39:05 +0000 (23:39 +0100)]
Split init_key_ctx_bi into send/recv init
This allows for only initialising one of the keys. This is needed
for epoch keys where key rotation of send/recv key can happen at
different time points.
Change-Id: If9e029bdac264dcc05b2d256c4d323315904a92b Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20241221223905.18820-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg30151.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Heiko Hund [Sat, 21 Dec 2024 22:41:36 +0000 (23:41 +0100)]
service: add utf8to16 function that takes a size
utf8to16_size() takes the size of the to be converted string. This is
needed to convert MULTI_SZ strings, which contain inline NUL characters,
but can be useful in other cases as well.
Change-Id: I6b4aa3d63c0b684bf95841271c04bc5d9c37793b Signed-off-by: Heiko Hund <heiko@ist.eigentlich.net> Acked-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20241221224136.20984-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg30158.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Sat, 21 Dec 2024 15:37:30 +0000 (16:37 +0100)]
Trigger renegotiation of data key if getting close to the AEAD usage limit
This implements the limitation of AEAD key usage[1] with a confidentiality
margin of 2^-57, the same as TLS 1.3. In this implementation, unlike
TLS 1.3 that counts the number of records, we count the actual number of
packets and plaintext blocks. TLS 1.3 can reasonable assume that for
large data transfers, full records are used and therefore the maximum
record size of 2**14 (2*10 blocks) is used to calculate the number of
records before a new key needs to be used.
For a VPN like OpenVPN, the same calculation would either require using a
pessimistic assumption of using a MTU size of 65k which limits us to
2^24 packets, which equals only 24 GB with more common MTU/MSS of 1400
or requiring a dynamic calculation which includes the actual MTU that
we allow to send. For 1500 the calculation yields 2*29.4 which is a
quite significant higher number of packets (923 GB at 1400 MSS/MTU).
To avoid this dynamic calculation and also avoid needing to know the
MSS/MTU size in the crypto layer, this implementation foregoes the
simplification of counting just packets but will count blocks and packets
instead and determines the limit from that.
This also has the side effect that connections with a lot of small packets
(like TCP ACKs) mixed with large packets will be able to keep using the same
key much longer until requiring a renegotiation.
This patch will set the limit where to trigger the renegotiation at 7/8
of the recommended maximum value.
The easiest way to test if this patch works as
intended is to manually change the return value of cipher_get_aead_limits
to some silly low value like 2048. After a bit of VPN traffic, a soft
reset should occur that indicates being over the
Heiko Hund [Fri, 13 Dec 2024 16:45:52 +0000 (17:45 +0100)]
dns: store IPv4 addresses in network byte order
This is done so that inet_ntop(3) can be used with IPv4 name server
addresses. It expects the binary address in network byte order. If they
are not that way the address octets are reversed.
Change-Id: I81d4bb0abdd421f5ba260c10c610918652334a4d Signed-off-by: Heiko Hund <heiko@ist.eigentlich.net> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20241213164552.265863-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg30111.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Thu, 12 Dec 2024 14:38:45 +0000 (15:38 +0100)]
Use XOR instead of concatenation for calculation of IV from implicit IV
This change prepares the extended packet id data where also the packet id
part of the IV will be derived using xor. Using xor also in the AEAD
case where this degenerates to a concatenation allows using the same
IV generation code later.
Change-Id: I74216d776d3e0a8dc987ec7b1671c8e8dcccdbd6 Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: MaxF <max@max-fillinger.net> Acked-by: Antonio Quartulli <antonio@mandelbit.com> Acked-by: Steffan Karger <steffan@karger.me> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20241212143845.4090-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg30097.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
forward: Fix potential unaligned access in drop_if_recursive_routing
ASAN error:
forward.c:1433:13: runtime error: member access within misaligned
address 0x51e00002f52e for type 'const struct in6_addr', which
requires 4 byte alignment
replace IN6_ARE_ADDR_EQUAL() which uses 32bit compares on Linux - alignment
sensitive - with our own OPENVPN_IN6_ARE_ADDR_EQUAL() macro, which always
does memcpy() and does not care for alignment.
v2: Use memcmp instead of memcpy
Change-Id: I74a9eec4954f3f9d208792b6b34357571f76ae4c Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20241211171349.8892-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg30074.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
corubba [Sat, 7 Dec 2024 23:17:05 +0000 (00:17 +0100)]
Fix IPv6 in port-share journal
getpeername() and getsockname() will truncate the result if it is
larger than the passed-in length. Because here always the size of the
`sa` IPv4 union member was passed in, all larger (aka IPv6) results
were truncated. Instead use the size of the `addr` union, which is the
maximum size of all union members.
Note: the full functionality of these changes depends on
https://review.haiku-os.org/c/haiku/+/8592
Signed-off-by: Alexander von Gluck <alex@terarocket.io> Acked-by: Gert Doering <gert@greenie.muc.de>
Change-Id: I1a22724f28c5cd47f6df178b49f44087d7c2b6fd
Message-Id: <20241205172459.4783-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg30023.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
* Special thanks to Sean Brady's hard work in GSoC 2023 towards creating
a TUN/TAP driver for Haiku!
* More kudos to Augustin Cavalier for making it functional :-)
Signed-off-by: Alexander von Gluck <alex@terarocket.io> Acked-by: Gert Doering <gert@greenie.muc.de>
Change-Id: I9a278374f492a538f0c174ced1746c3b1f82b8c9
Message-Id: <20241128101538.12810-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29947.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
We can't disable compression support on receive because
that would break too many configurations out there. But
we can remove the support for compressing outgoing traffic,
it was disabled by default anyway.
Makes "--allow-compression yes" an alias for
"--allow-compression asym" and removes all resulting dead code.
Change-Id: I402ba016b75cfcfec4fc8b2b01cc4eca7e2bcc60 Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20241108173851.436-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29718.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Mon, 4 Nov 2024 08:58:08 +0000 (09:58 +0100)]
send uname() release as IV_PLAT_VER= on non-windows versions
This is highly system specific, as the content of the uname()
structure elements is not specified very well - uname(3) says:
release Release level of the operating system
which translates to "IV_PLAT_VER=13.3-RELEASE-p6" (FreeBSD) or
"IV_PLAT_VER=22.6.0" (macOS) - the latter being the "Mach Kernel
version", not what Apple calls the OS.
It's still useful if a server operator needs to keep track of
client versions (and the GUI does not set the corresponding
environment variable, which neither Tunnelblick nor NM do).
Rémi Farault [Tue, 29 Oct 2024 11:06:35 +0000 (12:06 +0100)]
Add calls to nvlist_destroy to avoid leaks
Some memory leaks were detected by valgrind on the openvpn daemon, using
DCO mode on a FreeBSD platform. The leaks are caused by missing
nvlist_destroy calls in the file dco_freebsd.c.
Calls to nvlist_destroy were added, sometimes using local variables to
store nvlist pointers temporarly. A valgrind run on the updated daemon
confirmed that the leaks were gone.
Arne Schwabe [Mon, 28 Oct 2024 13:55:04 +0000 (14:55 +0100)]
Refuse clients if username or password is longer than USER_PASS_LEN
When OpenVPN is compiled without PKCS11 support USER_PASS_LEN is 128
bytes. If we encounter a username larger than this length, we would
only read the 2 bytes length header of the username/password. We did
then also NOT skip the username or password field meaning that we would
continue reading the rest of the packet at the wrong offset and get
garbage results like not having peerinfo and then rejecting a client
because of no common cipher or missing data v2 support.
This will tell the client that username/password is too regardless
of whether password/username authentication is used. This way we
do not leak if username/password authentication is active.
To reproduce this issue have the server compiled with a USER_PASS_LEN
set to 128 (e.g. without pkcs11 or manually adjusting the define) and
have the client with a larger USER_PASS_LEN to actually be able to
send the larger password. The server must also be set to use only
certificate authentication while the client must use certificates
and auth-user-pass because otherwise the user/pass verification will
reject the empty credentials.
Using the openvpn3 test client with overlong username/password also
works.
Change-Id: I60f02c919767eb8f1b95253689a8233f5f68621d Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20241028135505.28651-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29675.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Samuli Seppänen [Thu, 24 Oct 2024 13:32:17 +0000 (15:32 +0200)]
t_server_null: persist test log files
The goal is to help debug issues with t_server_null. The immediate goal
is to be able to debug server startup issues encountered on some of the
*BSD platforms.
Change-Id: I49f1e7d25edb62bf202ffceb45dedc213f2eafdd Signed-off-by: Samuli Seppänen <samuli.seppanen@gmail.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20241024133220.4864-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/search?l=mid&q=20241024133220.4864-1-gert@greenie.muc.de
URL: https://gerrit.openvpn.net/c/openvpn/+/776 Signed-off-by: Gert Doering <gert@greenie.muc.de>
io_work: pass event_arg object to event handler in case of socket event
In order to allow the code to work with multiple listening sockets
it is essential to allow the generic multi_io event handler
to distinguish between the various socket objects.
This can be achieved by passing an event_arg object that contains
a pointer to the link_socket.
This code path is used on clients as well as UDP servers.
Change-Id: I7ebf0d4fb2a23278e16003b2e35598178155d658 Signed-off-by: Antonio Quartulli <a@unstable.cc> Signed-off-by: Gianmarco De Gregori <gianmarco@mandelbit.com>
Message-Id: <20241023142030.731-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29625.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Instead of passing the shift argument as pointer, pass
directly its integer value. This will allow the code to
distinguish a shift value from a real object pointer,
like we already do in multi_tcp_process_io().
This change will allow us later to pass an event_arg
object as event handler argument instead of a simple
integer value.
Change-Id: Ib583bf17e35b14aed78fd8217b6e71e8c2b78089 Signed-off-by: Antonio Quartulli <a@unstable.cc> Signed-off-by: Gianmarco De Gregori <gianmarco@mandelbit.com> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20241023084208.12317-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29604.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
In order to prepare the code to work with distinct sockets,
it is essential that i/o functions do not operate on any
hard-coded socket object (i.e. c->c2.link_socket).
This patch changes all the low-level i/o functionis to work
with a socket specified as argument rather than a fixed one.
Change-Id: I8eae2d3356bbcc5d632eeb4fbe80de8009d9b40d Signed-off-by: Antonio Quartulli <a@unstable.cc> Signed-off-by: Gianmarco De Gregori <gianmarco@mandelbit.com> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20241023083444.27951-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29603.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Commit cd8e25a6e9 removed a variable because it looked as if
used only once anyway (1 assignment, 1 usage) - overlooking that
on _WIN32 it's changed to NULL, which wasn't adjusted...
This fix restores the wiped out "unsigned int *persistent" in
multi_tcp_wait(), undoing this particular change of the previous
commit.
Change-Id: I8526aadb5151ddc997c836d5a691bcdfee700938 Signed-off-by: Gianmarco De Gregori <gianmarco@mandelbit.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20241023113923.7420-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29612.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
event/multi: add event_arg object to make event handling more generic
In order to prepare the event handling code to deal with multiple
listening sockets, we have to make sure that it is possible to
distinguish which of these sockets have been poked by an incoming
connection request.
To achieve that, this patch changes the object being passed as
event handler argument, from a "partly integer-evaluated variable"
to a full struct with a proper type attribute.
This struct will allow the code to carry around the particular
listening socket where the connection is being established.
This change affects the TCP server code path only as UDP servers
use only one socket to handle all clients.
Change-Id: Icd7f6a2ad350cdc2312b3e80fa0dbdd7e4311d2e Signed-off-by: Antonio Quartulli <a@unstable.cc> Signed-off-by: Gianmarco De Gregori <gianmarco@mandelbit.com> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20241023080853.3710-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29602.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 18 Oct 2024 06:31:23 +0000 (08:31 +0200)]
Remove unused methods write_key/read_key
These were used in the key-method 1 that we remove by commit 36bef1b52 in 2020. That commit unfortunately missed that these
methods were only used for directly sending/receiving key material
over the control channel.
Change-Id: Ib480e57b62ea33f2aea52bee895badaf5607b72d Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20241018063123.11631-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29595.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 18 Oct 2024 06:37:17 +0000 (08:37 +0200)]
Remove a large number of unused structs and functions
These have been found by Clion's Inspect Code functionality and have
been verified by hand. A few functions like buf_read_u32 have been
kept since they still feel being useful while currently not being used.
Change-Id: I0d96ee06c355c6a5ce082af23921e329d3efae33 Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20241018063717.14629-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29594.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Steffan Karger [Thu, 17 Oct 2024 06:49:55 +0000 (08:49 +0200)]
Improve data channel crypto error messages
* Make decryption error messages better understandable.
* Increase verbosity level for authentication errors, because those can
be expected on bad connections.
For --dev null or --dev-type af_unix:lwipopenvn tests, there will be
no visible change to ifconfig or route output, so tests will fail
("how can this be?"). Set EXPECT_IFCONFIG4_<n>=- to skip this
check.
(Simply leaving both EXPECT_IFCONFIG* vars empty and using that as
trigger would interfere with the magic from commit df0b00c25)
v2: fix string-equal comparison
Change-Id: Iec1953415afb53755488dd44407568e72d28e854 Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20240928200508.23747-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29473.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Wed, 25 Sep 2024 15:11:04 +0000 (17:11 +0200)]
Remove null check after checking for checking for did_open_tun
If we indicate that the tun device has been opened the c1.tuntap struct
is guaranteed to be defined. This extra null check is something that
Coverity flags as we access a do a null check after already accessing fields
of tuntap
Change-Id: I9966636163c7dfa208d26f1cadbf5b81937f3a34 Signed-off-by: Arne Schwabe <arne-openvpn@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20240925151104.13036-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29447.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Wed, 25 Sep 2024 15:13:42 +0000 (17:13 +0200)]
Fix check for CMake not detecting struct cmsg
This check seems to have never worked and on Linux we hard coded
the check instead. Remove this hard coded assumption and use a
working check based on check_struct_has_member instead.
This is has the side-effect of port-sharing being enabled on macOS
when compiled with cmake.
Change-Id: Ia020c696f63a2a317f001c061b2ab4da69977750 Signed-off-by: Arne Schwabe <arne-openvpn@rfc2549.org> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20240925151342.13307-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29448.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Wed, 25 Sep 2024 06:30:16 +0000 (08:30 +0200)]
Ensure that the AF_UNIX socket pair has at least 65k of buffer space
Without this change, pinging a lwipovpn client with something like a
3000 byte payload on macOS often fails as the default buffer sizes on
macOS are 2048 for send and 4096 for receive.
Change-Id: Ice015df81543c01094479929f0cb3075ca4f3813 Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20240925063016.22532-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29413.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Tue, 24 Sep 2024 11:01:29 +0000 (13:01 +0200)]
Introduce DRIVER_AFUNIX backend for use with lwipovpn
lwipovpn is a using lwip TCP/IP implementation with an AF_UNIX
implementation to emulate a tun/tap device without messing with the
TCP/IP stack of the host.
For more information about lwipovpn see https://github.com/OpenVPN/lwipovpn
Change-Id: I65099ef00822d08fd3f5480c80892f3bf86c56e7 Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20240924110130.3910-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29379.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Sun, 22 Sep 2024 20:37:49 +0000 (22:37 +0200)]
Move to common backend_driver type in struct tuntap
With the introduction of utun on macOS and DCO on Linux, FreeBSD and
Windows, a lot of platforms have now more than one driver/backend for
the tun interface but each one uses a different mechanism. Unify these
approach with using a common enum that defines the driver_type.
Change-Id: I8c0e9f32b235cb262ca2be6aac8d520e49b30d74 Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20240922203749.9802-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29352.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Wed, 18 Sep 2024 16:29:17 +0000 (18:29 +0200)]
make t_server_null 'server alive?' check more robust
- use "$RUN_SUDO kill -0 $pid" to test if a given process is running, not
"ps -p $pid" - the latter will not work if security.bsd.see_other_uids=0
is set
- produce proper error messages if pid files can not be found or are
empty at server shutdown time
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20240918162917.6809-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29314.html
socket: Change return types of link_socket_write* to ssize_t
This is the actual return value of send/sendto/sendmsg.
We will leave it to the single caller of link_socket_write()
to decide how to map that to the int buffer world. For now
just cast it explicitly.
Change-Id: I7852c06d331326c1dbab7b642254c0c00d4cebb8 Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20240918204844.2820-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29323.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
- Increase required version to 2.60 since that is documented
minimum version for AC_USE_SYSTEM_EXTENSIONS
- Remove obsolete macros AC_C_CONST and AC_C_VOLATILE. They
are noops on every compiler we support.
- Add explicit call to AC_PROG_CC. We get this implictly as
dependency of other macros, but it is nicer to have it
explicitely as well.
- A few typo and whitespace fixes.
Change-Id: I7927a572611b7c1dc0b522fd6cdf05fd222a852d Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20240918204551.2530-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29321.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 19 Jul 2024 13:10:16 +0000 (15:10 +0200)]
Avoid SIGUSR1 to SIGHUP remapping when the configuration is read from stdin
If the configuration is read from stdin, we cannot reread the configuration
as stdin provides the configuration only once. So whenever we hit the
"close_context usr1 to hup" logic, the OpenVPN process will fail as tries
to restart with an empty configuration.
While OpenVPN tries to block USR1 from normal unix signal, I have observed
cases in my app which sends USR1 from management interface where the
CC_HARD_USR1_TO_HUP logic is trigger and breaking the OpenVPN process.
Change-Id: Icfc179490d6821e22d14817941fb0bad667c713f Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20240719131016.75042-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28941.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Implemented a safeguard to verify the returned value
from add_route3() when the default gateway is not a local
remote host.
Prior to this implementation, RT_DID_LOCAL flag was
erroneously set even in case of add_route3() failure.
This problem typically occurs when there's no default
route and the --redirect-gateway def1 option is specified,
and in case of reconnection makes it impossible for the client
to reobtain the route to the server.
This fix ensures OpenVPN accurately deletes the appropriate
route on exit by properly handling add_route3() return value.
Trac: #1457
Change-Id: I8a67b82eb4afdc8d82c5a879c18457b41e77cbe7 Signed-off-by: Gianmarco De Gregori <gianmarco@mandelbit.com> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20240221111814.942965-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28290.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
dco: mark peer as deleted from kernel after receiving CMD_DEL_PEER notification
some extra DCO calls may be made after receiving the DEL_PEER
notification (i.e. due to timeout), but this will result in
an error message due to the peer having disappeared already.
An extra call might be, for example, an explicit DEL_PEER
in the attempt of cleaning the peer state.
For this reason, inform userspace that there is no peer in
kernel anymore and prevent errors which may result confusing.
Change-Id: Ife50e37cd49d55ec81a70319a524ffeaf0625a56 Signed-off-by: Antonio Quartulli <antonio@mandelbit.com> Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Message-Id: <20240912165339.21058-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29226.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Wed, 10 Jul 2024 16:02:38 +0000 (18:02 +0200)]
Remove check for anonymous unions from configure and cmake config
Anonymous unions/structs are technically a custom GNU C99 feature but
was already widely supported by other compilers. With C11 this feature
has become a standard feature so all compilers nowadays support it.
Change-Id: I1ef5f6f21f0135a628a63553c39515fa4549ce87 Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20240710160238.190189-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28914.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Ensures all params are ready before invoking dco_set_peer()
In UDP case, on a p2mp server, dco_set_peer() is currently called
at the wrong time since the mssfix param is calculated later on in
tls_session_update_crypto_params_do_work().
Move the dco_set_peer() inside tls_session_update_crypto_params_do_work(),
and remove p2p_set_dco_keepalive() to avoid calling dco_set_peer()
twice on the client side.
This way, we'll ensure that all crypto and frame params are properly
initialized and if an update occurs DCO will be notified.
Change-Id: Ic8538e734dba53cd43fead3961e4401c8037e079 Signed-off-by: Gianmarco De Gregori <gianmarco@mandelbit.com> Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20240906145745.67596-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29086.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Using "tun" as the variable name for the return of
is_tun_p2p is probably a historical accident. But
it has actual consequences in that the other code
often seems to assume that it does less checks
than it actually does.
Use "tun_p2p" as the variable name and remove checks
that are not required. Also use is_tun_p2p in more
places.
Change-Id: Ice8b95f953c3f7e71657a78ea12b02a08c60aa67 Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Message-Id: <20240906162514.78671-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29091.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Ilia Shipitsin [Mon, 8 Jul 2024 21:08:18 +0000 (23:08 +0200)]
src/openvpn/init.c: handle strdup failures
Signed-off-by: Ilia Shipitsin <chipitsine@gmail.com> Acked-by: Antonio Quartulli <a@unstable.cc> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20240708210912.566-2-chipitsine@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28884.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Extend "--static-challenge" option to take a third
argument (= scrv1 or concat) to specify that the password and
response should be concatenated instead of using the
SCRV1 protocol. If unspecified, it defaults to "scrv1"
meaning that the SCRV1 protocol should be used.
v2: use scrv1|concat instead of 0|1 as option argument
fix typos
v3: improve and correct documentation in management-notes.txt
Change-Id: I59a90446bfe73d8856516025a58a6f62cc98ab0d Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20240719131407.75746-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28943.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Add a test for loading certificate and key to ssl context
The test certificate used in test_ssl.c is updated to use 2048 bit
RSA and the matching key is added.
Tests include loading certificate and key as inlined pem as well as
from files. Note that loading the key also checks that it matches
the certificate, providing an indirect test that the latter was loaded
correctly.
Change-Id: Ic6f089896191145f68ce9a11023587d05dcec4d8 Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20240906103814.36839-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29074.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
OpenSSL 3 has providers which can load keys and certificates
from various key stores and HSMs using a provider-specific URI.
While certificates are generally exportable, and some providers
support a PEM file that acts as a proxy for non-exportable private
keys, not all providers are expected to do so. A generic capability
to read keys and certificates from URIs appears useful.
This patch does this by extending the scope of the argument for
"--key" and "--cert" options to include URIs. Many of OpenSSL 3
utilities also work the same way: e.g., the "-in" option for
"openssl pkey" or "openssl x509" could be a filename or URI.
Other applications have started emulating this behaviour:
e.g., pkcs11: URI works as an alternative to a file name for
certificates and keys in apache. Even for files, this has a nice
side effect that non-PEM files get transparently parsed. E.g., a
pkcs12 file could be used in place of a PEM file without needing
any extra options.
This is backward compatible as OpenSSL falls back to treating URIs
with no scheme or unrecognized scheme as file names.
Parsing of inlined keys and certificates is unchanged (those
should be in PEM format).
Specification of URIs that OpenSSL accepts depends on the
providers that support them. Some are standard URIs such as
"file:/path", but providers may support non-standard URIs
with arbitrary scheme names. OpenSSL by itself recognizes
only file URI. However, the implementation is agnostic to the
URI specification as parsing is done by the provider that supports
the URI. A new URI gets automatically recognized when the provider
that supports it is loaded.
Below are some usage examples:
Relative or absolute path to a file or as a URI "file:/absolute/path":
--key mykey.pem (same as what is currently supported)
--key file:/path/to/mykey.pem
--cert file:/path/to/mycert.pem
Other file types supported by OpenSSL would also work:
--key client.p12
--cert client.p12
pkcs11-provider supports "pkcs11:" URI (RFC 7512):
Protect cached username, password and token on client
Keep the memory segment containing username and password in
"struct user_pass" encrypted. Works only on Windows.
Username and auth-token cached by the server are not covered
here.
v2: Encrypt username and password separately as it looks more
robust. We continue to depend on the username and password buffer
sizes to be a multiple of CRYPTPROTECTMEMORY_BLOCK_SIZE = 16,
which is the case now. An error is logged if this is not the case.
v3: move up ASSERT in auth_token.c
Change-Id: I42e17e09a02f01aedadc2b03f9527967f6e1e8ff Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20240906112908.1009-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29079.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Usage of credentials is a bit odd in this file.
Actually the copy of "struct user_pass" kept in p->up is not
required at all. It just defeats the purpose of auth-nocahe
as it never gets cleared.
Removing it is beyond the scope of this patch -- we just ensure
it's purged after use.
Change-Id: Ic6d63a319d272a56ac0e278f1356bc5241b56a34 Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20240905100724.4105-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29061.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Wed, 14 Feb 2024 13:27:19 +0000 (14:27 +0100)]
Implement support for AEAD tag at the end
Using the AEAD tag at the end is the standard way of doing AEAD. Several
APIs even only support the tag at the end (e.g. mbed TLS). Having the tag at
the front or end makes no difference for security but allows streaming HW
implementations like NICs to be much more efficient as they do not need to
buffer a whole packet content and encrypt it to finally write the tag but
instead just add the calculated tag at the end of processing.
Change-Id: I00821d75342daf3f813b829812d648fe298bea81 Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20240214132719.3031492-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28239.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Lev Stipakov [Fri, 9 Aug 2024 19:22:56 +0000 (21:22 +0200)]
Use a more robust way to get dco-win version
The current way doesn't work if the device is already in use.
Starting from 1.3.0, dco-win creates a non-exclusive
control device \\.\ovpn-dco-ver which can be opened by
multiple apps and supports a single IOCTL to get
a version number.
https://github.com/OpenVPN/ovpn-dco-win/pull/76
This will be expecially handy later when checking which
features driver supports.
Change-Id: Ieb6f3a9d14d76000c1caf8ee1e959c6d0de832bf Signed-off-by: Lev Stipakov <lev@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20240809192257.24208-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29009.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Selva Nair [Mon, 12 Aug 2024 23:21:58 +0000 (19:21 -0400)]
test_pkcs11.c: set file offset to 0 after ftruncate
Currently key and cert file fd's are reused after ftruncate()
without setting the offset to zero. This causes subsequent
data to be written at some finite offset with the hole in
the file automatically filled by zeros. Fix it by calling
lseek() to set the offset to zero.
The test works nevertheless because p11tool seem to generously
ignore any junk before the "BEGIN" marker.
Change-Id: Ib0fe15a4ba18d89216b0288e6cd6be66ed377bd4 Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20240812232158.3776869-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg29010.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Incompatible changes to the --dns server address and --dns server
exclude-domains options were introduced after the code for handling them
was released. Add and send a new IV_PROTO flag, so servers which act on
the flags set can differentiate between clients which have implemented
--dns and those which just support the new option. This enables them to
decide which variant of options to send to the client.
Change-Id: I975057c20c1457ef88111f8d142ca3fd2039d5ff Signed-off-by: Heiko Hund <heiko@ist.eigentlich.net> Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Message-Id: <20240725112248.21075-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28970.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Wed, 10 Jul 2024 14:06:23 +0000 (16:06 +0200)]
Allow trailing \r and \n in control channel message
Writing a reason from a script will easily end up adding extra \r\n characters
at the end of the reason. Our current code pushes this to the peer. So be more
liberal in accepting these message.
Github: closes OpenVPN/openvpn#568
Change-Id: I47c992b6b73b1475cbff8a28f720cf50dc1fbe3e Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20240710140623.172829-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28910.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
- exit after a timeout if unable to kill servers
- use sudo or equivalent only for server stop/start
- use /bin/sh directly instead of through /usr/bin/env
- simplify sudo call in the sample rc file
- remove misleading and outdated documentation
- make it work on OpenBSD 7.5
- make it work on NetBSD 10.0
- make server logs readable by normal users
Change-Id: I2cce8ad4e0d262e1404ab1eb6ff673d8590b6b3a Signed-off-by: Samuli Seppänen <samuli.seppanen@gmail.com> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20240704133337.26595-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28871.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Max Fillinger [Wed, 3 Jul 2024 17:41:58 +0000 (19:41 +0200)]
mbedtls: Warn if --tls-version-min is too low
Recent versions of mbedtls only support TLS 1.2. When the minimum
version is set to TLS 1.0 or 1.1, log a warning and use 1.2 as the
actual minimum version.
Change-Id: Ibc641388d8016533c94dfef3618376f6dfa91f4e Signed-off-by: Max Fillinger <maximilian.fillinger@foxcrypto.com> Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Message-Id: <20240703174158.7137-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28865.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
v2:
- simplify code by removing -llzo special handling
v3:
- reintroduce support for autodetection without pkg-config,
no need to break backwards compatibility right now
v7:
- Handle case correctly where lzo/lzo1x.h can not be included
at all. On most distros this works even though the .pc
file suggests to use it without. We had some partly
solution for that but it wasn't really working.
v8:
- Handle systems that do not implicitly include limits.h
in configure test builds.
lzodefs.h usually relies on lzoconf.h to include it.
Change-Id: I1c038dc4ec80d3499582d81eee61fee74f26e693 Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20240626161921.179301-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28848.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Caching proxy credentials was not working due to the
lack of handling already defined creds in get_user_pass(),
which prevented the caching from working properly.
Fix this issue by getting the value of c->first_time,
that indicates if we're at the first iteration
of the main loop and use it as second argument of the
get_user_pass_http(). Otherwise, on SIGUSR1 or SIGHUP
upon instance context restart credentials would be erased
every time.
The nocache member has been added to the struct
http_proxy_options and also a getter method to retrieve
that option from ssl has been added, by doing this
we're able to erase previous queried user credentials
to ensure correct operation.
Fixes: Trac #1187 Signed-off-by: Gianmarco De Gregori <gianmarco@mandelbit.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Change-Id: Ia3e06c0832c4ca0ab868c845279fb71c01a1a78a Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20240623200551.20092-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28835.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Lev Stipakov [Wed, 19 Jun 2024 14:46:08 +0000 (17:46 +0300)]
interactive.c: Improve access control for gui<->service pipe
At the moment everyone but anonymous are permitted
to create a pipe with the same name as interactive service creates,
which makes it possible for malicious process with SeImpersonatePrivilege
impersonate as local user.
This hardens the security of the pipe, making it possible only for
processes running as SYSTEM (such as interactive service) create the
pipe with the same name.
While on it, replace EXPLICIT_ACCESS structures with SDDL string.
CVE: 2024-4877
Change-Id: I35e783b79a332d247606e05a39e41b4d35d39b5d
Reported by: Zeze with TeamT5 <zeze7w@gmail.com> Signed-off-by: Lev Stipakov <lev@openvpn.net> Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20240619144629.1718-2-lev@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28808.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Mon, 27 May 2024 13:02:41 +0000 (15:02 +0200)]
Properly handle null bytes and invalid characters in control messages
This makes OpenVPN more picky in accepting control message in two aspects:
- Characters are checked in the whole buffer and not until the first
NUL byte
- if the message contains invalid characters, we no longer continue
evaluating a fixed up version of the message but rather stop
processing it completely.
Previously it was possible to get invalid characters to end up in log
files or on a terminal.
This also prepares the logic a bit in the direction of having a proper
framing of control messages separated by null bytes instead of relying
on the TLS framing for that. All OpenVPN implementations write the 0
bytes between control commands.
This patch also include several improvement suggestion from Reynir
(thanks!).
CVE: 2024-5594
Reported-By: Reynir Björnsson <reynir@reynir.dk>
Change-Id: I0d926f910637dabc89bf5fa919dc6beef1eb46d9 Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <20240619103004.56460-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28791.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
projects / thirdparty / openvpn.git / log
