[Openvpn-devel,M] Change in openvpn[master]: allow user to specify 'local' multiple times in config files

Message ID 13d3eb64cbd8db5e2a30be372713eb8710bfa4ea-HTML@gerrit.openvpn.net
State Not Applicable
Headers show
Series [Openvpn-devel,M] Change in openvpn[master]: allow user to specify 'local' multiple times in config files | expand

Commit Message

d12fk (Code Review) Nov. 15, 2023, 1:45 p.m. UTC
Attention is currently required from: flichtenheld, plaisthos.

Hello plaisthos, flichtenheld,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/436?usp=email

to review the following change.


Change subject: allow user to specify 'local' multiple times in config files
......................................................................

allow user to specify 'local' multiple times in config files

It is now possible to specify 'local' multiple times in a server
config to let it listen on multiple sockets (address:port) of
the same protocol.

Change-Id: I4d1c96662c5a8c750d883e3b20adde09529e2764
Signed-off-by: Antonio Quartulli <a@unstable.cc>
---
M src/openvpn/init.c
M src/openvpn/options.c
M src/openvpn/options.h
M src/openvpn/socket.c
4 files changed, 171 insertions(+), 39 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/36/436/1

Patch

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index f8dd01f..659c9e3 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -745,7 +745,7 @@ 
 
     init_connection_list(c);
 
-    c->c1.link_sockets_num = 1;
+    c->c1.link_sockets_num = c->options.ce.local_list->len;
 
     do_link_socket_addr_new(c);
 
