[Openvpn-devel] dco: fix crash when --multihome is used with --proto tcp

Message ID 20230815231555.6465-1-a@unstable.cc
State Accepted
Headers show
Series [Openvpn-devel] dco: fix crash when --multihome is used with --proto tcp | expand

Commit Message

Antonio Quartulli Aug. 15, 2023, 11:15 p.m. UTC
Although it's a combination of options that is not really useful,
when specifying --multihome along with --proto tcp and DCO is enabled,
OpenVPN will crash while attempting to access c2.link_socket_actual
(NULL for the TCP case) in order to retrieve the local address (in
function dco_multi_get_localaddr())

Prevent crash by running this code only if proto is UDP.
The same check is already performed in socket.c/h for the non-DCO
case.

Fixes: https://github.com/OpenVPN/openvpn/issues/390
Change-Id: I61adc26ce2ff737e020c3d980902a46758cb23e5
Signed-off-by: Antonio Quartulli <a@unstable.cc>
---
 src/openvpn/dco.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Gert Doering Sept. 10, 2023, 3:31 p.m. UTC | #1
Acked-by: Gert Doering <gert@greenie.muc.de>

Code looks good.

Done a quick test on a Linux/DCO server, with TCP and --multihome.  Without
the patch:

2023-09-10 17:15:20 us=552871 freebsd-14-amd64/194.97.140.5:33293 MULTI: primary virtual IPv6 for freebsd-14-amd64/194.97.140.5:33293: fd00:abcd:220:1::1002
Segmentation fault (core dumped)

with the patch:

2023-09-10 17:16:57 us=590075 freebsd-14-amd64/194.97.140.5:41489 MULTI: primary virtual IPv6 for freebsd-14-amd64/194.97.140.5:41489: fd00:abcd:220:1::1002
2023-09-10 17:16:57 us=590252 freebsd-14-amd64/194.97.140.5:41489 Data Channel MTU parms [ mss_fix:1366 max_frag:0 tun_mtu:1500 tun_max_mtu:1600 headroom:136 payload:1768 tailroom:562 ET:0 ]
2023-09-10 17:16:57 us=590348 freebsd-14-amd64/194.97.140.5:41489 Outgoing dynamic tls-crypt: Cipher 'AES-256-CTR' initialized with 256 bit key
..

"and it just works".

Since the logic for "what do we need to do differently?" is the same
as for the non-DCO case now, I have not done more in-depth tests
(though I maybe *should* set up a server with multiple v4+v6 addresses
and run client tests against either one).


I have adjusted the github tag - our established format is

  Github: fixes OpenVPN/openvpn#390

not anything else.

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

commit 0793eb105c5720c4eb31af71c9db81459439e510 (master)
commit 70ef43f2b9b938256e0f689f4ddba0d6fbb3a371 (release/2.6)
Author: Antonio Quartulli
Date:   Wed Aug 16 01:15:55 2023 +0200

     dco: fix crash when --multihome is used with --proto tcp

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


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
index 7c7eaac4..cd3e0ad3 100644
--- a/src/openvpn/dco.c
+++ b/src/openvpn/dco.c
@@ -509,7 +509,7 @@  dco_multi_get_localaddr(struct multi_context *m, struct multi_instance *mi,
 #if ENABLE_IP_PKTINFO
     struct context *c = &mi->context;
 
-    if (!(c->options.sockflags & SF_USE_IP_PKTINFO))
+    if (!proto_is_udp(c->c2.link_socket->info.proto) || !(c->options.sockflags & SF_USE_IP_PKTINFO))
     {
         return false;
     }