openvpn-devel — OpenVPN developers list

[Openvpn-devel] [PATCH v6] t_server_null: add multi-socket testing

[Gert D. <ge...@gr...>]

From: Samuli Seppänen <sam...@gm...>

This adds a new multi-socket server that listens on IPv4 and IPv6
localhost addresses for TCP and UDP connections respectively. It also
adds two success tests and one failure test with wrong protocol defined
at the client side.

Change-Id: I4ebe1158c36a641888131e824f59004a0f8fb4c5
Signed-off-by: Samuli Seppänen <sa...@pm...>
Acked-by: Gert Doering <ge...@gr...>
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/919
This mail reflects revision 6 of this Change.

Acked-by according to Gerrit (reflected above):
Gert Doering <ge...@gr...>

        
diff --git a/tests/t_server_null_default.rc b/tests/t_server_null_default.rc
index 41ec591..a1c68cd 100755
--- a/tests/t_server_null_default.rc
+++ b/tests/t_server_null_default.rc
@@ -38,12 +38,14 @@
 MAX_CLIENTS="10"
 CLIENT_MATCH="Test-Client"
 SERVER_EXEC="${top_builddir}/src/openvpn/openvpn"
-SERVER_BASE_OPTS="--local 127.0.0.1 --dev tun --topology subnet --max-clients $MAX_CLIENTS --persist-tun --verb 3 --duplicate-cn"
+SERVER_BASE_OPTS="--dev tun --topology subnet --max-clients $MAX_CLIENTS --persist-tun --verb 3 --duplicate-cn"
+SERVER_BIND_OPTS="--local 127.0.0.1"
 SERVER_CIPHER_OPTS=""
 SERVER_CERT_OPTS="--ca ${CA} --cert ${SERVER_CERT} --key ${SERVER_KEY} --tls-auth ${TA} 0"
-SERVER_CONF_BASE="${SERVER_BASE_OPTS} ${SERVER_CIPHER_OPTS} ${SERVER_CERT_OPTS}"
+SERVER_CONF_BASE="${SERVER_BASE_OPTS} ${SERVER_CIPHER_OPTS} ${SERVER_CERT_OPTS} ${SERVER_BIND_OPTS}"
+SERVER_CONF_BASE_MULTISOCKET="${SERVER_BASE_OPTS} ${SERVER_CIPHER_OPTS} ${SERVER_CERT_OPTS}"
 
-TEST_SERVER_LIST="1 2 3"
+TEST_SERVER_LIST="1 2 3 4"
 
 SERVER_NAME_1="t_server_null_server-1194_udp"
 SERVER_SERVER_1="--server 10.29.41.0 255.255.255.0"
@@ -63,6 +65,12 @@
 SERVER_EXEC_3="${SERVER_EXEC}"
 SERVER_CONF_3="${SERVER_CONF_BASE} ${SERVER_SERVER_3} --lport 1196 --proto udp --management 127.0.0.1 ${SERVER_MGMT_PORT_3} --dh none --cipher AES-192-CBC --data-ciphers DEFAULT:AES-192-CBC"
 
+SERVER_NAME_4="t_server_null_server-1197_multisocket_ipv4_ipv6"
+SERVER_SERVER_4="--server 10.29.44.0 255.255.255.0"
+SERVER_MGMT_PORT_4="11197"
+SERVER_EXEC_4="${SERVER_EXEC}"
+SERVER_CONF_4="${SERVER_CONF_BASE_MULTISOCKET} ${SERVER_SERVER_4} --local 127.0.0.1 1197 tcp --local ::1 1197 udp --management 127.0.0.1 ${SERVER_MGMT_PORT_4}"
+
 # Test client configurations
 CLIENT_EXEC="${top_builddir}/src/openvpn/openvpn"
 CLIENT_BASE_OPTS="--client --nobind --remote-cert-tls server --persist-tun --verb 3 --resolv-retry infinite --connect-retry-max 3 --server-poll-timeout 5 --explicit-exit-notify 3 --script-security 2"
@@ -72,7 +80,7 @@
 CLIENT_CIPHER_OPTS=""
 CLIENT_CERT_OPTS="--ca ${CA} --cert ${CLIENT_CERT} --key ${CLIENT_KEY} --tls-auth ${TA} 1"
 
-TEST_RUN_LIST="1 1L 2 2L 3 4a 4b 4c"
+TEST_RUN_LIST="1 1L 2 2L 3 4a 4b 4c 5a 5b 5c"
 CLIENT_CONF_BASE="${CLIENT_NULL_OPTS} ${CLIENT_BASE_OPTS} ${CLIENT_CIPHER_OPTS} ${CLIENT_CERT_OPTS}"
 CLIENT_CONF_BASE_LWIP="${CLIENT_LWIP_OPTS} ${CLIENT_BASE_OPTS} ${CLIENT_CIPHER_OPTS} ${CLIENT_CERT_OPTS}"
 
@@ -121,3 +129,18 @@
 SHOULD_PASS_4c="no"
 CLIENT_EXEC_4c="${CLIENT_EXEC}"
 CLIENT_CONF_4c="${CLIENT_CONF_BASE} --remote 127.0.0.1 1196 udp --proto udp --cipher AES-192-CBC --data-ciphers AES-128-CBC"
+
+TEST_NAME_5a="t_server_null_client.sh-openvpn_current_multisocket_ipv4_tcp"
+SHOULD_PASS_5a="yes"
+CLIENT_EXEC_5a="${CLIENT_EXEC}"
+CLIENT_CONF_5a="${CLIENT_CONF_BASE} --remote 127.0.0.1 1197 tcp"
+
+TEST_NAME_5b="t_server_null_client.sh-openvpn_current_multisocket_ipv6_udp"
+SHOULD_PASS_5b="yes"
+CLIENT_EXEC_5b="${CLIENT_EXEC}"
+CLIENT_CONF_5b="${CLIENT_CONF_BASE} --remote ::1 1197 udp"
+
+TEST_NAME_5c="t_server_null_client.sh-openvpn_current_multisocket_ipv6_tcp_fail"
+SHOULD_PASS_5c="no"
+CLIENT_EXEC_5c="${CLIENT_EXEC}"
+CLIENT_CONF_5c="${CLIENT_CONF_BASE} --remote ::1 1197 tcp"

[Openvpn-devel] [S] Change in openvpn[master]: t_server_null: add multi-socket testing

[cron2 (C. Review) <ge...@op...>]

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

cron2 has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/919?usp=email )

Change subject: t_server_null: add multi-socket testing
......................................................................


Patch Set 6: Code-Review+2


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

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I4ebe1158c36a641888131e824f59004a0f8fb4c5
Gerrit-Change-Number: 919
Gerrit-PatchSet: 6
Gerrit-Owner: mattock <sa...@pr...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: mattock <sa...@pr...>
Gerrit-Comment-Date: Tue, 08 Jul 2025 07:33:36 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Openvpn-devel] [S] Change in openvpn[master]: mac dns: do not run dns-updown in parallel

[plaisthos (C. Review) <ge...@op...>]

Attention is currently required from: d12fk, flichtenheld.

plaisthos has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/1076?usp=email )

Change subject: mac dns: do not run dns-updown in parallel
......................................................................


Patch Set 2:

(1 comment)

File distro/dns-scripts/macos-dns-updown.sh:

http://gerrit.openvpn.net/c/openvpn/+/1076/comment/49cd0df5_590aa1aa :
PS2, Line 30: lockfile=/tmp/openvpn-dns-updown.lock
> There is not /var/lock on my system, that is why I opted for /tmp
I have one that is from 2011 on my system 😊

But maybe /var/run is better than /tmp?



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

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I7adfaa08df6a17545cca8264d7230b5e65e49719
Gerrit-Change-Number: 1076
Gerrit-PatchSet: 2
Gerrit-Owner: d12fk <he...@op...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: d12fk <he...@op...>
Gerrit-Comment-Date: Mon, 07 Jul 2025 21:15:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: plaisthos <arn...@rf...>
Comment-In-Reply-To: d12fk <he...@op...>
Gerrit-MessageType: comment

[Openvpn-devel] [XS] Change in openvpn[master]: dns: do not run updown scripts with lwipovpn

[plaisthos (C. Review) <ge...@op...>]

Attention is currently required from: d12fk, flichtenheld.

plaisthos has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/1077?usp=email )

Change subject: dns: do not run updown scripts with lwipovpn
......................................................................


Patch Set 2: Code-Review+2


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

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I7e9a0c668e0950257632452cfd9eeb236f0120f2
Gerrit-Change-Number: 1077
Gerrit-PatchSet: 2
Gerrit-Owner: d12fk <he...@op...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: d12fk <he...@op...>
Gerrit-Comment-Date: Mon, 07 Jul 2025 21:13:43 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Openvpn-devel] [L] Change in openvpn[master]: PUSH_UPDATE: Added update_option() function.

[mrbff (C. Review) <ge...@op...>]

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

Hello flichtenheld, plaisthos, stipa, 

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

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

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

The change is no longer submittable: checks~ChecksSubmitRule is unsatisfied now.


Change subject: PUSH_UPDATE: Added update_option() function.
......................................................................

PUSH_UPDATE: Added update_option() function.

When the function receives an option to update, it first checks whether it has
already received an option of the same type within the same update message.
If it has already received it, it simply calls add_option(), otherwise it
deletes all the values ​​already present regarding that option.

Change-Id: Ia45c99e6df7b3ad24020c10b8a9b3577984ecdc2
Signed-off-by: Marco Baffo <ma...@ma...>
---
M src/openvpn/options.c
1 file changed, 247 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/10/810/15

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 6597610..66a2bc6 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -5634,6 +5634,13 @@
     return options->forward_compatible ? M_WARN : msglevel;
 }
 
+#define RESET_OPTION_ROUTES(option_ptr, field) \
+    if (option_ptr)                        \
+    {                                      \
+        option_ptr->field = NULL;          \
+        option_ptr->flags = 0;             \
+    }
+
 /**
  * @brief Resets options found in the PUSH_UPDATE message that are preceded by the `-` flag.
  *        This function is used in push-updates to reset specified options.
@@ -5688,11 +5695,7 @@
             delete_routes_v4(c->c1.route_list, c->c1.tuntap,
                              ROUTE_OPTION_FLAGS(&c->options),
                              es, &c->net_ctx);
-            if (options->routes)
-            {
-                options->routes->routes = NULL;
-                options->routes->flags = 0;
-            }
+            RESET_OPTION_ROUTES(options->routes, routes);
         }
     }
     else if (streq(p[0], "route-ipv6") && !p[1])
@@ -5703,11 +5706,7 @@
             delete_routes_v6(c->c1.route_ipv6_list, c->c1.tuntap,
                              ROUTE_OPTION_FLAGS(&c->options),
                              es, &c->net_ctx);
-            if (options->routes_ipv6)
-            {
-                options->routes_ipv6->routes_ipv6 = NULL;
-                options->routes_ipv6->flags = 0;
-            }
+            RESET_OPTION_ROUTES(options->routes_ipv6, routes_ipv6);
         }
     }
     else if (streq(p[0], "route-gateway") && !p[1])
@@ -5826,6 +5825,238 @@
 err:
     msg(msglevel, "Error occurred trying to remove %s option", p[0]);
 }
+
+/**
+ * @brief Processes an option to update. It first checks whether it has already
+ *        received an option of the same type within the same update message.
+ *        If the option has already been received, it calls add_option().
+ *        Otherwise, it deletes all existing values related to that option before calling add_option().
+ *
+ * @param c The context structure.
+ * @param options A pointer to the options structure.
+ * @param p An array of strings containing the options and their parameters.
+ * @param is_inline A boolean indicating if the option is inline.
+ * @param file The file where the function is called.
+ * @param line The line number where the function is called.
+ * @param level The level of the option.
+ * @param msglevel The message level for logging.
+ * @param permission_mask The permission mask used by VERIFY_PERMISSION().
+ * @param option_types_found A pointer to the variable where the flags corresponding to the options found are stored.
+ * @param es The environment set structure.
+ * @param update_options_found A pointer to the variable where the flags corresponding to the update options found are stored,
+ *                             used to check if an option of the same type has already been processed by update_option() within the same push-update message.
+ */
+static void
+update_option(struct context *c,
+              struct options *options,
+              char *p[],
+              bool is_inline,
+              const char *file,
+              int line,
+              const int level,
+              const int msglevel,
+              const unsigned int permission_mask,
+              unsigned int *option_types_found,
+              struct env_set *es,
+              unsigned int *update_options_found)
+{
+    const bool pull_mode = BOOL_CAST(permission_mask & OPT_P_PULL_MODE);
+    ASSERT(MAX_PARMS >= 7);
+
+    if (streq(p[0], "route") && p[1] && !p[5])
+    {
+        if (!(*update_options_found & OPT_P_U_ROUTE))
+        {
+            VERIFY_PERMISSION(OPT_P_ROUTE);
+            rol_check_alloc(options);
+            if (pull_mode)
+            {
+                if (!ip_or_dns_addr_safe(p[1], options->allow_pull_fqdn) && !is_special_addr(p[1])) /* FQDN -- may be DNS name */
+                {
+                    msg(msglevel, "route parameter network/IP '%s' must be a valid address", p[1]);
+                    goto err;
+                }
+                if (p[2] && !ip_addr_dotted_quad_safe(p[2])) /* FQDN -- must be IP address */
+                {
+                    msg(msglevel, "route parameter netmask '%s' must be an IP address", p[2]);
+                    goto err;
+                }
+                if (p[3] && !ip_or_dns_addr_safe(p[3], options->allow_pull_fqdn) && !is_special_addr(p[3])) /* FQDN -- may be DNS name */
+                {
+                    msg(msglevel, "route parameter gateway '%s' must be a valid address", p[3]);
+                    goto err;
+                }
+            }
+            if (c->c1.route_list)
+            {
+                delete_routes_v4(c->c1.route_list, c->c1.tuntap,
+                                 ROUTE_OPTION_FLAGS(&c->options),
+                                 es, &c->net_ctx);
+                RESET_OPTION_ROUTES(options->routes, routes);
+            }
+            *update_options_found |= OPT_P_U_ROUTE;
+        }
+    }
+    else if (streq(p[0], "route-ipv6") && p[1] && !p[4])
+    {
+        if (!(*update_options_found & OPT_P_U_ROUTE6))
+        {
+            VERIFY_PERMISSION(OPT_P_ROUTE);
+            rol6_check_alloc(options);
+            if (pull_mode)
+            {
+                if (!ipv6_addr_safe_hexplusbits(p[1]))
+                {
+                    msg(msglevel, "route-ipv6 parameter network/IP '%s' must be a valid address", p[1]);
+                    goto err;
+                }
+                if (p[2] && !ipv6_addr_safe(p[2]))
+                {
+                    msg(msglevel, "route-ipv6 parameter gateway '%s' must be a valid address", p[2]);
+                    goto err;
+                }
+                /* p[3] is metric, if present */
+            }
+            if (c->c1.route_ipv6_list)
+            {
+                delete_routes_v6(c->c1.route_ipv6_list, c->c1.tuntap,
+                                 ROUTE_OPTION_FLAGS(&c->options),
+                                 es, &c->net_ctx);
+                RESET_OPTION_ROUTES(options->routes_ipv6, routes_ipv6);
+            }
+            *update_options_found |= OPT_P_U_ROUTE6;
+        }
+    }
+    else if (streq(p[0], "redirect-gateway") || streq(p[0], "redirect-private"))
+    {
+        if (!(*update_options_found & OPT_P_U_REDIR_GATEWAY))
+        {
+            VERIFY_PERMISSION(OPT_P_ROUTE);
+            if (options->routes)
+            {
+                options->routes->flags = 0;
+            }
+            if (options->routes_ipv6)
+            {
+                options->routes_ipv6->flags = 0;
+            }
+            *update_options_found |= OPT_P_U_REDIR_GATEWAY;
+        }
+    }
+    else if (streq(p[0], "dns") && p[1])
+    {
+        if (!(*update_options_found & OPT_P_U_DNS))
+        {
+            VERIFY_PERMISSION(OPT_P_DHCPDNS);
+            if (streq(p[1], "server") && p[2] && p[3] && p[4])
+            {
+                long priority;
+                if (!dns_server_priority_parse(&priority, p[2], pull_mode))
+                {
+                    msg(msglevel, "--dns server: invalid priority value '%s'", p[2]);
+                    goto err;
+                }
+
+                struct dns_server server;
+                CLEAR(server);
+                if (streq(p[3], "address") && p[4])
+                {
+                    for (int i = 4; p[i]; ++i)
+                    {
+                        if (!dns_server_addr_parse(&server, p[i]))
+                        {
+                            msg(msglevel, "--dns server %ld: malformed address or maximum exceeded '%s'", priority, p[i]);
+                            goto err;
+                        }
+                    }
+                }
+                else if (streq(p[3], "dnssec") && !p[5])
+                {
+                    if (!streq(p[4], "yes") && !streq(p[4], "no") && !streq(p[4], "optional"))
+                    {
+                        msg(msglevel, "--dns server %ld: malformed dnssec value '%s'", priority, p[4]);
+                        goto err;
+                    }
+                }
+                else if (streq(p[3], "transport") && !p[5])
+                {
+                    if (!streq(p[4], "plain") && !streq(p[4], "DoH") && !streq(p[4], "DoT"))
+                    {
+                        msg(msglevel, "--dns server %ld: malformed transport value '%s'", priority, p[4]);
+                        goto err;
+                    }
+                }
+                else if (!streq(p[3], "resolve-domains")
+                         && !(streq(p[3], "sni") && !p[5]))
+                {
+                    msg(msglevel, "--dns server %ld: unknown option type '%s' or missing or unknown parameter", priority, p[3]);
+                    goto err;
+                }
+            }
+            else if (!(streq(p[1], "search-domains") && p[2]))
+            {
+                msg(msglevel, "--dns: unknown option type '%s' or missing or unknown parameter", p[1]);
+                goto err;
+            }
+
+            gc_free(&options->dns_options.gc);
+            CLEAR(options->dns_options);
+            *update_options_found |= OPT_P_U_DNS;
+        }
+    }
+#if defined(_WIN32) || defined(TARGET_ANDROID)
+    else if (streq(p[0], "dhcp-option") && p[1] && !p[3])
+    {
+        if (!(*update_options_found & OPT_P_U_DHCP))
+        {
+            struct tuntap_options *o = &options->tuntap_options;
+            VERIFY_PERMISSION(OPT_P_DHCPDNS);
+
+            o->domain = NULL;
+            o->netbios_scope = NULL;
+            o->netbios_node_type = 0;
+            o->dns6_len = 0;
+            CLEAR(o->dns6);
+            o->dns_len = 0;
+            CLEAR(o->dns);
+            o->wins_len = 0;
+            CLEAR(o->wins);
+            o->ntp_len = 0;
+            CLEAR(o->ntp);
+            o->nbdd_len = 0;
+            CLEAR(o->nbdd);
+            while (o->domain_search_list_len-- > 0)
+            {
+                o->domain_search_list[o->domain_search_list_len] = NULL;
+            }
+            o->disable_nbt = 0;
+            o->dhcp_options = 0;
+#if defined(TARGET_ANDROID)
+            o->http_proxy_port = 0;
+            o->http_proxy = NULL;
+#endif
+            *update_options_found |= OPT_P_U_DHCP;
+        }
+    }
+#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+    else if (streq(p[0], "dhcp-option") && p[1] && !p[3])
+    {
+        if (!(*update_options_found & OPT_P_U_DHCP))
+        {
+            VERIFY_PERMISSION(OPT_P_DHCPDNS);
+            delete_all_dhcp_fo(options, &es->list);
+            *update_options_found |= OPT_P_U_DHCP;
+        }
+    }
+#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+    add_option(options, p, is_inline, file, line,
+               level, msglevel, permission_mask,
+               option_types_found, es);
+    return;
+err:
+    msg(msglevel, "Error occurred trying to update %s option", p[0]);
+}
+
 bool
 apply_push_options(struct context *c,
                    struct options *options,
@@ -5839,6 +6070,7 @@
     int line_num = 0;
     const char *file = "[PUSH-OPTIONS]";
     const int msglevel = D_PUSH_ERRORS|M_OPTERR;
+    unsigned int update_options_found = 0;
 
     while (buf_parse(buf, ',', line, sizeof(line)))
     {
@@ -5864,6 +6096,11 @@
                 remove_option(c, options, p, false, file, line_num, msglevel,
                               permission_mask, option_types_found, es);
             }
+            else
+            {
+                update_option(c, options, p, false, file, line_num, 0, msglevel,
+                              permission_mask, option_types_found, es, &update_options_found);
+            }
         }
     }
     return true;

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

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ia45c99e6df7b3ad24020c10b8a9b3577984ecdc2
Gerrit-Change-Number: 810
Gerrit-PatchSet: 15
Gerrit-Owner: mrbff <ma...@ma...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-Reviewer: stipa <lst...@gm...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: mrbff <ma...@ma...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [L] Change in openvpn[master]: PUSH_UPDATE: Added remove_option() and do_update().

[mrbff (C. Review) <ge...@op...>]

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

Hello flichtenheld, plaisthos, stipa, 

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

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

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

The change is no longer submittable: checks~ChecksSubmitRule is unsatisfied now.


Change subject: PUSH_UPDATE: Added remove_option() and do_update().
......................................................................

PUSH_UPDATE: Added remove_option() and do_update().

* Added remove_option() function and some utility functions to remove options at
  runtime following the push-update logic.
* Added do_update() function to close and reopen the tun and apply option updates.

Change-Id: I507180d7397b6959844a30908010132bc3411067
Signed-off-by: Marco Baffo <ma...@ma...>
---
M src/openvpn/init.c
M src/openvpn/init.h
M src/openvpn/multi.c
M src/openvpn/options.c
M src/openvpn/push.c
M src/openvpn/route.c
M src/openvpn/route.h
7 files changed, 373 insertions(+), 42 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/09/809/15

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 78ebb17..9ab5378 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2470,7 +2470,7 @@
 
         if (pulled_options)
         {
-            if (!do_deferred_options(c, option_types_found))
+            if (!do_deferred_options(c, option_types_found, false))
             {
                 msg(D_PUSH_ERRORS, "ERROR: Failed to apply push options");
                 return false;
@@ -2594,6 +2594,55 @@
     return true;
 }
 
+bool
+do_update(struct context *c, unsigned int option_types_found)
+{
+    /* Not necessary since to receive the update the openvpn
+     * instance must be up and running but just in case
+     */
+    if (!c->c2.do_up_ran)
+    {
+        return false;
+    }
+
+    bool tt_dco_win = tuntap_is_dco_win(c->c1.tuntap);
+    if (tt_dco_win)
+    {
+        msg(M_NONFATAL, "dco-win doesn't yet support reopening TUN device");
+        return false;
+    }
+
+    if (!do_deferred_options(c, option_types_found, true))
+    {
+        msg(D_PUSH_ERRORS, "ERROR: Failed to apply push options");
+        return false;
+    }
+
+    do_close_tun(c, true);
+
+    management_sleep(1);
+    int error_flags = 0;
+    c->c2.did_open_tun = do_open_tun(c, &error_flags);
+    update_time();
+
+    if (c->c2.did_open_tun)
+    {
+        /* if --route-delay was specified, start timer */
+        if ((route_order(c->c1.tuntap) == ROUTE_AFTER_TUN) && c->options.route_delay_defined)
+        {
+            event_timeout_init(&c->c2.route_wakeup, c->options.route_delay, now);
+            event_timeout_init(&c->c2.route_wakeup_expire, c->options.route_delay + c->options.route_delay_window, now);
+            tun_standby_init(c->c1.tuntap);
+        }
+
+        initialization_sequence_completed(c, error_flags);
+    }
+
+    CLEAR(c->c1.pulled_options_digest_save);
+
+    return true;
+}
+
 /*
  * These are the option categories which will be accepted by pull.
  */
@@ -2672,11 +2721,8 @@
     return true;
 }
 
-/*
- * Handle non-tun-related pulled options.
- */
 bool
