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...@ist.eigentlich.net>
---
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...@openvpn.net>
Gerrit-Reviewer: flichtenheld <fr...@lichtenheld.com>
Gerrit-Reviewer: plaisthos <arne-open...@rfc2549.org>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: plaisthos <arne-open...@rfc2549.org>
Gerrit-Attention: flichtenheld <fr...@lichtenheld.com>
Gerrit-Attention: d12fk <he...@openvpn.net>
Gerrit-MessageType: newpatchset
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
