[Openvpn-devel,v3,2/3] Permit unlimited connection entries and remotes

Message ID 20210907223126.8440-2-selva.nair@gmail.com
State Accepted
Headers show
Series [Openvpn-devel,v3,1/3] Add remote-count and remote-entry query via management | expand

Commit Message

Selva Nair Sept. 7, 2021, 12:31 p.m. UTC
From: Selva Nair <selva.nair@gmail.com>

Currently we allow a max of 64 connection entries and remotes.
A larger number would allow users with 100's of independent
config files for different end points of same provider to
consolidate them to connection entries.

v2,v3: no change

Signed-off-by: Selva Nair <selva.nair@gmail.com>
---
 Changes.rst           |  2 ++
 src/openvpn/options.c | 34 ++++++++++++++++++++++++++++------
 src/openvpn/options.h |  6 ++++--
 3 files changed, 34 insertions(+), 8 deletions(-)

Comments

Arne Schwabe Dec. 26, 2022, 7:49 p.m. UTC | #1
Am 08.09.21 um 00:31 schrieb selva.nair@gmail.com:
> From: Selva Nair <selva.nair@gmail.com>
> 
> Currently we allow a max of 64 connection entries and remotes.
> A larger number would allow users with 100's of independent
> config files for different end points of same provider to
> consolidate them to connection entries.
> 
> v2,v3: no change
> 
> Signed-off-by: Selva Nair <selva.nair@gmail.com>

Acked-By: Arne Schwabe@rfc2549.org

We are now increasing by multiple of 64 entries. While a bit wasteful 
and we could have used smaller increments, the memory lost here is not 
really a concern.

Arne
Gert Doering Dec. 27, 2022, 9:23 a.m. UTC | #2
The patch needed a bit of convincing to go in (context change in options.h,
and braces-in-the-wrong-line).  Fixed.

Tested with a config with 200 somewhat random remote lines - without the
patch

  Options error: Maximum number of 'remote' options (64) exceeded

and with the patch "it just works"...

    remote-entry-count
    206
    END
    remote-entry-get 200 999
    200,1195.server.org,1195,udp
    201,1196.server.org,1196,udp
    202,1197.server.org,1197,udp
    203,1198.server.org,1198,udp
    204,1199.server.org,1199,udp
    205,1200.server.org,1200,udp
    END

(I did not try 100s of <connection>)


That said, I'm not convinced we really need to print

  2022-12-27 10:16:56 WARNING: Using --management on a TCP port WITHOUT passwords is STRONGLY discouraged and considered insecure

for every single --remote...  but that's a different issue, not caused
by *this* patch.

Your patch has been applied to the master branch.

commit 4954beb618e8bf2dc756019d5a36040d791a8f38 (master)
commit ceba452ee44439b426b786325db2b4e1b1c8b663 (release/2.6)
Author: Selva Nair
Date:   Tue Sep 7 18:31:25 2021 -0400

     Permit unlimited connection entries and remotes

     Signed-off-by: Selva Nair <selva.nair@gmail.com>
     Acked-by: Arne Schwabe <arne@rfc2549.org>
     Message-Id: <20210907223126.8440-2-selva.nair@gmail.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22816.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/Changes.rst b/Changes.rst
index fa5d5ffa..9b1fb294 100644
--- a/Changes.rst
+++ b/Changes.rst
@@ -4,6 +4,8 @@  Overview of changes in 2.6
 
 New features
 ------------
+Support unlimited number of connection entries and remote entries
+
 New management commands to enumerate and list remote entries
     Use ``remote-entry-count`` and ``remote-entry-get``
     commands from the management interface to get the number of
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index f00b3019..38f97a39 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -888,6 +888,14 @@  init_options(struct options *o, const bool init_gc)
 void
 uninit_options(struct options *o)
 {
+    if (o->connection_list) {
+        free(o->connection_list->array);
+        CLEAR(*o->connection_list);
+    }
+    if (o->remote_list) {
+        free(o->remote_list->array);
+        CLEAR(*o->remote_list);
+    }
     if (o->gc_owned)
     {
         gc_free(&o->gc);
@@ -1946,10 +1954,17 @@  alloc_connection_entry(struct options *options, const int msglevel)
     struct connection_list *l = alloc_connection_list_if_undef(options);
     struct connection_entry *e;
 
-    if (l->len >= CONNECTION_LIST_SIZE)
+    if (l->len == l->capacity)
     {
-        msg(msglevel, "Maximum number of 'connection' options (%d) exceeded", CONNECTION_LIST_SIZE);
-        return NULL;
+        int capacity = l->capacity + CONNECTION_LIST_SIZE;
+        struct connection_entry **ce = realloc(l->array, capacity*sizeof(struct connection_entry *));
+        if (ce == NULL)
+        {
+            msg(msglevel, "Unable to process more connection options: out of memory. Number of entries = %d", l->len);
+            return NULL;
+        }
+        l->array = ce;
+        l->capacity = capacity;
     }
     ALLOC_OBJ_GC(e, struct connection_entry, &options->gc);
     l->array[l->len++] = e;
@@ -1972,10 +1987,17 @@  alloc_remote_entry(struct options *options, const int msglevel)
     struct remote_list *l = alloc_remote_list_if_undef(options);
     struct remote_entry *e;
 
-    if (l->len >= CONNECTION_LIST_SIZE)
+    if (l->len == l->capacity)
     {
-        msg(msglevel, "Maximum number of 'remote' options (%d) exceeded", CONNECTION_LIST_SIZE);
-        return NULL;
+        int capacity = l->capacity + CONNECTION_LIST_SIZE;
+        struct remote_entry **re = realloc(l->array, capacity*sizeof(struct remote_entry *));
+        if (re == NULL)
+        {
+            msg(msglevel, "Unable to process more remote options: out of memory. Number of entries = %d", l->len);
+            return NULL;
+        }
+        l->array = re;
+        l->capacity = capacity;
     }
     ALLOC_OBJ_GC(e, struct remote_entry, &options->gc);
     l->array[l->len++] = e;
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index b0e40cb7..98977d41 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -168,15 +168,17 @@  struct remote_entry
 
 struct connection_list
 {
+    int capacity;
     int len;
     int current;
-    struct connection_entry *array[CONNECTION_LIST_SIZE];
+    struct connection_entry **array;
 };
 
 struct remote_list
 {
+    int capacity;
     int len;
-    struct remote_entry *array[CONNECTION_LIST_SIZE];
+    struct remote_entry **array;
 };
 
 enum vlan_acceptable_frames