-do_deferred_options(struct context *c, const unsigned int found)
+do_deferred_options(struct context *c, const unsigned int found, const bool is_update)
 {
     if (found & OPT_P_MESSAGES)
     {
@@ -2784,7 +2830,7 @@
     /* process (potentially) pushed options */
     if (c->options.pull)
     {
-        if (!check_pull_client_ncp(c, found))
+        if (!is_update && !check_pull_client_ncp(c, found))
         {
             return false;
         }
diff --git a/src/openvpn/init.h b/src/openvpn/init.h
index 5c6b9c1..25078a6 100644
--- a/src/openvpn/init.h
+++ b/src/openvpn/init.h
@@ -86,13 +86,29 @@
            bool pulled_options,
            unsigned int option_types_found);
 
+/**
+ * @brief A simplified version of the do_up() function. This function is called
+ *        after receiving a successful PUSH_UPDATE message. It closes and reopens
+ *        the TUN device to apply the updated options.
+ *
+ * @param c The context structure.
+ * @param option_types_found The options found in the PUSH_UPDATE message.
+ * @return true on success.
+ * @return false on error.
+ */
+bool do_update(struct context *c, unsigned int option_types_found);
+
 unsigned int pull_permission_mask(const struct context *c);
 
 const char *format_common_name(struct context *c, struct gc_arena *gc);
 
 void reset_coarse_timers(struct context *c);
 
-bool do_deferred_options(struct context *c, const unsigned int found);
+/*
+ * Handle non-tun-related pulled options.
+ * Set `is_update` param to true to skip NCP check.
+ */
+bool do_deferred_options(struct context *c, const unsigned int found, const bool is_update);
 
 void inherit_context_child(struct context *dest,
                            const struct context *src,
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index a760e07..7f0d890 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -2423,7 +2423,7 @@
     /*
      * Process sourced options.
      */
-    do_deferred_options(&mi->context, option_types_found);
+    do_deferred_options(&mi->context, option_types_found, false);
 
     /*
      * make sure we got ifconfig settings from somewhere
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 67fa906..6597610 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1085,6 +1085,40 @@
         gc_free(&gc);
     }
 }
+
+static void
+delete_all_dhcp_fo(struct options *o, struct env_item **list)
+{
+    struct env_item *current, *prev;
+
+    ASSERT(list);
+
+    for (current = *list, prev = NULL; current != NULL; current = current->next)
+    {
+        char *tmp_value = NULL;
+        if (!strncmp(current->string, "foreign_option_", sizeof("foreign_option_")-1))
+        {
+            tmp_value = strchr(current->string, '=');
+            if (tmp_value && ++tmp_value)
+            {
+                if (!strncmp(tmp_value, "dhcp-option ", sizeof("dhcp-option ")-1))
+                {
+                    if (prev)
+                    {
+                        prev->next = current->next;
+                    }
+                    else
+                    {
+                        *list = current->next;
+                    }
+                    o->foreign_option_index--;
+                }
+            }
+        }
+        prev = current;
+    }
+}
+
 #endif /* ifndef _WIN32 */
 
 static in_addr_t
@@ -3089,8 +3123,16 @@
         msg(M_INFO, "Flag 'def1' added to --redirect-gateway (iservice is in use)");
         opt->routes->flags |= RG_DEF1;
     }
+    else if (opt->routes
+             && ((opt->route_method != ROUTE_METHOD_SERVICE)
+                 || !(opt->routes->flags & RG_REROUTE_GW))
+             && (opt->routes->flags & RG_DEF1))
+    {
+        msg(M_INFO, "Flag 'def1' removed from --redirect-gateway");
+        opt->routes->flags &= ~RG_DEF1;
+    }
 }
-#endif
+#endif /* ifdef _WIN32 */
 
 /*
  * Save/Restore certain option defaults before --pull is applied.
@@ -5490,37 +5532,6 @@
     }
 }
 
-void
-options_server_import(struct options *o,
-                      const char *filename,
-                      int msglevel,
-                      unsigned int permission_mask,
-                      unsigned int *option_types_found,
-                      struct env_set *es)
-{
-    msg(D_PUSH, "OPTIONS IMPORT: reading client specific options from: %s", filename);
-    read_config_file(o,
-                     filename,
-                     0,
-                     filename,
-                     0,
-                     msglevel,
-                     permission_mask,
-                     option_types_found,
-                     es);
-}
-
-void
-options_string_import(struct options *options,
-                      const char *config,
-                      const int msglevel,
-                      const unsigned int permission_mask,
-                      unsigned int *option_types_found,
-                      struct env_set *es)
-{
-    read_config_string("[CONFIG-STRING]", options, config, msglevel, permission_mask, option_types_found, es);
-}
-
 #define VERIFY_PERMISSION(mask) {                                            \
         if (!verify_permission(p[0], file, line, (mask), permission_mask,        \
                                option_types_found, msglevel, options, is_inline)) \
@@ -5623,6 +5634,198 @@
     return options->forward_compatible ? M_WARN : msglevel;
 }
 
+/**
+ * @brief Resets options found in the PUSH_UPDATE message that are preceded by the `-` flag.
+ *        This function is used in push-updates to reset specified options.
+ *        The number of parameters `p` must always be 1. If the permission is verified,
+ *        all related options are erased or reset to their default values.
+ *        Upon successful permission verification (by VERIFY_PERMISSION()),
+ *        `option_types_found` is filled with the flag corresponding to the option.
+ *
+ * @param c The context structure.
+ * @param options A pointer to the options structure.
+ * @param p An array of strings containing the options and their parameters.
+ * @param is_inline A boolean indicating if the option is inline.
+ * @param file The file where the function is called.
+ * @param line The line number where the function is called.
+ * @param msglevel The message level.
+ * @param permission_mask The permission mask used by VERIFY_PERMISSION().
+ * @param option_types_found A pointer to the variable where the flags corresponding to the options found are stored.
+ * @param es The environment set structure.
+ */
+static void
+remove_option(struct context *c,
+              struct options *options,
+              char *p[],
+              bool is_inline,
+              const char *file,
+              int line,
+              const int msglevel,
+              const unsigned int permission_mask,
+              unsigned int *option_types_found,
+              struct env_set *es)
+{
+    int msglevel_fc = msglevel_forward_compatible(options, msglevel);
+
+    if (streq(p[0], "ifconfig") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_UP);
+        options->ifconfig_local = NULL;
+        options->ifconfig_remote_netmask = NULL;
+    }
+    else if (streq(p[0], "ifconfig-ipv6") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_UP);
+        options->ifconfig_ipv6_local = NULL;
+        options->ifconfig_ipv6_netbits = 0;
+        options->ifconfig_ipv6_remote = NULL;
+    }
+    else if (streq(p[0], "route") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_ROUTE);
+        if (c->c1.route_list)
+        {
+            delete_routes_v4(c->c1.route_list, c->c1.tuntap,
+                             ROUTE_OPTION_FLAGS(&c->options),
+                             es, &c->net_ctx);
+            if (options->routes)
+            {
+                options->routes->routes = NULL;
+                options->routes->flags = 0;
+            }
+        }
+    }
+    else if (streq(p[0], "route-ipv6") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_ROUTE);
+        if (c->c1.route_ipv6_list)
+        {
+            delete_routes_v6(c->c1.route_ipv6_list, c->c1.tuntap,
+                             ROUTE_OPTION_FLAGS(&c->options),
+                             es, &c->net_ctx);
+            if (options->routes_ipv6)
+            {
+                options->routes_ipv6->routes_ipv6 = NULL;
+                options->routes_ipv6->flags = 0;
+            }
+        }
+    }
+    else if (streq(p[0], "route-gateway") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_ROUTE_EXTRAS);
+        options->route_gateway_via_dhcp = false;
+        options->route_default_gateway = NULL;
+    }
+    else if (streq(p[0], "route-metric") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_ROUTE);
+        options->route_default_metric = 0;
+    }
+    else if (streq(p[0], "push-continuation") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_PULL_MODE);
+        options->push_continuation = 0;
+    }
+    else if ((streq(p[0], "redirect-gateway") || streq(p[0], "redirect-private")) && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_ROUTE);
+        if (options->routes)
+        {
+            options->routes->flags = 0;
+        }
+        if (options->routes_ipv6)
+        {
+            options->routes_ipv6->flags = 0;
+        }
+    }
+    else if (streq(p[0], "dns") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_DHCPDNS);
+        gc_free(&options->dns_options.gc);
+        CLEAR(options->dns_options);
+    }
+    else if (streq(p[0], "topology") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_UP);
+        options->topology = TOP_UNDEF;
+        helper_setdefault_topology(options);
+    }
+    else if (streq(p[0], "tun-mtu") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_PUSH_MTU|OPT_P_CONNECTION);
+        options->ce.tun_mtu = TUN_MTU_DEFAULT;
+        options->ce.tun_mtu_defined = false;
+        options->ce.occ_mtu = 0;
+    }
+    else if (streq(p[0], "block-ipv6") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_ROUTE);
+        options->block_ipv6 = false;
+    }
+#if defined(_WIN32) || defined(TARGET_ANDROID)
+    else if (streq(p[0], "dhcp-option") && !p[1])
+    {
+        struct tuntap_options *o = &options->tuntap_options;
+        VERIFY_PERMISSION(OPT_P_DHCPDNS);
+
+        o->domain = NULL;
+        o->netbios_scope = NULL;
+        o->netbios_node_type = 0;
+        o->dns6_len = 0;
+        memset(o->dns6, 0, sizeof(o->dns6));
+        o->dns_len = 0;
+        memset(o->dns, 0, sizeof(o->dns));
+        o->wins_len = 0;
+        memset(o->wins, 0, sizeof(o->wins));
+        o->ntp_len = 0;
+        memset(o->ntp, 0, sizeof(o->ntp));
+        o->nbdd_len = 0;
+        memset(o->nbdd, 0, sizeof(o->nbdd));
+        while (o->domain_search_list_len-- > 0)
+        {
+            o->domain_search_list[o->domain_search_list_len] = NULL;
+        }
+        o->disable_nbt = 0;
+        o->dhcp_options = 0;
+#if defined(TARGET_ANDROID)
+        o->http_proxy_port = 0;
+        o->http_proxy = NULL;
+#endif
+    }
+#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+#ifdef _WIN32
+    else if (streq(p[0], "block-outside-dns") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_DHCPDNS);
+        options->block_outside_dns = false;
+    }
+#else /* ifdef _WIN32 */
+    else if (streq(p[0], "dhcp-option") && !p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_DHCPDNS);
+        delete_all_dhcp_fo(options, &es->list);
+    }
+#endif
+    else
+    {
+        int i;
+        int msglevel_unknown = msglevel_fc;
+        /* Check if an option is in --ignore-unknown-option and
+         * set warning level to non fatal */
+        for (i = 0; options->ignore_unknown_option && options->ignore_unknown_option[i]; i++)
+        {
+            if (streq(p[0], options->ignore_unknown_option[i]))
+            {
+                msglevel_unknown = M_WARN;
+                break;
+            }
+        }
+        msg(msglevel_unknown, "Unrecognized option or missing or extra parameter(s) in %s:%d: -%s (%s)", file, line, p[0], PACKAGE_VERSION);
+    }
+    return;
+err:
+    msg(msglevel, "Error occurred trying to remove %s option", p[0]);
+}
 bool
 apply_push_options(struct context *c,
                    struct options *options,
@@ -5656,11 +5859,47 @@
                 add_option(options, p, false, file, line_num, 0, msglevel,
                            permission_mask, option_types_found, es);
             }
+            else if (push_update_option_flags & PUSH_OPT_TO_REMOVE)
+            {
+                remove_option(c, options, p, false, file, line_num, msglevel,
+                              permission_mask, option_types_found, es);
+            }
         }
     }
     return true;
 }
 
+void
+options_server_import(struct options *o,
+                      const char *filename,
+                      int msglevel,
+                      unsigned int permission_mask,
+                      unsigned int *option_types_found,
+                      struct env_set *es)
+{
+    msg(D_PUSH, "OPTIONS IMPORT: reading client specific options from: %s", filename);
+    read_config_file(o,
+                     filename,
+                     0,
+                     filename,
+                     0,
+                     msglevel,
+                     permission_mask,
+                     option_types_found,
+                     es);
+}
+
+void
+options_string_import(struct options *options,
+                      const char *config,
+                      const int msglevel,
+                      const unsigned int permission_mask,
+                      unsigned int *option_types_found,
+                      struct env_set *es)
+{
+    read_config_string("[CONFIG-STRING]", options, config, msglevel, permission_mask, option_types_found, es);
+}
+
 static void
 set_user_script(struct options *options,
                 const char **script,
diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index 0907226..a6c14ca 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -542,6 +542,11 @@
                 {
                     msg(M_WARN, "No updatable options found in incoming PUSH_UPDATE message");
                 }
+                else if (!do_update(c, option_types_found))
+                {
+                    msg(D_PUSH_ERRORS, "Failed to update options");
+                    goto error;
+                }
             }
         }
         event_timeout_clear(&c->c2.push_request_interval);
diff --git a/src/openvpn/route.c b/src/openvpn/route.c
index 156262a..89ebaee 100644
--- a/src/openvpn/route.c
+++ b/src/openvpn/route.c
@@ -1265,7 +1265,16 @@
               const struct tuntap *tt, unsigned int flags,
               const struct env_set *es, openvpn_net_ctx_t *ctx)
 {
-    if (rl && rl->iflags & RL_ROUTES_ADDED)
+    delete_routes_v4(rl, tt, flags, es, ctx);
+    delete_routes_v6(rl6, tt, flags, es, ctx);
+}
+
+void
+delete_routes_v4(struct route_list *rl, const struct tuntap *tt,
+                 unsigned int flags, const struct env_set *es,
+                 openvpn_net_ctx_t *ctx)
+{
+    if (rl && (rl->iflags & RL_ROUTES_ADDED))
     {
         struct route_ipv4 *r;
         for (r = rl->routes; r; r = r->next)
@@ -1281,8 +1290,14 @@
     {
         clear_route_list(rl);
     }
+}
 
-    if (rl6 && (rl6->iflags & RL_ROUTES_ADDED) )
+void
+delete_routes_v6(struct route_ipv6_list *rl6, const struct tuntap *tt,
+                 unsigned int flags, const struct env_set *es,
+                 openvpn_net_ctx_t *ctx)
+{
+    if (rl6 && (rl6->iflags & RL_ROUTES_ADDED))
     {
         struct route_ipv6 *r6;
         for (r6 = rl6->routes_ipv6; r6; r6 = r6->next)
diff --git a/src/openvpn/route.h b/src/openvpn/route.h
index 237375c..b89ec9f 100644
--- a/src/openvpn/route.h
+++ b/src/openvpn/route.h
@@ -335,6 +335,16 @@
                    const struct env_set *es,
                    openvpn_net_ctx_t *ctx);
 
+void
+delete_routes_v4(struct route_list *rl, const struct tuntap *tt,
+                 unsigned int flags, const struct env_set *es,
+                 openvpn_net_ctx_t *ctx);
+
+void
+delete_routes_v6(struct route_ipv6_list *rl6, const struct tuntap *tt,
+                 unsigned int flags, const struct env_set *es,
+                 openvpn_net_ctx_t *ctx);
+
 void setenv_routes(struct env_set *es, const struct route_list *rl);
 
 void setenv_routes_ipv6(struct env_set *es, const struct route_ipv6_list *rl6);

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

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I507180d7397b6959844a30908010132bc3411067
Gerrit-Change-Number: 809
Gerrit-PatchSet: 15
Gerrit-Owner: mrbff <ma...@ma...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-Reviewer: stipa <lst...@gm...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: mrbff <ma...@ma...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [L] Change in openvpn[master]: PUSH_UPDATE message sender: enabling the server to send PUSH_UPDATE c...

[mrbff (C. Review) <ge...@op...>]

Attention is currently required from: cron2, flichtenheld, mrbff, plaisthos, stipa.

Hello cron2, flichtenheld, plaisthos, stipa, 

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

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

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

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


Change subject: PUSH_UPDATE message sender: enabling the server to send PUSH_UPDATE control messages
......................................................................

PUSH_UPDATE message sender: enabling the server to send PUSH_UPDATE control messages

Using the management interface you can now target one or more clients (via broadcast,
via cid, via common name, via address) and send a PUSH_UPDATE control message
to update some options.

Change-Id: Ie82bcc7a8e583de9156b185d71d1a323ed8df3fc
Signed-off-by: Marco Baffo <ma...@ma...>
---
M CMakeLists.txt
M doc/management-notes.txt
M src/openvpn/manage.c
M src/openvpn/manage.h
M src/openvpn/multi.c
M src/openvpn/multi.h
M src/openvpn/push.h
M src/openvpn/push_util.c
M tests/unit_tests/openvpn/Makefile.am
M tests/unit_tests/openvpn/test_push_update_msg.c
10 files changed, 824 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/69/869/8

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 54cf503..1381e03 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -861,6 +861,7 @@
 	    src/openvpn/push_util.c
 	    src/openvpn/options_util.c
 	    src/openvpn/otime.c
+        src/openvpn/list.c
         )
 
     if (TARGET test_argv)
diff --git a/doc/management-notes.txt b/doc/management-notes.txt
index f1d2930..58393da 100644
--- a/doc/management-notes.txt
+++ b/doc/management-notes.txt
@@ -1028,6 +1028,51 @@
 stored outside of the filesystem (e.g. in Mac OS X Keychain)
 with OpenVPN via the management interface.
 
+COMMAND -- push-update-broad (OpenVPN 2.7 or higher)
+----------------------------------------------------
+Send a message to every connected client to update options at runtime.
+The updatable options are: "block-ipv6", "block-outside-dns", "dhcp-option",
+"dns", "ifconfig", "ifconfig-ipv6", "redirect-gateway", "redirect-private",
+"route", "route-gateway", "route-ipv6", "route-metric", "topology",
+"tun-mtu", "keepalive". When a valid option is pushed, the receiving client will
+delete every previous value and set new value, so the update of the option will 
+not be incremental even when theoretically possible (ex. with "redirect-gateway").
+The '-' symbol in front of an option means the option should be removed.
+When an option is used with '-', it cannot take any parameter.
+The '?' symbol in front of an option means the option's update is optional
+so if the client do not support it, that option will just be ignored without
+making fail the entire command. The '-' and '?' symbols can be used together.
+
+Option Format Ex.
+  `-?option`, `-option`, `?option parameters` are valid formats,
+  `?-option` is not a valid format.
+
+Example
+  push-update-broad "route 10.10.10.1 255.255.255.255, -dns, ?tun-mtu 1400"
+
+COMMAND -- push-update-cid (OpenVPN 2.7 or higher)
+----------------------------------------------------
+Same as push-update-broad but you must target a single client using client id.
+
+Example
+  push-update-cid 42 "route 10.10.10.1 255.255.255.255, -dns, ?tun-mtu 1400"
+
+COMMAND -- push-update-cn (OpenVPN 2.7 or higher)
+----------------------------------------------------
+Same as push-update-broad but target the clients based on the provided common name
+(usually just one client per common name is permitted except if "duplicate-cn" option is used).
+
+Example
+  push-update-cid Client0 "route 10.10.10.1 255.255.255.255, -dns, ?tun-mtu 1400"
+
+COMMAND -- push-update-addr (OpenVPN 2.7 or higher)
+----------------------------------------------------
+Same as push-update-broad but target only the client(s) connecting from the
+provided address (real address). Support both IPv4 and IPv6.
+
+Example
+  push-update-addr 9.9.9.9 1234 "route 10.10.10.1 255.255.255.255, -dns, ?tun-mtu 1400"
+
 OUTPUT FORMAT
 -------------
 
diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c
index 8836e79..251b076 100644
--- a/src/openvpn/manage.c
+++ b/src/openvpn/manage.c
@@ -24,7 +24,6 @@
 #ifdef HAVE_CONFIG_H
 #include "config.h"
 #endif
-
 #include "syshead.h"
 
 #ifdef ENABLE_MANAGEMENT
@@ -42,6 +41,7 @@
 #include "manage.h"
 #include "openvpn.h"
 #include "dco.h"
+#include "push.h"
 
 #include "memdbg.h"
 
@@ -124,6 +124,11 @@
     msg(M_CLIENT, "username type u        : Enter username u for a queried OpenVPN username.");
     msg(M_CLIENT, "verb [n]               : Set log verbosity level to n, or show if n is absent.");
     msg(M_CLIENT, "version [n]            : Set client's version to n or show current version of daemon.");
+    msg(M_CLIENT, "push-update-broad options : Broadcast a message to update the specified options.");
+    msg(M_CLIENT, "                            Ex. push-update-broad \"route something, -dns\"");
+    msg(M_CLIENT, "push-update-cid CID options : Send an update message to the client identified by CID.");
+    msg(M_CLIENT, "push-update-cn CN options : Send an update message to the client(s) with the specified Common Name.");
+    msg(M_CLIENT, "push-update-addr ip port options : Send an update message to the client(s) connecting from the provided address.");
     msg(M_CLIENT, "END");
 }
 
