wireguard.lists.zx2c4.com archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] Allow changing the transit namespace
@ 2018-09-08 12:18 Julian Orth
  2018-09-08 12:18 ` [PATCH 1/7] device: protect socket_init with device_update_lock Julian Orth
                   ` (7 more replies)
  0 siblings, 8 replies; 17+ messages in thread
From: Julian Orth @ 2018-09-08 12:18 UTC (permalink / raw)
  To: wireguard

This series allows users to change the transit namespace after the
Wireguard device has been created. The transit namespace is the
namespace in which the Wireguard UDP socket lives.

This allows Wireguard to be used in unprivileged containers [1]. This is
based on the following observation:

* Within the unprivileged container, the user has CAP_NET_ADMIN and can
  create a Wireguard device.
* In the init namespace, the user can create a UDP socket and bind to an
  unprivileged port.

Therefore, the following is possbile as an ordinary user:

$ unshare -r -U
$ export SAVED_PID=$$
$ unshare -n
$ ip link add wg0 type wireguard
$ wg set wg0 transit-net $SAVED_PID

wg(1) accepts the following new argument:

wg set <device> transit-net <pid|file-path>

The distinction is made based on the format of the argument. If it is an
unsigned 32 bit integer, then it is interpreted as a process id.
Otherwise it is interpreted as a file path. /proc does not need to be
mounted to use the process id interpretation. To force the
interpretation as a file-path, use a ./ prefix.

[1] https://stgraber.org/2014/01/17/lxc-1-0-unprivileged-containers/

Julian Orth (7):
  device: protect socket_init with device_update_lock
  device: rename creating_net to transit_net
  device: store a copy of the device net
  socket: allow modification of transit_net
  netlink: allow setting of transit net
  tools: allow setting of transit net
  tests: add test for transit-net

 src/device.c           | 46 ++++++++++++++++++++++++-------------
 src/device.h           |  6 +++--
 src/netlink.c          | 52 ++++++++++++++++++++++++++++++++----------
 src/socket.c           | 18 ++++++++-------
 src/socket.h           |  6 ++---
 src/tests/netns.sh     | 40 ++++++++++++++++++++++++++++++++
 src/tools/config.c     | 32 ++++++++++++++++++++++++++
 src/tools/containers.h |  6 ++++-
 src/tools/ipc.c        |  4 ++++
 src/tools/man/wg.8     |  9 ++++++--
 src/tools/set.c        |  2 +-
 src/uapi/wireguard.h   | 12 +++++++---
 12 files changed, 185 insertions(+), 48 deletions(-)

-- 
2.18.0

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/7] device: protect socket_init with device_update_lock
  2018-09-08 12:18 [PATCH 0/7] Allow changing the transit namespace Julian Orth
@ 2018-09-08 12:18 ` Julian Orth
  2018-09-08 12:18 ` [PATCH 2/7] device: rename creating_net to transit_net Julian Orth
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Julian Orth @ 2018-09-08 12:18 UTC (permalink / raw)
  To: wireguard

`set_port` in netlink.c races with `open` in device.c. This can cause
the following code flow:

* thread 1: set_port: device is not up
* thread 2: device is opened
* thread 2: open: called and calls socket_init with the original port
* thread 1: set_port: sets incoming_port to the new port and returns

incoming_port is then inconsistent. While this is not particularly
critical, it will become more critial when ste_port also sets the
transit namespace.
---
 src/device.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/device.c b/src/device.c
index 255ad49..88c228b 100644
--- a/src/device.c
+++ b/src/device.c
@@ -53,17 +53,18 @@ static int open(struct net_device *dev)
 #endif
 #endif
 
+	mutex_lock(&wg->device_update_lock);
 	ret = socket_init(wg, wg->incoming_port);
 	if (ret < 0)
-		return ret;
-	mutex_lock(&wg->device_update_lock);
+		goto out;
 	list_for_each_entry (peer, &wg->peer_list, peer_list) {
 		packet_send_staged_packets(peer);
 		if (peer->persistent_keepalive_interval)
 			packet_send_keepalive(peer);
 	}
+out:
 	mutex_unlock(&wg->device_update_lock);
