[PATCH v4 4/4] tests: test cases for seccomp-assisted syscall filtering
Dmitry V. Levin
ldv at altlinux.org
Sat Aug 31 16:26:39 UTC 2019
On Thu, Aug 29, 2019 at 04:01:13PM +0200, Paul Chaignon wrote:
> From: Chen Jingpiao <chenjingpiao at gmail.com>
>
> Test filter_seccomp-perf checks seccomp-filter is actually enabled by
> comparing the number of syscalls performed in a time interval when
> seccomp-filter is enabled vs. disabled. The number of syscalls should be
> one order of magnitude higher when seccomp-filter is enabled.
>
> Test filter_seccomp-flag ensures the audit_arch_vec[].flag constants do
> not conflict with syscall numbers. If this test fails, then the number of
> syscalls grew high enough that the code for seccomp-filter needs to be
> updated.
>
> * tests/status-none-f.c: New file.
> * tests/pure_executables.list: Add status-none-f.
> * tests/.gitignore: Add status-none-f, filter_seccomp-perf, and
> filter_seccomp-flag.
> * tests/init.sh (test_prog_set): New function.
> * tests/filter_seccomp.in: New file.
> * tests/filter_seccomp-perf.c: New file.
> * tests/filter_seccomp-perf.test: New file.
> * tests/filter_seccomp-flag.c: New file.
> * tests/filter_seccomp-flag.test: New file.
> * tests/Makefile.am (EXTRA_DIST): Add filter_seccomp.in.
> (MISC_TESTS): Add filter_seccomp-perf.test and filter_seccomp-flag.test.
> (check_PROGRAMS): Add filter_seccomp-perf and filter_seccomp-flag.
> * tests/gen_tests.in: Add threads-execve test with -n and filter_seccomp
> test_prog_set.
>
> Co-authored-by: Paul Chaignon <paul.chaignon at gmail.com>
> ---
> tests/.gitignore | 3 ++
> tests/Makefile.am | 5 +++
> tests/filter_seccomp-flag.c | 80 ++++++++++++++++++++++++++++++++++
> tests/filter_seccomp-flag.test | 10 +++++
> tests/filter_seccomp-perf.c | 38 ++++++++++++++++
> tests/filter_seccomp-perf.test | 25 +++++++++++
> tests/filter_seccomp.in | 4 ++
> tests/gen_tests.in | 2 +
> tests/init.sh | 5 +++
> tests/pure_executables.list | 1 +
> tests/status-none-f.c | 19 ++++++++
> 11 files changed, 192 insertions(+)
> create mode 100644 tests/filter_seccomp-flag.c
> create mode 100755 tests/filter_seccomp-flag.test
> create mode 100644 tests/filter_seccomp-perf.c
> create mode 100755 tests/filter_seccomp-perf.test
> create mode 100644 tests/filter_seccomp.in
> create mode 100644 tests/status-none-f.c
>
> diff --git a/tests/.gitignore b/tests/.gitignore
> index 4c854db8..83e525bb 100644
> --- a/tests/.gitignore
> +++ b/tests/.gitignore
> @@ -85,6 +85,7 @@ fdatasync
> fflush
> file_handle
> file_ioctl
> +filter_seccomp-perf
> filter-unavailable
> finit_module
> flock
> @@ -509,6 +510,7 @@ sched_yield
> scm_rights
> scno.h
> seccomp-filter
> +filter_seccomp-flag
> seccomp-filter-v
> seccomp-strict
> seccomp_get_action_avail
> @@ -588,6 +590,7 @@ statfs64
> status-all
> status-failed
> status-none
> +status-none-f
> status-none-threads
> status-successful
> status-unfinished
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 156e359a..7f00a6a0 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -90,6 +90,8 @@ check_PROGRAMS = $(PURE_EXECUTABLES) \
> delay \
> execve-v \
> execveat-v \
> + filter_seccomp-flag \
> + filter_seccomp-perf \
> filter-unavailable \
> fork-f \
> fsync-y \
> @@ -313,6 +315,8 @@ MISC_TESTS = \
> detach-sleeping.test \
> detach-stopped.test \
> fflush.test \
> + filter_seccomp-flag.test \
> + filter_seccomp-perf.test \
> filter-unavailable.test \
> filtering_fd-syntax.test \
> filtering_syscall-syntax.test \
> @@ -395,6 +399,7 @@ EXTRA_DIST = \
> eventfd.expected \
> fadvise.h \
> fcntl-common.c \
> + filter_seccomp.in \
> filter-unavailable.expected \
> fstatat.c \
> fstatx.c \
> diff --git a/tests/filter_seccomp-flag.c b/tests/filter_seccomp-flag.c
> new file mode 100644
> index 00000000..1b8a51f9
> --- /dev/null
> +++ b/tests/filter_seccomp-flag.c
> @@ -0,0 +1,80 @@
> +/*
> + * Check syscall numbers do not conflict with seccomp-filter flags.
> + *
> + * Copyright (c) 2019 Paul Chaignon <paul.chaignon at gmail.com>
> + * All rights reserved.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include <linux/audit.h>
> +#include <asm/unistd.h>
> +#include "defs.h"
> +
> +
> +/* Define these shorthand notations to simplify the syscallent files. */
> +#include "sysent_shorthand_defs.h"
> +
> +#define SEN(syscall_name) 0, 0
> +
> +const struct_sysent sysent0[] = {
> +#include "syscallent.h"
> +};
> +
> +#if SUPPORTED_PERSONALITIES > 1
> +# include PERSONALITY1_INCLUDE_FUNCS
> +static const struct_sysent sysent1[] = {
> +# include "syscallent1.h"
> +};
> +#endif
> +
> +#if SUPPORTED_PERSONALITIES > 2
> +# include PERSONALITY2_INCLUDE_FUNCS
> +static const struct_sysent sysent2[] = {
> +# include "syscallent2.h"
> +};
> +#endif
> +
> +const unsigned int nsyscall_vec[SUPPORTED_PERSONALITIES] = {
> + ARRAY_SIZE(sysent0),
> +#if SUPPORTED_PERSONALITIES > 1
> + ARRAY_SIZE(sysent1),
> +#endif
> +#if SUPPORTED_PERSONALITIES > 2
> + ARRAY_SIZE(sysent2),
> +#endif
> +};
> +
> +struct audit_arch_t {
> + unsigned int arch;
> + unsigned int flag;
> +};
> +
> +static const struct audit_arch_t audit_arch_vec[SUPPORTED_PERSONALITIES] = {
> +#if SUPPORTED_PERSONALITIES > 1
> + PERSONALITY0_AUDIT_ARCH,
> + PERSONALITY1_AUDIT_ARCH,
> +# if SUPPORTED_PERSONALITIES > 2
> + PERSONALITY2_AUDIT_ARCH,
> +# endif
> +#endif
> +};
> +
> +int
> +main(void)
> +{
> + for (unsigned int p = 0; p < SUPPORTED_PERSONALITIES; ++p) {
> + if (!audit_arch_vec[p].flag)
> + continue;
> + for (unsigned int nr = 1; nr < nsyscall_vec[p]; ++nr) {
> + if (audit_arch_vec[p].flag & nr) {
> + fprintf(stderr, "error: system call number %u "
> + "of personality %u conflicts with "
> + "seccomp-filter flag 0x%x\n", nr, p,
> + audit_arch_vec[p].flag);
> + return 1;
We prefer automatic %#x to manual 0x%x.
I wonder why don't you use error_msg_and_fail() instead of fprintf+return.
> + }
> + }
> + }
> + return 0;
> +}
> diff --git a/tests/filter_seccomp-flag.test b/tests/filter_seccomp-flag.test
> new file mode 100755
> index 00000000..4bb963c5
> --- /dev/null
> +++ b/tests/filter_seccomp-flag.test
> @@ -0,0 +1,10 @@
> +#!/bin/sh
> +#
> +# Check syscall numbers do not conflict with seccomp-filter flags.
> +#
> +# Copyright (c) 2019 Paul Chaignon <paul.chaignon at gmail.com>
> +# All rights reserved.
> +#
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +./filter_seccomp-flag
I think this file can be replaced with a single line in tests/gen_tests.in
file:
filter_seccomp-flag ../$NAME
> diff --git a/tests/filter_seccomp-perf.c b/tests/filter_seccomp-perf.c
> new file mode 100644
> index 00000000..6ea0cab0
> --- /dev/null
> +++ b/tests/filter_seccomp-perf.c
> @@ -0,0 +1,38 @@
> +/*
> + * Check seccomp-filter is enabled.
This is not a check whether seccomp filter is enabled,
this is a check for seccomp filter performance.
> + *
> + * Copyright (c) 2019 Paul Chaignon <paul.chaignon at gmail.com>
> + * All rights reserved.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "tests.h"
> +#include <signal.h>
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +
> +static volatile bool stop = false;
> +
> +static void
> +handler(int signo)
> +{
> + stop = true;
Strange tab stop here.
> +}
> +
> +int
> +main(void)
> +{
> + unsigned int i;
> + int rc = 0;
> +
> + signal(SIGALRM, handler);
> + alarm(1);
> +
> + for (i = 0; !stop; i++) {
> + rc |= chdir(".");
> + }
> + printf("%d\n", i);
> + return rc;
> +}
> diff --git a/tests/filter_seccomp-perf.test b/tests/filter_seccomp-perf.test
> new file mode 100755
> index 00000000..b73c73d6
> --- /dev/null
> +++ b/tests/filter_seccomp-perf.test
> @@ -0,0 +1,25 @@
> +#!/bin/sh
> +#
> +# Check seccomp-filter is enabled.
Likewise, this is not a check whether seccomp filter is enabled,
this is a check for seccomp filter performance.
> +#
> +# Copyright (c) 2019 Paul Chaignon <paul.chaignon at gmail.com>
> +# All rights reserved.
> +#
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +. "${srcdir=.}/init.sh"
> +
> +$STRACE -qfn -efchdir / > /dev/null 2> "$OUT" ||:
> +if grep "seccomp-filter is requested but unavailable" "$OUT" > /dev/null; then
> + skip_ "seccomp-filter is unavailable"
> +fi
I suggest moving this check into a separate filter_seccomp.sh file
because it's going to be used elsewhere:
#!/bin/sh
#
# Skip the test if seccomp filter is not available.
#
# Copyright (c) 2019 The strace developers.
# All rights reserved.
#
# SPDX-License-Identifier: GPL-2.0-or-later
$STRACE -n -f -e trace=fchdir / > /dev/null 2> "$LOG" ||:
if grep -x "[^:]*strace: seccomp-filter is requested but unavailable" \
"$LOG" > /dev/null; then
skip_ 'seccomp filter is unavailable'
fi
> +nb_no_seccomp=$($STRACE -qf -e signal=none -efchdir ../$NAME)
> +nb_seccomp=$($STRACE -qfn -e signal=none -efchdir ../$NAME 2> "$OUT")
Why redirecting stderr to $OUT?
What does nb_ prefix mean? If it's number, let's use num_ prefix instead:
num_regular="$(run_strace -f -qq -e signal=none -e trace=fchdir ../$NAME)"
mv "$LOG" "$LOG.regular"
num_seccomp="$(run_strace -n -f -qq -e signal=none -e trace=fchdir ../$NAME)"
mv "$LOG" "$LOG.seccomp"
match_diff "$LOG.regular" "$LOG.seccomp"
> +
> +# With seccomp-filter enabled, we should be able to complete at least 10 times
I've managed to find a quite slow system where this ratio is 8.
> +# more calls to chdir.
> +if [ "$nb_seccomp" -lt "$((10*nb_no_seccomp))" ]; then
> + ratio=$((nb_seccomp / nb_no_seccomp))
> + fail_ "Only $ratio more syscalls performed with seccomp-filter enabled"
$ratio times more
> +fi
I suggest to rewrite it this way:
min_ratio=8
# With seccomp-filter enabled, we should be able to complete
# at least $min_ratio times more chdir system calls.
ratio="$((num_seccomp / num_regular))"
if [ "$ratio" -lt "$min_ratio" ]; then
fail_ "Only $ratio times more syscalls performed with seccomp-filter enabled"
fi
> diff --git a/tests/filter_seccomp.in b/tests/filter_seccomp.in
> new file mode 100644
> index 00000000..dc741911
> --- /dev/null
> +++ b/tests/filter_seccomp.in
> @@ -0,0 +1,4 @@
> +fork-f -a26 -qq -f -e signal=none -e trace=chdir
> +vfork-f -a26 -qq -f -e signal=none -e trace=chdir
> +fork-f -a26 -qq -f -e signal=none -e trace=chdir,%memory,%ipc,%pure,%signal,%network -e status=failed
> +status-none-f -f -e trace=!ptrace -e status=none
> diff --git a/tests/gen_tests.in b/tests/gen_tests.in
> index ab844128..c3c983fe 100644
> --- a/tests/gen_tests.in
> +++ b/tests/gen_tests.in
> @@ -65,6 +65,7 @@ fcntl64 -a8
> fdatasync -a14
> file_handle -e trace=name_to_handle_at,open_by_handle_at
> file_ioctl +ioctl.test
> +filter_seccomp test_prog_set -n
This needs a prior check whether seccomp filter is unavailable, e.g.
filter_seccomp . "${srcdir=.}/filter_seccomp.sh"; test_prog_set -n
> finit_module -a25
> flock -a19
> fork-f -a26 -qq -f -e signal=none -e trace=chdir
> @@ -506,6 +507,7 @@ sync_file_range2
> sysinfo -a14
> syslog -a35
> tee
> +threads-execve +threads-execve.test -n
I don't think this works at all.
--
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20190831/136f8247/attachment.bin>
More information about the Strace-devel
mailing list