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
