* Setting the transit namespace at runtime @ 2018-09-03 16:16 Julian Orth 2018-09-06 20:42 ` Julian Orth 2018-09-07 1:26 ` Jason A. Donenfeld 0 siblings, 2 replies; 7+ messages in thread From: Julian Orth @ 2018-09-03 16:16 UTC (permalink / raw) To: wireguard Hi, Each Wireguard device remembers the network namespace in which it was created. In the documentation this is called the birthplace namespace [1] but I'll be calling it the transit namespace. Let's say I create a Wireguard device `wg0` in a network namespace called `vpn`. Then I would like to be able to run # wg set wg0 transit-namespace /proc/1/ns/net to change the Wireguard UDP socket to live in the init namespace. This has the following advantages over creating the device in the init namespace and then moving it to the `vpn` namespace: * If multiple processes are creating Wireguard devices at the same time, then their device namespaces are isolated as long as each process uses its own network namespace. This means that there is no problem if two processes try to create the `wg0` device at the same time. * The intention is for the `wg0` device to be used only within the `vpn` namespace. It does not feel clean that the device has to live in the init namespace for an arbitrarily short but non-zero amount of time. This also leaks the existence of the `wg0` device to all processes living in the init namespace. Could such a feature be implemented? Julian [1] https://www.wireguard.com/netns/ ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Setting the transit namespace at runtime 2018-09-03 16:16 Setting the transit namespace at runtime Julian Orth @ 2018-09-06 20:42 ` Julian Orth 2018-09-07 1:29 ` Jason A. Donenfeld 2018-09-07 1:26 ` Jason A. Donenfeld 1 sibling, 1 reply; 7+ messages in thread From: Julian Orth @ 2018-09-06 20:42 UTC (permalink / raw) To: wireguard Hi, After receiving some positive feedback on IRC, I've gone ahead and implemented this. You can see the code here: https://github.com/mahkoh/wireguard/commits/transit-namespace You can test it as follows: * Create a new netns: ip netns add test * Enter it: ip netns exec test bash * Use wg-quick to create a wireguard device * Try to connect anywhere: It doesn't work * Set the transit namespace to the init namespace: wg set wg0 transit-net /proc/1/ns/net * Try to connect anywhere: It works I haven't written any documentation yet but I hope that the commits are clear enough. Julian ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Setting the transit namespace at runtime 2018-09-06 20:42 ` Julian Orth @ 2018-09-07 1:29 ` Jason A. Donenfeld 0 siblings, 0 replies; 7+ messages in thread From: Jason A. Donenfeld @ 2018-09-07 1:29 UTC (permalink / raw) To: ju.orth; +Cc: WireGuard mailing list On Thu, Sep 6, 2018 at 2:43 PM Julian Orth <ju.orth@gmail.com> wrote: > > Hi, > > After receiving some positive feedback on IRC, I've gone ahead and implemented > this. You can see the code here: > > https://github.com/mahkoh/wireguard/commits/transit-namespace Thanks for this. Not sure I'll accept it based on the issues I mentioned in the last email (though I am open to discussion to change minds about it), but either way, the proper way to get a review on the code here or on LKML is to submit it with `git format-patch --cover-letter` and `git send-email`. Jason ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Setting the transit namespace at runtime 2018-09-03 16:16 Setting the transit namespace at runtime Julian Orth 2018-09-06 20:42 ` Julian Orth @ 2018-09-07 1:26 ` Jason A. Donenfeld 2018-09-07 19:06 ` Julian Orth 1 sibling, 1 reply; 7+ messages in thread From: Jason A. Donenfeld @ 2018-09-07 1:26 UTC (permalink / raw) To: ju.orth; +Cc: WireGuard mailing list Hi Julian, I'd thought of this early on, but failed to come up with what seemed like an actually realistic use case for it. On Thu, Sep 6, 2018 at 10:15 AM Julian Orth <ju.orth@gmail.com> wrote: > * If multiple processes are creating Wireguard devices at the same time, then > their device namespaces are isolated as long as each process uses its own > network namespace. This means that there is no problem if two processes try > to create the `wg0` device at the same time. The typical solution for this is to create "wg%d": zx2c4@thinkpad ~ $ ip link add wg%d type wireguard zx2c4@thinkpad ~ $ ip link add wg%d type wireguard zx2c4@thinkpad ~ $ ip link add wg%d type wireguard zx2c4@thinkpad ~ $ ip link add wg%d type wireguard zx2c4@thinkpad ~ $ ip link add wg%d type wireguard zx2c4@thinkpad ~ $ ip link show | grep wg 47: wg0: <POINTOPOINT,NOARP> mtu 1420 qdisc noop state DOWN mode DEFAULT group default qlen 1000 48: wg1: <POINTOPOINT,NOARP> mtu 1420 qdisc noop state DOWN mode DEFAULT group default qlen 1000 49: wg2: <POINTOPOINT,NOARP> mtu 1420 qdisc noop state DOWN mode DEFAULT group default qlen 1000 50: wg3: <POINTOPOINT,NOARP> mtu 1420 qdisc noop state DOWN mode DEFAULT group default qlen 1000 51: wg4: <POINTOPOINT,NOARP> mtu 1420 qdisc noop state DOWN mode DEFAULT group default qlen 1000 Or you just use a try-loop, incrementing until there are no races with another process who has already claimed it. Alternatively if you just generate a random byte sequence that's also a valid interface name, you get around 119.5 bits of randomness, which makes the possibility of collision for this use case sufficiently unlikely. (A random UUID only has 122 bits of randomness, for comparison.) > * The intention is for the `wg0` device to be used only within the `vpn` > namespace. It does not feel clean that the device has to live in the init > namespace for an arbitrarily short but non-zero amount of time. This also > leaks the existence of the `wg0` device to all processes living in the init > namespace. I wonder what happens with that "leak" that you're concerned with. It doesn't have to be configured with any information like ip addresses or routes, and the original name can be entirely different from the final name used. Jason ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Setting the transit namespace at runtime 2018-09-07 1:26 ` Jason A. Donenfeld @ 2018-09-07 19:06 ` Julian Orth 2018-09-09 22:27 ` Jason A. Donenfeld 0 siblings, 1 reply; 7+ messages in thread From: Julian Orth @ 2018-09-07 19:06 UTC (permalink / raw) To: wireguard Hi Jason, > I'd thought of this early on, but failed to come up with what seemed > like an actually realistic use case for it. How about creating Wireguard devices as a user that has no privileges/capabilites in the init namespace? $ unshare -r -U -m $ mount --bind /proc/self/ns/net init-ns $ unshare -n $ ./setup-wg0.sh $ wg set wg0 transit-net init-ns Julian ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Setting the transit namespace at runtime 2018-09-07 19:06 ` Julian Orth @ 2018-09-09 22:27 ` Jason A. Donenfeld 2018-09-10 7:16 ` Julian Orth 0 siblings, 1 reply; 7+ messages in thread From: Jason A. Donenfeld @ 2018-09-09 22:27 UTC (permalink / raw) To: Julian Orth; +Cc: WireGuard mailing list Hi Julian, On Fri, Sep 7, 2018 at 1:06 PM Julian Orth <ju.orth@gmail.com> wrote: > > I'd thought of this early on, but failed to come up with what seemed > > like an actually realistic use case for it. > > How about creating Wireguard devices as a user that has no > privileges/capabilites in the init namespace? > > $ unshare -r -U -m > $ mount --bind /proc/self/ns/net init-ns > $ unshare -n > $ ./setup-wg0.sh > $ wg set wg0 transit-net init-ns That looks to me like a security vulnerability. User namespace sets listen-port to < 1024, and then moves it into the target namespace, and bam, controls subverted. It might be wise, then, to include with this a capability check relative to the destination socket namespace, but that needs a check on both ends -- when you change the socket namespace and when you change the listen port, to make sure they correspond. However, if you're restricting setting the namespace and the listen port to cap_net_admin, then the above is no longer a good reason for this patchset, thereby begging my initial question. I saw you posted patches to the mailing list. I'll review these soon on my way back home on the flight tomorrow. In the mean time, if you send me your SSH public key (perhaps privately), we can add you to the WireGuard git repository, so that you can push to branches that begin with "jo/". Jason ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Setting the transit namespace at runtime 2018-09-09 22:27 ` Jason A. Donenfeld @ 2018-09-10 7:16 ` Julian Orth 0 siblings, 0 replies; 7+ messages in thread From: Julian Orth @ 2018-09-10 7:16 UTC (permalink / raw) To: Jason A. Donenfeld; +Cc: WireGuard mailing list [-- Attachment #1: Type: text/plain, Size: 1979 bytes --] Hello Jason, > That looks to me like a security vulnerability. User namespace sets > listen-port to < 1024, and then moves it into the target namespace, > and bam, controls subverted. Luckily this is not the case. The kernel code called by Wireguard to create a socket already checks that the executing program has the correct capabilities in the transit namespace. In the case you described, the call will fail if the program does not have CAP_NET_BIND_SERVICE in the transit namespace. To be clear: Moving the socket involves creating a new one in the target namespace and closing the old one. V1 of this series did, however, have a security vulnerability. It did not check if the program could create a socket in the transit namespace. While creating a socket in your current network namespace requires no capabilities, creating a socket in another network namespace requires CAP_SYS_ADMIN in that namespace: * Linux provides no syscalls to create a socket in a network namespace except for your current namespace. * Therefore you first have to enter this namespace to create the socket. * Entering a network namespace requires CAP_SYS_ADMIN in that namespace. I've talked about this in more detail here: [1]. In V2 I have therefore added the following capability check: To change the listen-port or the transit-netns, the caller must have CAP_NET_ADMIN in the transit namespace. At the same time, I've introduced a way to skip this capability check: If the caller can prove that he could implement Wireguard in userspace in the desired transit namespace, then he is allowed to open the socket without CAP_NET_ADMIN. The caller proves this by passing UDP sockets from the transit namespace into the kernel. And indeed, if Wireguard refused her, then she could simply bind these sockets to the desired port and run a userspace implementation of Wireguard. Julian [1] https://lists.zx2c4.com/pipermail/wireguard/2018-September/003342.html PS: I've attached my key. [-- Attachment #2: id_wireguard.pub --] [-- Type: application/vnd.ms-publisher, Size: 99 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-09-10 7:15 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-09-03 16:16 Setting the transit namespace at runtime Julian Orth 2018-09-06 20:42 ` Julian Orth 2018-09-07 1:29 ` Jason A. Donenfeld 2018-09-07 1:26 ` Jason A. Donenfeld 2018-09-07 19:06 ` Julian Orth 2018-09-09 22:27 ` Jason A. Donenfeld 2018-09-10 7:16 ` 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).