-	return 0;
+	return ret;
 }
 
 #if defined(CONFIG_PM_SLEEP) && !defined(CONFIG_ANDROID)
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/7] device: rename creating_net to transit_net
  2018-09-08 12:18 [PATCH 0/7] Allow changing the transit namespace Julian Orth
  2018-09-08 12:18 ` [PATCH 1/7] device: protect socket_init with device_update_lock Julian Orth
@ 2018-09-08 12:18 ` Julian Orth
  2018-09-08 12:18 ` [PATCH 3/7] device: store a copy of the device net Julian Orth
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Julian Orth @ 2018-09-08 12:18 UTC (permalink / raw)
  To: wireguard

---
 src/device.c | 20 ++++++++++----------
 src/device.h |  4 ++--
 src/socket.c |  8 ++++----
 3 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/src/device.c b/src/device.c
index 88c228b..92aefc4 100644
--- a/src/device.c
+++ b/src/device.c
@@ -243,8 +243,8 @@ static void destruct(struct net_device *dev)
 	skb_queue_purge(&wg->incoming_handshakes);
 	free_percpu(dev->tstats);
 	free_percpu(wg->incoming_handshakes_worker);
-	if (wg->have_creating_net_ref)
-		put_net(wg->creating_net);
+	if (wg->have_transit_net_ref)
+		put_net(wg->transit_net);
 	mutex_unlock(&wg->device_update_lock);
 
 	pr_debug("%s: Interface deleted\n", dev->name);
@@ -296,7 +296,7 @@ static int newlink(struct net *src_net, struct net_device *dev,
 	int ret = -ENOMEM;
 	struct wireguard_device *wg = netdev_priv(dev);
 
-	wg->creating_net = src_net;
+	wg->transit_net = src_net;
 	init_rwsem(&wg->static_identity.lock);
 	mutex_init(&wg->socket_update_lock);
 	mutex_init(&wg->device_update_lock);
@@ -396,13 +396,13 @@ static int netdevice_notification(struct notifier_block *nb,
 	if (action != NETDEV_REGISTER || dev->netdev_ops != &netdev_ops)
 		return 0;
 
-	if (dev_net(dev) == wg->creating_net && wg->have_creating_net_ref) {
-		put_net(wg->creating_net);
-		wg->have_creating_net_ref = false;
-	} else if (dev_net(dev) != wg->creating_net &&
-		   !wg->have_creating_net_ref) {
-		wg->have_creating_net_ref = true;
-		get_net(wg->creating_net);
+	if (dev_net(dev) == wg->transit_net && wg->have_transit_net_ref) {
+		put_net(wg->transit_net);
+		wg->have_transit_net_ref = false;
+	} else if (dev_net(dev) != wg->transit_net &&
+		   !wg->have_transit_net_ref) {
+		wg->have_transit_net_ref = true;
+		get_net(wg->transit_net);
 	}
 	return 0;
 }
diff --git a/src/device.h b/src/device.h
index 2499782..4b7552c 100644
--- a/src/device.h
+++ b/src/device.h
@@ -40,7 +40,7 @@ struct wireguard_device {
 	struct net_device *dev;
 	struct crypt_queue encrypt_queue, decrypt_queue;
 	struct sock __rcu *sock4, *sock6;
-	struct net *creating_net;
+	struct net *transit_net;
 	struct noise_static_identity static_identity;
 	struct workqueue_struct *handshake_receive_wq, *handshake_send_wq;
 	struct workqueue_struct *packet_crypt_wq;
@@ -56,7 +56,7 @@ struct wireguard_device {
 	unsigned int num_peers, device_update_gen;
 	u32 fwmark;
 	u16 incoming_port;
-	bool have_creating_net_ref;
+	bool have_transit_net_ref;
 };
 
 int device_init(void);
diff --git a/src/socket.c b/src/socket.c
index 2e9e44f..72f3e6a 100644
--- a/src/socket.c
+++ b/src/socket.c
@@ -384,18 +384,18 @@ int socket_init(struct wireguard_device *wg, u16 port)
 retry:
 #endif
 
-	ret = udp_sock_create(wg->creating_net, &port4, &new4);
+	ret = udp_sock_create(wg->transit_net, &port4, &new4);
 	if (ret < 0) {
 		pr_err("%s: Could not create IPv4 socket\n", wg->dev->name);
 		return ret;
 	}
 	set_sock_opts(new4);
-	setup_udp_tunnel_sock(wg->creating_net, new4, &cfg);
+	setup_udp_tunnel_sock(wg->transit_net, new4, &cfg);
 
 #if IS_ENABLED(CONFIG_IPV6)
 	if (ipv6_mod_enabled()) {
 		port6.local_udp_port = inet_sk(new4->sk)->inet_sport;
-		ret = udp_sock_create(wg->creating_net, &port6, &new6);
+		ret = udp_sock_create(wg->transit_net, &port6, &new6);
 		if (ret < 0) {
 			udp_tunnel_sock_release(new4);
 			if (ret == -EADDRINUSE && !port && retries++ < 100)
@@ -405,7 +405,7 @@ retry:
 			return ret;
 		}
 		set_sock_opts(new6);
-		setup_udp_tunnel_sock(wg->creating_net, new6, &cfg);
+		setup_udp_tunnel_sock(wg->transit_net, new6, &cfg);
 	}
 #endif
 
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/7] device: store a copy of the device net
  2018-09-08 12:18 [PATCH 0/7] Allow changing the transit namespace Julian Orth
  2018-09-08 12:18 ` [PATCH 1/7] device: protect socket_init with device_update_lock Julian Orth
  2018-09-08 12:18 ` [PATCH 2/7] device: rename creating_net to transit_net Julian Orth
@ 2018-09-08 12:18 ` Julian Orth
  2018-09-08 12:18 ` [PATCH 4/7] socket: allow modification of transit_net Julian Orth
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Julian Orth @ 2018-09-08 12:18 UTC (permalink / raw)
  To: wireguard

---
 src/device.c | 5 +++--
 src/device.h | 1 +
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/device.c b/src/device.c
index 92aefc4..cb54ae1 100644
--- a/src/device.c
+++ b/src/device.c
@@ -396,10 +396,11 @@ static int netdevice_notification(struct notifier_block *nb,
 	if (action != NETDEV_REGISTER || dev->netdev_ops != &netdev_ops)
 		return 0;
 
-	if (dev_net(dev) == wg->transit_net && wg->have_transit_net_ref) {
+	wg->dev_net = dev_net(dev);
+	if (wg->dev_net == wg->transit_net && wg->have_transit_net_ref) {
 		put_net(wg->transit_net);
 		wg->have_transit_net_ref = false;
-	} else if (dev_net(dev) != wg->transit_net &&
+	} else if (wg->dev_net != wg->transit_net &&
 		   !wg->have_transit_net_ref) {
 		wg->have_transit_net_ref = true;
 		get_net(wg->transit_net);
diff --git a/src/device.h b/src/device.h
index 4b7552c..0bd25f2 100644
--- a/src/device.h
+++ b/src/device.h
@@ -41,6 +41,7 @@ struct wireguard_device {
 	struct crypt_queue encrypt_queue, decrypt_queue;
 	struct sock __rcu *sock4, *sock6;
 	struct net *transit_net;
+	struct net *dev_net;
 	struct noise_static_identity static_identity;
 	struct workqueue_struct *handshake_receive_wq, *handshake_send_wq;
 	struct workqueue_struct *packet_crypt_wq;
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 4/7] socket: allow modification of transit_net
  2018-09-08 12:18 [PATCH 0/7] Allow changing the transit namespace Julian Orth
                   ` (2 preceding siblings ...)
  2018-09-08 12:18 ` [PATCH 3/7] device: store a copy of the device net Julian Orth
@ 2018-09-08 12:18 ` Julian Orth
  2018-09-08 12:18 ` [PATCH 5/7] netlink: allow setting of transit net Julian Orth
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Julian Orth @ 2018-09-08 12:18 UTC (permalink / raw)
  To: wireguard

---
 src/device.c  | 18 +++++++++++++++---
 src/device.h  |  1 +
 src/netlink.c |  2 +-
 src/socket.c  | 18 ++++++++++--------
 src/socket.h  |  6 +++---
 5 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/src/device.c b/src/device.c
index cb54ae1..8f2660a 100644
--- a/src/device.c
+++ b/src/device.c
@@ -54,7 +54,7 @@ static int open(struct net_device *dev)
 #endif
 
 	mutex_lock(&wg->device_update_lock);
-	ret = socket_init(wg, wg->incoming_port);
+	ret = socket_init(wg, wg->transit_net, wg->incoming_port);
 	if (ret < 0)
 		goto out;
 	list_for_each_entry (peer, &wg->peer_list, peer_list) {
@@ -112,7 +112,7 @@ static int stop(struct net_device *dev)
 	}
 	mutex_unlock(&wg->device_update_lock);
 	skb_queue_purge(&wg->incoming_handshakes);
-	socket_reinit(wg, NULL, NULL);
+	socket_reinit(wg, NULL, NULL, NULL);
 	return 0;
 }
 
@@ -228,7 +228,7 @@ static void destruct(struct net_device *dev)
 	rtnl_unlock();
 	mutex_lock(&wg->device_update_lock);
 	wg->incoming_port = 0;
-	socket_reinit(wg, NULL, NULL);
+	socket_reinit(wg, NULL, NULL, NULL);
 	allowedips_free(&wg->peer_allowedips, &wg->device_update_lock);
 	/* The final references are cleared in the below calls to destroy_workqueue. */
 	peer_remove_all(wg);
@@ -396,6 +396,7 @@ static int netdevice_notification(struct notifier_block *nb,
 	if (action != NETDEV_REGISTER || dev->netdev_ops != &netdev_ops)
 		return 0;
 
+	mutex_lock(&wg->device_update_lock);
 	wg->dev_net = dev_net(dev);
 	if (wg->dev_net == wg->transit_net && wg->have_transit_net_ref) {
 		put_net(wg->transit_net);
@@ -405,6 +406,7 @@ static int netdevice_notification(struct notifier_block *nb,
 		wg->have_transit_net_ref = true;
 		get_net(wg->transit_net);
 	}
+	mutex_unlock(&wg->device_update_lock);
 	return 0;
 }
 
@@ -450,3 +452,13 @@ void device_uninit(void)
 #endif
 	rcu_barrier_bh();
 }
+
+void device_set_transit_net(struct wireguard_device *wg, struct net *net)
+{
+	if (wg->have_transit_net_ref)
+		put_net(wg->transit_net);
+	wg->transit_net = net;
+	wg->have_transit_net_ref = wg->transit_net != wg->dev_net;
+	if (wg->have_transit_net_ref)
+		get_net(wg->transit_net);
+}
diff --git a/src/device.h b/src/device.h
index 0bd25f2..d31564c 100644
--- a/src/device.h
+++ b/src/device.h
@@ -62,5 +62,6 @@ struct wireguard_device {
 
 int device_init(void);
 void device_uninit(void);
+void device_set_transit_net(struct wireguard_device *wg, struct net *net);
 
 #endif /* _WG_DEVICE_H */
diff --git a/src/netlink.c b/src/netlink.c
index 0bd2b97..73d9a74 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -314,7 +314,7 @@ static int set_port(struct wireguard_device *wg, u16 port)
 		wg->incoming_port = port;
 		return 0;
 	}
-	return socket_init(wg, port);
+	return socket_init(wg, wg->transit_net, port);
 }
 
 static int set_allowedip(struct wireguard_peer *peer, struct nlattr **attrs)
diff --git a/src/socket.c b/src/socket.c
index 72f3e6a..70b751c 100644
--- a/src/socket.c
+++ b/src/socket.c
@@ -354,7 +354,7 @@ static inline void set_sock_opts(struct socket *sock)
 	sk_set_memalloc(sock->sk);
 }
 
-int socket_init(struct wireguard_device *wg, u16 port)
+int socket_init(struct wireguard_device *wg, struct net *net, u16 port)
 {
 	int ret;
 	struct udp_tunnel_sock_cfg cfg = {
@@ -384,18 +384,18 @@ int socket_init(struct wireguard_device *wg, u16 port)
 retry:
 #endif
 
-	ret = udp_sock_create(wg->transit_net, &port4, &new4);
+	ret = udp_sock_create(net, &port4, &new4);
 	if (ret < 0) {
 		pr_err("%s: Could not create IPv4 socket\n", wg->dev->name);
 		return ret;
 	}
 	set_sock_opts(new4);
-	setup_udp_tunnel_sock(wg->transit_net, new4, &cfg);
+	setup_udp_tunnel_sock(net, new4, &cfg);
 
 #if IS_ENABLED(CONFIG_IPV6)
 	if (ipv6_mod_enabled()) {
 		port6.local_udp_port = inet_sk(new4->sk)->inet_sport;
-		ret = udp_sock_create(wg->transit_net, &port6, &new6);
+		ret = udp_sock_create(net, &port6, &new6);
 		if (ret < 0) {
 			udp_tunnel_sock_release(new4);
 			if (ret == -EADDRINUSE && !port && retries++ < 100)
@@ -405,16 +405,16 @@ retry:
 			return ret;
 		}
 		set_sock_opts(new6);
-		setup_udp_tunnel_sock(wg->transit_net, new6, &cfg);
+		setup_udp_tunnel_sock(net, new6, &cfg);
 	}
 #endif
 
-	socket_reinit(wg, new4 ? new4->sk : NULL, new6 ? new6->sk : NULL);
+	socket_reinit(wg, net, new4 ? new4->sk : NULL, new6 ? new6->sk : NULL);
 	return 0;
 }
 
-void socket_reinit(struct wireguard_device *wg, struct sock *new4,
-		   struct sock *new6)
+void socket_reinit(struct wireguard_device *wg, struct net *net,
+		   struct sock *new4, struct sock *new6)
 {
 	struct sock *old4, *old6;
 
@@ -427,6 +427,8 @@ void socket_reinit(struct wireguard_device *wg, struct sock *new4,
 	rcu_assign_pointer(wg->sock6, new6);
 	if (new4)
 		wg->incoming_port = ntohs(inet_sk(new4)->inet_sport);
+	if (net && wg->transit_net != net)
+		device_set_transit_net(wg, net);
 	mutex_unlock(&wg->socket_update_lock);
 	synchronize_rcu_bh();
 	synchronize_net();
diff --git a/src/socket.h b/src/socket.h
index d873ffa..8419ee9 100644
--- a/src/socket.h
+++ b/src/socket.h
@@ -11,9 +11,9 @@
 #include <linux/if_vlan.h>
 #include <linux/if_ether.h>
 
-int socket_init(struct wireguard_device *wg, u16 port);
-void socket_reinit(struct wireguard_device *wg, struct sock *new4,
-		   struct sock *new6);
+int socket_init(struct wireguard_device *wg, struct net *net, u16 port);
+void socket_reinit(struct wireguard_device *wg, struct net *net,
+		   struct sock *new4, struct sock *new6);
 int socket_send_buffer_to_peer(struct wireguard_peer *peer, void *data,
 			       size_t len, u8 ds);
 int socket_send_skb_to_peer(struct wireguard_peer *peer, struct sk_buff *skb,
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 5/7] netlink: allow setting of transit net
  2018-09-08 12:18 [PATCH 0/7] Allow changing the transit namespace Julian Orth
                   ` (3 preceding siblings ...)
  2018-09-08 12:18 ` [PATCH 4/7] socket: allow modification of transit_net Julian Orth
@ 2018-09-08 12:18 ` Julian Orth
  2018-09-08 14:03   ` Aaron Jones
  2018-09-08 12:18 ` [PATCH 6/7] tools: " Julian Orth
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Julian Orth @ 2018-09-08 12:18 UTC (permalink / raw)
  To: wireguard

