]> git.ipfire.org Git - thirdparty/openvpn.git/log
thirdparty/openvpn.git
2 years agoMake sending plain text control message session aware
Arne Schwabe [Wed, 1 Mar 2023 13:53:53 +0000 (14:53 +0100)] 
Make sending plain text control message session aware

The control messages coming from auth pending should always be on the
session that triggered them (i.e. INITIAL or ACTIVE) and not always on the
active session.  Rework the code path that trigger those messsages from
management and plugin/script to specify the TLS session.

We only support the two TLS sessions that are supposed to be active. TLS
sessions in any lame slot (TM_LAME or KS_LAME) are not considered to be
candidates for sending messages as these slots only serve to keep key
material around.

Unfortunately, this fix requires the management interface to be changed
to allow including the specific session the messages should to go to. As
there are very few users of this interface with auth-pending, I made this
a hard change instead of adding hacky workaround code that is not always
working correctly anyway.

send_control_channel_string() will continue to only use the primary session
and key but the current users of that (push replys and exit notification)
already require the established session to be the active one, so there
no changes needed at the moment.

Github: fixes OpenVPN/openvpn#256

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230301135353.2811069-2-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26320.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoUse key_state instead of multi for tls_send_payload parameter
Arne Schwabe [Wed, 1 Mar 2023 13:53:52 +0000 (14:53 +0100)] 
Use key_state instead of multi for tls_send_payload parameter

Currently, this function and other parts of OpenVPN assume that
multi->session[TM_ACTIVE].key[KS_PRIMARY] is always the right session
to send control message.

This assumption was only achieve through complicated session moving and
shuffling in our state machine in the past. The old logic basically also
always assumed that control messages are always for fully authenticated
clients. This assumption was never really true (see AUTH_FAILED message)
but has been broken even more by auth-pending. Cleaning up the state machine
transitions in 7dcde87b7a broke this assumption even more.

This change now allows to specify the key_state/TLS session that is used to
send the control message.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230301135353.2811069-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26319.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agousing OpenSSL3 API for EVP PKEY type name reporting
Michael Baentsch [Sun, 19 Mar 2023 07:54:41 +0000 (08:54 +0100)] 
using OpenSSL3 API for EVP PKEY type name reporting

Signed-off-by: Michael Baentsch <info@baentsch.ch>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20230319075441.13021-1-info@baentsch.ch>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26439.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoSupport --inactive option for DCO
Lev Stipakov [Wed, 15 Mar 2023 13:38:08 +0000 (15:38 +0200)] 
Support --inactive option for DCO

When DCO is in use, userland doesn't see any traffic
which breaks --inactive option.

Fix by adding inactivity check to inactivity timeout
callback. Get the cumulative tun bytes count (ping packets
are excluded) from DCO and compare it to the previous value
stored in c2.inactivity_bytes. Reset inactivity timer and
update c2.inactivity_bytes if amount of new bytes exceeds
inactivity_minimum_bytes, otherwise terminate session
due to inactivity.

Github: Fixes OpenVPN/openvpn#228

Currently works only on Windows, since we don't yet have
single peer stats implementation for Linux and FreeBSD.

Change-Id: Ib417b965bc4a2c17b51935b43c9627b106716526
Signed-off-by: Lev Stipakov <lev@openvpn.net>
Acked-by: Heiko Hund <heiko@ist.eigentlich.net>
Message-Id: <20230315133808.1550-1-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26421.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoAdd a test for signing with certificates in Windows store
Selva Nair [Wed, 15 Mar 2023 01:35:16 +0000 (21:35 -0400)] 
Add a test for signing with certificates in Windows store

- For each sample certificate/key pair imported into the store,
  load the key into xkey-provider and sign a test message.
  As the key is "provided", signing will use appropriate
  backend (Windows CNG in this case).

  The signature is then verified using OpenSSL.

Change-Id: I520b34ba51e8c6d0247a82edc52bde181ab5a717
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230315013516.1256700-5-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26416.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoRefactor SSL_CTX_use_CryptoAPI_certificate()
Selva Nair [Wed, 15 Mar 2023 01:35:15 +0000 (21:35 -0400)] 
Refactor SSL_CTX_use_CryptoAPI_certificate()

- Loading the certificate and key into the provider is split out of
  setting up the SSL context. This allows testing of signing by
  cryptoapi-provider interface without dependence on SSL context
  or link-time wrapping.

Change-Id: I269b94589636425e1ba9bf953047d238fa830376
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230315013516.1256700-4-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26414.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoAdd tests for finding certificates in Windows cert store
Selva Nair [Wed, 15 Mar 2023 01:35:14 +0000 (21:35 -0400)] 
Add tests for finding certificates in Windows cert store

- find_certificate_in_store tested using 'SUBJ:', 'THUMB:'
  and 'ISSUER:' select strings. Uses test certificates
  imported into the store during the import test.

Change-Id: Ib5138465e6228538af592ca98b3d877277355f59
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230315013516.1256700-3-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26415.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoImport some sample certificates into Windows store for testing
Selva Nair [Wed, 15 Mar 2023 01:35:13 +0000 (21:35 -0400)] 
Import some sample certificates into Windows store for testing

- A few sample certificates are defined and imported into
  Windows certificate store (user store).
  This only tests the import process. Use of these certs to test the
  core functionality of 'cryptoapicert' are in following commits.

Change-Id: Ida5fc12c5bad5fde202da0bf0e8cdc71efe548c2
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230315013516.1256700-2-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26417.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoFix memory leaks in HMAC initial packet generation
Arne Schwabe [Wed, 15 Mar 2023 19:55:12 +0000 (20:55 +0100)] 
Fix memory leaks in HMAC initial packet generation

The HMAC leaks are just forgotten frees/deinitialisations.

tls_wrap_control() will sometimes return the original buffer (non
tls-crypt) and sometimes tls_wrap.work, so handling this buffer lifetime
is a bit more complicated.  Instead of further complicating that code
just give our work buffer the same lifetime as the other one inside
tls_wrap.work (put it into per-session gc_arena) as that is also more
consistent.

Second, packet_id_init() allocates a buffer with malloc and not using a
gc_arena, so we need to also manually free it.

Patch v2: add missing deallocations in unit tests of the new workbuf
Patch v3: remove useless allocation of 0 size buffer in
          tls_auth_standalone_init

Found-By: clang with asan
Change-Id: I0cff44f79ee7e3bcf7b5981fc94f469c15f21af3
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230315195512.323070-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoBugfix: Convert ECDSA signature form pkcs11-helper to DER encoded form
Selva Nair [Tue, 14 Mar 2023 12:21:34 +0000 (08:21 -0400)] 
Bugfix: Convert ECDSA signature form pkcs11-helper to DER encoded form

With OpenSSL 3.0 and xkey-provider, we use pkcs11h_certificate_signAny_ex()
which returns EC signature as raw r|s concatenated. But OpenSSL expects
a DER encoded ASN.1 structure.

Do this conversion as done in cryptoapi.c. For code re-use, ecdsa_bin2sig()
is consolidated with sig to DER conversion as ecdsa_bin2der() and
moved to xkey_helper.c

In the past when we used OpenSSL hooks installed by pkcs11-helper,
such a conversion was not required as it was internally handled by
the library.

Reported by: Tom <openvpn@sup-logistik.de>
Also see: https://bugzilla.redhat.com/show_bug.cgi?id=2177834
Tested-by: Florian Apolloner <florian@apolloner.eu>
Change-Id: Ie20cf81edd643ab8ef3c41321353d11fd66c188c
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20230314122134.1248576-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26406.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoFix memory leaks in open_tun_dco()
Arne Schwabe [Tue, 14 Mar 2023 14:48:54 +0000 (15:48 +0100)] 
Fix memory leaks in open_tun_dco()

open_tun_dco_generic() already allocates the tt->actual_name string, which
shadows the allocation in the FreeBSD/Linux specific methods.

Found-By: clang with asan
Change-Id: I51f5fcfff4e5f8203fdb9aec0245cfccd17043cc
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230314144854.182110-2-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26411.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agodco: print FreeBSD version
Kristof Provost [Thu, 9 Mar 2023 12:23:32 +0000 (13:23 +0100)] 
dco: print FreeBSD version

