Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1549294 > unrolled thread

[PATCH 0/2] Begin auditing SECCOMP_RET_ERRNO return actions

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/2] Begin auditing SECCOMP_RET_ERRNO return actions Tyler Hicks <tyhicks@canonical.com> - 2017-01-02 18:00 +0100
    [PATCH 1/2] seccomp: Allow for auditing functionality specific to return actions Tyler Hicks <tyhicks@canonical.com> - 2017-01-02 18:00 +0100
    [PATCH 2/2] seccomp: Audit SECCOMP_RET_ERRNO actions with errno values Tyler Hicks <tyhicks@canonical.com> - 2017-01-02 18:00 +0100
      Re: [PATCH 2/2] seccomp: Audit SECCOMP_RET_ERRNO actions with errno values Steve Grubb <sgrubb@redhat.com> - 2017-01-02 18:30 +0100
        Re: [PATCH 2/2] seccomp: Audit SECCOMP_RET_ERRNO actions with errno  values Tyler Hicks <tyhicks@canonical.com> - 2017-01-02 18:50 +0100
          Re: [PATCH 2/2] seccomp: Audit SECCOMP_RET_ERRNO actions with errno values Steve Grubb <sgrubb@redhat.com> - 2017-01-02 20:00 +0100
            Re: [PATCH 2/2] seccomp: Audit SECCOMP_RET_ERRNO actions with errno values Paul Moore <paul@paul-moore.com> - 2017-01-03 00:00 +0100
    Re: [PATCH 0/2] Begin auditing SECCOMP_RET_ERRNO return actions Paul Moore <paul@paul-moore.com> - 2017-01-02 23:50 +0100
      Re: [PATCH 0/2] Begin auditing SECCOMP_RET_ERRNO return actions Andy Lutomirski <luto@amacapital.net> - 2017-01-03 07:00 +0100
        Re: [PATCH 0/2] Begin auditing SECCOMP_RET_ERRNO return actions Paul Moore <paul@paul-moore.com> - 2017-01-03 20:40 +0100
      Re: [PATCH 0/2] Begin auditing SECCOMP_RET_ERRNO return actions Tyler Hicks <tyhicks@canonical.com> - 2017-01-03 14:40 +0100
        Re: [PATCH 0/2] Begin auditing SECCOMP_RET_ERRNO return actions Paul Moore <paul@paul-moore.com> - 2017-01-03 21:10 +0100
          Re: [PATCH 0/2] Begin auditing SECCOMP_RET_ERRNO return actions Kees Cook <keescook@chromium.org> - 2017-01-03 21:50 +0100
            Re: [PATCH 0/2] Begin auditing SECCOMP_RET_ERRNO return actions Steve Grubb <sgrubb@redhat.com> - 2017-01-03 22:00 +0100
            Re: [PATCH 0/2] Begin auditing SECCOMP_RET_ERRNO return actions Paul Moore <paul@paul-moore.com> - 2017-01-03 22:10 +0100
              Re: [PATCH 0/2] Begin auditing SECCOMP_RET_ERRNO return actions Kees Cook <keescook@chromium.org> - 2017-01-03 22:10 +0100
                Re: [PATCH 0/2] Begin auditing SECCOMP_RET_ERRNO return actions Paul Moore <paul@paul-moore.com> - 2017-01-03 22:20 +0100
                  Re: [PATCH 0/2] Begin auditing SECCOMP_RET_ERRNO return actions Kees Cook <keescook@chromium.org> - 2017-01-03 22:30 +0100
                    Re: [PATCH 0/2] Begin auditing SECCOMP_RET_ERRNO return actions Paul Moore <paul@paul-moore.com> - 2017-01-03 22:40 +0100
                      Re: [PATCH 0/2] Begin auditing SECCOMP_RET_ERRNO return actions Kees Cook <keescook@chromium.org> - 2017-01-03 22:50 +0100
                        Re: [PATCH 0/2] Begin auditing SECCOMP_RET_ERRNO return actions Tyler Hicks <tyhicks@canonical.com> - 2017-01-04 03:20 +0100
                          Re: [PATCH 0/2] Begin auditing SECCOMP_RET_ERRNO return actions Richard Guy Briggs <rgb@redhat.com> - 2017-01-04 05:50 +0100
                            Re: [PATCH 0/2] Begin auditing SECCOMP_RET_ERRNO return actions Kees Cook <keescook@chromium.org> - 2017-01-04 07:40 +0100
          Re: [PATCH 0/2] Begin auditing SECCOMP_RET_ERRNO return actions Tyler Hicks <tyhicks@canonical.com> - 2017-01-04 03:10 +0100
    Re: [PATCH 0/2] Begin auditing SECCOMP_RET_ERRNO return actions Andy Lutomirski <luto@kernel.org> - 2017-01-03 07:00 +0100
      Re: [PATCH 0/2] Begin auditing SECCOMP_RET_ERRNO return actions Tyler Hicks <tyhicks@canonical.com> - 2017-01-03 15:00 +0100

Page 1 of 2  [1] 2  Next page →

#1549294 — [PATCH 0/2] Begin auditing SECCOMP_RET_ERRNO return actions

This patch set creates the basis for auditing information specific to a given
seccomp return action and then starts auditing SECCOMP_RET_ERRNO return
actions. The audit messages for SECCOMP_RET_ERRNO return actions include the
errno value that will be returned to userspace.

Tyler

[toc] | [next] | [standalone]

#1549298 — [PATCH 1/2] seccomp: Allow for auditing functionality specific to return actions

This patch introduces the concept of auditing formats that are specific
to the action specified in a filter's return value. Initially, only
SECCOMP_RET_KILL has an auditing message that differs from other return
actions because it specifies the signal that is to be sent.

This patch causes a small functional change in that "sig=0" is not
printed when auditing seccomp actions other than SECCOMP_RET_KILL.

Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
---
 include/linux/audit.h | 39 +++++++++++++++++++++++++++++++++------
 kernel/auditsc.c      | 19 +++++++++++++++----
 kernel/seccomp.c      |  6 +++---
 3 files changed, 51 insertions(+), 13 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index f51fca8d..8c588c3 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -85,6 +85,11 @@ struct audit_field {
 	u32				op;
 };
 
+struct audit_seccomp_info {
+	int		code;
+	long		signr;
+};
+
 extern int is_audit_feature_set(int which);
 
 extern int __init audit_register_class(int class, unsigned *list);
@@ -243,7 +248,8 @@ extern void __audit_file(const struct file *);
 extern void __audit_inode_child(struct inode *parent,
 				const struct dentry *dentry,
 				const unsigned char type);
-extern void __audit_seccomp(unsigned long syscall, long signr, int code);
+extern void __audit_seccomp(unsigned long syscall,
+			    struct audit_seccomp_info *info);
 extern void __audit_ptrace(struct task_struct *t);
 
 static inline bool audit_dummy_context(void)
