Attention is currently required from: flichtenheld, its_Giaan, plaisthos. Hello flichtenheld, plaisthos,
I'd like you to reexamine a change. Please visit http://gerrit.openvpn.net/c/openvpn/+/1078?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Code-Review-1 by plaisthos 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, 13 insertions(+), 9 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/78/1078/2 diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c index 93e65e0..f62e0a3 100644 --- a/src/openvpn/mudp.c +++ b/src/openvpn/mudp.c @@ -216,16 +216,20 @@ 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]; + *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: 2 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: its_Giaan <gianma...@mandelbit.com> Gerrit-Attention: flichtenheld <fr...@lichtenheld.com> Gerrit-MessageType: newpatchset
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel