[pkg-apparmor] Bug#796589: Bug#796589: apparmor: Has init script in runlevel S but no matching service file

[Felix Geyer]

Hi,

On 26.08.2015 17:45, intrigeri wrote:
> Hi,
> 
> here are my initial notes and (incomplete) drafts, partly inspired by
> OpenSuSe's unit. I think we'll need at least two units. That's kind of
> blocked by the ongoing discussion on /usr and click-specific
> bits, though.
> 
> sys-kernel-security.mount
> =========================
> 
> [Unit]
> Description=Security File System
> Documentation=XXX
> DefaultDependencies=no
> ConditionPathExists=/sys/kernel/security
> 
> [Mount]
> What=none
> Where=/sys/kernel/security
> Type=securityfs

systemd mounts securityfs [1] automatically so this shouldn't be needed.

> apparmor-load-policy.service
> ============================

Unless we want to rename it for every init system we need to keep the name apparmor.

> [Unit]
> Description=Load AppArmor profiles
> 
> DefaultDependencies=no
> 
> # Load policy before bringing up the first network interface,
> # to be able to confine processes that access the network early,
> # such as dhclient:
> Wants=network-pre.target
> Before=network-pre.target
> 
> # ... however, let's not add an exagerated Before=basic.target
> # or Before=sysinit.target, meant to ensure that the policy for basic system
> # services is applied: in most case that's not needed, and it is prone
> # to introducing dependency loops
> # (https://wiki.debian.org/Teams/pkg-systemd/rcSMigration).
> # Instead, basic system services that should be confined with AppArmor
> # should add an After=apparmor.service, just like it's done already e.g.
> # by networking.service (Debian -specific) and libvirtd.service.

Is network-pre.target really enough?
You'd basically have to add After=apparmor.service to every daemon that doesn't depend on
network.target.

> After=local-fs.target systemd-journald-audit.socket

Why is systemd-journald-audit.socket needed given it's socket-activated?

> RequiresMountsFor=/sys/kernel/security

I think we can expect it to always be there except for containers which are already
covered below.

> # XXX: do we need to do anything at shutdown?
> # If yes, then add Conflicts=shutdown.target and Before=shutdown.target,
> # or we won't be gracefully stopped on shutdown due to DefaultDependencies=no.

No, we don't need to do anything on shutdown.
That's also how the init script is set up right now.

> ConditionSecurity=apparmor
> ConditionPathIsReadWrite=/sys/kernel/security/apparmor/.load

I'd rather have the unit fail if the file is not writable instead of covering it
in a blocked state.

> ConditionVirtualization=!container
> # do not perform start/stop/reload actions when running from the Ubuntu liveCD:
> ConditionPathExists=!/rofs/etc/apparmor.d
> 
> Documentation=man:apparmor(7)
> Documentation=http://wiki.apparmor.net/
> 
> [Service]
> Type=oneshot
> ExecStart=XXX
> ExecReload=XXX
> ExecRestart=XXX
> ExecStop=XXX

There is no ExecRestart, systemd translates restart to stop/start.
That makes it a bit challenging to have a well-defined reload/restart actions.

Currently we do this in the init script:
- start: load all profiles
- stop: clear cache
- reload/restart: clear cache, load profiles, unload obsolete profiles

Why does it clear the cache? Is the cache invalidation known to be problematic?

Imho ideally we'd do this:
- start/reload: load all profiles, unload obsolete
- stop: nop

Iirc detecting/unloading obsolete profiles is a bit expensive so we might not want to do
that on boot though.


Ultimately we need another tool (apparmorctl?) that provides these actions and some
more so it can be called from the systemd unit.
For example we need users to be able to trigger teardown and clear-cache.

Cheers,
Felix


[1] https://github.com/systemd/systemd/blob/master/src/core/mount-setup.c#L78