Implement dco_version_string() for FreeBSD.

Unlike Linux and Windows the DCO driver is built into the operating
system itself, so we log the OS version as a proxy for the DCO version.

Signed-off-by: Kristof Provost <kp@FreeBSD.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230309122332.92490-1-kprovost@netgate.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26367.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agodco: print version to log if available
Antonio Quartulli [Thu, 9 Mar 2023 13:14:19 +0000 (14:14 +0100)] 
dco: print version to log if available

In order to provide better support in case of troubleshooting issues,
it's important to know what exact DCO version is loaded on the user
system.

Therefore print the DCO version during bootup.

For Windows and FreeBSD we currently implement a placeholder printing 'v0'.
This should be improved with a follow-up patch.

For Linux we directly fetch the module version from /sys and print
something like:

DCO version: 0.1.20230206-15-g580608ec7c59

Change-Id: Ie1f6fa5d12a473d353d84fd119c2430b638e8bcd
Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20230309131419.29157-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26370.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agodco: don't use NetLink to exchange control packets
Antonio Quartulli [Thu, 9 Mar 2023 21:03:44 +0000 (22:03 +0100)] 
dco: don't use NetLink to exchange control packets

Using NetLink for control messages did not work out as it did lead to
kernel side buffer congestion during heavy client activity.

With this patch DCO will redirect control packets directly to the
transport socket without altering them, so that userspace can
happily process them as usual.

NOTE: this is an API breaking change.  Up to this commit, the userland
requests a kernel module called "ovpn-dco" which does control messages
via netlink.  From this commit on, OpenVPN requests a kernel module named
"ovpn-dco-v2" which brings the kernel change corresponding to this commit.

If the system only has "the wrong module" available (either way), OpenVPN
will log

   ... Kernel support for ovpn-dco missing, disabling data channel offload.

and proceed without kernel support.

Change-Id: Ia1297c3ae9a28b188ed21ad21ae96fff3d02ee4d
[lev@openvpn.net: ensure win_dco flag is still exposed]
Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20230309210344.5763-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26384.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agotests/unit_tests: Fix 'make distcheck' with subdir-objects enabled
Frank Lichtenheld [Wed, 8 Mar 2023 15:07:04 +0000 (16:07 +0100)] 
tests/unit_tests: Fix 'make distcheck' with subdir-objects enabled

Commit 7f72abcf8a56bb35a510a3409e03a4e2aaba50da enabled subdir-objects
when using automake 1.16+.

There is an issue with the handling of .deps directories with this option.
While automake 1.16 fixed subdir-objects to work at all when _SOURCES
contains "unexpanded references" and it did fix subdir-objects to work
with out-of-tree build for "source files specified with an explicit
'$(srcdir)'" those fixes are not transitive. "unexpanded references"
still break out-of-tree builds when enforcing a read-only source dir
like 'make distcheck' does. When using *explicit* references to
srcdir and top_srcdir it works correctly.

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20230308150704.128797-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26352.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoEnsure n = 2 is set in key2 struct in tls_crypt_v2_unwrap_client_key
Arne Schwabe [Thu, 9 Mar 2023 12:00:31 +0000 (13:00 +0100)] 
Ensure n = 2 is set in key2 struct in tls_crypt_v2_unwrap_client_key

The ASSERT in xor_key2 assumes that all methods that load a key2 struct
correctly set n=2. However, tls_crypt_v2_unwrap_client_key loads a key
without setting n = 2, triggering the assert.

Github: Closes and reported in OpenVPN/openvpn#272

Change-Id: Iaeb163d83b95818e0b26faf9d25e7737dc8ecb23
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <20230309120031.3780130-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26363.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoSet netlink socket to be non-blocking
Arne Schwabe [Wed, 8 Mar 2023 15:19:45 +0000 (16:19 +0100)] 
Set netlink socket to be non-blocking

Even though we use select/poll to explicitly query when the netlink
socket is ready for read, sometimes we end up reading from the socket
when it is not ready to read and then the process hangs for several
seconds (20-30s). Avoid this situation by setting the socket to be
non-blocking, so we get a status in this case that allows us to continue.

Change-Id: I35447c23a9350176007df5455bf9451021e9856d
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <20230308151945.3670151-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26353.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoAvoid warning about missing braces when initialising key struct
Antonio Quartulli [Wed, 8 Mar 2023 13:37:43 +0000 (14:37 +0100)] 
Avoid warning about missing braces when initialising key struct

This avoids the warning from gcc about initialising the key2 struct.

Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230308133743.5059-1-a@unstable.cc>
URL: https://www.mail-archive.com/search?l=mid&q=20230308133743.5059-1-a@unstable.cc
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoFreeBSD 12.x workaround for IPv6 ifconfig is needed on 12.4 as well
Gert Doering [Mon, 6 Mar 2023 08:07:44 +0000 (09:07 +0100)] 
FreeBSD 12.x workaround for IPv6 ifconfig is needed on 12.4 as well

Commit 16d7f2cd4d90 tried to remove an FreeBSD 12.x ifconfig inet6
workaround based on the understanding that the upstream fix for
bug 248172 went into 12.4, but that was a misread of the code - 12.4
needs the workaround as well, fixed in 13.0.

Also extend comment to point to /etc/network.subr, which is the real
source of the problematic code

        if checkyesno ipv6_activate_all_interfaces; then
                _ipv6_opts="-ifdisabled"
        elif [ "$1" != "lo0" ]; then                   <<<<
                _ipv6_opts="ifdisabled"                <<<<
        fi

Trac: 1226

Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20230306080744.66069-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26335.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoDynamic tls-crypt for secure soft_reset/session renegotiation
Arne Schwabe [Tue, 7 Mar 2023 15:02:33 +0000 (16:02 +0100)] 
Dynamic tls-crypt for secure soft_reset/session renegotiation

Currently we have only one slot for renegotiation of the session/keys.
If a replayed/faked packet is inserted by a malicous attacker, the
legimate peer cannot renegotiate anymore.

This commit introduces dynamic tls-crypt. When both peer support this
feature, both peer create a dynamic tls-crypt key using TLS EKM (export
key material) and will enforce using that key and tls-crypt for all
renegotiations. This also add an additional protection layer for
renegotiations to be taken over by an illegimate client, binding the
renegotiations tightly to the original session. Especially when 2FA,
webauth or similar authentication is used, many third party setup ignore
the need to secure renegotiation with an auth-token.

Since one of tls-crypt/tls-crypt-v2 purposes is to provide poor man's post
quantum crypto guarantees, we have to ensure that the dynamic key tls-crypt
key that replace the original tls-crypt key is as strong as the orginal key
to avoid problems if there is a weak RNG or TLS EKM produces weak keys. We
ensure this but XORing the original key with the key from TLS EKM. If
tls-crypt/tls-cryptv2 is not active, we use just the key generated by
TLS EKM. We also do not use hashing or anything else on the original key
before XOR to avoid any potential of a structure in the key or something
else that might weaken post-quantum use cases.

OpenVPN 2.x reserves the TM_ACTIVE session for renegotiations. When a
SOFT_RESET_V1 packet is received, the active TLS session is moved from
KS_PRIMARY to KS_SECONDARY. Here an attacker could theorectically send a
faked/replayed SOFT_RESET_V1 and first packet containing the TLS client
hello. If this happens, the session is blocked until the TLS
renegotiation attempt times out, blocking the legimitate client.

Using a dynamic tls-crypt key here blocks any SOFT_RESET_V1 (and following
packets) as replay and fake packets will not have a matching
authentication/encryption and will be discarded.

HARD_RESET packets that are from a reconnecting peer are instead put in the
TM_UNTRUSTED/KS_PRIMARY slot until they are sufficiently verified, so the
dynamic tls-crypt key is not used here.  Replay/fake packets also do not
block the legimitate client.

This commit delays the purging of the original tls-crypt key data from
directly after passing it to crypto library to tls_wrap_free. We do this
to allow us mixing the new exported key with the original key.
To be able to generate the dynamic tls-cryptn key, we need the original
key, so deleting the key is not an option if we need it later again to
generate another key. Even when the client does not support secure
renegotiation, deleting the key is not an option since when the
reconnecting client or (especially in p2p mode with float) another client
does the reconnect, we might need to generate a dynamic tls-crypt key
again. Delaying the deletion of the key has also little effect as the
key is still present in the OpenSSL/mbed TLS structures in the tls_wrap
structure, so only the number of times the keys is in memory would be
reduced.