@@ -1335,6 +1340,154 @@
 }
 
 static void
+man_push_update(struct management *man, const char **p, const push_update_type type)
+{
+    if (type == UPT_BROADCAST)
+    {
+        if (!man->persist.callback.push_update_broadcast)
+        {
+            man_command_unsupported("push-update-broad");
+            return;
+        }
+
+        const bool status = (*man->persist.callback.push_update_broadcast)(man->persist.callback.arg, p[1]);
+
+        if (status)
+        {
+            msg(M_CLIENT, "SUCCESS: push-update-broad command succeeded");
+        }
+        else
+        {
+            msg(M_CLIENT, "ERROR: push-update-broad command failed");
+        }
+    }
+    else if (type == UPT_BY_CID)
+    {
+        if (!man->persist.callback.push_update_by_cid)
+        {
+            man_command_unsupported("push-update-cid");
+            return;
+        }
+
+        unsigned long cid = 0;
+
+        if (!parse_cid(p[1], &cid))
+        {
+            msg(M_CLIENT, "ERROR: push-update-cid fail during cid parsing");
+            return;
+        }
+
+        const bool status = (*man->persist.callback.push_update_by_cid)(man->persist.callback.arg, cid, p[2]);
+
+        if (status)
+        {
+            msg(M_CLIENT, "SUCCESS: push-update-cid command succeeded");
+        }
+        else
+        {
+            msg(M_CLIENT, "ERROR: push-update-cid command failed");
+        }
+    }
+    else if (type == UPT_BY_CN)
+    {
+        if (!man->persist.callback.push_update_by_cn)
+        {
+            man_command_unsupported("push-update-cn");
+            return;
+        }
+
+        const bool status = (*man->persist.callback.push_update_by_cn)(man->persist.callback.arg, p[1], p[2]);
+
+        if (status)
+        {
+            msg(M_CLIENT, "SUCCESS: push-update-cn command succeeded");
+        }
+        else
+        {
+            msg(M_CLIENT, "ERROR: push-update-cn command failed");
+        }
+    }
+    else if (type == UPT_BY_ADDR)
+    {
+        if (!man->persist.callback.push_update_by_addr)
+        {
+            man_command_unsupported("push-update-addr");
+            return;
+        }
+
+        const char *ip_str = p[1];
+        const char *port_str = p[2];
+        const char *options = p[3];
+
+        if (!strlen(ip_str) || !strlen(port_str))
+        {
+            msg(M_CLIENT, "ERROR: push-update-addr parse");
+            return;
+        }
+
+        struct addrinfo *res = NULL;
+        int port = atoi(port_str);
+
+        if (port < 1 || port > 65535)
+        {
+            msg(M_CLIENT, "ERROR: port number is out of range: %s", port_str);
+            return;
+        }
+
+        int status = openvpn_getaddrinfo(GETADDR_MSG_VIRT_OUT, ip_str, port_str, 0, NULL, AF_UNSPEC, &res);
+
+        if (status != 0 || !res)
+        {
+            msg(M_CLIENT, "ERROR: error resolving address: %s (%s)", ip_str, gai_strerror(status));
+            return;
+        }
+
+        struct addrinfo *rp;
+        bool found_client = false;
+
+        /* Iterate through resolved addresses */
+        for (rp = res; rp != NULL; rp = rp->ai_next)
+        {
+            struct openvpn_sockaddr saddr;
+            struct mroute_addr maddr;
+
+            CLEAR(saddr);
+            switch (rp->ai_family)
+            {
+                case AF_INET:
+                    saddr.addr.in4 = *((struct sockaddr_in *)rp->ai_addr);
+                    break;
+
+                case AF_INET6:
+                    saddr.addr.in6 = *((struct sockaddr_in6 *)rp->ai_addr);
+                    break;
+
+                default:
+                    continue;
+            }
+
+            if (!mroute_extract_openvpn_sockaddr(&maddr, &saddr, true))
+            {
+                continue;
+            }
+
+            if ((*man->persist.callback.push_update_by_addr)(man->persist.callback.arg, &maddr, options))
+            {
+                msg(M_CLIENT, "SUCCESS: push-update sent to %s:%d", ip_str, port);
+                found_client = true;
+                break;
+            }
+        }
+
+        if (!found_client)
+        {
+            msg(M_CLIENT, "ERROR: no client found at address %s:%d", ip_str, port);
+        }
+        freeaddrinfo(res);
+    }
+}
+
+static void
 man_dispatch_command(struct management *man, struct status_output *so, const char **p, const int nparms)
 {
     struct gc_arena gc = gc_new();
@@ -1656,6 +1809,34 @@
             man_remote(man, p);
         }
     }
+    else if (streq(p[0], "push-update-broad"))
+    {
+        if (man_need(man, p, 1, 0))
+        {
+            man_push_update(man, p, UPT_BROADCAST);
+        }
+    }
+    else if (streq(p[0], "push-update-cid"))
+    {
+        if (man_need(man, p, 2, 0))
+        {
+            man_push_update(man, p, UPT_BY_CID);
+        }
+    }
+    else if (streq(p[0], "push-update-cn"))
+    {
+        if (man_need(man, p, 2, 0))
+        {
+            man_push_update(man, p, UPT_BY_CN);
+        }
+    }
+    else if (streq(p[0], "push-update-addr"))
+    {
+        if (man_need(man, p, 3, 0))
+        {
+            man_push_update(man, p, UPT_BY_ADDR);
+        }
+    }
 #if 1
     else if (streq(p[0], "test"))
     {
diff --git a/src/openvpn/manage.h b/src/openvpn/manage.h
index eb19a4e..fd7cb11 100644
--- a/src/openvpn/manage.h
+++ b/src/openvpn/manage.h
@@ -44,7 +44,6 @@
 #define MF_EXTERNAL_KEY_PSSPAD      (1<<16)
 #define MF_EXTERNAL_KEY_DIGEST      (1<<17)
 
-
 #ifdef ENABLE_MANAGEMENT
 
 #include "misc.h"
@@ -205,6 +204,10 @@
 #endif
     unsigned int (*remote_entry_count)(void *arg);
     bool (*remote_entry_get)(void *arg, unsigned int index, char **remote);
+    bool (*push_update_broadcast)(void *arg, const char *options);
+    bool (*push_update_by_cid)(void *arg, unsigned long cid, const char *options);
+    bool (*push_update_by_cn)(void *arg, const char *cn, const char *options);
+    bool (*push_update_by_addr)(void *arg, const struct mroute_addr *maddr, const char *options);
 };
 
 /*
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 7f0d890..3daf358 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -4072,7 +4072,7 @@
     }
 }
 
-static struct multi_instance *
+struct multi_instance *
 lookup_by_cid(struct multi_context *m, const unsigned long cid)
 {
     if (m)
@@ -4220,6 +4220,10 @@
         cb.client_auth = management_client_auth;
         cb.client_pending_auth = management_client_pending_auth;
         cb.get_peer_info = management_get_peer_info;
+        cb.push_update_broadcast = management_callback_send_push_update_broadcast;
+        cb.push_update_by_cid = management_callback_send_push_update_by_cid;
+        cb.push_update_by_cn = management_callback_send_push_update_by_cn;
+        cb.push_update_by_addr = management_callback_send_push_update_by_addr;
         management_set_callback(management, &cb);
     }
 #endif /* ifdef ENABLE_MANAGEMENT */
diff --git a/src/openvpn/multi.h b/src/openvpn/multi.h
index 40f7519..1e965a4 100644
--- a/src/openvpn/multi.h
+++ b/src/openvpn/multi.h
@@ -710,5 +710,10 @@
  */
 void multi_assign_peer_id(struct multi_context *m, struct multi_instance *mi);
 
+#ifdef ENABLE_MANAGEMENT
+struct multi_instance *
+lookup_by_cid(struct multi_context *m, const unsigned long cid);
+
+#endif
 
 #endif /* MULTI_H */
diff --git a/src/openvpn/push.h b/src/openvpn/push.h
index 18dfcd8..e67c3ac 100644
--- a/src/openvpn/push.h
+++ b/src/openvpn/push.h
@@ -42,6 +42,16 @@
 #define PUSH_OPT_TO_REMOVE (1<<0)
 #define PUSH_OPT_OPTIONAL (1<<1)
 
+/* Push-update message sender modes */
+typedef enum {
+    UPT_BROADCAST = 0,
+    UPT_BY_ADDR = 1,
+    UPT_BY_CN = 2,
+#ifdef ENABLE_MANAGEMENT
+    UPT_BY_CID = 3
+#endif
+} push_update_type;
+
 int process_incoming_push_request(struct context *c);
 
 /**
@@ -134,4 +144,33 @@
 void
 receive_auth_pending(struct context *c, const struct buffer *buffer);
 
+/**
+ * @brief A function to send a PUSH_UPDATE control message from server to client(s).
+ *
+ * @param m the multi_context, contains all the clients connected to this server.
+ * @param target the target to which to send the message. It should be:
+ * `NULL` if `type == UPT_BROADCAST`,
+ * a `mroute_addr *` if `type == UPT_BY_ADDR`,
+ * a `char *` if `type == UPT_BY_CN`,
+ * an `unsigned long *` if `type == UPT_BY_CID`.
+ * @param msg a string containing the options to send.
+ * @param type the way to address the message (broadcast, by cid, by cn, by address).
+ * @param push_bundle_size the maximum size of a bundle of pushed option. Just use PUSH_BUNDLE_SIZE macro.
+ * @return the number of clients to which the message was sent.
+ */
+int
+send_push_update(struct multi_context *m, const void *target, const char *msg, const push_update_type type, const int push_bundle_size);
+
+#ifdef ENABLE_MANAGEMENT
+
+bool management_callback_send_push_update_broadcast(void *arg, const char *options);
+
+bool management_callback_send_push_update_by_cid(void *arg, unsigned long cid, const char *options);
+
+bool management_callback_send_push_update_by_cn(void *arg, const char *cn, const char *options);
+
+bool management_callback_send_push_update_by_addr(void *arg, const struct mroute_addr *maddr, const char *options);
+
+#endif /* ifdef ENABLE_MANAGEMENT*/
+
 #endif /* ifndef PUSH_H */
diff --git a/src/openvpn/push_util.c b/src/openvpn/push_util.c
index b4d1e8b..28b303e 100644
--- a/src/openvpn/push_util.c
+++ b/src/openvpn/push_util.c
@@ -3,6 +3,8 @@
 #endif
 
 #include "push.h"
+#include "multi.h"
+#include "ssl_verify.h"
 
 int
 process_incoming_push_update(struct context *c,
@@ -42,3 +44,279 @@
 
     return ret;
 }
