Setting the transit namespace at runtime

Setting the transit namespace at runtime

[Julian Orth]

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/

Re: Setting the transit namespace at runtime

[Julian Orth]

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

Re: Setting the transit namespace at runtime

[Jason A. Donenfeld]

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

Re: Setting the transit namespace at runtime

[Jason A. Donenfeld]

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

Re: Setting the transit namespace at runtime

[Julian Orth]

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

Re: Setting the transit namespace at runtime

[Jason A. Donenfeld]

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

Re: Setting the transit namespace at runtime

[Julian Orth]

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