[Openvpn-devel] Handle exceeding 'max-clients'

Message ID 20220713083404.13227-2-kprovost@netgate.com
State Accepted
Headers show
Series [Openvpn-devel] Handle exceeding 'max-clients' | expand

Commit Message

Kristof Provost via Openvpn-devel July 12, 2022, 10:34 p.m. UTC
From: Kristof Provost <kp@FreeBSD.org>

If 'max-clients' is set multi_create_instance() can return NULL (for any
client that would take us over the client limit).

If mi is NULL we don't add it to the hash map, but we do potentially
dereference it to increment the session count.
Do not attempt to do so if 'mi == NULL'.

Signed-off-by: Kristof Provost <kprovost@netgate.com>
---
 src/openvpn/mudp.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

Comments

Arne Schwabe July 13, 2022, 1:43 a.m. UTC | #1
Am 13.07.22 um 10:34 schrieb Kristof Provost via Openvpn-devel:
> From: Kristof Provost <kp@FreeBSD.org>
> 
> If 'max-clients' is set multi_create_instance() can return NULL (for any
> client that would take us over the client limit).
> 
> If mi is NULL we don't add it to the hash map, but we do potentially
> dereference it to increment the session count.
> Do not attempt to do so if 'mi == NULL'.

Oops. This patch also becomes much easier to review with ignoring 
whitespace. Then the change is just moving the }

Acked-By: Arne Schwabe <arne@rffc2549.org>
Gert Doering July 13, 2022, 2 a.m. UTC | #2
Quite obvious in hindsight ;-) - good catch.

Your patch has been applied to the master branch.

commit 6e47cadd6219a8a9da8c2c1558652bbbbf274a10
Author: Kristof Provost
Date:   Wed Jul 13 10:34:04 2022 +0200

     Handle exceeding 'max-clients'

     Signed-off-by: Kristof Provost <kprovost@netgate.com>
     Acked-by: Arne Schwabe <arne@rfc2549.org>
     Message-Id: <20220713083404.13227-2-kprovost@netgate.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24678.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
Gert Doering July 13, 2022, 2:05 a.m. UTC | #3
Hi,

On Wed, Jul 13, 2022 at 02:00:30PM +0200, Gert Doering wrote:
> Quite obvious in hindsight ;-) - good catch.
> 
> Your patch has been applied to the master branch.
> 
> commit 6e47cadd6219a8a9da8c2c1558652bbbbf274a10

Apologies.  My tree was not clean, so this commit ID is wrong.

I noticed before pushing, but after having sent this e-mail - so the
right commit ID for this is

commit ac14d90e7e5a80c57d064b1d3a5deb1db63b0911 (HEAD -> master)
Author: Kristof Provost <kp@FreeBSD.org>
Date:   Wed Jul 13 10:34:04 2022 +0200

    Handle exceeding 'max-clients'

which goes on top of

commit ad085464b15d63324846d0a5151141f58ccb5a34
Author: Antonio Quartulli <a@unstable.cc>
Date:   Mon Jul 11 14:23:48 2022 +0200

    options: don't export local function pre_connect_save()

gert

Patch

diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
index 0810fada..0cbca1a9 100644
--- a/src/openvpn/mudp.c
+++ b/src/openvpn/mudp.c
@@ -241,15 +241,16 @@  multi_get_create_instance_udp(struct multi_context *m, bool *floated)
                         hash_add_fast(hash, bucket, &mi->real, hv, mi);
                         mi->did_real_hash = true;
                         multi_assign_peer_id(m, mi);
-                    }
-                    /* If we have a session id already, ensure that the
-                     * state is using the same */
-                    if (session_id_defined(&state.server_session_id)
-                        && session_id_defined((&state.peer_session_id)))
-                    {
-                        mi->context.c2.tls_multi->n_sessions++;
-                        struct tls_session *session = &mi->context.c2.tls_multi->session[TM_ACTIVE];
-                        session_skip_to_pre_start(session, &state, &m->top.c2.from);
+
+                        /* If we have a session id already, ensure that the
+                         * state is using the same */
+                        if (session_id_defined(&state.server_session_id)
+                            && session_id_defined((&state.peer_session_id)))
+                        {
+                            mi->context.c2.tls_multi->n_sessions++;
+                            struct tls_session *session = &mi->context.c2.tls_multi->session[TM_ACTIVE];
+                            session_skip_to_pre_start(session, &state, &m->top.c2.from);
+                        }
                     }
                 }
                 else