]> git.ipfire.org Git - thirdparty/openvpn.git/commitdiff
PUSH_UPDATE server: remove old IP(s) from vhash after sending a message containing...
authorMarco Baffo <marco@mandelbit.com>
Fri, 17 Oct 2025 20:19:12 +0000 (22:19 +0200)
committerGert Doering <gert@greenie.muc.de>
Fri, 17 Oct 2025 20:24:11 +0000 (22:24 +0200)
When sending a PUSH_UPDATE containing an ifconfig(-ipv6) option, we must add the new IP to the
multi_context vhash (hash table of the clients indexed by virtual IPs). Now in addition to
adding new client IPs, old IPs are also removed from vhash, allowing for a more complete update.

Change-Id: I07a8ddd9026eef64b6f5abde98702a9801616a5f
Signed-off-by: Marco Baffo <marco@mandelbit.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1253
Message-Id: <20251017201916.21697-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg33412.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
src/openvpn/multi.c
src/openvpn/multi.h
src/openvpn/push.c
src/openvpn/push.h
src/openvpn/push_util.c
tests/unit_tests/openvpn/test_push_update_msg.c

index 2863ff173e4b29ed8a409da795c1c8f31b74a835..558314cb068b9cf5bfe9c45a19173448d769e896 100644 (file)
@@ -4269,46 +4269,144 @@ tunnel_server(struct context *top)
     close_instance(top);
 }
 
+/* Searches for the address and deletes it if it is owned by the multi_instance */
+static void
+multi_unlearn_addr(struct multi_context *m, struct multi_instance *mi, const struct mroute_addr *addr)
+{
+    struct hash_element *he;
+    const uint32_t hv = hash_value(m->vhash, addr);
+    struct hash_bucket *bucket = hash_bucket(m->vhash, hv);
+    struct multi_route *r = NULL;
+
+    /* if route currently exists, get the instance which owns it */
+    he = hash_lookup_fast(m->vhash, bucket, addr, hv);
+    if (he)
+    {
+        r = (struct multi_route *)he->value;
+    }
+
+    /* if the route does not exist or exists but is not owned by the current instance, return */
+    if (!r || r->instance != mi)
+    {
+        return;
+    }
+
+    struct gc_arena gc = gc_new();
+    msg(D_MULTI_LOW, "MULTI: Unlearn: %s -> %s", mroute_addr_print(&r->addr, &gc), multi_instance_string(mi, false, &gc));
+    learn_address_script(m, NULL, "delete", &r->addr);
+    hash_remove_by_value(m->vhash, r);
+    multi_route_del(r);
+
+    gc_free(&gc);
+}
+
+/**
+ * @param m     The multi_context
+ * @param mi    The multi_instance of the client we are updating
+ * @param a     The new IPv4 address in network byte order
+ */
+static void
+multi_unlearn_in_addr_t(struct multi_context *m, struct multi_instance *mi, in_addr_t a)
+{
+    struct mroute_addr addr;
+    CLEAR(addr);
+
+    addr.type = MR_ADDR_IPV4;
+    addr.len = 4;
+    addr.v4.addr = a;
+
+    multi_unlearn_addr(m, mi, &addr);
+}
+
+/**
+ * @param m     The multi_context
+ * @param mi    The multi_instance of the client we are updating
+ * @param a6    The new IPv6 address
+ */
+static void
+multi_unlearn_in6_addr(struct multi_context *m, struct multi_instance *mi, struct in6_addr a6)
+{
+    struct mroute_addr addr;
+    CLEAR(addr);
+
+    addr.type = MR_ADDR_IPV6;
+    addr.len = 16;
+    addr.v6.addr = a6;
+
+    multi_unlearn_addr(m, mi, &addr);
+}
+
+/* Function to unlearn previous ifconfig of a client in the server multi_context after a PUSH_UPDATE */
+void
+unlearn_ifconfig(struct multi_context *m, struct multi_instance *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);
+    mi->context.c2.push_ifconfig_defined = false;
+    mi->context.c2.push_ifconfig_local = 0;
+}
+
+/* Function to unlearn previous ifconfig-ipv6 of a client in the server multi_context after a PUSH_UPDATE */
+void
+unlearn_ifconfig_ipv6(struct multi_context *m, struct multi_instance *mi)
+{
+    struct in6_addr old_addr6;
+    CLEAR(old_addr6);
+    old_addr6 = mi->context.c2.push_ifconfig_ipv6_local;
+    multi_unlearn_in6_addr(m, mi, old_addr6);
+    mi->context.c2.push_ifconfig_ipv6_defined = false;
+    CLEAR(mi->context.c2.push_ifconfig_ipv6_local);
+}
+
 /**
  * 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. This function should be called after a push-update
- * and old_ip/old_ipv6 are the previous addresses of the client in
- * ctx->options.ifconfig_local and ctx->options.ifconfig_ipv6_local.
+ * from the server.
+ *
+ * @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
  */
 void
-update_vhash(struct multi_context *m, struct multi_instance *mi, const char *old_ip, const char *old_ipv6)
+update_vhash(struct multi_context *m, struct multi_instance *mi, const char *new_ip, const char *new_ipv6)
 {
-    struct in_addr addr;
-    struct in6_addr new_ipv6;
-
-    if ((mi->context.options.ifconfig_local && (!old_ip || strcmp(old_ip, mi->context.options.ifconfig_local)))
-        && inet_pton(AF_INET, mi->context.options.ifconfig_local, &addr) == 1)
+    if (new_ip)
     {
-        in_addr_t new_ip = ntohl(addr.s_addr);
+        /* Remove old IP */
+        if (mi->context.c2.push_ifconfig_defined)
+        {
+            unlearn_ifconfig(m, mi);
+        }
 
         /* Add new IP */
-        multi_learn_in_addr_t(m, mi, new_ip, -1, true);
+        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))
+        {
+            mi->context.c2.push_ifconfig_defined = true;
+            mi->context.c2.push_ifconfig_local = ntohl(new_addr.s_addr);
+        }
     }
 
-    /* TO DO:
-     *  else if (old_ip)
-     *  {
-     *      // remove old IP
-     *  }
-     */
-
-    if ((mi->context.options.ifconfig_ipv6_local && (!old_ipv6 || strcmp(old_ipv6, mi->context.options.ifconfig_ipv6_local)))
-        && inet_pton(AF_INET6, mi->context.options.ifconfig_ipv6_local, &new_ipv6) == 1)
+    if (new_ipv6)
     {
+        /* Remove old IPv6 */
+        if (mi->context.c2.push_ifconfig_ipv6_defined)
+        {
+            unlearn_ifconfig_ipv6(m, mi);
+        }
+
         /* Add new IPv6 */
-        multi_learn_in6_addr(m, mi, new_ipv6, -1, true);
+        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))
+        {
+            mi->context.c2.push_ifconfig_ipv6_defined = true;
+            mi->context.c2.push_ifconfig_ipv6_local = new_addr6;
+        }
     }
-
-    /* TO DO:
-     *  else if (old_ipv6)
-     *  {
-     *      // remove old IPv6
-     *  }
-     */
 }
index b2b892bbb5d34cb29fbbfec3b70e087353b450cd..97bbc4a23a8fc67bb8957a3251ff090aae92ab16 100644 (file)
@@ -692,6 +692,8 @@ lookup_by_cid(struct multi_context *m, const unsigned long cid);
 #endif
 
 void
-update_vhash(struct multi_context *m, struct multi_instance *mi, const char *old_ip, const char *old_ipv6);
+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);
 
 #endif /* MULTI_H */
index 0c8eb84df6b572eb080d9a74d487ef70b0fb523e..2c717c7b28ceca8a8edb488736ae332af8b4639e 100644 (file)
@@ -1118,7 +1118,7 @@ process_incoming_push_msg(struct context *c, const struct buffer *buffer,
                         " To be able to process PUSH_UPDATE messages, be sure to use the --disable-dco option.");
             return PUSH_MSG_ERROR;
         }
