[PATCH] Allow changing `creator_net` after interface creation.

[PATCH] Allow changing `creator_net` after interface creation.

[Maarten de Vries]

This allows the wireguard interface to be created inside an empty
network namespace, and still use a different namespace for the tunnel
socket.

This means that the interface will never be visible outside of the
intended namespace. This is particularly useful if the interface is to
be created on the fly in an anonymous network namespace. There is no
need to generate a unique name for the interface, and there is no risk
that any process outside of the new namespace tries to configure or
otherwise use the wireguard interface.

An example of how this can be used:

$ ip netns create foo
$ ip -n foo link add wg0 type wireguard
$ ip netns exec foo wg set wg0 tunnel-network-namespace /proc/1/ns/net
$ ip netns exec foo wg set ...
$ ip -n foo link set wg0 up

Signed-off-by: Maarten de Vries <maarten@de-vri.es>
---

Currently, for the kernel implementation, the interface must be down to
change the namespace. This is because no attempt is made to re-initialize
already created tunnel sockets. It may be possible to loosen this
requirement, but this seemed like the simplest approach.

For the netlink protocol, I named the attribute `..._TUNNEL_NETNS_FD`,
to keep the option open to also add `..._TUNNEL_NETNS_ID` and/or
`..._TUNNEL_NETNS_PID` without changing the user facing API.
These would identify the namespace by namespace ID or PID respectively.
PID seems pointless though, since you could just open /proc/$PID/ns/net
and pass the FD.

Note that this adds tunnel_netns=... to the userspace configuration
protocol too. The assumption is that userspace implementations will
reply with an error if they don't understand the option or if they can't
change the namespace.

I didn't change the name of `wg_device->creator_net`, although it may
make more sense to call it `tunnel_net` now.

Feedback and comments are welcome, of course.

 src/netlink.c          | 32 +++++++++++++++++++++++++++++++-
 src/tools/config.c     | 23 +++++++++++++++++++++++
 src/tools/containers.h |  3 +++
 src/tools/ipc.c        | 15 +++++++++++++++
 src/tools/set.c        |  2 +-
 src/uapi/wireguard.h   |  3 +++
 6 files changed, 76 insertions(+), 2 deletions(-)

diff --git a/src/netlink.c b/src/netlink.c
index 9a33192..82e9030 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -24,7 +24,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_TUNNEL_NETNS_FD]	= { .type = NLA_U32 }
 };
 
 static const struct nla_policy peer_policy[WGPEER_A_MAX + 1] = {
@@ -472,6 +473,27 @@ out:
 	return ret;
 }
 
+static int set_tunnel_netns(struct wg_device *wg, u32 fd)
+{
+	struct net *new_net;
+
+	if (wg->sock4 != NULL || wg->sock6 != NULL)
+		return -EINVAL;
+
+	new_net = get_net_ns_by_fd(fd);
+
+	if (IS_ERR(new_net))
+		return PTR_ERR(new_net);
+
+	if (wg->have_creating_net_ref)
+		put_net(wg->creating_net);
+
+	wg->have_creating_net_ref = true;
+	wg->creating_net = new_net;
+
+	return 0;
+}
+
 static int wg_set_device(struct sk_buff *skb, struct genl_info *info)
 {
 	struct wg_device *wg = lookup_interface(info->attrs, skb);
@@ -558,6 +580,14 @@ static int wg_set_device(struct sk_buff *skb, struct genl_info *info)
 				goto out;
 		}
 	}
+
+	if (info->attrs[WGDEVICE_A_TUNNEL_NETNS_FD]) {
+		int fd = nla_get_u32(info->attrs[WGDEVICE_A_TUNNEL_NETNS_FD]);
+		ret = set_tunnel_netns(wg, fd);
+		if (ret < 0)
+			goto out;
+	}
+
 	ret = 0;
 
 out:
diff --git a/src/tools/config.c b/src/tools/config.c
index 5d15356..95ece02 100644
--- a/src/tools/config.c
+++ b/src/tools/config.c
@@ -157,6 +157,22 @@ out:
 	return ret;
 }
 