This commit adds two new arguments:

* WGDEVICE_A_TRANSIT_NET_PID

    The argument is a U32 process id that refers to a process whose
    network namespace is to be used as the transit namespace.

* WGDEVICE_A_TRANSIT_NET_FD

    The argument is a U32 file descriptor that refers to a network
    namespace that is to be used as the transit namespace.
---
 src/netlink.c        | 52 ++++++++++++++++++++++++++++++++++----------
 src/uapi/wireguard.h | 12 +++++++---
 2 files changed, 49 insertions(+), 15 deletions(-)

diff --git a/src/netlink.c b/src/netlink.c
index 73d9a74..321df1a 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -24,7 +24,9 @@ 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_TRANSIT_NET_PID]	= { .type = NLA_U32 },
+	[WGDEVICE_A_TRANSIT_NET_FD]	= { .type = NLA_U32 },
 };
 
 static const struct nla_policy peer_policy[WGPEER_A_MAX + 1] = {
@@ -302,19 +304,48 @@ static int get_device_done(struct netlink_callback *cb)
 	return 0;
 }
 
-static int set_port(struct wireguard_device *wg, u16 port)
+static int set_socket(struct wireguard_device *wg, struct nlattr **attrs)
 {
 	struct wireguard_peer *peer;
+	struct nlattr *port_attr, *net_pid, *net_fd;
+	u16 port;
+	struct net *net = NULL;
+	int ret = 0;
+
+	port_attr = attrs[WGDEVICE_A_LISTEN_PORT];
+	net_pid = attrs[WGDEVICE_A_TRANSIT_NET_PID];
+	net_fd = attrs[WGDEVICE_A_TRANSIT_NET_FD];
+
+	if (net_pid && net_fd)
+		return -EINVAL;
+	if (net_pid)
+		net = get_net_ns_by_pid(nla_get_u32(net_pid));
+	if (net_fd)
+		net = get_net_ns_by_fd(nla_get_u32(net_fd));
+	if (IS_ERR(net))
+		return PTR_ERR(net);
+	if (port_attr)
+		port = nla_get_u16(port_attr);
+	else
+		port = wg->incoming_port;
+
+	if (wg->incoming_port == port && (!net || wg->transit_net == net))
+		goto out;
 
-	if (wg->incoming_port == port)
-		return 0;
 	list_for_each_entry (peer, &wg->peer_list, peer_list)
 		socket_clear_peer_endpoint_src(peer);
 	if (!netif_running(wg->dev)) {
 		wg->incoming_port = port;
-		return 0;
+		if (net)
+			device_set_transit_net(wg, net);
+		goto out;
 	}
-	return socket_init(wg, wg->transit_net, port);
+	ret = socket_init(wg, net ? : wg->transit_net, port);
+
+out:
+	if (net)
+		put_net(net);
+	return ret;
 }
 
 static int set_allowedip(struct wireguard_peer *peer, struct nlattr **attrs)
@@ -496,12 +527,9 @@ static int set_device(struct sk_buff *skb, struct genl_info *info)
 			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]));
-		if (ret)
-			goto out;
-	}
+	ret = set_socket(wg, info->attrs);
+	if (ret)
+		goto out;
 
 	if (info->attrs[WGDEVICE_A_FLAGS] &&
 	    nla_get_u32(info->attrs[WGDEVICE_A_FLAGS]) &
diff --git a/src/uapi/wireguard.h b/src/uapi/wireguard.h
index 3d73ad7..8faea3e 100644
--- a/src/uapi/wireguard.h
+++ b/src/uapi/wireguard.h
@@ -72,9 +72,11 @@
  * WG_CMD_SET_DEVICE
  * -----------------
  *
- * May only be called via NLM_F_REQUEST. The command should contain the
- * following tree of nested items, containing one but not both of
- * WGDEVICE_A_IFINDEX and WGDEVICE_A_IFNAME:
+ * May only be called via NLM_F_REQUEST. The command must contain the following
+ * tree of nested items. Exactly one of WGDEVICE_A_IFINDEX and WGDEVICE_A_IFNAME
+ * must be provided. All other top-level items are optional. At most one of
+ * WGDEVICE_A_TRANSIT_NET_PID and WGDEVICE_A_TRANSIT_NET_FD may be provided.
+ *
  *
  *    WGDEVICE_A_IFINDEX: NLA_U32
  *    WGDEVICE_A_IFNAME: NLA_NUL_STRING, maxlen IFNAMESIZ - 1
@@ -82,6 +84,8 @@
  *                      peers should be removed prior to adding the list below.
  *    WGDEVICE_A_PRIVATE_KEY: len WG_KEY_LEN, all zeros to remove
  *    WGDEVICE_A_LISTEN_PORT: NLA_U16, 0 to choose randomly
+ *    WGDEVICE_A_TRANSIT_NET_PID: NLA_U32
+ *    WGDEVICE_A_TRANSIT_NET_FD: NLA_U32
  *    WGDEVICE_A_FWMARK: NLA_U32, 0 to disable
  *    WGDEVICE_A_PEERS: NLA_NESTED
  *        0: NLA_NESTED
@@ -154,6 +158,8 @@ enum wgdevice_attribute {
 	WGDEVICE_A_LISTEN_PORT,
 	WGDEVICE_A_FWMARK,
 	WGDEVICE_A_PEERS,
+	WGDEVICE_A_TRANSIT_NET_PID,
+	WGDEVICE_A_TRANSIT_NET_FD,
 	__WGDEVICE_A_LAST
 };
 #define WGDEVICE_A_MAX (__WGDEVICE_A_LAST - 1)
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 6/7] tools: allow setting of transit net
  2018-09-08 12:18 [PATCH 0/7] Allow changing the transit namespace Julian Orth
                   ` (4 preceding siblings ...)
  2018-09-08 12:18 ` [PATCH 5/7] netlink: allow setting of transit net Julian Orth
