On Thu, Feb 13, 2025 at 03:36:00PM +0100, Stefano Brivio wrote:
> On Thu, 13 Feb 2025 09:16:33 +0100 Christian Ehrhardt <christian.ehrhardt@xxxxxxxxxxxxx> wrote:
> > On Wed, Feb 5, 2025 at 6:22 PM Andrea Bolognani <abologna@xxxxxxxxxx> wrote:
> > > After looking into the situation, I am convinced that AppArmor
> > > confinement never really worked for unprivileged VMs. The whole
> > > mechanism is built around the concept of per-VM profiles that are
> > > dynamically generated and registered, but doing so requires write
> > > access to /etc/apparmor.d/ and in general permissions that
> > > unprivileged libvirt will by design not have.
> >
> > It becomes clear that, while it works well for the use cases that existed
> > when Jaimie was developing it, it no longer does so for many modern
> > approaches.
Session libvirt has just not received as much love as system libvirt
in general over the years. This is changing somewhat now that we have
a very prominent user (KubeVirt) and major improvements to external
components (passt) make using it a more viable prospect.
> So, about passt, the compromise we now have upstream (Debian packages
> coming in a bit) is probably the quickest way to restore functionality,
> but it's also the most... compromising, because passt isn't necessarily
> started by libvirt, and yet it's now going to allow read-write access to
> @{run}/user/[0-9]*/libvirt/qemu/run/passt/*, libvirt or not, and to
> execute itself.
Agreed. passt is just not the right component to address this. The
workaround you've applied works in a pinch, but it's very obviously a
kludge that we should aim to make unnecessary sooner rather than
later.
> > For the time being I think we should be ok with (non-too awkward)
> > compromise solutions. While it reduces the pain that would force
> > to do the big rework, it keeps users functional and that is what we
> > should care about the most.
> > I'm even tempted to suggest accepting [1] without insisting on too
> > big of a rework for the same reasons.
I've already expressed my opinion about that patch series. Believe
me, I don't enjoy blocking the fix any more than you and Georgia do,
but I'm genuinely convinced that changing how the driver works in
such a fundamental way just to address a single, somewhat narrow
failure scenario would prove to be a mistake in the long run :(
> > But even after the fourth time I like the compromise that you proposed.
> > It is much better than not isolating them at all, after all libvirtd itself
> > also has a liberal profile and despite being so "open" has prevented
> > quite some already.
>
> ...and, by the way, at least nothing is running as root with that
> profile.
>
> In this case, a strong isolation between users is already provided even
> without AppArmor. We just wouldn't isolate between VMs run by the same
> user.
To be clear, I like the proposed approach too. It doesn't really
change how the driver operates - if anything, it makes session mode
and system mode more alike than ever - and takes us pretty much as
far as we can reasonably go in terms of isolation given the
requirement to keep things unprivileged.
It should be reasonably straightforward to implement too. Is anyone
willing to take a stab at it? I'm afraid I can't commit the time
myself for the foreseeable future, though I should be able to
accommodate at least reviewing any patches that might show up.
--
Andrea Bolognani / Red Hat / Virtualization
