GitList

Browse code

Multi-socket: Fix assert triggered by stale peer-id reuse

Fixed a bug where clients using different transport
protocols (UDP, TCP) could interfere with each other
after a server restart.
The issue occurred when a client reused a previously
assigned peer-id that was now associated with a
different client using a different transport protocol.

For example, a UDP client could send packets with a
peer-id now assigned to a TCP client, which lacks
a valid context->c2.from which is filled by the
recvfrom(), causing an assert to be triggered.

A protocol check has been added to prevent packets
from different protocols from hijacking active
connections.

Github: OpenVPN/openvpn#773

Change-Id: Iecbbcf32c0059f2b16a05333b3794599060d7d6a
Signed-off-by: Gianmarco De Gregori <gianmarco@mandelbit.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20250718185559.4515-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg32220.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

Gianmarco De Gregori authored on 2025/07/19 03:55:53
Showing 1 changed files

src/openvpn/mudp.c

History View file @ fd93e4a

@@ -216,16 +216,20 @@ multi_get_create_instance_udp(struct multi_context *m, bool *floated,
                                  if (!peer_id_disabled && (peer_id < m->max_clients) && (m->instances[peer_id]))
+                                 {
                     -                mi = m->instances[peer_id];
+                    -
                     -                *floated = !link_socket_actual_match(&mi->context.c2.from, &m->top.c2.from);
+                    -
                     -                if (*floated)
                     +                /* Floating on TCP will never be possible, so ensure we only process
                     +                 * UDP clients */
                     +                if (m->instances[peer_id]->context.c2.link_sockets[0]->info.proto == sock->info.proto)
+                                     {
                     -                    /* reset prefix, since here we are not sure peer is the one it claims to be */
                     -                    ungenerate_prefix(mi);
                     -                    msg(D_MULTI_MEDIUM, "Float requested for peer %" PRIu32 " to %s", peer_id,
                     -                        mroute_addr_print(&real, &gc));
                     +                    mi = m->instances[peer_id];
                     +                    *floated = !link_socket_actual_match(&mi->context.c2.from, &m->top.c2.from);
+                    +
                     +                    if (*floated)
                     +                    {
                     +                        /* reset prefix, since here we are not sure peer is the one it claims to be */
                     +                        ungenerate_prefix(mi);
                     +                        msg(D_MULTI_MEDIUM, "Float requested for peer %" PRIu32 " to %s", peer_id,
                     +                            mroute_addr_print(&real, &gc));
                     +                    }
+                                     }
+                                 }
+                             }