3:49 p.m.
On Tue, Feb 7, 2017 at 4:33 PM, Kees Cook <keescook(a)chromium.org> wrote:
On Thu, Feb 2, 2017 at 9:37 PM, Tyler Hicks
<tyhicks(a)canonical.com> wrote:
> Add a new action, SECCOMP_RET_LOG, that logs a syscall before allowing
> the syscall. At the implementation level, this action is identical to
> the existing SECCOMP_RET_ALLOW action. However, it can be very useful when
> initially developing a seccomp filter for an application. The developer
> can set the default action to be SECCOMP_RET_LOG, maybe mark any
> obviously needed syscalls with SECCOMP_RET_ALLOW, and then put the
> application through its paces. A list of syscalls that triggered the
> default action (SECCOMP_RET_LOG) can be easily gleaned from the logs and
> that list can be used to build the syscall whitelist. Finally, the
> developer can change the default action to the desired value.
>
> This provides a more friendly experience than seeing the application get
> killed, then updating the filter and rebuilding the app, seeing the
> application get killed due to a different syscall, then updating the
> filter and rebuilding the app, etc.
>
> The functionality is similar to what's supported by the various LSMs.
> SELinux has permissive mode, AppArmor has complain mode, SMACK has
> bring-up mode, etc.
>
> SECCOMP_RET_LOG is given a lower value than SECCOMP_RET_ALLOW so that
> "allow" can be written to the max_action_to_log sysctl in order to get a
> list of logged actions without the, potentially larger, set of allowed
> actions.
>
> Signed-off-by: Tyler Hicks <tyhicks(a)canonical.com>
It came to my attention that OpenWRT has actually been running with a
SECCOMP_RET_LOG on at least the 3.18 kernels (added John to CC):
https://dev.openwrt.org/browser/trunk/target/linux/generic/patches-3.18/9...
This patch appears to have SECCOMP_RET_LOG perform a "skip" (rather
than allow, which contradicts the comment). (And since it's closer to
the KILL end, I assume the comment is wrong... well, inaccurate, it's
allowed in the sense that the process isn't killed, but the syscall
isn't executed, so it's not really allowed.)
Tyler's SECCOMP_RET_LOG performs the syscall and logs.
I'd like to make sure Tyler's series will meaningfully address what
OpenWRT needs, since it appears OpenWRT has a similar logging desire.
:)
John, what's the intention for this filter value? It looks like you
want to perform RET_ERRNO but get a log message? Would Tyler's global
log level API work for OpenWRT?
https://lkml.org/lkml/2017/2/3/13
i.e. change your default filter return to SECCOMP_RET_ERRNO, and set
the sysctl to ERRNO?
Thanks!
-Kees
> ---
> Documentation/prctl/seccomp_filter.txt | 6 ++++++
> include/uapi/linux/seccomp.h | 1 +
> kernel/seccomp.c | 4 ++++
> 3 files changed, 11 insertions(+)
>
> diff --git a/Documentation/prctl/seccomp_filter.txt
b/Documentation/prctl/seccomp_filter.txt
> index 1e469ef..ba55a91 100644
> --- a/Documentation/prctl/seccomp_filter.txt
> +++ b/Documentation/prctl/seccomp_filter.txt
> @@ -138,6 +138,12 @@ SECCOMP_RET_TRACE:
> allow use of ptrace, even of other sandboxed processes, without
> extreme care; ptracers can use this mechanism to escape.)
>
> +SECCOMP_RET_LOG:
> + Results in the system call being executed after it is logged. This
> + should be used by application developers to learn which syscalls their
> + application needs without having to iterate through multiple test and
> + development cycles to build the list.
> +
> SECCOMP_RET_ALLOW:
> Results in the system call being executed.
>
> diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
> index 0f238a4..67f72cd 100644
> --- a/include/uapi/linux/seccomp.h
> +++ b/include/uapi/linux/seccomp.h
> @@ -29,6 +29,7 @@
> #define SECCOMP_RET_TRAP 0x00030000U /* disallow and force a SIGSYS */
> #define SECCOMP_RET_ERRNO 0x00050000U /* returns an errno */
> #define SECCOMP_RET_TRACE 0x7ff00000U /* pass to a tracer or disallow */
> +#define SECCOMP_RET_LOG 0x7ffe0000U /* allow after logging */
This adds to UAPI, so it'd be good to think for a moment about how
this would work on older kernels: right now, if someone tried to use
this RET_LOG on an old kernel, it'll get treated like RET_KILL. Is
this sane?
I'm also trying to figure out if there is some other solution to this,
but they all involve tests against an otherwise RET_ALLOW case, which
I want to avoid. :)
So, I think, for now, this looks good, but I'd prefer this be
0x7ffc0000U, just to make sure we have not painted ourselves into a
numerical corner if we for some reason ever need to put something
between RET_ALLOW and RET_LOG.
> #define SECCOMP_RET_ALLOW 0x7fff0000U /* allow */
>
> /* Masks for the return value sections. */
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index 548fb89..8627481 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -650,6 +650,7 @@ static int __seccomp_filter(int this_syscall, const struct
seccomp_data *sd,
>
> return 0;
>
> + case SECCOMP_RET_LOG:
Given my protective feelings about the RET_ALLOW case, can you make
this a fully separate case statement? I'd rather have RET_ALLOW be
distinctly separate.
> case SECCOMP_RET_ALLOW:
> seccomp_log(this_syscall, 0, action);
> return 0;
> @@ -934,6 +935,7 @@ long seccomp_get_filter(struct task_struct *task, unsigned long
filter_off,
> #define SECCOMP_RET_TRAP_NAME "trap"
> #define SECCOMP_RET_ERRNO_NAME "errno"
> #define SECCOMP_RET_TRACE_NAME "trace"
> +#define SECCOMP_RET_LOG_NAME "log"
> #define SECCOMP_RET_ALLOW_NAME "allow"
>
> /* Largest strlen() of all action names */
> @@ -943,6 +945,7 @@ static char seccomp_actions_avail[] = SECCOMP_RET_KILL_NAME
" "
> SECCOMP_RET_TRAP_NAME " "
> SECCOMP_RET_ERRNO_NAME " "
> SECCOMP_RET_TRACE_NAME " "
> + SECCOMP_RET_LOG_NAME " "
> SECCOMP_RET_ALLOW_NAME;
>
> struct seccomp_action_name {
> @@ -955,6 +958,7 @@ static struct seccomp_action_name seccomp_action_names[] = {
> { SECCOMP_RET_TRAP, SECCOMP_RET_TRAP_NAME },
> { SECCOMP_RET_ERRNO, SECCOMP_RET_ERRNO_NAME },
> { SECCOMP_RET_TRACE, SECCOMP_RET_TRACE_NAME },
> + { SECCOMP_RET_LOG, SECCOMP_RET_LOG_NAME },
> { SECCOMP_RET_ALLOW, SECCOMP_RET_ALLOW_NAME },
> { }
> };
> --
> 2.7.4
>
-Kees
--
Kees Cook
Pixel Security
--
Kees Cook
Pixel Security