From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tyler Hicks Subject: Re: [PATCH] userspace: audit: ausearch doesn't return entries for AppArmor events that exist in the log Date: Fri, 6 Jun 2014 13:46:48 -0500 Message-ID: <20140606184648.GA15921@boyd> References: <53866422.5010709@suse.de> <31153503.SQnCbJNRtA@x2> <20140530201644.GA22335@boyd> <9810096.ghxOlbMYMG@x2> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============7664119594732356970==" Return-path: In-Reply-To: <9810096.ghxOlbMYMG@x2> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-audit-bounces@redhat.com Errors-To: linux-audit-bounces@redhat.com To: Steve Grubb Cc: wpreston@suse.com, seth.arnold@canonical.com, linux-audit@redhat.com List-Id: linux-audit@redhat.com --===============7664119594732356970== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="FCuugMFkClbJLl1L" Content-Disposition: inline --FCuugMFkClbJLl1L Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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 us= ing > > > > the > > > > common_lsm_audit() call but the audit userspace parsing code expect= s to > > > > see > > > > an SELinux tclass field. This patch doesn't address the lack of sup= port > > > > 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. > > > >=20 > > > > Based-on-work-by: William Preston > > > > Signed-off-by: Tony Jones > > >=20 > > > I was looking at this patch and was wondering something. Does AppArmor > > > produce AUDIT_AVC events? > >=20 > > It does. Here's an odd ball that I picked out of my audit log: >=20 > Uh-oh. I gave out the 1500 - 1599 block of events to App Armor so that th= is=20 > problem would never happen. >=20 > 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.=20 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 >=20 > When you have an event of the same number, it has to have the same fields= in=20 > the same order with same value representation. As you can see the reporti= ng=20 > tools is sensitive to this because they are optimized to handle huge log = files.=20 > Hmmm.... >=20 >=20 > > type=3DAVC msg=3Daudit(1400295012.391:11143): apparmor=3D"DENIED" > > operation=3D"mount" info=3D"failed type match" profile=3D"lxc-container= -default" > > name=3D"/sys/fs/cgroup/systemd/" pid=3D15761 comm=3D"mount" fstype=3D"c= group" > > srcname=3D"systemd" flags=3D"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? > >=20 > > As you can see above, they already flow through parse_avc(). >=20 > 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 w= ould=20 > then be free to include the field it needs in the order it needs. The aus= earch=20 > would get a function parse_aa_decision. That function would stuff a struc= t=20 > specially tuned for AA usage. Aureport would gain a new report. >=20 >=20 > > I spent a little time today getting a patch together to parse the > > 'apparmor=3D"DENIED" operation=3D"mount"' portion parsed to fill > > an.avc_result and an.avc_perm. > >=20 > > 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. >=20 > Well...I wished it had been clear a long time ago that the 1500 block was= free=20 > to do anything with for AA's needs so we don't overload the usage. Not su= re=20 > 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. =20 SELinux audit events are separated from other LSM audit events by the "avc: denied {" prefix. =20 AppArmor audit events are separated from other LSM audit events by the "apparmor=3D" prefix. =20 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 --FCuugMFkClbJLl1L Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBCgAGBQJTkgyYAAoJENaSAD2qAscKosYQALZt7pmo5jAWko4ALA7mcOcy 0mNtqA4UxFvTZGzHKNYeg1DlfFsRtQ8+Fo5/g+zclep2XSbbVwFrHEDLKnNKFTMM ZzUXctRu3iYFryfjdxHLUwPRUJu69MAh5NnRhBRqZPmOfq11GOB9ODdzmjaJKUcY 3qgkuXMxJsv5AvQIXUhXxrOPaHSU+bmDT80CX1x3nkdzjvrZ0LCKoRbafJyJ4/oZ WdAFUV8EQ03/yrC5teh9AXoywph2F1tqF+3TWIHag9PUdu1AX6yTlGB3h35DZvWb YtpKaqR1fyJ0zRFTFiBr35nIWQVR08Tc/62m8fsv3cpNLTDeNXmwKxOBuLoZ6oEK vgAo8HkqIFrmIlq4fqChCnDphe5JMH+HalLEe9ueEWIoQfII7ugLo0DNteZXeaQN WC48l5VfEC+nQ/RmYZ2WsdgWSdATjO9X1GXzk8A1C/Y1c15Z9nU8UxU0VYN44mT+ fJEZrCE97laHpP7VCon1uXLnaFZiMSb+SJ/zFUnlU8+ohzyR1xncDQ6CY9k0NejV xa0ALHKmCZry1tVeLZ8NSTlAxkZtQv81HdX8UCz8RbggunSHVZ9Tnsz3kKDhEIFs qPg1ZWugbb5zsLnvdFI1iHw3nZGiNjg7tAE1WIcIroNwPFG7Ld7aKHXwMrA5weGx dcFNOTVqaOFrINR0pHs1 =DALs -----END PGP SIGNATURE----- --FCuugMFkClbJLl1L-- --===============7664119594732356970== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline --===============7664119594732356970==--