Re: [libvirt] [PATCH] qemu: Label master key file

On Wed, Apr 13, 2016 at 07:58:06PM -0400, Cole Robinson wrote: >On 04/13/2016 11:56 AM, Cole Robinson wrote: >>On 04/13/2016 11:17 AM, Martin Kletzander wrote: >>>When creating the master key, we used mode 0600 (which we should) but >>>because we were creating it as root, the file is not readable by any >>>qemu running as non-root. Fortunately, it's just a matter of labelling >>>the file. We are generating the file path few times already, so let's >>>label it in the same function that has access to the path already. >>> >>>Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; >>>--- >>> src/qemu/qemu_domain.c | 15 ++++++++++++--- >>> src/qemu/qemu_domain.h | 3 ++- >>> src/qemu/qemu_process.c | 2 +- >>> 3 files changed, 15 insertions(+), 5 deletions(-) >>> >> >>ACK, makes sense and fixes things for me. One comment below >> >>>diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c >>>index 5d54fffcfb98..83e765ef6868 100644 >>>--- a/src/qemu/qemu_domain.c >>>+++ b/src/qemu/qemu_domain.c >>>@@ -504,11 +504,13 @@ qemuDomainGetMasterKeyFilePath(const char *libDir) >>> * Returns 0 on success, -1 on failure with error message indicating failure >>> */ >>> static int >>>-qemuDomainWriteMasterKeyFile(qemuDomainObjPrivatePtr priv) >>>+qemuDomainWriteMasterKeyFile(virQEMUDriverPtr driver, >>>+ virDomainObjPtr vm) >>> { >>> char *path; >>> int fd = -1; >>> int ret = -1; >>>+ qemuDomainObjPrivatePtr priv = vm->privateData; >>> >>> if (!(path = qemuDomainGetMasterKeyFilePath(priv->libDir))) >>> return -1; >>>@@ -525,6 +527,10 @@ qemuDomainWriteMasterKeyFile(qemuDomainObjPrivatePtr priv) >>> goto cleanup; >>> } >>> >>>+ if (virSecurityManagerDomainSetDirLabel(driver->securityManager, >>>+ vm->def, path) < 0) >>>+ goto cleanup; >>>+ >>> ret = 0; >>> >> >>I looked briefly at fixing this but know if there was a function to ask the >>security driver 'just set a on this arbitrary path'. I saw DirLabel but was >>thrown off by the 'Dir' name. Maybe change it to something more generic? >> Yes, it's just a naming; I had to add it for similar purpose when labelling directories, but it "Just Works" for arbitrary path. I'll rename that. > >Also adding some CC, I'm guessing virt-aa-helper.c needs to be extended to to >allow access to $libDir/master-key.aes > Actually, it shouldn't. It's in the per-domain directory and everything under that should be available.

Anyway, it's a pity that we're not very likely to have a test case for that. But couldn't the paths in virt-aa-helper be created from a security driver? It knows all the paths, doesn't it?