[Openvpn-devel,4/4] dns: also (re)place foreign dhcp options in env

Message ID 20220527012457.1819262-5-heiko@ist.eigentlich.net
State Accepted
Headers show
Series [Openvpn-devel,1/4] remove foreign_option() call for IPv6 DNS servers | expand

Commit Message

Heiko Hund May 27, 2022, 1:24 a.m. UTC
Override DNS related foreign_options with values set by the --dns
option. This is done, so that scripts looking for these options continue
to work if only --dns option were pushed, or the values in the
--dhcp-options differ fron what's pushed in --dns.

Signed-off-by: Heiko Hund <heiko@ist.eigentlich.net>
---
 src/openvpn/openvpn.c |  2 +-
 src/openvpn/options.c | 88 ++++++++++++++++++++++++++++++++++++++++---
 src/openvpn/options.h |  2 +-
 3 files changed, 85 insertions(+), 7 deletions(-)

Comments

Frank Lichtenheld June 27, 2022, 11:19 a.m. UTC | #1
Acked-By: Frank Lichtenheld <frank@lichtenheld.com>

Code definitely looks like it will be doing what it is
intended to do.

Would be a good opportunity for a UT, though.

On Fri, May 27, 2022 at 03:24:57AM +0200, Heiko Hund wrote:
> Override DNS related foreign_options with values set by the --dns
> option. This is done, so that scripts looking for these options continue

Superflouos comma.

> to work if only --dns option were pushed, or the values in the
> --dhcp-options differ fron what's pushed in --dns.

"from"

> Signed-off-by: Heiko Hund <heiko@ist.eigentlich.net>
> ---
>  src/openvpn/openvpn.c |  2 +-
>  src/openvpn/options.c | 88 ++++++++++++++++++++++++++++++++++++++++---
>  src/openvpn/options.h |  2 +-
>  3 files changed, 85 insertions(+), 7 deletions(-)
> 
> diff --git a/src/openvpn/openvpn.c b/src/openvpn/openvpn.c
> index a6389fed..15e21452 100644
> --- a/src/openvpn/openvpn.c
> +++ b/src/openvpn/openvpn.c
> @@ -248,7 +248,7 @@ openvpn_main(int argc, char *argv[])
>              }
>  
>              /* sanity check on options */
> -            options_postprocess(&c.options);
> +            options_postprocess(&c.options, c.es);
>  
>              /* show all option settings */
>              show_settings(&c.options);
> diff --git a/src/openvpn/options.c b/src/openvpn/options.c
> index 9a0634a5..750444fe 100644
> --- a/src/openvpn/options.c
> +++ b/src/openvpn/options.c
> @@ -1381,6 +1381,80 @@ tuntap_options_copy_dns(struct options *o)
>          }
>      }
>  }
> +#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];
> +        openvpn_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)
> +    {
> +        if (server->addr4_defined)
> +        {
> +            const char *argv[] = {
> +                "dhcp-option",
> +                "DNS",
> +                print_in_addr_t(server->addr4.s_addr, 0, &gc)
> +            };
> +            setenv_foreign_option(o, argv, 3, es);
> +        }
> +        if (server->addr6_defined)
> +        {
> +            const char *argv[] = {
> +                "dhcp-option",
> +                "DNS6",
> +                print_in6_addr(server->addr6, 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];
> +        openvpn_snprintf(name, sizeof(name), "foreign_option_%d", opt_max--);
> +        setenv_del(es, name);
> +    }
> +}
>  #endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
>  
>  #ifndef ENABLE_SMALL
> @@ -3368,7 +3442,7 @@ options_set_backwards_compatible_options(struct options *o)
>  }
>  
>  static void
> -options_postprocess_mutate(struct options *o)
> +options_postprocess_mutate(struct options *o, struct env_set *es)
>  {
>      int i;
>      /*
> @@ -3462,12 +3536,14 @@ options_postprocess_mutate(struct options *o)
>      {
>          dns_options_preprocess_pull(&o->dns_options);
>      }
> -#if defined(_WIN32) || defined(TARGET_ANDROID)
>      else
>      {
> +#if defined(_WIN32) || defined(TARGET_ANDROID)
>          tuntap_options_copy_dns(o);
> -    }
> +#else
> +        foreign_options_copy_dns(o, es);
>  #endif
> +    }
>      pre_connect_save(o);
>  }
>  
> @@ -3803,9 +3879,9 @@ options_postprocess_filechecks(struct options *options)
>   * options.
>   */
>  void
> -options_postprocess(struct options *options)
> +options_postprocess(struct options *options, struct env_set *es)
>  {
> -    options_postprocess_mutate(options);
> +    options_postprocess_mutate(options, es);
>      options_postprocess_verify(options);
>  #ifndef ENABLE_SMALL
>      options_postprocess_filechecks(options);
> @@ -3826,6 +3902,8 @@ options_postprocess_pull(struct options *o, struct env_set *es)
>          setenv_dns_options(&o->dns_options, es);
>  #if defined(_WIN32) || defined(TARGET_ANDROID)
>          tuntap_options_copy_dns(o);
> +#else
> +        foreign_options_copy_dns(o, es);
>  #endif
>      }
>      return success;
> diff --git a/src/openvpn/options.h b/src/openvpn/options.h
> index c2937dc3..0e50c19e 100644
> --- a/src/openvpn/options.h
> +++ b/src/openvpn/options.h
> @@ -813,7 +813,7 @@ char *options_string_extract_option(const char *options_string,
>                                      const char *opt_name, struct gc_arena *gc);
>  
>  
> -void options_postprocess(struct options *options);
> +void options_postprocess(struct options *options, struct env_set *es);
>  
>  bool options_postprocess_pull(struct options *o, struct env_set *es);
>  
> -- 
> 2.32.0
> 
> 
> 
> _______________________________________________
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Gert Doering June 28, 2022, 12:13 p.m. UTC | #2
I have stared at the code (... weeks ago already) but never came around
to actually test this - so, happy to see the ACK from Frank :-)