@ 2018-09-08 12:18 ` Julian Orth
  2018-09-08 14:04   ` Aaron Jones
  2018-09-08 14:09   ` Aaron Jones
  2018-09-08 12:18 ` [PATCH 7/7] tests: add test for transit-net Julian Orth
  2018-09-08 13:39 ` [PATCH 0/7] Allow changing the transit namespace Bruno Wolff III
  7 siblings, 2 replies; 17+ messages in thread
From: Julian Orth @ 2018-09-08 12:18 UTC (permalink / raw)
  To: wireguard

The command is

wg set <device> [...] transit-net <pid|file-path> [...]

For example:

wg set wg0 transit-net 1
wg set wg0 transit-net /proc/1/ns/net
---
 src/tools/config.c     | 32 ++++++++++++++++++++++++++++++++
 src/tools/containers.h |  6 +++++-
 src/tools/ipc.c        |  4 ++++
 src/tools/man/wg.8     |  9 +++++++--
 src/tools/set.c        |  2 +-
 5 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/src/tools/config.c b/src/tools/config.c
index 93525fb..b266026 100644
--- a/src/tools/config.c
+++ b/src/tools/config.c
@@ -14,6 +14,7 @@
 #include <sys/socket.h>
 #include <sys/stat.h>
 #include <errno.h>
+#include <fcntl.h>
 
 #include "config.h"
 #include "containers.h"
@@ -74,6 +75,30 @@ static inline bool parse_port(uint16_t *port, uint32_t *flags, const char *value
 	return ret == 0;
 }
 
+static bool parse_transit_net(struct wgdevice *device, const char *arg)
+{
+	/* U32 arg -> PID */
+	if (isdigit(*arg)) {
+		char *end;
+		unsigned long pid = strtoul(arg, &end, 10);
+		if (!*end && pid <= UINT32_MAX) {
+			device->transit_net_pid = pid;
+			device->flags |= WGDEVICE_HAS_TRANSIT_NET_PID;
+			return true;
+		}
+	}
+
+	/* Otherwise -> file path */
+	device->transit_net_fd = open(arg, O_RDONLY);
+	if (device->transit_net_fd >= 0) {
+		device->flags |= WGDEVICE_HAS_TRANSIT_NET_FD;
+		return true;
+	}
+
+	perror("fopen");
+	return false;
+}
+
 static inline bool parse_fwmark(uint32_t *fwmark, uint32_t *flags, const char *value)
 {
 	unsigned long ret;
@@ -392,6 +417,8 @@ static bool process_line(struct config_ctx *ctx, const char *line)
 	if (ctx->is_device_section) {
 		if (key_match("ListenPort"))
 			ret = parse_port(&ctx->device->listen_port, &ctx->device->flags, value);
+		else if (key_match("TransitNet"))
+			ret = parse_transit_net(ctx->device, value);
 		else if (key_match("FwMark"))
 			ret = parse_fwmark(&ctx->device->fwmark, &ctx->device->flags, value);
 		else if (key_match("PrivateKey")) {
@@ -525,6 +552,11 @@ struct wgdevice *config_read_cmd(char *argv[], int argc)
 				goto error;
 			argv += 2;
 			argc -= 2;
+		} else if (!strcmp(argv[0], "transit-net") && argc >= 2 && !peer) {
+			if (!parse_transit_net(device, argv[1]))
+				goto error;
+			argv += 2;
+			argc -= 2;
 		} else if (!strcmp(argv[0], "fwmark") && argc >= 2 && !peer) {
 			if (!parse_fwmark(&device->fwmark, &device->flags, argv[1]))
 				goto error;
diff --git a/src/tools/containers.h b/src/tools/containers.h
index 455d998..188f909 100644
--- a/src/tools/containers.h
+++ b/src/tools/containers.h
@@ -58,7 +58,9 @@ enum {
 	WGDEVICE_HAS_PRIVATE_KEY = 1U << 1,
 	WGDEVICE_HAS_PUBLIC_KEY = 1U << 2,
 	WGDEVICE_HAS_LISTEN_PORT = 1U << 3,
-	WGDEVICE_HAS_FWMARK = 1U << 4
+	WGDEVICE_HAS_FWMARK = 1U << 4,
+	WGDEVICE_HAS_TRANSIT_NET_PID = 1U << 5,
+	WGDEVICE_HAS_TRANSIT_NET_FD = 1U << 6,
 };
 
 struct wgdevice {
@@ -72,6 +74,8 @@ struct wgdevice {
 
 	uint32_t fwmark;
 	uint16_t listen_port;
+	uint32_t transit_net_pid;
+	int transit_net_fd;
 
 	struct wgpeer *first_peer, *last_peer;
 };
diff --git a/src/tools/ipc.c b/src/tools/ipc.c
index e3ef789..1bc98ed 100644
--- a/src/tools/ipc.c
+++ b/src/tools/ipc.c
@@ -569,6 +569,10 @@ again:
 			mnl_attr_put(nlh, WGDEVICE_A_PRIVATE_KEY, sizeof(dev->private_key), dev->private_key);
 		if (dev->flags & WGDEVICE_HAS_LISTEN_PORT)
 			mnl_attr_put_u16(nlh, WGDEVICE_A_LISTEN_PORT, dev->listen_port);
+		if (dev->flags & WGDEVICE_HAS_TRANSIT_NET_PID)
+			mnl_attr_put_u32(nlh, WGDEVICE_A_TRANSIT_NET_PID, dev->transit_net_pid);
+		if (dev->flags & WGDEVICE_HAS_TRANSIT_NET_FD)
+			mnl_attr_put_u32(nlh, WGDEVICE_A_TRANSIT_NET_FD, (uint32_t)dev->transit_net_fd);
 		if (dev->flags & WGDEVICE_HAS_FWMARK)
 			mnl_attr_put_u32(nlh, WGDEVICE_A_FWMARK, dev->fwmark);
 		if (dev->flags & WGDEVICE_REPLACE_PEERS)
diff --git a/src/tools/man/wg.8 b/src/tools/man/wg.8
index 5bae7ca..fd4caab 100644
--- a/src/tools/man/wg.8
+++ b/src/tools/man/wg.8
@@ -55,12 +55,17 @@ transfer-rx, transfer-tx, persistent-keepalive.
 Shows the current configuration of \fI<interface>\fP in the format described
 by \fICONFIGURATION FILE FORMAT\fP below.
 .TP
-\fBset\fP \fI<interface>\fP [\fIlisten-port\fP \fI<port>\fP] [\fIfwmark\fP \fI<fwmark>\fP] [\fIprivate-key\fP \fI<file-path>\fP] [\fIpeer\fP \fI<base64-public-key>\fP [\fIremove\fP] [\fIpreshared-key\fP \fI<file-path>\fP] [\fIendpoint\fP \fI<ip>:<port>\fP] [\fIpersistent-keepalive\fP \fI<interval seconds>\fP] [\fIallowed-ips\fP \fI<ip1>/<cidr1>\fP[,\fI<ip2>/<cidr2>\fP]...] ]...
+\fBset\fP \fI<interface>\fP [\fIlisten-port\fP \fI<port>\fP] [\fItransit-net\fP \fI<pid|file-path>\fP] [\fIfwmark\fP \fI<fwmark>\fP] [\fIprivate-key\fP \fI<file-path>\fP] [\fIpeer\fP \fI<base64-public-key>\fP [\fIremove\fP] [\fIpreshared-key\fP \fI<file-path>\fP] [\fIendpoint\fP \fI<ip>:<port>\fP] [\fIpersistent-keepalive\fP \fI<interval seconds>\fP] [\fIallowed-ips\fP \fI<ip1>/<cidr1>\fP[,\fI<ip2>/<cidr2>\fP]...] ]...
 Sets configuration values for the specified \fI<interface>\fP. Multiple
 \fIpeer\fPs may be specified, and if the \fIremove\fP argument is given
 for a peer, that peer is removed, not configured. If \fIlisten-port\fP
 is not specified, the port will be chosen randomly when the
-interface comes up. Both \fIprivate-key\fP and \fIpreshared-key\fP must
+interface comes up. If transit-net is not specified, the network namespace
+through which encrypted packets are routed is the one in which the device
+was created. Otherwise the network namespace through which encrypted packets are
+routed is the one specified by the argument. If the argument is an unsigned
+32-bit integer, it is interpeted as a process id, otherwise it is interpreted as
+a file path. Both \fIprivate-key\fP and \fIpreshared-key\fP must
 be a files, because command line arguments are not considered private on
 most systems but if you are using
 .BR bash (1),
diff --git a/src/tools/set.c b/src/tools/set.c
index d44fed9..fb11ed0 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>] [transit-net <pid|file path>] [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]);
 		return 1;
 	}
 
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 7/7] tests: add test for transit-net
  2018-09-08 12:18 [PATCH 0/7] Allow changing the transit namespace Julian Orth
                   ` (5 preceding siblings ...)
  2018-09-08 12:18 ` [PATCH 6/7] tools: " Julian Orth
@ 2018-09-08 12:18 ` Julian Orth
  2018-09-08 13:39 ` [PATCH 0/7] Allow changing the transit namespace Bruno Wolff III
  7 siblings, 0 replies; 17+ messages in thread
From: Julian Orth @ 2018-09-08 12:18 UTC (permalink / raw)
  To: wireguard

---
 src/tests/netns.sh | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/src/tests/netns.sh b/src/tests/netns.sh
index 568612c..4cfcf61 100755
--- a/src/tests/netns.sh
+++ b/src/tests/netns.sh
@@ -222,6 +222,46 @@ n1 wg set wg0 peer "$more_specific_key" remove
 ip1 link del wg0
 ip2 link del wg0
 
+# Test using transit namespace. We now change the topology to this with transit-net of wg0 = $ns0
+# ┌──────────────────────┐    ┌───────────────────────┐     ┌────────────────────────────────────────┐
+# │    $ns1 namespace    │    │     $ns0 namespace    │     │             $ns2 namespace             │
+# │                      │    │                       │     │                                        │
+# │  ┌─────┐             │    │ ┌──────┐              │     │  ┌─────┐            ┌─────┐            │
+# │  │ wg0 │             │    │ │vethrs│──────────────┼─────┼──│veths│────────────│ wg0 │            │
+# │  ├─────┴──────────┐  │    │ ├──────┴────────────┐ │     │  ├─────┴──────────┐ ├─────┴──────────┐ │
+# │  │192.168.241.1/24│  │    │ │10.0.0.1/24        │ │     │  │10.0.0.100/24   │ │192.168.241.2/24│ │
+# │  │fd00::1/24      │  │    │ │SNAT:192.168.1.0/24│ │     │  │                │ │fd00::2/24      │ │
+# │  └────────────────┘  │    │ └───────────────────┘ │     │  └────────────────┘ └────────────────┘ │
+# └──────────────────────┘    └───────────────────────┘     └────────────────────────────────────────┘
+
+ip1 link add dev wg0 type wireguard
+ip2 link add dev wg0 type wireguard
+configure_peers
+n1 wg set wg0 transit-net /run/netns/$netns0
+
+ip0 link add vethrs type veth peer name veths
+ip0 link set veths netns $netns2
+ip0 link set vethrs up
+ip0 addr add 10.0.0.1/24 dev vethrs
+ip2 addr add 10.0.0.100/24 dev veths
+ip1 route add default dev wg0
+ip2 link set veths up
+waitiface $netns0 vethrs
+waitiface $netns2 veths
+
+n1 wg set wg0 peer "$pub2" endpoint 10.0.0.100:2 persistent-keepalive 1
+n1 ping -W 1 -c 1 192.168.241.2
+n2 ping -W 1 -c 1 192.168.241.1
+[[ $(n2 wg show wg0 endpoints) == "$pub1	10.0.0.1:1" ]]
+# Demonstrate n2 can still send packets to n1, since persistent-keepalive will prevent connection tracking entry from expiring (to see entries: `n0 conntrack -L`).
+pp sleep 3
+n2 ping -W 1 -c 1 192.168.241.1
+
+ip0 link del vethrs
+
+ip1 link del wg0
+ip2 link del wg0
+
 # Test using NAT. We now change the topology to this:
 # ┌────────────────────────────────────────┐    ┌────────────────────────────────────────────────┐     ┌────────────────────────────────────────┐
 # │             $ns1 namespace             │    │                 $ns0 namespace                 │     │             $ns2 namespace             │
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/7] Allow changing the transit namespace
  2018-09-08 12:18 [PATCH 0/7] Allow changing the transit namespace Julian Orth
                   ` (6 preceding siblings ...)
  2018-09-08 12:18 ` [PATCH 7/7] tests: add test for transit-net Julian Orth
@ 2018-09-08 13:39 ` Bruno Wolff III
  2018-09-08 13:49   ` Julian Orth
  7 siblings, 1 reply; 17+ messages in thread
From: Bruno Wolff III @ 2018-09-08 13:39 UTC (permalink / raw)
  To: Julian Orth; +Cc: wireguard

On Sat, Sep 08, 2018 at 14:18:34 +0200,
  Julian Orth <ju.orth@gmail.com> wrote:
>
>wg set <device> transit-net <pid|file-path>
>
>The distinction is made based on the format of the argument. If it is an
>unsigned 32 bit integer, then it is interpreted as a process id.
>Otherwise it is interpreted as a file path. /proc does not need to be
>mounted to use the process id interpretation. To force the
>interpretation as a file-path, use a ./ prefix.

Ambiguity is generally not a good idea (especially for security applications). 
It's not my decision, but I'd rather see the syntax make it clear what the type 
of the parameter is supposed to be.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/7] Allow changing the transit namespace
  2018-09-08 13:39 ` [PATCH 0/7] Allow changing the transit namespace Bruno Wolff III
