Re: [PATCH] audit: Don't spam logs with SECCOMP_KILL/RET_ERRNO by default

Re: [PATCH] audit: Don't spam logs with SECCOMP_KILL/RET_ERRNO by default

[Paul Moore]

On Mon, Apr 11, 2016 at 12:13 AM, Andi Kleen <andi@firstfloor.org> wrote:
> From: Andi Kleen <ak@linux.intel.com>
>
> When I run chrome on my opensuse system every time I open
> a new tab the system log is spammed with:
>
> audit[16857]: SECCOMP auid=1000 uid=1000 gid=100 ses=1 pid=16857
> comm="chrome" exe="/opt/google/chrome/chrome" sig=0 arch=c000003e
> syscall=273 compat=0 ip=0x7fe27c11a444 code=0x50000
>
> This happens because chrome uses SECCOMP for its sandbox,
> and for some reason always reaches a SECCOMP_KILL or more likely
> SECCOMP_RET_ERRNO in the rule set.
>
> The seccomp auditing was originally added by Eric with
>
>     commit 85e7bac33b8d5edafc4e219c7dfdb3d48e0b4e31
>     Author: Eric Paris <eparis@redhat.com>
>     Date:   Tue Jan 3 14:23:05 2012 -0500
>
>         seccomp: audit abnormal end to a process due to seccomp
>
>         The audit system likes to collect information about processes that end
>         abnormally (SIGSEGV) as this may me useful intrusion detection information.
>         This patch adds audit support to collect information when seccomp
>         forces a task to exit because of misbehavior in a similar way.
>
> I don't have any other syscall auditing enabled,
> just the standard user space auditing used by the systemd
> and PAM userland. So basic auditing is alwas enabled,
> but no other kernel auditing.
>
> Add a sysctl to enable this unconditional behavior with default
> to off. This replaces an earlier patch that simply checked
> whether syscall auditing was on, but Paul Moore preferred
> this more elaborate approach.
>
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---
>  Documentation/sysctl/kernel.txt |  9 +++++++++
>  include/linux/audit.h           |  4 +++-
>  kernel/seccomp.c                |  4 ++++
>  kernel/sysctl.c                 | 11 +++++++++++
>  4 files changed, 27 insertions(+), 1 deletion(-)

Quick response as I'm traveling the next few days and
time/connectivity will be spotty ... thanks for sending an updated
patch, some initial thoughts:

* My thinking was that the sysctl knob could be a threshold value such
that setting it to 0x00030000 would only log TRAP and KILL.
* With the sysctl tunable defaulting to no-logging there is no need to
check for audit_enabled, further, checking for audit_enabled would
prevent logging to dmesg/syslog which I believe is valuable (you may
not).
* A bit nitpicky, but considering the possibility of logging to
dmesg/syslog when auditing is disabled, I think
"seccomp-log-threshold" or similar would be a better sysctl name.

> diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
> index 57653a4..abc6ef9 100644
> --- a/Documentation/sysctl/kernel.txt
> +++ b/Documentation/sysctl/kernel.txt
> @@ -21,6 +21,7 @@ show up in /proc/sys/kernel:
>  - acct
>  - acpi_video_flags
>  - auto_msgmni
> +- audit_log_seccomp
>  - bootloader_type           [ X86 only ]
>  - bootloader_version        [ X86 only ]
>  - callhome                  [ S390 only ]
> @@ -129,6 +130,14 @@ upon memory add/remove or upon ipc namespace creation/removal.
>  Echoing "1" into this file enabled msgmni automatic recomputing.
>  Echoing "0" turned it off. auto_msgmni default value was 1.
>
> +==============================================================
> +
> +audit_log_seccomp
> +
> +When this variable is set to 1 every SECCOMP_KILL/SECCOMP_RET_ERRNO
> +results in an audit log. This is generally a bad idea because
> +it leads to a audit message every time Chrome opens a new tab.
> +Defaults to 0.
>
>  ==============================================================
>
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index e38e3fc..c7787ba 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -315,9 +315,11 @@ static inline void audit_inode_child(struct inode *parent,
>  }
>  void audit_core_dumps(long signr);
>
> +extern int audit_log_seccomp;
> +
>  static inline void audit_seccomp(unsigned long syscall, long signr, int code)
>  {
> -       if (!audit_enabled)
> +       if (!audit_enabled || !audit_log_seccomp)
>                 return;
>
>         /* Force a record to be reported if a signal was delivered. */
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index e1e5a35..09a8b03 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -25,6 +25,10 @@
>  #include <asm/syscall.h>
>  #endif
>
> +#ifdef CONFIG_AUDIT
> +int audit_log_seccomp __read_mostly = 0;
> +#endif
> +
>  #ifdef CONFIG_SECCOMP_FILTER
>  #include <linux/filter.h>
>  #include <linux/pid.h>
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 725587f..0c7611e 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -65,6 +65,7 @@
>  #include <linux/sched/sysctl.h>
>  #include <linux/kexec.h>
>  #include <linux/bpf.h>
> +#include <linux/audit.h>
>
>  #include <asm/uaccess.h>
>  #include <asm/processor.h>
> @@ -529,6 +530,16 @@ static struct ctl_table kern_table[] = {
>                 .proc_handler   = proc_dointvec,
>         },
>  #endif
> +#ifdef CONFIG_AUDIT
> +       {
> +               .procname       = "audit-log-seccomp",
> +               .data           = &audit_log_seccomp,
> +               .maxlen         = sizeof(int),
> +               .mode           = 0644,
> +               .proc_handler   = proc_dointvec,
> +       },
> +
> +#endif
>         {
>                 .procname       = "print-fatal-signals",
>                 .data           = &print_fatal_signals,
> --
> 2.7.4
>



-- 
paul moore
www.paul-moore.com

Re: [PATCH] audit: Don't spam logs with SECCOMP_KILL/RET_ERRNO by default

[Eric Paris]

Just an FYI originally the idea was to follow the pattern of logging
set by core dumps see kernel/auditsc.c::audit_core_dumps(). Which is
gated by audit_enable but not anything else. I believe at that time the
only option was kill, which meant, much like the core dumper, spam was
not a likely result given the initiator is killed.

I'm all for a way to shut up unsolicited audit messages, especially
seccomp with errno or trap. I think it would be best to default 'KILL'
to on and everything else to off. I'm no so sure a sysctl is the right
way though. Enabling more forms of 'seccomp audit' should really be a
part of the audit policy.

(p.s. I think the action should be part of the seccomp message, as
right now all we know is that Andi's message isn't KILL since the
sig=0)

-Eric


On Mon, 2016-04-11 at 09:30 -0400, Paul Moore wrote:
> On Mon, Apr 11, 2016 at 12:13 AM, Andi Kleen <andi@firstfloor.org>
> wrote:
> > 
> > From: Andi Kleen <ak@linux.intel.com>
> > 
> > When I run chrome on my opensuse system every time I open
> > a new tab the system log is spammed with:
> > 
> > audit[16857]: SECCOMP auid=1000 uid=1000 gid=100 ses=1 pid=16857
> > comm="chrome" exe="/opt/google/chrome/chrome" sig=0 arch=c000003e
> > syscall=273 compat=0 ip=0x7fe27c11a444 code=0x50000
> > 
> > This happens because chrome uses SECCOMP for its sandbox,
> > and for some reason always reaches a SECCOMP_KILL or more likely
> > SECCOMP_RET_ERRNO in the rule set.
> > 
> > The seccomp auditing was originally added by Eric with
> > 
> >     commit 85e7bac33b8d5edafc4e219c7dfdb3d48e0b4e31
> >     Author: Eric Paris <eparis@redhat.com>
> >     Date:   Tue Jan 3 14:23:05 2012 -0500
> > 
> >         seccomp: audit abnormal end to a process due to seccomp
> > 
> >         The audit system likes to collect information about
> > processes that end
> >         abnormally (SIGSEGV) as this may me useful intrusion
> > detection information.
> >         This patch adds audit support to collect information when
> > seccomp
> >         forces a task to exit because of misbehavior in a similar
> > way.
> > 
> > I don't have any other syscall auditing enabled,
> > just the standard user space auditing used by the systemd
> > and PAM userland. So basic auditing is alwas enabled,
> > but no other kernel auditing.
> > 
> > Add a sysctl to enable this unconditional behavior with default
> > to off. This replaces an earlier patch that simply checked
> > whether syscall auditing was on, but Paul Moore preferred
> > this more elaborate approach.
> > 
> > Signed-off-by: Andi Kleen <ak@linux.intel.com>
> > ---
> >  Documentation/sysctl/kernel.txt |  9 +++++++++
> >  include/linux/audit.h           |  4 +++-
> >  kernel/seccomp.c                |  4 ++++
> >  kernel/sysctl.c                 | 11 +++++++++++
> >  4 files changed, 27 insertions(+), 1 deletion(-)
> Quick response as I'm traveling the next few days and
> time/connectivity will be spotty ... thanks for sending an updated
> patch, some initial thoughts:
> 
> * My thinking was that the sysctl knob could be a threshold value
> such
> that setting it to 0x00030000 would only log TRAP and KILL.
> * With the sysctl tunable defaulting to no-logging there is no need
> to
> check for audit_enabled, further, checking for audit_enabled would
> prevent logging to dmesg/syslog which I believe is valuable (you may
> not).
> * A bit nitpicky, but considering the possibility of logging to
> dmesg/syslog when auditing is disabled, I think
> "seccomp-log-threshold" or similar would be a better sysctl name.
> 
> > 
> > diff --git a/Documentation/sysctl/kernel.txt
> > b/Documentation/sysctl/kernel.txt
> > index 57653a4..abc6ef9 100644
> > --- a/Documentation/sysctl/kernel.txt
> > +++ b/Documentation/sysctl/kernel.txt
> > @@ -21,6 +21,7 @@ show up in /proc/sys/kernel:
> >  - acct
> >  - acpi_video_flags
> >  - auto_msgmni
> > +- audit_log_seccomp
> >  - bootloader_type           [ X86 only ]
> >  - bootloader_version        [ X86 only ]
> >  - callhome                  [ S390 only ]
> > @@ -129,6 +130,14 @@ upon memory add/remove or upon ipc namespace
> > creation/removal.
> >  Echoing "1" into this file enabled msgmni automatic recomputing.
> >  Echoing "0" turned it off. auto_msgmni default value was 1.
> > 
> > +==============================================================
> > +
> > +audit_log_seccomp
> > +
> > +When this variable is set to 1 every
> > SECCOMP_KILL/SECCOMP_RET_ERRNO
> > +results in an audit log. This is generally a bad idea because
> > +it leads to a audit message every time Chrome opens a new tab.
> > +Defaults to 0.
> > 
> >  ==============================================================
> > 
> > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > index e38e3fc..c7787ba 100644
> > --- a/include/linux/audit.h
> > +++ b/include/linux/audit.h
> > @@ -315,9 +315,11 @@ static inline void audit_inode_child(struct
> > inode *parent,
> >  }
> >  void audit_core_dumps(long signr);
> > 
> > +extern int audit_log_seccomp;
> > +
> >  static inline void audit_seccomp(unsigned long syscall, long
> > signr, int code)
> >  {
> > -       if (!audit_enabled)
> > +       if (!audit_enabled || !audit_log_seccomp)
> >                 return;
> > 
> >         /* Force a record to be reported if a signal was delivered.
> > */
> > diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> > index e1e5a35..09a8b03 100644
> > --- a/kernel/seccomp.c
> > +++ b/kernel/seccomp.c
> > @@ -25,6 +25,10 @@
> >  #include <asm/syscall.h>
> >  #endif
> > 
> > +#ifdef CONFIG_AUDIT
> > +int audit_log_seccomp __read_mostly = 0;
> > +#endif
> > +
> >  #ifdef CONFIG_SECCOMP_FILTER
> >  #include <linux/filter.h>
> >  #include <linux/pid.h>
> > diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> > index 725587f..0c7611e 100644
> > --- a/kernel/sysctl.c
> > +++ b/kernel/sysctl.c
> > @@ -65,6 +65,7 @@
> >  #include <linux/sched/sysctl.h>
> >  #include <linux/kexec.h>
> >  #include <linux/bpf.h>
> > +#include <linux/audit.h>
> > 
> >  #include <asm/uaccess.h>
> >  #include <asm/processor.h>
> > @@ -529,6 +530,16 @@ static struct ctl_table kern_table[] = {
> >                 .proc_handler   = proc_dointvec,
> >         },
> >  #endif
> > +#ifdef CONFIG_AUDIT
> > +       {
> > +               .procname       = "audit-log-seccomp",
> > +               .data           = &audit_log_seccomp,
> > +               .maxlen         = sizeof(int),
> > +               .mode           = 0644,
> > +               .proc_handler   = proc_dointvec,
> > +       },
> > +
> > +#endif
> >         {
> >                 .procname       = "print-fatal-signals",
> >                 .data           = &print_fatal_signals,
> > --
> > 2.7.4
> > 
> 
> 

Re: [PATCH] audit: Don't spam logs with SECCOMP_KILL/RET_ERRNO by default

[Andi Kleen]

On Mon, Apr 11, 2016 at 10:58:06AM -0500, Eric Paris wrote:
> Just an FYI originally the idea was to follow the pattern of logging
> set by core dumps see kernel/auditsc.c::audit_core_dumps(). Which is
> gated by audit_enable but not anything else. I believe at that time the
> only option was kill, which meant, much like the core dumper, spam was
> not a likely result given the initiator is killed.

Given that user space now uses audit independently for its own
logging I don't think making things depend only on audit_enable
is good practice anymore.

> 
> I'm all for a way to shut up unsolicited audit messages, especially
> seccomp with errno or trap. I think it would be best to default 'KILL'
> to on and everything else to off. I'm no so sure a sysctl is the right
> way though. Enabling more forms of 'seccomp audit' should really be a
> part of the audit policy.

That was my original patch -- make it conditional on syscall auditing.

If that's the right approach please apply that one.

-Andi

Re: [PATCH] audit: Don't spam logs with SECCOMP_KILL/RET_ERRNO by default

[Paul Moore]

On Monday, April 11, 2016 10:58:06 AM Eric Paris wrote:
> I'm all for a way to shut up unsolicited audit messages, especially
> seccomp with errno or trap. I think it would be best to default 'KILL'
> to on and everything else to off. I'm no so sure a sysctl is the right
> way though. Enabling more forms of 'seccomp audit' should really be a
> part of the audit policy.

The seccomp events are very useful for people who are working with seccomp 
filters and I want to ensure that we have the ability to emit these events 
regardless of if audit is enabled, or even compiled into the kernel using 
dmesg/syslog as we do today with other auditable events, e.g. SELinux.

Because of this desire to log regardless of audit, I figured a sysctl tunable 
made more sense than an audit based filter.  As I mentioned previously, I'm 
not completely sold on the sysctl based solution, but it is the best solution 
that I can think of at the moment.  Alternatives are welcome.

-- 
paul moore
www.paul-moore.com