Re: seccomp and audit_enabled
On Tue, Oct 13, 2015 at 1:18 PM, Tony Jones <tonyj(a)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(a)chromium.org> wrote:
>> On Mon, Oct 12, 2015 at 10:53 AM, Tony Jones <tonyj(a)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