+static inline bool parse_netns(char **netns, const char *value)
+{
+	char *dupped;
+
+	if (strchr(value, '\n'))
+		return false;
+
+	dupped = strdup(value);
+	if (!dupped)
+		return false;
+
+	free(*netns);
+	*netns = dupped;
+
+	return true;
+}
+
 static inline bool parse_ip(struct wgallowedip *allowedip, const char *value)
 {
 	allowedip->family = AF_UNSPEC;
@@ -394,6 +410,8 @@ static bool process_line(struct config_ctx *ctx, const char *line)
 			ret = parse_port(&ctx->device->listen_port, &ctx->device->flags, value);
 		else if (key_match("FwMark"))
 			ret = parse_fwmark(&ctx->device->fwmark, &ctx->device->flags, value);
+		else if (key_match("TunnelNetNS"))
+			ret = parse_netns(&ctx->device->tunnel_netns, value);
 		else if (key_match("PrivateKey")) {
 			ret = parse_key(ctx->device->private_key, value);
 			if (ret)
@@ -530,6 +548,11 @@ struct wgdevice *config_read_cmd(char *argv[], int argc)
 				goto error;
 			argv += 2;
 			argc -= 2;
+		} else if (!strcmp(argv[0], "tunnel-netns") && argc >= 2 && !peer) {
+			if (!parse_netns(&device->tunnel_netns, argv[1]))
+				goto error;
+			argv += 2;
+			argc -= 2;
 		} else if (!strcmp(argv[0], "private-key") && argc >= 2 && !peer) {
 			if (!parse_keyfile(device->private_key, argv[1]))
 				goto error;
diff --git a/src/tools/containers.h b/src/tools/containers.h
index 59a213e..ea6eaf2 100644
--- a/src/tools/containers.h
+++ b/src/tools/containers.h
@@ -79,6 +79,8 @@ struct wgdevice {
 	uint32_t fwmark;
 	uint16_t listen_port;
 
+	char *tunnel_netns;
+
 	struct wgpeer *first_peer, *last_peer;
 };
 
@@ -89,6 +91,7 @@ static inline void free_wgdevice(struct wgdevice *dev)
 {
 	if (!dev)
 		return;
+	free(dev->tunnel_netns);
 	for (struct wgpeer *peer = dev->first_peer, *np = peer ? peer->next_peer : NULL; peer; peer = np, np = peer ? peer->next_peer : NULL) {
 		for (struct wgallowedip *allowedip = peer->first_allowedip, *na = allowedip ? allowedip->next_allowedip : NULL; allowedip; allowedip = na, na = allowedip ? allowedip->next_allowedip : NULL)
 			free(allowedip);
diff --git a/src/tools/ipc.c b/src/tools/ipc.c
index 7ab3a62..7cf9d3e 100644
--- a/src/tools/ipc.c
+++ b/src/tools/ipc.c
@@ -15,6 +15,7 @@
 #include <sys/socket.h>
 #include <net/if.h>
 #include <errno.h>
+#include <fcntl.h>
 #include <stdbool.h>
 #include <stddef.h>
 #include <stdio.h>
@@ -220,6 +221,8 @@ static int userspace_set_device(struct wgdevice *dev)
 		fprintf(f, "listen_port=%u\n", dev->listen_port);
 	if (dev->flags & WGDEVICE_HAS_FWMARK)
 		fprintf(f, "fwmark=%u\n", dev->fwmark);
+	if (dev->tunnel_netns)
+		fprintf(f, "tunnel_netns=%s\n", dev->tunnel_netns);
 	if (dev->flags & WGDEVICE_REPLACE_PEERS)
 		fprintf(f, "replace_peers=true\n");
 
@@ -559,11 +562,19 @@ static int kernel_set_device(struct wgdevice *dev)
 	struct nlattr *peers_nest, *peer_nest, *allowedips_nest, *allowedip_nest;
 	struct nlmsghdr *nlh;
 	struct mnlg_socket *nlg;
+	int tunnel_netns_fd = -1;
 
 	nlg = mnlg_socket_open(WG_GENL_NAME, WG_GENL_VERSION);
 	if (!nlg)
 		return -errno;
 
+	if (dev->tunnel_netns) {
+		ret = tunnel_netns_fd = open(dev->tunnel_netns, O_RDONLY | O_CLOEXEC);
+		if (ret < 0)
+			goto out;
+		ret = 0;
+	}
+
 again:
 	nlh = mnlg_msg_prepare(nlg, WG_CMD_SET_DEVICE, NLM_F_REQUEST | NLM_F_ACK);
 	mnl_attr_put_strz(nlh, WGDEVICE_A_IFNAME, dev->name);
@@ -577,6 +588,8 @@ again:
 			mnl_attr_put_u16(nlh, WGDEVICE_A_LISTEN_PORT, dev->listen_port);
 		if (dev->flags & WGDEVICE_HAS_FWMARK)
 			mnl_attr_put_u32(nlh, WGDEVICE_A_FWMARK, dev->fwmark);
+		if (dev->tunnel_netns)
+			mnl_attr_put_u32(nlh, WGDEVICE_A_TUNNEL_NETNS_FD, tunnel_netns_fd);
 		if (dev->flags & WGDEVICE_REPLACE_PEERS)
 			flags |= WGDEVICE_F_REPLACE_PEERS;
 		if (flags)
@@ -681,6 +694,8 @@ send:
 
 out:
 	mnlg_socket_close(nlg);
+	if (tunnel_netns_fd > -1)
+		close(tunnel_netns_fd);
 	errno = -ret;
 	return ret;
 }
diff --git a/src/tools/set.c b/src/tools/set.c
index 19f4b92..26305da 100644
--- a/src/tools/set.c
+++ b/src/tools/set.c
@@ -18,7 +18,7 @@ int set_main(int argc, char *argv[])
 	int ret = 1;
 
 	if (argc < 3) {
-		fprintf(stderr, "Usage: %s %s <interface> [listen-port <port>] [fwmark <mark>] [private-key <file path>] [peer <base64 public key> [remove] [preshared-key <file path>] [endpoint <ip>:<port>] [persistent-keepalive <interval seconds>] [allowed-ips <ip1>/<cidr1>[,<ip2>/<cidr2>]...] ]...\n", PROG_NAME, argv[0]);
+		fprintf(stderr, "Usage: %s %s <interface> [listen-port <port>] [fwmark <mark>] [private-key <file path>] [peer <base64 public key> [remove] [preshared-key <file path>] [endpoint <ip>:<port>] [persistent-keepalive <interval seconds>] [tunnel-netns <netns path>] [allowed-ips <ip1>/<cidr1>[,<ip2>/<cidr2>]...] ]...\n", PROG_NAME, argv[0]);
 		return 1;
 	}
 
diff --git a/src/uapi/wireguard.h b/src/uapi/wireguard.h
index 071ce41..4ac5bf1 100644
--- a/src/uapi/wireguard.h
+++ b/src/uapi/wireguard.h
@@ -83,6 +83,8 @@
  *    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_TUNNEL_NETNS_FD: NLA_U32, fd referring to the netns to use for
+ *                                the tunnel sockets
  *    WGDEVICE_A_PEERS: NLA_NESTED
  *        0: NLA_NESTED
  *            WGPEER_A_PUBLIC_KEY: len WG_KEY_LEN
@@ -154,6 +156,7 @@ enum wgdevice_attribute {
 	WGDEVICE_A_LISTEN_PORT,
 	WGDEVICE_A_FWMARK,
 	WGDEVICE_A_PEERS,
+	WGDEVICE_A_TUNNEL_NETNS_FD,
 	__WGDEVICE_A_LAST
 };
 #define WGDEVICE_A_MAX (__WGDEVICE_A_LAST - 1)
-- 
2.20.1

_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

Re: [PATCH] Allow changing `creator_net` after interface creation.

[Julian Orth]

See [1] for a similar idea.

[1] https://lists.zx2c4.com/pipermail/wireguard/2018-December/003662.html
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

[PATCH] Allow changing `creator_net` after interface creation.

[Maarten de Vries]

This allows the wireguard interface to be created inside an empty
network namespace, and still use a different namespace for the tunnel
socket.

This means that the interface will never be visible outside of the
intended namespace. This is particularly useful if the interface is to
be created on the fly in an anonymous network namespace. There is no
need to generate a unique name for the interface, and there is no risk
that any process outside of the new namespace tries to configure or
otherwise use the wireguard interface.

An example of how this can be used:

$ ip netns create foo
$ ip -n foo link add wg0 type wireguard
$ ip netns exec foo wg set wg0 tunnel-netns /proc/1/ns/net
$ ip netns exec foo wg set ...
$ ip -n foo link set wg0 up

Signed-off-by: Maarten de Vries <maarten@de-vri.es>
---

Fixed the patch, previous one was broken by a manual edit. Also fixed an
error in the commit message.

 src/netlink.c          | 32 +++++++++++++++++++++++++++++++-
 src/tools/config.c     | 24 ++++++++++++++++++++++++
 src/tools/containers.h |  3 +++
 src/tools/ipc.c        | 15 +++++++++++++++
 src/tools/set.c        |  2 +-
 src/uapi/wireguard.h   |  3 +++
 6 files changed, 77 insertions(+), 2 deletions(-)

diff --git a/src/netlink.c b/src/netlink.c
index 9a33192..82e9030 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -24,7 +24,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_TUNNEL_NETNS_FD]	= { .type = NLA_U32 }
 };
 
 static const struct nla_policy peer_policy[WGPEER_A_MAX + 1] = {
@@ -472,6 +473,27 @@ out:
 	return ret;
 }
 
+static int set_tunnel_netns(struct wg_device *wg, u32 fd)
+{
+	struct net *new_net;
+
+	if (wg->sock4 != NULL || wg->sock6 != NULL)
+		return -EINVAL;
+
+	new_net = get_net_ns_by_fd(fd);
+
+	if (IS_ERR(new_net))
+		return PTR_ERR(new_net);
+
+	if (wg->have_creating_net_ref)
+		put_net(wg->creating_net);
+
+	wg->have_creating_net_ref = true;
+	wg->creating_net = new_net;
+
+	return 0;
+}
+
 static int wg_set_device(struct sk_buff *skb, struct genl_info *info)
 {
 	struct wg_device *wg = lookup_interface(info->attrs, skb);
@@ -558,6 +580,14 @@ static int wg_set_device(struct sk_buff *skb, struct genl_info *info)
 				goto out;
 		}
 	}
