#979500 dh-apparmor: please support local includes of abstractions like "abstraction/name"

Package:
dh-apparmor
Source:
apparmor
Submitter:
Christian Ehrhardt
Date:
2021-04-02 10:54:04 UTC
Severity:
normal
Tags:
#979500#5
Date:
2021-01-07 12:15:59 UTC
From:
To:
Hi,
a few packages have their own abstractions which in turn are reused in
other profiles.
An example is libvirt which has :
  /etc/apparmor.d/abstractions/libvirt-qemu

This abstraction is then used in the per VM-guest rules like:
$ cat /etc/apparmor.d/libvirt/libvirt-01d4cdfe-259c-4748-abcf-fd1fa96e52e1
...
  #include <abstractions/libvirt-qemu>
...

It turns out profiles for VMs are rather hard as there are so many
very different use cases.
It is always a balance between too open or too restrictive.
Therefore it is rather common for users to want to add local overrides
on their system.

Those can be done "per guest" in the files like
"/etc/apparmor.d/libvirt/libvirt-01d4cdfe-259c-4748-abcf-fd1fa96e52e1"
mentioned above. But chances are quite high you have some things you
want to allow in general for all your guests.

To do so one can edit "/etc/apparmor.d/abstractions/libvirt-qemu", but
as we all know that means either conffile prompts or being clobbered
on package upgrades.

The usual way out of this are local includes and dh-apparmor is
helpful in deploying them and writing the empty skeleton file as
needed.

But in the case above what one needs is not a local include for a
profile like /etc/apparmor.d/<profile> but instead a local include for
an abstraction.

In apparmor itself all is fine, if
/etc/apparmor.d/abstractions/libvirt-qemu does "#include
<local/abstractions/libvirt-qemu>" and you have a file in
/etc/apparmor.d/local/abstractions/libvirt-qemu that works.

But dh-apparmor as of today does break if you pass it
"abstractions/libvirt-qemu" to take care for. Therefore - up to now -
we've handled that in libvirt maintainer scripts (in Ubuntu) and are
starting to do so in Debian as well. But it would be nice and useful
if we could at some point use dh-apparmor for these just the same as
one would do with any other local include.

#979500#10
Date:
2021-01-07 17:33:16 UTC
From:
To:
Hello,

I'd argue that this is a problem that is already solved ;-)

Starting with AppArmor 3.0, all[1] upstream abstractions come with a
rule like (example taken from abstractions/base):

    include if exists <abstractions/base.d>

so if you create that directory and place a file there, it will be
included by the abstraction.

You don't need to provide those directories or dummy files via the
package, and in fact I'd say that they should only be created when
really needed to keep /etc/apparmor.d/ readable.

(Obviously, if a program needs to extend a specific abstraction,
packaging an   abstractions/$abstraction.d/$package   file makes sense.)


For abstractions shipped by individual package (like libvirt), it would
also make sense to add an   include if exists <abstractions/$whatever.d>
rule to make it easy to add something to an abstraction.



Note: up to AppArmor 2.13.x, the aa-* tools (aa-logprof etc.) break in
funny ways when hitting   include if exists   rules, and sadly that's
not easy to fix (ETOOBIGPATCH). Therefore I'd recommend not to backport
include if exists   rules to profiles or abstractions in older distros.

The aa-* tools from AppArmor 3.x fully support   include if exists
rules.


Regards,

Christian Boltz

[1] The only exception is abstractions/ubuntu-browsers because (for
    historic reasons) an abstractions/ubuntu-browsers.d directory
    already exists and is used in a different way.

#979500#15
Date:
2021-01-08 06:55:12 UTC
From:
To:
Hi,

Christian Boltz (2021-01-07):
I understand, it can happen in libvirt upstream, will benefit even
non-Debian distros, and does not require modifying dh-apparmor.

Christian Ehrhardt, how does it sound? Any reason why the approach you
initially suggested on this bug report is better?

Cheers!

#979500#26
Date:
2021-02-06 06:57:24 UTC
From:
To:
Hi,

intrigeri (2021-01-08):

Ping?

I'd like to add that one of the reasons for adding support for
"include if exists" in AppArmor upstream was to cancel the need for
distros to manage local override files via packaging machinery,
which long term will allow us to simplify things like dh-apparmor,
making them easier to maintain and to use :)

#979500#31
Date:
2021-02-08 10:32:15 UTC
From:
To:
one in my inbox-cracks.
Thanks Intrigeri for the ping.

I'm already part of the crowd waiting for "Include if exists" to be
widely available.
And yes, that would solve my problem as well.

But IMHO a huge problem with "Include if exists" is, that on older
apparmor it totally breaks the rule parsing.
That makes it hard to fully jump onto the new feature yet:
- upstreams don't know how far back their SW will be built, this would
need to become at least a build time version/feature check against
apparmor
- distro-packaging often enough is used for backports, where again
we'd need code to handle old and new feature sets

But thinking more about it I think I still agree that we can close this bug.
That is because in the (hopefully few) places we need this we can
handle it (a bit ugly) in the maintscripts.
If we'd fully support it in dh-apparmor it might encourage people "too
much" to use that instead of the hopefully better future of
"include-if-exists".

#979500#36
Date:
2021-04-02 10:08:11 UTC
From:
To:
Hi,

Christian Ehrhardt (2021-02-08):

I hear you and I understand this set of conflicting constraints is
difficult to disentangle :/

This makes sense to me. I'm marking this bug as wontfix for now,
so that other folks who wonder why dh-apparmor lacks this feature can
find the answer.

Thank you all for the constructive discussion,
cheers!