[apparmor] AppArmor and /etc/

[intrigeri]

Hi,

… and sorry for the delay!

John Johansen:
> On 11/25/2017 08:16 AM, intrigeri wrote:
>> Marco d'Itri:
>>> Why are policies generally installed in /etc/ and not in 
>>> /usr/share/apparmor/?
>> 
> It actually depends on the distro, eg. ubuntu touch moved the text
> policy to /var/lib/apparmor/ and the cache to /var/cache/apparmor

Interesting!

Then I'd like to try moving the cache to /var/cache on Debian and
Ubuntu to start with. This seems like a realistic goal for the Buster
development cycle.

Apart of the early boot dependency issue (which I think is not really
one in practice as we run After=local-fs.target on Debian), is there
any foreseeable problem I should be aware of?
Perhaps the Ubuntu folks have some insight to share based on their
past experience doing similar things?

Moving the text policy itself seems more involved and I doubt it'll
happen during the Buster development cycle.

> the /etc/apparmor.d/ location is just the upstream project default
> as its generic and should work on all distros, where /usr/ may not
> be available during early boot etc.

Last time I checked, bits of the code we run when starting
apparmor.service on Debian already depend on /usr. At least on Debian,
/usr is mounted by the initramfs so IIRC this can break only systems
that have a separate /usr filesystem *and* don't use an initramfs.
Nobody reported a problem with this so far so until there's practical
evidence I'm calling this a theoretical issue :)

>> 2. Our local override mechanism is incomplete
>> 
>>    Due to limitations in the policy language, some (rare) kinds of
>>    rules cannot be overridden without editing the original rule, e.g.
>>    https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/451422
>> 
>> 3. Our local override mechanism is Debian-specific
>> 
>>    AFAIK the "#include <local/$profile>" thing is the norm only on
>>    Debian and derivatives. Christian, what do you do at OpenSUSE?
>> 
> Local over rides are more of packaging issue, and never played into
> the original decision. If you wanted to override something you would
> modify the profile text.

Sure you can, but with dpkg such local changes will be overwritten by
the next package upgrade unless the modified file is a conffile.
This is why having a better overriding mechanism seems to be a blocker
for moving the default policy out of /etc. Or did I miss something?

>> It seems that /etc/apparmor.d/cache is created by the Debian
>> packaging, so presumably I could easily ship a CACHEDIR.TAG file
>> in there.
>> 
>> What do others think?

> hrmmm, maybe cache files aren't always generated locally and we
> have had cases of shipping precompiled policy.

I see. And then in this case, a backup of the cache would be needed to
restore a system, so having a CACHEDIR.TAG file would be a problem.

> Its certain is something that a distro could decide setting regardless
> of what the upstream project does.

OK. Given Debian has never shipped pre-compiled policy yet, my
assessment is that the benefits of shipping a CACHEDIR.TAG file
greatly outweigh the risk of making backups incomplete. I'm prepared
to do that but as discussed on https://bugs.debian.org/883584 (with
this mailing list in Cc) it requires a change in the cache clearing
C code, so moving the binary cache to /var/cache might actually be
simpler in practice, in addition to being a more correct solution.
So for now I'll focus on moving the cache out of /etc and will
postpone the CACHEDIR.TAG matter.

>>> If there is no "#include if exists" directive that could be used for 
>>> the /etc/apparmor.d/local/* files then I believe that maintainers should 
>>> be encouraged to ship empty files instead of each repeating the same 
>>> "you may modify this" comment (currently we have both styles).
>> 
>> I don't think we have "#include if exists".
>> 
> We sort of do. If you do a directory based include, like
>   include <local/>

> files are only included if they exist, and there is no failure even if
> the directory is empty.

> where this fails is you basically need an empty dir per profile
> unless the files are to be shared.

Urgh, this would work but does not look much nicer to me than the
current situation. I'd rather go for the "#include if exists" thing
instead:

> We don't currently have a file specific include that won't fail if
> the file doesn't exist, but its an extension we could add.

>> If we had one, then I think we should indeed drop these files
>> entirely. There's a README in that directory, that's just as
>> discoverable by users as these local override files, and would even be
>> more discoverable if it was the only file shipped in
>> /etc/apparmor.d/local/ by default. What do others think? If you agree
> I would disagree that its more discoverable, but its not like you can't
> have a comment pointing to it in the profile.

>> it's a good idea, then I'll file a bug report on LP about adding one
>> such directive (and will try to find someone whose C skills are better
>> than me, in order to implement it).
>> 
> yes allow a condition if it exists include makes sense

Cool. I've updated the bug we already have about it to record this new
reason to implement this: https://bugs.launchpad.net/apparmor/+bug/1206742

I'll try to find someone who can write the needed C code. Pointers and
description of non-obvious requirements would be welcome, as would be
directive name suggestions :)

>> Until we have one such directive, then I think we should indeed DRY,
>> ship empty files in there, and rely on the README being the only
>> non-empty file by default in /etc/apparmor.d/local/.
>> 
>> In most cases this text is generated by dh_apparmor
>> (/usr/share/debhelper/autoscripts/postinst-apparmor) so it's easy to
>> fix this at the distro level: upload dh-apparmor,

Done in Vcs-Bzr, will be part of next upload of src:apparmor to Debian:
https://alioth.debian.org/scm/loggerhead/collab-maint/apparmor/revision/1711

>> wait for all reverse-build-deps to be rebuilt, and at some point
>> request binNMUs to get rid of the long tail. I can do the
>> dh-apparmor upload. Marco, would you like to handle the next steps?

It's early enough in the Buster cycle so there are good chances most
of the dh-apparmor reverse dependencies get source uploads before the
freeze ⇒ I say let's not bother and let time do its job.

Cheers,
-- 
intrigeri