@ 2018-09-08 13:49   ` Julian Orth
  0 siblings, 0 replies; 17+ messages in thread
From: Julian Orth @ 2018-09-08 13:49 UTC (permalink / raw)
  To: Bruno Wolff III; +Cc: wireguard

On 9/8/18 3:39 PM, Bruno Wolff III wrote:
> On Sat, Sep 08, 2018 at 14:18:34 +0200,
>   Julian Orth <ju.orth@gmail.com> wrote:
>>
>> wg set <device> transit-net <pid|file-path>
>>
>> The distinction is made based on the format of the argument. If it is an
>> unsigned 32 bit integer, then it is interpreted as a process id.
>> Otherwise it is interpreted as a file path. /proc does not need to be
>> mounted to use the process id interpretation. To force the
>> interpretation as a file-path, use a ./ prefix.
> 
> Ambiguity is generally not a good idea (especially for security 
> applications). It's not my decision, but I'd rather see the syntax make 
> it clear what the type of the parameter is supposed to be.

The syntax is based on a similar feature in iproute2:

ip link set <device> netns <pid|name>

The syntax of wg seems to generally follow the iproute2 syntax so I 
tried to keep it consistent.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/7] netlink: allow setting of transit net
  2018-09-08 12:18 ` [PATCH 5/7] netlink: allow setting of transit net Julian Orth
