Merge pull request #3588 from kolyshkin/seccomp-flags-rework

seccomp: refactor flags support; add flags to features, set SPEC_ALLOW by default

features.go

@@ -59,10 +59,12 @@ var featuresCommand = cli.Command{
 
 		if seccomp.Enabled {
 			feat.Linux.Seccomp = &features.Seccomp{
-				Enabled:   &tru,
-				Actions:   seccomp.KnownActions(),
-				Operators: seccomp.KnownOperators(),
-				Archs:     seccomp.KnownArchs(),
+				Enabled:        &tru,
+				Actions:        seccomp.KnownActions(),
+				Operators:      seccomp.KnownOperators(),
+				Archs:          seccomp.KnownArchs(),
+				KnownFlags:     seccomp.KnownFlags(),
+				SupportedFlags: seccomp.SupportedFlags(),
 			}
 			major, minor, patch := seccomp.Version()
 			feat.Annotations[features.AnnotationLibseccompVersion] = fmt.Sprintf("%d.%d.%d", major, minor, patch)

libcontainer/seccomp/config.go

@@ -5,8 +5,13 @@ import (
 	"sort"
 
 	"github.com/opencontainers/runc/libcontainer/configs"
+	"github.com/opencontainers/runtime-spec/specs-go"
 )
 
+// flagTsync is recognized but ignored by runc, and it is not defined
+// in the runtime-spec.
+const flagTsync = "SECCOMP_FILTER_FLAG_TSYNC"
+
 var operators = map[string]configs.Operator{
 	"SCMP_CMP_NE":        configs.NotEqualTo,
 	"SCMP_CMP_LT":        configs.LessThan,
@@ -111,3 +116,35 @@ func ConvertStringToArch(in string) (string, error) {
 	}
 	return "", fmt.Errorf("string %s is not a valid arch for seccomp", in)
 }
+
+// List of flags known to this version of runc.
+var flags = []string{
+	flagTsync,
+	string(specs.LinuxSeccompFlagSpecAllow),
+	string(specs.LinuxSeccompFlagLog),
+}
+
+// KnownFlags returns the list of the known filter flags.
+// Used by `runc features`.
+func KnownFlags() []string {
+	return flags
+}
+
+// SupportedFlags returns the list of the supported filter flags.
+// This list may be a subset of one returned by KnownFlags due to
+// some flags not supported by the current kernel and/or libseccomp.
+// Used by `runc features`.
+func SupportedFlags() []string {
+	if !Enabled {
+		return nil
+	}
+
+	var res []string
+	for _, flag := range flags {
+		if FlagSupported(specs.LinuxSeccompFlag(flag)) == nil {
+			res = append(res, flag)
+		}
+	}
+
+	return res
+}

libcontainer/seccomp/patchbpf/enosys_linux.go

@@ -643,6 +643,7 @@ func filterFlags(config *configs.Seccomp, filter *libseccomp.ScmpFilter) (flags
 			flags |= uint(C.C_FILTER_FLAG_SPEC_ALLOW)
 		}
 	}
+	// XXX: add newly supported filter flags above this line.
 
 	for _, call := range config.Syscalls {
 		if call.Action == configs.Notify {

libcontainer/seccomp/seccomp_linux.go

@@ -87,27 +87,10 @@ func InitSeccomp(config *configs.Seccomp) (int, error) {
 		}
 	}
 
-	// Add extra flags
+	// Add extra flags.
 	for _, flag := range config.Flags {
-		switch flag {
-		case "SECCOMP_FILTER_FLAG_TSYNC":
-			// libseccomp-golang always use filterAttrTsync when
-			// possible so all goroutines will receive the same
-			// rules, so there is nothing to do. It does not make
-			// sense to apply the seccomp filter on only one
-			// thread; other threads will be terminated after exec
-			// anyway.
-		case specs.LinuxSeccompFlagLog:
-			if err := filter.SetLogBit(true); err != nil {
-				return -1, fmt.Errorf("error adding log flag to seccomp filter: %w", err)
-			}
-		case specs.LinuxSeccompFlagSpecAllow:
-			if err := filter.SetSSB(true); err != nil {
-				return -1, fmt.Errorf("error adding SSB flag to seccomp filter: %w", err)
-			}
-		// NOTE when adding more flags, make sure to also modify filterFlags in patchbpf.
-		default:
-			return -1, fmt.Errorf("seccomp flags %q not yet supported by runc", flag)
+		if err := setFlag(filter, flag); err != nil {
+			return -1, err
 		}
 	}
 
@@ -149,6 +132,67 @@ func InitSeccomp(config *configs.Seccomp) (int, error) {
 	return seccompFd, nil
 }
 
+type unknownFlagError struct {
+	flag specs.LinuxSeccompFlag
+}
+
+func (e *unknownFlagError) Error() string {
+	return "seccomp flag " + string(e.flag) + " is not known to runc"
+}
+
+func setFlag(filter *libseccomp.ScmpFilter, flag specs.LinuxSeccompFlag) error {
+	switch flag {
+	case flagTsync:
+		// libseccomp-golang always use filterAttrTsync when
+		// possible so all goroutines will receive the same
+		// rules, so there is nothing to do. It does not make
+		// sense to apply the seccomp filter on only one
+		// thread; other threads will be terminated after exec
+		// anyway.
+		return nil
+	case specs.LinuxSeccompFlagLog:
+		if err := filter.SetLogBit(true); err != nil {
+			return fmt.Errorf("error adding log flag to seccomp filter: %w", err)
+		}
+		return nil
+	case specs.LinuxSeccompFlagSpecAllow:
+		if err := filter.SetSSB(true); err != nil {
+			return fmt.Errorf("error adding SSB flag to seccomp filter: %w", err)
+		}
+		return nil
+	}
+	// NOTE when adding more flags above, do not forget to also:
+	// - add new flags to `flags` slice in config.go;
+	// - add new flag values to flags_value() in tests/integration/seccomp.bats;
+	// - modify func filterFlags in patchbpf/ accordingly.
+
+	return &unknownFlagError{flag: flag}
+}
+
+// FlagSupported checks if the flag is known to runc and supported by
+// currently used libseccomp and kernel (i.e. it can be set).
+func FlagSupported(flag specs.LinuxSeccompFlag) error {
+	filter := &libseccomp.ScmpFilter{}
+	err := setFlag(filter, flag)
+
+	// For flags we don't know, setFlag returns unknownFlagError.
+	var uf *unknownFlagError
+	if errors.As(err, &uf) {
+		return err
+	}
+	// For flags that are known to runc and libseccomp-golang but can not
+	// be applied because either libseccomp or the kernel is too old,
+	// seccomp.VersionError is returned.
+	var verErr *libseccomp.VersionError
+	if errors.As(err, &verErr) {
+		// Not supported by libseccomp or the kernel.
+		return err
+	}
+
+	// All other flags are known and supported.
+	return nil
+}
+
 // Convert Libcontainer Action to Libseccomp ScmpAction
 func getAction(act configs.Action, errnoRet *uint) (libseccomp.ScmpAction, error) {
 	switch act {

libcontainer/seccomp/seccomp_unsupported.go

@@ -7,6 +7,7 @@ import (
 	"errors"
 
 	"github.com/opencontainers/runc/libcontainer/configs"
+	"github.com/opencontainers/runtime-spec/specs-go"
 )
 
 var ErrSeccompNotEnabled = errors.New("seccomp: config provided but seccomp not supported")
@@ -19,6 +20,11 @@ func InitSeccomp(config *configs.Seccomp) (int, error) {
 	return -1, nil
 }
 
+// FlagSupported tells if a provided seccomp flag is supported.
+func FlagSupported(_ specs.LinuxSeccompFlag) error {
+	return ErrSeccompNotEnabled
+}
+
 // Version returns major, minor, and micro.
 func Version() (uint, uint, uint) {
 	return 0, 0, 0

libcontainer/specconv/spec_linux.go

@@ -1024,15 +1024,21 @@ func SetupSeccomp(config *specs.LinuxSeccomp) (*configs.Seccomp, error) {
 	newConfig.Syscalls = []*configs.Syscall{}
 
 	// The list of flags defined in runtime-spec is a subset of the flags
-	// in the seccomp() syscall
-	for _, flag := range config.Flags {
-		switch flag {
-		case "SECCOMP_FILTER_FLAG_TSYNC":
-			// Tsync can be silently ignored
-		case specs.LinuxSeccompFlagLog, specs.LinuxSeccompFlagSpecAllow:
+	// in the seccomp() syscall.
+	if config.Flags == nil {
+		// No flags are set explicitly (not even the empty set);
+		// set the default of specs.LinuxSeccompFlagSpecAllow,
+		// if it is supported by the libseccomp and the kernel.
+		if err := seccomp.FlagSupported(specs.LinuxSeccompFlagSpecAllow); err == nil {
+			newConfig.Flags = []specs.LinuxSeccompFlag{specs.LinuxSeccompFlagSpecAllow}
+		}
+	} else {
+		// Fail early if some flags are unknown or unsupported.
+		for _, flag := range config.Flags {
+			if err := seccomp.FlagSupported(flag); err != nil {
+				return nil, err
+			}
 			newConfig.Flags = append(newConfig.Flags, flag)
-		default:
-			return nil, fmt.Errorf("seccomp flag %q not yet supported by runc", flag)
 		}
 	}
 

tests/integration/seccomp.bats

@@ -66,11 +66,32 @@ function teardown() {
 	[[ "$output" == *"Network is down"* ]]
 }
 
-@test "runc run [seccomp] (SECCOMP_FILTER_FLAG_*)" {
-	# Linux 4.14: SECCOMP_FILTER_FLAG_LOG
-	# Linux 4.17: SECCOMP_FILTER_FLAG_SPEC_ALLOW
-	requires_kernel 4.17
+# Prints the numeric value of provided seccomp flags combination.
+# The parameter is flags string, as supplied in OCI spec, for example
+# '"SECCOMP_FILTER_FLAG_TSYNC","SECCOMP_FILTER_FLAG_LOG"'.
+function flags_value() {
+	# Numeric values of seccomp flags.
+	declare -A values=(
+		['"SECCOMP_FILTER_FLAG_TSYNC"']=0 # Supported but ignored by runc, thus 0.
+		['"SECCOMP_FILTER_FLAG_LOG"']=2
+		['"SECCOMP_FILTER_FLAG_SPEC_ALLOW"']=4
+		# XXX: add new values above this line.
+	)
+	# Split the flags.
+	IFS=',' read -ra flags <<<"$1"
 
+	local flag v sum=0
+	for flag in "${flags[@]}"; do
+		# This will produce "values[$flag]: unbound variable"
+		# error for a new flag yet unknown to the test.
+		v=${values[$flag]}
+		((sum += v)) || true
+	done
+
+	echo $sum
+}
+
+@test "runc run [seccomp] (SECCOMP_FILTER_FLAG_*)" {
 	update_config '   .process.args = ["/bin/sh", "-c", "mkdir /dev/shm/foo"]
 			| .process.noNewPrivileges = false
 			| .linux.seccomp = {
@@ -79,18 +100,35 @@ function teardown() {
 				"syscalls":[{"names":["mkdir", "mkdirat"], "action":"SCMP_ACT_ERRNO"}]
 			}'
 
-	declare -A FLAGS=(
-		['REMOVE']=0 # No setting, use built-in default.
-		['EMPTY']=0  # Empty set of flags.
-		['"SECCOMP_FILTER_FLAG_LOG"']=2
-		['"SECCOMP_FILTER_FLAG_SPEC_ALLOW"']=4
-		['"SECCOMP_FILTER_FLAG_TSYNC"']=0 # tsync flag is ignored.
-		['"SECCOMP_FILTER_FLAG_LOG","SECCOMP_FILTER_FLAG_SPEC_ALLOW"']=6
-		['"SECCOMP_FILTER_FLAG_LOG","SECCOMP_FILTER_FLAG_TSYNC"']=2
-		['"SECCOMP_FILTER_FLAG_SPEC_ALLOW","SECCOMP_FILTER_FLAG_TSYNC"']=4
-		['"SECCOMP_FILTER_FLAG_LOG","SECCOMP_FILTER_FLAG_SPEC_ALLOW","SECCOMP_FILTER_FLAG_TSYNC"']=6
+	# Get the list of flags supported by runc/seccomp/kernel,
+	# or "null" if no flags are supported or runc is too old.
+	mapfile -t flags < <(__runc features | jq -c '.linux.seccomp.supportedFlags' |
+		tr -d '[]\n' | tr ',' '\n')
+
+	# This is a set of all possible flag combinations to test.
+	declare -A TEST_CASES=(
+		['EMPTY']=0  # Special value: empty set of flags.
+		['REMOVE']=0 # Special value: no flags set.
 	)
-	for key in "${!FLAGS[@]}"; do
+
+	# If supported, runc should set SPEC_ALLOW if no flags are set.
+	if [[ " ${flags[*]} " == *' "SECCOMP_FILTER_FLAG_SPEC_ALLOW" '* ]]; then
+		TEST_CASES['REMOVE']=$(flags_value '"SECCOMP_FILTER_FLAG_SPEC_ALLOW"')
+	fi
+
+	# Add all possible combinations of seccomp flags
+	# and their expected numeric values to TEST_CASES.
+	if [ "${flags[0]}" != "null" ]; then
+		# Use shell {a,}{b,}{c,} to generate the powerset.
+		for fc in $(eval echo "$(printf "{'%s,',}" "${flags[@]}")"); do
+			# Remove the last comma.
+			fc="${fc/%,/}"
+			TEST_CASES[$fc]=$(flags_value "$fc")
+		done
+	fi
+
+	# Finally, run the tests.
+	for key in "${!TEST_CASES[@]}"; do
 		case "$key" in
 		'REMOVE')
 			update_config ' del(.linux.seccomp.flags)'
@@ -108,7 +146,7 @@ function teardown() {
 		[[ "$output" == *"mkdir:"*"/dev/shm/foo"*"Operation not permitted"* ]]
 
 		# Check the numeric flags value, as printed in the debug log, is as expected.
-		exp="\"seccomp filter flags: ${FLAGS[$key]}\""
+		exp="\"seccomp filter flags: ${TEST_CASES[$key]}\""
 		echo "flags $key, expecting $exp"
 		[[ "$output" == *"$exp"* ]]
 	done

types/features/features.go

@@ -53,13 +53,23 @@ type Seccomp struct {
 	// Nil value means "unknown", not "no support for any action".
 	Actions []string `json:"actions,omitempty"`
 
-	// Operators is the list of the recognized actions, e.g., "SCMP_CMP_NE".
+	// Operators is the list of the recognized operators, e.g., "SCMP_CMP_NE".
 	// Nil value means "unknown", not "no support for any operator".
 	Operators []string `json:"operators,omitempty"`
 
-	// Operators is the list of the recognized archs, e.g., "SCMP_ARCH_X86_64".
+	// Archs is the list of the recognized archs, e.g., "SCMP_ARCH_X86_64".
 	// Nil value means "unknown", not "no support for any arch".
 	Archs []string `json:"archs,omitempty"`
+
+	// KnownFlags is the list of the recognized filter flags, e.g., "SECCOMP_FILTER_FLAG_LOG".
+	// Nil value means "unknown", not "no flags are recognized".
+	KnownFlags []string `json:"knownFlags,omitempty"`
+
+	// SupportedFlags is the list of the supported filter flags, e.g., "SECCOMP_FILTER_FLAG_LOG".
+	// This list may be a subset of KnownFlags due to some flags
+	// not supported by the current kernel and/or libseccomp.
+	// Nil value means "unknown", not "no flags are supported".
+	SupportedFlags []string `json:"supportedFlags,omitempty"`
 }
 
 // Apparmor represents the "apparmor" field.