seccomp and audit_enabled

[Paul Moore]

On Tue, Oct 13, 2015 at 1:18 PM, Tony Jones <tonyj at suse.de> wrote:
> On 10/13/2015 09:11 AM, Paul Moore wrote:
>> 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:
>>>> 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?
>
> No, it's the default audit.rules (-D, -b320).   No actual rules loaded.
> Let me add some instrumentation and figure out what's going on.  auditd
> is masked (via systemd) but systemd-journal seems to set audit_enabled=1
> during startup (at least on our systems).

Yes, if systemd is involved it enables audit; we've had some
discussions with the systemd folks about fixing that, but they haven't
gone very far.  I'm still a little curious as to why
audit_dummy_context() is false in this case, but I haven't looked at
how systemd/auditctl start/config the system too closely.

>> 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);
>>>>  }
>
> I'm of the opinion that nothing should get output (through the audit system) if
> audit_enabled == 0.  What you advocate calls for more than 2 possible states for
> audit_enabled or logging the information through another mechanism than audit.

I don't really care if it is audit or not (although we will need to
output something via audit if it is enabled to keep the CC crowd
happy); if you feel strongly that it isn't audit, we can just make it
a printk, that would work well with Kees' goals.  To me the important
point here is that we send a message when seccomp alters the behavior
of the syscall (action != ALLOW).

-- 
paul moore
www.paul-moore.com