Re: Limiting SECCOMP audit events

On December 14, 2017 6:06:49 PM Tyler Hicks <tyhicks(a)canonical.com&gt; 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. > > 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. > > 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. 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. > > I hope this helps! > > Tyler > > 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); > -} > - Looks good to me but two things: * Change the name of __audit_seccomp() to audit_seccomp() since we don't have two functions anymore. * Are we sure about removing the audit_enabled check? People got pretty upset when it wasn't there in the past.

> 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); > } > > /* > > -- > Linux-audit mailing list > Linux-audit(a)redhat.com > https://www.redhat.com/mailman/listinfo/linux-audit