On 1/14/26 20:39, Erik Huelsmann wrote:
> Hi Michal,
>
> Thank you for your super quick response - including patches, no less!
>
>>> Several months ago, I ran into issue #135 which says that Qemu under
>>> AppArmor can't access LVM volume disks.
>
>> The problem here is, that AppArmor driver in libvirt generates the
>> profile in qemuSecurityGenLabel() phase. Long story short, starting up a
>> libvirt domain is broken down into several steps. One of them is
>> qemuProcessPrepareDomain() where domain XML is filled with (parts of)
>> live data. In this specific case, disk source is translated (from
>> storage pool/vol to a filepath). But it's not just that, various other
>> paths are generated (e.g. socket paths, render nodes for graphics,
>> etc.).
>
> Thanks for this context. Yes, I was unaware of that situation and
> completely focussed at the volume paths. Taking the extra paths into
> account complicates the matter.
>
>> Then, the next step is qemuProcessPrepareHost() where those files from
>> previous step are created, their seclabels are (possibly) set. This has
>> a caveat though.
>
>> Initially, libvirt cared about SELinux only (leave traditional uid/gid
>> perms (we call them DAC) aside for a brief moment). Here. an unique
>> seclabel is generated in the domain prepare phase, so that any helper
>> process that's used to create files in host prepare phase can run under
>> that label (and thus be restricted).
>
>> AppArmor does not work this way. So when an aforementioned helper
>> process wants to run (say /usr/bin/swtpm_setup) it already needs profile
>> to be defined. As "obvious" workaround, AppArmor generates the profile
>> in seclabel generation phase. But by that time path are not populated!
>
> I saw your patches. From the patches I concluded that the security
> policy is strictly applicable to Qemu (apparently). That greatly
> simplifies the solution space, but with your proposed patches, I see
> that the learning curve would have been too steep for me to be able to
> come up with a solution this simple.
Yeah, learning curve is a bit steep, libvirt's a large project after all.
>
>> IMO, quick and dirty fix would be to generate seclabels at the end of
>> qemuProcessPrepareDomain() instead of beginning (see below). The only
>> downside of this is that seclabels won't be available upfront, e.g. if
>> we want to copy them into a device specific seclabel (we do NOT do that
>> though).
>> diff --git i/src/qemu/qemu_process.c w/src/qemu/qemu_process.c
>> index a53bb40783..00b9a732f3 100644
>> --- i/src/qemu/qemu_process.c
>> +++ w/src/qemu/qemu_process.c
>> @@ -7024,15 +7024,6 @@ qemuProcessPrepareDomain(virQEMUDriver *driver,
>> return -1;
>>
>> if (!(flags & VIR_QEMU_PROCESS_START_PRETEND)) {
>> - /* If you are using a SecurityDriver with dynamic labelling,
>> - then generate a security label for isolation */
>> - VIR_DEBUG("Generating domain security label (if required)");
>> - if (qemuSecurityGenLabel(driver->securityManager, vm->def) < 0) {
>> - virDomainAuditSecurityLabel(vm, false);
>> - return -1;
>> - }
>> - virDomainAuditSecurityLabel(vm, true);
>> -
>> if (qemuProcessPrepareDomainNUMAPlacement(vm) < 0)
>> return -1;
>> }
>> @@ -7154,6 +7145,17 @@ qemuProcessPrepareDomain(virQEMUDriver *driver,
>> }
>> }
>>
>> + /* Keep this as the very last step because AppArmor already generates
>> + * profile at this point. IOW, we need all the paths filled in. */
>> + if (!(flags & VIR_QEMU_PROCESS_START_PRETEND)) {
>> + VIR_DEBUG("Generating domain security label (if required)");
>> + if (qemuSecurityGenLabel(driver->securityManager, vm->def) < 0) {
>> + virDomainAuditSecurityLabel(vm, false);
>> + return -1;
>> + }
>> + virDomainAuditSecurityLabel(vm, true);
>> + }
>> +
>> return 0;
>> }
>>
>>
>>
>> Another alternative is to move profile generation into its separate
>> step. So then we'd have:
>>
>> qemuProcessPrepareDomain():
>> 1) gen seclabel /* here only SELinux/DAC drivers would do something
>> useful. For AppArmor it'd be a NOP. */
>> 2) generate all the paths
>> 3) write profile /* only AppArmor would act upon, for SELinux and AppArmor
>> it'd be nop. */
>>
>> Let me see if that'd would fix the issue.
>
> From the fact that you submitted the patches, I think I understand the
> change above didn't work.
It did, but I'm not sure about all the implications of it. I mean, if
there's something, hidden under a dozen of function calls from
qemuProcessPrepareDomain() that expects seclabel to be generated, then
moving the generation at the end is no good.
> Is there anything I can do to help this
> patch set?
You can test the patch set and if it works you can reply to the cover
letter with your Tested-by tag.
Michal
