[M] Change in openvpn[master]: multi: assign clients from a named, tagged subnet-pool

Attention is currently required from: flichtenheld, ordex, plaisthos.

Hello flichtenheld, plaisthos, 

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#2).

The following approvals got outdated and were removed:
Code-Review-1 by flichtenheld


Change subject: multi: assign clients from a named, tagged subnet-pool
......................................................................

multi: assign clients from a named, tagged subnet-pool

Add two server directives:

  subnet-pool <tag> <network> <netmask> [<gateway>]   (main config)
  subnet-pool-tag <tag>                               (per-client CCD)

so groups of clients can be given addresses from subnets other than the
--server network (per-group firewall rules under --topology subnet, and
thus DCO). A client's subnet-pool-tag is resolved to its pool and served
a virtual IP from that subnet: a static --ifconfig-push within it, or the
next free address from a dynamic per-group pool shared by tag and created
on first use. Such a client is validated against its own subnet instead
of the global tunnel-network constraint and never falls back to the
global pool; a tag matching no configured pool is declined with
AUTH_FAILED.

Change-Id: Ia1e7db95615d99ef9687de7c4e996dc8033e2631
GitHub: closes openvpn/OpenVPN#987
Signed-off-by: Antonio Quartulli <[email protected]>
---
M doc/man-sections/server-options.rst
M src/openvpn/multi.c
M src/openvpn/multi.h
M src/openvpn/options.c
M src/openvpn/options.h
5 files changed, 214 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/83/1783/2

diff --git a/doc/man-sections/server-options.rst b/doc/man-sections/server-options.rst
index f420588..8800b98 100644
--- a/doc/man-sections/server-options.rst
+++ b/doc/man-sections/server-options.rst
@@ -330,6 +330,31 @@
   by ``--ifconfig``, OpenVPN will install a /32 host route for the ``local``
   IP address.
 
+--subnet-pool args
+  Define a named pool of addresses from a subnet different from the
+  ``--server`` network, so groups of clients can be given IPs from separate
+  subnets for per-group firewall rules under ``--topology subnet`` (and
+  therefore DCO). This is a main-config directive and may be repeated to
+  define several pools.
+
+  Valid syntax:
+  ::
+
+     subnet-pool tag network netmask [gateway]
+
+  ``tag`` is a name a client refers to with ``--subnet-pool-tag``; ``gateway``
+  defaults to the first address of ``network``/``netmask``. A client assigned
+  to a pool receives (dynamically, or via a static ``--ifconfig-push`` that
+  must lie within the subnet) an address from it, plus the matching netmask, a
+  ``--route-gateway`` inside the subnet, and a host route back to the server's
+  VPN IP, so it is reachable without manual per-client route pushes.
+
+--subnet-pool-tag tag
+  Assign this client to the ``--subnet-pool`` named ``tag``. Must be
+  associated with a specific client instance via ``--client-config-dir`` or
+  ``--client-connect``. Several clients may reference the same tag and are
+  then served from the same shared pool.
+
 --ifconfig-ipv6-push args
   for ``--client-config-dir`` per-client static IPv6 interface
   configuration, see ``--client-config-dir`` and ``--ifconfig-push`` for
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index c4293a7..84f25b3 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -53,6 +53,17 @@
 #include "dco.h"
 #include "reflect_filter.h"
 
+/*
+ * Dynamic address pool serving the clients of one --subnet-pool tag.  Shared
+ * by every client that references the tag, created on first use.
+ */
+struct subnet_pool
+{
+    struct subnet_pool *next;
+    const char *tag;
+    struct ifconfig_pool *pool;
+};
+
 /*#define MULTI_DEBUG_EVENT_LOOP*/
 
 #ifdef MULTI_DEBUG_EVENT_LOOP
@@ -612,7 +623,8 @@
 
         schedule_remove_entry(m->schedule, (struct schedule_entry *)mi);
 
