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

[Tyler Hicks]

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 at suse.com>
> > > > Signed-off-by: Tony Jones <tonyj at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/linux-audit/attachments/20140606/ff6f06b5/attachment.sig>