Remove tun/tap from the default device rules

[kolyshkin]

Looking through git blame, this was added by commit 9fac183
aka "Initial commit of runc binary", most probably by mistake.

Obviously, a container should not have access to tun/tap device, unless
it is explicitly specified in configuration.

Now, removing this might create a compatibility issue, but I see no
other choice.

Aside from the obvious misconfiguration, this should also fix the
annoying

Apr 26 03:46:56 foo.bar systemd[1]: Couldn't stat device /dev/char/10:200: No such file or directory

messages from systemd on every container start, when runc uses systemd
cgroup driver, and the system runs an old (< v240) version of systemd
(the message was presumably eliminated by [1]).

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1945929

[1] systemd/systemd@d5aecba

[rhatdan]

LGTM

[thaJeztah]

/cc @djs55 @justincormack @crosbymichael ptal

[thaJeztah]

SGTM

(I'm asking around a bit if others can see potential issues with the change)

[thaJeztah]

Got some feedback from some people; one person mentioned that this could affect users such as Tailscale, who may be using this. We have a joined Slack channel with them, so I put out a question to ask for feedback.

Also asking internally at Docker (Docker Desktop team didn't foresee issues they wouldn't be able to resolve), and asked the containerd maintainers.

[DentonGentry]

Got some feedback from some people; one person mentioned that this could affect users such as Tailscale, who may be using this. We have a joined Slack channel with them, so I put out a question to ask for feedback.

Tailscale's instructions and sample docker-compose.yml files say to explicitly include /dev/net/tun, anyone who referenced those shouldn't be impacted.

  devices:
    - /dev/net/tun:/dev/net/tun
  cap_add:
    - NET_ADMIN
    - NET_RAW

support@tailscale.com can watch for symptoms that look like a container which had been relying on getting a /dev/net/tun device via the prior default setting, and advise adding it in the docker-compose.yml or a command line argument.

[AkihiroSuda]

Cc @giuseppe Is this safe for rootless Podman?

[thaJeztah]

@kolyshkin would it be possible for you to post some test-builds from this PR somewhere for people to download?

[giuseppe]

Cc @giuseppe Is this safe for rootless Podman?

from a quick test, it seems the slirp4netns network works fine

[mrunalp]

LGTM

[kolyshkin]

@kolyshkin would it be possible for you to post some test-builds from this PR somewhere for people to download?

Those are actually available (for every PR), although not in a very consumable form. Click on "Checks" tab, when click "validate" from the left-side list, and on the bottom of that screen you'll see a big release-xxxxxx tarball under "Artifacts". It contains all the static binaries.

[thaJeztah]

@kolyshkin would it be possible for you to post some test-builds from this PR somewhere for people to download?

Those are actually available (for every PR), although not in a very consumable form. Click on "Checks" tab, when click "validate" from the left-side list, and on the bottom of that screen you'll see a big release-xxxxxx tarball under "Artifacts". It contains all the static binaries.

oh! I didn't know we were storing artefacts; let me pass that on!

[thaJeztah]

And.. there I went looking, but it's not in the Checks tab, it's in the top-level Actions tab; this is the last run of this PR; /~https://github.com/opencontainers/runc/actions/runs/2272679304

And download link is /~https://github.com/opencontainers/runc/suites/6379868023/artifacts/231538496

(super confusing!)

[fuweid]

LGTM(nb)

[cyphar]

LGTM, but I don't think this was a mistake -- the initial set of devices added was based on what was needed to get Docker containers to work with most programs people were using.

When I was reworking the devices handling code a year or two ago, I also wondered whether we should drop this (and I dropped some other far more dangerous rules, but left this one because @crosbymichael couldn't remember what this was needed for and I didn't want to break anything in a security update if I could help it).

That being said, Docker has their own default devices rules that are set externally to runc (so keeping this default is not necessary) and I don't see why a standard container would need tun/tap devices (though since the container is in a network namespace, I suspect that it wouldn't be an issue that they can create such devices). However I believe Docker generates their rules using our library functions, so maybe we should do a test build of Docker with this change and test that before we do a release?

[thaJeztah]

That being said, Docker has their own default devices rules that are set externally to runc (so keeping this default is not necessary)

Yes, I was thinking about that later; perhaps this change wouldn't affect the defaults on Docker (or Containerd for that matter).

However I believe Docker generates their rules using our library functions, so maybe we should do a test build of Docker with this change and test that before we do a release?

Good point; perhaps won't hurt to do a draft PR in the docker repo to verify (at least CI).

[thaJeztah]

opened moby/moby#43569 to get a run of CI with these changes

[kolyshkin]

opened moby/moby#43569 to get a run of CI with these changes

Seems passing. Seems that we have a consensus this can be merged. @opencontainers/runc-maintainers this is your last chance to say no :)

[rhatdan]

We removed CAP_MKNOD from default list in container tools years ago. I see no reason in the world that the default for containers should have this rule. We never hear complaints about this. and I believe CRI-O also defaults to no CAP_MKNOD.

[maggie44]

Biggest impact on the Kubernetes side for me so far, is having to run pods with privileged instead of NET_ADMIN (tailscale/tailscale#14262 (comment)). Seems like a big jump up in permissions just to gain the functionality that could have been achieved before with NET_ADMIN. I am creating /dev/net/tun in the container rather than host mounting, but have to grant elevated privileges to use it.

[nillestimothy]

Biggest impact on the Kubernetes side for me so far, is having to run pods with privileged instead of NET_ADMIN (tailscale/tailscale#14262 (comment)). Seems like a big jump up in permissions just to gain the functionality that could have been achieved before with NET_ADMIN. I am creating /dev/net/tun in the container rather than host mounting, but have to grant elevated privileges to use it.

It appears that we are trading one aspect of security for another. While I may not have extensive experience, isn’t specifying NET_ADMIN to limit access to /dev/net/tun an example of implementing the principle of least privilege?

Although this change was introduced two years ago, the recent update to containerd, which now uses the runc binary version 1.2.1 instead of 1.1.14, is beginning to impact a broader range of users. This includes Docker Swarm users, who are particularly affected by the inability to specify device mappings like /dev/net/tun directly in the deploy section of a Compose file.

[kolyshkin]

So, it looks like upper-level tools do not have ways to specify an additional device to be added to the list, and because of that users have to use privileged containers. This appears very wrong to me, so I'm open to reverting the change.

[maggie44]

So, it looks like upper-level tools do not have ways to specify an additional device to be added to the list, and because of that users have to use privileged containers. This appears very wrong to me, so I'm open to reverting the change.

It is certainly problematic having to grant privileged access, but that doesn’t change the fact that the original issue still stands, a container should not have access to tun/tap device, unless it is explicitly specified in configuration. I’m not sure the scope of NET_ADMIN, but mounting the device like with Docker is probably a better option overall than NET_ADMIN if it could be supported by upper level tools.

[thaJeztah]

So, it looks like upper-level tools do not have ways to specify an additional device to be added to the list,

For docker, it looks like it's possible for docker run (and docker compose) by adding it as device; #3468 (comment)

I'd have to check on the swarm services side; I know swarm services (by design) have been more constrained on some parts to keep the service-spec portable between nodes in a cluster.

[cyphar]

It also seems to be impacting a fair number of users, so we might have to revert it (especially if there really isn't a practical workaround in higher-level orchestration tools). Though, given my earlier comment about Docker having it's own device list maybe we could push for upstream runtimes to add this explicitly on their end? Idk...

It would be really nice if we could give a warning to users to let them know they should stop depending on this mis-feature and specify device configurations properly, but unfortunately there isn't a way of knowing ahead of time if a given container is going to use /dev/net/tun (there are ways of getting that information by tracing the container in various ways, but that kind of tracing would not fit with the runtime model of runc -- this would be something that higher level runtimes would need to implement, and I suspect they'd have little interest in doing that).

[ClashTheBunny]

Is there an issue we can follow that is tracking decisions on removal?

Is the only known way to get this working on kubernetes with 1.31 to elevate all of the internet accessable security critical containers to be fully privileged? k3s, for example, has hit 1.31 with the stable channel, so it's going to be going out widely, and once people put privileged: true into their manifests, you're not going to be getting that removed anytime soon, if ever.

[kolyshkin]

#4555

[SuzukiHonoka]

This breaks my application, I'll need to set privileged=true to use the tun device, which is very riscky than NET_ADMIN.