Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

oci/caps: limit available capabilities to current environment #42933

Merged
merged 1 commit into from
Oct 20, 2021

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Oct 13, 2021

fixes #42906 docker container is privileged tries to assume more capabilities than available
replaces / closes #42911 fix: don't attempt to set unavailable capabilities in privileged mode
fixes #42892 Recent test failures in CI (apply caps: operation not permitted) - except for TestHealthKillContainer, which is tracked in #41930)

Relates to:

In situations where docker runs in an environment where capabilities are limited,
sucn as docker-in-docker in a container created by older versions of docker, or
in a container where some capabilities have been disabled, starting a privileged
container may fail, because even though the kernel supports a capability, the
capability is not available.

This patch attempts to address this problem by limiting the list of "known" capa-
bilities on the set of effective capabilties for the current process. This code
is based on the code in containerd's "caps" package

- Description for the changelog

Detect list of available capabilities based on current environment, to allow running privileged containers in environments with a restricted set of capabilities.

- A picture of a cute animal (not mandatory but encouraged)

@thaJeztah thaJeztah added status/2-code-review kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. kind/bugfix PR's that fix bugs labels Oct 13, 2021
@thaJeztah thaJeztah added this to the 21.xx milestone Oct 13, 2021
@thaJeztah thaJeztah force-pushed the limit_caps_to_environment branch 2 times, most recently from f2f8d5d to 35a934f Compare October 13, 2021 13:01
}

// current returns a map of the effective known caps of the current process.
func current() (map[string]capability.Cap, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thaJeztah thaJeztah force-pushed the limit_caps_to_environment branch 3 times, most recently from e1c7b2a to 15d9973 Compare October 14, 2021 18:00
@thaJeztah
Copy link
Member Author

Looks like this doesn't solve one of the new test failures;

=== RUN   TestHealthKillContainer
    health_test.go:62: timeout hit after 10s: waiting for container to become healthy
--- FAIL: TestHealthKillContainer (12.48s)

@AkihiroSuda
Copy link
Member

TestHealthKillContainer

I guess that one is unrelated to caps?

@thaJeztah
Copy link
Member Author

I guess that one is unrelated to caps?

Yes, it looks unrelated to that. That test was already "flaky" #41930, but started failing 100% (?) at the same time as the capabilities issue occurred, which was when Jenkins agents were updated to kernel 5.11 (#42892), so I was hoping it would be related (and fixed by this PR), but more digging is needed for that one (perhaps it's just a race condition somehow)

@thaJeztah
Copy link
Member Author

Other than that, I updated this to use the containerd package and drop the go capabilities dependency as you suggested, so PTAL

There's still some code in the moby repository to do the capability "tweaking", and for the error handling etc, but tried to keep it close to what we had.

In situations where docker runs in an environment where capabilities are limited,
sucn as docker-in-docker in a container created by older versions of docker, or
in a container where some capabilities have been disabled, starting a privileged
container may fail, because even though the _kernel_ supports a capability, the
capability is not available.

This patch attempts to address this problem by limiting the list of "known" capa-
bilities on the set of effective capabilties for the current process. This code
is based on the code in containerd's "caps" package.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah force-pushed the limit_caps_to_environment branch from 7f508ef to 485cf38 Compare October 15, 2021 14:12
@thaJeztah
Copy link
Member Author

@tonistiigi PTAL

@thaJeztah
Copy link
Member Author

Only remaining failure is TestHealthKillContainer, which is tracked in #41930.

I'm bringing this one in; CI was just retriggered because I edited the top comment 😞, but it was "green" otherwise before

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bugfix PR's that fix bugs kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. status/2-code-review
Projects
None yet
3 participants