Re: [Openvpn-devel] [PATCH v2] Implement server_poll_timeout for socks

[Arne Schwabe]

Am 20.03.24 um 13:06 schrieb Gert Doering:

Hi,

On Fri, Mar 15, 2024 at 05:40:02PM +0100, Frank Lichtenheld wrote:

Code looks good and I tested build and default t_client tests.
However, not sure how exactly to verify that it actually works.
The SOCKS proxy I have doesn't exhibit any problems even with
--connect-timeout 1.

Any ideas for testing welcome.


As far as I understand this fix, it's replacing the 5s hard coded timeout
for "handshake with the SOCKS proxy" with --connect-timeout.

So to exhibit any change in behaviour, you need a slow SOCKs proxy...

I have a few ideas how to test this, like

  - --socks-proxy :10080
(should fail after 5 seconds without the patch, after --connect-timeout
with the patch)

  - hack "something that can speak SOCKS" (like, ssh) to be really slow
in answering - like, "add a sleep(15) in the socks handshake path",
and point OpenVPN to that.  Without the patch, this should fail, with
the patch, it should succeeds.

(Note: 'ssh -D ' will do SOCKS, but not UDP -> TCP server needed)

I have no time just now to go hacking on this, so just putting out the
ideas...

For our regular testbed, testing all these timeouts is complicated
(and even if you succeed, it's sllloowww if you want reproduceable
results, so not "milliseconds timers").



Or use something like dummynet. At least on macOS it used to be easy to 
simulate a long delay with that and pfctl.


Arne



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Change include order for tests

[Arne Schwabe]

Am 12.02.24 um 14:25 schrieb Juliusz Sosinowicz:

Including "ssl.h" conflicts with the wolfSSL ssl.h header file. The openvpn/src 
directory needs to be included before include/wolfssl. include/wolfssl needs to be 
included so that openvpn can pick up wolfSSL compatibility headers instead of OpenSSL 
headers without changing the paths.

src/openvpn/Makefile.am does not need to be modified because AM_CPPFLAGS is 
placed before AM_CFLAGS in the output Makefile.




That looks reasonable and having test includes in a more similar order 
than our normal includes is also a good thing.


Acked-By: Arne Schwabe 



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] wolfssl: include "ssl.h" by "src/openvpn/ssl.h"

[Arne Schwabe]

Am 09.02.24 um 16:51 schrieb Juliusz Sosinowicz:

Including "ssl.h" conflicts with the wolfSSL ssl.h header file. The 
include/wolfssl directory is included before openvpn/src. include/wolfssl needs to be 
included so that openvpn can pick up wolfSSL compatibility headers instead of OpenSSL 
headers without changing the paths.


Neither the ssl.h (2005, so forever) in OpenVPN nor the ssl.h in wolfssl 
(2014 according to git blame) are particulary new. Why is this now a 
problem when it was never before?


Arne



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] documentation: Fixes for previous fixes to --push-peer-info

[Arne Schwabe]

Am 06.02.24 um 18:47 schrieb Frank Lichtenheld:

- Clarify compression IV_ settings
- Clarify which settings might come from --setenv

Change-Id: Id8615515c8df6e38e931e357396811234faad796
Signed-off-by: Frank Lichtenheld 
---
  doc/man-sections/client-options.rst | 13 +
  1 file changed, 9 insertions(+), 4 deletions(-)

For master and release/2.6

diff --git a/doc/man-sections/client-options.rst 
b/doc/man-sections/client-options.rst
index cd3e565f..b92b1a46 100644
--- a/doc/man-sections/client-options.rst
+++ b/doc/man-sections/client-options.rst
@@ -374,10 +374,11 @@ configuration.
:code:`IV_GUI_VER= `
  The UI version of a UI if one is running, for example
  :code:`de.blinkt.openvpn 0.5.47` for the Android app.
+This may be set by the client UI/GUI using ``--setenv``.
  
:code:`IV_SSO=[crtext,][openurl,][proxy_url]`

  Additional authentication methods supported by the client.
-This may be set by the client UI/GUI using ``--setenv``
+This may be set by the client UI/GUI using ``--setenv``.
  
The following flags depend on which compression formats are compiled in

and whether compression is allowed by options. See `Protocol options`_
@@ -388,13 +389,15 @@ configuration.
  
  :code:`IV_LZO_STUB=1`

  If client was built with LZO stub capability. This is only sent if
-``IV_LZO=1`` is not sent.
+``IV_LZO=1`` is not sent. This means the client can talk to a server
+configured with ``--comp-lzo no``.
  
  :code:`IV_LZ4=1` and :code:`IV_LZ4v2=1`

  If the client supports LZ4 compression.
  
  :code:`IV_COMP_STUB=1` and :code:`IV_COMP_STUBv2=1`

-If the client supports stub compression.
+If the client supports stub compression. This means the client can talk
+to a server configured with ``--compress``.
  
When ``--push-peer-info`` is enabled the additional information consists

of the following data:
@@ -414,7 +417,9 @@ configuration.
  
:code:`IV_PLAT_VER=x.y`

  The version of the operating system, e.g. 6.1 for Windows 7.
-This is only sent on Windows operating systems.
+This may be set by the client UI/GUI using ``--setenv``.
+On Windows systems it is automatically determined by openvpn
+itself.
  
:code:`UV_=`

  Client environment variables whose names start with


Thanks. Collective ACK with the previous patch together.

Acked-By: Arne Schwabe 


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] documentation: Update and fix documentation for --push-peer-info

[Arne Schwabe]

Am 06.02.24 um 15:10 schrieb Frank Lichtenheld:

- description of IV_PROTO was outdated, missing a lot
   of flags
- complete list of compression flags, but separate them out
- various other style/grammar/typo fixes

Change-Id: I7f854a5a14d2a2a391ebb78a2a92b3e14cfd8be6
Signed-off-by: Frank Lichtenheld 
---
  doc/man-sections/client-options.rst | 44 -
  1 file changed, 31 insertions(+), 13 deletions(-)

This patch should be applied to release/2.6 and master.

diff --git a/doc/man-sections/client-options.rst 
b/doc/man-sections/client-options.rst
index 54c4ec63..cd3e565f 100644
--- a/doc/man-sections/client-options.rst
+++ b/doc/man-sections/client-options.rst
@@ -339,31 +339,31 @@ configuration.
:code:`IV_PLAT=[linux|solaris|openbsd|mac|netbsd|freebsd|win]`
  The client OS platform
  
-  :code:`IV_LZO_STUB=1`

-If client was built with LZO stub capability
-
-  :code:`IV_LZ4=1`
-If the client supports LZ4 compressions.
-
:code:`IV_PROTO`
  Details about protocol extensions that the peer supports. The
-variable is a bitfield and the bits are defined as follows
-(starting a bit 0 for the first (unused) bit:
+variable is a bitfield and the bits are defined as follows:
  
+- bit 0: Reserved, should always be zero

  - bit 1: The peer supports peer-id floating mechanism
  - bit 2: The client expects a push-reply and the server may
send this reply without waiting for a push-request first.
  - bit 3: The client is capable of doing key derivation using
RFC5705 key material exporter.
  - bit 4: The client is capable of accepting additional arguments
-  to the `AUTH_PENDING` message.
+  to the ``AUTH_PENDING`` message.
+- bit 5: The client supports doing feature negotiation in P2P mode
+- bit 6: The client is capable of parsing and receiving the ``--dns`` 
pushed option
+- bit 7: The client is capable of sending exit notification via control 
channel using ``EXIT`` message. Also, the client is accepting the 
protocol-flags pushed option for the EKM capability
+- bit 8: The client is capable of accepting ``AUTH_FAILED,TEMP`` messages
+- bit 9: The client is capable of dynamic tls-crypt
  





+:code:`IV_COMP_STUB=1` and :code:`IV_COMP_STUBv2=1`
+If the client supports stub compression.
+


Maybe add a note that IV_COMP_STUB and IV_LZO_STUB are *not* identical 
or compatible. (byte swap)



  
:code:`IV_PLAT_VER=x.y`

  The version of the operating system, e.g. 6.1 for Windows 7.
+This is only sent on Windows operating systems.



This is wrong. My android client also send this as setenv IV_PLAT_VER is 
allowed. And openvpn3 based clients also send this iirc.


Arne


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] OpenVPN data channel format using 64bit IV

[Arne Schwabe]

- add protocol-flag aead-packet-format-v2 This signals the client to 
switch to the new data channel format.



And finally have the data channel format. Since this format is 
negotiated like the cipher, there is no need to use another opcode if 
keep the peer id to just 24 bit. But we might want to extend the 
format to have it 8 byte aligned to also allow peer-id to be extended 
in the future.


I'd at least consider to not send the upper 32 bits of the counter over 
the wire. With some simple arithmetic you can detect the counter 
overflow and keep the high bits internal to the application. I'd expect 
such per-packet arithmetic to be much cheaper than transmitting 4 bytes 
extra for each packet.


If you take that approach, you would not even need to change the wire 
format.


Even more, you might not even have to negotiate the option with the 
peer, because the peer will initiate a renegotiation after 0xFF00 
packets if it doesn't support the "implicit long PID". New peers will 
postpone the reneg to 0xFF00 packets.


Even as nice as that sounds, I am not super convinced that we should be 
the "odd protocol" here that takes a different approach than all other 
protocol. All other protocols that I looked at, rather use longer IV 
instead of doing some hidden counter that is not transmitted.


So I would rather err on the safe side here and go for a 8 byte IV instead.

Arne



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] OpenBSD: repair --show-gateway

[Arne Schwabe]

Am 01.01.24 um 10:40 schrieb Gert Doering:

OpenBSD route sockets do not want to be passed RTA_IFP on RTM_GET
- if we do this, we get back EINVAL.

On other platforms, if we do not request RTA_IFP, we will not get
back interface information for queried routes - on OpenBSD, RTA_IFP
comes back always...

So we need to #ifdef this, RTA_IFP on all platforms except OpenBSD.

(Found this fix in OpenBSD's ports tree, in their patches for OpenVPN
2.6.8 - but they just remove RTA_IFP, no #ifdef, so we can't just apply
their patch)

While at it, add M_ERRNO to the "write to routing socket" error message.


Acked-By: Arne Schwabe 



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] get_default_gateway() HWADDR overhaul

[Arne Schwabe]

Am 01.01.24 um 10:27 schrieb Gert Doering:

commit f13331005d5a7 (gerrit/454) most painfully works around the limitations
of the SIOCGIFCONF API, with struct member access on an unaligned buffer,
possibly overrunning sockaddr structures, etc. - and the result still did
not work on OpenSolaris and OpenBSD (no AF_LINK in the returned elements).

Reading through OpenBSD "ifconfig" source, I found getifaddrs(3), which
is exactly what we want here - it works on FreeBSD, NetBSD, OpenBSD and
MacOS, and all returned pointers are properly aligned, so the code gets
shorter, easier to read, and UBSAN is still happy.

OpenSolaris does have getifaddrs(3), but (surprise) it does not work, as
in "it does not return AF_LINK addresses".  It does have SIOCGIFHWADDR,
instead, and "man if_tcp" claims "should behave in a manner compatible
with Linux" - so TARGET_SOLARIS gets a copy of the Linux code now (works).




-if (!strncmp(ifr.ifr_name, rgi->iface, IFNAMSIZ))


Mini nitpick from clang-tidy:

Clang-Tidy: No header providing "strncmp" is directly included.

Adding string.h would could be done if we want a v2 of the patch

Tested also on macOS.

Acked-By: Arne Schwabe 



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] fix(ssl): init peer_id when init tls_multi

[Arne Schwabe]

Am 19.10.23 um 19:12 schrieb yatta:

From: pushan <62173185+pusha...@users.noreply.github.com>

When openvpn run in UDP server mode, if ssl connections reach the max clients, the 
next connection would be failed in `multi_create_instance` and the half connection 
will be close in `multi_close_instance`, which may lead array `m->instances[0]` 
 covered unexpectedly and make the first connection  interrupt, this patch fix 
this problem by init `peer_id` with `MAX_PEER_ID` in `tils_multi_init`.


A bit more explanaition on this:

When we create an instance in multi_create_instance we call

inherit_context_child(>context, >top);

which in turn calls the tls_multi_init that is patched here.

When I encounter an error during the creation in multi_create_instance 
we call multi_close_instance in the goto err branch. That 
multi_close_instance clears the instance from the dict if the peer_id is 
not MAX_PEER_ID.


We probably should refactor this to be a bit cleaner in the future.

Acked-By: Arne Schwabe 


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v3] Add missing check for nl_socket_alloc failure

[Arne Schwabe]

This can happen if the memory alloc fails.

Patch V2: add goto error
Patch V3: return -ENOMEM instead of going to error

Change-Id: Iee66caa794d267ac5f8bee584633352893047171
Signed-off-by: Arne Schwabe 
---
 src/openvpn/dco_linux.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index b033f8543..3c91606b7 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -81,6 +81,12 @@ resolve_ovpn_netlink_id(int msglevel)
 int ret;
 struct nl_sock *nl_sock = nl_socket_alloc();
 
+if (!nl_sock)
+{
+msg(msglevel, "Allocating net link socket failed");
+return -ENOMEM;
+}
+
 ret = genl_connect(nl_sock);
 if (ret)
 {
-- 
2.39.3 (Apple Git-145)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] protocol_dump: support tls-crypt

[Arne Schwabe]

Am 30.10.23 um 11:58 schrieb Reynir:

Dear list,

Please find attached a patch to add support for tls-crypt packets in 
protocol_dump. Currently, protocol_dump will print garbage for tls-crypt 
packets.


This patch makes protocol_dump print the clear text parts of the packet 
such as the auth tag and replay packet id. It does not try to print the 
wKc for HARD_RESET_CLIENT_V3 or CONTROL_WKC_V1 packets. A previous 
iteration, not submitted to the list, printed ENCRYPTED placeholders for 
ack list and DATA, but I decided to cut down on the noise instead.


This is my first patch submitted to openvpn so please bear with me.



Code looks good and works fine here.

Acked-By: Arne Schwabe 



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] OpenVPN3 thread safety

[Arne Schwabe]

Am 12.11.2023 um 14:16 schrieb Savely Krasovsky:

Hello!

I am trying to use OpenVPN3 with Golang SWIG binding. It works pretty nice, but 
I have random segmentation faults without obvious reason. My current guess is 
that Golang calls OpenVPN3 from various threads and library is not ready for 
that sometime. Am I right? Documentation doesn't say a lot about threading.

If this is a case, I have two possible solutions:
  1. Use OPENVPN_ENABLE_BIGMUTEX build flag (I am not sure I need that).
  2. Move all calls to OpenVPN3 into single goroutine and use 
runtime.LockOSThread().

Could you please clarify, should I get segfaults in that case or not and the 
problems is unrelated.



Yes. The client is singlethreaded and expects to be called only from a 
single thread. You can have multiple clients in multiple threads but all 
our own software uses just a single thread for the client itself.



Arne



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] doc: fix argument name in --route-delay documentation

[Arne Schwabe]

Am 13.10.23 um 12:23 schrieb Frank Lichtenheld:

Also remove redundant "by default".



Acked-By: Arne Schwabe 



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] Remove openssl engine method for loading the key

[Arne Schwabe]

This is a contribution for loading engine key. OpenSSL engine is
deprecated since OpenSSL 3.0 and James Bottomlehy has not agreed
to the proposed license chagne. He is also okay with removing
with the feature from the current code base as it is obsolete with
OpenSSL 3.0.

Change-Id: I2d353a0cea0a62f289b8c1060244df66dd7a14cb
Signed-off-by: Arne Schwabe 
---
 .gitignore|   4 -
 configure.ac  |   1 -
 src/openvpn/crypto_openssl.c  |  60 -
 src/openvpn/crypto_openssl.h  |  12 --
 src/openvpn/ssl_openssl.c |   4 -
 tests/unit_tests/Makefile.am  |   3 -
 tests/unit_tests/engine-key/Makefile.am   |  31 -
 .../engine-key/check_engine_keys.sh   |  36 --
 tests/unit_tests/engine-key/libtestengine.c   | 116 --
 tests/unit_tests/engine-key/openssl.cnf.in|  12 --
 10 files changed, 279 deletions(-)
 delete mode 100644 tests/unit_tests/engine-key/Makefile.am
 delete mode 100755 tests/unit_tests/engine-key/check_engine_keys.sh
 delete mode 100644 tests/unit_tests/engine-key/libtestengine.c
 delete mode 100644 tests/unit_tests/engine-key/openssl.cnf.in

diff --git a/.gitignore b/.gitignore
index ed03aaa95..a1da36672 100644
--- a/.gitignore
+++ b/.gitignore
@@ -57,10 +57,6 @@ tests/t_client-*-20??-??/
 t_client.rc
 t_client_ips.rc
 tests/unit_tests/**/*_testdriver
-tests/unit_tests/engine-key/client.key
-tests/unit_tests/engine-key/log.txt
-tests/unit_tests/engine-key/openssl.cnf
-tests/unit_tests/engine-key/passwd
 
 src/openvpn/openvpn
 include/openvpn-plugin.h
diff --git a/configure.ac b/configure.ac
index 266b66f0f..128ab86f7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1532,7 +1532,6 @@ AC_CONFIG_FILES([
 tests/unit_tests/openvpn/Makefile
 tests/unit_tests/plugins/Makefile
 tests/unit_tests/plugins/auth-pam/Makefile
-   tests/unit_tests/engine-key/Makefile
sample/Makefile
 ])
 AC_CONFIG_FILES([tests/t_client.sh], [chmod +x tests/t_client.sh])
diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index 22c6d6840..fe1254f89 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -1374,66 +1374,6 @@ memcmp_constant_time(const void *a, const void *b, 
size_t size)
 return CRYPTO_memcmp(a, b, size);
 }
 
-#if HAVE_OPENSSL_ENGINE
-static int
-ui_reader(UI *ui, UI_STRING *uis)
-{
-SSL_CTX *ctx = UI_get0_user_data(ui);
-
-if (UI_get_string_type(uis) == UIT_PROMPT)
-{
-pem_password_cb *cb = SSL_CTX_get_default_passwd_cb(ctx);
-void *d = SSL_CTX_get_default_passwd_cb_userdata(ctx);
-char password[64];
-
-cb(password, sizeof(password), 0, d);
-UI_set_result(ui, uis, password);
-
-return 1;
-}
-return 0;
-}
-#endif
-
-EVP_PKEY *
-engine_load_key(const char *file, SSL_CTX *ctx)
-{
-#if HAVE_OPENSSL_ENGINE
-UI_METHOD *ui;
-EVP_PKEY *pkey;
-
-if (!engine_persist)
-{
-return NULL;
-}
-
-/* this will print out the error from BIO_read */
-crypto_msg(M_INFO, "PEM_read_bio failed, now trying engine method to load 
private key");
-
-ui = UI_create_method("openvpn");
-if (!ui)
-{
-crypto_msg(M_FATAL, "Engine UI creation failed");
-return NULL;
-}
-
-UI_method_set_reader(ui, ui_reader);
-
-ENGINE_init(engine_persist);
-pkey = ENGINE_load_private_key(engine_persist, file, ui, ctx);
-ENGINE_finish(engine_persist);
-if (!pkey)
-{
-crypto_msg(M_FATAL, "Engine could not load key file");
-}
-
-UI_destroy_method(ui);
-return pkey;
-#else  /* if HAVE_OPENSSL_ENGINE */
-return NULL;
-#endif /* if HAVE_OPENSSL_ENGINE */
-}
-
 #if (OPENSSL_VERSION_NUMBER >= 0x1010L) && 
!defined(LIBRESSL_VERSION_NUMBER)
 bool
 ssl_tls1_PRF(const uint8_t *seed, int seed_len, const uint8_t *secret,
diff --git a/src/openvpn/crypto_openssl.h b/src/openvpn/crypto_openssl.h
index 32849fd3f..c5a539345 100644
--- a/src/openvpn/crypto_openssl.h
+++ b/src/openvpn/crypto_openssl.h
@@ -118,16 +118,4 @@ void crypto_print_openssl_errors(const unsigned int flags);
 msg((flags), __VA_ARGS__); \
 } while (false)
 
-/**
- * Load a key file from an engine
- *
- * @param file  The engine file to load
- * @param uiThe UI method for the password prompt
- * @param data  The data to pass to the UI method
- *
- * @return  The private key if successful or NULL if not
- */
-EVP_PKEY *
-engine_load_key(const char *file, SSL_CTX *ctx);
-
 #endif /* CRYPTO_OPENSSL_H_ */
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index b5cc9a7f1..3548cd5d6 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -1057,10 +1057,6 @@ tls_ctx_load_priv_file(struct tls_root_ctx *ctx, const 
char *priv_key

Re: [Openvpn-devel] [PATCH] Log OpenSSL errors on failure to set certificate

[Arne Schwabe]

Am 01.10.23 um 19:49 schrieb selva.n...@gmail.com:

From: Selva Nair 

Currently we log a bogus error message saying private key password verification
failed when SSL_CTX_use_cert_and_key() fails in pkcs11_openssl.c. Instead print
OpenSSL error queue and exit promptly.

Also log OpenSSL errors when SSL_CTX_use_certiifcate() fails in cryptoapi.c
and elsewhere. Such logging could be useful especially when the ceritficate is
rejected by OpenSSL due to stricter security restrictions in recent versions
of the library.


Yeah, looks good.

Acked-By: Arne Schwabe 



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] pkcs11 config changes from 2.5.4 to 2.6.6 ?

[Arne Schwabe]

Am 29.09.23 um 03:25 schrieb Selva Nair:



On Thu, Sep 28, 2023 at 8:55 PM Arne Schwabe <mailto:a...@rfc2549.org>> wrote:



Am 29.09.2023 um 01:08 schrieb mike tancsa:


Hi Selva,

    Thank you for looking!



My guess is that something in the certificate or private key is
not to
OpenSSL 3.1's liking and it rejects it. Is there any way for you
to check the
contents of the token independently using a tool linked against
OpenSSL 3.1 ?


What am I looking for in that case ?  Taking a look at the cert
just with openssl 3.0 on FreeBSD releng14 it seems ok with it.
Same with the Windows version 3.1.x that comes with OpenVPN. Is it
possible it doesnt like the sha1RSA sig ?



OpenSSL 3.0 has security 1 by default (OpenSSL 3.1 has 2 by
default)  and that does not allow SHA1 signatures anymore. See
https://www.openssl.org/docs/man3.1/man3/SSL_CTX_set_security_level.html 
<https://www.openssl.org/docs/man3.1/man3/SSL_CTX_set_security_level.html>


Good point. But, unless the config has "tls-cert-profile foo", we still 
default to legacy and call SSL_CTX_set_security_level(ctx, 1), isn't it? 
Wouldn't that allow SHA1 with 3.1.x ?


For SHA1 you need security 0 aka tls-cert-profile insecure.

But we might update OpenVPN to longer fiddle with the default security 
level in OpenSSL 3+ to avoid downgrading security from 2 to 1 on OpenSSL 3.1


Arne


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] pkcs11 config changes from 2.5.4 to 2.6.6 ?

[Arne Schwabe]

Am 29.09.2023 um 01:08 schrieb mike tancsa:


Hi Selva,

    Thank you for looking!



My guess is that something in the certificate or private key is not to
OpenSSL 3.1's liking and it rejects it. Is there any way for you to 
check the
contents of the token independently using a tool linked against 
OpenSSL 3.1 ?


What am I looking for in that case ?  Taking a look at the cert just 
with openssl 3.0 on FreeBSD releng14 it seems ok with it. Same with 
the Windows version 3.1.x that comes with OpenVPN. Is it possible it 
doesnt like the sha1RSA sig ?



OpenSSL 3.0 has security 1 by default (OpenSSL 3.1 has 2 by default)  
and that does not allow SHA1 signatures anymore. See 
https://www.openssl.org/docs/man3.1/man3/SSL_CTX_set_security_level.html



Arne
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] GHA: new workflow to submit scan to Coverity Scan service

[Arne Schwabe]

Am 11.08.23 um 17:12 schrieb Gert Doering:

Hi,

generally good, but...

On Fri, Jul 28, 2023 at 02:40:05PM +0200, Frank Lichtenheld wrote:

index ..0620f638
--- /dev/null
+++ b/.github/workflows/coverity-scan.yml
@@ -0,0 +1,45 @@
+name: coverity-scan
+on:
+  schedule:
+- cron: '0 20 * * *' # Daily at 20:00 UTC
+  workflow_dispatch:


... not sure this is the best way to approach the run limit, as
we can have many days with no commits at all, so we'd waste credits.

Maybe cache the last commit ID and only run coverity if this has
changed?  Or run it "on commit", but cache the current day, and only
run it again on the next commit that's not on the same day?

(My typical workflow is "you folks queue up patches, and then I find
half a day of free time, and go merge as many as I can make sense of")


You can get that behaviour but it is tricky. You have to resort tricks 
like this: https://github.com/orgs/community/discussions/26519 of saving 
something to a cache and then reading the cache on the next run and 
check if something changed. We can probably implement like that and then 
trigger on push too?


Arne



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v2] show extra info for OpenSSL errors

[Arne Schwabe]

This also shows the extra data from the OpenSSL error function that
can contain extra information. For example, the command

openvpn --providers vollbit

will print out (on macOS):

 OpenSSL: error:12800067:DSO support routines::could not load the shared 
library:filename(/opt/homebrew/Cellar/openssl@3/3.1.1_1/lib/ossl-modules/vollbit.dylib):
 dlopen(/opt/homebrew/Cellar/openssl@3/3.1.1_1/lib/ossl-modules/vollbit.dylib, 
0x0002): tried: 
'/opt/homebrew/Cellar/openssl@3/3.1.1_1/lib/ossl-modules/vollbit.dylib' (no 
such file), 
'/System/Volumes/Preboot/Cryptexes/OS/opt/homebrew/Cellar/openssl@3/3.1.1_1/lib/ossl-modules/vollbit.dylib'
 (no such file), 
'/opt/homebrew/Cellar/openssl@3/3.1.1_1/lib/ossl-modules/vollbit.dylib' (no 
such file)

Patch v2: Format message more like current messages

Change-Id: Ic2ee89937dcd85721bcacd1b700a20c640364f80
Signed-off-by: Arne Schwabe 
---
 src/openvpn/crypto_openssl.c | 21 +++--
 src/openvpn/openssl_compat.h | 12 
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index b043bb95e..22c6d6840 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -238,9 +238,16 @@ void
 crypto_print_openssl_errors(const unsigned int flags)
 {
 unsigned long err = 0;
+int line, errflags;
+const char *file, *data, *func;
 
-while ((err = ERR_get_error()))
+while ((err = ERR_get_error_all(, , , , )) != 
0)
 {
+if (!(errflags & ERR_TXT_STRING))
+{
+data = "";
+}
+
 /* Be more clear about frequently occurring "no shared cipher" error */
 if (ERR_GET_REASON(err) == SSL_R_NO_SHARED_CIPHER)
 {
@@ -258,7 +265,17 @@ crypto_print_openssl_errors(const unsigned int flags)
 "tls-version-min 1.0 to the client configuration to use TLS 
1.0+ "
 "instead of TLS 1.0 only");
 }
-msg(flags, "OpenSSL: %s", ERR_error_string(err, NULL));
+
+/* print file and line if verb >=8 */
+if (!check_debug_level(D_TLS_DEBUG_MED))
+{
+msg(flags, "OpenSSL: %s:%s", ERR_error_string(err, NULL), data);
+}
+else
+{
+msg(flags, "OpenSSL: %s:%s:%s:%d:%s", ERR_error_string(err, NULL),
+data, file, line, func);
+}
 }
 }
 
diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
index ffb64adf6..736ce1bd5 100644
--- a/src/openvpn/openssl_compat.h
+++ b/src/openvpn/openssl_compat.h
@@ -43,6 +43,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /* Functionality missing in 1.1.0 */
 #if OPENSSL_VERSION_NUMBER < 0x10101000L && !defined(ENABLE_CRYPTO_WOLFSSL)
@@ -799,6 +800,17 @@ EVP_MD_free(const EVP_MD *md)
 /* OpenSSL 1.1.1 and lower use only const EVP_MD, nothing to free */
 }
 
+static inline unsigned long
+ERR_get_error_all(const char **file, int *line,
+  const char **func,
+  const char **data, int *flags)
+{
+static const char *empty = "";
+*func = empty;
+long err = ERR_get_error_line_data(file, line, data, flags);
+return err;
+}
+
 #endif /* OPENSSL_VERSION_NUMBER < 0x3000L */
 
 #endif /* OPENSSL_COMPAT_H_ */
-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v2] Fix unaligned access in macOS/Solaris hwaddr

[Arne Schwabe]

The undefined behaviour USAN clang checker found this.

This fix is a bit messy but so are the original structures.

Patch v2: handle the fact we need to beyond the struct ifr
  correctly when mapping the result to struct sockaddr_dl

Change-Id: Ia797c8801fa9a9bc10b6674efde5fdbd7132e4a8
Signed-off-by: Arne Schwabe 
---
 src/openvpn/route.c | 32 +++-
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/src/openvpn/route.c b/src/openvpn/route.c
index 90e981e97..bcf6fb878 100644
--- a/src/openvpn/route.c
+++ b/src/openvpn/route.c
@@ -3641,7 +3641,7 @@ get_default_gateway(struct route_gateway_info *rgi, 
openvpn_net_ctx_t *ctx)
 if (rgi->flags & RGI_IFACE_DEFINED)
 {
 struct ifconf ifc;
-struct ifreq *ifr;
+struct ifreq ifr;
 const int bufsize = 4096;
 char *buffer;
 
@@ -3666,23 +3666,37 @@ get_default_gateway(struct route_gateway_info *rgi, 
openvpn_net_ctx_t *ctx)
 
 for (cp = buffer; cp <= buffer + ifc.ifc_len - sizeof(struct ifreq); )
 {
-ifr = (struct ifreq *)cp;
+/* this is not always using an 8 byte alignment that struct ifr
+ * requires */
+memcpy(, cp, sizeof(struct ifreq));
 #if defined(TARGET_SOLARIS)
-const size_t len = sizeof(ifr->ifr_name) + sizeof(ifr->ifr_addr);
+const size_t len = sizeof(ifr.ifr_name) + sizeof(ifr.ifr_addr);
 #else
-const size_t len = sizeof(ifr->ifr_name) + 
max(sizeof(ifr->ifr_addr), ifr->ifr_addr.sa_len);
+const size_t len = sizeof(ifr.ifr_name) + 
max(sizeof(ifr.ifr_addr), ifr.ifr_addr.sa_len);
 #endif
 
-if (!ifr->ifr_addr.sa_family)
+if (!ifr.ifr_addr.sa_family)
 {
 break;
 }
-if (!strncmp(ifr->ifr_name, rgi->iface, IFNAMSIZ))
+if (!strncmp(ifr.ifr_name, rgi->iface, IFNAMSIZ))
 {
-if (ifr->ifr_addr.sa_family == AF_LINK)
+if (ifr.ifr_addr.sa_family == AF_LINK)
 {
-struct sockaddr_dl *sdl = (struct sockaddr_dl 
*)>ifr_addr;
-memcpy(rgi->hwaddr, LLADDR(sdl), 6);
+/* This is a broken member access. struct sockaddr_dl has
+ * 20 bytes while if_addr has only 16 bytes. So casting 
if_addr
+ * to struct sockaddr_dl gives (legitimate) warnings
+ *
+ * sockaddr_dl has 12 bytes space for the inrerface name 
and
+ * the hw address. So the last 4 that might be part of the
+ * hw address are not in if_addr, so we need
+ *
+ * So we use a memcpy here to avoid the warnings with ASAN
+ * that we are doing a very nasty cast here
+ */
+struct sockaddr_dl sdl = { 0 };
+memcpy(, cp + offsetof(struct ifreq, ifr_addr), 
sizeof(sdl));
+memcpy(rgi->hwaddr, LLADDR(), 6);
 rgi->flags |= RGI_HWADDR_DEFINED;
 }
 }
-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH OpenVPN3] Add 'pull' to ignored options

[Arne Schwabe]

Am 27.07.23 um 14:21 schrieb Merten Fermont:

Hi Arne,

I changed my patch to check the client and client+pull options.
Giving an error when neither options are declared.

This however may break current implementations that depend on 'client'
not being a required option?



Acked-By: Arne Schwabe 

Yes. It might. But then again there are people that run into strange 
problems because these configuration are not really working when 
switching to an OpenVPN 2.x client. We rather have consistency between 
clients here.


Arne



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH OpenVPN3] Add 'pull' to ignored options

[Arne Schwabe]

Am 27.07.23 um 10:52 schrieb Merten Fermont:

Fixes error that --pull is an unknown option in client config.
---
openvpn/client/cliopt.hpp | 1 +
1 file changed, 1 insertion(+)

diff --git a/openvpn/client/cliopt.hpp b/openvpn/client/cliopt.hpp
index f7be44a8..431791f3 100644
--- a/openvpn/client/cliopt.hpp
+++ b/openvpn/client/cliopt.hpp
@@ -797,6 +797,7 @@ class ClientOptions : public RC
 "mute-replay-warnings",
 "nobind", /* only behaviour in v3 client anyway */
 "prng",
+"pull", /* option is implied by 'client' */
 "rcvbuf", /* present in many configs */
 "replay-persist", /* Makes little sense in TLS mode */
 "script-security",



While that will work, it would be better to not ignore that option but 
handle it in a similar way to --client. In general we should actually 
throw an error if neither client or tls-client+pull are present as 
OpenVPN3 cannot operate without these in p2p mode.


Arne



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Porting OpenVPN Problems

[Arne Schwabe]

Am 24.07.23 um 10:43 schrieb Swan Geon:
it still doesn't change where it hangs though interestingly enough if I 
set the address within the command to that of the interface, then the 
interface will respond with `read from TUN/TAP returned 44` followed by


That happens if we get an unexpected status code from the tun/tap 
interface. It is probably worth looking at the tun driver to understand 
when/why it reports that code 44.



`Recursive routing detected, drop tun packet to []AF_INET]1.1.1.1:1194 
` 1.1.1.1 being the address of the interface but 
only once and that's it. 


Usually that happens when you have some kind of routing loop. If you 
give us a full log of the client that might help. If you are just 
starting I would also avoid tunneling everything over the tun interface 
(redirect-gateway) but instead just pick some random network like 
10.77.123.0/24 and only route that to the tun device and get that 
working to avoid the routing loops until that works solid.


Maybe I am confused about how that specific
command works and/or I have something set up wrong somewhere down the 
line. I know that this is a complicated issue with many facets and I am 
new to OpenVPN so I'm not super sure how to debug it currently so please 
please be patient with me and ask questions so that I can do my best to 
answer them.


Sure no problem. For some reason your whole mail is formatted a single 
blob of text. Something seems to have stripped all formatting.


Arne


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] GHA: Add macos-13

[Arne Schwabe]

Am 18.07.23 um 12:52 schrieb Frank Lichtenheld:

Change-Id: Ica2e4b5a6b8da1368b487a33cd4b03ed9fc36011
Signed-off-by: Frank Lichtenheld 
---
  .github/workflows/build.yaml | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml
index 2ae231ee..1b75a1a7 100644
--- a/.github/workflows/build.yaml
+++ b/.github/workflows/build.yaml
@@ -198,7 +198,7 @@ jobs:
matrix:
  ssllib: [ openssl11, openssl3, libressl]
  build: [ normal, asan ]
-os: [macos-11, macos-12]
+os: [macos-11, macos-12, macos-13]
  include:
- build: asan
  cflags: "-fsanitize=address -fno-optimize-sibling-calls 
-fsanitize-address-use-after-scope -fno-omit-frame-pointer -g -O1"



Acked-By: Arne Schwabe 



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] GHA: refactor mingw UTs and add missing tls_crypt

[Arne Schwabe]

Am 18.07.23 um 12:52 schrieb Frank Lichtenheld:

I thought instead about moving this into a loop inside
powershell. But then error handling becomes annoying.
So let's GitHub handle it.



Acked-By: Arne Schwabe 

I personally do not mind the long list of repeated unit tests in the 
file but this way is also fine.


I checked Frank's repository to verify that this actually works.

Arne



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] manage.c: document missing KID parameter

[Arne Schwabe]

Am 14.07.23 um 13:18 schrieb Lev Stipakov:

From: Lev Stipakov 

Commit a261e173 ("Make sending plain text control message session
aware") added KID parameter to "client-pending-auth" management command,
but forgot to mention it in the output of management help.


My fault.

Acked-By: Arne Schwabe 



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v3 3/4] Check if the -wrap argument is actually supported by the platform's ld

[Arne Schwabe]

This avoids build errors on macOS. Also the test_tls_crypt command works
just fine on FreeBSD with its linkers, so do not make that test Linux only.

Patch v2: allow running with old cmake version (cmake 3 on RHEL7 with EPEL
  is only 3.17)
Patch v3: add OPTIONAL keyword to Incldue required by some cmake versions

Change-Id: Id26676bdc576c7d3d6726afa43fe6c7a397c579b
Signed-off-by: Arne Schwabe 
---
 CMakeLists.txt | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 2d0cd5dd0..7dae6655d 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -16,6 +16,7 @@ find_package(PkgConfig REQUIRED)
 include(CheckSymbolExists)
 include(CheckIncludeFiles)
 include(CheckCCompilerFlag)
+include(CheckLinkerFlag OPTIONAL)
 include(CheckTypeSize)
 include(CheckStructHasMember)
 include(CTest)
@@ -564,18 +565,24 @@ if (BUILD_TESTING)
 )
 endif ()
 
-if (NOT MSVC)
-# MSVC does not support --wrap
+# MSVC and Apple's LLVM ld do not support --wrap
+# This test requires cmake >= 3.18, so check if check_linker_flag is
+# available
+if (COMMAND check_linker_flag)
+check_linker_flag(C -Wl,--wrap=parse_line LD_SUPPORTS_WRAP)
+endif()
+
+if (${LD_SUPPORTS_WRAP})
 list(APPEND unit_tests
 "test_argv"
+"test_tls_crypt"
 )
 endif ()
 
-# These tests work on only on Linux since they depend on special linker 
features
+# These tests work on only on Linux since they depend on special Linux 
features
 if (${CMAKE_SYSTEM_NAME} STREQUAL "Linux")
 list(APPEND unit_tests
 "test_networking"
-"test_tls_crypt"
 )
 endif ()
 
-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v2] Mock openvpn_exece on win32 also for test_tls_crypt

[Arne Schwabe]

This function is needed to commpile on win32 as run_command.c defines it
on Unix Linux but on windows it is defined in win32.c which pulls in too
many other unresolvable symbols.

Patch v2: Also add mock_win32_execve.c to automake files

Change-Id: I8c8fe298eb30e211279f3fc010584b9d3bc14b4a
Signed-off-by: Arne Schwabe 
---
 CMakeLists.txt   |  2 ++
 tests/unit_tests/openvpn/Makefile.am |  3 +-
 tests/unit_tests/openvpn/mock_win32_execve.c | 37 
 tests/unit_tests/openvpn/test_pkt.c  |  8 -
 4 files changed, 41 insertions(+), 9 deletions(-)
 create mode 100644 tests/unit_tests/openvpn/mock_win32_execve.c

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 597dc9074..2d0cd5dd0 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -679,6 +679,7 @@ if (BUILD_TESTING)
 )
 
 target_sources(test_pkt PRIVATE
+tests/unit_tests/openvpn/mock_win32_execve.c
 src/openvpn/argv.c
 src/openvpn/base64.c
 src/openvpn/crypto_mbedtls.c
@@ -740,6 +741,7 @@ if (BUILD_TESTING)
 -Wl,--wrap=buffer_write_file
 -Wl,--wrap=rand_bytes)
 target_sources(test_tls_crypt PRIVATE
+tests/unit_tests/openvpn/mock_win32_execve.c
 src/openvpn/argv.c
 src/openvpn/base64.c
 src/openvpn/crypto_mbedtls.c
diff --git a/tests/unit_tests/openvpn/Makefile.am 
b/tests/unit_tests/openvpn/Makefile.am
index dbf658ac7..6b56f8480 100644
--- a/tests/unit_tests/openvpn/Makefile.am
+++ b/tests/unit_tests/openvpn/Makefile.am
@@ -83,7 +83,7 @@ packet_id_testdriver_SOURCES = test_packet_id.c mock_msg.c 
mock_msg.h \
 pkt_testdriver_CFLAGS  = @TEST_CFLAGS@ \
-I$(top_srcdir)/include -I$(top_srcdir)/src/compat 
-I$(top_srcdir)/src/openvpn
 pkt_testdriver_LDFLAGS = @TEST_LDFLAGS@
-pkt_testdriver_SOURCES = test_pkt.c mock_msg.c mock_msg.h \
+pkt_testdriver_SOURCES = test_pkt.c mock_msg.c mock_msg.h mock_win32_execve.c \
$(top_srcdir)/src/openvpn/argv.c \
$(top_srcdir)/src/openvpn/base64.c \
$(top_srcdir)/src/openvpn/buffer.c \
@@ -110,6 +110,7 @@ tls_crypt_testdriver_LDFLAGS = @TEST_LDFLAGS@ \
-Wl,--wrap=parse_line \
-Wl,--wrap=rand_bytes
 tls_crypt_testdriver_SOURCES = test_tls_crypt.c mock_msg.c mock_msg.h \
+mock_win32_execve.c \
$(top_srcdir)/src/openvpn/argv.c \
$(top_srcdir)/src/openvpn/base64.c \
$(top_srcdir)/src/openvpn/buffer.c \
diff --git a/tests/unit_tests/openvpn/mock_win32_execve.c 
b/tests/unit_tests/openvpn/mock_win32_execve.c
new file mode 100644
index 0..4d37ebe33
--- /dev/null
+++ b/tests/unit_tests/openvpn/mock_win32_execve.c
@@ -0,0 +1,37 @@
+/*
+ *  OpenVPN -- An application to securely tunnel IP networks
+ * over a single TCP/UDP port, with support for SSL/TLS-based
+ * session authentication and key exchange,
+ * packet encryption, packet authentication, and
+ * packet compression.
+ *
+ *  Copyright (C) 2023 OpenVPN Inc 
+ *  Copyright (C) 2023 Arne Schwabe 
+ *
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2
+ *  as published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License along
+ *  with this program; if not, write to the Free Software Foundation, Inc.,
+ *  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include "config.h"
+#include "syshead.h"
+
+#include "win32.h"
+
+#ifdef _WIN32
+int
+openvpn_execve(const struct argv *a, const struct env_set *es, const unsigned 
int flags)
+{
+ASSERT(0);
+}
+#endif
diff --git a/tests/unit_tests/openvpn/test_pkt.c 
b/tests/unit_tests/openvpn/test_pkt.c
index 5a53f702e..9f49ee7bd 100644
--- a/tests/unit_tests/openvpn/test_pkt.c
+++ b/tests/unit_tests/openvpn/test_pkt.c
@@ -69,14 +69,6 @@ print_link_socket_actual(const struct link_socket_actual 
*act, struct gc_arena *
 return "dummy print_link_socket_actual from unit test";
 }
 
-#ifdef _WIN32
-int
-openvpn_execve(const struct argv *a, const struct env_set *es, const unsigned 
int flags)
-{
-ASSERT(0);
-}
-#endif
-
 struct test_pkt_context {
 struct tls_auth_standalone tas_tls_auth;
 struct tls_auth_standalone tas_crypt;
-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] Ignore Ipv6 route delete request on Android and set ipv4 verbosity to 7

[Arne Schwabe]

Android has no facility nor need one to delete routes as routes are
automatically cleaned up when the tun interface is closed. Also adjust
the IPv4 message to be only shown and verb 7 and rephrase the message.

Change-Id: If8f920d378c31e9ea773ce1f56f3df50f1ec36cd
Signed-off-by: Arne Schwabe 
---
 src/openvpn/route.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/openvpn/route.c b/src/openvpn/route.c
index 0b369da44..6028de9c2 100644
--- a/src/openvpn/route.c
+++ b/src/openvpn/route.c
@@ -2302,8 +2302,9 @@ delete_route(struct route_ipv4 *r,
 openvpn_execve_check(, es, 0, "ERROR: OpenBSD/NetBSD route delete 
command failed");
 
 #elif defined(TARGET_ANDROID)
-msg(M_NONFATAL, "Sorry, deleting routes on Android is not possible. The 
VpnService API allows routes to be set on connect only.");
-
+msg(D_ROUTE_DEBUG, "Deleting routes on Android is not possible/not "
+   "needed. The VpnService API allows routes to be set "
+   "on connect only and will clean up automatically.");
 #elif defined(TARGET_AIX)
 
 {
@@ -2490,7 +2491,10 @@ delete_route_ipv6(const struct route_ipv6 *r6, const 
struct tuntap *tt,
 network, r6->netbits, gateway);
 argv_msg(D_ROUTE, );
 openvpn_execve_check(, es, 0, "ERROR: AIX route add command failed");
-
+#elif defined(TARGET_ANDROID)
+msg(D_ROUTE_DEBUG, "Deleting routes on Android is not possible/not "
+   "needed. The VpnService API allows routes to be set "
+   "on connect only and will clean up automatically.");
 #else  /* if defined(TARGET_LINUX) */
 msg(M_FATAL, "Sorry, but I don't know how to do 'route ipv6' commands on 
this operating system.  Try putting your routes in a --route-down script");
 #endif /* if defined(TARGET_LINUX) */
-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v3] Introduce get_key_by_management_key_id helper function

[Arne Schwabe]

This function allows us to map from a management key id to a key structure
and also allows this function to be reused.

Patch v2: add message when key is not found.
Patch v3: only consider valid keys

Change-Id: I42d8785959c24bf688190965e58b9b98251b8557
Signed-off-by: Arne Schwabe 
---
 src/openvpn/ssl_common.h | 20 
 src/openvpn/ssl_verify.c | 23 +--
 2 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index 27b029479..be0f18746 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -722,4 +722,24 @@ get_primary_key(const struct tls_multi *multi)
 return >session[TM_ACTIVE].key[KS_PRIMARY];
 }
 
+#ifdef ENABLE_MANAGEMENT
+/**
+ * Gets the \c key_state  object that belong to the management key id or
+ * return NULL if not found.
+ */
+static inline struct key_state *
+get_key_by_management_key_id(struct tls_multi *multi, unsigned int mda_key_id)
+{
+for (int i = 0; i < KEY_SCAN_SIZE; ++i)
+{
+struct key_state *ks = get_key_scan(multi, i);
+if (ks->mda_key_id == mda_key_id && ks->state > S_UNDEF)
+{
+return ks;
+}
+}
+return NULL;
+}
+#endif
+
 #endif /* SSL_COMMON_H_ */
diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index 90416b69e..2395e55c8 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -1266,22 +1266,25 @@ tls_authentication_status(struct tls_multi *multi)
 bool
 tls_authenticate_key(struct tls_multi *multi, const unsigned int mda_key_id, 
const bool auth, const char *client_reason)
 {
-bool ret = false;
+struct key_state *ks = NULL;
 if (multi)
 {
-int i;
+
 auth_set_client_reason(multi, client_reason);
-for (i = 0; i < KEY_SCAN_SIZE; ++i)
+ks = get_key_by_management_key_id(multi, mda_key_id);
+
+if (ks)
 {
-struct key_state *ks = get_key_scan(multi, i);
-if (ks->mda_key_id == mda_key_id)
-{
-ks->mda_status = auth ? ACF_SUCCEEDED : ACF_FAILED;
-ret = true;
-}
+ks->mda_status = auth ? ACF_SUCCEEDED : ACF_FAILED;
 }
+else
+{
+msg(D_TLS_DEBUG_LOW, "%s: no key state found for management key id 
"
+"%d", __func__, mda_key_id);
+}
+
 }
-return ret;
+return (bool) ks;
 }
 #endif /* ifdef ENABLE_MANAGEMENT */
 
-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v3] Introduce get_key_by_management_key_id helper function

[Arne Schwabe]

This function allows us to map from a management key id to a key structure
and also allows this function to be reused.

Patch v2: add message when key is not found.
Patch v3: only consider valid keys

Change-Id: I42d8785959c24bf688190965e58b9b98251b8557
Signed-off-by: Arne Schwabe 
---
 src/openvpn/ssl_common.h | 20 
 src/openvpn/ssl_verify.c | 23 +--
 2 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index 27b029479..be0f18746 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -722,4 +722,24 @@ get_primary_key(const struct tls_multi *multi)
 return >session[TM_ACTIVE].key[KS_PRIMARY];
 }
 
+#ifdef ENABLE_MANAGEMENT
+/**
+ * Gets the \c key_state  object that belong to the management key id or
+ * return NULL if not found.
+ */
+static inline struct key_state *
+get_key_by_management_key_id(struct tls_multi *multi, unsigned int mda_key_id)
+{
+for (int i = 0; i < KEY_SCAN_SIZE; ++i)
+{
+struct key_state *ks = get_key_scan(multi, i);
+if (ks->mda_key_id == mda_key_id && ks->state > S_UNDEF)
+{
+return ks;
+}
+}
+return NULL;
+}
+#endif
+
 #endif /* SSL_COMMON_H_ */
diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index 90416b69e..2395e55c8 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -1266,22 +1266,25 @@ tls_authentication_status(struct tls_multi *multi)
 bool
 tls_authenticate_key(struct tls_multi *multi, const unsigned int mda_key_id, 
const bool auth, const char *client_reason)
 {
-bool ret = false;
+struct key_state *ks = NULL;
 if (multi)
 {
-int i;
+
 auth_set_client_reason(multi, client_reason);
-for (i = 0; i < KEY_SCAN_SIZE; ++i)
+ks = get_key_by_management_key_id(multi, mda_key_id);
+
+if (ks)
 {
-struct key_state *ks = get_key_scan(multi, i);
-if (ks->mda_key_id == mda_key_id)
-{
-ks->mda_status = auth ? ACF_SUCCEEDED : ACF_FAILED;
-ret = true;
-}
+ks->mda_status = auth ? ACF_SUCCEEDED : ACF_FAILED;
 }
+else
+{
+msg(D_TLS_DEBUG_LOW, "%s: no key state found for management key id 
"
+"%d", __func__, mda_key_id);
+}
+
 }
-return ret;
+return (bool) ks;
 }
 #endif /* ifdef ENABLE_MANAGEMENT */
 
-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] show extra info for OpenSSL errors

[Arne Schwabe]

This also shows the extra data from the OpenSSL error function that
can contain extra information. For example, the command

openvpn --providers vollbit

will print out (on macOS):

 OpenSSLerror:12800067:DSO support routines::could not load the shared 
library:filename(/opt/homebrew/Cellar/openssl@3/3.1.1_1/lib/ossl-modules/vollbit.dylib):
 dlopen(/opt/homebrew/Cellar/openssl@3/3.1.1_1/lib/ossl-modules/vollbit.dylib, 
0x0002): tried: 
'/opt/homebrew/Cellar/openssl@3/3.1.1_1/lib/ossl-modules/vollbit.dylib' (no 
such file), 
'/System/Volumes/Preboot/Cryptexes/OS/opt/homebrew/Cellar/openssl@3/3.1.1_1/lib/ossl-modules/vollbit.dylib'
 (no such file), 
'/opt/homebrew/Cellar/openssl@3/3.1.1_1/lib/ossl-modules/vollbit.dylib' (no 
such file)

Change-Id: Ic2ee89937dcd85721bcacd1b700a20c640364f80
Signed-off-by: Arne Schwabe 
---
 src/openvpn/crypto_openssl.c | 20 ++--
 src/openvpn/openssl_compat.h | 12 
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index b043bb95e..d6916fc9b 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -238,9 +238,16 @@ void
 crypto_print_openssl_errors(const unsigned int flags)
 {
 unsigned long err = 0;
+int line, errflags;
+const char *file, *data, *func;
 
-while ((err = ERR_get_error()))
+while ((err = ERR_get_error_all(, , , , )) != 
0)
 {
+if (!(errflags & ERR_TXT_STRING))
+{
+data = "";
+}
+
 /* Be more clear about frequently occurring "no shared cipher" error */
 if (ERR_GET_REASON(err) == SSL_R_NO_SHARED_CIPHER)
 {
@@ -258,7 +265,16 @@ crypto_print_openssl_errors(const unsigned int flags)
 "tls-version-min 1.0 to the client configuration to use TLS 
1.0+ "
 "instead of TLS 1.0 only");
 }
-msg(flags, "OpenSSL: %s", ERR_error_string(err, NULL));
+
+/* print file and line if verb >=8 */
+if (!check_debug_level(D_TLS_DEBUG_MED))
+{
+msg(flags, "OpenSSL%s:%s", ERR_error_string(err, NULL), data);
+}
+else
+{
+msg(flags, "OpenSSL (%s:%d %s): %s:%s", file, line, func, 
ERR_error_string(err, NULL), data);
+}
 }
 }
 
diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
index ffb64adf6..736ce1bd5 100644
--- a/src/openvpn/openssl_compat.h
+++ b/src/openvpn/openssl_compat.h
@@ -43,6 +43,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /* Functionality missing in 1.1.0 */
 #if OPENSSL_VERSION_NUMBER < 0x10101000L && !defined(ENABLE_CRYPTO_WOLFSSL)
@@ -799,6 +800,17 @@ EVP_MD_free(const EVP_MD *md)
 /* OpenSSL 1.1.1 and lower use only const EVP_MD, nothing to free */
 }
 
+static inline unsigned long
+ERR_get_error_all(const char **file, int *line,
+  const char **func,
+  const char **data, int *flags)
+{
+static const char *empty = "";
+*func = empty;
+long err = ERR_get_error_line_data(file, line, data, flags);
+return err;
+}
+
 #endif /* OPENSSL_VERSION_NUMBER < 0x3000L */
 
 #endif /* OPENSSL_COMPAT_H_ */
-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] work around false positive warning with mingw 12

[Arne Schwabe]

Am 06.07.23 um 19:19 schrieb Heiko Hund:

When cross compiling for Windows with Ubuntu 23.04 mingw complains about

   route.c:344:26: warning: ‘special.S_un.S_addr’ may be used uninitialized

which is wrong technically. However the workaround isn't really
intrusive and while there are other warnings caused by libtool, the
cmake mingw build completes with -Werror now.

Change-Id: I8a0f59707570722eab41af2db76980ced04e6d54
Signed-off-by: Heiko Hund 
---
  src/openvpn/route.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/openvpn/route.c b/src/openvpn/route.c
