On Thursday, December 14, 2017 6:06:30 PM EST Tyler Hicks wrote:
On 12/14/2017 09:19 AM, Steve Grubb wrote:
> On Thursday, December 14, 2017 10:04:48 AM EST Tyler Hicks wrote:
>> On 12/13/2017 05:58 PM, Steve Grubb wrote:
>> > Over the last month, the amount of seccomp events in audit logs is
>> >
>> > sky-rocketing. I have over a million events in the last 2 days. Most of
>> >
>> > this is generated by firefox and qt webkit.
>> >
>> >
>> >
>> > I am wondering if the audit package should ship a file for
>> >
>> >
>> >
>> > /usr/lib/sysctl.d/60-auditd.conf
>> >
>> >
>> >
>> > wherein it has
>> >
>> >
>> >
>> > kernel.seccomp.actions_logged = kill_process kill_thread errno
>>
>> I agree with Kees here. IMO, you only want "kill_process kill_thread"
>>
>> which is the default.
>
>
>
> The default appears to be all of the types of events without setting
> kernel.seccomp.actions_logged.
Ah, right. I didn't correctly remember the final implementation details.
The default sysctl setting is to allow all actions except for RET_ALLOW
to be logged.
I think the easiest description of the logic is in the commit message of
59f5cf44a38284eb9e76270c786fb6cc62ef8ac4:
if action == RET_ALLOW:
do not log
else if action == RET_KILL && RET_KILL in actions_logged:
log
else if action == RET_LOG && RET_LOG in actions_logged:
log
else if filter-requests-logging && action in actions_logged:
log
else if audit_enabled && process-is-being-audited:
log
else:
do not log
I think I originally misunderstood your first email in this thread. I
thought you were saying that you were experiencing more seccomp audit
events in 4.14 versus 4.13 and that you felt a regression had been
introduced. After rereading, I think you're asking why you're getting
seccomp RET_TRAP actions logged even though "trap" isn't in the
actions_logged sysctl.
Yes, exactly. I have been experiencing large amounts of SECCOMP events
starting with qt webkit in kde and thought 4.14 would finally let me tame
those events. I have opened a couple bz asking developers if they really meant
to go live with a policy that is experiencing so many denials. But the
consensus is this is intended. (But I think they also have not actually tried
to use their audit logs.)
The reason is because I didn't get clear direction from the
audit
folks about to do when audit is enabled and the process is being audited
and, therefore, I didn't feel comfortable rocking the boat. In that
situation, the decision to log is the same as it was in earlier kernels.
Specifically, you're hitting the last "else if" conditional in the
pseudocode above.
And here I thought you were also seeing large numbers of seccomp events and
were making a way to control what gets logged. In any event, I think we better
understand each other now. :-)
If you're happy with having the actions_logged sysctl control
whether or
not to log seccomp actions taken for processes that are being audited,
then I think the following (untested) patch should do exactly what you
want.
OK. Great. With developers starting to use the trap return value, audit logs
are getting swamped by benign events. We truly need a knob to eliminate the
noise from the signal.
I imagine that you'd also want seccomp to emit audit events
whenever the
value of the actions_logged sysctl is changed, which should be pretty easy
to do.
Sure. If you want to add it, then it should be roughly like this:
struct tty_struct *tty;
const struct cred *cred;
struct audit_buffer *ab;
char comm[sizeof(current->comm)];
ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
if (unlikely(!ab))
return;
cred = current_cred();
tty = audit_get_tty(current);
audit_log_format(ab, "pid=%d uid=%u auid=%u tty=%s ses=%u",
task_tgid_nr(current),
from_kuid(&init_user_ns, cred->uid),
from_kuid(&init_user_ns,
audit_get_loginuid(current)),
tty ? tty_name(tty) : "(none)",
audit_get_sessionid(current));
audit_put_tty(tty);
audit_log_task_context(ab);
audit_log_format(ab, " comm=");
audit_log_untrustedstring(ab, get_task_comm(comm, current));
audit_log_d_path_exe(ab, current->mm);
audit_log_format(ab, "op=seccomp-logging");
<You can log the new value here if you wish - just can't have spaces in the
value. Numbers or mask is fine.>
audit_log_format(ab, " res=%u", res);
where res above is a 1 for success and 0 for failure. Failure is likely to be
due to not having the capability that allows setting the sysctl.
I hope this helps!
Thanks!
-Steve
diff --git a/include/linux/audit.h b/include/linux/audit.h
index af410d9..095b5dd 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -304,12 +304,6 @@ static inline void audit_inode_child(struct inode
*parent, }
void audit_core_dumps(long signr);
-static inline void audit_seccomp(unsigned long syscall, long signr, int
code) -{
- if (audit_enabled && unlikely(!audit_dummy_context()))
- __audit_seccomp(syscall, signr, code);
-}
-
static inline void audit_ptrace(struct task_struct *t)
{
if (unlikely(!audit_dummy_context()))
@@ -502,8 +496,6 @@ static inline void audit_core_dumps(long signr)
{ }
static inline void __audit_seccomp(unsigned long syscall, long signr, int
code) { }
-static inline void audit_seccomp(unsigned long syscall, long signr, int
code) -{ }
static inline int auditsc_get_stamp(struct audit_context *ctx,
struct timespec64 *t, unsigned int *serial)
{
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index 5f0dfb2ab..914a707 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -590,12 +590,6 @@ static inline void seccomp_log(unsigned long syscall,
long signr, u32 action, */
if (log)
return __audit_seccomp(syscall, signr, action);
-
- /*
- * Let the audit subsystem decide if the action should be audited based
- * on whether the current task itself is being audited.
- */
- return audit_seccomp(syscall, signr, action);
}
/*