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 (#13).

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...@mandelbit.com>
---
M src/openvpn/options.c
1 file changed, 247 insertions(+), 10 deletions(-)


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

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index b35f82b..4626747 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -5560,6 +5560,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.
@@ -5614,11 +5621,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])
@@ -5629,11 +5632,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])
@@ -5752,6 +5751,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,
@@ -5765,6 +5996,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)))
     {
@@ -5790,6 +6022,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: 13
Gerrit-Owner: mrbff <ma...@mandelbit.com>
Gerrit-Reviewer: flichtenheld <fr...@lichtenheld.com>
Gerrit-Reviewer: plaisthos <arne-open...@rfc2549.org>
Gerrit-Reviewer: stipa <lstipa...@gmail.com>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: plaisthos <arne-open...@rfc2549.org>
Gerrit-Attention: flichtenheld <fr...@lichtenheld.com>
Gerrit-Attention: mrbff <ma...@mandelbit.com>
Gerrit-MessageType: newpatchset
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to