On Mon, Nov 17, 2025 at 08:50:30AM -0600, Wesley Hershberger wrote: > Hi all, > > I'm working on Gitlab #692 [1] and would appreciate a quick > discussion/review/sanity check of the direction I'm planning to take before > I invest many days in a solution that misses some major consideration. > > I'll provide a summary of the issue here and then lay out the two approaches > I'm considering. > > The AppArmor sVirt driver creates & loads a dynamic AppArmor profile by > passing > a domain's XML description to a separate program `virt-aa-helper` (see [2] for > more details). > > When a device is hotplugged to a domain, the `virt-aa-helper` is called with a > flag (`-F`) which appends a rule for that device's backing resource to the > profile. The profile is reloaded from the XML description whenever a device is > hotunplugged. > > However, the XML description for some devices does not contain enough > information to create the corresponding AppArmor rule. When a profile > containing > a dynamic rule is reloaded, the dynamic rule is lost, and the device fails > (this > has been observed with macvtap devices). > > A similar situation occurs when temporary write access to snapshot layers is > needed during block-commit; the `virt-aa-helper` is called with `-F` to add > the > temporary access and the profile is reloaded at the end of the operation to > remove it again. However, this can have the unintended side-effect of wiping > out other temporary accesses (for concurrent blockcommits) or removing access > for devices added with `-F` (macvtap). > > A solution must: > - Be persistent across libvirtd restarts > - Correctly synchronize concurrent profile modifications > > We've conceived of two broad approaches to this issue: > 1. Add fields to the domain XML schema (and persist them) so that the profile > can be correctly generated using only the domain's XML description. > 2. Create a separate XML file > `/etc/apparmor.d/libvirt/libvirt-UUID-dynamic.xml` > with only the information needed to produce the dynamic rules; access would > be synchronized by a lock in libvirtd. > `virt-aa-helper` would consume this file every time it runs; the file would > be removed on Domain shutdown. > > Option 1 (Add fields to Domain schema) Concerns: > ================================================ > - User-visible schema changes > - Do additional fields make sense in the domain schema? e.g. > - a macvtap device path `/dev/tapX` > - permissions info in `backingStore` elements > - It may be non-trivial to chase down every device that could be affected by > this issue and plumb through the additional fields (i.e. a complete solution > may not be possible) > > Option 2 (Separate AppArmor XML) Concerns: > ========================================== > - User-visible additional file > - Not generalizable for (future) sVirt drivers that depend on synchronizing a > global resource > - May require adding remove/restore hooks for the following sVirt interfaces: > - virSecurityDomainSetImageFDLabel domainSetSecurityImageFDLabel; > - virSecurityDomainSetTapFDLabel domainSetSecurityTapFDLabel; > > I'd appreciate any input you'd be able to provide regarding the above > trade-offs; specifically looking for guidance on what you would consider > merging > (in contrast to previous approaches [3]).
Hey Wesley, I was involved in the discussion back in January[3] and my position remains the same: regenerating the profile from scratch every time an entry needs to be removed feels terribly fragile. We should instead switch to an approach where items are removed when no longer needed, just as they currently get added on-demand. Among the options you offered for storage, I'm definitely partial to the first one. Any information about files that QEMU needs access to is going to be relevant not just to AppArmor, but to other security modules as well, so it doesn't make sense to relegate it to an AppArmor-specific file. Never mind the potential challenges inherent with keeping things in sync. It doesn't matter if the implemented solution doesn't cover all possible cases right away: the remaining issues can be dealt with over time, as they are discovered. And I don't see how that would be a unique challenges of Option 1 compared to Option 2 anyway :) > [1] https://gitlab.com/libvirt/libvirt/-/issues/692 > [2] https://gitlab.com/apparmor/apparmor/-/wikis/Libvirt > [3] https://www.mail-archive.com/[email protected]/msg07703.html -- Andrea Bolognani / Red Hat / Virtualization