+
+/**
+ * Return index of last `,` or `0` if it didn't find any.
+ * If there is a comma at index `0` it's an error anyway
+ */
+static int
+find_first_comma_of_next_bundle(const char *str, int ix)
+{
+    while (ix > 0)
+    {
+        if (str[ix] == ',')
+        {
+            return ix;
+        }
+        ix--;
+    }
+    return 0;
+}
+
+/* Allocate memory and asseble the final message */
+static char *
+forge_msg(const char *src, const char *continuation, struct gc_arena *gc)
+{
+    int src_len = strlen(src);
+    int con_len = continuation ? strlen(continuation) : 0;
+    char *ret = gc_malloc((src_len + sizeof(push_update_cmd) + con_len + 2) * sizeof(char), true, gc);
+    int i = sizeof(push_update_cmd) -1;
+
+    strcpy(ret, push_update_cmd);
+    ret[i++] = ',';
+    strcpy(&ret[i], src);
+    if (continuation)
+    {
+        i += src_len;
+        strcpy(&ret[i], continuation);
+    }
+    return ret;
+}
+
+static char *
+gc_strdup(const char *src, struct gc_arena *gc)
+{
+    char *ret = gc_malloc((strlen(src) + 1) * sizeof(char), true, gc);
+
+    strcpy(ret, src);
+    return ret;
+}
+
+/* It split the messagge (if necessay) and fill msgs with the message chunks.
+ * Return `false` on failure an `true` on success.
+ */
+static bool
+message_splitter(char *str, char **msgs, struct gc_arena *gc, const int safe_cap)
+{
+    if (!str || !*str)
+    {
+        return false;
+    }
+
+    int i = 0;
+    int im = 0;
+
+    while (*str)
+    {
+        /* + ',' - '/0' */
+        if (strlen(str) > safe_cap)
+        {
+            int ci = find_first_comma_of_next_bundle(str, safe_cap);
+            if (!ci)
+            {
+                /* if no commas were found go to fail, do not send any message */
+                return false;
+            }
+            str[ci] = '\0';
+            /* copy from i to (ci -1) */
+            msgs[im] = forge_msg(str, ",push-continuation 2", gc);
+            i = ci + 1;
+        }
+        else
+        {
+            if (im)
+            {
+                msgs[im] = forge_msg(str, ",push-continuation 1", gc);
+            }
+            else
+            {
+                msgs[im] = forge_msg(str, NULL, gc);
+            }
+            i = strlen(str);
+        }
+        str = &str[i];
+        im++;
+    }
+    return true;
+}
+
+/* It actually send the already divided messagge to one single client */
+static bool
+send_single_push_update(struct context *c, char **msgs)
+{
+    if (!msgs[0] || !*msgs[0])
+    {
+        return false;
+    }
+    int i = 0;
+    struct gc_arena gc = gc_new();
+
+    while (msgs[i] && *msgs[i])
+    {
+        unsigned int option_types_found = 0;
+        struct buffer buf = alloc_buf_gc(strlen(msgs[i]), &gc);
+
+        buf_write(&buf, msgs[i], strlen(msgs[i]));
+        if (!send_control_channel_string(c, msgs[i], D_PUSH))
+        {
+            return false;
+        }
+        i++;
+
+        /* After sending the control message, we update the options server-side in the client's context */
+        buf_string_compare_advance(&buf, push_update_cmd);
+        if (process_incoming_push_update(c, pull_permission_mask(c), &option_types_found, &buf) == PUSH_MSG_ERROR)
+        {
+            msg(M_WARN, "Failed to process push update message sent to client ID: %u",
+                c->c2.tls_multi ? c->c2.tls_multi->peer_id : UINT32_MAX);
+            continue;
+        }
+        c->options.push_option_types_found |= option_types_found;
+        if (!options_postprocess_pull(&c->options, c->c2.es))
+        {
+            msg(M_WARN, "Failed to post-process push update message sent to client ID: %u",
+                c->c2.tls_multi ? c->c2.tls_multi->peer_id : UINT32_MAX);
+        }
+    }
+    gc_free(&gc);
+    return true;
+}
+
+int
+send_push_update(struct multi_context *m, const void *target, const char *msg, const push_update_type type, const int push_bundle_size)
+{
+    if (!msg || !*msg || !m
+        || (!target && type != UPT_BROADCAST))
+    {
+        return -EINVAL;
+    }
+
+    struct gc_arena gc = gc_new();
+    /* extra space for possible trailing ifconfig and push-continuation */
+    const int extra = 84 + sizeof(push_update_cmd);
+    /* push_bundle_size is the maximum size of a message, so if the message
+     * we want to send exceeds that size we have to split it into smaller messages */
+    const int safe_cap = push_bundle_size - extra;
+    int msgs_num = (strlen(msg) / safe_cap) + ((strlen(msg) % safe_cap) != 0);
+    char **msgs = gc_malloc(sizeof(char *) * (msgs_num + 1), true, &gc);
+
+    msgs[msgs_num] = NULL;
+    if (!message_splitter(gc_strdup(msg, &gc), msgs, &gc, safe_cap))
+    {
+        gc_free(&gc);
+        return -EINVAL;
+    }
+
+#ifdef ENABLE_MANAGEMENT
+    if (type == UPT_BY_CID)
+    {
+        struct multi_instance *mi = lookup_by_cid(m, *((unsigned long *)target));
+
+        if (!mi)
+        {
+            return -ENOENT;
+        }
+        if (!mi->halt
+            && send_single_push_update(&mi->context, msgs))
+        {
+            gc_free(&gc);
+            return 1;
+        }
+        else
+        {
+            gc_free(&gc);
+            return 0;
+        }
+    }
+#endif /* ifdef ENABLE_MANAGEMENT */
+
+    int count = 0;
+    struct hash_iterator hi;
+    const struct hash_element *he;
+
+    hash_iterator_init(m->iter, &hi);
+    while ((he = hash_iterator_next(&hi)))
+    {
+        struct multi_instance *curr_mi = he->value;
+
+        if (curr_mi->halt)
+        {
+            continue;
+        }
+        if (type == UPT_BY_ADDR && !mroute_addr_equal(target, &curr_mi->real))
+        {
+            continue;
+        }
+        else if (type == UPT_BY_CN)
+        {
+            const char *curr_cn = tls_common_name(curr_mi->context.c2.tls_multi, false);
+            if (strcmp(curr_cn, target))
+            {
+                continue;
+            }
+        }
+        /* Either we found a matching client or type is UPT_BROADCAST so we update every client */
+        if (!send_single_push_update(&curr_mi->context, msgs))
+        {
+            msg(M_CLIENT, "ERROR: Peer ID: %u has not been updated",
+                curr_mi->context.c2.tls_multi ? curr_mi->context.c2.tls_multi->peer_id : UINT32_MAX);
+            continue;
+        }
+        count++;
+    }
+
+    hash_iterator_free(&hi);
+    gc_free(&gc);
+    return count;
+}
+
+#ifdef ENABLE_MANAGEMENT
+#define RETURN_UPDATE_STATUS(n_sent) \
+    do { \
+        if ((n_sent) > 0) { \
+            msg(M_CLIENT, "SUCCESS: %d client(s) updated", (n_sent)); \
+            return true; \
+        } else { \
+            msg(M_CLIENT, "ERROR: no client updated"); \
+            return false; \
+        } \
+    } while (0)
+
+
+bool
+management_callback_send_push_update_broadcast(void *arg, const char *options)
+{
+    int n_sent = send_push_update(arg, NULL, options, UPT_BROADCAST, PUSH_BUNDLE_SIZE);
+
+    RETURN_UPDATE_STATUS(n_sent);
+}
+
+bool
+management_callback_send_push_update_by_cid(void *arg, unsigned long cid, const char *options)
+{
+    int ret = send_push_update(arg, &cid, options, UPT_BY_CID, PUSH_BUNDLE_SIZE);
+
+    if (ret == -ENOENT)
+    {
+        msg(M_CLIENT, "ERROR: no client found with CID: %lu", cid);
+    }
+
+    return (ret > 0);
+}
+
+bool
+management_callback_send_push_update_by_cn(void *arg, const char *cn, const char *options)
+{
+    int n_sent = send_push_update(arg, cn, options, UPT_BY_CN, PUSH_BUNDLE_SIZE);
+
+    RETURN_UPDATE_STATUS(n_sent);
+}
+
+bool
+management_callback_send_push_update_by_addr(void *arg, const struct mroute_addr *maddr, const char *options)
+{
+    int n_sent = send_push_update(arg, maddr, options, UPT_BY_ADDR, PUSH_BUNDLE_SIZE);
+
+    RETURN_UPDATE_STATUS(n_sent);
+}
+#endif /* ifdef ENABLE_MANAGEMENT */
diff --git a/tests/unit_tests/openvpn/Makefile.am b/tests/unit_tests/openvpn/Makefile.am
index b24e03c..9a40512 100644
--- a/tests/unit_tests/openvpn/Makefile.am
+++ b/tests/unit_tests/openvpn/Makefile.am
@@ -343,4 +343,5 @@
 	$(top_srcdir)/src/openvpn/platform.c \
 	$(top_srcdir)/src/openvpn/push_util.c \
 	$(top_srcdir)/src/openvpn/options_util.c \
-	$(top_srcdir)/src/openvpn/otime.c
\ No newline at end of file
+	$(top_srcdir)/src/openvpn/otime.c \
+	$(top_srcdir)/src/openvpn/list.c
\ No newline at end of file
diff --git a/tests/unit_tests/openvpn/test_push_update_msg.c b/tests/unit_tests/openvpn/test_push_update_msg.c
index d0876bc..8420510 100644
--- a/tests/unit_tests/openvpn/test_push_update_msg.c
+++ b/tests/unit_tests/openvpn/test_push_update_msg.c
@@ -8,6 +8,7 @@
 #include <cmocka.h>
 #include "push.h"
 #include "options_util.h"
+#include "multi.h"
 
 /* mocks */
 
@@ -37,6 +38,12 @@
 }
 
 bool
+options_postprocess_pull(struct options *options, struct env_set *es)
+{
+    return true;
+}
+
+bool
 apply_push_options(struct context *c,
                    struct options *options,
                    struct buffer *buf,
@@ -94,6 +101,49 @@
     }
 }
 
+const char *
+tls_common_name(const struct tls_multi *multi, const bool null)
+{
+    return NULL;
+}
+
+#ifndef ENABLE_MANAGEMENT
+bool
+send_control_channel_string(struct context *c, const char *str, int msglevel)
+{
+    return true;
+}
+#else  /* ifndef ENABLE_MANAGEMENT */
+char **res;
+int i;
+
+bool
+send_control_channel_string(struct context *c, const char *str, int msglevel)
+{
+    if (res && res[i] && strcmp(res[i], str))
+    {
+        printf("\n\nexpected: %s\n\n  actual: %s\n\n", res[i], str);
+        return false;
+    }
+    i++;
+    return true;
+}
+
+struct multi_instance *
+lookup_by_cid(struct multi_context *m, const unsigned long cid)
+{
+    return *(m->instances);
+}
+
+bool
+mroute_extract_openvpn_sockaddr(struct mroute_addr *addr,
+                                const struct openvpn_sockaddr *osaddr,
+                                bool use_port)
+{
+    return true;
+}
+#endif /* ifndef ENABLE_MANAGEMENT */
+
 /* tests */
 
 static void
@@ -124,7 +174,6 @@
     free_buf(&buf);
 }
 
-
 static void
 test_incoming_push_message_error2(void **state)
 {
@@ -209,6 +258,205 @@
     free_buf(&buf);
 }
 
