Provide systemd service

[Bryan Quigley]

AppArmor is in the critical path for bootup for any systems that use it. Let's specify a systemd service file instead of having systemd use sysv compatibility mode.

There seems to be a few bugs tracking the precursor this work, like https://bugs.launchpad.net/apparmor/+bug/1488179, but no actual bug for the systemd service itself.

AUR has a simple service file, not sure if that would be useful - https://aur.archlinux.org/packages/apparmor/

[Christian Boltz (cboltz)]

openSUSE also has a service file. Basically it's a wrapper around the initscript, however maybe the dependencies could be "stolen" from it ;-)

https://build.opensuse.org/package/view_file/security:apparmor/apparmor/apparmor.service?expand=1

The goal should be to have one service file that works for all distributions, and to ship that in the AppArmor tarball.

[Seth Arnold (seth-arnold)]

A real systemd unit file would be best. The trouble with the simple ones is that I don't think they'd be suitable for Ubuntu's uses, since we store AppArmor policy in multiple places depending upon if it is shipped with the core distribution or packages (/etc/apparmor.d) or via snaps or clicks in /var/ somewhere.

The AUR files can be found at:
https://aur.archlinux.org/cgit/aur.git/tree/?h=apparmor

and are quite simple:
[Unit]
Description=AppArmor profiles
DefaultDependencies=no
After=local-fs.target
Before=sysinit.target

[Service]
Type=oneshot
ExecStart=/usr/bin/apparmor_load.sh
ExecStop=/usr/bin/apparmor_unload.sh
RemainAfterExit=yes

[Install]
WantedBy=multi-user.target

#!/bin/sh

aa_profiles='/etc/apparmor.d/'
aa_log='/var/log/apparmor.init.log'
/usr/bin/apparmor_parser -r $(find "$aa_profiles" -maxdepth 1 -type f) 2>> "$aa_log"

#!/bin/sh

aa_profiles='/etc/apparmor.d/'
aa_log='/var/log/apparmor.init.log'
PROFILES=`find "$aa_profiles" -maxdepth 1 -type f`
for profile in $PROFILES; do
    apparmor_parser -R "$profile" 2>> "$aa_log"
done

I think what we want instead is a templated systemd file that can work for /etc/apparmor.d/ and /var/whatever/click and /var/whatever/snap/ profiles as well as user-specific profiles in ~/.apparmor/ that can be run with user sessions, whenever we finally get there.

That'd also help remove the complicated calls to find and so forth, since apparmor_parser should be able to handle being called on the directories directly, e.g. apparmor_parser -r /etc/apparmor.d/

I also think 'stop' should be a no-op, since systemd implements 'restart' via separate 'stop' and 'start'; if we actually unload profiles, 'restart' cannot re-confine programs that are running. I'd rather provide a separate aa-unload tool if unloading all profiles is necessary.

I think http://0pointer.de/blog/projects/instances.html is the description for the feature I'm thinking of.

Thanks

[Seth Arnold (seth-arnold)]

Darix suggests the instances isn't a great idea. He instead recommends reading variables from a configuration file that are then used in the exec lines of the systemd unit file:

[Unit]
Description=AppArmor profiles
DefaultDependencies=no
After=local-fs.target
Before=sysinit.target

[Service]
Type=oneshot
EnvironmentFile=-/etc/sysconfig/apparmor
ExecStart=/usr/sbin/apparmor_parser -r $APPAMOR_SEARCH_LIST
ExecStop=/usr/sbin/apparmor_parser -R $APPAMOR_SEARCH_LIST
ExecReload=/usr/sbin/apparmor_parser --reload $APPAMOR_SEARCH_LIST
RemainAfterExit=yes

[Install]
WantedBy=multi-user.target

Then we could set the APPARMOR_SEARCH_LIST to /etc/apparmor.d/ on traditional systems, /etc/apparmor.d/ and /var/foo/click/apparmor.d/ on phones, /var/bar/snap/apparmor.d/ on snap-based things, etc.

cboltz reports that one-shot units do support ExecReload.

[Felipe Sateler (fsateler)]

@seth-arnold ,

I would probably use /etc/default/apparmor in more debian/ubuntu style.

But, what happens if apparmor_parser is invoked with an empty $APPARMOR_SEARCH_LIST (in the service you posted that would translate into no parameter being passed)? If that should not happen, then the EnvironmentFile should probably not have a - prefixed so that the unit fails when the file is missing.

In the more general case, systemd service files are configuration files themselves, so there is no need to introduce a new one: just set the default APPARMOR_SEARCH_LIST with an Environment key, that can be overriden via a drop-in in either /lib (by distro) or in /etc (by admin):

[Service]
Type=oneshot
Environment=APPARMOR_SEARCH_LIST=/etc/apparmor.d
ExecStart=/usr/sbin/apparmor_parser -r $APPAMOR_SEARCH_LIST
ExecStop=/usr/sbin/apparmor_parser -R $APPAMOR_SEARCH_LIST
ExecReload=/usr/sbin/apparmor_parser --reload $APPAMOR_SEARCH_LIST
RemainAfterExit=yes

And it could be overriden with a simple snippet (eg, in /lib/systemd/system/apparmor.service.d/50-click-search-list.conf):

[Service]
# Im a phone
Environment="APPARMOR_SEARCH_LIST=/etc/apparmor.d /var/foo/click/apparmor.d"

[Martin Pitt (pitti)]

I agree to Felipe here, there's not that much gain from providing another indirection with /etc/default/apparmor. systemd unit drop-ins can be put into /lib/systemd/system/ and thus shipped by packages like lxc-android-config without the hassle of conffiles or the user potentially breaking them. *If* you really want an /etc/default/apparmor which defines $APPAMOR_SEARCH_LIST, then I suggest to add

  ConditionPathExists=/etc/default/apparmor

to guard against the admin removing the conffile. (But this is just a tangent -- I still don't recommend adding a conffile for this; this is *not* configuration in the "admin can muck around with that" sense!)

I also agree that using templates seems like overkill here. They make restarting/reloading a much bigger hassle, you might run into issues with parallelization, and you'll need a generator to actually instantiate them. Iterating over a list of files and feeding them to apparmor_parser is so simple that this can be much more easily be done directly.

In terms of code management, I suggest to moving out the common code of /etc/init.d/apparmor and apparmor.service into /lib/apparmor/functions (or perhaps a different file) and calling it from both the init.d script and the systemd unit. I. e. what the above /usr/bin/apparmor_load.sh does, just perhaps not in $PATH (unless it actually makes sense for the user to call it directly).

[Dainius 'GreatEmerald' Masiliūnas (pastas4)]

I doubt it's ideal to have apparmor_parser called from a service file to begin with. AppArmor protects specific processes, and often times said processes are already started by systemd. There is an AppArmorProfile directive in systemd already, but unfortunately it just switches profiles instead of loading them. Having something akin to that, which both loads a profile and makes sure not to load the daemon if the profile it's associated with isn't loaded, would be nice. That would allow systemd to handle the boot parallelisation as well as security, both things that only PID1 can really be trusted to do right.

In the mean while, I'm using templates for handling that, with an apparmor.target for managing them all at the same time:

  [Unit]
  Description=AppArmor target
  DefaultDependencies=no
  Before=sysinit.target

  [Install]
  WantedBy=sysinit.target

And the template unit apparmor@.service:

  [Unit]
  Description=AppArmor profile: %i
  DefaultDependencies=no
  Before=apparmor.target

  [Service]
  Type=oneshot
  ExecStart=/sbin/apparmor_parser -r /etc/apparmor.d/%i
  ExecStop=/sbin/apparmor_parser -R /etc/apparmor.d/%i
  ExecReload=/sbin/apparmor_parser --reload /etc/apparmor.d/%i
  RemainAfterExit=yes

  [Install]
  WantedBy=apparmor.target

This does mean that you initially need to either manually specify each profile to load (using `systemctl enable <email address hidden>` for instance, which is fairly inefficient since it passes files to the parser one by one), and/or use a drop-in, like nginx.service.d/01-apparmor.conf:

  [Unit]
  <email address hidden>
  <email address hidden>

Which is quite nice, since it ensures that the service, when enabled and run, will make sure to load its own profile, or refuse to run if it fails to load. In this case the loading can happen at any time during boot, it doesn't have to be early boot. But it does mean that you need a drop-in file for every protected service.

[Christian Boltz (cboltz)]

Just a quick update about the situation on openSUSE - in the meantime, we got rid of the initscript and switched to a small wrapper script - see apparmor.systemd and apparmor.service on https://build.opensuse.org/package/show/security:apparmor/apparmor

That's obviously not the final solution, but at least better than the old initscript ;-)

[intrigeri (intrigeri)]

FTR a systemd unit was imported upstream: https://gitlab.com/apparmor/apparmor/merge_requests/81

[Bryan Quigley (bryanquigley)]

And it's now provided by Ubuntu and Debian!

..it mostly just points back to the init.d script, but that does lay the groundwork and prevent it from having to be generated.

Any future improvements I figure should be done in the gitlab (/closes bug).