[PATCH] userspace: audit: ausearch doesn't return entries for AppArmor events that exist in the log

[PATCH] userspace: audit: ausearch doesn't return entries for AppArmor events that exist in the log

[Tony Jones]

This patch came from our L3 department.  AppArmor LSM is logging using the common_lsm_audit() 
call but the audit userspace parsing code expects to see an SELinux tclass field. This patch 
doesn't address the lack of support for AppArmor in "aureport --avc".  Talking to Seth Arnold, 
Canonical apparently has patches for this; if this is true perhaps they can post for inclusion.

Based-on-work-by: William Preston <wpreston@suse.com>
Signed-off-by: Tony Jones <tonyj@suse.de>

--- a/src/ausearch-parse.c      2014-05-21 14:45:22.000000000 +0200
+++ b/src/ausearch-parse.c      2014-05-21 14:53:55.000000000 +0200
@@ -1735,17 +1735,15 @@ static int parse_avc(const lnode *n, sea

        // Now get the class...its at the end, so we do things different
        str = strstr(term, "tclass=");
-       if (str == NULL) {
-               rc = 9;
-               goto err;
+       if (str) {
+               str += 7;
+               term = strchr(str, ' ');
+               if (term)
+                       *term = 0;
+               an.avc_class = strdup(str);
+               if (term)
+                       *term = ' ';
        }
-       str += 7;
-       term = strchr(str, ' ');
-       if (term)
-               *term = 0;
-       an.avc_class = strdup(str);
-       if (term)
-               *term = ' ';

        if (audit_avc_init(s) == 0) {
                alist_append(s->avc, &an);

Re: [PATCH] userspace: audit: ausearch doesn't return entries for AppArmor events that exist in the log

[Tyler Hicks]

[-- Attachment #1.1: Type: text/plain, Size: 2106 bytes --]

On 2014-05-28 15:33:06, Tony Jones wrote:
> This patch came from our L3 department.  AppArmor LSM is logging using the common_lsm_audit() 
> call but the audit userspace parsing code expects to see an SELinux tclass field. This patch 
> doesn't address the lack of support for AppArmor in "aureport --avc".  Talking to Seth Arnold, 
> Canonical apparently has patches for this; if this is true perhaps they can post for inclusion.

Making the audit tools work with AppArmor generated events has been on
my todo list for quite a while, but no patches exist.

I'm surprised that this patch makes ausearch work correctly for AppArmor
AVC events. The first thing that parse_avc() does is look for the
"avc: " term in the AVCs that SELinux generates. AppArmor's AVCs don't
include that string, so an.avc_result and an.avc_perm would not be set,
would they?

Tyler

> 
> Based-on-work-by: William Preston <wpreston@suse.com>
> Signed-off-by: Tony Jones <tonyj@suse.de>
> 
> --- a/src/ausearch-parse.c      2014-05-21 14:45:22.000000000 +0200
> +++ b/src/ausearch-parse.c      2014-05-21 14:53:55.000000000 +0200
> @@ -1735,17 +1735,15 @@ static int parse_avc(const lnode *n, sea
> 
>         // Now get the class...its at the end, so we do things different
>         str = strstr(term, "tclass=");
> -       if (str == NULL) {
> -               rc = 9;
> -               goto err;
> +       if (str) {
> +               str += 7;
> +               term = strchr(str, ' ');
> +               if (term)
> +                       *term = 0;
> +               an.avc_class = strdup(str);
> +               if (term)
> +                       *term = ' ';
>         }
> -       str += 7;
> -       term = strchr(str, ' ');
> -       if (term)
> -               *term = 0;
> -       an.avc_class = strdup(str);
> -       if (term)
> -               *term = ' ';
> 
>         if (audit_avc_init(s) == 0) {
>                 alist_append(s->avc, &an);
> 
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] userspace: audit: ausearch doesn't return entries for AppArmor events that exist in the log

[Steve Grubb]

On Thursday, May 29, 2014 10:31:52 AM Tyler Hicks wrote:
> On 2014-05-28 15:33:06, Tony Jones wrote:
> > This patch came from our L3 department.  AppArmor LSM is logging using the
> > common_lsm_audit() call but the audit userspace parsing code expects to
> > see an SELinux tclass field. This patch doesn't address the lack of
> > support for AppArmor in "aureport --avc".  Talking to Seth Arnold,
> > Canonical apparently has patches for this; if this is true perhaps they
> > can post for inclusion.
> Making the audit tools work with AppArmor generated events has been on
> my todo list for quite a while, but no patches exist.
> 
> I'm surprised that this patch makes ausearch work correctly for AppArmor
> AVC events. The first thing that parse_avc() does is look for the
> "avc: " term in the AVCs that SELinux generates. AppArmor's AVCs don't
> include that string, so an.avc_result and an.avc_perm would not be set,
> would they?

I have a feeling a whole lot of testing is needed for apparmor, smack, tomoyo, 
or any other LSM besides SE Linux. (Maybe they work fine? I don't know.) 
Ausearch/report, auparse, and auvirt would all need updating. I'd also suggest 
sending patches to the ausearch test suite so that it can verify correctness 
of finding events. One last area, perhaps the prelude plugin might need some 
updating as well....but then again the prelude project kind of died any ways.

-Steve


> > Based-on-work-by: William Preston <wpreston@suse.com>
> > Signed-off-by: Tony Jones <tonyj@suse.de>
> > 
> > --- a/src/ausearch-parse.c      2014-05-21 14:45:22.000000000 +0200
> > +++ b/src/ausearch-parse.c      2014-05-21 14:53:55.000000000 +0200
> > @@ -1735,17 +1735,15 @@ static int parse_avc(const lnode *n, sea
> > 
> >         // Now get the class...its at the end, so we do things different
> >         str = strstr(term, "tclass=");
> > 
> > -       if (str == NULL) {
> > -               rc = 9;
> > -               goto err;
> > +       if (str) {
> > +               str += 7;
> > +               term = strchr(str, ' ');
> > +               if (term)
> > +                       *term = 0;
> > +               an.avc_class = strdup(str);
> > +               if (term)
> > +                       *term = ' ';
> > 
> >         }
> > 
> > -       str += 7;
> > -       term = strchr(str, ' ');
> > -       if (term)
> > -               *term = 0;
> > -       an.avc_class = strdup(str);
> > -       if (term)
> > -               *term = ' ';
> > 
> >         if (audit_avc_init(s) == 0) {
> >         
> >                 alist_append(s->avc, &an);
> > 
> > --
> > Linux-audit mailing list
> > Linux-audit@redhat.com
> > https://www.redhat.com/mailman/listinfo/linux-audit

Re: [PATCH] userspace: audit: ausearch doesn't return entries for AppArmor events that exist in the log

[Tyler Hicks]

[-- Attachment #1.1: Type: text/plain, Size: 3377 bytes --]

On 2014-05-29 11:01:38, Steve Grubb wrote:
> On Thursday, May 29, 2014 10:31:52 AM Tyler Hicks wrote:
> > On 2014-05-28 15:33:06, Tony Jones wrote:
> > > This patch came from our L3 department.  AppArmor LSM is logging using the
> > > common_lsm_audit() call but the audit userspace parsing code expects to
> > > see an SELinux tclass field. This patch doesn't address the lack of
> > > support for AppArmor in "aureport --avc".  Talking to Seth Arnold,
> > > Canonical apparently has patches for this; if this is true perhaps they
> > > can post for inclusion.
> > Making the audit tools work with AppArmor generated events has been on
> > my todo list for quite a while, but no patches exist.
> > 
> > I'm surprised that this patch makes ausearch work correctly for AppArmor
> > AVC events. The first thing that parse_avc() does is look for the
> > "avc: " term in the AVCs that SELinux generates. AppArmor's AVCs don't
> > include that string, so an.avc_result and an.avc_perm would not be set,
> > would they?
> 
> I have a feeling a whole lot of testing is needed for apparmor, smack, tomoyo, 
> or any other LSM besides SE Linux. (Maybe they work fine? I don't know.) 
> Ausearch/report, auparse, and auvirt would all need updating. I'd also suggest 
> sending patches to the ausearch test suite so that it can verify correctness 
> of finding events. 

Agreed. It felt to me like it would be a more work than just updating
parse_avc() to gain full support for other LSMs.

In addition, updating the ausearch test suite is a no-brainer so that
you can easily test with non-SELinux events.

With that said, I don't think these things should be prereqs for
Tony's patch being merged.

> One last area, perhaps the prelude plugin might need some updating as
> well....but then again the prelude project kind of died any ways.

I'm not really familiar with prelude, but I'll keep it in mind.

Tyler

> 
> -Steve
> 
> 
> > > Based-on-work-by: William Preston <wpreston@suse.com>
> > > Signed-off-by: Tony Jones <tonyj@suse.de>
> > > 
> > > --- a/src/ausearch-parse.c      2014-05-21 14:45:22.000000000 +0200
> > > +++ b/src/ausearch-parse.c      2014-05-21 14:53:55.000000000 +0200
> > > @@ -1735,17 +1735,15 @@ static int parse_avc(const lnode *n, sea
> > > 
> > >         // Now get the class...its at the end, so we do things different
> > >         str = strstr(term, "tclass=");
> > > 
> > > -       if (str == NULL) {
> > > -               rc = 9;
> > > -               goto err;
> > > +       if (str) {
> > > +               str += 7;
> > > +               term = strchr(str, ' ');
> > > +               if (term)
> > > +                       *term = 0;
> > > +               an.avc_class = strdup(str);
> > > +               if (term)
> > > +                       *term = ' ';
> > > 
> > >         }
> > > 
> > > -       str += 7;
> > > -       term = strchr(str, ' ');
> > > -       if (term)
> > > -               *term = 0;
> > > -       an.avc_class = strdup(str);
> > > -       if (term)
> > > -               *term = ' ';
> > > 
> > >         if (audit_avc_init(s) == 0) {
> > >         
> > >                 alist_append(s->avc, &an);
> > > 
> > > --
> > > Linux-audit mailing list
> > > Linux-audit@redhat.com
> > > https://www.redhat.com/mailman/listinfo/linux-audit
> 

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] userspace: audit: ausearch doesn't return entries for AppArmor events that exist in the log

[Tony Jones]

On 05/29/2014 01:31 AM, Tyler Hicks wrote:

> I'm surprised that this patch makes ausearch work correctly for AppArmor
> AVC events. The first thing that parse_avc() does is look for the
> "avc: " term in the AVCs that SELinux generates. AppArmor's AVCs don't
> include that string, so an.avc_result and an.avc_perm would not be set,
> would they?

That patch does "work" (tested w/ svn trunk).   After I read your comment I looked at the code and I was confused also as 'avc_result == AVC_UNSET' but find_avc() which checks against UNSET isn't being called,  rather the record gets selected for output by 'n = list_get_cur(l)' [ausearch-match.c:113].     I would need to spend more time to fully understand what is happening in the code.

$ cat log
type=AVC msg=audit(1390876383.602:15646): apparmor="DENIED" operation="open" parent=21147 profile="/tmp/ls" name="/var/log/audit/" pid=21598 comm="ls" requested_mask="r" denied_mask="r" fsuid=0 ouid=0
type=AVC msg=audit(1390936201.188:15647): apparmor="ALLOWED" operation="file_lock" parent=7873 profile="/usr/sbin/sshd" name="/tmp/pam_krb5_tmp_FqhNDa" pid=7875 comm="sshd" requested_mask="k" denied_mask="k" fsuid=0 ouid=0

$ /tmp/audit/sbin/ausearch -m AVC -if ./log
----
time->Mon Jan 27 18:33:03 2014
type=AVC msg=audit(1390876383.602:15646): apparmor="DENIED" operation="open" parent=21147 profile="/tmp/ls" name="/var/log/audit/" pid=21598 comm="ls" requested_mask="r" denied_mask="r" fsuid=0 ouid=0
----
time->Tue Jan 28 11:10:01 2014
type=AVC msg=audit(1390936201.188:15647): apparmor="ALLOWED" operation="file_lock" parent=7873 profile="/usr/sbin/sshd" name="/tmp/pam_krb5_tmp_FqhNDa" pid=7875 comm="sshd" requested_mask="k" denied_mask="k" fsuid=0 ouid=0


Without patch,  ausearch just outputs "<no matches>"

tony

Re: [PATCH] userspace: audit: ausearch doesn't return entries for AppArmor events that exist in the log

[Steve Grubb]

On Monday, June 02, 2014 06:00:54 PM Tony Jones wrote:
> On 05/29/2014 01:31 AM, Tyler Hicks wrote:
> > I'm surprised that this patch makes ausearch work correctly for AppArmor
> > AVC events. The first thing that parse_avc() does is look for the
> > "avc: " term in the AVCs that SELinux generates. AppArmor's AVCs don't
> > include that string, so an.avc_result and an.avc_perm would not be set,
> > would they?
> 
> That patch does "work" (tested w/ svn trunk).

Right. The parsing code aborts the parse if it doesn't find required/expected 
fields. So, if a field is missing, it skips the event. The --debug option will 
print events that get skipped due to being malformed.

> After I read your comment I
> looked at the code and I was confused also as 'avc_result == AVC_UNSET' but
> find_avc() which checks against UNSET isn't being called,  rather the
> record gets selected for output by 'n = list_get_cur(l)'
> [ausearch-match.c:113].     I would need to spend more time to fully
> understand what is happening in the code.

Ausearch is simple. It gathers fields only if they are relevant to the command 
line flags. If auid is given, it collects those fields, but the parser expects a 
well formed event or it will exclude it from the results.


> Without patch,  ausearch just outputs "<no matches>"

Yep. So, the question is really how to fix this. Should we have a different 
function that is swung in with #ifdef WITH_APPARMOR called parse_aa_avc? Then 
it can be tuned exactly for AppArmor's needs? Later, the kernel event number 
can be changed and the switch/case can pick that up. Also, are there other AA 
events that are missing in action? The ausearch-test should tell you.

-Steve

Re: [PATCH] userspace: audit: ausearch doesn't return entries for AppArmor events that exist in the log

[Tony Jones]

On 06/03/2014 07:47 AM, Steve Grubb wrote:
> Yep. So, the question is really how to fix this. Should we have a different 
> function that is swung in with #ifdef WITH_APPARMOR called parse_aa_avc? Then 
> it can be tuned exactly for AppArmor's needs? Later, the kernel event number 
> can be changed and the switch/case can pick that up. Also, are there other AA 
> events that are missing in action? The ausearch-test should tell you.

We'll take the patch (locally) for SLES.  Seems to me, since there really isn't any AppArmor awareness in audit at present that the AppArmor developers 
may as well fix the kernel event numbering first,  audit userspace after that .... anyhow, I see no point considering the previous patch for upstreaming.

Thanks

Tony

Re: [PATCH] userspace: audit: ausearch doesn't return entries for AppArmor events that exist in the log

[Tyler Hicks]

[-- Attachment #1.1: Type: text/plain, Size: 1826 bytes --]

On 2014-05-28 15:33:06, Tony Jones wrote:
> This patch came from our L3 department.  AppArmor LSM is logging using the common_lsm_audit() 
> call but the audit userspace parsing code expects to see an SELinux tclass field. This patch 
> doesn't address the lack of support for AppArmor in "aureport --avc".  Talking to Seth Arnold, 
> Canonical apparently has patches for this; if this is true perhaps they can post for inclusion.
> 
> Based-on-work-by: William Preston <wpreston@suse.com>
> Signed-off-by: Tony Jones <tonyj@suse.de>
> 
> --- a/src/ausearch-parse.c      2014-05-21 14:45:22.000000000 +0200
> +++ b/src/ausearch-parse.c      2014-05-21 14:53:55.000000000 +0200
> @@ -1735,17 +1735,15 @@ static int parse_avc(const lnode *n, sea
> 
>         // Now get the class...its at the end, so we do things different
>         str = strstr(term, "tclass=");
> -       if (str == NULL) {
> -               rc = 9;

It is nitpicky, but all error return codes after this should be
decremented (there is only one to do).

The rest of the patch looks fine to me.

Tyler

> -               goto err;
> +       if (str) {
> +               str += 7;
> +               term = strchr(str, ' ');
> +               if (term)
> +                       *term = 0;
> +               an.avc_class = strdup(str);
> +               if (term)
> +                       *term = ' ';
>         }
> -       str += 7;
> -       term = strchr(str, ' ');
> -       if (term)
> -               *term = 0;
> -       an.avc_class = strdup(str);
> -       if (term)
> -               *term = ' ';
> 
>         if (audit_avc_init(s) == 0) {
>                 alist_append(s->avc, &an);
> 
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] userspace: audit: ausearch doesn't return entries for AppArmor events that exist in the log

[Steve Grubb]

On Wednesday, May 28, 2014 03:33:06 PM Tony Jones wrote:
> This patch came from our L3 department.  AppArmor LSM is logging using the
> common_lsm_audit() call but the audit userspace parsing code expects to see
> an SELinux tclass field. This patch doesn't address the lack of support for
> AppArmor in "aureport --avc".  Talking to Seth Arnold, Canonical apparently
> has patches for this; if this is true perhaps they can post for inclusion.
> 
> Based-on-work-by: William Preston <wpreston@suse.com>
> Signed-off-by: Tony Jones <tonyj@suse.de>

I was looking at this patch and was wondering something. Does AppArmor produce 
AUDIT_AVC events? If not, how does the code even get into parse_avc? IOW, is 
there another part of the patch missing in the switch statement that direct 
AUDIT_APPARMOR_*  events into parse_avc?

-Steve

> --- a/src/ausearch-parse.c      2014-05-21 14:45:22.000000000 +0200
> +++ b/src/ausearch-parse.c      2014-05-21 14:53:55.000000000 +0200
> @@ -1735,17 +1735,15 @@ static int parse_avc(const lnode *n, sea
> 
>         // Now get the class...its at the end, so we do things different
>         str = strstr(term, "tclass=");
> -       if (str == NULL) {
> -               rc = 9;
> -               goto err;
> +       if (str) {
> +               str += 7;
> +               term = strchr(str, ' ');
> +               if (term)
> +                       *term = 0;
> +               an.avc_class = strdup(str);
> +               if (term)
> +                       *term = ' ';
>         }
> -       str += 7;
> -       term = strchr(str, ' ');
> -       if (term)
> -               *term = 0;
> -       an.avc_class = strdup(str);
> -       if (term)
> -               *term = ' ';
> 
>         if (audit_avc_init(s) == 0) {
>                 alist_append(s->avc, &an);

Re: [PATCH] userspace: audit: ausearch doesn't return entries for AppArmor events that exist in the log

[Tyler Hicks]

[-- Attachment #1.1: Type: text/plain, Size: 2868 bytes --]

On 2014-05-30 15:53:49, Steve Grubb wrote:
> On Wednesday, May 28, 2014 03:33:06 PM Tony Jones wrote:
> > This patch came from our L3 department.  AppArmor LSM is logging using the
> > common_lsm_audit() call but the audit userspace parsing code expects to see
> > an SELinux tclass field. This patch doesn't address the lack of support for
> > AppArmor in "aureport --avc".  Talking to Seth Arnold, Canonical apparently
> > has patches for this; if this is true perhaps they can post for inclusion.
> > 
> > Based-on-work-by: William Preston <wpreston@suse.com>
> > Signed-off-by: Tony Jones <tonyj@suse.de>
> 
> I was looking at this patch and was wondering something. Does AppArmor produce 
> AUDIT_AVC events?

It does. Here's an odd ball that I picked out of my audit log:

type=AVC msg=audit(1400295012.391:11143): apparmor="DENIED" operation="mount" info="failed type match" profile="lxc-container-default" name="/sys/fs/cgroup/systemd/" pid=15761 comm="mount" fstype="cgroup" srcname="systemd" flags="rw, nosuid, nodev, noexec"

> If not, how does the code even get into parse_avc? IOW, is 
> there another part of the patch missing in the switch statement that direct 
> AUDIT_APPARMOR_*  events into parse_avc?

As you can see above, they already flow through parse_avc().

I spent a little time today getting a patch together to parse the
'apparmor="DENIED" operation="mount"' portion parsed to fill
an.avc_result and an.avc_perm.

It worked well with ausearch and aureport during some quick manual
testing. I'd like to do some more testing next week and, hopefully, add
some automated tests before I send it to the list.

Tyler

> 
> -Steve
> 
> > --- a/src/ausearch-parse.c      2014-05-21 14:45:22.000000000 +0200
> > +++ b/src/ausearch-parse.c      2014-05-21 14:53:55.000000000 +0200
> > @@ -1735,17 +1735,15 @@ static int parse_avc(const lnode *n, sea
> > 
> >         // Now get the class...its at the end, so we do things different
> >         str = strstr(term, "tclass=");
> > -       if (str == NULL) {
> > -               rc = 9;
> > -               goto err;
> > +       if (str) {
> > +               str += 7;
> > +               term = strchr(str, ' ');
> > +               if (term)
> > +                       *term = 0;
> > +               an.avc_class = strdup(str);
> > +               if (term)
> > +                       *term = ' ';
> >         }
> > -       str += 7;
> > -       term = strchr(str, ' ');
> > -       if (term)
> > -               *term = 0;
> > -       an.avc_class = strdup(str);
> > -       if (term)
> > -               *term = ' ';
> > 
> >         if (audit_avc_init(s) == 0) {
> >                 alist_append(s->avc, &an);
> 
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] userspace: audit: ausearch doesn't return entries for AppArmor events that exist in the log

[Steve Grubb]

[-- Attachment #1.1: Type: text/plain, Size: 2938 bytes --]

On Friday, May 30, 2014 10:16:44 PM Tyler Hicks wrote:
> On 2014-05-30 15:53:49, Steve Grubb wrote:
> > On Wednesday, May 28, 2014 03:33:06 PM Tony Jones wrote:
> > > This patch came from our L3 department.  AppArmor LSM is logging using
> > > the
> > > common_lsm_audit() call but the audit userspace parsing code expects to
> > > see
> > > an SELinux tclass field. This patch doesn't address the lack of support
> > > for
> > > AppArmor in "aureport --avc".  Talking to Seth Arnold, Canonical
> > > apparently
> > > has patches for this; if this is true perhaps they can post for
> > > inclusion.
> > > 
> > > Based-on-work-by: William Preston <wpreston@suse.com>
> > > Signed-off-by: Tony Jones <tonyj@suse.de>
> > 
> > I was looking at this patch and was wondering something. Does AppArmor
> > produce AUDIT_AVC events?
> 
> It does. Here's an odd ball that I picked out of my audit log:

Uh-oh. I gave out the 1500 - 1599 block of events to App Armor so that this 
problem would never happen.

libaudit.h:
#define AUDIT_FIRST_SELINUX     1400
#define AUDIT_LAST_SELINUX      1499
#define AUDIT_FIRST_APPARMOR            1500
#define AUDIT_LAST_APPARMOR             1599

When you have an event of the same number, it has to have the same fields in 
the same order with same value representation. As you can see the reporting 
tools is sensitive to this because they are optimized to handle huge log files. 
Hmmm....


> type=AVC msg=audit(1400295012.391:11143): apparmor="DENIED"
> operation="mount" info="failed type match" profile="lxc-container-default"
> name="/sys/fs/cgroup/systemd/" pid=15761 comm="mount" fstype="cgroup"
> srcname="systemd" flags="rw, nosuid, nodev, noexec"
> > If not, how does the code even get into parse_avc? IOW, is
> > there another part of the patch missing in the switch statement that
> > direct AUDIT_APPARMOR_*  events into parse_avc?
> 
> As you can see above, they already flow through parse_avc().

This is a big mistake, IMHO. In theory, this is what should have happened:
 An access decisionl event should have been named in the 1500 block. It would 
then be free to include the field it needs in the order it needs. The ausearch 
would get a function parse_aa_decision. That function would stuff a struct 
specially tuned for AA usage. Aureport would gain a new report.


> I spent a little time today getting a patch together to parse the
> 'apparmor="DENIED" operation="mount"' portion parsed to fill
> an.avc_result and an.avc_perm.
> 
> It worked well with ausearch and aureport during some quick manual
> testing. I'd like to do some more testing next week and, hopefully, add
> some automated tests before I send it to the list.

Well...I wished it had been clear a long time ago that the 1500 block was free 
to do anything with for AA's needs so we don't overload the usage. Not sure 
what to do about this.

-Steve

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] userspace: audit: ausearch doesn't return entries for AppArmor events that exist in the log

[Tony Jones]

On 05/30/2014 02:00 PM, Steve Grubb wrote:

> This is a big mistake, IMHO. In theory, this is what should have happened:
>  An access decisionl event should have been named in the 1500 block. It would 
> then be free to include the field it needs in the order it needs. The ausearch 
> would get a function parse_aa_decision. That function would stuff a struct 
> specially tuned for AA usage. Aureport would gain a new report.

The very original AA submission logged everything from the kernel using AUDIT_AA which was defined in the submission as:

+#define AUDIT_AA 1500 /* AppArmor audit */

I'm not sure when the change was made to call common_lsm_audit() which logs as AUDIT_AVC. I agree with Steve, doesn't seem a good idea.

tony

Re: [PATCH] userspace: audit: ausearch doesn't return entries for AppArmor events that exist in the log

[Tyler Hicks]

[-- Attachment #1.1: Type: text/plain, Size: 4858 bytes --]

On 2014-05-30 17:00:04, Steve Grubb wrote:
> On Friday, May 30, 2014 10:16:44 PM Tyler Hicks wrote:
> > On 2014-05-30 15:53:49, Steve Grubb wrote:
> > > On Wednesday, May 28, 2014 03:33:06 PM Tony Jones wrote:
> > > > This patch came from our L3 department.  AppArmor LSM is logging using
> > > > the
> > > > common_lsm_audit() call but the audit userspace parsing code expects to
> > > > see
> > > > an SELinux tclass field. This patch doesn't address the lack of support
> > > > for
> > > > AppArmor in "aureport --avc".  Talking to Seth Arnold, Canonical
> > > > apparently
> > > > has patches for this; if this is true perhaps they can post for
> > > > inclusion.
> > > > 
> > > > Based-on-work-by: William Preston <wpreston@suse.com>
> > > > Signed-off-by: Tony Jones <tonyj@suse.de>
> > > 
> > > I was looking at this patch and was wondering something. Does AppArmor
> > > produce AUDIT_AVC events?
> > 
> > It does. Here's an odd ball that I picked out of my audit log:
> 
> Uh-oh. I gave out the 1500 - 1599 block of events to App Armor so that this 
> problem would never happen.
> 
> libaudit.h:
> #define AUDIT_FIRST_SELINUX     1400
> #define AUDIT_LAST_SELINUX      1499
> #define AUDIT_FIRST_APPARMOR            1500
> #define AUDIT_LAST_APPARMOR             1599

I wasn't involved with AppArmor when it was going through upstream
acceptance reviews, but I've asked around to get the history. 

As Tony mentioned, AppArmor was originally using the 1500-1599 block. At
some point (I couldn't find it in the list archives), it was said that
AppArmor needs to use common_lsm_audit() which unconditionally uses
AUDIT_AVC.

That function is used by SELinux, SMACK, and AppArmor. SMACK was the
first user of that function:

  http://git.kernel.org/linus/6e837fb152410e571a81aaadbd9884f0bc46a55e
  http://git.kernel.org/linus/ecfcc53fef3c357574bb6143dce6631e6d56295c

AppArmor has used common_lsm_audit() since its upstream acceptance:

  http://git.kernel.org/linus/67012e8209df95a8290d135753ff5145431a666e

> 
> When you have an event of the same number, it has to have the same fields in 
> the same order with same value representation. As you can see the reporting 
> tools is sensitive to this because they are optimized to handle huge log files. 
> Hmmm....
> 
> 
> > type=AVC msg=audit(1400295012.391:11143): apparmor="DENIED"
> > operation="mount" info="failed type match" profile="lxc-container-default"
> > name="/sys/fs/cgroup/systemd/" pid=15761 comm="mount" fstype="cgroup"
> > srcname="systemd" flags="rw, nosuid, nodev, noexec"
> > > If not, how does the code even get into parse_avc? IOW, is
> > > there another part of the patch missing in the switch statement that
> > > direct AUDIT_APPARMOR_*  events into parse_avc?
> > 
> > As you can see above, they already flow through parse_avc().
> 
> This is a big mistake, IMHO. In theory, this is what should have happened:
>  An access decisionl event should have been named in the 1500 block. It would 
> then be free to include the field it needs in the order it needs. The ausearch 
> would get a function parse_aa_decision. That function would stuff a struct 
> specially tuned for AA usage. Aureport would gain a new report.
> 
> 
> > I spent a little time today getting a patch together to parse the
> > 'apparmor="DENIED" operation="mount"' portion parsed to fill
> > an.avc_result and an.avc_perm.
> > 
> > It worked well with ausearch and aureport during some quick manual
> > testing. I'd like to do some more testing next week and, hopefully, add
> > some automated tests before I send it to the list.
> 
> Well...I wished it had been clear a long time ago that the 1500 block was free 
> to do anything with for AA's needs so we don't overload the usage. Not sure 
> what to do about this.

There must have been a disconnect between what you were envisioning in
the audit tools and the merging of common_lsm_audit().

There are now two options going forward:

1) AppArmor and SMACK keep the current audit values and the audit tools
   parse a little deeper.
   
   SELinux audit events are separated from other LSM audit events by the
   "avc:  denied  {" prefix.
   
   AppArmor audit events are separated from other LSM audit events by
   the "apparmor=" prefix.
   
   And so on for SMACK and others...

   I'm assuming that other audit event types can easily be separated
   this way, but I haven't looked into that yet.

2) AppArmor, SMACK, and other non-SELinux LSMs switch over to using
   their own block for audit events. This creates kernel work to change
   over to the new block. It also creates work in the AppArmor tools, as
   they do parsing of audit events, too.

   This is the cleanest solution but, as usual, requires the most work.

Tyler

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] userspace: audit: ausearch doesn't return entries for AppArmor events that exist in the log

[Tyler Hicks]

[-- Attachment #1.1: Type: text/plain, Size: 2321 bytes --]

[Added Eric to cc]

On 2014-06-06 13:46:48, Tyler Hicks wrote:
> On 2014-05-30 17:00:04, Steve Grubb wrote:
> > On Friday, May 30, 2014 10:16:44 PM Tyler Hicks wrote:
> > > On 2014-05-30 15:53:49, Steve Grubb wrote:
> > > > On Wednesday, May 28, 2014 03:33:06 PM Tony Jones wrote:
> > > > > This patch came from our L3 department.  AppArmor LSM is logging using
> > > > > the
> > > > > common_lsm_audit() call but the audit userspace parsing code expects to
> > > > > see
> > > > > an SELinux tclass field. This patch doesn't address the lack of support
> > > > > for
> > > > > AppArmor in "aureport --avc".  Talking to Seth Arnold, Canonical
> > > > > apparently
> > > > > has patches for this; if this is true perhaps they can post for
> > > > > inclusion.
> > > > > 
> > > > > Based-on-work-by: William Preston <wpreston@suse.com>
> > > > > Signed-off-by: Tony Jones <tonyj@suse.de>
> > > > 
> > > > I was looking at this patch and was wondering something. Does AppArmor
> > > > produce AUDIT_AVC events?
> > > 
> > > It does. Here's an odd ball that I picked out of my audit log:
> > 
> > Uh-oh. I gave out the 1500 - 1599 block of events to App Armor so that this 
> > problem would never happen.
> > 
> > libaudit.h:
> > #define AUDIT_FIRST_SELINUX     1400
> > #define AUDIT_LAST_SELINUX      1499
> > #define AUDIT_FIRST_APPARMOR            1500
> > #define AUDIT_LAST_APPARMOR             1599
> 
> I wasn't involved with AppArmor when it was going through upstream
> acceptance reviews, but I've asked around to get the history. 
> 
> As Tony mentioned, AppArmor was originally using the 1500-1599 block. At
> some point (I couldn't find it in the list archives), it was said that
> AppArmor needs to use common_lsm_audit() which unconditionally uses
> AUDIT_AVC.

I found the review that caused AppArmor to switch to the common LSM
audit function:

  https://lkml.org/lkml/2009/11/9/232

That email is almost 5 years old and minds can change over that time,
but Eric seemed to be against adding new audit event types for each LSM.
Instead, he wanted a lsm=<LSM> pair to be included in the message.

AppArmor can accommodate either approach so I think Steve and Eric ought
to come to an agreement on what non-SELinux LSMs should do when
auditing.

Tyler

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] userspace: audit: ausearch doesn't return entries for AppArmor events that exist in the log

[Tony Jones]

On 06/06/2014 02:10 PM, Tyler Hicks wrote:
> [Added Eric to cc]

You didn't actually add Eric to the Cc:    Adding him.   

> 
> On 2014-06-06 13:46:48, Tyler Hicks wrote:
>> On 2014-05-30 17:00:04, Steve Grubb wrote:
>>> On Friday, May 30, 2014 10:16:44 PM Tyler Hicks wrote:
>>>> On 2014-05-30 15:53:49, Steve Grubb wrote:
>>>>> On Wednesday, May 28, 2014 03:33:06 PM Tony Jones wrote:
>>>>>> This patch came from our L3 department.  AppArmor LSM is logging using
>>>>>> the
>>>>>> common_lsm_audit() call but the audit userspace parsing code expects to
>>>>>> see
>>>>>> an SELinux tclass field. This patch doesn't address the lack of support
>>>>>> for
>>>>>> AppArmor in "aureport --avc".  Talking to Seth Arnold, Canonical
>>>>>> apparently
>>>>>> has patches for this; if this is true perhaps they can post for
>>>>>> inclusion.
>>>>>>
>>>>>> Based-on-work-by: William Preston <wpreston@suse.com>
>>>>>> Signed-off-by: Tony Jones <tonyj@suse.de>
>>>>>
>>>>> I was looking at this patch and was wondering something. Does AppArmor
>>>>> produce AUDIT_AVC events?
>>>>
>>>> It does. Here's an odd ball that I picked out of my audit log:
>>>
>>> Uh-oh. I gave out the 1500 - 1599 block of events to App Armor so that this 
>>> problem would never happen.
>>>
>>> libaudit.h:
>>> #define AUDIT_FIRST_SELINUX     1400
>>> #define AUDIT_LAST_SELINUX      1499
>>> #define AUDIT_FIRST_APPARMOR            1500
>>> #define AUDIT_LAST_APPARMOR             1599
>>
>> I wasn't involved with AppArmor when it was going through upstream
>> acceptance reviews, but I've asked around to get the history. 
>>
>> As Tony mentioned, AppArmor was originally using the 1500-1599 block. At
>> some point (I couldn't find it in the list archives), it was said that
>> AppArmor needs to use common_lsm_audit() which unconditionally uses
>> AUDIT_AVC.
> 
> I found the review that caused AppArmor to switch to the common LSM
> audit function:
> 
>   https://lkml.org/lkml/2009/11/9/232
> 
> That email is almost 5 years old and minds can change over that time,
> but Eric seemed to be against adding new audit event types for each LSM.
> Instead, he wanted a lsm=<LSM> pair to be included in the message.
> 
> AppArmor can accommodate either approach so I think Steve and Eric ought
> to come to an agreement on what non-SELinux LSMs should do when
> auditing.
> 
> Tyler
> 
> 
> 
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
> 

Re: [PATCH] userspace: audit: ausearch doesn't return entries for AppArmor events that exist in the log

[Eric Paris]

I'm fine if other LSMs would like to use their own record type.  Makes
sense.

-Eric

On Mon, 23 Jun 2014 17:06:55 -0700
Tony Jones <tonyj@suse.de> wrote:

> On 06/06/2014 02:10 PM, Tyler Hicks wrote:
> > [Added Eric to cc]
> 
> You didn't actually add Eric to the Cc:    Adding him.   
> 
> > 
> > On 2014-06-06 13:46:48, Tyler Hicks wrote:
> >> On 2014-05-30 17:00:04, Steve Grubb wrote:
> >>> On Friday, May 30, 2014 10:16:44 PM Tyler Hicks wrote:
> >>>> On 2014-05-30 15:53:49, Steve Grubb wrote:
> >>>>> On Wednesday, May 28, 2014 03:33:06 PM Tony Jones wrote:
> >>>>>> This patch came from our L3 department.  AppArmor LSM is
> >>>>>> logging using the
> >>>>>> common_lsm_audit() call but the audit userspace parsing code
> >>>>>> expects to see
> >>>>>> an SELinux tclass field. This patch doesn't address the lack
> >>>>>> of support for
> >>>>>> AppArmor in "aureport --avc".  Talking to Seth Arnold,
> >>>>>> Canonical apparently
> >>>>>> has patches for this; if this is true perhaps they can post for
> >>>>>> inclusion.
> >>>>>>
> >>>>>> Based-on-work-by: William Preston <wpreston@suse.com>
> >>>>>> Signed-off-by: Tony Jones <tonyj@suse.de>
> >>>>>
> >>>>> I was looking at this patch and was wondering something. Does
> >>>>> AppArmor produce AUDIT_AVC events?
> >>>>
> >>>> It does. Here's an odd ball that I picked out of my audit log:
> >>>
> >>> Uh-oh. I gave out the 1500 - 1599 block of events to App Armor so
> >>> that this problem would never happen.
> >>>
> >>> libaudit.h:
> >>> #define AUDIT_FIRST_SELINUX     1400
> >>> #define AUDIT_LAST_SELINUX      1499
> >>> #define AUDIT_FIRST_APPARMOR            1500
> >>> #define AUDIT_LAST_APPARMOR             1599
> >>
> >> I wasn't involved with AppArmor when it was going through upstream
> >> acceptance reviews, but I've asked around to get the history. 
> >>
> >> As Tony mentioned, AppArmor was originally using the 1500-1599
> >> block. At some point (I couldn't find it in the list archives), it
> >> was said that AppArmor needs to use common_lsm_audit() which
> >> unconditionally uses AUDIT_AVC.
> > 
> > I found the review that caused AppArmor to switch to the common LSM
> > audit function:
> > 
> >   https://lkml.org/lkml/2009/11/9/232
> > 
> > That email is almost 5 years old and minds can change over that
> > time, but Eric seemed to be against adding new audit event types
> > for each LSM. Instead, he wanted a lsm=<LSM> pair to be included in
> > the message.
> > 
> > AppArmor can accommodate either approach so I think Steve and Eric
> > ought to come to an agreement on what non-SELinux LSMs should do
> > when auditing.
> > 
> > Tyler
> > 
> > 
> > 
> > --
> > Linux-audit mailing list
> > Linux-audit@redhat.com
> > https://www.redhat.com/mailman/listinfo/linux-audit
> > 
> 

Re: [PATCH] userspace: audit: ausearch doesn't return entries for AppArmor events that exist in the log

[Vincas Dargis]

Hi,

There was email about fixing ausearch for AppArmor:

https://www.redhat.com/archives/linux-audit/2014-May/msg00094.html

Is there any progress regarding that issue?

I have tried to search for AVC on Debian Testing (auditd 2.4.5), and
it fails to "grep" me AppArmor related events.

P.S. How do I actually reply to original thread that I did not
received, since I just subscribed? I though I could maybe find raw
message in archive https://www.redhat.com/archives/linux-audit/ but
there aren't (no such message in 2014-May/Jun gz) . Oh how I hate
using mailing lists so much... /rant.

Re: [PATCH] userspace: audit: ausearch doesn't return entries for AppArmor events that exist in the log

[Steve Grubb]

On Friday, April 29, 2016 10:03:02 AM Vincas Dargis wrote:
> There was email about fixing ausearch for AppArmor:
> 
> https://www.redhat.com/archives/linux-audit/2014-May/msg00094.html
> 
> Is there any progress regarding that issue?

You'll have to ask the AppArmor folks. I gave them a whole block of numbers to 
use for their own purposes so that we don't have any problems. If they instead 
create malformed SE Linux events, then things will never work right unless 
they patch them. I don't plan to carry a patch in the main utility because it 
completely violates all audit assumptions.

The main rule is that all audit records of the same type have to have the 
exact same fields, in the same order, with the same format or no one can 
analyze the events. You have to think of each record as a database table. Each 
record is a row, each field is a column.


> I have tried to search for AVC on Debian Testing (auditd 2.4.5), and
> it fails to "grep" me AppArmor related events.
> 
> P.S. How do I actually reply to original thread that I did not
> received, since I just subscribed? I though I could maybe find raw
> message in archive https://www.redhat.com/archives/linux-audit/ but
> there aren't (no such message in 2014-May/Jun gz) . Oh how I hate
> using mailing lists so much... /rant.

Just start a new one. Why worry?

-Steve

Re: [PATCH] userspace: audit: ausearch doesn't return entries for AppArmor events that exist in the log

[Vincas Dargis]

2016.04.29 16:39, Steve Grubb rašė:
> You'll have to ask the AppArmor folks. I gave them a whole block of numbers to
> use for their own purposes so that we don't have any problems. If they instead
> create malformed SE Linux events, then things will never work right unless
> they patch them.

Thank you for explanation, Steve. I'll bring this topic for them instead.

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit

Re: [PATCH] userspace: audit: ausearch doesn't return entries for AppArmor events that exist in the log

[Steve Grubb]

On Friday, April 29, 2016 07:07:06 PM Vincas Dargis wrote:
> 2016.04.29 16:39, Steve Grubb rašė:
> > You'll have to ask the AppArmor folks. I gave them a whole block of
> > numbers to use for their own purposes so that we don't have any problems.
> > If they instead create malformed SE Linux events, then things will never
> > work right unless they patch them.
> 
> Thank you for explanation, Steve. I'll bring this topic for them instead.

Just to clarify, its not like I don't want this to work. I do. The intention 
of giving app armor its own block was that things its doing might not be a 
100% fit for what SE Linux does. This was to give them the freedom to do their 
own thing. If they chose not to use the block and instead try to shoe-horn 
their events into a pre-existing one, there needs to be some discussion about 
how to make things right.

-Steve

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit

Re: [PATCH] userspace: audit: ausearch doesn't return entries for AppArmor events that exist in the log

[Paul Moore]

On Fri, Apr 29, 2016 at 12:30 PM, Steve Grubb <sgrubb@redhat.com> wrote:
> On Friday, April 29, 2016 07:07:06 PM Vincas Dargis wrote:
>> 2016.04.29 16:39, Steve Grubb rašė:
>> > You'll have to ask the AppArmor folks. I gave them a whole block of
>> > numbers to use for their own purposes so that we don't have any problems.
>> > If they instead create malformed SE Linux events, then things will never
>> > work right unless they patch them.
>>
>> Thank you for explanation, Steve. I'll bring this topic for them instead.
>
> Just to clarify, its not like I don't want this to work. I do. The intention
> of giving app armor its own block was that things its doing might not be a
> 100% fit for what SE Linux does. This was to give them the freedom to do their
> own thing. If they chose not to use the block and instead try to shoe-horn
> their events into a pre-existing one, there needs to be some discussion about
> how to make things right.
>
> -Steve
>
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

[NOTE: I'm adding the AppArmor maintainer, John Johansen to this thread]

Hi John,

Heads-up, it looks like there might be some issues with AppArmor and auditing...

-- 
paul moore
security @ redhat

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit

Re: [PATCH] userspace: audit: ausearch doesn't return entries for AppArmor events that exist in the log

[Richard Guy Briggs]

On 16/04/29, Vincas Dargis wrote:
> P.S. How do I actually reply to original thread that I did not
> received, since I just subscribed? I though I could maybe find raw
> message in archive https://www.redhat.com/archives/linux-audit/ but
> there aren't (no such message in 2014-May/Jun gz) . Oh how I hate
> using mailing lists so much... /rant.

The archives are a bit of a pain to download.  I have found that
chromium, wget, curl all get messed up half-text half-compressed files
from this archive server, but firefox is able to download them properly.
The messages are all in the archives.

You are welcome to your rant.  I quite like mailing lists and IRC.  I
hate most other social media and forums.  What do you suggest instead?

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635

Re: [PATCH] userspace: audit: ausearch doesn't return entries for AppArmor events that exist in the log

[Vincas Dargis]

2016.04.29 18:41, Richard Guy Briggs rašė:
> You are welcome to your rant.  I quite like mailing lists and IRC.  I
> hate most other social media and forums.  What do you suggest instead?

Hi,

I guess it's plain personal preference, but I just get annoyed once I have to deal with lists. That's why:

1. No explicit search function. Yes I know (_now_) that you can use your search engine with site: prefix, but that's too 
"hidden" feature, and one have to "know" that from somewhere. I don't know why Mailman or (whatever that list engine is) 
couldn't have at least search box that redirects to some search engine, if they don't want to incorporate local search 
engine by themselves.

2. No (easy?) way to continue thread if not subscribed earlier. Debian bug tracker, that is based on email, does in fact 
has "mbox" download link for specific post. But that's rarity I guess. In forums you just click "reply" button. In list, 
you have to download .gz archive (which apparently failed in my latest Firefox 46 case) and edit it to make single .mbox 
or whatever? ...

Steve suggested just to start new thread. Yeah, but.. that's kinda cheating, and issue still exists and I guess I am 
(too) picky for these kind of usability "papprecuts", hence, the rant :-) .

3. Once subscribed, you may get lot's of lot's of emails, even if you are interested in only that single topic (or some 
topics, maybe started long time ago), so you can just "lose"/forget/skip relevant reply. Meanwhile, forums have 
notification function you cane enable for topics you are "watching".

4. No formatting options. It's sometimes hard to read code or other technical text embedded inside plain text email. 
Meanwhile, using markdown/bbcode/whatever you can make technical keywords, file paths, or whole code blocks distinct 
from plain text. Makes eyes happy.

Forums also have issues, like.. having gazillion of accounts (that simple email activation in lists makes it much easier 
actually). Also, I imagine getting all list traffic by default might be good for maintainers who want to hear (ok, read) 
all relevant issues. In forums, you should click-through all "highlighted" topics manually? Oh my...

Anyway, this off-topic should be in Mailman list probably :-) . All these are actually small, "bells-and-wistles", 
"pappercut" issues I guess, but that's enough to make me *sigh* when I see I will have to deal with mailing list instead 
of full-fledged bug tracker or at least a forum.

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit