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

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(a)suse.com&gt; > > > Signed-off-by: Tony Jones <tonyj(a)suse.de&gt; > > > > 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.