diff mbox series

[Openvpn-devel] dns option: remove support for exclude-domains

Message ID 20230228044142.39803-1-heiko@ist.eigentlich.net
State Superseded
Headers show
Series [Openvpn-devel] dns option: remove support for exclude-domains | expand

Commit Message

Heiko Hund Feb. 28, 2023, 4:41 a.m. UTC 
No DNS resolver currently supports this and it is not possible to
emulate the behavior without the chance of errors. Finding the
effective default system DNS server(s) to specify the exclude
DNS routes is not trivial and cannot be verified to be correct
without resolver internal knowledge. So, it is better to not
support this instead of supporting it, but incorrectly.

Signed-off-by: Heiko Hund <heiko@ist.eigentlich.net>
---
 doc/man-sections/client-options.rst | 14 +++++---------
 src/openvpn/dns.c                   | 13 ++-----------
 src/openvpn/dns.h                   |  7 -------
 src/openvpn/options.c               | 16 ----------------
 4 files changed, 7 insertions(+), 43 deletions(-)

Comments

David Sommerseth March 2, 2023, 8:25 p.m. UTC | #1 
On 28/02/2023 05:41, Heiko Hund wrote:
> No DNS resolver currently supports this and it is not possible to
> emulate the behavior without the chance of errors. Finding the
> effective default system DNS server(s) to specify the exclude
> DNS routes is not trivial and cannot be verified to be correct
> without resolver internal knowledge. So, it is better to not
> support this instead of supporting it, but incorrectly.
> 
> Signed-off-by: Heiko Hund <heiko@ist.eigentlich.net>
> ---
>   doc/man-sections/client-options.rst | 14 +++++---------
>   src/openvpn/dns.c                   | 13 ++-----------
>   src/openvpn/dns.h                   |  7 -------
>   src/openvpn/options.c               | 16 ----------------
>   4 files changed, 7 insertions(+), 43 deletions(-)


I've only glared at the code and quickly done a few compile tests. 
LGTM.  Change itself also makes sense.

Acked-By: David Sommerseth <davids@openvpn.net>
Heiko Hund March 3, 2023, 1:41 p.m. UTC | #2 
On Donnerstag, 2. März 2023 21:25:03 CET David Sommerseth wrote:
> I've only glared at the code and quickly done a few compile tests.
> LGTM.  Change itself also makes sense.
> 
> Acked-By: David Sommerseth <davids@openvpn.net>

Thanks, please note that there is a newer version of this patch in gerrit. The 
delta is rather small. You can find the current version at [1]. 

FYI: testing gerrit with (all) the dns option related things and like it. Will 
send the final version to this list again after it has been approved in 
gerrit.

[1] https://gerrit.openvpn.net/c/openvpn/+/39
diff mbox series

Patch

diff --git a/doc/man-sections/client-options.rst b/doc/man-sections/client-options.rst
index 0b973adf..85d5610f 100644
--- a/doc/man-sections/client-options.rst
+++ b/doc/man-sections/client-options.rst
@@ -169,7 +169,7 @@  configuration.
 
      dns search-domains domain [domain ...]
      dns server n address addr[:port] [addr[:port]] [addr[:port]] [addr[:port]]
-     dns server n resolve-domains|exclude-domains domain [domain ...]
+     dns server n resolve-domains domain [domain ...]
      dns server n dnssec yes|optional|no
      dns server n transport DoH|DoT|plain
      dns server n sni server-name
@@ -191,14 +191,10 @@  configuration.
   Optionally a port can be appended after a colon. IPv6 addresses need to
   be enclosed in brackets if a port is appended.
 