Patch v2: fix spellings of reneg and renegotiations.
Patch v3: expand comment to original_tlscrypt_keydata and commit message,
          add Changes.rst
Patch v4: improve commit message, Changes.rst
Patch v5: fix spelling/grammar mistakes. Add more comments.
Patch v6: consistently calld this feature dynamic tls-crypt crypt. Note
          this changes the export label and makes it incompatible with
          previous patches.
Patch v7: also xor tls-auth key data into the dynamic tls-crypt key like
          tls-crypt key data
Patch v8: Avoid triggering ASSERT added in v7 by properly setting keys.n = 2
          when loading tls crypt v2 client keys. Add dyn-tls-crypt to
          protocol options printout.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Heiko Hund <heiko@ist.eigentlich.net>
Message-Id: <20230307150233.3551436-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26341.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoDo not save pointer to 'struct passwd' returned by getpwnam etc.
Selva Nair [Mon, 6 Mar 2023 05:33:45 +0000 (00:33 -0500)] 
Do not save pointer to 'struct passwd' returned by getpwnam etc.

- This pointer is to a static area which can change on further
  calls to getpwnam, getpwuid etc.
  Same with struct group returned by getgrnam.

  As the only field later referred to is uid or gid, fix
  by saving them instead.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230306053346.796992-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26332.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoReduce initialisation spam from verb <= 3 and print summary instead
Arne Schwabe [Tue, 14 Feb 2023 11:20:44 +0000 (12:20 +0100)] 
Reduce initialisation spam from verb <= 3 and print summary instead

The messages about cipher initialisation are currently very noisy,
especially if tls-auth/tls-crypt is in use.

Typically messages like this is display for AES-256-CBC with SHA256:

   Outgoing Data Channel: Cipher 'AES-256-CBC' initialized with 256 bit key
   Outgoing Data Channel: Using 256 bit message hash 'SHA256' for HMAC authentication
   Incoming Data Channel: Cipher 'AES-256-CBC' initialized with 256 bit key
   Incoming Data Channel: Using 256 bit message hash 'SHA256' for HMAC authentication

in addition to the tls-crypt/tls-auth messages that has the amount of
messages.

These message are not that helpful. The only meaningful information is
better suited in compat messages. This commit moves the spammy messages
to verb 4 and consistently prints out the cipher/auth used in the data
channel instead on verb 2:

   Data Channel: cipher 'AES-256-CBC' auth 'SHA256'

This patches also summarises other aspects of the imported options for VPN
connection and prints them (even if not coming from pulled options):

    Data Channel: cipher 'AES-256-GCM'
    Timers: ping 8, ping-restart 40
    Protocol options: explicit-exit-notify 1, protocol-flags tls-ekm

And move the OPTIONS IMPORT: xx modified that are included in the new
messages to D_PUSH_DEBUG (verb 7) since they do not add any useful
information anymore.

Patch v2: also compile with compression disabled

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230214112044.1021962-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26249.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agodco: define OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT on FreeBSD
Kristof Provost [Fri, 3 Mar 2023 11:05:11 +0000 (12:05 +0100)] 
dco: define OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT on FreeBSD

