[Openvpn-devel,v1] Mbed TLS: Error out if we have no valid tls-groups

Message ID 20260421055357.21708-1-gert@greenie.muc.de
State New
Headers show
Series [Openvpn-devel,v1] Mbed TLS: Error out if we have no valid tls-groups | expand

Commit Message

Gert Doering April 21, 2026, 5:53 a.m. UTC
From: Max Fillinger <maximilian.fillinger@sentyron.com>

Previously, when no valid groups were specified with the tls-groups
option, the Mbed TLS build of OpenVPN would start up and run, but fail
to complete a handshake, while the OpenSSL build would exit with an
error. This commit changes the behavior of the Mbed TLS build to match
the OpenSSL version.

Change-Id: Ica5f37e525c3812609021750ecd3986c1420e2a4
Signed-off-by: Max Fillinger <maximilian.fillinger@sentyron.com>
Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1633
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1633
This mail reflects revision 1 of this Change.

Acked-by according to Gerrit (reflected above):
Arne Schwabe <arne-openvpn@rfc2549.org>

Comments

Gert Doering April 27, 2026, 3:11 p.m. UTC | #1
Haven't tested this, but the change is quite obvious, and it's only
error handling in a particular niche case, so "reasonably safe".

Your patch has been applied to the master branch.

I'm not sure we need or want this in release/2.7 - you or Arne tell me ;-)

commit b2e3e0f0cf21a712b96efb8c053b740ca1947f54 (master)
Author: Max Fillinger
Date:   Tue Apr 21 07:53:50 2026 +0200

     Mbed TLS: Error out if we have no valid tls-groups

     Signed-off-by: Max Fillinger <maximilian.fillinger@sentyron.com>
     Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1633
     Message-Id: <20260421055357.21708-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg36699.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
Gert Doering April 27, 2026, 3:56 p.m. UTC | #2
Hi,

On Mon, Apr 27, 2026 at 05:11:59PM +0200, Gert Doering wrote:
> Haven't tested this, but the change is quite obvious, and it's only
> error handling in a particular niche case, so "reasonably safe".
> 
> Your patch has been applied to the master branch.
> 
> I'm not sure we need or want this in release/2.7 - you or Arne tell me ;-)
> 
> commit b2e3e0f0cf21a712b96efb8c053b740ca1947f54 (master)
> Author: Max Fillinger
> Date:   Tue Apr 21 07:53:50 2026 +0200
> 
>      Mbed TLS: Error out if we have no valid tls-groups

Arne told me, so here we are...

commit 573ccf82e90f03de3d65fb26aac9310a25c3e4ec (release/2.7)
Author: Max Fillinger <maximilian.fillinger@sentyron.com>
Date:   Tue Apr 21 07:53:50 2026 +0200

    Mbed TLS: Error out if we have no valid tls-groups

gert

Patch

diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index 85c771a..8a0f7d2 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -450,6 +450,12 @@ 
         }
     }
 
+    /* Check if any groups were valid. */
+    if (i == 0)
+    {
+        msg(M_FATAL, "Error: All groups in \"%s\" are invalid or unsupported.", groups);
+    }
+
     /* Recent mbedtls versions state that the list of groups must be terminated
      * with 0. Older versions state that it must be terminated with MBEDTLS_ECP_DP_NONE
      * which is also 0, so this works either way. */