[apparmor] [patch] logparser.py: improve file vs. network event recognition

[Christian Boltz]

Hello,

Am Mittwoch, 30. November 2016, 17:19:13 CET schrieb Steve Beattie:
> On Sun, Nov 20, 2016 at 04:52:46PM +0100, Christian Boltz wrote:
> > sometimes network events come with an operation keyword looking like
> > file_perm which makes them look like file events. Instead of
> > ignoring
> > these events (which was a hotfix to avoid crashes), improve the type
> > detection.
[...]

> I have one concern about this patch:
> > [ 02-logparser-improve-file-vs-net.diff ]
> > 
> > --- utils/apparmor/logparser.py	2016-11-20 16:00:37.374243431 +0100
> > +++ utils/apparmor/logparser.py	2016-11-20 16:01:19.158060468 

> > +    def op_type(self, event):
> >          """Returns the operation type if known, unkown otherwise"""
> > 
> > -        operation_type = self.OPERATION_TYPES.get(operation,
> > 'unknown') -        return operation_type
> > +
> > +        if ( event['operation'].startswith('file_') or
> > event['operation'].startswith('inode_') or event['operation'] in
> > self.OP_TYPE_FILE_OR_NET ): 
> > +            # file or network event?
> > +            if event['family'] and event['protocol'] and
> > event['sock_type']: 
> > +                # 'unix' events also use
> > keywords like 'connect', but protocol is 0 and should therefore be
> > filtered out 
> > +                return 'net'
> > +            elif event['denied_mask']:
> > +                return 'file'
> > +            else:
> > +                raise AppArmorException('unknown file or network
> > event type')
> If you raise an exception here, then you're essentially aborting
> aa-logprof when it runs across a type that it doesn't know about.
> Given that there is sometimes a lag between what types are emitted
> and what types are understood, I'm not sure that this leaves a good
> experience for the user. 

Understood, but...

This exception will only be raised for events that come with a keyword 
we already know (listed in OP_TYPE_FILE_OR_NET, or starting with file_ or 
inode_) _and_ the event does not contain the details that are expected 
for file or network events.

Therefore I'm quite sure this exception will only be triggered if 
something really strange happens. I really want to get a bugreport about 
such cases ;-)  (BTW: The error message will include the log line 
triggering the exception.)

If you really want, I can change the 2.10 version to "return 'unknown'" 
for "strange" file or network events so that we can be 100% sure that 
this patch doesn't cause a regression in 2.10.2 - but IMHO (and 
according to the test_multi log examples), I consider this unlikely.


Unknown keywords will run into the "return 'unknown'" branch, which 
means the event will be checked for other types - or be ignored if no 
type matches.

> Yet, skipping log messages ought to be communicated to the user 
> somehow.

That's another can of worms ;-)


Regards,

Christian Boltz
-- 
Schau mal ins Listenarchiv und versuch mal, Bernds Antworten zu
zählen - und dann schreib das noch mal. Jetzt hast du wirklich dem
Falschen ans Bein gepinkelt!      [Jan Trippler in suse-linux]
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20161201/51d93a81/attachment.pgp>