FreeBSD's if_ovpn will never emit this as a peer deletion reason
(because it doesn't support TCP), but this allows us to align the
defines between Linux and FreeBSD, and remove a Linux-specific case from
process_incoming_del_peer().

Signed-off-by: Kristof Provost <kprovost@netgate.com>
Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <20230303110511.9569-1-kprovost@netgate.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26324.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoconfigure: improve FreeBSD DCO check
Kristof Provost [Wed, 1 Mar 2023 09:18:48 +0000 (10:18 +0100)] 
configure: improve FreeBSD DCO check

The libnv check doesn't work as expected on FreeBSD 14.x, because
FreeBSD has namespaced libnv to avoid conflicts with libnvpair.
This means that the naive check generated by AC_CHECK_LIB() fails to
detect libnv even though it's present.

Instead check for the if_ovpn.h header. This is a more accurate check
anyway, as libnv is present on FreeBSD versions prior to 14 (which do
not support DCO).

Signed-off-by: Kristof Provost <kprovost@netgate.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230301091848.80760-1-kprovost@netgate.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26314.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agooptions.c: enforce a minimal fragment size
Kristof Provost [Wed, 1 Mar 2023 09:18:51 +0000 (10:18 +0100)] 
options.c: enforce a minimal fragment size

Very low values for 'fragment' can result in a division by zero in
optimal_fragment_size() (because it rounds max_frag_size down with
FRAG_SIZE_ROUND_MASK).

Enforce a minimal fragment size of 68 bytes, based on RFC 791 ("Every
internet module must be able to forward a datagram of 68 octets without
further fragmentation.")

Signed-off-by: Kristof Provost <kprovost@netgate.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230301091851.82243-1-kprovost@netgate.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26313.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoUse proper print format/casting when converting msg_channel handle
Arne Schwabe [Tue, 14 Feb 2023 13:43:23 +0000 (14:43 +0100)] 
Use proper print format/casting when converting msg_channel handle

The current casting triggers a warning on 32bit:

    init.c:1842:66: error: cast from pointer to integer of different size
[-Werror=pointer-to-int-cast]

Use the proper printf format specifier for printing a pointer avoiding
the cast alltogether.

In options.c use a cast to intptr_t before converting to a handle to
avoid having to ifdef atoll/atol for 32/64 bit.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20230214134323.1033590-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26255.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoAvoid management log loop with verb >= 6
Lev Stipakov [Fri, 17 Feb 2023 12:21:55 +0000 (14:21 +0200)] 
Avoid management log loop with verb >= 6

This log message is printed within check_tls(),
which is called by pre_select(), which is called
on every iteration of event loop.

When management is attached (and doesn't use own event loop),
this message sets management state to "wait write",
which arms event loop. When on the next iteration iowait
returns with "management write event is set", we call
pre_select() and print that message again, causing the loop.

Fix by simply removing this log message.

Signed-off-by: Lev Stipakov <lev@openvpn.net>
Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <20230217122156.541-1-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26284.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoUpdate issue templates
Antonio Quartulli [Sun, 26 Feb 2023 21:34:20 +0000 (22:34 +0100)] 
Update issue templates

With this change we extend the text exposed to people opening a bug in
the OpenVPN project.

Hopefully they will read and immediately understand that GH is not the
right place to report ossues about commercial products.

Change-Id: Idd039612698a6b08f9544450885d1a5f77fd95c6
Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230226213420.21201-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26305.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoconfigure: fix formatting of --disable-lz4 and --enable-comp-stub
Frank Lichtenheld [Mon, 6 Feb 2023 13:08:46 +0000 (14:08 +0100)] 
configure: fix formatting of --disable-lz4 and --enable-comp-stub

Make consistent with the other options.

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230206130846.63415-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26156.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoWindows: fix signedness errors with recv/send
Frank Lichtenheld [Fri, 3 Feb 2023 19:14:40 +0000 (20:14 +0100)] 
Windows: fix signedness errors with recv/send

On Linux those functions actually take void pointer,
so no behavior change there. On Windows, we avoid
warnings about unsigned char vs char.

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230203191440.136050-6-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26144.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoAdd a unit test for functions in cryptoapi.c
Selva Nair [Tue, 14 Feb 2023 20:08:04 +0000 (15:08 -0500)] 
Add a unit test for functions in cryptoapi.c

- Though named cryptoapi_testdriver, right now this only tests
  parsing of thumbprint specified as a selector for --cryptioapicert
  option. More tests coming..

v2: a line that belongs here was mistakenly included in the previous
commit. Corrected.
v3: add to list of tests run in github actions
v4: - correct comment above invalid strings (copy paste error)
    - make invalid strings differ from correct value only in the
      explicitly introduced invalid characters/separators (one had
      two distinct errors which is not a robust test).

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230214200804.600405-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26268.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoAdd logging for windows driver selection process
Lev Stipakov [Thu, 16 Feb 2023 16:01:29 +0000 (18:01 +0200)] 
Add logging for windows driver selection process

Signed-off-by: Lev Stipakov <lev@openvpn.net>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230216160129.994-1-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26281.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoconfigure: enable DCO by default on FreeBSD/Linux
Frank Lichtenheld [Wed, 15 Feb 2023 16:26:54 +0000 (17:26 +0100)] 
configure: enable DCO by default on FreeBSD/Linux

Automatically disabled when
- iproute2 is enabled
  (Don't want to force people specifying --disable-dco explicitely)
- libnv is missing on FreeBSD
  (FreeBSD version too old anyway)

Will still error out if libnl-genl is missing on Linux to
make people aware of new dependency.

v2: error out when libnl-genl is missing as discussed with ordex on
    IRC.
v3:
 - improvements to the messages, suggested by Selva
 - further improvements to the default specification, trying to make it clear
 - if enabling iproute2, do not test for libnl-genl
v4: add updates for GHA
v5:
 - v4 was missing the changes of v3. v5 combines the changes from v3 and v4
 - fix build failure GHA/ubuntu1804/mbedtls
 - fix build failure GHA/ubuntu2204/libressl

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <20230215162654.52137-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26272.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoDisable DCO if proxy is set via management
Lev Stipakov [Mon, 20 Feb 2023 09:06:01 +0000 (11:06 +0200)] 
Disable DCO if proxy is set via management

DCO doesn't support proxy and we already disable DCO
is proxy is set in profile.

Signed-off-by: Lev Stipakov <lev@openvpn.net>
Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <20230220090601.983-1-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26287.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoExit if a proper message instead of segfault on Android without management
Arne Schwabe [Mon, 20 Feb 2023 13:14:24 +0000 (14:14 +0100)] 
Exit if a proper message instead of segfault on Android without management

The Android implementation is relying on the management interface to be
always available. Trying to run the Android binary without the mangament
interface outside the app leads to a segfault. Exit with a FATAL error
instead.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230220131424.1749736-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26288.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoOption --cryptoapicert: support issuer name as a selector
Selva Nair [Sat, 28 Jan 2023 22:34:18 +0000 (17:34 -0500)] 
Option --cryptoapicert: support issuer name as a selector

- Certificate selection string can now specify a partial
  issuer name string as "--cryptoapicert ISSUER:<string>" where
  <string> is matched as a substring of the issuer (CA) name in
  the certificate.

  Partial case-insensitive matching against the "issuer name" is
  used. Here "issuer name" is a text representation of the RDN's
  separated by commas.

  E.g., "CA, Ontario, Toronto, Acme Inc., IT, Acme Root CA".

  See MSDN docs on CertFindCertificateInStore() with CERT_FIND_ISSUER_STR
  as "FindType" for more details.

  As the order of RDN's is not well-defined[*] and type names like "OU"
  or "CN" are not included, its best to match against a single attribute
  like the CN of the issuer:

  E.g., --cryptoapicert "ISSUER:Acme Root"

[*] Windows appears to order RDN's in the reverse order to which
its written in the certificate but do not rely on this.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230128223421.2207802-2-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26092.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agocryptoapi.c: simplify parsing of thumbprint hex string
Selva Nair [Sat, 4 Feb 2023 00:43:22 +0000 (19:43 -0500)] 
cryptoapi.c: simplify parsing of thumbprint hex string

v2: Moved the "parse_hexstring" chunk to a function for clarity
and to permit unit-testing.

A test is submitted as a follow up patch.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230204004322.250210-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26146.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agocryptoapi.c: remove pre OpenSSL-3.01 support
Selva Nair [Wed, 1 Feb 2023 23:03:40 +0000 (18:03 -0500)] 
cryptoapi.c: remove pre OpenSSL-3.01 support

- Require xkey-provider (thus OpenSSL 3.01+) for --cryptoapicert

Note:
  Ideally we should also make ENABLE_CRYPTOAPI conditional
  on HAVE_XKEY_PROVIDER but that looks hard unless we can agree
  to move HAVE_XKEY_PROVIDER to configure/config.h.

v2: use "binary" instead of "version" in the error message

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230201230340.2268781-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26131.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agocyryptapi.c: log the selected certificate's name
Selva Nair [Sat, 28 Jan 2023 22:34:19 +0000 (17:34 -0500)] 
cyryptapi.c: log the selected certificate's name

- With various ways of specifying the selector-string to the
  "--cryptoapicert" option, its not immediately obvious
  which certificate gets selected from the store. Log it.

  The "name" logged is a friendly name (if present), or a
  representative element of the subject (usually the common-name).

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230128223421.2207802-3-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26093.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoRevise the cipher negotiation info about OpenVPN3 in the man page
Arne Schwabe [Fri, 10 Feb 2023 14:27:10 +0000 (15:27 +0100)] 
Revise the cipher negotiation info about OpenVPN3 in the man page

Newer OpenVPN 3 core versions now allow limited configuration of ciphers:

    // Allow usage of legacy (cipher) algorithm that are no longer
    // considered safe
    // This includes BF-CBC, single DES and RC2 private key encryption.
    // With OpenSSL 3.0 this also instructs OpenSSL to load the legacy
    // provider.
    bool enableLegacyAlgorithms = false;

    // By default modern OpenVPN version (OpenVPN 2.6 and OpenVPN core
    // 3.7) will only allow
    // preferred algorithms (AES-GCM, Chacha20-Poly1305) that also work
    // with the newer DCO
    // implementations. If this is enabled, we fall back to allowing all
    // algorithms (if these are
    // supported by the crypto library)
    bool enableNonPreferredDCAlgorithms = false;

Adjust the man page section accordingly but only really mention the AEAD
ciphers to be always present and that they should be included in the
data-ciphers option.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230210142712.572303-7-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26226.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoAdd building unit tests with mingw to github actions
Arne Schwabe [Thu, 9 Feb 2023 16:37:05 +0000 (17:37 +0100)] 
Add building unit tests with mingw to github actions

This runs each test in its own action since order of stderr and stdout
is seemingly random in github action Windows output and this way at least
tests outputs are grouped by test

Patch v2: use -static-libgcc to avoid comping gcc runtime libraries.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20230209163705.466173-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26204.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoImprove format specifier for socket handle in Windows
Lev Stipakov [Fri, 10 Feb 2023 13:31:59 +0000 (15:31 +0200)] 
Improve format specifier for socket handle in Windows

Socket is a handle on Windows, which is usually logged in hex.
Also an interesting value is INVALID_SOCKET, which is ~0.

PRIuPTR prints decimals, and for INVALID_SOCKET it prints something like

  2023-02-10 14:45:21 us=906000 write to TUN/TAP : Jrjestelmkutsulle
annettu data-alue on liian pieni.   (fd=18446744073709551615,code=122)

PRIxPTR prints hex, and INVALID_SOCKET looks a bit nicer:

  2023-02-10 15:17:11 us=828000 write to TUN/TAP : Jrjestelmkutsulle
annettu data-alue on liian pieni.   (fd=ffffffffffffffff,code=122)

Reported-by: Selva Nair <selva.nair@gmail.com>
Signed-off-by: Lev Stipakov <lev@openvpn.net>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20230210133159.1336-1-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26220.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoUpdate the last sections in the man page to a be a bit less outdated
Arne Schwabe [Fri, 10 Feb 2023 14:27:09 +0000 (15:27 +0100)] 
Update the last sections in the man page to a be a bit less outdated

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20230210142712.572303-6-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26224.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoCombine extra_tun/frame parameter of frame_calculate_payload_overhead
Arne Schwabe [Fri, 10 Feb 2023 14:27:06 +0000 (15:27 +0100)] 
Combine extra_tun/frame parameter of frame_calculate_payload_overhead

Instead of passing a value and a bool just pass the value and 0 if
the caller does not want the value to be added. This also allows
the function to be used by a function without a frame struct.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20230210142712.572303-3-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26223.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agodco-win: use proper calling convention on x86
Lev Stipakov [Tue, 31 Jan 2023 12:54:48 +0000 (14:54 +0200)] 
dco-win: use proper calling convention on x86

WinAPI uses __stdcall calling convention on x86. Wrong
calling convention causes UB, which in this case breaks
dco-win functionality.

Signed-off-by: Lev Stipakov <lev@openvpn.net>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230131125448.1913-1-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26113.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoBuild unit tests in mingw Windows build
Selva Nair [Wed, 8 Feb 2023 00:59:25 +0000 (19:59 -0500)] 
Build unit tests in mingw Windows build

- Minor changes to the build system to include some
  dependencies for Windows build

- test_tls_crypt not built as it will pull in win32.c and
  its dependencies

- If cross-compiling, "make check" will only build the tests but not
  run any. Copy to Windows and run manually. Executables are in
  <buid-dir>/tests/unit_tests/openvpn/.libs/ and these depend on
  cmocka.dll in addition to openssl libs that some tests link to.

  Building with mingw on Windows should run the tests (untested).

v2: networking_testdriver was mistakenly enabled to run, while
originally it was only set to build. Corrected.

v3: exclude check_engine_keys.sh when cross-compiling
As suggested by Arne Schwabe <arne@rfc2549.org>

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20230208005925.393200-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26188.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoAdd missing stdint.h includes in unit tests files
Arne Schwabe [Wed, 8 Feb 2023 00:18:18 +0000 (01:18 +0100)] 
Add missing stdint.h includes in unit tests files

My mingw compiler/headers (mingw-w64 10.0.0 on macOS) seem to be more
pendantic than the one that comes with Ubuntu 22.04 (github actions) or
any of the other platforms including msvc/normal windows header.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20230208001819.244694-5-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26182.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoGet rid of unused 'bool tuntap_buffer' arguments.
Gert Doering [Wed, 1 Feb 2023 14:15:18 +0000 (14:15 +0000)] 
Get rid of unused 'bool tuntap_buffer' arguments.

overlapped_io_init() has a "bool tuntap_buffer" argument which is only
passed onwards to alloc_buf_sock_tun(), which does nothing with it.

Remove from both functions.

v2:
  move alloc_buf_sock_tun() to win32.c

v3:
  leave alloc_buf_sock_tun() where it is, and fix non-WIN32 call from
socket.c

Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20230130161730.110021-1-gert@greenie.muc.de>
URL:
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26099.h
tml
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit bdc842d72e92995261bac3579120c94f93e4064a)
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20230201141518.119157-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26122.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoConditionally add subdir-objects option to automake
Selva Nair [Sat, 4 Feb 2023 00:45:10 +0000 (19:45 -0500)] 
Conditionally add subdir-objects option to automake

- Eliminates repeated warnings such as
  warning: source file '$(openvpn_srcdir)/env_set.c' is in a subdirectory,
  but option 'subdir-objects' is disabled
- Enabled only for automake >= 1.16 as older versions have a buggy
  implementation of this option

Main side effect of this option is that object files like
openvpnserv-blockdns.o are now created in src/openvpn where block-dns.c
resides instead of in src/openvpnserv.

Same for object files for sources from $(openvpn_srcdir) compiled
into test executables.

See also past discussion on this topic:

https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg00013.html

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20230204004512.250271-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26147.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoAllow certain DHCP options to be used without DHCP server
Lev Stipakov [Tue, 7 Feb 2023 14:54:16 +0000 (16:54 +0200)] 
Allow certain DHCP options to be used without DHCP server

Followin DHCP options:

  DOMAIN, ADAPTER_DOMAIN_SUFFIX, DNS, WINS

don't require DHCP server in order to be used.

This change allows those options to be used with dco and wintun
drivers. If an option specified which requires DHCP server and
tap-windows6 driver is not used, print a clear error message
instead of obscure reference to --ip-win32.

Reported-by: Marek Zarychta
Signed-off-by: Lev Stipakov <lev@openvpn.net>
Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <20230207145416.1415-1-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26169.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoFix LibreSSL not building in Github Actions
Arne Schwabe [Thu, 9 Feb 2023 16:31:15 +0000 (17:31 +0100)] 
Fix LibreSSL not building in Github Actions

During the build of LibreSSL portable it pulls in a branch from OpenBSD
upstream. Unfortunately they use master there instead of a fixed branch.
So we work around this issue.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230209163115.465548-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/search?l=mid&q=20230209163115.465548-1-arne@rfc2549.org
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoWindows: fix unused variable in win32_get_arch
Frank Lichtenheld [Fri, 3 Feb 2023 19:14:39 +0000 (20:14 +0100)] 
Windows: fix unused variable in win32_get_arch

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20230203191440.136050-5-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26141.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoWindows: fix wrong printf format in x_check_status
Frank Lichtenheld [Tue, 7 Feb 2023 13:43:33 +0000 (14:43 +0100)] 
Windows: fix wrong printf format in x_check_status

Relevant defines/typedefs:
typedef UINT_PTR        SOCKET;
if defined(_WIN64)
 typedef unsigned __int64 UINT_PTR;
else
 typedef unsigned int UINT_PTR;
endif
ifdef _WIN64
 define PRIuPTR  PRIu64
else
 define PRIuPTR  PRIu32
endif

Remove duplicated include of inttypes.h

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20230207134333.52221-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26166.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoWindows: fix unused variables in delete_route_ipv6
Frank Lichtenheld [Fri, 3 Feb 2023 19:14:37 +0000 (20:14 +0100)] 
Windows: fix unused variables in delete_route_ipv6

At this point it might be easier to create a
dedicated function for Windows...

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20230203191440.136050-3-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26140.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoWindows: fix unused function setenv_foreign_option
Frank Lichtenheld [Fri, 3 Feb 2023 19:14:36 +0000 (20:14 +0100)] 
Windows: fix unused function setenv_foreign_option

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20230203191440.136050-2-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26145.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoblock-dns using iservice: fix a potential double free
Selva Nair [Wed, 1 Feb 2023 17:07:35 +0000 (12:07 -0500)] 
block-dns using iservice: fix a potential double free

- An item added to undo-list was not removed on error, causing
  attempt to free again in Undo().
  Also fix a memory leak possibility in the same context.

Github: fixes OpenVPN/openvpn#232

v2: Split add and delete functions and reuse the delete
function for cleanup.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20230201170735.2266851-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26130.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoChanges.rst: document removal of --keysize
Frank Lichtenheld [Wed, 1 Feb 2023 13:52:21 +0000 (14:52 +0100)] 
Changes.rst: document removal of --keysize

When reviweing OpenVPN/openvpn#231 I noticed this was
missing from Changes.rst.

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230201135221.36135-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26121.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoAdd printing USAN stack trace on github actions
Arne Schwabe [Mon, 30 Jan 2023 17:29:35 +0000 (18:29 +0100)] 
Add printing USAN stack trace on github actions

This allows identifying the source of undefined behaviour more easily
from the github action logs.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20230130172936.3444840-4-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26102.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoUpdate LibreSSL to 3.7.0 in Github actions
Arne Schwabe [Mon, 30 Jan 2023 17:29:34 +0000 (18:29 +0100)] 
Update LibreSSL to 3.7.0 in Github actions

The version 3.5.3 triggers undefined behaviour with the usan sanatizer.
Updating LibreSSSL to 3.7.0 does unfortunately does not fix the issue but
at least we are now using a current version.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20230130172936.3444840-3-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26105.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoFix unaligned access in auth-token
Arne Schwabe [Mon, 30 Jan 2023 17:29:32 +0000 (18:29 +0100)] 
Fix unaligned access in auth-token

The undefined behaviour USAN clang checker found this. The optimiser
of clang/gcc will optimise the memcpy away in the auth_token case and
output excactly the same assembly on amd64/arm64 but it is still better
to not rely on undefined behaviour.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20230130172936.3444840-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26103.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agomake dist: Ship ovpn_dco_freebsd.h, too
Matthias Andree [Fri, 27 Jan 2023 20:32:08 +0000 (21:32 +0100)] 
make dist: Ship ovpn_dco_freebsd.h, too

This file was missing from src/openvpn/Makefile.am.
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230127203208.305638-1-matthias.andree@gmx.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26085.html

Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agodco_linux: update license for ovpn_dco_linux.h
Antonio Quartulli [Wed, 25 Jan 2023 09:53:21 +0000 (10:53 +0100)] 
dco_linux: update license for ovpn_dco_linux.h

The linux userspace API header has acquired the MIT license (check the
ovpn-dco repository for the related change), therefore we simply bring
this change in our local copy to ensure compliancy.

Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230125095321.23063-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26077.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoopenvpnmsica: fix adapters discovery logic for DCO
Lev Stipakov [Tue, 24 Jan 2023 14:23:16 +0000 (16:23 +0200)] 
openvpnmsica: fix adapters discovery logic for DCO

Custom action "FindSystemInfo" finds adapters with certain hwid and
assigns found adapters' guids to a certain property. Later another custom
action "EvaluateTUNTAPAdapters" schedules adapter creation if the
abovementioned property is not set - which means no adapters exist
with given hwid.

I think this logic is needed to prevent duplicate adapter creation
if adapter was renamed and then new version is installed.

As one can see, there is a typo in property name ("OVPNDCOAPTERS"). As
a result of this typo, installer will always try to create DCO adapter
no matter if there are existing adapters. It however won't do anything
if adapter with the name "OpenVPN Data Channel Offload" already exists,
this is handled in schedule_adapter_create() function.

Because of that typo, following scenario works fine:

 1) Upcoming release of OpenVPN Connect is installed, which creates
adapter named "OpenVPN Connect DCO Adapter"

 2) OpenVPN-GUI is installed. Because of typo, it ignores adapter created
by Connect and creates own "OpenVPN Data Channel Offload" adapter

 3) OpenVPN Connect is uninstalled and it removes
