Re: [PATCH 1/3] qemu_passt: Don't make passt transition to svirt_t/virt_domain on start

On Tue, Feb 21, 2023 at 10:49:46PM +0100, Stefano Brivio wrote: > On Tue, 21 Feb 2023 19:43:33 +0000 > Daniel P. Berrangé <berrange(a)redhat.com&gt; wrote: > > > On Tue, Feb 21, 2023 at 08:19:05PM +0100, Stefano Brivio wrote: > > > qemuSecurityCommandRun() causes an explicit domain transition of the > > > new process, but passt ships with its own SELinux policy, with > > > external interfaces for libvirtd, so we simply need to transition > > > from virtd_t to passt_t as passt is executed. The qemu type > > > enforcement rules have little to do with it. > > > > Can you clarify the difference here ? > > Between...? > > I mean, virCommandRun() will just keep things running under virtd_t, so > that passt later can transition to passt_t. > > With qemuSecurityCommandRun(), there would be a transition from virtd_t > to svirt_t (it's the function that's called to start qemu, but > shouldn't be called here), and not to passt_t. > > But I'm not really sure that's what you were asking for. Yes, it is. > > > Runing passt under 'svirt_t' is not desirable as that allows > > many actions that are only relevant to QEMU. > > Right, that's what this patch avoids. There are also actions, such as > starting passt or killing it, that we don't want to allow QEMU to do. > > > Running passt under the MCS label that is associated with the > > VM is highly desirable though. Two passt instances belonging > > to separate VMs are isolated from each other if they each use > > the VM specific MCS label, than if they use the global default > > MCS label. > > > > To use the VM specific MCS label would require libvirt to > > explicitly set the desired selinux label on exec, it can't > > happen automatically via an SELinux transition rule. > > > > We do stil want to use the passt_t type though. > > > > IOW, if we have a VM running > > > > svirt_t:s0:c710,c716 > > > > IMHO we would its corresponding passt instance to be > > running > > > > passt_t:s0:c710,c716 > > > > > > not > > > > passt_t:s0:c0.c1023 > > Practically speaking, it doesn't make a huge difference for passt > because it unshares any relevant namespace right after it starts -- > that's *in theory* a strictly stronger isolation compared to what > SELinux provides (at least once we reach the main loop). Even docker/podman will apply SELinux isolation per container, rather than only relying on namespaces.

> But it makes sense, and I guess we should relabel to a specific MCS > with still 'virtd_t' as a type, then later the domain would transition > to passt_t. This could probably be done as an extension of > qemuSecurityCommandRun(), I haven't looked into it yet. I will. > > Anyway, right now, I think this provides better security than > 'setenforce 0', which is the only way to run passt from libvirt at the > moment on some distributions. If running with SELinux permissive, this patch has no effect, as it is unconfined regardless. That's not a situation we care about. If people want to run without protection they get to keep the pieces when it all goes wrong.

> I'm not sure how you handle these cases on libvirt, but generally > speaking, this patch is a vast improvement on the current situation, > and I can follow up with an extension or a different version of > qemuSecurityCommandRun() later. No, I don't think it is a vast improvement. The goal of sVirt is to guarantee isolation between VMs. Running passt under svirt_t:MCS is not ideal because the svirt_t policy allows things that passt should not get. It does still guarantee isolation between VMs, because the MCS label is present.

Switching to running passt_t:c0.c1023 will be more correct in terms of what permissions passt should be allowed, but it has disabled isolation of passt between VMs. This is a clear degradation of capabilities from the POV of sVirt's goals.