[Openvpn-devel] [L] Change in openvpn[master]: PUSH_UPDATE: add DCO and iroutes support on server

["mrbff (Code Review)" <gerrit@...515...>]

[-- Attachment #1: Type: text/plain, Size: 28805 bytes --]

Attention is currently required from: plaisthos.

Hello plaisthos,

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

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

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


Change subject: PUSH_UPDATE: add DCO and iroutes support on server
......................................................................

PUSH_UPDATE: add DCO and iroutes support on server

Renamed update_vhash() to update_vaddr().
Moved the -ifconfig logic from send_single_push_update() to update_vaddr().
Changed canary logic to use the current value of ifconfig (if present) as
canary. Added logic in the update_vaddr() to not unlearn and re-learn the same
address.

Added logic to update the peer's virtual address(es) in the DCO to the current
value(s) after sending a PUSH_UPDATE message on Linux.
Added dco_multi_update_peer_addr() and dco_update_peer_addr() to do so.

Added logic to update iroutes after changing the peer's virtual address(es).
Divided dco_delete_iroutes() in _v4() and _v6() to better updating iroutes.

Change-Id: I88f37e47026a7d7ea0f7c6bde1d79d32c14e5297
Signed-off-by: Marco Baffo <marco@...2726...>
---
M src/openvpn/dco.c
M src/openvpn/dco.h
M src/openvpn/dco_freebsd.c
M src/openvpn/dco_linux.c
M src/openvpn/dco_win.c
M src/openvpn/multi.c
M src/openvpn/multi.h
M src/openvpn/push_util.c
M tests/unit_tests/openvpn/test_push_update_msg.c
9 files changed, 377 insertions(+), 142 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/50/1450/3

diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
index 6a1a5c9..eccfac2 100644
--- a/src/openvpn/dco.c
+++ b/src/openvpn/dco.c
@@ -658,6 +658,41 @@
     return 0;
 }

+int
+dco_multi_update_peer_addr(struct multi_context *m, struct multi_instance *mi, int flags)
+{
+    if (!flags)
+    {
+        return 0;
+    }
+
+    struct context *c = &mi->context;
+    int peer_id = c->c2.tls_multi->dco_peer_id;
+
+    /* In server mode we need to fetch the remote addresses from the push config */
+    struct in_addr vpn_ip4 = { 0 };
+    struct in_addr *vpn_addr4 = NULL;
+    if (c->c2.push_ifconfig_defined)
+    {
+        vpn_ip4.s_addr = htonl(c->c2.push_ifconfig_local);
+        vpn_addr4 = &vpn_ip4;
+    }
+
+    struct in6_addr *vpn_addr6 = NULL;
+    if (c->c2.push_ifconfig_ipv6_defined)
+    {
+        vpn_addr6 = &c->c2.push_ifconfig_ipv6_local;
+    }
+
+    int ret = dco_update_peer_addr(&c->c1.tuntap->dco, peer_id, vpn_addr4, vpn_addr6, flags);
+    if (ret < 0)
+    {
+        return ret;
+    }
+
+    return 0;
+}
+
 void
 dco_install_iroute(struct multi_context *m, struct multi_instance *mi, struct mroute_addr *addr)
 {
@@ -723,7 +758,7 @@
 }

 void
-dco_delete_iroutes(struct multi_context *m, struct multi_instance *mi)
+dco_delete_iroutes_v4(struct multi_context *m, struct multi_instance *mi)
 {
 #if defined(TARGET_LINUX) || defined(TARGET_FREEBSD) || defined(_WIN32)
     if (!dco_enabled(&m->top.options))
@@ -734,7 +769,7 @@

     struct context *c = &mi->context;

-    if (mi->context.c2.push_ifconfig_defined)
+    if (c->c2.push_ifconfig_defined)
     {
         for (const struct iroute *ir = c->options.iroutes; ir; ir = ir->next)
         {
@@ -742,7 +777,7 @@
             dco_win_del_iroute_ipv4(&c->c1.tuntap->dco, htonl(ir->network), ir->netbits);
 #else
             net_route_v4_del(&m->top.net_ctx, &ir->network, ir->netbits,
-                             &mi->context.c2.push_ifconfig_local, c->c1.tuntap->actual_name, 0,
+                             &c->c2.push_ifconfig_local, c->c1.tuntap->actual_name, 0,
                              DCO_IROUTE_METRIC);
 #endif
         }
@@ -750,7 +785,7 @@
 #if !defined(_WIN32)
         /* Check if we added a host route as the assigned client IP address was
          * not in the on link scope defined by --ifconfig */
-        in_addr_t ifconfig_local = mi->context.c2.push_ifconfig_local;
+        in_addr_t ifconfig_local = c->c2.push_ifconfig_local;

         if (multi_check_push_ifconfig_extra_route(mi, htonl(ifconfig_local)))
         {
@@ -761,8 +796,22 @@
         }
 #endif
     }
+#endif /* if defined(TARGET_LINUX) || defined(TARGET_FREEBSD) || defined(_WIN32) */
+}