+#ifdef ENABLE_MANAGEMENT
+char *r0[] = {
+    "PUSH_UPDATE,redirect-gateway local,route 192.168.1.0 255.255.255.0"
+};
+char *r1[] = {
+    "PUSH_UPDATE,-dhcp-option,blablalalalalalalalalalalalalf, lalalalalalalalalalalalalalaf,push-continuation 2",
+    "PUSH_UPDATE, akakakakakakakakakakakaf, dhcp-option DNS 8.8.8.8,redirect-gateway local,push-continuation 2",
+    "PUSH_UPDATE,route 192.168.1.0 255.255.255.0,push-continuation 1"
+};
+char *r3[] = {
+    "PUSH_UPDATE,,,"
+};
+char *r4[] = {
+    "PUSH_UPDATE,-dhcp-option, blablalalalalalalalalalalalalf, lalalalalalalalalalalalalalaf,push-continuation 2",
+    "PUSH_UPDATE, akakakakakakakakakakakaf,dhcp-option DNS 8.8.8.8, redirect-gateway local,push-continuation 2",
+    "PUSH_UPDATE, route 192.168.1.0 255.255.255.0,,push-continuation 1"
+};
+char *r5[] = {
+    "PUSH_UPDATE,,-dhcp-option, blablalalalalalalalalalalalalf, lalalalalalalalalalalalalalaf,push-continuation 2",
+    "PUSH_UPDATE, akakakakakakakakakakakaf,dhcp-option DNS 8.8.8.8, redirect-gateway local,push-continuation 2",
+    "PUSH_UPDATE, route 192.168.1.0 255.255.255.0,push-continuation 1"
+};
+char *r6[] = {
+    "PUSH_UPDATE,-dhcp-option,blablalalalalalalalalalalalalf, lalalalalalalalalalalalalalaf,push-continuation 2",
+    "PUSH_UPDATE, akakakakakakakakakakakaf, dhcp-option DNS 8.8.8.8, redirect-gateway 10.10.10.10,,push-continuation 2",
+    "PUSH_UPDATE, route 192.168.1.0 255.255.255.0,,push-continuation 1"
+};
+char *r7[] = {
+    "PUSH_UPDATE,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,push-continuation 2",
+    "PUSH_UPDATE,,,,,,,,,,,,,,,,,,,push-continuation 1"
+};
+char *r8[] = {
+    "PUSH_UPDATE,-dhcp-option,blablalalalalalalalalalalalalf, lalalalalalalalalalalalalalaf,push-continuation 2",
+    "PUSH_UPDATE, akakakakakakakakakakakaf, dhcp-option DNS 8.8.8.8,redirect-gateway\n local,push-continuation 2",
+    "PUSH_UPDATE,route 192.168.1.0 255.255.255.0\n\n\n,push-continuation 1"
+};
+char *r9[] = {
+    "PUSH_UPDATE,,"
+};
+
+
+const char *msg0 = "redirect-gateway local,route 192.168.1.0 255.255.255.0";
+const char *msg1 = "-dhcp-option,blablalalalalalalalalalalalalf, lalalalalalalalalalalalalalaf,"
+                   " akakakakakakakakakakakaf, dhcp-option DNS 8.8.8.8,redirect-gateway local,route 192.168.1.0 255.255.255.0";
+const char *msg2 = "";
+const char *msg3 = ",,";
+const char *msg4 = "-dhcp-option, blablalalalalalalalalalalalalf, lalalalalalalalalalalalalalaf,"
+                   " akakakakakakakakakakakaf,dhcp-option DNS 8.8.8.8, redirect-gateway local, route 192.168.1.0 255.255.255.0,";
+const char *msg5 = ",-dhcp-option, blablalalalalalalalalalalalalf, lalalalalalalalalalalalalalaf,"
+                   " akakakakakakakakakakakaf,dhcp-option DNS 8.8.8.8, redirect-gateway local, route 192.168.1.0 255.255.255.0";
+const char *msg6 = "-dhcp-option,blablalalalalalalalalalalalalf, lalalalalalalalalalalalalalaf, akakakakakakakakakakakaf,"
+                   " dhcp-option DNS 8.8.8.8, redirect-gateway 10.10.10.10,, route 192.168.1.0 255.255.255.0,";
+const char *msg7 = ",,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,";
+const char *msg8 = "-dhcp-option,blablalalalalalalalalalalalalf, lalalalalalalalalalalalalalaf, akakakakakakakakakakakaf,"
+                   " dhcp-option DNS 8.8.8.8,redirect-gateway\n local,route 192.168.1.0 255.255.255.0\n\n\n";
+const char *msg9 = ",";
+const char *msg10 = "Voilà! In view, a humble vaudevillian veteran cast vicariously as both victim and villain by the vicissitudes"
+                    " of Fate. This visage no mere veneer of vanity is a vestige of the vox populi now vacant vanished. However this"
+                    " valorous visitation of a by-gone vexation stands vivified and has vowed to vanquish these venal and virulent"
+                    " vermin vanguarding vice and vouchsafing the violently vicious and voracious violation of volition. The only"
+                    " verdict is vengeance; a vendetta held as a votive not in vain for the value and veracity of such shall one"
+                    " day vindicate the vigilant and the virtuous. Verily this vichyssoise of verbiage veers most verbose so let"
+                    " me simply add that it is my very good honor to meet you and you may call me V.";
+
+#define PUSH_BUNDLE_SIZE_TEST 184
+
+static void
+test_send_push_msg0(void **state)
+{
+    i = 0;
+    res = r0;
+    struct multi_context *m = *state;
+    const unsigned long cid = 0;
+    assert_int_equal(send_push_update(m, &cid, msg0, UPT_BY_CID, PUSH_BUNDLE_SIZE_TEST), 1);
+}
+static void
+test_send_push_msg1(void **state)
+{
+    i = 0;
+    res = r1;
+    struct multi_context *m = *state;
+    const unsigned long cid = 0;
+    assert_int_equal(send_push_update(m, &cid, msg1, UPT_BY_CID, PUSH_BUNDLE_SIZE_TEST), 1);
+}
+
+static void
+test_send_push_msg2(void **state)
+{
+    i = 0;
+    res = NULL;
+    struct multi_context *m = *state;
+    const unsigned long cid = 0;
+    assert_int_equal(send_push_update(m, &cid, msg2, UPT_BY_CID, PUSH_BUNDLE_SIZE_TEST), -EINVAL);
+}
+
+static void
+test_send_push_msg3(void **state)
+{
+    i = 0;
+    res = r3;
+    struct multi_context *m = *state;
+    const unsigned long cid = 0;
+    assert_int_equal(send_push_update(m, &cid, msg3, UPT_BY_CID, PUSH_BUNDLE_SIZE_TEST), 1);
+}
+
+static void
+test_send_push_msg4(void **state)
+{
+    i = 0;
+    res = r4;
+    struct multi_context *m = *state;
+    const unsigned long cid = 0;
+    assert_int_equal(send_push_update(m, &cid, msg4, UPT_BY_CID, PUSH_BUNDLE_SIZE_TEST), 1);
+}
+
+static void
+test_send_push_msg5(void **state)
+{
+    i = 0;
+    res = r5;
+    struct multi_context *m = *state;
+    const unsigned long cid = 0;
+    assert_int_equal(send_push_update(m, &cid, msg5, UPT_BY_CID, PUSH_BUNDLE_SIZE_TEST), 1);
+}
+
+static void
+test_send_push_msg6(void **state)
+{
+    i = 0;
+    res = r6;
+    struct multi_context *m = *state;
+    const unsigned long cid = 0;
+    assert_int_equal(send_push_update(m, &cid, msg6, UPT_BY_CID, PUSH_BUNDLE_SIZE_TEST), 1);
+}
+
+static void
+test_send_push_msg7(void **state)
+{
+    i = 0;
+    res = r7;
+    struct multi_context *m = *state;
+    const unsigned long cid = 0;
+    assert_int_equal(send_push_update(m, &cid, msg7, UPT_BY_CID, PUSH_BUNDLE_SIZE_TEST), 1);
+}
+
+static void
+test_send_push_msg8(void **state)
+{
+    i = 0;
+    res = r8;
+    struct multi_context *m = *state;
+    const unsigned long cid = 0;
+    assert_int_equal(send_push_update(m, &cid, msg8, UPT_BY_CID, PUSH_BUNDLE_SIZE_TEST), 1);
+}
+
+static void
+test_send_push_msg9(void **state)
+{
+    i = 0;
+    res = r9;
+    struct multi_context *m = *state;
+    const unsigned long cid = 0;
+    assert_int_equal(send_push_update(m, &cid, msg9, UPT_BY_CID, PUSH_BUNDLE_SIZE_TEST), 1);
+}
+
+static void
+test_send_push_msg10(void **state)
+{
+    i = 0;
+    res = NULL;
+    struct multi_context *m = *state;
+    const unsigned long cid = 0;
+    assert_int_equal(send_push_update(m, &cid, msg10, UPT_BY_CID, PUSH_BUNDLE_SIZE_TEST), -EINVAL);
+}
+
+#undef PUSH_BUNDLE_SIZE_TEST
+
+static int
+setup2(void **state)
+{
+    struct multi_context *m = calloc(1, sizeof(struct multi_context));
+    m->instances = calloc(1, sizeof(struct multi_instance *));
+    struct multi_instance *mi = calloc(1, sizeof(struct multi_instance));
+    *(m->instances) = mi;
+    *state = m;
+    return 0;
+}
+
+static int
+teardown2(void **state)
+{
+    struct multi_context *m = *state;
+    free(*(m->instances));
+    free(m->instances);
+    free(m);
+    return 0;
+}
+#endif /* ifdef ENABLE_MANAGEMENT */
+
 static int
 setup(void **state)
 {
@@ -238,7 +486,20 @@
         cmocka_unit_test_setup_teardown(test_incoming_push_message_1, setup, teardown),
         cmocka_unit_test_setup_teardown(test_incoming_push_message_bad_format, setup, teardown),
         cmocka_unit_test_setup_teardown(test_incoming_push_message_mix, setup, teardown),
-        cmocka_unit_test_setup_teardown(test_incoming_push_message_mix2, setup, teardown)
+        cmocka_unit_test_setup_teardown(test_incoming_push_message_mix2, setup, teardown),
+#ifdef ENABLE_MANAGEMENT
+        cmocka_unit_test_setup_teardown(test_send_push_msg0, setup2, teardown2),
+        cmocka_unit_test_setup_teardown(test_send_push_msg1, setup2, teardown2),
+        cmocka_unit_test_setup_teardown(test_send_push_msg2, setup2, teardown2),
+        cmocka_unit_test_setup_teardown(test_send_push_msg3, setup2, teardown2),
+        cmocka_unit_test_setup_teardown(test_send_push_msg4, setup2, teardown2),
+        cmocka_unit_test_setup_teardown(test_send_push_msg5, setup2, teardown2),
+        cmocka_unit_test_setup_teardown(test_send_push_msg6, setup2, teardown2),
+        cmocka_unit_test_setup_teardown(test_send_push_msg7, setup2, teardown2),
+        cmocka_unit_test_setup_teardown(test_send_push_msg8, setup2, teardown2),
+        cmocka_unit_test_setup_teardown(test_send_push_msg9, setup2, teardown2),
+        cmocka_unit_test_setup_teardown(test_send_push_msg10, setup2, teardown2)
+#endif
     };
 
     return cmocka_run_group_tests(tests, NULL, NULL);

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

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ie82bcc7a8e583de9156b185d71d1a323ed8df3fc
Gerrit-Change-Number: 869
Gerrit-PatchSet: 8
Gerrit-Owner: mrbff <ma...@ma...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-Reviewer: stipa <lst...@gm...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: cron2 <ge...@gr...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: stipa <lst...@gm...>
Gerrit-Attention: mrbff <ma...@ma...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [S] Change in openvpn[master]: mac dns: compare servers before restoring backup

[flichtenheld (C. Review) <ge...@op...>]

Attention is currently required from: d12fk, plaisthos.

flichtenheld has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/1075?usp=email )

Change subject: mac dns: compare servers before restoring backup
......................................................................


Patch Set 2: Code-Review-2

(1 comment)

Patchset:

PS2: 
Fails t_client tests on macos buildbot worker. After some investigation the problem seems to be that the IPv6 DNS server address is not shown in global DNS configuration:

```
> show Setup:/Network/Service/0347C55F-C172-49FF-BECC-C533F04B4B13/DNS
<dictionary> {
  SearchDomains : <array> {
    0 : open.vpn
  }
  SearchOrder : 5000
  ServerAddresses : <array> {
    0 : 10.194.0.1
    1 : fd00:abcd:194::1
  }
}
> show State:/Network/Global/DNS
<dictionary> {
  SearchDomains : <array> {
    0 : open.vpn
  }
  SearchOrder : 5000
  ServerAddresses : <array> {
    0 : 10.194.0.1
  }
  __CONFIGURATION_ID__ : Default: 0
  __FLAGS__ : 2
  __ORDER__ : 0
}
```



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

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I1aabd62e60dd18408a57baccbb0f4ebd6d2f8d67
Gerrit-Change-Number: 1075
Gerrit-PatchSet: 2
Gerrit-Owner: d12fk <he...@op...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: d12fk <he...@op...>
Gerrit-Comment-Date: Mon, 07 Jul 2025 16:54:05 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Openvpn-devel] [S] Change in openvpn[master]: mac dns: do not run dns-updown in parallel

[d12fk (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, plaisthos.

d12fk has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/1076?usp=email )

Change subject: mac dns: do not run dns-updown in parallel
......................................................................


Patch Set 2:

(1 comment)

File distro/dns-scripts/macos-dns-updown.sh:

http://gerrit.openvpn.net/c/openvpn/+/1076/comment/068034ad_56387d43 :
PS2, Line 30: lockfile=/tmp/openvpn-dns-updown.lock
> shouldn't be this rather  /var/lock/openvpn-dns-updown. […]
There is not /var/lock on my system, that is why I opted for /tmp



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

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I7adfaa08df6a17545cca8264d7230b5e65e49719
Gerrit-Change-Number: 1076
Gerrit-PatchSet: 2
Gerrit-Owner: d12fk <he...@op...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Comment-Date: Mon, 07 Jul 2025 16:14:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: plaisthos <arn...@rf...>
Gerrit-MessageType: comment

[Openvpn-devel] [S] Change in openvpn[master]: mac dns: do not run dns-updown in parallel

[plaisthos (C. Review) <ge...@op...>]

Attention is currently required from: d12fk, flichtenheld.

plaisthos has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/1076?usp=email )

Change subject: mac dns: do not run dns-updown in parallel
......................................................................


Patch Set 2:

(1 comment)

File distro/dns-scripts/macos-dns-updown.sh:

http://gerrit.openvpn.net/c/openvpn/+/1076/comment/e89cd2a5_318167ed :
PS2, Line 30: lockfile=/tmp/openvpn-dns-updown.lock
shouldn't be this rather  /var/lock/openvpn-dns-updown.lock?



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

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I7adfaa08df6a17545cca8264d7230b5e65e49719
Gerrit-Change-Number: 1076
Gerrit-PatchSet: 2
Gerrit-Owner: d12fk <he...@op...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: d12fk <he...@op...>
Gerrit-Comment-Date: Mon, 07 Jul 2025 16:11:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[Openvpn-devel] [S] Change in openvpn[master]: Multi-socket: Fix assert triggered by stale peer-id reuse