"OpenVPN Connect DCO Adapter".

 4) OpenVPN-GUI still has its "OpenVPN Data Channel Offload" adapter

If we just fix a typo, OpenVPN-GUI won't create a adapter on step 2 and
after Connect removal on step 3 there won't be DCO adapters anymore
for OpenVPN-GUI to use.

The ultimate solution to this would be moving adapter creation to MSM,
a shared component which adds/removes the DCO driver. However this change
is not trivial and requires a lot of work. For the time being we apply
this band-aid by excluding Connect-created adapters from enumerations in
"FindSystemInfo" custom action. This makes sure that OpenVPN-GUI won't
rely on adapter created by Connnect (which is deleted on Connect uninstall)
and ensures that additional DCO adapters won't be created on upgrade
if user decides to rename adapter.

Signed-off-by: Lev Stipakov <lev@openvpn.net>
Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20230124142316.441-1-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26072.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoopenvpnmsica: remove unused declarations
Lev Stipakov [Tue, 24 Jan 2023 09:14:41 +0000 (11:14 +0200)] 
openvpnmsica: remove unused declarations

That code has been moved to MSM by commit 640c4d82
("openvpnmsica: remove dco installer custom actions")

Signed-off-by: Lev Stipakov <lev@openvpn.net>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230124091441.397-1-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26070.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoFix one more 'existing route may get deleted' case
Selva Nair [Sat, 21 Jan 2023 19:42:26 +0000 (14:42 -0500)] 
Fix one more 'existing route may get deleted' case

