Menu
▾
▴
openvpn-devel
|
From: mrbff (C. Review) <ge...@op...> - 2024-11-20 17:43:00
|
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/+/810?usp=email
to review the following change.
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, 216 insertions(+), 0 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/10/810/1
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 5b4419d..49bd242 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -5759,6 +5759,216 @@
err:
msg(msglevel, "Error occurred trying to remove %s option", p[0]);
}
+
+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)
+ {
+ destroy_routes_v4(c->c1.route_list, c->c1.tuntap,
+ ROUTE_OPTION_FLAGS(&c->options),
+ es, &c->net_ctx, options);
+ }
+ *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)
+ {
+ destroy_routes_v6(c->c1.route_ipv6_list, c->c1.tuntap,
+ ROUTE_OPTION_FLAGS(&c->options),
+ es, &c->net_ctx, options);
+ }
+ *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,
@@ -5772,6 +5982,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)))
{
@@ -5797,6 +6008,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: 1
Gerrit-Owner: mrbff <ma...@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
|
|
From: mrbff (C. Review) <ge...@op...> - 2024-12-03 20:40:18
|
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/+/810?usp=email
to look at the new patch set (#8).
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, 236 insertions(+), 0 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/10/810/8
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index b4ee450..d2b9d53 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -5778,6 +5778,236 @@
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)
+ {
+ destroy_routes_v4(c->c1.route_list, c->c1.tuntap,
+ ROUTE_OPTION_FLAGS(&c->options),
+ es, &c->net_ctx, options);
+ }
+ *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)
+ {
+ destroy_routes_v6(c->c1.route_ipv6_list, c->c1.tuntap,
+ ROUTE_OPTION_FLAGS(&c->options),
+ es, &c->net_ctx, options);
+ }
+ *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,
@@ -5791,6 +6021,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)))
{
@@ -5816,6 +6047,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: 8
Gerrit-Owner: mrbff <ma...@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
|
|
From: mrbff (C. Review) <ge...@op...> - 2025-01-21 15:55:31
|
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/+/810?usp=email
to look at the new patch set (#12).
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/12
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 7e38d2c..bedbbb0 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -5563,6 +5563,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.
@@ -5617,11 +5624,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])
@@ -5632,11 +5635,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])
@@ -5755,6 +5754,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,
@@ -5768,6 +5999,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)))
{
@@ -5793,6 +6025,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: 12
Gerrit-Owner: mrbff <ma...@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
|
|
From: stipa (C. Review) <ge...@op...> - 2025-01-22 09:17:38
|
Attention is currently required from: flichtenheld, mrbff, plaisthos. stipa has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/810?usp=email ) Change subject: PUSH_UPDATE: Added update_option() function. ...................................................................... Patch Set 12: Code-Review+2 (1 comment) Patchset: PS12: I tested those 3 patches together - looks good. I was able to replace routes, remove routes, change IP address and revert it back. It would me nice to make it work also for dco-win (which is a default driver) - at the moment we don't support reopening dco device - but this could be done later. -- 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: 12 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-Comment-Date: Wed, 22 Jan 2025 09:17:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment |
|
From: mrbff (C. Review) <ge...@op...> - 2025-03-21 04:58:04
|
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...@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/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...@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
|
|
From: mrbff (C. Review) <ge...@op...> - 2025-05-20 12:26:17
|
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 (#14).
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/14
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 33e1a61..ffefd27 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -5516,6 +5516,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.
@@ -5570,11 +5577,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])
@@ -5585,11 +5588,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])
@@ -5708,6 +5707,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,
@@ -5721,6 +5952,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)))
{
@@ -5746,6 +5978,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: 14
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
|
|
From: stipa (C. Review) <ge...@op...> - 2025-06-16 06:18:58
|
Attention is currently required from: flichtenheld, mrbff, plaisthos. stipa has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/810?usp=email ) Change subject: PUSH_UPDATE: Added update_option() function. ...................................................................... Patch Set 14: Code-Review+2 (1 comment) Patchset: PS14: JFTR: We have tested openvpn-gui 2.7alpha1 with push-update patches on top against corporate backend (CloudConnexa) which supports push-update server functionality. Looks good! Adding/removing routes and various DNS options works. -- 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: 14 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-Comment-Date: Mon, 16 Jun 2025 06:18:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment |
|
From: mrbff (C. Review) <ge...@op...> - 2025-07-07 17:16:14
|
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
|
|
From: mrbff (C. Review) <ge...@op...> - 2025-07-16 10:36:33
|
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 (#19).
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/19
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 22eaa8a..713df52 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -5642,6 +5642,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.
@@ -5696,11 +5703,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])
@@ -5711,11 +5714,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])
@@ -5834,6 +5833,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,
@@ -5847,6 +6078,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)))
{
@@ -5872,6 +6104,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: 19
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
|
|
From: mrbff (C. Review) <ge...@op...> - 2025-07-17 05:25:49
|
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 (#20).
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/20
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 22eaa8a..713df52 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -5642,6 +5642,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.
@@ -5696,11 +5703,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])
@@ -5711,11 +5714,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])
@@ -5834,6 +5833,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,
@@ -5847,6 +6078,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)))
{
@@ -5872,6 +6104,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: 20
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
|
|
From: cron2 (C. Review) <ge...@op...> - 2025-07-21 19:21:30
|
Attention is currently required from: flichtenheld, mrbff, plaisthos. cron2 has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/810?usp=email ) Change subject: PUSH_UPDATE: Added update_option() function. ...................................................................... Patch Set 21: Code-Review-1 (5 comments) Patchset: PS21: Nagging about code duplication... File src/openvpn/options.c: http://gerrit.openvpn.net/c/openvpn/+/810/comment/e80b547e_689dbd50 : PS21, Line 5897: } I find this duplication of syntax checking, mmmh, sub-optimal. We'll check the syntax anyway in `add_option()` later on... I could see user complaints about "I sent a PUSH_UPDATE with one invalid `route` in it, and all my routes were lost", but honestly, do we really care that much? (If yes, we could consider moving the syntax check into a small helper, if not, just drop the lines from `rol_check_alloc()` up to this line). http://gerrit.openvpn.net/c/openvpn/+/810/comment/e1f5b5bd_f34b1d7f : PS21, Line 5927: } same here http://gerrit.openvpn.net/c/openvpn/+/810/comment/4054ed13_3dac7037 : PS21, Line 6009: same here... lots of code that needs to be in sync with the add_option() checks... http://gerrit.openvpn.net/c/openvpn/+/810/comment/abc555fa_e0e5856c : PS21, Line 6045: #endif I note a distinct lack of syntax checking for `dhcp-option` ;-) -- 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: 21 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: flichtenheld <fr...@li...> Gerrit-Attention: mrbff <ma...@ma...> Gerrit-Comment-Date: Mon, 21 Jul 2025 19:21:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment |
|
From: mrbff (C. Review) <ge...@op...> - 2025-07-23 17:10:35
|
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/+/810?usp=email
to look at the new patch set (#22).
The following approvals got outdated and were removed:
Code-Review+2 by stipa, Code-Review-1 by cron2
The change is no longer submittable: Code-Review and checks~ChecksSubmitRule are 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, 331 insertions(+), 124 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/10/810/22
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 683dae6..d5f8826 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -5372,6 +5372,20 @@
struct env_set *es);
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);
+
+static void
read_config_file(struct options *options,
const char *file,
int level,
@@ -5557,6 +5571,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)))
{
@@ -5598,6 +5613,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;
@@ -5736,6 +5756,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.
@@ -5790,11 +5817,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])
@@ -5805,11 +5828,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])
@@ -5929,6 +5948,303 @@
msg(msglevel, "Error occurred trying to remove %s option", p[0]);
}
+
+static bool
+check_route_option(struct options *options, char *p[], const int msglevel, bool pull_mode)
+{
+ 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]);
+ return false;
+ }
+ 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]);
+ return false;
+ }
+ 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]);
+ return false;
+ }
+ }
+ return true;
+}
+
+
+static bool
+check_route6_option(struct options *options, char *p[], const int msglevel, bool pull_mode)
+{
+ 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]);
+ return false;
+ }
+ if (p[2] && !ipv6_addr_safe(p[2]))
+ {
+ msg(msglevel, "route-ipv6 parameter gateway '%s' must be a valid address", p[2]);
+ return false;
+ }
+ /* p[3] is metric, if present */
+ }
+ return true;
+}
+
+static bool
+check_dns_option(struct options *options, char *p[], const int msglevel, bool pull_mode)
+{
+ if (streq(p[1], "search-domains") && p[2])
+ {
+ dns_domain_list_append(&options->dns_options.search_domains, &p[2], &options->dns_options.gc);
+ }
+ else 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]);
+ return false;
+ }
+
+ struct dns_server *server = dns_server_get(&options->dns_options.servers, priority, &options->dns_options.gc);
+
+ 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]);
+ return false;
+ }
+ }
+ }
+ else if (streq(p[3], "resolve-domains"))
+ {
+ dns_domain_list_append(&server->domains, &p[4], &options->dns_options.gc);
+ }
+ else if (streq(p[3], "dnssec") && !p[5])
+ {
+ if (streq(p[4], "yes"))
+ {
+ server->dnssec = DNS_SECURITY_YES;
+ }
+ else if (streq(p[4], "no"))
+ {
+ server->dnssec = DNS_SECURITY_NO;
+ }
+ else if (streq(p[4], "optional"))
+ {
+ server->dnssec = DNS_SECURITY_OPTIONAL;
+ }
+ else
+ {
+ msg(msglevel, "--dns server %ld: malformed dnssec value '%s'", priority, p[4]);
+ return false;
+ }
+ }
+ else if (streq(p[3], "transport") && !p[5])
+ {
+ if (streq(p[4], "plain"))
+ {
+ server->transport = DNS_TRANSPORT_PLAIN;
+ }
+ else if (streq(p[4], "DoH"))
+ {
+ server->transport = DNS_TRANSPORT_HTTPS;
+ }
+ else if (streq(p[4], "DoT"))
+ {
+ server->transport = DNS_TRANSPORT_TLS;
+ }
+ else
+ {
+ msg(msglevel, "--dns server %ld: malformed transport value '%s'", priority, p[4]);
+ return false;
+ }
+ }
+ else if (streq(p[3], "sni") && !p[5])
+ {
+ server->sni = p[4];
+ }
+ else
+ {
+ msg(msglevel, "--dns server %ld: unknown option type '%s' or missing or unknown parameter", priority, p[3]);
+ return false;
+ }
+ }
+ else
+ {
+ msg(msglevel, "--dns: unknown option type '%s' or missing or unknown parameter", p[1]);
+ return false;
+ }
+ return true;
+}
+
+/**
+ * @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);
+ if (!check_route_option(options, p, msglevel, pull_mode))
+ {
+ 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);
+ if (!check_route6_option(options, p, msglevel, pull_mode))
+ {
+ goto err;
+ }
+ 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 (!check_dns_option(options, p, msglevel, pull_mode))
+ {
+ 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]);
+}
+
static void
set_user_script(struct options *options,
const char **script,
@@ -7292,45 +7608,18 @@
else if (streq(p[0], "route") && p[1] && !p[5])
{
VERIFY_PERMISSION(OPT_P_ROUTE);
- rol_check_alloc(options);
- if (pull_mode)
+ if (!check_route_option(options, p, msglevel, 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;
- }
- /* p[4] is metric, if specified */
+ goto err;
}
add_route_to_option_list(options->routes, p[1], p[2], p[3], p[4], options->route_default_table_id);
}
else if (streq(p[0], "route-ipv6") && p[1] && !p[4])
{
VERIFY_PERMISSION(OPT_P_ROUTE);
- rol6_check_alloc(options);
- if (pull_mode)
+ if (!check_route6_option(options, p, msglevel, 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 specified */
+ goto err;
}
add_route_ipv6_to_option_list(options->routes_ipv6, p[1], p[2], p[3], options->route_default_table_id);
}
@@ -8418,90 +8707,8 @@
else if (streq(p[0], "dns") && p[1])
{
VERIFY_PERMISSION(OPT_P_DHCPDNS);
-
- if (streq(p[1], "search-domains") && p[2])
+ if (!check_dns_option(options, p, msglevel, pull_mode))
{
- dns_domain_list_append(&options->dns_options.search_domains, &p[2], &options->dns_options.gc);
- }
- else 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 = dns_server_get(&options->dns_options.servers, priority, &options->dns_options.gc);
-
- 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], "resolve-domains"))
- {
- dns_domain_list_append(&server->domains, &p[4], &options->dns_options.gc);
- }
- else if (streq(p[3], "dnssec") && !p[5])
- {
- if (streq(p[4], "yes"))
- {
- server->dnssec = DNS_SECURITY_YES;
- }
- else if (streq(p[4], "no"))
- {
- server->dnssec = DNS_SECURITY_NO;
- }
- else if (streq(p[4], "optional"))
- {
- server->dnssec = DNS_SECURITY_OPTIONAL;
- }
- else
- {
- 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"))
- {
- server->transport = DNS_TRANSPORT_PLAIN;
- }
- else if (streq(p[4], "DoH"))
- {
- server->transport = DNS_TRANSPORT_HTTPS;
- }
- else if (streq(p[4], "DoT"))
- {
- server->transport = DNS_TRANSPORT_TLS;
- }
- else
- {
- msg(msglevel, "--dns server %ld: malformed transport value '%s'", priority, p[4]);
- goto err;
- }
- }
- else if (streq(p[3], "sni") && !p[5])
- {
- server->sni = p[4];
- }
- else
- {
- msg(msglevel, "--dns server %ld: unknown option type '%s' or missing or unknown parameter", priority, p[3]);
- goto err;
- }
- }
- else
- {
- msg(msglevel, "--dns: unknown option type '%s' or missing or unknown parameter", p[1]);
goto err;
}
}
--
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: 22
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
|
|
From: mrbff (C. Review) <ge...@op...> - 2025-07-23 17:12:12
|
Attention is currently required from: cron2, flichtenheld, plaisthos, stipa. mrbff has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/810?usp=email ) Change subject: PUSH_UPDATE: Added update_option() function. ...................................................................... Patch Set 22: (3 comments) File src/openvpn/options.c: http://gerrit.openvpn.net/c/openvpn/+/810/comment/edf64e35_8ed4afac : PS21, Line 5897: } > I find this duplication of syntax checking, mmmh, sub-optimal. […] idk, i thought it were nice to make some checks before deleting the options. :) http://gerrit.openvpn.net/c/openvpn/+/810/comment/6c0edd1d_f8769a03 : PS21, Line 5927: } > same here Done http://gerrit.openvpn.net/c/openvpn/+/810/comment/af06b38c_73d23b39 : PS21, Line 6009: > same here... lots of code that needs to be in sync with the add_option() checks... Done -- 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: 22 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-Comment-Date: Wed, 23 Jul 2025 17:12:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: cron2 <ge...@gr...> Gerrit-MessageType: comment |
|
From: cron2 (C. Review) <ge...@op...> - 2025-07-27 12:22:32
|
Attention is currently required from: flichtenheld, mrbff, plaisthos, stipa. cron2 has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/810?usp=email ) Change subject: PUSH_UPDATE: Added update_option() function. ...................................................................... Patch Set 22: Code-Review+2 -- 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: 22 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: flichtenheld <fr...@li...> Gerrit-Attention: mrbff <ma...@ma...> Gerrit-Attention: stipa <lst...@gm...> Gerrit-Comment-Date: Sun, 27 Jul 2025 12:22:09 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment |
|
From: Gert D. <ge...@gr...> - 2025-07-29 10:41:23
|
From: Marco Baffo <ma...@ma...> 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...> 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/+/810 This mail reflects revision 23 of this Change. Acked-by according to Gerrit (reflected above): Gert Doering <ge...@gr...> diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 3a8ce86..f056263 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -5372,6 +5372,20 @@ struct env_set *es); 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); + +static void read_config_file(struct options *options, const char *file, int level, @@ -5557,6 +5571,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))) { @@ -5602,6 +5617,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; @@ -5740,6 +5760,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. @@ -5794,11 +5821,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]) @@ -5809,11 +5832,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]) @@ -5933,6 +5952,303 @@ msg(msglevel, "Error occurred trying to remove %s option", p[0]); } + +static bool +check_route_option(struct options *options, char *p[], const int msglevel, bool pull_mode) +{ + 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]); + return false; + } + 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]); + return false; + } + 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]); + return false; + } + } + return true; +} + + +static bool +check_route6_option(struct options *options, char *p[], const int msglevel, bool pull_mode) +{ + 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]); + return false; + } + if (p[2] && !ipv6_addr_safe(p[2])) + { + msg(msglevel, "route-ipv6 parameter gateway '%s' must be a valid address", p[2]); + return false; + } + /* p[3] is metric, if present */ + } + return true; +} + +static bool +check_dns_option(struct options *options, char *p[], const int msglevel, bool pull_mode) +{ + if (streq(p[1], "search-domains") && p[2]) + { + dns_domain_list_append(&options->dns_options.search_domains, &p[2], &options->dns_options.gc); + } + else 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]); + return false; + } + + struct dns_server *server = dns_server_get(&options->dns_options.servers, priority, &options->dns_options.gc); + + 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]); + return false; + } + } + } + else if (streq(p[3], "resolve-domains")) + { + dns_domain_list_append(&server->domains, &p[4], &options->dns_options.gc); + } + else if (streq(p[3], "dnssec") && !p[5]) + { + if (streq(p[4], "yes")) + { + server->dnssec = DNS_SECURITY_YES; + } + else if (streq(p[4], "no")) + { + server->dnssec = DNS_SECURITY_NO; + } + else if (streq(p[4], "optional")) + { + server->dnssec = DNS_SECURITY_OPTIONAL; + } + else + { + msg(msglevel, "--dns server %ld: malformed dnssec value '%s'", priority, p[4]); + return false; + } + } + else if (streq(p[3], "transport") && !p[5]) + { + if (streq(p[4], "plain")) + { + server->transport = DNS_TRANSPORT_PLAIN; + } + else if (streq(p[4], "DoH")) + { + server->transport = DNS_TRANSPORT_HTTPS; + } + else if (streq(p[4], "DoT")) + { + server->transport = DNS_TRANSPORT_TLS; + } + else + { + msg(msglevel, "--dns server %ld: malformed transport value '%s'", priority, p[4]); + return false; + } + } + else if (streq(p[3], "sni") && !p[5]) + { + server->sni = p[4]; + } + else + { + msg(msglevel, "--dns server %ld: unknown option type '%s' or missing or unknown parameter", priority, p[3]); + return false; + } + } + else + { + msg(msglevel, "--dns: unknown option type '%s' or missing or unknown parameter", p[1]); + return false; + } + return true; +} + +/** + * @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); + if (!check_route_option(options, p, msglevel, pull_mode)) + { + 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); + if (!check_route6_option(options, p, msglevel, pull_mode)) + { + goto err; + } + 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 (!check_dns_option(options, p, msglevel, pull_mode)) + { + 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]); +} + static void set_user_script(struct options *options, const char **script, @@ -7296,45 +7612,18 @@ else if (streq(p[0], "route") && p[1] && !p[5]) { VERIFY_PERMISSION(OPT_P_ROUTE); - rol_check_alloc(options); - if (pull_mode) + if (!check_route_option(options, p, msglevel, 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; - } - /* p[4] is metric, if specified */ + goto err; } add_route_to_option_list(options->routes, p[1], p[2], p[3], p[4], options->route_default_table_id); } else if (streq(p[0], "route-ipv6") && p[1] && !p[4]) { VERIFY_PERMISSION(OPT_P_ROUTE); - rol6_check_alloc(options); - if (pull_mode) + if (!check_route6_option(options, p, msglevel, 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 specified */ + goto err; } add_route_ipv6_to_option_list(options->routes_ipv6, p[1], p[2], p[3], options->route_default_table_id); } @@ -8422,90 +8711,8 @@ else if (streq(p[0], "dns") && p[1]) { VERIFY_PERMISSION(OPT_P_DHCPDNS); - - if (streq(p[1], "search-domains") && p[2]) + if (!check_dns_option(options, p, msglevel, pull_mode)) { - dns_domain_list_append(&options->dns_options.search_domains, &p[2], &options->dns_options.gc); - } - else 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 = dns_server_get(&options->dns_options.servers, priority, &options->dns_options.gc); - - 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], "resolve-domains")) - { - dns_domain_list_append(&server->domains, &p[4], &options->dns_options.gc); - } - else if (streq(p[3], "dnssec") && !p[5]) - { - if (streq(p[4], "yes")) - { - server->dnssec = DNS_SECURITY_YES; - } - else if (streq(p[4], "no")) - { - server->dnssec = DNS_SECURITY_NO; - } - else if (streq(p[4], "optional")) - { - server->dnssec = DNS_SECURITY_OPTIONAL; - } - else - { - 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")) - { - server->transport = DNS_TRANSPORT_PLAIN; - } - else if (streq(p[4], "DoH")) - { - server->transport = DNS_TRANSPORT_HTTPS; - } - else if (streq(p[4], "DoT")) - { - server->transport = DNS_TRANSPORT_TLS; - } - else - { - msg(msglevel, "--dns server %ld: malformed transport value '%s'", priority, p[4]); - goto err; - } - } - else if (streq(p[3], "sni") && !p[5]) - { - server->sni = p[4]; - } - else - { - msg(msglevel, "--dns server %ld: unknown option type '%s' or missing or unknown parameter", priority, p[3]); - goto err; - } - } - else - { - msg(msglevel, "--dns: unknown option type '%s' or missing or unknown parameter", p[1]); goto err; } } |
|
From: Gert D. <ge...@gr...> - 2025-07-29 14:53:17
|
.. and the last one in the series, with similar text leading to it...
(so just look at 808+809).
Tested on the t_client/t_server framework, which exercises many of those
options where the handling even in the "normal path" is now changed (due
to moving the syntax check into smaller helper functions).
I'm still not convinced that we really need to do all these syntax checks
for "remove an option", but that message was not really heard... also, I'm
not really sure the current behaviour is that useful outside fairly limited
OpenVPN Corp scenarios ("why would a PUSH_UPDATE route statement remove
all routes I have in my config file, not only those sent by PUSH_REPLY?"),
but we all agreed that this is the beginning of a journey, also expecting
incremental improvements ("not reopen the tun device if there is no reason
for it", for example, because the server just pushes extra routes).
Your patch has been applied to the master branch.
commit dd9c6978ec851213deece9229e413d44d0289cad
Author: Marco Baffo
Date: Tue Jul 29 12:41:01 2025 +0200
PUSH_UPDATE: Added update_option() function.
Signed-off-by: Marco Baffo <ma...@ma...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg32408.html
Signed-off-by: Gert Doering <ge...@gr...>
--
kind regards,
Gert Doering
|
|
From: cron2 (C. Review) <ge...@op...> - 2025-07-29 14:53:07
|
cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/810?usp=email ) 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 for that option first. Change-Id: Ia45c99e6df7b3ad24020c10b8a9b3577984ecdc2 Signed-off-by: Marco Baffo <ma...@ma...> Acked-by: Gert Doering <ge...@gr...> Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg32408.html Signed-off-by: Gert Doering <ge...@gr...> --- M src/openvpn/options.c 1 file changed, 331 insertions(+), 124 deletions(-) diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 469360d..b7328d7 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -5360,6 +5360,20 @@ struct env_set *es); 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); + +static void read_config_file(struct options *options, const char *file, int level, @@ -5545,6 +5559,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))) { @@ -5590,6 +5605,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; @@ -5728,6 +5748,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. @@ -5782,11 +5809,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]) @@ -5797,11 +5820,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]) @@ -5921,6 +5940,303 @@ msg(msglevel, "Error occurred trying to remove %s option", p[0]); } + +static bool +check_route_option(struct options *options, char *p[], const int msglevel, bool pull_mode) +{ + 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]); + return false; + } + 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]); + return false; + } + 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]); + return false; + } + } + return true; +} + + +static bool +check_route6_option(struct options *options, char *p[], const int msglevel, bool pull_mode) +{ + 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]); + return false; + } + if (p[2] && !ipv6_addr_safe(p[2])) + { + msg(msglevel, "route-ipv6 parameter gateway '%s' must be a valid address", p[2]); + return false; + } + /* p[3] is metric, if present */ + } + return true; +} + +static bool +check_dns_option(struct options *options, char *p[], const int msglevel, bool pull_mode) +{ + if (streq(p[1], "search-domains") && p[2]) + { + dns_domain_list_append(&options->dns_options.search_domains, &p[2], &options->dns_options.gc); + } + else 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]); + return false; + } + + struct dns_server *server = dns_server_get(&options->dns_options.servers, priority, &options->dns_options.gc); + + 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]); + return false; + } + } + } + else if (streq(p[3], "resolve-domains")) + { + dns_domain_list_append(&server->domains, &p[4], &options->dns_options.gc); + } + else if (streq(p[3], "dnssec") && !p[5]) + { + if (streq(p[4], "yes")) + { + server->dnssec = DNS_SECURITY_YES; + } + else if (streq(p[4], "no")) + { + server->dnssec = DNS_SECURITY_NO; + } + else if (streq(p[4], "optional")) + { + server->dnssec = DNS_SECURITY_OPTIONAL; + } + else + { + msg(msglevel, "--dns server %ld: malformed dnssec value '%s'", priority, p[4]); + return false; + } + } + else if (streq(p[3], "transport") && !p[5]) + { + if (streq(p[4], "plain")) + { + server->transport = DNS_TRANSPORT_PLAIN; + } + else if (streq(p[4], "DoH")) + { + server->transport = DNS_TRANSPORT_HTTPS; + } + else if (streq(p[4], "DoT")) + { + server->transport = DNS_TRANSPORT_TLS; + } + else + { + msg(msglevel, "--dns server %ld: malformed transport value '%s'", priority, p[4]); + return false; + } + } + else if (streq(p[3], "sni") && !p[5]) + { + server->sni = p[4]; + } + else + { + msg(msglevel, "--dns server %ld: unknown option type '%s' or missing or unknown parameter", priority, p[3]); + return false; + } + } + else + { + msg(msglevel, "--dns: unknown option type '%s' or missing or unknown parameter", p[1]); + return false; + } + return true; +} + +/** + * @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); + if (!check_route_option(options, p, msglevel, pull_mode)) + { + 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); + if (!check_route6_option(options, p, msglevel, pull_mode)) + { + goto err; + } + 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 (!check_dns_option(options, p, msglevel, pull_mode)) + { + 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]); +} + static void set_user_script(struct options *options, const char **script, @@ -7284,45 +7600,18 @@ else if (streq(p[0], "route") && p[1] && !p[5]) { VERIFY_PERMISSION(OPT_P_ROUTE); - rol_check_alloc(options); - if (pull_mode) + if (!check_route_option(options, p, msglevel, 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; - } - /* p[4] is metric, if specified */ + goto err; } add_route_to_option_list(options->routes, p[1], p[2], p[3], p[4], options->route_default_table_id); } else if (streq(p[0], "route-ipv6") && p[1] && !p[4]) { VERIFY_PERMISSION(OPT_P_ROUTE); - rol6_check_alloc(options); - if (pull_mode) + if (!check_route6_option(options, p, msglevel, 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 specified */ + goto err; } add_route_ipv6_to_option_list(options->routes_ipv6, p[1], p[2], p[3], options->route_default_table_id); } @@ -8410,90 +8699,8 @@ else if (streq(p[0], "dns") && p[1]) { VERIFY_PERMISSION(OPT_P_DHCPDNS); - - if (streq(p[1], "search-domains") && p[2]) + if (!check_dns_option(options, p, msglevel, pull_mode)) { - dns_domain_list_append(&options->dns_options.search_domains, &p[2], &options->dns_options.gc); - } - else 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 = dns_server_get(&options->dns_options.servers, priority, &options->dns_options.gc); - - 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], "resolve-domains")) - { - dns_domain_list_append(&server->domains, &p[4], &options->dns_options.gc); - } - else if (streq(p[3], "dnssec") && !p[5]) - { - if (streq(p[4], "yes")) - { - server->dnssec = DNS_SECURITY_YES; - } - else if (streq(p[4], "no")) - { - server->dnssec = DNS_SECURITY_NO; - } - else if (streq(p[4], "optional")) - { - server->dnssec = DNS_SECURITY_OPTIONAL; - } - else - { - 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")) - { - server->transport = DNS_TRANSPORT_PLAIN; - } - else if (streq(p[4], "DoH")) - { - server->transport = DNS_TRANSPORT_HTTPS; - } - else if (streq(p[4], "DoT")) - { - server->transport = DNS_TRANSPORT_TLS; - } - else - { - msg(msglevel, "--dns server %ld: malformed transport value '%s'", priority, p[4]); - goto err; - } - } - else if (streq(p[3], "sni") && !p[5]) - { - server->sni = p[4]; - } - else - { - msg(msglevel, "--dns server %ld: unknown option type '%s' or missing or unknown parameter", priority, p[3]); - goto err; - } - } - else - { - msg(msglevel, "--dns: unknown option type '%s' or missing or unknown parameter", p[1]); goto err; } } -- 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: 24 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-MessageType: merged |
|
From: cron2 (C. Review) <ge...@op...> - 2025-07-29 14:53:10
|
cron2 has uploaded a new patch set (#24) to the change originally created by mrbff. ( http://gerrit.openvpn.net/c/openvpn/+/810?usp=email ) The following approvals got outdated and were removed: Code-Review+2 by cron2 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 for that option first. Change-Id: Ia45c99e6df7b3ad24020c10b8a9b3577984ecdc2 Signed-off-by: Marco Baffo <ma...@ma...> Acked-by: Gert Doering <ge...@gr...> Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg32408.html Signed-off-by: Gert Doering <ge...@gr...> --- M src/openvpn/options.c 1 file changed, 331 insertions(+), 124 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/10/810/24 diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 469360d..b7328d7 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -5360,6 +5360,20 @@ struct env_set *es); 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); + +static void read_config_file(struct options *options, const char *file, int level, @@ -5545,6 +5559,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))) { @@ -5590,6 +5605,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; @@ -5728,6 +5748,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. @@ -5782,11 +5809,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]) @@ -5797,11 +5820,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]) @@ -5921,6 +5940,303 @@ msg(msglevel, "Error occurred trying to remove %s option", p[0]); } + +static bool +check_route_option(struct options *options, char *p[], const int msglevel, bool pull_mode) +{ + 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]); + return false; + } + 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]); + return false; + } + 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]); + return false; + } + } + return true; +} + + +static bool +check_route6_option(struct options *options, char *p[], const int msglevel, bool pull_mode) +{ + 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]); + return false; + } + if (p[2] && !ipv6_addr_safe(p[2])) + { + msg(msglevel, "route-ipv6 parameter gateway '%s' must be a valid address", p[2]); + return false; + } + /* p[3] is metric, if present */ + } + return true; +} + +static bool +check_dns_option(struct options *options, char *p[], const int msglevel, bool pull_mode) +{ + if (streq(p[1], "search-domains") && p[2]) + { + dns_domain_list_append(&options->dns_options.search_domains, &p[2], &options->dns_options.gc); + } + else 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]); + return false; + } + + struct dns_server *server = dns_server_get(&options->dns_options.servers, priority, &options->dns_options.gc); + + 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]); + return false; + } + } + } + else if (streq(p[3], "resolve-domains")) + { + dns_domain_list_append(&server->domains, &p[4], &options->dns_options.gc); + } + else if (streq(p[3], "dnssec") && !p[5]) + { + if (streq(p[4], "yes")) + { + server->dnssec = DNS_SECURITY_YES; + } + else if (streq(p[4], "no")) + { + server->dnssec = DNS_SECURITY_NO; + } + else if (streq(p[4], "optional")) + { + server->dnssec = DNS_SECURITY_OPTIONAL; + } + else + { + msg(msglevel, "--dns server %ld: malformed dnssec value '%s'", priority, p[4]); + return false; + } + } + else if (streq(p[3], "transport") && !p[5]) + { + if (streq(p[4], "plain")) + { + server->transport = DNS_TRANSPORT_PLAIN; + } + else if (streq(p[4], "DoH")) + { + server->transport = DNS_TRANSPORT_HTTPS; + } + else if (streq(p[4], "DoT")) + { + server->transport = DNS_TRANSPORT_TLS; + } + else + { + msg(msglevel, "--dns server %ld: malformed transport value '%s'", priority, p[4]); + return false; + } + } + else if (streq(p[3], "sni") && !p[5]) + { + server->sni = p[4]; + } + else + { + msg(msglevel, "--dns server %ld: unknown option type '%s' or missing or unknown parameter", priority, p[3]); + return false; + } + } + else + { + msg(msglevel, "--dns: unknown option type '%s' or missing or unknown parameter", p[1]); + return false; + } + return true; +} + +/** + * @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); + if (!check_route_option(options, p, msglevel, pull_mode)) + { + 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); + if (!check_route6_option(options, p, msglevel, pull_mode)) + { + goto err; + } + 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 (!check_dns_option(options, p, msglevel, pull_mode)) + { + 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]); +} + static void set_user_script(struct options *options, const char **script, @@ -7284,45 +7600,18 @@ else if (streq(p[0], "route") && p[1] && !p[5]) { VERIFY_PERMISSION(OPT_P_ROUTE); - rol_check_alloc(options); - if (pull_mode) + if (!check_route_option(options, p, msglevel, 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; - } - /* p[4] is metric, if specified */ + goto err; } add_route_to_option_list(options->routes, p[1], p[2], p[3], p[4], options->route_default_table_id); } else if (streq(p[0], "route-ipv6") && p[1] && !p[4]) { VERIFY_PERMISSION(OPT_P_ROUTE); - rol6_check_alloc(options); - if (pull_mode) + if (!check_route6_option(options, p, msglevel, 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 specified */ + goto err; } add_route_ipv6_to_option_list(options->routes_ipv6, p[1], p[2], p[3], options->route_default_table_id); } @@ -8410,90 +8699,8 @@ else if (streq(p[0], "dns") && p[1]) { VERIFY_PERMISSION(OPT_P_DHCPDNS); - - if (streq(p[1], "search-domains") && p[2]) + if (!check_dns_option(options, p, msglevel, pull_mode)) { - dns_domain_list_append(&options->dns_options.search_domains, &p[2], &options->dns_options.gc); - } - else 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 = dns_server_get(&options->dns_options.servers, priority, &options->dns_options.gc); - - 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], "resolve-domains")) - { - dns_domain_list_append(&server->domains, &p[4], &options->dns_options.gc); - } - else if (streq(p[3], "dnssec") && !p[5]) - { - if (streq(p[4], "yes")) - { - server->dnssec = DNS_SECURITY_YES; - } - else if (streq(p[4], "no")) - { - server->dnssec = DNS_SECURITY_NO; - } - else if (streq(p[4], "optional")) - { - server->dnssec = DNS_SECURITY_OPTIONAL; - } - else - { - 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")) - { - server->transport = DNS_TRANSPORT_PLAIN; - } - else if (streq(p[4], "DoH")) - { - server->transport = DNS_TRANSPORT_HTTPS; - } - else if (streq(p[4], "DoT")) - { - server->transport = DNS_TRANSPORT_TLS; - } - else - { - msg(msglevel, "--dns server %ld: malformed transport value '%s'", priority, p[4]); - goto err; - } - } - else if (streq(p[3], "sni") && !p[5]) - { - server->sni = p[4]; - } - else - { - msg(msglevel, "--dns server %ld: unknown option type '%s' or missing or unknown parameter", priority, p[3]); - goto err; - } - } - else - { - msg(msglevel, "--dns: unknown option type '%s' or missing or unknown parameter", p[1]); goto err; } } -- 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: 24 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-MessageType: newpatchset |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X