[Openvpn-devel,2/2] FreeBSD DCO: introduce real subnet mode

Message ID 20221012145915.25810-2-gert@greenie.muc.de
State Accepted
Headers show
Series [Openvpn-devel,1/2] FreeBSD: for topology subnet, put tun interface into IFF_BROADCAST mode | expand

Commit Message

Gert Doering Oct. 12, 2022, 2:59 p.m. UTC
To be able to configure a FreeBSD interface to "subnet" mode
(as opposed to point-to-point mode), it needs to have its
if_iflags set to IFF_BROADCAST.  For tun(4) interface this is
done with the TUNSIFMODE ioctl(), but this does not work for
more modern interfaces like ovpn(4) which communicate over
a common SIOCSDRVSPEC ioctl() that contains a "cmd" and a
"parameter list".

Introduce OVPN_SET_IFMODE cmd, add dco_set_ifmode() function
to put kernel interface into IFF_BROADCAST or IFF_POINTOPOINT
as needed.

(This needs a kernel patch to add the OVPN_SET_IFMODE on the
other side - with an older kernel, OpenVPN will just fail now)

Signed-off-by: Gert Doering <gert@greenie.muc.de>
---
 src/openvpn/dco_freebsd.c      | 36 ++++++++++++++++++++++++++++++++++
 src/openvpn/ovpn_dco_freebsd.h |  1 +
 2 files changed, 37 insertions(+)

Comments

Kristof Provost Oct. 17, 2022, 11:56 a.m. UTC | #1
Signed-off-by:	Kristof Provost <kprovost@netgate.com>

On 12 Oct 2022, at 16:59, Gert Doering wrote:

> To be able to configure a FreeBSD interface to "subnet" mode
> (as opposed to point-to-point mode), it needs to have its
> if_iflags set to IFF_BROADCAST.  For tun(4) interface this is
> done with the TUNSIFMODE ioctl(), but this does not work for
> more modern interfaces like ovpn(4) which communicate over
> a common SIOCSDRVSPEC ioctl() that contains a "cmd" and a
> "parameter list".
>
> Introduce OVPN_SET_IFMODE cmd, add dco_set_ifmode() function
> to put kernel interface into IFF_BROADCAST or IFF_POINTOPOINT
> as needed.
>
> (This needs a kernel patch to add the OVPN_SET_IFMODE on the
> other side - with an older kernel, OpenVPN will just fail now)
>
> Signed-off-by: Gert Doering <gert@greenie.muc.de>
> ---
>  src/openvpn/dco_freebsd.c      | 36 ++++++++++++++++++++++++++++++++++
>  src/openvpn/ovpn_dco_freebsd.h |  1 +
>  2 files changed, 37 insertions(+)
>
> diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
> index c6da6ce3..8adbf7f1 100644
> --- a/src/openvpn/dco_freebsd.c
> +++ b/src/openvpn/dco_freebsd.c
> @@ -165,6 +165,34 @@ ovpn_dco_init(int mode, dco_context_t *dco)
>      return true;
>  }
>
> +static int
> +dco_set_ifmode(dco_context_t *dco, int ifmode)
> +{
> +    struct ifdrv drv;
> +    nvlist_t *nvl;
> +    int ret;
> +
> +    msg(M_INFO, "ifmode=%08x", ifmode);
> +    nvl = nvlist_create(0);
> +    nvlist_add_number(nvl, "ifmode", ifmode);
> +
> +    CLEAR(drv);
> +    snprintf(drv.ifd_name, IFNAMSIZ, "%s", dco->ifname);
> +    drv.ifd_cmd = OVPN_SET_IFMODE;
> +    drv.ifd_data = nvlist_pack(nvl, &drv.ifd_len);
> +
> +    ret = ioctl(dco->fd, SIOCSDRVSPEC, &drv);
> +    if (ret)
> +    {
> +        msg(M_WARN | M_ERRNO, "Failed to set ifmode");
> +    }
> +
> +    free(drv.ifd_data);
> +    nvlist_destroy(nvl);
> +
> +    return ret;
> +}
> +
>  static int
>  create_interface(struct tuntap *tt, const char *dev)
>  {
> @@ -205,6 +233,14 @@ create_interface(struct tuntap *tt, const char *dev)
>      snprintf(tt->dco.ifname, IFNAMSIZ, "%s", ifr.ifr_data);
>      tt->actual_name = string_alloc(tt->dco.ifname, NULL);
>
> +    /* see "Interface Flags" in ifnet(9) */
> +    int i = IFF_POINTOPOINT | IFF_MULTICAST;
> +    if (tt->topology == TOP_SUBNET)
> +    {
> +        i = IFF_BROADCAST | IFF_MULTICAST;
> +    }
> +    dco_set_ifmode(&tt->dco, i);
> +
>      return 0;
>  }
>
> diff --git a/src/openvpn/ovpn_dco_freebsd.h b/src/openvpn/ovpn_dco_freebsd.h
> index 7ceec06e..cf92d597 100644
> --- a/src/openvpn/ovpn_dco_freebsd.h
> +++ b/src/openvpn/ovpn_dco_freebsd.h
> @@ -60,5 +60,6 @@ enum ovpn_key_cipher {
>  #define OVPN_SEND_PKT           _IO('D', 9)
>  #define OVPN_POLL_PKT           _IO('D', 10)
>  #define OVPN_GET_PKT            _IO('D', 11)
> +#define OVPN_SET_IFMODE         _IO('D', 12)
>
>  #endif /* ifndef _NET_IF_OVPN_H_ */
> -- 
> 2.37.3
>
>
>
> _______________________________________________
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Gert Doering Oct. 19, 2022, 9:09 a.m. UTC | #2
Thanks, Kristof for the review, and for merging the kernel side.

I have added the kernel commit ID to the commit message for this
patch ("if the kernel is too old, SET_IFMODE will fail and 
topology subnet will not work") - and verified that this is indeed
the result, p2p mode with net30 works, subnet fails.  And of course,
tested with a newer if_ovpn.ko, and all works :-)

I have also taken the liberty to remove the debug message printing
the ifmode to-be-set, and to add the ifmode + 'dco_set_ifmode:' to
the error message, so log file output is more useful.

Patch has been applied to the master branch.

commit 22bc63c78439ed23b974b8f822330d75ec79c7fc
Author: Gert Doering
Date:   Wed Oct 12 16:59:15 2022 +0200

     FreeBSD DCO: introduce real subnet mode

     Signed-off-by: Gert Doering <gert@greenie.muc.de>
     Acked-by: Kristof Provost <kprovost@netgate.com>
     Message-Id: <20221012145915.25810-2-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25395.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
index c6da6ce3..8adbf7f1 100644
--- a/src/openvpn/dco_freebsd.c
+++ b/src/openvpn/dco_freebsd.c
@@ -165,6 +165,34 @@  ovpn_dco_init(int mode, dco_context_t *dco)
     return true;
 }
 
+static int
+dco_set_ifmode(dco_context_t *dco, int ifmode)
+{
+    struct ifdrv drv;
+    nvlist_t *nvl;
+    int ret;
+
+    msg(M_INFO, "ifmode=%08x", ifmode);
+    nvl = nvlist_create(0);
+    nvlist_add_number(nvl, "ifmode", ifmode);
+
+    CLEAR(drv);
+    snprintf(drv.ifd_name, IFNAMSIZ, "%s", dco->ifname);
+    drv.ifd_cmd = OVPN_SET_IFMODE;
+    drv.ifd_data = nvlist_pack(nvl, &drv.ifd_len);
+
+    ret = ioctl(dco->fd, SIOCSDRVSPEC, &drv);
+    if (ret)
+    {
+        msg(M_WARN | M_ERRNO, "Failed to set ifmode");
+    }
+
+    free(drv.ifd_data);
+    nvlist_destroy(nvl);
+
+    return ret;
+}
+
 static int
 create_interface(struct tuntap *tt, const char *dev)
 {
@@ -205,6 +233,14 @@  create_interface(struct tuntap *tt, const char *dev)
     snprintf(tt->dco.ifname, IFNAMSIZ, "%s", ifr.ifr_data);
     tt->actual_name = string_alloc(tt->dco.ifname, NULL);
 
+    /* see "Interface Flags" in ifnet(9) */
+    int i = IFF_POINTOPOINT | IFF_MULTICAST;
+    if (tt->topology == TOP_SUBNET)
+    {
+        i = IFF_BROADCAST | IFF_MULTICAST;
+    }
+    dco_set_ifmode(&tt->dco, i);
+
     return 0;
 }
 
diff --git a/src/openvpn/ovpn_dco_freebsd.h b/src/openvpn/ovpn_dco_freebsd.h
index 7ceec06e..cf92d597 100644
--- a/src/openvpn/ovpn_dco_freebsd.h
+++ b/src/openvpn/ovpn_dco_freebsd.h
@@ -60,5 +60,6 @@  enum ovpn_key_cipher {
 #define OVPN_SEND_PKT           _IO('D', 9)
 #define OVPN_POLL_PKT           _IO('D', 10)
 #define OVPN_GET_PKT            _IO('D', 11)
+#define OVPN_SET_IFMODE         _IO('D', 12)
 
 #endif /* ifndef _NET_IF_OVPN_H_ */