push: fix compilation with --disable-management and --enable-werror
The authfail_extended and buf variables are only used when
ENABLE_MANAGEMENT is defined. However, they are currently declared
outside of any ifdefs, thus triggering a warning.
Move the declaration of these 2 down, right before their usage (within
the existing "#ifdef ENABLE_MANAGEMENT" block.
Fixes: ("Cleanup receive_auth_failed and simplify method") Cc: Arne Schwabe <arne@rfc2549.org> Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20220803154049.1213-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24792.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
DCO will try to install keys upon generating them, however, this happens
when parsing pushed cipher options (due to NCP).
For this reason we need to postpone parsing pushed cipher options to
*after* the tunnel interface has been opened, otherwise we would have
no DCO netdev object to operate on.
At the same time we split the parsing code, so that we can ensure that
the NEW_PEER call can happen after the received peer-id has been parsed
(it is required by all DCO API calls).
Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20220803095012.24975-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24789.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
dco: periodically check and possibly rotate/delete keys
Data channel keys are periodically regenerated and installed in ovpn-dco.
However, there is a certain moment when keys are rotated in order
to elect the new primary one.
Check the key status in userspace so that kernelspace can be informed as
well when rotations happen.
Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20220802151604.2801-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24785.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Allow a few levels of recursion in virtual_output_callback()
Without this, replies to commands from the management client
are sometimes lost if the server is writing when a command
comes in and leads to a recursive call to this function.
For some reason I've not been able to trigger this on Linux,
but it does sometimes happen on Windows during intense write
activity by openvpn.exe sending log lines to the management
client.
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20220728034508.15180-2-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24751.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Do not skip ERROR:/SUCCESS: response from management interface
Generally we expect a response of SUCCESS: or ERROR: to every
command sent to the management interface. But, while in
the management-hold state, sending "signal foo" returns only
the following reply (with foo = SIGHUP, SIGUSR1 etc.):
>HOLD:Waiting for hold release:0
Fix by always responding
ERROR: signal 'foo' is currently ignored"
followed by the above line.
Though this is seldom seen in practice[*], such violation of the
protocol could stall clients like the GUI. So fix it.
[*] One way this happens is with SIGHUP sent before the daemon
is on hold state which it enters before the SIGHUP is received.
Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20220728034508.15180-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24750.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
The DCO logic is unable to proceed without --dev argument, therefore
just disable DCO if no --dev was specified by the user.
Right now, calling openvpn with DCO enabled (default) and no --dev
specified leads to a crash, because --dev is assumed to always be there.
Reported-by: Frank Lichtenheld <frank@lichtenheld.com> Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20220801150812.32561-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24772.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 29 Jul 2022 12:37:48 +0000 (14:37 +0200)]
Extract check_session_cipher into standalone function
This allow the code later to check if the cipher is okay to use and
update it for the calculation for the max MTU size.
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Patch v2: Name function check_session_cipher to better reflect its
function Acked-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20220729123748.3267207-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24766.html
dco: initialize context and save pointer in TLS object
Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-By: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20220720123021.24281-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24714.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
dco: introduce open_tun_dco_generic() to open dynamic or fixed-name DCO devices
This function is similar to the essence of open_tun_generic(), but
calling open_tun_dco() instead of trying to do a file open on
"/dev/%s"
Previous attempts to save code duplication by including this into
open_tun_generic() created additional #ifdef plus confusing call
paths. So this is a clean new function, leaving the door open for
a cleanup of open_tun_generic().
Also, introduce tun_dco_enabled(tt) to avoid the negative
"!tt->options.disable_dco" calls.
v11:
- add new function open_tun_dco_generic() for Linux (and FreeBSD, later)
instead of lumping this into open_tun_generic()
- pick up tun_dco_enabled() from a later patch in the series
(easier to bring this in right now than to convert the code back
and then patch it again later)
Signed-off-by: Antonio Quartulli <a@unstable.cc> Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Antonio Quartulli <antonio@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20220721182425.1569798-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24717.html
Gert Doering [Sat, 23 Jul 2022 12:19:09 +0000 (14:19 +0200)]
Fix error message about extended errors for IPv4-only sockets.
The new code to enable IPv6 extended error reporting will cause
an error ("Protocol not available (errno=92)") if trying to enable
that setsockopt() option on an IPv4-only socket.
Fix: pass sock->info.af to set_sock_extended_error_passing(), only
apply to AF_INET6 sockets.
To make that work, ensure that sock->info.af is set to not only
the value coming from config (which might be AF_UNSPEC) but to the
actual value used in socket creation (credits: Arne Schwabe).
Add comments to make explicit that the asymmetry here (IPv4 extended
socket error reporting is enabled on all sockets) is intentional.
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20220723121909.21943-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24731.html
The correct errno can get overwritten by the call to
format_extended_socket_error() which may set errno to EAGAIN
losing the original error and cause to bypass the error reporting
below. Fix by reading the errno of interest at the top of the
function.
Reported by: Gert Doering <gert@greenie.muc.de> Signed-off-by: Selva Nair <selva.nair@gmail.com> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20220722204007.7537-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24728.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 22 Jul 2022 13:02:24 +0000 (15:02 +0200)]
Error out if both remap-usr1 SIGHUP and config stdin are used
OpenVPN for Android uses config stdin to avoid writing the config
file containing private keys to 'disk'. However using stdin means
that config cannot be reread using SIGHUP. While there might be other
corner cases that trigger SIGHUP, this is an obvious one, so we
error out if we detect this misconfiguration.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20220722130224.2442759-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24720.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 24 Jun 2022 08:38:01 +0000 (10:38 +0200)]
tun: extract close_tun_handle into its own fucntion and print correct type
This moves closing the tun handle into its own function and also prints
the adapter type we are operating on, instead hardcoding it to
tap-windows.
While at it, set the handle to NULL after closing, to prevent a double
close due to multiple invocations of this helper.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20220624083809.23487-18-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24527.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
This helper encloses the (simple) logic used by OpenVPN to determine if
the name passed to --dev has to be considered a fixed interface name or
just a pattern.
Having a helper is useful because when this logic is required elsewhere,
we can just re-use this logic without duplicating the code (which may
mean introducing bugs if a future logic change should not update all
spots).
The logic is actually fairly simple: check if the name contains a number
(i.e. tun0). If so, consider the name a fixed device name.
While at it make has_digit() accept a signed argument because strings
are normally signed (also isdigit() accepts a signed argument).
Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20220712221655.19333-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24676.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
This new API can be used to retrieve the type of a specific interface.
It's mostly platform dependant, but right now expected values are
"ovpn-dco", "tun" or "tap".
Other values are possible too, but they are not of interest to us.
This commit also extends the networking unit-test by using the newly
introduced API in conjunction with iface_new and iface_del.
The t_net.sh script has been slightly adapted to allow running these
tests in standalone (as they don't require any iproute2 counterpart).
Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20220713124332.16147-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24688.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
If 'max-clients' is set multi_create_instance() can return NULL (for any
client that would take us over the client limit).
If mi is NULL we don't add it to the hash map, but we do potentially
dereference it to increment the session count.
Do not attempt to do so if 'mi == NULL'.
Signed-off-by: Kristof Provost <kprovost@netgate.com> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20220713083404.13227-2-kprovost@netgate.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24678.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
When using DCO iroutes and routes all live in the same routing table,
However, the latter should always come after the former.
for this reason assign a default metric of 200 to routes. iroutes will
later get a metric of 100.
Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20220628185623.1734-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24599.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
tls-crypt-v2: bail out if the client key is too small
The tls-crypt-v2 key should be at least 2 bytes long in order to read
the actual length. Bail out if the key is too short.
This looks like it could be abused to trigger a read of uninitialized
memory, but after close checking it won't:
We read from BEND(), so this is defined for TCP since the minimum
length there is 3 bytes (pkt len + opcode)
For UDP we might read past the beginning of the packet but since they
are buffers coming from the packet stack we have the headroom/tailroom,
so might read some random data (but not out of bound!).
So we copy some more or less random number into net_len/wkc_len but without
actually reading from undefined memory.
The next line will then almost definitively fail (buf_advance()).
While at it improve the error message a bit.
Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20220628094144.17471-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24580.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Heiko Hund [Fri, 13 May 2022 09:37:40 +0000 (11:37 +0200)]
signal --dns support in peer info
Have clients set a bit in IV_PROTO, so that servers can make an informed
decision on whether to push --dns to the client. While unknown options
are ignored by clients when pushed, they generate a warning in the log.
That can be circumvented by server backends by checking if bit 7 is set.
Signed-off-by: Heiko Hund <heiko@ist.eigentlich.net> Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20220513093740.1091639-1-heiko@ist.eigentlich.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24350.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Heiko Hund [Fri, 27 May 2022 01:24:57 +0000 (03:24 +0200)]
dns: also (re)place foreign dhcp options in env
Override DNS related foreign_options with values set by the --dns
option. This is done so that scripts looking for these options continue
to work if only --dns option were pushed, or the values in the
--dhcp-options differ from what's pushed in --dns.
Signed-off-by: Heiko Hund <heiko@ist.eigentlich.net> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20220527012457.1819262-5-heiko@ist.eigentlich.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24432.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Lev Stipakov [Tue, 24 May 2022 09:19:16 +0000 (12:19 +0300)]
Set o->use_peer_id flag for p2p mode
There are two flags to indicate peer-id usage, one is
in tls_multi struct and another one is in options.
For P2P mode we don't set this flag in options,
which is used in MTU calculation. As a result,
automatically calculated MSS value in P2P mode is wrong,
Fix by bring use_peer_id flag in options and tls_multi
into sync for P2P.
Signed-off-by: Lev Stipakov <lev@openvpn.net> Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <20220524091916.145-1-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24430.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Martin Janů [Fri, 10 Jun 2022 12:04:05 +0000 (12:04 +0000)]
Update the replay-window backtrack log message
The man pages reference a logging message which has been rephrased
in ac1310528a248c99e039e7afaf48724ad1b7f10e. This commit updates the
man page message to reflect the change for improved grep-ability.
Signed-off-by: Martin Janů <martin.janu@protonmail.com> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <SVrvuTydxR6Qs_mvwvG7mqT8iLV0inlcCMXoenZTMI8M0LkosV4pZsH9m_XCTwcRWAPN5H8Zdro0ubhJrnSp6v5KC2ZNAL9So0Y2SKiSe7g=@protonmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24472.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Heiko Hund [Fri, 27 May 2022 01:24:56 +0000 (03:24 +0200)]
rename foreign_option() and move it up
Add setenv_ prefix to foreign_option funtion so it is more obvious what
it does. Move it further up within options.c, so it is defined before
all future callers. Also declare all argv strings const.
Signed-off-by: Heiko Hund <heiko@ist.eigentlich.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20220527012457.1819262-4-heiko@ist.eigentlich.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24436.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Heiko Hund [Fri, 27 May 2022 01:24:54 +0000 (03:24 +0200)]
remove foreign_option() call for IPv6 DNS servers
The call survived since the initial commit 94bfc256d, where it was added
as a fallback, since no IPv6 DNS server handling was implemented at the
time. Now there's dhcp_option_dns6_parse() which adds the servers to the
tuntap options, just like how it is done with the v4 servers.
Signed-off-by: Heiko Hund <heiko@ist.eigentlich.net> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20220527012457.1819262-2-heiko@ist.eigentlich.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24433.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Mon, 23 May 2022 10:35:46 +0000 (12:35 +0200)]
Translate OpenSSL 3.0 digest names to OpenSSL 1.1 digest names
Since we used the OpenSSL <=1.1 names as part of our OCC message, they
are now unfortunately part of our wire protocol.
OpenSSL 3.0 will still accept the "old" names so we do not need to use
this translation table for forward lookup, only for returning the name
with md_kt_name()
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <20220523103546.3425388-2-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24423.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Mon, 16 May 2022 10:48:07 +0000 (12:48 +0200)]
Implement ED448 and ED25519 support in xkey_provider
OpenSSL's implementation of ED448 and ED25519 has a few idiosyncrasies.
Instead of belonging to the elliptic curve type or to a common Edwards
curve type, ED448 and ED25519 have each their own type.
Also, OpenSSL expects signatures using these curves to be done with the
EVP_DigestSign API instead of the EVP_Sign API but using md=NULL.
This has been tested using a "fake" external key that used a normal
software key instead of a hardware implementation but that makes no
difference from the perspective of xkey_provider/management interface.
Patch v2: remove name functions from ed448/ed25519, ensure md is NULL
for ed448/ed25519 and handle NULL/none better in general.
Patch v3: do not pass NULL as string for the OSSL params.
Heiko Hund [Tue, 17 May 2022 21:01:21 +0000 (23:01 +0200)]
pre-commit: uncrustify based on staged changes
Previously the generated patch was based on the file(s) in the working
directory. This is a problem if you have not to be commited changes
there and these changes fix formatting issues that exist in the staging
area. This effectively circumventes the script from rejecting the
commit.
An example:
git add file.c
git commit
... pre-commit hooks complains about formatting ...
... you fix the file manually, forget to git add ...
git commit
... succeeds, even though the commit still has issues ...
Signed-off-by: Heiko Hund <heiko@ist.eigentlich.net> Acked-by: Frank Lichtenheld <frank@lichtenheld.com> Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <20220517210121.1312072-1-heiko@ist.eigentlich.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24376.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Gert Doering [Fri, 13 May 2022 10:15:26 +0000 (12:15 +0200)]
Pass proper sockaddr_* structure for IPv6 socket errors.
commit 043c67f363429 enhances format_extended_socket_error() by
recognizing IPv6 extended socket errors, but neglected to change
the "sockaddr_in" buffer passed to recvmsg() to "sockaddr_storage".
According to documentation, recvmsg() should not have overrun
that buffer (we pass the size of the struct), but according to
ASAN it does... so, pass a pointer to the correct structure.
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20220513101526.11486-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24352.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Lev Stipakov [Tue, 3 May 2022 00:28:40 +0000 (03:28 +0300)]
Fix M_ERRNO behavior on Windows
We use M_ERRNO flag in logging to display error code
and error message. This has been broken on Windows,
where we use error code from GetLastError() and
error description from strerror(). strerror() expects
C runtime error code, which is quite different from
last error code from WinAPI call. As a result, we got
incorrect error description.
The ultimate fix would be introducing another flag
for WinAPI errors, like M_WINERR and use either that or
M_ERRNO depends on context. However, the change would be
quite intrusive and in some cases it is hard to say which
one to use without looking into internals.
Instead we stick to M_ERRNO and in Windows case we
first try to obtain error code from GetLastError() and
if it returns ERROR_SUCCESS (which is 0), we assume that
we have C runtime error and use errno. To get error
description we use strerror_win32() with GetLastError()
and strerror() with errno.
strerror_win32() uses FormatMessage() internally, which
is the right way to get WinAPI error description.
Gert Doering [Tue, 22 Feb 2022 14:35:14 +0000 (15:35 +0100)]
Implement --mtu-disc for IPv6 UDP sockets.
Commit 4225114b96 repaired "--mtu-disc yes" brokenness for IPv4 UDP sockets
(caused by autoconf/ifdef issues). This patch adds new functionality
to do --mtu-disc for IPv6 sockets as well.
Half of it (setsockopt(IPV6_MTU_DISCOVER)) was already there, but
receiving of detailed socket errors was missing the enablement of
setsockopt(IPV6_RECVERR) and parsing of IPPROTO_IPV6/IPV6_RECVERR
messages received.
With that, we now get (sending over a route with "mtu 1300"):
2022-02-22 15:28:07 write UDPv6 [EMSGSIZE Path-MTU=1300]: Message too long
(fd=3,code=90)
2022-02-22 15:28:07 Note adjusting 'mssfix 1400 mtu' to 'mssfix 1300 mtu'
according to path MTU discovery
2022-02-22 15:28:07 Note adjusting 'fragment 1400 mtu' to 'fragment 1300
mtu' according to path MTU discovery
Signed-off-by: Gert Doering <gert@greenie.muc.de> Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20220222143514.3480-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23879.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 6 May 2022 13:28:35 +0000 (15:28 +0200)]
Add uncrustify check to github actions
This adds checking if the code style is still clean github actions with the
exact version of uncrustify that is required and might also be helpful for
external commiters to get notified about code style problem when running
the Github actions on their own repository.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20220506132836.1318985-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24300.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Tue, 3 May 2022 11:29:00 +0000 (13:29 +0200)]
Extract read_incoming_tls_plaintext into its own function
This makes the tls_process_state function a bit easier to read
and allows extending the read_incoming_tls_plaintext function
later without making tls_process_state even longer.
Patch v2: fix compile error.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20220503112900.933975-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24268.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 22 Apr 2022 14:29:48 +0000 (16:29 +0200)]
Optimise three-way handshake condition for S_PRE_START to S_START
We move to the S_START when we have finished the three-way handshake. After
the three way handshake is done, the client will send the TLS Client Hello
packet.
Currently we consider the three way handshake only complete if all
outgoing packets have been acked (which in this case is the one
HARD_RESET_CLIENT or HARD_RESET_SERVER) and also all ACKs for incoming
packets have been sent out.
Waiting for the ack of our own packet is important as it signals that the
other side is really responding. However, the need to also send out all
ACKs for packets we received before moving to the next state breaks
piggybacking the ACKs onto the next control packet.
With this change both server and client will only send a P_CONTROL_V1 with
the TLS Client Hello and the TLS Server Hello with piggybacked ack instead
sending an P_ACK_V1 + P_CONTROL_V1, reducing the number of packets in a
handshake by 2.
This also allows the server to avoid resending P_CONTROL_HARD_RESET_V2
to complete the three-way handshake with HMAC. Only packets with
an ACK contain the remote session id that we need for HMAC session id
verification. The ACK_V1 packet that complets this three-way handshake
can get lost. But the P_CONTROL_V1 with the piggybacked ACK will get
retransmitted. This allows to put the burden of retransmission fully on
the client.
The S_GOT_KEY/S_SENT_KEY -> S_ACTIVE is similar. We do not need to wait
for the ack packet to be sent to move the state forward. This has however
no effect on actual packets since there are normally no outstanding ACKs
here.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20220422142953.3805364-14-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24161.html
Arne Schwabe [Thu, 5 May 2022 13:03:48 +0000 (15:03 +0200)]
Implement HMAC based session id for tls-crypt v2
Tls-crypt v2 is more complicated to implement a proper stateless
handshake. To allow state handshake this commit does
- introduce a new packet CONTROL_WKC_V1 that repeats the wrapped
client key.
- introduce a way to negotiate the support for this packet in the
three way handshake
Details about the protocol changes are in tls-crypt-v2.txt. Optional
arguments to the tls-crypt-v2 option have been added to explicitly
allow or disallow client that do not support the stateless handshake.
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Patch v3: improve grammar, style, comments, fix unit tests
Patch v4: remove explicit flag for ability to resend WKc,
clean up comments, improve code style in some instances Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20220505130348.1183195-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24287.html
Arne Schwabe [Wed, 4 May 2022 11:18:02 +0000 (13:18 +0200)]
Remove workaround for Android 4.4
Android 4.4 is now 9 years old and the main user of this API (OpenVPN
for Android) does not support this OS anymore. This workaround
is now safe to remove.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20220504111802.1050648-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24276.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Wed, 4 May 2022 11:31:58 +0000 (13:31 +0200)]
Fix format specifier for printing size_t on 32bit size_t platforms
Today even 32 bit platform generally use a 64bit size_t but Android
armeabi-v7a is an expection to that and uses a 32bit size_t. Use
z as correct specifier for a size_t.
Clang complained about this:
warning: format specifies type 'unsigned long' but the
argument has type 'size_t' (aka 'unsigned int') [-Wformat]
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20220504113158.1051861-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24277.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 22 Apr 2022 14:29:46 +0000 (16:29 +0200)]
Extract read_incoming_tls_ciphertext into function
This makes the code a bit more structured and easier to read. Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20220422142953.3805364-12-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24152.html
OpenVPN currently has a bit of a weakness in its early three way handshake
A single client reset packet (first packet of the handshake) will
- trigger creating a session on the server side leading to potential
ressource exhaustion
- make the server respond with 3 answers trying to get an ACK for its
P_CONTROL_HARD_RESET_SERVER_V2 answer making it an amplification
Instead of allocating a connection for each client on the initial packet
OpenVPN will now calculate a session id based on a HMAC that serves as
verifiable cookie that can be checked for authenticity when the client
responds with it. This eliminates the amplification attack and resource
exhaustion attacks.
For tls-crypt-v2 clients the HMAC based handshake is not used yet (will
be added in one of the next patches).
Patch v2: rebase on master
patch v3: fix unit tests, improve comment/style of code
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20220502154310.836947-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24262.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 22 Apr 2022 14:29:41 +0000 (16:29 +0200)]
Move CRL reload to key_state_init from S_START transition
The current place that we reload is a bit more efficient since it only
triggers reload after a completed 3way handshake. On the other hand the
key_state_init is a much more logical place and with the upcoming
HMAC based UDP code and TCP code, the initialisation will only be done
after a 3way handshake.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20220422142953.3805364-7-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24156.html
Arne Schwabe [Fri, 22 Apr 2022 14:29:40 +0000 (16:29 +0200)]
Remove pointless indentation from tls_process.
This is probably a result from earlier code that still needed to be
C89 compatible add probably added this to allow variable decleration Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20220422142953.3805364-6-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24166.html
Arne Schwabe [Fri, 22 Apr 2022 14:29:39 +0000 (16:29 +0200)]
Move tls_process_state into its own function
This function does most of the state transitions in the TLS state
machine. Moving it into its own function removes an intention area and
makes tls_process function easier to understand as the loop is more
obvious.
This is largely just a code move with small expection. bool active is
no longer directly set but inferred from to_link->len
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20220422142953.3805364-5-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24157.html
Arne Schwabe [Tue, 26 Apr 2022 13:23:23 +0000 (15:23 +0200)]
Change FULL_SYNC macro to no_pending_reliable_packets function
This changes this macro to a better named inline function. This
introduces a slight whitespace problem but the next refactoring will
move the incorrectly intended block to its own function anyway.
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20220426132324.76517-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24213.html
Arne Schwabe [Fri, 22 Apr 2022 14:29:37 +0000 (16:29 +0200)]
Extract session_move_pre_start as own function, use local buffer variable
This changes the C90 struct buffer declaration to a C99 style one. Also
move the state transition from S_INITIAL to S_PE_START into its own
function. Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20220422142953.3805364-3-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24151.html
Arne Schwabe [Fri, 22 Apr 2022 14:29:36 +0000 (16:29 +0200)]
Refactor tls-auth/tls-crypt wrapping into into own function
This allows the the wrapping to be easier reused by a function that
does not have access to a full TLS session. Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20220422142953.3805364-2-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24150.html
Marc Becker [Mon, 25 Apr 2022 21:58:22 +0000 (23:58 +0200)]
fix GitHub workflow working directories in MinGW builds
replace hardcoded directory names with env variable version info
bump pkcs11-helper version to 1.29.0
bump OpenSSL version to 1.1.1n
add OpenSSL version to cache key
use release file for pkcs11-helper archive
use OpenSSL URL endpoint with all/current versions
Signed-off-by: Marc Becker <becm@gmx.de> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20220425215822.18569-1-becm@gmx.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24202.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 22 Apr 2022 13:40:35 +0000 (15:40 +0200)]
Move ssl function related to control channel wrap/unwrap to ssl_pkt.c/h
This allows these functions to be relatively easily included into the
unit test without pulling ssl.c and all the dependencies of ssl.c into
a unit test.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20220422134038.3801239-7-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24149.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 22 Apr 2022 13:40:34 +0000 (15:40 +0200)]
Extend tls_pre_decrypt_lite to return type of packet and keep state
This allows us to keep the temporary data for a little bit longer
so we can use this to make further checks and ultimatively use the
state to craft the HMAC based RESET reply.
For now we do not use the extra information and keep behaviour
identical.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20220422134038.3801239-6-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24148.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Mon, 25 Apr 2022 12:27:09 +0000 (14:27 +0200)]
Move pre decrypt lite check to its own function
This prepares for extending this function with the HMAC based session ID
check.
Replace the check for m->top.c2.tls_auth_standalone with an ASSERT as this
code path is only used in multi udp server and OpenVPN initialises the
tls_auth_standalone always for the TOP context (CF_INIT_TLS_AUTH_STANDALONE),
even for the tcp m2mp server that does not use it).
Patch v2: replace if with ASSERT
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20220425122709.4148015-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24193.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 22 Apr 2022 14:29:44 +0000 (16:29 +0200)]
Make buf_write_u8/16/32 take the type they pretend to take
This functions should accept the type of integer they say to write. Calling
the u32 function with an integer that is actually 32 bit unsigned gives
compiler warnings. Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20220422142953.3805364-10-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24165.html
Arne Schwabe [Fri, 22 Apr 2022 13:40:30 +0000 (15:40 +0200)]
Remove tls_init_control_channel_frame_parameters wrapper function
While calling this wrapper function is strictly more correct, these
indirection layer with tiny wrapper make the code more complex and
going through more layer than it really needs to.
Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20220422134038.3801239-2-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24172.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
Arne Schwabe [Fri, 22 Apr 2022 14:29:35 +0000 (16:29 +0200)]
Remove EXPONENTIAL_BACKOFF define
We have EXPONENTIAL_BACKOFF as default forever (8c47de7, 2.1.1c,
2010). Remove the other code path that is dead code. Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20220422142953.3805364-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/search?l=mid&q=20220422142953.3805364-1-arne@rfc2549.org
Arne Schwabe [Fri, 22 Apr 2022 13:40:38 +0000 (15:40 +0200)]
Remove inc_pid argument from reliable_mark_deleted that is always true
This is a small cleanup to remove a superfluous argument Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20220422134038.3801239-10-arne@rfc2549.org>
URL: https://www.mail-archive.com/search?l=mid&q=20220422134038.3801239-10-arne@rfc2549.org
Arne Schwabe [Fri, 22 Apr 2022 13:40:31 +0000 (15:40 +0200)]
Remove dead PID_TEST code
Enabling this test produces compile errors and by the looks of it the
test has been broken for many years. Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20220422134038.3801239-3-arne@rfc2549.org>
URL: https://www.mail-archive.com/search?l=mid&q=20220422134038.3801239-3-arne@rfc2549.org
On most systems limits.h is pulled in by some other header and thus no
error is ever triggered, but it's possible to find the right environment
which lackis this and prevents compiling auth-pam.c (possibly when using
LibreSSL).
Include the header explicitly as it includes the definition of PATH_MAX.
(note that this bug is fixed in Gentoo since 2020 by including a custom
patch, but apparently the issue was never reported upstream)
Reported-by: Michelangelo Scopelliti <kernelpanic@gmx.com> Signed-off-by: Antonio Quartulli <a@unstable.cc> Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20220421131909.32053-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24136.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
projects / thirdparty / openvpn.git / log