index 0d04a5a3..0b369da4 100644
--- a/src/openvpn/route.c
+++ b/src/openvpn/route.c
@@ -319,7 +319,7 @@ init_route(struct route_ipv4 *r,
  const in_addr_t default_netmask = IPV4_NETMASK_HOST;
  bool status;
  int ret;
-struct in_addr special;
+struct in_addr special = {0};
  
  CLEAR(*r);

  r->option = ro;



Good enough.

Acked-By: Arne Schwabe 


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] GHA: Add work-around for rst2*.py not being directly executable on Windows

[Arne Schwabe]

Am 06.07.23 um 12:21 schrieb Frank Lichtenheld:

On Thu, Jul 06, 2023 at 12:04:07PM +0200, Frank Lichtenheld wrote:

We write a small .bat file wrapper and then force CMake
to use that.

Note that we need to specify the path with / instead of \
as path separator because otherwise run-cmake action will
mangle it.

Change-Id: I05d3f7f3f7f7418d1977e523c6dcfb6fa5feb604
Signed-off-by: Frank Lichtenheld 
---
  .github/workflows/build.yaml | 21 -
  1 file changed, 20 insertions(+), 1 deletion(-)

This is an alternate patch to "CMake: Support doc builds on
Windows machines that do not have .py file association" since
there was some doubt on IRC whether we want to have the complexity
inside of CMake or should delegate that to the caller.

Let me know your opinions.


Personally I prefer the original solution inside CMake.
It just solves the problem in a wide range of potential setups
without additional effort required by the caller. I think the
slightly increased complexity can be accepted for that.


I concur for the same reasons. This is also harder for people to 
replicate on their own setups. They will have to manually write scripts 
instead of jsut having cmake dealing with the problem.


Arne



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] CMake: Support doc builds on Windows machines that do not have .py file association

[Arne Schwabe]

Am 04.07.23 um 16:09 schrieb Frank Lichtenheld:

On Windows we might need to call python because .py files are not
directly executable. This is true e.g. for GHA runners.
For now we assume that rst2html and rst2man can be handled in the same
way and do not test both of them.

Commit e8881ec6dd63bd80ce05202573eac54ab8657fcb unconditionally
used $PYTHON, but that broke build on systems where the default
python can't be used and we need to respect the shebang.
Commit 5dbec1c019d14880ae7bf364b062d3589c7fd9e7 unconditionally
did not use $PYTHON, but that broke build on the aformentioned
GHA runners.
This commit tries to establish a solution that works for both
systems.



This is annoying and should be necessary but I also don't have no better 
idea how to handle this better.


Acked-BY: Arne Schwabe 


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] Mock openvpn_exece on win32 also for test_tls_crypt

[Arne Schwabe]

This function is needed to commpile on win32 as run_command.c defines it
on Unix Linux but on windows it is defined in win32.c which pulls in too
many other unresolvable symbols.

Change-Id: I8c8fe298eb30e211279f3fc010584b9d3bc14b4a
Signed-off-by: Arne Schwabe 
---
 CMakeLists.txt   |  2 ++
 tests/unit_tests/openvpn/mock_win32_execve.c | 37 
 tests/unit_tests/openvpn/test_pkt.c  |  8 -
 3 files changed, 39 insertions(+), 8 deletions(-)
 create mode 100644 tests/unit_tests/openvpn/mock_win32_execve.c

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 597dc9074..2d0cd5dd0 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -679,6 +679,7 @@ if (BUILD_TESTING)
 )
 
 target_sources(test_pkt PRIVATE
+tests/unit_tests/openvpn/mock_win32_execve.c
 src/openvpn/argv.c
 src/openvpn/base64.c
 src/openvpn/crypto_mbedtls.c
@@ -740,6 +741,7 @@ if (BUILD_TESTING)
 -Wl,--wrap=buffer_write_file
 -Wl,--wrap=rand_bytes)
 target_sources(test_tls_crypt PRIVATE
+tests/unit_tests/openvpn/mock_win32_execve.c
 src/openvpn/argv.c
 src/openvpn/base64.c
 src/openvpn/crypto_mbedtls.c
diff --git a/tests/unit_tests/openvpn/mock_win32_execve.c 
b/tests/unit_tests/openvpn/mock_win32_execve.c
new file mode 100644
index 0..4d37ebe33
--- /dev/null
+++ b/tests/unit_tests/openvpn/mock_win32_execve.c
@@ -0,0 +1,37 @@
+/*
+ *  OpenVPN -- An application to securely tunnel IP networks
+ * over a single TCP/UDP port, with support for SSL/TLS-based
+ * session authentication and key exchange,
+ * packet encryption, packet authentication, and
+ * packet compression.
+ *
+ *  Copyright (C) 2023 OpenVPN Inc 
+ *  Copyright (C) 2023 Arne Schwabe 
+ *
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2
+ *  as published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License along
+ *  with this program; if not, write to the Free Software Foundation, Inc.,
+ *  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include "config.h"
+#include "syshead.h"
+
+#include "win32.h"
+
+#ifdef _WIN32
+int
+openvpn_execve(const struct argv *a, const struct env_set *es, const unsigned 
int flags)
+{
+ASSERT(0);
+}
+#endif
diff --git a/tests/unit_tests/openvpn/test_pkt.c 
b/tests/unit_tests/openvpn/test_pkt.c
index 5a53f702e..9f49ee7bd 100644
--- a/tests/unit_tests/openvpn/test_pkt.c
+++ b/tests/unit_tests/openvpn/test_pkt.c
@@ -69,14 +69,6 @@ print_link_socket_actual(const struct link_socket_actual 
*act, struct gc_arena *
 return "dummy print_link_socket_actual from unit test";
 }
 
-#ifdef _WIN32
-int
-openvpn_execve(const struct argv *a, const struct env_set *es, const unsigned 
int flags)
-{
-ASSERT(0);
-}
-#endif
-
 struct test_pkt_context {
 struct tls_auth_standalone tas_tls_auth;
 struct tls_auth_standalone tas_crypt;
-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] OpenVPN Linking Exception - current status report update July

[Arne Schwabe]

Am 15.02.23 um 13:31 schrieb David Sommerseth:


OpenVPN 2.x is licensed under the GNU Public License v2.0 (GPL-2.0). 
This license has served us well in the past and we are not trying to 
change that.  However, changes in licenses of our dependencies put us in 
an unfortunate situation.


Since the last update, we resolved the license approval for a number of 
contributors and are now down to just



Note that the actions for the individual source are my opinion/suggestions.

- Jason A. Donenfeld 

  - Patches to remove possible contribution on mailing list

- James Bottomley 

No response to my license email, however was active this year on 
the mailing list.


* Add unit tests for engine keys
* engine key support

This commits are only relevant for OpenSSL 1.x and we can probably
remove support for this feature again if James does not positively.

- Jérémie Courrèges-Anglas 

   - raised the question if we can keep the OpenSSL license exception
 forever to keep LibreSSL support. I think that is reasonable to just
 keep the outdated exception even if it is just for a handful of
 people using OpenVPN with LibreSSL. When we drafted the mails we
 proposed to remove this because we overlooked LibreSSL.

* Print time_t as long long and suseconds_t as long
probably not trivial.
* Rest of the commits look very small and can probably classified 
as trivial


- Andris Kalnozols 

   Address bounces, could not find any alternative method of contacting
   this person. three commits:

 * extract_x509_extension(): hide status message during normal 
operation.

 * Fix some typos in the man page.

trivial

* Do not upcase x509-username-field for mixed-case arguments.
   non-trivial needs more investigation.


- Mathieu GIANNECCHINI 

   Mail address bounces, could not find an alternative contact.
  --tls-export-cert option only

   One commit having the following feature:

   "--tls-export-cert [directory] : Get peer cert in PEM format and 
store it \n"
   "  in an openvpn temporary file in [directory]. Peer 
cert is \n"
   "  stored before tls-verify script execution and 
deleted after.\n"


We can remove the feature, it seems a corner case. If someone needs
it can still be reimplemented.





Arne


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] CMake: Throw a clear error when config.h in top-level source directory

[Arne Schwabe]

Am 03.07.23 um 18:39 schrieb Frank Lichtenheld:

This causes weird, difficult to debug compilation errors. Usually
caused by trying to run CMake against a source that had an
in-tree autoconfig build.

Note that you're able to do out-of-tree autoconfig builds and
they can obviously mix with out-of-tree cmake builds.



Acked-By: Arne Schwabe 



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v2 3/4] Check if the -wrap argument is actually supported by the platform's ld

[Arne Schwabe]

This avoids build errors on macOS. Also the test_tls_crypt command works
just fine on FreeBSD with its linkers, so do not make that test Linux only.

Patch v2: allow running with old cmake version (cmake 3 on RHEL7 with EPEL
  is only 3.17)

Change-Id: Id26676bdc576c7d3d6726afa43fe6c7a397c579b
Signed-off-by: Arne Schwabe 
---
 CMakeLists.txt | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index acebbb73c..a982c478a 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -16,6 +16,7 @@ find_package(PkgConfig REQUIRED)
 include(CheckSymbolExists)
 include(CheckIncludeFiles)
 include(CheckCCompilerFlag)
+include(CheckLinkerFlag)
 include(CheckTypeSize)
 include(CheckStructHasMember)
 include(CTest)
@@ -560,18 +561,24 @@ if (BUILD_TESTING)
 )
 endif ()
 
-if (NOT MSVC)
-# MSVC does not support --wrap
+# MSVC and Apple's LLVM ld do not support --wrap
+# This test requires cmake >= 1.17, so check if check_linker_flag is
+# available
+if (COMMAND check_linker_flag)
+check_linker_flag(C -Wl,--wrap=parse_line LD_SUPPORTS_WRAP)
+endif()
+
+if (${LD_SUPPORTS_WRAP})
 list(APPEND unit_tests
 "test_argv"
+"test_tls_crypt"
 )
 endif ()
 
-# These tests work on only on Linux since they depend on special linker 
features
+# These tests work on only on Linux since they depend on special Linux 
features
 if (${CMAKE_SYSTEM_NAME} STREQUAL "Linux")
 list(APPEND unit_tests
 "test_networking"
-"test_tls_crypt"
 )
 endif ()
 
-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v2 4/4] Avoid unused function warning/error on FreeBSD (and potientially others)

[Arne Schwabe]

the funktion is_on_link is not used on FreeBSD and triggers a
warning/error (-Werror) on FreeBSD.

Patch v2: use actual platforms instead an ifndef FreeBSD

Change-Id: I6757d6509ff3ff522d6de417372a21e73ccca3ba
Signed-off-by: Arne Schwabe 
---
 src/openvpn/route.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/openvpn/route.c b/src/openvpn/route.c
index d18acd016..0d04a5a33 100644
--- a/src/openvpn/route.c
+++ b/src/openvpn/route.c
@@ -1541,13 +1541,15 @@ local_route(in_addr_t network,
 return LR_NOMATCH;
 }
 
-/* Return true if the "on-link" form of the route should be used.  This is 
when the gateway for a
+/* Return true if the "on-link" form of the route should be used.  This is 
when the gateway for
  * a route is specified as an interface rather than an address. */
+#if defined(TARGET_LINUX) || defined(_WIN32) || defined(TARGET_DARWIN)
 static inline bool
 is_on_link(const int is_local_route, const unsigned int flags, const struct 
route_gateway_info *rgi)
 {
 return rgi && (is_local_route == LR_MATCH || ((flags & ROUTE_REF_GW) && 
(rgi->flags & RGI_ON_LINK)));
 }
+#endif
 
 bool
 add_route(struct route_ipv4 *r,
-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 3/3] Add warning for the --show-groups command that some groups are missing

[Arne Schwabe]

OpenSSL has a weird way of only reporting EC curves that are implemented
in a certain way in the list of all EC cruves. Note this fact and point
out that also the very important curves X448 and X25519 are affected.

Change-Id: I86641bf60d62a50e9b2719e809d2429d65c00097
Signed-off-by: Arne Schwabe 
---
 src/openvpn/ssl_openssl.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 59bbdfc0a..442ae1871 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -2355,8 +2355,10 @@ show_available_tls_ciphers_list(const char *cipher_list,
 void
 show_available_curves(void)
 {
-printf("Consider using openssl 'ecparam -list_curves' as\n"
-   "alternative to running this command.\n");
+printf("Consider using 'openssl ecparam -list_curves' as alternative to 
running\n"
+   "this command.\n"
+   "Note this output does only list curves/group that OpenSSL 
considers as\n"
+   "builtin EC curves. It does not list additional curves nor X448 or 
X25519\n");
 #ifndef OPENSSL_NO_EC
 EC_builtin_curve *curves = NULL;
 size_t crv_len = 0;
-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 2/3] Print SSL peer signature information in handshake debug details

[Arne Schwabe]

This is more SSL debug information that most people do not really need
or care about. OpenSSL's own s_client also logs them:

Peer signing digest: SHA256
Peer signature type: ECDSA

The complete message looks like this:

   Control Channel: TLSv1.3, cipher TLSv1.3 TLS_AES_256_GCM_SHA384, peer 
certificate: 2048 bits RSA, signature: RSA-SHA256, server temp key: 253 bits 
X25519, peer signing digest/type: SHA256 RSASSA-PSS

or when forcing a specific group via tls-groups X448 with a ECDSA server:

   Control Channel: TLSv1.3, cipher TLSv1.3 TLS_AES_256_GCM_SHA384, peer 
certificate: 384 bits ECsecp384r1, signature: ecdsa-with-SHA256, server temp 
key: 448 bits X448, peer signing digest/type: SHA384 ECDSA

Change-Id: Ib5fc0c4b8f164596681ac5ad73002068ec6de1e5
Signed-off-by: Arne Schwabe 
---
 src/openvpn/ssl_openssl.c | 80 ++-
 1 file changed, 78 insertions(+), 2 deletions(-)

diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index da1417b82..59bbdfc0a 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -2167,6 +2167,80 @@ print_server_tempkey(SSL *ssl, char *buf, size_t buflen)
 EVP_PKEY_free(pkey);
 }
 
+#ifndef LIBRESSL_VERSION_NUMBER
+/**
+ * Translate an OpenVPN NID into a more human readable name
+ * @param nid
+ * @return
+ */
+static const char *
+get_sigtype(int nid)
+{
+/* Fix a few OpenSSL names to be better understandable */
+switch (nid)
+{
+case EVP_PKEY_RSA:
+/* will otherwise say rsaEncryption */
+return "RSA";
+
+case EVP_PKEY_DSA:
+/* dsaEncryption otherwise */
+return "DSA";
+
+case EVP_PKEY_EC:
+/* will say id-ecPublicKey */
+return "ECDSA";
+
+case -1:
+return "(error getting name)";
+
+default:
+return OBJ_nid2sn(nid);
+}
+}
+#endif /* ifndef LIBRESSL_VERSION_NUMBER */
+
+/**
+ * Get the type of the signature that is used by the peer during the
+ * TLS handshake
+ */
+static void
+print_peer_signature(SSL *ssl, char *buf, size_t buflen)
+{
+int peer_sig_nid = NID_undef, peer_sig_type_nid = NID_undef;
+const char *peer_sig = "";
+const char *peer_sig_type = "";
+
+/* Even though these methods use the deprecated NIDs instead of using
+ * string as new OpenSSL APIs do, there seem to be no API that replaces
+ * it yet */
+if (SSL_get_peer_signature_nid(ssl, _sig_nid)
+&& peer_sig_nid != NID_undef)
+{
+peer_sig = OBJ_nid2sn(peer_sig_nid);
+}
+
+#ifndef LIBRESSL_VERSION_NUMBER
+/* LibreSSL 3.7.x and 3.8.0 weirdly implment this function but fail on
+ * linking with an unresolved symbol */
+if (SSL_get_peer_signature_type_nid(ssl, _sig_type_nid)
+&& peer_sig_type_nid != NID_undef)
+{
+peer_sig_type = get_sigtype(peer_sig_type_nid);
+}
+#endif
+
+if (peer_sig_nid == NID_undef && peer_sig_type_nid == NID_undef)
+{
+return;
+}
+
+openvpn_snprintf(buf, buflen, ", peer signing digest/type: %s %s",
+ peer_sig, peer_sig_type);
+}
+
+
+
 /* **
  *
  * Information functions
@@ -2181,8 +2255,9 @@ print_details(struct key_state_ssl *ks_ssl, const char 
*prefix)
 char s1[256];
 char s2[256];
 char s3[256];
+char s4[256];
 
-s1[0] = s2[0] = s3[0] = 0;
+s1[0] = s2[0] = s3[0] = s4[0] = 0;
 ciph = SSL_get_current_cipher(ks_ssl->ssl);
 openvpn_snprintf(s1, sizeof(s1), "%s %s, cipher %s %s",
  prefix,
@@ -2197,8 +2272,9 @@ print_details(struct key_state_ssl *ks_ssl, const char 
*prefix)
 X509_free(cert);
 }
 print_server_tempkey(ks_ssl->ssl, s3, sizeof(s3));
+print_peer_signature(ks_ssl->ssl, s4, sizeof(s4));
 
-msg(D_HANDSHAKE, "%s%s%s", s1, s2, s3);
+msg(D_HANDSHAKE, "%s%s%s%s", s1, s2, s3, s4);
 }
 
 void
-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 1/3] Print server temp key details

[Arne Schwabe]

Change-Id: Iaf12bb51a2aac7bcf19070f0b56fa3b1a5863bc3
Signed-off-by: Arne Schwabe 
---
 src/openvpn/ssl_openssl.c | 56 ++-
 1 file changed, 44 insertions(+), 12 deletions(-)

diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 0b310de31..da1417b82 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -2050,18 +2050,11 @@ key_state_read_plaintext(struct key_state_ssl *ks_ssl, 
struct buffer *buf)
 return ret;
 }
 
-/**
- * Print human readable information about the certifcate into buf
- * @param cert  the certificate being used
- * @param buf   output buffer
- * @param buflenoutput buffer length
- */
 static void
-print_cert_details(X509 *cert, char *buf, size_t buflen)
+print_pkey_details(EVP_PKEY *pkey, char *buf, size_t buflen)
 {
 const char *curve = "";
 const char *type = "(error getting type)";
-EVP_PKEY *pkey = X509_get_pubkey(cert);
 
 if (pkey == NULL)
 {
@@ -2124,6 +2117,23 @@ print_cert_details(X509 *cert, char *buf, size_t buflen)
 #endif /* if OPENSSL_VERSION_NUMBER < 0x3000L */
 }
 
+openvpn_snprintf(buf, buflen, "%d bits %s%s",
+ EVP_PKEY_bits(pkey), type, curve);
+}
+
+/**
+ * Print human readable information about the certifcate into buf
+ * @param cert  the certificate being used
+ * @param buf   output buffer
+ * @param buflenoutput buffer length
+ */
+static void
+print_cert_details(X509 *cert, char *buf, size_t buflen)
+{
+EVP_PKEY *pkey = X509_get_pubkey(cert);
+char pkeybuf[128] = { 0 };
+print_pkey_details(pkey, pkeybuf, sizeof(pkeybuf));
+
 char sig[128] = { 0 };
 int signature_nid = X509_get_signature_nid(cert);
 if (signature_nid != 0)
@@ -2132,8 +2142,27 @@ print_cert_details(X509 *cert, char *buf, size_t buflen)
  OBJ_nid2sn(signature_nid));
 }
 
-openvpn_snprintf(buf, buflen, ", peer certificate: %d bit %s%s%s",
- EVP_PKEY_bits(pkey), type, curve, sig);
+openvpn_snprintf(buf, buflen, ", peer certificate: %s%s",
+ pkeybuf, sig);
+
+EVP_PKEY_free(pkey);
+}
+
+static void
+print_server_tempkey(SSL *ssl, char *buf, size_t buflen)
+{
+EVP_PKEY *pkey = NULL;
+SSL_get_peer_tmp_key(ssl, );
+if (!pkey)
+{
+return;
+}
+
+char pkeybuf[128] = { 0 };
+print_pkey_details(pkey, pkeybuf, sizeof(pkeybuf));
+
+openvpn_snprintf(buf, buflen, ", server temp key: %s",
+ pkeybuf);
 
 EVP_PKEY_free(pkey);
 }
@@ -2151,8 +2180,9 @@ print_details(struct key_state_ssl *ks_ssl, const char 
*prefix)
 const SSL_CIPHER *ciph;
 char s1[256];
 char s2[256];
+char s3[256];
 
-s1[0] = s2[0] = 0;
+s1[0] = s2[0] = s3[0] = 0;
 ciph = SSL_get_current_cipher(ks_ssl->ssl);
 openvpn_snprintf(s1, sizeof(s1), "%s %s, cipher %s %s",
  prefix,
@@ -2166,7 +2196,9 @@ print_details(struct key_state_ssl *ks_ssl, const char 
*prefix)
 print_cert_details(cert, s2, sizeof(s2));
 X509_free(cert);
 }
-msg(D_HANDSHAKE, "%s%s", s1, s2);
+print_server_tempkey(ks_ssl->ssl, s3, sizeof(s3));
+
+msg(D_HANDSHAKE, "%s%s%s", s1, s2, s3);
 }
 
 void
-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] test_tls_crypt: Improve mock() usage to be more portable

[Arne Schwabe]

Am 30.06.23 um 15:39 schrieb Frank Lichtenheld:

Use the casting variants of mock(). Using the mock_ptr_type
fixes an existing bug where test_tls_crypt.c couldn't
build in MinGW 32bit:


Acked-By: Arne Schwabe 



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 2/2] Implement using --peer-fingerprint without CA certificates

[Arne Schwabe]

Am 30.06.23 um 15:31 schrieb Maximilian Fillinger:

The grammar in the 3rd sentence in the comment below is messed up. (I think I 
understand it, but I'm not sure.)


+if (session->opt->verify_hash_no_ca)
+{
+/*
+ * If we decide to verify the peer certificate based on the fingerprint
+ * we ignore wrong dates and the certificate not being trusted.
+ * Any other problem with the certificate (wrong key, bad cert,...)
+ * will still trigger an error.
+ * Clearing these flags relies on verify_cert will later rejecting a
+ * certificate that has no matching fingerprint.
+ */
+uint32_t flags_ignore = MBEDTLS_X509_BADCERT_NOT_TRUSTED
+| MBEDTLS_X509_BADCERT_EXPIRED
+| MBEDTLS_X509_BADCERT_FUTURE;
+*flags = *flags & ~flags_ignore;
+}
+


Also, this comment is copied verbatim from Jason's commit 423ced962d which has 
been reverted. I'm not a lawyer, but since comments are relatively easy to 
rephrase, I think it's better to do that. My suggestion:


The comment is already mine. Jason never included an mBed TLS 
implementation. I attributed the commit to Jason but some of the code 
and this comment is already written by me.


Arne


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 2/4] [CMake] Only add -Wno-stringop-truncation on supported compilers

[Arne Schwabe]

The -Wno-stringop-truncation flag is only supported by some GCC versions
and not by Clang (macOS, FreeBSD) at all.

Move the includes to the top the file to have them available when running
the check_c_compiler_flag.

Change-Id: I452bc4ee935d13f8e9095d0a31805a3bbaff0cec
Signed-off-by: Arne Schwabe 
---
 CMakeLists.txt | 22 ++
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 3cbba5a38..acebbb73c 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -12,6 +12,14 @@ project(openvpn)
 # and OpenSSL having version 1.1.1+ and generally does not offer the same
 # configurability like autoconf
 
+find_package(PkgConfig REQUIRED)
+include(CheckSymbolExists)
+include(CheckIncludeFiles)
+include(CheckCCompilerFlag)
+include(CheckTypeSize)
+include(CheckStructHasMember)
+include(CTest)
+
 option(UNSUPPORTED_BUILDS "Allow unsupported builds" OFF)
 
 if (NOT WIN32 AND NOT ${UNSUPPORTED_BUILDS})
@@ -70,7 +78,12 @@ else ()
 set(CMAKE_CXX_FLAGS_RELEASE "-O2")
 set(CMAKE_C_FLAGS_DEBUG "-g -O1")
 set(CMAKE_CXX_FLAGS_DEBUG "-g -O1")
-add_compile_options(-Wall -Wuninitialized -Wno-stringop-truncation)
+add_compile_options(-Wall -Wuninitialized)
+check_c_compiler_flag(-Wno-stringop-truncation NoStringOpTruncation)
+
+if (${NoStringOpTruncation})
+add_compile_options(-Wno-stringop-truncation)
+endif()
 # We are not ready for this
 #add_compile_options(-Wconversion -Wno-sign-conversion -Wsign-compare)
 if (USE_WERROR)
@@ -78,13 +91,6 @@ else ()
 endif ()
 endif ()
 
-find_package(PkgConfig REQUIRED)
-include(CheckSymbolExists)
-include(CheckIncludeFiles)
-include(CheckTypeSize)
-include(CheckStructHasMember)
-include(CTest)
-
 find_program(PYTHON NAMES python3 python)
 execute_process(
 COMMAND ${PYTHON} 
${CMAKE_CURRENT_SOURCE_DIR}/contrib/cmake/parse-version.m4.py 
${CMAKE_CURRENT_SOURCE_DIR}/version.m4
-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 1/4] Do not blindly assume python3 is also the interpreter that runs rst2html

[Arne Schwabe]

On my system python3 is the macOS system python3 while rst2html has

   #!/opt/homebrew/opt/python@3.9/bin/python3.9

as its first line. Running that with a different python results in missing
python modules. So directly execute the rst2html script instead.

Change-Id: I7e27ae031179c91cc1bca8122caf2453d6396ec0
Signed-off-by: Arne Schwabe 
---
 doc/CMakeLists.txt | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/doc/CMakeLists.txt b/doc/CMakeLists.txt
index d38805513..2fba80bbd 100644
--- a/doc/CMakeLists.txt
+++ b/doc/CMakeLists.txt
@@ -50,13 +50,13 @@ if (_GENERATE_HTML_DOC)
 list(APPEND ALL_DOCS openvpn.8.html openvpn-examples.5.html)
 add_custom_command(
 OUTPUT openvpn.8.html
-COMMAND ${PYTHON} ${RST2HTML} ${RST_FLAGS} 
${CMAKE_CURRENT_SOURCE_DIR}/openvpn.8.rst 
${CMAKE_CURRENT_BINARY_DIR}/openvpn.8.html
+COMMAND ${RST2HTML} ${RST_FLAGS} 
${CMAKE_CURRENT_SOURCE_DIR}/openvpn.8.rst 
${CMAKE_CURRENT_BINARY_DIR}/openvpn.8.html
 MAIN_DEPENDENCY openvpn.8.rst
 DEPENDS ${OPENVPN_SECTIONS}
 )
 add_custom_command(
 OUTPUT openvpn-examples.5.html
-COMMAND ${PYTHON} ${RST2HTML} ${RST_FLAGS} 
${CMAKE_CURRENT_SOURCE_DIR}/openvpn-examples.5.rst 
${CMAKE_CURRENT_BINARY_DIR}/openvpn-examples.5.html
+COMMAND ${RST2HTML} ${RST_FLAGS} 
${CMAKE_CURRENT_SOURCE_DIR}/openvpn-examples.5.rst 
${CMAKE_CURRENT_BINARY_DIR}/openvpn-examples.5.html
 MAIN_DEPENDENCY openvpn-examples.5.rst
 DEPENDS ${OPENVPN_EXAMPLES_SECTIONS}
 )