Your patch has been applied to the master branch.

commit 8d345ff16db8d797a1b8485c65361e2949fd15d3
Author: Heiko Hund
Date:   Fri May 27 03:24:57 2022 +0200

     dns: also (re)place foreign dhcp options in env

     Signed-off-by: Heiko Hund <heiko@ist.eigentlich.net>
     Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
     Message-Id: <20220527012457.1819262-5-heiko@ist.eigentlich.net>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24432.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
Gert Doering June 28, 2022, 12:18 p.m. UTC | #3
Hi,

On Tue, Jun 28, 2022 at 02:13:29PM +0200, Gert Doering wrote:
> I have stared at the code (... weeks ago already) but never came around
> to actually test this - so, happy to see the ACK from Frank :-)
> 
> Your patch has been applied to the master branch.
> 
> commit 8d345ff16db8d797a1b8485c65361e2949fd15d3
> Author: Heiko Hund
> Date:   Fri May 27 03:24:57 2022 +0200

Uh.  Did some last-last minute fixes of the commit message ("Fron"),
so the actual commit ID changed again.

commit dac85fff2e4588fd922927737a2aafcbd9c9766e (HEAD -> master)
Author: Heiko Hund <heiko@ist.eigentlich.net>
Date:   Fri May 27 03:24:57 2022 +0200

    dns: also (re)place foreign dhcp options in env

sorry for the confusion.

gert

Patch

diff --git a/src/openvpn/openvpn.c b/src/openvpn/openvpn.c
index a6389fed..15e21452 100644
--- a/src/openvpn/openvpn.c
+++ b/src/openvpn/openvpn.c
@@ -248,7 +248,7 @@  openvpn_main(int argc, char *argv[])
             }
 
             /* sanity check on options */
-            options_postprocess(&c.options);
+            options_postprocess(&c.options, c.es);
 
             /* show all option settings */
             show_settings(&c.options);
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 9a0634a5..750444fe 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1381,6 +1381,80 @@  tuntap_options_copy_dns(struct options *o)
         }
     }
 }
+#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];
+        openvpn_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)
+    {
+        if (server->addr4_defined)
+        {
+            const char *argv[] = {
+                "dhcp-option",
+                "DNS",
+                print_in_addr_t(server->addr4.s_addr, 0, &gc)
+            };
+            setenv_foreign_option(o, argv, 3, es);
+        }
+        if (server->addr6_defined)
+        {
+            const char *argv[] = {
+                "dhcp-option",
+                "DNS6",
+                print_in6_addr(server->addr6, 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];
+        openvpn_snprintf(name, sizeof(name), "foreign_option_%d", opt_max--);
+        setenv_del(es, name);
+    }
+}
 #endif /* if defined(_WIN32) || defined(TARGET_ANDROID) */
 
 #ifndef ENABLE_SMALL
@@ -3368,7 +3442,7 @@  options_set_backwards_compatible_options(struct options *o)
 }
 
 static void
-options_postprocess_mutate(struct options *o)
+options_postprocess_mutate(struct options *o, struct env_set *es)
 {
     int i;
     /*
@@ -3462,12 +3536,14 @@  options_postprocess_mutate(struct options *o)
     {
         dns_options_preprocess_pull(&o->dns_options);
     }
-#if defined(_WIN32) || defined(TARGET_ANDROID)
     else
     {
+#if defined(_WIN32) || defined(TARGET_ANDROID)
         tuntap_options_copy_dns(o);
-    }
+#else
+        foreign_options_copy_dns(o, es);
 #endif
+    }
     pre_connect_save(o);
 }
 
@@ -3803,9 +3879,9 @@  options_postprocess_filechecks(struct options *options)
  * options.
  */
 void
-options_postprocess(struct options *options)
+options_postprocess(struct options *options, struct env_set *es)
 {
-    options_postprocess_mutate(options);
+    options_postprocess_mutate(options, es);
     options_postprocess_verify(options);
 #ifndef ENABLE_SMALL
     options_postprocess_filechecks(options);
@@ -3826,6 +3902,8 @@  options_postprocess_pull(struct options *o, struct env_set *es)
         setenv_dns_options(&o->dns_options, es);
 #if defined(_WIN32) || defined(TARGET_ANDROID)
         tuntap_options_copy_dns(o);
+#else
+        foreign_options_copy_dns(o, es);
 #endif
     }
     return success;
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index c2937dc3..0e50c19e 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -813,7 +813,7 @@  char *options_string_extract_option(const char *options_string,
                                     const char *opt_name, struct gc_arena *gc);
 
 
-void options_postprocess(struct options *options);
+void options_postprocess(struct options *options, struct env_set *es);
 
 bool options_postprocess_pull(struct options *o, struct env_set *es);