Treat dhcp-option DNS6 and DNS identical

OpenVPN3 accepts both IPv4 and IPv6 with option-dhcp DNS but throws an error for option-dhcp DNS6.

This patch makes OpenVPN2 accept IPv4/IPv6 for both DNS and DNS6

Patch V2: Put IPv6 parsing logic into own function similar as for for IPv4 DNS

---
 doc/openvpn.8         |  8 ++------
 src/openvpn/options.c | 38 ++++++++++++++++++++++++--------------
 2 files changed, 26 insertions(+), 20 deletions(-)

doc/openvpn.8 +0 -0

index 43bbc217..f9ccbb30 100644
@@ -5886,14 +5886,10 @@ across the VPN.
 Set Connection\-specific DNS Suffix.
 
 .B DNS addr \-\-

src/openvpn/options.c +23 -13

index 7c6528bc..d67044fc 100644
@@ -1228,6 +1228,20 @@ show_tuntap_options(const struct tuntap_options *o)
 
 #if defined(_WIN32) || defined(TARGET_ANDROID)
 static void
+dhcp_option_dns6_parse(const char *parm, struct in6_addr *dns6_list, int *len, int msglevel)
+{
+    struct in6_addr addr;
+    if (*len >= N_DHCP_ADDR)
+    {
+        msg(msglevel, "--dhcp-option DNS: maximum of %d IPv6 dns servers can be specified",
+            N_DHCP_ADDR);
+    }
+    else if (get_ipv6_addr(parm, &addr, NULL, msglevel))
+    {
+        dns6_list[(*len)++] = addr;
+    }
+}
+static void
 dhcp_option_address_parse(const char *name, const char *parm, in_addr_t *array, int *len, int msglevel)
 {
     if (*len >= N_DHCP_ADDR)

@@ -7088,6 +7102,7 @@ add_option(struct options *options,
     {
         struct tuntap_options *o = &options->tuntap_options;
         VERIFY_PERMISSION(OPT_P_IPWIN32);
+        bool ipv6dns = false;
 
         if (streq(p[1], "DOMAIN") && p[2])
         {

@@ -7108,22 +7123,17 @@ add_option(struct options *options,
             }
             o->netbios_node_type = t;
         }
-        else if (streq(p[1], "DNS") && p[2])
+        else if ((streq(p[1], "DNS") || streq(p[1], "DNS6")) && p[2] && (!strstr(p[2], ":") || ipv6_addr_safe(p[2])))
         {
-            dhcp_option_address_parse("DNS", p[2], o->dns, &o->dns_len, msglevel);
-        }
-        else if (streq(p[1], "DNS6") && p[2] && ipv6_addr_safe(p[2]))
-        {
-            struct in6_addr addr;
-            foreign_option(options, p, 3, es);
-            if (o->dns6_len >= N_DHCP_ADDR)
+            if (strstr(p[2], ":"))
             {
-                msg(msglevel, "--dhcp-option DNS6: maximum of %d dns servers can be specified",
-                    N_DHCP_ADDR);
-            }
-            else if (get_ipv6_addr(p[2], &addr, NULL, msglevel))
+                ipv6dns=true;
+                foreign_option(options, p, 3, es);
+                dhcp_option_dns6_parse(p[2], o->dns6, &o->dns6_len, msglevel);
+            }
+            else
             {
-                o->dns6[o->dns6_len++] = addr;
+                dhcp_option_address_parse("DNS", p[2], o->dns, &o->dns_len, msglevel);
             }
         }
         else if (streq(p[1], "WINS") && p[2])

OpenVPN3 accepts both IPv4 and IPv6 with option-dhcp DNS but throws an error for option-dhcp DNS6.

This patch makes OpenVPN2 accept IPv4/IPv6 for both DNS and DNS6

Patch V2: Put IPv6 parsing logic into own function similar as for for IPv4 DNS

---
 doc/openvpn.8         | 14 +++++---------
 src/openvpn/options.c | 39 ++++++++++++++++++++++++---------------
 2 files changed, 29 insertions(+), 24 deletions(-)

doc/openvpn.8 +5 -9

index 43bbc217..d083b908 100644
@@ -5886,17 +5886,13 @@ across the VPN.
 Set Connection\-specific DNS Suffix.
 
 .B DNS addr \-\-
-Set primary domain name server IPv4 address.  Repeat
+Set primary domain name server IPv4 or IPv6 address.  Repeat
 this option to set secondary DNS server addresses.
 
-.B DNS6 addr \-\-
-Set primary domain name server IPv6 address.  Repeat
-this option to set secondary DNS server IPv6 addresses.
-
-Note: currently this is handled using netsh (the
-existing DHCP code can only do IPv4 DHCP, and that protocol only
-permits IPv4 addresses anywhere).  The option will be put into the
-environment, so an
+Note: DNS IPv6 servers are currently set using netsh (the existing
+DHCP code can only do IPv4 DHCP, and that protocol only permits IPv4
+addresses anywhere).  The option will be put into the environment, so
+an
 .B \-\-up
 script could act upon it if needed.
 

src/openvpn/options.c +23 -14

index 75def7b6..f405d8a2 100644
@@ -705,8 +705,7 @@ static const char usage_message[] =
     "                    which allow multiple addresses,\n"
     "                    --dhcp-option must be repeated.\n"
     "                    DOMAIN name : Set DNS suffix\n"
-    "                    DNS addr    : Set domain name server address(es) (IPv4)\n"
-    "                    DNS6 addr   : Set domain name server address(es) (IPv6)\n"
+    "                    DNS addr    : Set domain name server address(es) (IPv4 and IPv6)\n"
     "                    NTP         : Set NTP server address(es)\n"
     "                    NBDD        : Set NBDD server address(es)\n"
     "                    WINS addr   : Set WINS server address(es)\n"

@@ -1228,6 +1227,20 @@ show_tuntap_options(const struct tuntap_options *o)
 
 #if defined(_WIN32) || defined(TARGET_ANDROID)
 static void
+dhcp_option_dns6_parse(const char *parm, struct in6_addr *dns6_list, int *len, int msglevel)
+{
+    struct in6_addr addr;
+    if (*len >= N_DHCP_ADDR)
+    {
+        msg(msglevel, "--dhcp-option DNS: maximum of %d IPv6 dns servers can be specified",
+            N_DHCP_ADDR);
+    }
+    else if (get_ipv6_addr(parm, &addr, NULL, msglevel))
+    {
+        dns6_list[(*len)++] = addr;
+    }
+}
+static void
 dhcp_option_address_parse(const char *name, const char *parm, in_addr_t *array, int *len, int msglevel)
 {
     if (*len >= N_DHCP_ADDR)

@@ -7070,6 +7083,7 @@ add_option(struct options *options,
     {
         struct tuntap_options *o = &options->tuntap_options;
         VERIFY_PERMISSION(OPT_P_IPWIN32);
+        bool ipv6dns = false;
 
         if (streq(p[1], "DOMAIN") && p[2])
         {

@@ -7090,22 +7104,17 @@ add_option(struct options *options,
             }
             o->netbios_node_type = t;
         }
-        else if (streq(p[1], "DNS") && p[2])
+        else if ((streq(p[1], "DNS") || streq(p[1], "DNS6")) && p[2] && (!strstr(p[2], ":") || ipv6_addr_safe(p[2])))
         {
-            dhcp_option_address_parse("DNS", p[2], o->dns, &o->dns_len, msglevel);
-        }
-        else if (streq(p[1], "DNS6") && p[2] && ipv6_addr_safe(p[2]))
-        {
-            struct in6_addr addr;
-            foreign_option(options, p, 3, es);
-            if (o->dns6_len >= N_DHCP_ADDR)
+            if (strstr(p[2], ":"))
             {
-                msg(msglevel, "--dhcp-option DNS6: maximum of %d dns servers can be specified",
-                    N_DHCP_ADDR);
+                ipv6dns=true;
+                foreign_option(options, p, 3, es);
+                dhcp_option_dns6_parse(p[2], o->dns6, &o->dns6_len, msglevel);
             }
-            else if (get_ipv6_addr(p[2], &addr, NULL, msglevel))
+            else
             {
-                o->dns6[o->dns6_len++] = addr;
+                dhcp_option_address_parse("DNS", p[2], o->dns, &o->dns_len, msglevel);
             }
         }
         else if (streq(p[1], "WINS") && p[2])

I've taken the liberty to change the commit message and mention that it's "v3" here :-)

commit 849006bf17bba524e6f3344598adcbe41bedf450 (master) commit 61a72ecd59a57a50d0757b56b16f4ed3ce700f3f (release/2.4)

Author: Arne Schwabe
Date:   Wed Jan 31 10:41:02 2018 +0100

Treat dhcp-option DNS6 and DNS identical

Hi,

Quoted 9 lines · Gert Doering

I've taken the liberty to change the commit message and mention that it's "v3" here :-)

commit 849006bf17bba524e6f3344598adcbe41bedf450 (master) commit 61a72ecd59a57a50d0757b56b16f4ed3ce700f3f (release/2.4)

Author: Arne Schwabe
Date:   Wed Jan 31 10:41:02 2018 +0100

Treat dhcp-option DNS6 and DNS identical

This is... interesting. The change is minor, but seems to have pushed the endless if/else if/else construction in options.c over an invisible cliff, and some buildbots (FreeBSD 7.4, Debian 8.5) now *sometimes* bomb with "signal 11" when gcc is compiling options.c ...

gcc -DHAVE_CONFIG_H -I. -I../.. -I../../include -I../../include -I../../src/compat -DPLUGIN_LIBDIR=\"/usr/local/lib/openvpn/plugins\" -g -O2 -std=c99 -MT options.o -MD -MP -MF .deps/options.Tpo -c -o options.o options.c
gcc: Internal error: Killed: 9 (program cc1) Please submit a full bug report.
See <URL:http://gcc.gnu.org/bugs.html> for instructions.

Quoted 12 lines

Hi,

Quoted 8 lines · Gisle Vanem

While compiling src/openvpn/options.c with MSVC 2015, I got this

error:

src/openvpn/options.c(5944): fatal error C1026: parser stack overflow, program too complex

As a quick fix you can locally edit options.c to split the large if ..endif block with 300 or so else-if's into two groups with a couple of goto's. I've no idea how msvc parses code, so this may or may not work.

Quoted 8 lines

Quoted 3 lines

I've also seen such error in GeoIP-lib due to all the 'if..else if' statements.

Can this be written using some kind of a table-lookup code instead?

The option parser code using if-else is very standard and the logic easy to follow, so I doubt there is a strong reason for re-writing it.

Selva