diff mbox series

[Openvpn-devel] dco: warn if DATA_V1 packets are sent to userspace

Message ID 20231022082751.8868-1-gert@greenie.muc.de
State Accepted
Headers show
Series [Openvpn-devel] dco: warn if DATA_V1 packets are sent to userspace | expand

Commit Message

Gert Doering Oct. 22, 2023, 8:27 a.m. UTC 
From: Lev Stipakov <lev@openvpn.net>

Servers 2.4.0 - 2.4.4 support peer-id and AEAD ciphers,
but only send DATA_V1 packets. With DCO enabled on the
client, connection is established but not working.

This is because DCO driver(s) are unable to handle
DATA_V1 packets and forwards them to userspace, where
they silently disappear since crypto context is in
DCO and not in userspace.

Starting from 2.4.5 server sends DATA_V2 so problem
doesn't happen.

We cannot switch to non-DCO on the fly, so we log this
and advice user to upgrade the server to 2.4.5 or newer.

Github: fixes OpenVPN/openvpn#422

Change-Id: I8cb2cb083e3cdadf187b7874979d79af3974e759
Signed-off-by: Lev Stipakov <lev@openvpn.net>
Acked-by: Gert Doering <gert@greenie.muc.de>
---

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

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/368
This mail reflects revision 3 of this Change.
Acked-by according to Gerrit (reflected above):
Gert Doering <gert@greenie.muc.de>

Comments

Gert Doering Oct. 22, 2023, 10:48 a.m. UTC | #1 
This is actually "V3" of the patch, but I forgot to add the -v3 when
sending from gerrit to the list.

The change is basically the same as in v1, just leaving the "real"
code alone, defusing it by setting c->c2.buf.len = 0 in the new
branch.  Plus comments :-)

As in v1, this adds diagnostics to detect a non-fixable incompatibility
between 2.4.0-2.4.4 servers and DCO-enabled clients (it can only be fixed
by upgrading the server, not by a code change on the client side, or by
disabling DCO on the client - but neither can be done automatically).

Tested on the server testbed, which has DCO and no-DCO peers, V1 and V2,
which should trigger "false alarms" nicely.

Your patch has been applied to the master and release/2.6 branch (compat).

commit df7beea404df48745a608c584d863c5a377b7a1e (master)
commit e78f88d8ea113585ca16945ef0361710b838ec7d (HEAD -> release/2.6)
Author: Lev Stipakov
Date:   Sun Oct 22 10:27:40 2023 +0200

     dco: warn if DATA_V1 packets are sent to userspace

     Signed-off-by: Lev Stipakov <lev@openvpn.net>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20231022082751.8868-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg27272.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index d8ad0d1..40f21bc 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -1047,6 +1047,24 @@ 
 
         if (c->c2.tls_multi)
         {
+            uint8_t opcode = *BPTR(&c->c2.buf) >> P_OPCODE_SHIFT;
+
+            /*
+             * If DCO is enabled, the kernel drivers require that the
+             * other end only sends P_DATA_V2 packets. V1 are unknown
+             * to kernel and passed to userland, but we cannot handle them
+             * either because crypto context is missing - so drop the packet.
+             *
+             * This can only happen with particular old (2.4.0-2.4.4) servers.
+             */
+            if ((opcode == P_DATA_V1) && dco_enabled(&c->options))
+            {
+                msg(D_LINK_ERRORS,
+                    "Data Channel Offload doesn't support DATA_V1 packets. "
+                    "Upgrade your server to 2.4.5 or newer.");
+                c->c2.buf.len = 0;
+            }
+
             /*
              * If tls_pre_decrypt returns true, it means the incoming
              * packet was a good TLS control channel packet.  If so, TLS code
@@ -1057,9 +1075,8 @@ 
              * will load crypto_options with the correct encryption key
              * and return false.
              */
-            uint8_t opcode = *BPTR(&c->c2.buf) >> P_OPCODE_SHIFT;
-            if (tls_pre_decrypt(c->c2.tls_multi, &c->c2.from, &c->c2.buf, &co,
-                                floated, &ad_start))
+            if (tls_pre_decrypt(c->c2.tls_multi, &c->c2.from, &c->c2.buf,
+                                &co, floated, &ad_start))
             {
                 /* Restore pre-NCP frame parameters */
                 if (is_hard_reset_method2(opcode))