diff mbox series

[Openvpn-devel,2/2] set_lladdr: use networking API net_addr_ll_set() on Linux

Message ID 20210903161113.30498-2-a@unstable.cc
State Accepted
Headers show
Series [Openvpn-devel,1/2] networking: add and implement net_addr_ll_set() API | expand

Commit Message

Antonio Quartulli Sept. 3, 2021, 6:11 a.m. UTC 
Make sure that set_addr() uses the proper networking backend when
setting the LL address of a TAP interface.

This operation was overlooked while implementing the networking APIs on
the Linux platform.

Reported-by: Jan Hugo Prins <jprins@betterbe.com>
Signed-off-by: Antonio Quartulli <a@unstable.cc>
---

This patch (along with 1/2) has been tested on buildbots, but I couldn't
run any specific test on any *BSD or Windows platform.

Linux with both iproute2 and sitnl works as expected.


 src/openvpn/init.c   |  5 +++--
 src/openvpn/lladdr.c | 30 +++++++++++++-----------------
 src/openvpn/lladdr.h |  3 ++-
 3 files changed, 18 insertions(+), 20 deletions(-)

Comments

Gert Doering Sept. 29, 2021, 7:39 a.m. UTC | #1 
Acked-by: Gert Doering <gert@greenie.muc.de>

Stared at the code, tested on FreeBSD, Linux/iproute2 and Linux/sitnl,
MAC addresses are properly set.

As discussed on IRC a while ago, I have added brackets to the
ugly assignment

      r = (net_addr_ll_set(ctx, ifname, addr) == 0);

and I have added the "Tested-By:" line from Jan Hugo Prins.

Your patch has been applied to the master branch, and pulled up
(3 squashed commits) to release/2.5

commit 7205cdd8508be0ec9a83ea2e012e2a495157cad0 (master)
commit e2fded0f72765467fd0d602562bbf41d48c58d39 (release/2.5)
Author: Antonio Quartulli
Date:   Fri Sep 3 18:11:13 2021 +0200

     set_lladdr: use networking API net_addr_ll_set() on Linux

     Signed-off-by: Antonio Quartulli <a@unstable.cc>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20210903161113.30498-2-a@unstable.cc>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22791.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 386aee23..a17fe859 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1176,7 +1176,7 @@  do_persist_tuntap(const struct options *options, openvpn_net_ctx_t *ctx)
                ctx);
         if (options->persist_mode && options->lladdr)
         {
-            set_lladdr(options->dev, options->lladdr, NULL);
+            set_lladdr(ctx, options->dev, options->lladdr, NULL);
         }
         return true;
 #else  /* ifdef ENABLE_FEATURE_TUN_PERSIST */
@@ -1853,7 +1853,8 @@  do_open_tun(struct context *c)
     /* set the hardware address */
     if (c->options.lladdr)
     {
-        set_lladdr(c->c1.tuntap->actual_name, c->options.lladdr, c->c2.es);
+        set_lladdr(&c->net_ctx, c->c1.tuntap->actual_name, c->options.lladdr,
+                   c->c2.es);
     }
 
     /* do ifconfig */
diff --git a/src/openvpn/lladdr.c b/src/openvpn/lladdr.c
index 22857eb7..f12c146f 100644
--- a/src/openvpn/lladdr.c
+++ b/src/openvpn/lladdr.c
@@ -15,10 +15,9 @@ 
 #include "lladdr.h"
 
 int
-set_lladdr(const char *ifname, const char *lladdr,
+set_lladdr(openvpn_net_ctx_t *ctx, const char *ifname, const char *lladdr,
            const struct env_set *es)
 {
-    struct argv argv = argv_new();
     int r;
 
     if (!ifname || !lladdr)
@@ -27,17 +26,13 @@  set_lladdr(const char *ifname, const char *lladdr,
     }
 
 #if defined(TARGET_LINUX)
-#ifdef ENABLE_IPROUTE
-    argv_printf(&argv,
-                "%s link set addr %s dev %s",
-                iproute_path, lladdr, ifname);
-#else
-    argv_printf(&argv,
-                "%s %s hw ether %s",
-                IFCONFIG_PATH,
-                ifname, lladdr);
-#endif
-#elif defined(TARGET_SOLARIS)
+    uint8_t addr[ETH_ALEN];
+
+    sscanf(lladdr, MAC_FMT, MAC_SCAN_ARG(addr));
+    r = net_addr_ll_set(ctx, ifname, addr) == 0;
+#else /* if defined(TARGET_LINUX) */
+    struct argv argv = argv_new();
+#if defined(TARGET_SOLARIS)
     argv_printf(&argv,
                 "%s %s ether %s",
                 IFCONFIG_PATH,
@@ -57,18 +52,19 @@  set_lladdr(const char *ifname, const char *lladdr,
                 "%s %s ether %s",
                 IFCONFIG_PATH,
                 ifname, lladdr);
-#else  /* if defined(TARGET_LINUX) */
+#else  /* if defined(TARGET_SOLARIS) */
     msg(M_WARN, "Sorry, but I don't know how to configure link layer addresses on this operating system.");
     return -1;
-#endif /* if defined(TARGET_LINUX) */
-
+#endif /* if defined(TARGET_SOLARIS) */
     argv_msg(M_INFO, &argv);
     r = openvpn_execve_check(&argv, es, M_WARN, "ERROR: Unable to set link layer address.");
+    argv_free(&argv);
+#endif /* if defined(TARGET_LINUX) */
+
     if (r)
     {
         msg(M_INFO, "TUN/TAP link layer address set to %s", lladdr);
     }
 
-    argv_free(&argv);
     return r;
 }
diff --git a/src/openvpn/lladdr.h b/src/openvpn/lladdr.h
index f6ea2b12..0c8b4164 100644
--- a/src/openvpn/lladdr.h
+++ b/src/openvpn/lladdr.h
@@ -3,6 +3,7 @@ 
  */
 
 #include "misc.h"
+#include "networking.h"
 
-int set_lladdr(const char *ifname, const char *lladdr,
+int set_lladdr(openvpn_net_ctx_t *ctx, const char *ifname, const char *lladdr,
                const struct env_set *es);