-        ifconfig_pool_release(m->ifconfig_pool, mi->vaddr_handle, false);
+        ifconfig_pool_release(mi->vaddr_pool ? mi->vaddr_pool : m->ifconfig_pool, mi->vaddr_handle,
+                              false);
 
         if (mi->did_iroutes)
         {
@@ -687,6 +699,14 @@
         schedule_free(m->schedule);
         mbuf_free(m->mbuf);
         ifconfig_pool_free(m->ifconfig_pool);
+        while (m->subnet_pools)
+        {
+            struct subnet_pool *sp = m->subnet_pools;
+
+            m->subnet_pools = sp->next;
+            ifconfig_pool_free(sp->pool);
+            free(sp);
+        }
         frequency_limit_free(m->new_connection_limiter);
         initial_rate_limit_free(m->initial_rate_limiter);
         multi_reap_free(m->reaper);
@@ -1384,6 +1404,12 @@
     const struct options *o = &c->options;
     if (o->push_ifconfig_constraint_defined && c->c2.push_ifconfig_defined)
     {
+        /* a subnet-pool client is validated against its own subnet */
+        if (o->subnet_pool_defined)
+        {
+            return (o->subnet_pool_netmask & c->c2.push_ifconfig_local)
+                   == o->subnet_pool_network;
+        }
         return (o->push_ifconfig_constraint_netmask & c->c2.push_ifconfig_local)
                == o->push_ifconfig_constraint_network;
     }
@@ -1393,6 +1419,47 @@
     }
 }
 
+static const struct subnet_pool_def *
+subnet_pool_by_tag(const struct options *o, const char *tag)
+{
+    for (const struct subnet_pool_def *d = o->subnet_pools; d; d = d->next)
+    {
+        if (!strcmp(d->tag, tag))
+        {
+            return d;
+        }
+    }
+    return NULL;
+}
+
+/*
+ * Return the dynamic pool serving the clients tagged with <tag>, creating it
+ * on first use from the matching --subnet-pool definition.
+ */
+static struct ifconfig_pool *
+multi_get_group_pool(struct multi_context *m, const char *tag)
+{
+    struct subnet_pool *sp;
+
+    for (sp = m->subnet_pools; sp; sp = sp->next)
+    {
+        if (!strcmp(sp->tag, tag))
+        {
+            return sp->pool;
+        }
+    }
+
+    const struct subnet_pool_def *d4 = subnet_pool_by_tag(&m->top.options, tag);
+    ALLOC_OBJ_CLEAR(sp, struct subnet_pool);
+    sp->tag = d4->tag;
+    sp->pool = ifconfig_pool_init(true, IFCONFIG_POOL_INDIV, d4->network + 2,
+                                  (d4->network | ~d4->netmask) - 1, m->top.options.duplicate_cn,
+                                  false, in6addr_any, 0);
+    sp->next = m->subnet_pools;
+    m->subnet_pools = sp;
+    return sp->pool;
+}
+
 /*
  * Select a virtual address for a new client instance.
  * Use an --ifconfig-push directive, if given (static IP).
@@ -1403,6 +1470,21 @@
 {
     struct gc_arena gc = gc_new();
 
+    /* resolve a CCD --subnet-pool-tag against the main-config --subnet-pool list
+     * (an unknown tag is rejected with AUTH_FAILED in multi_client_connect) */
+    if (mi->context.options.subnet_pool_tag && !mi->context.options.subnet_pool_defined)
+    {
+        const struct subnet_pool_def *d =
+            subnet_pool_by_tag(&m->top.options, mi->context.options.subnet_pool_tag);
+        if (d)
+        {
+            mi->context.options.subnet_pool_defined = true;
+            mi->context.options.subnet_pool_network = d->network;
+            mi->context.options.subnet_pool_netmask = d->netmask;
+            mi->context.options.subnet_pool_gateway = d->gateway;
+        }
+    }
+
     /*
      * If ifconfig addresses were set by dynamic config file,
      * release pool addresses, otherwise keep them.
@@ -1413,8 +1495,10 @@
          * release dynamic allocation */
         if (mi->vaddr_handle >= 0)
         {
-            ifconfig_pool_release(m->ifconfig_pool, mi->vaddr_handle, true);
+            ifconfig_pool_release(mi->vaddr_pool ? mi->vaddr_pool : m->ifconfig_pool,
+                                  mi->vaddr_handle, true);
             mi->vaddr_handle = -1;
+            mi->vaddr_pool = NULL;
         }
 
         mi->context.c2.push_ifconfig_defined = true;
@@ -1434,6 +1518,44 @@
                 "MULTI_sva: WARNING: if --ifconfig-push is used for IPv4, automatic IPv6 assignment from --ifconfig-ipv6-pool does not work.  Use --ifconfig-ipv6-push for IPv6 then.");
         }
     }
