[PATCH] apparmor: Add support for local profile customizations

[Jim Fehlig]

On 6/23/23 07:11, Andrea Bolognani wrote:
> On Thu, Jun 22, 2023 at 03:03:56PM -0600, Jim Fehlig wrote:
>> On 6/22/23 11:08, Jim Fehlig wrote:
>>> On 6/22/23 08:50, Andrea Bolognani wrote:
>>>> On Thu, Jun 08, 2023 at 10:37:43AM -0600, Jim Fehlig wrote:
>>>>> I assumed users would make VM customizations in the per-VM profiles. And I
>>>>> suppose overrides of abstractions seems a little odd to me, but that's
>>>>> subjective :-). I'm fine adding it if there's agreement.
>>>>
>>>> The per-VM profile is generated at runtime based on the template, no?
>>>> AFAIK there is no way for the admin to inject changes that affect a
>>>> single VM, but I could be wrong about this.
>>>
>>> The per-VM profile is only generated once, right? So in theory admins
>>> could amend existing per-VM profiles with custom config.
>>
>> While working on this I noticed there is no
>> /etc/apparmor.d/local/abstractions directory on SUSE-based distros. A lot of
>> packages put files in /etc/apparmor.d/local, but I couldn't find any adding
>> files to /etc/apparmor.d/local/abstractions. Nor could I find any apparmor
>> documentation regarding the use of that directory. Do you know if it's
>> common practice? Or is that Debian patch the only prior art?
> 
> In Debian, the directory and the files within it are created as part
> of the 'postinst' maintainer script. This makes some amount of sense,
> as having these files managed by the package manager would kinda
> defeat the purpose.
> 
> Unfortunately, it also makes it way harder to figure out whether any
> packages in the archive besides libvirt are implementing this
> pattern :(
> 
> I've come away empty-handed from my attempts, but I have also found a
> number of references[1] to libvirt's local abstractions overrides in
> other project's code and documentation, which IMO validates the idea
> that such a feature is indeed useful and should be available to all
> distros that use AppArmor, not just the Debian-based ones.

Agreed.

> That said, I've also found a comment in a somewhat recent Debian
> bug[2] that explains how support for this pattern comes
> out-of-the-box with AppArmor 3.0, with the caveat that it expects
> abstractions/foo.d/bar instead of local/abstractions/foo. Easy enough
> to adopt for upstream/SUSE, though Debian will have to consider how
> to carefully migrate things.

Thanks a lot for all the sleuthing! I'm fine with the apparmor 3.0 approach and 
will consider supporting it in SUSE distros where apparmor >= 3.0.

> The catch is that apparently the "include if exists" statement
> doesn't work well before 3.0, and our support matrix will include
> distros that are still on AppArmor 2.x for a couple more years :(

That is unfortunate :-(.

> However, not only you've added a few such statements in your recent
> commit 9b743ee19053, but I myself have done the same a couple months
> back with commit 7a39b04d683f, as part of enabling passt support. So
> in a way we've already started depending on AppArmor 3.0, in open
> contrast with our platform support policy.
> 
> I'm quite unclear on the best way forward :(

I'd prefer to defer support for local customizations of abstractions until 
upstream libvirt can support apparmor >= 3.0. In the meantime commit 
9b743ee19053 can be changed to 'include <local/foo>' since we provide local/foo. 
We'd need to drop the include entirely from your commit, and again defer until 
upstream supports apparmor >= 3.0.

Regards,
Jim