seccomp and audit_enabled

seccomp and audit_enabled

[Tony Jones]

Hi.

What is the expected handling of AUDIT_SECCOMP if audit_enabled == 0?   Opera browser makes use of a sandbox and if audit_enabled == 0 (and no auditd is running) there is a lot of messages dumped to the klog. The fix to __audit_seccomp() is trivial, similar to c2412d91c and I can send a patch, I'm just not sure if seccomp is somehow special?

Thanks

Tony

Re: seccomp and audit_enabled

[Paul Moore]

On Friday, October 09, 2015 08:50:01 PM Tony Jones wrote:
> Hi.
> 
> What is the expected handling of AUDIT_SECCOMP if audit_enabled == 0?  
> Opera browser makes use of a sandbox and if audit_enabled == 0 (and no
> auditd is running) there is a lot of messages dumped to the klog. The fix
> to __audit_seccomp() is trivial, similar to c2412d91c and I can send a
> patch, I'm just not sure if seccomp is somehow special?

I'm adding Kees to this since he looks after the seccomp kernel bits these 
days.  While there isn't anything special about seccomp from an audit 
perspective, the seccomp audit record can be a really nice thing as it is the 
only indication you may get that seccomp has stepped in and done "something" 
other than allow the syscall to progress normally.

I would be a little more concerned that you are seeing a flood of seccomp 
messages from Opera, that is something that most likely warrants some closer 
inspection.  Are all the records the same/similar?  Can you paste some into 
email?

-- 
paul moore
www.paul-moore.com

Re: seccomp and audit_enabled

[Paul Moore]

My apologies for the resend, I had the wrong email for Kees.

On Monday, October 12, 2015 11:29:43 AM Paul Moore wrote:
> On Friday, October 09, 2015 08:50:01 PM Tony Jones wrote:
> > Hi.
> > 
> > What is the expected handling of AUDIT_SECCOMP if audit_enabled == 0?
> > Opera browser makes use of a sandbox and if audit_enabled == 0 (and no
> > auditd is running) there is a lot of messages dumped to the klog. The fix
> > to __audit_seccomp() is trivial, similar to c2412d91c and I can send a
> > patch, I'm just not sure if seccomp is somehow special?
> 
> I'm adding Kees to this since he looks after the seccomp kernel bits these
> days.  While there isn't anything special about seccomp from an audit
> perspective, the seccomp audit record can be a really nice thing as it is
> the only indication you may get that seccomp has stepped in and done
> "something" other than allow the syscall to progress normally.
> 
> I would be a little more concerned that you are seeing a flood of seccomp
> messages from Opera, that is something that most likely warrants some closer
> inspection.  Are all the records the same/similar?  Can you paste some into
> email?

-- 
paul moore
www.paul-moore.com

Re: seccomp and audit_enabled

[Tony Jones]

On 10/12/2015 08:40 AM, Paul Moore wrote:
> My apologies for the resend, I had the wrong email for Kees.
> 
> On Monday, October 12, 2015 11:29:43 AM Paul Moore wrote:
>> On Friday, October 09, 2015 08:50:01 PM Tony Jones wrote:
>>> Hi.
>>>
>>> What is the expected handling of AUDIT_SECCOMP if audit_enabled == 0?
>>> Opera browser makes use of a sandbox and if audit_enabled == 0 (and no
>>> auditd is running) there is a lot of messages dumped to the klog. The fix
>>> to __audit_seccomp() is trivial, similar to c2412d91c and I can send a
>>> patch, I'm just not sure if seccomp is somehow special?
>>
>> I'm adding Kees to this since he looks after the seccomp kernel bits these
>> days.  While there isn't anything special about seccomp from an audit
>> perspective, the seccomp audit record can be a really nice thing as it is
>> the only indication you may get that seccomp has stepped in and done
>> "something" other than allow the syscall to progress normally.

The issue is that (without auditd running) the messages are output to klog regardless 
of whether audit_enabled is 0 or 1.  As I said, other occurrences of this such as with
login events has been corrected (c2412d91c). Attached patch does same for seccomp.

>> I would be a little more concerned that you are seeing a flood of seccomp
>> messages from Opera, that is something that most likely warrants some closer
>> inspection.  Are all the records the same/similar?  Can you paste some into
>> email?

Here is the logged messages per invocation of opera.  the use of the sandbox may well
be the result of a local suse config/packaging decision but I'm not sure that's relevant.

