| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250316111516.4vxnot4osduc7oeh@House.clients.dxld.at>
Date: Sun, 16 Mar 2025 12:15:16 +0100
From: Daniel Gröber <dxld@...kboxed.org>
To: greearb@...delatech.com
Cc: "Jason A. Donenfeld" <Jason@...c4.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
dsahern@...nel.org, wireguard@...ts.zx2c4.com,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] net: wireguard: Allow binding to specific ifindex
Hi Ben,
I sent a more general patch adding essentially same feature quite a while
ago (Nov 2023) "wireguard: Add netlink attrs for binding to address and
netdev"
https://lore.kernel.org/netdev/20240219114334.3057169-1-dxld@darkboxed.org/T/
Which also came with ready-to-go wireguard-tools userspace support.
Unfortunately I never got any real feedback on it.
I managed to catch Jason at FOSDEM this year at least and he seems to not
be convinced either address or inteface binding are useful or necessary
features for wg.
The 1) "scaling" argument in the v3 patch was shot down due to Linux not
being scalable in the number of interfaces due to linked lists anyway,
which is fair if true. I added that argument in v3 to make the patch more
apealing, previously I had some handwaving about multihoming
https://lists.zx2c4.com/pipermail/wireguard/2023-November/008256.html :-)
I still think there's plenty of operational reasons to want to do this
outside of that consideration and I just seem to be failing to communicate
them in a way that appeals to Jason's design beauty aesthetic.
Perhaps you could explain your use-case in more depth to make the technical
argument stronger than what I can muster? :-)
Quickly comparing your code with mine I see you set fl.flowi*_oif in
addition to bind_ifindex, did you find that necessary for packets to be
emited on the right device?
Thanks,
--Daniel
PS: I'm aware the v3 patch proably still has a subtle uninitialized memory
problem, just haven't had the motivation to work on it due to lack of
feedback.
PPS: Sorry for the late response I don't read netdev regularly and
apparently Jason's wireguard list has been more broken than I've been aware
of. I've got some sieve hacks to get wireguard mail from netdev now.
On Tue, Dec 03, 2024 at 11:39:39AM -0800, greearb@...delatech.com wrote:
> From: Ben Greear <greearb@...delatech.com>
>
> Which allows us to bind to VRF.
>
> Signed-off-by: Ben Greear <greearb@...delatech.com>
> ---
>
> v2: Fix bad use of comma, semicolon now used instead.
>
> drivers/net/wireguard/device.h | 1 +
> drivers/net/wireguard/netlink.c | 12 +++++++++++-
> drivers/net/wireguard/socket.c | 8 +++++++-
> include/uapi/linux/wireguard.h | 3 +++
> 4 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireguard/device.h b/drivers/net/wireguard/device.h
> index 43c7cebbf50b..9698d9203915 100644
> --- a/drivers/net/wireguard/device.h
> +++ b/drivers/net/wireguard/device.h
> @@ -53,6 +53,7 @@ struct wg_device {
> atomic_t handshake_queue_len;
> unsigned int num_peers, device_update_gen;
> u32 fwmark;
> + int lowerdev; /* ifindex of lower level device to bind UDP transport */
> u16 incoming_port;
> };
>
> diff --git a/drivers/net/wireguard/netlink.c b/drivers/net/wireguard/netlink.c
> index f7055180ba4a..5de3d59a17b0 100644
> --- a/drivers/net/wireguard/netlink.c
> +++ b/drivers/net/wireguard/netlink.c
> @@ -27,7 +27,8 @@ static const struct nla_policy device_policy[WGDEVICE_A_MAX + 1] = {
> [WGDEVICE_A_FLAGS] = { .type = NLA_U32 },
> [WGDEVICE_A_LISTEN_PORT] = { .type = NLA_U16 },
> [WGDEVICE_A_FWMARK] = { .type = NLA_U32 },
> - [WGDEVICE_A_PEERS] = { .type = NLA_NESTED }
> + [WGDEVICE_A_PEERS] = { .type = NLA_NESTED },
> + [WGDEVICE_A_LOWERDEV] = { .type = NLA_U32 },
> };
>
> static const struct nla_policy peer_policy[WGPEER_A_MAX + 1] = {
> @@ -232,6 +233,7 @@ static int wg_get_device_dump(struct sk_buff *skb, struct netlink_callback *cb)
> if (nla_put_u16(skb, WGDEVICE_A_LISTEN_PORT,
> wg->incoming_port) ||
> nla_put_u32(skb, WGDEVICE_A_FWMARK, wg->fwmark) ||
> + nla_put_u32(skb, WGDEVICE_A_LOWERDEV, wg->lowerdev) ||
> nla_put_u32(skb, WGDEVICE_A_IFINDEX, wg->dev->ifindex) ||
> nla_put_string(skb, WGDEVICE_A_IFNAME, wg->dev->name))
> goto out;
> @@ -530,6 +532,14 @@ static int wg_set_device(struct sk_buff *skb, struct genl_info *info)
> wg_socket_clear_peer_endpoint_src(peer);
> }
>
> + if (info->attrs[WGDEVICE_A_LOWERDEV]) {
> + struct wg_peer *peer;
> +
> + wg->lowerdev = nla_get_u32(info->attrs[WGDEVICE_A_LOWERDEV]);
> + list_for_each_entry(peer, &wg->peer_list, peer_list)
> + wg_socket_clear_peer_endpoint_src(peer);
> + }
> +
> if (info->attrs[WGDEVICE_A_LISTEN_PORT]) {
> ret = set_port(wg,
> nla_get_u16(info->attrs[WGDEVICE_A_LISTEN_PORT]));
> diff --git a/drivers/net/wireguard/socket.c b/drivers/net/wireguard/socket.c
> index 0414d7a6ce74..7cef4b27f6ba 100644
> --- a/drivers/net/wireguard/socket.c
> +++ b/drivers/net/wireguard/socket.c
> @@ -25,7 +25,8 @@ static int send4(struct wg_device *wg, struct sk_buff *skb,
> .daddr = endpoint->addr4.sin_addr.s_addr,
> .fl4_dport = endpoint->addr4.sin_port,
> .flowi4_mark = wg->fwmark,
> - .flowi4_proto = IPPROTO_UDP
> + .flowi4_proto = IPPROTO_UDP,
> + .flowi4_oif = wg->lowerdev,
> };
> struct rtable *rt = NULL;
> struct sock *sock;
> @@ -111,6 +112,9 @@ static int send6(struct wg_device *wg, struct sk_buff *skb,
> struct sock *sock;
> int ret = 0;
>
> + if (wg->lowerdev)
> + fl.flowi6_oif = wg->lowerdev;
> +
> skb_mark_not_on_list(skb);
> skb->dev = wg->dev;
> skb->mark = wg->fwmark;
> @@ -360,6 +364,7 @@ int wg_socket_init(struct wg_device *wg, u16 port)
> .family = AF_INET,
> .local_ip.s_addr = htonl(INADDR_ANY),
> .local_udp_port = htons(port),
> + .bind_ifindex = wg->lowerdev,
> .use_udp_checksums = true
> };
> #if IS_ENABLED(CONFIG_IPV6)
> @@ -369,6 +374,7 @@ int wg_socket_init(struct wg_device *wg, u16 port)
> .local_ip6 = IN6ADDR_ANY_INIT,
> .use_udp6_tx_checksums = true,
> .use_udp6_rx_checksums = true,
> + .bind_ifindex = wg->lowerdev,
> .ipv6_v6only = true
> };
> #endif
> diff --git a/include/uapi/linux/wireguard.h b/include/uapi/linux/wireguard.h
> index ae88be14c947..f3784885389a 100644
> --- a/include/uapi/linux/wireguard.h
> +++ b/include/uapi/linux/wireguard.h
> @@ -29,6 +29,7 @@
> * WGDEVICE_A_PUBLIC_KEY: NLA_EXACT_LEN, len WG_KEY_LEN
> * WGDEVICE_A_LISTEN_PORT: NLA_U16
> * WGDEVICE_A_FWMARK: NLA_U32
> + * WGDEVICE_A_LOWERDEV: NLA_U32
> * WGDEVICE_A_PEERS: NLA_NESTED
> * 0: NLA_NESTED
> * WGPEER_A_PUBLIC_KEY: NLA_EXACT_LEN, len WG_KEY_LEN
> @@ -83,6 +84,7 @@
> * WGDEVICE_A_PRIVATE_KEY: len WG_KEY_LEN, all zeros to remove
> * WGDEVICE_A_LISTEN_PORT: NLA_U16, 0 to choose randomly
> * WGDEVICE_A_FWMARK: NLA_U32, 0 to disable
> + * WGDEVICE_A_LOWERDEV: NLA_U32, ifindex to bind lower transport, 0 to disable
> * WGDEVICE_A_PEERS: NLA_NESTED
> * 0: NLA_NESTED
> * WGPEER_A_PUBLIC_KEY: len WG_KEY_LEN
> @@ -157,6 +159,7 @@ enum wgdevice_attribute {
> WGDEVICE_A_LISTEN_PORT,
> WGDEVICE_A_FWMARK,
> WGDEVICE_A_PEERS,
> + WGDEVICE_A_LOWERDEV,
> __WGDEVICE_A_LAST
> };
> #define WGDEVICE_A_MAX (__WGDEVICE_A_LAST - 1)
> --
> 2.42.0
>
>
Powered by blists - more mailing lists