Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AppArmor profile fails to load with AppArmor 3.0.0 installed #3659

Closed
zethexx opened this issue Oct 9, 2020 · 9 comments
Closed

AppArmor profile fails to load with AppArmor 3.0.0 installed #3659

zethexx opened this issue Oct 9, 2020 · 9 comments
Labels
bug Something isn't working

Comments

@zethexx
Copy link

zethexx commented Oct 9, 2020

Bug and expected behavior
Expected behaviour: The AppArmor profile loads without an error (i.e. the command sudo apparmor_parser -r /etc/apparmor.d/firejail-default exits with exit code 0), allowing myself to use AppArmor integration in firejail.

Actual behaviour: When loading the AppArmor profile using sudo apparmor_parser -r /etc/apparmor.d/firejail-default, I get Found reference to variable run, but is never declared and the process exits with exit code 1. As a result, Firejail cannot use AppArmor to confine applications. When running a program in Firejail using the apparmor option enabled, I get:

Warning: Cannot confine the application using AppArmor.
Maybe firejail-default AppArmor profile is not loaded into the kernel.
As root, run "aa-enforce firejail-default" to load it.

The program runs normally except there is no AppArmor confinement. Running sudo aa-status confirms this, it only says the module is loaded.

Probable cause: The apparmor package in my distribution has been upgraded from 2.13.4 to 3.0.0. A change in the syntax or something may have broken the profile.

Probable fix: Update AppArmor profile maybe? not sure how older apparmor versions will be managed then

Temporary fix for those affected: I simply downgraded the AppArmor package as described in the ArchWiki, this is only for Arch users of course. I had no side effects of downgrading, also I was able to immediately use AppArmor without need to reboot, but I would still recommend rebooting.

Reproduce
Install AppArmor 3.0.0 in your distribution and try to load the firejail-default AppArmor profile.

Environment
Distro: Arch Linux rolling, latest version as of 2020-10-09

Firejail version: git 5549f89

Should have the same issue if used with latest stable release.

Additional context
The error I get when trying to load the apparmor profile using sudo apparmor_parser -r /etc/apparmor.d/firejail-default:

Found reference to variable run, but is never declared

Happens when AppArmor 3.0.0 is used with Firejail.

Loading the apparmor profile using sudo aa-enforce firejail-default:

Setting /etc/apparmor.d/firejail-default to enforce mode.

ERROR: Found reference to variable run, but is never declared

Error code 1.

The loading profile using apparmor_parser is told from the archwiki, the loading profile using aa-enforce is told from firejail docs/program. I don't think it should matter

@SarchB
Copy link

SarchB commented Oct 9, 2020

Yep, same problem on Arch indeed.

@FOSSONLY
Copy link

FOSSONLY commented Oct 9, 2020

The maintainer at Arch does not seem to be very considerate about simply updating packages at any cost, although the AppArmor project explicitly warns against it. Especially since AppArmor 3.0 is only a transition release, which is not intended for productive use in a regular linux distribution. It is therefore purely experimental and intended to ease the transition in the face of major changes until the first productive release (AppArmor 3.1) in months. And given the amount of changes, AppArmor 3.x should be put on hold until the most serious bugs and problems are fixed.

@kris7t
Copy link
Collaborator

kris7t commented Oct 10, 2020

It seems we need to define the @{run} variables as per https://gitlab.com/apparmor/apparmor/-/commit/452b5b8735e449cba29a1fb25c9bff38ba8763ec

Adding

#include if exists <tunables/run>

