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.
> 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. Is there anything I can do to help this
patch set?
--
Bye,
Erik.
http://efficito.com -- Hosted accounting and ERP.
Robust and Flexible. No vendor lock-in.
