DCO: require valid netbits setting for non-primary iroutes.

The existing DCO code had extra logic for "if this is not
MR_WITH_NETBITS, set 32/128 as address length", but only for
iroute addition.  For iroute deletion, this was missing, and
subsequently iroute deletion for IPv4 host routes failed on
FreeBSD DCO (commit 3433577a99).

Iroute handling differenciates between "primary" iroutes (coming
from anm IP pool or ccd/ifconfig-push), and "non-primary" iroutes,
coming from --iroute and --iroute-ipv6 statements in per-client config.

"Primary" iroutes always use "-1" for their netbits, but since these
are not installed via DCO, this is of no concern here.  Whether these
can and should be changed needs further study on internal route
learning and cleanup.

Refactor options.c and multi.c to ensure that netbits is always set
for non-primary iroutes - and ASSERT() on this in the DCO path, so we can
find out if there might be other code violating this.

Change options.c::option_iroute() to always set netbits=32 for IPv4
host routes (options_iroute_ipv6() never differenciated).  Since
netmask_to_netbits() also insists on "-1" for host routes, change
to netmask_to_netbits2().

Remove all the extra MR_WITH_NETBITS logic from dco.c, where it should
have never appeared.

Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Kristof Provost <kprovost@netgate.com>
Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <20220820140124.11325-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25044.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
index ad35fee83718a00fa63512d8a5f44783c0fe3c8c..78023eea995bbe1079daeb38b85af84a6e7ec74f 100644 (file)
--- a/src/openvpn/dco.c
+++ b/src/openvpn/dco.c
@@ -603,26 +603,14 @@ dco_install_iroute(struct multi_context *m, struct multi_instance *mi,
 
     if (addrtype == MR_ADDR_IPV6)
     {
-        int netbits = 128;
-        if (addr->type & MR_WITH_NETBITS)
-        {
-            netbits = addr->netbits;
-        }
-
-        net_route_v6_add(&m->top.net_ctx, &addr->v6.addr, netbits,
+        net_route_v6_add(&m->top.net_ctx, &addr->v6.addr, addr->netbits,
                          &mi->context.c2.push_ifconfig_ipv6_local, dev, 0,
                          DCO_IROUTE_METRIC);
     }
     else if (addrtype == MR_ADDR_IPV4)
     {
-        int netbits = 32;
-        if (addr->type & MR_WITH_NETBITS)
-        {
-            netbits = addr->netbits;
-        }
-
         in_addr_t dest = htonl(addr->v4.addr);
-        net_route_v4_add(&m->top.net_ctx, &dest, netbits,
+        net_route_v4_add(&m->top.net_ctx, &dest, addr->netbits,
                          &mi->context.c2.push_ifconfig_local, dev, 0,
                          DCO_IROUTE_METRIC);
     }

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 95414429f3e212cf0fe4cbf6cd23949cdf6aa87a..b58bea7b943022e10c81d3b7bea1014cfdbe7305 100644 (file)
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -1241,6 +1241,7 @@ multi_learn_in_addr_t(struct multi_context *m,
         /* "primary" is the VPN ifconfig address of the peer and already
          * known to DCO, so only install "extra" iroutes (primary = false)
          */
+        ASSERT(netbits >= 0);           /* DCO requires populated netbits */
         dco_install_iroute(m, mi, &addr);
     }
 
@@ -1280,6 +1281,7 @@ multi_learn_in6_addr(struct multi_context *m,
         /* "primary" is the VPN ifconfig address of the peer and already
          * known to DCO, so only install "extra" iroutes (primary = false)
          */
+        ASSERT(netbits >= 0);           /* DCO requires populated netbits */
         dco_install_iroute(m, mi, &addr);
     }
 

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index f82457e47a9e69eefb801f3aad594ee5755da69a..a296086d4f0db917cf4c15fb37c5c13747339ed7 100644 (file)
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1572,12 +1572,14 @@ option_iroute(struct options *o,
 
     ALLOC_OBJ_GC(ir, struct iroute, &o->gc);
     ir->network = getaddr(GETADDR_HOST_ORDER, network_str, 0, NULL, NULL);
-    ir->netbits = -1;
+    ir->netbits = 32;           /* host route if no netmask given */
 
     if (netmask_str)
     {
         const in_addr_t netmask = getaddr(GETADDR_HOST_ORDER, netmask_str, 0, NULL, NULL);
-        if (!netmask_to_netbits(ir->network, netmask, &ir->netbits))
+        ir->netbits = netmask_to_netbits2(netmask);
+
+        if (ir->netbits < 0)
         {
             msg(msglevel, "in --iroute %s %s : Bad network/subnet specification",
                 network_str,