2016-04-11 04:17:08

by Andi Kleen

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

From: Andi Kleen <[email protected]>

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 <[email protected]>
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 <[email protected]>
---
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(-)

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


2016-04-11 04:32:44

by kernel test robot

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

Hi Andi,

[auto build test ERROR on pcmoore-audit/next]
[also build test ERROR on v4.6-rc3 next-20160408]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url: https://github.com/0day-ci/linux/commits/Andi-Kleen/audit-Don-t-spam-logs-with-SECCOMP_KILL-RET_ERRNO-by-default/20160411-122005
base: git://git.infradead.org/users/pcmoore/audit next
config: xtensa-allyesconfig (attached as .config)
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=xtensa

All errors (new ones prefixed by >>):

>> kernel/sysctl.c:524:13: error: 'audit_log_seccomp' undeclared here (not in a function)
.data = &audit_log_seccomp,
^

vim +/audit_log_seccomp +524 kernel/sysctl.c

518 .proc_handler = proc_dointvec,
519 },
520 #endif
521 #ifdef CONFIG_AUDIT
522 {
523 .procname = "audit-log-seccomp",
> 524 .data = &audit_log_seccomp,
525 .maxlen = sizeof(int),
526 .mode = 0644,
527 .proc_handler = proc_dointvec,

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (1.34 kB)
.config.gz (43.02 kB)
Download all attachments

2016-04-11 05:26:57

by kernel test robot

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

Hi Andi,

[auto build test ERROR on pcmoore-audit/next]
[also build test ERROR on v4.6-rc3 next-20160408]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url: https://github.com/0day-ci/linux/commits/Andi-Kleen/audit-Don-t-spam-logs-with-SECCOMP_KILL-RET_ERRNO-by-default/20160411-122005
base: git://git.infradead.org/users/pcmoore/audit next
config: i386-randconfig-r0-201615 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=i386

All errors (new ones prefixed by >>):

>> kernel/built-in.o:(.data+0xf3c): undefined reference to `audit_log_seccomp'

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (829.00 B)
.config.gz (32.00 kB)
Download all attachments

2016-04-11 13:30:23

by Paul Moore

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

On Mon, Apr 11, 2016 at 12:13 AM, Andi Kleen <[email protected]> wrote:
> From: Andi Kleen <[email protected]>
>
> 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 <[email protected]>
> 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 <[email protected]>
> ---
> 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
http://www.paul-moore.com

2016-04-11 15:58:12

by Eric Paris

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

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 <[email protected]>
> wrote:
> >
> > From: Andi Kleen <[email protected]>
> >
> > 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 <[email protected]>
> >     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 <[email protected]>
> > ---
> >  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
> >
>
>

2016-04-11 21:55:22

by Andi Kleen

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

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

2016-04-12 01:17:50

by Paul Moore

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

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
http://www.paul-moore.com