-        return process_incoming_push_update(c, permission_mask, option_types_found, &buf, false);
+        return process_push_update(c, &c->options, permission_mask, option_types_found, &buf, false);
     }
     else
     {
index 6b3275e4056361aa75f473f0b62cf19c80095751..19a029a415439f7a6823aa164fa87d38a6a6b12c 100644 (file)
@@ -61,11 +61,13 @@ int process_incoming_push_request(struct context *c);
  * message has not yet been received.
  *
  * @param c The context for the operation.
+ * @param o The options structure to be updated with the received push options.
  * @param permission_mask The permission mask specifying which options are allowed to be pulled.
  * @param option_types_found A pointer to a variable that will be filled with the types of options
  *                           found in the message.
  * @param buf A buffer containing the received message.
- * @param msg_sender A boolean indicating if function is called by the message sender (server).
+ * @param msg_sender A boolean indicating if the message is being processed on the client (false)
+ *                   or on the server (true).
  *
  * @return
  * - `PUSH_MSG_UPDATE`: The message was processed successfully, and the updates were applied.
@@ -74,9 +76,8 @@ int process_incoming_push_request(struct context *c);
  * - `PUSH_MSG_ERROR`: An error occurred during message processing, or the message is invalid.
  */
 
-int process_incoming_push_update(struct context *c, unsigned int permission_mask,
-                                 unsigned int *option_types_found, struct buffer *buf,
-                                 bool msg_sender);
+int process_push_update(struct context *c, struct options *o, unsigned int permission_mask,
+                        unsigned int *option_types_found, struct buffer *buf, bool msg_sender);
 
 int process_incoming_push_msg(struct context *c, const struct buffer *buffer,
                               bool honor_received_options, unsigned int permission_mask,
index 25c6ebe2d424966031e3eb337ec7c3c7705e8d7b..0e86ad301cbad9c0dee9691f1fc5eeffc6878491 100644 (file)
 #endif
 
 int
-process_incoming_push_update(struct context *c, unsigned int permission_mask,
-                             unsigned int *option_types_found, struct buffer *buf,
-                             bool msg_sender)
+process_push_update(struct context *c, struct options *o, unsigned int permission_mask,
+                    unsigned int *option_types_found, struct buffer *buf, bool msg_sender)
 {
     int ret = PUSH_MSG_ERROR;
     const uint8_t ch = buf_read_u8(buf);
     if (ch == ',')
     {
-        if (apply_push_options(c, &c->options, buf, permission_mask, option_types_found, c->c2.es,
+        if (apply_push_options(c, o, buf, permission_mask, option_types_found, c->c2.es,
                                true))
         {
-            switch (c->options.push_continuation)
+            switch (o->push_continuation)
             {
                 case 0:
                 case 1:
@@ -144,13 +143,27 @@ message_splitter(const char *s, struct buffer *msgs, struct gc_arena *gc, const
 
 /* send the message(s) prepared to one single client */
 static bool
-send_single_push_update(struct context *c, struct buffer *msgs, unsigned int *option_types_found)
+send_single_push_update(struct multi_context *m, struct multi_instance *mi, struct buffer *msgs)
 {
     if (!msgs[0].data || !*(msgs[0].data))
     {
         return false;
     }
+
     int i = -1;
+    unsigned int option_types_found = 0;
+    struct context *c = &mi->context;
+    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.
+     */
+    const char *canary = "canary";
+    o.ifconfig_local = canary;
+    o.ifconfig_ipv6_local = canary;
 
     while (msgs[++i].data && *(msgs[i].data))
     {
@@ -159,14 +172,14 @@ send_single_push_update(struct context *c, struct buffer *msgs, unsigned int *op
             return false;
         }
 
-        /* After sending the control message, we update the options
-         * server-side in the client's context so pushed options like
-         * ifconfig/ifconfig-ipv6 can actually work.
+        /* After sending the control message, we parse it, miming the behavior
+         * of `process_incoming_push_msg()` and we fill an empty `options` struct
+         * with the new options. If an `ifconfig_local` or `ifconfig_ipv6_local`
+         * options is found we update the vhash accordingly, so that the pushed
+         * ifconfig/ifconfig-ipv6 options can actually work.
          * If we don't do that, packets arriving from the client with the
          * new address will be rejected and packets for the new address
          * will not be routed towards the client.
-         * For the same reason we later update the vhash too in
-         * `send_push_update()` function.
          * Using `buf_string_compare_advance()` we mimic the behavior
          * inside `process_incoming_push_msg()`. However, we don't need
          * to check the return value here because we just want to `advance`,
@@ -176,17 +189,39 @@ send_single_push_update(struct context *c, struct buffer *msgs, unsigned int *op
          */
         struct buffer tmp_msg = msgs[i];
         buf_string_compare_advance(&tmp_msg, push_update_cmd);
-        if (process_incoming_push_update(c, pull_permission_mask(c), option_types_found, &tmp_msg, true) == PUSH_MSG_ERROR)
+        unsigned int permission_mask = pull_permission_mask(c);
+        if (process_push_update(c, &o, permission_mask, &option_types_found, &tmp_msg, true) == PUSH_MSG_ERROR)
         {
             msg(M_WARN, "Failed to process push update message sent to client ID: %u", c->c2.tls_multi->peer_id);
-            continue;
         }
-        c->options.push_option_types_found |= *option_types_found;
-        if (!options_postprocess_pull(&c->options, c->c2.es))
+    }
+
+    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))
         {
-            msg(M_WARN, "Failed to post-process push update message sent to client ID: %u", c->c2.tls_multi->peer_id);
+            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);
     }
+
     return true;
 }
 
@@ -229,8 +264,6 @@ send_push_update(struct multi_context *m, const void *target, const char *msg, c
     int msgs_num = (strlen(msg) / safe_cap) + ((strlen(msg) % safe_cap) != 0);
     struct buffer *msgs = gc_malloc((msgs_num + 1) * sizeof(struct buffer), true, &gc);
 
-    unsigned int option_types_found = 0;
-
     msgs[msgs_num].data = NULL;
     if (!message_splitter(msg, msgs, &gc, safe_cap))
     {
@@ -255,15 +288,9 @@ send_push_update(struct multi_context *m, const void *target, const char *msg, c
             return 0;
         }
 
-        const char *old_ip = mi->context.options.ifconfig_local;
-        const char *old_ipv6 = mi->context.options.ifconfig_ipv6_local;
         if (!mi->halt
-            && send_single_push_update(&mi->context, msgs, &option_types_found))
+            && send_single_push_update(m, mi, msgs))
         {
-            if (option_types_found & OPT_P_UP)
-            {
-                update_vhash(m, mi, old_ip, old_ipv6);
-            }
             gc_free(&gc);
             return 1;
         }
@@ -289,18 +316,11 @@ send_push_update(struct multi_context *m, const void *target, const char *msg, c
         }
 
         /* Type is UPT_BROADCAST so we update every client */
-        option_types_found = 0;
-        const char *old_ip = curr_mi->context.options.ifconfig_local;
-        const char *old_ipv6 = curr_mi->context.options.ifconfig_ipv6_local;
-        if (!send_single_push_update(&curr_mi->context, msgs, &option_types_found))
+        if (!send_single_push_update(m, curr_mi, msgs))
         {
             msg(M_CLIENT, "ERROR: Peer ID: %u has not been updated", curr_mi->context.c2.tls_multi->peer_id);
             continue;
         }
-        if (option_types_found & OPT_P_UP)
-        {
-            update_vhash(m, curr_mi, old_ip, old_ipv6);
-        }
         count++;
     }
 
index 60596ed2f05adb30a9e1cbfa82a9cf21813987db..7fadb696ad8ba9530c0d113d0f184f8e78ee91e9 100644 (file)
@@ -29,7 +29,19 @@ pull_permission_mask(const struct context *c)
 }
 
 void
-update_vhash(struct multi_context *m, struct multi_instance *mi, const char *old_ip, const char *old_ipv6)
+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)
 {
     return;
 }
@@ -95,7 +107,7 @@ process_incoming_push_msg(struct context *c, const struct buffer *buffer,
     }
     else if (honor_received_options && buf_string_compare_advance(&buf, push_update_cmd))
     {
-        return process_incoming_push_update(c, permission_mask, option_types_found, &buf, false);
+        return process_push_update(c, &c->options, permission_mask, option_types_found, &buf, false);
     }
     else
     {