- Ensure net_route_v4/v6_add/del() functions using iproute2 return
  error when route addition fails. Return value follows the same logic
  as corresponding functions using netlink though all failure reasons
  get the same error code of -1.

TODO: Preserve any preexisting direct route to VPN and optionally the
IPv6 connected net route.

v2: Following review, removed the poorly coded RL_DID_LOCAL-related chunks.
That part needs a better fix.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230121194226.2081637-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26067.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoCleanup route error and debug logging on Windows
Selva Nair [Fri, 20 Jan 2023 09:41:00 +0000 (04:41 -0500)] 
Cleanup route error and debug logging on Windows

Use a unified logging format for various route-methods

- Route add/delete errors are always logged with M_WARN, so
  log only additional information (succeed/exists) with D_ROUTE.

- Non-windows platforms log route errors with a prefix "ERROR:" and
  debug info with "ROUTE:". Do the same on Windows. Do not log
  errors or success multiple times.

- In add_route_ipv6, log the interface id instead of device name
  as the latter always point to the tun/tap adapter name on Windows.

Log lines prefixed with a PACKAGE_NAME "ROUTE" are unchanged.
They appear to use the same format on all platforms.

v2: rebase to master

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20230120094100.2063883-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26058.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoWarn when pkcs11-id or pkcs11-id-management options are ignored
Selva Nair [Fri, 20 Jan 2023 02:18:41 +0000 (21:18 -0500)] 
Warn when pkcs11-id or pkcs11-id-management options are ignored

- If there are no pkcs11-providers either directly specified or
  through p11-kit-proxy made available through a build-time detection,
  these options are ignored. Log a warning in such cases.

  Especially important on Windows where automatic loading of p11-kit
  is not enabled in our release builds.

- Document this behaviour.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20230120021841.2048791-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26056.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoopenvpnmsica: remove dco installer custom actions
Lev Stipakov [Thu, 19 Jan 2023 08:59:59 +0000 (10:59 +0200)] 
openvpnmsica: remove dco installer custom actions

Those have been moved into MSM to be reused by openvpn-gui and Connect.

Signed-off-by: Lev Stipakov <lev@openvpn.net>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230119085959.157-1-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26053.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoWorkaround: make ovpn-dco more reliable
Arne Schwabe [Thu, 12 Jan 2023 16:37:37 +0000 (17:37 +0100)] 
Workaround: make ovpn-dco more reliable

This workaround avoids the kernel trigger ENOBUFS when the kernel
internal queue is overrun with events of disconnectingh clients or
similar. This is a workaround until we come up with a more permanent
solution.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20230112163737.1240059-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25988.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoDon't clear capability bounding set on capng_change_id
Timo Rothenpieler [Wed, 18 Jan 2023 14:24:28 +0000 (15:24 +0100)] 
Don't clear capability bounding set on capng_change_id

The bounding set being empty will overpower the likes of su/sudo
and will make it impossible for any child processes to ever gain
additional privileges again.

Github: fixes OpenVPN/openvpn#220

Signed-off-by: Timo Rothenpieler <timo@rothenpieler.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230118142428.162-1-timo@rothenpieler.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26048.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoRepair special-casing of EEXIST for Linux/SITNL route install
Gert Doering [Wed, 18 Jan 2023 07:46:33 +0000 (08:46 +0100)] 
Repair special-casing of EEXIST for Linux/SITNL route install

The code in sitnl_route_set() used to treat "route can not be installed
because it already exists" (EEXIST) as "not an error".

This is arguably a reasonable approach, but needs to handled higher
up - if the low level add_route() function say "no error", we will try
to remove that route later on in delete_route(), possibly removing
someone else's "already existing" route then.

So:
 - remove special case in sitnl_route_set()
 - do not pass NLM_F_REPLACE flag to sitnl_route_set() call - this would
   cause netlink to just replace existing routes, never return EEXIST
   (see "man netlink(7)")
 - add detailed return code handling to add_route(), assign "2" on
"-EEXIST"
   (and log appropriate message).

(Note: sitnl_route_set() is a common function for sitnl route add and
delete, but EEXIST can not happen on delete - so this change has no
impact for the "delete" case)

v2: use RTA_ macros, also adjust add_route_ipv6()

Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20230118074633.27586-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26046.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoDefine and use macros for route addition status code
Selva Nair [Sun, 15 Jan 2023 16:48:18 +0000 (11:48 -0500)] 
Define and use macros for route addition status code

- Instead of 0, 1, 2 use RTA_ERROR, RTA_SUCCESS, RTA_EEXIST
  as the return code of route addition functions.

- Also fix a logging error: status -> (status == RTA_SUCCESS)

v2: fold long lines
    use "bool ret = .." pattern for android too
    fix two more lines where status was directly assigned to bool

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230115164818.1973210-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26041.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoFix OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT breakage on FreeBSD+DCO
Gert Doering [Fri, 13 Jan 2023 08:07:45 +0000 (09:07 +0100)] 
Fix OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT breakage on FreeBSD+DCO

commit 67c4eebdae introduces a new peer disconnect reason (transport
disconnected, aka "TCP session closed") which breaks compilation on
FreeBSD - OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT not part of the
enum in freebsd_dco.h, and no kernel support for TCP anyway.

This patch is an intermediate bandaid, making the offending code in
multi.c "linux only" while a better solution is discussed.

Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20230113080745.82783-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/search?l=mid&q=20230113080745.82783-1-gert@greenie.muc.de
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agodco: print proper message in case of transport disconnection
Antonio Quartulli [Wed, 11 Jan 2023 23:50:52 +0000 (00:50 +0100)] 
dco: print proper message in case of transport disconnection

Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20230111235052.24855-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25977.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoDeprecate OCC checking
Arne Schwabe [Wed, 11 Jan 2023 13:44:39 +0000 (14:44 +0100)] 
Deprecate OCC checking

