On Tue, Oct 13, 2015 at 1:18 PM, Tony Jones <to...@suse.de> wrote:
> On 10/13/2015 09:11 AM, Paul Moore wrote:
>> On Mon, Oct 12, 2015 at 4:45 PM, Kees Cook <keesc...@chromium.org> wrote:
>>> On Mon, Oct 12, 2015 at 10:53 AM, Tony Jones <to...@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
--
To unsubscribe from this list: send the line "unsubscribe
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
