From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tyler Hicks Subject: Re: Limiting SECCOMP audit events Date: Thu, 14 Dec 2017 23:06:30 +0000 Message-ID: <20171214230629.GA451@sec> References: <58203247.sCqcla2mis@x2> <36cd827f-201c-8f76-2883-ecd930cbb1f4@canonical.com> <3499769.OM7YpPIT3e@x2> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Content-Disposition: inline In-Reply-To: <3499769.OM7YpPIT3e@x2> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-audit-bounces@redhat.com Errors-To: linux-audit-bounces@redhat.com To: Steve Grubb Cc: Linux Audit List-Id: linux-audit@redhat.com 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 =3D kill_process kill_thread errno > = >> > = >> I agree with Kees here. IMO, you only want "kill_process kill_thread" > = >> which is the default. > = > =A0 > = > 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 =3D=3D RET_ALLOW: do not log else if action =3D=3D RET_KILL && RET_KILL in actions_logged: log else if action =3D=3D 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 *par= ent, } void audit_core_dumps(long signr); = -static inline void audit_seccomp(unsigned long syscall, long signr, int co= de) -{ - 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 co= de) -{ } 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); } = /*