- Move OCC warnings to debug level. This moves the only useful OCC message
  of compress-migrate to D_PUSH
- remove configure option --enable-strict-options
- ignore disable-occ in TLS mode as it is logged under debug now only
  disable-occ is now strictly a non-TLS option
- mark opt-verify and disable-occ as deprecated.

Patch v2: change one missed M_WARN to D_OCC

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230111134439.1107915-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25970.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agodocumentation: update 'unsupported options' section
Frank Lichtenheld [Wed, 11 Jan 2023 12:52:42 +0000 (13:52 +0100)] 
documentation: update 'unsupported options' section

We listed those in Changes, but did not update the documentation.

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230111125242.21025-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25968.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agocheck_engine_keys: make pass with OpenSSL 3
Frank Lichtenheld [Tue, 10 Jan 2023 17:02:57 +0000 (18:02 +0100)] 
check_engine_keys: make pass with OpenSSL 3

Not enabled by default with OpenSSL 3, so we don't
see this in our builds.
While here add missing entries to .gitignore (which
is what made me look at engine-key test in the first
place).

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230110170257.113527-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25949.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agooptions: Always define options->management_flags
Frank Lichtenheld [Sun, 27 Nov 2022 14:25:06 +0000 (15:25 +0100)] 
options: Always define options->management_flags

That makes it possible to remove several preprocessor
directives which is a good thing. The cost should be
negligible.

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20221127142506.41986-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25554.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoInclude CE_DISABLED status of remote in "remote-entry-get" response
Selva Nair [Wed, 11 Jan 2023 06:29:10 +0000 (01:29 -0500)] 
Include CE_DISABLED status of remote in "remote-entry-get" response

- The response to the management command "remote-entry-get" is
  amended to include the status of the remote entry. The status
  reads "disabled" if (ce->flag & DISABLED) is true, "enabled"
  otherwise.

- Update and correct the description of this option in
  management-notes.txt

  Example responses:
  In response to "remote-entry-get 0"

  0,vpn.example.com,udp,enabled
  END

  Or, in response to "remote-entry-get all"

  0,vpn.example.org,udp,enabled
  1,vpn.example.com,udp,enabled
  2,vpn.example.net,tcp-client,disabled
  END

This helps the management client to show only enabled remotes
to the user.
An alternative would require the  UI/GUI to have knowledge of
what makes the daemon set CE_DISABLED (--proto-force,
--htttp-proxy-override etc.).

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230111062910.1846688-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/search?l=mid&q=20230111062910.1846688-1-selva.nair@gmail.com
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoxkey_pkcs11h_sign: fix dangling pointer
Frank Lichtenheld [Tue, 10 Jan 2023 13:19:47 +0000 (14:19 +0100)] 
xkey_pkcs11h_sign: fix dangling pointer

Warning by GCC 12:
pkcs11_openssl.c:237:22: warning:
dangling pointer ‘tbs’ to ‘enc’ may be used [-Wdangling-pointer=]

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20230110131947.59552-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25942.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoUpdate copyright year to 2023
Frank Lichtenheld [Tue, 10 Jan 2023 16:05:31 +0000 (17:05 +0100)] 
Update copyright year to 2023

Manually excluded ovpn_dco_win.h because it is an
imported file. ovpn_dco_linux.h is already excluded
because it still says 2021.

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230110160531.81010-1-frank@lichtenheld.com>
URL: https://patchwork.openvpn.net/project/openvpn2/patch/20230110160531.81010-1-frank@lichtenheld.com/
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoLog peer-id if loglevel is D_DCO_DEBUG and dco is enabled
Arne Schwabe [Tue, 10 Jan 2023 15:19:01 +0000 (16:19 +0100)] 
Log peer-id if loglevel is D_DCO_DEBUG and dco is enabled

This enables logging the peer id in p2mp mode if dco is enabled
and the log level is high enough

Patch v2: use check_debug_level to check current log level

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230110151901.998479-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25946.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoReduce logspam about 'dco_update_keys: peer_id=-1' in p2p server mode
Gert Doering [Mon, 9 Jan 2023 20:00:11 +0000 (21:00 +0100)] 
Reduce logspam about 'dco_update_keys: peer_id=-1' in p2p server mode

p2p --tls-server with no active client/peer logs once per second

  "dco_update_keys: peer_id=-1"

which does exactly nothing, except fill the disk.  So skip the call to
dco_update_keys() if peer_id == -1.

Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <20230109200011.2525342-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25935.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoPropagate route error to initialization_completed()
Selva Nair [Thu, 5 Jan 2023 02:27:18 +0000 (21:27 -0500)] 
Propagate route error to initialization_completed()

Makes it possible to report management state as CONNECTED,ROUTE_ERROR
instead of CONNECTED,SUCCESS in case of routing errors.

This depends on treating "route already exists" as not
an error which right now works when using netlink on Linux
and IPAPI or iservice on Windows.

For route set via command line there is no easy way to get this
information and current behaviour is unchanged: i.e., the management
state continues to be reported as CONNECTED,SUCCESS.

Status notification to systemd is not affected.

To test on Linux, build with netlink and use a --route option with
an unreachable gateway like:
"--route 192.168.122.0 255.255.255.0 1.1.1.1"

Notes:
On windows, if the route method is "exe", setting a route
that exists *may* get logged as error and this patch will lead to
a slightly misleading CONNECTED,ROUTE_ERROR state message. This is
considered tolerable as no one should be using "exe" (i.e. route.exe)
as the route method.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230105022718.1641751-3-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25884.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoAdd connect-freq-initial option to limit initial connection responses
Arne Schwabe [Tue, 10 Jan 2023 01:59:01 +0000 (02:59 +0100)] 
Add connect-freq-initial option to limit initial connection responses

This limits the number of packets OpenVPN will respond to. This avoids
OpenVPN servers being abused for refelection attacks in a large scale
as we gotten a lot more efficient with the cookie approach in our
initial connection handling.

The defaults of 100 attempts per 10s should work for most people,
esepcially since completed three way handshakes are not counted. So
the default will throttle connection attempts on server with high packet
loss or that are actually under a DOS.

The 100 per 10s are similar in size to the old 2.5 and earlier behaviour
where every initial connection attempt would take up a slot of the
max-clients sessions and those would only expire after the TLS timeout.
This roughly translates to 1024 connection attempts in 60s on an
empty server.

OpenVPN will announce once per period when starting to drop packets and
ultimatively how many packets it dropped:

    Connection Attempt Note: --connect-freq-initial 100 10 rate limit
    exceeded, dropping initial handshake packets for the next 10 seconds

    Connection Attempt Dropped 217 initial handshake packets due to
    --connect-freq-initial 100 10

to inform an admin about the consequences of this feature.

Patch v2: use strtol instead of atoi to be able to differentiate between
          an error parsing and parsing 0. Use int64_t instead int to
          avoid overflow errors.

Patch v3: Add message when we start dropping. Add a few fixes to the logic.
          improve docs

Patch v4: missing missing return statement.
Patch v5: add build files for msvc build

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230110015901.933522-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25938.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoUndo FreeBSD 12.x workaround on IPv6 ifconfig for 12.4 and up
Gert Doering [Sat, 7 Jan 2023 16:25:58 +0000 (17:25 +0100)] 
Undo FreeBSD 12.x workaround on IPv6 ifconfig for 12.4 and up

commit 5e19cc2c1bf22d introduced a workaround for a race condition
that showed itself on IPv6 ifconfig on FreeBSD 12.x - sometimes breaking
IPv6 connectivity on tun/tap interfaces.

This was fixed on the FreeBSD side in 12.4, 13.1 and up, and 13.0 is
no longer supported.  So conditionalize the workaround on "12.0..12.3",
to be fully removed later when 12.3 is also running out of support.

v2: fix version number comparison

Trac: 1226

Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230107162558.59659-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25911.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoDistinguish route addition errors from route already exists
Selva Nair [Fri, 6 Jan 2023 15:04:12 +0000 (10:04 -0500)] 
Distinguish route addition errors from route already exists

