move path log event handling to hashlog

In logparser.py parse_event_for_tree, convert path handling to hashlog.
While on it, include 'owner' as part of hashlog so that aa.py doesn't
need to guess.

Also switch to a simple for loop instead of using log_str_to_mode() from
aamode.py to convert denied_mask to hasher keys (which would have been
needed to allow merging of several log events for the same path anyway).
Note that the check for 'mrawlk' (intentionally without 'x') is more
strict than the validate_log_mode(), but it should still cover all file
permissions. (validate_log_mode() also allows things like 'Px', which
we'll never hit in a logfile.)

In aa.py collapse_log() update the handling of path events to match the
additional [owner] key in hashlog/prelog. This makes the owner detection
in collapse_log() superfluous.

In aa.py handle_children(), remove 'path' handling from the 'path' or
'exec' section, and add an 'if True:' to avoid lots of whitespace
changes.

In aamode.py, drop the now unused split_mode() function, and
AA_OTHER_REMOVE() that was only used by split_mode().

Finally, remove sample log events with null-* hats from the list of
known failures in test-libapparmor-test_multi.py (we no longer filter
out null-* hats), and fix whitespace in two expected profiles.

libraries/libapparmor/testsuite/test_multi/syslog_datetime_10.profile

@@ -1,4 +1,5 @@
 /usr/lib/virtualbox/VBoxSVC {
+
   ^null-2d {
     /sys/class/power_supply/ r,
 

libraries/libapparmor/testsuite/test_multi/testcase_network_send_receive.profile

@@ -1,4 +1,5 @@
 /usr/bin/nginx-amplify-agent.py {
+
   ^null-/bin/dash {
     network inet stream,
 

utils/apparmor/aa.py

@@ -37,8 +37,6 @@ from apparmor.common import (AppArmorException, AppArmorBug, open_file_read, val
 
 import apparmor.ui as aaui
 
-from apparmor.aamode import str_to_mode, split_mode
-
 from apparmor.regex import (RE_PROFILE_START, RE_PROFILE_END,
                             RE_ABI, RE_PROFILE_ALIAS,
                             RE_PROFILE_BOOLEAN, RE_PROFILE_VARIABLE, RE_PROFILE_CONDITIONAL,
@@ -990,7 +988,7 @@ def handle_children(profile, hat, root):
                     # As unknown hat is denied no entry for it should be made
                     return None
 
-            elif typ == 'path' or typ == 'exec':
+            elif typ == 'exec':
                 # If path or exec then we (should) have pid, profile, hat, program, mode, details and to_name
                 pid, p, h, prog, aamode, mode, detail, to_name = entry[:8]
                 if not mode:
@@ -1003,26 +1001,13 @@ def handle_children(profile, hat, root):
 
                 # Give Execute dialog if x access requested for something that's not a directory
                 # For directories force an 'ix' Path dialog
-                do_execute = False
+                domainchange = 'change'
                 exec_target = detail
 
-                if mode & str_to_mode('x'):
+                if True:
                     if os.path.isdir(exec_target):
                         raise AppArmorBug('exec permissions requested for directory %s. This should not happen - please open a bugreport!' % exec_target)
-                    elif typ != 'exec':
-                        raise AppArmorBug('exec permissions requested for %(exec_target)s, but mode is %(mode)s instead of exec. This should not happen - please open a bugreport!' % {'exec_target': exec_target, 'mode':mode})
-                    else:
-                        do_execute = True
-                        domainchange = 'change'
 
-                if mode and mode != str_to_mode('x'):  # x is already handled in handle_children, so it must not become part of prelog
-                    path = detail
-
-                    if prelog[aamode][profile][hat]['path'].get(path, False):
-                        mode |= prelog[aamode][profile][hat]['path'][path]
-                    prelog[aamode][profile][hat]['path'][path] = mode
-
-                if do_execute:
                     if not aa[profile][hat]:
                         continue  # ignore log entries for non-existing profiles
 
@@ -1859,29 +1844,17 @@ def collapse_log():
                 log_dict[aamode][profile][hat] = ProfileStorage(profile, hat, 'collapse_log()')
 
                 for path in prelog[aamode][profile][hat]['path'].keys():
-                    mode = prelog[aamode][profile][hat]['path'][path]
+                    for owner in prelog[aamode][profile][hat]['path'][path]:
+                        mode = set(prelog[aamode][profile][hat]['path'][path][owner].keys())
 
-                    user, other = split_mode(mode)
+                        # logparser sums up multiple log events, so both 'a' and 'w' can be present
+                        if 'a' in mode and 'w' in mode:
+                            mode.remove('a')
 
-                    # logparser.py doesn't preserve 'owner' information, see https://bugs.launchpad.net/apparmor/+bug/1538340
+                        file_event = FileRule(path, mode, None, FileRule.ALL, owner=owner, log_event=True)
-                    # XXX re-check this code after fixing this bug
-                    if other:
-                        owner = False
-                        mode = other
-                    else:
-                        owner = True
-                        mode = user
 
-                    # python3 aa-logprof -f <(echo '[55826.822365] audit: type=1400 audit(1454355221.096:85479): apparmor="ALLOWED" operation="file_receive" profile="/usr/sbin/smbd" name="/foo.png" pid=28185 comm="smbd" requested_mask="w" denied_mask="w" fsuid=100 ouid=100')
+                        if not is_known_rule(aa[profile][hat], 'file', file_event):
-                    # happens via log_str_to_mode() called in logparser.py parse_event_for_tree()
+                            log_dict[aamode][profile][hat]['file'].add(file_event)
-                    # XXX fix this in the log parsing!
-                    if 'a' in mode and 'w' in mode:
-                        mode.remove('a')
-
-                    file_event = FileRule(path, mode, None, FileRule.ALL, owner=owner, log_event=True)
-
-                    if not is_known_rule(aa[profile][hat], 'file', file_event):
-                        log_dict[aamode][profile][hat]['file'].add(file_event)
 
                 for cap in prelog[aamode][profile][hat]['capability'].keys():
                     cap_event = CapabilityRule(cap, log_event=True)

utils/apparmor/aamode.py

@@ -20,13 +20,6 @@ def AA_OTHER(mode):
         other.add('::%s' % i)
     return other
 
-def AA_OTHER_REMOVE(mode):
-    other = set()
-    for i in mode:
-        if '::' in i:
-            other.add(i[2:])
-    return other
-
 AA_MAY_EXEC = set('x')
 AA_MAY_WRITE = set('w')
 AA_MAY_READ = set('r')
@@ -127,15 +120,6 @@ def hide_log_mode(mode):
     mode = mode.replace('::', '')
     return mode
 
-def split_mode(mode):
-    user = set()
-    for i in mode:
-        if not '::' in i:
-            user.add(i)
-    other = mode - user
-    other = AA_OTHER_REMOVE(other)
-    return user, other
-
 def log_str_to_mode(profile, string, nt_name):
     mode = str_to_mode(string)
     # If contains nx and nix

utils/apparmor/logparser.py

@@ -66,6 +66,7 @@ class ReadLog:
             'capability':   {},  # flat, no hasher needed
             'dbus':         hasher(),
             'network':      hasher(),
+            'path':         hasher(),
             'ptrace':       hasher(),
             'signal':       hasher(),
         }
@@ -256,19 +257,30 @@ class ReadLog:
             if not validate_log_mode(hide_log_mode(dmask)):
                 raise AppArmorException(_('Log contains unknown mode %s') % dmask)
 
+            owner = False
+
+            if '::' in dmask:
+                # old log styles used :: to indicate if permissions are meant for owner or other
+                (owner_d, other_d) = dmask.split('::')
+                if owner_d and other_d:
+                    raise AppArmorException('Found log event with both owner and other permissions. Please open a bugreport!')
+                if owner_d:
+                    dmask = owner_d
+                    owner = True
+                else:
+                    dmask = other_d
+
             if e.get('ouid') is not None and e['fsuid'] == e['ouid']:
-                # mark as "owner" event
+                # in current log style, owner permissions are indicated by a match of fsuid and ouid
-                if '::' not in rmask:
+                owner = True
-                    rmask = '%s::' % rmask
-                if '::' not in dmask:
-                    dmask = '%s::' % dmask
 
-            # convert rmask and dmask to mode arrays
+            for perm in dmask:
-            e['denied_mask'],  e['name2'] = log_str_to_mode(e['profile'], dmask, e['name2'])
+                if perm in 'mrwalk':  # intentionally not allowing 'x' here
-            e['request_mask'], e['name2'] = log_str_to_mode(e['profile'], rmask, e['name2'])
+                    self.hashlog[aamode][full_profile]['path'][e['name']][owner][perm] = True
+                else:
+                    raise AppArmorException(_('Log contains unknown mode %s') % dmask)
 
-            return(e['pid'], e['parent'], 'path',
+            return None
-                             [profile, hat, prog, aamode, e['denied_mask'], e['name'], ''])
 
         elif e['operation'] == 'capable':
             self.hashlog[aamode][full_profile]['capability'][e['name']] = True

utils/test/test-libapparmor-test_multi.py

@@ -154,27 +154,6 @@ log_to_profile_known_failures = [
     'testcase01',
     'testcase12',
     'testcase13',
-
-    # null-* hats get ignored by handle_children() if it didn't see an exec event for that null-* hat
-    'syslog_datetime_01',
-    'syslog_datetime_02',
-    'syslog_datetime_03',
-    'syslog_datetime_04',
-    'syslog_datetime_05',
-    'syslog_datetime_06',
-    'syslog_datetime_07',
-    'syslog_datetime_08',
-    'syslog_datetime_09',
-    'syslog_datetime_10',
-    'syslog_datetime_11',
-    'syslog_datetime_12',
-    'syslog_datetime_13',
-    'syslog_datetime_14',
-    'syslog_datetime_15',
-    'syslog_datetime_16',
-    'syslog_datetime_17',
-    'syslog_datetime_18',
-    'testcase_network_send_receive',
 ]
 
 # tests that cause crashes or need user interaction (will be skipped)