@ 2018-09-08 14:03   ` Aaron Jones
  2018-09-08 14:20     ` Julian Orth
  0 siblings, 1 reply; 17+ messages in thread
From: Aaron Jones @ 2018-09-08 14:03 UTC (permalink / raw)
  To: Julian Orth; +Cc: WireGuard mailing list

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Does this handle file descriptor 0 (unlikely, but perfectly valid) ?

-----BEGIN PGP SIGNATURE-----
Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/

iQIzBAEBCgAdFiEEYKVBwe43zZh/jkxPivBzdIirMBIFAluT1qQACgkQivBzdIir
MBKv2g/+Nvz3NmcakxPMzGAC07dXQiu4TVVjwFeGofUwgQmB5+wY6fy9Sqg+S+z6
Fkz2Mnkste0o38JGz2320wAPmH6IYsKrtA5TGzaoNP8I+Gp2Uy9GqjDtUe2+tNnj
hLdK2mucbE1Sp6MbBeTKyZi2ZD1VHaYcsFJupjPnL1wT7pVnVSnRjGwoEJjRT4VQ
htgEh4YGtAGOWHYzFMxlZF/oKM6QHAZXTcAm6k1xMLk7TvXNpaTamuNYCl2DLwIf
TpjxANzzd5itBcos95F0VKJtVlRJkbW0lIedaw3qyrjhIjM8rmxAF/pTGosMigcr
KIXVcPKaZhB0y/kMh/fe7LFMIcBb9N62skJEHtpFX4AUjT+XQYGtzp2Z9iiVDooX
kLn19X3EDgS5ln7LaNlprZnQaxjkHWhQa+X8xxDKyFVBRBJO4lQA72eT4ZjSDzbv
JGRkLSHsvGmHg5qOji5mamZB+HgLZcImbKAp2MJaEkgU9G9M9U8OPICp5dVcv9dp
1cY7E0qhhrBDyhda30VbuM+inOjEfqYHQ5ZaGGE530EN8oVE1eci421OcxdY/MTc
mmJ8FOYxxYsxNwzPZsMHjIX1xGD6W87zG0kOKLDBBrJ64FBWe77kGMZ9R55Is5pc
kfeTfDSR311OC4qHy4scudqNUyyh8WnySFziohCoOU9giypLNGQ=
=2J95
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 6/7] tools: allow setting of transit net
  2018-09-08 12:18 ` [PATCH 6/7] tools: " Julian Orth
@ 2018-09-08 14:04   ` Aaron Jones
  2018-09-08 14:09   ` Aaron Jones
  1 sibling, 0 replies; 17+ messages in thread
From: Aaron Jones @ 2018-09-08 14:04 UTC (permalink / raw)
  To: Julian Orth; +Cc: wireguard


[-- Attachment #1.1: Type: text/plain, Size: 714 bytes --]

On 08/09/18 12:18, Julian Orth wrote:
> +static bool parse_transit_net(struct wgdevice *device, const char *arg)
> +{
> +	/* U32 arg -> PID */
> +	if (isdigit(*arg)) {
> +		char *end;
> +		unsigned long pid = strtoul(arg, &end, 10);
> +		if (!*end && pid <= UINT32_MAX) {
> +			device->transit_net_pid = pid;
> +			device->flags |= WGDEVICE_HAS_TRANSIT_NET_PID;
> +			return true;
> +		}
> +	}
> +
> +	/* Otherwise -> file path */
> +	device->transit_net_fd = open(arg, O_RDONLY);
> +	if (device->transit_net_fd >= 0) {
> +		device->flags |= WGDEVICE_HAS_TRANSIT_NET_FD;
> +		return true;
> +	}
> +
> +	perror("fopen");
> +	return false;
> +}

Wrong function name given to perror(3).



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 6/7] tools: allow setting of transit net
  2018-09-08 12:18 ` [PATCH 6/7] tools: " Julian Orth
  2018-09-08 14:04   ` Aaron Jones
@ 2018-09-08 14:09   ` Aaron Jones
  2018-09-08 14:18     ` Julian Orth
  1 sibling, 1 reply; 17+ messages in thread
From: Aaron Jones @ 2018-09-08 14:09 UTC (permalink / raw)
  To: Julian Orth; +Cc: WireGuard mailing list


[-- Attachment #1.1: Type: text/plain, Size: 542 bytes --]

On 08/09/18 12:18, Julian Orth wrote:
> +static bool parse_transit_net(struct wgdevice *device, const char *arg)
> +{
> +	/* U32 arg -> PID */
> +	if (isdigit(*arg)) {
> +		char *end;
> +		unsigned long pid = strtoul(arg, &end, 10);
> +		if (!*end && pid <= UINT32_MAX) {

This condition will always be true on a 32-bit system (for a valid
integer). I'd prefer strtoull() and `unsigned long long' but given
that a valid file descriptor won't ever go that high... eh. shrug. But
you'll still get a compiler diagnostic for it.



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 6/7] tools: allow setting of transit net
  2018-09-08 14:09   ` Aaron Jones
@ 2018-09-08 14:18     ` Julian Orth
  2018-09-08 14:25       ` Aaron Jones
  0 siblings, 1 reply; 17+ messages in thread
From: Julian Orth @ 2018-09-08 14:18 UTC (permalink / raw)
  To: Aaron Jones; +Cc: WireGuard mailing list

On 9/8/18 4:09 PM, Aaron Jones wrote:
> On 08/09/18 12:18, Julian Orth wrote:
>> +static bool parse_transit_net(struct wgdevice *device, const char *arg)
>> +{
>> +	/* U32 arg -> PID */
>> +	if (isdigit(*arg)) {
>> +		char *end;
>> +		unsigned long pid = strtoul(arg, &end, 10);
>> +		if (!*end && pid <= UINT32_MAX) {
> 
> This condition will always be true on a 32-bit system (for a valid
> integer). I'd prefer strtoull() and `unsigned long long' but given
> that a valid file descriptor won't ever go that high... eh. shrug. But
> you'll still get a compiler diagnostic for it.

