Re: [PATCH v3 4/4] virt-aa-helper: store dynamically generated rules
On Wed, Jan 15, 2025 at 11:49:43AM -0300, Georgia Garcia wrote:
On Tue, 2025-01-14 at 12:13 -0600, Andrea Bolognani wrote:
> Going by the example presented in [1], IIUC your change would make it
> so the lines needed for macvtap use, specifically
>
> "/dev/net/tun" rwk,
> "/dev/tap82" rwk,
>
> would be written to .runtime_files instead of .files. That's good
> enough to safeguard them from disappearing when disks are unplugged,
> but what if the macvtap interface itself is? Wouldn't those lines
> linger around despite being no longer needed?
Yes, they would, and that is the current behavior - if you remove only
the macvtap, it will not be removed from .files
That's the current limitation because there are no security hooks
called when macvtap devices are unplugged. I thought it would be better
to be over-permissive (fd permissions linger throughout the runtime of
the vm) than over-restrictive to fix the issue given what's available
in the security side of libvirt.
From a functional standpoint sure, leaving rules behind is not a
problem. But considering that the whole point of a security driver is
to prevent the QEMU process from accessing resources that it
shouldn't have access to, I would consider it a pretty serious flaw.
> So I think we really need a --remove-file option that can be
used to
> carefully undo the changes applied by an earlier use of --add-file.
>
> Unfortunately this will likely involve a far more significant rework
> of the AppArmor driver, and we will certainly have to be careful
> about not introducing regressions in the process, but I'm really not
> a fan of half measures unless the trade-off is overwhelmingly stacked
> in their favor...
As I said earlier, it would also involve the addition of at least one
security hook, impacting all security drivers. But yes, this change
would basically involve rewriting the entire AppArmor driver and a part
of virt-aa-helper. While I'm not against it, unfortunately I will not
be able to dedicate the amount of time needed for such a significant
change.
I haven't looked in detail at how much work adding the ability to
remove rules on device unplug would require, but surely "basically
rewrite the entire driver" is an overexaggeration?
Look, I understand that you probably just want to fix the issue
that's affecting your customers then move on with your life, and
generally speaking I don't really have a problem with partial fixes
that merely get us closer to the solution instead of all the way
there.
However, the changes you're proposing here alter how the driver
operates in a pretty fundamental and, critically, user-visible way.
I'm not keen on switching to a new approach while already being aware
of the fact that a full fix with require yet another pivot...
--
Andrea Bolognani / Red Hat / Virtualization