Re: seccomp and audit_enabled

On Mon, Oct 12, 2015 at 4:45 PM, Kees Cook <keescook(a)chromium.org&gt; wrote: > On Mon, Oct 12, 2015 at 10:53 AM, Tony Jones <tonyj(a)suse.de&gt; wrote: >> From d6971ec9508244f7a1ab42f9ac4c59b7e1ca6145 Mon Sep 17 00:00:00 2001 >> From: Tony Jones <tonyj(a)suse.de&gt; >> Date: Sat, 10 Oct 2015 19:30:49 -0700 >> Subject: [PATCH] Don't log seccomp messages when audit is disabled >> >> Don't log seccomp messages when audit is disabled. > > This is intentional since violation of a seccomp policy ought to > indicate a misbehaving program, and we want these to always be > presented to the system log, regardless of audit being enabled. (I'd > like to even produce system log entries when there is no CONFIG_AUDIT > too, but that's for the future.) I agree. As I mentioned earlier these AUDIT_SECCOMP records are very handy. >> diff --git a/include/linux/audit.h b/include/linux/audit.h >> index b2abc99..8f70f3f 100644 >> --- a/include/linux/audit.h >> +++ b/include/linux/audit.h >> @@ -113,6 +113,12 @@ struct filename; >> >> extern void audit_log_session_info(struct audit_buffer *ab); >> >> +#ifdef CONFIG_AUDIT >> +extern u32 audit_enabled; >> +#else >> +#define audit_enabled 0 >> +#endif >> + >> #ifdef CONFIG_AUDIT_COMPAT_GENERIC >> #define audit_is_compat(arch) (!((arch) & __AUDIT_ARCH_64BIT)) >> #else >> @@ -213,7 +219,7 @@ void audit_core_dumps(long signr); >> static inline void audit_seccomp(unsigned long syscall, long signr, int code) >> { >> /* Force a record to be reported if a signal was delivered. */ >> - if (signr || unlikely(!audit_dummy_context())) > > What is dummy_context part of this actually do? I don't think reports > should be made when signr == 0. The idea behind audit_dummy_context() is to skip auditing when there are no audit rules configured, it's a performance tweak. My guess is that Tony's system loads some audit configuration at boot which enables audit (the kernel starts with audit_enabled=0 ...) and loads a few syscall filter rules which are enough to make audit_dummy_context() return false. Can you confirm that Tony?

As for logging seccomp actions when signr == 0, I personally think that still might be useful as the normal behavior has been altered; I tend to think any action != ALLOW is worth logging. However, I'm open to discussion on this if others feel strongly. >> + if (audit_enabled && (signr || unlikely(!audit_dummy_context()))) >> __audit_seccomp(syscall, signr, code); >> }