2015-10-10T19:35:23.237882-07:00 nohostname kernel: [  152.100348] audit: type=1326 audit(1444530923.236:356): auid=1000 uid=1000 gid=100 ses=1 pid=2048 comm="opera" exe="/usr/lib64/opera/opera" sig=0 arch=c000003e syscall=91 compat=0 ip=0x7ff926d94ab7 code=0x50000
2015-10-10T19:35:23.242867-07:00 nohostname kernel: [  152.105690] audit: type=1326 audit(1444530923.241:357): auid=1000 uid=1000 gid=100 ses=1 pid=2087 comm="opera" exe="/usr/lib64/opera/opera" sig=0 arch=c000003e syscall=273 compat=0 ip=0x7ff928325444 code=0x50000
2015-10-10T19:35:23.242873-07:00 nohostname kernel: [  152.105938] audit: type=1326 audit(1444530923.241:358): auid=1000 uid=1000 gid=100 ses=1 pid=2089 comm="opera" exe="/usr/lib64/opera/opera" sig=0 arch=c000003e syscall=273 compat=0 ip=0x7ff928325444 code=0x50000
2015-10-10T19:35:23.243890-07:00 nohostname kernel: [  152.106845] audit: type=1326 audit(1444530923.242:359): auid=1000 uid=1000 gid=100 ses=1 pid=2048 comm="opera" exe="/usr/lib64/opera/opera" sig=0 arch=c000003e syscall=2 compat=0 ip=0x7ff926d6daa1 code=0x30000
2015-10-10T19:35:23.275872-07:00 nohostname kernel: [  152.138819] audit: type=1326 audit(1444530923.273:360): auid=1000 uid=1000 gid=100 ses=1 pid=2093 comm="opera" exe="/usr/lib64/opera/opera" sig=0 arch=c000003e syscall=91 compat=0 ip=0x7f92e4bd7ab7 code=0x50000
2015-10-10T19:35:23.275885-07:00 nohostname kernel: [  152.138937] audit: type=1326 audit(1444530923.274:361): auid=1000 uid=1000 gid=100 ses=1 pid=2093 comm="opera" exe="/usr/lib64/opera/opera" sig=0 arch=c000003e syscall=91 compat=0 ip=0x7f92e4bd7ab7 code=0x50000
2015-10-10T19:35:23.280867-07:00 nohostname kernel: [  152.143147] audit: type=1326 audit(1444530923.279:362): auid=1000 uid=1000 gid=100 ses=1 pid=2096 comm="opera" exe="/usr/lib64/opera/opera" sig=0 arch=c000003e syscall=273 compat=0 ip=0x7f92e6168444 code=0x50000
2015-10-10T19:35:23.282055-07:00 nohostname kernel: [  152.144762] audit: type=1326 audit(1444530923.280:363): auid=1000 uid=1000 gid=100 ses=1 pid=2093 comm="opera" exe="/usr/lib64/opera/opera" sig=0 arch=c000003e syscall=2 compat=0 ip=0x7f92eb5f8587 code=0x50000
2015-10-10T19:35:23.282062-07:00 nohostname kernel: [  152.144890] audit: type=1326 audit(1444530923.280:364): auid=1000 uid=1000 gid=100 ses=1 pid=2093 comm="opera" exe="/usr/lib64/opera/opera" sig=0 arch=c000003e syscall=2 compat=0 ip=0x7f92e4b2ac8c code=0x50000
2015-10-10T19:35:23.282063-07:00 nohostname kernel: [  152.144988] audit: type=1326 audit(1444530923.280:365): auid=1000 uid=1000 gid=100 ses=1 pid=2093 comm="opera" exe="/usr/lib64/opera/opera" sig=0 arch=c000003e syscall=2 compat=0 ip=0x7f92e4b2ad70 code=0x50000


thanks

tony


>From d6971ec9508244f7a1ab42f9ac4c59b7e1ca6145 Mon Sep 17 00:00:00 2001
From: Tony Jones <tonyj@suse.de>
Date: Sat, 10 Oct 2015 19:30:49 -0700
Subject: [PATCH] Don't log seccomp messages when audit is disabled

Don't log seccomp messages when audit is disabled.   

Currently, each time the opera browser is executed 10 records similar to the 
following are output to klog (when !audit_enabled).

2015-10-10T19:35:23.237882-07:00 nohostname kernel: [  152.100348] audit: type=1326 audit(1444530923.236:356): auid=1000 uid=1000 gid=100 ses=1 pid=2048 comm="opera" exe="/usr/lib64/opera/opera" sig=0 arch=c000003e syscall=91 compat=0 ip=0x7ff926d94ab7 code=0x50000

Signed-off-by: Tony Jones <tonyj@suse.de>
---
 include/linux/audit.h | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index b2abc99..8f70f3f 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -113,6 +113,12 @@ struct filename;
 
 extern void audit_log_session_info(struct audit_buffer *ab);
 
+#ifdef CONFIG_AUDIT
+extern u32 audit_enabled;
+#else
+#define audit_enabled 0
+#endif
+
 #ifdef CONFIG_AUDIT_COMPAT_GENERIC
 #define audit_is_compat(arch)  (!((arch) & __AUDIT_ARCH_64BIT))
 #else
@@ -213,7 +219,7 @@ void audit_core_dumps(long signr);
 static inline void audit_seccomp(unsigned long syscall, long signr, int code)
 {
 	/* Force a record to be reported if a signal was delivered. */
-	if (signr || unlikely(!audit_dummy_context()))
+	if (audit_enabled && (signr || unlikely(!audit_dummy_context())))
 		__audit_seccomp(syscall, signr, code);
 }
 
@@ -498,7 +504,6 @@ extern int audit_rule_change(int type, __u32 portid, int seq,
 				void *data, size_t datasz);
 extern int audit_list_rules_send(struct sk_buff *request_skb, int seq);
 
-extern u32 audit_enabled;
 #else /* CONFIG_AUDIT */
 static inline __printf(4, 5)
 void audit_log(struct audit_context *ctx, gfp_t gfp_mask, int type,
@@ -544,7 +549,6 @@ static inline int audit_log_task_context(struct audit_buffer *ab)
 static inline void audit_log_task_info(struct audit_buffer *ab,
 				       struct task_struct *tsk)
 { }
-#define audit_enabled 0
 #endif /* CONFIG_AUDIT */
 static inline void audit_log_string(struct audit_buffer *ab, const char *buf)
 {
-- 
2.1.4

Re: seccomp and audit_enabled

[Kees Cook]

On Mon, Oct 12, 2015 at 10:53 AM, Tony Jones <tonyj@suse.de> wrote:
> On 10/12/2015 08:40 AM, Paul Moore wrote:
>> My apologies for the resend, I had the wrong email for Kees.

(I keep asking for that alias, but no luck...)

>> On Monday, October 12, 2015 11:29:43 AM Paul Moore wrote:
>>> On Friday, October 09, 2015 08:50:01 PM Tony Jones wrote:
>>>> Hi.
>>>>
>>>> What is the expected handling of AUDIT_SECCOMP if audit_enabled == 0?
>>>> Opera browser makes use of a sandbox and if audit_enabled == 0 (and no
>>>> auditd is running) there is a lot of messages dumped to the klog. The fix
>>>> to __audit_seccomp() is trivial, similar to c2412d91c and I can send a
>>>> patch, I'm just not sure if seccomp is somehow special?
>>>
>>> I'm adding Kees to this since he looks after the seccomp kernel bits these
>>> days.  While there isn't anything special about seccomp from an audit
>>> perspective, the seccomp audit record can be a really nice thing as it is
>>> the only indication you may get that seccomp has stepped in and done
>>> "something" other than allow the syscall to progress normally.
>
> The issue is that (without auditd running) the messages are output to klog regardless
> of whether audit_enabled is 0 or 1.  As I said, other occurrences of this such as with
> login events has been corrected (c2412d91c). Attached patch does same for seccomp.
>
>>> I would be a little more concerned that you are seeing a flood of seccomp
>>> messages from Opera, that is something that most likely warrants some closer
>>> inspection.  Are all the records the same/similar?  Can you paste some into
>>> email?
>
> Here is the logged messages per invocation of opera.  the use of the sandbox may well
> be the result of a local suse config/packaging decision but I'm not sure that's relevant.
>
> 2015-10-10T19:35:23.237882-07:00 nohostname kernel: [  152.100348] audit: type=1326 audit(1444530923.236:356): auid=1000 uid=1000 gid=100 ses=1 pid=2048 comm="opera" exe="/usr/lib64/opera/opera" sig=0 arch=c000003e syscall=91 compat=0 ip=0x7ff926d94ab7 code=0x50000
> 2015-10-10T19:35:23.242867-07:00 nohostname kernel: [  152.105690] audit: type=1326 audit(1444530923.241:357): auid=1000 uid=1000 gid=100 ses=1 pid=2087 comm="opera" exe="/usr/lib64/opera/opera" sig=0 arch=c000003e syscall=273 compat=0 ip=0x7ff928325444 code=0x50000
> 2015-10-10T19:35:23.242873-07:00 nohostname kernel: [  152.105938] audit: type=1326 audit(1444530923.241:358): auid=1000 uid=1000 gid=100 ses=1 pid=2089 comm="opera" exe="/usr/lib64/opera/opera" sig=0 arch=c000003e syscall=273 compat=0 ip=0x7ff928325444 code=0x50000
> 2015-10-10T19:35:23.243890-07:00 nohostname kernel: [  152.106845] audit: type=1326 audit(1444530923.242:359): auid=1000 uid=1000 gid=100 ses=1 pid=2048 comm="opera" exe="/usr/lib64/opera/opera" sig=0 arch=c000003e syscall=2 compat=0 ip=0x7ff926d6daa1 code=0x30000
> 2015-10-10T19:35:23.275872-07:00 nohostname kernel: [  152.138819] audit: type=1326 audit(1444530923.273:360): auid=1000 uid=1000 gid=100 ses=1 pid=2093 comm="opera" exe="/usr/lib64/opera/opera" sig=0 arch=c000003e syscall=91 compat=0 ip=0x7f92e4bd7ab7 code=0x50000
> 2015-10-10T19:35:23.275885-07:00 nohostname kernel: [  152.138937] audit: type=1326 audit(1444530923.274:361): auid=1000 uid=1000 gid=100 ses=1 pid=2093 comm="opera" exe="/usr/lib64/opera/opera" sig=0 arch=c000003e syscall=91 compat=0 ip=0x7f92e4bd7ab7 code=0x50000
> 2015-10-10T19:35:23.280867-07:00 nohostname kernel: [  152.143147] audit: type=1326 audit(1444530923.279:362): auid=1000 uid=1000 gid=100 ses=1 pid=2096 comm="opera" exe="/usr/lib64/opera/opera" sig=0 arch=c000003e syscall=273 compat=0 ip=0x7f92e6168444 code=0x50000
> 2015-10-10T19:35:23.282055-07:00 nohostname kernel: [  152.144762] audit: type=1326 audit(1444530923.280:363): auid=1000 uid=1000 gid=100 ses=1 pid=2093 comm="opera" exe="/usr/lib64/opera/opera" sig=0 arch=c000003e syscall=2 compat=0 ip=0x7f92eb5f8587 code=0x50000
> 2015-10-10T19:35:23.282062-07:00 nohostname kernel: [  152.144890] audit: type=1326 audit(1444530923.280:364): auid=1000 uid=1000 gid=100 ses=1 pid=2093 comm="opera" exe="/usr/lib64/opera/opera" sig=0 arch=c000003e syscall=2 compat=0 ip=0x7f92e4b2ac8c code=0x50000
> 2015-10-10T19:35:23.282063-07:00 nohostname kernel: [  152.144988] audit: type=1326 audit(1444530923.280:365): auid=1000 uid=1000 gid=100 ses=1 pid=2093 comm="opera" exe="/usr/lib64/opera/opera" sig=0 arch=c000003e syscall=2 compat=0 ip=0x7f92e4b2ad70 code=0x50000
>
>
> thanks
>
> tony
>
>
> From d6971ec9508244f7a1ab42f9ac4c59b7e1ca6145 Mon Sep 17 00:00:00 2001
> From: Tony Jones <tonyj@suse.de>
> Date: Sat, 10 Oct 2015 19:30:49 -0700
> Subject: [PATCH] Don't log seccomp messages when audit is disabled
>
> Don't log seccomp messages when audit is disabled.

This is intentional since violation of a seccomp policy ought to
indicate a misbehaving program, and we want these to always be
presented to the system log, regardless of audit being enabled. (I'd
like to even produce system log entries when there is no CONFIG_AUDIT
too, but that's for the future.)

> Currently, each time the opera browser is executed 10 records similar to the
> following are output to klog (when !audit_enabled).
>
> 2015-10-10T19:35:23.237882-07:00 nohostname kernel: [  152.100348] audit: type=1326 audit(1444530923.236:356): auid=1000 uid=1000 gid=100 ses=1 pid=2048 comm="opera" exe="/usr/lib64/opera/opera" sig=0 arch=c000003e syscall=91 compat=0 ip=0x7ff926d94ab7 code=0x50000

However, these are all sig=0, which would indicate a skipped syscall,
which I wouldn't expect to get reported at all.

>
> Signed-off-by: Tony Jones <tonyj@suse.de>
> ---
>  include/linux/audit.h | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index b2abc99..8f70f3f 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -113,6 +113,12 @@ struct filename;
>
>  extern void audit_log_session_info(struct audit_buffer *ab);
>
> +#ifdef CONFIG_AUDIT
> +extern u32 audit_enabled;
> +#else
> +#define audit_enabled 0
> +#endif
> +
>  #ifdef CONFIG_AUDIT_COMPAT_GENERIC
>  #define audit_is_compat(arch)  (!((arch) & __AUDIT_ARCH_64BIT))
>  #else
> @@ -213,7 +219,7 @@ void audit_core_dumps(long signr);
>  static inline void audit_seccomp(unsigned long syscall, long signr, int code)
>  {
>         /* Force a record to be reported if a signal was delivered. */
> -       if (signr || unlikely(!audit_dummy_context()))

What is dummy_context part of this actually do? I don't think reports
should be made when signr == 0.

-Kees

> +       if (audit_enabled && (signr || unlikely(!audit_dummy_context())))
>                 __audit_seccomp(syscall, signr, code);
>  }
>
> @@ -498,7 +504,6 @@ extern int audit_rule_change(int type, __u32 portid, int seq,
>                                 void *data, size_t datasz);
>  extern int audit_list_rules_send(struct sk_buff *request_skb, int seq);
>
> -extern u32 audit_enabled;
>  #else /* CONFIG_AUDIT */
>  static inline __printf(4, 5)
>  void audit_log(struct audit_context *ctx, gfp_t gfp_mask, int type,
> @@ -544,7 +549,6 @@ static inline int audit_log_task_context(struct audit_buffer *ab)
>  static inline void audit_log_task_info(struct audit_buffer *ab,
>                                        struct task_struct *tsk)
>  { }
> -#define audit_enabled 0
>  #endif /* CONFIG_AUDIT */
>  static inline void audit_log_string(struct audit_buffer *ab, const char *buf)
>  {
> --
> 2.1.4
>
>
>



-- 
Kees Cook
Chrome OS Security

Re: seccomp and audit_enabled

[Paul Moore]

On Mon, Oct 12, 2015 at 4:45 PM, Kees Cook <keescook@chromium.org> wrote:
> On Mon, Oct 12, 2015 at 10:53 AM, Tony Jones <tonyj@suse.de> wrote:
>> From d6971ec9508244f7a1ab42f9ac4c59b7e1ca6145 Mon Sep 17 00:00:00 2001
>> From: Tony Jones <tonyj@suse.de>
>> Date: Sat, 10 Oct 2015 19:30:49 -0700
>> Subject: [PATCH] Don't log seccomp messages when audit is disabled
>>
>> Don't log seccomp messages when audit is disabled.
>
> This is intentional since violation of a seccomp policy ought to
> indicate a misbehaving program, and we want these to always be
> presented to the system log, regardless of audit being enabled. (I'd
> like to even produce system log entries when there is no CONFIG_AUDIT
> too, but that's for the future.)

I agree.  As I mentioned earlier these AUDIT_SECCOMP records are very handy.

>> diff --git a/include/linux/audit.h b/include/linux/audit.h
>> index b2abc99..8f70f3f 100644
>> --- a/include/linux/audit.h
>> +++ b/include/linux/audit.h
>> @@ -113,6 +113,12 @@ struct filename;
>>
>>  extern void audit_log_session_info(struct audit_buffer *ab);
>>
>> +#ifdef CONFIG_AUDIT
>> +extern u32 audit_enabled;
>> +#else
>> +#define audit_enabled 0
>> +#endif
>> +
>>  #ifdef CONFIG_AUDIT_COMPAT_GENERIC
>>  #define audit_is_compat(arch)  (!((arch) & __AUDIT_ARCH_64BIT))
>>  #else
>> @@ -213,7 +219,7 @@ void audit_core_dumps(long signr);
>>  static inline void audit_seccomp(unsigned long syscall, long signr, int code)
>>  {
>>         /* Force a record to be reported if a signal was delivered. */
>> -       if (signr || unlikely(!audit_dummy_context()))
>
> What is dummy_context part of this actually do? I don't think reports
> should be made when signr == 0.

The idea behind audit_dummy_context() is to skip auditing when there
are no audit rules configured, it's a performance tweak.  My guess is
that Tony's system loads some audit configuration at boot which
enables audit (the kernel starts with audit_enabled=0 ...) and loads a
few syscall filter rules which are enough to make
audit_dummy_context() return false.  Can you confirm that Tony?

As for logging seccomp actions when signr == 0, I personally think
that still might be useful as the normal behavior has been altered; I
tend to think any action != ALLOW is worth logging.  However, I'm open
to discussion on this if others feel strongly.

>> +       if (audit_enabled && (signr || unlikely(!audit_dummy_context())))
>>                 __audit_seccomp(syscall, signr, code);
>>  }

-- 
paul moore
www.paul-moore.com

Re: seccomp and audit_enabled

[Tony Jones]

On 10/13/2015 09:11 AM, Paul Moore wrote:
> On Mon, Oct 12, 2015 at 4:45 PM, Kees Cook <keescook@chromium.org> wrote:
>> On Mon, Oct 12, 2015 at 10:53 AM, Tony Jones <tonyj@suse.de> wrote:
>>> From d6971ec9508244f7a1ab42f9ac4c59b7e1ca6145 Mon Sep 17 00:00:00 2001
>>> From: Tony Jones <tonyj@suse.de>
>>> Date: Sat, 10 Oct 2015 19:30:49 -0700
>>> Subject: [PATCH] Don't log seccomp messages when audit is disabled
>>>
>>> Don't log seccomp messages when audit is disabled.
>>
>> This is intentional since violation of a seccomp policy ought to
>> indicate a misbehaving program, and we want these to always be
>> presented to the system log, regardless of audit being enabled. (I'd
>> like to even produce system log entries when there is no CONFIG_AUDIT
>> too, but that's for the future.)
> 
> I agree.  As I mentioned earlier these AUDIT_SECCOMP records are very handy.
> 
>>> diff --git a/include/linux/audit.h b/include/linux/audit.h
>>> index b2abc99..8f70f3f 100644
>>> --- a/include/linux/audit.h
>>> +++ b/include/linux/audit.h
>>> @@ -113,6 +113,12 @@ struct filename;
>>>
>>>  extern void audit_log_session_info(struct audit_buffer *ab);
>>>
>>> +#ifdef CONFIG_AUDIT
>>> +extern u32 audit_enabled;
>>> +#else
>>> +#define audit_enabled 0
>>> +#endif
>>> +
>>>  #ifdef CONFIG_AUDIT_COMPAT_GENERIC
>>>  #define audit_is_compat(arch)  (!((arch) & __AUDIT_ARCH_64BIT))
>>>  #else
>>> @@ -213,7 +219,7 @@ void audit_core_dumps(long signr);
>>>  static inline void audit_seccomp(unsigned long syscall, long signr, int code)
>>>  {
>>>         /* Force a record to be reported if a signal was delivered. */
>>> -       if (signr || unlikely(!audit_dummy_context()))
>>
>> What is dummy_context part of this actually do? I don't think reports
>> should be made when signr == 0.
> 
> The idea behind audit_dummy_context() is to skip auditing when there
> are no audit rules configured, it's a performance tweak.  My guess is
> that Tony's system loads some audit configuration at boot which
> enables audit (the kernel starts with audit_enabled=0 ...) and loads a
> few syscall filter rules which are enough to make
> audit_dummy_context() return false.  Can you confirm that Tony?

No, it's the default audit.rules (-D, -b320).   No actual rules loaded. 
Let me add some instrumentation and figure out what's going on.  auditd
is masked (via systemd) but systemd-journal seems to set audit_enabled=1 
during startup (at least on our systems).

> As for logging seccomp actions when signr == 0, I personally think
> that still might be useful as the normal behavior has been altered; I
> tend to think any action != ALLOW is worth logging.  However, I'm open
> to discussion on this if others feel strongly.
> 
>>> +       if (audit_enabled && (signr || unlikely(!audit_dummy_context())))
>>>                 __audit_seccomp(syscall, signr, code);
>>>  }

I'm of the opinion that nothing should get output (through the audit system) if 
audit_enabled == 0.  What you advocate calls for more than 2 possible states for 
audit_enabled or logging the information through another mechanism than audit.

Tony

Re: seccomp and audit_enabled

[Paul Moore]

On Tue, Oct 13, 2015 at 1:18 PM, Tony Jones <tonyj@suse.de> wrote:
> On 10/13/2015 09:11 AM, Paul Moore wrote:
>> On Mon, Oct 12, 2015 at 4:45 PM, Kees Cook <keescook@chromium.org> wrote:
>>> On Mon, Oct 12, 2015 at 10:53 AM, Tony Jones <tonyj@suse.de> wrote:
>>>> diff --git a/include/linux/audit.h b/include/linux/audit.h
>>>> index b2abc99..8f70f3f 100644
>>>> --- a/include/linux/audit.h
>>>> +++ b/include/linux/audit.h
>>>> @@ -113,6 +113,12 @@ struct filename;
>>>>
>>>>  extern void audit_log_session_info(struct audit_buffer *ab);
>>>>
>>>> +#ifdef CONFIG_AUDIT
>>>> +extern u32 audit_enabled;
>>>> +#else
>>>> +#define audit_enabled 0
>>>> +#endif
>>>> +
>>>>  #ifdef CONFIG_AUDIT_COMPAT_GENERIC
>>>>  #define audit_is_compat(arch)  (!((arch) & __AUDIT_ARCH_64BIT))
>>>>  #else
>>>> @@ -213,7 +219,7 @@ void audit_core_dumps(long signr);
>>>>  static inline void audit_seccomp(unsigned long syscall, long signr, int code)
>>>>  {
>>>>         /* Force a record to be reported if a signal was delivered. */
>>>> -       if (signr || unlikely(!audit_dummy_context()))
>>>
>>> What is dummy_context part of this actually do? I don't think reports
>>> should be made when signr == 0.
>>
>> The idea behind audit_dummy_context() is to skip auditing when there
>> are no audit rules configured, it's a performance tweak.  My guess is
>> that Tony's system loads some audit configuration at boot which
>> enables audit (the kernel starts with audit_enabled=0 ...) and loads a
>> few syscall filter rules which are enough to make
>> audit_dummy_context() return false.  Can you confirm that Tony?
>
> No, it's the default audit.rules (-D, -b320).   No actual rules loaded.
> Let me add some instrumentation and figure out what's going on.  auditd
> is masked (via systemd) but systemd-journal seems to set audit_enabled=1
> during startup (at least on our systems).

Yes, if systemd is involved it enables audit; we've had some
discussions with the systemd folks about fixing that, but they haven't
gone very far.  I'm still a little curious as to why
audit_dummy_context() is false in this case, but I haven't looked at
how systemd/auditctl start/config the system too closely.

>> As for logging seccomp actions when signr == 0, I personally think
>> that still might be useful as the normal behavior has been altered; I
>> tend to think any action != ALLOW is worth logging.  However, I'm open
>> to discussion on this if others feel strongly.
>>
>>>> +       if (audit_enabled && (signr || unlikely(!audit_dummy_context())))
>>>>                 __audit_seccomp(syscall, signr, code);
>>>>  }
>
> I'm of the opinion that nothing should get output (through the audit system) if
> audit_enabled == 0.  What you advocate calls for more than 2 possible states for
> audit_enabled or logging the information through another mechanism than audit.

I don't really care if it is audit or not (although we will need to
output something via audit if it is enabled to keep the CC crowd
happy); if you feel strongly that it isn't audit, we can just make it
a printk, that would work well with Kees' goals.  To me the important
point here is that we send a message when seccomp alters the behavior
of the syscall (action != ALLOW).

-- 
paul moore
www.paul-moore.com

Re: seccomp and audit_enabled

[Tony Jones]

On 10/13/2015 12:19 PM, Paul Moore wrote:

>> No, it's the default audit.rules (-D, -b320).   No actual rules loaded.
>> Let me add some instrumentation and figure out what's going on.  auditd
>> is masked (via systemd) but systemd-journal seems to set audit_enabled=1
>> during startup (at least on our systems).
> 
> Yes, if systemd is involved it enables audit; we've had some
> discussions with the systemd folks about fixing that, but they haven't
> gone very far.  I'm still a little curious as to why
> audit_dummy_context() is false in this case, but I haven't looked at
> how systemd/auditctl start/config the system too closely.

I'll debug what's going on (easy) on the test system and report back.  I'm curious
too.  Have a bad cold today so I'm moving slower than normal.

> I don't really care if it is audit or not (although we will need to
> output something via audit if it is enabled to keep the CC crowd
> happy); if you feel strongly that it isn't audit, we can just make it
> a printk, that would work well with Kees' goals.  To me the important
> point here is that we send a message when seccomp alters the behavior
> of the syscall (action != ALLOW).

Yes, if audit is enabled, you should totally be able to use it. Rest sounds good also.

thanks!

Tony

Re: seccomp and audit_enabled

[Steve Grubb]

> No, it's the default audit.rules (-D, -b320).   No actual rules loaded.
> Let me add some instrumentation and figure out what's going on.  auditd
> is masked (via systemd) but systemd-journal seems to set audit_enabled=1
> during startup (at least on our systems).

Tony,

We have bz 1227379
https://bugzilla.redhat.com/show_bug.cgi?id=1227379

There is a patch attached to disable systemd's propensity to turn on the audit 
system. Are people complaining and opening bugs in your distribution? If so, 
that might add more ammunition to get that fixed.


On Tuesday, October 13, 2015 03:19:20 PM Paul Moore wrote:
> > I'm of the opinion that nothing should get output (through the audit
> > system) if audit_enabled == 0.  What you advocate calls for more than 2
> > possible states for audit_enabled or logging the information through
> > another mechanism than audit.
> I don't really care if it is audit or not (although we will need to
> output something via audit if it is enabled to keep the CC crowd
> happy);

The rules for CC are that any access decision must be auditable and selective. 
That means that we need to be able to choose if we want to audit success, 
and/or failure, and/or nothing.

The inability to turn off SE Linux AVCs in the audit logs is why the exclude 
filter was created. Seccomp could be the same way.

-Steve

> if you feel strongly that it isn't audit, we can just make it
> a printk, that would work well with Kees' goals.  To me the important
> point here is that we send a message when seccomp alters the behavior
> of the syscall (action != ALLOW).

Re: seccomp and audit_enabled

[Tony Jones]

On 10/13/2015 12:19 PM, Paul Moore wrote:

> Yes, if systemd is involved it enables audit; we've had some
> discussions with the systemd folks about fixing that, but they haven't
> gone very far.  I'm still a little curious as to why
> audit_dummy_context() is false in this case, but I haven't looked at
> how systemd/auditctl start/config the system too closely.

Sorry for the delay here. 

A context is allocated by audit_alloc() because there is no uid/gid filter for the task
but the dummy flag is left false.  Because audit has been disabled (manually following systemd enabling), 
dummy never gets set in the syscall entry path (based on !audit_n_rules). So the unlikely(!audit_dummy_context())
in audit_seccomp succeeds.  

Tony

Re: seccomp and audit_enabled

[Tony Jones]

On 10/13/2015 01:03 PM, Steve Grubb wrote:
>> No, it's the default audit.rules (-D, -b320).   No actual rules loaded.
>> Let me add some instrumentation and figure out what's going on.  auditd
>> is masked (via systemd) but systemd-journal seems to set audit_enabled=1
>> during startup (at least on our systems).
> 
> Tony,
> 
> We have bz 1227379
> https://bugzilla.redhat.com/show_bug.cgi?id=1227379
> 
> There is a patch attached to disable systemd's propensity to turn on the audit 
> system. Are people complaining and opening bugs in your distribution? If so, 
> that might add more ammunition to get that fixed.

Hi Steve

we only have the one bug and it's related to:
1) noisy klog between when systemd enables audit and user manually disables it (rh bz#1160046)
2) after user manually disables audit (audit_enabled=0) seccomp messages still are output.

tony

Re: seccomp and audit_enabled

[Tony Jones]

On 11/06/2015 01:36 PM, Tony Jones wrote:
> On 10/13/2015 12:19 PM, Paul Moore wrote:
> 
>> Yes, if systemd is involved it enables audit; we've had some
>> discussions with the systemd folks about fixing that, but they haven't
>> gone very far.  I'm still a little curious as to why
>> audit_dummy_context() is false in this case, but I haven't looked at
>> how systemd/auditctl start/config the system too closely.
> 
> Sorry for the delay here. 
> 
> A context is allocated by audit_alloc() because there is no uid/gid filter for the task
> but the dummy flag is left false.  Because audit has been disabled (manually following systemd enabling), 
> dummy never gets set in the syscall entry path (based on !audit_n_rules). So the unlikely(!audit_dummy_context())
> in audit_seccomp succeeds.  
> 
> Tony

Any comments on this?  Current interaction between enabled_enabled and dummy flag seems wrong to me.   I can code up
a patch.

Tony

Re: seccomp and audit_enabled

[Paul Moore]

On Fri, Nov 20, 2015 at 12:51 PM, Tony Jones <tonyj@suse.de> wrote:
> Any comments on this?  Current interaction between enabled_enabled and dummy flag seems wrong to me.   I can code up
> a patch.

It's on my todo list for this development cycle, I've just been a
little busy lately with the merge window and now some -rc1 testing.

-- 
paul moore
www.paul-moore.com