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.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/545
Acked-by: John Johansen <john.johansen@canonical.com>
At least on Debian, when DNSSEC validation is enabled, dnsmasq needs to read
/usr/share/dnsmasq-base/trust-anchors.conf
Bug-Debian: https://bugs.debian.org/934869
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.
... so that - if someone calls aa-status from the commandline - the next prompt doesn't get displayed in the same line as the json output.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/541
Acked-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
If multiple -p/--profile or -n/--namespace parameters are given to aa-exec, they are handled on a "last one wins" base.
This probably isn't expected behaviour, and erroring out in case of conflicting parameters probably makes more sense.
PR: https://gitlab.com/apparmor/apparmor/-/merge_requests/540
Acked-by: John Johansen <john.johansen@canonical.com>
If multiple -p/--profile or -n/--namespace parameters are given to
aa-exec, they are handled on a "last one wins" base.
This probably isn't expected behaviour, and erroring out in case of
conflicting parameters probably makes more sense.
This patch series moves include rule handling away from the `filelist` hasher to using IncludeRule and IncludeRuleset. This means that only variable handling is left in `filelist`.
As usual, check the individual commits for details.
PR: https://gitlab.com/apparmor/apparmor/-/merge_requests/537
Acked-by: John Johansen <john.johansen@canonical.com>
remove_duplicate_rules() composed the 'includes' list from a) the
include rules in the profile and b) the file_includes (preamble
includes), and then checked the profile includes against this
combination of profile and preamble includes.
Checking profile includes against preamble includes is wrong, and the
only reason why this went unnoticed for years is that preamble include
files (like tunables) won't work inside a profile and therefore never
appear there.
In theory this might be a backport candidate, even if this shouldn't
cause a real-world bug.
This function returns a list of all includes in a profile and its
included files.
It's split off from is_known_rule() and get_file_perms() which shared
lots of common code to recursively get the include list. These functions
now use include_list_recursive().
... or calling is_known_rule() on events for non-existing hats.
It's the usual hasher() "fun" again - accessing a non-existing element
will create its parent.
In theory this commit might be worth a backport. In practise, it doesn't cause
any visible problem.
However, starting with the next commit, it will cause lots of test errors.
Also add a missing is_known_rule() call for dbus rules, which might have
caused similar hasher() "fun".
rule_obj.get_full_paths() handles directories and non-existing files
(depending on 'if exists'), so we can simply feed the resulting list
into load_include()
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()
IncludeRuleset needs to import it (next commit), which is impossible
when it lives in aa.py (would cause recursive import loop because aa.py
imports IncludeRuleset)
... because this is now done via IncludeRule, and keeping the separate
code would mean asking twice.
Note that the user interface changes slightly.
The old workflow was
1 - #include <foo>
2 - #include <bar>
3 - #include <baz>
[select 2 and (A)dd, then get the next question for the remainder]
1 - #include <foo>
2 - #include <baz>
[(A)dd another one, or (I)gnore the remainder]
The new workflow will ask separately for each abstraction, and you can
(A)dd or (I)gnore it. This is probably easier to understand because it's
basically a yes/no question.
... 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.
aa-notify does not need to load the policy includes for its current
features, so drop the unneeded overhead.
Signed-off-by: John Johansen <john.johansen@canonical.com>
alid_include() checks if the given include file exists or is whitelisted in cfg\['settings'\]\['custom_includes'\].
The check if that include file is already part of the profile is unrelated to that. Move it to match_includes() where it fits much better (and drop the now superfluous profile parameter from valid_include())
In theory is_known_rule() should prevent that case from ever happening, but let's restrict this commit to moving the code around and keep this check just to be sure.
While on it, add some documentation to both functions.
PR: https://gitlab.com/apparmor/apparmor/-/merge_requests/534
Acked-by: John Johansen <john.johansen@canonical.com>
Also rename aa.py delete_duplicates() and make ruletypes a parameter.
See the commit messages for details.
This reduces usage of global variables.
The final change is that aa-genprof now asks about preamble rules that are \*Rule classes (currently `abi` and `include if exists`).
PR: https://gitlab.com/apparmor/apparmor/-/merge_requests/532
Acked-by: John Johansen <john.johansen@canonical.com>
valid_include() checks if the given include file exists or is
whitelisted in cfg['settings']['custom_includes'].
The check if that include file is already part of the profile is
unrelated to that. Move it to match_includes() where it fits much
better (and drop the now superfluous profile parameter from
valid_include())
In theory is_known_rule() should prevent that case from ever happening,
but let's restrict this commit to moving the code around and keep this
check just to be sure.
While on it, add some documentation to both functions.
For now, that means aa-mergeprof will ask for `abi` and `include if
exists` rules (currently hardcoded).
This needs storing of `active_profiles` in the Prof object - the
preamble `abi` and `include if exists` rules are stored there.
Since several functions expect an `include` dict, add an empty one to
ProfileList to prevent lots of errors and breakage. It can be removed
again when handling of `include` rules gets moved to IncludeRule.
Rename delete_duplicates() to delete_all_duplicates() to make the
function easier grep-able - the *rule classes have delete_duplicates()
which might be confused with the old name.
Also hand over 'ruletypes' as parameter to delete_all_duplicates()
instead of using the global variable.
ask_the_questions() stays the "main" function, loops over all profiles,
and calls ask_rule_questions() for each profile.
ask_rule_questions() asks the questions for all events in a specific
profile or hat.
This reduces the usage of global variables in ask_rule_questions().