to /etc/apparmor.d/firejail-default seems to do the trick (if exists helps for Apparmor version that don't have this tunable). It seems backwards compatible to add this code to Firejail, at least with Apparmor versions that understand if exists.

We might also need to add something like

abi <abi/3.0>,

to the profile to stop warnings, but I don't have any idea how to do that in a backwards-compatible way.

@glitsj16 glitsj16 added the bug Something isn't working label Oct 10, 2020
@glitsj16
Copy link
Collaborator

Adding
#include if exists <tunables/run>
to /etc/apparmor.d/firejail-default seems to do the trick (if exists helps for Apparmor version that don't have this tunable).
We might also need to add something like
abi <abi/3.0>,

@kris7t Do you happen to have a firejail-default file with the above changes so more people can test things?

Upstream committed a config option to disable ABI warnings (warn=no-abi) that might prove useful in this context, but the current Arch apparmor package 3.0.0-2 does not have that option.

@curiosityseeker
Copy link
Collaborator

curiosityseeker commented Oct 10, 2020

@kris7t Do you happen to have a firejail-default file with the above changes so more people can test things?

Well, I just added that change to the top of firejail-default, and it solves the problem, indeed:

#########################################
# Generic Firejail AppArmor profile
#########################################

#include if exists <tunables/run>

...

Upstream committed a config option to disable ABI warnings (warn=no-abi) that might prove useful in this context, but the current Arch apparmor package 3.0.0-2 does not have that option.

Hm, I don't see these warnings on Arch ... As a matter of fact I have multiple self-made AppArmor profiles, and they work flawlessly under AA 3.0 so far.

kris7t added a commit to kris7t/firejail that referenced this issue Oct 10, 2020
AppArmor introduces the @{run} variable, which is used in
<abstractions/dbus-session> and <abstractions/dbus-session-strict> among
other places. Thus, we must #include <tunables/run> to be able to call
these abstractions.

As an attempt at backwards compatibility, we #include if exists instead
of #include, since there is no <tunables/run> in AppArmor 2.x.

However, if exists is a relatively new feature, see e.g.
https://phabricator.kde.org/D14526
(However, do note that if exists does not appear as a new feature in
https://gitlab.com/apparmor/apparmor/-/wikis/Release_Notes_2.13).
Therefore, this commit restricts our compatibility to relatively new
(<10 months) old AppArmor releases only.

As an alternative, we could detect the AppArmor version at configure
time, and emit a firejail-default profile based on that.
@kris7t
Copy link
Collaborator

kris7t commented Oct 10, 2020

@glitsj16 I created a pull request #3660 with my simple fix, and also discussed some more robust (albeit more complicated) solutions.

@curiosityseeker Yeah, by default, the AppArmor parser doesn't run with --warn all, so there are no warnings. However, if I invoke it with --warn all, it does complain about ABI compatibility, because <abstractions/dbus-strict> and <abstractions/dbus-session-strict> are declared to be abi <abi/3.0>. It sounds like a bad idea to invoke those abstractions from a not-3.0 profile, because AppArmor will use the ABI of the profile to interpret the abstractions, despite their own ABI declarations (or so says the warning). I guess we are fine for now, but we may encounter some odd behavior in the future (if the abstractions decide to actually rely on ABI-specific behavior).

kris7t added a commit to kris7t/firejail that referenced this issue Oct 10, 2020
AppArmor introduces the @{run} variable, which is used in
<abstractions/dbus-strict> and <abstractions/dbus-session-strict> among
other places. Thus, we must #include <tunables/run> to be able to call
these abstractions.

As an attempt at backwards compatibility, we #include if exists instead
of #include, since there is no <tunables/run> in AppArmor 2.x.

However, if exists is a relatively new feature, see e.g.
https://phabricator.kde.org/D14526
(However, do note that if exists does not appear as a new feature in
https://gitlab.com/apparmor/apparmor/-/wikis/Release_Notes_2.13).
Therefore, this commit restricts our compatibility to relatively new
(<10 months) old AppArmor releases only.

As an alternative, we could detect the AppArmor version at configure
time, and emit a firejail-default profile based on that.
@kris7t kris7t closed this as completed in bba750c Oct 10, 2020
kris7t added a commit that referenced this issue Oct 10, 2020
Fix AppArmor 3.0 support (closes #3659)
@cboltz
Copy link

cboltz commented Oct 17, 2020

For the records: the abi <abi/3.0> declaration means that the "new" rule types network, dbus and unix rules will get enforced - under the following conditions (simplified, see below):

  • at least kernel 5.4
  • AppArmor 3.0 userspace
  • abi <abi/3.0> declaration in the profile

If these conditions aren't met, the "new" rule types will not be enforced. Instead, all network etc. acess will be allowed - basically you'll get the same behaviour as if your profile would have broad network, dbus, and unix, rules.

As I mentioned, the conditions above are simplified. Some additional details:

  • support for network rules is available since kernel 4.17 (needs AppArmor 3.0 userspace + the abi declaration)
  • Ubuntu kernels support the "new" rule types since some years, even with older userspace and without abi
  • SUSE and openSUSE kernels support network rules since years, even with older userspace and without abi

@curiosityseeker
Copy link
Collaborator

curiosityseeker commented Oct 21, 2020

@cboltz : Christian, thanks for this info! One question, though: The release notes for AppArmor 3.0 say:

Apprmor 3.0 is a bridge release between older AppArmor 2.x policy and the newer AppArmor 3 style policy which requires the declaration of a features abi. As such AppArmor 3.0 will be a short lived release, and will not receive long term support. The following AppArmor 3.1 feature release is planned to be a regular release, ...

Does that mean that your remark above that ...

If these conditions aren't met, the "new" rule types will not be enforced.

... is only valid for 3.0 but no longer for 3.1 and later?

@cboltz
Copy link

cboltz commented Oct 22, 2020

I should have written "at least AppArmor 3.0" and "at least abi <abi/3.0>" ;-)

The "new" rule types will be enforced starting with 3.0, and also with later versions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants