[Openvpn-devel,v2] Warn if push is used without --mode server/--server/--server-bridge

Message ID 20251023155614.20642-1-gert@greenie.muc.de
State Accepted
Headers show
Series [Openvpn-devel,v2] Warn if push is used without --mode server/--server/--server-bridge | expand

Commit Message

Gert Doering Oct. 23, 2025, 3:56 p.m. UTC
From: Arne Schwabe <arne@rfc2549.org>

This is not a supported configuration and will often work good enough
to get a connection working but will operate more in a weird pre P2P
negotiation compatibility way rather than actually negotiating
protocol features.

Also remove an anused macro.

Change-Id: I82c7c61be07593ecd5bf2f854767dda74ab5170c
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1288
---

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/+/1288
This mail reflects revision 2 of this Change.

Acked-by according to Gerrit (reflected above):
Frank Lichtenheld <frank@lichtenheld.com>

Comments

Gert Doering Oct. 28, 2025, 5:46 p.m. UTC | #1
Change is straightforward enough... interestingly enough I do have a
t_server test instance that is "p2p --mode tls-server with --push" so
I could test this right away...

2025-10-28 18:39:08 us=773113 Note: Using --push without --mode server is an unsupported configuration. Negotiation of OpenVPN features is expected to fail.

.. and there you go.  Indeed it then fails cipher negotiation (without
--pull on the other end, it won't PUSH_REPLY, but with --pull, it fails
p2p cipher negotiation... and goes to lala land)

Interesting find on PUSH_DEFINED()... you could have removed the redundant
PULL_DEFINED() as well, while at it.  Next time.

Your patch has been applied to the master branch.

commit d6a0cf599c68edc672c67c6afa4bdff0e1a30b8a
Author: Arne Schwabe
Date:   Thu Oct 23 17:56:08 2025 +0200

     Warn if push is used without --mode server/--server/--server-bridge

     Signed-off-by: Arne Schwabe <arne@rfc2549.org>
     Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1288
     Message-Id: <20251023155614.20642-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg33856.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/doc/man-sections/server-options.rst b/doc/man-sections/server-options.rst
index ccc1374..347a251 100644
--- a/doc/man-sections/server-options.rst
+++ b/doc/man-sections/server-options.rst
@@ -497,6 +497,9 @@ 
   ``--echo``, ``--comp-lzo``, ``--socket-flags``, ``--sndbuf``,
   ``--rcvbuf``, ``--session-timeout``
 
+  Note: using ``--push`` requires OpenVPN to run in ``--mode server`` (or
+  using of one of `--server`, `--server-bridge` helper directives).
+
 --push-remove opt
   Selectively remove all ``--push`` options matching "opt" from the option
   list for a client. ``opt`` is matched as a substring against the whole
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 65c6b3b..9c02a8c 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -2690,6 +2690,13 @@ 
         MUST_BE_UNDEF(vlan_accept, "vlan-accept");
         MUST_BE_UNDEF(vlan_pvid, "vlan-pvid");
         MUST_BE_UNDEF(force_key_material_export, "force-key-material-export");
+
+        if (options->push_list.head)
+        {
+            msg(M_WARN, "Note: Using --push without --mode server is an "
+                        "unsupported configuration. Negotiation of OpenVPN "
+                        "features is expected to fail.");
+        }
     }
 
     /*
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index 009904a..24253af 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -768,16 +768,11 @@ 
 #define OPT_P_DEFAULT (~(OPT_P_INSTANCE | OPT_P_PULL_MODE))
 
 #define PULL_DEFINED(opt) ((opt)->pull)
-#define PUSH_DEFINED(opt) ((opt)->push_list)
 
 #ifndef PULL_DEFINED
 #define PULL_DEFINED(opt) (false)
 #endif
 
-#ifndef PUSH_DEFINED
-#define PUSH_DEFINED(opt) (false)
-#endif
-
 #ifdef _WIN32
 #define ROUTE_OPTION_FLAGS(o) ((o)->route_method & ROUTE_METHOD_MASK)
 #else