Re: Limiting SECCOMP audit events

[Paul Moore <paul@paul-moore.com>]

On December 14, 2017 6:06:49 PM Tyler Hicks <tyhicks@canonical.com> 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@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit


--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit