From c7dc8b1fedc67c65b12d51fd98a27b547c4f3b93 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 29 Aug 2022 15:35:35 -0700 Subject: [PATCH 1/2] libct/seccomp/patchbpf: support SPEC_ALLOW Commit 58ea21daefea8e3447db added support for seccomp flags such as SPEC_ALLOW, but it does not work as expected, because since commit 7a8d7162f9d72f20d83ea we do not use libseccomp-golang's Load(), but handle flags separately in patchbfp. This fixes setting SPEC_ALLOW flag. Add a comment to not forget to amend filterFlags when adding new flags. Signed-off-by: Kir Kolyshkin --- libcontainer/seccomp/patchbpf/enosys_linux.go | 14 ++++++++++++-- libcontainer/seccomp/seccomp_linux.go | 1 + 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/libcontainer/seccomp/patchbpf/enosys_linux.go b/libcontainer/seccomp/patchbpf/enosys_linux.go index 29302db8b66..ab97726d457 100644 --- a/libcontainer/seccomp/patchbpf/enosys_linux.go +++ b/libcontainer/seccomp/patchbpf/enosys_linux.go @@ -43,6 +43,11 @@ const uintptr_t C_SET_MODE_FILTER = SECCOMP_SET_MODE_FILTER; #endif const uintptr_t C_FILTER_FLAG_LOG = SECCOMP_FILTER_FLAG_LOG; +#ifndef SECCOMP_FILTER_FLAG_SPEC_ALLOW +# define SECCOMP_FILTER_FLAG_SPEC_ALLOW (1UL << 2) +#endif +const uintptr_t C_FILTER_FLAG_SPEC_ALLOW = SECCOMP_FILTER_FLAG_SPEC_ALLOW; + #ifndef SECCOMP_FILTER_FLAG_NEW_LISTENER # define SECCOMP_FILTER_FLAG_NEW_LISTENER (1UL << 3) #endif @@ -641,8 +646,13 @@ func filterFlags(config *configs.Seccomp, filter *libseccomp.ScmpFilter) (flags flags |= uint(C.C_FILTER_FLAG_LOG) } } - - // TODO: Support seccomp flags not yet added to libseccomp-golang... + if apiLevel >= 4 { + if ssb, err := filter.GetSSB(); err != nil { + return 0, false, fmt.Errorf("unable to fetch SECCOMP_FILTER_FLAG_SPEC_ALLOW bit: %w", err) + } else if ssb { + flags |= uint(C.C_FILTER_FLAG_SPEC_ALLOW) + } + } for _, call := range config.Syscalls { if call.Action == configs.Notify { diff --git a/libcontainer/seccomp/seccomp_linux.go b/libcontainer/seccomp/seccomp_linux.go index ffbe537e7e3..59549f5b489 100644 --- a/libcontainer/seccomp/seccomp_linux.go +++ b/libcontainer/seccomp/seccomp_linux.go @@ -105,6 +105,7 @@ func InitSeccomp(config *configs.Seccomp) (int, error) { 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) } From 26dc55ef1a56ea0279492a58c52636b549796510 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 30 Aug 2022 16:45:58 -0700 Subject: [PATCH 2/2] seccomp: fix flag test to actually check the value Add a debug print of seccomp flags value, so the test can check those (without using something like strace, that is). Amend the flags setting test with the numeric values expected, and the logic to check those. Signed-off-by: Kir Kolyshkin --- libcontainer/seccomp/patchbpf/enosys_linux.go | 3 + tests/integration/seccomp.bats | 60 ++++++++++++------- 2 files changed, 41 insertions(+), 22 deletions(-) diff --git a/libcontainer/seccomp/patchbpf/enosys_linux.go b/libcontainer/seccomp/patchbpf/enosys_linux.go index ab97726d457..b92eba8922e 100644 --- a/libcontainer/seccomp/patchbpf/enosys_linux.go +++ b/libcontainer/seccomp/patchbpf/enosys_linux.go @@ -665,6 +665,9 @@ func filterFlags(config *configs.Seccomp, filter *libseccomp.ScmpFilter) (flags } func sysSeccompSetFilter(flags uint, filter []unix.SockFilter) (fd int, err error) { + // This debug output is validated in tests/integration/seccomp.bats + // by the SECCOMP_FILTER_FLAG_* test. + logrus.Debugf("seccomp filter flags: %d", flags) fprog := unix.SockFprog{ Len: uint16(len(filter)), Filter: &filter[0], diff --git a/tests/integration/seccomp.bats b/tests/integration/seccomp.bats index 55e6dc817ab..f3e12bbce93 100644 --- a/tests/integration/seccomp.bats +++ b/tests/integration/seccomp.bats @@ -70,31 +70,47 @@ function teardown() { # Linux 4.14: SECCOMP_FILTER_FLAG_LOG # Linux 4.17: SECCOMP_FILTER_FLAG_SPEC_ALLOW requires_kernel 4.17 - SECCOMP_FILTER_FLAGS=( - '' # no flag - '"SECCOMP_FILTER_FLAG_LOG"' - '"SECCOMP_FILTER_FLAG_SPEC_ALLOW"' - '"SECCOMP_FILTER_FLAG_TSYNC"' - '"SECCOMP_FILTER_FLAG_LOG","SECCOMP_FILTER_FLAG_SPEC_ALLOW"' - '"SECCOMP_FILTER_FLAG_LOG","SECCOMP_FILTER_FLAG_TSYNC"' - '"SECCOMP_FILTER_FLAG_SPEC_ALLOW","SECCOMP_FILTER_FLAG_TSYNC"' - '"SECCOMP_FILTER_FLAG_LOG","SECCOMP_FILTER_FLAG_SPEC_ALLOW","SECCOMP_FILTER_FLAG_TSYNC"' + + update_config ' .process.args = ["/bin/sh", "-c", "mkdir /dev/shm/foo"] + | .process.noNewPrivileges = false + | .linux.seccomp = { + "defaultAction":"SCMP_ACT_ALLOW", + "architectures":["SCMP_ARCH_X86","SCMP_ARCH_X32","SCMP_ARCH_X86_64","SCMP_ARCH_AARCH64","SCMP_ARCH_ARM"], + "syscalls":[{"names":["mkdir"], "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 ) - for flags in "${SECCOMP_FILTER_FLAGS[@]}"; do - update_config ' .process.args = ["/bin/sh", "-c", "mkdir /dev/shm/foo"] - | .process.noNewPrivileges = false - | .linux.seccomp = { - "defaultAction":"SCMP_ACT_ALLOW", - "architectures":["SCMP_ARCH_X86","SCMP_ARCH_X32","SCMP_ARCH_X86_64","SCMP_ARCH_AARCH64","SCMP_ARCH_ARM"], - "flags":['"${flags}"'], - "syscalls":[{"names":["mkdir"], "action":"SCMP_ACT_ERRNO"}] - }' - - # This test checks that the flags are accepted without errors but does - # not check they are effectively applied - runc run test_busybox + for key in "${!FLAGS[@]}"; do + case "$key" in + 'REMOVE') + update_config ' del(.linux.seccomp.flags)' + ;; + 'EMPTY') + update_config ' .linux.seccomp.flags = []' + ;; + *) + update_config ' .linux.seccomp.flags = [ '"${key}"' ]' + ;; + esac + + runc --debug run test_busybox [ "$status" -ne 0 ] [[ "$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]}\"" + echo "flags $key, expecting $exp" + [[ "$output" == *"$exp"* ]] done }