On 1/15/26 11:24, Peter Krempa wrote:
> On Thu, Jan 15, 2026 at 11:14:04 +0100, Michal Prívozník wrote:
>> On 1/15/26 08:49, Peter Krempa wrote:
>>> On Thu, Jan 15, 2026 at 08:38:36 +0100, Michal Prívozník via Devel wrote:
>>>> On 1/14/26 20:39, Erik Huelsmann wrote:
>>>>> Hi Michal,
> 
> [...]
> 
>> Another idea I had to fix this issue is: with AppArmor libvirt generates
>> basically two files (both under /etc/apparmor.d/libvirt/):
>>
>> 1) libvirt-$UUID
>> 2) libvirt-$UUID.files
>>
>> where the first one basically just includes abstractions/libvirt-qemu
>> AND libvirt-$UUID.files. The libvirt-$UUID.files then contains all those
>> domain specific paths, like:
>>
>> /run/libvirt/qemu/swtpm/1-guest-swtpm.sock
>> /var/lib/libvirt/qemu/domain-1-guest/fs0-fs.sock
>>
>> and so on (intentionally picked interesting paths to illustrate my
>> point). Thing is, this list of paths is constructed in two ways:
>>
>> a) in qemuProcessPrepareDomain() during qemuSecurityGenLabel() from
>> domain XML (we know this is problematic),
>> b) in qemuProcessPrepareHost() where qemuSecurityDomainSetPathLabel()
>> ends up executing virt-aa-helper in such fashion, that the domain XML on
>> its input is ignored, but desired path is appended into the
>> libvirt-$UUID.files. In the end, the libvirt-$UUID.files is a list of
>> paths with mixed sources (some come from domain XML, others from runtime
>> decisions). With that, it's impossible to keep the profile true. So what
>> if we split the file? We'd have:
>>
>> 1) libvirt-$UUID
>> 2) libvirt-$UUID.files
>> 3) libvirt-$UUID.internal
>>
>> Now, libvirt-$UUID would just include the other two files,
>> libvirt-$UUID.files would contain all the paths collected from domain
>> XML and thus can be regenerated whenever domain XML changes, and
>> finally, libvirt-$UUID.internal would then contain those specific,
>> internal paths like /var/lib/libvirt/qemu/domain-1-guest/master-key.aes
>> for instance.
> 
> I've just reviewed some patches which tried to add some data to "status
> XML" for labelling with apparmor.
> 
> One of the patches tried to store paths to the TAP devices we create.
> Now it was broken because it stored paths containing PID of virtqemud,
> which makes no sense because we close the FDs not long after and also
> changes if virtqemud restarts.

I've seen those patches and I plan onto reviewing them, but mind you -
virt-aa-helper is fed live XML, NOT status XML:

static int
load_profile(virSecurityManager *mgr G_GNUC_UNUSED,
             const char *profile,
             virDomainDef *def,
             const char *fn,
             bool append)
{
    ...

    if (virDomainDefFormatInternal(def, NULL, &buf,
                                   VIR_DOMAIN_DEF_FORMAT_SECURE |

VIR_DOMAIN_DEF_FORMAT_VOLUME_TRANSLATED) < 0)
        return -1;

    xml = virBufferContentAndReset(&buf);
    ...
    cmd = virCommandNewArgList(VIRT_AA_HELPER,
                               create ? "-c" : "-r",
                               "-u", profile, NULL);
    ...
    virCommandSetInputBuffer(cmd, xml);
    return virCommandRun(cmd, NULL);
}

Anyway, let's have this discussion there.

Michal

Reply via email to