Re: [PATCH] audit: create explicit AUDIT_SECCOMP event type

Steve Grubb
2012-11-28T19:30:45Z

On Monday, November 26, 2012 09:45:56 AM Kees Cook wrote:
> On Mon, Nov 26, 2012 at 6:14 AM, Steve Grubb <sgrubb@redhat.com> wrote:
> > On Monday, November 19, 2012 01:56:53 PM Kees Cook wrote:
> >> The seccomp path was using AUDIT_ANOM_ABEND from when seccomp mode 1
> >> could only kill a process. While we still want to make sure an audit
> >> record is forced on a kill, this should use a separate record type since
> >> seccomp mode 2 introduces other behaviors. In the case of "handled"
> >> behaviors (process wasn't killed), only emit a record if the process is
> >> under inspection.
> >
> > Under the old record type, we know that the process is being terminated.
> > Therefore the meaning of the action is known as its implicit in the record
> > type. With this new type, we need to record what behavior is being
> > enforced on the process. I don't see where that is being recorded.
>
> The action is encoded in the "code=". If one is doing seccomp
> auditing, this code will be meaningful already.
>
> > Could we add that?
>
> I'd rather not expand the code into the separate meanings if we don't
> have to. It's part of the BPF already, so it's useful to leave it
> as-is, IMO.

Support for this has been added in the user space utilities. This event type
switch actually fixes a problem where the seccomp use of AUDIT_ANOM_ABEND makes
it malformed because it has different fields. This could be pushed into stable
(after testing) in my opinion since it corrects a problem.

ack: Steve Grubb <sgrubb@redhat.com>

> >> Cc: Julien Tinnes <jln@google.com>
> >> Cc: Will Drewry <wad@google.com>
> >> Signed-off-by: Kees Cook <keescook@chromium.org>
> >> ---
> >>
> >> include/linux/audit.h | 3 ++-
> >> include/uapi/linux/audit.h | 1 +
> >> kernel/auditsc.c | 14 +++++++++++---
> >> 3 files changed, 14 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/include/linux/audit.h b/include/linux/audit.h
> >> index bce729a..9d5104d 100644
> >> --- a/include/linux/audit.h
> >> +++ b/include/linux/audit.h
> >> @@ -157,7 +157,8 @@ void audit_core_dumps(long signr);
> >>
> >> static inline void audit_seccomp(unsigned long syscall, long signr, int
> >>
> >> code) {
> >> - if (unlikely(!audit_dummy_context()))
> >> + /* Force a record to be reported if a signal was delivered. */
> >> + if (signr || unlikely(!audit_dummy_context()))
> >>
> >> __audit_seccomp(syscall, signr, code);
> >>
> >> }
> >>
> >> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> >> index 76352ac..09a2d94 100644
> >> --- a/include/uapi/linux/audit.h
> >> +++ b/include/uapi/linux/audit.h
> >> @@ -106,6 +106,7 @@
> >>
> >> #define AUDIT_MMAP 1323 /* Record showing descriptor and
> >> flags in>
> > mmap */
> >
> >> #define AUDIT_NETFILTER_PKT 1324 /* Packets traversing netfilter
> >> chains
> >
> > */
> >
> >> #define AUDIT_NETFILTER_CFG 1325 /* Netfilter chain modifications */
> >>
> >> +#define AUDIT_SECCOMP 1326 /* Secure Computing event
> >> */
> >>
> >> #define AUDIT_AVC 1400 /* SE Linux avc denial or grant */
> >> #define AUDIT_SELINUX_ERR 1401 /* Internal SE Linux Errors */
> >>
> >> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> >> index 2f186ed..157e989 100644
> >> --- a/kernel/auditsc.c
> >> +++ b/kernel/auditsc.c
> >> @@ -2735,7 +2735,7 @@ void __audit_mmap_fd(int fd, int flags)
> >>
> >> context->type = AUDIT_MMAP;
> >>
> >> }
> >>
> >> -static void audit_log_abend(struct audit_buffer *ab, char *reason, long
> >> signr) +static void audit_log_task(struct audit_buffer *ab)
> >>
> >> {
> >>
> >> kuid_t auid, uid;
> >> kgid_t gid;
> >>
> >> @@ -2753,6 +2753,11 @@ static void audit_log_abend(struct audit_buffer
> >> *ab,
> >> char *reason, long signr) audit_log_task_context(ab);
> >>
> >> audit_log_format(ab, " pid=%d comm=", current->pid);
> >> audit_log_untrustedstring(ab, current->comm);
> >>
> >> +}
> >> +
> >> +static void audit_log_abend(struct audit_buffer *ab, char *reason, long
> >> signr) +{
> >> + audit_log_task(ab);
> >>
> >> audit_log_format(ab, " reason=");
> >> audit_log_string(ab, reason);
> >> audit_log_format(ab, " sig=%ld", signr);
> >>
> >> @@ -2783,8 +2788,11 @@ void __audit_seccomp(unsigned long syscall, long
> >> signr, int code) {
> >>
> >> struct audit_buffer *ab;
> >>
> >> - ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_ANOM_ABEND);
> >> - audit_log_abend(ab, "seccomp", signr);
> >> + ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_SECCOMP);
> >> + if (unlikely(!ab))
> >> + return;
> >> + audit_log_task(ab);
> >> + audit_log_format(ab, " sig=%ld", signr);
> >>
> >> audit_log_format(ab, " syscall=%ld", syscall);
> >> audit_log_format(ab, " compat=%d", is_compat_task());
> >> audit_log_format(ab, " ip=0x%lx", KSTK_EIP(current));