+    else if (mi->context.options.subnet_pool_defined)
+    {
+        /* dynamic address from this client's --subnet-pool group */
+
+        /* the CCD is read after a global-pool address may already have been
+         * acquired; drop it so we can serve this client from its group */
+        if (mi->vaddr_handle >= 0 && !mi->vaddr_pool)
+        {
+            ifconfig_pool_release(m->ifconfig_pool, mi->vaddr_handle, true);
+            mi->vaddr_handle = -1;
+        }
+
+        if (mi->vaddr_handle < 0)
+        {
+            in_addr_t local = 0, remote = 0;
+            const char *cn = NULL;
+
+            if (!mi->context.options.duplicate_cn)
+            {
+                cn = tls_common_name(mi->context.c2.tls_multi, true);
+            }
+
+            mi->vaddr_pool = multi_get_group_pool(m, mi->context.options.subnet_pool_tag);
+            mi->vaddr_handle = ifconfig_pool_acquire(mi->vaddr_pool, &local, &remote, NULL, cn);
+            if (mi->vaddr_handle >= 0)
+            {
+                mi->context.c2.push_ifconfig_local = remote;
+                mi->context.c2.push_ifconfig_remote_netmask =
+                    mi->context.options.subnet_pool_netmask;
+                mi->context.c2.push_ifconfig_defined = true;
+            }
+            else
+            {
+                msg(D_MULTI_ERRORS, "MULTI: no free --subnet-pool addresses are available for %s",
+                    multi_instance_string(mi, false, &gc));
+            }
+        }
+    }
     else if (m->ifconfig_pool && mi->vaddr_handle < 0) /* otherwise, choose a pool address */
     {
         in_addr_t local = 0, remote = 0;
@@ -2339,10 +2461,9 @@
         const char *ifconfig_constraint_netmask =
             print_in_addr_t(mi->context.options.push_ifconfig_constraint_netmask, 0, &gc);
 
-        /* JYFIXME -- this should cause the connection to fail */
         msg(D_MULTI_ERRORS,
-            "MULTI ERROR: primary virtual IP for %s (%s) "
-            "violates tunnel network/netmask constraint (%s/%s)",
+            "WARNING: primary virtual IP for %s (%s) "
+            "violates tunnel network/netmask constraint (%s/%s) and may not work",
             multi_instance_string(mi, false, &gc),
             print_in_addr_t(mi->context.c2.push_ifconfig_local, 0, &gc),
             ifconfig_constraint_network, ifconfig_constraint_netmask);
@@ -2450,6 +2571,7 @@
 
     /* reset pool handle to null */
     mi->vaddr_handle = -1;
+    mi->vaddr_pool = NULL;
 
     /* do --client-connect setenvs */
     multi_select_virtual_addr(m, mi);
@@ -2741,6 +2863,15 @@
         cc_succeeded = false;
     }
 
+    if (mi->context.options.subnet_pool_tag
+        && !subnet_pool_by_tag(&m->top.options, mi->context.options.subnet_pool_tag))
+    {
+        msg(D_MULTI_ERRORS, "MULTI: client has been rejected due to unknown --subnet-pool-tag '%s'",
+            mi->context.options.subnet_pool_tag);
+        auth_set_client_reason(mi->context.c2.tls_multi, "unknown subnet-pool-tag");
+        cc_succeeded = false;
+    }
+
     if (cc_succeeded)
     {
         multi_client_connect_late_setup(m, mi, *option_types_found);
diff --git a/src/openvpn/multi.h b/src/openvpn/multi.h
index 8b837fa..f6815f4 100644
--- a/src/openvpn/multi.h
+++ b/src/openvpn/multi.h
@@ -121,6 +121,8 @@
     struct mroute_addr real; /**< External network address of the
                               *   remote peer. */
     ifconfig_pool_handle vaddr_handle;
+    /* pool vaddr_handle belongs to (NULL = the global one) */
+    struct ifconfig_pool *vaddr_pool;
     char msg_prefix[MULTI_PREFIX_MAX_LENGTH];
 
     /* queued outgoing data in Server/TCP mode */
@@ -176,6 +178,8 @@
                                         *   instances. */
     struct multi_io *multi_io;         /**< I/O state and events tracker */
     struct ifconfig_pool *ifconfig_pool;
+    /* per-group --subnet-pool dynamic pools */
+    struct subnet_pool *subnet_pools;
     struct frequency_limit *new_connection_limiter;
     struct initial_packet_rate_limit *initial_rate_limiter;
     struct mroute_helper *route_helper;
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 87218d4..3f9f7de 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -443,6 +443,11 @@
     "                  If seconds=0, file will be treated as read-only.\n"
     "--ifconfig-ipv6-pool base-IP/bits : set aside an IPv6 network block\n"
     "                  to be dynamically allocated to connecting clients.\n"
+    "--subnet-pool tag network netmask [gateway] : Define a named pool of\n"
+    "                  addresses outside the --server network, selected per\n"
+    "                  client with --subnet-pool-tag.\n"
+    "--subnet-pool-tag tag : Assign this client to the --subnet-pool named tag.\n"
+    "                  Only valid in a client-specific config file.\n"
     "--ifconfig-push local remote-netmask : Push an ifconfig option to remote,\n"
     "                  overrides --ifconfig-pool dynamic allocation.\n"
     "                  Only valid in a client-specific config file.\n"
@@ -7631,6 +7636,33 @@
             goto err;
         }
     }
+    else if (streq(p[0], "subnet-pool") && p[1] && p[2] && p[3] && !p[5])
+    {
+        struct subnet_pool_def *sp;
+        in_addr_t network, netmask;
+
+        VERIFY_PERMISSION(OPT_P_GENERAL);
+        network = getaddr(GETADDR_HOST_ORDER | GETADDR_RESOLVE, p[2], 0, NULL, NULL);
+        netmask = getaddr(GETADDR_HOST_ORDER, p[3], 0, NULL, NULL);
+        if (!network || !netmask)
+        {
+            msg(msglevel, "cannot parse --subnet-pool addresses");
+            goto err;
+        }
+        ALLOC_OBJ_GC(sp, struct subnet_pool_def, &options->gc);
+        sp->tag = p[1];
+        sp->network = network & netmask;
+        sp->netmask = netmask;
+        sp->gateway = p[4] ? getaddr(GETADDR_HOST_ORDER | GETADDR_RESOLVE, p[4], 0, NULL, NULL)
+                           : (network & netmask) + 1;
+        sp->next = options->subnet_pools;
+        options->subnet_pools = sp;
+    }
+    else if (streq(p[0], "subnet-pool-tag") && p[1] && !p[2])
+    {
+        VERIFY_PERMISSION(OPT_P_INSTANCE);
+        options->subnet_pool_tag = p[1];
+    }
     else if (streq(p[0], "ifconfig-push-constraint") && p[1] && p[2] && !p[3])
     {
         in_addr_t network, netmask;
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index a111cf8..0c810b3 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -250,6 +250,17 @@
     struct verify_hash_list *next;
 };
 
+/* A named --subnet-pool: a subnet clients can be assigned from, referenced
+ * from a CCD file by its tag via --subnet-pool-tag. */
+struct subnet_pool_def
+{
+    struct subnet_pool_def *next;
+    const char *tag;
+    in_addr_t network;
+    in_addr_t netmask;
+    in_addr_t gateway;
+};
+
 /* Command line options */
 struct options
 {
@@ -515,6 +526,12 @@
     bool push_ifconfig_constraint_defined;
     in_addr_t push_ifconfig_constraint_network;
     in_addr_t push_ifconfig_constraint_netmask;
+    struct subnet_pool_def *subnet_pools; /* named pools (server config) */
+    const char *subnet_pool_tag;          /* which pool this client uses (CCD) */
+    bool subnet_pool_defined;             /* resolved from the tag at connect */
+    in_addr_t subnet_pool_network;
+    in_addr_t subnet_pool_netmask;
+    in_addr_t subnet_pool_gateway;
     bool push_ifconfig_ipv4_blocked;           /* IPv4 */
     bool push_ifconfig_ipv6_defined;           /* IPv6 */
     struct in6_addr push_ifconfig_ipv6_local;  /* IPv6 */

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1783?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ia1e7db95615d99ef9687de7c4e996dc8033e2631
Gerrit-Change-Number: 1783
Gerrit-PatchSet: 2
Gerrit-Owner: ordex <[email protected]>
Gerrit-Reviewer: flichtenheld <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
Gerrit-Attention: flichtenheld <[email protected]>
Gerrit-Attention: ordex <[email protected]>

_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel