fix: provide only available capabilities to insecure environment

[smira]

The problem this change is trying to fix are the environments where some
capabilities are already dropped, so they can't be granted to the
job with --security=insecure.

I know that probably fixed set of capabilities was implemented to
provide a stable build environment, but at the same time this breaks
environments with reduced capabilities.

Signed-off-by: Andrey Smirnov [email protected]

[smira]

One more idea is that we could find intersection of what is available and desired set of capabilities, so that build environment doesn't have more capabilities than the fixed list.

[tonistiigi]

Not sure if this is desired. We should provide a consistent environment between different machines and installations. If an app uses a capability in one system it should always work on another as well. And fail if the installation isn't capable of providing such an environment.

[smira]

Not sure if this is desired. We should provide a consistent environment between different machines and installations. If an app uses a capability in one system it should always work on another as well. And fail if the installation isn't capable of providing such an environment.

I fully agree with this, but I don't know at the same time how to combine that with potentially reduced capabilities.

Some background: in the last version of Talos we introduced a security feature to drop some of the capabilities from all processed but PID 1. The idea was that Talos should be able to use kexec, while no other process in the system should be able to use kexec for security reasons. This seems like a reasonable way to provide fast reboots on bare metal while we don't sacrifice security. So if we launch buildkit/buildx on top of Talos, it will have reduced set of capabilities to start with. We use Talos in Talos CI to run Talos builds which rely on --security=insecure to run some unit-tests which in turn launch containerd, so we need best of both worlds.

I don't know if the idea of keeping set of capabilities as it was before this change minus those which are not available right now would make more sense (so that job might have less capabilities, but not more).

[smira]

Also related, similar issue in dockerd moby/moby#42906

By the way, privileged pods launched with Kubernetes/containerd correctly handle reduced capability set: launched pods have maximum available capabilities, but it correctly workarounds not allowed capabilities.

[tonistiigi]

Generally, the solution for this should be to define more specific constraint like cap_admin or maybe something like --security=insecure,-sys_boot. Not very user-friendly though.

Feels hacky but I think I would even be more willing to just do an exception for kexec/sys_boot as it does feel a bit special case. I'd like to avoid case where someone implements something that doesn't have sys_admin and then even something as basic as mount will fail in the build in unexpected way(or build in a wrong way).

[smira]

I would rather see the list of capabilities being dynamic to some extent, as in Talos we dropped CAP_SYS_BOOT and CAP_SYS_MODULES (because Talos doesn't have kernel modules, and we don't want random pods to even being able to access it). So in the future I would rather see defaults going towards having less capabilities available. Another option might be to set a list of "insecure" capabilities via some configuration? as it's more specific to the host rather than a build?

Also looks like current list of capabilities in buildkit is not complete (kernel has more caps now), so what are we going to do with that in the future? add more or keep the list fixed?

[AkihiroSuda]

I'd like to avoid case where someone implements something that doesn't have sys_admin and then even something as basic as mount will fail in the build in unexpected way(or build in a wrong way).

Maybe we should just print a warning (or error out) when cap.Current() lacks SYS_ADMIN ?

[smira]

And third option is to print a warning if some capability is not available, but skip it to avoid failures? So that one can see that build environment is not exactly same (some caps missing), but at least proceed with the build?

[smira]

Updated the PR to skip caps (with warning) which are not available right now.

[tonistiigi]

CAP_SYS_BOOT and CAP_SYS_MODULES

This could be like --security=kernel but there will probably be many more specific cases. https://github.com/moby/libentitlement was supposed to define these logical groups but unfortunately it didn't go anywhere.

We don't currently have a good method for sending warnings to client progress. There are many use cases for it though if somebody is willing to spend some time on it.

[thaJeztah]

FWIW, I also opened a PR for consideration in runc (opencontainers/runc#3240), to make it match the runtime spec (opencontainers/runc#2854)

[smira]

I'm looking at opencontainers/runc#3240 can provide a better solution for unavailable caps. We might still want to expand capability list, but that would be way easier

[smira]

@tonistiigi I updated the PR, fixed all the tests. Two test failures seem to be unrelated to my changes.