Menu
▾
▴
openvpn-devel
|
From: d12fk (C. Review) <ge...@op...> - 2025-03-06 22:15:37
|
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/+/904?usp=email
to review the following change.
Change subject: dns: clean up --dhcp-options when --dns is active
......................................................................
dns: clean up --dhcp-options when --dns is active
Since --dns setting overrule DNS related --dhcp-options,
remove the latter when we got some via --dns.
Change-Id: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Signed-off-by: Heiko Hund <he...@is...>
---
M src/openvpn/options.c
1 file changed, 47 insertions(+), 0 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/04/904/1
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index d7f6b16..c42d933 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -4160,6 +4160,53 @@
if (success)
{
dns_options_postprocess_pull(&o->dns_options);
+
+#if defined(_WIN32) || defined(TARGET_ANDROID)
+ /* If there's --dns servers, remove dns related --dhcp-options */
+ if (o->dns_options.servers)
+ {
+ o->tuntap_options.dns_len = 0;
+ o->tuntap_options.dns6_len = 0;
+ o->tuntap_options.domain = NULL;
+ o->tuntap_options.domain_search_list_len = 0;
+ }
+#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+ /* Clean up env from overridden DNS config */
+ struct gc_arena gc = gc_new();
+ struct buffer name = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+ struct buffer value = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+
+ const int fo_count = o->foreign_option_index;
+ o->foreign_option_index = 0;
+
+ for (int i = 1; i <= fo_count; ++i)
+ {
+ buf_clear(&name);
+ buf_printf(&name, "foreign_option_%d", i);
+ const char *env_str = env_set_get(es, BSTR(&name));
+ const char *item_val = strchr(env_str, '=') + 1;
+ buf_clear(&value);
+ buf_printf(&value, "%s", item_val);
+
+ /* Remove foreign option item from env set */
+ env_set_del(es, BSTR(&name));
+
+ item_val = BSTR(&value);
+ if (strncmp(item_val, "dhcp-option ", 12) != 0
+ || (strncmp(item_val + 12, "ADAPTER-DOMAIN-SUFFIX ", 22) != 0
+ && strncmp(item_val + 12, "DOMAIN-SEARCH ", 14) != 0
+ && strncmp(item_val + 12, "DOMAIN ", 7) != 0
+ && strncmp(item_val + 12, "DNS6 ", 5) != 0
+ && strncmp(item_val + 12, "DNS ", 4) != 0))
+ {
+ /* Re-set the item with potentially updated name */
+ buf_clear(&name);
+ buf_printf(&name, "foreign_option_%d", ++o->foreign_option_index);
+ setenv_str(es, BSTR(&name), BSTR(&value));
+ }
+ }
+ gc_free(&gc);
+#endif /* defined(_WIN32) || defined(TARGET_ANDROID) */
}
return success;
}
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/904?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: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Gerrit-Change-Number: 904
Gerrit-PatchSet: 1
Gerrit-Owner: d12fk <he...@op...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-MessageType: newchange
|
|
From: flichtenheld (C. Review) <ge...@op...> - 2025-03-07 11:03:47
|
Attention is currently required from: d12fk, plaisthos. flichtenheld has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/904?usp=email ) Change subject: dns: clean up --dhcp-options when --dns is active ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/904?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: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a Gerrit-Change-Number: 904 Gerrit-PatchSet: 2 Gerrit-Owner: d12fk <he...@op...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: d12fk <he...@op...> Gerrit-Comment-Date: Fri, 07 Mar 2025 11:03:22 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment |
|
From: d12fk (C. Review) <ge...@op...> - 2025-03-12 04:07:08
|
Attention is currently required from: d12fk, plaisthos.
Hello flichtenheld, plaisthos,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/904?usp=email
to look at the new patch set (#3).
The change is no longer submittable: checks~ChecksSubmitRule is unsatisfied now.
Change subject: dns: clean up --dhcp-options when --dns is active
......................................................................
dns: clean up --dhcp-options when --dns is active
Since --dns setting overrule DNS related --dhcp-options,
remove the latter when we got some via --dns.
Change-Id: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Signed-off-by: Heiko Hund <he...@is...>
---
M src/openvpn/options.c
1 file changed, 47 insertions(+), 0 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/04/904/3
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index afb3999..1bcd7bf 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -4161,6 +4161,53 @@
if (success)
{
dns_options_postprocess_pull(&o->dns_options);
+
+#if defined(_WIN32) || defined(TARGET_ANDROID)
+ /* If there's --dns servers, remove dns related --dhcp-options */
+ if (o->dns_options.servers)
+ {
+ o->tuntap_options.dns_len = 0;
+ o->tuntap_options.dns6_len = 0;
+ o->tuntap_options.domain = NULL;
+ o->tuntap_options.domain_search_list_len = 0;
+ }
+#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+ /* Clean up env from overridden DNS config */
+ struct gc_arena gc = gc_new();
+ struct buffer name = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+ struct buffer value = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+
+ const int fo_count = o->foreign_option_index;
+ o->foreign_option_index = 0;
+
+ for (int i = 1; i <= fo_count; ++i)
+ {
+ buf_clear(&name);
+ buf_printf(&name, "foreign_option_%d", i);
+ const char *env_str = env_set_get(es, BSTR(&name));
+ const char *item_val = strchr(env_str, '=') + 1;
+ buf_clear(&value);
+ buf_printf(&value, "%s", item_val);
+
+ /* Remove foreign option item from env set */
+ env_set_del(es, BSTR(&name));
+
+ item_val = BSTR(&value);
+ if (strncmp(item_val, "dhcp-option ", 12) != 0
+ || (strncmp(item_val + 12, "ADAPTER-DOMAIN-SUFFIX ", 22) != 0
+ && strncmp(item_val + 12, "DOMAIN-SEARCH ", 14) != 0
+ && strncmp(item_val + 12, "DOMAIN ", 7) != 0
+ && strncmp(item_val + 12, "DNS6 ", 5) != 0
+ && strncmp(item_val + 12, "DNS ", 4) != 0))
+ {
+ /* Re-set the item with potentially updated name */
+ buf_clear(&name);
+ buf_printf(&name, "foreign_option_%d", ++o->foreign_option_index);
+ setenv_str(es, BSTR(&name), BSTR(&value));
+ }
+ }
+ gc_free(&gc);
+#endif /* defined(_WIN32) || defined(TARGET_ANDROID) */
}
return success;
}
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/904?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: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Gerrit-Change-Number: 904
Gerrit-PatchSet: 3
Gerrit-Owner: d12fk <he...@op...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: d12fk <he...@op...>
Gerrit-MessageType: newpatchset
|
|
From: d12fk (C. Review) <ge...@op...> - 2025-03-14 04:21:57
|
Attention is currently required from: d12fk, flichtenheld, plaisthos.
Hello flichtenheld, plaisthos,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/904?usp=email
to look at the new patch set (#4).
The following approvals got outdated and were removed:
Code-Review+2 by flichtenheld
The change is no longer submittable: Code-Review and checks~ChecksSubmitRule are unsatisfied now.
Change subject: dns: clean up --dhcp-options when --dns is active
......................................................................
dns: clean up --dhcp-options when --dns is active
Since --dns settings overrule DNS related --dhcp-options,
remove the latter when we got some via --dns.
To stay as backward compatible as possible, the --dns server addresses
and search domains are added as foreign_options env vars, so that an
existing --up script can use them to set up DNS as before, without the
immediate need to change after an openvpn upgrade.
In case an --up script is defined, the --dns-script is not run to
prevent potential double DNS configuration.
Change-Id: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Signed-off-by: Heiko Hund <he...@is...>
---
M src/openvpn/dns.c
M src/openvpn/options.c
2 files changed, 106 insertions(+), 1 deletion(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/04/904/4
diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c
index 6ddf5c2..147509a 100644
--- a/src/openvpn/dns.c
+++ b/src/openvpn/dns.c
@@ -703,7 +703,7 @@
static void
run_up_down_script(bool up, struct options *o, const struct tuntap *tt, struct dns_script_runner_info *script_runner)
{
- if (!o->dns_options.script)
+ if (!o->dns_options.script || o->up_script)
{
return;
}
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 4c331dc..b26c864 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -4163,6 +4163,111 @@
if (success)
{
dns_options_postprocess_pull(&o->dns_options);
+ if (!o->dns_options.servers)
+ {
+ return true;
+ }
+
+#if defined(_WIN32) || defined(TARGET_ANDROID)
+ /* If there's --dns servers, remove dns related --dhcp-options */
+ o->tuntap_options.dns_len = 0;
+ o->tuntap_options.dns6_len = 0;
+ o->tuntap_options.domain = NULL;
+ o->tuntap_options.domain_search_list_len = 0;
+#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+
+ /* Clean up env from --dhcp-option DNS config */
+ struct gc_arena gc = gc_new();
+ struct buffer name = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+ struct buffer value = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+
+ const int fo_count = o->foreign_option_index;
+ o->foreign_option_index = 0;
+
+ for (int i = 1; i <= fo_count; ++i)
+ {
+ buf_clear(&name);
+ buf_printf(&name, "foreign_option_%d", i);
+ const char *env_str = env_set_get(es, BSTR(&name));
+ const char *item_val = strchr(env_str, '=') + 1;
+ buf_clear(&value);
+ buf_printf(&value, "%s", item_val);
+
+ /* Remove foreign option item from env set */
+ env_set_del(es, BSTR(&name));
+
+ item_val = BSTR(&value);
+ if (strncmp(item_val, "dhcp-option ", 12) != 0
+ || (strncmp(item_val + 12, "ADAPTER-DOMAIN-SUFFIX ", 22) != 0
+ && strncmp(item_val + 12, "DOMAIN-SEARCH ", 14) != 0
+ && strncmp(item_val + 12, "DOMAIN ", 7) != 0
+ && strncmp(item_val + 12, "DNS6 ", 5) != 0
+ && strncmp(item_val + 12, "DNS ", 4) != 0))
+ {
+ /* Re-set the item with potentially updated name */
+ buf_clear(&name);
+ buf_printf(&name, "foreign_option_%d", ++o->foreign_option_index);
+ setenv_str(es, BSTR(&name), BSTR(&value));
+ }
+ }
+
+ /* Set foreign option env vars from --dns config */
+ if (!o->up_script)
+ {
+ /* No need to, when there is no --up script */
+ return true;
+ }
+ const char *p[] = { "dhcp-option", NULL, NULL };
+ size_t p_len = sizeof(p) / sizeof(p[0]);
+
+ p[1] = "DOMAIN";
+ const struct dns_domain *d = o->dns_options.search_domains;
+ while (d)
+ {
+ p[2] = d->name;
+ setenv_foreign_option(o, (const char **)p, p_len, es);
+ d = d->next;
+ }
+
+ const struct dns_server *s = o->dns_options.servers;
+ while (s)
+ {
+ bool non_standard_server_port = false;
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].port && s->addr[i].port != 53)
+ {
+ non_standard_server_port = true;
+ break;
+ }
+ }
+ if ((s->transport && s->transport != DNS_TRANSPORT_PLAIN)
+ || (s->dnssec && s->dnssec != DNS_SECURITY_NO)
+ || non_standard_server_port)
+ {
+ /* Ignore servers requiring too much config to be set */
+ s = s->next;
+ continue;
+ }
+
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].family == AF_INET)
+ {
+ p[1] = "DNS";
+ p[2] = print_in_addr_t(s->addr[i].in.a4.s_addr, IA_NET_ORDER, &gc);
+ }
+ else
+ {
+ p[1] = "DNS6";
+ p[2] = print_in6_addr(s->addr[i].in.a6, 0, &gc);
+ }
+ setenv_foreign_option(o, (const char **)p, p_len, es);
+ }
+ s = s->next;
+ }
+ gc_free(&gc);
+#endif /* defined(_WIN32) || defined(TARGET_ANDROID) */
}
return success;
}
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/904?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: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Gerrit-Change-Number: 904
Gerrit-PatchSet: 4
Gerrit-Owner: d12fk <he...@op...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: d12fk <he...@op...>
Gerrit-MessageType: newpatchset
|
|
From: d12fk (C. Review) <ge...@op...> - 2025-03-18 02:54:36
|
Attention is currently required from: d12fk, flichtenheld, plaisthos.
Hello flichtenheld, plaisthos,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/904?usp=email
to look at the new patch set (#5).
Change subject: dns: clean up --dhcp-options when --dns is active
......................................................................
dns: clean up --dhcp-options when --dns is active
Since --dns settings overrule DNS related --dhcp-options,
remove the latter when we got some via --dns.
To stay as backward compatible as possible, the --dns server addresses
and search domains are added as foreign_options env vars, so that an
existing --up script can use them to set up DNS as before, without the
immediate need to change after an openvpn upgrade.
In case an --up script is defined, the --dns-script is not run to
prevent potential double DNS configuration.
Change-Id: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Signed-off-by: Heiko Hund <he...@is...>
---
M src/openvpn/dns.c
M src/openvpn/options.c
2 files changed, 152 insertions(+), 1 deletion(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/04/904/5
diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c
index 6ddf5c2..147509a 100644
--- a/src/openvpn/dns.c
+++ b/src/openvpn/dns.c
@@ -703,7 +703,7 @@
static void
run_up_down_script(bool up, struct options *o, const struct tuntap *tt, struct dns_script_runner_info *script_runner)
{
- if (!o->dns_options.script)
+ if (!o->dns_options.script || o->up_script)
{
return;
}
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 4c331dc..e94af84 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -4163,6 +4163,157 @@
if (success)
{
dns_options_postprocess_pull(&o->dns_options);
+ if (!o->dns_options.servers)
+ {
+ return true;
+ }
+
+#if defined(_WIN32) || defined(TARGET_ANDROID)
+ /* Remove DNS related --dhcp-options */
+ struct tuntap_options *tt = &o->tuntap_options;
+ tt->dns_len = 0;
+ tt->dns6_len = 0;
+ tt->domain = NULL;
+ tt->domain_search_list_len = 0;
+
+ /* Set options from --dns config */
+ const struct dns_domain *d = o->dns_options.search_domains;
+ while (d && tt->domain_search_list_len + 1 < N_SEARCH_LIST_LEN)
+ {
+ tt->domain_search_list[tt->domain_search_list_len++] = d->name;
+ d = d->next;
+ }
+
+ const struct dns_server *s = o->dns_options.servers;
+ while (s)
+ {
+ bool non_standard_server_port = false;
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].port && s->addr[i].port != 53)
+ {
+ non_standard_server_port = true;
+ break;
+ }
+ }
+ if ((s->transport && s->transport != DNS_TRANSPORT_PLAIN)
+ || (s->dnssec && s->dnssec != DNS_SECURITY_NO)
+ || non_standard_server_port)
+ {
+ /* Skip servers requiring unsupported config to be set */
+ s = s->next;
+ }
+ else
+ {
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].family == AF_INET && tt->dns_len + 1 < N_DHCP_ADDR)
+ {
+ tt->dns[tt->dns_len++] = s->addr[i].in.a4.s_addr;
+ }
+ else if (tt->dns6_len + 1 < N_DHCP_ADDR)
+ {
+ tt->dns6[tt->dns6_len] = s->addr[i].in.a6;
+ }
+ }
+ break;
+ }
+ }
+#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+
+ /* Clean up env from --dhcp-option DNS config */
+ struct gc_arena gc = gc_new();
+ struct buffer name = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+ struct buffer value = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+
+ const int fo_count = o->foreign_option_index;
+ o->foreign_option_index = 0;
+
+ for (int i = 1; i <= fo_count; ++i)
+ {
+ buf_clear(&name);
+ buf_printf(&name, "foreign_option_%d", i);
+ const char *env_str = env_set_get(es, BSTR(&name));
+ const char *item_val = strchr(env_str, '=') + 1;
+ buf_clear(&value);
+ buf_printf(&value, "%s", item_val);
+
+ /* Remove foreign option item from env set */
+ env_set_del(es, BSTR(&name));
+
+ item_val = BSTR(&value);
+ if (strncmp(item_val, "dhcp-option ", 12) != 0
+ || (strncmp(item_val + 12, "ADAPTER-DOMAIN-SUFFIX ", 22) != 0
+ && strncmp(item_val + 12, "DOMAIN-SEARCH ", 14) != 0
+ && strncmp(item_val + 12, "DOMAIN ", 7) != 0
+ && strncmp(item_val + 12, "DNS6 ", 5) != 0
+ && strncmp(item_val + 12, "DNS ", 4) != 0))
+ {
+ /* Re-set the item with potentially updated name */
+ buf_clear(&name);
+ buf_printf(&name, "foreign_option_%d", ++o->foreign_option_index);
+ setenv_str(es, BSTR(&name), BSTR(&value));
+ }
+ }
+
+ /* Set foreign option env vars from --dns config */
+ if (!o->up_script)
+ {
+ /* No need to, when there is no --up script */
+ return true;
+ }
+ const char *p[] = { "dhcp-option", NULL, NULL };
+ size_t p_len = sizeof(p) / sizeof(p[0]);
+
+ p[1] = "DOMAIN";
+ const struct dns_domain *d = o->dns_options.search_domains;
+ while (d)
+ {
+ p[2] = d->name;
+ setenv_foreign_option(o, (const char **)p, p_len, es);
+ d = d->next;
+ }
+
+ const struct dns_server *s = o->dns_options.servers;
+ while (s)
+ {
+ bool non_standard_server_port = false;
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].port && s->addr[i].port != 53)
+ {
+ non_standard_server_port = true;
+ break;
+ }
+ }
+ if ((s->transport && s->transport != DNS_TRANSPORT_PLAIN)
+ || (s->dnssec && s->dnssec != DNS_SECURITY_NO)
+ || non_standard_server_port)
+ {
+ /* Skip servers requiring unsupported config to be set */
+ s = s->next;
+ }
+ else
+ {
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].family == AF_INET)
+ {
+ p[1] = "DNS";
+ p[2] = print_in_addr_t(s->addr[i].in.a4.s_addr, IA_NET_ORDER, &gc);
+ }
+ else
+ {
+ p[1] = "DNS6";
+ p[2] = print_in6_addr(s->addr[i].in.a6, 0, &gc);
+ }
+ setenv_foreign_option(o, (const char **)p, p_len, es);
+ }
+ break;
+ }
+ }
+ gc_free(&gc);
+#endif /* defined(_WIN32) || defined(TARGET_ANDROID) */
}
return success;
}
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/904?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: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Gerrit-Change-Number: 904
Gerrit-PatchSet: 5
Gerrit-Owner: d12fk <he...@op...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: d12fk <he...@op...>
Gerrit-MessageType: newpatchset
|
|
From: d12fk (C. Review) <ge...@op...> - 2025-03-18 05:31:04
|
Attention is currently required from: d12fk, flichtenheld, plaisthos.
Hello flichtenheld, plaisthos,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/904?usp=email
to look at the new patch set (#6).
Change subject: dns: clean up --dhcp-options when --dns is active
......................................................................
dns: clean up --dhcp-options when --dns is active
Since --dns settings overrule DNS related --dhcp-options,
remove the latter when we got some via --dns.
To stay as backward compatible as possible, the --dns server addresses
and search domains are added as foreign_options env vars, so that an
existing --up script can use them to set up DNS as before, without the
immediate need to change after an openvpn upgrade. Same goes for the
DNS information in struct tuntap_options, which is used by Android.
In case an --up script is defined, the --dns-updown is not run to
prevent potential double DNS configuration.
Change-Id: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Signed-off-by: Heiko Hund <he...@is...>
---
M src/openvpn/dns.c
M src/openvpn/options.c
2 files changed, 155 insertions(+), 1 deletion(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/04/904/6
diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c
index 0ad8e44..6a5999a 100644
--- a/src/openvpn/dns.c
+++ b/src/openvpn/dns.c
@@ -703,7 +703,7 @@
static void
run_up_down_command(bool up, struct options *o, const struct tuntap *tt, struct dns_updown_runner_info *updown_runner)
{
- if (!o->dns_options.updown)
+ if (!o->dns_options.updown || o->up_script)
{
return;
}
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index b63f929..966230a 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -4163,6 +4163,160 @@
if (success)
{
dns_options_postprocess_pull(&o->dns_options);
+ if (!o->dns_options.servers)
+ {
+ return true;
+ }
+
+#if defined(_WIN32) || defined(TARGET_ANDROID)
+ /* Remove DNS related --dhcp-options */
+ struct tuntap_options *tt = &o->tuntap_options;
+ tt->dns_len = 0;
+ tt->dns6_len = 0;
+ tt->domain = NULL;
+ tt->domain_search_list_len = 0;
+
+#if defined(TARGET_ANDROID)
+ /* Set options from --dns config */
+ const struct dns_domain *d = o->dns_options.search_domains;
+ while (d && tt->domain_search_list_len + 1 < N_SEARCH_LIST_LEN)
+ {
+ tt->domain_search_list[tt->domain_search_list_len++] = d->name;
+ d = d->next;
+ }
+
+ const struct dns_server *s = o->dns_options.servers;
+ while (s)
+ {
+ bool non_standard_server_port = false;
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].port && s->addr[i].port != 53)
+ {
+ non_standard_server_port = true;
+ break;
+ }
+ }
+ if ((s->transport && s->transport != DNS_TRANSPORT_PLAIN)
+ || (s->dnssec && s->dnssec != DNS_SECURITY_NO)
+ || non_standard_server_port)
+ {
+ /* Skip servers requiring unsupported config to be set */
+ s = s->next;
+ }
+ else
+ {
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].family == AF_INET && tt->dns_len + 1 < N_DHCP_ADDR)
+ {
+ tt->dns[tt->dns_len++] = s->addr[i].in.a4.s_addr;
+ }
+ else if (tt->dns6_len + 1 < N_DHCP_ADDR)
+ {
+ tt->dns6[tt->dns6_len] = s->addr[i].in.a6;
+ }
+ }
+ break;
+ }
+ }
+#endif /* if defined(TARGET_ANDROID) */
+
+#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+
+ /* Clean up env from --dhcp-option DNS config */
+ struct gc_arena gc = gc_new();
+ struct buffer name = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+ struct buffer value = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+
+ const int fo_count = o->foreign_option_index;
+ o->foreign_option_index = 0;
+
+ for (int i = 1; i <= fo_count; ++i)
+ {
+ buf_clear(&name);
+ buf_printf(&name, "foreign_option_%d", i);
+ const char *env_str = env_set_get(es, BSTR(&name));
+ const char *item_val = strchr(env_str, '=') + 1;
+ buf_clear(&value);
+ buf_printf(&value, "%s", item_val);
+
+ /* Remove foreign option item from env set */
+ env_set_del(es, BSTR(&name));
+
+ item_val = BSTR(&value);
+ if (strncmp(item_val, "dhcp-option ", 12) != 0
+ || (strncmp(item_val + 12, "ADAPTER-DOMAIN-SUFFIX ", 22) != 0
+ && strncmp(item_val + 12, "DOMAIN-SEARCH ", 14) != 0
+ && strncmp(item_val + 12, "DOMAIN ", 7) != 0
+ && strncmp(item_val + 12, "DNS6 ", 5) != 0
+ && strncmp(item_val + 12, "DNS ", 4) != 0))
+ {
+ /* Re-set the item with potentially updated name */
+ buf_clear(&name);
+ buf_printf(&name, "foreign_option_%d", ++o->foreign_option_index);
+ setenv_str(es, BSTR(&name), BSTR(&value));
+ }
+ }
+
+ /* Set foreign option env vars from --dns config */
+ if (!o->up_script)
+ {
+ /* No need to, when there is no --up script */
+ return true;
+ }
+ const char *p[] = { "dhcp-option", NULL, NULL };
+ size_t p_len = sizeof(p) / sizeof(p[0]);
+
+ p[1] = "DOMAIN";
+ const struct dns_domain *d = o->dns_options.search_domains;
+ while (d)
+ {
+ p[2] = d->name;
+ setenv_foreign_option(o, (const char **)p, p_len, es);
+ d = d->next;
+ }
+
+ const struct dns_server *s = o->dns_options.servers;
+ while (s)
+ {
+ bool non_standard_server_port = false;
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].port && s->addr[i].port != 53)
+ {
+ non_standard_server_port = true;
+ break;
+ }
+ }
+ if ((s->transport && s->transport != DNS_TRANSPORT_PLAIN)
+ || (s->dnssec && s->dnssec != DNS_SECURITY_NO)
+ || non_standard_server_port)
+ {
+ /* Skip servers requiring unsupported config to be set */
+ s = s->next;
+ }
+ else
+ {
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].family == AF_INET)
+ {
+ p[1] = "DNS";
+ p[2] = print_in_addr_t(s->addr[i].in.a4.s_addr, IA_NET_ORDER, &gc);
+ }
+ else
+ {
+ p[1] = "DNS6";
+ p[2] = print_in6_addr(s->addr[i].in.a6, 0, &gc);
+ }
+ setenv_foreign_option(o, (const char **)p, p_len, es);
+ }
+ break;
+ }
+ }
+ gc_free(&gc);
+#endif /* defined(_WIN32) || defined(TARGET_ANDROID) */
}
return success;
}
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/904?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: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Gerrit-Change-Number: 904
Gerrit-PatchSet: 6
Gerrit-Owner: d12fk <he...@op...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: d12fk <he...@op...>
Gerrit-MessageType: newpatchset
|
|
From: d12fk (C. Review) <ge...@op...> - 2025-03-20 02:52:02
|
Attention is currently required from: d12fk, flichtenheld, plaisthos.
Hello flichtenheld, plaisthos,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/904?usp=email
to look at the new patch set (#8).
Change subject: dns: deal with --dhcp-options when --dns is active
......................................................................
dns: deal with --dhcp-options when --dns is active
Since --dns settings overrule DNS related --dhcp-options,
remove the latter when values were defined via --dns.
To stay as backward compatible as possible, we add foreign_options to
the script hook environment from the --dns values when a --up script
is defined. In that case the default --dns-updown is not run, even
when --dns values are present, to prevent double DNS configuration.
This way an existing --up script that deals with DNS can run, without
the immediate need to change after an openvpn upgrade and a server
pushing --dns options.
If you specify a custom --dns-updown, or force running the default
dns-updown that comes with openvpn, those compat env vars are not set
for --up scripts and the dns-updown command is run, even when there's
an --up script present.
Since Android uses the DNS values from tuntap_options, we always
override those with --dns stuff unconditionally.
Change-Id: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Signed-off-by: Heiko Hund <he...@is...>
---
M doc/man-sections/script-options.rst
M src/openvpn/dns.c
M src/openvpn/options.c
3 files changed, 231 insertions(+), 158 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/04/904/8
diff --git a/doc/man-sections/script-options.rst b/doc/man-sections/script-options.rst
index 8285122..765518a 100644
--- a/doc/man-sections/script-options.rst
+++ b/doc/man-sections/script-options.rst
@@ -258,9 +258,11 @@
Note that DNS-related ``--dhcp-option``\ s will not be handled by this hook.
If any ``--dns server`` option is present, DNS-related ``--dhcp-option``\ s will
be ignored. If an ``--up`` script is defined, foreign_option env vars will be
- generated from ``--dns`` options and passed to the script. The ``--dns-updown``
- command is not run if a ``--up`` script is defined. Both is done for backward
- compatibility.
+ generated from ``--dns`` options and passed to the script for backward compatibility.
+
+ Also for backward compatibility, the ``--dns-updown`` command is not run if a
+ ``--up`` script is defined, unless a user-defined ``dns-updown`` was specified,
+ or running the default updown is forced via the ``--dns-updown force`` option.
--down cmd
Run command ``cmd`` after TUN/TAP device close (post ``--user`` UID
diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c
index 0ad8e44..c654d78 100644
--- a/src/openvpn/dns.c
+++ b/src/openvpn/dns.c
@@ -703,7 +703,8 @@
static void
run_up_down_command(bool up, struct options *o, const struct tuntap *tt, struct dns_updown_runner_info *updown_runner)
{
- if (!o->dns_options.updown)
+ struct dns_options *dns = &o->dns_options;
+ if (!dns->updown || (o->up_script && !dns->user_defined_updown))
{
return;
}
@@ -713,7 +714,7 @@
if (!updown_runner->required)
{
/* Run dns updown directly */
- status = do_run_up_down_command(up, NULL, &o->dns_options, tt);
+ status = do_run_up_down_command(up, NULL, dns, tt);
}
else
{
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 1a9c337..22c882b 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -530,7 +530,9 @@
" sni <domain> : DNS server name indication\n"
"--dns search-domains <domain> [domain ...]:\n"
" Add domains to DNS domain search list\n"
- "--dns-updown cmd|disable : Run cmd as user defined dns config command or disable it\n"
+ "--dns-updown cmd|force|disable :\n"
+ " Run cmd as user defined dns config command, force running the\n"
+ " default one even when --up exists, or disable running it.\n"
"--auth-retry t : How to handle auth failures. Set t to\n"
" none (default), interact, or nointeract.\n"
"--static-challenge t e [<scrv1|concat>]: Enable static challenge/response protocol using\n"
@@ -1374,149 +1376,6 @@
}
}
}
-
-/*
- * If DNS options are set use these for TUN/TAP options as well.
- * Applies to DNS, DNS6 and DOMAIN-SEARCH.
- * Existing options will be discarded.
- */
-static void
-tuntap_options_copy_dns(struct options *o)
-{
- struct tuntap_options *tt = &o->tuntap_options;
- struct dns_options *dns = &o->dns_options;
-
- if (dns->search_domains)
- {
- tt->domain_search_list_len = 0;
- const struct dns_domain *domain = dns->search_domains;
- while (domain && tt->domain_search_list_len < N_SEARCH_LIST_LEN)
- {
- tt->domain_search_list[tt->domain_search_list_len++] = domain->name;
- domain = domain->next;
- }
- if (domain)
- {
- msg(M_WARN, "WARNING: couldn't copy all --dns search-domains to --dhcp-option");
- }
- tt->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
- }
-
- if (dns->servers)
- {
- tt->dns_len = 0;
- tt->dns6_len = 0;
- bool overflow = false;
- const struct dns_server *server = dns->servers;
- while (server)
- {
- for (int i = 0; i < server->addr_count; ++i)
- {
- if (server->addr[i].family == AF_INET)
- {
- if (tt->dns_len >= N_DHCP_ADDR)
- {
- overflow = true;
- continue;
- }
- tt->dns[tt->dns_len++] = ntohl(server->addr[i].in.a4.s_addr);
- }
- else
- {
- if (tt->dns6_len >= N_DHCP_ADDR)
- {
- overflow = true;
- continue;
- }
- tt->dns6[tt->dns6_len++] = server->addr[i].in.a6;
- }
- }
- server = server->next;
- }
- if (overflow)
- {
- msg(M_WARN, "WARNING: couldn't copy all --dns server addresses to --dhcp-option");
- }
- tt->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
- }
-}
-#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
-static void
-foreign_options_copy_dns(struct options *o, struct env_set *es)
-{
- const struct dns_domain *domain = o->dns_options.search_domains;
- const struct dns_server *server = o->dns_options.servers;
- if (!domain && !server)
- {
- return;
- }
-
- /* reset the index since we're starting all over again */
- int opt_max = o->foreign_option_index;
- o->foreign_option_index = 0;
-
- for (int i = 1; i <= opt_max; ++i)
- {
- char name[32];
- snprintf(name, sizeof(name), "foreign_option_%d", i);
-
- const char *env_str = env_set_get(es, name);
- const char *value = strchr(env_str, '=') + 1;
- if ((domain && strstr(value, "dhcp-option DOMAIN-SEARCH") == value)
- || (server && strstr(value, "dhcp-option DNS") == value))
- {
- setenv_del(es, name);
- }
- else
- {
- setenv_foreign_option(o, &value, 1, es);
- }
- }
-
- struct gc_arena gc = gc_new();
-
- while (server)
- {
- for (size_t i = 0; i < server->addr_count; ++i)
- {
- if (server->addr[i].family == AF_INET)
- {
- const char *argv[] = {
- "dhcp-option",
- "DNS",
- print_in_addr_t(server->addr[i].in.a4.s_addr, 0, &gc)
- };
- setenv_foreign_option(o, argv, 3, es);
- }
- else
- {
- const char *argv[] = {
- "dhcp-option",
- "DNS6",
- print_in6_addr(server->addr[i].in.a6, 0, &gc)
- };
- setenv_foreign_option(o, argv, 3, es);
- }
- }
- server = server->next;
- }
- while (domain)
- {
- const char *argv[] = { "dhcp-option", "DOMAIN-SEARCH", domain->name };
- setenv_foreign_option(o, argv, 3, es);
- domain = domain->next;
- }
-
- gc_free(&gc);
-
- /* remove old leftover entries */
- while (o->foreign_option_index < opt_max)
- {
- char name[32];
- snprintf(name, sizeof(name), "foreign_option_%d", opt_max--);
- setenv_del(es, name);
- }
-}
#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
#ifndef ENABLE_SMALL
@@ -3616,6 +3475,207 @@
}
}
+#if defined(_WIN32) || defined(TARGET_ANDROID)
+/**
+ * @brief Postprocess DNS related settings
+ *
+ * Discard existing TUN/TAP DNS options and override them with --dns
+ * values for Android. If there are no --dns servers leave things alone.
+ *
+ * @param o pointer to the options struct
+ */
+static void
+tuntap_options_postprocess_dns(struct options *o)
+{
+ struct dns_options *dns = &o->dns_options;
+ if (!dns->servers)
+ {
+ return;
+ }
+
+ /* Remove DNS related --dhcp-options */
+ struct tuntap_options *tt = &o->tuntap_options;
+ tt->dns_len = 0;
+ tt->dns6_len = 0;
+ tt->domain = NULL;
+ tt->domain_search_list_len = 0;
+
+#if defined(TARGET_ANDROID)
+ /* Set options from --dns config */
+ const struct dns_domain *d = dns->search_domains;
+ while (d && tt->domain_search_list_len + 1 < N_SEARCH_LIST_LEN)
+ {
+ tt->domain_search_list[tt->domain_search_list_len++] = d->name;
+ d = d->next;
+ }
+ if (domain)
+ {
+ msg(M_WARN, "WARNING: couldn't copy all --dns search-domains to TUN/TAP");
+ }
+
+ const struct dns_server *s = dns->servers;
+ while (s)
+ {
+ bool non_standard_server_port = false;
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].port && s->addr[i].port != 53)
+ {
+ non_standard_server_port = true;
+ break;
+ }
+ }
+ if ((s->transport && s->transport != DNS_TRANSPORT_PLAIN)
+ || (s->dnssec && s->dnssec != DNS_SECURITY_NO)
+ || non_standard_server_port)
+ {
+ /* Skip servers requiring unsupported config to be set */
+ s = s->next;
+ }
+ else
+ {
+ bool overflow = false;
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].family == AF_INET && tt->dns_len + 1 < N_DHCP_ADDR)
+ {
+ tt->dns[tt->dns_len++] = s->addr[i].in.a4.s_addr;
+ }
+ else if (tt->dns6_len + 1 < N_DHCP_ADDR)
+ {
+ tt->dns6[tt->dns6_len] = s->addr[i].in.a6;
+ }
+ else
+ {
+ overflow = true;
+ }
+ }
+ if (overflow)
+ {
+ msg(M_WARN, "WARNING: couldn't copy all --dns server addresses to TUN/TAP");
+ }
+ return;
+ }
+ }
+#endif /* if defined(TARGET_ANDROID) */
+}
+
+#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+
+/**
+ * @brief Postprocess DNS related settings in the env_set
+ *
+ * Discard existing --dhcp-options and set values from --dns
+ * instead. If there are no --dns servers leave things alone.
+ *
+ * @param o pointer to the options struct
+ * @param es env set to modify potentially
+ */
+static void
+foreign_options_postprocess_dns(struct options *o, struct env_set *es)
+{
+ struct dns_options *dns = &o->dns_options;
+ if (!dns->servers)
+ {
+ return;
+ }
+
+ /* Clean up env from --dhcp-option DNS config */
+ struct gc_arena gc = gc_new();
+ struct buffer name = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+ struct buffer value = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+
+ const int fo_count = o->foreign_option_index;
+ o->foreign_option_index = 0;
+
+ for (int i = 1; i <= fo_count; ++i)
+ {
+ buf_clear(&name);
+ buf_printf(&name, "foreign_option_%d", i);
+ const char *env_str = env_set_get(es, BSTR(&name));
+ const char *item_val = strchr(env_str, '=') + 1;
+ buf_clear(&value);
+ buf_printf(&value, "%s", item_val);
+
+ /* Remove foreign option item from env set */
+ env_set_del(es, BSTR(&name));
+
+ item_val = BSTR(&value);
+ if (strncmp(item_val, "dhcp-option ", 12) != 0
+ || (strncmp(item_val + 12, "ADAPTER-DOMAIN-SUFFIX ", 22) != 0
+ && strncmp(item_val + 12, "DOMAIN-SEARCH ", 14) != 0
+ && strncmp(item_val + 12, "DOMAIN ", 7) != 0
+ && strncmp(item_val + 12, "DNS6 ", 5) != 0
+ && strncmp(item_val + 12, "DNS ", 4) != 0))
+ {
+ /* Re-set the item with potentially updated name */
+ buf_clear(&name);
+ buf_printf(&name, "foreign_option_%d", ++o->foreign_option_index);
+ setenv_str(es, BSTR(&name), BSTR(&value));
+ }
+ }
+
+ /* Set foreign option env vars from --dns config */
+ if (!o->up_script || dns->user_defined_updown)
+ {
+ /* No need to, when there is no --up script or a custom updown */
+ return;
+ }
+
+ const char *p[] = { "dhcp-option", NULL, NULL };
+ size_t p_len = sizeof(p) / sizeof(p[0]);
+
+ p[1] = "DOMAIN";
+ const struct dns_domain *d = dns->search_domains;
+ while (d)
+ {
+ p[2] = d->name;
+ setenv_foreign_option(o, (const char **)p, p_len, es);
+ d = d->next;
+ }
+
+ const struct dns_server *s = dns->servers;
+ while (s)
+ {
+ bool non_standard_server_port = false;
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].port && s->addr[i].port != 53)
+ {
+ non_standard_server_port = true;
+ break;
+ }
+ }
+ if ((s->transport && s->transport != DNS_TRANSPORT_PLAIN)
+ || (s->dnssec && s->dnssec != DNS_SECURITY_NO)
+ || non_standard_server_port)
+ {
+ /* Skip servers requiring unsupported config to be set */
+ s = s->next;
+ }
+ else
+ {
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].family == AF_INET)
+ {
+ p[1] = "DNS";
+ p[2] = print_in_addr_t(s->addr[i].in.a4.s_addr, IA_NET_ORDER, &gc);
+ }
+ else
+ {
+ p[1] = "DNS6";
+ p[2] = print_in6_addr(s->addr[i].in.a6, 0, &gc);
+ }
+ setenv_foreign_option(o, (const char **)p, p_len, es);
+ }
+ break;
+ }
+ }
+ gc_free(&gc);
+}
+#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+
static void
options_postprocess_mutate(struct options *o, struct env_set *es)
{
@@ -3801,9 +3861,9 @@
else
{
#if defined(_WIN32) || defined(TARGET_ANDROID)
- tuntap_options_copy_dns(o);
+ tuntap_options_postprocess_dns(o);
#else
- foreign_options_copy_dns(o, es);
+ foreign_options_postprocess_dns(o, es);
#endif
}
if (o->auth_token_generate && !o->auth_token_renewal)
@@ -4186,9 +4246,9 @@
{
dns_options_postprocess_pull(&o->dns_options);
#if defined(_WIN32) || defined(TARGET_ANDROID)
- tuntap_options_copy_dns(o);
+ tuntap_options_postprocess_dns(o);
#else
- foreign_options_copy_dns(o, es);
+ foreign_options_postprocess_dns(o, es);
#endif
}
return success;
@@ -8089,19 +8149,29 @@
goto err;
}
#ifdef ENABLE_DNS_UPDOWN
+ struct dns_options *dns = &options->dns_options;
if (streq(p[1], "disable"))
{
- options->dns_options.updown = NULL;
+ dns->updown = NULL;
+ dns->user_defined_updown = false;
+ }
+ else if (streq(p[1], "force"))
+ {
+ /* force dns-updown run, even if a --up script is defined */
+ if (dns->updown)
+ {
+ dns->user_defined_updown = true;
+ }
}
else
{
- if (options->dns_options.user_defined_updown == false)
+ if (dns->user_defined_updown == false)
{
/* Unset the default command to prevent warnings */
- options->dns_options.updown = NULL;
+ dns->updown = NULL;
}
- set_user_script(options, &options->dns_options.updown, p[1], p[0], false);
- options->dns_options.user_defined_updown = true;
+ set_user_script(options, &dns->updown, p[1], p[0], false);
+ dns->user_defined_updown = true;
}
#endif /* ENABLE_DNS_UPDOWN */
}
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/904?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: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Gerrit-Change-Number: 904
Gerrit-PatchSet: 8
Gerrit-Owner: d12fk <he...@op...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: d12fk <he...@op...>
Gerrit-MessageType: newpatchset
|
|
From: flichtenheld (C. Review) <ge...@op...> - 2025-03-20 11:34:30
|
Attention is currently required from: d12fk, plaisthos. flichtenheld has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/904?usp=email ) Change subject: dns: deal with --dhcp-options when --dns is active ...................................................................... Patch Set 8: Code-Review-1 (2 comments) Patchset: PS8: build failure File src/openvpn/options.c: http://gerrit.openvpn.net/c/openvpn/+/904/comment/0bb84115_ddb1f6f9 : PS8, Line 3511: ) `error: use of undeclared identifier 'domain'` -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/904?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: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a Gerrit-Change-Number: 904 Gerrit-PatchSet: 8 Gerrit-Owner: d12fk <he...@op...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: d12fk <he...@op...> Gerrit-Comment-Date: Thu, 20 Mar 2025 11:34:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment |
|
From: d12fk (C. Review) <ge...@op...> - 2025-03-20 12:10:05
|
Attention is currently required from: d12fk, flichtenheld, plaisthos.
Hello flichtenheld, plaisthos,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/904?usp=email
to look at the new patch set (#9).
The following approvals got outdated and were removed:
Code-Review-1 by flichtenheld
Change subject: dns: deal with --dhcp-options when --dns is active
......................................................................
dns: deal with --dhcp-options when --dns is active
Since --dns settings overrule DNS related --dhcp-options,
remove the latter when values were defined via --dns.
To stay as backward compatible as possible, we add foreign_options to
the script hook environment from the --dns values when a --up script
is defined. In that case the default --dns-updown is not run, even
when --dns values are present, to prevent double DNS configuration.
This way an existing --up script that deals with DNS can run, without
the immediate need to change after an openvpn upgrade and a server
pushing --dns options.
If you specify a custom --dns-updown, or force running the default
dns-updown that comes with openvpn, those compat env vars are not set
for --up scripts and the dns-updown command is run, even when there's
an --up script present.
Since Android uses the DNS values from tuntap_options, we always
override those with --dns stuff unconditionally.
Change-Id: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Signed-off-by: Heiko Hund <he...@is...>
---
M doc/man-sections/script-options.rst
M src/openvpn/dns.c
M src/openvpn/options.c
3 files changed, 231 insertions(+), 158 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/04/904/9
diff --git a/doc/man-sections/script-options.rst b/doc/man-sections/script-options.rst
index 8285122..765518a 100644
--- a/doc/man-sections/script-options.rst
+++ b/doc/man-sections/script-options.rst
@@ -258,9 +258,11 @@
Note that DNS-related ``--dhcp-option``\ s will not be handled by this hook.
If any ``--dns server`` option is present, DNS-related ``--dhcp-option``\ s will
be ignored. If an ``--up`` script is defined, foreign_option env vars will be
- generated from ``--dns`` options and passed to the script. The ``--dns-updown``
- command is not run if a ``--up`` script is defined. Both is done for backward
- compatibility.
+ generated from ``--dns`` options and passed to the script for backward compatibility.
+
+ Also for backward compatibility, the ``--dns-updown`` command is not run if a
+ ``--up`` script is defined, unless a user-defined ``dns-updown`` was specified,
+ or running the default updown is forced via the ``--dns-updown force`` option.
--down cmd
Run command ``cmd`` after TUN/TAP device close (post ``--user`` UID
diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c
index 0ad8e44..c654d78 100644
--- a/src/openvpn/dns.c
+++ b/src/openvpn/dns.c
@@ -703,7 +703,8 @@
static void
run_up_down_command(bool up, struct options *o, const struct tuntap *tt, struct dns_updown_runner_info *updown_runner)
{
- if (!o->dns_options.updown)
+ struct dns_options *dns = &o->dns_options;
+ if (!dns->updown || (o->up_script && !dns->user_defined_updown))
{
return;
}
@@ -713,7 +714,7 @@
if (!updown_runner->required)
{
/* Run dns updown directly */
- status = do_run_up_down_command(up, NULL, &o->dns_options, tt);
+ status = do_run_up_down_command(up, NULL, dns, tt);
}
else
{
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 1a9c337..25f97e0 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -530,7 +530,9 @@
" sni <domain> : DNS server name indication\n"
"--dns search-domains <domain> [domain ...]:\n"
" Add domains to DNS domain search list\n"
- "--dns-updown cmd|disable : Run cmd as user defined dns config command or disable it\n"
+ "--dns-updown cmd|force|disable :\n"
+ " Run cmd as user defined dns config command, force running the\n"
+ " default one even when --up exists, or disable running it.\n"
"--auth-retry t : How to handle auth failures. Set t to\n"
" none (default), interact, or nointeract.\n"
"--static-challenge t e [<scrv1|concat>]: Enable static challenge/response protocol using\n"
@@ -1374,149 +1376,6 @@
}
}
}
-
-/*
- * If DNS options are set use these for TUN/TAP options as well.
- * Applies to DNS, DNS6 and DOMAIN-SEARCH.
- * Existing options will be discarded.
- */
-static void
-tuntap_options_copy_dns(struct options *o)
-{
- struct tuntap_options *tt = &o->tuntap_options;
- struct dns_options *dns = &o->dns_options;
-
- if (dns->search_domains)
- {
- tt->domain_search_list_len = 0;
- const struct dns_domain *domain = dns->search_domains;
- while (domain && tt->domain_search_list_len < N_SEARCH_LIST_LEN)
- {
- tt->domain_search_list[tt->domain_search_list_len++] = domain->name;
- domain = domain->next;
- }
- if (domain)
- {
- msg(M_WARN, "WARNING: couldn't copy all --dns search-domains to --dhcp-option");
- }
- tt->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
- }
-
- if (dns->servers)
- {
- tt->dns_len = 0;
- tt->dns6_len = 0;
- bool overflow = false;
- const struct dns_server *server = dns->servers;
- while (server)
- {
- for (int i = 0; i < server->addr_count; ++i)
- {
- if (server->addr[i].family == AF_INET)
- {
- if (tt->dns_len >= N_DHCP_ADDR)
- {
- overflow = true;
- continue;
- }
- tt->dns[tt->dns_len++] = ntohl(server->addr[i].in.a4.s_addr);
- }
- else
- {
- if (tt->dns6_len >= N_DHCP_ADDR)
- {
- overflow = true;
- continue;
- }
- tt->dns6[tt->dns6_len++] = server->addr[i].in.a6;
- }
- }
- server = server->next;
- }
- if (overflow)
- {
- msg(M_WARN, "WARNING: couldn't copy all --dns server addresses to --dhcp-option");
- }
- tt->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
- }
-}
-#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
-static void
-foreign_options_copy_dns(struct options *o, struct env_set *es)
-{
- const struct dns_domain *domain = o->dns_options.search_domains;
- const struct dns_server *server = o->dns_options.servers;
- if (!domain && !server)
- {
- return;
- }
-
- /* reset the index since we're starting all over again */
- int opt_max = o->foreign_option_index;
- o->foreign_option_index = 0;
-
- for (int i = 1; i <= opt_max; ++i)
- {
- char name[32];
- snprintf(name, sizeof(name), "foreign_option_%d", i);
-
- const char *env_str = env_set_get(es, name);
- const char *value = strchr(env_str, '=') + 1;
- if ((domain && strstr(value, "dhcp-option DOMAIN-SEARCH") == value)
- || (server && strstr(value, "dhcp-option DNS") == value))
- {
- setenv_del(es, name);
- }
- else
- {
- setenv_foreign_option(o, &value, 1, es);
- }
- }
-
- struct gc_arena gc = gc_new();
-
- while (server)
- {
- for (size_t i = 0; i < server->addr_count; ++i)
- {
- if (server->addr[i].family == AF_INET)
- {
- const char *argv[] = {
- "dhcp-option",
- "DNS",
- print_in_addr_t(server->addr[i].in.a4.s_addr, 0, &gc)
- };
- setenv_foreign_option(o, argv, 3, es);
- }
- else
- {
- const char *argv[] = {
- "dhcp-option",
- "DNS6",
- print_in6_addr(server->addr[i].in.a6, 0, &gc)
- };
- setenv_foreign_option(o, argv, 3, es);
- }
- }
- server = server->next;
- }
- while (domain)
- {
- const char *argv[] = { "dhcp-option", "DOMAIN-SEARCH", domain->name };
- setenv_foreign_option(o, argv, 3, es);
- domain = domain->next;
- }
-
- gc_free(&gc);
-
- /* remove old leftover entries */
- while (o->foreign_option_index < opt_max)
- {
- char name[32];
- snprintf(name, sizeof(name), "foreign_option_%d", opt_max--);
- setenv_del(es, name);
- }
-}
#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
#ifndef ENABLE_SMALL
@@ -3616,6 +3475,207 @@
}
}
+#if defined(_WIN32) || defined(TARGET_ANDROID)
+/**
+ * @brief Postprocess DNS related settings
+ *
+ * Discard existing TUN/TAP DNS options and override them with --dns
+ * values for Android. If there are no --dns servers leave things alone.
+ *
+ * @param o pointer to the options struct
+ */
+static void
+tuntap_options_postprocess_dns(struct options *o)
+{
+ struct dns_options *dns = &o->dns_options;
+ if (!dns->servers)
+ {
+ return;
+ }
+
+ /* Remove DNS related --dhcp-options */
+ struct tuntap_options *tt = &o->tuntap_options;
+ tt->dns_len = 0;
+ tt->dns6_len = 0;
+ tt->domain = NULL;
+ tt->domain_search_list_len = 0;
+
+#if defined(TARGET_ANDROID)
+ /* Set options from --dns config */
+ const struct dns_domain *d = dns->search_domains;
+ while (d && tt->domain_search_list_len + 1 < N_SEARCH_LIST_LEN)
+ {
+ tt->domain_search_list[tt->domain_search_list_len++] = d->name;
+ d = d->next;
+ }
+ if (d)
+ {
+ msg(M_WARN, "WARNING: couldn't copy all --dns search-domains to TUN/TAP");
+ }
+
+ const struct dns_server *s = dns->servers;
+ while (s)
+ {
+ bool non_standard_server_port = false;
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].port && s->addr[i].port != 53)
+ {
+ non_standard_server_port = true;
+ break;
+ }
+ }
+ if ((s->transport && s->transport != DNS_TRANSPORT_PLAIN)
+ || (s->dnssec && s->dnssec != DNS_SECURITY_NO)
+ || non_standard_server_port)
+ {
+ /* Skip servers requiring unsupported config to be set */
+ s = s->next;
+ }
+ else
+ {
+ bool overflow = false;
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].family == AF_INET && tt->dns_len + 1 < N_DHCP_ADDR)
+ {
+ tt->dns[tt->dns_len++] = s->addr[i].in.a4.s_addr;
+ }
+ else if (tt->dns6_len + 1 < N_DHCP_ADDR)
+ {
+ tt->dns6[tt->dns6_len] = s->addr[i].in.a6;
+ }
+ else
+ {
+ overflow = true;
+ }
+ }
+ if (overflow)
+ {
+ msg(M_WARN, "WARNING: couldn't copy all --dns server addresses to TUN/TAP");
+ }
+ return;
+ }
+ }
+#endif /* if defined(TARGET_ANDROID) */
+}
+
+#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+
+/**
+ * @brief Postprocess DNS related settings in the env_set
+ *
+ * Discard existing --dhcp-options and set values from --dns
+ * instead. If there are no --dns servers leave things alone.
+ *
+ * @param o pointer to the options struct
+ * @param es env set to modify potentially
+ */
+static void
+foreign_options_postprocess_dns(struct options *o, struct env_set *es)
+{
+ struct dns_options *dns = &o->dns_options;
+ if (!dns->servers)
+ {
+ return;
+ }
+
+ /* Clean up env from --dhcp-option DNS config */
+ struct gc_arena gc = gc_new();
+ struct buffer name = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+ struct buffer value = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+
+ const int fo_count = o->foreign_option_index;
+ o->foreign_option_index = 0;
+
+ for (int i = 1; i <= fo_count; ++i)
+ {
+ buf_clear(&name);
+ buf_printf(&name, "foreign_option_%d", i);
+ const char *env_str = env_set_get(es, BSTR(&name));
+ const char *item_val = strchr(env_str, '=') + 1;
+ buf_clear(&value);
+ buf_printf(&value, "%s", item_val);
+
+ /* Remove foreign option item from env set */
+ env_set_del(es, BSTR(&name));
+
+ item_val = BSTR(&value);
+ if (strncmp(item_val, "dhcp-option ", 12) != 0
+ || (strncmp(item_val + 12, "ADAPTER-DOMAIN-SUFFIX ", 22) != 0
+ && strncmp(item_val + 12, "DOMAIN-SEARCH ", 14) != 0
+ && strncmp(item_val + 12, "DOMAIN ", 7) != 0
+ && strncmp(item_val + 12, "DNS6 ", 5) != 0
+ && strncmp(item_val + 12, "DNS ", 4) != 0))
+ {
+ /* Re-set the item with potentially updated name */
+ buf_clear(&name);
+ buf_printf(&name, "foreign_option_%d", ++o->foreign_option_index);
+ setenv_str(es, BSTR(&name), BSTR(&value));
+ }
+ }
+
+ /* Set foreign option env vars from --dns config */
+ if (!o->up_script || dns->user_defined_updown)
+ {
+ /* No need to, when there is no --up script or a custom updown */
+ return;
+ }
+
+ const char *p[] = { "dhcp-option", NULL, NULL };
+ size_t p_len = sizeof(p) / sizeof(p[0]);
+
+ p[1] = "DOMAIN";
+ const struct dns_domain *d = dns->search_domains;
+ while (d)
+ {
+ p[2] = d->name;
+ setenv_foreign_option(o, (const char **)p, p_len, es);
+ d = d->next;
+ }
+
+ const struct dns_server *s = dns->servers;
+ while (s)
+ {
+ bool non_standard_server_port = false;
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].port && s->addr[i].port != 53)
+ {
+ non_standard_server_port = true;
+ break;
+ }
+ }
+ if ((s->transport && s->transport != DNS_TRANSPORT_PLAIN)
+ || (s->dnssec && s->dnssec != DNS_SECURITY_NO)
+ || non_standard_server_port)
+ {
+ /* Skip servers requiring unsupported config to be set */
+ s = s->next;
+ }
+ else
+ {
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].family == AF_INET)
+ {
+ p[1] = "DNS";
+ p[2] = print_in_addr_t(s->addr[i].in.a4.s_addr, IA_NET_ORDER, &gc);
+ }
+ else
+ {
+ p[1] = "DNS6";
+ p[2] = print_in6_addr(s->addr[i].in.a6, 0, &gc);
+ }
+ setenv_foreign_option(o, (const char **)p, p_len, es);
+ }
+ break;
+ }
+ }
+ gc_free(&gc);
+}
+#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+
static void
options_postprocess_mutate(struct options *o, struct env_set *es)
{
@@ -3801,9 +3861,9 @@
else
{
#if defined(_WIN32) || defined(TARGET_ANDROID)
- tuntap_options_copy_dns(o);
+ tuntap_options_postprocess_dns(o);
#else
- foreign_options_copy_dns(o, es);
+ foreign_options_postprocess_dns(o, es);
#endif
}
if (o->auth_token_generate && !o->auth_token_renewal)
@@ -4186,9 +4246,9 @@
{
dns_options_postprocess_pull(&o->dns_options);
#if defined(_WIN32) || defined(TARGET_ANDROID)
- tuntap_options_copy_dns(o);
+ tuntap_options_postprocess_dns(o);
#else
- foreign_options_copy_dns(o, es);
+ foreign_options_postprocess_dns(o, es);
#endif
}
return success;
@@ -8089,19 +8149,29 @@
goto err;
}
#ifdef ENABLE_DNS_UPDOWN
+ struct dns_options *dns = &options->dns_options;
if (streq(p[1], "disable"))
{
- options->dns_options.updown = NULL;
+ dns->updown = NULL;
+ dns->user_defined_updown = false;
+ }
+ else if (streq(p[1], "force"))
+ {
+ /* force dns-updown run, even if a --up script is defined */
+ if (dns->updown)
+ {
+ dns->user_defined_updown = true;
+ }
}
else
{
- if (options->dns_options.user_defined_updown == false)
+ if (dns->user_defined_updown == false)
{
/* Unset the default command to prevent warnings */
- options->dns_options.updown = NULL;
+ dns->updown = NULL;
}
- set_user_script(options, &options->dns_options.updown, p[1], p[0], false);
- options->dns_options.user_defined_updown = true;
+ set_user_script(options, &dns->updown, p[1], p[0], false);
+ dns->user_defined_updown = true;
}
#endif /* ENABLE_DNS_UPDOWN */
}
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/904?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: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Gerrit-Change-Number: 904
Gerrit-PatchSet: 9
Gerrit-Owner: d12fk <he...@op...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: d12fk <he...@op...>
Gerrit-MessageType: newpatchset
|
|
From: d12fk (C. Review) <ge...@op...> - 2025-03-20 12:10:28
|
Attention is currently required from: flichtenheld, plaisthos. d12fk has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/904?usp=email ) Change subject: dns: deal with --dhcp-options when --dns is active ...................................................................... Patch Set 9: (1 comment) File src/openvpn/options.c: http://gerrit.openvpn.net/c/openvpn/+/904/comment/f01dbf32_e7fe61de : PS8, Line 3511: ) > `error: use of undeclared identifier 'domain'` Done -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/904?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: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a Gerrit-Change-Number: 904 Gerrit-PatchSet: 9 Gerrit-Owner: d12fk <he...@op...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-Comment-Date: Thu, 20 Mar 2025 12:10:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: flichtenheld <fr...@li...> Gerrit-MessageType: comment |
|
From: d12fk (C. Review) <ge...@op...> - 2025-03-31 07:52:34
|
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/+/904?usp=email
to look at the new patch set (#12).
Change subject: dns: deal with --dhcp-options when --dns is active
......................................................................
dns: deal with --dhcp-options when --dns is active
Since --dns settings overrule DNS related --dhcp-options,
remove the latter when values were defined via --dns.
To stay as backward compatible as possible, we add foreign_options to
the script hook environment from the --dns values when a --up script
is defined. In that case the default --dns-updown is not run, even
when --dns values are present, to prevent double DNS configuration.
This way an existing --up script that deals with DNS can run, without
the immediate need to change after an openvpn upgrade and a server
pushing --dns options.
If you specify a custom --dns-updown, or force running the default
dns-updown that comes with openvpn, those compat env vars are not set
for --up scripts and the dns-updown command is run, even when there's
an --up script present.
Since Android uses the DNS values from tuntap_options, we always
override those with --dns stuff unconditionally. Also on Windows when
--ip-win32 is dynamic or adaptive, since DHCP relies on these as well.
Change-Id: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Signed-off-by: Heiko Hund <he...@is...>
---
M src/openvpn/dns.c
M src/openvpn/dns.h
M src/openvpn/options.c
3 files changed, 311 insertions(+), 186 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/04/904/12
diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c
index 221e9a9..9927961 100644
--- a/src/openvpn/dns.c
+++ b/src/openvpn/dns.c
@@ -703,7 +703,8 @@
static void
run_up_down_command(bool up, struct options *o, const struct tuntap *tt, struct dns_updown_runner_info *updown_runner)
{
- if (!o->dns_options.updown)
+ struct dns_options *dns = &o->dns_options;
+ if (!dns->updown || (o->up_script && !dns->user_set_updown))
{
return;
}
@@ -713,7 +714,7 @@
if (!updown_runner->required)
{
/* Run dns updown directly */
- status = do_run_up_down_command(up, NULL, &o->dns_options, tt);
+ status = do_run_up_down_command(up, NULL, dns, tt);
}
else
{
diff --git a/src/openvpn/dns.h b/src/openvpn/dns.h
index c56d603..8e3556d 100644
--- a/src/openvpn/dns.h
+++ b/src/openvpn/dns.h
@@ -76,7 +76,28 @@
#endif
};
+#ifndef N_DHCP_ADDR
+#define N_DHCP_ADDR 4
+#endif
+
+#ifndef N_SEARCH_LIST_LEN
+#define N_SEARCH_LIST_LEN 10
+#endif
+
+struct dhcp_options {
+ in_addr_t dns[N_DHCP_ADDR];
+ int dns_len;
+
+ struct in6_addr dns6[N_DHCP_ADDR];
+ int dns6_len;
+
+ const char *domain;
+ const char *domain_search_list[N_SEARCH_LIST_LEN];
+ int domain_search_list_len;
+};
+
struct dns_options {
+ struct dhcp_options from_dhcp;
struct dns_domain *search_domains;
struct dns_server *servers_prepull;
struct dns_server *servers;
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index e2e4835..0131ef4 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1333,7 +1333,6 @@
#endif /* ifndef ENABLE_SMALL */
#endif /* ifdef _WIN32 */
-#if defined(_WIN32) || defined(TARGET_ANDROID)
static void
dhcp_option_dns6_parse(const char *parm, struct in6_addr *dns6_list, int *len, int msglevel)
{
@@ -1376,150 +1375,6 @@
}
}
-/*
- * If DNS options are set use these for TUN/TAP options as well.
- * Applies to DNS, DNS6 and DOMAIN-SEARCH.
- * Existing options will be discarded.
- */
-static void
-tuntap_options_copy_dns(struct options *o)
-{
- struct tuntap_options *tt = &o->tuntap_options;
- struct dns_options *dns = &o->dns_options;
-
- if (dns->search_domains)
- {
- tt->domain_search_list_len = 0;
- const struct dns_domain *domain = dns->search_domains;
- while (domain && tt->domain_search_list_len < N_SEARCH_LIST_LEN)
- {
- tt->domain_search_list[tt->domain_search_list_len++] = domain->name;
- domain = domain->next;
- }
- if (domain)
- {
- msg(M_WARN, "WARNING: couldn't copy all --dns search-domains to --dhcp-option");
- }
- tt->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
- }
-
- if (dns->servers)
- {
- tt->dns_len = 0;
- tt->dns6_len = 0;
- bool overflow = false;
- const struct dns_server *server = dns->servers;
- while (server)
- {
- for (int i = 0; i < server->addr_count; ++i)
- {
- if (server->addr[i].family == AF_INET)
- {
- if (tt->dns_len >= N_DHCP_ADDR)
- {
- overflow = true;
- continue;
- }
- tt->dns[tt->dns_len++] = ntohl(server->addr[i].in.a4.s_addr);
- }
- else
- {
- if (tt->dns6_len >= N_DHCP_ADDR)
- {
- overflow = true;
- continue;
- }
- tt->dns6[tt->dns6_len++] = server->addr[i].in.a6;
- }
- }
- server = server->next;
- }
- if (overflow)
- {
- msg(M_WARN, "WARNING: couldn't copy all --dns server addresses to --dhcp-option");
- }
- tt->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
- }
-}
-#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
-static void
-foreign_options_copy_dns(struct options *o, struct env_set *es)
-{
- const struct dns_domain *domain = o->dns_options.search_domains;
- const struct dns_server *server = o->dns_options.servers;
- if (!domain && !server)
- {
- return;
- }
-
- /* reset the index since we're starting all over again */
- int opt_max = o->foreign_option_index;
- o->foreign_option_index = 0;
-
- for (int i = 1; i <= opt_max; ++i)
- {
- char name[32];
- snprintf(name, sizeof(name), "foreign_option_%d", i);
-
- const char *env_str = env_set_get(es, name);
- const char *value = strchr(env_str, '=') + 1;
- if ((domain && strstr(value, "dhcp-option DOMAIN-SEARCH") == value)
- || (server && strstr(value, "dhcp-option DNS") == value))
- {
- setenv_del(es, name);
- }
- else
- {
- setenv_foreign_option(o, &value, 1, es);
- }
- }
-
- struct gc_arena gc = gc_new();
-
- while (server)
- {
- for (size_t i = 0; i < server->addr_count; ++i)
- {
- if (server->addr[i].family == AF_INET)
- {
- const char *argv[] = {
- "dhcp-option",
- "DNS",
- print_in_addr_t(server->addr[i].in.a4.s_addr, 0, &gc)
- };
- setenv_foreign_option(o, argv, 3, es);
- }
- else
- {
- const char *argv[] = {
- "dhcp-option",
- "DNS6",
- print_in6_addr(server->addr[i].in.a6, 0, &gc)
- };
- setenv_foreign_option(o, argv, 3, es);
- }
- }
- server = server->next;
- }
- while (domain)
- {
- const char *argv[] = { "dhcp-option", "DOMAIN-SEARCH", domain->name };
- setenv_foreign_option(o, argv, 3, es);
- domain = domain->next;
- }
-
- gc_free(&gc);
-
- /* remove old leftover entries */
- while (o->foreign_option_index < opt_max)
- {
- char name[32];
- snprintf(name, sizeof(name), "foreign_option_%d", opt_max--);
- setenv_del(es, name);
- }
-}
-#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
-
#ifndef ENABLE_SMALL
static const char *
print_vlan_accept(enum vlan_acceptable_frames mode)
@@ -3617,6 +3472,256 @@
}
}
+#if defined(_WIN32) || defined(TARGET_ANDROID)
+/**
+ * @brief Postprocess DNS related settings
+ *
+ * Set TUN/TAP DNS options with values from either --dns
+ * or --dhcp-option.
+ *
+ * @param o pointer to the options struct
+ */
+static void
+tuntap_options_postprocess_dns(struct options *o)
+{
+ struct dns_options *dns = &o->dns_options;
+ struct tuntap_options *tt = &o->tuntap_options;
+ if (!dns->servers)
+ {
+ /* Copy --dhcp-options to tuntap_options */
+ struct dhcp_options *dhcp = &dns->from_dhcp;
+ assert(sizeof(dhcp->dns) == sizeof(tt->dns));
+ assert(sizeof(dhcp->dns6) == sizeof(tt->dns6));
+ assert(sizeof(dhcp->domain_search_list) == sizeof(tt->domain_search_list));
+
+ tt->domain = dhcp->domain;
+ tt->dns_len = dhcp->dns_len;
+ tt->dns6_len = dhcp->dns6_len;
+ tt->domain_search_list_len = dhcp->domain_search_list_len;
+
+ memcpy(tt->dns, dhcp->dns, sizeof(tt->dns));
+ memcpy(tt->dns6, dhcp->dns6, sizeof(tt->dns6));
+ memcpy(tt->domain_search_list, dhcp->domain_search_list, sizeof(tt->domain_search_list));
+
+ return;
+ }
+
+#if defined(_WIN32)
+ if (tt->ip_win32_type != IPW32_SET_DHCP_MASQ && tt->ip_win32_type != IPW32_SET_ADAPTIVE)
+ {
+ return; /* Not in DHCP mode */
+ }
+#endif /* if defined(_WIN32) */
+
+ /* Copy --dns options to tuntap_options */
+ const struct dns_domain *d = dns->search_domains;
+ while (d && tt->domain_search_list_len + 1 < N_SEARCH_LIST_LEN)
+ {
+ tt->domain_search_list[tt->domain_search_list_len++] = d->name;
+ d = d->next;
+ }
+ if (d)
+ {
+ msg(M_WARN, "WARNING: couldn't copy all --dns search-domains to TUN/TAP");
+ }
+
+ const struct dns_server *s = dns->servers;
+ while (s)
+ {
+ bool non_standard_server_port = false;
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].port && s->addr[i].port != 53)
+ {
+ non_standard_server_port = true;
+ break;
+ }
+ }
+ if ((s->transport && s->transport != DNS_TRANSPORT_PLAIN)
+ || (s->dnssec && s->dnssec != DNS_SECURITY_NO)
+ || non_standard_server_port)
+ {
+ /* Skip servers requiring unsupported config to be set */
+ s = s->next;
+ }
+ else
+ {
+ bool overflow = false;
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].family == AF_INET && tt->dns_len + 1 < N_DHCP_ADDR)
+ {
+ tt->dns[tt->dns_len++] = s->addr[i].in.a4.s_addr;
+ }
+ else if (tt->dns6_len + 1 < N_DHCP_ADDR)
+ {
+ tt->dns6[tt->dns6_len] = s->addr[i].in.a6;
+ }
+ else
+ {
+ overflow = true;
+ }
+ }
+ if (overflow)
+ {
+ msg(M_WARN, "WARNING: couldn't copy all --dns server addresses to TUN/TAP");
+ }
+ return;
+ }
+ }
+}
+
+#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+
+/**
+ * @brief Postprocess DNS related settings
+ *
+ * Discard existing --dhcp-options from the env if needed and possibly
+ * replace them with values from --dns. If no --dns servers are set copy
+ * the --dhcp-option values over for --dns-updown runs.
+ *
+ * @param o pointer to the options struct
+ * @param es env set to modify potentially
+ */
+static void
+dhcp_options_postprocess_dns(struct options *o, struct env_set *es)
+{
+ struct gc_arena gc = gc_new();
+ struct dns_options *dns = &o->dns_options;
+
+ if (dns->servers || dns->user_set_updown)
+ {
+ /* Clean up env from --dhcp-option DNS config */
+ struct buffer name = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+ struct buffer value = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+
+ const int fo_count = o->foreign_option_index;
+ o->foreign_option_index = 0;
+
+ for (int i = 1; i <= fo_count; ++i)
+ {
+ buf_clear(&name);
+ buf_printf(&name, "foreign_option_%d", i);
+ const char *env_str = env_set_get(es, BSTR(&name));
+ const char *item_val = strchr(env_str, '=') + 1;
+ buf_clear(&value);
+ buf_printf(&value, "%s", item_val);
+
+ /* Remove foreign option item from env set */
+ env_set_del(es, BSTR(&name));
+
+ item_val = BSTR(&value);
+ if (strncmp(item_val, "dhcp-option ", 12) != 0
+ || (strncmp(item_val + 12, "ADAPTER-DOMAIN-SUFFIX ", 22) != 0
+ && strncmp(item_val + 12, "DOMAIN-SEARCH ", 14) != 0
+ && strncmp(item_val + 12, "DOMAIN ", 7) != 0
+ && strncmp(item_val + 12, "DNS6 ", 5) != 0
+ && strncmp(item_val + 12, "DNS ", 4) != 0))
+ {
+ /* Re-set the item with potentially updated name */
+ buf_clear(&name);
+ buf_printf(&name, "foreign_option_%d", ++o->foreign_option_index);
+ setenv_str(es, BSTR(&name), BSTR(&value));
+ }
+ }
+ }
+
+ if (!dns->servers)
+ {
+ /* Copy --dhcp-options to dns_options */
+ struct dhcp_options *dhcp = &dns->from_dhcp;
+
+ if (dhcp->dns_len || dhcp->dns6_len)
+ {
+ struct dns_domain **entry = &dns->search_domains;
+ ALLOC_OBJ_CLEAR_GC(*entry, struct dns_domain, &dns->gc);
+ struct dns_domain *new = *entry;
+ new->name = dhcp->domain;
+ entry = &new->next;
+
+ for (size_t i = 0; i < dhcp->domain_search_list_len; ++i)
+ {
+ ALLOC_OBJ_CLEAR_GC(*entry, struct dns_domain, &dns->gc);
+ struct dns_domain *new = *entry;
+ new->name = dhcp->domain_search_list[i];
+ entry = &new->next;
+ }
+
+ struct dns_server *server = dns_server_get(&dns->servers, 0, &dns->gc);
+ const size_t max_addrs = SIZE(server->addr);
+ for (size_t i = 0; i < dhcp->dns_len && server->addr_count < max_addrs; ++i)
+ {
+ server->addr[server->addr_count].in.a4.s_addr = htonl(dhcp->dns[i]);
+ server->addr[server->addr_count].family = AF_INET;
+ server->addr_count += 1;
+ }
+ for (size_t i = 0; i < dhcp->dns6_len && server->addr_count < max_addrs; ++i)
+ {
+ server->addr[server->addr_count].in.a6 = dhcp->dns6[i];
+ server->addr[server->addr_count].family = AF_INET6;
+ server->addr_count += 1;
+ }
+ }
+ }
+ else if (o->up_script && !dns->user_set_updown)
+ {
+ /* Set foreign option env vars from --dns config */
+ const char *p[] = { "dhcp-option", NULL, NULL };
+ size_t p_len = sizeof(p) / sizeof(p[0]);
+
+ p[1] = "DOMAIN";
+ const struct dns_domain *d = dns->search_domains;
+ while (d)
+ {
+ p[2] = d->name;
+ setenv_foreign_option(o, (const char **)p, p_len, es);
+ d = d->next;
+ }
+
+ const struct dns_server *s = dns->servers;
+ while (s)
+ {
+ bool non_standard_server_port = false;
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].port && s->addr[i].port != 53)
+ {
+ non_standard_server_port = true;
+ break;
+ }
+ }
+ if ((s->transport && s->transport != DNS_TRANSPORT_PLAIN)
+ || (s->dnssec && s->dnssec != DNS_SECURITY_NO)
+ || non_standard_server_port)
+ {
+ /* Skip servers requiring unsupported config to be set */
+ s = s->next;
+ }
+ else
+ {
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].family == AF_INET)
+ {
+ p[1] = "DNS";
+ p[2] = print_in_addr_t(s->addr[i].in.a4.s_addr, IA_NET_ORDER, &gc);
+ }
+ else
+ {
+ p[1] = "DNS6";
+ p[2] = print_in6_addr(s->addr[i].in.a6, 0, &gc);
+ }
+ setenv_foreign_option(o, (const char **)p, p_len, es);
+ }
+ break;
+ }
+ }
+ }
+
+ gc_free(&gc);
+}
+#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+
static void
options_postprocess_mutate(struct options *o, struct env_set *es)
{
@@ -3802,9 +3907,9 @@
else
{
#if defined(_WIN32) || defined(TARGET_ANDROID)
- tuntap_options_copy_dns(o);
+ tuntap_options_postprocess_dns(o);
#else
- foreign_options_copy_dns(o, es);
+ dhcp_options_postprocess_dns(o, es);
#endif
}
if (o->auth_token_generate && !o->auth_token_renewal)
@@ -4187,9 +4292,9 @@
{
dns_options_postprocess_pull(&o->dns_options);
#if defined(_WIN32) || defined(TARGET_ANDROID)
- tuntap_options_copy_dns(o);
+ tuntap_options_postprocess_dns(o);
#else
- foreign_options_copy_dns(o, es);
+ dhcp_options_postprocess_dns(o, es);
#endif
}
return success;
@@ -8210,19 +8315,42 @@
goto err;
}
}
-#if defined(_WIN32) || defined(TARGET_ANDROID)
else if (streq(p[0], "dhcp-option") && p[1])
{
- struct tuntap_options *o = &options->tuntap_options;
+ struct dhcp_options *dhcp = &options->dns_options.from_dhcp;
VERIFY_PERMISSION(OPT_P_DHCPDNS);
- if ((streq(p[1], "DOMAIN") || streq(p[1], "ADAPTER_DOMAIN_SUFFIX"))
- && p[2] && !p[3])
+ if ((streq(p[1], "DOMAIN") || streq(p[1], "ADAPTER_DOMAIN_SUFFIX")) && p[2] && !p[3])
{
- o->domain = p[2];
- o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
+ dhcp->domain = p[2];
}
- else if (streq(p[1], "NBS") && p[2] && !p[3])
+ else if (streq(p[1], "DOMAIN-SEARCH") && p[2] && !p[3])
+ {
+ if (dhcp->domain_search_list_len < N_SEARCH_LIST_LEN)
+ {
+ dhcp->domain_search_list[dhcp->domain_search_list_len++] = p[2];
+ }
+ else
+ {
+ msg(msglevel, "--dhcp-option %s: maximum of %d search entries can be specified",
+ p[1], N_SEARCH_LIST_LEN);
+ }
+ }
+ else if ((streq(p[1], "DNS") || streq(p[1], "DNS6")) && p[2] && !p[3]
+ && (!strstr(p[2], ":") || ipv6_addr_safe(p[2])))
+ {
+ if (strstr(p[2], ":"))
+ {
+ dhcp_option_dns6_parse(p[2], dhcp->dns6, &dhcp->dns6_len, msglevel);
+ }
+ else
+ {
+ dhcp_option_address_parse("DNS", p[2], dhcp->dns, &dhcp->dns_len, msglevel);
+ }
+ }
+#if defined(_WIN32) || defined(TARGET_ANDROID)
+ struct tuntap_options *o = &options->tuntap_options;
+ if (streq(p[1], "NBS") && p[2] && !p[3])
{
o->netbios_scope = p[2];
o->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
@@ -8239,23 +8367,9 @@
o->netbios_node_type = t;
o->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
}
- else if ((streq(p[1], "DNS") || streq(p[1], "DNS6")) && p[2] && !p[3]
- && (!strstr(p[2], ":") || ipv6_addr_safe(p[2])))
- {
- if (strstr(p[2], ":"))
- {
- dhcp_option_dns6_parse(p[2], o->dns6, &o->dns6_len, msglevel);
- }
- else
- {
- dhcp_option_address_parse("DNS", p[2], o->dns, &o->dns_len, msglevel);
- o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
- }
- }
else if (streq(p[1], "WINS") && p[2] && !p[3])
{
dhcp_option_address_parse("WINS", p[2], o->wins, &o->wins_len, msglevel);
- o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
}
else if (streq(p[1], "NTP") && p[2] && !p[3])
{
@@ -8267,19 +8381,6 @@
dhcp_option_address_parse("NBDD", p[2], o->nbdd, &o->nbdd_len, msglevel);
o->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
}
- else if (streq(p[1], "DOMAIN-SEARCH") && p[2] && !p[3])
- {
- if (o->domain_search_list_len < N_SEARCH_LIST_LEN)
- {
- o->domain_search_list[o->domain_search_list_len++] = p[2];
- }
- else
- {
- msg(msglevel, "--dhcp-option %s: maximum of %d search entries can be specified",
- p[1], N_SEARCH_LIST_LEN);
- }
- o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
- }
else if (streq(p[1], "DISABLE-NBT") && !p[2])
{
o->disable_nbt = 1;
@@ -8297,8 +8398,10 @@
msg(msglevel, "--dhcp-option: unknown option type '%s' or missing or unknown parameter", p[1]);
goto err;
}
- }
+#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+ setenv_foreign_option(options, (const char **)p, 3, es);
#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+ }
#ifdef _WIN32
else if (streq(p[0], "show-adapters") && !p[1])
{
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/904?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: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Gerrit-Change-Number: 904
Gerrit-PatchSet: 12
Gerrit-Owner: d12fk <he...@op...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-MessageType: newpatchset
|
|
From: stipa (C. Review) <ge...@op...> - 2025-04-23 09:44:12
|
Attention is currently required from: d12fk, flichtenheld, plaisthos. stipa has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/904?usp=email ) Change subject: dns: deal with --dhcp-options when --dns is active ...................................................................... Patch Set 14: Code-Review-1 (1 comment) File src/openvpn/options.c: http://gerrit.openvpn.net/c/openvpn/+/904/comment/b309b0cc_f864d13c : PS14, Line 3504: memcpy(tt->domain_search_list, dhcp->domain_search_list, sizeof(tt->domain_search_list)); C:\Users\lev\Projects\openvpn-build\src\openvpn\src\openvpn\options.c(3504,18): error C2220: the following warning is treated as an error C:\Users\lev\Projects\openvpn-build\src\openvpn\src\openvpn\options.c(3504,18): warning C4090: 'function': different 'const' qualifiers -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/904?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: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a Gerrit-Change-Number: 904 Gerrit-PatchSet: 14 Gerrit-Owner: d12fk <he...@op...> 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: d12fk <he...@op...> Gerrit-Comment-Date: Wed, 23 Apr 2025 09:43:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment |
|
From: stipa (C. Review) <ge...@op...> - 2025-04-23 10:52:34
|
Attention is currently required from: d12fk, flichtenheld, plaisthos. stipa has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/904?usp=email ) Change subject: dns: deal with --dhcp-options when --dns is active ...................................................................... Patch Set 14: (1 comment) File src/openvpn/options.c: http://gerrit.openvpn.net/c/openvpn/+/904/comment/af9bbdf4_cef7db0a : PS14, Line 8350: } is "else" missing here? I have "--dhcp-option DNS 8.8.4.4" pushed by server which got applied, but I also got a warning "Options error: --dhcp-option: unknown option type 'DNS' or missing or unknown parameter". -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/904?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: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a Gerrit-Change-Number: 904 Gerrit-PatchSet: 14 Gerrit-Owner: d12fk <he...@op...> 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: d12fk <he...@op...> Gerrit-Comment-Date: Wed, 23 Apr 2025 10:52:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment |
|
From: d12fk (C. Review) <ge...@op...> - 2025-04-28 06:12:26
|
Attention is currently required from: flichtenheld, plaisthos, stipa. d12fk has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/904?usp=email ) Change subject: dns: deal with --dhcp-options when --dns is active ...................................................................... Patch Set 14: (2 comments) File src/openvpn/options.c: http://gerrit.openvpn.net/c/openvpn/+/904/comment/d8e761d9_4194af85 : PS14, Line 3504: memcpy(tt->domain_search_list, dhcp->domain_search_list, sizeof(tt->domain_search_list)); > C:\Users\lev\Projects\openvpn-build\src\openvpn\src\openvpn\options. […] Done http://gerrit.openvpn.net/c/openvpn/+/904/comment/842e2bc7_5ade9b3b : PS14, Line 8350: } > is "else" missing here? I have "--dhcp-option DNS 8.8.4. […] Done -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/904?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: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a Gerrit-Change-Number: 904 Gerrit-PatchSet: 14 Gerrit-Owner: d12fk <he...@op...> 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: stipa <lst...@gm...> Gerrit-Comment-Date: Mon, 28 Apr 2025 06:12:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: stipa <lst...@gm...> Gerrit-MessageType: comment |
|
From: d12fk (C. Review) <ge...@op...> - 2025-04-28 06:13:25
|
Attention is currently required from: flichtenheld, plaisthos, stipa.
Hello flichtenheld, plaisthos, stipa,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/904?usp=email
to look at the new patch set (#15).
The following approvals got outdated and were removed:
Code-Review-1 by stipa
Change subject: dns: deal with --dhcp-options when --dns is active
......................................................................
dns: deal with --dhcp-options when --dns is active
Since --dns settings overrule DNS related --dhcp-options,
remove the latter when values were defined via --dns.
To stay as backward compatible as possible, we add foreign_options to
the script hook environment from the --dns values when a --up script
is defined. In that case the default --dns-updown is not run, even
when --dns values are present, to prevent double DNS configuration.
This way an existing --up script that deals with DNS can run, without
the immediate need to change after an openvpn upgrade and a server
pushing --dns options.
If you specify a custom --dns-updown, or force running the default
dns-updown that comes with openvpn, those compat env vars are not set
for --up scripts and the dns-updown command is run, even when there's
an --up script present.
Since Android uses the DNS values from tuntap_options, we always
override those with --dns stuff unconditionally. Also on Windows when
--ip-win32 is dynamic or adaptive, since DHCP relies on these as well.
Change-Id: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Signed-off-by: Heiko Hund <he...@is...>
---
M src/openvpn/dns.c
M src/openvpn/dns.h
M src/openvpn/options.c
3 files changed, 315 insertions(+), 184 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/04/904/15
diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c
index 221e9a9..9927961 100644
--- a/src/openvpn/dns.c
+++ b/src/openvpn/dns.c
@@ -703,7 +703,8 @@
static void
run_up_down_command(bool up, struct options *o, const struct tuntap *tt, struct dns_updown_runner_info *updown_runner)
{
- if (!o->dns_options.updown)
+ struct dns_options *dns = &o->dns_options;
+ if (!dns->updown || (o->up_script && !dns->user_set_updown))
{
return;
}
@@ -713,7 +714,7 @@
if (!updown_runner->required)
{
/* Run dns updown directly */
- status = do_run_up_down_command(up, NULL, &o->dns_options, tt);
+ status = do_run_up_down_command(up, NULL, dns, tt);
}
else
{
diff --git a/src/openvpn/dns.h b/src/openvpn/dns.h
index c56d603..8e3556d 100644
--- a/src/openvpn/dns.h
+++ b/src/openvpn/dns.h
@@ -76,7 +76,28 @@
#endif
};
+#ifndef N_DHCP_ADDR
+#define N_DHCP_ADDR 4
+#endif
+
+#ifndef N_SEARCH_LIST_LEN
+#define N_SEARCH_LIST_LEN 10
+#endif
+
+struct dhcp_options {
+ in_addr_t dns[N_DHCP_ADDR];
+ int dns_len;
+
+ struct in6_addr dns6[N_DHCP_ADDR];
+ int dns6_len;
+
+ const char *domain;
+ const char *domain_search_list[N_SEARCH_LIST_LEN];
+ int domain_search_list_len;
+};
+
struct dns_options {
+ struct dhcp_options from_dhcp;
struct dns_domain *search_domains;
struct dns_server *servers_prepull;
struct dns_server *servers;
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 55d31f4..61da4d9c 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1333,7 +1333,6 @@
#endif /* ifndef ENABLE_SMALL */
#endif /* ifdef _WIN32 */
-#if defined(_WIN32) || defined(TARGET_ANDROID)
static void
dhcp_option_dns6_parse(const char *parm, struct in6_addr *dns6_list, int *len, int msglevel)
{
@@ -1376,150 +1375,6 @@
}
}
-/*
- * If DNS options are set use these for TUN/TAP options as well.
- * Applies to DNS, DNS6 and DOMAIN-SEARCH.
- * Existing options will be discarded.
- */
-static void
-tuntap_options_copy_dns(struct options *o)
-{
- struct tuntap_options *tt = &o->tuntap_options;
- struct dns_options *dns = &o->dns_options;
-
- if (dns->search_domains)
- {
- tt->domain_search_list_len = 0;
- const struct dns_domain *domain = dns->search_domains;
- while (domain && tt->domain_search_list_len < N_SEARCH_LIST_LEN)
- {
- tt->domain_search_list[tt->domain_search_list_len++] = domain->name;
- domain = domain->next;
- }
- if (domain)
- {
- msg(M_WARN, "WARNING: couldn't copy all --dns search-domains to --dhcp-option");
- }
- tt->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
- }
-
- if (dns->servers)
- {
- tt->dns_len = 0;
- tt->dns6_len = 0;
- bool overflow = false;
- const struct dns_server *server = dns->servers;
- while (server)
- {
- for (int i = 0; i < server->addr_count; ++i)
- {
- if (server->addr[i].family == AF_INET)
- {
- if (tt->dns_len >= N_DHCP_ADDR)
- {
- overflow = true;
- continue;
- }
- tt->dns[tt->dns_len++] = ntohl(server->addr[i].in.a4.s_addr);
- }
- else
- {
- if (tt->dns6_len >= N_DHCP_ADDR)
- {
- overflow = true;
- continue;
- }
- tt->dns6[tt->dns6_len++] = server->addr[i].in.a6;
- }
- }
- server = server->next;
- }
- if (overflow)
- {
- msg(M_WARN, "WARNING: couldn't copy all --dns server addresses to --dhcp-option");
- }
- tt->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
- }
-}
-#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
-static void
-foreign_options_copy_dns(struct options *o, struct env_set *es)
-{
- const struct dns_domain *domain = o->dns_options.search_domains;
- const struct dns_server *server = o->dns_options.servers;
- if (!domain && !server)
- {
- return;
- }
-
- /* reset the index since we're starting all over again */
- int opt_max = o->foreign_option_index;
- o->foreign_option_index = 0;
-
- for (int i = 1; i <= opt_max; ++i)
- {
- char name[32];
- snprintf(name, sizeof(name), "foreign_option_%d", i);
-
- const char *env_str = env_set_get(es, name);
- const char *value = strchr(env_str, '=') + 1;
- if ((domain && strstr(value, "dhcp-option DOMAIN-SEARCH") == value)
- || (server && strstr(value, "dhcp-option DNS") == value))
- {
- setenv_del(es, name);
- }
- else
- {
- setenv_foreign_option(o, &value, 1, es);
- }
- }
-
- struct gc_arena gc = gc_new();
-
- while (server)
- {
- for (size_t i = 0; i < server->addr_count; ++i)
- {
- if (server->addr[i].family == AF_INET)
- {
- const char *argv[] = {
- "dhcp-option",
- "DNS",
- print_in_addr_t(server->addr[i].in.a4.s_addr, 0, &gc)
- };
- setenv_foreign_option(o, argv, 3, es);
- }
- else
- {
- const char *argv[] = {
- "dhcp-option",
- "DNS6",
- print_in6_addr(server->addr[i].in.a6, 0, &gc)
- };
- setenv_foreign_option(o, argv, 3, es);
- }
- }
- server = server->next;
- }
- while (domain)
- {
- const char *argv[] = { "dhcp-option", "DOMAIN-SEARCH", domain->name };
- setenv_foreign_option(o, argv, 3, es);
- domain = domain->next;
- }
-
- gc_free(&gc);
-
- /* remove old leftover entries */
- while (o->foreign_option_index < opt_max)
- {
- char name[32];
- snprintf(name, sizeof(name), "foreign_option_%d", opt_max--);
- setenv_del(es, name);
- }
-}
-#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
-
#ifndef ENABLE_SMALL
static const char *
print_vlan_accept(enum vlan_acceptable_frames mode)
@@ -3617,6 +3472,260 @@
}
}
+#if defined(_WIN32) || defined(TARGET_ANDROID)
+/**
+ * @brief Postprocess DNS related settings
+ *
+ * Set TUN/TAP DNS options with values from either --dns
+ * or --dhcp-option.
+ *
+ * @param o pointer to the options struct
+ */
+static void
+tuntap_options_postprocess_dns(struct options *o)
+{
+ struct dns_options *dns = &o->dns_options;
+ struct tuntap_options *tt = &o->tuntap_options;
+ if (!dns->servers)
+ {
+ /* Copy --dhcp-options to tuntap_options */
+ struct dhcp_options *dhcp = &dns->from_dhcp;
+ assert(sizeof(dhcp->dns) == sizeof(tt->dns));
+ assert(sizeof(dhcp->dns6) == sizeof(tt->dns6));
+ assert(sizeof(dhcp->domain_search_list) == sizeof(tt->domain_search_list));
+
+ tt->domain = dhcp->domain;
+ tt->dns_len = dhcp->dns_len;
+ tt->dns6_len = dhcp->dns6_len;
+
+ memcpy(tt->dns, dhcp->dns, sizeof(tt->dns));
+ memcpy(tt->dns6, dhcp->dns6, sizeof(tt->dns6));
+
+ tt->domain_search_list_len = dhcp->domain_search_list_len;
+ for (size_t i = 0; i < SIZE(tt->domain_search_list); ++i)
+ {
+ tt->domain_search_list[i] = dhcp->domain_search_list[i];
+ }
+
+ return;
+ }
+
+#if defined(_WIN32)
+ if (tt->ip_win32_type != IPW32_SET_DHCP_MASQ && tt->ip_win32_type != IPW32_SET_ADAPTIVE)
+ {
+ return; /* Not in DHCP mode */
+ }
+#endif /* if defined(_WIN32) */
+
+ /* Copy --dns options to tuntap_options */
+ const struct dns_domain *d = dns->search_domains;
+ while (d && tt->domain_search_list_len + 1 < N_SEARCH_LIST_LEN)
+ {
+ tt->domain_search_list[tt->domain_search_list_len++] = d->name;
+ d = d->next;
+ }
+ if (d)
+ {
+ msg(M_WARN, "WARNING: couldn't copy all --dns search-domains to TUN/TAP");
+ }
+
+ const struct dns_server *s = dns->servers;
+ while (s)
+ {
+ bool non_standard_server_port = false;
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].port && s->addr[i].port != 53)
+ {
+ non_standard_server_port = true;
+ break;
+ }
+ }
+ if ((s->transport && s->transport != DNS_TRANSPORT_PLAIN)
+ || (s->dnssec && s->dnssec != DNS_SECURITY_NO)
+ || non_standard_server_port)
+ {
+ /* Skip servers requiring unsupported config to be set */
+ s = s->next;
+ }
+ else
+ {
+ bool overflow = false;
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].family == AF_INET && tt->dns_len + 1 < N_DHCP_ADDR)
+ {
+ tt->dns[tt->dns_len++] = s->addr[i].in.a4.s_addr;
+ }
+ else if (tt->dns6_len + 1 < N_DHCP_ADDR)
+ {
+ tt->dns6[tt->dns6_len] = s->addr[i].in.a6;
+ }
+ else
+ {
+ overflow = true;
+ }
+ }
+ if (overflow)
+ {
+ msg(M_WARN, "WARNING: couldn't copy all --dns server addresses to TUN/TAP");
+ }
+ return;
+ }
+ }
+}
+
+#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+
+/**
+ * @brief Postprocess DNS related settings
+ *
+ * Discard existing --dhcp-options from the env if needed and possibly
+ * replace them with values from --dns. If no --dns servers are set copy
+ * the --dhcp-option values over for --dns-updown runs.
+ *
+ * @param o pointer to the options struct
+ * @param es env set to modify potentially
+ */
+static void
+dhcp_options_postprocess_dns(struct options *o, struct env_set *es)
+{
+ struct gc_arena gc = gc_new();
+ struct dns_options *dns = &o->dns_options;
+
+ if (dns->servers || dns->user_set_updown)
+ {
+ /* Clean up env from --dhcp-option DNS config */
+ struct buffer name = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+ struct buffer value = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+
+ const int fo_count = o->foreign_option_index;
+ o->foreign_option_index = 0;
+
+ for (int i = 1; i <= fo_count; ++i)
+ {
+ buf_clear(&name);
+ buf_printf(&name, "foreign_option_%d", i);
+ const char *env_str = env_set_get(es, BSTR(&name));
+ const char *item_val = strchr(env_str, '=') + 1;
+ buf_clear(&value);
+ buf_printf(&value, "%s", item_val);
+
+ /* Remove foreign option item from env set */
+ env_set_del(es, BSTR(&name));
+
+ item_val = BSTR(&value);
+ if (strncmp(item_val, "dhcp-option ", 12) != 0
+ || (strncmp(item_val + 12, "ADAPTER-DOMAIN-SUFFIX ", 22) != 0
+ && strncmp(item_val + 12, "DOMAIN-SEARCH ", 14) != 0
+ && strncmp(item_val + 12, "DOMAIN ", 7) != 0
+ && strncmp(item_val + 12, "DNS6 ", 5) != 0
+ && strncmp(item_val + 12, "DNS ", 4) != 0))
+ {
+ /* Re-set the item with potentially updated name */
+ buf_clear(&name);
+ buf_printf(&name, "foreign_option_%d", ++o->foreign_option_index);
+ setenv_str(es, BSTR(&name), BSTR(&value));
+ }
+ }
+ }
+
+ if (!dns->servers)
+ {
+ /* Copy --dhcp-options to dns_options */
+ struct dhcp_options *dhcp = &dns->from_dhcp;
+
+ if (dhcp->dns_len || dhcp->dns6_len)
+ {
+ struct dns_domain **entry = &dns->search_domains;
+ ALLOC_OBJ_CLEAR_GC(*entry, struct dns_domain, &dns->gc);
+ struct dns_domain *new = *entry;
+ new->name = dhcp->domain;
+ entry = &new->next;
+
+ for (size_t i = 0; i < dhcp->domain_search_list_len; ++i)
+ {
+ ALLOC_OBJ_CLEAR_GC(*entry, struct dns_domain, &dns->gc);
+ struct dns_domain *new = *entry;
+ new->name = dhcp->domain_search_list[i];
+ entry = &new->next;
+ }
+
+ struct dns_server *server = dns_server_get(&dns->servers, 0, &dns->gc);
+ const size_t max_addrs = SIZE(server->addr);
+ for (size_t i = 0; i < dhcp->dns_len && server->addr_count < max_addrs; ++i)
+ {
+ server->addr[server->addr_count].in.a4.s_addr = htonl(dhcp->dns[i]);
+ server->addr[server->addr_count].family = AF_INET;
+ server->addr_count += 1;
+ }
+ for (size_t i = 0; i < dhcp->dns6_len && server->addr_count < max_addrs; ++i)
+ {
+ server->addr[server->addr_count].in.a6 = dhcp->dns6[i];
+ server->addr[server->addr_count].family = AF_INET6;
+ server->addr_count += 1;
+ }
+ }
+ }
+ else if (o->up_script && !dns->user_set_updown)
+ {
+ /* Set foreign option env vars from --dns config */
+ const char *p[] = { "dhcp-option", NULL, NULL };
+ size_t p_len = sizeof(p) / sizeof(p[0]);
+
+ p[1] = "DOMAIN";
+ const struct dns_domain *d = dns->search_domains;
+ while (d)
+ {
+ p[2] = d->name;
+ setenv_foreign_option(o, (const char **)p, p_len, es);
+ d = d->next;
+ }
+
+ const struct dns_server *s = dns->servers;
+ while (s)
+ {
+ bool non_standard_server_port = false;
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].port && s->addr[i].port != 53)
+ {
+ non_standard_server_port = true;
+ break;
+ }
+ }
+ if ((s->transport && s->transport != DNS_TRANSPORT_PLAIN)
+ || (s->dnssec && s->dnssec != DNS_SECURITY_NO)
+ || non_standard_server_port)
+ {
+ /* Skip servers requiring unsupported config to be set */
+ s = s->next;
+ }
+ else
+ {
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].family == AF_INET)
+ {
+ p[1] = "DNS";
+ p[2] = print_in_addr_t(s->addr[i].in.a4.s_addr, IA_NET_ORDER, &gc);
+ }
+ else
+ {
+ p[1] = "DNS6";
+ p[2] = print_in6_addr(s->addr[i].in.a6, 0, &gc);
+ }
+ setenv_foreign_option(o, (const char **)p, p_len, es);
+ }
+ break;
+ }
+ }
+ }
+
+ gc_free(&gc);
+}
+#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+
static void
options_postprocess_mutate(struct options *o, struct env_set *es)
{
@@ -3802,9 +3911,9 @@
else
{
#if defined(_WIN32) || defined(TARGET_ANDROID)
- tuntap_options_copy_dns(o);
+ tuntap_options_postprocess_dns(o);
#else
- foreign_options_copy_dns(o, es);
+ dhcp_options_postprocess_dns(o, es);
#endif
}
if (o->auth_token_generate && !o->auth_token_renewal)
@@ -4187,9 +4296,9 @@
{
dns_options_postprocess_pull(&o->dns_options);
#if defined(_WIN32) || defined(TARGET_ANDROID)
- tuntap_options_copy_dns(o);
+ tuntap_options_postprocess_dns(o);
#else
- foreign_options_copy_dns(o, es);
+ dhcp_options_postprocess_dns(o, es);
#endif
}
return success;
@@ -8210,18 +8319,43 @@
goto err;
}
}
-#if defined(_WIN32) || defined(TARGET_ANDROID)
else if (streq(p[0], "dhcp-option") && p[1])
{
+ struct dhcp_options *dhcp = &options->dns_options.from_dhcp;
+#if defined(_WIN32) || defined(TARGET_ANDROID)
struct tuntap_options *o = &options->tuntap_options;
+#endif
VERIFY_PERMISSION(OPT_P_DHCPDNS);
- if ((streq(p[1], "DOMAIN") || streq(p[1], "ADAPTER_DOMAIN_SUFFIX"))
- && p[2] && !p[3])
+ if ((streq(p[1], "DOMAIN") || streq(p[1], "ADAPTER_DOMAIN_SUFFIX")) && p[2] && !p[3])
{
- o->domain = p[2];
- o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
+ dhcp->domain = p[2];
}
+ else if (streq(p[1], "DOMAIN-SEARCH") && p[2] && !p[3])
+ {
+ if (dhcp->domain_search_list_len < N_SEARCH_LIST_LEN)
+ {
+ dhcp->domain_search_list[dhcp->domain_search_list_len++] = p[2];
+ }
+ else
+ {
+ msg(msglevel, "--dhcp-option %s: maximum of %d search entries can be specified",
+ p[1], N_SEARCH_LIST_LEN);
+ }
+ }
+ else if ((streq(p[1], "DNS") || streq(p[1], "DNS6")) && p[2] && !p[3]
+ && (!strstr(p[2], ":") || ipv6_addr_safe(p[2])))
+ {
+ if (strstr(p[2], ":"))
+ {
+ dhcp_option_dns6_parse(p[2], dhcp->dns6, &dhcp->dns6_len, msglevel);
+ }
+ else
+ {
+ dhcp_option_address_parse("DNS", p[2], dhcp->dns, &dhcp->dns_len, msglevel);
+ }
+ }
+#if defined(_WIN32) || defined(TARGET_ANDROID)
else if (streq(p[1], "NBS") && p[2] && !p[3])
{
o->netbios_scope = p[2];
@@ -8239,23 +8373,9 @@
o->netbios_node_type = t;
o->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
}
- else if ((streq(p[1], "DNS") || streq(p[1], "DNS6")) && p[2] && !p[3]
- && (!strstr(p[2], ":") || ipv6_addr_safe(p[2])))
- {
- if (strstr(p[2], ":"))
- {
- dhcp_option_dns6_parse(p[2], o->dns6, &o->dns6_len, msglevel);
- }
- else
- {
- dhcp_option_address_parse("DNS", p[2], o->dns, &o->dns_len, msglevel);
- o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
- }
- }
else if (streq(p[1], "WINS") && p[2] && !p[3])
{
dhcp_option_address_parse("WINS", p[2], o->wins, &o->wins_len, msglevel);
- o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
}
else if (streq(p[1], "NTP") && p[2] && !p[3])
{
@@ -8267,19 +8387,6 @@
dhcp_option_address_parse("NBDD", p[2], o->nbdd, &o->nbdd_len, msglevel);
o->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
}
- else if (streq(p[1], "DOMAIN-SEARCH") && p[2] && !p[3])
- {
- if (o->domain_search_list_len < N_SEARCH_LIST_LEN)
- {
- o->domain_search_list[o->domain_search_list_len++] = p[2];
- }
- else
- {
- msg(msglevel, "--dhcp-option %s: maximum of %d search entries can be specified",
- p[1], N_SEARCH_LIST_LEN);
- }
- o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
- }
else if (streq(p[1], "DISABLE-NBT") && !p[2])
{
o->disable_nbt = 1;
@@ -8297,8 +8404,10 @@
msg(msglevel, "--dhcp-option: unknown option type '%s' or missing or unknown parameter", p[1]);
goto err;
}
- }
+#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+ setenv_foreign_option(options, (const char **)p, 3, es);
#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+ }
#ifdef _WIN32
else if (streq(p[0], "show-adapters") && !p[1])
{
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/904?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: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Gerrit-Change-Number: 904
Gerrit-PatchSet: 15
Gerrit-Owner: d12fk <he...@op...>
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: stipa <lst...@gm...>
Gerrit-MessageType: newpatchset
|
|
From: stipa (C. Review) <ge...@op...> - 2025-05-01 09:06:15
|
Attention is currently required from: d12fk, flichtenheld, plaisthos. stipa has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/904?usp=email ) Change subject: dns: deal with --dhcp-options when --dns is active ...................................................................... Patch Set 15: Code-Review+1 (2 comments) Patchset: PS15: Stared at the code and slightly tested on Windows, both --dhcp-option dns and --dns work, in various combinations. I haven't checked env variables and non-Windows parts. While not the fault of this patch, I noticed that inside run_up_down_service() we do not log success of setting DNS via service, like we normally do when we talk to the service: 2025-05-01 11:49:09 us=187000 INET address service: add 10.8.0.2/24 2025-05-01 11:49:09 us=203000 IPv4 MTU set to 1500 on interface 20 using service Let's do that for DNS, too, in a separate patch. File src/openvpn/options.c: http://gerrit.openvpn.net/c/openvpn/+/904/comment/dbd3e34f_fbb036f8 : PS15, Line 4299: tuntap_options_postprocess_dns(o); Why WIN32 and Android require special treatment? I see that it was not added in this patch, but still a comment "why" could be in place, since we touch this fragment. -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/904?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: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a Gerrit-Change-Number: 904 Gerrit-PatchSet: 15 Gerrit-Owner: d12fk <he...@op...> 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: d12fk <he...@op...> Gerrit-Comment-Date: Thu, 01 May 2025 09:05:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment |
|
From: d12fk (C. Review) <ge...@op...> - 2025-05-19 08:46:36
|
Attention is currently required from: flichtenheld, plaisthos, stipa. d12fk has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/904?usp=email ) Change subject: dns: deal with --dhcp-options when --dns is active ...................................................................... Patch Set 20: (1 comment) File src/openvpn/options.c: http://gerrit.openvpn.net/c/openvpn/+/904/comment/b4f06df1_f654bf62 : PS15, Line 4299: tuntap_options_postprocess_dns(o); > Why WIN32 and Android require special treatment? I see that it was not added in this patch, but stil […] The reason why is that both had DNS before and they were using the DNS related elements in `struct tuntap_options` which were filled by `--dhcp-option`s. Now we want the dhcp-option also in the dns_option struct and vice versa, for compatibility. For tap-windows with DHCP: I think we now do double configuration of DNS, now that I think about it more. Don't know if this is a problem that needs attention. What do you think? -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/904?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: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a Gerrit-Change-Number: 904 Gerrit-PatchSet: 20 Gerrit-Owner: d12fk <he...@op...> 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: stipa <lst...@gm...> Gerrit-Comment-Date: Mon, 19 May 2025 08:46:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: stipa <lst...@gm...> Gerrit-MessageType: comment |
|
From: cron2 (C. Review) <ge...@op...> - 2025-05-23 06:14:20
|
Attention is currently required from: d12fk, flichtenheld, plaisthos, stipa. cron2 has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/904?usp=email ) Change subject: dns: deal with --dhcp-options when --dns is active ...................................................................... Patch Set 21: (1 comment) File src/openvpn/options.c: http://gerrit.openvpn.net/c/openvpn/+/904/comment/aaf0a34c_938d33d0 : PS15, Line 4299: tuntap_options_postprocess_dns(o); > The reason why is that both had DNS before and they were using the DNS related elements in `struct t […] I have thought about this a while now, and would suggest to do the following - *if and only if* we are using the tap driver with ip-win32 dhcp, keep DNS things "as they are in 2.6" -> fill in DHCP options, do not set DNS via iservice - if DCO is used or a non-DHCP mode -> use iservice this will effectively make us use iservice most of the time, but if someone really wants old behaviour and relies on DHCP, they can have that (... and, alas, we see users that use a) tap, b) dhcp, and c) no iservice... - which is strongly not recommended, but...) -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/904?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: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a Gerrit-Change-Number: 904 Gerrit-PatchSet: 21 Gerrit-Owner: d12fk <he...@op...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-Reviewer: stipa <lst...@gm...> Gerrit-CC: cron2 <ge...@gr...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-Attention: d12fk <he...@op...> Gerrit-Attention: stipa <lst...@gm...> Gerrit-Comment-Date: Fri, 23 May 2025 06:14:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: d12fk <he...@op...> Comment-In-Reply-To: stipa <lst...@gm...> Gerrit-MessageType: comment |
|
From: d12fk (C. Review) <ge...@op...> - 2025-06-04 23:16:33
|
Attention is currently required from: d12fk, flichtenheld, plaisthos, stipa.
Hello flichtenheld, plaisthos, stipa,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/904?usp=email
to look at the new patch set (#22).
The following approvals got outdated and were removed:
Code-Review+1 by stipa
Change subject: dns: deal with --dhcp-options when --dns is active
......................................................................
dns: deal with --dhcp-options when --dns is active
Since --dns settings overrule DNS related --dhcp-options,
remove the latter when values were defined via --dns.
To stay as backward compatible as possible, we add foreign_options to
the script hook environment from the --dns values when a --up script
is defined. In that case the default --dns-updown is not run, even
when --dns values are present, to prevent double DNS configuration.
This way an existing --up script that deals with DNS can run, without
the immediate need to change after an openvpn upgrade and a server
pushing --dns options.
If you specify a custom --dns-updown, or force running the default
dns-updown that comes with openvpn, those compat env vars are not set
for --up scripts and the dns-updown command is run, even when there's
an --up script present.
Since Android uses the DNS values from tuntap_options, we always
override those with --dns stuff unconditionally. Also on Windows when
--ip-win32 is dynamic or adaptive, since DHCP relies on these as well.
Change-Id: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Signed-off-by: Heiko Hund <he...@is...>
---
M src/openvpn/dns.c
M src/openvpn/dns.h
M src/openvpn/options.c
3 files changed, 323 insertions(+), 184 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/04/904/22
diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c
index 7cf1b63..939ae09 100644
--- a/src/openvpn/dns.c
+++ b/src/openvpn/dns.c
@@ -691,7 +691,8 @@
static void
run_up_down_command(bool up, struct options *o, const struct tuntap *tt, struct dns_updown_runner_info *updown_runner)
{
- if (!o->dns_options.updown)
+ struct dns_options *dns = &o->dns_options;
+ if (!dns->updown || (o->up_script && !dns->user_set_updown))
{
return;
}
@@ -701,7 +702,7 @@
if (!updown_runner->required)
{
/* Run dns updown directly */
- status = do_run_up_down_command(up, NULL, &o->dns_options, tt);
+ status = do_run_up_down_command(up, NULL, dns, tt);
}
else
{
@@ -852,6 +853,14 @@
{
return;
}
+#ifdef _WIN32
+ /* Don't use iservice in DHCP mode */
+ struct tuntap_options *tto = &o->tuntap_options;
+ if (tto->ip_win32_type == IPW32_SET_DHCP_MASQ || tto->ip_win32_type == IPW32_SET_ADAPTIVE)
+ {
+ return;
+ }
+#endif
/* Warn about adding servers of unsupported AF */
const struct dns_server *s = o->dns_options.servers;
diff --git a/src/openvpn/dns.h b/src/openvpn/dns.h
index 60f5471..688daa7 100644
--- a/src/openvpn/dns.h
+++ b/src/openvpn/dns.h
@@ -76,7 +76,28 @@
#endif
};
+#ifndef N_DHCP_ADDR
+#define N_DHCP_ADDR 4
+#endif
+
+#ifndef N_SEARCH_LIST_LEN
+#define N_SEARCH_LIST_LEN 10
+#endif
+
+struct dhcp_options {
+ in_addr_t dns[N_DHCP_ADDR];
+ int dns_len;
+
+ struct in6_addr dns6[N_DHCP_ADDR];
+ int dns6_len;
+
+ const char *domain;
+ const char *domain_search_list[N_SEARCH_LIST_LEN];
+ int domain_search_list_len;
+};
+
struct dns_options {
+ struct dhcp_options from_dhcp;
struct dns_domain *search_domains;
struct dns_server *servers_prepull;
struct dns_server *servers;
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 6ea01d4..33f387c 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1328,7 +1328,6 @@
#endif /* ifndef ENABLE_SMALL */
#endif /* ifdef _WIN32 */
-#if defined(_WIN32) || defined(TARGET_ANDROID)
static void
dhcp_option_dns6_parse(const char *parm, struct in6_addr *dns6_list, int *len, int msglevel)
{
@@ -1371,150 +1370,6 @@
}
}
-/*
- * If DNS options are set use these for TUN/TAP options as well.
- * Applies to DNS, DNS6 and DOMAIN-SEARCH.
- * Existing options will be discarded.
- */
-static void
-tuntap_options_copy_dns(struct options *o)
-{
- struct tuntap_options *tt = &o->tuntap_options;
- struct dns_options *dns = &o->dns_options;
-
- if (dns->search_domains)
- {
- tt->domain_search_list_len = 0;
- const struct dns_domain *domain = dns->search_domains;
- while (domain && tt->domain_search_list_len < N_SEARCH_LIST_LEN)
- {
- tt->domain_search_list[tt->domain_search_list_len++] = domain->name;
- domain = domain->next;
- }
- if (domain)
- {
- msg(M_WARN, "WARNING: couldn't copy all --dns search-domains to --dhcp-option");
- }
- tt->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
- }
-
- if (dns->servers)
- {
- tt->dns_len = 0;
- tt->dns6_len = 0;
- bool overflow = false;
- const struct dns_server *server = dns->servers;
- while (server)
- {
- for (int i = 0; i < server->addr_count; ++i)
- {
- if (server->addr[i].family == AF_INET)
- {
- if (tt->dns_len >= N_DHCP_ADDR)
- {
- overflow = true;
- continue;
- }
- tt->dns[tt->dns_len++] = ntohl(server->addr[i].in.a4.s_addr);
- }
- else
- {
- if (tt->dns6_len >= N_DHCP_ADDR)
- {
- overflow = true;
- continue;
- }
- tt->dns6[tt->dns6_len++] = server->addr[i].in.a6;
- }
- }
- server = server->next;
- }
- if (overflow)
- {
- msg(M_WARN, "WARNING: couldn't copy all --dns server addresses to --dhcp-option");
- }
- tt->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
- }
-}
-#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
-static void
-foreign_options_copy_dns(struct options *o, struct env_set *es)
-{
- const struct dns_domain *domain = o->dns_options.search_domains;
- const struct dns_server *server = o->dns_options.servers;
- if (!domain && !server)
- {
- return;
- }
-
- /* reset the index since we're starting all over again */
- int opt_max = o->foreign_option_index;
- o->foreign_option_index = 0;
-
- for (int i = 1; i <= opt_max; ++i)
- {
- char name[32];
- snprintf(name, sizeof(name), "foreign_option_%d", i);
-
- const char *env_str = env_set_get(es, name);
- const char *value = strchr(env_str, '=') + 1;
- if ((domain && strstr(value, "dhcp-option DOMAIN-SEARCH") == value)
- || (server && strstr(value, "dhcp-option DNS") == value))
- {
- setenv_del(es, name);
- }
- else
- {
- setenv_foreign_option(o, &value, 1, es);
- }
- }
-
- struct gc_arena gc = gc_new();
-
- while (server)
- {
- for (size_t i = 0; i < server->addr_count; ++i)
- {
- if (server->addr[i].family == AF_INET)
- {
- const char *argv[] = {
- "dhcp-option",
- "DNS",
- print_in_addr_t(server->addr[i].in.a4.s_addr, 0, &gc)
- };
- setenv_foreign_option(o, argv, 3, es);
- }
- else
- {
- const char *argv[] = {
- "dhcp-option",
- "DNS6",
- print_in6_addr(server->addr[i].in.a6, 0, &gc)
- };
- setenv_foreign_option(o, argv, 3, es);
- }
- }
- server = server->next;
- }
- while (domain)
- {
- const char *argv[] = { "dhcp-option", "DOMAIN-SEARCH", domain->name };
- setenv_foreign_option(o, argv, 3, es);
- domain = domain->next;
- }
-
- gc_free(&gc);
-
- /* remove old leftover entries */
- while (o->foreign_option_index < opt_max)
- {
- char name[32];
- snprintf(name, sizeof(name), "foreign_option_%d", opt_max--);
- setenv_del(es, name);
- }
-}
-#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
-
#ifndef ENABLE_SMALL
static const char *
print_vlan_accept(enum vlan_acceptable_frames mode)
@@ -3603,6 +3458,260 @@
}
}
+#if defined(_WIN32) || defined(TARGET_ANDROID)
+/**
+ * @brief Postprocess DNS related settings
+ *
+ * Set TUN/TAP DNS options with values from either --dns
+ * or --dhcp-option.
+ *
+ * @param o pointer to the options struct
+ */
+static void
+tuntap_options_postprocess_dns(struct options *o)
+{
+ struct dns_options *dns = &o->dns_options;
+ struct tuntap_options *tt = &o->tuntap_options;
+ if (!dns->servers)
+ {
+ /* Copy --dhcp-options to tuntap_options */
+ struct dhcp_options *dhcp = &dns->from_dhcp;
+ assert(sizeof(dhcp->dns) == sizeof(tt->dns));
+ assert(sizeof(dhcp->dns6) == sizeof(tt->dns6));
+ assert(sizeof(dhcp->domain_search_list) == sizeof(tt->domain_search_list));
+
+ tt->domain = dhcp->domain;
+ tt->dns_len = dhcp->dns_len;
+ tt->dns6_len = dhcp->dns6_len;
+
+ memcpy(tt->dns, dhcp->dns, sizeof(tt->dns));
+ memcpy(tt->dns6, dhcp->dns6, sizeof(tt->dns6));
+
+ tt->domain_search_list_len = dhcp->domain_search_list_len;
+ for (size_t i = 0; i < SIZE(tt->domain_search_list); ++i)
+ {
+ tt->domain_search_list[i] = dhcp->domain_search_list[i];
+ }
+
+ return;
+ }
+
+#if defined(_WIN32)
+ if (tt->ip_win32_type != IPW32_SET_DHCP_MASQ && tt->ip_win32_type != IPW32_SET_ADAPTIVE)
+ {
+ return; /* Not in DHCP mode */
+ }
+#endif /* if defined(_WIN32) */
+
+ /* Copy --dns options to tuntap_options */
+ const struct dns_domain *d = dns->search_domains;
+ while (d && tt->domain_search_list_len + 1 < N_SEARCH_LIST_LEN)
+ {
+ tt->domain_search_list[tt->domain_search_list_len++] = d->name;
+ d = d->next;
+ }
+ if (d)
+ {
+ msg(M_WARN, "WARNING: couldn't copy all --dns search-domains to TUN/TAP");
+ }
+
+ const struct dns_server *s = dns->servers;
+ while (s)
+ {
+ bool non_standard_server_port = false;
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].port && s->addr[i].port != 53)
+ {
+ non_standard_server_port = true;
+ break;
+ }
+ }
+ if ((s->transport && s->transport != DNS_TRANSPORT_PLAIN)
+ || (s->dnssec && s->dnssec != DNS_SECURITY_NO)
+ || non_standard_server_port)
+ {
+ /* Skip servers requiring unsupported config to be set */
+ s = s->next;
+ }
+ else
+ {
+ bool overflow = false;
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].family == AF_INET && tt->dns_len + 1 < N_DHCP_ADDR)
+ {
+ tt->dns[tt->dns_len++] = s->addr[i].in.a4.s_addr;
+ }
+ else if (tt->dns6_len + 1 < N_DHCP_ADDR)
+ {
+ tt->dns6[tt->dns6_len] = s->addr[i].in.a6;
+ }
+ else
+ {
+ overflow = true;
+ }
+ }
+ if (overflow)
+ {
+ msg(M_WARN, "WARNING: couldn't copy all --dns server addresses to TUN/TAP");
+ }
+ return;
+ }
+ }
+}
+
+#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+
+/**
+ * @brief Postprocess DNS related settings
+ *
+ * Discard existing --dhcp-options from the env if needed and possibly
+ * replace them with values from --dns. If no --dns servers are set copy
+ * the --dhcp-option values over for --dns-updown runs.
+ *
+ * @param o pointer to the options struct
+ * @param es env set to modify potentially
+ */
+static void
+dhcp_options_postprocess_dns(struct options *o, struct env_set *es)
+{
+ struct gc_arena gc = gc_new();
+ struct dns_options *dns = &o->dns_options;
+
+ if (dns->servers || dns->user_set_updown)
+ {
+ /* Clean up env from --dhcp-option DNS config */
+ struct buffer name = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+ struct buffer value = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+
+ const int fo_count = o->foreign_option_index;
+ o->foreign_option_index = 0;
+
+ for (int i = 1; i <= fo_count; ++i)
+ {
+ buf_clear(&name);
+ buf_printf(&name, "foreign_option_%d", i);
+ const char *env_str = env_set_get(es, BSTR(&name));
+ const char *item_val = strchr(env_str, '=') + 1;
+ buf_clear(&value);
+ buf_printf(&value, "%s", item_val);
+
+ /* Remove foreign option item from env set */
+ env_set_del(es, BSTR(&name));
+
+ item_val = BSTR(&value);
+ if (strncmp(item_val, "dhcp-option ", 12) != 0
+ || (strncmp(item_val + 12, "ADAPTER-DOMAIN-SUFFIX ", 22) != 0
+ && strncmp(item_val + 12, "DOMAIN-SEARCH ", 14) != 0
+ && strncmp(item_val + 12, "DOMAIN ", 7) != 0
+ && strncmp(item_val + 12, "DNS6 ", 5) != 0
+ && strncmp(item_val + 12, "DNS ", 4) != 0))
+ {
+ /* Re-set the item with potentially updated name */
+ buf_clear(&name);
+ buf_printf(&name, "foreign_option_%d", ++o->foreign_option_index);
+ setenv_str(es, BSTR(&name), BSTR(&value));
+ }
+ }
+ }
+
+ if (!dns->servers)
+ {
+ /* Copy --dhcp-options to dns_options */
+ struct dhcp_options *dhcp = &dns->from_dhcp;
+
+ if (dhcp->dns_len || dhcp->dns6_len)
+ {
+ struct dns_domain **entry = &dns->search_domains;
+ ALLOC_OBJ_CLEAR_GC(*entry, struct dns_domain, &dns->gc);
+ struct dns_domain *new = *entry;
+ new->name = dhcp->domain;
+ entry = &new->next;
+
+ for (size_t i = 0; i < dhcp->domain_search_list_len; ++i)
+ {
+ ALLOC_OBJ_CLEAR_GC(*entry, struct dns_domain, &dns->gc);
+ struct dns_domain *new = *entry;
+ new->name = dhcp->domain_search_list[i];
+ entry = &new->next;
+ }
+
+ struct dns_server *server = dns_server_get(&dns->servers, 0, &dns->gc);
+ const size_t max_addrs = SIZE(server->addr);
+ for (size_t i = 0; i < dhcp->dns_len && server->addr_count < max_addrs; ++i)
+ {
+ server->addr[server->addr_count].in.a4.s_addr = htonl(dhcp->dns[i]);
+ server->addr[server->addr_count].family = AF_INET;
+ server->addr_count += 1;
+ }
+ for (size_t i = 0; i < dhcp->dns6_len && server->addr_count < max_addrs; ++i)
+ {
+ server->addr[server->addr_count].in.a6 = dhcp->dns6[i];
+ server->addr[server->addr_count].family = AF_INET6;
+ server->addr_count += 1;
+ }
+ }
+ }
+ else if (o->up_script && !dns->user_set_updown)
+ {
+ /* Set foreign option env vars from --dns config */
+ const char *p[] = { "dhcp-option", NULL, NULL };
+ size_t p_len = sizeof(p) / sizeof(p[0]);
+
+ p[1] = "DOMAIN";
+ const struct dns_domain *d = dns->search_domains;
+ while (d)
+ {
+ p[2] = d->name;
+ setenv_foreign_option(o, (const char **)p, p_len, es);
+ d = d->next;
+ }
+
+ const struct dns_server *s = dns->servers;
+ while (s)
+ {
+ bool non_standard_server_port = false;
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].port && s->addr[i].port != 53)
+ {
+ non_standard_server_port = true;
+ break;
+ }
+ }
+ if ((s->transport && s->transport != DNS_TRANSPORT_PLAIN)
+ || (s->dnssec && s->dnssec != DNS_SECURITY_NO)
+ || non_standard_server_port)
+ {
+ /* Skip servers requiring unsupported config to be set */
+ s = s->next;
+ }
+ else
+ {
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].family == AF_INET)
+ {
+ p[1] = "DNS";
+ p[2] = print_in_addr_t(s->addr[i].in.a4.s_addr, IA_NET_ORDER, &gc);
+ }
+ else
+ {
+ p[1] = "DNS6";
+ p[2] = print_in6_addr(s->addr[i].in.a6, 0, &gc);
+ }
+ setenv_foreign_option(o, (const char **)p, p_len, es);
+ }
+ break;
+ }
+ }
+ }
+
+ gc_free(&gc);
+}
+#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+
static void
options_postprocess_mutate(struct options *o, struct env_set *es)
{
@@ -3786,9 +3895,9 @@
else
{
#if defined(_WIN32) || defined(TARGET_ANDROID)
- tuntap_options_copy_dns(o);
+ tuntap_options_postprocess_dns(o);
#else
- foreign_options_copy_dns(o, es);
+ dhcp_options_postprocess_dns(o, es);
#endif
}
if (o->auth_token_generate && !o->auth_token_renewal)
@@ -4171,9 +4280,9 @@
{
dns_options_postprocess_pull(&o->dns_options);
#if defined(_WIN32) || defined(TARGET_ANDROID)
- tuntap_options_copy_dns(o);
+ tuntap_options_postprocess_dns(o);
#else
- foreign_options_copy_dns(o, es);
+ dhcp_options_postprocess_dns(o, es);
#endif
}
return success;
@@ -8162,18 +8271,43 @@
goto err;
}
}
-#if defined(_WIN32) || defined(TARGET_ANDROID)
else if (streq(p[0], "dhcp-option") && p[1])
{
+ struct dhcp_options *dhcp = &options->dns_options.from_dhcp;
+#if defined(_WIN32) || defined(TARGET_ANDROID)
struct tuntap_options *o = &options->tuntap_options;
+#endif
VERIFY_PERMISSION(OPT_P_DHCPDNS);
- if ((streq(p[1], "DOMAIN") || streq(p[1], "ADAPTER_DOMAIN_SUFFIX"))
- && p[2] && !p[3])
+ if ((streq(p[1], "DOMAIN") || streq(p[1], "ADAPTER_DOMAIN_SUFFIX")) && p[2] && !p[3])
{
- o->domain = p[2];
- o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
+ dhcp->domain = p[2];
}
+ else if (streq(p[1], "DOMAIN-SEARCH") && p[2] && !p[3])
+ {
+ if (dhcp->domain_search_list_len < N_SEARCH_LIST_LEN)
+ {
+ dhcp->domain_search_list[dhcp->domain_search_list_len++] = p[2];
+ }
+ else
+ {
+ msg(msglevel, "--dhcp-option %s: maximum of %d search entries can be specified",
+ p[1], N_SEARCH_LIST_LEN);
+ }
+ }
+ else if ((streq(p[1], "DNS") || streq(p[1], "DNS6")) && p[2] && !p[3]
+ && (!strstr(p[2], ":") || ipv6_addr_safe(p[2])))
+ {
+ if (strstr(p[2], ":"))
+ {
+ dhcp_option_dns6_parse(p[2], dhcp->dns6, &dhcp->dns6_len, msglevel);
+ }
+ else
+ {
+ dhcp_option_address_parse("DNS", p[2], dhcp->dns, &dhcp->dns_len, msglevel);
+ }
+ }
+#if defined(_WIN32) || defined(TARGET_ANDROID)
else if (streq(p[1], "NBS") && p[2] && !p[3])
{
o->netbios_scope = p[2];
@@ -8191,23 +8325,9 @@
o->netbios_node_type = t;
o->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
}
- else if ((streq(p[1], "DNS") || streq(p[1], "DNS6")) && p[2] && !p[3]
- && (!strstr(p[2], ":") || ipv6_addr_safe(p[2])))
- {
- if (strstr(p[2], ":"))
- {
- dhcp_option_dns6_parse(p[2], o->dns6, &o->dns6_len, msglevel);
- }
- else
- {
- dhcp_option_address_parse("DNS", p[2], o->dns, &o->dns_len, msglevel);
- o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
- }
- }
else if (streq(p[1], "WINS") && p[2] && !p[3])
{
dhcp_option_address_parse("WINS", p[2], o->wins, &o->wins_len, msglevel);
- o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
}
else if (streq(p[1], "NTP") && p[2] && !p[3])
{
@@ -8219,19 +8339,6 @@
dhcp_option_address_parse("NBDD", p[2], o->nbdd, &o->nbdd_len, msglevel);
o->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
}
- else if (streq(p[1], "DOMAIN-SEARCH") && p[2] && !p[3])
- {
- if (o->domain_search_list_len < N_SEARCH_LIST_LEN)
- {
- o->domain_search_list[o->domain_search_list_len++] = p[2];
- }
- else
- {
- msg(msglevel, "--dhcp-option %s: maximum of %d search entries can be specified",
- p[1], N_SEARCH_LIST_LEN);
- }
- o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
- }
else if (streq(p[1], "DISABLE-NBT") && !p[2])
{
o->disable_nbt = 1;
@@ -8249,8 +8356,10 @@
msg(msglevel, "--dhcp-option: unknown option type '%s' or missing or unknown parameter", p[1]);
goto err;
}
- }
+#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+ setenv_foreign_option(options, (const char **)p, 3, es);
#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+ }
#ifdef _WIN32
else if (streq(p[0], "show-adapters") && !p[1])
{
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/904?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: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Gerrit-Change-Number: 904
Gerrit-PatchSet: 22
Gerrit-Owner: d12fk <he...@op...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-Reviewer: stipa <lst...@gm...>
Gerrit-CC: cron2 <ge...@gr...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: d12fk <he...@op...>
Gerrit-Attention: stipa <lst...@gm...>
Gerrit-MessageType: newpatchset
|
|
From: d12fk (C. Review) <ge...@op...> - 2025-06-05 10:21:39
|
Attention is currently required from: cron2, flichtenheld, plaisthos, stipa. d12fk has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/904?usp=email ) Change subject: dns: deal with --dhcp-options when --dns is active ...................................................................... Patch Set 22: (1 comment) File src/openvpn/options.c: http://gerrit.openvpn.net/c/openvpn/+/904/comment/99c9aa4d_22bf37c4 : PS15, Line 4299: tuntap_options_postprocess_dns(o); > I have thought about this a while now, and would suggest to do the following […] Think all this is covered in the latest push, please check. -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/904?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: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a Gerrit-Change-Number: 904 Gerrit-PatchSet: 22 Gerrit-Owner: d12fk <he...@op...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-Reviewer: stipa <lst...@gm...> Gerrit-CC: cron2 <ge...@gr...> 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: Thu, 05 Jun 2025 10:21:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: cron2 <ge...@gr...> Comment-In-Reply-To: d12fk <he...@op...> Comment-In-Reply-To: stipa <lst...@gm...> Gerrit-MessageType: comment |
|
From: d12fk (C. Review) <ge...@op...> - 2025-06-18 12:47:42
|
Attention is currently required from: cron2, flichtenheld, plaisthos, stipa.
Hello flichtenheld, plaisthos, stipa,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/904?usp=email
to look at the new patch set (#24).
Change subject: dns: deal with --dhcp-options when --dns is active
......................................................................
dns: deal with --dhcp-options when --dns is active
Since --dns settings overrule DNS related --dhcp-options,
remove the latter when values were defined via --dns.
To stay as backward compatible as possible, we add foreign_options to
the script hook environment from the --dns values when a --up script
is defined. In that case the default --dns-updown is not run, even
when --dns values are present, to prevent double DNS configuration.
This way an existing --up script that deals with DNS can run, without
the immediate need to change after an openvpn upgrade and a server
pushing --dns options.
If you specify a custom --dns-updown, or force running the default
dns-updown that comes with openvpn, those compat env vars are not set
for --up scripts and the dns-updown command is run, even when there's
an --up script present.
Since Android uses the DNS values from tuntap_options, we always
override those with --dns stuff unconditionally. Also on Windows when
--ip-win32 is dynamic or adaptive, since DHCP relies on these as well.
Change-Id: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Signed-off-by: Heiko Hund <he...@is...>
---
M src/openvpn/dns.c
M src/openvpn/dns.h
M src/openvpn/options.c
3 files changed, 323 insertions(+), 184 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/04/904/24
diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c
index 7cf1b63..939ae09 100644
--- a/src/openvpn/dns.c
+++ b/src/openvpn/dns.c
@@ -691,7 +691,8 @@
static void
run_up_down_command(bool up, struct options *o, const struct tuntap *tt, struct dns_updown_runner_info *updown_runner)
{
- if (!o->dns_options.updown)
+ struct dns_options *dns = &o->dns_options;
+ if (!dns->updown || (o->up_script && !dns->user_set_updown))
{
return;
}
@@ -701,7 +702,7 @@
if (!updown_runner->required)
{
/* Run dns updown directly */
- status = do_run_up_down_command(up, NULL, &o->dns_options, tt);
+ status = do_run_up_down_command(up, NULL, dns, tt);
}
else
{
@@ -852,6 +853,14 @@
{
return;
}
+#ifdef _WIN32
+ /* Don't use iservice in DHCP mode */
+ struct tuntap_options *tto = &o->tuntap_options;
+ if (tto->ip_win32_type == IPW32_SET_DHCP_MASQ || tto->ip_win32_type == IPW32_SET_ADAPTIVE)
+ {
+ return;
+ }
+#endif
/* Warn about adding servers of unsupported AF */
const struct dns_server *s = o->dns_options.servers;
diff --git a/src/openvpn/dns.h b/src/openvpn/dns.h
index 60f5471..688daa7 100644
--- a/src/openvpn/dns.h
+++ b/src/openvpn/dns.h
@@ -76,7 +76,28 @@
#endif
};
+#ifndef N_DHCP_ADDR
+#define N_DHCP_ADDR 4
+#endif
+
+#ifndef N_SEARCH_LIST_LEN
+#define N_SEARCH_LIST_LEN 10
+#endif
+
+struct dhcp_options {
+ in_addr_t dns[N_DHCP_ADDR];
+ int dns_len;
+
+ struct in6_addr dns6[N_DHCP_ADDR];
+ int dns6_len;
+
+ const char *domain;
+ const char *domain_search_list[N_SEARCH_LIST_LEN];
+ int domain_search_list_len;
+};
+
struct dns_options {
+ struct dhcp_options from_dhcp;
struct dns_domain *search_domains;
struct dns_server *servers_prepull;
struct dns_server *servers;
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 6ea01d4..d758a67 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1328,7 +1328,6 @@
#endif /* ifndef ENABLE_SMALL */
#endif /* ifdef _WIN32 */
-#if defined(_WIN32) || defined(TARGET_ANDROID)
static void
dhcp_option_dns6_parse(const char *parm, struct in6_addr *dns6_list, int *len, int msglevel)
{
@@ -1371,150 +1370,6 @@
}
}
-/*
- * If DNS options are set use these for TUN/TAP options as well.
- * Applies to DNS, DNS6 and DOMAIN-SEARCH.
- * Existing options will be discarded.
- */
-static void
-tuntap_options_copy_dns(struct options *o)
-{
- struct tuntap_options *tt = &o->tuntap_options;
- struct dns_options *dns = &o->dns_options;
-
- if (dns->search_domains)
- {
- tt->domain_search_list_len = 0;
- const struct dns_domain *domain = dns->search_domains;
- while (domain && tt->domain_search_list_len < N_SEARCH_LIST_LEN)
- {
- tt->domain_search_list[tt->domain_search_list_len++] = domain->name;
- domain = domain->next;
- }
- if (domain)
- {
- msg(M_WARN, "WARNING: couldn't copy all --dns search-domains to --dhcp-option");
- }
- tt->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
- }
-
- if (dns->servers)
- {
- tt->dns_len = 0;
- tt->dns6_len = 0;
- bool overflow = false;
- const struct dns_server *server = dns->servers;
- while (server)
- {
- for (int i = 0; i < server->addr_count; ++i)
- {
- if (server->addr[i].family == AF_INET)
- {
- if (tt->dns_len >= N_DHCP_ADDR)
- {
- overflow = true;
- continue;
- }
- tt->dns[tt->dns_len++] = ntohl(server->addr[i].in.a4.s_addr);
- }
- else
- {
- if (tt->dns6_len >= N_DHCP_ADDR)
- {
- overflow = true;
- continue;
- }
- tt->dns6[tt->dns6_len++] = server->addr[i].in.a6;
- }
- }
- server = server->next;
- }
- if (overflow)
- {
- msg(M_WARN, "WARNING: couldn't copy all --dns server addresses to --dhcp-option");
- }
- tt->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
- }
-}
-#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
-static void
-foreign_options_copy_dns(struct options *o, struct env_set *es)
-{
- const struct dns_domain *domain = o->dns_options.search_domains;
- const struct dns_server *server = o->dns_options.servers;
- if (!domain && !server)
- {
- return;
- }
-
- /* reset the index since we're starting all over again */
- int opt_max = o->foreign_option_index;
- o->foreign_option_index = 0;
-
- for (int i = 1; i <= opt_max; ++i)
- {
- char name[32];
- snprintf(name, sizeof(name), "foreign_option_%d", i);
-
- const char *env_str = env_set_get(es, name);
- const char *value = strchr(env_str, '=') + 1;
- if ((domain && strstr(value, "dhcp-option DOMAIN-SEARCH") == value)
- || (server && strstr(value, "dhcp-option DNS") == value))
- {
- setenv_del(es, name);
- }
- else
- {
- setenv_foreign_option(o, &value, 1, es);
- }
- }
-
- struct gc_arena gc = gc_new();
-
- while (server)
- {
- for (size_t i = 0; i < server->addr_count; ++i)
- {
- if (server->addr[i].family == AF_INET)
- {
- const char *argv[] = {
- "dhcp-option",
- "DNS",
- print_in_addr_t(server->addr[i].in.a4.s_addr, 0, &gc)
- };
- setenv_foreign_option(o, argv, 3, es);
- }
- else
- {
- const char *argv[] = {
- "dhcp-option",
- "DNS6",
- print_in6_addr(server->addr[i].in.a6, 0, &gc)
- };
- setenv_foreign_option(o, argv, 3, es);
- }
- }
- server = server->next;
- }
- while (domain)
- {
- const char *argv[] = { "dhcp-option", "DOMAIN-SEARCH", domain->name };
- setenv_foreign_option(o, argv, 3, es);
- domain = domain->next;
- }
-
- gc_free(&gc);
-
- /* remove old leftover entries */
- while (o->foreign_option_index < opt_max)
- {
- char name[32];
- snprintf(name, sizeof(name), "foreign_option_%d", opt_max--);
- setenv_del(es, name);
- }
-}
-#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
-
#ifndef ENABLE_SMALL
static const char *
print_vlan_accept(enum vlan_acceptable_frames mode)
@@ -3603,6 +3458,260 @@
}
}
+#if defined(_WIN32) || defined(TARGET_ANDROID)
+/**
+ * @brief Postprocess DNS related settings
+ *
+ * Set TUN/TAP DNS options with values from either --dns
+ * or --dhcp-option.
+ *
+ * @param o pointer to the options struct
+ */
+static void
+tuntap_options_postprocess_dns(struct options *o)
+{
+ struct dns_options *dns = &o->dns_options;
+ struct tuntap_options *tt = &o->tuntap_options;
+ if (!dns->servers)
+ {
+ /* Copy --dhcp-options to tuntap_options */
+ struct dhcp_options *dhcp = &dns->from_dhcp;
+ assert(sizeof(dhcp->dns) == sizeof(tt->dns));
+ assert(sizeof(dhcp->dns6) == sizeof(tt->dns6));
+ assert(sizeof(dhcp->domain_search_list) == sizeof(tt->domain_search_list));
+
+ tt->domain = dhcp->domain;
+ tt->dns_len = dhcp->dns_len;
+ tt->dns6_len = dhcp->dns6_len;
+
+ memcpy(tt->dns, dhcp->dns, sizeof(tt->dns));
+ memcpy(tt->dns6, dhcp->dns6, sizeof(tt->dns6));
+
+ tt->domain_search_list_len = dhcp->domain_search_list_len;
+ for (size_t i = 0; i < SIZE(tt->domain_search_list); ++i)
+ {
+ tt->domain_search_list[i] = dhcp->domain_search_list[i];
+ }
+
+ return;
+ }
+
+#if defined(_WIN32)
+ if (tt->ip_win32_type != IPW32_SET_DHCP_MASQ && tt->ip_win32_type != IPW32_SET_ADAPTIVE)
+ {
+ return; /* Not in DHCP mode */
+ }
+#endif /* if defined(_WIN32) */
+
+ /* Copy --dns options to tuntap_options */
+ const struct dns_domain *d = dns->search_domains;
+ while (d && tt->domain_search_list_len + 1 < N_SEARCH_LIST_LEN)
+ {
+ tt->domain_search_list[tt->domain_search_list_len++] = d->name;
+ d = d->next;
+ }
+ if (d)
+ {
+ msg(M_WARN, "WARNING: couldn't copy all --dns search-domains to TUN/TAP");
+ }
+
+ const struct dns_server *s = dns->servers;
+ while (s)
+ {
+ bool non_standard_server_port = false;
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].port && s->addr[i].port != 53)
+ {
+ non_standard_server_port = true;
+ break;
+ }
+ }
+ if ((s->transport && s->transport != DNS_TRANSPORT_PLAIN)
+ || (s->dnssec && s->dnssec != DNS_SECURITY_NO)
+ || non_standard_server_port)
+ {
+ /* Skip servers requiring unsupported config to be set */
+ s = s->next;
+ }
+ else
+ {
+ bool overflow = false;
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].family == AF_INET && tt->dns_len + 1 < N_DHCP_ADDR)
+ {
+ tt->dns[tt->dns_len++] = s->addr[i].in.a4.s_addr;
+ }
+ else if (tt->dns6_len + 1 < N_DHCP_ADDR)
+ {
+ tt->dns6[tt->dns6_len++] = s->addr[i].in.a6;
+ }
+ else
+ {
+ overflow = true;
+ }
+ }
+ if (overflow)
+ {
+ msg(M_WARN, "WARNING: couldn't copy all --dns server addresses to TUN/TAP");
+ }
+ return;
+ }
+ }
+}
+
+#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+
+/**
+ * @brief Postprocess DNS related settings
+ *
+ * Discard existing --dhcp-options from the env if needed and possibly
+ * replace them with values from --dns. If no --dns servers are set copy
+ * the --dhcp-option values over for --dns-updown runs.
+ *
+ * @param o pointer to the options struct
+ * @param es env set to modify potentially
+ */
+static void
+dhcp_options_postprocess_dns(struct options *o, struct env_set *es)
+{
+ struct gc_arena gc = gc_new();
+ struct dns_options *dns = &o->dns_options;
+
+ if (dns->servers || dns->user_set_updown)
+ {
+ /* Clean up env from --dhcp-option DNS config */
+ struct buffer name = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+ struct buffer value = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+
+ const int fo_count = o->foreign_option_index;
+ o->foreign_option_index = 0;
+
+ for (int i = 1; i <= fo_count; ++i)
+ {
+ buf_clear(&name);
+ buf_printf(&name, "foreign_option_%d", i);
+ const char *env_str = env_set_get(es, BSTR(&name));
+ const char *item_val = strchr(env_str, '=') + 1;
+ buf_clear(&value);
+ buf_printf(&value, "%s", item_val);
+
+ /* Remove foreign option item from env set */
+ env_set_del(es, BSTR(&name));
+
+ item_val = BSTR(&value);
+ if (strncmp(item_val, "dhcp-option ", 12) != 0
+ || (strncmp(item_val + 12, "ADAPTER-DOMAIN-SUFFIX ", 22) != 0
+ && strncmp(item_val + 12, "DOMAIN-SEARCH ", 14) != 0
+ && strncmp(item_val + 12, "DOMAIN ", 7) != 0
+ && strncmp(item_val + 12, "DNS6 ", 5) != 0
+ && strncmp(item_val + 12, "DNS ", 4) != 0))
+ {
+ /* Re-set the item with potentially updated name */
+ buf_clear(&name);
+ buf_printf(&name, "foreign_option_%d", ++o->foreign_option_index);
+ setenv_str(es, BSTR(&name), BSTR(&value));
+ }
+ }
+ }
+
+ if (!dns->servers)
+ {
+ /* Copy --dhcp-options to dns_options */
+ struct dhcp_options *dhcp = &dns->from_dhcp;
+
+ if (dhcp->dns_len || dhcp->dns6_len)
+ {
+ struct dns_domain **entry = &dns->search_domains;
+ ALLOC_OBJ_CLEAR_GC(*entry, struct dns_domain, &dns->gc);
+ struct dns_domain *new = *entry;
+ new->name = dhcp->domain;
+ entry = &new->next;
+
+ for (size_t i = 0; i < dhcp->domain_search_list_len; ++i)
+ {
+ ALLOC_OBJ_CLEAR_GC(*entry, struct dns_domain, &dns->gc);
+ struct dns_domain *new = *entry;
+ new->name = dhcp->domain_search_list[i];
+ entry = &new->next;
+ }
+
+ struct dns_server *server = dns_server_get(&dns->servers, 0, &dns->gc);
+ const size_t max_addrs = SIZE(server->addr);
+ for (size_t i = 0; i < dhcp->dns_len && server->addr_count < max_addrs; ++i)
+ {
+ server->addr[server->addr_count].in.a4.s_addr = htonl(dhcp->dns[i]);
+ server->addr[server->addr_count].family = AF_INET;
+ server->addr_count += 1;
+ }
+ for (size_t i = 0; i < dhcp->dns6_len && server->addr_count < max_addrs; ++i)
+ {
+ server->addr[server->addr_count].in.a6 = dhcp->dns6[i];
+ server->addr[server->addr_count].family = AF_INET6;
+ server->addr_count += 1;
+ }
+ }
+ }
+ else if (o->up_script && !dns->user_set_updown)
+ {
+ /* Set foreign option env vars from --dns config */
+ const char *p[] = { "dhcp-option", NULL, NULL };
+ size_t p_len = sizeof(p) / sizeof(p[0]);
+
+ p[1] = "DOMAIN";
+ const struct dns_domain *d = dns->search_domains;
+ while (d)
+ {
+ p[2] = d->name;
+ setenv_foreign_option(o, (const char **)p, p_len, es);
+ d = d->next;
+ }
+
+ const struct dns_server *s = dns->servers;
+ while (s)
+ {
+ bool non_standard_server_port = false;
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].port && s->addr[i].port != 53)
+ {
+ non_standard_server_port = true;
+ break;
+ }
+ }
+ if ((s->transport && s->transport != DNS_TRANSPORT_PLAIN)
+ || (s->dnssec && s->dnssec != DNS_SECURITY_NO)
+ || non_standard_server_port)
+ {
+ /* Skip servers requiring unsupported config to be set */
+ s = s->next;
+ }
+ else
+ {
+ for (int i = 0; i < s->addr_count; ++i)
+ {
+ if (s->addr[i].family == AF_INET)
+ {
+ p[1] = "DNS";
+ p[2] = print_in_addr_t(s->addr[i].in.a4.s_addr, IA_NET_ORDER, &gc);
+ }
+ else
+ {
+ p[1] = "DNS6";
+ p[2] = print_in6_addr(s->addr[i].in.a6, 0, &gc);
+ }
+ setenv_foreign_option(o, (const char **)p, p_len, es);
+ }
+ break;
+ }
+ }
+ }
+
+ gc_free(&gc);
+}
+#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+
static void
options_postprocess_mutate(struct options *o, struct env_set *es)
{
@@ -3786,9 +3895,9 @@
else
{
#if defined(_WIN32) || defined(TARGET_ANDROID)
- tuntap_options_copy_dns(o);
+ tuntap_options_postprocess_dns(o);
#else
- foreign_options_copy_dns(o, es);
+ dhcp_options_postprocess_dns(o, es);
#endif
}
if (o->auth_token_generate && !o->auth_token_renewal)
@@ -4171,9 +4280,9 @@
{
dns_options_postprocess_pull(&o->dns_options);
#if defined(_WIN32) || defined(TARGET_ANDROID)
- tuntap_options_copy_dns(o);
+ tuntap_options_postprocess_dns(o);
#else
- foreign_options_copy_dns(o, es);
+ dhcp_options_postprocess_dns(o, es);
#endif
}
return success;
@@ -8162,18 +8271,43 @@
goto err;
}
}
-#if defined(_WIN32) || defined(TARGET_ANDROID)
else if (streq(p[0], "dhcp-option") && p[1])
{
+ struct dhcp_options *dhcp = &options->dns_options.from_dhcp;
+#if defined(_WIN32) || defined(TARGET_ANDROID)
struct tuntap_options *o = &options->tuntap_options;
+#endif
VERIFY_PERMISSION(OPT_P_DHCPDNS);
- if ((streq(p[1], "DOMAIN") || streq(p[1], "ADAPTER_DOMAIN_SUFFIX"))
- && p[2] && !p[3])
+ if ((streq(p[1], "DOMAIN") || streq(p[1], "ADAPTER_DOMAIN_SUFFIX")) && p[2] && !p[3])
{
- o->domain = p[2];
- o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
+ dhcp->domain = p[2];
}
+ else if (streq(p[1], "DOMAIN-SEARCH") && p[2] && !p[3])
+ {
+ if (dhcp->domain_search_list_len < N_SEARCH_LIST_LEN)
+ {
+ dhcp->domain_search_list[dhcp->domain_search_list_len++] = p[2];
+ }
+ else
+ {
+ msg(msglevel, "--dhcp-option %s: maximum of %d search entries can be specified",
+ p[1], N_SEARCH_LIST_LEN);
+ }
+ }
+ else if ((streq(p[1], "DNS") || streq(p[1], "DNS6")) && p[2] && !p[3]
+ && (!strstr(p[2], ":") || ipv6_addr_safe(p[2])))
+ {
+ if (strstr(p[2], ":"))
+ {
+ dhcp_option_dns6_parse(p[2], dhcp->dns6, &dhcp->dns6_len, msglevel);
+ }
+ else
+ {
+ dhcp_option_address_parse("DNS", p[2], dhcp->dns, &dhcp->dns_len, msglevel);
+ }
+ }
+#if defined(_WIN32) || defined(TARGET_ANDROID)
else if (streq(p[1], "NBS") && p[2] && !p[3])
{
o->netbios_scope = p[2];
@@ -8191,23 +8325,9 @@
o->netbios_node_type = t;
o->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
}
- else if ((streq(p[1], "DNS") || streq(p[1], "DNS6")) && p[2] && !p[3]
- && (!strstr(p[2], ":") || ipv6_addr_safe(p[2])))
- {
- if (strstr(p[2], ":"))
- {
- dhcp_option_dns6_parse(p[2], o->dns6, &o->dns6_len, msglevel);
- }
- else
- {
- dhcp_option_address_parse("DNS", p[2], o->dns, &o->dns_len, msglevel);
- o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
- }
- }
else if (streq(p[1], "WINS") && p[2] && !p[3])
{
dhcp_option_address_parse("WINS", p[2], o->wins, &o->wins_len, msglevel);
- o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
}
else if (streq(p[1], "NTP") && p[2] && !p[3])
{
@@ -8219,19 +8339,6 @@
dhcp_option_address_parse("NBDD", p[2], o->nbdd, &o->nbdd_len, msglevel);
o->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
}
- else if (streq(p[1], "DOMAIN-SEARCH") && p[2] && !p[3])
- {
- if (o->domain_search_list_len < N_SEARCH_LIST_LEN)
- {
- o->domain_search_list[o->domain_search_list_len++] = p[2];
- }
- else
- {
- msg(msglevel, "--dhcp-option %s: maximum of %d search entries can be specified",
- p[1], N_SEARCH_LIST_LEN);
- }
- o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
- }
else if (streq(p[1], "DISABLE-NBT") && !p[2])
{
o->disable_nbt = 1;
@@ -8249,8 +8356,10 @@
msg(msglevel, "--dhcp-option: unknown option type '%s' or missing or unknown parameter", p[1]);
goto err;
}
- }
+#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+ setenv_foreign_option(options, (const char **)p, 3, es);
#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
+ }
#ifdef _WIN32
else if (streq(p[0], "show-adapters") && !p[1])
{
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/904?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: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a
Gerrit-Change-Number: 904
Gerrit-PatchSet: 24
Gerrit-Owner: d12fk <he...@op...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-Reviewer: stipa <lst...@gm...>
Gerrit-CC: cron2 <ge...@gr...>
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-MessageType: newpatchset
|
|
From: cron2 (C. Review) <ge...@op...> - 2025-06-18 12:48:36
|
Attention is currently required from: d12fk, flichtenheld, plaisthos, stipa. cron2 has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/904?usp=email ) Change subject: dns: deal with --dhcp-options when --dns is active ...................................................................... Patch Set 24: Code-Review+2 -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/904?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: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a Gerrit-Change-Number: 904 Gerrit-PatchSet: 24 Gerrit-Owner: d12fk <he...@op...> 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: d12fk <he...@op...> Gerrit-Attention: stipa <lst...@gm...> Gerrit-Comment-Date: Wed, 18 Jun 2025 12:48:16 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment |
|
From: Gert D. <ge...@gr...> - 2025-06-18 12:48:49
|
From: Heiko Hund <he...@is...> Since --dns settings overrule DNS related --dhcp-options, remove the latter when values were defined via --dns. To stay as backward compatible as possible, we add foreign_options to the script hook environment from the --dns values when a --up script is defined. In that case the default --dns-updown is not run, even when --dns values are present, to prevent double DNS configuration. This way an existing --up script that deals with DNS can run, without the immediate need to change after an openvpn upgrade and a server pushing --dns options. If you specify a custom --dns-updown, or force running the default dns-updown that comes with openvpn, those compat env vars are not set for --up scripts and the dns-updown command is run, even when there's an --up script present. Since Android uses the DNS values from tuntap_options, we always override those with --dns stuff unconditionally. Also on Windows when --ip-win32 is dynamic or adaptive, since DHCP relies on these as well. Change-Id: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a Signed-off-by: Heiko Hund <he...@is...> 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/+/904 This mail reflects revision 24 of this Change. Acked-by according to Gerrit (reflected above): Gert Doering <ge...@gr...> diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c index 7cf1b63..939ae09 100644 --- a/src/openvpn/dns.c +++ b/src/openvpn/dns.c @@ -691,7 +691,8 @@ static void run_up_down_command(bool up, struct options *o, const struct tuntap *tt, struct dns_updown_runner_info *updown_runner) { - if (!o->dns_options.updown) + struct dns_options *dns = &o->dns_options; + if (!dns->updown || (o->up_script && !dns->user_set_updown)) { return; } @@ -701,7 +702,7 @@ if (!updown_runner->required) { /* Run dns updown directly */ - status = do_run_up_down_command(up, NULL, &o->dns_options, tt); + status = do_run_up_down_command(up, NULL, dns, tt); } else { @@ -852,6 +853,14 @@ { return; } +#ifdef _WIN32 + /* Don't use iservice in DHCP mode */ + struct tuntap_options *tto = &o->tuntap_options; + if (tto->ip_win32_type == IPW32_SET_DHCP_MASQ || tto->ip_win32_type == IPW32_SET_ADAPTIVE) + { + return; + } +#endif /* Warn about adding servers of unsupported AF */ const struct dns_server *s = o->dns_options.servers; diff --git a/src/openvpn/dns.h b/src/openvpn/dns.h index 60f5471..688daa7 100644 --- a/src/openvpn/dns.h +++ b/src/openvpn/dns.h @@ -76,7 +76,28 @@ #endif }; +#ifndef N_DHCP_ADDR +#define N_DHCP_ADDR 4 +#endif + +#ifndef N_SEARCH_LIST_LEN +#define N_SEARCH_LIST_LEN 10 +#endif + +struct dhcp_options { + in_addr_t dns[N_DHCP_ADDR]; + int dns_len; + + struct in6_addr dns6[N_DHCP_ADDR]; + int dns6_len; + + const char *domain; + const char *domain_search_list[N_SEARCH_LIST_LEN]; + int domain_search_list_len; +}; + struct dns_options { + struct dhcp_options from_dhcp; struct dns_domain *search_domains; struct dns_server *servers_prepull; struct dns_server *servers; diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 6ea01d4..d758a67 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -1328,7 +1328,6 @@ #endif /* ifndef ENABLE_SMALL */ #endif /* ifdef _WIN32 */ -#if defined(_WIN32) || defined(TARGET_ANDROID) static void dhcp_option_dns6_parse(const char *parm, struct in6_addr *dns6_list, int *len, int msglevel) { @@ -1371,150 +1370,6 @@ } } -/* - * If DNS options are set use these for TUN/TAP options as well. - * Applies to DNS, DNS6 and DOMAIN-SEARCH. - * Existing options will be discarded. - */ -static void -tuntap_options_copy_dns(struct options *o) -{ - struct tuntap_options *tt = &o->tuntap_options; - struct dns_options *dns = &o->dns_options; - - if (dns->search_domains) - { - tt->domain_search_list_len = 0; - const struct dns_domain *domain = dns->search_domains; - while (domain && tt->domain_search_list_len < N_SEARCH_LIST_LEN) - { - tt->domain_search_list[tt->domain_search_list_len++] = domain->name; - domain = domain->next; - } - if (domain) - { - msg(M_WARN, "WARNING: couldn't copy all --dns search-domains to --dhcp-option"); - } - tt->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED; - } - - if (dns->servers) - { - tt->dns_len = 0; - tt->dns6_len = 0; - bool overflow = false; - const struct dns_server *server = dns->servers; - while (server) - { - for (int i = 0; i < server->addr_count; ++i) - { - if (server->addr[i].family == AF_INET) - { - if (tt->dns_len >= N_DHCP_ADDR) - { - overflow = true; - continue; - } - tt->dns[tt->dns_len++] = ntohl(server->addr[i].in.a4.s_addr); - } - else - { - if (tt->dns6_len >= N_DHCP_ADDR) - { - overflow = true; - continue; - } - tt->dns6[tt->dns6_len++] = server->addr[i].in.a6; - } - } - server = server->next; - } - if (overflow) - { - msg(M_WARN, "WARNING: couldn't copy all --dns server addresses to --dhcp-option"); - } - tt->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL; - } -} -#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */ -static void -foreign_options_copy_dns(struct options *o, struct env_set *es) -{ - const struct dns_domain *domain = o->dns_options.search_domains; - const struct dns_server *server = o->dns_options.servers; - if (!domain && !server) - { - return; - } - - /* reset the index since we're starting all over again */ - int opt_max = o->foreign_option_index; - o->foreign_option_index = 0; - - for (int i = 1; i <= opt_max; ++i) - { - char name[32]; - snprintf(name, sizeof(name), "foreign_option_%d", i); - - const char *env_str = env_set_get(es, name); - const char *value = strchr(env_str, '=') + 1; - if ((domain && strstr(value, "dhcp-option DOMAIN-SEARCH") == value) - || (server && strstr(value, "dhcp-option DNS") == value)) - { - setenv_del(es, name); - } - else - { - setenv_foreign_option(o, &value, 1, es); - } - } - - struct gc_arena gc = gc_new(); - - while (server) - { - for (size_t i = 0; i < server->addr_count; ++i) - { - if (server->addr[i].family == AF_INET) - { - const char *argv[] = { - "dhcp-option", - "DNS", - print_in_addr_t(server->addr[i].in.a4.s_addr, 0, &gc) - }; - setenv_foreign_option(o, argv, 3, es); - } - else - { - const char *argv[] = { - "dhcp-option", - "DNS6", - print_in6_addr(server->addr[i].in.a6, 0, &gc) - }; - setenv_foreign_option(o, argv, 3, es); - } - } - server = server->next; - } - while (domain) - { - const char *argv[] = { "dhcp-option", "DOMAIN-SEARCH", domain->name }; - setenv_foreign_option(o, argv, 3, es); - domain = domain->next; - } - - gc_free(&gc); - - /* remove old leftover entries */ - while (o->foreign_option_index < opt_max) - { - char name[32]; - snprintf(name, sizeof(name), "foreign_option_%d", opt_max--); - setenv_del(es, name); - } -} -#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */ - #ifndef ENABLE_SMALL static const char * print_vlan_accept(enum vlan_acceptable_frames mode) @@ -3603,6 +3458,260 @@ } } +#if defined(_WIN32) || defined(TARGET_ANDROID) +/** + * @brief Postprocess DNS related settings + * + * Set TUN/TAP DNS options with values from either --dns + * or --dhcp-option. + * + * @param o pointer to the options struct + */ +static void +tuntap_options_postprocess_dns(struct options *o) +{ + struct dns_options *dns = &o->dns_options; + struct tuntap_options *tt = &o->tuntap_options; + if (!dns->servers) + { + /* Copy --dhcp-options to tuntap_options */ + struct dhcp_options *dhcp = &dns->from_dhcp; + assert(sizeof(dhcp->dns) == sizeof(tt->dns)); + assert(sizeof(dhcp->dns6) == sizeof(tt->dns6)); + assert(sizeof(dhcp->domain_search_list) == sizeof(tt->domain_search_list)); + + tt->domain = dhcp->domain; + tt->dns_len = dhcp->dns_len; + tt->dns6_len = dhcp->dns6_len; + + memcpy(tt->dns, dhcp->dns, sizeof(tt->dns)); + memcpy(tt->dns6, dhcp->dns6, sizeof(tt->dns6)); + + tt->domain_search_list_len = dhcp->domain_search_list_len; + for (size_t i = 0; i < SIZE(tt->domain_search_list); ++i) + { + tt->domain_search_list[i] = dhcp->domain_search_list[i]; + } + + return; + } + +#if defined(_WIN32) + if (tt->ip_win32_type != IPW32_SET_DHCP_MASQ && tt->ip_win32_type != IPW32_SET_ADAPTIVE) + { + return; /* Not in DHCP mode */ + } +#endif /* if defined(_WIN32) */ + + /* Copy --dns options to tuntap_options */ + const struct dns_domain *d = dns->search_domains; + while (d && tt->domain_search_list_len + 1 < N_SEARCH_LIST_LEN) + { + tt->domain_search_list[tt->domain_search_list_len++] = d->name; + d = d->next; + } + if (d) + { + msg(M_WARN, "WARNING: couldn't copy all --dns search-domains to TUN/TAP"); + } + + const struct dns_server *s = dns->servers; + while (s) + { + bool non_standard_server_port = false; + for (int i = 0; i < s->addr_count; ++i) + { + if (s->addr[i].port && s->addr[i].port != 53) + { + non_standard_server_port = true; + break; + } + } + if ((s->transport && s->transport != DNS_TRANSPORT_PLAIN) + || (s->dnssec && s->dnssec != DNS_SECURITY_NO) + || non_standard_server_port) + { + /* Skip servers requiring unsupported config to be set */ + s = s->next; + } + else + { + bool overflow = false; + for (int i = 0; i < s->addr_count; ++i) + { + if (s->addr[i].family == AF_INET && tt->dns_len + 1 < N_DHCP_ADDR) + { + tt->dns[tt->dns_len++] = s->addr[i].in.a4.s_addr; + } + else if (tt->dns6_len + 1 < N_DHCP_ADDR) + { + tt->dns6[tt->dns6_len++] = s->addr[i].in.a6; + } + else + { + overflow = true; + } + } + if (overflow) + { + msg(M_WARN, "WARNING: couldn't copy all --dns server addresses to TUN/TAP"); + } + return; + } + } +} + +#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */ + +/** + * @brief Postprocess DNS related settings + * + * Discard existing --dhcp-options from the env if needed and possibly + * replace them with values from --dns. If no --dns servers are set copy + * the --dhcp-option values over for --dns-updown runs. + * + * @param o pointer to the options struct + * @param es env set to modify potentially + */ +static void +dhcp_options_postprocess_dns(struct options *o, struct env_set *es) +{ + struct gc_arena gc = gc_new(); + struct dns_options *dns = &o->dns_options; + + if (dns->servers || dns->user_set_updown) + { + /* Clean up env from --dhcp-option DNS config */ + struct buffer name = alloc_buf_gc(OPTION_PARM_SIZE, &gc); + struct buffer value = alloc_buf_gc(OPTION_PARM_SIZE, &gc); + + const int fo_count = o->foreign_option_index; + o->foreign_option_index = 0; + + for (int i = 1; i <= fo_count; ++i) + { + buf_clear(&name); + buf_printf(&name, "foreign_option_%d", i); + const char *env_str = env_set_get(es, BSTR(&name)); + const char *item_val = strchr(env_str, '=') + 1; + buf_clear(&value); + buf_printf(&value, "%s", item_val); + + /* Remove foreign option item from env set */ + env_set_del(es, BSTR(&name)); + + item_val = BSTR(&value); + if (strncmp(item_val, "dhcp-option ", 12) != 0 + || (strncmp(item_val + 12, "ADAPTER-DOMAIN-SUFFIX ", 22) != 0 + && strncmp(item_val + 12, "DOMAIN-SEARCH ", 14) != 0 + && strncmp(item_val + 12, "DOMAIN ", 7) != 0 + && strncmp(item_val + 12, "DNS6 ", 5) != 0 + && strncmp(item_val + 12, "DNS ", 4) != 0)) + { + /* Re-set the item with potentially updated name */ + buf_clear(&name); + buf_printf(&name, "foreign_option_%d", ++o->foreign_option_index); + setenv_str(es, BSTR(&name), BSTR(&value)); + } + } + } + + if (!dns->servers) + { + /* Copy --dhcp-options to dns_options */ + struct dhcp_options *dhcp = &dns->from_dhcp; + + if (dhcp->dns_len || dhcp->dns6_len) + { + struct dns_domain **entry = &dns->search_domains; + ALLOC_OBJ_CLEAR_GC(*entry, struct dns_domain, &dns->gc); + struct dns_domain *new = *entry; + new->name = dhcp->domain; + entry = &new->next; + + for (size_t i = 0; i < dhcp->domain_search_list_len; ++i) + { + ALLOC_OBJ_CLEAR_GC(*entry, struct dns_domain, &dns->gc); + struct dns_domain *new = *entry; + new->name = dhcp->domain_search_list[i]; + entry = &new->next; + } + + struct dns_server *server = dns_server_get(&dns->servers, 0, &dns->gc); + const size_t max_addrs = SIZE(server->addr); + for (size_t i = 0; i < dhcp->dns_len && server->addr_count < max_addrs; ++i) + { + server->addr[server->addr_count].in.a4.s_addr = htonl(dhcp->dns[i]); + server->addr[server->addr_count].family = AF_INET; + server->addr_count += 1; + } + for (size_t i = 0; i < dhcp->dns6_len && server->addr_count < max_addrs; ++i) + { + server->addr[server->addr_count].in.a6 = dhcp->dns6[i]; + server->addr[server->addr_count].family = AF_INET6; + server->addr_count += 1; + } + } + } + else if (o->up_script && !dns->user_set_updown) + { + /* Set foreign option env vars from --dns config */ + const char *p[] = { "dhcp-option", NULL, NULL }; + size_t p_len = sizeof(p) / sizeof(p[0]); + + p[1] = "DOMAIN"; + const struct dns_domain *d = dns->search_domains; + while (d) + { + p[2] = d->name; + setenv_foreign_option(o, (const char **)p, p_len, es); + d = d->next; + } + + const struct dns_server *s = dns->servers; + while (s) + { + bool non_standard_server_port = false; + for (int i = 0; i < s->addr_count; ++i) + { + if (s->addr[i].port && s->addr[i].port != 53) + { + non_standard_server_port = true; + break; + } + } + if ((s->transport && s->transport != DNS_TRANSPORT_PLAIN) + || (s->dnssec && s->dnssec != DNS_SECURITY_NO) + || non_standard_server_port) + { + /* Skip servers requiring unsupported config to be set */ + s = s->next; + } + else + { + for (int i = 0; i < s->addr_count; ++i) + { + if (s->addr[i].family == AF_INET) + { + p[1] = "DNS"; + p[2] = print_in_addr_t(s->addr[i].in.a4.s_addr, IA_NET_ORDER, &gc); + } + else + { + p[1] = "DNS6"; + p[2] = print_in6_addr(s->addr[i].in.a6, 0, &gc); + } + setenv_foreign_option(o, (const char **)p, p_len, es); + } + break; + } + } + } + + gc_free(&gc); +} +#endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */ + static void options_postprocess_mutate(struct options *o, struct env_set *es) { @@ -3786,9 +3895,9 @@ else { #if defined(_WIN32) || defined(TARGET_ANDROID) - tuntap_options_copy_dns(o); + tuntap_options_postprocess_dns(o); #else - foreign_options_copy_dns(o, es); + dhcp_options_postprocess_dns(o, es); #endif } if (o->auth_token_generate && !o->auth_token_renewal) @@ -4171,9 +4280,9 @@ { dns_options_postprocess_pull(&o->dns_options); #if defined(_WIN32) || defined(TARGET_ANDROID) - tuntap_options_copy_dns(o); + tuntap_options_postprocess_dns(o); #else - foreign_options_copy_dns(o, es); + dhcp_options_postprocess_dns(o, es); #endif } return success; @@ -8162,18 +8271,43 @@ goto err; } } -#if defined(_WIN32) || defined(TARGET_ANDROID) else if (streq(p[0], "dhcp-option") && p[1]) { + struct dhcp_options *dhcp = &options->dns_options.from_dhcp; +#if defined(_WIN32) || defined(TARGET_ANDROID) struct tuntap_options *o = &options->tuntap_options; +#endif VERIFY_PERMISSION(OPT_P_DHCPDNS); - if ((streq(p[1], "DOMAIN") || streq(p[1], "ADAPTER_DOMAIN_SUFFIX")) - && p[2] && !p[3]) + if ((streq(p[1], "DOMAIN") || streq(p[1], "ADAPTER_DOMAIN_SUFFIX")) && p[2] && !p[3]) { - o->domain = p[2]; - o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL; + dhcp->domain = p[2]; } + else if (streq(p[1], "DOMAIN-SEARCH") && p[2] && !p[3]) + { + if (dhcp->domain_search_list_len < N_SEARCH_LIST_LEN) + { + dhcp->domain_search_list[dhcp->domain_search_list_len++] = p[2]; + } + else + { + msg(msglevel, "--dhcp-option %s: maximum of %d search entries can be specified", + p[1], N_SEARCH_LIST_LEN); + } + } + else if ((streq(p[1], "DNS") || streq(p[1], "DNS6")) && p[2] && !p[3] + && (!strstr(p[2], ":") || ipv6_addr_safe(p[2]))) + { + if (strstr(p[2], ":")) + { + dhcp_option_dns6_parse(p[2], dhcp->dns6, &dhcp->dns6_len, msglevel); + } + else + { + dhcp_option_address_parse("DNS", p[2], dhcp->dns, &dhcp->dns_len, msglevel); + } + } +#if defined(_WIN32) || defined(TARGET_ANDROID) else if (streq(p[1], "NBS") && p[2] && !p[3]) { o->netbios_scope = p[2]; @@ -8191,23 +8325,9 @@ o->netbios_node_type = t; o->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED; } - else if ((streq(p[1], "DNS") || streq(p[1], "DNS6")) && p[2] && !p[3] - && (!strstr(p[2], ":") || ipv6_addr_safe(p[2]))) - { - if (strstr(p[2], ":")) - { - dhcp_option_dns6_parse(p[2], o->dns6, &o->dns6_len, msglevel); - } - else - { - dhcp_option_address_parse("DNS", p[2], o->dns, &o->dns_len, msglevel); - o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL; - } - } else if (streq(p[1], "WINS") && p[2] && !p[3]) { dhcp_option_address_parse("WINS", p[2], o->wins, &o->wins_len, msglevel); - o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL; } else if (streq(p[1], "NTP") && p[2] && !p[3]) { @@ -8219,19 +8339,6 @@ dhcp_option_address_parse("NBDD", p[2], o->nbdd, &o->nbdd_len, msglevel); o->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED; } - else if (streq(p[1], "DOMAIN-SEARCH") && p[2] && !p[3]) - { - if (o->domain_search_list_len < N_SEARCH_LIST_LEN) - { - o->domain_search_list[o->domain_search_list_len++] = p[2]; - } - else - { - msg(msglevel, "--dhcp-option %s: maximum of %d search entries can be specified", - p[1], N_SEARCH_LIST_LEN); - } - o->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL; - } else if (streq(p[1], "DISABLE-NBT") && !p[2]) { o->disable_nbt = 1; @@ -8249,8 +8356,10 @@ msg(msglevel, "--dhcp-option: unknown option type '%s' or missing or unknown parameter", p[1]); goto err; } - } +#else /* if defined(_WIN32) || defined(TARGET_ANDROID) */ + setenv_foreign_option(options, (const char **)p, 3, es); #endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */ + } #ifdef _WIN32 else if (streq(p[0], "show-adapters") && !p[1]) { |
|
From: Gert D. <ge...@gr...> - 2025-06-18 13:01:22
|
So - finally the Grand DNS Rewrite is reaching the final commits - thanks
Heiko for all the work.
This patch brings working --dns-updown for configs where the server still
pushes "dhcp-options DNS" - and vice versa, compat "foreign_option..."
settings in --up for configs where the server pushes "--dns ..."
Windows will see DNS via iservice "but not if TAP + DHCP is active"
(compat again).
Lev has tested the various windows variants, I have tested all the
"incoming" variants (reference server can push this or that, depending
on client requests) and verified that --dns-updown applies whatever
makes sense, and alternatively "--up printenv.sh" will show the correct
DNS options.
There might be bugs lurking, but overall this does what it aims for, and
the code looks good (I stared at it quite a bit over the last months ;-) ).
NOTE: Android has not explicitely been tested, but given that it shares
the "tuntap" option handling with Windows, I expect nothing to break there.
Your patch has been applied to the master branch.
commit 2dfc4f82792b197dfa47d2bfe32c2af66b329611
Author: Heiko Hund
Date: Wed Jun 18 14:48:29 2025 +0200
dns: deal with --dhcp-options when --dns is active
Signed-off-by: Heiko Hund <he...@is...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg31922.html
Signed-off-by: Gert Doering <ge...@gr...>
--
kind regards,
Gert Doering
|
|
From: cron2 (C. Review) <ge...@op...> - 2025-06-18 12:50:06
|
Attention is currently required from: d12fk, flichtenheld, plaisthos, stipa. cron2 has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/904?usp=email ) Change subject: dns: deal with --dhcp-options when --dns is active ...................................................................... Patch Set 24: (2 comments) Patchset: PS24: Tested various combinations of incoming --dns/--dhcp-option DNS and outgoing --up and --dns-updown, and with the missing ++, it now looks really nice. File src/openvpn/options.c: http://gerrit.openvpn.net/c/openvpn/+/904/comment/a940614d_7efe3270 : PS15, Line 4299: tuntap_options_postprocess_dns(o); > Think all this is covered in the latest push, please check. Done -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/904?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: I635c4018fb43b5976a39b6a90cb2e9cb2570cd6a Gerrit-Change-Number: 904 Gerrit-PatchSet: 24 Gerrit-Owner: d12fk <he...@op...> 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: d12fk <he...@op...> Gerrit-Attention: stipa <lst...@gm...> Gerrit-Comment-Date: Wed, 18 Jun 2025 12:49:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: cron2 <ge...@gr...> Comment-In-Reply-To: d12fk <he...@op...> Comment-In-Reply-To: stipa <lst...@gm...> Gerrit-MessageType: comment |
×
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