[apparmor] [patch] [1/2] support 'owner' file events in logparser.py

[Christian Boltz]

Hello,

Am Montag, 31. Juli 2017, 21:27:23 CEST schrieb Seth Arnold:
> On Sun, Jul 30, 2017 at 10:51:38PM +0200, Christian Boltz wrote:
> > logparser.py failed to notice if file events are owner-only in
> > modern
> > audit.log (using fsuid=... and ouid=...).
> > 
> > This patch adds a comparison of fsuid and ouid and marks file events
> > as 'owner' if they match.
> > 
> > Note that log events without fsuid=... or ouid=... will have
> > 18446744073709551615 as fsuid / ouid value (that's 2^64 - 1).
> > 'None' would clearly be better ;-)
> > 
> > 
> > References: https://bugs.launchpad.net/apparmor/+bug/1538340
> > 
> > 
> > I propose this patch for trunk and 2.11
> > (unfortunately it doesn't work on 2.10)
> > 
> > 
> > [ 02-logparser-owner.diff ]
> > 
> > === modified file ./utils/apparmor/logparser.py
> > --- utils/apparmor/logparser.py 2017-07-16 21:43:30.718865501 +0200
> > +++ utils/apparmor/logparser.py 2017-07-30 21:56:10.829026386 +0200
> > @@ -118,6 +118,10 @@
> > 
> >          ev['protocol'] = event.net_protocol
> >          ev['sock_type'] = event.net_sock_type
> > 
> > +        if event.ouid != 18446744073709551615:  # 2^64 - 1
> > +            ev['fsuid'] = event.fsuid
> > +            ev['ouid'] = event.ouid
> > +
> 
> Why is this one UID handled magically?

My *guess* is that it is actually -1, but either libapparmor or the 
python bindings handle it as unsigned 64bit integer - and 
2^64 -1 == 18446744073709551615

I don't say this is perfect (it's probably a bug), but until someone 
fixes libapparmor or the python bindings, we'll have to live with this 
number. And even after fixing libapparmor, we should probably carry it 
for a while to be compatible with older libapparmor versions.
(After making it a signed int, we need to check for -1.)

> >          if ev['operation'] and ev['operation'] == 'signal':
> >              ev['signal'] = event.signal
> >              ev['peer'] = event.peer
> > 
> > @@ -270,6 +274,13 @@
> > 
> >              if not validate_log_mode(hide_log_mode(dmask)):
> >                  raise AppArmorException(_('Log contains unknown
> >                  mode %s') % dmask)
> > 
> > +            if e.get('ouid') is not None and e['fsuid'] ==
> > e['ouid']: +                # mark as "owner" event
> > +                if '::' not in rmask:
> > +                    rmask = '%s::' % rmask
> > +                if '::' not in dmask:
> > +                    dmask = '%s::' % dmask
> > +
> 
> Just to double-check, the whole test suite still runs after this
> change? It feels kind of big :)

With the test_multi/*.profile adjusted in the second patch: yes :-)

The most important "trick" was to set ev['fsuid'] and ev['ouid'] only if 
they are != -1 (or 2^64 -1, see above). Without this condition, I'd have 
to change the logparser results in several test-*.py files instead of 
only in test-logparser.py.


Oh, BTW - as you already guessed, the superfluous trailing whitespace in 
the second patch is caused by Konsole and/or KMail.


Regards,

Christian Boltz
-- 
> Do not say it too loud, or next year systemd can on-demand install
> RPMs as PackageKit & rpm replacement.
Don't worry, more likely systemd will implement any other functionality
so that you simply don't need to install any other packages anymore.
[> Marcus Meissner and Ruediger Meier in opensuse-packaging]
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20170731/0e75a9c1/attachment.pgp>