When possible, functions that add a route now return 1 on success,
or 2 if route already exists or 0 on other errors instead of true/false.

Note:
net_route_v4/v6_add using netlink filters out EEXIST before returning
this looks like a bug as add_route() and add_route_ipv6() should set
RT_ADDED only if route was really added.

v2: "succeeded/skipped" --> "succeeded" in log.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230106150412.1667492-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25903.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agotun: move print_windows_driver() out of tun.h
Lev Stipakov [Mon, 9 Jan 2023 11:30:46 +0000 (13:30 +0200)] 
tun: move print_windows_driver() out of tun.h

We got warnings from MinGW about function being defined
but not used when compiling modules which include tun.h.

This function is not defined as inline, so its definition
should not be in header. Since this is not a performance
critical, no need to make it inline.

Leave declaration in tun.h and move definition to tun.c.

Signed-off-by: Lev Stipakov <lev@openvpn.net>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230109113046.1678-1-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25923.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoAssign and honour signal priority order
Selva Nair [Sun, 1 Jan 2023 21:51:07 +0000 (16:51 -0500)] 
Assign and honour signal priority order

Signals are ordered as SIGUSR2, SIGUSR1, SIGHUP, SIGTERM, SIGINT
in increasing priority. Lower priority signals are not allowed to
overwrite higher ones.

This should fix Trac #311, #639 -- SIGTER/SIGINT lost during dns
resolution (except for the Windows-specific bug handled in previous commit).

On sending SIGTERM during dns resolution, it still takes several seconds
to terminate as the signal will get processed only after getaddrinfo times
out twice (in phase1 and phase2 inits).

Github: fixes OpenVPN/openvpn#205
Trac: #311, #639

Note: one has to still wait for address resolution to time out as
getaddrinfo() is no interruptible. But a single ctrl-C (and some
patience) is enough.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20230101215109.1521549-4-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25871.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoFix signal handling on Windows
Selva Nair [Fri, 6 Jan 2023 00:54:38 +0000 (19:54 -0500)] 
Fix signal handling on Windows

- In win32_signal_get() re-order the check so that Windows
  signals are picked up even if signal_received is non-zero

- When management is not active, management_sleep() becomes sleep()
  but it is not interruptible by signals on Windows. Fix this by
  periodically checking for signal.

Trac: #311 #639 (windows specific part)
Github: Fixes OpenVPN/openvpn#205 (windows specific part)

Note: if stuck in address resolution, press ctrl-C and wait for
getaddrinfo() to timeout.

v2: WIN32 --> _WIN32
    add a chunk in management_sleep that was missed by sloppy
    conflict-resolution

v3: following review by Lev Stipakov <lstipakov@gmail.com>
  win32_sleep()
    - Early fallback to Sleep() if no wait handles -- less indentation
    - Check signal only if wait-object triggered
    - Exit the while loop if not safe to continue
  Behaviour of win32_sleep(0) checking signal is retained though may be
  redundant

v4: Avoid Sleep(0) and never loop back to wait again if wait-failed

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20230106005438.1664046-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25895.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoUse IPAPI for setting ipv6 routes when iservice not available
Selva Nair [Thu, 5 Jan 2023 02:27:16 +0000 (21:27 -0500)] 
Use IPAPI for setting ipv6 routes when iservice not available

Currently we use netsh for this. The new code closely follows
what interactive service does.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20230105022718.1641751-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25886.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agodco: improve comment about hidden debug message
Antonio Quartulli [Tue, 3 Jan 2023 20:23:30 +0000 (21:23 +0100)] 
dco: improve comment about hidden debug message

While at it also improve the debug message itself
to be more self-explanatory.

Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20230103202330.1835-3-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25883.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agodco: bail out when no peer-specific message is delivered
Antonio Quartulli [Tue, 3 Jan 2023 20:23:29 +0000 (21:23 +0100)] 
dco: bail out when no peer-specific message is delivered

multi_process_incoming_dco() is currently partly processing
messages that were actually discarded. This results in a bogus
message being printed:

  "Received packet for peer-id unknown to OpenVPN: -1, type 0, reason 2"

Change the flow so that we bail out immediately when we know that no
message was truly delivered by DCO.
Currently this can be verified by checking that the peer_is is greater
than -1.

Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20230103202330.1835-2-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25882.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agodco: properly re-initialize dco_del_peer_reason
Antonio Quartulli [Tue, 3 Jan 2023 20:23:28 +0000 (21:23 +0100)] 
dco: properly re-initialize dco_del_peer_reason

After processing a message, all fields of the dco object should be
re-initialized so that future processings are not affected by stale
values.

This includes dco_del_peer_reason.

Since its values can start at 0, re-initialize it with -1.

Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20230103202330.1835-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25881.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoRefactor signal handling in openvpn_getaddrinfo
Selva Nair [Sun, 1 Jan 2023 21:51:06 +0000 (16:51 -0500)] 
Refactor signal handling in openvpn_getaddrinfo

Pass in sig_info struct to use register signal instead of
modifying signal_received.

No functional changes though some may be warranted.
Questions:
  - Why are we overwriting SIGUSR1 in this function?
  - Why the special interrupted syscall treatment for getaddrinfo?
    Its not a syscall, is it?

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230101215109.1521549-3-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25872.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoPreparing for better signal handling: some code refactoring
Selva Nair [Sun, 1 Jan 2023 21:51:05 +0000 (16:51 -0500)] 
Preparing for better signal handling: some code refactoring

- Do not directly update signal_received: always use register_signal()
  throw_signal() or signal_reset().
  To facilitate this, register_signal() now takes c->sig as an argument
  instead of the context c itself, and sig_info struct is passed-in to
  functions that need to set a signal.

- openvpn_getaddrinfo() is updated in a following commit as it
  could benefit from some logic changes that we may or may not want
  to do.

No functional changes.

TODO:
(i)   update signal handling in openvpn_getaddrinfo
(ii)  enforce signal priority
(iii) fix signal handling on Windows
for 2.7?
(iv)  replace system-V signal with POSIX sigaction

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20230101215109.1521549-2-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25874.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoCleanup: Close duplicated handles in interactive service
Selva Nair [Thu, 29 Dec 2022 18:27:39 +0000 (13:27 -0500)] 
Cleanup: Close duplicated handles in interactive service

Several handles from openvpn.exe are duplicated in the
service for registering ring buffer memory maps with the
driver. These handles are not required after registration,
as all access is through handles in openvpn.exe. Only the
map base address (send_ring, rceive_ring) need be retained
for later unmapping.

Use local variables for duplicated handles and close them
soon after use.

The struct ring_buffer_handles_t is renamed to ring_buffer_maps_t
as there are no handles in there any longer.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20221229182739.1477336-2-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25863.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoUse undo_lists for saving ring-buffer handles in interactive service
Selva Nair [Thu, 29 Dec 2022 18:27:38 +0000 (13:27 -0500)] 
Use undo_lists for saving ring-buffer handles in interactive service

HandleRegisterRingBuffers() in interactive.c did not follow the
the original API of HandleMessage(): a new argument was added
to HandleMessage to pass-in prer-process ring-buffer handles. The
existing undo lists argument is meant for such use.

Rewrite following the original design.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20221229182739.1477336-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25864.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agoProperly unmap ring buffer file-map in interactive service
Selva Nair [Thu, 29 Dec 2022 13:47:29 +0000 (08:47 -0500)] 
Properly unmap ring buffer file-map in interactive service

The return value of MapViewOfFile must be passed to UnmapViewofFile,
instead of the file handle.

Github: Fixes OpenVPN/openvpn#206

v2: move *ring = NULL inside if {}

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Message-Id: <20221229134729.1474034-1-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25859.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2 years agooptions.c: update usage description of --cipher
Frank Lichtenheld [Wed, 28 Dec 2022 17:13:14 +0000 (18:13 +0100)] 
options.c: update usage description of --cipher

GCC with -O3 complains:
warning: ‘%s’ directive argument is null [-Wformat-overflow=]

And indeed:
--cipher alg    : Encrypt packets with cipher algorithm alg
                  (default=(null)).

Since there is no real default anymore, remove it.
While here also indicate the somewhat-deprecated status
of the option.

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221228171314.133115-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25851.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>