capabilities: WARN, not ERROR, for unknown / unavailable capabilities #2854

thaJeztah · 2021-03-15T12:05:31Z

go.mod: OCI runtime-spec v1.0.3-0.20210326190908-1c3f411f0417

full diff: opencontainers/runtime-spec@a8c4a9e...1c3f411

Fix seccomp notify inconsistencies
- seccomp: Add missing const for seccomp notify action
- schema/defs-linux: Fix inconsistencies with seccomp notify
Proposal: runtime should ignore capabilities that cannot be granted

capabilities: WARN, not ERROR, for unknown / unavailable capabilities

This updates handling of capabilities to match the updated runtime specification,
in opencontainers/runtime-spec#1094.

Prior to that change, the specification required runtimes to produce a (fatal)
error if a container configuration requested capabilities that could not be
granted (either the capability is "unknown" to the runtime, not supported by the
kernel version in use, or not available in the environment that the runtime
operates in).

This caused problems in situations where the runtime was running in a restricted
environment (for example, docker-in-docker), or if there is a mismatch between
the list of capabilities known by higher-level runtimes and the OCI runtime.

Some examples:

Kernel 5.8 introduced CAP_PERFMON, CAP_BPF, and CAP_CHECKPOINT_RESTORE
capabilities. Docker 20.10.0 ("higher level runtime") shipped with
an updated list of capabilities, and when creating a "privileged" container,
would determine what capabilities are known by the kernel in use, and request
all those capabilities (by including them in the container config).
However, runc did not yet have an updated list of capabilities, and therefore
reject the container specification, producing an error because the new
capabilities were "unknown".
When running nested containers, for example, when running docker-in-docker,
the "inner" container may be using a more recent version of docker than the
"outer" container. In this situation, the "outer" container may be missing
capabilities that the inner container expects to be supported (based on
kernel version). However, starting the container would fail, because the OCI
runtime could not grant those capabilities (them not being available in the
environment it's running in).

WARN (but otherwise ignore) capabilities that cannot be granted

This patch changes the handling to WARN (but otherwise ignore) capabilities that
are requested in the container config, but cannot be granted, alleviating higher
level runtimes to detect what capabilities are supported (by the kernel, and
in the current environment), as well as avoiding failures in situations where
the higher-level runtime is aware of capabilities that are not (yet) supported
by runc.

Impact on security

Given that capabilities is an "allow-list", ignoring unknown capabilities does
not impose a security risk; worst case, a container does not get all requested
capabilities granted and, as a result, some actions may fail.

Backward-compatibility

This change should be fully backward compatible. Higher-level runtimes that
already dynamically adjust the list of requested capabilities can continue to do
so. Runtimes that do not adjust will see an improvement (containers can start
even if some of the requested capabilities are not granted). Container processes
MAY fail (as described in "impact on security"), but users can debug this
situation either by looking at the warnings produces by the OCI runtime, or using
tools such as capsh / libcap to get the list of actual capabilities in the
container.

Signed-off-by: Sebastiaan van Stijn [email protected]

thaJeztah · 2021-03-15T13:22:05Z

@kolyshkin @AkihiroSuda ptal; upstream change wasn't accepted yet, but we can probably already discuss this (possible) implementation.

thaJeztah · 2021-03-15T13:23:21Z

libcontainer/capabilities/capabilities.go

+	var unknownCaps = map[string]struct{}{}
+	for _, c := range warnings {
+		if _, ok := unknownCaps[c]; !ok {
+			logrus.WithField("capability", c).Warn("ignoring unknown or unavailable capability")


Only consideration I had here was if we wanted to pass a logger to New(), instead of always using the default logrus logger (in case this is used elsewhere as library)

Also not sure if it must be a WARN level, or if we think INFO (possibly with "warning" in the log message) to reduce noise

I'd rather have a single message listing all the capabilities, maybe something like Warning: unknown capabilities [CAP_ONE CAP_TWO ...] are ignored.

libcontainer/capabilities/capabilities.go

kolyshkin · 2021-03-16T17:38:07Z

libcontainer/capabilities/capabilities.go

+	}
+}
+
+// capSlice converts the given slice of capability names, and converts them


nit: converts ... and converts ?

vendor/modules.txt

thaJeztah · 2021-03-17T16:32:00Z

@kolyshkin I played around with some approaches; did some refactoring (first two commits); let me know if you like this approach

libcontainer/capabilities/capabilities.go

kolyshkin · 2021-03-19T03:07:32Z

I played around with some approaches; did some refactoring (first two commits); let me know if you like this approach

Thanks, I like the refactoring and the map approach; left a few nits wrt naming and a way to easier print the unknown caps.

thaJeztah · 2021-03-19T09:52:47Z

@kolyshkin updated; PTAL

Let me also open the refactor (first two commits) as a separate PR

thaJeztah · 2021-03-19T09:56:37Z

opened #2863

thaJeztah · 2021-03-22T08:48:17Z

Rebased to get rid of the commits of #2863

kolyshkin

LGTM

full diff: opencontainers/runtime-spec@a8c4a9e...1c3f411 - Fix seccomp notify inconsistencies - seccomp: Add missing const for seccomp notify action - schema/defs-linux: Fix inconsistencies with seccomp notify - Proposal: runtime should ignore capabilities that cannot be granted Signed-off-by: Sebastiaan van Stijn <[email protected]>

This updates handling of capabilities to match the updated runtime specification, in opencontainers/runtime-spec#1094. Prior to that change, the specification required runtimes to produce a (fatal) error if a container configuration requested capabilities that could not be granted (either the capability is "unknown" to the runtime, not supported by the kernel version in use, or not available in the environment that the runtime operates in). This caused problems in situations where the runtime was running in a restricted environment (for example, docker-in-docker), or if there is a mismatch between the list of capabilities known by higher-level runtimes and the OCI runtime. Some examples: - Kernel 5.8 introduced CAP_PERFMON, CAP_BPF, and CAP_CHECKPOINT_RESTORE capabilities. Docker 20.10.0 ("higher level runtime") shipped with an updated list of capabilities, and when creating a "privileged" container, would determine what capabilities are known by the kernel in use, and request all those capabilities (by including them in the container config). However, runc did not yet have an updated list of capabilities, and therefore reject the container specification, producing an error because the new capabilities were "unknown". - When running nested containers, for example, when running docker-in-docker, the "inner" container may be using a more recent version of docker than the "outer" container. In this situation, the "outer" container may be missing capabilities that the inner container expects to be supported (based on kernel version). However, starting the container would fail, because the OCI runtime could not grant those capabilities (them not being available in the environment it's running in). WARN (but otherwise ignore) capabilities that cannot be granted -------------------------------------------------------------------------------- This patch changes the handling to WARN (but otherwise ignore) capabilities that are requested in the container config, but cannot be granted, alleviating higher level runtimes to detect what capabilities are supported (by the kernel, and in the current environment), as well as avoiding failures in situations where the higher-level runtime is aware of capabilities that are not (yet) supported by runc. Impact on security -------------------------------------------------------------------------------- Given that `capabilities` is an "allow-list", ignoring unknown capabilities does not impose a security risk; worst case, a container does not get all requested capabilities granted and, as a result, some actions may fail. Backward-compatibility -------------------------------------------------------------------------------- This change should be fully backward compatible. Higher-level runtimes that already dynamically adjust the list of requested capabilities can continue to do so. Runtimes that do not adjust will see an improvement (containers can start even if some of the requested capabilities are not granted). Container processes MAY fail (as described in "impact on security"), but users can debug this situation either by looking at the warnings produces by the OCI runtime, or using tools such as `capsh` / `libcap` to get the list of actual capabilities in the container. Signed-off-by: Sebastiaan van Stijn <[email protected]>

thaJeztah · 2021-03-26T20:25:44Z

Proposal was accepted in the runtime-spec; I updated the runtime spec in go.mod so that this change matches the version of the spec we implement, rebase, and moved this out of draft

@AkihiroSuda @kolyshkin @mrunalp PTAL

cpuguy83 · 2021-03-31T17:11:00Z

libcontainer/capabilities/capabilities_linux_test.go

@@ -17,10 +21,36 @@ func TestNew(t *testing.T) {
 		Ambient:     cs,
 	}

+	hook := test.NewGlobal()


Yes, I had a bright spark and recalled seeing something about "testing" in the logrus readme, and thought to give it a try; https://github.com/sirupsen/logrus#testing.

cpuguy83

LGTM

kolyshkin

LGTM

kolyshkin · 2021-04-01T22:21:56Z

close/reopen to kick ci

thaJeztah force-pushed the runc_warn_unknown_caps branch from 877182d to f87e767 Compare March 15, 2021 12:06

thaJeztah changed the title ~~capabilities: WARN instead or ERROR for unknown / unavailable capabilities~~ capabilities: WARN, not ERROR, for unknown / unavailable capabilities Mar 15, 2021

thaJeztah force-pushed the runc_warn_unknown_caps branch 2 times, most recently from 75fefcf to 04c3abc Compare March 15, 2021 12:13

thaJeztah mentioned this pull request Mar 15, 2021

runtime should WARN / ignore capabilities that cannot be granted opencontainers/runtime-spec#1094

Merged

thaJeztah commented Mar 15, 2021

View reviewed changes

kolyshkin reviewed Mar 16, 2021

View reviewed changes

libcontainer/capabilities/capabilities.go Outdated Show resolved Hide resolved

kolyshkin reviewed Mar 16, 2021

View reviewed changes

vendor/modules.txt Show resolved Hide resolved

AkihiroSuda added impact/changelog enhancement labels Mar 16, 2021

AkihiroSuda added this to the 1.0.0-rc94 milestone Mar 16, 2021

thaJeztah force-pushed the runc_warn_unknown_caps branch 2 times, most recently from af9e9f1 to a06504b Compare March 17, 2021 16:28

kolyshkin reviewed Mar 19, 2021

View reviewed changes

libcontainer/capabilities/capabilities.go Outdated Show resolved Hide resolved

kolyshkin reviewed Mar 19, 2021

View reviewed changes

libcontainer/capabilities/capabilities.go Outdated Show resolved Hide resolved

kolyshkin reviewed Mar 19, 2021

View reviewed changes

libcontainer/capabilities/capabilities.go Outdated Show resolved Hide resolved

kolyshkin reviewed Mar 19, 2021

View reviewed changes

libcontainer/capabilities/capabilities.go Outdated Show resolved Hide resolved

kolyshkin reviewed Mar 19, 2021

View reviewed changes

libcontainer/capabilities/capabilities.go Outdated Show resolved Hide resolved

thaJeztah force-pushed the runc_warn_unknown_caps branch from a06504b to 9a48577 Compare March 19, 2021 09:51

thaJeztah mentioned this pull request Mar 19, 2021

capabilities.Caps: use a map for capability-types #2863

Merged

thaJeztah force-pushed the runc_warn_unknown_caps branch 3 times, most recently from f5b43cd to 28f805b Compare March 22, 2021 08:08

kolyshkin previously approved these changes Mar 25, 2021

View reviewed changes

thaJeztah added 2 commits March 26, 2021 21:22

thaJeztah dismissed kolyshkin’s stale review via 5fb831a March 26, 2021 20:23

thaJeztah force-pushed the runc_warn_unknown_caps branch from 28f805b to 5fb831a Compare March 26, 2021 20:23

thaJeztah marked this pull request as ready for review March 26, 2021 20:24

AkihiroSuda approved these changes Mar 27, 2021

View reviewed changes

thaJeztah requested a review from kolyshkin March 27, 2021 18:25

cpuguy83 reviewed Mar 31, 2021

View reviewed changes

cpuguy83 approved these changes Mar 31, 2021

View reviewed changes

AkihiroSuda requested a review from cyphar April 1, 2021 04:22

kolyshkin approved these changes Apr 1, 2021

View reviewed changes

kolyshkin closed this Apr 1, 2021

kolyshkin reopened this Apr 1, 2021

kolyshkin merged commit 6538f9f into opencontainers:master Apr 2, 2021

thaJeztah deleted the runc_warn_unknown_caps branch April 2, 2021 06:53

kolyshkin mentioned this pull request May 3, 2021

rc94 discussion (mid-April 2021?) #2790

Closed

thaJeztah mentioned this pull request May 6, 2021

oci/caps: refactor, remove unused code, and improved error messages moby/moby#41459

Merged

AkihiroSuda mentioned this pull request Jul 19, 2021

Revert "Temporarily disable CAP_PERFMON, CAP_BPF, and CAP_CHECKPOINT_RESTORE moby/moby#42011

Merged

thaJeztah mentioned this pull request Oct 13, 2021

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

Merged

kolyshkin mentioned this pull request Oct 4, 2024

Treating unknown/unavailable capabilities #4426

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

capabilities: WARN, not ERROR, for unknown / unavailable capabilities #2854

capabilities: WARN, not ERROR, for unknown / unavailable capabilities #2854

thaJeztah commented Mar 15, 2021 •

edited

Loading

thaJeztah commented Mar 15, 2021

thaJeztah Mar 15, 2021

thaJeztah Mar 15, 2021

kolyshkin Mar 16, 2021

kolyshkin Mar 16, 2021

thaJeztah commented Mar 17, 2021

kolyshkin commented Mar 19, 2021

thaJeztah commented Mar 19, 2021

thaJeztah commented Mar 19, 2021

thaJeztah commented Mar 22, 2021

kolyshkin left a comment

thaJeztah commented Mar 26, 2021

cpuguy83 Mar 31, 2021

thaJeztah Mar 31, 2021

cpuguy83 left a comment

kolyshkin left a comment

kolyshkin commented Apr 1, 2021

capabilities: WARN, not ERROR, for unknown / unavailable capabilities #2854

capabilities: WARN, not ERROR, for unknown / unavailable capabilities #2854

Conversation

thaJeztah commented Mar 15, 2021 • edited Loading

go.mod: OCI runtime-spec v1.0.3-0.20210326190908-1c3f411f0417

capabilities: WARN, not ERROR, for unknown / unavailable capabilities

WARN (but otherwise ignore) capabilities that cannot be granted

Impact on security

Backward-compatibility

thaJeztah commented Mar 15, 2021

thaJeztah Mar 15, 2021

Choose a reason for hiding this comment

thaJeztah Mar 15, 2021

Choose a reason for hiding this comment

kolyshkin Mar 16, 2021

Choose a reason for hiding this comment

kolyshkin Mar 16, 2021

Choose a reason for hiding this comment

thaJeztah commented Mar 17, 2021

kolyshkin commented Mar 19, 2021

thaJeztah commented Mar 19, 2021

thaJeztah commented Mar 19, 2021

thaJeztah commented Mar 22, 2021

kolyshkin left a comment

Choose a reason for hiding this comment

thaJeztah commented Mar 26, 2021

cpuguy83 Mar 31, 2021

Choose a reason for hiding this comment

thaJeztah Mar 31, 2021

Choose a reason for hiding this comment

cpuguy83 left a comment

Choose a reason for hiding this comment

kolyshkin left a comment

Choose a reason for hiding this comment

kolyshkin commented Apr 1, 2021

thaJeztah commented Mar 15, 2021 •

edited

Loading