Repository: opencontainers/runc
Pull request #2854 (2021-03-15 12:05:31)

capabilities: WARN, not ERROR, for unknown / unavailable capabilities
### go.mod: OCI runtime-spec v1.0.3-0.20210326190908-1c3f411f0417

full diff: https://github.com/opencontainers/runtime-spec/compare/a8c4a9ee0f6b...1c3f411f0417

- 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 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 <github@gone.nl>

thaJeztah: @kolyshkin @AkihiroSuda ptal; upstream change wasn't accepted yet, but we can probably already discuss this (possible) implementation.
thaJeztah: @kolyshkin I played around with some approaches; did some refactoring (first two commits); let me know if you like this approach
kolyshkin: > 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: @kolyshkin updated; PTAL

Let me also open the refactor (first two commits) as a separate PR
thaJeztah: opened https://github.com/opencontainers/runc/pull/2863
thaJeztah: Rebased to get rid of the commits of https://github.com/opencontainers/runc/pull/2863

Patch:
diff --git a/go.mod b/go.mod
index 00322231..f05571cc 100644
--- a/go.mod
+++ b/go.mod
@@ -13,7 +13,7 @@ require (
 	github.com/golang/protobuf v1.4.3
 	github.com/moby/sys/mountinfo v0.4.0
 	github.com/mrunalp/fileutils v0.5.0
-	github.com/opencontainers/runtime-spec v1.0.3-0.20210316141917-a8c4a9ee0f6b
+	github.com/opencontainers/runtime-spec v1.0.3-0.20210326190908-1c3f411f0417
 	github.com/opencontainers/selinux v1.8.0
 	github.com/pkg/errors v0.9.1
 	github.com/seccomp/libseccomp-golang v0.9.1
diff --git a/go.sum b/go.sum
index 07922aad..6875f919 100644
--- a/go.sum
+++ b/go.sum
@@ -34,8 +34,8 @@ github.com/moby/sys/mountinfo v0.4.0 h1:1KInV3Huv18akCu58V7lzNlt+jFmqlu1EaErnEHE
 github.com/moby/sys/mountinfo v0.4.0/go.mod h1:rEr8tzG/lsIZHBtN/JjGG+LMYx9eXgW2JI+6q0qou+A=
 github.com/mrunalp/fileutils v0.5.0 h1:NKzVxiH7eSk+OQ4M+ZYW1K6h27RUV3MI6NUTsHhU6Z4=
 github.com/mrunalp/fileutils v0.5.0/go.mod h1:M1WthSahJixYnrXQl/DFQuteStB1weuxD2QJNHXfbSQ=
-github.com/opencontainers/runtime-spec v1.0.3-0.20210316141917-a8c4a9ee0f6b h1:ZDY8P/luqXqGJSNCux8+9GeKmBDS+JVgVuIwKTauiwM=
-github.com/opencontainers/runtime-spec v1.0.3-0.20210316141917-a8c4a9ee0f6b/go.mod h1:jwyrGlmzljRJv/Fgzds9SsS/C5hL+LL3ko9hs6T5lQ0=
+github.com/opencontainers/runtime-spec v1.0.3-0.20210326190908-1c3f411f0417 h1:3snG66yBm59tKhhSPQrQ/0bCrv1LQbKt40LnUPiUxdc=
+github.com/opencontainers/runtime-spec v1.0.3-0.20210326190908-1c3f411f0417/go.mod h1:jwyrGlmzljRJv/Fgzds9SsS/C5hL+LL3ko9hs6T5lQ0=
 github.com/opencontainers/selinux v1.8.0 h1:+77ba4ar4jsCbL1GLbFL8fFM57w6suPfSS9PDLDY7KM=
 github.com/opencontainers/selinux v1.8.0/go.mod h1:RScLhm78qiWa2gbVCcGkC7tCGdgk3ogry1nUQF8Evvo=
 github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
diff --git a/libcontainer/capabilities/capabilities.go b/libcontainer/capabilities/capabilities.go
index dea33159..1099d32b 100644
--- a/libcontainer/capabilities/capabilities.go
+++ b/libcontainer/capabilities/capabilities.go
@@ -3,10 +3,11 @@
 package capabilities
 
 import (
-	"fmt"
+	"sort"
 	"strings"
 
 	"github.com/opencontainers/runc/libcontainer/configs"
+	"github.com/sirupsen/logrus"
 	"github.com/syndtr/gocapability/capability"
 )
 
@@ -34,27 +35,21 @@ func init() {
 }
 
 // New creates a new Caps from the given Capabilities config. Unknown Capabilities
-// or Capabilities that are unavailable in the current environment produce an error.
+// 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
-		c   = Caps{caps: make(map[capability.CapType][]capability.Cap, len(capTypes))}
+		c   Caps
 	)
 
