seccomp and audit_enabled

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?

Quoted 7 lines · 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?

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?

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

Quoted 14 lines · Paul Moore

Quoted 7 lines · 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?

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.

Quoted 4 lines

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?

From d6971ec9508244f7a1ab42f9ac4c59b7e1ca6145 Mon Sep 17 00:00:00 2001

Quoted 1 line · Paul Moore

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

Quoted 10 lines · Paul Moore

Quoted 9 lines · Tony Jones

Quoted 7 lines

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?

Quoted 5 lines

Quoted 5 lines

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.

Quoted 4 lines

Quoted 4 lines

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

---
 include/linux/audit.h | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

include/linux/audit.h +6 -0

index b2abc99..8f70f3f 100644
@@ -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

include/linux/audit.h +1 -3

+       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

Quoted 5 lines · Tony Jones

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

Quoted 1 line

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.)

Quoted 17 lines

include/linux/audit.h +6 -0

index b2abc99..8f70f3f 100644
@@ -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

include/linux/audit.h void audit_core_dumps(long signr); · line 216

Quoted 5 lines

include/linux/audit.h +0 -1

@@ -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.

Quoted 3 lines

include/linux/audit.h +1 -0

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

Quoted 6 lines · Kees Cook

Quoted 5 lines · Tony Jones

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

Quoted 7 lines

Quoted 1 line

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.

Quoted 17 lines

Quoted 17 lines

include/linux/audit.h +6 -0

index b2abc99..8f70f3f 100644
@@ -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

Quoted 8 lines

include/linux/audit.h void audit_core_dumps(long signr); · line 216

Quoted 5 lines

include/linux/audit.h +0 -1

@@ -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.

Quoted 3 lines

Quoted 3 lines

include/linux/audit.h +1 -0

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

Quoted 28 lines · Paul Moore

Quoted 18 lines · Kees Cook

Quoted 17 lines · Tony Jones

include/linux/audit.h +6 -0

index b2abc99..8f70f3f 100644
@@ -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

Quoted 8 lines

include/linux/audit.h void audit_core_dumps(long signr); · line 216

Quoted 5 lines

include/linux/audit.h +0 -1

@@ -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.

Quoted 6 lines

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).

Quoted 5 lines

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.

Quoted 3 lines

Quoted 3 lines

include/linux/audit.h void audit_core_dumps(long signr); · line 224

Quoted 3 lines

include/linux/audit.h +1 -0

+       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.

Quoted 4 lines

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 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).

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).

Quoted 4 lines

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).

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.

Quoted 4 lines

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.

Quoted 5 lines · Paul Moore

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.