* [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
* 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
* [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
* 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
* [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