On Sun, Nov 20, 2016 at 04:52:46PM +0100, Christian Boltz wrote: > Hello, > > 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. > > In detail, this means: > - replace OPERATION_TYPES (which was basically a list of network event > keywords) with OP_TYPE_FILE_OR_NET (which is a list of keywords for > file and network events) > - change op_type() parameters to expect the whole event, not only the > operation keyword, and rebuild the type detection based on the event > details > - as a side effect, this simplifies the detection for file event > operations in parse_event_for_tree() > - remove workaround code from parse_event_for_tree() > > Also add 4 new testcases with log messages that were ignored before. > > References: > > a) various bugreports about crashes caused by unexpected operation keywords: > https://bugs.launchpad.net/apparmor/+bug/1466812 > https://bugs.launchpad.net/apparmor/+bug/1509030 > https://bugs.launchpad.net/apparmor/+bug/1540562 > https://bugs.launchpad.net/apparmor/+bug/1577051 > https://bugs.launchpad.net/apparmor/+bug/1582374 > > b) the summary bug for this patch > https://bugs.launchpad.net/apparmor/+bug/1613061 > > (that will make quite some --fixes for bzr commit ;-) > > I propose this patch for trunk and 2.10.
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 +0100
> +
> - def op_type(self, operation):
> + 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. Yet, skipping log messages ought to be communicated to
the user somehow.
> + else:
> + return 'unknown'
>
> def profile_exists(self, program):
> """Returns True if profile exists, False otherwise"""
>
--
Steve Beattie
<[email protected]>
http://NxNW.org/~steve/
signature.asc
Description: PGP signature
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