[its_Giaan (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, plaisthos.

Hello flichtenheld, plaisthos, 

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

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

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


Change subject: Multi-socket: Fix assert triggered by stale peer-id reuse
......................................................................

Multi-socket: Fix assert triggered by stale peer-id reuse

Fixed a bug where clients using different transport
protocols (UDP, TCP) could interfere with each other
after a server restart.
The issue occurred when a client reused a previously
assigned peer-id that was now associated with a
different client using a different transport protocol.

For example, a UDP client could send packets with a
peer-id now assigned to a TCP client, which lacks
a valid context->c2.from which is filled by the
recvfrom(), causing an assert to be triggered.

A protocol check has been added to prevent packets
from different protocols from hijacking active
connections.

Github: #773

Change-Id: Iecbbcf32c0059f2b16a05333b3794599060d7d6a
Signed-off-by: Gianmarco De Gregori <gia...@ma...>
---
M src/openvpn/mudp.c
1 file changed, 13 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/78/1078/3

diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
index 93e65e0..ee8446a 100644
--- a/src/openvpn/mudp.c
+++ b/src/openvpn/mudp.c
@@ -216,16 +216,20 @@
 
             if (!peer_id_disabled && (peer_id < m->max_clients) && (m->instances[peer_id]))
             {
-                mi = m->instances[peer_id];
-
-                *floated = !link_socket_actual_match(&mi->context.c2.from, &m->top.c2.from);
-
-                if (*floated)
+                /* Floating on TCP will never be possible, so ensure we only process
+                 * UDP clients */
+                if (m->instances[peer_id]->context.c2.link_sockets[0]->info.proto == sock->info.proto)
                 {
-                    /* reset prefix, since here we are not sure peer is the one it claims to be */
-                    ungenerate_prefix(mi);
-                    msg(D_MULTI_MEDIUM, "Float requested for peer %" PRIu32 " to %s", peer_id,
-                        mroute_addr_print(&real, &gc));
+                    mi = m->instances[peer_id];
+                    *floated = !link_socket_actual_match(&mi->context.c2.from, &m->top.c2.from);
+
+                    if (*floated)
+                    {
+                        /* reset prefix, since here we are not sure peer is the one it claims to be */
+                        ungenerate_prefix(mi);
+                        msg(D_MULTI_MEDIUM, "Float requested for peer %" PRIu32 " to %s", peer_id,
+                            mroute_addr_print(&real, &gc));
+                    }
                 }
             }
         }

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

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Iecbbcf32c0059f2b16a05333b3794599060d7d6a
Gerrit-Change-Number: 1078
Gerrit-PatchSet: 3
Gerrit-Owner: its_Giaan <gia...@ma...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [XS] Change in openvpn[master]: Build on all four possible Android ABIs

[plaisthos (C. Review) <ge...@op...>]

plaisthos has abandoned this change. ( http://gerrit.openvpn.net/c/openvpn/+/897?usp=email )

Change subject: Build on all four possible Android ABIs
......................................................................


Abandoned

considered to be too much for now
-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/897?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I09d7e8cf4f15ae5810e0adafda15e489a3375892
Gerrit-Change-Number: 897
Gerrit-PatchSet: 1
Gerrit-Owner: plaisthos <arn...@rf...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: abandon

[Openvpn-devel] [S] Change in openvpn[master]: Multi-socket: Fix assert triggered by stale peer-id reuse

[its_Giaan (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, plaisthos.

its_Giaan has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/1078?usp=email )

Change subject: Multi-socket: Fix assert triggered by stale peer-id reuse
......................................................................


Patch Set 1:

(1 comment)

File src/openvpn/mudp.c:

http://gerrit.openvpn.net/c/openvpn/+/1078/comment/5867a758_0966ea4a :
PS1, Line 225:                     if (mi)
> This looks fishy. This basically can never fail. […]
Done



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

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Iecbbcf32c0059f2b16a05333b3794599060d7d6a
Gerrit-Change-Number: 1078
Gerrit-PatchSet: 1
Gerrit-Owner: its_Giaan <gia...@ma...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Comment-Date: Mon, 07 Jul 2025 15:05:05 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: plaisthos <arn...@rf...>
Gerrit-MessageType: comment

[Openvpn-devel] [S] Change in openvpn[master]: Multi-socket: Fix assert triggered by stale peer-id reuse

[its_Giaan (C. Review) <ge...@op...>]

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

Hello flichtenheld, plaisthos, 

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

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

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

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


Change subject: Multi-socket: Fix assert triggered by stale peer-id reuse
......................................................................

Multi-socket: Fix assert triggered by stale peer-id reuse

Fixed a bug where clients using different transport
protocols (UDP, TCP) could interfere with each other
after a server restart.
The issue occurred when a client reused a previously
assigned peer-id that was now associated with a
different client using a different transport protocol.

For example, a UDP client could send packets with a
peer-id now assigned to a TCP client, which lacks
a valid context->c2.from which is filled by the
recvfrom(), causing an assert to be triggered.

A protocol check has been added to prevent packets
from different protocols from hijacking active
connections.

Github: #773

Change-Id: Iecbbcf32c0059f2b16a05333b3794599060d7d6a
Signed-off-by: Gianmarco De Gregori <gia...@ma...>
---
M src/openvpn/mudp.c
1 file changed, 13 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/78/1078/2

diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
index 93e65e0..f62e0a3 100644
--- a/src/openvpn/mudp.c
+++ b/src/openvpn/mudp.c
@@ -216,16 +216,20 @@
 
             if (!peer_id_disabled && (peer_id < m->max_clients) && (m->instances[peer_id]))
             {
-                mi = m->instances[peer_id];
-
-                *floated = !link_socket_actual_match(&mi->context.c2.from, &m->top.c2.from);
-
-                if (*floated)
+                /* Ensure that clients from previous sessions do not attempt to
+                 * hijack instances of newly connected clients in multi-protocol scenarios */
+                if (m->instances[peer_id]->context.c2.link_sockets[0]->info.proto == sock->info.proto)
                 {
-                    /* reset prefix, since here we are not sure peer is the one it claims to be */
-                    ungenerate_prefix(mi);
-                    msg(D_MULTI_MEDIUM, "Float requested for peer %" PRIu32 " to %s", peer_id,
-                        mroute_addr_print(&real, &gc));
+                    mi = m->instances[peer_id];
+                    *floated = !link_socket_actual_match(&mi->context.c2.from, &m->top.c2.from);
+
+                    if (*floated)
+                    {
+                        /* reset prefix, since here we are not sure peer is the one it claims to be */
+                        ungenerate_prefix(mi);
+                        msg(D_MULTI_MEDIUM, "Float requested for peer %" PRIu32 " to %s", peer_id,
+                            mroute_addr_print(&real, &gc));
+                    }
                 }
             }
         }

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

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Iecbbcf32c0059f2b16a05333b3794599060d7d6a
Gerrit-Change-Number: 1078
Gerrit-PatchSet: 2
Gerrit-Owner: its_Giaan <gia...@ma...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: its_Giaan <gia...@ma...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [S] Change in openvpn[master]: Added PQE to WolfSSL

[cron2 (C. Review) <ge...@op...>]

cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1046?usp=email )

Change subject: Added PQE to WolfSSL
......................................................................

Added PQE to WolfSSL

Change-Id: Ie0529c2074964b3be034f01e0ef53090a6edbd35
Signed-off-by: comododragon <rei...@fo...>
Acked-by: Arne Schwabe <arn...@rf...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg32043.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M README.wolfssl
M src/openvpn/ssl_openssl.c
2 files changed, 31 insertions(+), 1 deletion(-)




diff --git a/README.wolfssl b/README.wolfssl
index a5dfe31..3918d0f 100644
--- a/README.wolfssl
+++ b/README.wolfssl
@@ -28,3 +28,33 @@
  * blowfish support (BF-CBC), you must use something like
    cipher AES-128-CBC to avoid trying to use BF-CBC
  * Windows CryptoAPI support
+
+*************************************************************************
+To build WolfSSL with post-quantum KEMs built in, the following command is used:
+
+./configure --enable-openvpn --enable-kyber=all --enable-curve25519
+
+WolfSSL supports the following post-quantum KEMs and post-quantum hybrid KEMs which must be specified
+using the tls-groups option in an OpenVPN config. Unlike OpenSSL, which includes X25519MLKEM768
+in the default config, WolfSSL requires explicit configuration of tls-groups to include
+at least one post-quantum KEM.
+
+ML_KEM_512
+ML_KEM_768
+ML_KEM_1024
+
+P256_ML_KEM_512
+X25519_ML_KEM_512
+
+P384_ML_KEM_768
+P256_ML_KEM_768
+X448_ML_KEM_768
+X25519_ML_KEM_768
+
+P384_ML_KEM_1024
+P521_ML_KEM_1024
+
+The naming conventions of algorithms differ between WolfSSL and OpenSSL. An example is that
+OpenSSL omits underscores for their naming notation whereas WolfSSL expects them. Additionally,
+OpenSSL does not accept the P curve notation and instead uses the equivalent secp notation.
+A specific example is that WolfSSL expects P384_ML_KEM_1024, while OpenSSL expects secp384r1MLKEM1024.
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 2fc77d8..4c11cd4 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -560,7 +560,7 @@
 tls_ctx_set_tls_groups(struct tls_root_ctx *ctx, const char *groups)
 {
     ASSERT(ctx);
-#if OPENSSL_VERSION_NUMBER < 0x30000000L
+#if OPENSSL_VERSION_NUMBER < 0x30000000L && !defined(ENABLE_CRYPTO_WOLFSSL)
     struct gc_arena gc = gc_new();
     /* This method could be as easy as
      *  SSL_CTX_set1_groups_list(ctx->ctx, groups)

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

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ie0529c2074964b3be034f01e0ef53090a6edbd35
Gerrit-Change-Number: 1046
Gerrit-PatchSet: 7
Gerrit-Owner: comododragon <rei...@fo...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: merged

[Openvpn-devel] [S] Change in openvpn[master]: Added PQE to WolfSSL

[cron2 (C. Review) <ge...@op...>]

cron2 has uploaded a new patch set (#7) to the change originally created by comododragon. ( http://gerrit.openvpn.net/c/openvpn/+/1046?usp=email )

The following approvals got outdated and were removed:
Code-Review+2 by plaisthos


Change subject: Added PQE to WolfSSL
......................................................................

Added PQE to WolfSSL

Change-Id: Ie0529c2074964b3be034f01e0ef53090a6edbd35
Signed-off-by: comododragon <rei...@fo...>
Acked-by: Arne Schwabe <arn...@rf...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg32043.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M README.wolfssl
M src/openvpn/ssl_openssl.c
2 files changed, 31 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/46/1046/7

diff --git a/README.wolfssl b/README.wolfssl
index a5dfe31..3918d0f 100644
--- a/README.wolfssl
+++ b/README.wolfssl
@@ -28,3 +28,33 @@
  * blowfish support (BF-CBC), you must use something like
    cipher AES-128-CBC to avoid trying to use BF-CBC
  * Windows CryptoAPI support
+
+*************************************************************************
+To build WolfSSL with post-quantum KEMs built in, the following command is used:
+
+./configure --enable-openvpn --enable-kyber=all --enable-curve25519
+
+WolfSSL supports the following post-quantum KEMs and post-quantum hybrid KEMs which must be specified
+using the tls-groups option in an OpenVPN config. Unlike OpenSSL, which includes X25519MLKEM768
+in the default config, WolfSSL requires explicit configuration of tls-groups to include
+at least one post-quantum KEM.
+
+ML_KEM_512
+ML_KEM_768
+ML_KEM_1024
+
+P256_ML_KEM_512
+X25519_ML_KEM_512
+
+P384_ML_KEM_768
+P256_ML_KEM_768
+X448_ML_KEM_768
+X25519_ML_KEM_768
+
+P384_ML_KEM_1024
+P521_ML_KEM_1024
+
+The naming conventions of algorithms differ between WolfSSL and OpenSSL. An example is that
+OpenSSL omits underscores for their naming notation whereas WolfSSL expects them. Additionally,
+OpenSSL does not accept the P curve notation and instead uses the equivalent secp notation.
+A specific example is that WolfSSL expects P384_ML_KEM_1024, while OpenSSL expects secp384r1MLKEM1024.
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 2fc77d8..4c11cd4 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -560,7 +560,7 @@
 tls_ctx_set_tls_groups(struct tls_root_ctx *ctx, const char *groups)
 {
     ASSERT(ctx);
-#if OPENSSL_VERSION_NUMBER < 0x30000000L
+#if OPENSSL_VERSION_NUMBER < 0x30000000L && !defined(ENABLE_CRYPTO_WOLFSSL)
     struct gc_arena gc = gc_new();
     /* This method could be as easy as
      *  SSL_CTX_set1_groups_list(ctx->ctx, groups)

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

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ie0529c2074964b3be034f01e0ef53090a6edbd35
Gerrit-Change-Number: 1046
Gerrit-PatchSet: 7
Gerrit-Owner: comododragon <rei...@fo...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [PATCH applied] Re: Added PQE to WolfSSL

[Gert D. <ge...@gr...>]

For anything non-wolfssl this is a no-op, so I did not test anything
(just verified that the buildbots agreed with the "it will not break
anything" assessment).  Arne tested :-)

git declared that it fixed 5 whitespace errors on commit -> "git show"
looks clean now.

Your patch has been applied to the master branch.

commit 1b133cce839f46902c9df32943646c3289c34889
Author: rein.vanbaaren
Date:   Mon Jul 7 15:34:39 2025 +0200

     Added PQE to WolfSSL

     Signed-off-by: comododragon <rei...@fo...>
     Acked-by: Arne Schwabe <arn...@rf...>
     Message-Id: <202...@gr...>
     URL: https://www.mail-archive.com/ope...@li.../msg32043.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [S] Change in openvpn[master]: Multi-socket: Fix assert triggered by stale peer-id reuse

[plaisthos (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, its_Giaan.

plaisthos has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/1078?usp=email )

Change subject: Multi-socket: Fix assert triggered by stale peer-id reuse
......................................................................


Patch Set 1: Code-Review-1

(2 comments)

Patchset:

PS1: 
The check for NULL after already accessing it, looks very fishy.


File src/openvpn/mudp.c:

http://gerrit.openvpn.net/c/openvpn/+/1078/comment/5cf6ed60_e43a0b88 :
PS1, Line 225:                     if (mi)
This looks fishy. This basically can never fail. Since we already access `m->instances[peer_id]` in the if condition, this would have already segfaulted before even getting to this if (mi) check



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

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Iecbbcf32c0059f2b16a05333b3794599060d7d6a
Gerrit-Change-Number: 1078
Gerrit-PatchSet: 1
Gerrit-Owner: its_Giaan <gia...@ma...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: its_Giaan <gia...@ma...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Comment-Date: Mon, 07 Jul 2025 14:05:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Openvpn-devel] [PATCH v6] Added PQE to WolfSSL

[Gert D. <ge...@gr...>]

From: rein.vanbaaren <rei...@fo...>

Change-Id: Ie0529c2074964b3be034f01e0ef53090a6edbd35
Signed-off-by: comododragon <rei...@fo...>
Acked-by: Arne Schwabe <arn...@rf...>
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1046
This mail reflects revision 6 of this Change.

Signed-off-by line for the author was added as per our policy.

Acked-by according to Gerrit (reflected above):
Arne Schwabe <arn...@rf...>

        
diff --git a/README.wolfssl b/README.wolfssl
index a5dfe31..7475164 100644
--- a/README.wolfssl
+++ b/README.wolfssl
@@ -28,3 +28,33 @@
  * blowfish support (BF-CBC), you must use something like
    cipher AES-128-CBC to avoid trying to use BF-CBC
  * Windows CryptoAPI support
+
+*************************************************************************
+To build WolfSSL with post-quantum KEMs built in, the following command is used:
+
+./configure --enable-openvpn --enable-kyber=all --enable-curve25519
+
+WolfSSL supports the following post-quantum KEMs and post-quantum hybrid KEMs which must be specified 
+using the tls-groups option in an OpenVPN config. Unlike OpenSSL, which includes X25519MLKEM768 
+in the default config, WolfSSL requires explicit configuration of tls-groups to include 
+at least one post-quantum KEM.
+
+ML_KEM_512
+ML_KEM_768
+ML_KEM_1024
+
+P256_ML_KEM_512
+X25519_ML_KEM_512
+
+P384_ML_KEM_768
+P256_ML_KEM_768
+X448_ML_KEM_768
+X25519_ML_KEM_768
+
+P384_ML_KEM_1024
+P521_ML_KEM_1024
+
+The naming conventions of algorithms differ between WolfSSL and OpenSSL. An example is that
+OpenSSL omits underscores for their naming notation whereas WolfSSL expects them. Additionally, 
+OpenSSL does not accept the P curve notation and instead uses the equivalent secp notation. 
+A specific example is that WolfSSL expects P384_ML_KEM_1024, while OpenSSL expects secp384r1MLKEM1024.
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 2fc77d8..4c11cd4 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -560,7 +560,7 @@
 tls_ctx_set_tls_groups(struct tls_root_ctx *ctx, const char *groups)
 {
     ASSERT(ctx);
-#if OPENSSL_VERSION_NUMBER < 0x30000000L
+#if OPENSSL_VERSION_NUMBER < 0x30000000L && !defined(ENABLE_CRYPTO_WOLFSSL)
     struct gc_arena gc = gc_new();
     /* This method could be as easy as
      *  SSL_CTX_set1_groups_list(ctx->ctx, groups)

[Openvpn-devel] [S] Change in openvpn[master]: Multi-socket: Fix assert triggered by stale peer-id reuse

[its_Giaan (C. Review) <ge...@op...>]

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/+/1078?usp=email

to review the following change.


Change subject: Multi-socket: Fix assert triggered by stale peer-id reuse
......................................................................

Multi-socket: Fix assert triggered by stale peer-id reuse

Fixed a bug where clients using different transport
protocols (UDP, TCP) could interfere with each other
after a server restart.
The issue occurred when a client reused a previously
assigned peer-id that was now associated with a
different client using a different transport protocol.

For example, a UDP client could send packets with a
peer-id now assigned to a TCP client, which lacks
a valid context->c2.from which is filled by the
recvfrom(), causing an assert to be triggered.

A protocol check has been added to prevent packets
from different protocols from hijacking active
connections.

Github: #773

Change-Id: Iecbbcf32c0059f2b16a05333b3794599060d7d6a
Signed-off-by: Gianmarco De Gregori <gia...@ma...>
---
M src/openvpn/mudp.c
1 file changed, 17 insertions(+), 9 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/78/1078/1

diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
index 93e65e0..c47ed16 100644
--- a/src/openvpn/mudp.c
+++ b/src/openvpn/mudp.c
@@ -216,16 +216,24 @@
 
             if (!peer_id_disabled && (peer_id < m->max_clients) && (m->instances[peer_id]))
             {
-                mi = m->instances[peer_id];
-
-                *floated = !link_socket_actual_match(&mi->context.c2.from, &m->top.c2.from);
-
-                if (*floated)
+                /* Ensure that clients from previous sessions do not attempt to
+                 * hijack instances of newly connected clients in multi-protocol scenarios */
+                if (m->instances[peer_id]->context.c2.link_sockets[0]->info.proto == sock->info.proto)
                 {
-                    /* reset prefix, since here we are not sure peer is the one it claims to be */
-                    ungenerate_prefix(mi);
-                    msg(D_MULTI_MEDIUM, "Float requested for peer %" PRIu32 " to %s", peer_id,
-                        mroute_addr_print(&real, &gc));
+                    mi = m->instances[peer_id];
+
+                    if (mi)
+                    {
+                        *floated = !link_socket_actual_match(&mi->context.c2.from, &m->top.c2.from);
+
+                        if (*floated)
+                        {
+                            /* reset prefix, since here we are not sure peer is the one it claims to be */
+                            ungenerate_prefix(mi);
+                            msg(D_MULTI_MEDIUM, "Float requested for peer %" PRIu32 " to %s", peer_id,
+                                mroute_addr_print(&real, &gc));
+                        }
+                    }
                 }
             }
         }

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

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Iecbbcf32c0059f2b16a05333b3794599060d7d6a
Gerrit-Change-Number: 1078
Gerrit-PatchSet: 1
Gerrit-Owner: its_Giaan <gia...@ma...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-MessageType: newchange