@@ -65,13 +65,13 @@ if (_GENERATE_MAN_DOC)
 list(APPEND ALL_DOCS openvpn.8 openvpn-examples.5)
 add_custom_command(
 OUTPUT openvpn.8
-COMMAND ${PYTHON} ${RST2MAN} ${RST_FLAGS} 
${CMAKE_CURRENT_SOURCE_DIR}/openvpn.8.rst ${CMAKE_CURRENT_BINARY_DIR}/openvpn.8
+COMMAND ${RST2MAN} ${RST_FLAGS} 
${CMAKE_CURRENT_SOURCE_DIR}/openvpn.8.rst ${CMAKE_CURRENT_BINARY_DIR}/openvpn.8
 MAIN_DEPENDENCY openvpn.8.rst
 DEPENDS ${OPENVPN_SECTIONS}
 )
 add_custom_command(
 OUTPUT openvpn-examples.5
-COMMAND ${PYTHON} ${RST2MAN} ${RST_FLAGS} 
${CMAKE_CURRENT_SOURCE_DIR}/openvpn-examples.5.rst 
${CMAKE_CURRENT_BINARY_DIR}/openvpn-examples.5
+COMMAND ${RST2MAN} ${RST_FLAGS} 
${CMAKE_CURRENT_SOURCE_DIR}/openvpn-examples.5.rst 
${CMAKE_CURRENT_BINARY_DIR}/openvpn-examples.5
 MAIN_DEPENDENCY openvpn-examples.5.rst
 DEPENDS ${OPENVPN_EXAMPLES_SECTIONS}
 )
-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 4/4] Avoid unused function warning/error on FreeBSD

[Arne Schwabe]

the funktion is_on_link is not used on FreeBSD and triggers a
warning/error (-Werror) on FreeBSD.

Change-Id: I6757d6509ff3ff522d6de417372a21e73ccca3ba
Signed-off-by: Arne Schwabe 
---
 src/openvpn/route.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/openvpn/route.c b/src/openvpn/route.c
index d18acd016..2180b7d1a 100644
--- a/src/openvpn/route.c
+++ b/src/openvpn/route.c
@@ -1541,13 +1541,15 @@ local_route(in_addr_t network,
 return LR_NOMATCH;
 }
 
-/* Return true if the "on-link" form of the route should be used.  This is 
when the gateway for a
+/* Return true if the "on-link" form of the route should be used.  This is 
when the gateway for
  * a route is specified as an interface rather than an address. */
+#ifndef TARGET_FREEBSD
 static inline bool
 is_on_link(const int is_local_route, const unsigned int flags, const struct 
route_gateway_info *rgi)
 {
 return rgi && (is_local_route == LR_MATCH || ((flags & ROUTE_REF_GW) && 
(rgi->flags & RGI_ON_LINK)));
 }
+#endif
 
 bool
 add_route(struct route_ipv4 *r,
-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 3/4] Check if the -wrap argument is actually supported by the platform's ld

[Arne Schwabe]

This avoids build errors on macOS. Also the test_tls_crypt command works
just fine on FreeBSD with its linkers, so do not make that test Linux only.

Change-Id: Id26676bdc576c7d3d6726afa43fe6c7a397c579b
Signed-off-by: Arne Schwabe 
---
 CMakeLists.txt | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index acebbb73c..d2445b414 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -16,6 +16,7 @@ find_package(PkgConfig REQUIRED)
 include(CheckSymbolExists)
 include(CheckIncludeFiles)
 include(CheckCCompilerFlag)
+include(CheckLinkerFlag)
 include(CheckTypeSize)
 include(CheckStructHasMember)
 include(CTest)
@@ -560,18 +561,20 @@ if (BUILD_TESTING)
 )
 endif ()
 
-if (NOT MSVC)
-# MSVC does not support --wrap
+# MSVC and Apple's LLVM ld do not support --wrap
+check_linker_flag(C -Wl,--wrap=parse_line LD_SUPPORTS_WRAP)
+
+if (${LD_SUPPORTS_WRAP})
 list(APPEND unit_tests
 "test_argv"
+"test_tls_crypt"
 )
 endif ()
 
-# These tests work on only on Linux since they depend on special linker 
features
+# These tests work on only on Linux since they depend on special Linux 
features
 if (${CMAKE_SYSTEM_NAME} STREQUAL "Linux")
 list(APPEND unit_tests
 "test_networking"
-"test_tls_crypt"
 )
 endif ()
 
-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 0/4] Restore ability to compile on macOS/FreeBSD with Cmake

[Arne Schwabe]

The patches to the cmake files did a lot of improvements but broke compiling
on macOS and FreeBSD. This patch set restores the ability to compile again
with these two platforms.

Arne Schwabe (4):
  Do not blindly assume python3 is also the interpreter that runs
rst2html
  [CMake] Only add -Wno-stringop-truncation on supported compilers
  Check if the -wrap argument is actually supported by the platform's ld
  Avoid unused function warning/error on FreeBSD

 CMakeLists.txt  | 33 +
 doc/CMakeLists.txt  |  8 
 src/openvpn/route.c |  4 +++-
 3 files changed, 28 insertions(+), 17 deletions(-)

-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 2/2] Implement using --peer-fingerprint without CA certificates

[Arne Schwabe]

This is implements --peer-fingerprint command to support OpenVPN authentication
without involving a PKI.

The current implementation in OpenVPN for peer fingerprint has been already
extensively rewritten from the original submission from Jason [1]. The commit
preserved the original author since it was based on Jason code/idea.

The current code uses two commits to prepare the --peer-fingerprint solution
as which choose to use a simple to use --peer-fingerprint directive instead
of using using a --tls-verify script like the v1 of the patch proposed.
The two commit preparing this are:

 - Extend verify-hash to allow multiple hashes
 - Implement peer-fingerprint to check fingerprint of peer certificate

This perparing patches make this actual patch quite short. There are some
lines in this patch that bear some similarity to the ones like

if (!preverify_ok && !session->opt->verify_hash_no_ca)

vs

if (!preverify_ok && !session->opt->ca_file_none)

But these similarities are one line fragments and dictated by the
surrounding style and program flow, so even a complete black box
implementation will likely end up with the same lines.

[1] 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16781.html

Change-Id: Ie74c3d606c5429455c293c367462244566a936e3
Signed-off-by: Arne Schwabe 
---
 src/openvpn/init.c   |  1 +
 src/openvpn/options.c| 26 +-
 src/openvpn/options.h|  1 +
 src/openvpn/ssl_common.h |  1 +
 src/openvpn/ssl_verify_mbedtls.c | 16 
 src/openvpn/ssl_verify_openssl.c |  2 +-
 6 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index c023b33c6..d358ad003 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -3347,6 +3347,7 @@ do_init_crypto_tls(struct context *c, const unsigned int 
flags)
 to.verify_hash = options->verify_hash;
 to.verify_hash_algo = options->verify_hash_algo;
 to.verify_hash_depth = options->verify_hash_depth;
