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]
Description: This is a digitally signed message part.
-- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor