Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

seccomp: do not ignore SPEC_ALLOW flag #3580

Merged
merged 2 commits into from
Sep 13, 2022
Merged

seccomp: do not ignore SPEC_ALLOW flag #3580

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c7dc8b1
libct/seccomp/patchbpf: support SPEC_ALLOW
kolyshkin Aug 29, 2022
26dc55e
seccomp: fix flag test to actually check the value
kolyshkin Aug 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions libcontainer/seccomp/patchbpf/enosys_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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...
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this the only flag not supported by libseccomp-golang? If not, is this comment still relevant?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, libseccomp-golang is on par with the runtime-spec, thus I removed this comment.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently correct, but I'd like support for SECCOMP_FILTER_FLAG_WAIT_KILLABLE_RECV (Linux 5.19, torvalds/linux@c2aa2dfef243).

opencontainers/runtime-spec#1161

Not yet in libseccomp: seccomp/libseccomp#387

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but I guess we'll support it in libseccomp and libseccomp-golang first, when here in runc.

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 {
Expand All @@ -655,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)
kolyshkin marked this conversation as resolved.
Show resolved Hide resolved
fprog := unix.SockFprog{
Len: uint16(len(filter)),
Filter: &filter[0],
Expand Down
1 change: 1 addition & 0 deletions libcontainer/seccomp/seccomp_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
60 changes: 38 additions & 22 deletions tests/integration/seccomp.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down