Attention is currently required from: flichtenheld, plaisthos.

Hello plaisthos, flichtenheld,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1078?usp=email

to review the following change.


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

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: #773

Change-Id: Iecbbcf32c0059f2b16a05333b3794599060d7d6a
Signed-off-by: Gianmarco De Gregori <gianma...@mandelbit.com>
---
M src/openvpn/mudp.c
1 file changed, 17 insertions(+), 9 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/78/1078/1

diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
index 93e65e0..c47ed16 100644
--- a/src/openvpn/mudp.c
+++ b/src/openvpn/mudp.c
@@ -216,16 +216,24 @@

             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)
+                /* Ensure that clients from previous sessions do not attempt to
+                 * hijack instances of newly connected clients in 
multi-protocol scenarios */
+                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];
+
+                    if (mi)
+                    {
+                        *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));
+                        }
+                    }
                 }
             }
         }

--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1078?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Iecbbcf32c0059f2b16a05333b3794599060d7d6a
Gerrit-Change-Number: 1078
Gerrit-PatchSet: 1
Gerrit-Owner: its_Giaan <gianma...@mandelbit.com>
Gerrit-Reviewer: flichtenheld <fr...@lichtenheld.com>
Gerrit-Reviewer: plaisthos <arne-open...@rfc2549.org>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: plaisthos <arne-open...@rfc2549.org>
Gerrit-Attention: flichtenheld <fr...@lichtenheld.com>
Gerrit-MessageType: newchange
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to