linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] audit: create explicit AUDIT_SECCOMP event type
@ 2012-11-19 21:56 Kees Cook
  2012-11-26 14:14 ` Steve Grubb
  0 siblings, 1 reply; 4+ messages in thread
From: Kees Cook @ 2012-11-19 21:56 UTC (permalink / raw)
  To: linux-kernel
  Cc: Al Viro, Eric Paris, Jeff Layton, Kees Cook, Eric W. Biederman,
	Julien Tinnes, Will Drewry, Steve Grubb, linux-audit

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.

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));
-- 
1.7.9.5


-- 
Kees Cook
Chrome OS Security

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH] audit: create explicit AUDIT_SECCOMP event type
  2012-11-19 21:56 [PATCH] audit: create explicit AUDIT_SECCOMP event type Kees Cook
@ 2012-11-26 14:14 ` Steve Grubb
  2012-11-26 17:45   ` Kees Cook
  0 siblings, 1 reply; 4+ messages in thread
From: Steve Grubb @ 2012-11-26 14:14 UTC (permalink / raw)
  To: Kees Cook
  Cc: linux-kernel, Al Viro, Eric Paris, Jeff Layton,
	Eric W. Biederman, Julien Tinnes, Will Drewry, linux-audit

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.

Could we add that?

Thanks,
-Steve


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

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] audit: create explicit AUDIT_SECCOMP event type
  2012-11-26 14:14 ` Steve Grubb
@ 2012-11-26 17:45   ` Kees Cook
  2012-11-28 19:30     ` Steve Grubb
  0 siblings, 1 reply; 4+ messages in thread
From: Kees Cook @ 2012-11-26 17:45 UTC (permalink / raw)
  To: Steve Grubb
  Cc: linux-kernel, Al Viro, Eric Paris, Jeff Layton,
	Eric W. Biederman, Julien Tinnes, Will Drewry, linux-audit

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.

-Kees

>
> Thanks,
> -Steve
>
>
>> 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));



-- 
Kees Cook
Chrome OS Security

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] audit: create explicit AUDIT_SECCOMP event type
  2012-11-26 17:45   ` Kees Cook
@ 2012-11-28 19:30     ` Steve Grubb
  0 siblings, 0 replies; 4+ messages in thread
From: Steve Grubb @ 2012-11-28 19:30 UTC (permalink / raw)
  To: Kees Cook
  Cc: linux-kernel, Al Viro, Eric Paris, Jeff Layton,
	Eric W. Biederman, Julien Tinnes, Will Drewry, linux-audit

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

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-11-28 19:31 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-19 21:56 [PATCH] audit: create explicit AUDIT_SECCOMP event type Kees Cook
2012-11-26 14:14 ` Steve Grubb
2012-11-26 17:45   ` Kees Cook
2012-11-28 19:30     ` Steve Grubb

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).