| Message ID | tencent_C49D67EAA5678D180C293706A9469EFE8307@qq.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel] fix(ssl): init peer_id when init tls_multi | expand |
Am 19.10.23 um 19:12 schrieb yatta: > From: pushan <62173185+pushan01@users.noreply.github.com> > > When openvpn run in UDP server mode, if ssl connections reach the max clients, the next connection would be failed in `multi_create_instance` and the half connection will be close in `multi_close_instance`, which may lead array `m->instances[0]` covered unexpectedly and make the first connection interrupt, this patch fix this problem by init `peer_id` with `MAX_PEER_ID` in `tils_multi_init`. A bit more explanaition on this: When we create an instance in multi_create_instance we call inherit_context_child(&mi->context, &m->top); which in turn calls the tls_multi_init that is patched here. When I encounter an error during the creation in multi_create_instance we call multi_close_instance in the goto err branch. That multi_close_instance clears the instance from the dict if the peer_id is not MAX_PEER_ID. We probably should refactor this to be a bit cleaner in the future. Acked-By: Arne Schwabe <arne@rfc2549.org>
Hi, On Fri, Oct 20, 2023 at 01:12:13AM +0800, yatta wrote: > From: yatta <ytzhang01@foxmail.com> > From: pushan <62173185+pushan01@users.noreply.github.com> So, we have a patch that fixes a problem and an ACK for it, but for a proper commit I actually need something like a real name and a working mail address... Could you provide a Signed-off-by: line, like what we have on other commits, for example: Signed-off-by: David Sommerseth <davids@openvpn.net> (this does get relevant in case we end up having to do a license change again, and wanting to contact contributors...) thanks, gert
Unfortunately I haven't received a response to my query for a full
name to put into the logs, so "yatta" will have to do.
Thanks to Arne for the detailed insights on what is happening here, and
why this is the correct fix (refactoring for "master" welcome :-) ).
I have not tested this "for real" (on a p2mp server, actually filling
max users) - need to do this. But it passes all the basic tests, not
surprisingly.
Your patch has been applied to the master and release/2.6 branch (bugfix).
commit 3e30504d86f0fe5556acc0cb8e6975c5b2277661 (master)
commit 6dffbf6a2a0fdacb3509410f665a69dfa3b28cbc (release/2.6)
Author: yatta
Date: Fri Oct 20 01:12:13 2023 +0800
fix(ssl): init peer_id when init tls_multi
Signed-off-by: yatta <ytzhang01@foxmail.com
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <tencent_C49D67EAA5678D180C293706A9469EFE8307@qq.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg27260.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 5e6205cc..c4420b2e 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -64,6 +64,7 @@ #include "dco.h" #include "memdbg.h" +#include "openvpn.h" #ifdef MEASURE_TLS_HANDSHAKE_STATS @@ -1312,6 +1313,7 @@ tls_multi_init(struct tls_options *tls_options) /* get command line derived options */ ret->opt = *tls_options; ret->dco_peer_id = -1; + ret->peer_id = MAX_PEER_ID; return ret; }
From: pushan <62173185+pushan01@users.noreply.github.com> When openvpn run in UDP server mode, if ssl connections reach the max clients, the next connection would be failed in `multi_create_instance` and the half connection will be close in `multi_close_instance`, which may lead array `m->instances[0]` covered unexpectedly and make the first connection interrupt, this patch fix this problem by init `peer_id` with `MAX_PEER_ID` in `tils_multi_init`. --- src/openvpn/ssl.c | 2 ++ 1 file changed, 2 insertions(+)