As discussed in https://gitlab.com/apparmor/apparmor/merge_requests/395
RE_LOG_ALL should be more broad so that it doesn't accidently
overlook/ignore valid log events.
Instead of adding more and more known log formats to logparser.py,
simplify the regex to do only a basic check.
If we really hand over a line to libapparmor that isn't an AppArmor log
event, worst thing that can happen is that we waste a few milliseconds
for handing over that line to libapparmor, and get AA_RECORD_INVALID as a
result.
commit 7297e2f6a8 assumed that exec events
always have a "target=...", but this is only true for events in complain
mode. In enforce mode, the log line doesn't include "target=...".
This commits sets an empty target instead of erroring out on every exec
event in enforce mode.
Also adjust ask_exec() in aa.py to only update
hashlog[aamode][target_profile]['final_name'] if target_profile is set
(hashlog[aamode][''] doesn't exist and trying to write to it would error
out)
convert_regexp() needs to escape '(' and ')' in filenames taken from a
logfile to get rid of their special meaning, and to avoid a crash on
unbalanced parenthesis (which makes the regex invalid if they are not
escaped).
Note: The added tests include an example log line, but the tests itsself
don't/didn't trigger the crash because they don't call convert_regexp().
Fixes: https://bugs.launchpad.net/bugs/1835311
v2:
- parse partial log line broken at \n
- add testcase_dbus_10.* for partial log line
- remove quotes from testcasw_dbus_09.profile
The following log format has been seen in the wild, and currently results
in a RECORD_INVALID
[4835959.046111] audit: type=1107 audit(1561053426.749:186): pid=640 uid=103 auid=4294967295 ses=4294967295 msg='apparmor="ALLOWED" operation="dbus_method_call" bus="system" path="/org/freedesktop/systemd1" interface="org.freedesktop.systemd1.Manager" member="LookupDynamicUserByName" mask="send" name="org.freedesktop.systemd1" pid=20596 label="/usr/sbin/sshd" peer_pid=1 peer_label="unconfined"
exe="/usr/bin/dbus-daemon" sauid=103 hostname=? addr=? terminal=?'
Test parsing the above message with and without the \n embedded between
peer_label= and exec=
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
When dc010bc034 was
backported to the apparmor-2.13 branch (in commit
75236d62e2), it did not take into
account cb8c3377ba, which creates the
common/list_af_names.sh script as used in the test case, was not also
backported to the apparmor-2.13 branch.
Change the test case to get the list of network AF names via the same
make invocation taken by the utils/vim/create-apparmor.vim.py script
before the common/list_af_names.sh existed.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/391
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
The translated action character for Deny conflicted with the
untranslated action character for Finish in the Swedish translation.
Remote it, and hope for more action translations.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
finishing was always false, resulting in always returning 'NORMAL'.
Remove the variable, remove the unused condition - and change the only
place (aa-genprof) that expected a return value from do_logprof_pass()
to not expect it anymore.
found by Coverity, CID 198637
Rewrite log handling
logparser.py puts each log event on a big "stack" in self.pid. Later,
handle_children() in aa.py then converts that (named 'log' in aa.py) to
the prelog hasher.
This commit changes logparser.py to create a prelog-like structure itsself
(named hashlog), which
- removes one level of indirection
- probably saves some memory because the hashlog automatically
de-duplicates events
In aa.py, collapse_log() gets updated to work with hashlog. (There's
also a handle_hashlog() function in this patch series, but it didn't
survive the final patches ;-)
OTOH, the now superfluous code handling capability, network etc.
events gets removed from handle_children(). The remaining parts of
this function get split into ask_exec() and ask_addhat().
logparser.py gets a new function init_hashlog() to initialize hashlog
for each profile. It also gets changed to store capability, network etc.
events into hashlog instead of storing them in self.pid.
hashlog uses the full profile name as key, which is the first baby step
to support nested child profiles. (for now, collapse_log() still
splits the profile name into profile and hat.)
There are many more details, see the individual commits ;-)
See merge request apparmor/apparmor!377
Acked-by: Seth Arnold <seth.arnold@canonical.com>
If final_name still includes null-*, that's most likely caused by nested
execs which aren't supported by the tools yet. Ignoring them is better
than creating a useless null-* hat.
Note: The tools always had this restriction, so this is not a regression ;-)
Also note that test-libapparmor-test_multi expects that null-* hats get
created (which makes sense because the one-line log sniplets don't have
any exec indication), therefore add an optional parameter to keep this
behaviour for the tests.
Now that all log events arrive in hashlog, having a separate 'prelog' no
longer makes sense. Changing collapse_log() to accept 'hashlog' directly
removes that level of indirection.
It's only used by two functions:
- handle_hashlog() - writes to prelog, and now returns it
- collapse_log() - reads prelog, and now gets it as parameter
'final_name' by default is the profile name, but ask_exec() will change
it for the target profile (which is a null-* profile at this stage)
based on exec mode choice. ask_addhat() will also change it based on the
chosen hat.
Choosing "deny" or "unconfined" will result in an empty final_name and
ignoring these log events.
All other choices set final_name to the full profile name ("foo" for Px,
"foo//bar" for Cx, current profile for ix).
Also fix the order of handling log events - since ask_exec() changes the
hashlog final_name, it has to run first so that ask_addhat() (which
"only" adjusts the hat name in final_name) and handle_hashlog() can work
with the updated profile name.
Finally, update test-libapparmor-test_multi.py to ignore final_name when
checking if hashlog is empty, and fix the call order of ask_exec() etc.
profile and hat don't get used in the following lines, and later get
overwritten in the next round of the for loop.
The deleted code was last useful for setting the (dropped)
profile_changes array.
... and self.pid which is also unused.
This simple change also means to adjust all the code that uses ReadLog.
We get rid of log_pid in aa.py, and have to change lots of test-*
After all the changes in this branch, parse_event_for_tree always
returns None, which makes the "if event is not None" branch dead code.
This branch was the only place where add_to_tree() was called, therefore
remove this function.
This also makes self.pid and self.log unused. They'll be removed with
separate commits.
The only remaining job of handle_children() was to handle exec events.
(And recursively calling itsself if it hits nested log events, but
logparser.py never created such a log structure.)
Therefore:
- drop the dead code handling nested log (type != str)
- rename the remaining function to ask_exec()
- drop checks for typ = 'exec' (now done as part of the for loop
- drop the "else" branch for unknown event types
- change 'return' to 'continue' because ask_exec handles all exec events
in a loop instead of being called multiple times
- oh, and of course switch over to using hashlog
Finally, change do_logprof_pass() and the tests to call ask_exec()
instead of handle_children().
While on it, update a comment in test-translations.py which held the
last reference to handle_children().
The 'hat' and 'prog' variables are no longer needed, drop them.
Also no longer include denied_mask in the event - operation='exec' means
an exec event, no need to additionally hand over 'x' permissions.
Note: This commit introduces a "brain split", which means exec handling
is temporarily broken. Later commits will fix this.
parse_event_for_tree() always sets prog = 'HINT' and to_name = '' for
exec events.
The only exception is in the 'mandatory profile missing' case where
prog = aamode and to_name = target_profile, but I've never seen that in
practise.
This means the prog != 'HINT' branch in exec handling never gets used
and can be dropped.
Erroring out "if to_name:" also never gets used (to_name is always ''),
therefore drop it as well.
