... and add a test to ensure that everything works as expected.
Note that broken variable names like '@{foo' match the (quite
permissive) regex, but are invalid nevertheless.
... by calling active_profiles.get_all_merged_variables()
Also remove vars/vars_bad_add_assignment_1.sd from the
exception_not_raised list again - now it raises an exception as
expected.
Add set_variables() to severity.py to set the variables for severity
rating. It typically gets the data from the get_all_merged_variables()
result.
This replaces the slightly broken load_variables() that parsed profile
files for variables. (For example, parsing "@{foo} = /bar" resulted
in a variable name "@{foo} " with trailing space.)
Also adjust aa.py and the severity tests to use set_variables() (with
get_all_merged_variables()) instead of load_variables().
This also re-adds the checks that were removed in the "Store variables
in active_profiles (ProfileList)" commit earlier, while still fixing
lp:1331856.
With this change, unload_variables() becomes useless (the variables get
overwritten in set_variables() anyway), drop it and its calls.
Note that load_variables() silently ignored non-existing files while the
get_all_merged_variables() call only works for existing files that are
known to active_profiles. Since the input of ask_the_questions() and
ask_exec() comes from log_dict (= audit.log or a profile to merge), add
a check if that profile actually exists in the set of active profiles.
Also adjust the severity tests to use set_variables().
Finally, drop the tests that check for handling non-existing include
files, redefining and adding to non-existing variables - all these
things get now handled in include_list_recursive() and
get_all_merged_variables() and their tests.
Fixes: https://bugs.launchpad.net/apparmor/+bug/1331856
This function returns a dict with all variables and their values for a
given profile file. It also checks for redefined variables, and adding
to non-existing variables, and errors out in both cases. Note that it
does not check the include order and is therefore more forgiving than
apparmor_parser.
Also add some tests for get_all_merged_variables().
Note: the tests are based on reading "real" profiles, therefore we need
to initialize apparmor.aa and call some of its functions.
The alternative would be to construct ProfileList objects for some
profiles and includes manually, but doing that in a way that can replace
the tests with "real" profiles would be quite some work, and I'm not
bored enough for that ;-)
Everything handled in 'filelist' gets handled in active_profiles now.
Note: the 'elif' branch in delete_all_duplicates() was probably never
hit because `if include.get(...)` always matched. The only possible
exception might be non-existing include files, but those cause a 'file
not found' error anyway.
... instead of filelist[file]['lvar'], and also write them from there.
Also fix detection of variable definitions inside a profile, which is
not allowed.
Note that ProfileList has a different write order than the old code -
first includes, then variable definitions. This makes more sense because
typical profiles first include tunables/global, and then define
additonal variables (that might use variables from tunables/global) or
extend variables defined in tunables/global.
This change also fixes some problems with the simple_test test profiles.
The "adding to non-existing variable" check currently doesn't exist,
which "fixes" lp:1331856.
OTOH this also means that such cases are not detected, therefore add
vars_bad_add_assignment_1.sd to the exception_not_raised list.
The check will be re-added in a later commit
in get_all_merged_variables().
With the includes rule class landing, this particular test failed
in a test vm due to the sort ordering being different. It's not
clear that there should be an expectation of ordering returned from
get_full_paths(), so sort the result.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/548
strip_quotes() assumed its parameter is at least one character long, and
errored out on an empty string.
It also converted a string consisting of a single quote to an empty
string because that single quote had a quote as first and last char.
This commit fixes these two bugs.
Also rewrite TestStripQuotes to use tests[], and add some test for an empty
string, a one-char path (just a slash) and a single quote.
This function returns a list of paths for an include rule. This can be
- a single path if the include file is a file and exists
- a single path if the include file doesn't exist, but doesn't have
'if exists' (this will cause a 'file not found' error when used)
- a list of paths if the include is a directory
- an empty list if the rule has 'if exists' and the file doesn't exist
Also add some tests for get_full_paths()
... and write them (only) from 'inc_ie' (IncludeRuleset), which can
handle both "include" and "include if exists" rules.
This duplicates storage of include rules because 'include' is still used
and needed at various places that work on/with the include rules.
With this, we get removal of duplicate include lines insinde a profile
in aa-cleanprof "for free" - extend cleanprof_test.in to confirm this.
... but not for abi rules, which (according to the simple_tests
profiles) do not share these bugs)
For unquoted paths, make sure that the path doesn't include whitespace.
... because after the previous three commits, nothing reads/needs this
anymore
Note: file_name in ask_exec() was only used in the (dropped) filelist
usage.
Instead of checking filelist[file]['profiles'] for duplicate hats, check
profile_data[profile][hat].
With this, the duplicate hat check is done in the same way as the check
for duplicate profiles and child profiles.
Also add tests for duplicate child profiles and duplicate hats.
The profile repo is dead since years and most likely won't come back, so
there's no point in keeping and maintaining the code for uploading and
downloading profiles.
- add_inc_ie() stores include and include if exists rules
- get_clean() and get_raw() return the profile preamble (currently only
the include rules)
Also add tests for the new functions.
This is similar to get_clean(), but keeps the original rule order
instead of sorting them.
This is useful for include rules in the preamble, where the order might
be relevant - for example if the first include defines a variable that
is then used or extended in the second include file.
Merge branch 'cboltz-profile-list-rename-add' into 'master'
See merge request apparmor/apparmor!502
Acked-by: Steve Beattie <steve.beattie@canonical.com>
For now, only 'include if exists' rules will be handled by IncludeRule.
Using it also for 'include' rules needs some more code changes so that
included files still get checked etc.
Also remove some testcases from test-parser-simple-tests.py unknown_line
which no longer fail.
These classes are meant to handle 'include' and 'include if exists'
rules.
Due to restrictions in re_match_include_parse(), some cases in
is_covered_localvars() and is_equal_localvars() can't be reached in the
unittests.
Also, IncludeRule isn't used in aa-logprof (yet?), which means
logprof_header_localvars() result format isn't decided yet, and
therefore not tested.
This means test coverage for the new classes isn't 100% this time ;-)
