If an exec gets denied in enforce mode, there are no log events for the
target binary/profile. Therefore, trying to set the final_name for the
target will crash with a KeyError.
Check for the existence of hashlog[aamode][target_profile] in all exec
options to prevent this crash.
Note that the log doesn't include enough information for EXEC MODE and EXEC COND, therefore aa-logprof will always propose ALL as EXEC COND (comm= might give a hint about EXEC COND, but isn't good enough).
With the added support in aa-logprof, remove the changeprofile tests from the known-failing list in test-libapparmor-test_multi.py.
Also add another test log (from darix) / expected profile to the libapparmor testsuite.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/631
Acked-by: John Johansen <john.johansen@canonical.com>
Note that the log doesn't include enough information for EXEC MODE and
EXEC COND, therefore aa-logprof will always propose ALL as EXEC COND
(comm= might give a hint about EXEC COND, but isn't good enough).
With the added support in aa-logprof, remove the changeprofile tests
from the known-failing list in test-libapparmor-test_multi.py.
Also add another test log (from darix) / expected profile to the
libapparmor testsuite.
... when reading /proc/$pid/attr/{apparmor/,}current
Also add a comment about _not_ adding support for the 'unconfined'
profile mode, because that would give a quite confusing output.
This means moving the code that reads the 'current' file into a new
function read_proc_current()Then call that function for both
/proc/$pid/attr/apparmor/current (preferred) and /proc/$pid/attr/current
(fallback).
serialize_profile() assumes that active_profiles has the
/etc/apparmor.d/ filename of a profile initialized.
This patch makes sure this is true even when using an extra profile by
initializing it in get_profile().
Ideally serialize_profile() shouldn't always use active_profiles, but
that will be part of a bigger change.
Reported by zt1024 including a proposed patch on
https://gitlab.com/apparmor/apparmor/-/merge_requests/604
but of course ;-) this patch is better because it selectively does the
initialization only in the case that needs it.
utils `make check_severity_db` will fail the build if a (probably new)
capability in not listed in severity.db. This also means it should print
out an ERROR, not a warning.
This is a follow-up of lp#1890547 and
https://gitlab.com/apparmor/apparmor/-/merge_requests/589
Nobody told the tools that log events with operation="symlink" exist.
Add this keyword to the list of file or network operations (I don't
expect network symlinks ;-) but keeping everything in that list makes
things easier than special-casing it.)
Also add the log sample and expected result to the libapparmor tests.
Fixes https://gitlab.com/apparmor/apparmor/-/issues/107
Similar to the profiles/ check using the python utilities, the
tests for the python utilities were not including the path for the
swig libapparmor library in the LD_LIBRARY_PATH variable, only in
PYTHONPATH. This commit fixes that, renaming the variable used for
the built libapparmor check.
v2:
- actually use the LIBAPPARMOR_PATH variable when defining
LD_LIBRARY_PATH
Bug: https://gitlab.com/apparmor/apparmor/-/issues/98
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/586
The enforce profile mode is the default but specifying it explicitly
has not been supported. Allow enforce to be specified as a mode. If
no mode is specified the default is still enforce.
The kernel has supported kill and unconfined profile modes for a
long time now. And support to the parser so that profiles can make
use of these modes.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/440
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/7
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
This is not a real change - since some commits, include_dir_filelist()
gets only called with absolute paths.
Add a check to ensure this, drop the now superfluous get_include_path()
call, and replace usage of include_name_abs with include_name (which are
the same now).
Also drop the superfluous profile_dir parameter, and adjust the only
caller accordingly.
With the check that incfile starts with a '/', incfile and incfile_abs
(as returned by get_include_path()) are always the same.
Drop the get_include_path() call and setting incfile_abs, and replace
usage of incfile_abs with incfile.
This is just a cleanup, no behaviour change.
... so that the include rules proposed by aa-logprof continue to be
relative to the profile directory.
This fixes the behaviour change introduced in the previous commit.
This removes the need to remove profile_dir from include paths at
various places.
A side effect is that aa-logprof / match_includes() now propose more
include rules, for example matching local/ files.
Another side effect is that proposals for include rules
(match_includes() again) now come with the full path.
Both side effects will be fixed in the next commits.
This is needed for running the tests, because test/logprof.conf contains
a relative path, and tests only "manually" set the profile_dir if they
need/have a modified copy of the profiles.
IncludeRule: sort files in included directory
... instead of relying on the filesystem(!) ordering, which will look
random to both users and unittests.
Also partially revert the test changes from
c5a7bcd50e /
https://gitlab.com/apparmor/apparmor/-/merge_requests/548 -
sorting the result only in the tests is a bad idea.
See merge request apparmor/apparmor!552
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
Tag profiles and abstractions with abi information.
Tagging abstractions is not strictly necessary but allows the parser
to detect when their is a mismatch and that policy will need an
update for abi.
We do not currently tag the tunables because variable declarations
are not currently affected by abi.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/491
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
... instead of relying on the filesystem(!) ordering, which will look
random to both users and unittests.
Also partially revert the test changes from
c5a7bcd50e /
https://gitlab.com/apparmor/apparmor/-/merge_requests/548 -
sorting the result only in the tests is a bad idea.
This fixes cases when two aliases with the same left side were
configured - instead of "last one wins" in the dict, AliasRuleset now
keeps both.
ProfileList add_alias() changes its parameters and now expects an
AliasRule object. Adjust all callers to that.
Drop the no longer needed write_alias().
Also adjust the tests to use AliasRule and add a dedup test promised in
an earlier patch series.
Add VariableRule and VariableRuleset and use it for variable handling
Besides the usual advantages of switching to classes, we finally get rid of the `filelist` hasher.
While on it, also fix some bugs around variable handling, including https://bugs.launchpad.net/apparmor/+bug/1331856 and some that maybe nobody noticed before.
As usual, see the individual commits for details.
See merge request apparmor/apparmor!544
Acked-by: Steve Beattie <steve.beattie@canonical.com>
Trailing commas in variable values are not allowed (unless they are
quoted). Fix the regex to avoid "eating" the comma, and add a check to
detect invalid commas.
As usual, add some tests, and remove some testcases from the
exception_not_raised list.
... 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().
