From 073e085ca42e5cdfd7c7080df0b88f5ff5ed51f9 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 22 Jul 2021 12:00:28 -0700 Subject: [PATCH 1/3] libct/seccomp: ConvertStringToAction: fix doc As of commit caca840972 (Nov 12 2015) SCMP_ACT_TRACE is supported. Signed-off-by: Kir Kolyshkin --- libcontainer/seccomp/config.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/libcontainer/seccomp/config.go b/libcontainer/seccomp/config.go index b54b7eead3b..2e5adfd3a52 100644 --- a/libcontainer/seccomp/config.go +++ b/libcontainer/seccomp/config.go @@ -56,9 +56,7 @@ func ConvertStringToOperator(in string) (configs.Operator, error) { } // ConvertStringToAction converts a string into a Seccomp rule match action. -// Actions use the names they are assigned in Libseccomp's header, though some -// (notable, SCMP_ACT_TRACE) are not available in this implementation and will -// return errors. +// Actions use the names they are assigned in Libseccomp's header. // Attempting to convert a string that is not a valid action results in an // error. func ConvertStringToAction(in string) (configs.Action, error) { From e44bee1026f83beec3f0aad64566fa9e6bfe6a04 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 23 Jul 2021 12:30:14 -0700 Subject: [PATCH 2/3] libct/seccomp: warn about unknown syscalls Rather than silently ignoring unknown syscalls, print a warning. While at it, fix imports ordering (stdlib, others, ours). [v2: demote Warn to Debug] Signed-off-by: Kir Kolyshkin --- libcontainer/seccomp/seccomp_linux.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/libcontainer/seccomp/seccomp_linux.go b/libcontainer/seccomp/seccomp_linux.go index fbbe8219782..0265062a28e 100644 --- a/libcontainer/seccomp/seccomp_linux.go +++ b/libcontainer/seccomp/seccomp_linux.go @@ -6,11 +6,12 @@ import ( "errors" "fmt" - "github.com/opencontainers/runc/libcontainer/configs" - "github.com/opencontainers/runc/libcontainer/seccomp/patchbpf" - libseccomp "github.com/seccomp/libseccomp-golang" + "github.com/sirupsen/logrus" "golang.org/x/sys/unix" + + "github.com/opencontainers/runc/libcontainer/configs" + "github.com/opencontainers/runc/libcontainer/seccomp/patchbpf" ) var ( @@ -151,10 +152,11 @@ func matchCall(filter *libseccomp.ScmpFilter, call *configs.Syscall) error { return errors.New("empty string is not a valid syscall") } - // If we can't resolve the syscall, assume it's not supported on this kernel - // Ignore it, don't error out + // If we can't resolve the syscall, assume it is not supported + // by this kernel. Warn about it, don't error out. callNum, err := libseccomp.GetSyscallFromName(call.Name) if err != nil { + logrus.Debugf("unknown seccomp syscall %q ignored", call.Name) return nil } From 5dd92fd9b454e35ee543ced670f84344de67c902 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 22 Jul 2021 14:31:46 -0700 Subject: [PATCH 3/3] libct/seccomp: skip redundant rules This fixes using runc with podman on my system (Fedora 34). > $ podman --runtime `pwd`/runc run --rm --memory 4M fedora echo it works > Error: unable to start container process: error adding seccomp filter rule for syscall bdflush: permission denied: OCI permission denied The problem is, libseccomp returns EPERM when a redundant rule (i.e. the rule with the same action as the default one) is added, and podman (on my machine) sets the following rules in config.json: <....> "seccomp": { "defaultAction": "SCMP_ACT_ERRNO", "architectures": [ "SCMP_ARCH_X86_64", "SCMP_ARCH_X86", "SCMP_ARCH_X32" ], "syscalls": [ { "names": [ "bdflush", "io_pgetevents", <....> ], "action": "SCMP_ACT_ERRNO", "errnoRet": 1 }, <....> (Note that defaultErrnoRet is not set, but it defaults to 1). With this commit, it works: > $ podman --runtime `pwd`/runc run --memory 4M fedora echo it works > it works Add an integration test (that fails without the fix). Similar crun commit: * https://github.com/containers/crun/commit/08229f3fb904c5ea19a7d9 Signed-off-by: Kir Kolyshkin --- libcontainer/seccomp/seccomp_linux.go | 21 +++++++++++++-------- tests/integration/start_hello.bats | 12 ++++++++++++ 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/libcontainer/seccomp/seccomp_linux.go b/libcontainer/seccomp/seccomp_linux.go index 0265062a28e..58cad90262b 100644 --- a/libcontainer/seccomp/seccomp_linux.go +++ b/libcontainer/seccomp/seccomp_linux.go @@ -68,7 +68,7 @@ func InitSeccomp(config *configs.Seccomp) error { if call == nil { return errors.New("encountered nil syscall while initializing Seccomp") } - if err := matchCall(filter, call); err != nil { + if err := matchCall(filter, call, defaultAction); err != nil { return err } } @@ -143,7 +143,7 @@ func getCondition(arg *configs.Arg) (libseccomp.ScmpCondition, error) { } // Add a rule to match a single syscall -func matchCall(filter *libseccomp.ScmpFilter, call *configs.Syscall) error { +func matchCall(filter *libseccomp.ScmpFilter, call *configs.Syscall, defAct libseccomp.ScmpAction) error { if call == nil || filter == nil { return errors.New("cannot use nil as syscall to block") } @@ -152,6 +152,17 @@ func matchCall(filter *libseccomp.ScmpFilter, call *configs.Syscall) error { return errors.New("empty string is not a valid syscall") } + // Convert the call's action to the libseccomp equivalent + callAct, err := getAction(call.Action, call.ErrnoRet) + if err != nil { + return fmt.Errorf("action in seccomp profile is invalid: %w", err) + } + if callAct == defAct { + // This rule is redundant, silently skip it + // to avoid error from AddRule. + return nil + } + // If we can't resolve the syscall, assume it is not supported // by this kernel. Warn about it, don't error out. callNum, err := libseccomp.GetSyscallFromName(call.Name) @@ -160,12 +171,6 @@ func matchCall(filter *libseccomp.ScmpFilter, call *configs.Syscall) error { return nil } - // Convert the call's action to the libseccomp equivalent - callAct, err := getAction(call.Action, call.ErrnoRet) - if err != nil { - return fmt.Errorf("action in seccomp profile is invalid: %w", err) - } - // Unconditional match - just add the rule if len(call.Args) == 0 { if err := filter.AddRule(callNum, callAct); err != nil { diff --git a/tests/integration/start_hello.bats b/tests/integration/start_hello.bats index 858847032c3..1c3c906fa53 100644 --- a/tests/integration/start_hello.bats +++ b/tests/integration/start_hello.bats @@ -76,3 +76,15 @@ function teardown() { runc run test_hello [ "$status" -eq 0 ] } + +@test "runc run [redundant seccomp rules]" { + update_config ' .linux.seccomp = { + "defaultAction": "SCMP_ACT_ALLOW", + "syscalls": [{ + "names": ["bdflush"], + "action": "SCMP_ACT_ALLOW", + }] + }' + runc run test_hello + [ "$status" -eq 0 ] +}