Allow access to /dev/net/tun inside containers #18822

wlhlm · 2016-09-21T20:19:24Z

This allows containers to access /dev/net/tun by adding an option containers.<name>.enableTun that enables the correct capabilities and permissions for the container.

Also adds two generic options containers.<name>.additionalCapabilitites and containers.<name>.allowedDevices that help adding things like FUSE access (/dev/fuse) later down the road.

Motivation for this change

This change is useful for programs running in containers trying to set up tunnel interfaces. The current DevicePolicy prevents containers from accessing most device files thus explicitly allow /dev/net/tun.

Fixes #18708.

/cc @Mic92

Things done

Tested using nixos-rebuild test
Built on platform(s)
- NixOS
- OS X
- Linux
Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Fits CONTRIBUTING.md.

mention-bot · 2016-09-21T20:19:25Z

@wlhlm, thanks for your PR! By analyzing the annotation information on this pull request, we identified @kampfschlaefer, @edolstra and @wavewave to be potential reviewers

Mic92 · 2016-09-21T21:20:24Z

nixos/modules/virtualisation/containers.nix

@@ -518,6 +518,7 @@ in
        KillSignal = "WINCH";

        DevicePolicy = "closed";
+        DeviceAllow = "/dev/net/tun rw";


I am not sure, what a good policy would be here.
One device people could also request in future is /dev/fuse.

I think preventing containers from accessing device files by default using a strict DevicePolicy (closed or strict) and explicitly whitelisting devices that are "safe" to use is a good choice here. Additional devices can be allowed in subsequent PRs, of course.

Personally, I'm not sure how safe it is to use FUSE in containers (but I don't see an obvious problem with it).

Maybe adding a list of allowed devices (with a default value?), would be helpful, so people can decide them self, what to allow.

That sounds like a good idea. I see what I can do.

Hmm, problem is that I can't have multiple DeviceAllow lines, since nix doesn't allow multiple attributes with the same name.

argh, this is really an issue in nix's generating of systemd units, which I encountered multiple times already.

I opened an issue regarding this, but I currently lack of a good idea, how this could be solved properly in future.

It is possible to pass a list to DeviceAllow instead.

kampfschlaefer · 2016-09-23T08:10:59Z

Hi,

could you please also make the list of allowed devices a configuration thing? I actually don't want all my containers to work with tun/tap devices. Only the single instance that is running openvpn (in the future).

And you might want to write a test for that. There are quite a number of container-networking tests already. You could add one that has one host with two containers, one with tun/tap enable, one without and then check whether access to tun device works or not. Also Maybe the openvpn module could make use of a test that uses two containers instead of two VMs? (Okay, openvpn could actually use a test.)

tadfisher · 2016-09-24T19:28:26Z

If DeviceAllow were configurable from the container definition, could the OpenVPN module add the required device itself? That would solve the problem of requiring extra container-level configuration when running/not running this service.

kampfschlaefer · 2016-09-24T19:37:49Z

In the first iteration this would mean that admins creating the container with openvpn inside would have to add the option. But it would prevent all other (existing) admins from opening their containers via undesired tun/tap. So this is firstly a security issue for me.

The openvpn module/service in nixos could then detect whether its inside a container (I think boot.isContainer == true) and maybe add the relevant option.

I could also help you with writing the test. Somehow I started liking system tests in NixOS so much I started writing them a lot on my spare time 😉

wlhlm · 2016-09-24T21:35:45Z

I'm not sure if OpenVPN should implicitly give itself the necessary privileges (capability+device access) when it detects that it's inside a container.

Basically, what I was thinking was to merge this PR with the other one (#18823 - capabilities) to make things easier. Than I'd add options containers.<name>.additionalCapabilities for configuring capabilities and containers.<name>.allowedDevices for giving the container access to device files. Having these generic options would make things easier when adding things like FUSE access later down the road.

I would then add a third option containers.<name>.enableTun that would add CAP_NET_ADMIN and /dev/net/tun to the previously mentioned options. An assertion or lib.warn would be added to the openvpn module indicating to the admin that containers.<name>.enableTun = true is required when it detects that it runs inside a container. That way, the admin has to explicitly enable tunnel support in the container.

What do you think?

This adds the containers.<name>.enableTun option allowing containers to access /dev/net/tun. This is required by openvpn, tinc, etc. in order to work properly inside containers. The new option builds on top of two generic options containers.<name>.additionalCapabilities and containers.<name>.allowedDevices which also can be used for example when adding support for FUSE later down the road.

wlhlm · 2016-09-25T17:55:02Z

I have updated the branch according to the plan I described in the previous comment. Please have a look!

This is currently missing tests, which I'll be looking into.

kampfschlaefer · 2016-09-26T08:06:36Z

Sadly no new container test. How do we know it actually works. And how do we ensure that it still works in 17.03?

wlhlm · 2016-09-26T08:13:13Z

As I said, I'm still working on tests. I've tested against the master snapshot of nixpkgs with nixos-rebuild test -I nixpkgs=nixpkgs-master which currently corresponds to 17.03 as I understand it.

kampfschlaefer · 2016-09-26T10:08:51Z

Ah, I am sorry. The new review-feature of github irritated me and let me believe this is already merged. Sorry. Keep on testing 😉

kampfschlaefer · 2016-09-27T07:41:11Z

nixos/modules/virtualisation/containers.nix

+                information.
+              '';
+            };
+            enableTun = mkOption {


I am unsure whether "enableTun" should be an explicit option or just a good example for "allowedDevices". It will probably be the primary use-case, but is it worth the special handling?

It hides implementation details, people might don't know what is needed in order to use tun devices inside a container. Therefor it improves usability. As it only describe the semantics, in future more options might be required to enable tun devices container. In this respect it is comparable to systemd's hardening flags: PrivateDevices=true creates a private /dev with /dev/null, /dev/zero and /dev/random. However in future this list can be extended without breaking the semantic.

@kampfschlaefer The plan is to add more similar options later on. For example, I can imagine an enableFuse option, because FUSE requires access to /dev/fuse. Having multiple options like containers.<name>.enable{Tun,Fuse} that abstract over capabilities and device access make things easier for admins.

EDIT: Basically, what @Mic92 already said. 😉

For /dev/fuse you may find this info helpful: systemd/systemd#2178 (comment)

Mic92 · 2016-09-30T22:19:11Z

Thanks!

Mic92 reviewed Sep 21, 2016

View reviewed changes

Mic92 mentioned this pull request Sep 22, 2016

systemd.services.<service>.serviceConfig should support multiple entries per option #18838

Closed

LnL7 added the 6.topic: nixos label Sep 22, 2016

wlhlm mentioned this pull request Sep 25, 2016

Add option to grant additional capabilities to containers #18823

Closed

7 tasks

Mic92 approved these changes Sep 25, 2016

View reviewed changes

tadfisher approved these changes Sep 25, 2016

View reviewed changes

kampfschlaefer reviewed Sep 27, 2016

View reviewed changes

Mic92 merged commit a8c172c into NixOS:master Sep 30, 2016

rasendubi mentioned this pull request Oct 13, 2016

Allow access to /dev/net/tun inside containers (port to 16.09) #19523

Merged

7 tasks

nh2 mentioned this pull request Dec 8, 2017

How to specify container options with targetEnv = "container"? NixOS/nixops#802

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow access to /dev/net/tun inside containers #18822

Allow access to /dev/net/tun inside containers #18822

wlhlm commented Sep 21, 2016 •

edited

mention-bot commented Sep 21, 2016

Mic92 Sep 21, 2016

wlhlm Sep 22, 2016

Mic92 Sep 22, 2016

wlhlm Sep 22, 2016

wlhlm Sep 22, 2016

Mic92 Sep 22, 2016 •

edited

Mic92 Sep 22, 2016

Mic92 Sep 22, 2016

kampfschlaefer commented Sep 23, 2016

tadfisher commented Sep 24, 2016

kampfschlaefer commented Sep 24, 2016

wlhlm commented Sep 24, 2016 •

edited

wlhlm commented Sep 25, 2016 •

edited

kampfschlaefer commented Sep 26, 2016

wlhlm commented Sep 26, 2016 •

edited

kampfschlaefer commented Sep 26, 2016

kampfschlaefer Sep 27, 2016

Mic92 Sep 27, 2016 •

edited

wlhlm Sep 27, 2016 •

edited

nh2 Dec 8, 2017

Mic92 commented Sep 30, 2016

Allow access to /dev/net/tun inside containers #18822

Allow access to /dev/net/tun inside containers #18822

Conversation

wlhlm commented Sep 21, 2016 • edited

Motivation for this change

Things done

mention-bot commented Sep 21, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mic92 Sep 22, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kampfschlaefer commented Sep 23, 2016

tadfisher commented Sep 24, 2016

kampfschlaefer commented Sep 24, 2016

wlhlm commented Sep 24, 2016 • edited

wlhlm commented Sep 25, 2016 • edited

kampfschlaefer commented Sep 26, 2016

wlhlm commented Sep 26, 2016 • edited

kampfschlaefer commented Sep 26, 2016

Choose a reason for hiding this comment

Mic92 Sep 27, 2016 • edited

Choose a reason for hiding this comment

wlhlm Sep 27, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mic92 commented Sep 30, 2016

wlhlm commented Sep 21, 2016 •

edited

Mic92 Sep 22, 2016 •

edited

wlhlm commented Sep 24, 2016 •

edited

wlhlm commented Sep 25, 2016 •

edited

wlhlm commented Sep 26, 2016 •

edited

Mic92 Sep 27, 2016 •

edited

wlhlm Sep 27, 2016 •

edited