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.
the video abstraction currently it only contains the following rules:
@{sys}/class/video4linux r,
@{sys}/class/video4linux/** r,
Judging by the v4l path, this abstraction should be used whenever some
app wants to use for instance a webcam or other USB cameras to stream
video usually in chat apps. I was testing some apps, and it looks like
the following rules are needed to make the video streaming possible:
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/159
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/740
Signed-off-by: John Johansen <john.johansen@canonical.com>
... 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.
Also run severity tests with the official severity.db instead of the slightly outdated copy in test/.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/737
Acked-by: John Johansen <john.johansen@canonical.com>
... 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
Merge branch 'cboltz-move-write_header' into 'master'
As a preparation, change change_profile_flags() to use ProfileStorage,
and replace some regex usage with startswith() to make the code easier
to read and (probably) more performant.
See the individual commits for details.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/734
Acked-by: Steve Beattie <steve@nxnw.org>
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.
dhclient wants to set its thread names to functional names for
introspection purposes. Eg.
$ pstree -at 3395
dhclient ens3
├─{isc-socket}
├─{isc-timer}
└─{isc-worker0000}
When denied this can result in dhclient breaking and failing to obtain
IPv4 addresses.
Fixes: https://bugs.launchpad.net/ubuntu/+source/isc-dhcp/+bug/1918410
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Doing so adds the $ac_tool_prefix during cross compilation and will end up using the correct, architecture-dependent python-config.
This is the second and last upstreamable change from https://bugs.debian.org/984582. It looks a little simpler here, because apparmor evolved upstream compared to the Debian version. Fortunately, it got a lot simpler in the process.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/729
Acked-by: John Johansen <john.johansen@canonical.com>
Doing so adds the $ac_tool_prefix during cross compilation and will end
up using the correct, architecture-dependent python-config.
Link: https://bugs.debian.org/984582
AC_CHECK_FILE is meant to check for host files and therefore fails hard during cross compilation unless one supplies a cached check result. Here we want to know about the presence of a build system file though, so AC_CHECK_FILE is the wrong tool.
This is part of https://bugs.debian.org/984582.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/728
Acked-by: John Johansen <john.johansen@canonical.com>
AC_CHECK_FILE is meant to check for host files and therefore fails hard during cross compilation unless one supplies a cached check result. Here we want to know about the presence of a build system file though, so AC_CHECK_FILE is the wrong tool.
Add additional info about complain mode, its behavior, how to enable
it and add warnings about its use.
In addition add info on how to set kernel parameters on boot for
the various options that are covered.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/722
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Apparmor provides the wayland abstraction, but it looks like it misses
some rules. For instance, there are many wayland compositors which are
based on wlroots, and it looks like that when you try to use such
compositor, all GUI apps executed in the graphical environment can't
work properly (they don't event start). They all want the following
rule:
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/143
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/725
Signed-off-by: John Johansen <john.johansen@canonical.com>
Currently the wutmp abstraction has the following rules:
/var/log/lastlog rwk,
/var/log/wtmp wk,
@{run}/utmp rwk,
According to what I see in my apparmor profiles, just a few apps want
to interact with the files listed above, especially with the
/var/log/wtmp . But when the apps do this, they sometimes want the
read access to this file. An example could be the last command. Is
there any reason for not having the r in the rule? The second thing
is the file /var/log/btmp (which isn't included in the
abstracion). Whenever I see an app, which wants to access the
/var/log/wtmp file, it also tries to interact with the /var/log/btmp
file, for instance lightdm/sddm or su . Most of the time they need
just wk permissions, but sometimes apps need also r on this file, an
example could be the lastb command, which is just a link to last.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/152
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/724
Signed-off-by: John Johansen <john.johansen@canonical.com>
Directory traversal does not have a guaranteed walk order which can
cause ordering problems on profile loads when explicit dependencies
are missing.
Combined with MR:703 this provides a userspace work around for issue
147.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/147
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/706
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve.beattie@canonical.com>
Rule downgrades are used to provide some confinement when a feature
is only partially supported by the kernel.
Eg. On a kernel that doesn't support fine grained af_unix mediation
but does support network mediation.
unix (connect, receive, send)
type=stream
peer=(addr="@/tmp/.ICE-unix/[0-9]*"),
will be downgraded to
network unix type=stream,
Which while more permissive still provides some mediation while
allowing the appication to still function. However making the rule
a deny rule result in tightening the profile.
Eg.
deny unix (connect, receive, send)
type=stream
peer=(addr="@/tmp/.ICE-unix/[0-9]*"),
will be downgraded to
deny network unix type=stream,
and that deny rule will take priority over any allow rule. Which means
that if the profile also had unix allow rules they will get blocked by
the downgraded deny rule, because deny rules have a higher priority,
and the application will break. Even worse there is no way to add the
functionality back to the profile without deleting the offending deny
rule.
To fix this we drop deny rules that can't be downgraded in a way that
won't break the application.
Fixes: https://bugzilla.opensuse.org/show_bug.cgi?id=1180766
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/700
Signed-off-by: John Johansen <john.johansen@canonical.com>
This is done by introducing a match_line_against_rule_classes() function to get of repeating code in parse_profile_data().
A side effect is a better separation of preamble and profile data, which needed changes at several places - especially include rules in the preamble were funny\[tm\].
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/719
Acked-by: John Johansen <john.johansen@canonical.com>
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.
in_preamble keeps track of the current parsing position.
It's True while parsing the preamble of a profile file, and when loading
an include file that is included in the preamble (typically tunables/*).
While inside a profile or parsing abstractions/*, it is False.
This commit only hands the information around and keeps in_preamble
updated, but it doesn't really get used yet.
Also adjust the tests to hand over the additional parameter to
parse_profile_data().