@@ -3821,8 +3821,8 @@ 
 
         /* init each socket with its specific port */
         link_socket_init_phase1(c->c2.link_sockets[i],
-                                c->options.ce.local,
-                                c->options.ce.local_port,
+                                c->options.ce.local_list->array[i]->local,
+                                c->options.ce.local_list->array[i]->port,
                                 c->options.ce.remote,
                                 c->options.ce.remote_port,
                                 c->c1.dns_cache,
@@ -3836,7 +3836,7 @@ 
 #ifdef ENABLE_DEBUG
                                 c->options.gremlin,
 #endif
-                                c->options.ce.bind_local,
+                                c->options.ce.local_list->array[i]->bind_local,
                                 c->options.ce.remote_float,
                                 &c->c1.link_socket_addrs[i],
                                 c->options.ipchange,
@@ -4978,6 +4978,7 @@ 
     if (dest->mode == CM_CHILD_UDP)
     {
         ASSERT(!dest->c2.link_sockets);
+        ASSERT(dest->options.ce.local_list);
 
         /* inherit buffers */
         dest->c2.buffers = src->c2.buffers;
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index b88fea9..9611423 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -124,7 +124,13 @@ 
     "--version       : Show copyright and version information.\n"
     "\n"
     "Tunnel Options:\n"
-    "--local host    : Local host name or ip address. Implies --bind.\n"
+    "--local host|* [port] : Local host name or ip address and port. '*' can be used\n"
+    "                        as hostname and means 'any host' (openvpn will listen on\n"
+    "                        what is returned by the OS). Implies --bind.\n"
+    "                        0.0.0.0 or :: can be used to specifically open a socket\n"
+    "                        listening on any IPv4 or IPv6 address respectively.\n"
+    "                        The user can specify multiple --local entries to have\n"
+    "                        a server listen on multiple sockets at the same time.\n"
     "--remote host [port] : Remote host name or ip address.\n"
     "--remote-random : If multiple --remote options specified, choose one randomly.\n"
     "--remote-random-hostname : Add a random string to remote DNS name.\n"
@@ -982,8 +988,9 @@ 
                         const int i)
 {
     setenv_str_i(es, "proto", proto2ascii(e->proto, e->af, false), i);
-    setenv_str_i(es, "local", e->local, i);
-    setenv_str_i(es, "local_port", e->local_port, i);
+    /* expected to befor single socket contexts only */
+    setenv_str_i(es, "local", e->local_list->array[0]->local, i);
+    setenv_str_i(es, "local_port", e->local_list->array[0]->port, i);
     setenv_str_i(es, "remote", e->remote, i);
     setenv_str_i(es, "remote_port", e->remote_port, i);
 
@@ -1701,8 +1708,12 @@ 
 show_connection_entry(const struct connection_entry *o)
 {
     msg(D_SHOW_PARMS, "  proto = %s", proto2ascii(o->proto, o->af, false));
-    SHOW_STR(local);
-    SHOW_STR(local_port);
+    msg(D_SHOW_PARMS, "  Local Sockets:");
+    for (int i = 0; i < o->local_list->len; i++)
+    {
+        msg(D_SHOW_PARMS, "    [%s]:%s", o->local_list->array[i]->local,
+            o->local_list->array[i]->port);
+    }
     SHOW_STR(remote);
     SHOW_STR(remote_port);
     SHOW_BOOL(remote_float);
@@ -2151,6 +2162,37 @@ 
 
 #endif /* ifdef ENABLE_MANAGEMENT */
 
+static struct local_list *
+alloc_local_list_if_undef(struct connection_entry *ce, struct gc_arena *gc)
+{
+    if (!ce->local_list)
+    {
+        ALLOC_OBJ_CLEAR_GC(ce->local_list, struct local_list, gc);
+    }
+    return ce->local_list;
+}
+
+static struct local_entry *
+alloc_local_entry(struct connection_entry *ce, const int msglevel,
+                  struct gc_arena *gc)
+{
+    struct local_list *l = alloc_local_list_if_undef(ce, gc);
+    struct local_entry *e;
+
+    if (l->len >= CONNECTION_LIST_SIZE)
+    {
+        msg(msglevel, "Maximum number of 'local' options (%d) exceeded",
+            CONNECTION_LIST_SIZE);
+
+        return NULL;
+    }
+
+    ALLOC_OBJ_CLEAR_GC(e, struct local_entry, gc);
+    l->array[l->len++] = e;
+
+    return e;
+}
+
 static struct connection_list *
 alloc_connection_list_if_undef(struct options *options)
 {
@@ -2343,6 +2385,15 @@ 
             "--proto tcp-server or --proto tcp-client");
     }
 
+    /*
+     * Sanity check on daemon mode
+     */
+
+    if (ce->remote && ce->local_list->len > 1)
+    {
+        msg(M_USAGE, "multiple --local do not make sense in Client mode");
+    }
+
     if (options->lladdr && dev != DEV_TYPE_TAP)
     {
         msg(M_USAGE, "--lladdr can only be used in --dev tap mode");
@@ -2368,13 +2419,6 @@ 
      * Sanity check on --local, --remote, and --ifconfig
      */
 
-    if (proto_is_net(ce->proto)
-        && string_defined_equal(ce->local, ce->remote)
-        && string_defined_equal(ce->local_port, ce->remote_port))
-    {
-        msg(M_USAGE, "--remote and --local addresses are the same");
-    }
-
     if (string_defined_equal(ce->remote, options->ifconfig_local)
         || string_defined_equal(ce->remote, options->ifconfig_remote_netmask))
     {
@@ -2383,13 +2427,6 @@ 
             "addresses");
     }
 
-    if (string_defined_equal(ce->local, options->ifconfig_local)
-        || string_defined_equal(ce->local, options->ifconfig_remote_netmask))
-    {
-        msg(M_USAGE,
-            "--local addresses must be distinct from --ifconfig addresses");
-    }
-
     if (string_defined_equal(options->ifconfig_local,
                              options->ifconfig_remote_netmask))
     {
@@ -2402,12 +2439,6 @@ 
         msg(M_USAGE, "--bind and --nobind can't be used together");
     }
 
-    if (ce->local && !ce->bind_local)
-    {
-        msg(M_USAGE,
-            "--local and --nobind don't make sense when used together");
-    }
-
     if (ce->local_port_defined && !ce->bind_local)
     {
         msg(M_USAGE,
@@ -2419,6 +2450,29 @@ 
         msg(M_USAGE, "--nobind doesn't make sense unless used with --remote");
     }
 
+    for (int i = 0; i < ce->local_list->len; i++)
+    {
+        struct local_entry *le = ce->local_list->array[i];
+
+        if (proto_is_net(ce->proto)
+            && string_defined_equal(le->local, ce->remote)
+            && string_defined_equal(le->port, ce->remote_port))
+        {
+            msg(M_USAGE, "--remote and a --local addresses are the same");
+        }
+
+        if (string_defined_equal(le->local, options->ifconfig_local)
+            || string_defined_equal(le->local, options->ifconfig_remote_netmask))
+        {
+            msg(M_USAGE, "--local addresses must be distinct from --ifconfig addresses");
+        }
+
+        if (le->local && !ce->bind_local)
+        {
+            msg(M_USAGE, "--local and --nobind don't make sense when used together");
+        }
+    }
+
     /*
      * Check for consistency of management options
      */
@@ -3112,7 +3166,7 @@ 
     }
 
     /* an option is present that requires local bind to enabled */
-    bool need_bind = ce->local || ce->local_port_defined || ce->bind_defined;
+    bool need_bind = ce->local_port_defined || ce->bind_defined;
 
     /* socks proxy is enabled */
     bool uses_socks = ce->proto == PROTO_UDP && ce->socks_proxy_server;
@@ -3125,6 +3179,11 @@ 
         ce->bind_local = false;
     }
 
+    if (ce->proto == PROTO_UDP && ce->socks_proxy_server && !ce->local_list && !ce->local_port_defined && !ce->bind_defined)
+    {
+        ce->bind_local = false;
+    }
+
     if (!ce->bind_local)
     {
         ce->local_port = NULL;
@@ -3245,6 +3304,16 @@ 
     }
 }
 
+static void
+options_postprocess_mutate_le(struct options *o, struct local_entry *le)
+{
+    /* use the global port if none is specified */
+    if (!le->port)
+    {
+        le->port = o->ce.local_port;
+    }
+}
+
 #ifdef _WIN32
 /* If iservice is in use, we need def1 method for redirect-gateway */
 static void
@@ -3691,6 +3760,29 @@ 
         options_postprocess_mutate_ce(o, o->connection_list->array[i]);
     }
 
+    if (o->ce.local_list)
+    {
+        for (i = 0; i < o->ce.local_list->len; i++)
+        {
+            options_postprocess_mutate_le(o, o->ce.local_list->array[i]);
+        }
+    }
+    else
+    {
+        /* if no 'local' directive was specified, convert the global port
+         * setting to a listen entry */
+        struct local_entry *e = alloc_local_entry(&o->ce, M_USAGE, &o->gc);
+        ASSERT(e);
+        e->port = o->ce.local_port;
+        e->bind_local = o->ce.bind_local;
+    }
+
+    /* use the same listen list for every outgoing connection */
+    for (i = 0; i < o->connection_list->len; ++i)
+    {
+        o->connection_list->array[i]->local_list = o->ce.local_list;
+    }
+
     if (o->tls_server)
     {
         /* Check that DH file is specified, or explicitly disabled */
@@ -6072,10 +6164,29 @@ 
         VERIFY_PERMISSION(OPT_P_UP);
         options->ifconfig_nowarn = true;
     }
-    else if (streq(p[0], "local") && p[1] && !p[2])
+    else if (streq(p[0], "local") && p[1] && !p[3])
     {
+        struct local_entry *e;
+
         VERIFY_PERMISSION(OPT_P_GENERAL|OPT_P_CONNECTION);
-        options->ce.local = p[1];
+
+        e = alloc_local_entry(&options->ce, M_USAGE, &options->gc);
+        ASSERT(e);
+
+        /* '*' is treated as 'ask the system to get some socket',
+         * therefore force binding on a particular address only when
+         * actually specified. */
+        if (strcmp(p[1], "*") != 0)
+        {
+            e->local = p[1];
+            e->bind_local = true;
+        }
+
+        if (p[2])
+        {
+            e->port = p[2];
+            e->bind_local = true;
+        }
     }
     else if (streq(p[0], "remote-random") && !p[1])
     {
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index 5a37316..747ed56 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -94,14 +94,21 @@ 
 #error "At least one of OpenSSL or mbed TLS needs to be defined."
 #endif
 
+struct local_entry
+{
+    const char *local;
+    const char *port;
+    bool bind_local;
+};
+
 struct connection_entry
 {
+    struct local_list *local_list;
     int proto;
     sa_family_t af;
     const char *local_port;
     bool local_port_defined;
     const char *remote_port;
-    const char *local;
     const char *remote;
     bool remote_float;
     bool bind_defined;
@@ -179,6 +186,12 @@ 
 
 #define CONNECTION_LIST_SIZE 64
 
+struct local_list
+{
+    int len;
+    struct local_entry *array[CONNECTION_LIST_SIZE];
+};
+
 struct connection_list
 {
     int capacity;
diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
index 17bd263..5d9e111 100644
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -342,7 +342,7 @@ 
 void
 do_preresolve(struct context *c)
 {
-    int i;
+    int i, j;
     struct connection_list *l = c->options.connection_list;
     const unsigned int preresolve_flags = GETADDR_RESOLVE
                                           |GETADDR_UPDATE_MANAGEMENT_STATE
@@ -416,12 +416,19 @@ 
             }
         }
 
-        if (ce->bind_local)
+        flags |= GETADDR_PASSIVE;
+        flags &= ~GETADDR_RANDOMIZE;
+
+        for (j = 0; j < ce->local_list->len; j++)
         {
-            flags |= GETADDR_PASSIVE;
-            flags &= ~GETADDR_RANDOMIZE;
-            status = do_preresolve_host(c, ce->local, ce->local_port,
-                                        ce->af, flags);
+            struct local_entry *le = ce->local_list->array[j];
+
+            if (!le->local)
+            {
+                continue;
+            }
+
+            status = do_preresolve_host(c, le->local, le->port, ce->af, flags);
             if (status != 0)
             {
                 goto err;