+
+	if (info->attrs[WGDEVICE_A_TUNNEL_NETNS_FD]) {
+		int fd = nla_get_u32(info->attrs[WGDEVICE_A_TUNNEL_NETNS_FD]);
+		ret = set_tunnel_netns(wg, fd);
+		if (ret < 0)
+			goto out;
+	}
+
 	ret = 0;
 
 out:
diff --git a/src/tools/config.c b/src/tools/config.c
index 5d15356..1328f74 100644
--- a/src/tools/config.c
+++ b/src/tools/config.c
@@ -157,6 +157,23 @@ out:
 	return ret;
 }
 
+static inline bool parse_netns(char **netns, const char *value)
+{
+	char *dupped;
+
+	if (strchr(value, '\n'))
+		return false;
+
+	dupped = strdup(value);
+	if (!dupped)
+		return false;
+
+	free(*netns);
+	*netns = dupped;
+
+	return true;
+}
+
 static inline bool parse_ip(struct wgallowedip *allowedip, const char *value)
 {
 	allowedip->family = AF_UNSPEC;
@@ -394,6 +411,8 @@ static bool process_line(struct config_ctx *ctx, const char *line)
 			ret = parse_port(&ctx->device->listen_port, &ctx->device->flags, value);
 		else if (key_match("FwMark"))
 			ret = parse_fwmark(&ctx->device->fwmark, &ctx->device->flags, value);
+		else if (key_match("TunnelNetNS"))
+			ret = parse_netns(&ctx->device->tunnel_netns, value);
 		else if (key_match("PrivateKey")) {
 			ret = parse_key(ctx->device->private_key, value);
 			if (ret)
@@ -530,6 +549,11 @@ struct wgdevice *config_read_cmd(char *argv[], int argc)
 				goto error;
 			argv += 2;
 			argc -= 2;
+		} else if (!strcmp(argv[0], "tunnel-netns") && argc >= 2 && !peer) {
+			if (!parse_netns(&device->tunnel_netns, argv[1]))
+				goto error;
+			argv += 2;
+			argc -= 2;
 		} else if (!strcmp(argv[0], "private-key") && argc >= 2 && !peer) {
 			if (!parse_keyfile(device->private_key, argv[1]))
 				goto error;
diff --git a/src/tools/containers.h b/src/tools/containers.h
index 59a213e..ea6eaf2 100644
--- a/src/tools/containers.h
+++ b/src/tools/containers.h
@@ -79,6 +79,8 @@ struct wgdevice {
 	uint32_t fwmark;
 	uint16_t listen_port;
 
+	char *tunnel_netns;
+
 	struct wgpeer *first_peer, *last_peer;
 };
 
@@ -89,6 +91,7 @@ static inline void free_wgdevice(struct wgdevice *dev)
 {
 	if (!dev)
 		return;
+	free(dev->tunnel_netns);
 	for (struct wgpeer *peer = dev->first_peer, *np = peer ? peer->next_peer : NULL; peer; peer = np, np = peer ? peer->next_peer : NULL) {
 		for (struct wgallowedip *allowedip = peer->first_allowedip, *na = allowedip ? allowedip->next_allowedip : NULL; allowedip; allowedip = na, na = allowedip ? allowedip->next_allowedip : NULL)
 			free(allowedip);
diff --git a/src/tools/ipc.c b/src/tools/ipc.c
index 7ab3a62..7cf9d3e 100644
--- a/src/tools/ipc.c
+++ b/src/tools/ipc.c
@@ -15,6 +15,7 @@
 #include <sys/socket.h>
 #include <net/if.h>
 #include <errno.h>
+#include <fcntl.h>
 #include <stdbool.h>
 #include <stddef.h>
 #include <stdio.h>
@@ -220,6 +221,8 @@ static int userspace_set_device(struct wgdevice *dev)
 		fprintf(f, "listen_port=%u\n", dev->listen_port);
 	if (dev->flags & WGDEVICE_HAS_FWMARK)
 		fprintf(f, "fwmark=%u\n", dev->fwmark);
+	if (dev->tunnel_netns)
+		fprintf(f, "tunnel_netns=%s\n", dev->tunnel_netns);
 	if (dev->flags & WGDEVICE_REPLACE_PEERS)
 		fprintf(f, "replace_peers=true\n");
 
@@ -559,11 +562,19 @@ static int kernel_set_device(struct wgdevice *dev)
 	struct nlattr *peers_nest, *peer_nest, *allowedips_nest, *allowedip_nest;
 	struct nlmsghdr *nlh;
 	struct mnlg_socket *nlg;
+	int tunnel_netns_fd = -1;
 
 	nlg = mnlg_socket_open(WG_GENL_NAME, WG_GENL_VERSION);
 	if (!nlg)
 		return -errno;
 
+	if (dev->tunnel_netns) {
+		ret = tunnel_netns_fd = open(dev->tunnel_netns, O_RDONLY | O_CLOEXEC);
+		if (ret < 0)
+			goto out;
+		ret = 0;
+	}
+
 again:
 	nlh = mnlg_msg_prepare(nlg, WG_CMD_SET_DEVICE, NLM_F_REQUEST | NLM_F_ACK);
 	mnl_attr_put_strz(nlh, WGDEVICE_A_IFNAME, dev->name);
@@ -577,6 +588,8 @@ again:
 			mnl_attr_put_u16(nlh, WGDEVICE_A_LISTEN_PORT, dev->listen_port);
 		if (dev->flags & WGDEVICE_HAS_FWMARK)
 			mnl_attr_put_u32(nlh, WGDEVICE_A_FWMARK, dev->fwmark);
+		if (dev->tunnel_netns)
+			mnl_attr_put_u32(nlh, WGDEVICE_A_TUNNEL_NETNS_FD, tunnel_netns_fd);
 		if (dev->flags & WGDEVICE_REPLACE_PEERS)
 			flags |= WGDEVICE_F_REPLACE_PEERS;
 		if (flags)
@@ -681,6 +694,8 @@ send:
 
 out:
 	mnlg_socket_close(nlg);
+	if (tunnel_netns_fd > -1)
+		close(tunnel_netns_fd);
 	errno = -ret;
 	return ret;
 }
diff --git a/src/tools/set.c b/src/tools/set.c
index 19f4b92..b7c5cb2 100644
--- a/src/tools/set.c
+++ b/src/tools/set.c
@@ -18,7 +18,7 @@ int set_main(int argc, char *argv[])
 	int ret = 1;
 
 	if (argc < 3) {
-		fprintf(stderr, "Usage: %s %s <interface> [listen-port <port>] [fwmark <mark>] [private-key <file path>] [peer <base64 public key> [remove] [preshared-key <file path>] [endpoint <ip>:<port>] [persistent-keepalive <interval seconds>] [allowed-ips <ip1>/<cidr1>[,<ip2>/<cidr2>]...] ]...\n", PROG_NAME, argv[0]);
+		fprintf(stderr, "Usage: %s %s <interface> [listen-port <port>] [fwmark <mark>] [private-key <file path>] [peer <base64 public key> [remove] [preshared-key <file path>] [endpoint <ip>:<port>] [persistent-keepalive <interval seconds>] [tunnel-netns <netns path>] [allowed-ips <ip1>/<cidr1>[,<ip2>/<cidr2>]...] ]...\n", PROG_NAME, argv[0]);
 		return 1;
 	}
 
diff --git a/src/uapi/wireguard.h b/src/uapi/wireguard.h
index 071ce41..a019c05 100644
--- a/src/uapi/wireguard.h
+++ b/src/uapi/wireguard.h
@@ -83,6 +83,8 @@
  *    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_TUNNEL_NETNS_FD: NLA_U32, fd referring to the netns to use for
+ *                                the tunnel sockets
  *    WGDEVICE_A_PEERS: NLA_NESTED
  *        0: NLA_NESTED
  *            WGPEER_A_PUBLIC_KEY: len WG_KEY_LEN
@@ -154,6 +156,7 @@ enum wgdevice_attribute {
 	WGDEVICE_A_LISTEN_PORT,
 	WGDEVICE_A_FWMARK,
 	WGDEVICE_A_PEERS,
+	WGDEVICE_A_TUNNEL_NETNS_FD,
 	__WGDEVICE_A_LAST
 };
 #define WGDEVICE_A_MAX (__WGDEVICE_A_LAST - 1)
-- 
2.20.1

_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

[PATCH] Check CAP_NET_ADMIN in old and new ns before changing network ns.

[Maarten de Vries]

---

Forgot to check for CAP_NET_ADMIN. Quite important actually :)

 src/netlink.c | 60 +++++++++++++++++++++++++++++----------------------
 1 file changed, 34 insertions(+), 26 deletions(-)

diff --git a/src/netlink.c b/src/netlink.c
index 82e9030..2999593 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -473,30 +473,10 @@ out:
 	return ret;
 }
 
-static int set_tunnel_netns(struct wg_device *wg, u32 fd)
-{
-	struct net *new_net;
-
-	if (wg->sock4 != NULL || wg->sock6 != NULL)
-		return -EINVAL;
-
-	new_net = get_net_ns_by_fd(fd);
-
-	if (IS_ERR(new_net))
-		return PTR_ERR(new_net);
-
-	if (wg->have_creating_net_ref)
-		put_net(wg->creating_net);
-
-	wg->have_creating_net_ref = true;
-	wg->creating_net = new_net;
-
-	return 0;
-}
-
 static int wg_set_device(struct sk_buff *skb, struct genl_info *info)
 {
 	struct wg_device *wg = lookup_interface(info->attrs, skb);
+	struct net *new_net = NULL;
 	int ret;
 
 	if (IS_ERR(wg)) {
@@ -509,10 +489,34 @@ static int wg_set_device(struct sk_buff *skb, struct genl_info *info)
 
 	ret = -EPERM;
 	if ((info->attrs[WGDEVICE_A_LISTEN_PORT] ||
-	     info->attrs[WGDEVICE_A_FWMARK]) &&
+	     info->attrs[WGDEVICE_A_FWMARK] ||
+	     info->attrs[WGDEVICE_A_TUNNEL_NETNS_FD]) &&
 	    !ns_capable(wg->creating_net->user_ns, CAP_NET_ADMIN))
 		goto out;
 
+	if (info->attrs[WGDEVICE_A_TUNNEL_NETNS_FD]) {
+		int fd = nla_get_u32(info->attrs[WGDEVICE_A_TUNNEL_NETNS_FD]);
+		new_net = get_net_ns_by_fd(fd);
+
+		if (IS_ERR(new_net)) {
+			ret = PTR_ERR(new_net);
+			new_net = NULL;
+			goto out;
+		}
+
+		/* Also check that we've got CAP_NET_ADMIN in the new namespace. */
+		if (!ns_capable(new_net->user_ns, CAP_NET_ADMIN)) {
+			ret = -EPERM;
+			goto out;
+		}
+
+		/* And check that there are no initialized sockets. */
+		if (wg->sock4 != NULL || wg->sock6 != NULL) {
+			ret = -EINVAL;
+			goto out;
+		}
+	}
+
 	++wg->device_update_gen;
 
 	if (info->attrs[WGDEVICE_A_FWMARK]) {
@@ -582,15 +586,19 @@ static int wg_set_device(struct sk_buff *skb, struct genl_info *info)
 	}
 
 	if (info->attrs[WGDEVICE_A_TUNNEL_NETNS_FD]) {
-		int fd = nla_get_u32(info->attrs[WGDEVICE_A_TUNNEL_NETNS_FD]);
-		ret = set_tunnel_netns(wg, fd);
-		if (ret < 0)
-			goto out;
+		if (wg->have_creating_net_ref)
+			put_net(wg->creating_net);
+
+		wg->have_creating_net_ref = true;
+		wg->creating_net = new_net;
+		new_net = NULL;
 	}
 
 	ret = 0;
 
 out:
+	if (new_net)
+		put_net(new_net);
 	mutex_unlock(&wg->device_update_lock);
 	rtnl_unlock();
 	dev_put(wg->dev);
-- 
2.20.1

_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

Re: [PATCH] Check CAP_NET_ADMIN in old and new ns before changing network ns.

[Maarten de Vries]

On Mon, 4 Feb 2019 at 01:11, Maarten de Vries <maarten@de-vri.es> wrote:
>
> ---
>
> Forgot to check for CAP_NET_ADMIN. Quite important actually :)
>
>  src/netlink.c | 60 +++++++++++++++++++++++++++++----------------------
>  1 file changed, 34 insertions(+), 26 deletions(-)
>
> diff --git a/src/netlink.c b/src/netlink.c
> index 82e9030..2999593 100644
> --- a/src/netlink.c
> +++ b/src/netlink.c
> @@ -473,30 +473,10 @@ out:
>         return ret;
>  }
>
> -static int set_tunnel_netns(struct wg_device *wg, u32 fd)
> -{
> -       struct net *new_net;
> -
> -       if (wg->sock4 != NULL || wg->sock6 != NULL)
> -               return -EINVAL;
> -
> -       new_net = get_net_ns_by_fd(fd);
> -
> -       if (IS_ERR(new_net))
> -               return PTR_ERR(new_net);
> -
> -       if (wg->have_creating_net_ref)
> -               put_net(wg->creating_net);
> -
> -       wg->have_creating_net_ref = true;
> -       wg->creating_net = new_net;
> -
> -       return 0;
> -}
> -
>  static int wg_set_device(struct sk_buff *skb, struct genl_info *info)
>  {
>         struct wg_device *wg = lookup_interface(info->attrs, skb);
> +       struct net *new_net = NULL;
>         int ret;
>
>         if (IS_ERR(wg)) {
> @@ -509,10 +489,34 @@ static int wg_set_device(struct sk_buff *skb, struct genl_info *info)
>
>         ret = -EPERM;
>         if ((info->attrs[WGDEVICE_A_LISTEN_PORT] ||
> -            info->attrs[WGDEVICE_A_FWMARK]) &&
> +            info->attrs[WGDEVICE_A_FWMARK] ||
> +            info->attrs[WGDEVICE_A_TUNNEL_NETNS_FD]) &&
>             !ns_capable(wg->creating_net->user_ns, CAP_NET_ADMIN))
>                 goto out;
>
> +       if (info->attrs[WGDEVICE_A_TUNNEL_NETNS_FD]) {
> +               int fd = nla_get_u32(info->attrs[WGDEVICE_A_TUNNEL_NETNS_FD]);
> +               new_net = get_net_ns_by_fd(fd);
> +
> +               if (IS_ERR(new_net)) {
> +                       ret = PTR_ERR(new_net);
> +                       new_net = NULL;
> +                       goto out;
> +               }
> +
> +               /* Also check that we've got CAP_NET_ADMIN in the new namespace. */
> +               if (!ns_capable(new_net->user_ns, CAP_NET_ADMIN)) {
> +                       ret = -EPERM;
> +                       goto out;
> +               }
> +
> +               /* And check that there are no initialized sockets. */
> +               if (wg->sock4 != NULL || wg->sock6 != NULL) {
> +                       ret = -EINVAL;
> +                       goto out;
> +               }
> +       }
> +
>         ++wg->device_update_gen;
>
>         if (info->attrs[WGDEVICE_A_FWMARK]) {
> @@ -582,15 +586,19 @@ static int wg_set_device(struct sk_buff *skb, struct genl_info *info)
>         }
>
>         if (info->attrs[WGDEVICE_A_TUNNEL_NETNS_FD]) {
> -               int fd = nla_get_u32(info->attrs[WGDEVICE_A_TUNNEL_NETNS_FD]);
> -               ret = set_tunnel_netns(wg, fd);
> -               if (ret < 0)
> -                       goto out;
> +               if (wg->have_creating_net_ref)
> +                       put_net(wg->creating_net);
> +
> +               wg->have_creating_net_ref = true;
> +               wg->creating_net = new_net;
> +               new_net = NULL;
>         }
>
>         ret = 0;
>
>  out:
> +       if (new_net)
> +               put_net(new_net);
>         mutex_unlock(&wg->device_update_lock);
>         rtnl_unlock();
>         dev_put(wg->dev);
> --
> 2.20.1
>
> _______________________________________________
> WireGuard mailing list
> WireGuard@lists.zx2c4.com
> https://lists.zx2c4.com/mailman/listinfo/wireguard

Hmm, I did send this with --in-reply-to, but at least in gmail it is
treated as a separate thread. This was meant to be a reply to [PATCH]
Allow changing `creator_net` after interface creation.
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

Re: [PATCH] Allow changing `creator_net` after interface creation.

[Jason A. Donenfeld]

Hey Maarten,

Thanks for the patch. It turns out that in the git repo, the
jo/transit-namespace branch from Julian (CC'd) already has something
that seems very familiar with this. Pending upstreaming, we're holding
off on adding new features, but following that, we'll start to
evaluate options like this very seriously.

Do you see a difference between what your patchset does and what Julian's does?

Jason
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

Re: [PATCH] Allow changing `creator_net` after interface creation.

[Maarten de Vries]

On Mon, 4 Feb 2019 at 00:57, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> Hey Maarten,
>
> Thanks for the patch. It turns out that in the git repo, the
> jo/transit-namespace branch from Julian (CC'd) already has something
> that seems very familiar with this. Pending upstreaming, we're holding
> off on adding new features, but following that, we'll start to
> evaluate options like this very seriously.
>
> Do you see a difference between what your patchset does and what Julian's does?
>
> Jason

I'm not sure. I will read through Julian's patches tomorrow, but at a
glance it does appear to address the same issue.

I still sent a follow-up to add missing checks for CAP_NET_ADMIN.
Didn't feel right to leave it here without that, even if none of it is
merged in favour of Julian's patches :)

Kind regards,
Maarten de Vries
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard