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

Message ID 20210825040122.14244-2-selva.nair@gmail.com
State Superseded
Headers show
Series
  • [Openvpn-devel,1/2] Add remote-count and remote-entry query via management
Related show

Commit Message

Selva Nair Aug. 25, 2021, 4:01 a.m.
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.

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

Gert Doering Aug. 25, 2021, 12:36 p.m. | #1
Hi,

On Wed, Aug 25, 2021 at 12:01:22AM -0400, selva.nair@gmail.com wrote:
> 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.
> 
> Signed-off-by: Selva Nair <selva.nair@gmail.com>

The combination of this with the previous patch sounds quite useful
(= no longer distribute multiple different .ovpn for tcp/udp/... but
have them all in the same file).  

I assume support in the UI is also already brewing :-)

There is one catch: I assume that this conflicts heavily with the 
SRV patch (due to that one also changing connection entry handling),
and the SRV patch is overdue for final review and merging - thus, when
coming back to *this* patch, it might need a rebase.

gert
Selva Nair Aug. 25, 2021, 4:05 p.m. | #2
Hi,

On Wed, Aug 25, 2021 at 8:37 AM Gert Doering <gert@greenie.muc.de> wrote:

> Hi,
>
> On Wed, Aug 25, 2021 at 12:01:22AM -0400, selva.nair@gmail.com wrote:
> > 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.
> >
> > Signed-off-by: Selva Nair <selva.nair@gmail.com>
>
> The combination of this with the previous patch sounds quite useful
> (= no longer distribute multiple different .ovpn for tcp/udp/... but
> have them all in the same file).
>
> I assume support in the UI is also already brewing :-)
>

Yes, that's the plan..


> There is one catch: I assume that this conflicts heavily with the
> SRV patch (due to that one also changing connection entry handling),
> and the SRV patch is overdue for final review and merging - thus, when
> coming back to *this* patch, it might need a rebase.


I think, either way it would be a simple rebase. This one against the SRV
patch or the other way.

Any idea why "options_preprocess_verify_ce()", which is called for each
connection-entry, contains many checks that apply to common,
non-ce-specific options? This causes some warnings like "using management
without password is strongly discouraged.." printed once for every
connection entry  -- 100 times if you have 100 entries.

Selva
<div dir="ltr"><div dir="ltr">Hi,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Aug 25, 2021 at 8:37 AM Gert Doering &lt;<a href="mailto:gert@greenie.muc.de">gert@greenie.muc.de</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi,<br>
<br>
On Wed, Aug 25, 2021 at 12:01:22AM -0400, <a href="mailto:selva.nair@gmail.com" target="_blank">selva.nair@gmail.com</a> wrote:<br>
&gt; From: Selva Nair &lt;<a href="mailto:selva.nair@gmail.com" target="_blank">selva.nair@gmail.com</a>&gt;<br>
&gt; <br>
&gt; Currently we allow a max of 64 connection entries and remotes.<br>
&gt; A larger number would allow users with 100&#39;s of independent<br>
&gt; config files for different end points of same provider to<br>
&gt; consolidate them to connection entries.<br>
&gt; <br>
&gt; Signed-off-by: Selva Nair &lt;<a href="mailto:selva.nair@gmail.com" target="_blank">selva.nair@gmail.com</a>&gt;<br>
<br>
The combination of this with the previous patch sounds quite useful<br>
(= no longer distribute multiple different .ovpn for tcp/udp/... but<br>
have them all in the same file).  <br>
<br>
I assume support in the UI is also already brewing :-)<br></blockquote><div><br></div><div>Yes, that&#39;s the plan..</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
There is one catch: I assume that this conflicts heavily with the <br>
SRV patch (due to that one also changing connection entry handling),<br>
and the SRV patch is overdue for final review and merging - thus, when<br>
coming back to *this* patch, it might need a rebase.</blockquote><div> </div><div>I think, either way it would be a simple rebase. This one against the SRV patch or the other way.</div><div><br></div><div>Any idea why &quot;options_preprocess_verify_ce()&quot;, which is called for each connection-entry, contains many checks that apply to common, non-ce-specific options? This causes some warnings like &quot;using management without password is strongly discouraged..&quot; printed once for every connection entry  -- 100 times if you have 100 entries.</div><div><br></div><div>Selva</div></div></div>

Patch

diff --git a/Changes.rst b/Changes.rst
index e5ac8098..6e3c535e 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 index``
     commands from the management interface to get the number of
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 0e398c0f..5d0aa8af 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -889,6 +889,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);
@@ -1947,10 +1955,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;
@@ -1973,10 +1988,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