[apparmor] [patch] logparser.py: change mask only for path events
Christian Boltz
apparmor at cboltz.de
Fri Apr 3 14:50:43 UTC 2015
Hello,
Am Dienstag, 31. März 2015 schrieb Seth Arnold:
> On Sun, Mar 29, 2015 at 11:04:10PM +0200, Christian Boltz wrote:
> > this patch moves the code that does the c -> a and d -> w
> > replacement
> > in denied_mask and requested_mask so that it only runs for path
> > events, but not for other events (like dbus and ptrace). The
> > validate_log_mode() and log_str_to_mode() calls are also moved.
> >
> > Technically, this means moving code from parse_event() to the path
> > section in add_event_to_tree().
> >
> > This also means aa-logprof no longer crashes if it hits a ptrace or
> > dbus event in the log.
> >
> > The "if dmask:" and "if rmask:" checks are removed - if a path event
> > doesn't have these two, it is totally broken and worth a aa-logprof
> > crash ;-)
> >
> > Also adjust the parse_event() tests to expect the "raw" mask instead
> > of a set.
> >
> >
> > This patch fixes
> > https://bugs.launchpad.net/apparmor/+bug/1426651 and
> > https://bugs.launchpad.net/apparmor/+bug/1243932
> >
> >
> > I manually tested that
> > - c and d log events are still converted to a and w
> > - ptrace events no longer crash aa-logprof
> >
> > Note: add_event_to_tree() is not covered by tests.
>
> This is a hard one; the code changes all seem plausible enough and
> they do fix very real bugs that are annoying users today, but the
> changes to the datatypes that result in the test changes really
> worries me. Python won't tell us if something is left somewhere in
> the program that requires them to be sets rather than strings.
Well, actually it does - if 'mode' is a string, it will throw an
exception for things like
if mode & str_to_mode('x'):
I just learned that the hard way ;-)
> and the only solution is to exhaustively test the program.
>
> Would you say you've tested it exhaustively after this patch?
In the meantime I tend to say yes - luckily the testing for the doubled
arrow in named exec brought this up :-)
Here's v2, with log_str_to_mode() calls for e['operation'] == 'exec'
added. This time I even checked logparser.py to make sure there are no
more sections that use denied_mask or request_mask ;-)
[ 30-logparser-change-mask-only-for-path-events.diff ]
=== modified file utils/apparmor/logparser.py
--- utils/apparmor/logparser.py 2015-04-03 16:33:17.913350220 +0200
+++ utils/apparmor/logparser.py 2015-04-03 16:24:41.914848079 +0200
@@ -1,5 +1,6 @@
# ----------------------------------------------------------------------
# Copyright (C) 2013 Kshitij Gupta <kgupta8592 at gmail.com>
+# Copyright (C) 2015 Christian Boltz <apparmor at cboltz.de>
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of version 2 of the GNU General Public
@@ -112,34 +113,14 @@
ev['task'] = event.task
ev['info'] = event.info
ev['error_code'] = event.error_code
- dmask = event.denied_mask
- rmask = event.requested_mask
+ ev['denied_mask'] = event.denied_mask
+ ev['request_mask'] = event.requested_mask
ev['magic_token'] = event.magic_token
if ev['operation'] and self.op_type(ev['operation']) == 'net':
ev['family'] = event.net_family
ev['protocol'] = event.net_protocol
ev['sock_type'] = event.net_sock_type
LibAppArmor.free_record(event)
- # Map c (create) to a and d (delete) to w, logprof doesn't support c and d
- if rmask:
- rmask = rmask.replace('c', 'a')
- rmask = rmask.replace('d', 'w')
- if not validate_log_mode(hide_log_mode(rmask)):
- raise AppArmorException(_('Log contains unknown mode %s') % rmask)
- if dmask:
- dmask = dmask.replace('c', 'a')
- dmask = dmask.replace('d', 'w')
- if not validate_log_mode(hide_log_mode(dmask)):
- raise AppArmorException(_('Log contains unknown mode %s') % dmask)
- #print('parse_event:', ev['profile'], dmask, ev['name2'])
- mask, name = log_str_to_mode(ev['profile'], dmask, ev['name2'])
-
- ev['denied_mask'] = mask
- ev['name2'] = name
-
- mask, name = log_str_to_mode(ev['profile'], rmask, ev['name2'])
- ev['request_mask'] = mask
- ev['name2'] = name
if not ev['time']:
ev['time'] = int(time.time())
@@ -254,6 +235,10 @@
if profile != 'null-complain-profile' and not self.profile_exists(profile):
return None
if e['operation'] == 'exec':
+ # convert rmask and dmask to mode arrays
+ e['denied_mask'], e['name2'] = log_str_to_mode(e['profile'], e['denied_mask'], e['name2'])
+ e['request_mask'], e['name2'] = log_str_to_mode(e['profile'], e['request_mask'], e['name2'])
+
if e.get('info', False) and e['info'] == 'mandatory profile missing':
self.add_to_tree(e['pid'], e['parent'], 'exec',
[profile, hat, aamode, 'PERMITTING', e['denied_mask'], e['name'], e['name2']])
@@ -267,7 +252,25 @@
e['operation'] in ['open', 'truncate', 'mkdir', 'mknod', 'chmod', 'rename_src',
'rename_dest', 'unlink', 'rmdir', 'symlink_create', 'link',
'sysctl', 'getattr', 'setattr', 'xattr'] ):
- #print(e['operation'], e['name'])
+
+ # Map c (create) to a and d (delete) to w (logging is more detailed than the profile language)
+ rmask = e['request_mask']
+ rmask = rmask.replace('c', 'a')
+ rmask = rmask.replace('d', 'w')
+ if not validate_log_mode(hide_log_mode(rmask)):
+ raise AppArmorException(_('Log contains unknown mode %s') % rmask)
+
+ dmask = e['denied_mask']
+ dmask = dmask.replace('c', 'a')
+ dmask = dmask.replace('d', 'w')
+ if not validate_log_mode(hide_log_mode(dmask)):
+ raise AppArmorException(_('Log contains unknown mode %s') % dmask)
+
+ # convert rmask and dmask to mode arrays
+ e['denied_mask'], e['name2'] = log_str_to_mode(e['profile'], dmask, e['name2'])
+ e['request_mask'], e['name2'] = log_str_to_mode(e['profile'], rmask, e['name2'])
+
+ # check if this is an exec event
is_domain_change = False
if e['operation'] == 'inode_permission' and (e['denied_mask'] & AA_MAY_EXEC) and aamode == 'PERMITTING':
following = self.peek_at_next_log_entry()
=== modified file utils/test/test-capability.py
--- utils/test/test-capability.py 2015-04-03 16:33:17.914350162 +0200
+++ utils/test/test-capability.py 2015-04-03 15:58:16.782390170 +0200
@@ -102,8 +102,8 @@
parsed_event = parser.parse_event(event)
self.assertEqual(parsed_event, {
- 'request_mask': set(),
- 'denied_mask': set(),
+ 'request_mask': None,
+ 'denied_mask': None,
'error_code': 0,
'magic_token': 0,
'parent': 0,
=== modified file utils/test/test-logparser.py
--- utils/test/test-logparser.py 2015-04-03 16:33:17.914350162 +0200
+++ utils/test/test-logparser.py 2015-04-03 15:58:16.783390112 +0200
@@ -26,7 +26,7 @@
self.assertEqual(parsed_event['name'], '/home/www/foo.bar.in/httpdocs/apparmor/images/test/image 1.jpg')
self.assertEqual(parsed_event['profile'], '/usr/sbin/httpd2-prefork//vhost_foo')
self.assertEqual(parsed_event['aamode'], 'PERMITTING')
- self.assertEqual(parsed_event['request_mask'], set(['w', 'a', '::w', '::a']))
+ self.assertEqual(parsed_event['request_mask'], 'wc')
self.assertIsNotNone(ReadLog.RE_LOG_v2_6_audit.search(event))
self.assertIsNone(ReadLog.RE_LOG_v2_6_syslog.search(event))
@@ -37,7 +37,7 @@
self.assertEqual(parsed_event['name'], '/home/foo/.bash_history')
self.assertEqual(parsed_event['profile'], 'foo bar')
self.assertEqual(parsed_event['aamode'], 'PERMITTING')
- self.assertEqual(parsed_event['request_mask'], set(['r', 'w', 'a','::r' , '::w', '::a']))
+ self.assertEqual(parsed_event['request_mask'], 'rw')
self.assertIsNotNone(ReadLog.RE_LOG_v2_6_audit.search(event))
self.assertIsNone(ReadLog.RE_LOG_v2_6_syslog.search(event))
@@ -49,7 +49,7 @@
self.assertEqual(parsed_event['name'], '/dev/tty')
self.assertEqual(parsed_event['profile'], '/home/cb/linuxtag/apparmor/scripts/hello')
self.assertEqual(parsed_event['aamode'], 'PERMITTING')
- self.assertEqual(parsed_event['request_mask'], set(['r', 'w', 'a', '::r', '::w', '::a']))
+ self.assertEqual(parsed_event['request_mask'], 'rw')
self.assertIsNone(ReadLog.RE_LOG_v2_6_audit.search(event))
self.assertIsNotNone(ReadLog.RE_LOG_v2_6_syslog.search(event))
@@ -61,7 +61,7 @@
self.assertEqual(parsed_event['name'], '/usr/bin/')
self.assertEqual(parsed_event['profile'], '/home/simi/bin/aa-test')
self.assertEqual(parsed_event['aamode'], 'PERMITTING')
- self.assertEqual(parsed_event['request_mask'], set(['r', '::r']))
+ self.assertEqual(parsed_event['request_mask'], 'r')
self.assertIsNone(ReadLog.RE_LOG_v2_6_audit.search(event))
self.assertIsNotNone(ReadLog.RE_LOG_v2_6_syslog.search(event))
@@ -75,7 +75,7 @@
'aamode': 'ERROR', # aamode for disconnected paths overridden aamode in parse_event()
'active_hat': None,
'attr': None,
- 'denied_mask': {'r', '::r'},
+ 'denied_mask': 'r',
'error_code': 13,
'info': 'Failed name lookup - disconnected path',
'magic_token': 0,
@@ -85,7 +85,7 @@
'parent': 0,
'pid': 25333,
'profile': '/sbin/klogd',
- 'request_mask': {'r', '::r'},
+ 'request_mask': 'r',
'resource': 'Failed name lookup - disconnected path',
'task': 0,
'time': 1424425690
Regards,
Christian Boltz
--
> btw: Entnehme ich Deinen Worten, daß XP und Sicherheit
> in irgendeiner Weise eine Verbindung eingehen können?
Klar können sie. eine logische XOR-Verbindung.
[U. Ohse und S. Posner in dasr]
More information about the AppArmor
mailing list