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

Tyler Hicks
2014-06-06T18:46:48Z

[-- 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 --]