-	if c.caps[capability.BOUNDING], err = capSlice(capConfig.Bounding); err != nil {
-		return nil, err
-	}
-	if c.caps[capability.EFFECTIVE], err = capSlice(capConfig.Effective); err != nil {
-		return nil, err
-	}
-	if c.caps[capability.INHERITABLE], err = capSlice(capConfig.Inheritable); err != nil {
-		return nil, err
-	}
-	if c.caps[capability.PERMITTED], err = capSlice(capConfig.Permitted); err != nil {
-		return nil, err
-	}
-	if c.caps[capability.AMBIENT], err = capSlice(capConfig.Ambient); err != nil {
-		return nil, err
+	unknownCaps := make(map[string]struct{})
+	c.caps = map[capability.CapType][]capability.Cap{
+		capability.BOUNDING:    capSlice(capConfig.Bounding, unknownCaps),
+		capability.EFFECTIVE:   capSlice(capConfig.Effective, unknownCaps),
+		capability.INHERITABLE: capSlice(capConfig.Inheritable, unknownCaps),
+		capability.PERMITTED:   capSlice(capConfig.Permitted, unknownCaps),
+		capability.AMBIENT:     capSlice(capConfig.Ambient, unknownCaps),
 	}
 	if c.pid, err = capability.NewPid2(0); err != nil {
 		return nil, err
@@ -62,22 +57,35 @@ func New(capConfig *configs.Capabilities) (*Caps, error) {
 	if err = c.pid.Load(); err != nil {
 		return nil, err
 	}
+	if len(unknownCaps) > 0 {
+		logrus.Warn("ignoring unknown or unavailable capabilities: ", mapKeys(unknownCaps))
+	}
 	return &c, nil
 }
 
 // capSlice converts the slice of capability names in caps, to their numeric
 // equivalent, and returns them as a slice. Unknown or unavailable capabilities
-// produce an error.
-func capSlice(caps []string) ([]capability.Cap, error) {
-	out := make([]capability.Cap, len(caps))
-	for i, c := range caps {
-		v, ok := capabilityMap[c]
-		if !ok {
-			return nil, fmt.Errorf("unknown capability %q", c)
+// are not returned, but appended to unknownCaps.
+func capSlice(caps []string, unknownCaps map[string]struct{}) []capability.Cap {
+	var out []capability.Cap
+	for _, c := range caps {
+		if v, ok := capabilityMap[c]; !ok {
+			unknownCaps[c] = struct{}{}
+		} else {
+			out = append(out, v)
 		}
-		out[i] = v
 	}
-	return out, nil
+	return out
+}
+
+// mapKeys returns the keys of input in sorted order
+func mapKeys(input map[string]struct{}) []string {
+	var keys []string
+	for c := range input {
+		keys = append(keys, c)
+	}
+	sort.Strings(keys)
+	return keys
 }
 
 // Caps holds the capabilities for a container.
diff --git a/vendor/github.com/opencontainers/runtime-spec/specs-go/config.go b/vendor/github.com/opencontainers/runtime-spec/specs-go/config.go
index df1f4321..6a7a91e5 100644
--- a/vendor/github.com/opencontainers/runtime-spec/specs-go/config.go
+++ b/vendor/github.com/opencontainers/runtime-spec/specs-go/config.go
@@ -650,6 +650,7 @@ const (
 	ActTrace       LinuxSeccompAction = "SCMP_ACT_TRACE"
 	ActAllow       LinuxSeccompAction = "SCMP_ACT_ALLOW"
 	ActLog         LinuxSeccompAction = "SCMP_ACT_LOG"
+	ActNotify      LinuxSeccompAction = "SCMP_ACT_NOTIFY"
 )
 
 // LinuxSeccompOperator used to match syscall arguments in Seccomp
diff --git a/vendor/modules.txt b/vendor/modules.txt
index d968afe3..4d06108f 100644
--- a/vendor/modules.txt
+++ b/vendor/modules.txt
@@ -36,7 +36,7 @@ github.com/moby/sys/mountinfo
 # github.com/mrunalp/fileutils v0.5.0
 ## explicit
 github.com/mrunalp/fileutils
-# github.com/opencontainers/runtime-spec v1.0.3-0.20210316141917-a8c4a9ee0f6b
+# github.com/opencontainers/runtime-spec v1.0.3-0.20210326190908-1c3f411f0417
 ## explicit
 github.com/opencontainers/runtime-spec/specs-go
 # github.com/opencontainers/selinux v1.8.0
@@ -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


Test patch:
diff --git a/libcontainer/capabilities/capabilities_linux_test.go b/libcontainer/capabilities/capabilities_linux_test.go
index 36b8d186..13a50e83 100644
--- a/libcontainer/capabilities/capabilities_linux_test.go
+++ b/libcontainer/capabilities/capabilities_linux_test.go
@@ -1,14 +1,18 @@
 package capabilities
 
 import (
+	"io/ioutil"
+	"os"
 	"testing"
 
 	"github.com/opencontainers/runc/libcontainer/configs"
+	"github.com/sirupsen/logrus"
+	"github.com/sirupsen/logrus/hooks/test"
 	"github.com/syndtr/gocapability/capability"
 )
 
 func TestNew(t *testing.T) {
-	cs := []string{"CAP_CHOWN"}
+	cs := []string{"CAP_CHOWN", "CAP_UNKNOWN", "CAP_UNKNOWN2"}
 	conf := configs.Capabilities{
 		Bounding:    cs,
 		Effective:   cs,
@@ -17,10 +21,36 @@ func TestNew(t *testing.T) {
 		Ambient:     cs,
 	}
 
+	hook := test.NewGlobal()
+	defer hook.Reset()
+
+	logrus.SetOutput(ioutil.Discard)
 	caps, 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))
+	}
+
+	expectedLogs := logrus.Entry{
+		Level:   logrus.WarnLevel,
+		Message: "ignoring unknown or unavailable capabilities: [CAP_UNKNOWN CAP_UNKNOWN2]",
+	}
+
+	l := hook.LastEntry()
+	if l == nil {
+		t.Fatal("expected a warning, but got none")
+	}
+	if l.Level != expectedLogs.Level {
+		t.Errorf("expected %q, got %q", expectedLogs.Level, l.Level)
+	}
+	if l.Message != expectedLogs.Message {
+		t.Errorf("expected %q, got %q", expectedLogs.Message, l.Message)
+	}
 
 	if len(caps.caps) != len(capTypes) {
 		t.Errorf("expected %d capability types, got %d: %v", len(capTypes), len(caps.caps), caps.caps)
@@ -36,4 +66,6 @@ func TestNew(t *testing.T) {
 			continue
 		}
 	}
+
+	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 00000000..b16d0665
--- /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)
+}