seccomp and audit_enabled

[Paul Moore]

On Mon, Oct 12, 2015 at 4:45 PM, Kees Cook <keescook at chromium.org> wrote:
> On Mon, Oct 12, 2015 at 10:53 AM, Tony Jones <tonyj at suse.de> wrote:
>> From d6971ec9508244f7a1ab42f9ac4c59b7e1ca6145 Mon Sep 17 00:00:00 2001
>> From: Tony Jones <tonyj at suse.de>
>> 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);
>>  }

-- 
paul moore
www.paul-moore.com