From 75fefcf537094f342a4fd19275a069f936800682 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 15 Mar 2021 12:19:51 +0100 Subject: [PATCH] capabilities: WARN, not ERROR, for unknown / unavailable capabilities This updates handling of capabilities to match the updated runtime specification, in https://github.com/opencontainers/runtime-spec/pull/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 --- libcontainer/capabilities/capabilities.go | 56 ++++++++---- .../capabilities/capabilities_linux_test.go | 59 ++++++++++++ .../sirupsen/logrus/hooks/test/test.go | 91 +++++++++++++++++++ vendor/modules.txt | 1 + 4 files changed, 190 insertions(+), 17 deletions(-) create mode 100644 libcontainer/capabilities/capabilities_linux_test.go create mode 100644 vendor/github.com/sirupsen/logrus/hooks/test/test.go diff --git a/libcontainer/capabilities/capabilities.go b/libcontainer/capabilities/capabilities.go index adbf6330c48..60022ab99b2 100644 --- a/libcontainer/capabilities/capabilities.go +++ b/libcontainer/capabilities/capabilities.go @@ -3,10 +3,10 @@ package capabilities import ( - "fmt" "strings" "github.com/opencontainers/runc/libcontainer/configs" + "github.com/sirupsen/logrus" "github.com/syndtr/gocapability/capability" ) @@ -24,27 +24,31 @@ func init() { } } -// New creates a new Caps from the given Capabilities config. +// New creates a new Caps from the given Capabilities config. Unknown Capabilities +// or Capabilities that are unavailable in the current environment are ignored, +// printing a warning instead. func New(capConfig *configs.Capabilities) (*Caps, error) { var ( - err error - caps Caps + err error + caps Caps + ignored []string + warnings = make([]string, 0) ) - if caps.bounding, err = capSlice(capConfig.Bounding); err != nil { - return nil, err + if caps.bounding, ignored = capSlice(capConfig.Bounding); len(ignored) > 0 { + warnings = append(warnings, ignored...) } - if caps.effective, err = capSlice(capConfig.Effective); err != nil { - return nil, err + if caps.effective, ignored = capSlice(capConfig.Effective); len(ignored) > 0 { + warnings = append(warnings, ignored...) } - if caps.inheritable, err = capSlice(capConfig.Inheritable); err != nil { - return nil, err + if caps.inheritable, ignored = capSlice(capConfig.Inheritable); len(ignored) > 0 { + warnings = append(warnings, ignored...) } - if caps.permitted, err = capSlice(capConfig.Permitted); err != nil { - return nil, err + if caps.permitted, ignored = capSlice(capConfig.Permitted); len(ignored) > 0 { + warnings = append(warnings, ignored...) } - if caps.ambient, err = capSlice(capConfig.Ambient); err != nil { - return nil, err + if caps.ambient, ignored = capSlice(capConfig.Ambient); len(ignored) > 0 { + warnings = append(warnings, ignored...) } if caps.pid, err = capability.NewPid2(0); err != nil { return nil, err @@ -52,19 +56,37 @@ func New(capConfig *configs.Capabilities) (*Caps, error) { if err = caps.pid.Load(); err != nil { return nil, err } + printWarnings(warnings) return &caps, nil } -func capSlice(caps []string) ([]capability.Cap, error) { +func printWarnings(warnings []string) { + if len(warnings) == 0 { + return + } + var unknownCaps = map[string]struct{}{} + for _, c := range warnings { + if _, ok := unknownCaps[c]; !ok { + logrus.WithField("capability", c).Warn("ignoring unknown or unavailable capability") + unknownCaps[c] = struct{}{} + } + } +} + +// capSlice converts the given slice of capability names, and converts them +// to their numeric equivalent. Names of unknown or unavailable capabilities +// are returned and can be used by the caller to print as a warning. +func capSlice(caps []string) ([]capability.Cap, []string) { out := make([]capability.Cap, len(caps)) + ignored := make([]string, 0) for i, c := range caps { v, ok := capabilityMap[c] if !ok { - return nil, fmt.Errorf("unknown capability %q", c) + ignored = append(ignored, c) } out[i] = v } - return out, nil + return out, ignored } // Caps holds the capabilities for a container. diff --git a/libcontainer/capabilities/capabilities_linux_test.go b/libcontainer/capabilities/capabilities_linux_test.go new file mode 100644 index 00000000000..4bb6db9955f --- /dev/null +++ b/libcontainer/capabilities/capabilities_linux_test.go @@ -0,0 +1,59 @@ +package capabilities + +import ( + "io/ioutil" + "os" + "testing" + + "github.com/opencontainers/runc/libcontainer/configs" + "github.com/sirupsen/logrus" + "github.com/sirupsen/logrus/hooks/test" +) + +func TestNewWarnings(t *testing.T) { + cs := []string{"CAP_CHOWN", "CAP_UNKNOWN"} + conf := configs.Capabilities{ + Bounding: cs, + Effective: cs, + Inheritable: cs, + Permitted: cs, + Ambient: cs, + } + + hook := test.NewGlobal() + defer hook.Reset() + + logrus.SetOutput(ioutil.Discard) + _, err := New(&conf) + logrus.SetOutput(os.Stderr) + + if err != nil { + t.Error(err) + } + e := hook.AllEntries() + if len(e) != 1 { + t.Errorf("expected 1 warning, got %d", len(e)) + } + + expected := logrus.Entry{ + Data: logrus.Fields{"capability": "CAP_UNKNOWN"}, + Level: logrus.WarnLevel, + Message: "ignoring unknown or unavailable capability", + } + + l := hook.LastEntry() + if l == nil { + t.Fatal("expected a warning, but got none") + } + if l.Level != expected.Level { + t.Errorf("expected %s, got %s", expected.Level, l.Level) + } + if l.Data["capability"] != expected.Data["capability"] { + t.Errorf("expected %v, got %v", expected.Data["capability"], l.Data["capability"]) + } + if l.Message != expected.Message { + t.Errorf("expected %s, got %s", expected.Message, l.Message) + } + + hook.Reset() +} diff --git a/vendor/github.com/sirupsen/logrus/hooks/test/test.go b/vendor/github.com/sirupsen/logrus/hooks/test/test.go new file mode 100644 index 00000000000..b16d06654ae --- /dev/null +++ b/vendor/github.com/sirupsen/logrus/hooks/test/test.go @@ -0,0 +1,91 @@ +// The Test package is used for testing logrus. +// It provides a simple hooks which register logged messages. +package test + +import ( + "io/ioutil" + "sync" + + "github.com/sirupsen/logrus" +) + +// Hook is a hook designed for dealing with logs in test scenarios. +type Hook struct { + // Entries is an array of all entries that have been received by this hook. + // For safe access, use the AllEntries() method, rather than reading this + // value directly. + Entries []logrus.Entry + mu sync.RWMutex +} + +// NewGlobal installs a test hook for the global logger. +func NewGlobal() *Hook { + + hook := new(Hook) + logrus.AddHook(hook) + + return hook + +} + +// NewLocal installs a test hook for a given local logger. +func NewLocal(logger *logrus.Logger) *Hook { + + hook := new(Hook) + logger.Hooks.Add(hook) + + return hook + +} + +// NewNullLogger creates a discarding logger and installs the test hook. +func NewNullLogger() (*logrus.Logger, *Hook) { + + logger := logrus.New() + logger.Out = ioutil.Discard + + return logger, NewLocal(logger) + +} + +func (t *Hook) Fire(e *logrus.Entry) error { + t.mu.Lock() + defer t.mu.Unlock() + t.Entries = append(t.Entries, *e) + return nil +} + +func (t *Hook) Levels() []logrus.Level { + return logrus.AllLevels +} + +// LastEntry returns the last entry that was logged or nil. +func (t *Hook) LastEntry() *logrus.Entry { + t.mu.RLock() + defer t.mu.RUnlock() + i := len(t.Entries) - 1 + if i < 0 { + return nil + } + return &t.Entries[i] +} + +// AllEntries returns all entries that were logged. +func (t *Hook) AllEntries() []*logrus.Entry { + t.mu.RLock() + defer t.mu.RUnlock() + // Make a copy so the returned value won't race with future log requests + entries := make([]*logrus.Entry, len(t.Entries)) + for i := 0; i < len(t.Entries); i++ { + // Make a copy, for safety + entries[i] = &t.Entries[i] + } + return entries +} + +// Reset removes all Entries from this test hook. +func (t *Hook) Reset() { + t.mu.Lock() + defer t.mu.Unlock() + t.Entries = make([]logrus.Entry, 0) +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 940300a1e18..f9f836c3236 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -57,6 +57,7 @@ github.com/shurcooL/sanitized_anchor_name # github.com/sirupsen/logrus v1.7.0 ## explicit github.com/sirupsen/logrus +github.com/sirupsen/logrus/hooks/test # github.com/syndtr/gocapability v0.0.0-20200815063812-42c35b437635 ## explicit github.com/syndtr/gocapability/capability