@@ -313,14 +319,31 @@ 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)
+static inline void audit_seccomp_signal(unsigned long syscall, long signr,
+					int code)
 {
 	if (!audit_enabled)
 		return;
 
 	/* Force a record to be reported if a signal was delivered. */
-	if (signr || unlikely(!audit_dummy_context()))
-		__audit_seccomp(syscall, signr, code);
+	if (signr || unlikely(!audit_dummy_context())) {
+		struct audit_seccomp_info info = { .code = code,
+						   .signr = signr };
+
+		__audit_seccomp(syscall, &info);
+	}
+}
+
+static inline void audit_seccomp_common(unsigned long syscall, int code)
+{
+	if (!audit_enabled)
+		return;
+
+	if (code || unlikely(!audit_dummy_context())) {
+		struct audit_seccomp_info info = { .code = code };
+
+		__audit_seccomp(syscall, &info);
+	}
 }
 
 static inline void audit_ptrace(struct task_struct *t)
@@ -485,9 +508,13 @@ static inline void audit_inode_child(struct inode *parent,
 { }
 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,
+				   struct audit_seccomp_info *info);
+{ }
+static inline void audit_seccomp_signal(unsigned long syscall, long signr,
+					int code)
 { }
-static inline void audit_seccomp(unsigned long syscall, long signr, int code)
+static inline void audit_seccomp_common(unsigned long syscall, int code)
 { }
 static inline int auditsc_get_stamp(struct audit_context *ctx,
 			      struct timespec *t, unsigned int *serial)
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index cf1fa43..b3472f2 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -74,6 +74,7 @@
 #include <linux/string.h>
 #include <linux/uaccess.h>
 #include <uapi/linux/limits.h>
+#include <uapi/linux/seccomp.h>
 
 #include "audit.h"
 
@@ -2415,7 +2416,7 @@ void audit_core_dumps(long signr)
 	audit_log_end(ab);
 }
 
-void __audit_seccomp(unsigned long syscall, long signr, int code)
+void __audit_seccomp(unsigned long syscall, struct audit_seccomp_info *info)
 {
 	struct audit_buffer *ab;
 
@@ -2423,9 +2424,19 @@ void __audit_seccomp(unsigned long syscall, long signr, int code)
 	if (unlikely(!ab))
 		return;
 	audit_log_task(ab);
-	audit_log_format(ab, " sig=%ld arch=%x syscall=%ld compat=%d ip=0x%lx code=0x%x",
-			 signr, syscall_get_arch(), syscall,
-			 in_compat_syscall(), KSTK_EIP(current), code);
+
+	switch (info->code) {
+	case SECCOMP_RET_KILL:
+		audit_log_format(ab, " sig=%ld", info->signr);
+		break;
+	default:
+		break;
+	}
+
+	audit_log_format(ab,
+			 " arch=%x syscall=%ld compat=%d ip=0x%lx code=0x%x",
+			 syscall_get_arch(), syscall, in_compat_syscall(),
+			 KSTK_EIP(current), info->code);
 	audit_log_end(ab);
 }
 
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index f7ce79a..54c01b6 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -532,7 +532,7 @@ static void __secure_computing_strict(int this_syscall)
 #ifdef SECCOMP_DEBUG
 	dump_stack();
 #endif
-	audit_seccomp(this_syscall, SIGKILL, SECCOMP_RET_KILL);
+	audit_seccomp_signal(this_syscall, SIGKILL, SECCOMP_RET_KILL);
 	do_exit(SIGKILL);
 }
 
@@ -635,14 +635,14 @@ static int __seccomp_filter(int this_syscall, const struct seccomp_data *sd,
 
 	case SECCOMP_RET_KILL:
 	default:
-		audit_seccomp(this_syscall, SIGSYS, action);
+		audit_seccomp_signal(this_syscall, SIGSYS, action);
 		do_exit(SIGSYS);
 	}
 
 	unreachable();
 
 skip:
-	audit_seccomp(this_syscall, 0, action);
+	audit_seccomp_common(this_syscall, action);
 	return -1;
 }
 #else
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1549299 — [PATCH 2/2] seccomp: Audit SECCOMP_RET_ERRNO actions with errno values

Generate audit records for SECCOMP_RET_ERRNO actions, which were
previously not audited.

Additionally, include the errno value that will be set in the audit
message.

Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
---
 include/linux/audit.h | 19 ++++++++++++++++++-
 kernel/auditsc.c      |  3 +++
 kernel/seccomp.c      |  4 +++-
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index 8c588c3..6815812 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -87,7 +87,10 @@ struct audit_field {
 
 struct audit_seccomp_info {
 	int		code;
-	long		signr;
+	union {
+		int	errno;
+		long	signr;
+	};
 };
 
 extern int is_audit_feature_set(int which);
@@ -319,6 +322,20 @@ static inline void audit_inode_child(struct inode *parent,
 }
 void audit_core_dumps(long signr);
 
+static inline void audit_seccomp_errno(unsigned long syscall, int errno,
+				       int code)
+{
+	if (!audit_enabled)
+		return;
+
+	if (errno || unlikely(!audit_dummy_context())) {
+		struct audit_seccomp_info info = { .code = code,
+						   .errno = errno };
+
+		__audit_seccomp(syscall, &info);
+	}
+}
+
 static inline void audit_seccomp_signal(unsigned long syscall, long signr,
 					int code)
 {
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index b3472f2..db5fc9d 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -2426,6 +2426,9 @@ void __audit_seccomp(unsigned long syscall, struct audit_seccomp_info *info)
 	audit_log_task(ab);
 
 	switch (info->code) {
+	case SECCOMP_RET_ERRNO:
+		audit_log_format(ab, " errno=%d", info->errno);
+		break;
 	case SECCOMP_RET_KILL:
 		audit_log_format(ab, " sig=%ld", info->signr);
 		break;
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index 54c01b6..e99c566 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -576,9 +576,11 @@ static int __seccomp_filter(int this_syscall, const struct seccomp_data *sd,
 		/* Set low-order bits as an errno, capped at MAX_ERRNO. */
 		if (data > MAX_ERRNO)
 			data = MAX_ERRNO;
+
+		audit_seccomp_errno(this_syscall, data, action);
 		syscall_set_return_value(current, task_pt_regs(current),
 					 -data, 0);
-		goto skip;
+		return -1;
 
 	case SECCOMP_RET_TRAP:
 		/* Show the handler the original registers. */
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1549319 — Re: [PATCH 2/2] seccomp: Audit SECCOMP_RET_ERRNO actions with errno values

On Monday, January 2, 2017 4:53:10 PM EST Tyler Hicks wrote:
> Generate audit records for SECCOMP_RET_ERRNO actions, which were
> previously not audited.
> 
> Additionally, include the errno value that will be set in the audit
> message.
> 
> Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
> ---
>  include/linux/audit.h | 19 ++++++++++++++++++-
>  kernel/auditsc.c      |  3 +++
>  kernel/seccomp.c      |  4 +++-
>  3 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 8c588c3..6815812 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -87,7 +87,10 @@ struct audit_field {
> 
>  struct audit_seccomp_info {
>  	int		code;
> -	long		signr;
> +	union {
> +		int	errno;
> +		long	signr;
> +	};
>  };
> 
>  extern int is_audit_feature_set(int which);
> @@ -319,6 +322,20 @@ static inline void audit_inode_child(struct inode
> *parent, }
>  void audit_core_dumps(long signr);
> 
> +static inline void audit_seccomp_errno(unsigned long syscall, int errno,
> +				       int code)
> +{
> +	if (!audit_enabled)
> +		return;
> +
> +	if (errno || unlikely(!audit_dummy_context())) {
> +		struct audit_seccomp_info info = { .code = code,
> +						   .errno = errno };
> +
> +		__audit_seccomp(syscall, &info);
> +	}
> +}
> +
>  static inline void audit_seccomp_signal(unsigned long syscall, long signr,
>  					int code)
>  {
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index b3472f2..db5fc9d 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -2426,6 +2426,9 @@ void __audit_seccomp(unsigned long syscall, struct
> audit_seccomp_info *info) audit_log_task(ab);
> 
>  	switch (info->code) {
> +	case SECCOMP_RET_ERRNO:
> +		audit_log_format(ab, " errno=%d", info->errno);
> +		break;

"exit" is the field name that syscalls use to return errno to user space. I'd
rather not see another field created that maps to the same thing. You can check
the translation with the auformat utility:

http://people.redhat.com/sgrubb/files/auformat.tar.gz

$ ausearch --start today --just-one -m syscall -sv no --raw | ./auformat "%EXIT\n"

Also, I am working to normalize all the records. That mean every event record
of the same type has the same fields, in the same order, with the same
representation. I would think "exit" could be added to the current record after
syscall so that its ordered similarly to a syscall record.

-Steve

>  	case SECCOMP_RET_KILL:
>  		audit_log_format(ab, " sig=%ld", info->signr);
>  		break;
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index 54c01b6..e99c566 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -576,9 +576,11 @@ static int __seccomp_filter(int this_syscall, const
> struct seccomp_data *sd, /* Set low-order bits as an errno, capped at
> MAX_ERRNO. */
>  		if (data > MAX_ERRNO)
>  			data = MAX_ERRNO;
> +
> +		audit_seccomp_errno(this_syscall, data, action);
>  		syscall_set_return_value(current, task_pt_regs(current),
>  					 -data, 0);
> -		goto skip;
> +		return -1;
> 
>  	case SECCOMP_RET_TRAP:
>  		/* Show the handler the original registers. */

[toc] | [prev] | [next] | [standalone]

#1549326 — Re: [PATCH 2/2] seccomp: Audit SECCOMP_RET_ERRNO actions with errno values

[Multipart message — attachments visible in raw view] — view raw

On 2017-01-02 12:20:53, Steve Grubb wrote:
> On Monday, January 2, 2017 4:53:10 PM EST Tyler Hicks wrote:
> > Generate audit records for SECCOMP_RET_ERRNO actions, which were
> > previously not audited.
> > 
> > Additionally, include the errno value that will be set in the audit
> > message.
> > 
> > Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
> > ---
> >  include/linux/audit.h | 19 ++++++++++++++++++-
> >  kernel/auditsc.c      |  3 +++
> >  kernel/seccomp.c      |  4 +++-
> >  3 files changed, 24 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > index 8c588c3..6815812 100644
> > --- a/include/linux/audit.h
> > +++ b/include/linux/audit.h
> > @@ -87,7 +87,10 @@ struct audit_field {
> > 
> >  struct audit_seccomp_info {
> >  	int		code;
> > -	long		signr;
> > +	union {
> > +		int	errno;
> > +		long	signr;
> > +	};
> >  };
> > 
> >  extern int is_audit_feature_set(int which);
> > @@ -319,6 +322,20 @@ static inline void audit_inode_child(struct inode
> > *parent, }
> >  void audit_core_dumps(long signr);
> > 
> > +static inline void audit_seccomp_errno(unsigned long syscall, int errno,
> > +				       int code)
> > +{
> > +	if (!audit_enabled)
> > +		return;
> > +
> > +	if (errno || unlikely(!audit_dummy_context())) {
> > +		struct audit_seccomp_info info = { .code = code,
> > +						   .errno = errno };
> > +
> > +		__audit_seccomp(syscall, &info);
> > +	}
> > +}
> > +
> >  static inline void audit_seccomp_signal(unsigned long syscall, long signr,
> >  					int code)
> >  {
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index b3472f2..db5fc9d 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -2426,6 +2426,9 @@ void __audit_seccomp(unsigned long syscall, struct
> > audit_seccomp_info *info) audit_log_task(ab);
> > 
> >  	switch (info->code) {
> > +	case SECCOMP_RET_ERRNO:
> > +		audit_log_format(ab, " errno=%d", info->errno);
> > +		break;
> 
> "exit" is the field name that syscalls use to return errno to user space. I'd
> rather not see another field created that maps to the same thing. You can check
> the translation with the auformat utility:

Thanks for having a look at the field name I was using. Although I
prefer "errno" over "exit" in terms of clarity, I agree that it makes
sense to be consistent with the field names across record types. "exit"
works for me.

> 
> http://people.redhat.com/sgrubb/files/auformat.tar.gz
> 
> $ ausearch --start today --just-one -m syscall -sv no --raw | ./auformat "%EXIT\n"
> 
> Also, I am working to normalize all the records. That mean every event record
> of the same type has the same fields, in the same order, with the same
> representation. I would think "exit" could be added to the current record after
> syscall so that its ordered similarly to a syscall record.

This patch goes against your normalization efforts in more ways than
just the placement of the "exit" field. If the action is
SECCOMP_RET_KILL, a "sig" field is present but if the action is
SECCOMP_RET_ERRNO, the "sig" field will not be present but the "errno"
field will be present. This happens all within the AUDIT_SECCOMP record
type. How would you suggest normalizing AUDIT_SECCOMP records for
different seccomp return actions?

Tyler

> 
> -Steve
> 
> >  	case SECCOMP_RET_KILL:
> >  		audit_log_format(ab, " sig=%ld", info->signr);
> >  		break;
> > diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> > index 54c01b6..e99c566 100644
> > --- a/kernel/seccomp.c
> > +++ b/kernel/seccomp.c
> > @@ -576,9 +576,11 @@ static int __seccomp_filter(int this_syscall, const
> > struct seccomp_data *sd, /* Set low-order bits as an errno, capped at
> > MAX_ERRNO. */
> >  		if (data > MAX_ERRNO)
> >  			data = MAX_ERRNO;
> > +
> > +		audit_seccomp_errno(this_syscall, data, action);
> >  		syscall_set_return_value(current, task_pt_regs(current),
> >  					 -data, 0);
> > -		goto skip;
> > +		return -1;
> > 
> >  	case SECCOMP_RET_TRAP:
> >  		/* Show the handler the original registers. */
> 
> 

[toc] | [prev] | [next] | [standalone]

#1549360 — Re: [PATCH 2/2] seccomp: Audit SECCOMP_RET_ERRNO actions with errno values

On Monday, January 2, 2017 5:42:47 PM EST Tyler Hicks wrote:
> On 2017-01-02 12:20:53, Steve Grubb wrote:
> > On Monday, January 2, 2017 4:53:10 PM EST Tyler Hicks wrote:
> > > Generate audit records for SECCOMP_RET_ERRNO actions, which were
> > > previously not audited.
> > > 
> > > Additionally, include the errno value that will be set in the audit
> > > message.
> > > 
> > > Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
> > > ---
> > > 
> > >  include/linux/audit.h | 19 ++++++++++++++++++-
> > >  kernel/auditsc.c      |  3 +++
> > >  kernel/seccomp.c      |  4 +++-
> > >  3 files changed, 24 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > > index 8c588c3..6815812 100644
> > > --- a/include/linux/audit.h
> > > +++ b/include/linux/audit.h
> > > @@ -87,7 +87,10 @@ struct audit_field {
> > > 
> > >  struct audit_seccomp_info {
> > >  
> > >  	int		code;
> > > 
> > > -	long		signr;
> > > +	union {
> > > +		int	errno;
> > > +		long	signr;
> > > +	};
> > > 
> > >  };
> > >  
> > >  extern int is_audit_feature_set(int which);
> > > 
> > > @@ -319,6 +322,20 @@ static inline void audit_inode_child(struct inode
> > > *parent, }
> > > 
> > >  void audit_core_dumps(long signr);
> > > 
> > > +static inline void audit_seccomp_errno(unsigned long syscall, int
> > > errno,
> > > +				       int code)
> > > +{
> > > +	if (!audit_enabled)
> > > +		return;
> > > +
> > > +	if (errno || unlikely(!audit_dummy_context())) {
> > > +		struct audit_seccomp_info info = { .code = code,
> > > +						   .errno = errno };
> > > +
> > > +		__audit_seccomp(syscall, &info);
> > > +	}
> > > +}
> > > +
> > > 
> > >  static inline void audit_seccomp_signal(unsigned long syscall, long
> > >  signr,
> > >  
> > >  					int code)
> > >  
> > >  {
> > > 
> > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > > index b3472f2..db5fc9d 100644
> > > --- a/kernel/auditsc.c
> > > +++ b/kernel/auditsc.c
> > > @@ -2426,6 +2426,9 @@ void __audit_seccomp(unsigned long syscall, struct
> > > audit_seccomp_info *info) audit_log_task(ab);
> > > 
> > >  	switch (info->code) {
> > > 
> > > +	case SECCOMP_RET_ERRNO:
> > > +		audit_log_format(ab, " errno=%d", info->errno);
> > > +		break;
> > 
> > "exit" is the field name that syscalls use to return errno to user space.
> > I'd rather not see another field created that maps to the same thing. You
> > can check
> > the translation with the auformat utility:
> Thanks for having a look at the field name I was using. Although I
> prefer "errno" over "exit" in terms of clarity, I agree that it makes
> sense to be consistent with the field names across record types. "exit"
> works for me.
> 
> > http://people.redhat.com/sgrubb/files/auformat.tar.gz
> > 
> > $ ausearch --start today --just-one -m syscall -sv no --raw | ./auformat
> > "%EXIT\n"
> > 
> > Also, I am working to normalize all the records. That mean every event
> > record of the same type has the same fields, in the same order, with the
> > same representation. I would think "exit" could be added to the current
> > record after syscall so that its ordered similarly to a syscall record.
> 
> This patch goes against your normalization efforts in more ways than
> just the placement of the "exit" field. If the action is
> SECCOMP_RET_KILL, a "sig" field is present but if the action is
> SECCOMP_RET_ERRNO, the "sig" field will not be present but the "errno"
> field will be present. This happens all within the AUDIT_SECCOMP record
> type. How would you suggest normalizing AUDIT_SECCOMP records for
> different seccomp return actions?

Typically when the layout has to change, we just give it a new record type.

 -Steve

 
> > >  	case SECCOMP_RET_KILL:
> > >  		audit_log_format(ab, " sig=%ld", info->signr);
> > >  		break;
> > > 
> > > diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> > > index 54c01b6..e99c566 100644
> > > --- a/kernel/seccomp.c
> > > +++ b/kernel/seccomp.c
> > > @@ -576,9 +576,11 @@ static int __seccomp_filter(int this_syscall, const
> > > struct seccomp_data *sd, /* Set low-order bits as an errno, capped at
> > > MAX_ERRNO. */
> > > 
> > >  		if (data > MAX_ERRNO)
> > >  		
> > >  			data = MAX_ERRNO;
> > > 
> > > +
> > > +		audit_seccomp_errno(this_syscall, data, action);
> > > 
> > >  		syscall_set_return_value(current, task_pt_regs(current),
> > >  		
> > >  					 -data, 0);
> > > 
> > > -		goto skip;
> > > +		return -1;
> > > 
> > >  	case SECCOMP_RET_TRAP:
> > >  		/* Show the handler the original registers. */

[toc] | [prev] | [next] | [standalone]

#1549442 — Re: [PATCH 2/2] seccomp: Audit SECCOMP_RET_ERRNO actions with errno values

On Mon, Jan 2, 2017 at 1:49 PM, Steve Grubb <sgrubb@redhat.com> wrote:
> On Monday, January 2, 2017 5:42:47 PM EST Tyler Hicks wrote:
>> On 2017-01-02 12:20:53, Steve Grubb wrote:
>> > On Monday, January 2, 2017 4:53:10 PM EST Tyler Hicks wrote:

...

>> Thanks for having a look at the field name I was using. Although I
>> prefer "errno" over "exit" in terms of clarity, I agree that it makes
>> sense to be consistent with the field names across record types. "exit"
>> works for me.

FWIW, we have a nice (searchable due to GitHub CSV magic) audit field
database at the link below.  I will admit that it may be a bit crusty
in places, but we are making a new effort to keep it updated, if you
notice anything wrong, send email and/or a PR.

* https://github.com/linux-audit/audit-documentation/blob/master/specs/fields/field-dictionary.csv

>> > http://people.redhat.com/sgrubb/files/auformat.tar.gz
>> >
>> > $ ausearch --start today --just-one -m syscall -sv no --raw | ./auformat
>> > "%EXIT\n"
>> >
>> > Also, I am working to normalize all the records. That mean every event
>> > record of the same type has the same fields, in the same order, with the
>> > same representation. I would think "exit" could be added to the current
>> > record after syscall so that its ordered similarly to a syscall record.
>>
>> This patch goes against your normalization efforts in more ways than
>> just the placement of the "exit" field. If the action is
>> SECCOMP_RET_KILL, a "sig" field is present but if the action is
>> SECCOMP_RET_ERRNO, the "sig" field will not be present but the "errno"
>> field will be present. This happens all within the AUDIT_SECCOMP record
>> type. How would you suggest normalizing AUDIT_SECCOMP records for
>> different seccomp return actions?
>
> Typically when the layout has to change, we just give it a new record type.

I'm going to be very loathe to accept any new record types that *only*
reorder fields; if you need to add a new field, simply add it to the
end of the record.  From my perspective new record types are really
only an option if we need to remove a field that is bogus/confusing or
some other similar case that is not easily solved.  New record types
are a last resort.

-- 
paul moore
www.paul-moore.com

[toc] | [prev] | [next] | [standalone]

#1549437

On Mon, Jan 2, 2017 at 11:53 AM, Tyler Hicks <tyhicks@canonical.com> wrote:
> This patch set creates the basis for auditing information specific to a given
> seccomp return action and then starts auditing SECCOMP_RET_ERRNO return
> actions. The audit messages for SECCOMP_RET_ERRNO return actions include the
> errno value that will be returned to userspace.

I'm replying to this patchset posting because it his my inbox first,
but my comments here apply to both this patchset and the other
seccomp/audit patchset you posted.

In my experience, we have two or three problems (the count varies
depending on perspective) when it comes to seccomp filter reporting:

1. Inability to log all filter actions.
2. Inability to selectively enable filtering; e.g. devs want noisy
logging, users want relative quiet.
3. Consistent behavior with audit enabled and disabled.

My current thinking - forgive me, this has been kicking around in my
head for the better part of six months (longer?) and I haven't
attempted to code it up - is to create a sysctl knob for a system wide
seccomp logging threshold that would be applied to the high 16-bits of
*every* triggered action: if the action was at/below the threshold a
record would be emitted, otherwise silence.  This should resolve
problems #1 and #2, and the code should be relatively straightforward
and small.

As part of the code above, I expect that all seccomp logging would get
routed through a single logging function (sort of like a better
implementation of the existing audit_seccomp()) that would check the
threshold and trigger the logging if needed.  This function could be
augmented to check for CONFIG_AUDIT and in the case where audit was
not built into the kernel, a simple printk could be used to log the
seccomp event; solving problem #3.

We could also add a SECCOMP_RET_AUDIT, or similar, if we still feel
that is important (I personally waffle on this), but I think that is
independent of the ideas above.

Thoughts?

-- 
paul moore
www.paul-moore.com

[toc] | [prev] | [next] | [standalone]

#1549546

On Mon, Jan 2, 2017 at 2:47 PM, Paul Moore <paul@paul-moore.com> wrote:
> On Mon, Jan 2, 2017 at 11:53 AM, Tyler Hicks <tyhicks@canonical.com> wrote:
>> This patch set creates the basis for auditing information specific to a given
>> seccomp return action and then starts auditing SECCOMP_RET_ERRNO return
>> actions. The audit messages for SECCOMP_RET_ERRNO return actions include the
>> errno value that will be returned to userspace.
>
> I'm replying to this patchset posting because it his my inbox first,
> but my comments here apply to both this patchset and the other
> seccomp/audit patchset you posted.
>
> In my experience, we have two or three problems (the count varies
> depending on perspective) when it comes to seccomp filter reporting:
>
> 1. Inability to log all filter actions.
> 2. Inability to selectively enable filtering; e.g. devs want noisy
> logging, users want relative quiet.
> 3. Consistent behavior with audit enabled and disabled.
>
> My current thinking - forgive me, this has been kicking around in my
> head for the better part of six months (longer?) and I haven't
> attempted to code it up - is to create a sysctl knob for a system wide
> seccomp logging threshold that would be applied to the high 16-bits of
> *every* triggered action: if the action was at/below the threshold a
> record would be emitted, otherwise silence.  This should resolve
> problems #1 and #2, and the code should be relatively straightforward
> and small.
>
> As part of the code above, I expect that all seccomp logging would get
> routed through a single logging function (sort of like a better
> implementation of the existing audit_seccomp()) that would check the
> threshold and trigger the logging if needed.  This function could be
> augmented to check for CONFIG_AUDIT and in the case where audit was
> not built into the kernel, a simple printk could be used to log the
> seccomp event; solving problem #3.

Would this not be doable with a seccomp tracepoint and a BPF filter?

--Andy

[toc] | [prev] | [next] | [standalone]

#1550118

On Tue, Jan 3, 2017 at 12:56 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Mon, Jan 2, 2017 at 2:47 PM, Paul Moore <paul@paul-moore.com> wrote:
>> On Mon, Jan 2, 2017 at 11:53 AM, Tyler Hicks <tyhicks@canonical.com> wrote:
>>> This patch set creates the basis for auditing information specific to a given
>>> seccomp return action and then starts auditing SECCOMP_RET_ERRNO return
>>> actions. The audit messages for SECCOMP_RET_ERRNO return actions include the
>>> errno value that will be returned to userspace.
>>
>> I'm replying to this patchset posting because it his my inbox first,
>> but my comments here apply to both this patchset and the other
>> seccomp/audit patchset you posted.
>>
>> In my experience, we have two or three problems (the count varies
>> depending on perspective) when it comes to seccomp filter reporting:
>>
>> 1. Inability to log all filter actions.
>> 2. Inability to selectively enable filtering; e.g. devs want noisy
>> logging, users want relative quiet.
>> 3. Consistent behavior with audit enabled and disabled.
>>
>> My current thinking - forgive me, this has been kicking around in my
>> head for the better part of six months (longer?) and I haven't
>> attempted to code it up - is to create a sysctl knob for a system wide
>> seccomp logging threshold that would be applied to the high 16-bits of
>> *every* triggered action: if the action was at/below the threshold a
>> record would be emitted, otherwise silence.  This should resolve
>> problems #1 and #2, and the code should be relatively straightforward
>> and small.
>>
>> As part of the code above, I expect that all seccomp logging would get
>> routed through a single logging function (sort of like a better
>> implementation of the existing audit_seccomp()) that would check the
>> threshold and trigger the logging if needed.  This function could be
>> augmented to check for CONFIG_AUDIT and in the case where audit was
>> not built into the kernel, a simple printk could be used to log the
>> seccomp event; solving problem #3.
>
> Would this not be doable with a seccomp tracepoint and a BPF filter?

One of the motivations for the above idea is to make it easier for
admins/users to customize the seccomp logging on their own systems,
it's not just to make devs lives easier.  I feel okay providing
guidance to an admin/user the involves setting a sysctl variable, I
can't say the same about asking them to write their own BPF ;)

-- 
paul moore
www.paul-moore.com

[toc] | [prev] | [next] | [standalone]

#1549777

[Multipart message — attachments visible in raw view] — view raw

On 01/02/2017 04:47 PM, Paul Moore wrote:
> On Mon, Jan 2, 2017 at 11:53 AM, Tyler Hicks <tyhicks@canonical.com> wrote:
>> This patch set creates the basis for auditing information specific to a given
>> seccomp return action and then starts auditing SECCOMP_RET_ERRNO return
>> actions. The audit messages for SECCOMP_RET_ERRNO return actions include the
>> errno value that will be returned to userspace.
> 
> I'm replying to this patchset posting because it his my inbox first,
> but my comments here apply to both this patchset and the other
> seccomp/audit patchset you posted.
> 
> In my experience, we have two or three problems (the count varies
> depending on perspective) when it comes to seccomp filter reporting:
> 
> 1. Inability to log all filter actions.
> 2. Inability to selectively enable filtering; e.g. devs want noisy
> logging, users want relative quiet.
> 3. Consistent behavior with audit enabled and disabled.

Agreed. Those three logging issues are what have been nagging me the most.

> My current thinking - forgive me, this has been kicking around in my
> head for the better part of six months (longer?) and I haven't
> attempted to code it up - is to create a sysctl knob for a system wide
> seccomp logging threshold that would be applied to the high 16-bits of
> *every* triggered action: if the action was at/below the threshold a
> record would be emitted, otherwise silence.  This should resolve
> problems #1 and #2, and the code should be relatively straightforward
> and small.

I like that idea quite a bit. To be completely honest, for #1, I
personally only care about logging SECCOMP_RET_ERRNO actions but this
idea solves it in a nice and general way.

> As part of the code above, I expect that all seccomp logging would get
> routed through a single logging function (sort of like a better
> implementation of the existing audit_seccomp()) that would check the
> threshold and trigger the logging if needed.  This function could be
> augmented to check for CONFIG_AUDIT and in the case where audit was
> not built into the kernel, a simple printk could be used to log the
> seccomp event; solving problem #3.

That doesn't fully solve #3 for me. In Ubuntu (and I think Debian), we
build with CONFIG_AUDIT enabled but don't ship auditd by default so
audit_enabled is false. In that default configuration, we still want
seccomp audit messages to be printk'ed. I'll need to figure out how to
cleanly allow opting into seccomp audit messages when CONFIG_AUDIT is
enabled and audit_enabled is false.

> We could also add a SECCOMP_RET_AUDIT, or similar, if we still feel
> that is important (I personally waffle on this), but I think that is
> independent of the ideas above.

I agree that it is independent but SECCOMP_RET_AUDIT would still be
important to Ubuntu.

Tyler

[toc] | [prev] | [next] | [standalone]

#1550149

On Tue, Jan 3, 2017 at 8:31 AM, Tyler Hicks <tyhicks@canonical.com> wrote:
> On 01/02/2017 04:47 PM, Paul Moore wrote:
>> On Mon, Jan 2, 2017 at 11:53 AM, Tyler Hicks <tyhicks@canonical.com> wrote:
>>> This patch set creates the basis for auditing information specific to a given
>>> seccomp return action and then starts auditing SECCOMP_RET_ERRNO return
>>> actions. The audit messages for SECCOMP_RET_ERRNO return actions include the
>>> errno value that will be returned to userspace.
>>
>> I'm replying to this patchset posting because it his my inbox first,
>> but my comments here apply to both this patchset and the other
>> seccomp/audit patchset you posted.
>>
>> In my experience, we have two or three problems (the count varies
>> depending on perspective) when it comes to seccomp filter reporting:
>>
>> 1. Inability to log all filter actions.
>> 2. Inability to selectively enable filtering; e.g. devs want noisy
>> logging, users want relative quiet.
>> 3. Consistent behavior with audit enabled and disabled.
>
> Agreed. Those three logging issues are what have been nagging me the most.

/me nods

>> My current thinking - forgive me, this has been kicking around in my
>> head for the better part of six months (longer?) and I haven't
>> attempted to code it up - is to create a sysctl knob for a system wide
>> seccomp logging threshold that would be applied to the high 16-bits of
>> *every* triggered action: if the action was at/below the threshold a
>> record would be emitted, otherwise silence.  This should resolve
>> problems #1 and #2, and the code should be relatively straightforward
>> and small.
>
> I like that idea quite a bit. To be completely honest, for #1, I
> personally only care about logging SECCOMP_RET_ERRNO actions but this
> idea solves it in a nice and general way.

Yeah, I'd much rather solve this problem generally; everybody has
their favorite action and I'd like to avoid solving the same problem
multiple times.

Sooo ... you want to take a whack at coding this up? ;)

>> As part of the code above, I expect that all seccomp logging would get
>> routed through a single logging function (sort of like a better
>> implementation of the existing audit_seccomp()) that would check the
>> threshold and trigger the logging if needed.  This function could be
>> augmented to check for CONFIG_AUDIT and in the case where audit was
>> not built into the kernel, a simple printk could be used to log the
>> seccomp event; solving problem #3.
>
> That doesn't fully solve #3 for me. In Ubuntu (and I think Debian), we
> build with CONFIG_AUDIT enabled but don't ship auditd by default so
> audit_enabled is false. In that default configuration, we still want
> seccomp audit messages to be printk'ed. I'll need to figure out how to
> cleanly allow opting into seccomp audit messages when CONFIG_AUDIT is
> enabled and audit_enabled is false.

Heh, so you've got audit built into the kernel but you're not using
it; that sounds "fun".

Anyway, I think the logging consolidation could still help you, if for
no other reason than everything is going through the same function at
that point.  We could do some other stuff there to handle the case
where audit is compiled, but auditd is not running ... we already have
some code in place to handle that for other reasons, check
kernel/audit.c for more information.  I'd still work on the other
stuff first and then we can add this in at the end of the patchset.

>> We could also add a SECCOMP_RET_AUDIT, or similar, if we still feel
>> that is important (I personally waffle on this), but I think that is
>> independent of the ideas above.
>
> I agree that it is independent but SECCOMP_RET_AUDIT would still be
> important to Ubuntu.
>
> Tyler

-- 
paul moore
www.paul-moore.com

[toc] | [prev] | [next] | [standalone]

#1550173

On Tue, Jan 3, 2017 at 11:42 AM, Paul Moore <paul@paul-moore.com> wrote:
> On Tue, Jan 3, 2017 at 8:31 AM, Tyler Hicks <tyhicks@canonical.com> wrote:
>> On 01/02/2017 04:47 PM, Paul Moore wrote:
>>> On Mon, Jan 2, 2017 at 11:53 AM, Tyler Hicks <tyhicks@canonical.com> wrote:
>>>> This patch set creates the basis for auditing information specific to a given
>>>> seccomp return action and then starts auditing SECCOMP_RET_ERRNO return
>>>> actions. The audit messages for SECCOMP_RET_ERRNO return actions include the
>>>> errno value that will be returned to userspace.
>>>
>>> I'm replying to this patchset posting because it his my inbox first,
>>> but my comments here apply to both this patchset and the other
>>> seccomp/audit patchset you posted.
>>>
>>> In my experience, we have two or three problems (the count varies
>>> depending on perspective) when it comes to seccomp filter reporting:
>>>
>>> 1. Inability to log all filter actions.
>>> 2. Inability to selectively enable filtering; e.g. devs want noisy
>>> logging, users want relative quiet.
>>> 3. Consistent behavior with audit enabled and disabled.
>>
>> Agreed. Those three logging issues are what have been nagging me the most.
>
> /me nods

I think this sounds fine too.

>>> My current thinking - forgive me, this has been kicking around in my
>>> head for the better part of six months (longer?) and I haven't
>>> attempted to code it up - is to create a sysctl knob for a system wide
>>> seccomp logging threshold that would be applied to the high 16-bits of
>>> *every* triggered action: if the action was at/below the threshold a
>>> record would be emitted, otherwise silence.  This should resolve
>>> problems #1 and #2, and the code should be relatively straightforward
>>> and small.
>>
>> I like that idea quite a bit. To be completely honest, for #1, I
>> personally only care about logging SECCOMP_RET_ERRNO actions but this
>> idea solves it in a nice and general way.
>
> Yeah, I'd much rather solve this problem generally; everybody has
> their favorite action and I'd like to avoid solving the same problem
> multiple times.
>
> Sooo ... you want to take a whack at coding this up? ;)
>
>>> As part of the code above, I expect that all seccomp logging would get
>>> routed through a single logging function (sort of like a better
>>> implementation of the existing audit_seccomp()) that would check the
>>> threshold and trigger the logging if needed.  This function could be
>>> augmented to check for CONFIG_AUDIT and in the case where audit was
>>> not built into the kernel, a simple printk could be used to log the
>>> seccomp event; solving problem #3.
>>
>> That doesn't fully solve #3 for me. In Ubuntu (and I think Debian), we
>> build with CONFIG_AUDIT enabled but don't ship auditd by default so
>> audit_enabled is false. In that default configuration, we still want
>> seccomp audit messages to be printk'ed. I'll need to figure out how to
>> cleanly allow opting into seccomp audit messages when CONFIG_AUDIT is
>> enabled and audit_enabled is false.
>
> Heh, so you've got audit built into the kernel but you're not using
> it; that sounds "fun".
>
> Anyway, I think the logging consolidation could still help you, if for
> no other reason than everything is going through the same function at
> that point.  We could do some other stuff there to handle the case
> where audit is compiled, but auditd is not running ... we already have
> some code in place to handle that for other reasons, check
> kernel/audit.c for more information.  I'd still work on the other
> stuff first and then we can add this in at the end of the patchset.

Yeah, I think the "should I report it?" threshold sysctl could just
check if audit is enabled...

I still wonder, though, isn't there a way to use auditctl to get all
the seccomp messages you need?

-Kees

>
>>> We could also add a SECCOMP_RET_AUDIT, or similar, if we still feel
>>> that is important (I personally waffle on this), but I think that is
>>> independent of the ideas above.
>>
>> I agree that it is independent but SECCOMP_RET_AUDIT would still be
>> important to Ubuntu.
>>
>> Tyler
>
> --
> paul moore
> www.paul-moore.com



-- 
Kees Cook
Nexus Security

[toc] | [prev] | [next] | [standalone]

#1550183

On Tuesday, January 3, 2017 12:44:41 PM EST Kees Cook wrote:
> >> That doesn't fully solve #3 for me. In Ubuntu (and I think Debian), we
> >> build with CONFIG_AUDIT enabled but don't ship auditd by default so
> >> audit_enabled is false. In that default configuration, we still want
> >> seccomp audit messages to be printk'ed. I'll need to figure out how to
> >> cleanly allow opting into seccomp audit messages when CONFIG_AUDIT is
> >> enabled and audit_enabled is false.
> > 
> > Heh, so you've got audit built into the kernel but you're not using
> > it; that sounds "fun".
> > 
> > Anyway, I think the logging consolidation could still help you, if for
> > no other reason than everything is going through the same function at
> > that point.  We could do some other stuff there to handle the case
> > where audit is compiled, but auditd is not running ... we already have
> > some code in place to handle that for other reasons, check
> > kernel/audit.c for more information.  I'd still work on the other
> > stuff first and then we can add this in at the end of the patchset.
> 
> Yeah, I think the "should I report it?" threshold sysctl could just
> check if audit is enabled...
> 
> I still wonder, though, isn't there a way to use auditctl to get all
> the seccomp messages you need?

If you do "auditctl -e 1" then auditing will be enabled and it will send 
events to syslog if the audit daemon is not running.

-Steve

[toc] | [prev] | [next] | [standalone]

#1550192

On Tue, Jan 3, 2017 at 3:44 PM, Kees Cook <keescook@chromium.org> wrote:
> I still wonder, though, isn't there a way to use auditctl to get all
> the seccomp messages you need?

Not all of the seccomp actions are currently logged, that's one of the
problems (and the biggest at the moment).

-- 
paul moore
www.paul-moore.com

[toc] | [prev] | [next] | [standalone]

#1550197

On Tue, Jan 3, 2017 at 12:54 PM, Paul Moore <paul@paul-moore.com> wrote:
> On Tue, Jan 3, 2017 at 3:44 PM, Kees Cook <keescook@chromium.org> wrote:
>> I still wonder, though, isn't there a way to use auditctl to get all
>> the seccomp messages you need?
>
> Not all of the seccomp actions are currently logged, that's one of the
> problems (and the biggest at the moment).

Well... sort of. It all gets passed around, but the logic isn't very
obvious (or at least I always have to go look it up).

include/linux/audit.h:

#ifdef CONFIG_AUDITSYSCALL
...
static inline void audit_seccomp(unsigned long syscall, long signr, int code)
{
        if (!audit_enabled)
                return;

        /* Force a record to be reported if a signal was delivered. */
        if (signr || unlikely(!audit_dummy_context()))
                __audit_seccomp(syscall, signr, code);
}
...
#else /* CONFIG_AUDITSYSCALL */

static inline void audit_seccomp(unsigned long syscall, long signr, int code)
{ }
...
#endif

kernel/seccomp.c:

        switch (action) {
        case SECCOMP_RET_ERRNO:
                /* Set low-order bits as an errno, capped at MAX_ERRNO. */
                if (data > MAX_ERRNO)
                        data = MAX_ERRNO;
                syscall_set_return_value(current, task_pt_regs(current),
                                         -data, 0);
                goto skip;
...
        case SECCOMP_RET_KILL:
        default:
                audit_seccomp(this_syscall, SIGSYS, action);
                do_exit(SIGSYS);
        }

        unreachable();

skip:
        audit_seccomp(this_syscall, 0, action);


Current state:

- if CONFIG_AUDITSYSCALL=n, then nothing is ever reported.

- if audit is disabled, nothing is ever reported.

- if a process isn't being specifically audited
(!audit_dummy_context()), only signals (RET_KILL) are reported.

- when being specifically audited, everything is reported.


So, shouldn't it be possible to specifically audit a process and
examine the resulting logs for the RET_* level one is interested in
("code=0x%x" in __audit_seccomp())?

-Kees

-- 
Kees Cook
Nexus Security

[toc] | [prev] | [next] | [standalone]

#1550207

On Tue, Jan 3, 2017 at 4:03 PM, Kees Cook <keescook@chromium.org> wrote:
> On Tue, Jan 3, 2017 at 12:54 PM, Paul Moore <paul@paul-moore.com> wrote:
>> On Tue, Jan 3, 2017 at 3:44 PM, Kees Cook <keescook@chromium.org> wrote:
>>> I still wonder, though, isn't there a way to use auditctl to get all
>>> the seccomp messages you need?
>>
>> Not all of the seccomp actions are currently logged, that's one of the
>> problems (and the biggest at the moment).
>
> Well... sort of. It all gets passed around, but the logic isn't very
> obvious (or at least I always have to go look it up).

Last time I checked SECCOMP_RET_ALLOW wasn't logged (as well as at
least one other action, but I can't remember which off the top of my
head)?

> include/linux/audit.h:
>
> #ifdef CONFIG_AUDITSYSCALL
> ...
> static inline void audit_seccomp(unsigned long syscall, long signr, int code)
> {
>         if (!audit_enabled)
>                 return;
>
>         /* Force a record to be reported if a signal was delivered. */
>         if (signr || unlikely(!audit_dummy_context()))
>                 __audit_seccomp(syscall, signr, code);
> }
> ...
> #else /* CONFIG_AUDITSYSCALL */
>
> static inline void audit_seccomp(unsigned long syscall, long signr, int code)
> { }
> ...
> #endif
>
> kernel/seccomp.c:
>
>         switch (action) {
>         case SECCOMP_RET_ERRNO:
>                 /* Set low-order bits as an errno, capped at MAX_ERRNO. */
>                 if (data > MAX_ERRNO)
>                         data = MAX_ERRNO;
>                 syscall_set_return_value(current, task_pt_regs(current),
>                                          -data, 0);
>                 goto skip;
> ...
>         case SECCOMP_RET_KILL:
>         default:
>                 audit_seccomp(this_syscall, SIGSYS, action);
>                 do_exit(SIGSYS);
>         }
>
>         unreachable();
>
> skip:
>         audit_seccomp(this_syscall, 0, action);
>
>
> Current state:
>
> - if CONFIG_AUDITSYSCALL=n, then nothing is ever reported.
>
> - if audit is disabled, nothing is ever reported.
>
> - if a process isn't being specifically audited
> (!audit_dummy_context()), only signals (RET_KILL) are reported.
>
> - when being specifically audited, everything is reported.
>
>
> So, shouldn't it be possible to specifically audit a process and
> examine the resulting logs for the RET_* level one is interested in
> ("code=0x%x" in __audit_seccomp())?
>
> -Kees
>
> --
> Kees Cook
> Nexus Security



-- 
paul moore
www.paul-moore.com

[toc] | [prev] | [next] | [standalone]

#1550210

On Tue, Jan 3, 2017 at 1:13 PM, Paul Moore <paul@paul-moore.com> wrote:
> On Tue, Jan 3, 2017 at 4:03 PM, Kees Cook <keescook@chromium.org> wrote:
>> On Tue, Jan 3, 2017 at 12:54 PM, Paul Moore <paul@paul-moore.com> wrote:
>>> On Tue, Jan 3, 2017 at 3:44 PM, Kees Cook <keescook@chromium.org> wrote:
>>>> I still wonder, though, isn't there a way to use auditctl to get all
>>>> the seccomp messages you need?
>>>
>>> Not all of the seccomp actions are currently logged, that's one of the
>>> problems (and the biggest at the moment).
>>
>> Well... sort of. It all gets passed around, but the logic isn't very
>> obvious (or at least I always have to go look it up).
>
> Last time I checked SECCOMP_RET_ALLOW wasn't logged (as well as at
> least one other action, but I can't remember which off the top of my
> head)?

Sure, but if you're using audit, you don't need RET_ALLOW to be logged
because you'll get a full syscall log entry. Logging RET_ALLOW is
redundant and provides no new information, it seems to me.

-Kees

-- 
Kees Cook
Nexus Security

[toc] | [prev] | [next] | [standalone]

#1550225

On Tue, Jan 3, 2017 at 4:21 PM, Kees Cook <keescook@chromium.org> wrote:
> On Tue, Jan 3, 2017 at 1:13 PM, Paul Moore <paul@paul-moore.com> wrote:
>> On Tue, Jan 3, 2017 at 4:03 PM, Kees Cook <keescook@chromium.org> wrote:
>>> On Tue, Jan 3, 2017 at 12:54 PM, Paul Moore <paul@paul-moore.com> wrote:
>>>> On Tue, Jan 3, 2017 at 3:44 PM, Kees Cook <keescook@chromium.org> wrote:
>>>>> I still wonder, though, isn't there a way to use auditctl to get all
>>>>> the seccomp messages you need?
>>>>
>>>> Not all of the seccomp actions are currently logged, that's one of the
>>>> problems (and the biggest at the moment).
>>>
>>> Well... sort of. It all gets passed around, but the logic isn't very
>>> obvious (or at least I always have to go look it up).
>>
>> Last time I checked SECCOMP_RET_ALLOW wasn't logged (as well as at
>> least one other action, but I can't remember which off the top of my
>> head)?
>
> Sure, but if you're using audit, you don't need RET_ALLOW to be logged
> because you'll get a full syscall log entry. Logging RET_ALLOW is
> redundant and provides no new information, it seems to me.

I only bring this up as it might be a way to help solve the
SECCOMP_RET_AUDIT problem that Tyler mentioned.

-- 
paul moore
www.paul-moore.com

[toc] | [prev] | [next] | [standalone]

#1550236

On Tue, Jan 3, 2017 at 1:31 PM, Paul Moore <paul@paul-moore.com> wrote:
> On Tue, Jan 3, 2017 at 4:21 PM, Kees Cook <keescook@chromium.org> wrote:
>> On Tue, Jan 3, 2017 at 1:13 PM, Paul Moore <paul@paul-moore.com> wrote:
>>> On Tue, Jan 3, 2017 at 4:03 PM, Kees Cook <keescook@chromium.org> wrote:
>>>> On Tue, Jan 3, 2017 at 12:54 PM, Paul Moore <paul@paul-moore.com> wrote:
>>>>> On Tue, Jan 3, 2017 at 3:44 PM, Kees Cook <keescook@chromium.org> wrote:
>>>>>> I still wonder, though, isn't there a way to use auditctl to get all
>>>>>> the seccomp messages you need?
>>>>>
>>>>> Not all of the seccomp actions are currently logged, that's one of the
>>>>> problems (and the biggest at the moment).
>>>>
>>>> Well... sort of. It all gets passed around, but the logic isn't very
>>>> obvious (or at least I always have to go look it up).
>>>
>>> Last time I checked SECCOMP_RET_ALLOW wasn't logged (as well as at
>>> least one other action, but I can't remember which off the top of my
>>> head)?
>>
>> Sure, but if you're using audit, you don't need RET_ALLOW to be logged
>> because you'll get a full syscall log entry. Logging RET_ALLOW is
>> redundant and provides no new information, it seems to me.
>
> I only bring this up as it might be a way to help solve the
> SECCOMP_RET_AUDIT problem that Tyler mentioned.

So, I guess I want to understand why something like this doesn't work,
with no changes at all to the kernel:

Imaginary "seccomp-audit.c":

...
    pid = fork();
    if (pid) {
        char cmd[80];

        sprintf(cmd, "auditctl -a always,exit -S all -F pid=%d", pid);
        system(cmd);
        release...
     } else {
        wait for release...
        execv(argv[1], argv + 1);
     }
...

This should dump all syscalls (both RET_ALLOW and RET_ERRNO), as well
as all seccomp actions of any kind. (Down side is the need for root to
launch auditctl...)

Perhaps an improvement to this could be enabling audit when seccomp
syscall is seen? I can't tell if auditctl already has something to do
this ("start auditing this process and all children when syscall X is
performed").

-Kees

-- 
Kees Cook
Nexus Security

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web