Even with clang's -Weverything I don't get a warning when I compile the 
following:

#include <stdint.h>

int f(unsigned int i);

int f(unsigned int i)
{
	return i <= UINT32_MAX;
}

I don't see why it would be different for unsigned long on a 32-bit system.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/7] netlink: allow setting of transit net
  2018-09-08 14:03   ` Aaron Jones
@ 2018-09-08 14:20     ` Julian Orth
  2018-09-08 14:28       ` Aaron Jones
  0 siblings, 1 reply; 17+ messages in thread
From: Julian Orth @ 2018-09-08 14:20 UTC (permalink / raw)
  To: Aaron Jones; +Cc: WireGuard mailing list

On 9/8/18 4:03 PM, Aaron Jones wrote:
> Does this handle file descriptor 0 (unlikely, but perfectly valid) ?

I believe so.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 6/7] tools: allow setting of transit net
  2018-09-08 14:18     ` Julian Orth
@ 2018-09-08 14:25       ` Aaron Jones
  0 siblings, 0 replies; 17+ messages in thread
From: Aaron Jones @ 2018-09-08 14:25 UTC (permalink / raw)
  To: Julian Orth; +Cc: WireGuard mailing list


[-- Attachment #1.1: Type: text/plain, Size: 486 bytes --]

On 08/09/18 14:18, Julian Orth wrote:
> Even with clang's -Weverything I don't get a warning when I compile the
> following:
> 
> #include <stdint.h>
> 
> int f(unsigned int i);
> 
> int f(unsigned int i)
> {
>     return i <= UINT32_MAX;
> }

Interesting... I don't either (gcc 8, clang 8). Perhaps I've been
spending too much time reading PVS-Studio reports.

Still, the condition will always be true; sizeof(unsigned long) is
sizeof(uint32_t) in this case.



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/7] netlink: allow setting of transit net
  2018-09-08 14:20     ` Julian Orth
@ 2018-09-08 14:28       ` Aaron Jones
  0 siblings, 0 replies; 17+ messages in thread
From: Aaron Jones @ 2018-09-08 14:28 UTC (permalink / raw)
  To: Julian Orth; +Cc: WireGuard mailing list


[-- Attachment #1.1: Type: text/plain, Size: 248 bytes --]

On 08/09/18 14:20, Julian Orth wrote:
> On 9/8/18 4:03 PM, Aaron Jones wrote:
>> Does this handle file descriptor 0 (unlikely, but perfectly valid) ?
> 
> I believe so.

Ah, so it does; I misread the start of set_socket(). My apologies.



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2018-09-08 14:28 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-08 12:18 [PATCH 0/7] Allow changing the transit namespace Julian Orth
2018-09-08 12:18 ` [PATCH 1/7] device: protect socket_init with device_update_lock Julian Orth
2018-09-08 12:18 ` [PATCH 2/7] device: rename creating_net to transit_net Julian Orth
2018-09-08 12:18 ` [PATCH 3/7] device: store a copy of the device net Julian Orth
2018-09-08 12:18 ` [PATCH 4/7] socket: allow modification of transit_net Julian Orth
2018-09-08 12:18 ` [PATCH 5/7] netlink: allow setting of transit net Julian Orth
2018-09-08 14:03   ` Aaron Jones
2018-09-08 14:20     ` Julian Orth
2018-09-08 14:28       ` Aaron Jones
2018-09-08 12:18 ` [PATCH 6/7] tools: " Julian Orth
2018-09-08 14:04   ` Aaron Jones
2018-09-08 14:09   ` Aaron Jones
2018-09-08 14:18     ` Julian Orth
2018-09-08 14:25       ` Aaron Jones
2018-09-08 12:18 ` [PATCH 7/7] tests: add test for transit-net Julian Orth
2018-09-08 13:39 ` [PATCH 0/7] Allow changing the transit namespace Bruno Wolff III
2018-09-08 13:49   ` Julian Orth

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).