[Openvpn-devel] Ignore --explicit-exit-notify in TCP mode.

Message ID 20210802133127.25000-1-gert@greenie.muc.de
State Accepted
Headers show
Series [Openvpn-devel] Ignore --explicit-exit-notify in TCP mode. | expand

Commit Message

Gert Doering Aug. 2, 2021, 3:31 a.m. UTC
Mixed udp+tcp configs can not have --explicit-exit-notify in them
today because this option is refused in TCP mode.  At the same time,
it was always possible to push the option both in UDP and TCP mode
(with a warning logged in TCP mode, and the option reset to 0).

Do the same thing for local config - warn, and reset to 0.

(Leaving it enabled in TCP mode is harmless, but causes extra error
messages in the log which is undesired behaviour.  Maybe one should
just fix the underlying logic for TCP mode instead, but this is more
invasive)

Signed-off-by: Gert Doering <gert@greenie.muc.de>
---
 src/openvpn/options.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

Comments

Antonio Quartulli Aug. 3, 2021, 11:31 p.m. UTC | #1
Hi,

On 02/08/2021 15:31, Gert Doering wrote:
> Mixed udp+tcp configs can not have --explicit-exit-notify in them
> today because this option is refused in TCP mode.  At the same time,
> it was always possible to push the option both in UDP and TCP mode
> (with a warning logged in TCP mode, and the option reset to 0).
> 
> Do the same thing for local config - warn, and reset to 0.
> 
> (Leaving it enabled in TCP mode is harmless, but causes extra error
> messages in the log which is undesired behaviour.  Maybe one should
> just fix the underlying logic for TCP mode instead, but this is more
> invasive)
> 
> Signed-off-by: Gert Doering <gert@greenie.muc.de>

I gave this patch a run in various udp/tcp combinations and it all
looked good to me.

Having both:

proto tcp
explicit-exit-notify

in the client config does not lead to any fatal error anymore.
The client logs the fact with a NOTICE and moves on.

If the server will also push explict-exit-notify, the client will log
once again, as always happened (this behavior wasn't modified).

The same behavior can be observed on the server side.

Code change looks good and small enough :)

Acked-by: Antonio Quartulli <antonio@openvpn.net>
Gert Doering Aug. 5, 2021, 5:29 a.m. UTC | #2
Patch has been applied to the master branch.

commit 7953b07bf56c1df0f895ef0702a7732564de5ce9
Author: Gert Doering
Date:   Mon Aug 2 15:31:27 2021 +0200

     Ignore --explicit-exit-notify in TCP mode.

     Signed-off-by: Gert Doering <gert@greenie.muc.de>
     Acked-by: Antonio Quartulli <antonio@openvpn.net>
     Message-Id: <20210802133127.25000-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22690.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 63cda1e8..7e146db9 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -2266,12 +2266,6 @@  options_postprocess_verify_ce(const struct options *options,
     }
 #endif
 
-    if (!proto_is_udp(ce->proto) && ce->explicit_exit_notification)
-    {
-        msg(M_USAGE,
-            "--explicit-exit-notify can only be used with --proto udp");
-    }
-
     if (!ce->remote && ce->proto == PROTO_TCP_CLIENT)
     {
         msg(M_USAGE, "--remote MUST be used in TCP Client mode");
@@ -2978,6 +2972,13 @@  options_postprocess_mutate_ce(struct options *o, struct connection_entry *ce)
         connection_entry_preload_key(&ce->tls_crypt_v2_file,
                                      &ce->tls_crypt_v2_file_inline, &o->gc);
     }
+
+    if (!proto_is_udp(ce->proto) && ce->explicit_exit_notification)
+    {
+        msg(M_WARN, "NOTICE: --explicit-exit-notify ignored for --proto tcp");
+        ce->explicit_exit_notification = 0;
+    }
+
 }
 
 #ifdef _WIN32