-  The ``resolve-domains`` and ``exclude-domains`` options take one or
-  more DNS domains which are explicitly resolved or explicitly not resolved
-  by a server. Only one of the options can be configured for a server.
-  ``resolve-domains`` is used to define a split-dns setup, where only
-  given domains are resolved by a server. ``exclude-domains`` is used to
-  define domains which will never be resolved by a server (e.g. domains
-  which can only be resolved locally). Systems which do not support fine
-  grained DNS domain configuration, will ignore these settings.
+  The ``resolve-domains`` option takes one or more DNS domains used to define
+  a split-dns or dns-routing setup, where only the given domains are resolved
+  by the server. Systems which do not support fine grained DNS domain
+  configuration, will ignore this setting.
 
   The ``dnssec`` option is used to configure validation of DNSSEC records.
   While the exact semantics may differ for resolvers on different systems,
diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c
index 18f6e58b..b1486c86 100644
--- a/src/openvpn/dns.c
+++ b/src/openvpn/dns.c
@@ -408,11 +408,9 @@  setenv_dns_options(const struct dns_options *o, struct env_set *es)
 
         if (s->domains)
         {
-            const char *format = s->domain_type == DNS_RESOLVE_DOMAINS ?
-                                 "dns_server_%d_resolve_domain_%d" : "dns_server_%d_exclude_domain_%d";
             for (j = 1, d = s->domains; d != NULL; j++, d = d->next)
             {
-                setenv_dns_option(es, format, i, j, d->name);
+                setenv_dns_option(es, "dns_server_%d_resolve_domain_%d", i, j, d->name);
             }
         }
 
@@ -491,14 +489,7 @@  show_dns_options(const struct dns_options *o)
         struct dns_domain *domain = server->domains;
         if (domain)
         {
-            if (server->domain_type == DNS_RESOLVE_DOMAINS)
-            {
-                msg(D_SHOW_PARMS, "    resolve domains:");
-            }
-            else
-            {
-                msg(D_SHOW_PARMS, "    exclude domains:");
-            }
+            msg(D_SHOW_PARMS, "    resolve domains:");
             while (domain)
             {
                 msg(D_SHOW_PARMS, "      %s", domain->name);
diff --git a/src/openvpn/dns.h b/src/openvpn/dns.h
index 34f864dd..22cb333b 100644
--- a/src/openvpn/dns.h
+++ b/src/openvpn/dns.h
@@ -27,12 +27,6 @@ 
 #include "buffer.h"
 #include "env_set.h"
 
-enum dns_domain_type {
-    DNS_DOMAINS_UNSET,
-    DNS_RESOLVE_DOMAINS,
-    DNS_EXCLUDE_DOMAINS
-};
-
 enum dns_security {
     DNS_SECURITY_UNSET,
     DNS_SECURITY_NO,
@@ -69,7 +63,6 @@  struct dns_server {
     struct dns_server_addr addr4[2];
     struct dns_server_addr addr6[2];
     struct dns_domain *domains;
-    enum dns_domain_type domain_type;
     enum dns_security dnssec;
     enum dns_server_transport transport;
     const char *sni;
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 7ea1994a..19731b53 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -8016,22 +8016,6 @@  add_option(struct options *options,
             }
             else if (streq(p[3], "resolve-domains"))
             {
-                if (server->domain_type == DNS_EXCLUDE_DOMAINS)
-                {
-                    msg(msglevel, "--dns server %ld: cannot use resolve-domains and exclude-domains", priority);
-                    goto err;
-                }
-                server->domain_type = DNS_RESOLVE_DOMAINS;
-                dns_domain_list_append(&server->domains, &p[4], &options->dns_options.gc);
-            }
-            else if (streq(p[3], "exclude-domains"))
-            {
-                if (server->domain_type == DNS_RESOLVE_DOMAINS)
-                {
-                    msg(msglevel, "--dns server %ld: cannot use exclude-domains and resolve-domains", priority);
-                    goto err;
-                }
-                server->domain_type = DNS_EXCLUDE_DOMAINS;
                 dns_domain_list_append(&server->domains, &p[4], &options->dns_options.gc);
             }
             else if (streq(p[3], "dnssec") && !p[5])