All the calling code (directly or indirectly) uses write_flags=True,
therefore drop the parameter to simplify the code.
A few tests called get_header() with write_flags=False. Adjust or drop
those tests.
Note: to keep the diff readable, the test changes are as small as
possible. The next commit will cleanup the now-superfluous write_flags
values in the tests.
.. instead of preserving the original leading whitespace.
This change affects the behaviour of aa-complain, aa-enforce and aa-audit.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/757
Acked-by: John Johansen <john@jjmx.net>
... instead of always writing hats as '^hat'.
When writing a profile, prepending '^' or 'hat' to a hat name moves from
aa.py write_piece() to ProfileStorage.get_header().
Also extend cleanprof_test.* with 'hat bar {...}'.
... and into parse_profile_start_line() (which is used by
ProfileStorage.parse()).
With this change, the section handling RE_PROFILE_HAT_DEF in
parse_profile_data() becomes superfluous.
A nice side effect is that two simple_tests parse failures get
accidently ;-) fixed.
... and add some tests for other error conditions that don't imply
nested childs, so that the intended failure gets tested.
(This is probably a leftover of the `hat == profile` -> `hat = None`
(while not in a hat/child profile) change.)
... and make them class functions of ProfileStorage.
parse_profile_start_to_storage() gets renamed to parse().
Also move the tests for parse_profile_start() and
parse_profile_start_to_storage() to test-profile-storage.py.
The 'profile' flag means "this profile is a profile or a child profile,
but not a hat". Since that's true for most cases, rename the flag to
'is_hat'.
Note that `'profile' == True` translates to `'is_hat' == False`
Also adjust all code to switch from 'profile' to 'is_hat'.
This value is True if we are in a child profile (not: hat), but that's
information we get "for free", so there's no need to hand it around.
Besides that, it was wrongly set to False for main profiles (which are
not hats).
Remove the pps_set_profile return value from parse_profile_start(), and
always assume True unless we were parsing a hat. For completeness,
explicitely set it to False when parsing a hat.
To make sure child profiles and hats don't get mixed up, add a child
profile to cleanprof_test.{in,out}.
test-libapparmor-test_multi.py always interpreted foo//bar as being
a hat, therefore explicitely mark them as such. (Technically not really
needed since this is the default, but it helps to make things clear.)
Change the tools to use merged profile names (`var['foo//bar']`) instead of the profile/hat layout (`var[profile][hat]`) in many places. Also storage gets moved to ProfileList instead of using a hasher.
Already changed places (in this MR) are parsing profiles, writing profiles, handling and storing of extra profiles, log handling and asking the user about profile additions.
Remaining usage of the `var[profile][hat]` layout are the `aa` and `original_aa` hashers, they'll be replaced in a separate MR.
See the individual commits for details. I'd also recommend to do the review on the individual commits, because the big diff is probably unreadable ;-)
While this is a big chain of changes, each commit contains working code, converting between the two storage layouts with `split_to_merged()` and `merged_to_split()` as needed, with merged layout "bubbling up" in more and more functions.
The long-term goal of these changes is to enable support for nested child profiles in the tools, but - one step after the other ;-)
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/736
Acked-by: John Johansen <john.johansen@canonical.com>
The function decides on the filename of a profile, therefore use
'filename' as variable name instead of the somewhat confusing 'profile'
and 'full_profilename'.
If an include file includes itsself (for example if local/foo has
'#include <local/foo>'), print a warning instead of calling
load_include() again and again.
This fixes a crash when hitting such a case:
RecursionError: maximum recursion depth exceeded while calling a Python object
Fixes: https://bugzilla.suse.com/show_bug.cgi?id=1184779 for the tools.
The parser will also need a fix.
if a profile doesn't have an attachment specified and the profile name
starts with '/', set the attachment to the profile name. This allows to
have one add_profile() call instead of two very similar ones.
exit early if profile_data is empty (which means we did read an empty
file). This allows to simplify the if conditions to "if active_profile:"
and "else:".
... and adjust all callers and the tests.
For bonus points ;-) this also removes a hasher usage, and extends the
test to check that only the expected profile gets created.
... instead of the 'extras' hasher.
Also adjust all code that previously used 'extras' to use
'extra_profiles'. This affects get_profile() and read_profile().
Add a prof_storage parameter to add_profile() to hand over the actual
profile data/rules as ProfileStorage.
Also adjust several tests to hand over a (dummy) ProfileStorage object.
Note: For now, the parameter is optional because it needs some more changes
in aa.py to be really useable. This will change in a later commit.
... instead of converting log_dict to traditional [profile][hat] layout
in do_logprof_pass().
A nice side effect is that we get sorting the main profile before its
hats for free and can remove the sorting code.
Also update a comment in ask_rule_questions().
Finally, adjust aa-mergeprof so that it hands over a merged log_dict (using
split_to_merged())
... instead of the old [profile][hat] structure.
This needs changes in do_logprof_pass() when calling ask_the_questions()
(using merged_to_split() for now).
Also adjust test-libapparmor-test_multi.py logfile_to_profile() to
expect the merged structure.
... and convert them back to the [profile][hat] layout at the end so
that callers still get the expected result.
As a side effect, log_dict no longer needs to be a hasher().
... instead of the old [profile][hat] structure.
This needs changes in read_profile() (now using the merged profile name)
and attach_profile_data() (using merged_to_split() for now).
Also adjust test-aa.py to expect the merged structure.
Change parse_profile_data() to internally use merged profile names
(`foo//bar`) instead of separate profile and hat, and only split it up
again to the [profile][hat] layout at the very end with
merged_to_split().
A nice side effect is that we get rid of a hasher() usage.
parse_profile_data() also gets changed to use `hat = None` (instead of
`hat = profile`) if not inside a child profile. As a result,
parse_profile_start() and one of its tests need a small change.
Besides that small change, calling code should not see a difference, and
the tests also stay working.
... by adding some new tests, and by marking two lines as "pragma: no
branch" because I didn't find a testcase that doesn't let them continue
with the next line.
Finally, remove severity.py from the "not 100% covered" list in
test/Makefile.
Add tests with invalid type to ensure error handling works as expected.
Merge branch 'cboltz/cboltz-profile-storage-tests'
[Fixed conflict with prior change to utils/test/test-profile-storage.py]
Acked-by: Steve Beattie <steve@nxnw.org>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/735
Also adjust the calling code to use get_header() instead of
write_header().
Finally, move the tests to test-profile-storage.py and do a few
adjustments needed by the change. Part of these adjustments is to hand
over empty params with the correct type instead of just "None".
... instead of profile_data\[profile\]\[profile\]\['name'\] which is not always correct.
Note: setting 'name' will be fixed in a later commit/MR, but don't hold your breath for it ;-)
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/733
Acked-by: John Johansen <john.johansen@canonical.com>
AUDIT events get skipped when parsing the log (they are not relevant for updating a profile), therefore stop returning an always-empty AUDIT section when reading the log.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/731
Acked-by: John Johansen <john.johansen@canonical.com>
... instead of profile_data[profile][profile]['name'] which is not
always correct.
Note: setting 'name' will be fixed in a later commit, but don't hold
your breath for it ;-)
AUDIT events get skipped when parsing the log (they are not relevant for
updating a profile), therefore stop returning an always-empty AUDIT
section when reading the log.
This needed several changes because so far data for all includes was
stored in include[]. However, preamble data for everything else gets
stored in active_profiles (and with this commit, preamble includes also
get stored in active_profiles).
The needed changes to store preamble includes in active_profiles are:
* include_list_recursive(): add and honor in_preamble flag
* add this flag at several places calling include_list_recursive() for
a preamble
* parse_profile_data(): call active_profiles.init_file() for all files.
Before, empty/comment-only files weren't (indirectly) added to
active_profiles because none of the add_$ruletype functions was
called, which could lead to KeyErrors for comment-only preamble include
files (prevented by the now-obsolete and removed check in
get_all_merged_variables()).
For this, we have to hand over in_preamble, and to do a slightly
different handling for preamble and profile rules.
For adding preamble rules to ProfileList, add a add_rule() function that
accepts the rule type as parameter. This is easier than using one of the
add_$type functions depending on the rule type.
With this change, match_line_against_rule_classes() handles nearly all
rule types that have a *Rule class, with the exception of include rules
which need some additional work.