+to.verify_hash_no_ca = options->verify_hash_no_ca;
 #ifdef ENABLE_X509ALTUSERNAME
 memcpy(to.x509_username_field, options->x509_username_field, 
sizeof(to.x509_username_field));
 #else
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index fe9285384..e4c596b89 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -2991,21 +2991,11 @@ options_postprocess_verify_ce(const struct options 
*options,
 else
 {
 #ifdef ENABLE_CRYPTO_MBEDTLS
-if (!(options->ca_file))
-{
-msg(M_USAGE, "You must define CA file (--ca)");
-}
-
 if (options->ca_path)
 {
 msg(M_USAGE, "Parameter --capath cannot be used with the mbed 
TLS version version of OpenVPN.");
 }
-#else  /* ifdef ENABLE_CRYPTO_MBEDTLS */
-if ((!(options->ca_file)) && (!(options->ca_path)))
-{
-msg(M_USAGE, "You must define CA file (--ca) or CA path 
(--capath)");
-}
-#endif
+#endif  /* ifdef ENABLE_CRYPTO_MBEDTLS */
 if (pull)
 {
 
@@ -3737,6 +3727,13 @@ options_postprocess_mutate(struct options *o, struct 
env_set *es)
 options_postprocess_http_proxy_override(o);
 }
 #endif
+if (!o->ca_file && !o->ca_path && o->verify_hash
+&& o->verify_hash_depth == 0)
+{
+msg(M_INFO, "Using certificate fingerprint to verify peer (no CA "
+"option set). ");
+o->verify_hash_no_ca = true;
+}
 
 if (o->config && streq(o->config, "stdin") && o->remap_sigusr1 == SIGHUP)
 {
@@ -4032,8 +4029,11 @@ options_postprocess_filechecks(struct options *options)
 errs |= check_file_access_inline(options->dh_file_inline, CHKACC_FILE,
  options->dh_file, R_OK, "--dh");
 
-errs |= check_file_access_inline(options->ca_file_inline, CHKACC_FILE,
- options->ca_file, R_OK, "--ca");
+if (!options->verify_hash_no_ca)
+{
+errs |= check_file_access_inline(options->ca_file_inline, CHKACC_FILE,
+ options->ca_file, R_OK, "--ca");
+}
 
 errs |= check_file_access_chroot(options->chroot_dir, CHKACC_FILE,
  options->ca_path, R_OK, "--capath");
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index 95f1158a4..f5890b90f 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -604,6 +604,7 @@ struct options
 struct verify_hash_list *verify_hash;
 hash_algo_type verify_hash_algo;
 int verify_hash_depth;
+bool verify_hash_no_ca;
 unsigned int ssl_flags; /* set

Re: [Openvpn-devel] [PATCH] [CMake] Only add -Wno-stringop-truncation on supported compilers

[Arne Schwabe]

Am 29.06.23 um 13:39 schrieb Arne Schwabe:

The -Wno-stringop-truncation flag is only supported by some GCC versions
and not by Clang (macOS, FreeBSD) at all.

Change-Id: I452bc4ee935d13f8e9095d0a31805a3bbaff0cec



Ingore this version.



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] [CMake] Only add -Wno-stringop-truncation on supported compilers

[Arne Schwabe]

The -Wno-stringop-truncation flag is only supported by some GCC versions
and not by Clang (macOS, FreeBSD) at all.

Change-Id: I452bc4ee935d13f8e9095d0a31805a3bbaff0cec
Signed-off-by: Arne Schwabe 
---
 CMakeLists.txt | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 3cbba5a38..ec0915bb0 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -12,6 +12,14 @@ project(openvpn)
 # and OpenSSL having version 1.1.1+ and generally does not offer the same
 # configurability like autoconf
 
+find_package(PkgConfig REQUIRED)
+include(CheckSymbolExists)
+include(CheckIncludeFiles)
+include(CheckCCompilerFlag)
+include(CheckTypeSize)
+include(CheckStructHasMember)
+include(CTest)
+
 option(UNSUPPORTED_BUILDS "Allow unsupported builds" OFF)
 
 if (NOT WIN32 AND NOT ${UNSUPPORTED_BUILDS})
@@ -70,7 +78,10 @@ else ()
 set(CMAKE_CXX_FLAGS_RELEASE "-O2")
 set(CMAKE_C_FLAGS_DEBUG "-g -O1")
 set(CMAKE_CXX_FLAGS_DEBUG "-g -O1")
-add_compile_options(-Wall -Wuninitialized -Wno-stringop-truncation)
+check_c_compiler_flag(-Wno-stringop-truncation NoStringOpTruncation)
+if (${NoStringOpTruncation})
+add_compile_options(-Wall -Wuninitialized )
+endif()
 # We are not ready for this
 #add_compile_options(-Wconversion -Wno-sign-conversion -Wsign-compare)
 if (USE_WERROR)
@@ -78,13 +89,6 @@ else ()
 endif ()
 endif ()
 
-find_package(PkgConfig REQUIRED)
-include(CheckSymbolExists)
-include(CheckIncludeFiles)
-include(CheckTypeSize)
-include(CheckStructHasMember)
-include(CTest)
-
 find_program(PYTHON NAMES python3 python)
 execute_process(
 COMMAND ${PYTHON} 
${CMAKE_CURRENT_SOURCE_DIR}/contrib/cmake/parse-version.m4.py 
${CMAKE_CURRENT_SOURCE_DIR}/version.m4
-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] Remove key_type argument from generate_key_random

[Arne Schwabe]

This part of the function is not used by any part of
our source code. It looks also broken if called with kt!=NULL
The function cipher_kt_key_size expects its argument to be not
NULL and would break. So remove the unused code instead of fixing
it.

Found by Coverity.

Change-Id: Id56628cfb3dfd2f306bd9bdcca2e567ac0ca9ab2
Signed-off-by: Arne Schwabe 
---
 src/openvpn/crypto.c | 38 +++---
 src/openvpn/crypto.h |  2 --
 2 files changed, 11 insertions(+), 29 deletions(-)

diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index b5ae17ec8..930f15a42 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -957,41 +957,25 @@ check_replay_consistency(const struct key_type *kt, bool 
packet_id)
 }
 
 /*
- * Generate a random key.  If key_type is provided, make
- * sure generated key is valid for key_type.
+ * Generate a random key.
  */
-void
-generate_key_random(struct key *key, const struct key_type *kt)
+static void
+generate_key_random(struct key *key)
 {
 int cipher_len = MAX_CIPHER_KEY_LENGTH;
 int hmac_len = MAX_HMAC_KEY_LENGTH;
 
 struct gc_arena gc = gc_new();
 
-do
+CLEAR(*key);
+if (!rand_bytes(key->cipher, cipher_len)
+|| !rand_bytes(key->hmac, hmac_len))
 {
-CLEAR(*key);
-if (kt)
-{
-cipher_len = cipher_kt_key_size(kt->cipher);
-
-int kt_hmac_length = md_kt_size(kt->digest);
-
-if (kt->digest && kt_hmac_length > 0 && kt_hmac_length <= hmac_len)
-{
-hmac_len = kt_hmac_length;
-}
-}
-if (!rand_bytes(key->cipher, cipher_len)
-|| !rand_bytes(key->hmac, hmac_len))
-{
-msg(M_FATAL, "ERROR: Random number generator cannot obtain entropy 
for key generation");
-}
-
-dmsg(D_SHOW_KEY_SOURCE, "Cipher source entropy: %s", 
format_hex(key->cipher, cipher_len, 0, ));
-dmsg(D_SHOW_KEY_SOURCE, "HMAC source entropy: %s", 
format_hex(key->hmac, hmac_len, 0, ));
+msg(M_FATAL, "ERROR: Random number generator cannot obtain entropy for 
key generation");
+}
 
-} while (kt && !check_key(key, kt));
+dmsg(D_SHOW_KEY_SOURCE, "Cipher source entropy: %s", 
format_hex(key->cipher, cipher_len, 0, ));
+dmsg(D_SHOW_KEY_SOURCE, "HMAC source entropy: %s", format_hex(key->hmac, 
hmac_len, 0, ));
 
 gc_free();
 }
@@ -1398,7 +1382,7 @@ write_key_file(const int nkeys, const char *filename)
 char *fmt;
 
 /* generate random bits */
-generate_key_random(, NULL);
+generate_key_random();
 
 /* format key as ascii */
 fmt = format_hex_ex((const uint8_t *),
diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h
index 229a4eb1c..88f8f4472 100644
--- a/src/openvpn/crypto.h
+++ b/src/openvpn/crypto.h
@@ -304,8 +304,6 @@ void read_key_file(struct key2 *key2, const char *file, 
const unsigned int flags
  */
 int write_key_file(const int nkeys, const char *filename);
 
-void generate_key_random(struct key *key, const struct key_type *kt);
-
 void check_replay_consistency(const struct key_type *kt, bool packet_id);
 
 bool check_key(struct key *key, const struct key_type *kt);
-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] Fix use-after-free with EVP_CIPHER_free

[Arne Schwabe]

In many scenerios the context will still have a reference to the cipher, so
this use-after-free does not explode but it is still wrong.

Change-Id: I59002d6613eaef36d5a47b20b56073e399cfa1df
Signed-off-by: Arne Schwabe 
---
 src/openvpn/crypto_openssl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index c2ac80b74..8fe56fc78 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -839,11 +839,12 @@ cipher_ctx_init(EVP_CIPHER_CTX *ctx, const uint8_t *key,
 crypto_msg(M_FATAL, "EVP cipher init #2");
 }
 
-EVP_CIPHER_free(kt);
 /* make sure we used a big enough key */
 ASSERT(EVP_CIPHER_CTX_key_length(ctx) <= EVP_CIPHER_key_length(kt));
+EVP_CIPHER_free(kt);
 }
 
+
 int
 cipher_ctx_iv_length(const EVP_CIPHER_CTX *ctx)
 {
-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Persist-key: enable persist-key option by default.

[Arne Schwabe]

Am 09.05.2023 um 17:46 schrieb Gianmarco De Gregori:

-bool persist_key;   /* Don't re-read key files on SIGUSR1 or 
PING_RESTART */


The downside of always enabling this option is that you can no longer 
replace the certificate and key without restarting the server completley.


Arne



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 2/2] Implement using --peer-fingerprint without CA certificates

[Arne Schwabe]

This is implements --peer-fingerprint command to support OpenVPN authentication
without involving a PKI.

The current implementation in OpenVPN for peer fingerprint has been already
extensively rewritten from the original submission from Jason [1]. The commit
preserved the original author since it was based on Jason code/idea.

The current code uses two commits to prepare the --peer-fingerprint solution
as which choose to use a simple to use --peer-fingerprint directive instead
of using using a --tls-verify script like the v1 of the patch proposed.
The two commit preparing this are:

 - Extend verify-hash to allow multiple hashes
 - Implement peer-fingerprint to check fingerprint of peer certificate

This perparing patches make this actual patch quite short. There are some
lines in this patch that bear some similarity to the ones like

if (!preverify_ok && !session->opt->verify_hash_no_ca)

vs

if (!preverify_ok && !session->opt->ca_file_none)

But these similarities are one line fragments and dictated by the
surrounding style and program flow, so even a complete black box
implementation will likely end up with the same lines.

[1] 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16781.html

Change-Id: Ie74c3d606c5429455c293c367462244566a936e3
Signed-off-by: Arne Schwabe 
---
 src/openvpn/init.c   |  1 +
 src/openvpn/options.c| 26 +-
 src/openvpn/options.h|  1 +
 src/openvpn/ssl_common.h |  1 +
 src/openvpn/ssl_verify_mbedtls.c | 16 
 src/openvpn/ssl_verify_openssl.c |  2 +-
 6 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index c023b33c6..d358ad003 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -3347,6 +3347,7 @@ do_init_crypto_tls(struct context *c, const unsigned int 
flags)
 to.verify_hash = options->verify_hash;
 to.verify_hash_algo = options->verify_hash_algo;
 to.verify_hash_depth = options->verify_hash_depth;
+to.verify_hash_no_ca = options->verify_hash_no_ca;
 #ifdef ENABLE_X509ALTUSERNAME
 memcpy(to.x509_username_field, options->x509_username_field, 
sizeof(to.x509_username_field));
 #else
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index fe9285384..e4c596b89 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -2991,21 +2991,11 @@ options_postprocess_verify_ce(const struct options 
*options,
 else
 {
 #ifdef ENABLE_CRYPTO_MBEDTLS
-if (!(options->ca_file))
-{
-msg(M_USAGE, "You must define CA file (--ca)");
-}
-
 if (options->ca_path)
 {
 msg(M_USAGE, "Parameter --capath cannot be used with the mbed 
TLS version version of OpenVPN.");
 }
-#else  /* ifdef ENABLE_CRYPTO_MBEDTLS */
-if ((!(options->ca_file)) && (!(options->ca_path)))
-{
-msg(M_USAGE, "You must define CA file (--ca) or CA path 
(--capath)");
-}
-#endif
+#endif  /* ifdef ENABLE_CRYPTO_MBEDTLS */
 if (pull)
 {
 
@@ -3737,6 +3727,13 @@ options_postprocess_mutate(struct options *o, struct 
env_set *es)
 options_postprocess_http_proxy_override(o);
 }
 #endif
+if (!o->ca_file && !o->ca_path && o->verify_hash
+&& o->verify_hash_depth == 0)
+{
+msg(M_INFO, "Using certificate fingerprint to verify peer (no CA "
+"option set). ");
+o->verify_hash_no_ca = true;
+}
 
 if (o->config && streq(o->config, "stdin") && o->remap_sigusr1 == SIGHUP)
 {
@@ -4032,8 +4029,11 @@ options_postprocess_filechecks(struct options *options)
 errs |= check_file_access_inline(options->dh_file_inline, CHKACC_FILE,
  options->dh_file, R_OK, "--dh");
 
-errs |= check_file_access_inline(options->ca_file_inline, CHKACC_FILE,
- options->ca_file, R_OK, "--ca");
+if (!options->verify_hash_no_ca)
+{
+errs |= check_file_access_inline(options->ca_file_inline, CHKACC_FILE,
+ options->ca_file, R_OK, "--ca");
+}
 
 errs |= check_file_access_chroot(options->chroot_dir, CHKACC_FILE,
  options->ca_path, R_OK, "--capath");
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index 95f1158a4..f5890b90f 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -604,6 +604,7 @@ struct options
 struct verify_hash_list *verify_hash;
 hash_algo_type verify_hash_algo;
 int verify_hash_depth;
+bool verify_hash_no_ca;
 unsigned int ssl_flags; /* set

[Openvpn-devel] [PATCH 1/2] Revert commit 423ced962d

[Arne Schwabe]

This reverts commit 423ced962db3129b4ed551c489624faba4340652, which
has Jason A. Donenfeld listed as author as the patch was based on his
initial submission.

We have not received permission to relicense the original patch.

Change-Id: I8142753928498169032450c56d0497a5042bdc9b
Signed-off-by: Arne Schwabe 
---
 src/openvpn/init.c   |  1 -
 src/openvpn/options.c| 26 +-
 src/openvpn/options.h|  1 -
 src/openvpn/ssl_common.h |  1 -
 src/openvpn/ssl_verify_mbedtls.c | 16 
 src/openvpn/ssl_verify_openssl.c |  2 +-
 6 files changed, 14 insertions(+), 33 deletions(-)

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index d358ad003..c023b33c6 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -3347,7 +3347,6 @@ do_init_crypto_tls(struct context *c, const unsigned int 
flags)
 to.verify_hash = options->verify_hash;
 to.verify_hash_algo = options->verify_hash_algo;
 to.verify_hash_depth = options->verify_hash_depth;
-to.verify_hash_no_ca = options->verify_hash_no_ca;
 #ifdef ENABLE_X509ALTUSERNAME
 memcpy(to.x509_username_field, options->x509_username_field, 
sizeof(to.x509_username_field));
 #else
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index e4c596b89..fe9285384 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -2991,11 +2991,21 @@ options_postprocess_verify_ce(const struct options 
*options,
 else
 {
 #ifdef ENABLE_CRYPTO_MBEDTLS
+if (!(options->ca_file))
+{
+msg(M_USAGE, "You must define CA file (--ca)");
+}
+
 if (options->ca_path)
 {
 msg(M_USAGE, "Parameter --capath cannot be used with the mbed 
TLS version version of OpenVPN.");
 }
-#endif  /* ifdef ENABLE_CRYPTO_MBEDTLS */
+#else  /* ifdef ENABLE_CRYPTO_MBEDTLS */
+if ((!(options->ca_file)) && (!(options->ca_path)))
+{
+msg(M_USAGE, "You must define CA file (--ca) or CA path 
(--capath)");
+}
+#endif
 if (pull)
 {
 
@@ -3727,13 +3737,6 @@ options_postprocess_mutate(struct options *o, struct 
env_set *es)
 options_postprocess_http_proxy_override(o);
 }
 #endif
-if (!o->ca_file && !o->ca_path && o->verify_hash
-&& o->verify_hash_depth == 0)
-{
-msg(M_INFO, "Using certificate fingerprint to verify peer (no CA "
-"option set). ");
-o->verify_hash_no_ca = true;
-}
 
 if (o->config && streq(o->config, "stdin") && o->remap_sigusr1 == SIGHUP)
 {
@@ -4029,11 +4032,8 @@ options_postprocess_filechecks(struct options *options)
 errs |= check_file_access_inline(options->dh_file_inline, CHKACC_FILE,
  options->dh_file, R_OK, "--dh");
 
-if (!options->verify_hash_no_ca)
-{
-errs |= check_file_access_inline(options->ca_file_inline, CHKACC_FILE,
- options->ca_file, R_OK, "--ca");
-}
+errs |= check_file_access_inline(options->ca_file_inline, CHKACC_FILE,
+ options->ca_file, R_OK, "--ca");
 
 errs |= check_file_access_chroot(options->chroot_dir, CHKACC_FILE,
  options->ca_path, R_OK, "--capath");
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index f5890b90f..95f1158a4 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -604,7 +604,6 @@ struct options
 struct verify_hash_list *verify_hash;
 hash_algo_type verify_hash_algo;
 int verify_hash_depth;
-bool verify_hash_no_ca;
 unsigned int ssl_flags; /* set to SSLF_x flags from ssl.h */
 
 #ifdef ENABLE_PKCS11
diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index 27b029479..c0b3caa71 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -345,7 +345,6 @@ struct tls_options
 const char *remote_cert_eku;
 struct verify_hash_list *verify_hash;
 int verify_hash_depth;
-bool verify_hash_no_ca;
 hash_algo_type verify_hash_algo;
 #ifdef ENABLE_X509ALTUSERNAME
 char *x509_username_field[MAX_PARMS];
diff --git a/src/openvpn/ssl_verify_mbedtls.c b/src/openvpn/ssl_verify_mbedtls.c
index e3437f740..c9ef7a171 100644
--- a/src/openvpn/ssl_verify_mbedtls.c
+++ b/src/openvpn/ssl_verify_mbedtls.c
@@ -62,22 +62,6 @@ verify_callback(void *session_obj, mbedtls_x509_crt *cert, 
int cert_depth,
 struct buffer cert_fingerprint = x509_get_sha256_fingerprint(cert, );
 cert_hash_remember(session, cert_depth, _fingerprint);
 
-if (session->opt->verify_hash_no_ca)
-{
-/*
- * If we decide to verify the

[Openvpn-devel] [PATCH v2 1/2] Introduce get_key_by_management_key_id helper function

[Arne Schwabe]

This function allows us to map from a management key id to a key structure
and also allows this function to be reused.

Patch v2: add message when key is not found.

Signed-off-by: Arne Schwabe 
---
 src/openvpn/ssl_common.h | 20 
 src/openvpn/ssl_verify.c | 23 +--
 2 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index 27b029479..ebfd25432 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -722,4 +722,24 @@ get_primary_key(const struct tls_multi *multi)
 return >session[TM_ACTIVE].key[KS_PRIMARY];
 }
 
+#ifdef ENABLE_MANAGEMENT
+/**
+ * Gets the \c key_state  object that belong to the management key id or
+ * return NULL if not found.
+ */
+static inline struct key_state *
+get_key_by_management_key_id(struct tls_multi *multi, unsigned int mda_key_id)
+{
+for (int i = 0; i < KEY_SCAN_SIZE; ++i)
+{
+struct key_state *ks = get_key_scan(multi, i);
+if (ks->mda_key_id == mda_key_id)
+{
+return ks;
+}
+}
+return NULL;
+}
+#endif
+
 #endif /* SSL_COMMON_H_ */
diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index 1b589f1a6..d4d291098 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -1268,22 +1268,25 @@ tls_authentication_status(struct tls_multi *multi)
 bool
 tls_authenticate_key(struct tls_multi *multi, const unsigned int mda_key_id, 
const bool auth, const char *client_reason)
 {
-bool ret = false;
+struct key_state *ks = NULL;
 if (multi)
 {
-int i;
+
 auth_set_client_reason(multi, client_reason);
-for (i = 0; i < KEY_SCAN_SIZE; ++i)
+ks = get_key_by_management_key_id(multi, mda_key_id);
+
+if (ks)
 {
-struct key_state *ks = get_key_scan(multi, i);
-if (ks->mda_key_id == mda_key_id)
-{
-ks->mda_status = auth ? ACF_SUCCEEDED : ACF_FAILED;
-ret = true;
-}
+ks->mda_status = auth ? ACF_SUCCEEDED : ACF_FAILED;
 }
+else
+{
+msg(D_TLS_DEBUG_LOW, "%s: no key state found for management key id 
"
+"%d", __func__, mda_key_id);
+}
+
 }
-return ret;
+return (bool) ks;
 }
 #endif /* ifdef ENABLE_MANAGEMENT */
 
-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 2/2] Fix CR_RESPONSE mangaement message using wrong key_id

[Arne Schwabe]

Am 19.05.23 um 15:45 schrieb Selva Nair:

Hi,

While this bugfix should be merged, I'm a conflicted about the way these 
two patches are split up. It just makes reviewing harder than it should 
be. They actually form two independent changes but with one half 
intersecting with the other for no reason.


Yeah. I moved the logging to 1/2. It seems to fit much better in that patch.

Arne



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v2 2/2] Fix CR_RESPONSE mangaement message using wrong key_id

[Arne Schwabe]

the management interface expects the management key id instead
of the openvpn key id. In the past they often were the same for low ids
which hid the bug quite well.

Also do not pick uninitialised keystates (management key_id is not valid
in these).

Patch v2: do not add logging

Change-Id: If9fa1165a0e886b570b3738546ed810a32367cbe
Signed-off-by: Arne Schwabe 
---
 src/openvpn/push.c   | 4 ++--
 src/openvpn/ssl_common.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index 8e9627199..8f0a534ac 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -267,9 +267,9 @@ receive_cr_response(struct context *c, const struct buffer 
*buffer)
 struct tls_session *session = >c2.tls_multi->session[TM_ACTIVE];
 struct man_def_auth_context *mda = session->opt->mda_context;
 struct env_set *es = session->opt->es;
-int key_id = get_primary_key(c->c2.tls_multi)->key_id;
+unsigned int mda_key_id = get_primary_key(c->c2.tls_multi)->mda_key_id;
 
-management_notify_client_cr_response(key_id, mda, es, m);
+management_notify_client_cr_response(mda_key_id, mda, es, m);
 #endif
 #if ENABLE_PLUGIN
 verify_crresponse_plugin(c->c2.tls_multi, m);
diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index ebfd25432..be0f18746 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -733,7 +733,7 @@ get_key_by_management_key_id(struct tls_multi *multi, 
unsigned int mda_key_id)
 for (int i = 0; i < KEY_SCAN_SIZE; ++i)
 {
 struct key_state *ks = get_key_scan(multi, i);
-if (ks->mda_key_id == mda_key_id)
+if (ks->mda_key_id == mda_key_id && ks->state > S_UNDEF)
 {
 return ks;
 }
-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] Print a more user-friendly error when tls-crypt-v2 client auth fails

[Arne Schwabe]

While it might be clear to people being (too?) well versed in
typical crypto applications that an authentication failure probably
mean wrong decryption key, this is not really obvious for the typical
user/server admin.

Change-Id: If0f0e7d53f915d39ab69c43dc73bb9c26ae9
Signed-off-by: Arne Schwabe 
---
 src/openvpn/tls_crypt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
index 88b2d6d7c..73542368e 100644
--- a/src/openvpn/tls_crypt.c
+++ b/src/openvpn/tls_crypt.c
@@ -524,6 +524,8 @@ tls_crypt_v2_unwrap_client_key(struct key2 *client_key, 
struct buffer *metadata,
 dmsg(D_CRYPTO_DEBUG, "tag_check: %s",
  format_hex(tag_check, sizeof(tag_check), 0, ));
 CRYPT_ERROR("client key authentication error");
+msg(D_TLS_DEBUG_LOW, "This might be a client-key that was generated 
for "
+"a different tls-crypt-v2 server key)");
 }
 
 if (buf_len() < sizeof(client_key->keys))
-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 1/2] Remove contribution from Jason A. Donenfeld

[Arne Schwabe]

This reverts commit 423ced962db3129b4ed551c489624faba4340652, which
has Jason A. Donenfeld as author.

Jason has expressed that he does not want to be bothered with the
license change of OpenVPN and unfortunately that leaves us no alternative
other than to remove his contribution from OpenVPN in order to be able to
go forward with the license change.

Change-Id: I8142753928498169032450c56d0497a5042bdc9b
Signed-off-by: Arne Schwabe 
---
 src/openvpn/init.c   |  1 -
 src/openvpn/options.c| 26 +-
 src/openvpn/options.h|  1 -
 src/openvpn/ssl_common.h |  1 -
 src/openvpn/ssl_verify_mbedtls.c | 16 
 src/openvpn/ssl_verify_openssl.c |  2 +-
 6 files changed, 14 insertions(+), 33 deletions(-)

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index d358ad003..c023b33c6 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -3347,7 +3347,6 @@ do_init_crypto_tls(struct context *c, const unsigned int 
flags)
 to.verify_hash = options->verify_hash;
 to.verify_hash_algo = options->verify_hash_algo;
 to.verify_hash_depth = options->verify_hash_depth;
-to.verify_hash_no_ca = options->verify_hash_no_ca;
 #ifdef ENABLE_X509ALTUSERNAME
 memcpy(to.x509_username_field, options->x509_username_field, 
sizeof(to.x509_username_field));
 #else
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index e4c596b89..fe9285384 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -2991,11 +2991,21 @@ options_postprocess_verify_ce(const struct options 
*options,
 else
 {
 #ifdef ENABLE_CRYPTO_MBEDTLS
+if (!(options->ca_file))
+{
+msg(M_USAGE, "You must define CA file (--ca)");
+}
+
 if (options->ca_path)
 {
 msg(M_USAGE, "Parameter --capath cannot be used with the mbed 
TLS version version of OpenVPN.");
 }
-#endif  /* ifdef ENABLE_CRYPTO_MBEDTLS */
+#else  /* ifdef ENABLE_CRYPTO_MBEDTLS */
+if ((!(options->ca_file)) && (!(options->ca_path)))
+{
+msg(M_USAGE, "You must define CA file (--ca) or CA path 
(--capath)");
+}
+#endif
 if (pull)
 {
 
@@ -3727,13 +3737,6 @@ options_postprocess_mutate(struct options *o, struct 
env_set *es)
 options_postprocess_http_proxy_override(o);
 }
 #endif
-if (!o->ca_file && !o->ca_path && o->verify_hash
-&& o->verify_hash_depth == 0)
-{
-msg(M_INFO, "Using certificate fingerprint to verify peer (no CA "
-"option set). ");
-o->verify_hash_no_ca = true;
-}
 
 if (o->config && streq(o->config, "stdin") && o->remap_sigusr1 == SIGHUP)
 {
@@ -4029,11 +4032,8 @@ options_postprocess_filechecks(struct options *options)
 errs |= check_file_access_inline(options->dh_file_inline, CHKACC_FILE,
  options->dh_file, R_OK, "--dh");
 
-if (!options->verify_hash_no_ca)
-{
-errs |= check_file_access_inline(options->ca_file_inline, CHKACC_FILE,
- options->ca_file, R_OK, "--ca");
-}
+errs |= check_file_access_inline(options->ca_file_inline, CHKACC_FILE,
+ options->ca_file, R_OK, "--ca");
 
 errs |= check_file_access_chroot(options->chroot_dir, CHKACC_FILE,
  options->ca_path, R_OK, "--capath");
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index f5890b90f..95f1158a4 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -604,7 +604,6 @@ struct options
 struct verify_hash_list *verify_hash;
 hash_algo_type verify_hash_algo;
 int verify_hash_depth;
-bool verify_hash_no_ca;
 unsigned int ssl_flags; /* set to SSLF_x flags from ssl.h */
 
 #ifdef ENABLE_PKCS11
diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index 27b029479..c0b3caa71 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -345,7 +345,6 @@ struct tls_options
 const char *remote_cert_eku;
 struct verify_hash_list *verify_hash;
 int verify_hash_depth;
-bool verify_hash_no_ca;
 hash_algo_type verify_hash_algo;
 #ifdef ENABLE_X509ALTUSERNAME
 char *x509_username_field[MAX_PARMS];
diff --git a/src/openvpn/ssl_verify_mbedtls.c b/src/openvpn/ssl_verify_mbedtls.c
index e3437f740..c9ef7a171 100644
--- a/src/openvpn/ssl_verify_mbedtls.c
+++ b/src/openvpn/ssl_verify_mbedtls.c
@@ -62,22 +62,6 @@ verify_callback(void *session_obj, mbedtls_x509_crt *cert, 
int cert_depth,
 struct buffer cert_fingerprint = x509_get_sha256_fingerprint(cert, );
 cert_hash_remember(session, cert_depth, _fin

[Openvpn-devel] [PATCH 2/2] Implement using --peer-fingerprint without CA certificates

[Arne Schwabe]

This is implements --peer-fingerprint command to support OpenVPN authentication
without involving a PKI.

The current implementation in OpenVPN for peer fingerprint has been already
extensively rewritten from the original submission from Jason. The commit
preserved the original author since it was based on Jason code/idea.

The code uses two commits to prepare the --peer-fingerprint solution as
which choose to use a simple to use --peer-fingerprint directive instead
of using using a --tls-verify script like the v1 of the patch proposed.
The two commit preparing this are:

 - Extend verify-hash to allow multiple hashes
 - Implement peer-fingerprint to check fingerprint of peer certificate

This perparing patches make this actual patch quite short. There are some
lines in this patch that bear some similarity to the ones like

if (!preverify_ok && !session->opt->verify_hash_no_ca)

vs

if (!preverify_ok && !session->opt->ca_file_none)

But these similarities are one line fragments and dictated by the
surrounding style and program flow, so even a complete black box
implementation will likely end up with the same lines.

Patch V2: Changes in V2 (by Arne Schwabe):
  - Only check peer certificates, not all cert levels, if you need
multiple levels of certificate you should use a real CA
  - Use peer-fingerprint instead tls-verify on server side in example.
  - rename variable ca_file_none to verify_hash_no_ca
  - do no require --ca none but allow --ca simply
to be absent when --peer-fingprint is present
  - adjust warnings/errors messages to also point to
peer-fingerprint as valid verification method.
  - Fix mbed TLS version of not requiring CA
not working

Patch v3: Fix minor style. Remove unessary check of verify_hash_no_ca in
ssl.c.

Patch v4: remove the last parts of Jason's original patch.

Change-Id: Ie74c3d606c5429455c293c367462244566a936e3
Signed-off-by: Arne Schwabe 
---
 src/openvpn/init.c   |  1 +
 src/openvpn/options.c| 26 +-
 src/openvpn/options.h|  1 +
 src/openvpn/ssl_common.h |  1 +
 src/openvpn/ssl_verify_mbedtls.c | 16 
 src/openvpn/ssl_verify_openssl.c |  2 +-
 6 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index c023b33c6..d358ad003 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -3347,6 +3347,7 @@ do_init_crypto_tls(struct context *c, const unsigned int 
flags)
 to.verify_hash = options->verify_hash;
 to.verify_hash_algo = options->verify_hash_algo;
 to.verify_hash_depth = options->verify_hash_depth;
+to.verify_hash_no_ca = options->verify_hash_no_ca;
 #ifdef ENABLE_X509ALTUSERNAME
 memcpy(to.x509_username_field, options->x509_username_field, 
sizeof(to.x509_username_field));
 #else
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index fe9285384..e4c596b89 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -2991,21 +2991,11 @@ options_postprocess_verify_ce(const struct options 
*options,
 else
 {
 #ifdef ENABLE_CRYPTO_MBEDTLS
-if (!(options->ca_file))
-{
-msg(M_USAGE, "You must define CA file (--ca)");
-}
-
 if (options->ca_path)
 {
 msg(M_USAGE, "Parameter --capath cannot be used with the mbed 
TLS version version of OpenVPN.");
 }
-#else  /* ifdef ENABLE_CRYPTO_MBEDTLS */
-if ((!(options->ca_file)) && (!(options->ca_path)))
-{
-msg(M_USAGE, "You must define CA file (--ca) or CA path 
(--capath)");
-}
-#endif
+#endif  /* ifdef ENABLE_CRYPTO_MBEDTLS */
 if (pull)
 {
 
@@ -3737,6 +3727,13 @@ options_postprocess_mutate(struct options *o, struct 
env_set *es)
 options_postprocess_http_proxy_override(o);
 }
 #endif
+if (!o->ca_file && !o->ca_path && o->verify_hash
+&& o->verify_hash_depth == 0)
+{
+msg(M_INFO, "Using certificate fingerprint to verify peer (no CA "
+"option set). ");
+o->verify_hash_no_ca = true;
+}
 
 if (o->config && streq(o->config, "stdin") && o->remap_sigusr1 == SIGHUP)
 {
@@ -4032,8 +4029,11 @@ options_postprocess_filechecks(struct options *options)
 errs |= check_file_access_inline(options->dh_file_inline, CHKACC_FILE,
  options->dh_file, R_OK, "--dh");
 
-errs |= check_file_access_inline(options->ca_file_inline, CHKACC_FILE,
- options->ca_file, R_OK, "--ca");
+if (!options->verify_hash_no_ca)
+{
+err

Re: [Openvpn-devel] OpenVPN Linking Exception - current status report

[Arne Schwabe]

Am 15.02.23 um 13:31 schrieb David Sommerseth:


OpenVPN 2.x is licensed under the GNU Public License v2.0 (GPL-2.0). 
This license has served us well in the past and we are not trying to 
change that.  However, changes in licenses of our dependencies put us in 
an unfortunate situation.


So a good amount of time has passed and we got a lot of positive 
feedback to the license agreement. We got 107 positive responses back 
contributors including a positive one from Fox IT that covers all Fox It 
employers and a positive one from OpenVPN Inc that cover all code that 
OpenVPN Inc owns. And big thanks to everyone who said yes so far.



With that we are down to a small number of people (10) that are still 
missing acknowledgment.



Note that the actions for the individual source are my opinion/suggestions.

- Jason A. Donenfeld 

  Has stated to me in a private IRC message that I should bothering him
  about this. Only negative reaction so far.

  * Support fingerprint authentication without CA certificate

  The original v1 patch of this has been significantly rewritten/changed
  by me. We need to check which parts of the v3 patch that of the
  original patch can be still attributed to Jason and rewrite that part
  unless we still get a positive reaction.

- James Bottomley 

   No response to my license email, however was active this year on the 
mailing list.


   * Add unit tests for engine keys
   * engine key support

   This commits are only relevant for OpenSSL 1.x and we can probably
   remove support for this feature again if James does not positively.

- Josh Cepek 

  No response at all to my emails. Commits were in 2013 and 2016

  All seven commits look trivial to me. Need someone else to form an 
opinion if this can be seen as trivial in total or not.



- Guido Vranken 

  No response at all to my emails. Commit are in 2017.

  - 7 smallish commit, not sure if they are trivial. Best to try to
reach this person
  - might be part of some security audit, need to double check if there
are some strings attached to that.

- Jérémie Courrèges-Anglas 

  - raised the question if we can keep the OpenSSL license exception
forever to keep LibreSSL support. I think that is reasonable to just
keep the outdated exception even if it is just for a handful of
people using OpenVPN with LibreSSL. When we drafted the mails we
proposed to remove this because we overlooked LibreSSL.

   * Print time_t as long long and suseconds_t as long
probably not trivial.
   * Rest of the commits look very small and can probably classified as 
   trivial


- Andris Kalnozols 

  Address bounces, could not find any alternative method of contacting
  this person. three commits:

* extract_x509_extension(): hide status message during normal 
operation.

* Fix some typos in the man page.

trivial

   * Do not upcase x509-username-field for mixed-case arguments.
  non-trivial needs more investigation.


- Mathieu GIANNECCHINI 

  Mail address bounces, could not find an alternative contact.
 --tls-export-cert option only

  One commit having the following feature:

  "--tls-export-cert [directory] : Get peer cert in PEM format and 
store it \n"
  "  in an openvpn temporary file in [directory]. Peer 
cert is \n"
  "  stored before tls-verify script execution and 
deleted after.\n"


   We can remove the feature, it seems a corner case. If someone needs
   it can still be reimplemented.

- Holger Kummert 

  Reached out to other ex-Sophos employees, will try to contact him.

  One commit bordering on trivial.


- Markus Koetter 

  Email address bounces, found no alternative contact method. One 
commit in 2011


  Add extv3 X509 field support to --x509-username-field

  Needs --enable-x509-alt-username configure option to be enabled, not
  enabled by default.


- Vasily Kulikov 

  No response so far. Would be probably good to check for alternative
  email address

  One trivial commit and

  * Mac OS X Keychain management client

   contrib/keychain-mcd already removed since it had code quality issues.

   Code for management-external-cert still in OpenVPN, is not trivial



Arne


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 2/2] Fix CR_RESPONSE mangaement message using wrong key_id

[Arne Schwabe]

the management interface expects the management key id instead
of the openvpn key id. In the past they often were the same for low ids
which hid the bug quite well.

Also do not pick uninitialised keystates (management key_id is not valid
in these) and provide better logging when a key state is not found.

Change-Id: If9fa1165a0e886b570b3738546ed810a32367cbe
Signed-off-by: Arne Schwabe 
---
 src/openvpn/push.c   | 4 ++--
 src/openvpn/ssl_common.h | 2 +-
 src/openvpn/ssl_verify.c | 5 +
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index 8e9627199..8f0a534ac 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -267,9 +267,9 @@ receive_cr_response(struct context *c, const struct buffer 
*buffer)
 struct tls_session *session = >c2.tls_multi->session[TM_ACTIVE];
 struct man_def_auth_context *mda = session->opt->mda_context;
 struct env_set *es = session->opt->es;
-int key_id = get_primary_key(c->c2.tls_multi)->key_id;
+unsigned int mda_key_id = get_primary_key(c->c2.tls_multi)->mda_key_id;
 
-management_notify_client_cr_response(key_id, mda, es, m);
+management_notify_client_cr_response(mda_key_id, mda, es, m);
 #endif
 #if ENABLE_PLUGIN
 verify_crresponse_plugin(c->c2.tls_multi, m);
diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index ebfd25432..be0f18746 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -733,7 +733,7 @@ get_key_by_management_key_id(struct tls_multi *multi, 
unsigned int mda_key_id)
 for (int i = 0; i < KEY_SCAN_SIZE; ++i)
 {
 struct key_state *ks = get_key_scan(multi, i);
-if (ks->mda_key_id == mda_key_id)
+if (ks->mda_key_id == mda_key_id && ks->state > S_UNDEF)
 {
 return ks;
 }
diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index 4c78c2b2c..567f55ea3 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -1279,6 +1279,11 @@ tls_authenticate_key(struct tls_multi *multi, const 
unsigned int mda_key_id, con
 {
 ks->mda_status = auth ? ACF_SUCCEEDED : ACF_FAILED;
 }
+else
+{
+msg(D_TLS_DEBUG_LOW, "%s: no key state found for management key id 
"
+"%d", __func__, mda_key_id);
+}
 }
 return (bool) ks;
 }
-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 1/2] Introduce get_key_by_management_key_id helper function

[Arne Schwabe]

This function allows us to map from a management key id to a key structure
and also allows this function to be reused.

Signed-off-by: Arne Schwabe 
---
 src/openvpn/ssl_common.h | 20 
 src/openvpn/ssl_verify.c | 17 +++--
 2 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index 27b029479..ebfd25432 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -722,4 +722,24 @@ get_primary_key(const struct tls_multi *multi)
 return >session[TM_ACTIVE].key[KS_PRIMARY];
 }
 
+#ifdef ENABLE_MANAGEMENT
+/**
+ * Gets the \c key_state  object that belong to the management key id or
+ * return NULL if not found.
+ */
+static inline struct key_state *
+get_key_by_management_key_id(struct tls_multi *multi, unsigned int mda_key_id)
+{
+for (int i = 0; i < KEY_SCAN_SIZE; ++i)
+{
+struct key_state *ks = get_key_scan(multi, i);
+if (ks->mda_key_id == mda_key_id)
+{
+return ks;
+}
+}
+return NULL;
+}
+#endif
+
 #endif /* SSL_COMMON_H_ */
diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index 1b589f1a6..4c78c2b2c 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -1268,22 +1268,19 @@ tls_authentication_status(struct tls_multi *multi)
 bool
 tls_authenticate_key(struct tls_multi *multi, const unsigned int mda_key_id, 
const bool auth, const char *client_reason)
 {
-bool ret = false;
+struct key_state *ks = NULL;
 if (multi)
 {
-int i;
+
 auth_set_client_reason(multi, client_reason);
-for (i = 0; i < KEY_SCAN_SIZE; ++i)
+ks = get_key_by_management_key_id(multi, mda_key_id);
+
+if (ks)
 {
-struct key_state *ks = get_key_scan(multi, i);
-if (ks->mda_key_id == mda_key_id)
-{
-ks->mda_status = auth ? ACF_SUCCEEDED : ACF_FAILED;
-ret = true;
-}
+ks->mda_status = auth ? ACF_SUCCEEDED : ACF_FAILED;
 }
 }
-return ret;
+return (bool) ks;
 }
 #endif /* ifdef ENABLE_MANAGEMENT */
 
-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] Fix CR_RESPONSE mangaement message using wrong key_id

[Arne Schwabe]

the management interface expects the management key id instead
of the openvpn key id. In the past they often were the same for low ids
which hid the bug quite well.

Also do not pick uninitialised keystates (management key_id is not valid
in these) and provide better logging when a key state is not found.

Change-Id: If9fa1165a0e886b570b3738546ed810a32367cbe
Signed-off-by: Arne Schwabe 
---
 src/openvpn/push.c   | 4 ++--
 src/openvpn/ssl_common.h | 2 +-
 src/openvpn/ssl_verify.c | 5 +
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index 54e53f6aa..99abe5440 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -267,9 +267,9 @@ receive_cr_response(struct context *c, const struct buffer 
*buffer)
 struct tls_session *session = >c2.tls_multi->session[TM_ACTIVE];
 struct man_def_auth_context *mda = session->opt->mda_context;
 struct env_set *es = session->opt->es;
-int key_id = get_primary_key(c->c2.tls_multi)->key_id;
+unsigned int mda_key_id = get_primary_key(c->c2.tls_multi)->mda_key_id;
 
-management_notify_client_cr_response(key_id, mda, es, m);
+management_notify_client_cr_response(mda_key_id, mda, es, m);
 #endif
 #if ENABLE_PLUGIN
 verify_crresponse_plugin(c->c2.tls_multi, m);
diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index ebfd25432..be0f18746 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -733,7 +733,7 @@ get_key_by_management_key_id(struct tls_multi *multi, 
unsigned int mda_key_id)
 for (int i = 0; i < KEY_SCAN_SIZE; ++i)
 {
 struct key_state *ks = get_key_scan(multi, i);
-if (ks->mda_key_id == mda_key_id)
+if (ks->mda_key_id == mda_key_id && ks->state > S_UNDEF)
 {
 return ks;
 }
diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index 4c78c2b2c..567f55ea3 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -1279,6 +1279,11 @@ tls_authenticate_key(struct tls_multi *multi, const 
unsigned int mda_key_id, con
 {
 ks->mda_status = auth ? ACF_SUCCEEDED : ACF_FAILED;
 }
+else
+{
+msg(D_TLS_DEBUG_LOW, "%s: no key state found for management key id 
"
+"%d", __func__, mda_key_id);
+}
 }
 return (bool) ks;
 }
-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v3] Add Apache2 linking with for new commits

[Arne Schwabe]

Am 15.05.23 um 19:26 schrieb Jeremie Courreges-Anglas:

On Wed, Apr 26 2023, Arne Schwabe  wrote:

After first round of mailing people with more than 10 commits we have
almost all committers have agreed. This put this license in the realm
of having a realistic change to work. Had any of these contributers
disagreed, rewriting all their code might have been not feasible.

The rationale of adding this exception now is to avoid having to
have a second round of agreement for new contributers and ensure
that all new code will include the exemption.


The proposal here (already committed) merely adds an exception for
linking with Apache 2 libraries.  As discussed privately with Arne, the
proposal that I have received was to *replace* the OpenSSL linking
exception with the Apache 2 linking exception.  The rationale being that
OpenSSL 3 has moved to Apache 2, and that OpenVPN plans to drop support
for OpenSSL 1.X at some point.

The LibreSSL project has not moved to Apache 2 and doesn't plan to do
so.  OpenVPN currently builds and runs fine using LibreSSL, without any
patch.  I don't know about the state of OpenSSL 3 APIs in LibreSSL, but
I suggest not to tie together licensing matters and technical matters.

I think the existing OpenSSL linking exception should be kept as is.
For the (admittedly few and uncomplicated) changes I have contributed to
the OpenVPN code base, I would agree with a licensing change that
doesn't remove this exception.


The reason we worded it to replace the original exception is that after 
we drop OpenSSL 1.0.2 and 1.1.1 support there would be no perceived 
reason to keep the original exception as OpenSSL will not back to that 
license and no other library will pick up that weird license. As long as 
we support OpenSSL 1.0.2 and 1.1.1 we have to keep that license 
exception anyway (which will be probably at least a few years to support 
RHEL8 (EOL 2029)). When we wrote those mails we did not consider LibreSSL.


Until this time, we intent to keep both exceptions anyway. Both 
exceptions are written in a way that they can be dropped at any time by 
any fork.


Arne



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Automatically restart Linux systemd OpenVPN client service on failure

[Arne Schwabe]

Am 13.05.23 um 16:47 schrieb Melvin Vermeeren:

Hi Arne,

On Saturday, 13 May 2023 16:28:29 CEST Arne Schwabe wrote:

Can you provide some more detail here? Otherwise this seem a bit
nebulously to me what exactly explodes and goes wrong.


I changed the --keepalive setting on the server, lowering the timeout from 120
to 60 seconds to make things somewhat more responsive. Figured a small push
change like this wouldn't really cause trouble.

This triggered a TUN device close/reopen, which makes sense in hindsight.
Clients are configured to drop privilege with --user and --group after
connecting, so they could not make the change needed.


NOTE: Pulled options changed on restart, will need to close and reopen
TUN/TAP device.
...
ERROR: Cannot ioctl TUNSETIFF tun_vpn: Operation not permitted (errno=1)
Exiting due to fatal error


This is not the only case though, I also had some problems with DCO on a
Debian testing client recently, resulting in failure after network was offline
for quite a while. Official Debian package DCO 0.0+git20230324-1.



Yes the option we ignore to check if we have to reopen the tun device is 
quite short. We should probably turn this into a positive list instead 
of assuming that all options need to trigger to a tun reopen/close.


Arne



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Automatically restart Linux systemd OpenVPN client service on failure

[Arne Schwabe]

Am 13.05.23 um 16:24 schrieb Melvin Vermeeren:

Hi all,

Today I changed some OpenVPN server configuration and restarted the service,
thinking all clients will reconnect just fine as usual. Unlike other days
however, all Linux clients ended up exploding due to unexpected tun-device
recreation and lack of permission to do so, resulting in fatal exit with
service failure. Quite a surprise to me, and a bit of a mess to fix!


Can you provide some more detail here? Otherwise this seem a bit 
nebulously to me what exactly explodes and goes wrong.



Arne


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [Openvpn-users] auth-token-user/auth-token issue with "TLS Auth Error: username attempted to change"

[Arne Schwabe]

Am 05.05.23 um 09:33 schrieb Gert Doering:

Hi,

On Fri, May 05, 2023 at 09:14:03AM +0200, Ralf Hildebrandt via Openvpn-users 
wrote:

May  5 09:06:00 openvpn-gw170-int openvpn-udp[29574]: 
hildeb/10.31.192.115:55334 TLS Auth Error: username attempted to change from 
'hildeb' to 'hildeb::1f047fb6' -- tunnel disabled
May  5 09:06:00 openvpn-gw170-int openvpn-udp[29574]: 
hildeb/10.31.192.115:55334 TLS Auth Error: Auth Username/Password verification 
failed for peer

What do we have to do to make the server accept the the
auth-token-user it pushed to the client?


As of today, there is no way to make it do that.  On initial TLS
authentication, the session is tied hard to the initial username(*),
and changing username on reauthentication is not allowed.

That said, this breaks at least two use cases

  - there was no initial username/password authentication at all - and
you absolutely need auth-token-user here, to make the client actually
send the auth-token back.  It fails, because the server is locked
to "empty username" - I have sent a patch for this, but Arne is not
fully agreeing with me that it's the right thing

  - actually *changing* the auth-token-user from an original username/password
authentication - it runs into the same problem, but this might be
workaroundable by not actually pushing a new "user".  So, question to
you, what is the intention behind changing the username here?


For "my case", I think handling the "username was empty before" case
specially is the easy way forward (even if Arne disagrees), but for
"your case" we'd need to disable the "tie TLS session to username",
which is a no-go for Arne...


So the empty username is problematic since we use the auth-token to 
reauthenticate with that username and then OpenVPN will tell you that 
you have an empty username that is fully authenticated. And I am not 
sure what that would mean in any context. Also if you have no username, 
you either need to track the session in the auth token to ensure that 
you the same user as before or that session could be used with any 
certificate.


For changing the username, that looks simple on the first glance but we 
need to carefully go through our to see where the username is 
used/locked in our code to understand what implications this has.


Arne



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] Remove unused variable line

[Arne Schwabe]

The newer compilers started to complain about this.

Change-Id: I784def4d941b7d21c7979f84f8681719c9ff7a53
Signed-off-by: Arne Schwabe 
---
 src/openvpn/pool.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/openvpn/pool.c b/src/openvpn/pool.c
index f899b95d2..4af9bcb10 100644
--- a/src/openvpn/pool.c
+++ b/src/openvpn/pool.c
@@ -608,7 +608,6 @@ ifconfig_pool_read(struct ifconfig_pool_persist *persist, 
struct ifconfig_pool *
 struct gc_arena gc = gc_new();
 struct buffer in = alloc_buf_gc(256, );
 char *cn_buf, *ip_buf, *ip6_buf;
-int line = 0;
 
 ALLOC_ARRAY_CLEAR_GC(cn_buf, char, buf_size, );
 ALLOC_ARRAY_CLEAR_GC(ip_buf, char, buf_size, );
@@ -621,7 +620,6 @@ ifconfig_pool_read(struct ifconfig_pool_persist *persist, 
struct ifconfig_pool *
 {
 break;
 }
-++line;
 if (!BLEN())
 {
 continue;
-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v3] Add Apache2 linking with for new commits

[Arne Schwabe]

After first round of mailing people with more than 10 commits we have
almost all committers have agreed. This put this license in the realm
of having a realistic change to work. Had any of these contributers
disagreed, rewriting all their code might have been not feasible.

The rationale of adding this exception now is to avoid having to
have a second round of agreement for new contributers and ensure
that all new code will include the exemption.

patch v2: add explaination and use exception rather than excemption
patch v3: actually send v3

Change-Id: Ide83f914f383b53ef37ddf628e4da5a78e241bf0
Signed-off-by: Arne Schwabe 
---
 COPYING | 47 +++
 1 file changed, 47 insertions(+)

diff --git a/COPYING b/COPYING
index e12c51414..a6f8a6f5f 100644
--- a/COPYING
+++ b/COPYING
@@ -31,6 +31,53 @@ OpenVPN license:
   file, but you are not obligated to do so.  If you do not wish to
   do so, delete this exception statement from your version.
 
+Apache2 linking exception:
+---
+OpenVPN is currently undergoing a license change to add an exception for
+Apache 2 linking. The following exception is only valid for new contributions
+after COMMITDATE and past contribution where the authors have already agreed
+to the exception.
+
+  In addition, as a special exception, OpenVPN Inc and the
+  contributors give permission to link the code of this program to
+  libraries (the "Libraries") licensed under the Apache License
+  version 2.0 (this work and any linked library the "Combined Work")
+  and copy and distribute the Combined Work without an obligation to
+  license the Libraries under the GNU General Public License v2
+  (GPL-2.0) as required by Section 2 of the GPL-2.0, and without an
+  obligation to refrain from imposing any additional restrictions in
+  the Apache License version 2 that are not in the GPL-2.0, as
+  required by Section 6 of the GPL-2.0.  You must comply with the`
+  GPL-2.0 in all other respects for the Combined Work, including
+  the obligation to provide source code.  If you modify this file, you
+  may extend this exception to your version of the file, but you are
+  not obligated to do so.  If you do not wish to do so, delete this
+  exception statement from your version.
+
+For better understanding, in plain non-legalese English this basically says:
+
+ * The intention for this license exception is to allow OpenVPN to be
+   linked against APL-2 licensed libraries, even where the GPL-2.0 and
+   APL-2 licenses conflict from a legal perspective.
+
+ * OpenVPN itself will stay GPL-2.0 and the code belonging to the
+   OpenVPN project must comply to the GPL-2.0 license.  This is NOT
+   dual-licensing of the OpenVPN code base.
+
+ * This license exception DOES NOT require NOR expect a license change
+   of the APL-2 based library.  This exception allows using the APL-2
+   library as-is.  However, when distributing a compiled OpenVPN binary
+   linking against APL-2 libraries ("Combined Work"), the REQUIREMENT is
+   that the APL-2 library MUST also be available on similar terms as in
+   GPL-2.0, like providing the source code of the library upon request,
+   except in the two specific ways mentioned.
+
+ * If the APL-2 based library forbids such linking and distribution,
+   this license exception DOES NOT overrule the restriction of the APL-2
+   based library.  If the APL-2 library cannot satisfy the requirements
+   in this license exception, you CANNOT distribute an OpenVPN binary
+   linked with this library.
+
 LZO license:
 
 
-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v2] Add Apache2 linking with for new commits

[Arne Schwabe]

After first round of mailing people with more than 10 commits we have
almost all committers have agreed. This put this license in the realm
of having a realistic change to work. Had any of these contributers
disagreed, rewriting all their code might have been not feasible.

The rationale of adding this exception now is to avoid having to
have a second round of agreement for new contributers and ensure
that all new code will include the exemption.

patch v2: add explaination and use exception rather than excemption

Change-Id: Ide83f914f383b53ef37ddf628e4da5a78e241bf0
Signed-off-by: Arne Schwabe 
---
 COPYING | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/COPYING b/COPYING
index e12c51414..b4a148494 100644
--- a/COPYING
+++ b/COPYING
@@ -31,6 +31,29 @@ OpenVPN license:
   file, but you are not obligated to do so.  If you do not wish to
   do so, delete this exception statement from your version.
 
+Apache2 linking exemption:
+---
+OpenVPN is currently undergoing a license change to add an exemption for
+Apache 2 linking. The following exemption is only valid for new contributions
+after COMMITDATE and past contribution where the authors have already agreed
+to the exemption.
+
+  In addition, as a special exception, OpenVPN Inc and the
+  contributors give permission to link the code of this program to
+  libraries (the "Libraries") licensed under the Apache License
+  version 2.0 (this work and any linked library the "Combined Work")
+  and copy and distribute the Combined Work without an obligation to
+  license the Libraries under the GNU General Public License v2
+  (GPL-2.0) as required by Section 2 of the GPL-2.0, and without an
+  obligation to refrain from imposing any additional restrictions in
+  the Apache License version 2 that are not in the GPL-2.0, as
+  required by Section 6 of the GPL-2.0.  You must comply with the
+  GPL-2.0 in all other respects for the Combined Work, including
+  the obligation to provide source code.  If you modify this file, you
+  may extend this exception to your version of the file, but you are
+  not obligated to do so.  If you do not wish to do so, delete this
+  exception statement from your version.
+
 LZO license:
 
 
-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v2] Add missing check for nl_socket_alloc failure

[Arne Schwabe]

This can happen if the memory alloc fails.

Patch V2: add goto error

Change-Id: Iee66caa794d267ac5f8bee584633352893047171
Signed-off-by: Arne Schwabe 
---
 src/openvpn/dco_linux.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 41540c0f8..95fe94848 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -83,6 +83,13 @@ resolve_ovpn_netlink_id(int msglevel)
 int ret;
 struct nl_sock *nl_sock = nl_socket_alloc();
 
+if (!nl_sock)
+{
+msg(msglevel, "Allocating net link socket failed");
+ret = -1;
+goto err_sock;
+}
+
 ret = genl_connect(nl_sock);
 if (ret)
 {
-- 
2.37.1 (Apple Git-137.1)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] Add Apache2 linking with for new commits

[Arne Schwabe]

After first round of mailing people with more than 10 commits we have
almost all committers have agreed. This put this license in the realm
of having a realistic change to work. Had any of these contributers
disagreed, rewriting all their code might have been not feasible.

The rationale of adding this exception now is to avoid having to
have a second round of agreement for new contributers and ensure
that all new code will include the exemption.

Change-Id: Ide83f914f383b53ef37ddf628e4da5a78e241bf0
Signed-off-by: Arne Schwabe 
---
 COPYING | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/COPYING b/COPYING
index e12c51414..b4a148494 100644
--- a/COPYING
+++ b/COPYING
@@ -31,6 +31,29 @@ OpenVPN license:
   file, but you are not obligated to do so.  If you do not wish to
   do so, delete this exception statement from your version.
 
+Apache2 linking exemption:
+---
+OpenVPN is currently undergoing a license change to add an exemption for
+Apache 2 linking. The following exemption is only valid for new contributions
+after COMMITDATE and past contribution where the authors have already agreed
+to the exemption.
+
+  In addition, as a special exception, OpenVPN Inc and the
+  contributors give permission to link the code of this program to
+  libraries (the "Libraries") licensed under the Apache License
+  version 2.0 (this work and any linked library the "Combined Work")
+  and copy and distribute the Combined Work without an obligation to
+  license the Libraries under the GNU General Public License v2
+  (GPL-2.0) as required by Section 2 of the GPL-2.0, and without an
+  obligation to refrain from imposing any additional restrictions in
+  the Apache License version 2 that are not in the GPL-2.0, as
+  required by Section 6 of the GPL-2.0.  You must comply with the
+  GPL-2.0 in all other respects for the Combined Work, including
+  the obligation to provide source code.  If you modify this file, you
+  may extend this exception to your version of the file, but you are
+  not obligated to do so.  If you do not wish to do so, delete this
+  exception statement from your version.
+
 LZO license:
 
 
-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Fix compile error on TARGET_ANDROID

[Arne Schwabe]

Am 17.04.23 um 15:40 schrieb Arne Schwabe:

Commit 3132bead49 accidentially was submitted with a missing semicolon
at the end of the line. Whoops.

Signed-off-by: Arne Schwabe 
---
  src/openvpn/socket.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
index ab8cc754..fc643c1c 100644
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -1165,7 +1165,7 @@ protect_fd_nonlocal(int fd, const struct sockaddr *addr)
  {
  if (!management)
  {
-msg(M_FATAL, "Required management interface not available.")
+msg(M_FATAL, "Required management interface not available.");
  }
  
  /* pass socket FD to management interface to pass on to VPNService API


Note that this patch is uncrusity clean on my own machine but fails on 
github for some reason. Diff is:


--- a/src/openvpn/route.c
+++ b/src/openvpn/route.c
@@ -1103,7 +1103,7 @@ redirect_default_route_to_vpn(struct route_list 
*rl, const struct tuntap *tt,

 ret = add_route3(0, 0, rl->spec.remote_endpoint, tt,
  flags, >rgi, es, ctx) && ret;
 }
-#endif
+#endif /* ifdef TARGET_ANDROID */
 }

 /* set a flag so we can undo later */


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] Fix compile error on TARGET_ANDROID

[Arne Schwabe]

Commit 3132bead49 accidentially was submitted with a missing semicolon
at the end of the line. Whoops.

Signed-off-by: Arne Schwabe 
---
 src/openvpn/socket.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
index ab8cc754..fc643c1c 100644
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -1165,7 +1165,7 @@ protect_fd_nonlocal(int fd, const struct sockaddr *addr)
 {
 if (!management)
 {
-msg(M_FATAL, "Required management interface not available.")
+msg(M_FATAL, "Required management interface not available.");
 }
 
 /* pass socket FD to management interface to pass on to VPNService API
-- 
2.39.2 (Apple Git-143)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] doc: run rst2* with --strict to catch warnings

[Arne Schwabe]

Am 31.03.23 um 15:24 schrieb Frank Lichtenheld:

Basically -Werror for docutils.

Fix all issues raised by this. The following issue
classes were reported:

Possible title underline, too short for the title.
Treating it as ordinary text because it's so short.
(:: at the start of the line directly below text,
either add empty line of merge into : on previous line)

Enumerated list start value not ordinal-1
(error in numbering)


Thanks. That helps catching these mistakes early.

Acked-By: Arne Schwabe 



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v2] Add missing check for nl_socket_alloc failure

[Arne Schwabe]

This can happen if the memory alloc fails.

Patch V2: add goto error

Change-Id: Iee66caa794d267ac5f8bee584633352893047171
Signed-off-by: Arne Schwabe 
---
 src/openvpn/dco_linux.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 41540c0f8..95fe94848 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -83,6 +83,13 @@ resolve_ovpn_netlink_id(int msglevel)
 int ret;
 struct nl_sock *nl_sock = nl_socket_alloc();
 
+if (!nl_sock)
+{
+msg(msglevel, "Allocating net link socket failed");
+ret = -1;
+goto err_sock;
+}
+
 ret = genl_connect(nl_sock);
 if (ret)
 {
-- 
2.37.1 (Apple Git-137.1)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v4] Parse compression options and bail out when compression is disabled

[Arne Schwabe]

This change keeps the option parsing of compression options even when
compression is disabled. This allows OpenVPN to also refuse/reject connections
that try to use compression when compression is completely disabled.

Patch v4: fix one missing USE_COMP

Change-Id: I9d7afd8f1d67d2455b4ec6bc12f4dcde80140c4f
Signed-off-by: Arne Schwabe 
---
 src/openvpn/comp.c| 14 ---
 src/openvpn/comp.h| 85 ++-
 src/openvpn/init.c|  4 +-
 src/openvpn/multi.c   |  2 -
 src/openvpn/options.c | 12 +-
 src/openvpn/options.h |  4 --
 6 files changed, 56 insertions(+), 65 deletions(-)

diff --git a/src/openvpn/comp.c b/src/openvpn/comp.c
index c7a562f5a..27b640ce0 100644
--- a/src/openvpn/comp.c
+++ b/src/openvpn/comp.c
@@ -29,10 +29,11 @@
 
 #include "syshead.h"
 
-#ifdef USE_COMP
-
 #include "comp.h"
 #include "error.h"
+
+#ifdef USE_COMP
+
 #include "otime.h"
 
 #include "memdbg.h"
@@ -158,6 +159,7 @@ comp_generate_peer_info_string(const struct 
compress_options *opt, struct buffer
 buf_printf(out, "IV_COMP_STUB=1\n");
 buf_printf(out, "IV_COMP_STUBv2=1\n");
 }
+#endif /* USE_COMP */
 
 bool
 check_compression_settings_valid(struct compress_options *info, int msglevel)
@@ -170,8 +172,13 @@ check_compression_settings_valid(struct compress_options 
*info, int msglevel)
 if (info->alg != COMP_ALGV2_UNCOMPRESSED && info->alg != COMP_ALG_UNDEF
 && (info->flags & COMP_F_ALLOW_NOCOMP_ONLY))
 {
+#ifdef USE_COMP
 msg(msglevel, "Compression or compression stub framing is not allowed "
 "since data-channel offloading is enabled.");
+#else
+msg(msglevel, "Compression or compression stub framing is not allowed "
+"since OpenVPN was built without compression support.");
+#endif
 return false;
 }
 
@@ -199,6 +206,3 @@ check_compression_settings_valid(struct compress_options 
*info, int msglevel)
 #endif
 return true;
 }
-
-
-#endif /* USE_COMP */
diff --git a/src/openvpn/comp.h b/src/openvpn/comp.h
index 71b350d09..89940cf3a 100644
--- a/src/openvpn/comp.h
+++ b/src/openvpn/comp.h
@@ -28,12 +28,19 @@
 #ifndef OPENVPN_COMP_H
 #define OPENVPN_COMP_H
 
-#ifdef USE_COMP
+/* We always parse all compression options, so we include these defines/struct
+ * outside of the USE_COMP define */
 
-#include "buffer.h"
-#include "mtu.h"
-#include "common.h"
-#include "status.h"
+/* Compression flags */
+#define COMP_F_ADAPTIVE (1<<0) /* COMP_ALG_LZO only */
+#define COMP_F_ALLOW_COMPRESS   (1<<1) /* not only downlink is compressed 
but also uplink */
+#define COMP_F_SWAP (1<<2) /* initial command byte is swapped 
with last byte in buffer to preserve payload alignment */
+#define COMP_F_ADVERTISE_STUBS_ONLY (1<<3) /* tell server that we only support 
compression stubs */
+#define COMP_F_ALLOW_STUB_ONLY  (1<<4) /* Only accept stub compression, 
even with COMP_F_ADVERTISE_STUBS_ONLY
+* we still accept other 
compressions to be pushed */
+#define COMP_F_MIGRATE  (1<<5) /* push stub-v2 or comp-lzo no when 
we see a client with comp-lzo in occ */
+#define COMP_F_ALLOW_ASYM   (1<<6) /* Compression was explicitly set 
to allow asymetric compression */
+#define COMP_F_ALLOW_NOCOMP_ONLY(1<<7) /* Do not allow compression framing 
(breaks DCO) */
 
 /* algorithms */
 #define COMP_ALG_UNDEF  0
@@ -51,16 +58,37 @@
  #define COMP_ALGV2_SNAPPY   13
  */
 
-/* Compression flags */
-#define COMP_F_ADAPTIVE (1<<0) /* COMP_ALG_LZO only */
-#define COMP_F_ALLOW_COMPRESS   (1<<1) /* not only downlink is compressed 
but also uplink */
-#define COMP_F_SWAP (1<<2) /* initial command byte is swapped 
with last byte in buffer to preserve payload alignment */
-#define COMP_F_ADVERTISE_STUBS_ONLY (1<<3) /* tell server that we only support 
compression stubs */
-#define COMP_F_ALLOW_STUB_ONLY  (1<<4) /* Only accept stub compression, 
even with COMP_F_ADVERTISE_STUBS_ONLY
-* we still accept other 
compressions to be pushed */
-#define COMP_F_MIGRATE  (1<<5) /* push stub-v2 or comp-lzo no when 
we see a client with comp-lzo in occ */
-#define COMP_F_ALLOW_ASYM   (1<<6) /* Compression was explicitly set 
to allow asymetric compression */
-#define COMP_F_ALLOW_NOCOMP_ONLY(1<<7) /* Do not allow compression framing 
(breaks DCO) */
+/*
+ * Information that basically identifies a compression
+ * algorithm and related flags.
+ */
+struct compress_options
+{
+int alg;
+unsigned int flags;
+};
+
+static inline bool
+comp_non_stub_enabled(const struct compress_options *inf

[Openvpn-devel] [PATCH v4] Add 'allow-compression stub-only' internally for DCO

[Arne Schwabe]

This changes the "no" setting of allow-compression to also refuse framing.
This is important for our DCO implementation as these do not implement framing.

This behaviour surfaced when a commercial VPN provider was pushing
"comp-lzo no" to a client with DCO. While we are technically at fault here
for announcing comp-lzo no support by announcing IV_LZO_STUB=1, the
VPN provider continues to push "comp-lzo no" even in absense of that
flag.

As the new default we default to allow-compression stub-only if a stub option
is found in the config and to allow-compression no otherwise. This ensures
that we only enable DCO when no compression framing is used.

This will now also bail out if the server pushes a compression setting that
we do not support as mismatching compression is almost never a working
connection. In my lz4-v2 and lzo-v2 you might have a connection that works
mostly but some packets will be dropped since they compressed which is not
desirable either since it becomes very hard to debug.

Patch v2: bail out if server pushes an unsupported method. Also include this
  bail out logic when OpenVPN is compiled without compression support.

Patch v3: always parse all compression option and move logic to check method
Patch v4: fix for not setting correct default for non-dco

Change-Id: Ibd0c77af24e2214b3055d585dc23a4b06dccd414
Signed-off-by: Arne Schwabe 
---
 doc/man-sections/protocol-options.rst |  4 ++-
 src/openvpn/comp.c| 47 ++-
 src/openvpn/comp.h|  2 +-
 src/openvpn/options.c | 14 ++--
 4 files changed, 46 insertions(+), 21 deletions(-)

diff --git a/doc/man-sections/protocol-options.rst 
b/doc/man-sections/protocol-options.rst
index 248f65cfd..055d08f95 100644
--- a/doc/man-sections/protocol-options.rst
+++ b/doc/man-sections/protocol-options.rst
@@ -25,7 +25,9 @@ configured in a compatible way between both the local and 
remote side.
   compression at the same time is not a feasible option.
 
   :code:`no`  (default)
-  OpenVPN will refuse any non-stub compression.
+  OpenVPN will refuse any compression.  If data-channel offloading
+  is enabled. OpenVPN will additionally also refuse compression
+  framing (stub).
 
   :code:`yes`
   OpenVPN will send and receive compressed packets.
diff --git a/src/openvpn/comp.c b/src/openvpn/comp.c
index d6d8029da..c7a562f5a 100644
--- a/src/openvpn/comp.c
+++ b/src/openvpn/comp.c
@@ -134,36 +134,51 @@ comp_print_stats(const struct compress_context *compctx, 
struct status_output *s
 void
 comp_generate_peer_info_string(const struct compress_options *opt, struct 
buffer *out)
 {
-if (opt)
+if (!opt || opt->flags & COMP_F_ALLOW_NOCOMP_ONLY)
+{
+return;
+}
+
+bool lzo_avail = false;
+if (!(opt->flags & COMP_F_ADVERTISE_STUBS_ONLY))
 {
-bool lzo_avail = false;
-if (!(opt->flags & COMP_F_ADVERTISE_STUBS_ONLY))
-{
 #if defined(ENABLE_LZ4)
-buf_printf(out, "IV_LZ4=1\n");
-buf_printf(out, "IV_LZ4v2=1\n");
+buf_printf(out, "IV_LZ4=1\n");
+buf_printf(out, "IV_LZ4v2=1\n");
 #endif
 #if defined(ENABLE_LZO)
-buf_printf(out, "IV_LZO=1\n");
-lzo_avail = true;
+buf_printf(out, "IV_LZO=1\n");
+lzo_avail = true;
 #endif
-}
-if (!lzo_avail)
-{
-buf_printf(out, "IV_LZO_STUB=1\n");
-}
-buf_printf(out, "IV_COMP_STUB=1\n");
-buf_printf(out, "IV_COMP_STUBv2=1\n");
 }
+if (!lzo_avail)
+{
+buf_printf(out, "IV_LZO_STUB=1\n");
+}
+buf_printf(out, "IV_COMP_STUB=1\n");
+buf_printf(out, "IV_COMP_STUBv2=1\n");
 }
 
 bool
 check_compression_settings_valid(struct compress_options *info, int msglevel)
 {
+/*
+ * We also allow comp-stub-v2 here as it technically allows escaping of
+ * weird mac address and IPv5 protocol but practically always is used
+ * as an way to disable all framing.
+ */
+if (info->alg != COMP_ALGV2_UNCOMPRESSED && info->alg != COMP_ALG_UNDEF
+&& (info->flags & COMP_F_ALLOW_NOCOMP_ONLY))
+{
+msg(msglevel, "Compression or compression stub framing is not allowed "
+"since data-channel offloading is enabled.");
+return false;
+}
+
 if ((info->flags & COMP_F_ALLOW_STUB_ONLY) && comp_non_stub_enabled(info))
 {
 msg(msglevel, "Compression is not allowed since allow-compression is "
-"set to 'no'");
+"set to 'stub-only'");
 return false;
 }
 #ifndef ENABLE_LZ4
diff --git a/src/openvpn/comp.h b/src/openvpn/comp.h
index 8636727ab..71b350d09 100644
--- a/src/op

[Openvpn-devel] [PATCH v3 4/4] Parse compression options and bail out when compression is disabled

[Arne Schwabe]

This change keeps the option parsing of compression options even when
compression is disabled. This allows OpenVPN to also refuse/reject connections
that try to use compression when compression is completely disabled.

Change-Id: I9d7afd8f1d67d2455b4ec6bc12f4dcde80140c4f
Signed-off-by: Arne Schwabe 
---
 src/openvpn/comp.c| 14 ---
 src/openvpn/comp.h| 85 ++-
 src/openvpn/init.c|  2 -
 src/openvpn/multi.c   |  2 -
 src/openvpn/options.c | 12 +-
 src/openvpn/options.h |  4 --
 6 files changed, 54 insertions(+), 65 deletions(-)

diff --git a/src/openvpn/comp.c b/src/openvpn/comp.c
index c7a562f5a..27b640ce0 100644
--- a/src/openvpn/comp.c
+++ b/src/openvpn/comp.c
@@ -29,10 +29,11 @@
 
 #include "syshead.h"
 
-#ifdef USE_COMP
-
 #include "comp.h"
 #include "error.h"
+
+#ifdef USE_COMP
+
 #include "otime.h"
 
 #include "memdbg.h"
@@ -158,6 +159,7 @@ comp_generate_peer_info_string(const struct 
compress_options *opt, struct buffer
 buf_printf(out, "IV_COMP_STUB=1\n");
 buf_printf(out, "IV_COMP_STUBv2=1\n");
 }
+#endif /* USE_COMP */
 
 bool
 check_compression_settings_valid(struct compress_options *info, int msglevel)
@@ -170,8 +172,13 @@ check_compression_settings_valid(struct compress_options 
*info, int msglevel)
 if (info->alg != COMP_ALGV2_UNCOMPRESSED && info->alg != COMP_ALG_UNDEF
 && (info->flags & COMP_F_ALLOW_NOCOMP_ONLY))
 {
+#ifdef USE_COMP
 msg(msglevel, "Compression or compression stub framing is not allowed "
 "since data-channel offloading is enabled.");
+#else
+msg(msglevel, "Compression or compression stub framing is not allowed "
+"since OpenVPN was built without compression support.");
+#endif
 return false;
 }
 
@@ -199,6 +206,3 @@ check_compression_settings_valid(struct compress_options 
*info, int msglevel)
 #endif
 return true;
 }
-
-
-#endif /* USE_COMP */
diff --git a/src/openvpn/comp.h b/src/openvpn/comp.h
index 71b350d09..89940cf3a 100644
--- a/src/openvpn/comp.h
+++ b/src/openvpn/comp.h
@@ -28,12 +28,19 @@
 #ifndef OPENVPN_COMP_H
 #define OPENVPN_COMP_H
 
-#ifdef USE_COMP
+/* We always parse all compression options, so we include these defines/struct
+ * outside of the USE_COMP define */
 
-#include "buffer.h"
-#include "mtu.h"
-#include "common.h"
-#include "status.h"
+/* Compression flags */
+#define COMP_F_ADAPTIVE (1<<0) /* COMP_ALG_LZO only */
+#define COMP_F_ALLOW_COMPRESS   (1<<1) /* not only downlink is compressed 
but also uplink */
+#define COMP_F_SWAP (1<<2) /* initial command byte is swapped 
with last byte in buffer to preserve payload alignment */
+#define COMP_F_ADVERTISE_STUBS_ONLY (1<<3) /* tell server that we only support 
compression stubs */
+#define COMP_F_ALLOW_STUB_ONLY  (1<<4) /* Only accept stub compression, 
even with COMP_F_ADVERTISE_STUBS_ONLY
+* we still accept other 
compressions to be pushed */
+#define COMP_F_MIGRATE  (1<<5) /* push stub-v2 or comp-lzo no when 
we see a client with comp-lzo in occ */
+#define COMP_F_ALLOW_ASYM   (1<<6) /* Compression was explicitly set 
to allow asymetric compression */
+#define COMP_F_ALLOW_NOCOMP_ONLY(1<<7) /* Do not allow compression framing 
(breaks DCO) */
 
 /* algorithms */
 #define COMP_ALG_UNDEF  0
@@ -51,16 +58,37 @@
  #define COMP_ALGV2_SNAPPY   13
  */
 
-/* Compression flags */
-#define COMP_F_ADAPTIVE (1<<0) /* COMP_ALG_LZO only */
-#define COMP_F_ALLOW_COMPRESS   (1<<1) /* not only downlink is compressed 
but also uplink */
-#define COMP_F_SWAP (1<<2) /* initial command byte is swapped 
with last byte in buffer to preserve payload alignment */
-#define COMP_F_ADVERTISE_STUBS_ONLY (1<<3) /* tell server that we only support 
compression stubs */
-#define COMP_F_ALLOW_STUB_ONLY  (1<<4) /* Only accept stub compression, 
even with COMP_F_ADVERTISE_STUBS_ONLY
-* we still accept other 
compressions to be pushed */
-#define COMP_F_MIGRATE  (1<<5) /* push stub-v2 or comp-lzo no when 
we see a client with comp-lzo in occ */
-#define COMP_F_ALLOW_ASYM   (1<<6) /* Compression was explicitly set 
to allow asymetric compression */
-#define COMP_F_ALLOW_NOCOMP_ONLY(1<<7) /* Do not allow compression framing 
(breaks DCO) */
+/*
+ * Information that basically identifies a compression
+ * algorithm and related flags.
+ */
+struct compress_options
+{
+int alg;
+unsigned int flags;
+};
+
+static inline bool
+comp_non_stub_enabled(const struct compress_options *info)
+{
+return info-&

[Openvpn-devel] [PATCH v3 2/4] Refuse connection if server pushes an option contradicting allow-compress

[Arne Schwabe]

This removes also the checks in options.c itself as they we now bail out
later and no longer need to ignore them during parsing.

Change-Id: I872c06f402c35112194ba77c3d6aee78e22547cb
Signed-off-by: Arne Schwabe 
---
 Changes.rst   |  4 
 src/openvpn/comp.c| 29 +
 src/openvpn/comp.h|  8 
 src/openvpn/init.c|  8 
 src/openvpn/multi.c   |  8 
 src/openvpn/options.c | 27 ---
 6 files changed, 61 insertions(+), 23 deletions(-)

diff --git a/Changes.rst b/Changes.rst
index dedb97ee4..77bcef266 100644
--- a/Changes.rst
+++ b/Changes.rst
@@ -232,6 +232,10 @@ User-visible Changes
 - The ``client-pending-auth`` management command now requires also the
   key id. The management version has been changed to 5 to indicate this change.
 
+- (OpenVPN 2.6.2) A client will now refuse a connection if pushed compression
+  settings will contradict the setting of allow-compression as this almost
+  always results in a non-working connection.
+
 Common errors with OpenSSL 3.0 and OpenVPN 2.6
 --
 Both OpenVPN 2.6 and OpenSSL 3.0 tighten the security considerable, so some
diff --git a/src/openvpn/comp.c b/src/openvpn/comp.c
index 3b8d78996..d6d8029da 100644
--- a/src/openvpn/comp.c
+++ b/src/openvpn/comp.c
@@ -157,4 +157,33 @@ comp_generate_peer_info_string(const struct 
compress_options *opt, struct buffer
 }
 }
 
+bool
+check_compression_settings_valid(struct compress_options *info, int msglevel)
+{
+if ((info->flags & COMP_F_ALLOW_STUB_ONLY) && comp_non_stub_enabled(info))
+{
+msg(msglevel, "Compression is not allowed since allow-compression is "
+"set to 'no'");
+return false;
+}
+#ifndef ENABLE_LZ4
+if (info->alg == COMP_ALGV2_LZ4 || info->alg == COMP_ALG_LZ4)
+{
+msg(msglevel, "OpenVPN is compiled without LZ4 support. Requested "
+"compression cannot be enabled.");
+return false;
+}
+#endif
+#ifndef ENABLE_LZO
+if (info->alg == COMP_ALG_LZO || info->alg == COMP_ALG_LZ4)
+{
+msg(msglevel, "OpenVPN is compiled without LZO support. Requested "
+"compression cannot be enabled.");
+return false;
+}
+#endif
+return true;
+}
+
+
 #endif /* USE_COMP */
diff --git a/src/openvpn/comp.h b/src/openvpn/comp.h
index 685f40391..8636727ab 100644
--- a/src/openvpn/comp.h
+++ b/src/openvpn/comp.h
@@ -196,5 +196,13 @@ comp_non_stub_enabled(const struct compress_options *info)
&& info->alg != COMP_ALG_UNDEF;
 }
 
+/**
+ * Checks if the compression settings are valid. Takes into account the
+ * flags of allow-compression and also the whether algorithms are compiled
+ * in
+ */
+bool
+check_compression_settings_valid(struct compress_options *info, int msglevel);
+
 #endif /* USE_COMP */
 #endif /* ifndef OPENVPN_COMP_H */
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 3a6f624fd..14859499d 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2637,6 +2637,14 @@ do_deferred_options(struct context *c, const unsigned 
int found)
 #ifdef USE_COMP
 if (found & OPT_P_COMP)
 {
+if (!check_compression_settings_valid(>options.comp, D_PUSH_ERRORS))
+{
+msg(D_PUSH_ERRORS, "OPTIONS ERROR: server pushed compression "
+"settings that are not allowed and will result "
+"in a non-working connection. "
+"See also allow-compression in the manual.");
+return false;
+}
 msg(D_PUSH_DEBUG, "OPTIONS IMPORT: compression parms modified");
 comp_uninit(c->c2.comp_context);
 c->c2.comp_context = comp_init(>options.comp);
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 1480bf477..ac090ef5a 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -2766,6 +2766,14 @@ multi_connection_established(struct multi_context *m, 
struct multi_instance *mi)
 cc_succeeded = false;
 }
 
+#ifdef USE_COMP
+if (!check_compression_settings_valid(>context.options.comp, 
D_MULTI_ERRORS))
+{
+msg(D_MULTI_ERRORS, "MULTI: client has been rejected due to invalid 
compression options");
+cc_succeeded = false;
+}
+#endif
+
 if (cc_succeeded)
 {
 multi_client_connect_late_setup(m, mi, *option_types_found);
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 2bed4ce99..435e1ca9e 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -3779,6 +3779,9 @@ options_postprocess_mutate(struct options *o, struct 
env_set *es)
 /* this depends on o->windows_driver, which is set above */
 options_postprocess_mutate_invariant(o);
 
+/* check that compression settings in the options are okay */
+check

[Openvpn-devel] [PATCH v3 1/4] Simplify --compress parsing in options.c

[Arne Schwabe]

This removes a level of identation and make the "stub" condition
easier to see.

Change-Id: Iae47b191f522625f81eedd3a237b272cb7374d90
Signed-off-by: Arne Schwabe 
---
 src/openvpn/options.c | 87 +--
 1 file changed, 43 insertions(+), 44 deletions(-)

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 64a8250b2..2bed4ce99 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -8458,60 +8458,59 @@ add_option(struct options *options,
 else if (streq(p[0], "compress") && !p[2])
 {
 VERIFY_PERMISSION(OPT_P_COMP);
+const char *alg = "stub";
 if (p[1])
 {
-if (streq(p[1], "stub"))
-{
-options->comp.alg = COMP_ALG_STUB;
-options->comp.flags |= 
(COMP_F_SWAP|COMP_F_ADVERTISE_STUBS_ONLY);
-}
-else if (streq(p[1], "stub-v2"))
-{
-options->comp.alg = COMP_ALGV2_UNCOMPRESSED;
-options->comp.flags |= COMP_F_ADVERTISE_STUBS_ONLY;
-}
-else if (streq(p[1], "migrate"))
-{
-options->comp.alg = COMP_ALG_UNDEF;
-options->comp.flags = COMP_F_MIGRATE;
+alg = p[1];
+}
 
-}
-else if (options->comp.flags & COMP_F_ALLOW_STUB_ONLY)
-{
-/* Also printed on a push to hint at configuration problems */
-msg(msglevel, "Cannot set compress to '%s', "
-"allow-compression is set to 'no'", p[1]);
-goto err;
-}
+if (streq(alg, "stub"))
+{
+options->comp.alg = COMP_ALG_STUB;
+options->comp.flags |= (COMP_F_SWAP|COMP_F_ADVERTISE_STUBS_ONLY);
+}
+else if (streq(alg, "stub-v2"))
+{
+options->comp.alg = COMP_ALGV2_UNCOMPRESSED;
+options->comp.flags |= COMP_F_ADVERTISE_STUBS_ONLY;
+}
+else if (streq(alg, "migrate"))
+{
+options->comp.alg = COMP_ALG_UNDEF;
+options->comp.flags = COMP_F_MIGRATE;
+
+}
+else if (options->comp.flags & COMP_F_ALLOW_STUB_ONLY)
+{
+/* Also printed on a push to hint at configuration problems */
+msg(msglevel, "Cannot set compress to '%s', "
+"allow-compression is set to 'no'", alg);
+goto err;
+}
 #if defined(ENABLE_LZO)
-else if (streq(p[1], "lzo"))
-{
-options->comp.alg = COMP_ALG_LZO;
-options->comp.flags &= ~(COMP_F_ADAPTIVE | COMP_F_SWAP);
-}
+else if (streq(alg, "lzo"))
+{
+options->comp.alg = COMP_ALG_LZO;
+options->comp.flags &= ~(COMP_F_ADAPTIVE | COMP_F_SWAP);
+}
 #endif
 #if defined(ENABLE_LZ4)
-else if (streq(p[1], "lz4"))
-{
-options->comp.alg = COMP_ALG_LZ4;
-options->comp.flags |= COMP_F_SWAP;
-}
-else if (streq(p[1], "lz4-v2"))
-{
-options->comp.alg = COMP_ALGV2_LZ4;
-}
-#endif
-else
-{
-msg(msglevel, "bad comp option: %s", p[1]);
-goto err;
-}
+else if (streq(alg, "lz4"))
+{
+options->comp.alg = COMP_ALG_LZ4;
+options->comp.flags |= COMP_F_SWAP;
+}
+else if (streq(alg, "lz4-v2"))
+{
+options->comp.alg = COMP_ALGV2_LZ4;
 }
+#endif
 else
 {
-options->comp.alg = COMP_ALG_STUB;
-options->comp.flags |= COMP_F_SWAP;
+msg(msglevel, "bad comp option: %s", alg);
+goto err;
 }
+
 show_compression_warning(>comp);
 }
 #endif /* USE_COMP */
-- 
2.37.1 (Apple Git-137.1)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v3 3/4] Add 'allow-compression stub-only' internally for DCO

[Arne Schwabe]

This changes the "no" setting of allow-compression to also refuse framing.
This is important for our DCO implementation as these do not implement framing.

This behaviour surfaced when a commercial VPN provider was pushing
"comp-lzo no" to a client with DCO. While we are technically at fault here
for announcing comp-lzo no support by announcing IV_LZO_STUB=1, the
VPN provider continues to push "comp-lzo no" even in absense of that
flag.

As the new default we default to allow-compression stub-only if a stub option
is found in the config and to allow-compression no otherwise. This ensures
that we only enable DCO when no compression framing is used.

This will now also bail out if the server pushes a compression setting that
we do not support as mismatching compression is almost never a working
connection. In my lz4-v2 and lzo-v2 you might have a connection that works
mostly but some packets will be dropped since they compressed which is not
desirable either since it becomes very hard to debug.

Patch v2: bail out if server pushes an unsupported method. Also include this
  bail out logic when OpenVPN is compiled without compression support.

Patch v3: always parse all compression option and move logic to check method

Change-Id: Ibd0c77af24e2214b3055d585dc23a4b06dccd414
Signed-off-by: Arne Schwabe 
---
 doc/man-sections/protocol-options.rst |  4 ++-
 src/openvpn/comp.c| 47 ++-
 src/openvpn/comp.h|  2 +-
 src/openvpn/options.c | 18 --
 4 files changed, 50 insertions(+), 21 deletions(-)

diff --git a/doc/man-sections/protocol-options.rst 
b/doc/man-sections/protocol-options.rst
index 248f65cfd..055d08f95 100644
--- a/doc/man-sections/protocol-options.rst
+++ b/doc/man-sections/protocol-options.rst
@@ -25,7 +25,9 @@ configured in a compatible way between both the local and 
remote side.
   compression at the same time is not a feasible option.
 
   :code:`no`  (default)
-  OpenVPN will refuse any non-stub compression.
+  OpenVPN will refuse any compression.  If data-channel offloading
+  is enabled. OpenVPN will additionally also refuse compression
+  framing (stub).
 
   :code:`yes`
   OpenVPN will send and receive compressed packets.
diff --git a/src/openvpn/comp.c b/src/openvpn/comp.c
index d6d8029da..c7a562f5a 100644
--- a/src/openvpn/comp.c
+++ b/src/openvpn/comp.c
@@ -134,36 +134,51 @@ comp_print_stats(const struct compress_context *compctx, 
struct status_output *s
 void
 comp_generate_peer_info_string(const struct compress_options *opt, struct 
buffer *out)
 {
-if (opt)
+if (!opt || opt->flags & COMP_F_ALLOW_NOCOMP_ONLY)
+{
+return;
+}
+
+bool lzo_avail = false;
+if (!(opt->flags & COMP_F_ADVERTISE_STUBS_ONLY))
 {
-bool lzo_avail = false;
-if (!(opt->flags & COMP_F_ADVERTISE_STUBS_ONLY))
-{
 #if defined(ENABLE_LZ4)
-buf_printf(out, "IV_LZ4=1\n");
-buf_printf(out, "IV_LZ4v2=1\n");
+buf_printf(out, "IV_LZ4=1\n");
+buf_printf(out, "IV_LZ4v2=1\n");
 #endif
 #if defined(ENABLE_LZO)
-buf_printf(out, "IV_LZO=1\n");
-lzo_avail = true;
+buf_printf(out, "IV_LZO=1\n");
+lzo_avail = true;
 #endif
-}
-if (!lzo_avail)
-{
-buf_printf(out, "IV_LZO_STUB=1\n");
-}
-buf_printf(out, "IV_COMP_STUB=1\n");
-buf_printf(out, "IV_COMP_STUBv2=1\n");
 }
+if (!lzo_avail)
+{
+buf_printf(out, "IV_LZO_STUB=1\n");
+}
+buf_printf(out, "IV_COMP_STUB=1\n");
+buf_printf(out, "IV_COMP_STUBv2=1\n");
 }
 
 bool
 check_compression_settings_valid(struct compress_options *info, int msglevel)
 {
+/*
+ * We also allow comp-stub-v2 here as it technically allows escaping of
+ * weird mac address and IPv5 protocol but practically always is used
+ * as an way to disable all framing.
+ */
+if (info->alg != COMP_ALGV2_UNCOMPRESSED && info->alg != COMP_ALG_UNDEF
+&& (info->flags & COMP_F_ALLOW_NOCOMP_ONLY))
+{
+msg(msglevel, "Compression or compression stub framing is not allowed "
+"since data-channel offloading is enabled.");
+return false;
+}
+
 if ((info->flags & COMP_F_ALLOW_STUB_ONLY) && comp_non_stub_enabled(info))
 {
 msg(msglevel, "Compression is not allowed since allow-compression is "
-"set to 'no'");
+"set to 'stub-only'");
 return false;
 }
 #ifndef ENABLE_LZ4
diff --git a/src/openvpn/comp.h b/src/openvpn/comp.h
index 8636727ab..71b350d09 100644
--- a/src/openvpn/comp.h
+++ b/src/openvpn/comp.h
@@ -60,7 +60,7 

Re: [Openvpn-devel] [PATCH v4] dco-freebsd: use m->instances[] instead of m->hash

[Arne Schwabe]

Am 23.03.23 um 09:03 schrieb Gert Doering:

From: Antonio Quartulli 

When retrieving the multi_instance of a specific peer,
there is no need to peform a linear search across the
whole m->hash list. We can directly access the needed
object via m->instances[peer-id] in constant time (and
just one line of code).

Adapt the dco-freebsd code to do so.

v4: use "peerid" everywhere as that's what FreeBSD does, change message text

Cc: Kristof Provost 
Change-Id: I8d8af6f872146604a9710edf443db65df48ac3cb
Signed-off-by: Antonio Quartulli 
---
  src/openvpn/dco_freebsd.c | 22 ++
  1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
index 225b3cf8..a334d5d2 100644
--- a/src/openvpn/dco_freebsd.c
+++ b/src/openvpn/dco_freebsd.c
@@ -674,27 +674,17 @@ dco_event_set(dco_context_t *dco, struct event_set *es, 
void *arg)
  static void
  dco_update_peer_stat(struct multi_context *m, uint32_t peerid, const nvlist_t 
*nvl)
  {
-struct hash_element *he;
-struct hash_iterator hi;
  
-hash_iterator_init(m->hash, );

-
-while ((he = hash_iterator_next()))
+if (peerid >= m->max_clients || !m->instances[peerid])
  {
-struct multi_instance *mi = (struct multi_instance *) he->value;
-
-if (mi->context.c2.tls_multi->peer_id != peerid)
-{
-continue;
-}
-
-mi->context.c2.dco_read_bytes = nvlist_get_number(nvl, "in");
-mi->context.c2.dco_write_bytes = nvlist_get_number(nvl, "out");
-
+msg(M_WARN, "dco_update_peer_stat: invalid peer ID %d returned by 
kernel", peerid);
  return;
  }
  
-msg(M_INFO, "Peer %d returned by kernel, but not found locally", peerid);

+struct multi_instance *mi = m->instances[peerid];
+
+mi->context.c2.dco_read_bytes = nvlist_get_number(nvl, "in");
+mi->context.c2.dco_write_bytes = nvlist_get_number(nvl, "out");
  }
  
  int


Acked-By: Arne Schwabe 


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v2] Add 'allow-compression stub-only and refuse framing with 'allow-compression no'

[Arne Schwabe]

This changes the "no" setting of allow-compression to also refuse framing. This
is important for our DCO implementation as these do not implement framing.

This behaviour surfaced when a commercial VPN provider was pushing
"comp-lzo no" to a client with DCO. While we are technically at fault here
for announcing comp-lzo no support by announcing IV_LZO_STUB=1, the
VPN provider continues to push "comp-lzo no" even in absense of that
flag.

As the new default we default to allow-compression stub-only if a stub option
is found in the config and to allow-compression no otherwise. This ensures
that we only enable DCO when no compression framing is used.

This will now also bail out if the server pushes a compression setting that
we do not support as mismatching compression is almost never a working
connection. In my lz4-v2 and lzo-v2 you might have a connection that works
mostly but some packets will be dropped since they compressed which is not
desirable either since it becomes very hard to debug.

Patch v2: bail out if server pushes an unsupported method. Also include this
  bail out logic when OpenVPN is compiled without compression support.

Change-Id: Ieefb501038b06c7520ed105c660a1c79887476f3
Signed-off-by: Arne Schwabe 
---
 Changes.rst   |   6 ++
 doc/man-sections/protocol-options.rst |   3 +
 src/openvpn/comp.c|  32 +++---
 src/openvpn/comp.h|  44 
 src/openvpn/dco.c |   4 +-
 src/openvpn/init.c|  12 ++-
 src/openvpn/options.c | 150 --
 src/openvpn/options.h |   2 -
 8 files changed, 156 insertions(+), 97 deletions(-)

diff --git a/Changes.rst b/Changes.rst
index dedb97ee4..17b4c4d04 100644
--- a/Changes.rst
+++ b/Changes.rst
@@ -232,6 +232,12 @@ User-visible Changes
 - The ``client-pending-auth`` management command now requires also the
   key id. The management version has been changed to 5 to indicate this change.
 
+- (OpenVPN 2.6.2) ``--allow-compression no`` has been changed to not allow
+  compression or compression framing at all now and is the new default.
+  Use ``--allow-compression stub-only`` for the old ``no`` behaviour of OpenVPN
+  2.5 and OpenVPN 2.6.0.
+
+
 Common errors with OpenSSL 3.0 and OpenVPN 2.6
 --
 Both OpenVPN 2.6 and OpenSSL 3.0 tighten the security considerable, so some
diff --git a/doc/man-sections/protocol-options.rst 
b/doc/man-sections/protocol-options.rst
index 248f65cfd..76c323413 100644
--- a/doc/man-sections/protocol-options.rst
+++ b/doc/man-sections/protocol-options.rst
@@ -25,6 +25,9 @@ configured in a compatible way between both the local and 
remote side.
   compression at the same time is not a feasible option.
 
   :code:`no`  (default)
+  OpenVPN will refuse any compression or compression framing (stub).
+
+  :code:`stub-only`
   OpenVPN will refuse any non-stub compression.
 
   :code:`yes`
diff --git a/src/openvpn/comp.c b/src/openvpn/comp.c
index 3b8d78996..c7ec6c7f5 100644
--- a/src/openvpn/comp.c
+++ b/src/openvpn/comp.c
@@ -134,27 +134,29 @@ comp_print_stats(const struct compress_context *compctx, 
struct status_output *s
 void
 comp_generate_peer_info_string(const struct compress_options *opt, struct 
buffer *out)
 {
-if (opt)
+if (!opt || opt->flags & COMP_F_ALLOW_NOCOMP_ONLY)
+{
+return;
+}
+
+bool lzo_avail = false;
+if (!(opt->flags & COMP_F_ADVERTISE_STUBS_ONLY))
 {
-bool lzo_avail = false;
-if (!(opt->flags & COMP_F_ADVERTISE_STUBS_ONLY))
-{
 #if defined(ENABLE_LZ4)
-buf_printf(out, "IV_LZ4=1\n");
-buf_printf(out, "IV_LZ4v2=1\n");
+buf_printf(out, "IV_LZ4=1\n");
+buf_printf(out, "IV_LZ4v2=1\n");
 #endif
 #if defined(ENABLE_LZO)
-buf_printf(out, "IV_LZO=1\n");
-lzo_avail = true;
+buf_printf(out, "IV_LZO=1\n");
+lzo_avail = true;
 #endif
-}
-if (!lzo_avail)
-{
-buf_printf(out, "IV_LZO_STUB=1\n");
-}
-buf_printf(out, "IV_COMP_STUB=1\n");
-buf_printf(out, "IV_COMP_STUBv2=1\n");
 }
+if (!lzo_avail)
+{
+buf_printf(out, "IV_LZO_STUB=1\n");
+}
+buf_printf(out, "IV_COMP_STUB=1\n");
+buf_printf(out, "IV_COMP_STUBv2=1\n");
 }
 
 #endif /* USE_COMP */
diff --git a/src/openvpn/comp.h b/src/openvpn/comp.h
index 685f40391..49b715b10 100644
--- a/src/openvpn/comp.h
+++ b/src/openvpn/comp.h
@@ -28,6 +28,29 @@
 #ifndef OPENVPN_COMP_H
 #define OPENVPN_COMP_H
 
+/* Compression flags */
+#define COMP_F_ADAPTIVE (1<<0) /* COMP_ALG_LZO only */
+#define COMP_F_ALLOW_COMPRESS   (1<<1) /* not only downli

Re: [Openvpn-devel] [PATCH] dco_freebsd: use m->instances[] instead of m->hash

[Arne Schwabe]

Am 22.03.23 um 00:10 schrieb Antonio Quartulli:

When retrieving the multi_instance of a specific peer,
there is no need to peform a linear search across the
whole m->hash list. We can directly access the needed
object via m->instances[peer-id] in constant time (and
just one line of code).

Adapt the dco-freebsd code to do so.



Acked-By: Arne Schwabe 

Arne



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Add missing check for nl_socket_alloc failure

[Arne Schwabe]

Am 14.02.23 um 15:01 schrieb Gert Doering:

Hi,

On Tue, Feb 14, 2023 at 02:56:58PM +0100, Arne Schwabe wrote:

  resolve_ovpn_netlink_id(int msglevel)
  {
-int ret;
  struct nl_sock *nl_sock = nl_socket_alloc();
  
-ret = genl_connect(nl_sock);

+if (!nl_sock)
+{
+msg(msglevel, "Allocating net link socket failed");
+}


... shouldn't we abort in this case, aka, M_FATAL (or "return -errno")?


I don't think so. This function is called from two code paths:

- dco_available: I don't think if detection of dco fails, we should quit 
openvpn completely.


- ovpn_dco_init_netlink: Here we already call it with M_ERR, which is 
  (M_FATAL | M_ERRNO)


Arne



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v2] Improve description of compat-mode

[Arne Schwabe]

Explicitly say that the version specified is the one of the peer and not
the version we try to emulate.

Patch v2: Improve grammar.
Change-Id: I3bd27a8d34d8cb4896a3b78508b7d16911571543

Change-Id: If4fb45b3426f5e0dbe6c87d5bd05681b9d733827
Signed-off-by: Arne Schwabe 
---
 doc/man-sections/generic-options.rst | 22 --
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/doc/man-sections/generic-options.rst 
b/doc/man-sections/generic-options.rst
index c827651d6..97e1b5aa6 100644
--- a/doc/man-sections/generic-options.rst
+++ b/doc/man-sections/generic-options.rst
@@ -53,10 +53,17 @@ which mode OpenVPN is configured as.
   need for /dev/urandom to be available.
 
 --compat-mode version
-  This option provides a way to alter the default of OpenVPN to be more
-  compatible with the version ``version`` specified. All of the changes
-  this option does can also be achieved using individual configuration
-  options.
+  This option provides a convenient way to alter the defaults of OpenVPN
+  to be more compatible with the version ``version`` specified. All of
+  the changes this option applies can also be achieved using individual
+  configuration options.
+
+  The version specified with this option is the version of OpenVPN peer
+  OpenVPN should try to be compatible with. In general OpenVPN should be
+  compatible with the last two previous version without this option. E.g.
+  OpenVPN 2.6.0 should be compatible with 2.5.x and 2.4.x without this option.
+  However, there might be some edge cases that still require this option even
+  in these cases.
 
   Note: Using this option reverts defaults to no longer recommended
   values and should be avoided if possible.
@@ -67,12 +74,15 @@ which mode OpenVPN is configured as.
   - 2.5.x or lower: ``--allow-compression asym`` is automatically added
 to the configuration if no other compression options are present.
   - 2.4.x or lower: The cipher in ``--cipher`` is appended to
-``--data-ciphers``
+``--data-ciphers``.
   - 2.3.x or lower: ``--data-cipher-fallback`` is automatically added with
-the same cipher as ``--cipher``
+the same cipher as ``--cipher``.
   - 2.3.6 or lower: ``--tls-version-min 1.0`` is added to the configuration
 when ``--tls-version-min`` is not explicitly set.
 
+  If not required, this is option should be avoided. Setting this option can
+  lower security or disable features like data-channel offloading.
+
 --config file
   Load additional config options from ``file`` where each line corresponds
   to one command line option, but with the leading :code:`--` removed.
-- 
2.37.1 (Apple Git-137.1)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] using OpenSSL3 API for EVP PKEY type name reporting

[Arne Schwabe]

Am 19.03.23 um 08:54 schrieb Michael Baentsch:

Signed-off-by: Michael Baentsch 


Acked-By: Arne Schwabe 

Thanks. We had a discussion/review round on gihtub before this. 
Basically the problem is that trying to print the algorithm
for algorithms that are not part of the old OpenSSL 1.x API (like a pq 
algorithm from a provider) results in putting an error on the OpenSSL 
stack and that has than bad consequences.


Arne


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v3] Fix memory leaks in HMAC initial packet id

[Arne Schwabe]

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, 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 as that is also more consistent.

Also packet_id_init will allocated a buffer with malloc and not using a
gc_arena, so we need to manually also 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 
---
 src/openvpn/init.c  |  3 +++
 src/openvpn/ssl.c   | 11 ++
 src/openvpn/ssl.h   |  6 ++
 src/openvpn/ssl_pkt.c   |  8 +--
 src/openvpn/ssl_pkt.h   |  2 +-
 tests/unit_tests/openvpn/test_pkt.c | 33 +++--
 6 files changed, 49 insertions(+), 14 deletions(-)

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 124ac76bd..fa2681dc7 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -3483,6 +3483,7 @@ do_init_frame_tls(struct context *c)
 frame_print(>c2.tls_auth_standalone->frame, D_MTU_INFO,
 "TLS-Auth MTU parms");
 c->c2.tls_auth_standalone->tls_wrap.work = 
alloc_buf_gc(BUF_SIZE(>c2.frame), >c2.gc);
+c->c2.tls_auth_standalone->workbuf = 
alloc_buf_gc(BUF_SIZE(>c2.frame), >c2.gc);
 }
 }
 
@@ -3881,6 +3882,8 @@ do_close_tls(struct context *c)
 md_ctx_cleanup(c->c2.pulled_options_state);
 md_ctx_free(c->c2.pulled_options_state);
 }
+
+tls_auth_standalone_free(c->c2.tls_auth_standalone);
 }
 
 /*
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 78cec90a1..fe6390fad 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -1361,6 +1361,17 @@ tls_auth_standalone_init(struct tls_options *tls_options,
 return tas;
 }
 
+void
+tls_auth_standalone_free(struct tls_auth_standalone *tas)
+{
+if (!tas)
+{
+return;
+}
+
+packet_id_free(>tls_wrap.opt.packet_id);
+}
+
 /*
  * Set local and remote option compatibility strings.
  * Used to verify compatibility of local and remote option
diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index 58ff4b9b4..a050cd5c9 100644
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -180,6 +180,12 @@ void tls_multi_init_finalize(struct tls_multi *multi, int 
tls_mtu);
 struct tls_auth_standalone *tls_auth_standalone_init(struct tls_options 
*tls_options,
  struct gc_arena *gc);
 
+/**
+ * Frees a standalone tls-auth verification object.
+ * @param tas   the object to free. May be NULL.
+ */
+void tls_auth_standalone_free(struct tls_auth_standalone *tas);
+
 /*
  * Setups the control channel frame size parameters from the data channel
  * parameters
diff --git a/src/openvpn/ssl_pkt.c b/src/openvpn/ssl_pkt.c
index 17a7f8917..8b3391e76 100644
--- a/src/openvpn/ssl_pkt.c
+++ b/src/openvpn/ssl_pkt.c
@@ -434,7 +434,10 @@ tls_reset_standalone(struct tls_wrap_ctx *ctx,
  uint8_t header,
  bool request_resend_wkc)
 {
-struct buffer buf = alloc_buf(tas->frame.buf.payload_size);
+/* Copy buffer here to point at the same data but allow tls_wrap_control
+ * to potentially change buf to point to another buffer without
+ * modifying the buffer in tas */
+struct buffer buf = tas->workbuf;
 ASSERT(buf_init(, tas->frame.buf.headroom));
 
 /* Reliable ACK structure */
@@ -461,7 +464,8 @@ tls_reset_standalone(struct tls_wrap_ctx *ctx,
 buf_write_u16(, EARLY_NEG_FLAG_RESEND_WKC);
 }
 
-/* Add tls-auth/tls-crypt wrapping, this might replace buf */
+/* Add tls-auth/tls-crypt wrapping, this might replace buf with
+ * ctx->work */
 tls_wrap_control(ctx, header, , own_sid);
 
 return buf;
diff --git a/src/openvpn/ssl_pkt.h b/src/openvpn/ssl_pkt.h
index ec7b48daf..ef4852e5d 100644
--- a/src/openvpn/ssl_pkt.h
+++ b/src/openvpn/ssl_pkt.h
@@ -77,6 +77,7 @@
 struct tls_auth_standalone
 {
 struct tls_wrap_ctx tls_wrap;
+struct buffer workbuf;
 struct frame frame;
 };
 
@@ -220,7 +221,6 @@ read_control_auth(struct buffer *buf,
  * This function creates a reset packet using the information
  * from the tls pre decrypt state.
  *
- * The returned buf needs to be free with \c free_buf
  */
 struct buffer
 tls_reset_standalone(struct tls_wrap_ctx *ctx,
diff --git a/tests/unit_tests/openvpn/test_pkt.c 
b/tests/unit_tests/openvpn/test_pkt.c
index f11e52a11..736f13178 100644
--- a/tests/unit_tests/openvpn/test_pkt.c
+++ b/tests/unit_tests/openvpn/test_pkt.c
@@ -203,6 +203,7 @@ init_tas_auth(int key_directi

[Openvpn-devel] [PATCH v2] Fix memory leaks in HMAC initial packet id

[Arne Schwabe]

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, 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 as that is also more consistent.

Also packet_id_init will allocated a buffer with malloc and not using a
gc_arena, so we need to manually also free it.

Patch v2: add missing deallocations in unit tests of the new workbuf

Found-By: clang with asan
Signed-off-by: Arne Schwabe 
---
 src/openvpn/init.c  |  3 +++
 src/openvpn/ssl.c   | 12 +++
 src/openvpn/ssl.h   |  6 ++
 src/openvpn/ssl_pkt.c   |  8 +--
 src/openvpn/ssl_pkt.h   |  2 +-
 tests/unit_tests/openvpn/test_pkt.c | 33 +++--
 6 files changed, 50 insertions(+), 14 deletions(-)

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 124ac76bd..fa2681dc7 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -3483,6 +3483,7 @@ do_init_frame_tls(struct context *c)
 frame_print(>c2.tls_auth_standalone->frame, D_MTU_INFO,
 "TLS-Auth MTU parms");
 c->c2.tls_auth_standalone->tls_wrap.work = 
alloc_buf_gc(BUF_SIZE(>c2.frame), >c2.gc);
+c->c2.tls_auth_standalone->workbuf = 
alloc_buf_gc(BUF_SIZE(>c2.frame), >c2.gc);
 }
 }
 
@@ -3881,6 +3882,8 @@ do_close_tls(struct context *c)
 md_ctx_cleanup(c->c2.pulled_options_state);
 md_ctx_free(c->c2.pulled_options_state);
 }
+
+tls_auth_standalone_free(c->c2.tls_auth_standalone);
 }
 
 /*
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 78cec90a1..f2331f712 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -1358,9 +1358,21 @@ tls_auth_standalone_init(struct tls_options *tls_options,
 packet_id_init(>tls_wrap.opt.packet_id, tls_options->replay_window,
tls_options->replay_time, "TAS", 0);
 
+tas->workbuf = alloc_buf_gc(tas->frame.buf.payload_size, gc);
 return tas;
 }
 
+void
+tls_auth_standalone_free(struct tls_auth_standalone *tas)
+{
+if (!tas)
+{
+return;
+}
+
+packet_id_free(>tls_wrap.opt.packet_id);
+}
+
 /*
  * Set local and remote option compatibility strings.
  * Used to verify compatibility of local and remote option
diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index 58ff4b9b4..a050cd5c9 100644
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -180,6 +180,12 @@ void tls_multi_init_finalize(struct tls_multi *multi, int 
tls_mtu);
 struct tls_auth_standalone *tls_auth_standalone_init(struct tls_options 
*tls_options,
  struct gc_arena *gc);
 
+/**
+ * Frees a standalone tls-auth verification object.
+ * @param tas   the object to free. May be NULL.
+ */
+void tls_auth_standalone_free(struct tls_auth_standalone *tas);
+
 /*
  * Setups the control channel frame size parameters from the data channel
  * parameters
diff --git a/src/openvpn/ssl_pkt.c b/src/openvpn/ssl_pkt.c
index 17a7f8917..8b3391e76 100644
--- a/src/openvpn/ssl_pkt.c
+++ b/src/openvpn/ssl_pkt.c
@@ -434,7 +434,10 @@ tls_reset_standalone(struct tls_wrap_ctx *ctx,
  uint8_t header,
  bool request_resend_wkc)
 {
-struct buffer buf = alloc_buf(tas->frame.buf.payload_size);
+/* Copy buffer here to point at the same data but allow tls_wrap_control
+ * to potentially change buf to point to another buffer without
+ * modifying the buffer in tas */
+struct buffer buf = tas->workbuf;
 ASSERT(buf_init(, tas->frame.buf.headroom));
 
 /* Reliable ACK structure */
@@ -461,7 +464,8 @@ tls_reset_standalone(struct tls_wrap_ctx *ctx,
 buf_write_u16(, EARLY_NEG_FLAG_RESEND_WKC);
 }
 
-/* Add tls-auth/tls-crypt wrapping, this might replace buf */
+/* Add tls-auth/tls-crypt wrapping, this might replace buf with
+ * ctx->work */
 tls_wrap_control(ctx, header, , own_sid);
 
 return buf;
diff --git a/src/openvpn/ssl_pkt.h b/src/openvpn/ssl_pkt.h
index ec7b48daf..ef4852e5d 100644
--- a/src/openvpn/ssl_pkt.h
+++ b/src/openvpn/ssl_pkt.h
@@ -77,6 +77,7 @@
 struct tls_auth_standalone
 {
 struct tls_wrap_ctx tls_wrap;
+struct buffer workbuf;
 struct frame frame;
 };
 
@@ -220,7 +221,6 @@ read_control_auth(struct buffer *buf,
  * This function creates a reset packet using the information
  * from the tls pre decrypt state.
  *
- * The returned buf needs to be free with \c free_buf
  */
 struct buffer
 tls_reset_standalone(struct tls_wrap_ctx *ctx,
diff --git a/tests/unit_tests/openvpn/test_pkt.c 
b/tests/unit_tests/openvpn/test_pkt.c
index f11e52a11..736f13178 100644
--- a/tests/unit_tests/openvpn/test_pkt.c
+++ 

Re: [Openvpn-devel] [PATCH v2] Bugfix: Convert ECDSA signature form pkcs11-helper to DER encoded form

[Arne Schwabe]

Am 14.03.23 um 13:21 schrieb selva.n...@gmail.com:

From: Selva Nair 

- 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.


Even though the commit is quite long, it is mostly moving the 
ecdsa_bin2der function into xkey_helper.c. While I have not tested it 
myself the code changes make sense and look good and we got a positive 
test report.


Acked-By: Arne Schwabe 





___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 2/2] Fix memory leaks in dco open tun

[Arne Schwabe]

The open_tun_dco_generic already allocates the actual_name string, this
shadows the allocation in the FreeBSD/Linux specific methods.

Found-By: clang with asan
Change-Id: I51f5fcfff4e5f8203fdb9aec0245cfccd17043cc
Signed-off-by: Arne Schwabe 
---
 src/openvpn/dco_freebsd.c | 1 -
 src/openvpn/dco_linux.c   | 1 -
 2 files changed, 2 deletions(-)

diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
index ecca2a076..225b3cf88 100644
--- a/src/openvpn/dco_freebsd.c
+++ b/src/openvpn/dco_freebsd.c
@@ -232,7 +232,6 @@ create_interface(struct tuntap *tt, const char *dev)
 }
 
 snprintf(tt->dco.ifname, IFNAMSIZ, "%s", ifr.ifr_data);
-tt->actual_name = string_alloc(tt->dco.ifname, NULL);
 
 /* see "Interface Flags" in ifnet(9) */
 int i = IFF_POINTOPOINT | IFF_MULTICAST;
diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index b2fdbf53f..e5cea3c71 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -457,7 +457,6 @@ open_tun_dco(struct tuntap *tt, openvpn_net_ctx_t *ctx, 
const char *dev)
 msg(M_FATAL, "DCO: cannot retrieve ifindex for interface %s", dev);
 }
 
-tt->actual_name = string_alloc(dev, NULL);
 tt->dco.dco_message_peer_id = -1;
 
 ovpn_dco_register(>dco);
-- 
2.37.1 (Apple Git-137.1)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 1/2] Fix memory leaks in HMAC initial packet id

[Arne Schwabe]

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, 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 as that is also more consistent.

Also packet_id_init will allocated a buffer with malloc and not using a
gc_arena, so we need to manually also free it.

Found-By: clang with asan
Change-Id: I0cff44f79ee7e3bcf7b5981fc94f469c15f21af3
Signed-off-by: Arne Schwabe 
---
 src/openvpn/init.c  |  3 +++
 src/openvpn/ssl.c   | 12 
 src/openvpn/ssl.h   |  6 ++
 src/openvpn/ssl_pkt.c   |  8 ++--
 src/openvpn/ssl_pkt.h   |  2 +-
 tests/unit_tests/openvpn/test_pkt.c |  2 ++
 6 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 124ac76bd..fa2681dc7 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -3483,6 +3483,7 @@ do_init_frame_tls(struct context *c)
 frame_print(>c2.tls_auth_standalone->frame, D_MTU_INFO,
 "TLS-Auth MTU parms");
 c->c2.tls_auth_standalone->tls_wrap.work = 
alloc_buf_gc(BUF_SIZE(>c2.frame), >c2.gc);
+c->c2.tls_auth_standalone->workbuf = 
alloc_buf_gc(BUF_SIZE(>c2.frame), >c2.gc);
 }
 }
 
@@ -3881,6 +3882,8 @@ do_close_tls(struct context *c)
 md_ctx_cleanup(c->c2.pulled_options_state);
 md_ctx_free(c->c2.pulled_options_state);
 }
+
+tls_auth_standalone_free(c->c2.tls_auth_standalone);
 }
 
 /*
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 78cec90a1..f2331f712 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -1358,9 +1358,21 @@ tls_auth_standalone_init(struct tls_options *tls_options,
 packet_id_init(>tls_wrap.opt.packet_id, tls_options->replay_window,
tls_options->replay_time, "TAS", 0);
 
+tas->workbuf = alloc_buf_gc(tas->frame.buf.payload_size, gc);
 return tas;
 }
 
+void
+tls_auth_standalone_free(struct tls_auth_standalone *tas)
+{
+if (!tas)
+{
+return;
+}
+
+packet_id_free(>tls_wrap.opt.packet_id);
+}
+
 /*
  * Set local and remote option compatibility strings.
  * Used to verify compatibility of local and remote option
diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index 58ff4b9b4..a050cd5c9 100644
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -180,6 +180,12 @@ void tls_multi_init_finalize(struct tls_multi *multi, int 
tls_mtu);
 struct tls_auth_standalone *tls_auth_standalone_init(struct tls_options 
*tls_options,
  struct gc_arena *gc);
 
+/**
+ * Frees a standalone tls-auth verification object.
+ * @param tas   the object to free. May be NULL.
+ */
+void tls_auth_standalone_free(struct tls_auth_standalone *tas);
+
 /*
  * Setups the control channel frame size parameters from the data channel
  * parameters
diff --git a/src/openvpn/ssl_pkt.c b/src/openvpn/ssl_pkt.c
index 17a7f8917..8b3391e76 100644
--- a/src/openvpn/ssl_pkt.c
+++ b/src/openvpn/ssl_pkt.c
@@ -434,7 +434,10 @@ tls_reset_standalone(struct tls_wrap_ctx *ctx,
  uint8_t header,
  bool request_resend_wkc)
 {
-struct buffer buf = alloc_buf(tas->frame.buf.payload_size);
+/* Copy buffer here to point at the same data but allow tls_wrap_control
+ * to potentially change buf to point to another buffer without
+ * modifying the buffer in tas */
+struct buffer buf = tas->workbuf;
 ASSERT(buf_init(, tas->frame.buf.headroom));
 
 /* Reliable ACK structure */
@@ -461,7 +464,8 @@ tls_reset_standalone(struct tls_wrap_ctx *ctx,
 buf_write_u16(, EARLY_NEG_FLAG_RESEND_WKC);
 }
 
-/* Add tls-auth/tls-crypt wrapping, this might replace buf */
+/* Add tls-auth/tls-crypt wrapping, this might replace buf with
+ * ctx->work */
 tls_wrap_control(ctx, header, , own_sid);
 
 return buf;
diff --git a/src/openvpn/ssl_pkt.h b/src/openvpn/ssl_pkt.h
index ec7b48daf..ef4852e5d 100644
--- a/src/openvpn/ssl_pkt.h
+++ b/src/openvpn/ssl_pkt.h
@@ -77,6 +77,7 @@
 struct tls_auth_standalone
 {
 struct tls_wrap_ctx tls_wrap;
+struct buffer workbuf;
 struct frame frame;
 };
 
@@ -220,7 +221,6 @@ read_control_auth(struct buffer *buf,
  * This function creates a reset packet using the information
  * from the tls pre decrypt state.
  *
- * The returned buf needs to be free with \c free_buf
  */
 struct buffer
 tls_reset_standalone(struct tls_wrap_ctx *ctx,
diff --git a/tests/unit_tests/openvpn/test_pkt.c 
b/tests/unit_tests/openvpn/test_pkt.c
index f11e52a11..8574afc6a 100644
--- a/tests/unit_tests/openvpn/test_pkt.c
+++ b/tests/unit_tests/openvpn/test_pkt.c
@@ -563,6 +563,7 @@

[Openvpn-devel] [PATCH v3] Fix memory leaks in HMAC initial packet id and dco open tun

[Arne Schwabe]

The open_tun_dco_generic already allocates the actual_name string, this
shadows the allocation in the FreeBSD/Linux specific methods.

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, 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 as that is also more consistent.

Found-By: clang with asan

Patch v2: rebase. Include linux bits accidentially forgotten.
Patch v3: fix also tls-crypt.

Change-Id: I3c344af047abe94c0178bde1781eb450f10d157d
Signed-off-by: Arne Schwabe 
---
 src/openvpn/dco_freebsd.c |  1 -
 src/openvpn/dco_linux.c   |  1 -
 src/openvpn/init.c|  3 +++
 src/openvpn/ssl.c | 12 
 src/openvpn/ssl.h |  6 ++
 src/openvpn/ssl_pkt.c |  8 ++--
 src/openvpn/ssl_pkt.h |  2 +-
 7 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
index ecca2a076..225b3cf88 100644
--- a/src/openvpn/dco_freebsd.c
+++ b/src/openvpn/dco_freebsd.c
@@ -232,7 +232,6 @@ create_interface(struct tuntap *tt, const char *dev)
 }
 
 snprintf(tt->dco.ifname, IFNAMSIZ, "%s", ifr.ifr_data);
-tt->actual_name = string_alloc(tt->dco.ifname, NULL);
 
 /* see "Interface Flags" in ifnet(9) */
 int i = IFF_POINTOPOINT | IFF_MULTICAST;
diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index b2fdbf53f..e5cea3c71 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -457,7 +457,6 @@ open_tun_dco(struct tuntap *tt, openvpn_net_ctx_t *ctx, 
const char *dev)
 msg(M_FATAL, "DCO: cannot retrieve ifindex for interface %s", dev);
 }
 
-tt->actual_name = string_alloc(dev, NULL);
 tt->dco.dco_message_peer_id = -1;
 
 ovpn_dco_register(>dco);
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 124ac76bd..fa2681dc7 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -3483,6 +3483,7 @@ do_init_frame_tls(struct context *c)
 frame_print(>c2.tls_auth_standalone->frame, D_MTU_INFO,
 "TLS-Auth MTU parms");
 c->c2.tls_auth_standalone->tls_wrap.work = 
alloc_buf_gc(BUF_SIZE(>c2.frame), >c2.gc);
+c->c2.tls_auth_standalone->workbuf = 
alloc_buf_gc(BUF_SIZE(>c2.frame), >c2.gc);
 }
 }
 
@@ -3881,6 +3882,8 @@ do_close_tls(struct context *c)
 md_ctx_cleanup(c->c2.pulled_options_state);
 md_ctx_free(c->c2.pulled_options_state);
 }
+
+tls_auth_standalone_free(c->c2.tls_auth_standalone);
 }
 
 /*
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 78cec90a1..f2331f712 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -1358,9 +1358,21 @@ tls_auth_standalone_init(struct tls_options *tls_options,
 packet_id_init(>tls_wrap.opt.packet_id, tls_options->replay_window,
tls_options->replay_time, "TAS", 0);
 
+tas->workbuf = alloc_buf_gc(tas->frame.buf.payload_size, gc);
 return tas;
 }
 
+void
+tls_auth_standalone_free(struct tls_auth_standalone *tas)
+{
+if (!tas)
+{
+return;
+}
+
+packet_id_free(>tls_wrap.opt.packet_id);
+}
+
 /*
  * Set local and remote option compatibility strings.
  * Used to verify compatibility of local and remote option
diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index 58ff4b9b4..a050cd5c9 100644
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -180,6 +180,12 @@ void tls_multi_init_finalize(struct tls_multi *multi, int 
tls_mtu);
 struct tls_auth_standalone *tls_auth_standalone_init(struct tls_options 
*tls_options,
  struct gc_arena *gc);
 
+/**
+ * Frees a standalone tls-auth verification object.
+ * @param tas   the object to free. May be NULL.
+ */
+void tls_auth_standalone_free(struct tls_auth_standalone *tas);
+
 /*
  * Setups the control channel frame size parameters from the data channel
  * parameters
diff --git a/src/openvpn/ssl_pkt.c b/src/openvpn/ssl_pkt.c
index 17a7f8917..8b3391e76 100644
--- a/src/openvpn/ssl_pkt.c
+++ b/src/openvpn/ssl_pkt.c
@@ -434,7 +434,10 @@ tls_reset_standalone(struct tls_wrap_ctx *ctx,
  uint8_t header,
  bool request_resend_wkc)
 {
-struct buffer buf = alloc_buf(tas->frame.buf.payload_size);
+/* Copy buffer here to point at the same data but allow tls_wrap_control
+ * to potentially change buf to point to another buffer without
+ * modifying the buffer in tas */
+struct buffer buf = tas->workbuf;
 ASSERT(buf_init(, tas->frame.buf.headroom));
 
 /* Reliable ACK structure */
@@ -461,7 +464,8 @@ tls_reset_standalone(struct tls_wrap_ctx *ctx,
 buf_write_u16(, EARLY_NEG

[Openvpn-devel] [PATCH v2] Fix memory leaks in HMAC initial packet id and dco open tun

[Arne Schwabe]

The open_tun_dco_generic already allocates the actual_name string, this
shadows the allocation in the FreeBSD/Linux specific methods.

The HMAC leaks are just forgotten frees/deinitialisations.

Found-By: clang with asan

Patch v2: rebase. Include linux bits accidentially forgotten.

Change-Id: I3c344af047abe94c0178bde1781eb450f10d157d
Signed-off-by: Arne Schwabe 
---
 src/openvpn/dco_freebsd.c |  1 -
 src/openvpn/dco_linux.c   |  1 -
 src/openvpn/init.c|  2 ++
 src/openvpn/mudp.c|  1 +
 src/openvpn/ssl.c | 11 +++
 src/openvpn/ssl.h |  6 ++
 6 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
index 92de5f04b..e605f2a9d 100644
--- a/src/openvpn/dco_freebsd.c
+++ b/src/openvpn/dco_freebsd.c
@@ -230,7 +230,6 @@ create_interface(struct tuntap *tt, const char *dev)
 }
 
 snprintf(tt->dco.ifname, IFNAMSIZ, "%s", ifr.ifr_data);
-tt->actual_name = string_alloc(tt->dco.ifname, NULL);
 
 /* see "Interface Flags" in ifnet(9) */
 int i = IFF_POINTOPOINT | IFF_MULTICAST;
diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 2b349529f..0f5fc48d9 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -457,7 +457,6 @@ open_tun_dco(struct tuntap *tt, openvpn_net_ctx_t *ctx, 
const char *dev)
 msg(M_FATAL, "DCO: cannot retrieve ifindex for interface %s", dev);
 }
 
-tt->actual_name = string_alloc(dev, NULL);
 tt->dco.dco_message_peer_id = -1;
 
 ovpn_dco_register(>dco);
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 124ac76bd..e59edd742 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -3881,6 +3881,8 @@ do_close_tls(struct context *c)
 md_ctx_cleanup(c->c2.pulled_options_state);
 md_ctx_free(c->c2.pulled_options_state);
 }
+
+tls_auth_standalone_free(c->c2.tls_auth_standalone);
 }
 
 /*
diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
index 8698aefc8..813160639 100644
--- a/src/openvpn/mudp.c
+++ b/src/openvpn/mudp.c
@@ -61,6 +61,7 @@ send_hmac_reset_packet(struct multi_context *m,
 m->hmac_reply = c->c2.buffers->aux_buf;
 m->hmac_reply_dest = >top.c2.from;
 msg(D_MULTI_DEBUG, "Reset packet from client, sending HMAC based reset 
challenge");
+free_buf();
 }
 
 
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 78cec90a1..fe6390fad 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -1361,6 +1361,17 @@ tls_auth_standalone_init(struct tls_options *tls_options,
 return tas;
 }
 
+void
+tls_auth_standalone_free(struct tls_auth_standalone *tas)
+{
+if (!tas)
+{
+return;
+}
+
+packet_id_free(>tls_wrap.opt.packet_id);
+}
+
 /*
  * Set local and remote option compatibility strings.
  * Used to verify compatibility of local and remote option
diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index 58ff4b9b4..a050cd5c9 100644
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -180,6 +180,12 @@ void tls_multi_init_finalize(struct tls_multi *multi, int 
tls_mtu);
 struct tls_auth_standalone *tls_auth_standalone_init(struct tls_options 
*tls_options,
  struct gc_arena *gc);
 
+/**
+ * Frees a standalone tls-auth verification object.
+ * @param tas   the object to free. May be NULL.
+ */
+void tls_auth_standalone_free(struct tls_auth_standalone *tas);
+
 /*
  * Setups the control channel frame size parameters from the data channel
  * parameters
-- 
2.37.1 (Apple Git-137.1)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel