[PATCH] auparse: apparmor fields

[PATCH] auparse: apparmor fields

[Marcelo Cerri]

Hi,

I'm taking a look at the AVC records generated by apparmor and some fields in
these records, such as "apparmor" and "operation", are quoted and are not listed
in the auparse/typetab.h file.

Searching in the kernel source, I've found the piece of code that generates the
AVC records and I noticed that it uses the function audit_log_string, which
just add quotes to the string, instead of audit_log_untrustedstring, which seems
to be targeted to escaped fields.

I'd like to know if these fields should be treated the same way as escaped
fields by libauparse or maybe it should be changed in the kernel.

If libauparse really needs to be changed, please consider this patch that
includes the fields "apparmor" and "operation" in the file auparse/typetab.h as
escaped fields.

Regards,
Marcelo
---
 auparse/typetab.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/auparse/typetab.h b/auparse/typetab.h
index 7838c17..0a81e25 100644
--- a/auparse/typetab.h
+++ b/auparse/typetab.h
@@ -86,3 +86,5 @@ _S(AUPARSE_TYPE_NFPROTO,	"family"	)
 _S(AUPARSE_TYPE_ICMPTYPE,	"icmptype"	)
 _S(AUPARSE_TYPE_PROTOCOL,	"proto"		)
 _S(AUPARSE_TYPE_ADDR,		"addr"		)
+_S(AUPARSE_TYPE_ESCAPED,	"apparmor"	)
+_S(AUPARSE_TYPE_ESCAPED,	"operation"	)
-- 
1.7.1

[PATCH v2] auparse: apparmor fields

[Marcelo Cerri]

Just an update for the last patch. This patch contains a complete list of
apparmor avc fields.

Regards,
Marcelo
---
 auparse/typetab.h |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/auparse/typetab.h b/auparse/typetab.h
index 7838c17..27c0ffb 100644
--- a/auparse/typetab.h
+++ b/auparse/typetab.h
@@ -86,3 +86,10 @@ _S(AUPARSE_TYPE_NFPROTO,	"family"	)
 _S(AUPARSE_TYPE_ICMPTYPE,	"icmptype"	)
 _S(AUPARSE_TYPE_PROTOCOL,	"proto"		)
 _S(AUPARSE_TYPE_ADDR,		"addr"		)
+_S(AUPARSE_TYPE_ESCAPED,	"apparmor"	)
+_S(AUPARSE_TYPE_ESCAPED,	"operation"	)
+_S(AUPARSE_TYPE_ESCAPED,	"denied_mask"	)
+_S(AUPARSE_TYPE_ESCAPED,	"info"		)
+_S(AUPARSE_TYPE_ESCAPED,	"profile"	)
+_S(AUPARSE_TYPE_ESCAPED,	"requested_mask")
+
-- 
1.7.1

Re: [PATCH v2] auparse: apparmor fields

[Steve Grubb]

On Monday, February 20, 2012 09:17:06 AM Marcelo Cerri wrote:
> Just an update for the last patch. This patch contains a complete list of
> apparmor avc fields.

It needed #ifdef WITH_APPARMOR since its a compile time option. I added the 
ifdef, you might want to make sure it still works.  Applied.

-Steve

> Regards,
> Marcelo
> ---
>  auparse/typetab.h |    7 +++++++#ifdef WITH_APPARMOR
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/auparse/typetab.h b/auparse/typetab.h
> index 7838c17..27c0ffb 100644
> --- a/auparse/typetab.h
> +++ b/auparse/typetab.h
> @@ -86,3 +86,10 @@ _S(AUPARSE_TYPE_NFPROTO,	"family"	)
>  _S(AUPARSE_TYPE_ICMPTYPE,	"icmptype"	)
>  _S(AUPARSE_TYPE_PROTOCOL,	"proto"		)
>  _S(AUPARSE_TYPE_ADDR,		"addr"		)
> +_S(AUPARSE_TYPE_ESCAPED,	"apparmor"	)
> +_S(AUPARSE_TYPE_ESCAPED,	"operation"	)
> +_S(AUPARSE_TYPE_ESCAPED,	"denied_mask"	)
> +_S(AUPARSE_TYPE_ESCAPED,	"info"		)
> +_S(AUPARSE_TYPE_ESCAPED,	"profile"	)
> +_S(AUPARSE_TYPE_ESCAPED,	"requested_mask")
> +

Re: [PATCH v2] auparse: apparmor fields

[Marcelo Henrique Cerri]

Thanks, Steve.

I've run some tests and it's ok. The same for the other two patches.

I just had to fix some small bugs in the output of configure. Please,
take a look at the patch I've just sent.

Regards,
Marcelo

On Tue, 28 Feb 2012 18:15:48 -0500
Steve Grubb <sgrubb@redhat.com> wrote:

> On Monday, February 20, 2012 09:17:06 AM Marcelo Cerri wrote:
> > Just an update for the last patch. This patch contains a complete
> > list of apparmor avc fields.
> 
> It needed #ifdef WITH_APPARMOR since its a compile time option. I
> added the ifdef, you might want to make sure it still works.  Applied.
> 
> -Steve
> 
> > Regards,
> > Marcelo
> > ---
> >  auparse/typetab.h |    7 +++++++#ifdef WITH_APPARMOR
> >  1 files changed, 7 insertions(+), 0 deletions(-)
> > 
> > diff --git a/auparse/typetab.h b/auparse/typetab.h
> > index 7838c17..27c0ffb 100644
> > --- a/auparse/typetab.h
> > +++ b/auparse/typetab.h
> > @@ -86,3 +86,10 @@ _S(AUPARSE_TYPE_NFPROTO,	"family"	)
> >  _S(AUPARSE_TYPE_ICMPTYPE,	"icmptype"	)
> >  _S(AUPARSE_TYPE_PROTOCOL,	"proto"		)
> >  _S(AUPARSE_TYPE_ADDR,		"addr"		)
> > +_S(AUPARSE_TYPE_ESCAPED,	"apparmor"	)
> > +_S(AUPARSE_TYPE_ESCAPED,	"operation"	)
> > +_S(AUPARSE_TYPE_ESCAPED,	"denied_mask"	)
> > +_S(AUPARSE_TYPE_ESCAPED,	"info"		)
> > +_S(AUPARSE_TYPE_ESCAPED,	"profile"	)
> > +_S(AUPARSE_TYPE_ESCAPED,	"requested_mask")
> > +
> 

Re: [PATCH] auparse: apparmor fields

[Eric Paris]

On Fri, 2012-02-17 at 10:42 -0200, Marcelo Cerri wrote:

> I'd like to know if these fields should be treated the same way as escaped
> fields by libauparse or maybe it should be changed in the kernel.

Users of the audit system may choose to use it however they like.  Steve
and I have agreed to disagree (or at least realized that we will never
agree) on the use of 'audit_log_string'.  As the audit kernel maintainer
I will not ask AppArmor to change what they use and I will continue to
request more users of the kernel audit system to use the *log_string
functions instead of using %s.

Given that, I think the only reasonable option is your patch, so thank
you so much!