-    if (mi->context.c2.push_ifconfig_ipv6_defined)
+void
+dco_delete_iroutes_v6(struct multi_context *m, struct multi_instance *mi)
+{
+#if defined(TARGET_LINUX) || defined(TARGET_FREEBSD) || defined(_WIN32)
+    if (!dco_enabled(&m->top.options))
+    {
+        return;
+    }
+    ASSERT(TUNNEL_TYPE(mi->context.c1.tuntap) == DEV_TYPE_TUN);
+
+    struct context *c = &mi->context;
+
+    if (c->c2.push_ifconfig_ipv6_defined)
     {
         for (const struct iroute_ipv6 *ir6 = c->options.iroutes_ipv6; ir6; ir6 = ir6->next)
         {
@@ -770,7 +819,7 @@
             dco_win_del_iroute_ipv6(&c->c1.tuntap->dco, ir6->network, ir6->netbits);
 #else
             net_route_v6_del(&m->top.net_ctx, &ir6->network, ir6->netbits,
-                             &mi->context.c2.push_ifconfig_ipv6_local, c->c1.tuntap->actual_name, 0,
+                             &c->c2.push_ifconfig_ipv6_local, c->c1.tuntap->actual_name, 0,
                              DCO_IROUTE_METRIC);
 #endif
         }
@@ -778,7 +827,7 @@
         /* Checked if we added a host route as the assigned client IP address was
          * outside the --ifconfig-ipv6 tun interface config */
 #if !defined(_WIN32)
-        struct in6_addr *dest = &mi->context.c2.push_ifconfig_ipv6_local;
+        struct in6_addr *dest = &c->c2.push_ifconfig_ipv6_local;
         if (multi_check_push_ifconfig_ipv6_extra_route(mi, dest))
         {
             /* On windows we do not install these routes, so we also do not need to delete them */
diff --git a/src/openvpn/dco.h b/src/openvpn/dco.h
index cd6e32a..3bc28a8 100644
--- a/src/openvpn/dco.h
+++ b/src/openvpn/dco.h
@@ -191,6 +191,30 @@
                  int keepalive_timeout, int mss);

 /**
+ * Update the peer's virtual address(es) in the DCO to the current value(s)
+ *
+ * @param dco       DCO device contect
+ * @param peerid    the ID of the peer to be modified
+ * @param vpn_ipv4  new IPv4 or NULL to remove the outdated one, only used when `(1 << AF_INET)` flag is present
+ * @param vpn_ipv6  new IPv6 or NULL to remove the outdated one, only used when `(1 << AF_INET6)` flag is present
+ * @param flags     `(1 << AF_INET)`, `(1 << AF_INET6)` or `(1 << AF_INET) | (1 << AF_INET6)` depending on wich address has changed
+ * @return int      the DCO message status on success or a negative error code otherwise
+ */
+int
+dco_update_peer_addr(dco_context_t *dco, unsigned int peerid, struct in_addr *vpn_ipv4, struct in6_addr *vpn_ipv6, int flags);
+
+/**
+ * Update the peer's virtual address(es) in the DCO to the current value(s)
+ *
+ * @param m         server multi context
+ * @param mi        peer multi instnace
+ * @param flags     `(1 << AF_INET)`, `(1 << AF_INET6)` or `(1 << AF_INET) | (1 << AF_INET6)` depending on wich address has changed
+ * @return int      0 on success or a negative error code otherwise
+ */
+int
+dco_multi_update_peer_addr(struct multi_context *m, struct multi_instance *mi, int flags);
+
+/**
  * Remove a peer from DCO
  *
  * @param c         the main instance context of the peer to remove
@@ -218,12 +242,20 @@
                         struct mroute_addr *addr);

 /**
- * Remove all routes added through the specified client
+ * Remove all IPv4 routes added through the specified client
  *
  * @param m         the server context
  * @param mi        the client instance for which routes have to be removed
  */
-void dco_delete_iroutes(struct multi_context *m, struct multi_instance *mi);
+void dco_delete_iroutes_v4(struct multi_context *m, struct multi_instance *mi);
+
+/**
+ * Remove all IPv6 routes added through the specified client
+ *
+ * @param m         the server context
+ * @param mi        the client instance for which routes have to be removed
+ */
+void dco_delete_iroutes_v6(struct multi_context *m, struct multi_instance *mi);

 /**
  * Update traffic statistics for all peers
@@ -361,7 +393,12 @@
 }

 static inline void
-dco_delete_iroutes(struct multi_context *m, struct multi_instance *mi)
+dco_delete_iroutes_v4(struct multi_context *m, struct multi_instance *mi)
+{
+}
+
+static inline void
+dco_delete_iroutes_v6(struct multi_context *m, struct multi_instance *mi)
 {
 }

diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
index d1ad092..af20ab8 100644
--- a/src/openvpn/dco_freebsd.c
+++ b/src/openvpn/dco_freebsd.c
@@ -559,6 +559,12 @@
     return ret;
 }

+int
+dco_update_peer_addr(dco_context_t *dco, unsigned int peerid, struct in_addr *vpn_ipv4, struct in6_addr *vpn_ipv6, int flags)
+{
+    return 0;
+}
+
 static void
 dco_update_peer_stat(struct multi_context *m, uint32_t peerid, const nvlist_t *nvl)
 {
diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 2664029..eb9e4f9 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -684,6 +684,50 @@
     return ret;
 }

+int
+dco_update_peer_addr(dco_context_t *dco, unsigned int peerid, struct in_addr *vpn_ipv4, struct in6_addr *vpn_ipv6, int flags)
+{
+    msg(D_DCO_DEBUG, "%s: peer-id %d", __func__, peerid);
+
+    if (!flags)
+    {
+        return 0;
+    }
+
+    struct nl_msg *nl_msg = ovpn_dco_nlmsg_create(dco, OVPN_CMD_PEER_SET);
+    if (!nl_msg)
+    {
+        return -ENOMEM;
+    }
+
+    struct nlattr *attr = nla_nest_start(nl_msg, OVPN_A_PEER);
+    int ret = -EMSGSIZE;
+    NLA_PUT_U32(nl_msg, OVPN_A_PEER_ID, peerid);
+
+    if (flags & (1 << AF_INET))
+    {
+        NLA_PUT_U32(nl_msg, OVPN_A_PEER_VPN_IPV4, vpn_ipv4 ? vpn_ipv4->s_addr : 0);
+    }
+
+    if (flags & (1 << AF_INET6))
+    {
+        struct in6_addr addr6;
+        if (!vpn_ipv6)
+        {
+            CLEAR(addr6);
+            vpn_ipv6 = &addr6;
+        }
+        NLA_PUT(nl_msg, OVPN_A_PEER_VPN_IPV6, sizeof(struct in6_addr), vpn_ipv6);
+    }
+
+    nla_nest_end(nl_msg, attr);
+    ret = ovpn_nl_msg_send(dco, nl_msg, __func__);
+
+nla_put_failure:
+    nlmsg_free(nl_msg);
+    return ret;
+}
+
 /* This function parses the reply provided by the kernel to the CTRL_CMD_GETFAMILY
  * message. We parse the reply and we retrieve the multicast group ID associated
  * with the "ovpn-dco" netlink family.
diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c
index 94f043f..1a11927 100644
--- a/src/openvpn/dco_win.c
+++ b/src/openvpn/dco_win.c
@@ -526,6 +526,12 @@
 }

 int
+dco_update_peer_addr(dco_context_t *dco, unsigned int peerid, struct in_addr *vpn_ipv4, struct in6_addr *vpn_ipv6, int flags)
+{
+    return 0;
+}
+
+int
 dco_new_key(dco_context_t *dco, unsigned int peerid, int keyid, dco_key_slot_t slot,
             const uint8_t *encrypt_key, const uint8_t *encrypt_iv, const uint8_t *decrypt_key,
             const uint8_t *decrypt_iv, const char *ciphername, bool epoch)
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 329d0a3..878871f 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -476,6 +476,40 @@
     set_prefix(mi);
 }

+static void
+multi_del_iroutes_v4(struct multi_context *m, struct multi_instance *mi)
+{
+    if (TUNNEL_TYPE(mi->context.c1.tuntap) != DEV_TYPE_TUN)
+    {
+        return;
+    }
+
+    const struct iroute *ir;
+
+    dco_delete_iroutes_v4(m, mi);
+    for (ir = mi->context.options.iroutes; ir != NULL; ir = ir->next)
+    {
+        mroute_helper_del_iroute46(m->route_helper, ir->netbits);
+    }
+}
+
+static void
+multi_del_iroutes_v6(struct multi_context *m, struct multi_instance *mi)
+{
+    if (TUNNEL_TYPE(mi->context.c1.tuntap) != DEV_TYPE_TUN)
+    {
+        return;
+    }
+
+    const struct iroute_ipv6 *ir6;
+
+    dco_delete_iroutes_v6(m, mi);
+    for (ir6 = mi->context.options.iroutes_ipv6; ir6 != NULL; ir6 = ir6->next)
+    {
+        mroute_helper_del_iroute46(m->route_helper, ir6->netbits);
+    }
+}
+
 /*
  * Tell the route helper about deleted iroutes so
  * that it can update its mask of currently used
@@ -484,23 +518,8 @@
 static void
 multi_del_iroutes(struct multi_context *m, struct multi_instance *mi)
 {
-    const struct iroute *ir;
-    const struct iroute_ipv6 *ir6;
-
-    dco_delete_iroutes(m, mi);
-
-    if (TUNNEL_TYPE(mi->context.c1.tuntap) == DEV_TYPE_TUN)
-    {
-        for (ir = mi->context.options.iroutes; ir != NULL; ir = ir->next)
-        {
-            mroute_helper_del_iroute46(m->route_helper, ir->netbits);
-        }
-
-        for (ir6 = mi->context.options.iroutes_ipv6; ir6 != NULL; ir6 = ir6->next)
-        {
-            mroute_helper_del_iroute46(m->route_helper, ir6->netbits);
-        }
-    }
+    multi_del_iroutes_v4(m, mi);
+    multi_del_iroutes_v6(m, mi);
 }

 static void
@@ -1282,6 +1301,64 @@
     return owner;
 }

+static void
+multi_add_iroutes_v4(struct multi_context *m, struct multi_instance *mi)
+{
+    if (TUNNEL_TYPE(mi->context.c1.tuntap) != DEV_TYPE_TUN)
+    {
+        return;
+    }
+
+    struct gc_arena gc = gc_new();
+    const struct iroute *ir;
+
+    mi->did_iroutes = true;
+    for (ir = mi->context.options.iroutes; ir != NULL; ir = ir->next)
+    {
+        if (ir->netbits >= 0)
+        {
+            msg(D_MULTI_LOW, "MULTI: internal route %s/%d -> %s",
+                print_in_addr_t(ir->network, 0, &gc), ir->netbits,
+                multi_instance_string(mi, false, &gc));
+        }
+        else
+        {
+            msg(D_MULTI_LOW, "MULTI: internal route %s -> %s",
+                print_in_addr_t(ir->network, 0, &gc), multi_instance_string(mi, false, &gc));
+        }
+
+        mroute_helper_add_iroute46(m->route_helper, ir->netbits);
+
+        multi_learn_in_addr_t(m, mi, ir->network, ir->netbits, false);
+    }
+    gc_free(&gc);
+}
+
+static void
+multi_add_iroutes_v6(struct multi_context *m, struct multi_instance *mi)
+{
+    if (TUNNEL_TYPE(mi->context.c1.tuntap) != DEV_TYPE_TUN)
+    {
+        return;
+    }
+
+    struct gc_arena gc = gc_new();
+    const struct iroute_ipv6 *ir6;
+
+    mi->did_iroutes = true;
+    for (ir6 = mi->context.options.iroutes_ipv6; ir6 != NULL; ir6 = ir6->next)
+    {
+        msg(D_MULTI_LOW, "MULTI: internal route %s/%d -> %s",
+            print_in6_addr(ir6->network, 0, &gc), ir6->netbits,
+            multi_instance_string(mi, false, &gc));
+
+        mroute_helper_add_iroute46(m->route_helper, ir6->netbits);
+
+        multi_learn_in6_addr(m, mi, ir6->network, ir6->netbits, false);
+    }
+    gc_free(&gc);
+}
+
 /*
  * A new client has connected, add routes (server -> client)
  * to internal routing table.
@@ -1289,42 +1366,8 @@
 static void
 multi_add_iroutes(struct multi_context *m, struct multi_instance *mi)
 {
-    struct gc_arena gc = gc_new();
-    const struct iroute *ir;
-    const struct iroute_ipv6 *ir6;
-    if (TUNNEL_TYPE(mi->context.c1.tuntap) == DEV_TYPE_TUN)
-    {
-        mi->did_iroutes = true;
-        for (ir = mi->context.options.iroutes; ir != NULL; ir = ir->next)
-        {
-            if (ir->netbits >= 0)
-            {
-                msg(D_MULTI_LOW, "MULTI: internal route %s/%d -> %s",
-                    print_in_addr_t(ir->network, 0, &gc), ir->netbits,
-                    multi_instance_string(mi, false, &gc));
-            }
-            else
-            {
-                msg(D_MULTI_LOW, "MULTI: internal route %s -> %s",
-                    print_in_addr_t(ir->network, 0, &gc), multi_instance_string(mi, false, &gc));
-            }
-
-            mroute_helper_add_iroute46(m->route_helper, ir->netbits);
-
-            multi_learn_in_addr_t(m, mi, ir->network, ir->netbits, false);
-        }
-        for (ir6 = mi->context.options.iroutes_ipv6; ir6 != NULL; ir6 = ir6->next)
-        {
-            msg(D_MULTI_LOW, "MULTI: internal route %s/%d -> %s",
-                print_in6_addr(ir6->network, 0, &gc), ir6->netbits,
-                multi_instance_string(mi, false, &gc));
-
-            mroute_helper_add_iroute46(m->route_helper, ir6->netbits);
-
-            multi_learn_in6_addr(m, mi, ir6->network, ir6->netbits, false);
-        }
-    }
-    gc_free(&gc);
+    multi_add_iroutes_v4(m, mi);
+    multi_add_iroutes_v6(m, mi);
 }

 /*
@@ -4327,9 +4370,10 @@
 }

 /* Function to unlearn previous ifconfig of a client in the server multi_context after a PUSH_UPDATE */
-void
+static void
 unlearn_ifconfig(struct multi_context *m, struct multi_instance *mi)
 {
+    multi_del_iroutes_v4(m, mi);
     in_addr_t old_addr = 0;
     old_addr = htonl(mi->context.c2.push_ifconfig_local);
     multi_unlearn_in_addr_t(m, mi, old_addr);
@@ -4339,9 +4383,10 @@
 }

 /* Function to unlearn previous ifconfig-ipv6 of a client in the server multi_context after a PUSH_UPDATE */
-void
+static void
 unlearn_ifconfig_ipv6(struct multi_context *m, struct multi_instance *mi)
 {
+    multi_del_iroutes_v6(m, mi);
     struct in6_addr old_addr6;
     CLEAR(old_addr6);
     old_addr6 = mi->context.c2.push_ifconfig_ipv6_local;
@@ -4352,59 +4397,118 @@
 }

 /**
- * Update the vhash with new IP/IPv6 addresses in the multi_context when a
- * push-update message containing ifconfig/ifconfig-ipv6 options is sent
- * from the server.
+ * When a push-update message containing ifconfig/ifconfig-ipv6 options is sent
+ * from the server, update the vhash with new IP/IPv6 addresses in the
+ * multi_context and update the virtual peer addresses in the DCO.
+ * Also remove the iroutes and readd it with the new IP if needed.
  *
  * @param m         The multi_context
  * @param mi        The multi_instance of the client we are updating
- * @param new_ip    The new IPv4 address or NULL if no change
- * @param new_ipv6  The new IPv6 address or NULL if no change
+ * @param new_ip    The new IPv4 address, NULL for -ifconfig or the current IPv4 if no change
+ * @param new_ipv6  The new IPv6 address, NULL for -ifconfig-ipv6 or the current IPv6 if no change
  */
 void
-update_vhash(struct multi_context *m, struct multi_instance *mi, const char *new_ip, const char *new_ipv6)
+update_vaddr(struct multi_context *m, struct multi_instance *mi, const char *new_ip, const char *new_ipv6)
 {
+    /* to avoid resetting an IP to its current value, the dco_update_peer_addr() function used to update the DCO,
+     * needs to know if actually an IPv4 or an IPv6 has been modified
+     */
+    int update_flags = 0;
+
+    /* -ifconfig */
+    if (!new_ip && mi->context.c2.push_ifconfig_defined)
+    {
+        unlearn_ifconfig(m, mi);
+        update_flags |= (1 << AF_INET);
+    }
+    /* -ifconfig-ipv6 */
+    if (!new_ipv6 && mi->context.c2.push_ifconfig_ipv6_defined)
+    {
+        unlearn_ifconfig_ipv6(m, mi);
+        update_flags |= (1 << AF_INET6);
+    }
+
     if (new_ip)
     {
-        /* Remove old IP */
-        if (mi->context.c2.push_ifconfig_defined)
-        {
-            unlearn_ifconfig(m, mi);
-        }
-
-        /* Add new IP */
         struct in_addr new_addr;
         CLEAR(new_addr);
-        if (inet_pton(AF_INET, new_ip, &new_addr) == 1
-            && multi_learn_in_addr_t(m, mi, ntohl(new_addr.s_addr), -1, true))
+        if (inet_pton(AF_INET, new_ip, &new_addr) == 1)
         {
-            mi->context.c2.push_ifconfig_defined = true;
-            mi->context.c2.push_ifconfig_local = ntohl(new_addr.s_addr);
-            /* set our client's VPN endpoint for status reporting purposes */
-            mi->reporting_addr = mi->context.c2.push_ifconfig_local;
+            new_addr.s_addr = ntohl(new_addr.s_addr);
+            /* if no previous IP or the new IP is different */
+            if (!mi->context.c2.push_ifconfig_defined
+                || (mi->context.c2.push_ifconfig_defined
+                    && new_addr.s_addr != mi->context.c2.push_ifconfig_local))
+            {
+                /* Remove old IP */
+                if (mi->context.c2.push_ifconfig_defined)
+                {
+                    unlearn_ifconfig(m, mi);
+                }
+
+                /* Add new IP */
+                if (multi_learn_in_addr_t(m, mi, new_addr.s_addr, -1, true))
+                {
+                    mi->context.c2.push_ifconfig_defined = true;
+                    mi->context.c2.push_ifconfig_local = new_addr.s_addr;
+                    /* set our client's VPN endpoint for status reporting purposes */
+                    mi->reporting_addr = mi->context.c2.push_ifconfig_local;
+                    update_flags |= (1 << AF_INET);
+                }
+            }
         }
     }

     if (new_ipv6)
     {
-        /* Remove old IPv6 */
-        if (mi->context.c2.push_ifconfig_ipv6_defined)
-        {
-            unlearn_ifconfig_ipv6(m, mi);
-        }
-
-        /* Add new IPv6 */
         struct in6_addr new_addr6;
         CLEAR(new_addr6);
-        if (inet_pton(AF_INET6, new_ipv6, &new_addr6) == 1
-            && multi_learn_in6_addr(m, mi, new_addr6, -1, true))
+        if (inet_pton(AF_INET6, new_ipv6, &new_addr6) == 1)
         {
-            mi->context.c2.push_ifconfig_ipv6_defined = true;
-            mi->context.c2.push_ifconfig_ipv6_local = new_addr6;
-            /* set our client's VPN endpoint for status reporting purposes */
-            mi->reporting_addr_ipv6 = mi->context.c2.push_ifconfig_ipv6_local;
+            /* if no previous IPv6 or the new IPv6 is different */
+            if (!mi->context.c2.push_ifconfig_ipv6_defined
+                || (mi->context.c2.push_ifconfig_ipv6_defined
+                    && memcmp(&new_addr6, &mi->context.c2.push_ifconfig_ipv6_local, sizeof(struct in6_addr))))
+            {
+                /* Remove old IPv6 */
+                if (mi->context.c2.push_ifconfig_ipv6_defined)
+                {
+                    unlearn_ifconfig_ipv6(m, mi);
+                }
+
+                /* Add new IPv6 */
+                if (multi_learn_in6_addr(m, mi, new_addr6, -1, true))
+                {
+                    mi->context.c2.push_ifconfig_ipv6_defined = true;
+                    mi->context.c2.push_ifconfig_ipv6_local = new_addr6;
+                    /* set our client's VPN endpoint for status reporting purposes */
+                    mi->reporting_addr_ipv6 = mi->context.c2.push_ifconfig_ipv6_local;
+                    update_flags |= (1 << AF_INET);
+                }
+            }
         }
     }
+
+    if (!update_flags)
+    {
+        return;
+    }
+
+    /* we update the virtual addresses of the peer in the DCO */
+    if (dco_enabled(&m->top.options))
+    {
+        dco_multi_update_peer_addr(m, mi, update_flags);
+    }
+
+    /* we add the iroutes with the new VPN IP */
+    if (update_flags & (1 << AF_INET) && new_ip)
+    {
+        multi_add_iroutes_v4(m, mi);
+    }
+    if (update_flags & (1 << AF_INET6) && new_ipv6)
+    {
+        multi_add_iroutes_v6(m, mi);
+    }
 }

 bool
diff --git a/src/openvpn/multi.h b/src/openvpn/multi.h
index 1209dfb..7bda347 100644
--- a/src/openvpn/multi.h
+++ b/src/openvpn/multi.h
@@ -714,8 +714,6 @@
 #endif

 void
-update_vhash(struct multi_context *m, struct multi_instance *mi, const char *new_ip, const char *new_ipv6);
-void unlearn_ifconfig(struct multi_context *m, struct multi_instance *mi);
-void unlearn_ifconfig_ipv6(struct multi_context *m, struct multi_instance *mi);
+update_vaddr(struct multi_context *m, struct multi_instance *mi, const char *new_ip, const char *new_ipv6);

 #endif /* MULTI_H */
diff --git a/src/openvpn/push_util.c b/src/openvpn/push_util.c
index 51c7b5f..bcab1d0 100644
--- a/src/openvpn/push_util.c
+++ b/src/openvpn/push_util.c
@@ -151,14 +151,35 @@
     struct options o;
     CLEAR(o);

-    /* Set canary values to detect ifconfig options in push-update messages.
-     * These placeholder strings will be overwritten to NULL by the option
-     * parser if -ifconfig or -ifconfig-ipv6 options are present in the
-     * push-update.
+    /* Set canary values to detect -ifconfig and -ifconfig-ipv6 options in
+     * push-update messages. These placeholder strings will be overwritten
+     * to NULL by the option parser if -ifconfig or -ifconfig-ipv6 options
+     * are present in the push-update, by the a new value if present, or
+     * they will remain equals to the current values otherwise.
+     * If the current values are not defined they will remain NULL.
      */
-    const char *canary = "canary";
-    o.ifconfig_local = canary;
-    o.ifconfig_ipv6_local = canary;
+
+    char ifconfig_local[INET_ADDRSTRLEN];
+    struct in_addr addr4 = { .s_addr = htonl(mi->context.c2.push_ifconfig_local) };
+    if (mi->context.c2.push_ifconfig_defined)
+    {
+        o.ifconfig_local = inet_ntop(AF_INET, &addr4, ifconfig_local, sizeof(ifconfig_local));
+        if (!o.ifconfig_local)
+        {
+            return false;
+        }
+    }
+
+    char ifconfig_ipv6_local[INET6_ADDRSTRLEN];
+    if (mi->context.c2.push_ifconfig_ipv6_defined)
+    {
+        o.ifconfig_ipv6_local = inet_ntop(AF_INET6, &mi->context.c2.push_ifconfig_ipv6_local,
+                                          ifconfig_ipv6_local, sizeof(ifconfig_ipv6_local));
+        if (!o.ifconfig_ipv6_local)
+        {
+            return false;
+        }
+    }

     struct buffer_entry *e = msgs->head;
     while (e)
@@ -195,28 +216,8 @@

     if (option_types_found & OPT_P_UP)
     {
-        /* -ifconfig */
-        if (!o.ifconfig_local && mi->context.c2.push_ifconfig_defined)
-        {
-            unlearn_ifconfig(m, mi);
-        }
-        /* -ifconfig-ipv6 */
-        if (!o.ifconfig_ipv6_local && mi->context.c2.push_ifconfig_ipv6_defined)
-        {
-            unlearn_ifconfig_ipv6(m, mi);
-        }
-
-        if (o.ifconfig_local && !strcmp(o.ifconfig_local, canary))
-        {
-            o.ifconfig_local = NULL;
-        }
-        if (o.ifconfig_ipv6_local && !strcmp(o.ifconfig_ipv6_local, canary))
-        {
-            o.ifconfig_ipv6_local = NULL;
-        }
-
-        /* new ifconfig or new ifconfig-ipv6 */
-        update_vhash(m, mi, o.ifconfig_local, o.ifconfig_ipv6_local);
+        /* ifconfig or ifconfig-ipv6 */
+        update_vaddr(m, mi, o.ifconfig_local, o.ifconfig_ipv6_local);
     }

     return true;
@@ -253,12 +254,14 @@
 static int
 send_push_update(struct multi_context *m, const void *target, const char *msg, const push_update_type type, const size_t push_bundle_size)
 {
+#if !defined(TARGET_LINUX)
     if (dco_enabled(&m->top.options))
     {
         msg(M_WARN, "WARN: PUSH_UPDATE messages cannot currently be sent while DCO is enabled."
                     " To send a PUSH_UPDATE message, be sure to use the --disable-dco option.");
         return 0;
     }
+#endif

     if (!msg || !*msg || !m || (!target && type != UPT_BROADCAST))
     {
diff --git a/tests/unit_tests/openvpn/test_push_update_msg.c b/tests/unit_tests/openvpn/test_push_update_msg.c
index 9b7978e..2d2fc93 100644
--- a/tests/unit_tests/openvpn/test_push_update_msg.c
+++ b/tests/unit_tests/openvpn/test_push_update_msg.c
@@ -31,19 +31,7 @@
 }

 void
-unlearn_ifconfig(struct multi_context *m, struct multi_instance *mi)
-{
-    return;
-}
-
-void
-unlearn_ifconfig_ipv6(struct multi_context *m, struct multi_instance *mi)
-{
-    return;
-}
-
-void
-update_vhash(struct multi_context *m, struct multi_instance *mi, const char *new_ip, const char *new_ipv6)
+update_vaddr(struct multi_context *m, struct multi_instance *mi, const char *new_ip, const char *new_ipv6)
 {
     return;
 }

--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1450?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: I88f37e47026a7d7ea0f7c6bde1d79d32c14e5297
Gerrit-Change-Number: 1450
Gerrit-PatchSet: 3
Gerrit-Owner: mrbff <marco@...2726...>
Gerrit-Reviewer: plaisthos <arne-openvpn@...1227...>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: plaisthos <arne-openvpn@...1227...>

[-- Attachment #2: Type: text/html, Size: 60832 bytes --]