[Openvpn-devel] [S] Change in openvpn[master]: Added PQE to WolfSSL

[plaisthos (C. Review) <ge...@op...>]

Attention is currently required from: comododragon, cron2, flichtenheld.

plaisthos has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/1046?usp=email )

Change subject: Added PQE to WolfSSL
......................................................................


Patch Set 6: Code-Review+2

(1 comment)

File README.wolfssl:

http://gerrit.openvpn.net/c/openvpn/+/1046/comment/590f267a_2d630a04 :
PS6, Line 59: OpenSSL does not accept the P curve notation and instead uses the equivalent secp notation. 
Still white space errors. Maybe we can fix them on merge.



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

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ie0529c2074964b3be034f01e0ef53090a6edbd35
Gerrit-Change-Number: 1046
Gerrit-PatchSet: 6
Gerrit-Owner: comododragon <rei...@fo...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: cron2 <ge...@gr...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: comododragon <rei...@fo...>
Gerrit-Comment-Date: Mon, 07 Jul 2025 12:06:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Openvpn-devel] [S] Change in openvpn[master]: Added PQE to WolfSSL

[comododragon (C. Review) <ge...@op...>]

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

comododragon has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/1046?usp=email )

Change subject: Added PQE to WolfSSL
......................................................................


Patch Set 6:

(1 comment)

Patchset:

PS5: 
> I tested the patch and it works and is also a lot cleaner than the first version. […]
Let me write this down so I can look at it later. Thanks!



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

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ie0529c2074964b3be034f01e0ef53090a6edbd35
Gerrit-Change-Number: 1046
Gerrit-PatchSet: 6
Gerrit-Owner: comododragon <rei...@fo...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: cron2 <ge...@gr...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Comment-Date: Mon, 07 Jul 2025 11:03:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: plaisthos <arn...@rf...>
Gerrit-MessageType: comment

[Openvpn-devel] [S] Change in openvpn[master]: Added PQE to WolfSSL

[comododragon (C. Review) <ge...@op...>]

Attention is currently required from: comododragon, cron2, flichtenheld, plaisthos.

Hello cron2, flichtenheld, plaisthos, 

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

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

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

The following approvals got outdated and were removed:
Code-Review+1 by cron2, Code-Review+1 by plaisthos


Change subject: Added PQE to WolfSSL
......................................................................

Added PQE to WolfSSL

Change-Id: Ie0529c2074964b3be034f01e0ef53090a6edbd35
---
M README.wolfssl
M src/openvpn/ssl_openssl.c
2 files changed, 31 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/46/1046/6

diff --git a/README.wolfssl b/README.wolfssl
index a5dfe31..7475164 100644
--- a/README.wolfssl
+++ b/README.wolfssl
@@ -28,3 +28,33 @@
  * blowfish support (BF-CBC), you must use something like
    cipher AES-128-CBC to avoid trying to use BF-CBC
  * Windows CryptoAPI support
+
+*************************************************************************
+To build WolfSSL with post-quantum KEMs built in, the following command is used:
+
+./configure --enable-openvpn --enable-kyber=all --enable-curve25519
+
+WolfSSL supports the following post-quantum KEMs and post-quantum hybrid KEMs which must be specified 
+using the tls-groups option in an OpenVPN config. Unlike OpenSSL, which includes X25519MLKEM768 
+in the default config, WolfSSL requires explicit configuration of tls-groups to include 
+at least one post-quantum KEM.
+
+ML_KEM_512
+ML_KEM_768
+ML_KEM_1024
+
+P256_ML_KEM_512
+X25519_ML_KEM_512
+
+P384_ML_KEM_768
+P256_ML_KEM_768
+X448_ML_KEM_768
+X25519_ML_KEM_768
+
+P384_ML_KEM_1024
+P521_ML_KEM_1024
+
+The naming conventions of algorithms differ between WolfSSL and OpenSSL. An example is that
+OpenSSL omits underscores for their naming notation whereas WolfSSL expects them. Additionally, 
+OpenSSL does not accept the P curve notation and instead uses the equivalent secp notation. 
+A specific example is that WolfSSL expects P384_ML_KEM_1024, while OpenSSL expects secp384r1MLKEM1024.
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 2fc77d8..4c11cd4 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -560,7 +560,7 @@
 tls_ctx_set_tls_groups(struct tls_root_ctx *ctx, const char *groups)
 {
     ASSERT(ctx);
-#if OPENSSL_VERSION_NUMBER < 0x30000000L
+#if OPENSSL_VERSION_NUMBER < 0x30000000L && !defined(ENABLE_CRYPTO_WOLFSSL)
     struct gc_arena gc = gc_new();
     /* This method could be as easy as
      *  SSL_CTX_set1_groups_list(ctx->ctx, groups)

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

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ie0529c2074964b3be034f01e0ef53090a6edbd35
Gerrit-Change-Number: 1046
Gerrit-PatchSet: 6
Gerrit-Owner: comododragon <rei...@fo...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: cron2 <ge...@gr...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: comododragon <rei...@fo...>
Gerrit-MessageType: newpatchset

Re: [Openvpn-devel] [SPAM] [PATCH 1/1] Accept "issuer" in `x509-username-field` for the peer certifi

[Klemens N. <kl...@ya...>]

> 8 мая 2025 г., в 14:46, Arne Schwabe <arne rfc2549 ! org> написал(а):
> 
> Am 21.04.25 um 23:44 schrieb Klemens Nanni:
>> This allows for accepting clients based on their certificate authority:
>> 	x509-username-field issuer CN
>> 	verify-x509-name    ...CA=ExampleCA_ match-prefix
>> 
>> `tls-verify` or `plugin` can do the equivalent, but require additional code
>> execution and always incur overhead or may not be an option when running with
>> reduced privileges, e.g. `chroot`
> 
> I am trying to understand the use case for this patch. Issuer is only 
> something you can trust and verify if you verified the fingerprint of 
> the certificate or that the certificate is issued by a given CA. But if 
> it is already verified to belong to a trusted CA, then you don't need 
> issuer CN anymore.

—ca contains the root CA and the intermediate CA issuing client certificates
for use as VPN use.

Under the same root CA, another intermediate CA exists not intended for VPN.

The problem is OpenVPN successfully validates both certificate chains whilst
only one intermediate CA should allow peers to connect.

AFAIU, this is expected OpenSSL behaviour, at least when OpenVPN peers
send not only their own, but also their issuer CA’s certificate via —cert.

Having said that, —remote-cert-ku might be a viable alternative, but that
requires the X509v3 extension and respective key usage bits set up front;
(I have not tried that approach.)

Thus reusing the customisable username mechanism allows for limiting to
certain CAs, i.e. rejecting undesired peers, early during handshake.

> I would also be good to try to add a unit test. Since is is probably a 
> quite exotic use case, this will not be tested regularly and as such is 
> in danger to be become broken and since this an auth related option that 
> might then be an authentication bypass. We really want to avoid that.

If the patch still makes sense to you and seems worth pursuing, I’ll happily
work on tests next.

[Openvpn-devel] [PATCH] Replace deprecated OpenSSL.crypto.load_crl

[Christian S. <sp...@fe...>]

OpenSSL.crypto.load_crl was deprecated with with pyOpenSSL 23.3.0 and
eventually removed in 24.3.0. pyOpenSSL recommends using cryptography.x509's
CRL functions as a replacement.
See also: https://github.com/pyca/pyopenssl/blob/main/CHANGELOG.rst

Signed-off-by: Christian Schürmann <sp...@fe...>
---
 contrib/extract-crl/extractcrl.py | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/contrib/extract-crl/extractcrl.py b/contrib/extract-crl/extractcrl.py
index 441464e..c387ea1 100755
--- a/contrib/extract-crl/extractcrl.py
+++ b/contrib/extract-crl/extractcrl.py
@@ -42,17 +42,17 @@ def measure_time(method):
 def load_crl(filename, format):
 
     def try_openssl_module(filename, format):
-        from OpenSSL import crypto
-        types = {
-            FILETYPE_PEM: crypto.FILETYPE_PEM,
-            FILETYPE_DER: crypto.FILETYPE_ASN1
+        from cryptography import x509
+        load_crl_functions = {
+            FILETYPE_PEM: x509.load_pem_x509_crl,
+            FILETYPE_DER: x509.load_der_x509_crl
         }
         if filename == '-':
-            crl = crypto.load_crl(types[format], sys.stdin.buffer.read())
+            crl = load_crl_functions[format](sys.stdin.buffer.read())
         else:
             with open(filename, 'rb') as f:
-                crl = crypto.load_crl(types[format], f.read())
-        return set(int(r.get_serial(), 16) for r in crl.get_revoked())
+                crl = load_crl_functions[format](f.read())
+        return set(r.serial_number for r in crl)
 
     def try_openssl_exec(filename, format):
         args = ['openssl', 'crl', '-inform', format, '-text']
-- 
2.50.0